CalvinBackup/gstack
1
0
Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-05-02 03:35:09 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: resolve codex exec -C repo root eagerly to prevent wrong-project reviews (v0.12.6.0) (#549)

Browse Source 
* refactor: remove 6 dead resolver function copies from gen-skill-docs.ts

These functions were moved to scripts/resolvers/{review,design}.ts but the
old copies in gen-skill-docs.ts were never deleted. They are defined but
never called — the RESOLVERS map from resolvers/index.ts is the live
dispatch. The dead copies had already diverged from the live versions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: resolve codex exec -C repo root eagerly to prevent wrong-project reviews

When codex exec commands run in background bash tasks (e.g., Conductor
workspaces), $(git rev-parse --show-toplevel) evaluates in whatever cwd
the background shell inherits, which may be a different project. Fix by
resolving _REPO_ROOT once at the top of each bash block and referencing
the stored value in -C.

12 occurrences fixed across 4 source files:
- codex/SKILL.md.tmpl (3)
- autoplan/SKILL.md.tmpl (3)
- scripts/resolvers/review.ts (3)
- scripts/resolvers/design.ts (3)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: regression guard for codex exec inline git rev-parse in -C flag

Scans all .tmpl and resolver .ts source files for codex exec commands
that use inline $(git rev-parse --show-toplevel) in the -C flag. This
pattern causes wrong-project reviews in Conductor workspaces. The test
ensures nobody reintroduces the old pattern.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: bump version and changelog (v0.12.6.0)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address adversarial review findings — codex review cwd, test scope, fail-loud

1. codex review commands now cd to $_REPO_ROOT (review doesn't support -C)
2. Autoplan codex commands converted from prose "Prerequisite" to fenced bash blocks
3. || pwd fallback replaced with hard fail — silent wrong-dir is worse than error
4. Regression test now scans all resolver .ts files + generated SKILL.md files

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: harden regression test — Bun.Glob, SKILL.md scan, codex review check

Fixes three gaps found by adversarial review:
1. fs.readdirSync recursive hits ELOOP on .claude/skills/gstack symlink.
   Switched to Bun.Glob with followSymlinks:false.
2. Generated SKILL.md files now scanned (not just .tmpl sources).
3. New test: codex review commands must not use inline git rev-parse
   (codex review doesn't support -C, so cd "$_REPO_ROOT" is the fix).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-03-26 23:52:05 -06:00
committed by GitHub
parent b343ba2797
commit 18bf4244ac
19 changed files with 198 additions and 732 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/gen-skill-docs.test.ts
+88
View File
@@ -1671,3 +1671,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([]);
});
});