Skip to content

test: promote six regression anchors, each measured green on main and red without its fix - #2094

Open
kriszyp wants to merge 4 commits into
mainfrom
kris/qa-promote-measured-anchors
Open

test: promote six regression anchors, each measured green on main and red without its fix#2094
kriszyp wants to merge 4 commits into
mainfrom
kris/qa-promote-measured-anchors

Conversation

@kriszyp

@kriszyp kriszyp commented Aug 5, 2026

Copy link
Copy Markdown
Member

Six exploratory-QA specs promoted to permanent regression anchors. Test-only, no product code.

Every one was measured twice — green on current main, and red with the commit that fixed the issue it anchors rewound. Only the delta is evidence, so both numbers are stated:

spec anchors green on main red with fix rewound
security/mcp-record-scoped-rbac #1422 11/11 5/11
server/sse-throw-midstream #1628 4/4 3/4
server/log-rotation-fd-reuse #683 3/3 2/3
database/condition-mutation-integrity #1572 17/17 12/17
database/eviction-phantom-null #1894 5/5 3/5
database/longtxn-index-orphan #1407 3/3 2/3

43/43 running together.

Why six and not thirteen

These came from 13 candidates that a citation-based gate classified as ratified invariants — each cites a closed issue, so each looked like a regression anchor. Measuring them changed the answer:

  • 7 are red on main anyway. Identical fail counts with and without the fix, so rewinding it changed nothing and the red proved nothing about detection. They fail criterion 1 (green on current main) and are not promotable in either direction.
  • 1 could not execute on main at all.

Those eight need triage — stale spec versus a genuine regression in main — and that is worth doing separately, because if any is the latter it is a defect report rather than a test problem. Listed in the branch's commit message.

This is the second time in this batch's history that measurement contradicted a plausible classification. Worth stating plainly: a citation-gated auto-merge lane would have merged all 13, including 7 tests that cannot fail and 1 that does not run.

Where to look

The two runs per spec are the claim. Nothing else here is subtle — these are QA-authored specs promoted close to verbatim, renamed and re-homed by area, with FIXTURE_PATH retargeted. If you want to spot-check one, database/condition-mutation-integrity is the most informative: 17 tests, 12 of which go red when #1572's fix is rewound.

Two fixture resources.js files needed unused-parameter fixes. They had never been linted, because the promotion gate globbed *.test.* only and never the fixtures sitting beside them — a gap in the gate, now known.

What these do and do not prove

They prove each spec detects the specific defect it anchors, on the engine and surface it exercises. They do not prove anything about the areas the dropped eight covered — allowRead enforcement across delivery surfaces, MCP tool authz, deployment-payload boundaries, env/config boot, redeploy staleness, astral-plane index bounds — that coverage is still missing and now has a measured reason for being missing.

Generated by Claude Opus 5.

@@ -0,0 +1,292 @@
/**
* QA-686 — log rotation `maxSize` unit-parsing + on-disk enforcement probe (gh#1877,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (non-blocking): this spec's own docstring, resources.js comment, and schema.graphql comment all describe it as testing maxSize unit-parsing (gh#1877) — it never inspects file descriptors or exercises the fd-reopen-after-rotation path. But the directory/file name (log-rotation-fd-reuse) and the PR body's anchors table both attribute it to #683 (the fd-leak-on-rotate bug, fixed in commit 3fb99677c). Consider renaming the fixture/file to reflect its actual target (e.g. log-rotation-maxsize-unit-parsing) and correcting the PR body's anchors column to #1877, so the table doesn't imply fd-leak regression coverage that isn't actually present here.

@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found. Diff unchanged since prior review (rebase + removal of an unrelated product-code commit only). One non-blocking suggestion still open inline (log-rotation-fd-reuse.test.ts).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a comprehensive suite of integration tests covering critical database and server behaviors, including query condition mutation integrity, TTL eviction index cleanup, multi-store transaction timeouts, MCP row-level RBAC enforcement, log rotation unit parsing, and mid-stream SSE generator errors. The review feedback provides highly actionable improvements to ensure these tests are robust and reliable. Key recommendations include replacing fixed sleeps with bounded polling loops to prevent test flakiness, wrapping test bodies and teardown hooks in try/finally blocks to guarantee process cleanup, and using unique temporary filenames incorporating the process ID to avoid conflicts on shared CI environments.

Comment thread integrationTests/security/mcp-record-scoped-rbac.test.ts
Comment thread integrationTests/security/mcp-record-scoped-rbac.test.ts Outdated
Comment thread integrationTests/server/log-rotation-fd-reuse.test.ts
Comment thread integrationTests/database/longtxn-index-orphan.test.ts Outdated
Comment thread integrationTests/server/sse-throw-midstream.test.ts Outdated
Comment thread integrationTests/server/sse-throw-midstream.test.ts Outdated
Comment thread integrationTests/server/sse-throw-midstream.test.ts
Each of these was verified twice: GREEN on current main, and RED with the commit
that fixed the issue it anchors rewound. Only the delta is evidence, so both runs
are stated per spec.

  security/mcp-record-scoped-rbac         #1422   11/11 green   5/11 red rewound
  server/sse-throw-midstream              #1628    4/4  green   3/4  red rewound
  server/log-rotation-fd-reuse            #683     3/3  green   2/3  red rewound
  database/condition-mutation-integrity   #1572   17/17 green  12/17 red rewound
  database/eviction-phantom-null          #1894    5/5  green   3/5  red rewound
  database/longtxn-index-orphan           #1407    3/3  green   2/3  red rewound

43/43 together, ~consistent with each spec's solo run.

Selected from 13 candidates that a citation-based gate called ratified invariants.
Seven were dropped because they are red on main ANYWAY — with identical fail counts
with and without the fix, so rewinding changed nothing and the red proved nothing.
One could not execute on main at all. Those eight need triage (stale spec vs a real
regression in main) and are not promotable either way: criterion 1 is green-on-main
and they fail it.

Also fixed here: two fixture resources.js files carried unused-parameter lint
errors. They were never caught because the promotion gate only ever linted
`*.test.*` and never the fixtures beside them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@kriszyp
kriszyp force-pushed the kris/qa-promote-measured-anchors branch from a172a96 to 63c28e4 Compare August 17, 2026 11:35
@kriszyp

kriszyp commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

Refreshed before flipping this out of draft, because the reason it was still open is the same thing it argues about.

It was 123 commits behind main. The green CI on it was green against that stale base — which is exactly the failure F-287 describes and the freshness assertion now added to the QA skill exists to prevent. A green on a stale main does not satisfy criterion 1, so flipping it on that evidence would have been the wrong shape of confidence for a PR whose whole claim is "measured".

Rebased onto current main (11a1c4891) and re-ran all six specs: 43/43 pass, 0 fail, 0 behind. The red-without-the-fix half is unchanged — that is a property of each spec against its own anchored fix, not of the base.

Also removed a commit that did not belong here. fix(config): extend Windows atomic-write retry budget (config/configUtils.ts) had been pushed onto this branch and existed nowhere else — no PR of its own. This PR says "test-only, no product code changes", which was no longer true. It has been relocated verbatim onto current main as #2191 so it is not lost, and dropped from here. This PR is test-only again.

Claude Opus 5

@kriszyp
kriszyp marked this pull request as ready for review August 17, 2026 11:35
kriszyp and others added 3 commits August 17, 2026 08:03
Replace fixed settling sleeps with bounded state polling, isolate the MCP diagnostic output, and guarantee fixture teardown across partial failures.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
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