Skip to content

Commit 9644a00

Browse files
sahrizviHaiderclaude
authored
fix: auto-resolve question tool in non-interactive contexts (#937)
* fix: auto-resolve question tool in non-interactive contexts `Question.ask()` awaits an Effect Deferred that only resolves on a TUI click. When `altimate-code run` is invoked as a subprocess (Claude Code's Bash tool, CI, plugin host) and a skill that uses `question` fires, nobody can ever click — the deferred awaits forever and the parent eventually TaskStops the subprocess. The symptom is indistinguishable from a hang: 0% CPU, no log activity, no error. In non-interactive contexts (no TTY, or explicit env-var opt-in), auto-resolve `question` with a conservative-by-default policy and flag the auto-answer in the tool result so the calling LLM can adapt instead of treating it as a real user choice. Resolution policy (env-var controlled): - Detect non-interactive: `!process.stdin.isTTY`. Overrides: ALTIMATE_FORCE_INTERACTIVE=1 — keep the original interactive Deferred path even when isTTY is false. ALTIMATE_NON_INTERACTIVE=1 — force non-interactive even when isTTY is true (useful for tests + CI assertions). - Default in non-TTY (ALTIMATE_AUTO_ANSWER=last): pick the option whose label/description contains a safe keyword (skip, cancel, no, abort, profile only, decline, deny, stop); fall back to the last option (UX convention: safer/cancel typically sits at end). - ALTIMATE_AUTO_ANSWER=first / =skip / =<exact label>: explicit overrides for callers who want a specific behavior. Tool result prefix reflects mode — "Running in non-interactive mode (no TTY). Auto-answered with safe defaults: ..." vs the original "User has answered your questions: ..." — so the agent knows the choice was not a real user answer. Tests: 6 new bun:test cases covering safe-keyword selection, last-option fallback, each ALTIMATE_AUTO_ANSWER mode, and the prefix wording. Existing 2 legacy tests gated with ALTIMATE_FORCE_INTERACTIVE=1 so they preserve their original intent under non-TTY CI. Closes #936 * fix(question): return Unanswered in non-interactive mode The earlier non-interactive policy in this branch scanned option label text for "safe" keywords (skip/cancel/no/abort/...) and fell back to the last option. That tried to recover semantics the LLM already knew at construction time, and false-positived on substrings — "no" matched inside "Snowflake", "Annotate", "Knowledge", "Honor". The fix: don't guess. Return Unanswered for every question when no TTY is present and let the agent decide. The agent has full context — it knows what action it was about to take and why it asked. It can pick a safe path from that context or report that user input is required. Pretending a decision was made that wasn't is the worse failure mode. Changes: - Drop SAFE_KEYWORDS and the label-text scan entirely. - Default non-interactive behavior returns [] (renders as "Unanswered" via the existing format()). - Cache isNonInteractive() once at execute() entry so the result prefix can't disagree with the path that produced the answer. - Non-interactive prefix tells the agent how to proceed AND names the escape hatch (ALTIMATE_AUTO_ANSWER=first|last|<label>) so it can surface it to the user when reporting back. - Keep =first / =last / =<exact label> as explicit user opt-ins for callers who genuinely want a default. Drop =skip — it's the new default. Addresses cubic-dev-ai review feedback on #937 by removing the heuristic that needed the word-boundary fix in the first place. Tests: 9 pass / 0 fail in packages/opencode/test/tool/question.test.ts. Refs #936 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(question): address PR #937 consensus review findings Five fixes flowing from the multi-model consensus review on the prior revision. Touches the `run` entrypoint, the bash tool, the question tool, and its tests. - `run.ts:447`: `process.stdin.isTTY` was still dereferenced without a guard. The prior commit closed null-safety in `question.ts` only; this closes the same risk in the `run` entrypoint. Addresses the dev-punia-altimate review comment in full. - `run.ts` handler: gate the `ALTIMATE_NON_INTERACTIVE=1` assignment on `!args.attach`. In attach mode the agent runs on the remote server, so the local env var is a no-op and only pollutes the local process env for other tools that may consult it. - `bash.ts`: strip `ALTIMATE_NON_INTERACTIVE` from `mergedEnv` before `spawn`. Without this, child processes spawned by the bash tool (e.g. nested `altimate-code serve`) would inherit the flag from the parent `run` process and silently disable their own HTTP question- reply path — the exact regression the prior commit is fixing for other surfaces. - `question.test.ts`: regression tests for `ALTIMATE_FORCE_INTERACTIVE=1` precedence over `NON_INTERACTIVE=1` and for `ALTIMATE_NON_INTERACTIVE=0` honored as explicit opt-out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(question): close stdin stall + test isolation gap from PR #937 review Two small fixes flagged by coderabbit + cubic on the prior commit (d2a8516). - `run.ts:459`: the `!process.stdin?.isTTY` null-safety guard turned a crash (undefined stdin) into a stall (we still entered the branch and awaited `Bun.stdin.text()` on a stream that would never EOF). Use `process.stdin && !process.stdin.isTTY` so undefined stdin skips the read entirely — the only sensible behavior in an embedded runtime where there is no stdin to read from. - `question.test.ts`: the `tool.question non-interactive auto-answer` describe block sets `ALTIMATE_NON_INTERACTIVE=1` in beforeEach but didn't clear `ALTIMATE_FORCE_INTERACTIVE`. A parent-shell export could silently flip the suite to the interactive path and lose non-interactive coverage. Delete in both hooks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Haider <haider@altimate.ai> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 37072bc commit 9644a00

4 files changed

Lines changed: 381 additions & 7 deletions

File tree

packages/opencode/src/cli/cmd/run.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,30 @@ export const RunCommand = cmd({
368368
// altimate_change end
369369
},
370370
handler: async (args) => {
371+
// altimate_change start — `run` is the only entrypoint without an answer
372+
// channel for the question tool: no TUI is mounted and the in-process
373+
// Server.Default() shim below does not bind a port, so a connected IDE
374+
// or web client cannot POST /question/:requestID/reply. Without this
375+
// flag, Question.ask() awaits a Deferred forever and the parent
376+
// supervisor TaskStops the subprocess — looking exactly like a hang.
377+
// Server commands (serve/web/acp/workspace-serve) intentionally leave
378+
// this unset so their HTTP reply path stays live.
379+
//
380+
// Skipped when --attach is set: the agent runs on the remote server, so
381+
// the local env var would be a no-op and would only pollute the local
382+
// process env for other tools that may consult it.
383+
//
384+
// Child processes spawned by the bash tool would inherit this flag and
385+
// misbehave if they themselves are server-mode entrypoints; bash.ts
386+
// strips ALTIMATE_NON_INTERACTIVE from mergedEnv to prevent that leak.
387+
//
388+
// Users can opt out by exporting ALTIMATE_NON_INTERACTIVE=0 before
389+
// launching `run`.
390+
if (!args.attach && process.env["ALTIMATE_NON_INTERACTIVE"] === undefined) {
391+
process.env["ALTIMATE_NON_INTERACTIVE"] = "1"
392+
}
393+
// altimate_change end
394+
371395
let message = [...args.message, ...(args["--"] || [])]
372396
.map((arg) => (arg.includes(" ") ? `"${arg.replace(/"/g, '\\"')}"` : arg))
373397
.join(" ")
@@ -430,7 +454,14 @@ export const RunCommand = cmd({
430454
message = [extractedParts.join("\n\n"), message].filter(Boolean).join("\n\n")
431455
}
432456

433-
if (!process.stdin.isTTY) message += "\n" + (await Bun.stdin.text())
457+
// altimate_change start — null-safe stdin read. process.stdin can be
458+
// undefined in embedded/child runtimes (dev-punia review, PR #937).
459+
// Earlier revision used `!process.stdin?.isTTY`, which turned the crash
460+
// into a stall: undefined stdin satisfied the guard and we then awaited
461+
// Bun.stdin.text() on a stream that would never EOF. Skip the read
462+
// entirely when there is no stdin to read from.
463+
if (process.stdin && !process.stdin.isTTY) message += "\n" + (await Bun.stdin.text())
464+
// altimate_change end
434465

435466
if (message.trim().length === 0 && !args.command) {
436467
UI.error("You must provide a message or a command")

packages/opencode/src/tool/bash.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,15 @@ export const BashTool = Tool.define("bash", async () => {
168168

169169
// altimate_change start — prepend bundled tools dir (ALTIMATE_BIN_DIR) and user tools dirs to PATH
170170
const mergedEnv: Record<string, string | undefined> = { ...process.env, ...shellEnv.env }
171+
// altimate_change start — strip ALTIMATE_NON_INTERACTIVE from child env.
172+
// `run` sets this flag on its own process so the question tool short-
173+
// circuits, but child processes spawned by the bash tool may themselves
174+
// be server-mode entrypoints (e.g. `altimate-code serve`) that need
175+
// their HTTP question-reply path live. Without this delete, the parent
176+
// process.env spread above would silently disable that path in every
177+
// nested server invocation. See PR #937 review (Issue #3).
178+
delete mergedEnv["ALTIMATE_NON_INTERACTIVE"]
179+
// altimate_change end
171180
const sep = process.platform === "win32" ? ";" : ":"
172181
const basePath = mergedEnv.PATH ?? mergedEnv.Path ?? ""
173182
const pathEntries = new Set(basePath.split(sep).filter(Boolean))

packages/opencode/src/tool/question.ts

Lines changed: 79 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,82 @@ import { Tool } from "./tool"
33
import { Question } from "../question"
44
import DESCRIPTION from "./question.txt"
55

6+
// altimate_change start — non-interactive handling for the question tool.
7+
//
8+
// Question.ask() resolves via either a TUI click or an HTTP reply at
9+
// POST /question/:requestID/reply. Server commands (serve / web / acp /
10+
// workspace-serve) expose the HTTP path, so an IDE or web client CAN answer
11+
// even though their stdin is not a TTY. Only `altimate-code run` is
12+
// genuinely headless: it uses an in-process Server.Default() shim with no
13+
// bound port, so no client can reach the reply route and Question.ask()
14+
// awaits forever.
15+
//
16+
// Detection is therefore opt-in via env var rather than TTY-based:
17+
// `run` sets ALTIMATE_NON_INTERACTIVE=1 on startup; every other entrypoint
18+
// defaults to interactive. Earlier revisions used !process.stdin.isTTY,
19+
// which misclassified server mode and silently disabled the HTTP reply path
20+
// for IDE users (see PR #937 review).
21+
//
22+
// Policy when non-interactive: return Unanswered for every question and let
23+
// the calling agent decide. The agent knows what it was about to do and
24+
// why it asked; it can pick a safe path from context or report that input
25+
// is required. We deliberately do NOT guess based on label text — every
26+
// heuristic we tried (safe-keyword scan, last-option fallback) either
27+
// invented decisions the user didn't make or false-positive'd on labels
28+
// like "Snowflake" that happened to contain "no".
29+
//
30+
// Explicit overrides (for users who genuinely want a default and accept
31+
// the responsibility):
32+
// ALTIMATE_AUTO_ANSWER=first — always pick the first option
33+
// ALTIMATE_AUTO_ANSWER=last — always pick the last option
34+
// ALTIMATE_AUTO_ANSWER="<label>" — pick the option whose label matches
35+
//
36+
// Mode overrides:
37+
// ALTIMATE_FORCE_INTERACTIVE=1 — keep Question.ask() (e.g. tests)
38+
// ALTIMATE_NON_INTERACTIVE=1 — set by `run`; opt-in elsewhere
39+
40+
function isNonInteractive(): boolean {
41+
if (process.env["ALTIMATE_FORCE_INTERACTIVE"] === "1") return false
42+
return process.env["ALTIMATE_NON_INTERACTIVE"] === "1"
43+
}
44+
45+
function autoAnswer(questions: Question.Info[]): Question.Answer[] {
46+
const mode = process.env["ALTIMATE_AUTO_ANSWER"]?.toLowerCase()
47+
return questions.map((q) => {
48+
if (!mode) return [] // default — Unanswered, agent decides
49+
if (mode === "first") return q.options[0] ? [q.options[0].label] : []
50+
if (mode === "last") {
51+
const last = q.options[q.options.length - 1]
52+
return last ? [last.label] : []
53+
}
54+
const match = q.options.find((o) => o.label.toLowerCase() === mode)
55+
return match ? [match.label] : []
56+
})
57+
}
58+
// altimate_change end
59+
660
export const QuestionTool = Tool.define("question", {
761
description: DESCRIPTION,
862
parameters: z.object({
963
questions: z.array(Question.Info.omit({ custom: true })).describe("Questions to ask"),
1064
}),
1165
async execute(params, ctx) {
12-
const answers = await Question.ask({
13-
sessionID: ctx.sessionID,
14-
questions: params.questions,
15-
tool: ctx.callID ? { messageID: ctx.messageID, callID: ctx.callID } : undefined,
16-
})
66+
// altimate_change start — short-circuit when no human is listening.
67+
// Cache the mode once: env vars can change across the `await` below, and
68+
// we want the result prefix to describe the path the answer actually
69+
// came from, not whatever state we observe later.
70+
const nonInteractive = isNonInteractive()
71+
let answers: Question.Answer[]
72+
if (nonInteractive) {
73+
answers = autoAnswer(params.questions)
74+
} else {
75+
answers = await Question.ask({
76+
sessionID: ctx.sessionID,
77+
questions: params.questions,
78+
tool: ctx.callID ? { messageID: ctx.messageID, callID: ctx.callID } : undefined,
79+
})
80+
}
81+
// altimate_change end
1782

1883
function format(answer: Question.Answer | undefined) {
1984
if (!answer?.length) return "Unanswered"
@@ -22,9 +87,17 @@ export const QuestionTool = Tool.define("question", {
2287

2388
const formatted = params.questions.map((q, i) => `"${q.question}"="${format(answers[i])}"`).join(", ")
2489

90+
// altimate_change start — split the whole message per mode. The original
91+
// trailer "continue with the user's answers in mind" contradicts the
92+
// non-interactive branch which tells the agent no user was available.
93+
const output = nonInteractive
94+
? `Running in non-interactive mode (no answer channel available). No user was available to answer. Either pick a safe path from the context of the action you were about to take, or report that user input is required to proceed — the user can set ALTIMATE_AUTO_ANSWER=first|last|<exact option label> to pre-answer questions in this mode. Result: ${formatted}.`
95+
: `User has answered your questions: ${formatted}. You can now continue with the user's answers in mind.`
96+
// altimate_change end
97+
2598
return {
2699
title: `Asked ${params.questions.length} question${params.questions.length > 1 ? "s" : ""}`,
27-
output: `User has answered your questions: ${formatted}. You can now continue with the user's answers in mind.`,
100+
output,
28101
metadata: {
29102
answers,
30103
},

0 commit comments

Comments
 (0)