Conversation
The strict-gate harness merged with #37403 was wired into nothing and has never executed once. Since 2026-09-08 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. A `strict-gate` execution in core-web/pom.xml, inside the existing `validate` profile beside lint-test and format-test. `successCodes` lists 0 and nothing else -- findings (1) and a harness that could not run (2) both fail. A 180s timeout fails the same way, bounding the harness's unshallow git-fetch fallback. It runs in two contexts, and both are load-bearing for different reasons. `pull_request` is where the author reads it: the harness emits `::error file=,line=,col=` lines that GitHub renders inline on the changed lines, plus a job summary. `merge_group` is where it is ENFORCED. This repository declares no required status checks on main -- the ruleset requires a pull request, one approval, thread resolution and signed commits -- 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 first 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. Maven property activation has no OR, hence two profiles rather than one condition. No workflow file changes; activation reads the runner's own GITHUB_EVENT_NAME. `useMavenLogger` is pinned false with a comment even though it is the default, because 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. One output-only harness change: every run states its cost in the job summary. The gate now sits on the critical path of every frontend merge, so the real runtime distribution matters; the harness already measured every run and simply never printed it. What it decides is unchanged. Known gap, accepted knowingly: there is no escape hatch. A frontend change that legitimately must add a violation cannot merge until it is fixed or the gate is switched off repository-wide. The spike measured 0 false positives across its corpus, so the expected frequency is low, and inventing a bypass before anyone needs one tends to produce the bypass everybody uses. Scope: CI only. No local git hook; core-web/.husky/ and lint-staged.config.mjs are untouched, so no contributor's local workflow changes. No new dependency. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…typecheck-gate-into-ci-merged-on-main-but-never-executes
…hook reference (#37536) Addresses review on #37545. The cost line counted (config -> file) assignments, not changed files. `selectConfigs` claims a source under EVERY eligible config, so a project whose lib and spec configs both include a file produced two target entries for one changed file, and `targets[].files.length` summed them -- reporting a one-file diff as two. Unmapped files were dropped from the count entirely. Both defeat the only reason that line exists: to be the cost-versus-diff-size evidence for the runtime question. Now counts distinct paths across targets and unmapped. The original test never caught this because it used distinct files across targets. Two new cases pin the two failure modes directly. The job summary also never mentioned unmapped files, so a changed TypeScript file that no project compiles read as a clean pass with nothing said about it -- the edge case the spec names, and a worse one now the gate blocks. The summary now lists them with the reason, in both the passing and the failing branch. Not a failure signal: the difference between "nothing was wrong" and "nothing was looked at". Also removed a comment in core-web/pom.xml pointing at core-web/.husky/pre-push, a file this pull request does not add. It was left behind when the local hook was cut, and would have sent a developer looking for something that was never there. Reconciled two related staleness bugs: the harness README called itself "not production tooling" next to a section saying it is wired and live, and data-model.md documented `report.files.length` as the diff-size source when buildReport exposes no such field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…omments (#37536) Second round of review on #37545. Nothing here changes the gate's decision logic; it is all output correctness, test strength, and comments that taught mechanisms that do not exist. The test suite was weak in a way worth describing, because the reviewer found it by mutation testing rather than by reading. Four separate mutations of the implementation passed the whole suite: replacing the config count with a distinct-project count, splitting the duration onto its own line (defeating the stated purpose of pairing it with the diff size), removing the blank line that makes the findings table render, and dropping the unmapped note from the findings branch entirely. The common cause was whole-document regexes: formatMarkdown returns a multi-line document, and the unmapped note renders the same number-then-"file" shape the assertions searched for, so an assertion could be satisfied by a line other than the one under test. One assertion -- `doesNotMatch(/\b2\s*file/i)` -- could never fail for the bug it targeted, because the real output reads "2 changed file" and `\s*` does not match " changed ". Assertions are now equality against an extracted cost line, which is how the sibling suites already work. Two real bugs came out of that. The sub-second test asserted a value three rounding steps clear of the boundary; `toFixed(1)` renders anything under 50ms as "0.0s", and the measured no-op path is ~175ms -- a factor of four, reachable on a faster runner. Sub-second runs now render in milliseconds. And `durationMs.total` was unguarded: formatMarkdown is exported and buildReport's default only fires on undefined, so a partial object rendered "NaNs" in the job summary. The cost line also read "Checked N changed file(s)" directly above a note saying some of them were not examined -- two adjacent lines asserting opposite things about the same file. The verb is gone; the count is what it always was. Three POM comments taught mechanisms that are not there: - the skip was described as trunk/nightly having an empty diff. It is by event name. Trunk and nightly also accept workflow_dispatch from a feature branch, nightly disables change detection entirely, and cicd_5-lts.yml runs the frontend suite on release-* pushes -- so "it would be a harmless no-op" is false for cases that are nonetheless skipped. - the timeout was described as failing through successCodes. The watchdog throws directly and never consults them, so adding a success code to "allow timeouts" would do nothing. - the "no required status checks" premise holds today, but the ruleset providing it is an org-level `2026-08-24_incident-response`, and a separate `Default Merge Queue` ruleset does define required checks with enforcement disabled. Whoever re-enables it should see that this design assumed the opposite. Also removed the plugin-level <configuration> duplicated into the validate profile: verified against `help:effective-pom` that the base <build> entry already supplies executable, workingDirectory, PATH and NODE_OPTIONS -- the copy omitted NODE_OPTIONS and forked the PATH comment, so a future edit to the base would have silently missed it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This is the finding that would have made the gate unusable, caught in review on #37545 and reproduced in this PR's own CI run (job 104166954128): strict-gate: warning — no merge base between 'origin/main' and head; comparing against the tip of 'origin/main' instead. The cause is structural, not incidental. cicd_comp_test-phase.yml checks out at fetch-depth: 1 and compensates with `git fetch --depth=1`, so both sides of the comparison are truncated, `git merge-base` cannot traverse, and the harness takes its documented base-tip fallback. That fallback is survivable for `nx affected` -- over-including projects for lint is harmless on a lint-clean main -- which is why it went unnoticed while that was the only consumer. It is not survivable for a BLOCKING strict check, because main is deliberately not strict-clean while #37198 is in flight: every commit main moved ahead of the branch gets attributed to the pull request and strict-checked, so authors would be rejected for violations they never wrote. The harness's own comment on that code path records it producing 50 findings, essentially none of them the branch's own. The spike's "0 false positives" figure was measured with a correct merge base and does not transfer. Fix: resolve the base from the event payload (pull_request.base.sha, or merge_group.base_sha), fetch that one commit shallow, and hand it to the gate through a new `strict.gate.base` property. With the base already AT the divergence point, the base-tip fallback stops being a degradation and becomes exactly right -- no deepening, no extra fetch cost. `strict.gate.base` is deliberately its own property rather than a change to ${git.origin.branch}: lint-test and format-test tolerate an approximate base, this gate does not, and widening the blast radius to the two neighbouring checks is not something this change should do quietly. It defaults to ${git.origin.branch}, so a local run behaves exactly as before. Side effect worth naming: this also settles the hardcoded-origin/main concern raised separately in review. A pull request targeting a branch other than main now compares against its own base rather than against main. If the base commit cannot be fetched the step warns and leaves the gate on origin/main, which is today's behaviour rather than a new failure mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Throwaway. Proves three things no local run can: that the merge-base fix in 61f3fde removed the base-tip warning, that the annotation renders inline on the diff, and that exit 1 actually fails the check. DO NOT MERGE. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
11 tasks
github-actions
Bot
added
Area : CI/CD
Contributor
|
Claude finished @nicobytes's task in 1m 40s —— View job Claude finished @nicobytes's task in 1m 20s —— View job ReviewThis is the throwaway probe for #37536 / #37545 — explicitly DO NOT MERGE, red-by-design. I reviewed the substantive changes anyway (strict-gate New Issues
Notes (non-blocking)
VerdictCode is sound. The expected outcome — red, with one annotation on
· branch |
3 tasks
Member
Author
|
Done its job — evidence on #37536. The gate blocked as designed: annotation anchored to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Throwaway probe for #37545. Do not merge, do not review. Will be closed as soon as it has produced its evidence.
It carries every commit from #37545 plus one deliberate
TS7006atcore-web/libs/utils/src/lib/dot-utils.ts:270, because three things cannot be demonstrated from a local run and #37545 itself changes no.tsfile:no merge base between 'origin/main' and head; comparing against the tip of 'origin/main' instead, which withmaindeliberately not strict-clean would have rejected authors for violations they never wrote. 61f3fde resolves the base from the event payload. That warning should be absent here.::error file=,line=,col=to GitHub-annotation chain, which belongs to GitHub and cannot be tested anywhere else.Expected outcome: this PR is red, with one annotation on the added line. A green run here would mean the gate does not block, which is the defect #37536 exists to fix.
Raised by @oidacra in review on #37545.