Repository navigation
acpmux: one blank-id rule and one MDM read per operation in the full-access bound (#23322 follow-up) - #25985
Conversation
…irst reads it (red) A start, a mode switch, an ask, a pooled report and a pooled start each read MDM's off more than once. These tests run an off and an unset right after the first read; each expects the operation to hold the off. Seams sit after the first read for now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…access bound known_id is the one rule for a blank session id: a report's (StartReports::note) and the harness session a session/new answer names (StartReports::of, of_answer, for a probe and a pooled agent). A start, a pooled start, a pooled agent's report, a mode switch and an ask now each read MDM's off once, at their entry (Hub::off_latch), and pass that OffLatch to every check they make: refuse_pinned_full_access, refuse_started_in_full_access, managed_off_runs_full_access, cap_full_access, leave_full_access_if_off, replay_config, pool_take_claimed_for and pool_end_reported take it, and a pooled start's completion judges it under the pool lock. An off that comes and goes after the entry read still holds for the operation (reviews 73, 75 and 77 of #23322). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…xer once it is unset (red) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he one MDM caps hold_subsession and hold_to_starter compared policies with approve-all capped as ask under MDM's off, so an approve-all chat held to an approve-edits caller kept approve-all and ran laxer than its caller once the off was unset. They now compare and store the set policies (set_policy_of, policy_set_laxer); messaging still ranks the capped ones (review 26). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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)
web and size checks: green |
|
Owned by an agent session · session |
|
Bead |
|
Found 2 test failures on Blacksmith runners: Failures
|
…came and went (red) Review 120: a pooled entry out of both the pool and the claims while an off push comes and goes is out of the drain's reach; it may not reach a chat, nor go back to the pool, after the unset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ad it first Review 120: an entry a chat was claiming when an off came and went is out of the push's drain. Pooled::started keeps the OffLatch its start took (pool_want), so the take (pool_take_claimed_for) and the return (pool_return_claimed) count an off the entry lived through, though it is unset by then. probe_one and probe_live take their one snapshot before anything else they await. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Its tag was another test's ("faclaim"), so the two shared one home and socket when they ran
together.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e and went (red) Review 121: a claim that waits on a starting pooled agent can see an off come and go before that start reads it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review 121: a claim that waits on a starting pooled agent could see an off come and go before that start read it. pool_claim now takes its OffLatch before anything else and the claimed entry keeps it (Pooled::claimed_under); the take and the return count an off the start or the claim lived through. handle_fs_read compares its policy only to deny-all, which MDM's off never makes: it reads the stored policy, so the one read of an fs ask is handle_permission_for's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Base #25685 ranks both sides of a family message check under one MdmSnapshot, which settles review 125; this branch takes it and drops its own OffLatch version and tests. Base #26035's Pooled::claim folds into claimed_under (read at the claim's entry); off_since_claim keeps its discard at the take. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
handle_permission_for gained an argument on base; policy_laxer lost its last caller (the hold compares set policies), so the test ranks with policy_laxer_under. A pooled agent claimed through an off is now discarded at the take (review 69), so the adoption test blips at the pooled start instead, where Pooled::lived_off still carries it to the take. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review 127: reported_by_left_host (#26054) filtered a blank agent session id inline and compared a report's sessionId raw. Both now go through known_id, so the one blank-id rule covers the restart read-back too; no behaviour changes. A table-style test covers blank ids there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/40537790758 at b0ec872 failed: cmux-next generated files. |
|
Shipped in:
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
A #23322 follow-up. Reviews 70 and 74 found the same blank-id bug in two places, and reviews 73, 75 and 77 found the same off-then-unset race in three. This PR fixes each class once, so no review has to find a third copy:
full_access_bound::known_id(Option<&str>) -> Option<&str>maps an empty or all-whitespace session id toNone. Every place a harness or report session id enters the full-access bound now goes through it.Hub::off_latch), and passes thatOffLatchdown to every check it makes. The helpers that read the off themselves are gone from these paths, so a second read cannot creep back in.The decision this bounds
claude-sr2: "an MDM-managed value is an upper bound the user can't override. If MDM sets allowFullAccess=false, Full access is unavailable and the mode menu says "Managed by your organization". If MDM is unset, nothing changes. This only narrows access."
What changed
Blank ids (reviews 70 and 74)
StartReports::note(a report'ssessionId) andStartReports::of(the harness session) both callknown_id, which replaces the two inline filters.StartReports::of_answer(new)reads the harness session that asession/newanswer names. The probe (probe_one) and the pool (held_reports_full_access) both call it, so neither readsres["sessionId"]on its own any more.reported_by_left_host, from acpmux: under MDM's off, a left host is judged by what its log says it reported (#23322 b2) #26054) matches a subagent report'ssessionIdagainst the chat's storedagent_session_id. Both sides now go throughknown_id(review 127), replacing acpmux: under MDM's off, a left host is judged by what its log says it reported (#23322 b2) #26054's inline filter. Behaviour is unchanged. Test:full_access_bound::tests::a_left_hosts_read_back_takes_a_blank_session_id_for_no_session.agent_session_iditself. Making thatNonewould change what an MDM-unset chat does (forward_aswould answer "no agent session"). It is normalized only where the bound reads it.One read per operation (reviews 73, 75 and 77)
OffLatch::off()holds when MDM's off held as the operation began, or began while it ran, even if an unset followed. The newOffLatch::allows_full_access()is the settings file read under that off.ensure_child)refuse_pinned_full_access,pool_take_claimed_for, the pooled take's report check,cap_full_access(leave_full_access_if_off,managed_off_runs_full_access),cold_start(refuse_started_in_full_accesstwice,resumable_mode,replay_config's mode and options)pool_want)refuse_pinned_full_access,start_pooled(its report race and its end check), and the completion check under the pool lock, which used to take a second latchpool_reported)reported_off,pool_end_reportedpool_claim, reviews 121 and 122)Pooled::claimed_under), so a claim that waited on a starting agent counts an off that came and went before that start read itPooled::lived_off)pool_take_claimed_forandpool_return_claimedcheck it beside the chat's own. A chat that takes the entry merges it into its own snapshot (OffLatch::merge), so the report check at the take andcap_full_accesscount an off the entry lived through. An entry a chat was claiming when an off came and went was out of the push's drain, so it now never reaches a chat or goes back to the pool after the unsetprobe_one,probe_live)unattended_cwdandwait_startup(review 120)probe_bound, thesession/newcheck, the report raceforward_as)handle_permission_for)full_access_on, the policy under the permission lock, and the full-access answer's look again (lock-free, so it is fine under the permission lock)Holding a new chat to its caller
Found while listing the reads.
hold_subsessionandhold_to_startercompared policies with approve-all capped as ask, so under MDM's off an approve-all chat held to an approve-edits caller kept approve-all. Once the off was unset, it ran laxer than its caller. They now compare and store the policies each chat is set to run (set_policy_of,policy_set_laxer), which never read MDM's off. Messaging still ranks the capped policies (review 26).policy_for's cap moved intocapped_policy(policy, managed_off).policy_forstill reads the off once per call, and the ask now callscapped_policywith its own snapshot.Every remaining read of MDM's off, and why it is safe
full_access_bound.rsfull_access_allowed,approve_all_allowed,full_access_managed_offlifecycle.rsreach_or_start's teardown check (from #26034)off.off()); its own second read is gone.lifecycle.rschild_for_sincefast path;web_control.rsmode writeoff_latch), and each only decides whether to go the slow way (ensure_child) or wake the teardown task, which judges under its own read.hosts.rsadopt_agent_hosts;idle.rsend_every_managed_full_access(from #26034)full_access_bound.rsoff_latch→start_mode_state.rsfull_access_off_nowlifecycle.rsprobe_one,live_models.rsprobe_liveprobe_bound.pool/mod.rspool_return_claimedpermissions.rspolicy_for: called fromrespond_permission_annotated,permission_groups.rs,btw.rspermissions.rshandle_fs_readhandle_permission_for.handoff/ops.rs:236and:261(twopolicy_forcalls)subsession_guard.rsreceiver_checkandfamily_dispatch_checkMdmSnapshot(#25685, review 86): each check reads it once, andpolicy_laxer_underranks the sender and the receiver under it. The dispatch check takes the onefamily_holdreads under the gate. This PR only removespolicy_laxer, which reads its own snapshot, because nothing calls it any more.subsession_guard.rs:428(insettle_subsession),effective_policy(onemdm_snapshot);subsessions.rs:693forward_as, which checks it under its own snapshot.turns.rs:361(base'smdm_snapshotfor the dispatch check before the turn starts)family_gate) before it is written.start_mode.rs:522and:536(inherited_mode);start_mode.rs:711and:680(start_mode_answerandstart_level)set_mode→forward_as, which checks it under its own snapshot.server/requests.rs:902and:1870;server/remote_guard.rs:754With MDM unset (no keys, an unforced value or a forced on),
OffLatch::off()is false andallows_full_access()is the settings file's answer, the same asfull_access_allowed(). So nothing changes from base, the person's own off included: only MDM's forced off moves the off count.Tests
Red first, at 528ab87 (run https://github.com/manaflow-ai/cmux/actions/runs/40456155527). Each operation gets an off and an unset right after it first reads MDM's off (the
AFTER_OFF_COUNTseam in lib tests; the debug-buildACPMUX_TEST_OFF_BLIP_FILEseam for a pooled start). All five fail on their own assert:one_read_tests::a_start_holds_…: a profile pinning--yolostarted.one_read_tests::a_switch_holds_…: a switch into full access ran.one_read_tests::an_ask_holds_…: full access answered the ask.one_read_tests::a_pooled_report_holds_…: the report was not kept.session_pool::a_pinned_prewarm_holds_an_off_that_came_and_went_as_it_began: a--yoloprofile was prewarmed and kept ready.Red at b111915 (run https://github.com/manaflow-ai/cmux/actions/runs/40458074206):
full_access_bound::tests::a_chat_held_to_its_caller_under_an_mdm_off_is_no_laxer_once_it_is_unsetfails. An approve-all chat held to an approve-edits caller under the off ran approve-all once the off was unset.In the fix the inline seams are gone. The lib seam fires inside
off_latch, right after each entry snapshot. The pool seam stays where it was, which is after the entry snapshot.New table test
probe_off_tests::a_blank_session_id_names_no_session_at_every_entry: a blank id (""," ","\t\n") at each entry point: a report, asession/newanswer, a pooled agent's held lines, and a live probe under MDM's off whose harness answers" "and reports full access for another session. It passes on base too, because the two copies found so far were already fixed. It guards the one rule.Review 120 (BLOCK at 9140496) found three gaps:
Red at 37242d3 (run https://github.com/manaflow-ai/cmux/actions/runs/40466080679): both new
session_pooltests fail.a_full_access_pooled_agent_claimed_through_an_mdm_off_is_not_taken: a chat took the entry.…_does_not_go_back: the entry was ready again.The seam is
ACPMUX_TEST_OFF_BLIP_FILE, which now also fires while a claim holds the entry. The probe fix is ordering only: the snapshot moved before the awaits.probe_off_tests::a_pinned_probe_never_starts_after_an_off_that_came_and_went_as_it_beganstill covers a blip right after that snapshot.Review 121 (BLOCK at 81a3f15) found two more gaps:
handle_fs_readread the off before its nested ask did.Red at 9f5ad62 (run https://github.com/manaflow-ai/cmux/actions/runs/40486673732):
session_pool::a_full_access_pooled_start_a_claim_waited_on_through_an_mdm_off_is_not_takenfails, because the chat took the entry. It uses a new debug-build seam,ACPMUX_TEST_POOL_START_BLIP_FILE: the pooled start waits for a claim to wait on it, then the off comes and goes.Finding 2 gets no red test, because no behaviour changes. The read only fed a deny-all comparison, and the nested ask (a Web turn or an
askrule) never auto-approves.Review 122 (BLOCK at 59d325d) found two more gaps:
pool_claimstill took its snapshot after two awaits.Red at dc9dfda (run https://github.com/manaflow-ai/cmux/actions/runs/40489298880): two
session_pooltests fail.a_pooled_agent_claimed_through_an_mdm_off_that_reports_at_the_take_is_kept_from_the_chat: the chat took the agent.a_full_access_pooled_start_claimed_as_an_mdm_off_came_and_went_is_not_taken: the chat took the entry.The claim blip now fires at the claim's entry, before its awaits. In the second test the pooled start also waits for the claim, so neither read sees the blip without the fix.
OffLatch::mergekeeps the earlier of two reads. The off count only grows, so the earlier read saw every off the later one did. The unit testone_read_tests::merged_reads_hold_an_off_either_one_sawcovers it.a_start_holds_…now blips after every read, the start's own included, because a chat's creation reads the off for its pool claim first.Review 123 (BLOCK at 50c7c57) found two more late reads:
Red at 4f1c1fb (run https://github.com/manaflow-ai/cmux/actions/runs/40492043910): two tests fail on a new seam,
AT_OPERATION_ENTRY, which runs the off and the unset as each operation is entered.one_read_tests::a_start_holds_an_off_that_came_and_went_as_it_was_enteredone_read_tests::an_ask_holds_an_off_that_came_and_went_as_it_was_enteredReview 124 (BLOCK at 8ad8a91): the pooled start awaited a debug seam before its read.
pool_wantnow reads first, and the start seam is gone. The claim seam reads the off, makes it come and go, then waits for a pooled start to begin. So the start reads after the claim, and the claim waits on it.Proof that
a_full_access_pooled_start_claimed_as_an_mdm_off_came_and_went_is_not_takencovers the claim's read: a probe commit, 0a5b9ff, drops the claim fromPooled::lived_off. There the test fails twice (run https://github.com/manaflow-ai/cmux/actions/runs/40505425323).Merged base 3a0cd92, which brings in #23322 b1 (#26034). Two conflicts:
managed_off_runs_full_access: it keeps b1's pending-end check and takes the operation's snapshot.forward_as's stop: it keeps b1'send_if_runs_full_accessunder the switch's snapshot.b1's
reach_or_startteardown read the off a second time; it now takes the start's read.Review 125 (BLOCK at 121d211) found that
policy_laxerread the off once per side. If the off held for the first read and an unset landed before the second, an approve-all sender ranked uncapped against an approve-edits receiver that had been ranked capped. 2c91e0f fixed this red-first (probe 519ae04, run https://github.com/manaflow-ai/cmux/actions/runs/40517953587). Base #25685 then landed the same fix throughMdmSnapshot, and the merge takes base's version.Merged base b7b9749, which brings in #25685 (a3 fence), #26035 (review 69), #26054 (b2) and #24809. Conflicts:
subsession_guard.rs: kept base'sreceiver_check_under,policy_laxer_underandeffective_policy_under. This PR keepsset_policy_ofandpolicy_set_laxerfor the hold.pool/mod.rsandpool/start.rs: acpmux: a pooled agent an off passed by while it was claimed is neither restored nor handed out (#23322, review 69) #26035'sPooled::claim, read inside the claim loop, is now this PR'sclaimed_under, read once at the claim's entry before any await.off_since_claimkeeps acpmux: a pooled agent an off passed by while it was claimed is neither restored nor handed out (#23322, review 69) #26035's discard at the take and the return.lifecycle.rs: acpmux: a pooled agent an off passed by while it was claimed is neither restored nor handed out (#23322, review 69) #26035'stake_off_seamstays after the take.tests/session_pool.rs: both sides' seam env vars are kept.Because #26035 now discards an entry claimed through an off at the take, that entry never reaches adoption. The review-122 adoption test therefore blips at the pooled start instead, and
Pooled::lived_offstill carries that read to the take. It is renameda_pooled_agent_started_through_an_mdm_off_that_reports_at_the_take_is_kept_from_the_chat. Probe 5a4ab54 drops the merge at the take, and there the test fails twice (run https://github.com/manaflow-ai/cmux/actions/runs/40528232574).Review 127 (BLOCK at 69b1578) found that the merged restart read-back filtered blank ids inline and compared a report's
sessionIdraw. Both now go throughknown_id, and a blank-id test covers the read-back. No behaviour changes, so there is no red.Green at the head 56eb51d (base 9faa04b merges in cleanly; its new acpmux commits, #26198 and #25822, touch none of these files):
--all-targets -D warningsclean.own_servers_run_under_the_daemon_prefix.session_poolwithfull_access_managed: 33/33 twice.hub_integration510/510;durable_agents81/81;send_steer5/5;subsessions_delivery1/1.Earlier, at 69b1578: probe 5a4ab54 (run https://github.com/manaflow-ai/cmux/actions/runs/40528232574) fails the renamed adoption test.
Risk
risk:normal. The PR changes no behaviour with MDM unset. Under MDM's off, an operation that began while the off held, or saw one begin, now holds it for its whole run even after an unset. That only narrows access. No new lock:
OffLatch::off()reads a watch channel and takes no lock of the start-mode state, so it is safe under the pool lock and the permission lock. No new task, wait or timer.No capture: daemon-only change, no UI.
Takes back #26035: only its
Pooled::claimfield, which is folded into this PR'sclaimed_under(one read at the claim's entry, before any await).off_since_claimkeeps #26035's discard at the take and the return, and its tests pass unchanged.Review (independent Codex, gpt-6-astra xhigh): SHIP at 56eb51d
Earlier BLOCKs were fixed above. That review's one non-blocking note, naming
turns.rs:361andsettle_subsessionin the inventory, is applied here.Agent session: agent session
🤖 Generated with Claude Code