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 是首次建目录链时持久化发布不同步完整链。修好或明确阶段边界后再审。
3b086e7 to
47c5c50
Compare
likun666661
left a comment
There was a problem hiding this comment.
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.
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
47c5c50 to
913164e
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 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
EEXISTwinner race. Object verification and bounded materialization also stream data instead of buffering a Bundle in memory.Semantics covered
revision_conflict.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?