Skip to content

Commit 2380179

Browse files
committed
fix: correct bash installer exit code and isolate live eval auth
install-package.sh reported failure (exit 1) on a successful normal install or --clean reinstall, because the EXIT trap's cleanup check used `[ -d "$stage" ] && rm -rf "$stage"` as a bare statement: when $stage no longer exists (the normal success case), that false test is exempt from set -e but its own exit status becomes the trap's return value, which becomes the script's final exit code. Every bash wrapper script ends by calling install-package.sh as its last statement, so this silently broke default-mode and --clean install reporting for all callers. Fixed with explicit if/fi. Caught by a new scripts/test-installers.sh bash test suite mirroring the existing PowerShell coverage. run-behavioral-evals.mjs left CODEX_HOME pointed at the real user's .codex directory even though HOME/USERPROFILE were overridden to a disposable sandbox, so live two-pass eval runs could inherit real skill and session state instead of the isolated fixture copy. Fixed by pointing CODEX_HOME at the disposable home. A fully empty disposable home cannot authenticate, so the runner now seeds it with a copy of the real auth.json only, nothing else. Verified against a real codex exec run: the failure mode moved from a missing-directory error, to 401 Unauthorized, to a real account usage-limit response, confirming both the isolation and the authentication seeding work as intended. A full live pass/fail verdict for the two named cases is still blocked by that account-level usage limit, unrelated to this code.
1 parent 42f794d commit 2380179

5 files changed

Lines changed: 118 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,32 @@
2121
- Extend the existing behavioral-evaluation runner with the authored
2222
no-date-only-churn and source-drift two-pass cases, an optional model
2323
override, and stronger tested-skill isolation evidence.
24-
- Add installer behavior tests covering both managed packages.
24+
- Add installer behavior tests covering both managed packages, including a
25+
new Bash test suite (`scripts/test-installers.sh`) mirroring the existing
26+
PowerShell coverage.
27+
- Fix a Bash installer defect where a successful normal install or `--clean`
28+
reinstall printed a success message but exited with status 1, because the
29+
`EXIT` trap's cleanup check used `[ -d "$stage" ] && rm -rf "$stage"` as a
30+
bare statement; a false test in that position is exempt from `set -e` but
31+
still sets the trap function's own exit status, which becomes the script's
32+
final exit code. Every Bash wrapper script (`install-user.sh`,
33+
`install-project.sh`, `install-codex-user.sh`, `install-codex-project.sh`,
34+
`install-bootstrap-user.sh`) ends by calling `install-package.sh` as its
35+
last statement, so this previously caused those default-mode and
36+
`--clean` invocations to report failure to callers despite installing
37+
correctly. Fixed by using explicit `if`/`fi` instead of `&&` for both the
38+
trap cleanup and the post-replace backup removal. Caught by the new Bash
39+
test suite.
40+
- Fix the behavioral-evaluation runner's Codex sandbox isolation: `CODEX_HOME`
41+
was left pointing at the real user's `.codex` directory even though `HOME`
42+
and `USERPROFILE` were overridden to a disposable sandbox, and the
43+
disposable `.codex` directory was never created. Both meant Codex could
44+
read real global state during a nominally isolated eval run. Fixed by
45+
pointing `CODEX_HOME` at the disposable home and creating that directory
46+
before invocation. A fully empty disposable `CODEX_HOME` cannot
47+
authenticate, so the runner now seeds it with a copy of the real
48+
`auth.json` only - no other real Codex state (skills, session history,
49+
config) is copied in.
2550

2651
These changes remain unreleased. They do not select a release version, create a
2752
tag, or imply publication.

README.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,13 @@ representative multi-case subset command is implemented.
224224

225225
The runner keeps disposable fixtures and raw traces outside the repository by
226226
default. It copies the canonical skill into disposable user-level and
227-
project-local locations and records content hashes. Runtime skill precedence
228-
remains observable evidence: if Codex selects another installed copy, the trace
229-
must be treated as an isolation failure rather than a behavior result.
227+
project-local locations and records content hashes. It also points `CODEX_HOME`
228+
at the disposable home rather than the real one, seeding it with a copy of the
229+
real `auth.json` only so live runs can authenticate without inheriting any
230+
other real Codex state (registered skills, session history, config). Runtime
231+
skill precedence remains observable evidence: if Codex selects another
232+
installed copy, the trace must be treated as an isolation failure rather than
233+
a behavior result.
230234

231235
Observable limits are intentional and documented:
232236

scripts/install-package.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ mkdir -p "$parent"
115115
stage="$(mktemp -d "$parent/.${leaf}.stage.XXXXXX")"
116116
backup="$parent/.${leaf}.backup.$$"
117117
moved_existing=false
118-
cleanup() { [ -d "$stage" ] && rm -rf "$stage"; }
118+
cleanup() { if [ -d "$stage" ]; then rm -rf "$stage"; fi; }
119119
trap cleanup EXIT
120120

121121
copy_managed "$SOURCE_DIR" "$stage"
@@ -133,5 +133,5 @@ if ! mv "$stage" "$DEST_DIR" || ! validate_package "$DEST_DIR" "Installed"; then
133133
exit 1
134134
fi
135135

136-
[ "$moved_existing" = true ] && rm -rf "$backup"
136+
if [ "$moved_existing" = true ]; then rm -rf "$backup"; fi
137137
if [ "$CLEAN" = true ]; then echo "Clean reinstall complete."; else echo "Installation complete."; fi

scripts/run-behavioral-evals.mjs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,9 +276,9 @@ async function runLiveCase(args) {
276276
env_overrides: {
277277
USERPROFILE: isolatedHomeRoot,
278278
HOME: isolatedHomeRoot,
279-
CODEX_HOME: process.env.CODEX_HOME || path.join(process.env.USERPROFILE || os.homedir(), ".codex"),
279+
CODEX_HOME: path.join(isolatedHomeRoot, ".codex"),
280280
},
281-
fallback_prevention: "The runner sets USERPROFILE and HOME to a disposable home containing only the copied codebase-orient skill, and passes --ignore-user-config to codex exec.",
281+
fallback_prevention: "The runner sets USERPROFILE, HOME, and CODEX_HOME to a disposable home containing only the copied codebase-orient skill, and passes --ignore-user-config to codex exec. The disposable CODEX_HOME is seeded with a copy of the real auth.json only, so live cases can authenticate without inheriting any other real Codex state such as registered skills or session history.",
282282
},
283283
snapshots: {
284284
initial: initialSnapshots,
@@ -729,6 +729,7 @@ async function writeRunnerFailure({
729729

730730
async function prepareIsolatedSkillHome(homeRoot, fixtureRoot) {
731731
await resetDirectory(homeRoot);
732+
await seedIsolatedCodexHome(homeRoot);
732733
const isolatedSkillDir = path.join(homeRoot, ".agents", "skills", "codebase-orient");
733734
await copyDirectory(canonicalSkillDir, isolatedSkillDir);
734735
const fixtureSkillDir = path.join(fixtureRoot, ".agents", "skills", "codebase-orient");
@@ -751,6 +752,21 @@ async function prepareIsolatedSkillHome(homeRoot, fixtureRoot) {
751752
};
752753
}
753754

755+
async function seedIsolatedCodexHome(homeRoot) {
756+
const isolatedCodexHome = path.join(homeRoot, ".codex");
757+
await fsp.mkdir(isolatedCodexHome, { recursive: true });
758+
const realCodexHome = path.join(process.env.USERPROFILE || os.homedir(), ".codex");
759+
const realAuthPath = path.join(realCodexHome, "auth.json");
760+
try {
761+
await fsp.copyFile(realAuthPath, path.join(isolatedCodexHome, "auth.json"));
762+
} catch (error) {
763+
throw new Error(
764+
`Unable to seed isolated Codex auth from ${realAuthPath}: ${error.message}. ` +
765+
"Live cases require an authenticated local Codex install; only auth.json is copied, no other real Codex state.",
766+
);
767+
}
768+
}
769+
754770
async function createFixtureRepo(fixtureType, fixtureRoot) {
755771
await resetDirectory(fixtureRoot);
756772
await runGit(["init"], fixtureRoot);
@@ -791,7 +807,7 @@ async function invokeCodexExec({
791807
...process.env,
792808
USERPROFILE: isolatedHomeRoot,
793809
HOME: isolatedHomeRoot,
794-
CODEX_HOME: process.env.CODEX_HOME || path.join(process.env.USERPROFILE || os.homedir(), ".codex"),
810+
CODEX_HOME: path.join(isolatedHomeRoot, ".codex"),
795811
};
796812
const commandArgs = [
797813
"exec",

scripts/test-installers.sh

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
#!/usr/bin/env bash
2+
set -euo pipefail
3+
4+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
5+
HELPER="$SCRIPT_DIR/install-package.sh"
6+
REPO_ROOT="$(dirname "$SCRIPT_DIR")"
7+
TEST_ROOT="$(mktemp -d)"
8+
9+
cleanup() { rm -rf "$TEST_ROOT"; }
10+
trap cleanup EXIT
11+
12+
assert_throws() {
13+
local message="$1"
14+
shift
15+
if "$@" >/dev/null 2>&1; then
16+
echo "$message" >&2
17+
exit 1
18+
fi
19+
}
20+
21+
for package in codebase-orient install-codebase-orient; do
22+
source_dir="$REPO_ROOT/skills/$package"
23+
dest="$TEST_ROOT/$package-dest"
24+
25+
bash "$HELPER" "$package" "$source_dir" "$dest" >/dev/null
26+
assert_throws "Normal install did not refuse existing $package destination." \
27+
bash "$HELPER" "$package" "$source_dir" "$dest"
28+
29+
printf 'preserve' > "$dest/extra.txt"
30+
bash "$HELPER" "$package" "$source_dir" "$dest" --force >/dev/null
31+
if [ ! -f "$dest/extra.txt" ]; then
32+
echo "Overlay reinstall deleted an extra $package file." >&2
33+
exit 1
34+
fi
35+
36+
bash "$HELPER" "$package" "$source_dir" "$dest" --clean >/dev/null
37+
if [ -f "$dest/extra.txt" ]; then
38+
echo "Clean reinstall retained an extra $package file." >&2
39+
exit 1
40+
fi
41+
42+
assert_throws 'Conflicting flags were accepted.' \
43+
bash "$HELPER" "$package" "$source_dir" "$dest" --force --clean
44+
45+
rm -f "$dest/SKILL.md"
46+
mkdir -p "$dest/SKILL.md"
47+
assert_throws 'Overlay managed type conflict was accepted.' \
48+
bash "$HELPER" "$package" "$source_dir" "$dest" --force
49+
50+
rm -rf "$dest"
51+
mkdir -p "$dest"
52+
printf 'keep' > "$dest/sentinel.txt"
53+
bad_source="$TEST_ROOT/$package-bad-source"
54+
cp -r "$source_dir" "$bad_source"
55+
printf '\ncorrupt\n' >> "$bad_source/SKILL.md"
56+
assert_throws 'Malformed source package was accepted.' \
57+
bash "$HELPER" "$package" "$bad_source" "$dest" --clean
58+
if [ ! -f "$dest/sentinel.txt" ]; then
59+
echo "Source validation failure mutated the destination." >&2
60+
exit 1
61+
fi
62+
done
63+
64+
echo "Installer tests passed."

0 commit comments

Comments
 (0)