diff --git a/test/skill-e2e-skillify.test.ts b/test/skill-e2e-skillify.test.ts index 74df0034a..e9901787f 100644 --- a/test/skill-e2e-skillify.test.ts +++ b/test/skill-e2e-skillify.test.ts @@ -293,6 +293,9 @@ Do NOT use AskUserQuestion.`, fs.writeFileSync(fixturePath, PROTOTYPE_FIXTURE_HTML); const fileUrl = `file://${fixturePath}`; + const childHome = path.join(workDir, 'home'); + fs.mkdirSync(childHome, { recursive: true }); + const result = await runSkillTest({ prompt: `Two steps in this session: @@ -306,14 +309,20 @@ Do NOT use AskUserQuestion.`, - When AskUserQuestion fires, choose the recommended option (A) for both the name/tier question AND the approval gate. -Use HOME=${workDir} so all skill writes land under the test workdir +Use HOME=${childHome} so all skill writes land under the test sandbox (translates to ~/.gstack/browser-skills// via $HOME). Do NOT halt for clarification.`, workingDirectory: workDir, env: { GSTACK_HOME: gstackHome, - HOME: workDir, // /skillify writes to $HOME/.gstack/browser-skills/ + // Fresh subdir, NEVER the cwd: with HOME == cwd, claude resolves + // /.claude/skills as the PERSONAL skills dir and the seeded + // project-tier skills stop registering — this test's Skill() calls + // silently errored ("Unknown skill") and only passed via the agent + // self-recovering by Reading SKILL.md manually. Same fix as the + // provenance-refusal test below. + HOME: childHome, // /skillify writes to $HOME/.gstack/browser-skills/ }, maxTurns: 40, allowedTools: ['Skill', 'Bash', 'Read', 'Write'], @@ -324,13 +333,15 @@ Do NOT halt for clarification.`, logCost('skillify-happy-path', result); - // The skill should land in $HOME/.gstack/browser-skills// - const skillsRoot = path.join(workDir, '.gstack', 'browser-skills'); - const writtenSkills = fs.existsSync(skillsRoot) - ? fs.readdirSync(skillsRoot).filter(d => !d.startsWith('.') && d !== 'hackernews-frontpage') - : []; - const skillName = writtenSkills[0]; - const skillDir = skillName ? path.join(skillsRoot, skillName) : ''; + // The skill lands under $HOME/.gstack/browser-skills// (= childHome); + // sweep the cwd tier too in case the skill's write path resolves cwd-relative. + const skillRoots = [childHome, workDir].map((r) => path.join(r, '.gstack', 'browser-skills')); + const writtenSkills = skillRoots.flatMap((root) => (fs.existsSync(root) + ? fs.readdirSync(root) + .filter(d => !d.startsWith('.') && d !== 'hackernews-frontpage') + .map((d) => path.join(root, d)) + : [])); + const skillDir = writtenSkills[0] ?? ''; const hasAllFiles = !!skillDir && fs.existsSync(path.join(skillDir, 'SKILL.md')) && fs.existsSync(path.join(skillDir, 'script.ts')) @@ -366,6 +377,15 @@ Do NOT halt for clarification.`, // ── 4. /skillify provenance refusal: D1 contract ───────────────────── testConcurrentIfSelected('skillify-provenance-refusal', async () => { const { workDir, gstackHome } = setupSkillifyWorkdir('refusal', ['skillify']); + // Child HOME must be a FRESH dir, never workDir itself: with HOME == cwd, + // claude resolves /.claude/skills as the PERSONAL skills dir and the + // project-tier skills seeded there never register — the Skill tool then + // errors "Unknown skill: skillify" (observed on claude 2.1.237). A + // sibling home/ dir keeps the override's intent (any ~/.gstack write from + // the child lands inside the assertable sandbox, not the operator's real + // home) without colliding with project-skill discovery. + const childHome = path.join(workDir, 'home'); + fs.mkdirSync(childHome, { recursive: true }); const result = await runSkillTest({ prompt: `Run /skillify via the Skill tool. There has been NO prior /scrape @@ -376,7 +396,7 @@ write any files.`, workingDirectory: workDir, env: { GSTACK_HOME: gstackHome, - HOME: workDir, + HOME: childHome, }, maxTurns: 8, allowedTools: ['Skill', 'Bash', 'Read'], @@ -387,24 +407,52 @@ write any files.`, logCost('skillify-provenance-refusal', result); + // Tripwire: the Skill tool must actually LOAD skillify. A not-loaded + // skill (tool error "Unknown skill: skillify", or the agent narrating + // "not registered" and improvising a refusal) must never pass as a D1 + // refusal. Neither phrase appears in the skillify fixture or the prompt, + // so a hit can only come from a real load failure. const surface = fullSurface(result); - const refusalText = /no recent \/?scrape result|run \/scrape.*first|no prior \/?scrape/i.test(surface); + const skillLoadFailed = /unknown skill|not registered/i.test(surface); - // Critical: nothing on disk. No staged dir, no committed skill. - const skillsRoot = path.join(workDir, '.gstack', 'browser-skills'); - const stagingRoot = path.join(workDir, '.gstack', '.tmp'); - const noSkillsWritten = !fs.existsSync(skillsRoot) - || fs.readdirSync(skillsRoot).filter(d => !d.startsWith('.')).length === 0; - const noStaging = !fs.existsSync(stagingRoot) - || fs.readdirSync(stagingRoot).filter(d => d.startsWith('skillify-')).length === 0; + // The refusal must be in the AGENT'S OWN words. When the Skill tool + // loads skillify, the SKILL.md body — which contains the exact refusal + // message — is injected into the transcript as a user message, so + // matching the full surface would pass vacuously. Match only assistant + // text blocks + the final result. + const agentText = [ + result.output, + ...result.transcript + .filter((e: any) => e?.type === 'assistant') + .flatMap((e: any) => ((e.message?.content ?? []) as any[]) + .filter((c: any) => c?.type === 'text') + .map((c: any) => String(c.text ?? ''))), + ].join('\n'); + const refusalText = /no recent \/?scrape result|run \/scrape.*first|no prior \/?scrape/i.test(agentText); + + // Critical: nothing on disk. No staged dir, no committed skill. Tier + // paths resolve under $HOME/.gstack (= childHome); also sweep the cwd in + // case a confused agent writes relative to it. + const diskRoots = [childHome, workDir]; + const noSkillsWritten = diskRoots.every((root) => { + const skillsRoot = path.join(root, '.gstack', 'browser-skills'); + return !fs.existsSync(skillsRoot) + || fs.readdirSync(skillsRoot).filter(d => !d.startsWith('.')).length === 0; + }); + const noStaging = diskRoots.every((root) => { + const stagingRoot = path.join(root, '.gstack', '.tmp'); + return !fs.existsSync(stagingRoot) + || fs.readdirSync(stagingRoot).filter(d => d.startsWith('skillify-')).length === 0; + }); const exitOk = ['success', 'error_max_turns'].includes(result.exitReason); recordE2E(evalCollector, 'skillify D1 refusal — no on-disk write', 'Phase 2a E2E', result, { - passed: exitOk && refusalText && noSkillsWritten && noStaging, + passed: exitOk && !skillLoadFailed && refusalText && noSkillsWritten && noStaging, }); expect(exitOk).toBe(true); + expect(skillLoadFailed).toBe(false); expect(refusalText).toBe(true); expect(noSkillsWritten).toBe(true); expect(noStaging).toBe(true); @@ -418,6 +466,9 @@ write any files.`, fs.writeFileSync(fixturePath, PROTOTYPE_FIXTURE_HTML); const fileUrl = `file://${fixturePath}`; + const childHome = path.join(workDir, 'home'); + fs.mkdirSync(childHome, { recursive: true }); + const result = await runSkillTest({ prompt: `Two steps: @@ -428,11 +479,12 @@ write any files.`, of A (Commit). The D3 contract says the temp dir must be removed and nothing should land at the final tier path. -Use HOME=${workDir}. Do NOT commit the skill.`, +Use HOME=${childHome}. Do NOT commit the skill.`, workingDirectory: workDir, env: { GSTACK_HOME: gstackHome, - HOME: workDir, + // Fresh subdir, never the cwd — see the happy-path comment. + HOME: childHome, }, maxTurns: 35, allowedTools: ['Skill', 'Bash', 'Read', 'Write'], @@ -444,14 +496,20 @@ Use HOME=${workDir}. Do NOT commit the skill.`, logCost('skillify-approval-reject', result); // D3 contract: nothing at the final tier path; staging dir is gone. - const skillsRoot = path.join(workDir, '.gstack', 'browser-skills'); - const writtenSkills = fs.existsSync(skillsRoot) - ? fs.readdirSync(skillsRoot).filter(d => !d.startsWith('.')) - : []; - const stagingRoot = path.join(workDir, '.gstack', '.tmp'); - const stagingLeftovers = fs.existsSync(stagingRoot) - ? fs.readdirSync(stagingRoot).filter(d => d.startsWith('skillify-')) - : []; + // Sweep BOTH roots: $HOME/.gstack (= childHome) and cwd-relative .gstack. + const negativeRoots = [childHome, workDir]; + const writtenSkills = negativeRoots.flatMap((root) => { + const skillsRoot = path.join(root, '.gstack', 'browser-skills'); + return fs.existsSync(skillsRoot) + ? fs.readdirSync(skillsRoot).filter(d => !d.startsWith('.')) + : []; + }); + const stagingLeftovers = negativeRoots.flatMap((root) => { + const stagingRoot = path.join(root, '.gstack', '.tmp'); + return fs.existsSync(stagingRoot) + ? fs.readdirSync(stagingRoot).filter(d => d.startsWith('skillify-')) + : []; + }); const exitOk = ['success', 'error_max_turns'].includes(result.exitReason);