Skip to content

acpmux: a prompt right behind another is accepted queued at once (fuzz g1) - #29175

Merged
cmux-bot[bot] merged 5 commits into
feat-cmux-nextfrom
fix/acpmux-g1-queued-accept
Oct 31, 2026
Merged

cmux-bot[bot] merged 5 commits into
feat-cmux-nextfrom
fix/acpmux-g1-queued-accept

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fuzz finding acpmux g1 (fuzz row g, seed 26370; STATUS 1052): a prompt that comes just behind another prompt in the same chat got no _acpmux/prompt_accepted until the first prompt's whole turn ended. With a long turn, the sender got no answer at all. In the app this reads as a lost message: the composer keeps the text, and Enter and Send do nothing. No durable answer was lost.

Cause: a prompt takes turn_lock in queue_slot, but its turn shows in session.turn only later, in start_turn, after the awaits of check_dispatch, family_dispatch_check and hold_retry_start. queue_slot read only the turn and the queue count, and its check and its count increment were two steps. So a second prompt in that window (or one that came at the same time) did not queue. It waited on turn_lock.lock().await for the first prompt's whole turn, with no acceptance.

Fix (acpmux, hub/queue.rs, hub/subsession_runs.rs, hub/session.rs):

  • The prompt that takes the lock for its turn marks it (Session::turn_claimed, set by ClaimedTurn with the lock, cleared before the lock is released). queue_slot counts that mark as a turn.
  • The check whether a prompt must wait and its increment of the queued count are one step under a new leaf lock, Session::admit (no await under it; order admit, queue, turn; only queue_slot takes it). Two prompts that come together never both find the chat free.
  • So the second prompt is journaled (its queued record) and accepted at once with queued: true, state: "queued", position: n. Its turn's records follow when its turn starts.
  • position is now the prompt's real place in the queue (insert index + 1, so 1 = next), as an inbox digest already reports its own. Before, it was the queued count, which a prompt about to start its turn is still in. The journal's queued record carries the same value.
  • An automatic continuation (after_turn) never queues, so it still waits for the lock and check_cut decides after, as before. A no_queue subagent retry still gets retry.busy.
  • The acceptance says what happened, additively: state is queued, started or steered (docs/acpmux-schema.json). An older client reads queued and steer as before; the webview (direct.ts), the Swift pane, cmux-tui-core and the CLI read only promptId and queued. A duplicate or inbox acceptance sends no state.

No client timer and no client-side guess: the daemon answers when the prompt is in its journal. The fuzzer's 2 s oracle is unchanged.

Behavior changes (a prompt sent while the earlier prompt is still starting is now a queued prompt, so it acts as any other queued prompt does):

  • If the earlier turn fails after its prompt reached the agent, the queue pause holds this prompt. Before, it ran.
  • A Stop pressed while the earlier prompt starts refuses only that prompt; the queued one runs next. Before, both were refused.
  • An agent message that comes while an agent-message turn starts can join that turn's digest. Before, it ran as its own turn (first_work_tests now expects turn_count 1).

Clients: the app's agent pane already draws a queued prompt as a queue row from the queued record (no number by design, Leo 2026-10-21), so it needs no change: the fix makes the record and the acceptance exist in this race. The CLI (acpmux send --no-wait) prints queued as before.

Testing

All runs are on the Linux fleet (cmux-ci run --class light --script scripts/ci/cmux-tui-rust-check.sh). Nothing was built on a developer machine. PR head: 31771d801ee41836bf5efc8cf9edcdbc6b008ba1. Each job names its exact commit. Two commits are scratch trees, never merged:

  • c8988bc0992c96c8974a02f9dccb15ca59ed0691 (fuzz-red/acpmux-g1-unit) = the red test commit 12e2f3f1ef6 + prompt_accept_tests.rs only.
  • a585103a3733c5eb1760c32a3ef0ec964df73721 (fuzz-red/acpmux-g1-fix-31771d80) = the PR head 31771d801ee + the 2 row-g harness commits of test(acpmux): fuzz row g, kill -9 crash fuzz of durable answers #29103 (1673bda5a4f, a585103a373; they add only tests/fuzz_crash_durable.rs and .config/nextest.toml).
What Job Exact commit Result
Red: test fuzz_g1 53abab535b78cfd4af4e6848 12e2f3f1ef61bbcd50598ad7f8fe29c12bd8492f (test commit) FAIL fuzz_g1_a_prompt_right_behind_another_is_accepted_within_two_seconds, FUZZ-G1 silent=14 of 20; control passes
Green: test fuzz_g1 26c66c5b91f5e5257a90f8d2 31771d801ee41836bf5efc8cf9edcdbc6b008ba1 (head) 3 of 3 pass
Green: test fuzz_g1_ (fuzz owner) c74bf1f299becad8bd95ece1 31771d801ee41836bf5efc8cf9edcdbc6b008ba1 (head) 3 of 3 pass
Red: test prompt_accept_tests 310423536f1472badb2e7649 c8988bc0992c96c8974a02f9dccb15ca59ed0691 (scratch: base + test) FAIL prompt_accept_tests.rs:61 "the second prompt is accepted before the first turn ends: Elapsed"
Green: test prompt_accept_tests 7c109d38cdaa7c02c1ffdb78 31771d801ee41836bf5efc8cf9edcdbc6b008ba1 (head) pass
clippy 0ec29d124c7f9cfb88f31986 31771d801ee41836bf5efc8cf9edcdbc6b008ba1 (head) green
Fuzz row g, run 1 (test fuzz_g_crash_durable) dfb5bc06bd757ea5628141ff a585103a3733c5eb1760c32a3ef0ec964df73721 (scratch: head + harness) 4 of 4 seeds, 5,955 ops, 144 kills, 0 violations (0 stalls, 0 durable losses), p99 86-93 ms, max 173 ms (feat: max 22,105 ms, 3 stalls)
Fuzz row g, repeat 4 of 5 f1a586e8613b75c0f74e863e a585103a3733c5eb1760c32a3ef0ec964df73721 (scratch: head + harness) 4 tests pass
Full test 602fe20ef88ac341935d8da7 31771d801ee41836bf5efc8cf9edcdbc6b008ba1 (head) 13,532 of 13,543 pass, 11 fail
Full test (base) 60e35145dcccb1b3dd7382e2 400b94dba51f5bf321cb257e165cb1b652404dce (base) 13 fail

10 of the 11 head fails also fail at the base (cmux-tui::setup x6, cli_audit, peer_restart, startup_load, subsessions_safety). daemon_list_guard::clients_asking_for_the_whole_list_over_and_over_cost_the_daemon_a_few_percent failed once at the head and passed on the solo rerun in the same job: a CPU-budget flake, not in the queue path.

Earlier head, superseded (for the record only): 31eeaa9f154 green fuzz_g1 job ce7a383f04fe5a957d7335cc (3 of 3).

Observed / Not run

Observed: the fleet runs above, red and green, each on its own assertion.

Not run: a live app check in a tagged build (the pane code is unchanged). The admit step has no deterministic red of its own (the unit test drives the claim path); the 20-trial socket test and fuzz row g cover it. The fuzz owner adds the remaining row-g repeats (5 of 5) as comments, each on a585103a373 (head + harness).

Changelog

Fixed: A prompt sent just behind another one in an agent chat shows as queued at once, not after the first turn ends

Proof

No capture: daemon change only; the pane's queue row is unchanged.

…pted until the first turn ends (fuzz row g, g1)
… (fuzz g1)

A prompt that took the turn lock holds the chat before start_turn shows its
turn (start_turn runs after the awaits of check_dispatch,
family_dispatch_check and hold_retry_start). queue_slot read only the turn
and the queue count, so a second prompt in that window did not queue: it
waited on turn_lock for the first prompt's whole turn and heard no
_acpmux/prompt_accepted. With a long turn the sender got no answer at all.

Now the prompt that takes the lock for its turn marks it
(Session::turn_claimed, set by ClaimedTurn with the lock and cleared before
the lock is released). queue_slot counts that mark as a turn: the second
prompt is journaled (its queued record) and accepted at once, queued, with
its position, and its turn's records follow when it starts. The count is
read before the mark, and a prompt takes the mark before it leaves the
count, so no prompt passes between them. An automatic continuation never
queues, so it still waits for the lock and check_cut decides after, as
before.

The acceptance also says what happened, additively: state queued, started
or steered (docs/acpmux-schema.json). An older client reads queued and
steer as before.

Red: fuzz_g1_a_prompt_right_behind_another_is_accepted_within_two_seconds
at the test commit. New: fuzz_g1_a_prompt_right_behind_another_is_accepted_queued_next.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 31771d8: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr29175 -f ref=fix/acpmux-g1-queued-accept -f platform=macos -f backend_mode=local

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): FIX at 31eeaa9

Read-only review of origin/feat-cmux-next...31eeaa9f154 (I did not build or run anything). The claim mechanism is sound where it applies. ClaimedTurn sets the mark before the count goes down (queue.rs:283 fresh path, queue.rs:665 queue_turn path) and clears it before the lock is released (subsession_runs.rs:208). A reader reads queued() before turn_claimed() (queue.rs:241), so after a claim, a newcomer always sees either the count or the claim. I found no lost wakeup. A prompt queued behind a claim waits in queue_turn on turn_lock.lock(). When the claimant's guard drops (turn ended, or refused before start_turn with turn still None), the waiter takes the lock as head with turn().is_none() and runs. Holders without a claim (carried_turn.rs:352, hosts.rs:1198) always have session.turn set, so newcomers queue on turn().is_some() as before. Short maintenance holders (route_switch, transfer_out, idle, policy_mode, stop_if_idle, tools) are unchanged. Excluding after_turn keeps check_cut's "never joins the queue" contract. no_queue retries still get busy. The new state field is additive: direct.ts, Swift AcpmuxRequestIds, cmux-tui-core agent_session_stream.rs/agent_message_acpmux.rs and cli/output.rs read only promptId (and queued).

Findings

  1. The g1 race is narrower but still there: the admission check and the count increment are not one step (cmux-tui/crates/acpmux/src/hub/queue.rs:236-250, then :280). Two prompts of one chat can both read queued()==0, turn_claimed()==false, turn()==None before either runs queued.fetch_add at :250. Both then take the fresh path. One takes the lock and claims it. The other blocks on turn_lock.lock().await at :280 with no prompt_accepted for the whole turn. That is the exact g1 symptom. Nothing serializes two session/prompt calls on one session before this point. Both run the same awaits first (session_log, wait_adopted, child.lock, cold_prompt_check), and when a shared wake releases them together, their check windows overlap on two runtime workers. This is the fuzz case (two clients, one chat). The comment at :237-240 ("a prompt counts in queued until it holds the claim") holds only after the fetch_add.
    Fix: make admission one critical section. Add a small admit: StdMutex<()> to Session, or use the session.queue guard and compute held from it, not from session.queue(), which locks again. Read stop_held/held/queued/turn_claimed/turn and do the fetch_add under that lock. Release it before enqueue. A second prompt then always sees the first one's count or its claim. A test seam between the waiting read and the fetch_add (like prompt_start_seam) gives a deterministic red for this. The 20-trial socket test cannot.

  2. The new test can fail on a correct daemon: the position == 1 assert (cmux-tui/crates/acpmux/tests/fuzz_g_prompt_accept_race.rs:245). Say B reads queued() after A's fetch_add (:250) but before A claims and runs fetch_sub (:283-285). B is then queued with position = 2 (fetch_add returns 1) while it is next. This happens without finding 1, and the fix proposed there makes it happen more often. The schema text "1 = next" added in docs/acpmux-schema.json:137 claims the same thing and is also wrong when prompts are held (held entries are in queued, but a new prompt runs ahead of them).
    Fix: in the test, assert state == "queued" and queued == true, and assert only position >= 1. Or compute position from the non-held entries of the queue list, so it means "1 = next". If you keep the count, remove "1 = next" from the schema. The race test (:189, 20 trials, must be 0 silent) will also stay flaky at a low rate until finding 1 is fixed.

Behavior changes a person can see (intended, I think; please confirm in the PR description)

  • A prompt sent while the previous one is still starting is now a real queued prompt. If that previous turn fails after its prompt reached the agent, pause_queue (turns.rs:1323) holds the newer prompt. Before, it ran.
  • Its Stop epoch is now sampled when it takes the turn (turns.rs if waiting { cancels = ... }). A Stop pressed during the earlier prompt's start or turn now refuses only the earlier one, and the queued one runs next. Before, both were refused (stopped_before_prompt).
  • An agent message that queues behind an agent-message turn still in its start can now be taken into that turn's digest (take_joined). Before, it ran as a turn of its own.

All three match what any other queued prompt does, so they are not defects.

@blacksmith-sh

blacksmith-sh Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Found 3 test failures on Blacksmith runners:

Failures

Test View Logs
test_a_picked_signing_label_has_a_rerun_alternative (main.RunsOnFallbackTests.test_
a_picked_signing_label_has_a_rerun_alternative)/
test_a_picked_signing_label_has_a_rerun_alternative (main.RunsOnFallbackTests.test_
a_picked_signing_label_has_a_rerun_alternative)
View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs
test_no_runs_on_chain_ends_on_the_reserved_side_label (main.RunsOnFallbackTests.tes
t_no_runs_on_chain_ends_on_the_reserved_side_label)/
test_no_runs_on_chain_ends_on_the_reserved_side_label (main.RunsOnFallbackTests.tes
t_no_runs_on_chain_ends_on_the_reserved_side_label)
View Logs

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

…s its real place

Review (FIX at 31eeaa9) found the g1 race still open: queue_slot's
check whether a prompt must wait and its increment of the queued count
were two steps, so two prompts of one chat could both find it free; one
then waited on turn_lock with no prompt_accepted. Both now run under one
leaf lock (Session::admit), held for no await.

The acceptance's position is now where the prompt went into the queue
(1: next), as an inbox digest already names its own, not the count, which
a prompt about to start its turn is still in.

A deterministic regression test (prompt_accept_tests.rs) holds the first
prompt at prompt_start_seam, after it took the lock and before its turn
shows, and requires the second prompt's acceptance, queued at position 1,
with its queued record in the journal, while the first still waits.

first_work_tests: two agent messages queued while the first work is about
to start both join its turn now (turn_count 1, was 2): each queues behind
the claim, as one behind a shown turn does.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): SHIP at 31771d8

I re-reviewed the repair delta 31eeaa9f154..31771d801ee and checked it against the whole PR diff. I only read the code; I did not build or run anything.

Both earlier findings are fixed

  1. Admission race (fixed). The "must it wait" check and queued.fetch_add now run as one step under Session::admit (hub/queue.rs:239-257). Nothing is awaited under the lock. The early return for no_queue busy drops the guard. With this, two prompts that arrive together cannot both find the chat free. The second one sees one of these:

    • the first prompt's count, before the first prompt claims the lock;
    • the claim, after the first prompt's fetch_sub (the claim is set before the fetch_sub and read after queued()).

    Lock order: admit, then queue (in session.queue()), then turn (in session.turn()). admit is used only in queue_slot. No code takes queue or turn and then admit, so this order cannot deadlock.

  2. Position (fixed). The position is now the prompt's insert index + 1, computed under the queue lock in enqueue (queue.rs:526). It is the prompt's real place in the queue: a claimant that is not in the queue does not count, and person prompts go in ahead of held prompts and agent messages. "1 = next" is now true. The queued journal record uses the same value. Its only reader is subsession_send.rs:154 / subsession_mcp.rs:236, which shows it as "queued at position N", so this is an improvement. The integration test's position == 1 assert is now deterministic. Because of the single admission step, the 20-trial race test no longer depends on timing.

I found no new defect. first_work_tests.rs now expects turn_count == 1. That is correct: both agent messages now queue behind the first work's claim and join its digest, and the delegator assert still checks what the test is for.

Notes (they do not block the merge)

  • Coverage gap: the new unit test prompt_accept_tests.rs uses prompt_start_seam. That seam tests the claim, not the admit step, so the test passes at 31eeaa9f154 too. The admission race has only the socket race test, which can miss it. A seam between the waiting read and the fetch_add would give the admit step a deterministic red. I accept this as a coverage gap, not a defect.
  • PR description: it still has RED_PENDING, GREEN_PENDING and OBSERVED_PENDING. It does not describe the admit step, the new position meaning (in the acceptance and the queued record), or the queued-prompt behavior changes from my first review. Those changes are: pause_queue holds a prompt sent during the earlier prompt's start, the Stop epoch is sampled at dequeue, and agent messages can join a claimant's digest. Please fill these in before you add ready-to-land.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fuzz row g (owner of rows f/g/h) on head 31771d8:

  1. Red tests fuzz_g1_ at 31771d8, job c74bf1f299becad8bd95ece1 (Actions run 41189208180): 3 of 3 PASS (a_prompt_right_behind_another_is_accepted_queued_next, ..._accepted_within_two_seconds, control).
  2. Full row g fuzz fuzz_g_crash_durable on scratch branch fuzz-red/acpmux-g1-fix-31771d80 (= 31771d8 + the 2 harness commits of test(acpmux): fuzz row g, kill -9 crash fuzz of durable answers #29103), head a585103, job dfb5bc06bd757ea5628141ff (Actions run 41189208045): 4 of 4 PASS.
    • seed_a: seeds=3 ops=1496 kills=36 violations=0 latency_ms p99=93 max=173
    • seed_b: seeds=3 ops=1484 kills=36 violations=0 latency_ms p99=86 max=158
    • seed_c: seeds=3 ops=1631 kills=36 violations=0 latency_ms p99=90 max=168
    • seed_d: seeds=3 ops=1344 kills=36 violations=0 latency_ms p99=86 max=164
    • In all: 5,955 ops, 144 kills, 0 violations (0 stalls, 0 durable losses), latency max 173 ms. On feat (job ea2a7c69880bc22c2df33873), seed_b gave 3 violations (g1 stalls) and max 22,105 ms.

Limit: this is one run. Catalog rule 5 says that a flake stays red until it passes 5 of 5. The g1 stall came in 6 to 8 of 20 trials on feat, so one green run of 12 seeds is strong evidence, but it is not 5 of 5. I run it 4 more times and add the result here.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Row g repeats on a585103 (= 31771d8 + row g harness), for the 5-of-5 rule:

Run Job g1 stalls Durable violations Latency max
1 dfb5bc06bd757ea5628141ff 0 0 173 ms
2 23a88f2a4e2c21acd430c517 0 0 160 ms
3 1d48ade22ee21ff95da5cc89 0 0 152 ms
4 f1a586e8613b75c0f74e863e 0 0 169 ms
5 74fc575c39c118270101ec7a 0 2 (lost-draft, seed 26369) 170 ms
  • g1 (the stall that this PR fixes): 0 stalls in 5 of 5 runs, 60 seeds, about 29,000 ops, 708 kills. On feat the same seeds gave 3 stalls and a max of 22,105 ms. g1 is fixed by this head, 5 of 5.
  • Run 5 found a different fault, g2: an acknowledged _acpmux/draft_set was lost after kill -9 (STATUS 1150). This PR does not touch the draft path (queue.rs, subsession_runs.rs, session.rs admission). I have not yet checked whether feat has g2 too, so I do not say that this PR causes it. The red test and the feat check are in progress (fuzz-findings/acpmux/g2/). This comment does not hold the PR.

@lawrencecchen lawrencecchen removed the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

g2 replays (filter fuzz_g_crash_durable_seed_a, 5 each): base 400b94d + harness (553aa64): jobs 9cef87e9, 6a1b0f18, 089600ff, 4b30f680, 17792454: lost-draft 0 of 5, g1 stalls in 4 of 5 (expected at the base). Head 31771d8 + harness: jobs accf47b9, 69185296, 4f897bac, 83b8a51e, 7ba0438d: 0 violations in 5 of 5. g2 is a rare race (1 of 11 head runs, 0 of 5 base runs), so these replays do not show where it is. By code reading (STATUS 1179) the cause is a lost update between two save_meta calls of one chat (hub/mod.rs:1867, store.rs:1411), which this PR does not touch. A deterministic fault-point red test on feat is in progress; it gives the proof.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 10:43 Inactive
@cmux-bot
cmux-bot Bot merged commit 4a2e866 into feat-cmux-next Oct 31, 2026
206 of 207 checks passed
@cmux-bot
cmux-bot Bot deleted the fix/acpmux-g1-queued-accept branch October 31, 2026 11:23

This branch was previously deployed

3 inactive deployments
artifacts — 08f22714 Deployed Oct 31, 2026 by cmux-bot[bot] via add the Windows daemon to the tree #89306
ci-routing-agent — 08f22714 Deployed Oct 31, 2026 by github-actions[bot] via cmux-next macOS placement #53987
sentry-dsyms — 08f22714 Deployed Oct 31, 2026 by cmux-bot[bot] via upload cmux-tui dSYMs to Sentry #89306
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant