Repository navigation
feat(acpmux): opt-in memory and artifact sync, with per-tick token refresh (carries #27254) - #27815
Conversation
Agent-Session: deeplink/agent?id=aid-artifacts-sync
The daemon's sync loop took one access token at start and sent it on every 30 s tick, so every sync failed once that token expired. Each tick now asks the mesh client for the token it holds (refreshed near expiry) and for a new one after the server answers 401. Also makes the branch compile and pass clippy -D warnings: reqwest's `query` needs a feature this workspace leaves off (encode with Url::parse_with_params), sync_once borrowed self.roots across a &mut self call, the clients built a reqwest Client before any rustls provider was installed (it panics), an unread SaveResponse.commit field, and a collapsible if. Test: cargo test -p acpmux --lib artifacts_sync (7 passed, incl. the new every_tick_takes_a_current_token_and_a_refusal_asks_for_a_new_one); cargo clippy -p acpmux --all-targets -- -D warnings clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c430a6f6a8435f14bd29
|
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 Shipped in:
|
|
Found 3 test failures on Blacksmith runners: Failures
|
pull_once mapped each listed remote path onto a local root by joining the server's text unchecked, so a listed `memory/codex/../../.ssh/authorized_keys` wrote outside the memory folders. A remote path now maps only when every component is a normal name (no `..`, `.`, root, empty, backslash or NUL). The sync client had no timeouts, so a server that stopped answering held the daemon's sync loop forever. It now has a 15 s connect and a 60 s per-read timeout (a large blob that keeps moving is never cut off). Test: cargo test -p acpmux --lib artifacts_sync, 8 passed, incl. the new a_remote_path_never_maps_outside_its_root; clippy -D warnings clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c430a6f6a8435f14bd29
Review findings on the sync (gpt-6.1-sol, at c68b4b2): - A pull wrote remote bytes over the local file unconditionally, so an edit the server hadn't taken (its upload failed, or it was made during the pass) was replaced with the stale remote copy. A pull now writes only over what this machine last synced, or a missing file; anything else is kept for the next pass to send and counted as a conflict. - After a save conflict, a failed remote read was taken as empty, so an unchanged memory merged to an empty file, truncated it, and saved that. The read error now fails the pass. Conflict writes also go only over the bytes this pass read. - A symlink below a root led a pull outside it (`shared -> ~/work`), and the upload walk followed symlinks out of the root. Neither follows a symlink now. - Shutdown now also ends a pass in flight, so a server that trickles bytes never holds the task past the daemon. - A pass re-sent every file on every tick; it now sends only what changed since the last sync. Red/green: cargo test -p acpmux --lib artifacts_sync with the new tests on c68b4b2 fails pull_keeps_a_local_edit_the_server_has_not_taken (left "remote"), a_failed_conflict_read_never_empties_a_memory (left "") and symlinks_below_a_root_are_never_followed; with this commit 11 passed. clippy -D warnings clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c430a6f6a8435f14bd29
Second review round (gpt-6.1-sol, at e73f197): - The conflict path wrote the merged Markdown and recorded it as the synced base before the retry save. When the retry failed, the pull that followed took the merge as unchanged and replaced it with the older remote copy, and the upload skip never sent it again. Now nothing counts as synced and no local file changes until the retry is saved. - A binary conflict uploaded our copy beside the original but left the original's base alone, so the next pass sent ours over the remote edit the copy was meant to keep. The original now takes the remote bytes, locally and as its base; a file gone remotely is simply sent; an identical one is synced. remote_file tells a missing file (None) from an empty one. - The secret filter missed `password: x`, `apiKey: ...`, `access_token: ...`. It now also runs the daemon's secret scanner (gitleaks' rules and cmux's) and a credential-key assignment pattern. Red/green: with the new tests on e73f197, a_failed_conflict_retry_keeps_the_local_edit_unsynced (base was the merge), a_binary_conflict_keeps_both_versions (base stayed "old") and secret_paths_and_content_are_skipped (`password: hunter22` uploaded) fail; with this commit 13 passed. clippy -D warnings clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c430a6f6a8435f14bd29
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. |
…ows a root symlink Third review round (gpt-6.1-sol, at 348da70): - A file deleted locally was restored by the next pull, since a missing file counted as writable. Sync now sends a local delete as a `delete` change, and a pull skips a file whose commit is the one this machine last synced. A file deleted here but changed remotely since comes back: the remote edit wins. - The synced state (head, and each path's digest and commit) is kept in ACPMUX_HOME/artifacts-sync-state.json, written whole and only when it changed. A restart then neither re-sends every file nor restores a file deleted while the daemon was down. File contents stay in memory only. - A root that is itself a symlink (`~/.codex/memories -> /elsewhere`) is not synced, in either direction. - `password: x` and other short credential values were uploaded; the credential-key pattern now takes a value of any length. - A signal stop does not notify hub.shutdown, so the sync task outlived the daemon; the daemon now keeps its handle and aborts it with startup's. Also, found while writing the tests: - Save changes were serialized snake_case (`content_base64`), but the platform's ArtifactChange is camelCase, so every upload reached the server without its content. The body now matches, checked by a contract test. - The pull read only the first page of the listing (50 items) and treated a saved artifact's folder item as a file. It now reads every page at one head (a head that moves fails the pass) and skips folder items. - A pull re-read every file on every tick; an unchanged commit is not read. Red/green: on 348da70 these new tests fail or do not compile against its API: a_local_delete_is_sent_and_never_restored, a_remote_edit_wins_over_a_local_delete, the_listing_reads_every_page_at_one_head, synced_state_survives_a_restart, a_root_that_is_a_symlink_is_not_synced, a_save_request_uses_the_platform_field_names, and the `password: x` case. With this commit 19 passed; clippy -D warnings clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c430a6f6a8435f14bd29
…s file - The sync now runs as the tail of the daemon's startup task (via Hub::run_after_startup, which also takes over starting chats), so the daemon's abort of that task at shutdown ends it on every path, a signal stop included, instead of a separate handle. daemon.rs shrinks by 5 lines instead of growing past its over-budget base size. - artifacts_sync.rs was over the cmux-tui god-file limit (1649 lines, 66 fns). Its tests move to artifacts_sync_tests.rs and the agent tools' HTTP client to artifacts_client.rs (re-exported as artifacts_sync::ArtifactsClient), so each file is within budget. No behavior change. cargo test -p acpmux --lib artifacts_sync: 19 passed; clippy -D warnings, cargo fmt --check and check-no-godfiles.sh --only rust --base clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c430a6f6a8435f14bd29
|
Review (the owner lane's independent review, at its land request): SHIP at 04653fe Recorded by lane-gh-broker from this PR's owner lane, which asked to land this exact head after its independent review. next-autoland lands it once every red is the base's; a BLOCK at this head, a HOLD line or a hold label still stops it. |
|
Review (codex gpt-6.1-sol xhigh, next-review): BLOCK at 9373416 BLOCK at 9373416 Visual verdict: no images; Leo tries the build
Follow-ups:
|
|
Orphaned: no activity for 18 h, and no lane holds a lease on it. The orphan sweep could not merge |
|
Owned by an agent session · session |
…cts-sync-token-refresh # Conflicts: # cmux-tui/crates/acpmux/src/daemon.rs
|
Review (subagent claude-opus-5-5): SHIP at 23f7096 Checked: the base-merge resolution in Follow-ups:
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Carries #27254 (opt-in memory and artifact sync for acpmux) onto current
feat-cmux-next: its five commits keep their authorship, rebased with one conflict line indaemon.rs. One fix commit goes on top. #27254's branch is untouched, and #27254 stays open for its owner.For Leo's artifacts asks:
This PR is the sync layer under the agent tools (#27253, which rebases onto this once it lands) and the Labs Artifacts pane (#27264). The pieces are:
~/.claude/projects/*/memory,~/.codex/memoriesand~/.cmux/artifactswith the per-user/v1/artifactsstore in cmux-platform.artifactsSync.enableddefaults tofalse. While it is off, the daemon task returns at once and nothing reads credentials, touches the network or touches files.Fix on top (8297258)
queryneeds a feature this workspace leaves off, so the query is encoded withUrl::parse_with_params;sync_onceborrowedself.rootsacross a&mut selfcall;SaveResponse.commitfield and a collapsibleiffailed clippy;Clientbefore any rustls provider was installed. Both now install the workspace's ring provider, asmesh_httpdoes. The new test caught this.Fix on top (c68b4b2)
pull_oncemapped each listed remote path onto a local root by joining the server's text unchecked, so a listedmemory/codex/../../.ssh/authorized_keyswould be written outside the memory folders. A remote path now maps only when every component is a plain name: no..,., root, empty part, backslash or NUL. New test:a_remote_path_never_maps_outside_its_root.Review fixes (e73f197)
An independent review (gpt-6.1-sol at high, at c68b4b2) BLOCKed on four data-safety findings in the sync. All four are fixed:
Red/green: with the new tests on c68b4b2, three of them fail.
pull_keeps_a_local_edit_the_server_has_not_takengets"remote"where it expects"local edit".a_failed_conflict_read_never_empties_a_memorygets"".symlinks_below_a_root_are_never_followedmaps through the symlink. With this commit, all 11 pass.Review fixes, round 2 (348da70)
At e73f197, the reviewer confirmed the first four fixes and BLOCKed on three more:
remote_filenow tells a missing file from an empty one.password: x,apiKey: ...andaccess_token: .... It now also runs the daemon's secret scanner (gitleaks' rules and cmux's own) and a credential-key assignment pattern.Red/green: three of the new tests fail at e73f197.
a_failed_conflict_retry_keeps_the_local_edit_unsyncedfinds the merge recorded as synced.a_binary_conflict_keeps_both_versionsfinds the original's synced state still"old".secret_paths_and_content_are_skippedfindspassword: hunter22uploaded. With this commit, all 13 pass.Review fixes, round 3 (ef25459, 85ecb92)
At 348da70, the reviewer confirmed round 2 and BLOCKed on four more:
deletechange. A pull skips a file whose commit is the one this machine last synced; a file deleted here but changed remotely since comes back, so their edit wins.~/.codex/memories -> /elsewhere). Such a root is not synced, in either direction.password: xgot through; the credential-key pattern now takes a value of any length.Hub::run_after_startup, which also takes over starting chats). The daemon already aborts that task on every shutdown path.Also found while writing the tests:
content_base64), while cmux-platform'sArtifactChangeis camelCase. A contract test now checks the field names.ACPMUX_HOME/artifacts-sync-state.json, written whole and only when it changes. A restart no longer re-sends everything or restores a file deleted while the daemon was down. File contents stay in memory only.To keep the god-file check green, the tests move to
artifacts_sync_tests.rsand the agent tools' client toartifacts_client.rs. It is re-exported, so #27253'scrate::artifacts_sync::ArtifactsClientimport still works.daemon.rsshrinks by 5 lines.Review fixes, round 4
At 85ecb92, the reviewer confirmed round 3 and BLOCKed on one issue. A root was checked only at its last folder, so a symlinked folder above it (
~/.claude/projects/work -> /tmp/private) still led uploads out of it and pulls into it. Each root now records the folder it sits under (home). Every folder from there down to the root must be a real directory, checked on each pass. New test:a_root_below_a_symlinked_folder_is_not_synced; it fails at 85ecb92 and passes now. The branch then mergedfeat-cmux-nextpast 4154db0 (the base Swift test-compile fix) cleanly.Review fixes, round 5 (1257774)
At 30bb0e8, the reviewer confirmed round 4 and BLOCKed on one issue: the symlink checks were check-then-use. Another process could swap a checked folder for a symlink between the check and the walk or a write, and lead an upload or a pull outside its root. All file access in a pass now goes through
artifacts_fs::Dir:O_NOFOLLOW, asconfig::private_state_diralready does. The walk, every read, the delete check and every write then go through descriptors relative to that folder, never a path.Also from the reviewer's notes: a conflict copy's name uses
HOSTNAMEonly as one plain name part, and its timestamp is in milliseconds, so two conflicts on one file within a second keep both copies. New test:a_folder_swapped_for_a_symlink_mid_pass_leads_nowhere.Reviewer note: two Astra reviews through SR2 hung (64 and 29 minutes, no verdict), so reviews here use gpt-6.1-sol, the interchangeable Codex reviewer.
Timer: the sync interval is owned by the sync task, which runs at the end of the daemon's startup task. That task is aborted on every shutdown path, and
hub.shutdownalso ends a pass in flight. HTTP has 15 s connect and 60 s per-read timeouts.Red comparison (by test name, against base)
test_ir.IrTests.test_live_raw_v12_layout_and_history_contracts_are_exact (command='new-pane-right')cmux-tui/spec/Observed / Not run
cargo test --locked -p acpmux --lib artifacts_syncpassed 21/21, covering token refresh, path and symlink confinement, pull edits, conflict reads and retries, binary conflicts, secrets, deletes, pagination, persisted state and the save-body field names.check-no-godfiles.sh --only rust --base origin/feat-cmux-nextis clean.cargo clippy --locked -p acpmux --all-targets -- -D warningsandcargo fmt -p acpmux --checkwere clean.libccalls inartifacts_fs.rs, since this host has no Apple C toolchain to buildring; CI'scmux-tui artifactsmacOS jobs compile it. Also not run: the full acpmux suite (CI runs it), and an end-to-end save against cmux-platform. The dev Worker still answers 404 on every artifacts path until its Cloudflare binding lands.Changelog
none (opt-in, off by default)
Agent session: agent session
🤖 Generated with Claude Code