Repository navigation
refactor(agent-status): one Claude transcript watch per pane, read before every event - #24075
brennanb2025 wants to merge 5 commits into
Conversation
…fore every event Generalizes the idle Ctrl+C `agents_killed` watch into the machinery two planned fixes need: a list of reasons to watch (one today, a working agent child), one cursor per pane armed or repointed only from an accepted row, a catch-up before every Claude event (before the desktop's accept-or-roll-back snapshot; the relay has no such step), and a one-second tick that restates the row rebuilt from the listener's records whenever the stored row does not show it. The cancel latch now re-marks a local Claude pane's record cancelled when it holds a late main-agent hook, as it already does for Codex, so the tick has no stale record to restate once the latch window ends. One stamp function for the shell fact; the watch's second copy is gone. The idle Ctrl+C capture fixtures' transcript records carry raw `lines`.
…ill retires the child on the desktop
 |
…r its new key Moving a pane to another key carried its transcript cursor along but left no tick scheduled for the new key, so an idle Ctrl+C that killed a background agent stayed invisible until the pane's next hook.
…ents instead of re-marking the record When Orca inferred a Ctrl+C that Claude did not act on (one that only cleared a typed draft), the latch held the turn's later tool hooks and the old fix wrote the latch's cancel into the listener's record. The turn's own plain Stop then ended as Interrupted instead of Done, on every local Claude pane. The listener's record now keeps the held hooks' evidence, as before the watch. A tick row that carries no transcript fact is tagged `restatesRecords` (stripped at the store's entry, never stored, persisted or sent over the relay) and the latch holds it, so a tick still cannot reopen a held cancel after the window. Rows carrying a fact are untagged and still re-fold under the latch. A still-waiting row the tick restates now keeps the hook that raised its wait unless the fact retired that wait's own child, so a kill read just before a main-agent permission prompt no longer lets a sibling call clear the card. The watch tests that assert nothing changed now first wait for a tick to read past an appended line, so a starved runner fails them instead of passing them.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the full PR at bf54046: #22722's single-purpose Claude agents_killed poll is replaced by a per-pane transcript watch that reads before every event and on a tick.
- Shared watch machinery (
providers/claude-transcript-watch.ts,claude-transcript-watch-ticks.ts): one cursor per pane armed at the file's end, repointed only from an accepted row on a session change, reasons re-derived from records rather than stored, incremental reads with a substring pre-filter, and a tick that rebuilds the row from the listener's records and publishes it only when the stored row does not already show it. - Catch-up ordering: desktop runs it at the ingest boundary before the shell/cron accept-or-roll-back snapshot (
server-ingest-normalization.ts); the relay has no rollback step and simply runs it first (agent-hook-result-retry-scheduler.ts). - W1: the desktop re-marks the local listener record cancelled when the cancel latch holds a late main-agent working Claude hook, preserving the published verdict's clock (
server-status-update.ts,claude-roster-state.ts), so the tick has nothing stale to restate. - Single shell stamp:
claudeRunningNonAgentTaskis now the one function used by both the listener and the watch; #22722's second copy is gone. - Cleanup: old module, its test, the generic-poll anchor special case, and the Claude arm of
transcript-poll-policy.tsare deleted; fixtures changeagents_killed_recordstolines.
I traced the stated writer census (OSC repaint, cancel latch, permission hold, inferred answer, user dismissal, alias transfer, teardown, relay cache) against the watch's publish/disarm paths, and the cursor/read spec (arm-at-end, repoint, carry, prefix-skip, reason grace). I could not construct a sequence where the tick produces a wrong final stored status, republishes indefinitely, misses a retirement it should apply, or double-applies a fact. Two independent specialist passes over the watch-vs-other-writers and cursor/read lenses reached the same conclusion.
ℹ️ Verification breadth
The PR body is candid that Linux, Windows, a live SSH relay, and a real Claude were not exercised, and that the OSC-reachability claim rests only on Orca's own HTTP-hook Claude integration. The relay catch-up path (agent-hook-result-retry-scheduler.ts) is the highest-value unverified surface, since the execution host owns the watch there per ssh-execution-boundary.md. No change requested — flagging it as the remaining confidence gap rather than a defect.
Technical details
# Verification gaps carried forward
## Unverified surfaces
- `src/relay/agent-hook-result-retry-scheduler.ts` — catch-up + watch ownership on the SSH/WSL relay (no live relay run).
- `src/relay/agent-hook-server.ts` — `syncClaudeTranscriptWatch` placement after `forward`.
- Platform matrix: Linux, Windows, and the `claudeTranscriptWatchPath` Windows drive-path rule (unit-tested in isolation only).
- OSC reachability: the census counts the OSC terminal-status writer as the one writer the watch changes; the test hand-drives `ingestTerminalStatus`, and the PR notes real Claude reports by HTTP hooks.
## Pre-existing behavior retained (not introduced here)
- `readJsonlCursor`'s >1 MiB `skippedPrefix` path can silently drop a matching line; the old poll used the same reader and filter, so this is inherited rather than a regression.
- Pane-key alias transfer moves the cursor via `movePaneCacheState` but does not reschedule the tick; the watch resumes on the pane's next event (same as #22722).
## Open questions for the human
- Is the stacked base (`brennanb2025/claude-idle-ctrl-c-subagents`) the intended integration target, i.e. is this landing only after #22722?DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes (incremental, since the prior review at bf54046)
Two fix commits landed; both clean.
- The cancel latch holds the watch's restatements instead of re-marking the record (
7636014dfc). The desktop no longer callsmarkClaudeLeadTurnInterruptedon a held late main-agent hook. A tick row that carries no fact is now taggedrestatesRecords: true(ClaudeTranscriptRow), stripped inapplyNormalizedStatus, andresolveCancelVerdictLatchreturnsHOLDfor it once its early-apply checks pass. Result: after a late main-agent hook the latch refused, the listener record keeps that hook's evidence (the turn ends as its own plain done), and the tick cannot reopen the cancel. The flag is desktop-only and the relay strips it beforeapplyEvent, so nothing changes on the wire. - Waiting cards keep their hook (
7636014dfc). A tick's waiting row now keeps the card'shookEventName/toolUseIdunless the fact retired that wait's own child, so a sibling finishing no longer rewrites the card; a fact retiring another child preserves the prompt. - A moved pane's watch resumes (
093e4781c2).transferPaneAuthoritycalls the newresumeClaudeTranscriptWatch(transferredStatus)after clearing the old key, so the tick continues under the new key rather than going dormant until the next event. Covered bya pane moved to another key. - Test timing hardened:
afterTickappends a non-matching line and waits for the cursor offset to reach EOF, so the "nothing changed" assertions provably run after a tick instead of racing one. Ablations and the new tests (moved-key, plain-done settle for an armed and an unarmed pane, fact-retires-another-child) pin the behavior.
Re-checked the redesign against the writer census and the cursor/read path. The key question was whether the new latch hold could swallow a legitimate transition: it cannot, because incoming.payload.mainAgent?.state === 'done' and opensNewTurn are evaluated before the hold, so a real Stop or a new prompt still applies. On a relay the tick compares against the relay's own cache (built from the same records), so it does not restate a plain working row — which is why the desktop-only flag is sufficient. No new issues found.
Technical details
# Increment review notes (bf54046..7636014)
## Verified
- `resolveCancelVerdictLatch` order: early apply on `!previous`, not latched, agent-type mismatch, `restatesAnotherPrompt`, `incoming.mainAgent.state === 'done'`, `opensNewTurn`; only then `restatesRecords → HOLD`. So a settled main agent and a new turn escape the hold. (`server-cancel-verdict-latch.ts:100-114`)
- `applyNormalizedStatus` destructures `restatesRecords` before `canWriteLegacyStatusRow` and before the stored row, so the flag is never persisted or emitted. (`server-status-update.ts:30,159-161`)
- Relay strips `restatesRecords` in the watch publish, so `applyEvent` / `buildRelayHookEnvelope` never carry it. (`relay/agent-hook-result-retry-scheduler.ts:64`)
- Relay tick self-restatement is bounded: `observeClaudeTranscript` compares against `lastStatusByPaneKey` (the relay's cache, produced by its own records), so an unchanged record yields no row.
- `resumeClaudeTranscriptWatch` is on the inheritance chain (`StatusRetries` → `AuthorityEvidence` → `AuthorityAliases`) and runs after `clearTranscriptPoll(previousOwnerPaneKey)` with the row already moved/re-admitted under `toPaneKey`.
- Removed `markClaudeLeadTurnInterrupted` `verdict` param has no remaining callers passing it.
## Not raised
- `unpublished` is dropped on a same-event repoint, and `settleClaudeTranscriptWatch`'s one-more-read grace keys on `read !== undefined` rather than the `unpublished` attribution. Both are narrow; no reachable visible miss could be constructed (a repointing event's own accepted row is rebuilt from the already-retired records).DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…iting row keeps its hook Moving a pane now keeps ticking only a transcript cursor an accepted row armed; a row restored from disk and moved before its first hook arms nothing. A row the tick restates while it still waits keeps the hook that raised the wait. The extra check against the retired child never decided anything: a fact that retires the wait's owner has already ended the wait. Adds a relay regression test: after the desktop infers a cancel the relay never learns of, relay ticks past the latch window forward nothing and the desktop keeps the cancel through the child's next hook.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes (incremental, since the prior review at 7636014)
Three changes landed in fe3a427c8d; all clean.
- A moved pane only resumes a watch it had.
resumeClaudeTranscriptWatchnow callssynconly when a cursor already exists under the moved-to key, so a row and roster hydrated from disk after a restart (no cursor exists at startup) can no longer arm a watch. An ablation confirms the new restart test fails without the guard (expected true to be false). - The tick's waiting-row hook rule is simplified. The
attribution.toolAgentId === current.toolAgentIdexclusion is gone; it was unreachable because the only fact,applyAgentsKilled, clears the wait when it retires the wait's owner (clearClaudePendingWaitForAgent), andstateBeforeWaitis neverwaiting. Behavior is unchanged, and the store doc is updated to match. - Relay regression test. A new test proves the desktop's inferred cancel survives relay ticks while the relay's own records still say its main agent is working, with a hand-built
afterRelayTickhelper plus aRelayAgentHookServer._getStateForTests()accessor.
I verified the guard's ordering (movePaneCacheState moves the cursor before the resume check, and clearTranscriptPoll(previousOwnerPaneKey) cannot delete the moved cursor), that the removed keepsWait branch was dead, and that the new tests can fail. All 14 and 17 tests in the two touched test files pass.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Closing this intermediate shared-reader branch and rebuilding the reader with its current consumers. The permission and background-work fixes remain tracked; this closure would not claim those gaps are fixed. |
1 participant
ELI5
Claude Code does some things without telling Orca through a hook. Examples: you answer a permission dialog with No or press Esc, or you kill a background task from
/tasks. The only record of those is a line Claude writes to its own session log (its "transcript"). #22722 already reads that log for one reason: an idle Ctrl+C that kills background subagents.Two planned fixes need to read the same log for other reasons. Without a shared reader, each fix would bolt its own copy onto #22722's single-purpose reader, in the same file.
This PR turns #22722's reader into that shared reader, once: one "transcript watch" per Claude pane. Nothing you can see changes. The one difference is theoretical: if something in a Claude pane printed Orca's OSC 9999 terminal status, the watch would restore the subagents that repaint drops within a second instead of at the next hook. Nothing Orca ships emits OSC 9999 for Claude, and none of 32 real Claude terminal captures contains one.
The plan (this PR is step 1 of 3):
queue-operationlog row (fixes "done while a shell still runs" after Ctrl+C, and a/taskskill that leaves the pane on Monitoring).A follow-up, F1, then makes Claude's own interrupt notice end every cancelled turn on the host that runs Claude (STA-2413), which retires the temporary pieces listed under Why.
What Changed
Before (#22722): a single-purpose watch. It armed only while the pane had a working background agent, read the log only on a one-second timer, and published a row only when it found an
agents_killedline. It had its own copy of the "shell running" stamp and a Claude-only special case in the generic transcript-poll code.After: one watch per pane, built as a small list of reasons to watch.
claude-transcript-watch.ts). Each entry says when it holds (re-derived from the listener's records, never stored), which raw lines it needs (a substring test), and what fact it applies. This PR ships fix(agent-status): reflect Claude's idle-prompt Ctrl+C killing background subagents #22722's reason only:agent-child-working, which ends when the pane has no working agent child (SubagentStop, anagents_killedline, the Stop inventory, SessionStart, pane teardown). R1b and R2b each add one entry./clear,/resume, a fork), the cursor repoints at that file's end and keeps its reasons, because a reason can outlive the session (a background shell survives/clear, per a 2.1.285 capture). It disarms only after a read that applied no fact with no reason left, and at pane teardown or server/relay stop.stat, then only the new bytes, capped at 1 MiB per read and 256 KiB per line. Lines are pre-filtered by the watched reasons' substrings beforeJSON.parse, so Claude's ~55 KB per-turn snapshot rows never reach the parser. Panes with no reason pay nothing.SubagentStopand the retired child's id, as fix(agent-status): reflect Claude's idle-prompt Ctrl+C killing background subagents #22722 published).claudeRunningNonAgentTaskinclaude-roster-state.ts) for the shell-running flag, used by the listener and the watch. fix(agent-status): reflect Claude's idle-prompt Ctrl+C killing background subagents #22722's second copy is deleted. R1b re-points this one function at its task record.transcript-poll-policy.tsandserver-state.tsare back to main's shape.kind: 'transcript'record now carrieslines: the raw scrubbed lines Claude appended since the previous transcript record, in file order. The four idle Ctrl+C fixtures are converted; only field names changed, plus one record whose line repeated an earlier scan and now lists none. No new capture lines were committed.Why
Deliberate choices, and what is temporary:
claude -pin the pane moves the watch to its own log (see Review). Follow-up: model a nested session in a pane.Linked Issue
Closes none. Part of STA-3344 and STA-3259.
Visual Proof
N/A. This PR is meant to change nothing you can see; the writer census and its tests below show that, not screenshots.
Testing
Typecheck
tc-gate pnpm tc:nodeandtc:cliatfe3a427c8d6: pass.oxlintandoxfmton every touched file: pass. Vitest on 53 named files (everysrc/main/agent-hooks/server*.test.ts, the relay's hook-server tests, everyagent-hook-listener*test): 665 passed.New tests, built from the committed 2.1.280 idle Ctrl+C capture (hook bodies plus its one
agents_killedline), with hand-built fields or timing where each test says:PostCompact) is published by the next tick.claude -pdoes (pinned)./clear: the cursor repoints and keeps its reasons, then disarms only after the next read.Unit tests (hand-built): arming, never resetting on a reason change, the session-change repoint, reading for a reason that ended just before the read, the pre-filter never parsing a 56 KB line, the restatement marker, the waiting-row attribution, a dismissed row, and a row another host or agent owns.
Ablations (one mechanism reverted, named file run, then restored):
Regression guards (#22722's tests; they pass):
server-claude-idle-ctrl-c-captures.test.ts,server-relayed-claude-cancel.test.ts, the renamed unit test, andagent-hook-listener-relay-dependency.test.ts(now lists the two watch modules the relay imports).Platforms: macOS unit and server tests only. Not run: Linux, Windows, a live SSH relay, or a real Claude. No agent CLI was launched.
Review
Census of every writer of the stored status row, and whether it can disagree with the watch:
restates the children an OSC repaint dropped….Merging with other open PRs: #23521 and #23947 change the relay's hook ingest. Whichever lands second must route relay hooks through the retry scheduler's
normalizeHook, notnormalizeHookPayloaddirectly, or the relay catch-up silently disappears.catches up on the relay before it normalizes the next hookfails if it is missed.Other things a reviewer should know:
claude -p(temporary): it inherits the pane key and its rows are accepted, so the cursor follows the nested session's file and outeragents_killedlines written meanwhile are missed. The killed child lingers until the next inventory: the safe direction, and fix(agent-status): reflect Claude's idle-prompt Ctrl+C killing background subagents #22722's behaviour today.remote-wire-compatibility.md): no field, opcode, enum value or capability changes; the restatement marker never leaves the host. The relay now restates a row in more situations, always in shapes it already publishes.ssh-execution-boundary.md).statper event and per second plus the appended bytes. Nothing is persisted.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)