CI gap: strictTemplates / app-build errors merge to master green (nx build spikersoft not gated) #824

New Issue

2026-07-24T02:56:31Z

spikerj commented

2026-07-24 02:56:31 +00:00

Problem

A strictTemplates type error broke master's production app build for at least one merge cycle without CI going red (see #823 — humanizeDuration(number | null) in photo-lightbox). The merge gates did not catch it because:

The full nx build spikersoft --configuration production (which type-checks every component template it pulls in) is not run as a required merge check, and
feature-photo-gallery:test — whose vitest builder performs the same template type-check — is among the Angular projects not gated (path-filtered / untested), per the known coverage-gap epic.

Net effect: PR #560 merged green having only satisfied the eqeqeq ESLint rule, while the underlying template type error remained and broke the deployable build.

Impact

master can be in a state where no deployable app bundle builds, and nothing surfaces it until someone runs a local/production build. Lint + the gated unit subset are green, giving false confidence.

Suggested fixes (pick per cost)

Add a required CI job that runs nx build spikersoft --configuration production (or at least nx affected -t build) on PRs touching libraries/** or projects/**.
Ensure feature-*:test (which template-type-checks via the vitest builder) is not path-filtered out for changes in its project graph — nx affected -t test rather than a static path allowlist.
Longer term: land the root typecheck gate (blocked by #664) so template errors fail fast without a full build.

Relates to the CI test-coverage-gaps thread and #823.

## Problem A strictTemplates type error broke `master`'s production app build for at least one merge cycle without CI going red (see #823 — `humanizeDuration(number | null)` in photo-lightbox). The merge gates did not catch it because: - The full **`nx build spikersoft --configuration production`** (which type-checks every component template it pulls in) is not run as a required merge check, and - **`feature-photo-gallery:test`** — whose vitest builder performs the same template type-check — is among the Angular projects not gated (path-filtered / untested), per the known coverage-gap epic. Net effect: PR #560 merged green having only satisfied the `eqeqeq` ESLint rule, while the underlying template type error remained and broke the deployable build. ## Impact `master` can be in a state where **no deployable app bundle builds**, and nothing surfaces it until someone runs a local/production build. Lint + the gated unit subset are green, giving false confidence. ## Suggested fixes (pick per cost) 1. Add a required CI job that runs `nx build spikersoft --configuration production` (or at least `nx affected -t build`) on PRs touching `libraries/**` or `projects/**`. 2. Ensure `feature-*:test` (which template-type-checks via the vitest builder) is not path-filtered out for changes in its project graph — `nx affected -t test` rather than a static path allowlist. 3. Longer term: land the root `typecheck` gate (blocked by #664) so template errors fail fast without a full build. Relates to the CI test-coverage-gaps thread and #823.

spikerj referenced this issue

2026-07-24 18:56:45 +00:00

Angular master e2e-anonymous RED since #827 PR #564: /sponsor/donate-family/:familyKey unclassified in route-manifest → publish skipped, frontend not deploying #828

spikerj commented

2026-07-24 18:59:24 +00:00

Second instance of this exact gap today, worth folding into the scope here.

#828: PR #564 added a route to `routes.ts` without classifying it in `e2e/support/route-manifest.ts`. That is only caught by `route-coverage.spec.ts` in the `e2e-anonymous` job, which is not required for merge — so it merged green and then failed on master, and because `publish` is gated on the e2e jobs, the frontend stopped deploying entirely (learn.spikersoft.com served a pre-#564 bundle for over an hour without anyone being paged).

So the blast radius of the missing gate is bigger than a red badge: a post-merge-only failure in an e2e job silently halts production deploys. Whatever gating this ticket lands should cover both `nx build spikersoft` and the route-manifest check, and ideally a notification when `publish` is skipped on master.

— Opus 5 Agent

Second instance of this exact gap today, worth folding into the scope here. #828: PR #564 added a route to \`routes.ts\` without classifying it in \`e2e/support/route-manifest.ts\`. That is only caught by \`route-coverage.spec.ts\` in the \`e2e-anonymous\` job, which is not required for merge — so it merged green and then failed on master, and because \`publish\` is gated on the e2e jobs, **the frontend stopped deploying entirely** (learn.spikersoft.com served a pre-#564 bundle for over an hour without anyone being paged). So the blast radius of the missing gate is bigger than a red badge: a post-merge-only failure in an e2e job silently halts production deploys. Whatever gating this ticket lands should cover both \`nx build spikersoft\` and the route-manifest check, and ideally a notification when \`publish\` is skipped on master. — Opus 5 Agent

spikerj commented

2026-07-29 15:55:27 +00:00

Resolved — all three suggested fixes have landed, via the epic #722 CI restructure (the workflow is now .gitea/workflows/main.yml; the old tests.yml this ticket was written against no longer exists). Verified against origin/master:

Fix 1 — the production app build is now a PR gate. main.yml:114 defines a build job with needs: [compute_version], running pnpm run build at :191-192. That script is nx build spikersoft --configuration production --verbose --skip-nx-cache, i.e. the full build that type-checks every template it pulls in. The workflow triggers on pull_request: branches: [master] (:5-6) with no paths: filter, and the job carries no continue-on-error and no if: guard — so it runs on every PR regardless of which files changed. publish additionally needs: [test-and-lint, e2e-anonymous, build, compute_version] (:479), so a broken app build blocks the pipeline rather than merging green.

Fix 2 — the static path allowlist is gone, replaced by nx affected. main.yml:407-410 runs pnpm nx affected -t lint --base=origin/master --head=HEAD on PRs, and the test step does the same with -t test, falling back to the full run-many/test-all on pushes to master. The comment at :401-405 records the reasoning: PRs lint/test the affected graph slice, pushes keep the full run so merge-batch effects can't slip through an affected diff against HEAD~1. That's exactly what this ticket asked for — feature-photo-gallery:test is now reached whenever it's in the changed project graph, rather than depending on a path match.

Fix 3 — the typecheck gate landed too, which is notable because this ticket recorded it as blocked by #664. Rather than the root tsc --noEmit (unusable across mixed project configs, which was #664's problem), main.yml:421-427 runs a per-project typecheck target inferred by the local tools/workspace-plugin from tsconfig.lib.json / tsconfig.spec.json plus the app tsconfig — same affected-on-PR / full-on-push split. So template errors now fail fast without needing a full build, and #664's blocker was routed around rather than waited on.

Net effect: the #823 scenario — a strictTemplates error merging green because only eqeqeq was checked — is now caught three separate ways.

Closing.

Resolved — **all three suggested fixes have landed**, via the epic #722 CI restructure (the workflow is now `.gitea/workflows/main.yml`; the old `tests.yml` this ticket was written against no longer exists). Verified against `origin/master`: **Fix 1 — the production app build is now a PR gate.** `main.yml:114` defines a `build` job with `needs: [compute_version]`, running `pnpm run build` at `:191-192`. That script is `nx build spikersoft --configuration production --verbose --skip-nx-cache`, i.e. the full build that type-checks every template it pulls in. The workflow triggers on `pull_request: branches: [master]` (`:5-6`) with **no `paths:` filter**, and the job carries no `continue-on-error` and no `if:` guard — so it runs on every PR regardless of which files changed. `publish` additionally `needs: [test-and-lint, e2e-anonymous, build, compute_version]` (`:479`), so a broken app build blocks the pipeline rather than merging green. **Fix 2 — the static path allowlist is gone, replaced by `nx affected`.** `main.yml:407-410` runs `pnpm nx affected -t lint --base=origin/master --head=HEAD` on PRs, and the test step does the same with `-t test`, falling back to the full `run-many`/`test-all` on pushes to master. The comment at `:401-405` records the reasoning: PRs lint/test the affected graph slice, pushes keep the full run so merge-batch effects can't slip through an affected diff against `HEAD~1`. That's exactly what this ticket asked for — `feature-photo-gallery:test` is now reached whenever it's in the changed project graph, rather than depending on a path match. **Fix 3 — the typecheck gate landed too**, which is notable because this ticket recorded it as blocked by #664. Rather than the root `tsc --noEmit` (unusable across mixed project configs, which was #664's problem), `main.yml:421-427` runs a **per-project** `typecheck` target inferred by the local `tools/workspace-plugin` from `tsconfig.lib.json` / `tsconfig.spec.json` plus the app tsconfig — same affected-on-PR / full-on-push split. So template errors now fail fast without needing a full build, and #664's blocker was routed around rather than waited on. Net effect: the #823 scenario — a strictTemplates error merging green because only `eqeqeq` was checked — is now caught three separate ways. Closing.

spikerj closed this issue

2026-07-29 15:55:40 +00:00

spikerj referenced this issue

2026-07-29 16:07:05 +00:00

storybook: static build broken on master (addon-actions import, node built-ins, worker tsconfig) #728

spikerj referenced this issue

2026-07-29 16:07:17 +00:00

Hygiene: event-modal debug console.logs ship to prod + 6 templates spam NG8011 projection warnings every build #761

spikerj referenced this issue

2026-07-29 17:16:30 +00:00

EPIC: Angular 22 + Nx 23 full-leverage modernization (signal forms, resource APIs, affected CI, remote cache) #722

spikerj referenced this issue