Repository navigation
acpmux: under MDM's forced off, model probes and pooled agents never run in full access (#23322 a4) - #25549
Conversation
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Owned by an agent session · session |
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
…full access, and no pooled agent in full access is kept (#23322 a4; red) Round 9: an ACP probe of a pinned profile is not started under the off. Review 25: nor is a Claude live model list; a probe or list that runs when the off lands ends with it; and a pooled agent that came up in full access is not kept for a chat to take. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…robes, the pool) never run in full access (#23322 a4) The off is kept in a watch set with start-mode.json under its lock. An ACP probe and a Claude or Codex live model list check it once the folder is known and race it while they run, so an off that lands between the check and the start, or while the probe waits on its agent, ends the probe and its child. A pooled agent whose argv, mode or config options run in full access is not kept: checked at completion under the pool lock, which the push drain takes after recording the off, so an entry warming when the off lands cannot slip in after the drain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tead of going back to the pool (#23322 a4, review 27) The push drains only ready entries; one claimed at that moment and returned after it (a session/new that failed or was cancelled) would wait in the pool again. Read under the pool lock, as at completion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in full access, and no pooled agent that reported it at start is kept (#23322 a4, review 32; red) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hat its harness reports is not kept (#23322 a4, review 32) A model probe reads the mode and config options its session/new answer reports, not only its profile argv. A pooled entry folds the mode and option updates held with it (a mode from a harness that declares none included) into its check at completion and when a chat takes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ts keeps nothing under an MDM off (#23322 a4, review 41; red) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… while its session starts (#23322 a4, review 41) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Bead |
6c4b155 to
8fbc7f2
Compare
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
|
A probe or pooled agent that reports an asking mode before answering in full access, a probe whose report comes after 256 other updates, a ready pooled agent that reports full access while it waits, and an off that an unset push follows before the probe looks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd a later unset hides no off (#23322 a4, review 46) - A probe or pooled agent counts every mode or option it reported in full access while nothing watched it (StartReports), whatever it reported after or answered session/new with, in either order. Bounded by the sessions that reported, not the updates, so no report is dropped. - A ready or claimed pooled agent that reports full access while it waits ends then, outside the pool lock. - The off watch counts the times the off began: a probe ends on an off though an unset follows it before the probe looks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d counts every startup report (#23322 a4, review 74) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… off does not erase it (#23322 a4, review 75; red) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e lock (#23322 a4, review 75) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… agent keeps it from the chat after an unset (#23322 a4, review 76; red) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s through a claim, and the pool lock never nests the start-mode lock (#23322 a4, review 76) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…off that came and went, and a report at the pool handoff keeps the agent from the chat after an unset (#23322 a4, review 77; red) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the off it began under, and a pooled report made under the off holds through the handoff (#23322 a4, review 77) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ust/23322-A4c # Conflicts: # cmux-tui/crates/acpmux/src/hub/full_access_bound.rs
|
Found 7 test failures on Blacksmith runners: Failures
|
|
Review (codex gpt-6-astra xhigh, next-review): BLOCK at a2c266d
|
|
Shipped in:
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
What (part a4 of #23322)
This is part a4 of #23322 (sessions.allowFullAccess under MDM). a1 (#23322) has landed, and this PR is rebased onto
feat-cmux-nextafter it. Review 46's four findings are fixed at c5e49d6, review 70's at f2a0075, review 71's at b332687, review 73's at 28d5566, review 74's at 20e1b04, review 75's at 46577ca, review 76's at 55df8e6, and review 77's at 3553cb9 (see the table and Tests). At about 1120 changed lines (about half of it tests) it is over the usual 800: the review-round fixes share the one bound and its tests, and splitting them now would land a partial bound. It was split out of a1 to keep a1 under about 800 lines, after review 25 of a1 found the three gaps below.It builds the decision on #23322, quoted word for word:
a1 bounds every session's agent. This PR bounds the agents acpmux starts outside any session, which nothing in a1 or b ends:
watch, updated with start-mode.json under its lock (StartModeCell,Hub::full_access_off_watch). It is a count, not the current value, so an unset push that follows an off before a probe looks does not hide the off (review 46).probe_boundrefuses a profile whose argv pins full access while the off holds, and otherwise hands the probe the watch. The ACP probe (probe_one) and the Claude and Codex live model lists (probe_live) check it once their folder is known, right before the spawn, and race it while they run (full_access_forced_off, which ends once the count passes the one the probe started under). An off that lands between the check and the spawn, or while the probe waits on its agent, ends the probe and its child. A profile that does not pin full access is never watched.session/newanswer reports (FullAccessView::of_session_new). A profile that pins nothing but whose harness's own settings start it in full access keeps no models under the off, and the probe records why. The off is read once the session answers, so an off that lands during the handshake counts (review 32). It also counts every mode or option the harness reports in full access for its own session while it starts (StartReports). A mode from a harness that declares none counts too; a subagent's do not (review 41). Any such report counts, whatever the harness reported after it or answered with, in either order (review 46).StartReportskeeps the sessions that reported, at most 16, not the updates themselves, so no report is dropped; past 16, or for a report that names no session, every session counts. The pool's check uses the sameStartReports.Pooled::runs_full_access, the one predicate) is not kept under the off: it is checked when its start completes, under the pool lock. The check also counts what the harness reported for the entry's own session while the entry was held (held.rs,StartReports), including a mode from a harness that declares none. A chat taking a claimed entry runs the same check first and starts cold instead, sincecap_full_accessmay read the session before those held updates reach it (review 32). The push records the off, then takes that lock to drain, so an entry warming when the off lands is either drained or refused. A claimed entry that nobody took (asession/newthat failed or was cancelled) ends instead of going back to the pool while the off holds, checked under the same lock (review 27 of b found the restore path). A ready or claimed entry that reports full access while it waits ends at once (holding_tapcallspool_end_reported). The entry is removed under the lock and ended outside it, through the boundedpool_discard(review 46).With MDM unset nothing changes:
probe_boundhands an unpinned profile no watch, a pinned one a watch that never fires, and the pool checks only under the off.Past review findings this PR covers:
PROBES"round 9: a profile pinning full access is not probed under the off"PROBES"review 25: a Claude profile pinning full access is not started to list its models"spawn_blocking(unattended_cwd), and its child belonged to nothing revocation endsPROBES"review 25: a probe running when the off lands ends": with MDM unset, a refresh starts both pinned profiles, which hang; the off lands; the refresh answers within 8 s (not the probes' 50 s and 15 s) and both children endsession_poolunder_an_mdm_off_a_pooled_agent_that_came_up_in_full_access_is_not_keptsession/newreports a full-access mode or option completed under the offPROBES"reviews 32, 41, 46: a probe in full access as its session starts keeps nothing" (self-full: no pinned argv, starts infull-access)session_poolunder_an_mdm_off_a_pooled_agent_that_reported_full_access_at_start_is_not_kept(a harness with no modes that reportsbypassPermissionswhile its session starts, via the fake agent's newFAKE_STARTUP_MODE). The take-time check has no deterministic test of its own: an update has to arrive after completion and before the claim.session/updates its harness sent while its session started, so a harness with no modes that reportsbypassPermissionswas probed under the offPROBEScase (self-reporter: no modes,FAKE_STARTUP_MODE=bypassPermissions). Updates are kept only untilsession/newanswers.session/newanswer was replayed after it, so an earlier asking mode overwrote the answer's full access (both the probe and the pool)PROBES"reviews 32, 41, 46: a probe in full access as its session starts keeps nothing" (self-masked);session_poolunder_an_mdm_off_a_pooled_agent_that_reported_full_access_at_start_is_not_kept(famask)PROBEScase (self-busy: 300 other updates first, via the fake agent's newFAKE_STARTUP_NOISE)session_poolunder_an_mdm_off_a_ready_pooled_agent_that_reports_full_access_ends(the fake agent's newFAKE_LATE_MODE)an_off_ends_a_probe_though_an_unset_follows_ita_report_under_a_blank_session_id_counts_for_every_session;PROBESself-blank(the fake agent's newFAKE_STARTUP_SID);session_poolfablanksession/newanswer ran on past the off, and an unset after the off let it keep its modelsPROBES"review 71: a probe that reported full access ends at the off, an unset after it too" (self-holder, via the fake agent's newFAKE_NEW_HOLD)session_poolunder_an_mdm_off_a_report_held_past_the_take_keeps_the_agent_from_the_chat(debug seamACPMUX_TEST_POOL_ADOPT_REPORT_FILE)PROBES"review 73: a probe started under the off ends on a report that comes after an unset" (self-gated, via the fake agent's newFAKE_NEW_GATE)session/newanswer ran on until it answered (up to the start budget), and an unset meanwhile could keep itsession_poolunder_an_mdm_off_a_starting_pooled_agent_that_reports_full_access_ends_thensession/newanswer that named a blank session id was taken as a known session, so a report under the real id did not count (the ready pooled agent that reports full access later stayed ready)a_report_under_a_blank_session_id_counts_for_every_session;session_poolfalateblank. The probe (self-blank-new) and starting-pool (fablanknew) cases were already caught by the review 71/73 race, and they now guard it.off_latch, and the pinned probe'sprobe_bound)an_unset_racing_a_start_does_not_erase_the_off_it_began_under(a test-only seam runs the unset between the reads)session_poolunder_an_mdm_off_a_report_while_claimed_keeps_the_agent_from_the_chat_after_an_unset(debug seamACPMUX_TEST_POOL_CLAIM_REPORT_FILE). The lock order change has no test: no path takes the two the other way round today, so nothing could deadlock;OffLatch::offnow reads no state lock.probe_bound), so an off that came and went between the reads started it. And a report made under the off after the take's check counted at the handoff only while the off still heldprobe_off_tests::a_pinned_probe_never_starts_after_an_off_that_came_and_went_as_it_began;session_poolunder_an_mdm_off_a_report_at_the_handoff_keeps_the_agent_from_the_chat_after_an_unset(the adopt seam's newunsetword)Tests
Red at f74faa7 (a1 plus these tests, run 40378057503):
full_access_managedfails the threePROBEScases, andsession_poolfailsunder_an_mdm_off_a_pooled_agent_that_came_up_in_full_access_is_not_kept(the entry stayed ready).Green at 6f0e9e8 (run 40378057299): fmt and clippy are clean, lib passes 1101/1102,
full_access_managedandhub_integrationfull_access(14/14) pass, andsession_poolpasses 18/18. 3e95ab5 adds the restore check (run 40379205351: fmt and clippy clean,session_pool17/18 with only the base redpooled_sessions_end_with_the_daemon_and_after_a_crash). The lib red is base'sagent_toolsown_servers_run_under_the_daemon_prefix.Review 32 round: red at eced0d9 (3e95ab5 plus the two new tests, run 40387048972):
full_access_managedfails only "review 32: a probe whose session comes up in full access keeps nothing under the off" (self-full kept models m1 and m2), andsession_poolfails onlyunder_an_mdm_off_a_pooled_agent_that_reported_full_access_at_start_is_not_kept(the entry stayed ready).Green at f3640c5:
full_access_managedpasses,hub_integrationpasses 491/491, andsession_poolpasses 18/19 (run 40387230158). Clippy with-D warningsis clean (run 40387230042). Lib passes 1101/1102 (run 40387230230). The only reds are base'spooled_sessions_end_with_the_daemon_and_after_a_crashandown_servers_run_under_the_daemon_prefix.Review 41 round: red at e2d07f9 (f3640c5 plus the new case, run 40397367018):
full_access_managedfails only "review 41: a probe whose harness reports full access while it starts keeps nothing" (self-reporter kept models m1 and m2). Green at 6c4b155 (the head):full_access_managedpasses,hub_integrationpasses 491/491,session_poolpasses 18/19 with only the base red (run 40397483894); fmt and clippy-D warningsare clean and lib passes 1101/1102 with only the base red (run 40397484029).Review 46 round: red at ecb517d (8fbc7f2 plus the new tests, run 40405698379). All four fail for their stated reasons:
an_off_ends_a_probe_though_an_unset_follows_it: "the unset after the off hid it from the probe".full_access_managedkeeps models forself-maskedandself-busy.session_poolkeeps thefamaskentry.session_poolleaves the late reporter ready after 10 s.Green at c5e49d6 (the head):
-D warningsare clean,full_access_managedpasses,session_poolpasses 19/20, and lib passes 1143/1144.hub_integrationpasses 497/497 anddurable_agents57/57.pooled_sessions_end_with_the_daemon_and_after_a_crashandown_servers_run_under_the_daemon_prefix.Review 70 round: red at 616efff (c5e49d6 plus the new tests, run 40413774853). All three fail for the stated reason:
full_access_managedkeeps models forself-blank;session_poolkeeps thefablankentry.Green at f2a0075 (the head). A blank
sessionIdnow counts for every session, as a missing one does.-D warningsare clean,full_access_managedpasses,session_poolpasses 20/20, and lib passes 1144/1145. The one lib failure is base'sown_servers_run_under_the_daemon_prefix.hub_integrationpasses 497/497 anddurable_agents57/57.Review 71 round: red at 904dec4 (f2a0075 plus the new tests, run 40416406224). Only the two new cases fail, each for its stated reason:
full_access_managed: "the refresh waited on a probe in full access past the off";session_pool: "a chat took a pooled agent that reported full access under the off".Green at b332687 (the head):
reported_under_off,full_access_off_since).adopt_pooledreads the held lines under the tap's lock as they go live. A pooled agent whose lines report full access is stopped under the off before the chat gets it.-D warningsare clean,full_access_managedpasses,session_poolpasses 20/21, and lib passes 1144/1145.hub_integrationpasses 497/497 anddurable_agents57/57.pooled_sessions_end_with_the_daemon_and_after_a_crashandown_servers_run_under_the_daemon_prefix.Review 73 round: red at a963ba9 (b332687 plus the new tests, run 40422410169). Only the two new cases fail:
full_access_managed: "the refresh waited on a probe the off started under";session_pool: "a starting pooled agent in full access ran on under the off until it answered: 31.15s".Green at 28d5566 (the head):
OffLatchrecords, as a probe or pooled start begins, whether the off holds and the off count. The off holds for that start if it held then or began since, even if an unset followed.-D warningsare clean,full_access_managedpasses,session_poolpasses 21/22, and lib passes 1144/1145.hub_integrationpasses 497/497 anddurable_agents57/57.pooled_sessions_end_with_the_daemon_and_after_a_crashandown_servers_run_under_the_daemon_prefix.Review 74 round: red at 937c571 (28d5566 plus the new tests, run 40426275239). Two tests fail:
session_poolfalateblank: "a ready pooled agent that reported full access stayed under the MDM off".Green at 20e1b04.
StartReports::oftreats a blank harness session id as unknown.-D warningsare clean,full_access_managedpasses,session_poolpasses 21/22, and lib passes 1144/1145.hub_integrationpasses 497/497 anddurable_agents57/57.Review 75 round: red at b14df5b (run 40428166677): "an unset during the latch's read erased the off".
Green at 46577ca.
Hub::full_access_off_nowreads the count and the state together under the state lock, and both the latch andprobe_bounduse it.-D warningsare clean,full_access_managedpasses,session_poolpasses 21/22, and lib passes 1144/1146.tailnet::cli_tests::every_cli_call_sets_tailscale_be_cli, a known flake; it passes alone at this head (run 40430713553).hub_integrationpasses 497/497 anddurable_agents57/57.Review 76 round: red at c66101a (run 40431910858): "a chat took a pooled agent that reported full access under the off".
Green at 55df8e6.
reported_off, set by the holding tap throughpool_reported).pool_take_claimed_forandpool_return_claimedend such an entry even after an unset.OffLatchread before the pool lock.-D warningsare clean,full_access_managedpasses,session_poolpasses 22/23, and lib passes 1145/1146.hub_integrationpasses 497/497 anddurable_agents57/57.Review 77 round: red at 19896bc (run 40436342708). Both new tests fail:
Green at 3553cb9 (the head):
probe_boundtakes the probe's ownOffLatchinstead of reading the off again.ensure_childreads the entry'sreported_offat the handoff, whatever the off says now.-D warningsare clean,full_access_managedpasses,session_poolpasses 23/24, and lib passes 1146/1147.hub_integrationpasses 497/497 anddurable_agents57/57.Timers and loops: none new. The watch and the probe's
Notifyhave no task of their own: each probe or pooled start awaits them inside its own bounded run (50 s, or the pool's start budget) and drops them when it ends.No capture: no UI changes.
Base merge at a2c266d. a3 (#25538) landed meanwhile. The merge takes
feat-cmux-nextat 292779b and has one conflict, infull_access_bound.rs; both sides are kept: a4'sprobe_bound(latch, argv)andoff_latch, then a3'sfull_access_turned_off(changed)with its barrier.full_access_managedpasses,session_poolpasses 26/26,subsessions_delivery1/1,send_steer5/5, and lib passes 1282/1283 (only base'sown_servers_run_under_the_daemon_prefix).unnecessary_sort_byatpool/policy.rs:239(from base 3c07730; this PR does not touch that line). With only that lint allowed, clippy-D warningsis clean (run 40440480415).hub_integrationpasses 507/507 anddurable_agents61/61.Review: review 78 (Codex gpt-6-astra xhigh) found nothing blocking at 3553cb9 (SHIP). Review 79 (Codex gpt-6-astra xhigh) of the base merge found nothing blocking at a2c266d (SHIP).
Agent session: agent session
🤖 Generated with Claude Code