ci(core-web): block merges that add strict-mode violations (#37536)

[nicobytes]

Proposed Changes

Fixes #37536. Supersedes #37448.

PR #37403 merged the strict-gate harness to main on 2026-09-08 and wired it into nothing. Since then no pull request has been annotated, reported on, or blocked by it, and main has kept accumulating non-strict TypeScript exactly as before.

This wires it into CI as a blocking gate: a change that adds a strict-mode violation on a line it wrote does not merge.

File	Change
core-web/pom.xml	strict-gate execution in the validate profile + two activation profiles
core-web/tools/scripts/strict-gate/lib/format.mjs	job summary now states the run's cost (output only)
core-web/tools/scripts/strict-gate/format.duration.test.mjs	new — unit tests for the above
core-web/tools/scripts/strict-gate/README.md	no longer claims the gate is wired into nothing

CI only. No local git hook — core-web/.husky/ and lint-staged.config.mjs are untouched, so no contributor's local workflow changes. No workflow file changes. No new dependency.

It runs in two contexts, and both are load-bearing

Event	Role
pull_request	Where you read it. ::error file=,line=,col= lines render inline on the changed lines, plus a job summary.
merge_group	Where it is enforced.

That second row is the part worth reviewing. This repository declares no required status checks on main — the ruleset requires a pull request, one approval, thread resolution and signed commits, and no checks. So a red check on a pull request does not by itself stop a merge. A job that fails in the merge queue ejects the pull request, and that does.

Wiring only the pull_request half would have produced a gate that goes red and merges anyway. Trunk and nightly stay skipped — HEAD equals origin/main there, so the diff is empty and the run could only ever pass.

Maven property activation has no OR, hence two profiles rather than one condition. Activation reads the runner's own GITHUB_EVENT_NAME, which is why no workflow file needs to change.

Semantics

• successCodes lists 0 and nothing else. Findings (1) and a harness that could not run (2) both fail. A gate nobody has to obey is a report; a gate that reports "clean" without having looked is worse than no gate, because it is indistinguishable from a clean pull request.
• A 180 s <timeout> fails the same way, bounding the harness's unshallow git-fetch fallback (a full fetch here is ~1.1 GB / ~20 min).
• useMavenLogger is pinned false with a comment, even though it is the default — that default is the only reason the ::error lines reach the log at column 0 where GitHub can render them. Setting it true kills every annotation while leaving the build green.

Known gap, accepted knowingly

There is no escape hatch. A frontend change that legitimately must add a violation — porting legacy code, an unavoidable third-party shape — cannot merge until the violation is fixed or the gate is switched off repository-wide. No label, no per-file suppression.

The spike measured 0 false positives across its corpus (11 findings across 5 pull requests, all real), so the expected frequency is low, and inventing a bypass before anyone needs one tends to produce the bypass everybody uses. If this bites, say so on #37536.

Reviewer check

# Before this PR this is empty. After it, it must match.
git grep -n "strict-gate" -- core-web/pom.xml

Verified locally with the real CI command, on a branch carrying a deliberate TS7006:

Context	Expected	Result
No gate (before)	violation invisible	BUILD SUCCESS, no gate output at all
GITHUB_EVENT_NAME=pull_request	annotation + summary	::error …dot-utils.ts,line=270,col=35:: at column 0, summary table written
GITHUB_EVENT_NAME=merge_group, violation present	fails	BUILD FAILURE — Failed to execute goal …:exec (strict-gate) … Process exited with an error: 1

Activation measured with help:evaluate -Dexpression=skip.strict.gate:

GITHUB_EVENT_NAME	value	effect
pull_request	false	runs
merge_group	false	runs
push	true	skipped
schedule	true	skipped
unset (local)	true	skipped

Three deliberate deviations from the issue's acceptance list

1. It blocks. The issue scoped this non-blocking and named "flipping to blocking" under Out of scope. Being addressed on Wire the diff-scoped strict typecheck gate into CI — merged on main but never executes #37536 directly so the decision is recorded where the next reader looks.
2. No lint-staged.config.mjs hook, and no local hook of any kind.
3. <skip> keys off its own skip.strict.gate, not ${skip.validate}. The issue asks for the latter verbatim. Deliberate: a blocking gate that the documented way to silence its two neighbours also silences is not much of a gate. The flip side is that a plain local ./mvnw -Pvalidate runs lint and prettier but not this.

Worth recording for whoever picks up a local hook later — the harness compares two committed points, so a commit-time hook would examine the previous commit and report "clean" on the very violation being committed. That was verified, not theorised (checked 0 project config(s) against an uncommitted change). A local hook has to sit at push time, or the harness needs a notion of staged content. Reasoning preserved in specs/37536-wire-strict-gate-ci/spec.md under C-001/C-002.

Out of scope

• The template arm (2.2× compiler time on the largest app).
• The three untried closure optimisations.

Checklist

• Tests — 11 unit tests for the summary change, written first and confirmed failing before implementation. The Maven execution and annotation rendering have no automated test in this repo (no harness exists for POM behaviour; rendering belongs to GitHub); both underlying mechanisms were verified empirically with throwaway poms, and the composed result is verified by this PR's own CI run.
• Translations — n/a
• Security Implications Contemplated — no secrets, no new child-process invocation. The harness already invokes every subprocess with an argument array rather than a shell string, because refs and branch names come from pull-request metadata and are untrusted input; this PR does not weaken that.

Additional Info

The gate runs — confirmed. exec:3.0.0:exec (strict-gate) appears at line 1448 of job 104166954128, so the two-profile activation works and the harness is no longer dead code.

But this run proves less than that sentence suggests, and one finding from review changed the picture materially. That same run showed the gate had no merge base and was comparing against the tip of origin/main — which, with main deliberately not strict-clean, would have rejected authors for violations they never wrote. Fixed in 61f3fde by resolving the base from the event payload. Details on the review thread.

Still owed before merge: a throwaway branch carrying a deliberate TS7006, confirming exit 1, the annotation rendering inline on the diff, and the merge_group behaviour. This branch changes no .ts, so none of that is exercised yet.

What lands on the team: from the merge onward, a frontend pull request with a strict violation on a line it wrote will not merge. The spike's rate was ~2 findings per frontend pull request, all of them real.

Follow-up owed on the issue: observed duration and diff size from at least five real pull requests. The gate now sits on the critical path of every frontend merge, so if the real distribution is worse than the corpus suggested (8.4–9.4 s average, 12 s tail), the conversation is about making the harness faster.

Known unrelated redness: corpus.acceptance.test.mjs needs network access to the GitHub API and carries machine-dependent timing assertions. It is red offline both before and after this change; the other 10 test files pass with zero failures.

🤖 Generated with Claude Code

[claude]

Claude finished @nicobytes's task in 3m 16s —— View job

---

PR Review — strict-gate CI wiring

I reviewed the current diff (git diff origin/main...HEAD) focusing on the code that actually ships: core-web/pom.xml, lib/format.mjs, format.duration.test.mjs, and the base-resolution logic in .github/workflows/cicd_comp_test-phase.yml.

New Issues

• 🟡 Medium: .github/workflows/cicd_comp_test-phase.yml:290-305 — the first-parent base resolution is a genuinely better approach than the pull_request.base.sha one described in the earlier review threads (it survives a long-lived branch where the pinned base SHA goes stale). But the merge_group half rests on an assumption I can't verify from the repo. Assumption: the merge-queue checkout (github.event.merge_group.head_sha) is a two-parent merge commit whose first parent is the pre-this-PR queue state. What to verify: if the repo's merge method ever produces a linear queue commit (single parent — e.g. rebase/squash queue strategy), SECOND_PARENT is empty, STRICT_GATE_BASE is never exported, and the gate silently falls back to comparing against the origin/main tip — at the one place (merge_group) where that over-blames the author for violations main introduced. The ::warning fires, but it lands unprefixed in a long Maven log with useMavenLogger=false, so in practice it's easy to miss. The fallback is safe-by-degradation rather than safe-by-correctness at the enforcement point. Worth confirming the queue actually yields a merge commit for this repo's config, or the enforcement path is the weakest-attributed one.

Resolved

Every finding from the prior Copilot and @oidacra rounds is addressed in the current diff:

• ✅ core-web/tools/scripts/strict-gate/lib/format.mjs:135-144 — costLine now counts distinct paths across targets[].files ∪ unmapped[].path (via Set), fixing both the config-assignment double-count and the dropped-unmapped-file undercount.
• ✅ lib/format.mjs:118-124 — durationMs.total is now guarded (?s on a malformed object), and sub-second runs render in ms rather than rounding to 0.0s.
• ✅ lib/format.mjs:170-205 — unmappedNote now renders in both the passing and findings branches; the note is hoisted to a single const note.
• ✅ lib/format.mjs:151 — the pass summary's duplicated "across N project config(s)" was dropped from the first sentence; costLine owns it. Verb changed from "Checked" to avoid the two-adjacent-lines-contradicting each other issue.
• ✅ format.duration.test.mjs — rewritten to equality assertions on an extracted cost line (costLineOf), closing the four surviving mutations; added findings-branch unmapped coverage, the blank-line-before-table case, the 40ms boundary, and the NaN→?s guard.
• ✅ core-web/pom.xml:519 — successCodes lists only 0 (blocking), documented; the deviation is recorded on Wire the diff-scoped strict typecheck gate into CI — merged on main but never executes #37536.
• ✅ core-web/pom.xml — the redundant plugin-level <configuration> block was removed (inherited from base <build>); comment explains why none is restated. ${skip.strict.gate} / strict.gate.base are their own properties with rationale; trunk/nightly/LTS activation traps documented; the "no required status checks" premise now names the 2026-08-24_incident-response vs disabled Default Merge Queue ruleset dependency.
• ✅ specs/.../data-model.md:87 — "Diff size" now correctly describes the unique-path derivation including unmapped; the non-existent report.files.length reference is gone.
• ✅ README.md:1-9 — the spike/production contradiction is reconciled.
• ✅ pre-push hook removed — the Copilot husky findings (stdin refs, exit-2 mislabel, stale origin/main, changed-path preflight) no longer apply to any committed code.

Notes (non-blocking)

• The base-resolution rewrite (first-parent) supersedes the base.sha approach the review threads discuss — the threads are now stale relative to the code. That's fine, just flagging for anyone reading the discussion top-to-bottom.
• The outstanding empirical verification the author committed to (throwaway TS7006 branch: exit 1, inline annotation, merge_group behaviour) is per the discussion already done via [DO NOT MERGE] probe: prove the strict gate blocks (#37536) #37564 and posted on Wire the diff-scoped strict typecheck gate into CI — merged on main but never executes #37536. No .ts changes here, so this PR's own CI still doesn't exercise the annotation/blocking path — the evidence lives on the issue.

Verdict: No blocking issues in the diff. The one Medium above is a "confirm the assumption" item on the merge_group attribution, not a demonstrated defect. The three deferred items (exit-2 on base-tip fallback, ::warning for unmapped, run.mjs internal deadline) are reasonably scoped to a follow-up since they cross the harness's decision logic, which this wiring deliberately doesn't touch.

• Read the diff
• Recheck prior review findings
• Check for new bugs
• Post review

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain in CI wiring, reporting, and pre-push behavior.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Activates the diff-scoped strict TypeScript gate for pull requests and adds local pre-push enforcement.

Changes:

• Wires the gate into Maven CI for pull requests.
• Adds pre-push enforcement.
• Adds duration reporting, tests, documentation, and specifications.

File summaries

File	Description	Review findings
specs/37536-wire-strict-gate-ci/spec.md	Requirements and acceptance criteria	No final review comments.
specs/37536-wire-strict-gate-ci/data-model.md	Outcome and duration model	Nit (2 votes): Documents report.files.length, which the implementation does not expose and cannot use for the documented diff-size calculation.
specs/37536-wire-strict-gate-ci/contracts/ci-hooks.md	CI and hook contracts	No final review comments.
core-web/tools/scripts/strict-gate/README.md	Gate documentation and status	Nit (1 vote): Contradictory “spike” and “wired and live” status wording.
core-web/tools/scripts/strict-gate/lib/format.mjs	Duration and summary formatting	Moderate (1 vote): Omits unmapped files from both summary branches. Moderate (3 votes): Reports assignment counts rather than unique changed files, excluding unmapped files from the measurement.
core-web/tools/scripts/strict-gate/format.duration.test.mjs	Duration summary tests	No final review comments.
core-web/pom.xml	Pull-request Maven gate execution	Critical (1 vote): Hard-codes origin/main, producing incorrect or failed comparisons for master pull requests. Moderate (1 vote): GitHub output lacks the scope/dependency-discard preamble.
core-web/.husky/pre-push	Local strict-gate enforcement	Moderate (3 votes): Runs the toolchain without a changed-path preflight for non-frontend pushes. Moderate (1 vote): Mislabels exit-2 execution failures as violations. Moderate (1 vote): Relies on potentially stale origin/main, diverging from CI.

Review details

Suppressed comments (6)

core-web/.husky/pre-push:52

• The hook ignores the refs supplied on pre-push stdin and always checks HEAD. A valid git push origin other-local-branch:remote-branch (or a multi-ref push) can therefore be allowed or refused based on a different commit than the one being sent. Parse the local ref/SHA from stdin, skip deletions, and pass that SHA as --head, or explicitly restrict the hook to current-branch pushes.

pnpm exec node tools/scripts/strict-gate/run.mjs \
    --base origin/main \
    --head HEAD \

core-web/.husky/pre-push:68

• For exit 2 (for example, an unresolved base or unreadable project graph), the harness may print no violations, but this wrapper still tells the developer to “Fix the violations above.” That misclassifies a harness failure and obscures the recovery path; distinguish findings (1) from a gate execution error before printing this guidance.

    echo "Push refused by the strict-gate check (exit $status)."
    echo "  Fix the violations above, or push anyway with:  git push --no-verify"

core-web/.husky/pre-push:51

• This local invocation assumes origin/main is current, but the hook never refreshes the remote-tracking ref; CI explicitly fetches origin/main immediately before Maven. A developer with a stale ref can therefore get a different result from the pull-request run, violating the documented claim that local and remote answers agree. Refresh the target base (or make the freshness prerequisite explicit and enforce it) before running the check.

pnpm exec node tools/scripts/strict-gate/run.mjs \
    --base origin/main \

core-web/pom.xml:518

• --format=github emits only ::error annotations; formatGithub() does not emit the required scope/dependency-discard preamble. The new CI log therefore starts with findings without telling agents that only changed lines are in scope (the separate Markdown summary does not change the formatter output). Add the scope preamble to the GitHub formatter before enabling this invocation.

                                        <argument>--format=github</argument>

core-web/tools/scripts/strict-gate/README.md:80

• This new section says the gate is "Wired and live", but the unchanged introduction still says "This is spike output, not production tooling" (line 9). After adding CI and pre-push enforcement, that description is contradictory and can lead contributors to treat the live hooks as unsupported or removable spike artifacts. Update the introduction/status wording to reflect that this is now temporary production tooling.

**Wired and live since #37536.** Two invocation points, deliberately asymmetric:

core-web/tools/scripts/strict-gate/lib/format.mjs:146

• formatMarkdown is the job summary used by the new --format=github CI path, but the pass branch never renders report.unmapped (and the findings branch omits it too). A changed TypeScript file that no project claims therefore appears as a clean pass with no indication that it was unexamined, contrary to the gate's edge-case contract; include the unmapped files/reasons in both summary branches.

            costLine(report)

• Files reviewed: 8/8 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[nicobytes]

Thanks — triaged all of it. Fixed in db56f5b.

Important context for anyone reading the bot reviews above: both were produced against earlier commits. Copilot reviewed the first push, which still contained core-web/.husky/pre-push; that hook was removed afterwards, so its four pre-push findings (stdin refs, exit-2 mislabel, stale origin/main, no changed-path preflight) no longer apply to any committed code. The Claude review caught the PR body still describing that hook — that was real, and the description has since been rewritten (the PR also became blocking, which it was not when either bot looked).

Fixed

🟠 Dangling core-web/.husky/pre-push reference in pom.xml — correct, and my mistake. When the hook was cut I updated the properties comment and missed this one inside the execution block. A developer would have gone looking for a file that was never added. Removed.

🟡 costLine counted assignments, not changed files — correct and the sharpest finding here. selectConfigs claims a source under every eligible config, so a project whose lib and spec configs both include a file yields two target entries for one changed file; targets[].files.length summed them and reported a one-file diff as two. Unmapped files were dropped entirely. Both defeat the only reason that line exists. Now counts distinct paths across targets[].files and unmapped[].path.

You were also right that the existing test never exercised it — it used distinct files across targets. Two new cases pin the two failure modes directly, and both were confirmed failing before the fix.

🟡 formatMarkdown never rendered unmapped — correct, and worse now the gate blocks: a changed TypeScript file that no project compiles read as a clean pass with nothing said about it, which is the edge case the spec names. The summary now lists them with the reason, in both the passing and the failing branch. Deliberately not a failure signal — it is the difference between "nothing was wrong" and "nothing was looked at".

🟡 data-model.md documented report.files.length — correct, no such field exists. Rewritten to describe the unique-path derivation including unmapped, so the contract and the implementation now say the same thing.

🟡 README contradiction — correct. "This is spike output, not production tooling" sat directly above a section saying it is wired and live. Reconciled.

Not changed, with reasons

🔴/🟡 --base=${git.origin.branch} hard-codes origin/main while the workflow also triggers for master PRs — the trigger list is real (cicd_1-pr.yml lines 25-29), but origin/master does not exist in this repository (git ls-remote --heads origin master returns nothing), so no pull request can target it today. This is latent, not active.

Two things make me want to leave it: the property is the pre-existing ${git.origin.branch} that lint-test and format-test already use, so the gate introduces no new divergence; and resolving the real base branch would add an untestable code path for a branch that does not exist. The bot's sharper point stands and is worth recording though — strict-gate is the first place where an unresolved base fails the build rather than degrading quietly. If master is ever created, this needs fixing first.

🟡 --format=github emits no scope preamble — accurate: formatGithub emits only ::error lines, and the scope rule lives in the Markdown job summary that the same run writes to $GITHUB_STEP_SUMMARY. So a human on the run page does get it; an agent scraping raw log lines does not.

That is a real concern and it is exactly what the harness's own design notes worry about. I am leaving it because changing the shape of the annotation output is more than the output-only carve-out this PR is operating under, and it is a harness design question from #37403 rather than something this wiring introduced. Worth its own issue if it bites.

Verification

• 7/7 duration tests green (3 of them new, all confirmed Red first)
• The 10 non-network harness test files pass with zero failures
• core-web/pom.xml parses; prettier --check clean

corpus.acceptance.test.mjs remains red offline both before and after — it resolves its corpus through the GitHub API and carries machine-dependent timing assertions.

[nicobytes]

Both of the things I owed you are moving.

Probe: #37564 — draft, DO NOT MERGE. Carries this branch plus one deliberate TS7006 at core-web/libs/utils/src/lib/dot-utils.ts:270. It should come back red, with the merge-base warning absent, one annotation on the added line, and the check failing on exit 1. A green run there would mean the gate does not block, which is the defect this issue exists to fix. I will post the three confirmations on #37536 and close it.

Scope recorded on #37536 — #37536 (comment). It states plainly that the blocking flip was a direct decision during implementation and was not re-approved against the issue's stated scope, lists all four deviations from the acceptance list, and carries the merge-base finding including the part I think matters most for anyone reading the original evidence: the "0 false positives" figure was measured with a correct merge base and does not transfer to the configuration the gate was about to ship in. That framing is yours.

Deliberately not in this pull request, tracked for a follow-up, each answered on its own thread: exiting 2 on a base-tip fallback, a ::warning for changed files no project claims, and an internal deadline in run.mjs for the watchdog-grandchild question. All three cross the harness's decision logic, which is the one boundary this change has held. Say so if you would rather any of them block here.

Thank you for the mutation testing — four of my assertions were passing for the wrong reason and one could never have failed at all. That is not something I would have found by rereading.

[nicobytes]

Probe evidence is in: #37536 (comment) (see the following comment). All three confirmed — base from the event payload, annotation anchored to dot-utils.ts:270 at failure level, and BUILD FAILURE on exit 1 with the job concluding failure.

#37564 closed and its branch deleted.

One thing the probe turned up that I would not have predicted: the base-tip warning still fires, and now fires on every run, because both sides remain shallow so merge-base can never traverse. The behaviour is right — the base is the divergence point — but the message still reads "Findings may include changes the base introduced", which is now false. An always-present warning that says something untrue is the kind of thing people learn to skip, which is a small version of the failure this issue is about. Added to the follow-up next to the exit-2 question, since the harness cannot presently distinguish a degraded fallback from a correct one.

From my side this is ready. Holding for your call on whether any of the three follow-ups should block here.