Skip to content

feat: codex rollout harness driver#113

Open
Coldaine wants to merge 4 commits into
feat/codex-replay-corpusfrom
feat/codex-driver
Open

feat: codex rollout harness driver#113
Coldaine wants to merge 4 commits into
feat/codex-replay-corpusfrom
feat/codex-driver

Conversation

@Coldaine

@Coldaine Coldaine commented Jul 23, 2026

Copy link
Copy Markdown
Owner

User description

What

Implements PR-2 of the Codex replay series per docs/plans/plan-codex-replay.md D1/D2, stacked on #112.

  • src/capture/drivers/codex/ — the Codex HarnessDriver: normalizer.ts implements the full D1 mapping table (session_meta, response_item message/reasoning/custom_tool_call(_output)/function_call(_output), event_msg task lifecycle, patch_apply_end, mcp_tool_call_end, web_search_end, token_count sampling, context compaction, thread_goal_updated, the user-message dedupe rule), discovery.ts walks ~/.codex/sessions recursively for rollout-*.jsonl (CODEX_SESSIONS_DIR env override), capabilities.ts declares Codex's risk-heuristic/tool-name-mapping profile. Registered in src/capture/drivers/index.ts alongside claude-code, which remains the default driver.
  • D2src/inference/context-packager.ts now sets observedAgent to the dominant harnessId across the event window instead of hardcoding 'claude-code'.
  • D4 — new repeated_searches risk heuristic in src/shared/derive.ts (the GHCR-403 stuck signature: ≥3 near-identical web_search/*search* queries in a window), capability-gated like every other heuristic, declared by the Codex driver.
  • Determinism — event IDs are codex:{sessionId}:{entryIndex}:{subIndex}, derived from a per-session counter that resets on session_meta, not randomUUID() — required for reproducible replay (PR-3).
  • Teststests/capture/drivers/codex-normalizer.test.ts (hand-written entries per D1 mapping row, including patch_apply_end's fail path, token_count sampling, and the dedupe rule), tests/capture/drivers/codex-fixture.test.ts (streams the real 3,677-line PR-1 fixture through the full parser → normalizer pipeline and asserts aggregate counts against its verified type/payload-type profile, plus a determinism check that normalizing the fixture twice produces identical ID sequences), tests/capture/drivers/codex-discovery.test.ts (walks a temp dir via CODEX_SESSIONS_DIR), plus additions to tests/inference.test.ts (D2) and tests/derive-capabilities.test.ts (D4).
  • Live-test driver-awareness fixtests/live/read-real-session.test.ts hardcoded the claude-code normalizer, an assumption that broke the moment a second driver was registered (this machine's newest real session turned out to be a genuine Codex rollout, discovered correctly but then fed through the wrong parser). Fixed to route through driverRegistry.getForSource(session.source), the same pattern session-manager.ts already uses in production — verified against that real local Codex session.

Deliberate deviations from the design doc (one-line justifications each)

  • User-message dedupe direction: the doc says "keep the event_msg, skip the response_item." In the actual fixture the response_item copy always arrives first (same timestamp), so I keep whichever arrives first and skip the mirrored event_msg instead — net effect (exactly one message per duplicated user turn) is identical, but doesn't require buffering/lookahead that the single-entry normalizeEntry(entry, sessionId, source) signature has no room for.
  • driverVersion: the ask was "driver object... driverVersion '0.1.0'," but HarnessDriver has no such field (only CanonicalEvent does). Stamped driverVersion: '0.1.0' on every emitted event instead, which is what the schema actually supports.
  • wait special-case: the doc frames this as a custom_tool_call case, but in the fixture wait is encoded via function_call. Applied the coordinatorIdle flag to whichever encoding actually carries name === 'wait'.
  • toolNameMap: not explicitly spelled out in the D1 capabilities bullet, but added (apply_patchedit, execbash) so the shell_churn/phase-detection checks the capabilities list already declares aren't permanently dead for Codex's tool vocabulary — mirrors the "Codex flavour" scenario already covered by the existing derive-capabilities.test.ts.

Test plan

  • npm test (tsc --noEmit && vitest run) — 51 files, 455 tests, all passing
  • npm run test:live — passes, and now correctly discovers + normalizes a real local Codex session
  • Pre-push hook ran clean (no bypass)

🤖 Generated with Claude Code

https://claude.ai/code/session_01U9EpjZNoWaXLzfG93tdvTU


CodeAnt-AI Description

Add Codex session support and use the right driver in mixed sessions

What Changed

  • Added support for discovering and reading Codex rollout sessions, including recursive session lookup and Codex-specific event mapping.
  • Codex events now keep deterministic IDs, label their source correctly, and translate Codex tool, message, and lifecycle events into the shared event format.
  • The context packet now identifies the agent from the events in the window instead of always reporting Claude Code.
  • Added a new repeated-search risk check for sessions that keep retrying the same search query.

Impact

✅ Codex sessions can be captured and replayed
✅ Correct agent label in mixed-harness transcripts
✅ Fewer missed stuck-search warnings

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Coldaine and others added 4 commits July 23, 2026 14:15
Implements the Codex HarnessDriver per docs/plans/plan-codex-replay.md D1:
normalizer covers the full mapping table (session_meta, response_item
message/reasoning/custom_tool_call(_output)/function_call(_output),
event_msg task lifecycle, patch_apply_end, mcp_tool_call_end,
web_search_end, token_count sampling, context compaction,
thread_goal_updated, and the user-message dedupe rule), discovery walks
~/.codex/sessions (CODEX_SESSIONS_DIR override) for rollout-*.jsonl, and
capabilities declares tool-args file attention, a toolNameMap onto the
generic vocabulary derive.ts's phase/shell_churn checks already key off
of, and the new repeated_searches heuristic (implemented in a follow-up
commit). Event IDs are deterministic (sessionId + entry index), required
for reproducible replay. Registered in the driver registry alongside
claude-code, which remains the default.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9EpjZNoWaXLzfG93tdvTU
docs/plans/plan-codex-replay.md D2: context-packager.ts hardcoded
observedAgent: 'claude-code' regardless of which driver actually produced
the event window. Replace with the dominant harnessId across the events
passed to packContext, falling back to 'claude-code' when no event
carries one (legacy fixtures, hand-rolled test events) — the same
default the driver registry already uses elsewhere.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9EpjZNoWaXLzfG93tdvTU
docs/plans/plan-codex-replay.md D1/D4: new capability-driven risk check
for the GHCR-403 stuck signature — >=3 near-identical web_search/search
tool queries in a window (normalized whitespace/case equality for v1).
Implemented in derive.ts's RISK_CHECKS table like every other heuristic,
gated by the 'repeated_searches' capability ID drivers opt into (already
declared by the Codex driver's capabilities.ts); no driver branching.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9EpjZNoWaXLzfG93tdvTU
tests/live/read-real-session.test.ts hardcoded the claude-code
normalizer, an implicit assumption that held only because
discoverActiveSession() previously had just one driver to dispatch to.
Registering the Codex driver breaks that assumption on any machine where
a real Codex session is newer than the newest real Claude session: the
generic dispatcher (rightly) surfaces the Codex session, but the test
then fed its entries through the Claude parser and failed with a
misleading "0 events" instead of exercising the matching driver.

Fix: look up the driver via `driverRegistry.getForSource(session.source)`
(falling back to the registry default), mirroring the exact pattern
session-manager.ts already uses in production. Verified locally: this
now discovers and correctly normalizes a real ~/.codex/sessions rollout
on this machine.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9EpjZNoWaXLzfG93tdvTU
Copilot AI review requested due to automatic review settings July 23, 2026 19:21
@codeant-ai

codeant-ai Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR 8ba49fa Jul 23, 2026 · 19:21 19:26

@codeant-ai

codeant-ai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 041988a0-75da-444d-b301-c5c7ea38229d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/codex-driver

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codeant-ai codeant-ai Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Jul 23, 2026
Comment on lines +44 to +46
const entryIndexBySession = new Map<string, number>();
const tokenCountIndexBySession = new Map<string, number>();
const lastUserMessageBySession = new Map<string, string | null>();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The per-session Maps are module-level and keyed by sessionId, but entries are never removed after a session is done. In a long-running capture process that sees many unique sessions, this state grows without bound and leaks memory. Add explicit cleanup (or bounded eviction) for completed/inactive session IDs. [memory leak]

Severity Level: Critical 🚨
❌ Capture driver retains per-session maps across unlimited sessions.
⚠️ Memory usage grows with number of Codex sessions.
⚠️ Long-running replay risks OOM or degraded performance.
Steps of Reproduction ✅
1. In `src/capture/drivers/codex/normalizer.ts:36-41`, the inline comment states that
`normalizeEntry()` is called once per parsed line by a `session-manager` for Codex rollout
files, meaning a long-lived capture process repeatedly invokes this function for many
sessions.

2. For a first Codex session, e.g. `sessionId = "session-1"`, `normalizeEntry()` processes
an initial `session_meta` entry, which calls `resetSessionState(sessionId)` at lines
48-52. This initializes entries in `entryIndexBySession`, `tokenCountIndexBySession`, and
`lastUserMessageBySession` keyed by `"session-1"` and leaves them in the Maps.

3. As the session continues, each call to `normalizeEntry(entry, "session-1", ...)`
updates the stored values via `nextEntryIndex()` (lines 54-57), the token-count sampler
(lines 395-400), and the dedupe watermark (lines 231-236), but there is no code anywhere
in this file that deletes the `"session-1"` keys from the Maps once the session ends.

4. When the same process later handles additional sessions with distinct IDs (e.g.
`"session-2"`, `"session-3"`, etc.), each new `session_meta` entry again calls
`resetSessionState(sessionId)` (lines 48-52), which creates or overwrites entries for
those IDs but never removes old ones; over time, `entryIndexBySession`,
`tokenCountIndexBySession`, and `lastUserMessageBySession` grow monotonically with the
number of unique sessionIds, causing unbounded memory usage in the capture process.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/capture/drivers/codex/normalizer.ts
**Line:** 44:46
**Comment:**
	*Memory Leak: The per-session Maps are module-level and keyed by `sessionId`, but entries are never removed after a session is done. In a long-running capture process that sees many unique sessions, this state grows without bound and leaks memory. Add explicit cleanup (or bounded eviction) for completed/inactive session IDs.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +350 to +355
if (last !== null && last !== undefined && text.trim() === last) {
lastUserMessageBySession.set(sessionId, null);
return events;
}
events.push(messageEvent(sessionId, source, timestamp, entryIndex, 'user', { text }));
return events;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The dedupe watermark is only cleared on an exact match, so when a mirrored event_msg/user_message is missing (or delayed), the old watermark can persist and later suppress a legitimate user message with the same text. Clear or scope the watermark per immediate pair so dedupe only applies to the next expected mirror event. [stale reference]

Severity Level: Major ⚠️
❌ Some user messages dropped from CanonicalEvent stream.
⚠️ Interaction history incomplete for affected Codex sessions.
⚠️ Risk heuristics see fewer or wrong user turns.
Steps of Reproduction ✅
1. For a given Codex session, `normalizeEntry()` receives a `response_item` entry with
`payloadType === 'message'` and `role === 'user'` (lines 222-237). It emits a `user`
message CanonicalEvent and sets the dedupe watermark via
`lastUserMessageBySession.set(sessionId, text.trim())` (lines 231-236).

2. Due to upstream irregularities (e.g., the mirrored `event_msg/user_message` entry for
that turn never being logged), the expected `event_msg` with the same text does not
arrive. Because the dedupe branch in the `event_msg` handler at lines 338-356 only clears
the watermark on an exact match, `lastUserMessageBySession` continues to hold the old text
value indefinitely.

3. Later in the same session, the user sends another legitimate message whose text matches
the previous one. The harness emits only an `event_msg` with `payloadType ===
'user_message'`, which `normalizeEntry()` processes through the `event_msg` branch at
lines 338-356.

4. In that branch, `lastUserMessageBySession.get(sessionId)` returns the stale watermark;
the condition `if (last !== null && last !== undefined && text.trim() === last)` at lines
350-351 evaluates true, so `normalizeEntry()` clears the watermark but immediately returns
`events` without pushing a new message event (lines 350-355), silently dropping this valid
later user message from the CanonicalEvent stream.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/capture/drivers/codex/normalizer.ts
**Line:** 350:355
**Comment:**
	*Stale Reference: The dedupe watermark is only cleared on an exact match, so when a mirrored `event_msg/user_message` is missing (or delayed), the old watermark can persist and later suppress a legitimate user message with the same text. Clear or scope the watermark per immediate pair so dedupe only applies to the next expected mirror event.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@charliecreates charliecreates Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes

I found three high-confidence correctness/reliability issues on 8ba49fa07d48fb67b582f94f33cfbbde3cf7e981.

Blocking findings

  1. Codex file attention is effectively disabled for the real exec stream.
    custom_tool_call entries carry input as raw JavaScript, and the normalizer stores that string directly in payload.args (src/capture/drivers/codex/normalizer.ts:272-275). The Codex capability declares fileAttention: 'tool-args', but derive.ts only extracts filePath/file_path/path from object-shaped args (src/shared/derive.ts:43-57). The PR-1 fixture has 636 exec calls with string inputs, so the primary Codex tool path contributes no file-attention data. Normalize the relevant path fields or add a Codex-aware extractor, and add a fixture assertion that file attention is populated.

  2. User-message dedupe can both duplicate and drop real turns.
    The implementation only handles the response_item/messageevent_msg/user_message order and only clears the watermark on an exact match (src/capture/drivers/codex/normalizer.ts:231-236, 338-355). If the mirrored event arrives first, both copies are emitted; if a mirror is missing/delayed, the old watermark remains and a later legitimate message with the same text can be suppressed. Scope dedupe to an immediately adjacent pair (or otherwise make the ordering state explicit), and cover both arrival orders plus a missing-mirror/repeated-text case.

  3. Per-session normalizer state grows without bound.
    The three module-level maps (src/capture/drivers/codex/normalizer.ts:44-51) are reset for a session but never retired. A long-running desktop process that discovers many distinct rollouts retains an entry in every map for each session indefinitely. Add lifecycle cleanup or bounded eviction so session state cannot accumulate for the lifetime of the process.

Non-blocking follow-ups

  • D1 specifies the session_meta session_started actor as codex, while this implementation and its tests use system; align the contract or document the intentional generic actor.
  • The fixture integration test hardcodes the session ID instead of exercising discovery → normalization. Add one end-to-end assertion for the real fixture filename/metadata so the stacked-driver identity behavior is covered.

The existing GitHub test and GitGuardian checks are green for this head; I did not rerun CI locally during this review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds first-class support for ingesting Codex “rollout-*.jsonl” sessions into the capture pipeline, including deterministic event IDs for replay, driver-aware session normalization, and a new capability-gated risk heuristic for repeated searches. This fits into the multi-harness capture architecture by registering Codex alongside Claude Code while keeping claude-code as the default driver.

Changes:

  • Introduces a Codex HarnessDriver (discovery + normalization + capabilities) and registers it in the singleton driver registry.
  • Updates context packaging to set observedAgent from the dominant harnessId in the event window (instead of hardcoding claude-code).
  • Adds the repeated_searches risk heuristic (capability-gated) plus comprehensive unit/integration tests and a driver-aware live-session test fix.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/schema.test.ts Extends schema contract test to include codex-rollout as a known event source.
tests/live/read-real-session.test.ts Routes live-session normalization through the discovered session’s owning driver (multi-driver safe).
tests/inference.test.ts Adds tests for dominant-harness observedAgent selection behavior.
tests/derive-capabilities.test.ts Adds capability-gated tests for the new repeated_searches risk heuristic.
tests/capture/drivers/codex-normalizer.test.ts New unit tests covering Codex D1 mapping rows, dedupe, sampling, and deterministic IDs.
tests/capture/drivers/codex-fixture.test.ts New integration test streaming the real Codex fixture through parser→normalizer with aggregate assertions + determinism.
tests/capture/drivers/codex-discovery.test.ts New tests for Codex session discovery, UUID sessionId extraction, env override, and registry integration.
src/shared/schema.ts Adds codex-rollout to KnownEventSources.
src/shared/derive.ts Implements the new repeated_searches risk heuristic within the capability-gated risk check framework.
src/inference/context-packager.ts Computes observedAgent from the dominant harnessId in the event window (with legacy fallback).
src/capture/drivers/index.ts Registers codexDriver after claudeCodeDriver (preserving default-driver behavior).
src/capture/drivers/codex/normalizer.ts New Codex rollout normalizer with deterministic IDs, D1 mapping coverage, dedupe, and token_count sampling.
src/capture/drivers/codex/index.ts New Codex driver entrypoint wiring capabilities + normalizer + discovery.
src/capture/drivers/codex/discovery.ts New recursive discovery strategy for ~/.codex/sessions/**/rollout-*.jsonl (env overridable).
src/capture/drivers/codex/capabilities.ts Declares Codex capabilities, including toolNameMap and opt-in to repeated_searches.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +55 to +58
const info = await stat(fullPath);
if (info.isDirectory()) {
const nested = await findRolloutFiles(fullPath);
results.push(...nested);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The recursive directory walk follows directories discovered via stat without guarding against symlink cycles, so a symlink pointing to an ancestor can cause unbounded recursion and eventual stack/resource exhaustion. Use lstat/realpath with a visited-set (or explicitly skip symlinked directories) before recursing. [performance]

Severity Level: Major ⚠️
- ❌ Codex session discovery may hang on symlink cycles.
- ⚠️ Off-host inference blocked when discovery never returns.
Steps of Reproduction ✅
1. Configure a Codex sessions directory with a symlink cycle, e.g. under
`~/.codex/sessions/` create `loop/` containing a symlink that points back to
`~/.codex/sessions` (used by `getCodexSessionsDir()` in
`src/capture/drivers/codex/discovery.ts:30-32`).

2. Call `discoverActiveSession()` from `src/capture/session-discovery.ts:48-95` with no
`overridePath`, which iterates `driverRegistry.registeredIds`
(`src/capture/drivers/index.ts:16-18`) and invokes
`codexDriver.discovery.discoverSessions()` (`src/capture/drivers/codex/index.ts:6-11`).

3. Inside `createCodexDiscovery().discoverSessions()`
(`src/capture/drivers/codex/discovery.ts:74-88`), `findRolloutFiles(sessionsDir)` (line
41) walks the directory tree using `stat(fullPath)` (line 55) and recurses into any path
where `info.isDirectory()` is true (line 56).

4. When `findRolloutFiles()` encounters the symlinked directory that points to an
ancestor, `stat(fullPath)` follows the symlink and still reports a directory, so the
function repeatedly calls `findRolloutFiles(fullPath)` with no visited-set or symlink
guard, causing unbounded recursion and eventual stack or resource exhaustion during Codex
session discovery.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/capture/drivers/codex/discovery.ts
**Line:** 55:58
**Comment:**
	*Performance: The recursive directory walk follows directories discovered via `stat` without guarding against symlink cycles, so a symlink pointing to an ancestor can cause unbounded recursion and eventual stack/resource exhaustion. Use `lstat`/`realpath` with a visited-set (or explicitly skip symlinked directories) before recursing.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

const packet: ShadowContextPacket = {
sessionId: state.sessionId,
observedAgent: 'claude-code',
observedAgent: dominantHarnessId(events),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: observedAgent is computed from the full events array, while the packet itself only includes a recent/truncated event window. In mixed-harness sessions this can label the packet with an agent that is not the one producing the included context, which misleads downstream inference. Compute the dominant harness from the same event subset that is actually emitted in recentEvents (and after truncation if trimming occurs). [logic error]

Severity Level: Major ⚠️
- ⚠️ Shadow context mislabels active harness for multi-harness sessions.
- ⚠️ Inference prompt Agent field may reference inactive driver.
Steps of Reproduction ✅
1. Build a `DerivedState` using the same pattern as `makeState()` in
`tests/inference.test.ts:9-18`, and construct a `CanonicalEvent[]` where the first 40
events have `harnessId: 'claude-code'` and the last 20 have `harnessId: 'codex'`, so the
total length exceeds `MAX_RECENT_EVENTS` (30) defined at
`src/inference/context-packager.ts:10-13`.

2. Call `buildContextPacket(state, events)` from
`src/inference/context-packager.ts:50-54`, either directly as in
`tests/inference.test.ts:16-20` or via `runInference()` in
`src/inference/shadow-inference-engine.ts:15-24` where `events = await buffer.getAll()` is
passed into `buildContextPacket`.

3. Inside `packContext()` (`src/inference/context-packager.ts:68-82`), `recentEvents` is
computed as `events.slice(-recentWindowSize)` (line 82), so `packet.recentEvents` contains
only the last 30 events, which in this scenario are all from the 'codex' harness.

4. However `observedAgent` is set to `dominantHarnessId(events)` (line 116), and
`dominantHarnessId()` (`src/inference/context-packager.ts:33-47`) counts harnessIds over
the full 60-event array, yielding `'claude-code'` as the majority; the resulting
`ShadowContextPacket` labels the agent as 'claude-code' even though the included
recentEvents and the prompt context fed to the model are dominated by 'codex' events.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/inference/context-packager.ts
**Line:** 116:116
**Comment:**
	*Logic Error: `observedAgent` is computed from the full `events` array, while the packet itself only includes a recent/truncated event window. In mixed-harness sessions this can label the packet with an agent that is not the one producing the included context, which misleads downstream inference. Compute the dominant harness from the same event subset that is actually emitted in `recentEvents` (and after truncation if trimming occurs).

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@kilo-code-bot

kilo-code-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity Count
WARNING 4
Issue Details (click to expand)

WARNING

File Line Issue
src/capture/drivers/codex/normalizer.ts 46 Per-session Maps grow without bound across unique sessions (memory leak)
src/capture/drivers/codex/normalizer.ts 355 Dedupe watermark can persist and suppress legitimate user messages with same text
src/capture/drivers/codex/discovery.ts 58 Recursive directory walk vulnerable to symlink cycles (stack/resource exhaustion)
src/inference/context-packager.ts 116 observedAgent computed from full events while recentEvents is truncated
Files Reviewed (14 files)
  • src/capture/drivers/codex/capabilities.ts
  • src/capture/drivers/codex/discovery.ts
  • src/capture/drivers/codex/index.ts
  • src/capture/drivers/codex/normalizer.ts
  • src/capture/drivers/index.ts
  • src/inference/context-packager.ts
  • src/shared/derive.ts
  • src/shared/schema.ts
  • tests/capture/drivers/codex-discovery.test.ts
  • tests/capture/drivers/codex-fixture.test.ts
  • tests/capture/drivers/codex-normalizer.test.ts
  • tests/derive-capabilities.test.ts
  • tests/inference.test.ts
  • tests/live/read-real-session.test.ts
  • tests/schema.test.ts

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

Labels

size:XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants