feat(framework): drive the domain preset loop as the review phase (#252)

.changeset/open-loop-domain-loop-review-phase.md

@@ -0,0 +1,19 @@
+---
+'@gemstack/framework': minor
+'@gemstack/ai-autopilot': minor
+---
+
+The domain loop drives the production-grade review phase (#252).
+
+When a run has a domain preset, its review loop now *replaces* the built-in
+checklist: each pass dispatches a `major-change` event through the preset's
+driver-backed loop, so its review chain (e.g. code review, test coverage, security
+review) fires through the wrapped agent, and Bootstrap's pass / improve / maxPasses
+machinery gates on the union of the `{ blockers }` verdicts the chain reports. A
+preset with no loop for the build event falls back to the built-in checklist, so a
+run is never left unreviewed. New: `domainLoopChecklist` + `verdictFromLoopRun`
+(@gemstack/framework).
+
+The shipped Software Development preset's review prompts (code review, test
+coverage, security review) now end with a `{ blockers }` verdict so the loop
+actually gates rather than only running.

packages/ai-autopilot/presets/software-development/prompts/code-review.md

@@ -19,3 +19,5 @@ Focus on:
 
 For each finding give one line on what is wrong and one line on the fix. Skip nits
 the linter already catches. If the change is sound, say so plainly and stop.
+
+End your reply with a fenced ```json block: `{ "blockers": ["<what must be fixed>", ...] }`. List only what must be fixed before this is production-grade; an empty array means nothing blocks.

packages/ai-autopilot/presets/software-development/prompts/security-review.md

@@ -20,3 +20,5 @@ Look for:
 
 Report each concrete risk with the file, why it is exploitable, and the fix. If the
 change introduces no new exposure, say so and stop.
+
+End your reply with a fenced ```json block: `{ "blockers": ["<what must be fixed>", ...] }`. List only what must be fixed before this is production-grade; an empty array means nothing blocks.

packages/ai-autopilot/presets/software-development/prompts/test-coverage.md

@@ -19,3 +19,5 @@ Ask:
 
 Name the specific untested paths and, for each, the one test worth adding. If the
 change is a pure refactor with existing coverage, say so and stop.
+
+End your reply with a fenced ```json block: `{ "blockers": ["<what must be fixed>", ...] }`. List only what must be fixed before this is production-grade; an empty array means nothing blocks.

packages/framework/src/run.test.ts

@@ -1,9 +1,10 @@
 import { strict as assert } from 'node:assert'
 import { test } from 'node:test'
-import { defineFrameworkExtension, definePersona, defineSkill } from '@gemstack/ai-autopilot'
+import { defineDomainPreset, defineFrameworkExtension, defineLoop, definePersona, defineSkill } from '@gemstack/ai-autopilot'
+import type { Prompt } from '@gemstack/ai-autopilot'
 import { DEFAULT_MAX_PASSES, runFramework } from './run.js'
 import { FAKE_DEPLOY, FAKE_INTENT, FAKE_SIGNALS, fakeDriver } from './fake-script.js'
-import type { Driver } from './driver/index.js'
+import { FakeDriver, type Driver } from './driver/index.js'
 import type { FrameworkEvent } from './events.js'
 
 /** A driver that records the `system` framing it is started with, delegating the run to the fake. */
@@ -215,6 +216,73 @@ test('no memory option leaves the framing unchanged (#260)', async () => {
   assert.doesNotMatch(system(), /Project memory/)
 })
 
+/** A minimal domain preset whose major-change loop runs one review prompt. */
+function reviewPreset() {
+  const review: Prompt = {
+    id: 'review',
+    name: 'review',
+    title: 'Review',
+    description: 'Review the change.',
+    instructions: 'Review the app and end with a { blockers } verdict.',
+    passes: 1,
+    appliesTo: [],
+  }
+  return defineDomainPreset({
+    name: 'test-domain',
+    title: 'Test Domain',
+    loops: [defineLoop({ on: 'major-change', run: ['review'] })],
+    prompts: [review],
+    skills: [],
+  })
+}
+
+test("a domain preset's loop drives the production-grade review phase (#252)", async () => {
+  const events: FrameworkEvent[] = []
+  const driver = new FakeDriver({
+    turns: [
+      { text: '```json\n{"stack":"X","narration":"n","decisions":[]}\n```' }, // architect
+      { text: 'Built the app.' }, // build
+      { text: 'Reviewed.\n```json\n{"blockers":[]}\n```' }, // the domain review prompt, clean
+    ],
+    sessionId: 'test',
+  })
+  const { result, loop } = await runFramework({
+    intent: FAKE_INTENT,
+    driver,
+    cwd: '/tmp/ws',
+    signals: FAKE_SIGNALS,
+    preset: reviewPreset(),
+    onEvent: e => events.push(e),
+  })
+  assert.ok(loop) // the preset loop was materialized...
+  assert.ok(events.some(e => e.kind === 'log' && /Test Domain loop drives/.test(e.message))) // ...and announced as the reviewer
+  assert.equal(result.productionGrade, true)
+  assert.equal(result.passes, 1) // cleared on the first domain-review pass (not the built-in checklist)
+})
+
+test('the domain review loop blocks, improve runs, then it clears (#252)', async () => {
+  const driver = new FakeDriver({
+    turns: [
+      { text: '```json\n{"stack":"X","narration":"n","decisions":[]}\n```' }, // architect
+      { text: 'Built the app.' }, // build
+      { text: 'Reviewed.\n```json\n{"blockers":["needs error handling"]}\n```' }, // review, pass 1: blocks
+      { text: 'Added error handling.' }, // improve
+      { text: 'Reviewed.\n```json\n{"blockers":[]}\n```' }, // review, pass 2: clean
+    ],
+    sessionId: 'test',
+  })
+  const { result } = await runFramework({
+    intent: FAKE_INTENT,
+    driver,
+    cwd: '/tmp/ws',
+    signals: FAKE_SIGNALS,
+    preset: reviewPreset(),
+    onEvent: () => {},
+  })
+  assert.equal(result.passes, 2) // blocked once, improved, cleared — the domain loop drove both passes
+  assert.equal(result.productionGrade, true)
+})
+
 test('a registered extension auto-activates by its signal, no opt-in needed (#190)', async () => {
   const { driver, system } = recordingDriver()
   const audit = defineFrameworkExtension({

packages/framework/src/run.ts

@@ -27,7 +27,7 @@ import {
 } from '@gemstack/ai-autopilot'
 import type { Driver, DriverEvent, DriverSession } from './driver/index.js'
 import { memoryFraming, type LoadedMemory } from './memory.js'
-import { decideDeploy, deployWith, driverArchitect, driverBuild, driverChecklist, driverImprove, driverLoopPrompts } from './steps.js'
+import { decideDeploy, deployWith, domainLoopChecklist, driverArchitect, driverBuild, driverChecklist, driverImprove, driverLoopPrompts } from './steps.js'
 import { hasSessionIdPlaceholder, resolveSessionLink, type FrameworkEvent } from './events.js'
 
 /**
@@ -179,8 +179,9 @@ export interface RunFrameworkResult {
   /**
    * The domain preset's review policy, materialized against this run's driver:
    * its loops plus its prompts as driver-backed passes. Present only when a
-   * {@link RunFrameworkOptions.preset} was supplied. Driving it (dispatching a
-   * `major-change` / `bug-fix` event) is left to the caller for now (#252).
+   * {@link RunFrameworkOptions.preset} was supplied. It also drives the run's
+   * production-grade review phase (#252): each checklist pass dispatches a
+   * `major-change` event through it, so its chain replaces the built-in checklist.
    */
   loop?: LoopEngine
 }
@@ -278,7 +279,7 @@ export async function runFramework(opts: RunFrameworkOptions): Promise<RunFramew
     const modeNote = opts.modes?.length ? ` (modes: ${opts.modes.join(', ')})` : ''
     emit({
       kind: 'log',
-      message: `Domain preset: ${domainPreset.title}${modeNote}; ${domainPreset.loops.length}-loop review policy available`,
+      message: `Domain preset: ${domainPreset.title}${modeNote}; ${domainPreset.loops.length}-loop review policy in effect`,
     })
   }
 
@@ -305,16 +306,40 @@ export async function runFramework(opts: RunFrameworkOptions): Promise<RunFramew
     onEvent: onDriverEvent,
   })
 
+  const ledger = new DecisionLedger()
+
+  // Materialize the domain preset's review policy against this run's driver: its
+  // loops, with its prompts as driver-backed passes sharing the run's ledger and
+  // abort signal. Exposed on the result, and driven as the review phase below (#252).
+  const loop = domainPreset
+    ? new LoopEngine({
+        loops: [...domainPreset.loops],
+        prompts: driverLoopPrompts(session, domainPreset.prompts, {
+          ledger,
+          ...(opts.signal ? { signal: opts.signal } : {}),
+        }),
+      })
+    : undefined
+
+  // The production-grade review phase. Default: the built-in checklist. With a
+  // domain preset, its loop *replaces* the checklist (#252) — each pass fires the
+  // preset's review chain through the driver — falling back to the built-in when
+  // the preset has no loop for the build event, so a run is never left unreviewed.
+  const reviewChecklist = loop
+    ? domainLoopChecklist(loop, { fallback: driverChecklist(session) })
+    : driverChecklist(session)
+  if (loop) emit({ kind: 'log', message: `Review policy: the ${domainPreset!.title} loop drives the production-grade checks` })
+
   // Boot-and-serve gate: adopt the agent's workspace so the checklist can gate
-  // on the app actually running (mergeChecklists unions the agent review with a
-  // real serveCheck). The runner adopts, never deletes, the driver's cwd.
+  // on the app actually running (mergeChecklists unions the review with a real
+  // serveCheck). The runner adopts, never deletes, the driver's cwd.
   let runner: LocalRunnerSession | undefined
   if (opts.serve) runner = await new LocalRunner().adopt(opts.cwd)
   const s = opts.serve
   const checklist =
     runner && s
       ? mergeChecklists(
-          driverChecklist(session),
+          reviewChecklist,
           serveCheck(runner, {
             serve: s.command,
             ...(s.install ? { install: s.install } : {}),
@@ -325,7 +350,7 @@ export async function runFramework(opts: RunFrameworkOptions): Promise<RunFramew
             onProgress: message => emit({ kind: 'log', message: `serve: ${message}` }),
           }),
         )
-      : driverChecklist(session)
+      : reviewChecklist
 
   // A real driver writes files to the workspace, so the build/improve steps can
   // detect an empty workspace and hard-scaffold it (#182). The fake driver writes
@@ -334,21 +359,6 @@ export async function runFramework(opts: RunFrameworkOptions): Promise<RunFramew
   const verifyWorkspace = opts.driver.name !== 'fake'
   const workspaceOpt = verifyWorkspace ? { verifyWorkspace: true } : {}
 
-  const ledger = new DecisionLedger()
-
-  // Materialize the domain preset's review policy against this run's driver: its
-  // loops, with its prompts as driver-backed passes sharing the run's ledger and
-  // abort signal. Exposed on the result; who drives it is the follow-up (#252).
-  const loop = domainPreset
-    ? new LoopEngine({
-        loops: [...domainPreset.loops],
-        prompts: driverLoopPrompts(session, domainPreset.prompts, {
-          ledger,
-          ...(opts.signal ? { signal: opts.signal } : {}),
-        }),
-      })
-    : undefined
-
   let preview: AppPreview | undefined
   try {
     const bootstrap = new Bootstrap({

packages/framework/src/steps.test.ts

@@ -3,11 +3,12 @@ import { test } from 'node:test'
 import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
-import { DecisionLedger, type DeployTarget, type SupervisorEvent } from '@gemstack/ai-autopilot'
+import { DecisionLedger, LoopEngine, defineLoop, definePrompt, type DeployTarget, type SupervisorEvent } from '@gemstack/ai-autopilot'
 import { FakeDriver } from './driver/index.js'
 import {
   architectPrompt,
   deployWith,
+  domainLoopChecklist,
   driverArchitect,
   driverBuild,
   driverChecklist,
@@ -16,6 +17,7 @@ import {
   isWorkspaceEmpty,
   MISSING_VERDICT_BLOCKER,
   parseArchitectPlan,
+  verdictFromLoopRun,
 } from './steps.js'
 
 /** Make a throwaway workspace dir, seeded with the given relative files. */
@@ -31,6 +33,47 @@ function makeWorkspace(files: Record<string, string> = {}): string {
 
 const PLAN = { stack: 'Vike + universal-orm', narration: 'orders app', decisions: [] }
 
+test('domainLoopChecklist dispatches a review event and unions the prompts blockers (#252)', async () => {
+  const loop = new LoopEngine({
+    loops: [defineLoop({ on: 'major-change', run: ['a', 'b'] })],
+    prompts: [
+      definePrompt({ id: 'a', run: async () => 'reviewed\n```json\n{"blockers":["fix X"]}\n```' }),
+      definePrompt({ id: 'b', run: async () => 'reviewed\n```json\n{"blockers":[]}\n```' }),
+    ],
+  })
+  const verdict = await domainLoopChecklist(loop)({ pass: 1, plan: PLAN, intent: 'build a thing', blockers: [] })
+  assert.deepEqual(verdict.blockers, ['fix X']) // union across the chain (b reported none)
+})
+
+test('domainLoopChecklist falls back to the built-in checklist when no loop matches the event (#252)', async () => {
+  const loop = new LoopEngine({
+    loops: [defineLoop({ on: 'bug-fix', run: ['x'] })], // no major-change loop
+    prompts: [definePrompt({ id: 'x', run: async () => '' })],
+  })
+  let fellBack = false
+  const checklist = domainLoopChecklist(loop, {
+    fallback: async () => {
+      fellBack = true
+      return { blockers: ['from built-in'] }
+    },
+  })
+  const verdict = await checklist({ pass: 1, plan: PLAN, intent: 'x', blockers: [] })
+  assert.equal(fellBack, true)
+  assert.deepEqual(verdict.blockers, ['from built-in'])
+})
+
+test('verdictFromLoopRun surfaces a review that failed to execute as a blocker', () => {
+  const blockers = verdictFromLoopRun({
+    event: { kind: 'major-change' },
+    matched: true,
+    outcomes: [
+      { promptId: 'a', passes: [], ok: false, passing: false },
+      { promptId: 'b', passes: [], ok: true, passing: true },
+    ],
+  }).blockers
+  assert.deepEqual(blockers, ['review "a" did not complete'])
+})
+
 test('parseArchitectPlan reads a fenced json plan', () => {
   const text = 'Here is the plan:\n```json\n{"stack":"Vike","narration":"n","decisions":[{"choice":"SSR","why":"data"}]}\n```'
   const plan = parseArchitectPlan(text, 'an app')

packages/framework/src/steps.ts

@@ -1,6 +1,6 @@
 import { readdirSync } from 'node:fs'
 import { join } from 'node:path'
-import { definePrompt, parseVerdict, promptInstructions, renderTask, STACK_TRADEOFFS } from '@gemstack/ai-autopilot'
+import { definePrompt, LoopEngine, parseVerdict, promptInstructions, renderTask, STACK_TRADEOFFS } from '@gemstack/ai-autopilot'
 import type {
   ArchitectAlternative,
   ArchitectContext,
@@ -11,8 +11,10 @@ import type {
   DeployContext,
   DeployOutcome,
   DeployTarget,
+  LoopEvent,
   LoopPassContext,
   LoopPrompt,
+  LoopRunResult,
   PlannedSubtask,
   Prompt,
   SubtaskResult,
@@ -281,6 +283,47 @@ export function driverChecklist(
 export const MISSING_VERDICT_BLOCKER =
   'End your reply with the required fenced ```json { "blockers": [...] } verdict; it was missing.'
 
+/**
+ * A checklist step backed by a domain preset's review loop (#252): each pass
+ * dispatches a `major-change` (by default) loop event, so the preset's review
+ * chain fires through the wrapped agent, and returns the union of the `{ blockers }`
+ * verdicts its prompts reported. This is what makes "the domain loop replaces the
+ * built-in checklist" concrete — Bootstrap keeps its pass/improve/maxPasses
+ * machinery, the domain policy just decides what "production-grade" means.
+ *
+ * A preset with no loop for the event kind is not a review at all: it falls back
+ * to `fallback` (the built-in production-grade checklist) when given, so a run is
+ * never left silently unreviewed, else nothing blocks.
+ */
+export function domainLoopChecklist(
+  loop: LoopEngine,
+  opts: { kind?: string; fallback?: (ctx: LoopPassContext) => Promise<Verdict> } = {},
+): (ctx: LoopPassContext) => Promise<Verdict> {
+  const kind = opts.kind ?? 'major-change'
+  return async ctx => {
+    const event: LoopEvent = { kind, summary: ctx.intent }
+    if (loop.matches(event).length === 0) {
+      return opts.fallback ? opts.fallback(ctx) : { blockers: [] }
+    }
+    return verdictFromLoopRun(await loop.handle(event))
+  }
+}
+
+/**
+ * Fold a loop run into one {@link Verdict}: the union of every prompt's reported
+ * blockers. A prompt that ran but reported no verdict is advisory (it does not
+ * block); a prompt that failed to execute is surfaced as a blocker so an errored
+ * review is not mistaken for a pass.
+ */
+export function verdictFromLoopRun(run: LoopRunResult): Verdict {
+  const blockers: string[] = []
+  for (const outcome of run.outcomes) {
+    if (outcome.verdict && outcome.verdict.blockers.length) blockers.push(...outcome.verdict.blockers)
+    else if (!outcome.passing) blockers.push(`review "${outcome.promptId}" did not complete`)
+  }
+  return { blockers }
+}
+
 /** The improve step: a fresh invocation that fixes the current blockers. */
 export function driverImprove(
   session: DriverSession,