Skip to content

README: inline demo video - #4

Merged
sonichi merged 1 commit into
mainfrom
inline-video
Mar 28, 2026
Merged

sonichi merged 1 commit into
mainfrom
inline-video

Conversation

@sonichi

@sonichi sonichi commented Mar 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • Replace YouTube thumbnail with GitHub-hosted inline video
  • Video plays directly in the README

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonichi
sonichi merged commit 0fcc80d into main Mar 28, 2026
@sonichi
sonichi deleted the inline-video branch March 28, 2026 17:07
sonichi pushed a commit that referenced this pull request Apr 11, 2026
sonichi GitHub account is currently down (404). Update all
references to point to liususan091219/sutando and
liususan091219/bodhi_realtime_agent.

This PR can be reverted once the sonichi account is back.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sonichi added a commit that referenced this pull request Apr 11, 2026
* Add voice agent observability + call diagnostics skill (#3)

* Add voice agent observability: events + tool calls + transcript

Track voice agent sessions in data/voice-metrics.jsonl with the same
format as phone agent's call-metrics.jsonl:
- Events: session start/end, tool calls/results, user/assistant speech, errors
- Tool calls: name, duration, timestamp
- Transcript: user and assistant turns

Writes on session end and on shutdown. Diagnosis skill updated to
read both phone and voice metrics for unified analysis.

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

* Voice observability: full utterances + 7s user timestamp shift

- Remove 60-char truncation on user/assistant speech in events
- Add 7s backward shift for user timestamps (measured STT pipeline lag
  for voice agent is ~7s vs 12s for phone agent via Twilio)
- Update diagnosis skill: --metrics flag selects input file, source-aware
  labels (Voice Session vs Call), separate tracker HTML per source

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

* Add call-diagnostics skill to repo

Copy the diagnosis skill from local ~/.claude/skills/ into the repo
so it's tracked. Includes --metrics flag for voice vs phone analysis,
source-aware labels, and unified tracker HTML generation.

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

* Refactor diagnose.py: 1057 → 737 lines (30% reduction)

- Extract CSS/chart JS into module constants
- Consolidate repair recommendations with _make_repair helper
- Compact issue detection (single-line dicts, _ts_short helper)
- Use list append + join for HTML generation
- Remove redundant whitespace in HTML output

No change to CLI behavior or output appearance.

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

* Fix duplicate metrics: guard writeVoiceMetrics with metricsWritten flag

onSessionEnd and shutdown() both call writeVoiceMetrics(). Flag prevents
duplicate entries. Reset on session start for reconnect support.

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

* Remove 60-char truncation from phone agent event data collection

Full utterances stored in call-metrics.jsonl — matches voice agent.
Diagnosis HTML can truncate for display if needed.

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

* Diagnosis: add -c flag for timeline context around each issue

Shows ±2 surrounding events with >>> marking the issue point.
Helps cross-reference issues with the observability timeline.

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

* Fix narration drift, diagnostics false positives, get_task_status, test cleanup

- Narration guardrail: "ONLY describe what you SEE" prevents Gemini from
  searching Google during record_screen_with_narration
- Diagnostics: exclude work tool from "failed N times" check (async delegate
  returns 0ms by design), exclude file paths from inline-keyword match
- get_task_status: use in-memory pendingTasks counter instead of scanning
  filesystem (task files get deleted after processing)
- Remove 5 incompatible test stubs (node:test instead of vitest)

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

* Restore test stubs removed in previous commit

These test files use node:test (not vitest) and need migration,
not deletion. Keeping them as placeholders.

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

* Fix CI: update bodhi dependency to liususan091219 fork

sonichi GitHub account is gone (404). CI fails on npm ci because
it can't clone github:sonichi/bodhi_realtime_agent. Updated to
github:liususan091219/bodhi_realtime_agent (same code, forked
before account went down).

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

---------

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

* Migrate repo references from sonichi to liususan091219 (#4)

sonichi GitHub account is currently down (404). Update all
references to point to liususan091219/sutando and
liususan091219/bodhi_realtime_agent.

This PR can be reverted once the sonichi account is back.

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

* startup: auto-install fswatch via Homebrew when missing

Instead of just reporting "fswatch not found", startup.sh now
auto-installs it via `brew install fswatch` if Homebrew is available.
Falls back to the original error message if brew is not installed.

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

---------

Co-authored-by: Susan Xueqing Liu <xliu127@stevens.edu>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Chi <wangchi@Chis-Mac-mini.hsd1.wa.comcast.net>
sonichi added a commit that referenced this pull request Apr 17, 2026
scripts/results-health.sh embodies rule #4 from
notes/post-mortem-dm-flood-2026-04-15.md: "When a PR changes restart
semantics, check `results/` accumulation before merging. `ls results/ |
wc -l` is ~1s of work. I skipped it." Standalone bash; warns + exit 1
on flood-risk signals (>10 .txt files, any zero-byte, any >24h old).
--quiet flag suppresses output on clean state.

scripts/test-results-health.sh — 4 tests covering clean, zero-byte,
stale, --quiet exit-code preservation. The test surfaced a real bug
in results-health.sh during development: BSD `find -mtime +1`
actually means >48h, not >24h as the script documented. Fixed inline
to `-mmin +1440` before this PR.

Both scripts are general-purpose reliability tooling, not tied to any
specific incident. Suitable for use by proactive-loop step 3 (health
check) and pre-merge on any PR that touches results/ delivery paths.

ICLR talk-day scripts (iclr-talk-monitor, prep-context-for-talk,
record-iclr-backups) deliberately moved to notes/iclr-tooling/ instead
— event-specific, not long-term repo material.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi added a commit that referenced this pull request Apr 17, 2026
scripts/results-health.sh embodies rule #4 from
notes/post-mortem-dm-flood-2026-04-15.md: "When a PR changes restart
semantics, check `results/` accumulation before merging. `ls results/ |
wc -l` is ~1s of work. I skipped it." Standalone bash; warns + exit 1
on flood-risk signals (>10 .txt files, any zero-byte, any >24h old).
--quiet flag suppresses output on clean state.

scripts/test-results-health.sh — 4 tests covering clean, zero-byte,
stale, --quiet exit-code preservation. The test surfaced a real bug
in results-health.sh during development: BSD `find -mtime +1`
actually means >48h, not >24h as the script documented. Fixed inline
to `-mmin +1440` before this PR.

Both scripts are general-purpose reliability tooling, not tied to any
specific incident. Suitable for use by proactive-loop step 3 (health
check) and pre-merge on any PR that touches results/ delivery paths.

ICLR talk-day scripts (iclr-talk-monitor, prep-context-for-talk,
record-iclr-backups) deliberately moved to notes/iclr-tooling/ instead
— event-specific, not long-term repo material.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi pushed a commit that referenced this pull request Apr 17, 2026
Wire the bodhi `onTurnLatency` hook into the VoiceSession config so each
turn emits a single `[Latency]` / `[Latency:SLOW]` line with the per-segment
breakdown (totalE2EMs + gemini/clientToBackend/backendToGemini/etc.).
Threshold for :SLOW tag is 1500ms; slow turns also drop a `slow_turn:{ms}`
event into the session's voiceEvents array (picked up by writeVoiceMetrics).

Matches the pattern already used by onError / onToolResult — no new
infrastructure, no new dependencies.

Motivation
----------
Deliverable #4 from the Mini overnight-ICLR split (ref pass 429).
Originally scoped as a separate tail script, but the script needed a log
format to parse and none existed. Wiring the hook is both smaller and
more directly useful: `grep Latency:SLOW logs/voice-agent.log` is all
that's needed for a live readout during the talk.

Example output (speculative — I couldn't end-to-end a real turn before
pushing because voice-agent restart on Mini needs a touch/kill, and that
would reset the 0-FATAL monitor window on the bodhi de64333 bump that's
been running for ~35 min):

    04:12:03.451 [Latency] turn=t_8fj3 t=842ms gemini=720ms c2b=18ms b2g=12ms g2b=9ms b2c=83ms
    04:13:11.109 [Latency:SLOW] turn=t_9ak1 t=2103ms gemini=1980ms c2b=22ms b2g=14ms g2b=11ms b2c=76ms

Out of scope
------------
- No tail script (grep is enough for ICLR)
- No threshold configurability (1500ms is hard-coded; change in follow-up
  if the owner wants a tighter bar for rehearsal)
- No phone-call observability (conversation-server.ts does NOT import this
  hook; separate PR if needed — phone path is tighter anyway)

Testing
-------
- `npx tsc --noEmit` clean
- `npm test` 71/71 passing
- Hook is additive to existing hooks; zero behavior change for any existing
  code path. If the hook ever throws the bodhi runtime catches it per their
  VoiceHook contract.
liususan091219 pushed a commit that referenced this pull request Apr 18, 2026
Per Susan's 'general edit to correct outdated descriptions' ask. Applied
Maddy's review hits that I could verify locally:

- pkill pattern: `-f "src/Sutando"` → `-x Sutando` (hit #8: safer, matches
  the actual process name regardless of launch path).
- Uninstall step 5: softened from `brew uninstall imsg wacli` to a
  generic 'remove via the package manager you used' (hit #3: wacli isn't
  necessarily brew).
- Capabilities table:
  - Task delegation: `task-bridge.ts` → `task-bridge.ts + watch-tasks.sh + tasks/`
    (hit #4: reflects the actual file-based flow).
  - Notes: 'via CLAUDE.md' → '`notes/` directory (YAML-frontmatter markdown)'
    (hit #5: notes/ is the real store).
  - Added rows for cross-node sync, info-radar, menu-bar avatar states
    (hit #6: recent features missing from the matrix).
- Services table: added 7845 (screen-capture), 3100 (conversation-server),
  4040 (ngrok admin UI) (hit #7).
- Removed stray duplicate `---` separator after the services table
  (hit #9).

Skipped:
- Hit #1 (L77 subscription line) — already removed earlier.
- Hit #2 (screencapture note) — Maddy confirmed still accurate.
- Hit #10 (swiftc missing `-framework AVFoundation`) — main.swift does NOT
  import AVFoundation (verified: import Cocoa, Carbon, UserNotifications
  only). Current compile line is correct.
- L101 permission rewrite (Maddy's 4-bullet proposal) — substantive
  rewrite, deferred to Susan's specific approval before applying.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi pushed a commit that referenced this pull request Apr 19, 2026
Bumps bodhi from de64333 → 4d1592e. New commits include:
- feat(hooks): onTurnLatency hook with totalE2EMs heuristic
- build: rebuild dist to include onTurnLatency hook + PR #4 guard

This lands the upstream GoAway-on-CLOSED FATAL guard (via PR #4's
handleGoAway check, which was already on de64333 but not built into
the shipped dist until PR #5's 2fb90e8) + the new onTurnLatency hook
(Sutando doesn't consume it yet; available for future observability).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi added a commit that referenced this pull request Apr 19, 2026
Bumps bodhi from de64333 → 4d1592e. New commits include:
- feat(hooks): onTurnLatency hook with totalE2EMs heuristic
- build: rebuild dist to include onTurnLatency hook + PR #4 guard

This lands the upstream GoAway-on-CLOSED FATAL guard (via PR #4's
handleGoAway check, which was already on de64333 but not built into
the shipped dist until PR #5's 2fb90e8) + the new onTurnLatency hook
(Sutando doesn't consume it yet; available for future observability).

Co-authored-by: Chi <wangchi@Chis-Mac-mini.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219 added a commit that referenced this pull request Apr 19, 2026
* docs(README): update 'Summon' voice trigger example to 'Summon my computer to zoom'

The short form 'Summon' is false-positive-prone in voice transcripts. Owner
uses the longer 'Summon my computer to zoom' phrase in practice. This docs
update matches what actually triggers the Zoom summon reliably.

Code-side prompts are NOT changed in this PR (per CLAUDE.md's 'prompts tuned
through testing' rule) — the underlying summonTool already matches both
phrases via the 'summon' keyword.

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

* docs(README): add two troubleshooting entries (Claude context drift + phone-server error)

Per Susan's Discord msg: two common failure modes worth surfacing in
Troubleshooting:
1. Claude hallucinates more as context fills → tell users to restart the
   Claude Code session periodically.
2. Phone call answers 'We are sorry, an error has occurred' → the
   conversation-server on port 3100 isn't running; relaunch via
   startup.sh or restart.sh.

Verified the correct server path: skills/phone-conversation/scripts/
conversation-server.ts (not a standalone start-phone-server.sh, which
doesn't exist).

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

* docs(README): apply Maddy's verified hits — general cleanup

Per Susan's 'general edit to correct outdated descriptions' ask. Applied
Maddy's review hits that I could verify locally:

- pkill pattern: `-f "src/Sutando"` → `-x Sutando` (hit #8: safer, matches
  the actual process name regardless of launch path).
- Uninstall step 5: softened from `brew uninstall imsg wacli` to a
  generic 'remove via the package manager you used' (hit #3: wacli isn't
  necessarily brew).
- Capabilities table:
  - Task delegation: `task-bridge.ts` → `task-bridge.ts + watch-tasks.sh + tasks/`
    (hit #4: reflects the actual file-based flow).
  - Notes: 'via CLAUDE.md' → '`notes/` directory (YAML-frontmatter markdown)'
    (hit #5: notes/ is the real store).
  - Added rows for cross-node sync, info-radar, menu-bar avatar states
    (hit #6: recent features missing from the matrix).
- Services table: added 7845 (screen-capture), 3100 (conversation-server),
  4040 (ngrok admin UI) (hit #7).
- Removed stray duplicate `---` separator after the services table
  (hit #9).

Skipped:
- Hit #1 (L77 subscription line) — already removed earlier.
- Hit #2 (screencapture note) — Maddy confirmed still accurate.
- Hit #10 (swiftc missing `-framework AVFoundation`) — main.swift does NOT
  import AVFoundation (verified: import Cocoa, Carbon, UserNotifications
  only). Current compile line is correct.
- L101 permission rewrite (Maddy's 4-bullet proposal) — substantive
  rewrite, deferred to Susan's specific approval before applying.

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

* docs(README): revise 'How it works' diagram — phone agent + accurate process topology

Per Susan's ask ('diagram isn't quite right', 'phone agent rarely covered')
and Maddy's accuracy verification:

Diagram changes:
- Web client → Voice agent is now WebSocket on :9900 (was implicit file bridge)
- Phone (Twilio) → Conversation server added as peer entry path
- results/ flow annotated as per-channel (speak via voice/phone vs text
  via Telegram/Discord), not one unified speak path

Process topology changes:
- "Two processes work together" → "Three processes" (voice-agent,
  conversation-server, core-agent) — verified against
  conversation-server.ts by Maddy (writes to tasks/ directly at lines
  289, 964, 1315; not hierarchically downstream of voice-agent).

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

* docs(README): surface phone-agent in prereqs + services list

Per Susan's ask ('phone agent is rarely covered in the README, it should be
mentioned when appropriate'):
- Prereqs: added optional Twilio + ngrok bullet so readers know the phone
  path exists and what they need to enable it.
- 'This starts all services' sentence: added 'phone conversation server' to
  the list so readers see it's a default-started daemon, not an opt-in.

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

* docs(README): rewrite permission section (4-bullet 'why elevated perms') per Maddy's proposal

Replaces the conservative CYA note ('you should review what it does. All
actions are logged') with Maddy's framing that tells the reader:
1. It's local (no remote control plane)
2. You control the audience (3-tier access gating + VERIFIED_CALLERS)
3. Actions are auditable (build_log + tasks/ + results/ + logs/)
4. Hooks are your brake pedal (git-rules-guard.sh Discord 👍/👎)

Susan's direction was 'lead the work and Maddy please review iteratively'.
Applying Maddy's already-drafted proposal; she reviews next.

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

* docs(README): add inline-tools path to 'How it works' diagram

Per Susan's ask — voice agent and conversation server have two
response paths (inline tools for instant in-process calls;
tasks/ for anything the core agent needs to execute), while
Telegram/Discord bridges only use tasks/.

— Lucy (Mac Studio bot)

* docs(README): swap 'mute' for real inline tool per Maddy's review

mute is a menu-bar ⌃M shortcut, not an inline tool. Replace with
get_current_time (src/inline-tools.ts:376) and broaden the prose
sentence to list only genuine inline tools.

— Lucy (Mac Studio bot)

* docs(README): add running-costs section + price-aware prereqs

Per Susan's in-#dev review (consolidated 4-edit bundle, 👍'd):
- Line 96: drop unqualified "free" on API key — usage-billed per
  Running costs
- Line 97: add ~$0.01–0.03/min Twilio cost reference
- New "## Running costs" section: Gemini Live $0.30–$1.30/hr,
  Twilio $0.0125/min (US local), free for browser/Telegram/Discord
- New ffmpeg bullet in prereqs (optional, for video/audio)

Sources: Gemini pricing docs, Twilio voice pricing docs.

— Lucy (Mac Studio bot)

* docs(README): bundle 4 follow-ups from #susan iteration

Per Susan's 4× 👍 iterative review on PR #450:
- Diagram: add `↻` on Core agent + legend explaining the cron-driven
  proactive loop (5-min fires for tasks, health, build-log work).
- Running costs table: Twilio row now bidirectional — ~$0.013–$0.018/min
  covers inbound ($0.0085) + outbound ($0.014) + Media Streams ($0.004).
- Prereq bullet: rename "Google AI Studio API key" → "Gemini API key"
  and inline the ~$0.30–$1.30/hour voice cost.
- macOS permissions: moved from inline Quick start bullets to the
  Security section with a per-permission "why it's needed" expansion;
  Quick start keeps a one-line pointer.

— Lucy (Mac Studio bot)

* docs(README): hedge Gemini price claims — free tier vs paid unclear

Susan and Chi are still sorting out the free-tier vs paid-tier
picture; pull back the specific $0.30-$1.30/hr claim since it
misrepresents the free-tier case:

- Prereq bullet: "may be free on the free tier or usage-billed
  depending on your Google Cloud account" instead of a rate
- Running costs table Gemini row: "Free tier available - paid
  usage varies"; notes keep the published paid rates as reference
  but point readers at their billing dashboard for ground truth
- Drop the personal $13/602-min datapoint from the typical-call
  example (may conflate with non-voice Gemini spend)

Twilio row unchanged - that math is from published rates.

— Lucy (Mac Studio bot)

* docs(README): Option B — remove Running costs section + price mentions

Per Susan's "B" direction in #dev: the free-tier vs paid-tier
picture for Gemini isn't settled, and leaving any price language
in the PR risks misleading first-time readers.

- Delete the entire "## Running costs" section (table + example +
  sources line).
- Revert the Gemini API key prereq bullet to `(click "Get API key")`
  — drop the usage-billed caveat entirely.
- Revert the Twilio prereq bullet to drop the `~$0.01–0.03/min`
  price reference.

ffmpeg bullet, diagram rewrite, macOS permissions in Security
section, `↻` cron legend, phone-agent surfacing — all stay.

— Lucy (Mac Studio bot)

---------

Co-authored-by: Xueqing Liu <xueqingliu@Xueqings-Mac-Studio.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219 pushed a commit that referenced this pull request Apr 29, 2026
…n meeting

Phone summon test #4 (2026-04-29 16:06): a11y Join click landed correctly,
but the meeting window took 31s to appear after the click on Studio Zoom
Workplace. Bodhi's default tool timeout is 30s, so the tool returned
error 30001ms BEFORE the share-menu fired. The share itself eventually
fired (logged 16:06:57) but by then the assistant had already told the
caller "I'm having some trouble summoning."

`60_000` covers the worst-case meeting-open + share-menu + mute AppleScripts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219 added a commit that referenced this pull request Apr 30, 2026
…sweep (#546)

* fix(open-file): move openFileTool to inline-tools, decouple from recording

`openFileTool` lived in `src/recording-tools.ts` since #274 with
recording-flavored helpers (`findRecording()` fallback, `sutando-recording`
substring check, `/tmp/sutando-playback-path` write, `demoStateRef.value
= 'idle'` reset, ffprobe duration probe) coupled into the function body.
PR #353 introduced most of those; PR #392 cleaned the worst (made
`find_recording=true` opt-in), but kept the helpers as deliberate residuals.
This commit removes them entirely and moves the function to a generic
home.

Changes:

1. **Move `openFileTool` to `src/inline-tools.ts`.** Required `path` only.
   Stripped: `find_recording`, `findRecording()` fallback,
   `sutando-recording` substring write, `demoStateRef` reset,
   `recPath` rename → `filePath`, "play_video" instruction string,
   ffprobe duration extraction.

2. **Keep `fullscreen=true` flow in `openFileTool`** (per Chi's confirmation
   in #dev — fullscreen-on-open is general video-fullscreen UX, not
   recording-specific). The QuickTime force-open + present-mode
   AppleScript + Zoom-survives keystroke routing all stay. The runtime
   guard ("don't fullscreen=true when QT already has docs") stays.

3. **Re-export hygiene:**
   - `inline-tools.ts` line-17 re-export and line-18 import: dropped
     `openFileTool` (now defined locally in this file).
   - `browser-tools.ts` recording-tools re-export block: dropped
     `openFileTool`.
   - `recording-tools.ts`: removed definition + the misleading
     "polymorphic playRecordingTool" comment block above it. One-line
     marker pointing to the new location.

4. **Recording-flavored side effects relocated to recording-tools:**
   - `/tmp/sutando-playback-path` write moved to `screenRecordTool`
     stop handler AND its auto-stop `setTimeout` callback. Recording
     tool now stamps the playback hint itself when the recording
     finishes — no model courier through `open_file` needed.
   - ffprobe duration extraction moved to `screenRecordTool` stop
     result (`duration_seconds` field).
   - Stop handler instruction updated to mention both
     `open_file(path)` and `open_file(path, fullscreen=true)` so the
     model knows how to invoke each path of the demo flow.

README demo flow (`screen_record → open_file(path) → play_video`)
continues to work end-to-end: `screen_record stop` writes
`/tmp/sutando-playback-path` proactively, and `play_video`'s existing
read of that path (`startPlayback()` line ~454) is unchanged.

Refs: #274 (initial split), #315 (rename), #353 (introduced
coupling), #392 (partial cleanup, kept residuals), #408/#516/#520/#524/#526
(fullscreen + QuickTime fixes — fullscreen=true preserved as general UX).

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

* fix(tools): deduplicate open_url/capture_screen/type_text across browser-tools/inline-tools

Three tool names had two registered definitions (one in src/browser-tools.ts
and one in src/inline-tools.ts), which both inlineTools and ownerOnlyTools
arrays then picked up. Gemini Live rejects setup payloads with duplicate
function names (close 1011 "Internal error") — see
reference_gemini_1011_tool_name_conflict.md.

Canonical placement per layer responsibility:
- open_url      → browser-tools.ts  (browser-side action)
- capture_screen → inline-tools.ts  (inline screen capture)
- type_text     → inline-tools.ts  (inline keystroke / clipboard paste)

inline-tools.ts now re-exports openUrlTool from browser-tools so consumers
keep importing from inline-tools unchanged. Removed the duplicate
definitions on the non-canonical side; left a one-line marker comment so
greps for the old name still land somewhere meaningful.

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

* fix(tools): remove dead cancel_task duplicate from task-bridge

`cancelTask` was defined in src/task-bridge.ts:171 (Chi, commit bbc2631
2026-03-28) AND `cancelTaskTool` in src/inline-tools.ts:404. The
inline-tools version is the one registered with Gemini; the
task-bridge export was imported by src/voice-agent.ts:39 but never
referenced anywhere — dead import.

Confirmed safe to delete via independent grep:
- 0 call sites in src/
- 0 test references
- only consumer was the dead import

Removes the third source of the duplicate-tool-name 1011 setup error
documented in reference_gemini_1011_tool_name_conflict.md.

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

* fix(summon): use zoommtg:// deeplink + tighten meeting-window readiness check

Two bugs caused phone summon on Studio to log "Screen share started" while
nothing actually shared:

1. **HTTPS join URL unreliable on Zoom Workplace.** The `zoomRunning=true`
   branch was using `https://zoom.us/j/{id}?pwd=...`, which routes through
   the browser and doesn't reliably hand off to the in-app meeting on Zoom
   Workplace (Studio). Result: Zoom stayed on its home dashboard, no
   meeting joined. Replaced with the `zoommtg://` deeplink for both
   running-and-not-running cases — direct to the Zoom app, no browser hop.

2. **Readiness check counted any window.** The old check waited for `(count
   of windows of process "zoom.us") > 0`, which the always-on "Zoom
   Workplace" home dashboard already satisfies. So even with no meeting
   ever opened, the check passed, the share-screen menu fired into an
   inapplicable context, the AppleScript silently no-op'd, and the catch
   block still logged success. Now checks window names for `Zoom Meeting`
   / `zoom share` / `floating video` — matches `alreadyInMeeting`'s
   detection.

Verified 2026-04-29 phone summon on Studio (`/tmp/conversation-server.log`
15:54:27–15:54:36) showed the false-positive log; instrumented a11y dump
confirmed only "Zoom Workplace" was open after the supposed join.

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

* fix(summon): click Join by a11y description; tighten host-joined check

Phone summon on Studio kept missing the actual Join button: the cliclick
coordinate (`preview.x + 0.80*w, preview.y + 0.93*h`) didn't land on the
button on Zoom Workplace's preview layout. The meeting never opened,
the host-joined check then matched "Personal Meeting Room" (a preview
window!) on `winNames.includes('Meeting')` and reported `Host joined: true`
falsely, so the phone dial-in path fired anyway, and the new strict
share-readiness gate timed out 30s later. Net: `summon` returned error
30002ms after a chain of internal false positives.

Two fixes:

1. **Join click via a11y description, not coordinates.** Iterate `buttons of
   w whose description is "Join"` across all zoom.us windows, click the
   first one. The Join button's a11y description is stable across Zoom
   Workplace layouts (verified by direct dump 2026-04-29).

2. **Host-joined check uses the strict meeting-window set.** Same
   `Zoom Meeting | zoom share | floating video` set as the share-readiness
   gate. Plain `Meeting` was matching preview windows.

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

* fix(summon): bump tool timeout to 60s — Studio Zoom takes ~30s to open meeting

Phone summon test #4 (2026-04-29 16:06): a11y Join click landed correctly,
but the meeting window took 31s to appear after the click on Studio Zoom
Workplace. Bodhi's default tool timeout is 30s, so the tool returned
error 30001ms BEFORE the share-menu fired. The share itself eventually
fired (logged 16:06:57) but by then the assistant had already told the
caller "I'm having some trouble summoning."

`60_000` covers the worst-case meeting-open + share-menu + mute AppleScripts.

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

* fix(record-with-narration): write playback-path on auto-stop so play_video can find the file

After PR #546 made open_file generic (no findRecording fallback), there
was a behavioral gap: record_screen_with_narration's auto-stop didn't
persist a playback-path hint, while screen_record stop did. Result on
2026-04-29 phone test: user said "open the recording" → model called
open_file with no path → got generic "no path provided" error → user
gave up.

This commit closes the gap WITHOUT re-coupling open_file to recording:
- record_screen_with_narration auto-stop now writes /tmp/sutando-playback-path
  with the best-available file (subtitled > narrated > raw), matching
  screen_record stop's existing pattern.
- Tool description appends "After auto-stop, to play back or open the
  recording, call play_video — it auto-finds the file. Do NOT call
  open_file unless you have the absolute path." to nudge the model
  toward the correct tool.

open_file remains general-purpose; recording-tools owns the playback-hint
write. play_video already reads /tmp/sutando-playback-path with a
findRecording fallback.

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

* fix(record-with-narration): return predicted recording paths so model can pass to open_file

The 2026-04-29 phone-summon test still failed on "open the recorded video"
even after `19ecfb8` wrote `/tmp/sutando-playback-path` on auto-stop —
because the model called `open_file` (the natural verb match for "open"),
not `play_video`, and `open_file` is generic and needs a path arg.

Now `record_screen_with_narration` captures `record.py start`'s JSON output
to extract the absolute `recording_path` and computes the
`narrated_path = path.replace('.mov', '-narrated.mov')`. Both paths
are returned in the start result + spelled out in the `message` field
("After auto-stop, the file will be at ... — pass that exact path to
open_file when the user asks to open the recording.").

The model now has the path inline from the moment recording starts and can
pass it to open_file (or any other path-taking tool) without guessing. No
recording-specific code in open_file. No reliance on the model picking up
the prior tool description hint.

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

* fix(record-with-narration): return subtitled_path so model can open the burned-in version

After 35cce47 returned `recording_path` + `narrated_path`, the model still
couldn't open the subtitled version because it didn't know that path. The
2026-04-29 21:00 phone test had subtitle burn succeed (file at
/tmp/sutando-recording-X-narrated-subtitled.mov) but the model called
open_file three times with narrated/raw paths only — Susan kept saying
"this is not the one with the subtitle".

Now the start result includes `subtitled_path` and the message string
spells out the priority order: pass subtitled_path to open_file when the
user asks for "the recording" / "with subtitles". Fall back to
narrated_path only when subtitled burn failed (e.g., libass missing,
which is now known-fixed via the homebrew-ffmpeg/ffmpeg tap).

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

* fix(recording/open-file): clarify subtitle support + nudge model away from path hallucination

Two regressions from phone summon test #7 (CA5c9c0a711cfcacc2cc5458c1d5eca04d
21:09 ET):

1. **Model hallucinated "I can't add subtitles"** — record_screen_with_narration's
   tool description didn't mention that subtitle burn happens automatically when
   the live transcript exists. User said "add subtitle", model refused, even
   though the auto-stop callback ALWAYS calls burnLiveTranscriptSubtitles when
   liveTranscriptRecordingStart > 0. Description now states subtitles ARE always
   burned in — never refuse a "with subtitles" request.

2. **Model hallucinated path `/tmp/screen_recording_with_narration.mov`** when
   asked to open the recording — invented a generic name instead of using the
   `subtitled_path` returned by start. Two-pronged nudge:
   - Recording-tools description: "Do NOT invent file paths — only use the exact
     paths returned by this tool" + clarifies subtitled_path is the right one
     for "with subtitles".
   - open_file's `File not found` error explicitly says "Do not invent paths"
     and routes to play_video for unknown-path recording-open.

`open_file` itself remains generic — only its error string text changed, no
recording-specific code. play_video already has the findRecording fallback.

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

* fix(open-file): strip ALL video-related logic — back to truly generic open

Per Susan: open_file should not contain any video-specific behavior. The
fullscreen=true flow forced QuickTime, ran a present-mode AppleScript with
30-iteration polling, and had a guard against re-opening when QT already
had documents. All video-specific.

Removed:
- `fullscreen` parameter
- QT force-open via `open -a "QuickTime Player"`
- Path-matched present-mode AppleScript (Cmd+Ctrl+F)
- QT-already-has-doc guard
- Description text about video presentation, .mp4/.mov, "fullscreen on a
  video that is already open"

Now: `execFileSync('open', [filePath])` — LaunchServices picks the user's
default app for that file type. Mirrors what a human running `open foo.x`
in a terminal would get.

For fullscreen on a just-opened video, the model should call the existing
`fullscreen` tool as a separate step. Two-tool sequence is simpler than
the polled AppleScript embedded in open_file.

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

* fix: address Mini PR #546 review — summon retry/return, generic fullscreen, soften subtitle claim

Mini's review on PR #546:

**1. summon Join click + return values (Medium)** — the prior split (one-shot
Join + 30s readiness) let a late preview-window race leave the meeting
un-joined while readiness ran out the clock; the tool then still returned
"summoned" / "joined with screen sharing". Now: combined 60s loop where
each iteration first checks for a real meeting window (Zoom Meeting / zoom
share / floating video), then if not present retries the a11y Join click.
Track `zoomReady` and `shareStarted` accurately and surface them in the
return payload + status string (`summoned` | `joined_share_failed` |
`meeting_window_not_detected`). instruction text reflects what actually
happened.

**2. summon timeout (Medium)** — 60s was still tight per Mini's worst-case
calc (Join 10s + readiness 60s + share 15s + mute/cleanup 10s + dial-in
host wait 20s ≈ 115s). Bumped to 120s.

**3. record_screen_with_narration subtitle claim (Low)** — softened "IS
automatically burned" to "are attempted automatically when both transcript
text and a libass-capable ffmpeg are present". Burn can silently skip on
missing transcript / unreadable narrated / missing libass; honesty matters
more than over-promising. Description still says "never refuse a 'with
subtitles' request" so the model doesn't gate refusal on apparent failure.

**Bonus (Susan):** open_file gets a *generic* `fullscreen` parameter
back — but app-agnostic this time. Just `open <path>` (LaunchServices
routes to default app) → 1.5s settle → System Events Cmd+Ctrl+F. Works for
video in QuickTime, PDF in Preview, web pages in Chrome — no QT
hardcoding, no path-match polling. Replaces the QT-specific present-mode
flow with a one-keystroke generic toggle.

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

* fix(summon): address Mini PR #546 round-2 — dialIn order + 1-line instruction fix

Medium #1 (code change): dialIn previously ran a separate host-joined wait
BEFORE the readiness/Join-retry loop. Preview-still-up race made dialIn
fire against an unjoined meeting. Restructured: readiness/Join-retry loop
runs FIRST and gates the dialIn block on zoomReady. Removed redundant
standalone host-joined wait.

Low #2 (text only): instruction in phoneJoined branch falsely claimed
"Screen is shared..." when shareStarted=false. One-line fix:
`(shareStarted ? 'Screen is shared and ' : '')` prefix.

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

---------

Co-authored-by: Xueqing Liu <xueqingliu@Xueqings-Mac-Studio.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi added a commit that referenced this pull request Apr 30, 2026
)

Migration #4 + #5 in the private-repo plan. Notes/ are SHARED across the
fleet (top-level private dir); pending-questions.md, PERSONAL_CLAUDE.md,
tab-aliases.json are PER-MACHINE (machine-<host>/, like #548).

Two resolver families:

- `personal_path()` (existing, #548) — per-machine: $SUTANDO_PRIVATE_DIR/machine-<host>/<file>
- `shared_personal_path()` (new) — fleet-shared: $SUTANDO_PRIVATE_DIR/<file>

New `src/util_paths.ts` is the TS twin of util_paths.py with both helpers
exported. voice-agent.ts replaces its inline `personalPath` with imports
from the new module.

Wired callers:

NOTES (shared_personal_path):
- src/inline-tools.ts NOTES_DIR → save_note / read_note / list_notes / delete_note
- src/voice-agent.ts: meeting-note save (line 285)
- src/dashboard.py: /notes/{slug} route + /notes index (lines 48, 437)
- src/daily-insight.py: NOTES_DIR
- src/friction-detector.py: notes_dir
- src/discord-bridge.py: SEND_ALLOWED_ROOTS adds private notes path
- src/health-check.py: notes-dir check now points at canonical home

PER-MACHINE (personal_path):
- src/dashboard.py: get_pending_count
- src/agent-api.py: pq_file (2 sites)
- src/friction-detector.py: pending-questions.md
- src/check-pending-questions.py: PQ_FILE
- src/session-handoff.sh: pending-questions.md (via inline python3)

PERSONAL_CLAUDE.md and tab-aliases.json: no runtime callers — only
referenced in migrate.sh bundle/restore (#548 already covers per-machine
shape, so they ride along without extra wiring).

Test plan:
- [x] `npx tsc --noEmit` clean
- [x] All Python files compile (`ast.parse`)
- [x] Resolver verified end-to-end with SUTANDO_PRIVATE_DIR set:
      shared_personal_path('notes') → /Users/wangchi/.sutando-memory-sync/notes
      personal_path('pending-questions.md') → .../machine-Chis-MacBook-Air/pending-questions.md
- [ ] Drop a voice-tool note via save_note, verify it lands in private dir
- [ ] Open the note in dashboard, verify /notes/{slug} reads from private dir
- [ ] Restart check-pending-questions cron, verify it reads private PQ file

After merge: kickstart voice-agent (touches voice-agent.ts + inline-tools.ts),
restart web-client/dashboard/agent-api/discord-bridge (Python files), restart
conversation-server. Same pattern as #548.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219 added a commit that referenced this pull request May 3, 2026
…, timeout label

Builds on the silent-timeout + nudge work. Susan's follow-ups today:

1. The yellow ⏱ icon was visible but no "TIMEOUT" word — easy to miss.
   web-client.ts: render a small uppercase "TIMEOUT" pill next to the
   icon when status === 'timeout', styled as `.task-status-label.timeout`
   (yellow on dark-yellow, parallel to the existing badge color).

2. "Just one nudge then nothing" was too quiet for a genuinely-stuck
   core. New behavior: nudges fire at 1×, 2×, 3× NUDGE_AT_MS (= 3×, 6×,
   9× TASK_TIMEOUT_MS). Each carries a "[Sutando-status-check #N]"
   prefix and the original task text. After the 3rd nudge fires, the
   task is escalated via Discord DM to the owner — task-bridge writes
   `results/proactive-stuck-{ts}.txt`, which the existing Discord
   bridge picks up and DMs the owner. Per-task dedupe means escalation
   fires at most once per task per result-cycle (cleared when the
   real result arrives).

State change: `_nudgeFired: Set<string>` → `_nudgeCount: Map<string,
number>` to support periodic firing. New module-level `_escalated`
guards the once-per-task DM. Both reset in `_processResult`.

Test "nudges fire periodically up to MAX_NUDGES, then escalate via
Discord DM" walks the full ladder: timeout-only at age < NUDGE_AT_MS,
nudge #1 at 1×, nudge #2 at 2×, nudge #3 + escalation at 3×, then
asserts capping (no nudge #4) and escalation dedupe (no second DM)
at 4×. Still silent throughout — `onResultCalls.length === 0`.

All 6 timeout tests pass; full suite still green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
qingyun-wu added a commit that referenced this pull request May 18, 2026
…iew #4 + 2 leaks) (#801)

Post-LGTM cleanup after Chi's re-review on PR #794:

- **#4 — renderGoal reorder** (tools.ts): goal-required guard now runs
  BEFORE renderGoal, so we never produce a string with an un-substituted
  `{goal}` placeholder. Today's effect is harmless (the unfilled string
  was unused), but the wasted call is the kind of bug that lands a
  prompt-injected `{goal}` in a future code path.

- **Leak 1 — tools.ts error hint** mentioned `vision_query / take_note /
  look_up_reference` as fallback behavior. Those tools don't exist yet
  (tracked in issue #797). Replaced with a neutral "operate normally
  with whatever tools the session already has registered."

- **Leak 2 — SKILL.md Privacy section** said transcripts get persisted
  "via the `take_note` tool" — also a forward reference to an unbuilt
  tool. Updated to acknowledge the tool is planned (#797) and that
  today "remember this" requests are persisted as part of the
  conversation transcript.

Chi's verdict was LGTM with these as cleanup-not-blockers — landing them
in the same PR before merge anyway.
qingyun-wu added a commit that referenced this pull request May 18, 2026
Two small clarifications from sonichi's #858 review:

1. **"Memory falls back to State" is a feature, not a gap.** Previously
   framed as impl gap #3 in the open-decisions section, which reads
   like a TODO. Reframe inline at the Memory section: the fallback is
   intentional onboarding-friction reduction; fresh installs work
   without `SUTANDO_PRIVATE_DIR`. The lifecycle distinction is
   documented but not enforced. Stops a future contributor from
   "fixing" this by removing the fallback.

2. **IDE / editor config angle.** Added a short section before
   "Relationship to other docs" covering `.cursorrules`, `.claude/`,
   `.vscode/`. The 3-space model doesn't have a clean home for these
   — answer is the existing IDE-aware convention (`.gitignore`
   per-user variants, check in shared ones). Flagging because new
   contributors ask "should .cursorrules go in Memory?" and the
   answer (no, stays in Code-or-untracked per IDE convention) isn't
   obvious from the 3-space framing alone.

Two other nits from the review are followup PRs, not addressed here:

- (#2) Extract `mkdtempSync` test-isolation pattern to
  `tests/_helpers/temp-workspace.ts`. Worth a small follow-up — Mini
  offered to take it.
- (#3) File the 7-item implementation-gaps list as separate issues
  so they don't bit-rot in the doc. Will do after owner sign-off on
  the alignment direction in #857.

Mini's votes on the 6 alignment decisions in #857 are mostly aligned
with the proposal; the deltas (per-read deprecation logging,
`SUTANDO_HOST_LABEL` override, opt-in state-sync allowlist, secrets
stay at .env) are good refinements worth incorporating into the doc
once owner confirms direction.
qingyun-wu pushed a commit that referenced this pull request Sep 11, 2026
…held

`src/pool_delivery.py` deliberately recognises the pre-rename `.claimed`
acceptance during the pool rollout; `src/check-pending-tasks.sh` spelled its
own suffix set and knew only `.txt` and `.accepted`. A task a worker had
accepted under the legacy name therefore read as UNPROCESSED, and the hook
told the core to answer work already in flight.

Two spellings of one grammar is the defect, so the fix is one owner rather
than a third copy: `pool_delivery.held_by_worker()` plus a `--held <id>` CLI
(exit 0/1, prints the holder), which the hook now calls.

    parent 6c6834d:  FAIL a worker's legacy .claimed exempts
                            wanted allow, got block
                       FAIL pool_delivery --held reports the legacy hold
                            exit was non-zero
    this commit:       ok   a worker's legacy .claimed exempts
                       ok   pool_delivery --held reports the legacy hold

`tests/check-pending-tasks-worker-held.test.sh` pins core/worker across
pending/.accepted/.claimed through the real hook. The two controls carry as
much weight as the case: an undelivered task must still block, and the CORE's
own inbox must never be a hold — the core declining is what puts a task in
front of this hook. `TestHeldByWorker` pins the same grammar in-process, where
the coverage gate can see it (a shell test drives a subprocess and scores zero).

Without a resolver-supplied interpreter the hook cannot ask, and cannot emit
a block either (it already degrades to `{}`), so the exemption is unreachable
rather than wrong.

Reported by @keweichen (P1 #4).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
qingyun-wu pushed a commit that referenced this pull request Sep 11, 2026
…held

`src/pool_delivery.py` deliberately recognises the pre-rename `.claimed`
acceptance during the pool rollout; `src/check-pending-tasks.sh` spelled its
own suffix set and knew only `.txt` and `.accepted`. A task a worker had
accepted under the legacy name therefore read as UNPROCESSED, and the hook
told the core to answer work already in flight.

Two spellings of one grammar is the defect, so the fix is one owner rather
than a third copy: `pool_delivery.held_by_worker()` plus a `--held <id>` CLI
(exit 0/1, prints the holder), which the hook now calls.

    parent 6c6834d:  FAIL a worker's legacy .claimed exempts
                            wanted allow, got block
                       FAIL pool_delivery --held reports the legacy hold
                            exit was non-zero
    this commit:       ok   a worker's legacy .claimed exempts
                       ok   pool_delivery --held reports the legacy hold

`tests/check-pending-tasks-worker-held.test.sh` pins core/worker across
pending/.accepted/.claimed through the real hook. The two controls carry as
much weight as the case: an undelivered task must still block, and the CORE's
own inbox must never be a hold — the core declining is what puts a task in
front of this hook. `TestHeldByWorker` pins the same grammar in-process, where
the coverage gate can see it (a shell test drives a subprocess and scores zero).

Without a resolver-supplied interpreter the hook cannot ask, and cannot emit
a block either (it already degrades to `{}`), so the exemption is unreachable
rather than wrong.

Reported by @keweichen (P1 #4).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
qingyun-wu pushed a commit that referenced this pull request Sep 11, 2026
…held

`src/pool_delivery.py` deliberately recognises the pre-rename `.claimed`
acceptance during the pool rollout; `src/check-pending-tasks.sh` spelled its
own suffix set and knew only `.txt` and `.accepted`. A task a worker had
accepted under the legacy name therefore read as UNPROCESSED, and the hook
told the core to answer work already in flight.

Two spellings of one grammar is the defect, so the fix is one owner rather
than a third copy: `pool_delivery.held_by_worker()` plus a `--held <id>` CLI
(exit 0/1, prints the holder), which the hook now calls.

    parent 6c6834d:  FAIL a worker's legacy .claimed exempts
                            wanted allow, got block
                       FAIL pool_delivery --held reports the legacy hold
                            exit was non-zero
    this commit:       ok   a worker's legacy .claimed exempts
                       ok   pool_delivery --held reports the legacy hold

`tests/check-pending-tasks-worker-held.test.sh` pins core/worker across
pending/.accepted/.claimed through the real hook. The two controls carry as
much weight as the case: an undelivered task must still block, and the CORE's
own inbox must never be a hold — the core declining is what puts a task in
front of this hook. `TestHeldByWorker` pins the same grammar in-process, where
the coverage gate can see it (a shell test drives a subprocess and scores zero).

Without a resolver-supplied interpreter the hook cannot ask, and cannot emit
a block either (it already degrades to `{}`), so the exemption is unreachable
rather than wrong.

Reported by @keweichen (P1 #4).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
sonichi added a commit that referenced this pull request Sep 18, 2026
…r-submitted must not wait, periodic self-re-arm

Four real blockers from keweichen's third review round (head 1ee49d5):

1. Claim-aware selection still bypassed the handler in the pre-claim
   window: next_pending_task only excluded a task already claimed in
   CLAIMS_DIR, which can't see a required handler still probing it.
   Ported Codex's already-shipped probe_optional_task_handler pattern:
   next_pending_task now iterates task_dispatch's pending-candidates and
   re-probes each fresh, skipping any whose handler answers 4 (required,
   not yet claimed) and any already holding a fallback receipt.

2. The first Enter wasn't bound to the exact composer content: staging
   was a substring match (grep the marker), so owner text landing in
   the gap between send-keys and the staged-tail capture would sit
   alongside our marker and still satisfy it. composer_text() (new,
   src/core-input-watch.py, shared with the empty-composer check) now
   reconstructs the composer's full dewrapped content -- stripping any
   footer/gate/border line wherever it renders -- and staging requires
   EXACT equality against the prompt we sent.

3. An unsent delivery could wait out COMPLETION_TIMEOUT (up to an hour)
   for a result that could never appear, blocking the single-threaded
   event loop from ever getting to another queued task: deliver_prompt's
   two "Enter never sent" branches returned 0 (proceed to the completion
   wait) instead of 1 (leave queued, matching the existing composer-busy
   branch). Both now return 1.

4. A task left queued by any of the above had no other trigger if no
   unrelated task file ever arrived -- it would wait forever. The main
   loop now self-re-arms via a read timeout (default 30s) and retries
   the highest-pending task on timeout, not only on a real wake.

Comment-cap: 5 pre-existing + newly-touched blocks trimmed to the 2-line cap.

Verified:
- Real pre-fix-fails/post-fix-passes for all 4, each isolated:
  - #1: direct against next_pending_task (bypassing the real watcher,
    which settles a required task from its own startup sweep before any
    event reaches this notifier, masking the race at that layer).
  - #2: caught a genuine reconstruction bug of my own first attempt too
    -- a pane-border row sitting between the composer and the footer
    leaked into the result; a direct Python unit test (not just the bash
    integration path, which never exercised that fixture shape) caught
    it before push.
  - #3: measures wall-clock elapsed, asserts it stays far under
    COMPLETION_TIMEOUT.
  - #4: real fswatch-driven main loop with a busy composer and NO second
    task file ever written; caught a real set-e-adjacent bug in my own
    first attempt (an if/then/fi wrapper around read erases read's real
    exit code, since the if-statement's own status is 0 whenever the
    then-branch didn't run) and a second real bug: macOS's own /bin/bash
    3.2 returns 1 for a read timeout same as EOF, unlike a modern bash's
    128-plus-signal convention, which would have silently killed the
    notifier on its very first quiet period in production.
- 23/23 claude-task-notifier (was 19), 85/85 core-input-watch (was 78,
  +7 direct unit tests for the new composer_text()), 40/40
  core-input-watch-chat-escalation, adjacent 55/55 task-dispatch-contract
  and core-input-watch-gateway-probe/prompt-excerpt clean.
- bash -n, shellcheck -S warning, ruff, git diff --check, review-checks.sh
  (against origin/main), gen-src-map --check all clean.

Stand: Echo Act IV Pro
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sonichi added a commit that referenced this pull request Sep 18, 2026
…ode (#4307)

* feat(claude-core): add external task-file-injection notifier for Claude Code

Standby path for task delivery alongside the live core's self-armed
Monitor watch: matches Codex/agy's proven shape (external
watch-tasks-stream.sh + tmux send-keys injection), free-standing and
not wired into start-cli.sh's launch path.

Idle/busy gating trusts core-status.json first (Claude's own "Work
Status" self-report), falling back to the pane only when that record
is stale — same combined contract as Codex's core_is_idle(). The pane
check itself delegates gate/idle-footer classification to the real
src/core-input-watch.py (_is_idle_ready) rather than re-deriving
Claude's trust/login/selection/permission pattern list a third time —
that module already owns it for the M0-M4 core supervisor, and
CLAUDE.md's "Shared adapter policy" rule treats a fresh copy as a
defect when a real shared owner exists. has_result()/next_pending_task()
stay duplicated from Codex's script (no shared module exists yet to
delegate to — a named follow-up, not this PR's scope, per agy's own
task-notifier.sh header).

Investigated Claude's pane idle/busy signal by hand against a real,
throwaway Claude Code v2.1.261 session in tmux (not any live core):
the footer is a clear, stable "esc to interrupt" toggle, unambiguous —
so this was built rather than stopped on.

Proven end-to-end against that same real session, not just the
hermetic suite: dropped a task file, watched the real notifier type
and submit it, and confirmed the correct result landed on disk. That
run caught one real bug the hermetic tests alone would have missed —
the submit-confirm check read the submitted prompt's own scrollback
echo as "still staged" (3 spurious C-m re-presses), the exact failure
class agy's PR already documented for its own pane. Fixed to confirm
on the pane going busy instead, re-verified live (0 retries), and
pinned with a dedicated hermetic regression test.

Every fix is proven by disabling it and watching its test fail: the
has_result skip, idle-gating (status file + pane), staging-retry,
submit-confirm re-press, the busy-vs-vanishing-text confirm fix, the
core-input-watch.py delegation, and the fswatch main-loop wiring.

Stand: Echo Act IV Mini

* docs(src-map): add the new Claude task-notifier entry

python3 scripts/gen-src-map.py, regenerated after adding
src/agent/claude/cli/task-notifier.sh — the CI staleness gate caught it.

Stand: Echo Act IV Mini

* refactor(claude-core): consume the shared task_dispatch policy instead of a hand-rolled duplicate

Rebased onto main (now carrying #4317's src/delivery/task_dispatch.py, the
one-owner completion/pick policy for every external task-notifier). This
PR's own has_result()/next_pending_task() were written before that module
existed and duplicated its exact two bugs: `[ -f results/<f> ]` read an
empty placeholder as delivered regardless of content, and the epoch-archive
find pattern could collide with an unrelated task sharing a name prefix.

Both functions now delegate:
  has_result()        -> task_dispatch.py has-result <results_dir> <filename>
  next_pending_task()  -> task_dispatch.py next-pending <tasks_dir> <results_dir>

Verified the fix actually changes behavior, not just delegates cosmetically:
new regression test (empty live placeholder, nothing ready anywhere) fails
against the unmodified pre-rebase code -- the old check treated the empty
file's mere existence as "delivered" and never dispatched the task at all --
and passes with the delegation. Full suite: 14/14 (13 existing + 1 new).
shellcheck clean, git diff --check clean, review-checks.sh clean
(hardcoded-paths + root-artifacts; prose-cap skipped, detached diff).

Per Chi's merge sequence (#4317 -> #4303/#4307 consuming it -> the wiring
PR): this is the #4307 half. Taking over from Mini per Chi's direct
"if not you finish" -- no Mini activity in #bot2bot for 9+ hours across 3
separate pings/claims on this exact work.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(claude-core): positive idle-gate check + episode-bound staging/retry; comment-length cleanup

Addresses the 2 remaining standing blockers plus the repository-rule
comment-length finding. The task_dispatch consumption round already fixed
the completion-policy blocker.

1. core_is_idle()'s fresh-idle branch only checked "not busy", never the
   positive core_pane_is_idle_ready() gate check the stale-running branch
   already uses. A trust/login/permission gate is "not busy" (no in-flight
   turn to interrupt), so an old idle record surviving a restart could let
   the notifier type/submit straight into a gate. Both branches now require
   the same positive check.

2. prompt_is_staged() only checked whether the marker string was present in
   the last 20 pane lines -- a PREVIOUS episode's own prompt still visible
   in scrollback (e.g. after a requeue) reads identically to a fresh paste.
   Combined with a swallowed paste, that meant "staged" without anything
   actually having been typed this episode. Fixed by capturing a baseline
   pane snapshot before each send-keys and requiring the tail to both
   contain the marker AND differ from that baseline.

   The submit-retry loop had a related gap: it re-pressed C-m based only on
   "not confirmed yet", with no check that the composer still held our own
   prompt. If the pane changed for any OTHER reason (e.g. the owner started
   typing), the retry would submit their partial input. Now re-validates
   the pane is byte-identical to what was staged before every retry press;
   any other change fails closed instead of pressing Enter.

3. 8 comment blocks (own new code + pre-existing) exceeded the repository's
   2-line cap, some naming the reviewer -- trimmed all to the constraint
   itself, no incident narration.

Tests: 4 new regressions (fresh-idle+trust-gate, stale-marker+swallowed-
paste, pane-changed-after-enter), each verified failing against the
pre-fix code (1 timeout, 2 assertion failures) and passing after. Full
suite 16/16. shellcheck clean, git diff --check clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* test(claude-task-notifier): poll for fswatch attachment instead of a fixed sleep

qingyun-wu's review at c052edf: MainLoopWiringTest's real-watcher
integration test failed twice locally (fswatch 1.21.0) while passing in
CI, where fswatch is unavailable and the test is skipped entirely -- so
CI green never actually exercised this path either way.

Could not reproduce the failure directly (16/16 clean across 4 separate
runs on this host, same fswatch version). The test's own design has a
real, deterministic-fixable gap regardless: it slept a fixed 1.5s and
assumed fswatch had attached to the watched dir by then, before dropping
the task file. Under load (a busy review sandbox running several suites
at once) that assumption can be too short, flaking a run that exercises
no real bug.

Replaced the blind sleep with `_wait_for_fswatch_attach()`, which polls
`ps -axo command` for the actual fswatch child watching this test's
tasks dir (up to 10s) before writing the task file. Negative control:
pointed at a fresh harness with nothing watching, it correctly times out
and returns False in ~1.5s; against the real subprocess it returns True
almost immediately (full test now finishes in ~11s vs ~12s before).

Full suite still 16/16. git diff --check clean.

Stand: Echo Act IV Pro

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(claude-task-notifier): claim-aware queue selection + empty-composer gate

Two blockers from keweichen's fresh review (unwired script, no production
caller yet -- fixing before it's wired, per the review's own framing):

1. Queue rescanning bypassed required task-handler claims. Any wake
   (from ANY task) made the notifier re-scan the whole queue and pick by
   priority alone via next_pending_task -> task_dispatch.py next-pending
   with no --claims-dir, so a task explicitly claimed must-handle by a
   required handler (src/watch-tasks-stream.sh's rc==4 boundary) could
   still be typed into the unrestricted live core.

   Fixed by deriving CLAIMS_DIR the same way watch-tasks-stream.sh does
   (its own SUTANDO_WORKSPACE_DIR override, honored here too, so both
   agree under a test override) and passing --claims-dir through both
   next_pending_task call sites. task_dispatch.py already supported the
   flag (shared policy, CLAUDE.md "Shared adapter policy") -- this was a
   caller-side wiring gap, not a missing capability.

2. The first Enter was not bound to a verified-empty, unchanged composer.
   _is_idle_ready (core-input-watch.py) classifies gates/turn state and
   says nothing about an unsent owner draft sitting in the composer --
   idle-ready and a draft are not mutually exclusive. And
   prompt_is_staged accepted "our marker exists anywhere in the tail plus
   the tail changed at all" -- a stale copy of our OWN marker sitting in
   OLDER scrollback, combined with an unrelated pane change (a concurrent
   owner draft), satisfied both halves without our prompt ever landing.
   The first C-m was also sent unconditionally, before checking staging
   succeeded.

   Fixed with three changes:
   - New core-input-watch.py _composer_is_empty(), delegated the same way
     _is_idle_ready already is. deliver_prompt() requires it before every
     type attempt (not just the first) and fails closed (leaves the task
     queued) rather than typing over a draft.
   - prompt_is_staged now searches only composer_from_tail(tail) -- the
     suffix from the LAST ❯ prompt line onward -- so a stale marker above
     the live composer can never satisfy staging.
   - The first C-m now requires staged=1 AND re-verifies the exact
     composer content immediately before pressing it, matching the
     discipline the retry loop already applied to every later Enter.

Evidence (tests/claude-task-notifier.test.py, 19/19 green):

  ok test_claimed_task_is_never_selected_by_an_unrelated_wake (real fswatch
     + real watcher; a pre-existing must-handle claim is never typed)
  ok test_composer_draft_blocks_typing (an unsent draft is never typed over;
     the task stays queued, not consumed)
  ok test_stale_marker_plus_concurrent_owner_draft_is_not_mistaken_for_staged

Reverting each fix in isolation reproduces its failure against the same
tests: dropping --claims-dir types the claimed task straight into the pane;
reverting composer_from_tail to a bare tail-wide grep drops the concurrent-
draft test's type count from 2 to 1 (staged wrongly on the first, swallowed
attempt); dropping the composer-empty gate times out waiting for a result
the draft-blocking test never provides (the task got typed and submitted
instead of staying queued).

Full suite green: claude-task-notifier (19/19, includes the real-watcher
class), core-input-watch (74/74), core-input-watch-chat-escalation (40/40),
task-dispatch-contract (55/55, unchanged -- confirms the shared policy
layer already supported --claims-dir; this PR only wires the caller).
shellcheck clean (task-notifier.sh); ruff clean (core-input-watch.py,
the test file); review-checks.sh clean (hardcoded-paths + root-artifacts +
prose-cap, after trimming four over-cap test comments to the 2-line rule).
No production caller yet, so blast radius is unchanged by this fix --
matches the reviewer's own framing (fix now, keep unwired either way).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* test(core-input-watch): direct unit coverage for _composer_is_empty

CI's diff-coverage gate flagged src/core-input-watch.py:250-253 (the new
_composer_is_empty function) at 20% -- it was only exercised indirectly
through task-notifier.sh's subprocess harness in
tests/claude-task-notifier.test.py, which coverage.py cannot instrument
across a process boundary. Added a direct unit test class importing the
module in-process, matching this file's own existing convention (mirrors
TestRefusedTurn's sibling predicate).

Verified: `coverage run --branch tests/core-input-watch.test.py` no
longer lists 250-253 in core-input-watch.py's missing lines (confirmed
via `coverage report -m`, isolating this file). gh pr diff 4307 --name-only
confirms src/core-input-watch.py is the only production .py file in this
PR's diff. 78/78 (was 74/74) local tests pass; claude-task-notifier
19/19 unaffected. ruff + review-checks.sh clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(claude-core): claim-aware fresh probe, exact composer match, never-submitted must not wait, periodic self-re-arm

Four real blockers from keweichen's third review round (head 1ee49d5):

1. Claim-aware selection still bypassed the handler in the pre-claim
   window: next_pending_task only excluded a task already claimed in
   CLAIMS_DIR, which can't see a required handler still probing it.
   Ported Codex's already-shipped probe_optional_task_handler pattern:
   next_pending_task now iterates task_dispatch's pending-candidates and
   re-probes each fresh, skipping any whose handler answers 4 (required,
   not yet claimed) and any already holding a fallback receipt.

2. The first Enter wasn't bound to the exact composer content: staging
   was a substring match (grep the marker), so owner text landing in
   the gap between send-keys and the staged-tail capture would sit
   alongside our marker and still satisfy it. composer_text() (new,
   src/core-input-watch.py, shared with the empty-composer check) now
   reconstructs the composer's full dewrapped content -- stripping any
   footer/gate/border line wherever it renders -- and staging requires
   EXACT equality against the prompt we sent.

3. An unsent delivery could wait out COMPLETION_TIMEOUT (up to an hour)
   for a result that could never appear, blocking the single-threaded
   event loop from ever getting to another queued task: deliver_prompt's
   two "Enter never sent" branches returned 0 (proceed to the completion
   wait) instead of 1 (leave queued, matching the existing composer-busy
   branch). Both now return 1.

4. A task left queued by any of the above had no other trigger if no
   unrelated task file ever arrived -- it would wait forever. The main
   loop now self-re-arms via a read timeout (default 30s) and retries
   the highest-pending task on timeout, not only on a real wake.

Comment-cap: 5 pre-existing + newly-touched blocks trimmed to the 2-line cap.

Verified:
- Real pre-fix-fails/post-fix-passes for all 4, each isolated:
  - #1: direct against next_pending_task (bypassing the real watcher,
    which settles a required task from its own startup sweep before any
    event reaches this notifier, masking the race at that layer).
  - #2: caught a genuine reconstruction bug of my own first attempt too
    -- a pane-border row sitting between the composer and the footer
    leaked into the result; a direct Python unit test (not just the bash
    integration path, which never exercised that fixture shape) caught
    it before push.
  - #3: measures wall-clock elapsed, asserts it stays far under
    COMPLETION_TIMEOUT.
  - #4: real fswatch-driven main loop with a busy composer and NO second
    task file ever written; caught a real set-e-adjacent bug in my own
    first attempt (an if/then/fi wrapper around read erases read's real
    exit code, since the if-statement's own status is 0 whenever the
    then-branch didn't run) and a second real bug: macOS's own /bin/bash
    3.2 returns 1 for a read timeout same as EOF, unlike a modern bash's
    128-plus-signal convention, which would have silently killed the
    notifier on its very first quiet period in production.
- 23/23 claude-task-notifier (was 19), 85/85 core-input-watch (was 78,
  +7 direct unit tests for the new composer_text()), 40/40
  core-input-watch-chat-escalation, adjacent 55/55 task-dispatch-contract
  and core-input-watch-gateway-probe/prompt-excerpt clean.
- bash -n, shellcheck -S warning, ruff, git diff --check, review-checks.sh
  (against origin/main), gen-src-map --check all clean.

Stand: Echo Act IV Pro
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(claude-task-notifier): capture scrollback for composer staging, not just the visible pane

capture_tail() called `tmux capture-pane -p` with no -S, so it only ever
saw the current viewport. A single-line prompt that wraps taller than the
pane's own height pushes its leading composer marker into scrollback,
where no amount of `tail` on that view can recover it — staging then
fails every retry and the task is left permanently queued behind what
looks like a nonempty composer.

Fix: pass -S with a generous scrollback window, and drop the trailing
`tail -20` — composer_text() already finds the bottommost marker itself,
so pre-truncating the capture can only reintroduce the same class of miss.

Added a regression test (fake tmux with a 2-line viewport) that fails
against the pre-fix capture_tail() and passes against the fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(claude-task-notifier): scrollback capture is bounded by tmux's own history-limit, not just our env var

Two independent reviewers (rui, yixuan-ag2) on the round-5 fix pointed out
that SUTANDO_NOTIFIER_CAPTURE_SCROLLBACK_LINES only helps when it is the
binding constraint. tmux never retains more than its own history-limit
regardless of what -S asks for, so a host with a smaller history-limit (or
a prompt tall enough to exceed either bound) silently reverts to the exact
same permanent-stall bug, with nothing on disk looking misconfigured.

Fix: effective_scrollback_lines() reads tmux's real history-limit and uses
min(that, our env var) for every -S request and every truncation check, so
"we returned at least the effective cap" is now a meaningful signal
regardless of which side actually binds. When it fires, both give-up sites
(composer-not-empty pre-check and the post-retry staging check) now log a
distinct, loud line naming the condition, per yixuan's specific ask to make
this "detectable" rather than indistinguishable from a generic stall.

Also moved core_pane_composer_is_empty() onto the same scrollback-aware
capture_tail() -- its own marker search suffered the identical visibility
gap on a retry's second pass, discovered while writing this fix's own test
(a prior attempt at the composer-not-empty guard bailed early before ever
reaching the new detection code).

New HistoryLimitBoundTests regression (fake tmux with HISTORY_LIMIT=2):
fails against the pre-fix capture_may_be_truncated() (which compared
against the static env var) and passes against the fix. Full suite 25/25
(was 24), core-input-watch 85/85 unchanged, shellcheck clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(claude-task-notifier): pane-scoped history limit, keep interior composer rows, re-check busy before every keystroke

Round 7, addressing keweichen's CHANGES_REQUESTED and yixuan-ag2's finding
on 31e16c5, each reproduced by them in real tmux:

- history-limit was read from `show-options -g`, the global default; tmux
  fixes #{history_limit} at pane creation, so the global can drift above
  what the pane retains and the new truncation warning was keyed on the
  wrong number (silent in exactly the overflow case). Now reads the target
  pane's own #{history_limit} and #{history_size} via display-message; the
  truncation test compares retained history to the effective cap and counts
  RAW physical rows (blank rows occupy history) when the size is unreadable.

- _composer_text dropped ANY row matching a gate/footer signature, so an
  owner continuation row like "permission to continue" vanished and a mixed
  composer compared equal to the bare prompt and was submitted. Only the
  status bar legitimately trails an editable composer (a gate replaces it,
  and the idle/busy checks refuse first), so only TRAILING status rows are
  stripped and the marker row never is; every other row is typed text.

- the idle gate was not re-checked before the paste: a turn starting between
  the gate and send-keys put the busy footer into the baseline, staging
  compared clean, and Enter landed on a live turn. Busy is now re-checked
  immediately before the paste and before every Enter.

Test harness now models a real pane: typed text lands on the ❯ row above
the status bar, wraps at WRAP_COLS, `-S -N` returns N history rows plus the
viewport, `display-message` reports the pane's limit/size separately from
the global, and each capture is counted so a test can flip the pane busy on
the read right before the paste without hard-coding the notifier's order.

Mutation controls, each run against the exact test that should catch it:
  global-limit read restored  -> HistoryLimitBoundTests       FAIL
  `| tail -20` restored       -> TallComposerScrollbackTests  FAIL
  pre-paste busy check removed-> BusyBeforePasteTests         FAIL
  signature rows dropped again-> OwnerRowResemblingUiTextTests FAIL (+11 in TestComposerText)
Two older tests changed expectation from "retype once" to "refuse, composer
not empty": with text on the marker row, a mixed composer is non-empty and
typing over it would paste our prompt onto owner text.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(core-input-watch): the CLI's empty-composer hint is not an owner draft

Found by the isolated live witness, not by any fake: Claude Code v2.1.275
renders a hint in the EMPTY composer (`❯ Try "refactor <filepath>"`), and a
plain capture-pane loses the dimming that distinguishes it from typed text.
_composer_is_empty read it as an unsent draft, so the notifier refused every
task on a genuinely idle pane ("composer not empty ... failing closed") --
correctly cautious, and wrong.

The hint vanishes on the first typed character, so a marker row of exactly
that shape is empty; a row that merely starts like it is still a draft.
_composer_text returns "" for it too. The fake tmux now types onto the
bottommost ❯ row wherever it sits (below it may be a box border, then the
status footer) instead of assuming it is the last non-footer line, which the
real 2.1.275 layout is not.

Mutation control: with the placeholder treated as a draft again,
PlaceholderComposerTests FAILS (two pastes, no Enter). notifier 28/28,
core-input-watch 91/91, review-checks PASS.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(claude-task-notifier): staging equality ignores whitespace -- the input box word-wraps and indents

Second live-witness finding: on a 120-column pane Claude Code's input box
word-wraps the pasted prompt and indents every continuation row by two
spaces, so the dewrapped capture reads "task,  and write" where the prompt
says "task, and write". Exact equality failed on every real paste, the
retry then saw its own text as a foreign draft, and the task stayed queued.

prompt_is_staged now compares both sides with all whitespace removed. The
interleaved-owner-text guard is unchanged in effect: owner text adds
non-space characters, so a mix still compares unequal (pinned alongside).
The fake tmux grows a "word" wrap style mirroring the real layout.

Mutation control: raw equality restored -> LiveWordWrapTests FAILS with
the exact live shape (paste logged, no Enter). notifier 30/30, review-checks
PASS.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(claude-task-notifier): one canonical workspace root, structural footer strip, single judged snapshot before the paste

Round 8, addressing keweichen's and qingyun-wu's CHANGES_REQUESTED on
5dd6417, each reproduced by them:

- Workspace authority: the handler probe passed `--workspace
  "$(dirname "$TASKS_DIR")"` while claims used SUTANDO_WORKSPACE_DIR, so with
  a separate task inbox the handler was asked about the inbox's parent,
  answered "optional", and a protected task was selectable by the
  unrestricted core. One WORKSPACE_DIR now feeds the probe, claims, receipts,
  status, results and logs. Regression: separate-inbox layout with a handler
  that returns required only for the canonical root.

- _composer_text stripped trailing rows by classifying them in a loop, so
  after the real footer went, an owner row reading "for agents" became
  trailing, matched the idle regex, and was eaten; the mix then equalled the
  prompt and was submitted. The strip is now structural: trailing box rules,
  at most ONE status row, box rules again; nothing exposed is re-classified,
  and an owner row of box characters survives too. The interior border
  filter is gone (the box rule only ever trails the composer).

- The busy re-check was followed by one more unchecked read (the baseline)
  before the paste. Now ONE snapshot is taken, judged positively idle-ready
  (a gate or a turn fails it, not just "esc to interrupt") and composer-empty,
  and that same snapshot is the baseline. Every pane predicate has a text
  form so a single capture can answer all three.

- Diagnostic: the truncation warning treated a visible empty marker like no
  marker; composer_has_marker distinguishes None from "".

Fake tmux: an owner continuation row is inserted as its own line under the
composer (the previous fixture appended it to the marker row and could not
expose the re-classification), and a trust gate can replace the pane on the
Nth capture. Mutation controls, each run against the exact test:
  probe --workspace back to dirname TASKS_DIR   -> separate-inbox test FAIL
  status-row pop turned back into a loop        -> for-agents e2e + 16 unit FAIL
  baseline re-read after the judgement          -> BusyBeforePasteTests FAIL (2)
notifier 33/33, core-input-watch 94/94, review-checks PASS.

Not done here, deliberately: adopting #4320's shared pane-policy owner.
#4320 is itself open and CHANGES_REQUESTED on a stacked base; this PR will
rebase onto it and delegate once it lands rather than stack onto a blocked PR.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
sonichi added a commit that referenced this pull request Sep 28, 2026
…ged (#4874)

* fix(health-check): detect a credential-proxy that's listening but wedged

check_credential_proxy() only ever proved the port is TCP-listening
("probe=False: a forwarding proxy has no liveness endpoint, so an HTTP
probe is forwarded and misread as wedged"). That left the exact failure
mode from the 2026-09-27 incident invisible: the proxy stays up and
accepts connections while every request 401s/502s because its stored
credential can't be refreshed -- 55min-2h of every seat losing the API
before a human noticed, because health-check kept reporting "ok".

credential-proxy.ts already tracks this per request via
recordCredentialState(), writing credential_state/_detail/_at to
quota-state.json: 'ok' on each successful token injection, 'exhausted'
when a request couldn't get a usable credential. That signal already
exists and is self-healing (the next successful request flips it back
to 'ok'); nothing needed to change in the proxy itself.

check_credential_proxy() now reads it and escalates to 'warn' when the
proxy's own last-recorded state says 'exhausted', with the recorded
detail and how long ago. Only consulted while the port is already
'ok'/'stale' (never what marks a genuinely-down proxy), silent on any
read failure (advisory, same rule mark_stale_if_outdated already
follows for this exact file).

Known gap this does NOT close: recordCredentialState is only called
from the request path, so a background refresh failing before any
request arrives (the original incident's literal trigger) stays
invisible until #4842's give-up-after-N-failures exits the process --
which is now bounded (~15.5 min at the default threshold, down from the
unbounded wedge this incident hit) rather than closed. Proposed fix #4
(per-seat runtime fallback) is separate follow-on work, not attempted
here.

Fixes proposal #1 from the "Credential proxy (:7846) wedges silently"
post-mortem (ww room, Issues DB row rmukn84lu4akyf), alongside already-
merged #4842 (fix #2) and #4845 (fix #3, mine). Requested by Chi.

Also isolates tests/health-check-credential-proxy-staleness.test.py from
this host's real quota-state.json: it didn't patch WORKSPACE_DIR, and my
new read path would otherwise have read live host state through it.

* fix(review): ignore a stale 'exhausted' record from before a restart

Kewei/Qingyun review on #4874: recordCredentialState() writes on a
transition only and is never reset at startup, so a restarted proxy
(the exact recovery path #4842/#4845 rely on) inherits whatever
'exhausted' was last written before it existed -- a false warn until
the first request succeeds. Compares credential_state_at against the
current process's start time (_proc_lstarts, already used elsewhere in
this same check) and ignores a record older than it.

Also trims the function/module docstrings per review: the repo's
comment rule, and prose-cap, flag narrative/incident history in
comments -- that context now lives in the PR body instead.

* fix(review): correct the docstring's backwards phrasing

The escalation fires when the record was recorded by the current
process, not when it predates it — the docstring said the opposite.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant