fix(send): submit multiline prompts + never nudge a session paused on a permission dialog#2357
Open
axisrow wants to merge 3 commits into
Open
fix(send): submit multiline prompts + never nudge a session paused on a permission dialog#2357axisrow wants to merge 3 commits into
axisrow wants to merge 3 commits into
Conversation
) ActivityWaitingInput conflated two opposite pauses: an agent at an empty prompt awaiting its next instruction (safe for automation to message) and an agent stopped on a pending permission/approval dialog, where a stray keystroke can answer the dialog on the user's behalf. Split them by restoring ActivityBlocked, which the original activity model had until it was removed as then-dead code (AgentWrapper#62); the sessions table CHECK constraint still allows 'blocked', so the state returns with zero migrations. - domain: ActivityBlocked const; IsSticky covers it (a permission dialog can sit for hours; the silence reaper must not kill it); NeedsInput() groups the family for consumers. - adapters: the nine permission-request producers (codex, cursor, qwen, copilot, goose, cline, kilocode, kiro, autohand) now derive blocked; claudecode splits notifications (idle_prompt stays waiting_input, permission_prompt goes blocked); droid's undiscriminated notification maps to the conservative blocked. - status: both family states render needs_input, so the dashboard vocabulary and frontend are unchanged. - lifecycle: the NeedsInput notification and waiting_input_entered/exited telemetry fire on the family boundary (an in-family escalation neither re-notifies nor splits dwell); the five reaction guards skip blocked sessions for the same reason automated nudges must. - ingress: POST /activity accepts "blocked" (enum + regenerated spec). Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA Co-authored-by: axisrow <axisrow@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…primitive (#5) * fix(send): confirm prompt submission, gate by harness, pause before Enter ao send returned 200 the moment tmux send-keys exited 0, but a large multiline prompt's trailing Enter could be absorbed by the paste — claude-code kept the text as an unsubmitted draft, user-prompt-submit never fired, and the orchestrator could not tell a hung message from an idle worker (AgentWrapper#2342). Three layers, all best-effort and none failing the send: - tmux: pause 300ms between pasting a non-empty message and the trailing Enter (mirrors conpty's ptyInputEnterDelay), fixing the race at its source for every messenger caller including the lifecycle reaction nudges. An empty message presses Enter alone — the nudge contract now documented on ports.AgentMessenger. - session_manager: after delivery, confirm acceptance by polling Activity.State and re-sending an Enter-only nudge until active or the budget (3 attempts, 2s each, 300ms polls) is spent. Decision safety: a session observed blocked (pending permission/approval dialog) stops confirmation with no nudge — checked every poll AND re-read just before each nudge, closing the poll→send race — while sticky waiting_input keeps nudging, being the unsubmitted-draft case the retry exists for. - capability gate (ports.ActivitySignaler): the Enter nudge is SAFE only when the harness emits BOTH a prompt-submit signal and a blocked signal. EmitsBlockedActivity splits the two: 12 harnesses that install a permission/notification->blocked hook run the loop (hook delegators grok/continueagent/devin advertise it via claude-code); goose, opencode and agy submit but install no permission hook, so they report false and are never nudged (an Enter could answer a decision they cannot signal); copilot stays opted out entirely (its -p mode never fires prompt hooks). The same just-in-time blocked re-read guards the automated lifecycle nudges: sendOnce re-reads session state immediately before every paste+Enter and skips a terminated or needs-input (waiting_input/blocked) session, so a permission dialog appearing after a reaction's entry guard cannot be answered by an Enter across any of the five nudge paths. Also consume the Notification types added in Claude Code CLI 2.1.198: agent_needs_input derives blocked (ambiguous need — never nudge on ambiguity), agent_completed derives idle (Stop semantics). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * feat(sessionguard): add guarded pane-write primitive Every write into a live session's pane must re-read the session immediately before pasting and refuse when the paste could answer a pending permission decision (the runtime appends Enter). Three review cycles on the send- confirmation work found the same stale-state TOCTOU at three different call-sites; this package centralizes the invariant in one tested place. Guard.Deliver carries user-initiated writes (suppressed only on blocked); Guard.Nudge carries AO-initiated writes (suppressed on the whole NeedsInput family). Both fail closed on a store error and report an explicit sent/suppressed Outcome so delivery-recording callers never stamp a suppressed write as delivered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * refactor(send): fold every pane-write path onto the sessionguard primitive lifecycle.sendOnce and session_manager.Send/confirmActive each carried their own copy of the just-in-time state re-read that keeps an automated paste+Enter out of a pending permission dialog. Three review cycles found the same stale-state TOCTOU at three call-sites; the invariant now lives only in sessionguard. - lifecycle: the Manager holds a Guard instead of a raw messenger; sendOnce delegates the pre-paste re-read to Guard.Nudge and keeps its accounted/ suppressed contract (a suppressed review nudge is still never stamped delivered). - session_manager: Send delivers through Guard.Deliver, so a message to a blocked session is now refused with ErrAwaitingDecision (API 409 SESSION_AWAITING_DECISION) instead of pasted into the dialog; missing and terminated sessions surface ErrNotFound/ErrTerminated. confirmActive's Enter-only nudge goes through the same guard, replacing the local isBlocked pre-check. - service: toAPIError maps the new sentinel to a 409 Conflict. Behavior change (deliberate): `ao send` into a session paused on a permission decision returns 409 instead of typing into the dialog. Harnesses that emit no blocked signal never store the state, so their sends are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * fix(claudecode): clear stale blocked state via PreToolUse/PostToolUse hooks Answering a permission dialog fires no hook, so the sticky `blocked` written by Notification(permission_prompt) outlived the dialog until turn-end Stop — and Send kept refusing with 409 SESSION_AWAITING_DECISION for minutes after the user had already answered (review finding, cycle 1). Install PreToolUse and PostToolUse hooks mapping to `active`: PreToolUse runs before the dialog (its verdict feeds the permission decision), so the earliest post-answer signals are the approved tool's PostToolUse and the next tool's PreToolUse. Same-state repeats are dropped daemon-side without a DB write, so the per-tool-call frequency costs one store read. grok/continue/devin inherit the fix through the delegated claude-code hooks. Known residuals (follow-up, not this change): a deny stays blocked until the next tool call or Stop; codex/qwen/cline/kiro/cursor have no post-answer signal and keep the staleness on their harnesses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * revert(claudecode): drop PreToolUse/PostToolUse activity hooks — they clear a live blocked Reverts 128350d (review cycle 2, both reviewers converged). Claude Code fires pre/post-tool-use hooks for subagent tool calls with the same AO_SESSION_ID, and ApplyActivitySignal is last-writer-wins: while one thread's permission dialog is on screen, a parallel subagent's tool signal overwrites `blocked` with `active`, bypassing exactly the protections this PR exists to add — the Send 409, the lifecycle Nudge refusal, confirmActive's waitBlocked abort — and erasing the needs_input status and its notification while a dialog is unanswered, a visibility regression against upstream. The sequential case was safe; the concurrent one is not distinguishable without correlating the approved tool's own PostToolUse (tool_use_id), which is follow-up machinery, not a one-line mapping. The cycle-1 staleness this tried to fix (blocked outliving an answered dialog until turn-end Stop) returns as a known, documented limitation: the 409 can persist for the remainder of the turn after an approval. Tracked in the follow-up issue on correlated blocked-clearing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * fix(apispec): declare the 409 Conflict response on sendSessionMessage The guarded send added SESSION_TERMINATED / SESSION_AWAITING_DECISION 409s, but the operation registry (and thus openapi.yaml and the generated TS schema) still declared only 200/400/404/500 — generated clients could not model the blocked-session outcome (review cycle 3 finding). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * chore: apply non-blocking review findings from cycles 1-3 before merge - sessionguard refuses ActivityExited alongside IsTerminated (defense-in- depth: an exited pane execs an interactive shell, so a paste would run there as commands; today unreachable — every exited writer also terminates — but the invariant no longer depends on writer discipline). - tmux: the pre-Enter pause and trailing Enter now run on a context detached from the caller (bounded by its own timeout): a cancellation landing inside the 300ms pause no longer strands a pasted-but- unsubmitted draft that a retry would double-paste. Pinned by TestSendMessageEnterSurvivesCallerCancel. - conpty: the 300ms pre-Enter pause is skipped for empty (Enter-only) nudges, aligning the cross-runtime nudge contract with tmux. - confirmActive logs the guard outcome instead of assuming "became blocked" (the session may equally have terminated or vanished); sessionguard.Outcome gains String() for logs. - TestSend_NoNudgeWhenBlockedAppearsBeforeNudge now actually exercises the JIT pre-nudge suppression branch (attemptDeadline 0 makes the read sequence deterministic; the store flips blocked on read #4 — the JIT re-read), instead of duplicating the mid-wait test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA --------- Co-authored-by: axisrow <axisrow@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…signal (#7) * feat(activity): carry hook event and tool-use identity on the activity signal The activity POST now carries {state, event, toolName?, toolUseId?}: the AO hook sub-command that produced the state, plus the native tool identity lifted generically from the hook payload (tool_name/tool_use_id are shared vocabulary across agent CLIs that emit them; adapters without them yield empty strings). Groundwork for #6: lifecycle needs to know WHICH tool call a signal concerns to clear a stale blocked state only when the specific approved tool finishes. All three fields are optional and additive: the daemon decodes the body leniently, so an old daemon ignores them, and a signal without an event keeps today's state-only semantics end to end. Ingress sanitizes and caps the new strings; overlong values are dropped, not truncated (a truncated id could never match its pre/post counterpart). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * feat(lifecycle): clear stale blocked via the approved tool's own post signal Answering a permission dialog fires no hook, so the sticky `blocked` written when the dialog appears used to outlive it until turn-end Stop — `ao send` kept returning 409 for minutes after the user had answered (#6). The naive fix (any tool signal -> active) was reverted in PR #5's review: tool hooks also fire for parallel subagents on the same session, and their traffic cleared a dialog that was still on screen. This is the correlated version. claude-code now installs the tool-use trio (PreToolUse/PostToolUse/PostToolUseFailure -> active) plus PermissionRequest (-> blocked, carrying the blocking tool_name; `ao hooks` writes nothing to stdout so it never injects a permission decision). Lifecycle keeps per-session in-memory tool-flight state and applies a precedence rule to event-tagged signals: - entering blocked snapshots the in-flight tool-use ids matching the blocking tool_name — the approved tool is one of them; - while blocked, ONLY that candidate's post (the approval executed) or a turn boundary (user-prompt-submit / stop / session-end) may change the state; subagent and sibling traffic, idle_prompt and agent_completed notifications are tracked but suppressed; - tool signals never demote sticky states in general (waiting_input included); - signals without an event — old CLIs and the 12 other adapters — keep last-writer-wins semantics untouched. Fail-safe by construction: no candidates (unknown tool, daemon restart, denied tool never posting) degrades to turn-boundary clearing — stale but safe, never a spurious clear. grok/continue/devin inherit the hooks via delegation. Closes #6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * test(lifecycle): pin the tool-flight precedence contract The precedence suite covers both directions of the invariant: the approved tool's post (success or failure) clears blocked, turn boundaries clear blocked, and everything else — subagent/sibling traffic, notification sub-types, uncorrelated posts after a restart — is suppressed. Two pins guard against regressions of past review findings: legacy signals without an event keep last-writer-wins semantics (the 12 untagged adapters and old CLIs), and the same-name-sibling residual is asserted deliberately so a behavior change there is a conscious decision. CLI tests pin the wire fields per event; ingress tests pin sanitization and the drop-not-truncate cap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * fix(lifecycle): fail closed when the blocking tool name is ambiguous The blocked-clearing snapshot keyed on tool_name only, so two same-name tools in flight when the dialog appeared both counted as the approved tool — a sibling's post then cleared a still-open permission dialog, re-opening the paste-into-dialog hole this feature closes (review cycle 1, both reviewers). The permission hook payload carries no tool_use_id (confirmed against the real CLI 2.1.198 payloads), so the strongest available correlation is a UNIQUE in-flight tool of the blocking name. Record a single blockedCandidate only when exactly one matches; two or more sets ambiguousBlock and nothing tool-shaped may clear it — the block lifts only at a turn boundary. Fail closed, never a spurious clear. Also refreshes the stale GetAgentHooks doc comment (cleanup). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * fix(lifecycle): reset ambiguousBlock on each fresh dialog The cycle-1 fix set ambiguousBlock=true but never cleared it, so a stale flag from an ambiguous dialog leaked into a later dialog in the SAME turn (no turn boundary deletes the flights entry between them): a subsequent unique dialog re-snapshotted blockedCandidate correctly but stayed ambiguous, so its legitimate approving post was suppressed and the session stuck blocked until turn end (review cycle 2). Reset ambiguousBlock=false both when re-snapshotting a fresh block and when a correlated post clears one, so each dialog starts from a clean slate. Pinned by TestToolPrecedence_SequentialDialogsResetAmbiguity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA * fix(lifecycle): release tool-flight state on reaper termination A crash/SIGKILL is reaped via ApplyRuntimeObservation, which sets IsTerminated but fired no session-end hook to drain the session's tool-flight entry — and later observations return early on cur.IsTerminated, so the entry leaked for the daemon's lifetime (up to maxInflightTools ids per reaped session), violating the intended map bound (review cycle 3). Delete m.flights[id] in the reaper's terminate mutation (under m.mu, held by mutate). The other termination paths already release it: MarkTerminated deletes directly, and a session-end hook is a turn boundary that deletes before IsTerminated is set. Pinned by TestToolPrecedence_ReaperTerminationReleasesFlight. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9VXrURRSGGsRnK7SFKMBA --------- Co-authored-by: axisrow <axisrow@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2342.
Problem
ao sendreturns HTTP 200 the momenttmux send-keysexits 0, but for a large multiline prompt claude-code's TUI can absorb the trailingEnterand leave the text as an unsubmitted draft (#2342).UserPromptSubmitnever fires, so the orchestrator can't tell the worker started and polls endlessly.Fixing that surfaced a second, sharper problem: a session paused on a permission dialog looked identical to a session waiting at an idle prompt, so an automated re-send (or any reaction nudge) could paste
Enterinto the dialog and answer a decision on the user's behalf — an unrecoverable action.What this does
Three commits, each a self-contained layer:
feat(activity): reintroduce blocked state— splits the overloadedwaiting_inputintowaiting_input(idle at the prompt, safe to message) andblocked(paused on a permission/approval decision, must not be auto-messaged). Zero DB migrations — thesessionsCHECK constraint already permitsblocked(it was removed as dead code without rebuilding the table). Adds aNeedsInput()predicate; both states map to the existingneeds_inputstatus, so the UI and telemetry are unchanged.fix(send): confirm prompt submission via a guarded pane-write primitive— afterSend, a bounded confirm loop (3×2s) re-sendsEnteruntil the session reportsactive, so a big multiline paste actually submits. All pane-writing paths (session_manager.Send/confirmActive,lifecycle.sendOnce) funnel through a newinternal/sessionguardprimitive that re-reads session state immediately before writing and refuses when the session isblocked/terminated (fail-closed).ao sendinto a blocked session now returns 409SESSION_AWAITING_DECISIONinstead of pasting into the dialog. tmux gains a 300ms paste→Enter settle (matching conpty), on a detached context so a cancel mid-pause can't strand a half-submitted draft.feat(activity): clear stale blocked via the approved tool's own post— answering a permission dialog fires no hook, so the stickyblockedwould otherwise persist until turn-end. claude-code installsPreToolUse/PostToolUse/PostToolUseFailure/PermissionRequesthooks; lifecycle keeps per-session tool-flight state and clearsblockedonly when the uniquely-identified approved tool's post arrives (or a turn boundary), correlating by the single in-flight tool of the blocking name — ambiguous same-name batches fail closed. Subagent/sibling tool traffic can never clear a live dialog. The activity signal gains optional{event, toolName, toolUseId}fields (additive; old daemons ignore them).Compatibility
No DB migrations, no changes to
ports.AgentMessenger,lifecycle.New/session_manager.Depssignatures, or the hooks wire protocol beyond three additive optional fields. Every adapter that doesn't tag its signals keeps last-writer-wins behavior.openapi.yaml/schema.tsregenerated.Testing
go build ./... && go vet ./... && go test -race ./...— all packages green; newsessionguardand tool-flight suites pin the guard and precedence invariants.blocked,ao sendreturns 409, and approving in the terminal clears it toactivebefore turn end via the approved tool's post.This work went through three rounds of adversarial review (Claude + Codex) on my fork; the same-name-sibling clear, an ambiguity-reset leak, and a reaper flight-state leak were each caught and fixed there.
🤖 Generated with Claude Code