[kyle-sexton]

Closes #6493

Summary

Every vendored skill's drift-check script now exits 0 for no drift, 1 for upstream drift and 2 for unverifiable. The weekly maintenance-check-upstream-drift workflow uses that one rule for all eight rows. Before this change it needed a separate drift rule for each row, because dometrain swapped 1 and 2, context7 exited 0 on drift, and firecrawl exited 1 when its CLI was not installed.

Fix

• dometrain sync (update.sh): drift now exits 1 (it was 2) and unverifiable exits 2 (it was 1). A missing baseline counts as drift, and an unknown mode exits 2.
• context7 lookup (update.sh): drift now exits 1 (it was 0) and a missing tool exits 2 (it was 1). A ctx7 that is not installed is reported but no longer counted as drift. When both happen in one run, drift wins over unverifiable, as in the user-interface drift scripts. A failed git ls-remote now marks the skill checks unverifiable instead of exiting early.
• firecrawl update (update.sh --check): a CLI that is not installed is reported and no longer counts as drift. recorded_field now reads the whole value after Label: and treats placeholders such as never and (none recorded) as empty, so the report prints the recorded last-sync date and SHA256 instead of recorded).
• firecrawl, after the pre-ready code review (8f5e9fa): a failed npm-registry read no longer cuts the upstream SKILL.md comparison short, so drift found there still exits 1. recorded_field also trims trailing whitespace.
• playwright, boris, skill-authoring: their scripts already followed the contract. Their tests now pin run_check's 0/1/2 results.
• Workflow: removed the drift/unverifiable matrix keys and the exit:/line:/text: matcher. Exit 1 fails the run, exit 2 warns, and any other code fails the run as outside the contract.
• The dometrain and context7 skill bodies now state the exit codes. docs/upstream/good-css.md no longer names the removed matrix keys.
• Changelog fragments: firecrawl patch, dometrain and context7 minor (a consumer-visible exit-code change), playwright and playbooks none (test-only).

Verification

• New network-free tests: plugins/context7/skills/lookup/scripts/update.test.sh (20 checks) and plugins/dometrain/skills/sync/scripts/update.test.sh (14 checks). Each runs the script from a copied skill tree with a PATH limited to the tools it needs, plus stubs for curl, npm, git and ctx7.
• Extended tests: firecrawl (38 checks), playwright (28), boris (42), skill-authoring (23). All pass, and so do check-good-css-drift.test.sh and check-emilkowalski-drift.test.sh.
• Run against the pre-change scripts, the new context7 test fails 6 checks, the new dometrain test fails 4, and the new firecrawl cases fail 6 (the recorded) misread and the missing-CLI exit 1).
• scripts/check-affected.sh --tests: 8 passed, 1 skipped (ruff, no .py files). shellcheck --rcfile .shellcheckrc passes on every changed script, actionlint passes on the workflow, and scripts/check-changelog-fragments.sh --check passes.
• Left out: a live run of the weekly workflow. It runs on schedule or dispatch only, and these tests make no network calls.

Related

• feat: benchmark browser CLIs, add a weekly upstream drift check, and tighten the playwright skill #6477 added the weekly drift workflow and the per-row rules removed here.
• chore: sync four vendored skills that have drifted from upstream #6492 (syncing the four drifted skills) is separate content work.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AQ9amjLiLKnwj4WGMRcofR

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-18T10:51:22.510917Z	8f5e9fa	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-review reviewed 8f5e9fa2aff00093a3d0062a883094dfb91894c0: 0 finding(s), 0 as inline comments.

No blocking findings. I read the full diff, the PR context and #6493, and the unchanged parts of the context7 and firecrawl scripts. I did not run any of the tests; the test counts in the PR body are author-claimed and unverified by me.

What I checked

• Closing issue #6493. Each criterion is met:
  • The context7, dometrain and firecrawl scripts now follow the 0/1/2 contract. Playwright, boris and skill-authoring already did.
  • Each of the six has a test that pins the exit codes.
  • The workflow matrix now uses one rule for every row, and an exit code outside 0/1/2 fails the run.
• Firecrawl baseline read. recorded_field in plugins/firecrawl/skills/update/scripts/update.sh matches the real upstream.md lines (- Last sync: 2026-10-11, - Upstream SHA256: …). It treats never and (none recorded) as empty. A failed registry read or SKILL.md fetch no longer cuts the check short, and drift still outranks unverifiable.
• Context7 control flow. Both FIND_DOCS_RC and CLI_SKILL_RC are set on every path, so set -u is safe. A failed git ls-remote now ends in exit 2 unless drift was also found. CLI_DRIFT defaults to 0 before the summary.
• Dometrain. Every exit 1 and exit 2 was swapped. An unknown mode now exits 2.
• Workflow. The case "$code" block is correct under set -e, which was switched off only around the script call. docs/upstream/good-css.md no longer names the removed matrix keys.

Minor, not worth blocking

• plugins/context7/skills/lookup/scripts/update.sh: with --refresh-baseline and an installed ctx7 that is behind latest, the script sets CLI_DRIFT=1 and exits 1 even though both baselines refreshed. The header comment says a completed refresh exits 0. This was already the case before the PR (it also exited 0 on drift), so the PR did not introduce it, but the comment's wording is slightly off.

[github-actions]

claude-security-review reviewed 8f5e9fa2aff00093a3d0062a883094dfb91894c0: 0 finding(s), 0 as inline comments.

Security review: no findings

I reviewed PR #8207 at head 8f5e9fa. It gives the vendored drift-check scripts one 0/1/2 exit-code contract. The PR touches the scheduled drift workflow, the context7, dometrain and firecrawl update.sh scripts, test scripts, and changelog fragments.

What I checked

• Workflow (maintenance-check-upstream-drift.yml):
  • The change removes the per-row drift and unverifiable matrix fields and replaces the matching logic with a case "$code".
  • No new expressions are interpolated into run:. SKILL, SCRIPT and ARGS still arrive through env: from the checked-in matrix.
  • Permissions and triggers are unchanged, and the workflow header says it holds no secret. Nothing here needs trust-boundary reasoning beyond what zizmor already covers.
• Scripts:
  • The new unknown-mode check allowlists the argument (report, --fix, --refresh-baseline), which narrows the input surface.
  • The URLs fetched by curl and git ls-remote are the same as before. The context7 commit SHA is still validated against ^[0-9a-f]{40}$ before it goes into a URL.
  • The firecrawl recorded_field rewrite only reads and trims text from the checked-in upstream.md. It runs no eval, no command substitution on that data, and no unquoted expansion.
  • The exit-code remapping never turns a failure into a success. Exit 2 still surfaces as a warning, and an exit code outside 0/1/2 now fails the job.
• Instruction surface: No CLAUDE.md, AGENTS.md, rules file, hook allowlist or permission control is deleted or softened. The prose edits only document the exit codes. I did not need the instruction exception register for this diff.

One behaviour note (not a finding)

• A missing ctx7 or firecrawl CLI is no longer counted as drift.
• Upstream-content and baseline drift is still reported, so this does not hide any security-relevant signal.