mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-13 11:57:19 +01:00
Keep /review and /ship working in copied Windows installs (GST-2347) (#4286)
* Keep /review and /ship working in copied Windows installs (GST-2347) On Windows, setup copies bin/ and lib/ into each non-Claude host's runtime folder, but left out the scripts/ modules they import. gstack-review-plan, which /review and /ship run first, failed with "Cannot find module '../scripts/resolvers/fix-check'". Setup now also copies fix-check.ts, discover-skills.ts and lib/ci-history.ts, and the runtime-root test follows every static import of the commands each host's skills run. Co-authored-by: Satria Suria <57977752+SatriaSuriaAriffin@users.noreply.github.com> Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Say the copied-install import check follows static imports only The changelog entry claimed the new test follows every import; it follows static imports. TODOS.md now lists the three deferred commands with one remedy and records what the check does not see: lazy import() and require() calls, modules a shell script launches, and bare package imports. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep the production-dependency check's walk sentinel on a module setup does not name setup now names scripts/discover-skills.ts, so the check scans it as a root file and its import walk no longer reaches it. The walk's sentinel moves to scripts/gen-agents-digest.ts, which only gen-skill-docs imports, and discover-skills.ts stays asserted as scanned. Fixes the quick-bun-default failure in CI run 42857125482. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: Satria Suria <57977752+SatriaSuriaAriffin@users.noreply.github.com> Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
5 files changed
+85
-12
No files matched your search
@@ -20,6 +20,7 @@ import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { HOST_CONFIG_MAP } from '../hosts';
|
||||
import { bunIsolationArgs } from '../lib/bun-isolation';
|
||||
import { runGeneration } from '../scripts/gen-skill-docs';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
@@ -43,7 +44,9 @@ const ROOT_BUILDERS: Record<string, string> = {
|
||||
// Deferred, with the reason recorded in TODOS.md: /ship's measure loop runs
|
||||
// scripts/ship-measure.ts, which imports gstack's test helpers and eval
|
||||
// harness; a runtime root would need the whole development tree.
|
||||
const DEFERRED = new Set(['scripts/ship-measure.ts']);
|
||||
// gstack-model-benchmark (/benchmark-models) and the merge queue's
|
||||
// gstack-ship-queue are deferred the same way: both import that tree.
|
||||
const DEFERRED = new Set(['scripts/ship-measure.ts', 'bin/gstack-model-benchmark', 'bin/gstack-ship-queue']);
|
||||
|
||||
// Compiled outputs: the stub source carries placeholder binaries, so a fresh
|
||||
// checkout without a build still exercises the links to them.
|
||||
@@ -97,17 +100,52 @@ function makeSource(): void {
|
||||
}
|
||||
}
|
||||
|
||||
function buildRoot(host: string): { root: string; stderr: string; status: number | null } {
|
||||
const root = path.join(tmp, 'home', host, 'skills', 'gstack');
|
||||
// windows: setup's IS_WINDOWS=1 arm, where bin/, lib/ and the runtime dists
|
||||
// are real copies instead of links, so a module they import resolves only if
|
||||
// setup copied it beside them. It copies from the checkout itself, because
|
||||
// cp -R copies the stub source's top-level links as links.
|
||||
function buildRoot(host: string, windows = false): { root: string; stderr: string; status: number | null } {
|
||||
const root = path.join(tmp, windows ? 'home-windows' : 'home', host, 'skills', 'gstack');
|
||||
fs.mkdirSync(path.dirname(root), { recursive: true });
|
||||
const functions = ['_link_or_copy', '_link_runtime_dists', '_copy_runtime_skill_refs', '_copy_skill_md',
|
||||
'_sidecar_root_user_owned', '_gstack_generated_header', ROOT_BUILDERS[host]!]
|
||||
.filter(name => SETUP_SRC.includes(`\n${name}() {`)).map(extractFunction).join('\n');
|
||||
const r = spawnSync('bash', ['-c', `set -e\nIS_WINDOWS=0\n${functions}\n${ROOT_BUILDERS[host]} "${src}" "${root}"`],
|
||||
const r = spawnSync('bash', ['-c', `set -e\nIS_WINDOWS=${windows ? 1 : 0}\n${functions}\n${ROOT_BUILDERS[host]} "${windows ? ROOT : src}" "${root}"`],
|
||||
{ encoding: 'utf8', timeout: 30_000, env: { PATH: process.env.PATH ?? '/usr/bin:/bin', HOME: path.join(tmp, 'home') } });
|
||||
return { root, stderr: r.stderr, status: r.status };
|
||||
}
|
||||
|
||||
const transpiler = new Bun.Transpiler({ loader: 'ts' });
|
||||
function isBunSource(file: string): boolean {
|
||||
if (!fs.existsSync(file) || !fs.statSync(file).isFile()) return false;
|
||||
return /\.(ts|js|mjs)$/.test(file) || (!path.extname(file) && /^#!.*\bbun\b/.test(fs.readFileSync(file, 'utf8')));
|
||||
}
|
||||
|
||||
/**
|
||||
* Static imports reachable from the code a host's skills run that do not
|
||||
* resolve inside its runtime root. Paths resolve from where the root holds
|
||||
* each file, never through its links, which is how a copied root loads them.
|
||||
*/
|
||||
function importsOutsideRoot(host: string, root: string): string[] {
|
||||
const queue = [...referencedPaths(host)].filter(rel => !DEFERRED.has(rel)).map(rel => path.join(root, rel)).filter(isBunSource);
|
||||
const seen = new Set<string>();
|
||||
const outside = new Set<string>();
|
||||
while (queue.length) {
|
||||
const file = queue.pop()!;
|
||||
if (seen.has(file)) continue;
|
||||
seen.add(file);
|
||||
for (const { path: spec, kind } of transpiler.scanImports(fs.readFileSync(file, 'utf8').replace(/^#!.*/, ''))) {
|
||||
if (kind !== 'import-statement' || !spec.startsWith('.')) continue;
|
||||
const target = path.resolve(path.dirname(file), spec);
|
||||
const found = [target, target.replace(/\.js$/, '.ts'), `${target}.ts`, `${target}.js`, path.join(target, 'index.ts')]
|
||||
.find(c => c.startsWith(root + path.sep) && fs.existsSync(c) && fs.statSync(c).isFile());
|
||||
if (!found) outside.add(`${path.relative(root, file)} -> ${path.relative(root, target)}`);
|
||||
else if (isBunSource(found)) queue.push(found);
|
||||
}
|
||||
}
|
||||
return [...outside].sort();
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
const result = await runGeneration({ host: 'all', outputRoot: renderDir });
|
||||
if (result.exitCode !== 0) throw new Error(result.diagnostics.filter(d => d.kind === 'error').map(d => d.message).join('\n'));
|
||||
@@ -139,6 +177,26 @@ describe.skipIf(process.platform === 'win32')('runtime roots carry every $GSTACK
|
||||
});
|
||||
}
|
||||
|
||||
for (const host of Object.keys(ROOT_BUILDERS)) {
|
||||
test(`${host}: every module the skills' commands import is inside the runtime root`, () => {
|
||||
const { root, status } = buildRoot(host);
|
||||
expect(status).toBe(0);
|
||||
expect(importsOutsideRoot(host, root)).toEqual([]);
|
||||
});
|
||||
}
|
||||
|
||||
test('the review planner starts from a copied (Windows) runtime root', () => {
|
||||
const { root, stderr, status } = buildRoot('codex', true);
|
||||
expect(stderr).toBe('');
|
||||
expect(status).toBe(0);
|
||||
expect(fs.lstatSync(path.join(root, 'bin')).isSymbolicLink()).toBe(false);
|
||||
const r = spawnSync(process.execPath, [...bunIsolationArgs(), path.join(root, 'bin', 'gstack-review-plan'), '--help'],
|
||||
{ cwd: tmp, encoding: 'utf8', timeout: 30_000, env: { PATH: process.env.PATH ?? '/usr/bin:/bin', HOME: path.join(tmp, 'home-windows') } });
|
||||
expect(r.stderr).not.toContain('Cannot find module');
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toContain('--fix-check-prompt');
|
||||
});
|
||||
|
||||
test('a runtime root never gains a whole skill directory beside the rendered copies', () => {
|
||||
const { root } = buildRoot('codex');
|
||||
for (const dir of ['plan-design-review', 'office-hours']) {
|
||||
|
||||
@@ -572,7 +572,8 @@ describe('setup and runtime need only production dependencies', () => {
|
||||
queue.push(target);
|
||||
}
|
||||
}
|
||||
expect(Object.keys(reached)).toContain('scripts/discover-skills.ts');
|
||||
expect(Object.keys(reached)).toContain('scripts/gen-agents-digest.ts');
|
||||
expect(Object.keys({ ...files, ...reached })).toContain('scripts/discover-skills.ts');
|
||||
expect(Object.keys(reached)).toContain('test/helpers/providers/claude.ts');
|
||||
expect(nonProductionImports(PKG, reached)).toEqual([]);
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user