🤖 feat: adopt pre-sharing sub-agent notebooks into the shared store - #4220
🤖 feat: adopt pre-sharing sub-agent notebooks into the shared store#4220ThomasK33 wants to merge 9 commits into
Conversation
…ed-substitute mutations mergeKeys copies a subtree's pins/usage onto a second logical key (keeping the source; larger counters win, the pin stays with the target unless the caller says the source's is newer). loadWithHealth distinguishes a sidecar that exists but could not be read from a missing one: reads still heal to empty, but that view is neither cached nor written back by a mutation, and getEntriesOrThrow refuses it for callers that consume the entries destructively. --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$34.37`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=34.37 -->
…red store Sub-agents created before sharing kept /memories/workspace under their own session dir; the owner redirect made those notes invisible and removal would delete them. On the child's first shared-store access per process (and again, strictly, before removal deletes its session dir) every legacy file is copied into the owner's store — same relPath when free or identical, imported/<child>/ on a conflict — via a staged rename under the owner store lock, with child-keyed pins/usage folded onto the owner key and a manifest (beside the legacy root, never inside it) recording what landed where. The legacy directory is left untouched for downgraded builds; identical files are skipped, notes edited on a downgraded build re-adopt under imported/. --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$34.37`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=34.37 -->
…ocks Removal of a sub-agent runs the legacy-notebook handover before any destructive step and again inside the removal lock set immediately before the tombstone (beforeTombstone), sealing the tombstone BEFORE the checkout is deleted so nothing can land in between; a handover that cannot place a note aborts the removal (SharedMemoryRemovalAbortedError) with the workspace intact, unless force accepts the loss. A checkout deletion refused after sealing rolls the tombstone back. The tombstone lease is held from before the earliest publish point. --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$34.37`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=34.37 -->
…n-thenable --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$34.37`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=34.37 -->
ThomasK33
changed the title
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 365d8d6679
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…arantine malformed manifests Codex round 1 on the legacy-notebook layer: - The owner's own workspace-memory access now runs the access-time adoption for every registered descendant resolving to it (one config snapshot per pass, one lstat per child without a legacy root), so a sub-agent that finished before the upgrade no longer has to touch memory itself for its notes to become visible. - The access-time pass memoizes its legacy-store check key even when notes were left unrepresented (owner store full, unrepresentable entries): an unchanged legacy store cannot adopt more on retry, so it is no longer re-walked on every access. Removal's strict pass still throws. - A malformed adoption manifest is quarantined beside itself (<name>.malformed-<ts>) and the pass continues from an empty record map (re-adoption is idempotent); a failing quarantine rename or an unreadable manifest still fails closed. --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$41.17`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=41.17 -->
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 894bde6119
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…y, roll back stale seals Codex round 2 on the legacy-notebook layer: - The adoption pass counts transient failures (a read, stage, install or sidecar write that failed) apart from permanent skips (over the cap, doubly conflicting, not text, escaping destination) and memoizes its check key only when none were transient, so a note behind a permission interval is retried on the next access while an unchanged over-cap notebook is not re-walked. Removal's strict pass is unchanged. - Legacy notes are read as bytes and decoded with a fatal UTF-8 decoder: a literal U+FFFD is adopted, invalid byte sequences are skipped. - The removal tombstone is rolled back whenever the removal ends with the workspace still registered (a teardown step failing after the seal, a refused checkout deletion, deregistration failing), not only before the runtime-deletion commit point; the consolidation teardown gate is released on the same truth and harvest records are finalized only after deregistration. --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$51.16`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=51.16 -->
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e15bad86d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…date legacy names Codex round 3 on the legacy-notebook layer: - The legacy store's change stamp walks the FULL directory (strict, uncapped, sizes and mtimes only), so an edit to a note sorted past the capped listing re-runs the adoption pass; adoption itself stays capped. - A descendant's first adoption onto an identical owner note keeps the owner's pin only when the owner wrote that note; when the note is another descendant's adopted copy (its manifest records it as created), the descendants' pins combine (mergeKeys pinned: "either"). - Finalizing harvest records after deregistration is best-effort: a failure is logged and the committed removal still succeeds and emits its event. - A legacy file whose name the memory path grammar rejects is never copied into the shared store (a permanent skip removal still reports). --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$52.95`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=52.95 -->
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6102b00bf8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… to later layers) Scope reduction after review: keep this layer at owner-side adoption plus the R1/R2 hardening. The two R3 additions below grew scope beyond the layer and are deferred: - legacyStoreStamp goes back to the capped, non-strict listing; an over-cap legacy store fingerprints only the capped prefix (edits past it are picked up by a restart or removal's forced pass). The throttled full-store fingerprint lands with the multi-backend layer. - mergeKeys drops the `pinned: "either"` mode and MemoryService no longer inspects sibling manifests to combine descendants' pins on a first adoption; the owner's pin stands as before. Kept from R3: best-effort finalize in workspaceService and the unaddressable-legacy-name skip with its test. --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$77.94`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=77.94 -->
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08d1a7a09c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…bytes strictly, retry unreadable ones The destination-side comparison of the legacy adoption pass (both the placement in legacyImportTarget and the prior-copy re-check in inspectAdoptedCopy) now goes through one helper, inspectAdoptionDestination: - lstat first, regular file required: a FIFO/socket/device, directory or symlink at a destination is occupied owner state and is never opened (open() on a FIFO blocks until a peer appears and hung the pass). - destination bytes are read raw and decoded strictly, so an owner note holding invalid UTF-8 no longer settles a legacy note that literally contains U+FFFD as "already present"; the legacy note is copied byte-exact under imported/<child>/ instead. - an EACCES/EIO on the destination's lstat or read propagates as a TRANSIENT skip (no copy, no manifest record, pass not memoized, retried on the next access) instead of being read as a mismatch that duplicated the note under imported/<child>/; removal still refuses on skipped > 0. Tests: FIFO at both slots (skips if mkfifo is unavailable), U+FFFD vs invalid-UTF-8 pair, EACCES on the destination then cleared. --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `high` • Cost: `$81.16`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=high costs=81.16 -->
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fdba459a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
Notes a sub-agent wrote to its private workspace notebook before notebook sharing (#4210) are adopted into the task tree's shared store the next time the child or the owner touches workspace memory, and handed over under the removal locks before a child's session directory is deleted. Adoption is idempotent, never deletes the legacy source outside a successful removal handoff, keeps a name clash as
imported/<child>/<file>, and folds the child's sidecar pins and usage into the owner's keys.Stack: layer 3 of the split of #4139 (base:
memory-sharing-l1b, #4217). This is the layer that makes the bundle#4210 + #4217 + #4220safe to ship: without it, pre-upgrade child notes become invisible after #4210 and are deleted with the child's session directory after #4217.Background
Sub-agents have existed for weeks, so private child notebooks exist in the wild. Codex flagged this on #4210 (deferred here) and again on #4217 for the downgrade case: an older build ignores sharing and writes to the child's private store; on re-upgrade those notes must reappear. Both cases are the same adoption pass.
Implementation
memoryLegacyAdoption.ts(new): adoption manifestmemory-adoption-manifest.jsonin the child's session dir, one record per adopted legacy path (content,sidecar,target, optionalcreated/pending/targetStamp). Records parse fail-closed and ignore unknown fields so a later layer can add reconciliation fields.MemoryService.openWorkspaceStore: readable check, adoption pass, readable check.adoptLegacyPrivateStore(access time, non-throwing, memoized per legacy-store stamp so an unchanged store costs one stat pass) andadoptLegacyPrivateStoreForRemoval(strict: unreadable legacy state or an unplaceable note throws).adoptLegacyPrivateStoreOrThrowlists the legacy store strictly (dotfiles included, unrepresentable entries refuse), stages copies in a temp dir, writes the manifest before installing, and installs with a rename only if the target is still absent; a clash goes toimported/<child>/.MemoryStore.listFiles/kindgainstrict/includeDotfilesoptions.MemoryMetaService.mergeKeysfolds the child's sidecar entries into the owner keys (pins win, usage takes the max);loadWithHealth/getEntriesOrThrowrefuse to mutate on top of a healed-substitute sidecar, so removal never treats an unreadable sidecar as empty.sealSubAgentForRemovalUnderMemoryLocksruns the strict handover under the full lock set (refine, session dir, child store, shared store, owner store, history) and publishes the tombstone in the same critical section, before the checkout is deleted; a handover failure raisesSharedMemoryRemovalAbortedErrorand a non-forced removal aborts with the workspace registered and its session directory intact.forceaccepts the loss the handover cannot place.Simplifications in this layer (next layer: downgrade reconciliation, required before this stack ships)
imported/<child>/<file>; the earlier copy stays. A second edit of a note that already lives underimported/<child>/cannot be placed and is reported by the strict removal handover (non-forced removal refuses) until the reconciliation layer adds generation-stamped in-place replacement.imported/<childId>/segment for child IDs the memory path grammar rejects, lstat classification of unknown dirent types during strict walks, and BOM-preserving byte-exact compare/copy of legacy notes.Validation
Tests cover: visibility of a pre-existing private note through the child and the owner with the pin carried, idempotent second access, clash placement, capacity cap, dot entries and unrepresentable entries, unreadable/malformed legacy sidecar and manifest under strict removal,
__proto__paths, symlinked legacy roots, self-fallback notes written while the owner was unresolvable, a read tombstoned during the adoption pass, handover under the held owner lock aborting without a tombstone, handover ordering before checkout deletion, teardown gate released on abort,mergeKeyssemantics. Sibling suites: 1791 pass.Risks
Medium. New on-disk artifacts: the
memory-adoption-manifest.jsonmanifest in the child's session dir and amemory-adoption-stagingdirectory beside the owner'smemory/dir; both are additive and ignored by older builds. Adoption runs inside the store mutation lock on first access after upgrade; an unchanged legacy store is memoized afterwards.Generated with
xum• Model:anthropic:claude-fable-5-1• Thinking:high• Cost:$1015.74