Repository navigation
feat(session): decode task owner locators from transcript replay - #6307
Conversation
Journal target binding needs the effective locator after strict v4 header patch replay, without importing storage effects or a future async scanner. Expose the complete pure parser as a standalone prerequisite and reject malformed owner patches instead of falling back to an earlier locator. Lore-id: c0248b61 Constraint: decoded locators never confer filesystem or deletion authority Constraint: no scanner stub, compatibility alias or automatic owner activation Tested: 125 tests, 8 existing skips and 514 assertions across three suites Tested: coding-agent vendor, Biome and TypeScript checks Confidence: high Scope-risk: narrow Reversibility: revert
|
Exact-head code-event verification completed for Local exact-clean qualification remains 125 passed / 8 existing platform skips / 514 assertions across three suites plus package checks. Fresh protection audit confirms original HEAD |
probepark
left a comment
There was a problem hiding this comment.
Review (head 052b307, gajae-reviewer on behalf of probepark)
Approve held: CI plan did not run cd packages/coding-agent && bun run check (check:@gajae-code/coding-agent = biome + check:types) on 052b307; run it at this head (or add it to the plan) and re-request review. The PR body's "Biome, and TypeScript checks passed" has no command/exit line tied to this head, so it does not satisfy the gate. A comment of the form <cmd> → exit 0 at 052b307 will.
CI: planned checks green (Affected path validation incl. test:…/task-artifact-owner-transcript.test.ts, ts-build coding-agent, gjc-state-gates); check:@gajae-code/coding-agent not in plan.
Scope: +153 / -0, 2 files — packages/coding-agent/src/session/internal (68 prod lines), tests (85).
Conventions: CHANGELOG none (inert internal API with no caller; acceptable as stated in the body), generated files none, labels none, no console.*, no mock.module/spies.
Notable (non-blocking):
task-artifact-owner-transcript.ts:L29-L32— the non-JSON fallback throwstask_artifact_owner_patch_invalidwhenever an unparseable line merely contains bothheader_patchandtaskArtifactOwner. A crash-truncated trailingmessagerecord whose text mentions those words (e.g. a session discussing this code) fails the whole replay closed even though it is not a patch. If this is intended fail-closed policy, consider restricting the check to a line that starts with{"type":"header_patch", or to the final unterminated line only.task-artifact-owner-transcript.ts:L15— replay is gated onversion >= 4but has no upper bound;buildFileEntriesFromRecordsrejectsversion > CURRENT_SESSION_VERSION(isSupportedSessionVersion, session-manager.ts:L2800). A future-version transcript would be replayed here with v4/v5 semantics instead of being refused. Mirroring the bound keeps the two replay paths aligned.
Checked and clean: patch key/type allow-list matches isHeaderPatchRecord (session-manager.ts:L2813-L2829) plus taskArtifactOwner; locator decode reuses parseTaskArtifactOwnerLocator (strict 4-key, canonical ids) with errors rewrapped; the last valid patch wins; v3 headers skip replay just like buildFileEntriesFromRecords; tests are pure (no mocks) and cover the trailing-newline, v3, malformed, truncated/invalid-UTF-8 and wrong-session paths.
Blocking: none in code. Approval is waiting only on the missing local check above.
|
Explicit local package gate for review: |
|
@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 052b307, gajae-reviewer on behalf of probepark)
need-local satisfied by #6307 (comment) (maintainer: cd packages/coding-agent && bun run check → exit 0 at 052b307; biome 0 errors, check:types clean) and #6307 (comment) (author: bun --cwd=packages/coding-agent run check → exit 0 at 052b3074e9a7e2024cf55e341670a699a4d973a9, checkout clean). This resolves the hold in our review 5404421452.
CI: green. All planned checks pass, including Affected path validation (test:…/task-artifact-owner-transcript.test.ts, ts-build coding-agent) and gjc-state-gates. The check:@gajae-code/coding-agent gap was covered by the local runs above.
Scope: +153 / -0, 2 files. packages/coding-agent/src/session/internal has 68 prod lines; the rest (85 lines) are tests.
Conventions: no CHANGELOG entry (an inert internal API with no caller, as the body says), no generated files, no labels, no console.*, no mock.module/spies.
Notable (non-blocking, unchanged from the previous review):
task-artifact-owner-transcript.ts:L29-L32: the non-JSON fallback fails closed on any unparseable line that contains bothheader_patchandtaskArtifactOwner. A truncatedmessagerecord that mentions those words would fail the replay. Consider checking only for a{"type":"header_patch"prefix or only the final unterminated line.task-artifact-owner-transcript.ts:L15:version >= 4has no upper bound.isSupportedSessionVersion(session-manager.ts:L2800) refuses> CURRENT_SESSION_VERSION, so mirroring that bound here would keep the two replay paths aligned.
Blocking: none.
PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:1ebdb1b801a84b2855f6cf83a3e66726f58e7f276a4f7ccc8ccfa6c62916564c reviewer:human reviewer-id:probepark evidence:ci-green;need-local-check-exit0-at-head-by-maintainer;replay-allowlist-and-version-gate-read;tests-pure
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:1ebdb1b801a84b2855f6cf83a3e66726f58e7f276a4f7ccc8ccfa6c62916564c reviewer:human reviewer-id:probepark evidence:ci-green;need-local-check-exit0-at-head-by-maintainer;replay-allowlist-and-version-gate-read;tests-pure
|
Merged into dev as
Thanks @snowykr. — |
Scope
A complete, pure transcript-to-owner-locator replay API for the safe current-dev installation of the bounded #6240 split. This is a real parser prerequisite, not a scanner stub, compatibility alias, DTO-only placeholder, or owner-effect activation.
052b3074e9a7e2024cf55e341670a699a4d973a9.550afd5d3476a775747684160b2b6ded7764df0f.The API validates the logical session header, decodes the strict path-free locator, and replays v4 owner header patches. It rejects malformed or noncanonical later owner metadata rather than silently using the earlier header locator. Legacy v3 replay remains unchanged. Truncated and invalid UTF-8 owner-patch records fail closed.
A decoded locator remains data, not filesystem permission, deletion authority, or native retirement proof. There is no filesystem mutation, broker acceptance, async sibling scanner, GC cleanup effect, or automatic persistent-owner/task-admission activation in this change. Future journal APIs can consume this complete parser without importing a not-yet-installed scanner.
Exact-head verification
08a-transcript-parser-independent-base550afd5dpassed with the checkout clean before and after execution:3708c0a4a667f5aa87d7d1fe39f7f003cdb79c141af9cf920e939843cfaf9dbe, matching committed metadata and native source SHA-25626555fb71debdd40e0cdb70811847765452facd4cbb4107b646df7cc1a25dcaf; no trusted metadata rewrite.The initial malformed test fixture omitted the required locator schema version and failed four cases. That failure is retained separately; the fixture was corrected without weakening the decoder or assertions. Earlier historical-parent consumer qualification is not transferred to this head. Code-event CI and exact-head maintainer review remain separate requirements; the leader does not merge.
Preservation and installation boundary
The protected original 44-file staged candidate and reference #6240 at
7e5f7ef2a745bc1c3d012362f00be9a441bdeb76, source hashsha256:0ef698e426d34102d9be3e0c250a880059221bfa77c0ace2351bea28c97528ab, remain unchanged. No wholesale transplantation or baseline overwrite was used.The current-dev config-secret redaction change is retained. This inert internal API changes no public command, workflow definition, generated catalog, or runtime default. Activation guidance and user-visible release notes remain part of the separate, fully qualified activation layer.