From 733e39abae967a73b4208c16bdc34c45f16adf6a Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Mon, 7 Sep 2026 19:17:24 -0700 Subject: [PATCH] Fix workflow artifact and target syntax review gaps Keep cross-family protocol coverage separate from the manually opted-in full workflow diagnostic lane. AI assistance: Codex, under maintainer direction. --- .github/workflows/ci.yml | 2 ++ tests/ci-test-plan.test.mjs | 3 +++ tests/skill-behavior-harness.test.mjs | 12 ++++++++---- tests/skill-behavior/harness.mjs | 2 +- tests/skill-workflow/full-build.test.mjs | 3 ++- 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 80c3c8b15..d8c84e8e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -677,6 +677,8 @@ jobs: run: bun run test:skill-behavior skill-workflow: + # Expensive, manually opted-in completion diagnostics. This does not + # replace the multi-family skill-behavior protocol suite above. runs-on: ubuntu-latest if: github.event_name == 'workflow_dispatch' && inputs.skill_workflow timeout-minutes: 70 diff --git a/tests/ci-test-plan.test.mjs b/tests/ci-test-plan.test.mjs index 75fa6e8e1..670c135fb 100644 --- a/tests/ci-test-plan.test.mjs +++ b/tests/ci-test-plan.test.mjs @@ -18,6 +18,9 @@ describe('ci-test-plan', () => { 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/); + assert.doesNotMatch(protocol, /IMPECCABLE_SKILL_BEHAVIOR_MODELS:/, 'protocol coverage must retain the multi-family defaults'); + assert.match(protocol, /GOOGLE_CLOUD_API_KEY:/); + assert.match(protocol, /ANTHROPIC_API_KEY:/); }); it('keeps docs-only pull requests on the core suite', () => { const outputs = runPlan({ diff --git a/tests/skill-behavior-harness.test.mjs b/tests/skill-behavior-harness.test.mjs index 8192655ab..f36f6a536 100644 --- a/tests/skill-behavior-harness.test.mjs +++ b/tests/skill-behavior-harness.test.mjs @@ -353,12 +353,14 @@ it('context-only routing tools reject shell searches and compound commands befor '.claude/skills/impeccable/scripts/impeccable context; echo bad > index.html', '.claude/skills/impeccable/scripts/impeccable context --target index.html; echo bad > index.html', '.claude/skills/impeccable/scripts/impeccable context --target "$(echo bad > index.html)"', + '.claude/skills/impeccable/scripts/impeccable context --target=index.html; echo bad > index.html', + '.claude/skills/impeccable/scripts/impeccable context --target="$(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, 5, 'rejected attempts remain observable'); + assert.equal(trace.bashCommands.length, 7, 'rejected attempts remain observable'); assert.ok(trace.toolCalls.every((call) => call.mutatedPaths.length === 0)); } finally { cleanupWorkspace(workspace); @@ -370,10 +372,12 @@ 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/); + for (const separator of [' ', '=']) { + for (const target of ['index.html', 'src/routes/+page.svelte', '"src/routes/+page.svelte"', '"my page.html"', "'my page.html'"]) { + assert.match(await tools.bash.execute({ command: `.claude/skills/impeccable/scripts/impeccable context --target${separator}${target}` }), /^exit=0\n/); + } + assert.match(await tools.bash.execute({ command: `.claude/skills/impeccable/scripts/impeccable context --target${separator}../outside.html` }), /^Error:/); } - 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/harness.mjs b/tests/skill-behavior/harness.mjs index 84f8d86d1..aa24cc5b9 100644 --- a/tests/skill-behavior/harness.mjs +++ b/tests/skill-behavior/harness.mjs @@ -172,7 +172,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'); diff --git a/tests/skill-workflow/full-build.test.mjs b/tests/skill-workflow/full-build.test.mjs index 1c2d1282b..9c3ab5b72 100644 --- a/tests/skill-workflow/full-build.test.mjs +++ b/tests/skill-workflow/full-build.test.mjs @@ -20,7 +20,7 @@ import { detectProvider, getModel, hasKey, resolveModelList, PROVIDERS } from '. 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'; +import { assertCompleted, assertFreshCaptures, assertDocumentationArtifacts } 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'); @@ -211,6 +211,7 @@ for (const modelId of process.env.IMPECCABLE_SKILL_BEHAVIOR_MODELS ? resolveMode 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'); + assertDocumentationArtifacts(design, fs.readFileSync(path.join(workspace, '.impeccable/design.json'), 'utf8')); } finally { cleanupWorkspace(workspace); }