Expand troubleshooting in README by sonichi · Pull Request #14 · sonichi/sutando

sonichi · 2026-03-28T18:09:47Z

Adds common voice agent errors (API key, port conflict) and restart.sh.

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

Pulls in the bodhi #14 silent-context-injection fix on Gemini reconnect. Combined with PR #515 (merged into main, now in this branch via merge), this covers both reconnect paths: - Gemini-internal reconnect: silent injection drops "Say I'm back" prompt - Client reconnect <60s: getSecondsSinceLastTurn suppresses "Welcome back" Voice-agent verified: skill-loader still fires 3 tools on boot (PID 23502). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ged)" This reverts commit f9a4be4.

Pairs with the system_instruction presenter marker (374cebf): with the marker now authoritative + sent on every session.start, the new bodhi's silent context injection should be safe — the per-cue gating in voice- context.txt is enforced from the system_instruction, not relying on the flat history replay to anchor model behavior. Trade-off in case of regression: revert this commit, voice-agent runs on old bodhi 4d1592eb (welcome-back fires, but no concatenation risk). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(mode-indicator): full 3-mode UI recovered from Apr 21 session jsonl Restores the 3-mode (active / meeting / presenter) UI that was lost when `git reset --hard HEAD~2` on dev/apr-21-local-ships at 2026-04-23 06:22 PT discarded uncommitted working-tree edits alongside two sync-memory commits. Recovered by replaying 15 Edit calls from session 6eb8f10e jsonl (2026-04-21 23:18-23:54 PT) on top of dev/apr-21-local-ships's presenter-dot indicator base. Replay applied 15/15 cleanly. Sutando.app (`src/Sutando/main.swift`): - New properties: `voiceMode`, `presenterModeActive`, three menu-item weak refs (`modeActiveMenuItem`, `modeMeetingMenuItem`, `modePresenterMenuItem`). - Three-mode radio in menu bar dropdown: "Mode: Active" / "Mode: Meeting" / "Mode: Presenter". Exactly one has ● at a time. Clicking any item switches: Active/Meeting write `state/voice-mode.request` (voice-agent picks up on 1s poll); Presenter POSTs to `:7877/presenter/on`. - Avatar badge: `avatarImage(presenterActive:meetingActive:)` paints a small purple dot when presenter is active, or amber dot for meeting — matches the web UI mode-pill colors. - New polls: `pollPresenterMode()` + `pollVoiceMode()` every 1s on the same timer. Both silent-fail if their backend is down. - `updateModeMenuItem()` recomposes the radio whenever either signal flips. Priority: presenter > meeting > active. Voice-agent (`src/voice-agent.ts`): - `switchModeTool.execute()` writes `state/voice-mode.txt` on switch_mode("active"|"meeting") so cross-process readers (web-client, Sutando.app) can resolve the unified mode. - `applyModeRequest()` polls `state/voice-mode.request` every 1s, applies the mode flip, deletes the request file. Lets Sutando.app send mode requests without an HTTP server in voice-agent. Web client (`src/web-client.ts`): - `mode` field added to /sse-status response — built from sync read of `state/voice-mode.txt` + cached presenter-active boolean (refreshed every 1s in background from :7877/presenter). - Mode pill in top-bar UI: 3 CSS variants (mode-active dim / mode-meeting amber / mode-presenter purple-glow). Polls /sse-status every 1.5s. Recovery details: notes/3-mode-recovered-edits-2026-04-21.md. Earlier #518 (simpler text-only version) is closed in favor of this fuller recovery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(voice-agent): sync voice-mode sentinel on switch_mode + after Zoom detect Two related bugs in the recovered 3-mode work surfaced when Chi tested the menu radio: clicking "Mode: Meeting" silently no-op'd because the in-memory `meetingActive` flag and the on-disk `state/voice-mode.txt` sentinel had diverged. (1) `switchModeTool.execute` flipped `meetingActive` but never called `writeVoiceModeSentinel()`. So a voice-triggered switch_mode kept the sentinel stale, and a subsequent menu click that wrote `voice-mode.request="meeting"` hit the `meetingActive === want` early-return in `applyModeRequest` — request consumed without writing the sentinel. Sutando.app's pollVoiceMode kept reading "active" → menu radio stuck. (2) The startup `writeVoiceModeSentinel()` call ran BEFORE the Zoom auto-detect, so even when Zoom was detected as in-meeting and `meetingActive` was set to true, the sentinel had already been written as "active" with no second write. Fix: call `writeVoiceModeSentinel()` inside `switchModeTool.execute`, and move the startup call to after the Zoom auto-detect block. Verified live: kickstart → sentinel "meeting" matches in-memory state (Zoom-running case); write voice-mode.request="meeting" → sentinel flips, log line "External request applied: mode=meeting" fires. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(voice-agent): elevate presenter_mode to direct tool trigger Mirrors the switch_mode pattern. Without this, "presenter mode on" / "the talk starts" routes to the work tool, producing "working on it" instead of silently flipping the mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(inline-tools): restore skill manifest loader stripped by PR #505 PR #505 (dup-name guard) inadvertently removed loadSkillManifestTools() along with the personalTools spread into inlineTools. Voice-agent had no way to register highlight_slide / presenter_mode tools from skills/personal-iclr-highlight/, so the autonav cue silently no-op'd during the ICLR talk rehearsal. Restored verbatim from 9b545c2 (the original local-ships commit). Smoke- tested: voice-agent boots with "[skill-loader] loaded 3 tool(s) from iclr-highlight" — highlightSlideTool, presenterModeTool, fullscreenTool (renamed to fullscreen_presenter to dodge the dup-name guard). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(voice-agent): reject filler words; disambig screen_record from fullscreen Two pre-talk hardenings observed during ICLR rehearsal: 1. recording-tools.ts: screen_record description now explicitly rejects "fullscreen" / "full screen" / "play fullscreen" cues. Chi asked to play the cross-owner video fullscreen and Sutando self-fired screen_record (06:34:46) right after fullscreen_presenter + play_video — STT or model association from the word "screen" was matching screen_record. 2. voice-agent.ts: restored FILLERS ARE NOT REQUESTS rule (originally on the unmerged 9b545c2 local-ships branch). Short utterances "hmm" / "um" / "ok" / "[BLANK_AUDIO]" are not instructions — Sutando should stay silent or ack, not call work and say "queued up". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(deps): bump bodhi-realtime-agent to ee58489 (PR #14 merged) Pulls in the bodhi #14 silent-context-injection fix on Gemini reconnect. Combined with PR #515 (merged into main, now in this branch via merge), this covers both reconnect paths: - Gemini-internal reconnect: silent injection drops "Say I'm back" prompt - Client reconnect <60s: getSecondsSinceLastTurn suppresses "Welcome back" Voice-agent verified: skill-loader still fires 3 tools on boot (PID 23502). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "chore(deps): bump bodhi-realtime-agent to ee58489 (PR #14 merged)" This reverts commit f9a4be4. * feat(voice-agent): inject presenter-state into greeting + reconnect prompts Closes the gap that surfaced during talk rehearsal: voice-agent restart loses co-presenter mode anchor. Even though :7877/presenter still says active=true, the model gets the default Sutando system prompt and defaults to "Echo Act IV" generic greeting, routing slide-topic phrases to work instead of highlight_slide+narrate. Adds getPresenterStateMarker() that synchronously curls the highlight server and returns "[System: PRESENTER MODE IS CURRENTLY ACTIVE — apply the CO-PRESENTER protocol...]" when active. Appended to both greeting paths: - Fresh connect (line 484): generic greeting + presenter marker - Reconnect with history (line 453): reconnect prompt + presenter marker Failure-silent — if curl fails the marker is empty string, so non-talk sessions and missing iclr-highlight skill are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(voice-agent): suppress "Welcome back" when presenter mode is active Reconnect path was still emitting "Welcome back" when presenter is on, which breaks the co-presenter flow mid-talk. Reuses the existing quick-reconnect silent-reconnect hint when presenterActive is true, keyed off getPresenterStateMarker() returning non-empty. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(voice-agent): make instructions a per-session factory + inject presenter marker Previous a284190 + 5eb09af put the presenter-state marker in the GREETING string. Gemini Live treats the greeting as a user-style turn — the model called get_core_status to verify the "claim" instead of trusting it, and answered "presenter mode is not currently active" despite :7877/presenter being active. System instructions are authoritative. This commit: 1. Converts mainAgent.instructions from a static joined string to a factory function `() => [...].join('\n')`. Each session.start() now re-evaluates the array, picking up live state. 2. Adds getPresenterStateMarker() as the FIRST slot in the array — so the system prompt opens with "[System: PRESENTER MODE IS CURRENTLY ACTIVE...]" when the iclr-highlight server says active=true. Failure-silent — if the curl fails, the marker is empty string and the slot drops out of the join cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(deps): re-bump bodhi-realtime-agent to ee58489 (PR #14) Pairs with the system_instruction presenter marker (374cebf): with the marker now authoritative + sent on every session.start, the new bodhi's silent context injection should be safe — the per-cue gating in voice- context.txt is enforced from the system_instruction, not relying on the flat history replay to anchor model behavior. Trade-off in case of regression: revert this commit, voice-agent runs on old bodhi 4d1592eb (welcome-back fires, but no concatenation risk). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(open-file): QuickTime fullscreen survives Zoom screen-share The 'present' Apple event succeeds but the visible window doesn't come forward when Zoom is screen-sharing — Zoom's floating control bar holds the foreground z-order. Switch to Ctrl+Cmd+F routed through 'tell process "QuickTime Player"' so System Events targets QT directly instead of going through global keystroke focus that Zoom is grabbing. Same pattern as the Chrome fullscreen fix in skills/personal-iclr-highlight/tools.ts. Validated locally — TS 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>

…ndo-skills (#995) The main-repo `skills/info-radar/` was a SKILL.md-only stub: no scripts, no manifest, and no cron driving it. The functional info-radar — adaptive-x-feed recommender + x-radar-daily + append-signal + tests — lives in the private `sonichi/sutando-skills` repo (added #14, fixed #15), whose SKILL.md is a strict superset of this stub (keeps the static topic-monitor as a Legacy section, adds the adaptive recommender). Keeping both created a dual-home: `install.sh` skipped re-linking info-radar ("symlink points elsewhere"), pinning `~/.claude/skills/info-radar` to this non-functional stub while the real version stayed dormant. Removing the stub lets the next skills-sync install re-link to the functional copy. info-radar is intentionally staged private (sutando-skills) while its scripts are not yet owner-validated; it returns to the public repo once tested. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ichi#14) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…chi#520) * feat(mode-indicator): full 3-mode UI recovered from Apr 21 session jsonl Restores the 3-mode (active / meeting / presenter) UI that was lost when `git reset --hard HEAD~2` on dev/apr-21-local-ships at 2026-04-23 06:22 PT discarded uncommitted working-tree edits alongside two sync-memory commits. Recovered by replaying 15 Edit calls from session 6eb8f10e jsonl (2026-04-21 23:18-23:54 PT) on top of dev/apr-21-local-ships's presenter-dot indicator base. Replay applied 15/15 cleanly. Sutando.app (`src/Sutando/main.swift`): - New properties: `voiceMode`, `presenterModeActive`, three menu-item weak refs (`modeActiveMenuItem`, `modeMeetingMenuItem`, `modePresenterMenuItem`). - Three-mode radio in menu bar dropdown: "Mode: Active" / "Mode: Meeting" / "Mode: Presenter". Exactly one has ● at a time. Clicking any item switches: Active/Meeting write `state/voice-mode.request` (voice-agent picks up on 1s poll); Presenter POSTs to `:7877/presenter/on`. - Avatar badge: `avatarImage(presenterActive:meetingActive:)` paints a small purple dot when presenter is active, or amber dot for meeting — matches the web UI mode-pill colors. - New polls: `pollPresenterMode()` + `pollVoiceMode()` every 1s on the same timer. Both silent-fail if their backend is down. - `updateModeMenuItem()` recomposes the radio whenever either signal flips. Priority: presenter > meeting > active. Voice-agent (`src/voice-agent.ts`): - `switchModeTool.execute()` writes `state/voice-mode.txt` on switch_mode("active"|"meeting") so cross-process readers (web-client, Sutando.app) can resolve the unified mode. - `applyModeRequest()` polls `state/voice-mode.request` every 1s, applies the mode flip, deletes the request file. Lets Sutando.app send mode requests without an HTTP server in voice-agent. Web client (`src/web-client.ts`): - `mode` field added to /sse-status response — built from sync read of `state/voice-mode.txt` + cached presenter-active boolean (refreshed every 1s in background from :7877/presenter). - Mode pill in top-bar UI: 3 CSS variants (mode-active dim / mode-meeting amber / mode-presenter purple-glow). Polls /sse-status every 1.5s. Recovery details: notes/3-mode-recovered-edits-2026-04-21.md. Earlier sonichi#518 (simpler text-only version) is closed in favor of this fuller recovery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(voice-agent): sync voice-mode sentinel on switch_mode + after Zoom detect Two related bugs in the recovered 3-mode work surfaced when Chi tested the menu radio: clicking "Mode: Meeting" silently no-op'd because the in-memory `meetingActive` flag and the on-disk `state/voice-mode.txt` sentinel had diverged. (1) `switchModeTool.execute` flipped `meetingActive` but never called `writeVoiceModeSentinel()`. So a voice-triggered switch_mode kept the sentinel stale, and a subsequent menu click that wrote `voice-mode.request="meeting"` hit the `meetingActive === want` early-return in `applyModeRequest` — request consumed without writing the sentinel. Sutando.app's pollVoiceMode kept reading "active" → menu radio stuck. (2) The startup `writeVoiceModeSentinel()` call ran BEFORE the Zoom auto-detect, so even when Zoom was detected as in-meeting and `meetingActive` was set to true, the sentinel had already been written as "active" with no second write. Fix: call `writeVoiceModeSentinel()` inside `switchModeTool.execute`, and move the startup call to after the Zoom auto-detect block. Verified live: kickstart → sentinel "meeting" matches in-memory state (Zoom-running case); write voice-mode.request="meeting" → sentinel flips, log line "External request applied: mode=meeting" fires. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(voice-agent): elevate presenter_mode to direct tool trigger Mirrors the switch_mode pattern. Without this, "presenter mode on" / "the talk starts" routes to the work tool, producing "working on it" instead of silently flipping the mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(inline-tools): restore skill manifest loader stripped by PR sonichi#505 PR sonichi#505 (dup-name guard) inadvertently removed loadSkillManifestTools() along with the personalTools spread into inlineTools. Voice-agent had no way to register highlight_slide / presenter_mode tools from skills/personal-iclr-highlight/, so the autonav cue silently no-op'd during the ICLR talk rehearsal. Restored verbatim from 9b545c2 (the original local-ships commit). Smoke- tested: voice-agent boots with "[skill-loader] loaded 3 tool(s) from iclr-highlight" — highlightSlideTool, presenterModeTool, fullscreenTool (renamed to fullscreen_presenter to dodge the dup-name guard). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(voice-agent): reject filler words; disambig screen_record from fullscreen Two pre-talk hardenings observed during ICLR rehearsal: 1. recording-tools.ts: screen_record description now explicitly rejects "fullscreen" / "full screen" / "play fullscreen" cues. Chi asked to play the cross-owner video fullscreen and Sutando self-fired screen_record (06:34:46) right after fullscreen_presenter + play_video — STT or model association from the word "screen" was matching screen_record. 2. voice-agent.ts: restored FILLERS ARE NOT REQUESTS rule (originally on the unmerged 9b545c2 local-ships branch). Short utterances "hmm" / "um" / "ok" / "[BLANK_AUDIO]" are not instructions — Sutando should stay silent or ack, not call work and say "queued up". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(deps): bump bodhi-realtime-agent to ee58489 (PR sonichi#14 merged) Pulls in the bodhi sonichi#14 silent-context-injection fix on Gemini reconnect. Combined with PR sonichi#515 (merged into main, now in this branch via merge), this covers both reconnect paths: - Gemini-internal reconnect: silent injection drops "Say I'm back" prompt - Client reconnect <60s: getSecondsSinceLastTurn suppresses "Welcome back" Voice-agent verified: skill-loader still fires 3 tools on boot (PID 23502). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "chore(deps): bump bodhi-realtime-agent to ee58489 (PR sonichi#14 merged)" This reverts commit f9a4be4. * feat(voice-agent): inject presenter-state into greeting + reconnect prompts Closes the gap that surfaced during talk rehearsal: voice-agent restart loses co-presenter mode anchor. Even though :7877/presenter still says active=true, the model gets the default Sutando system prompt and defaults to "Echo Act IV" generic greeting, routing slide-topic phrases to work instead of highlight_slide+narrate. Adds getPresenterStateMarker() that synchronously curls the highlight server and returns "[System: PRESENTER MODE IS CURRENTLY ACTIVE — apply the CO-PRESENTER protocol...]" when active. Appended to both greeting paths: - Fresh connect (line 484): generic greeting + presenter marker - Reconnect with history (line 453): reconnect prompt + presenter marker Failure-silent — if curl fails the marker is empty string, so non-talk sessions and missing iclr-highlight skill are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(voice-agent): suppress "Welcome back" when presenter mode is active Reconnect path was still emitting "Welcome back" when presenter is on, which breaks the co-presenter flow mid-talk. Reuses the existing quick-reconnect silent-reconnect hint when presenterActive is true, keyed off getPresenterStateMarker() returning non-empty. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(voice-agent): make instructions a per-session factory + inject presenter marker Previous a284190 + 5eb09af put the presenter-state marker in the GREETING string. Gemini Live treats the greeting as a user-style turn — the model called get_core_status to verify the "claim" instead of trusting it, and answered "presenter mode is not currently active" despite :7877/presenter being active. System instructions are authoritative. This commit: 1. Converts mainAgent.instructions from a static joined string to a factory function `() => [...].join('\n')`. Each session.start() now re-evaluates the array, picking up live state. 2. Adds getPresenterStateMarker() as the FIRST slot in the array — so the system prompt opens with "[System: PRESENTER MODE IS CURRENTLY ACTIVE...]" when the iclr-highlight server says active=true. Failure-silent — if the curl fails, the marker is empty string and the slot drops out of the join cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(deps): re-bump bodhi-realtime-agent to ee58489 (PR sonichi#14) Pairs with the system_instruction presenter marker (374cebf): with the marker now authoritative + sent on every session.start, the new bodhi's silent context injection should be safe — the per-cue gating in voice- context.txt is enforced from the system_instruction, not relying on the flat history replay to anchor model behavior. Trade-off in case of regression: revert this commit, voice-agent runs on old bodhi 4d1592eb (welcome-back fires, but no concatenation risk). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(open-file): QuickTime fullscreen survives Zoom screen-share The 'present' Apple event succeeds but the visible window doesn't come forward when Zoom is screen-sharing — Zoom's floating control bar holds the foreground z-order. Switch to Ctrl+Cmd+F routed through 'tell process "QuickTime Player"' so System Events targets QT directly instead of going through global keystroke focus that Zoom is grabbing. Same pattern as the Chrome fullscreen fix in skills/personal-iclr-highlight/tools.ts. Validated locally — TS 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>

…ndo-skills (sonichi#995) The main-repo `skills/info-radar/` was a SKILL.md-only stub: no scripts, no manifest, and no cron driving it. The functional info-radar — adaptive-x-feed recommender + x-radar-daily + append-signal + tests — lives in the private `sonichi/sutando-skills` repo (added sonichi#14, fixed sonichi#15), whose SKILL.md is a strict superset of this stub (keeps the static topic-monitor as a Legacy section, adds the adaptive recommender). Keeping both created a dual-home: `install.sh` skipped re-linking info-radar ("symlink points elsewhere"), pinning `~/.claude/skills/info-radar` to this non-functional stub while the real version stayed dormant. Removing the stub lets the next skills-sync install re-link to the functional copy. info-radar is intentionally staged private (sutando-skills) while its scripts are not yet owner-validated; it returns to the public repo once tested. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… regex qingyun-wu's second review round on #3323 cited REVIEW.md rule #14 ("never assert on source text as a stand-in for a behavioral claim") against the guard-based structural test — even anchored to the skip branch (88f3839), it still only regexes discord-bridge.py's source rather than exercising the real decision. Extract the skip check into an importable, dependency-light helper — has_skip_action(actions) in src/result_markers.py — and update poll_proactive/poll_dm_fallback in discord-bridge.py to call it instead of an inline `any(a.kind == "skip" for a in actions)`. A third site (the [deduped:] cross-channel-rejection branch) needs the full Action object, not a bool, and is left untouched — different shape, out of scope per CLAUDE.md's "fix the policy that duplicates, not every shape that happens to look similar." The test now exercises has_skip_action() behaviorally with real parse_markers() output (no-send/REPLIED/deduped all skip; ordinary text does not), plus narrowly-scoped structural pins (delegation to the shared helper, no inline reimplementation, discard-before-continue ordering) — legitimate under REVIEW.md's structural exception because each is paired with the behavioral test above, never a substitute. Verified: py_compile clean, review-checks.sh clean, 8 sibling tests unaffected (bridge-marker-no-leak, result-markers 48 tests, proactive-channel-redirect-loud-failure, health-check-proactive- quarantine 8 tests, bridge-dedup-poll-loop 4 tests, bridge-dedup- wrappers 4 tests, dedup-cross-channel 15 tests, dedup-holder-delivered 18 tests). Live post-restart round trip on the production host: restarted discord-bridge, a [no-send]-marked proactive file was cleanly dropped (no send attempt), and a new ordinary proactive file delivered normally — zero new results/undelivered/ items since restart.

…onor suppression markers qingyun-wu's second CHANGES_REQUESTED on this PR, verbatim: Blocking: the new head narrows the string search, but the regression test still does not prove that the suppression predicate controls the terminal drop. For example, changing the production branch to `if False and any(a.kind == "skip" for a in _pp.actions):` leaves parse_markers, the `kind == "skip"` text, `.drop(...)`, `continue`, and their ordering intact, so every guard in this test still passes while `[no-send]` files again fall through to Discord delivery. This is the same mutation called out on the previous head. Please exercise `poll_proactive` behavior (or extract the parse/drop decision into a small callable and test it) so that disabling the condition makes the regression test fail. f669570's extraction of has_skip_action() into src/result_markers.py was still insufficient: applying qingyun's exact mutation, `if False and has_skip_action(_pp.actions):`, inside poll_proactive and rerunning tests/proactive-suppression-marker-honored.test.py still PASSED (should have failed) — that test only greps poll_proactive's source text for `has_skip_action(`, `.drop(`, `continue`, never executes the function. Rather than extract poll_proactive/poll_dm_fallback into separate `_route_*_file` callables (the larger refactor originally proposed), this adds tests/proactive-suppression-marker-behavioral.test.py, which directly executes the real, UNMODIFIED poll_proactive() and poll_dm_fallback() coroutines — the alternative qingyun's own comment explicitly offered ("or exercise poll_proactive behavior") — using the same asyncio.run()+sleep-sentinel stub-and-redirect pattern already established by tests/proactive-dm-failure-keeps-file-behaviour.test.py. A large code-motion extraction was rejected: it would need many continue->return conversions in deeply-nested try/except blocks (real control-flow risk for zero behavior change), and since plain `git diff` treats moved code as delete+add, it would trip this repo's prose_cap:2 comment-line cap on every relocated multi-line comment (e.g. the ~13-line "Claim-by-rename" and "#1147 follow-up" comments) for no functional benefit. Both existing structural tests (proactive-suppression-marker-honored, proactive-channel-redirect-loud-failure) are left untouched, per the task constraint not to weaken them. For poll_proactive, the mutation-resistance signal is the ProactiveClaimFence terminal call (.drop() vs .confirm()), not send content: parse_markers() already blanks the body for ANY skip-matched marker independent of has_skip_action, so a [no-send] file's clean_text is "" whether or not the predicate is disabled, and a content-only check (e.g. "was deliver() called with real text") cannot see the mutation. The fence call can: disabling the predicate flips poll_proactive from .drop() (suppressed) to .confirm() (treated as sent) for that file, which is exactly the "does the suppression predicate control the terminal drop" property under review. A _FenceProxy wraps the real ProactiveClaimFence and records only calls poll_proactive itself makes through _proactive_fence() (drop()'s own internal self.confirm() call stays invisible to the wrapper), giving an unambiguous, uncontaminated signal. For poll_dm_fallback, the signal is whether the dm-result.py subprocess is invoked at all (its argv), not whether a DM was ultimately sent — dm-result.py has its own independent, redundant skip-marker check (tests/dm-result-skip-markers.test.py) that would otherwise mask a mutation of poll_dm_fallback's own predicate. check_mutation_makes_it_fail() is a PERMANENT guard, not a one-off manual step: it reads the real discord-bridge.py source, applies qingyun's exact mutation via a verified-unique string substitution to a COPY, writes it to a temp file, loads it as a genuinely separate module via importlib (the same hyphenated-filename pattern used by ~80 other tests in this repo), and asserts the real behavioral signal flips. This executes the mutated code rather than matching its text, directly answering rule #14 in REVIEW.md ("never assert on source text as a stand-in for a behavioral claim"). Manual verification (mutation applied directly to the real, tracked src/discord-bridge.py in this checkout, then reverted — not just the test's internal copy-based mechanism): $ python3 tests/proactive-suppression-marker-behavioral.test.py; echo $? PASS: poll_proactive/poll_dm_fallback behaviorally honor suppression markers, and disabling the check (mutation) makes this test fail 0 $ python3 - <<'EOF' # apply qingyun's exact mutation to both call sites ... if has_skip_action(_pp.actions): -> if False and has_skip_action(_pp.actions): ... if has_skip_action(_parsed_fb.actions): -> if False and has_skip_action(_parsed_fb.actions): EOF $ python3 tests/proactive-suppression-marker-behavioral.test.py; echo $? FAIL: poll_proactive: [no-send] file ... did not take the fence .drop() (suppressed) path — fence calls were ['confirm'], expected ['drop'] FAIL: poll_dm_fallback: [no-send] file ... reached the dm-result.py subprocess 1 $ git checkout -- src/discord-bridge.py $ python3 tests/proactive-suppression-marker-behavioral.test.py; echo $? PASS: ... 0 Both functions' predicates are proven to control the observed outcome in both directions (mutation makes it fail, revert makes it pass again). Also fixes a separate drift found mid-review: src/result_markers.py's has_skip_action() (added in f669570) was never synced to its vendored mirror at packages/ag2-sparrow/ag2_sparrow/result_markers.py, so packages/ag2-sparrow/tools/test_no_drift.py was failing. Regenerated via `python3 packages/ag2-sparrow/tools/sync_from_src.py` — the diff is the 6-line has_skip_action() addition, verbatim from src/, with no other mirrored module touched. Regression (all green): python3 -m py_compile src/discord-bridge.py src/result_markers.py tests/proactive-suppression-marker-honored.test.py tests/proactive-suppression-marker-behavioral.test.py (new) tests/bridge-marker-no-leak.test.py tests/result-markers.test.py tests/proactive-channel-redirect-loud-failure.test.py tests/health-check-proactive-quarantine.test.py tests/bridge-dedup-poll-loop.test.py tests/bridge-dedup-wrappers.test.py tests/dedup-cross-channel.test.py tests/dedup-holder-delivered.test.py tests/proactive-dm-failure-keeps-file-behaviour.test.py packages/ag2-sparrow/tools/test_no_drift.py git diff | bash scripts/review-checks.sh -> PASS No production code (src/discord-bridge.py) was modified. This commit adds one test file and syncs the ag2-sparrow mirror. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(discord-bridge): honor suppression markers in poll_proactive poll_dm_fallback() checks for skip-kind markers ([no-send]/[REPLIED]/ [deduped:]) before attempting DM delivery, and its own docstring claims poll_proactive() already does the same. It didn't: a suppressed proactive-*.txt still got DM-attempted, the attempt failed (no valid send target), and fail() parked it permanently in results/undelivered/ instead of the clean discard the marker asked for. 77 files had accumulated this way. Fix mirrors poll_dm_fallback's own check: parse the body once (already done for the redirect-marker parse just below), and if any action is skip-kind, drop the claim via the existing ProactiveClaimFence.drop() path (already used elsewhere for unresolvable recipients) instead of proceeding to delivery. Regression test: tests/proactive-suppression-marker-honored.test.py FAILs on the pre-fix loop body, PASSes with the fix. * fix(tests): anchor suppression-marker guards to the skip branch, not the whole function Guard 3 (`.drop(` presence) searched the entire poll_proactive body, but the function has a second, unrelated `.drop(f, "no human user in allowFrom")` call — so the guard passed even when the skip branch's own drop() was swapped for release() (verified: a mutated .release() build passed all 5 guards before this fix). Anchor it to the same skip_idx..skip_idx+400 window Guard 5 already uses. Guard 4 (send-ordering) only checked the literal `.send(` token, which never appears on the primary text-DM path — that goes through the `deliver_text(` helper. `.send(` only occurs on file-attachment sub-paths, so the guard gave no real signal about the path that actually matters. Now checks both tokens and orders against whichever occurs first. Verified fail-before (pre-fix commit)/pass-after and against a synthetic release()-regression build; sibling tests (proactive-channel-redirect-loud-failure, health-check-proactive-quarantine) still pass. Reported by kewei-red-ag2space in review of PR #3323. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(tests): make suppression-marker check behavioral, not source-text regex qingyun-wu's second review round on #3323 cited REVIEW.md rule #14 ("never assert on source text as a stand-in for a behavioral claim") against the guard-based structural test — even anchored to the skip branch (88f3839), it still only regexes discord-bridge.py's source rather than exercising the real decision. Extract the skip check into an importable, dependency-light helper — has_skip_action(actions) in src/result_markers.py — and update poll_proactive/poll_dm_fallback in discord-bridge.py to call it instead of an inline `any(a.kind == "skip" for a in actions)`. A third site (the [deduped:] cross-channel-rejection branch) needs the full Action object, not a bool, and is left untouched — different shape, out of scope per CLAUDE.md's "fix the policy that duplicates, not every shape that happens to look similar." The test now exercises has_skip_action() behaviorally with real parse_markers() output (no-send/REPLIED/deduped all skip; ordinary text does not), plus narrowly-scoped structural pins (delegation to the shared helper, no inline reimplementation, discard-before-continue ordering) — legitimate under REVIEW.md's structural exception because each is paired with the behavioral test above, never a substitute. Verified: py_compile clean, review-checks.sh clean, 8 sibling tests unaffected (bridge-marker-no-leak, result-markers 48 tests, proactive-channel-redirect-loud-failure, health-check-proactive- quarantine 8 tests, bridge-dedup-poll-loop 4 tests, bridge-dedup- wrappers 4 tests, dedup-cross-channel 15 tests, dedup-holder-delivered 18 tests). Live post-restart round trip on the production host: restarted discord-bridge, a [no-send]-marked proactive file was cleanly dropped (no send attempt), and a new ordinary proactive file delivered normally — zero new results/undelivered/ items since restart. * test(proactive): behaviorally prove poll_proactive/poll_dm_fallback honor suppression markers qingyun-wu's second CHANGES_REQUESTED on this PR, verbatim: Blocking: the new head narrows the string search, but the regression test still does not prove that the suppression predicate controls the terminal drop. For example, changing the production branch to `if False and any(a.kind == "skip" for a in _pp.actions):` leaves parse_markers, the `kind == "skip"` text, `.drop(...)`, `continue`, and their ordering intact, so every guard in this test still passes while `[no-send]` files again fall through to Discord delivery. This is the same mutation called out on the previous head. Please exercise `poll_proactive` behavior (or extract the parse/drop decision into a small callable and test it) so that disabling the condition makes the regression test fail. f669570's extraction of has_skip_action() into src/result_markers.py was still insufficient: applying qingyun's exact mutation, `if False and has_skip_action(_pp.actions):`, inside poll_proactive and rerunning tests/proactive-suppression-marker-honored.test.py still PASSED (should have failed) — that test only greps poll_proactive's source text for `has_skip_action(`, `.drop(`, `continue`, never executes the function. Rather than extract poll_proactive/poll_dm_fallback into separate `_route_*_file` callables (the larger refactor originally proposed), this adds tests/proactive-suppression-marker-behavioral.test.py, which directly executes the real, UNMODIFIED poll_proactive() and poll_dm_fallback() coroutines — the alternative qingyun's own comment explicitly offered ("or exercise poll_proactive behavior") — using the same asyncio.run()+sleep-sentinel stub-and-redirect pattern already established by tests/proactive-dm-failure-keeps-file-behaviour.test.py. A large code-motion extraction was rejected: it would need many continue->return conversions in deeply-nested try/except blocks (real control-flow risk for zero behavior change), and since plain `git diff` treats moved code as delete+add, it would trip this repo's prose_cap:2 comment-line cap on every relocated multi-line comment (e.g. the ~13-line "Claim-by-rename" and "#1147 follow-up" comments) for no functional benefit. Both existing structural tests (proactive-suppression-marker-honored, proactive-channel-redirect-loud-failure) are left untouched, per the task constraint not to weaken them. For poll_proactive, the mutation-resistance signal is the ProactiveClaimFence terminal call (.drop() vs .confirm()), not send content: parse_markers() already blanks the body for ANY skip-matched marker independent of has_skip_action, so a [no-send] file's clean_text is "" whether or not the predicate is disabled, and a content-only check (e.g. "was deliver() called with real text") cannot see the mutation. The fence call can: disabling the predicate flips poll_proactive from .drop() (suppressed) to .confirm() (treated as sent) for that file, which is exactly the "does the suppression predicate control the terminal drop" property under review. A _FenceProxy wraps the real ProactiveClaimFence and records only calls poll_proactive itself makes through _proactive_fence() (drop()'s own internal self.confirm() call stays invisible to the wrapper), giving an unambiguous, uncontaminated signal. For poll_dm_fallback, the signal is whether the dm-result.py subprocess is invoked at all (its argv), not whether a DM was ultimately sent — dm-result.py has its own independent, redundant skip-marker check (tests/dm-result-skip-markers.test.py) that would otherwise mask a mutation of poll_dm_fallback's own predicate. check_mutation_makes_it_fail() is a PERMANENT guard, not a one-off manual step: it reads the real discord-bridge.py source, applies qingyun's exact mutation via a verified-unique string substitution to a COPY, writes it to a temp file, loads it as a genuinely separate module via importlib (the same hyphenated-filename pattern used by ~80 other tests in this repo), and asserts the real behavioral signal flips. This executes the mutated code rather than matching its text, directly answering rule #14 in REVIEW.md ("never assert on source text as a stand-in for a behavioral claim"). Manual verification (mutation applied directly to the real, tracked src/discord-bridge.py in this checkout, then reverted — not just the test's internal copy-based mechanism): $ python3 tests/proactive-suppression-marker-behavioral.test.py; echo $? PASS: poll_proactive/poll_dm_fallback behaviorally honor suppression markers, and disabling the check (mutation) makes this test fail 0 $ python3 - <<'EOF' # apply qingyun's exact mutation to both call sites ... if has_skip_action(_pp.actions): -> if False and has_skip_action(_pp.actions): ... if has_skip_action(_parsed_fb.actions): -> if False and has_skip_action(_parsed_fb.actions): EOF $ python3 tests/proactive-suppression-marker-behavioral.test.py; echo $? FAIL: poll_proactive: [no-send] file ... did not take the fence .drop() (suppressed) path — fence calls were ['confirm'], expected ['drop'] FAIL: poll_dm_fallback: [no-send] file ... reached the dm-result.py subprocess 1 $ git checkout -- src/discord-bridge.py $ python3 tests/proactive-suppression-marker-behavioral.test.py; echo $? PASS: ... 0 Both functions' predicates are proven to control the observed outcome in both directions (mutation makes it fail, revert makes it pass again). Also fixes a separate drift found mid-review: src/result_markers.py's has_skip_action() (added in f669570) was never synced to its vendored mirror at packages/ag2-sparrow/ag2_sparrow/result_markers.py, so packages/ag2-sparrow/tools/test_no_drift.py was failing. Regenerated via `python3 packages/ag2-sparrow/tools/sync_from_src.py` — the diff is the 6-line has_skip_action() addition, verbatim from src/, with no other mirrored module touched. Regression (all green): python3 -m py_compile src/discord-bridge.py src/result_markers.py tests/proactive-suppression-marker-honored.test.py tests/proactive-suppression-marker-behavioral.test.py (new) tests/bridge-marker-no-leak.test.py tests/result-markers.test.py tests/proactive-channel-redirect-loud-failure.test.py tests/health-check-proactive-quarantine.test.py tests/bridge-dedup-poll-loop.test.py tests/bridge-dedup-wrappers.test.py tests/dedup-cross-channel.test.py tests/dedup-holder-delivered.test.py tests/proactive-dm-failure-keeps-file-behaviour.test.py packages/ag2-sparrow/tools/test_no_drift.py git diff | bash scripts/review-checks.sh -> PASS No production code (src/discord-bridge.py) was modified. This commit adds one test file and syncs the ag2-sparrow mirror. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Round-9 review findings: 1. The readiness-check guard from round 7 contradicted the readiness owner's own contract: src/delivery/readiness.py treats existence as NOT readiness (empty/unreadable/partial -> None), but the guard skip-continued on ANY existing file when PYBIN was unavailable -- silently reading a genuinely empty, undelivered result as done. Reproduced: PYBIN empty + a 0-byte result -> {} with no warning, exactly the false-negative readiness.py exists to prevent. Fixed by routing readiness-unknown into UNPROCESSED (preserving unprocessed status) so the existing no-interpreter warning/ fail-open path handles it honestly instead. 2. Both source-text scans from rounds 7-8 were behavioral surrogates (REVIEW.md #14) that pass on a dead comment or miss execution through a different variable ($_cand instead of $_stub). Replaced the PYBIN scan with a genuine behavioral witness: run the hook under `bash -x` and grep the trace for `+ '' ...` (bash's own rendering of an empty command about to execute) -- verified it catches a dead-comment false-pass the old regex missed. Replaced the stub-execution scan with the seam the review named: override `_sutando_git_is_system_stub` to classify a REAL RECORDING script as the stub (decoupling "is-stub" from "resolves to /usr/bin/git", which is what made a genuine witness possible at all), force xcode-select to fail, and assert the recording script's log stays empty. Verified against both named mutations (`"$_stub" --version` and execution through $_cand). 3. Trimmed the remaining cited comment blocks (one production, six test) to the 2-line cap. 22/22 + 15/15 local checks pass; review-checks.sh and shellcheck clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…lated repo's worktree (#4323) * fix(hooks): Stop-hook queue/turn-gate skips guest sessions in an unrelated repo's worktree check-pending-tasks.sh is inherited by every Claude Code session with this repo as an ancestor via CLAUDE.md, including one whose actual cwd is a worktree of a genuinely different repo (e.g. a skill-repo fix branch) that merely picked up this file. Per CLAUDE.md's "Chat-path task tracking" guest carve-out, such a session is a guest, not the core: Sutando's own task/result queue and its per-turn message discipline are the core's operating rules, not a universal Claude Code requirement, so a guest's Stop must not be held hostage to either. Detect this by comparing `git rev-parse --git-common-dir` for the CWD against the same for this hook's own repo (resolved from $0, not cwd) — worktrees of the SAME repo share one common-dir, so an own-repo worktree still counts as the core and still gets the gate; only a different repo's common-dir exits early with `{}` before the task-queue loop AND before turn_ledger.py's stop-gate. Tests: two new cases in check-pending-tasks-workspace.test.sh. Case 7 pins the defect (a pending core task must not block a session cwd'd into an unrelated repo's worktree); case 8 is the control proving the carve-out is keyed on "different repo", not "any worktree" (a worktree of THIS repo must still block). Mutation-tested: reverting the fix fails case 7 and leaves case 8 green, confirming each case measures what it claims. Stand: Echo Act IV Pro * fix(hooks): resolve the repo's git-common-dir with --path-format=absolute CI caught what this host's git couldn't reproduce: plain `git -C <dir> rev-parse --git-common-dir`, run from a DIFFERENT cwd, can print a path relative to <dir> (e.g. plain ".git") rather than to the caller. The previous fix's normalization then resolved that relative path against the CALLER's cwd, not <dir>'s — silently computing the wrong absolute path. Reproduced directly: a plain (non-submodule) repo, queried via -C from an unrelated cwd, returns ".git"; resolving that the old way computes "<caller-cwd>/.git" instead of "<dir>/.git". That collapses both directions of the guest-vs-core comparison: - case 7 (unrelated repo): the caller's own real common-dir and the miscomputed "repo" common-dir both landed on the caller's cwd -> read as SAME repo -> guest carve-out did not fire -> task wrongly blocked. - case 8 (this repo's own worktree): the worktree's real (absolute) common-dir did not match the miscomputed value -> read as DIFFERENT repo -> carve-out wrongly fired -> block wrongly skipped. `--path-format=absolute` (git >= 2.31; CI runs 2.55.0, this host 2.50.1) always prints an absolute path regardless of -C or cwd, removing the ambiguity outright -- and drops the dirname/cd/basename normalization that was reconstructing it by hand. Verified against the exact failing shape: a plain repo queried via -C from an unrelated cwd now returns an absolute path directly. Full suite (9 checks) passes locally; CI will confirm on this push. Stand: Echo Act IV Pro * fix(hooks): canonicalize the git-common-dir comparison past symlinks too Per Mini's review of this PR (#bot2bot, sonichi/sutando-skills#784 revival thread): --path-format=absolute fixes relative-vs-cwd, but two paths to the same directory can still differ as STRINGS if either crosses a symlink (macOS /tmp -> /private/tmp being the standing example in this codebase's own test helpers). Resolve both sides with `cd <dir> && pwd -P` after the git call, matching the `_real()` idiom this test file already uses for the same reason. Verified empirically that --path-format=absolute alone already handles every scenario Mini's review named (subdir-of-primary-checkout, a same-repo worktree, and a genuinely unrelated repo, tested directly against a plain non-submodule checkout) -- no live bug reproduced beyond the one already fixed. Added the pwd -P pass anyway as a costless, requested safety margin. Full suite still 9/9. Stand: Echo Act IV Pro * fix(hooks): route the Stop hook's git probes through safe git resolution keweichen (round 2, #4323): the guest-worktree carve-out's two git-common-dir probes called bare `git`, which resolves to the macOS CLT stub when the developer tools are absent -- invoking it raises a modal system dialog on every Stop, exactly what src/git_binary.py exists to prevent (REVIEW.md lesson 7). Added scripts/git-binary.sh, the shell twin of src/git_binary.py (no such bash-side resolver existed yet): PATH-first non-stub git, walking every PATH entry rather than trusting the first hit, falling back to the system git only when `xcode-select -p` confirms the developer tools are installed, or nothing. check-pending-tasks.sh now sources it and gates both probes on a resolved binary; when none is runnable it fails closed the same way the pre-fix hook always ran (proceeds as core -- cannot prove a different repo). Verified: tests/git-binary-sh.test.sh (8 new checks, pinning the same never-execute-to-probe contract as tests/python-binary-sh.test.sh) and the existing tests/check-pending-tasks-workspace.test.sh (9 checks) both pass. Mutation-tested by reverting check-pending-tasks.sh's hunk alone: the new "no bare git" / "sources git-binary.sh" checks fail exactly as expected, confirming they exercise the fix rather than passing regardless. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): non-Git bundle carve-out + symlink-to-stub in git resolver keweichen (round 2, #4323): two real blockers on the round-2 fix. 1. src/check-pending-tasks.sh's guest carve-out required BOTH CWD_COMMON_DIR and REPO_COMMON_DIR non-empty before comparing. A shipped app bundle has no .git at all, so REPO_COMMON_DIR is empty by design -- which made a genuinely foreign Git cwd read as "cannot tell" and fall through to blocking on the bundle's own queue anyway. Fixed: skip whenever CWD has a Git identity that isn't REPO_DIR's, including REPO_DIR having none; a CWD with no Git identity at all still fails closed (proceeds as core), matching the existing "no runnable git" fail-closed direction. 2. scripts/git-binary.sh's stub check compared only the candidate's DIRECTORY, not its resolved target -- so $HOME/bin/git symlinked to /usr/bin/git read as an ordinary PATH git and was returned even with no developer tools installed, and the hook then executed the actual CLT stub through the symlink. Added a hand-rolled realpath (no GNU-only `readlink -f`, no shelling to python3 -- that would re-enter the same landmine) that resolves the full candidate before comparing, and fails toward refusing (not toward "must be fine") when a symlink can't be resolved -- e.g. `readlink` itself missing from a minimal PATH. Also require a regular file, not just `-x` (a directory literally named `git` passes `-x` too). Verified: two direct reproductions before fixing (non-Git bundle + foreign Git cwd wrongly blocked; a stub-symlink resolved as non-empty), both fixed after. New regression cases: tests/git-binary-sh.test.sh gets a symlink-to-stub case (plus the same case with readlink unavailable) and a directory-named-git case (11 checks, was 8); tests/check-pending-tasks- workspace.test.sh gets the packaged-bundle deployment matrix keweichen named -- non-Git bundle + foreign Git cwd (skip) and non-Git bundle + non-Git cwd (still gates, the control) (11 checks, was 9). Mutation-tested both fixes independently: reverting each one's hunk alone fails exactly its own new check(s) and none of the others; restoring passes all 22. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): fail-open symlink bound, ambiguous-probe carve-out, CI portability keweichen (round 3, #4323): three real blockers. 1. scripts/git-binary.sh's realpath resolver stopped at a 20-hop bound without checking whether the target was STILL a symlink -- a chain longer than that (macOS permits up to 32) fell through as "resolved" to a mid-chain link that trivially wasn't the literal stub path, so a sufficiently long symlink chain to the real stub bypassed detection. Raised the bound to 40 and added the missing check: if the loop exits while `_target` is still a symlink, that's an unresolved chain, not a resolved one -- return failure (fails toward refusing, same direction as an unresolvable single-hop symlink already does). 2. src/check-pending-tasks.sh's round-2 fix (drop the "both sides non-empty" requirement) conflated two things that produce the same empty REPO_COMMON_DIR: a packaged bundle with no .git AT ALL (intentional), and a real checkout whose git probe on REPO_DIR merely FAILED (unresolved/ambiguous). Only the first should skip the guest gate. Now checks for the `.git` marker itself before deciding which branch applies: marker present -> only skip when REPO_COMMON_DIR is ALSO resolved and differs; marker absent -> any CWD Git identity is foreign. An ambiguous probe on a real checkout now fails closed (gates) instead of being read as license to skip. 3. tests/git-binary-sh.test.sh's new directory-named-git case omitted the `OSTYPE=darwin25` pin every other Darwin-specific case in this file uses -- on non-Darwin CI it silently took the non-Darwin branch (a bare `command -v git` with zero directory-rejection logic), so it could pass without exercising the code it names. Pinned, and added the missing positive control: a symlink to a REAL non-system git must still be accepted, so a fix that over-corrects into "reject every symlink" cannot pass unnoticed. Verified: finding #1's bug reproduced directly against the internal _sutando_git_realpath/_sutando_git_is_system_stub functions (a 45-hop chain), bypassing this host's own (lower-than-32) native ELOOP limit, which would otherwise mask the defect through resolve_git's outer `-f` gate. Finding #2 reproduced via a real checkout with a present-but- unparseable `.git`. New tests/git-binary-sh.test.sh cases 7c, 8b (14 checks, was 11); new check-pending-tasks-workspace.test.sh case 11 (12 checks, was 11). Mutation-tested findings #1 and #2 independently: reverting either fix's hunk alone fails exactly its own new check(s); restored, both suites fully green. Finding #3 is a CI-portability defect this host's own platform can't locally exercise as a failure (it's already Darwin) -- fixed by direct code-path reasoning and mirrored against every other Darwin-pinned case already in the file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): identity-first carve-out, dangling-symlink marker, test activation keweichen (round 5, #4323): round 4's three fixes confirmed correct and verified independently (bounded symlink resolution, its controls, case 8 reaching Darwin logic, present-.git + failed-probe gating). Two new blockers. 1. check-pending-tasks.sh's marker-first branch discarded a KNOWN, matching identity: a REPO_DIR that is a subdirectory of a real checkout with no .git of its own still resolves correctly (git walks up to the ancestor), but the old code checked the marker BEFORE the identity and treated any non-empty CWD as foreign regardless. Reordered: a known REPO_COMMON_DIR now wins outright, and the marker only breaks the tie when it's empty. Also: `-e` alone reads a DANGLING `.git` symlink as absent (marker-absent, "intentional bundle") when it should read as present (marker-present, ambiguous, fail closed) -- added the `-L` check. 2. The activated-no-CLT proof had two real gaps: the source-scan regex (`\bgit -C`) only covered the -C'd probe, so reverting the OTHER call to bare `git rev-parse` (no -C) passed it clean -- widened to catch any `$(git ...)`. And workspace test case 6 never actually copied scripts/git-binary.sh into its fixture, so the whole git-identity block silently no-op'd on an undefined `resolve_git` rather than being exercised at all -- added the file, a recording `git` stub proving activation, a non-Git cwd so the guest branch can't short-circuit before reaching the code under test, and a stderr assertion. Also rebased onto origin/main (one commit behind) and trimmed comments introduced across this PR's rounds to the repo's 2-line cap, moving reviewer/round history out of code and into commit messages where it stays checkable. Verified: both new findings reproduced directly before fixing (a nested subdir losing same-repo identity; the source-scan gap, confirmed by reproducing the exact mutation it should have caught and didn't). New cases 12/13 in check-pending-tasks-workspace.test.sh (16 checks, was 14); case 6 reworked to prove activation, not just output shape. All checks green; git-binary-sh.test.sh unchanged at 14/14 (source-scan fix verified separately, not via a new numbered case). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): genuine no-own-.git fixture, resolver-empty integration case, comment trim Round-6 review findings: 1. Case 12 (no-own-.git subdir) never constructed its claimed scenario -- git init ran directly in the hook's own REPO_DIR, giving it a real .git. Rebuilt with a genuine outer-repo/inner-child (no .git of its own) structure; verified the OLD marker-first code fails it, current code passes. 2. Added a resolver-empty/poison-stub integration case: a symlink-to-system- git on PATH is refused by resolve_git (xcode-select forced to fail), so GIT_BIN stays empty and the hook fails closed -- while a GIT_BIN=git bypass would find that same PATH entry usable and wrongly skip. Verified the case fails against exactly that bypass mutation. Also routed the test fixtures' own bare git setup calls through the same resolver the hook uses, so the suite isn't exposed to the failure mode it tests for. 3. Trimmed remaining comment blocks over the 2-line cap in git-binary.sh, check-pending-tasks.sh, and removed a stale reviewer/issue reference from git-binary-sh.test.sh. 18/18 + 16/16 local checks pass; review-checks.sh and shellcheck clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): readiness-check PYBIN guard, checked fixture init, exact-count witnesses Round-7 review findings: 1. Fixture setup could fail vacuously under set -u (which doesn't catch command failures). Added a checked _git_fixture_repo() helper that aborts the whole suite loudly on a failed init/commit, replacing every ad-hoc "init && commit" call site. Added explicit identity assertions (both common-dir probes nonempty, and equal where intended) to cases 11-14 so a silently-broken fixture can no longer pass for the wrong reason. 2. Case 6 and case 14's activation witnesses were loose (>=2 / >=1 calls, unchecked stderr). Tightened to exact expected counts, exact argv records, and the exact rejection message. "Never executes a candidate to decide" for the poison-stub case is structurally unwitnessable without touching the real /usr/bin/git it symlinks to; that property is proven once, generically, by git-binary-sh.test.sh's existing stub-ran witness (same code path, any candidate) -- noted in a comment at the call site. Also found and fixed a real gap while verifying this: the readiness-check loop invoked "" unconditionally, so an unavailable interpreter plus an EXISTING result file produced a stray "command not found" and silently misclassified a real, non-empty result as EMPTY (masked today by the outer no-PYBIN short-circuit, but not something a future refactor could rely on). Reproduced before fixing; guarded and pinned with a new case. 3. Trimmed the two comment blocks over the cap (case 12's version-history narration moved out entirely, case 14's header condensed) plus git-binary-sh.test.sh's file header and cases 7c/8 headers. 20/20 + 14/14 local checks pass; review-checks.sh and shellcheck clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): guard the turn-ledger PYBIN call, exact-match witnesses, stub-exec test Round-8 review findings: 1. The readiness-check PYBIN guard from round 7 was incomplete: the turn-ledger call at line ~105 still invoked "$PYBIN" unconditionally, accidentally safe only because an empty-command failure's rc (127) happens not to equal the gate's explicit-refusal rc (1) -- fragile, and stderr- hidden by its own 2>/dev/null so no black-box test can see the difference. Added an explicit guard; since old and new code are byte-identical in observable output, pinned the guard's PRESENCE structurally instead (both call sites checked via source grep) rather than claim a behavioral test that cannot exist. Fixed case 6b's own gap in the same pass: the earlier no-result task was never removed, so the truly-result-only/empty-queue scenario this case claims to test never actually ran. 2. Case 6/14's assertions were tightened from substring (case ... *pattern*) to exact whole-value equality (stderr, and the whole git-calls.log against its full expected content) -- a substring match lets extra noise or extra argv on the same line slide through. Added a genuine system-stub execution witness at the git-binary.sh level: black-box execution can't be witnessed without touching the real /usr/bin/git a symlink points at, so this is a structural scan (splitting on ; && || the way the shell would) asserting $_stub is never used as a command, only tested/printed -- verified it catches the exact "$_stub" --version mutation described in the review. Also replaced the test file's own "|| echo python3" fallback (6 sites) with a safely-resolved $TEST_PY, same fix as $TEST_GIT two rounds ago. 3. Trimmed 8 more comment blocks over the cap across both test files. 21/21 + 15/15 local checks pass; review-checks.sh and shellcheck clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): readiness-owner contract, real xtrace/override witnesses Round-9 review findings: 1. The readiness-check guard from round 7 contradicted the readiness owner's own contract: src/delivery/readiness.py treats existence as NOT readiness (empty/unreadable/partial -> None), but the guard skip-continued on ANY existing file when PYBIN was unavailable -- silently reading a genuinely empty, undelivered result as done. Reproduced: PYBIN empty + a 0-byte result -> {} with no warning, exactly the false-negative readiness.py exists to prevent. Fixed by routing readiness-unknown into UNPROCESSED (preserving unprocessed status) so the existing no-interpreter warning/ fail-open path handles it honestly instead. 2. Both source-text scans from rounds 7-8 were behavioral surrogates (REVIEW.md #14) that pass on a dead comment or miss execution through a different variable ($_cand instead of $_stub). Replaced the PYBIN scan with a genuine behavioral witness: run the hook under `bash -x` and grep the trace for `+ '' ...` (bash's own rendering of an empty command about to execute) -- verified it catches a dead-comment false-pass the old regex missed. Replaced the stub-execution scan with the seam the review named: override `_sutando_git_is_system_stub` to classify a REAL RECORDING script as the stub (decoupling "is-stub" from "resolves to /usr/bin/git", which is what made a genuine witness possible at all), force xcode-select to fail, and assert the recording script's log stays empty. Verified against both named mutations (`"$_stub" --version` and execution through $_cand). 3. Trimmed the remaining cited comment blocks (one production, six test) to the 2-line cap. 22/22 + 15/15 local checks pass; review-checks.sh and shellcheck clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): split PYBIN witness by call site, fix nested-trace-depth blind spot Round-10 review finding: the round-9 xtrace witness was false-green for the turn-ledger (second) PYBIN site, in two independent ways. 1. Its fixture (a task with a result present) exits at the readiness-check's own UNPROCESSED/no-interpreter branch and never reaches the turn-ledger call at all -- confirmed by tracing: zero occurrences of turn_ledger in that fixture's trace. Deleting the turn-ledger guard entirely still left the case green. Split into two fixtures: a result-present queue isolates the readiness-check site, and only a genuinely EMPTY task queue reaches the turn-ledger site (any task, with or without a matching result, exits earlier when PYBIN is empty). 2. The regex `^\+ '' ` anchors on exactly one `+`, but bash traces a command SUBSTITUTION (both call sites use one) one level deeper -- `++ '' ...`, confirmed by direct reproduction. Widened to `^\+{1,} '' `, forced PS4 so the prefix character can't drift with the environment, and added a positive-control assertion that the regex matches a known nested-depth line before trusting either site's negative result. Verified: with the fix, both sites pass; reverting either guard makes its OWN dedicated case fail (confirmed independently for both sites this round). 24/24 + 15/15 local checks pass; review-checks.sh and shellcheck clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): argv-boundary + real xtrace-capture witnesses for #4323 round 11 keweichen's round-11 review (at head 34c8ce4) confirmed the reachability/ nested-trace fix from round 10 and found two more real gaps in the tests: 1. The recording git stub logged argv via "$*", which joins with a space -- three separate args (-C, /a, b) and one quoted arg ("/a b") serialize IDENTICALLY, so the exact-equality check proved character content, not argv boundaries. Reproduced directly: both cases produced the same log line. Fixed by recording argc + one line per argument, and added an isolated positive control proving the technique actually distinguishes the two cases (this fixture's own args never contain spaces, so the main check alone can't prove the general property). 2. The xtrace "positive control" fed a hand-built "++ '' ..." string straight to grep -- fully decoupled from the real bash -x/PS4/redirect pipeline the two site checks depend on. Reproduced directly: dropping -x from either site's real capture line left every check (including the positive control) reporting ok, on a genuinely empty trace file. Fixed two ways: the positive control now drives a known-bad script through the real PS4+bash -x+redirect mechanism at both nesting depths, and each real site now asserts a stable trace marker (TASKS_DIR=) is present before trusting an absence of the bad pattern -- both confirmed to turn the exact -x-dropped mutation into a caught failure. 26/26 local (was 24), 15/15 sibling suite unchanged, shellcheck clean (pre-existing SC2016 info-notes only, same pattern as existing lines), review-checks.sh clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): sed-slicing false-green + stub-activation witness for #4323 round 12 keweichen's round-12 review (at head 59f1afb) found two more real gaps: 1. Round-11's isolated argv positive control used sed line-range slices ('1,4p' / '5,7p') over ONE shared log file, assuming each invocation writes a fixed number of lines. Reproduced exactly as described: a regression back to "$*" recording writes one line per call (two total); the first slice silently swallows BOTH lines, the second is empty, the slices trivially differ, and the control reports ok without ever comparing the two calls to each other. Fixed by giving each invocation its own log file and doing exact whole-file equality on the complete record. 2. git-binary-sh.test.sh case 10 (the stub-is-never-executed witness) only asserted ran.log's ABSENCE. If the classifier override seam were bypassed -- resolve_git never calling it, or calling a differently-named function -- ran.log would also stay absent, for the wrong reason, and the case would pass without exercising the intended branch. Fixed by asserting the classifier override WAS invoked (its own marker), the dev-tools probe WAS invoked (its own marker), resolve_git's stdout is captured and asserted empty (not discarded), and a genuine positive control that the same recording mechanism does write when called directly. Both mutation-tested: reverting the argv fix to sed-slicing reproduces the exact false-green; renaming the classifier override function reproduces the exact bypassed-seam false-pass keweichen described, and each fix turns its mutation into a caught failure. 26/26 + 16/16 local (was 26/24... i.e. unchanged counts, content fixed; git-binary-sh grew 15->16), shellcheck clean (pre-existing SC2016 info-notes only), review-checks.sh clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): trim 7 comment-cap violations reintroduced across rounds 9-12 CLAUDE.md caps code comments at 2 physical lines with no incident/review narration. Rounds 9-12's fixes each added an explanation of the reviewer finding they addressed directly into the test-file comments, and by round 12 that had grown to 7 oversized runs (up to 17 consecutive lines) carrying review-history narration this repo's own rule puts in the commit/PR body, not the source. Trimmed all 7 to the constraint each exists to state, in at most 2 lines each; the mutation-tested behavior is unchanged -- this is comment-only. 26/26 + 16/16 unchanged, shellcheck clean, review-checks.sh clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): collapse 4 adjacent-2-line comment runs to single <=2-line blocks keweichen's round-13 review: two back-to-back 2-line explanations with no blank/code separator between them are still one contiguous 4-line run under CLAUDE.md's cap, not two compliant 2-line comments. Round-12's trim reduced line count but kept this shape in 4 places. Collapsed each entire run to a single tight <=2-line comment instead of two stacked ones. Mechanically verified: 0 comment runs over 2 lines remain in the diff (down from 4). 26/26 + 16/16 unchanged (comment-only), shellcheck clean, review-checks.sh clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): isolate the PYBIN-empty comment from the baseline block above it keweichen's round-14: their mechanical audit had only grouped diff-added comment lines; a full-source scan that intersects each physical comment run with this PR's added lines found one more -- src/check-pending-tasks.sh: the PR-added 2-line PYBIN-empty comment sat immediately after a pre-existing 6-line baseline comment with no separator, forming one 9-line run under the cap. Inserted a blank line before the new comment to isolate it, without touching the baseline block's own content. Verified with the same intersection method across all 3 touched files: 0 violations. 26/26 + 16/16 unchanged, shellcheck clean, review-checks.sh clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): stub classification uses filesystem identity, not spelling git-binary.sh's -ef fix mirrors src/git_binary.py's new same_file() check: os.path.realpath / the shell realpath twin do not case-fold on a case-insensitive volume, so a case-variant spelling of the system stub (e.g. /USR/BIN/GIT) shared the real stub's inode but compared unequal by `=`/`==` and slipped past the guard. Reproduced directly on this checkout's case-insensitive volume: /usr/bin/git, /USR/bin/git and /USR/BIN/GIT all report the same st_ino. The added shell test (case 11) fails against the old `=` comparison ("expected [], got [/USR/BIN/git]") and passes with -ef; the added Python tests (SameFileHelper + test_case_variant_alias_of_the_stub_is_still_recognized) cover the same defect at the select_git() level. Addresses keweichen's review at b281d71. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): identity-probe failure fails toward the stub, not away from it An unreadable candidate is unknown, not proven non-stub. _same_file caught either stat() OSError and returned False, so select_git handed back an unverified binary with no CLT check on a race/permission failure -- the exact modal-dialog bug this module exists to prevent. Split the two stats: the candidate failing is now treated as the stub (fails toward caution); SYSTEM_GIT itself being absent still means nothing can be it. Fixed two comment-cap violations the automated review-checks.sh gate cannot see (it scans .py only, not .sh): scripts/git-binary.sh's -ef comment and the new shell test's header, both trimmed to <=2 lines, the latter also stripped of a PR/reviewer reference per CLAUDE.md's comment rule. Added tests: SameFileHelper::test_unreadable_candidate_fails_closed_ toward_the_stub exercises the fixed policy directly; SelectGitOrdering:: test_default_wiring_recognizes_a_real_hardlinked_alias exercises the production composition (default realpath AND default same_file together, no mocks) via a real hardlinked file. Replaced 5 uses of a bare, never- created "fixture-bin/git" path string with a real temp file -- the fix makes an unstatable candidate fail toward "is the stub," so a synthetic nonexistent path silently started depending on the host's actual CLT install state instead of testing the intended logic. Addresses keweichen's round-17 review at b0b02dd. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): a reference-side stat error is unknown, not proof of absence _same_file's SYSTEM_GIT-side stat caught every OSError uniformly and returned False ("nothing can be the stub"), but only FileNotFoundError actually proves absence -- a PermissionError/EIO/sandbox denial on the reference path leaves identity unknown, and reopens the modal-dialog bug for a candidate whose own stat happens to succeed. Split that except by type: FileNotFoundError still returns False (safe); every other OSError now returns True (unknown -> fail toward "could be the stub"), matching the candidate side's existing direction. Addresses keweichen's round-18 review at 73d230c. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): the shell resolver gets genuine same/distinct/unknown states Python's twin already distinguished a candidate-side stat failure (always unknown) from a reference-side one (ENOENT = absent/safe, anything else unknown) -- the shell resolver still only had `-ef`, a single boolean that cannot represent "unknown" at all, so any stat failure on either side silently read as "distinct" and returned the candidate with no CLT check. Added `_sutando_git_stat_state()`: a real `/usr/bin/stat` call (its own inode, never in the CLT-stub hardlink group, so always safe to invoke), distinguishing ENOENT (genuinely absent) from any other failure (unknown) by its stderr text -- exit 0/1/2 for exists/absent/unknown. Rewired `_sutando_git_is_system_stub` around it: candidate-side failure of any kind fails toward "could be the stub" (matching the Python side); on the reference side, only confirmed absence clears a candidate, everything else stays cautious. Added the two controls this needed: a candidate that fails its own stat (a TOCTOU-shaped vanish since resolve_git's own -f/-x check), and a reference-side failure that is NOT absence (permission/IO), with a positive control proving genuine absence still resolves correctly. Addresses keweichen's round-19 review at 3786977. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): the final identity compare is captured values, never a fresh -ef Two real gaps. First, /usr/bin/stat -f is BSD-only syntax; on Linux (the shell suite's own OSTYPE=darwin25 cases run on real Linux CI hardware) GNU stat reads -f as filesystem mode with no format flag, so every probe failed and every candidate misclassified. Split flag selection into a pure function keyed on the REAL kernel (/usr/bin/uname -s, absolute -- several existing tests deliberately run under a PATH with no /usr/bin, where a bare `uname` does not resolve either) -- OSTYPE stays the test-only knob it always was, never consulted for the flag itself. Second, even with per-side unknown/absent handling, the classifier still finished with a fresh `[ "$_resolved" -ef "$_sb" ]` -- a stat racing between the earlier checks and that line collapses to the same "false" a genuinely distinct file produces, reopening the exact bug one line later. Replaced it: the new stat helper prints dev+ino on success, so both sides' identities are already-captured values by the time they are compared -- the compare itself touches no filesystem, so nothing can race it. Addresses keweichen's round-20 review at e0837a0. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): force C locale on the identity probe, tighten the -ef pin Non-blocking hardening from round-21 review, implemented because it's real and cheap: the ENOENT-text match in _sutando_git_stat_id depends on stat's error string being English, which a non-C locale on a real GNU host could localize -- LC_ALL=C forces it. Not observable on this host (BSD stat's error text stays English regardless of locale here, confirmed by testing under de_DE.UTF-8), so this is a defensive fix for the Linux path, not a demonstrated local bug. Also tightened the shell test's own structural -ef pin: it grepped a relative path and collapsed "no match" (rc=1, the real pass) with "grep itself failed" (rc=2) into the same "ok" branch. Now absolute-pathed and rc-distinguished, plus a new pin asserting LC_ALL=C precedes the stat call. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): anchor the LC_ALL=C pin to the command, not the comment test 15b's `grep -q 'LC_ALL=C.*stat'` matched the comment explaining the fix as readily as the fix itself -- reconstructed the exact mutation (strip LC_ALL=C from the executable _out= line, leave the comment) and confirmed the old pattern stayed green against it. Anchored the pattern to a line starting with `_out="$(LC_ALL=C /usr/bin/stat`, which only the executable assignment can match, and added the mutation as a negative control run every test pass rather than a one-off manual check: strip LC_ALL=C from a temp copy's command line, assert the anchored pattern now refuses. Addresses keweichen's round-22 review at 4827fe3. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(tests): case 15b's negative control could false-green on a broken setup An unchecked mktemp/sed left the mutation-file grep able to return rc=2 (missing file), which the plain if/else read the same as rc=1 (refusal) -- the exact rc-collapse case 15 above it already guards against. Check the lab dir and mutated file were actually built, then classify grep's rc with the same 0/1/* case case 15 uses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): a failed repo-side git probe must fail closed, not guest The guest-worktree carveout treated an empty REPO_COMMON_DIR the same whether it meant "genuinely not a git checkout" or "the -C probe (or its canonicalization) failed for an unrelated reason" -- and in the markerless same-repo case, the second reading wrongly took the guest exit. Both enforcement gates (task queue + turn-ledger) were then skipped on a core session for a transient/path-specific failure. Two fixes: retry the repo-side probe via a plain cd'd rev-parse when the -C form comes back empty, and track the pre-canonicalization result separately so a value that resolves but then fails to cd is treated as ambiguous (fall through to the gate) rather than as no identity at all. 2 new pinned cases reproducing the exact cross-product with a real git wrapper (fails only a bare -C invocation; a genuinely distinct binary, not a symlink, so resolve_git() doesn't mistake it for the CLT stub) and a wrapper that hands back a path it then deletes. Both fail against the parent commit, pass here. Existing 27 cases + git-binary-sh (29/29) + git-binary-resolution (36/36) unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): confirmed-absent gates the guest exit, not any probe failure qingyun-wu (round 25, at 81f2432): the retry only closes the case where -C fails but a cd'd retry succeeds. If BOTH forms fail, the markerless-child layout still fell through to the guest exit even though nothing confirmed the repo was actually unrelated. Now the guest exit requires git's own stderr to say "not a git repository" on both attempts; any other failure (permission error, transient I/O) falls through to the gate as ambiguous instead. Added case 12d (mutation-tested: fails at the parent commit, passes here) covering a both-probes-fail error that never claims "not a git repository". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): the identity probes force a stable locale and ignore an inherited discovery ceiling keweichen's and qingyun-wu's round-25 findings, confirmed by direct execution. Both repo-identity probes in check-pending-tasks.sh inherited whatever the caller's environment happened to set, and either inheritance alone could turn a real, still-ours directory into a false "confirmed absent": - GIT_CEILING_DIRECTORIES stops git's ancestor walk at a caller-chosen boundary. A no-own-.git child whose real repo sits just past that boundary gets the exact same "not a git repository" text a genuinely different repo would produce -- confirmed on real git 2.50.1: setting the ceiling to the child's own parent makes the identical fixture from round 12 (a no-own-.git child of a real repo) fail both probes and silently guest-exit, losing a real pending queue. - The classification's only signal is an English substring match (line 52/now line 57). Git localizes its diagnostics; this repo's git build has no NLS support to prove it live, so the reviewers used a translating wrapper (this fix's tests do too) -- but the code's own logic is locale-fragile regardless of what any one build can currently produce: a genuinely non-Git bundle in a foreign Git cwd, under any locale that DOES translate the message, is no longer recognized as absent and the core's queue wrongly gates an unrelated session. Fixed by forcing `env -u GIT_CEILING_DIRECTORIES LC_ALL=C LANGUAGE=C` on both probes (and the `-C`-vs-`cd` fallback retry) -- the probes' whole purpose is a full, unambiguous identity answer, so neither a caller's ceiling nor its locale should ever be allowed to narrow that answer. 2 new cases (12e: real GIT_CEILING_DIRECTORIES against round 12's own fixture; 12f: case 9's non-Git-bundle-plus-foreign-cwd fixture behind a translating git wrapper), mutation-tested against the parent (e9fe2e1) by swapping the hook file in place and restoring it via `cp` -- exactly these 2 fail there, nothing else. 29/29 (git-binary-sh) + 36/36 (git-binary-resolution) + all cases in check-pending-tasks-workspace unchanged and green. Still outstanding, named not attempted: the live post-restart witness (a foreign-repo session completing while a same-repo session with the same pending queue blocks) both reviewers have asked for across several rounds now -- restarting the core is owner-gated, not mine to run. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): clear inherited GIT_DIR/GIT_COMMON_DIR from the identity probes keweichen and qingyun-wu both found the same real bug: the guest carve-out in src/check-pending-tasks.sh only unset an inherited GIT_CEILING_DIRECTORIES before probing, but left GIT_DIR and GIT_COMMON_DIR intact. A foreign repository whose caller inherits either variable pointed at this repo's own git-common-dir makes the CWD probe report OUR identity instead of its own, so the guest is misclassified as core and its unrelated Stop turn gets held by this repo's task queue. Reproduced directly on real git 2.50.1: $ cd /tmp/foreign-repo && GIT_DIR=<sutando-common-dir> \ env -u GIT_CEILING_DIRECTORIES LC_ALL=C git rev-parse --git-common-dir <sutando-common-dir> # should be the foreign repo's own .git and end-to-end through the actual hook with a pending task in the queue: before the fix it returned a block decision for an unrelated foreign worktree; after, '{}'. Fixed by adding both variables to the same env -u list the ceiling override already uses. Also fixed, both named by keweichen on this round's own diff: - tests/check-pending-tasks-workspace.test.sh's locale case (12f) merged stderr into the captured output and asserted exact equality with '{}' -- a real hosted Bash emitting its own "cannot change locale" warning to stderr (glibc-specific; reproduced here via a wrapped bash that injects the exact reported line) made a correct production result fail the test. Split stdout/stderr; assert '{}' on stdout only. - 3 comment blocks over the repo's 2-line cap (this file's prose-cap check only covers .py per REVIEW.md's checks.prose_exts, so .sh slipped through uncaught) trimmed to 2 lines each; the stale "line 52" reference in one of them (already wrong -- the real match had drifted to line 58) removed rather than updated, since a line-number citation goes stale on the next edit regardless. 2 new regression cases (12g/12h), mutation-tested against the parent (3f30c3f): swapped in the parent's version of the one changed line (via git show, never git checkout), confirmed exactly those 2 fail against it, restored byte-identical. Full check-pending-tasks-workspace suite (34 cases) + git-binary-sh (29/29) + git-binary-resolution (36/36) green. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): claimed_by_a_worker refuses when task_dispatch.py itself is missing A minimal packaged bundle (the deployment-matrix test fixtures ship only check-pending-tasks.sh + git-binary.sh, no src/delivery/) has no worker-pool capability at all. Python's exit code for "script file not found" is 2, the same code claimed_by_a_worker() already treats as "deliveries root unreadable -> hold, don't report to the core". The two causes collapsed onto one branch, so a bundle-mode task got silently treated as claimed-by-a-worker instead of gating -- reproduced live via bash -x, root-caused to this exact conflation. Fix: check the script file exists before invoking it. Absent -> no worker-pool capability -> not claimed (return 1), never "hold". Present-but-erroring -> unchanged (rc 2 still holds, per its own contract). Found while rebasing #4323 onto a moved main; confirmed pre-existing (byte- identical region on the pre-rebase head) and now confirmed CI-visible on this exact push (typecheck + node & shell tests). All 9 previously-failing cases in tests/check-pending-tasks-workspace.test.sh now pass; full suite green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): the guest exit no longer bypasses an enrolled worker's own gate The guest-cwd carve-out (a foreign repo's Stop must not gate on Sutando's queue) fired unconditionally, before the worker-mode branch further down ever ran. A worker-pool worker legitimately runs with a foreign cwd (spawn_worker.py, start-cli.sh both support a --cwd outside this repo), so an enrolled worker (SUTANDO_INSTANCE_ID set) could exit as a guest and skip its own delivery/turn gate, ending a turn with delegated work unanswered while the core correctly assumed the worker had it. Both guest-exit branches now require SUTANDO_INSTANCE_ID to be unset. An ordinary guest (no instance id) is unaffected. Also trims three comment blocks that exceeded the 2-line cap (review-checks.sh's prose-cap only covers .py, so it missed them in this .sh file). Adds a foreign-repo enrolled-worker regression (pending + ready-result cases) alongside the existing ordinary-guest control, so the distinction this fix depends on stays pinned. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(hooks): trim two comment runs past the 2-line cap CLAUDE.md's comment rule (at most 2 physical lines) missed these two in the new worker-carve-out test cases; also drops the PR-number reference, which belongs in the commit/PR history, not the code. * fix(hooks): a marked core session in a foreign cwd is the core, not a guest Both guest exits exempted only an enrolled worker, so the launcher's own core session, which carries SUTANDO_CORE_SESSION=1 and may run with a foreign cwd, took the guest exit and skipped the queue and turn-ledger gates. An identified session is now either a worker or the marked core, and both exits key on that; the foreign-repo and bundle suites pin the marked-core case beside the guest and worker controls. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

…ish last Round 6 of #4399 (Susan's and Kewei's CHANGES_REQUESTED at de30fbe). Blocker: the move re-checked the generation and then unlinked the live NAME, so a writer replacing tasks/<name> in that window lost its new task with errors=[]. No code unlinks a live name any more. The contract lives in core: - `task_archive.claim_generation` takes the name by one atomic rename into `<name>.claim-<pid>-<token>` in the same directory (same filesystem, never a copy); `place_claim` judges the claim through its own descriptor (`file_generation`: dev:ino:size:mtime_ns) and only a match is placed by `move_without_clobbering` (now the exported owner of `.N` naming, taking directory handles so no path is re-resolved). A mismatch links the claim back to its live name without replacing; a name a writer already retook leaves the claimed bytes quarantined under the claim name, recorded and reported. `move_generation` composes the two. `_move_without_clobbering` keeps its callers' behaviour: they move files the bridge itself already processed, for which no planned generation exists. - `result_publish.publish_text(..., exclusive=True)` commits by link and refuses an existing name; `task_transition` no longer carries its own link + fsync. Both vendored copies synced (drift guard green). - `task_transition.transition()` is a staged, journaled transaction with `tasks/` and `tasks/archive/` opened once as O_DIRECTORY|O_NOFOLLOW handles that stay open through the pass and its recovery; identity is re-checked before each claim, so a directory swapped for a symlink after pre-flight is refused, never followed. The journal (in the archive dir, beside a flock'd lock the owner holds while alive) records `planned`, each claim, each landing or quarantine, then `publishing` with the reserved notice name; it is removed when the notice commits. The notice is rendered from the FINAL landed names (`task-a.txt.1` after a collision) and published last. Recovery continues a dead owner's journal from its stage: an unplaced claim is placed, a placed-but-unrecorded one is found in the archive by its generation, a reserved notice is published unless it already exists, then the journal goes. Each generation ends with one copy, live or archived; one notice iff publication committed; a second recovery pass finds nothing. - classify.py: `recovery_plan()` keeps the planned body (within BODY_MARGIN_B of the ceiling) and lists `pending_notices`; `apply_plan()` renders the notice from `landed` and reports `quarantined`/`recovered`. The bullet strips a `.N` tail when comparing file and id; the requeue line tells the owner to drop the tail. Nits: error lines say where a file is (back live / quarantined as claim / archived by another pass); the ceiling pin is labelled a REVIEW.md #14 data pin; the bare-name docstring no longer mentions os.replace. Tests: task-transition 15 -> 31 (replaced between the generation check and the claim, name retaken, writer re-creating the name mid-move, archive dir swapped after pre-flight, `.N` named in the notice, crash after each stage recovered once, live pass's journal never taken, handles held across recovery); task-archive 39 -> 48 (the contract on task_archive itself, handles only, cross-device through handles); classify 132 -> 134; result-publish 22 -> 26. Mutation-checked: fstat verify removed (both suites), give-back by replace, directory re-check removed, lock ignored, publish-before-moves, reserved name not reused — each fails its test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

README: expand troubleshooting with common errors and restart.sh

4ccac34

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

sonichi merged commit f1b8b66 into main Mar 28, 2026

sonichi deleted the improve-troubleshooting branch March 28, 2026 18:09

liususan091219 mentioned this pull request Mar 31, 2026

Fix remaining CodeQL security alerts (round 2) #69

Merged

1 of 2 tasks

sonichi mentioned this pull request Apr 25, 2026

feat(mode-indicator): full 3-mode UI recovered from Apr 21 jsonl #519

Closed

2 of 5 tasks

sonichi added a commit that referenced this pull request Apr 25, 2026

Revert "chore(deps): bump bodhi-realtime-agent to ee58489 (PR #14 mer…

48c1fb5

…ged)" This reverts commit f9a4be4.

sonichi mentioned this pull request May 21, 2026

chore(skills): remove public info-radar stub — consolidated into sutando-skills #995

Merged

liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026

README: expand troubleshooting with common errors and restart.sh (son…

e2cd80a

…ichi#14) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

qingyun-wu mentioned this pull request Aug 21, 2026

fix(health-check): guard the STALE bridge auto-restart with the canonical-checkout check #3249

Open

sonichi mentioned this pull request Aug 28, 2026

feat(mention-gate): owner-controllable PER-HOST mute for outbound @-mentions #3473

Merged

Navigation Menu

Repository navigation

Expand troubleshooting in README - #14

Expand troubleshooting in README#14
sonichi merged 1 commit into
mainfrom
improve-troubleshooting

sonichi commented Mar 28, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

sonichi commented Mar 28, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant