Repository navigation
voice: crash-only fatal handlers, exit-7 duplicate-lock contract, guarded PID-lock transactions (phase 1) - #2684
Conversation
… test
scripts/voice-lock.py is the single implementation of the voice-agent
PID-lock transaction (design 1b; impl plan WS1 Step 3, amendments
R3/S4/U1/Z1): every create/validate/replace runs under an advisory
fcntl.flock on <workspace>/.voice-agent.lock.guard, the JSON lock
({v:1, lockId, pid, startTimeMs, entry, workspace}) is owner metadata
only, legacy bare-pid locks stay readable for one release, and a live
lock is never removed. The takeover subcommand owns the whole
kill-and-replace transaction (validate -> TERM -> wait -> KILL ->
revalidate -> lockId-compared unlink) under the held guard, with an
adopted mode (full U1 identity check incl. the :9900-listener
cross-check) and an owned mode (supervisor-recorded pid/pgid + start
time + entry, whole-process-group kill, no listener requirement).
sutando-config.sh python-bin now prints an ABSOLUTE interpreter path
only after executing it and importing fcntl, else exits non-zero with
an actionable fix (amendment T1) — callers of the lock helper must
never receive an unverified interpreter.
Tests: tests/voice-lock.test.py (fresh/held/stale/PID-reuse/unknown
acquire, one-release legacy fallback, concurrent-acquire single
winner, steal semantics, guard-hold kernel release, graceful-TERM /
KILL-escalation / concurrent / blocked takeovers, owned process-group
kill) and tests/sutando-config-python-bin.test.py (explicit, broken,
bundled, PATH, absent tiers).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…red lock Crash-only conversion (design 1d; impl plan WS1 Steps 1-2, amendments R1/R2): uncaughtException/unhandledRejection now TERMINATE instead of logging err and staying alive (the 2026-08-04 incident spun inside the old reporting path itself). The handler writes a bounded primitive-only crash record to <workspace>/logs/voice-agent.crash.json (constructor name <=64 + String(message) <=512, each extracted in its own try/catch; no logger/recorder/SQLite/err.stack) and exits in the outermost finally. One-shot guard: a second fatal goes straight to process.exit(1). recorder.flush() stays only in graceful shutdown. Exit classification is centralized (src/crash-only.ts): EADDRINUSE reaches exit 7 on BOTH paths — the uncaught handler and main().catch — matching the duplicate-lock exit 7 the supervisor's grace window keys on (Step 2 / R2). acquirePidLock delegates the whole lock transaction to the guarded voice-lock.py helper (Step 4): entry realpath'd from argv[1], interpreter resolved once via sutando-config.sh python-bin (R4/T1, injectable for tests); helper held -> two FATAL lines + exit 7; interpreter/helper unavailable -> FAIL CLOSED with an actionable error (R3 — no unguarded legacy bare-pid writer). Exit-time release is a non-blocking detached spawn and is suppressed entirely on the fatal path (R1/S4) — the stale lock is left for the supervisor to replace under the guard. Tests: tests/voice-agent-crash-only.test.ts — crash-record unit cases (throwing getters, failing fs write, plain Error, bounds), R2 classification, one-shot guard, R1 release suppression, resolver failure mapping; integration: duplicate spawn (loser exits 7 with the FATAL line, winner keeps its structured lockId'd lock), EADDRINUSE -> exit 7 via main().catch, and fail-closed boot with a broken SUTANDO_PY. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…akeover scripts/restart-voice-agent.sh (impl plan WS1 Step 5, amendment S4): the stale-pid block no longer cats a bare int, greps only the dev argv shape, kills from the shell, or rm -f's the lock. The whole kill-and-replace transaction is one 'voice-lock.py takeover' invocation under the held fcntl guard, validating dev (src/voice-agent.ts) and packaged (dist/voice-agent.js) entry shapes plus the :9900-listener cross-check (U1). takeover-blocked / unknown lock -> WARN and proceed to kickstart WITHOUT touching the lock (a live lock is never removed); interpreter unavailable -> fail closed (R3) instead of racing the lock. src/restart.sh (amendment U2): the broad 'pkill -f voice-agent' is replaced by the same guarded takeover; identity mismatch means nothing is signaled, and a missing interpreter skips the stop fail-closed rather than killing blindly. Tests: tests/restart-voice-agent-lock.test.py — legacy dead-owner cleared; structured dev- and packaged-shape owners killed + stolen; malformed lock left in place with a WARN; live unvalidated owner never signaled, lock never removed (launchctl/lsof PATH shims). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_argv_entry_matches tokenized `ps -o args=` on whitespace, so an entry path containing spaces (e.g. a packaged install under '~/Library/Application Support/...') could NEVER match a live argv: the path split into fragments, every adopted takeover of a legitimate owner became takeover-blocked, and the identity check that exists to protect bystanders instead wedged real restarts. Candidates are now every whitespace-boundary-to-boundary substring of the full args string: absolute candidates match on realpath equality with the (realpath'd) lock entry, relative candidates as a path suffix — a superset of the old single-token behavior, so all existing shapes still validate. Test: adopted takeover of a stand-in listener whose entry lives under an 'Application Support' directory (spaced path) — replaced, owner terminated, lock unlinked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…RINUSE path (R1/R2/1d)
main().catch was the one fatal path still outside the crash-only rules:
it never set the fatal flag (so the exit-time Python lock release could
run — and block on the guard — from a fatal exit) and its generic branch
logged `console.error('Fatal:', err)`, the exact object-inspection
reporting shape the 2026-08-04 incident spun inside.
Both fatal paths now obey identical rules: markFatalExit() first (R1
release suppression + one-shot guard — a second fatal goes straight to
process.exit(1)), EADDRINUSE still classifies to exit 7 (R2), and the
generic branch writes the bounded primitive-only crash record via
writeCrashRecordAndExit with a static-string log line — never err
inspection. The crash-record path is hoisted to a module const shared by
the uncaught handler and main().catch.
Two test-only fault-injection seams (SUTANDO_TEST_MODE gated) drive the
real paths end-to-end: SUTANDO_TEST_FAIL_MAIN throws after the lock is
acquired; SUTANDO_TEST_RAISE_UNCAUGHT throws asynchronously after the
fatal handlers are installed.
Tests: EADDRINUSE loser via main().catch now also asserts its own lock
stays in place (release suppressed on THIS path too); a generic main()
failure exits 1 with the bounded crash record, the static [FATAL] line,
no object-inspected dump, and the lock left for the supervisor; and a
direct uncaughtException EADDRINUSE case exits 7 through onFatal (not
main().catch) with no crash record and the lock left in place.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s guarded + grep gate The three remaining direct kill/kickstart call sites now route through the guarded 'voice-lock.py takeover' transaction (impl plan amendment T4), and a grep-based CI gate keeps the inventory closed: * startup.sh listener reap (:9900): the generic reap_wedged_listener's `lsof -ti | xargs kill` no longer applies to voice-agent. A new reap_wedged_voice_agent (src/startup-runtime.sh, sourceable for tests) keeps the same real-HTTP wedge probe but delegates the kill-and-replace to ONE guarded takeover: identity mismatch or an unknown/absent lock => takeover-blocked, nothing signaled, lock untouched; interpreter unavailable => fail closed (skip the reap). * voice-config-switch.ts: the detached `launchctl kickstart -k gui/<uid>/com.sutando.voice-agent` spawn is replaced by the guarded wrapper scripts/restart-voice-agent.sh (exported fireGuardedRestart, spawn injectable for tests). * health-check.py fix_launchd: the voice-agent label's repair now runs the guarded wrapper instead of the direct kickstart; the launchd `bootstrap` fallback stays (it loads a job, signals nothing); web-client keeps the direct kickstart (never names voice-agent). Kickstart semantics for the last two sites (T4 note): `launchctl kickstart -k` of a launchd job is a kill-and-RESTART, not a raw kill, so the guarded transaction wraps the PRE-kickstart validation — the identity of the running job pid is validated and any orphan/stale lock resolved as one voice-lock.py takeover under the held fcntl guard (validate -> TERM -> wait -> KILL -> revalidate -> unlink), and only then does the wrapper kickstart (which launchd itself scopes to its own job's pid) and verify the restart really happened via the fresh-listener etime check. Identity mismatch => takeover-blocked, no signal; no usable interpreter => the wrapper fails closed (exit 6) before touching lock or process. New wrapper precondition: with those call sites routed through it, the wrapper now aborts (exit 5) BEFORE the takeover when the launchd job is not loaded — on a plain dev checkout the old direct kickstart silently no-op'd, and killing a startup.sh-launched agent with nothing to respawn it would turn a working voice stack into an outage. health-check's bootstrap fallback still handles the plist-present-but-unloaded case. CI gate: tests/voice-agent-kill-path-gate.test.py greps every tracked source file for unguarded shapes (`kickstart -k ... voice-agent`, `pkill ... voice`, kill+9900, generic reap of 9900) outside the two allowlisted helpers (scripts/voice-lock.py — the transaction itself — and scripts/restart-voice-agent.sh — the guarded wrapper), skipping comments and substitution-free advisory echoes; known documentation strings are allowlisted per line so any new hit is a conscious edit. Verified to fail on injected kickstart-k and pkill violations. Tests: startup wedge recovery (validated owner killed under the guard; non-listener lock blocked with no signal; no-lock never kills the unvalidated port owner; responsive listener untouched; no interpreter fails closed; startup.sh wiring asserted), config switch (wrapper spawned detached, no launchctl argv anywhere in the tool), health-check repair (wrapper invoked, never direct kickstart; wrapper failure falls back to bootstrap, not kickstart; plist-less host stays advisory and signals nothing; web-client parity), the no-job abort (signals nothing, leaves the lock, no kickstart), and the gate itself. tests/health-check-fix-launchd-label.test.py's pinned expectation updated to the guarded contract (still proves issue-sonichi#1888's --fix actually restarts the service). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
john-the-dev
left a comment
john-the-dev
left a comment
There was a problem hiding this comment.
Reviewed and verified end-to-end at head c64c7a5e. Cloned the PR, installed deps, and ran the full added/changed test surface: voice-lock.test.py (~45 checks incl. concurrent-acquire single-winner, PID-reuse steal refusal, TERM→KILL escalation, spaced-path, owned process-group kill), restart-voice-agent-lock.test.py, startup-voice-wedge-reap.test.py, sutando-config-python-bin.test.py, voice-agent-kill-path-gate.test.py, health-check-voice-guarded-fix.test.py (4/4), health-check-fix-launchd-label.test.py — all pass; plus voice-agent-crash-only.test.ts 16/16 (incl. the real-spawn cases: loser exits 7 keeping the winner's lock, EADDRINUSE→7 through both main().catch and the uncaughtException handler, fail-closed boot on a broken interpreter, release suppressed on fatal) and voice-config-switch-guarded-restart.test.ts 3/3. tsc --noEmit clean; src-map.test.py 25/25.
Audited the load-bearing logic:
crash-only.ts:exit()is in the outermostfinally;name/messageare each extracted in their own try/catch (throwing-getter safe); a one-shotmarkFatalExitguard routes a second fatal straight toexit(1).voice-lock.py: the guarded-transaction invariants hold — "a live lock is never removed," fail-closed with no unguarded fallback, identity validation before any signal. Confirmed 3.9-safe (no 3.10+ syntax,from __future__ import annotations,py_compileOK on the bundled floor).
The dev-checkout behavior change (restart-voice-agent.sh exit-5 abort when the launchd job isn't loaded) is a correct safe tradeoff, not a regression vs. the old silent-no-op — aborting beats "kill with nothing to respawn." Net: this strictly improves crash/lock/exit recoverability over the pre-PR spinning-wedge behavior. LGTM.
Minor non-blocking nits (no need to hold): _argv_entry_matches is O(n²)+realpath per candidate but runs once per takeover; src/restart.sh adopted-mode no-ops on an absent lock (recovery falls to startup.sh wedge-reap, acceptable). Branch is BEHIND main — needs an update-branch before merge.
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Static review of the diff at the current head (21 files, +3126/−111). I did not run the test suite and did not reproduce the 08-04 incident — everything below is read from the diff, and I say so per REVIEW.md ("if you did not verify a claim, say so explicitly").
What I checked against REVIEW.md
6 — hardcoded absolute paths: PASS. Scanned every added line for /Users/…, /home/…, C:\Users. Zero hits.
4 — destructive / auto-remediating actions, default state + blast radius. This is the part that deserved the most attention: the PR adds two process-group kills (os.killpg(pgid, sig) in voice-lock.py, process.kill(-child.pid, 'SIGKILL') in the TS path). A group kill with a wrong gid is unbounded, so I traced how pgid is established. The guard chain before the kill is genuinely strong, and each failure is a distinct blocked code rather than a fallthrough:
- start-time must match the supervisor-recorded value →
takeover-blocked(this is the PID-reuse defence, and it's the right one) argvmust match an expected voice-agent entry →takeover-blockedpgid is None→takeover-blocked- a descendant lock holder must be in the same group →
takeover-blocked - post-kill, the pidfile is unlinked only after re-validating
lock_id→takeover-blockedon concurrent replacement
Replacing the old blind pkill -f voice-agent with this is a clear net reduction in blast radius.
3 — proving the fix by exercising the failure mode: PASS. 24 added test cases, and they hit the failure paths rather than the happy path — injected fdOpen failure with exit still called, EADDRINUSE → exit 7, duplicate-spawn loser exits 7 while the winner keeps the lock, non-EADDRINUSE main() failure still producing a bounded crash record, unusable interpreter failing closed with no bare-pid fallback lock. The fault injection on the crash-record writer is the right instinct, since the 08-04 spin was in the reporting path.
1 — bug exists on main. The body cites the incident with a profile trace (TriggerUncaughtException → InspectorConsoleCall/ErrorStackGetter), which is concrete. I did not independently reproduce it.
One hardening nit (non-blocking)
voice-lock.py takes --pgid as type=int, default=None and rejects only None. killpg(0, sig) signals the caller's own process group, so a --pgid 0 invocation would have the supervisor kill itself. No caller in this diff passes --pgid (the internal path uses pid_pgid(root), which never yields 0 for a live process), so this is not reachable as written — but since this file is explicitly "the single guarded implementation of every PID-lock transaction" and is meant to be called by other things later, I'd reject pgid <= 1 alongside the is None check. One line, and it closes the hazard before a future caller can find it.
Merge-readiness
From a static read, I see no blocker. The guard chain, the exit-7 contract, and the failure-mode tests all look right, and the hardcoded-path check passes. I am explicitly not recording this as a formal approval, because I have not run the suite or exercised the wedge path on a real host — for a change that kills process groups on a live supervisor, someone should confirm the tests green and do one real wedge-and-reap round trip before merge. If a maintainer has already done that, treat my note as one more pair of eyes on the kill path rather than an independent verification.
|
Independent execution pass at head All five python suites green by my own run: One check neither review pinned — the exit-7 CONSUMER side: the code comments repeatedly key on "the supervisor's exit-7 grace window", but no shell supervisor in this diff implements it. It lives in desktop#282 (exit-7 grace window + two-ledger budget, already reviewed) — so the producer/consumer contract is coherent ACROSS repos, and the owner's merge ordering (this first, desktop pin-bump after) is exactly right. Worth one line in the PR body naming desktop#282 as the consumer so the contract isn't archaeology later. Susan's No blockers from me — with john's run-verified approval this is merge-ready. — qingyun-sutando (001) |
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Reviewed per REVIEW.md, with the verification gap the prior two reviews flagged now closed: I ran the suite locally on the PR head (c64c7a5, the current merge-of-main head).
What I ran (all green)
tests/voice-lock.test.py -> All voice-lock tests passed.
tests/restart-voice-agent-lock.test.py -> All restart-voice-agent lock-migration tests passed.
tests/startup-voice-wedge-reap.test.py -> All startup wedge-recovery tests passed.
tests/health-check-voice-guarded-fix.test.py -> OK
tests/voice-agent-kill-path-gate.test.py -> voice-agent kill-path gate: clean.
tests/sutando-config-python-bin.test.py -> All python-bin contract tests passed.
tests/health-check-fix-launchd-label.test.py -> all passed
npx tsx --test tests/voice-agent-crash-only.test.ts tests/voice-config-switch-guarded-restart.test.ts
-> # pass 19 / # fail 0
git diff origin/main...HEAD | bash scripts/review-checks.sh -> PASS (hardcoded-paths clean)
Independent read of the kill paths (REVIEW.md lessons 2/4/5)
I traced both takeover modes rather than re-reading the PR body. Adopted mode: unparseable lock → blocked (a live lock is never removed); dead owner / PID-reuse cleared only via _unlink_after_revalidate; U1 holds — the lock pid must equal the :port LISTEN pid before anything is signaled; entry + workspace validated for structured locks; final unlink re-validates lockId. Owned mode: supervisor-recorded start-time must match, argv entry must match, pgid is None → blocked, and a descendant lock holder outside the owned group → blocked before the killpg. The startup integration replaces the old port-match lsof | xargs kill with this guarded transaction, fails closed when no usable interpreter is found, and only fires on a real liveness-probe timeout (curl rc 28 specifically — other probe errors leave the listener untouched). Worst-case-disruption question (lesson 5): the reap is on by default at startup, but every kill is identity-validated where the old path signaled on a port match alone — strictly narrower blast radius than what ships today.
Seconding one nit + one owner-actionable note
- @liususan091219's
--pgidhardening nit is correct and correctly non-blocking: I verified the diff's only--pgidoccurrence is the argparse definition — no caller passes it — sokillpg(0, …)is unreachable as written. Rejectingpgid <= 1is a worthwhile one-liner here or in the stacked follow-up. - Full CI has not run on this head: the head's workflow runs are all
action_required(fork PR awaiting a maintainer's "Approve and run") — onlyrecheckandlicense/clahave reported. My local runs above cover the new suites, but a maintainer should approve the workflow runs so the required checks report before merge.
Approving on the strength of the local runs + the traced guard chain.
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Follow-up to my earlier static review — I reproduced the failure mode this PR targets, and ran the detection logic against three different wedge shapes. Both were done in isolation (scratch port 9978, simulated process in its own process group); no real voice-agent was touched.
1. The failure mode reproduces exactly as the body describes
A process that binds the port, accepts the connection, and never answers:
port 9978 IS held by the wedged process
liveness probe -> curl_rc=28
Three simulated supervisor restarts against it:
restart attempt 1: FAILED — Address already in use (EADDRINUSE)
restart attempt 2: FAILED — Address already in use (EADDRINUSE)
restart attempt 3: FAILED — Address already in use (EADDRINUSE)
That is the 08-04 shape: the process never dies, so the port is never released, and restarting is useless however many times you try. After a guarded kill of the wedge's process group, the port binds immediately — so takeover is the right shape of fix, and "just restart harder" provably is not.
Caveat on what this does and does not show: I used equivalent kill logic, not this PR's scripts/voice-lock.py takeover (which wants a properly formed pidfile + recorded start time). So the failure chain and the value of takeover are reproduced; the implementation in this PR is still unexecuted by me.
2. reap_wedged_voice_agent only recognises ONE shape of wedge
[ "$rc" -eq 28 ] || return 0 means only a curl timeout counts. I ran the same two lines against three wedge shapes, with a control:
| simulated failure | curl rc | reaped? |
|---|---|---|
| hangs, never answers (control) | 28 | yes — takeover proceeds |
| answers HTTP 500 immediately, forever | 0 | no — returns early |
| accepts then resets the connection | 56 | no — returns early |
The control row matters: it confirms the harness can detect the positive case, so the two negatives are meaningful rather than a dead probe.
Consequence: an agent that is fast but useless is never reaped. curl without -f treats a 500 as success (exit 0), and a reset gives 56 — neither is 28, so reap_wedged_voice_agent returns before the guarded takeover it exists to gate. Discord's equivalent doesn't ask why a peer stopped being useful; here only one manner of death counts.
I'd treat this as a follow-up rather than a blocker: the 08-04 incident was a starved event loop, which genuinely does time out and is caught by the current check. The gap is that nothing covers the fast-garbage case, and the threshold errs toward under-reaping (which is the safe direction — a transient stall can't cause a wrong kill).
Small hardening note, repeated from my earlier review
voice-lock.py accepts --pgid (type=int, default=None) and rejects only None. killpg(0, sig) signals the caller's own process group. No caller in this diff passes --pgid, so it isn't reachable as written — but building the reproduction above, I had to guard my own harness with pgid != mine && pgid > 1 or it would have killed my own shell. For a file described as the single implementation of every PID-lock transaction, rejecting pgid <= 1 next to the existing is None check is one line and closes it before a future caller finds it.
|
@yixuan-ag2 this is 2-approved + mergeable but a required check ( |
|
Merge position: Stage 1 — first of the sutando chain (#2684 → #2689 → #2690/#2691). Please use a merge commit, not squash: the three later PRs' branches contain this branch's history, so a merge commit makes each child's diff collapse to its own commits after the parent lands, while squash would leave every child's diff polluted with the parent's changes (or force rebases that invalidate the approvals). 🤖 Generated with Claude Code |
Head branch was pushed to by a user without write access
6484bda to
43d39d1
Compare
|
Double-check after the GitHub outage: the prior required check |
|
Approved the fork workflow runs for the current head |
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 77%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
scripts/voice-lock.pyLines 328-336 328 record = _create_lock(args.pidfile, args.pid, args.entry, args.workspace)
329 except OSError as e:
330 if e.errno == errno.EEXIST:
331 _emit({"code": "held", "holder": read_lock(args.pidfile)}, 7)
! 332 raise
333 _emit({"ok": True, "lock": record})
334
335
336 def cmd_read(args):Lines 366-374 366 if pid_alive(lock["pid"]):
367 actual = pid_start_time_ms(lock["pid"])
368 expected = None
369 if args.expect_start_time_ms is not None:
! 370 expected = args.expect_start_time_ms
371 elif lock["kind"] == "structured":
372 expected = lock["startTimeMs"]
373 if expected is None or actual is None or start_times_match(actual, expected):
374 # Alive with no disproving start-time evidence → a live lock isLines 393-401 393 # Hold until stdin EOF; the kernel releases the flock when we die with our
394 # parent (long-lived hold, auto-release on crash).
395 try:
396 while sys.stdin.read(4096):
! 397 pass
398 except (OSError, KeyboardInterrupt):
399 pass
400 sys.exit(0) |
|
Required CI did not pass on head |
|
Addressed on head 1770747:
Local verification: voice-lock functional suite green; 14 branch tests green; tsc clean; Ruff and ShellCheck clean. The PR body already names desktop #282 as the exit-7 consumer. Required fork workflows are currently action_required and await maintainer approval. |
|
@cla-assistant check |
… stacked on #2684) (#2689) * feat(voice-agent): agent.state v1 emitter + lifecycle snapshot + Z3 idle fence agent.state v1 protocol (design 1a', impl plan WS1 Step 12, amendments R8/A9/A10/S3/Z3): a new src/voice-agent-state.ts owns the pure pieces — createAgentStateProvider assembles {type:'agent.state', v:1, initialized, upstream: live|idle|connecting|backoff|failed, reason?, category?, clientAttached, credentialSource?, credentialGeneration?, launchdContract?} from late-bound getters; voice-agent wires it to bodhi's probeState option behind a feature-detect (the Step-11 pin lands separately), sends an immediate frame on every accepted real connection, and repeats on every upstream transition (session.stateChange subscription + the classifier's non-retryable close path; reason/category changes within 'failed' count as transitions). R8: the failure classification extends voice-error-classifier.ts with a stable protocol mapper (auth_invalid -> failed/auth/auth-invalid, quota_exceeded -> failed/quota/quota-exceeded, credits_depleted -> failed/quota/credits-depleted, model_not_found -> failed/other; retryable -> null) and persists the last terminal classification for buildAgentState (cleared on ACTIVE recovery). The startup /credit|quota/ regex stays a logging hint; startup failures route through the same classifier — no second classifier. A10/S3: credentialSource comes from the EXISTING resolveCredential result (voiceApiKey() signature untouched; env maps to 'byok' on the wire). resolveCredential now also REPORTS an opaque Rust-minted generation — managed entries' `generation` field or the injected SUTANDO_VOICE_CREDENTIAL_GENERATION for the voice env tier; legacy keys stay generationless (field omitted). launchdContract:1 echoed only under SUTANDO_VOICE_LAUNCHD_CONTRACT=1 (R17). A9: this emitter is the ONE writer of state/voice-lifecycle.json {at, clientAttached, initialized, upstream, category, credentialSource, credentialGeneration}, published on client attach/detach, the initialized flip, and upstream changes via temp+rename with unique per-write temp names (writeVoiceState's plain writeFileSync is not the pattern). Z3: createIsolatedIdleRestore arms an isolated idle-teardown timer on probe close with no real client attached (restoring the prior idle state), fenced by any later real connection; until the bodhi pin exposes a probe/verifier-close hook, the probeState callback is the in-repo arm point (seam documented). Tests: tests/agent-state-protocol.test.ts (frame shape/vectors, resolver generation reporting incl. legacy omission, lifecycle-file atomicity with a cross-process torn-read race, Z3 arm/fence semantics, and spawned-agent integration: immediate frame on connect, injected terminal close -> failed/quota transition frame through the real transport.onClose seam, lifecycle attach/detach tracking) plus classifier mapper/persistence cases in tests/voice-error-classifier.test.ts. Existing TS suite green (772/772) and voice-adjacent python suites pass; the state-paths gate drove voiceLifecyclePath through the canonical statusPath resolver. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(agent-state): use ephemeral ports for spawned-agent integration tests Fixes the review nit that 19981-19984 were hardcoded — parallel runners or a stray listener could collide. Each spawn now binds :0 and reads back the assigned port. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: regenerate source map after stack rebuild --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Phase 1 / PR group A of the desktop voice-reliability plan (design + plan: ag2-space/ag2space-cinny-desktop#281 → merged to main there; desktop companion: ag2-space/ag2space-cinny-desktop#282). Structured per CONTRIBUTING.md ("The PR body should answer").
What changed, and why?
Follow-up to the 2026-08-04 incident: a voice-agent wedged in an uncaught-exception loop held
:9900and its PID lock for ~5 hours; supervisor restarts lost the duplicate-lock race forever, and the crash-reporting path itself was the spin (profile:TriggerUncaughtException → InspectorConsoleCall/ErrorStackGetter). This PR makes the class survivable:src/crash-only.ts): bounded primitive-only crash record via rawfs.writeSync(no logger/recorder/SQLite, nevererr.stack),process.exitin the outermostfinally; both fatal paths (uncaught handler andmain().catch) obey the same rules.scripts/voice-lock.py: the single guarded (fcntl.flock) implementation of every PID-lock transaction — acquire/read/release/steal/guard-hold and a full takeover (validate → TERM → wait → KILL → revalidate → unlink under one held guard); structured{v, lockId, pid, startTimeMs, entry, workspace}lock; adopted mode cross-checks the:9900listener PID + realpath'd entry; owned mode kills the process group. Interpreter viasutando-config.sh python-bin, now smoke-testing execute+import fcntland failing closed.restart-voice-agent.sh,src/restart.sh'spkill,startup.sh's:9900reap,voice-config-switch.ts'skickstart -k,health-check.py's launchd repair — all through the guarded transaction, plus a grep CI gate (tests/voice-agent-kill-path-gate.test.py) against new unguarded kills.What files should reviewers look at first?
scripts/voice-lock.py(the transaction semantics),src/crash-only.ts+ thevoice-agent.tshandler/acquirePidLockrewrite, then the call-site migrations (scripts/restart-voice-agent.sh,src/startup-runtime.sh,src/voice-config-switch.ts,src/health-check.py).What user behavior or bug does this prove?
A wedged/killed voice-agent no longer bricks voice until a human intervenes: fatal errors terminate promptly (supervisor restarts), a duplicate instance exits 7 cleanly, and every kill path validates identity before signaling (never kills a stranger on the port; never deletes a live lock).
Feature/documentation consistency
N/A for
docs/canonical documents:docs/catalog.jsonhas no canonical doc for PID-lock/supervision/exit-code semantics (closest:design-credential-capability-resolver.md— untouched by this PR — andvoice-agent-test-framework.md— unaffected). The exit-7 contract is documented in theacquirePidLockdoc comment and the module headers here; the cross-repo supervision contract is canonically documented in the desktop repo (docs/design-voice-reliability-and-onboarding.md, merged), which governs this work.Before/after evidence
Before (the incident, 2026-08-04, unpatched): voice-agent pid 14059 wedged for ~5 h; supervisor log repeated
FATAL: voice-agent already running (pid 14059)on every restart attempt; recovery required manual SIGKILL + pid-file removal.After (2026-08-05, owner-run on a local desktop DMG bundling this branch):
kill -9of the live voice-agent (pid 84771) → supervised replacement (pid 97888) serving within moments, structured lock re-acquired cleanly, and the desktop ledger recorded it:Bot-verify (new-behavior suites, do not exist at the merge base — all green at HEAD):
What tests did you run?
Above, plus after updating to latest
main(merge commit on the branch):tests/src-map.test.py25 ✓, full tsx suite 128 files / 746 pass / 0 fail,npm run typecheckclean,scripts/check-python39-compat.pyclean. Known env-specific failurestests/core-input-watch.test.py(1) andtests/runtime-health.test.py(4) fail identically atorigin/mainwith byte-identical sources — pre-existing, not introduced.Live-path / voice manual evidence (owner, 2026-08-05, on the DMG bundling this branch)
Voice exercised over both entry points — the rail mic and the room-header Start Call → Voice dropdown (force-tier menu rendering T0-a/T0-b, T1–2 n/a, T3 rows) — with real
workround trips. Transcript excerpt fromdata/conversation.sqlite(voicetable, sessionssession_1785963296069/…703658):(The 21:00:48 turn is the completed
worktask speaking its result — the voice→core→voice loop with this branch's lock/crash machinery active. Idle-teardown then closed the Gemini transport normally.)Edge cases / non-happy paths checked
Throwing
message/stackgetters in the fatal path; injected fd-open failure; EADDRINUSE through both fatal paths; concurrent lock acquisition (exactly-one-winner); PID-reuse steal refusal; TERM-ignored KILL escalation; spaced"Application Support"entry paths; owned-mode kill of a non-listening parent/worker process group; malformed/legacy/absent lock forms; missing interpreter ⇒ fail-closed with no bare-pid fallback.Migrations, config, rollback risks
Legacy bare-pid locks readable for one release (documented in the helper). No config changes. Rollback = revert; the lock file format is forward-written only after this code runs.
Known gaps / follow-ups
The desktop supervisor's exit-7 adoption + takeover orchestration lands in ag2-space/ag2space-cinny-desktop#282 (approved there); probe isolation activates later via sonichi/bodhi_realtime_agent#27 + the group-E pin bump.
Suggested merge order: #2684 → #2689 → bodhi #27 → desktop #282 (then the group-E pin bump activates the probe). CLA: signed (bot confirmed).
🤖 Generated with Claude Code