Repository navigation
link: cmux link seed pull brings a seeded Cloud folder's changes back as a diff - #30923
Conversation
…diff Piece 6, Mac side, of the Cloud project seed (design/cloud.md, rule 6). Calls git.seed.export, streams the bundle down over fs-v1, fetches it into the git_back quarantine (alternates, fsckObjects), then merges three ways against the seed: HEAD fast-forward of the same branch only, the machine's index when ours is unchanged, the working tree per file between the seed's WORK, the machine's WORK and this Mac's. Conflicts change nothing and answer sync_conflict; otherwise files move through place.rs's transaction and the branch and index move last by compare-and-swap. The answer's base is the next pull's and delta push's base. Base shas come as flags until piece 4's record lands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-3bab30503f0b9a7059e5
The same review finding as the machine's export, on the way up: an untracked secret whose name is not UTF-8 was removed from WORK by a lossy spelling that matched nothing, so its contents went to the machine. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-3bab30503f0b9a7059e5
…as overrides; under seed_cli beside push The pull's CLI moves to link/seed_cli/pull.rs (beside push and clone); its engine stays in sync_cli/seed_pull.rs. The base comes from piece 4's record of the folder unless all three --base-* flags are given, and every pull keeps the machine's HEAD, INDEX and WORK as the new record, so the next pull and the next push --since-last build on it. The journey seeds with cmux link seed push and uses base's CMUX_TEST_GIT_SEED_GITHUB seam. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-3bab30503f0b9a7059e5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-3bab30503f0b9a7059e5
|
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)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 6e7cd15 to compare; do not merge until green; see lane-rules Shipped in:
|
|
Owned by an agent session · session |
|
Review (codex gpt-6-astra high, router-review): BLOCK at 9139a54 BLOCK at 9139a54 Visual verdict: no images; Leo tries the build
Isolated Git probes confirmed the snapshot mismatches behind findings 1–2. Rust tests could not run because Cargo and rustc are unavailable. Follow-ups:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 10 files
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cmux-tui/crates/cmux-tui/src/link/seed_cli.rs">
<violation number="1" location="cmux-tui/crates/cmux-tui/src/link/seed_cli.rs:251">
P3: This new pull route accepts `//` or `///` as a remote path but normalizes them to an empty string. Reject an empty normalized path so malformed input gets the documented bad-usage response instead of entering the pull flow.</violation>
</file>
<file name="cmux-tui/crates/cmux-tui/tests/seed_pull_journey.rs">
<violation number="1" location="cmux-tui/crates/cmux-tui/tests/seed_pull_journey.rs:107">
P2: This seed-plan subprocess inherits the developer's real HOME and global Git configuration, so URL rewrites and other user settings can change the test result. Run it through the machine's isolated command environment and disable global/system Git configuration, as the other Git test setup does.</violation>
</file>
<file name="cmux-tui/crates/cmux-tui/src/link/sync_cli/seed_pull.rs">
<violation number="1" location="cmux-tui/crates/cmux-tui/src/link/sync_cli/seed_pull.rs:318">
P2: A local type change during the pull can make `Transaction::delete` do nothing, but this call still counts the deletion and advances the base. Treat a non-regular or missing target that no longer matches the planned file as `Changed` so the pull reports a conflict instead of silently losing the remote deletion.</violation>
<violation number="2" location="cmux-tui/crates/cmux-tui/src/link/sync_cli/seed_pull.rs:606">
P1: The staging failure path ignores branch-rollback errors. If that rollback fails, `seed_pull` restores files but returns with the branch moved; propagate and surface the rollback failure instead of discarding it.</violation>
</file>
<file name="cmux-tui/crates/cmux-tui/src/link/seed/plan.rs">
<violation number="1" location="cmux-tui/crates/cmux-tui/src/link/seed/plan.rs:442">
P2: `current` unnecessarily requires the local repository to retain a GitHub remote, so a seeded folder whose remote was removed or changed cannot be pulled back. Build the current HEAD/index/work commits through a local-only repository opener instead of `Repo::open`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| && let Err(error) = checked(local(folder).arg("read-tree").arg(tree)) | ||
| { | ||
| if let Some(head) = head { | ||
| let _ = move_ref(&old, head); |
There was a problem hiding this comment.
P1: The staging failure path ignores branch-rollback errors. If that rollback fails, seed_pull restores files but returns with the branch moved; propagate and surface the rollback failure instead of discarding it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-tui/src/link/sync_cli/seed_pull.rs, line 606:
<comment>The staging failure path ignores branch-rollback errors. If that rollback fails, `seed_pull` restores files but returns with the branch moved; propagate and surface the rollback failure instead of discarding it.</comment>
<file context>
@@ -0,0 +1,695 @@
+ && let Err(error) = checked(local(folder).arg("read-tree").arg(tree))
+ {
+ if let Some(head) = head {
+ let _ = move_ref(&old, head);
+ }
+ return Err(error.context("cannot stage the machine's changes"));
</file context>
| fn seed(&self, repo: &Path, name: &str) -> Base { | ||
| let bundle = self.root.join(format!("{name}.bundle")); | ||
| let (ok, plan) = run_json( | ||
| Command::new(bin()) |
There was a problem hiding this comment.
P2: This seed-plan subprocess inherits the developer's real HOME and global Git configuration, so URL rewrites and other user settings can change the test result. Run it through the machine's isolated command environment and disable global/system Git configuration, as the other Git test setup does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-tui/tests/seed_pull_journey.rs, line 107:
<comment>This seed-plan subprocess inherits the developer's real HOME and global Git configuration, so URL rewrites and other user settings can change the test result. Run it through the machine's isolated command environment and disable global/system Git configuration, as the other Git test setup does.</comment>
<file context>
@@ -0,0 +1,361 @@
+ fn seed(&self, repo: &Path, name: &str) -> Base {
+ let bundle = self.root.join(format!("{name}.bundle"));
+ let (ok, plan) = run_json(
+ Command::new(bin())
+ .args(["link", "seed", "plan", "--local"])
+ .arg(repo)
</file context>
| transaction.write(relative, staged, expected.as_ref()).map(|()| written += 1) | ||
| } | ||
| Step::Delete { relative, expected } => { | ||
| transaction.delete(relative, expected).map(|()| deleted += 1) |
There was a problem hiding this comment.
P2: A local type change during the pull can make Transaction::delete do nothing, but this call still counts the deletion and advances the base. Treat a non-regular or missing target that no longer matches the planned file as Changed so the pull reports a conflict instead of silently losing the remote deletion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-tui/src/link/sync_cli/seed_pull.rs, line 318:
<comment>A local type change during the pull can make `Transaction::delete` do nothing, but this call still counts the deletion and advances the base. Treat a non-regular or missing target that no longer matches the planned file as `Changed` so the pull reports a conflict instead of silently losing the remote deletion.</comment>
<file context>
@@ -0,0 +1,695 @@
+ transaction.write(relative, staged, expected.as_ref()).map(|()| written += 1)
+ }
+ Step::Delete { relative, expected } => {
+ transaction.delete(relative, expected).map(|()| deleted += 1)
+ }
+ };
</file context>
| } | ||
|
|
||
| pub(crate) fn current(folder: &Path) -> Result<Current, Fallback> { | ||
| let repo = Repo::open(folder)?; |
There was a problem hiding this comment.
P2: current unnecessarily requires the local repository to retain a GitHub remote, so a seeded folder whose remote was removed or changed cannot be pulled back. Build the current HEAD/index/work commits through a local-only repository opener instead of Repo::open.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-tui/src/link/seed/plan.rs, line 442:
<comment>`current` unnecessarily requires the local repository to retain a GitHub remote, so a seeded folder whose remote was removed or changed cannot be pulled back. Build the current HEAD/index/work commits through a local-only repository opener instead of `Repo::open`.</comment>
<file context>
@@ -421,6 +426,33 @@ const SEED_NAME: &str = "cmux seed";
+}
+
+pub(crate) fn current(folder: &Path) -> Result<Current, Fallback> {
+ let repo = Repo::open(folder)?;
+ let scratch = Scratch::new()?;
+ let made = commits(&repo, &scratch)?;
</file context>
| ("pull", [local, remote]) if plain && !since_last => Some(Command::Pull { | ||
| socket: socket?, | ||
| local: PathBuf::from(local), | ||
| remote: absolute(remote)?, |
There was a problem hiding this comment.
P3: This new pull route accepts // or /// as a remote path but normalizes them to an empty string. Reject an empty normalized path so malformed input gets the documented bad-usage response instead of entering the pull flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-tui/src/link/seed_cli.rs, line 251:
<comment>This new pull route accepts `//` or `///` as a remote path but normalizes them to an empty string. Reject an empty normalized path so malformed input gets the documented bad-usage response instead of entering the pull flow.</comment>
<file context>
@@ -220,8 +234,25 @@ fn parse(args: &[String]) -> Option<Command> {
+ ("pull", [local, remote]) if plain && !since_last => Some(Command::Pull {
+ socket: socket?,
+ local: PathBuf::from(local),
+ remote: absolute(remote)?,
+ base,
+ }),
</file context>
…om; hold the index from its check to its replacement Review (gpt-6-astra) at 9139a54: a file saved after the snapshot (or one git assumes unchanged) could be overwritten, since what the transaction expected was read later from disk; and the staged changes were read apart from the snapshot and could be replaced after a newer git add. Now the bytes the transaction expects are the bytes checked against the planned blob (git hash-object --path, through the path's filters), the branch and index are read on both sides of the snapshot, and the index is replaced under git's own index.lock from its check on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-3bab30503f0b9a7059e5
|
Found 6 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cmux-tui/crates/cmux-tui/src/link/sync_cli/seed_pull.rs">
<violation number="1" location="cmux-tui/crates/cmux-tui/src/link/sync_cli/seed_pull.rs:198">
P2: This digest comparison is not an atomic snapshot of `plan::current`. If the index changes away and back while `current` runs, a machine index matching the transient snapshot is treated as already applied; the pull succeeds, records that index as the new base, and never installs it locally. Hold the index snapshot stable across the merge decision, or use a snapshot mechanism that cannot pass this ABA change.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| let here_git = git_back::base_of(&folder)?; | ||
| let here = super::super::seed::plan::current(&folder) | ||
| .map_err(|fallback| anyhow::anyhow!("cannot read this folder: {}", fallback.why))?; | ||
| if git_back::base_of(&folder)? != here_git { |
There was a problem hiding this comment.
P2: This digest comparison is not an atomic snapshot of plan::current. If the index changes away and back while current runs, a machine index matching the transient snapshot is treated as already applied; the pull succeeds, records that index as the new base, and never installs it locally. Hold the index snapshot stable across the merge decision, or use a snapshot mechanism that cannot pass this ABA change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-tui/src/link/sync_cli/seed_pull.rs, line 198:
<comment>This digest comparison is not an atomic snapshot of `plan::current`. If the index changes away and back while `current` runs, a machine index matching the transient snapshot is treated as already applied; the pull succeeds, records that index as the new base, and never installs it locally. Hold the index snapshot stable across the merge decision, or use a snapshot mechanism that cannot pass this ABA change.</comment>
<file context>
@@ -189,9 +189,17 @@ pub(crate) fn seed_pull(
let here = super::super::seed::plan::current(&folder)
.map_err(|fallback| anyhow::anyhow!("cannot read this folder: {}", fallback.why))?;
- let here_git = git_back::base_of(&folder)?;
+ if git_back::base_of(&folder)? != here_git {
+ bail!(
+ "this Mac's branch or staged changes changed during the pull; nothing was brought back"
</file context>
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Review (codex, router-review): SHIP at ee086f4 Reviewer: codex gpt-6-astra high, fast tier. Follow-ups: 1. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward. SHIP at ee086f4 Visual verdict: no images; Leo tries the build.
|
|
This PR added |
…d forms (#31097) #30921 (af670df, cmux mcp video_frames/video_info) and #30923 (b91b9c7, cmux link seed pull) added clippy::indexing_slicing hits in cmux-tui production code, a deny lint of the panic ratchet: cmux-tui went from 831 to 855. Each read now uses a checked form; valid input behaves the same, and a missing or non-object field reads as it did through serde_json's Index (null): - video_tools: the app's answer and error fields through Value::get; the params writes through cmux_json::JsonSet::set (the params are always an object, as before). - seed_pull export: the job and status fields through Value::get; the bundle copy's chunk through buffer.get(..read), an error if a read ever claims more than its buffer. - seed_cli pull keep: summary.base.{head,index,work} through Value::get. No allow attributes, no baseline change. Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
… APFS (macOS base red) (#35889) a_secret_with_a_non_utf8_name_is_left_out (#30923) and a_secret_with_a_non_utf8_name_never_goes (#30633) create a file named b".env-\xff". APFS refuses a name that is not UTF-8 (EILSEQ, 'Illegal byte sequence'), so both panicked at setup on every macOS run since they landed; no folder on macOS can hold such a secret, so the product path they check cannot meet one there. They now accept EILSEQ on macOS only, as git_seed/secrets.rs does since #32091; on Linux (ext4 takes raw bytes) the assertions run unchanged.
Leo's words:
This is piece 6 of the Cloud project seed, rule 6 of "Getting the project onto the machine" in the Cloud design spec: the folder's changes come back to the Mac as a diff, not as a copy of the folder. It is the Mac side of
git.seed.export(#30633, merged) and reads its base from the seed record thatcmux link seed pushkeeps (#30634, merged).What it does
cmux link seed pull --socket PATH LOCAL REMOTE [--base-head SHA --base-index SHA --base-work SHA]--base-*flags, given all together or not at all, override it. It answersseed.no_previousin these cases:cmux link sync pullbrings back instead;git.seed.exportsince that base, wait for the job, and download the bundle overfs-v1. The machine must serve bothfs-v1andgit-seed-export-v1; otherwise the answer ismachine_outdated.transfer.fsckObjects, and checks that the bundle builds on the base..git/HEADconflict.{"ok":false,"error_code":"sync_conflict","conflicts":[...]}.place.rs's transaction. The branch then moves by compare-and-swap and the index follows. Any failure puts back what changed..gitpath, a hook (includingcore.hooksPath), a nested repo, a symlink or submodule the machine changed, an ignored file, or an untracked secret.baseis saved as the new record, so the next pull and the nextcmux link seed push --since-lastbuild on it. A machine with no changes answersunchangedand nothing is downloaded.The answer is
{ok, unchanged, written, deleted, bytes, bundle_bytes, fast_forwarded, staged, head, base{head,index,work}, left_out}.Layout
link/seed_cli/pull.rs, next to push and clone, and handles the record.link/sync_cli/seed_pull.rs, because it uses the sync internals:Remote,git_back's quarantine, andplace.link/seed/plan.rsgainscurrent(), shared by the seed and the pull, and leaves out a secret by its raw (non-UTF-8) name.Size
About 1,400 lines, of which about 360 are the journey test. The engine is one state machine (export, receive, the three-way plan, the transaction) with its tests. Splitting it would land a part that can't be run on its own.
Tests
tests/seed_pull_journey.rsruns real Mac and VM daemons on Linux, with the VM cloning fromfile://through theCMUX_TEST_GIT_SEED_GITHUBseam:cmux link seed push, then pulls from the record. Covered: commits, edits, adds, deletes and staged changes come back; ignored files and secrets stay on the VM; a second pull isunchanged; the nextpush --since-lastbuilds on the pulled base..git/HEADconflict.link::seed24 passed,seed_pull_journey3 passed,link_seed6 passed,cargo fmtclean. Clippy flags only files already flagged on base (start_chat_cli.rs,environments.rs).Risk: new CLI only. Nothing calls
seed pullyet, and the push side is unchanged.Agent session: agent session
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Adds
cmux link seed pull, the Mac side of the Cloud project seed: after a push copies a folder to the machine, pull brings its changes back as a diff merged against the seed, never as a folder copy. Nothing calls it yet and the push side is unchanged.New Features
sync_conflict.--base-*flags given all together, and every pull keeps the machine's state as the next pull's andpush --since-last's base.Bug Fixes
index.lock.Written for commit ee086f4. Summary will update on new commits.