Feed the Codex watcher state trackers raw lines, not redacted ones

[alexeyzimarev]

Closes #528 (AI-1844 follow-up — the Codex counterpart of #517).

The bug

Two watcher state trackers read the redacted line list. SecretRedactor.RedactLine replaces any
line over 64 KiB with a placeholder carrying no call_id and a type neither tracker recognises,
and an oversized function_call_output — a build log, a large file read, a verbose test run — is
routine rather than exceptional.

var newLines = drainRead.Lines.Select(SecretRedactor.RedactLine).ToList();   // for SENDING
...
foreach (var line in newLines) UpdateCodexPendingToolCalls(...);             // for DECIDING

SeedCodexSubagentTurnState (WatchCommand.cs:1170) already folds both trackers from the rollout
raw on disk. Only the live drain path was inconsistent — the seed had it right.

Why it matters more for Codex than it did for Claude

1. Sessions stuck Active forever. The stranded call_id keeps PendingCodexToolCalls
non-empty for the watcher's life, pinning toolInFlight true so ShouldEndOnIdle can never fire.
For Codex that isn't a degraded backstop — the desktop app's shared codex app-server never exits
per conversation, so the idle timeout is the only per-conversation session-end path. The session
stays Active in the read model indefinitely and the watcher never exits.

2. #526's live subagent-stop never posts. ShouldPostSubagentStop requires no tool call in
flight, so the same stranded call_id blocks it permanently — reinstating the exact symptom #526
fixed (a finished child's chat card spinning for the parent's whole lifetime).

3. Premature subagent-stop. Observe treats any response_item as the turn re-opening. An
oversized one isn't recognised, so a child that re-engaged after task_complete can be reported
stopped while still working.

(1) and (2) are silent and permanent; (3) is a race.

The fix

Both trackers read drainRead.Lines. Neither emits anything — they read call_id and type only —
so no unredacted content leaves the process and redaction still governs everything sent to the
server. Identical to the Claude fix at WatchCommand.cs:1896-1899.

Testing

CodexToolTrackingSourceTests mirrors ClaudeToolTrackingSourceTests, covering both directions:
the call_id is stranded when the redacted line is used and cleared when the raw one is, and a
redacted response_item fails to re-open the turn while the raw one re-opens it. Payloads are sized
off SecretRedactor.MaxRedactableLineChars rather than hard-coded, per the review of #517.

Being straight about coverage: these tests pin the invariant, and they passed the moment they were
written. The defect is at a call site inside RunWatch's local DrainNewLines, which closes over
loop state and isn't reachable from a unit test — the same limitation as #517. What the tests
guarantee is that if RedactLine's placeholder or either tracker changes shape, the reason these
call sites must use raw lines is asserted rather than assumed.

• CodexToolTrackingSourceTests 2/2, CodexSubagentTurnTrackerTests 18/18,
  UpdateCodexPendingToolCallsTests 6/6, ClaudeToolTrackingSourceTests 7/7, WatchCommandTests 77/77.
• AOT publish clean, no IL3050/IL2026.
• Full unit suite 72 failures vs the 56-77 band this repo's daemon cluster produces on a loaded dev
  machine; integration 3 vs 2 on main, overlapping names (SchemePresent_ProbeHitsAuthConfig fails
  on both). No failures in the affected area on either.

Not fixed here

UpdateCodexPendingToolCalls has no equivalent of #517's resume backfill for the session watcher,
so a watcher reconnecting mid-tool starts with an empty pending set and could idle-end a live Codex
session after KCAP_CODEX_IDLE_MINUTES. Pre-existing and independent of the redaction bug —
SeedCodexSubagentTurnState covers only the collab-child path. Left out to keep this reviewable;
noted in #528.

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Fix Codex watcher tool tracking by using raw drain lines (not redacted)

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Feed Codex in-flight tool tracking from raw drain lines to avoid stranding call_ids.
• Prevent Codex sessions from staying Active forever and unblock live subagent-stop posting.
• Add unit tests proving redaction placeholders break tracking while raw lines preserve state.

Diagram

graph TD
  TF[("Codex JSONL")]
  DN["DrainNewLines"]
  SR["SecretRedactor"]
  SV{{"Server"}}
  PT["Pending tool calls"]
  TT["Subagent turn tracker"]

  TF --> DN --> SR --> SV
  DN --> PT
  DN --> TT

  subgraph Legend
    direction LR
    _file[("On-disk file")] ~~~ _proc["In-process logic"] ~~~ _ext{{"External system"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Include metadata in oversize redaction placeholder

• ➕ Trackers could safely run on the already-redacted list without needing raw lines
• ➕ Avoids maintaining two parallel line streams (raw for state, redacted for send)
• ➖ Requires changing SecretRedactor placeholder shape and ensuring all callers/consumers treat it consistently
• ➖ Risk of accidentally leaking identifiers/structure assumptions into downstream processing; higher blast radius than a call-site fix

2. Make tracker parsing more tolerant (infer state without type/call_id)

• ➕ Would avoid dependence on raw lines entirely
• ➖ Not realistically possible: call_id is the key needed to clear pending calls; guessing would be incorrect and brittle

Recommendation: Keep the PR’s approach: consume raw drainRead.Lines for in-process state decisions while continuing to send only redacted lines. It is the smallest-scope, lowest-risk fix and matches the existing Claude handling; changing redaction output formats would have broader compatibility and safety implications.

Files changed (2) +67 / -2

Bug fix (1) +11 / -2

WatchCommand.csUse raw drained lines for Codex state trackers +11/-2

Use raw drained lines for Codex state trackers

• Switch Codex pending-tool-call tracking and CodexSubagentTurn observation to iterate over drainRead.Lines instead of the redacted newLines list. Adds rationale comments explaining how oversize-line placeholders can strand call_id and break idle-exit and live subagent-stop behavior.

src/Capacitor.Cli/Commands/WatchCommand.cs

Tests (1) +56 / -0

WatchCommandTests.csAdd Codex tool-tracking source regression tests for oversize redaction +56/-0

Add Codex tool-tracking source regression tests for oversize redaction

• Introduce CodexToolTrackingSourceTests to demonstrate that redacted oversize placeholders strand call_id and fail to reopen turns, while raw lines correctly clear pending calls and reset turn completion. Sizes payloads relative to SecretRedactor.MaxRedactableLineChars to stay coupled to the real threshold.

test/Capacitor.Cli.Tests.Unit/WatchCommandTests.cs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1d10fda88

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Restrict raw Codex parsing to Codex transcripts

When a non-Codex transcript contains an oversized line, such as a multi-megabyte Claude or Gemini build log, this unconditional loop now passes the entire raw payload to JsonDocument.Parse even though it cannot affect Codex state; previously newLines contained the tiny oversize placeholder. Because the drain imposes no maximum line size, this adds potentially large UTF-8/JSON allocations and parsing work to every vendor and can stall or exhaust the watcher on large dumps. Gate the raw-line loop on vendor == "codex" so only Codex transcripts pay this necessary cost.

Useful? React with 👍 / 👎.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Verbose DrainNewLines comment block ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The PR introduces overly verbose narrative comments: a long explanatory block inside
WatchCommand.DrainNewLines and lengthy XML doc summaries on the new CodexToolTrackingSourceTests
class/methods that restate issue/PR context instead of keeping code and tests self-explanatory. This
adds noise and increases maintenance burden as behavior and implementation evolve.

Code

src/Capacitor.Cli/Commands/WatchCommand.cs[R1888-1891]

+            // Raw drainRead.Lines, not the redacted newLines: an oversized function_call_output
+            // (a build log) redacts to a placeholder with no call_id, which would strand the id
+            // and pin toolInFlight true forever — and for Codex the idle timeout is the ONLY
+            // per-conversation session-end path, so the session would stay Active for good.

Evidence

Compliance rule 3 requires comments to be concise and to avoid embedding extensive rationale in
code. The citations indicate that WatchCommand.cs now contains a multi-line narrative explanation
(covering idle timeout, a stranded call_id, and a session remaining Active) rather than a minimal
intent-focused comment, and that WatchCommandTests.cs adds multi-sentence XML <summary> blocks
describing detailed desktop app/session lifecycle context; together, these show commentary that is
more verbose than necessary to understand the code/test intent.

CLAUDE.md: Prefer self-explanatory code; avoid overly verbose comments
src/Capacitor.Cli/Commands/WatchCommand.cs[1888-1891]
test/Capacitor.Cli.Tests.Unit/WatchCommandTests.cs[850-855]
test/Capacitor.Cli.Tests.Unit/WatchCommandTests.cs[865-869]
test/Capacitor.Cli.Tests.Unit/WatchCommandTests.cs[885-889]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR added long narrative comments in both production code and tests: `WatchCommand.DrainNewLines` contains a multi-line historical/rationale block, and the new unit tests include lengthy XML `<summary>` documentation that restates issue context. Compliance rule 3 expects comments to be concise and for code/tests to remain largely self-explanatory.

## Issue Context
The underlying implementation and test intent appear clear without embedding extensive background narrative (e.g., session lifecycle details, idle timeouts, stranded `call_id` scenarios). Replace these with brief intent-level comments (and, where helpful, a short issue/PR reference such as `// See #528`) so future implementation changes don’t require maintaining large explanatory prose.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/WatchCommand.cs[1888-1891]
- test/Capacitor.Cli.Tests.Unit/WatchCommandTests.cs[850-855]
- test/Capacitor.Cli.Tests.Unit/WatchCommandTests.cs[865-869]
- test/Capacitor.Cli.Tests.Unit/WatchCommandTests.cs[885-889]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Raw parsing for all vendors ✓ Resolved 🐞 Bug ➹ Performance

Description

DrainNewLines now feeds raw drainRead.Lines into UpdateCodexPendingToolCalls unconditionally, so
non-Codex watchers will JsonDocument.Parse potentially very large lines (e.g., big tool outputs)
that previously redacted to a small oversize placeholder. This can add avoidable latency and
memory/CPU pressure in the watch loop for vendors that don’t use Codex response_item records.

Code

src/Capacitor.Cli/Commands/WatchCommand.cs[R1892-1894]

+            foreach (var line in drainRead.Lines) {
                UpdateCodexPendingToolCalls(state.PendingCodexToolCalls, line);
            }

Evidence

The Codex pending-call loop is executed without a vendor guard and now iterates drainRead.Lines
(raw). UpdateCodexPendingToolCalls always parses the whole line via JsonDocument.Parse.
SecretRedactor explicitly replaces lines over 64KiB with a small placeholder, so using raw lines
removes that previous size-based work bound for non-Codex watchers.

src/Capacitor.Cli/Commands/WatchCommand.cs[1883-1894]
src/Capacitor.Cli/Commands/WatchCommand.cs[1386-1421]
src/Capacitor.Cli/SecretRedactor.cs[5-25]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DrainNewLines` iterates `drainRead.Lines` (raw lines) for Codex pending-tool tracking even when `vendor != "codex"`. Because `UpdateCodexPendingToolCalls` always performs `JsonDocument.Parse(line)`, this can parse very large non-Codex JSON lines that would otherwise have been reduced to `SecretRedactor.OversizeLinePlaceholder` on the redacted path.

### Issue Context
The change is correct for Codex correctness (needs raw lines so oversized `function_call_output` can clear `call_id`). The regression is that the Codex-only tracker runs for every vendor and now loses the 64KiB redaction bound for those other vendors.

### Fix Focus Areas
- src/Capacitor.Cli/Commands/WatchCommand.cs[1883-1894]

### Suggested fix
Wrap the Codex pending-call tracking loop with `if (vendor == "codex") { ... }` (still not gated on title phase/threshold), so only Codex watchers pay the cost of parsing raw lines. If any other vendors truly emit Codex-shaped `response_item` tool call records, extend the condition to those explicit vendor values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[alexeyzimarev]

Both fixed in 643204e.

Raw parsing for all vendors (codex P2 / qodo 2) — confirmed, and a regression I introduced.
You both landed on the same thing and it's a genuine one. That loop was never gated on vendor, on the
original reasoning that a non-Codex line has no response_item and so is a cheap no-op. That held
only while it read the redacted list, where any oversized line arrived as a ~90-byte placeholder.
Reading raw lines removes the 64 KiB bound, so a Claude or Gemini watcher would JsonDocument.Parse
a multi-megabyte tool output on every drain to rediscover that it isn't a Codex record. The Claude
tracker added in #517 was gated (TracksClaudeToolCalls); this one simply never needed to be until
now.

Now gated by TracksCodexToolCalls(vendor), mirroring that predicate so the decision is testable
rather than an inline condition — which is what caught the analogous bug last time. Deliberately not
gated on watcher role: a collab child needs the tracking for ShouldPostSubagentStop, so it's
vendor == "codex" for both roles, covered by the new parameterised test.

The CodexSubagentTurn.Observe loop was already gated (vendor == "codex" && agentId is not null),
so it never had the exposure.

Verbose comments (qodo 1) — fair, trimmed. Cut the production block at DrainNewLines and the
three XML doc blocks in the tests down to intent plus a #528 reference, keeping only what isn't
recoverable from the code: why raw lines are required, and why the two trackers fail in opposite
directions.

Verified after the change: CodexToolTrackingSourceTests 6/6, CodexSubagentTurnTrackerTests 18/18,
UpdateCodexPendingToolCallsTests 6/6, ClaudeToolTrackingSourceTests 7/7, WatchCommandTests 77/77,
AOT publish clean, full unit suite 60 failures against the 56-77 band this repo's daemon cluster
produces on a loaded dev machine — none in the affected area.