diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d930a36f..1badc6d94 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,11 @@ on: pull_request: branches: [main] workflow_dispatch: + inputs: + skill_workflow: + description: 'Run billed, browser-backed Claude workflow completion tests' + type: boolean + default: false # Nightly full live-e2e matrix. The smoke groups already gate every PR; the # full sweep is too slow for that, so it runs once a day against main. schedule: @@ -665,5 +670,44 @@ jobs: - name: Install dependencies run: bun install + - name: Prepare engine for protocol tests + run: bun run fetch:engine + - name: Run skill behavior tests run: bun run test:skill-behavior + + skill-workflow: + runs-on: ubuntu-latest + if: github.event_name == 'workflow_dispatch' && inputs.skill_workflow + timeout-minutes: 70 + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + IMPECCABLE_SKILL_BEHAVIOR_MODELS: claude-sonnet-5 + IMPECCABLE_SKILL_BEHAVIOR_TRACE_DIR: ${{ runner.temp }}/skill-workflow-traces + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Setup Node + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 24 + - name: Setup Bun + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: latest + - name: Install dependencies + run: bun install --frozen-lockfile + - name: Prepare engine and browser before billing + run: | + test -n "$ANTHROPIC_API_KEY" || { echo 'ANTHROPIC_API_KEY is required'; exit 1; } + bun run fetch:engine + bunx playwright install --with-deps chromium + - name: Run completed skill workflows + run: bun run test:skill-workflow + - name: Retain diagnostic traces + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: skill-workflow-traces + path: ${{ runner.temp }}/skill-workflow-traces + retention-days: 7 diff --git a/package.json b/package.json index fe86c503b..68a6c7527 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,7 @@ "test:new-work-e2e": "node scripts/run-tests.mjs new-work-e2e", "test:live-e2e-agent": "node scripts/run-tests.mjs live-e2e-agent", "test:skill-behavior": "node scripts/run-tests.mjs skill-behavior", + "test:skill-workflow": "node scripts/run-tests.mjs skill-workflow", "test:live-svelte-adapter-deepseek": "node scripts/run-tests.mjs live-svelte-adapter-deepseek", "smoke:hooks": "node scripts/smoke-provider-hooks.mjs", "audit": "bun audit --audit-level=moderate", diff --git a/scripts/test-suites.mjs b/scripts/test-suites.mjs index 21a3790de..58936d202 100644 --- a/scripts/test-suites.mjs +++ b/scripts/test-suites.mjs @@ -8,6 +8,7 @@ export const OPT_IN_SUITES = [ 'live-e2e-accept-cleanup', 'new-work-e2e', 'skill-behavior', + 'skill-workflow', 'live-svelte-adapter-deepseek', ]; @@ -267,38 +268,40 @@ export const SUITES = { ], }, 'skill-behavior': { - description: 'LLM-backed skill setup behavior scenarios.', + description: 'LLM-backed protocol checkpoints, not full builds.', optIn: true, triggers: [ ...COMMON_INFRA_PATTERNS, /^skill\/SKILL\.src\.md$/, - /^skill\/reference\/(init|document|brand|product|shape|craft|audit|polish|live|routing)\.md$/, + /^skill\/reference\//, /^ENGINE_VERSION$/, /^tests\/skill-behavior\//, ], + commands: [{ + runner: 'node', + timeoutMs: 240000, + wallClockMs: 1_800_000, + files: ['tests/skill-behavior/scenarios.test.mjs'], + }], + }, + 'skill-workflow': { + description: 'Explicitly opt-in completed workflows with a preflighted browser.', + optIn: true, + needsPlaywright: true, + triggers: [ + ...COMMON_INFRA_PATTERNS, + /^skill\//, + /^ENGINE_VERSION$/, + /^tests\/skill-workflow\//, + /^tests\/skill-behavior\//, + ], commands: [ + { runner: 'node', files: ['tests/skill-workflow-browser.test.mjs'] }, { runner: 'node', - // 300000 was too low to measure what these scenarios assert. The - // workflow-contract turns run 20+ steps against a frontier model, and - // the *correct* path is the slow one: a run that stops to put the - // concept to the user before building was measured at 579s, while the - // runs that skipped that checkpoint and failed the assertion finished - // in 130-200s. At a 300s cap the thorough path is killed and the hasty - // path is graded, so the cap was selecting for the behavior the suite - // exists to forbid. timeoutMs: 900000, - // Overall wall-clock safety cap for the whole sweep: if a provider - // call wedges past every inner guard (the harness's 840s per-turn - // AbortSignal and the 900s per-test timeout), the runner SIGKILLs the - // process group so the sweep still ends with a per-provider tally - // instead of hanging overnight. Sized well above a healthy two-provider - // sweep; override with IMPECCABLE_TEST_WALL_CLOCK_MS to scope it down. wallClockMs: 3_600_000, - files: [ - 'tests/skill-behavior/scenarios.test.mjs', - 'tests/skill-behavior/workflow-contract.test.mjs', - ], + files: ['tests/skill-workflow/full-build.test.mjs'], }, ], }, diff --git a/tests/ci-test-plan.test.mjs b/tests/ci-test-plan.test.mjs index d87c53aa1..5e1154349 100644 --- a/tests/ci-test-plan.test.mjs +++ b/tests/ci-test-plan.test.mjs @@ -8,6 +8,16 @@ import { tmpdir } from 'node:os'; const SCRIPT = 'scripts/ci-test-plan.mjs'; describe('ci-test-plan', () => { + it('requires explicit manual opt-in and preprovisions the full workflow job', () => { + const workflow = readFileSync('.github/workflows/ci.yml', 'utf8'); + assert.match(workflow, /skill_workflow:\s*description:[^\n]+\s*type: boolean\s*default: false/); + const job = workflow.split('\n skill-workflow:')[1]; + assert.match(job, /if: github.event_name == 'workflow_dispatch' && inputs.skill_workflow/); + assert.ok(job.indexOf('bun run fetch:engine') < job.indexOf('bun run test:skill-workflow')); + assert.ok(job.indexOf('playwright install --with-deps chromium') < job.indexOf('bun run test:skill-workflow')); + const protocol = workflow.split('\n skill-behavior:')[1].split('\n skill-workflow:')[0]; + assert.match(protocol, /bun run fetch:engine/); + }); it('keeps docs-only pull requests on the core suite', () => { const outputs = runPlan({ GITHUB_EVENT_NAME: 'pull_request', diff --git a/tests/skill-behavior-harness.test.mjs b/tests/skill-behavior-harness.test.mjs index 074b1d43d..797317e34 100644 --- a/tests/skill-behavior-harness.test.mjs +++ b/tests/skill-behavior-harness.test.mjs @@ -6,6 +6,30 @@ import { MockLanguageModelV3 } from 'ai/test'; import { prepareWorkspace, cleanupWorkspace, makeTools, runTurn, fileLoaded, SKILL_BODY } from './skill-behavior/harness.mjs'; import { assertPlanningFallbackWarning, assertNewWorkLifecycle } from './skill-behavior/assertions.mjs'; import { CASE_STUDY_ANSWER } from './skill-behavior/fixtures.mjs'; +import { sourceHash as hashSources } from './skill-workflow/source-hash.mjs'; +import { assertCompleted, assertFreshCaptures } from './skill-workflow/assertions.mjs'; + +it('full workflows reject exhausted budgets and stale or absent visual evidence', () => { + for (const outcome of ['checkpoint', 'step-budget', 'output-limit', 'error']) { + assert.throws(() => assertCompleted({ outcome, steps: 50 }), /did not finish/); + } + assert.doesNotThrow(() => assertCompleted({ outcome: 'complete', steps: 12 })); + const workspace = prepareWorkspace({ files: { 'index.html': '

Test

' } }); + try { + const sourceHash = hashSources(workspace); + const edit = { mutatedPaths: ['index.html'] }; + const shots = ['desktop', 'mobile'].map((viewport) => ({ capture: { target: 'index.html', viewport, sourceHash } })); + const check = (toolCalls) => assertFreshCaptures({ toolCalls }, workspace, 'index.html'); + assert.doesNotThrow(() => check([edit, ...shots])); + assert.throws(() => check([edit]), /missing desktop screenshot/); + assert.throws(() => check([...shots, edit]), /missing desktop screenshot/); + assert.throws(() => check([edit, shots[0]]), /missing mobile screenshot/); + fs.writeFileSync(path.join(workspace, 'style.css'), 'h1 { color: red; }'); + assert.throws(() => check([edit, ...shots]), /missing desktop screenshot/); + } finally { + cleanupWorkspace(workspace); + } +}); it('case-study user supplies evidence now instead of promising a future message', async () => { const workspace = prepareWorkspace(); @@ -142,6 +166,24 @@ it('DeepSeek gets an explicit output ceiling instead of the compatibility SDK de } }); +it('protocol checkpoints stop at successful evidence without claiming task completion', async () => { + const workspace = prepareWorkspace(); + try { + const model = new MockLanguageModelV3({ modelId: 'claude-sonnet-5', doGenerate: { + content: [{ type: 'tool-call', toolCallId: 'load', toolName: 'read', input: JSON.stringify({ path: '.claude/skills/impeccable/reference/polish.md' }) }], + finishReason: { unified: 'tool-calls', raw: 'tool-calls' }, + usage: { inputTokens: { total: 1 }, outputTokens: { total: 1 } }, warnings: [], + } }); + const result = await runTurn({ workspace, model, userPrompt: 'Route only.', maxSteps: 10, + stopAfter: (trace) => fileLoaded(trace, 'polish.md') }); + assert.equal(result.outcome, 'checkpoint'); + assert.equal(result.steps, 1); + assert.equal(model.doGenerateCalls.length, 1); + const exhausted = await runTurn({ workspace, model, userPrompt: 'Complete work.', maxSteps: 1 }); + assert.equal(exhausted.outcome, 'step-budget'); + } finally { cleanupWorkspace(workspace); } +}); + it('optional diagnostics retain tool evidence when a provider turn fails', async () => { const workspace = prepareWorkspace({ files: { 'PRODUCT.md': 'Synthetic product context.' } }); const previous = process.env.IMPECCABLE_SKILL_BEHAVIOR_TRACE_DIR; @@ -244,6 +286,9 @@ it('successful-loader controls accept a workspace-relative target', { skip: !pro try { const { tools } = makeTools(workspace, {}, {}, { contextOnlyBash: true }); assert.match(await tools.bash.execute({ command: '.claude/skills/impeccable/scripts/impeccable context --target index.html' }), /^exit=0\n/); + for (const target of ['src/routes/+page.svelte', '"src/routes/+page.svelte"']) { + assert.match(await tools.bash.execute({ command: `.claude/skills/impeccable/scripts/impeccable context --target ${target}` }), /^exit=0\n/); + } assert.match(await tools.bash.execute({ command: '.claude/skills/impeccable/scripts/impeccable context --target ../outside.html' }), /^Error:/); } finally { cleanupWorkspace(workspace); diff --git a/tests/skill-behavior/README.md b/tests/skill-behavior/README.md index 86a9681a9..4716561b6 100644 --- a/tests/skill-behavior/README.md +++ b/tests/skill-behavior/README.md @@ -37,6 +37,42 @@ IMPECCABLE_SKILL_BEHAVIOR_MODELS=claude-sonnet-5 IMPECCABLE_SKILL_BEHAVIOR_VERBO ## How it works +### Protocol versus full completion + +`test:skill-behavior` now runs only `scenarios.test.mjs`. Routing cases stop +at the successful reference/context checkpoint they assert, with a ten-step +ceiling; shell access is context-only. They do **not** claim that a page was +built or reviewed. Editing/fallback controls retain their original assertions. +The focused S1/S2/S3/S4/S19 rerun passed 21/21 across the three default models. +A broader run exposed the context-only allowlist rejecting Svelte's valid +`+page.svelte` target. It was stopped, the allowlist fixed with a failing-then- +passing unit test, and S8 passed 3/3 on the focused rerun. Failed file reads +do not count as project exploration. + +Full workflows moved to `tests/skill-workflow/full-build.test.mjs`: + +```bash +bun run fetch:engine +bunx playwright install chromium +bun run test:skill-workflow +``` + +This separately billed suite defaults to Claude only; use +`IMPECCABLE_SKILL_BEHAVIOR_MODELS` to explicitly choose another model or sweep. +It preflights a local server and Chromium before each provider turn, exposing +real desktop/mobile screenshot and PNG viewing tools. Text-only fixtures use +system fonts and block external browser requests. No extra skill prose is added. +The API harness is not the actual Claude Code host, nor is its shell sandboxed. + +Each workflow has a 50-step/840-second ceiling. Reaching a budget or output +limit fails explicitly; routing checkpoints cannot satisfy completion. UI +workflows require desktop and mobile captures matching the final local sources after +its last edit. Approval/brief-before-code and redesign documentation-at-finish +checks remain, as does exactly one context load across the completed turn. +CI runs this lane only when its manual `skill_workflow` checkbox is enabled. +Ordinary protocol CI now fetches its engine instead of silently skipping for +a missing binary. Full-build results must be reported separately from routing. + Each scenario: 1. `prepareWorkspace()` uses the production transformer to build current source @@ -275,7 +311,7 @@ evidence, not rejected bash reads. The initial unrestricted run (stopped after host-wide search attempts), earlier rejected-read results, and broader suite's sandboxed provider DNS errors are excluded from this baseline. -The workflow-contract file adds end-to-end assertions for attended fresh init, +The full-build file adds end-to-end assertions for attended fresh init, an initialized natural build request, replacement-world redesign, scope-preserving bolder refinement, and critique's closing question. It checks question order and context/artifact writes rather than only reference-file loading. @@ -452,7 +488,7 @@ when bisecting one scenario: ```bash IMPECCABLE_QUESTION_DISABLED=1 CI=1 IMPECCABLE_SKILL_BEHAVIOR_MODELS=deepseek-v4-flash \ node --test --test-timeout=300000 --test-force-exit \ - --test-name-pattern="bolder refinement" tests/skill-behavior/workflow-contract.test.mjs + --test-name-pattern="bolder refinement" tests/skill-workflow/full-build.test.mjs ``` Use the suite's current 900000ms timeout for full workflow cases; the 300000ms diff --git a/tests/skill-behavior/harness.mjs b/tests/skill-behavior/harness.mjs index b725315ca..fd9b8a275 100644 --- a/tests/skill-behavior/harness.mjs +++ b/tests/skill-behavior/harness.mjs @@ -154,7 +154,7 @@ function safeResolve(root, userPath) { } function isContextOnlyCommand(workspace, command) { - const match = command.trim().match(/^\.claude\/skills\/impeccable\/scripts\/impeccable context(?: --target (?:"([a-zA-Z0-9_./ -]+)"|'([a-zA-Z0-9_./ -]+)'|([a-zA-Z0-9_./-]+)))?$/); + const match = command.trim().match(/^\.claude\/skills\/impeccable\/scripts\/impeccable context(?: --target (?:"([a-zA-Z0-9_./+ -]+)"|'([a-zA-Z0-9_./+ -]+)'|([a-zA-Z0-9_./+-]+)))?$/); if (!match) return false; const target = match[1] ?? match[2] ?? match[3]; return target === undefined || (!target.startsWith('-') && typeof safeResolve(workspace, target) === 'string'); @@ -400,8 +400,9 @@ export function makeTools(workspace, extraEnv = {}, simulatedUser = {}, { contex // run is never killed. The timer is unref'd (it must not keep the loop alive // after a healthy turn) and cleared on completion. const TURN_TIMEOUT_MS = Number(process.env.IMPECCABLE_SKILL_BEHAVIOR_TURN_TIMEOUT_MS) || 840_000; -export async function runTurn({ workspace, model, userPrompt, priorMessages = [], maxSteps = 8, env = {}, simulatedUser = {}, timeoutMs = TURN_TIMEOUT_MS, contextOnlyBash = false, denyBash = false }) { +export async function runTurn({ workspace, model, userPrompt, priorMessages = [], maxSteps = 8, env = {}, simulatedUser = {}, timeoutMs = TURN_TIMEOUT_MS, contextOnlyBash = false, denyBash = false, stopAfter, additionalTools, environment = '' }) { const { tools, trace } = makeTools(workspace, env, simulatedUser, { contextOnlyBash, denyBash }); + if (additionalTools) Object.assign(tools, additionalTools(trace)); const messages = [ ...priorMessages, { role: 'user', content: userPrompt }, @@ -423,11 +424,11 @@ export async function runTurn({ workspace, model, userPrompt, priorMessages = [] try { result = await generateText({ model, - system: SKILL_BODY, + system: environment ? `${SKILL_BODY}\n\nRuntime environment: ${environment}` : SKILL_BODY, messages, tools, onStepFinish: tracePath ? (step) => saveTrace({ status: 'in-progress', lastStepMessages: step.response.messages }) : undefined, - stopWhen: [stepCountIs(maxSteps)], + stopWhen: [stepCountIs(maxSteps), ...(stopAfter ? [() => stopAfter(trace)] : [])], // Real client-side deadline on the provider call: without it a stalled // stream wedges the whole sweep with no tally. abortSignal: controller.signal, @@ -449,14 +450,20 @@ export async function runTurn({ workspace, model, userPrompt, priorMessages = [] } const generatedResponseMessages = result.responseMessages ?? result.response?.messages ?? []; const responseMessages = [...messages, ...generatedResponseMessages]; + const outcome = stopAfter?.(trace) ? 'checkpoint' + : result.finishReason === 'length' ? 'output-limit' + : result.finishReason === 'tool-calls' && result.steps.length >= maxSteps ? 'step-budget' + : result.finishReason === 'stop' ? 'complete' : result.finishReason; saveTrace({ status: 'completed', responseMessages, - finishReason: result.finishReason, steps: result.steps.length, usage: result.usage }); + outcome, finishReason: result.finishReason, steps: result.steps.length, usage: result.totalUsage ?? result.usage }); return { trace, + outcome, + steps: result.steps.length, text: result.text ?? '', stepTexts: result.steps.map((step) => step.text ?? ''), finishReason: result.finishReason, - usage: result.usage, + usage: result.totalUsage ?? result.usage, responseMessages, }; } diff --git a/tests/skill-behavior/scenarios.test.mjs b/tests/skill-behavior/scenarios.test.mjs index 9ae7888fc..63948ee36 100644 --- a/tests/skill-behavior/scenarios.test.mjs +++ b/tests/skill-behavior/scenarios.test.mjs @@ -18,7 +18,7 @@ import path from 'node:path'; import { prepareWorkspace, cleanupWorkspace, - runTurn, + runTurn as runHarnessTurn, bashCommandsMatching, readsMatching, fileLoaded, @@ -40,7 +40,19 @@ import { SVELTE_PROJECT_FILES, } from './fixtures.mjs'; +// Protocol-only shell access; successful checkpoints end observation, not the task. +async function runTurn({ checkpoint, ...options }) { + return runHarnessTurn({ contextOnlyBash: true, timeoutMs: 180000, ...options, + stopAfter: typeof checkpoint === 'function' ? checkpoint + : checkpoint ? (trace) => fileLoaded(trace, checkpoint) : undefined }); +} + const CRAFT_PROMPT = '/impeccable craft a landing page for the project in this workspace'; +function projectCodeReads(trace) { + return trace.toolCalls.filter((call) => call.name === 'read' && call.succeeded + && /\.(css|svelte|tsx?|jsx?|astro)$/i.test(call.input.path) + && !call.input.path.includes('.claude/skills/')).map((call) => call.input.path); +} const SHAPE_PROMPT = '/impeccable shape a landing page for the project in this workspace'; const NATURAL_BUILD_PROMPT = 'Build a landing page for the project in this workspace.'; const TEACH_PROMPT = '/impeccable teach'; @@ -101,16 +113,14 @@ for (const modelId of resolveModelList()) { return; } const model = getModel(modelId); - // Claude and Gemini may inspect the workspace before loading references. - // Three steps truncated valid Claude setup; six-step diagnostics reached - // the same required references. Keep the budget bounded, not a requirement - // that every provider batches its tool calls like OpenAI. - const setupMaxSteps = provider === 'openai' ? 3 : 6; + // Observe a routing decision, with room for setup reads but no full build. + const setupMaxSteps = 10; it('scenario 1: no PRODUCT.md / DESIGN.md', async () => { const workspace = prepareWorkspace({ files: {} }); try { const { trace, text } = await runTurn({ + checkpoint: 'init.md', workspace, model, userPrompt: CRAFT_PROMPT, @@ -144,6 +154,7 @@ for (const modelId of resolveModelList()) { }); try { const { trace, text } = await runTurn({ + checkpoint: 'new-work.md', workspace, model, userPrompt: CRAFT_PROMPT, @@ -172,6 +183,7 @@ for (const modelId of resolveModelList()) { }); try { const { trace, text } = await runTurn({ + checkpoint: 'new-work.md', workspace, model, userPrompt: CRAFT_PROMPT, @@ -216,6 +228,7 @@ for (const modelId of resolveModelList()) { // Turn 1: prime the conversation so impeccable context gets run and its // output enters the message history. const turn1 = await runTurn({ + checkpoint: (trace) => trace.bashOutputs.some((output) => output.startsWith('exit=0\n')), workspace, model, userPrompt: PRIMER_PROMPT, @@ -231,6 +244,7 @@ for (const modelId of resolveModelList()) { // Turn 2: the real ask. The skill says "skip if you've already // loaded it". Verify the agent honors that. const turn2 = await runTurn({ + checkpoint: 'new-work.md', workspace, model, userPrompt: 'Now, /impeccable craft a landing page based on what you saw.', @@ -256,6 +270,7 @@ for (const modelId of resolveModelList()) { }); try { const { trace, text } = await runTurn({ + checkpoint: 'new-work.md', workspace, model, userPrompt: CRAFT_PROMPT, @@ -287,6 +302,7 @@ for (const modelId of resolveModelList()) { }); try { const { trace, text } = await runTurn({ + checkpoint: 'polish.md', workspace, model, userPrompt: '/impeccable polish index.html', @@ -313,6 +329,7 @@ for (const modelId of resolveModelList()) { }); try { const { trace, text } = await runTurn({ + checkpoint: 'audit.md', workspace, model, userPrompt: '/impeccable audit index.html', @@ -339,6 +356,7 @@ for (const modelId of resolveModelList()) { }); try { const { trace, text } = await runTurn({ + checkpoint: (trace) => projectCodeReads(trace).length > 0, workspace, model, userPrompt: '/impeccable polish src/routes/+page.svelte', @@ -349,9 +367,7 @@ for (const modelId of resolveModelList()) { // agent should read at least one project code file (CSS / tokens / // component / page), not just the skill's PRODUCT.md / DESIGN.md // / reference files. - const projectReads = trace.readPaths.filter((p) => - /\.(css|svelte|tsx?|jsx?|astro)$/i.test(p) && !p.includes('.claude/skills/'), - ); + const projectReads = projectCodeReads(trace); assert.ok( projectReads.length >= 1, `agent should read at least one project code file to understand the existing design system.\n` + @@ -426,6 +442,7 @@ for (const modelId of resolveModelList()) { }); try { const { trace, text } = await runTurn({ + checkpoint: 'polish.md', workspace, model, userPrompt: '/impeccable polish index.html', @@ -464,6 +481,7 @@ for (const modelId of resolveModelList()) { const workspace = prepareWorkspace({ files: {} }); try { const { trace, text } = await runTurn({ + checkpoint: 'init.md', workspace, model, userPrompt: SHAPE_PROMPT, @@ -489,6 +507,7 @@ for (const modelId of resolveModelList()) { const workspace = prepareWorkspace({ files: {} }); try { const { trace, text } = await runTurn({ + checkpoint: 'init.md', workspace, model, userPrompt: NATURAL_BUILD_PROMPT, @@ -516,6 +535,7 @@ for (const modelId of resolveModelList()) { const workspace = prepareWorkspace({ files: {} }); try { const { trace, text } = await runTurn({ + checkpoint: 'init.md', workspace, model, userPrompt: TEACH_PROMPT, @@ -549,6 +569,7 @@ for (const modelId of resolveModelList()) { }); try { const { trace, text } = await runTurn({ + checkpoint: 'ios.md', workspace, model, userPrompt: '/impeccable craft a tide detail screen for the project in this workspace', @@ -584,6 +605,7 @@ for (const modelId of resolveModelList()) { }); try { const { trace, text } = await runTurn({ + checkpoint: 'audit.native.md', workspace, model, userPrompt: '/impeccable audit the app in this workspace', diff --git a/tests/skill-workflow-browser.test.mjs b/tests/skill-workflow-browser.test.mjs new file mode 100644 index 000000000..20fdc5001 --- /dev/null +++ b/tests/skill-workflow-browser.test.mjs @@ -0,0 +1,50 @@ +import { it, mock } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { prepareBrowser, imageOutput } from './skill-workflow/browser.mjs'; +import { chromium } from 'playwright'; + +it('fails browser preflight with an actionable error before starting a workflow', async () => { + const launch = mock.method(chromium, 'launch', async () => { throw new Error('browser missing'); }); + try { + await assert.rejects(prepareBrowser('/unused'), /playwright install chromium/); + } finally { + launch.mock.restore(); + } +}); + +it('prepares real browser captures, interactions, and multimodal image results offline', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-workflow-browser-')); + let browser; + try { + fs.writeFileSync(path.join(root, 'index.html'), 'Fixture'); + browser = await prepareBrowser(root); + const trace = { toolCalls: [] }; + const tools = browser.tools(trace); + const capture = await tools.browser_snapshot.execute({ path: './index.html', viewport: 'desktop', click: 'button' }); + assert.equal(capture.target, 'index.html'); + assert.match(capture.text, /Done/); + assert.ok(fs.existsSync(path.join(root, capture.screenshot))); + assert.equal(capture.viewport, 'desktop'); + assert.equal(trace.toolCalls[0].name, 'browser_snapshot'); + const output = imageOutput({ output: capture }); + assert.equal(output.type, 'content'); + assert.ok(output.value.some((part) => part.mediaType === 'image/png')); + const viewed = await tools.view_image.execute({ path: capture.screenshot }); + assert.equal(viewed.image, capture.image); + const captures = await Promise.all(['desktop', 'mobile'].map((viewport) => tools.browser_snapshot.execute({ path: 'index.html', viewport }))); + assert.deepEqual(captures.map((result) => result.viewport), ['desktop', 'mobile']); + assert.notEqual(captures[0].image, captures[1].image, 'parallel viewports must not share mutable page state'); + assert.ok(browser.blockedRequests.some((url) => url.includes('example.invalid'))); + await assert.rejects(tools.browser_snapshot.execute({ path: '../outside.html', viewport: 'mobile' }), /workspace/); + await assert.rejects(tools.view_image.execute({ path: 'index.html' }), /PNG/); + fs.symlinkSync(os.tmpdir(), path.join(root, 'escape')); + const response = await fetch(`${browser.origin}/escape/`); + assert.equal(response.status, 403); + } finally { + await browser?.close(); + fs.rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/tests/skill-workflow/assertions.mjs b/tests/skill-workflow/assertions.mjs new file mode 100644 index 000000000..541c15190 --- /dev/null +++ b/tests/skill-workflow/assertions.mjs @@ -0,0 +1,17 @@ +import assert from 'node:assert/strict'; +import { sourceHash } from './source-hash.mjs'; + +export function assertCompleted(result) { + assert.equal(result.outcome, 'complete', `workflow did not finish: ${result.outcome} after ${result.steps} steps`); +} + +export function assertFreshCaptures(trace, workspace, target) { + const calls = trace.toolCalls; + const lastEdit = calls.findLastIndex((call) => (call.mutatedPaths || []).includes(target)); + const hash = sourceHash(workspace); + for (const viewport of ['desktop', 'mobile']) { + assert.ok(calls.some((call, index) => index > lastEdit && call.capture?.target === target + && call.capture.viewport === viewport && call.capture.sourceHash === hash), + `missing ${viewport} screenshot of the final ${target}; pre-edit captures do not count`); + } +} diff --git a/tests/skill-workflow/browser.mjs b/tests/skill-workflow/browser.mjs new file mode 100644 index 000000000..08644fc95 --- /dev/null +++ b/tests/skill-workflow/browser.mjs @@ -0,0 +1,131 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import http from 'node:http'; +import { sourceHash as hashSources } from './source-hash.mjs'; +import { chromium } from 'playwright'; +import { tool } from 'ai'; +import { z } from 'zod'; + +const VIEWPORTS = { desktop: { width: 1440, height: 1000 }, mobile: { width: 390, height: 844 } }; +const TYPES = { '.html': 'text/html', '.css': 'text/css', '.js': 'text/javascript', '.mjs': 'text/javascript', '.svg': 'image/svg+xml', '.png': 'image/png', '.woff2': 'font/woff2' }; +const PNG = Buffer.from('89504e470d0a1a0a', 'hex'); + +function resolveFile(root, name) { + if (path.isAbsolute(name)) throw new Error('Use a workspace-relative path'); + const file = path.resolve(root, name); + const rel = path.relative(root, file); + if (rel === '..' || rel.startsWith(`..${path.sep}`)) throw new Error('Path escapes workspace'); + const real = fs.realpathSync(file); + const realRel = path.relative(fs.realpathSync(root), real); + if (realRel === '..' || realRel.startsWith(`..${path.sep}`)) throw new Error('Path escapes workspace through a symlink'); + return real; +} + +export function imageOutput({ output }) { + const { image, ...metadata } = output; + return { type: 'content', value: [ + { type: 'text', text: JSON.stringify(metadata) }, + { type: 'file', mediaType: 'image/png', data: { type: 'data', data: image } }, + ] }; +} + +/** Preflight before any billed call; no runtime installs or browser discovery. */ +export async function prepareBrowser(root) { + let browser; + try { + browser = await chromium.launch({ headless: true, timeout: 15000 }); + } catch (error) { + throw new Error('Workflow browser preflight failed. Run `bunx playwright install chromium` before billed tests.', { cause: error }); + } + const blockedRequests = []; + const server = http.createServer((req, res) => { + try { + const name = decodeURIComponent(new URL(req.url, 'http://localhost').pathname).replace(/^\//, '') || 'index.html'; + if (name.split('/').some((part) => part.startsWith('.'))) throw new Error('Private workspace path'); + const file = resolveFile(root, name); + if (!fs.statSync(file).isFile()) throw new Error('Not a file'); + res.writeHead(200, { 'Content-Type': TYPES[path.extname(file)] || 'application/octet-stream', 'Cache-Control': 'no-store' }); + res.end(fs.readFileSync(file)); + } catch (error) { + res.writeHead(error.code === 'ENOENT' ? 404 : 403); + res.end('Not available'); + } + }); + try { + await new Promise((resolve, reject) => { server.once('error', reject); server.listen(0, '127.0.0.1', resolve); }); + } catch (error) { + await browser.close(); + throw error; + } + const origin = `http://127.0.0.1:${server.address().port}`; + let context; + try { + context = await browser.newContext({ reducedMotion: 'reduce', serviceWorkers: 'block' }); + await context.route('**/*', (route) => { + const url = route.request().url(); + if (new URL(url).origin === origin || url.startsWith('data:')) return route.continue(); + blockedRequests.push(url); + return route.abort(); + }); + } catch (error) { + await browser.close(); + await new Promise((resolve) => { server.close(resolve); server.closeAllConnections(); }); + throw error; + } + return { + origin, blockedRequests, + environment: `Workspace: ${root}. A local server and Chromium are already running. browser_snapshot renders a workspace-relative HTML path at desktop/mobile size, saves a screenshot, and returns the actual image plus DOM text. view_image opens saved PNGs. No browser installation is needed. External browser requests are blocked; this text-only fixture uses system fonts. No image-generation or subagent tools are available.`, + tools(trace) { + return { + browser_snapshot: tool({ + description: 'Render and inspect an HTML file with the ready Chromium browser. Returns an actual screenshot and visible DOM text; optionally click a CSS selector before capture. Captures save to .impeccable/review/{desktop|mobile}.png.', + inputSchema: z.object({ path: z.string(), viewport: z.enum(['desktop', 'mobile']), click: z.string().optional() }), + execute: async ({ path: target, viewport, click }) => { + const file = resolveFile(root, target); + if (!/\.html?$/i.test(file)) throw new Error('Expected an HTML artifact'); + const relativeTarget = path.relative(fs.realpathSync(root), file).split(path.sep).join('/'); + const call = { name: 'browser_snapshot', input: { path: target, viewport, click }, mutatedPaths: [] }; + trace.toolCalls.push(call); + const page = await context.newPage(); + page.setDefaultTimeout(10000); + try { + const sourceHash = hashSources(root); + await page.setViewportSize(VIEWPORTS[viewport]); + await page.goto(`${origin}/${relativeTarget.split('/').map(encodeURIComponent).join('/')}`, { waitUntil: 'load', timeout: 15000 }); + await page.evaluate(() => document.fonts.ready); + if (click) await page.locator(click).click(); + const screenshot = `.impeccable/review/${viewport}.png`; + if (fs.existsSync(path.join(root, '.impeccable'))) resolveFile(root, '.impeccable'); + fs.mkdirSync(path.join(root, '.impeccable/review'), { recursive: true }); + resolveFile(root, '.impeccable/review'); + if (fs.existsSync(path.join(root, screenshot))) resolveFile(root, screenshot); + const image = await page.screenshot({ path: path.join(root, screenshot), fullPage: true, animations: 'disabled' }); + call.mutatedPaths = [screenshot]; + if (sourceHash !== hashSources(root)) throw new Error('Artifact changed during capture; retry'); + call.capture = { target: relativeTarget, viewport, screenshot, sourceHash }; + return { ...call.capture, text: (await page.locator('body').innerText()).slice(0, 12000), image: image.toString('base64') }; + } finally { + await page.close(); + } + }, + toModelOutput: imageOutput, + }), + view_image: tool({ + description: 'Inspect an existing workspace PNG as an actual image, not raw file bytes.', + inputSchema: z.object({ path: z.string() }), + execute: async ({ path: name }) => { + const bytes = fs.readFileSync(resolveFile(root, name)); + if (!bytes.subarray(0, 8).equals(PNG)) throw new Error('Expected a PNG image'); + trace.toolCalls.push({ name: 'view_image', input: { path: name }, mutatedPaths: [], loadedImages: [name] }); + return { path: name, image: bytes.toString('base64') }; + }, + toModelOutput: imageOutput, + }), + }; + }, + async close() { + await browser.close(); + await new Promise((resolve) => { server.close(resolve); server.closeAllConnections(); }); + }, + }; +} diff --git a/tests/skill-behavior/workflow-contract.test.mjs b/tests/skill-workflow/full-build.test.mjs similarity index 82% rename from tests/skill-behavior/workflow-contract.test.mjs rename to tests/skill-workflow/full-build.test.mjs index 27fd0e99c..1c2d1282b 100644 --- a/tests/skill-behavior/workflow-contract.test.mjs +++ b/tests/skill-workflow/full-build.test.mjs @@ -10,15 +10,39 @@ import path from 'node:path'; import { prepareWorkspace, cleanupWorkspace, - runTurn, + runTurn as runHarnessTurn, fileLoaded, summarizeTrace, ENGINE_BIN, ENGINE_MISSING_MESSAGE, -} from './harness.mjs'; -import { detectProvider, getModel, hasKey, resolveModelList, PROVIDERS } from './providers.mjs'; -import { assertNewWorkLifecycle } from './assertions.mjs'; -import { PRODUCT_MD_SAMPLE, DESIGN_MD_SAMPLE, CASE_STUDY_ANSWER } from './fixtures.mjs'; +} from '../skill-behavior/harness.mjs'; +import { detectProvider, getModel, hasKey, resolveModelList, PROVIDERS } from '../skill-behavior/providers.mjs'; +import { assertNewWorkLifecycle } from '../skill-behavior/assertions.mjs'; +import { PRODUCT_MD_SAMPLE, DESIGN_MD_SAMPLE as ORIGINAL_DESIGN, CASE_STUDY_ANSWER } from '../skill-behavior/fixtures.mjs'; +import { prepareBrowser } from './browser.mjs'; +import { assertCompleted, assertFreshCaptures } from './assertions.mjs'; + +const DESIGN_MD_SAMPLE = ORIGINAL_DESIGN.replace(/GT Sectra \(commercial\)/g, 'Georgia (system)').replace(/JetBrains Mono/g, 'monospace').replace(/Inter/g, 'Arial'); + +async function runTurn(options) { + // Preflight happens before the first provider call. These are text-only + // HTML fixtures: no dependencies, font downloads, or browser discovery. + const browser = await prepareBrowser(options.workspace); + try { + const result = await runHarnessTurn({ + ...options, maxSteps: 50, timeoutMs: 840000, + userPrompt: `${options.userPrompt}\nUse system fonts and no external assets for this text-only fixture. The browser_snapshot and view_image tools are ready for visual review.`, + environment: browser.environment, + additionalTools: (trace) => browser.tools(trace), + }); + assertCompleted(result); + const contextCalls = result.trace.toolCalls.filter(({ name, input }) => name === 'bash' && /impeccable\s+context\b/.test(input.command)); + assert.equal(contextCalls.length, 1, 'completed workflow must load context exactly once'); + return result; + } finally { + await browser.close(); + } +} const LEGACY_DESIGN = `# Design @@ -101,7 +125,9 @@ function workflowTraceMessage(trace) { return JSON.stringify(summarizeTrace(trace), null, 2); } -for (const modelId of resolveModelList()) { +// Full builds are separately opt-in and default to one provider. The existing +// model selection variable can explicitly request a cross-provider sweep. +for (const modelId of process.env.IMPECCABLE_SKILL_BEHAVIOR_MODELS ? resolveModelList() : ['claude-sonnet-5']) { const provider = detectProvider(modelId); const keyPresent = hasKey(provider); @@ -123,7 +149,6 @@ for (const modelId of resolveModelList()) { workspace, model, userPrompt: '/impeccable init for a harbor operations product, then finish setup.', - maxSteps: 24, }); const question = firstCall(trace, ({ name }) => name === 'ask_user_question'); const productWrite = firstMutation(trace, /(^|\/)PRODUCT\.md$/i); @@ -149,12 +174,14 @@ for (const modelId of resolveModelList()) { model, userPrompt: '/impeccable create a concise evidence-led case-study page. Leave it at index.html.', simulatedUser: { answer: () => CASE_STUDY_ANSWER }, - maxSteps: 22, }); const question = firstCall(trace, ({ name }) => name === 'ask_user_question'); assert.ok(fileLoaded(trace, 'new-work.md'), `new-work.md was not loaded.\n${workflowTraceMessage(trace)}`); assert.ok(question >= 0, `task concept was never put to the user.\n${workflowTraceMessage(trace)}`); assertNewWorkLifecycle(trace, { target: 'index.html' }); + assertFreshCaptures(trace, workspace, 'index.html'); + assert.ok(fileLoaded(trace, 'finish-reviewer.md'), 'new-work must run the shipped finish review'); + assert.ok(fileLoaded(trace, 'documenter.md'), 'new-work must run the shipped documentation pass'); assert.equal(fs.existsSync(path.join(workspace, 'index.html')), true, 'new-work must still produce the requested artifact'); } finally { cleanupWorkspace(workspace); @@ -174,12 +201,14 @@ for (const modelId of resolveModelList()) { workspace, model, userPrompt: '/impeccable redesign current.html for this product. Leave the result at current.html.', - maxSteps: 26, }); const question = firstCall(trace, ({ name }) => name === 'ask_user_question'); assert.ok(fileLoaded(trace, 'new-work.md'), `redesign did not route through new-work.\n${workflowTraceMessage(trace)}`); assert.ok(question >= 0, `replacement world was not put to the user.\n${workflowTraceMessage(trace)}`); assertNewWorkLifecycle(trace, { target: 'current.html', redesign: true }); + assertFreshCaptures(trace, workspace, 'current.html'); + assert.ok(fileLoaded(trace, 'finish-reviewer.md'), 'redesign must run the shipped finish review'); + assert.ok(fileLoaded(trace, 'documenter.md'), 'redesign must run the shipped documentation pass'); const design = fs.readFileSync(path.join(workspace, 'DESIGN.md'), 'utf8'); assert.notEqual(design.trim(), LEGACY_DESIGN.trim(), 'redesign preserved the old visual world verbatim'); } finally { @@ -200,7 +229,6 @@ for (const modelId of resolveModelList()) { workspace, model, userPrompt: '/impeccable bolder current.html, only the #case-study section. Keep everything else untouched.', - maxSteps: 16, }); const productWrite = firstMutation(trace, /(^|\/)PRODUCT\.md$/i); const designWrite = firstMutation(trace, /(^|\/)DESIGN\.md$/i); @@ -209,6 +237,7 @@ for (const modelId of resolveModelList()) { assert.equal(productWrite, -1, `refinement rewrote PRODUCT.md.\n${workflowTraceMessage(trace)}`); assert.equal(designWrite, -1, `refinement rewrote DESIGN.md.\n${workflowTraceMessage(trace)}`); assert.ok(implementation >= 0, `refinement did not write current.html.\n${workflowTraceMessage(trace)}`); + assertFreshCaptures(trace, workspace, 'current.html'); const artifact = fs.readFileSync(path.join(workspace, 'current.html'), 'utf8'); assert.match(artifact, /data-untouched="header"/); assert.match(artifact, /data-untouched="footer"/); @@ -236,9 +265,9 @@ for (const modelId of resolveModelList()) { workspace, model, userPrompt: '/impeccable critique current.html', - maxSteps: 30, }); assert.ok(fileLoaded(trace, 'critique.md'), `critique.md was not loaded.\n${workflowTraceMessage(trace)}`); + assertFreshCaptures(trace, workspace, 'current.html'); const parts = assistantParts(responseMessages); const allText = parts.filter((p) => p.kind === 'text').map((p) => p.value).join('\n'); diff --git a/tests/skill-workflow/source-hash.mjs b/tests/skill-workflow/source-hash.mjs new file mode 100644 index 000000000..528aa8ba9 --- /dev/null +++ b/tests/skill-workflow/source-hash.mjs @@ -0,0 +1,22 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import crypto from 'node:crypto'; + +// Include local styles/scripts/assets too: an unchanged HTML file is not +// evidence of a current capture when an external stylesheet changed. +export function sourceHash(root) { + const hash = crypto.createHash('sha256'); + function visit(directory, prefix = '') { + for (const entry of fs.readdirSync(directory, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name))) { + if (entry.name.startsWith('.') || entry.name === 'node_modules') continue; + const relative = `${prefix}${entry.name}`; + const file = path.join(directory, entry.name); + if (entry.isDirectory()) visit(file, `${relative}/`); + else if (entry.isFile() && /\.(html?|css|m?js|svg|png|jpe?g|webp|woff2?)$/i.test(entry.name)) { + hash.update(relative).update('\0').update(fs.readFileSync(file)).update('\0'); + } + } + } + visit(root); + return hash.digest('hex'); +} diff --git a/tests/test-suites.test.mjs b/tests/test-suites.test.mjs index d2621b52a..e69cd6b04 100644 --- a/tests/test-suites.test.mjs +++ b/tests/test-suites.test.mjs @@ -12,6 +12,13 @@ import { } from '../scripts/test-suites.mjs'; describe('test suite registry', () => { + it('separates protocol checkpoints from opt-in browser-backed completion', () => { + assert.deepEqual(suiteFiles(['skill-behavior']), ['tests/skill-behavior/scenarios.test.mjs']); + assert.ok(OPT_IN_SUITES.includes('skill-workflow')); + assert.equal(SUITES['skill-workflow'].needsPlaywright, true); + assert.ok(suiteFiles(['skill-workflow']).includes('tests/skill-workflow/full-build.test.mjs')); + assert.equal(DEFAULT_SUITES.includes('skill-workflow'), false); + }); it('assigns every test file to a default or opt-in suite', () => { const allDiscovered = findTestFiles(); const allRegistered = new Set(suiteFiles([...DEFAULT_SUITES, ...OPT_IN_SUITES]));