Skip to content

test(scripts): fail loudly when REQUIRED_CONTEXTS drifts from ci.yml's job names - #7819

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
xfodev:fix/required-contexts-workflow-sync-test-7774
Jul 21, 2026
Merged

test(scripts): fail loudly when REQUIRED_CONTEXTS drifts from ci.yml's job names#7819
JSONbored merged 1 commit into
JSONbored:mainfrom
xfodev:fix/required-contexts-workflow-sync-test-7774

Conversation

@xfodev

@xfodev xfodev commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

What & why

scripts/check-stuck-required-checks.mjs's REQUIRED_CONTEXTS is hardcoded — branch protection's required-checks list needs Administration read, which the ephemeral workflow GITHUB_TOKEN can't get (the file's own comment documents this). That makes it silent-drift-prone: if the workflow's required aggregate check is renamed without a matching manual edit here, the stuck-check watchdog (built after a real incident) goes blind to it, with no error or CI signal anywhere.

Change

  • Add a describe block to the existing test/unit/check-stuck-required-checks-script.test.ts that parses .github/workflows/ci.yml's job names (via the yaml package, as codecov-policy.test.ts does) and asserts every workflow-sourced entry in REQUIRED_CONTEXTS still names a real ci.yml job, plus that the single documented required aggregate (validate) stays listed. Deriving which checks are required from YAML alone isn't possible (the same permissions limitation), so this guarantees forward consistency and fails loudly on the drift it can detect.
  • Move the "which required checks are external / not declared by any workflow" knowledge (currently "Superagent Security Scan", a third-party GitHub App check) into the script as an exported, documented EXTERNAL_REQUIRED_CHECKS constant beside REQUIRED_CONTEXTS, rather than hiding it in the test. The test imports it and also asserts it's a subset of REQUIRED_CONTEXTS.

scripts/** is outside the Codecov coverage.include set, so codecov/patch does not gate this. Verified locally: all 12 tests in the file pass, root tsc --noEmit clean, git diff --check clean, node --check on the script passes, and injecting a bogus required context genuinely fails the test (then reverted).

Closes #7774

(Supersedes #7814, which the gate auto-closed: a test-only change ran under scoped --changed selection and produced an empty coverage/lcov.info. Touching the script here runs the full suite, so coverage is non-empty.)

…s job names

check-stuck-required-checks.mjs's REQUIRED_CONTEXTS is hardcoded (branch
protection's required-checks list needs Administration read, which the ephemeral
workflow token can't get), so a workflow-side rename of the required aggregate
check without a matching edit here silently blinds the stuck-check watchdog to it.

- Add a test that parses .github/workflows/ci.yml's job names and asserts every
  workflow-sourced entry in REQUIRED_CONTEXTS still names a real job, plus that
  the single documented required aggregate ("validate") stays listed. Deriving
  which checks are required from YAML alone isn't possible (same permissions
  limitation), so this guarantees forward consistency and fails loudly on the
  drift it can detect (verified: injecting a bogus required context fails it).
- Move the "which required checks are external / not workflow-declared" knowledge
  (currently "Superagent Security Scan", a third-party App check) into the script
  as an exported, documented EXTERNAL_REQUIRED_CHECKS constant beside
  REQUIRED_CONTEXTS, rather than hiding it in the test — the test imports it and
  also asserts it's a subset of REQUIRED_CONTEXTS.

Closes JSONbored#7774
@xfodev
xfodev requested a review from JSONbored as a code owner July 21, 2026 13:32
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.65%. Comparing base (3fd145b) to head (13911f4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7819   +/-   ##
=======================================
  Coverage   88.65%   88.65%           
=======================================
  Files         730      730           
  Lines       76959    76959           
  Branches    22787    22787           
=======================================
  Hits        68229    68229           
  Misses       7684     7684           
  Partials     1046     1046           
Flag Coverage Δ
shard-1 53.62% <ø> (ø)
shard-2 51.53% <ø> (ø)
shard-3 50.23% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-21 13:59:18 UTC

3 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds a drift-detection test suite that parses .github/workflows/ci.yml and asserts every workflow-sourced entry in the hardcoded REQUIRED_CONTEXTS still names a real ci.yml job, plus extracts a new exported EXTERNAL_REQUIRED_CHECKS constant to distinguish workflow-derived checks from genuinely external ones (Superagent Security Scan). The logic is sound: it correctly derives a check-run's context name as job.name ?? jobId matching GitHub's actual behavior, includes a premise guard against a stale EXTERNAL_CHECKS set, and validates the external/required subset relationship bidirectionally. This closes #7774 and is a narrow, well-targeted test-only change with no production code path affected.

Nits — 4 non-blocking
  • The four new test cases in check-stuck-required-checks-script.test.ts:135-186 all call ciJobCheckNames() independently, re-parsing and re-reading the YAML file each time — could hoist to a beforeAll/module-level const for minor efficiency.
  • EXTERNAL_CHECKS is just a local alias for the imported EXTERNAL_REQUIRED_CHECKS (test/unit/check-stuck-required-checks-script.test.ts:139); consider using the imported name directly to avoid the indirection.
  • Consider hoisting the ciJobCheckNames() result to a shared variable across the four tests in the new describe block to avoid redundant file reads and YAML parses.
  • If EXTERNAL_REQUIRED_CHECKS grows beyond one entry in the future, the per-item loop assertions already scale fine — no change needed there.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7774
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 71 registered-repo PR(s), 42 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor xfodev; Gittensor profile; 71 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Addressed
The PR adds a test suite that parses ci.yml job names via the yaml package and asserts every workflow-sourced entry in REQUIRED_CONTEXTS matches a real job (plus that 'validate' stays present), which fails loudly on drift exactly as requested; the author also documents and verified locally that injecting a bogus context fails the test.

Review context
  • Author: xfodev
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 71 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@JSONbored
JSONbored merged commit 9c9188b into JSONbored:main Jul 21, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

check-stuck-required-checks.mjs's hardcoded REQUIRED_CONTEXTS list has no automated drift check against ci.yml's real job names

2 participants