Skip to content

#27 fix: check out skills-vendor/ in the test job - #30

Merged
gregoryfoster merged 1 commit into
mainfrom
27-ci-checkout-submodules
Aug 7, 2026
Merged

#27 fix: check out skills-vendor/ in the test job#30
gregoryfoster merged 1 commit into
mainfrom
27-ci-checkout-submodules

Conversation

@gregoryfoster

Copy link
Copy Markdown
Contributor

Fixes #27.

tests/test_skills_hook.py::test_the_hook_the_symlink_points_at_actually_exists
dereferences the .claude/hooks/ symlink into skills-vendor/. actions/checkout
does not fetch submodules by default, so the link dangled on the runner and the test
could not pass in CI at allmain red since 7bf5988, 443 of 444.

Change

One key on the test job's checkout, plus the comment explaining why it is load-bearing.

  • true, not recursive — neither submodule carries a .gitmodules, so recursion
    only adds a foreach pass to checkout and post-job cleanup.
  • test job only — ruff never reads skills-vendor/; fetching two submodules to
    run a linter is cost for nothing.
  • No auth change — both remotes are public HTTPS in .gitmodules. No token, no WIF,
    no wheelhouse impact.

Also widens the assertion message, which named only .skills/doctor.sh — developer-machine
advice that reads as nonsense on a runner.

Correcting the issue's evidence table

The third row is misattributed. Run 31117568515
(Merge #26) failed at Set up job with Failed to resolve action download info. Error: Service Unavailable — yesterday's Actions outage. It never reached checkout, let alone
pytest, and its lint job was cancelled at 15m rather than failed. Two runs share this
cause, not three; afb1f97 would have failed the same way, but that was never observed.

On the #22 v1.2 hold

Unaffected in both directions. The suspended refresh is a developer-machine SessionStart
hook CI never invokes, and checkout resolves the SHA pinned in this superproject rather
than upstream tip — so the pin the hold froze stays frozen.

Verification

  • Local: 444 passed, 18 deselected (was 443 + 1 failed in CI).
  • test green with 444, not 443 — the second assertion is os.access(HOOK, os.X_OK), and
    the target is mode 100755 in the skills index.
  • lint untouched and still green.
  • Runtime unchanged in practice: 2.0M + 2.2M working trees at depth 1, against a ~1m10s
    baseline.

Coupling accepted knowingly

The test job now depends on two third-party public repos being fetchable at checkout.
An upstream force-push that GCs a pinned SHA would fail the job at checkout — an error
that looks nothing like a test failure. Noted in the workflow comment so the next person
reading a checkout failure knows where to look.

🤖 Generated with Claude Code

tests/test_skills_hook.py dereferences the .claude/hooks/ symlink into
skills-vendor/, so it needs a populated submodule, not merely a
correctly-shaped link. actions/checkout does not fetch submodules by
default, so the link dangled on the runner and the test could not pass in
CI at all — red on main since 7bf5988, 443 of 444.

`true` rather than `recursive`: neither submodule nests, so recursion only
adds a foreach pass. Scoped to the test job — ruff never reads
skills-vendor/. Both remotes are public HTTPS, so this needs no token and
does not touch the wheelhouse WIF auth.

Unrelated to the #22 v1.2 hold in both directions: the suspended refresh is
a developer-machine SessionStart hook CI never invokes, and checkout
resolves the SHA pinned here rather than upstream tip, so the pin the hold
froze stays frozen.

The assertion message named only the local remedy, which reads as nonsense
on a runner; it now names both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gregoryfoster
gregoryfoster merged commit e476683 into main Aug 7, 2026
2 checks passed
@gregoryfoster
gregoryfoster deleted the 27-ci-checkout-submodules branch August 7, 2026 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: main red since #16 — the test job never checks out skills-vendor/, so test_skills_hook dereferences a dangling symlink

1 participant