Skip to content

feat(hostevent): codex-rollout transcript scanner — enables codex save-nudge - #358

Merged
wcatz merged 4 commits into
mainfrom
feat/codex-rollout-scanner
Aug 24, 2026
Merged

feat(hostevent): codex-rollout transcript scanner — enables codex save-nudge#358
wcatz merged 4 commits into
mainfrom
feat/codex-rollout-scanner

Conversation

@waltskinner

Copy link
Copy Markdown
Collaborator

Follow-up to the Phase 2 adapters: codex stops now get the full save-nudge.

  • ScanCodexRollout parses codex rollout JSONL (timestamped envelopes; only response_item payloads carry model traffic) and counts function_call + local_shell_call items
  • Ghost-save detection is separator-agnostic over codex's flattened tool identity (namespace+name, MCP namespaces take mcp__<server>): both mcp__ghost+ghost_memory_save and legacy flat forms match; other servers cannot collide
  • Parse completes absent codex envelopes to codex-rollout instead of none
  • Golden fixture pins meta/event noise, errored shells, cross-server non-collisions, junk lines

Shapes verified against openai/codex source: rollout/src/lib.rs (decode_rollout_line), protocol/src/models.rs (ResponseItem::FunctionCall, optional namespace, test fixture "namespace": "mcp__codex_apps__gmail"), tools/mod.rs (flat_tool_name concatenates namespace+name), plus recorder test fixtures.

@coderabbitai review

…e-nudge

Rollout JSONL lines are timestamped envelopes ({type: session_meta |
response_item | event_msg | …}); only response_item payloads carry model
traffic. Scanner counts function_call + local_shell_call items; Ghost save
detection is separator-agnostic over codex's flattened tool identity
(namespace+name, MCP namespaces take mcp__<server>), so mcp__ghost +
ghost_memory_save and legacy flat forms both match while other servers
cannot collide.

Parse now completes absent codex envelopes to codex-rollout instead of
none, turning on the capability-gated save-nudge for codex stops.
Golden fixture pins meta/event noise, errored shells, and cross-server
non-collisions. Verified shapes against openai/codex source (rollout
decode_rollout_line, protocol ResponseItem, tools::flat_tool_name) and
the recorder test fixtures.

Signed-off-by: agentskinner <agentskinner@proton.me>
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 84daa08a-9ac3-4e1f-9586-3f3f50ff8bf0


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.

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 02926c6)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@wcatz

wcatz commented Aug 24, 2026

Copy link
Copy Markdown
Owner

/review

@github-actions

Copy link
Copy Markdown

Preparing review...

@wcatz

wcatz commented Aug 24, 2026

Copy link
Copy Markdown
Owner

/review

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 41130d9

@wcatz

wcatz commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Confirmed — the reviewer finding is real. flat := ns + name then strings.Contains(flat, "ghost_memory_save") matches when another server hosts a tool literally named ghost_memory_save (mcp__other + ghost_memory_savemcp__otherghost_memory_save). This contradicts the stated non-collision guarantee in both the code comment and the golden-fixture rationale. Suggested fix per the review: exact-equality against the known flattened identities, or namespace ∈ {"", "mcp__ghost"} AND exact tool-name match.

Substring detection let a different server's identically-named tool
(namespace mcp__other + name ghost_memory_save) count as a Ghost save,
skipping the codex nudge when nothing was saved. Replaced with an exact
identity set: bare legacy names plus mcp__ghost-namespaced forms, per
codex's no-separator namespace+name flattening. Adversarial case added.

Signed-off-by: agentskinner <agentskinner@proton.me>
@waltskinner

Copy link
Copy Markdown
Collaborator Author

Fixed in c908371: save detection is now exact-match against the four flattened identities (bare legacy names + mcp__ghost-namespaced forms). Your adversarial case — mcp__other + ghost_memory_save — is pinned as a test and correctly counts as a non-save. Thanks for the catch.

@wcatz

wcatz commented Aug 24, 2026

Copy link
Copy Markdown
Owner

/review

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 02926c6

@wcatz
wcatz merged commit 594f596 into main Aug 24, 2026
6 of 7 checks passed
@wcatz
wcatz deleted the feat/codex-rollout-scanner branch August 24, 2026 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants