Repository navigation
feat(session): add immutable task owner protocol codecs - #6283
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Decode captured owner and native continuation records without loading artifact stores or granting live deletion authority. Keep protocol parsing separate from future provisioning and retirement effects. Lore-id: 73ca041d Constraint: parsing data never proves physical retirement or writer closure Confidence: high Scope-risk: narrow Reversibility: easy Tested: 8 direct codec adversaries 44 assertions and corrected package types Tested: current settings GC storage codec union 196 passed 8 existing skips Not-tested: real owner provisioning and native retirement in this pure prefix
b5506d4 to
c2f29e0
Compare
probepark
left a comment
There was a problem hiding this comment.
Review (head c2f29e0, gajae-reviewer on behalf of probepark) — Approve held
Approve held: CI plan did not run cd packages/coding-agent && bun run check on c2f29e0; run it at this head (or add it to the plan) and re-request review.
The head's affected-path plan has no check:@gajae-code/coding-agent shard, so biome + check:types never ran in CI for this head. That matters here specifically: the new module is ~578 lines of type-narrowing over unknown (Record<string, unknown> property narrowing, as const tuple iteration at task-artifact-owner-codec.ts:510-518, the as NativeExactUnlinkResult cast at :550) — exactly the surface tsc --noEmit vets. The PR body reports a local bun --cwd=packages/coding-agent run check pass, but it does not tie that run to c2f29e0 with the command's exit code. A reply showing that command run at this exact head with exit 0 is enough to release the hold.
CI: 0 pending, 0 failed (gate checks excluded); typecheck shard absent from plan — see above.
Scope: +892 / -0, 2 files — packages/coding-agent/src/session/task-artifact-owner-codec.ts (+578, new, no production importer yet), packages/coding-agent/test/task-artifact-owner-codec.test.ts (+314).
Conventions: no changelog fragment (justified in body: inert internal DTO module); no generated files; no console.*; no new Worker; no labels.
Notable:
task-artifact-owner-codec.ts:240-266—retainedTreeDoesNotExpandAuthorityduplicates the helper of the same name insession-storage.tsbut adds a stricter mtime/ctime equality check whensha256is absent. Two diverging copies of an authority check is a drift risk once consumers land; consider making one delegate to the other.- Code review found no blocking candidates: unknown keys, non-canonical u64 ids, path traversal, parent/root mismatch, tree expansion and contradictory native
ok/code/payloadDurableflags all fail closed.
Blocking: none from code review; verdict withheld only for the missing typecheck run.
|
Exact-head package check proof for the held review: This is the already-executed current-head qualification, not an inference from CI or a later consumer head. The recorded Existing Biome large-file/unused-loader warnings and String.raw info were retained, not suppressed. The CI typecheck-shard omission is acknowledged; code-event CI success alone was not used as package-typecheck proof. Please re-review this exact head using the explicit exit-0 package result. The duplicate shrinking-tree check is recorded as an advisory integration risk; later consumers must not silently weaken its stricter timestamp/authority behavior. This inert root introduces no production importer or automatic owner producer. |
probepark
left a comment
There was a problem hiding this comment.
Review (head c2f29e0, gajae-reviewer on behalf of probepark)
need-local satisfied by #6283 (comment) — author ran bun --cwd=packages/coding-agent run check (verify:markit-vendor + biome check . + check:types / tsc -p tsconfig.json --noEmit) at exact head c2f29e0894f103a4872628eda481fd8482ff2933 on a clean checkout, exit code 0. This was the only condition of the earlier hold (review 5401970991).
CI: green — 0 pending, 0 failed (gate checks excluded); the plan omitted the check:@gajae-code/coding-agent shard, which the local exact-head run above covers.
Scope: +892 / -0, 2 files — packages/coding-agent/src/session/task-artifact-owner-codec.ts (+578, new, no production importer yet), packages/coding-agent/test/task-artifact-owner-codec.test.ts (+314). Reviewable source 578 lines (test excluded).
Conventions: no changelog fragment (justified in body: inert internal DTO module); no generated files; no console.*; no new Worker; no labels.
Notable:
task-artifact-owner-codec.ts:240-266—retainedTreeDoesNotExpandAuthorityduplicates thesession-storage.tshelper with a stricter mtime/ctime check whensha256is absent; the author has recorded this as an advisory integration risk. When consumers land, have one copy delegate to the other.
Blocking: none.
PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:ca1145415b4a50f7d775a38a044bd6b8259e6e66e3bdee390d1515dff06ec016 reviewer:human reviewer-id:probepark evidence:ci-green;code-reviewed-no-blocking;need-local-check-exit0-at-head-by-author-comment-5972271030
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:ca1145415b4a50f7d775a38a044bd6b8259e6e66e3bdee390d1515dff06ec016 reviewer:human reviewer-id:probepark evidence:ci-green;code-reviewed-no-blocking;need-local-check-exit0-at-head-by-author-comment-5972271030
|
Merged to dev: approved at c2f29e0, checks green, no open concerns. |
Summary
Review range and sequencing
dev, captured at8e709f5964987ab7dc206ca9d4b86c9e039f3803.c2f29e0894f103a4872628eda481fd8482ff2933.85155636fe40056498a99674bdb90aa338ef1568, not the later prepared branch head. This PR now contains that actual merge through dev ancestry and was freshly rebased and qualified on8e709f5964987ab7dc206ca9d4b86c9e039f3803. Its production review range remains 578; the earlier oversized cumulative prepared branch is not presented as a small PR.Verification (exact clean tracked source, independent QA)
bun test packages/coding-agent/test/task-artifact-owner-codec.test.ts packages/coding-agent/test/session-storage.test.ts packages/coding-agent/test/gc-disk-retention.test.ts— passed.bun --cwd=packages/coding-agent run check— vendor, Biome and TypeScript passed; pre-existing warnings/info preserved.04-owner-codec-merged-retirement-qualification.json; full output retained alongside it. Internal reports are evidence retention, not public CI substitutes.Limits
Darwin-focused API/package proof, not a full task-move/persistent-owner or public SDK deletion end-to-end claim. Existing platform skips/warnings remain uncredited. No physical reclamation or writer revocation claim. Exact-head code-event Dev CI
37139616416completed successfully atc2f29e0894f103a4872628eda481fd8482ff2933. Metadata-only37139647696was skipped and is not code evidence; old-headb550…run37139119934was cancelled and is not green. Exact-head write-access maintainer review remains required (REVIEW_REQUIREDat capture); old reviews are not transferred. No leader merge.No changelog fragment: this inert internal DTO module does not change observable runtime behavior. Original 44-file staged candidate and consolidated #6240 reference remain untouched and are not incorporated wholesale.