feat(storage): add composable shared storage backends#217
Conversation
Support filesystem, SQLite, PostgreSQL, and S3-backed writable authority state alongside immutable staged Caplets. Add replica-safe runtime activation, Current Host administration, migrations and recovery, dashboard and CLI parity, provider packaging, operator documentation, and deterministic cross-provider verification.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces composable shared storage with filesystem, SQLite, PostgreSQL, and S3-compatible authorities; generation-aware runtime coordination; authority-backed dashboard, CLI, vault, OAuth, session, and setup state; migration and backup workflows; and deterministic/provider-specific validation. ChangesShared storage and runtime
Estimated code review effort: 5 (Critical) | ~180 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Preview DeployedLanding: https://pr-217.preview.caplets.dev Built from commit a7b0078 |
|
| Filename | Overview |
|---|---|
| packages/core/src/storage/s3-authority.ts | S3 provider with sharded receipt/session indexes, immutable candidate objects, and ETag-based CAS for head. The encodeKeyPart function is non-injective, creating a latent collision for inputs containing literal _XX sequences vs. characters that percent-encode to %XX. |
| packages/core/src/storage/backup.ts | AES-256-GCM backup with authenticated header, provider/namespace/generation identity cross-checks, and key fingerprint guard. Format is correct and self-consistent. |
| packages/core/src/storage/migration.ts | Fenced, empty-target migration with source race recheck, staged publish, and post-publish verification. Fence release is correctly guarded by finally blocks at both source and destination levels. |
| packages/core/src/dashboard/session-store.ts | Dual session store implementations: filesystem (directory-lock mutex) and authority-backed (encrypted CSRF token, touch-throttled auxiliary commits). The assertAuthorityOperator helper silently passes when the snapshot has no remoteCredentials. |
| packages/core/src/dashboard/activity-log.ts | Dual activity log implementations: JSONL append-file (no cross-process lock) and authority-backed. The filesystem class has a multi-process truncation race near MAX_ACTIVITY_ENTRIES, but is only used in single-node deployments. |
| packages/core/src/storage/composition.ts | Clean precedence composition: staged filesystem sources then authority records, with staged/authority ID collision detection and SHA-256 fingerprinting of staged paths. |
| packages/core/src/remote/authority-codec.ts | Domain codec encrypts commit results with AES-GCM via the vault key; decrypts replayed results from stored receipts. Timing-safe equality for hash comparison. |
| packages/core/src/storage/coordinator.ts | Runtime coordinator with reference-counted PreparedRuntimeView, polling-based authority refresh, epoch-lease tracking, and graceful shutdown. |
| packages/core/src/storage/factory.ts | Provider registry with deduplication guard and lifecycle identity assertion. Clean separation between provider registration and authority construction. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant C as Dashboard Client
participant H as HTTP Handler
participant RS as RemoteCredentialService
participant SS as AuthorityDashboardSessionStore
participant CDC as AuthorityDomainCodec
participant A as WritableAuthority
C->>H: POST /dashboard/login/complete
H->>RS: completePendingLogin(requiredRole: operator)
RS-->>H: credentials clientId
H->>SS: create(operatorClientId)
SS->>CDC: read()
CDC->>A: readHead plus readGeneration
A-->>CDC: generation snapshot
CDC-->>SS: head and snapshot
SS->>SS: assertAuthorityOperator pruneExpiredSessions
SS->>CDC: commit snapshot plus session
CDC->>A: commit SemanticCommandEnvelope
Note over A: CAS on expectedGeneration
A-->>CDC: AuthorityCommitResult
CDC-->>SS: kind generation result
SS->>A: commitAuxiliary session_touch
SS-->>H: cookieValue session
H-->>C: Set-Cookie plus JSON session
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant C as Dashboard Client
participant H as HTTP Handler
participant RS as RemoteCredentialService
participant SS as AuthorityDashboardSessionStore
participant CDC as AuthorityDomainCodec
participant A as WritableAuthority
C->>H: POST /dashboard/login/complete
H->>RS: completePendingLogin(requiredRole: operator)
RS-->>H: credentials clientId
H->>SS: create(operatorClientId)
SS->>CDC: read()
CDC->>A: readHead plus readGeneration
A-->>CDC: generation snapshot
CDC-->>SS: head and snapshot
SS->>SS: assertAuthorityOperator pruneExpiredSessions
SS->>CDC: commit snapshot plus session
CDC->>A: commit SemanticCommandEnvelope
Note over A: CAS on expectedGeneration
A-->>CDC: AuthorityCommitResult
CDC-->>SS: kind generation result
SS->>A: commitAuxiliary session_touch
SS-->>H: cookieValue session
H-->>C: Set-Cookie plus JSON session
Reviews (4): Last reviewed commit: "refactor(storage): expose provider-shape..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| type MaintenanceFenceLease, | ||
| } from "./migration"; | ||
|
|
||
| const BACKUP_MAGIC = Buffer.from("CAPLETS-AUTHORITY-BACKUP\\0", "utf8"); |
There was a problem hiding this comment.
BACKUP_MAGIC constant uses a double-escaped \\0, which in JavaScript source produces a literal backslash character (\, 0x5C) followed by the digit 0 (0x30) — not a null terminator byte. Because both writer and reader share the same constant the format is self-consistent today, but the magic is 26 bytes ending in \0 (the two-character sequence) rather than the intended 25 bytes ending in a NUL byte. Any future correction of this escape to "\0" would silently invalidate every backup written by this version, since the magic check would fail on the first byte of the suffix.
| const BACKUP_MAGIC = Buffer.from("CAPLETS-AUTHORITY-BACKUP\\0", "utf8"); | |
| const BACKUP_MAGIC = Buffer.from("CAPLETS-AUTHORITY-BACKUP\0", "utf8"); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/serve/http.ts (1)
1527-1548: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep the runtime lease alive for attachEvents SSE responses. The handler returns a long-lived
text/event-stream, but the request lease is released as soon as the route handler resolves.attachEventsResponse()continues usingsource.manifestRevision()andengine.onReload()after that, so a runtime reload can retire the view while the stream is still open. Hold the lease until the SSE stream closes, like the MCP/attach-session paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/serve/http.ts` around lines 1527 - 1548, The attachEvents route must retain its runtime lease for the entire SSE lifetime instead of releasing it when the handler resolves. Update the handler around attachEventsResponse and requestEngine to acquire or reuse the lease and release it from the stream’s close/cancel cleanup, preserving the existing error response path and ensuring source.manifestRevision() and engine.onReload() remain valid until the stream closes.packages/core/src/serve/session.ts (1)
153-164: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winLease can leak if pre-close cleanup throws.
unsubscribeReload(),codeModeSessions.close(), andclearRegistrations()run outside thetry/finally. If any of them throws,this.closedis alreadytrue, so a laterclose()call is a no-op andruntimeLease.release()never runs — permanently pinning the retainedPreparedRuntimeViewopen.CapletsEngine.close()already uses try/finally for this exact reason; mirror it here.🔒️ Guard all cleanup so the lease is always released
async close(): Promise<void> { if (this.closed) return; this.closed = true; - this.unsubscribeReload(); - this.codeModeSessions.close(); - this.clearRegistrations(); try { + this.unsubscribeReload(); + this.codeModeSessions.close(); + this.clearRegistrations(); await this.server.close(); } finally { this.runtimeLease?.release(); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/serve/session.ts` around lines 153 - 164, Update Session.close() so unsubscribeReload(), codeModeSessions.close(), clearRegistrations(), and server.close() all execute within the try/finally that releases runtimeLease. Preserve the existing closed guard and cleanup order while ensuring runtimeLease?.release() runs even when any pre-close cleanup throws.
🧹 Nitpick comments (6)
packages/core/README.md (1)
138-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDangling intro line under "Configuration and source ownership".
Line 138 ("The bootstrap provider union is:") ends with a colon but no union body follows before line 140 restates the intro. Reads like an editing artifact — either drop line 138 or fold it into the field list on lines 142–146.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/README.md` around lines 138 - 146, The “The bootstrap provider union is:” line is a dangling introduction in the “Configuration and source ownership” section. Remove it or merge its wording into the following provider-specific field list, ensuring the section has only one clear introduction before the filesystem, sqlite, postgresql, and s3 entries.packages/core/src/storage/sql/migrations/postgres/0000_initial.sql (1)
61-63: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant index duplicates the primary key.
authority_eventsalready hasPRIMARY KEY (authority_id, watermark), which creates a btree on exactly(authority_id, watermark). The extraauthority_events_after_idxon the same columns/order is fully redundant and only adds write overhead on a table this PR scales to thousands of events. Since this is a new, checksummed migration, dropping it now is safe before release.♻️ Proposed change
-CREATE INDEX IF NOT EXISTS authority_events_after_idx ON authority_events (authority_id, watermark);The same redundant index also appears in
packages/core/src/storage/sql/migrate.ts(POSTGRES_INITIAL_SQLandSQLITE_INITIAL_SQL); keep all copies in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/storage/sql/migrations/postgres/0000_initial.sql` around lines 61 - 63, Remove the redundant authority_events_after_idx creation from the PostgreSQL migration and from both POSTGRES_INITIAL_SQL and SQLITE_INITIAL_SQL in migrate.ts, while preserving the composite primary key on (authority_id, watermark). Keep all initial schema definitions synchronized.packages/core/src/storage/filesystem-authority.ts (1)
1280-1282: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse a deterministic receipt comparator
stableJsonStringify(...).localeCompare(...)is locale-sensitive, so receipt order can differ across runtimes.authorityExportDigesthashes the full export object, so that order change also changes the digest and can make cross-machine restore/migration comparisons disagree. Switch both receipt sort sites to a bytewise/codepoint comparison.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/storage/filesystem-authority.ts` around lines 1280 - 1282, Update both receipt sort sites to use a locale-independent bytewise/codepoint comparator instead of localeCompare when comparing stableJsonStringify results. Preserve the existing receipt ordering input to authorityExportDigest so export digests remain deterministic across runtimes.packages/core/src/storage/sql/migrations/sqlite/0000_initial.sql (1)
54-62: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRedundant index duplicates the primary key.
authority_eventsalready hasPRIMARY KEY (authority_id, watermark), which SQLite backs with an index on exactly those columns in that order.authority_events_after_idxon(authority_id, watermark)adds no lookup benefit and only costs write/storage overhead. Consider dropping it, or if the intent was range scans keyed differently, adjust the column set (e.g. add a covering/ordering column).♻️ Drop the redundant index
-CREATE INDEX IF NOT EXISTS authority_events_after_idx ON authority_events (authority_id, watermark);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/storage/sql/migrations/sqlite/0000_initial.sql` around lines 54 - 62, Remove the redundant authority_events_after_idx definition from the authority_events migration, since the composite PRIMARY KEY on (authority_id, watermark) already provides the same index ordering. Keep the authority_events table and primary-key constraint unchanged.packages/core/src/storage/migration.ts (1)
1033-1033: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant no-op ternary.
Both branches evaluate to
recordId, so the condition has no effect. Thecaplets/recordscase is already handled by the dedicated branch just below (whichcontinues), so this only ever runs for other keys where the value isrecordIdregardless. Simplify to avoid the misleading appearance of branch-specific behavior.♻️ Suggested simplification
- const nestedRecordId = key === "caplets" || key === "records" ? recordId : recordId; + const nestedRecordId = recordId;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/storage/migration.ts` at line 1033, In the migration logic, replace the redundant conditional assignment to nestedRecordId with a direct recordId assignment. The dedicated caplets/records branch and its continue behavior should remain unchanged.packages/core/src/storage/s3-authority.ts (1)
3138-3143: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused watermark helpers Remove the unused watermark helpers
nextWatermark()andlatestAuxiliaryWatermark().parseEvent()andeventsPrefix()only supportlatestAuxiliaryWatermark(), so keep them if that helper stays.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/storage/s3-authority.ts` around lines 3138 - 3143, Remove the unused nextWatermark() and latestAuxiliaryWatermark() helpers, along with any now-unused supporting logic. Since parseEvent() and eventsPrefix() exist only for latestAuxiliaryWatermark(), remove those methods as well unless latestAuxiliaryWatermark() is retained.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 58-59: Update both actions/checkout steps in the CI workflow to
set persist-credentials to false, ensuring the GITHUB_TOKEN is not retained in
the checked-out repository for subsequent steps.
In `@apps/dashboard/src/components/DashboardApp.tsx`:
- Around line 2110-2124: The delete flow around dashboardMutationStatus must
only perform success side effects when phase is "active". Keep the pending
branch polling as-is, but handle degraded without refreshing, clearing the
editor, or showing the deletion-success toast; rely on the existing
MutationStatusNotice state instead.
In `@docs/product/self-hosting.md`:
- Around line 23-25: The MinIO image reference in the s3 self-hosting
documentation uses an invalid 63-character digest. Re-derive the correct SHA-256
digest for the pinned MinIO image and replace the value in the deterministic
local fixture description, preserving the digest-pinned image format.
In `@packages/core/src/dashboard/session-store.ts`:
- Around line 440-449: Update the session validation flow around
codec.commitAuxiliary and the touched result so only “missing” and “revoked”
throw AUTH_FAILED. Handle “conflict” as a valid session outcome by returning the
session with persistedLastUsedAt, or skip the touch while preserving the
existing session response behavior.
- Around line 665-684: Update assertAuthorityOperator so a missing, non-object,
or array-valued remoteCredentials immediately throws the existing AUTH_FAILED
CapletsError instead of returning. Preserve the current client lookup and
operator/revocation checks for valid remoteCredentials objects, ensuring create
and validate fail closed when the authority domain is absent.
In `@packages/core/src/engine.ts`:
- Around line 775-794: Update assertSynchronousAuthority to catch and ignore
only CapletsError instances with code CONFIG_NOT_FOUND; allow
malformed-bootstrap and other CapletsError failures from loadAuthorityBootstrap
to propagate, while preserving the existing filesystem-provider check and
ASYNC_AUTHORITY_REQUIRED behavior.
In `@packages/core/src/remote-control/client.ts`:
- Around line 186-195: Update currentHostMutationArguments to normalize an
explicit null expectedGeneration to the unset representation before constructing
the mutation arguments, so dispatch.ts never receives null while undefined
behavior remains unchanged. Alternatively, if null is not a supported semantic,
remove it from RemoteCurrentHostMutationOptions and preserve the existing
non-null forwarding behavior.
In `@packages/core/src/remote-control/dispatch.ts`:
- Around line 792-814: Update optionalGeneration to treat a null argument value
the same as undefined and return undefined, while preserving validation and
identity parsing for non-null values. Ensure nullable expectedGeneration inputs
remain valid through the remote-control request path without changing unrelated
mutation behavior.
In `@packages/core/src/serve/stdio.ts`:
- Around line 21-32: Wrap the asyncAuthority setup block around
assembleCapletsHost, runtimeHost.retain, and CapletsMcpSession construction in
try/catch; if retain or session creation fails, await runtimeHost.close() before
rethrowing the original error. Keep the non-asyncAuthority path unchanged and
preserve the existing successful connection flow.
In `@packages/core/src/storage/bundle-cache.ts`:
- Around line 378-384: Fix the top-level markdown fallback in inferEntryPath by
matching a literal period before “md” rather than a backslash, so exactly one
normalized top-level .md path is returned. Preserve the existing CAPLET.md
preference and CONFIG_INVALID error for zero or multiple candidates.
- Around line 164-209: Update cleanup() to consult the live this.pending set
when deciding whether to remove each fingerprint directory, rather than relying
only on the initial pending snapshot; preserve the existing protections for
active, pinned, and referenced fingerprints. Add a regression test covering
materialize() starting after cleanup begins for an already-cached fingerprint,
ensuring cleanup does not remove that directory.
In `@packages/core/src/storage/composition.ts`:
- Around line 421-422: Replace locale-dependent entry sorting with deterministic
code-point/ordinal comparison in fingerprintDirectory and the sibling collectors
collectFilesSync and collectFiles. Update each comparator to compare names using
the same ordinal ordering, preserving the existing collection and fingerprint
behavior.
In `@packages/core/test/packed-package.test.ts`:
- Around line 158-165: Update optionalProviderDenyLoader’s blocked regex so the
drizzle-orm alternation matches the package prefix without the redundant inner
separator/end boundary, ensuring drizzle-orm and all subpath imports such as
drizzle-orm/sqlite-core are denied consistently.
In `@packages/core/test/storage-sql-migrations.test.ts`:
- Around line 21-27: Remove the metadata-only test “pins the stable Drizzle and
native driver packaging” from storage-sql-migrations.test.ts. Keep the existing
behavioral migration coverage unchanged, relying on those tests and package
review rather than asserting literal dependency versions.
In `@README.md`:
- Around line 234-245: Revise the S3 and Cloudflare R2 sentence in the
“Self-hosted shared storage” section to state the provider support or validation
status clearly and naturally. Preserve the requirement for credentialed live
verification, while replacing the awkward “before they should be called
validated” phrasing.
---
Outside diff comments:
In `@packages/core/src/serve/http.ts`:
- Around line 1527-1548: The attachEvents route must retain its runtime lease
for the entire SSE lifetime instead of releasing it when the handler resolves.
Update the handler around attachEventsResponse and requestEngine to acquire or
reuse the lease and release it from the stream’s close/cancel cleanup,
preserving the existing error response path and ensuring
source.manifestRevision() and engine.onReload() remain valid until the stream
closes.
In `@packages/core/src/serve/session.ts`:
- Around line 153-164: Update Session.close() so unsubscribeReload(),
codeModeSessions.close(), clearRegistrations(), and server.close() all execute
within the try/finally that releases runtimeLease. Preserve the existing closed
guard and cleanup order while ensuring runtimeLease?.release() runs even when
any pre-close cleanup throws.
---
Nitpick comments:
In `@packages/core/README.md`:
- Around line 138-146: The “The bootstrap provider union is:” line is a dangling
introduction in the “Configuration and source ownership” section. Remove it or
merge its wording into the following provider-specific field list, ensuring the
section has only one clear introduction before the filesystem, sqlite,
postgresql, and s3 entries.
In `@packages/core/src/storage/filesystem-authority.ts`:
- Around line 1280-1282: Update both receipt sort sites to use a
locale-independent bytewise/codepoint comparator instead of localeCompare when
comparing stableJsonStringify results. Preserve the existing receipt ordering
input to authorityExportDigest so export digests remain deterministic across
runtimes.
In `@packages/core/src/storage/migration.ts`:
- Line 1033: In the migration logic, replace the redundant conditional
assignment to nestedRecordId with a direct recordId assignment. The dedicated
caplets/records branch and its continue behavior should remain unchanged.
In `@packages/core/src/storage/s3-authority.ts`:
- Around line 3138-3143: Remove the unused nextWatermark() and
latestAuxiliaryWatermark() helpers, along with any now-unused supporting logic.
Since parseEvent() and eventsPrefix() exist only for latestAuxiliaryWatermark(),
remove those methods as well unless latestAuxiliaryWatermark() is retained.
In `@packages/core/src/storage/sql/migrations/postgres/0000_initial.sql`:
- Around line 61-63: Remove the redundant authority_events_after_idx creation
from the PostgreSQL migration and from both POSTGRES_INITIAL_SQL and
SQLITE_INITIAL_SQL in migrate.ts, while preserving the composite primary key on
(authority_id, watermark). Keep all initial schema definitions synchronized.
In `@packages/core/src/storage/sql/migrations/sqlite/0000_initial.sql`:
- Around line 54-62: Remove the redundant authority_events_after_idx definition
from the authority_events migration, since the composite PRIMARY KEY on
(authority_id, watermark) already provides the same index ordering. Keep the
authority_events table and primary-key constraint unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bc48427d-1ece-4a76-b53c-8cde55d56aad
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (124)
.changeset/composable-shared-storage.md.github/workflows/ci.ymlCONCEPTS.mdREADME.mdapps/dashboard/src/components/DashboardApp.test.tsxapps/dashboard/src/components/DashboardApp.tsxapps/docs/src/content/docs/reference/config.mdxapps/landing/public/config.schema.jsondocs/agents/domain.mddocs/plans/2026-07-11-002-feat-portable-cloud-runtime-deployment-plan.mddocs/plans/2026-07-11-003-feat-composable-shared-storage-plan.mddocs/product/self-hosting.mddocs/research/2026-07-11-cloud-runtime-provider-feasibility.mdpackage.jsonpackages/core/README.mdpackages/core/package.jsonpackages/core/prototypes/cloud-project-binding/model.tspackages/core/prototypes/cloud-project-binding/tui.tspackages/core/rolldown.config.tspackages/core/src/auth.tspackages/core/src/auth/store.tspackages/core/src/cli.tspackages/core/src/cli/auth.tspackages/core/src/cli/commands.tspackages/core/src/cli/completion.tspackages/core/src/cli/install.tspackages/core/src/cli/setup-caplet.tspackages/core/src/cli/storage.tspackages/core/src/cli/vault.tspackages/core/src/cloud/runtime-adapter.tspackages/core/src/config.tspackages/core/src/current-host/caplet-operations.tspackages/core/src/current-host/catalog-operations.tspackages/core/src/current-host/catalog.tspackages/core/src/current-host/client-operations.tspackages/core/src/current-host/operations.tspackages/core/src/current-host/settings-operations.tspackages/core/src/current-host/vault-operations.tspackages/core/src/dashboard/activity-log.tspackages/core/src/dashboard/session-store.tspackages/core/src/downstream.tspackages/core/src/engine.tspackages/core/src/errors.tspackages/core/src/google-discovery/manager.tspackages/core/src/index.tspackages/core/src/native.tspackages/core/src/native/service.tspackages/core/src/remote-control/client.tspackages/core/src/remote-control/dispatch.tspackages/core/src/remote-control/types.tspackages/core/src/remote/authority-codec.tspackages/core/src/remote/server-credential-store.tspackages/core/src/runtime.tspackages/core/src/serve/http.tspackages/core/src/serve/index.tspackages/core/src/serve/native-session.tspackages/core/src/serve/session.tspackages/core/src/serve/stdio.tspackages/core/src/setup/local-store.tspackages/core/src/setup/runner.tspackages/core/src/storage/backup.tspackages/core/src/storage/bundle-cache.tspackages/core/src/storage/composition.tspackages/core/src/storage/conformance.tspackages/core/src/storage/coordinator.tspackages/core/src/storage/factory.tspackages/core/src/storage/filesystem-authority.tspackages/core/src/storage/migration.tspackages/core/src/storage/s3-authority.tspackages/core/src/storage/sql/authority.tspackages/core/src/storage/sql/migrate.tspackages/core/src/storage/sql/migrations/postgres/0000_initial.sqlpackages/core/src/storage/sql/migrations/postgres/0001_head_guard.sqlpackages/core/src/storage/sql/migrations/postgres/0002_maintenance_lease.sqlpackages/core/src/storage/sql/migrations/postgres/meta/_journal.jsonpackages/core/src/storage/sql/migrations/sqlite/0000_initial.sqlpackages/core/src/storage/sql/migrations/sqlite/0001_head_guard.sqlpackages/core/src/storage/sql/migrations/sqlite/0002_maintenance_lease.sqlpackages/core/src/storage/sql/migrations/sqlite/meta/_journal.jsonpackages/core/src/storage/sql/schema-postgres.tspackages/core/src/storage/sql/schema-sqlite.tspackages/core/src/storage/types.tspackages/core/src/vault/access.tspackages/core/src/vault/index.tspackages/core/src/vault/keys.tspackages/core/src/vault/types.tspackages/core/test/attach-service-wiring.test.tspackages/core/test/authority-access-session.test.tspackages/core/test/authority-codec.test.tspackages/core/test/authority-secrets.test.tspackages/core/test/cli-completion.test.tspackages/core/test/cli-remote.test.tspackages/core/test/cli-storage.test.tspackages/core/test/current-host-administration.test.tspackages/core/test/current-host-catalog-operations.test.tspackages/core/test/dashboard-activity.test.tspackages/core/test/dashboard-api.test.tspackages/core/test/dashboard-authority-mutations.test.tspackages/core/test/fixtures/storage/minio-pinned.compose.ymlpackages/core/test/fixtures/storage/provider-matrix.compose.ymlpackages/core/test/packed-package.test.tspackages/core/test/remote-control-client.test.tspackages/core/test/serve-http.test.tspackages/core/test/serve-session.test.tspackages/core/test/setup-runner.test.tspackages/core/test/storage-backup.test.tspackages/core/test/storage-bundle-cache.test.tspackages/core/test/storage-composition.test.tspackages/core/test/storage-contract.test.tspackages/core/test/storage-coordinator.test.tspackages/core/test/storage-filesystem-authority.test.tspackages/core/test/storage-migration.test.tspackages/core/test/storage-multi-replica.test.tspackages/core/test/storage-postgres-authority-batching.test.tspackages/core/test/storage-postgres-authority.test.tspackages/core/test/storage-provider-contract.tspackages/core/test/storage-provider-matrix.test.tspackages/core/test/storage-s3-authority.test.tspackages/core/test/storage-s3-conformance.test.tspackages/core/test/storage-sql-migrations.test.tspackages/core/test/storage-sqlite-authority.test.tspnpm-workspace.yamlschemas/caplets-config.schema.jsonscripts/test-storage-providers.ts
| - name: Checkout | ||
| uses: actions/checkout@v7 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Set persist-credentials: false on actions/checkout.
Neither checkout disables credential persistence, so the GITHUB_TOKEN is written to .git/config and remains available to any later step. This is especially relevant for storage-provider-live, which also loads long-lived cloud secrets into the job environment.
🔒️ Proposed hardening
- name: Checkout
uses: actions/checkout@v7
+ with:
+ persist-credentials: falseAs per static analysis (zizmor artipacked: does not set persist-credentials: false).
Also applies to: 94-95
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 58-59: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 58 - 59, Update both actions/checkout
steps in the CI workflow to set persist-credentials to false, ensuring the
GITHUB_TOKEN is not retained in the checked-out repository for subsequent steps.
Source: Linters/SAST tools
| - `s3` — an S3-compatible authority using conditional object writes and ETags. The deterministic | ||
| local fixture uses the digest-pinned MinIO image | ||
| `docker.io/minio/minio@sha256:14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the pinned digest length in the doc.
rg -n '14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936' docs/product/self-hosting.md
python3 - <<'PY'
d="14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936"
print("length:", len(d), "valid_sha256:", len(d)==64)
PYRepository: spiritledsoftware/caplets
Length of output: 295
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '360,375p' docs/product/self-hosting.md | cat -n
rg -n '14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936' docs/product/self-hosting.mdRepository: spiritledsoftware/caplets
Length of output: 1453
Pinned MinIO digest is invalid.
sha256:14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936 is only 63 hex chars, so it cannot be pulled as a valid SHA-256 image reference. Re-derive the digest and replace this reference.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/product/self-hosting.md` around lines 23 - 25, The MinIO image reference
in the s3 self-hosting documentation uses an invalid 63-character digest.
Re-derive the correct SHA-256 digest for the pinned MinIO image and replace the
value in the deterministic local fixture description, preserving the
digest-pinned image format.
| const touched = await this.codec.commitAuxiliary({ | ||
| kind: "session_touch", | ||
| sessionId: session.sessionId, | ||
| lastUsedAt: now.toISOString(), | ||
| expectedRevision: auxiliaryRecord?.revision ?? "", | ||
| expectedGeneration: read.head, | ||
| }); | ||
| if (touched.kind === "revoked" || touched.kind === "missing" || touched.kind === "conflict") { | ||
| throw new CapletsError("AUTH_FAILED", "Dashboard session is no longer active."); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -C4 '"session_touch"|kind:\s*"conflict"' packages/core/src/storage
ast-grep outline packages/core/src/storage/sql/authority.ts --match 'commitAuxiliary'Repository: spiritledsoftware/caplets
Length of output: 23875
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the session-store flow around the reported lines.
sed -n '380,500p' packages/core/src/dashboard/session-store.ts
# Inspect the auxiliary commit result type and the storage implementations that define `conflict`.
sed -n '1,120p' packages/core/src/storage/types.ts
sed -n '1640,1710p' packages/core/src/storage/s3-authority.ts
sed -n '1310,1385p' packages/core/src/storage/sql/authority.ts
sed -n '400,435p' packages/core/src/storage/filesystem-authority.tsRepository: spiritledsoftware/caplets
Length of output: 17238
Do not fail validation on session_touch conflicts. commitAuxiliary() returns conflict for stale expectedGeneration/expectedRevision, so a concurrent authority commit or touch can make a valid session fail here. Only missing/revoked mean the session is gone; on conflict, return the session with persistedLastUsedAt (or skip the touch) instead of throwing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/dashboard/session-store.ts` around lines 440 - 449, Update
the session validation flow around codec.commitAuxiliary and the touched result
so only “missing” and “revoked” throw AUTH_FAILED. Handle “conflict” as a valid
session outcome by returning the session with persistedLastUsedAt, or skip the
touch while preserving the existing session response behavior.
| function assertAuthorityOperator(root: Record<string, unknown>, clientId: string): void { | ||
| const remote = root.remoteCredentials; | ||
| if (!remote || typeof remote !== "object" || Array.isArray(remote)) return; | ||
| const clients = (remote as { clients?: unknown }).clients; | ||
| if (!Array.isArray(clients)) return; | ||
| const client = clients.find( | ||
| (candidate) => | ||
| candidate && | ||
| typeof candidate === "object" && | ||
| !Array.isArray(candidate) && | ||
| (candidate as Record<string, unknown>).clientId === clientId, | ||
| ); | ||
| if ( | ||
| !client || | ||
| (client as Record<string, unknown>).role !== "operator" || | ||
| (client as Record<string, unknown>).revokedAt | ||
| ) { | ||
| throw new CapletsError("AUTH_FAILED", "Dashboard operator client is no longer authorized."); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -C3 'remoteCredentials' packages/core/src/remote packages/core/src/dashboardRepository: spiritledsoftware/caplets
Length of output: 1400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== session-store outline =="
ast-grep outline packages/core/src/dashboard/session-store.ts --view expanded | sed -n '1,220p'
echo
echo "== server-credential-store outline =="
ast-grep outline packages/core/src/remote/server-credential-store.ts --view expanded | sed -n '1,260p'
echo
echo "== session-store relevant slices =="
sed -n '600,760p' packages/core/src/dashboard/session-store.ts
echo
echo "== server-credential-store relevant slices =="
sed -n '820,980p' packages/core/src/remote/server-credential-store.ts
echo
echo "== search for validate/create and remoteCredentials.clients =="
rg -n -C 3 'remoteCredentials|clients|create\(|validate\(' packages/core/src/dashboard/session-store.ts packages/core/src/remote/server-credential-store.tsRepository: spiritledsoftware/caplets
Length of output: 40480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== session-store create/validate slices =="
sed -n '317,460p' packages/core/src/dashboard/session-store.ts
echo
echo "== authority-domain codec references =="
rg -n -C 3 'class AuthorityDomainCodec|domainSnapshot\(|read\(|write\(|snapshot' packages/core/src/remote/server-credential-store.ts packages/core/src/dashboard/session-store.ts
echo
echo "== root mutation / domain snapshot handling around codec methods =="
sed -n '1410,1495p' packages/core/src/remote/server-credential-store.ts
echo
echo "== dashboard store authority mutate slices =="
sed -n '317,430p' packages/core/src/dashboard/session-store.tsRepository: spiritledsoftware/caplets
Length of output: 37239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find AuthorityDomainCodec declaration =="
rg -n -C 4 'class AuthorityDomainCodec|domainSnapshot\(|commitAuxiliary\(|readAuxiliary\(' packages/core/src
echo
echo "== codec file candidates =="
fd -a 'authority-domain|codec|domain' packages/core/src
echo
echo "== likely codec file outlines =="
for f in $(fd -a 'authority-domain' packages/core/src); do
echo "--- $f ---"
ast-grep outline "$f" --view expanded | sed -n '1,220p'
doneRepository: spiritledsoftware/caplets
Length of output: 27257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== search for remoteCredentials restore/initialization/migration =="
rg -n -C 3 'remoteCredentials|dashboardSessions|restoreState|exportState|migrate|snapshot.*remoteCredentials|snapshot.*dashboardSessions' packages/core/src
echo
echo "== authority storage restore semantics =="
rg -n -C 3 'async restoreState|restoreState\(|exportState\(' packages/core/src/storage
echo
echo "== authority snapshot types / domains =="
rg -n -C 3 'type AuthorityExport|type AuthoritySnapshot|domains?:|snapshot:' packages/core/src/storage packages/core/src/remote packages/core/src/dashboardRepository: spiritledsoftware/caplets
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== instantiations of dashboard session stores =="
rg -n -C 2 'new (Authority)?DashboardSessionStore|DashboardSessionStore\(' packages/core/src
echo
echo "== authority read/restore export shapes =="
sed -n '1,120p' packages/core/src/remote/authority-codec.ts
echo
sed -n '120,220p' packages/core/src/remote/authority-codec.ts
echo
echo "== filesystem authority restore/export relevant slices =="
sed -n '483,560p' packages/core/src/storage/filesystem-authority.ts
echo
sed -n '885,980p' packages/core/src/storage/s3-authority.tsRepository: spiritledsoftware/caplets
Length of output: 15110
Fail closed when remoteCredentials is missing
AuthorityDomainCodec.read() can return an empty snapshot, and assertAuthorityOperator() returns without checking remoteCredentials.clients in that case. That lets create mint a dashboard session for any operatorClientId, and validate skips operator revocation checks for existing sessions. Treat the missing domain as unauthorized.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/dashboard/session-store.ts` around lines 665 - 684, Update
assertAuthorityOperator so a missing, non-object, or array-valued
remoteCredentials immediately throws the existing AUTH_FAILED CapletsError
instead of returning. Preserve the current client lookup and operator/revocation
checks for valid remoteCredentials objects, ensuring create and validate fail
closed when the authority domain is absent.
| function inferEntryPath(paths: string[]): string { | ||
| const markdown = paths.filter((path) => path === "CAPLET.md" || path.endsWith("/CAPLET.md")); | ||
| if (markdown.length === 1) return markdown[0]!; | ||
| const topLevel = paths.filter((path) => !path.includes("/") && /\\.md$/iu.test(path)); | ||
| if (topLevel.length === 1) return topLevel[0]!; | ||
| throw new CapletsError("CONFIG_INVALID", "Authority bundle must identify one entry document"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Dead fallback: entry-path inference from a single top-level .md file can never succeed.
const topLevel = paths.filter((path) => !path.includes("/") && /\\.md$/iu.test(path));The regex is \\.md$ — an escaped literal backslash character followed by any char (.) then md$, not an escaped dot. Since path values only ever contain forward slashes at this point (normalizeBundlePath already converts \ → / before any path reaches here), this regex can never match a real bundle path. This means: whenever a bundle has no CAPLET.md/*/CAPLET.md and no explicit entryPath, the "single top-level markdown file" fallback is dead code, and inferEntryPath always throws "Authority bundle must identify one entry document" even for a bundle with exactly one clear top-level .md file. It fails safe (clear CONFIG_INVALID error) rather than corrupting anything, but the documented fallback behavior doesn't work.
🐛 Proposed fix
- const topLevel = paths.filter((path) => !path.includes("/") && /\\.md$/iu.test(path));
+ const topLevel = paths.filter((path) => !path.includes("/") && /\.md$/iu.test(path));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/storage/bundle-cache.ts` around lines 378 - 384, Fix the
top-level markdown fallback in inferEntryPath by matching a literal period
before “md” rather than a backslash, so exactly one normalized top-level .md
path is returned. Preserve the existing CAPLET.md preference and CONFIG_INVALID
error for zero or multiple candidates.
| const entries = await readdir(current, { withFileTypes: true }); | ||
| entries.sort((left, right) => left.name.localeCompare(right.name)); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Node.js String.prototype.localeCompare determinism across locales LANG LC_COLLATE ICU
💡 Result:
In Node.js, the behavior of String.prototype.localeCompare is not inherently deterministic across different environments because it depends on the International Components for Unicode (ICU) library, which implements locale-sensitive string comparisons [1][2]. Determinism Factors: 1. ICU Data Availability: The accuracy and availability of locale-specific sorting depend on the ICU data set included with the Node.js binary [1][3]. Modern Node.js versions (v13+) include "full-icu" support by default, providing comprehensive locale data [4][5]. Older versions or builds using "small-icu" only include English data, leading to non-locale-aware fallback behavior for other languages [1][3]. 2. Environment Variables: ICU respects the LANG and LC_ALL environment variables to determine the default locale if one is not explicitly provided as an argument to localeCompare [6][7]. Changes to these environment variables in the host system will alter the results of localeCompare calls that omit the locale argument [6]. 3. Upstream Changes: Because localeCompare relies on ICU (and by extension the V8 engine), updates to the underlying ICU library or V8 engine in newer Node.js releases can introduce changes in sorting behavior for specific locales [6][4]. To achieve deterministic behavior: * Explicitly specify the locale and collation options in the localeCompare call (e.g., string.localeCompare(other, 'en-US', { sensitivity: 'base' })) instead of relying on the environment's default locale [6]. * Ensure the Node.js environment is configured with the expected ICU data set (e.g., ensuring "full-icu" is available) to avoid differences between platforms or build configurations [1][8][5]. If locale-independent sorting is required, standard comparison operators (<, >, ===) or string locale-agnostic methods should be used instead of localeCompare, as they perform comparisons based on UCS code point values [9].
Citations:
- 1: https://nodejs.org/dist/latest/docs/api/intl.html
- 2: https://github.com/nodejs/node/blob/main/doc/api/intl.md
- 3: https://nodejs.org/docs/latest-v22.x/api/intl.html
- 4: https://stackoverflow.com/questions/77937348/localecompare-yields-different-results-in-different-node-js-versions
- 5: String.localeCompare with sensitivity:"accent" works incorrectly for Turkish dotless/dotted "i" nodejs/node#34608
- 6: localeCompare changes behavior between versions nodejs/node#4067
- 7: Node respects LANG env var, but not LC_TIME, LC_NUMERIC, LC_MONETARY, ... nodejs/node#23301
- 8: String::localeCompare support required. nodejs/node#12529
- 9: localeCompare regression nodejs/node-v0.x-archive#25762
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section and any other localeCompare uses in the file.
sed -n '380,560p' packages/core/src/storage/composition.ts
printf '\n--- localeCompare search ---\n'
rg -n "localeCompare\(" packages/core/src/storage/composition.ts
printf '\n--- surrounding repository matches ---\n'
rg -n "localeCompare\(" packages/core/src -g '!**/dist/**' -g '!**/build/**'Repository: spiritledsoftware/caplets
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe localeCompare behavior in this Node runtime with a few locales and sample strings.
node - <<'JS'
const pairs = [
['a', 'A'],
['ä', 'z'],
['ß', 'ss'],
['é', 'e'],
['file2', 'file10'],
];
const locales = [undefined, 'en', 'sv', 'de', 'fr', 'und'];
for (const [a, b] of pairs) {
console.log(`PAIR ${JSON.stringify(a)} vs ${JSON.stringify(b)}`);
for (const locale of locales) {
const label = locale === undefined ? 'default' : locale;
try {
const res = locale === undefined ? a.localeCompare(b) : a.localeCompare(b, locale);
console.log(` ${label}: ${res}`);
} catch (e) {
console.log(` ${label}: ERROR ${e && e.message}`);
}
}
}
JSRepository: spiritledsoftware/caplets
Length of output: 534
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant function and any nearby comments/docstrings with line numbers.
nl -ba packages/core/src/storage/composition.ts | sed -n '390,540p'
printf '\n--- search for fingerprint-related wording ---\n'
rg -n "fingerprint|replica|determin|mount-path" packages/core/src/storage/composition.tsRepository: spiritledsoftware/caplets
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe whether the default locale changes with environment variables in this runtime.
for envspec in \
"LANG=C LC_ALL=C" \
"LANG=en_US.UTF-8 LC_ALL=en_US.UTF-8" \
"LANG=sv_SE.UTF-8 LC_ALL=sv_SE.UTF-8" \
"LANG=de_DE.UTF-8 LC_ALL=de_DE.UTF-8" \
"LANG=fr_FR.UTF-8 LC_ALL=fr_FR.UTF-8"
do
echo "== $envspec =="
bash -lc "$envspec node - <<'JS'
const opts = new Intl.Collator().resolvedOptions();
console.log(JSON.stringify(opts));
console.log('ä'.localeCompare('z'));
JS"
doneRepository: spiritledsoftware/caplets
Length of output: 1077
Use ordinal sorting here and in the sibling file collectors. localeCompare() without an explicit locale can change with host locale/ICU data, so identical staged trees can hash to different fingerprints on different replicas. Switch to code-point comparison in fingerprintDirectory, collectFilesSync, and collectFiles.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/storage/composition.ts` around lines 421 - 422, Replace
locale-dependent entry sorting with deterministic code-point/ordinal comparison
in fingerprintDirectory and the sibling collectors collectFilesSync and
collectFiles. Update each comparator to compare names using the same ordinal
ordering, preserving the existing collection and fingerprint behavior.
| function optionalProviderDenyLoader(): string { | ||
| return `const blocked = /^(?:@aws-sdk\\/client-s3|better-sqlite3|drizzle-orm(?:\\/|$)|postgres)(?:\\/|$)/; | ||
| export async function resolve(specifier, context, nextResolve) { | ||
| if (blocked.test(specifier)) throw new Error("optional provider evaluated: " + specifier); | ||
| return nextResolve(specifier, context, nextResolve); | ||
| } | ||
| `; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '130,190p' packages/core/test/packed-package.test.tsRepository: spiritledsoftware/caplets
Length of output: 2051
🏁 Script executed:
python3 - <<'PY'
import re
pattern = re.compile(r'^(?:`@aws-sdk`\/client-s3|better-sqlite3|drizzle-orm(?:\/|$)|postgres)(?:\/|$)')
samples = [
"drizzle-orm",
"drizzle-orm/",
"drizzle-orm/sqlite-core",
"drizzle-orm/pg-core",
"drizzle-orm/better-sqlite3",
"`@aws-sdk/client-s3`",
"`@aws-sdk/client-s3/foo`",
"better-sqlite3/abc",
"postgres/sql",
]
for s in samples:
print(s, bool(pattern.search(s)), pattern.search(s).group(0) if pattern.search(s) else None)
PYRepository: spiritledsoftware/caplets
Length of output: 486
Fix the drizzle-orm branch in the deny-loader regex
drizzle-orm(?:\/|$) already consumes the separator, so drizzle-orm/sqlite-core and other subpath imports bypass the boundary check. Drop the inner (?:\/|$) from that alternation.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile as execFileCallback } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/test/packed-package.test.ts` around lines 158 - 165, Update
optionalProviderDenyLoader’s blocked regex so the drizzle-orm alternation
matches the package prefix without the redundant inner separator/end boundary,
ensuring drizzle-orm and all subpath imports such as drizzle-orm/sqlite-core are
denied consistently.
| it("pins the stable Drizzle and native driver packaging", () => { | ||
| expect(corePackage.dependencies["drizzle-orm"]).toBe("0.45.2"); | ||
| expect(corePackage.dependencies["postgres"]).toBe("3.4.9"); | ||
| expect(corePackage.dependencies["better-sqlite3"]).toBe("12.11.1"); | ||
| expect(corePackage.devDependencies["drizzle-kit"]).toBe("0.31.10"); | ||
| expect(corePackage.devDependencies["@types/better-sqlite3"]).toBe("7.6.13"); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Metadata-pin test duplicates package.json literals without covering behavior.
This test only re-asserts dependency/devDependency version strings from package.json. The rest of the suite already exercises the migration behavior these pins support (histories, replay idempotency, advisory-lock serialization). As per coding guidelines: "Do not add tests that only assert package metadata... when stronger behavior coverage already exists." Consider dropping it; a stray dependency bump should be caught by the behavioral migration tests and lockfile review rather than a literal-equality snapshot.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/test/storage-sql-migrations.test.ts` around lines 21 - 27,
Remove the metadata-only test “pins the stable Drizzle and native driver
packaging” from storage-sql-migrations.test.ts. Keep the existing behavioral
migration coverage unchanged, relying on those tests and package review rather
than asserting literal dependency versions.
Source: Coding guidelines
ian-pascoe
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/config.ts (1)
2050-2062: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winExplicit default filesystem roots should not hard-fail The inventory builder always adds
resolveCapletsRoot(globalPath)as anauthoritysource, and the duplicate check rejects repeated canonical paths even for the same owner. That makesstorage.provider = "filesystem"withpathset to the documented default root fail withduplicate ownership (authority, authority), while omittingpathworks. Skip the extra authority entry when it matches the configured root, or ignore identical-owner duplicates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/config.ts` around lines 2050 - 2062, Update the canonical ownership validation around canonicalOwners so repeated canonical paths with the same owner do not throw; retain the CONFIG_INVALID error for duplicates claimed by different owners. Ensure explicit filesystem paths matching the default authority root remain valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/cli.ts`:
- Around line 4337-4349: Update resolveCliAuthorityContext() so a JSON parse
failure for an existing defaultStorageConfigPath surfaces CONFIG_INVALID instead
of returning undefined. Preserve the existing direct-storage and profile
resolution behavior for valid configurations, and ensure callers cannot fall
back to FileVaultStore or other local setup after this invalid-config condition.
In `@packages/core/src/storage/s3-authority.ts`:
- Around line 54-77: Update canonicalS3Root and its callers so the S3 object
prefix incorporates the authority namespace alongside path, ensuring authorities
sharing a bucket and path remain isolated. Preserve the existing path
sanitization and unsafe-segment validation while constructing a deterministic
namespace-specific .caplets prefix.
---
Outside diff comments:
In `@packages/core/src/config.ts`:
- Around line 2050-2062: Update the canonical ownership validation around
canonicalOwners so repeated canonical paths with the same owner do not throw;
retain the CONFIG_INVALID error for duplicates claimed by different owners.
Ensure explicit filesystem paths matching the default authority root remain
valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b9b7049-6bbf-4668-9673-3b389a94578c
📒 Files selected for processing (46)
.changeset/composable-shared-storage.mdREADME.mdapps/dashboard/src/components/DashboardApp.test.tsxapps/dashboard/src/components/DashboardApp.tsxapps/docs/src/content/docs/configuration.mdxapps/docs/src/content/docs/reference/config.mdxapps/landing/public/config.schema.jsondocs/plans/2026-07-12-001-refactor-provider-shaped-storage-config-plan.mddocs/product/self-hosting.mdpackages/core/README.mdpackages/core/src/cli.tspackages/core/src/cli/storage.tspackages/core/src/config.tspackages/core/src/engine.tspackages/core/src/index.tspackages/core/src/serve/http.tspackages/core/src/serve/stdio.tspackages/core/src/storage/backup.tspackages/core/src/storage/coordinator.tspackages/core/src/storage/factory.tspackages/core/src/storage/filesystem-authority.tspackages/core/src/storage/migration.tspackages/core/src/storage/s3-authority.tspackages/core/src/storage/sql/authority.tspackages/core/src/storage/sql/migrate.tspackages/core/src/storage/sql/migrations/postgres/0000_initial.sqlpackages/core/src/storage/sql/migrations/postgres/0001_head_guard.sqlpackages/core/src/storage/sql/migrations/postgres/0002_maintenance_lease.sqlpackages/core/src/storage/sql/schema-postgres.tspackages/core/test/cli-storage.test.tspackages/core/test/config.test.tspackages/core/test/dashboard-authority-mutations.test.tspackages/core/test/serve-http.test.tspackages/core/test/serve-stdio.test.tspackages/core/test/storage-backup.test.tspackages/core/test/storage-contract.test.tspackages/core/test/storage-coordinator.test.tspackages/core/test/storage-migration.test.tspackages/core/test/storage-postgres-authority-batching.test.tspackages/core/test/storage-postgres-authority.test.tspackages/core/test/storage-s3-authority.test.tspackages/core/test/storage-s3-conformance.test.tspackages/core/test/storage-sql-migrations.test.tsschemas/caplets-config.schema.jsonscripts/generate-docs-reference.tsscripts/test-storage-providers.ts
💤 Files with no reviewable changes (1)
- packages/core/test/storage-backup.test.ts
🚧 Files skipped from review as they are similar to previous changes (27)
- packages/core/src/storage/sql/migrations/postgres/0001_head_guard.sql
- packages/core/src/storage/sql/migrations/postgres/0002_maintenance_lease.sql
- .changeset/composable-shared-storage.md
- README.md
- packages/core/src/storage/sql/schema-postgres.ts
- packages/core/test/storage-postgres-authority.test.ts
- packages/core/src/storage/sql/migrations/postgres/0000_initial.sql
- packages/core/src/serve/stdio.ts
- packages/core/test/storage-postgres-authority-batching.test.ts
- apps/dashboard/src/components/DashboardApp.test.tsx
- packages/core/README.md
- packages/core/src/storage/factory.ts
- packages/core/src/engine.ts
- packages/core/src/storage/backup.ts
- packages/core/test/storage-s3-conformance.test.ts
- packages/core/test/storage-contract.test.ts
- scripts/test-storage-providers.ts
- packages/core/test/dashboard-authority-mutations.test.ts
- packages/core/src/cli/storage.ts
- packages/core/test/storage-s3-authority.test.ts
- packages/core/src/storage/migration.ts
- packages/core/src/storage/coordinator.ts
- packages/core/src/storage/sql/authority.ts
- apps/dashboard/src/components/DashboardApp.tsx
- packages/core/src/storage/filesystem-authority.ts
- packages/core/src/storage/sql/migrate.ts
- packages/core/src/serve/http.ts
| if (existsSync(defaultStorageConfigPath)) { | ||
| try { | ||
| const parsed = JSON.parse(readFileSync(defaultStorageConfigPath, "utf8")) as { | ||
| storage?: unknown; | ||
| }; | ||
| hasDirectStorage = | ||
| parsed !== null && typeof parsed === "object" && parsed.storage !== undefined; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
| const profile = hasDirectStorage ? undefined : singleStorageProfileName(env); | ||
| if (!hasDirectStorage && profile === undefined) return undefined; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm this is the only place shared-config parse errors are handled and that
# downstream selectors would otherwise surface the error.
rg -nP -C4 'catch\s*\{\s*\n?\s*return undefined' packages/core/src/cli.ts
rg -nP -C3 'JSON\.parse\(readFileSync' packages/core/src/cli.tsRepository: spiritledsoftware/caplets
Length of output: 767
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant config helper and authority resolution flow.
sed -n '640,700p' packages/core/src/cli.ts
printf '\n---\n'
sed -n '4300,4465p' packages/core/src/cli.ts
printf '\n---\n'
rg -n -C3 'readUserConfigObject\(|singleStorageProfileName\(|resolveCliAuthorityContext\(|runWithAuthority\(' packages/core/src/cli.tsRepository: spiritledsoftware/caplets
Length of output: 14440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect representative commands that choose local stores when authority is absent.
sed -n '3028,3198p' packages/core/src/cli.ts
printf '\n---\n'
sed -n '3226,3338p' packages/core/src/cli.ts
printf '\n---\n'
sed -n '3432,3570p' packages/core/src/cli.tsRepository: spiritledsoftware/caplets
Length of output: 16476
Avoid silently falling back to local stores when the shared config is invalid. If defaultStorageConfigPath exists but can’t be parsed, resolveCliAuthorityContext() returns undefined, and these CLI paths drop to FileVaultStore/local setup instead of surfacing CONFIG_INVALID. That can write secrets to the wrong storage target.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/cli.ts` around lines 4337 - 4349, Update
resolveCliAuthorityContext() so a JSON parse failure for an existing
defaultStorageConfigPath surfaces CONFIG_INVALID instead of returning undefined.
Preserve the existing direct-storage and profile resolution behavior for valid
configurations, and ensure callers cannot fall back to FileVaultStore or other
local setup after this invalid-config condition.
| function canonicalS3Root(path: string | undefined): string { | ||
| const normalized = (path ?? "").replace(/^\/+|\/+$/gu, ""); | ||
| if (!normalized) return ".caplets/"; | ||
| for (const character of normalized) { | ||
| const codePoint = character.codePointAt(0); | ||
| if ( | ||
| codePoint === undefined || | ||
| codePoint < 0x20 || | ||
| (codePoint >= 0x7f && codePoint <= 0x9f) || | ||
| character === "\\" | ||
| ) { | ||
| throw new CapletsError("CONFIG_INVALID", "S3 storage path contains unsafe characters"); | ||
| } | ||
| } | ||
| const segments = normalized.split("/"); | ||
| if ( | ||
| segments.some( | ||
| (segment) => segment === "" || segment === "." || segment === ".." || segment === ".caplets", | ||
| ) | ||
| ) { | ||
| throw new CapletsError("CONFIG_INVALID", "S3 storage path contains an unsafe segment"); | ||
| } | ||
| return `${segments.join("/")}/.caplets/`; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether namespace ever contributes to S3 key generation, and how prior code scoped keys.
rg -nP -C2 'this\.namespace' packages/core/src/storage/s3-authority.ts | rg -nP 'Key|prefix|rootPrefix|key\('
rg -nP -C3 'rootPrefix|canonicalS3Root' packages/core/src/storage/s3-authority.ts
# Confirm docs/config express a per-namespace path requirement for S3.
rg -nP -C2 'namespace|path' schemas/caplets-config.schema.json | rg -i 's3|namespace|path'Repository: spiritledsoftware/caplets
Length of output: 8035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the S3 authority file and inspect the parts that touch namespace/path.
ast-grep outline packages/core/src/storage/s3-authority.ts --view expanded
printf '\n--- constructor / namespace usage ---\n'
rg -n -C3 'namespace|rootPrefix|canonicalS3Root|validateGeneration|CONFIG_INVALID' packages/core/src/storage/s3-authority.ts
printf '\n--- S3 schema/docs around path ---\n'
sed -n '90,150p' schemas/caplets-config.schema.json
printf '\n--- any tests around S3 authority path/namespace isolation ---\n'
rg -n -C2 's3-authority|namespace|path' packages/core test tests | head -n 200Repository: spiritledsoftware/caplets
Length of output: 50381
Namespace needs to be part of the S3 prefix rootPrefix is derived only from path, so two authorities sharing a bucket/path but using different namespaces will target the same objects and then fail validateGeneration with CONFIG_INVALID. Either include namespace in the key prefix or make the unique-path requirement explicit and enforced.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/storage/s3-authority.ts` around lines 54 - 77, Update
canonicalS3Root and its callers so the S3 object prefix incorporates the
authority namespace alongside path, ensuring authorities sharing a bucket and
path remain isolated. Preserve the existing path sanitization and unsafe-segment
validation while constructing a deterministic namespace-specific .caplets
prefix.
1 participant
Summary
Self-hosting operators can keep immutable container-staged Caplets while choosing filesystem, SQLite, PostgreSQL, or S3-compatible storage for dashboard-managed Caplets, credentials, settings, sessions, activity, backups, restores, and migrations. The public contract is now one provider-shaped
storageblock; internal generation identity, namespaces, and resolved secret material remain implementation details rather than operator-facing configuration.This gives single-node and horizontally scaled deployments the same administration and lifecycle model without treating a replica-local filesystem as shared state.
Design decisions
storage.providerselects filesystem, SQLite, PostgreSQL, or S3 with only that provider's physical placement and secret-reference fields. Legacyauthority, public authority IDs, target namespaces, and provider-specific top-level aliases are rejected.capletsschema; S3 owns a validated bucket/root prefix; local providers canonicalize their paths and reject overlap with staged or migration inputs.CAPLETS_STORAGE_PROFILE_<NAME>declaration and fail closed on malformed or unresolved input.Validation
pnpm verifypassed end to end:agent-browsersmoke tests loaded/dashboard/runtime,/dashboard/settings, and/dashboard/caplets; the disconnected Current Host authorization shell rendered with no page or console errors. Authenticated Storage Generation interactions require a running Current Host and operator client, so they are covered by dashboard component and HTTP integration tests.TEST_POSTGRES_URLwas not available for the final refactor pass, so live PostgreSQL authority suites remained explicitly skipped; deterministic migration byte-alignment and fixed-schema assertions passed.Related
Fixes #210.
Fixes #211.
Fixes #212.
Fixes #213.
Fixes #214.
Fixes #215.
Fixes #216.
Summary by CodeRabbit