Skip to content

fix(codex): keep foreign memory out of chats - #8989

Open
SunkenInTime wants to merge 1 commit into
pingdotgg:mainfrom
SunkenInTime:t3code/fix-computer-history-leak
Open

fix(codex): keep foreign memory out of chats#8989
SunkenInTime wants to merge 1 commit into
pingdotgg:mainfrom
SunkenInTime:t3code/fix-computer-history-leak

Conversation

@SunkenInTime

@SunkenInTime SunkenInTime commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Codex background memory work can begin emitting item notifications on an existing app-server subscription without sending a new thread/started notification. The stateful memory filter never learns that provider thread ID, so its assistant text can be projected into the active T3 chat.

This treats the active Codex provider thread ID as the chat ownership boundary. Known notifications addressed to another thread are suppressed even when their lifecycle start was missed. Registered multi-agent children still use their dedicated routing, and serverRequest/resolved still passes through for approval correlation cleanup.

The regression replay matches the observed ordering: agent message delta and item completion for a memory thread with no preceding thread/started.

Verification:

  • vp test run apps/server/src/provider/Layers/CodexSessionRuntime.test.ts apps/server/src/provider/Layers/CodexCollabWire.test.ts apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts
  • vp lint --type-aware --type-check --report-unused-disable-directives apps/server/src/provider/Layers/CodexSessionRuntime.ts apps/server/src/provider/Layers/CodexSessionRuntime.test.ts apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts
  • vp fmt --check apps/server/src/provider/Layers/CodexSessionRuntime.ts apps/server/src/provider/Layers/CodexSessionRuntime.test.ts apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts
  • git diff --check upstream/main...HEAD

Generated with GPT-5.6 Sol in T3 Code via the Codex harness.


Note

Medium Risk
Changes Codex notification routing for all non-root threads; incorrect suppression could hide legitimate cross-thread traffic, though collab interception and serverRequest/resolved exceptions limit blast radius.

Overview
Fixes background Codex memory work showing up in the active T3 chat when item deltas arrive on an existing app-server subscription without a preceding thread/started, so the stateful memory filter never learns that thread id.

The runtime now treats the active provider thread id as the chat boundary: exported shouldSuppressForeignConversationNotification drops any known notification whose threadId differs from the root, including item/agentMessage/delta and item/completed, independent of lifecycle registration. Registered collab children still go through interception first; serverRequest/resolved still passes through for approval correlation.

Suppression logic is split so v1 receiver-map children keep lifecycle-only routing via shouldSuppressChildConversationNotification, while the new helper handles the broader foreign-thread case (including the memory leak ordering).

Tests cover the helper directly and extend the collab integration replay to assert no events leak for the memory thread.

Reviewed by Cursor Bugbot for commit 509d07f. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Suppress foreign memory-thread notifications in makeCodexSessionRuntime

  • Adds shouldSuppressForeignConversationNotification to classify non-root-thread notifications as suppressible, except serverRequest/resolved.
  • The notification loop now suppresses all notifications addressed to non-root provider threads (item deltas, completions) instead of only selected lifecycle events, preventing background memory output from appearing in the parent session.
  • Adds unit tests for the new predicate and an integration assertion that no events from the background memory thread leak without a prior thread/started.
  • Risk: makeCodexSessionRuntime now drops all non-root-thread item/* notifications; any consumer relying on foreign thread deltas appearing in the session stream will no longer receive them.

Macroscope summarized 509d07f.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 1, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 509d07f

Macroscope's review found this PR approvable — This is a focused Codex bug fix that prevents foreign-thread memory output from being emitted into the active chat while preserving root events, child routing, and approval correlation. The runtime change is narrowly scoped and accompanied by unit and integration regression coverage.

You can add or adjust custom eligibility rules. Learn more.

@gfdelarue

Copy link
Copy Markdown

Confirmed another occurrence on 2026-09-05 with T3 desktop 0.0.38 on NixOS, source checkout 77b655c47, and Codex CLI 0.153.3. The running package already includes the earlier #5468 filter.

Observed reproduction:

  1. Ask an ordinary Git question in a Codex chat with automatic memories enabled.
  2. After the answer completes, background memory consolidation starts and spawns three workers.
  3. Their memory-extraction commentary, command activity, and final replies appear in the original chat, interleaved with subsequent user messages.

Read-only inspection confirmed persistence, not just a rendering problem. Sanitized identities:

Codex conversation A: Git question and answer; no worker reports
Codex logs: memory consolidation M spawned workers B, C, D
T3 projection_thread_messages: worker replies stored under A's T3 thread
T3 projection_thread_activities: same T3 thread, but data.threadId = B/C/D
Worker command cwd: ~/.codex/memories

The reports only entered conversation A's Codex transcript when I pasted the mixed UI transcript back to the assistant. This is the same user-visible failure as #4683 despite the earlier fix.

I checked this PR's actual shouldSuppressForeignConversationNotification at 509d07f: all 12 combinations of the three observed worker IDs and four message/command event methods were suppressed. Root messages and serverRequest/resolved still passed. This was a filter-level check, not an installed end-to-end test of the PR; the incident does not establish whether each worker's thread/started was absent or missed.

The follow-up filter remains absent from upstream main at 4ca71463a and nightly 0.0.39-nightly.20260905.1289. This PR addresses the observed routing gap. No private transcripts or raw logs attached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants