P0: main went red

Workflow: CI concluded failure on main.

Failing run: https://github.com/edobry/minsky/actions/runs/33832040062
HEAD SHA: 6395bac100235ce0765ccd33bbbe66e22998092a
Head commit: fix(mt#4945): Resolve identifier-specifier dynamic imports in the test selector

Summary

The fast related-test gate selects by walking a static import graph, so a test that reaches its
subject through a dynamic import could be missed — a breaking change to that subject passes every
local gate and fails in CI. That happened on mt#4937: sql-generation-proof.test.ts is arguably the
most on-point test for PostgresVectorStorage, was not selected, and contributed 4 of 4 CI failures
on an already-approved PR.

The task's framing was too broad by one case, and that is the headline

extractImportSpecifiers has always matched import("literal"). Verified directly rather than

extractImportSpecifiers('await import("@scope/pkg/mod")')        -> ["@scope/pkg/mod"]
extractImportSpecifiers('const M = "…"; await import(M)')        -> []

Its \s* spans newlines, so a wrapped literal is covered too. So the gap is not "dynamic imports"
— it is exactly import(IDENT), where the specifier is a bare identifier. Measured on this
tree: 16 call sites across 6 test files, and 16 of 16 bind to a same-file string literal. Zero
are computed, imported, or parameterised. 254 of the 270 test-side dynamic imports were never
broken.

That reframing is what made the fix small: a same-module binding lookup feeding the existing
resolveSpecifier, no AST pass, no new dependency on a gate with a 60s partition budget.

Key changes

• extractImportSpecifiers resolves import(IDENT) through a same-module const IDENT = "literal"
  (also let/var, and a type-annotated const).
• SC3 — an unresolvable edge must not read as an absent one. A test carrying an import(IDENT)
  we cannot resolve joins EVERY selection rather than being dropped. buildDataReadGraph now
  returns that set beside its graph, collected in a loop that already holds every test file's
  content — so it costs no extra I/O over ~1,300 files.
• hasUnresolvedIdentifierImport strips comments first; extraction deliberately does not. The two
  have opposite failure directions: a spurious EDGE over-selects, which this module documents as
  the accepted direction, while a spurious INCLUSION burns one file on every run forever.

A defect in the fix, found by measuring it

The first draft's inclusion path flagged find-related-tests.test.ts itself — because that file's
own prose writes the call form out to explain the feature, and the matcher read documentation as
code. That is the same class of defect this task exists to fix.

It surfaced only because the selection count came back 15 where 14 was expected, and I asked
why instead of accepting a green result. Fixed by the comment-stripping above. The residual — the
same form inside a string literal — is documented in the function's docblock rather than papered
over, and the new test names avoid it deliberately, which is itself noted there so a later author
does not reintroduce it.

Testing

Execution evidence:

bun test --preload ./tests/setup.ts scripts/find-related-tests.test.ts tests/scripts/find-related-tests.test.ts tests/scripts/run-related-tests.test.ts

 86 pass
 0 fail
 162 expect() calls
Ran 86 tests across 3 files.

Nine new tests, all in the two mt#4945 describes:

(pass) dynamic import via an identifier specifier (mt#4945) > CONTROL: a direct string-literal specifier was already resolved
(pass) ... > resolves an identifier specifier through a same-module const binding
(pass) ... > resolves it through a TYPED const, and through let/var
(pass) ... > an identifier with no literal binding invents no specifier, and IS reported
(pass) ... > an identifier import written in a COMMENT does not force conservative inclusion
(pass) ... > a block comment is stripped too
(pass) ... > a computed specifier is reported unresolved rather than matched
(pass) findRelatedTestFiles selects across an identifier-specifier import (mt#4945) > a change to the subject selects the test that imports it via an identifier
(pass) ... > SC3: a test with an UNRESOLVABLE identifier import joins every selection

AT1 / SC1 — the regression case, run before and after:

$ bun scripts/run-related-tests.ts packages/domain/src/storage/vector/postgres-vector-storage.ts
BEFORE: 13 files — tests/domain/storage/vector/sql-generation-proof.test.ts ABSENT
AFTER : 14 files — ...tests/domain/persistence/migration-validation.test.ts,
                   tests/domain/storage/vector/sql-generation-proof.test.ts
        Ran 340 tests across 14 files. 0 fail.

AT3 / SC4 — selection-cost measurement, before/after on five subjects spanning a domain module,
an adapter, a hook, an MCP file and a large facade:

                                                            before  after
packages/domain/src/storage/vector/postgres-vector-storage.ts   13     14   +1
src/adapters/shared/commands/knowledge/index.ts                  1      2   +1
.minsky/hooks/entity-linkify.ts                                  6      6    0
src/mcp/command-mapper.ts                                        1      1    0
packages/domain/src/tasks/taskService.ts                       313    314   +1

+1 on exactly the subjects an identifier import reaches, 0 elsewhere. Criterion 4 asked whether the
fix "routinely doubles selections" — it does not; the maximum observed delta is one file.

SC2 — the resolvable and unresolvable forms are enumerated in resolveIdentifierImports's
docblock, with the measurement that bounds them and an explicit statement that it is not
scope-aware.

SC5 — the classification (why the identifiers exist at all) is recorded in mt#4945's ## Outcome
with counts. Short version: all 16 are named path constants used 1–3 times in one file — ordinary
magic-string avoidance, not a load-bearing lazy boundary. SC0's branch decision and the Branch B
recommendation are recorded there too; see below.

Negative control — full revert of the selector change

git stash push -- scripts/find-related-tests.ts, then the same suite:

 25 pass
 1 fail
 1 error
Ran 26 tests across 2 files.

Only 26 of 62 tests reach execution, because the reverted module no longer provides the exports.
This is the FULL revert of the fix rather than one line of it (mt#4512), so it is faithful — but it
fails coarsely, which is why the behavioural control below matters more.

Negative control — the behavioural one, against the real corpus

The AT3 table above IS a control: it was produced by stashing the production change and re-running
the selector over the real tree, so the "before" column is measured, not remembered. The regression
file drops out and reappears with the change.

An independent regression case, beyond the spec's own

src/adapters/shared/commands/knowledge/index.ts selected 1 test before and 2 after, the
new one being packages/domain/src/knowledge/reconciliation/conflicts.test.ts. That is the exact
miss mt#4946 hit hours earlier tonight: it changed that command's signature, the gate picked only
the sibling test, and three genuinely-broken call sites in conflicts.test.ts were caught by
neither the gate nor the compiler. This fix catches the selection half of it.

Surfaced, not executed: converting the 16 sites

Criterion 0 routes a Branch-B conclusion to a scope change rather than a silent edit, so this is a
recommendation, not part of the diff.

A dynamic import with a non-literal specifier is invisible to TYPECHECKING as well, and no
selector change can recover that. Verified with a control pair: in a file reached that way, a plain
type error DOES fail, while a call through the dynamically imported symbol passing 5 arguments to a
2-parameter function does NOT. mt#4946 demonstrated the cost the same evening — 17 type errors in
the statically-importing test file, zero in conflicts.test.ts, which held three broken calls.

So converting those 16 identifier specifiers to direct literals is worth doing on top of this
change — it recovers typechecking, it is one line per site, and the named constants can stay (only
the import() argument needs to be the literal). It is out of ## Scope here by design, and
converting ALONE would leave the gate blind to the next case, which is why this change ships first.

Deploy verification

[no-deploy-impact] — run over the actual changed files rather than recalled:

$ bun -e 'import { isDeploySurfaceFile } from "./packages/domain/src/deployment/deploy-surface.ts"; …'
false scripts/find-related-tests.ts
false scripts/find-related-tests.test.ts

Both false, so no deploy follows and no post-merge deploy verification applies.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F1LhVjJvmY8oX8KxUK9vaA

Co-Authored-By: minsky-ai[bot] <minsky-ai[bot]@users.noreply.github.com>

What this means

A push to main triggered CI and the workflow above did not conclude success. Per
CLAUDE.md user preference ("main must never be broken"), this is severity-1.

Diagnostic checklist

1. Open the failing run URL above; identify which job/step failed.
2. Check whether the offending PR was merged with a known-failing required check
   (operator-API bypass via gh api PUT /merge despite enforce_admins).
3. Confirm enforce_admins is currently enabled:

   gh api repos/edobry/minsky/branches/main/protection --jq .enforce_admins.enabled
   

   Expected: true post-mt#1938. If false, that is itself a separate finding.

Recovery

1. Open a hotfix branch off current main.
2. Apply the smallest fix that turns CI green (often a formatter pass or a config
   flip).
3. Land via the standard Minsky session flow:
   tasks_create → session_start → session_commit → session_pr_create → /review-pr → session_pr_merge.
4. Verify the post-merge main build is green within ~5 minutes.
5. Close this issue with a link to the hotfix PR.

Cross-references

• mt#1938 — structural fix for the main-red coverage holes
• mt#1928 — prior hotfix that unblocked main on 2026-05-19
• PR feat(mt#1927): install marketing-site-design skill bundle with cyberbrain brand register lock #1163 / mt#1927 — originating incident
• .claude/hooks/SPEC.md §Layered enforcement model — three-layer model
• This issue is auto-filed by .github/workflows/main-watch.yml.