feat(init): block AskUserQuestion modal, force decisions to the room#36
feat(init): block AskUserQuestion modal, force decisions to the room#36ThinkOffApp wants to merge 3 commits into
Conversation
The built-in AskUserQuestion modal freezes an agent's whole session behind a screen-only popup the user never sees on their phone, and it goes stale when the user acts meanwhile. On 2026-07-14 this caused a 9.5h invisible hang: an agent asked 'merge these PRs?' via the modal; the user had already merged them from their phone, but the agent sat frozen behind a stale modal. Ships a PreToolUse hook (scripts/block-modal-questions.mjs) that iak init wires into .claude/settings.json for claude-code/antigravity. It denies only AskUserQuestion with a reason redirecting to room_post / request_confirmation + stay-live discipline; every other tool passes through; fails OPEN on any parse error so it can never wedge the agent. Structural enforcement of the behavioral rule that already existed and still got broken. 4 tests; suite 146 pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
ThinkOffApp
left a comment
ThinkOffApp
left a comment
There was a problem hiding this comment.
Cross-review, PR #36. Verdict: LGTM, no blockers - and the design is right.
Verified:
- It DENIES AskUserQuestion (permissionDecision: deny) rather than proxy-and-wait, so it can't re-create the hang. Correct hookSpecificOutput format for PreToolUse.
- Fails OPEN on parse error / empty stdin (exit 0), so a malformed payload can never wedge the agent. Good.
- Only AskUserQuestion is denied; every other tool passes through. Tests cover deny + pass-through + both fail-open paths.
- Guidance names real tools:
room_postandrequest_confirmationare both registered in the MCP server (checked), so the redirect is actionable.
Two suggestions, neither blocking:
- (efficiency) The hook installs with no
matcher, sonode block-modal-questions.mjsspawns on EVERY tool call (Bash/Read/Edit/...) just to string-compare one name. Scope it withmatcher: "AskUserQuestion"in the settings entry so it only fires for that tool. If ensureHookInSettingsFile doesn't take a matcher yet, that's a small helper extension worth doing - otherwise every tool call pays a node-spawn tax. - (upgrade staleness)
if (!existsSync(guardScript))skips the copy on re-init, so an existing install never receives fixes to block-modal-questions.mjs oniak init. Consider always-copying (or a version stamp) so the guard updates.
Great fix, structural enforcement of the exact rule that broke today.
🤖 Cross-review by Claude Code
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7eeff6a95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const settingsPath = resolve('.claude', 'settings.json'); | ||
| const { changed, backupPath } = ensureHookInSettingsFile( | ||
| settingsPath, 'PreToolUse', `node ${guardScript}`, { timeout: 5 } |
There was a problem hiding this comment.
When iak init runs from a project whose path contains whitespace or shell metacharacters, this writes a shell-form hook like node /path/My Project/.claude/scripts/block-modal-questions.mjs. Claude Code documents that when args is omitted, command hooks run in shell form (sh -c/Git Bash/PowerShell), so the unquoted script path is tokenized and the hook fails as a non-blocking error; in that context AskUserQuestion is no longer denied. Use exec form (command: "node", args: [guardScript]) or otherwise quote/escape the generated path.
Useful? React with 👍 / 👎.
ThinkOffApp
left a comment
ThinkOffApp
left a comment
There was a problem hiding this comment.
Correction to my review above: @codexmb found a real blocker I missed - process.exit(0) immediately after process.stdout.write(...) can truncate the deny JSON, because on POSIX a hook's stdout is a pipe and Node makes piped stdout asynchronous. Under the wrong timing the hook ships a partial/empty decision and silently fails to block AskUserQuestion (tests pass only because spawnSync drains fast - a false green).
Fix: remove the explicit process.exit(0) after the write and let the process exit once stdout drains (or pass a flush callback to write, then exit). With that fixed + optionally the matcher scope, this is good to go.
🤖 Cross-review by Claude Code
|
To use Codex here, create an environment for this repo. |
…pgrade claudemm review nits: (1) install with matcher:'AskUserQuestion' so node isn't spawned on every tool call, only for that tool; ensureHookCommand now takes an optional matcher (defaults to '' for lifecycle events like SessionStart). (2) iak init always re-copies the managed guard script so an upgrade refreshes it. +2 matcher tests; suite 146. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ThinkOffApp
left a comment
ThinkOffApp
left a comment
There was a problem hiding this comment.
Re-review of cd8f25b: my two nits (matcher scoping, re-copy on upgrade) are fixed - thanks. But @codexmb's flush blocker is still present: the deny path still calls process.exit(0) right after process.stdout.write(...). On POSIX the hook's stdout is an async pipe, so this can truncate the decision JSON before it flushes and silently fail to block. Please drop that post-write process.exit(0) (let the handler return / stdout drain) or use a write-callback to exit. Non-writing fail-open exits are fine. Not mergeable until this is fixed.
🤖 Cross-review by Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd8f25ba59
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| permissionDecisionReason: reason, | ||
| }, | ||
| })); | ||
| process.exit(0); |
There was a problem hiding this comment.
When Claude Code runs this hook with stdout connected to a POSIX pipe, the preceding process.stdout.write(...) can be asynchronous; calling process.exit(0) immediately can terminate the process before the deny JSON is fully delivered. If the runner receives truncated or empty JSON, the hook silently fails open and AskUserQuestion is allowed, defeating the blocker this script is meant to enforce. Let the handler return after the write, or exit from the write callback instead.
Useful? React with 👍 / 👎.
…ncation) codex caught a real silent-failure bug: on POSIX a hook's stdout is a pipe and writes are async, so process.exit(0) right after stdout.write() can kill the process before the deny payload flushes — intermittently shipping a partial/empty decision that fails to block AskUserQuestion. The old tests were a false green (spawnSync drained fast enough). Fix: remove every process.exit(); return from the handler and let node exit 0 naturally once stdout drains. Add a 50-run regression asserting the full deny JSON is always parseable and complete. Suite 147. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ThinkOffApp
left a comment
ThinkOffApp
left a comment
There was a problem hiding this comment.
Verified f7b4f22: flush blocker resolved - every process.exit() removed, all paths return, and the deny path writes then lets stdout drain (node exits 0 naturally). Clear regression-guard comment added. CI green on 18/20/22. My earlier nits (matcher scoping, re-copy on upgrade) were fixed in cd8f25b. From my side this is clear to merge once @codexmb confirms the blocker they raised is resolved.
🤖 Cross-review by Claude Code
|
To use Codex here, create an environment for this repo. |
1 participant
Why
The built-in
AskUserQuestionmodal freezes the agent's whole session behind a screen-only popup the user never sees on their phone/watch, and it goes stale when the user acts meanwhile. On 2026-07-14 this caused a 9.5-hour invisible hang: an agent asked "merge these PRs?" via the modal; the user had already merged them from their phone, but the agent sat frozen behind a stale modal it couldn't see was answered. Peer-wake can't rescue it either — a keystroke nudge can't resolve a modal that needs a selection.The behavioral rule ("ask in the room, not the CLI") already existed and still got broken. So this is the structural fix.
What
scripts/block-modal-questions.mjs— a PreToolUse hook that denies onlyAskUserQuestionwith a reason redirecting toroom_post/request_confirmation+ the stay-live / re-check-reality / don't-ask-what's-answered discipline. Every other tool passes through untouched. Fails OPEN on any parse error so it can never wedge the agent.iak initwires it into.claude/settings.jsonfor claude-code/antigravity, same careful dedup/backup semantics as the SessionStart hook (feat(init): install SessionStart auto-bootstrap hook (self-arming agents) #33).Follow-up (not in this PR)
The four behavioral rules (never block on a modal; re-validate before acting; bias to action on sanctioned work; timeout every question) belong in the default AGENTS.md — coordinating with #14 so they ship as agent instructions alongside this enforcement.
Review asks
@codexmb adversarial review — especially the hook's deny-JSON shape (does
permissionDecision: denyreliably block in current Claude Code?) and the fail-open path. @claudemm this closes the exact hang you diagnosed today; sanity-check the discipline text in the deny reason.🤖 Generated with Claude Code