Skip to content

fix(coding-agent): coalesce child-usage attribution and gate agent-status persistence on real changes - #2050

Open
snimu wants to merge 7 commits into
mainfrom
fix/coalesced-bookkeeping-appends
Open

fix(coding-agent): coalesce child-usage attribution and gate agent-status persistence on real changes#2050
snimu wants to merge 7 commits into
mainfrom
fix/coalesced-bookkeeping-appends

Conversation

@snimu

@snimu snimu commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Purpose

Two derived-bookkeeping paths appended durable journal entries at the wrong granularity, growing parent session files without bound and (for one of them) re-paying model calls forever:

  • Every child assistant message_end synchronously appended a child_usage_attributed entry to the parent journal and re-scanned the child transcript for the origin prompt — one journal append per model request of every RLM child, keeping parent files permanently "changed" for every metadata rescan (feeds the amplification fixed in fix(coding-agent): incremental single-flight session metadata scans #2043).
  • The 25s summarizer sweep appended an agent_status entry on every idle pass: a session whose generation cannot succeed (no prime-inference auth, model resolution failure, persistent parse failure) fabricated a needs_input fallback whose empty summary re-armed the retry forever — one identical durable entry per idle session per 25s, a persisted verdict the model never produced, and a paid model call per sweep in the parse-failure case.

Mechanism

  • Attribution (agent-session.ts): completions accumulate in memory (the parent's in-memory aggregate still updates per completion) and flush one summed entry per child agent_end, with a run-settlement backstop for error/cancel paths. All consumers fold attribution entries linearly (loader, scanner, context-tree), so the coalesced entry reloads to the same own-spend.
  • Agent status (daemon-session-summarizer.ts): persistence now requires a real generated classification that differs from the latest persisted entry (getLatestAgentStatus); fabricated fallbacks stay in memory only (the roster activity axis still settles); idle generations stop retrying after three failures on the same settled content until new activity arrives.

Measurement

  • One idle session without summary auth, one day of 25s sweeps (real SessionManager + summarizer driver): main appends 3456 agent_status entries (+591KB journal, unbounded); this PR appends 0 and stops generation after 3 attempts (3456 -> 3 generate calls — for persistent parse failures those are paid calls).
  • A child tool-loop turn with N model requests appended N attribution entries; now 1 per turn (pinned: the 2-request loop test went 2 -> 1 with summed usage).

Validation

  • Three pins verified fail-unfixed on main: coalesced tool-loop attribution (2 entries -> 1, usage summed), fabricated fallback never persisted + retry ceiling (5 sweeps: main appends 5/generates 5; fixed appends 0/generates 3), idle re-settle matching the latest persisted status appends nothing.
  • Suites: agent-session-recursion (115), daemon-session-summarizer (23), daemon-session-summarizer-lifecycle, session-manager/agent-status — all pass; root npm run check passes.

LOC

Total src: +128/−26 (net +102); tests: +200/−42 (net +158).
Src +64/−18 (net +46): both changes wire persistence to existing boundaries (child agent_end / run settlement; the summarizer's existing latest-persisted truth) rather than adding machinery. Tests +79/−9.

Squashes discussions #1788 and #1752; with #2043 this also removes the main journal-growth driver behind the #1503 worker OOM profile.

Linear: RES-1272 https://linear.app/primeintellect/issue/RES-1272


Note

Medium Risk
Touches durable session journal semantics for billing attribution and agent-status history; behavior is covered by new tests but incorrect coalescing or persistence gating could skew cost totals or dashboard status replay.

Overview
Stops unbounded parent session journal growth from two bookkeeping paths that were appending on every child model turn and every 25s idle sweep.

RLM child usage: Child assistant completions no longer call appendChildUsageAttribution on each message_end. Usage is accumulated in memory by origin (spawn_task, agent_message, direct_user) and flushed once per settle boundary (child agent_end, run cleanup, staleness after 60s, or a timer backstop). In-memory parent totals still update per completion; reload semantics stay consistent because consumers fold attribution entries linearly.

Daemon idle status: The summarizer caps failed idle model calls (3 attempts per settled content key, 30-minute backoff keyed by leaf id + message count) and only persists agent_status when the model actually returned a classification that differs from getLatestAgentStatus. Fabricated needs_input fallbacks remain in memory for the roster but no longer duplicate into the journal on unchanged idle sweeps.

Reviewed by Cursor Bugbot for commit 6b0af5d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Coalesce child-usage attribution in AgentSession and gate idle status persistence on real changes

  • AgentSession.runRlmChild now aggregates multiple per-request child usage records into one attribution entry per origin, flushing at child-turn settlement, cleanup, or after a 60-second RLM_CHILD_USAGE_FLUSH_MAX_PENDING_MS backstop. Failed bookkeeping appends no longer interrupt run settlement.
  • DaemonSessionSummarizer.summarize now caps idle-generation retries at three failed attempts per content key with a 30-minute backoff, and suppresses persisting fallback needs_input verdicts or duplicate settled statuses. Idle statuses are persisted only when the summary, task state, or message count differs from the latest persisted status.
  • Risk: The new 60-second flush timer in runRlmChild is unref'ed, so it will not keep the process alive; pending attribution can be lost on crash beyond the 60s window. The DaemonSessionSummarizer now relies on getLeafId from the session manager for content-key identity, so any caller not providing this mock data in tests will break.

Macroscope summarized 6b0af5d.

…dary

Every child assistant message_end appended a durable child_usage_attributed
entry to the parent journal and re-scanned the child transcript for the origin
prompt, so one chatty tool-looping child grew the parent file per model
request and kept it permanently changed for every metadata rescan. Completions
now accumulate in memory (the parent aggregate still updates per completion)
and flush one summed entry per child agent_end, with a settlement backstop for
error and cancel paths. All consumers fold attribution entries linearly, so
the coalesced entry reloads to the same own-spend. Fixes the defect reported
in discussion #1788.
…lassifications

The 25s summarizer sweep appended an agent_status entry on every idle pass:
a session whose generation could not succeed (no auth, model resolution
failure, persistent parse failure) fabricated a needs_input fallback whose
empty summary re-armed the retry forever, appending one identical durable
entry per sweep per idle session and re-paying the model call for parse
failures. Fabricated fallbacks now stay in memory (the roster axis still
settles), persistence requires a real generated status that differs from the
latest persisted entry, and idle generations stop retrying after three
failures on the same settled content until new activity arrives. Fixes the
defect reported in discussion #1752.
Comment thread packages/coding-agent/src/modes/daemon/daemon-session-summarizer.ts Outdated
Comment thread packages/coding-agent/src/core/agent-session.ts Outdated
…iling, key it to content

Review fixes for the bookkeeping-append gates: pending child usage now
accumulates per origin (steered agent_message usage is no longer mislabeled
spawn_task) and a batch older than 60s flushes before it grows or a tool
starts, so a process crash loses at most that window instead of a whole turn;
the idle-generation retry ceiling keys on settled content (message count plus
last timestamp, so a branch/edit back to the same length re-arms) and expires
after a 30-minute backoff so transient outages and late credentials recover
at one attempt per backoff instead of never; a forget() during an in-flight
generation no longer repopulates the failure map for a closed session.
@snimu

snimu commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review in 7bb135f, itemized:

  • M1 (crash durability) — fixed with a bounded compromise. Pending usage now flushes whenever the batch is older than 60s at the next accumulation or tool start, in addition to agent_end/settlement. A process crash loses at most 60s of accumulated child usage instead of an arbitrary turn; typical turns still produce one entry, so the per-message-append amplification stays dead. The tradeoff is now also stated in the code comment. Pin: a stale batch splits into two entries with correct partial sums (fails on the previous head with one [7,7] entry).
  • M2 (permanent ceiling) — fixed. The failure record expires after a 30-minute backoff: an externally-caused failure (outage, late credentials) recovers at one attempt per backoff (48/day worst case vs 3456/day before this PR) instead of never. A dedicated auth-config hook was declined as extra wiring the backoff already bounds. Pin with fake time (fails unfixed).
  • M3 (messageCount is not content identity) — fixed. The ceiling keys on messageCount + the last message's timestamp, so a branch/edit landing back on the same length re-arms classification. Pin (fails unfixed).
  • L1 (multi-origin interval) — fixed. Accumulation is per origin (Map<origin, Usage>), one flushed entry per origin present; the origin scan was rewritten as a backward loop (no slice/reverse copy), so computing it per completion is cheaper than the old per-append code.
  • L2 (forget() race repopulates the map) — fixed. Failures are recorded only when the generation's controller was not aborted; forget()/stop() abort in-flight calls, so closed sessions leave no record. Pin (fails unfixed).

All four new pins verified fail-unfixed against the previous head; suites (recursion 116, summarizer 26 + lifecycle) pass under a sanitized env; npm run check green.

Comment thread packages/coding-agent/src/core/agent-session.ts

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7bb135f. Configure here.

Comment thread packages/coding-agent/src/core/agent-session.ts
Message count plus last-message timestamp is not a branch identity: sibling
branches can collide on both, so branch navigation could stay blocked by a
failure record for different content. The leaf entry id is the branch tip
identity - appends, edits, and branch navigation all move it - so the ceiling
now keys on it, and navigation to a sibling branch re-arms classification.
@snimu

snimu commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the branch-navigation thread in 1566d09 + 601d8d0: the concern was valid.

Trace: SessionManager.branch(branchFromId) sets leafId = branchFromId on every navigation, while the previous key (message count + last message timestamp) is not a branch identity — sibling tips can collide on both (equal-length branches, shared-prefix last messages, same-ms edits). The retry ceiling now keys on getLeafId() (unique entry ids; appends, edits, and navigation all move it) plus the message count. activeSessionId does not change on branch navigation, so the per-session record must re-arm on leaf movement — pinned: after three failures on leaf A, navigating to sibling leaf B with identical count and messages re-attempts classification (fails on the previous head).

…wall-clock backstop

Two review findings on the coalesced attribution flush: the stale-batch flush
on message_end ran after attributeChildUsage folded the new completion, so the
flushed entry's aggregateUsage included a completion whose childUsage was not
yet durable and a replay of that prefix inflated the parent's own spend - the
flush now runs before the fold, making every persisted aggregate exact for
any prefix. And the staleness bound only fired at event checkpoints, so a
crash during one long tool execution could still lose the pending batch - a
60s unref'd timer now flushes the batch when no checkpoint arrives.
@snimu

snimu commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Both new threads addressed in 1d1e233:

  • Fold/flush ordering (cursor 14:35) — valid, fixed. The stale flush on message_end ran after attributeChildUsage folded the new completion, so the flushed entry's aggregateUsage included a completion whose childUsage was not yet durable; replaying that prefix inflated the parent's own spend by exactly the unflushed completions. The flush now runs before the fold, restoring the invariant that every persisted aggregate covers exactly the completions whose childUsage is durable with or before it — exact under any interleaving (agent_end, settlement, checkpoint, and timer flushes all happen outside the fold+accumulate pair, which is synchronous). Pinned with a replay-sum assertion: per-entry aggregates [3, 7] (fails on the previous head with [7, 7]) and reloaded parent aggregate minus persisted childUsage totals equals the original own spend.
  • Loss bound during long tool executions (macroscope 14:32) — valid; timer chosen over the checkpoint move. Entry-growth trace for "flush before any tool execution when pending is non-empty": every model request in a tool loop ends with a completion followed by its tool start, so that checkpoint flushes once per request — per-message granularity again (the 2->1 coalesce pin would revert to 2 entries). Declined. Instead a 60s unref'd timer is scheduled when a batch starts and cleared on every flush: the pending batch becomes durable within the bound even when no event checkpoint arrives, entry growth stays at most one entry per 60s per active child, and typical turns still produce one entry. The timer path reuses the pinned flush function; its wiring is six lines (schedule at batch start, clear in flush).

Recursion suite 116/116 under a sanitized env; npm run check green.

…date its pins

Comment blocks collapse to one- or two-line invariants. The tool-loop pins
share one session builder, the ceiling re-arm pins become one table (leaf move
and backoff expiry) whose warmup also carries the fabricated-fallback and
never-persist assertions, and the forget() pin reuses the shared setup. Every
fail-unfixed behavior keeps its assertion.
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