refactor(core): deepen Caplets architecture seams#205
Merged
Conversation
Unify HTTP-like inline, local-artifact, and remote-reference results across progressive, direct, native, and hosted adapters. Preserve mixed MCP content, enforce GraphQL artifact bounds, and keep filesystem paths behind explicit host capability.
Construct one named runtime over the seven backend managers, route progressive, direct, completion-discovery, and nested calls through its six-operation interface, and retain MCP-only resource, prompt, and completion behavior on an explicit capability.
Make one generation-bound projection authoritative for MCP, Attach, Native, and Code Mode registration. Preserve contextual completion and fail closed on stale or malformed partial discovery state.
Keep recovery facts at the config seam so setup and doctor no longer parse warning prose. Preserve safe warning presentation while distinguishing recoverable quarantine from loader failure.
Route dashboard and Operator bearer administration through one typed operations boundary with exact role enforcement, shared redaction, and actor-attributed outcomes. Keep Raw Vault Reveal dashboard-only with no-store responses and ephemeral browser state.
Reconstruct approved anonymous event envelopes at PostHog before-send time, preserve transport identity without person profiles, and isolate provider failures across all public sites. Record verified provider-side IP discard separately from event-level GeoIP suppression.
Contributor
📝 WalkthroughWalkthroughThis PR unifies media results, backend dispatch, exposure projection, Current Host administration, telemetry sanitization, Vault quarantine reporting, Project Binding lifecycle handling, and dashboard reveal invalidation. It also adds extensive coverage for streaming artifacts, stale projections, authorization, races, and observability failures. ChangesArchitecture and runtime refactor
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant CapletsMcpSession
participant CapletsEngine
participant BackendOperationRuntime
participant MediaArtifactWriter
Client->>CapletsMcpSession: invoke projected tool
CapletsMcpSession->>CapletsEngine: execute generation-bound route
CapletsEngine->>BackendOperationRuntime: callTool(server, request)
BackendOperationRuntime->>MediaArtifactWriter: stream oversized response
MediaArtifactWriter-->>BackendOperationRuntime: local-artifact or remote-reference
BackendOperationRuntime-->>CapletsEngine: structured result
CapletsEngine-->>CapletsMcpSession: annotated result
CapletsMcpSession-->>Client: content and artifact metadata
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
Preview DeployedLanding: https://pr-205.preview.caplets.dev Built from commit daa5db8 |
Contributor
|
| Filename | Overview |
|---|---|
| packages/core/src/media/artifacts.ts | Major rewrite: converts sync FS to async streams with a transactional publication flow, in-process and file-system locking, and backup/rollback. Mixes a few remaining lstatSync/existsSync calls with the otherwise-async surface. |
| packages/core/src/media/results.ts | New file defining the inline/local-artifact/remote-reference MediaResult union type and JSON-Schema helper. Clean, no issues. |
| packages/core/src/exposure/projection.ts | Major expansion adding manifest-based projection builders and the NativeProjectionMerge helpers. Logic is well-structured and the shadowing/suppression rules are consistently applied. |
| packages/core/src/native/project-binding-lifecycle.ts | New NativeProjectBindingLifecycle class with correct serialization of start/update/replace/close. State machine handles the closing-during-replacement and cleanup-failed cases. |
| packages/core/src/current-host/vault-operations.ts | Extracts vault CRUD and access-grant logic from the dashboard adapter. No new issues beyond the previously-flagged rollback edge case. |
| packages/core/src/current-host/operations.ts | Central dispatch for all Current Host operations. assertOperatorPrincipal enforces role + client-ID format before any operation; error messages are comprehensively redacted. |
| packages/core/src/backend-operation-dispatch.ts | New typed dispatch table for all seven backend kinds. Clean adapter-table pattern; MCP-only operations are kept separate in McpOperationAdapter. |
| apps/dashboard/src/components/DashboardApp.tsx | Vault reveal hardened with generation counters, isMounted guard, ACTION_DISCARDED sentinel, and ephemeral expiry timer. Confirmation stale-response fix via confirmationRef is correct. |
| apps/dashboard/src/lib/ephemeral-reveal.ts | New small module: EphemeralRevealExpiry with replace/cancel. Clean and well-tested. |
| packages/web-observability/src/posthog.ts | New centralized PostHog sanitization. sanitizePostHogCapture validates UUID, token, distinct_id, and privacy flags; categoricalProperties drops unknown keys via assertWebEventSafeProperties. Failures are silently swallowed as designed. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant C as Caller
participant W as ArtifactWriter
participant L as PublicationLock
participant FS as FileSystem
C->>W: createMediaArtifactWriter(input)
W->>FS: mkdir(dir)
W->>FS: open(tempPath, wx)
W-->>C: writer
C->>W: write(chunk)
W->>FS: file.write(chunk)
W->>W: hash.update + byteLength++
C->>W: complete()
W->>FS: file.close()
W->>L: serializeArtifactPublication(target)
L->>FS: mkdir(lockDir) exclusive
L->>FS: writeFile(owner.json)
L->>W: run operation()
W->>FS: recoverOrScavengeArtifactBackups()
W->>FS: chmod(tempPath)
W->>FS: rename target to backupPath if exists
W->>FS: rename tempPath to target
W->>W: "completed = true"
W->>FS: removeArtifactBackups(backupPath)
L->>FS: rm(lockDir)
W-->>C: MediaArtifact
alt Error during publish
W->>FS: restorePreviousArtifacts rollback
W-->>C: throw error
end
alt Caller aborts
C->>W: abort()
W->>FS: closeFile + removeStaging
end
%%{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 Caller
participant W as ArtifactWriter
participant L as PublicationLock
participant FS as FileSystem
C->>W: createMediaArtifactWriter(input)
W->>FS: mkdir(dir)
W->>FS: open(tempPath, wx)
W-->>C: writer
C->>W: write(chunk)
W->>FS: file.write(chunk)
W->>W: hash.update + byteLength++
C->>W: complete()
W->>FS: file.close()
W->>L: serializeArtifactPublication(target)
L->>FS: mkdir(lockDir) exclusive
L->>FS: writeFile(owner.json)
L->>W: run operation()
W->>FS: recoverOrScavengeArtifactBackups()
W->>FS: chmod(tempPath)
W->>FS: rename target to backupPath if exists
W->>FS: rename tempPath to target
W->>W: "completed = true"
W->>FS: removeArtifactBackups(backupPath)
L->>FS: rm(lockDir)
W-->>C: MediaArtifact
alt Error during publish
W->>FS: restorePreviousArtifacts rollback
W-->>C: throw error
end
alt Caller aborts
C->>W: abort()
W->>FS: closeFile + removeStaging
end
Reviews (4): Last reviewed commit: "fix(core): harden artifact and reveal re..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
packages/core/test/mcp-test-client.ts (1)
20-42: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
close()doesn't propagate to the peer transport.
close()only invokesthis.onclose, neverthis.peer?.onclose. Any test relying on the server-side session observing disconnect (e.g., session cleanup, resource release assertions) won't see it fire when the client side closes, and vice versa.🔧 Proposed fix
async close(): Promise<void> { + const peer = this.peer; + this.peer = undefined; + if (peer) { + peer.peer = undefined; + peer.onclose?.(); + } this.onclose?.(); }🤖 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/mcp-test-client.ts` around lines 20 - 42, LinkedTransport.close() only notifies the local transport, so peer disconnect handlers never run. Update close() to invoke both this.onclose and this.peer?.onclose, preserving safe handling when no peer exists and preventing duplicate notifications if both sides close.packages/core/src/google-discovery/manager.ts (1)
142-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInline-threshold fallback omitted here vs. explicit
DEFAULT_MAX_RESPONSE_BYTESinhttp-actions.ts.When
mediaInlineThresholdBytesis unset, this manager omitsmaxInlineBytesentirely (relying onreadHttpLikeResponse's internal default), whereashttp-actions.tsexplicitly falls back toDEFAULT_MAX_RESPONSE_BYTES. If those two defaults ever diverge, inline/artifact behavior will differ across backends for the same underlying response-parsing utility. Consider aligning both managers on the same explicit fallback.Also applies to: 215-217
🤖 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/google-discovery/manager.ts` around lines 142 - 144, Align the fallback handling in the manager’s response options with http-actions.ts: update the option construction near both mediaInlineThresholdBytes usages to always set maxInlineBytes, using this.options.mediaInlineThresholdBytes when provided and DEFAULT_MAX_RESPONSE_BYTES otherwise. Import or reference the shared constant consistently so both backends use the same explicit default.packages/core/src/current-host/vault-operations.ts (1)
245-265: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffOverlay config is reloaded from disk on every grant/set-with-grant call.
vaultAccessOrigincallsloadLocalOverlayConfigWithSourcessynchronously per operation. Given this runs on the request path for Current Host administration, consider caching the overlay per request/session if this proves to be a hot path under load.🤖 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/current-host/vault-operations.ts` around lines 245 - 265, Avoid reloading overlay configuration from disk on every Vault grant or set-with-grant request. Refactor vaultAccessOrigin and its callers to reuse a per-request or session-scoped overlay, while preserving control-context, missing-caplet, and shadow validation; invalidate or refresh the cached overlay when configuration changes.apps/dashboard/src/components/DashboardApp.test.tsx (1)
205-237: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing assertion for the "current" (non-stale) reveal outcome.
After
current.resolve(...)(Lines 233-236) there is no assertion. The test proves the stale response is ignored, but never proves the newer response is actually applied (e.g.toast.successcalled / vault refreshed). Without this, a bug that silently discards all reveal responses (not just stale ones) would still pass.✅ Suggested addition
await act(async () => { current.resolve({ value: "current secret" }); }); await flush(); + + expect(dashboardApi).toHaveBeenCalled(); + expect(toast.success).toHaveBeenCalled(); });🤖 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 `@apps/dashboard/src/components/DashboardApp.test.tsx` around lines 205 - 237, Add assertions after resolving current in “does not refresh or toast when a newer reveal invalidates an older response” to verify the non-stale result is applied, including the expected success toast and vault refresh/API call with “current secret” as appropriate. Keep the existing stale-response assertions unchanged.packages/core/test/runtime.test.ts (1)
420-421: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove mock-shape assertions.
These assertions only prove that
mockServer()lacks registration methods. Lines 422-425 already validate the meaningful runtime behavior: failed refresh removes tools and reports the unsupported registration failure.Proposed fix
- expect(server).not.toHaveProperty("registerResource"); - expect(server).not.toHaveProperty("registerPrompt"); expect(runtime.registeredToolIds()).toEqual([]);As per coding guidelines, avoid tests that “merely restate implementation literals” when stronger behavior coverage exists.
🤖 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/runtime.test.ts` around lines 420 - 421, Remove the mock-shape assertions checking that `server` lacks `registerResource` and `registerPrompt`; retain the existing runtime behavior assertions that verify failed refresh removes tools and reports the unsupported registration failure.Source: Coding guidelines
apps/dashboard/src/components/DashboardApp.tsx (1)
2382-2384: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrefer
useReffor the expiry controller.createEphemeralRevealExpiry(...)returns mutable timer state, souseMemois the wrong storage mechanism here; React may discard memoized values. A ref or lazyuseStateinitializer gives this controller the stable identity it needs.🤖 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 `@apps/dashboard/src/components/DashboardApp.tsx` around lines 2382 - 2384, Replace the useMemo-based expiry controller in DashboardApp with a stable useRef or lazy useState initializer, preserving the setRevealed callback and ensuring createEphemeralRevealExpiry is invoked only once for the component lifetime.packages/core/src/http/response.ts (1)
62-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated JSON-MIME matching logic.
The
application/json/+json//jsonsuffix check used to gateinspectChunk(Lines 63-66) duplicates the same matching logic insideshouldInline(Lines 190-192). Consider extracting a sharedisJsonMime(mimeType: string): booleanhelper to avoid drift if the JSON-detection rule changes later.♻️ Proposed refactor
+function isJsonMime(mimeType: string): boolean { + return ( + mimeType === "application/json" || mimeType.endsWith("+json") || mimeType.endsWith("/json") + ); +} + const inspectChunk = - settings.mimeType === "application/json" || - settings.mimeType.endsWith("+json") || - settings.mimeType.endsWith("/json") + isJsonMime(settings.mimeType) ? options.inspectChunk : undefined;function shouldInline(response: Response, mimeType: string): boolean { if (isAttachment(response)) { return false; } return ( mimeType === "" || - mimeType === "application/json" || - mimeType.endsWith("+json") || - mimeType.endsWith("/json") || + isJsonMime(mimeType) || mimeType.startsWith("text/") ); }Also applies to: 184-195
🤖 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/http/response.ts` around lines 62 - 67, Extract the repeated JSON MIME detection from the inspectChunk assignment and shouldInline into a shared isJsonMime(mimeType: string): boolean helper. Implement the helper with the existing application/json, +json, and /json checks, then use it in both locations to keep the matching rule consistent.packages/core/src/project-binding/workspaces.ts (1)
6-6: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueSolid resiliency hardening: atomic writes + defensive JSON parsing.
Temp-file +
renameSyncatomic writes with restrictive permissions, plus try/catch + runtime type guards for metadata/lease parsing, correctly harden this store against torn/corrupt files and partial writes.One optional follow-up: leftover
*.tmpfiles from a crash betweenwriteFileSyncandrenameSyncare never swept sinceleasesFor()only globs.json. Consider havingcleanup()also remove stale*.tmpartifacts.Also applies to: 234-268, 289-325
🤖 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/project-binding/workspaces.ts` at line 6, The cleanup logic does not remove crash-leftover temporary files. Update cleanup-related logic, including leasesFor() and cleanup(), to discover and delete stale *.tmp artifacts alongside JSON lease files, while preserving the existing atomic-write behavior.packages/core/test/google-discovery.test.ts (1)
1661-1678: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
artifactByteLengthmanual narrowing works but is verbose; low priority.A small type guard or shared helper would reduce the repeated
in-checks, but this is test-only code with no current pain point.🤖 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/google-discovery.test.ts` around lines 1661 - 1678, Refactor the verbose manual narrowing in artifactByteLength into a reusable type guard or shared artifact-result helper, preserving support for local-artifact and remote-reference results and the existing error behavior.packages/core/src/caplet-sets.ts (1)
133-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
callToolwidened toBackendCallToolResult, error path still casts to the narrowerCompatibilityCallToolResult.Functionally correct since
CompatibilityCallToolResultis a member of theBackendCallToolResultunion, but the cast on Line 154 no longer matches the declared return type shape used elsewhere in this function.♻️ Optional cleanup
- return errorResult(error) as CompatibilityCallToolResult; + return errorResult(error) as BackendCallToolResult;🤖 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/caplet-sets.ts` around lines 133 - 156, Update callTool’s catch branch to return errorResult(error) using the declared BackendCallToolResult type, removing the outdated CompatibilityCallToolResult cast while preserving the existing error handling.
🤖 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/current-host/catalog-operations.ts`:
- Around line 104-126: Update each catalog install/update failure handler,
including the blocks around the current catch and the handlers near the other
referenced sections, to record one failure activity per requested caplet ID
instead of only capletIds[0]. Use appendFailureActivity with an array derived
from all capletIds, falling back to a single current-host catalog target when no
IDs are provided, preserving the catalog type for every target.
In `@packages/core/test/dashboard-catalog.test.ts`:
- Around line 193-195: Remove the duplicate error property declarations from the
response type literals used by the dashboard response parsing, including the
declarations near dashboardError and the corresponding block around the second
referenced location; retain a single error member with the intended { code:
string; message: string } shape in each type.
---
Nitpick comments:
In `@apps/dashboard/src/components/DashboardApp.test.tsx`:
- Around line 205-237: Add assertions after resolving current in “does not
refresh or toast when a newer reveal invalidates an older response” to verify
the non-stale result is applied, including the expected success toast and vault
refresh/API call with “current secret” as appropriate. Keep the existing
stale-response assertions unchanged.
In `@apps/dashboard/src/components/DashboardApp.tsx`:
- Around line 2382-2384: Replace the useMemo-based expiry controller in
DashboardApp with a stable useRef or lazy useState initializer, preserving the
setRevealed callback and ensuring createEphemeralRevealExpiry is invoked only
once for the component lifetime.
In `@packages/core/src/caplet-sets.ts`:
- Around line 133-156: Update callTool’s catch branch to return
errorResult(error) using the declared BackendCallToolResult type, removing the
outdated CompatibilityCallToolResult cast while preserving the existing error
handling.
In `@packages/core/src/current-host/vault-operations.ts`:
- Around line 245-265: Avoid reloading overlay configuration from disk on every
Vault grant or set-with-grant request. Refactor vaultAccessOrigin and its
callers to reuse a per-request or session-scoped overlay, while preserving
control-context, missing-caplet, and shadow validation; invalidate or refresh
the cached overlay when configuration changes.
In `@packages/core/src/google-discovery/manager.ts`:
- Around line 142-144: Align the fallback handling in the manager’s response
options with http-actions.ts: update the option construction near both
mediaInlineThresholdBytes usages to always set maxInlineBytes, using
this.options.mediaInlineThresholdBytes when provided and
DEFAULT_MAX_RESPONSE_BYTES otherwise. Import or reference the shared constant
consistently so both backends use the same explicit default.
In `@packages/core/src/http/response.ts`:
- Around line 62-67: Extract the repeated JSON MIME detection from the
inspectChunk assignment and shouldInline into a shared isJsonMime(mimeType:
string): boolean helper. Implement the helper with the existing
application/json, +json, and /json checks, then use it in both locations to keep
the matching rule consistent.
In `@packages/core/src/project-binding/workspaces.ts`:
- Line 6: The cleanup logic does not remove crash-leftover temporary files.
Update cleanup-related logic, including leasesFor() and cleanup(), to discover
and delete stale *.tmp artifacts alongside JSON lease files, while preserving
the existing atomic-write behavior.
In `@packages/core/test/google-discovery.test.ts`:
- Around line 1661-1678: Refactor the verbose manual narrowing in
artifactByteLength into a reusable type guard or shared artifact-result helper,
preserving support for local-artifact and remote-reference results and the
existing error behavior.
In `@packages/core/test/mcp-test-client.ts`:
- Around line 20-42: LinkedTransport.close() only notifies the local transport,
so peer disconnect handlers never run. Update close() to invoke both
this.onclose and this.peer?.onclose, preserving safe handling when no peer
exists and preventing duplicate notifications if both sides close.
In `@packages/core/test/runtime.test.ts`:
- Around line 420-421: Remove the mock-shape assertions checking that `server`
lacks `registerResource` and `registerPrompt`; retain the existing runtime
behavior assertions that verify failed refresh removes tools and reports the
unsupported registration failure.
🪄 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: b58fc530-9cad-4041-b1ca-1dfccd7046e5
📒 Files selected for processing (107)
.changeset/calm-media-seam.mdapps/catalog/src/scripts/observability.tsapps/catalog/test/observability.test.tsapps/dashboard/src/components/DashboardApp.test.tsxapps/dashboard/src/components/DashboardApp.tsxapps/dashboard/src/lib/ephemeral-reveal.test.tsapps/dashboard/src/lib/ephemeral-reveal.tsapps/dashboard/vitest.config.tsapps/docs/src/content/docs/privacy/indexing.mdxapps/docs/src/content/docs/reference/code-mode-api.mdxapps/docs/src/content/docs/troubleshooting.mdxapps/docs/src/scripts/observability.tsapps/docs/test/observability.test.tsapps/landing/src/scripts/observability.tsapps/landing/test/observability.test.tsdocs/architecture.mddocs/plans/2026-07-09-001-refactor-caplets-architecture-deepening-plan.mddocs/product/anonymous-telemetry.mddocs/product/telemetry-provider-readiness.mdpackages/core/src/attach/api.tspackages/core/src/backend-operation-dispatch.tspackages/core/src/caplet-sets.tspackages/core/src/cli.tspackages/core/src/cli/code-mode.tspackages/core/src/cli/doctor.tspackages/core/src/cloud/client.tspackages/core/src/cloud/presence.tspackages/core/src/code-mode/runtime-api.d.tspackages/core/src/code-mode/runtime-api.generated.tspackages/core/src/config.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/vault-operations.tspackages/core/src/downstream.tspackages/core/src/engine.tspackages/core/src/exposure/discovery.tspackages/core/src/exposure/projection.tspackages/core/src/generated-tool-input-schema.tspackages/core/src/google-discovery/manager.tspackages/core/src/google-discovery/operations.tspackages/core/src/graphql.tspackages/core/src/http-actions.tspackages/core/src/http/response.tspackages/core/src/index.tspackages/core/src/media/artifacts.tspackages/core/src/media/index.tspackages/core/src/media/input.tspackages/core/src/media/results.tspackages/core/src/native/project-binding-lifecycle.tspackages/core/src/native/remote.tspackages/core/src/native/service.tspackages/core/src/openapi.tspackages/core/src/project-binding/workspaces.tspackages/core/src/remote-control/dispatch.tspackages/core/src/remote-control/types.tspackages/core/src/result-content.tspackages/core/src/runtime.tspackages/core/src/serve/http.tspackages/core/src/serve/index.tspackages/core/src/serve/session.tspackages/core/src/tools.tspackages/core/test/attach-api.test.tspackages/core/test/backend-operation-dispatch.test.tspackages/core/test/backend-operation-runtime.tspackages/core/test/caplet-sets.test.tspackages/core/test/cli-tools.test.tspackages/core/test/cli.test.tspackages/core/test/cloud-presence.test.tspackages/core/test/code-mode-cli.test.tspackages/core/test/code-mode-mcp.test.tspackages/core/test/config.test.tspackages/core/test/current-host-administration.test.tspackages/core/test/dashboard-api.test.tspackages/core/test/dashboard-catalog.test.tspackages/core/test/dashboard-session.test.tspackages/core/test/dashboard-vault.test.tspackages/core/test/doctor-cli.test.tspackages/core/test/downstream.test.tspackages/core/test/exposure-discovery.test.tspackages/core/test/exposure-projection.test.tspackages/core/test/fixtures/stdio-mixed-content-server.tspackages/core/test/fixtures/stdio-server.tspackages/core/test/google-discovery.test.tspackages/core/test/graphql.test.tspackages/core/test/http-actions.test.tspackages/core/test/mcp-test-client.tspackages/core/test/media-artifacts.test.tspackages/core/test/native-remote.test.tspackages/core/test/native.test.tspackages/core/test/openapi.test.tspackages/core/test/project-binding-integration.test.tspackages/core/test/remote-control-dispatch.test.tspackages/core/test/result-content.test.tspackages/core/test/runtime.test.tspackages/core/test/serve-http.test.tspackages/core/test/serve-session.test.tspackages/core/test/telemetry-docs.test.tspackages/core/test/tools.test.tspackages/pi/src/index.tspackages/pi/test/pi.test.tspackages/web-observability/src/index.tspackages/web-observability/src/posthog.tspackages/web-observability/src/privacy.tspackages/web-observability/test/posthog-provider-contract.test.tspackages/web-observability/test/web-observability.test.ts
💤 Files with no reviewable changes (3)
- packages/core/test/telemetry-docs.test.ts
- packages/web-observability/src/privacy.ts
- packages/web-observability/test/web-observability.test.ts
Own accepted Caplet IDs, serialized updates, cleanup-last close, and atomic replacement behind one native lifecycle while preserving distinct Cloud and self-hosted failure policies. Reauthorize and serialize self-hosted lease mutations so stale sockets cannot revive terminal bindings.
ian-pascoe
force-pushed
the
worktree/green-harbor-e0e3
branch
from
2780afe to
3e58bd2
Compare
Close linked MCP transports symmetrically, preserve per-target catalog failure activity, and make media and privacy boundaries explicit. Harden Vault reveal race coverage and simplify redundant assertions and casts.
ian-pascoe
changed the title
Contributor
Author
Review disposition — df6879dAddressed the valid findings:
Verified but intentionally unchanged:
Validation: full |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/dashboard/src/components/DashboardApp.test.tsx`:
- Around line 217-219: Update the waitFor predicate in the DashboardApp test to
synchronize on the new confirm request by requiring the second "vault/reveal"
call, or clear dashboardApi.mock before triggering it and wait for the first
subsequent call. Ensure the predicate cannot pass due to the earlier reveal
request.
🪄 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: 0dc8226f-caa2-4f80-9734-b200686fb445
📒 Files selected for processing (26)
.changeset/calm-media-seam.mdapps/dashboard/src/components/DashboardApp.test.tsxapps/dashboard/src/components/DashboardApp.tsxdocs/architecture.mdpackages/core/src/caplet-sets.tspackages/core/src/cloud/client.tspackages/core/src/cloud/presence.tspackages/core/src/current-host/catalog-operations.tspackages/core/src/google-discovery/manager.tspackages/core/src/http/response.tspackages/core/src/media/artifacts.tspackages/core/src/native/project-binding-lifecycle.tspackages/core/src/native/service.tspackages/core/src/project-binding/workspaces.tspackages/core/src/serve/http.tspackages/core/test/cloud-presence.test.tspackages/core/test/current-host-administration.test.tspackages/core/test/mcp-test-client.tspackages/core/test/media-artifacts.test.tspackages/core/test/native-remote.test.tspackages/core/test/native.test.tspackages/core/test/project-binding-integration.test.tspackages/core/test/runtime.test.tspackages/core/test/serve-http.test.tspackages/web-observability/src/posthog.tspackages/web-observability/src/privacy.ts
💤 Files with no reviewable changes (1)
- packages/core/test/runtime.test.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/calm-media-seam.md
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/core/src/google-discovery/manager.ts
- packages/core/test/project-binding-integration.test.ts
- packages/core/test/mcp-test-client.ts
- docs/architecture.md
- packages/web-observability/src/posthog.ts
- packages/core/test/cloud-presence.test.ts
- packages/core/test/media-artifacts.test.ts
- packages/core/src/project-binding/workspaces.ts
- packages/core/src/cloud/client.ts
- packages/core/src/caplet-sets.ts
- packages/core/src/http/response.ts
- packages/core/test/current-host-administration.test.ts
- packages/core/src/current-host/catalog-operations.ts
- apps/dashboard/src/components/DashboardApp.tsx
- packages/core/src/media/artifacts.ts
- packages/core/src/native/project-binding-lifecycle.ts
- packages/core/src/cloud/presence.ts
- packages/core/test/native-remote.test.ts
- packages/core/src/native/service.ts
- packages/core/test/serve-http.test.ts
- packages/core/test/native.test.ts
- packages/core/src/serve/http.ts
Comment on lines
+217
to
+219
| await waitFor(() => | ||
| dashboardApi.mock.calls.some(([path]) => path === "vault/reveal") ? true : undefined, | ||
| ); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect confirmReveal / reveal helpers to confirm whether dispatch is awaited
rg -nP 'const (confirmReveal|reveal|openRevealConfirmation)\b' -A 15 apps/dashboard/src/components/DashboardApp.test.tsxRepository: spiritledsoftware/caplets
Length of output: 163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant test section and nearby helper definitions.
sed -n '180,240p' apps/dashboard/src/components/DashboardApp.test.tsx
printf '\n---\n'
rg -n 'confirmReveal|reveal|openRevealConfirmation|dashboardApi\.mock\.calls|toast\.error|waitFor' apps/dashboard/src/components/DashboardApp.test.tsxRepository: spiritledsoftware/caplets
Length of output: 4714
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '120,136p' apps/dashboard/src/components/DashboardApp.test.tsxRepository: spiritledsoftware/caplets
Length of output: 626
Make the wait gate on the second vault/reveal call. dashboardApi.mock.calls already contains the earlier reveal request, so this predicate is true before the new confirm request lands. Capture the call count or clear the mock first, otherwise this waitFor doesn’t synchronize anything and can hide a race.
🤖 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 `@apps/dashboard/src/components/DashboardApp.test.tsx` around lines 217 - 219,
Update the waitFor predicate in the DashboardApp test to synchronize on the new
confirm request by requiring the second "vault/reveal" call, or clear
dashboardApi.mock before triggering it and wait for the first subsequent call.
Ensure the predicate cannot pass due to the earlier reveal request.
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
1 participant
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.
Summary
Caplets now derives cross-host behavior from seven authoritative modules instead of letting each adapter reconstruct policy, callable identity, result locality, or lifecycle ordering. The refactor preserves the established CLI, MCP, Attach, dashboard, native, and public-site experiences while making failures generation-safe, authorization-safe, privacy-safe, and retryable.
inline,local-artifact, orremote-referenceunder shared byte, MIME, integrity, path-visibility, and transactional-write policysessionEndedpolicyDesign decisions
Cache-Control: no-store, ephemeral in-memory display, cancellation of stale responses, and no persistence across expiry, navigation, or refresh.distinct_id, approved categorical properties, and required privacy controls. The unified production/preview PostHog project was re-read with IP capture configured to Discard on 2026-07-10.Validation
pnpm verify— formatting, lint, generated Code Mode API, config schema, public docs, all package typechecks, 158 test files / 2,237 tests, deterministic benchmark freshness, all package builds, and runtime package checks passed.200response withCache-Control: no-store, verified automatic removal after 30 seconds, and verified no value persisted or flashed across Activity navigation or a full refresh.@caplets/coreChangeset.Summary by CodeRabbit