Skip to content

fix(setup): don't auto-install plan-tune AUQ hooks in Conductor (fixes #2207) - #2208

Closed
philhie wants to merge 1 commit into
garrytan:mainfrom
philhie:fix/conductor-auq-no-autoinstall
Closed

philhie wants to merge 1 commit into
garrytan:mainfrom
philhie:fix/conductor-auq-no-autoinstall

Conversation

@philhie

@philhie philhie commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Fixes the Conductor + Claude AskUserQuestion breakage described in #2207.

Problem

In Conductor + Claude Code (Agent SDK, CLAUDE_CODE_ENTRYPOINT=sdk-ts), the plan-tune PreToolUse hook question-preference-hook participates in Conductor's canUseTool elicitation flow for AskUserQuestion. Its presence corrupts the round-trip — even when it returns defer — so AUQ comes back as [Tool result missing due to internal error] and no options render.

Native AUQ works fine in Conductor until this hook is registered. So the "flaky mcp__conductor__AskUserQuestion" that the v1.58.1.0 Conductor-prose backstop was built to work around appears to be caused by the backstop itself. Codex sessions never read ~/.claude/settings.json, so they're unaffected — which is exactly why AUQ breaks in Claude but works in Codex on the same workspace.

setup currently treats any Conductor workspace as an implicit opt-in for these hooks, so a routine /gstack-upgrade silently plants the breakage.

Change

setup: drop the Conductor implicit opt-in for the plan-tune AUQ hooks. A Conductor workspace with no explicit --plan-tune-hooks / GSTACK_PLAN_TUNE_HOOKS / plan_tune_hooks config now falls through to the normal non-interactive skip, keeping native AUQ working. Explicit opt-in still installs the hooks for anyone who wants the prose backstop.

Minimal, comment-heavy, no behavior change outside Conductor.

Verification

  • bash -n setup passes.
  • Reproduced on real Conductor hardware: with the hooks registered, AUQ returns [Tool result missing due to internal error]; after unregistering them (equivalently: this change → no auto-install), AUQ renders and answers correctly (confirmed 3×).
  • Cross-checked against Codex on the same workspace (works throughout), isolating the hook as the cause.

Notes / open questions

  • I kept the diff to the auto-install trigger only. If you'd prefer, the deeper question is whether the PostToolUse capture/fallback hooks are also unsafe on the Conductor bridge — they fire after the result so they're likely fine, but I didn't want to over-reach without a test you'd trust.
  • Left VERSION / CHANGELOG.md untouched since versioning is your release call — happy to add a CHANGELOG entry if you want it in the PR.

Environment: gstack v1.58.5.0, Claude Code 2.1.201 (sdk-ts), Conductor/macOS. Regression introduced in v1.58.1.0 (c7ae632).

The PreToolUse question-preference-hook participates in the Agent-SDK
canUseTool elicitation flow Conductor uses to render AskUserQuestion.
Its presence corrupts that round-trip even when it returns `defer`, so
AUQ comes back as [Tool result missing due to internal error] and no
options render. Native AUQ works fine in Conductor until this hook is
registered — the flakiness the backstop worked around is caused by the
backstop itself. Codex is unaffected (it doesn't read settings.json),
which is why AUQ breaks in Claude but not Codex on the same workspace.

Stop treating a Conductor workspace as an implicit opt-in; fall through
to the normal (non-interactive skip) decision so native AUQ keeps
working. Explicit opt-in via --plan-tune-hooks / GSTACK_PLAN_TUNE_HOOKS
/ plan_tune_hooks config still installs the hooks.

Refs garrytan#2207
@trunk-io

trunk-io Bot commented Jul 9, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@time-attack

time-attack commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

@philhie Please rewrite setup migration so existing Conductor users also have the problematic hook removed. The current change only prevents future installation and leaves it in the ALREADY_INSTALLED path.

manhit96 pushed a commit to manhit96/gstack that referenced this pull request Jul 22, 2026
Round-1 fixer pass against review findings on
fix/auq-hook-neutral-passthrough (4c082806).

Tests:
- empty-stdin case uses expectNoDecision (strict empty stdout; drops
  dead JSON.parse assertions that were always true when stdout was empty)
- add MCP-named AUQ + no preference → expectNoDecision (real failure
  shape for garrytan#2006/garrytan#2035/garrytan#2292/garrytan#2310)

Docs (docs/spikes/claude-code-hook-mutation.md):
- restore multi-hook precedence fact: deny > ask > allow (most
  restrictive wins), keep defer-is-not-pass-through clarification
- move "no output" out of permissionDecision values list
- mark revised date for the pass-through correction

Scope note for PR body: this PR addresses silent pass-through for the
hook itself (garrytan#2006/garrytan#2035/garrytan#2292/garrytan#2310). garrytan#2207 Conductor auto-install
belongs to garrytan#2208 and is not claimed fixed here.

Co-authored-by: dpnascimento <dpnascimento@users.noreply.github.com>
Co-authored-by: jacksondc <jacksondc@users.noreply.github.com>
@capy-ai

capy-ai Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Thank you, @philhie. Your fix is folded into v1.91.19.0 (#3025), which is now merged, and you're credited as a co-author on the release commit and in the CHANGELOG. Closing this PR because the change is on main now.

@capy-ai capy-ai Bot closed this Oct 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants