Skip to content

test(runtime): pin interrupted-turn pending-user-input settlement - #6284

Open
Hmbown wants to merge 4 commits into
mainfrom
codex/interrupt-user-input-settle-6275
Open

Hmbown wants to merge 4 commits into
mainfrom
codex/interrupt-user-input-settle-6275

Conversation

@Hmbown

@Hmbown Hmbown commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Closes #6275

Pins the last unpinned turn-death surface for pending user inputs. Terminal and failed paths already had coverage; this adds the interrupt twin (interrupted_turn_cancels_pending_user_input_and_clears_snapshot): a TurnComplete carrying Interrupted settles the outstanding prompt with a terminal user_input.canceled event, the engine is told to cancel, and the snapshot clears.

Audit note (in the commit): the two turn-start active_turn = None failure sites run record cleanup before any engine turn exists (no registration can reference them); the terminal clear site pairs with settle_user_inputs_for_terminal_turn; monitor-failure recovery settles via its receipt path. No driven path leaks a pending prompt.

The live symptom that motivated the issue — install prompts sticking in a client UI — is the client-side card queue fed by request_plugin_install metadata (see #6274 for the source fix); the server-side pending_user_inputs map settles on every turn-death path this harness can drive.

Verification: cargo nextest run -p codewhale-tui --lib -j 5 -E 'test(interrupted_turn_cancels_pending_user_input)'Summary: 1 test run: 1 passed (mirrors the existing terminal/failed twins).

CodeWhale Bot added 4 commits September 16, 2026 00:33
…loaded skills

The <recommended_plugins> user-turn fragment fired on every turn for any
conversation that merely mentioned a plugin's domain keywords, and it could
not tell that a loaded local skill already covered the domain — a takeover
session talking about handoffs was nudged to install the handoff plugin four
times in one evening. A plugin id is now suggested at most once per engine
lifetime, and a plugin whose name (or alias) matches a skill in the
session's catalogue is never suggested. Dismissals still apply. The
suppression snapshot is taken at engine construction from the same
catalogue the system prompt indexes (workspace roots + configured skills
dir + plugin-sourced skills); a skill installed mid-session is not
suppressed until the next engine starts (known limitation, documented on
the gate).

Closes #6274

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Every surface pinned the original MCP revision and the stdio client
required an exact match, so current servers negotiating newer revisions
could not connect. The server and both clients now advertise 2025-06-18
and negotiate over the supported set [2025-06-18, 2025-03-26, 2024-11-05]:
the server echoes the client's revision when supported and answers with
the latest otherwise, the stdio client accepts any supported revision,
the main client validates the negotiated version against the set, and
streamable HTTP sends the required MCP-Protocol-Version header on every
post-initialize request.

Verification (parallel lane receipt, this checkout): codewhale-mcp 79/79,
codewhale-tui --lib mcp 425/425, cargo fmt --all -- --check clean; the
existing 2024-11-05 and 2025-06-18 fixtures pass because both are in the
accepted set.

First half of #6280 (the 2026-07-28 rmcp layer follows).

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
…t-plugin.py

--bundle <package-dir> consumes a real dsh npm bundle end to end: reads
package.json -> dsh.bundle.patch -> parses cordis.patch.yml with !!js tags
captured (never evaluated), applies the patch list with applyEntryPatches
parity (insert, group inserts, keyed overrides), lowers the three !!js
idioms (process.execPath -> node, process.env.X || 'literal', template
paths), snapshots stdio arg host directories into mcp/<server>, imports
dsh-skill-filesystem rows as customSkillDirs skills, and routes everything
unportable into CONVERSION.md as named diagnostics instead of dropping it.

Verified against our own codewhale-cu-plugin/integrations/dsh reference
bundle (previously could not round-trip): 1 MCP declaration prepared,
mcp/computer-use/server.mjs snapshotted from the installed app, tool
timeout and version carried over. All 46 converter tests pass, including
6 new bundle-mode tests (patch evaluation, !!js lowering, group inserts,
skill import, non-execution guarantees).

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
An outstanding user-input prompt must not outlive the turn that asked it,
whatever way that turn ends. Terminal and failed paths were already pinned
(terminal_turn_cancels_pending_user_input_and_clears_snapshot,
failed_turn_cancels_pending_user_input_and_clears_snapshot); this adds the
interrupt twin: a TurnComplete carrying Interrupted settles the prompt with
a terminal user_input.canceled event, the engine is told to cancel the
outstanding request, and the thread snapshot's pending_user_inputs clears.

Audit note for the remaining turn-death surfaces: the two
`active_turn = None` sites at turn-start persistence failures run
cleanup_unaccepted_turn_records before any engine turn exists, so no
user-input registration can reference them; the terminal site pairs with
settle_user_inputs_for_terminal_turn immediately before it; and the
monitor-failure recovery path settles via its own receipt handling. No
driven path leaks a pending prompt.

Closes #6275

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Copilot AI lite review requested due to automatic review settings September 16, 2026 07:42
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@claude

claude Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 2m 13s —— View job


Review of PR #6284

  • Gather context (read diff, CLAUDE.md/AGENTS.md, changed files)
  • Understand the request (code review of the PR)
  • Review correctness of new test / engine changes
  • Review provider/model/route safety implications
  • Review reuse/simplification opportunities
  • Review test coverage
  • Review security implications
  • Post final review

Summary

Scope note (most important finding): this PR's 4 commits bring in far more than the title/description say. git log origin/main..HEAD shows:

CHANGELOG.md/crates/tui/CHANGELOG.md already carry dedicated entries for #6274 and #6280 — confirming these are independent, already-scoped changes riding along on this branch rather than being part of "pin interrupted-turn settlement" (#6275). If this merges as-is, the PR will ship three unrelated features under a title that only describes a test addition, which muddies review/changelog/issue attribution. Worth splitting into separate PRs (or at minimum confirming #6274/#6280 aren't already open elsewhere and duplicating this merge) before landing.

The described change (interrupted-turn test)

Verified correct and consistent with its stated claim:

  • crates/tui/src/runtime_threads.rs:12001-12005 calls settle_user_inputs_for_terminal_turn unconditionally for every terminal turn (Completed/Interrupted/Failed) before publishing completion — so the new test is pinning existing behavior, not introducing new production logic, matching the PR body.
  • The test itself (crates/tui/src/runtime_threads/tests.rs:11359-11458) mirrors the terminal/failed twins: drives UserInputRequired, waits for pending_user_inputs to land, sends TurnComplete{status: Interrupted}, then asserts the user_input.canceled (terminal) event and an emptied snapshot. No issues found.

Other bundled commits (lighter pass, since out of stated scope)

  • MCP negotiation (crates/mcp/src/lib.rs, stdio_client.rs, crates/tui/src/mcp.rs, mcp/http.rs, mcp/streamable_http.rs, mcp_server.rs): negotiation logic looks correct — server echoes a supported requested revision or falls back to newest, stdio/TUI clients accept any revision in MCP_SUPPORTED_PROTOCOL_VERSIONS, and the MCP-Protocol-Version header is only sent once negotiated (correct default-when-absent behavior per spec). Test coverage added for both directions.
  • RecommendedPluginGate (crates/tui/src/plugins/recommend.rs, core/engine.rs): reasonable, documented known-limitation (skill snapshot taken once at Engine construction) instead of silently under-covering a mid-session skill install. StdMutex + PoisonError::into_inner recovery is the right call for a non-critical suggestion gate. Tests cover both the once-per-lifetime and skill-suppression paths.
  • convert-plugin.py --bundle: the riskiest-looking piece (parses !!js dsh tags) is handled carefully — lower_js/js_literal only resolve a small allowlist of literal patterns (process.execPath, process.env.X[ || default], template literals with only ${process.env.X} substitutions) and refuse everything else; nothing is eval'd. test_dsh_bundle_never_executes_js_and_records_unlowerable_rows proves this with a sentinel-file JS payload that never executes. Path-escape guards (../, absolute paths, is_relative_to(bundle)) are tested for both the patch file and customSkillDirs. The plain_path change (tolerating a missing ancestor via FileNotFoundError) doesn't weaken the symlink check — lstat is still used, so no TOCTOU is introduced.

No provider/model prefix-inference issues (EPIC #2608) were introduced by any of the four commits — none of this touches route/provider selection.

No correctness bugs, unwrap/panic risks, or security issues found in the reviewed diff.

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.

v0.9.14: unanswered user-input prompts stick in pending_user_inputs forever when their turn dies without the terminal settle

2 participants