feat(storage): add durable file SessionRepository - #4674
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
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 是首次建目录链时持久化发布不同步完整链。修好或明确阶段边界后再审。
Generated-by: Codex
Generated-by: Codex
3b086e7 to
47c5c50
Compare
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 yet a public package entrypoint or a production composition consumer.
The adapter keeps immutable Bundle and Manifest bytes 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.
All control-plane state mutations take a cross-process atomic-mkdir lock. The control-plane document fsyncs a private temporary file before atomic rename and syncs its containing directory. Immutable objects are independently published through a fsynced temporary file, no-replace link, and directory-chain sync to the storage root. A crash may leave a lock directory, in which case writers fail closed instead of guessing that it is safe to steal; reads continue from the last complete state document.
Semantics covered
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
Refs #2370
Verification
AI use
Tool(s) and scope: Codex implemented the local durable adapter, its tests, and this PR description under human contributor ownership.
Checklist
Does this PR entail a change in behavior?