fix(validation): support nested Git workspace roots - #298
Conversation
|
For context, this surfaced while using Opcore in a monorepo where validation was intentionally scoped to one nested workspace rather than the entire Git worktree. In that configuration, Opcore workspace paths are relative to the selected directory, while some Git operations require paths relative to the worktree root. Git listing commands also behave differently depending on the current directory and whether an explicit pathspec is provided. The result was a mismatch: before-state file reads could fail, and unrelated sibling changes could enter an otherwise scoped validation run. The fix preserves workspace-relative paths at the validation boundary, translates them only when accessing Git objects, and explicitly limits Git discovery to the selected workspace. This should support incremental or path-scoped Opcore adoption in monorepos without depending on any particular repository layout. The basic root-level workflow is unchanged: when the selected repo is the Git worktree root, the prefix is empty and these operations retain their existing behavior. The observable change is limited to nested |
EivMeyer
left a comment
There was a problem hiding this comment.
Reproduced this before looking at the patch. Two-package workspace, git root above both, check --changed --repo packages/app:
status: infrastructure_failure exitCode: 1 checks run: 0
cause: fatal: path 'packages/app/src/index.ts' exists, but not 'src/index.ts'
Zero checks run, so this isn't an edge case — opcore just doesn't work below the worktree root.
Patch applies clean to main too (workspace.ts is identical on main and dev). After it: 25 checks run, the app's TS2322 lands at src/index.ts, the sibling package's error is correctly not reported, --repo . at the root still reports both with full paths, and --staged works in the nested case.
Full suite on my machine: 948 tests, one failure in maps pyright timeout, signal, and spawn failures. That one fails identically on clean main and passes in isolation both patched and unpatched, so it's a load flake, not yours. Focused adapter suite is 27/27, matching your proof.
Two nits, neither blocking:
- the
-- .pathspec onlistRepoFiles/listTreeSnapshotFilesis a no-op at the root and the existing tests cover it, but it's the one place behavior could shift for people already running at the worktree root - the new test's temp prefix is
lattice-..., the legacy name we purged in #110 — it matches the rest of the file, so take it or leave it
CI never ran because fork PRs need a manual approve. That's on us, not you — approved now, merging once it's green. Thanks for the clean repro and the test.
|
Merged. Thanks again for this one. It's queued to ship in 0.2.2 — bump is in #301, and the release notes credit you. Publishing goes out once that promotion lands on main. If you hit anything else in that monorepo setup, file it. You're the first person outside the team to send a patch here and the repro quality made it easy to verify. |
Summary
Proof
Degraded coverage