chore: Phase 3 review cleanup items #47

New Issue

2026-06-02T09:05:51Z

xiaoju commented

2026-06-02 09:05:51 +00:00

From PR #46 review

Warnings discovered during Phase 3 code review. Can be addressed during Phase 4/5 or independently.

Items

validateName() duplication — identical implementations in packages/core/src/store.ts (MemoryVarStore) and packages/fs/src/var-store.ts (FsVarStore). Extract to a shared module (e.g. errors.ts or validation.ts in core, importable by fs).
Memory/FS VarStore logic duplication — ~80% shared logic (history, conflict checking, set/get/remove/update/list/history). Consider extracting shared logic or a base implementation.
CLI prompts doc stale — packages/cli/src/prompts/usage.md still references old openStoreAndVarStore() pattern. Update to reflect OcasStore API.
Legacy Store type cleanup — old Store type in types.ts (with put returning Hash | Promise<Hash>) still exported. Deprecate or remove once all consumers migrated.
Unnecessary async — bootstrap() and putSchema() are async but CasStore.put() is synchronous. Consider making sync.

Timing

Phase 4 (#42 FsStore) will naturally touch VarStore code — good opportunity for the duplication items. The rest can be done anytime.

小橘 🍊（NEKO Team）

## From PR #46 review Warnings discovered during Phase 3 code review. Can be addressed during Phase 4/5 or independently. ### Items - [ ] **`validateName()` duplication** — identical implementations in `packages/core/src/store.ts` (MemoryVarStore) and `packages/fs/src/var-store.ts` (FsVarStore). Extract to a shared module (e.g. `errors.ts` or `validation.ts` in core, importable by fs). - [ ] **Memory/FS VarStore logic duplication** — ~80% shared logic (history, conflict checking, set/get/remove/update/list/history). Consider extracting shared logic or a base implementation. - [ ] **CLI prompts doc stale** — `packages/cli/src/prompts/usage.md` still references old `openStoreAndVarStore()` pattern. Update to reflect `OcasStore` API. - [ ] **Legacy `Store` type cleanup** — old `Store` type in `types.ts` (with `put` returning `Hash | Promise<Hash>`) still exported. Deprecate or remove once all consumers migrated. - [ ] **Unnecessary async** — `bootstrap()` and `putSchema()` are async but `CasStore.put()` is synchronous. Consider making sync. ## Timing Phase 4 (#42 FsStore) will naturally touch VarStore code — good opportunity for the duplication items. The rest can be done anytime. -------- 小橘 🍊（NEKO Team）

xiaoju referenced this issue from a commit

2026-06-02 09:27:53 +00:00

feat(fs): add FsStore var/tag test coverage and share validateName

xiaoju referenced this issue

2026-06-02 09:34:46 +00:00

fix(fs): add FsStore var/tag test coverage and share validateName #48

xiaoju referenced this issue

2026-06-02 09:37:53 +00:00

refactor: unified Store type — CasStore + VarStore + TagStore #38

xiaoju referenced this issue from a commit

2026-06-02 10:08:52 +00:00

fix(core,fs): phase 3 cleanup — drop legacy Store, sync bootstrap, dedup VarStore