Skip to content

fix(release): drain two v3.8.52 base-reds — cycles membership test, drifted skills mirrors - #15682

Open
woodsonl wants to merge 14 commits into
diegosouzapw:release/v3.8.52from
woodsonl:fix/release-v3.8.52-basereds-r2
Open

woodsonl wants to merge 14 commits into
diegosouzapw:release/v3.8.52from
woodsonl:fix/release-v3.8.52-basereds-r2

Conversation

@woodsonl

@woodsonl woodsonl commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #15306

Summary

Follow-up drain to #15590 (merged as b8c50e1). CI-only: no runtime code changes. Fixes the two base-reds that red every PR off release/v3.8.52 and can be fixed mechanically. Cut fresh from tip f43c4679e6; base has since been merged in twice (+90 commits at the r2 cut, then +83 more at 98afa2c5a4), and a docs-reconciliation commit (8991e0544b) rides on top. Reviewed through two gstack fix cycles, a clean certification on the pre-merge tree, and a clean re-certification on the merged tree.

  1. Membership test (bf7c24a66d, hardened in 9707949fac, 2bb94e374e) — the fix(release): drain three v3.8.52 base-reds — cycles fast-gate, stale PR base sha, integration port race #15590 squash carried the quality.yml move of cycles from the plain gates array into ratchet_gates, but not the matching update to tests/unit/quality-rail-gate-membership.test.ts. The test still asserted the old needle "cycles lockfile duplication dead-code type-coverage compression-budget", which no longer exists as a substring. Every PR off the base failed Unit Tests (3/8) and Unit Tests fast-path (3/4) with AssertionError: fast-gates must contain "cycles lockfile duplication dead-code type-coverage compression-budget". Evidence: run 37377709748, jobs 111992436180 and 111992148972, 1 fail in 5802 tests, both shards, same test. Confirmed still live on base tip before this fix.

    The fix pins the guard in both directions: positive needles for lockfile duplication dead-code type-coverage compression-budget (plain gates) and secrets vuln-ratchet workflows openapi-breaking cycles (ratchet_gates), plus a negative assertion that matches the whole gates=(...) array slice and fails on any standalone cycles token inside it. An injection rig (a standalone cycles line added inside the array) trips the guard; the pristine workflow passes 5/5. A comment carries the rationale: since fix(ci): make check-cycles honest — real graph walk + cycles ratchet (#15159 G-01/G-02) #15281 (69cb18d), check-cycles.mjs exits 1 on ANY cycle without --ratchet, so fast-gates must run it with the frozen ceiling in quality-baseline.json (metrics.cycles = 14), the same check:cycles:ratchet semantics ci.yml already runs.

  2. Skills mirrors (c833ee0f8b) — check:agent-skills-sync dry-run reported Generated: 2: the omni-auth SKILL.md and the omni-settings/references/endpoints.md mirror drifted from their sources, failing the Merge integrity (changelog + generated skills) job on every PR. Evidence: job 111991763543 (run 37377709748). Same failure class as chore(skills): regenerate omni-providers endpoints reference from openapi.yaml #15652, which regenerated omni-providers. Fix: ran node --import tsx/esm scripts/skills/generate-agent-skills.mjs --apply and committed the generator output verbatim (2 files, +5/−3).

  3. Comment corrections (45bfd7fbbc, d051c0b6ac) — scripts/check/check-cycles.mjs header and the readBaselineCyclesValue JSDoc claimed plain mode is "advisory" with no ceiling. fix(ci): make check-cycles honest — real graph walk + cycles ratchet (#15159 G-01/G-02) #15281 shipped those comments together with the fail-closed exits they contradict: plain mode exits 1 on any cycle, and --ratchet exits 1 above the ceiling or when the baseline is missing/invalid (zero cycles exits 0 in every mode). Comment-only; behavior verified unchanged (probes below).

  4. Changelog fragments (23643076b4, split by 1809bb367c) — changelog.d/fixes/15682-basereds-r2-membership-test.md (fix bullet) and changelog.d/maintenance/15682-basereds-r2-skills-mirrors.md (chore bullet), matching the repo's fragment taxonomy.

Related Issues

Validation

  • Change type: CI/tooling only (1 test file, 2 generated skill mirrors, 2 changelog fragments, comment-only edits in 1 check script). No src/, open-sse/, electron/, or bin/ runtime code touched.
  • Red-first evidence: the membership test fails on base tip (fast-gates must contain "cycles lockfile duplication …"), passes 5/5 after the fix.
  • Guard hardening evidence: injection rig proves the array-slice assertion trips on a same-array-different-line cycles re-entry; the earlier line-anchored version did not.
  • Green runs: membership test 5/5 at head d051c0b6ac; skills dry-run Generated: 0 · Unchanged: 46 · Pruned: 0 · Orphans: 0 · Errors: 0; check:changelog-integrity OK; check:cycles exits 1 (the guarded failure mode) and check:cycles -- --ratchet exits 0.
  • Pre-commit gates green on all seven commits: lint-staged (prettier + eslint), check-docs-sync, check:any-budget:t11, check:tracked-artifacts, check:ai-attribution.
  • Base reconciliation: branch cut from origin/release/v3.8.52 tip f43c4679e6; no merge-base drift.
  • Mutation check: n/a (no runtime code changed).
  • Complexity ratchets: n/a (no source files changed).

Tests Added Or Updated

  • tests/unit/quality-rail-gate-membership.test.ts — updated the fast-gates membership needles and added the array-slice absence assertion (5 assertions in the file, all green). No new test files; the skills regeneration is validated by the existing check:agent-skills-sync gate.

Coverage Notes

No runtime code changed, so the coverage gate is not exercised by this diff. The one changed test file is itself a guard test.

Review record (gstack, 2026-10-06)

  • ponytail-review: nothing to cut, net: 0 lines possible.
  • Adversarial (Claude in-host + Codex outside), two fix cycles, then a clean certification pass on d051c0b6ac: all findings fixed in this PR (the guard hardening and comment corrections above); final verdicts "approve/merge as-is" and "no blocking findings". Review log: status clean, 0 issues, quality 10/10, converged after 2 cycles.
  • gstack-investigate root cause for the comment drift: #15281 (69cb18d) shipped the advisory wording and the fail-closed code in the same commit; the comments were wrong from birth.
  • Exploratory QA + /qa: 6 contracts per invocation (membership guard, injection-rig rejection, skills-sync, changelog integrity, plain-cycles fail-closed, ratchet pass), all green with evidence receipts FRESH on the final tree 8991e0544b.
  • Base merge (+83 commits) re-certified: the delta is byte-consistent with the twice-certified revision; none of the 83 commits touch any delta-invalidating surface; Codex verdict "retain certification".
  • Documentation audit (/document-release): 72 doc files reconciled in 8991e0544b — QUALITY_GATES.md check:cycles row corrected (same "advisory" error the script comments had), RELEASE_CHECKLIST switched to check:cycles:ratchet, and the strict docs-counts drift (migrations 193 → 194, inherited from base) fixed across README/AGENTS/llm.txt + 66 locale mirrors. check:docs-all passes except a pre-existing base red (check:env-doc-sync: NEXT_MANUAL_SIG_HANDLE and OMNIROUTE_ESTIMATOR_CALIBRATION in code but missing from .env.example) — deferred, needs execution-input changes, not docs.

Reviewer Notes

⚠️ Merge approval flag (operator): commit 8991e0544b touches AGENTS.md and llm.txt (+ 66 locale mirrors of llm.txt) — agent-instruction surfaces that require explicit operator approval to merge per the repo's review rules. The changes are mechanical (migration-count 193 → 194, matching base tip's actual migrations; precedent #15329), but the flag is recorded here because the rule requires it. QUALITY_GATES.md and RELEASE_CHECKLIST.md in the same commit are ordinary docs.

Still expected red on this PR, all inherited and tracked in #15306, none fixable honestly in this diff:

Completes the diegosouzapw#15590 landing: the squash carried the quality.yml move of
cycles from the plain gates array into ratchet_gates but not the matching
membership-test update, so tests/unit/quality-rail-gate-membership.test.ts
reds every PR off release/v3.8.52 (CI Unit Tests 3/8 + fast-path 3/4,
2026-10-05 run 37377709748). The G0 guard requires the edit to ride along
with the gate move, with the reason in the diff. Base-red diegosouzapw#15306.
check:agent-skills-sync reds every PR off release/v3.8.52: the omni-auth
SKILL.md and omni-settings endpoints reference drifted from their sources
(Merge integrity job, run 37377709748). Same regeneration as diegosouzapw#15652 for
omni-providers. Base-red diegosouzapw#15306.
@woodsonl
woodsonl requested a review from diegosouzapw as a code owner October 6, 2026 21:30
Adversarial review finding on diegosouzapw#15682: the needles pinned cycles' presence
in ratchet_gates but not its absence from the plain gates array, so a
future PR could re-add cycles to gates[] while the test stayed green —
plain check-cycles.mjs then exits 1 on ANY cycle and reds every PR at
the frozen 14-SCC ceiling. assert.doesNotMatch on the plain-gates line
closes it.
…-closed reality

diegosouzapw#15281 (69cb18d) shipped both the comments and the fail-closed exits in
the same commit: plain mode exits 1 on ANY cycle and --ratchet with a
missing/invalid baseline falls through to exit 1 (main() lines 464-497),
but the header and readBaselineCyclesValue docstring still described
plain mode as advisory with no ceiling. Comment-only; behavior unchanged.
The chore(skills) bullet sat in fixes/; the repo's changelog taxonomy
keeps non-fix bullets under changelog.d/maintenance/ (precedent:
15225, 15252, 15389). Review finding on diegosouzapw#15682; no gate change.
Re-certification rig on diegosouzapw#15682: the line-anchored doesNotMatch missed a
same-array-different-line re-entry (cycles on its own line inside
gates=(...)); the test passed while plain check-cycles would exit 1 in
CI. Match the full gates=(...) array slice so any line inside it trips.
Verified: pristine workflow 5/5; injected standalone-cycles line trips.
main() exits 0 before reading the baseline when cycles.length === 0, in
every mode; the previous wording implied a missing baseline blocks even
that path. Comment-only.
- README/AGENTS/llm.txt (+ i18n mirrors): 193 to 194 SQL migrations,
  clearing the strict docs-counts drift inherited from release/v3.8.52
- QUALITY_GATES.md + RELEASE_CHECKLIST.md: bare `check:cycles` exits 1
  on any cycle since diegosouzapw#15281 (fail-closed, not advisory); the release
  checklist now runs `check:cycles:ratchet`
diegosouzapw added a commit that referenced this pull request Oct 8, 2026
Drains 9 v3.8.52 tip unit reds: 1 production fix (combo family-fallback skipped for request-scoped refusals, regression of #13603) + 8 test alignments to intentional contract changes (#15301 #15132 #15035 #15478 #15310 #15710 #15487), each annotated. Validated on a combined board with #15902 after clean npm ci: 32 previously-red files 327/329 (remaining 2 = quality-rail-gate-membership owned by #15682, and route-namespaces fixed in #15902), typecheck/open-sse/api typecheck, lint, 22 gates.
woodsonl and others added 2 commits October 9, 2026 12:54
Preserve the measured 202 migrations from the release base, retain the cycle membership regression guard and update the two generated mirror contracts without claiming to resolve unrelated skill drift.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Integrate release/v3.8.52 at ba594f1 without rewriting contributor history or changing the nine-file repair delta.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Owner

Maintainer reconciliation update — 2026-10-09

Published head: ed0c40fa645d59e8317dbf9da89384e670b20c1b.
Validation base: ba594f1003f9d8907db187201b563a9fd7cda634.
Candidate tree: 1c71b02e2f55f4531b37b0862afaf2ab63bd3490.

The normal merge preserves the contributor history and Lance Woodson's authorship.
The effective repair delta remains nine files. Count conflicts retain the release's
measured 202 SQL migrations; the French README now agrees. Cycle membership guards,
fail-closed cycle documentation and the OIDC/settings mirror corrections remain.
The changelog no longer claims two mirror corrections resolve all generated-skill drift.

Executed against the reconciled candidate, Node 24.20.0:

  • Focused membership/cycle tests: 27 PASS, 0 FAIL, 0 SKIP, exit 0.
  • npm run check:docs-all: exit 0.
  • GITHUB_BASE_SHA=ba594f1003f9d8907db187201b563a9fd7cda634 node scripts/check/check-test-masking.mjs: exit 0, no weakening.
  • Normal pre-commit/commit-msg hooks and own-delta whitespace check: exit 0.

Physical dependencies were initialized with a successful npm ci on the earlier
35ef635 base. The base update only promotes an already-installed
intl-messageformat@11.2.14 to a root dev dependency; every non-root lockfile package
record remains identical and npm ls intl-messageformat --depth=0 passes. This is
not a claim that a second clean install or full release matrix ran on the new head.

HOLD for merge. The canonical, environment-neutral generated-skills dry run still
exits 2: 20 drifted, 26 unchanged, 0 generation errors. Existing #15945/#15948 are
separate repair candidates, not silently folded into this PR. Required remote checks
for this new head were queued at the last inspection. No full unit/MCP/UI, coverage,
build, package, boot or release-green acceptance is claimed. No admin bypass,
force-push, baseline increase, release merge or deployment was performed.

diegosouzapw added a commit that referenced this pull request Oct 9, 2026
…ossary, route map, ToS-default test fallout (#16092)

Validated on the branch: 94/94 node tests across the touched files + glossary/route-map gates, the 4 vitest files 20/20 run individually, API typecheck and eslint clean. Remaining tip reds owned elsewhere: executor golden (#16058), quality-rail (#15682), chaosVirtualCombo (#16090), skills sync (#15945/#15948).
Preserve the contributor refresh and validate against 7510677. The effective delta remains eight tooling/documentation files; generated skills and full release gates remain separate holds.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Owner

Maintainer reconciliation — 2026-10-10

Thanks @woodsonl for the original repair and the contributor-side base refresh.
The normal fast-forward push now publishes head
1392697f0b7169a06885ce9132ddf6bb891c26f6, tree
49135cb411e5e39bad5f12f32f171f1c96afa762, against the pinned live release base
7510677f2f8f5317f1947284dab650d01333915d.

The merge retains your latest 5d724ed44bfd95e83e0079a95d1fa80d97dcf9e1
and the release as real parents. Lance Woodson remains the author; the maintainer
is credited as co-author. The effective delta is eight tooling/documentation files.
No runtime implementation, baseline or assertion was weakened.

Evidence on this prepared candidate (Node 24.20.0):

  • Exact-base membership test: RED, 4 PASS / 1 FAIL, exit 1.
  • Candidate membership test: 5 PASS, exit 0.
  • Focused membership, cycle-blind-spot and BaseExecutor cycle suites: 29 PASS,
    0 FAIL / SKIP / CANCELLED, exit 0.
  • npm run check:docs-all: exit 0; 98 advisory potential-drift warnings remain.
  • Test-masking against the pinned base: exit 0, no weakening.
  • Changelog integrity, own-delta whitespace and normal commit hooks: exit 0.

These runs used the earlier physical dependency cache. The new base changes
Next/@next dependency records, so the runs are diagnostic, not a fresh-install or
full acceptance certificate. A clean install of the pinned current base is being
tracked separately; it is not claimed complete here.

HOLD for merge. The exact base's generated-skills check still reports
20 drifted / 26 unchanged / 0 generation errors (exit 2, job 114156146098).
PRs #15945 and #15948 remain separate content/root-cause repairs. Current-base
Node compatibility tests also have real failures outside this PR; no claim is
made that this change clears the entire release matrix. Fresh required checks
for the published head must succeed before admission.

No admin override, force push, hook bypass, baseline increase, merge to release/main,
tag, publication or deployment was performed by this reconciliation.

This branch has not been deployed

No deployments
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.

2 participants