fix(turn): keep eBPF-sourced turns whose opening call was missed

[vaderyang]

Problem

An eBPF-captured Claude Code agent turn whose opening call is missed by the source (connection-setup / uprobe-attach timing) is dropped entirely — every call in the turn ends up at the LlmCalls level with turn_id=None, never assembled into an AgentTurn.

Root cause

Claude Code is a stateless client: every request carries the full messages history plus the current roundtrip's tool_result. So within one turn:

• The opening call (the one whose messages[-1] is user text) is the only call that reports is_user_turn_start = Some(true).
• Every continuation call has messages[-1] = user:tool_result, so is_user_turn_start correctly returns Some(false).

When the eBPF source misses that opening call, no captured call reports a user-turn-start. The tracker's has_user_start guard in emit_or_discard then fails, and the entire partition is dropped as TurnDiscardedNoUserStart — including the real continuation calls and the end_turn terminator.

is_user_turn_start is not buggy here; the verdict is right, the opening call just isn't among the captured calls.

Fix

The captured continuation calls do carry a shared eBPF ProcessInfo (pid / comm / exe) — proof they are one process's contiguous agent traffic. Add a partition_has_pid_attribution fallback to has_user_start:

• When no main-agent user-turn-start survived, but the partition's main-agent calls share process attribution, keep the partition (bump TurnKeptByPidAttribution) instead of discarding it.
• The opening call is gone, but the rest of the turn is real and would otherwise be lost entirely.

Applied symmetrically in refresh_active_snapshot so the in-progress registry view matches the persisted Completed (the metric is bumped only at finalize, not on every snapshot refresh, to avoid double-counting).

Safety

• Turn boundaries unchanged — still driven by is_turn_terminal (end_turn etc.); PID never participates in partitioning.
• Sub-agent guard preserved — the fallback only considers subagent_name.is_none() calls, so sub-agent-only partitions are still dropped even if they happen to carry a PID.
• Inert for passive taps — non-eBPF calls carry process = None, so the fallback never fires; existing passive-tap behavior is unchanged.
• is_user_turn_start semantics untouched — no profile / wire-api change.

Known limitation

A turn kept via this fallback has user_input_preview = None (the opening call that would supply the user text was missed). final_answer_preview, call_ids, and token statistics are unaffected (they derive from the terminal call, not the user-start call). Extracting user text from messages[0] is a profile-layer follow-up, out of scope here.

Commits

• 6d42328 — reproduction tests (continuation-shape verdict + control cases)
• c92be76 — the fallback fix + h-turn unit tests (PID fallback keeps turn; no-PID control still discards)

Verification

• cargo test --workspace green (incl. --features fault-injection concurrent tests).
• New h-turn tests: pid_attribution_keeps_turn_when_opening_call_missed (fallback fires, turn kept, TurnKeptByPidAttribution bumped) and no_pid_no_user_start_still_discarded (control: passive-tap partition still discarded).
• No new clippy warnings on changed files.

[github-actions]

I have enough to write a thorough review. Let me verify one key thing: whether the old extract_user_input reads messages.last() (which would mean a tool_result continuation yields user_input=None), confirming the phantom-row concern for kept-by-PID turns.

Looking at extract_user_input at line 624: it reads messages.last(). For a continuation call whose messages[-1] is user with tool_result blocks only, the filter for type == "text" yields an empty vec, joins to "", trims to empty, returns None. So user_input is None for every continuation call. That means a kept-by-PID partition will produce a turn with user_input_preview = None and user_call_id = None — exactly the "phantom Incomplete with user_input=None" shape that the sub-agent guard was added to prevent. The difference is this is now intentional for eBPF-missed turns. The UI already handles null gracefully ("—"). This is acceptable but worth flagging as a known consequence.

Now I have everything I need to write the review.

Summary

PR #168 fixes a real production drop: when the eBPF source misses an agent turn's opening call (the only call whose messages[-1] is user text), every captured call is a continuation with is_user_turn_start=false, and emit_or_discard was discarding the entire partition as TurnDiscardedNoUserStart. The fix adds a partition_has_pid_attribution fallback: if no main-agent user-start survived but the partition's calls carry eBPF process attribution, keep the turn (new TurnKeptByPidAttribution counter). The logic is sound, the sub-agent guard is correctly preserved, the fallback is inert for passive taps (no process), and it's applied symmetrically in refresh_active_snapshot. The test pair (kept-with-PID, discarded-without-PID) pins both branches. Recommendation: APPROVE with one non-blocking concern about user_input_preview shape and one verification question about PID sharedness.

Blocking

None.

Suggestions

• server/h-turn/src/tracker.rs:688 — partition_has_pid_attribution checks only call.process.is_some(), not that the PID is shared across the partition's calls. The commit message and the internal_metrics.rs:217 comment both describe the rationale as "a shared eBPF ProcessInfo (pid/comm/exe), which is proof they are one process's contiguous agent traffic," but the implementation accepts a partition where a single call has a PID and the rest have none. In practice all eBPF calls for a session usually carry the same PID, so this is unlikely to misfire — but the code is looser than the documented invariant. Either tighten to "≥2 calls share the same pid" or soften the comments to "any main-agent call carries process attribution." Low-impact; flagging because the doc and the code disagree.
• server/h-turn/src/tracker.rs:853 — A kept-by-PID partition will produce a Completed turn with user_input_preview = None and user_call_id = None. extract_user_input (anthropic.rs:624) reads messages.last(), which for every continuation call is a tool_result-only user message → filtered to empty → None. The user_pick fallback at line 855 then finds no main-agent call with user_input.is_some(), so both fields stay None. This is the same "phantom row with user_input=None" shape the sub-agent guard was added to suppress (see comment at line 724), now deliberately emitted for eBPF-missed turns. The UI handles null gracefully (agent-turns.tsx:111 renders "—"), so this is acceptable — but worth a one-line comment in build_turn noting that kept-by-PID turns intentionally have user_input_preview=None, so a future reader doesn't reintroduce a guard that re-breaks this path.
• server/h-turn/src/tracker.rs:730 — Minor: refs is now collected unconditionally before the discard check, whereas before it was only built after the has_user_start short-circuit passed. For the discard path this does one extra small Vec allocation per finalized partition. Negligible, but if the hot path matters the has_user_start check could scan calls.iter() directly first and build refs only when emitting. Not worth changing unless this is a measured hotspot.

Questions

• server/h-turn/src/tracker.rs:694 — partition_has_pid_attribution accepts any call.process.is_some(), but the commit subject says "shared eBPF ProcessInfo." Is the intent to keep any PID-attributed partition, or only partitions where ≥2 calls share a PID? If the latter, the test at line 1387 (two PID'd calls) doesn't cover the "single PID'd call + others without" edge case, and the predicate should compare pid values.

Verified

• Schema mirror: TurnKeptByPidAttribution is a backend-only internal metric (counter in h-common/src/internal_metrics.rs:218), not a persisted column or a serde field on AgentTurn — no console/src/types/api.ts mirror needed. Confirmed no ServiceRow/AgentTurn field changed.
• Metric registration: added to the register_worker list in all four sites that enumerate Turn metrics — stage.rs:73, test_support.rs:30, tracker.rs:1010 (test_metrics), tests/integration.rs:884, tests/reorder.rs:37. No site that bumps the counter is missing the registration.
• Symmetry between refresh_active_snapshot and emit_or_discard: both use main_agent_user_turn_start + partition_has_pid_attribution with the same !has_user_start && guard (tracker.rs:308 and :743). Metric is correctly bumped only in emit_or_discard, not in the refresh path — avoids double-counting, as the comment at :305 notes.
• Sub-agent guard preserved: both new helpers (main_agent_user_turn_start, partition_has_pid_attribution) filter on subagent_name.is_none(), so a sub-agent-only partition with a PID is still discarded. Existing subagent_only_partition_with_user_turn_start_is_discarded test at :1318 is unchanged.
• Passive-tap inertness: LlmCall.process is Option<ProcessInfo> (model.rs:83); passive-tap calls set it to None, so partition_has_pid_attribution returns false for them — the no_pid_no_user_start_still_discarded test at :1427 pins this.
• is_user_turn_start semantics untouched: the anthropic.rs diff is comment + test-fixture only (renamed cc_turn_first_call_request → cc_continuation_call_request, replaced the Chinese-text fixture with an English one, and flipped the FIXME test to pin Some(false)). No change to the function body at :653. Confirmed the three tests now assert the documented behavior.
• No sensitive-info leakage: the original commit's test fixture contained Chinese text ("修改9000上的litellm，把对外GLM5.1转给GLM5.2") that looked like a real operator instruction; the second commit replaced it with generic English ("update the gateway config to route model A to model B"). No IPs, hostnames, credentials, or machine paths in the diff.

---

🤖 Reviewed by the review bot • workflow run

[github-actions]

No sensitive-info leakage. Note the first commit's test body originally contained "修改9000上的litellm，把对外GLM5.1转给GLM5.2" (referencing port 9000 and internal service names "litellm/GLM") but the second commit replaced it with a generic "update the gateway config to route model A to model B" — so the final state is clean. Good.

Now I have everything I need. Let me verify the one remaining concern: whether partition_has_pid_attribution being called with an empty iterator could ever matter (defensive). Looking at emit_or_discard, calls.is_empty() returns early at line 721, so the predicate only runs on non-empty partitions. In refresh_active_snapshot, partition.is_empty() returns early at line 295-297. Safe.

One last thing — let me confirm the refs Vec allocation in emit_or_discard isn't a regression: before, has_user_start iterated calls.iter() directly with a closure; now it materializes refs: Vec<&AgentCall> first (line 730) then reuses it for build_turn. This is fine — the refs Vec was already allocated just below before the change; the diff just moves it up. No perf concern.

Summary

PR #168 adds a partition_has_pid_attribution fallback to the turn-tracker's discard rule: when an eBPF-captured agent turn's opening call was missed (so no main-agent is_user_turn_start=true survives), but the partition's main-agent calls carry a process attribution, the turn is kept (TurnKeptByPidAttribution) instead of dropped. The fix is scoped correctly — turn boundaries are still driven by is_turn_terminal, the sub-agent guard is preserved, and the fallback is inert for passive taps (which carry process=None). Symmetric application in refresh_active_snapshot keeps the in-progress registry view consistent with the persisted turn. Tests are thorough and self-verifying. The approach is sound and the safety argument holds. Recommendation: APPROVE with one non-blocking observation on intent-vs-implementation wording.

Suggestions

• server/h-turn/src/tracker.rs:688–695 — partition_has_pid_attribution is documented (and the commit message describes) as detecting a shared/common PID across the partition's calls ("a shared PID still proves these are one process's contiguous agent traffic", "carry a common eBPF ProcessInfo"). The implementation only checks any(|ic| ... process.is_some()) — it never compares PIDs across calls. A single call with any PID passes; two calls with different PIDs also pass. In practice eBPF attribution is per-flow (stamped once from the first attributed packet in tcp.rs:166), so a (source_id, session_id) partition's calls will almost always share one PID anyway — so this is unlikely to mis-fire. But the doc/comment claims a stronger invariant than the code enforces. Either tighten the code to actually verify PIDs match (e.g. calls.map(...).collect::<BTreeSet<_>>().len() == 1), or relax the doc to say "any main-agent call carries eBPF attribution (which is per-flow, hence shared)".

Questions

• server/h-turn/src/tracker.rs:744–750 — A partition kept purely by PID attribution will emit a turn with user_input_preview = None and user_call_id = None (no user-start call exists; tool_result continuation bodies yield no user_input in build_turn's user_pick fallback at line 855). This is the same "phantom row with no user_input" shape that subagent_only_partition_with_user_turn_start_is_discarded guards against for sub-agents — here it's deliberately allowed through for main-agent PID-attributed calls. Is the intent that the console should display these turns (likely with a blank user-input column), or should they carry a flag in metadata (e.g. {"kept_by_pid_attribution": true}) so the UI / ops can distinguish "real turn with missing opening" from "turn whose user input we captured"? The metric exists for ops observability, but the persisted row has no marker.

Verified

• Metric registration consistency: TurnKeptByPidAttribution is added to define_metrics! (internal_metrics.rs:218), and to every register_worker call site that lists its siblings TurnDiscardedNoUserStart — stage.rs:73 (production turn workers), test_support.rs:30, tracker.rs:1010 (test_metrics), integration.rs:884, reorder.rs:37. No site that registers the sibling misses the new one.
• "Passive taps carry no process" safety claim: traced process population — only set via eBPF in h-capture/src/ebpf/mod.rs:248 and h-protocol/src/tcp.rs:166 (which stamps from pkt.process, itself eBPF-only). All passive-tap / test construction sites set process: None. The fallback genuinely cannot fire for passive captures.
• Sub-agent guard preserved: partition_has_pid_attribution filters ic.agent.subagent_name.is_none(), so a sub-agent-only partition with PIDs still fails the predicate and is discarded. main_agent_user_turn_start applies the same guard.
• Symmetry between emit_or_discard and refresh_active_snapshot: both now compute has_user_start || kept_by_pid with identical predicates; the metric is bumped only in emit_or_discard (correctly noted in the inline comment to avoid double-counting on the per-ingest snapshot refresh path).
• No body-scan SQL smell: no DuckDB/ClickHouse query changes in this PR.
• No schema/TS drift: no Rust serde type fields changed; the new metric is internal-only.
• No sensitive-info leakage: scanned the full diff (bd463e3...261d6f5) for RFC1918/CGNAT IPs, private-key blocks, passwords, and machine paths — none. The first commit's Chinese test string referencing internal service names was replaced by a generic English string in the second commit.
• is_user_turn_start semantics untouched: read the function at anthropic.rs:653 — the PR adds only tests around it, no logic change to the verdict itself. The pinned Some(false) for the CC continuation shape is correct given the function inspects the last non-system message.

---

🤖 Reviewed by the review bot • workflow run