Atlas Agent: replace the Cersei engine with a one-time Codex port

[Ukaykhingmarma28]

Replaces Atlas's native agent — the Cersei SDK — with a one-time hard fork of OpenAI's Codex, shipped as Atlas Agent, talking to the Atlas AI gateway.

Executes ADR-0003. The plan, the evidence behind it, and every decision are in docs/atlas-agent-codex-port-spec.md and four research documents under docs/research/.

Why

Every serious defect in the native path required forking someone else's crate: the UTF-8 streaming corruption and the tool-cancel race lived on only as Atlas-maintained vendored patches, redone against every upstream release. Users felt that ownership gap as dropped connections and fragile streaming. Codex was chosen because it already ships the reliability machinery our audit found missing — verified against source, not taken on faith: real end-to-end cancel, typed retry with history resumption so tool calls are never re-run, and lossless incremental UTF-8 SSE decoding that makes our patched bug class impossible.

What changed

• Engine. 110 crates vendored under vendor/codex/, quarantined first, then wired in-process through the app-server client. No upstream tracking — from here the engine is ours.
• Provider. A new OpenAI Chat Completions dialect against the Atlas gateway, plus an Atlas error-classification arm: 402 is terminal and surfaces the quota detail, 429 honours Retry-After, 401 token_expired refreshes once. Upstream auto-retried a 402 five times — the loop the gateway doc was written to prevent.
• Auth. Per-request token provider on the engine's ExternalAuth seam, re-minting inside the JWT TTL, replacing a static bearer that would have killed every session at ten minutes.
• Deleted. atlas-cersei, the Cersei SDK dependency, both vendored patch forks, their patch-table entries, and the dead call sites.
• Phone-home. Three sites removed at source, not configured off — including one in an elevated Windows process that was outside the ticket.
• Licensing. Atlas is MIT, the engine is Apache-2.0. LICENSE and NOTICE now ship inside the app bundle; modified fork files carry change notices.

Merge with 0.3.1

Merged cleanly against six conflict hunks plus two semantic breaks git resolved textually but wrongly. Notably: 0.3.1's fork fix — a branch belongs to the source session's project, not the active workspace — was preserved by porting it into the shared fork helper, where it now reaches both /fork doors. Details in the merge commit.

State

16 of 18 tickets closed (#38–#50, #52, #54, #55). Draft because two remain, and both need a human, not an agent:

• Chat: make the send keybinding configurable (default should be Enter-to-send, Shift+Enter for newline) #51 — a live turn from a signed-in, entitled account
• add required linux build dependencies to README #53 — the acceptance bar, including the outbound-traffic audit and the capped-grant 402 check, both by observation

Green locally: typecheck, lint, format, 887 frontend tests, 332 src-tauri tests, cargo check --workspace across 130 members.

One decision needed before merge

docs/adr/ now holds two 0003s and two 0004s — 0.3.1 published 0003–0006 while this branch was adding its own 0003 and 0004. The port's two should renumber to 0007/0008, but that restamps identifiers cited across 18 published tickets, the spec, and code comments, and one file (docs/reference/atlas-ai-api.md) contains an unrelated ADR-0003 belonging to the gateway repo that a blind sweep would corrupt. Left deliberately unresolved for a maintainer call.

[ahammadnafiz]

Deep review — full pass over all 43 commits

Reviewed 5e548a6f (base 0.3.1) → 24ee88db (head). 4,125 files changed, but the reviewable surface is much smaller than the headline: 137 non-vendor files (+27,415 / −52,944) plus 92 vendored files touched after the bulk import, of which ~2,700 lines under vendor/codex/codex-api/src/atlas_chat/ are Atlas-authored code that happens to live in vendor/.

Verdict: the architecture is sound and the reasoning behind it is unusually well documented. The central reliability premise holds up under direct attack. Six defects block merge, and four of them defeat a guarantee this PR explicitly claims to have established.

	Count
Blocking	6
Non-blocking	11
Verified sound (checked deliberately, held)	15
Candidate findings killed during verification	7

---

1. How this review was run

Three phases, because a diff this size cannot be read linearly.

Phase 1 — map the ground. Five read-only agents in parallel, one question each, none overlapping: (a) trace one complete chat turn end to end through every hop; (b) find dangling references left by the atlas-cersei deletion; (c) establish this repo's own Rust conventions and check the new engine against them; (d) trace the account credential from storage to outbound header; (e) enumerate every remaining outbound network path and test whether the guard tests can actually fail.

Phase 2 — review the commits. Six agents, one per thematic commit group, each given the specific attack questions for its area. Each dispatched its own verifiers. Groups: (1) docs/workspace/deps, (2) vendoring/phone-home/licensing, (3) engine seam and concurrency, (4) dialect and error classification, (5) auth/UX/deletion/rename, (6) the fix tail and the merge.

Phase 3 — verify before reporting. Every finding below was re-opened at the path:line it names, in the main thread, before it reached this comment. A finding from an agent is a claim, not evidence. Seven candidates died at this stage and are listed in §6 — that section matters as much as the findings, because it's what separates this from a list of plausible-sounding guesses.

Each finding carries a confidence tier:

• Executed — I ran a command and captured its output.
• Read — I opened the file(s) and traced the mechanism myself.
• Traced by reviewer — an agent traced it through vendored code; I confirmed the Atlas-side half only. Flagged explicitly, never presented as executed.

---

2. What was actually executed

Everything below ran against a clean worktree of 24ee88db.

Full frontend suite — passes, exactly as claimed

$ npx vitest run

 Test Files  53 passed (53)
      Tests  887 passed (887)
   Duration  1.76s

The PR body's "887 frontend tests" is accurate to the test.

The five guard tests, on a full clone — pass

$ npx vitest run tests/codex-no-phone-home.test.ts tests/codex-quarantine.test.ts \
    tests/vendor-licensing.test.ts tests/cargo-workspace.test.ts tests/cargo-deps-unification.test.ts

 Test Files  5 passed (5)
      Tests  48 passed (48)

This is the result that hides B3. See below.

Rust compiles

$ cargo check -p atlas-native-agent --message-format=short
    Checking codex-app-server v0.0.0 (…/vendor/codex/app-server)
    Checking codex-app-server-client v0.0.0 (…/vendor/codex/app-server-client)
    Checking atlas-native-agent v0.1.0 (…/crates/atlas-native-agent)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 5m 55s

Exit 0. One future-incompat warning on a transitive proc-macro-error2 v2.0.1 — pre-existing, not from this PR. I did not run cargo check --workspace across all 130 members, and no Rust test suite was executed.

[lints] census

$ git ls-tree -r --name-only pr210 | grep 'Cargo.toml$' | while read -r f; do
    git show "pr210:$f" | grep -q '^\[lints\]' && echo "$f"
  done | tee /tmp/lints.txt | wc -l
110
$ grep -c '^vendor/' /tmp/lints.txt
110
$ grep -cE '^(crates/|src-tauri/)' /tmp/lints.txt
0

Every manifest that opts into lints is vendored. This is B5.

Sole .adopt( call site

$ git grep -n "\.adopt(" pr210 -- crates/ src-tauri/
pr210:src-tauri/src/commands/agent_host.rs:1335:                history.adopt(session_id, thread_id);

One call site in the entire first-party tree. This is B1.

0.3.1 merge integrity

$ git log pr210..origin/0.3.1
(empty)

Then, for each of the six substantive 0.3.1 commits, I extracted every added line longer than 25 characters and grepped the head tree for its presence:

Commit	Subject	Added lines checked	Absent from head
5e548a6f	acp agent terminal auth flow refinement	483	0
c817820a	session history sidebar bug patch	412	6
0ee3b431	persist timeline day range	41	0
b2a02bbd	updater patch for both mac architectures	66	0
1ad63980	mdbr-leaf-ir embedding model	1	0
226a9f5a	MiniLM-L12-v2 embedding model	1	0

The six absences in c817820a are all in agent_transcript.rs — two doc-comment lines and a test call whose signature this PR itself changed by adding the live_id parameter. Not drops. Nothing from 0.3.1 was lost in the merge.

---

3. Blocking

Ranked by blast radius.

---

B1 — Reopening any pre-cutover conversation mints a duplicate sidebar row and orphans the original

Confidence: Read — traced end to end across three files, plus one executed grep.

resume_thread branches on whether the stored row has a session id:

// src-tauri/src/commands/agent_host.rs:1324
let (opened, resumed_without_history) = match thread.session_id.clone() {
    // A draft: there is no stored conversation to reopen, so this is a
    // new one. The row is bound to it first, or the live feed would
    // mint a second row for the thread the user just clicked.
    None => {
        let opened = self.manager.new_session(record.agent.clone(), work_dirs).await…;
        let session_id = lock_thread(&opened).session_id().clone();
        history.adopt(session_id, thread_id);          // ← line 1335
        (opened, false)
    }
    Some(session_id) => {
        let resumed = self.manager.resume_stored_session(…).await…;
        (resumed.thread, resumed.mode == ResumeMode::WithoutHistory)
        //  ↑ no adopt
    }
};

The draft arm's own comment states the invariant precisely — "the row is bound to it first, or the live feed would mint a second row for the thread the user just clicked" — and the resume arm violates it. The executed grep in §2 confirms history.adopt is called exactly once in the entire first-party tree.

Now trace what the resume arm actually gets back. EngineConnection::resume_session tries thread/resume, falls back to thread/read, and if both fail opens a fresh thread:

// crates/atlas-native-agent/src/engine/connection.rs:983
let started: v2::ThreadStartResponse = self
    .call(|request_id| ClientRequest::ThreadStart { … })
    .await?;
(started.thread.id, Vec::new())

That returns a new engine thread id. And for any pre-cutover row it is guaranteed to be reached — the engine has no rollout for a Cersei-era session id, so thread/resume and thread/read both fail. The code says so itself at connection.rs:1016: "A pre-cutover row took the fresh-thread arm above and has no turns."

The live feed then resolves which history row to write to:

// crates/atlas-thread-metadata/src/recorder.rs:198
fn resolve(&self, session_id: &acp::SessionId) -> ThreadId {
    let mut bound = self.lock();
    if let Some(thread_id) = bound.get(session_id) { return *thread_id; }   // ← only adopt() populates this
    let thread_id = self.store
        .thread_for_session(session_id)                                     // ← row carries the OLD id
        .map(|thread| thread.thread_id)
        .unwrap_or_else(ThreadId::new);                                     // ← so: a brand new row
    bound.insert(session_id.clone(), thread_id);
    thread_id
}

Three tiers, all three miss. The new id was never adopted; no stored row carries it; so ThreadId::new().

The comment at connection.rs:1004 asserts the opposite outcome:

"Keyed by the id the engine will stamp on its events… For a pre-cutover row that is a new id, and the live feed rebinds the store row to it — the same path a draft takes."

A draft rebinds because resume_thread explicitly calls adopt. This path does not, so it is not the same path.

Trigger. Every existing user, on the first reopen of any native-agent conversation. Because this PR swaps the engine wholesale, every stored native-agent conversation is pre-cutover — this is the default upgrade experience, not an edge case.

Blast radius. One duplicate sidebar row per reopen. The row the user clicked stays behind, unarchived but orphaned, still pointing at the dead id. Clicking it again produces another.

Fix. In the Some(session_id) arm, compare the engine id that came back against the stored one, and history.adopt(new_id, thread_id) when they differ. resume_session already knows which arm it took — plumbing that out is cleaner than inferring it.

Related, same code path (B1b). The double-failure fallback is logged at tracing::info! (connection.rs:977) and its message asserts a cause it hasn't established — "the engine does not know thread {session_id}; opening it fresh (a row from before the engine changed)". A transport failure, a deserialization error, or an engine-busy condition all land in the same arm and get the same message. The chat still repaints from Atlas's own transcript, so it looks correct while the model continues with no context. This should be warn!, with "unknown thread" distinguished from "the call failed".

---

B2 — Stop is silently discarded during the turn-start window, and the button disappears with it

Confidence: Read — the full TurnWaiters impl, the prompt path, the cancel path, and the ACP thread cancel.

TurnWaiters::active is the map cancel consults. It has exactly one writer:

// crates/atlas-native-agent/src/engine/connection.rs:124
fn register(&self, thread_id: &str, turn_id: &str) -> oneshot::Receiver<v2::Turn> {
    …
    self.waiters().insert(turn_id.to_string(), tx);
    self.active()                                              // ← line 139
        .insert(thread_id.to_string(), turn_id.to_string());   // ← line 140
    rx
}

complete and forget only ever remove. So active is populated solely by register — and register runs after the round trip:

// connection.rs:1326
let started: v2::TurnStartResponse = requests
    .request_typed(ClientRequest::TurnStart { … })
    .await                                    // ← the window opens here
    .map_err(|e| anyhow!("{e}"))?;
…
let waiter = turns.register(&thread_id, &started.turn.id);   // ← line 1351, window closes

Meanwhile cancel reads that map and gives up when it's empty:

// connection.rs:1395
fn cancel(&self, session_id: &acp::SessionId) {
    let thread_id = session_id.to_string();
    let Some(turn_id) = self.turns.active_turn(&thread_id) else {   // ← line 1397
        tracing::debug!(target: "atlas_native_agent::engine",
            "cancel for {thread_id} with no turn in flight");
        return;
    };

The interleaving. The manager calls begin_turn(), so running_turn is Some and the composer shows Stop. The prompt future is still awaiting turn/start at line 1326. The user presses Stop:

// crates/atlas-acp-thread/src/thread.rs:1740
fn cancel_inner(&mut self, permission_outcome: RequestPermissionOutcome) {
    self.cancel_outstanding_elicitations();
    if self.running_turn.take().is_none() { return; }   // ← line 1743: takes it — UI now reads idle
    self.mark_pending_entries_as_canceled(permission_outcome);
    self.connection.cancel(&self.session_id);           // ← finds active_turn == None, logs debug, returns
    self.emit(AcpThreadEvent::StatusChanged);
}

running_turn.take() succeeds, so the UI flips to idle and Stop becomes Send. Then the call into the connection finds nothing and returns. No turn/interrupt is ever sent.

Blast radius. The turn runs to completion — including executing shell commands — while the UI reports idle and the affordance to stop it is gone. The user cannot press Stop again; there is no Stop button. This is the worst shape a lost cancel can take, because the two failures compound: the cancel is dropped and the recovery path is removed in the same instant.

The module comment at connection.rs:1398-1400 anticipates the benign reading of this state — "a cancel can race a turn that just finished" — and that's true for the after-completion case. It does not cover the before-registration case, which is the harmful one.

Test gap. crates/atlas-native-agent/tests/engine_turn.rs:337-343 retries cancel in a 100 ms loop, so it cannot fail on this. Production presses once.

Also inert, permanently. The slash-command paths (/compact, /diff, /status, /undo, connection.rs:1075-1323) return before any register call, so active is never set for them at all. Cancel is a no-op for the entire lifetime of those operations, not just a window.

Fix. Record a pending-cancel per thread in TurnWaiters; have prompt check and consume it immediately after turn/start returns, before register. That closes the window without introducing a lock ordering between the two paths.

---

B3 — The Apache-2.0 compliance test fails on every CI run

Confidence: Executed — reproduced against a real shallow clone, with output.

tests/vendor-licensing.test.ts derives its own baseline from history:

// tests/vendor-licensing.test.ts:54
function vendoringCommit(): string {
  const commits = git("log", "--format=%H", "--", "vendor/codex").trim().split("\n");
  return commits[commits.length - 1];   // oldest commit touching vendor/codex
}

That needs full history. CI does not have it:

# .github/workflows/ci.yml:37
  runs-on: ubuntu-latest
  steps:
    - uses: actions/checkout@v4        # ← line 39, no fetch-depth anywhere in the file

actions/checkout@v4 defaults to fetch-depth: 1. I reproduced the consequence:

$ git clone --depth 1 --branch pr210 --no-local file:///…/atlas shallow
$ cd shallow
$ git log --format=%H -- vendor/codex | wc -l
1
$ V=$(git log --format=%H -- vendor/codex | tail -1)
$ echo "vendoringCommit=$V  HEAD=$(git rev-parse HEAD)"
vendoringCommit=24ee88db38edfe91b175a53744696ccdf3bdaf0b  HEAD=24ee88db38edfe91b175a53744696ccdf3bdaf0b
$ git diff --name-only --diff-filter=d $V -- vendor/codex | wc -l
0

vendoringCommit() resolves to HEAD, so the diff against it is empty. Then the real test:

$ npx vitest run tests/vendor-licensing.test.ts

 ❯ tests/vendor-licensing.test.ts (7 tests | 1 failed) 423ms
     × finds the modification set (parser health) 271ms

 FAIL  tests/vendor-licensing.test.ts > §4(b) — modified files say they were modified
       > finds the modification set (parser health)
AssertionError: expected 0 to be greater than 5
 ❯ tests/vendor-licensing.test.ts:133:44
    131|   it("finds the modification set (parser health)", () => {
    132|     // If this returned nothing, the rule below would hold vacuously f…
    133|     expect(modifiedVendoredFiles().length).toBeGreaterThan(5);
       |                                            ^

 Test Files  1 failed (1)
      Tests  1 failed | 6 passed (7)

The same file passes on a full clone (§2), which is exactly why "green locally" missed it.

Worth saying: the parser-health floor did its job. Without that assertion the §4(b) rule would have passed vacuously in CI forever — zero files checked, zero violations found, green. The guard caught its own blind spot. It just fires in the one environment nobody has run it in yet.

Fix. fetch-depth: 0 on the frontend job. Consider also asserting vendoringCommit() !== HEAD with a message naming the shallow-clone cause, so the next person reads the reason instead of the symptom.

---

B4 — A malformed SSE chunk is dropped and the turn still reports a complete answer

Confidence: Read.

process_chat_sse handles three frame kinds. The error frame correctly records a failure:

// vendor/codex/codex-api/src/atlas_chat/sse.rs:317
if is_error_frame(&sse.data) {
    let disposition = atlas_gateway::classify_stream_frame(&sse.data);
    stream_error = Some(disposition.into_api_error());
    continue;
}

let chunk: ChatChunk = match serde_json::from_str(&sse.data) {
    Ok(chunk) => chunk,
    Err(err) => {
        debug!(error = %err, "failed to parse a chat.completion.chunk");
        continue;                    // ← line 326: no stream_error set
    }
};

An unparseable chunk sets nothing. If [DONE] then arrives, the terminal arm sees a clean state:

// sse.rs:300
if sse.data.trim() == DONE_SENTINEL {
    if let Some(error) = stream_error { … return; }   // ← not taken
    emit_turn(&tx_event, state, &dialect).await;      // ← turn reported SUCCESSFUL
    return;
}

The turn completes successfully, missing whatever that chunk carried.

Why this matters more than an ordinary parse-error nit. The file's own header states the rule it exists to enforce:

"a mid-stream failure emits data: {"error":…} and withholds data: [DONE] — two independent signals, either sufficient alone; a stream that ends without [DONE] is incomplete, never a short success." — sse.rs:13-17

Both stated signals are honoured. But the file infers "no error signalled ⇒ nothing was lost", and the parse-failure arm breaks that inference from inside. Silent content truncation is the exact bug class the port was undertaken to eliminate; this is a different mechanism than the UTF-8 one, in the layer above the decoder — the decoder itself is fine (§5).

Trigger. Any chunk the ChatChunk struct cannot deserialize. Concretely: an explicit null where #[serde(default)] is declared — #[serde(default)] covers a missing key, not a present-but-null one, so {"id":"c1","choices":null} errors. More realistically: any future gateway shape change that adds or retypes a field. The failure mode is the same either way, and it is silent by construction.

Fix. Set stream_error in the parse-failure arm, exactly as the error-frame arm two blocks above does. A stream carrying a frame this client cannot read is not a stream it can honestly report as complete.

---

B5 — The lint that "makes it impossible again" cannot fail

Confidence: Executed — full census in §2.

11d48ea3 adds a workspace lint table with an explicit rationale:

# Cargo.toml:556
[workspace.lints]

# `unexpected_cfgs` is a deny because its warning form already cost a live
# failure: #54 deleted the `ported-engine` feature, four blocks stayed behind
# `#[cfg(feature = "ported-engine")]`, and a cfg on a feature that no longer
# exists compiles to NOTHING — the token-source registration vanished and the
# first real gateway turn went out with no Authorization header. Cargo warned;
# nobody greps warnings. Now it fails the build.
rust = { unexpected_cfgs = "deny" }        # ← line 564

The diagnosis is exactly right and the intent is exactly right. But Cargo does not apply [workspace.lints] to a member unless that member declares [lints] workspace = true. The census in §2:

• 110 manifests in the tree carry a [lints] section
• 110 of them are under vendor/codex/
• 0 are under crates/ or src-tauri/

src-tauri/Cargo.toml — the crate where the outage actually happened — has no [lints] section. The entire table is dead configuration, and that includes the clippy denies below it (expect_used, await_holding_lock, await_holding_invalid_type, disallowed_methods, and the rest).

Trigger. Re-add #[cfg(feature = "anything-undeclared")] anywhere in src-tauri. Cargo emits a warning; nothing fails. .github/workflows/ci.yml:89 runs a bare cargo test for src-tauri with no -D warnings, and the clippy job at :165 covers only the crates matrix — so it doesn't fail there either. That is precisely the #54 outage, reproducible today.

Worth adding: tests/ipc-contract.test.ts splits generate_handler! textually, so a #[cfg]'d-out registration still counts as present there. The IPC guard cannot catch this class either.

Fix. Add to each first-party manifest:

[lints]
workspace = true

One caveat, so this doesn't land as a surprise: turning the table on is not free. It will surface existing violations — connection.rs:131 has a non-test .expect("position just found"), which clippy::expect_used will flag. That's the lint working, not a reason to skip it, but budget for a cleanup pass or start with unexpected_cfgs alone and add the clippy denies behind it.

---

B6 — Switching permission mode mid-turn changes the picker, not the running turn

Confidence: Read (Atlas side) / Traced by reviewer (engine side).

The mode mapping itself is good — genuinely one of the better-reasoned parts of this PR. It sets both halves and explains why either alone is a trap:

// crates/atlas-native-agent/src/engine/modes.rs:96
pub fn engine_policy(mode: &str) -> (AskForApproval, SandboxPolicy) {
    match normalise(mode) {
        "bypass"      => (AskForApproval::Never,        SandboxPolicy::DangerFullAccess),
        "plan"        => (AskForApproval::Never,        SandboxPolicy::ReadOnly { network_access: false }),
        "acceptEdits" => (AskForApproval::OnRequest,    workspace_write()),
        _             => (AskForApproval::UnlessTrusted, workspace_write()),   // fails closed
    }
}

The problem is set_mode, which has no in-flight-turn check:

// crates/atlas-native-agent/src/engine/connection.rs:1728
fn set_mode(&self, mode: acp::SessionModeId) -> BoxFuture<'static, Result<()>> {
    …
    let (approval_policy, sandbox_policy) = modes::engine_policy(&mode.0);
    connection.update_settings(&session_id, |params| {
        params.approval_policy = Some(approval_policy);
        params.sandbox_policy  = Some(sandbox_policy);
    }).await?;
    // Recorded only after the engine accepted it.
    connection.session_modes.lock()….insert(session_id, mode);
    Ok(())
}

It sends the update unconditionally, then records the new mode in session_modes — which is exactly what current_mode() (connection.rs:1711) reads to render the picker. Nothing gates the control on the frontend either: the picker and the Shift+Tab cycle both fire while a turn is generating.

Traced by the group-3 reviewer, not independently confirmed by me: engine-side, Session::update_settings writes only session_configuration, and the running turn keeps its frozen TurnContext. If that's right — and it matches how the mode is passed at turn start — the running turn's remaining tool calls execute under the policy the turn began with.

Trigger. Start a turn in Bypass, switch to Plan while it generates. The picker reads "Plan — read-only, no edits or commands" while the turn continues with AskForApproval::Never + DangerFullAccess.

Blast radius. The user believes they have revoked write and shell access and has visible confirmation of it. They have not. This is the one failure mode where a security control being displayed as active is worse than it being absent.

Fix. Pick one and be explicit: refuse the change while a turn is in flight and say why, or apply it and interrupt the turn. Silently showing the new mode is the only option that misleads. If the engine genuinely cannot rebind a running turn, refusing is the honest answer.

---

4. Non-blocking

---

N1 — Signing out does not invalidate the access token the engine already holds

Confidence: Read + executed grep.

$ git show pr210:crates/atlas-native-agent/src/engine/auth.rs | grep -n "pub fn"
80:pub fn register_token_source(source: Arc<dyn AtlasTokenSource>)
85:pub fn registered_token_source() -> Option<Arc<dyn AtlasTokenSource>>
151:    pub fn new(source: Arc<dyn AtlasTokenSource>) -> Self
155:    pub fn with_clock(source: Arc<dyn AtlasTokenSource>, clock: Arc<dyn Clock>) -> Self

No clear(), no invalidate(), no Drop. The cache serves the token until exp − 60s:

// auth.rs:163
fn cached_if_fresh(&self) -> Option<String> {
    let now = self.clock.now_unix();
    let cached = self.cached.lock().unwrap_or_else(|p| p.into_inner());
    cached.as_ref().filter(|c| now < c.renew_after).map(|c| c.token.clone())
}

auth_sign_out (src-tauri/src/commands/auth.rs:214) clears disk and revokes the session token. The access JWT is verified statelessly against JWKS, so revoking the session token does not invalidate it. An open native-agent connection therefore keeps making authenticated, org-billed gateway calls until the JWT's own expiry — up to ~9 minutes.

The doc comment at src-tauri/src/auth/core.rs:1294 still says "There is still no in-memory access token to clear." 34ac769b made that false and the comment wasn't updated, which is how this stayed invisible.

Fix. Give AtlasExternalAuth a clear() and call it from the sign-out broadcast; or drop the engine connection on sign-out, which also handles the in-flight turn.

---

N2 — A fourth phone-home path ships, and the guard test is structurally unable to see it

Confidence: Read. The three removals the PR names are genuinely gone at source — verified independently, and the Windows one removed telemetry only (see §5). This is a fourth site the PR does not mention.

vendor/codex/feedback/src/lib.rs:42
  "https://ae32ed50620d7a7792c1ce5df38b3e3e@o33249.ingest.us.sentry.io/4510195390611458"
vendor/codex/feedback/src/lib.rs:438
  dsn: Some(Dsn::from_str(SENTRY_DSN)…)

It is not dead code in the dependency graph:

crates/atlas-native-agent/Cargo.toml:57   codex-feedback = { path = "../../vendor/codex/feedback" }
crates/atlas-native-agent/src/engine/runtime.rs:52    use codex_feedback::CodexFeedback;
crates/atlas-native-agent/src/engine/runtime.rs:194       feedback: CodexFeedback::new(),

It is unreachable today, and I want to be precise about that rather than overstate it. Nothing in src/, crates/ or src-tauri/src/ constructs a feedback RPC, and the crate installs no global Sentry hook — grep -n "sentry::init\|panic::set_hook" over the crate returns nothing; the client is built inside upload_feedback and nowhere else. So the DSN ships in the binary without firing.

Why it's still worth raising. The guard test enumerates three known removals rather than proving absence of egress. Its full structure:

$ git show pr210:tests/codex-no-phone-home.test.ts | grep -n "describe("
117:describe("the engine's egress surface (parser health)", …
132:describe("the Statsig metrics exporter is gone", …
155:describe("the ChatGPT analytics client is gone", …

Three blocks, ending at line 182. It scans vendor/codex only (:36), greps no sentry or ingest literal, and has no assertion at all for the Windows-sandbox removal that 0f16c4cf also made. It proves the three removals stayed removed. It does not — and structurally cannot — prove there is no fourth.

That distinction matters because #53's outbound-traffic audit is the open acceptance item this PR is drafted against, and a reader could reasonably take "the phone-home guard passes" as having discharged it.

Fix. Either drop the codex-feedback dependency (nothing calls it), or add a describe block that greps the whole vendored tree for hardcoded hosts against an explicit allowlist, so a fifth site fails rather than passes.

---

N3 — No CI job covers any vendored engine crate

Confidence: Read.

• .github/workflows/ci.yml:89 — cargo test, working-directory: src-tauri. Member-scoped, so it selects only that package.
• .github/workflows/ci.yml:110-139 — the crates matrix, 19 entries, all atlas-*.
• No job anywhere runs anything under vendor/.

atlas-cersei had a matrix entry; 1f82a358 correctly removed it. Its replacement — the whole ported engine, including the Atlas-authored atlas_chat/ dialect (~2,700 lines) with request_tests.rs (554 lines) and sse_tests.rs (508 lines) — got none. Those ~1,060 lines of tests, which are the primary evidence for the dialect's correctness, never run in CI.

tests/ci-coverage.test.ts reads crates/ and therefore cannot notice the gap.

Every test count in these commit messages is local-only. That's fine as a statement of what the author ran; it is not a statement about what CI protects.

Fix. One matrix entry running cargo test -p codex-api would cover the Atlas-authored dialect, which is the part that's genuinely yours. Covering all 110 vendored crates is not the ask — and per the group-2 reviewer, ~30 upstream vendored tests are now permanently red after the analytics removal, so a blanket job would fail immediately.

---

N4 — The vendored engine compiles at opt-level = 0 in dev

Confidence: Read + executed count.

The root manifest is careful about this and explains itself:

# Cargo.toml:673
# `"*"` above reaches dependencies ONLY — workspace members are excluded from
# it by cargo. Before the workspace, every `crates/atlas-*` package was a plain
# path dependency of src-tauri and therefore covered; as members they would
# silently drop to opt-level 0…
[profile.dev.package.atlas-acp-thread]
opt-level = 1
… (18 stanzas)

The reasoning is right and it was applied to all 18 first-party members. It was not applied to the ~110 vendored members, which are members too (Cargo.toml:47-…):

$ git show pr210:Cargo.toml | grep -c "profile.dev.package.codex"
0

The guard was deliberately scoped out, with a documented expiry:

// tests/cargo-workspace.test.ts:215
// …on no runtime path until the seam is rewired (#45)… Revisit in #45.

#45 (a630f480) and #54 (61277ab3) are both on this branch, and crates/atlas-native-agent/Cargo.toml:49-63 declares the engine crates as unconditional path dependencies. The condition the exemption was written under has expired.

Impact. Every bunx tauri dev turn runs ~600k LOC of engine — streaming, rollout I/O, sandboxing, apply-patch — unoptimized.

This is a genuine tradeoff, not an obvious bug: adding the stanzas costs a longer first clean build. The point is that the decision should be re-made now that the condition changed, rather than inherited from a comment that no longer applies.

---

N5 — Session state in the sink grows for the life of the process

Confidence: Read + executed grep (Atlas side) / Traced by reviewer (the 100 ms abort trigger).

$ git show pr210:crates/atlas-native-agent/src/engine/sink.rs | grep -n "remove(\|clear()\|retain\|insert("
 78:        self.lock().insert(                       # EngineSessions — the only insert
127:        let output = session.command_output.entry(item_id.to_string()).or_default();
134:            session.command_output.remove(item_id);   # the ONLY .remove( in the file
156:            session.streamed.insert(item_id.to_string());

Three separate growths:

1. EngineSessions has insert (:77) and no removal path at all. EngineConnection implements no close_session. Per agent_host.rs, the connection's lifetime is the process lifetime.
2. streamed: HashSet<String> (:43) only ever inserts (:156). One String per streamed item, for the process lifetime.
3. command_output is cleared only on ItemCompleted (:134, reached from :395).

Only thread is Weak (sink.rs:35); command_output, streamed, cwd and skills are owned by the map entry and outlive the AcpThread being dropped.

The trigger for (3), traced by the group-6 reviewer through vendored code I did not open myself: the engine's GRACEFULL_INTERRUPTION_TIMEOUT_MS is 100 ms (vendor/codex/core/src/tasks/mod.rs:66); past that it calls task.handle.abort() (:913). An aborted task emits no ItemCompleted, so sink.rs:395 never runs and that command's accumulated output is retained permanently. Press stop on a few verbose builds and it accumulates.

Fix. A close_session on the connection that removes the map entry covers all three. Failing that, clear command_output on turn completion as well as item completion.

---

N6 — Native-agent file edits never reach shared cross-agent memory

Confidence: Read. Two independent blockers, both new in 733cda07.

First, the name never matches. The sink builds the tool call with a human-readable title and no machine name:

// crates/atlas-native-agent/src/engine/sink.rs:239
let mut call = acp::ToolCall::new(id.clone(), title)
    .kind(acp::ToolKind::Edit)

The consumer tests for an exact string:

// src-tauri/src/commands/memory_delta.rs:148
fn is_file_mutation(tool_name: &str) -> bool {
    let n = tool_name.to_lowercase();
    n == "edit" || n == "write" || n == "create"
        || n.contains("str_replace") || n.contains("create_file")
        || n.contains("apply_patch") || n.contains("multiedit")
}

"edit src/foo.rs" is not "edit", and matches none of the contains arms either.

Second, even on a match, the path lookup misses. The sink writes a plural array:

// sink.rs:248
.raw_input(serde_json::json!({
    "paths": changes.iter().map(|c| c.path.clone()).collect::<Vec<_>>(),
}));

The extractor probes five singular keys, none of them paths:

// memory_delta.rs:159
for key in ["file_path", "path", "filePath", "target_file", "file"] {

Blast radius. Claude Code and Codex see no file_changed signal for anything Atlas Agent writes — the cross-agent memory feature silently excludes the native agent. No test covers it.

The repo already solved the first half. crates/atlas-checkpoint/src/tools.rs:102 has a canonical_name that does leading-token matching specifically for this agent. memory_delta never calls it. Reusing it, plus adding paths to the key list (or emitting a singular path when there's one change), fixes both.

---

N7 — Retry-After is honoured with no upper bound

Confidence: Read (parser) / Traced by reviewer (the sleep site).

// vendor/codex/codex-api/src/atlas_gateway.rs:94
fn retry_after(header: Option<&str>) -> Duration {
    header.and_then(|v| v.trim().parse::<u64>().ok())
        .map(Duration::from_secs)
        .unwrap_or(Duration::from_secs(60))
}

The fallbacks are right and deliberate: absent, malformed, or HTTP-date-formatted values all fail to parse and fall back to 60s, which is the safe direction. But a parseable absurd value is honoured verbatim — Retry-After: 86400 yields a 24-hour sleep. Per the group-4 reviewer, responses_retry.rs:124 sleeps it directly, and the MAX_CONNECTION_RETRY_DELAY clamp covers only the connection branch.

Trigger. A 429 carrying a large Retry-After from the gateway or from any intervening CDN or WAF — classify falls back gracefully on a non-envelope body, so an infrastructure-generated 429 reaches the same arm.

Fix. Clamp to the same bound the connection branch uses. The gateway documents only 1 and 60; anything beyond a few minutes is a bug or an intermediary, and either way the user should get an error rather than a silent multi-hour stall behind "Reconnecting…".

---

N8 — Approval delivery failures are swallowed with no log

Confidence: Read.

// crates/atlas-native-agent/src/engine/connection.rs:505
match answer.result {
    Ok(value) => {
        let _ = client.resolve_server_request(answer.request_id, value).await;   // 507
    }
    Err(message) => {
        let _ = client.reject_server_request(…).await;                           // 510
    }
}

If either send fails, the engine turn waits forever for an answer that will never arrive, and nothing is written anywhere. The biased; select above it exists precisely because "a turn is blocked on every one of them" — which makes the silent-drop the worst case for this specific channel.

The repo's own convention logs the equivalent: crates/atlas-agent-servers/src/connection.rs:158 logs a transport failure on the analogous path.

Fix. if let Err(e) = … { tracing::warn!(…) }. One line each.

---

N9 — CI does not pass --locked, so the new lockfile guard is advisory

Confidence: Traced by reviewer — I did not re-run the CI config analysis myself.

tests/cargo-deps-unification.test.ts asserts the libsqlite3-sys pin and single-resolution of rusqlite / tree-sitter against the committed Cargo.lock, from the frontend job, which has no Rust toolchain. Meanwhile the app and crates jobs run cargo without --locked, so cargo freely re-resolves in-runner and discards the result.

A manifest edit committed without its regenerated lock passes both: cargo silently updates the lock it's using, and vitest validates the stale file on disk. The links = "sqlite3" collision then surfaces at release-build time, far from its cause.

This is newly meaningful — origin/0.3.1 had 21 per-crate locks and no such guard, so there was nothing to be stale.

Fix. --locked on the three cargo invocations.

---

N10 — Transparent PNGs composite to black when downscaled

Confidence: Read.

// src/features/chat/components/message-input.tsx:145
const ctx = canvas.getContext("2d");
if (!ctx) return image;
ctx.drawImage(bitmap, 0, 0, width, height);     // no fillRect first
…
const encoded = canvas.toDataURL("image/jpeg", quality);

A fresh canvas is transparent; JPEG has no alpha channel, so transparent pixels composite to black. A macOS window capture — rounded corners and drop shadow, frequently over the 512 KB budget — comes out with black corners and a black halo.

Fix. ctx.fillStyle = "#fff"; ctx.fillRect(0, 0, width, height); before drawImage.

Related, smaller (N10b). PER_IMAGE_BUDGET_BYTES is BODY_CAP_BYTES / 4 and enforced per image only (image-policy.ts:33), so four in-budget attachments plus prompt, tools and history guarantee a 413. The 413 handling itself is correct and deliberate — terminal, not retried, gateway message surfaced (atlas_gateway.rs:178-184, tested at :358) — and the module's decision to let the gateway be the backstop is documented and defensible. The gap is only that nothing warns before sending.

---

N11 — Stale documentation points at deleted code

Confidence: Read (reported by the deletion scout, spot-checked by me).

Nothing here breaks at runtime; all of it misleads the next reader.

• ARCHITECTURE.md:347 — describes [patch.crates-io] overrides in src-tauri/Cargo.toml and the _CERSEI_UTF8_PATCH_GUARD / _CERSEI_CANCEL_PATCH_GUARD compile guards. All three are gone. It ends "Don't delete the guards to fix the build."
• ARCHITECTURE.md:218,313,325-326 — points at crates/atlas-cersei/ARCHITECTURE.md ("read it before touching agent lifecycle") and vendor/cersei-provider / vendor/cersei-agent. vendor/ now contains only codex.
• CONTRIBUTING.md:175 — documents cargo test -p atlas-cersei, which now fails.
• README.md:66 — "The Atlas agent runs in-process on Cersei, our Rust agent framework", contradicted by CONTEXT.md:26.
• crates/atlas-memory/README.md:22 and MIGRATION.md:10 — cite atlas-cersei/ARCHITECTURE.md §6e and atlas-cersei/src/memory.rs as the frozen seam spec.
• tests/cersei-containment.test.ts:37 — ALLOWED_CERSEI_MANIFESTS still allowlists crates/atlas-cersei/Cargo.toml. Harmless (the walker only visits existing directories), but the test's own header at :21 says to shrink this set in the deletion commit.
• crates/atlas-native-agent/src/engine/mod.rs:3 — "Everything in here is gated on the ported-engine feature", contradicted by Cargo.toml:42 ("No longer optional"). A reader trusting this assumes a kill switch that no longer exists.

The ~40 remaining "cersei" string literals in src-tauri/ and src/ are not in this list — CONTEXT.md:26 and lib.rs document that the id is a deliberately-stable storage key, and f6ffabcf correctly left it alone.

---

5. What holds up

Reported because a review that lists only defects misrepresents the work. Each of these was checked deliberately, most of them because they were the claims most worth doubting.

The central reliability premise is real. SSE framing and incremental UTF-8 decoding are delegated to the eventsource-stream crate (vendor/codex/codex-api/Cargo.toml:27; sse.rs:257 stream.eventsource()), so Atlas's code only ever sees fully-decoded String frames. A multi-byte codepoint split across TCP chunks, a 4-byte emoji, \r\n vs \n, a chunk boundary landing mid-field or exactly on data: — all of that is a maintained library's problem, not a hand-rolled decoder's. The patched bug class genuinely cannot recur here. B4 is a different failure, one layer up, and does not undermine this.

A stream that ends without [DONE] fails closed (sse.rs:278-289) and there's an idle timeout on every poll (:266). Both correct.

Terminal events cannot be dropped. The in-process transport splits delivery guarantees by notification type:

// vendor/codex/app-server/src/in_process.rs:109
fn server_notification_requires_delivery(notification: &ServerNotification) -> bool {
    matches!(notification,
        ServerNotification::TurnCompleted(_)
            | ServerNotification::ThreadQueueChanged(_)
            | ServerNotification::ThreadSettingsUpdated(_)
            | ServerNotification::ExternalAgentConfigImportCompleted(_))
}

Those take a blocking .send().await (:735); everything else takes try_send with a warn-on-full (:746). The entire TurnWaiters design depends on TurnCompleted never being dropped, and it isn't. I went looking for a hung-waiter bug here and it isn't there.

The register-after-response race is closed, deliberately. TurnWaiters keeps an unclaimed: VecDeque<v2::Turn> (connection.rs:120) so a completion that arrives before anyone registers is claimed rather than lost (:127-136), bounded at 16 entries with a documented rationale. Careful work — and it's why B2 is specifically about cancel, not about completion.

Burst batching cannot lose events. 91ced74f was the commit I most expected to find a bug in, and it's clean. next_event is self.event_rx.recv().await over a tokio::sync::mpsc::Receiver (in_process.rs:268,320), which is cancel-safe — so neither the select! dropping the future nor the now_or_never drain (connection.rs:537) can swallow an event. There's no timer, so there's no partial buffer that can sit un-flushed. Merging is adjacency-only on already-decoded strings, so no character or code fence gets split. The commit comment explains all of this correctly.

Permission modes fail closed and are enforced at the point of action. engine_policy's _ arm is UnlessTrusted + workspace-write — the strictest non-plan pairing (modes.rs:106). The sandbox is a real process-level control, not a UI state. The doc comment explaining why sandbox-alone and approval-alone are each a trap (modes.rs:15-16) is the kind of thing that stops the next person from breaking it.

The entitlement check is a UX pre-flight, and says so. agent_entitlement.rs:11-17 explains that the real gate is the gateway's own 403 and this exists so a user isn't told to type a message that will fail. Unknown is deliberately distinct from NoGrant so an offline user isn't sent to their admin. This is the right split — I went in expecting a client-side authorization gate and it isn't one.

The shared request-id counter is correct and complete. One AtomicI64 per connection, shared across every control path. Ids are Atlas-minted; the engine rejects only concurrently-in-flight duplicates; unknown-id responses are logged and dropped; shutdown drains pending waiters with an internal error. No collision, wrap, or cross-matching is reachable.

Approvals cannot be mismatched. Matching is by engine-minted id echoed back, so cross-matching isn't possible. Dropping the thread resolves an outstanding waiter as Cancelled rather than leaking it.

The Windows sandbox lost telemetry, not a security control. This was the highest-risk item in 0f16c4cf and it checks out: the 141 removed lines from wfp_setup.rs are entirely WfpSetupMetric / emit_wfp_setup_metric / metric-name constants. install_wfp_filters_for_account is still called identically, still inside catch_unwind, wfp.rs was not touched by the commit at all, and the elevation payload lost only its otel field, symmetrically on both producer and consumer.

The Statsig and analytics removals are real deletions, not stubs. No HTTP client, no URL, no key survives in vendor/codex/analytics or vendor/codex/otel; analytics/Cargo.toml has no reqwest; core/src/config/otel.rs:24 defaults the metrics exporter to None. Nothing was left re-enablable by config.

Licensing obligations are met. LICENSE and NOTICE are wired into src-tauri/tauri.conf.json:46-48, and every modified vendored file carries the §4(b) change notice. B3 is that the test proving this cannot run in CI — not that the compliance is absent.

The merge dropped nothing from 0.3.1. git log pr210..origin/0.3.1 is empty, and the added-line scan in §2 found no real absences. The group-6 reviewer additionally ran git merge-tree against the auto-merge and found exactly one intentional deviation — the fork-fix port — with the two apparent losses (commands/mcp.rs, agents_set_compress) both being deliberate upstream and #54 deletions respectively.

The fork fix reaches both doors. One helper, src/features/chat/lib/fork-session.ts, called from the header menu (chat-panel.tsx:172) and the composer's /fork (message-input.tsx:1406,1662). 0.3.1's rule — a branch belongs to the source session's project, not the active workspace — survives as tier 1:

cwd: sess.workingDirectory
  || workspacePathForTab(tabId)
  || useProjectStore.getState().currentProject?.path
  || "",

The added fallbacks cannot mask it, because every session is created with a non-empty cwd that gets stamped into workingDirectory. Consolidating two call sites into one helper was the right call and it was done correctly.

No dangling references, and no silently dropped IPC. Every invoke() name in src/ resolves to a generate_handler! entry, and every registration has a #[tauri::command] definition. The one removed command, agents_set_compress, lost its definition (agents.rs:1455) and its only caller (chat-store.ts:233) in the same PR. No atlas:* event emitted on the base is missing on the head. The root Cargo.toml has no atlas-cersei member and no cersei-* patch entries.

Workspace adoption changed no resolution. No [workspace.dependencies] was introduced, no member moved a dependency to workspace = true, no feature list was edited, and resolver = "2" is pinned explicitly at Cargo.toml:24 — a workspace root defaults to resolver 1, and that was caught. Release profiles were moved from src-tauri/Cargo.toml, not rewritten (a comment-stripped diff of the two is empty). Every rusqlite declaration carries features = ["bundled"], so the 0.32→0.39 bump cannot link a system SQLite, and the highest SQL floor in the tree (STRICT, 3.37) is far below the pinned version.

The bulk vendoring commit is clean. No .github/ workflows that would activate in this repo, no submodules, no network-touching build.rs, no credentials, no real API keys in fixtures. Largest non-source blob is a 620 KB JSON schema.

71fb02fe is correct. install_manager still runs before app.manage(AuthState::new(…)), but both registrations resolve lazily via try_state, so no panic is reachable. Fixed by deferral rather than reorder — deliberate, and documented as such.

939e31bc fixed the root cause, not the symptom. The stop-button crash was a bare tokio::spawn from a sync Tauri command on the main thread, where there's no ambient runtime. The fix uses self.runtime.handle().spawn (connection.rs:1417) with the reasoning written down. No catch_unwind, no swallowed Result, no Result→Option broadening. (One asymmetry noted below.)

Checkpoints are not per tool call. I went looking for unbounded checkpoint growth in 733cda07 and it isn't there — checkpoints are per git commit, and there's no N+1 on the thread store; the IPC path stays linear via a suffix diff in the projector.

---

6. Candidate findings that died during verification

Listing these because they're what makes the rest trustworthy. Each looked real until the code was opened.

1. "A dropped TurnCompleted hangs the waiter forever, with no timeout anywhere on the chain." Refuted — TurnCompleted is in the guaranteed-delivery set and uses a blocking send (in_process.rs:109-117,735).
2. "The select! and now_or_never batching can swallow an event if next_event isn't cancel-safe." Refuted — it's mpsc::Receiver::recv, which is cancel-safe (in_process.rs:268,320).
3. "Concurrent token minting is a thundering herd with a lost update." Real in mechanism, benign in effect — mint_fresh holds no lock across the await, so N concurrent resolves each mint. But every minted token is valid, and renew_after is derived from each token's own exp, so a "lost update" installs a token that simply re-mints sooner. Extra round trips, not a correctness bug. Downgraded out of the report.
4. ".max(now + 1) caches an already-expired JWT." True and deliberate — it's the documented meltdown-avoidance for a token whose exp is already inside the margin. Worst case is one 401, which refresh() handles by always re-minting. Self-correcting.
5. "The entitlement gate is client-side only." Refuted — it's explicitly a pre-flight for UX; the gateway's 403 is the gate (agent_entitlement.rs:11-17).
6. "The plaintext session token is a new security regression." Refuted — it pre-exists on origin/0.3.1. The diff to src-tauri/src/auth/store.rs in this PR is only an expansion of the doc comment ratifying it as a recorded D14 exception. Flagging it as introduced here would have been wrong.
7. "Declining a PermissionsRequestApproval is transmitted as an approval." Refuted by the group-3 reviewer's verifier — an empty GrantedPermissionProfile is the wire encoding of denial. (The inverse is a real, smaller gap: Allow grants nothing extra, so the model can re-request and re-prompt. That's documented deliberately at connection.rs:720-724, but with no loop guard.)

---

7. Commit by commit

"Concern" means a finding above lands in that commit. It is not a verdict on the commit's intent — several of the concern rows are otherwise strong commits.

Group 1 — docs, workspace, dependency unification

Commit	Subject	Verdict	Note
da49c887	ADR-0003 + port spec + research	Sound	Docs only
291e0a48	port spec, research, gateway reference	Sound	Three .gitignore negations; the six new docs carry no sk-/eyJ/AKIA/BEGIN matches, nothing that should be tracked became hidden
acbe75b6	#38 adopt the root cargo workspace	Concern	N4. Resolution itself is unchanged; resolver = "2" pinned explicitly, profiles moved not rewritten
330c453d	#39 unify rusqlite and tree-sitter pins	Sound	All declarations bundled; SQL floor far below the pin; sqlite_floor.rs reads version_number() — the runtime library, which is what it claims to pin
34d54987	#40 record Phase 0 verification findings	Sound	Spec text
25d40032	#41 correct D14's premise	Sound	Spec text
988a9a38	#41 reconcile D14, ratify storage exception	Sound	Documents a pre-existing decision rather than introducing one — see §6.6

Group 2 — vendoring, phone-home, licensing

Commit	Subject	Verdict	Note
67fb707d	#42 vendor the engine closure, quarantined	Sound	No workflows, submodules, blobs or credentials. Quarantine is a vitest text scan, not a cargo gate — all 110 crates are members, so a bare cargo build at root compiles them
0f16c4cf	#43 rip the phone-home paths out	Concern	Removals genuine, no security control lost. N2 — a fourth path survives
a48547e7	#44 put the Apache-2.0 obligations in force	Blocking	B3. The obligations themselves are correctly met

Group 3 — linking the engine, building the seam

Commit	Subject	Verdict	Note
340fc593	#45 ADR-0004 and the open-question-3 trace	Sound	Docs only
c5823a75	#45 link the engine, config assembly	Sound	enable_codex_api_key_env: false is the right call
a630f480	#45 rewire the seam over the in-process client	Concern	Establishes register-after-response, which B2 rides on. The unclaimed buffer correctly closes the completion half of that race
adb8d305	#46 cancel, retry, stop reasons	Blocking	B2
f9e3f0c8	#47 permission modes and the effort knob	Blocking	Mapping is correct and enforced at the point of action; B6 is the mid-turn change
693da6aa	#47 share one request-id counter	Sound	Correct and complete — no collision, wrap, or mismatch reachable
aef46141	#47 tool approvals round-trip	Sound	Matching by engine-minted id; no cross-matching possible
0a38f8dc	#46 cancel kills the command, retry does not re-run	Concern	The retry test is a genuine filesystem-level proof. The cancel test cannot fail on B2 (100 ms retry loop)

Group 4 — memory, history, dialect, error classification

Commit	Subject	Verdict	Note
a237fbd0	#48 search_memory; dead Cersei call sites go	Sound	Limit clamped, empty query refused, every branch answers. No timeout around the injected retrieval future
ed265811	#49 history continuity across the swap	Blocking	B1 — the resume fallback. The sink fix itself is correct and mutation-checked
c04539bd	#50 the D13 error-classification arm	Concern	Table is right and its tests would fail if it weren't. N7 — Retry-After has no upper bound
7db3bdff	#50 the gateway Chat Completions dialect	Blocking	B4. Otherwise the strongest commit in the group — allowlist-by-type is the right defence, and the byte-by-byte UTF-8 split test is real
8cc8f2af	#50 wire the dialect into the engine	Sound	Keying the override on WireApi rather than provider id is the right seam
224b5b58	#50 static catalogue + four review fixes	Sound	All four fixes real; the nested-stream_options test is a genuine assertion

Group 5 — auth, UX policies, unwiring Cersei, rename

Commit	Subject	Verdict	Note
34ac769b	#51 authenticate with the user's Atlas account	Concern	Token never on disk, no Debug on the cache, resolved at connect not construction, with a test pinning it. N1 is the sign-out gap
9a102096	#52 the three gateway UX policies (D15)	Concern	Entitlement and retry-delay are solid; the image policy is the weak leg (N10)
61277ab3	#54 unwire the Cersei path	Concern	Shipped a total auth outage for four commits — four #[cfg(feature = "ported-engine")] blocks survived the feature's deletion, so register_token_source never ran. Fixed by 11d48ea3; confirm the two land together
f6ffabcf	#55 rename to Atlas Agent + trademark scrub	Sound	No wire value, persisted key, config key, event name or model slug was renamed. CERSEI_AGENT_ID correctly left as the literal "cersei" — it is a storage key, and the code says so
16a42978	#54 remove the unwired Cersei path from disk	Sound	Nothing references it; no persisted state breaks (no store is persist-backed, so there is nothing to migrate)
1f82a358	#54 drop the deleted crate from the CI matrix	Concern	Correct in itself — but N3, the replacement got no job

Group 6 — the fix tail and the 0.3.1 merge

Commit	Subject	Verdict	Note
194a0d53	Atlas Agent tells the app what it can do	Sound
614795ce	frontend stops treating it as the BYOK agent	Sound
11d48ea3	restore #54's registrations + the lint	Blocking	Fix correct and the diagnosis in its comment is exactly right. B5 — the guard it adds is inert
21755285	every gateway request names the paying org	Sound	Lazy try_state used correctly. Header is absent when there is no active org — worth confirming that is the intended gateway behaviour
71fb02fe	registrations must not touch AuthState early	Sound	Fixed by deferral rather than reorder; no panic reachable
1675da62	slash commands survive a resume	Sound
02a5d6a5	reopened session shows its conversation	Concern	Superseded by b992a18b
b992a18b	a reopened session replays under its own id	Blocking	B1 lands here. Also sets supports_load_session = true, which removes the "can't replay past messages" notice that ed265811's doc comment relied on — so B1 now lands with no explanation shown to the user
91ced74f	streamed replies apply in bursts	Sound	Attacked specifically; see §5
630a9357	/undo, /goal, /review, skills, /fork, /queue	Sound
939e31bc	the stop button must never take the app down	Sound	Root cause, not symptom. Applied asymmetrically: cancel holds Arc<EngineRuntime> while EngineSessionControls (connection.rs:1859) holds a bare Handle, and EngineRuntime::drop calls shutdown_background(). set_effort is safe only because its caller happens to hold the connection across the call — incidental, not structural
733cda07	tool calls reach the thread, with checkpoints	Concern	N5, N6. No unbounded checkpointing and no N+1 — see §5
24ee88db	Merge upstream/0.3.1	Sound	Nothing dropped; exactly one deliberate hunk beyond the auto-merge

Two bare tokio::spawn calls remain detached at connection.rs:705 and :757 (approvals, dynamic tools). They're safe today because both are reachable only from pump_events, which runs on the engine runtime — but that's a property of the current call graph, not an enforced invariant, and it's the same shape as the bug 939e31bc fixed. Worth a comment at minimum.

---

8. The open ADR-numbering question

Leaving this for a maintainer was the right call, and the stated hazard is real — docs/reference/atlas-ai-api.md carries an unrelated ADR-0003 belonging to the gateway repo that a blind sweep would corrupt.

One consideration the PR body doesn't weigh: the costs are not symmetric over time. Renumbering to 0007/0008 is a one-time sweep across 18 tickets, the spec, and some code comments — all documents a human reads, where a wrong link is visible and fixable. Leaving two 0003s and two 0004s makes every future reference permanently ambiguous, and the ambiguity is silent — nothing fails, readers just land on the wrong document and don't know it.

Renumber, excluding the gateway file by explicit path rather than by pattern.

---

9. Limits

Stated plainly, because several conclusions above depend on them.

• No Windows build. windows-sandbox-rs is cfg(target_os = "windows") and no CI job builds it, so the hand-edited wfp_setup.rs (including a stray misindent noted at win.rs:1017) has never seen a compiler. The removal was verified by reading the diff, not by compiling it.
• No tauri build. LICENSE and NOTICE were verified in tauri.conf.json, not in a produced .app.
• No live gateway turn. Chat: make the send keybinding configurable (default should be Enter-to-send, Shift+Enter for newline) #51 and add required linux build dependencies to README #53 remain the right gate and both need a human. Nothing here substitutes for the outbound-traffic audit or the capped-grant 402 check by observation.
• cargo check covered atlas-native-agent only, not the full 130-member workspace, and no Rust test suite was run — neither the 332 src-tauri tests nor the crate suites. The frontend numbers in §2 are the only test evidence in this review.
• Claims about engine-internal retry and task-abort behaviour (N5's 100 ms trigger, N7's sleep site, B6's frozen turn context, the 401 retry-count question) come from reading vendored source that no CI job exercises. They're marked "traced by reviewer" above and should be treated as strong leads rather than settled facts.
• The 401 token_expired "refresh once" claim is one I could not settle. The classification is unambiguously correct at atlas_gateway.rs:152 (RefreshAuthThenRetryOnce). Whether "once" is actually enforced downstream — the group-4 reviewer traced a path where an exhausted UnauthorizedRecovery re-classifies into a retryable stream error and issues further requests with the dead credential — I did not independently confirm. Worth one look before merge, since it's a stated policy.