From b2ed58c415a54a6ddf91c9b1909df3a91d172db9 Mon Sep 17 00:00:00 2001 From: David Metcalfe <80915+DavidMetcalfe@users.noreply.github.com> Date: Sat, 18 Jul 2026 16:42:38 -0700 Subject: [PATCH] test(desktop): move NS*UsageDescription pin from pytest to Vitest (tests-js) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../desktop-mac-usage-descriptions.test.ts | 180 ++++++++++++++++++ tests/test_desktop_mac_entitlements.py | 140 -------------- 2 files changed, 180 insertions(+), 140 deletions(-) create mode 100644 tests-js/desktop-mac-usage-descriptions.test.ts delete mode 100644 tests/test_desktop_mac_entitlements.py diff --git a/tests-js/desktop-mac-usage-descriptions.test.ts b/tests-js/desktop-mac-usage-descriptions.test.ts new file mode 100644 index 0000000000..88af6e2bad --- /dev/null +++ b/tests-js/desktop-mac-usage-descriptions.test.ts @@ -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 { + assert.ok(fs.existsSync(DESKTOP_PKG), `missing ${DESKTOP_PKG}`) + return JSON.parse(fs.readFileSync(DESKTOP_PKG, 'utf-8')) +} + +function extendInfo(): Record { + const pkg = desktopPkg() + const build = (pkg.build ?? {}) as Record + const mac = (build.mac ?? {}) as Record + 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 + // Narrow to Record 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 +} + +// 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.' + ) +}) \ No newline at end of file diff --git a/tests/test_desktop_mac_entitlements.py b/tests/test_desktop_mac_entitlements.py deleted file mode 100644 index a4efaa2291..0000000000 --- a/tests/test_desktop_mac_entitlements.py +++ /dev/null @@ -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." - )