Skip to content

Fix Codex Agent Chat Stop interrupt request - #15837

Merged
teamleaderleo merged 3 commits into
mainfrom
fix/15779-codex-stop-turn-id
Sep 30, 2026
Merged

teamleaderleo merged 3 commits into
mainfrom
fix/15779-codex-stop-turn-id

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Fix Agent Chat Codex Stop so it interrupts the active turn with the protocol-required thread and turn IDs.

The adapter now waits for turn/started during the startup window, guards the late ID against completed or later generations, and surfaces interrupt request failures in the chat. The regression test covers the complete request shape, missing-ID handling, and generation guard.

Changelog

Fixed: Codex Agent Chat Stop now interrupts the active turn reliably.

Validation

  • bun test/codex-stop.test.ts
  • bun test/codex-route.test.ts
  • bun test test/catalog.test.ts
  • git diff --check
  • Full bun run check could not start because this fresh worktree has no installed bun type definitions or React dependencies; fleet CI should provide the full check.

Closes #15779. Related terminal Stop UI work: #15234.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes Codex Agent Chat Stop so it interrupts the active turn with the protocol-required thread and turn IDs.

  • Stop now waits for turn/started during startup and sends the interrupt with both IDs instead of sending an incomplete request.
  • A late turn ID is discarded if the generation completed or a later one started.
  • Interrupt request failures are surfaced in the chat instead of being silently swallowed.
  • Adds regression tests that drive codexAdapter.stop() against a fake server, covering the request shape, missing-ID handling, and generation guard.

Closes #15779.

Written for commit 1f1c718. Summary will update on new commits.

Review in cubic

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: dc7bb215-2655-426b-80f9-6c3718f4f26a

📥 Commits

Reviewing files that changed from the base of the PR and between 990efc1 and 1f1c718.

📒 Files selected for processing (2)
  • agent-chat/adapters/codex.ts
  • agent-chat/test/codex-stop.test.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo teamleaderleo added the dev-build Build a fleet dogfood build of each push (newest head under load) label Sep 30, 2026
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Dogfood build of 1f1c718e0d0b2b90cff08cd5eb913f287bc7a16d

cmux DEV pr-15837-1f1c718e.app

The link opens this exact commit in the cmux dev menu bar app; the page waits until the build is ready. Builds run only while this PR has the dev-build label. Under load the fleet builds the newest push each time a worker frees up, so some pushes are skipped. It signs in against production, so Cloud or backend changes still need a tagged build with a development backend.

Covers ab741838..1f1c718e (commits: 1) since the previous link, cmux DEV pr-15837-ab741838.app; if that push was skipped, its page names the newer build. To build a commit in between: cmux-ci build cmux --ref <sha> --tag bisect-<sha8> --workspace https://github.com/manaflow-ai/cmux/pull/15837.

The test only exercised the two pure helpers, so it passed even with the
pre-fix `request("turn/interrupt", { threadId })` body restored: nothing
asserted on what stop() actually sends. It now injects a fake app server and
checks the params that reach it, covering the known-turn-id path, the late
turn/started path, the newer-generation guard, the no-op cases, and a rejected
interrupt surfacing as an error event instead of being swallowed.

Verified by restoring the pre-fix stop() body: the test fails with
`Interrupt must carry both protocol IDs: {"threadId":"thread-1"}`.

codexSetSharedServerForTest is the injection seam; stop() reads the shared
connection directly, so there was no way to drive it without one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review subagent on the exact diff: the fix itself is correct. TurnInterruptParams in the codex app-server protocol (codex-rs/app-server-protocol/src/protocol/v2/turn.rs upstream) carries a required threadId and turnId, so the old request("turn/interrupt", { threadId }) could only ever be rejected, and the old .catch(() => {}) swallowed the rejection, which is why Stop looked like it did nothing. Three findings on the test, all about the test rather than the fix.

Review:

  1. The new test could not fail on the bug it was fixed for. It only called the two pure helpers and never invoked codexAdapter.stop, so it passed with the pre-fix body restored. That is a source-shape test, not a behavior test.
  2. The test file never runs in CI.
  3. console.log("codex stop assertions passed") and export {} sat ahead of the last two assertions, so the file could report success before finishing.

Fixed (1f1c718):

  • 1: the test now injects a fake app server and asserts on the params that actually reach it. Five cases: known turn id sends {threadId, turnId}, Stop pressed before turn/started sends nothing until the id arrives and then sends it complete, a late turn id does not interrupt a newer generation on the same thread, no thread or no active turn sends nothing, and a rejected interrupt surfaces as an error event instead of being swallowed. codexSetSharedServerForTest is the seam, because stop() reads the module-level shared connection directly.
    Proof it now pins the bug: with the pre-fix stop() body restored the test fails with Interrupt must carry both protocol IDs: {"threadId":"thread-1"}, and passes again with the fix in place.
  • 3: all assertions now run before the console.log.

Left:

  • 2 does not hold, so nothing changed for it. .github/workflows/ci-guards.yml:340 runs bun run test in agent-chat on the preflight group, and agent-chat/run-tests.ts discovers tests with new Glob("*.test.ts").scanSync(...) rather than a list of names, so this file was already picked up. Local full-suite run on this head: 21 scripts and 8 bun:test files ran, 0 fail, and bun x tsc --noEmit clean.

Auto-merge on squash; it lands when CI is green.

— Raindrop g2 🫧
Run: run_worker_20260930_3fc64ba6

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 30, 2026 08:39
@teamleaderleo
teamleaderleo merged commit 11216d2 into main Sep 30, 2026
57 checks passed
@teamleaderleo
teamleaderleo deleted the fix/15779-codex-stop-turn-id branch September 30, 2026 08:43
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 1f1c718e0d: every check was green at merge (13 verified; 15 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 30, 2026
0e44675 test: bound remote bootstrap subprocess waits (manaflow-ai#15608)
a192a14 fix(agent-chat): show ACP plans as structured step lists (manaflow-ai#15889)
d7f59a3 ci: place attempt 2 like attempt 1, owned minis first (manaflow-ai#15406)
d87c3be feat(agent-chat): register Cursor Agent as an ACP provider (manaflow-ai#15877)
1bd5083 fix: preserve Codex provider for workspace auto-naming (manaflow-ai#15635)
03e1245 fix(worktree-seed): budget each pattern and refuse dangling escapes (manaflow-ai#15860)
5c28fcb fix(agent-chat): stop a disposed ACP session from resurrecting its agent (manaflow-ai#15872)
11216d2 Fix Codex Agent Chat Stop interrupt request (manaflow-ai#15837)
d6b8c15 ci: watch Unix cmux-tui installer changes (manaflow-ai#15874)
0fc35d6 feat(agent-chat): register goose as an ACP provider (manaflow-ai#15871)
7f27bfc cmux ssh: security hardening from the ssh audit (manaflow-ai#15768)
8599250 fix(agent-chat): launch gemini with --experimental-acp (manaflow-ai#15868)
849376a docs: classify contributor issue difficulty (manaflow-ai#15627)
2761cc9 Keep agents with live background work out of hibernation (manaflow-ai#15278)
eae02a6 Cloud: rebake the devbox ladder with cmux-tui 02dac3c (manaflow-ai#15866)
7ed2f6b ci: bound open pull-request media revisions (manaflow-ai#15861)
13c417c Notify on SubagentStop in the notifications hook docs (manaflow-ai#15854)
5cfc6a6 fix: make cmux-tui installs immutable across release uploads (manaflow-ai#15859)
4eee1b1 fix: preserve longest Claude upstream cooldown (manaflow-ai#15856)
204b936 Pin Cloud panes to the daemon's terminal grid (manaflow-ai#15792)
fc13b7c cmux-tui: fix the replay row scroll and stale hook fence tests breaking the full gate (manaflow-ai#15240)
87d66af Add Cloud to the menu bar extra and a main-menu Cloud menu (manaflow-ai#15822)

# Conflicts:
#	.github/workflows/ci-failure-attribution.yml
#	.github/workflows/ci-macos.yml
#	.github/workflows/ci-owned-pool-rescue.yml
#	.github/workflows/ci-queue-janitor.yml
#	.github/workflows/ci.yml
#	.github/workflows/cmux-tui-artifacts.yml
#	.github/workflows/cmux-tui-build-package.yml
#	.github/workflows/cmux-tui-sdks.yml
#	.github/workflows/pr-media-prune.yml
#	.github/workflows/remote-daemon.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev-build Build a fleet dogfood build of each push (newest head under load)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agent Chat: Codex Stop omits required turnId and silently discards cancellation errors

1 participant