fix: auto-resolve question tool in non-interactive contexts#937
Open
sahrizvi wants to merge 1 commit into
Open
fix: auto-resolve question tool in non-interactive contexts#937sahrizvi wants to merge 1 commit into
sahrizvi wants to merge 1 commit into
Conversation
`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
📝 WalkthroughWalkthroughThe question tool now supports non-interactive environments by auto-generating answers when TTY is unavailable. A safe-keyword matching policy detects "safe" options (skip, cancel, no, abort, profile only, decline, deny, stop) and selects them by default; the tool falls back to the last option when no safe keyword matches. Environment variables ChangesNon-interactive auto-answer support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/opencode/src/tool/question.ts (2)
39-57: 💤 Low valueConsider logging when ALTIMATE_AUTO_ANSWER doesn't match any known mode or option label.
When
ALTIMATE_AUTO_ANSWERis set to a value that doesn't match "skip", "first", "last", or any option label (case-insensitive), the function silently returns empty arrays (Unanswered). This is safe but could make debugging harder if a user misspells a mode or label.Consider logging a warning in this case to help users diagnose configuration issues.
📝 Optional: Add debug logging for unmatched modes
// exact label match for explicit answers, e.g. ALTIMATE_AUTO_ANSWER="Profile only" const match = q.options.find((o) => o.label.toLowerCase() === mode) + if (!match && mode !== "skip" && mode !== "first" && mode !== "last") { + console.warn(`ALTIMATE_AUTO_ANSWER="${mode}" did not match any option label; returning Unanswered`) + } return match ? [match.label] : []🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/tool/question.ts` around lines 39 - 57, The autoAnswer function silently returns empty answers when ALTIMATE_AUTO_ANSWER contains an unknown mode or when the explicit-label match fails; add a warning log to aid debugging: detect when mode is not "skip"/"first"/"last" and no q.options label matches the lowercase mode, and emit a single warning (e.g., console.warn or the module's logger) including the provided ALTIMATE_AUTO_ANSWER value and question id/label (use Question.Info properties) so users know the env value didn't match any known mode or option; place this check inside autoAnswer just before returning an empty array for unmatched cases and reference autoAnswer, ALTIMATE_AUTO_ANSWER, and q.options in the change.
86-92: 💤 Low valueConsider mode-specific prefix wording for accuracy.
The prefix says "Auto-answered with safe defaults" but this is only accurate when
ALTIMATE_AUTO_ANSWERis "last" (default). When the mode is "first" or an exact label match, the selection isn't necessarily using safe defaults—it's just picking the first option or the specified label.While the key information ("Running in non-interactive mode") is accurate and sufficient, you might consider mode-specific wording for precision:
- "last" → "Auto-answered with safe defaults"
- "first" → "Auto-selected first option"
- exact match → "Auto-selected option: {mode}"
- "skip" → "Auto-skipped (Unanswered)"
This is a minor clarity improvement; the current wording is acceptable since the primary goal is signaling non-interactive execution.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/tool/question.ts` around lines 86 - 92, The prefix message built in question.ts (variable prefix) misstates the non-interactive auto-answer behavior by always saying "Auto-answered with safe defaults"; update the logic that sets prefix (referencing isNonInteractive() and the ALTIMATE_AUTO_ANSWER mode) to choose mode-specific wording: "Auto-answered with safe defaults" for "last", "Auto-selected first option" for "first", "Auto-selected option: {mode}" for exact label matches, and "Auto-skipped (Unanswered)" for "skip", while keeping the existing "Running in non-interactive mode" text and preserving the interactive branch "User has answered your questions".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/opencode/src/tool/question.ts`:
- Around line 39-57: The autoAnswer function silently returns empty answers when
ALTIMATE_AUTO_ANSWER contains an unknown mode or when the explicit-label match
fails; add a warning log to aid debugging: detect when mode is not
"skip"/"first"/"last" and no q.options label matches the lowercase mode, and
emit a single warning (e.g., console.warn or the module's logger) including the
provided ALTIMATE_AUTO_ANSWER value and question id/label (use Question.Info
properties) so users know the env value didn't match any known mode or option;
place this check inside autoAnswer just before returning an empty array for
unmatched cases and reference autoAnswer, ALTIMATE_AUTO_ANSWER, and q.options in
the change.
- Around line 86-92: The prefix message built in question.ts (variable prefix)
misstates the non-interactive auto-answer behavior by always saying
"Auto-answered with safe defaults"; update the logic that sets prefix
(referencing isNonInteractive() and the ALTIMATE_AUTO_ANSWER mode) to choose
mode-specific wording: "Auto-answered with safe defaults" for "last",
"Auto-selected first option" for "first", "Auto-selected option: {mode}" for
exact label matches, and "Auto-skipped (Unanswered)" for "skip", while keeping
the existing "Running in non-interactive mode" text and preserving the
interactive branch "User has answered your questions".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2e6c3623-6d27-4e80-adc6-f385f03948eb
📒 Files selected for processing (2)
packages/opencode/src/tool/question.tspackages/opencode/test/tool/question.test.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/tool/question.ts">
<violation number="1" location="packages/opencode/src/tool/question.ts:47">
P2: `"no"` is matched as an arbitrary substring, causing false safe-keyword hits and incorrect auto-answers.</violation>
<violation number="2" location="packages/opencode/src/tool/question.ts:89">
P3: Mode is recomputed after awaiting, so output can claim non-interactive/interactive status inconsistent with how answers were produced.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (mode === "last") { | ||
| const safe = q.options.find((o) => { | ||
| const text = `${o.label} ${o.description}`.toLowerCase() | ||
| return SAFE_KEYWORDS.some((k) => text.includes(k)) |
There was a problem hiding this comment.
P2: "no" is matched as an arbitrary substring, causing false safe-keyword hits and incorrect auto-answers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tool/question.ts, line 47:
<comment>`"no"` is matched as an arbitrary substring, causing false safe-keyword hits and incorrect auto-answers.</comment>
<file context>
@@ -3,17 +3,78 @@ import { Tool } from "./tool"
+ if (mode === "last") {
+ const safe = q.options.find((o) => {
+ const text = `${o.label} ${o.description}`.toLowerCase()
+ return SAFE_KEYWORDS.some((k) => text.includes(k))
+ })
+ if (safe) return [safe.label]
</file context>
Suggested change
| return SAFE_KEYWORDS.some((k) => text.includes(k)) | |
| return SAFE_KEYWORDS.some((k) => (k === "no" ? /\bno\b/.test(text) : text.includes(k))) |
| // altimate_change start — flag auto-answers explicitly so the agent | ||
| // knows the user didn't actually answer and can decide whether to | ||
| // proceed with that choice or fail back gracefully. | ||
| const prefix = isNonInteractive() |
There was a problem hiding this comment.
P3: Mode is recomputed after awaiting, so output can claim non-interactive/interactive status inconsistent with how answers were produced.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tool/question.ts, line 89:
<comment>Mode is recomputed after awaiting, so output can claim non-interactive/interactive status inconsistent with how answers were produced.</comment>
<file context>
@@ -22,9 +83,17 @@ export const QuestionTool = Tool.define("question", {
+ // altimate_change start — flag auto-answers explicitly so the agent
+ // knows the user didn't actually answer and can decide whether to
+ // proceed with that choice or fail back gracefully.
+ const prefix = isNonInteractive()
+ ? `Running in non-interactive mode (no TTY). Auto-answered with safe defaults: `
+ : `User has answered your questions: `
</file context>
Contributor
❌ Tests — Failures DetectedTypeScript — 15 failure(s)
Next StepPlease address the failing cases above and re-run verification. cc @sahrizvi |
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
2 participants
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.
Closes #936
Summary
packages/opencode/src/tool/question.tscallsQuestion.ask(), which awaits an EffectDeferredthat only resolves on a TUI click. Whenaltimate-code runis invoked as a subprocess (Claude Code's Bash tool, CI,subprocess.run, plugin host) and a skill that uses thequestiontool fires, nobody can ever click — the deferred awaits forever and the parent eventuallyTaskStops the subprocess. Symptom: 0% CPU, no log activity, no error, indistinguishable from a hang.This PR short-circuits the
questiontool in non-interactive contexts with a conservative-by-default auto-answer policy.Resolution policy
!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).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 the end).ALTIMATE_AUTO_ANSWER=first— always pick first option.ALTIMATE_AUTO_ANSWER=skip— returnUnansweredfor all questions.ALTIMATE_AUTO_ANSWER="<exact label>"— exact-match an option's label (case-insensitive)."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 and can adapt strategy.Why not just always pick "cancel"?
Picking cancel/abort blindly fails open in the opposite direction: skills that ask permission to do reasonable work would always get a no, breaking the user's actual intent. The safe-keyword scan tries to match the question author's intent (these are typically "may I do destructive thing X?" prompts) without blocking legitimate flows.
Where this lives
Two reasonable choices:
packages/opencode/src/tool/question.tsshort-circuits before callingQuestion.ask().Question.ask()itself, so any caller ofQuestion.ask(not just the tool) benefits.Option 1 is what this PR ships; the scope is contained and the diff is reviewable. Option 2 is the deeper, more invasive change and may be the right long-term home. Happy to refactor if reviewers prefer.
Test plan
bun test test/tool/question.test.ts→ 8 pass, 0 fail, 12 expect() calls. Verified locally before pushing.ALTIMATE_FORCE_INTERACTIVE=1inbeforeEachso they preserve their original intent under non-TTY CI).ALTIMATE_AUTO_ANSWER=first,=skip,=<exact label>, non-interactive prefix wording.questionfrom Claude Code's Bash tool — expect completion in seconds with auto-answered output, not a hang.Diff size
The patch is roughly 70 lines of source change plus tests. Larger than #935 (the stdin-wedge guard) because the resolution policy has real branching to implement; still bounded to one tool file.
Risk
Low. Default behavior under TTY is unchanged (the new branch only fires when
!isTTYorALTIMATE_NON_INTERACTIVE=1). The non-TTY auto-answer surfaces explicitly in the tool result, so a downstream agent treating the choice as a real user click is impossible.ALTIMATE_FORCE_INTERACTIVE=1provides an escape hatch for any consumer that wants the original behavior even in non-TTY.Links
data-parityskill's PII consent question wedge)plugin-skill-experiments/03-issues-and-fixes.mdIssue chore(deps): Bump @gitlab/gitlab-ai-provider from 3.6.0 to 4.1.0 #5run.ts— same surface area, different code path)Summary by cubic
Fixes hangs in the
questiontool when no TTY is present by auto-answering conservatively and labeling the output as non-interactive, addressing #936. Interactive behavior is unchanged.!process.stdin.isTTY; overrides:ALTIMATE_FORCE_INTERACTIVE=1,ALTIMATE_NON_INTERACTIVE=1.ALTIMATE_AUTO_ANSWER=last): pick a safe-keyword option (skip/cancel/no/abort/profile only/decline/deny/stop) or fall back to the last option; overrides:first,skip, or"<exact label>".Written for commit a49442b. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Tests