ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 9 Packages Projects Releases Wiki Activity

test(desktop): move NS*UsageDescription pin from pytest to Vitest (tests-js)

Browse Source 
Address maintainer review feedback (PR #66215, comment by @teknium1):

> `tests/test_desktop_mac_entitlements.py:47` reads `apps/desktop/package.json`
> from pytest. `AGENTS.md:1319-1329` requires assertions about `package.json`
> and JS-side artifacts to be in the JS/Vitest suite; otherwise CI
> classification can skip the regression test on a JS-only change.

The CI change classifier (`scripts/ci/classify_changes.py`) marks
`apps/desktop/package.json` as `_FRONTEND` (in `_PY_SKIP`), so a Python test
that reads it would be skipped on a JS-only PR — regression goes green on
the PR, red on main.

Move the regression to `tests-js/desktop-mac-usage-descriptions.test.ts`,
following the same convention as commit dbf86b923 ("test: port macOS
entitlements test from Python to vitest"), which ports an earlier Python
entitlements regression into `tests-js/desktop-mac-entitlements.test.ts`
for the identical reason. The new file is a sibling of that one — both
pin Desktop macOS manifest contracts, but they assert against different
files (`entitlements.mac.plist` vs `build.mac.extendInfo` in package.json).

The Vitest port mirrors the original assertions 1:1: every
`NS*UsageDescription` key pinned (parametrized over key + required
substring + reason), no leading/trailing whitespace or newlines in any
`extendInfo` string, and a drift-protection assertion that fails when a
new privacy key is added to the build config without a matching row.

A runtime type guard on `extendInfo` ensures a non-string plist scalar
raises a clean assertion error here ("`X` in build.mac.extendInfo must
be a string (got boolean)") rather than crashing the test runner with
`value.trim is not a function` deep in the whitespace test — caught by
Flash + GPT-OSS cross-vendor review.

Verified:
- `cd tests-js && npm run check` → typecheck clean, 14/14 tests pass
  (4 files including the new one with 5 tests).
- Mutation: removing `NSAppleMusicUsageDescription` from
  `apps/desktop/package.json` flips 1 test red with the exact symptom
  ("Info.plist privacy usage description \`NSAppleMusicUsageDescription\`
  is missing"). Restore → 14/14 green.
- Mutation: adding an unpinned `NSSpeechRecognitionUsageDescription` with
  whitespace flips 2 tests red (drift-protection + whitespace).
- Mutation: adding a non-string `CFBundleBooleanTest: true` flips the
  whole file red with the clean "must be a string (got boolean)"
  assertion (no downstream crash).
- `apps/desktop` Electron Vitest project still passes (42 files,
  432 tests + 1 skipped).

Closes the maintainer comment thread on PR #66215.

Fixes #54551
This commit is contained in:
David Metcalfe
2026-07-18 16:42:38 -07:00
committed by Teknium
parent f0e9902664
commit b2ed58c415
2 changed files with 180 additions and 140 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests-js/desktop-mac-usage-descriptions.test.ts
+180
View File
@@ -0,0 +1,180 @@
/**
* Regression for #54551: macOS Info.plist privacy usage descriptions
* declared by the Desktop electron-builder config
* (`apps/desktop/package.json -> build.mac.extendInfo`) must pin every
* `NS*UsageDescription` key the renderer relies on.
*
* Each entry is a key/value pair that lands in the packaged Hermes.app's
* Info.plist via electron-builder's `extendInfo` merge. Missing or mis-stated
* keys cause macOS to either silently deny the related API or surface a
* mysteriously-worded system permission prompt at runtime (TCC's
* `kTCCServiceMediaLibrary`, `kTCCServiceAppleEvents`, etc.).
*
* The Desktop renderer initializes Chromium's audio stack on user gesture
* (completion chimes, TTS playback, voice mode). On macOS 26+, that init can
* register the helper with the media subsystem and surface as a
* "Hermes wants to access Music" prompt unless the Info.plist disclaims it
* explicitly. This test pins every usage-description string the desktop
* currently relies on so accidental drops break CI instead of breaking users.
*
* Why this test lives in tests-js/, not tests/*.py
* -------------------------------------------------
*
* `AGENTS.md:1319-1329` requires assertions about `package.json` and JS-side
* artifacts to live in the JS/Vitest suite: the CI change classifier can
* skip Python coverage on a JS-only PR (the classifier's `python` lane is
* skipped when all paths match `_FRONTEND` or `_PY_SKIP`, both of which
* cover `apps/desktop/package.json`). A regression would then go green on
* the PR and red on `main` where the classifier fails open. See also
* `tests-js/desktop-mac-entitlements.test.ts` which ports an earlier Python
* entitlements regression for the same reason.
*
* Why this test exists
* --------------------
*
* The project has a recurring class of bug: a macOS privacy-sensitive API is
* called at runtime, but the Info.plist doesn't declare the corresponding
* `NS*UsageDescription` key, so the system prompt is either silent (with a
* generic "denied" error to the agent) or worded in a way that confuses the
* user ("Hermes wants to access Music" when Hermes never touches the Music
* library). The closed-PR family (#59486 / its duplicates #59833, #59915,
* #59950, #60013 for Contacts; #39854 for Calendar; #64582 for Reminders)
* established that the right fix shape is: add the key + pin it in a test.
* This file is the canonical test for that pattern at the Desktop layer.
*
* When adding a new NS*UsageDescription key to `build.mac.extendInfo`, add a
* matching row to EXPECTED_USAGE_DESCRIPTIONS below. The drift-protection
* assertion at the bottom of this file will fail otherwise.
*/
import assert from 'node:assert/strict'
import fs from 'node:fs'
import path from 'node:path'
import { test } from 'vitest'
const REPO_ROOT = path.resolve(__dirname, '..')
const DESKTOP_PKG = path.join(REPO_ROOT, 'apps', 'desktop', 'package.json')
interface UsageDescriptionRow {
key: string
requiredSubstring: string
reason: string
}
function desktopPkg(): Record<string, unknown> {
assert.ok(fs.existsSync(DESKTOP_PKG), `missing ${DESKTOP_PKG}`)
return JSON.parse(fs.readFileSync(DESKTOP_PKG, 'utf-8'))
}
function extendInfo(): Record<string, string> {
const pkg = desktopPkg()
const build = (pkg.build ?? {}) as Record<string, unknown>
const mac = (build.mac ?? {}) as Record<string, unknown>
assert.ok(
typeof mac.extendInfo === 'object' &&
mac.extendInfo !== null &&
!Array.isArray(mac.extendInfo),
'build.mac.extendInfo is missing or invalid in apps/desktop/package.json'
)
const extend = mac.extendInfo as Record<string, unknown>
// Narrow to Record<string, string> with a runtime guard — the value type
// for NS*UsageDescription is string, but electron-builder's `extendInfo`
// accepts arbitrary plist scalars (bool, number, array, object) and we want
// a clean assertion error here, not a downstream `value.trim is not a
// function` crash in the whitespace test.
for (const [key, value] of Object.entries(extend)) {
assert.equal(
typeof value,
'string',
`\`${key}\` in build.mac.extendInfo must be a string (got ${typeof value})`
)
}
return extend as Record<string, string>
}
// Each entry: Info.plist key, required substring (case-insensitive), and a
// plain-language reason. The substring check lets future copy edits pass
// while still catching silent drops of the key itself.
const EXPECTED_USAGE_DESCRIPTIONS: UsageDescriptionRow[] = [
{
key: 'NSMicrophoneUsageDescription',
requiredSubstring: 'microphone',
reason: 'Microphone capture is required for voice input mode.'
},
{
key: 'NSAudioCaptureUsageDescription',
requiredSubstring: 'audio',
reason: 'Audio capture backs the voice conversation pipeline.'
},
{
key: 'NSAppleMusicUsageDescription',
requiredSubstring: 'Music',
reason:
"Disclaim MediaLibrary access so the system audio stack does not " +
'surface a misleading Apple Music permission prompt ' +
'(kTCCServiceMediaLibrary) when the renderer initializes audio for ' +
'completion chimes, TTS, or voice.'
}
]
test.each(EXPECTED_USAGE_DESCRIPTIONS)(
'`$key` is declared in build.mac.extendInfo',
({ key, requiredSubstring, reason }) => {
const info = extendInfo()
const value = info[key]
assert.ok(
value !== undefined,
`Info.plist privacy usage description \`${key}\` is missing from ` +
'apps/desktop/package.json build.mac.extendInfo. macOS will surface ' +
'a misleading system prompt or silently deny the related API.\n' +
`Reason: ${reason}`
)
assert.ok(
value.toLowerCase().includes(requiredSubstring.toLowerCase()),
`\`${key}\` exists but does not mention '${requiredSubstring}'. ` +
`Current value: ${JSON.stringify(value)}. Reason: ${reason}`
)
}
)
test('every extendInfo value is free of leading/trailing whitespace and newlines', () => {
const info = extendInfo()
for (const [key, value] of Object.entries(info)) {
assert.equal(
value,
value.trim(),
`\`${key}\` in build.mac.extendInfo has leading/trailing whitespace: ` +
JSON.stringify(value)
)
// electron-builder writes strings as-is; newlines would render as
// literal control chars in the macOS prompt.
assert.ok(
!value.includes('\n') && !value.includes('\r'),
`\`${key}\` contains a newline; macOS will render it as a control ` +
'character in the system permission prompt.'
)
}
})
test('every NS*UsageDescription in extendInfo is pinned in this test', () => {
const info = extendInfo()
const declaredKeys = new Set(EXPECTED_USAGE_DESCRIPTIONS.map((row) => row.key))
// Non-privacy keys (CFBundleDisplayName etc.) are exempt — this test
// only governs NS*UsageDescription entries.
const privacyKeysInPlist = new Set(
Object.keys(info).filter(
(k) => k.startsWith('NS') && k.endsWith('UsageDescription')
)
)
const missing = [...privacyKeysInPlist].filter((k) => !declaredKeys.has(k))
assert.deepEqual(
missing,
[],
`extendInfo declares privacy usage keys ${JSON.stringify(missing.sort())} ` +
'that this test does not pin. Add them to EXPECTED_USAGE_DESCRIPTIONS ' +
'with a reason, or remove them from the build config.'
)
})
tests/test_desktop_mac_entitlements.py
-140
View File
@@ -1,140 +0,0 @@
"""Pin macOS Info.plist privacy usage descriptions declared by the Desktop
electron-builder config (`apps/desktop/package.json -> build.mac.extendInfo`).
Each entry is a key/value pair that lands in the packaged Hermes.app's
Info.plist via electron-builder's `extendInfo` merge. Missing or mis-stated
keys cause macOS to either silently deny the related API or surface a
mysteriously-worded system permission prompt at runtime (TCC's
`kTCCServiceMediaLibrary`, `kTCCServiceAppleEvents`, etc.).
The Desktop renderer initializes Chromium's audio stack on user gesture
(completion chimes, TTS playback, voice mode). On macOS 26+, that init can
register the helper with the media subsystem and surface as a "Hermes wants
to access Music" prompt unless the Info.plist disclaims it explicitly. This
test pins every usage-description string the desktop currently relies on so
accidental drops break CI instead of breaking users.
Why this test exists
--------------------
The project has a recurring class of bug: a macOS privacy-sensitive API is
called at runtime, but the Info.plist doesn't declare the corresponding
`NS*UsageDescription` key, so the system prompt is either silent (with a
generic "denied" error to the agent) or worded in a way that confuses the
user ("Hermes wants to access Music" when Hermes never touches the Music
library). The closed-PR family (#59486 / its duplicates #59833, #59915,
#59950, #60013 for Contacts; #39854 for Calendar; #64582 for Reminders)
established that the right fix shape is: add the key + pin it in a test.
This file is the canonical test for that pattern at the Desktop layer.
When adding a new NS*UsageDescription key to `build.mac.extendInfo`, add a
matching row to EXPECTED_USAGE_DESCRIPTIONS below. The drift-protection
assertion at the bottom of this file will fail otherwise.
"""
from __future__ import annotations
import json
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parents[1]
PACKAGE_JSON = REPO_ROOT / "apps" / "desktop" / "package.json"
def _load_extend_info() -> dict[str, str]:
data = json.loads(PACKAGE_JSON.read_text(encoding="utf-8"))
return data["build"]["mac"]["extendInfo"]
@pytest.fixture(scope="module")
def extend_info() -> dict[str, str]:
return _load_extend_info()
# Each entry: (Info.plist key, required substring, plain-language reason).
# Required-substring checks let future copy edits pass while still catching
# silent drops of the key itself.
EXPECTED_USAGE_DESCRIPTIONS: list[tuple[str, str, str]] = [
(
"NSMicrophoneUsageDescription",
"microphone",
"Microphone capture is required for voice input mode.",
),
(
"NSAudioCaptureUsageDescription",
"audio",
"Audio capture backs the voice conversation pipeline.",
),
(
"NSAppleMusicUsageDescription",
"Music",
"Disclaim MediaLibrary access so the system audio stack does not "
"surface a misleading Apple Music permission prompt (kTCCServiceMediaLibrary) "
"when the renderer initializes audio for completion chimes, TTS, or voice.",
),
]
@pytest.mark.parametrize(("key", "required_substring", "reason"), EXPECTED_USAGE_DESCRIPTIONS)
def test_required_privacy_usage_descriptions_are_declared(
extend_info: dict[str, str], key: str, required_substring: str, reason: str
) -> None:
"""Each macOS privacy usage key Hermes relies on must be present in
build.mac.extendInfo, with copy that names the protected resource.
A missing key causes macOS to silently deny the underlying API or surface
a generic system prompt with no usage description, which reads to users
as the app misbehaving. Pin the keys so the next refactor can't drop one
by accident.
"""
value = extend_info.get(key)
assert value is not None, (
f"Info.plist privacy usage description `{key}` is missing from "
f"apps/desktop/package.json build.mac.extendInfo. macOS will surface "
f"a misleading system prompt or silently deny the related API.\n"
f"Reason: {reason}"
)
assert required_substring.lower() in value.lower(), (
f"`{key}` exists but does not mention '{required_substring}'. "
f"Current value: {value!r}. Reason: {reason}"
)
def test_extend_info_keys_have_no_trailing_whitespace(extend_info: dict[str, str]) -> None:
"""electron-builder merges extendInfo into Info.plist verbatim; trailing
whitespace in a usage string renders as a system prompt that breaks off
mid-sentence. Pin the hygiene."""
for key, value in extend_info.items():
assert value == value.strip(), (
f"`{key}` in build.mac.extendInfo has leading/trailing whitespace: {value!r}"
)
# electron-builder writes strings as-is; newlines would render as
# literal control chars in the macOS prompt.
assert "\n" not in value and "\r" not in value, (
f"`{key}` contains a newline; macOS will render it as a control "
f"character in the system permission prompt."
)
def test_extend_info_does_not_silently_drop_unexpected_keys(
extend_info: dict[str, str],
) -> None:
"""If a future PR adds a new privacy-sensitive key, this test will start
failing — forcing the author to update EXPECTED_USAGE_DESCRIPTIONS and
document why the new key is needed. This is the safety net the closed PR
family (#59486, #59915, #59950, #60013) established for the Contacts and
Apple Events keys; the same shape applies here."""
declared_keys = {key for key, _, _ in EXPECTED_USAGE_DESCRIPTIONS}
# Non-privacy keys (CFBundleDisplayName etc.) are exempt — this test
# only governs NS*UsageDescription entries.
privacy_keys_in_plist = {
key for key in extend_info if key.startswith("NS") and key.endswith("UsageDescription")
}
missing = privacy_keys_in_plist - declared_keys
assert not missing, (
f"extendInfo declares privacy usage keys {sorted(missing)} that this "
f"test does not pin. Add them to EXPECTED_USAGE_DESCRIPTIONS with a "
f"reason, or remove them from the build config."
)