fix(native-chat): date a session by its own lifecycle, not its subagents' work - #22520
Conversation
…agents' The journal reducer's lastActivityAt is the structured status summary's updatedAt, which the status row uses as its completion stamp and acknowledgement clock. It took the max over every journal row, and a session's subagents write into the same journal after its own agent has settled, so an idle parent was re-dated and marked unread on child work. A row now dates the session only when the session's own agent produced it: not a row whose producer linkage names a subagent, and not a subagent roster row (a subagent-group block), which the session writes but revises on every child transition. The roster rule is derived from the row body; no new persisted field. Replay folds through the same rule, so existing journals are re-dated to their own last row on reopen. Claude: a backgrounded subagent emits no child frames, so its re-dating came entirely from roster revisions (task_updated, task_notification) and from the stale-roster revision written when a journal reopens. Codex: the roster is revised on every child token-usage report; child-thread rows carry no producer linkage yet, and read as the session's own until they do.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (19)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds journal-derived status start times to structured session summaries and adds recovery timestamps to rendered journal items. The status feed uses the status clock when deciding whether to republish. Host ingest and the renderer bridge use that clock to date status rows, with fallback behavior for summaries without it. Activity events now distinguish state start time from observation time, and Activity threads retain pane status entries. Tests cover status transitions, recovered turns, subagent activity, and row dating. Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to Child settlement can still make a parent session appear recently active through the journal clock. Preserve child ownership during lifecycle revisions before merging, unless that remaining recency behavior is explicitly accepted. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description is detailed and covers the change, motivation, testing, visual proof, review notes, and compatibility considerations. However, it does not provide the required linked issue and omits the template's Checklist section.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 855730ba-0967-4e3d-be23-0ede82bf9fcc
📒 Files selected for processing (4)
src/main/native-chat/agent-session-journal/journal-reducer.tssrc/main/native-chat/agent-session-journal/journal-session-clock.test.tssrc/main/native-chat/agent-session-journal/journal-session-clock.tssrc/main/native-chat/agent-session-wire/structured-agent-session-subagent-recency.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one informational note inline.
Reviewed changes
- Reducer clock gated on producer and roster —
applyJournalRownow advanceslastActivityAtonly when the newjournalRowDatesSessionsays the row is the session's own agent and not a subagent-roster revision, so an idle parent is no longer re-dated by its children (journal-reducer.ts,journal-session-clock.ts). - Row-level rule coverage —
journal-session-clock.test.tscovers item, tombstone, lifecycle-batch and own-system rows, plus the reopen-and-settle path. - End-to-end coverage —
structured-agent-session-subagent-recency.test.tsdrives the real Claude translator and Codex roster through the deferred sink, durable journal and status feed, assertinglastActivityAt, publishedupdatedAtand the publication count hold while a child works and resume on the next own turn.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
… date the session Reopening a journal settles rows the previous host left live (a working subagent roster, a live background task) to unverifiable. Those revisions were appended at the reopen moment, and a background-task row is the session's own non-roster row, so a crash-restarted session with a live shell was re-dated to the restart although no agent acted. The reconciler now writes each verdict revision with the row's own observed time. That is one rule at the one writer, covering both settle shapes; the render item's observedAt was already pinned to the row's first write, so nothing the transcript shows changes. The live-transition roster exclusion stays: live roster revisions are written by the providers, not here. The `recovered` row flag is not used as the discriminator: the live unexpected-exit settlement also writes recovered rows, and a clock rule keyed on it would stop dating a provider crash the host just observed.
There was a problem hiding this comment.
ℹ️ The reopen fix in
bf00b3dis clean — no new issues in the delta.
Reviewed changes
- Reopen verdicts inherit the row's own time —
staleSubagentRosterRevisionsnow carries each stale row's pinnedobservedAt, andsettleStaleSubagentRostersthreads it intoappendItem(journal-store-open.ts,journal-store-restore.ts,journal-subagent-liveness.ts). A crash-reopened journal'sunverifiablesettlement therefore writestsequal to the row's first write instead of the restart, so the root background-task row can no longer re-date an idle session (journal-row-builders.ts:43,journal-reducer.ts:73). - Reopen coverage —
journal-session-clock.test.tsadds a live-background-task reopen case asserting the settling revision landed (revision 2) whilelastActivityAtholds; the roster reopen case is retained.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
… a row wrote The clock read producer linkage off the raw row. A lifecycle batch names no row-level producer, a tombstone names none, and a revision may name none while the reducer still attributes the item to a subagent, so each of those dated an idle parent. The clock now asks the reducer: after a row applies, whether any item it wrote is the session's own work; before a removal, whether the item it removes was.
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the clock refactor reads the reducer's folded attribution correctly, and the new child-tombstone test is a real guard. Submitting as a comment rather than an approval because a previously flagged Codex follow-up (lifecycle-batch producer linkage) remains open and is not addressed by this commit.
Reviewed changes
- Clock reads the reducer's attribution, not the raw row —
applyJournalRownow decides removals before applying (journalRowRemovesSessionWork) and writes after (journalRowWroteSessionWork), so a tombstone or batch is judged by the item the reducer folded. This stops a child row's removal from re-dating an idle parent: the tombstone names no producer, but the item it removes does (journal-reducer.ts,journal-session-clock.ts). - Regression coverage for the removal route —
journal-session-clock.test.tsgains the child-tombstone case assertinglastActivityAtholds at the session's own row time; it fails against the prior commit's raw-body check.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: b24022dd-54f2-423e-a79c-e10e01ee455d
📒 Files selected for processing (6)
src/main/native-chat/agent-session-journal/journal-reducer.tssrc/main/native-chat/agent-session-journal/journal-session-clock.test.tssrc/main/native-chat/agent-session-journal/journal-session-clock.tssrc/main/native-chat/agent-session-journal/journal-store-open.tssrc/main/native-chat/agent-session-journal/journal-store-restore.tssrc/main/native-chat/agent-session-journal/journal-subagent-liveness.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/native-chat/agent-session-journal/journal-session-clock.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.
| } | ||
| if (row.kind === 'lifecycle-batch') { | ||
| return row.mutations.some( | ||
| (mutation) => mutation.kind === 'item' && isSessionWork(currentItem(lookup, mutation.itemId)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
git diff 563dd5487fb6ca2f9a79b12075df2ed054934d7d..85cdfca4130c68862ad15541d319591bc1ef7620 -- src/main/native-chat/agent-session-journal/journal-session-clock.ts src/main/native-chat/agent-session-journal/journal-reducer.ts src/main/native-chat/agent-session-journal/journal-row-builders.ts
rg -n -C 4 'journalRenderItem|lifecycle-batch|agentId|settle' src/main/native-chat/agent-session-journal src/main/codex
sed -n '1,130p' src/main/native-chat/agent-session-journal/journal-session-clock.tsRepository: stablyai/orca
Length of output: 45545
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- symbols ---'
rg -n -C 3 'journalLifecycleBatchRowBuilder|journalRenderItem|currentItem|journalRowWroteSessionWork|lifecycle-batch|agentId|settleCodexJournalSession|appendCodexLifecycleMutations|recency|lastActivityAt' \
src/main/native-chat/agent-session-journal/journal-row-builders.ts \
src/main/native-chat/agent-session-journal/journal-reducer.ts \
src/main/native-chat/agent-session-journal/journal-row-schema.ts \
src/main/native-chat/agent-session-journal \
src/main/codex/codex-structured-journal-settlement.ts \
src/main/codex/codex-structured-journal-sink.ts \
src/main/codex \
src/shared/agent-session-journal-producer.ts \
src/shared/agent-session-journal-types.ts \
| head -n 1200
printf '%s\n' '--- builders ---'
sed -n '1,260p' src/main/native-chat/agent-session-journal/journal-row-builders.ts
printf '%s\n' '--- reducer ---'
sed -n '1,300p' src/main/native-chat/agent-session-journal/journal-reducer.ts
printf '%s\n' '--- schema ---'
sed -n '1,260p' src/main/native-chat/agent-session-journal/journal-row-schema.ts
printf '%s\n' '--- settlement ---'
sed -n '1,220p' src/main/codex/codex-structured-journal-settlement.ts
printf '%s\n' '--- sink ---'
sed -n '1,240p' src/main/codex/codex-structured-journal-sink.tsRepository: stablyai/orca
Length of output: 43113
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- row builders 80-165 ---'
sed -n '80,165p' src/main/native-chat/agent-session-journal/journal-row-builders.ts
printf '%s\n' '--- render item ---'
sed -n '1,240p' src/main/native-chat/agent-session-journal/journal-render-item.ts
printf '%s\n' '--- reducer 60-125 ---'
sed -n '60,125p' src/main/native-chat/agent-session-journal/journal-reducer.ts
printf '%s\n' '--- identity and linkage definitions ---'
rg -n -C 5 'type AgentJournalItemIdentity|interface AgentJournalItemIdentity|producerLinkage|agentId|parentAgentId|codexJournalItem|codexStreamingJournalItem' \
src/shared/agent-session-journal-types.ts \
src/main/codex/codex-structured-item-translation.ts \
src/main/codex/codex-structured-journal-items.ts \
src/main/codex
printf '%s\n' '--- recency tests and settlement translator tests ---'
rg -l -n 'journalRowWroteSessionWork|lastActivityAt|parent.*recency|recency|settlement' \
src/main/native-chat/agent-session-journal \
src/main/codex \
| head -n 80Repository: stablyai/orca
Length of output: 42702
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Codex linkage writes and active item registration ---'
rg -n -C 6 'agentId|parentAgentId|producerKind|activeItems\.set|activeItems|appendItem\(|tryAppendItem\(|appendLifecycleBatch|tryAppendLifecycleBatch' \
src/main/codex \
src/main/native-chat/agent-session-journal \
| head -n 1600
printf '%s\n' '--- session clock tests ---'
sed -n '1,320p' src/main/native-chat/agent-session-journal/journal-session-clock.test.ts
printf '%s\n' '--- Codex settlement tests ---'
sed -n '1,420p' src/main/codex/codex-structured-journal-translation-settlement.test.tsRepository: stablyai/orca
Length of output: 42722
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- all producer-linkage references in relevant source ---'
rg -n -C 4 'AgentJournalProducerLinkage|agentJournalLinkageFields|agentId|parentAgentId|producerKind|providerParentRef' \
src/main/codex \
src/main/native-chat/agent-session-journal \
src/main/native-chat/agent-session-wire \
src/shared \
| head -n 1200
printf '%s\n' '--- sink option and journal append contracts ---'
rg -n -C 8 'tryAppendItem|appendItem|tryAppendLifecycleBatch|appendLifecycleBatch|lifecycle.*true|coalescingKey|linkage' \
src/main/native-chat/agent-session-wire \
src/main/native-chat/agent-session-journal \
| head -n 1400
printf '%s\n' '--- Codex subagent execution linkage context ---'
sed -n '1,260p' src/main/codex/codex-subagent-executions.ts
sed -n '1,180p' src/main/codex/codex-subagent-execution-projection.tsRepository: stablyai/orca
Length of output: 45543
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Codex production agentId references ---'
rg -n 'agentId|parentAgentId|producerKind|providerParentRef' src/main/codex || true
printf '%s\n' '--- linkage helper ---'
rg -l 'function agentJournalLinkageFields|agentJournalLinkageFields' src/shared src/main/native-chat
for f in $(rg -l 'function agentJournalLinkageFields|agentJournalLinkageFields' src/shared src/main/native-chat); do
echo "--- $f ---"
rg -n -C 12 'agentJournalLinkageFields|AgentJournalProducerLinkage' "$f"
done
printf '%s\n' '--- append option declarations ---'
rg -n -C 14 'type JournalItemAppendOptions|interface JournalItemAppendOptions|JournalItemAppendOptions|JournalLifecycleBatchInput' \
src/main/native-chat/agent-session-journal \
src/main/native-chat/agent-session-wire \
src/sharedRepository: stablyai/orca
Length of output: 42439
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- structured sink option declarations ---'
rg -n -C 20 'export type StructuredAgentSessionAppendOptions|type StructuredAgentSessionAppendOptions|interface StructuredAgentSessionAppendOptions|StructuredAgentSessionAppendOptions' \
src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts \
src/main/native-chat/agent-session-wire
printf '%s\n' '--- sink forwarding implementation ---'
rg -n -C 12 'structuredAgentSessionJournalAppendOptions|appendItem:|tryAppendItem:|appendLifecycleBatch:|tryAppendLifecycleBatch:' \
src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.tsRepository: stablyai/orca
Length of output: 41588
Preserve child ownership in lifecycle-batch revisions.
When a Codex settlement revises a child-linked item, the lifecycle batch omits producer linkage. journalRenderItem then replaces the item with a render item that has no agentId. The reducer and journalRowWroteSessionWork read that replacement as root-owned, so the child revision can advance the parent session’s lastActivityAt.
Carry producer ownership per mutation, or preserve the existing item’s ownership when applying settlement revisions. Add a settlement-to-reducer test. The current clock test supplies agentId manually and does not exercise the translator path.
A subagent writes into its parent's journal and keeps going after the parent settles. Every one of its rows advanced the summary's updatedAt, and the status row re-dated a done parent to it, so an idle parent read as newly finished and unread on each child step. The host now publishes statusStartedAt beside updatedAt: when the session's own agent entered its status, read off edges only it writes. Idle is when its newest turn ended; working is when the running turn was requested, or the earliest send still unanswered; attention is its own oldest pending ask, or a subagent's when that alone holds it. A turn that recovery settled after its host went away ended when that settle was written, so it reads as a completion the user has not seen; it carries no outcome, so no completion event or notification calls it a success. Render items carry recoveredAt, the recovered row's own write time, so nothing new is persisted. The sidebar bridge and the host ingest date the row and the main agent's clock from it whenever the row shows the main agent's own state, and keep their existing rules for a row child work holds open or a summary from an older host. The status feed republishes when the clock moves instead of on every idle row.
The row filter this branch put on the reducer's lastActivityAt decided which rows could date a session: a list of exclusions that each new row kind could slip past. The session's state is now dated by its own lifecycle edges, so the filter, its attribution helper and the backdated reopen verdicts go back to main. lastActivityAt, and the summary's updatedAt it feeds, is again the evidence clock over every row, including a subagent's.
…rk holds open A row held open by live child work is dated by when each reader saw the publish, and mobile decays a working row whose evidence is older than the staleness window. Suppressing row-activity republishes for every dated idle session froze that evidence, so a subagent running more than 30 minutes past its parent's turn made the row read idle on mobile. Only a session nothing holds open stays quiet on row activity now; its state clock is unchanged.
There was a problem hiding this comment.
Important
One must-address gap: a crash-recovered turn row loses its recoveredAt marker on the next live revision, so statusStartedAt becomes undefined for an unverifiable turn and the idle parent can be re-dated by its subagent's rows again — the exact bug this PR fixes, for crash-restarted sessions. Details inline.
Reviewed changes
- Dated a session by its own lifecycle edges —
6f0af76addsstatusStartedAt, derived from the newest turn record, unanswered sends, and pending asks, and the host ingest and sidebar bridge adopt it for the row'sstateStartedAt(structured-agent-session-status-started-at.ts). - Reverted the prior reducer/clock design —
4d38423removed thejournal-session-clock.tsrow filter, its attribution helper, and the backdated reopen verdicts, returninglastActivityAt/updatedAtto the evidence clock over every row. - Suppressed idle republication on row activity —
summariesEqualnow republishes only when the clock moves, so a subagent'supdatedAtmove no longer rewrites a dated idle row (structured-agent-session-status-feed.ts). - Made recovery the completion edge — a crash-settled turn reads as done as of the recovery write and carries no outcome, so no completion event or notification is sent.
- Added regression coverage — end-to-end Claude/Codex recency, status-feed clock, host-ingest and bridge clock, and row-level
statusStartedAttests (all green locally: 7 files, 62 tests).
ℹ️ Nitpicks
- The PR description still documents the reverted
journalRowDatesSession/ backdated-reopen design and should be rewritten for thestatusStartedAtapproach.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| return undefined | ||
| } | ||
| // A turn recovery settled ended when that settle was written: when the user learns it stopped. | ||
| return turnItem.recoveredAt ?? turn.completedAt |
There was a problem hiding this comment.
The idle clock depends on turnItem.recoveredAt, but a recovered turn row loses that marker on the next live revision, so an unverifiable turn's statusStartedAt becomes undefined and the feed falls back to republishing idle on updatedAt, letting a subagent's rows re-date the idle parent again. upsertItem spreads the new render item and pins only body/sequence/observedAt, and the structured revision path cannot stamp recovered, so a late context write that revises the recovered row drops recoveredAt.
Technical details
# A recovered turn row's `recoveredAt` is dropped by the next live revision
## Affected sites
- `src/shared/structured-agent-session-status-started-at.ts:35` — `turnItem.recoveredAt ?? turn.completedAt` returns `undefined` once `recoveredAt` is gone and the turn is `unverifiable` (no `completedAt`).
- `src/main/native-chat/agent-session-journal/journal-render-item.ts:24` — sets `recoveredAt` only when `row.recovered`.
- `src/main/native-chat/agent-session-journal/journal-reducer.ts:208-214` (`upsertItem`) — spreads the NEW render item, pinning only `body`/`sequence`/`observedAt`; `recovered`/`recoveredAt` come from the new row and are therefore cleared.
- `src/main/native-chat/agent-session-wire/structured-agent-session-journal-append-options.ts:16-20` — the structured append path forwards `observedAt` and linkage only; `StructuredAgentSessionAppendOptions` (`structured-agent-session-event-sink.ts:30-37`) has no `recovered` field, even though `JournalItemAppendOptions` supports one.
- Reachable via `writeClaudeTurnRow` → `tryReviseResolvedItem` with `{ newest: true }` after recovery (`claude-turn-row-revision.ts:129-146`, `claude-context-facts.ts:191-196` and `:213-221`), which the existing `src/main/claude/claude-context-usage-restart.test.ts:226-253` already exercises: `turn-a` is settled to `unverifiable` by `settleStaleSessionStateOnAcquire`, then revised by the drained context write, and the test asserts only `state`/`contextUsage`, not `recovered`.
## Required outcome
A turn row that crash recovery settled must keep dating the session at the recovery write after any later live revision. Either `recoveredAt` survives revision, or `statusStartedAt` must not fall back to an undefined idle clock that re-enables `updatedAt` republication for a recovered session.
## Suggested approach (optional)
Pin the marker in `upsertItem` beside `observedAt`/`sequence` (a revision refreshes content only), e.g. keep `existing.recoveredAt`/`existing.recovered` when the incoming row lacks them; or forward the row's `recovered` through the revision path. Coverage for the `unverifiable`-then-revised case would keep this from regressing.
## Open questions for the human (optional)
After recovery, should a later live context write keep the recovery instant as the completion edge, or should the revision's own time become the edge?
There was a problem hiding this comment.
ℹ️ This delta is clean — no new issues in
753f8e44. The earlier must-address finding on the recovered turn clock is still open, so this stays non-approving.
Reviewed changes
- Kept a child-held idle session republishing —
summariesEqualnow republishes an idle session whose live child work holds the row open even whenstatusStartedAtis unchanged, so the host row'sevidenceObservedAtkeeps refreshing instead of decaying to stale (structured-agent-session-status-feed.ts).stateStartedAtstill holds, since the row is not done and uses the continuity rule. - Covered the child-held path — a new Claude case in
structured-agent-session-subagent-recency.test.tsasserts the host row staysworking, keeps itsstateStartedAt, and refreshesevidenceObservedAtpast the staleness window while only a subagent writes.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
An answered ask returns a settled parent to its own turn's end, so its done repeats the time of the done before the ask. Activity keyed and ordered events by that time: the new done collided with the old one and was dropped, and the row took its state from the newest-dated event, the blocked ask, so a done parent read Blocked and needed attention. Each state switch now records the `updatedAt` it was seen at. Events are keyed and ordered by that, while unread and "Clear completed" still compare the state's own time, so the answer neither re-lights unread nor revives a cleared done. The row's state comes from the pane's own status entry, so a clear that hid the done cannot leave it reading Blocked either.
There was a problem hiding this comment.
ℹ️ This delta is clean — no new issues in
fc01d31. The earlier must-address finding on the recovered turn clock is untouched by this commit and remains open, so this stays non-approving.
Reviewed changes
- Ordered the activity timeline by observation, not by state start —
ActivityEventgainsobservedAt(when Orca saw the switch into the state) whiletimestampkeeps dating the state itself for unread and "Clear completed"; the event id, event cap, and thread sort now key onobservedAt, so an answered ask'sdone(which re-uses the turn's own end time) sorts after the ask that postdates it (activity-thread-types.ts,activity-pane-events.ts,activity-event-cap.ts,activity-thread-builder.ts). - Tracked each state's observation clock at the store — new
resolveAgentStatusLiveEntryStateHistoryextracts the history computation and stampsstateObservedAtwith theupdatedAtof the write that switched the state, carrying the previous state's clock onto the history entry it pushes (agent-status-live-entry-builder.ts,agent-status-live-entry-state-history.ts,agent-state-history.ts,agent-status-types.ts). - Read the row's state from the pane's own status row —
AgentPaneThreadgainspaneEntry, threaded through the event builder and caches, and the status classifier prefers its state over the newest event's so a row whose answereddonepredates itsblockedevent still readsdoneand stays clearable (activity-thread-presentation.ts,activity-event-builder.ts,activity-event-builder-sources.ts,activity-event-build-cache.ts,use-agent-pane-threads.ts). - Covered the path end to end — a renderer test drives the answered-ask timeline plus the clear interaction at exact event times, and a Codex test runs the real translator, deferral sink, durable journal and status feed through the bridge and Activity pipeline (3 tests pass; the activity suite runs 36 files / 198 tests green).
ℹ️ Nitpicks
- The PR description's "What Changed" still describes only the journal clock; it predates this commit and does not mention the Activity timeline /
stateObservedAtwork now bundled in.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
… stale turn Three parts of ordering the timeline by when each state was seen had no test that failed without them: - A second ask moves the answered done into history. Both dones share the turn's end, so only the history entry's own seen time keeps them apart; without it one done collided with the other and the timeline showed two Blocked events in a row. Three asks also exceed the per-pane cap, which must keep the most recently seen events, not the most recently started. - "Clear completed" on an answered row must cut off past the ask, which is dated after the done, or the cleared row stays listed. A done that the user cleared must also stay hidden once a later ask moves it into history. - A stale working row must not read as running just because the pane's own status says working.
There was a problem hiding this comment.
ℹ️ This delta is clean — no new issues in
13da358. The earlier must-address finding on the recovered turn clock is untouched by this commit and remains open, so this stays non-approving.
Reviewed changes
- Pinned the answered-ask timeline across repeated asks —
activity-answered-ask-timeline.test.tsxdrives three ask/answer cycles and asserts the exact event order andobservedAtvalues, so a repeateddonethat reuses the turn's own end time still sorts after the ask that postdates it. - Extended the clear coverage — the answered-row test now clears the row and asserts the post-dated ask clears with it, and a new case asserts a cleared
donestays cleared once a later ask moves it into history. - Pinned the pane-row state read —
ActivityPrototypePage.test.tsasserts a staleworkingpane row still reads asdone.
All 21 tests in the two changed files pass. The new assertions are exact-value (event order, observedAt, activityThreadStatusId), so they fail under the pre-fc01d31 timestamp ordering rather than absorbing whatever the pipeline returns.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
brennanb2025
changed the title
Review status: ready for reviewHead: What changed during review
Review loops: six rounds. The last one found nothing to change. Every central part of the fix was disabled in turn, and its tests failed for the intended reason. Final readiness check: passes, with nothing blocking. Wire compatibility (the new fields are optional), no new persisted data, no new docs files, and performance were all checked. It merges cleanly with #22532. Manual testing (macOS, real Claude CLI): before and after on the subagent scenario, a subagent's approval answered on an idle parent, and a crash in the middle of a turn. Screenshots are in the description. Known and accepted
Not covered: manual runs on Linux, Windows and SSH. The rules run on whichever host owns the session. |
1 participant
ELI5
A chat agent can start a helper (a subagent), finish its own turn, and go idle while the helper keeps working. Before this change, every step the helper took made the idle parent look like it had just finished: its row jumped to "now", moved up the list, and was marked unread again, even though you had already read it. Now the parent is dated only by what it did itself: when its own turn ended, when its own work started, or when it asked you something.
What Changed
Before. A session's status time came from the newest row in its log. The helper writes into the same log, so helper activity re-dated the idle parent in the sidebar, the Activity panel,
worktree ps, mobile and the dashboard, and re-marked it unread.After. The execution host works out when the session entered its current state from the session's own lifecycle, and publishes it as a new optional
statusStartedAton the session status summary (src/shared/agent-session-wire.ts). The rules live in one place,src/shared/structured-agent-session-status-started-at.ts:Helpers never create turns for their parent, so no rule has to filter rows by who wrote them.
Readers. The sidebar status bridge (
StructuredAgentSessionStatusBridge.tsx) and host ingest (server-ingest-structured.ts, which feedsworktree ps, mobile and the dashboard) usestatusStartedAtfor the row's state time whenever the row shows the session's own state. A row that is only held open by a helper's live work keeps the previous rule.Republishing. An idle session with a known start time now republishes only when that time changes. While a helper's live work holds the parent open, it keeps republishing on row activity as before, so mobile's 30-minute freshness rule does not flip a working row to idle.
Answered approvals. When a helper asks for approval while the parent is idle, the parent shows as blocked from the moment of the ask. When you answer, it goes back to done at its own original finish time. It is not re-dated and not marked unread.
Activity panel. Two changes, so an answered approval reads correctly there too:
stateObservedAton the live entry,observedAton history entries). The state's own time still drives unread and "Clear completed". So "Blocked → done" after an answer is its own event, listed after "Blocked". It is never dropped as a duplicate, and it does not re-light unread.Journal render items gain a transmitted
recoveredAt, needed because an item'sobservedAtstays fixed at creation. Nothing new is persisted.Why
Linked Issue
None filed. This is a bug found in the structured chat status work. The related follow-up for a false "running" turn from workflow progress updates is #22701.
Visual Proof
Real Claude CLI
2.1.280in a structured chat, dev app launched in the background and driven over CDP. Scenario: the parent starts a background helper, finishes its own turn, and the user switches away while the helper runs.Before (main): the idle parent in Workspace A reads
nowagain and Workspace A shows the unread bell, from the helper's work alone.After: while the helper is still running, the parent keeps its own finish time (
1mand counting) and is not marked unread.Helper approval on an idle parent, pending: the Activity panel shows the parent under Blocked.
After answering Allow: the row reads Done, not "needs attention", and it is not marked unread. The done time went back to the parent's own finish, 1.2 s before the ask.
Turn cut off by a crash: after the app was killed mid-turn and relaunched, the tool reads "1 failed", there is no success badge, and the chat is dated at the relaunch.
Testing
Manual, macOS only, with a real Claude CLI: the helper scenario on
mainand on this branch; the approval and crash scenarios on this branch.New tests:
structured-agent-session-status-started-at.test.ts: the date rules for each state, including a message sent mid-turn, a helper's ask compared with the session's own, and a recovered turn.structured-agent-session-subagent-recency.test.ts: end to end with the real Claude translator, journal and status feed. An idle parent with a running helper is not re-dated, and a helper that runs longer than 30 minutes keeps the host's live evidence fresh.structured-agent-session-recovered-turn-clock.test.ts: a turn settled at recovery is dated at recovery and sends no completion.structured-agent-session-status-feed-clock.test.ts,server-ingest-structured-state-clock.test.ts,structured-agent-session-status-bridge-clock.test.tsx: the feed's republish rule and both readers.activity-answered-ask-timeline.test.tsx: idle parent, then a helper asks, then the user answers. The panel reads Done, the timeline lists Blocked then done, and unread is not re-lit. Also covers repeated asks, "Clear completed" after an answer, and a cleared done that stays hidden.tests/e2e/codex-child-approval-activity-row.unit.test.ts: the same approval flow with a Codex helper, onmain's Codex translator.Each central part of the fix was disabled in turn, and its tests failed for the intended reason: the host date rule (12 tests), the readers (6), the republish rule (1), the Activity event id (3), the live-state lookup (1), and dating by recovery (4). The PR also merges cleanly with #22532, and the combined tree typechecks and passes the affected suites.
Also passing:
pnpm tc,pnpm exec oxlinton the changed files,pnpm run check:code-quality:changed,pnpm run check:react-doctor:changed, and thetests/e2e/cross-version-wiresuite.Not verified: Linux, Windows and SSH were not run manually. The rules run on whichever host owns the session, so they are the same on every host.
Review
statusStartedAt. Readers then keep their previous rule, so a new app paired with an old host gets no fix.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
updatedAtalready used.main.