Repository navigation
feat(session): authenticate readonly managed GC journal probes - #6316
Conversation
Disk dry runs and residual accounting need existing scope authority without initialization, lock creation, reaping or security repair. Bind exact roots and reuse immutable prepared history before exposing readonly discovery. Lore-id: 8240f16d Constraint: DTOs and canonical path absence never establish native retirement Confidence: high Scope-risk: narrow Reversibility: easy Tested: exact readonly API tree focused journal and owner-storage tests Tested: package vendor Biome TypeScript and fresh Bun absence discovery Not-tested: current-dev rebased cumulative consumers and activation
|
Current exact head Source repair is in progress on this PR branch, with nonmutating descriptor reads/identity/security validation and unchanged no-mutation assertions. No blind CI replay, snapshot-field exclusion, skip, timeout increase, native-result stub or gate weakening. Current Darwin focused qualification is not Linux readonly proof and does not make this head ready. Approval of #6314 does not transfer. |
probepark
left a comment
There was a problem hiding this comment.
Review (head 9efcbee, gajae-reviewer on behalf of probepark)
CI: PR-caused failure ×1 — Affected path validation / test:packages/coding-agent/test/managed-gc-retirement-journal.test.ts (job https://github.com/Yeachan-Heo/gajae-code/actions/runs/37188615722/job/111398496842) fails the new test resolves and reads owner journals without changing managed bytes, modes, ctimes, or private directories at managed-gc-retirement-journal.test.ts:456; evidence producer and the Affected path validation aggregate fail downstream. The test is new in this PR, so this is not base breakage. check:@gajae-code/coding-agent, ts-build, cli-smoke, and the other shards are green.
Scope: +386 / -2, 2 files — packages/coding-agent/src/session/internal/managed-session-scope.ts (+241/-2), packages/coding-agent/test/managed-gc-retirement-journal.test.ts (+145)
Conventions: CHANGELOG not needed (internal API, fragment landed in #6314), no generated files, no labels
Notable:
- Blocking: the readonly probe writes on Linux. The CI diff for the snapshot shows the readonly path creating
<scope>/.gjc-recovery/(dir, mode 0700) and<scope>/.gjc-recovery/.gjc-reaper-cursor(file), and the scope directory's ctime moving. The call chain at this head:resolveManagedGcScopeForReadsetsmanagedDirectoryAuthorities.set(scope, undefined)(managed-session-scope.ts, new hunk at L723+).managedGcScopeStore(managed-session-scope.ts:2267) then passes noretainedauthority toManagedSessionDescendantStore.- The constructor's non-retained branch (
managed-session-storage.ts:1561-1576) runsnativeSessionStorage().openRecoveryFsRoot(this.#baseDir)on Linux only. That native open initializes the recovery sidecar/reaper cursor in the scope directory. readManagedGcSessionRetirementReceiptReadOnlyanddiscoverManagedGcSessionRetirementReceiptsboth reach this path, so on Linux "readonly, never initializes storage" is false.- The PR body's qualification is "Darwin execution only", and this branch is gated on
process.platform === "linux", which is why the local run was green and CI is red. - Fix direction: make the readonly store skip
openRecoveryFsRoot(or use a native open that does not create.gjc-recovery/the cursor), then keep thesnapshotTreeassertion as the guard. Do not weaken the test.
- Note:
discoverManagedGcSessionRetirementReceiptsthrowsmanaged_gc_scope_authority_mismatchon the first malformedv2-*sibling, so one corrupt unrelated scope hides every other scope's residual receipts. If fail-closed for the whole root is intended, a one-line comment would make that explicit.
Blocking: 1 (readonly path mutates managed scope on Linux; the PR's own test fails in CI)
PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 needs-human sha256:aa2a42d175b872558652a15413db1a0c0caaa08e0bfa6c8186b7fba2fc4cec8e reviewer:critic reviewer-id:gajae-reviewer evidence:linux-readonly-probe-creates-gjc-recovery-reaper-cursor;new-test-fails-ci-job-111398496842
Verdict: gajae.pr-review-verdict.v1 needs-human sha256:aa2a42d175b872558652a15413db1a0c0caaa08e0bfa6c8186b7fba2fc4cec8e reviewer:critic reviewer-id:gajae-reviewer evidence:linux-readonly-probe-creates-gjc-recovery-reaper-cursor;new-test-fails-ci-job-111398496842
Linux retained-directory derivation and recovery-root reopening can create or reap recovery state. Journal discovery and live owner verification must use a path-backed readonly capability with the same identity/security checks, not a native recovery writer. Reject readonly mutations and retained authority exposure before effects. Lore-id: bd8036a1 Constraint: preserve full bytes/modes/ctimes snapshot assertions and strict native gates Constraint: no native redesign, unsafe unlink or snapshot-as-effect-authority Tested: readonly recovery-open/retain spies, continuation verification and mutation refusal; four focused suites; full coding-agent package check exit0 Not-tested: repaired Linux CI until fresh code event Confidence: high Scope-risk: bounded Reversibility: revert
|
Repair published at exact head |
|
Fresh pull_request Dev CI37191477305 SUCCESS at exact repaired head |
probepark
left a comment
There was a problem hiding this comment.
Review (head 47a83e6, gajae-reviewer on behalf of probepark)
CI: green. All planned checks pass at this head, including Affected path validation / test:packages/coding-agent/test/managed-gc-retirement-journal.test.ts (https://github.com/Yeachan-Heo/gajae-code/actions/runs/37191477305/job/111406309388), which failed at 9efcbee, plus check:@gajae-code/coding-agent, Virtual integration validation, and the gjc-state-gates shards. The approve gate returned ALLOW. Only the human-approval gate checks are left.
Scope: +523 / -10, 6 files: managed-session-scope.ts (+260), managed-session-storage.ts (+18), task-artifact-owner-access.ts (+15), task-artifact-owner-retirement.ts (+5), managed-gc-retirement-journal.test.ts (+232), and the changelog.d/task-owner-readonly-recovery.md fragment.
Conventions: CHANGELOG fragment present. No generated files, no released-section edits, no labels. Tests use no mock.module; both spyOn sites restore in finally.
Notable:
- The blocking item from our previous review (
9efcbee) is resolved. On Linux, the readonly probe calledopenRecoveryFsRootand created.gjc-recovery/and the reaper cursor. At this head,managed-session-storage.ts:1568opens the native recovery root only whenaccess === "read-write". The constructor rejects a read-only store that borrowsretainedauthority or has noexpectedSubtreeRoot(L1481-1484).#assertWritable()guards#beforeMutation(so everyensureDirectory/write path),retainAuthority, andmoveFileNoReplace.managedGcScopeReader(managed-session-scope.ts:2283) andnewSessionRootReaderStore(task-artifact-owner-access.ts) are the only constructors that use"read-only". Write paths keep the default"read-write". bindManagedGcSessionRetirementTargetnow uses the reader store. Its other callers (assertManagedGcTranscriptUnchangedOrAbsent) only callverifyRootSecurity/assertBound/readSnapshotSync, so moving the bind to the reader removes no mutation the publish path relied on. On the read-only path,captureDirectoryIdentityandreadExpecteduse the path-backed chain check (L1684, L2778).- The regression test makes
native.openRecoveryFsRootandretainManagedDirectorythrow, and it keeps the full bytes/modes/ctimessnapshotTreeequality. It is not weakened. - Non-blocking (carried from the last review):
discoverManagedGcSessionRetirementReceiptsthrowsmanaged_gc_scope_authority_mismatchon the first malformedv2-*sibling (managed-session-scope.ts, discovery loop). One corrupt scope therefore hides every other scope's residual receipts. If fail-closed for the whole root is intended, add a one-line comment.
Blocking: none.
Body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:bd2339852cf050442613d0f30f2259dee42a777d66636b65297371cff82b91f0 reviewer:human reviewer-id:probepark evidence:ci-green-run-37191477305;readonly-store-skips-openRecoveryFsRoot;assertWritable-guards-mutation;snapshotTree-regression-kept
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:bd2339852cf050442613d0f30f2259dee42a777d66636b65297371cff82b91f0 reviewer:human reviewer-id:probepark evidence:ci-green-run-37191477305;readonly-store-skips-openRecoveryFsRoot;assertWritable-guards-mutation;snapshotTree-regression-kept
Scope
Split #6240 after external journal merge #6314. Base
6ea157e6f014dc160d5a86bcee1d4f58f3fdbe78; exact head47a83e64b2321329d7036d9bae8d969eed8f6105. Actual production review cost 298 additions + deletions: Scope260, retained store18, owner reader factory15 and live retirement verification5. Tests232 and release fragment3 excluded. Complete independently callable readonly scope resolver, retirement reader and residual-journal discovery; no placeholders or destructive consumers.Unlike the existing lease-bound journal reader, these probes do not initialize directories, take/reap leases, repair bindings or mutate metadata. They authenticate configured-root/profile/session/scope identities, strict journal history and live physical completion; residual receipts remain discoverable after canonical transcript absence. Resolving metadata is not native effect authority. Automatic owner production and post-move task admission remain OFF.
Linux CI defect and source repair
Prior head
9efcbee07902040c6dc436b65491aa7cbfc73206failed Dev CI37188615722's Linux readonly snapshot assertion: recovery directory/cursor creation and scope ctime change. This was a real changed-code defect, not infrastructure; no blind replay or waived assertion. Native retained-directory derivation initializes/reaps recovery even when called for identity data, and opening an existing native recovery root can reap it.The repaired head uses an explicit path-backed read-only store requiring an existing exact subtree identity; it cannot borrow/open native recovery authority or mutate/retain authority. Target binding, readonly journal/discovery and physical/continuation live verification use that capability. Default write paths and native effect authority remain unchanged. New regressions spy on real mutating native open/retain boundaries while retaining the complete bytes/modes/ctimes/private-tree assertion. No native API invention, snapshot-field exclusion, compatibility fallback, skip or timeout increase. A release fragment documents the actual regression repair.
Exact qualification
Separate checkout clean before and after at the exact head:
bun test packages/coding-agent/test/managed-gc-retirement-journal.test.ts packages/coding-agent/test/managed-gc-retirement-codec.test.ts packages/coding-agent/test/task-artifact-owner-transcript.test.ts packages/coding-agent/test/session-storage.test.ts packages/coding-agent/test/gc-disk-retention.test.ts packages/coding-agent/test/task-artifact-owner-retirement.test.ts packages/coding-agent/test/task-artifact-owner-access.test.ts: exit0, 208 passed / 8 existing platform skips / 0 failed / 1092 assertions.bun --cwd=packages/coding-agent run check: exit0 on47a83e64b2321329d7036d9bae8d969eed8f6105. Full vendor/Biome/typecheck, includingtsc -p tsconfig.json --noEmit. Baseline2 warnings/1 info retained.Genuine current-source native0.18.6 addon SHA256
ecca056f0c63eca5139c674999a86ca8f4c32926cb18b4c9804efd0d00c32030, source SHA25626555fb71debdd40e0cdb70811847765452facd4cbb4107b646df7cc1a25dcaf. Successful ci-profile build atb1fbdafa94f0ad68825d2d46590ba4ad81e013e0; native crates tree identical at qualification head. Build-generated descriptor already externally installed through #6314; no metadata rewrite during qualification, historical/released artifact substitution, digest/version/source waiver, timeout increase or new skip.Darwin execution only; this is not cumulative SDK/GC/task-admission/platform qualification. The separate upstream over-limit reaper performance scenario remains outside these focused green claims: full 50,001-file candidate locally passed but failed exact QA's unchanged20s timeout; no candidate delivery or waiver claimed.
Delivery boundaries
Internal readonly APIs and the concrete readonly regression repair; no CLI/UI/producer activation. Existing journal release fragment landed in #6314, and this PR supplies its own readonly-recovery fix fragment. All native-effect/owner-producing consumers remain separately gated. The distinction between mutating lease-bound and readonly readers is explicitly documented in source/API names and above.
Protected original HEAD
1a12e32f6bbcb5c4a469dd480b4b5dacb37dc95d/ 44 staged files and protected reference HEAD7e5f7ef2a745bc1c3d012362f00be9a441bdeb76/ sourceHashsha256:0ef698e426d34102d9be3e0c250a880059221bfa77c0ace2351bea28c97528abpreserved.Fresh exact-head code-event CI and approving write-access maintainer review required; #6314 approval does not transfer. Leader does not merge. No final cohort/critic/whole-goal completion claim.