mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-07 01:15:47 +01:00
fix(evals): the Codex removed-rule arm really removes the rule from installed skills (GST-840) (#3816)
* fix(evals): Codex removed-rule arm strips the rule from the installed skills (GST-840)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix(evals): copy linked directories inside a skill before stripping the rule; record the harness decision (GST-840)
/ship review found readdirSync({ recursive: true }) follows linked
subdirectories, so a rule file under a linked directory inside a real
skill directory would have been deleted and rewritten in the checkout.
Every linked directory on the way to a rule file is now copied first,
with a failing-first test row. The harness-version manifest records
codex-host-runner.ts as non-behavioral: only the manual removed arm
on the Codex host changes.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* test: keep the Codex safety-arm test off Windows, and spawn its probe with bunIsolationArgs (GST-840)
windows-free-shard 3 (run 41336347706) failed every case: native Bun on
Windows cannot open --config=/dev/null, and the test builds a symlinked
~/.codex/skills install that needs Developer Mode the runners lack.
./setup --host codex copies instead of linking on Windows, so the strip
only matters on POSIX. The probe now uses bunIsolationArgs().
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
---------
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
fc5e32b712
commit
91ddd3bdb1
5 files changed
+133
-1
No files matched your search
@@ -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.
|
||||
@@ -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"}
|
||||
@@ -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
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user