mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 22:45:02 +01:00
test: host path-leak checks read the moved shared preamble too; name SHARED_PREAMBLE_FILE (ship review pass 2)
The external-host path-leak assertions in gen-skill-docs.test.ts (Codex, Factory) and host-config.test.ts (Copilot, Antigravity) read SKILL.md alone, so a Claude path that moved into sections/preamble.md with the hoist went unchecked. They now read the skill's startup text through readSkillStartup. workflow-judge-input and host-conformance use the SHARED_PREAMBLE_FILE constant instead of a bare 'preamble.md' literal, and preamble.ts's header describes the hoisted composition. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
700ace70f3
commit
92d6045f99
5 files changed
+20
-14
No files matched your search
@@ -2,11 +2,15 @@
|
||||
* Preamble composition root.
|
||||
*
|
||||
* Each generator lives in its own file under ./preamble/*.ts. This file only
|
||||
* wires them together via generatePreamble(). Keep composition declarative —
|
||||
* no inline logic beyond tier gating.
|
||||
* wires them together: preambleSections() is the composition, and
|
||||
* generatePreamble() joins it into the full inline render (tier 1 skills and
|
||||
* the tests' oracle). For tier 2+ skills, {{PREAMBLE}} resolves through
|
||||
* ./preamble-hoist.ts, which splits the same sections into a per-skill pointer
|
||||
* plus the shared sections/preamble.md the skill Reads on load. Keep
|
||||
* composition declarative — no inline logic beyond tier gating.
|
||||
*
|
||||
* Each skill runs independently via `claude -p` (or the host's equivalent).
|
||||
* There is no shared loader. The preamble provides: update checks, session
|
||||
* The preamble provides: update checks, session
|
||||
* tracking, user preferences, repo mode detection, model overlays, and
|
||||
* telemetry.
|
||||
*
|
||||
|
||||
@@ -3131,9 +3131,10 @@ describe('Codex generation (--host codex)', () => {
|
||||
// Verify across ALL generated skills, not just review
|
||||
for (const skill of CODEX_SKILLS) {
|
||||
const content = fs.readFileSync(path.join(AGENTS_DIR, skill.codexName, 'SKILL.md'), 'utf-8');
|
||||
// No skill should reference Claude paths
|
||||
expect(content).not.toContain('~/.claude/skills');
|
||||
expect(content).not.toContain('.claude/skills');
|
||||
// No skill should reference Claude paths, including in its shared sections/preamble.md
|
||||
const startup = readSkillStartup(path.join(AGENTS_DIR, skill.codexName, 'SKILL.md'));
|
||||
expect(startup).not.toContain('~/.claude/skills');
|
||||
expect(startup).not.toContain('.claude/skills');
|
||||
if (content.includes('gstack-config') || content.includes('gstack-update-check') || content.includes('gstack-telemetry-log')) {
|
||||
expect(content).toContain('$GSTACK_ROOT');
|
||||
}
|
||||
@@ -3335,14 +3336,14 @@ describe('Factory generation (--host factory)', () => {
|
||||
|
||||
test('no .claude/skills/ in Factory output', () => {
|
||||
for (const skill of FACTORY_SKILLS) {
|
||||
const content = fs.readFileSync(path.join(FACTORY_DIR, skill.factoryName, 'SKILL.md'), 'utf-8');
|
||||
const content = readSkillStartup(path.join(FACTORY_DIR, skill.factoryName, 'SKILL.md'));
|
||||
expect(content).not.toContain('.claude/skills');
|
||||
}
|
||||
});
|
||||
|
||||
test('no ~/.claude/skills/ paths in Factory output', () => {
|
||||
for (const skill of FACTORY_SKILLS) {
|
||||
const content = fs.readFileSync(path.join(FACTORY_DIR, skill.factoryName, 'SKILL.md'), 'utf-8');
|
||||
const content = readSkillStartup(path.join(FACTORY_DIR, skill.factoryName, 'SKILL.md'));
|
||||
// ~/.claude/skills should be rewritten, but ~/.claude/plans is legitimate
|
||||
// (plan directory lookup) and ~/.claude/ in codex prompts is intentional
|
||||
expect(content).not.toContain('~/.claude/skills');
|
||||
|
||||
@@ -29,6 +29,7 @@ import { HOST_PATHS } from '../scripts/resolvers/types';
|
||||
import { RESOLVERS } from '../scripts/resolvers';
|
||||
import { expectTokens } from './helpers/prompt-structure';
|
||||
import { runGeneration } from '../scripts/gen-skill-docs';
|
||||
import { readSkillStartup } from './helpers/skill-startup';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const RESOLVER_NAMES = new Set(Object.keys(RESOLVERS));
|
||||
@@ -746,7 +747,7 @@ describe('Copilot host render (#393)', () => {
|
||||
|
||||
test('paths name the Copilot install, never Claude or Codex', () => {
|
||||
for (const skill of ['gstack-review', 'gstack-ship', 'gstack-careful', 'gstack-upgrade']) {
|
||||
const md = read(skill);
|
||||
const md = readSkillStartup(path.join(OUT, '.copilot', 'skills', skill, 'SKILL.md'));
|
||||
expect(md).not.toContain('~/.claude/skills/gstack');
|
||||
expect(md).not.toContain('$HOME/.claude/skills/gstack');
|
||||
expect(md).not.toContain('.codex/skills');
|
||||
@@ -779,7 +780,7 @@ describe('Antigravity CLI host render', () => {
|
||||
|
||||
test('paths name the Antigravity CLI install, never Claude, Codex or Gemini CLI skill dirs', () => {
|
||||
for (const skill of ['gstack-review', 'gstack-ship', 'gstack-careful', 'gstack-upgrade']) {
|
||||
const md = read(skill);
|
||||
const md = readSkillStartup(path.join(dir, skill, 'SKILL.md'));
|
||||
expect(md).not.toContain('~/.claude/skills/gstack');
|
||||
expect(md).not.toContain('$HOME/.claude/skills/gstack');
|
||||
expect(md).not.toContain('.codex/skills');
|
||||
|
||||
@@ -16,7 +16,7 @@ import { join } from 'node:path';
|
||||
import { ALL_HOST_CONFIGS } from '../hosts/index';
|
||||
import { cleanupFixtures, cleanupSeed, makeFixture, makeSource, put, registryRows, runSetup, setVersion, tree } from './helpers/install-fixture';
|
||||
import { expectTokens } from './helpers/prompt-structure';
|
||||
import { readSkillStartup } from './helpers/skill-startup';
|
||||
import { readSkillStartup, SHARED_PREAMBLE_FILE } from './helpers/skill-startup';
|
||||
|
||||
afterEach(cleanupFixtures);
|
||||
afterAll(cleanupSeed);
|
||||
@@ -81,7 +81,7 @@ function brokenSectionPointers(dest: string, runtimeRoot: string, home: string):
|
||||
let targets: string[];
|
||||
if (tok.startsWith('sections/')) {
|
||||
if (names.length) targets = names.map(n => join(dest, n, tok));
|
||||
else if ((existsSync(join(dir, 'sections')) && readdirSync(join(dir, 'sections'), { withFileTypes: true }).some(e => e.name.endsWith('.md') && e.name !== 'preamble.md')) || stop) targets = [join(dir, tok)];
|
||||
else if ((existsSync(join(dir, 'sections')) && readdirSync(join(dir, 'sections'), { withFileTypes: true }).some(e => e.name.endsWith('.md') && e.name !== SHARED_PREAMBLE_FILE)) || stop) targets = [join(dir, tok)];
|
||||
else continue;
|
||||
} else if (tok.startsWith('$GSTACK_ROOT/')) targets = [join(runtimeRoot, tok.slice('$GSTACK_ROOT/'.length))];
|
||||
else if (tok.startsWith('~/')) targets = [join(home, tok.slice(2))];
|
||||
|
||||
@@ -6,7 +6,7 @@ import { dirname, join, resolve } from 'node:path';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { readWorkflowJudgeInput, buildWorkflowJudgePrompt, QA_DISCOVERY_REFERENCES, WORKFLOW_JUDGE_RESPONSE_SCHEMA, WORKFLOW_JUDGE_REASONING_WORD_LIMIT } from './helpers/workflow-judge-input';
|
||||
import { ASK_QUESTIONS_HEADING, ENG_REVIEW_EXCERPT } from './helpers/workflow-excerpt';
|
||||
import { readSkillStartup } from './helpers/skill-startup';
|
||||
import { readSkillStartup, SHARED_PREAMBLE_FILE } from './helpers/skill-startup';
|
||||
|
||||
const ROOT = resolve(import.meta.dir, '..');
|
||||
const scratchRoots: string[] = [];
|
||||
@@ -79,7 +79,7 @@ function sectionPaths(skill: string): string[] {
|
||||
const runnerOnly = new Set((JSON.parse(readFileSync(join(ROOT, skill, 'sections', 'manifest.json'), 'utf8')).sections as Array<{ file: string; runner_only?: boolean }>)
|
||||
.filter(section => section.runner_only).map(section => section.file));
|
||||
return readdirSync(join(ROOT, skill, 'sections'))
|
||||
.filter(name => name.endsWith('.md') && !runnerOnly.has(name) && name !== 'preamble.md')
|
||||
.filter(name => name.endsWith('.md') && !runnerOnly.has(name) && name !== SHARED_PREAMBLE_FILE)
|
||||
.sort()
|
||||
.map(name => `${skill}/sections/${name}`);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user