diff --git a/docs/first-run-dialogs.md b/docs/first-run-dialogs.md new file mode 100644 index 0000000..f2a1dcd --- /dev/null +++ b/docs/first-run-dialogs.md @@ -0,0 +1,133 @@ +# First-run dialogs on a new session + +A new Claude Code or Codex session could stop at a dialog before running the +task it was launched with. Nobody is watching a pane the manager or another +agent just opened, and a freshly booted CLI reports `idle`, so a coordinator +polling the session saw "finished" while the work had not started. + +Everything below was measured by launching the installed CLIs — Claude Code +2.1.232, Codex 0.149.0 — against copied credentials in brand-new folders. + +## What a new session hits + +| CLI | dialog | when | keyed on | recorded in | +|---|---|---|---|---| +| both | do you trust this folder? | any new folder | absolute path | Claude: `projects[].hasTrustDialogAccepted` in `.claude.json`. Codex: `[projects.""] trust_level` in `config.toml` | +| Codex | **Update available! → Update now / Skip / Skip until next version** | a launch **with no prompt**, when the cache holds a newer version | the version cache | `dismissed_version` in `version.json` | +| Claude | managed settings / telemetry approval | when the managed settings change | a hash of those settings | `remote-settings-consent.json` | +| Claude | "running in Bypass Permissions mode" | until answered | global | `skipDangerousModePermissionPrompt` in `settings.json` | + +### Trust inheritance differs per CLI — do not generalise + +- **Claude Code 2.1.232 inherits.** With only `` marked + `hasTrustDialogAccepted: true`, a brand-new `/child` ran its launch task + with no dialog. That is why its answer can be one boot-time entry on the + workspaces root instead of a write per session. +- **Codex 0.149.0 does not.** The same shape — parent trusted, child new — + showed the trust dialog. Codex needs the exact path. + +An earlier version of this document said neither inherits. That was wrong, and +it was wrong in the expensive direction: it justified writing CLI state on every +session start when Claude needed no runtime write at all. + +### The Codex update screen is real, and an earlier version of this document ruled it out + +It does not appear when a task is passed on the command line, which is how every +run in the first investigation was done — so the first pass concluded, wrongly, +that the operator's "do you want to update now" must have been one of the trust +dialogs. It is its own dialog. Launch `codex` with **no prompt** against a cache +holding a newer version and it blocks before the session: + +``` +✨ Update available! 0.149.0 -> 999.0.0 +› 1. Update now (runs `npm install -g @openai/codex`) + 2. Skip + 3. Skip until next version + Press enter to continue +``` + +That is exactly a session created without a task — the operator clicks "new +agent", types the first prompt, and it goes into the dialog. `DISABLE_AUTOUPDATER` +does not suppress it and there is no CLI flag or env switch for it. + +## What happens to a prompt while a dialog is up + +- **A launch argument survives.** It is queued and runs once the dialog is + answered, on both CLIs. +- **A prompt sent while the dialog is showing is partly eaten.** The dialog's + key handler consumes the leading characters and the trailing Enter dismisses + it, releasing the queued launch prompt. `reply with exactly SECOND` arrived as + `with exactly SECOND`. +- **`waitForInputReady` cannot help.** It waits for the screen to go quiet, and + a dialog is a quiet screen. + +So the answer has to be that the question was already answered. + +## The fix, and why each part is where it is + +Two rules, both learned from a review that reproduced a silent data loss in the +first attempt: **never rewrite CLI-owned state on the session path**, and when +boot has to write, write once and only when the answer is missing. + +| what | where | why there | +|---|---|---| +| Claude folder trust | **boot**, one entry on the workspaces root | it inherits, so this covers every session; boot runs before the app spawns anything | +| Claude bypass warning | **boot**, `settings.json` | that file is already written by the app's hooks installer, so it is not CLI-owned state | +| Codex folder trust | **per launch**, appended to `config.toml` | Codex neither inherits nor honours a `-c` override for trust, so the answer must be in the file — but it is **appended**, never rewritten, and an existing entry is found by parsing the TOML rather than matching text | +| a session whose path escapes the root | **refused at launch** | `path` is only checked lexically when recorded, so a symlink under a workspace can point out of the tree. Claude applies inherited trust to the RESOLVED directory, so an escaping link lands outside the one trusted root and stops on the dialog. Refusing keeps the boundary the recorded path already implies | +| Codex update prompt | **per launch**, `dismissed_version` | the cache is refreshed in the background hours after boot, so once at boot is not enough | + +### Why append instead of rewrite + +The first attempt read all of `.claude.json`, changed one entry, and renamed the +result over the live file on every new session. A rename prevents a *torn* file; +it is not a lock. A review reproduced the loss deterministically — a concurrent +CLI write vanished, and because nothing ends up malformed, nothing downstream +ever notices. + +An append cannot do that: it never writes another process's bytes. The worst +case is that our own few bytes are lost if Codex rewrites the file at the same +instant, and the only consequence is the dialog appearing once more. + +**One place still rewrites a whole file, and it is stated rather than hidden.** +Setting a field in `version.json` means writing the JSON object back, and that +write can lose a refresh Codex performs in the same instant — measured, not +theoretical: with a review's repro the interceptor fires, our rename lands after +it, and the newer `latest_version` is gone. A compare-and-set after the rename +does not help, because by then the file holds our own snapshot. + +That is accepted here because of what the file is: a cache Codex rewrites on its +own schedule. Losing one refresh means the operator learns about an update at the +next background check rather than this one, and the modal may appear once. The +guarantee the code and its test actually make is the narrow one — the file ends +up self-consistent, so the blocking modal cannot open. `.claude.json` holds state +the operator cannot reconstruct, which is why nothing rewrites that one. + +`codexTrustedPaths()` decodes every `[projects.KEY]` header — basic strings, +literal strings (`[projects.'/path']`), and bare keys — because appending a +second table for a path that already has one under a different legal spelling +produces a file Codex refuses to load ("declared twice"). + +### Suppressing a prompt is not the same as never updating + +`dismissCodexUpdatePrompt()` changes **only** `dismissed_version`: it never +disables the check, never edits `latest_version` or `last_checked_at` itself, and +`codex update` still does its job. The version found is logged at boot and +before the launch that dismissed it: + +``` +[first-run] codex 999.0.0 is available (running 0.149.0); its update prompt is +dismissed so it cannot block a session — run `codex update` or rebuild the image +``` + +Nothing here disables update *checking*, and nothing changes what an agent is +allowed to do: `permissions.defaultMode` and Codex's `approval_policy` / +`sandbox_mode` are left as found. + +## Durability + +Both config trees are already carried by `scripts/agent-state.sh`: +`CLAUDE_CONFIG_DIR` ⇄ `$DATA_DIR/state/claude` (restored whole) and `CODEX_HOME` +⇄ `$DATA_DIR/state/codex` (restored whole except databases and caches). The live +copies are on local disk and are wiped every deploy; they are there because the +bridge restores them at boot. Nothing new had to be made durable. diff --git a/server/package-lock.json b/server/package-lock.json index 9b62988..733f33f 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -13,6 +13,7 @@ "cron-parser": "^5.10.0", "express": "^4.19.2", "node-pty": "^1.0.0", + "smol-toml": "^1.8.0", "web-push": "^3.6.7", "ws": "^8.18.0" }, @@ -1030,6 +1031,18 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/smol-toml": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/smol-toml/-/smol-toml-1.8.0.tgz", + "integrity": "sha512-kCZr2V3ch9i00x8zXRhjUNVcjG9ijES5dDudkXvUVCT5QlJNQWElSJdZqyPemffHoLNUYwOcou0Fy+ojN0uHSQ==", + "license": "BSD-3-Clause", + "engines": { + "node": ">= 18" + }, + "funding": { + "url": "https://github.com/sponsors/cyyynthia" + } + }, "node_modules/statuses": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.2.tgz", diff --git a/server/package.json b/server/package.json index 3041a55..ed13dad 100644 --- a/server/package.json +++ b/server/package.json @@ -26,6 +26,7 @@ "cron-parser": "^5.10.0", "express": "^4.19.2", "node-pty": "^1.0.0", + "smol-toml": "^1.8.0", "web-push": "^3.6.7", "ws": "^8.18.0" }, diff --git a/server/src/config.js b/server/src/config.js index b871583..c79746e 100644 --- a/server/src/config.js +++ b/server/src/config.js @@ -46,6 +46,11 @@ export function refreshVersions() { } } +/** The installed version of one CLI, once the boot-time pass has answered. */ +export function cliVersion(id) { + return versionCache.get(id) || null; +} + // tmux is no longer part of the terminal path: sessions are PTYs held by this // process with a libghostty-vt grid as the authoritative screen (see runner.js). @@ -77,6 +82,9 @@ export const isRemote = (cli) => cli === 'remote'; const CODEX_INPUT_SIGNALS = '-c \'tui.notifications=["approval-requested","plan-mode-prompt"]\'' + ' -c \'tui.notification_method="osc9"\'' + ' -c \'tui.notification_condition="always"\''; +// A `-c 'projects."".trust_level="trusted"'` override was tried first and +// does NOT reach Codex's trust check — it reads the config file itself, so the +// answer has to be in the file (see trustCodexWorkspace in first-run.js). const codexCommand = (tail = '') => `codex ${CODEX_INPUT_SIGNALS}${tail ? ` ${tail}` : ''}`; export const CLIS = [ diff --git a/server/src/first-run.js b/server/src/first-run.js new file mode 100644 index 0000000..ce52d36 --- /dev/null +++ b/server/src/first-run.js @@ -0,0 +1,226 @@ +// Pre-answering the dialogs a new agent session would otherwise stop at. +// +// Everything here is measured against the installed CLIs rather than read off +// their docs; docs/first-run-dialogs.md records the runs. Three facts shape the +// design, and each one narrows what has to be written at runtime: +// +// 1. Claude Code 2.1.232 INHERITS folder trust from a parent. Trusting the +// workspaces root once therefore covers every session under it, so nothing +// has to be written when a session starts. +// 2. Codex 0.149.0 does NOT inherit, and it reads trust from config.toml +// itself — a `-c` override does not reach the check. So its answer has to +// be written, but it is APPENDED: a few bytes at the end, never a rewrite +// of anybody else's, which is the difference that matters below. +// 3. Codex's blocking "Update available!" screen is driven by its own +// `version.json` cache and is suppressed by that file's `dismissed_version` +// — the same field its "Skip until next version" option writes. +// +// Why that matters: an earlier version of this file read all of `.claude.json`, +// changed one entry and renamed the result over the live file on every new +// session. Rename prevents a torn file; it is not a lock. A review reproduced a +// lost concurrent write deterministically, and the loss is silent because +// nothing ends up malformed. The rule now is: never rewrite CLI-owned state on +// the session path, and when boot has to write, write once and only if needed. +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { parse as tomlParse } from 'smol-toml'; +import { WORKSPACES_DIR } from './config.js'; + +const homeDir = () => process.env.HOME || os.homedir(); + +export const claudeStateFile = () => + path.join(process.env.CLAUDE_CONFIG_DIR || homeDir(), '.claude.json'); +export const codexConfigFile = () => + path.join(process.env.CODEX_HOME || path.join(homeDir(), '.codex'), 'config.toml'); +export const codexVersionFile = () => + path.join(process.env.CODEX_HOME || path.join(homeDir(), '.codex'), 'version.json'); + +function writeAtomic(file, text) { + fs.mkdirSync(path.dirname(file), { recursive: true }); + const tmp = `${file}.am-tmp`; + fs.writeFileSync(tmp, text, { mode: 0o600 }); + fs.renameSync(tmp, file); +} + +/** Read JSON, or null when it is absent or not an object we understand. */ +function readObject(file) { + let raw; + try { + raw = fs.readFileSync(file, 'utf8'); + } catch (e) { + if (e.code === 'ENOENT') return {}; + throw e; + } + const parsed = JSON.parse(raw); + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) return null; + return parsed; +} + +/** + * Trust the workspaces ROOT for Claude, once. + * + * Claude inherits trust downwards, so this answers the folder question for + * every session that will ever run under it — which is the whole reason this + * can be a boot-time write rather than a per-session one. Called before the app + * spawns anything, and it rewrites nothing when the root is already trusted, so + * in practice it writes once in the life of a config. + * + * Returns 'already' | 'written' | 'skipped' so the caller can log honestly. + */ +export function trustWorkspacesRoot(dir = WORKSPACES_DIR) { + const file = claudeStateFile(); + try { + const cfg = readObject(file); + if (cfg === null) { console.warn(`[first-run] ${file} is not an object; leaving it alone`); return 'skipped'; } + if (cfg.projects !== undefined + && (typeof cfg.projects !== 'object' || cfg.projects === null || Array.isArray(cfg.projects))) { + console.warn(`[first-run] ${file} has a projects field that is not an object; leaving it alone`); + return 'skipped'; + } + const projects = cfg.projects || {}; + if (projects[dir] && projects[dir].hasTrustDialogAccepted === true) return 'already'; + cfg.projects = { + ...projects, + [dir]: { ...(typeof projects[dir] === 'object' && projects[dir] ? projects[dir] : {}), hasTrustDialogAccepted: true }, + }; + writeAtomic(file, `${JSON.stringify(cfg, null, 2)}\n`); + return 'written'; + } catch (e) { + console.warn(`[first-run] could not trust ${dir} for claude: ${e && e.message}`); + return 'skipped'; + } +} + +/** + * The bypass-permissions warning, whose default button is "No, exit" — a blind + * Enter on it quits the session, so it can never be answered by typing. This + * lives in settings.json, which the app already writes (the hooks installer), + * so it is not CLI-owned state. + */ +export function ensureClaudeDialogDefaults() { + const dir = process.env.CLAUDE_CONFIG_DIR; + if (!dir) return false; + const file = path.join(dir, 'settings.json'); + try { + const cfg = readObject(file); + if (cfg === null) { console.warn(`[first-run] ${file} is not an object; leaving it alone`); return false; } + if (cfg.skipDangerousModePermissionPrompt === true) return false; + cfg.skipDangerousModePermissionPrompt = true; + writeAtomic(file, `${JSON.stringify(cfg, null, 2)}\n`); + return true; + } catch (e) { + console.warn(`[first-run] could not write ${file}: ${e && e.message}`); + return false; + } +} + +/** Numeric compare of dotted versions; non-numeric parts sort as 0. */ +export function isNewer(candidate, current) { + const part = (v) => String(v || '').split('.').map((n) => Number.parseInt(n, 10) || 0); + const a = part(candidate); + const b = part(current); + for (let i = 0; i < Math.max(a.length, b.length); i++) { + if ((a[i] || 0) !== (b[i] || 0)) return (a[i] || 0) > (b[i] || 0); + } + return false; +} + +/** + * Codex's update screen blocks a session before it starts — on a launch with no + * prompt, which is exactly how a session created without a task starts. Mark + * the known version as dismissed so the screen does not open. + * + * This deliberately does NOT stop Codex checking. `latest_version` and + * `last_checked_at` are left exactly as Codex wrote them, so the cache still + * records what is available and `codex update` still works; the only thing + * suppressed is the modal. The version found is logged, so "we are behind" is + * something the operator can read rather than something we quietly swallowed. + * + * Writes at most one small file, and only when a newer version is present and + * not already dismissed. + */ +export function dismissCodexUpdatePrompt(currentVersion) { + // Without knowing what we are running, "newer" has no meaning: isNewer(x, + // null) is true, which would dismiss an update on a startup that has not yet + // resolved its versions (refreshVersions() is async and the server serves + // requests before it finishes). Do nothing; a later launch will have it. + if (!currentVersion) return false; + const file = codexVersionFile(); + try { + const cache = readObject(file); + if (cache === null || !cache.latest_version) return false; + if (!isNewer(cache.latest_version, currentVersion)) return false; + if (cache.dismissed_version === cache.latest_version) return false; + writeAtomic(file, `${JSON.stringify({ ...cache, dismissed_version: cache.latest_version })}\n`); + console.log(`[first-run] codex ${cache.latest_version} is available (running ${currentVersion}); ` + + 'its update prompt is dismissed so it cannot block a session — run `codex update` or rebuild the image to take it'); + return true; + } catch (e) { + console.warn(`[first-run] could not read/write ${file}: ${e && e.message}`); + return false; + } +} + +/** + * Which paths this config already trusts. + * + * Parsed, not matched. An earlier version looked for a `[projects.""]` + * header with a regex anchored at end-of-line, which missed legal spellings — a + * trailing comment (`[projects."/p"] # note`) and whitespace inside the dotted + * key (`[projects . "/p"]`) among them. Appending a second table for a path that + * is already there produces a file Codex refuses to load ("declared twice"), so + * getting this wrong corrupts the operator's config rather than merely showing a + * dialog. TOML has a grammar; use it. + * + * Returns null when the file cannot be parsed — the caller must then leave it + * alone, because appending to a file we do not understand is how a broken config + * becomes a broken config with our text in it. + */ +export function codexTrustedPaths(text) { + let parsed; + try { + parsed = tomlParse(text); + } catch { + return null; + } + const projects = parsed && typeof parsed === 'object' ? parsed.projects : null; + if (!projects || typeof projects !== 'object' || Array.isArray(projects)) return new Set(); + return new Set(Object.keys(projects)); +} + +/** + * Trust one workspace for Codex. + * + * APPEND-ONLY, and that is the point. The version of this a review rejected read + * all of `.claude.json`, edited it, and renamed the result over the live file — + * silently discarding whatever the CLI had written in between. An append never + * writes another process's bytes: at worst our own few are lost if Codex + * rewrites the file at that instant, and the only consequence is the dialog + * appearing once more. + */ +export function trustCodexWorkspace(dir) { + if (!dir || !path.isAbsolute(dir)) return false; + const file = codexConfigFile(); + try { + let text = ''; + try { + text = fs.readFileSync(file, 'utf8'); + } catch (e) { + if (e.code !== 'ENOENT') throw e; + } + const trusted = codexTrustedPaths(text); + if (trusted === null) { + console.warn(`[first-run] ${file} is not valid TOML; leaving it alone (codex will ask about ${dir})`); + return false; + } + if (trusted.has(dir)) return false; + const key = dir.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.appendFileSync(file, `${text.length && !text.endsWith('\n') ? '\n' : ''}\n[projects."${key}"]\ntrust_level = "trusted"\n`); + return true; + } catch (e) { + console.warn(`[first-run] could not trust ${dir} for codex: ${e && e.message}`); + return false; + } +} diff --git a/server/src/index.js b/server/src/index.js index 748fb23..01f93d5 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -18,6 +18,7 @@ import * as order from './order.js'; import * as demo from './demo.js'; import * as hidden from './hidden.js'; import * as crons from './crons.js'; +import { ensureClaudeDialogDefaults, trustWorkspacesRoot } from './first-run.js'; import { attach, agentInfo, deriveState, stop, stopAll, ensureRunning, sendInput, pasteInput, isRunning, waitForInputReady, capturePane, ghosttyReady, ghosttyError, @@ -65,6 +66,14 @@ hidden.init(); // discovery must refuse to guess. Both installers are non-fatal; the existing // fallback remains available if either cannot be installed. installClaudeRepinHook(); +// The two first-run answers that belong to the whole Space rather than to one +// session: Claude's folder trust, which it inherits from the workspaces root +// down to every session under it, and the bypass-mode warning whose default +// button is "No, exit" (so a blind Enter on it kills the session). Both run +// before anything is spawned, and neither writes if the answer is already +// there. See first-run.js. +trustWorkspacesRoot(); +ensureClaudeDialogDefaults(); // OpenCode's global plugin reports the root session chosen by /new (/clear), // and the next prompt after switching to an existing session. installOpencodeRepinPlugin(); diff --git a/server/src/runner.js b/server/src/runner.js index bbf3ec7..11aede5 100644 --- a/server/src/runner.js +++ b/server/src/runner.js @@ -2,10 +2,11 @@ import os from 'node:os'; import path from 'node:path'; import crypto from 'node:crypto'; import pty from 'node-pty'; +import { dismissCodexUpdatePrompt, trustCodexWorkspace } from './first-run.js'; import fs from 'node:fs'; import fsp from 'node:fs/promises'; import { remoteState, setPaused } from './remote.js'; -import { cliById, isRemote, PORT, STATE_DIR, WORKSPACES_DIR } from './config.js'; +import { cliById, cliVersion, isRemote, PORT, STATE_DIR, WORKSPACES_DIR } from './config.js'; import { update, list } from './sessions.js'; import { captureOpencodeSession, opencodeSessionExists, opencodeSessionInfo, readTrace } from './traces.js'; import { @@ -1842,6 +1843,35 @@ export function ensureRunning(session, cols = 120, rows = 34) { const folder = session.path ?? session.id; const workdir = path.join(WORKSPACES_DIR, folder); fs.mkdirSync(workdir, { recursive: true }); + // A session runs inside the workspaces root, and `path` is only checked + // lexically when it is recorded — so a symlink under a workspace can point + // out of the tree, and an agent can make one. Resolve it and refuse: Claude + // applies inherited trust to the RESOLVED directory, so an escaping link + // would land outside the one trusted root and stop the pane on the trust + // dialog with its task queued behind it. Refusing keeps the boundary the + // recorded path already implies, and needs no per-launch write to fix. + const realRoot = fs.realpathSync(WORKSPACES_DIR); + const realWork = fs.realpathSync(workdir); + if (realWork !== realRoot && !realWork.startsWith(`${realRoot}${path.sep}`)) { + throw new Error(`${folder} resolves to ${realWork}, outside the workspaces root — ` + + 'a session has to run inside it. Point the session at a folder in the tree, ' + + 'or copy what you need into one.'); + } + + // Folder trust is answered before we get here for Claude: one boot-time entry + // on the workspaces root, which it inherits down to every folder under it, so + // nothing is written on this path. Codex neither inherits nor honours a `-c` + // override for trust, so its answer is APPENDED to config.toml here — an + // append cannot discard another process's bytes, which a rewrite can. + // + // Its update screen is the other thing that can block, and only on a launch + // with no prompt. The cache behind it is refreshed in the background hours + // after boot, so it is checked per launch rather than once, as a + // compare-and-set. See first-run.js. + if (session.cli === 'codex') { + trustCodexWorkspace(realWork); + dismissCodexUpdatePrompt(cliVersion('codex')); + } // The login shell knows its own PTY-root pid before any `exec`. Adapters use // this marker to discard nested agent lifecycle events BEFORE they can // overwrite the top-level pane's breadcrumb; runner validation repeats the diff --git a/server/test/first-run.test.mjs b/server/test/first-run.test.mjs new file mode 100644 index 0000000..c4db569 --- /dev/null +++ b/server/test/first-run.test.mjs @@ -0,0 +1,225 @@ +// Pre-answering the first-run dialogs, and the two rules that keep it safe: +// never rewrite CLI-owned state on the session path, and recognise an existing +// answer however it was legally spelled. +// +// Run with: node --test test/first-run.test.mjs +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import test from 'node:test'; +import { spawn } from 'node:child_process'; + +const root = fs.mkdtempSync(path.join(os.tmpdir(), 'first-run-')); +process.env.DATA_DIR = path.join(root, 'data'); +process.env.CLAUDE_CONFIG_DIR = path.join(root, 'claude'); +process.env.CODEX_HOME = path.join(root, 'codex'); +for (const d of [process.env.CLAUDE_CONFIG_DIR, process.env.CODEX_HOME]) fs.mkdirSync(d, { recursive: true }); + +const fr = await import('../src/first-run.js'); +const readJson = (f) => JSON.parse(fs.readFileSync(f, 'utf8')); + +// ---- Claude: one boot-time entry on the root, which it inherits downwards ---- + +test('claude: the workspaces root is trusted, and nothing else is disturbed', () => { + fs.writeFileSync(fr.claudeStateFile(), JSON.stringify({ + hasCompletedOnboarding: true, userID: 'keep-me', + projects: { '/elsewhere': { hasTrustDialogAccepted: true, allowedTools: ['a'] } }, + })); + assert.equal(fr.trustWorkspacesRoot('/data/workspaces'), 'written'); + const cfg = readJson(fr.claudeStateFile()); + assert.equal(cfg.projects['/data/workspaces'].hasTrustDialogAccepted, true); + assert.equal(cfg.hasCompletedOnboarding, true); + assert.equal(cfg.userID, 'keep-me'); + assert.deepEqual(cfg.projects['/elsewhere'].allowedTools, ['a']); +}); + +test('claude: an existing root entry keeps its other keys', () => { + fs.writeFileSync(fr.claudeStateFile(), JSON.stringify({ + projects: { '/data/workspaces': { allowedTools: ['x'] } }, + })); + assert.equal(fr.trustWorkspacesRoot('/data/workspaces'), 'written'); + const entry = readJson(fr.claudeStateFile()).projects['/data/workspaces']; + assert.equal(entry.hasTrustDialogAccepted, true); + assert.deepEqual(entry.allowedTools, ['x']); +}); + +test('claude: already trusted writes nothing at all', () => { + fs.writeFileSync(fr.claudeStateFile(), JSON.stringify({ + projects: { '/data/workspaces': { hasTrustDialogAccepted: true } }, + })); + const before = fs.readFileSync(fr.claudeStateFile(), 'utf8'); + assert.equal(fr.trustWorkspacesRoot('/data/workspaces'), 'already'); + assert.equal(fs.readFileSync(fr.claudeStateFile(), 'utf8'), before); +}); + +test('claude: a file we do not understand is left alone', () => { + fs.writeFileSync(fr.claudeStateFile(), '["not","an","object"]'); + assert.equal(fr.trustWorkspacesRoot('/data/workspaces'), 'skipped'); + assert.equal(fs.readFileSync(fr.claudeStateFile(), 'utf8'), '["not","an","object"]'); + fs.writeFileSync(fr.claudeStateFile(), '{ this is not json'); + assert.equal(fr.trustWorkspacesRoot('/data/workspaces'), 'skipped'); +}); + +// ---- Codex: append-only, and an existing answer is recognised in any spelling ---- + +test('codex: an existing entry is found however TOML legally spells the key', () => { + const paths = fr.codexTrustedPaths([ + '[projects."/basic/string"]', 'trust_level = "trusted"', + "[projects.'/literal/string']", 'trust_level = "trusted"', + ' [projects."/indented"] ', 'trust_level = "trusted"', + '[projects."/with\\"quote"]', 'trust_level = "trusted"', + // the two that an end-of-line regex missed, and that made the file unloadable + '[projects."/with/comment"] # retained comment', 'trust_level = "trusted"', + '[projects . "/spaced/key"]', 'trust_level = "trusted"', + ].join('\n')); + assert.ok(paths.has('/basic/string')); + assert.ok(paths.has('/literal/string'), 'a literal-string key is the same key'); + assert.ok(paths.has('/indented')); + assert.ok(paths.has('/with"quote'), 'escapes are undone before comparing'); + assert.ok(paths.has('/with/comment'), 'a comment after the header does not hide it'); + assert.ok(paths.has('/spaced/key'), 'whitespace inside a dotted key is legal'); +}); + +test('codex: a header with a trailing comment is not duplicated', () => { + const before = '[projects."/work/legal"] # retained comment\ntrust_level = "trusted"\n'; + fs.writeFileSync(fr.codexConfigFile(), before); + assert.equal(fr.trustCodexWorkspace('/work/legal'), false); + assert.equal(fs.readFileSync(fr.codexConfigFile(), 'utf8'), before, 'not one byte added'); +}); + +test('codex: a file we cannot parse is left alone rather than appended to', () => { + const broken = 'this = is = not = toml\n'; + fs.writeFileSync(fr.codexConfigFile(), broken); + assert.equal(fr.codexTrustedPaths(broken), null); + assert.equal(fr.trustCodexWorkspace('/work/anything'), false); + assert.equal(fs.readFileSync(fr.codexConfigFile(), 'utf8'), broken); +}); + +test('codex: a literal-string entry is not duplicated (this made the file unloadable)', () => { + fs.writeFileSync(fr.codexConfigFile(), "[projects.'/work/legal']\ntrust_level = \"trusted\"\n"); + assert.equal(fr.trustCodexWorkspace('/work/legal'), false, 'already answered'); + const text = fs.readFileSync(fr.codexConfigFile(), 'utf8'); + assert.equal((text.match(/\[projects\./g) || []).length, 1, 'exactly one table for that path'); +}); + +test('codex: a new path is appended, and existing bytes are untouched', () => { + const before = 'approval_policy = "never"\nmodel = "gpt-5"\n\n[projects."/old"]\ntrust_level = "trusted"\n'; + fs.writeFileSync(fr.codexConfigFile(), before); + assert.equal(fr.trustCodexWorkspace('/work/new'), true); + const text = fs.readFileSync(fr.codexConfigFile(), 'utf8'); + assert.ok(text.startsWith(before), 'append-only: the original bytes are still the prefix'); + assert.match(text, /\[projects\."\/work\/new"\]\ntrust_level = "trusted"/); +}); + +test('codex: appending twice does not duplicate, and a quote in the path is escaped', () => { + fs.writeFileSync(fr.codexConfigFile(), ''); + assert.equal(fr.trustCodexWorkspace('/work/od"d'), true); + assert.equal(fr.trustCodexWorkspace('/work/od"d'), false); + assert.equal((fs.readFileSync(fr.codexConfigFile(), 'utf8').match(/\[projects\./g) || []).length, 1); +}); + +test('only absolute paths are written', () => { + fs.writeFileSync(fr.codexConfigFile(), ''); + assert.equal(fr.trustCodexWorkspace('relative'), false); + assert.equal(fr.trustCodexWorkspace(''), false); +}); + +// ---- Codex updates: suppress the modal, keep the check ---- + +test('a newer version is dismissed, and the check itself is left intact', () => { + const checked = '2026-08-21T16:00:00.000000000Z'; + fs.writeFileSync(fr.codexVersionFile(), JSON.stringify({ + latest_version: '0.200.0', last_checked_at: checked, dismissed_version: null, + })); + assert.equal(fr.dismissCodexUpdatePrompt('0.149.0'), true); + const cache = readJson(fr.codexVersionFile()); + assert.equal(cache.dismissed_version, '0.200.0', 'the modal will not open'); + assert.equal(cache.latest_version, '0.200.0', 'what is available is still recorded'); + assert.equal(cache.last_checked_at, checked, 'and when it was checked'); +}); + +test('nothing is written when there is no newer version, or it is already dismissed', () => { + fs.writeFileSync(fr.codexVersionFile(), JSON.stringify({ latest_version: '0.149.0', dismissed_version: null })); + assert.equal(fr.dismissCodexUpdatePrompt('0.149.0'), false, 'same version'); + fs.writeFileSync(fr.codexVersionFile(), JSON.stringify({ latest_version: '0.100.0', dismissed_version: null })); + assert.equal(fr.dismissCodexUpdatePrompt('0.149.0'), false, 'older'); + fs.writeFileSync(fr.codexVersionFile(), JSON.stringify({ latest_version: '0.200.0', dismissed_version: '0.200.0' })); + assert.equal(fr.dismissCodexUpdatePrompt('0.149.0'), false, 'already dismissed'); +}); + +test('an unknown running version dismisses nothing', () => { + // A fresh fixture on purpose: with dismissed_version already set this would + // pass for the wrong reason. refreshVersions() is async and the server serves + // requests before it finishes, so cliVersion('codex') really can be null. + fs.writeFileSync(fr.codexVersionFile(), JSON.stringify({ + latest_version: '0.200.0', last_checked_at: 'then', dismissed_version: null, + })); + assert.equal(fr.dismissCodexUpdatePrompt(null), false); + assert.equal(fr.dismissCodexUpdatePrompt(undefined), false); + assert.equal(fr.dismissCodexUpdatePrompt(''), false); + assert.equal(readJson(fr.codexVersionFile()).dismissed_version, null, 'nothing was written'); + assert.equal(fr.isNewer('0.200.0', null), true, 'isNewer alone still says newer — the guard is in the caller'); +}); + +test('versions compare by number, not by string', () => { + assert.equal(fr.isNewer('0.200.0', '0.149.0'), true); + assert.equal(fr.isNewer('0.9.0', '0.10.0'), false, '9 is not newer than 10'); + assert.equal(fr.isNewer('1.0.0', '0.149.0'), true); + assert.equal(fr.isNewer('0.149.0', '0.149.0'), false); +}); + +// ---- the bypass warning, in the file the app already owns ---- + +test('the bypass-mode warning is answered once and grants nothing', () => { + const settings = path.join(process.env.CLAUDE_CONFIG_DIR, 'settings.json'); + fs.writeFileSync(settings, JSON.stringify({ hooks: { SessionStart: ['keep'] } })); + assert.equal(fr.ensureClaudeDialogDefaults(), true); + const cfg = readJson(settings); + assert.equal(cfg.skipDangerousModePermissionPrompt, true); + assert.deepEqual(cfg.hooks.SessionStart, ['keep']); + assert.equal(cfg.permissions, undefined, 'permission mode is not touched'); + assert.equal(fr.ensureClaudeDialogDefaults(), false, 'idempotent'); +}); + +test.after(() => fs.rmSync(root, { recursive: true, force: true })); + +test('a concurrent refresh can be lost, and the modal still cannot open', async () => { + // The review's repro, kept as a test of what actually happens rather than of + // what would be nice. A big cache widens the window and a second process + // rewrites the file the moment our temp file appears — which is what Codex's + // background refresh does. Measured outcome: our rename can land after theirs + // and their newer `latest_version` is gone. A compare-and-set after the rename + // does NOT catch that ordering, because by then the file holds our snapshot. + // + // So the guarantee is deliberately the narrow one: whatever the interleaving, + // the file ends up self-consistent and the blocking modal cannot open. Losing + // one refresh costs a delayed notification, and Codex's next check repairs it. + const file = fr.codexVersionFile(); + fs.writeFileSync(file, JSON.stringify({ + latest_version: '0.200.0', last_checked_at: '10:00', dismissed_version: null, + pad: 'x'.repeat(4 * 1024 * 1024), + })); + const racer = spawn(process.execPath, ['-e', ` + const fs = require('node:fs'); + const tmp = ${JSON.stringify(`${file}.am-tmp`)}; + const deadline = Date.now() + 8000; + (function poll() { + if (fs.existsSync(tmp)) { + return fs.writeFileSync(${JSON.stringify(file)}, JSON.stringify({ + latest_version: '0.201.0', last_checked_at: '11:00', dismissed_version: null, + })); + } + if (Date.now() < deadline) setImmediate(poll); + })(); + `], { stdio: 'ignore' }); + try { + assert.equal(fr.dismissCodexUpdatePrompt('0.149.0'), true); + const cache = readJson(file); + assert.equal(cache.dismissed_version, cache.latest_version, + `the modal opens unless the dismissal matches this file's own latest_version ` + + `(saw ${cache.dismissed_version} against ${cache.latest_version})`); + } finally { + racer.kill(); + } +});