Repository navigation
feat(packs): durable pack document store (T1) - #435
johnnyelwailer merged 3 commits into
Conversation
…istribution persistence test The isolation test only asserted event count, so a leaked pack-B upsert would have passed. The distribution persistence test lived under apps/server/scripts, which vitest does not include, so it never ran; it now lives in src/. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Thread transfer impact
This comment will update automatically after the next completed run. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d48cba331
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (existing !== undefined) { | ||
| if (stableStringify(existing) === stableStringify(definition)) continue; | ||
| throw new Error(`Duplicate pack persistence: ${id}`); |
There was a problem hiding this comment.
Allow runtime persistence to override compiled definitions
When a packaged distribution registers a pack and T3TEAM_PACKS_DIR resolves a higher-precedence version of that same pack with changed collection metadata, this duplicate check throws. runT3TeamServerCommand catches that error for the entire runtime registration map, so the updated pack keeps the stale compiled definition and unrelated runtime packs lose their stores as well. Preserve the normal compiled-baseline/runtime-override behavior instead of treating this expected cross-source replacement as a duplicate.
AGENTS.md reference: AGENTS.md:L51-L56
Useful? React with 👍 / 👎.
| if ( | ||
| options.ttlMs !== undefined && | ||
| (!Number.isSafeInteger(options.ttlMs) || options.ttlMs < 1) | ||
| ) |
There was a problem hiding this comment.
Reject TTLs outside the representable date range
A TTL such as Number.MAX_SAFE_INTEGER passes this validation, but adding it to the current epoch exceeds the ECMAScript date range, so the subsequent DateTime.formatIso defects instead of returning the store's declared typed error. A trusted pack can therefore terminate its operation fiber simply by supplying a large otherwise-valid integer; validate that the computed expiration is representable before formatting it.
Useful? React with 👍 / 👎.
…ition (#442) * test(packs): a runtime pack's persistence must override the compiled definition Fails today with "Duplicate pack persistence" (#435 review, Codex P1). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(packs): a runtime pack's persistence overrides its compiled definition Compiled-in and runtime persistence shared one registry that treated any differing definition for the same pack id as a duplicate. When T3TEAM_PACKS_DIR held a newer version of a compiled pack, runtime registration threw, the boot catch dropped the whole runtime map, and every runtime pack lost its store. Each source now registers into its own layer (conflicts within one source still fail atomically) and the effective map is the compiled baseline overlaid by the runtime layer, matching how the other compiled-in content is overridden. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Phil J <philip.jonientz@nexplore.ch> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Packs get a durable, keyed document store (seam S3, slice T1): a pack can insert-or-get, compare-and-set, increment and list its own documents, and a client can subscribe to them live. Pack A can never see pack B's documents.
What it adds
105 PackDocuments(t3team_pack_documents; highest prior id was 104).T3TeamPackDocumentStore.forPack(packId)→get / list / insertOrGet / put(ifVersion, ttlMs) / increment; every SQL statement is bound to the pack id in the closure. Writes are serialized and run in a transaction, then published.t3team.subscribePackDocuments { packId, collection, key? | prefix? }→snapshot | upsert | removed; subscribes to the PubSub before reading the snapshot. Read scope; unregistered pack ids are refused.capabilities.t3team.packStore.state/pack-documents), gated on the capability.contents.persistence+store:v1read at boot (runtime packs and compiled distributions);defineCollections/mergePackCollectionsDefinitionsin@t3team/pack-api.Tests
insert-or-get race (one winner), CAS conflicts, subscribe-then-snapshot buffering, pack isolation (reads, lists, subscriptions), counters/pagination/validation, manifest loading, client reducer, migration ledger.
Not in T1 (per plan row T1)
Retention and quota eviction,
remove/touch,t3team.packStore.putfor views, script-context store (S4).expires_atis stored but reads do not filter on it yet; that lands with retention.Reviewed by Cursor and Copilot (Codex was out of budget); no code findings beyond the items above.
🤖 Generated with Claude Code