feat: let doctor gate CI without making every warning fatal

[vivek7405]

Problem

webjs doctor runs in no CI, in this repo or in a scaffolded app, so every advisory it carries is findable but not unmissable.

• This repo: .github/workflows/ci.yml has ten jobs (conventions L24, unit L64, bun L86, db-postgres L277, browser L315, the two e2e jobs L329 / L381, dist L410, apps L425, docker L470). None runs doctor, and the root package.json has no doctor script.
• A scaffolded app: packages/cli/templates/.github/workflows/ci.yml has four jobs (conventions L26 running npm run check, unit L38, browser L58, e2e L72). None runs doctor. The scaffold does get a doctor: 'webjs doctor' script (packages/cli/lib/create.js L405), documented as the thing a contributor runs after onboarding.

That is deliberate today (packages/cli/AGENTS.md calls doctor "An onboarding/setup-verify tool, NOT a scaffold-CI hard gate"), but it has a concrete cost. The UNMARKED_ASSET_LINKS advisory added in #1095 (merged as 060617e) warns when a route module writes a <link rel="stylesheet" href="/public/..."> without asset(), which is the deploy-staleness bug that shipped a visible regression on webjs.dev. Because nothing runs doctor, that advisory would not have caught the original regression. The same holds for STATIC_ASSET_FRESHNESS and ELISION_CARRIERS.

The blocker is that doctor's exit is all-or-nothing: the default fails only on a broken toolchain, and --strict makes every warning fatal, including four that are environment-shaped and would red a clean CI run. GIT_HOOK wants a local pre-commit hook a runner has no reason to have, ENV_DRIFT compares against a .env CI does not carry, VENDOR_PIN performs a network fetch, and FRAMEWORK_RESOLVE is environment-dependent.

Measured starting state

Running doctor on all four in-repo apps today, so the plan is designed against reality rather than against an assumption:

App	fails	warns
examples/blog	0	ENV_DRIFT, WEBJS_VERSIONS, ELISION_CARRIERS (3 modules)
website	0	ENV_DRIFT, WEBJS_VERSIONS, ELISION_CARRIERS (2 modules)
docs	0	WEBJS_VERSIONS
packages/ui/packages/website	0	WEBJS_VERSIONS, ENV_DRIFT

UNMARKED_ASSET_LINKS passes on all four, so gating it error is green on the day it lands. ELISION_CARRIERS, WEBJS_VERSIONS, and ENV_DRIFT warn in this repo, so gating any of those would red main on merge.

Implementation plan

Decision: per-check severity becomes CONFIG, not a flag. A new webjs.doctor.gate map in the app's package.json maps a stable DOCTOR_CODES code to off / warn / error, the CLI applies it over the pure check results, and the existing failing = fail > 0 || (strict && warn > 0) formula is left byte-identical so an app with no gate block behaves exactly as today.

Two things settled it. First, this is the shape every mature linter converged on: ESLint's per-rule severity is a three-level scale (off / warn / error) declared in the config file, with --max-warnings as the only CI knob layered on top, and Next.js ships exactly that shape at packages/eslint-plugin-next/src/index.ts, where recommendedRules is a severity map keyed by stable rule id and coreWebVitalsRules escalates precisely two of those rules from warn to error as a named preset (re-exported by packages/eslint-config-next/src/core-web-vitals.ts). tsc by contrast has no per-diagnostic severity at all and is all-or-nothing, which is exactly the --strict shape that fails here. Second, WebJs already has one established home for project config, the "webjs" block in package.json, schema-validated and typed by WebjsConfig, and the CLI already reads keys out of it (readAppTasks reads webjs.dev / webjs.start). Putting the policy there means the CI workflow, the scaffold's npm run doctor, and an agent's webjs doctor --json loop all read ONE policy that travels with the repo, instead of three copies of a code list.

On top of the map, a result that reports "could not check" rather than a real finding is CAPPED at warn and can never be escalated, which is what keeps the best-effort-network posture in lib/doctor.js's module header true after gating exists.

Rejected:

• A --gate=CODE,CODE flag: policy would live in workflow YAML, so a local webjs doctor and CI disagree about what is fatal, and the code list is duplicated in every caller (this repo's ci.yml, the scaffold's ci.yml, an agent loop).
• A --profile=ci preset: hard-codes one policy judgement inside the CLI, and every app inherits an opinion it can neither narrow nor widen.
• A separate severity baseline file (.webjsdoctorrc and friends): a second config file, a second parser, and a second thing to document, for one map that the already-schema-validated webjs block holds fine.
• Making --strict take an EXCLUDE list: keeps the binary and inverts the safe default, since a newly added check would become fatal in every existing CI until someone excludes it.
• Moving the advisories into webjs check: root AGENTS.md reserves webjs check for correctness (code that is wrong to ship), and these are project-health warnings a sensible app can legitimately leave unfixed.
• A per-check severity field on the checks themselves: the checks are pure and policy-unaware by design, and runDoctorChecks already attaches the code centrally, so policy belongs one layer up in the bin.

The severity model

Two pure exported functions in packages/cli/lib/doctor.js, composed by the bin. The CHECKS stay policy-unaware.

readDoctorPolicy(appDir) reads package.json webjs.doctor.gate and returns { gate, unknownCodes, badSeverities }. gate holds only entries whose key is a value of DOCTOR_CODES and whose value is off / warn / error. A missing block, a missing file, or unparseable JSON returns an empty gate with no problems, so an app with no config is untouched.

applyDoctorPolicy(results, gate) returns a NEW array (never mutates) where each result gains severity, the EFFECTIVE level it contributes to the summary:

• status === 'pass' gives severity: 'pass', whatever the gate says. A rule that did not fire contributes nothing, matching ESLint, where severity rides a message and a passing rule emits none.
• Otherwise the gate entry for the code wins, else the default: error for status === 'fail', warn for status === 'warn'.
• One cap: a result carrying bestEffort: true is clamped to warn even when the gate says error.

severity is the EFFECTIVE level rather than the raw configured level on purpose. It removes a footgun: a consumer writing results.some(r => r.severity === 'error') would otherwise get a false positive off a PASSING check whose code happens to be gated error. With the effective level that one-liner is exactly right.

bestEffort: true marks the four could-not-check results, and only those: checkVendorPin's toolchain-unavailable branch (L326) and network-unreachable branch (L353), and checkImportmapCoherence's toolchain-unavailable branch (L582) and could-not-verify branch (L675). Those report "I could not check" rather than a finding, so an outage must never be escalatable.

No-regression guarantees

1. The exit formula is textually unchanged. const failing = fail > 0 || (strict && warn > 0) stays exactly as written at packages/cli/bin/webjs.js L699. Only the inputs are recomputed.
2. The counts are byte-identical for an app with no gate block. pass stays status === 'pass'. fail becomes results at severity === 'error', which for an ungated app is exactly status === 'fail'. warn becomes results at severity === 'warn', which is exactly status === 'warn'. A new off counts what the gate silenced, and is 0 without a gate.
3. The --json shape is additive only. severity joins each result, bestEffort appears where set, off joins the summary. Nothing is renamed or removed, and the summary key stays fail (not error) so an existing consumer keeps working. Every doctor consumer in the repo was grepped: the only ones are test/cli/doctor.test.mjs, test/cli/help.test.mjs, and the docs-site prose. The MCP does not expose doctor at all.
4. No new flag, so the usage string is unchanged. test/cli/help.test.mjs L48 and L131 assert Usage: webjs doctor [--json] [--strict] verbatim.
5. CI cannot acquire a network flake from this. Doctor's only network touches are the vendor-pin freshness fetch and the importmap-coherence live jspm resolve. Neither can produce a fail, and neither code is gated, so neither can red the job. IMPORTMAP_COHERENCE's could-not-verify branch is additionally bestEffort-clamped, so it stays a warn even if an app gates it. Every vendor fetch is already bounded by an AbortSignal timeout in packages/server/src/vendor.js, so there is no hang vector either. Measured cost of the added step: about 6s on website (9 vendor packages, live jspm) and about 1s on examples/blog.
6. A typo in the gate map is loud, never silently un-gating. An unknown code or a bad severity value exits 1 naming the offender and the valid code list, WITHOUT running the checks.
7. The required-context display name is untouched. The step goes inside the existing conventions job, whose name: Conventions (webjs check) is one of the five contexts in gh api repos/webjsdev/webjs/branches/main/protection. Renaming it, or putting the step in a NEW job, would silently make it non-required.
8. No runtime config validation to break. The webjs block's JSON Schema is an editor and drift-test surface, and no server reader rejects an unknown key. The schema still gains doctor because packages/server/test/config/webjs-config-schema.test.js drift-asserts in both directions.

Steps

1. packages/cli/lib/doctor.js, the DoctorResult typedef (L48): add bestEffort?: boolean and severity?. Add a DoctorSeverity typedef for the three CONFIG values and export a DOCTOR_SEVERITIES list.
2. packages/cli/lib/doctor.js: set bestEffort: true on the four could-not-check results named above, and only those. Extend the module header's best-effort paragraph (L36-38) to say the flag is what the gate reads, and add a short severity-policy paragraph.
3. packages/cli/lib/doctor.js: add export function readDoctorPolicy(appDir), pure, per the contract above.
4. packages/cli/lib/doctor.js: add export function applyDoctorPolicy(results, gate), pure, per the contract above.
5. packages/cli/bin/webjs.js, case 'doctor' (L680): read the policy before rendering. If unknownCodes or badSeverities is non-empty, print the offending entries plus the valid code list from DOCTOR_CODES and exit 1 without running the checks. Under --json that path emits { results: [], summary: { pass: 0, warn: 0, fail: 0, off: 0, strict, ok: false }, configErrors: [...] } and exits 1.
6. packages/cli/bin/webjs.js, case 'doctor': pass the results through applyDoctorPolicy, then compute the counts from severity instead of status. Leave const failing = fail > 0 || (strict && warn > 0) (L699) textually unchanged.
7. packages/cli/bin/webjs.js, the render block (L715 onward): extend marker with '[off]' and pick it from severity. When the gate moved a result off its default, append the reason to the code line, for example (UNMARKED_ASSET_LINKS, gated: error). Add the off count to the summary line and name gating in the failure reason. Include severity (and bestEffort where set) in the --json results and off in the --json summary.
8. packages/cli/bin/webjs.js: update the USAGE doctor entry (L60-61) and the HELP.doctor entry (L145-153) to document the config gate. The usage string stays webjs doctor [--json] [--strict] exactly, since no flag is added and test/cli/help.test.mjs L48 and L131 assert that string.
9. Config lockstep, all three surfaces plus the drift list, per packages/server/AGENTS.md L167: add the doctor property to packages/server/webjs-config.schema.json (object, additionalProperties: false, one gate property whose propertyNames match ^[A-Z][A-Z0-9_]*$ and whose values are the off / warn / error enum); add WebjsDoctorConfig plus doctor?: WebjsDoctorConfig to packages/core/src/webjs-config.d.ts beside the dev / start pair (L191-192); add 'doctor', // readDoctorPolicy (cli/lib/doctor.js), CLI-read not server to KNOWN_KEYS in packages/server/test/config/webjs-config-schema.test.js (L40-57), which is where dev and start already live as the CLI-read exceptions.
10. This repo's gate policy: add "webjs": { "doctor": { "gate": { "UNMARKED_ASSET_LINKS": "error" } } } to website/package.json and examples/blog/package.json. Both layouts already call asset() (website/app/layout.ts L164, examples/blog/app/layout.ts L138), so both are green on the day the gate lands. ELISION_CARRIERS, WEBJS_VERSIONS, and ENV_DRIFT are deliberately NOT gated (see the measured table above). STATIC_ASSET_FRESHNESS is not gated either: its output is gitignored and absent on a runner, so it passes vacuously in CI while warning constantly on a working local checkout.
11. .github/workflows/ci.yml, the conventions job: add a doctor step directly after the existing webjs check loop (L35-41), reusing that loop's shape over the same four apps and invoking the CLI directly (node "$GITHUB_WORKSPACE/packages/cli/bin/webjs.js" doctor), because the repo has no doctor npm script. A separate step, not a merged loop, so a doctor failure is attributable in the log. Do NOT rename the job's name: Conventions (webjs check) (L25): branch protection matches the required context by that exact string, so renaming it silently un-requires the job.
12. packages/cli/lib/create.js, the emitted webjs block (L482): add doctor: { gate: { UNMARKED_ASSET_LINKS: 'error' } } beside dev / start, for both templates. The scaffold's root layout already writes href=\${asset('/public/tailwind.css')} (L1253), and the gallery's caching demo shows the link only as HTML-escaped <link ...> sample text (packages/cli/templates/gallery/app/features/caching/page.ts L57), which the scanner does not match, so a fresh app is green.
13. packages/cli/templates/.github/workflows/ci.yml, the conventions job (L26-36): add - run: npm run doctor after npm run check, and extend the job comment to say the gated codes come from the app's own webjs.doctor.gate block, so an app owner narrows or widens the gate in package.json rather than in the workflow.
14. Generate an app and prove the loop end to end: webjs create a fresh app, run npm run doctor in it and confirm exit 0, then hand-edit one route module's stylesheet link to the bare /public/tailwind.css form and confirm the same command now exits 1 naming UNMARKED_ASSET_LINKS. The generators emit strings, so an escaping bug in the new block only shows in a generated app.

Sequencing: land the bestEffort flag and the clamp BEFORE the CI step. Reversing them would briefly give the required conventions job a network dependency with no outage protection, which is exactly the #1150 class of flake.

Tests

• Unit: test/cli/doctor.test.mjs, extending the pure-function section, asserting readDoctorPolicy returns an empty gate for a missing block / missing package.json / unparseable JSON, keeps a well-formed entry, and reports an unknown code and a bad severity value separately; and asserting applyDoctorPolicy gives a fail result error and a warn result warn by default, gives a pass result pass even when its code is gated error, honours an override in both directions, CLAMPS a bestEffort: true result to warn when the gate says error, and does not mutate its input.
• Unit: test/cli/doctor.test.mjs, extending the CLI integration section (the runCliArgs harness at L575), asserting a fixture app whose only finding is a gated warn exits 1 and prints [fail] ... (gated: error), that the SAME fixture with no gate block exits 0 (the counterfactual pair, mirroring the --strict pair at L608), that off silences a warn even under --strict, that an unknown gate code exits 1 naming the code without running the checks, and that --json carries severity on every result plus off in the summary.
• Unit: test/cli/doctor.test.mjs, asserting the four could-not-check branches carry bestEffort: true (extend the existing vendor-pin network test at L280 and the coherence could-not-verify test at L391), which is the assertion that keeps a jspm or npm outage from redding CI once doctor runs in the required job.
• Unit: test/cli/help.test.mjs, asserting webjs help doctor documents the webjs.doctor.gate config and still prints the unchanged Usage: webjs doctor [--json] [--strict] line.
• Unit: packages/server/test/config/webjs-config-schema.test.js (the existing drift assertions cover the new key once doctor is in KNOWN_KEYS and the schema) and test/types/webjs-config.test-d.ts, asserting a doctor.gate literal type-checks and a bad severity string does not.
• Unit: test/scaffolds/scaffold-template-validation.test.js, asserting the generated package.json carries webjs.doctor.gate.UNMARKED_ASSET_LINKS === 'error' and the generated .github/workflows/ci.yml runs npm run doctor.
• Browser / e2e / Bun parity: N/A, because the change touches no runtime-sensitive surface. It adds a package.json read and an exit-code computation in the CLI, with no serializer, listener, SSR / action / CSRF dispatch, stream, node:crypto, TS-stripper, or auth / session / cors involvement, so no test/bun/** cross-runtime assertion applies. A bun-flavored scaffold runs the same webjs doctor script (packages/cli/lib/create.js L377 keeps the tooling scripts on plain webjs ...), and JSON reading is identical on both runtimes.
• Counterfactual: revert ONLY the gate-override branch in applyDoctorPolicy so severity always derives from status. The gated-warn CLI test reds (exit 0 where 1 is expected) while every other doctor test stays green, proving the gate itself is what flips the exit and not some unrelated hard fail.

Verification

1. node --test test/cli/doctor.test.mjs test/cli/help.test.mjs and node --test packages/server/test/config/webjs-config-schema.test.js.
2. Run the counterfactual toggle described above, after committing the fix.
3. Run the exact CI loop locally over all four apps and confirm every one exits 0.
4. Prove the gate fires in this repo: temporarily rewrite one website route module's stylesheet link to the bare /public/tailwind.css form, confirm the same command now exits 1 naming UNMARKED_ASSET_LINKS, and restore.
5. Generate a fresh app end to end per step 14.
6. Full npm test, plus node --test test/scaffolds/*.js.
7. The four-app dogfood boot check per the definition of done, since the change touches packages/cli.

Doc surfaces

• packages/cli/AGENTS.md, the webjs doctor row (L156), replacing the "An onboarding/setup-verify tool, NOT a scaffold-CI hard gate" sentence with the gated reality, and documenting the severity model, the best-effort cap, and the unknown-code hard error.
• packages/cli/README.md (L49), whose webjs doctor comment currently reads "local onboarding, not CI".
• Root AGENTS.md, the CLI reference line for webjs doctor (L509), naming webjs.doctor.gate alongside --json and --strict.
• .agents/skills/webjs/references/built-ins.md, the webjs config-block section plus the asset() doctor paragraph (L96), which is the place an agent learns the advisory exists and now learns how to make it fatal. This file is the single source the scaffold copies (packages/cli/lib/create.js L653-662), so editing it covers a generated app's skill too.
• website/app/docs/configuration/page.ts, the webjs doctor block (L46-49) for the severity model, and the webjs config-block listing (L98 onward) for the new key.
• packages/server/webjs-config.schema.json and packages/core/src/webjs-config.d.ts, per step 9. These are doc surfaces as much as code: the schema is what the scaffold's .vscode/settings.json wires up for editor completion.

Implementation notes (for the implementing agent)

Where to edit:

• packages/cli/bin/webjs.js, case 'doctor' (L680): owns rendering and the exit code. The failing computation (L699) is the seam. The USAGE banner (L52, doctor at L60) and the HELP map entry (L145, with usage / options / examples) both document the surface and must gain the config gate.
• packages/cli/lib/doctor.js: DOCTOR_CODES (L65) is the stable code vocabulary to filter on. runDoctorChecks(appDir, opts) (L1365) is PURE and attaches r.code centrally in the return; the gating helpers live beside it but the CHECKS stay unaware of gating policy, which is why readDoctorPolicy / applyDoctorPolicy are separate exported functions the bin composes.
• .github/workflows/ci.yml: the step goes in the conventions job (L24), which is already a required status check, so the gate is enforced on merge the moment it lands with no protection change. That file is also touched by fix: a flaky plain-link test aborts the whole Browser CI job #1135 (a flaky plain-link test aborting the Browser job) and test: keep a live jspm outage from redding the required CI job #1150 (keeping a live jspm outage from redding the required job); expect to rebase around them.
• packages/cli/templates/.github/workflows/ci.yml: the scaffold's copy, shipped into every new app. The scaffold already has the doctor npm script, so a npm run doctor step fits its existing style.

Landmines / gotchas:

• --strict is not usable as-is in CI, which is why this task exists rather than a one-line workflow step. GIT_HOOK verifies a local .git pre-commit hook a CI checkout has no reason to have; ENV_DRIFT compares .env against .env.example and CI has no .env; VENDOR_PIN freshness performs a network fetch; FRAMEWORK_RESOLVE is environment-shaped. Under --strict all four are fatal. The gate model leaves every one of them at its default warn, so none of them can red a clean run.
• test: keep a live jspm outage from redding the required CI job #1150 gets stricter, not looser, once doctor runs in the required job. checkImportmapCoherence resolves the live importmap through jspm (resolveVendorImports, L599), so the conventions job acquires a network dependency it did not have. The bestEffort cap is what makes that safe, and it must land BEFORE the CI step. Do not reorder those two.
• Do not rename the conventions job. Branch protection matches required contexts by display name, so a "Conventions (check + doctor)" rename silently makes the job non-required.
• A new job would be advisory. If the step is put in a NEW job instead of conventions, it does not gate anything until someone adds the context to protection (scripts/protect-main.sh).
• A typo in the gate map must not silently pass. An unknown code that is quietly ignored is the worst outcome for a mechanism whose whole job is to make a check fatal, hence the exit-1-on-unknown-code rule in step 5.
• off is uniform, including on the two hard-fail checks. NODE_VERSION and TSCONFIG_ERASABLE can be silenced like any other code, matching ESLint, where any rule can be turned off. Say so plainly in the docs rather than carving out an exception the reader has to discover.
• Adding a config key means the three-surface lockstep in packages/server/AGENTS.md (JSON Schema + WebjsConfig type + reader + the KNOWN_KEYS drift test). dev / start are the precedent for a CLI-read key living in the same block.

Invariants to respect:

• Root AGENTS.md's correctness-vs-convention line: webjs check is correctness, doctor is project health. This task does not move any check between them; it only lets a project declare which health signals it treats as fatal.
• The doctor exit contract in lib/doctor.js's module header (hard-fail reserved for a broken toolchain; warns are informational) and its best-effort-network rule. The failing formula stays textually identical and an app with no webjs.doctor block sees no behaviour change at all.
• lib/doctor.js stays PURE: no process.exit, no printing. readDoctorPolicy reads a file and returns data; the bin decides what to do about it.

Acceptance criteria

• webjs doctor can fail the exit on a chosen subset of checks without making every warning fatal
• The subset is declared in the app's package.json under webjs.doctor.gate, keyed by the stable DOCTOR_CODES code, with off / warn / error values
• The environment-dependent checks (GIT_HOOK, ENV_DRIFT, VENDOR_PIN, FRAMEWORK_RESOLVE) do not red a clean CI run
• A network failure in the vendor-pin check still cannot fail CI, and neither can a jspm outage in the importmap-coherence check, because a bestEffort result is clamped to warn even when the gate says error
• An unknown code or a bad severity value in the gate map exits 1 naming the offender, rather than being silently ignored
• The existing exit contract is unchanged for callers using no flag and for --strict, and an app with no webjs.doctor block produces byte-identical counts
• This repo's ci.yml runs the gated doctor inside the already-required conventions job, whose display name is unchanged
• The scaffold's ci.yml template runs the gated doctor, and a freshly generated app passes it
• A counterfactual proves the gate fires (a gated warning fails the exit; an ungated one does not)
• The new doctor key is in the JSON Schema, the WebjsConfig type, and the KNOWN_KEYS drift list
• Tests cover the new behaviour at every layer it touches
• Docs updated: packages/cli/AGENTS.md, packages/cli/README.md, root AGENTS.md, .agents/skills/webjs/references/built-ins.md, the docs-site configuration page, and the CLI usage + help text