mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(test): skillify family — HOME==cwd broke project-skill registration
Root cause (forensically pinned from stream-json init events + a
kill-after-init probe): with HOME set EQUAL to the child's cwd, claude
resolves <cwd>/.claude/skills as the PERSONAL skills directory and the
seeded project-tier skills never register — the Skill tool returned
'Unknown skill'. The provenance-refusal test then improvised a refusal
whose wording missed the regex (the deterministic CI+local red); the
happy-path and approval-reject siblings passed only because their
agents self-recovered by Reading SKILL.md manually — silently not
exercising the Skill-tool path at all.
All three tests now use HOME=<workDir>/home (a fresh subdir keeps the
override's intent: child ~/.gstack writes land in the assertable
sandbox, without the cwd collision). Refusal test additionally: a
'not registered/unknown skill' tripwire (a not-loaded skill can never
pass as a refusal) and the refusal regex now matches assistant text
only — the skill BODY echoed into the transcript contains the exact
refusal message, so the old full-surface match could pass vacuously
once the skill loaded. Sibling disk assertions sweep both $HOME/.gstack
and cwd .gstack roots (positives and negatives).
Verified paid: refusal 2x consecutive green with the skill's EXACT
message rendered ('Launching skill: skillify' in-transcript), then the
full file 5/5 green (~$1.35) with both siblings driving real Skill
calls (25-27 turns each).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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/<name>/ 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
|
||||
// <cwd>/.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/<name>/
|
||||
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/<name>/ (= 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 <cwd>/.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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user