Files
msd-core/gsd-core
Tom Boucher 07de60523c fix(#2657): untrack the nine ADR-457 migration-gap compiled artifacts (#3011)
* test(#2657): add failing-first regression for tracked bin/lib compiled artifacts

Nine gsd-core/bin/lib/*.cjs artifacts are tracked in git despite having
src/*.cts sources, violating ADR-457's build-at-publish contract. This
regression test asserts the ADR-457 end state (untracked, gitignored,
empty-set reported by the #2656 sync guard) and fails until the tracking
is fixed.

* fix(#2657): untrack the nine ADR-457 migration-gap compiled artifacts

Nine gsd-core/bin/lib/*.cjs artifacts (api-coverage, assumption-delta,
claude-orchestration, claude-orchestration-command-router, external-job,
markdown-table, runtime-artifact-install-plan, state-transition,
write-set) were tracked in git despite each having a matching src/*.cts
source, letting the committed bytes drift silently from source (#2653
demonstrated this for api-coverage.cjs).

Seven had no .gitignore entry at all; two (markdown-table.cjs,
write-set.cjs) had a pattern added by #2248 but were never
git rm --cached. Both gaps produce the same tracked-file symptom.

Untracks all nine and adds the seven missing .gitignore entries next to
their two siblings, reaching ADR-457's end state: bin/lib/*.cjs is a
gitignored build artifact built via prepare/pretest/prepublishOnly, never
checked-in source of truth. The #2656 artifact-sync guard is
regime-agnostic by design and needed no code change; it now reports the
empty-set end state.

* test(#2657): consolidate repeated still-tracked/unmatched assertion shape

Code-review finding (Standards axis, Duplicated Code): the three
'none of the nine should still be in bad state X' checks shared an
identical filter-then-assert-empty shape. Extracted assertNoneStillBad()
as a shared helper; behavior is unchanged.

* fix(#2657): make the .gitignore-match assertion existence-independent

The regression test's check-ignore assertion used --no-index, which
locally exercises the pattern correctly but was reported failing on
gsd-test's fresh shallow clone. Switched to plain 'git check-ignore -q'
(no --no-index): verified via a real git worktree checkout at both
origin/next (fails: all nine report not-ignored, since check-ignore
correctly special-cases the still-tracked pre-fix state) and this
branch's tip (passes: all nine report ignored). Plain check-ignore is
also semantically stronger than --no-index here, since it honors the
'a tracked path is never reported ignored' rule that --no-index
bypasses -- exactly the property under test for the two paths whose
.gitignore pattern predates this fix (#2248) but were never untracked.

Also reconciled the .gitignore comment: it previously read 'these
seven' beside seven new lines with no indication of the other two (of
nine total) that already had a pattern from #2248. Annotated both
groups so the count is unambiguous at the point of the diff.

* test(#2657): make every git invocation self-diagnosing

b6f915bc0 failed in the runner with a shape that turned out not to be
about .gitignore content or the merge: two of the five failures in this
file were 'Command failed' / 'Got unwanted exception' -- git itself
erroring, not answering. The old code used execFileSync + try/catch,
which conflates 'git said no' with 'git could not run' -- both looked
like the same negative result to the test, exactly the failure mode
that produced 'unmatched: <all nine>' twice on two different
assertions for two different reasons.

Switched every git invocation to spawnSync (never throws) and made
every assertion check the exit status explicitly before interpreting
output:
  - git ls-files: must exit 0, or the assertion fails loud with cwd,
    exit status, and stderr instead of silently reading an error as
    'nothing tracked'.
  - git check-ignore -q: only exit 0 (ignored) and exit 1 (not
    ignored) are legitimate answers per check-ignore(1); any other
    status is now a thrown infrastructure failure, never read as
    'not ignored'.
  - trackedCompiledArtifacts(): a thrown error is now reported as
    what it is (its internal git call failed), not swallowed into a
    'still tracked' verdict.
  - the sync-guard subprocess check now reports cwd/stderr/stdout on
    a non-zero exit instead of a bare doesNotThrow.

This is a genuine, independent test defect (a test that reads a
failed command's empty output as a meaningful answer can pass or fail
for the wrong reason) as well as the mechanism for finally surfacing
why b6f915bc0 failed in the runner: the next run's assertion messages
will show the resolved cwd and git's actual stderr instead of an
opaque 'unmatched: <all nine>'.

* fix(#2657): trust the repo root for git calls under dubious-ownership

Root cause of the failing runner verdict, harvested from the
diagnostics commit: every git invocation in the container exits 128
with 'fatal: detected dubious ownership in repository at /work' --
the checkout there is owned by a different uid than the process
running the tests, and git refuses to operate at all. The old
assertions read that hard failure as 'not ignored' / 'still tracked',
producing the all-nine symptom seen on both b6f915bc0 (plain
check-ignore) and 34052f836 (--no-index). Nothing was ever wrong with
the untracking, the .gitignore content, or the merge -- confirmed by
exhaustive local reproduction (git worktree, real shallow clone, the
runner's exact clone+checkout+merge sequence from its own Go source)
that could never surface the bug because this machine owns its own
checkouts.

Fixed at both git() call sites in this exact seam by passing
'-c safe.directory=<repo root>' per-invocation (never written to any
config file, so trust is scoped to the single call):
  - tests/fix-2657-untrack-compiled-artifacts.test.cjs
  - scripts/lint-compiled-artifact-sync.cjs -- a SHIPPED script with
    the identical defect (its own git ls-files failed the same way in
    the same run), which would fail identically for any containerized
    CI lane whose checkout uid differs from the running user, not just
    this branch. Folded in under the no-defer rule rather than filed
    separately, since it sits in the exact tracked-compiled-artifact
    guard this issue is about.

The status-code guards added in 31858818e stay in place -- they are
what turned an unexplainable 'unmatched: <all nine>' into a one-line
diagnosis, and they must keep any future infrastructure fault from
silently reading as a substantive result.

* chore(#2657): backfill changeset PR number to 3011

* chore(#2657): backfill changeset PR number to 3011

---------

Co-authored-by: sim <sim@local>
2026-08-02 21:22:43 -04:00
..