Skip to content

feat(agent-status): child work records say what the child is doing, how it ended, and when - #22521

Open
brennanb2025 wants to merge 14 commits into
mainfrom
brennanb2025/c2-child-record-contract
Open

brennanb2025 wants to merge 14 commits into
mainfrom
brennanb2025/c2-child-record-contract

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 10 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1548 $\color{#cf222e}{\Huge{\mathbf{−}}}$​28 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1520
Prod 12 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​876 $\color{#cf222e}{\Huge{\mathbf{−}}}$​147 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​729

ELI5

Orca keeps one record per subagent (and per background shell) that a session starts. Today that record can say a child exists and whether it is live, but not what it is doing right now, how it ended, or when. This PR teaches the record those three things, adds one read-only "view" of it for the sidebar and chat strip to read later, and adds the one function that decides which dot a subagent row shows. Nothing writes these new facts yet and nothing reads them yet, so nothing changes on screen. This is the shared contract the rest of the stack builds on: #22536 (Claude) and #22553 (Codex) write these facts, and #22614 makes the sidebar and chat strip read them.

Merge order

What Changed

All changes are in pure shared code (src/shared/agent-status-child-work*). There is no producer wiring and no surface change.

Record fields (AgentChildWorkInput, all optional)

Field Meaning Written by Travels on the view
parentChildWorkId?: AgentChildWorkId The child that owns the current invocation: a nested agent's spawner, or the agent that launched a shell. When absent, the session's main agent owns it. producer yes, but only once resolved (see below)
residency?: 'foreground' | 'background' Whether the provider says the child may outlive the turn that launched it. When absent, the provider did not say. producer no (host-only; settlement reads it)
operation?: { toolName: string; input?: string; basis: 'open' | 'reported'; observedAt: number } What the child is doing now. open means a start edge was seen with no end edge yet. reported means the provider's latest heartbeat named it; no end edge will come, so the next report or the settlement replaces it. input is the same one-line preview a status row carries as toolInput. producer yes
lastMessage?: string (≤ 512 chars, one line) The newest thing the child said. outcome tells whether that is a result or an error. producer yes
settledAt?: number Host time the current invocation settled admission only, never a producer yes

The existing observedAt stays the child's own evidence clock. AGENT_CHILD_WORK_STATES, AGENT_CHILD_WORK_OUTCOMES and the kinds are unchanged, so this adds no enum values.

Why a separate outcome vocabulary from mainAgent.outcome? The child outcome enum is not new here — main already carries AGENT_CHILD_WORK_OUTCOMES; this PR only makes it required on settled records. It names the verdict on a child's whole invocation (a Codex child spans several turns), not one turn, and settled records store unknown explicitly so the admission rule can refine a stored unknown to the real ending while never downgrading a definite one — with an absent-means-unknown convention, 'settled with unknown' and 'not yet settled' would be the same shape.

Legality matrix (enforced by parseAgentChildWorkInput, so by every store write and snapshot restore)

membership legal state outcome settledAt operation
live working, waiting, blocked, idle, unverifiable; monitoring only for kind command/monitor absent absent only with working/waiting/blocked
settled done only required required, with firstObservedAt ≤ settledAt ≤ observedAt absent

An agent record never stores monitoring. Its monitoring is derived (see the display rule below).

Codec rules

  • This codec is host-internal only. It is the store's integrity gate and rejects a whole record over one unknown key or enum arm, and this PR itself adds five keys an older strict parser would reject. Anything that decodes records or views from another build must ignore unknown keys and degrade unknown arms, or negotiate (docs/reference/remote-wire-compatibility.md, Rules 1 and 4). A ratchet test (agent-status-child-work-codec-boundary.test.ts) fails if anything outside the host store and admission modules imports the codec.
  • A top-level key that is not on the allow-list rejects the record, as before.
  • Every malformed field rejects the record, old or new, descriptive or not. Admission (below) turns bad provider facts into "not said" before they reach the codec, so a value the codec sees outside that form is a bug in whatever wrote it, and the codec refuses it rather than repairing it. The only production writer is admission; snapshot restore and the transport envelope have no production caller.
  • Text fields (name, agentType, model ≤512, description ≤8,000, operation.toolName ≤60, operation.input ≤160, lastMessage ≤512) must be exactly what the one text normalizer (normalizeChildWorkText) outputs: one line, trimmed, within the cap. The codec's check is defined as "normalizing it changes nothing", so it accepts every value admission can store and nothing else. Line and paragraph separators (U+2028, U+2029) and NEL (U+0085), which the old check let through and a row renders as a line break, are now refused.
  • operation must also have a known basis and an observedAt inside [firstObservedAt, observedAt]; residency must be in the vocabulary; parentChildWorkId must be a valid id other than the child's own.
  • A well-formed field in an illegal cell (for example an operation on a settled record) rejects the record.
  • A non-timestamp settledAt or an out-of-vocabulary outcome rejects the record.
  • A settled record written without outcome / settledAt (the shape the previous codec accepted) reads as outcome: 'unknown' and settledAt: observedAt, never as success. A restored legacy settled child therefore survives the stricter codec (store snapshot restore and resume history are both tested).

Admission rules (announce / adopt / resume)

  • AgentChildWorkObservationFields accepts parentChildWorkId, residency, operation and lastMessage. There is no settledAt on requests.
  • Producers may pass raw provider text for every text field, labels included. Admission runs each through normalizeChildWorkText at the codec's cap: the existing status-field preview (normalizeOptionalField, unchanged for its other callers), then any control character, NEL, U+2028 or U+2029 becomes a space, then the ends are trimmed. A label cut on a space, or one with a line break, is now stored in one-line form instead of rejecting the announce.
  • Admission parses before it merges. Each descriptive request field becomes either a value the codec accepts or "not said": text as above; operation by the state rule, a known basis and the clock clamp; the owner if it is a valid id other than the child's own; residency if in the vocabulary; tokens if a safe integer ≥ 0; providerTiming through the codec's own timing parser. A malformed fact is therefore dropped from that request only: it neither rejects the observation nor erases the stored value.
  • Admission clamps operation.observedAt into [firstObservedAt, request observedAt]. An operation stamped in provider time, or slightly ahead of the request, is kept with the clamped time instead of being silently dropped by the codec. A non-numeric value is still dropped as malformed.
  • Admission drops operation when the request's membership/state cannot carry one, instead of rejecting the request. A stale descriptive field can never block a settle.
  • settledAt is set to the request's observedAt on the first settled observation of an invocation (a newly created settled child, or a live→settled update). Later settled evidence (for example a late lastMessage) keeps it. A resume records the superseded invocation's settledAt in previousInvocations (previously it used the newest observedAt).
  • Settled history only gains precision (same invocation, settled request). An omitted outcome counts as unknown.
    • Same outcome: admitted; later evidence such as a late lastMessage updates the record.
    • Stored unknown → a definite outcome: admitted, and the original settledAt is kept. This is the normal Claude background settle: the roster omission lands first and the frame naming the outcome follows in the same tick. It also covers a late spawn result.
    • Stored definite → a different definite outcome: rejected (stale-invocation), with the record unchanged.
    • Stored definite → unknown (explicit or omitted): admitted, keeping the stored outcome. An unknown claims nothing about the ending, so a definite one is never downgraded, while the rest of that evidence (a late last message, tokens, a new alias) still lands.
    • A settled child receiving live evidence is still rejected; reactivation goes through resume (generation + 1).
    • conflictsWithSettled also requires request.state === child.state. Settled state is always done by the legality matrix, so the equality only rejects a malformed settle early with stale-invocation instead of letting it fail later as invalid; it can never block a legal refinement.
  • buildAgentChildWork(request, host: { childWorkId, firstObservedAt, invocation, previousInvocations?, settledAt? }, prior?): the positional parameters became one host-fields object. prior is the stored record on update and resume.
  • A sparse observation never erases what the record knows. One merge applies one rule per fact, over a type that lists every descriptive request field, so a new field without a rule fails to compile. Labels and residency: replaced when said, otherwise kept. Tokens: the larger count. Outcome: refine-only (see above). Owner, last message and providerTiming: replaced when said, kept within the invocation, and reset by a new one. operation: replaced, and cleared when not said. See "Architecture review".

Alias kinds

AgentChildWorkAliasKind is now 'task_id' | 'tool_use_id' | 'thread_id' (AGENT_CHILD_WORK_ALIAS_KINDS). A Codex subagent is aliased by its own child thread id under thread_id (the Codex producer uses this). The CLI hook lane needs no new kind: Claude's hook agent_id is the same registry id as the SDK task id, so the hook lane registers it under task_id; Codex's hook agent_id is the child thread id, registered under thread_id. No agent_id alias kind exists on purpose. The view's provider-id preference is a Record<AgentChildWorkAliasKind, number>, so a new kind must be ranked before it compiles. Alias records are host-internal: aliasKind has no reference outside the shared child-work modules, and the store snapshot/transport envelope has no production consumer, so this needs no wire negotiation.

View (src/shared/agent-status-child-work-view.ts)

type AgentChildWorkView = {
  id: AgentChildWorkId            // childWorkId
  providerId?: string             // id today's wire uses (tasks[].id / subagents[].id)
  kind; name?; description?; agentType?; model?
  state; membership; outcome?
  operation?; lastMessage?
  parentChildWorkId?              // only when resolved
  firstObservedAt; observedAt; settledAt?
  totalTokens?; stoppable
  invocation                      // the fence a targeted stop names
}
projectAgentChildWorkViews(records: readonly AgentChildWorkInput[], aliases: readonly AgentChildWorkViewAlias[]): AgentChildWorkView[]
  • The view drops residency, previousInvocations, provenance, providerTiming, parent, provider and revision. Nested objects are copied, so a view never aliases store internals. Input order is kept.
  • providerId is the child's alias for the current invocation, preferring task_id, then thread_id, then tool_use_id (stable handles before per-call ones). It is absent when there is none, and the legacy shapes then omit the row. A lane that publishes a background-task id today registers it as the task_id alias so the legacy id is unchanged.
  • parentChildWorkId is kept only when the owner is in the same projection, belongs to the same parent subject, and the record is not on an ownership cycle. Otherwise the main agent owns the work.

Display state

type AgentChildDisplayState = 'working' | 'monitoring' | 'waiting' | 'blocked' | 'done' | 'failed' | 'interrupted' | 'idle' | 'unverifiable'   // every value is an AgentStateDot state
agentChildWorkOwnedLiveness(views, ownerId): AgentChildWorkLiveness   // live work beneath the child at ANY depth
deriveAgentChildDisplayState(view, ownedLiveness): AgentChildDisplayState

The mapping into the existing parent-row fold (foldAgentLeadStatus, imported, not edited):

  • unverifiable bypasses the fold and stays unverifiable. A stored monitoring (shell or monitor only) returns monitoring.
  • Own state done or idle, or membership settled, enters the fold as leadState: 'done'. working, waiting and blocked pass through unchanged. interrupted is always false, because a child's cancel never hides the work it left running.
  • If the fold returns something other than done, the display is workingMode ?? stateName. So an idle or finished child with a live shell it owns reads monitoring, exactly as a CLI agent's row does, and one owning a live agent reads working.
  • If the fold returns done, a live (idle) child reads idle. A settled child reads by outcome: succeeded→done, failed→failed, cancelled→interrupted, unknown→idle (neutral).
  • The inherited fold changed on main after this branched, and the child display inherits it, deliberately. The imported fold and liveness gained a waiting-child arm (feat(agent-status): publish the main agent's own state beside the combined row state #22452/feat(agent-status): combine Codex child work through the shared main-agent status fold #22475): a live descendant in waiting now makes its owner read waiting, for a child row exactly as for a parent row, because both go through the one fold. Verified against current main: the merge is clean and every suite here plus the fold/liveness/parity suites pass on the merged tree.

Legacy shapes, derived from the view

AgentChildWorkLegacyProjectionCandidate is now a flat subset of AgentChildWorkView, so every view is a candidate. A published background task still converts through agentChildWorkProjectionCandidateFromBackgroundTask, and the status bridge's call is textually unchanged. Output for today's inputs is byte-identical: a golden captured on unmodified main is the first commit and passes unchanged after. For record-derived input:

  • projectAgentChildWorkLegacySubagents admits live agents only (the legacy roster never listed settled children).
  • projectAgentChildWorkLegacyBackgroundTasks publishes a settled view's run state as today's host does: succeeded→done, failed→blocked, cancelled→idle, unknown→done. So an old strip reads a settled view exactly as it reads a settled task now.

Why

A child is materialised three times today: the provider tracker's DTO read by the chat strip, a renderer-side conversion for the sidebar, and the hook-lane roster. Each hop drops a fact: monitoring collapses to working, outcome is lost (failed becomes blocked), and the tool name is discarded. The fix is one host-owned record per child with every surface reading a projection of it. This PR is that record's contract. It stores the facts (owner, residency, operation, last message, settle time), keeps outcome separate from lifecycle state as the turn-outcome work already does, and derives display rather than storing it. Legacy wire shapes are derived from the same view, so the old and new shapes cannot disagree.

The per-child display goes through the same fold a parent row uses rather than a second policy. The requirement is that a subagent running a shell looks the way a CLI agent running a shell looks, and sharing the function makes that true by construction.

Alternatives considered:

  • Store-enforced owner integrity. Rejected: removing an owner would reject whole retention mutations, and a live shell record would be lost if its owner were not yet recorded. Resolving ownership in the view degrades to "the main agent owns it", which is the documented fallback.
  • Rejecting a settle that still names an operation. Rejected: that would strand a child live forever over a stale descriptive field.
  • A boolean "is nested" instead of parentChildWorkId. Rejected: it cannot say which child owns a shell, and that is exactly what the per-child monitoring derivation asks.

Linked Issue

None — part of the structured chat status/orchestration program.

Visual Proof

N/A. This PR adds shared contract code only. No producer writes the new fields and no surface reads the view yet, so no pixels change. The legacy projections' output for today's inputs is pinned byte-identical by a golden captured on unmodified main.

Testing

  • pnpm tc:node, tc:web and tc:cli pass (tsc exit 0, 0 errors each).
  • Every test file that imports a changed module, the status bridge, or the structured status fold: 34 files, 351 passed, 1 expected-fail (a pre-existing it.fails), run with env -u ORCA_STRUCTURED_SESSION.
  • New tests:
    • agent-status-child-work-codec-boundary.test.ts: ratchet on the codec's importers.
    • agent-status-child-work-legality.test.ts: 19 illegal cells rejected, 17 legal cells admitted unchanged, text already in one-line form admitted at every cap, 24 malformed fields rejected (including a label or message with U+2028, NEL, a tab, or a trailing space), and the legacy settled restore through a store snapshot and resume history.
    • agent-status-child-work-admission-activity.test.ts: normalization, clamping the operation clock, owner and residency pass-through, operation cleared at idle and at settle, settledAt stamping and keeping, omitted-outcome repeat, unknown → definite refinement keeping the first settledAt, definite → different definite rejected, definite → unknown (explicit and omitted) keeping the definite outcome while its last message and alias land, resume history, and the thread_id alias.
    • agent-status-child-work-view.test.ts: view shape, provider-id preference, owner resolution (dangling, cross-session, cycle), a 16-row literal display table including idle child + owned live shell → monitoring, fold parity for idle and finished children, transitive owned liveness, legacy shapes from views, and an end-to-end admission → store → view → display run (finished child reads monitoring while its shell runs, then done).
    • agent-status-child-work-legacy-golden.test.ts: captured on unmodified main (first commit), passes unchanged after.
  • Ablations, each deleting one mechanism with an exactly-once edit, run at the final head and then restored:
Deleted Result
owned-work input to the child fold 8 red
the idle → done fold mapping (the literal fold) 3 red (idle + owned shell / agent, parity)
settled → unknown default 2 red (restore)
default + the settled-requires-outcome clause 2 red (codec admits an outcome-less settled record)
the legality matrix call all 19 illegal cells red
operation clearing in admission 2 red
settle-time keeping on update 3 red
settle time in resume history 2 red
legacy settled outcome mapping 3 red
legacy subagents live-only filter 1 red
owner resolution 1 red
thread_id alias kind 1 red
unknown → definite refinement 1 red
a later unknown keeping the definite outcome 2 red (explicit and omitted unknown)
definite → different definite admitted (substitution; the rejection has no deletable form) 1 red
omitted outcome counted as unknown 1 red
operation clock clamp 1 red
codec boundary ratchet (planted a renderer importer) 1 red, naming the planted file
  • pnpm run check:code-quality:changed: 0 new findings. oxlint on all 20 changed files is clean (with a positive control that it reports). pnpm run audit:anti-slop exits 0.
  • Rebase: this PR was rebased onto main after feat(agent-status): publish the main agent's own state beside the combined row state #22452 and feat(agent-status): combine Codex child work through the shared main-agent status fold #22475 merged. A one-line test fix (the fold-parity call in agent-status-child-work-view.test.ts passes its input through a const, as production code does) keeps it compiling once fix(agent-status): a cancel never hides live work #22476 removes interrupted from the fold input.
  • Parse → merge → check (the restructure described under "Admission rules" and "Codec rules"):
    • New agent-status-child-work-admission-parse.test.ts, 117 tests. For each of the 7 text fields, labels included: a tab, CRLF, U+2028, U+2029, NEL, a no-break space, an escape, DEL, whitespace only, a space at character cap−1 / cap / cap+1, and a surrogate pair split by the cut. Each announce is accepted, the stored value equals normalizeChildWorkText(raw), normalizing it again changes nothing, and it independently has no line breaker, no untrimmed edge, nothing past the cap and no half pair at the end. A lone surrogate already inside provider text is left alone on purpose: it renders as a replacement glyph, serializes safely, and only the cut can create one, which the normalizer already prevents. An erasure table: for each descriptive fact, a record holding a good value survives a request carrying a malformed one (15 rows). A first sighting with only malformed facts is admitted without them, and a settle with a malformed token count lands.
    • Deletion ablations, each edit matched exactly once, run at the final head and restored:
Deleted or changed Result
the owner parse 4 red (the three owner erasure rows, the all-malformed first sighting)
the final trim in normalizeChildWorkText 9 red (each of the 7 text fields with a space at the cap, a last message of line breakers, the existing cut-on-a-space case)
isChildWorkText put back as isBoundedString 2 red (the codec rows for U+2028 and NEL; admission's own output is unchanged, so its fuzz rows stay green)
the token parse 3 red (NaN and unsafe-integer erasure rows, the all-malformed first sighting)
owner reset on a new invocation 1 red (resume without a spawner)
a dummy optional request field added tc:node fails in both the parse and the merge
control: the codec's silent drop restored, refactor kept every admission, store and relay suite stays green; only the codec's own 20 reject rows go red
providerTiming merged from the stored record instead of the current run (the previous rule) 1 red (a resumed live child still carries the first run's completedAt)
providerTiming retention within the run 15 red (every erasure row: the stored startedAt is lost)
  • Gates at the final head: tc:node, tc:web, tc:cli exit 0. Child-work, store, relay-context, status-bridge, status-feed, lead-fold and parity suites: 28 files, 468 passed, 1 pre-existing expected fail. oxlint on every changed file exits 0, and check:code-quality:changed passes.

  • Platforms: macOS only (pure shared code; no platform-dependent paths).

  • What feat(native-chat): Claude sessions write their subagents into the host status store #22536 must delete when it restacks: in src/shared/agent-status-child-work-evidence-admission.ts, the childWorkLabel function, its constants CHILD_WORK_LABEL_MAX_LENGTH and LABEL_SCAN_MAX_LENGTH, and the normalizeOptionalField import; pass child.name, child.description and child.agentType through unchanged. Admission now normalizes labels at the codec caps. The one visible difference: a description is then capped at the codec's 8,000 characters instead of 512.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

Review

Not verified:

Architecture review

An architecture pass on this PR found that admission replaced the whole record on every update. Only the host fields (id, first observation, invocation, history, settle time) survived; everything else came from the new request alone. That left producers with an unstated duty: re-send everything they knew on every announce, or lose it. Two commits change this.

A1: a sparse observation never erases what the record already knows (mergeObservationFacts in agent-status-child-work-admission-core.ts, used by update and resume).

  • name, description, agentType, model and residency: a request that carries one replaces it; a request that omits one (or carries a malformed one) keeps the stored value. They are never cleared.
  • parentChildWorkId follows the same rule within one invocation, and a new invocation starts from what its own request says. Both producers name the owner of each run from that run's own spawn: Claude from the agent whose traffic made the spawn call, Codex from the spawner thread. Keeping the old owner across a restart would leave a child the main agent restarted nested under the child that first spawned it.
  • totalTokens never shrinks: a late or duplicate frame with a smaller count keeps the larger one. An invalid count keeps the stored count, so it can no longer block a settle.
  • lastMessage is kept until a request carries a new one, within one invocation. A resume starts a new invocation without the old ending's message.
  • providerTiming follows the same rule. It is the provider's start and end of one run, so a resumed child that is live again does not carry the previous run's completion time.
  • operation keeps replace/clear semantics: omitting it means the child stopped doing it. The shared evidence layer both producers write through carries the current operation on every live frame, so omission happens only when the child stopped. No producer writes providerTiming and no view carries it.
  • Why: real ending frames are sparse. The Claude roster omission knows only that the child is gone, and the outcome frame that refines an unknown ending often names only the outcome. Before this change, both erased the child's name, model, tokens and last message at the moment it ended. The rule now lives in the one function every update and resume goes through, not in each producer.
  • Owner, residency and live→settled lastMessage go beyond the minimum the review named (labels, tokens, a settled message). They fail the same way: a settle that omits them moved the child to the main agent and dropped what it last said.
  • AgentChildWorkObservationFields now states that an observation may be sparse, and what that means. buildAgentChildWork takes an optional third argument, the stored record.

Decision: a refinement stamped behind the stored settle time is still refused. For example: settled at 20, then an outcome at 15. The store already refuses any child write whose observedAt is behind the stored one, for every child and every field. Accepting this write would need either an evidence clock that runs backwards or a clamp that records a time the evidence was not seen. The outcome itself would be safe to accept in any order, but the clock would not. This case needs a producer whose clock goes backwards. The producers in this stack stamp host time when the frame is journaled, and the lane's evidence layer already raises its time to the record's clock. A test keeps the refusal and the unchanged record.

A2: the provider-id preference is keyed by alias kind. PROVIDER_ID_ALIAS_ORDER was a second, hand-kept list of the alias kinds. A kind added to AGENT_CHILD_WORK_ALIAS_KINDS but not to that list gave its children no providerId, and the legacy shapes then dropped the row without any error. The order is now derived from a Record<AgentChildWorkAliasKind, number>, so an unranked kind fails to compile.

A3 (recorded, no code change): being settled is still one-way within an invocation. Settled history can gain an outcome, but a settled child cannot become live again without resume (a new generation). For Claude the generation moves only on a new spawn call. So if the task roster ever briefly left out a task that was still running, the record would settle unknown, and the child's next progress frame, on the same run, would be refused until the session ends. Today's background-task tracker has the same behaviour: it treats the roster as authoritative and drops progress for a task it has finished. No capture shows the roster doing this, so it is unverified. If a capture ever shows it, the fix is one more admission rule: live evidence that names the current run reopens a settled record. It would not be a producer-side guard. The CLI hook lane has the same exposure through a lost SubagentStop.

A4 (recorded deviation, kept): child records live in host memory only. A restart loses settled child history; live children announce themselves again when the session resumes. The session journal still holds each child's durable rows, so after a restart "how did that child end" survives in the transcript but not in the sidebar or strip. This matches the status store's design (status lives in the host store, and the journal is the transcript of record) and today's tracker. The contract leaves room for a fix: a resume-time producer that rebuilds settled rows from the journal would write through the restore provenance and the existing snapshot codec, with no contract change. No PR in this stack owns that yet. Settled-row retention (#22614) lands after the producers, so until then settled records are removed only when their session ends, within the store's existing caps.

Producers above this PR

  • feat(native-chat): Claude sessions write their subagents into the host status store #22536 (Claude) typechecks and its suites pass with these commits applied.
  • feat(native-chat): Codex sessions write their subagents into the host status store #22553 (Codex) adds a turn_id alias kind, which A2 requires it to rank. Its branch now ranks turn_id last, after tool_use_id, because it names one run.
  • Neither branch was edited. Their shared evidence layer (agent-status-child-work-evidence-admission.ts) can now drop its copies: the ?? existing.* label fills and the existing.model copy in the live path, and in settleAgentChildWork the copied labels, owner, residency and ?? existing.lastMessage. Its ?? existing.totalTokens can go too, and swapping it for the admission max-merge also stops tokens from shrinking. The owner fill in the live path can go as well. The run-scoped lastMessage ?? prior.lastMessage matches the new rule and can go.

Tests and ablations

  • New agent-status-child-work-admission-sparse.test.ts, 8 tests: an ending that names nothing keeps the labels and tokens; a refinement keeps the last message and name; a settle keeps the last message, owner and residency; tokens never shrink; a refinement stamped behind the settle is refused; a carried label replaces the stored one and an invalid count keeps the stored count; a resume keeps the labels and tokens but not the old ending's message, provider timing or spawner; a resume naming a new spawner nests under it.
  • Against this PR's previous admission code, 6 of the 7 fail. The refusal test passes both before and after, by design.
  • Deletion ablations at the final head, each restored afterwards:
Deleted Result
label fill 4 red
token max-merge 4 red
last-message retention 2 red
invocation scope of last-message retention 1 red
owner fill 1 red
residency fill 1 red
A2 with an unranked kind added compile error in the view; before this change it compiled cleanly
  • tc:node, tc:web and tc:cli exit 0. Full oxlint exits 0. check:code-quality:changed finds 0 new findings, and audit:anti-slop exits 0. All child-work, store and relay-context suites pass: 17 files, 183 passed, plus 1 pre-existing expected fail.

Agent skill upstream boundary

  • 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

  • Wire / mixed versions: there are no wire changes in this PR. Child records and alias records are host-memory only: never persisted, and the store snapshot/transport envelope has no production consumer. The view is not published yet. When it is, it travels as a new optional field, and an old client keeps reading the legacy shapes, which are derived from the same view with today's output.
  • SSH / WSL / folder workspaces: the parent subject already carries the execution scope, and owner resolution requires the same parent subject. Nothing here assumes a local host or a git worktree.
  • Boundary with the main-agent status lane: the fold is imported, not edited. interrupted: false is passed through a non-literal object (in production code and in the fold-parity test), so these calls compile both before and after fix(agent-status): a cancel never hides live work #22476 removes the input. The fold's own policy did change on main since this branched (see "Display state").
  • Record lifetime: records are host memory inside the session's status-store entry: they die with the session (and Claude's provider ended clears them), and no producer writes them yet. The settled-row retention rule (kept until the parent's next turn, with a per-parent cap) is store policy that lands with the read switch in feat(native-chat): the chat strip and the sidebar read the host's child records #22614, so this PR adds no unbounded obligation.
  • Security and performance: all inputs are bounded (existing store limits, and new text caps of 60/160/512). The owner walk is cycle-safe and linear per record.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ Minor suggestions only — one cleanup nit inline, one blast-radius note for when producers land.

Reviewed changes

  • Record fields — AgentChildWorkInput gains parentChildWorkId, residency, operation, lastMessage, and admission-only settledAt; all optional and validated on every store write/restore.
  • Activity codec — parseAgentChildWorkActivityFields folds owner/operation/last message; a malformed new descriptive field is dropped while the record is kept.
  • Lifecycle matrix — isAgentChildWorkLifecycleLegal rejects illegal membership/state/outcome/settledAt/operation cells; a settled record written without outcome/settledAt reads as unknown at its newest evidence.
  • Admission — settledAt stamped once and kept, resume records the superseded invocation's settle time, operation is cleared when state cannot carry it, and an omitted outcome repeats a stored unknown.
  • View + display — projectAgentChildWorkViews copies a surface-safe projection (owner resolved, providerId by stable alias preference) and deriveAgentChildDisplayState routes through foldAgentLeadStatus.
  • Legacy shapes — subagent/background-task projections derive from the view, with thread_id added as an alias kind.

Traced and confirmed this run: the legacy golden is byte-identical to unmodified main for every published background task (main's projection logic is equivalent for live candidates, and all background-task candidates are membership live); the display table matches foldAgentLeadStatus; and no production path writes child/alias records, so the new legality gate and alias kind cannot regress existing data or the wire today. The full related suite (17 files, 179 tests, 1 expected-fail) passes.

ℹ️ A codec rejection fails the whole store snapshot, not just the child

parseAgentChildWorkInput now enforces the lifecycle matrix, and agentStatusStoreStateFromSnapshot returns null for the entire snapshot when any child fails to parse. Nothing writes these records in production yet, so this is latent — but once the producer PRs land, a single record the codec rejects (an older reader meeting a newer legal shape, or a producer bug) would drop the whole replicated store rather than that child.

Technical details
# Lifecycle rejection blast radius

## Affected sites
- `src/shared/agent-status-child-work-codec.ts:219` — the new `isAgentChildWorkLifecycleLegal` call turns an illegal cell into `null`.
- `src/shared/agent-status-store-state.ts:217` — `agentStatusStoreStateFromSnapshot` returns `null` if any `parseAgentChildWorkRecord` fails.
- `src/shared/agent-status-store.ts:153` — `applySnapshot` then rejects the snapshot wholesale.

## Required outcome
- Confirm all-or-nothing snapshot rejection is the intended contract once producers write the new fields, or make the new-field rejections record-local the way malformed descriptive fields already are.

ℹ️ Nitpicks

  • residency is stored on the record and dropped from the view, but nothing reads it anywhere yet — the PR text says "settlement reads it", so worth confirming that reader is planned for a follow-up rather than assumed present.
  • AGENT_CHILD_WORK_LAST_MESSAGE_MAX_LENGTH (512) is applied through normalizeOptionalField at admission, but the activity codec re-validates with isBoundedString, which rejects tab (0x09) and other control characters — a folded preview that still contains an interior tab is dropped rather than kept.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

}

function isAliasKind(value: unknown): value is AgentChildWorkAliasKind {
return typeof value === 'string' && ALIAS_KIND_SET.has(value)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR extracted isRecord, hasOnlyKeys and isBoundedString into agent-status-child-work-value-guards.ts (now used by the codec and the new activity codec), but this file still keeps private copies at lines 43–75. Since this module is already touched here, importing from agent-status-child-work-value-guards (passing MAX_ALIAS_PART_LENGTH for the bound) would leave one owner for these predicates instead of two that can drift.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 2471aa94-53f8-4d17-afa0-a3e126d44646

📥 Commits

Reviewing files that changed from the base of the PR and between 41d814b and 3936581.

📒 Files selected for processing (4)
  • src/shared/agent-status-child-work-admission-activity.test.ts
  • src/shared/agent-status-child-work-admission-core.ts
  • src/shared/agent-status-child-work-admission.ts
  • src/shared/agent-status-child-work-view.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/shared/agent-status-child-work-admission.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The changes add child-work activity, residency, ownership, and settlement fields with validation rules. Admission normalizes activity text, controls when operations are retained, and records settlement times across updates and resumes. New view functions resolve provider IDs and ownership, derive display state and liveness, and produce legacy projections. Tests cover validation, admission, view behavior, and legacy output shapes.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 39365

No actionable issue remains from this review. The new grouping calls are compatible with the supported runtime, so the change is mergeable after normal checks.

Architecture Summary

Architecture risk: 🟡 Medium · up to 39365

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 21 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in src/relay/agent-status-store-relay-context.test.ts: Added four shared child-work modules to the files checked for forbidden Electron, main, and renderer imports.
  • observed — Modified behavior in src/shared/agent-status-child-work-activity-codec.ts: Adds imports, residency and operation-basis membership sets, activity field and clock types, and membership guards for those enum values.
  • observed — Modified behavior in src/shared/agent-status-child-work-activity-codec.ts: Adds operation parsing that rejects records with extra or missing required keys, invalid or over-limit tool names or input, unrecognized bases, invalid timestamps, or timestamps outside the child clock interval; accepted operations retain the validated fields.
  • observed — Modified behavior in src/shared/agent-status-child-work-activity-codec.ts: Adds the exported activity-field parser. It omits invalid descriptive values and self-referential or invalid parent IDs, while including an operation only when operation parsing succeeds.

Reliability and maintainability

  • inferred — Risk-relevant change factors for src: blast_radius_1; direct_dependents_1
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 48 functions across 21 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change: extending child-work records with activity, outcome, and settlement-time data.
Description check ✅ Passed The description is detailed, structured, and covers the change, rationale, testing, limitations, compatibility, and checklist. The required Linked Issue is marked as None instead of providing an issue…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c581c256-71a7-4cd4-8c19-3d9cffa1e48a

📥 Commits

Reviewing files that changed from the base of the PR and between a77f87e and b17cf6f.

📒 Files selected for processing (19)
  • src/relay/agent-status-store-relay-context.test.ts
  • src/shared/agent-status-child-work-activity-codec.ts
  • src/shared/agent-status-child-work-admission-activity.test.ts
  • src/shared/agent-status-child-work-admission-core.ts
  • src/shared/agent-status-child-work-admission-operations.ts
  • src/shared/agent-status-child-work-admission.ts
  • src/shared/agent-status-child-work-alias.ts
  • src/shared/agent-status-child-work-codec.ts
  • src/shared/agent-status-child-work-legacy-golden.test.ts
  • src/shared/agent-status-child-work-legality.test.ts
  • src/shared/agent-status-child-work-legality.ts
  • src/shared/agent-status-child-work-projection.test.ts
  • src/shared/agent-status-child-work-projection.ts
  • src/shared/agent-status-child-work-resume.ts
  • src/shared/agent-status-child-work-value-guards.ts
  • src/shared/agent-status-child-work-view.test.ts
  • src/shared/agent-status-child-work-view.ts
  • src/shared/agent-status-child-work.test.ts
  • src/shared/agent-status-child-work.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.

if (record.membership === 'live') {
return (
record.state !== 'done' &&
(record.state !== 'monitoring' || storesMonitoring(record.kind)) &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
# Find producers that set child-work state, and check whether any set 'monitoring' for agent/workflow kinds.
rg -nP --type=ts -C4 "state:\s*'monitoring'" -g '!**/*.test.ts' src
rg -nP --type=ts -C4 '\b(announce|adopt|resume)\s*\(' -g '!**/*.test.ts' src | head -200
# Check how the snapshot/mutation codecs react to a child that fails parseAgentChildWorkRecord.
rg -nP --type=ts -C6 'parseAgentChildWorkRecord|parseAgentChildWorkInput' src/shared/agent-status-store-codec.ts src/shared/agent-status-store-mutation.ts src/shared/agent-status-store-persistence.ts

Repository: stablyai/orca

Length of output: 22652


🏁 Script executed:

#!/bin/bash
set -eu
cat -n src/shared/agent-status-child-work-legality.ts
sed -n '1,300p' src/shared/agent-status-child-work-codec.ts
rg -n -C3 --type=ts 'buildAgentChildWork|AgentChildWorkInput|AgentChildWorkAnnounceRequest|AgentChildWorkAdoptRequest|AgentChildWorkResumeRequest|\.announce\(|\.adopt\(|\.resume\(' src/shared src/main src/relay -g '!**/*.test.ts' | head -500
sed -n '1,330p' src/shared/agent-status-store-codec.ts

Repository: stablyai/orca

Length of output: 42517


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- exact admission bindings and callers ---'
rg -n -C4 --type=ts 'announceAgentChildWork|adoptAgentChildWork|resumeAgentChildWork|createAgentChildWorkAdmission|AgentChildWorkAdmission|agentChildWorkAdmission|buildAgentChildWork|commitAgentChildWork' src -g '!**/*.test.ts'
printf '%s\n' '--- child-work state and kind construction ---'
rg -n -C5 --type=ts "kind: '(agent|workflow|unknown|command|monitor)'|state: '(monitoring|working|waiting|blocked|done)'" src -g '!**/*.test.ts' | head -600
printf '%s\n' '--- admission core ---'
sed -n '1,280p' src/shared/agent-status-child-work-admission-core.ts

Repository: stablyai/orca

Length of output: 42143


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- relevant PR diff ---'
git diff --unified=8 563dd5487fb6ca2f9a79b12075df2ed054934d7d b17cf6fa05022240ff05bd9ef2c35fa68cebc72b -- src/shared/agent-status-child-work.test.ts src/shared/agent-status-child-work-codec.ts src/shared/agent-status-child-work-legality.ts
printf '%s\n' '--- observation-field references ---'
rg -n -C3 --type=ts 'AgentChildWorkObservationFields|AgentChildWorkAnnounceRequest|announceAgentChildWork\(|adoptAgentChildWork\(' src
printf '%s\n' '--- all monitoring literals ---'
rg -n -C3 --type=ts "state: 'monitoring'" src

Repository: stablyai/orca

Length of output: 41892


Preserve legacy monitoring child records.

parseAgentChildWorkInput now rejects live monitoring records whose kind is agent, workflow, or unknown. The same parser is used by admission, so such an update can return invalid. Snapshot and mutation parsing also reject the entire payload when one child fails.

The previous codec accepted these records, and the existing round-trip test stored a workflow child in monitoring. Changing that fixture to monitor does not migrate older snapshots or store mutations.

If older data can contain these records, normalize them at the compatibility boundary, or keep accepting them in snapshot and mutation codecs and reject them only before admission.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found in the new commit.

Reviewed changes

  • Settled-outcome refinement — settledEvidence (agent-status-child-work-admission-core.ts) now lets a stored unknown ending gain a definite one for the same invocation while keeping the original settledAt; a definite ending never changes to a different one, and a later explicit/omitted unknown is ignored with accepted: true and no write.
  • Operation clock clamp — admission clamps operation.observedAt into [firstObservedAt, request.observedAt], so a stamp in provider time is kept (clamped) instead of being dropped by the codec.
  • Codec-boundary ratchet — agent-status-child-work-codec-boundary.test.ts asserts the record codec's only importers are the eight host store/admission modules, and the codec JSDoc now spells out that it is a strict host-internal gate, never a cross-version decoder.

Traced this run: the settledEvidence matrix matches the PR's stated rules for every arm (agentChildWorkSettledAt returns the stored settledAt on each update, so the refine keeps the first settle time); the clamp always lands inside the codec's accepted window and a non-numeric stamp degrades to a dropped operation with the record kept. Ran the five child-work suites at this head — 108 passed. The ratchet's allowlist matches the current importers exactly. One prior Pullfrog cleanup concern (duplicated value-guard predicates in agent-status-child-work-alias.ts) is untouched by this commit and stays open; nothing here is blocking.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No new issues in the two new commits. The sparse-observation retention is correct, and its tests can fail when retention is removed. One prior cleanup thread on agent-status-child-work-alias.ts is untouched by these commits and stays open, which is why this review is not an approval.

Reviewed changes

  • Sparse-observation retention — buildAgentChildWork now takes the stored record as prior, and retainedFacts merges it so a sparse observation fills or replaces labels, tokens, owner, residency and last message instead of clearing them.
  • Owner, residency and last message survive a settle — lastMessage is keyed to the invocation (survives live→settled and sparse live frames, not a resume); parentChildWorkId and residency last for the child.
  • Tokens never shrink — a valid request count max-merges with the stored one; an out-of-range count passes through unmerged so the codec still refuses it.
  • Provider-id ranking — the alias-kind preference is a Record<AgentChildWorkAliasKind, number> sorted over AGENT_CHILD_WORK_ALIAS_KINDS, so a new alias kind cannot compile without a rank.
  • Tests — new agent-status-child-work-admission-sparse.test.ts (7 cases) covering label retention, last-message retention through an outcome refinement, owner/residency/last-message retention through a settle that names none of them, the token floor, a refinement stamped behind its settle, and resume carrying labels/tokens but not the old message.

Traced this run: retainedFacts only ever receives a codec-validated prior, so every retained value is legal for its record; the sameInvocation gate is always true on updateExistingAgentChildWork and false on resume, which matches the contract; the token branch never emits a non-safe-integer that the codec would reject without the request itself having been invalid. Ran the five child-work suites at this head — 114 passed with env -u ORCA_STRUCTURED_SESSION.

ℹ️ Nitpicks

  • The PR description's "Admission rules" section still describes the old whole-record replacement and does not mention that an omitted label, token count, owner, residency, or last message now keeps its stored value. Since this PR is the shared contract the rest of the stack builds on, a line in that section would keep the description in step with the code.
  • Owner and residency retention across a resume is implemented (retainedFacts sees the prior record) but the resume test asserts only labels and tokens; a direct assertion would pin that half too. Optional — no consumer reads them yet.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025
brennanb2025 force-pushed the brennanb2025/c2-child-record-contract branch from 3bb417f to 6de7f20 Compare September 24, 2026 20:51
…ow it ended, and when

A child-work record gains the facts every surface needs from one host-owned
record: the child that owns it (parentChildWorkId), whether the provider said
it may outlive its launch turn (residency, host-only), what it is doing now
(operation, with an open/reported basis), the newest thing it said
(lastMessage), and when its current invocation settled (settledAt, stamped by
admission, never by a producer).

The codec enforces one membership x state legality matrix: live work is never
done and carries no outcome or settle time; only a shell or monitor stores
monitoring; settled work is done with an outcome and a settle time inside its
own evidence window; an operation exists only while live and working, waiting
or blocked. A settled record written without an outcome reads as unknown, never
success. Malformed descriptive fields drop and keep the record.

A new read-only view (AgentChildWorkView) is the one projection surfaces read;
the legacy subagent and background-task shapes are derived from it with
today's output unchanged for today's inputs. deriveAgentChildDisplayState
folds a child's own state and the liveness of the work it owns through the
same fold a parent row uses, so a child whose own work is idle or done reads
monitoring while a shell it launched runs.

Codex children get a thread_id alias kind.
… operation clock clamped

A settled child whose ending was first recorded as unknown (a roster omission
can land a tick before the frame naming the outcome) now accepts the definite
outcome for the same invocation and keeps its original settle time. A definite
ending still never changes, and a later unknown ending is ignored rather than
downgrading it.

Admission clamps operation.observedAt into the child's evidence window, so an
operation stamped in provider time is kept instead of silently dropped.

The record codec is pinned as host-internal: it rejects a whole record over one
unknown key, so a ratchet test fails if anything outside the host store and
admission path imports it.
…ecord already knows

Admission merged a later observation by replacing the whole record, so an
ending that knew only that the child was gone dropped its name, model and
token count, and an outcome refinement dropped the recorded last message.
Labels now fill or replace but never clear, tokens never shrink, and a
settled ending keeps its last message unless new evidence carries one.
The provider-id preference is keyed by alias kind so a new kind cannot
compile without a rank.
…e a sparse observation

A settle that knows only that the child is gone dropped who owned it and
whether it ran in the background, and the last thing the child said while
live. They now survive like the labels do: the last message for its
invocation, owner and residency for the child.
@brennanb2025
brennanb2025 force-pushed the brennanb2025/c2-child-record-contract branch from 6de7f20 to 41d814b Compare September 24, 2026 23:22
…ll lands its evidence

A settled child's later `unknown` (or omitted) ending was acknowledged without a write, so a
late last message, token count, alias or reclassification it carried was dropped while the
caller was told it was accepted. The outcome now merges like every other sparse fact: an
`unknown` claims nothing and keeps the stored definite outcome, and only a different definite
ending conflicts.
Appending by spread copied each bucket on every insert, quadratic in a bucket's size on the
projection and per-row liveness paths.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — one minor documentation note below. The two new commits are correct: the settled-outcome change admits the frame's evidence without ever downgrading a definite ending, and the grouping rewrite is behavior-preserving. The prior cleanup thread on agent-status-child-work-alias.ts is untouched by these commits and stays open, which is why this is not an approval.

Reviewed changes

  • An unknown ending keeps a definite outcome and still lands its evidence — settledEvidence's ignore branch is gone; conflictsWithSettled now rejects a settled request only when it changes membership, changes state, or names a different definite outcome. A stored definite ending plus an unknown/omitted-outcome frame is admitted, retainedFacts keeps the stored outcome, and the frame's lastMessage/aliases are written instead of dropped.
  • Grouping in one pass — projectAgentChildWorkViews and agentChildWorkOwnedLiveness replace the hand-rolled Map accumulation with Map.groupBy (already used in automation-run-retention.ts); grouping and iteration order are unchanged.
  • Test update — the settled-outcome case is renamed and now asserts the definite outcome and first settledAt survive while the later message and aliases land, so a revert to the old ignore path fails it.

Traced this run: conflictsWithSettled admits exactly the cells the codec still accepts; retainedFacts retains outcome only under sameInvocation, so a resume cannot carry the old ending forward; the store's input.observedAt < previous.observedAt guard (agent-status-store-mutation.ts:174) is the only place a behind-stamped refinement is still refused, matching the documented decision.

ℹ️ Nitpicks

  • The PR description's "Admission rules" table still says a stored definite outcome plus an explicit/omitted unknown is "ignored" and returns with an unchanged revision. The new commit admits that write and lands its evidence (the outcome itself is still never downgraded). A line in that section would keep the contract description in step with the code.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…upBy

The relay runs this core on Node 18, which lacks Map.groupBy; a plain loop into a Map is
equally linear and portable.
Admission folded raw provider text with the status-row normalizer, which can leave a tab or
other control character and can end a truncation on a space. The record codec drops such a
field, so a long command cut at a space, a tab in a command, or an escape in a message
silently erased the child's current operation or last message. Admission now folds control
characters to spaces and trims the cut, with the codec's own control-character predicate.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No new issues in the two new commits. Both are correct and behavior-preserving where they should be. The prior cleanup thread on agent-status-child-work-alias.ts is untouched by these commits and stays open, which is why this is not an approval.

Reviewed changes

  • Raw preview text always lands — admission routes operation.toolName, operation.input and the retained lastMessage through a new childWorkPreview that normalizes, turns every control character into a space, then trims. A preview cut landing on a space (at the length cap) or carrying an interior tab/escape no longer gets dropped by the codec's isBoundedString.
  • One control-character predicate — isControlCharCode is extracted to agent-status-child-work-value-guards.ts and reused by isBoundedString (identical behavior) and childWorkPreview.
  • Grouping without Map.groupBy — projectAgentChildWorkViews and agentChildWorkOwnedLiveness drop Map.groupBy for a local groupedBy helper, because the relay bundle's esbuild target is node18 and the relay runs as node relay.js on remote hosts (Node 18 lacks the runtime API).

Traced this run: childWorkPreview cannot push a field past its cap (control chars map 1:1 to spaces, then trim only shortens), so the codec still accepts; its undefined on an all-control field lets retainedFacts fall back to the stored lastMessage instead of erasing it. groupedBy groups by SameValueZero in one pass with the same iteration order Map.groupBy gave. The added activity case is falsifiable: before the fix the codec drops both fields. Ran the seven child-work/store/relay suites at 6ea0e402 — 118 passed.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Admission merged provider values before anything knew they were valid, and
the codec then either rejected the whole record or silently dropped the
field depending on how old the field was. A malformed owner erased the
stored one, a label cut on a space rejected the announce, and a bad token
count blocked a settle.

Admission now parses every descriptive fact into a value the codec accepts
or "not said", merges it over the stored record with one rule per fact (a
typed map, so a new request field without a rule fails to compile), and the
codec checks the result. Text goes through one normalizer and the codec
accepts exactly its image; any value outside it is a writer bug and
rejects. The owner is now a fact of the invocation, like the last message.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No new issues in the three new commits. The restructure makes admission the single place that turns untrusted provider facts into codec-valid values, and the codec now rejects any malformed descriptive field instead of silently dropping it — which is exactly the contract the new tests pin. The prior cleanup thread on agent-status-child-work-alias.ts is untouched by these commits and stays open, which is why this is not an approval.

Reviewed changes

  • Parse, merge, then check — buildAgentChildWork now runs parseObservationFacts (each provider fact becomes a codec-valid value or "not said"), merges it over the stored record, and lets parseAgentChildWorkInput enforce the invariants; the old retainedFacts whole-record merge is gone.
  • Codec rejects instead of drops — parseAgentChildWorkActivityFields returns null when a present activity field is outside admission's image (owner, residency, last message, operation), and labels plus operation text are validated with the new isChildWorkText ("normalizing changes nothing") instead of isBoundedString.
  • One text normalizer — normalizeChildWorkText folds control characters and line-drawing code points to spaces and trims the cut, and is shared by admission and the codec's isChildWorkText guard; isControlCharCode becomes private.
  • Tests — new agent-status-child-work-admission-parse.test.ts (117 tests: hostile text per field plus a per-fact erasure table), added codec reject rows, and sparse/resume assertions that a malformed count keeps the stored value and a restart drops the prior spawner.

Traced this run: admission's per-field caps match the codec's (name/agentType/model 512, description 8,000, tool name 60, input 160, last message 512); the merge keeps outcome/parentChildWorkId/lastMessage scoped to the invocation and labels/residency/timing/tokens lifetime-scoped; and the operation clock clamp always lands inside the codec's accepted window. Ran the changed suites plus the codec-boundary ratchet, view, relay-context, legacy-golden and store suites at this head — 259 passed, 1 pre-existing expected fail. The codec importer ratchet still matches the 8-module allowlist.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

providerTiming records the provider's start and end of one run. Keeping it
across a resume left a live restarted child claiming the previous run's
completion time. It now follows the owner and last message: kept within an
invocation, reset by a new one.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No new issues in the new commit. Provider timing is now correctly scoped to its invocation. The prior cleanup thread on agent-status-child-work-alias.ts is untouched by this commit and stays open, which is why this is not an approval.

Reviewed changes

  • Provider timing lasts only for its own run — mergeObservationFacts reads providerTiming from run (the stored record only when the observation continues the same invocation) instead of prior, so a resumed live child no longer carries the previous run's completedAt; within a run, an omitted timing still keeps the stored start.
  • Test — the sparse resume case now seeds providerTiming on the settled first run, asserts it is present before the resume, and asserts the resumed live child drops it.

Traced this run: run is prior only when the fences are equal, so a resume falls back to the request's own timing; parseAgentChildWorkProviderTiming accepts startedAt/completedAt independently, so retaining a start-only timing on a live→settled update inside the same run remains legal. Ran the five child-work suites at this head — 232 passed. The codec importer ratchet is unaffected.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: ready at 467e1f53d8

Five review loops, an architecture challenge, a precedent check, readiness checklists at the start and end, and live Electron QA. The last loop came back clean with no changes. CI: 30 checks, none failing or pending. Mergeable.

What changed during review

Commit Change Why
af93ae5898 An unknown or missing ending after a definite one keeps the definite outcome and still records the rest of the update. Before, that update was accepted and then thrown away, so a late last message, token count or new alias was silently lost.
3936581d9d, e17dd35eab Aliases and owned work are grouped in one pass with a plain Map loop. Spread-append was quadratic (about 39 ms at the 256-child cap, now 0.8 ms). Map.groupBy is not available on the relay's Node 18.
6ea0e40210 → 9efde6b0f3 Admission now runs parse → merge → invariant check. There is one text normalizer (normalizeChildWorkText), and the codec's text check is defined as "normalizing it changes nothing". A merge type that lists every field means a new field with no merge rule fails to compile. Three loops kept finding the same bug class: admission merged a request value before knowing it was valid, then the codec quietly dropped it and erased the stored value. Examples: a long command cut on a space, a tab in a command, a malformed owner, or a U+2028 in a label. Rewriting the structure removes that bug class instead of adding more guards.
9efde6b0f3 The owner and last message last only for their own run. Both producers send the spawner for each run, so a restarted child now nests under whoever restarted it.
467e1f53d8 Provider timing lasts only for its own run. A resumed live child kept the previous run's completedAt.

Each behaviour fix has a test that fails when the fix is deleted (red for the right reason). The legacy projection golden test is unchanged against main.

Electron QA (hidden background launch, fresh profile, head 7bf43e20d3)

The intended result is no visible change: nothing reads the new fields yet. Checked with a real claude CLI in both the terminal lane and the structured chat lane:

  • One subagent plus one background shell: the child row was working while the subagent ran, then "Monitoring background tasks", then done with no leftover rows.
  • Structured chat: the strip read "1 agent · 1 shell" with the correct Done and Working rows, then cleared; a subagent seen mid-run showed its row working, then settled.
  • The served code was confirmed to be this branch. No console errors mention child-work, agent-status or the codec.

The only change after the QA head (467e1f53d8) is inside admission, which nothing calls yet.

Live subagent + shell Subagent done, shell live All settled
Structured strip, live Structured, settled Structured, live subagent Structured, subagent settled

Follow-ups for the stacked PRs (not this PR)

Not covered

  • No Electron run of the producer PRs on top of this head, since they have not been restacked yet.
  • No mobile or Windows run: this PR changes no mobile or platform surface.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant