fix(miner): check out baseBranch on the fresh-clone path so the first attempt succeeds (#9682) - #9903
fix(miner): check out baseBranch on the fresh-clone path so the first attempt succeeds (#9682)#9903kai392 wants to merge 1 commit into
Conversation
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-29 20:51:06 UTC
Review summary Nits — 3 non-blocking
CI checks failing
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
… attempt succeeds (JSONbored#9682) `ensureRepoClonedUnlocked`'s docstring promises both branches land on the base branch, but the fresh-clone branch returned right after `git clone`. A plain clone creates only origin's default-HEAD local branch, so the consumer's `git worktree add -b <attempt> <path> <baseBranch>` cannot rev-parse a bare non-default `<baseBranch>` -- the first attempt against any repo whose base isn't the origin default fails with `invalid reference` (surfacing as blocked_worktree_preparation_failed), and only the second attempt (which hits the fetch+reset path that DWIM-creates the local branch) works. - After a successful clone, run `git checkout <baseBranch>` and return the identical `{ ok:false, error: stderr || "git_checkout_failed" }` shape the existing-clone branch already uses. No fetch/reset is added -- a fresh clone is already at origin's tip. The isUnsafeGitArgValue guard still runs first. Tests: three named regression cases (checkout happens after clone on the first-use path; a failing checkout returns git_checkout_failed / stderr; the fresh path emits exactly clone+checkout, no fetch/reset). The existing per-repo concurrency test's pinned git-op sequence is updated to include the new checkout (its non-overlapping guarantee is unchanged). All new assertions fail against the current code. Closes JSONbored#9682 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
|
LoopOver had already started reviewing this pull request — closing it to dodge the one-shot review process is not allowed. Please open a new pull request with the issues addressed. |
What
ensureRepoClonedUnlocked's docstring promises both branches land on the base branch, but the fresh-clone branch returned right aftergit clone. A plain clone creates only origin's default-HEAD local branch, so the consumer'sgit worktree add -b <attempt> <path> <baseBranch>(worktree-plan.ts:80-84) can't rev-parse a bare non-default<baseBranch>. The first attempt against any repo whose base isn't the origin default fails withfatal: invalid reference: <baseBranch>(surfacing asblocked_worktree_preparation_failed), and only the second attempt — which hits the fetch+reset path that DWIM-creates the local tracking branch — works. This is reachable in production (attempt-cli.ts:578passes the PR's real base;attempt-worktree.ts:88defaults to"main", failing on the first attempt against anymaster-default repo).How
After a successful
git clone, rungit checkout <baseBranch>inrepoPathand return the identical{ ok: false, repoPath, error: stderr || "git_checkout_failed" }shape the existing-clone branch already uses (repo-clone.ts:335-336). Nofetch/resetis added — a fresh clone is already at origin's tip. TheisUnsafeGitArgValue(baseBranch)guard still runs first.Tests
Three named regression cases in
test/unit/miner-repo-clone.test.ts, each failing against the current code:baseBranch: "develop", the injectedrunGitreceives["checkout", "develop"]after["clone", ...].{ ok: false, error: "git_checkout_failed" }(and surfaces git's stderr verbatim when present).clonethencheckout— nofetch/reset.The existing per-repo concurrency test's pinned git-op sequence is updated to include the new checkout; its non-overlapping serialization guarantee is unchanged.
Closes #9682