Skip to content

Commit 3fc9ead

Browse files
authored
fix(daemon): close stdin on AskUserQuestion tool_use so runs don't hang (#4277)
* fix(daemon): close stdin on AskUserQuestion tool_use so runs don't hang (#4273) Remove the dead AskUserQuestion detection branch from applyClaudeStreamJsonRunBookkeeping. The branch (left over from PR #4114 cleanup) added the tool_use id to pendingHostAnswers and returned early when the CLI emitted an AskUserQuestion tool_use event. This blocked the subsequent non-tool_use turn_end from closing stdin, leaving the run stdinOpen indefinitely — the cleanTerminalTurn condition could never be satisfied because pendingHostAnswers.size was always > 0. The run stayed `running` forever and the web Continue button stayed disabled. The AskUserQuestion tool wiring and POST /api/runs/:id/tool-result endpoint were intentionally removed in PR #4114; per AGENTS.md the stream-json stdin skeleton is retained only as generic infrastructure. The detection branch had no live counterpart to answer it and was therefore permanently blocking. Fix: remove the branch, the pendingHostAnswers type field, and the pendingHostAnswers guard in the cleanTerminalTurn condition. A non-tool_use turn_end now correctly closes stdin regardless of what tool_use events preceded it. * docs(spec): reconcile stale AskUserQuestion/tool-result references Address @PerishCode review on #4277: the 'Why OD diverged' section and the 'Interactive intact' acceptance test still described the removed host-answer contract (stream-json tool_result / POST /api/runs/:id/tool-result) as live. Rewrite both to the current <question-form> -> POST /api/chat path, citing the PR #4114 removal already stated earlier in the spec, so future maintainers cannot reintroduce the dead path. * docs(spec): correct stdin lifecycle direction in stream-json note Address @PerishCode second review on #4277: prior cleanup wrote that stdin closes on a tool_use stop reason, inverting the invariant the daemon code and regression test rely on. Per applyClaudeStreamJsonRunBookkeeping, stdin stays open across tool_use pauses and closes only on a terminal non-tool_use turn_end/usage. Reword the clause to match.
1 parent 4b3bf91 commit 3fc9ead

3 files changed

Lines changed: 56 additions & 28 deletions

File tree

apps/daemon/src/server.ts

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4574,7 +4574,6 @@ export function classifyChatRunCloseStatus(params: {
45744574

45754575
type ClaudeStreamJsonBookkeepingRun = {
45764576
stdinOpen?: boolean;
4577-
pendingHostAnswers?: Set<string>;
45784577
turnCompletedCleanly?: boolean;
45794578
child?: {
45804579
stdin?: {
@@ -4601,30 +4600,18 @@ export function applyClaudeStreamJsonRunBookkeeping(
46014600
stopReason?: unknown;
46024601
};
46034602

4604-
if (
4605-
run.stdinOpen &&
4606-
event.type === 'tool_use' &&
4607-
(event.name === 'AskUserQuestion' || event.name === 'ask_user_question') &&
4608-
typeof event.id === 'string'
4609-
) {
4610-
if (!run.pendingHostAnswers) run.pendingHostAnswers = new Set();
4611-
run.pendingHostAnswers.add(event.id);
4612-
return;
4613-
}
4614-
46154603
const cleanTerminalTurn =
4616-
((event.type === 'turn_end' &&
4604+
(event.type === 'turn_end' &&
46174605
// `stop_reason: tool_use` means the model paused to wait for tool
4618-
// execution (claude-code is about to run an internal tool, or we owe a
4619-
// host tool_result). Either way the conversation is still in flight.
4606+
// execution (claude-code is about to run an internal tool). The
4607+
// conversation is still in flight.
46204608
event.stopReason !== 'tool_use') ||
4621-
(event.type === 'usage' && event.stopReason !== 'tool_use')) &&
4622-
(!run.pendingHostAnswers || run.pendingHostAnswers.size === 0);
4609+
(event.type === 'usage' && event.stopReason !== 'tool_use');
46234610
if (!cleanTerminalTurn) return;
46244611

4625-
// Record clean completion even if stdin was already closed by the
4626-
// host-answer path. The close-status classifier reads this to ignore late
4627-
// SessionEnd hook failures after the final assistant turn completed.
4612+
// Record clean completion even if stdin was already closed. The
4613+
// close-status classifier reads this to ignore late SessionEnd hook
4614+
// failures after the final assistant turn completed.
46284615
run.turnCompletedCleanly = true;
46294616
if (run.stdinOpen) {
46304617
if (run.child?.stdin && !run.child.stdin.destroyed) {

apps/daemon/tests/chat-run-artifact-quiet-period.test.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,6 @@ describe('applyClaudeStreamJsonRunBookkeeping', () => {
551551
it('keeps stdin open when usage reports a tool_use stop reason', () => {
552552
const run = {
553553
stdinOpen: true,
554-
pendingHostAnswers: new Set<string>(),
555554
turnCompletedCleanly: false,
556555
child: {
557556
stdin: {
@@ -571,4 +570,35 @@ describe('applyClaudeStreamJsonRunBookkeeping', () => {
571570
expect(run.stdinOpen).toBe(true);
572571
expect(run.child.stdin.end).not.toHaveBeenCalled();
573572
});
573+
574+
it('closes stdin and records clean completion after an AskUserQuestion tool_use followed by end_turn (#4273)', () => {
575+
// Regression test: the dead AskUserQuestion detection branch used to add
576+
// the tool_use id to pendingHostAnswers and return early, preventing stdin
577+
// from ever closing when the subsequent turn_end arrived.
578+
const run = {
579+
stdinOpen: true,
580+
turnCompletedCleanly: false,
581+
child: {
582+
stdin: {
583+
destroyed: false,
584+
end: vi.fn(),
585+
},
586+
},
587+
};
588+
589+
applyClaudeStreamJsonRunBookkeeping(run, {
590+
type: 'tool_use',
591+
name: 'AskUserQuestion',
592+
id: 'auq_123',
593+
});
594+
595+
applyClaudeStreamJsonRunBookkeeping(run, {
596+
type: 'turn_end',
597+
stopReason: 'end_turn',
598+
});
599+
600+
expect(run.turnCompletedCleanly).toBe(true);
601+
expect(run.stdinOpen).toBe(false);
602+
expect(run.child.stdin.end).toHaveBeenCalled();
603+
});
574604
});

specs/change/20260529-claude-session-resume/spec.md

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@ Constraints:
3434
- Do not regress any existing behavior. Resume is best-effort: when a guard
3535
fails, the capability is absent, or `--resume` is rejected at runtime, the
3636
daemon falls back to today's full-transcript spawn for that turn.
37-
- Do not break the interactive `stream-json` / `AskUserQuestion` machinery
38-
(`pendingHostAnswers`, `POST /api/runs/:id/tool-result`).
37+
- Do not break the `stream-json` input skeleton (generic mid-turn stdin
38+
infrastructure). Note: the `AskUserQuestion` tool wiring and
39+
`POST /api/runs/:id/tool-result` endpoint were removed in PR #4114; the
40+
`pendingHostAnswers` dead branch was cleaned up in PR #4273.
3941
- Keep the daemon the source of truth: the stored session pointer is a cache
4042
keyed on daemon-owned conversation state, never the authoritative history.
4143
- Claude-only in v1. Other adapters keep `resumesSessionViaCli` unset and the
@@ -110,9 +112,14 @@ OD is a synchronous, interactive design-chat, not an async issue/task runner.
110112
These traits are why resume must be guarded rather than unconditional, and why
111113
it stays Claude-first and opt-out-able:
112114

113-
1. **Interactive mid-turn tools.** OD keeps `stream-json` stdin open to answer
114-
`AskUserQuestion` with a real `tool_result`. multica disables it
115-
(`--disallowedTools AskUserQuestion`). Resume must not disturb this path.
115+
1. **Interactive mid-turn clarification.** OD routes clarifying questions
116+
through the `<question-form>` markdown artifact; answers flow back as the
117+
next user message (`POST /api/chat`). The `AskUserQuestion` tool wiring and
118+
`POST /api/runs/:id/tool-result` endpoint were removed (PR #4114); the
119+
`stream-json` stdin skeleton is retained only as generic mid-turn input
120+
infrastructure, with stdin staying open across `tool_use` pauses and closing only after a terminal non-`tool_use` turn/end result. multica
121+
disables `AskUserQuestion` entirely (`--disallowedTools AskUserQuestion`).
122+
Resume must not disturb the `<question-form>` clarification path.
116123
2. **Per-turn prompt rewriting.** OD recomposes the system prompt + skills +
117124
memory + design system into the `# Instructions` block of the stdin user
118125
message every turn. Because the instructions ride the stdin message (not a
@@ -334,8 +341,12 @@ per `AGENTS.md`:
334341
- **Capability probe.** A `claude --help` without `--resume` → flag never sent
335342
AND every turn carries the full transcript (skip-transcript stays off because
336343
`resolveResumeDecision` returns null without the capability).
337-
- **Interactive intact.** An `AskUserQuestion` turn still resolves via
338-
`POST /api/runs/:id/tool-result` under resume.
344+
- **Interactive intact.** A clarifying-question turn (`<question-form>` artifact)
345+
still resolves through `POST /api/chat` (the user's answer as the next message)
346+
under resume; the `<question-form>` path is independent of session state and
347+
is unaffected by `--resume`. (The `AskUserQuestion` tool wiring and
348+
`POST /api/runs/:id/tool-result` endpoint were removed in PR #4114 and are
349+
not part of this feature.)
339350

340351
Human verification (per `AGENTS.md`, two namespaced runtimes: `main` vs branch):
341352
drive a multi-turn chat through production HTTP only; confirm continuity holds

0 commit comments

Comments
 (0)