From 55528d07151141b7e6c01dba3e1c0ac0819641f4 Mon Sep 17 00:00:00 2001 From: Lior Date: Thu, 28 May 2026 09:10:43 -0400 Subject: [PATCH 1/2] =?UTF-8?q?feat(workflow-engine):=20PrReviewLifecycle?= =?UTF-8?q?=20PoC=20=E2=80=94=20producing-side=20review=20work=20substrate?= =?UTF-8?q?=20companion=20to=20B-0867.20=20ReviewLifetime=20(Aaron=202026-?= =?UTF-8?q?05-28=20'does=20it=20give=20you=20time=20to=20look=20at=20prs?= =?UTF-8?q?=20and=20put=20comments');=2018=20tests=20pass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Aaron 2026-05-28 substrate-engineering substrate-engineering gap- recognition: AutoLoopLifetime (PR #5805) only models SHIP work, not REVIEW work. This DU makes producing-side review-substrate explicit. PrReviewLifecycle DU (7 variants): - observe read PR + diff + context - identify-finding substrate-engineering issue / question / praise - compose write review comment with substantive content - verify-finding grep substrate-anchor before posting (substrate-honest) - post ship via gh api / GraphQL mutation - follow-up engage on response if any - conclude no further engagement ReviewFindingKind taxonomy (8 finding shapes): - bug (critical/major/minor) - design-question - substrate-engineering-suggestion - naming-improvement - test-gap - substrate-honest-praise - documentation-gap - composes-with-substrate PrReviewFeedback DU per asymmetric-authorship: - PrNotAccessible / PeerAgentTerritory / FindingUnsubstantiated - RateLimitExhausted / NoActionableFinding isPeerAgentTerritory discriminator per fighting-past-self-vs-peer-agent: - self / unknown → false - peer-* / human-aaron → true (don't touch commits but review-allowed) Tests (18; all pass): - Universe + 7 variants - Happy-path transitions - Substantiated vs unsubstantiated verify-finding feedback - ReviewFindingKind taxonomy (8 finding shapes) - isPeerAgentTerritory discriminator - newReviewContext constructor - Type-level exhaustive Composes with: - B-0867.20 ReviewLifetime (PR #5758; receiving-side; sibling) - AutoLoopLifetime (PR #5805; will integrate when both merge) - .claude/rules/fighting-past-self-vs-peer-agent-distinguisher (don't-touch + review-allowed) - .claude/rules/asymmetric-authorship (reviewer AUTHORS feedback) - .claude/rules/honor-those-that-came-before (peer-agent work honored via substantive review) - .claude/rules/glass-halo-bidirectional (review comments are public substrate; compound) - .claude/rules/grep-substrate-anchors-before-razor-as-metaphysical (verify-finding state encodes the discipline) Co-Authored-By: Claude Opus 4.7 --- .../pr-review-lifecycle.test.ts | 176 ++++++++++++ tools/workflow-engine/pr-review-lifecycle.ts | 256 ++++++++++++++++++ 2 files changed, 432 insertions(+) create mode 100644 tools/workflow-engine/pr-review-lifecycle.test.ts create mode 100644 tools/workflow-engine/pr-review-lifecycle.ts diff --git a/tools/workflow-engine/pr-review-lifecycle.test.ts b/tools/workflow-engine/pr-review-lifecycle.test.ts new file mode 100644 index 0000000000..801a9c0439 --- /dev/null +++ b/tools/workflow-engine/pr-review-lifecycle.test.ts @@ -0,0 +1,176 @@ +// Invariant tests for PrReviewLifecycle — producing-side review work substrate. + +import { describe, expect, test } from "bun:test"; +import { + PR_REVIEW_LIFECYCLE_UNIVERSE, + dispatchPrReviewTransition, + isPeerAgentTerritory, + newReviewContext, + type PrReviewLifecycle, + type ReviewFinding, +} from "./pr-review-lifecycle.js"; + +describe("PrReviewLifecycle universe", () => { + test("7 distinct review-lifecycle states", () => { + expect(PR_REVIEW_LIFECYCLE_UNIVERSE.length).toBe(7); + const kinds = PR_REVIEW_LIFECYCLE_UNIVERSE.map((s) => s.kind); + expect(kinds).toContain("observe"); + expect(kinds).toContain("identify-finding"); + expect(kinds).toContain("verify-finding"); // grep-substrate-anchors step + expect(kinds).toContain("post"); + expect(kinds).toContain("conclude"); + }); +}); + +describe("dispatch transitions (happy path)", () => { + const baseContext = newReviewContext(5805, "self", "workflow-engine"); + + test("observe with NO findings → conclude (no-engagement)", () => { + const r = dispatchPrReviewTransition({ kind: "observe" }, baseContext); + if (r.ok) { + expect(r.outcome.nextState.kind).toBe("conclude"); + expect(r.outcome.artifact?.kind).toBe("no-engagement-warranted"); + } + }); + + test("observe WITH findings → identify-finding", () => { + const finding: ReviewFinding = { + prNumber: 5805, + kind: { kind: "substrate-honest-praise", reason: "rank-4 substrate-primitive lands cleanly" }, + content: "lgtm", + substrateAnchors: ["PR #5792 rank-4 substrate-primitive memory"], + }; + const ctx = { ...baseContext, findings: [finding] }; + const r = dispatchPrReviewTransition({ kind: "observe" }, ctx); + if (r.ok) { + expect(r.outcome.nextState.kind).toBe("identify-finding"); + } + }); + + test("identify-finding → compose", () => { + const r = dispatchPrReviewTransition({ kind: "identify-finding" }, baseContext); + if (r.ok) { + expect(r.outcome.nextState.kind).toBe("compose"); + } + }); + + test("compose → verify-finding (grep-substrate-anchors discipline)", () => { + const r = dispatchPrReviewTransition({ kind: "compose" }, baseContext); + if (r.ok) { + expect(r.outcome.nextState.kind).toBe("verify-finding"); + } + }); + + test("verify-finding with substantiated finding → post", () => { + const finding: ReviewFinding = { + prNumber: 5805, + kind: { kind: "bug", severity: "minor" }, + content: "off-by-one suspected", + substrateAnchors: ["line 50: index < len"], // anchor present + }; + const ctx = { ...baseContext, findings: [finding] }; + const r = dispatchPrReviewTransition({ kind: "verify-finding" }, ctx); + if (r.ok) { + expect(r.outcome.nextState.kind).toBe("post"); + } + }); + + test("verify-finding with UNSUBSTANTIATED finding → FindingUnsubstantiated feedback", () => { + const finding: ReviewFinding = { + prNumber: 5805, + kind: { kind: "bug", severity: "minor" }, + content: "vibes off", + substrateAnchors: [], // no anchor — per grep-substrate-anchors rule fails + }; + const ctx = { ...baseContext, findings: [finding] }; + const r = dispatchPrReviewTransition({ kind: "verify-finding" }, ctx); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.feedback.kind).toBe("FindingUnsubstantiated"); + } + }); + + test("post → follow-up with review-comment-posted artifact", () => { + const r = dispatchPrReviewTransition({ kind: "post" }, baseContext); + if (r.ok) { + expect(r.outcome.nextState.kind).toBe("follow-up"); + expect(r.outcome.artifact?.kind).toBe("review-comment-posted"); + } + }); + + test("follow-up → conclude", () => { + const r = dispatchPrReviewTransition({ kind: "follow-up" }, baseContext); + if (r.ok) { + expect(r.outcome.nextState.kind).toBe("conclude"); + } + }); + + test("conclude is terminal (stays at conclude)", () => { + const r = dispatchPrReviewTransition({ kind: "conclude" }, baseContext); + if (r.ok) { + expect(r.outcome.nextState.kind).toBe("conclude"); + } + }); +}); + +describe("ReviewFindingKind taxonomy", () => { + test("supports bug + design-question + substrate-engineering + naming + test-gap + praise + docs-gap + composes-with", () => { + const findings: ReviewFinding[] = [ + { prNumber: 1, content: "x", kind: { kind: "bug", severity: "critical" } }, + { prNumber: 1, content: "x", kind: { kind: "design-question", subject: "lifecycle DU shape?" } }, + { prNumber: 1, content: "x", kind: { kind: "substrate-engineering-suggestion", alternative: "prefer Result" } }, + { prNumber: 1, content: "x", kind: { kind: "naming-improvement", current: "foo", proposed: "bar" } }, + { prNumber: 1, content: "x", kind: { kind: "test-gap", uncovered: "edge case X" } }, + { prNumber: 1, content: "x", kind: { kind: "substrate-honest-praise", reason: "composes cleanly" } }, + { prNumber: 1, content: "x", kind: { kind: "documentation-gap", missing: "F# interop note" } }, + { prNumber: 1, content: "x", kind: { kind: "composes-with-substrate", relatedRef: "PR #5805" } }, + ]; + expect(findings.length).toBe(8); + }); +}); + +describe("isPeerAgentTerritory discriminator", () => { + test("self → false (my own work)", () => { + expect(isPeerAgentTerritory("self")).toBe(false); + }); + test("peer-otto → true", () => { + expect(isPeerAgentTerritory("peer-otto")).toBe(true); + }); + test("peer-codex / peer-lior / peer-alexa → true", () => { + expect(isPeerAgentTerritory("peer-codex")).toBe(true); + expect(isPeerAgentTerritory("peer-lior")).toBe(true); + expect(isPeerAgentTerritory("peer-alexa")).toBe(true); + }); + test("human-aaron → true (substantive engagement; don't touch substrate)", () => { + expect(isPeerAgentTerritory("human-aaron")).toBe(true); + }); + test("unknown → false (default; treat as substrate-honest mine)", () => { + expect(isPeerAgentTerritory("unknown")).toBe(false); + }); +}); + +describe("newReviewContext constructor", () => { + test("initializes with empty findings + zero observations", () => { + const ctx = newReviewContext(5805, "self", "workflow-engine"); + expect(ctx.prNumber).toBe(5805); + expect(ctx.authorLane).toBe("self"); + expect(ctx.substrateScope).toBe("workflow-engine"); + expect(ctx.findings.length).toBe(0); + expect(ctx.observationsMade).toBe(0); + }); +}); + +describe("type-level PrReviewLifecycle exhaustive switch (compile check)", () => { + test("all 7 variants distinguishable", () => { + const variants: PrReviewLifecycle[] = [ + { kind: "observe" }, + { kind: "identify-finding" }, + { kind: "compose" }, + { kind: "verify-finding" }, + { kind: "post" }, + { kind: "follow-up" }, + { kind: "conclude" }, + ]; + expect(variants.length).toBe(7); + }); +}); diff --git a/tools/workflow-engine/pr-review-lifecycle.ts b/tools/workflow-engine/pr-review-lifecycle.ts new file mode 100644 index 0000000000..393022451a --- /dev/null +++ b/tools/workflow-engine/pr-review-lifecycle.ts @@ -0,0 +1,256 @@ +// tools/workflow-engine/pr-review-lifecycle.ts +// +// PrReviewLifecycle — substrate-naming substrate for PRODUCING-side +// review work. Companion to B-0867.20 ReviewLifetime (receiving-side; +// reviewer-feedback gate-state). +// +// Per Aaron 2026-05-28: "also does it give you time to look at prs and +// put comments?" — substrate-engineering substrate-engineering substrate +// gap: AutoLoopLifetime (PR #5805) only models SHIP work, not REVIEW +// work. This DU makes producing-side review-substrate explicit. +// +// Composes with: +// - .claude/rules/fighting-past-self-vs-peer-agent-distinguisher-fix-your-own-coordinate-on-peers-dont-punt-by-default.md +// (peer-agent commits = don't touch; peer-agent reviews = substantively engage) +// - .claude/rules/asymmetric-authorship-substrate-entity-defines-consent-channel-recipient-acknowledges.md +// (reviewer AUTHORS feedback; receiving-side ACKNOWLEDGES) +// - .claude/rules/honor-those-that-came-before.md (peer-agent work honored via substantive review) +// - .claude/rules/glass-halo-bidirectional.md (review comments are public substrate; compound) +// - B-0867.20 ReviewLifetime DU (PR #5758) — receiving-side; this PR +// provides producing-side complement +// - AutoLoopLifetime (PR #5805) — will compose; producing-side review +// work becomes additional state-transition in loop substrate + +import { type LifetimeState } from "./world.js"; + +// ───────────────────────────────────────────────────────────────────── +// PrReviewLifecycle — producing-side state machine +// ───────────────────────────────────────────────────────────────────── + +/** + * PrReviewLifecycle — substrate-naming substrate for PRODUCING-side + * review work. Captures the state machine for reviewing peer PRs + + * composing + posting substantive review comments. + * + * Distinguishes from B-0867.20 ReviewLifetime which captures + * RECEIVING-side gate states (my-PR-under-review states). This DU is + * for when I AM the reviewer of someone else's work. + */ +export interface PrReviewLifecycle extends LifetimeState { + readonly kind: + | "observe" // read PR + diff + context; no engagement yet + | "identify-finding" // substrate-engineering issue / question / praise surfaced + | "compose" // write review comment with substantive content + | "verify-finding" // grep substrate-anchor; check claim before posting (per substrate-honest discipline) + | "post" // ship the review via gh api / GraphQL mutation + | "follow-up" // engage on author's response if any + | "conclude"; // no further engagement; mark as concluded +} + +// ───────────────────────────────────────────────────────────────────── +// Review-finding substrate +// ───────────────────────────────────────────────────────────────────── + +/** + * ReviewFindingKind — substrate-engineering taxonomy of review-comment + * shapes. Each kind has different operational discipline. + */ +export type ReviewFindingKind = + | { kind: "bug"; severity: "critical" | "major" | "minor" } + | { kind: "design-question"; subject: string } + | { kind: "substrate-engineering-suggestion"; alternative: string } + | { kind: "naming-improvement"; current: string; proposed: string } + | { kind: "test-gap"; uncovered: string } + | { kind: "substrate-honest-praise"; reason: string } + | { kind: "documentation-gap"; missing: string } + | { kind: "composes-with-substrate"; relatedRef: string }; + +/** + * ReviewFinding — substrate-engineering observation worth posting as + * review comment. + */ +export interface ReviewFinding { + readonly prNumber: number; + readonly filePath?: string; // optional; some findings are PR-scoped + readonly lineNumber?: number; // optional; some findings are file-scoped + readonly kind: ReviewFindingKind; + readonly content: string; // substantive review-comment body + readonly substrateAnchors?: ReadonlyArray; // grep-substrate-anchors per rule +} + +// ───────────────────────────────────────────────────────────────────── +// Review context + outcome +// ───────────────────────────────────────────────────────────────────── + +/** + * ReviewContext — substrate carried across the review-lifecycle. + */ +export interface ReviewContext { + readonly prNumber: number; + readonly authorLane: "self" | "peer-otto" | "peer-codex" | "peer-lior" | "peer-alexa" | "peer-vera" | "peer-riven" | "peer-amara" | "peer-kestrel" | "peer-prism" | "peer-mika" | "human-aaron" | "unknown"; + readonly substrateScope: "workflow-engine" | "encryption" | "zflash" | "ferry-preservation" | "rule-update" | "memory-file" | "infrastructure" | "other"; + readonly findings: ReadonlyArray; + readonly observationsMade: number; // count of observe → identify cycles +} + +/** + * ReviewOutcome — what the review-lifecycle produced. + */ +export interface ReviewOutcome { + readonly nextState: PrReviewLifecycle; + readonly artifact?: { + readonly kind: "review-comment-posted" | "thread-resolved" | "approval-given" | "no-engagement-warranted"; + readonly threadId?: string; + }; +} + +// ───────────────────────────────────────────────────────────────────── +// PrReviewFeedback — asymmetric-authorship per rule +// ───────────────────────────────────────────────────────────────────── + +export type PrReviewFeedback = + | { kind: "PrNotAccessible"; prNumber: number; reason: string } + | { kind: "PeerAgentTerritory"; prNumber: number; lane: string } // don't-touch-commits but review-allowed + | { kind: "FindingUnsubstantiated"; finding: ReviewFinding } // failed verify step + | { kind: "RateLimitExhausted"; budget: "rest" | "graphql"; resetAt: number } + | { kind: "NoActionableFinding"; prNumber: number }; + +export type PrReviewResult = + | { ok: true; outcome: T } + | { ok: false; feedback: PrReviewFeedback }; + +// ───────────────────────────────────────────────────────────────────── +// State transition dispatch +// ───────────────────────────────────────────────────────────────────── + +/** + * Dispatch the next PrReviewLifecycle state given current context. + * + * Per substrate-smoothness rule: exhaustive switch on PrReviewLifecycle + * variants; no if-statement chains. Per asymmetric-authorship: each + * transition AUTHORS feedback channel. + */ +export function dispatchPrReviewTransition( + current: PrReviewLifecycle, + context: ReviewContext, +): PrReviewResult { + switch (current.kind) { + case "observe": + // After observing, identify findings (or conclude if no findings) + if (context.findings.length === 0) { + return { + ok: true, + outcome: { + nextState: { kind: "conclude" }, + artifact: { kind: "no-engagement-warranted" }, + }, + }; + } + return { + ok: true, + outcome: { + nextState: { kind: "identify-finding" }, + }, + }; + + case "identify-finding": + // After identifying, compose the review comment + return { + ok: true, + outcome: { + nextState: { kind: "compose" }, + }, + }; + + case "compose": + // After composing, verify (grep substrate-anchors) + return { + ok: true, + outcome: { + nextState: { kind: "verify-finding" }, + }, + }; + + case "verify-finding": + // After verifying, post (or back to compose if unsubstantiated) + if (context.findings.length > 0 && context.findings[0]!.substrateAnchors !== undefined && context.findings[0]!.substrateAnchors.length === 0) { + return { + ok: false, + feedback: { kind: "FindingUnsubstantiated", finding: context.findings[0]! }, + }; + } + return { + ok: true, + outcome: { + nextState: { kind: "post" }, + }, + }; + + case "post": + // After posting, await follow-up (or conclude if no follow-up expected) + return { + ok: true, + outcome: { + nextState: { kind: "follow-up" }, + artifact: { kind: "review-comment-posted" }, + }, + }; + + case "follow-up": + // After follow-up window, conclude + return { + ok: true, + outcome: { + nextState: { kind: "conclude" }, + }, + }; + + case "conclude": + // Terminal state; stays at conclude + return { + ok: true, + outcome: { + nextState: { kind: "conclude" }, + }, + }; + } +} + +// ───────────────────────────────────────────────────────────────────── +// Reusable substrate exports +// ───────────────────────────────────────────────────────────────────── + +export const PR_REVIEW_LIFECYCLE_UNIVERSE: ReadonlyArray = [ + { kind: "observe" }, + { kind: "identify-finding" }, + { kind: "compose" }, + { kind: "verify-finding" }, + { kind: "post" }, + { kind: "follow-up" }, + { kind: "conclude" }, +]; + +/** + * Determines if a PR is in peer-agent territory (don't touch commits but + * review-allowed per fighting-past-self-vs-peer-agent-distinguisher rule). + */ +export function isPeerAgentTerritory(authorLane: ReviewContext["authorLane"]): boolean { + return authorLane.startsWith("peer-") || authorLane === "human-aaron"; +} + +/** + * Empty/initial ReviewContext for starting a new review. + */ +export function newReviewContext( + prNumber: number, + authorLane: ReviewContext["authorLane"], + substrateScope: ReviewContext["substrateScope"], +): ReviewContext { + return { + prNumber, + authorLane, + substrateScope, + findings: [], + observationsMade: 0, + }; +} From ae54b2146d9793df6ecf31bb497e332248e24461 Mon Sep 17 00:00:00 2001 From: Lior Date: Thu, 28 May 2026 09:40:49 -0400 Subject: [PATCH 2/2] fix(PR #5810): verify-finding correctness + PeerAgentTerritory lane typing + role-refs + test assertions (Copilot threads) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four substantive threads on pr-review-lifecycle: A. (P1 line 10) Persona attribution: "Per Aaron 2026-05-28" → "Per the human maintainer (2026-05-28)". Also the `authorLane` DU value `"human-aaron"` → `"human-operator"` to remove first-name from the in-code-value substrate (role-ref naming for the lane). B. (P1 line 113) `PrReviewFeedback.PeerAgentTerritory.lane: string` lost type-safety + can drift from `authorLane` universe. Changed to `lane: ReviewContext["authorLane"]` so feedback + context stay in lockstep when new lanes are added. C. (P1 line 187) `verify-finding` transition had three correctness bugs: 1. Only validated `findings[0]`, ignoring additional findings. 2. Treated missing `substrateAnchors` as "substantiated" (only failed when array was present-but-empty). Per grep-substrate-anchors-before-razor-as-metaphysical rule: substrate-anchors must be PRESENT AND NON-EMPTY for a finding to be substantiated; missing == unsubstantiated. 3. Advanced to `post` even when `findings.length === 0` — would post an empty review. Now returns `NoActionableFinding` feedback in the zero-findings case. Rewrote case to iterate ALL findings with `findings.find((f) => f.substrateAnchors === undefined || f.substrateAnchors.length === 0)`; first unsubstantiated finding surfaces in `FindingUnsubstantiated` feedback; zero-findings surfaces in `NoActionableFinding`. D. (P1 test line 34) 8 sites used `if (r.ok)` narrowing without explicit `expect(r.ok).toBe(true)` — would silently pass on ok:false. Bulk-added the assertion via perl substitution. Plus: dropped `.js` extension on `./world.js` import per repo convention (`./world` extensionless; matches other tools/workflow-engine imports). Tests: 18 pass (unchanged count; existing tests adapted to new authorLane value via the `"human-aaron"` → `"human-operator"` substitution which the test file inherits). Autonomous-loop tick 2026-05-28T14:18Z resolution of PR #5810 BLOCKED gate (4 unresolved Copilot threads + 1 required-check flake which clears after #5817 mise fix merged). Co-Authored-By: Claude --- .../pr-review-lifecycle.test.ts | 10 +++++- tools/workflow-engine/pr-review-lifecycle.ts | 33 ++++++++++++++----- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/tools/workflow-engine/pr-review-lifecycle.test.ts b/tools/workflow-engine/pr-review-lifecycle.test.ts index 801a9c0439..8d785edad2 100644 --- a/tools/workflow-engine/pr-review-lifecycle.test.ts +++ b/tools/workflow-engine/pr-review-lifecycle.test.ts @@ -27,6 +27,7 @@ describe("dispatch transitions (happy path)", () => { test("observe with NO findings → conclude (no-engagement)", () => { const r = dispatchPrReviewTransition({ kind: "observe" }, baseContext); + expect(r.ok).toBe(true); if (r.ok) { expect(r.outcome.nextState.kind).toBe("conclude"); expect(r.outcome.artifact?.kind).toBe("no-engagement-warranted"); @@ -42,6 +43,7 @@ describe("dispatch transitions (happy path)", () => { }; const ctx = { ...baseContext, findings: [finding] }; const r = dispatchPrReviewTransition({ kind: "observe" }, ctx); + expect(r.ok).toBe(true); if (r.ok) { expect(r.outcome.nextState.kind).toBe("identify-finding"); } @@ -49,6 +51,7 @@ describe("dispatch transitions (happy path)", () => { test("identify-finding → compose", () => { const r = dispatchPrReviewTransition({ kind: "identify-finding" }, baseContext); + expect(r.ok).toBe(true); if (r.ok) { expect(r.outcome.nextState.kind).toBe("compose"); } @@ -56,6 +59,7 @@ describe("dispatch transitions (happy path)", () => { test("compose → verify-finding (grep-substrate-anchors discipline)", () => { const r = dispatchPrReviewTransition({ kind: "compose" }, baseContext); + expect(r.ok).toBe(true); if (r.ok) { expect(r.outcome.nextState.kind).toBe("verify-finding"); } @@ -70,6 +74,7 @@ describe("dispatch transitions (happy path)", () => { }; const ctx = { ...baseContext, findings: [finding] }; const r = dispatchPrReviewTransition({ kind: "verify-finding" }, ctx); + expect(r.ok).toBe(true); if (r.ok) { expect(r.outcome.nextState.kind).toBe("post"); } @@ -92,6 +97,7 @@ describe("dispatch transitions (happy path)", () => { test("post → follow-up with review-comment-posted artifact", () => { const r = dispatchPrReviewTransition({ kind: "post" }, baseContext); + expect(r.ok).toBe(true); if (r.ok) { expect(r.outcome.nextState.kind).toBe("follow-up"); expect(r.outcome.artifact?.kind).toBe("review-comment-posted"); @@ -100,6 +106,7 @@ describe("dispatch transitions (happy path)", () => { test("follow-up → conclude", () => { const r = dispatchPrReviewTransition({ kind: "follow-up" }, baseContext); + expect(r.ok).toBe(true); if (r.ok) { expect(r.outcome.nextState.kind).toBe("conclude"); } @@ -107,6 +114,7 @@ describe("dispatch transitions (happy path)", () => { test("conclude is terminal (stays at conclude)", () => { const r = dispatchPrReviewTransition({ kind: "conclude" }, baseContext); + expect(r.ok).toBe(true); if (r.ok) { expect(r.outcome.nextState.kind).toBe("conclude"); } @@ -142,7 +150,7 @@ describe("isPeerAgentTerritory discriminator", () => { expect(isPeerAgentTerritory("peer-alexa")).toBe(true); }); test("human-aaron → true (substantive engagement; don't touch substrate)", () => { - expect(isPeerAgentTerritory("human-aaron")).toBe(true); + expect(isPeerAgentTerritory("human-operator")).toBe(true); }); test("unknown → false (default; treat as substrate-honest mine)", () => { expect(isPeerAgentTerritory("unknown")).toBe(false); diff --git a/tools/workflow-engine/pr-review-lifecycle.ts b/tools/workflow-engine/pr-review-lifecycle.ts index 393022451a..c0bfc418a2 100644 --- a/tools/workflow-engine/pr-review-lifecycle.ts +++ b/tools/workflow-engine/pr-review-lifecycle.ts @@ -4,7 +4,7 @@ // review work. Companion to B-0867.20 ReviewLifetime (receiving-side; // reviewer-feedback gate-state). // -// Per Aaron 2026-05-28: "also does it give you time to look at prs and +// Per the human maintainer (2026-05-28): "also does it give you time to look at prs and // put comments?" — substrate-engineering substrate-engineering substrate // gap: AutoLoopLifetime (PR #5805) only models SHIP work, not REVIEW // work. This DU makes producing-side review-substrate explicit. @@ -21,7 +21,7 @@ // - AutoLoopLifetime (PR #5805) — will compose; producing-side review // work becomes additional state-transition in loop substrate -import { type LifetimeState } from "./world.js"; +import { type LifetimeState } from "./world"; // ───────────────────────────────────────────────────────────────────── // PrReviewLifecycle — producing-side state machine @@ -87,7 +87,7 @@ export interface ReviewFinding { */ export interface ReviewContext { readonly prNumber: number; - readonly authorLane: "self" | "peer-otto" | "peer-codex" | "peer-lior" | "peer-alexa" | "peer-vera" | "peer-riven" | "peer-amara" | "peer-kestrel" | "peer-prism" | "peer-mika" | "human-aaron" | "unknown"; + readonly authorLane: "self" | "peer-otto" | "peer-codex" | "peer-lior" | "peer-alexa" | "peer-vera" | "peer-riven" | "peer-amara" | "peer-kestrel" | "peer-prism" | "peer-mika" | "human-operator" | "unknown"; readonly substrateScope: "workflow-engine" | "encryption" | "zflash" | "ferry-preservation" | "rule-update" | "memory-file" | "infrastructure" | "other"; readonly findings: ReadonlyArray; readonly observationsMade: number; // count of observe → identify cycles @@ -110,7 +110,7 @@ export interface ReviewOutcome { export type PrReviewFeedback = | { kind: "PrNotAccessible"; prNumber: number; reason: string } - | { kind: "PeerAgentTerritory"; prNumber: number; lane: string } // don't-touch-commits but review-allowed + | { kind: "PeerAgentTerritory"; prNumber: number; lane: ReviewContext["authorLane"] } // don't-touch-commits but review-allowed | { kind: "FindingUnsubstantiated"; finding: ReviewFinding } // failed verify step | { kind: "RateLimitExhausted"; budget: "rest" | "graphql"; resetAt: number } | { kind: "NoActionableFinding"; prNumber: number }; @@ -171,12 +171,26 @@ export function dispatchPrReviewTransition( }, }; - case "verify-finding": - // After verifying, post (or back to compose if unsubstantiated) - if (context.findings.length > 0 && context.findings[0]!.substrateAnchors !== undefined && context.findings[0]!.substrateAnchors.length === 0) { + case "verify-finding": { + // Per grep-substrate-anchors-before-razor-as-metaphysical: every + // finding MUST carry substrate-anchors before advancing to post. + // Both missing `substrateAnchors` AND an empty array mean + // unsubstantiated. Iterate all findings (not just findings[0]). + // Zero findings = NoActionableFinding feedback (can't post review + // when there's nothing to say). + if (context.findings.length === 0) { + return { + ok: false, + feedback: { kind: "NoActionableFinding", prNumber: context.prNumber }, + }; + } + const unsubstantiated = context.findings.find((f) => + f.substrateAnchors === undefined || f.substrateAnchors.length === 0 + ); + if (unsubstantiated !== undefined) { return { ok: false, - feedback: { kind: "FindingUnsubstantiated", finding: context.findings[0]! }, + feedback: { kind: "FindingUnsubstantiated", finding: unsubstantiated }, }; } return { @@ -185,6 +199,7 @@ export function dispatchPrReviewTransition( nextState: { kind: "post" }, }, }; + } case "post": // After posting, await follow-up (or conclude if no follow-up expected) @@ -235,7 +250,7 @@ export const PR_REVIEW_LIFECYCLE_UNIVERSE: ReadonlyArray = [ * review-allowed per fighting-past-self-vs-peer-agent-distinguisher rule). */ export function isPeerAgentTerritory(authorLane: ReviewContext["authorLane"]): boolean { - return authorLane.startsWith("peer-") || authorLane === "human-aaron"; + return authorLane.startsWith("peer-") || authorLane === "human-operator"; } /**