fix(repo): make the CI that runs actually do what it claims - #30
Merged
Conversation
Three inherited defects, all the same class — configuration that looks correct,
does nothing it claims, and fails silently.
**safe.directory was hardcoded to the upstream repository name.** commit.yml
granted the exception to /__w/spectral/spectral. This fork's workspace is
/__w/spotlight-tools/spotlight-tools, so the exception applied to a directory
that does not exist and every git invocation died with "detected dubious
ownership" (exit 128). That is the only genuine job failure in an otherwise green
matrix. Now ${{ github.workspace }}, so it survives the next rename too.
**The Actions caches have never worked.** The keys used CircleCI template syntax:
key: yarn-packages-{{ arch }}-{{ checksum "yarn.lock" }}
GitHub Actions does not interpolate that; it is a literal string, which the
runner log confirms by reporting a miss against the literal text. Two effects,
the second worse than the first: every run re-downloaded the whole dependency
tree, and because the key was constant, once a cache saved under it every later
run would restore that same cache regardless of what yarn.lock said. A dependency
cache that cannot tell one lockfile from another is a reproducibility problem,
not a performance one. Now hashFiles('yarn.lock') with runner.os and runner.arch,
and a prefix restore-key so a changed lockfile still gets a warm start.
The harness actions had the same defect, keyed on a SHA written to /tmp by a
git rev-parse step. That step is deleted rather than repaired: it existed only to
build the broken key, and it was also the thing failing on safe.directory. The
cache now keys on github.sha with a prefix restore-key, which expresses the same
intent without shelling out to git at all.
**test-windows could never fire.** It carried `if: github.ref ==
'refs/heads/develop'` — a develop remnant the sweep missed. No such branch exists
here, so Windows was skipped on every run in 0s, which in a run summary reads
much like passing. It would not have fired on pull requests upstream either,
since github.ref is refs/pull/N/merge on that event.
Its harness step also read `if: matrix.node-version == '18.20.8'` in a job with
no matrix, so the expression was always empty and the step would have been
skipped even had the job run. Pinned to 18.20.8 to match the Linux harness.
spectral is a cross-platform tool with Windows-specific path handling and a
dedicated Windows harness action. This is the first run in this fork's history
where any of it is exercised, so a Windows failure here is a finding rather than
a regression, and belongs on #3.
Closes #29
Refs #3, #15, #16
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kinlane
added a commit
that referenced
this pull request
Aug 3, 2026
Three inherited defects, all the same class — configuration that looks correct,
does nothing it claims, and fails silently.
**safe.directory was hardcoded to the upstream repository name.** commit.yml
granted the exception to /__w/spectral/spectral. This fork's workspace is
/__w/spotlight-tools/spotlight-tools, so the exception applied to a directory
that does not exist and every git invocation died with "detected dubious
ownership" (exit 128). That is the only genuine job failure in an otherwise green
matrix. Now ${{ github.workspace }}, so it survives the next rename too.
**The Actions caches have never worked.** The keys used CircleCI template syntax:
key: yarn-packages-{{ arch }}-{{ checksum "yarn.lock" }}
GitHub Actions does not interpolate that; it is a literal string, which the
runner log confirms by reporting a miss against the literal text. Two effects,
the second worse than the first: every run re-downloaded the whole dependency
tree, and because the key was constant, once a cache saved under it every later
run would restore that same cache regardless of what yarn.lock said. A dependency
cache that cannot tell one lockfile from another is a reproducibility problem,
not a performance one. Now hashFiles('yarn.lock') with runner.os and runner.arch,
and a prefix restore-key so a changed lockfile still gets a warm start.
The harness actions had the same defect, keyed on a SHA written to /tmp by a
git rev-parse step. That step is deleted rather than repaired: it existed only to
build the broken key, and it was also the thing failing on safe.directory. The
cache now keys on github.sha with a prefix restore-key, which expresses the same
intent without shelling out to git at all.
**test-windows could never fire.** It carried `if: github.ref ==
'refs/heads/develop'` — a develop remnant the sweep missed. No such branch exists
here, so Windows was skipped on every run in 0s, which in a run summary reads
much like passing. It would not have fired on pull requests upstream either,
since github.ref is refs/pull/N/merge on that event.
Its harness step also read `if: matrix.node-version == '18.20.8'` in a job with
no matrix, so the expression was always empty and the step would have been
skipped even had the job run. Pinned to 18.20.8 to match the Linux harness.
spectral is a cross-platform tool with Windows-specific path handling and a
dedicated Windows harness action. This is the first run in this fork's history
where any of it is exercised, so a Windows failure here is a finding rather than
a regression, and belongs on #3.
Closes #29
Refs #3, #15, #16
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #29. Refs #3, #15, #16.
Stacked on #28 — that PR unblocks
yarn install, so this one is based on it in order to get a CI run that actually reaches these fixes. Merge #28 first and GitHub will retarget this tomainautomatically.Three inherited defects, all the same class: configuration that looks correct, does nothing it claims, and fails silently.
1.
safe.directorywas hardcoded to the upstream repository nameA rename artifact from #2. The exception was granted to a path that does not exist here, so every
gitinvocation died withdetected dubious ownershipand exit 128 — the only genuine job failure in an otherwise green matrix. Usinggithub.workspacemeans it survives the next rename too.2. The Actions caches have never worked
The keys were CircleCI syntax:
{{ arch }}and{{ checksum "yarn.lock" }}. GitHub Actions does not interpolate those, and the runner log confirms the miss against the literal text.The performance cost is the obvious half. The correctness cost is worse: the key was constant, so once a cache saved under it, every later run would restore that same cache no matter what
yarn.locksaid. A dependency cache that cannot distinguish one lockfile from another is a reproducibility problem — relevant to #15, not just to build times.Now
hashFiles('yarn.lock')withrunner.osandrunner.arch, plus a prefixrestore-keysso a changed lockfile still gets a warm start rather than a cold one.3.
test-windowscould never firetest-windows: - if: github.ref == 'refs/heads/develop'Another
developremnant #16 missed. Windows was skipped every run in0s— which in a run summary reads a lot like passing. It would not have fired on pull requests upstream either, sincegithub.refisrefs/pull/N/mergeon that event.Underneath it, the harness step read
if: matrix.node-version == '18.20.8'in a job with no matrix, so the expression was always empty and the step would have been skipped even if the job had run. Pinned to18.20.8to match Linux.On the harness actions
I deleted the
git rev-parsestep rather than repairing it. It existed only to write SHAs to/tmpfor the broken cache key, and it was simultaneously the thing failing onsafe.directory. The cache now keys ongithub.shawith a prefix restore-key, which expresses the same intent without shelling out to git at all — one less thing to go wrong on a fresh clone.What to expect from this run
spectralis a cross-platform tool with Windows-specific path handling and a dedicated Windows harness action, and nothing has verified any of it in this fork's history. This is the first run where it is exercised.If Windows fails here, that is a finding and not a regression, and it belongs on #3 — which currently records Windows as unverified. I would rather learn that now than keep a job that reports
0sand looks fine.Two acceptance criteria on #29 stay open until this run reports: confirming a cache hit appears, and checking whether a stale cache is already pinned under the old constant key and needs purging.