Repository navigation
feat(session): add strict managed GC retirement receipt codec - #6293
Conversation
probepark
left a comment
There was a problem hiding this comment.
Review (head 7e48d3d, gajae-reviewer on behalf of probepark) — approve held
CI: green. All planned checks passed (affected tests for managed-gc-retirement-codec, notifications-live-stream, session-manager-resident-cache; ts-build for coding-agent; native-build; gjc-state-gates; Virtual integration validation). The rest were skipped as not applicable.
Scope: +713 / -0, 2 files. packages/coding-agent/src/session/internal/managed-gc-retirement-codec.ts (319, new) and its test (394, new).
Conventions: no CHANGELOG entry. The body explains why: these are internal, inert DTO APIs, so that is acceptable. No generated files, no labels, no console.*, no new Worker.
Code review (3단계): no blocking candidates.
managed-gc-retirement-codec.ts:L175-213: the expected key set for each state is exact, andpayload_retiredadds its two flags only conditionally. L245-252 cross-checks those flags againstoutcome.kindin both directions. Covered by the "contradictory payload-retired owner flags" and "structurally consistent payload-retired" tests.managed-gc-retirement-codec.ts:L71-100: the identity decode is strict. It requires exactly 6 keys, canonical decimal strings, a signedmtimeNsthat rejects-0,nlinkequal to"1", and lowercase sha256. Covered by the six-field identity test.- Note (non-blocking):
continuationExtendsPrevious(L277) is exported and unit-tested directly, but nothing in this PR calls it. The body says the future journal reader will consume it. Keep that in mind so it doesn't stay a dead export.
Approve held: the CI plan did not run cd packages/coding-agent && bun run check (biome + check:types) on 7e48d3d. ts-build:ts:coding-agent only runs bun run build.ts, which bundles and does not run tsc --noEmit. The PR body reports bun --cwd=packages/coding-agent run check exit 0 at this exact head, but the reviewer's gate needs that check observed in CI, or confirmed in a reply after this review that gives the command and exit code at this head. Run it at this head (or add it to the plan) and re-request review.
7e48d3d to
1a6d58a
Compare
probepark
left a comment
There was a problem hiding this comment.
Review held: the approval gate reported a missing required CI surface at this exact head.
CI: gate pending — gaja-approve-gate.py returned NEED_LOCAL because check:@gajae-code/coding-agent was not in the head CI plan; Biome and check:types were not observed for this head.
Required action: run cd packages/coding-agent && bun run check at 1a6d58aa878bb8ad47d4a0b47e3a6f036acdc9b3 (or add the check to the plan), then re-request review.
No approval or body verdict update was made.
|
Current independent pure GC codec head |
|
Re: #6293 (comment) Approval remains held at The comment says "focused/package verification passed", but it does not name the command or give an exit code. To clear the hold, do one of the following:
When that is posted, a new exact-head verdict follows. |
Keep immutable captured authority and typed native continuation records separate from journal effects. An accepted DTO is never live deletion permission or physical retirement proof. Lore-id: 82c56bd0 Constraint: atomic journal consumers must add live proof before effects Confidence: high Scope-risk: narrow Reversibility: easy Tested: 19 access GC codec adversaries and package vendor Biome types Not-tested: journal ordering live retirement and GC activation in this pure prefix
1a6d58a to
e480e8f
Compare
|
@probepark could you re-review this head? — |
probepark
left a comment
There was a problem hiding this comment.
Review (head e480e8f, gajae-reviewer on behalf of probepark)
CI: green. All planned checks passed at this head: Affected path validation (plan, native-build, evidence producer, ts-build coding-agent, tests for managed-gc-retirement-codec, notifications-live-stream, session-manager-resident-cache), gjc-state-gates, Virtual integration validation, and Local public surfaces. Nothing is pending or failed. The rest were skipped as not applicable.
Scope: +713 / -0, 2 files. packages/coding-agent/src/session/internal/managed-gc-retirement-codec.ts (319, new) and packages/coding-agent/test/managed-gc-retirement-codec.test.ts (394, new).
Conventions: no CHANGELOG entry. These are internal, inert DTO APIs, as the body explains, so that is acceptable. No generated files, no labels, no console.*, no new Worker.
Delta vs prior reviews: git diff 7e48d3d e480e8f and git diff 1a6d58a e480e8f, limited to the two PR files, are both empty. This head is an owned-only rebase onto dev 73475e1, and the codec and test bytes are unchanged. The code review from 7e48d3d therefore still applies:
managed-gc-retirement-codec.ts:L175-213/L245-252: each state has an exact key set.payload_retiredflags are cross-checked againstoutcome.kindin both directions, and tests cover both the contradictory and the consistent case.managed-gc-retirement-codec.ts:L71-100: identity decode is strict. It requires 6 keys, canonical decimals, signedmtimeNsrejecting-0,nlinkequal to"1", and lowercase sha256.- Note (non-blocking, carried over):
continuationExtendsPrevious(L271) is exported and tested, but nothing in this PR calls it yet.
Prior hold resolved: the only open item was NEED_LOCAL cd packages/coding-agent && bun run check (Biome + check:types), which is missing from the head CI plan. need-local satisfied by #6293 (comment). That comment is from the repo owner, posted at 05:56Z, after the head commit at 05:12Z. It reports exit 0 on exact head e480e8f: Biome found no errors, and tsc -p tsconfig.json --noEmit found no errors.
Blocking: none.
Body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:952f3fbb7f83346ba84a007440a7aef17a6c714e0ac043f6410dbfcfcf088e1c reviewer:human reviewer-id:probepark evidence:ci-green;codec-bytes-unchanged-since-7e48d3d;need-local-check-exit0-by-owner-at-head
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:952f3fbb7f83346ba84a007440a7aef17a6c714e0ac043f6410dbfcfcf088e1c reviewer:human reviewer-id:probepark evidence:ci-green;codec-bytes-unchanged-since-7e48d3d;need-local-check-exit0-by-owner-at-head
Summary
Complete pure strict GC retirement receipt codec, independently callable without owner access, stores, native effects or automatic GC/task/session production. Four state DTOs: prepared, artifacts_removed, owner_pending, owner_retired; exact original owner/context identities and shrinking continuation shapes.
A decoded DTO is data, never native physical completion, fresh payload scrub, writer closure or deletion permission. Contiguous numbered history, ordering, original prepared binding and actual effect authority belong to the later complete journal reader/publisher, not this single-record parser. Attempt 2 alone may be valid data; it does not prove a complete history.
Exact independent review range
Base
dev, captured73475e180dfa4e72580288148baa8cb769a6dca3.Head
e480e8f0f59f2d243bc0fb30f1aa833d954755de.Production owned AND actual dev diff: 319 added+deleted; tests add 394 lines. This genuinely independent root does not include unmerged access337 or hide a cumulative parent diff. Runtime imports only the already-merged pure owner codec; storage types are type-only.
Exact clean-source verification
bun test packages/coding-agent/test/managed-gc-retirement-codec.test.ts packages/coding-agent/test/task-artifact-owner-codec.test.ts packages/coding-agent/test/task-artifact-owner-retirement.test.ts packages/coding-agent/test/session-storage.test.ts— exit 0: 136 passed, 8 existing platform skips, 577 assertions, four files.bun --cwd=packages/coding-agent run check— exit 0 at exactlye480e8f0f59f2d243bc0fb30f1aa833d954755de, vendor/Biome andtsc -p tsconfig.json --noEmitincluded.Independent exact detached clean tracked QA; actual addon matches committed strict 0.18.5 metadata and unchanged native source; no metadata rewrite. Source clean before/after.
09-gc-codec-independent-base73475e18-qualification.jsonrecords head and command exit codes; full output retained.Historical
1a6d58aa878bb8ad47d4a0b47e3a6f036acdc9b3on base03c code-event37154515048completed SUCCESS; it remains historical and does not approve this current734-based head. Prior head7e48d3d91acfc6fd000dca8f59fcb856bdd00c60code-event37148094157completed SUCCESS. This is an owned-only rebase to audited advancing dev, no codec change. New exact clean focused/package gates passed; old CI or review is not transferred, current-head CI/review required.Limits
Pure DTO tests are not physical retirement or replay/crash behavior proof. Darwin API/package execution only; existing platform skips and warnings/info uncredited. Automatic consumers/producers intentionally unchanged until the complete effects stack. No user-facing changelog for inert internal DTO APIs. Current code-event CI and exact write-access maintainer review remain required; no leader merge, transferred approval or whole-goal completion claim. Protected44 and consolidated #6240/reference retained unchanged.