diff --git a/.changeset/open-loop-domain-loop-review-phase.md b/.changeset/open-loop-domain-loop-review-phase.md new file mode 100644 index 0000000..f93f540 --- /dev/null +++ b/.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. diff --git a/packages/ai-autopilot/presets/software-development/prompts/code-review.md b/packages/ai-autopilot/presets/software-development/prompts/code-review.md index 52238e7..73594e8 100644 --- a/packages/ai-autopilot/presets/software-development/prompts/code-review.md +++ b/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": ["", ...] }`. List only what must be fixed before this is production-grade; an empty array means nothing blocks. diff --git a/packages/ai-autopilot/presets/software-development/prompts/security-review.md b/packages/ai-autopilot/presets/software-development/prompts/security-review.md index 68ba3eb..09c75c9 100644 --- a/packages/ai-autopilot/presets/software-development/prompts/security-review.md +++ b/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": ["", ...] }`. List only what must be fixed before this is production-grade; an empty array means nothing blocks. diff --git a/packages/ai-autopilot/presets/software-development/prompts/test-coverage.md b/packages/ai-autopilot/presets/software-development/prompts/test-coverage.md index 225c1de..6f67f60 100644 --- a/packages/ai-autopilot/presets/software-development/prompts/test-coverage.md +++ b/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": ["", ...] }`. List only what must be fixed before this is production-grade; an empty array means nothing blocks. diff --git a/packages/framework/src/run.test.ts b/packages/framework/src/run.test.ts index f7670e7..6ae8256 100644 --- a/packages/framework/src/run.test.ts +++ b/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({ diff --git a/packages/framework/src/run.ts b/packages/framework/src/run.ts index f6f8a3c..6bb38d0 100644 --- a/packages/framework/src/run.ts +++ b/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 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 = {}): 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') diff --git a/packages/framework/src/steps.ts b/packages/framework/src/steps.ts index ab4253e..5326371 100644 --- a/packages/framework/src/steps.ts +++ b/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 } = {}, +): (ctx: LoopPassContext) => Promise { + 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,