mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-06 13:45:35 +02:00
Merge remote-tracking branch 'origin/main' into garrytan/merge-community-prs
# Conflicts: # package.json
This commit is contained in:
@@ -263,6 +263,43 @@ describe('gen-skill-docs', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('bash blocks with shell globs are zsh-safe (setopt guard or find)', () => {
|
||||
for (const skill of ALL_SKILLS) {
|
||||
const content = fs.readFileSync(path.join(ROOT, skill.dir, 'SKILL.md'), 'utf-8');
|
||||
const bashBlocks = [...content.matchAll(/```bash\n([\s\S]*?)```/g)].map(m => m[1]);
|
||||
|
||||
for (const block of bashBlocks) {
|
||||
const lines = block.split('\n');
|
||||
|
||||
for (const line of lines) {
|
||||
const trimmed = line.trimStart();
|
||||
if (trimmed.startsWith('#')) continue;
|
||||
if (!trimmed.includes('*')) continue;
|
||||
// Skip lines where * is inside find -name, git pathspecs, or $(find)
|
||||
if (/\bfind\b/.test(trimmed)) continue;
|
||||
if (/\bgit\b/.test(trimmed)) continue;
|
||||
if (/\$\(find\b/.test(trimmed)) continue;
|
||||
|
||||
// Check 1: "for VAR in <glob>" must use $(find ...) — caught above by the
|
||||
// $(find check, so any surviving for-in with a glob pattern is a violation
|
||||
if (/\bfor\s+\w+\s+in\b/.test(trimmed) && /\*\./.test(trimmed)) {
|
||||
throw new Error(
|
||||
`Unsafe for-in glob in ${skill.dir}/SKILL.md: "${trimmed}". ` +
|
||||
`Use \`for f in $(find ... -name '*.ext')\` for zsh compatibility.`
|
||||
);
|
||||
}
|
||||
|
||||
// Check 2: ls/cat/rm/grep with glob file args must have setopt guard
|
||||
const isGlobCmd = /\b(?:ls|cat|rm|grep)\b/.test(trimmed) &&
|
||||
/(?:\/\*[a-z.*]|\*\.[a-z])/.test(trimmed);
|
||||
if (isGlobCmd) {
|
||||
expect(block).toContain('setopt +o nomatch');
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('preamble-using skills have correct skill name in telemetry', () => {
|
||||
const PREAMBLE_SKILLS = [
|
||||
{ dir: '.', name: 'gstack' },
|
||||
@@ -1721,3 +1758,91 @@ describe('telemetry', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('codex commands must not use inline $(git rev-parse --show-toplevel) for cwd', () => {
|
||||
// Regression test: inline $(git rev-parse --show-toplevel) in codex exec -C
|
||||
// or codex review without cd evaluates in whatever cwd the background shell
|
||||
// inherits, which may be a different project in Conductor workspaces.
|
||||
// The fix is to resolve _REPO_ROOT eagerly at the top of each bash block.
|
||||
|
||||
// Scan all source files that could contain codex commands
|
||||
// Use Bun.Glob to avoid ELOOP from .claude/skills/gstack symlink back to ROOT
|
||||
const tmplGlob = new Bun.Glob('**/*.tmpl');
|
||||
const sourceFiles = [
|
||||
...Array.from(tmplGlob.scanSync({ cwd: ROOT, followSymlinks: false })),
|
||||
...fs.readdirSync(path.join(ROOT, 'scripts/resolvers'))
|
||||
.filter(f => f.endsWith('.ts'))
|
||||
.map(f => `scripts/resolvers/${f}`),
|
||||
'scripts/gen-skill-docs.ts',
|
||||
];
|
||||
|
||||
test('no codex exec command uses inline $(git rev-parse --show-toplevel) in -C flag', () => {
|
||||
const violations: string[] = [];
|
||||
for (const rel of sourceFiles) {
|
||||
const abs = path.join(ROOT, rel);
|
||||
if (!fs.existsSync(abs)) continue;
|
||||
const content = fs.readFileSync(abs, 'utf-8');
|
||||
const lines = content.split('\n');
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
if (line.includes('codex exec') && line.includes('-C') && line.includes('$(git rev-parse --show-toplevel)')) {
|
||||
violations.push(`${rel}:${i + 1}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
expect(violations).toEqual([]);
|
||||
});
|
||||
|
||||
test('no generated SKILL.md has codex exec with inline $(git rev-parse --show-toplevel) in -C flag', () => {
|
||||
const violations: string[] = [];
|
||||
const skillMdGlob = new Bun.Glob('**/SKILL.md');
|
||||
const skillMdFiles = Array.from(skillMdGlob.scanSync({ cwd: ROOT, followSymlinks: false }));
|
||||
for (const rel of skillMdFiles) {
|
||||
const abs = path.join(ROOT, rel);
|
||||
if (!fs.existsSync(abs)) continue;
|
||||
const content = fs.readFileSync(abs, 'utf-8');
|
||||
const lines = content.split('\n');
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
if (line.includes('codex exec') && line.includes('-C') && line.includes('$(git rev-parse --show-toplevel)')) {
|
||||
violations.push(`${rel}:${i + 1}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
expect(violations).toEqual([]);
|
||||
});
|
||||
|
||||
test('codex review commands must be preceded by cd "$_REPO_ROOT" (no -C support)', () => {
|
||||
// codex review does not support -C, so the pattern must be:
|
||||
// _REPO_ROOT=$(git rev-parse --show-toplevel) || { ... }
|
||||
// cd "$_REPO_ROOT"
|
||||
// codex review ...
|
||||
// NOT: codex review ... with inline $(git rev-parse --show-toplevel)
|
||||
const allFiles = [
|
||||
...Array.from(tmplGlob.scanSync({ cwd: ROOT, followSymlinks: false })),
|
||||
...Array.from(new Bun.Glob('**/SKILL.md').scanSync({ cwd: ROOT, followSymlinks: false })),
|
||||
...fs.readdirSync(path.join(ROOT, 'scripts/resolvers'))
|
||||
.filter(f => f.endsWith('.ts'))
|
||||
.map(f => `scripts/resolvers/${f}`),
|
||||
'scripts/gen-skill-docs.ts',
|
||||
];
|
||||
const violations: string[] = [];
|
||||
for (const rel of allFiles) {
|
||||
const abs = path.join(ROOT, rel);
|
||||
if (!fs.existsSync(abs)) continue;
|
||||
const content = fs.readFileSync(abs, 'utf-8');
|
||||
const lines = content.split('\n');
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
// Skip non-executable lines (markdown table cells, prose references)
|
||||
if (line.includes('|') && line.includes('`/codex review`')) continue;
|
||||
if (line.includes('`codex review`')) continue;
|
||||
// Check for codex review with inline $(git rev-parse)
|
||||
if (line.includes('codex review') && line.includes('$(git rev-parse --show-toplevel)')) {
|
||||
violations.push(`${rel}:${i + 1} — inline git rev-parse in codex review`);
|
||||
}
|
||||
}
|
||||
}
|
||||
expect(violations).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user