Repository navigation
feat(session): preserve exact task owner retirement authority - #6306
Conversation
Retain immutable parent and tree evidence across native partial removal. Fresh durable payload scrub can retire payload while the retained namespace remains pending; absence and old flags must never manufacture completion. Lore-id: 02a93d5f Constraint: no unsafe unlink native writer rewrite or physical cleanup fiction Confidence: high Scope-risk: narrow Reversibility: easy Tested: real native forwarding shrinking continuations forged flags and substitutions Tested: 41 owner capability tests 231 assertions plus vendor Biome types Not-tested: GC SDK journal consumers and final producer activation
probepark
left a comment
There was a problem hiding this comment.
Review — approve held (head c57cd5e, gajae-reviewer on behalf of probepark)
Approve held: CI plan did not run cd packages/coding-agent && bun run check (biome + check:types) on c57cd5e; run it at this head (or add check:@gajae-code/coding-agent to the plan) and re-request review. Post the command and its exit code in a PR comment — the body's "Biome, and TypeScript checks passed" line has no command/exit code and is not counted.
CI: planned checks green (Affected path validation incl. test:…/task-artifact-owner-retirement.test.ts, ts-build:coding-agent, gjc-state-gates, Virtual integration validation); check:@gajae-code/coding-agent absent from the head plan.
Scope: +820 / -0, 2 files — packages/coding-agent/src/session/task-artifact-owner-retirement.ts (+475, new), test (+345). Reviewable 475 lines.
Conventions: no changelog.d fragment (consistent with predecessor stack commits c2f29e0 / 9ec419e for this not-yet-wired internal API; no callers in src/), no generated files, no labels, no console.*, no new Worker.
Code review (no blocking candidates found):
task-artifact-owner-retirement.ts:276-282,308-314— pending.staging/.replacementpublication markers refuse before any native call, both on captured evidence and on each re-captured candidate root.:315-321— candidate roots require root dev/ino equal to the locator andretainedTreeDoesNotExpandAuthority(baseline, snapshot); baseline is the continuation's retained snapshot, so authority only shrinks across continuations.:322-340— canonical-path candidate is revalidated throughcaptureValidatedOwnerTree(session/manifest/identity) before removal.:368-384— parent identity re-checked after native removal; mismatch →uncertaincarryingnativeOutcome.:441-442—completedrequires nativeok, both canonical and.removingabsent, and no remaining side paths;verifyTaskArtifactOwnerPhysicalRetirement(:177-197) re-checks physically rather than trusting a decoded DTO.:443-460—payload_retiredrequirescleanup_pending+ freshpayloadDurable+ every residual file size 0 / empty sha256.- Tests:
vi.spyOn(native, "exactRemoveDirectoryTree")wraps the real native call;afterEach(vi.restoreAllMocks)at test:45 plus in-test restores — no leakage.
Notes (non-blocking):
:101-104—payloadDurableis sticky-true across continuations; this is fine becauseverifyTaskArtifactOwnerRetirementContinuation(:219-225) re-checks the residual payload, butretireTaskArtifactOwneritself only uses the freshnativeOutcome.payloadDurableat:446, so the stored flag is historical-only — a one-line comment would make that explicit.:143-148and:166-173capture parent identity twice innativeResultSidePathsRemain; the first check is redundant with the final one.
Blocking: none in code. Verdict deferred until the bun run check evidence exists at this head.
Suggested verdict line once satisfied: gajae.pr-review-verdict.v1 merge-approved sha256:8c85b986ad93574567e49388926842faf715c11f6ce77f5382cc5d91ebb7ae3f reviewer:human reviewer-id:probepark evidence:ci-green;owner-retirement-read;authority-shrink-checked;spy-restore-checked
|
@probepark this covers the gate your review held on. Please re-review the same head. — |
probepark
left a comment
There was a problem hiding this comment.
Review (head c57cd5e, gajae-reviewer on behalf of probepark)
need-local satisfied by #6306 (comment): cd packages/coding-agent && bun run check was run on exact head c57cd5e and exited 0 (biome 0 errors; the warnings are in untouched test/sdk-diagnostics-* files; check:types clean). This was the only condition holding the earlier review (#6306 (review)).
CI: planned checks green; approve gate's only open item was check:@gajae-code/coding-agent (NEED_LOCAL), covered by the comment above.
Scope: +820 / -0, 2 files. packages/coding-agent/src/session/task-artifact-owner-retirement.ts (+475, new) and its test (+345).
Conventions: no changelog.d fragment (internal API, not wired yet, same as the predecessor stack commits), no generated files, no labels, no console.*, no new Worker.
Notable: the code findings are unchanged from the held review. Pending-marker refusal at :276-282,308-314, authority-shrink check at :315-321, parent re-identity after the native remove at :368-384, and the completed/payload_retired conditions at :441-460. The spies are restored in afterEach. The two non-blocking notes still apply: sticky payloadDurable (:101-104) and a duplicate parent-identity capture (:143-148/:166-173).
Blocking: none.
PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:8c85b986ad93574567e49388926842faf715c11f6ce77f5382cc5d91ebb7ae3f reviewer:human reviewer-id:probepark evidence:ci-green;owner-retirement-read;authority-shrink-checked;spy-restore-checked;need-local-check-exit0-by-comment
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:8c85b986ad93574567e49388926842faf715c11f6ce77f5382cc5d91ebb7ae3f reviewer:human reviewer-id:probepark evidence:ci-green;owner-retirement-read;authority-shrink-checked;spy-restore-checked;need-local-check-exit0-by-comment
|
Merged into dev as
— |
Scope
Complete, independently callable task-artifact-owner retirement and live-verification APIs for the bounded split of reference #6240. Automatic persistent-owner production, task admission, GC driver integration, and broker effects are not activated by this PR.
c57cd5eddcf1a93fc8766e82499272d572504e12.61f2e17243af522e1572caf8d82f8f0e6a696684.Contracts
Retirement binds the exact original owner/session/profile/root/manifest/parent and native tree authority. Continuations retain the immutable evidence ceiling and only shrink residual tree authority. Native disposition, fresh durable payload scrub, physical namespace reclamation, and writable-descriptor revocation remain distinct.
Completed claims require original native outcome and fresh physical verification. Missing canonical paths, decoded DTOs, empty payload files, and historical payload durability never manufacture completion. Actual pending namespaces and separate native side-role diagnostics remain represented. Initial and restored staging/replacement publications are refused; no unsafe pathname unlink, empty-owner replacement, fabricated native success, or writer/coordinator redesign is introduced.
Exact-head qualification
07-owner-retirement-independent-base61f2e172passed in an exact clean checkout before and after execution:3708c0a4a667f5aa87d7d1fe39f7f003cdb79c141af9cf920e939843cfaf9dbe, matching committed diagnostic metadata and native source SHA-25626555fb71debdd40e0cdb70811847765452facd4cbb4107b646df7cc1a25dcaf. Qualification did not rewrite trusted metadata.An independent
setup:worktreebuild was also exercised. Its generated tracked metadata/types were archived separately and restored to the exact committed versions before strict byte-bound qualification; its different generated digest is not substituted for the trusted qualification digest.Darwin execution does not claim Linux, Windows, WSL/NTFS, arbitrary-FD revocation, or final task-after-move qualification. Code-event CI and exact-head write-access maintainer review remain separate requirements. The leader does not merge.
Preservation
Reference #6240 remains pinned at
7e5f7ef2a745bc1c3d012362f00be9a441bdeb76, source hashsha256:0ef698e426d34102d9be3e0c250a880059221bfa77c0ace2351bea28c97528ab. The original 44-file staged candidate is not transplanted, reset, stashed, or overwritten. Previously consolidated reviews do not approve this split head.Existing public workflow definitions and generated catalogs are unchanged. This inert internal API does not add a public command or automatic runtime behavior; activation-specific release guidance belongs to the separately qualified activation layer.