fix: clone notes fetch and carryover snapshot for non-repo CWD#838
fix: clone notes fetch and carryover snapshot for non-repo CWD#838
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
src/daemon.rs
Outdated
| } else { | ||
| // No family resolved — this can happen for clone/init from a | ||
| // non-repo CWD when def_repo is not emitted or family | ||
| // resolution fails. Clone notes sync only needs the target | ||
| // path, not a family, so apply it directly. | ||
| self.maybe_apply_familyless_clone_side_effect(&applied); |
There was a problem hiding this comment.
🟡 Missing exit_code guard in familyless clone side-effect path
The familied path guards against applying clone notes sync for failed commands via the if cmd.exit_code != 0 { ... return Ok(()); } early return at src/daemon.rs:6112. However, the new maybe_apply_familyless_clone_side_effect function has no such check, and neither does its call site at line 6388. This means when a git clone fails (exit_code != 0) from a non-repo CWD where family_key is None, the code will still attempt to resolve the clone target and call apply_clone_notes_sync_side_effect, potentially issuing unnecessary network requests (fetch_authorship_notes) against a partially-created or broken repository. Errors are caught and logged so there's no crash, but this is inconsistent with the familied path's behavior.
| } else { | |
| // No family resolved — this can happen for clone/init from a | |
| // non-repo CWD when def_repo is not emitted or family | |
| // resolution fails. Clone notes sync only needs the target | |
| // path, not a family, so apply it directly. | |
| self.maybe_apply_familyless_clone_side_effect(&applied); | |
| } else { | |
| // No family resolved — this can happen for clone/init from a | |
| // non-repo CWD when def_repo is not emitted or family | |
| // resolution fails. Clone notes sync only needs the target | |
| // path, not a family, so apply it directly. | |
| if applied.command.exit_code == 0 { | |
| self.maybe_apply_familyless_clone_side_effect(&applied); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if let Some(target) = target_from_def_repo.as_ref() { | ||
| candidates.push(target.clone()); | ||
| } | ||
| if let Some(target) = target_from_def_repo.as_ref() { | ||
| if let Some(target) = target_from_argv.as_ref() { | ||
| let duplicate = candidates.iter().any(|existing| existing == target); | ||
| if !duplicate { | ||
| candidates.push(target.clone()); |
There was a problem hiding this comment.
🟡 Fallback worktree preference not updated to match swapped candidate ordering
The PR swaps the candidate ordering at lines 931–938 to prefer target_from_def_repo (always absolute) over target_from_argv (potentially relative), with an explicit comment (lines 924–930) explaining that def_repo should be preferred to avoid storing relative paths. However, the !resolved fallback at line 970 still uses target_from_argv.or(target_from_def_repo), which prefers the argv-derived target — directly contradicting the stated intent.
When both candidates fail common_dir_for_repo_path (e.g., the clone just finished but filesystem state is slightly delayed), the fallback stores the potentially-relative argv path into pending.worktree. Downstream, maybe_apply_familyless_clone_side_effect (src/daemon.rs:6403-6435) uses cmd.worktree to call apply_clone_notes_sync_side_effect, which runs git -C <path> — a relative path will fail to resolve from the daemon's unrelated CWD, causing clone notes sync to silently not happen.
Was this helpful? React with 👍 or 👎 to provide feedback.
svarlamov
force-pushed
the
fix/clone-notes-carryover-snapshot
branch
from
591dfad to
ac00b76
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Root cause: during `git clone`, child processes (remote-https, index-pack, rev-list) emit trace2 def_repo events with the CWD as worktree — not the clone destination. These arrive after the root process's correct def_repo and overwrite pending.worktree, so the daemon tried to sync notes against the parent directory (e.g. ~/projects) instead of the cloned repo. Fix: for clone/init, once the first def_repo is captured (root process), skip subsequent child def_repo events. Supporting changes: - Skip carryover snapshot for clone/init (target repo doesn't exist yet) - Add familyless clone handler for when family resolution fails - Prefer def_repo (absolute) over argv (may be relative/URL) in candidate and fallback ordering - Add unit test for child def_repo overwrite scenario - Add integration tests for absolute-target and implicit-target clone from non-repo CWD Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
svarlamov
force-pushed
the
fix/clone-notes-carryover-snapshot
branch
from
ac00b76 to
f441e36
Compare
1 participant
Summary
clone/initcommands — they create repos, so there is no pre-existing state to snapshot, and the worktree hint may point to a non-repo CWD causing a spurious errorCloneCompleted::target(from argv) whencmd.worktreedoesn't contain a git repo, fixing clone notes fetch whendef_repohasn't updated the worktree hintTest plan
notes_sync_regression— 48/48 passed (all modes)daemon_mode— 50/50 passedintegration(wrapped-daemon) — 3056/3056 passedcargo clippy— no warningsnotes_sync_clone_from_inside_another_repo_fetches_authorship_notespasses in all 5 modes🤖 Generated with Claude Code