diff --git a/changelog.d/garrytan-gsta-211-codex-removed-arm.md b/changelog.d/garrytan-gsta-211-codex-removed-arm.md new file mode 100644 index 000000000..ea8026670 --- /dev/null +++ b/changelog.d/garrytan-gsta-211-codex-removed-arm.md @@ -0,0 +1,5 @@ +**The rule-removed control for safety-rule evals now really removes the rule on the Codex host (GST-840).** The Codex host installs every gstack skill under `~/.codex/skills` as a link into the checkout. A removed-arm run therefore still had the rule in the installed skill, and in GSTA-211's `safety-pair-agent-block` run the model read it there and passed the control. + +### Fixed +- **`test/helpers/codex-host-runner.ts`**: under `GSTACK_SAFETY_ARM=removed`, each installed skill whose markdown carries a registered safety-rule paragraph gets a private copy of every linked directory on the way to that file (the skill directory itself or a linked directory inside it), and the file is rewritten with that paragraph deleted. Files are replaced, never written in place, so the checkout is never touched. The `rule` arm is unchanged. +- **`test/codex-host-safety-arm.test.ts`** checks both arms on a linked install like the one `./setup --host codex` makes, including a rule file under a linked directory inside a real skill directory, with no model and no setup run. diff --git a/scripts/harness-pins/test/helpers/codex-host-runner.ts.json b/scripts/harness-pins/test/helpers/codex-host-runner.ts.json index 9888d6845..8fffde4ca 100644 --- a/scripts/harness-pins/test/helpers/codex-host-runner.ts.json +++ b/scripts/harness-pins/test/helpers/codex-host-runner.ts.json @@ -1 +1 @@ -{"blob":"bff225017e235f0be6ece29f04971ac04b95b544","since":6,"nonBehavioral":"Claude-host verdicts are unchanged: the Codex host (EVALS_HOST=codex, codex-host-runner) runs only when selected and records its own per-model series; the Codex runner refactor (execCodex extraction, onSpawn hook) keeps its events and verdicts and adds token cost accounting; host fields on the plan/report are absent for claude"} +{"blob":"4596f41514cf87e2fe5f1791c00ea4991cba548c","since":6,"nonBehavioral":"Only GSTACK_SAFETY_ARM=removed runs on the Codex host change (GST-840): the installed ~/.codex/skills copies lose the registered rule paragraph, as the removed-arm fixture already did, so the removed control measures a missing rule. That arm is a manual comparison arm; gate and CI panels run the default rule arm, whose installed skills, prompts and verdicts are unchanged, and Claude-host runs never load this runner"} diff --git a/scripts/lib/windows-curation.ts b/scripts/lib/windows-curation.ts index dc82a023c..ecc2108af 100644 --- a/scripts/lib/windows-curation.ts +++ b/scripts/lib/windows-curation.ts @@ -57,6 +57,8 @@ export const KNOWN_WINDOWS_INCOMPATIBLE: Array<{ file: string; reason: string }> { file: 'test/shared-libs-index-flags-readonly.test.ts', reason: 'its assume-unchanged replay seeds the review through test/helpers/shared-libs-path-fixture.ts, which launches bin/gstack-review-log (a bash script) directly; windows-latest refuses it as not found in $PATH (windows-free-shard 5, run 40727825859)' }, { file: 'test/credential-rejection.test.ts', reason: 'replays run 39696775546 through the --report and --rerun-plan CLI of the paid lane, which runs only on Linux CI; on windows-latest that CLI exits 1 with EINVAL in open() (windows-free-shard run 39982914852)' }, // Field list 2, W11: the review-voice install is a POSIX launcher contract, exercised end to end on Linux and macOS. + // GST-840: the Codex host runtime links each installed skill into the checkout, as ./setup --host codex does on POSIX. + { file: 'test/codex-host-safety-arm.test.ts', reason: 'builds a symlinked ~/.codex/skills install (a linked skill folder and a linked folder inside a real one), which needs Developer Mode that windows-latest runners lack; ./setup --host codex copies instead of linking on Windows, so the removed-arm strip it tests only matters on POSIX (windows-free-shard 3, run 41336347706)' }, { file: 'test/capy-review-voice.test.ts', reason: 'copies the checkout with tar -C into the temp root (GNU tar reads a D: path as a remote host) and launches the bash --for review-voice installer with a colon-joined GSTACK_LAUNCH PATH' }, // C-pregate. Since fix wave PR 5 (item 10b) test/helpers/pregate-fixture.ts and restamp-fixture.ts // runBin launch bins through lib/script-invocation.ts, so gstack-pregate runs on windows-latest diff --git a/test/codex-host-safety-arm.test.ts b/test/codex-host-safety-arm.test.ts new file mode 100644 index 000000000..cddec998f --- /dev/null +++ b/test/codex-host-safety-arm.test.ts @@ -0,0 +1,89 @@ +/** + * The rule-removed safety arm on the Codex host (GST-840). The Codex runtime + * installs every skill under ~/.codex/skills as a link into this checkout, so + * a removed-arm fixture alone left the rule readable there: in GSTA-211's + * safety-pair-agent-block run the model read the installed pair-agent skill, + * rule included, and the control passed. These tests build that layout with no + * ./setup and no model. Each prepares its runtime in its own process, because + * prepareCodexHostRuntime caches the first root it returns per process. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { bunIsolationArgs } from '../lib/bun-isolation'; +import { locateRule, safetyRule } from './helpers/safety-rules'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const RULE = safetyRule('pair-agent-full-block'); +const dirs: string[] = []; +afterEach(() => { for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); }); + +/** A preset Codex runtime root whose installed pair-agent skill links to a stand-in checkout copy. */ +function presetRoot() { + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'codex-arm-')); + dirs.push(base); + const checkout = path.join(base, 'checkout', 'gstack-pair-agent'); + fs.mkdirSync(path.join(checkout, 'sections'), { recursive: true }); + const skill = fs.readFileSync(path.join(ROOT, 'pair-agent', 'SKILL.md'), 'utf8'); + fs.writeFileSync(path.join(checkout, 'SKILL.md'), skill); + fs.writeFileSync(path.join(checkout, 'sections', 'preamble.md'), '# preamble\n'); + const root = path.join(base, 'runtime'); + const skills = path.join(root, 'home', '.codex', 'skills'); + fs.mkdirSync(skills, { recursive: true }); + fs.symlinkSync(checkout + path.sep, path.join(skills, 'gstack-pair-agent')); + return { root, installed: path.join(skills, 'gstack-pair-agent'), checkout, skill }; +} + +/** prepareCodexHostRuntime on a preset root under one arm, in a fresh process. */ +function prepare(root: string, arm: 'rule' | 'removed') { + const probe = `import { prepareCodexHostRuntime } from ${JSON.stringify(path.join(ROOT, 'test', 'helpers', 'codex-host-runner.ts'))}; +const root = prepareCodexHostRuntime({ ...process.env, GSTACK_CODEX_HOST_ROOT: ${JSON.stringify(root)}, GSTACK_SAFETY_ARM: ${JSON.stringify(arm)} }); +if (root !== ${JSON.stringify(root)}) throw new Error('unexpected runtime root ' + root);`; + const run = Bun.spawnSync([process.execPath, ...bunIsolationArgs(), '-e', probe], { stdout: 'pipe', stderr: 'pipe', timeout: 30_000 }); + expect(run.stderr.toString()).toBe(''); + expect(run.exitCode).toBe(0); +} + +describe('Codex host runtime under GSTACK_SAFETY_ARM', () => { + // Value: protects=the removed-rule control on the Codex host cannot read the rule from the installed skill; + // fails_when=prepareCodexHostRuntime leaves an installed SKILL.md carrying a registered rule paragraph under the removed arm, or writes through the link; + // why_new=GSTA-211's Codex removed arm read ~/.codex/skills/gstack-pair-agent/SKILL.md with the rule intact; seam=prepareCodexHostRuntime; + // no_claim=runs no ./setup and no model; the paid rule/removed pair checks the model side + test('removed: the installed skill lacks the rule, and the checkout copy is untouched', () => { + const { root, installed, checkout, skill } = presetRoot(); + prepare(root, 'removed'); + const text = fs.readFileSync(path.join(installed, 'SKILL.md'), 'utf8'); + expect(locateRule(text, RULE)).toBeNull(); + expect(text).toContain('### If different machine (option B):'); + expect(fs.lstatSync(installed).isSymbolicLink()).toBe(false); + expect(fs.readFileSync(path.join(installed, 'sections', 'preamble.md'), 'utf8')).toBe('# preamble\n'); + expect(fs.readFileSync(path.join(checkout, 'SKILL.md'), 'utf8')).toBe(skill); + }); + + // Value: protects=the checkout behind a linked directory inside a real installed skill directory (the ~/.codex/skills/gstack layout); + // fails_when=the strip deletes or rewrites a rule-bearing file through a linked subdirectory instead of copying that directory first; + // why_new=/ship review of GST-840 found readdirSync({ recursive: true }) follows linked subdirectories; seam=prepareCodexHostRuntime; + // no_claim=see the removed case + test('removed: a rule file under a linked subdirectory is copied, never written through', () => { + const { root, checkout, skill } = presetRoot(); + const sidecar = path.join(root, 'home', '.codex', 'skills', 'gstack'); + fs.mkdirSync(sidecar); + fs.symlinkSync(checkout, path.join(sidecar, 'linked')); + prepare(root, 'removed'); + expect(fs.lstatSync(sidecar).isSymbolicLink()).toBe(false); + expect(fs.lstatSync(path.join(sidecar, 'linked')).isSymbolicLink()).toBe(false); + expect(locateRule(fs.readFileSync(path.join(sidecar, 'linked', 'SKILL.md'), 'utf8'), RULE)).toBeNull(); + expect(fs.readFileSync(path.join(checkout, 'SKILL.md'), 'utf8')).toBe(skill); + }); + + // Value: protects=the shipped-rule arm still runs the install exactly as ./setup left it; + // fails_when=the strip also runs for GSTACK_SAFETY_ARM=rule; why_new=negative control for the removed case; seam=prepareCodexHostRuntime; + // no_claim=see the removed case + test('rule: the installed link and its rule stay in place', () => { + const { root, installed } = presetRoot(); + prepare(root, 'rule'); + expect(fs.lstatSync(installed).isSymbolicLink()).toBe(true); + expect(locateRule(fs.readFileSync(path.join(installed, 'SKILL.md'), 'utf8'), RULE)).not.toBeNull(); + }); +}); diff --git a/test/helpers/codex-host-runner.ts b/test/helpers/codex-host-runner.ts index bff225017..4596f4151 100644 --- a/test/helpers/codex-host-runner.ts +++ b/test/helpers/codex-host-runner.ts @@ -36,6 +36,7 @@ import { spawnSync } from 'child_process'; import { CODEX_DRAIN_GRACE_MS, CodexHarnessError, codexUsageCostUsd, execCodex, type CodexResult } from './codex-session-runner'; import { getProjectEvalDir } from './eval-store'; import { hermeticChildEnv } from './hermetic-env'; +import { SAFETY_RULES, locateRule, removeRule, safetyArm } from './safety-rules'; import { appendSessionLedger, sessionKey, type SessionEnd } from './session-ledger'; import { sanitizeTestName, scanBrowseErrors, type SkillTestResult } from './session-runner'; import { CODEX_FRONTIER_MODEL } from '../../scripts/resolvers/constants'; @@ -165,6 +166,39 @@ export function codexHostTranscript(rawLines: readonly string[], cwd: string, ou let runtimeRoot: string | undefined; +/** + * The rule-removed safety arm (GSTACK_SAFETY_ARM=removed) must hold on the + * installed skills too, or the model reads the rule from ~/.codex/skills + * (GST-840). Each installed skill whose markdown carries a registered rule + * paragraph reaches this checkout through a link (the skill directory itself, + * or a linked directory inside it), so every linked directory on the way to + * such a file becomes a private copy, and the file is rewritten without the + * paragraph. Files are replaced, never written in place, so nothing reaches + * the checkout through a link. + */ +function removeSafetyRulesFromInstall(skillsDir: string): void { + for (const name of fs.readdirSync(skillsDir)) { + const dir = path.join(skillsDir, name); + if (!fs.statSync(dir).isDirectory()) continue; + const files = (fs.readdirSync(dir, { recursive: true }) as string[]).filter(file => file.endsWith('.md')) + .map(file => ({ file, text: fs.readFileSync(path.join(dir, file), 'utf8') })) + .filter(({ text }) => SAFETY_RULES.some(rule => locateRule(text, rule))); + for (const { file, text } of files) { + let out = dir; + for (const part of file.split(path.sep)) { + if (fs.lstatSync(out).isSymbolicLink()) { + const target = fs.realpathSync(out); + fs.unlinkSync(out); + fs.cpSync(target, out, { recursive: true }); + } + out = path.join(out, part); + } + fs.rmSync(out, { force: true }); + fs.writeFileSync(out, SAFETY_RULES.reduce((current, rule) => locateRule(current, rule) ? removeRule(current, rule) : current, text)); + } + } +} + /** * Install the full gstack Codex runtime once per process: `./setup --host codex` * into a private HOME/CODEX_HOME, with the login home's auth.json copied in. @@ -178,6 +212,7 @@ export function prepareCodexHostRuntime(env: NodeJS.ProcessEnv = process.env): s if (preset) { if (!fs.existsSync(path.join(preset, 'home', '.codex'))) throw new Error(`GSTACK_CODEX_HOST_ROOT ${preset} has no home/.codex`); fs.mkdirSync(path.join(preset, 'tmp'), { recursive: true }); + if (safetyArm(env) === 'removed') removeSafetyRulesFromInstall(path.join(preset, 'home', '.codex', 'skills')); return runtimeRoot = preset; } const root = fs.mkdtempSync(path.join(os.tmpdir(), 'codex-host-')); @@ -196,6 +231,7 @@ export function prepareCodexHostRuntime(env: NodeJS.ProcessEnv = process.env): s throw new Error(`./setup --host codex failed (exit ${setup.status}${setup.error ? `, ${setup.error.message}` : ''}): ${(setup.stderr || setup.stdout || '').slice(-2000)}`); } process.once('exit', () => { try { fs.rmSync(root, { recursive: true, force: true }); } catch { /* best effort */ } }); + if (safetyArm(env) === 'removed') removeSafetyRulesFromInstall(path.join(codexHome, 'skills')); runtimeRoot = root; return root; }