Fix phone greeting hallucination by sonichi · Pull Request #27 · sonichi/sutando

sonichi · 2026-03-29T02:52:33Z

Enforces exact greeting on inbound calls. Prevents Gemini from inventing scenarios.

Gemini was making up tasks ("could you check my screen?") on inbound calls. Now says exact greeting and waits for user to speak first. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix command injection in sips call (CodeQL js/command-line-injection) Use JSON.stringify for shell argument escaping instead of template literal interpolation. The safePath regex already strips most chars, but proper escaping prevents edge cases. Remaining 15 CodeQL alerts are false positives: - py/path-injection: all guarded by is_relative_to() checks - js/reflected-xss: all user input escaped via esc() before XML - js/clear-text-logging: phone number already masked in output Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review: use execFileSync for sips (no shell, no injection) Per Wenjix's review: sips doesn't support -- as end-of-options, so the previous fix was silently broken (resize became a no-op). Switch to execFileSync with argv array — eliminates shell interpretation entirely. Also fix .replace('.png') to use regex anchor so non-.png inputs don't silently no-op. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add CodeQL #27 citation comment to sips fix Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The ffmpeg call at line 195-198 interpolated `${ffmpegBin}`, `${videoPath}`, `${LIVE_TRANSCRIPT_SRT_PATH}`, and `${outPath}` directly into a shell string. `ffmpegBin` comes from `$FFMPEG_SUBTITLE_BIN` env var; `videoPath`/`outPath` are caller-controlled. Same CodeQL js/command-line-injection class as #27 (already fixed for sips at line 239). Convert to execFileSync with an argv array — no shell parsing, no interpolation surface. The `subtitles=PATH:force_style='...'` string is now a single argv element which ffmpeg's own -vf parser handles (single quotes inside intact). Cold-review finding from notes/cold-review-log.md PR #392 row. typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…title burn + open_file (#408) * fix(recording-tools): use execFileSync for ffmpeg subtitle burn The ffmpeg call at line 195-198 interpolated `${ffmpegBin}`, `${videoPath}`, `${LIVE_TRANSCRIPT_SRT_PATH}`, and `${outPath}` directly into a shell string. `ffmpegBin` comes from `$FFMPEG_SUBTITLE_BIN` env var; `videoPath`/`outPath` are caller-controlled. Same CodeQL js/command-line-injection class as #27 (already fixed for sips at line 239). Convert to execFileSync with an argv array — no shell parsing, no interpolation surface. The `subtitles=PATH:force_style='...'` string is now a single argv element which ffmpeg's own -vf parser handles (single quotes inside intact). Cold-review finding from notes/cold-review-log.md PR #392 row. typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(recording-tools): use execFileSync for `open` and `ffprobe` in open_file Same CodeQL js/command-line-injection class as the ffmpeg subtitle-burn fix in this branch. `open_file`'s `recPath` comes from caller-supplied `filePath` arg (or `findRecording()` output) — was interpolated into: - `execSync(\`open "${recPath}"\`)` line 492 - `execSync(\`/opt/homebrew/bin/ffprobe ... "${recPath}"\`)` line 497 A path containing a `"` would inject. Convert both to execFileSync argv arrays. typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ual app routing (#605) Closes #560. Voice / text users can now direct `open_file` to a specific macOS app instead of always going through the file-type default. Resolves the "open this in Sublime Text", "open the SQLite db in TablePlus" flow. Resolution per issue: 1. Explicit `app` arg → `open -a <app> <path>` 2. No `app` → `open <path>` (macOS LaunchServices picks default) Contextual inference (rule 2 from the issue body — model passes `app` based on prior conversation, even without a literal "with" phrase) is the model's job. The tool description spells out when Gemini should populate `app`; the tool itself only honors what it's told. Argv-form preserved: `open -a "<app>" "<path>"` is invoked via `execFileSync` with both as separate argv entries — no shell interpolation, no command-line-injection risk (same CodeQL `js/command-line-injection` class as #27). Tests: 4 cases — schema shape (optional string, parses with and without `app`), description mentions the `app` trigger phrasing, File-not-found error surfaces correctly both with and without `app`. Did NOT add tests that actually open files (would mutate the test machine's app state). Verified: - 4/4 unit tests pass - `npx tsc --noEmit` clean Co-authored-by: Xueqing Liu <xueqingliu@Xueqings-Mac-Studio.local> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…i#27) Gemini was making up tasks ("could you check my screen?") on inbound calls. Now says exact greeting and waits for user to speak first. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix command injection in sips call (CodeQL js/command-line-injection) Use JSON.stringify for shell argument escaping instead of template literal interpolation. The safePath regex already strips most chars, but proper escaping prevents edge cases. Remaining 15 CodeQL alerts are false positives: - py/path-injection: all guarded by is_relative_to() checks - js/reflected-xss: all user input escaped via esc() before XML - js/clear-text-logging: phone number already masked in output Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review: use execFileSync for sips (no shell, no injection) Per Wenjix's review: sips doesn't support -- as end-of-options, so the previous fix was silently broken (resize became a no-op). Switch to execFileSync with argv array — eliminates shell interpretation entirely. Also fix .replace('.png') to use regex anchor so non-.png inputs don't silently no-op. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add CodeQL sonichi#27 citation comment to sips fix Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…title burn + open_file (sonichi#408) * fix(recording-tools): use execFileSync for ffmpeg subtitle burn The ffmpeg call at line 195-198 interpolated `${ffmpegBin}`, `${videoPath}`, `${LIVE_TRANSCRIPT_SRT_PATH}`, and `${outPath}` directly into a shell string. `ffmpegBin` comes from `$FFMPEG_SUBTITLE_BIN` env var; `videoPath`/`outPath` are caller-controlled. Same CodeQL js/command-line-injection class as sonichi#27 (already fixed for sips at line 239). Convert to execFileSync with an argv array — no shell parsing, no interpolation surface. The `subtitles=PATH:force_style='...'` string is now a single argv element which ffmpeg's own -vf parser handles (single quotes inside intact). Cold-review finding from notes/cold-review-log.md PR sonichi#392 row. typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(recording-tools): use execFileSync for `open` and `ffprobe` in open_file Same CodeQL js/command-line-injection class as the ffmpeg subtitle-burn fix in this branch. `open_file`'s `recPath` comes from caller-supplied `filePath` arg (or `findRecording()` output) — was interpolated into: - `execSync(\`open "${recPath}"\`)` line 492 - `execSync(\`/opt/homebrew/bin/ffprobe ... "${recPath}"\`)` line 497 A path containing a `"` would inject. Convert both to execFileSync argv arrays. typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…contextual app routing (sonichi#605) Closes sonichi#560. Voice / text users can now direct `open_file` to a specific macOS app instead of always going through the file-type default. Resolves the "open this in Sublime Text", "open the SQLite db in TablePlus" flow. Resolution per issue: 1. Explicit `app` arg → `open -a <app> <path>` 2. No `app` → `open <path>` (macOS LaunchServices picks default) Contextual inference (rule 2 from the issue body — model passes `app` based on prior conversation, even without a literal "with" phrase) is the model's job. The tool description spells out when Gemini should populate `app`; the tool itself only honors what it's told. Argv-form preserved: `open -a "<app>" "<path>"` is invoked via `execFileSync` with both as separate argv entries — no shell interpolation, no command-line-injection risk (same CodeQL `js/command-line-injection` class as sonichi#27). Tests: 4 cases — schema shape (optional string, parses with and without `app`), description mentions the `app` trigger phrasing, File-not-found error surfaces correctly both with and without `app`. Did NOT add tests that actually open files (would mutate the test machine's app state). Verified: - 4/4 unit tests pass - `npx tsc --noEmit` clean Co-authored-by: Xueqing Liu <xueqingliu@Xueqings-Mac-Studio.local> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…_CONFIG_DIR The bridges resolve channel config at MODULE level, so it runs during exec_module, before a test can intervene: src/discord-bridge.py:205 channels_env = claude_home_path("channels","discord",".env") src/discord-bridge.py:555 ACCESS_FILE = channel_access_path("discord") channel_access_path() reads $CLAUDE_CONFIG_DIR and falls back to the legacy real-home ~/.claude/channels/<ch>/access.json. A test that never sets CLAUDE_CONFIG_DIR inherits whatever the developer has, so the symptom is machine-dependent: legacy-fallback deprecation warning on a clean box, silent read of the operator real channel allowlist on an operator box. Green everywhere, trustworthy nowhere. Setting a bot token does not help -- that stops the .env read, never the access resolution. Verified by re-running the import with CLAUDE_CONFIG_DIR popped from the env: ACCESS_FILE = /Users/<operator>/.claude/channels/discord/access.json Current tree: 42 bridge-importing tests -- 9 clean, 7 mitigated, 26 violating. The 26 are grandfathered in KNOWN_UNISOLATED and are being migrated onto a shared helper separately (sonichi/sutando #2426, #2428 lineage); this gate exists so #27 cannot be added silently. Two behaviours worth calling out, both found while testing this lint: 1. Isolation is detected as an ASSIGNMENT, never a substring. A first pass keyed on the presence of "CLAUDE_CONFIG_DIR" wrongly exonerated three files that only name it in a comment -- including tests/bridge-audit-wiring.test.py, whose comment CLAIMED hermeticity while the test read live config. Comments lie; predicates should not read them. There is a regression proof for exactly this case. 2. Post-import mitigation is its own, non-fatal category. Seven tests rebind mod.ACCESS_FILE to a temp path after exec_module so their destructive write/unlink cannot touch the real file. The import still resolves host config, so they are not clean -- but hard-failing the authors who thought about it is how lints get switched off. They report as a NOTE. A stale KNOWN_UNISOLATED entry is also a NOTE, not a failure. Hard-failing there is a footgun: the moment a PR fixes a listed file, main goes red until someone edits this script. #2428 fixes tests/bridge-audit-wiring.test.py, and a fatal stale-check would have reddened main on its merge. Evidence (proofs run in-tree, files git-added so git ls-files sees them): new violating test -> exit 1, file named same test, comment only -> exit 1 (comment correctly rejected) same test, real isolation -> exit 0 clean tree -> exit 0, "42 scanned, 26 grandfathered, 7 mitigated" No production code touched; adds one script and one workflow. Stand: Echo Act IV Pro

…_CONFIG_DIR (#2429) * chore(lint): refuse bridge tests that import without isolating CLAUDE_CONFIG_DIR The bridges resolve channel config at MODULE level, so it runs during exec_module, before a test can intervene: src/discord-bridge.py:205 channels_env = claude_home_path("channels","discord",".env") src/discord-bridge.py:555 ACCESS_FILE = channel_access_path("discord") channel_access_path() reads $CLAUDE_CONFIG_DIR and falls back to the legacy real-home ~/.claude/channels/<ch>/access.json. A test that never sets CLAUDE_CONFIG_DIR inherits whatever the developer has, so the symptom is machine-dependent: legacy-fallback deprecation warning on a clean box, silent read of the operator real channel allowlist on an operator box. Green everywhere, trustworthy nowhere. Setting a bot token does not help -- that stops the .env read, never the access resolution. Verified by re-running the import with CLAUDE_CONFIG_DIR popped from the env: ACCESS_FILE = /Users/<operator>/.claude/channels/discord/access.json Current tree: 42 bridge-importing tests -- 9 clean, 7 mitigated, 26 violating. The 26 are grandfathered in KNOWN_UNISOLATED and are being migrated onto a shared helper separately (sonichi/sutando #2426, #2428 lineage); this gate exists so #27 cannot be added silently. Two behaviours worth calling out, both found while testing this lint: 1. Isolation is detected as an ASSIGNMENT, never a substring. A first pass keyed on the presence of "CLAUDE_CONFIG_DIR" wrongly exonerated three files that only name it in a comment -- including tests/bridge-audit-wiring.test.py, whose comment CLAIMED hermeticity while the test read live config. Comments lie; predicates should not read them. There is a regression proof for exactly this case. 2. Post-import mitigation is its own, non-fatal category. Seven tests rebind mod.ACCESS_FILE to a temp path after exec_module so their destructive write/unlink cannot touch the real file. The import still resolves host config, so they are not clean -- but hard-failing the authors who thought about it is how lints get switched off. They report as a NOTE. A stale KNOWN_UNISOLATED entry is also a NOTE, not a failure. Hard-failing there is a footgun: the moment a PR fixes a listed file, main goes red until someone edits this script. #2428 fixes tests/bridge-audit-wiring.test.py, and a fatal stale-check would have reddened main on its merge. Evidence (proofs run in-tree, files git-added so git ls-files sees them): new violating test -> exit 1, file named same test, comment only -> exit 1 (comment correctly rejected) same test, real isolation -> exit 0 clean tree -> exit 0, "42 scanned, 26 grandfathered, 7 mitigated" No production code touched; adds one script and one workflow. Stand: Echo Act IV Pro * fix(lint): drop the ~/.claude literal from the failure message The REVIEW.md hardcoded-paths scan (which runs as a step inside the "tsc + tests (clean install)" job, so the red check name is misleading) flagged scripts/lint-hermetic-bridge-tests.py:218 -- a real per-user path literal inside a runtime message string. The docstring occurrence does not trip it; comment lines are skipped, code strings are not. Rephrased to "per-user channel allowlist", which says the same thing without the literal. Verified locally the way CI does it -- `git diff origin/main...HEAD` piped to `bash scripts/review-checks.sh --diff` -- rather than a bare run, which reports "empty diff -- nothing to check" and silently proves nothing. Caught by @Sutando-Mini on the PR. Stand: Echo Act IV Pro * test(lint): cover the hermetic-bridge lint; exempt its own fixture file The diff-coverage gate was red at 88.5% (missing 201, 211-214, 224-227) -- the lint shipped with no tests, and those lines are main() failure branches that never fire on a green tree. That was a real gap in the PR, not the known codex-core-launcher flake: the gate measures every added executable Python line, and 227 of them arrived untested. Adds tests/lint-hermetic-bridge-tests.test.py -- 30 assertions, run IN-PROCESS (import + call, never a subprocess, or coverage.py traces a child interpreter and sees nothing). Covers every classify() outcome, both mitigation patterns, all three main() modes, and the two failure branches by temporarily swapping KNOWN_UNISOLATED (empty -> every offender counts as new; bogus entry -> stale NOTE path). Local gate now reports scripts/lint-hermetic-bridge-tests.py (100%), 0 missing. Pins the regressions that were actually wrong in an earlier draft: - a COMMENT naming CLAUDE_CONFIG_DIR must not count as isolation - post-import ACCESS_FILE / channels_env rebinding is MITIGATED, not a hard failure - a stale grandfather entry is a NOTE and must NOT fail the run Also adds SELF_EXEMPT for the lint own test file. Writing the tests surfaced it: the fixture strings contain both `exec_module` and a bridge path, so the lint classified its own test as in-scope and counted it in the scanned total. Same self-exemption convention as scripts/lint-claude-home-path.sh, which allows itself for quoting the pattern it forbids. Scanned/grandfathered/mitigated counts are unchanged at 42/26/7. Stand: Echo Act IV Pro * fix(lint): make the isolation check AST-based and order-aware Addresses qingyun P1 on #2429. The regex predicate was bypassable two ways, both of his repro cases: * an assignment-SHAPED comment -- `# os.environ["CLAUDE_CONFIG_DIR"] = tempfile.mkdtemp()` matched ISOLATES, because a text scan cannot tell code from a comment * a REAL assignment placed AFTER exec_module() -- matched too, because the regex had no way to express ordering, and isolation that runs after the import is useless: the module-level resolution has already read host config Either one let a live-config-reading test pass the gate, which defeats the point of the gate. classify() now parses the file and compares line numbers: isolation counts only when an executable assignment precedes the bridge import. Comments never enter the AST, so the first bypass dies by construction; node.lineno kills the second. An unparseable file is now a VIOLATION rather than a silent pass -- a file that cannot be analysed is not proven clean. This was NOT theoretical. Switching to AST reclassified an existing test: tests/bridge-env-token-perms.test.py sets CLAUDE_CONFIG_DIR at line 179 but calls exec_module at line 124, so it reads host config for 55 lines. The regex draft called it clean. Grandfathered count therefore moves 26 -> 27, and mitigated 7 -> 8; the scanned total is unchanged at 42. Commits both adversarial regressions he asked for, plus ordering and unparseable cases: BYPASS 1 assignment-shaped comment -> violation BYPASS 2 real assignment after import -> violation same assignment BEFORE the import -> clean unparseable file -> violation late HOME assignment -> violation Drops the now-dead ISOLATES/ALT_ISOLATES/MITIGATES regexes rather than leaving them as decoration. Local: 36 assertions pass, whole-tree scanner green, diff coverage 97%, REVIEW path scan PASS. Stand: Echo Act IV Pro * test(lint): pin the util_paths comment bypass shut (qingyun #2429 second finding) His 20:04Z review landed on 0620c70, one commit before the AST rewrite (2272653), so the finding was already closed by that rewrite -- ALT_ISOLATES no longer exists. Verified with his exact repro rather than assuming: util_paths_comment -> violation util_paths_comment_after -> violation real_patch_before -> clean He asked for a committed regression for the case, which is fair independent of the fix, so this adds three: the comment before the import, the same comment after it, and a REAL patch() of channel_access_path before the import (which must still count as isolation, so the predicate is not merely tightened into uselessness). 36 -> 39 assertions. Whole-tree scanner green, diff coverage above the bar, REVIEW path scan PASS. Stand: Echo Act IV Pro * fix(lint): narrow CLEAN to the one provable shape (qingyun #2429 reviews 5-6) Every finding in those two reviews is a FALSE CLEAN, and the pattern matters more than the individual holes: I was trying to prove a test hermetic, which static analysis cannot do (env precedence, execution context, reachability). Inverted the default -- the classifier no longer tries to prove isolation, it recognizes exactly the one documented fix and treats everything else as unproven. Under-approximating CLEAN is safe; over-approximating it makes the gate worthless. CLEAN now requires a MODULE-LEVEL `os.environ["CLAUDE_CONFIG_DIR"] = ...` before the import. Each rejected shape and why: expired_patch_context `with patch(...): pass` then import -- the patch has EXITED by the time exec_module runs. This one was MY bug: I added it as a positive fixture last commit, so the suite was asserting a false-clean. Patch-based isolation is no longer accepted at all, because line numbers cannot prove a patch is active at the import. unrelated_dict `cfg["CLAUDE_CONFIG_DIR"] = ...` -- a dict that is not the environment. The receiver is now checked, not just the key. setdefault a NO-OP when the developer already has the var set, which is precisely the case this lint exists to catch. HOME / CLAUDE_HOME only lower precedence than an inherited CLAUDE_CONFIG_DIR. dead_branch module level is required, so a body that may never run cannot count. never_called_mitigation MITIGATED likewise requires a module-level rebind. Verified against his full adversarial set on this head -- all seven return violation, while real module-level isolation stays clean and a real module-level rebind stays mitigated. Counts move to 42 scanned / 35 grandfathered / 2 mitigated. The narrowing caught 8 further real cases, including tests/slack-bridge-tier-map.test.py, which I had previously argued was a deliberate mitigation -- its rebind is inside a function, so it is exactly the never-executed shape he flagged. I was wrong to defend it. 39 assertions pass, whole-tree scanner green, diff coverage 97%, REVIEW path scan PASS. Stand: Echo Act IV Pro * fix(lint): require os.environ exactly, and bind mitigation to the imported module Addresses qingyun review 7 on #2429. Two remaining false-nonfatal verdicts, both his repros: RECEIVER. _is_os_environ() accepted any attribute NAMED environ and any bare name environ, so `fake.environ["CLAUDE_CONFIG_DIR"] = ...` and a shadowed `environ = {}` both classified clean while the real inherited CLAUDE_CONFIG_DIR stayed active. Now requires literally `os.environ` (Attribute on Name "os"). Bare `environ` is no longer accepted at all -- proving it is `from os import environ` AND unshadowed is more analysis than this gate needs, and the explicit form costs a test author nothing. MITIGATION. _mitigation_line() accepted ANY module-level .ACCESS_FILE / .channels_env assignment, regardless of receiver or order. Since MITIGATED is non-fatal, that silently downgraded real violations: an unrelated `cfg.ACCESS_FILE = ...` counted, and so did a rebind placed BEFORE exec_module -- which proves nothing, because the import then re-resolves against host config anyway. Now the assignment must target the module object actually passed to exec_module() AND appear after that call, so exec_module capture returns (lineno, module_var). Adversarial set on this head: fake_attribute_environ -> violation shadowed_bare_environ -> violation unrelated_mitigation_after_import -> violation mitigation_before_import -> violation GOOD_real_isolation -> clean GOOD_real_mitigation -> mitigated Committed regressions for all four bypasses plus the positive mitigation case, so the permissive direction stays pinned and the predicate cannot be tightened into uselessness unnoticed. Counts: 42 scanned / 37 grandfathered / 0 mitigated. Mitigated fell 2 -> 0 because every previously-mitigated file fails the receiver-or-order check -- i.e. none of them was actually mitigating. Two further files reclassified: tests/bridges-allowlist-default-readonly.test.py and tests/slack-bridge-access-durable-backup.test.py. 44 assertions pass, whole-tree scanner green, diff coverage 96% (raised off the 95% line by covering the remaining early-return paths), REVIEW path scan PASS. Stand: Echo Act IV Pro * chore(lint): drop bridge-audit-wiring from KNOWN_UNISOLATED — #2428 merged john-the-dev merged #2428 at 21:27:59Z, so tests/bridge-audit-wiring.test.py now isolates CLAUDE_CONFIG_DIR on main and no longer belongs in the grandfather list. Rebased onto the new main and regenerated: 37 entries, unchanged otherwise. Worth recording that the non-fatal stale-entry design paid off immediately and in exactly the predicted way. Before this commit the lint printed: NOTE - KNOWN_UNISOLATED entries no longer violating tests/bridge-audit-wiring.test.py and still exited 0. Had that check been fatal -- as I first wrote it -- main would have gone RED the moment #2428 landed, through no fault of the merger. The footgun was real, not hypothetical. Stand: Echo Act IV Pro * fix(lint): close the exec() scope hole and require a seeded access.json Addresses qingyun review 8 and Rui (john-the-dev) concurring. Two defects, and the second is the one that matters. SCOPE. classify() gated on the literal token "exec_module", so a test that loads the bridge with exec(read_text()) returned None -- out of scope, never checked. Out-of-scope is a SILENT PASS, the worst verdict a gate can give. Rui tests/discord-bridge-public-notice-suppression.test.py (#2431) is exactly that shape. _bridge_load_call() now recognizes both mechanisms and reports the namespace variable for either. SEED. CLEAN accepted a bare CLAUDE_CONFIG_DIR assignment. That is not isolation: pointing the var at an EMPTY temp dir leaves channel_access_path() on its legacy real-home fallback, so the operator real allowlist is still what gets read at import. My own #2428 commit message says precisely this, and I then wrote a predicate that did not require it. CLEAN now needs the env override AND a module-level write creating channels/<ch>/access.json, both before the load. The seed check matches a WRITE, never a mention: Rui file names access.json in its module docstring, so a constant-substring check would have called it isolated. Same shape as "a comment is not isolation", one level down. Counts move to 60 scanned / 59 grandfathered / 0 mitigated. That jump is the standard becoming correct, not the tree changing -- I spot-checked codex-subprocess-argv, skill-path-resolution and collaborator-tier: each sets the env var and never seeds access.json. The single CLEAN file is tests/bridge-audit-wiring.test.py, the one written to this standard in #2428. Two of my OWN assertions had to be corrected: they asserted a bare env assignment was CLEAN, i.e. the suite was encoding the defect. Updated rather than worked around. Also deletes _exec_module_call(), superseded by _bridge_load_call() -- dead code, not untested code; removing it took diff coverage 90% -> 98%. Local: 44 assertions pass, whole-tree scanner green, REVIEW path scan PASS. Stand: Echo Act IV Pro * fix(lint): count an access.json seed built through a variable The seed check recognized "access.json" only inside the write call's own receiver expression, so the equivalent two-line form read as no-seed: p = chan / "access.json" p.write_text("{}") # was: violation Binding a path to a name is not a behavioral difference. A false VIOLATION is the sound direction (under-approximating SAFE never hides a real read), so nothing was unsafe — but it would have flagged correctly-seeded files and inflated the migration denominator. _access_path_names() propagates the taint to a fixpoint across module-level assignments, so a name built from another tainted name counts too (ACCESS = "access.json" -> p = dir / ACCESS -> p.write_text). Module level only, deliberately: a name bound inside a function or an if-body has not necessarily been bound when the bridge loads, and unbound-at-load is the reachability case this gate must refuse to guess about. Population is unchanged — 60 scanned / 59 grandfathered / 1 clean. This closes a latent hole; no real file's verdict moves. Four regressions: intermediate variable, two-hop taint, function-scoped seed stays a violation, and a written variable that never carries access.json stays a violation (taint tracks the path, not any write). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H * fix(lint): recognize a seed written through a helper, not just a method PR #2356 introduces write_private_text(path, data) as the canonical way access files are written (born 0600 rather than narrowed afterwards). It takes the path as an ARGUMENT, while the seed check only recognized method-style writes where the path is the RECEIVER: (chan / "access.json").write_text(...) # recognized write_private_text(chan / "access.json", …) # was: no seed -> violation So the moment #2356 lands, correctly-seeded tests would start failing this gate and it would look like #2356 broke them. Found by reviewing #2356, not by being bitten by it. Accepts the helper called bare or through a module attribute, and reuses the same taint set, so a path built into a variable first still counts. A helper write of a NON-access path (.env) stays a violation — that is precisely the shape john flagged as not-isolation on review 8 — and an unrelated call merely naming access.json is still not a seed: only a recognized WRITE counts. Population unchanged: 60 scanned / 59 grandfathered / 1 clean. Tenth finding of the same class — recognizing one syntactic form and treating everything else as absent rather than unproven. Receiver, ordering, loader, seed, variable, now call shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H * fix(lint): a seed must target the CONFIGURED channels/<ch>/access.json Review 10 (qingyun): the predicate proved only that some path expression contained "access.json" — never that the write targeted $CLAUDE_CONFIG_DIR/channels/<ch>/access.json. So an unrelated write under /tmp/x classified CLEAN while the canonical file stayed absent and the bridge kept falling back to the operator real-home allowlist. Both the method and the helper shape inherited it. Inverts what the taint is seeded FROM. It was seeded by the string "access.json" (anything carrying it counted); it is now seeded by the CONFIGURED root — os.environ["CLAUDE_CONFIG_DIR"] or the name assigned into it — and propagated to a fixpoint over module-level assignments, accumulating literal path segments. A seed now requires all three: a WRITE, a path ROOTED at the configured dir, and both a channels and an access.json segment. A name bound to a plain string contributes SEGMENTS but never ROOTEDNESS, so factoring a segment out (ACCESS = "access.json") still works while it cannot rescue an unrooted write. Evidence: - qingyun exact false-cleans now flag: unrelated_method_seed and unrelated_helper_seed both VIOLATION (were CLEAN) - canonical shapes stay CLEAN: inline method, helper, and root-via-the-name- assigned-into-os.environ - 5 regressions added, incl. rooted-but-no-channels-segment - whole-tree scan UNCHANGED before/after: 60 scanned, 59 grandfathered, exit 0 (4 real tests lose credited-seed status but were already grandfathered) The submitted fixtures encoded the defect: several seeded a relative or /tmp path. Rerooted at the configured dir, which is what a hermetic test does. Stand: Echo Act IV Pro * fix(lint): the seed must match the BRIDGE being imported, not just any channel Review 11 (qingyun): the seed check proved a rooted channels/<x>/access.json write but never that <x> matched the bridge under import. channel_access_path() resolves PER CHANNEL, so seeding channels/discord/access.json does nothing for a test that loads telegram-bridge.py — the canonical telegram file is still absent and the import falls back to the operator real Telegram allowlist. Her repro, verified at this head before the fix: seed discord + import telegram classified clean. Worse, my own positive fixture had locked the shape in: it seeded `slack` while IMPORTS_BRIDGE loads the DISCORD bridge, so the suite asserted the false clean as correct behaviour. Corrected to seed discord. _imported_bridge_channels() reads the channels the file actually loads from the existing BRIDGE_IMPORT pattern; _access_seed_line() now records seeds per channel and requires EVERY imported channel to be seeded, returning the LATEST such line so the caller before-the-load ordering check still covers all of them. 6 regressions, mutation-checked. Dropping the channel requirement kills exactly the 4 discriminating ones and leaves the 2 matched-channel positives green: seed discord + import telegram -> violation seed telegram + import slack -> violation seed slack + import discord -> violation (helper-write shape) two bridges imported, one seed -> violation seed telegram + import telegram -> clean seed slack + import slack -> clean Whole-tree scan unchanged before/after: 61 scanned, 59 grandfathered, exit 0. Diff coverage 97% (gate 95%). py_compile, git diff --check and review-checks all clean. Stand: Echo Act IV Pro * fix(lint): a helper seed must be the PATH argument, not any argument Review 12 (qingyun): the helper-call analysis scanned every argument, so a canonical access path passed as the DATA argument satisfied the seed check while the write landed elsewhere. Her exact-head repro returned clean without ever creating $CLAUDE_CONFIG_DIR/channels/discord/access.json. Argument ROLE matters: write_private_text(path, data) writes to arg 0. Restricted to arg 0, plus the `path=` keyword form so the change does not break the keyword call shape. 3 regressions, mutation-checked. Reverting to scan-all-arguments kills the DATA-argument case AND the keyword case, so both are discriminating: path in DATA arg, write elsewhere -> violation (was clean) path in arg 0 -> clean path= keyword form -> clean Focused suite passes; whole-tree scan unchanged at 61 scanned, 59 grandfathered, exit 0. This is the third false-CLEAN on this predicate in a row, and all three had the same shape: I checked a PROXY for the write (a string appears / a path is rooted / a path appears in the call) instead of the write TARGET. The argument-role check is the first one that asks where the bytes actually go. Stand: Echo Act IV Pro * fix(lint): isolation means a TEMP dir, not merely an assignment Found by self-audit, not review. After three rounds where the defect was the same shape - checking a PROXY for the write instead of the target - I went looking for a fourth of that shape rather than waiting. There was one, and it is the worst so far. _isolation_line proved that `os.environ["CLAUDE_CONFIG_DIR"] = ...` HAPPENED. It never proved the value pointed anywhere safe. So: os.environ["CLAUDE_CONFIG_DIR"] = os.path.expanduser("~/.claude") (cfg / "channels" / "discord" / "access.json").write_text(...) classified CLEAN while writing into the OPERATOR REAL allowlist. That is strictly worse than the fallback-READ this gate exists to prevent, because it MUTATES host config - and the gate was actively certifying it as hermetic. Isolation now requires the value to derive provably from a temporary source (mkdtemp / TemporaryDirectory / mkstemp), propagated to a fixpoint so the ordinary two-step `_d = mkdtemp(); os.environ[...] = _d` shape still passes. Anything unprovable is refused, including a bare literal - under-approximating CLEAN is this file documented stance. 5 regressions; mutation kills 3. Whole-tree scan UNCHANGED before and after (61 scanned, 59 grandfathered, exit 0) and ZERO real tests change verdict, so the tightening costs nothing: every real test already uses tempfile. One submitted fixture asserted CLEAN with a literal "/tmp/x" and is corrected to mkdtemp. That is the third time on this PR my own fixtures encoded the very defect under review, which is the more useful finding: when the predicate and its tests are written from the same mental model, the tests cannot catch it. Stand: Echo Act IV Pro * fix(lint): a seed must be REACHABLE on import, not merely present Review 13 (qingyun): ast.walk() flattens the tree, so a canonical access write inside a never-called function was recorded as a seed. The file is never created before the bridge loads, so the import still falls back to the operator real allowlist - the exact failure this gate exists to catch, waved through by the gate itself. Her repro classified clean on the previous head; it is a violation now. _reachable_nodes() replaces ast.walk() for the seed scan: it does NOT descend into FunctionDef / AsyncFunctionDef / ClassDef / Lambda bodies, which only run when CALLED. Conditionals (if / try / with) ARE still descended into - they execute in module order, and refusing them would reject the ordinary `with tempfile.TemporaryDirectory() as d:` seeding shape. 5 regressions; mutation (revert to ast.walk) kills 2. Both directions pinned: seed inside `def never_called()` -> violation seed inside a class body -> violation module-level inline seed -> clean seed inside a `with` block -> clean seed inside try/except -> clean Whole-tree scan unchanged before and after: 61 scanned, 59 grandfathered, exit 0 - no real test is newly flagged by the stricter reachability rule. Fifth false-CLEAN on this predicate. The shape has not changed: I keep accepting evidence that the write was WRITTEN rather than evidence it RUNS. Stand: Echo Act IV Pro * fix(lint): only bodies that run UNCONDITIONALLY on import can hold a seed Review 14 (qingyun): skipping def/class bodies was not enough. _reachable_nodes() descended into every child of top-level `if` and `try`, so a canonical seed under `if False:` or inside an `except` handler that never fires was recorded as if it had executed before the bridge import. Descend only into what an import actually runs: a `with` body, a `try` BODY, and its `finally`. Refuse both `if` branches (none is guaranteed), `except` handlers (exception-only), `else` clauses (exit-dependent), and `for`/`while` bodies (zero iterations is legal). if False: seed -> violation (her repro) except-only seed -> violation (her repro) for-body seed -> violation module-level seed -> clean with-block seed -> clean try-BODY seed -> clean 4 regressions; mutation (descend into everything again) kills 3. COST, stated rather than hidden: a legitimate `if not (cfg / "access.json").exists(): seed()` now reads as a violation. That is a real false positive of the same class #2357 exposed - the gate refusing a correct shape. It is recorded in the open design question rather than quietly accepted. Measured impact on this tree: ZERO real tests change verdict, and the whole-tree scan is unchanged at 61 scanned, 59 grandfathered, exit 0. Coverage 98%. Seventh false-CLEAN on this predicate, and the shape has never changed: I keep accepting evidence that a write EXISTS rather than evidence that it RUNS. Stand: Echo Act IV Pro * fix(lint): a try body runs only up to its first definite terminator Review 15 (qingyun, round 8): _reachable_nodes() treated every statement in a `try` body as guaranteed. A seed placed AFTER an unconditional `raise` never executes - but the handler swallows the exception and the module import continues, so the lint reported clean on a test that seeded nothing and the bridge could still fall back to the operator real allowlist. _until_terminator() stops at the first raise/return/continue/break. Applied to both the try body and its finally. seed AFTER an unconditional raise -> violation (her repro) seed BEFORE the raise -> clean (terminator must not retroactively invalidate what precedes it) plain try-body seed -> clean 2 regressions; mutation (traverse the whole body again) kills 1. Tree scan unchanged at 61 scanned, 59 grandfathered, exit 0. Coverage 98%. Eighth round. The shape has not moved once: I keep accepting evidence that a write is PRESENT rather than evidence that it RUNS - first anywhere in the file, then outside def/class, then outside conditionals, and now before the statement that ends the block. Stand: Echo Act IV Pro * fix(tests): seed discord too — bridge-timeout-guards loads BOTH bridges CI caught this the moment #1886 merged into main: the stricter hermetic lint flagged tests/bridge-timeout-guards.test.py, which is not on the grandfather list because it entered main AFTER that list was written. The gate is right. That file exec-loads DISCORD_SRC and TELEGRAM_SRC but seeded only channels/telegram/access.json. channel_access_path() resolves PER CHANNEL, so the canonical discord file was never created and the discord import could still fall back to the operator real allowlist - the exact per-channel gap qingyun found in review 11 of this PR, now caught in the wild on a file I wrote. Seeds channels/discord/access.json alongside telegram. before: lint FAIL - tests/bridge-timeout-guards.test.py after : lint ok (63 scanned, 59 grandfathered, exit 0) The test itself still passes, and the lint suite is unchanged. Deliberately NOT grandfathered. Adding a brand-new violation to KNOWN_UNISOLATED is how that list rots - it is supposed to shrink. The file is three hours old and the fix is three lines. Stand: Echo Act IV Pro * fix(lint): a seed must prove the parent dir exists, not just name the path qingyun (#2429 review 14) found the predicate accepting a write whose parents never existed: `mkdtemp()` hands back an EMPTY dir, so `channels/<ch>/` is absent and `(ccd/"channels"/"discord"/"access.json").write_text(...)` raises FileNotFoundError having created nothing. Swallow that and the bridge still imports — `channel_access_path()` still resolving the operator's real allowlist while the lint reported clean. Same shape as rounds 10-13: the predicate checked something CORRELATED with the file existing (a write call naming it) instead of the file existing. So rather than patch her repro, I probed the shape. Seven more members, all CLEAN before this commit, all committed as regressions: swallowed missing-parent write <- hers bare write, no mkdir anywhere <- simpler than hers, equally broken mkdir without parents=True <- `channels/` is absent too mkdir AFTER the write mkdir for a DIFFERENT channel mkdir under `if False:` mkdir inside a never-called def write swallowed by `except Exception` A seed now requires a REACHABLE recursive directory creation for that channel, strictly before the write: `.mkdir(parents=True, ...)` or `os.makedirs(...)`, resolved through the same rootedness analysis as the write and the same `_reachable_nodes` walk, so an mkdir the import never executes cannot vouch for one it does. Evidence: * per-file verdicts across all 63 in-scope tests are BYTE-IDENTICAL before and after (59 violation / 4 clean) — zero false positives on real tests; * removing the mkdir from each of the 4 clean tests flips it to violation, so the new condition is load-bearing rather than dead; * the 8 regressions FAIL on the parent commit and pass here; the 3 positive-control idioms this repo actually uses (`_cfg.mkdir`, `p.parent.mkdir`, `os.makedirs`) stay clean in both directions. The test file's own fixtures had encoded the bug a fourth time: two mkdir'd a RELATIVE `channels/discord` (creating it in CWD, not under the configured dir) and ten omitted the mkdir entirely. Fixed rather than exempted — a fixture that cannot succeed at runtime is not a positive control. `write_private_text` is held to the same precondition. #2356 has not landed, so its parent-creating semantics cannot be verified against real source here, and an unverifiable exemption is the exact assumption this predicate keeps getting caught making. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H Stand: Echo Act IV Pro * test(lint-hermetic): pin the known false positives, and name a third one Resolves the open design question on this PR the way I said I was leaning — ship the gate and assert the limitation — but as TESTS rather than only prose, so the known-bad shapes are a checked contract instead of a docstring claim that can drift. Three shapes are hermetic and SHOULD classify clean. They do not: 1. isolation and load inside ONE function body 2. a seed guarded by `if not (…/access.json).exists():` 3. the temp dir routed through a variable rather than assigned inline (1) and (2) are the structural asymmetry already described in `_reachable_nodes`: isolation is only counted where it provably runs on import, while the load side uses `ast.walk` and sees a load anywhere. Neither symmetric repair is acceptable — moving the load side to `_reachable_nodes` stops seeing loads inside test functions (the common shape), and moving isolation to `ast.walk` re-admits the `if False:` seed that round 7 closed. Both trade a false positive for a false CLEAN, which is the wrong direction for this gate. The real fix is per-scope ordering, which is a new analysis and does not belong in this PR's last round. (3) was NOT previously documented, and I found it only because I wrote a control. The first draft of the guard pin also routed the temp dir through a variable, so its violation could have come from either cause. The 2x2 disambiguates: direct + unconditional -> clean direct + if-guarded -> violation indirect + unconditional -> violation indirect + if-guarded -> violation So the guard and the indirection are independent limitations, and (3) is not scope-related at all — the scope-aware fix will NOT remove it. It needs the isolation predicate to follow a local binding. Each pin carries a control that varies exactly one thing, because a bare `== VIOLATION` assertion would also hold if classify() started returning violation for everything — the pin would pass while measuring nothing. Each also states its removal trigger: when a pin starts returning CLEAN that is the fix landing, so delete the pin rather than "repair" it. Pinning is not endorsement. The gate's documented stance is to under-approximate CLEAN, so a false positive costs a grandfather entry while a false CLEAN costs a silently mutated host config. Checks: full lint suite PASS (incl. 3 pins + 2 controls) · lint self-check over the repo ok (63 scanned, 59 grandfathered, 0 mitigated) · py_compile · git diff --check · review-checks PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H Stand: Echo Act IV Pro * chore(lint): recognise the property, not the name — two false-CLEAN paths qingyun-wu's two exact-head findings on 1e7794e were both the same shape: a check that matched a NAME instead of the property that name stands for. Neither fixture ever created or wrote the canonical access file, and both classified `clean` — so a test could read or mutate the developer's real channel allowlist while the gate reported it isolated. 1. `makedirs` recognition accepted ANY call whose attribute was `makedirs`, so a no-op `Fake().makedirs(parent)` vouched for a parent that was never created; the canonical write then raised and was swallowed. Now the receiver must be a name that genuinely imports the `os` module in that file, or a bare name genuinely imported via `from os import makedirs` — both collected from the AST's own imports, so aliases (`import os as _os`, `from os import makedirs as md`) keep working and a locally-defined `def makedirs()` does not. `.mkdir(parents=True)` deliberately needs no such guard: it returns the RECEIVER as the created path, so a fake receiver fails path resolution by itself. `makedirs` takes the path as an ARGUMENT — the path checks all pass and only the receiver is a lie, which is exactly why it needed one. 2. Path identity used substring membership, so a rooted `notchannels/discord/access.json.bak` matched on both `"channels" in seg` and `"access.json" in seg`. Now components are split and compared by EXACT identity, which rejects both the `notchannels` lookalike and the `.bak` sibling. Ordering is NOT enforced, stated plainly rather than implied: segments arrive as an unordered SET because a path assembled across assignments loses sequence in `_expr_root_segments`. Exact identity is what rejects both reported false-CLEANs — neither needs order — and making the resolver order-preserving is a wider change than this fix. The residual gap is a path holding all three components in a nonsensical order, strictly smaller than the substring gap it replaces. Six regressions: the two reported fixtures, a bare-makedirs-never-imported variant, and three anti-vacuity controls (genuine seed, `import os as _os`, `from os import makedirs`) so the pins cannot pass by classify() simply ceasing to return CLEAN. Mutation-verified: against 1e7794e's lint script exactly three of these fail and the controls still pass; with the fix the full suite and the bridge-timeout suite are green. Stand: Echo Act IV Pro * chore(lint): order-preserving path resolution + reject a rebound os alias Both remaining P1s, and both are the same shape a third time: the check accepted a weaker property than the one it needs. 1. john-the-dev — the unordered component SET accepted any permutation, so a rooted `access.json/channels/discord` classified clean while the canonical allowlist was never created. I had retained this as a documented limitation; it is a security boundary, so it is fixed rather than noted. `_expr_root_segments()` now walks the `/` spine structurally instead of via `ast.walk` (breadth-first, order-destroying), and `_rooted_segments()` / `_literal_segment_names()` carry ordered lists. `_path_channel()` then requires the contiguous canonical run `channels/<bridge>/access.json`. `.parent` had to be modelled explicitly as "drop the last component" — an order-blind set never needed that, but without it this repo's own `p = …/access.json; p.parent.mkdir(parents=True)` idiom stops resolving to the channel directory. The suite caught it immediately. 2. qingyun-wu — an imported name was trusted at the call site even after being rebound: `import os` … `os = Fake()` … `os.makedirs(p)` created nothing and still classified clean. Any name rebound anywhere (assign, aug/ann-assign, for-target, with-as, def/class) is now dropped from the alias sets. Only a bare Name target counts as a rebinding. Walking into targets marked `os` shadowed from `os.environ[k] = v`, which turned every correctly-isolated test into a violation — setting CLAUDE_CONFIG_DIR is the first thing they all do. Caught by the controls, not by inspection. Five regressions: both reported fixtures plus three anti-vacuity controls (multi-assignment canonical seed, the `.parent` idiom, and `os.environ[...] = ...` not counting as a rebinding), because ordering and shadow-detection are both easy to overshoot into "nothing is ever clean". Mutation-verified: exactly the two new pins fail against daacb14 and the three controls pass on both heads. Full hermetic-lint suite and bridge-timeout suite green here. Stand: Echo Act IV Pro * chore(lint): last write wins — a same-length rebinding is not a no-op qingyun-wu, #2429: `_rooted_segments()` replaced a binding only when the new segment list was strictly LONGER, so rebinding a path variable to a same-length non-canonical path was silently ignored: _cfg = <ccd>/"channels"/"discord" ; _cfg.mkdir(parents=True) _cfg = <ccd>/"channels"/"slack" ; (_cfg/"access.json").write_text("{}") <import src/discord-bridge.py> kept `_cfg -> channels/discord` and classified CLEAN, while at runtime only the SLACK allowlist exists and the Discord canonical file is absent — so the import can still fall back to the operator's real allowlist. Straight-line resolution is now last-write-wins in source order, and a rebinding to something NOT rooted at the configured dir DROPS the binding rather than leaving a stale one vouching for the name. Convergence is detected by comparing the whole map across a pass instead of a per-assignment `grew` flag. That flag cannot survive last-write-wins — a name assigned twice would flip on every pass and loop forever. Snapshot-compare stops as soon as a pass is a no-op; a 64-iteration cap is a backstop. Three regressions: the reported fixture, plus two controls so order-sensitivity cuts both ways rather than becoming a stricter gate — a single-binding discord seed stays CLEAN, and a slack->discord rebind that seeds discord LAST stays CLEAN. Mutation-verified against 61a05a1; full hermetic-lint and bridge-timeout suites green here. Stand: Echo Act IV Pro * fix(lint): honor rebinding in every name tracker, and select targets structurally Round 19. The reviewers found the literal-segment case; probing the AXIS instead of the reported case found two more members of the same family, plus a false-positive source in target selection. ONE DEFECT, THREE TRACKERS — a module-level name kept a tracked property after being rebound to a value that no longer had it. Each yields a false CLEAN, i.e. the canonical channels/discord/access.json is absent at import and the bridge falls back to the operator real allowlist: 1. _literal_segment_names (reported by john-the-dev + qingyun-wu). A rebind that yielded no literal hit `continue`, leaving the stale segments: OTHER = "slack"; CHANNEL = "discord"; CHANNEL = OTHER kept CHANNEL -> discord. Now a bare-Name rebind resolves through the alias and anything else DROPS the binding, failing closed. Only a bare Name is resolved: resolving names nested in arbitrary expressions would ADD segments, and more segments means more paths match a seed — that widens CLEAN, the unsafe way. 2. _ccd_root_names (found here). The root name kept its status forever, so os.environ["CLAUDE_CONFIG_DIR"] = _ccd ; _ccd = "/tmp/elsewhere" (Path(_ccd)/"channels"/"discord"/"access.json").write_text(...) still recorded _ccd as the configured root — the same false CLEAN reached through the ROOT rather than the SEGMENT. 3. _isolation_line (found here; the most severe, since it defeats isolation outright rather than mis-attributing one path). A monotone set said d = mkdtemp(); d = "/Users/<me>/.claude" os.environ["CLAUDE_CONFIG_DIR"] = d was "provably isolated" while it points the env var at the operator real config dir. Fixed by asking the question where it is asked — POINT-IN-TIME. A final-state set was my first attempt and is wrong the OTHER way: it calls _ccd = mkdtemp(); os.environ[...] = _ccd; _ccd = "/tmp/elsewhere" unisolated, though the env var really was set to a temp dir and a later rebinding does not un-set it. That false positive is pinned by a control. _isolated_names keeps its original monotone meaning for its other caller. TARGET SELECTION WAS TEXTUAL. `BRIDGE_IMPORT.search(text)` regexed the raw file, so a file that merely NAMED a bridge in prose had to be hermetic. It fired on a real merged test: tests/discord-read-forwarded.test.py (#2458) mentions discord-bridge.py in its module docstring while importing only src/discord-read.py, which resolves no channel config at import. _code_strings() now selects on strings the module actually evaluates. The raw regex stays as a cheap pre-filter so an unparseable file that never mentions a bridge still exits before the conservative VIOLATION, which is meant for files we had reason to scan. KNOWN_UNISOLATED is net-flat: dropped dm-result-multipart-upload.test.py (stale — prose-only mention) and added discord-access-backup.test.py, a real hole that landed in #2358 after the baseline was measured. Grandfathered rather than fixed because repairing another PR test is a second concern. 10 regressions incl. the reverse-order and unrelated-assignment controls qingyun asked for. Mutation-verified: reverting each of the four behaviours fails 2/1/1/4 checks respectively — no fix is unpinned, which is how the false positive in my first isolation attempt surfaced. Stand: Echo Act IV Pro * refactor(lint): drop _isolated_names, orphaned by the point-in-time isolation fix The diff-coverage gate failed at 94.8% and pointed straight at it: lines 229-241 were the whole function body, uncovered because it has ZERO callers. Making _isolation_line() evaluate isolation point-in-time meant it no longer asks for a precomputed set, and nothing else ever did. Dead code should be deleted, not tested. Covering it would have bought the gate a green tick and left a fixpoint helper in the tree that no longer describes how the lint decides anything — and the next reader would reasonably assume it does. Local gate after removal: 97.9% on the changed lines (was 94.8%, threshold 95%). Whole-tree lint rc=0, invariant suite PASS. _is_isolated_value is untouched and still used by _isolation_line. Stand: Echo Act IV Pro --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>

bodhi sonichi#27 merged via merge commit — 25cc6e8 is the canonical main head containing the probe/verify/takeover roles + dist rebuild. Replaces the draft-only fork stand-in; lock regenerated; installed dist re-verified to carry the role code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…marker (activation switch) (#2713) * chore(voice): group E — bodhi probe-isolation pin + capability marker (activation switch) Bumps bodhi-realtime-agent to the PR-27 head (probe/verify/takeover connection roles, dist rebuilt) and publishes the capability marker state/voice-agent.capabilities.json {probeIsolation:true} once at voice-agent startup — the strict-true flag the desktop supervisor's ws-upgrade probe battery and exit-7 holder checks gate on. Pin and marker land in the same change on purpose: the marker is the activation switch and must never precede the capability. NOT READY until sonichi/bodhi_realtime_agent#27 merges — the pin then repoints to the post-merge sonichi main SHA (fork SHA is a draft-only stand-in) and the lock regenerates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(voice): repoint bodhi pin to the post-#27 sonichi main SHA bodhi #27 merged via merge commit — 25cc6e8 is the canonical main head containing the probe/verify/takeover roles + dist rebuild. Replaces the draft-only fork stand-in; lock regenerated; installed dist re-verified to carry the role code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(voice-agent): bind the capability marker to its publishing process (review) The marker was a one-way latch: only ever {probeIsolation: true}, nothing deletes or invalidates it, and older builds don't write it at all. After an engine rollback below the capable pin, the stale true on disk would keep the desktop supervisor's ws-upgrade probe battery armed against a build without probe isolation — a ?probe=1 upgrade could then steal a live call, the exact failure the isolation exists to prevent. Fix: the marker now records pid (the publisher's own). Publication runs strictly after acquirePidLock() — a lock loser exits 7 first — so marker.pid equals the live lock holder's pid exactly when the marker describes the running build. The desktop reader (paired in the group-H activation PR) requires that equality plus holder liveness and fails closed on mismatch. Also: package-lock.json rebuilt from main + the pin change only — the previous regen dropped six unrelated "peer": true flags (npm-version churn); the lock diff is now exactly the bodhi pin (spec/resolved/integrity). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(voice-agent): bind the capability marker to the acquisition token, not just pid (review) pid alone is not stale-marker-proof: OS pids are reused, so after a rollback to a build without probe isolation a later run can land on the stale marker's pid and the reader's pid equality would incorrectly arm the probe battery. The structured lock already carries the right token: lockId (vl1-<uuid4>), unique per acquisition — the same class of defense voice-lock.py uses (startTimeMs matching) against pid reuse on release, with exact string equality instead of clock-tolerance semantics. acquireVoiceLock now surfaces the helper's emitted lock record's lockId; voice-agent threads it into the marker. A marker published without a token (helper output unparseable) is unbound: the desktop reader requires the token and fails closed (dormant). The paired desktop reader requires marker.pid AND marker.lockId to equal the current structured lock holder's, plus liveness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(agent-state): fix a pre-existing race — assert the lifecycle file mirrors the LATEST failed classification The integration test pinned category === 'quota' on the lifecycle FILE, but the fake key's own startup auth close arrives through the same wrapped onClose seam and can land after the injected quota close, legitimately overwriting the category (R8: reason/category changes within 'failed' are meaningful transitions — recordTerminalClassification → emit → publish runs for both). The file only holds the LAST state, so a 150ms poll for 'quota' misses forever once 'auth' lands. Captured live under parallel load: at timeout the file read failed/auth while the quota frame sat in the frames array; reproduced 2/10 sequential and ~1/9 parallel on the pre-fix assertion. The quota mapping stays pinned end-to-end by the FRAME assertions; the file's real invariant — mirrors the latest failed classification — is what the wait now asserts. 15/15 parallel runs green post-fix. Pre-existing since the test landed (#2689); surfaced on the PR head run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(agent-state): activate the group-E probe-vs-live-client acceptance test The file's Step-11 note scheduled the probe-frame + live-call acceptance test for the group-E pin bump — which this PR is, making the note stale and the promised test due. Against the pinned bodhi, with a real client attached: a ?probe=1 upgrade is intercepted before client attachment, gets exactly its agent.state frame + close(1000), and the client keeps its slot (socket stays OPEN, lifecycle still counts it attached) — the steal the isolation exists to prevent, asserted end-to-end. The same spawn also proves the capability marker in vivo: published (capable pin) and BOUND to the structured lock — marker.pid and marker.lockId equal the lock file's. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(voice-agent): gate the capability marker on the probeState feature-detect (review) The marker publish was unconditional while bodhiSupportsProbeState sat in scope ~100 lines earlier — the never-precede-the-capability invariant held by shipping discipline alone. It breaks the moment npm resolves a bodhi other than the lockfile's (failed or cached install, hand-edited pin): the marker would advertise probeIsolation to a build that routes probes down the normal attach path, and a supervisor probing on a timer could preempt a live call. Gate publication on the same feature-detect that wires probeState; log the dormant branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(voice-agent): never publish an unbound marker; prove both gate branches (review round 5) Two findings, one contract change: 1. The unbound marker failed closed only in another repository: a missing acquisition token still published {probeIsolation, at, pid}, and the entire defense was the desktop reader requiring the token — a safety property living across a repo boundary on a one-way-latch file. An unbound marker has no consumer that benefits from it, so skip publication instead: lockId is now a REQUIRED parameter of publishCapabilitiesMarker and the caller gates on having one. Fail closed at the writer; a marker on disk is always fully bound. The desktop reader's token requirement remains as a second fence (it still guards files left by older builds). 2. The gate was the safety property and nothing tested it — deleting if (bodhiSupportsProbeState) left the suite green. New integration row spawns the agent with a SUTANDO_TEST_MODE-only detect-override seam (same pattern as the injected-close seam), proves the wiring init ran past the marker site (first frame observed), and asserts NO marker file plus the dormant-branch log line. The gate now regresses loudly, on the real caller, for both branches (the TRUE branch is pinned by the probe acceptance test's in-vivo marker-binding assertions). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style(voice): condense group-E comments to the 2-line policy cap CLAUDE.md: comments state the constraint or non-obvious reason in at most two lines; design rationale, incident history, and review references live in the PR body and commit messages. Five blocks added across this PR's review rounds exceeded the cap (marker docblock, marker-call gate, detect seam, token assignment, acquire parse) — each condensed to its invariant. The acquire-parse comment also dropped a stale claim: since lockId became required, no unbound marker is ever published. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>

…a Gemini key is named, campaign work goes to campaign-runner Three owner-visible misroutes from user feedback (2026-09-17 to 09-20), each a wrong or missing sentence rather than a missing capability. 1. Sending email "still needs an app password" (#28 on the tracker). The gmail-write-guard hook only matches the claude.ai Gmail MCP tools; the Station Gmail connector (composio_find → composio_exec) is a different tool name it never touches and it works. But the guard's denial REASON led with the app-password IMAP/SMTP path, so a core that reached for the claude.ai tool was pointed at generating an app password by hand even when Gmail was connected in Settings → Integrations. The reason now routes to the Station connector first, requires a read-back of the sent message (recipient + subject) before reporting a send — the check the draft/send mismatch incidents in field report 05cb849a lacked — and names IMAP/SMTP only as the fallback when the Station tools are absent. docs/built-in-tools.md and hooks/README.md say the same. 2. Gemini image generation on a free-tier key (#5). A free-tier key has 0 image requests per day; Google answers 429 RESOURCE_EXHAUSTED, which generate.py collapsed into api_error "try again in a moment" — the wrong advice for a quota. A 429, or any RESOURCE_EXHAUSTED/quota body, is now its own `quota` outcome whose remedy says to enable billing on the key's project or use the managed key, and the message names which key ran out (the owner's own GEMINI_API_KEY vs the managed one) because resolve_key() already knew. 3. Outbound campaigns went to the bare Smartlead connector (#27), which can only create, read and delete a campaign shell; attaching the mailbox, uploading leads and writing the sequence were left to the owner by hand — while the installed campaign-runner skill does all of it. precheck_apps.json gains a `prefer_skill` field (smartlead → campaign-runner), listed first so its phrases beat Gmail's bare "email"; match_apps lets a longer phrase win over a shorter one it contains ("cold email campaign" names Smartlead, not Gmail); the precheck line reports `prefer_skill=… (installed|not installed)` and never proposes a Connect card for such an app; the skill text says to invoke the skill or offer to install it, never the bare connector. Tests: gmail-write-guard (routing + read-back + fallback order), image-generation-cli (429 → quota with the key named; quota body on a 400; a plain 400 stays api_error), connect-apps-precheck (phrase precedence; prefer_skill line with and without the skill installed). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…a Gemini key is named, campaign work goes to campaign-runner (#4661) * fix(owner-facing): email routes to the Station connector, a zero-quota Gemini key is named, campaign work goes to campaign-runner Three owner-visible misroutes from user feedback (2026-09-17 to 09-20), each a wrong or missing sentence rather than a missing capability. 1. Sending email "still needs an app password" (#28 on the tracker). The gmail-write-guard hook only matches the claude.ai Gmail MCP tools; the Station Gmail connector (composio_find → composio_exec) is a different tool name it never touches and it works. But the guard's denial REASON led with the app-password IMAP/SMTP path, so a core that reached for the claude.ai tool was pointed at generating an app password by hand even when Gmail was connected in Settings → Integrations. The reason now routes to the Station connector first, requires a read-back of the sent message (recipient + subject) before reporting a send — the check the draft/send mismatch incidents in field report 05cb849a lacked — and names IMAP/SMTP only as the fallback when the Station tools are absent. docs/built-in-tools.md and hooks/README.md say the same. 2. Gemini image generation on a free-tier key (#5). A free-tier key has 0 image requests per day; Google answers 429 RESOURCE_EXHAUSTED, which generate.py collapsed into api_error "try again in a moment" — the wrong advice for a quota. A 429, or any RESOURCE_EXHAUSTED/quota body, is now its own `quota` outcome whose remedy says to enable billing on the key's project or use the managed key, and the message names which key ran out (the owner's own GEMINI_API_KEY vs the managed one) because resolve_key() already knew. 3. Outbound campaigns went to the bare Smartlead connector (#27), which can only create, read and delete a campaign shell; attaching the mailbox, uploading leads and writing the sequence were left to the owner by hand — while the installed campaign-runner skill does all of it. precheck_apps.json gains a `prefer_skill` field (smartlead → campaign-runner), listed first so its phrases beat Gmail's bare "email"; match_apps lets a longer phrase win over a shorter one it contains ("cold email campaign" names Smartlead, not Gmail); the precheck line reports `prefer_skill=… (installed|not installed)` and never proposes a Connect card for such an app; the skill text says to invoke the skill or offer to install it, never the bare connector. Tests: gmail-write-guard (routing + read-back + fallback order), image-generation-cli (429 → quota with the key named; quota body on a 400; a plain 400 stays api_error), connect-apps-precheck (phrase precedence; prefer_skill line with and without the skill installed). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(owner-facing): trim the guard-test comment, name the key in use, keep a rate-limited 429 retryable, route through the Claude-home helper Review findings on the previous revision, all addressed here. Blocker: `typecheck + node & shell tests` was red because tests/gmail-write-guard.test.py:60 carried a four-line comment block with incident history (prose-cap allows two lines). The comment now states the assertion only; the history stays in the PR body. The dated paragraphs in the gmail-write-guard.py module docstring and the classify_http_error docstring are trimmed the same way. skill_installed() in skills/connect-apps/hooks/connect-precheck.py re-implemented Claude-home resolution as CLAUDE_CONFIG_DIR -> ~/.claude, skipping the CLAUDE_HOME level of the canonical chain, so a setup that only sets $CLAUDE_HOME was told the skill is missing. It now resolves through util_paths.claude_home_path("skills") the way workspace_for() already resolves the workspace, with the stock ~/.claude only when no core tree is importable. Tests cover CLAUDE_HOME-only, CLAUDE_CONFIG_DIR outranking it, and the no-core-tree fallback. key_note("env") always named GEMINI_API_KEY, but the resolver and the fallback chain both accept GEMINI_VOICE_API_KEY for gemini-image, so a voice-key-only install was pointed at the wrong variable. The note now names whichever variable holds the key that was used. classify_http_error() treated every 429 (and any body containing "quota") as the free-tier zero-quota case, whose SKILL.md remedy is "enable billing, never retry". A paid key hitting the per-minute rate limit is also a 429 and needs the opposite advice. `quota` now requires plan-level wording (per day / free tier / limit 0 / billing) in the body; any other 429 stays `api_error` with its retry remedy. Tests use Google's free-tier per-day body and add a per-minute body that must stay retryable. match_apps() kept one keyword per app and then dropped a hit whose keyword sat inside another app's longer phrase, so "reply to the cold email from Bob in my inbox" lost Gmail although "inbox" names it. Every matched keyword now counts; an app is dropped only when all of its keywords are inside other apps' phrases. Test added for that sentence. Verification: python3 tests/gmail-write-guard.test.py (PASS), tests/image-generation-cli.test.py (29 OK), tests/connect-apps-precheck.test.py (26 OK); scripts/review-checks.sh --diff PASS; diff coverage of changed Python lines 100%; lint-workspace-resolution --diff clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(connect-precheck): suppress a shorter keyword by match span, not by keyword string `match_apps` was meant to keep the "email" inside Smartlead's "cold email campaign" from also naming Gmail. The previous head compared keyword *strings*: a keyword was dropped whenever it was a substring of any keyword another app had matched anywhere in the text. So Gmail vanished from "draft an email to Bob and check my email campaign stats" (two separate requests, no shared span), from "email me when the email sequence finishes", and from "send an email about the cold email campaign" — it survived only when it also matched a word that is not a substring of anything, such as "inbox". The PR's own test covered just that single-overlap case, which is why CI stayed green with the defect live. Now each match is kept as (keyword, span) — `pat.finditer(low)` over every occurrence, since `search` would stop at a first occurrence that happens to sit inside another app's phrase — and a match is discarded only when its span lies strictly inside another app's longer match at that position (containment with an unequal span; equal spans of two apps both stay). An app is reported when any of its matches survives, with the first surviving keyword in table order as before. Against the shipped precheck_apps.json: smartlead+gmail reply to the cold email from Bob in my inbox smartlead+gmail email me when the email sequence finishes smartlead+gmail send an email about the cold email campaign smartlead+gmail draft an email to Bob and check my email campaign stats smartlead+gmail set up a cold email campaign, then email Bob the results smartlead set up a Smartlead campaign smartlead cold email campaign The last row is the intended suppression: Gmail's only match is the "email" inside "cold email" / "cold email campaign", same position, so it stays quiet. The new test pins all seven texts plus the reported keyword for the mixed case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

Phone: enforce exact greeting, prevent hallucinated scenarios

7326fca

Gemini was making up tasks ("could you check my screen?") on inbound calls. Now says exact greeting and waits for user to speak first. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

sonichi merged commit c323dd8 into main Mar 29, 2026

sonichi deleted the fix-phone-greeting branch March 29, 2026 02:52

This was referenced Apr 5, 2026

Fix command injection in sips call (CodeQL) #177

Merged

Infrastructure, security & tooling improvements #178

Merged

liususan091219 added a commit that referenced this pull request Apr 6, 2026

Add CodeQL #27 citation comment to sips fix

4921dc9

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

sonichi mentioned this pull request Apr 17, 2026

fix(recording-tools): use execFileSync for ffmpeg/open/ffprobe in subtitle burn + open_file #408

Merged

This was referenced Apr 28, 2026

fix(open-file): move out of recording-tools, fully decouple #544

Closed

fix(tools): dedup + open-file generic + summon/recording reliability sweep #546

Merged

feat(#560): open_file optional app parameter for explicit + contextual app routing #605

Merged

JasonOA888 mentioned this pull request Jun 4, 2026

security: systematic execSync → execFileSync hardening (67 call sites, 1 unescaped injection surface) #1451

Closed

sonichi mentioned this pull request Jul 30, 2026

chore(lint): refuse bridge tests that import without isolating CLAUDE_CONFIG_DIR #2429

Merged

sonichi mentioned this pull request Jul 31, 2026

fix(discord-bridge): keep internal notices out of public channels (seed notice → owner DM; swallow sandbox fallback) #2431

Draft

sonichi mentioned this pull request Aug 8, 2026

fix(tests): #2615 rebound 2 of 9 import-time paths, so one still hit live state #2738

Merged

elCaptnCode mentioned this pull request Sep 7, 2026

feat(windows): Windows support across core services + Claude Code core #1789

Merged

bassilkhilo-ag2 mentioned this pull request Sep 23, 2026

fix(owner-facing): email routes to the Station connector, a zero-quota Gemini key is named, campaign work goes to campaign-runner #4661

Merged

Navigation Menu

Repository navigation

Fix phone greeting hallucination - #27

Fix phone greeting hallucination#27
sonichi merged 1 commit into
mainfrom
fix-phone-greeting

sonichi commented Mar 29, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

sonichi commented Mar 29, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant