E2: one make-pdf resolver for readiness and every $P block on env-var hosts

make-pdf exported $P from its readiness block, which Codex and the other
env-var hosts lose between blocks. binaryAssignment gains 'make-pdf': the
MAKE-PDF SETUP readiness check and the runtime prelude both use it, so every
block that runs $P derives the same make-pdf/dist/pdf binary, with
MAKE_PDF_BIN (dev builds) winning when it is executable. Claude output keeps
its repo-local-first probe; only the false 'subsequent blocks' comment
changed. Root plus P=... fits the 400-byte prelude budget; the fixed
MAKE_PDF_OVERRIDE segment is the one exemption, named in runtime-root.ts and
the conformance test.
This commit is contained in:
garrytan committed 2026-10-04 17:50:11 +00:00
1 parent 337d34fe16
commit d16a1687f9
3 files changed
+100 -34

No files matched your search

+12 -15
View File
@@ -1,29 +1,26 @@
import { type TemplateContext, toShellPath } from './types';
import { type TemplateContext } from './types';
import { binaryAssignment } from './runtime-root';
/**
* {{MAKE_PDF_SETUP}} — emits the shell preamble that resolves $P to the
* make-pdf binary. Mirrors generateDesignSetup.
* {{MAKE_PDF_SETUP}} — emits the readiness check that resolves $P to the
* make-pdf binary (make-pdf/dist/pdf). Mirrors generateDesignSetup: the
* assignment comes from binaryAssignment(ctx, 'make-pdf'), the same resolver
* the runtime prelude puts into every later block that uses $P on env-var
* hosts, so readiness and use always name the same binary.
*
* $P = make-pdf/dist/pdf.
*
* Resolution order:
* 1. Local skill root: $_ROOT/{localSkillRoot}/make-pdf/dist/pdf
* 2. Global: ~/{globalRoot}/make-pdf/dist/pdf
* 3. Env override (MAKE_PDF_BIN) — for contributor dev builds
* Resolution order: MAKE_PDF_BIN (contributor dev builds), then the
* repo-local install, then the global install (env-var hosts: $GSTACK_ROOT,
* which the prelude resolves repo-local first).
*/
export function generateMakePdfSetup(ctx: TemplateContext): string {
return `## MAKE-PDF SETUP (run this check BEFORE any make-pdf command)
\`\`\`bash
_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
P=""
[ -n "$MAKE_PDF_BIN" ] && [ -x "$MAKE_PDF_BIN" ] && P="$MAKE_PDF_BIN"
[ -z "$P" ] && [ -n "$_ROOT" ] && [ -x "$_ROOT/${ctx.paths.localSkillRoot}/make-pdf/dist/pdf" ] && P="$_ROOT/${ctx.paths.localSkillRoot}/make-pdf/dist/pdf"
[ -z "$P" ] && P="${toShellPath(ctx.paths.skillRoot)}/make-pdf/dist/pdf"
${binaryAssignment(ctx, 'make-pdf')}
if [ -x "$P" ]; then
echo "MAKE_PDF_READY: $P"
alias _p_="$P" # shellcheck alias helper (not exported)
export P # available as $P in subsequent blocks within the same skill invocation
export P # env-var hosts re-derive $P in every later block (runtime prelude)
else
echo "MAKE_PDF_NOT_AVAILABLE (run './setup' in the gstack repo to build it)"
fi
+28 -9
View File
@@ -2,8 +2,8 @@
* Runtime root for env-var hosts (C1, #1159): the one owner of how generated
* bash finds gstack on Codex and every other `usesEnvVars` host.
*
* Those hosts run each fenced block in a fresh shell, so `GSTACK_ROOT`, `B`
* and `D` set by an earlier block are gone. The preamble's root resolution and
* Those hosts run each fenced block in a fresh shell, so `GSTACK_ROOT`, `B`,
* `D` and `P` set by an earlier block are gone. The preamble's root resolution and
* the per-fence prelude come from `runtimeRootPrelude()`; `insertRuntimePreludes()`
* is the single post-render pass (gen-skill-docs, before rewriteInstallRoot)
* that puts it into every fence that uses these variables without assigning
@@ -19,6 +19,13 @@ import { toShellPath, type TemplateContext } from './types';
/** Per-fence byte budget for the prelude (T-ENG1), asserted by the INV-3 test. */
export const PRELUDE_BYTE_BUDGET = 400;
/**
* make-pdf's dev-build override (E2), appended to `P`'s assignment. It is the
* one fixed segment outside PRELUDE_BYTE_BUDGET: root plus `P=...` fits the
* budget like `B`/`D` do, and the override is needed in every block that runs
* `$P` so readiness and use name the same binary.
*/
export const MAKE_PDF_OVERRIDE = ';[ -x "${MAKE_PDF_BIN:-}" ]&&P=$MAKE_PDF_BIN||:';
const ROOT_VARS = /\$\{?GSTACK_(?:ROOT|BIN|BROWSE|DESIGN|MAKE_PDF)\b/;
const assigns = (name: string) => new RegExp(`(?:^|[\\s;&|(])${name}=`, 'm');
@@ -48,13 +55,23 @@ export function runtimeRootPrelude(ctx: TemplateContext): string {
}
/**
* Lines that assign `B` (browse) or `D` (design). BROWSE SETUP, DESIGN SETUP and
* the prelude all use this, so a later fence runs the binary setup checked.
* Env-var hosts derive from `GSTACK_ROOT`, which the prelude already points at
* the repo-local install when one exists; other hosts keep their
* repo-local-first probe.
* Lines that assign `B` (browse), `D` (design) or `P` (make-pdf). BROWSE SETUP,
* DESIGN SETUP, MAKE-PDF SETUP and the prelude all use this, so a later fence
* runs the binary setup checked. Env-var hosts derive from `GSTACK_ROOT`, which
* the prelude already points at the repo-local install when one exists; other
* hosts keep their repo-local-first probe. make-pdf's binary is
* `make-pdf/dist/pdf`, and `MAKE_PDF_BIN` (a contributor dev build) wins on
* every host.
*/
export function binaryAssignment(ctx: TemplateContext, tool: 'browse' | 'design'): string {
export function binaryAssignment(ctx: TemplateContext, tool: 'browse' | 'design' | 'make-pdf'): string {
if (tool === 'make-pdf') {
if (getHostConfig(ctx.host).usesEnvVars) return `P=$GSTACK_ROOT/make-pdf/dist/pdf${MAKE_PDF_OVERRIDE}`;
return `_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
P=""
[ -n "$MAKE_PDF_BIN" ] && [ -x "$MAKE_PDF_BIN" ] && P="$MAKE_PDF_BIN"
[ -z "$P" ] && [ -n "$_ROOT" ] && [ -x "$_ROOT/${ctx.paths.localSkillRoot}/make-pdf/dist/pdf" ] && P="$_ROOT/${ctx.paths.localSkillRoot}/make-pdf/dist/pdf"
[ -z "$P" ] && P="${toShellPath(ctx.paths.skillRoot)}/make-pdf/dist/pdf"`;
}
const v = tool === 'browse' ? 'B' : 'D';
const dir = tool === 'browse' ? ctx.paths.browseDir : ctx.paths.designDir;
if (getHostConfig(ctx.host).usesEnvVars) return `${v}=$GSTACK_ROOT/${tool}/dist/${tool}`;
@@ -69,8 +86,9 @@ export function fencePrelude(ctx: TemplateContext, body: string): string {
if (!getHostConfig(ctx.host).usesEnvVars) return '';
const needB = uses('B').test(body) && !assigns('B').test(body);
const needD = uses('D').test(body) && !assigns('D').test(body);
const needP = uses('P').test(body) && !assigns('P').test(body);
if (assigns('GSTACK_ROOT').test(body)) return '';
const needRoot = needB || needD || ROOT_VARS.test(body);
const needRoot = needB || needD || needP || ROOT_VARS.test(body);
const derived = Object.entries(DERIVED)
.filter(([name]) => uses(`GSTACK_${name}`).test(body) && !assigns(`GSTACK_${name}`).test(body))
.map(([name, dir]) => `GSTACK_${name}=$GSTACK_ROOT/${dir}`);
@@ -79,6 +97,7 @@ export function fencePrelude(ctx: TemplateContext, body: string): string {
derived.join(' '),
needB ? binaryAssignment(ctx, 'browse') : '',
needD ? binaryAssignment(ctx, 'design') : '',
needP ? binaryAssignment(ctx, 'make-pdf') : '',
].filter(Boolean).join('\n');
}
+60 -10
View File
@@ -16,13 +16,14 @@ import * as os from 'os';
import * as path from 'path';
import { ALL_HOST_CONFIGS } from '../hosts';
import { runGeneration } from '../scripts/gen-skill-docs';
import { binaryAssignment, fencePrelude, insertRuntimePreludes, PRELUDE_BYTE_BUDGET, runtimeRootPrelude } from '../scripts/resolvers/runtime-root';
import { generateMakePdfSetup } from '../scripts/resolvers/make-pdf';
import { binaryAssignment, fencePrelude, insertRuntimePreludes, MAKE_PDF_OVERRIDE, PRELUDE_BYTE_BUDGET, runtimeRootPrelude } from '../scripts/resolvers/runtime-root';
import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types';
import { lintFence, normalizePlaceholders } from './helpers/generated-bash-lint';
const ENV_HOSTS = ALL_HOST_CONFIGS.filter(h => h.usesEnvVars);
const FORBIDDEN = /^(?:\/bin\/|\/browse|\/design|\/gstack-)/;
const PRELUDE_LINE = /^\[ -d "\$\{GSTACK_ROOT:-\/-\}\/bin" \]|^(?:GSTACK_(?:BIN|BROWSE|DESIGN|MAKE_PDF)=\$GSTACK_ROOT\/\S+ ?)+$|^[BD]=\$GSTACK_ROOT\//;
const PRELUDE_LINE = /^\[ -d "\$\{GSTACK_ROOT:-\/-\}\/bin" \]|^(?:GSTACK_(?:BIN|BROWSE|DESIGN|MAKE_PDF)=\$GSTACK_ROOT\/\S+ ?)+$|^[BDP]=\$GSTACK_ROOT\//;
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-env-fences-'));
const renderDir = path.join(tmp, 'render');
@@ -70,8 +71,10 @@ function fenceProblems(fence: Fence): string[] {
if (/\$\{?GSTACK_(?:ROOT|BIN|BROWSE|DESIGN|MAKE_PDF)\b/.test(fence.body) && !assigned('GSTACK_ROOT')) problems.push(`${where} uses GSTACK_* without resolving GSTACK_ROOT`);
if (/\$\{?B\b/.test(fence.body) && !assigned('B')) problems.push(`${where} uses $B without deriving it`);
if (/\$\{?D\b/.test(fence.body) && !assigned('D')) problems.push(`${where} uses $D without deriving it`);
if (/\$\{?P\b/.test(fence.body) && !/(?:^|[\s;&|(])P=/m.test(fence.body)) problems.push(`${where} uses $P without deriving it`);
if (fence.body.includes('gstack: no install found')) {
const bytes = Buffer.byteLength(fence.body.split('\n').filter(l => PRELUDE_LINE.test(l)).join('\n') + '\n');
// MAKE_PDF_OVERRIDE is the one fixed segment outside the budget (E2).
const bytes = Buffer.byteLength(fence.body.split('\n').filter(l => PRELUDE_LINE.test(l)).join('\n').replace(MAKE_PDF_OVERRIDE, '') + '\n');
if (bytes > PRELUDE_BYTE_BUDGET) problems.push(`${where} prelude is ${bytes} bytes (budget ${PRELUDE_BYTE_BUDGET})`);
if (fence.body.split('gstack: no install found').length !== 2) problems.push(`${where} carries the prelude more than once`);
}
@@ -84,7 +87,7 @@ function sh(script: string, cwd: string, env: Record<string, string>) {
return spawnSync('bash', ['-uc', script], { cwd, encoding: 'utf8', timeout: 10_000, env: { PATH: process.env.PATH ?? '/usr/bin:/bin', ...env } });
}
function mkroot(dir: string) {
for (const sub of ['bin', 'lib', 'browse/dist', 'design/dist']) fs.mkdirSync(path.join(dir, sub), { recursive: true });
for (const sub of ['bin', 'lib', 'browse/dist', 'design/dist', 'make-pdf/dist']) fs.mkdirSync(path.join(dir, sub), { recursive: true });
return dir;
}
@@ -95,13 +98,13 @@ beforeAll(async () => {
afterAll(() => fs.rmSync(tmp, { recursive: true, force: true }));
describe('C1: every env-var host fence resolves its own runtime paths', () => {
test('each fence that uses $GSTACK_*, $B or $D assigns them in the same fence, within the byte budget', () => {
test('each fence that uses $GSTACK_*, $B, $D or $P assigns them in the same fence, within the byte budget', () => {
const problems: string[] = [];
let checked = 0;
for (const host of ENV_HOSTS) {
for (const file of renderedDocs(renderDir, host.hostSubdir)) {
for (const fence of bashFences(fs.readFileSync(file, 'utf8'), path.relative(renderDir, file))) {
if (!/\$\{?(?:GSTACK_(?:ROOT|BIN|BROWSE|DESIGN|MAKE_PDF)|B|D)\b/.test(fence.body)) continue;
if (!/\$\{?(?:GSTACK_(?:ROOT|BIN|BROWSE|DESIGN|MAKE_PDF)|B|D|P)\b/.test(fence.body)) continue;
checked++;
problems.push(...fenceProblems(fence));
}
@@ -234,6 +237,7 @@ describe('INV-3: every rendered fence parses, runs its prelude and passes the li
for (const host of ENV_HOSTS) {
const root = mkroot(path.join(home, host.name === 'codex' ? '.codex/skills/gstack' : host.globalRoot));
for (const tool of ['browse', 'design']) fs.writeFileSync(path.join(root, tool, 'dist', tool), `#!/bin/sh\necho "STUB_${tool}"\n`, { mode: 0o755 });
fs.writeFileSync(path.join(root, 'make-pdf', 'dist', 'pdf'), '#!/bin/sh\necho "STUB_pdf"\n', { mode: 0o755 });
const preludes = new Set<string>();
for (const file of renderedDocs(renderDir, host.hostSubdir)) {
for (const fence of bashFences(fs.readFileSync(file, 'utf8'))) {
@@ -244,8 +248,8 @@ describe('INV-3: every rendered fence parses, runs its prelude and passes the li
expect(preludes.size).toBeGreaterThan(0);
for (const prelude of preludes) {
ran++;
const vars = [...new Set([...prelude.matchAll(/(?:^|\s)(GSTACK_[A-Z_]+|[BD])=/gm)].map(m => m[1]))];
const consumers = vars.filter(v => v === 'B' || v === 'D').map(v => `"$${v}"`).join('\n');
const vars = [...new Set([...prelude.matchAll(/(?:^|\s)(GSTACK_[A-Z_]+|[BDP])=/gm)].map(m => m[1]))];
const consumers = vars.filter(v => v === 'B' || v === 'D' || v === 'P').map(v => `"$${v}"`).join('\n');
const script = `${prelude}\n${vars.map(v => `printf '%s=%s\\n' ${v} "$${v}"`).join('\n')}\n${consumers}`;
const r = spawnSync('env', ['-i', `HOME=${home}`, 'PATH=/usr/bin:/bin', 'bash', '-uc', script], { cwd: w, encoding: 'utf8', timeout: 10_000 });
if (r.status !== 0) { problems.push(`${host.name}: exit ${r.status}: ${r.stderr.trim()}\n${prelude}`); continue; }
@@ -253,8 +257,8 @@ describe('INV-3: every rendered fence parses, runs its prelude and passes the li
const value = line.slice(line.indexOf('=') + 1);
if (!value || FORBIDDEN.test(value)) problems.push(`${host.name}: ${line}`);
}
for (const v of vars.filter(x => x === 'B' || x === 'D')) {
if (!r.stdout.includes(`STUB_${v === 'B' ? 'browse' : 'design'}`)) problems.push(`${host.name}: $${v} did not run the stub binary`);
for (const v of vars.filter(x => x === 'B' || x === 'D' || x === 'P')) {
if (!r.stdout.includes(`STUB_${{ B: 'browse', D: 'design', P: 'pdf' }[v]}`)) problems.push(`${host.name}: $${v} did not run the stub binary`);
}
}
}
@@ -302,3 +306,49 @@ describe('INV-3: negative controls (planted bad fences fail each check)', () =>
expect(fenceProblems({ file: 'x', line: 1, body: twice })).toContain('x:1 carries the prelude more than once');
});
});
// E2: make-pdf's $P comes from one resolver (binaryAssignment 'make-pdf') for
// the readiness check and every later block, honoring MAKE_PDF_BIN.
describe('E2: make-pdf $P on env-var hosts', () => {
const pdfStub = (file: string, label: string) => {
fs.mkdirSync(path.dirname(file), { recursive: true });
fs.writeFileSync(file, `#!/bin/sh\necho "${label} $*"\n`, { mode: 0o755 });
};
test('readiness and a later block resolve the same binary, MAKE_PDF_BIN wins, paths with spaces work', () => {
const w = fs.mkdtempSync(path.join(tmp, 'pdf with spaces-'));
const home = path.join(w, 'home dir');
const root = mkroot(path.join(w, 'gstack root'));
pdfStub(path.join(root, 'make-pdf', 'dist', 'pdf'), 'ROOT_PDF');
const dev = path.join(w, 'dev build', 'pdf');
pdfStub(dev, 'DEV_PDF');
const c = { ...ctx('codex'), skillName: 'make-pdf' };
const readiness = bashFences(insertRuntimePreludes(generateMakePdfSetup(c), c))[0].body;
const later = fencePrelude(c, '"$P" generate letter.md') + '\n"$P" generate letter.md';
const env = (extra: Record<string, string> = {}) =>
['-i', `HOME=${home}`, 'PATH=/usr/bin:/bin', `GSTACK_ROOT=${root}`, ...Object.entries(extra).map(([k, v]) => `${k}=${v}`)];
const run = (script: string, extra?: Record<string, string>) =>
spawnSync('env', [...env(extra), 'bash', '-uc', script], { cwd: w, encoding: 'utf8', timeout: 10_000 });
let r = run(readiness);
expect(r.stdout, r.stderr).toContain(`MAKE_PDF_READY: ${root}/make-pdf/dist/pdf`);
r = run(later);
expect(r.stdout.trim(), r.stderr).toBe('ROOT_PDF generate letter.md');
r = run(readiness, { MAKE_PDF_BIN: dev });
expect(r.stdout, r.stderr).toContain(`MAKE_PDF_READY: ${dev}`);
r = run(later, { MAKE_PDF_BIN: dev });
expect(r.stdout.trim(), r.stderr).toBe('DEV_PDF generate letter.md');
// A MAKE_PDF_BIN that is not executable falls back to the install, in both places.
r = run(later, { MAKE_PDF_BIN: path.join(w, 'missing pdf') });
expect(r.stdout.trim(), r.stderr).toBe('ROOT_PDF generate letter.md');
});
test('Claude keeps its repo-local-first probe with MAKE_PDF_BIN first; only the comment changed', () => {
const block = bashFences(generateMakePdfSetup({ ...ctx('claude'), skillName: 'make-pdf' }))[0].body;
expect(block).toContain(binaryAssignment(ctx('claude'), 'make-pdf'));
expect(block.indexOf('MAKE_PDF_BIN')).toBeLessThan(block.indexOf('_ROOT/'));
expect(block).not.toContain('available as $P in subsequent blocks');
});
});