From 2fa0888dbf82267e49ec54e0b4b4c73b6777cc96 Mon Sep 17 00:00:00 2001 From: Proactive Runtime Bot Date: Thu, 5 Nov 2026 22:43:47 -0800 Subject: [PATCH 1/2] fix: read verified Codex rollout paths and clarify hook limits --- CHANGELOG.md | 4 + crates/relay-node/docs/relay-core.md | 14 ++ .../src/relay/local/directory_trait.rs | 4 +- .../src/relay/process/codex_hook.rs | 4 +- .../src/relay/process/codex_hook/scaling.rs | 196 ++++++++++++++---- 5 files changed, 176 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70457bfe0..47888def3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `agent-relay-node` can deliver messages to registered VS Code Copilot chats on macOS and resume the same chat while it waits. - Copilot messages with uncertain delivery stay pending confirmation and are never blindly submitted again. +### Changed + +- Codex SessionStart hooks skip the Relay preamble and early auto-activation for multi-chat app-server/IDE sessions and CLI roots holding live subagent locks. + ### Fixed - `relay init`'s Codex hooks keep a subagent from receiving its parent chat's pending messages or registration guidance. diff --git a/crates/relay-node/docs/relay-core.md b/crates/relay-node/docs/relay-core.md index bdd273e4f..4ed876879 100644 --- a/crates/relay-node/docs/relay-core.md +++ b/crates/relay-node/docs/relay-core.md @@ -391,6 +391,20 @@ the bridge socket is for UIs, the relay-send socket for sessions session's identity, and a lock file's recorded pid counts only while that pid still holds the lock. A process inside no supported session is `not_a_relay_session`; another user's is `not_this_user`. +- **Codex hooks.** PostToolUse and Stop bind a nonempty hook `turn_id` to + the requested thread's active turn in its locally resolved, kernel-held + rollout, alongside the trusted writer and captured file-birth checks. + A missing turn ID, unavailable birth-time evidence, or an unresolved rollout + leaves offers and registration guidance pending; the hook client returns + `{}`. Rollout lookup uses the core's configured `CODEX_HOME` (default + `~/.codex`) and `CODEX_SQLITE_HOME`, so a different home set only on a hook + process is not discovered by this route. SessionStart has no turn ID and + runs only when the trusted writer holds that thread alone. Multi-chat + app-server/IDE sessions and CLI roots with live subagents skip the SessionStart + preamble and early auto-activation; those actions are not replayed later. + A later verified PostToolUse can still supply registration guidance. + These are local context-binding checks, not authentication against a + malicious process running as the same user. - **Routes.** `POST /send` and `GET /agents` need the caller's registration; `GET /whoami`, `POST`/`DELETE /register` and the setup routes do not. Every route except `/whoami` counts against the session's 30 requests a diff --git a/crates/relay-node/src/relay/local/directory_trait.rs b/crates/relay-node/src/relay/local/directory_trait.rs index de2b02bcf..300015d8a 100644 --- a/crates/relay-node/src/relay/local/directory_trait.rs +++ b/crates/relay-node/src/relay/local/directory_trait.rs @@ -331,8 +331,8 @@ impl SessionDirectory for LocalSessions { self.config.uid, thread, &path, - || { - codex::rollout::RolloutSummary::read(&path) + |verified_path| { + codex::rollout::RolloutSummary::read(verified_path) .is_some_and(|summary| summary.active_turn.as_deref() == Some(turn)) }, ) diff --git a/crates/relay-node/src/relay/process/codex_hook.rs b/crates/relay-node/src/relay/process/codex_hook.rs index e5ed7e146..987acebc1 100644 --- a/crates/relay-node/src/relay/process/codex_hook.rs +++ b/crates/relay-node/src/relay/process/codex_hook.rs @@ -130,7 +130,7 @@ pub(crate) fn with_codex_hook_rollout( uid: u32, thread: &str, rollout: &Path, - read: impl FnOnce() -> bool, + read: impl FnOnce(&Path) -> bool, ) -> bool { let Some(chain) = writer_chain(table, pid, uid, thread) else { return false; @@ -148,7 +148,7 @@ pub(crate) fn with_codex_hook_rollout( return false; }; holds_rollout(table, writer.pid, &path, birth) - && read() + && read(&path) && file_birth_ns(&path) == Some(birth) && holds_rollout(table, writer.pid, &path, birth) && has_codex_writer_for_caller(table, pid, uid, thread) diff --git a/crates/relay-node/src/relay/process/codex_hook/scaling.rs b/crates/relay-node/src/relay/process/codex_hook/scaling.rs index fc453c281..b5967d3d7 100644 --- a/crates/relay-node/src/relay/process/codex_hook/scaling.rs +++ b/crates/relay-node/src/relay/process/codex_hook/scaling.rs @@ -5,6 +5,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; struct CountingFiles { files: Vec, + after: Option>, scans: AtomicUsize, } @@ -28,8 +29,12 @@ impl ProcessTable for CountingFiles { self.files.iter().map(|file| file.path.clone()).collect() } fn opened_files(&self, _: i32) -> Vec { - self.scans.fetch_add(1, Ordering::Relaxed); - self.files.clone() + let scan = self.scans.fetch_add(1, Ordering::Relaxed); + if scan == 0 { + self.files.clone() + } else { + self.after.as_ref().unwrap_or(&self.files).clone() + } } fn cwd(&self, _: i32) -> Option { None @@ -39,50 +44,59 @@ impl ProcessTable for CountingFiles { } } +const PARENT: &str = "550e8400-e29b-41d4-a716-000000000000"; +const CHILD: &str = "550e8400-e29b-41d4-a716-000000000001"; + +/// Build real rollout headers with captured file births for the snapshot tests. +fn fixture(count: usize) -> anyhow::Result<(tempfile::TempDir, CountingFiles)> { + let temp = tempfile::tempdir()?; + + let mut files = Vec::new(); + for index in 0..count { + let thread = format!("550e8400-e29b-41d4-a716-{index:012}"); + let path = temp.path().join(format!("rollout-fixture-{thread}.jsonl")); + let payload = if index == 1 { + serde_json::json!({"id":thread,"thread_source":"subagent", + "parent_thread_id":PARENT, + "source":{"subagent":{"thread_spawn":{"parent_thread_id":PARENT}}}}) + } else { + serde_json::json!({"id":thread}) + }; + std::fs::write( + &path, + format!( + "{}\n", + serde_json::json!({ + "type":"session_meta","payload":payload + }) + ), + )?; + files.push(OpenedFile { + birth_ns: file_birth_ns(&path), + path, + }); + files.push(OpenedFile { + path: temp + .path() + .join("thread-writer-locks") + .join(format!("{thread}.lock")), + birth_ns: None, + }); + } + let table = CountingFiles { + files, + after: None, + scans: AtomicUsize::new(0), + }; + Ok((temp, table)) +} + #[test] fn hook_rollout_file_scan_count_is_bounded() -> anyhow::Result<()> { let mut counts = Vec::new(); for count in [2, 12] { - let temp = tempfile::tempdir()?; - let parent = "550e8400-e29b-41d4-a716-000000000000"; - let child = "550e8400-e29b-41d4-a716-000000000001"; - let mut files = Vec::new(); - for index in 0..count { - let thread = format!("550e8400-e29b-41d4-a716-{index:012}"); - let path = temp.path().join(format!("rollout-fixture-{thread}.jsonl")); - let payload = if index == 1 { - serde_json::json!({"id":thread,"thread_source":"subagent", - "parent_thread_id":parent, - "source":{"subagent":{"thread_spawn":{"parent_thread_id":parent}}}}) - } else { - serde_json::json!({"id":thread}) - }; - std::fs::write( - &path, - format!( - "{}\n", - serde_json::json!({ - "type":"session_meta","payload":payload - }) - ), - )?; - files.push(OpenedFile { - birth_ns: file_birth_ns(&path), - path, - }); - files.push(OpenedFile { - path: temp - .path() - .join("thread-writer-locks") - .join(format!("{thread}.lock")), - birth_ns: None, - }); - } - let table = CountingFiles { - files, - scans: AtomicUsize::new(0), - }; - assert!(held_descendant_of(&table, 10, child, parent)); + let (_temp, table) = fixture(count)?; + assert!(held_descendant_of(&table, 10, CHILD, PARENT)); counts.push(table.scans.load(Ordering::Relaxed)); } assert_eq!( @@ -92,3 +106,101 @@ fn hook_rollout_file_scan_count_is_bounded() -> anyhow::Result<()> { ); Ok(()) } + +#[test] +fn hook_rollout_second_snapshot_changes_deny() -> anyhow::Result<()> { + for change in ["parent_lock", "rollout", "birth"] { + let (_temp, mut table) = fixture(2)?; + let mut after = table.files.clone(); + match change { + "parent_lock" => { + after.retain(|file| codex_thread_of_lock(&file.path).as_deref() != Some(PARENT)) + } + "rollout" => { + after.retain(|file| rollout_thread_of_path(&file.path).as_deref() != Some(CHILD)) + } + _ => { + for file in &mut after { + if rollout_thread_of_path(&file.path).as_deref() == Some(PARENT) { + file.birth_ns = file.birth_ns.map(|birth| birth + 1); + } + } + } + } + table.after = Some(after); + assert!(!held_descendant_of(&table, 10, CHILD, PARENT), "{change}"); + assert_eq!(table.scans.load(Ordering::Relaxed), 2, "{change}"); + } + Ok(()) +} + +#[cfg(unix)] +#[test] +fn hook_rollout_reader_uses_verified_canonical_path() -> anyhow::Result<()> { + use crate::relay::codex::rollout::RolloutSummary; + let temp = tempfile::tempdir()?; + let rollout = temp.path().join(format!("rollout-fixture-{PARENT}.jsonl")); + let other = temp.path().join("other.jsonl"); + let alias = temp.path().join("alias.jsonl"); + for (path, turn) in [(&rollout, "verified-turn"), (&other, "other-turn")] { + std::fs::write( + path, + format!( + "{}\n", + serde_json::json!({ + "type":"event_msg","payload":{"type":"task_started","turn_id":turn} + }) + ), + )?; + } + std::os::unix::fs::symlink(&rollout, &alias)?; + let canonical = rollout.canonicalize()?; + let table = FakeProcesses::default(); + let executable = temp.path().join("codex"); + table.trust(&executable); + table.insert( + 10, + FakeProcess { + parent: 1, + uid: 501, + start: 10, + executable: Some(executable), + arguments: vec!["codex".into(), "app-server".into()], + open_files: vec![ + rollout, + temp.path() + .join("thread-writer-locks") + .join(format!("{PARENT}.lock")), + ], + ..FakeProcess::default() + }, + ); + table.insert( + 20, + FakeProcess { + parent: 10, + uid: 501, + start: 20, + ..FakeProcess::default() + }, + ); + assert!(with_codex_hook_rollout( + &table, + 20, + 501, + PARENT, + &alias, + |verified_path| { + assert_eq!(verified_path, canonical); + std::fs::remove_file(&alias).unwrap(); + std::os::unix::fs::symlink(&other, &alias).unwrap(); + assert_eq!( + RolloutSummary::read(&alias).unwrap().active_turn.as_deref(), + Some("other-turn") + ); + RolloutSummary::read(verified_path) + .is_some_and(|summary| summary.active_turn.as_deref() == Some("verified-turn")) + } + )); + Ok(()) +} From 00fec7ebdc585c50ed1c75a4ad6e9ab1ddb09c1e Mon Sep 17 00:00:00 2001 From: Proactive Runtime Bot Date: Thu, 5 Nov 2026 22:47:36 -0800 Subject: [PATCH 2/2] test: prove canonical rollout reads against merged baseline --- .../cases/codex-hook-canonical-read/case.json | 15 +++ .../cases/codex-hook-canonical-read/run.mjs | 106 ++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 tests/relayflows/cases/codex-hook-canonical-read/case.json create mode 100644 tests/relayflows/cases/codex-hook-canonical-read/run.mjs diff --git a/tests/relayflows/cases/codex-hook-canonical-read/case.json b/tests/relayflows/cases/codex-hook-canonical-read/case.json new file mode 100644 index 000000000..903805d67 --- /dev/null +++ b/tests/relayflows/cases/codex-hook-canonical-read/case.json @@ -0,0 +1,15 @@ +{ + "version": 1, + "id": "codex-hook-canonical-read", + "kind": "bugfix", + "title": "Read the verified canonical Codex rollout after an alias changes", + "runner": { + "command": ["node", "tests/relayflows/cases/codex-hook-canonical-read/run.mjs"] + }, + "requirements": [], + "timeoutSeconds": 900, + "expected": { + "base": { "outcome": "bug", "signature": "rollout_reader_follows_retargeted_alias" }, + "head": { "outcome": "fixed", "signature": "rollout_reader_uses_verified_canonical_path" } + } +} diff --git a/tests/relayflows/cases/codex-hook-canonical-read/run.mjs b/tests/relayflows/cases/codex-hook-canonical-read/run.mjs new file mode 100644 index 000000000..c8fbea466 --- /dev/null +++ b/tests/relayflows/cases/codex-hook-canonical-read/run.mjs @@ -0,0 +1,106 @@ +#!/usr/bin/env node +import { execFileSync, spawnSync } from 'node:child_process'; +import { mkdir, readFile, rm, writeFile } from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const caseId = 'codex-hook-canonical-read'; +const arm = process.env.RELAY_PR_PROOF_ARM; +const target = process.env.RELAY_PR_PROOF_TARGET_DIR; +const resultPath = process.env.RELAY_PR_PROOF_RESULT_PATH; +const head = process.env.RELAY_PR_PROOF_HEAD_SHA; +const expected = process.env[arm === 'base' ? 'RELAY_PR_PROOF_BASE_SHA' : 'RELAY_PR_PROOF_HEAD_SHA']; +const harness = + process.env.RELAY_PR_PROOF_HARNESS_DIR ?? + path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../../../..'); +const sha = (cwd) => execFileSync('git', ['rev-parse', 'HEAD'], { cwd, encoding: 'utf8' }).trim(); +if (!['base', 'head'].includes(arm) || !target || !resultPath || !expected || !head) { + throw new Error('RelayFlow proof environment is incomplete'); +} +await rm(resultPath, { force: true }); +for (const [cwd, revision] of [ + [target, expected], + [harness, head], +]) { + if (sha(cwd) !== revision) throw new Error('Target or harness is not the exact expected revision'); + const clean = spawnSync('git', ['diff', '--quiet', revision, '--'], { cwd }); + if (clean.error) throw clean.error; + if (clean.status !== 0) throw new Error('Target or harness has modified tracked inputs'); +} + +const fixturePath = 'crates/relay-node/src/relay/process/codex_hook/scaling.rs'; +const fixtureFile = path.join(target, fixturePath); +const original = await readFile(fixtureFile); +let fixture = execFileSync('git', ['show', `${head}:${fixturePath}`], { cwd: harness, encoding: 'utf8' }); +if (arm === 'base') { + // Test-only ABI adaptation: the old FnOnce() reader captures its original + // alias, exactly as LocalSessions did. The new FnOnce(&Path) reader receives + // the verified canonical path. Neither arm changes production code. + const callback = '|verified_path| {'; + const assertion = 'assert_eq!(verified_path, canonical);'; + const canonical = 'let canonical = rollout.canonicalize()?;'; + for (const text of [callback, assertion, canonical]) { + if (fixture.split(text).length !== 2) throw new Error('Unexpected canonical-read fixture shape'); + } + fixture = fixture + .replace(callback, '|| { let verified_path = &alias;') + .replace(assertion, '') + .replace(canonical, ''); +} +let result; +try { + await writeFile(fixtureFile, fixture); + result = spawnSync( + 'cargo', + [ + 'test', + '--manifest-path', + 'crates/relay-node/Cargo.toml', + '--lib', + 'hook_rollout_reader_uses_verified_canonical_path', + ], + { + cwd: target, + env: { ...process.env, CARGO_TERM_COLOR: 'never' }, + encoding: 'utf8', + maxBuffer: 16 * 1024 * 1024, + timeout: 12 * 60 * 1000, + } + ); + process.stdout.write(`${result.stdout ?? ''}${result.stderr ?? ''}`); + if (result.error) throw result.error; +} finally { + await writeFile(fixtureFile, original); +} +const output = `${result.stdout ?? ''}${result.stderr ?? ''}`; +const named = (status) => + new RegExp( + String.raw`\btest\s+(?:\w+::)*hook_rollout_reader_uses_verified_canonical_path\s*\.{3}\s*${status}\b` + ).test(output); +const passed = + result.status === 0 && named('ok') && /test result:\s*ok\.\s+1 passed;\s+0 failed/.test(output); +const failed = + result.status !== 0 && + named('FAILED') && + /test result:\s*FAILED\.\s+0 passed;\s+1 failed/.test(output) && + output.includes('assertion failed: with_codex_hook_rollout'); +if (!passed && !failed) throw new Error('Canonical-read regression did not run with a recognized result'); +await mkdir(path.dirname(resultPath), { recursive: true }); +await writeFile( + resultPath, + `${JSON.stringify( + { + version: 1, + caseId, + arm, + outcome: passed ? 'fixed' : 'bug', + signature: passed + ? 'rollout_reader_uses_verified_canonical_path' + : 'rollout_reader_follows_retargeted_alias', + details: + 'Actual rollout binding and reader with a symlink retargeted after verification; only the base fixture adapts the callback ABI and captures the original alias.', + }, + null, + 2 + )}\n` +);