Skip to content

Commit 5569517

Browse files
rafaelgilnRafaelclaude
authored
chore(issue-pipeline): void infra aborts, require a pre-fix flake rate, gate the quarantine lift (#1082) (#1085)
Seven gaps surfaced while working #1060 with the prose orchestrator; the deterministic pipeline could not represent any of them. The environment can now answer for the spec and be recognised as such: classifyRun splits a run into clean / infra-void / real-failure from its failure messages, so a wedged backend dropping /api/v1/auto_login is re-run instead of counted. A failure the classifier cannot read stays real — the classification can never silence a genuine red. DEBUG gained the evidence it was missing: a pre-fix repro rate for flake issues (three clean VALIDATE runs do not distinguish a fix from luck at single-digit flake rates), one verdict per symptom row (an issue's table can list failures with different causes, and one may belong to another issue), and an artifacts command that reads the failing run's own JSON blob. VALIDATE now gates the quarantine lift, and the PR gate checks branch purity against origin/main plus a CI verdict that makes an ambient-red merge carry a written justification. Also: FORCE_FAIL requires red runs only for runnable titles — a test.fixme never executes, so demanding one deadlocked the phase. Co-authored-by: Rafael <rafael@oriontech.me> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ce3ccb0 commit 5569517

12 files changed

Lines changed: 1097 additions & 41 deletions

File tree

.claude/skills/langflow-e2e-issue-deterministic/DESIGN.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ Type variations (same 6 classes as the prose skill):
7070
npx tsx pipeline/cli.ts next 493 # validate state, run mechanical work, emit exact next instruction
7171
npx tsx pipeline/cli.ts complete 493 <step> --evidence <args>
7272
npx tsx pipeline/cli.ts escalate 493 debug --reason "…" # park current phase, enter DEBUG from anywhere
73+
npx tsx pipeline/cli.ts artifacts 493 --run <run-id> [--filter "<title>"] # per-attempt status/error from the failing run's own JSON artifact
74+
npx tsx pipeline/cli.ts repro-run 493 --spec <path> [--grep "<title>"] [--runs 10] # DEBUG only: PRE-fix flake rate on the unmodified spec
7375
npx tsx pipeline/cli.ts status 493
7476
npx tsx pipeline/cli.ts abort 493 --reason "…"
7577
npx tsx pipeline/cli.ts metrics 493 # benchmark summary (JSON)
@@ -150,6 +152,29 @@ prose skill / `langflow-e2e` references instead.
150152
`Closes #NNN` and the correct template; `roadmap` label present for wave issues.
151153
Post-merge instruction: verify the issue actually closed (the edited-`Fixes`
152154
GitHub quirk) and delete the branch.
155+
7. **Environment aborts are void, not verdicts** (#1082): `classifyRun` splits a
156+
run into `clean | infra-void | real-failure` from its failure messages. A run
157+
whose every failure carries an infra signature (`/api/v1/auto_login` timeout,
158+
`socket hang up`, connection refused) is re-run and counted as neither; past
159+
`PIPELINE_MAX_INFRA_VOIDS` (3) the phase stops naming the instance. A failure
160+
the classifier cannot read stays a real failure — it can never silence a red.
161+
8. **Pre-fix flake rate** (#1082): for a flake-shaped issue, DEBUG completes only
162+
with a `repro-run` baseline (≥5 runs on the unmodified spec) — or, when the
163+
defect never reproduced, an explicit `evidence.mechanismProof`. Three clean
164+
VALIDATE runs do not distinguish a fix from luck at single-digit flake rates.
165+
9. **One verdict per symptom row** (#1082): every `spec.ts:line` row in the
166+
issue's table needs its own verdict; a row owned by another issue carries
167+
`ownedBy:"#NNNN"` and must be referenced in the PR body.
168+
10. **Quarantine lift** (#1082): when the issue quarantined a test, VALIDATE
169+
fails while a `test.fixme` survives in a touched spec, and (when the issue
170+
asks for it) while a quarantined title lacks `@stable`. Correspondingly,
171+
FORCE_FAIL requires red runs only for *runnable* titles — a muted test
172+
cannot be force-failed, and demanding it would deadlock the phase.
173+
11. **Branch purity + CI verdict** (#1082): the PR gate diffs
174+
`origin/main..HEAD` against the files SPECIFY/IMPLEMENT recorded (plus
175+
`QA-CHECKLIST.md`), failing closed when the base ref cannot be resolved; and
176+
it requires `ciVerdict: green | ambient-red`, where `ambient-red` must carry
177+
the URL of a justification comment that actually exists on the PR.
153178

154179
## State file
155180

.claude/skills/langflow-e2e-issue-deterministic/SKILL.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,19 @@ Other commands, only when the situation calls for them:
4242
```bash
4343
$PIPE escalate <NNN> debug --reason "" # behavior looks broken, any phase
4444
$PIPE ff-run <NNN> --file <spec> --test "<title>" --mutation "<desc>"
45+
$PIPE artifacts <NNN> --run <workflow-run-id> [--filter "<test title>"]
46+
$PIPE repro-run <NNN> --spec <path> [--grep "<title>"] [--runs 10] # DEBUG only
4547
$PIPE status <NNN>
4648
$PIPE metrics <NNN> # benchmark data
4749
$PIPE abort <NNN> --reason ""
4850
$PIPE authorize-pr <NNN> --quote "<user's exact words>" # ONLY after explicit user authorization
4951
```
5052

53+
`artifacts` downloads the failing run's `playwright-json-daily-<run>` blob and
54+
prints each attempt's status, duration and error — the first thing to read on a
55+
daily-failure issue, before any theory. `repro-run` measures the **pre-fix**
56+
rate of a flake on the unmodified spec (it refuses a dirty spec file).
57+
5158
## Hard rules
5259

5360
- **Tests exist to catch real Langflow regressions, never to go green**: the
@@ -69,8 +76,37 @@ $PIPE authorize-pr <NNN> --quote "<user's exact words>" # ONLY after explicit
6976
slow spec for zero added signal (a real session ran a spec ~10× this way).
7077
Scout runs to design/debug are fine; redundant confirmation bursts are not.
7178
Need more/fewer runs? Set `PIPELINE_BURST`, don't loop manually.
79+
- **An environment abort is not a spec result.** A run whose every failure
80+
carries an infra signature (`/api/v1/auto_login` timeout, `socket hang up`,
81+
connection refused — the wedged-backend class measured in #1074) is recorded
82+
**void** and re-run; it counts neither as a failure nor as one of the clean
83+
burst runs. Never "fix" a spec against one, and never report it as a defect.
84+
Repeated voids stop the phase naming the **instance** as the blocker: restart
85+
it, don't loosen the test. `PIPELINE_MAX_INFRA_VOIDS` (default 3) tunes the cap.
86+
- **A flake issue needs its PRE-fix rate.** Run `repro-run` on the unmodified
87+
spec BEFORE changing anything; DEBUG will not complete without it. VALIDATE's
88+
clean burst is not evidence on its own — at an 8 %-per-run flake, three green
89+
runs is the expected outcome of doing nothing (#1060). If the baseline never
90+
reproduces, prove the mechanism another way and say how in
91+
`evidence.mechanismProof`.
92+
- **One verdict per symptom row.** A dedicated issue's table can list failures
93+
with different causes; #1060's second row was another issue's `auto_login`
94+
timeout, not the defect being fixed. Each row gets its own verdict, and a row
95+
another issue owns carries `ownedBy:"#NNNN"` and must be named in the PR body.
96+
- **Lifting a quarantine is a deliverable, and it is gated.** If the issue
97+
quarantined a test, VALIDATE fails while a `test.fixme` survives in a touched
98+
spec — and, when the issue asks for the tag back, while a quarantined title
99+
lacks `@stable`.
72100
- **Never bypass the CLI**: no `gh pr create`, no commit/push, no phase
73101
skipping, no editing files under `.claude/issue-pipeline/`.
102+
- **The branch carries this issue's files and nothing else.** The PR gate diffs
103+
against `origin/main`; a rebase onto a local `main` that a parallel session
104+
already committed to drags their work into your PR (#1060 — caught by hand).
105+
Fix it with `git rebase --onto origin/main <their-commit> <your-branch>`.
106+
- **A red CI check is fixed or justified in writing, never ignored.** The PR
107+
gate takes `ciVerdict: green | ambient-red`; `ambient-red` requires the URL of
108+
a PR comment naming the cause, the evidence that it is ambient, and why
109+
merging is still the right call.
74110
- **Never fabricate evidence**: `userConfirmed: true` only after the user
75111
actually confirmed in chat; `--quote` only with words the user actually said.
76112
Runner-written keys (runs/typecheck/lint/nightly/qaDiff/ff/finalGreen) are
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { test } from 'node:test'
2+
import assert from 'node:assert/strict'
3+
import { summarizeRunArtifact, firstLines } from './artifacts.ts'
4+
5+
// Shape of a real playwright-json-daily-<run> artifact, trimmed to what the
6+
// summarizer reads: three provider projects of the same spec line, one flaky
7+
// (passed then failed on a serial-group retry) and one hard failure.
8+
const ARTIFACT = JSON.stringify({
9+
suites: [{
10+
title: 'agent-context-id-isolation.spec.ts',
11+
suites: [
12+
{
13+
title: 'Agent Context ID Isolation [openai / gpt-4o-mini]',
14+
specs: [{
15+
title: "switching the agent's context_id re-tags new turns",
16+
file: 'llm-agents/agent-context-id-isolation.spec.ts',
17+
line: 570,
18+
tests: [{
19+
projectName: 'chromium',
20+
status: 'flaky',
21+
results: [
22+
{ retry: 0, status: 'passed', duration: 25000 },
23+
{ retry: 2, status: 'failed', duration: 56000, error: { message: 'Error: expect(received).toBe(expected)\n\nExpected: "turns-tagged"\nReceived: "turn-2 message(s) with wrong context_id"' } },
24+
],
25+
}],
26+
}],
27+
},
28+
{
29+
title: 'Agent Context ID Isolation [google / gemini-2.5-flash]',
30+
specs: [{
31+
title: "switching the agent's context_id re-tags new turns",
32+
file: 'llm-agents/agent-context-id-isolation.spec.ts',
33+
line: 570,
34+
tests: [{
35+
projectName: 'chromium',
36+
status: 'unexpected',
37+
results: [{ retry: 0, status: 'failed', duration: 58000, error: { message: 'TimeoutError: apiRequestContext.get: Timeout 20000ms exceeded.\nCall log:\n - → GET http://localhost:7860/api/v1/auto_login' } }],
38+
}],
39+
}],
40+
},
41+
{
42+
title: 'some other spec',
43+
specs: [{
44+
title: 'an unrelated test',
45+
file: 'other.spec.ts',
46+
line: 10,
47+
tests: [{ projectName: 'chromium', status: 'expected', results: [{ retry: 0, status: 'passed', duration: 1000 }] }],
48+
}],
49+
},
50+
],
51+
}],
52+
})
53+
54+
test('summarizeRunArtifact reports every attempt of every matching test', () => {
55+
const lines = summarizeRunArtifact(ARTIFACT, 'context_id')
56+
const text = lines.join('\n')
57+
assert.equal(lines.filter(l => l.startsWith('llm-agents/')).length, 2)
58+
assert.match(text, /\[chromium\].* flaky/)
59+
assert.match(text, /attempt 0: passed \(25s\)/)
60+
assert.match(text, /attempt 2: failed \(56s\)/)
61+
assert.match(text, /wrong context_id/)
62+
// The two rows had different causes — the summary must show both verbatim.
63+
assert.match(text, /auto_login/)
64+
assert.ok(!text.includes('an unrelated test'))
65+
})
66+
67+
test('summarizeRunArtifact without a filter covers the whole run', () => {
68+
const text = summarizeRunArtifact(ARTIFACT).join('\n')
69+
assert.match(text, /an unrelated test/)
70+
})
71+
72+
test('summarizeRunArtifact says so when nothing matches or the blob is broken', () => {
73+
assert.match(summarizeRunArtifact(ARTIFACT, 'nope')[0], /no test matching/)
74+
assert.match(summarizeRunArtifact('not json')[0], /not valid JSON/)
75+
})
76+
77+
test('firstLines strips ANSI colouring and caps the error head', () => {
78+
const msg = 'Error: boom\n\nline two\nline three\nline four'
79+
const out = firstLines(msg)
80+
assert.ok(!out.includes('['))
81+
assert.equal(out.split(' ⏎ ').length, 3)
82+
})
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/**
2+
* Reading a daily run's own artifact is the highest-signal step of a
3+
* daily-failure investigation: the `playwright-json-daily-<run>` blob carries
4+
* every attempt's status, duration and error, which is what separates a real
5+
* defect from an environment abort (#1060: the `unknown` signature on one row
6+
* turned out to be #1030's auto_login timeout, and the flaky row's error named
7+
* the stale context_id outright).
8+
*
9+
* The summarizer is pure so the shape of a run report can be unit-tested; the
10+
* download lives in runners.ts.
11+
*/
12+
13+
interface AttemptLike {
14+
retry?: number
15+
status?: string
16+
duration?: number
17+
error?: { message?: string }
18+
}
19+
20+
interface TestLike {
21+
projectName?: string
22+
status?: string
23+
results?: AttemptLike[]
24+
}
25+
26+
interface SpecLike {
27+
title?: string
28+
file?: string
29+
line?: number
30+
tests?: TestLike[]
31+
specs?: SpecLike[]
32+
suites?: SpecLike[]
33+
}
34+
35+
export function firstLines(message: string, max = 3): string {
36+
return message
37+
// eslint-disable-next-line no-control-regex
38+
.replace(/\[[0-9;]*m/g, '')
39+
.split('\n')
40+
.filter(l => l.trim() !== '')
41+
.slice(0, max)
42+
.join(' ⏎ ')
43+
}
44+
45+
function walk(node: SpecLike | undefined, out: SpecLike[]): void {
46+
if (!node) return
47+
for (const s of node.specs ?? []) {
48+
if (s.tests) out.push(s)
49+
walk(s, out)
50+
}
51+
for (const s of node.suites ?? []) walk(s, out)
52+
}
53+
54+
/**
55+
* One block per matching test: its file:line, then one line per attempt with
56+
* status, duration and the head of the error. `filter` is a case-insensitive
57+
* substring matched against the test title — pass the failing title from the
58+
* issue to cut the report down to the rows under investigation.
59+
*/
60+
export function summarizeRunArtifact(raw: string, filter?: string): string[] {
61+
let data: { suites?: SpecLike[] }
62+
try { data = JSON.parse(raw) } catch { return ['✖ artifact is not valid JSON'] }
63+
const specs: SpecLike[] = []
64+
for (const s of data.suites ?? []) walk(s, specs)
65+
66+
const needle = filter?.toLowerCase()
67+
const matched = specs.filter(s => !needle || (s.title ?? '').toLowerCase().includes(needle))
68+
if (matched.length === 0) {
69+
return [filter ? `no test matching "${filter}" in the artifact` : 'artifact contains no tests']
70+
}
71+
72+
const lines: string[] = []
73+
for (const spec of matched) {
74+
for (const t of spec.tests ?? []) {
75+
lines.push(`${spec.file ?? '?'}:${spec.line ?? '?'} [${t.projectName ?? 'default'}] ${spec.title ?? '?'}${t.status ?? '?'}`)
76+
for (const r of t.results ?? []) {
77+
const secs = Math.round((r.duration ?? 0) / 1000)
78+
lines.push(` attempt ${r.retry ?? 0}: ${r.status ?? '?'} (${secs}s)`)
79+
if (r.error?.message) lines.push(` ${firstLines(r.error.message)}`)
80+
}
81+
}
82+
}
83+
return lines
84+
}

.claude/skills/langflow-e2e-issue-deterministic/pipeline/cli.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { test } from 'node:test'
22
import assert from 'node:assert/strict'
3-
import { parseArgs, sanitizeEvidence, releaseCycleOf } from './cli.ts'
3+
import { parseArgs, sanitizeEvidence, releaseCycleOf, allowedBranchFiles } from './cli.ts'
44

55
test('parseArgs extracts command, issue, and flags', () => {
66
const a = parseArgs(['next', '493', '--spec', 'tests/x.spec.ts', '--evidence-json', '{"a":1}'])
@@ -24,3 +24,26 @@ test('releaseCycleOf extracts major.minor', () => {
2424
assert.equal(releaseCycleOf('1.11.2.dev3'), '1.11')
2525
assert.equal(releaseCycleOf('1.5.1'), '1.5')
2626
})
27+
28+
test('sanitizeEvidence also strips the new runner-written keys', () => {
29+
const out = sanitizeEvidence({
30+
reproRate: { runs: 99 }, artifactRuns: ['1'], verdict: 'test-defect',
31+
})
32+
assert.deepEqual(out, { verdict: 'test-defect' })
33+
})
34+
35+
test('allowedBranchFiles unions what SPECIFY and IMPLEMENT recorded', () => {
36+
const state = {
37+
steps: {
38+
SPECIFY: { evidence: { specDoc: 'docs/a.md', existingSpec: 'tests/a.spec.ts' } },
39+
IMPLEMENT: { evidence: { files: ['tests/a.spec.ts', 'tests/helpers/h.ts'] } },
40+
},
41+
} as never
42+
assert.deepEqual(allowedBranchFiles(state), [
43+
'docs/a.md', 'tests/a.spec.ts', 'tests/helpers/h.ts', 'QA-CHECKLIST.md',
44+
])
45+
})
46+
47+
test('allowedBranchFiles still allows the checklist bullet on a bare state', () => {
48+
assert.deepEqual(allowedBranchFiles({ steps: {} } as never), ['QA-CHECKLIST.md'])
49+
})

0 commit comments

Comments
 (0)