Repository navigation
Cloud: recover completed repository attachments - #30887
Conversation
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (claude-opus-5-5-medium): SHIP at 75d7f2a
This restores the completed-jobs change from closed #30452.
Review (subagent claude-opus-5-5 medium): SHIP at 75d7f2a
#30452 completed-jobs, rebase review. Old: 6bf7cb2..4c982df (3 commits). New: 54c63f9..75d7f2a (4 commits). I compared the added/removed lines of both whole diffs.
Differences, classified:
- Commit 2 (keep fetched repos during reconnect and test live progress) is patch-identical.
- (a) Commit 1: the completed-result check
completed["repo"] != repo || completed["folder"] != json!(folder) || completed["state"] != "done" || !completed["head"].as_str()...is now inget()form. It is equivalent for missing keys and non-string values: Null != "x" matches None != Some("x"), and Null != json!(folder) matches None != Some(&json!(folder)). The progress write's inversion (== "running" && throttled) is kept, inget()form.existing["job"]andrecent.insert(... repo["repo"] ...)were converted the same way. - (a) Commit 3:
self.optional_install_url(&answer["connect"])becameoptional_install_url(connect)inside base'sif let Some(connect).catalog.disconnected = trueis kept. - (a) New commit 75d7f2a: store::record's job retirement. The old code nested three ifs: name from repo, then
doc["jobs"][session][name]["repo"] == repo["repo"], thenget_mutand remove. The new code is one let-chain withjobs.get(name).and_then(|j| j.get("repo")) == repo.get("repo"). That is equivalent: a missing job or a job without a repo was Null == "owner/x" (false) and is now None == Some (false). It removes only on a matching repo, still inside the same CAS write. - No (b) or (c) items.
Base not reverted: no stack file overlaps the newer base commits, and the stack-file diff against the current base equals the three-dot diff.
Follow-ups: none new. I did not run a compile.
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. |
There was a problem hiding this comment.
1 issue found across 5 files
You’re at about 92% 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-core/src/cloud_repos/store.rs">
<violation number="1" location="cmux-tui/crates/cmux-tui-core/src/cloud_repos/store.rs:257">
P3: This retires any job for the repo, including a `running` one. Limit the removal to completed jobs so a forwarded `remember` cannot delete an in-flight clone's progress.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| .get_mut("jobs") | ||
| .and_then(|jobs| jobs.get_mut(session)) | ||
| .and_then(Value::as_object_mut) | ||
| && jobs.get(name).and_then(|job| job.get("repo")) == repo.get("repo") |
There was a problem hiding this comment.
P3: This retires any job for the repo, including a running one. Limit the removal to completed jobs so a forwarded remember cannot delete an in-flight clone's progress.
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-core/src/cloud_repos/store.rs, line 257:
<comment>This retires any job for the repo, including a `running` one. Limit the removal to completed jobs so a forwarded `remember` cannot delete an in-flight clone's progress.</comment>
<file context>
@@ -246,6 +246,18 @@ pub(super) fn record(mux: &Mux, session: &str, repo: &Value) -> Result<(), Failu
+ .get_mut("jobs")
+ .and_then(|jobs| jobs.get_mut(session))
+ .and_then(Value::as_object_mut)
+ && jobs.get(name).and_then(|job| job.get("repo")) == repo.get("repo")
+ {
+ jobs.remove(name);
</file context>
| && jobs.get(name).and_then(|job| job.get("repo")) == repo.get("repo") | |
| && jobs.get(name).is_some_and(|job| job.get("state").and_then(Value::as_str) == Some("done") && job.get("repo") == repo.get("repo")) |
54c63f9 to
964ceb9
Compare
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (claude-opus-5-5-medium): SHIP at c24d19f
Review (claude-opus-5-5 medium): SHIP at c24d19f (#30887)
Carried from 75d7f2a (r-rebase-30452.md). The one-commit delta in cloud_repos/runtime.rs rewrites !x.is_some_and(|h| !h.is_empty()) as x.is_none_or(|h| h.is_empty()). That is the same boolean (None or empty, so the result is refused), changed only to satisfy clippy nonminimal_bool.
c24d19f to
0d3088a
Compare
|
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)
Shipped in:
|
|
Review (subagent claude-opus-5-5 medium, carried): SHIP at 0d3088a Carried from the SHIP at c24d19f. The restack onto 71f7bae (#30434's squash) leaves the PR's own diff unchanged: |
Leo asked for “this kind of recovery” when attaching a repository to a Cloud chat.
A completed clone now persists its result until the chat attachment is recorded. Retrying after a record failure reuses that saved result even when the seed job has expired, and attachment recording clears the job atomically.
Follow-up 4 to #30361; replaces closed #30452 at its restored and reviewed head. A regression test injects record failure, shuts down the fake seed socket, and verifies retry records the completed clone without another start or status call. Attachment recording clears the saved job in the same projection transaction.
Repository discovery also retains pages already fetched if authentication expires on a later page. An unsafe installation link is omitted without hiding the repositories; disconnected state is tracked separately so it cannot falsely complete the Connect GitHub wait. Fake socket tests check that partial catalog and the actual pending clone progress before completion, addressing the recovery review follow-ups.
Validation: the full stack at 7dca439 passed the Rust cloud_repo suite on the fleet (step 9e6433bb9eb7ac166da6d706). Formatting and static diff checks pass.
Agent session: agent session