Skip to content

Make the family walk survive real profiles, then make it fast - #1370

Open
snimu wants to merge 22 commits into
core02-host-request-dispatcherfrom
review/c3-fork-lineage-and-perf
Open

Make the family walk survive real profiles, then make it fast#1370
snimu wants to merge 22 commits into
core02-host-request-dispatcherfrom
review/c3-fork-lineage-and-perf

Conversation

@snimu

@snimu snimu commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Make the family walk survive real profiles, then make it fast

Follow-up to #1333, based on its current head (8b21a40). Targets v080/core-split-c3-managed-catalog so it can be reviewed on its own and merged into the PR branch with one click. Companion to #1357, which patches #1243 below this stack.

All changes are in packages/coding-agent/src/modes/daemon/daemon-catalog-process.ts and its test file.

listCatalogFamilySessions runs on every agent-to-agent message, and also backs session create-with-name and rename. None of those call sites have a fallback: if the walk throws, all three operations stop working for the whole profile. I ran the current head against a real ~/.prime/agent profile and it failed for two separate reasons, and once past those it took several seconds per call. Both bugs have the same root cause: the validation was written against assumed data shapes, not the ones the product actually writes.

Bug 1: forked sessions treated as corruption

The walk assumes every session in the flat sessions dir is a root and throws as soon as one of them claims a parent. But /fork (and /new with lineage) writes a parentSession field into exactly those headers — that's fork history, not agent hierarchy. Both real header shapes kill the walk:

  • (a) parentSession set, rlmDepth absent (older versions wrote this) — dies in readTrustedSession ("session header lacks trustworthy topology claims").
  • (b) parentSession set with a numeric rlmDepth (what /fork writes today) — dies in the walk ("managed session seed claims a parent").

So one forked session anywhere in the profile breaks agent messaging, create, and rename. The profile I tested has 5.

Fix: if a flat-dir session's parent claim points at another file in the same sessions dir, it's a fork — include it as a normal root and drop the claim from the returned row (fork history must not show up as an agent-hierarchy edge). Shape (a) no longer throws either: a missing rlmDepth is a missing claim, not corruption. Parent claims pointing outside the sessions dir still fail closed.

Bug 2: the child-identity check rejects every registry the daemon has ever written

Checked against the real profile (597 live edges across 33 registries) and against the writer code in daemon-mode.ts:

  • Registries store childId as the rlm node id ("sub-779ec82e"), never the session UUID. The walk checked child.id !== entry.childId, which fails for 597 of 597 real edges — any profile that ever spawned a subagent is broken. The reliable identity anchor is the filename: children are always named after their header id, and all 597 edges satisfy basename(sessionFile) === header id. That's what the walk checks now. Registry keying by childId is unchanged, and a copied session file registered under the wrong name still fails closed.
  • A session's child registry lives at <dirname(sessionDir)>/session-artifacts/<header id> (that's where rlmSubagentRegistryPathForInfo writes it), not under dirname(sessionFile) as the walk assumed for nested parents. The walk was looking at a path that has never existed, so every family was silently cut off at depth 1. It now derives the writer's path — and finds the 11 nested registries (79 depth-2 sessions) the current head drops.
  • Older versions didn't write rlmDepth on children at all (429 of 597 edges). The walk demanded it and failed. A missing depth is now derived from the traversed edge (parent + 1); a present-but-wrong depth still throws.
  • Real registries are bigger than the 1MB cap — entries carry prompts and spawn code, the largest real one is 3.3MB. The cap is now 16MB.

With both bugs fixed, the walk returns the full real family: 775 sessions (177 roots incl. forks, 520 depth-1, 78 depth-2) in 1.7s cold / ~0.2s warm.

Perf: 7.5s → under 30ms warm per walk

Two problems: the helper shipped entire session files base64-encoded over stdout even though the caller only reads the first line (433MB profile → ~645MB piped per walk), and it spawned one python3 process per file (~330 spawns, ~22ms each). Measured on a synthetic profile mirroring the real one (150 flat sessions with multi-MB bodies, fork headers in both shapes, registries with realistic sub-* ids, nested grandchildren; 162 members, 479MB):

base (pr-1333-review) this branch
family(), cold 7.4–7.9s* 591ms (≈500ms is the pre-existing listAll scan)
family(), warm 7.4–7.9s 28–29ms
bytes piped per walk ~645MB a few hundred KB
helper processes per walk ~330 1

* the base can't walk this profile at all (fork seeds throw); its row was measured on the friendliest possible input — no forks, equal-id registries.

Two commits:

  1. Header-only reads. The helper gets request modes: header stops at the first newline under a 256KB budget; stat checks existence and identity with no content transfer (this now serves the parent-existence probe, which used to re-read the whole parent file and throw the bytes away); read keeps full content for the bounded registries. The fstat-before/after identity check is unchanged in all modes.
  2. One helper per walk. The python3 -I helper now loops: newline-delimited JSON requests on stdin, one response line each, exit on EOF. Both authority roots are passed as fds at spawn, so every open stays descriptor-relative under the O_NOFOLLOW roots. One helper session per walk, closed in the same finally as the root descriptors. Async spawn, per-request timeout, shell: false; any protocol violation (unsolicited output, oversized response, unexpected exit) kills the helper and fails the walk closed.

Also fixed in passing: the managedRoots error path closed the session-root descriptor without decrementing the test-visible fd counter.

Deliberately not done: caching parsed headers by (dev, ino) across walks. It would shave the remaining warm cost but couples cache invalidation to the helper lifecycle; not worth it at current profile sizes.

Alternative design: supervisor-owned spawn ledger

Worth being honest about what the walk's re-validation buys. Every file it reads is writable by the same user the daemon runs as, so re-deriving the family on every message verifies that the claims are consistent — not that they're true. Anyone who can write the sessions dir already owns the trust domain.

The daemon itself admits every spawn. If the supervisor recorded parent-child edges at spawn time in its own append-only ledger — outside the sessions dir, written under its existing lock — family() would become one small file read plus a stat pass. No openat helper, no walk, and both bug classes fixed here become structurally impossible, because topology would never be re-derived from files other writers produce. Same security against the same-user threat model; the ledger just trusts one file the daemon wrote itself instead of N files written by everything else. This PR keeps the current on-disk contract because it needs no migration — but the ledger is the better end-state.


Note

High Risk
Changes security-critical agent-family reachability, name reservation, and catalog I/O on the hot path for agent-to-agent messaging; failures or mis-merged topology fail closed but can break whole profiles.

Overview
Makes the daemon family catalog match real on-disk profiles and stops re-deriving agent topology from live runtime state on every message, create, and rename.

The catalog process adds a family command and reimplements listSavedSessionSiblings via listCatalogFamilySessions: O_NOFOLLOW authority roots, a single per-walk Python openat helper with header / stat / metadata modes (not full session bodies), and hard caps on nodes, edges, depth, and registry size. The walk treats flat-dir parentSession claims as fork lineage (roots, no hierarchy edge), keys registry children by sub-* writer layout and basename ↔ header id, derives missing child rlmDepth, and fails closed on cycles, symlinks, hostile topology, and post-enumeration file swaps.

session-manager gains buffer-only readSessionHeaderInfoFromBuffer / readSessionInfoFromBuffer so trusted reads never follow legacy parent paths.

agent-messages tightens sameAgentFamilyParent to a unique catalog-resolved parent, validates isAgentFamilyParent when both id and path are present, and adds a name-reservation-only direct parent-claim fallback for passive children (not used for reach). agentFamilyRelationship / assertAgentFamilyReach take an optional full catalog argument.

daemon-mode builds agentFamilyCatalogEntries once (saved, passive, resident, remote with claim-aware merge) and uses that snapshot for observe, roster, hydration, and agent-origin messaging; CLI relationship labels can degrade if the catalog is unavailable.

daemon-supervisor scopes familyCatalogEntries, name checks, sibling listing, and pre-wake a2a authorization to sessionDir, merges persisted / artifact / live catalog rows, and passes sessionDir on inactive rename_saved_session (protocol schema revision 17).

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

Note

Scope agent family catalog walks to session directories and enforce strict topology authorization

  • AgentDaemon now builds a single immutable family catalog snapshot per message delivery by merging live, persisted, and artifact-resident entries; authorization and sibling labeling use this snapshot rather than re-deriving from runtime state
  • familyCatalogEntries in daemon-supervisor.ts and agentFamilyCatalogEntries in daemon-mode.ts now accept a sessionDir to scope discovery, preventing cross-directory topology leakage
  • daemon-catalog-process.ts adds bounded, authority-anchored family traversal (listCatalogFamilySessionsWithLimit) with strict limits on nodes, edges, and depth, and cycle/duplicate detection
  • isAgentFamilyParent and sameAgentFamilyParent in agent-messages.ts now require catalog-resolved, unambiguous parent edges; contradictory id/path claims are rejected
  • rename_saved_session daemon commands now carry sessionDir and require schema revision 17 when present; legacy forms without sessionDir remain backward-compatible at protocol 7
  • Risk: cross-worker message delivery is rejected if the active target session ID does not match the ID authorized in the pre-wake snapshot, or if the family catalog contains duplicate/ambiguous entries

Macroscope summarized 4e2bbd2.

sethkarten and others added 7 commits August 12, 2026 22:02
Reconstruct the unique net delta from PR #1261, excluding propagation merges.
Scope live family catalogs, anchor relative parent paths, and retain legacy rename compatibility without weakening authority-aware requests.
… root open fails

managedRoots closed the session root descriptor on the error path but never
decremented the test-visible open-descriptor counter, so a failed artifacts
open left the count permanently drifted.
…orruption

/fork and lineage-carrying /new write parentSession into flat sessions-dir
headers as fork ancestry, not rlm topology. The family walk treated every
flat-dir session as an rlm seed, so one forked session anywhere in the
profile made family() throw invalidFamilyTopology — hard-failing agent
messaging, create-with-name, and rename. Two real-world header shapes both
triggered it: parentSession with a numeric rlmDepth (current fork writer)
and parentSession with no rlmDepth at all (older writer).

A flat-dir session whose parent claim resolves inside the same sessions dir
is now a depth-0 family root with the fork claim dropped. Parent claims that
escape the sessions dir still fail closed, and registry-reached children
still enforce the strict parent/depth invariants (absent depth on a child is
now an explicit error instead of an accidental one).
… family walk

The walk transferred every session file whole (128MB limit, base64 over
stdout) although only the first header line participates in the trust
decision, and it re-read the claimed parent file whole just to discard the
bytes. On a real profile (177 sessions, 433MB) that cost ~4.6s and ~570MB of
pipe churn per family() call.

The openat helper now takes a mode: 'header' stops at the first newline
under a 256KB budget, 'stat' verifies existence and identity without
transferring content, and 'read' keeps the old behavior for the small
bounded registries. Display metadata (names, previews) comes from the
caller's listing or the ordinary cached read and is bound to the
descriptor-read header by the id cross-check; topology claims always come
from the header bytes.
Every trusted read spawned a fresh python3 -I process, so a walk over N
sessions paid N+ interpreter startups. The helper now loops: newline-
delimited JSON requests on stdin, one JSON response line each, exit on stdin
EOF. Both authority roots are passed at spawn (session root on fd 3,
artifacts root on fd 4 when present) and selected per request, keeping every
open descriptor-relative under the O_NOFOLLOW roots.

listCatalogFamilySessions creates one TrustedReadSession per walk and closes
it in the same finally as the authority roots. The helper is spawned async
with a per-request timeout; any protocol violation (unsolicited output,
oversized response, write failure, unexpected exit) kills the helper and
fails the walk closed.
…lly writes

Three walk assumptions did not survive contact with a real profile:

- Registry childId is the rlm child id (e.g. "sub-1a2b3c4d"), never the
  session id, so the identity check 'child.id !== entry.childId' rejected
  every real registry edge. The trustworthy identity anchor is the child's
  filename: the session writer names each child file after its header id, so
  the walk now requires basename(sessionFile) === header id. Registry keying
  (latest-wins per childId) is unchanged.
- A session's child registry lives beside its session dir
  (<dirname(sessionDir)>/session-artifacts/<id>), not under
  dirname(sessionFile). The old derivation looked in the wrong place for
  every artifact-resident parent, silently truncating families at depth 1.
- Old writers persisted no rlmDepth on children (429 of 597 live edges in
  the measured profile). An absent child depth is now derived from the
  traversed edge; a present-but-contradicting claim still fails closed.

Also raise MAX_RLM_REGISTRY_BYTES to 16MB: registry records carry prompts
and spawn code, and real registries exceed the old 1MB cap.

Synthetic registries in the tests now mirror reality (sub-* childIds,
writer-layout registry paths, nested children under the parent's session
dir), one legacy equal-id variant remains, and a new end-to-end fixture
covers fork seeds plus a two-level registry family.
Comment thread packages/coding-agent/src/modes/daemon/daemon-catalog-process.ts
Comment thread packages/coding-agent/src/modes/daemon/daemon-catalog-process.ts Outdated
@sethkarten

Copy link
Copy Markdown
Contributor

Progress update at exact public head 87a84dae143745e17c4979d101f6a88d9938808e:

The latest local follow-up candidate (ad0) was rejected and was not pushed. Confirmed remaining issues are: truncated/oversized claimed-session headers do not reliably reach the strict classifier, the base64-encoded header response can exceed its bound, and metadata compaction can corrupt the request ID used to bind helper responses. The next change will be a clean append-only fix with deterministic regressions for all three. The public head is unchanged; existing review threads remain open.

No MCP work and no merges.

Base automatically changed from v080/core-split-c3-managed-catalog to core02-host-request-dispatcher August 13, 2026 21:00
@sethkarten

Copy link
Copy Markdown
Contributor

Published the validated append-only catalog correction at 731af01e7bf1ba8c4eb5d38a32eecbbbcc2b996d (fast-forward from the prior public head).

Exact-tip local gates completed before publication:

  • strict root tsgo --noEmit -p tsconfig.json
  • exact Biome 2.5.5 and git diff --check
  • focused catalog suite: 31/31 tests passed
  • independent exact-tip source review passed

This includes bounded incremental root enumeration, the fixed production cap, descriptor-budget enforcement, ENOENT-only skipping, and conservative malformed-prefix handling. CI on this public SHA is now the remaining public gate; this update does not merge into main.

@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 731af01. Configure here.

Comment thread packages/coding-agent/src/modes/daemon/daemon-catalog-process.ts
@sethkarten

Copy link
Copy Markdown
Contributor

Published append-only correction 4e2bbd21504bd9acac5c6dd58b3bd8eff256de94.

This preserves exact catalog root device/inode identity using bigint filesystem metadata, including values above JavaScript’s safe integer range. A deterministic regression distinguishes adjacent identities that would otherwise round to the same Number.

Validation on the exact commit:

  • strict root typecheck passed
  • exact Biome on changed files passed
  • focused catalog suites passed: 33 tests
  • git diff --check passed
  • independent exact-tip review found no correctness issue
  • validation symlink and caches removed; worktree clean

The update was a verified fast-forward from the exact API and HTTPS branch lease. No merge performed.

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.

2 participants