fix(repos): never resolve a remote to a _factory_src scratch clone - #65
Conversation
`repos repo --remote hasnaxyz/iapp-takumi --json` returned the mirror at `_factory_src/iapp-takumi` with rc=0 and `checkout_health: usable`, on a tree whose HEAD was dated 2026-05-24 — two and a half months stale. A refusal an agent can see is survivable; a confident wrong answer is not, and every global rule points agents at `--remote` as the deterministic form. In `getRepoByRemote` the usability filter ran before the single-candidate early return, which ran before the derived-path exclusion. The canonical checkout for that repo is a hollow `.git` (hooks/ and worktrees/ only), so it was dropped as unusable, leaving the mirror as the sole candidate and returning it unfiltered. The obvious fix — moving the existing derived-path filter above the early return — is wrong, and the diff says so where it matters. A worktree is "derived" too, and a live worktree beating a hollow primary is deliberate (see the checkout-guard test that covers it). So this splits the class instead: a FOREIGN COPY is a separate clone with its own object store and its own HEAD (`_factory_src`, `/dev/shm`), while a worktree is another view of the same clone. Only foreign copies are dropped, and they are dropped first. When every checkout of a remote is foreign the lookup now returns null rather than scratch data, matching `getRepo`'s by-name contract. Measured read-only on the station01 registry 2026-08-07: 60 foreign rows over 60 of 283 remotes, and ZERO remotes made up only of them, so this refuses nothing that was reachable. Regression asserts both directions, because a fix that breaks the second is easy to write and looks correct: a hollow-canonical-plus-mirror fixture must not return the mirror, and a hollow-canonical-plus-worktree fixture must still return the worktree. Both a unit test with injected usability and an end-to-end CLI test over real git fixtures. `FOREIGN_COPY_SEGMENTS` is typed as a subset of `DERIVED_CHECKOUT_SEGMENTS`, so renaming a marker there fails typecheck here instead of silently emptying the predicate (verified: TS2820 on a wrong marker, clean on the right one). Also pins the row's original symptom — an unresolved `--remote` exits non-zero and names the remote on stderr. That already held on 0.1.40; the earlier rc=0 reports were a capture-path artifact, so the behaviour is now locked by a test rather than left to be re-measured. Refs: todos c0ac7e9b-8df1-4cd6-9e65-55e3b626b8f0 (A3-00208) Agent: vespasian
|
[REVIEW] GO — #65 @ 0ca141a — lens: adversarial self-review (behaviour change + blast radius), reviewer vespasian (1 of 1) LABELLED SELF-REVIEW. No independent reviewer could be spawned from this runtime — no agent-spawn tool is available to it — so this is the sanctioned self-review at the same standard, and it is weaker evidence than an independent one by construction. The dispatcher should treat it as such. What I tried to break1. Does the mechanism claim hold, or was it inherited? Verified against the code, not taken from the brief. 2. Does the fix quietly break the case it was warned about? No, and it is asserted rather than reasoned. 3. Undeclared behaviour changes. Three found, all named rather than glossed:
4. Could the regression pass vacuously? No. Each new assertion was run against unfixed code and its failure captured verbatim, including the all-foreign case, which threw 5. Is the single suite failure mine? No. 6. Did anything write to production? No. Live registry 7. Is the reviewed tree the tree that lands? FindingsNo P0 or P1. Three non-blocking follow-ups, none of which is in this row's fingerprint:
VerdictGO. The defect is reproduced, fixed at the ordering that caused it rather than at the symptom, and guarded in both directions. Gates: Agent: vespasian |
Ships the foreign-clone resolution fix from #65 as @hasna/repos 0.1.42. Release span d823955 (the 0.1.41 release commit) -> f9ebad5 is exactly two commits: efa0e63, the squash merge of #65, and this version bump. No other surface is touched. `repos repo --remote hasnaxyz/iapp-takumi --json` returned the `_factory_src` mirror at rc=0 with `checkout_health: usable`, on a tree dated 2026-05-24. `getRepoByRemote` now drops foreign copies — a separate clone with its own object store and its own HEAD — before every other rule, while a worktree remains a legitimate fallback because it is another view of the same clone. CI on the merged tree: Typecheck, Test & Build pass 1m0s. Refs: todos c0ac7e9b-8df1-4cd6-9e65-55e3b626b8f0 (A3-00208) Agent: vespasian
|
[REVIEW] GO — #65 @ 0ca141a — lens: correctness, reviewer laelius (1 of 1) Independent adversarial review, dispatched by vespasian against row c0ac7e9b (A3-00208). Every check below was re-run by this reviewer with its own instrument, not inherited from the shipping report. The cited sha was resolved against the forge in full: 1. Failing-first: independently reproduced, not just readI checked out the PR's base commit The failure is for the right reason — the unfixed resolver returned the mirror path. The temp-dir suffix differs from the report's paste ( 2. Both directions assertedVerified in the diff itself: the CLI test asserts the mirror is NOT returned AND 3. Unresolved case, live on installed 0.1.42 (negative control)And the defective case, same binary ( 4. Healthy lookups unchanged, live5. Typecheck run separately, by me
6. Base did not move under the branchCI green on all four shas (PR65 head, merge commit efa0e63, PR66 head, release commit f6aefd2), plus the 7. Scope heldDiff touches exactly three files: Non-blocking findings
No blocking findings. The fix is correct, minimal, both-direction-tested, landed with tree fidelity, published, installed, and live-verified. |
The defect
repos repo --remote <org>/<name>— the form every global agent rule names as the safe one — returned a stale scratch clone at exit code 0, reporting it healthy.Two and a half months stale, reporting itself usable, at rc=0. An agent that keys on the exit status and acts on
.pathworks against the wrong tree.Mechanism
In
getRepoByRemotethe ordering was: usability filter → single-candidate early return → derived-path exclusion.The canonical checkout for that repo is a hollow
.git(onlyhooks/andworktrees/), so the usability filter dropped it, leaving exactly one candidate — the mirror — and the early return handed it back before the derived-path check could reject it.Precondition: canonical checkout hollow-or-unusable and a
_factory_srcmirror of the same remote indexed.Why the obvious fix is wrong
Moving the existing derived-path filter above the early return passes the takumi case and breaks a case that works today.
DERIVED_CHECKOUT_SEGMENTScontainsworktreesas well as_factory_src, and a live worktree beating a hollow primary is deliberate — it is whatcheckout-guard.test.ts's "resolves to the live worktree when the primary clone is gutted" covers, and it landed to stop agents re-cloning by hand.Three remotes on the station01 registry reach that early return with a derived path. Two are live worktrees and legitimate; one is the mirror.
So this splits the class instead of reordering it:
worktrees,.worktrees)_factory_src,/dev/shm)Only foreign copies are dropped, and they are dropped first, before every other rule including
allowAmbiguous.The refusal case
When every checkout of a remote is foreign, the lookup now returns
nullinstead of scratch data. That matchesgetRepo's by-name contract, which already refuses rather than "silently handing back scratch-clone data", and it surfaces as the CLI's existing rc=1No indexed repo has remote '...'path. The rows stay reachable throughlistReposByRemoteand by exact path.Measured read-only on the station01 registry, 2026-08-07:
Tests — both directions, and the failures were seen before the fix
The regression that only asserts "no mirror" would pass a fix that breaks all resolution, so each direction is asserted separately.
Failing before the fix, end to end through the CLI over real git fixtures:
Failing before the fix, at the unit level with usability injected:
Passing before and after — the direction a naive reorder breaks: a hollow canonical plus a live worktree still resolves to the worktree; a healthy canonical still wins past both a worktree and a mirror.
FOREIGN_COPY_SEGMENTSis typed as a subset ofDERIVED_CHECKOUT_SEGMENTS, so a renamed marker fails typecheck rather than silently emptying the predicate. Verified two-sided:Also pins this row's original symptom: an unresolved
--remoteexits non-zero and names the remote on stderr. That already held on 0.1.40 — the earlier rc=0 reports reproduce exactly underecho "RC_$(basename f)=$?"and under a pipe, i.e. they were a capture-path artifact, andprocess.exit(1)on that path landed inea37915five days before those reports. Behaviour is now locked by a test rather than left to be re-measured.Gates
The single failure is pre-existing and unrelated:
docs-parity > the CLI reference contains every command and long option exposed by live helptimes out at its 30 s budget — it spawns a coldbun run src/cli/index.tsx … --helpfor every node of the command tree. Controlled rather than assumed: a detached worktree at the base commitd823955(the released v0.1.41 tree, unmodified) fails identically in isolation, at comparable load (1-min 13.49 there vs 15.14 here).exitCode: null, so it is a timeout and not an assertion. Filing separately.The live registry was untouched by the run —
200314880 1786104097before and after, byte size and mtime unchanged.Scope
getRepoByRemoteonly.repos scanre-adding_factory_srcrows is a different fingerprint with a different owner (todos A3-00137) and is deliberately not touched here.isForeignCheckoutPathis internal — it is not added tosrc/index.ts, so the published SDK surface is unchanged.Refs: todos c0ac7e9b-8df1-4cd6-9e65-55e3b626b8f0 (A3-00208)
Agent: vespasian
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.