From 044a04fd0dcaf3512ca125bff5d34ef66a7eb754 Mon Sep 17 00:00:00 2001 From: babylon1x Date: Sat, 5 Sep 2026 23:01:58 +0100 Subject: [PATCH] docs: add workflow guide for command entry points (#737) * docs: add workflow guide for command entry points * Refine workflow guidance into advice-only routing Reuse the existing routing reference and docs map instead of shipping a parallel workflow catalog. Add reference-backed command comparisons, advice-only tests, and explicit-command precedence coverage. AI-assisted maintainer revision prepared with Codex. * Include routing guidance in behavior-test triggers AI-assisted maintainer revision prepared with Codex. * Constrain routing behavior tests to fixture-safe tools Keep the real context loader but reject arbitrary host shell searches in the new advice scenarios. Preserve observable project writes and protect the staged skill; cover the restriction with offline regression tests. AI-assisted maintainer revision prepared with Codex. * Require actual reference reads in restricted routing tests Do not count rejected shell reads as reference loading. Record the nine measured advice cases; explicit-command measurements remain pending the stricter retest. AI-assisted maintainer revision prepared with Codex. * Record measured workflow-routing baseline All twelve focused cases pass across Claude Sonnet 5, GPT-5.6 Terra, and Gemini 3.7 Flash, including the stricter explicit-command retest. AI-assisted verification prepared with Codex. * Trim workflow routing guidance Reduce added skill prose from 286 to 59 words while retaining the routing regression assertions. Record the missing-context reference-read flake and passing repeat. AI assistance: prepared and verified with Codex under maintainer direction. --------- Co-authored-by: Paul Bakaus --- scripts/test-suites.mjs | 3 +- skill/SKILL.src.md | 3 +- skill/reference/routing.md | 8 ++- tests/skill-behavior-harness.test.mjs | 41 ++++++++++++++ tests/skill-behavior/README.md | 32 +++++++++++ tests/skill-behavior/fixtures.mjs | 8 +++ tests/skill-behavior/harness.mjs | 21 ++++++-- tests/skill-behavior/scenarios.test.mjs | 71 +++++++++++++++++++++++++ 8 files changed, 179 insertions(+), 8 deletions(-) create mode 100644 tests/skill-behavior-harness.test.mjs diff --git a/scripts/test-suites.mjs b/scripts/test-suites.mjs index 4ee177ec2..dc10248e7 100644 --- a/scripts/test-suites.mjs +++ b/scripts/test-suites.mjs @@ -73,6 +73,7 @@ export const SUITES = { 'tests/release.test.mjs', 'tests/bundle-signing.test.mjs', 'tests/skill-reference.test.mjs', + 'tests/skill-behavior-harness.test.mjs', 'tests/readme-gitignore.test.mjs', 'tests/test-suites.test.mjs', ], @@ -265,7 +266,7 @@ export const SUITES = { triggers: [ ...COMMON_INFRA_PATTERNS, /^skill\/SKILL\.src\.md$/, - /^skill\/reference\/(init|document|brand|product|shape|craft|audit|polish|live)\.md$/, + /^skill\/reference\/(init|document|brand|product|shape|craft|audit|polish|live|routing)\.md$/, /^ENGINE_VERSION$/, /^tests\/skill-behavior\//, ], diff --git a/skill/SKILL.src.md b/skill/SKILL.src.md index 7bae18f31..72d35f7c9 100644 --- a/skill/SKILL.src.md +++ b/skill/SKILL.src.md @@ -70,7 +70,8 @@ Choose the mode from the requested surface, not the product, and persist it only Routing: - **No argument:** read [routing.md](reference/routing.md) and present its context-aware menu; never auto-run a command. -- **Explicit or clearly implied command:** load its reference (native variant on native platforms) and follow it. Ask once if two commands fit. +- **Explicit or clearly implied request to run a command:** load its reference (native variant on native platforms) and follow it. Ask once if two commands fit. +- **Workflow or command-selection question:** read [Workflow questions](reference/routing.md#workflow-questions). - **Otherwise:** treat the request as general design work. Missing PRODUCT.md routes a new surface or replacement world through init, then new-work; a narrow refinement of existing code proceeds on the incumbent implementation as `impeccable context` directs, offering init afterward rather than blocking on it. - `teach` aliases `init`. `craft` is a deprecated alias for ordinary new-work and adds nothing. `shape` owns task discovery, then enters new-work only for visual-world and surface-concept decisions. diff --git a/skill/reference/routing.md b/skill/reference/routing.md index 210fdf521..7f68c8681 100644 --- a/skill/reference/routing.md +++ b/skill/reference/routing.md @@ -1,4 +1,10 @@ -# No-argument routing: the context-aware menu +# Command guidance + +## Workflow questions + +Give advice without executing commands; the menu below is only for bare invocations. Consult relevant command references as needed for prerequisites and scope. Link to the [docs](https://impeccable.style/docs/) for the broader workflow guide. If the user also requests execution, follow that request. + +## No-argument routing: the context-aware menu Read this when the user invokes `{{command_prefix}}impeccable` with no argument. They are asking "what should I do?" Make the menu context-aware instead of static. diff --git a/tests/skill-behavior-harness.test.mjs b/tests/skill-behavior-harness.test.mjs new file mode 100644 index 000000000..90b0235ac --- /dev/null +++ b/tests/skill-behavior-harness.test.mjs @@ -0,0 +1,41 @@ +import { it } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { prepareWorkspace, cleanupWorkspace, makeTools } from './skill-behavior/harness.mjs'; + +it('context-only routing tools reject shell searches and compound commands before execution', async () => { + const workspace = prepareWorkspace({ files: { 'index.html': 'before' } }); + try { + const { tools, trace } = makeTools(workspace, {}, {}, { contextOnlyBash: true }); + for (const command of [ + 'find / -name routing.md', + '.claude/skills/impeccable/scripts/impeccable context; echo bad > index.html', + 'echo bad > index.html', + ]) { + assert.match(await tools.bash.execute({ command }), /^Error:/); + } + assert.equal(fs.readFileSync(path.join(workspace, 'index.html'), 'utf8'), 'before'); + assert.equal(trace.bashCommands.length, 3, 'rejected attempts remain observable'); + assert.ok(trace.toolCalls.every((call) => call.mutatedPaths.length === 0)); + } finally { + cleanupWorkspace(workspace); + } +}); + +it('context-only routing tools keep project writes observable but protect the staged skill', async () => { + const workspace = prepareWorkspace({ files: { 'index.html': 'before' } }); + try { + const { tools, trace } = makeTools(workspace, {}, {}, { contextOnlyBash: true }); + const skillPath = '.claude/skills/impeccable/reference/routing.md'; + const before = await tools.read.execute({ path: skillPath }); + assert.match(await tools.write.execute({ path: skillPath, contents: 'bad' }), /^Error:/); + assert.equal(await tools.read.execute({ path: skillPath }), before); + await tools.write.execute({ path: 'index.html', contents: 'after' }); + assert.equal(fs.readFileSync(path.join(workspace, 'index.html'), 'utf8'), 'after'); + assert.deepEqual(trace.writePaths, [skillPath, 'index.html']); + assert.deepEqual(trace.toolCalls.flatMap((call) => call.mutatedPaths), ['index.html']); + } finally { + cleanupWorkspace(workspace); + } +}); diff --git a/tests/skill-behavior/README.md b/tests/skill-behavior/README.md index 463fa0756..9db7678cd 100644 --- a/tests/skill-behavior/README.md +++ b/tests/skill-behavior/README.md @@ -72,6 +72,38 @@ 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 | +| 18 | existing surface; explicitly requests polish followed by a next-command recommendation | loads `polish.md` rather than substituting workflow advice for the requested work | + +## Workflow-advice baseline (2026-09-05, PR #737) + +The four cases in scenarios 16-18 are new; prior scenario results do not +establish their behavior. The advice-only assertions inspect write-tool calls +and file mutations from bash (excluding context's internal `.impeccable/` +state, but not critique reports), require an actual answer, and reject +interviews and menu scans. Scenario 18 checks command-reference precedence; +it does not assert completion of a full polish pass. These cases allow only +the exact context-loader command through bash; references use read/list, and +the write tool remains available for project files so unsolicited edits still +fail the test. Writes to the staged skill are rejected. This keeps the routing +measurement from running arbitrary shell searches outside its fixture. + +| Scenario | claude-sonnet-5 | gpt-5.6-terra | gemini-3.7-flash | +|---|---|---|---| +| 16 (existing project) | pass | pass | pass | +| 16 (missing product context) | flaky (1 of 2) | pass | pass | +| 17 (command comparison) | pass | pass | pass | +| 18 (explicit command) | pass | pass | pass | + +Rechecked after reducing the skill addition to 59 words. Claude's first +missing-context response stayed read-only but skipped `routing.md`; an unchanged +repeat passed on all three providers. Keep that reference-read miss visible as +a flake rather than adding instructions for a single observation. All assertions +remain unchanged. Scenario 18 uses an eight-step budget and actual read-tool +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, an initialized natural build request, replacement-world redesign, scope-preserving bolder diff --git a/tests/skill-behavior/fixtures.mjs b/tests/skill-behavior/fixtures.mjs index 429d70950..610f5c7a2 100644 --- a/tests/skill-behavior/fixtures.mjs +++ b/tests/skill-behavior/fixtures.mjs @@ -335,3 +335,11 @@ separates major regions. No drop shadows under 16px blur. - Cards: avoid; prefer hairlined regions and inline lists. - Forms: floating labels, no border on the input — underline only. `; + +// A real surface keeps workflow advice and explicit-command precedence tests +// answerable without requiring the model to invent a project. +export const WORKFLOW_ADVICE_FILES = { + 'PRODUCT.md': PRODUCT_MD_SAMPLE, + 'DESIGN.md': DESIGN_MD_SAMPLE, + 'index.html': MINIMAL_LANDING_HTML, +}; diff --git a/tests/skill-behavior/harness.mjs b/tests/skill-behavior/harness.mjs index 2bcca2b9b..224226abe 100644 --- a/tests/skill-behavior/harness.mjs +++ b/tests/skill-behavior/harness.mjs @@ -219,7 +219,7 @@ function defaultSimulatedAnswer(question) { return 'Use the brief, preserve real operational content, and make the primary decision obvious.'; } -export function makeTools(workspace, extraEnv = {}, simulatedUser = {}) { +export function makeTools(workspace, extraEnv = {}, simulatedUser = {}, { contextOnlyBash = false } = {}) { const trace = { toolCalls: [], bashCommands: [], @@ -242,13 +242,21 @@ export function makeTools(workspace, extraEnv = {}, simulatedUser = {}) { } const tools = { bash: tool({ - description: - 'Run a bash command in the workspace root. Use this to invoke skill commands (e.g. `.claude/skills/impeccable/scripts/impeccable context`).', + 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.' + : '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.'), }), execute: async ({ command }) => { const call = record('bash', { command }); + // 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/.'; + 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)); @@ -284,6 +292,9 @@ export function makeTools(workspace, extraEnv = {}, simulatedUser = {}) { const call = record('write', { path: p, contents }); const resolved = safeResolve(workspace, p); if (typeof resolved !== 'string') return `Error: ${resolved.error}`; + if (contextOnlyBash && path.relative(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 }); fs.writeFileSync(resolved, contents); call.mutatedPaths = [p]; @@ -359,8 +370,8 @@ export function makeTools(workspace, extraEnv = {}, simulatedUser = {}) { // 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 }) { - const { tools, trace } = makeTools(workspace, env, simulatedUser); +export async function runTurn({ workspace, model, userPrompt, priorMessages = [], maxSteps = 8, env = {}, simulatedUser = {}, timeoutMs = TURN_TIMEOUT_MS, contextOnlyBash = false }) { + const { tools, trace } = makeTools(workspace, env, simulatedUser, { contextOnlyBash }); const messages = [ ...priorMessages, { role: 'user', content: userPrompt }, diff --git a/tests/skill-behavior/scenarios.test.mjs b/tests/skill-behavior/scenarios.test.mjs index 5b22eb883..2607ed849 100644 --- a/tests/skill-behavior/scenarios.test.mjs +++ b/tests/skill-behavior/scenarios.test.mjs @@ -34,6 +34,7 @@ import { MINIMAL_IOS_SOURCE, DESIGN_MD_SAMPLE, MINIMAL_LANDING_HTML, + WORKFLOW_ADVICE_FILES, SVELTE_PROJECT_FILES, } from './fixtures.mjs'; @@ -79,6 +80,16 @@ 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); @@ -597,5 +608,65 @@ for (const modelId of resolveModelList()) { cleanupWorkspace(workspace); } }); + + for (const [label, files] of [ + ['existing project', WORKFLOW_ADVICE_FILES], + ['missing product context', { 'index.html': MINIMAL_LANDING_HTML }], + ]) { + it(`scenario 16: workflow advice stays read-only (${label})`, async () => { + const workspace = prepareWorkspace({ files }); + try { + const { trace, text } = await runTurn({ + workspace, + model, + userPrompt: "I'm joining this project. Where should I start with Impeccable?", + maxSteps: 8, + contextOnlyBash: true, + }); + 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); + } finally { + cleanupWorkspace(workspace); + } + }); + } + + it('scenario 17: command comparison reads references without running them', async () => { + const workspace = prepareWorkspace({ files: WORKFLOW_ADVICE_FILES }); + try { + const { trace, text } = await runTurn({ + workspace, + model, + userPrompt: 'Should I use critique or polish on index.html? Is a critique required before polishing?', + maxSteps: 8, + contextOnlyBash: true, + }); + 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); + } finally { + cleanupWorkspace(workspace); + } + }); + + it('scenario 18: explicit command request takes precedence over workflow advice', async () => { + const workspace = prepareWorkspace({ files: WORKFLOW_ADVICE_FILES }); + try { + const { trace, text } = await runTurn({ + workspace, + model, + userPrompt: '/impeccable polish index.html. Please do the polish pass now; afterward tell me which command would be useful next.', + maxSteps: 8, + contextOnlyBash: true, + }); + logTrace('S18', 'explicit-command', modelId, trace, { textSample: text.slice(0, 300) }); + assert.ok(readsMatching(trace, 'reference/polish.md').length, 'the requested command must not be replaced with advice'); + } finally { + cleanupWorkspace(workspace); + } + }); }); }