Skip to content

feat(storage): add durable file SessionRepository - #4674

Open
MicroGery wants to merge 3 commits into
codex/2370-session-repository-contractfrom
codex/2370-local-durable-adapter
Open

feat(storage): add durable file SessionRepository#4674
MicroGery wants to merge 3 commits into
codex/2370-session-repository-contractfrom
codex/2370-local-durable-adapter

Conversation

@MicroGery

@MicroGery MicroGery commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements phase 2 of #2370 on top of #4662: an internal durable local adapter for the established Session checkpoint contract. It is intentionally not a public package entrypoint or a production composition consumer.

Immutable Bundle and Manifest bytes live in non-overwritable local files verified by digest. Session heads, CAS revisions, commit receipts, and Fork records live in a small atomically replaced local control-plane document.

Control-plane mutations take a cross-process atomic-mkdir lock. The document fsyncs a private temporary file before atomic rename and syncs its containing directory. Immutable publication streams file-backed objects into a fsynced private temporary file, links without overwrite, then always syncs the directory chain to the storage root, including an EEXIST winner race. Object verification and bounded materialization also stream data instead of buffering a Bundle in memory.

Semantics covered

  • Current and exact checkout survive adapter reopen.
  • Concurrent adapters serialize head CAS, so one stale writer receives revision_conflict.
  • State decode fails closed unless Session revision allocation, immutable lineage, and retained commit receipts describe a non-contradictory history.
  • Existing-Session and stale-CAS preflight errors win before an unrelated candidate object is read; state is rechecked under the writer lock.
  • Pending Fork claims retain the admitted source checkpoint across source-head advance and adapter reopen. The inherited bounded materialization port can recover the exact retained Bundle bytes for the later Fork hydration or repack owner.
  • Altered or missing immutable bytes and corrupt control-plane JSON fail closed.

This is a stacked PR: its base is the #4662 branch. It must merge after #4662 is merged or be retargeted and rebased then.

Non-goals

  • Remote or cloud metadata and object-store adapter.
  • A public package export or production composition consumer.
  • Automatic recovery or stealing of a stale local writer lock.
  • Activation and Fork orchestration, target state re-keying, or Bundle hydration.

Refs #2370

Verification

  • Storage typecheck; Core and Storage builds; Biome check
  • Focused file SessionRepository suite: 8 passed
  • Focused in-memory SessionRepository suite: 18 passed
  • Full compiled Storage suite: 1,124 passed, 8 skipped

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the local durable adapter, its tests, and this PR description under human contributor ownership.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and affected suites pass locally

Does this PR entail a change in behavior?

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head 3b086e70a3b5307de0a70e5e565cf02ebebc0370 (OPEN, MERGEABLE, no hosted check-runs yet). One P1 and one P2 below; the diff is 2 files (+1215: 1004 implementation, 211 tests).

P1 — the new adapter has no package surface and no production consumer, if it claims to deliver a usable workspace adapter

packages/storage/src/file-session-repository.ts:72-80 exports openFileSessionRepository, but packages/storage/package.json:7-62 has no ./file-session-repository export. Importing it from the built package entry reproducibly yields ERR_PACKAGE_PATH_NOT_EXPORTED, and a repo-wide search finds only the new tests importing via relative path — no production caller. So the 1004-line implementation is currently unreachable to @maka/storage consumers; the feature is effectively test-only. If this is intentionally an intermediate foundation with no consumer yet, please state that staging boundary in the PR; otherwise add the export, a real caller, and integration tests before merging.

P2 — first creation of an object-prefix directory does not durably publish the full directory chain

file-session-repository.ts:290-303 creates objects/<2-char-prefix> with recursive mkdir, writes the temp file with sync(), then after linking only calls syncDirectory(dirname(destination)). But session-repository.ts:77-85 requires publish to return only once the exact bytes/metadata are durably readable. When the prefix (or objects) is newly created in this call, fsyncing just the leaf directory does not persist the parent directory entries; after a crash the whole new prefix can be lost, violating that contract. The existing stable-storage.ts:102-123 already provides syncDirectoryChain with a root boundary, and other publish paths in this repo use it after new-directory creation. Please sync to the storage root (or an equivalent full boundary) and add a crash/persistence test for first-time new-prefix creation.

What was checked on this head

@maka/core and @maka/storage builds pass; compiled file-session tests 4/4 and existing session-repository tests 18/18 pass; git diff --check clean. What I could not judge: real power-loss directory-entry durability is not proven by local tests, and there are no hosted checks on this head yet.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

本条结论全部来自 @未开智选手 的审查。我自己没有读这份 diff;我核的是当前 head 有没有漂移、以及 exact-head 的 CI 状态。当前 head 是 3b086e7,可合并未关闭,暂无线上检查。P1 是新增实现没有包导出也没有生产调用方,P2 是首次建目录链时持久化发布不同步完整链。修好或明确阶段边界后再审。

@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Sep 3, 2026
@MicroGery
MicroGery force-pushed the codex/2370-local-durable-adapter branch from 3b086e7 to 47c5c50 Compare September 4, 2026 02:55

@likun666661 likun666661 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 47c5c509d2f56430b71bee7f6cdb4e18ed81663a and the stacked interaction with #4662. Focused tests pass on the exact head and on a synthetic merge with current main; the full Storage suite passes with Node experimental warnings suppressed. Two P1 correctness/durability gaps and two P2 conformance/operability gaps are inline. No hosted checks are currently reported for this head.

Comment thread packages/storage/src/file-session-repository.ts
Comment thread packages/storage/src/file-session-repository.ts Outdated
Comment thread packages/storage/src/file-session-repository.ts
Comment thread packages/storage/src/file-session-repository.ts Outdated
@MicroGery
MicroGery force-pushed the codex/2370-local-durable-adapter branch from 47c5c50 to 913164e Compare September 4, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants