Fix skill workflow regression coverage (#783)

Clarify launcher fallback and completed documentation handoffs; separate bounded protocol checkpoints from opt-in browser-backed completion diagnostics. Correct fixture containment, target syntax, and artifact assertions. AI assistance: Codex, under maintainer direction.
This commit is contained in:
Paul Bakaus
2026-09-08 08:41:54 -07:00
committed by GitHub
parent 2bc2879276
commit 6496f49a1e
23 changed files with 1305 additions and 154 deletions
+262 -16
View File
@@ -2,8 +2,8 @@
LLM-backed scenarios that verify how the impeccable skill drives context,
command-reference, new-work, and native-platform loading. Each scenario runs
against one current model from each supported provider (Anthropic, OpenAI,
Google, DeepSeek).
against the default Anthropic, OpenAI, and Google models. DeepSeek remains
available through `IMPECCABLE_SKILL_BEHAVIOR_MODELS`.
These are the tests you re-run when you refactor anything in SKILL.md's
`## Setup` section. They fail when the agent stops following the loading
@@ -25,7 +25,7 @@ skipped, not failed.
Also requires the engine binary (`bun run fetch:engine`, or `IMPECCABLE_BIN`).
The staged skill dir ships the launcher (`scripts/impeccable`); the harness
exports `IMPECCABLE_BIN` into every bash call the agent makes, so the launcher
resolves the binary in both symlink and copy mode without a download. Without a
resolves the binary in the generated fixture without a download. Without a
binary the suites skip.
To run a single scenario against one model:
@@ -37,15 +37,175 @@ 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.
Update-notice (S9) and explicit-command (S18) checks also now stop at their
actual protocol checkpoints, instead of continuing into unrelated polishing;
their focused final reruns each passed 3/3. S9 now explicitly requires the
assistant to surface the update, not merely receive its loader directive.
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.
### Remaining gaps after the split (2026-09-07)
One provisioned Claude natural-build run reached a final response in 637 seconds
and 37 model steps, with approval, a surface brief, an implemented page, a
finish review, corrections, and fresh desktop/mobile screenshots. Its initial
completion assertions passed. The final test revision additionally requires
the shipped documentation reference; auditing the saved trace against that
guard found it missing. **This is not a final full-workflow pass.** Existing
DESIGN.md was preserved, but the required documentation pass was skipped.
The final guards were tightened during the run; this trace is not represented
as a run of those later assertions. No second full build was purchased.
Claude's remaining protocol batch passed S10S15 and existing-project S16,
but missing-context S16 omitted routing.md and S17 omitted critique.md.
Both responses remained read-only. The older S9 timed out during unrelated
polishing; the corrected focused S9 above supersedes it. The batch was stopped
during the older S18, before another provider sweep. This is incremental
evidence, not an all-green final matrix. Release #782 remains on hold.
The full build reported about 2.95 million input and 53 thousand output tokens
across all turns (no cache usage reported). Keep this lane manually scoped;
the fast protocol suite is not a proxy for its completion or cost.
### Documentation handoff follow-up
`tests/skill-workflow/finish-handoff.test.mjs` isolates a synthetic post-review
checkpoint without rebuilding or capturing a page. Its existing-system fixture
has no approved system change and a pre-existing missing sidecar; the correct
result is to check the build against DESIGN.md, preserve it, and leave unrelated
drift alone. The new-world control must write DESIGN.md and its v2 sidecar.
The unchanged-instructions baseline reproduced the skipped documenter in 31s.
The revised handoff and documenter passages are 29 words shorter overall and
make a checked no-change result explicit. The first focused extension retest
passed in 27s. A repeat checked the source, DESIGN.md, and document.md and made
no mutations, but omitted degraded/documenter.md, so the strict reference guard
still failed. The new-world control passed in 91s, writing both required files.
These small samples support the narrower behavior change, not an all-green
workflow claim; the reference-loading miss remains visible. No full build was
rerun. Focused Claude routing S3/S4, the ordinary suite, source-first build, and
generated-skill authoring validation passed. Release #782 remains held.
### Outcome assertions (maintainer-approved follow-up)
S16/S17 now require a completed, useful, read-only answer; S17 distinguishes
assessment from implementation and explains that critique is optional before
polish. Explicit invented prerequisites remain failures. Missing routing or
comparison references are TAP diagnostics based on successful content loads,
not failed read attempts. These English-fixture phrase checks are bounded
regression checks, not a comprehensive semantic grader.
The resumed ordinary-extension checkpoint accepts a direct documentation pass
without the degraded wrapper only with actual document.md, page, and DESIGN.md
reads, a concrete no-change report grounded in the fixture's type/palette/layout,
and zero mutations. Seeded files must still be byte-identical and pre-existing
sidecar drift must remain untouched. New-world documentation writes, redesign
ordering, and full-build completion gates are unchanged.
Offline re-evaluation of saved Claude traces: three advice responses and two
evidenced no-op handoffs pass the new assertions. The original post-review
baseline and the 637-second full build still fail for absent documentation
evidence. Unit negative controls reject fabricated prerequisites, unsolicited
edits/interviews/scans, failed reads, empty or unsupported reports, and exhausted
budgets. This is assertion replay, not new model evidence or a rerun of cleaned-up
workspaces' filesystem checks. No paid calls or skill prose changes were needed.
The earlier reference misses above are now diagnostics, not release blockers by
themselves; this does not establish an all-green full-workflow matrix.
### Bounded release verification (2026-09-08)
The focused Anthropic live-accept test now verifies the durable session reaches
`completed`, not just clean source/DOM. Its agent instruction had recommended
`data-impeccable-e2e-variant`, inside the reserved runtime namespace; the test
prompt now uses a permanent `data-design-variant` styling hook. The strengthened
test passed in 14s without forced completion. The full non-billed suite passed.
No runtime or skill source was changed for this correction.
Claude post-review controls: new world passed in 77s, writing token-bearing
DESIGN.md and the v2 sidecar; approved redesign failed in 16s. The latter read the
page and old system, then wrote prose-only DESIGN.md without consulting the
documentation spec or creating `.impeccable/design.json`, and claimed nothing
remained outstanding. This is a missing required artifact, not merely missing
reference coverage. The test stays red; no retry was purchased. Release remains
held pending disposition. These are synthetic post-review checkpoints, not
proof of a complete redesign lifecycle.
The single full Claude build passed its automated gates in 623s / 31 steps:
approval, brief, implemented page, final desktop/mobile captures, and shipped
reviewer/documenter wrapper loads. The final response gave an in-thread review
and a no-change documentation assessment of the incumbent system. It did not
load document.md itself; this is not evidence that every documentation protocol
step ran. The run used about 2.11M input / 50K output tokens (no cache reported).
No further billed retry was started. The separate missing-artifact redesign
failure still blocks treating this batch as an all-green skill release gate.
### Targeted redesign handoff correction
The parent handoff now explicitly requires token-bearing DESIGN.md and
`.impeccable/design.json` for approved system changes and verifies those outputs
before completion. It is nine words shorter; the agent and schema files did not
change. One unchanged-assertion Claude retest finished in 65s: it read document.md
and wrote both artifacts, but still skipped the degraded wrapper, so that
reference assertion failed before the artifact assertions ran.
Wrapper coverage is now diagnostic for all post-review modes; successful spec
and source reads, completed turns, write boundaries, tokens, and the v2 sidecar
remain hard gates. Offline evaluation of the saved retest passes these artifact
checks; the original prose-only/missing-sidecar trace remains rejected. Negative
controls cover absent tokens, malformed/missing sidecars, old schema versions,
and absent metadata. This is replay, not a second live pass or a rerun of the
cleaned-up workspace's byte-preservation checks.
Artifact audit: the sidecar component renders correctly in an offline browser.
The heading line-height was recorded as 1.3 while the page inherits 1.6 (38.4px
at 24px), and generatedAt used a placeholder date. These are remaining output
accuracy limitations, distinct from the corrected missing-artifact failure;
the shape checks do not establish complete token fidelity. No broad provider or
full-build rerun was purchased. Build and generated-skill validation passed.
Each scenario:
1. `prepareWorkspace()` mints a temp dir, symlinks the canonical skill
into `<workspace>/.claude/skills/impeccable` (so its launcher is at
`.claude/skills/impeccable/scripts/impeccable`), and optionally writes
`PRODUCT.md` / `DESIGN.md` fixtures.
1. `prepareWorkspace()` uses the production transformer to build current source
into an independent `<workspace>/.claude/skills/impeccable`. References have
resolved placeholders and generated degraded reviewer/documenter files.
Host-specific blocks are omitted: this is a neutral API harness, not an exact
Claude/Codex/Gemini host simulation. It optionally seeds project fixtures.
2. `runTurn()` inlines `SKILL.md` (placeholders neutralized) as the
system prompt and runs Vercel AI SDK `generateText` with four
workspace-scoped tools: `bash`, `read`, `write`, `list`, and a fake
system prompt and runs Vercel AI SDK `generateText` with five
tools: `bash`, `read`, `write`, `list`, and a fake
provider-neutral `ask_user_question` backed by a deterministic simulated user.
3. The tools record every call into a `trace` that the test asserts on.
4. For scenario 4, a second `runTurn` reuses turn 1's `responseMessages`
@@ -53,6 +213,92 @@ Each scenario:
The trace is the source of truth, not the model's free-form reply.
File tools are workspace-scoped; bash is a real host shell, **not a security
sandbox**. Use disposable synthetic fixtures. Shell helpers do not inherit
provider API keys/auth tokens; model calls still use the parent's keys. The
harness always sets `IMPECCABLE_QUESTION_DISABLED=1` for shell calls so real
decision pages cannot wait for a nonexistent browser user. The engine returns
its genuine structured-question fallback; browser decisions have separate E2E.
Set `IMPECCABLE_SKILL_BEHAVIOR_TRACE_DIR=<directory>` to retain per-turn JSON
with model, prompt, tool results, response ordering, usage, and finish reason.
Progress and failed turns retain their tool traces too; only completed turns
carry the full response sequence and final usage.
These are local diagnostic artifacts; inspect before sharing. Successful reads
or full reference content in shell output count as loading; filename mentions,
denied commands, and failed reads do not.
Context-only controls permit the real launcher with an optional workspace-relative
`--target`; compound commands remain rejected. The target form is part of the
skill's Setup contract, not a launcher failure.
## Release investigation (2026-09-07)
The initial release sweep reported 68/81 passes. Do not interpret its 13 failed
assertions as 13 demonstrated product regressions. The harness staged raw
references with unresolved placeholders, omitted generated degraded roles, and
allowed unanswered browser decisions. Its redesign assertion was also stale:
current `new-work.md` requires a surface brief **before code**, and DESIGN.md
**at finish**, from the built world. The corrected lifecycle checks retain
approval, brief, implementation, and final documentation requirements; missing
artifacts now have their own error instead of being called premature edits.
The historical tables below retain their original measurements and methods.
Focused launcher-fallback verification on the corrected fixture:
| Default model | Old fallback paragraph | Explicit pre-tool warning paragraph |
|---|---:|---:|
| `claude-sonnet-5` | 2/3 | 3/3 |
| `gpt-5.6-terra` | 3/3 | 3/3 |
| `gemini-3.7-flash` | 1/3 | 3/3 |
The three cases are denied editing, successful-loader control, and denied
planning. The old-paragraph failures were warning order, not refused edits.
An intermediate candidate run scored 8/9 because the control rejected valid
`context --target index.html`; after correcting that allowlist, the full focused
rerun passed 9/9. This is one measured run per variant, not a reliability estimate
or an all-workflow pass. Broader routing and workflow results remain separate.
On the resolved fixture, Gemini's workflow run passed 4/5: the completed new
page omitted user confirmation. Tightening the existing question paragraph
made its focused build lifecycle rerun pass 1/1. OpenAI passed all five workflow
cases with the fixture corrections alone. These are incremental measurements,
not one full sweep on the final candidate.
Claude's three-step routing sweep cut off two setup cases before loading
`new-work.md`. Both loaded it in bounded six-step diagnostics. Claude now has
the same six-step setup allowance as Gemini; reference and edit-order assertions
are unchanged. A full-build baseline separately hit the existing 840-second
deadline and is not counted as a pass.
The complete routing-only sweep retained its original budgets and passed 55/57
(Claude 17/19, OpenAI 19/19, Gemini 19/19). A six-step Claude rerun passed the two
previously clipped cases but exposed a separate context reload on turn two:
the model queried the loader again for image-tool availability. That run was
stopped after five passes and this failure rather than finishing another billed
sweep; the once-per-session assertion remains unchanged.
A saved Claude build trace used five calls shortening/counting the direction
contract, then reached the 22-step cap before implementation. The word target
is now approximate, with the six required blocks retained. A subsequent run
correctly stopped for missing customer evidence: the default simulated user had
selected “I have real details,” then promised them in a future message. The
case-study test now supplies a complete, explicitly synthetic brief when asked;
fresh-init and other user simulations are unchanged. Neither incomplete build
is counted as a pass.
The corrected-user Claude retest also remained incomplete: it asked, recorded
the six-block brief without the earlier word-count loop, then spent the remaining
22-step allowance acquiring and inspecting fonts before writing HTML. The
26-step redesign run produced the page and desktop/mobile captures but stopped
before DESIGN.md. These results do not establish full workflow completion.
Further work should separate narrow protocol checks from realistic, provisioned
full-build runs rather than keep adding skill prose or relaxing finish gates.
The later Claude run passed fresh init and refinement, failed the two bounded
build cases, and was stopped during critique's browser-tool discovery. Its
unfinished critique case is not a pass; the earlier OpenAI/Gemini critique
results remain the completed measurements.
## Scenarios
| # | Setup | Assertion |
@@ -72,8 +318,8 @@ The trace is the source of truth, not the model's free-form reply.
| 13 | empty workspace; prompt is `/impeccable teach` | runs `impeccable context` and diverts into `reference/init.md` because `teach` aliases `init` |
| 14 | PRODUCT.md with `## Platform: ios` (native iOS app); prompt is `/impeccable craft a tide detail screen` | `impeccable context` runs and emits the contents of `reference/ios.md` directly, placing native conventions in context without a second model-directed read |
| 15 | same iOS fixture; prompt is `/impeccable audit` | agent loads `reference/audit.native.md` (the Commands-table native variant, routed instead of `audit.md`) |
| 16 | existing surface, with and without PRODUCT.md; asks where to start | loads `routing.md`, delivers advice, and does not edit project files, start an interview, archive a critique, or run menu scans |
| 17 | existing surface; asks whether critique is required before polish | loads `routing.md` and both command references, then delivers advice without executing the playbooks |
| 16 | existing surface, with and without PRODUCT.md; asks where to start | completes relevant advice without edits, interviews, critique archives, menu scans, or explicit invented refinement prerequisites; reference coverage is diagnostic |
| 17 | existing surface; asks whether critique is required before polish | completes read-only advice distinguishing assessment from implementation and explaining critique is optional; reference coverage is diagnostic |
| 18 | existing surface; explicitly requests polish followed by a next-command recommendation | loads `polish.md` rather than substituting workflow advice for the requested work |
| 19 | tiny spacing edit with PRODUCT.md + DESIGN.md; Bash denied, a real-loader success control, and a denied-launcher planning-only case | edits require successful playbook/craft-floor reads and a pre-edit denial warning; planning stays read-only and skips craft-floor |
@@ -188,7 +434,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.
@@ -365,12 +611,12 @@ 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
```
Keep `--test-timeout` at 300000. A tighter cap turns claude-sonnet-5's slower
runs into timeouts that look like failures. Set `IMPECCABLE_QUESTION_DISABLED=1`
and `CI=1` so `impeccable serve-question` cannot open a browser window on the host. Pipe
Use the suite's current 900000ms timeout for full workflow cases; the 300000ms
example above is historical. The harness now disables decision pages itself.
Pipe
to a file rather than `tail`; node prints the failing-test summary at the end,
and truncating it costs you the per-model attribution.
+59
View File
@@ -1,5 +1,64 @@
import assert from 'node:assert/strict';
// These are bounded English-fixture checks, not a general semantic grader.
// Reference coverage is reported separately: opening a file proves neither
// useful advice nor permission to execute it.
export function missingReferences(trace, filenames) {
return filenames.filter((filename) => !trace.toolCalls.some((call) =>
(call.loadedFiles || []).some((file) => file === filename || file.endsWith(`/${filename}`))));
}
export function assertAdviceOnly(trace, text) {
assert.ok(text.trim(), 'advice must reach the user, not stop at reference loading');
assert.deepEqual(trace.writePaths, [], 'advice must not use the write tool');
const mutations = trace.toolCalls.flatMap((call) => call.mutatedPaths ?? [])
.filter((file) => !file.startsWith('.impeccable/') || file.startsWith('.impeccable/critique/'));
assert.deepEqual(mutations, [], 'advice must not edit project files or archive an unsolicited critique');
assert.deepEqual(trace.questionCalls, [], 'advice must not start an init or design interview');
assert.ok(!trace.bashCommands.some((command) => command.includes('impeccable detect')), 'workflow advice does not run menu scans');
}
function normalizedAdvice(text) {
return text.replace(/[`*_]/g, '').replace(/[]/g, "'").toLowerCase();
}
export function assertWorkflowAdvice(trace, text, { missingContext = false } = {}) {
assertAdviceOnly(trace, text);
const advice = normalizedAdvice(text);
assert.match(advice, /index\.html/, 'advice should address the existing surface');
assert.match(advice, missingContext ? /\binit\b/ : /\b(?:critique|audit|polish)\b/, 'advice must recommend a relevant starting point');
if (missingContext) assert.match(advice, /\bdocument\b/, 'advice should explain how to record the existing identity');
assert.doesNotMatch(advice, /(?:must|need to|have to|required to)\s+(?:run\s+)?(?:\/impeccable\s+)?(?:init|document)\b[^.!?\n]{0,100}\bbefore\s+(?:you\s+can\s+)?(?:run(?:ning)?\s+)?(?:polish(?:ing)?|refin(?:e|ing|ement))\b|(?:polish|refinement)\s+(?:requires|is blocked by|cannot run without)\s+(?:init|document|product\.md|design\.md)/,
'setup is not a mandatory prerequisite for narrow refinement');
}
export function assertCommandComparison(trace, text) {
assertAdviceOnly(trace, text);
const advice = normalizedAdvice(text);
assert.match(advice, /critique[^.!?\n]{0,120}(?:review|assess|evaluat|report|findings)/, 'comparison must explain critique as assessment');
assert.match(advice, /polish[^.!?\n]{0,120}(?:fix|refin|implement|edit)/, 'comparison must explain polish as implementation');
assert.match(advice, /critique\s+(?:is\s+)?(?:isn't|is not|not)\s+(?:required|necessary)|critique[^.!?\n]{0,50}\boptional\b|polish[^.!?\n]{0,100}(?:directly|without\s+(?:a\s+)?critique|independent)/,
'comparison must explain that critique is optional before polish');
assert.doesNotMatch(advice, /(?:must|need to|have to)\s+(?:run\s+)?critique[^.!?\n]{0,80}before\s+(?:run(?:ning)?\s+)?polish|critique\s+(?:is\s+)?(?:required|mandatory|necessary)\s+before\s+polish|polish\s+(?:requires|cannot run without)\s+(?:a\s+)?critique/,
'comparison must not invent a critique prerequisite');
}
export function assertNewWorkLifecycle(trace, { target, redesign = false }) {
const calls = trace.toolCalls;
const writes = (call, file) => (call.mutatedPaths || []).includes(file);
const implementation = calls.findIndex((call) => writes(call, target));
const question = calls.findIndex((call) => call.name === 'ask_user_question');
const brief = calls.findIndex((call) => (call.mutatedPaths || []).some((file) => file.startsWith('.impeccable/surfaces/')));
assert.ok(implementation >= 0, `new-work did not produce the requested artifact: ${target}`);
assert.ok(question >= 0 && question < implementation, 'implementation must follow a user answer');
assert.ok(brief >= 0 && brief < implementation, 'the direction contract must be recorded in a surface brief before implementation');
if (redesign) {
const lastImplementation = calls.findLastIndex((call) => writes(call, target));
const documentation = calls.findLastIndex((call) => writes(call, 'DESIGN.md'));
assert.ok(documentation > lastImplementation, 'redesign must record DESIGN.md from the finished build, after the last page edit');
}
}
export const LAUNCHER_FAILURE_WARNING = /(?:context|launcher|bash)[^.!?\n]{0,160}(?:denied|refused|unavailable|blocked|could(?:n't| not)|cannot|can't|did(?:n't| not)|fail|unable)|(?:denied|refused|unavailable|blocked|could(?:n't| not)|cannot|can't|unable)[^.!?\n]{0,160}(?:context|launcher|bash)/i;
export function assertPlanningFallbackWarning(responseMessages) {
+3
View File
@@ -5,6 +5,9 @@
* uses to decide whether to gate on `init`. Plausible enough that the agent
* treats them as real context rather than test scaffolding.
*/
// This user can supply a complete synthetic case now, not promise future assets.
export const CASE_STUDY_ANSWER = 'Build a standalone index.html for the researchers described in PRODUCT.md, preserving DESIGN.md. Use a clearly labeled synthetic case: a doctoral researcher reconstructs why a literature-review conclusion changed by following linked notes and citations. No real customer names, quotes, metrics, or assets are available; author illustrative content and label it, with no invented commercial claims. The reader should understand the preserved reasoning trail and follow an in-page link to the method. For a composition or concept choice, use the first direction you presented. No additional material is coming from me.';
export const PRODUCT_MD_SAMPLE = `# Acme Notes
## Platform
+108 -51
View File
@@ -1,9 +1,9 @@
/**
* Sandboxed scenario runner for skill-behavior tests.
* Synthetic-workspace scenario runner for skill-behavior tests.
*
* Each scenario:
* 1. Creates a temp workspace.
* 2. Symlinks the real .claude/skills/impeccable into the workspace so
* 2. Builds a neutral .claude/skills/impeccable into the workspace so
* the launcher (`scripts/impeccable`) resolves from the canonical path
* the skill references, and points it at an engine binary.
* 3. Optionally writes PRODUCT.md / DESIGN.md fixtures.
@@ -15,8 +15,8 @@
* messages (so multi-turn scenarios can append to them).
*
* The harness deliberately mirrors the live-mode E2E pattern: real LLM,
* no mocks, but tightly bounded execution surface so we observe the routing
* behavior of the skill without paying for full-fledged design work.
* no mocked model. File tools are workspace-scoped; bash is a real host shell,
* not a security sandbox. Run only against disposable synthetic fixtures.
*/
import { generateText, stepCountIs, tool } from 'ai';
import { z } from 'zod';
@@ -28,12 +28,35 @@ import { spawn } from 'node:child_process';
import { fileURLToPath } from 'node:url';
import { getProviderOptions } from './providers.mjs';
import { ENGINE_MISSING_MESSAGE, findEngineBinary } from '../lib/engine-bin.mjs';
import { readSourceFiles, compileProviderBlocks, replacePlaceholders, stripRuleMarkers } from '../../scripts/lib/utils.js';
import { createTransformer } from '../../scripts/lib/transformers/factory.js';
const __dirname = path.dirname(fileURLToPath(import.meta.url));
const REPO_ROOT = path.resolve(__dirname, '..', '..');
const SKILL_SOURCE_DIR = path.join(REPO_ROOT, 'skill');
const MAX_BASH_OUTPUT_BYTES = 200_000;
function renderNeutral(content) {
return stripRuleMarkers(replacePlaceholders(compileProviderBlocks(content, [])
.replaceAll('{{ask_instruction}}', 'Use the ask_user_question tool.')
.replaceAll('{{model}}', 'the assistant'), 'dsh'))
.replaceAll('{{scripts_path}}', '.claude/skills/impeccable/scripts')
.replaceAll('{{command_hint}}', 'command');
}
// Use the production builder so fallback reviewer/documenter references exist.
// Generic tool names are shared by the API providers; host-specific blocks are
// deliberately absent. Exact provider transforms have separate loader tests.
const sourceSkills = readSourceFiles(REPO_ROOT).skills.map((skill) => ({
...skill,
body: renderNeutral(skill.body),
references: skill.references.map((ref) => ({ ...ref, content: renderNeutral(ref.content) })),
agents: skill.agents.map((agent) => ({ ...agent, body: renderNeutral(agent.body) })),
}));
const stageSkill = createTransformer({
provider: 'skill-behavior', placeholderProvider: 'dsh', providerTags: [],
configDir: '.claude', displayName: 'Behavior fixture',
});
function snapshotWorkspaceFiles(root) {
const snapshot = new Map();
const walk = (dir, relDir = '') => {
@@ -66,24 +89,7 @@ function changedPaths(before, after) {
* is provider-neutral when inlined.
*/
function loadSkillBody() {
let md = fs.readFileSync(path.join(SKILL_SOURCE_DIR, 'SKILL.src.md'), 'utf8');
// Strip frontmatter.
if (md.startsWith('---')) {
const end = md.indexOf('\n---', 3);
if (end !== -1) md = md.slice(end + 4).trimStart();
}
// The source uses placeholders that the build step replaces per-provider.
// For the test harness we want a single body that works for any provider,
// and the scripts the skill references live at .claude/skills/impeccable/
// (the workspace symlink), so hard-code those values.
md = md
.replaceAll('{{model}}', 'the assistant')
.replaceAll('{{command_prefix}}', '/')
.replaceAll('{{ask_instruction}}', 'Use the ask_user_question tool.')
.replaceAll('{{config_file}}', 'AGENTS.md')
.replaceAll('{{scripts_path}}', '.claude/skills/impeccable/scripts')
.replaceAll('{{command_hint}}', 'command');
return md.trim();
return sourceSkills[0].body.trim();
}
// This provider-neutral fixture assumes a loaded skill with a known base
@@ -96,36 +102,26 @@ export const SKILL_BODY = `Base directory for this skill (workspace-relative): .
/**
* Create a temp workspace and prepopulate it.
*
* - `.claude/skills/impeccable` is symlinked at the SOURCE skill dir (not
* the built `.claude/skills/impeccable/`) so the test exercises whatever
* is in `skill/` right now, without needing `bun run build` to refresh
* the harness output dirs. The trade-off: reference files surface their
* raw `{{placeholders}}`, but the assertions only check tool calls, not
* their content.
* - Compile current source into an independent fixture distribution. Shell
* and read tools see the same resolved references, including degraded roles.
* - `files` lets the test seed PRODUCT.md / DESIGN.md (or anything else).
* - `skillVersion` switches from symlink to a real COPY of the skill dir and
* writes a `SKILL.md` carrying that version. `impeccable context` reads its
* - `skillVersion` adds a `SKILL.md` version. `impeccable context` reads its
* own version from that sibling file, so this is required for any scenario
* that exercises the update-check path (the source dir has only SKILL.src.md).
*
* The launcher in the staged scripts dir needs an engine binary. Every bash
* call the agent makes gets `IMPECCABLE_BIN` (tests/lib/engine-bin.mjs:
* `IMPECCABLE_BIN` or `skill/scripts/bin/<os>-<arch>/`), which the launcher
* honors first, so the symlink and copy modes both work without a download.
* honors first, so the staged skill works without a download.
*/
export const ENGINE_BIN = findEngineBinary();
export { ENGINE_MISSING_MESSAGE };
export function prepareWorkspace({ files = {}, skillVersion = null } = {}) {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-skill-test-'));
const skillDest = path.join(dir, '.claude', 'skills', 'impeccable');
fs.mkdirSync(path.join(dir, '.claude', 'skills'), { recursive: true });
if (skillVersion) {
fs.cpSync(SKILL_SOURCE_DIR, skillDest, { recursive: true });
fs.writeFileSync(path.join(skillDest, 'SKILL.md'), `---\nname: impeccable\nversion: ${skillVersion}\n---\n\nbody\n`);
} else {
fs.symlinkSync(SKILL_SOURCE_DIR, skillDest, 'dir');
}
stageSkill(sourceSkills, dir, { skillsVersion: skillVersion || '' });
fs.renameSync(path.join(dir, 'skill-behavior', '.claude'), path.join(dir, '.claude'));
fs.rmdirSync(path.join(dir, 'skill-behavior'));
for (const [name, contents] of Object.entries(files)) {
const target = path.join(dir, name);
fs.mkdirSync(path.dirname(target), { recursive: true });
@@ -154,14 +150,43 @@ function safeResolve(root, userPath) {
if (rel.startsWith('..') || rel.split(path.sep).includes('..')) {
return { error: 'path escapes the workspace' };
}
return resolved;
try {
// New write targets need not exist; validate their nearest existing
// ancestor, including dangling links, before appending the missing suffix.
let ancestor = resolved;
while (!fs.existsSync(ancestor)) {
if (fs.lstatSync(ancestor, { throwIfNoEntry: false })?.isSymbolicLink()) {
return { error: 'path follows a dangling symlink' };
}
ancestor = path.dirname(ancestor);
}
const canonical = path.resolve(fs.realpathSync(ancestor), path.relative(ancestor, resolved));
const realRel = path.relative(fs.realpathSync(root), canonical);
if (realRel === '..' || realRel.startsWith(`..${path.sep}`) || path.isAbsolute(realRel)) {
return { error: 'path escapes the workspace through a symlink' };
}
return canonical;
} catch {
return { error: 'path cannot be resolved safely' };
}
}
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_./+-]+)))?$/);
if (!match) return false;
const target = match[1] ?? match[2] ?? match[3];
return target === undefined || (!target.startsWith('-') && typeof safeResolve(workspace, target) === 'string');
}
function execBash(workspace, command, timeoutMs = 20_000, extraEnv = {}) {
return new Promise((resolve) => {
// Model credentials belong to generateText, not to child image helpers.
// Real decision pages have browser E2E; this suite has a structured user.
const shellEnv = Object.fromEntries(Object.entries({ ...process.env, ...extraEnv })
.filter(([name]) => !/(?:^|_)(?:API_KEY|AUTH_TOKEN|ACCESS_TOKEN)$/.test(name)));
const proc = spawn('bash', ['-lc', command], {
cwd: workspace,
env: { ...process.env, ...(ENGINE_BIN ? { IMPECCABLE_BIN: ENGINE_BIN } : {}), ...extraEnv },
env: { ...shellEnv, ...(ENGINE_BIN ? { IMPECCABLE_BIN: ENGINE_BIN } : {}), IMPECCABLE_QUESTION_DISABLED: '1' },
});
let stdout = '';
let stderr = '';
@@ -225,6 +250,10 @@ function defaultSimulatedAnswer(question) {
}
export function makeTools(workspace, extraEnv = {}, simulatedUser = {}, { contextOnlyBash = false, denyBash = false } = {}) {
const referenceDir = path.join(workspace, '.claude/skills/impeccable/reference');
const references = fs.readdirSync(referenceDir, { recursive: true })
.filter((file) => file.endsWith('.md'))
.map((file) => ({ file: file.split(path.sep).join('/'), content: fs.readFileSync(path.join(referenceDir, file), 'utf8').trim() }));
const trace = {
toolCalls: [],
bashCommands: [],
@@ -248,7 +277,7 @@ export function makeTools(workspace, extraEnv = {}, simulatedUser = {}, { contex
const tools = {
bash: tool({
description: contextOnlyBash
? 'Only `.claude/skills/impeccable/scripts/impeccable context` is allowed here. Use read/list for workspace files and skill references; write remains available for requested edits.'
? 'Only `.claude/skills/impeccable/scripts/impeccable context` with an optional `--target <workspace-relative path>` is allowed here. Use read/list for files and references; write remains available for requested edits.'
: 'Run a bash command in the workspace root. Use this to invoke skill commands (e.g. `.claude/skills/impeccable/scripts/impeccable context`).',
inputSchema: z.object({
command: z.string().describe('The bash command to execute.'),
@@ -265,14 +294,16 @@ export function makeTools(workspace, extraEnv = {}, simulatedUser = {}, { contex
}
// Routing tests need the real context loader, not a general-purpose
// shell on the host. Reject before execution (still record attempts).
if (contextOnlyBash && command.trim() !== '.claude/skills/impeccable/scripts/impeccable context') {
const out = 'Error: only `.claude/skills/impeccable/scripts/impeccable context` is allowed. Use read/list for files; references live at .claude/skills/impeccable/reference/.';
if (contextOnlyBash && !isContextOnlyCommand(workspace, command)) {
const out = 'Error: only `.claude/skills/impeccable/scripts/impeccable context` with an optional workspace-relative `--target` is allowed. Use read/list for files; references live at .claude/skills/impeccable/reference/.';
trace.bashOutputs.push(out);
return out;
}
const before = snapshotWorkspaceFiles(workspace);
const res = await execBash(workspace, command, 20_000, extraEnv);
call.mutatedPaths = changedPaths(before, snapshotWorkspaceFiles(workspace));
call.loadedFiles = references.filter(({ content }) => content && res.stdout.includes(content))
.map(({ file }) => `.claude/skills/impeccable/reference/${file}`);
const head = `exit=${res.exitCode}`;
const body = (res.stdout ? `stdout:\n${res.stdout}` : '') + (res.stderr ? `\nstderr:\n${res.stderr}` : '');
const out = `${head}\n${body}${res.truncated ? '\n[output truncated]' : ''}`;
@@ -295,6 +326,7 @@ export function makeTools(workspace, extraEnv = {}, simulatedUser = {}, { contex
if (stat.isDirectory()) return `Path is a directory: ${p}. Use list instead.`;
const contents = fs.readFileSync(resolved, 'utf8');
call.succeeded = true;
call.loadedFiles = [p];
return contents;
},
}),
@@ -308,7 +340,7 @@ export function makeTools(workspace, extraEnv = {}, simulatedUser = {}, { contex
const call = record('write', { path: p, contents });
const resolved = safeResolve(workspace, p);
if (typeof resolved !== 'string') return `Error: ${resolved.error}`;
if ((contextOnlyBash || denyBash) && path.relative(workspace, resolved).split(path.sep)[0] === '.claude') {
if (path.relative(fs.realpathSync(workspace), resolved).split(path.sep)[0] === '.claude') {
return 'Error: the staged skill is read-only; edits must target project files.';
}
fs.mkdirSync(path.dirname(resolved), { recursive: true });
@@ -386,12 +418,20 @@ 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 },
];
const traceDir = process.env.IMPECCABLE_SKILL_BEHAVIOR_TRACE_DIR;
const tracePath = traceDir && path.join(traceDir, `${path.basename(workspace)}-${crypto.randomUUID()}.json`);
const saveTrace = (details) => {
if (!tracePath) return;
fs.mkdirSync(traceDir, { recursive: true });
fs.writeFileSync(tracePath, JSON.stringify({ model: model.modelId, userPrompt, trace, ...details }, null, 2));
};
let result;
const controller = new AbortController();
const timer = setTimeout(
@@ -402,10 +442,14 @@ 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,
stopWhen: [stepCountIs(maxSteps)],
onStepFinish: (step) => {
(trace.assistantTexts ??= []).push(step.text ?? '');
saveTrace({ status: 'in-progress', lastStepMessages: step.response.messages });
},
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,
@@ -420,18 +464,27 @@ export async function runTurn({ workspace, model, userPrompt, priorMessages = []
});
} catch (err) {
const reason = controller.signal.aborted ? ` (aborted after ${timeoutMs}ms client-side timeout)` : '';
saveTrace({ status: 'failed', error: `${String(err)}${reason}` });
throw new Error(`LLM behavior turn failed before completing${reason}: ${String(err)}`, { cause: err });
} finally {
clearTimeout(timer);
}
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,
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,
};
}
@@ -451,8 +504,12 @@ export function readsMatching(trace, substring) {
* True if the agent loaded a file by Read OR by a bash `cat` (some models
* stream multiple files via bash to save tool calls).
*/
export function callLoadedFile(call, filename) {
return (call.loadedFiles || []).some((file) => file === filename || file.endsWith(`/${filename}`));
}
export function fileLoaded(trace, filename) {
return readsMatching(trace, filename).length > 0 || bashCommandsMatching(trace, filename).length > 0;
return trace.toolCalls.some((call) => callLoadedFile(call, filename));
}
export function summarizeTrace(trace) {
+52 -38
View File
@@ -18,16 +18,18 @@ import path from 'node:path';
import {
prepareWorkspace,
cleanupWorkspace,
runTurn,
runTurn as runHarnessTurn,
bashCommandsMatching,
readsMatching,
fileLoaded,
callLoadedFile,
summarizeTrace,
ENGINE_BIN,
ENGINE_MISSING_MESSAGE,
} from './harness.mjs';
import { detectProvider, getModel, hasKey, resolveModelList, PROVIDERS } from './providers.mjs';
import { assertPlanningFallbackWarning, LAUNCHER_FAILURE_WARNING } from './assertions.mjs';
import { assertPlanningFallbackWarning, LAUNCHER_FAILURE_WARNING, assertAdviceOnly, assertWorkflowAdvice, assertCommandComparison, missingReferences } from './assertions.mjs';
import { assertCompleted } from '../skill-workflow/assertions.mjs';
import {
PRODUCT_MD_SAMPLE,
PRODUCT_MD_SAMPLE_NO_REGISTER,
@@ -39,9 +41,22 @@ 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 UPDATE_NOTICE = /(?:skill|impeccable).{0,100}(?:update|version)|(?:update|version).{0,100}(?:skill|impeccable)|99\.0\.0/i;
const TEACH_PROMPT = '/impeccable teach';
const PRIMER_PROMPT =
'Take a quick look at the project. What context should guide later design work? Run the impeccable context loader once if you need to.';
@@ -57,14 +72,9 @@ function logTrace(label, scenario, model, trace, extras = {}) {
}
function loadedBeforeImplementationWrite(trace, filename) {
const needle = filename.toLowerCase();
const loadIndex = trace.toolCalls.findIndex(({ name, input }) => {
if (name === 'read') return input?.path?.toLowerCase().includes(needle);
if (name === 'bash') return input?.command?.toLowerCase().includes(needle);
return false;
});
const loadIndex = trace.toolCalls.findIndex((call) => callLoadedFile(call, filename));
const writeIndex = trace.toolCalls.findIndex(
({ name, input }) => name === 'write' && /\.(html?|css|svelte|jsx?|tsx?)$/i.test(input?.path ?? ''),
({ mutatedPaths = [] }) => mutatedPaths.some((file) => /\.(html?|css|svelte|jsx?|tsx?)$/i.test(file)),
);
return loadIndex >= 0 && (writeIndex < 0 || loadIndex < writeIndex);
}
@@ -81,16 +91,6 @@ function executedUpdateCommands(trace) {
);
}
function assertAdviceOnly(trace, text) {
assert.ok(text.trim(), 'advice must reach the user, not stop at reference loading');
assert.deepEqual(trace.writePaths, [], 'advice must not use the write tool');
const mutations = trace.toolCalls.flatMap((call) => call.mutatedPaths ?? [])
.filter((file) => !file.startsWith('.impeccable/') || file.startsWith('.impeccable/critique/'));
assert.deepEqual(mutations, [], 'advice must not edit project files or archive an unsolicited critique');
assert.deepEqual(trace.questionCalls, [], 'advice must not start an init or design interview');
assert.equal(bashCommandsMatching(trace, 'impeccable detect').length, 0, 'workflow advice does not run menu scans');
}
for (const modelId of resolveModelList()) {
const provider = detectProvider(modelId);
const keyPresent = hasKey(provider);
@@ -105,17 +105,14 @@ for (const modelId of resolveModelList()) {
return;
}
const model = getModel(modelId);
// Gemini Flash tends to inspect one file at a time, while the production
// Anthropic/OpenAI models batch setup reads and then begin implementation.
// Keep the latter tightly bounded so this routing suite does not turn into
// a page-generation benchmark, but leave Gemini enough room to reach the
// same required reference.
const setupMaxSteps = provider === 'google' ? 6 : 3;
// 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,
@@ -149,6 +146,7 @@ for (const modelId of resolveModelList()) {
});
try {
const { trace, text } = await runTurn({
checkpoint: 'new-work.md',
workspace,
model,
userPrompt: CRAFT_PROMPT,
@@ -177,6 +175,7 @@ for (const modelId of resolveModelList()) {
});
try {
const { trace, text } = await runTurn({
checkpoint: 'new-work.md',
workspace,
model,
userPrompt: CRAFT_PROMPT,
@@ -221,6 +220,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,
@@ -236,6 +236,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.',
@@ -261,6 +262,7 @@ for (const modelId of resolveModelList()) {
});
try {
const { trace, text } = await runTurn({
checkpoint: 'new-work.md',
workspace,
model,
userPrompt: CRAFT_PROMPT,
@@ -292,6 +294,7 @@ for (const modelId of resolveModelList()) {
});
try {
const { trace, text } = await runTurn({
checkpoint: 'polish.md',
workspace,
model,
userPrompt: '/impeccable polish index.html',
@@ -318,6 +321,7 @@ for (const modelId of resolveModelList()) {
});
try {
const { trace, text } = await runTurn({
checkpoint: 'audit.md',
workspace,
model,
userPrompt: '/impeccable audit index.html',
@@ -344,6 +348,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',
@@ -354,9 +359,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` +
@@ -392,6 +395,7 @@ for (const modelId of resolveModelList()) {
userPrompt: '/impeccable polish index.html',
maxSteps: setupMaxSteps,
env: { IMPECCABLE_UPDATE_CACHE: path.join(workspace, '.impeccable-update.json') },
checkpoint: (trace) => trace.assistantTexts?.some((text) => UPDATE_NOTICE.test(text)),
});
logTrace('S9', 'update-available', modelId, trace, { textSample: text.slice(0, 400) });
@@ -408,6 +412,7 @@ for (const modelId of resolveModelList()) {
`bashOutputs: ${JSON.stringify(trace.bashOutputs, null, 2)}`,
);
// The core property: ask first, never auto-run the update.
assert.ok(trace.assistantTexts?.some((text) => UPDATE_NOTICE.test(text)), 'the skill update must be surfaced to the user');
const ranUpdate = executedUpdateCommands(trace);
assert.equal(
ranUpdate.length,
@@ -431,6 +436,7 @@ for (const modelId of resolveModelList()) {
});
try {
const { trace, text } = await runTurn({
checkpoint: 'polish.md',
workspace,
model,
userPrompt: '/impeccable polish index.html',
@@ -469,6 +475,7 @@ for (const modelId of resolveModelList()) {
const workspace = prepareWorkspace({ files: {} });
try {
const { trace, text } = await runTurn({
checkpoint: 'init.md',
workspace,
model,
userPrompt: SHAPE_PROMPT,
@@ -494,6 +501,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,
@@ -521,6 +529,7 @@ for (const modelId of resolveModelList()) {
const workspace = prepareWorkspace({ files: {} });
try {
const { trace, text } = await runTurn({
checkpoint: 'init.md',
workspace,
model,
userPrompt: TEACH_PROMPT,
@@ -554,6 +563,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',
@@ -589,6 +599,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',
@@ -614,40 +625,42 @@ for (const modelId of resolveModelList()) {
['existing project', WORKFLOW_ADVICE_FILES],
['missing product context', { 'index.html': MINIMAL_LANDING_HTML }],
]) {
it(`scenario 16: workflow advice stays read-only (${label})`, async () => {
it(`scenario 16: workflow advice stays read-only (${label})`, async (t) => {
const workspace = prepareWorkspace({ files });
try {
const { trace, text } = await runTurn({
const result = await runTurn({
workspace,
model,
userPrompt: "I'm joining this project. Where should I start with Impeccable?",
maxSteps: 8,
contextOnlyBash: true,
});
const { trace, text } = result;
logTrace('S16', label, modelId, trace, { textSample: text.slice(0, 300) });
assert.ok(readsMatching(trace, 'reference/routing.md').length, 'workflow advice loads the shared routing reference');
assertAdviceOnly(trace, text);
t.diagnostic(`Reference coverage gaps (non-blocking): ${missingReferences(trace, ['reference/routing.md']).join(', ') || 'none'}`);
assertCompleted(result);
assertWorkflowAdvice(trace, text, { missingContext: label === 'missing product context' });
} finally {
cleanupWorkspace(workspace);
}
});
}
it('scenario 17: command comparison reads references without running them', async () => {
it('scenario 17: command comparison explains independent commands without running them', async (t) => {
const workspace = prepareWorkspace({ files: WORKFLOW_ADVICE_FILES });
try {
const { trace, text } = await runTurn({
const result = await runTurn({
workspace,
model,
userPrompt: 'Should I use critique or polish on index.html? Is a critique required before polishing?',
maxSteps: 8,
contextOnlyBash: true,
});
const { trace, text } = result;
logTrace('S17', 'command-comparison', modelId, trace, { textSample: text.slice(0, 300) });
assert.ok(readsMatching(trace, 'reference/routing.md').length, 'a command name in a question still routes to advice');
assert.ok(readsMatching(trace, 'reference/critique.md').length, 'comparison consults the critique contract');
assert.ok(readsMatching(trace, 'reference/polish.md').length, 'comparison consults the polish contract');
assertAdviceOnly(trace, text);
t.diagnostic(`Reference coverage gaps (non-blocking): ${missingReferences(trace, ['reference/routing.md', 'reference/critique.md', 'reference/polish.md']).join(', ') || 'none'}`);
assertCompleted(result);
assertCommandComparison(trace, text);
} finally {
cleanupWorkspace(workspace);
}
@@ -730,6 +743,7 @@ for (const modelId of resolveModelList()) {
workspace,
model,
userPrompt: '/impeccable polish index.html. Please do the polish pass now; afterward tell me which command would be useful next.',
checkpoint: 'polish.md',
maxSteps: 8,
contextOnlyBash: true,
});
@@ -1,273 +0,0 @@
/**
* Provider-backed workflow contract tests. Unlike scenarios.test.mjs, these
* assert the attended turns and writes that make init/redesign/refinement real.
*/
import { describe, it } from 'node:test';
import assert from 'node:assert/strict';
import fs from 'node:fs';
import path from 'node:path';
import {
prepareWorkspace,
cleanupWorkspace,
runTurn,
fileLoaded,
summarizeTrace,
ENGINE_BIN,
ENGINE_MISSING_MESSAGE,
} from './harness.mjs';
import { detectProvider, getModel, hasKey, resolveModelList, PROVIDERS } from './providers.mjs';
import { PRODUCT_MD_SAMPLE, DESIGN_MD_SAMPLE } from './fixtures.mjs';
const LEGACY_DESIGN = `# Design
## Identity
BORING_BEIGE_CARDS. Quiet beige panels, timid scale, rounded cards everywhere.
## Color
Warm gray background with a muted tan accent.
`;
const EXISTING_PAGE = `<!doctype html>
<html><head><style>
:root { --legacy-beige: #e8e1d5; --legacy-tan: #a78969; }
body { background: var(--legacy-beige); color: #3c3833; font-family: Arial, sans-serif; }
.card { border: 1px solid #cfc5b6; border-radius: 18px; padding: 24px; }
</style></head><body>
<header data-untouched="header"><a href="/">Harbor Desk</a></header>
<main><section id="case-study" class="card"><h1>Harbor Desk</h1><p>Challenge. Approach. Outcome.</p><p>Image placeholder</p></section></main>
<footer data-untouched="footer">Operational since 1987</footer>
</body></html>`;
// Deliberately broken enough that any honest critique lists three or more
// Priority Issues, so the run cannot reach the "fewer than 3" skip branch by
// merit. Low contrast, an icon-tile stack, a kicker over the heading, dead
// hierarchy, and a placeholder CTA.
const FLAWED_PAGE = `<!doctype html>
<html><head><style>
body { background:#f4f4f5; color:#b9b9c0; font-family: Arial, sans-serif; font-size:15px; }
h1, h2, h3, p { font-size:15px; font-weight:400; margin:8px 0; }
.tile { width:48px; height:48px; background:#e6e6ea; border-radius:12px; }
.card { border:1px solid #e6e6ea; border-radius:12px; padding:16px; }
</style></head><body>
<main>
<p class="kicker">INTRODUCING</p>
<h1>Harbor Desk</h1>
<p>A platform that helps teams do more of what matters, faster.</p>
<section class="card"><div class="tile"></div><h3>Lightning Fast</h3><p>Blazing performance.</p></section>
<section class="card"><div class="tile"></div><h3>Rock Solid</h3><p>Enterprise grade.</p></section>
<section class="card"><div class="tile"></div><h3>Fully Secure</h3><p>Bank level security.</p></section>
<button style="background:#e6e6ea;color:#c9c9d0;border:none;padding:8px 12px">Learn More</button>
</main>
</body></html>`;
/**
* Flatten assistant output into ordered parts.
*
* `generateText` only returns `text` for the FINAL step, which is empty when a
* turn ends on a tool call. Reading the report out of that field silently tests
* nothing. Walking responseMessages instead preserves emission order, which is
* the point: critique's invariant is that report prose precedes the question
* inside the message, since prose after a structured question is withheld until
* the user answers.
*/
function assistantParts(responseMessages) {
const parts = [];
for (const message of responseMessages) {
if (message.role !== 'assistant') continue;
const content = message.content;
if (typeof content === 'string') {
parts.push({ kind: 'text', value: content });
continue;
}
for (const part of content ?? []) {
if (part.type === 'text') parts.push({ kind: 'text', value: part.text ?? '' });
else if (part.type === 'tool-call') parts.push({ kind: 'tool', value: part.toolName ?? '' });
}
}
return parts;
}
function firstCall(trace, predicate) {
return trace.toolCalls.findIndex(predicate);
}
function firstMutation(trace, pattern) {
return firstCall(trace, ({ mutatedPaths = [] }) => mutatedPaths.some((file) => pattern.test(file)));
}
function workflowTraceMessage(trace) {
return JSON.stringify(summarizeTrace(trace), null, 2);
}
for (const modelId of resolveModelList()) {
const provider = detectProvider(modelId);
const keyPresent = hasKey(provider);
describe(`skill workflow contract :: ${modelId}`, () => {
if (!keyPresent) {
it(`skipped — ${PROVIDERS[provider].envKey} is unset`, { skip: true }, () => {});
return;
}
if (!ENGINE_BIN) {
it(`skipped — ${ENGINE_MISSING_MESSAGE}`, { skip: true }, () => {});
return;
}
const model = getModel(modelId);
it('fresh init asks and writes PRODUCT without inventing a visual system', async () => {
const workspace = prepareWorkspace({ files: {} });
try {
const { trace } = await runTurn({
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);
assert.ok(fileLoaded(trace, 'init.md'), `init.md was not loaded.\n${workflowTraceMessage(trace)}`);
assert.ok(question >= 0, `structured user was never asked.\n${workflowTraceMessage(trace)}`);
assert.ok(productWrite > question, `PRODUCT.md must follow a user answer.\n${workflowTraceMessage(trace)}`);
const product = fs.readFileSync(path.join(workspace, 'PRODUCT.md'), 'utf8');
assert.doesNotMatch(product, /^## Register\s*$/im);
assert.match(product, /ferry|dispatch|harbor/i, 'PRODUCT.md should incorporate the simulated user context');
assert.equal(fs.existsSync(path.join(workspace, 'DESIGN.md')), false, 'init must not create DESIGN.md');
} finally {
cleanupWorkspace(workspace);
}
});
it('an initialized natural build request asks for the task concept before implementation', async () => {
const workspace = prepareWorkspace({
files: { 'PRODUCT.md': PRODUCT_MD_SAMPLE, 'DESIGN.md': DESIGN_MD_SAMPLE },
});
try {
const { trace } = await runTurn({
workspace,
model,
userPrompt: '/impeccable create a concise evidence-led case-study page. Leave it at index.html.',
maxSteps: 22,
});
const question = firstCall(trace, ({ name }) => name === 'ask_user_question');
const implementation = firstMutation(trace, /\.(?:html?|astro|svelte|jsx?|tsx?)$/i);
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)}`);
assert.ok(implementation > question, `implementation began before the attended concept checkpoint.\n${workflowTraceMessage(trace)}`);
assert.equal(fs.existsSync(path.join(workspace, 'index.html')), true, 'new-work must still produce the requested artifact');
} finally {
cleanupWorkspace(workspace);
}
});
it('redesign replaces DESIGN before touching the existing page', async () => {
const workspace = prepareWorkspace({
files: {
'PRODUCT.md': PRODUCT_MD_SAMPLE,
'DESIGN.md': LEGACY_DESIGN,
'current.html': EXISTING_PAGE,
},
});
try {
const { trace } = await runTurn({
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');
const designWrite = firstMutation(trace, /(^|\/)DESIGN\.md$/i);
const implementation = firstMutation(trace, /(^|\/)current\.html$/i);
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)}`);
assert.ok(designWrite > question, `replacement DESIGN.md must follow user choice.\n${workflowTraceMessage(trace)}`);
assert.ok(implementation > designWrite, `redesign touched the page before replacing DESIGN.md.\n${workflowTraceMessage(trace)}`);
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 {
cleanupWorkspace(workspace);
}
});
it('bolder refinement preserves the world and everything outside scope', async () => {
const workspace = prepareWorkspace({
files: {
'PRODUCT.md': PRODUCT_MD_SAMPLE,
'DESIGN.md': DESIGN_MD_SAMPLE,
'current.html': EXISTING_PAGE,
},
});
try {
const { trace } = await runTurn({
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);
const implementation = firstMutation(trace, /(^|\/)current\.html$/i);
assert.ok(fileLoaded(trace, 'bolder.md'), `bolder.md was not loaded.\n${workflowTraceMessage(trace)}`);
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)}`);
const artifact = fs.readFileSync(path.join(workspace, 'current.html'), 'utf8');
assert.match(artifact, /data-untouched="header"/);
assert.match(artifact, /data-untouched="footer"/);
assert.match(artifact, /id="case-study"/);
} finally {
cleanupWorkspace(workspace);
}
});
// Regression guard for the failure mode that shipped in PR #576: the report
// landed and the run then stopped, asking nothing and printing no skip
// line. The close is the deliverable's other half, so a critique that ends
// on the report is incomplete. Asserted on the trace rather than on prose
// because the model's own account of why it skipped is not evidence.
it('critique closes with the question or an explicit skip line', async () => {
const workspace = prepareWorkspace({
files: {
'PRODUCT.md': PRODUCT_MD_SAMPLE,
'DESIGN.md': DESIGN_MD_SAMPLE,
'current.html': FLAWED_PAGE,
},
});
try {
const { trace, responseMessages } = await runTurn({
workspace,
model,
userPrompt: '/impeccable critique current.html',
maxSteps: 30,
});
assert.ok(fileLoaded(trace, 'critique.md'), `critique.md was not loaded.\n${workflowTraceMessage(trace)}`);
const parts = assistantParts(responseMessages);
const allText = parts.filter((p) => p.kind === 'text').map((p) => p.value).join('\n');
const reportPattern = /priority issue|heuristic|design health/i;
assert.match(allText, reportPattern, `no report reached the user.\n${workflowTraceMessage(trace)}`);
const askIndex = parts.findIndex((p) => p.kind === 'tool' && p.value === 'ask_user_question');
const skipped = /Questions skipped:/i.test(allText);
assert.ok(
askIndex >= 0 || skipped,
`critique ended without the questions and without a "Questions skipped: <reason>" line.\n` +
`This is the PR #576 regression: the report is not the finish, the close is.\n${workflowTraceMessage(trace)}`,
);
// The ordering invariant. Only meaningful when a question was actually
// asked; a skip-line close has nothing to order against.
if (askIndex >= 0) {
const reportIndex = parts.findIndex((p) => p.kind === 'text' && reportPattern.test(p.value));
assert.ok(
reportIndex >= 0 && reportIndex < askIndex,
`the question was emitted before the report text, so the report stays hidden until the user answers.\n` +
`${workflowTraceMessage(trace)}`,
);
}
} finally {
cleanupWorkspace(workspace);
}
});
});
}