Skip to content

ci(merge-train): run the ci.yml:lint family on the combined tree - #14489

Merged
diegosouzapw merged 4 commits into
release/v3.8.51from
fix/merge-train-lint-family-gates
Sep 24, 2026
Merged

diegosouzapw merged 4 commits into
release/v3.8.51from
fix/merge-train-lint-family-gates

Conversation

@diegosouzapw

@diegosouzapw diegosouzapw commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

Why

The 2026-09-18 drain merged 170 PRs through merge-train.sh and left release/v3.8.51 red on eleven ci.yml:lint / docs-sync-strict gates. Not one of them could have failed on a single PR — each PR was green on its own fast-gates, and only the combined tree broke the contract:

What broke on the tip Gate that would have caught it
Two boarded PRs both claiming migration 181 (#12814 + #13610) — 226 unit tests red on the next train check:migration-numbering
5 process.env.* reads with no .env.example entry (#14006, #13222, …) check:env-doc-sync
/v1/responses/input_tokens route without Zod validation (#13167) check:route-validation:t06
Generated SKILL.md left stale by the open-wa service routes check:agent-skills-sync
README/AGENTS/llm.txt migration counts drifting from the code check:docs-counts
TS2698 under the dashboard tsconfig (#13711) check:dashboard-typecheck

The train validated exactly five static gates (typecheck:core, file-size, the two complexity ratchets, changelog-integrity), so all of that stayed invisible until CI ran on the tip hours later — and every PR boarding afterwards inherited the red.

What changes

  • STATIC_GATES gains the cheap half of the family — migration-numbering, env-doc-sync, route-validation:t06, db-rules, vitest-exclusions, tracked-artifacts, cycles, provider-consistency, error-helper, known-symbols. ~1 min total on the devbox, measured on the tip.
  • FULL_ONLY_GATES (new tier) holds the minutes-long ones — agent-skills-sync (76s), route-guard-membership (46s), docs-counts (164s), dashboard-typecheck (minutes) — so an intra-day --fast train stays fast and the daily FULL train covers them.
  • --plan prints both tiers (--fast: 25 steps, full: 29).

Every gate carries an inline comment naming the failure class it catches, so the next person extending the list knows what each one is for.

Evidence

  • tests/unit/merge-train-plan.test.ts — 8/8 green.
  • bash -n clean; --plan --fast and --plan both enumerate correctly.
  • Timings measured on origin/release/v3.8.51 (listed in the commit message).

Second commit — classify a red gate instead of blaming the train

Adding the gates above only helps if the train can tell whose red it found. It could not: it aborted on the first failure, with no statement about the base. During the 2026-09-22 drain that meant a full stop per gate, four of which (docs counts, mutation coverage, two API typecheck errors) were already red on the release tip.

merge-gates.md §3 already requires reproducing a failure on origin/<base> before anyone may call it inherited. The script now does that itself:

outcome on origin/<base> what the train does
gate is green the train owns the red → abort, exactly as before
gate is red, same violation lines reported as ⚠ INHERITED, recorded, train continues
gate is red, train adds a line abort — and the added lines are printed

That third row is the point: a bare "it was already red" is precisely how a real regression rides in behind an inherited failure. Forgiveness is granted per violation line, never per gate name.

The base probe is a detached worktree created lazily — nothing is cut while the suite is green — and it is removed by the cleanup trap that already existed.

Only the npm run check:* gates are discriminated. The unit and vitest gates run files the boarded PRs added; those do not exist on the base, so a red there would be "file not found", not "inherited". They abort and bisect as before.

The closing summary lists every inherited gate, and the per-PR evidence line carries the count so a merge made over an inherited red is self-documenting.

Tests: merge-train-plan 10/10 — two new, covering the discrimination wiring (base probe, teardown, per-line comm, which loops pass the flag, and that vitest does not) and the --plan text.

Rework (merge-batch 2026-09-23)

  • Merged origin/release/v3.8.51 (real merge, no rebase).
  • INHERITED now fails closed. Before, gate_violations only matched lines starting with ✗/✖/×/FAIL/✘. A gate that reports errors another way, such as tsc error TS… from dashboard-typecheck, produced an empty set on both sides, and the empty comm read as "inherited" even when the train added errors. The new classify_gate_red works as follows:
    • GATE_ERROR_RE now also matches error TS[0-9]+.
    • A red gate with no recognisable violation line is UNCLASSIFIABLE, so the train owns it (exit 2).
    • A violation line that exists on the train but not on the base is NEW.
    • If the train has more violation lines than the base (a repeated violation), it is NEW.
    • It is INHERITED only when none of the above applies.
  • The base probe now hard-links node_modules with cp -al, following the repo convention. If it hits "Too many links", it drops the partial tree and falls back to a symlink. The main train worktree's ln -s (which predates this PR) was left as is.
  • New behavioral tests in tests/unit/merge-train-plan.test.ts pull the functions out of the script and run them in bash. The cases:
    • tsc gate red on the base, train adds error TS2339 → not forgiven.
    • Identical TS errors → INHERITED.
    • Red with no recognisable violation → UNCLASSIFIABLE.
    • Repeated violation → NEW.
    • Red→green: with the old logic 3 of 14 tests fail. With the fix, 14/14 pass.
  • Gates: typecheck:core shows only the inherited cliproxyAccountHealth.ts(157,5). check:open-sse-typecheck fails only on the inherited auggie.ts, which this PR doesn't touch. check-file-size OK, eslint and prettier OK, bash -n OK.

A 170-PR drain (2026-09-18) left release/v3.8.51 red on eleven ci.yml:lint and
docs-sync-strict gates. None of them could fail on a single PR: every PR was
green on its own fast-gates, and only the merged tree broke the contract — two
boarded PRs both claiming migration version 181 (a 226-test red on the next
train), process.env reads with no .env.example entry, a new route without Zod
validation, generated SKILL.md left stale, README/AGENTS counts drifting from
the code.

The train validated exactly five static gates, so none of that was visible
until CI ran on the tip hours later. This adds the cheap members of the family
to STATIC_GATES (~1 min total on the devbox) and puts the minutes-long ones in
a new FULL_ONLY_GATES tier, so an intra-day --fast train stays fast while the
daily FULL train covers the rest. --plan prints both tiers.

Timings measured on the release tip: migration-numbering 3s, env-doc-sync 4s,
route-validation 4s, db-rules 5s, vitest-exclusions 2s, tracked-artifacts 3s,
cycles 2s, provider-consistency 2s, error-helper 17s, known-symbols 27s;
FULL-only: agent-skills-sync 76s, route-guard-membership 46s, docs-counts 164s,
dashboard-typecheck minutes.

tests/unit/merge-train-plan.test.ts: 8/8.
…ing the train

The train aborted on the first red gate and said nothing about whose red it was.
During the 2026-09-22 drain that cost a full stop per gate: docs counts, mutation
coverage and two API typecheck errors were all already red on the release tip, and
each one had to be reproduced on `origin/<base>` by hand before the train could
move again.

merge-gates.md §3 already requires that reproduction before a red may be called
inherited. This makes the script do it:

- On a red static gate, the same command is re-run in a throwaway worktree cut from
  `origin/<base>` (created lazily — no cost when the suite is green, torn down by the
  existing cleanup trap).
- Green on the base → the train owns the red, abort exactly as before.
- Red on the base → compare violation lines. Identical set → reported as INHERITED,
  recorded, and the train CONTINUES. Any line the train adds → abort, and the added
  lines are printed, so "it was already red" can no longer wave a real regression
  through.
- The final summary lists every inherited gate and repeats that the base stays red
  until they are drained; the per-PR evidence line carries the count.

Only `npm run check:*` gates are discriminated. The unit and vitest gates run files
the boarded PRs added — those do not exist on the base, so a red there would mean
"file not found", not "inherited". They abort and bisect as before.

Tests: merge-train-plan 10/10 (2 new — the discrimination wiring and the --plan text).
The inherited-red check only matched ✗/✖/×/FAIL/✘ marker lines, so a gate
reporting errors another way (tsc 'error TS…' from dashboard-typecheck) had an
empty violation set on both sides and was forgiven even when the train added
errors. Now tsc diagnostics count as violations, an empty set is UNCLASSIFIABLE
(the train owns it), and a growth in violation-line count is NEW. The base probe
hard-links node_modules (cp -al) instead of symlinking it.
@diegosouzapw
diegosouzapw merged commit 705a02e into release/v3.8.51 Sep 24, 2026
14 of 21 checks passed
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