From 1136af793052beb7e405dcc8f02935a22ff3fd7d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 20 Sep 2026 23:14:13 -0700 Subject: [PATCH] chore: remove review artifacts --- .../ATTRIBUTION-parent-label.md | 748 ------------------ docs/audits/attr-parent-label/PR-BODY.md | 165 ---- 2 files changed, 913 deletions(-) delete mode 100644 docs/audits/attr-parent-label/ATTRIBUTION-parent-label.md delete mode 100644 docs/audits/attr-parent-label/PR-BODY.md diff --git a/docs/audits/attr-parent-label/ATTRIBUTION-parent-label.md b/docs/audits/attr-parent-label/ATTRIBUTION-parent-label.md deleted file mode 100644 index aa91d7ebef1..00000000000 --- a/docs/audits/attr-parent-label/ATTRIBUTION-parent-label.md +++ /dev/null @@ -1,748 +0,0 @@ -# Defect B — a parent row displays its child's text - -Base: `779667c1e7` (origin/main at time of writing). -Status: **diagnosis complete, mechanism NOT chosen.** Do not pick a fix before reading -"Constraints" and the reference findings that will be appended to this file. - -## Observed - -A structured agent session (native chat) ran a subagent. The **parent** sidebar row displayed text -that the **subagent** had written. The parent itself had produced nothing since launching the child. - -## Verified mechanism - -Read directly at `779667c1e7`: - -1. `src/main/claude/claude-structured-journal-translation.ts:142-144` - ```ts - if (envelope.parentToolUseId) { - subagents.observeChildActivity(envelope.parentToolUseId) - } - ``` - A frame belonging to a **child** is noted and then **falls through** — no early return, and no - producer tag is attached to the item that is subsequently appended. The child's assistant prose - is appended to the *parent's* journal, indistinguishable from the parent's own output. - -2. `src/shared/structured-agent-session-projection.ts:253-269` - ```ts - export function latestStructuredAgentSessionAssistantMessage( - items: readonly AgentJournalRenderItem[] - ): string { - for (let index = items.length - 1; index >= 0; index -= 1) { - const body = items[index]?.body - if (body?.kind === 'message' && body.role === 'user') { - return '' - } - if (body?.kind === 'message' && body.role === 'assistant') { - ``` - Scans **all** items backwards, stopping only at a user-role message. There is **no filter on - which agent produced the item**, so the newest assistant prose may be the child's. - - Note the function's own docstring says *"The newest assistant prose in the latest user turn."* - The code has no such scoping. **Comment and implementation disagree about what "assistant" - means** — that gap is the defect. - -3. That value becomes the row's `lastAssistantMessage` and is rendered as the row's secondary line. - Confirmed present as a live field: `orca worktree ps --json` returns `lastAssistantMessage` on - 183 of the agent rows on this machine. - -## Reported but NOT personally verified - -From a code map; confirm before relying on any of it: - -- `src/shared/structured-agent-session-live-turn.ts:81-95` — `activeStructuredAgentSessionToolCall` - reportedly scans backwards for the newest running `tool-call`, also without a parentage filter, - so a **child's** running tool can be shown on the **parent** row (`toolName` / `toolInput`). -- `src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tsx:43-51` — reported render - site for the secondary line. - -## Constraints — read before proposing anything - -- `docs/reference/agent-status-store.md:55-62`: readers keep **only** presentation policy; - precedence is decided once, at write time. -- `docs/reference/agent-status-store.md:5-8`: do **not** remove the renderer bridge or its - publication filters in this slice — they still carry native-chat child rows. -- **Test gap, not test coverage:** no test in - `src/shared/structured-agent-session-projection.test.ts` feeds subagent-parented items into - `projectStructuredAgentSessionStatusSummary`; every case there uses a single flat conversation. - So this leak is untested in both directions. A regression test is part of the fix. - -## Design question this defect raises - -Is the right fix to **filter at the reader** (projection skips child-produced items), or to -**attribute at the producer** (journal items carry the producing agent, and every projection is -scoped by it)? The second is the larger change and likely also resolves defects A and C. - -Do not assume. The reference research appended to this file should decide it — and note that a -reader-side filter would be the second reader-side policy in a subsystem whose own documentation -says readers should hold none. - -## Your task - -1. Re-baseline against current `origin/main`; this file may be stale. -2. Confirm or refute the above **at source**. Do not trust this document. -3. Propose a mechanism. Prefer fixing the architecture over adding a guard. State plainly whether - your fix removes the bug class or just this instance. -4. State what the fix does to defects A (`attr-parent-recency`) and C (`attr-child-clock`). -5. Mark every claim VERIFIED or UNVERIFIED. Cite file:line. - ---- - -# Reference findings - -Five independent implementations were reviewed. Projects are deliberately not named here; describe -any conclusion in repo-native terms only. Every mechanism below was read at source and verified. - -## Converged — all five agree - -1. **A child's activity never touches the parent row's lifecycle state, label, or timestamp.** -2. **Attribution happens at ingestion (the producer boundary), never as a reader-side filter.** -3. **Exactly one writer of a row's status/recency.** None of the five keeps two writers apart with - publication filters. They make the second writer *impossible*. -4. **Child -> parent rollups do exist** — but they roll up *status* or a *count*, never a label and - never a timestamp, and each is explicit and documented. -5. **The row label is a stored title derived from the user's own prompt** — never a backward scan - for the newest assistant message. - -## Closest architectural match (same platform: Electron, host-service process + renderer sidebar -## that nests parent and subagent rows) - -- Subagent hook events are routed to a **separate roster at ingestion, before any store write**. - The ingestion comment reads: *"Subagent activity is not the terminal's lifecycle: no chime, no - status change, no session id capture."* -- The store method recording subagent events is documented *"Never touches the parent binding's - lifecycle state."* -- The renderer **derives** status on every read inside a `useMemo` and **stores nothing**. -- The state-entry stamp is preserved server-side as - `prior !== undefined && !sessionChanged ? prior.startedAt : occurredAt`. - **There is no done-state special case.** That is the shape of Orca's *main* canonical writer - (`server-ingest-structured.ts:65`), not the renderer bridge's. - -## How the references make a second writer impossible - -- One reference's generic update API **structurally omits** the status/recency fields, so the type - itself forbids a second writer; a lint rule confines cache writes to owner modules. -- One consolidated *"ten independent writers across six slices"* into a single registry writer and - recorded the decision as an architecture decision record. -- One gates client updates behind a monotonic sequence check and replaces **whole rows**, so a - client cannot mutate an individual field. - -## Where the references DIVERGE — do not claim precedent here - -The child clock (defect C) has **no single answer**: -- one shows **no timestamp at all** on the desktop row, carrying liveness via status + derived counts; -- one shows a **live-ticking elapsed duration** that freezes at completion; -- one carries a true **advancing per-child progress field**. - -All three do quarantine the spawn stamp from display. So C has several legitimate answers — choose -one with stated reasons; you may not justify it by precedent alone. - -## Material deviation to disclose (repo-native terms, for the eventual PR body) - -> Orca keeps two real writers of one status row apart with publication filters. Every reference -> implementation examined removes the possibility of a second writer rather than suppressing its -> traffic. - -## Feasibility fact established in Orca - -The producer already has the attribution and discards it: -- `src/main/claude/claude-structured-item-translation.ts:57` already parses - `parentToolUseId: claudeText(frame.parent_tool_use_id)`. -- `src/main/claude/claude-structured-journal-translation.ts:143-144` uses it only for - `observeChildActivity(...)`, then drops it; the item is journaled with no producer trace. -- The journal schemas carry **no producer field at all** — zero hits for - `parentToolUseId|agentId|parentAgentId|producer|subagentId` in - `src/shared/agent-session-journal-schemas.ts` and `src/shared/agent-session-journal-types.ts`. -- A canonical subagent id already exists next door: - `src/main/claude/claude-subagent-roster.ts:121` (`this.ids.canonical(parentToolUseId)`). - -So attributing at the producer means **persisting a value we already compute and throw away three -lines later** — not plumbing a new one end to end. - ---- - -# Review and mechanism decision (second pass) - -Re-verified at source in this worktree. Every claim below is marked **VERIFIED** (I read the -cited line myself, in this worktree, at the head recorded under "Re-baseline") or **UNVERIFIED**. - -## Re-baseline - -**VERIFIED.** `git fetch origin main` moved `origin/main` from `779667c1e7` (the base this -document was written against) to `25dd70e6118b152cd87c9786cfb54e321bfe13c9`. That is **one** -commit: `25dd70e611 Test: target question card title by testid instead of text (#21153)`, -touching four files — `NativeChatQuestionCard.tsx`, `browser-history-match-budget.ts`, -`browser-history-match.performance.test.ts`, `native-chat-ask-user-question-card.spec.ts`. - -**No drift.** `git diff --stat 779667c1e7..origin/main` over every file this document cites -(the projection, the live-turn readers, both Claude translation modules, both journal -type/schema modules, the subagent roster, the compact agent row, `agent-status-store.md`, the -projection test) returns empty. Every line number this document quotes still says what it says. -Two corrections to line numbers are noted inline below; neither is drift, both were slightly off -when written. - -## Section 1 — the two items the document marked NOT personally verified - -### `activeStructuredAgentSessionToolCall` — **VERIFIED, and the reported line range was off by four** - -`src/shared/structured-agent-session-live-turn.ts:85-98` (the document said 81-95; 82-84 is the -docstring). The body is: - -```ts -for (let index = items.length - 1; index >= 0; index -= 1) { - const body = items[index]?.body - if (readAgentJournalTurn(body)) { - return null - } - if (body?.kind === 'tool-call' && body.state === 'running') { - return body - } -} -``` - -There is no parentage filter, and the scan's only terminator is a turn record. The premise the -report depended on is also **VERIFIED**: a child's tool calls really are journaled into the -parent's journal. `src/main/claude/claude-structured-journal-translation.ts:168-176` appends -every `claudeToolUses(...)` entry unconditionally — the `parentToolUseId` branch four lines -above it does not return — and `:177-192` does the same for tool results. So while a subagent -runs a tool, the newest `running` tool-call in the parent's item list is the child's, and -`projectStructuredAgentSessionStatusSummary` (`structured-agent-session-projection.ts:303-312`) -publishes it as the parent row's `toolName` / `toolInput`. - -**A third reader has the same defect, not reported in the document.** -`isStructuredAgentSessionThinking` (`structured-agent-session-live-turn.ts:54-80`) decides -"thinking" from the newest content item before the turn record, with no parentage filter. A -child's `reasoning` message therefore makes the **parent's** chat show the thinking indicator. -Same mechanism, same scan shape, third instance. - -### The render site — **VERIFIED, with a correction** - -`src/renderer/src/components/sidebar/worktree-card-compact-agent-row.tsx`. The function is -`getCompactAgentSecondary` at **:26-57** (the document said 43-51, which is the inner -`lastAssistantMessage` read only). The precedence the row actually applies: - -- `:43-46` tool preview (`formatAgentToolPreview`, `src/renderer/src/lib/agent-row-tool-preview.ts:17-30`) -- `:47-51` `lastAssistantMessage` -- `:53-56` fallbacks - -So the secondary line shows the **child's tool** first and the **child's prose** second. Both -leaks land on the same line, tool first. `:140` calls it; `:143-145` places it in `trailingText` -and the row `title`. - -`orca worktree ps --json` returning `lastAssistantMessage` on 183 rows: **UNVERIFIED by me** — I -did not re-run it. The field is present on the wire regardless -(`src/main/agent-hooks/server/server-ingest-structured.ts:58-60`). - -## Section 2 — what I verified independently that changes the analysis - -1. **Orca already classifies root vs child at this exact producer, three times, and throws the - answer away twice.** **VERIFIED.** - - `src/main/claude/claude-turn-opening.ts:58-60` — `isRootClaudeFrame(frame)`. - - `:91` — `createClaudeTurnOpener` refuses to open a turn for a non-root frame. - - `:43` — the send-echo turn requires `frame.parent_tool_use_id === null`. - - `src/main/claude/claude-streamed-block-identity.ts:31-33` — the streamed-block registry keys - its map on `` `${sessionId}/${parentToolUseId ?? ''}` ``. It computes the discriminant at - `:67` and does not return it. - - `src/main/claude/claude-structured-history-window.ts:94-104` — `claudePromptBlocks` refuses a - transcript record with `parent_tool_use_id != null` (and `isSidechain`) when deciding what - the user's own prompt was. - - `src/main/codex/codex-subagent-activity.ts:73-76` — `isCodexRootAgentActivity` does the same - job on the Codex lane, from an agent **path** rather than a single parent id. - - This is the decisive fact. Producer-side parentage classification is not a new idea in this - repo; it is established practice on both provider lanes and in the Claude transcript reader. - The journal item is the one artifact that does not carry it. - -2. **The turn boundary the scans stop at is already root-scoped.** **VERIFIED.** On the Claude - lane a `message`/`role: 'user'` render item comes from the write-ahead submission row - (`journal-reducer.ts:239-260`, `applySubmission` upserts the submission body as an item), and - provider user frames journal no message body at all — - `claudeOutputEnvelope` (`claude-structured-item-translation.ts:79-87`) filters a user - envelope's content down to `tool_result` parts, so `claudeMessageBody` returns `null` for one. - Turn records are likewise root-only, per `createClaudeTurnOpener` above. - - **This is what makes the defect exact rather than vague:** the window every scan reads is - bounded by root-written markers and filled with root + child content. The window is - *guaranteed* to contain foreign items, and during a subagent run the newest item in it is - almost always the child's. - -3. **There is no producer field anywhere.** **VERIFIED** — zero hits for - `parentToolUseId|agentId|parentAgentId|producer|subagentId|producedBy` in - `src/shared/agent-session-journal-schemas.ts` and `src/shared/agent-session-journal-types.ts`. - Positive control run on the same files (`kind`: 11 and 14 hits), so the empty result is a fact - about the files and not about my grep. - -4. **`AgentJournalRenderItem` already carries exactly one optional provenance marker, and it is - row-level, not body-level.** **VERIFIED.** `recovered?: true` is declared on - `JournalRowBase` (`journal-row-schema.ts:20-31`), threaded through - `JournalItemAppendOptions` (`journal-store-contracts.ts:42`), `JournalItemAppender` - (`journal-item-appender.ts:11,21-34`) and `journalItemRowBuilder` - (`journal-row-builders.ts:28-44`), and copied onto the render item by the reducer - (`journal-reducer.ts:82` and `:107`). It is declared on the shared type at - `agent-session-journal-types.ts:242-250` and admitted by - `AgentJournalRenderItemSchema` at `agent-session-journal-schemas.ts:223`. That is a complete, - shipped template for the field this fix needs. - -5. **`projectStructuredAgentSessionStatusSummary` has exactly one production caller.** - **VERIFIED** — `src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts:233`. - Blast radius of changing its contents is one host-side writer. - -6. **The test gap is real.** **VERIFIED** — zero hits for `parent|subagent|child` in - `src/shared/structured-agent-session-projection.test.ts` (24 cases, all single-agent). And - the input fixture for the leak already exists and asserts nothing about it: - `src/main/claude/claude-structured-journal-translation-subagents.test.ts:184-199` feeds an - assistant frame with `parent_tool_use_id: 'toolu_1'` carrying the text `'looking'` and - asserts only the roster row. Nothing checks where `'looking'` went. - -## Section 3 — the central question - -### The failure mechanism, stated plainly - -One journal is the durable record of **one agent session**. It is being used as the record of -**N agents** — the session's own agent plus every subagent it spawns — with no field that says -which one wrote a row. Every "what is this agent doing" reader is a backward scan that stops at a -marker only the root agent writes. So the scans are root-scoped at their boundaries and -unscoped in their contents. - -The bug class is therefore: **a shared timeline with no producer field, read by newest-item-before-the-boundary scans.** It is not one function. Three readers have it today -(`latestStructuredAgentSessionAssistantMessage`, `activeStructuredAgentSessionToolCall`, -`isStructuredAgentSessionThinking`), and any fourth reader written the same way inherits it. - -### The verdict: attribute at the producer. This is not close. - -**A reader-side filter is not implementable at the reader.** **VERIFIED.** An -`AgentJournalRenderItem` carries `itemId`, `revision`, `body`, `sequence`, `observedAt`, -`recovered` — and nothing else (`agent-session-journal-types.ts:242-250`). A child's items are -appended under the *parent's* identity shape (`claudeMessageIdentity` → -`{ provider: 'claude', sessionId, uuid }`) and with the *parent's* `session_id` — visible in the -existing fixture, where the child frame carries `session_id: 'claude-session'`, the same value -the parent's frames carry (`claude-structured-journal-translation-subagents.test.ts:192` -against `:64`). There is no bit anywhere in the render item to filter on. - -So "filter at the reader" can only mean *infer* parentage from neighbouring items — "an item -that follows a still-`running` tool-call named `Task` belongs to a child." That option is worse -on every axis: - -- It is a second reader-side adjudication rule in a subsystem whose own documentation forbids - exactly that: `docs/reference/agent-status-store.md:58-59`, "Precedence is decided once, at - write time, with provenance recorded on the row. Readers never re-adjudicate." **VERIFIED at - source.** Note the wording — *provenance recorded on the row* is literally the field this fix - adds. -- It is wrong under concurrency. Two spawns in flight, a backgrounded child still emitting after - its spawn call returned (the backgrounded case is real and already tested — - `claude-structured-journal-translation-subagents.test.ts` "leaves a backgrounded child running - past the end of its turn"), or the parent producing its own prose while a child runs, all - defeat the inference. -- It has to be written four times (projection, two live-turn readers, and mobile, which imports - these same shared functions — `mobile/src/session/use-mobile-structured-agent-session.ts:12-13`). -- It leaves the durable record permanently ambiguous, so the next reader starts from the same - nothing. - -**Does producer attribution REMOVE the bug class or guard it?** It removes it, with one honest -qualification. It removes the *ambiguity* — after it, a reader that wants root-scoped truth can -have it, and a reader that wants the whole transcript can have that. It does not by itself stop -someone writing a fifth unscoped scan; what stops that is that the scoped predicate is the -obvious thing to reach for and lives beside the type. That is a real improvement over today, -where the correct code is impossible to write, but it is not a type-level impossibility. I am -not going to claim more than that. - -### Where I argue AGAINST the producer-attribution framing as this document states it - -Line 74 of this document says the producer change "is the larger change and likely also resolves -defects A and C." **That is wrong, and Section 6 below shows why.** Producer attribution -resolves B. It is a *precondition* for one plausible fix to A, whose cost is a genuine trade-off -that deserves its own decision. It does nothing at all for C. Adopting the field because it -"probably fixes three things" would be adopting it for a reason that is two-thirds false. - -I also reject one shape the framing invites: stamping the **roster's** child identity on the -item. The roster already owns per-child identity, with canonical task ids and tool ids as -aliases because "Claude re-announces a resumed task under a NEW `tool_use_id`" -(`src/main/claude/claude-subagent-roster.ts:1-12`, **VERIFIED**). Duplicating that identity onto -journal rows creates a second copy that disagrees after a resume — the exact thing -`AGENTS.md`'s "prefer deriving state over storing it" warns against. The journal needs to answer -"is this mine", nothing more. See the field shape below. - -## Section 4 — the field, precisely - -### Declaration - -On the row, beside `recovered`, in `JournalRowBase` -(`src/main/native-chat/agent-session-journal/journal-row-schema.ts:20-31`): - -```ts -/** Set when a subagent running inside this session produced the row, rather than - * the session's own agent. Absent on every root row and on every row written - * before this field — so readers test presence and never read absence as unknown. */ -producedBySubagent?: true -``` - -Mirrored onto the render item (`src/shared/agent-session-journal-types.ts`, beside `recovered` -at `:248-249`) and admitted by `AgentJournalRenderItemSchema` -(`src/shared/agent-session-journal-schemas.ts:223`) as -`producedBySubagent: z.literal(true).optional()`. - -**Type: `true | undefined`. Optional. Never `false`.** Same shape as `recovered`, for the same -reason: `false` and absent would be two spellings of one fact, and the reducer's spread -(`...(row.recovered ? { recovered: row.recovered } : {})`) is the established idiom. - -**Why a bare `true` and not a child id.** Readers need one bit: "is this the session's own -agent". Per-child identity already has an owner — the roster, whose ids survive resumes through -an alias map. A raw `parent_tool_use_id` on the row would be a second, alias-unaware copy, and -its presence would invite a future join to the roster that is wrong after any task resume. -Presence also generalises to Codex, where the discriminant is a path of depth ≥ 1 -(`codex-subagent-activity.ts:73-76`) and a single parent id would not fit. If a per-child -transcript view is ever wanted, add a second field then, with the alias question answered. - -**Why on the row and not in the body.** Three reasons, all verified: -- It is provenance of the append, not content — the same category as `recovered`, `ts` and - `fence`, all of which live on the row. -- The body is a deep-validated discriminated union (`AgentJournalItemBodySchema`, - `agent-session-journal-schemas.ts:153-215`) and is also the surface downgraded per client - capability (`src/main/runtime/rpc/methods/structured-agent-session-turn-item-capability.ts`). - A body field means touching the `message`, `tool-call`, `diff` and `status` arms — four - places — and entangling attribution with the capability downgrade. -- `journalRowSchemaVersion()` (`agent-session-journal-types.ts:24-28`) derives the row's version - from its **body kinds**. A row-level field leaves that function untouched, which is what keeps - the migration free (next). - -### Where it is stamped - -Exactly one producer writes Claude journal items, and it already holds the answer. - -1. `src/main/claude/claude-structured-journal-translation.ts`, inside `handleMessage`. The - envelope is parsed with `parentToolUseId` already on it - (`claude-structured-item-translation.ts:57`, **VERIFIED**). One local constant next to the - existing `:143-145` block, passed as an append option at every `deps.sink.appendItem` in that - function: `:165` (message body), `:171` (tool uses), `:183` (tool results), `:195` - (reasoning), and through `appendUnmodeledContent` at `:205`. - -2. **The streamed path, which is not inside `handleMessage` and is the part a naive fix misses.** - Streamed text is persisted from a callback at `:99-106` that has no envelope. The producer is - still available: `claude-streamed-block-identity.ts` already scopes its registry on - `(sessionId, parentToolUseId)` at `:31-33` and computes it at `:67`. Return it on - `ClaudeStreamedTextDelta`, thread it through `createClaudeStreamedTextCheckpoints`'s - `append`/`persist` (`claude-streamed-text-checkpoints.ts:10,17,46-59,75-79`), and the - checkpoint row is attributed. This is deriving, not a new store: the flag is a property of the - block identity the registry already minted inside that scope. - -3. **Lifecycle rows are root by construction** and are left unstamped. Turn records are only ever - opened by root frames (`claude-turn-opening.ts:91`), so `tryAppendLifecycleTransition` and - `appendLifecycleBatch` need no option. Say so in a comment rather than leaving it to be - rediscovered. - -Carrier for (1) and (2): `StructuredAgentSessionAppendOptions` -(`structured-agent-session-event-sink.ts:25-32`) gains the field and forwards it at `:179-192` -(`appendItem`) and `:193-205` (`tryAppendItem`), into `JournalItemAppendOptions` -(`journal-store-contracts.ts:42`), `JournalItemAppender` (`journal-item-appender.ts:11,21-34`) -and `journalItemRowBuilder` (`journal-row-builders.ts:28-44`) — every one of which already -carries `recovered` along the identical path. - -Out for this slice: the Codex producer. The field is provider-neutral and Codex has the -discriminant (`isCodexRootAgentActivity`), but whether Codex journals child output into the -parent journal is **UNVERIFIED** and is its own investigation. Absent on Codex rows means root, -which is exactly today's behaviour there — no regression, no silent half-fix, because nothing -about the Codex lane changes. - -### Rows already on disk — migration and backfill, explicitly - -**There is no migration and no backfill, and both of those are decisions, not omissions.** - -- **No schema version bump.** `AGENT_SESSION_JOURNAL_SCHEMA_VERSION` stays 3. This matters: - `parseJournalRow` fails **closed** on a higher version — `if (version > AGENT_SESSION_JOURNAL_SCHEMA_VERSION) return { ok: false, unreadable: true }` - (`journal-row-schema.ts:148-150`, **VERIFIED**) — and an unreadable row latches the host - read-only rather than skipping. A version bump for an optional provenance flag would make - every journal a new host touches unreadable to an older host, for a field that older host does - not read. Not bumping is not a shortcut; it is the correct read of that gate. -- **An older host reads a new row fine.** `isJournalRow` (`journal-row-schema.ts:178-231`) - validates `kind`, `epoch`, `seq`, `fence`, `ts`, `itemId`, `revision` and the body, and ignores - unknown top-level keys. **VERIFIED** at the `item` arm, `:192-198`. The old host drops the flag - and behaves exactly as it does today. -- **No upcaster.** `upcastRow` (`journal-row-schema.ts:157-166`) has no case to add. There is - nothing to compute for an old row. -- **Old rows read as root, and that is the safe default rather than a lossy one.** Backfill is - impossible in principle — the frame's `parent_tool_use_id` was discarded at ingestion and the - journal is append-only by contract (`journal-row-schema.ts:1-6`). Treating an old row as root - reproduces today's behaviour for that history **exactly**: old transcripts keep rendering as - they render now, and only turns produced on a build that has the field are corrected. Nothing - regresses; the fix simply takes effect going forward. -- **Old rows are the normal case, and here is the bounded consequence.** A session live across - the upgrade has unattributed rows below attributed ones. A backward scan that reaches into the - pre-upgrade region can still pick up a child's prose. It cannot reach far: every scan stops at - the first turn record or user item, and the first turn opened after the upgrade is written by - the new build. The exposure is at most one turn, for sessions open at upgrade, once. Not worth - a repair pass; worth a line in the PR body. -- **`undefined` must never be read as "unknown".** This is the failure mode - `docs/reference/remote-wire-compatibility.md:198-217` documents for `agentWait`, and the - difference is worth stating rather than assuming: `agentWait` has three states because the host - may genuinely not have evaluated a pane. Here there is exactly one producer of Claude journal - items, it parses `parentToolUseId` on every message envelope - (`claude-structured-item-translation.ts:57`), and it therefore always knows. Absence is a - positive claim of root-ness, not an absence of evaluation. Write that in the field's doc - comment so a later reader does not add an "unknown" arm. - -## Section 5 — wire compatibility (mandatory) - -Read `docs/reference/remote-wire-compatibility.md` in full. Three distinct changes, three -categories: - -**(a) `producedBySubagent` on the persisted journal row — not a protocol surface.** Covered -above: no `v` bump, `isJournalRow` ignores unknown keys, old hosts degrade to today's behaviour. - -**(b) `producedBySubagent` on `AgentJournalRenderItem`, which crosses `agentSession.history` and -`agentSession.subscribe` — Rule 1** (`remote-wire-compatibility.md:12-29`), a new optional JSON -field on an existing frame. Safe, with the Rule 1 condition met explicitly: **no reader may ever -require it.** A new client against an old host receives items with no flag and must render -exactly today's behaviour — which it does, because the shared predicate reads absence as root. -Confirmed non-hostile decoding: `AgentJournalRenderItemSchema` is a non-strict `z.object` used -only through `safeParse(...).success` as a type guard -(`agent-session-journal-schemas.ts:217-224, 249-253`), so an unknown key neither fails validation -nor is stripped from the object the caller keeps; and the schemas file's own header -(`:11-14`) states that unknown object keys pass by design. **VERIFIED.** - -**(c) What the host publishes in `AgentSessionStatusSummary` — Rule 3** -(`remote-wire-compatibility.md:61-76`). The frame shape does not move, but the *content* of -`lastAssistantMessage`, `toolName` and `toolInput` changes for the duration of a subagent run. -This is the category that is easy to get wrong, so the argument in full: - -- Rule 3's test is whether old clients can interpret the new projection correctly. They can: the - same fields carry the same kind of value, with the same units and nullability. Only the - attribution is corrected. -- No old client requires them. `getCompactAgentSecondary` - (`worktree-card-compact-agent-row.tsx:26-57`) falls through tool preview → - `lastAssistantMessage` → agent-type label, and `formatAgentToolPreview` - (`agent-row-tool-preview.ts:17-30`) returns `''` on empty input. **VERIFIED.** -- The row does not go blank during a subagent run. With child items excluded, the newest running - tool-call is the spawn call itself, so `toolName` stays populated (`Task`, with the spawn - description as `toolInput`). The row gets *more* accurate, not emptier. -- **No capability gate, and that is a deliberate refusal.** Gating this would mean deliberately - continuing to serve old clients the child's text as the parent's — preserving the defect for - them by design. A gate is the right tool when an old client would *misread* new content - (as with the `turn` item at `remote-wire-compatibility.md:219-244`). It is the wrong tool when - the old client reads the field correctly and the field was simply wrong. -- `orca worktree ps --json` and mobile read the same summary through the same store - (`agent-status-store.md:47-51`), so all surfaces change together. No reader-side divergence is - introduced, which is the property `agent-status-store.md:55-62` asks for. - -**Not in scope, and flagged because it IS a Rule 3 change with teeth:** any change to what -`summary.updatedAt` is derived from. See Section 6, defect A. - -**Harness note.** `tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts` -covers this surface. Adding an optional field keeps it green (Rule 1). Do **not** add an -assertion of the form "the old side lacks `producedBySubagent`" — the baseline is a rolling -release tag and `remote-wire-compatibility.md:136-159` explains why such an expectation reddens -unrelated PRs the first time a release ships the field. - -## Section 6 — the docstring, and the two other defects - -### The docstring: intent is right, the code is the bug — but the docstring hid it - -`latestStructuredAgentSessionAssistantMessage`'s comment -(`structured-agent-session-projection.ts:251-252`) reads *"The newest assistant prose in the -latest user turn."* - -The turn-scoping claim is **not** the gap the document supposed. **VERIFIED:** on the Claude -lane the `role === 'user'` terminator *is* the latest user turn's boundary, because user render -items come from the write-ahead submission row (`journal-reducer.ts:239-260`) and provider user -frames journal no message body (`claude-structured-item-translation.ts:79-87`). Scanning back to -the newest user item and scanning back to the start of the latest user turn are the same scan. - -**So the code implements the docstring, and both are wrong in the same place.** The word doing -the lying is *"assistant"*. In a single-agent conversation it means "the session's agent". In a -session with subagents it silently means "any agent". The docstring is correct about the window -and silent about the producer, which is exactly why the defect survived review. - -**Decision:** the intent is right — the sidebar wants the session's own agent's latest line. -Implement the producer scoping, and rewrite the comment to say *whose* prose, because leaving -"assistant" unqualified is what let the code and the comment agree while both were wrong. A -comment that both parties read as correct is not corroboration. - -### Defect A — idle parent stamped "now" - -**What the field does for A: it makes A fixable in one place. It does not fix A.** Mark the -following mechanism **VERIFIED as a code path** and **UNVERIFIED as the cause of the reported -symptom** — I have not seen defect A's own document or a repro, and I did not reproduce it. - -- `journal-reducer.ts:69` — `state.lastActivityAt = Math.max(state.lastActivityAt, row.ts)`, - applied to **every** row, child rows included. -- `structured-agent-session-status-feed.ts:264` — `updatedAt: journal.lastActivityAt() || now()`. -- `:68` in `summariesEqual` — `(a.status !== 'idle' || a.updatedAt === b.updatedAt)`, so while a - session reads idle, a bare `updatedAt` move is significant and forces a republish. -- `server-ingest-structured.ts:62` — `evidenceObservedAt: summary.updatedAt`. - -So a backgrounded child emitting after its parent's turn settled advances the idle parent's -recency clock. That is one honest mechanism for "idle parent stamped now". - -Note what is **not** broken: `server-ingest-structured.ts:63` reads -`stateStartedAt: priorStatus?.state === state ? priorStatus.stateStartedAt : summary.updatedAt` -— the state-entry stamp **is** preserved across same-state updates, with no done-state special -case. **VERIFIED**, and this is the line the reference section describes (it cited `:65`; the -actual line is `:63` — a two-line correction, not drift). Whatever A turns out to be, it is not -this line. - -**Why A is a separate decision.** Excluding child rows from `lastActivityAt` has a real cost: -that value is the journal's general recency and feeds the 30-minute display decay and -`worktree ps` freshness. A parent whose only live work is a backgrounded child would then decay -to stale while the child genuinely runs — trading a false "active" for a false "stale". That is a -product judgement about which lie is worse, and it should be made in A's own change with its own -evidence, not carried along by B. - -### Defect C — working child showing spawn time - -**The field does nothing for C. Nothing.** **VERIFIED:** the child row's clock never touches a -journal item. `src/renderer/src/components/sidebar/worktree-subagent-child-rows.ts:41` sets -`startedAt` from the roster's `subagent.startedAt` (the spawn stamp), falling back to the -parent's `stateStartedAt`; `worktree-card-compact-agent-row.tsx:59-66` renders it through -`formatShortTimeAgo` for any row that is not `done`. Roster entry to render, no projection in -between. - -The reference section already records that the three implementations reviewed **diverge** on the -child clock, so C has several legitimate answers and none of them is settled by precedent. C -needs its own decision with its own stated reasons. - -### The honest answer to "does one mechanism cover all three" - -**No.** One mechanism covers **one** defect completely (B, in three readers), is a -**precondition** for one plausible fix to a second (A) whose cost is a genuine trade-off, and is -**irrelevant** to the third (C). The current document's line 74 — "likely also resolves defects A -and C" — should be struck. - -## Section 7 — regression tests - -No test today feeds subagent-parented items into the projection. Below is what to add. **Every -one of these must be shown to fail with the stamping removed** — a test that passes against -`main` pins nothing, and a scan asserting "no leak" is a claim about the instrument until a -positive control proves the instrument fires. - -**Producer side** — `src/main/claude/claude-structured-journal-translation-subagents.test.ts`. -The harness at `:20-54` records `{ identity, body }` per `appendItem` and **discards the third -argument**. It must capture options first, or every producer assertion below silently passes on -`undefined`. - -1. A frame with `parent_tool_use_id` set stamps `producedBySubagent` on its message item, its - tool-use items and its tool-result items. Extend the existing case at `:184-199`, which - already feeds exactly this frame and asserts only the roster. -2. A root frame (`parent_tool_use_id: null`) stamps **nothing** — the positive/negative pair. - Without this, an implementation that stamps every row passes test 1. -3. The **streamed** path: `stream_event` deltas under a `parent_tool_use_id` scope produce a - checkpoint row carrying the flag; root deltas do not. This is the case a `handleMessage`-only - fix fails, and it is the single most valuable test here. -4. Lifecycle/turn rows carry no flag (pins the "root by construction" claim). - -**Journal round-trip** — `src/main/native-chat/agent-session-journal/journal-reducer.test.ts`. - -5. A row with the flag reduces to a render item with it; a row without reduces to an item - without. Both upsert paths — the plain `item` row (`journal-reducer.ts:76-83`) and the - `lifecycle-batch` path (`:101-108`) — because they are two separate spreads. -6. **The old-row case, which is the normal case.** A journal whose rows predate the field - (no key at all) parses, reduces, and reads as all-root. Assert against `parseJournalRow` with - a literal legacy line, not a constructed row, so it also pins that `isJournalRow` does not - reject an unknown key and that no version bump crept in. - -**Reader side** — `src/shared/structured-agent-session-projection.test.ts` and -`src/shared/structured-agent-session-live-turn.test.ts`. Both directions, as asked: - -7. *Child text is not leaked to the parent.* Items: root user submission → root assistant prose - `"delegating"` → root `Task` tool-call `running` → child assistant prose `"looking"` (flagged) - → child `Grep` tool-call `running` (flagged). Assert - `latestStructuredAgentSessionAssistantMessage` returns `"delegating"`, not `"looking"`; and - `activeStructuredAgentSessionToolCall` returns the `Task` call, not `Grep`. Assert both on the - summary too, through `projectStructuredAgentSessionStatusSummary` — the field names the - sidebar actually reads. -8. *Parent text is not leaked to the child, and the child's output is not deleted.* There is no - per-child projection today — child rows come from the roster — so the meaningful second - direction is that scoping the status readers does **not** scope the transcript. - `projectStructuredItemsToNativeChat` over the same item list must still return the child's - `"looking"` message. A fix that filters at the wrong level deletes subagent output from the - chat, and only this test catches it. -9. *Same-shaped case for the thinking indicator.* A child `reasoning` item after a root - `Task` call must not make `isStructuredAgentSessionThinking` true. -10. *The mixed-generation case.* Root items, then **unflagged** child items (as a pre-upgrade - journal has), then flagged ones. Pins the documented behaviour — old rows read as root — so a - later change cannot quietly convert absence into "unknown". - -**Not a test, a check:** confirm no mobile RPC golden recording contains journal render items -before landing. `mobile/src/test-support/rpc-recording/` shows no `agentSession.*` family, so the -risk looks nil, but an added field on a recorded payload forces a re-record with a `baseline` -bump and it is cheap to confirm. - -## Section 8 — implementation plan - -Executable in this order. Each step compiles on its own; behaviour changes only at step 11. - -| # | File | Change | -|---|---|---| -| 1 | `src/shared/agent-session-journal-types.ts` | `producedBySubagent?: true` on `AgentJournalRenderItem`, beside `recovered` (`:248-249`), with the doc comment from Section 4 | -| 2 | `src/shared/agent-session-journal-schemas.ts` | `producedBySubagent: z.literal(true).optional()` in `AgentJournalRenderItemSchema` (`:217-224`) | -| 3 | `src/shared/agent-session-journal-producer.ts` *(new)* | `isRootAgentJournalItem(item)` — one predicate, the only place absence is interpreted | -| 4 | `.../agent-session-journal/journal-row-schema.ts` | field on `JournalRowBase` (`:20-31`); comment recording that `v` is deliberately not bumped and why | -| 5 | `.../journal-store-contracts.ts`, `journal-item-appender.ts`, `journal-row-builders.ts` | thread the option exactly where `recovered` is threaded | -| 6 | `.../journal-reducer.ts` | copy onto the render item at `:76-83` and `:101-108`, mirroring `recovered` | -| 7 | `.../agent-session-wire/structured-agent-session-event-sink.ts` | field on `StructuredAgentSessionAppendOptions` (`:25-32`); forward in `appendItem` (`:179-192`) and `tryAppendItem` (`:193-205`); comment that lifecycle appends are root by construction | -| 8 | `src/main/claude/claude-streamed-block-identity.ts` | return the parentage it already computes at `:67` on `ClaudeStreamedTextDelta` | -| 9 | `src/main/claude/claude-streamed-text-checkpoints.ts` | carry it from `append` to `persist` | -| 10 | `src/main/claude/claude-structured-journal-translation.ts` | one local constant in `handleMessage`; pass at `:165`, `:171`, `:183`, `:195`, `:205`; and in the streamed `persist` at `:99-106` | -| 11 | `src/shared/structured-agent-session-live-turn.ts` | scope `activeStructuredAgentSessionToolCall` (`:85-98`) and `isStructuredAgentSessionThinking` (`:54-80`) | -| 12 | `src/shared/structured-agent-session-projection.ts` | scope `latestStructuredAgentSessionAssistantMessage` (`:253-269`) and rewrite its docstring to name the producer; scope `latestStructuredAgentSessionPrompt` (`:239-249`); leave `projectStructuredItemsToNativeChat` and `hasPersistedStructuredAgentSessionTurn` alone, with a one-line reason at each | -| 13 | tests | Section 7, with the ablation for each | - -Two of those "leave alone" decisions need their reason recorded, not just stated: - -- `projectStructuredItemsToNativeChat` — **the transcript shows every agent's output.** The line - this change draws is: *the transcript renders every item; every "what is this agent doing right - now" scan renders only the session's own agent's items.* Scoping the transcript would delete - subagent output from the chat. -- `hasPersistedStructuredAgentSessionTurn` — an existence test ("is this session listable at - all"), not an attribution test. Scoping it could make a session with content read as having - none, and a missing row is worse than a correctly-attributed one. - -`latestStructuredAgentSessionPrompt` **is** scoped even though it is provably a no-op on the -Claude lane today (Section 2, item 2). The reason is the Codex lane, where user-item provenance -is **UNVERIFIED**: scoping costs one predicate call and removes a live question. - -### Risks, each with its handling - -- **`max-lines`.** `claude-structured-journal-translation.ts` is 310 raw lines and the repo caps - at 300/400/600/800 by path (`.oxlintrc.json:160-178`). If step 10 trips the cap, **extract** - — `AGENTS.md` forbids a `max-lines` disable or a per-file bump, without exception. -- **The streamed path is the one that gets missed.** A fix that only edits `handleMessage` looks - complete, passes every test written against `handleMessage`, and leaks on every streamed reply - — which is the common case under `--include-partial-messages`. Test 3 in Section 7 exists - solely for this, and it should be written before step 10, not after. -- **The subagent test harness silently swallows the third argument** (`:20-54`). Fix the harness - in the same commit as the first producer test, or every producer assertion is vacuous. -- **Byte accounting.** `estimateStructuredAgentSessionItemBytes` does not see append options, so - the flag is ~24 bytes per child row uncounted against the sink watermarks. Immaterial at the - 16 MB pause watermark; noted so nobody rediscovers it as a mystery. -- **Cross-version suite.** Run - `pnpm exec vitest run --config config/vitest.config.ts tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts`. - It should stay green under Rule 1. Add no "old side lacks X" assertion. -- **Running tests rewrites `pnpm-lock.yaml`.** Restore with `git checkout -- pnpm-lock.yaml` — - that path only, never a bare checkout. - -### Gates - -`pnpm tc`; `pnpm test src/shared/structured-agent-session-projection.test.ts`, -`src/shared/structured-agent-session-live-turn.test.ts`, -`src/main/claude/claude-structured-journal-translation-subagents.test.ts`, -`src/main/native-chat/agent-session-journal/journal-reducer.test.ts`, -`src/main/native-chat/agent-session-journal/journal-row-schema.test.ts`; the cross-version -agent-session suite; `pnpm run check:code-quality:changed`. - -### For the PR body - -The material deviation this document already records — that Orca keeps two writers of one status -row apart with publication filters where every implementation examined removes the possibility of -a second writer — is **not** what this change fixes, and the PR should not imply otherwise. This -change fixes something narrower and adjacent: one row's *content* was being written from another -agent's output. Describe it in repo-native terms only, and name no external project anywhere in -the PR, the commits, the branch, or the code. - -### Evidence gap to disclose - -The exact frame shapes Claude emits for a subagent's inner tool calls and their results — in -particular that a child's `tool_result` frame carries the same `parent_tool_use_id` as the -`tool_use` that preceded it — are **VERIFIED by code path** (both flow through the one -`envelope.parentToolUseId` read at `claude-structured-item-translation.ts:57`) and -**UNVERIFIED against a captured transcript**. If the producer tests are written from remembered -frame shapes rather than a recorded stream, that assumption is untested. Capturing one before -step 10 would close the last gap in this plan. diff --git a/docs/audits/attr-parent-label/PR-BODY.md b/docs/audits/attr-parent-label/PR-BODY.md deleted file mode 100644 index 2d807798423..00000000000 --- a/docs/audits/attr-parent-label/PR-BODY.md +++ /dev/null @@ -1,165 +0,0 @@ -## ELI5 - -When a chat agent hands part of a job to a helper agent, both of them write into the same -notebook. Nothing in the notebook said who wrote each line. So when the sidebar asked "what is my -agent doing right now?", it read the most recent line — usually the helper's — and showed the -helper's words and the helper's tool on the main agent's row. - -Now every line records who wrote it, and the "what is my agent doing" readers only read their own -agent's lines. The full conversation still shows everything both agents wrote. - -## What Changed - -**Before.** Start a chat session and let the agent spawn a helper agent. While the helper worked, -the parent's sidebar row showed the helper's text as its own secondary line, and showed the -helper's running tool (for example `Grep`) as the tool the parent was running. The parent's chat -also showed the "thinking" indicator whenever the helper was reasoning, even though the parent was -not. The parent had produced nothing since handing the work over. - -**After.** The parent's row shows the parent's own last line and the parent's own running tool — -which, while a helper runs, is the call that spawned it. The thinking indicator follows the -parent's own reasoning. The transcript is unchanged: it still shows everything the helper wrote, -and the helper's own rows in the helper list are unchanged. - -**The mechanism.** A session's durable record is one file, but a session that spawns helpers writes -their output into it too, and no field on a row said which agent produced it. Every "what is this -agent doing right now" reader is a scan backwards from the end of the list that stops at a marker -only the top-level agent ever writes. So those scans were correctly bounded at their edges and -completely unscoped in their contents — the window they read was guaranteed to contain another -agent's rows, and during a helper run the newest row in it was almost always the helper's. - -The change records the producer where the row is written, and scopes the readers by it: - -- Rows gain an optional `producedBySubagent` marker, shaped exactly like the existing `recovered` - marker next to it — present or absent, never `false`. Absence means the session's own agent. -- The Claude translator already read each frame's parent-call id and discarded it. It now stamps - every row that frame produces: the message body, each tool call, each tool result, reasoning, and - the generic fallback row. It also stamps **streamed** text, which is written from a callback that - has no frame in scope — that path takes the marker from the streamed-block registry, which - already scopes itself on the same parent-call id. A fix that only covered the frame path would - have looked complete and still leaked on every streamed reply. -- Turn records and other lifecycle rows are left unstamped, because a turn is only ever opened by a - top-level frame. That is now stated in a comment and pinned by a test. -- The three readers that leaked — the newest assistant line, the active tool call, and the thinking - indicator — now skip rows another agent produced, through one shared predicate that is the only - place absence is interpreted. -- Two readers are deliberately **not** scoped, each with its reason recorded in the file: the - transcript renders every agent's output, and the "does this session have any content" check is an - existence test, not an attribution one. - -No schema version bump, no upcaster, no backfill. A row from a higher version is treated as -unreadable and puts the host into read-only, whereas an unknown key is simply ignored — so a -version bump for an optional marker would make journals written by a newer build unreadable to an -older one, for a field the older build never reads. Rows written before this change read as the -session's own agent, which reproduces today's behaviour for existing history exactly. The one -bounded consequence: a session that is open across the upgrade can still pick up a helper's line -for at most one turn, because the scans stop at the first turn marker and the first turn opened -after the upgrade is written by the new build. - -## Why - -The alternative was to filter at the reader — infer that a row belongs to a helper because it -follows a still-running spawn call. That is not implementable honestly: a helper's rows are written -under the parent's identity and the parent's session id, so there is no bit in the rendered row to -filter on. Inference also breaks under exactly the cases that already exist and are already tested, -such as a backgrounded helper still emitting after its spawn call has returned, or two helpers in -flight at once. It would have to be written four times over, and it would leave the durable record -permanently ambiguous, so the next reader starts from the same nothing. - -Recording the producer where the row is written is also what this subsystem's own reference -document asks for: precedence is decided once, at write time, with provenance on the row, and -readers never re-adjudicate. Producer-side classification of top-level versus helper frames is -already established practice here — it exists in turn opening, in the streamed-block registry, in -the transcript reader, and on the Codex lane. The journal row was the one artifact that did not -carry it. - -The marker is a bare `true`, not the helper's id. Readers need one bit: "is this mine". Per-helper -identity already has an owner — the helper roster, whose ids survive a task resume through an alias -map — and copying that identity onto rows would create a second copy that disagrees after any -resume. A presence bit also generalises to Codex, where the discriminant is a path rather than a -single id. - -Honest scope: this removes the ambiguity, so a reader that wants its own agent's truth can now have -it. It does not make a fifth unscoped scan impossible to write; what discourages one is that the -scoped predicate now lives beside the type and is the obvious thing to reach for. - -**Material deviation.** Orca keeps two writers of one status row apart with publication filters; -this change does not remove that structure. It fixes something narrower and adjacent: one row's -*content* was being written from another agent's output. - -## Linked Issue - -Fixes # - -## Visual Proof - -Not attached. The user-visible change is one line of text and one tool name on a sidebar row, and -reproducing it needs a live session that spawns a helper agent mid-turn. The behaviour is pinned -instead by tests asserting the exact fields the row reads — `lastAssistantMessage`, `toolName` and -`toolInput` — in both directions, each shown to fail without the change. - -## Testing - -`pnpm tc`, `pnpm test` on the five affected files (108 tests), `oxlint`, and -`pnpm run check:code-quality:changed` all pass. The cross-version wire suite stays green: the new -render-item field is optional, and no reader requires it. - -Every new test was checked against the pre-change behaviour and observed to fail, then pass after: - -- Removing the reader scoping reddens 7 of the reader tests. -- Removing the producer stamping reddens both stamping tests. -- Removing **only** the streamed-text stamping — the shape a frame-path-only fix would take — - reddens exactly the streamed test and nothing else. -- Over-scoping the readers reddens the "the parent's own line still shows" tests. -- Scoping the transcript by mistake reddens exactly the "the helper's output is still in the chat" - test, and nothing else. -- Stamping lifecycle rows reddens the "turn records stay unstamped" test. -- Dropping the marker in the row builder or in the reducer reddens the round-trip tests. - -Two tests fail in a full local run and are unrelated: one drives the real Claude CLI and asserts -against this machine's own command catalogue (it fails identically with the change reverted), and -one is a temporary-directory cleanup race that passes in isolation. - -Platforms: logic-only, no platform-specific code paths; exercised on macOS. No change to remote or -SSH behaviour — the same summary reaches every surface through the same store, so no reader-side -divergence is introduced. - -- [x] I manually tested these changes locally -- [x] Automated tests added/updated, or explained why not below - -## AI Disclosure - -## Review - -## Agent skill upstream boundary - -- [x] Not applicable, or this change follows `docs/reference/agent-skill-sharing-upstream-boundary.md` and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation. - -## Notes - -- **Backwards compatibility.** Covered above: no version bump, unknown keys are ignored by older - hosts, older rows read as the session's own agent. -- **Remote wire.** Three surfaces, three categories. The persisted row is not a protocol surface. The - rendered item gains an optional field, which is safe as long as no reader requires it — none does, - because absence is read as top-level. The *content* of the status summary changes for the duration - of a helper run; this is deliberately **not** capability-gated, because gating it would mean - continuing to serve older clients the helper's text as the parent's, preserving the defect for them - by design. Old clients read those fields correctly; the fields were simply wrong. -- **Mobile.** Mobile reads the same shared readers, so it is corrected by the same change. No golden - RPC recording carries journal render items, so no re-record is forced. -- **Performance.** One optional boolean per row; the readers gain one property check per item. -- **Security.** No new data crosses any boundary; the marker is derived from a field the producer - already parsed. -- **Not in scope.** The Codex producer. The marker is provider-neutral and Codex has the same - discriminant available, but whether Codex journals helper output into the parent's record is - unverified and is its own investigation. Absent on Codex rows means top-level, which is exactly - today's behaviour there. - -## Checklist - -- [x] This PR is small and focused -- [x] I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives) -- [x] Before/after screenshots or videos attached for UI changes, or `N/A` with reason -- [x] Self-reviewed for correctness, security, and performance -- [x] Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A) -- [x] `pnpm lint`, `pnpm typecheck`, `pnpm test`, and `pnpm build` pass (or CI will cover; local preferred)