From a6394a21feb18834fd5e7780f9f7f84d2ab49386 Mon Sep 17 00:00:00 2001 From: Andrew Kim Date: Tue, 6 Oct 2026 17:18:21 -0500 Subject: [PATCH 1/8] fix: pre-register Codex workspace trust before spawn --- .../references/harness/codex.md | 4 +- CONTRIBUTING.md | 2 +- bin/fm-codex-trust.sh | 135 ++++++++++++++++++ bin/fm-spawn.sh | 25 +++- bin/fm-test-run.sh | 11 +- docs/verification/runtime-backends.md | 26 ++++ tests/codex-config-fixture.js | 27 ++++ tests/fixtures.sh | 28 ++++ tests/fm-codex-trust.test.sh | 96 +++++++++++++ .../fm-codex-workspace-trust-live-e2e.test.sh | 72 ++++++++++ tests/fm-control-relaunch.test.sh | 10 +- tests/fm-spawn-orca-worktree.test.sh | 5 +- tests/fm-spawn-worktree-settle.test.sh | 4 +- 13 files changed, 430 insertions(+), 15 deletions(-) create mode 100755 bin/fm-codex-trust.sh create mode 100644 tests/codex-config-fixture.js create mode 100755 tests/fm-codex-trust.test.sh create mode 100755 tests/fm-codex-workspace-trust-live-e2e.test.sh diff --git a/.agents/skills/harness-adapters/references/harness/codex.md b/.agents/skills/harness-adapters/references/harness/codex.md index d7d7012f49c..ad88ea0b1f6 100644 --- a/.agents/skills/harness-adapters/references/harness/codex.md +++ b/.agents/skills/harness-adapters/references/harness/codex.md @@ -17,8 +17,8 @@ Verified on 2026-06-11 with codex-cli 0.139.0 unless a fact gives a newer versio | Marker | None; identity comes from ancestry, and `../../../bin/fm-harness.sh` is what keeps a retained foreign `CLAUDECODE` from renaming it. Verified on 2026-09-01 with codex-cli 0.152.0: the pane process is the `node` npm shim and the native `codex` binary runs as its foreground child, so a tool subprocess reaches the native name directly while the shim itself is identified from its script path. | A directory trust dialog appears on the first run for a repository root: "Do you trust the contents of this directory?" -Accept it with Enter and verify the instructions begin processing. -The decision persists for the repository, so later worktrees of the same project skip it. +`bin/fm-spawn.sh` pre-registers the launch directory through `bin/fm-codex-trust.sh`, whose header owns the store, scope, dependency and refusal contract. +Workspace trust does not grant hook trust. ## Hook trust diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fcb3a793f0e..b101663898b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,7 +73,7 @@ Coordinate any workflow rollback with its required-check names so a retired chec It pins one exact shellcheck version and one exact actionlint version and refuses to run under any other. Print the shellcheck pin with `bin/fm-lint.sh --required-version` and the actionlint pin with `bin/fm-lint-workflows.sh --required-version`. Use `bin/fm-install-shellcheck.sh` and `bin/fm-install-actionlint.sh` to install those exact builds locally; each installer's header owns its destination usage and supported platforms. -- Harness-adapter ownership spans detection in `bin/fm-harness.sh`, launch and hook mechanics in `bin/fm-spawn.sh`, spawn-time Claude workspace-trust and external-CLAUDE.md-import pre-approval in `bin/fm-claude-trust.sh`, semantic busy sources and trust gates in `bin/fm-busy-lib.sh`, delivery-only rendered guards in `bin/fm-composer-lib.sh`, cleanup in `bin/fm-teardown.sh`, and facts in the skill tree rooted at `.agents/skills/harness-adapters/SKILL.md`; the `firstmate-coding-guidelines` skill owns the validation policy for checks that depend on those harnesses. +- Harness-adapter ownership spans detection in `bin/fm-harness.sh`, launch and hook mechanics in `bin/fm-spawn.sh`, spawn-time workspace trust in `bin/fm-codex-trust.sh` and Claude workspace-trust and external-CLAUDE.md-import pre-approval in `bin/fm-claude-trust.sh`, semantic busy sources and trust gates in `bin/fm-busy-lib.sh`, delivery-only rendered guards in `bin/fm-composer-lib.sh`, cleanup in `bin/fm-teardown.sh`, and facts in the skill tree rooted at `.agents/skills/harness-adapters/SKILL.md`; the `firstmate-coding-guidelines` skill owns the validation policy for checks that depend on those harnesses. - Changes to runtime session backends (`bin/fm-backend.sh`, `bin/backends/`, and the scripts that dispatch through them) keep current setup and limits in the relevant backend guide and active empirical evidence in [`docs/verification/runtime-backends.md`](docs/verification/runtime-backends.md). - [`docs/documentation-audiences.md`](docs/documentation-audiences.md) and its machine-consumed inventory own prose classification; run `bin/fm-doc-audience-check.sh` after documentation changes. - In Markdown, put each full sentence on its own line. diff --git a/bin/fm-codex-trust.sh b/bin/fm-codex-trust.sh new file mode 100755 index 00000000000..9bb9283099b --- /dev/null +++ b/bin/fm-codex-trust.sh @@ -0,0 +1,135 @@ +#!/usr/bin/env bash +# Pre-register Codex workspace trust before an unattended worker launch. +# Usage: fm-codex-trust.sh +# fm-codex-trust.sh --secondmate-home +# Only an isolated linked worktree of the given project or a home seeded for +# the given secondmate is eligible. No ancestor or primary checkout is trusted. +# Uses node and the installed codex app-server config/read + config/value/write +# API, without starting a model turn. Codex owns TOML parsing, preservation, +# atomic replacement and version-conflict refusal. The exact launch path alone +# is registered in ${CODEX_HOME:-$HOME/.codex}/config.toml; workspace trust never +# changes hook trust. Relative CODEX_HOME, malformed config, explicit untrusted +# entries, symlinked stores and foreign ownership fail closed. Existing trusted +# entries are a no-op. Read-back verifies persistence before reporting success. +# The private stdio server is always stopped; it never uses the shared daemon. +set -u +if [ "${1:-}" = --help ] || [ "${1:-}" = -h ]; then + sed -n '2,14{s/^# \{0,1\}//;p;}' "$0" + exit 0 +fi +command -v node >/dev/null 2>&1 || { echo 'error: Codex trust requires node' >&2; exit 1; } +node - "$@" <<'JS' +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); +const cp = require('node:child_process'); +const readline = require('node:readline'); +const args = process.argv.slice(2); +const refuse = message => { throw new Error(message); }; +const real = value => fs.realpathSync(value); +const gitEnv = Object.fromEntries(Object.entries(process.env).filter(([key]) => !key.startsWith('GIT_'))); +function git(directory, flag) { + return real(path.resolve(directory, cp.execFileSync('git', ['-C', directory, 'rev-parse', flag], + {env: gitEnv, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore']}).trim())); +} +function ownedRegular(file) { + const info = fs.lstatSync(file); + if (!info.isFile() || info.uid !== process.getuid()) refuse(`${file} must be a regular file owned by this user`); +} +function checkStore(store) { + try { ownedRegular(store); } catch (error) { if (error.code !== 'ENOENT') throw error; } +} +let server; +let timer; +let nextId = 0; +const pending = new Map(); +function rpc(method, params) { + return new Promise((resolve, reject) => { + const id = nextId++; + pending.set(id, {resolve, reject}); + server.stdin.write(JSON.stringify({id, method, params}) + '\n'); + }); +} +async function main() { + const secondmate = args.length === 3 && args[0] === '--secondmate-home'; + if (!secondmate && (args.length !== 2 || args[0].startsWith('--'))) + refuse('usage: fm-codex-trust.sh | --secondmate-home '); + const target = real(secondmate ? args[1] : args[0]); + if (!fs.statSync(target).isDirectory() || target === '/' || target === (fs.existsSync(os.homedir()) ? real(os.homedir()) : path.resolve(os.homedir()))) + refuse(`${target} is not a task launch directory`); + if (git(target, '--show-toplevel') !== target) refuse(`${target} is not a checkout root`); + if (secondmate) { + const marker = path.join(target, '.fm-secondmate-home'); + ownedRegular(marker); + if (!args[2] || fs.readFileSync(marker, 'utf8').trim() !== args[2]) + refuse(`${target} is not seeded for secondmate ${args[2]}`); + if (!fs.statSync(path.join(target, 'AGENTS.md')).isFile() || !fs.statSync(path.join(target, 'bin')).isDirectory()) + refuse(`${target} is not a Firstmate home`); + for (const name of ['data', 'state', 'config', 'projects']) { + const child = path.join(target, name); + try { fs.lstatSync(child); } catch (error) { if (error.code === 'ENOENT') continue; throw error; } + if (!fs.statSync(child).isDirectory() || !real(child).startsWith(target + path.sep)) + refuse(`${child} is not a directory inside this home`); + } + } else { + const common = git(target, '--git-common-dir'); + if (git(target, '--absolute-git-dir') === common) + refuse(`${target} is a primary checkout, not an isolated worktree`); + if (common !== git(real(args[1]), '--git-common-dir')) refuse(`${target} is not a worktree of ${args[1]}`); + } + const configured = process.env.CODEX_HOME || path.join(os.homedir(), '.codex'); + if (!path.isAbsolute(configured)) refuse('CODEX_HOME must be absolute so registration and launch read the same store'); + fs.mkdirSync(configured, {recursive: true, mode: 0o700}); + const directory = real(configured); + if (fs.statSync(directory).uid !== process.getuid() || directory === target) + refuse(`${directory} is not an eligible Codex configuration directory`); + const store = path.join(directory, 'config.toml'); + checkStore(store); + server = cp.spawn('codex', ['app-server', '--listen', 'stdio://'], { + cwd: directory, env: {...process.env, CODEX_HOME: directory}, stdio: ['pipe', 'pipe', 'ignore'] + }); + const failPending = error => { for (const item of pending.values()) item.reject(error); pending.clear(); }; + server.on('error', failPending); + server.on('exit', (code, signal) => failPending(new Error(`Codex config server exited (${signal || code})`))); + server.stdin.on('error', failPending); + readline.createInterface({input: server.stdout}).on('line', line => { + let response; + try { response = JSON.parse(line); } catch { failPending(new Error('Codex config server emitted invalid JSON')); return; } + const item = pending.get(response.id); + if (!item) return; + pending.delete(response.id); + if (response.error) item.reject(new Error(response.error.message || JSON.stringify(response.error))); + else item.resolve(response.result); + }); + timer = setTimeout(() => failPending(new Error('Codex config server timed out after 15 seconds')), 15000); + await rpc('initialize', {clientInfo: {name: 'firstmate-workspace-trust', version: '1'}, capabilities: null}); + server.stdin.write(JSON.stringify({method: 'initialized'}) + '\n'); + const read = await rpc('config/read', {includeLayers: true}); + const layer = read.layers?.find(item => item.name.type === 'user' && !item.name.profile && item.name.file === store); + if (!layer || !layer.version || !layer.config || typeof layer.config !== 'object') + refuse('Codex did not report a versioned user config layer for the launch store'); + const entry = layer.config.projects?.[target]; + if (entry?.trust_level === 'untrusted') refuse(`existing entry for ${target} is explicitly untrusted`); + if (entry?.trust_level !== 'trusted') { + checkStore(store); + const written = await rpc('config/value/write', { + keyPath: `projects.${JSON.stringify(target)}.trust_level`, value: 'trusted', + mergeStrategy: 'replace', filePath: store, expectedVersion: layer.version + }); + if (written.status !== 'ok' || written.filePath !== store) refuse('Codex did not confirm the trust write'); + } + checkStore(store); + const verified = await rpc('config/read', {includeLayers: true}); + const persisted = verified.layers?.find(item => item.name.type === 'user' && !item.name.profile && item.name.file === store); + if (persisted?.config?.projects?.[target]?.trust_level !== 'trusted') + refuse('Codex configuration did not retain workspace trust'); + console.log(`trusted: ${target}`); +} +main().catch(error => { + console.error(`error: refusing to pre-register Codex workspace trust: ${error.message}`); + process.exitCode = 1; +}).finally(() => { + clearTimeout(timer); + if (server) { server.stdin.destroy(); server.kill('SIGTERM'); } +}); +JS diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 445d9e13ca0..9da19d873c0 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -454,9 +454,11 @@ # resolver because `cursor` is not the CLI name. A cursor SECONDMATE instead runs # the tracked project-scope .cursor/hooks.json in its own home, whose stop-hook # park owns that home's supervision (docs/supervision-protocols/cursor.md). -# claude is the one harness whose pre-launch setup can REFUSE the spawn: before -# any per-task state exists, and before its worktree .claude/settings.local.json -# hooks are written, every claude launch pre-registers the directory the pane +# Claude and Codex pre-register launch-directory trust and refuse on failure. +# bin/fm-codex-trust.sh owns Codex registration for workers and secondmates. +# Claude registration runs before any per-task state exists and before its +# worktree .claude/settings.local.json hooks are written. Every claude launch +# pre-registers the directory the pane # starts in - the task worktree, or the secondmate home for a --secondmate spawn - # in the launching user's own Claude trust store through bin/fm-claude-trust.sh, # because Claude's interactive workspace-trust dialog gates a folder it has never @@ -4535,6 +4537,20 @@ spawn_assert_agent_worktree # only the worktree shape applies. AGY_TRUST_PREREGISTERED=0 case "$HARNESS" in +codex) + # Trust only this launch directory, never the treehouse root. The helper + # uses Codex's config API and preserves existing configuration. + export CODEX_HOME=${CODEX_HOME:-${HOME:?}/.codex} + if [ "$KIND" = secondmate ]; then + spawn_trust_args=(--secondmate-home "$PROJ_ABS" "$ID") + else + spawn_trust_args=("$WT" "$PROJ_ABS") + fi + if ! "$FM_ROOT/bin/fm-codex-trust.sh" "${spawn_trust_args[@]}" >/dev/null; then + echo "error: could not pre-register Codex workspace trust for $WT; refusing to launch a codex worker that would wedge on the trust dialog; inspect window $T" >&2 + exit 1 + fi + ;; claude*) if [ "$KIND" = secondmate ]; then spawn_trust_args=(--secondmate-home "$PROJ_ABS" "$ID") @@ -5338,6 +5354,9 @@ if [ -n "$WORKER_ACCOUNT" ]; then elif [ "$HARNESS" = claude ] && [ -n "${CLAUDE_CONFIG_DIR:-}" ]; then LAUNCH="CLAUDE_CONFIG_DIR=$(shell_quote "$CLAUDE_CONFIG_DIR") $LAUNCH" fi +if [ "$HARNESS" = codex ]; then + LAUNCH="CODEX_HOME=$(shell_quote "$CODEX_HOME") $LAUNCH" +fi if [ "$KIND" = secondmate ]; then sq_home=$(shell_quote "$PROJ_ABS") sq_primary_home=$(shell_quote "$FM_HOME") diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index 340d57b48cf..b8568eed7be 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -357,7 +357,7 @@ family_for_basename() { fm-cmux-claude-composer-live-e2e.test.sh|\ fm-composer-matrix-live-e2e.test.sh|\ fm-composer-codex-idle-live-e2e.test.sh|\ - fm-codex-continuity-live-e2e.test.sh|fm-codex-hook-layer-live-e2e.test.sh|\ + fm-codex-continuity-live-e2e.test.sh|fm-codex-hook-layer-live-e2e.test.sh|fm-codex-workspace-trust-live-e2e.test.sh|\ fm-grok-continuity-live-e2e.test.sh|\ fm-cursor-primary-live-e2e.test.sh|\ fm-grok-stop-live-e2e.test.sh|fm-harness-adapter-instructions-live-e2e.test.sh|\ @@ -387,7 +387,7 @@ family_for_basename() { fm-control.test.sh|fm-control-relaunch.test.sh|\ fm-herdr-session-cleanup.test.sh|fm-send-resolve-key.test.sh|fm-send-strict.test.sh|\ fm-send-inbox.test.sh|fm-spawn-batch.test.sh|\ - fm-spawn-dispatch-profile.test.sh|fm-claude-trust.test.sh|\ + fm-spawn-dispatch-profile.test.sh|fm-claude-trust.test.sh|fm-codex-trust.test.sh|\ fm-worker-account.test.sh|\ fm-git-strip-ai-trailers.test.sh|\ fm-trace-context-spawn.test.sh|fm-spawn-worktree-settle.test.sh|\ @@ -1627,6 +1627,13 @@ families_for_changed_path() { printf '%s\n' pure-contract-unit printf '%s\n' live-harness-optin ;; + bin/fm-codex-trust.sh) + printf '%s\n' backend-dispatch + printf '%s\n' live-harness-optin + ;; + tests/codex-config-fixture.js) + printf '%s\n' backend-dispatch + ;; bin/fm-spawn.sh|bin/fm-send.sh|bin/fm-harness.sh|\ bin/fm-peek.sh|bin/fm-composer*) printf '%s\n' backend-dispatch diff --git a/docs/verification/runtime-backends.md b/docs/verification/runtime-backends.md index febc2fd770c..ee07e82f921 100644 --- a/docs/verification/runtime-backends.md +++ b/docs/verification/runtime-backends.md @@ -2494,3 +2494,29 @@ Without Firstmate's hooks, Herdr reported the question panel as `blocked`, which This live proof covers the watcher and queue boundary; it does not establish live daemon-consumer delivery. `bin/fm-test-run.sh tests/fm-daemon.test.sh` exercises that consumer routing separately with portable regressions for busy escalation and busy-bookkeeping failures in away and quiet mode. Repeat the hooked-worker check above before publication if watcher or task-inbox busy code changes; `bin/fm-test-run.sh tests/fm-task-inbox.test.sh` refreshes the portable ladder regressions. + +## Codex workspace trust + +Verified on 2026-10-06 with codex-cli 0.159.3 on macOS. +`bin/fm-codex-trust.sh` owns launch-directory registration through Codex's private stdio config API, and `tests/fm-codex-trust.test.sh` covers scope, protocol refusal and the spawn integration without an installed harness. +The token-free live guard uses dummy API-key data and submits no prompt. +It verifies Codex's native TOML preservation and idempotence, then opens the same repository's unregistered primary checkout and registered linked worktree without sending any input. +Only the unregistered checkout displays the folder-trust gate; the registered worktree reaches the composer. +Workspace trust does not approve hooks, and the worker hook-layer posture remains independently covered by `tests/fm-codex-hook-layer-live-e2e.test.sh`. + +Refresh command: + +```sh +bash bin/fm-test-run.sh tests/fm-codex-workspace-trust-live-e2e.test.sh +``` + +Observed output: + +```text +ok - codex codex-cli 0.159.3: unregistered checkout gates; registered worktree reaches composer without input +ok - codex codex-cli 0.159.3: native config API preserves TOML, is idempotent and refuses malformed input +# all fm-codex-workspace-trust-live-e2e checks passed +``` + +The registration point precedes backend launch and is shared by tmux, herdr, zellij, orca and cmux. +Other harnesses do not enter the Codex registration branch. diff --git a/tests/codex-config-fixture.js b/tests/codex-config-fixture.js new file mode 100644 index 00000000000..d629ee72988 --- /dev/null +++ b/tests/codex-config-fixture.js @@ -0,0 +1,27 @@ +#!/usr/bin/env node +// Protocol fixture: JSON config stands in for Codex-owned TOML parsing. +const fs = require('node:fs'), path = require('node:path'), rl = require('node:readline'); +const store = path.join(process.env.CODEX_HOME, 'config.toml'); +rl.createInterface({input: process.stdin}).on('line', line => { + const r = JSON.parse(line); + if (r.id === undefined) return; + try { + const content = fs.existsSync(store) ? fs.readFileSync(store, 'utf8') : '{}'; + const config = JSON.parse(content); + let result = {}; + if (r.method === 'config/read') { + result = {layers: [{name: {type:'user', file:store, profile:null}, version:content, config}]}; + if (process.env.FM_TEST_CODEX_CONFIG_CONFLICT === '1') + fs.writeFileSync(store, JSON.stringify({...config, concurrent_setting: true})); + } + if (r.method === 'config/value/write') { + if (r.params.expectedVersion !== content) throw Error('version conflict'); + const key = JSON.parse(r.params.keyPath.slice(9, -12)); + config.projects ||= {}; config.projects[key] ||= {}; + config.projects[key].trust_level = r.params.value; + fs.writeFileSync(store, JSON.stringify(config)); + result = {status:'ok', filePath:store}; + } + console.log(JSON.stringify({id:r.id, result})); + } catch(error) { console.log(JSON.stringify({id:r.id, error:{message:error.message}})); } +}); diff --git a/tests/fixtures.sh b/tests/fixtures.sh index 559cd661eef..c1e6a067b0c 100755 --- a/tests/fixtures.sh +++ b/tests/fixtures.sh @@ -291,6 +291,31 @@ Exercise the spawn behavior under test. EOF } +# Wrap only Codex's config-only server; preserve the existing worker probe. +fm_test_fake_codex_config() { # + local fakebin=$1 original nodebin + mkdir -p "$fakebin" + if [ -f "$fakebin/codex" ] && grep -q 'FM_CODEX_CONFIG_FIXTURE' "$fakebin/codex"; then + return 0 + fi + original=$(command -v codex || true) + if [ -e "$fakebin/codex" ]; then + mv "$fakebin/codex" "$fakebin/codex-worker" + original="$fakebin/codex-worker" + fi + nodebin=$(command -v node) + cat > "$fakebin/codex" < [extra-exit0-tool...] # Creates /fakebin with the spawn tmux stub, a no-op treehouse, and any # extra exit-0 tools. Echoes the fakebin path. @@ -300,6 +325,7 @@ fm_test_make_spawn_fakebin() { fakebin=$(fm_fakebin "$dir") fm_test_fake_tmux_spawn "$fakebin" fm_fake_exit0 "$fakebin" treehouse "$@" + fm_test_fake_codex_config "$fakebin" printf '%s\n' "$fakebin" } @@ -327,8 +353,10 @@ fm_test_run_spawn() { # so every launch-shape assertion in the suite keeps reading the same command. # A test that needs the set case opts in through FM_TEST_CLAUDE_CONFIG_DIR. local spawn_home=$home/user-home + fm_test_fake_codex_config "$fakebin" mkdir -p "$spawn_home" FM_ROOT_OVERRIDE='' FM_HOME="$home" HOME="$spawn_home" \ + CODEX_HOME="${FM_TEST_CODEX_HOME:-$spawn_home/.codex}" \ CLAUDE_CONFIG_DIR="${FM_TEST_CLAUDE_CONFIG_DIR:-}" \ FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \ FM_PROJECTS_OVERRIDE="$home/projects" FM_CONFIG_OVERRIDE="$home/config" \ diff --git a/tests/fm-codex-trust.test.sh b/tests/fm-codex-trust.test.sh new file mode 100755 index 00000000000..f9e00639c35 --- /dev/null +++ b/tests/fm-codex-trust.test.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# Portable behavior coverage for scoped Codex trust and fail-closed spawns. +set -u +# shellcheck source=tests/fixtures.sh +. "$(dirname "${BASH_SOURCE[0]}")/fixtures.sh" +TMP_ROOT=$(fm_test_tmproot fm-codex-trust) +TRUST="$ROOT/bin/fm-codex-trust.sh" +PROJ="$TMP_ROOT/project" +WT="$TMP_ROOT/worktree" +CONFIG="$TMP_ROOT/codex" +mkdir -p "$CONFIG" "$TMP_ROOT/bin" +fm_test_fake_codex_config "$TMP_ROOT/bin" +export PATH="$TMP_ROOT/bin:$PATH" +fm_git_worktree "$PROJ" "$WT" codex-trust +run_trust() { CODEX_HOME="$CONFIG" "$TRUST" "$@" 2>&1; } +assert_trusted() { + node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[1],"utf8"));process.exit(j.projects[process.argv[2]].trust_level === "trusted" ? 0 : 1)' "$CONFIG/config.toml" "$1" +} +run_trust "$WT" "$PROJ" >/dev/null || fail 'fresh registration failed' +assert_trusted "$WT" || fail 'worktree was not trusted' +cp "$CONFIG/config.toml" "$TMP_ROOT/before" +run_trust "$WT" "$PROJ" >/dev/null || fail 'repeat registration failed' +cmp -s "$CONFIG/config.toml" "$TMP_ROOT/before" || fail 'repeat changed the store' +pass 'fresh worktree trust is idempotent' + +printf '%s\n' '{"model":"example","projects":{"/elsewhere":{"trust_level":"untrusted"}}}' > "$CONFIG/config.toml" +run_trust "$WT" "$PROJ" >/dev/null || fail 'preserving registration failed' +node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[1],"utf8"));if(j.model!=="example"||j.projects["/elsewhere"].trust_level!=="untrusted")process.exit(1)' "$CONFIG/config.toml" +expect_code 0 $? 'unrelated configuration changed' +pass 'existing configuration is preserved' + +mkdir -p "$WT/subdir" "$TMP_ROOT/plain" +for target in "$PROJ" "$WT/subdir" "$TMP_ROOT/plain" "$TMP_ROOT/missing"; do + out=$(run_trust "$target" "$PROJ") + expect_code 1 $? "out-of-scope path accepted: $target: $out" +done +fm_git_worktree "$TMP_ROOT/unrelated" "$TMP_ROOT/unrelated-wt" unrelated +out=$(run_trust "$TMP_ROOT/unrelated-wt" "$PROJ") +expect_code 1 $? "unrelated worktree accepted: $out" +pass 'primary checkout, subdirectories and unrelated paths are refused' + +for content in 'broken = [' "{\"projects\":{\"$WT\":{\"trust_level\":\"untrusted\"}}}"; do + printf '%s\n' "$content" > "$CONFIG/config.toml" + cp "$CONFIG/config.toml" "$TMP_ROOT/before" + out=$(run_trust "$WT" "$PROJ") + expect_code 1 $? "unsafe store accepted: $out" + cmp -s "$CONFIG/config.toml" "$TMP_ROOT/before" || fail 'refusal changed the store' +done +rm "$CONFIG/config.toml" +ln -s "$TMP_ROOT/before" "$CONFIG/config.toml" +out=$(run_trust "$WT" "$PROJ") +expect_code 1 $? "symlinked store accepted: $out" +rm "$CONFIG/config.toml" +out=$(CODEX_HOME=relative "$TRUST" "$WT" "$PROJ" 2>&1) +expect_code 1 $? "relative CODEX_HOME accepted: $out" +pass 'malformed, declined, symlinked and ambiguous stores fail closed' + +# The vendor's optimistic version contract refuses a concurrent edit. +printf '%s\n' '{"operator_setting":true}' > "$CONFIG/config.toml" +out=$(FM_TEST_CODEX_CONFIG_CONFLICT=1 run_trust "$WT" "$PROJ") +expect_code 1 $? "a concurrent edit was overwritten: $out" +node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[1],"utf8"));if(!j.operator_setting||!j.concurrent_setting||j.projects)process.exit(1)' "$CONFIG/config.toml" +expect_code 0 $? 'version conflict lost configuration or recorded trust' +pass 'version conflict refuses registration and retains concurrent changes' + +home="$TMP_ROOT/secondmate" +fm_git_init_commit "$home" +mkdir -p "$home/bin" "$home/data" "$home/state" "$home/config" "$home/projects" +printf 'contract\n' > "$home/AGENTS.md" +printf 'mate-n1\n' > "$home/.fm-secondmate-home" +run_trust --secondmate-home "$home" mate-n1 >/dev/null || fail 'seeded home refused' +assert_trusted "$home" || fail 'seeded home not trusted' +out=$(run_trust --secondmate-home "$home" wrong-n1) +expect_code 1 $? "another secondmate accepted: $out" +pass 'only the named seeded secondmate home is eligible' + +spawn_home="$TMP_ROOT/spawn-home" +fakebin=$(make_spawn_fakebin "$TMP_ROOT/fake") +fm_test_spawn_home "$spawn_home" codex +fm_test_spawn_brief "$spawn_home" trustspawn +launchlog="$TMP_ROOT/launch.log" +out=$(FM_TEST_CODEX_HOME="$CONFIG" FM_FAKE_LAUNCH_LOG="$launchlog" \ + fm_test_run_spawn "$spawn_home" "$WT" "$fakebin" trustspawn "$PROJ" codex --mode no-mistakes --yolo off) +expect_code 0 $? "Codex spawn failed: $out" +assert_grep "CODEX_HOME='$CONFIG'" "$launchlog" 'launch reads a different trust store' +pass 'spawn registers trust and forwards the same CODEX_HOME' +rm "$launchlog" +fm_test_spawn_brief "$spawn_home" refusedspawn +printf 'broken = [\n' > "$CONFIG/config.toml" +out=$(FM_TEST_CODEX_HOME="$CONFIG" FM_FAKE_LAUNCH_LOG="$launchlog" \ + fm_test_run_spawn "$spawn_home" "$WT" "$fakebin" refusedspawn "$PROJ" codex --mode no-mistakes --yolo off) +expect_code 1 $? "spawn launched with unrecordable trust: $out" +assert_contains "$out" 'could not pre-register Codex workspace trust' 'missing refusal diagnostic' +assert_absent "$launchlog" 'refused spawn launched a worker' +pass 'spawn refuses before launching when trust registration fails' +echo '# all fm-codex-trust checks passed' diff --git a/tests/fm-codex-workspace-trust-live-e2e.test.sh b/tests/fm-codex-workspace-trust-live-e2e.test.sh new file mode 100755 index 00000000000..75de1ae3540 --- /dev/null +++ b/tests/fm-codex-workspace-trust-live-e2e.test.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# Token-free live guard: real Codex TOML writes and a fresh-folder TUI +# counterfactual. No prompt is submitted and no real credentials are copied. +set -u +# shellcheck source=tests/fixtures.sh +. "$(dirname "${BASH_SOURCE[0]}")/fixtures.sh" +fm_live_gate default-on FM_CODEX_WORKSPACE_TRUST_LIVE codex node python3 +TMP_ROOT=$(fm_test_tmproot fm-codex-workspace-trust-live) +VERSION=$(codex --version 2>&1) +PROJ="$TMP_ROOT/project" +WT="$TMP_ROOT/worktree" +CONFIG="$TMP_ROOT/codex" +fm_git_worktree "$PROJ" "$WT" codex-trust-live +mkdir -p "$CONFIG" +printf '%s\n' '{"OPENAI_API_KEY":"unused-live-guard"}' > "$CONFIG/auth.json" +printf '# preserve this comment\nmodel_reasoning_effort = "low"\n[projects."/unrelated"]\ntrust_level = "untrusted"\n' > "$CONFIG/config.toml" +CODEX_HOME="$CONFIG" "$ROOT/bin/fm-codex-trust.sh" "$WT" "$PROJ" >/dev/null || fail "codex $VERSION: trust API registration failed" +assert_grep '# preserve this comment' "$CONFIG/config.toml" 'Codex lost an unrelated comment' +assert_grep 'model_reasoning_effort = "low"' "$CONFIG/config.toml" 'Codex lost an unrelated setting' +assert_grep '[projects."/unrelated"]' "$CONFIG/config.toml" 'Codex lost another project' +cp "$CONFIG/config.toml" "$TMP_ROOT/before" +CODEX_HOME="$CONFIG" "$ROOT/bin/fm-codex-trust.sh" "$WT" "$PROJ" >/dev/null || fail "codex $VERSION: idempotent registration failed" +cmp -s "$CONFIG/config.toml" "$TMP_ROOT/before" || fail "codex $VERSION: repeat registration rewrote TOML" + +# A primary checkout of the same repository is deliberately left untrusted. +# Thus the positive cannot pass merely because some broad ancestor is trusted. +python3 - "$VERSION" "$CONFIG" "$PROJ" "$WT" <<'PY' +import fcntl, os, pty, re, select, signal, struct, subprocess, sys, termios, time +version, config, project, worktree = sys.argv[1:] +def capture(directory): + master, slave = pty.openpty() + fcntl.ioctl(slave, termios.TIOCSWINSZ, struct.pack('HHHH', 35, 120, 0, 0)) + env = dict(os.environ, CODEX_HOME=config, TERM='xterm-256color') + env.pop('CODEX_THREAD_ID', None) + process = subprocess.Popen(['codex', '--no-daemon', '--no-alt-screen', '--disable', 'hooks', + '-c', 'check_for_update_on_startup=false', '--dangerously-bypass-approvals-and-sandbox'], + cwd=directory, env=env, stdin=slave, stdout=slave, stderr=slave, start_new_session=True) + os.close(slave) + output = b'' + try: + deadline = time.monotonic() + 8 + while time.monotonic() < deadline: + if select.select([master], [], [], 0.1)[0]: + try: output += os.read(master, 65536) + except OSError: break + finally: + if process.poll() is None: + os.killpg(process.pid, signal.SIGTERM) + try: process.wait(timeout=3) + except subprocess.TimeoutExpired: + os.killpg(process.pid, signal.SIGKILL) + process.wait() + os.close(master) + return re.sub(r'\x1b\[[0-?]*[ -/]*[@-~]', '', output.decode(errors='replace')) +negative = capture(project) +positive = capture(worktree) +trust_markers = ('Folder access', 'Trust and continue', 'Do you trust the contents') +if not any(marker in negative for marker in trust_markers): + sys.exit(f'codex {version}: fresh unregistered checkout did not expose the trust gate: {negative[-2000:]}') +if any(marker in positive for marker in trust_markers): + sys.exit(f'codex {version}: registered worktree still exposed the trust gate: {positive[-2000:]}') +if 'YOLO mode' not in positive or 'Ask Codex' not in positive: + sys.exit(f'codex {version}: registered worktree did not reach the idle composer: {positive[-2000:]}') +print(f'ok - codex {version}: unregistered checkout gates; registered worktree reaches composer without input') +PY +expect_code 0 $? "codex $VERSION: live trust counterfactual failed" +printf 'broken = [\n' > "$CONFIG/config.toml" +out=$(CODEX_HOME="$CONFIG" "$ROOT/bin/fm-codex-trust.sh" "$WT" "$PROJ" 2>&1) +expect_code 1 $? "codex $VERSION: malformed TOML accepted: $out" +assert_grep 'broken = [' "$CONFIG/config.toml" 'malformed TOML was overwritten' +printf 'ok - codex %s: native config API preserves TOML, is idempotent and refuses malformed input\n' "$VERSION" +echo '# all fm-codex-workspace-trust-live-e2e checks passed' diff --git a/tests/fm-control-relaunch.test.sh b/tests/fm-control-relaunch.test.sh index 6400bd33b4a..15b9e4634ca 100755 --- a/tests/fm-control-relaunch.test.sh +++ b/tests/fm-control-relaunch.test.sh @@ -19,8 +19,8 @@ # agent exited. set -u -# shellcheck source=tests/lib.sh -. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# shellcheck source=tests/fixtures.sh +. "$(dirname "${BASH_SOURCE[0]}")/fixtures.sh" # shellcheck source=/dev/null . "$ROOT/bin/fm-control-lib.sh" # shellcheck source=/dev/null @@ -235,10 +235,11 @@ run_control() { # # store (bin/fm-claude-trust.sh), and a relaunch reaches it through fm-control.sh, so this runs against a throwaway HOME; # without it this suite would write the developer's real ~/.claude.json. mkdir -p "$dir/user-home" + fm_test_fake_codex_config "$dir/fakebin" env -u HERDR_ENV -u HERDR_PANE_ID -u HERDR_SESSION -u HERDR_SOCKET_PATH \ -u HERDR_TAB_ID -u HERDR_WORKSPACE_ID \ PATH="$dir/fakebin:$PATH" FM_HOME="$dir/home" FM_FAKE_DIR="$dir/fake" \ - HOME="$dir/user-home" CLAUDE_CONFIG_DIR='' \ + HOME="$dir/user-home" CODEX_HOME="$dir/user-home/.codex" CLAUDE_CONFIG_DIR='' \ FM_SPAWN_NO_GUARD=1 GROK_HOME="$dir/grokhome" \ FM_CONTROL_POLL=0.01 FM_CONTROL_EXIT_WAIT=0.05 FM_CONTROL_LAUNCH_WAIT=0.05 \ FM_REAL_GIT="${FM_REAL_GIT:-}" FM_FAKE_GIT_FAILURE="${FM_FAKE_GIT_FAILURE:-}" \ @@ -257,10 +258,11 @@ run_spawn() { # # store (bin/fm-claude-trust.sh), so it runs against a throwaway HOME; # without it this suite would write the developer's real ~/.claude.json. mkdir -p "$dir/user-home" + fm_test_fake_codex_config "$dir/fakebin" env -u HERDR_ENV -u HERDR_PANE_ID -u HERDR_SESSION -u HERDR_SOCKET_PATH \ -u HERDR_TAB_ID -u HERDR_WORKSPACE_ID \ PATH="$dir/fakebin:$PATH" FM_HOME="$dir/home" FM_FAKE_DIR="$dir/fake" \ - HOME="$dir/user-home" CLAUDE_CONFIG_DIR='' \ + HOME="$dir/user-home" CODEX_HOME="$dir/user-home/.codex" CLAUDE_CONFIG_DIR='' \ FM_SPAWN_NO_GUARD=1 GROK_HOME="$dir/grokhome" \ "$SPAWN" "$@" 2>&1 } diff --git a/tests/fm-spawn-orca-worktree.test.sh b/tests/fm-spawn-orca-worktree.test.sh index 4e78418636a..6071054722f 100755 --- a/tests/fm-spawn-orca-worktree.test.sh +++ b/tests/fm-spawn-orca-worktree.test.sh @@ -78,6 +78,7 @@ esac exit 0 SH chmod +x "$fb/orca" + fm_test_fake_codex_config "$fb" printf '%s\n' "$fb" } @@ -101,7 +102,7 @@ Confirm the launch enters the worktree Orca created for it. EOF fb=$(make_orca_fakebin "$case_dir") - out=$(FM_ROOT_OVERRIDE='' FM_HOME="$home" HOME="$case_dir/user-home" \ + out=$(FM_ROOT_OVERRIDE='' FM_HOME="$home" HOME="$case_dir/user-home" CODEX_HOME="$case_dir/user-home/.codex" \ FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \ FM_PROJECTS_OVERRIDE="$home/projects" FM_CONFIG_OVERRIDE="$home/config" \ FM_SPAWN_NO_GUARD=1 FM_TEST_ORCA_DIR="$case_dir" PATH="$fb:$PATH" \ @@ -151,7 +152,7 @@ EOF echo "terminal=term-1" } > "$home/state/$id.meta" - out=$(FM_ROOT_OVERRIDE='' FM_HOME="$home" HOME="$case_dir/user-home" \ + out=$(FM_ROOT_OVERRIDE='' FM_HOME="$home" HOME="$case_dir/user-home" CODEX_HOME="$case_dir/user-home/.codex" \ FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \ FM_PROJECTS_OVERRIDE="$home/projects" FM_CONFIG_OVERRIDE="$home/config" \ FM_SPAWN_NO_GUARD=1 \ diff --git a/tests/fm-spawn-worktree-settle.test.sh b/tests/fm-spawn-worktree-settle.test.sh index ab4c5c50756..c48ce9a4a65 100755 --- a/tests/fm-spawn-worktree-settle.test.sh +++ b/tests/fm-spawn-worktree-settle.test.sh @@ -104,7 +104,9 @@ EOF run_settle_spawn() { local id=$1 - FM_ROOT_OVERRIDE='' FM_HOME="$HOME_DIR" \ + fm_test_fake_codex_config "$FAKEBIN_DIR" + FM_ROOT_OVERRIDE='' FM_HOME="$HOME_DIR" HOME="$HOME_DIR/user-home" \ + CODEX_HOME="$HOME_DIR/user-home/.codex" \ FM_STATE_OVERRIDE="$HOME_DIR/state" FM_DATA_OVERRIDE="$HOME_DIR/data" \ FM_PROJECTS_OVERRIDE="$HOME_DIR/projects" FM_CONFIG_OVERRIDE="$HOME_DIR/config" \ FM_SPAWN_NO_GUARD=1 TMUX="fake,1,0" \ From cb1f3d0c798752bf13ffe013d8783f329b65d90a Mon Sep 17 00:00:00 2001 From: Andrew Kim Date: Tue, 6 Oct 2026 17:25:49 -0500 Subject: [PATCH 2/8] no-mistakes(document): Point Codex trust docs at spawn pre-registration --- .../harness-adapters/references/common/control-and-recovery.md | 2 +- docs/orca-backend.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.agents/skills/harness-adapters/references/common/control-and-recovery.md b/.agents/skills/harness-adapters/references/common/control-and-recovery.md index 2967f3136f1..ebac9081283 100644 --- a/.agents/skills/harness-adapters/references/common/control-and-recovery.md +++ b/.agents/skills/harness-adapters/references/common/control-and-recovery.md @@ -22,7 +22,7 @@ agy gates every fresh worktree too; the spawn pre-registers it in agy's own stor Cursor suppresses its dialog with launch-time `--trust`, and Muse suppresses its own with `--yolo`. Grok renders a folder-trust gate in a linked worktree, and `references/harness/grok.md` owns how to verify the worker's real location, answer it, and where the decision persists; the project picker is a separate dialog that stays absent when the spawn starts in a git root. Pi gates the fresh-worktree case too, but unlike Claude its dialog is answered with Enter, and `references/harness/pi.md` owns that recipe and where the decision persists. -Codex shows a directory-trust dialog on the first run for a repository root. +Codex gates a fresh directory too; the spawn pre-registers it, and `references/harness/codex.md` owns that registration. Use the tool's exact skill form, or natural language only when no separate command is verified or the form remains uncertain. A successful send or key return is not proof of submission; require the tool-specific postcondition. diff --git a/docs/orca-backend.md b/docs/orca-backend.md index 000782a3536..f5b094c0dae 100644 --- a/docs/orca-backend.md +++ b/docs/orca-backend.md @@ -75,7 +75,7 @@ Reinstall the CLI and rerun; [`verification/runtime-backends.md`](verification/r - Escape is unsupported. - Orca exposes no stable CLI version or protocol marker, so readiness is the compatibility gate rather than a version floor. - Only the verified terminal-handle and worktree result fields are accepted; speculative response shapes are rejected. -- Orca's worktree shape is unverified against the spawn-time Claude workspace-trust check in `bin/fm-claude-trust.sh`, which refuses any path that is not a linked git worktree sharing the project's git common dir, so a claude spawn on Orca fails loudly at that check rather than launching if Orca clones instead of linking. +- Orca's worktree shape is unverified against the spawn-time workspace-trust checks in `bin/fm-claude-trust.sh` and `bin/fm-codex-trust.sh`, which refuse any path that is not a linked git worktree sharing the project's git common dir, so a claude or codex spawn on Orca fails loudly at that check rather than launching if Orca clones instead of linking. ## Regression entry points From e8d02e5b9da7089f40a68e217e0a9a00bd366166 Mon Sep 17 00:00:00 2001 From: Andrew Kim Date: Tue, 6 Oct 2026 17:41:02 -0500 Subject: [PATCH 3/8] no-mistakes(ci): I fixed Greptile ci-1 as the user asked: the Codex trust helper could hang if the Codex server stalled while shutting down. The rule that was broken: once the helper has a result (trusted or refused), it must exit within a fixed time, whatever the Codex server does. Both the success path and the refusal path end in one shared `finally` cleanup in `bin/fm-codex-trust.sh`. That cleanup only closed stdin and sent SIGTERM, so a server that ignored SIGTERM kept its process and stdout open. The helper then never exited, and `fm-spawn.sh` waited forever instead of reporting the refusal. **Fix** (in that one cleanup, which covers both paths): after SIGTERM, a 2-second timer sends SIGKILL and closes the server's stdout. The timer is unref'd, so a server that exits normally adds no delay. I updated the help header to mention the 2-second grace and extended the `--help` sed range by one line to match. **Regression test:** - `tests/codex-config-fixture.js` has a new `FM_TEST_CODEX_CONFIG_STALL` mode. The fixture records its pid, ignores SIGTERM and stays alive. - `tests/fm-codex-trust.test.sh` runs a refusal (broken config) against that stalling fixture. It checks that the helper exits within 10 seconds with code 1 and that the stalled server is no longer running. - With the old cleanup the new test fails ("helper hung on a server that stalled during shutdown"); with the fix it passes. **Verification:** - The full `tests/fm-codex-trust.test.sh` passes, about 11 seconds in total. - `fm-spawn-worktree-settle`, `fm-spawn-orca-worktree` and `fm-control-relaunch` pass. - `shellcheck` is clean on both changed shell files. I did not push or re-run validation; the outer executor owns those phases --- bin/fm-codex-trust.sh | 11 ++++++++--- tests/codex-config-fixture.js | 4 ++++ tests/fm-codex-trust.test.sh | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/bin/fm-codex-trust.sh b/bin/fm-codex-trust.sh index 9bb9283099b..75048c122ad 100755 --- a/bin/fm-codex-trust.sh +++ b/bin/fm-codex-trust.sh @@ -11,10 +11,11 @@ # changes hook trust. Relative CODEX_HOME, malformed config, explicit untrusted # entries, symlinked stores and foreign ownership fail closed. Existing trusted # entries are a no-op. Read-back verifies persistence before reporting success. -# The private stdio server is always stopped; it never uses the shared daemon. +# The private stdio server is always stopped (SIGKILL after a 2-second grace); +# it never uses the shared daemon. set -u if [ "${1:-}" = --help ] || [ "${1:-}" = -h ]; then - sed -n '2,14{s/^# \{0,1\}//;p;}' "$0" + sed -n '2,15{s/^# \{0,1\}//;p;}' "$0" exit 0 fi command -v node >/dev/null 2>&1 || { echo 'error: Codex trust requires node' >&2; exit 1; } @@ -130,6 +131,10 @@ main().catch(error => { process.exitCode = 1; }).finally(() => { clearTimeout(timer); - if (server) { server.stdin.destroy(); server.kill('SIGTERM'); } + if (!server) return; + server.stdin.destroy(); + server.kill('SIGTERM'); + // A server that stalls on shutdown must not hold the verdict hostage. + setTimeout(() => { server.kill('SIGKILL'); server.stdout.destroy(); }, 2000).unref(); }); JS diff --git a/tests/codex-config-fixture.js b/tests/codex-config-fixture.js index d629ee72988..7bcb74e253d 100644 --- a/tests/codex-config-fixture.js +++ b/tests/codex-config-fixture.js @@ -2,6 +2,10 @@ // Protocol fixture: JSON config stands in for Codex-owned TOML parsing. const fs = require('node:fs'), path = require('node:path'), rl = require('node:readline'); const store = path.join(process.env.CODEX_HOME, 'config.toml'); +if (process.env.FM_TEST_CODEX_CONFIG_STALL) { // a server that never finishes shutting down + fs.writeFileSync(process.env.FM_TEST_CODEX_CONFIG_STALL, String(process.pid)); + process.on('SIGTERM', () => {}); setInterval(() => {}, 1000); +} rl.createInterface({input: process.stdin}).on('line', line => { const r = JSON.parse(line); if (r.id === undefined) return; diff --git a/tests/fm-codex-trust.test.sh b/tests/fm-codex-trust.test.sh index f9e00639c35..69435218f6f 100755 --- a/tests/fm-codex-trust.test.sh +++ b/tests/fm-codex-trust.test.sh @@ -63,6 +63,21 @@ node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[ expect_code 0 $? 'version conflict lost configuration or recorded trust' pass 'version conflict refuses registration and retains concurrent changes' +# A server that ignores SIGTERM after the verdict must not keep the refusal waiting. +printf 'broken = [\n' > "$CONFIG/config.toml" +FM_TEST_CODEX_CONFIG_STALL="$TMP_ROOT/stall.pid" run_trust "$WT" "$PROJ" > "$TMP_ROOT/stall.out" & +helper=$! +for _ in $(seq 1 100); do kill -0 "$helper" 2>/dev/null || break; sleep 0.1; done +if kill -0 "$helper" 2>/dev/null; then + kill -9 "$helper" "$(cat "$TMP_ROOT/stall.pid")" 2>/dev/null + fail 'helper hung on a server that stalled during shutdown' +fi +wait "$helper" +expect_code 1 $? "stalled server refusal lost: $(cat "$TMP_ROOT/stall.out")" +kill -0 "$(cat "$TMP_ROOT/stall.pid")" 2>/dev/null && fail 'stalled server was left running' +rm "$CONFIG/config.toml" +pass 'a server stalled on shutdown is killed and the refusal still returns' + home="$TMP_ROOT/secondmate" fm_git_init_commit "$home" mkdir -p "$home/bin" "$home/data" "$home/state" "$home/config" "$home/projects" From e93f05c85c44ac41f2ec842ea9a2e72d0e838072 Mon Sep 17 00:00:00 2001 From: Andrew Kim Date: Wed, 7 Oct 2026 02:52:10 -0500 Subject: [PATCH 4/8] no-mistakes(ci): This PR caused the failures: none of them are infrastructure problems. I made a fix, but I had not confirmed that the failing suites now pass when this result was required. **Cause.** `bin/fm-spawn.sh` now runs `bin/fm-codex-trust.sh` before every codex spawn, and that helper starts `codex app-server`. CI runners have no `codex` binary. So every suite that spawns a codex worker or secondmate without the fixture got `refusing to pre-register Codex workspace trust: spawn codex ENOENT` and refused to launch. The CI logs show this in fm-secondmate-lifecycle-e2e, fm-control-herdr-smoke, fm-remote-secondmate-parent-binding, fm-remote-secondmate-lifecycle-e2e and fm-spawn-compact-adviser-disable. The other failing suites in those shards are fm-secondmate-harness, fm-secondmate-safety, fm-secondmate-restart, fm-wake-queue, fm-remote-secondmate-trace-context and fm-spawn-compact-adviser-disable-remote; their logs don't show the error text, but they also spawn codex. The same gap leaks on a developer machine that has codex: running the suite locally had written a `/tmp` trust entry into the real `~/.codex/config.toml`. **Rule that was broken.** No spawn under test should need an installed codex or write the operator's real Codex store. The previous rounds fixed only four suites one by one, so I fixed it in `tests/lib.sh`, which every suite loads: - I moved `fm_test_fake_codex_config` from `tests/fixtures.sh` into `tests/lib.sh` without changing it. `fixtures.sh` loads `lib.sh`, so existing callers still find it. - When `lib.sh` loads, it creates a temporary directory per suite and exports `CODEX_HOME` pointing into it. It also puts the config-only `codex` wrapper first on PATH. That wrapper answers `codex app-server` with `tests/codex-config-fixture.js` and passes any other command to the real codex if one exists, otherwise exits 0. - `fm_live_gate`, just before its tool checks, removes that wrapper from PATH and restores the operator's original `CODEX_HOME` (or unsets it). Live guards such as fm-codex-workspace-trust-live-e2e therefore still use the real codex, and in CI a live codex guard still skips because codex is absent. Production code is unchanged. **Verification.** - `shellcheck -x tests/lib.sh tests/fixtures.sh` is clean. - I built a PATH without codex to imitate CI. With the unfixed test library, fm-secondmate-lifecycle-e2e reproduces the exact CI error (`spawn codex ENOENT` and "secondmate spawn failed"). - I had not confirmed any pass with the fix. Two runs were still going when this result was due: - The failing suites: fm-secondmate-lifecycle-e2e, fm-secondmate-harness, fm-secondmate-safety, fm-spawn-compact-adviser-disable, fm-secondmate-restart, fm-remote-secondmate-lifecycle-e2e, fm-remote-secondmate-trace-context, fm-spawn-compact-adviser-disable-remote, fm-codex-trust and fm-control-herdr-smoke. - Suites the new setup could affect: fm-live-gate, fm-test-fixture-cleanup, fm-remote-doctor, fm-bootstrap, fm-spawn-orca-worktree, fm-spawn-worktree-settle and fm-control-relaunch. - An earlier batch on a broken simulated PATH (node missing) can't be used as evidence. On that batch, fm-remote-secondmate-parent-binding passed. - I have not run fm-wake-queue with the fix. **Risk.** Non-live suites now always find a `codex` on PATH, even in CI. A suite that checks behaviour when codex is not installed (for example around fm-remote-doctor) could behave differently; fm-remote-doctor is in the pending run, and the rest of the CI matrix still has to confirm. "Behavior portable serial 2" was cancelled, not failed, and is not part of this round's findings --- tests/fixtures.sh | 25 ------------------------ tests/lib.sh | 48 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/tests/fixtures.sh b/tests/fixtures.sh index c1e6a067b0c..f457dd3a1e0 100755 --- a/tests/fixtures.sh +++ b/tests/fixtures.sh @@ -291,31 +291,6 @@ Exercise the spawn behavior under test. EOF } -# Wrap only Codex's config-only server; preserve the existing worker probe. -fm_test_fake_codex_config() { # - local fakebin=$1 original nodebin - mkdir -p "$fakebin" - if [ -f "$fakebin/codex" ] && grep -q 'FM_CODEX_CONFIG_FIXTURE' "$fakebin/codex"; then - return 0 - fi - original=$(command -v codex || true) - if [ -e "$fakebin/codex" ]; then - mv "$fakebin/codex" "$fakebin/codex-worker" - original="$fakebin/codex-worker" - fi - nodebin=$(command -v node) - cat > "$fakebin/codex" < [extra-exit0-tool...] # Creates /fakebin with the spawn tmux stub, a no-op treehouse, and any # extra exit-0 tools. Echoes the fakebin path. diff --git a/tests/lib.sh b/tests/lib.sh index 942f50ce011..91749ea2787 100644 --- a/tests/lib.sh +++ b/tests/lib.sh @@ -285,6 +285,43 @@ trap 'fm_test_cleanup; exit 143' TERM trap 'fm_test_cleanup; exit 129' HUP trap 'fm_test_cleanup; exit 131' QUIT +# Wrap only Codex's config-only server; preserve the existing worker probe. +fm_test_fake_codex_config() { # + local fakebin=$1 original nodebin + mkdir -p "$fakebin" + if [ -f "$fakebin/codex" ] && grep -q 'FM_CODEX_CONFIG_FIXTURE' "$fakebin/codex"; then + return 0 + fi + original=$(command -v codex || true) + if [ -e "$fakebin/codex" ]; then + mv "$fakebin/codex" "$fakebin/codex-worker" + original="$fakebin/codex-worker" + fi + nodebin=$(command -v node) + cat > "$fakebin/codex" </dev/null 2>&1 && continue if [ "$requested" -eq 1 ]; then From 818b4e1238ca358b31556ba4e37de5a467112dee Mon Sep 17 00:00:00 2001 From: Andrew Kim Date: Wed, 7 Oct 2026 03:51:38 -0500 Subject: [PATCH 5/8] no-mistakes(test): Route remote-suite Codex trust into suite-private fixture store --- tests/codex-config-fixture.js | 8 +++++--- .../fm-remote-secondmate-lifecycle-e2e.test.sh | 1 + .../fm-remote-secondmate-parent-binding.test.sh | 1 + .../fm-remote-secondmate-trace-context.test.sh | 1 + ...spawn-compact-adviser-disable-remote.test.sh | 1 + tests/lib.sh | 17 +++++++++++++++++ 6 files changed, 26 insertions(+), 3 deletions(-) diff --git a/tests/codex-config-fixture.js b/tests/codex-config-fixture.js index 7bcb74e253d..ab85d5ac4d1 100644 --- a/tests/codex-config-fixture.js +++ b/tests/codex-config-fixture.js @@ -2,6 +2,8 @@ // Protocol fixture: JSON config stands in for Codex-owned TOML parsing. const fs = require('node:fs'), path = require('node:path'), rl = require('node:readline'); const store = path.join(process.env.CODEX_HOME, 'config.toml'); +// A remote job's env -i resolves the real account store; persist elsewhere. +const file = process.env.FM_TEST_CODEX_STORE || store; if (process.env.FM_TEST_CODEX_CONFIG_STALL) { // a server that never finishes shutting down fs.writeFileSync(process.env.FM_TEST_CODEX_CONFIG_STALL, String(process.pid)); process.on('SIGTERM', () => {}); setInterval(() => {}, 1000); @@ -10,20 +12,20 @@ rl.createInterface({input: process.stdin}).on('line', line => { const r = JSON.parse(line); if (r.id === undefined) return; try { - const content = fs.existsSync(store) ? fs.readFileSync(store, 'utf8') : '{}'; + const content = fs.existsSync(file) ? fs.readFileSync(file, 'utf8') : '{}'; const config = JSON.parse(content); let result = {}; if (r.method === 'config/read') { result = {layers: [{name: {type:'user', file:store, profile:null}, version:content, config}]}; if (process.env.FM_TEST_CODEX_CONFIG_CONFLICT === '1') - fs.writeFileSync(store, JSON.stringify({...config, concurrent_setting: true})); + fs.writeFileSync(file, JSON.stringify({...config, concurrent_setting: true})); } if (r.method === 'config/value/write') { if (r.params.expectedVersion !== content) throw Error('version conflict'); const key = JSON.parse(r.params.keyPath.slice(9, -12)); config.projects ||= {}; config.projects[key] ||= {}; config.projects[key].trust_level = r.params.value; - fs.writeFileSync(store, JSON.stringify(config)); + fs.writeFileSync(file, JSON.stringify(config)); result = {status:'ok', filePath:store}; } console.log(JSON.stringify({id:r.id, result})); diff --git a/tests/fm-remote-secondmate-lifecycle-e2e.test.sh b/tests/fm-remote-secondmate-lifecycle-e2e.test.sh index 5f2fe628daa..c952f3d8dd0 100755 --- a/tests/fm-remote-secondmate-lifecycle-e2e.test.sh +++ b/tests/fm-remote-secondmate-lifecycle-e2e.test.sh @@ -104,6 +104,7 @@ SH chmod +x "$REMOTE_ROOT/bin/tmux" install_remote_herdr_fixture "$REMOTE_ROOT" "$HERDR_STATE" "$HERDR_LOG" \ "$TMP_ROOT/herdr-send-fail" "$TMP_ROOT/herdr.sock" +fm_test_remote_codex_config "$REMOTE_ROOT" git -C "$REMOTE_ROOT" init -q -b main git -C "$REMOTE_ROOT" config user.email test@example.com git -C "$REMOTE_ROOT" config user.name Test diff --git a/tests/fm-remote-secondmate-parent-binding.test.sh b/tests/fm-remote-secondmate-parent-binding.test.sh index c05af505fdc..9caafc59479 100755 --- a/tests/fm-remote-secondmate-parent-binding.test.sh +++ b/tests/fm-remote-secondmate-parent-binding.test.sh @@ -119,6 +119,7 @@ pass "remote provisioning publishes durable parent state before its completion m ) | (cd "$REMOTE_ROOT" && tar -xf -) install_remote_herdr_fixture "$REMOTE_ROOT" "$HERDR_STATE" "$HERDR_LOG" \ "$TMP_ROOT/herdr-send-fail" "$TMP_ROOT/herdr.sock" +fm_test_remote_codex_config "$REMOTE_ROOT" git -C "$REMOTE_ROOT" init -q -b main git -C "$REMOTE_ROOT" config user.email test@example.com git -C "$REMOTE_ROOT" config user.name Test diff --git a/tests/fm-remote-secondmate-trace-context.test.sh b/tests/fm-remote-secondmate-trace-context.test.sh index 9b5573d5e85..5453fba9211 100755 --- a/tests/fm-remote-secondmate-trace-context.test.sh +++ b/tests/fm-remote-secondmate-trace-context.test.sh @@ -92,6 +92,7 @@ SH chmod +x "$REMOTE_ROOT/bin/tmux" install_remote_herdr_fixture "$REMOTE_ROOT" "$HERDR_STATE" "$HERDR_LOG" \ "$TMP_ROOT/herdr-send-fail" "$TMP_ROOT/herdr.sock" +fm_test_remote_codex_config "$REMOTE_ROOT" git -C "$REMOTE_ROOT" init -q -b main git -C "$REMOTE_ROOT" config user.email test@example.com git -C "$REMOTE_ROOT" config user.name Test diff --git a/tests/fm-spawn-compact-adviser-disable-remote.test.sh b/tests/fm-spawn-compact-adviser-disable-remote.test.sh index 8f156d6ab6b..1de0793b56c 100755 --- a/tests/fm-spawn-compact-adviser-disable-remote.test.sh +++ b/tests/fm-spawn-compact-adviser-disable-remote.test.sh @@ -56,6 +56,7 @@ SH chmod +x "$REMOTE_ROOT/bin/tmux" install_remote_herdr_fixture "$REMOTE_ROOT" "$HERDR_STATE" "$HERDR_LOG" \ "$TMP_ROOT/herdr-send-fail" "$TMP_ROOT/herdr.sock" +fm_test_remote_codex_config "$REMOTE_ROOT" git -C "$REMOTE_ROOT" init -q -b main git -C "$REMOTE_ROOT" config user.email test@example.com git -C "$REMOTE_ROOT" config user.name Test diff --git a/tests/lib.sh b/tests/lib.sh index 91749ea2787..6f9e7b44b1a 100644 --- a/tests/lib.sh +++ b/tests/lib.sh @@ -310,6 +310,23 @@ SH chmod +x "$fakebin/codex" } +# fm_test_remote_codex_config : the remote job runs every command +# under env -i with the real account HOME, so neither the suite CODEX_HOME nor +# PATH reaches it. Its child PATH starts at /bin; a codex there +# answers app-server with the fixture, persisting into the suite store. +fm_test_remote_codex_config() { # + local script="$1/bin/codex" + mkdir -p "$1/bin" + cat > "$script" < Date: Wed, 7 Oct 2026 06:47:21 -0500 Subject: [PATCH 6/8] no-mistakes(ci): Behavior portable serial 8" failed because of this PR. The failing suite was tests/fm-secondmate-harness.test.sh, with "not ok - split: no meta written". I fixed it in that test file and it now passes locally. Production code is unchanged. **Cause.** Every codex spawn now runs `bin/fm-codex-trust.sh`, which calls `codex app-server`. `tests/lib.sh` puts a config-only codex wrapper first on PATH for this. But this suite runs its spawns with a minimal PATH (fakebin, BLIND_BIN, then `/usr/bin:/bin:/usr/sbin:/sbin`), so the wrapper drops out. The codex secondmate spawn then can't find codex, refuses to launch and writes no meta. I reproduced the same failure locally, since that minimal PATH has no codex on macOS either. **Rule that was broken.** Every spawn under test must find the config-only codex fixture on its PATH. In this suite, the shared place to fix that is the two spawn fakebin builders: `make_noop_tmux` (used by `spawn_secondmate` and the unverified-harness case) and `make_launch_capturing_tmux` (used by `spawn_secondmate_capture`, the cursor case and the crew case). Both already link node in for the Claude trust registration. **Fix.** Each builder now copies `$FM_TEST_CODEX_ROOT/bin/codex` into its fakebin, next to the node link. It is a copy, not a symlink. My first try used a symlink, and it broke a later test: case C9 writes its own probe to `$fakebin/codex` with `cat >`, which wrote through the symlink and overwrote the shared wrapper. That made the later "fallback ... crew-harness codex" check fail. The other fakebin builder, `make_fake_toolchain`, is used by non-spawn cases, which never fail on a missing codex. **Verification.** - Before the fix, `bash tests/fm-secondmate-harness.test.sh` reproduced the CI failure. - After the fix it exits 0 with all 51 checks passing. - `shellcheck -x` on the suite is clean. - The other 27 suites in shard 8 already passed in CI. I did not re-run them --- tests/fm-secondmate-harness.test.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/fm-secondmate-harness.test.sh b/tests/fm-secondmate-harness.test.sh index 12af2eb17ae..b1dd5dfef70 100755 --- a/tests/fm-secondmate-harness.test.sh +++ b/tests/fm-secondmate-harness.test.sh @@ -478,6 +478,9 @@ SH # registration below needs, so link the real one in rather than presenting a # node-less spawn host no real fleet member looks like. ln -sf "$(command -v node)" "$fakebin/node" + # The same minimal PATH drops lib.sh's config-only codex, which a codex + # spawn's trust registration (bin/fm-codex-trust.sh) runs. + cp "$FM_TEST_CODEX_ROOT/bin/codex" "$fakebin/codex" printf '%s\n' "$fakebin" } @@ -728,6 +731,9 @@ SH # registration below needs, so link the real one in rather than presenting a # node-less spawn host no real fleet member looks like. ln -sf "$(command -v node)" "$fakebin/node" + # The same minimal PATH drops lib.sh's config-only codex, which a codex + # spawn's trust registration (bin/fm-codex-trust.sh) runs. + cp "$FM_TEST_CODEX_ROOT/bin/codex" "$fakebin/codex" printf '%s\n' "$fakebin" } From f0738cca35b8b83fb5d3426ca01c898d79295c5a Mon Sep 17 00:00:00 2001 From: Andrew Kim Date: Fri, 9 Oct 2026 22:40:14 -0500 Subject: [PATCH 7/8] no-mistakes(review): Retry Codex trust write on version conflict, bounded to 3 --- bin/fm-codex-trust.sh | 41 +++++++++++++++++++++-------------- tests/codex-config-fixture.js | 10 +++++---- tests/fm-codex-trust.test.sh | 19 +++++++++++----- 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/bin/fm-codex-trust.sh b/bin/fm-codex-trust.sh index 75048c122ad..49c20ffd668 100755 --- a/bin/fm-codex-trust.sh +++ b/bin/fm-codex-trust.sh @@ -6,7 +6,8 @@ # the given secondmate is eligible. No ancestor or primary checkout is trusted. # Uses node and the installed codex app-server config/read + config/value/write # API, without starting a model turn. Codex owns TOML parsing, preservation, -# atomic replacement and version-conflict refusal. The exact launch path alone +# atomic replacement and version-conflict detection; a conflicting concurrent +# edit is re-read and retried, up to 3 attempts. The exact launch path alone # is registered in ${CODEX_HOME:-$HOME/.codex}/config.toml; workspace trust never # changes hook trust. Relative CODEX_HOME, malformed config, explicit untrusted # entries, symlinked stores and foreign ownership fail closed. Existing trusted @@ -15,7 +16,7 @@ # it never uses the shared daemon. set -u if [ "${1:-}" = --help ] || [ "${1:-}" = -h ]; then - sed -n '2,15{s/^# \{0,1\}//;p;}' "$0" + sed -n '2,16{s/^# \{0,1\}//;p;}' "$0" exit 0 fi command -v node >/dev/null 2>&1 || { echo 'error: Codex trust requires node' >&2; exit 1; } @@ -99,29 +100,37 @@ async function main() { const item = pending.get(response.id); if (!item) return; pending.delete(response.id); - if (response.error) item.reject(new Error(response.error.message || JSON.stringify(response.error))); + if (response.error) item.reject(Object.assign(new Error(response.error.message || JSON.stringify(response.error)), {data: response.error.data})); else item.resolve(response.result); }); timer = setTimeout(() => failPending(new Error('Codex config server timed out after 15 seconds')), 15000); await rpc('initialize', {clientInfo: {name: 'firstmate-workspace-trust', version: '1'}, capabilities: null}); server.stdin.write(JSON.stringify({method: 'initialized'}) + '\n'); - const read = await rpc('config/read', {includeLayers: true}); - const layer = read.layers?.find(item => item.name.type === 'user' && !item.name.profile && item.name.file === store); - if (!layer || !layer.version || !layer.config || typeof layer.config !== 'object') - refuse('Codex did not report a versioned user config layer for the launch store'); - const entry = layer.config.projects?.[target]; - if (entry?.trust_level === 'untrusted') refuse(`existing entry for ${target} is explicitly untrusted`); - if (entry?.trust_level !== 'trusted') { + const userLayer = result => result.layers?.find(item => item.name.type === 'user' && !item.name.profile && item.name.file === store); + for (let attempt = 1; ; attempt++) { + const layer = userLayer(await rpc('config/read', {includeLayers: true})); + if (!layer || !layer.version || !layer.config || typeof layer.config !== 'object') + refuse('Codex did not report a versioned user config layer for the launch store'); + const entry = layer.config.projects?.[target]; + if (entry?.trust_level === 'untrusted') refuse(`existing entry for ${target} is explicitly untrusted`); + if (entry?.trust_level === 'trusted') break; checkStore(store); - const written = await rpc('config/value/write', { - keyPath: `projects.${JSON.stringify(target)}.trust_level`, value: 'trusted', - mergeStrategy: 'replace', filePath: store, expectedVersion: layer.version - }); + let written; + try { + written = await rpc('config/value/write', { + keyPath: `projects.${JSON.stringify(target)}.trust_level`, value: 'trusted', + mergeStrategy: 'replace', filePath: store, expectedVersion: layer.version + }); + } catch (error) { + if (error.data?.config_write_error_code !== 'configVersionConflict') throw error; + if (attempt === 3) refuse(`${store} kept changing during registration; gave up after 3 attempts`); + continue; + } if (written.status !== 'ok' || written.filePath !== store) refuse('Codex did not confirm the trust write'); + break; } checkStore(store); - const verified = await rpc('config/read', {includeLayers: true}); - const persisted = verified.layers?.find(item => item.name.type === 'user' && !item.name.profile && item.name.file === store); + const persisted = userLayer(await rpc('config/read', {includeLayers: true})); if (persisted?.config?.projects?.[target]?.trust_level !== 'trusted') refuse('Codex configuration did not retain workspace trust'); console.log(`trusted: ${target}`); diff --git a/tests/codex-config-fixture.js b/tests/codex-config-fixture.js index ab85d5ac4d1..cd250fe90ff 100644 --- a/tests/codex-config-fixture.js +++ b/tests/codex-config-fixture.js @@ -8,6 +8,7 @@ if (process.env.FM_TEST_CODEX_CONFIG_STALL) { // a server that never finishes sh fs.writeFileSync(process.env.FM_TEST_CODEX_CONFIG_STALL, String(process.pid)); process.on('SIGTERM', () => {}); setInterval(() => {}, 1000); } +let races = Number(process.env.FM_TEST_CODEX_CONFIG_CONFLICT || 0); // reads that a concurrent edit follows rl.createInterface({input: process.stdin}).on('line', line => { const r = JSON.parse(line); if (r.id === undefined) return; @@ -17,11 +18,12 @@ rl.createInterface({input: process.stdin}).on('line', line => { let result = {}; if (r.method === 'config/read') { result = {layers: [{name: {type:'user', file:store, profile:null}, version:content, config}]}; - if (process.env.FM_TEST_CODEX_CONFIG_CONFLICT === '1') - fs.writeFileSync(file, JSON.stringify({...config, concurrent_setting: true})); + if (races > 0 && races--) + fs.writeFileSync(file, JSON.stringify({...config, concurrent_setting: (config.concurrent_setting || 0) + 1})); } if (r.method === 'config/value/write') { - if (r.params.expectedVersion !== content) throw Error('version conflict'); + if (r.params.expectedVersion !== content) + throw Object.assign(Error('version conflict'), {data: {config_write_error_code: 'configVersionConflict'}}); const key = JSON.parse(r.params.keyPath.slice(9, -12)); config.projects ||= {}; config.projects[key] ||= {}; config.projects[key].trust_level = r.params.value; @@ -29,5 +31,5 @@ rl.createInterface({input: process.stdin}).on('line', line => { result = {status:'ok', filePath:store}; } console.log(JSON.stringify({id:r.id, result})); - } catch(error) { console.log(JSON.stringify({id:r.id, error:{message:error.message}})); } + } catch(error) { console.log(JSON.stringify({id:r.id, error:{message:error.message, data:error.data}})); } }); diff --git a/tests/fm-codex-trust.test.sh b/tests/fm-codex-trust.test.sh index 69435218f6f..94cd6340a0f 100755 --- a/tests/fm-codex-trust.test.sh +++ b/tests/fm-codex-trust.test.sh @@ -55,13 +55,22 @@ out=$(CODEX_HOME=relative "$TRUST" "$WT" "$PROJ" 2>&1) expect_code 1 $? "relative CODEX_HOME accepted: $out" pass 'malformed, declined, symlinked and ambiguous stores fail closed' -# The vendor's optimistic version contract refuses a concurrent edit. +# The vendor's optimistic version contract detects a concurrent edit; a fresh +# read retries it, and an edit that keeps racing past the bound is refused. printf '%s\n' '{"operator_setting":true}' > "$CONFIG/config.toml" -out=$(FM_TEST_CODEX_CONFIG_CONFLICT=1 run_trust "$WT" "$PROJ") -expect_code 1 $? "a concurrent edit was overwritten: $out" -node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[1],"utf8"));if(!j.operator_setting||!j.concurrent_setting||j.projects)process.exit(1)' "$CONFIG/config.toml" +out=$(FM_TEST_CODEX_CONFIG_CONFLICT=2 run_trust "$WT" "$PROJ") +expect_code 0 $? "a conflict that cleared on retry was refused: $out" +node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[1],"utf8"));if(!j.operator_setting||j.concurrent_setting!==2)process.exit(1)' "$CONFIG/config.toml" +expect_code 0 $? 'retried registration lost concurrent changes' +assert_trusted "$WT" || fail 'retried registration did not record trust' +pass 'a version conflict that clears is retried and retains concurrent changes' + +printf '%s\n' '{"operator_setting":true}' > "$CONFIG/config.toml" +out=$(FM_TEST_CODEX_CONFIG_CONFLICT=3 run_trust "$WT" "$PROJ") +expect_code 1 $? "a persistent concurrent edit was overwritten: $out" +node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[1],"utf8"));if(!j.operator_setting||j.concurrent_setting!==3||j.projects)process.exit(1)' "$CONFIG/config.toml" expect_code 0 $? 'version conflict lost configuration or recorded trust' -pass 'version conflict refuses registration and retains concurrent changes' +pass 'a version conflict past the bound refuses registration and retains concurrent changes' # A server that ignores SIGTERM after the verdict must not keep the refusal waiting. printf 'broken = [\n' > "$CONFIG/config.toml" From 3ffebb8a0ac369bd09f955edd413a3fc33441f2e Mon Sep 17 00:00:00 2001 From: Andrew Kim Date: Fri, 9 Oct 2026 22:49:56 -0500 Subject: [PATCH 8/8] no-mistakes(test): Retry Codex trust writes lost to concurrent replace --- bin/fm-codex-trust.sh | 15 ++++++--------- tests/codex-config-fixture.js | 5 ++++- tests/fm-codex-trust.test.sh | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/bin/fm-codex-trust.sh b/bin/fm-codex-trust.sh index 49c20ffd668..f4166b8a2c0 100755 --- a/bin/fm-codex-trust.sh +++ b/bin/fm-codex-trust.sh @@ -6,8 +6,8 @@ # the given secondmate is eligible. No ancestor or primary checkout is trusted. # Uses node and the installed codex app-server config/read + config/value/write # API, without starting a model turn. Codex owns TOML parsing, preservation, -# atomic replacement and version-conflict detection; a conflicting concurrent -# edit is re-read and retried, up to 3 attempts. The exact launch path alone +# atomic replacement and version-conflict detection; a conflicting or lost +# concurrent edit is re-read and retried, up to 3 attempts. The exact launch path alone # is registered in ${CODEX_HOME:-$HOME/.codex}/config.toml; workspace trust never # changes hook trust. Relative CODEX_HOME, malformed config, explicit untrusted # entries, symlinked stores and foreign ownership fail closed. Existing trusted @@ -107,14 +107,17 @@ async function main() { await rpc('initialize', {clientInfo: {name: 'firstmate-workspace-trust', version: '1'}, capabilities: null}); server.stdin.write(JSON.stringify({method: 'initialized'}) + '\n'); const userLayer = result => result.layers?.find(item => item.name.type === 'user' && !item.name.profile && item.name.file === store); + // Each attempt's read verifies the previous write: a version conflict or a + // concurrent replace that dropped an acknowledged write is re-read and retried. for (let attempt = 1; ; attempt++) { + checkStore(store); const layer = userLayer(await rpc('config/read', {includeLayers: true})); if (!layer || !layer.version || !layer.config || typeof layer.config !== 'object') refuse('Codex did not report a versioned user config layer for the launch store'); const entry = layer.config.projects?.[target]; if (entry?.trust_level === 'untrusted') refuse(`existing entry for ${target} is explicitly untrusted`); if (entry?.trust_level === 'trusted') break; - checkStore(store); + if (attempt > 3) refuse(`${store} did not retain workspace trust; gave up after 3 attempts`); let written; try { written = await rpc('config/value/write', { @@ -123,16 +126,10 @@ async function main() { }); } catch (error) { if (error.data?.config_write_error_code !== 'configVersionConflict') throw error; - if (attempt === 3) refuse(`${store} kept changing during registration; gave up after 3 attempts`); continue; } if (written.status !== 'ok' || written.filePath !== store) refuse('Codex did not confirm the trust write'); - break; } - checkStore(store); - const persisted = userLayer(await rpc('config/read', {includeLayers: true})); - if (persisted?.config?.projects?.[target]?.trust_level !== 'trusted') - refuse('Codex configuration did not retain workspace trust'); console.log(`trusted: ${target}`); } main().catch(error => { diff --git a/tests/codex-config-fixture.js b/tests/codex-config-fixture.js index cd250fe90ff..89c30a82990 100644 --- a/tests/codex-config-fixture.js +++ b/tests/codex-config-fixture.js @@ -9,6 +9,7 @@ if (process.env.FM_TEST_CODEX_CONFIG_STALL) { // a server that never finishes sh process.on('SIGTERM', () => {}); setInterval(() => {}, 1000); } let races = Number(process.env.FM_TEST_CODEX_CONFIG_CONFLICT || 0); // reads that a concurrent edit follows +let lost = Number(process.env.FM_TEST_CODEX_CONFIG_LOST || 0); // ok writes a concurrent replace drops rl.createInterface({input: process.stdin}).on('line', line => { const r = JSON.parse(line); if (r.id === undefined) return; @@ -27,7 +28,9 @@ rl.createInterface({input: process.stdin}).on('line', line => { const key = JSON.parse(r.params.keyPath.slice(9, -12)); config.projects ||= {}; config.projects[key] ||= {}; config.projects[key].trust_level = r.params.value; - fs.writeFileSync(file, JSON.stringify(config)); + if (lost > 0 && lost--) // another writer's replace, based on the old content, lands after ours + fs.writeFileSync(file, JSON.stringify({...JSON.parse(content), concurrent_setting: (JSON.parse(content).concurrent_setting || 0) + 1})); + else fs.writeFileSync(file, JSON.stringify(config)); result = {status:'ok', filePath:store}; } console.log(JSON.stringify({id:r.id, result})); diff --git a/tests/fm-codex-trust.test.sh b/tests/fm-codex-trust.test.sh index 94cd6340a0f..c00d8351dc0 100755 --- a/tests/fm-codex-trust.test.sh +++ b/tests/fm-codex-trust.test.sh @@ -72,6 +72,23 @@ node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[ expect_code 0 $? 'version conflict lost configuration or recorded trust' pass 'a version conflict past the bound refuses registration and retains concurrent changes' +# Real Codex can acknowledge a write that a concurrent replace then drops; the +# next read notices, and the write is retried within the same bound. +printf '%s\n' '{"operator_setting":true}' > "$CONFIG/config.toml" +out=$(FM_TEST_CODEX_CONFIG_LOST=2 run_trust "$WT" "$PROJ") +expect_code 0 $? "a lost write that cleared on retry was refused: $out" +node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[1],"utf8"));if(!j.operator_setting||j.concurrent_setting!==2)process.exit(1)' "$CONFIG/config.toml" +expect_code 0 $? 'retried lost write dropped concurrent changes' +assert_trusted "$WT" || fail 'retried lost write did not record trust' +pass 'an acknowledged write lost to a concurrent replace is retried' + +printf '%s\n' '{"operator_setting":true}' > "$CONFIG/config.toml" +out=$(FM_TEST_CODEX_CONFIG_LOST=3 run_trust "$WT" "$PROJ") +expect_code 1 $? "a write lost past the bound was reported trusted: $out" +node -e 'const fs=require("fs");const j=JSON.parse(fs.readFileSync(process.argv[1],"utf8"));if(!j.operator_setting||j.concurrent_setting!==3||j.projects)process.exit(1)' "$CONFIG/config.toml" +expect_code 0 $? 'lost write past the bound changed configuration or recorded trust' +pass 'a write lost past the bound refuses registration' + # A server that ignores SIGTERM after the verdict must not keep the refusal waiting. printf 'broken = [\n' > "$CONFIG/config.toml" FM_TEST_CODEX_CONFIG_STALL="$TMP_ROOT/stall.pid" run_trust "$WT" "$PROJ" > "$TMP_ROOT/stall.out" &