Skip to content

scripts(checks): check-checkers-wired counts a prose comment mention as an "imported by a sibling script" home #10048

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

scripts/check-checkers-wired.ts exists so a scripts/check-*.ts that runs nowhere fails CI instead of
"guarding nothing while looking exactly like a guard" (its own header). It resolves a checker's home to one of
test-ci, workflow, imported, allowed or none. The imported branch is a raw substring scan of every
sibling script's full text:

// scripts/check-checkers-wired.ts:129-134
  // Imported by a sibling script => a shared module, not an entry point. Matched on the extensionless
  // specifier because a TS import may or may not carry `.ts`/`.js`.
  const base = input.file.replace(/\.ts$/, "");
  if (input.otherScriptSources.some((source) => source.includes(`${base}.ts`) || source.includes(`${base}.js`) || source.includes(`./${base}"`) || source.includes(`./${base}'`))) {
    return { kind: "imported", via: base };
  }

source.includes(...) is applied to raw source text, so a comment or a string literal satisfies it exactly as
well as an import statement does. That is not hypothetical — the repo already contains such a mention:

// scripts/check-fixture-clock-races.ts:95
      // STRING LITERALS, exactly as check-turbo-typecheck-inputs.ts's own fixtures do. Those are not real

Nothing under scripts/ imports check-turbo-typecheck-inputs.ts — the only real references are
package.json's "turbo-inputs:check": "tsx scripts/check-turbo-typecheck-inputs.ts" and
test/unit/check-turbo-typecheck-inputs.test.ts. So if turbo-inputs:check were ever dropped from test:ci
(a one-token edit in a ~55-command chain, which is the exact class of drift this file's danglingScriptReferences
was written for), resolveCheckerHome would classify it as { kind: "imported" } on the strength of a prose
sentence and report the checker as having a home. The check would stay green while a real guard stopped
running — the precise failure mode the file exists to make impossible.

The same looseness is what ALLOWED_UNWIRED is deliberately kept tiny for: a detected home is trusted
absolutely, so a false "detected" is strictly worse than a listed exception.

Requirements

  • resolveCheckerHome's imported branch must require an actual import/re-export statement, not a bare
    substring occurrence. It must match, in a sibling script's source:

    • import ... from "./check-foo" / "./check-foo.js" / "./check-foo.ts"
    • export ... from the same three specifier forms
    • import("./check-foo") (and the same two suffixed forms)

    and must NOT match the same token inside a // line comment, a /* */ block comment, or a plain string
    literal that is not an import specifier.

  • Detection must stay textual — no TypeScript program, no new dependency. Strip // and /* */ comments from
    each sibling's source before matching, then require the specifier to appear in an import/export
    statement position.

  • The precedence order in resolveCheckerHome must not change: test-ci, then workflow, then imported,
    then allowed, then none.

  • danglingScriptReferences, listCheckerScripts, reachableNpmScripts, npmScriptsInvoking and
    ALLOWED_UNWIRED must NOT change.

  • npm run checkers-wired:check must exit 0 on the resulting tree — every scripts/check-*.ts that is
    genuinely a shared module imported by a sibling must still resolve to imported.

⚠️ Required pattern: keep the fix inside resolveCheckerHome (scripts/check-checkers-wired.ts:114-138),
which is already pure and fully injectable — its otherScriptSources parameter is exactly the seam a test
drives. What does NOT satisfy this issue: (a) deleting the offending comment in
scripts/check-fixture-clock-races.ts:95 and declaring the problem solved — the detector is what is wrong,
and the next comment reintroduces it; (b) adding check-turbo-typecheck-inputs.ts to ALLOWED_UNWIRED,
which inverts the file's stated contract; (c) tightening the unrelated workflow branch instead of the
imported one; (d) requiring a real module resolver / TypeScript program, a scope the file's own header
rejects; (e) a test-only PR.

Deliverables

  • resolveCheckerHome({ file: "check-foo.ts", scripts: {}, reachableFromTestCi: new Set(), workflowText: "", otherScriptSources: ["// see check-foo.ts for the pattern\n"] }) returns
    { kind: "none" }, asserted in test/unit/check-checkers-wired.test.ts.
  • The same call with otherScriptSources: ['import { helper } from "./check-foo";\n'] returns
    { kind: "imported", via: "check-foo" }, asserted in the same file — and likewise for
    "./check-foo.js", "./check-foo.ts", an export ... from "./check-foo" re-export, and a dynamic
    await import("./check-foo").
  • The same call with otherScriptSources: ['/* check-foo.ts is the sibling */\n'] returns
    { kind: "none" } (block comment), asserted in the same file.
  • npm run checkers-wired:check exits 0, with no new ALLOWED_UNWIRED entry.
  • A regression test at test/unit/check-checkers-wired.test.ts named for this bug that uses the real
    sentence from scripts/check-fixture-clock-races.ts:95 as the otherScriptSources fixture and asserts
    the result is { kind: "none" }.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that strips // comments but not /* */ blocks, or that adds the regression test without changing the
matcher — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, packages/loopover-engine/src/**/*.ts, packages/loopover-{miner,mcp}/{lib,bin}/**/*.ts,
packages/loopover-contract/src/**/*.ts and packages/discovery-index/src/**/*.ts. It does not cover
scripts/**, and codecov.yml's ignore: list names scripts/** explicitly — Codecov does not gate the
patch on this change.

The tests are still mandatory and still gating: the root vitest suite (include: ["test/**/*.test.ts"]) runs
test/unit/check-checkers-wired.test.ts on every PR and a failure there is a red required check.

Every branch the change introduces needs both arms asserted in that file: comment-stripping (a source WITH a
// comment mention and one with a real import; a source WITH a /* */ comment mention and one with a real
import); each accepted specifier suffix (none / .js / .ts) matched, and a near-miss
("./check-foobar", which must NOT mark check-foo.ts as imported); static import, export ... from, and
dynamic import(...) each matched. The four pre-existing CheckerHome kinds (test-ci, workflow,
allowed, none) must each keep an assertion so precedence is pinned.

Expected Outcome

A scripts/check-*.ts that is merely mentioned in a sibling's comment no longer counts as having a home, so
dropping a checker from test:ci fails npm run checkers-wired:check instead of being absorbed by a prose
sentence — restoring the guarantee the file was written to provide.

Links & Resources

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions