Skip to content

feat(check-overlay-deps): gate unresolvable overlay imports - #12

Merged
poysama merged 8 commits into
mainfrom
feat/check-overlay-deps
Aug 4, 2026
Merged

feat(check-overlay-deps): gate unresolvable overlay imports#12
poysama merged 8 commits into
mainfrom
feat/check-overlay-deps

Conversation

@poysama

@poysama poysama commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Adds a check-overlay-deps composite action. It fails when a premium overlay imports a package as a value that resolves neither from the overlay nor from the host root . the defect that took out every open PR's preview deploy on 2026-08-03.

What broke, and why nothing caught it

b4m-bob#92 added import { Link } from '@tanstack/react-router' and import { useQueryClient } from '@tanstack/react-query' to src/spa/BobReport.tsx. Neither package is declared in b4m-bob's package.json; both live in the host's apps/client. Once the overlay hydrates to packages/premium/bob, neither resolves from that directory and the Next build dies with Module not found.

b4m-bob's CI was green, and ci / Typecheck & Test is a required check with no bypass actor on that repo. Nobody merged red. The check simply could not see the file: b4m-bob's tsconfig.json excludes src/spa/** and src/api/**, and glue_consumers covers @bike4mind/scripts, not @bike4mind/client. Staging could not catch it either, because reconcile.yml keys desired state on the app repo's SHA alone, so an overlay-only merge never triggers a composed build.

How it decides

For every source file in the overlay, parse with the TypeScript compiler and classify each module specifier as a value import or type-only. A value import must resolve: from the overlay's own manifest, from the host root manifest, or via real require.resolve from the importing file's directory. Two allowlists cover specifiers that legitimately resolve undeclared . bundler-aliased (react, react-dom, next/*, which Next rewrites before resolution) and host tsconfig aliases (@server/*, @client/*).

Type-only imports are reported as notes and never fail. This matters more than it sounds: import { Request, Response } from 'express' appears in the same overlay and carries no type keyword, but both bindings are used only in type positions, so SWC erases the import before the bundler resolves anything. The action reproduces that elision rule from the AST. Skip it and the gate reports express, aws-lambda and sst as build-breakers on four of six overlays, and gets switched off within a week.

One case that is easy to get wrong and is covered: TypeScript uses the same node kind for a class's extends clause and for implements / interface-extends, so a naive walker treats a base class as erasable. It is not . the prototype chain needs it at runtime. An undeclared base-class package would otherwise pass silently.

Validation

Verified red-then-green against the real incident. At b4m-bob 5f2c467 the action reports exactly two errors, @tanstack/react-router at src/spa/BobReport.tsx:10 and @tanstack/react-query at :11 . the same file and lines as the Turbopack failure in deployer run 30785499528. At b4m-bob PR 107's head it passes.

Run across all six overlay trees, 946 files: zero errors on b4m-libreoncology, b4m-overwatch, b4m-pi, b4m-tavern and b4m-optihashi, notes only. Day-one adoption is clean, so there is no pressure to disable it.

32 unit tests, run by a new job in scripts-ci.yml.

No dependency for consumers

The action resolves typescript from the host tree via createRequire, so it must run after pnpm install and adds nothing to any consumer's install. The typescript install in this repo's CI is test-only.

Not in this PR

Wiring the step into overlay-ci.yml follows separately so this can be reviewed as a unit. b4m-optihashi runs a bespoke ci.yml and gets the step by hand. A deploy-path backstop lands in bike4mind-deployer.

Scope worth naming: this asks whether a module resolves, not whether the code typechecks. src/api/** and src/spa/** remain excluded from all type checking in every overlay, which is tracked separately.

@poysama

poysama commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 3962b3f: action inputs now go through env: rather than being interpolated into run:.

Credit where due . this came out of reviewing #14, where Matt does it correctly. My own final review flagged the same thing independently, so it was on the list twice.

The inputs are workflow-authored and therefore trusted today, so this is not a live vulnerability. It is worth not shipping the pattern at all from a public action that private repos consume: a future caller passing a value derived from anything untrusted would inject shell. github.action_path stays interpolated, since that is runner-provided rather than caller-provided.

Verified behaviourally identical rather than assumed. Running the new step body with the env vars the action would set: the broken b4m-bob fixture still exits 1 with exactly two ::error lines, the PR-107 fixture still exits 0, and passing extra_allowlist through the env var still suppresses both errors on the broken fixture, which proves the third argument actually reaches the CLI. Also confirmed the substitution form is now inert . OVERLAY_PATH='$(echo PWNED)' reaches Node as a literal, appearing verbatim inside the resulting ENOENT path rather than being evaluated. 32/32 unit tests unchanged.

One small thing left unfixed deliberately, so it does not get lost: a bad overlay_path surfaces as a raw Node ENOENT rather than a ::error:: annotation. Same nit exists in #14. Worth one existsSync in both, as a follow-up rather than in either PR.

@poysama
poysama merged commit de01813 into main Aug 4, 2026
2 checks passed
@poysama
poysama deleted the feat/check-overlay-deps branch August 4, 2026 05:17
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.

1 participant