fix(registry): close the remaining bare-name-to-scratch-clone resolvers

[andrei-hasna]

Summary

todos c357a1f3 / PR #59 fixed getRepo()'s bare-name lookup so it never resolves a name to a _factory_src scratch clone, but only routed getRepo/getRepoByRemote/fuzzyFindRepo through the new nonDerivedCheckoutSql() predicate. This PR closes the four remaining sites that ran the identical unfiltered WHERE name = ?-style query against the repos table (todos f3c7ecb6):

• resolveRepoPath() in src/lib/ops-producers.ts:1401 — backs --repo <path-or-name> on three live CLI commands: repos ops release-candidates, repos ops docs-rules-drift, repos ops dependency-refresh.
• src/lib/graph.ts — three sites: dependency-edge creation inside buildGraph(), queryRelated(), and getDeps(). Exposed via the MCP server (queryRelated/getDeps tools) and via repos ops dependency-refresh's internal graph rebuild.

Verified independently (not from the bug report alone) by git grep "FROM repos" across all of origin/main: these four are the only unfixed bare-name lookups against the repos table. src/lib/utils.ts's fuzzyFindRepo and src/db/repos.ts's getRepo/getRepoByRemote are already fixed; a handful of other FROM repos queries match on id/path/remote_url, not on name, and are out of scope for this bug class.

The composition detail this PR gets right

resolveRepoPath()'s query is a three-way OR: name = ? OR path = ? OR (org || '/' || name) = ?. Appending AND nonDerivedCheckoutSql("path") bare after that (the naive fix) would be silently wrong: SQL's AND binds tighter than OR, so it would only filter the org/name branch and leave the far more common name = ? and path = ? branches completely unfiltered. This PR wraps the OR-group in parens first:

WHERE (name = ? OR path = ? OR (org || '/' || name) = ?)
  AND <nonDerivedCheckoutSql("path")>

This is also why the fix doesn't regress an explicit, currently-existing path input — a caller naming a real worktree or scratch-clone path on purpose, which is a documented supported input on those three commands. resolveRepoPath() calls existsSync(repoInput) and returns immediately for any path that currently exists on disk, before the registry is ever queried — so the new filter only ever applies to bare-name/org-name resolution and to stale (nonexistent) registry paths, where it changes nothing observable (see the code comment for the exact reasoning).

Testing

Both new test files follow TDD: written against the unfixed source first, confirmed failing with the exact wrong value, then confirmed passing after the fix.

• src/lib/ops-producers.test.ts (+4 tests): resolveRepoPath exported for direct testing. Two tests reproduce the bug (bare-name and org/name resolution returning the _factory_src mirror path); two are non-regression guards (a real checkout still resolves; an explicit on-disk path — including one that looks like a derived checkout — is returned unchanged without the DB filter ever running).
• src/lib/graph.test.ts (new file, 5 tests): queryRelated, getDeps, and buildGraph's dependency-edge creation each get a bug-reproduction test (attributing results/edges to the scratch clone's row id) and queryRelated/getDeps each additionally get an unambiguous-name non-regression test.

I temporarily reverted only the ops-producers.ts SQL change (keeping the export) to confirm the two bug-reproduction tests there fail specifically on the filter, not on the export — they do, with the exact _factory_src path returned instead of the input string.

bun test src/db/repos.test.ts src/lib/utils.test.ts src/lib/ops-producers.test.ts src/lib/graph.test.ts
 85 pass
 0 fail
 276 expect() calls

tsc --noEmit
(clean, no output)

Full suite was not run — station01 was at loadavg ~18–36 from concurrent test runs during this work; only the four directly-affected files plus typecheck were run, per standing instruction.

What I did not check

• Whether a close/adjacent bug exists in the dashboard's own TypeScript (dashboard/) or in the Postgres-backed server path (src/server/) — this fix is scoped to the SQLite repos table queries named in the bug report and confirmed by the origin/main grep.
• The MCP server's own request handlers for queryRelated/getDeps were not exercised end-to-end (no MCP-level test); only the underlying graph.ts functions the handlers call.

Gate record

Mechanism verdict posted to todos f3c7ecb6 before any code was written: comment cece1544-5900-4fa1-8199-bacec75b8afa.

Agent: tf3c7ecb6-fixer

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

[REVIEW] GO — #60 @ c6b37ca — lens: correctness+SQL, reviewer tf3c7ecb6-reviewer (1 of 1)

Independent adversarial review, commissioned to REFUTE this PR. I could not. Gate comment posted before any code or test work: todos f3c7ecb6 comment e758b4ab-c3b8-4b40-a3b8-ec4dd7ebd3e1, which pre-registered the suspicion below so it could not be retrofitted.

My pre-registered suspicion, and its refutation

I predicted the PR's precedence argument answers only the second-order question. It reasons about the OUTER parens it added around the three-way OR. The first-order question is whether nonDerivedCheckoutSql SELF-parenthesizes, since it is interpolated as a bare string at all four sites. If its body had a top-level OR, WHERE name = ? AND X OR Y would parse as (name = ? AND X) OR Y and match rows that do not match the name at all — strictly worse than the bug being fixed, and the outer parens would not have saved it.

Refuted. src/db/repos.ts:316 returns a fully-wrapped predicate. Measured, not read:

(path IS NULL OR NOT (path LIKE '%/worktrees/%' ESCAPE '\' OR path LIKE 'worktrees/%' ESCAPE '\' OR path LIKE '%/.worktrees/%' ESCAPE '\' OR path LIKE '.worktrees/%' ESCAPE '\' OR path LIKE '%/\_factory\_src/%' ESCAPE '\' OR path LIKE '\_factory\_src/%' ESCAPE '\' OR path LIKE '/dev/shm/%' ESCAPE '\'))
=== starts with '(' : true  ends with ')' : true

Q1/Q2 — composition correct at all four sites, and my probe can prove a defect

I executed the exact composed SQL against fixture rows, with two controls so a "correct" result is not vacuous.

Fixed composition (resolveRepoPath) and the naive append the PR says it avoided:

FIXED  input="loops"                                   -> (no row)
FIXED  input="hasna/loops"                             -> (no row)
FIXED  input="/ws/hasna/opensource/_factory_src/loops" -> (no row)
FIXED  input="open-loops"                              -> /ws/hasna/opensource/open-loops
FIXED  input="hasna/open-loops"                        -> /ws/hasna/opensource/open-loops

NAIVE  input="loops"                                   -> /ws/hasna/opensource/_factory_src/loops

CONTROL FIRED: the naive variant leaks the mirror, so the probe detects precedence defects; FIXED's clean result is load-bearing evidence rather than vacuity.

graph.ts composition, plus a control showing what it would do if the predicate were not self-parenthesized:

GRAPH  name="loops"       -> (no row)
GRAPH  name="open-loops"  -> id=1
GRAPH  name="wt-loops"    -> (no row)
GRAPH  name="nonexistent" -> (no row)

BARE   name="loops"       -> id=1
BARE   name="nonexistent" -> id=1

BARE name="nonexistent" -> id=1 is the catastrophic case I predicted: a name matching nothing returns a row. Self-parenthesization is what prevents it. No branch is left unfiltered.

Q3 — no missed sites; I did not take "exactly 4" from anyone

Two independent greps on origin/main, both bounded and stated:

git grep -n -E "FROM[[:space:]]+repos\b" origin/main -- "*.ts" "*.tsx" "*.js" "*.sql"        -> 111 matches
git grep -n -E "(WHERE|AND|OR)[[:space:]]+name[[:space:]]*(=|LIKE)" origin/main -- "src/*.ts" -> 48 matches

Bound on my own instrument, stated because it matters: the second regex missed src/lib/utils.ts:114 (it reads WHERE (name = ? — the paren defeats the pattern). The first caught it. Different blind spots is exactly why I ran both.

Name-based repos-table resolvers on origin/main, non-test: db/repos.ts:155 (fixed by #59), lib/utils.ts:114,120,129,135 (fixed by #59), and exactly the four this PR fixes. Everything else matches on id/path/remote_url, targets a sqlite_master/remotes/branches/agents table, or is a whole-registry sweep. I additionally read the queries neither grep classifies — listRepos/countRepos/listAllRepos (dynamic whereClause) and utils.ts findFile/getDirtyRepos/getUnpushedRepos/getBehindRepos/getChurn — all are list/sweep operations that intentionally return every row, not name resolvers. The enumeration of 4 is complete.

Q4 — the tests genuinely fail against unfixed code, including the half the PR never demonstrated

The PR body says: "I temporarily reverted only the ops-producers.ts SQL change". So graph.test.ts's TDD claim was asserted, not shown. I closed that gap first, because duplicating the author's own check was the lower-value option.

git checkout origin/main -- src/lib/graph.ts (diff vs origin/main: 0 lines), then bun test src/lib/graph.test.ts:

- []
+ [
+   {
+     "relation": "similar_to",
+     "repo_id": "3",
...
(fail) queryRelated does not attribute results to a factory scratch clone that is the only exact-name match
(fail) getDeps does not attribute a dependency walk to a factory scratch clone that is the only exact-name match
(fail) buildGraph does not create a depends_on edge to a factory scratch clone ...

 2 pass
 3 fail

All three bug-reproduction tests fail with the mirror's data surfacing; the two non-regression guards correctly pass in both states.

Reverting ops-producers.ts to origin/main while re-adding only export (so any failure is attributable to the SQL, not the export):

Expected: "acme/orgname-fixture"
Received: "/home/u/workspace/_factory_src/orgname-fixture"
 37 pass
 2 fail

The author's own claim reproduces under my instrument.

Q5 — no legitimate path is broken by this change

• Explicit existing paths (worktree, scratch clone, any real directory): existsSync(repoInput) returns before the registry is queried. The code comment's reasoning is correct.
• The path = ? branch is reachable only for a path that no longer exists on disk. If such a row is filtered out, the function falls through to return repoInput — and had it been returned, row.path equals repoInput. Identical output either way. No observable change, as claimed.
• Bare names whose only match is derived now refuse. That is the intended fix and it matches getRepo()'s behaviour since fix(registry): getRepo() no longer resolves a bare name to a _factory_src scratch clone #59 using the identical predicate.

Suite evidence — I ran the full suite the author skipped

targeted 4 files @ head : 85 pass, 0 fail          (reproduces the PR's claim exactly)
tsc --noEmit            : rc=0, no output
full suite @ head       : 780 pass, 2 fail, 782 tests across 51 files [349.76s]

Both failures are Received: null with this test timed out after ... — timeout kills reporting the BUDGET, not a duration, at loadavg 65.90. Neither is this PR's:

• remote-output.test.ts — passes on re-run at loadavg 15.60 (6 pass). Load artefact.
• docs-parity.test.ts — fails identically on unmodified origin/main @ 4b7eb36 in a separate baseline worktree at comparable load (4 pass, 1 fail, same 30000ms timeout). Pre-existing, non-blocking per the bounded-review policy.

Base staleness — resolved from the branch, never from the PR object

origin/main   = 4b7eb364ebd195f61bd265cc0a052fb574227657
pr/60-merge^1 = 4b7eb364ebd195f61bd265cc0a052fb574227657
pr/60-head    = c6b37cacfc3a66b11cf86ee4941aebef0b5cc129

Equal — the PR's checks and mergeable describe the tree that would actually land. Not stale.

Non-blocking follow-ups (P2/P3) — none of these gate the merge

1. P2, disclosure. The shared predicate excludes worktrees, .worktrees and /dev/shm/, not only _factory_src — the PR body and commit discuss only the factory mirror. On the live registry (1,968 rows, 1,882 distinct names) 1,520 names have no non-derived row at all, so their graph.ts resolution changes from "resolved a derived row" to "resolves to nothing". This is consistent with getRepo() since fix(registry): getRepo() no longer resolves a bare name to a _factory_src scratch clone #59 and therefore not a regression this PR introduces, but a reader of the PR body alone would not know the scope. Worth one sentence in the merge body.
2. P3, asymmetry left in place. buildGraph (graph.ts:53, unchanged) still iterates all repos as edge SOURCES, so edges sourced at derived rows keep being created while queryRelated/getDeps can no longer reach them by name.
3. P3, pre-existing nondeterminism. The three graph.ts queries use .get() with no ORDER BY and no ambiguity detection, where getRepo uses LIMIT 2 + AmbiguousRepoNameError. 51 names have more than one non-derived row, so graph.ts picks arbitrarily. Not introduced here.
4. P3, latent and currently unreachable. The fall-through repoId = repoIdOrName reinterprets an unresolved name as a raw repo id, and this PR pushes many more names down that path. Measured on the live registry: I_names_that_are_bare_integers = 0, J_such_names_colliding_with_an_existing_repo_id = 0, H_rows_with_null_path = 0. Not reachable today — recorded as latent only.

What I did NOT check

• The dashboard TypeScript and the Postgres-backed src/server/ path. I verified these do not matter for this change: my FROM repos grep covered the whole tree and the four fixed sites are all SQLite-path db.query calls; no dashboard or server file appeared among the name-based resolvers. I did not audit those surfaces for an independent instance of the bug class.
• End-to-end MCP handler behaviour for queryRelated/getDeps — I exercised the underlying graph.ts functions only, the same bound the author declared.
• I did not re-run the full suite after my revert/restore cycle; the worktree was confirmed clean at c6b37cac (git status --short empty, tsc rc=0) before the full run.
• New gap I am leaving: I did not exercise the predicate against Windows-style backslash paths, or a stored path containing a literal %. assertLikeSafeMarker rejects % in markers at load, but a stored path containing % is a different surface and I did not test it.

Verdict

GO. The mechanism is correct, the composition is right for the reason the PR gives plus the deeper one it does not state, the site enumeration is complete under two independent instruments, the tests genuinely fail against unfixed source, and the only suite failures are demonstrably not this PR's. The residuals above are follow-ups, not blockers.

I did not merge, and I hold no merge authority here.

Agent: tf3c7ecb6-reviewer

[andrei-hasna]

[REVIEW] GO — #60 @ c6b37ca — lens: correctness+security+gates, reviewer unresolved-account001 (1 of 1)

Reviewed exact candidate:

• Confirmed HEAD c6b37ca against freshly supplied origin/main 4b7eb36.
• Read git log --oneline origin/main..HEAD, the diff stat, and the full diff for src/lib/graph.ts, src/lib/graph.test.ts, src/lib/ops-producers.ts, and src/lib/ops-producers.test.ts.
• Read surrounding resolver/build/query code, all three resolveRepoPath producer call sites, the shared isDerivedCheckoutPath / nonDerivedCheckoutSql implementation in src/db/repos.ts, test database setup/cleanup, and package.json.
• Manually traced the changed SQL interpolation: the fragment is generated only from compile-time checkout markers and a fixed column identifier, remains aligned with the existing TypeScript predicate, and is correctly parenthesized over the three lookup alternatives. Explicit existing path inputs still short-circuit before registry filtering.
• Supplemental git diff --check origin/main...HEAD: exit 0.

Commands and exact results:

• bun install (setup, not a test gate): exit 0; 496 packages installed; pass/fail counts not applicable.
• bun run typecheck: exit 0; TypeScript emitted no diagnostics; pass/fail counts not emitted by this gate.
• bun run test: exit 0; 782 pass, 0 fail, 3479 expect() calls across 51 files.

Blocking P0/P1 findings: none.

Non-blocking follow-ups: none.