fix(store): resolve four packages/store bugs (#499, #500, #501, #502) - #522
Draft
TYRMars wants to merge 1 commit into
Draft
fix(store): resolve four packages/store bugs (#499, #500, #501, #502)#522TYRMars wants to merge 1 commit into
TYRMars wants to merge 1 commit into
Conversation
…rite tmp, WS metadata round-trip Resolves four packages/store code-review bugs (#499, #500, #501, #502): - #502 atomicWrite shared a fixed `<path>.tmp` across all writers of a row, so two concurrent writes to one id deterministically ENOENT on rename and the loser's turn was silently dropped. Stage to a per-write unique name (`<path>.<pid>.<uuid>.tmp`) and clean up on failure → last-writer-wins. - #501 JsonFileConversationStore.list blind-cast every `.json` in the base dir and dereferenced `stored.messages.length` with no shape guard, so one foreign file (e.g. workspaces.json flushed into the same dir) permanently 500'd GET /v1/conversations. Skip records whose `messages` isn't an array and wrap each per-file read in try/catch. - #499 encodeId leaves `.`/`..` intact, so a project_id of `..` joined as a directory component escaped the requirements partition into the sibling conversation dir. Add encodeDirComponent (rejects `.`/`..`/empty) and use it in every directory-partition helper (requirement / comment / activity / requirement-run stores). - #500 the WS chat path saved via ConversationStoreBase.save, which hard-codes defaultMetadata(), so every completed turn erased project_id and un-archived the conversation. Round-trip ConversationMetadata through loadEnvelope on resume and saveEnvelope on done, keeping it in sync on new/reset. Adds tests for the three store-level fixes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ehdr6rdrz54r9gWN6oJ7P
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-resolves four code-review bug reports in
packages/store, all in the JSON-file backend and the WS chat save path.#502 —
atomicWriteshared.tmp→ concurrent writes to one id ENOENTThe staging file was derived solely from the target path, so all concurrent writers of a row shared one
.tmp; whoever renamed first consumed it and the secondrenamerejected withENOENT, silently dropping a turn (both message routes catch-and-ignore save failures). Now each write stages to a unique<path>.<pid>.<uuid>.tmpand cleans it up on failure → last-writer-wins.#501 —
list()no shape guard → one foreign.json500sGET /v1/conversationsJsonFileConversationStore.listblind-cast every parsed.jsonand dereferencedstored.messages.length. A well-formed but foreign file (e.g.workspaces.jsonflushed into the same dir, or a..-escaped requirement row) threw aTypeErrorthat killed the whole endpoint. Now records without an arraymessagesare skipped and each per-file read is wrapped intry/catch, matchingactivity-store/observability.#499 —
encodeIdleaves..intact → partition escapeencodeId("..") === "..", so aproject_idof..joined as a directory component escaped the requirements partition into the sibling conversation dir (corrupting it and permanently breaking the conversation list). AddedencodeDirComponent(rejects./../empty viaStoreError) and applied it to every directory-partition helper: requirement, comment, activity, and requirement-run stores.#500 — WS chat save clobbers conversation metadata
The WS turn-completion handler saved via
ConversationStoreBase.save, which hard-codesdefaultMetadata(), so every completed turn erasedproject_idand resetlifecycletoactive— permanently un-archiving requirement-run conversations and dropping their project link. The handler now round-tripsConversationMetadata: loaded vialoadEnvelopeonresume, kept in sync onnew/reset, and persisted viasaveEnvelopeondone.Tests
store.test.ts:listskips a foreign JSON file; concurrent same-id saves don't ENOENT and leave no.tmplitter;encodeDirComponentrejects escaping ids.requirement-store.test.ts: JSON-file store refuses a partition-escapingproject_id.Verification
pnpm -r typecheckclean across the workspace.packages/store+packages/servertests pass except pre-existing environmental failures (unbuildablebetter-sqlite3/node-ptynative modules — same failures on the clean base, unrelated to this change).🤖 Generated with Claude Code
https://claude.ai/code/session_013ehdr6rdrz54r9gWN6oJ7P
Generated by Claude Code