Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion plugins/flow/mcp-server/dist/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -28151,7 +28151,17 @@ var ExecutionManifestSchema = external_exports.object({
// Distinct from 'verdict-failed' (which implies a quality problem) and from
// 'reviewer-no-session-result' (file-absent skipped-first-call path). Carries
// the FR45 guidance so the operator knows how to resolve the setup issue.
"review-could-not-run"
"review-could-not-run",
// Story native:01KVVJNMHDWZZS366S1BY188CG: the VERDICT step itself could not
// produce a readable result — processReviewerTranscript errored (e.g. an
// unrecognised recommendedVerdict token / non-JSON reviewer-result file →
// ReviewerResultFileMalformedError → MCP isError with no `next`) or its
// courier relay garbled (seam _parseError). A green, finished story whose
// verdict step cannot produce a result is a re-runnable SETUP problem, NOT a
// quality failure — so it is held distinctly from 'verdict-failed' (a quality
// problem) and, mirroring 'review-could-not-run' but naming the VERDICT step,
// distinctly from a review that could not RUN (AC3).
"verdict-could-not-run"
]).optional(),
/**
* Structured violation list for manifests blocked by `planning-discipline`.
Expand Down
12 changes: 11 additions & 1 deletion plugins/flow/mcp-server/dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38130,7 +38130,17 @@ var ExecutionManifestSchema = external_exports.object({
// Distinct from 'verdict-failed' (which implies a quality problem) and from
// 'reviewer-no-session-result' (file-absent skipped-first-call path). Carries
// the FR45 guidance so the operator knows how to resolve the setup issue.
"review-could-not-run"
"review-could-not-run",
// Story native:01KVVJNMHDWZZS366S1BY188CG: the VERDICT step itself could not
// produce a readable result — processReviewerTranscript errored (e.g. an
// unrecognised recommendedVerdict token / non-JSON reviewer-result file →
// ReviewerResultFileMalformedError → MCP isError with no `next`) or its
// courier relay garbled (seam _parseError). A green, finished story whose
// verdict step cannot produce a result is a re-runnable SETUP problem, NOT a
// quality failure — so it is held distinctly from 'verdict-failed' (a quality
// problem) and, mirroring 'review-could-not-run' but naming the VERDICT step,
// distinctly from a review that could not RUN (AC3).
"verdict-could-not-run"
]).optional(),
/**
* Structured violation list for manifests blocked by `planning-discipline`.
Expand Down
10 changes: 10 additions & 0 deletions plugins/flow/mcp-server/src/schemas/execution-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,16 @@ export const ExecutionManifestSchema = z
// 'reviewer-no-session-result' (file-absent skipped-first-call path). Carries
// the FR45 guidance so the operator knows how to resolve the setup issue.
"review-could-not-run",
// Story native:01KVVJNMHDWZZS366S1BY188CG: the VERDICT step itself could not
// produce a readable result — processReviewerTranscript errored (e.g. an
// unrecognised recommendedVerdict token / non-JSON reviewer-result file →
// ReviewerResultFileMalformedError → MCP isError with no `next`) or its
// courier relay garbled (seam _parseError). A green, finished story whose
// verdict step cannot produce a result is a re-runnable SETUP problem, NOT a
// quality failure — so it is held distinctly from 'verdict-failed' (a quality
// problem) and, mirroring 'review-could-not-run' but naming the VERDICT step,
// distinctly from a review that could not RUN (AC3).
"verdict-could-not-run",
])
.optional(),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,13 @@ async function runRun(opts: {
if (label.startsWith("verdict:")) {
const ref = refFromLabel(label, "verdict:");
const o = outcomes[ref]!;
if (o.kind === "verdict-blocked") return { next: "blocked-something" };
// A GENUINE reviewer block (recommendedVerdict === BLOCKED). The run
// classifies this recognised verdict to the real `reviewer-verdict-blocked`
// quality-block reason (native:01KVVJNMHDWZZS366S1BY188CG split the verdict
// catch-all). Previously this harness used a stand-in `blocked-something`;
// that token is now (correctly) reclassified as a verdict-step setup error,
// so we inject the real verdict value the production code recognises.
if (o.kind === "verdict-blocked") return { next: "done-blocked-reviewer-blocked" };
if (o.kind === "needs-changes-then-merge") {
const round = (verdictRound[ref] ??= 0);
verdictRound[ref] = round + 1;
Expand Down Expand Up @@ -360,7 +366,7 @@ describe("run concurrent dispatch (Story 8.22)", () => {
// Each blocked entry preserved its reason verbatim.
const block4 = n.blocked.find((b: any) => b.ref === "s:4");
const block5 = n.blocked.find((b: any) => b.ref === "s:5");
expect(block4.blocked_by).toBe("blocked-something");
expect(block4.blocked_by).toBe("reviewer-verdict-blocked");
expect(block5.blocked_by).toBe("dev-no-handoff");
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ interface RunOpts {
throwSeam?: { ref?: string; prefix: string };
/** Refs that should drive the needs-human-decision path, → their verbatim question. */
needsHuman?: Record<string, string>;
/**
* Override the `verdict:` seam's `next` value PER REF. Lets a case drive a
* GENUINE reviewer verdict (e.g. `done-blocked-reviewer-blocked`) through the
* real run.workflow.js verdict-classification branch, distinct from a garbled
* relay (which yields a _parseError, not a recognised `next`). Refs absent from
* the map use the default `done-ready-for-merge`.
*/
verdictNext?: Record<string, string>;
/**
* What the `claim:` seam returns once the queue is exhausted. Default
* `"queue-emptied"` (the happy terminal). Set `"waiting-on-in-progress"` to
Expand Down Expand Up @@ -205,7 +213,11 @@ async function runRun(opts: RunOpts = {}): Promise<{
reviewerPrompt: "REV-PERSONA",
};
}
if (label.startsWith("verdict:")) return { next: "done-ready-for-merge" };
if (label.startsWith("verdict:")) {
const ref = refOfLabel(label);
const overridden = ref ? (opts.verdictNext ?? {})[ref] : undefined;
return { next: overridden ?? "done-ready-for-merge" };
}
if (label.startsWith("gate:")) {
return { decision: "pause-needs-human", reason: "no-agreement-history" };
}
Expand Down Expand Up @@ -469,6 +481,91 @@ describe("run fault-injection — honest reporting under misbehaving seams (nati
assertHonestyInvariant(result, thrown, [REF_A, REF_B]);
});

// ── native:01KVVJNMHDWZZS366S1BY188CG — verdict-step error vs genuine quality block ──
//
// A green, FINISHED story whose VERDICT STEP errors or returns an unreadable /
// unrecognised result must be surfaced as a re-runnable SETUP problem
// (`verdict-could-not-run`), NEVER dumped in the blocked pile as a false
// `verdict-failed`. A GENUINE reviewer block (`done-blocked-reviewer-blocked`)
// must still be held back as a real quality decision (`reviewer-verdict-blocked`),
// exactly as today. These two pin BOTH directions in the real run.workflow.js body.

it("AC1/AC3: a verdict step that returns an UNREADABLE result on a green story routes to 'verdict-could-not-run' (a re-runnable setup problem) — never 'verdict-failed'", async () => {
// The injected garble feeds the verdict seam a non-JSON relay, so seam() yields
// a _parseError and the workflow's `v` (verdict.next) is undefined — exactly the
// shape processReviewerTranscript produces when readReviewerResultFile throws
// ReviewerResultFileMalformedError (an unrecognised recommendedVerdict token or a
// non-JSON reviewer-result file) and surfaces as an MCP isError with no `next`.
const queue = [
{ ref: REF_A, title: "Story A (green; verdict step returns unreadable result)" },
{ ref: REF_B, title: "Story B (clean sibling)" },
];
const { result, thrown } = await runRun({
queue,
maxConcurrency: 2,
garble: { ref: REF_A, prefix: "verdict:" },
});

expect(thrown).toBeUndefined();

// A is surfaced as a re-runnable SETUP problem naming the VERDICT step — NOT the
// false-failure 'verdict-failed', and distinct from 'review-could-not-run' (AC3).
const a = result.blocked.find((b: any) => b.ref === REF_A);
expect(a, "story A should be surfaced as a setup problem").toBeDefined();
expect(a.blocked_by).toBe("verdict-could-not-run");
expect(a.blocked_by).not.toBe("verdict-failed");
expect(a.blocked_by).not.toBe("review-could-not-run");
// It never reached the green-verdict completion / merge path.
expect(result.completed).not.toContain(REF_A);
expect(result.merged.some((m: any) => m.ref === REF_A)).toBe(false);

// The blockStory give-up move stamped the SAME setup reason on the manifest move
// (the block-story seam fires with that label/reason), confirming the manifest is
// pulled out of in-progress/ as a setup problem, not silently swallowed.
// (A green sibling still finishes normally below.)
expect(result.pausedForHuman.some((p: any) => p.ref === REF_A)).toBe(false);

// The run kept going and finished the OTHER story cleanly.
expect(result.completed).toContain(REF_B);
expect(result.pausedForHuman.some((p: any) => p.ref === REF_B)).toBe(true);

expect(result.runReason).toBe("queue-emptied");
assertHonestyInvariant(result, thrown, [REF_A, REF_B]);
});

it("AC2: a GENUINE reviewer BLOCK verdict still lands as the real quality decision 'reviewer-verdict-blocked' (setup-problem handling does NOT apply)", async () => {
// The verdict seam returns a recognised `done-blocked-reviewer-blocked` — a real
// quality verdict the reviewer made (recommendedVerdict === BLOCKED). This must be
// held back EXACTLY as today; the new verdict-could-not-run handling applies ONLY
// when NO real verdict could be produced.
const queue = [
{ ref: REF_A, title: "Story A (reviewer genuinely BLOCKS)" },
{ ref: REF_B, title: "Story B (clean sibling)" },
];
const { result, thrown } = await runRun({
queue,
maxConcurrency: 2,
verdictNext: { [REF_A]: "done-blocked-reviewer-blocked" },
});

expect(thrown).toBeUndefined();

// A is held back as a REAL quality block — its existing reason, NOT the new
// setup-problem reason.
const a = result.blocked.find((b: any) => b.ref === REF_A);
expect(a, "story A should be held back as a quality block").toBeDefined();
expect(a.blocked_by).toBe("reviewer-verdict-blocked");
expect(a.blocked_by).not.toBe("verdict-could-not-run");
expect(result.completed).not.toContain(REF_A);
expect(result.merged.some((m: any) => m.ref === REF_A)).toBe(false);

// The sibling still finishes normally.
expect(result.completed).toContain(REF_B);

expect(result.runReason).toBe("queue-emptied");
assertHonestyInvariant(result, thrown, [REF_A, REF_B]);
});

it("AC2: a garbled GATE relay (after a green verdict) buckets that story in BLOCKED — a story whose merge gate cannot be confirmed never reaches done/, run keeps going", async () => {
// fix/run-isolation-coordination-honesty: a garbled gate relay means the
// gate decision (incl. the CI-green check) could NOT be confirmed. Under the
Expand Down
22 changes: 20 additions & 2 deletions plugins/flow/workflows/internal/run.workflow.js
Original file line number Diff line number Diff line change
Expand Up @@ -714,8 +714,26 @@ async function processStory({ ref, title, manifestPath, resumeAtReview = false,
await blockStoryGiveUp(ref, 'review-could-not-run')
return
}
blocked.push({ ref, blocked_by: v || verdict?._parseError || 'verdict-failed' })
await blockStoryGiveUp(ref, v === 'done-blocked-reviewer-blocked' ? 'reviewer-verdict-blocked' : 'verdict-failed')
// A GENUINE quality block from the reviewer (recommendedVerdict === BLOCKED) —
// still held back as a real quality decision (AC2, unchanged behaviour).
if (v === 'done-blocked-reviewer-blocked') {
blocked.push({ ref, blocked_by: 'reviewer-verdict-blocked' })
await blockStoryGiveUp(ref, 'reviewer-verdict-blocked')
return
}
// No RECOGNISED verdict reached this point: the verdict STEP itself errored or
// returned an unreadable/unrecognised result — e.g. processReviewerTranscript
// threw ReviewerResultFileMalformedError (an unrecognised recommendedVerdict
// token or non-JSON file) and surfaced as an MCP isError response with no
// `next`, or the courier relay garbled and seam() returned a _parseError. That
// is a SETUP problem, NOT a quality verdict: a green, finished story must NEVER
// be dumped into the blocked pile as 'verdict-failed' for the operator to rescue
// by hand (the false-failure this story fixes). Route it to a distinct,
// re-runnable setup-error outcome whose reason names the VERDICT step, so it
// reads distinctly from 'review-could-not-run' (a review that could not RUN) —
// satisfying AC3's requirement to tell the two apart.
blocked.push({ ref, blocked_by: 'verdict-could-not-run' })
await blockStoryGiveUp(ref, 'verdict-could-not-run')
return
}

Expand Down
Loading