feat: Enclave VM Streaming Runtime #32
Conversation
📝 WalkthroughWalkthroughAdds a full streaming runtime: session subsystem in enclave-vm, broker, runtime worker, client SDK, React bindings, NDJSON streaming protocol with encryption/reconnect, embedded and lambda deployment flows, an end-to-end demo app, many tests, TypeScript/Jest/Nx scaffolding, HTTP adapters, event filtering, and extensive docs including architecture guidance. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Broker
participant Enclave
Client->>Broker: POST /sessions/embedded { code }
activate Broker
Broker->>Enclave: create session, run(code)
activate Enclave
Enclave->>Broker: session_init
Broker->>Client: stream session_init
Enclave->>Broker: tool_call(callId, toolName, args)
Broker->>Client: stream tool_call
Broker->>Broker: execute tool (local)
Broker->>Enclave: tool_result(callId, success, value)
Enclave->>Broker: tool_result_applied(callId)
Broker->>Client: stream tool_result_applied
Enclave->>Broker: final (ok/result)
Broker->>Client: stream final, close
deactivate Enclave
deactivate Broker
sequenceDiagram
participant Client
participant Broker
participant RuntimeWS as Runtime (WS)
participant Enclave
Client->>Broker: POST /sessions/lambda { code }
activate Broker
Broker->>RuntimeWS: open WS, send { execute, sessionId, code }
activate RuntimeWS
RuntimeWS->>Enclave: start session, run(code)
activate Enclave
Enclave->>RuntimeWS: tool_call
RuntimeWS->>Broker: forward tool_call
Broker->>Client: stream tool_call
Broker->>Broker: execute tool (local)
Broker->>RuntimeWS: tool_result(callId, result)
RuntimeWS->>Enclave: submitToolResult
Enclave->>RuntimeWS: tool_result_applied
RuntimeWS->>Broker: forward tool_result_applied
Broker->>Client: stream tool_result_applied
Enclave->>RuntimeWS: final
RuntimeWS->>Broker: forward final
Broker->>Client: stream final, close
deactivate Enclave
deactivate RuntimeWS
deactivate Broker
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @apps/streaming-demo/src/client-server.ts:
- Around line 325-329: The POST handler for '/api/execute' incorrectly calls
app.handle(req, res) (Express has no public handle method); instead extract the
logic from the '/api/execute/embedded' route into a reusable function (e.g.,
executeEmbeddedLogic or executeHandler) and have both the
'/api/execute/embedded' route and this '/api/execute' route call that function
directly, or call the embedded route's handler function directly; update the
'/api/execute' block to invoke that shared function with (req, res) rather than
attempting to forward via app.handle.
In @apps/streaming-demo/src/runtime-server.ts:
- Around line 134-144: The session_init payload sent via sendEvent is missing
the required cancelUrl field from SessionInitPayload; update the payload object
in the sendEvent call (the session_init branch) to include a cancelUrl property
(e.g., a valid URL string or computed cancel URL tied to sessionId) so the
payload satisfies SessionInitPayload's type requirements.
- Around line 288-292: The pending tool calls loop only logs callIds but never
rejects them, leaving promises hanging; update the loop over
session.pendingToolCalls to invoke each resolver with a rejection (e.g., call
resolver.reject(new Error(`Tool call cancelled: ${callId}`)) or otherwise call
the rejection callback on the resolver object) before clearing the map, keep the
existing log, and ensure callId and resolver from session.pendingToolCalls are
used so each pending promise is rejected prior to
session.pendingToolCalls.clear().
- Around line 87-104: handleMessage is unsafely casting the loose input to
specific request types (ExecuteRequest, ToolResultRequest, CancelRequest); add
runtime validation/type-guards before calling handlers: implement and use type
predicate functions (e.g., isExecuteRequest, isToolResultRequest,
isCancelRequest) that check the presence and types of required fields for
ExecuteRequest, ToolResultRequest and CancelRequest, and only call
handleExecute, handleToolResult, or handleCancel when the guard returns true;
otherwise send an error/panic response or log and ignore the message to avoid
unsafe assertions.
In @libs/enclavejs-broker/package.json:
- Around line 24-34: Update the package.json entry points so they reference the
built output under dist: change the "main" value to "./dist/src/index.js", the
"types" value to "./dist/src/index.d.ts", and update every export condition
(development, types, import, default) to point to "./dist/src/..." instead of
"./src/..."; also ensure your build output matches this by aligning
tsconfig.lib.json's outDir (or build config) so compiled JS and declaration
files are emitted to the dist/src location referenced by those package.json
keys.
In @libs/enclavejs-broker/src/broker-session.ts:
- Around line 139-147: The code reads heartbeatIntervalMs from this.limits but
that property lives on BrokerSessionConfig; update BrokerSession to use the
config value instead of limits by wiring the config into the instance (e.g.,
store the passed BrokerSessionConfig in this.config in the constructor) and
change the heartbeat setup to read this.config.heartbeatIntervalMs (or
alternatively add heartbeatIntervalMs to the SessionLimits object when
constructing this.limits); ensure heartbeatInterval is still set via setInterval
and that emitter.emitHeartbeat and isTerminal() usage remains unchanged.
In @libs/enclavejs-broker/src/http/express-adapter.ts:
- Around line 156-173: The router methods in createExpressRouter are currently
no-ops because they only reassign the local parameter; change them to actually
register handlers by mutating the routes array: in get/post/delete/options, if a
matching route exists set its handler property (route.handler = routeHandler),
otherwise push a new route object ({ method: 'get'|'post'|'delete'|'options',
path, handler: routeHandler }) into routes. Alternatively, remove this helper
and call registerExpressRoutes where needed; but if you keep
createExpressRouter, ensure the returned methods update the shared routes array
(or register onto an internal Express Router) instead of reassigning the
parameter.
In @libs/enclavejs-react/src/types.ts:
- Around line 145-149: The file uses React.ReactNode for the children type but
React is not imported; fix by importing the type from React and/or changing the
declaration to use the imported symbol: add an import like `import type {
ReactNode } from 'react'` (or `import React from 'react'`) and update the
`children: React.ReactNode` line in the types declaration to `children:
ReactNode` (or keep React.ReactNode if you imported default React); this
resolves the TypeScript compilation error for the children property in the
types.ts declaration.
In @libs/enclavejs-runtime/package.json:
- Around line 25-32: The exports map uses the "import" conditional while
package.json declares "type": "commonjs", causing a mismatch; either replace the
"import" entry with a "require" entry in the exports map so consumers using
CommonJS resolve to "./dist/src/index.js", or convert to a dual-publish layout
by adding both "import" and "require" keys (and updating package "type" or
providing "exports" entries for both formats) so "./" in exports contains
matching "types", "import" (ESM) and "require" (CJS) entries; update the exports
object around the "." key accordingly to ensure consistency with the package
"type" field.
- Around line 41-43: The package "ws" is needed at runtime by WebSocketChannel
and createWebSocketChannel (used in
libs/enclavejs-runtime/src/channels/websocket-channel.ts) but is listed under
devDependencies; move "ws": "^8.0.0" from devDependencies into dependencies in
package.json (preserve the version), update the lockfile by reinstalling
(npm/yarn pnpm install) and verify builds/tests to ensure the runtime require
succeeds in Node.js environments.
- Around line 36-40: The package.json currently lists "ws" under devDependencies
but it is required at runtime and re-exported by your public API (see
websocket-channel.ts and symbols WebSocketServer and WebSocketChannel); move
"ws" from devDependencies into dependencies in package.json so consumers
installing the SDK get the runtime package, ensuring the exact version currently
declared under devDependencies is copied into the dependencies section and
removed from devDependencies.
In @libs/enclavejs-stream/project.json:
- Around line 1-34: The project.json for the enclavejs-stream library lacks a
"test" target; add a "test" target entry under "targets" using the "@nx/js:jest"
executor with outputs pointing to
"{workspaceRoot}/coverage/libs/enclavejs-stream" and options specifying
"jestConfig": "libs/enclavejs-stream/jest.config.ts" (mirror the pattern used in
other libs like enclavejs-broker), and ensure a corresponding jest.config.ts
exists in the library root and references the tsconfig.spec.json; update
project.json's "targets" block to include the new "test" target so Nx can run
Jest for this library.
🟡 Minor comments (16)
libs/enclavejs-runtime/package.json-33-35 (1)
33-35: Set executable permissions on the CLI source file.The CLI entry point has the proper shebang (
#!/usr/bin/env node), but the source file atlibs/enclavejs-runtime/src/cli.tsneeds executable permissions (currently 644) for proper development and local installation. Set it to 755.README-ARCHITECTURE.md-13-24 (1)
13-24: Implementation status table is inconsistent with the PR content.The table shows
@enclavejs/client,@enclavejs/runtime, and@enclavejs/reactas "🔲 Planned", but based on the AI summary and file changes in this PR, these packages have code being added (e.g.,libs/enclavejs-react/with hooks, provider, and tests).📝 Suggested fix
| `@enclavejs/broker` | ✅ Done | Middleware/tool broker with HTTP API | -| `@enclavejs/client` | 🔲 Planned | Browser + Node client SDK | -| `@enclavejs/runtime` | 🔲 Planned | Extracted runtime worker | -| `@enclavejs/react` | 🔲 Planned | React hooks | +| `@enclavejs/client` | ✅ Done | Browser + Node client SDK | +| `@enclavejs/runtime` | ✅ Done | Extracted runtime worker | +| `@enclavejs/react` | ✅ Done | React hooks |README-ARCHITECTURE.md-662-662 (1)
662-662: Minor grammar fix needed.-- [ ] Should middleware resolve refs automatically for browser clients (size/policy based)? +- [ ] Should middleware resolve refs automatically for browser clients (size/policy-based)?libs/enclave-vm/src/session/session-state-machine.ts-154-170 (1)
154-170: Remove the unused 'start' event type or clarify its intended use.The
startevent maps to thestartingstate, butstartingis the initial state with no valid incoming transitions defined inVALID_TRANSITIONS. This means callingtransition({ type: 'start' })would always fail with an "Invalid state transition" error. The event is defined but unused in the codebase and appears to be dead code.libs/enclavejs-client/src/client.ts-161-173 (1)
161-173: Heartbeat timeout uses hardcoded default instead of configured value.The
HeartbeatMonitorusesDEFAULT_CONFIG.heartbeatTimeoutMsbut ignores any user-provided timeout configuration. If users can configure heartbeat intervals vialimits.heartbeatIntervalMs, they may also expect to configure the timeout.Suggested fix
// Create heartbeat monitor const heartbeatMonitor = new HeartbeatMonitor({ - timeoutMs: DEFAULT_CONFIG.heartbeatTimeoutMs, + timeoutMs: options.limits?.heartbeatIntervalMs + ? options.limits.heartbeatIntervalMs * 3 // Reasonable multiplier + : DEFAULT_CONFIG.heartbeatTimeoutMs, onTimeout: () => {Or consider adding
heartbeatTimeoutMsto the client config.libs/enclavejs-runtime/src/cli.ts-112-113 (1)
112-113: Unhandled promise rejection in signal handlers.The
shutdownfunction is async but its returned promise is not handled. Ifworker.stop()throws, the error will be an unhandled rejection.Proposed fix
- process.on('SIGINT', () => shutdown('SIGINT')); - process.on('SIGTERM', () => shutdown('SIGTERM')); + process.on('SIGINT', () => shutdown('SIGINT').catch(console.error)); + process.on('SIGTERM', () => shutdown('SIGTERM').catch(console.error));libs/enclavejs-react/src/useEnclaveSession.ts-117-140 (1)
117-140: Potential duplicateonErrorinvocation.The
onErrorcallback may be invoked twice for the same error:
- Line 118: Called during streaming via the handle's
onErrorcallback- Line 140: Called again after
handle.wait()resolves with an error resultThis could confuse consumers expecting a single error notification per failure.
🐛 Suggested fix: avoid duplicate onError calls
Consider tracking whether an error was already reported during streaming:
+ let streamErrorReported = false; const handle = client.executeStream(code, { // ... onError: (code, message) => { + streamErrorReported = true; options.onError?.({ code, message }); }, }); // ... } else { setState('error'); setResult(sessionResult); setError(sessionResult.error ?? { message: 'Unknown error' }); - options.onError?.(sessionResult.error ?? { message: 'Unknown error' }); + if (!streamErrorReported) { + options.onError?.(sessionResult.error ?? { message: 'Unknown error' }); + } }libs/enclavejs-runtime/src/integration.spec.ts-139-154 (1)
139-154: Test doesn't verify bidirectional message passing.This test sets up
onMessagehandlers for both client and server but never callssend()to verify messages are actually forwarded between them. The test only asserts that both channels are open, which doesn't validate bidirectional communication.🔧 Suggested improvement
it('should enable bidirectional communication', () => { const { client, server } = createMemoryChannelPair(); const clientReceived: unknown[] = []; const serverReceived: unknown[] = []; client.onMessage((msg) => clientReceived.push(msg)); server.onMessage((msg) => serverReceived.push(msg)); - // Note: The channel pair forwards messages, so we need to send - // directly and check if messages propagate + // Test client -> server + client.send({ test: 'from client' }); + expect(serverReceived).toContainEqual({ test: 'from client' }); + + // Test server -> client + server.send({ test: 'from server' }); + expect(clientReceived).toContainEqual({ test: 'from server' }); expect(client.isOpen).toBe(true); expect(server.isOpen).toBe(true); });libs/enclavejs-runtime/src/integration.spec.ts-38-46 (1)
38-46: Weak assertion may not verify intended behavior.The assertion
expect(sessions.length).toBeGreaterThanOrEqual(0)on line 43 will always pass since array length is never negative. Consider asserting a more specific expectation based on session lifecycle:🔧 Suggested fix
// Wait for session to process await new Promise((resolve) => setTimeout(resolve, 100)); // Verify session was created const sessions = worker.getSessions(); - expect(sessions.length).toBeGreaterThanOrEqual(0); + // Session may be active or completed depending on timing + expect(sessions.length).toBeLessThanOrEqual(1); await worker.stop();libs/enclavejs-broker/src/http/session-handler.spec.ts-307-325 (1)
307-325: Test name doesn't match behavior.The test is named "should replay past events for completed session" but actually verifies the 404 behavior for a non-existent session. The test uses
's_completed'as the session ID (line 315), which never existed - the actual session created bybroker.execute()has a different auto-generated ID and is removed after completion.Consider either:
- Renaming the test to reflect what it actually tests (404 for removed sessions), or
- Modifying the broker to retain session history for replay and testing actual replay behavior.
🔧 Suggested rename
- it('should replay past events for completed session', async () => { + it('should return 404 for session removed after completion', async () => { // Create and execute a session const result = await broker.execute('return 42'); expect(result.success).toBe(true); - // The session is auto-removed after completion in broker.execute() - // So this test verifies the 404 case for completed sessions + // Sessions are auto-removed after completion in broker.execute() const req = createRequest({ params: { sessionId: 's_completed' }, query: { fromSeq: '0' }, });libs/enclavejs-broker/src/http/session-handler.ts-268-304 (1)
268-304: Potential race condition between replaying past events and subscribing.Between
session.getEvents()(Line 269) andsession.onEvent()(Line 283), new events could be emitted and missed. Consider subscribing first, then replaying past events, and deduplicating based onseq.Additionally, the abort event listener (Lines 296-304) should be cleaned up when the session completes normally to prevent potential memory leaks.
apps/streaming-demo/src/broker-server.ts-280-287 (1)
280-287: WebSocket message parsing lacks validation.The message handler parses JSON and casts directly to
StreamEventwithout validation. Malformed messages could cause unexpected behavior:const message = JSON.parse(data.toString()) as StreamEvent;Consider adding basic structure validation using Zod (already imported) or at minimum type guards.
libs/enclavejs-broker/src/broker-session.ts-342-351 (1)
342-351: Redundant duration calculation and incorrectendTimelogic.Line 345 computes
now - this.createdAtin both branches, making the ternary unnecessary. Additionally,endTimeshould capture the actual completion time once the session terminates, not recalculate it on everygetStats()call.🐛 Proposed fix
Store actual end time when session terminates:
+ private endedAt = 0; + // In runExecution finally block or terminal state transitions: + this.endedAt = Date.now(); getStats(): ExecutionStats { const now = Date.now(); return { - duration: this.isTerminal() ? now - this.createdAt : now - this.createdAt, + duration: now - this.createdAt, toolCallCount: this.toolCallCount, iterationCount: 0, startTime: this.createdAt, - endTime: this.isTerminal() ? now : 0, + endTime: this.endedAt, }; }libs/enclavejs-broker/src/index.ts-9-43 (1)
9-43: Add.jsextensions to relative imports for ESM compatibility with NodeNext module resolution.The file uses bare relative imports (e.g.,
'./broker'), but other files in the@enclavejs/*packages (likelibs/enclavejs-stream,libs/enclavejs-client, andlibs/enclavejs-react) use explicit.jsextensions (e.g.,'./ndjson.js'). With the project'sNodeNextmodule resolution, explicit extensions are required. Update all relative imports:-export { Broker, createBroker } from './broker'; +export { Broker, createBroker } from './broker.js';Apply the same change to all other relative imports:
./tool-registry,./broker-session,./session-manager, and./http.libs/enclavejs-client/src/client.spec.ts-184-194 (1)
184-194: Floating promise in test.The
client.execute('return 1')call on line 191 is not awaited, which means the test assertion on line 193 may run before the fetch is actually called.Proposed fix
- client.execute('return 1'); + await client.execute('return 1');libs/enclavejs-stream/src/crypto/aes-gcm.ts-117-122 (1)
117-122: Incorrect error code for nonce validation in encrypt.Lines 117-122 use
EncryptionErrorCode.DecryptionFailedfor an encryption-related nonce validation error. This is semantically incorrect.💡 Use appropriate error code
Consider using or adding an
EncryptionFailederror code for encryption-specific errors.
…ages and React integration examples
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @README-ARCHITECTURE.md:
- Around line 15-23: Update the packages table to match the implementation
status in the status table by removing the "(Planned)" suffix for
@enclavejs/client, @enclavejs/runtime, and @enclavejs/react so the descriptions
no longer indicate they are planned; ensure the package entries for
`@enclavejs/client`, `@enclavejs/runtime`, and `@enclavejs/react` read as the
final descriptions (without "(Planned)") to keep both tables consistent.
In @README.md:
- Around line 19-21: Remove or defer the npm badge and install references for
the unpublished packages by deleting or commenting out the npm badge lines that
reference @enclavejs/broker, @enclavejs/client, and @enclavejs/react and
removing or replacing the installation instructions that run npm/yarn add
@enclavejs/broker @enclavejs/client @enclavejs/react with a short placeholder
note such as “Package will be published to npm — install instructions coming in
a future release” so readers don’t hit 404s.
🧹 Nitpick comments (1)
README-ARCHITECTURE.md (1)
82-82: Optional: Add language specifiers to ASCII art code blocks.The markdown linter flags these fenced code blocks as missing language specifiers. While this is a minor formatting issue, you could add
textorplaintextas the language identifier for ASCII art diagrams to satisfy linters.Example fix
-``` +```text ┌─────────────────────────────────────────────────────────────────────────────┐ │ Your Application │ ...Based on learnings, this is a nitpick-level suggestion that can be safely deferred.
Also applies to: 124-124, 202-202, 222-222, 247-247, 332-332, 551-551
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README-ARCHITECTURE.mdREADME.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-08T18:40:23.681Z
Learnt from: CR
Repo: agentfront/enclave PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T18:40:23.681Z
Learning: Enforce global coverage threshold for `enclave-vm` as configured in `libs/enclave-vm/jest.config.ts`
Applied to files:
README.md
🪛 markdownlint-cli2 (0.18.1)
README-ARCHITECTURE.md
82-82: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
124-124: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
202-202: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
222-222: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
247-247: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
332-332: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
551-551: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (8)
README-ARCHITECTURE.md (5)
118-364: Excellent deployment scenarios documentation.The four deployment scenarios are well-structured and clearly demonstrate the architecture's flexibility. The security principle of keeping secrets in the middleware/broker is consistently applied across all scenarios, and the code examples properly demonstrate the separation of concerns.
397-503: Well-designed streaming protocol.The protocol includes essential features for production use:
- Protocol versioning for future evolution
- Monotonic sequence numbers for ordering and gap detection
- Comprehensive event types covering the full session lifecycle
- NDJSON format is a pragmatic choice that simplifies encryption compared to SSE
506-611: Strong tool system and reference sidecar design.The tool registration API with Zod validation provides excellent type safety and the explicit secrets declaration pattern ensures proper access control. The reference sidecar with AST-based auto-ref is an elegant solution to avoid moving large/sensitive payloads through the runtime when they're only passed through to other tools.
614-636: Solid security architecture.The layered security approach is well-designed:
- TLS baseline with proper authentication
- Clear authorization model treating sessionId as a routing key
- Optional per-session encryption using modern cryptography (ECDH, HKDF, AES-GCM)
- Replay protection through sequence numbers and nonces
This provides defense in depth while keeping the optional encryption practical.
639-664: Helpful platform guidance and thoughtful open questions.The platform capability matrix provides clear guidance for deployment decisions, and the recommendations are practical. The open questions section appropriately captures known tradeoffs (NDJSON vs WebSocket, ref resolution UX) that can be addressed as the system evolves.
README.md (3)
97-129: Excellent React integration example.The example demonstrates proper usage patterns:
- Correct provider setup with
brokerUrl- Proper hook usage with destructured state
- Good UX practices (disabled button during execution, displaying stdout and results)
- Clear and practical code that developers can adapt
131-154: Clear architecture reference and practical demo instructions.The architecture section provides a helpful overview and directs readers to detailed documentation. The demo section offers a simple command to run the full system and clearly explains the three-server architecture, reinforcing the security design principle of separation between client, broker, and runtime.
32-39: Excellent value proposition in "Why Enclave?" section.The benefits are clearly articulated and align well with the architecture. The reference to the security audit adds credibility, and the linked document exists and is properly maintained.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @libs/enclavejs-broker/src/event-filter.ts:
- Around line 217-256: The code currently compiles and executes user-provided
regexes directly in compilePatterns and matchRegex, ignoring
EventFilterConfig.regexTimeoutMs; fix by validating and wrapping regexes with
the existing safe utilities: in compilePatterns use
isPotentiallyVulnerableRegex(pattern.pattern) to skip or log risky patterns and
call createSafeRegex(pattern.pattern, flags, this.config.regexTimeoutMs) instead
of new RegExp(...), storing the returned safe regex object in compiledRegexes;
in matchRegex use safeTest(value, compiled.patternOrWrapper, maxLength) or the
safe regex wrapper's test method (which enforces input length and timeout)
rather than new RegExp(...). Ensure any fallback paths respect regexTimeoutMs
and that compiledRegexes stores whatever wrapper object createSafeRegex returns
so matchRegex can invoke it safely.
In @libs/enclavejs-broker/src/http/session-handler.ts:
- Around line 270-271: The parsed fromSeq can be NaN (from
req.query['fromSeq']), which breaks the event filtering; update the parsing of
fromSeq in session-handler.ts to validate the Number (e.g., const fromSeq =
parseInt(...); if (!Number.isFinite(fromSeq) || Number.isNaN(fromSeq)) return a
400 Bad Request with an explanatory message) or, alternatively, coerce invalid
values to a safe default (0) before using it in the event.seq >= fromSeq
comparison; ensure you reference and validate req.query['fromSeq'] and the
fromSeq variable so events are not silently skipped.
🧹 Nitpick comments (4)
.husky/pre-commit (1)
15-17: Valid safety improvement, but consider a more efficient approach.The file existence check correctly handles edge cases where
--diff-filter=ACMRpasses a file that no longer exists on disk. However, spawning a subshell per file withxargs -I {} sh -c '...'is inefficient for large changesets.♻️ Consider a single-process filter
-git diff --name-only -z --diff-filter=ACMR main... | grep -zE '\.(js|jsx|ts|tsx|json|css|scss|md|html|yml|yaml)$' | xargs -0 -I {} sh -c '[ -f "$1" ] && printf "%s\0" "$1"' _ {} | xargs -0 -r npx prettier --check +git diff --name-only -z --diff-filter=ACMR main... | grep -zE '\.(js|jsx|ts|tsx|json|css|scss|md|html|yml|yaml)$' | while IFS= read -r -d '' file; do [ -f "$file" ] && printf '%s\0' "$file"; done | xargs -0 -r npx prettier --checkAlternatively, if available, use
xargswith a filter script file rather than inline shell invocation.libs/enclavejs-broker/src/http/session-handler.ts (3)
89-103: Unexpected CORS fallback behavior when origin doesn't match.When a request's
Originheader doesn't match any allowed origin, the code falls back toallowedOrigins[0](line 97). This means the response will include anAccess-Control-Allow-Originheader for an origin that wasn't actually requested, which browsers will reject anyway. However, this fallback could be confusing and might leak information about configured origins.Consider returning no CORS header (or denying) when the origin doesn't match:
♻️ Suggested improvement
private addCorsHeaders(req: BrokerRequest, res: BrokerResponse): void { if (!this.cors) return; const origin = req.headers['origin'] as string | undefined; - const allowedOrigin = this.allowedOrigins.includes('*') - ? '*' - : this.allowedOrigins.includes(origin ?? '') - ? origin - : this.allowedOrigins[0]; - - res.setHeader('Access-Control-Allow-Origin', allowedOrigin ?? '*'); + if (this.allowedOrigins.includes('*')) { + res.setHeader('Access-Control-Allow-Origin', '*'); + } else if (origin && this.allowedOrigins.includes(origin)) { + res.setHeader('Access-Control-Allow-Origin', origin); + res.setHeader('Vary', 'Origin'); + } else { + // Origin not allowed - don't set CORS headers + return; + } res.setHeader('Access-Control-Allow-Methods', 'GET, POST, DELETE, OPTIONS'); res.setHeader('Access-Control-Allow-Headers', 'Content-Type, Accept, Last-Event-ID'); res.setHeader('Access-Control-Max-Age', '86400'); }Note: Adding
Vary: Originis important when reflecting specific origins to ensure proper caching behavior.
198-220: Potential double-unsubscribe and cleanup handler not removed on normal completion.The
cleanupfunction (line 199-206) and thefinallyblock (line 217-220) both callunsubscribe(). If the client disconnects during execution,unsubscribe()will be called twice. Also, the abort listener isn't removed when the session completes normally, leaving a dangling reference.Consider consolidating cleanup logic:
♻️ Suggested cleanup pattern
// Handle client disconnect + let cleanedUp = false; const cleanup = () => { + if (cleanedUp) return; + cleanedUp = true; unsubscribe(); if (!session.isTerminal()) { session.cancel('Client disconnected').catch(() => { // Ignore cancellation errors }); } }; if (req.signal) { req.signal.addEventListener('abort', cleanup, { once: true }); } // Start execution try { await session.execute(body.code); } catch { // Error already emitted through events } finally { - unsubscribe(); + cleanup(); res.end(); }
382-396: Consider tracking toolCallCount in SessionInfo.Line 393 hardcodes
toolCallCount: 0because it's unavailable fromSessionInfo. This creates an inconsistency betweensessionToInfo()(which has accurate stats) andsessionInfoToResponse(). If this field is important for API consumers, consider enrichingSessionInfoin a follow-up.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.husky/pre-commit.prettierignorelibs/enclavejs-broker/package.jsonlibs/enclavejs-broker/src/event-filter.tslibs/enclavejs-broker/src/http/session-handler.tslibs/enclavejs-broker/src/http/types.tslibs/enclavejs-broker/src/index.tslibs/enclavejs-types/src/filter.tslibs/enclavejs-types/src/index.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/enclavejs-broker/package.json
- libs/enclavejs-broker/src/index.ts
- libs/enclavejs-broker/src/http/types.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use ESLint flat config with unused parameters and variables prefixed with underscore_
Prefer workspace import paths (e.g.,import { validate } from 'ast-guard') over deep cross-library relative imports
Files:
libs/enclavejs-broker/src/event-filter.tslibs/enclavejs-broker/src/http/session-handler.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/enclavejs-broker/src/event-filter.tslibs/enclavejs-broker/src/http/session-handler.ts
🧠 Learnings (2)
📚 Learning: 2026-01-08T18:40:23.681Z
Learnt from: CR
Repo: agentfront/enclave PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T18:40:23.681Z
Learning: Use Prettier for code formatting with 120 column limit, single quotes, and trailing commas
Applied to files:
.husky/pre-commit
📚 Learning: 2026-01-08T18:40:23.681Z
Learnt from: CR
Repo: agentfront/enclave PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T18:40:23.681Z
Learning: Do not commit generated artifacts: `coverage/`, `dist/`, `tmp/` directories
Applied to files:
.prettierignore
🧬 Code graph analysis (1)
libs/enclavejs-broker/src/event-filter.ts (1)
libs/enclavejs-types/src/filter.ts (9)
EventFilterConfig(86-101)DEFAULT_ALWAYS_ALLOW(106-106)FilterMode(14-19)FilterMode(21-21)FilterRule(76-81)ContentFilter(64-69)ContentPattern(42-51)PatternType(26-35)PatternType(37-37)
🪛 ast-grep (0.40.4)
libs/enclavejs-broker/src/event-filter.ts
[warning] 226-226: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern, flags)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 247-247: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern.pattern, flags)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (16)
.husky/pre-commit (1)
1-25: Overall script logic is sound.The hook correctly:
- Falls back gracefully when
mainbranch doesn't exist- Filters files by extension with null-termination to handle special characters
- Captures Prettier's exit code and fails fast if formatting issues are found
- Runs
lint-stagedas a final step.prettierignore (1)
7-9: Good practice to exclude non-yarn lock files from formatting.Ignoring package-lock.json and pnpm-lock.yaml from Prettier is appropriate for this monorepo standardized on yarn. This prevents unnecessary formatting changes and potential merge conflicts on auto-generated files.
libs/enclavejs-broker/src/http/session-handler.ts (7)
1-48: LGTM!The imports are well-organized, and the
SessionHandlerConfiginterface provides clear documentation with sensible defaults. TheRouteHandlertype signature correctly represents async route handlers.
50-84: LGTM!The class structure is clean, and
getRoutes()provides a well-organized route registry. Using.bind(this)ensures proper context when handlers are registered with external frameworks.
113-132: LGTM!Simple and clean implementation with proper error handling.
227-248: LGTM!Clean implementation with proper parameter validation and 404 handling.
309-328: Logic is correct but could be clearer.The dual handling of the
finalevent (lines 314-318 for filtered case, lines 323-327 for unfiltered case) works correctly due to the earlyreturnat line 318. Consider consolidating thefinalevent handling at the end of the callback for readability, but this is optional.
342-363: LGTM!Clean implementation with proper async handling and error responses.
398-422: LGTM!The utility methods and factory function are clean and well-implemented. Good use of optional chaining for
res.flush.libs/enclavejs-broker/src/event-filter.ts (7)
1-41: LGTM!Clean imports and well-documented interfaces. The
onErrorcallback provides a good error handling hook for consumers.
50-64: LGTM!Good performance optimization with pre-compiled regex patterns and
Setfor O(1) lookup of always-allowed event types.
66-93: LGTM!Clear and correct filtering logic. The include/exclude mode semantics are well-implemented.
105-117: Empty rule matches all events - verify if intentional.If a
FilterRulehas neithertypesnorcontent, bothtypeMatchandcontentMatchdefault totrue(lines 107-108), causing the rule to match every event. This might be intentional for a "catch-all" rule, but could also lead to unexpected behavior if rules are accidentally created without filters.
139-177: LGTM!Pattern matching is well-implemented with proper handling for all pattern types and robust error handling via the
onErrorcallback.
179-215: LGTM!Safe field extraction with proper null/undefined handling and sensible default behavior.
258-294: LGTM!Clean factory functions providing convenient APIs for common filtering patterns.
…aDB and ast-guard
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @docs/api-reference/enclave-vm.mdx:
- Around line 207-213: Update the ScoringGateOptions.scorer union to match the
implementation: replace the current "'rule-based' | 'custom'" with "'disabled' |
'rule-based' | 'local-llm' | 'external-api' | 'progressive'"; ensure the
interface name ScoringGateOptions and the scorer property are updated in the API
reference so it aligns with the source type used in the scoring implementation
(e.g., the types declared alongside ScoringResult).
In @docs/api-reference/vectoriadb.mdx:
- Around line 412-418: The example assumes db.get returns a Document but it can
return null; add a null check after calling get() (check that the returned value
assigned to doc is not null) before accessing doc.metadata or calling db.update.
Update the snippet around the get()/update() example so it guards on the result
(e.g., if (!doc) return or throw) and only then spreads doc.metadata into the
metadata update passed to db.update.
In @docs/core-libraries/ast-guard/custom-rules.mdx:
- Around line 226-237: The example uses an invalid JavaScript token `[...]` for
the UnknownGlobalRule's allowedGlobals, which will confuse readers; replace
`[...]` with a valid placeholder such as an explicit empty array `[]`, a short
example list like `['window', 'console']`, or an inline comment `/* ... */` so
the UnknownGlobalRule({ allowedGlobals: ... }) expression is syntactically
correct while still indicating truncation; ensure the change is applied in the
snippet that constructs JSAstValidator and references UnknownGlobalRule and
allowedGlobals.
In @docs/core-libraries/enclave-vm/worker-pool.mdx:
- Around line 65-84: Add the missing optional callback properties to the
WorkerPoolConfig interface: declare onWorkerRecycled?: (workerId: string) =>
void and onStatsUpdate?: (stats: WorkerPoolStats) => void (or the appropriate
stats type used elsewhere), and ensure WorkerPoolStats is imported/defined or
replace with the correct type; make them optional to match the examples in the
"Monitoring Worker Health" section so the interface aligns with the documented
usage.
In @docs/enclavejs/runtime.mdx:
- Around line 296-316: The examples incorrectly call non-existent RuntimeWorker
methods (executeSync and getState); update the Lambda handler example to use the
actual channel-based request pattern instead of executeSync by creating a
request channel and invoking the RuntimeWorker.handleRequest() flow used by the
"Session Executor" section (retain createRuntimeWorker, worker.start(), and
sessionId/code/limits extraction), and update the API docs to list only the real
RuntimeWorker members (start(), stop(), getStats(), getSessions(), isRunning)
with a note pointing readers to the handleRequest() pattern for executing code
rather than suggesting executeSync or getState.
In @docs/examples/encrypted-sessions.mdx:
- Around line 233-235: The method signature uses invalid TypeScript for an async
generator; change the function declaration for executeStreaming to use the
async* generator syntax (async * executeStreaming(...):
AsyncGenerator<StreamEvent, void, unknown>) so it returns an async generator of
StreamEvent and update the implementation body to yield values accordingly.
In @docs/examples/tool-calling.mdx:
- Around line 47-48: The callback for users.map currently reuses the name
userOrders and thereby shadows the outer const userOrders; update the inner
variable used in orders.filter (inside the users.map callback) to a distinct
name such as ordersForUser or userOrdersForUser to avoid shadowing and make the
intent clear (i.e., replace the inner userOrders from orders.filter(o =>
o.userId === user.id) with a non-conflicting identifier).
In @docs/getting-started/welcome.mdx:
- Line 100: Update the table cell text "Production Ready Security" to use
consistent hyphenation as "Production-ready Security" and audit other table
entries to apply the same hyphenation convention where compound modifiers appear
(e.g., ensure "feature-ready", "production-ready", or similar compounds match
across the table).
- Around line 3-6: Update the sentence that currently reads "Enclave is a
production ready JavaScript sandbox for executing AI-generated code safely." to
hyphenate the compound adjective by changing "production ready" to
"production-ready" so it reads "Enclave is a production-ready JavaScript sandbox
for executing AI-generated code safely."
🧹 Nitpick comments (11)
docs/changelog.mdx (1)
8-84: Consider including package names in version labels for clarity.The changelog mixes versions from different packages (monorepo root, enclave-vm, ast-guard) in chronological order. While the tags clarify the package, readers scanning quickly might find it confusing that v2.7.0 appears after v2.0.0.
Consider using labels like
enclave-vm v2.7.0orast-guard v2.4.0to make package ownership immediately visible:-<Update label="v2.7.0" description="2025-12-15" tags={["enclave-vm"]}> +<Update label="enclave-vm v2.7.0" description="2025-12-15" tags={["enclave-vm"]}>docs/enclavejs/runtime.mdx (1)
267-288: Consider updating Docker Compose version syntax.The
version: '3.8'key is deprecated in Docker Compose v2+ and is now optional. While this still works, modern Docker Compose files typically omit the version field entirely.Suggested update
-version: '3.8' - services: broker:docs/api-reference/enclavejs-runtime.mdx (1)
365-381: Consider clearing the status interval on shutdown.The
setIntervalfor status logging (line 373) isn't cleared in the SIGTERM handler, which could delay graceful shutdown. Consider adding cleanup:+ const statusInterval = setInterval(() => { - setInterval(() => { const status = runtime.getStatus(); logger.info({ activeExecutions: status.activeExecutions, totalExecutions: status.totalExecutions, workers: status.workers, }, 'Runtime status'); }, 60000); + + // Graceful shutdown + process.on('SIGTERM', async () => { + logger.info('Shutting down...'); + clearInterval(statusInterval); + await runtime.disconnect(); + process.exit(0); + });docs/examples/streaming-ui.mdx (2)
403-412: Add type assertion for error in catch block.The
errorin the catch block is of typeunknownin TypeScript. Consider adding a type guard or assertion for consistency with the React example (line 101):} catch (error) { - res.write(`event: error\ndata: ${JSON.stringify({ message: error.message })}\n\n`); + res.write(`event: error\ndata: ${JSON.stringify({ message: (error as Error).message })}\n\n`); } finally {
423-425: Consider handling potential null response body.The non-null assertion on
response.body!could fail if the response doesn't have a body. Consider adding a check:+ if (!response.body) { + throw new Error('Response body is null'); + } const reader = response.body!.getReader();This makes the example more robust for production use.
docs/guides/first-agent.mdx (1)
171-177: Consider clarifying the prompt structure.Using the
systemrole for the entire prompt (including user request) is unconventional. Most LLM patterns separate the system instructions from the user message. This works but may confuse readers familiar with standard patterns.💡 Alternative prompt structure
const completion = await openai.chat.completions.create({ model: 'gpt-4', messages: [ - { role: 'system', content: buildPrompt(userRequest) }, + { role: 'system', content: buildSystemPrompt() }, + { role: 'user', content: userRequest }, ], temperature: 0, });This would require splitting
buildPromptinto a system prompt (with rules and tools) and passing the user request separately.docs/examples/encrypted-sessions.mdx (1)
256-256: Add null check for response body.The
response.bodycan be null. Consider adding a null check before accessing the reader.📝 Suggested fix
- const reader = response.body!.getReader(); + if (!response.body) { + throw new Error('Response body is null'); + } + const reader = response.body.getReader();docs/core-libraries/vectoriadb/overview.mdx (1)
89-94: Consider adding TF-IDF to the Related links.Based on the PR context, there's a
tfidf.mdxdocumentation page in the same directory. Consider adding it to maintain consistency with the complete documentation suite.📝 Suggested addition
## Related - [Indexing](/core-libraries/vectoriadb/indexing) - Adding and updating documents - [Search](/core-libraries/vectoriadb/search) - Querying the index - [Persistence](/core-libraries/vectoriadb/persistence) - Storage adapters - [HNSW](/core-libraries/vectoriadb/hnsw) - Scaling to large datasets +- [TF-IDF](/core-libraries/vectoriadb/tfidf) - Hybrid search with keyword matchingdocs/getting-started/installation.mdx (2)
164-176: Hardcoded version numbers will become stale.This version table will require manual updates with every release. Consider either:
- Removing specific versions and directing users to npm/package.json for current versions
- Using a dynamic approach if Mintlify supports it
- Adding a "last updated" note to set expectations
134-142: Unused hook return values in example.The
stdoutandresultvalues fromuseEnclaveSession()are destructured but not used in the example. Consider either removing them or demonstrating their usage to provide a more complete example.📝 Option A: Remove unused values
function CodeRunner() { - const { execute, stdout, result } = useEnclaveSession(); + const { execute } = useEnclaveSession(); return ( <button onClick={() => execute('console.log("Hello!")')}> Run </button> ); }📝 Option B: Demonstrate usage
function CodeRunner() { const { execute, stdout, result } = useEnclaveSession(); return ( - <button onClick={() => execute('console.log("Hello!")')}> - Run - </button> + <div> + <button onClick={() => execute('console.log("Hello!")')}> + Run + </button> + <pre>{stdout}</pre> + {result && <div>Result: {JSON.stringify(result)}</div>} + </div> ); }docs/core-libraries/vectoriadb/search.mdx (1)
74-90: Undefined variable in example may confuse readers.The
userHasPermissionvariable on line 85 is referenced but not defined in this code example. Consider adding a comment or defining it to make the example self-contained.📝 Suggested clarification
const results = await toolIndex.search('sensitive operation', { filter: (metadata) => { + // Assume userHasPermission is defined in your application context + const userHasPermission = checkUserPermissions(); // your auth logic + // Must be owned by specific teams const allowedOwners = ['billing', 'users', 'orders']; if (!allowedOwners.includes(metadata.owner)) return false;Or simply add a comment:
- if (metadata.risk === 'destructive' && !userHasPermission) return false; + // userHasPermission: boolean from your auth context + if (metadata.risk === 'destructive' && !userHasPermission) return false;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (62)
docs/api-reference/ast-guard.mdxdocs/api-reference/enclave-vm.mdxdocs/api-reference/enclavejs-broker.mdxdocs/api-reference/enclavejs-client.mdxdocs/api-reference/enclavejs-react.mdxdocs/api-reference/enclavejs-runtime.mdxdocs/api-reference/enclavejs-types.mdxdocs/api-reference/vectoriadb.mdxdocs/changelog.mdxdocs/concepts/agentscript.mdxdocs/concepts/architecture.mdxdocs/concepts/security-model.mdxdocs/concepts/streaming-protocol.mdxdocs/core-libraries/ast-guard/agentscript-preset.mdxdocs/core-libraries/ast-guard/code-transform.mdxdocs/core-libraries/ast-guard/custom-rules.mdxdocs/core-libraries/ast-guard/overview.mdxdocs/core-libraries/ast-guard/pre-scanner.mdxdocs/core-libraries/ast-guard/security-rules.mdxdocs/core-libraries/enclave-vm/ai-scoring.mdxdocs/core-libraries/enclave-vm/configuration.mdxdocs/core-libraries/enclave-vm/double-vm.mdxdocs/core-libraries/enclave-vm/overview.mdxdocs/core-libraries/enclave-vm/reference-sidecar.mdxdocs/core-libraries/enclave-vm/security-levels.mdxdocs/core-libraries/enclave-vm/tool-system.mdxdocs/core-libraries/enclave-vm/worker-pool.mdxdocs/core-libraries/vectoriadb/hnsw.mdxdocs/core-libraries/vectoriadb/indexing.mdxdocs/core-libraries/vectoriadb/overview.mdxdocs/core-libraries/vectoriadb/persistence.mdxdocs/core-libraries/vectoriadb/search.mdxdocs/core-libraries/vectoriadb/tfidf.mdxdocs/docs.jsondocs/enclavejs/broker.mdxdocs/enclavejs/client.mdxdocs/enclavejs/overview.mdxdocs/enclavejs/react.mdxdocs/enclavejs/runtime.mdxdocs/enclavejs/stream.mdxdocs/enclavejs/types.mdxdocs/examples/encrypted-sessions.mdxdocs/examples/hello-world.mdxdocs/examples/multi-tenant.mdxdocs/examples/streaming-ui.mdxdocs/examples/tool-calling.mdxdocs/getting-started/installation.mdxdocs/getting-started/quickstart.mdxdocs/getting-started/welcome.mdxdocs/guides/first-agent.mdxdocs/guides/production-deployment.mdxdocs/guides/react-code-editor.mdxdocs/guides/scaling.mdxdocs/guides/security-hardening.mdxdocs/guides/tool-integration.mdxdocs/libraries/ast-guard.mdxdocs/libraries/enclave.mdxdocs/libraries/vectoriadb.mdxdocs/troubleshooting/common-errors.mdxdocs/troubleshooting/debugging.mdxdocs/troubleshooting/faq.mdxdocs/updates.mdx
✅ Files skipped from review due to trivial changes (22)
- docs/concepts/security-model.mdx
- docs/enclavejs/broker.mdx
- docs/core-libraries/ast-guard/overview.mdx
- docs/concepts/agentscript.mdx
- docs/enclavejs/client.mdx
- docs/enclavejs/react.mdx
- docs/core-libraries/ast-guard/code-transform.mdx
- docs/api-reference/ast-guard.mdx
- docs/api-reference/enclavejs-client.mdx
- docs/enclavejs/overview.mdx
- docs/concepts/architecture.mdx
- docs/core-libraries/enclave-vm/ai-scoring.mdx
- docs/core-libraries/enclave-vm/tool-system.mdx
- docs/enclavejs/stream.mdx
- docs/api-reference/enclavejs-types.mdx
- docs/api-reference/enclavejs-react.mdx
- docs/core-libraries/vectoriadb/tfidf.mdx
- docs/api-reference/enclavejs-broker.mdx
- docs/examples/hello-world.mdx
- docs/examples/multi-tenant.mdx
- docs/enclavejs/types.mdx
- docs/getting-started/quickstart.mdx
🧰 Additional context used
📓 Path-based instructions (2)
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/core-libraries/vectoriadb/persistence.mdxdocs/api-reference/vectoriadb.mdxdocs/core-libraries/enclave-vm/security-levels.mdxdocs/enclavejs/runtime.mdxdocs/api-reference/enclavejs-runtime.mdxdocs/core-libraries/ast-guard/security-rules.mdxdocs/core-libraries/enclave-vm/configuration.mdxdocs/core-libraries/enclave-vm/overview.mdxdocs/getting-started/installation.mdxdocs/core-libraries/vectoriadb/search.mdxdocs/core-libraries/enclave-vm/double-vm.mdxdocs/core-libraries/enclave-vm/reference-sidecar.mdxdocs/core-libraries/ast-guard/custom-rules.mdxdocs/core-libraries/vectoriadb/hnsw.mdxdocs/examples/streaming-ui.mdxdocs/getting-started/welcome.mdxdocs/guides/first-agent.mdxdocs/core-libraries/ast-guard/agentscript-preset.mdxdocs/core-libraries/vectoriadb/indexing.mdxdocs/concepts/streaming-protocol.mdxdocs/api-reference/enclave-vm.mdxdocs/core-libraries/enclave-vm/worker-pool.mdxdocs/changelog.mdxdocs/examples/encrypted-sessions.mdxdocs/docs.jsondocs/core-libraries/ast-guard/pre-scanner.mdxdocs/core-libraries/vectoriadb/overview.mdxdocs/examples/tool-calling.mdx
docs/docs.json
⚙️ CodeRabbit configuration file
docs/docs.json: Mintlify navigation configuration. Important structure: - dropdowns[].dropdown == "Documentation":
- Contains a "versions" array.
- Latest version entry: "version": "vX.Y (latest)", "default": true, with
paths like "docs/getting-started/welcome" and "updates" (NO "v/X.Y" prefix).- Older versions: "version": "v0.2", "version": "v0.1", etc., with paths
like "docs/v/0.2/getting-started/welcome".- dropdowns[].dropdown == "Blog":
- Contains blog groups (e.g. "blog/november-2025/introducing-frontmcp").
Review guidelines: - Do NOT suggest editing the Blog dropdown unless the PR explicitly adds/changes blogs. - Ensure latest version uses non-versioned "docs/..." paths, and older versions
use "docs/v/{minor}/..." paths.- If you see changes that try to route archived versions away from docs/v/**,
flag that as incorrect.
Files:
docs/docs.json
🧠 Learnings (4)
📚 Learning: 2026-01-08T18:40:23.681Z
Learnt from: CR
Repo: agentfront/enclave PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T18:40:23.681Z
Learning: Enforce global coverage threshold for `enclave-vm` as configured in `libs/enclave-vm/jest.config.ts`
Applied to files:
docs/core-libraries/enclave-vm/configuration.mdxdocs/core-libraries/enclave-vm/overview.mdxdocs/getting-started/installation.mdxdocs/api-reference/enclave-vm.mdx
📚 Learning: 2026-01-08T18:40:23.681Z
Learnt from: CR
Repo: agentfront/enclave PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T18:40:23.681Z
Learning: Applies to libs/enclave-vm/src/__tests__/perf/**/*.perf.spec.ts : Performance tests for enclave-vm should be placed under `libs/enclave-vm/src/__tests__/perf/**/*.perf.spec.ts`
Applied to files:
docs/getting-started/installation.mdx
📚 Learning: 2026-01-08T18:40:23.681Z
Learnt from: CR
Repo: agentfront/enclave PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T18:40:23.681Z
Learning: PRs must include problem/solution description, test evidence (`yarn test` or `nx test <project>`), and security-impact notes for sandbox/VM changes
Applied to files:
docs/getting-started/installation.mdx
📚 Learning: 2026-01-08T18:40:23.681Z
Learnt from: CR
Repo: agentfront/enclave PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T18:40:23.681Z
Learning: Applies to **/src/__tests__/**/*.spec.ts : Place test files under `src/__tests__/**/*.spec.ts` with performance tests under `libs/enclave-vm/src/__tests__/perf/**/*.perf.spec.ts`
Applied to files:
docs/getting-started/installation.mdx
🪛 LanguageTool
docs/core-libraries/vectoriadb/persistence.mdx
[style] ~121-~121: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ashmatch? 3. Doesversionmatch? 4. DoesmodelName` match? If any check fails,...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/api-reference/enclavejs-runtime.mdx
[style] ~16-~16: Consider using “who” when you are referring to a person instead of an object.
Context: ...nclaveRuntime Class The runtime worker that connects to a broker and executes code....
(THAT_WHO)
docs/core-libraries/enclave-vm/overview.mdx
[style] ~74-~74: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...ts Enclave returns a structured result with success/error status and execution stats: ```t...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
docs/core-libraries/enclave-vm/double-vm.mdx
[grammar] ~203-~203: Ensure spelling is correct
Context: ...ds minimal overhead: - Latency: ~1-2ms per tool call for validation - **Memory...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/core-libraries/vectoriadb/hnsw.mdx
[style] ~77-~77: To form a complete sentence, be sure to include a subject.
Context: ...efSearch Controls search-time quality. Can be adjusted per-query: ```ts const db ...
(MISSING_IT_THERE)
docs/getting-started/welcome.mdx
[grammar] ~6-~6: Use a hyphen to join words.
Context: ... execution' --- Enclave is a production ready JavaScript sandbox for executing A...
(QB_NEW_EN_HYPHEN)
[uncategorized] ~100-~100: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ription | |---------|-------------| | Production Ready Security | 500+ security tests, 6 def...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
docs/core-libraries/ast-guard/pre-scanner.mdx
[style] ~27-~27: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ...lnerable to: - Memory exhaustion - Extremely large files - Stack overflow - Deeply nes...
(EN_WEAK_ADJECTIVE)
docs/core-libraries/vectoriadb/overview.mdx
[style] ~22-~22: You have already used ‘right’ in nearby sentences. Consider using an alternative word to let your writing stand out and sound more polished.
Context: ...aDB - Tool discovery - Surface the right tool from natural-language queries - **...
(REP_RIGHT)
docs/examples/tool-calling.mdx
[grammar] ~6-~6: Use a hyphen to join words.
Context: ...--- Examples demonstrating various tool calling patterns with Enclave. ## Basic...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (30)
docs/changelog.mdx (5)
1-6: LGTM!Frontmatter is correctly structured for Mintlify MDX documentation.
8-34: LGTM!Comprehensive documentation of the streaming runtime release. Package list and feature descriptions align well with the PR's new additions.
36-54: LGTM!Session API and security enhancements are well-documented with clear feature descriptions.
56-68: LGTM!Security feature documentation is clear and concise.
70-84: LGTM!Initial release documentation provides clear context for the monorepo's foundational packages.
docs/core-libraries/enclave-vm/worker-pool.mdx (1)
1-160: Well-structured documentation with clear examples.The documentation is comprehensive, with good coverage of features, presets, configuration, and use cases. The ASCII diagram effectively illustrates the dual-layer sandbox concept, and the "When to Use" section provides practical guidance.
docs/core-libraries/ast-guard/security-rules.mdx (1)
1-285: LGTM!The security rules documentation is comprehensive and well-structured. Each rule includes clear code examples, configuration options, and explicit "Blocks" and "Allows" sections that help developers understand the behavior. The CVE-2021-42574 reference for Trojan Source attacks is accurate and the warning is appropriately placed.
docs/core-libraries/enclave-vm/configuration.mdx (1)
1-219: LGTM!The configuration reference is comprehensive and well-organized. The tables clearly document all options with their types, defaults, and descriptions. The
ExecutionResultinterface and error codes are particularly helpful for developers integrating with the API.docs/core-libraries/enclave-vm/double-vm.mdx (1)
1-225: LGTM!The Double VM Layer documentation clearly explains the nested isolation architecture with helpful ASCII diagrams. The built-in suspicious pattern detection table, custom pattern examples, and performance considerations are particularly valuable. The interface definitions provide good TypeScript reference for developers implementing custom detection logic.
docs/api-reference/enclave-vm.mdx (1)
1-377: Well-structured API reference.The API reference is comprehensive, covering the Enclave class, types, factory functions, and includes a complete example. The interface definitions and method signatures provide good TypeScript reference documentation.
docs/core-libraries/ast-guard/agentscript-preset.mdx (1)
1-182: LGTM!The AgentScript Preset documentation is well-organized with clear explanations of what the preset blocks and why. The configuration options table, default allowed globals list, and the "What AgentScript Blocks" table with security rationale are particularly helpful for developers understanding the security model.
docs/core-libraries/ast-guard/pre-scanner.mdx (1)
1-189: LGTM!The Pre-Scanner documentation effectively explains the Layer 0 defense concept and why it's critical to scan before parsing. The mandatory limits table, preset comparison, ReDoS detection patterns with vulnerability scores, and Unicode security sections provide valuable security context. The three regex handling modes (block, analyze, allow) are clearly explained with examples.
docs/core-libraries/ast-guard/custom-rules.mdx (1)
1-265: Well-structured documentation with comprehensive coverage.The documentation effectively covers custom rule creation, preset usage, and testing patterns. The code examples are clear and demonstrate practical use cases.
docs/core-libraries/enclave-vm/overview.mdx (2)
68-70: Verify ifdispose()should be awaited.If
enclave.dispose()is asynchronous (returns a Promise), it should be awaited to ensure proper cleanup. Consider verifying the method signature and updating if needed:// Clean up -enclave.dispose(); +await enclave.dispose();
1-109: Clear and comprehensive overview documentation.The documentation effectively introduces Enclave VM with a defense-in-depth explanation, practical quick start example, and well-structured error code reference.
docs/core-libraries/enclave-vm/security-levels.mdx (1)
1-144: Excellent security documentation with clear guidance.The security levels are well-documented with a comprehensive comparison table, appropriate use case examples, and clear warnings about using STRICT for untrusted code. The defense-in-depth section effectively summarizes the multi-layer security approach.
docs/api-reference/enclavejs-runtime.mdx (1)
1-425: Comprehensive API reference with excellent deployment examples.The documentation covers all aspects of the runtime package including interfaces, methods, events, and provides practical Docker/Kubernetes deployment examples. The complete example demonstrates production-ready patterns.
docs/api-reference/vectoriadb.mdx (1)
1-478: Thorough API reference with excellent coverage.The documentation comprehensively covers the vectoriadb API including all methods, filter query syntax, persistence adapters, indexing options, and embedders. The complete example effectively demonstrates end-to-end usage.
docs/core-libraries/enclave-vm/reference-sidecar.mdx (1)
1-187: Clear documentation for the Reference Sidecar feature.The documentation effectively explains the lazy resolution model, security considerations around composite blocking, and provides practical use cases. The memory management and monitoring sections are particularly helpful for production use.
docs/examples/streaming-ui.mdx (1)
1-508: Comprehensive streaming UI guide with practical examples.The documentation provides excellent coverage of streaming patterns including a reusable React hook, console and timeline components, and both WebSocket and SSE alternatives. The complete example ties everything together effectively.
docs/concepts/streaming-protocol.mdx (1)
1-313: Documentation is comprehensive and well-structured.The streaming protocol documentation covers all essential aspects: event types with clear JSON examples, NDJSON format benefits, tool call flow, encryption details, reconnection support, and event filtering. The protocol diagram effectively illustrates the Client-Broker-Runtime communication flow.
docs/examples/tool-calling.mdx (1)
1-464: Excellent coverage of tool calling patterns.The documentation provides comprehensive examples covering validation with Zod, async operations, error handling, logging, rate limiting, and caching. The complete example at the end effectively demonstrates how to combine these patterns.
docs/guides/first-agent.mdx (1)
1-341: Well-structured step-by-step guide.The guide effectively walks through building an AI agent with clear progression from setup to production-ready patterns. The inclusion of error handling, validation, and streaming options provides good coverage for real-world use cases.
docs/examples/encrypted-sessions.mdx (1)
1-553: Comprehensive E2E encryption documentation.The guide provides a complete implementation covering ECDH key exchange, AES-GCM encryption/decryption, client/server implementation, streaming support, React integration, and security considerations. The architecture diagram effectively illustrates the key exchange flow.
docs/docs.json (1)
56-195: Navigation structure follows Mintlify guidelines correctly.The latest version (v2.0) uses non-versioned paths like
"getting-started/welcome"as required by the coding guidelines. The structure is well-organized with logical groupings and appropriate icons for each section.docs/getting-started/welcome.mdx (1)
1-141: Excellent welcome page with clear navigation.The page effectively serves as a documentation hub with well-organized CardGroups, comprehensive feature tables, and clear pathways to different documentation sections. The "Why Enclave?" section immediately communicates value propositions.
docs/core-libraries/vectoriadb/persistence.mdx (1)
1-209: Comprehensive persistence documentation.The documentation thoroughly covers all three storage adapters (Memory, File, Redis) with clear use cases, configuration options, and practical code examples. The cache invalidation explanation and warm-up pattern are particularly valuable for production deployments.
docs/core-libraries/vectoriadb/hnsw.mdx (1)
1-233: Well-structured HNSW documentation.The documentation provides comprehensive coverage of HNSW parameters, performance characteristics, and tuning guidelines. The code examples are clear and demonstrate practical configuration scenarios effectively.
The static analysis hint about line 77 ("Can be adjusted per-query:") being a fragment is acceptable in documentation context where terse descriptions are the norm.
docs/core-libraries/vectoriadb/indexing.mdx (1)
1-188: Comprehensive indexing documentation.The documentation thoroughly covers the indexing API with clear explanations of:
- Single and batch document operations
- Type-safe metadata patterns
- Update semantics (when re-embedding occurs vs. metadata-only updates)
- Document limits for DoS prevention
The code examples are practical and demonstrate real-world usage patterns.
docs/core-libraries/vectoriadb/search.mdx (1)
1-184: Well-documented search API.The search documentation provides thorough coverage of:
- Basic and advanced search options
- Simple and complex filtering patterns
- Similarity threshold guidance
- Performance optimization tips
- Error handling
The code examples are practical and the Options Reference table is helpful.
… handling in streaming execution
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @apps/streaming-demo/src/client-server.ts:
- Around line 342-344: The explicit root handler app.get('/', (_req, res) =>
res.sendFile(...)) is flagged for missing rate limiting and may be redundant
since express.static already serves the public directory; either remove this
handler and rely on the existing express.static middleware, or add rate
limiting: import express-rate-limit, create a limiter (e.g., staticLimiter) with
desired windowMs and max, and apply it as middleware to the root route by
changing the handler to app.get('/', staticLimiter, ...). Ensure the import and
the staticLimiter identifier are added and used so requests to the root are
rate-limited if you keep the explicit route.
- Around line 206-213: The Promise that waits for ws 'open' uses setTimeout but
never clears it, leaking the timer; capture the timeout ID (e.g., const timeout
= setTimeout(...)) and call clearTimeout(timeout) when the socket opens (inside
the ws.on('open') handler) and also when the socket errors (inside
ws.on('error') or its rejection path) so the timer is always cleared regardless
of outcome; ensure you still resolve/reject only once.
In @apps/streaming-demo/src/runtime-server.ts:
- Around line 349-353: The loop logs pending tool calls but never invokes the
resolver, leaving Promises hanging; update the rejection loop in
runtime-server.ts to call each resolver for entries in session.pendingToolCalls
(e.g., invoke resolver(new Error('Session closed') or an appropriate
CancellationError) so the corresponding Promises in toolHandler reject
immediately), then clear the map as currently done; reference the
session.pendingToolCalls iteration where callId and resolver are deconstructed
to locate and modify the code.
In @docs/examples/encrypted-sessions.mdx:
- Around line 434-440: The catch block accesses error.message on an unknown
type; add a type guard or cast so the caught error is typed as Error before
using .message (e.g., ensure the variable used to build errorEvent is a string
from (error instanceof Error ? error.message : String(error))). Update the
construction of errorEvent and the subsequent encrypt(...) call (which uses
session.sharedSecret and JSON.stringify(errorEvent)) and then call
res.write(...) as before so TypeScript strict mode no longer reports accessing
.message on unknown.
In @docs/examples/streaming-ui.mdx:
- Around line 382-404: The example uses req.body.code inside the Express route
but never registers a body parser, so req.body will be undefined; add the JSON
body-parsing middleware (e.g., call express.json()) on the Express app instance
before registering the POST route (insert app.use(express.json()) after const
app = express() and before app.post('/execute', ...)) so req.body.code is
populated.
In @docs/examples/tool-calling.mdx:
- Line 6: Change the compound modifier "tool calling" to the hyphenated form
"tool-calling" in the docs examples text (the string "Examples demonstrating
various tool calling patterns with Enclave"); update that occurrence so the
phrase reads "Examples demonstrating various tool-calling patterns with Enclave"
to satisfy the static analysis rule.
🧹 Nitpick comments (8)
docs/examples/encrypted-sessions.mdx (2)
271-281: Consider adding error handling for JSON parsing in the streaming loop.If the server sends malformed JSON or the decrypted content is not valid JSON,
JSON.parsewill throw and break the stream processing. Consider wrapping in try-catch for production code.💡 Suggested improvement
for (const line of lines) { if (!line.trim()) continue; + try { const encryptedEvent = JSON.parse(line); const decryptedEvent = await decrypt( this.sharedSecret!, encryptedEvent ); yield JSON.parse(decryptedEvent); + } catch (error) { + console.error('Failed to parse stream event:', error); + // Optionally yield an error event or continue + } }
485-492: Add cleanup function to useEffect to prevent resource leaks.The useEffect creates a client and initiates connection but doesn't clean up on unmount. If the component unmounts before connection completes, or during an active session, the connection remains open. Consider adding error handling and cleanup.
💡 Suggested improvement
useEffect(() => { const newClient = new EncryptedEnclaveClient('https://api.example.com'); setClient(newClient); + let mounted = true; - newClient.connect().then(() => { - setConnected(true); - }); + newClient.connect() + .then(() => { + if (mounted) setConnected(true); + }) + .catch((error) => { + console.error('Connection failed:', error); + }); + + return () => { + mounted = false; + // Add client.disconnect() if available + }; }, []);apps/streaming-demo/src/client-server.ts (1)
43-109: Consider extracting shared streaming logic to reduce duplication.The
handleEmbeddedExecute(lines 43-109) and lambda handler (lines 116-182) share nearly identical logic. Consider extracting a shared helper:♻️ Proposed refactor
async function handleBrokerProxy( req: Request, res: Response, endpoint: string, modeName: string ): Promise<void> { const { code } = req.body; if (!code || typeof code !== 'string') { res.status(400).json({ error: 'Code is required' }); return; } console.log(`\x1b[34m[${modeName}]\x1b[0m Executing: ${code.slice(0, 50)}...`); res.setHeader('Content-Type', 'application/x-ndjson'); res.setHeader('Cache-Control', 'no-cache'); res.setHeader('Connection', 'keep-alive'); try { const response = await fetch(`${BROKER_URL}${endpoint}`, { method: 'POST', headers: { 'Content-Type': 'application/json', Accept: 'application/x-ndjson' }, body: JSON.stringify({ code }), }); if (!response.ok) throw new Error(`Broker returned ${response.status}`); const reader = response.body?.getReader(); if (!reader) throw new Error('No response body'); const decoder = new TextDecoder(); let lastEvent: unknown = null; while (true) { const { done, value } = await reader.read(); if (done) break; const chunk = decoder.decode(value, { stream: true }); res.write(chunk); for (const line of chunk.split('\n').filter((l) => l.trim())) { try { const event = JSON.parse(line); lastEvent = event; if (event.type === 'tool_call') { console.log(`\x1b[34m[${modeName}]\x1b[0m Tool: ${event.payload.toolName}`); } } catch { /* Ignore */ } } } if (lastEvent && (lastEvent as { type: string }).type === 'final') { const finalEvent = lastEvent as { payload: { ok: boolean; result: unknown; stats?: unknown } }; res.write(JSON.stringify({ type: 'client_result', result: { success: finalEvent.payload.ok, value: finalEvent.payload.result, stats: finalEvent.payload.stats }, }) + '\n'); } res.end(); console.log(`\x1b[34m[${modeName}]\x1b[0m Complete`); } catch (error) { console.error(`\x1b[31m[${modeName} error]\x1b[0m`, error); res.write(JSON.stringify({ type: 'client_error', error: String(error) }) + '\n'); res.end(); } } // Usage: app.post('/api/execute/embedded', (req, res) => handleBrokerProxy(req, res, '/sessions/embedded', 'Client → Embedded')); app.post('/api/execute/lambda', (req, res) => handleBrokerProxy(req, res, '/sessions/lambda', 'Client → Broker → Lambda'));Also applies to: 116-182
docs/examples/streaming-ui.mdx (2)
226-226: Unused variableiin map callback.The index parameter is declared but never used since
call.idis correctly used as the key.Suggested fix
- {toolCalls.map((call, i) => ( + {toolCalls.map((call) => (
439-446: Consider adding error handling for SSE parsing.The
JSON.parsecall on line 443 could throw if the server sends malformed data. For a production-ready example, wrapping this in a try-catch would improve robustness.Suggested improvement
for (const chunk of lines) { if (chunk.startsWith('event:')) { const [eventLine, dataLine] = chunk.split('\n'); const event = eventLine.replace('event: ', ''); - const data = JSON.parse(dataLine.replace('data: ', '')); - handleEvent(event, data); + try { + const data = JSON.parse(dataLine.replace('data: ', '')); + handleEvent(event, data); + } catch (e) { + console.error('Failed to parse SSE data:', e); + } } }apps/streaming-demo/src/runtime-server.ts (3)
166-185: Consider moving interface definitions before their usage.The interfaces are defined after the validator functions that reference them (lines 87-126). While TypeScript allows this due to hoisting, placing interfaces before their first use improves readability.
326-332: Consider documenting the error handling contract.The pattern of resolving with
{ __error: true, ...request.error }instead of rejecting is a deliberate design choice. A brief inline comment or type definition would help future maintainers understand this contract between the runtime and enclave.
356-365: Consider handling SIGINT in addition to SIGTERM.For better developer experience (Ctrl+C), consider also handling
SIGINT:♻️ Suggested improvement
+function gracefulShutdown() { + console.log('\x1b[35m[Runtime]\x1b[0m Shutting down...'); + wss.close(); + for (const session of sessions.values()) { + session.enclave.dispose(); + } + sessions.clear(); + process.exit(0); +} + // Graceful shutdown -process.on('SIGTERM', () => { - console.log('\x1b[35m[Runtime]\x1b[0m Shutting down...'); - wss.close(); - for (const session of sessions.values()) { - session.enclave.dispose(); - } - sessions.clear(); - process.exit(0); -}); +process.on('SIGTERM', gracefulShutdown); +process.on('SIGINT', gracefulShutdown);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/codeql/codeql-config.ymlapps/streaming-demo/src/client-server.tsapps/streaming-demo/src/runtime-server.tsdocs/api-reference/vectoriadb.mdxdocs/core-libraries/ast-guard/custom-rules.mdxdocs/examples/encrypted-sessions.mdxdocs/examples/streaming-ui.mdxdocs/examples/tool-calling.mdxdocs/getting-started/welcome.mdxlibs/enclavejs-broker/src/event-filter.tslibs/enclavejs-broker/src/http/session-handler.ts
✅ Files skipped from review due to trivial changes (2)
- docs/api-reference/vectoriadb.mdx
- docs/core-libraries/ast-guard/custom-rules.mdx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use ESLint flat config with unused parameters and variables prefixed with underscore_
Prefer workspace import paths (e.g.,import { validate } from 'ast-guard') over deep cross-library relative imports
Files:
apps/streaming-demo/src/client-server.tsapps/streaming-demo/src/runtime-server.ts
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/getting-started/welcome.mdxdocs/examples/streaming-ui.mdxdocs/examples/encrypted-sessions.mdxdocs/examples/tool-calling.mdx
🪛 GitHub Check: CodeQL
apps/streaming-demo/src/client-server.ts
[failure] 342-344: Missing rate limiting
This route handler performs a file system access, but is not rate-limited.
🪛 LanguageTool
docs/examples/tool-calling.mdx
[grammar] ~6-~6: Use a hyphen to join words.
Context: ...--- Examples demonstrating various tool calling patterns with Enclave. ## Basic...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Performance
🔇 Additional comments (26)
.github/codeql/codeql-config.yml (1)
3-13: Consider the security implications of excluding the demo app from CodeQL analysis.Excluding test files from CodeQL is standard practice since they contain mock data and aren't production code. However, excluding
apps/streaming-demomeans any security vulnerabilities in that demo app won't be detected.If the demo contains example code patterns that developers might copy into production, those patterns won't be scanned for issues like injection vulnerabilities or insecure data handling.
Verify this is intentional and the demo app is truly isolated from production code paths. If the demo serves as a reference implementation, consider keeping it in the scan scope.
docs/examples/encrypted-sessions.mdx (1)
1-556: Well-structured documentation for encrypted sessions.This documentation provides a comprehensive guide for implementing end-to-end encrypted code execution. The cryptographic patterns (ECDH P-256 for key exchange, AES-GCM for symmetric encryption) are industry-standard and correctly implemented. The security considerations section appropriately highlights important production concerns.
apps/streaming-demo/src/client-server.ts (4)
29-34: LGTM for demo purposes.Wildcard CORS is acceptable for a local demo server. For production use, this should be restricted to specific origins.
219-285: LGTM!Good defensive programming with
res.writablechecks before writing, proper WebSocket cleanup on client disconnect, and appropriate error handling.
294-325: LGTM!The local tool handler appropriately demonstrates the limitations of direct mode with clear warning messages. Type assertions are acceptable here given the demo context.
346-363: LGTM!Clear and informative startup logging that helps developers understand the available endpoints and configuration.
docs/examples/streaming-ui.mdx (1)
1-511: Well-structured documentation for streaming UI patterns.This documentation provides comprehensive examples covering the streaming lifecycle from basic usage through React integration to SSE alternatives. The code examples are clear and demonstrate progressive complexity well.
docs/examples/tool-calling.mdx (2)
1-4: Well-structured documentation!This is comprehensive coverage of tool-calling patterns, progressing from basic usage through validation, error handling, and advanced patterns like rate limiting and caching. The examples are clear and practical.
460-464: All referenced documentation links exist and are properly configured in the repository navigation.docs/getting-started/welcome.mdx (10)
1-6: LGTM!The frontmatter and introductory content clearly establish the product positioning and key value propositions.
8-16: LGTM!The "Why Enclave?" section effectively communicates the core value propositions with consistent terminology (6-layer security is mentioned consistently throughout the document).
17-40: LGTM!The CardGroup components are well-structured with appropriate icons and consistent link paths that align with the navigation table later in the document.
42-60: LGTM!The EnclaveJS Streaming Runtime section cleanly separates the individual package cards from the overview, providing clear navigation paths for users.
62-94: LGTM!The "Understand the System" and "Learn by Example" sections provide a clear separation between conceptual documentation and hands-on guides, with consistent URL patterns.
96-107: LGTM!The feature table provides a comprehensive overview with appropriate technical details (encryption standards, isolation modes) that help users understand the capabilities at a glance.
109-123: LGTM!The installation commands are well-organized with clear comments separating each package category, making it easy for users to install only what they need.
125-134: LGTM!The "Explore the Docs" table provides a useful table-of-contents style navigation that complements the card-based sections above.
136-141: LGTM!The footer links provide quick access to the most important resources and the GitHub repository.
19-134: All referenced documentation pages exist. All 17 internal links in this welcome page have corresponding .mdx files in the repository.apps/streaming-demo/src/runtime-server.ts (7)
14-17: LGTM!Imports are well-organized and use workspace import paths as recommended.
19-32: LGTM!Server setup and session management structure are appropriate for this WebSocket-based runtime server.
79-84: LGTM!The
sendEventfunction properly checks WebSocket state before sending, preventing errors on closed connections.
128-164: LGTM!Clean switch-case structure with proper validation before processing each message type.
229-250: LGTM!The timeout mechanism for tool calls is well-implemented with proper cleanup of the pending call map on both timeout and successful resolution.
53-60: Unhandled promise rejection risk.The
try-catchonly catches JSON parse errors. IfhandleMessagethrows an error asynchronously, it will result in an unhandled promise rejection since theawaitis inside the try block but the error handling is focused on parsing.Consider wrapping the entire async operation:
🛠️ Suggested fix
ws.on('message', async (data: Buffer) => { try { const message = JSON.parse(data.toString()); await handleMessage(ws, message); - } catch (error) { - console.error('\x1b[31m[Runtime]\x1b[0m Failed to parse message:', error); + } catch (error) { + console.error('\x1b[31m[Runtime]\x1b[0m Message handling error:', error); } });Likely an incorrect or invalid review comment.
190-206: No issue found: sequence numbers remain monotonically increasing despite concurrent tool calls.The
seqvariable increments synchronously via++seqwith eachsendEvent()call, regardless of when async tool results are resolved. JavaScript's single-threaded execution ensures that even if tool results arrive out of order, the resolver callbacks execute sequentially on the event loop, each incrementingseqsynchronously. For example, if tool B completes before tool A, the sequence would be:tool_call A (seq=2) → tool_call B (seq=3) → tool_result_applied B (seq=4) → tool_result_applied A (seq=5), maintaining strict monotonic ordering.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @apps/streaming-demo/src/client-server.ts:
- Around line 29-34: The CORS middleware added via app.use(...) sets headers but
doesn't short-circuit OPTIONS preflight requests; update the middleware used in
apps/streaming-demo/src/client-server.ts (the app.use((_req, res, next) => { ...
}) block) to detect req.method === 'OPTIONS', set the same CORS headers and
immediately respond with a 204 (or 200) and end the response instead of calling
next(), otherwise allow other methods to continue by calling next().
- Around line 43-109: The NDJSON handling in handleEmbeddedExecute is unsafe
because the chunk-based split at the reader.read loop can break JSON objects
across chunk boundaries and lastEvent can be missed; replace the inline parsing
loop with a buffered NDJSON parser that accumulates partial data between chunks,
parses complete lines into events, updates lastEvent, and only leaves the
remainder in the buffer for the next chunk (consider extracting this into a
helper like pipeNdjsonAndTrackLastEvent to reuse for the lambda handler). Also
attach an AbortController to the fetch to allow cancellation and listen for
req.on('close') (and res.on('close') if needed) to call abort(), and ensure the
reader is released/closed on abort to avoid leaks; update the code paths that
currently reference reader.read, lastEvent, and the final-event check to use the
buffered helper and handle aborts cleanly.
In @apps/streaming-demo/src/runtime-server.ts:
- Around line 357-369: The gracefulShutdown function currently calls wss.close()
and session.enclave.dispose() without waiting and then immediately calls
process.exit(0); make gracefulShutdown async, convert wss.close() to a Promise
(or use its callback) and await it, collect and await all
session.enclave.dispose() results (Promise.all) to ensure each enclave is fully
disposed, handle/log disposal errors, only call sessions.clear() and
process.exit(0) after all awaits complete (or exit with non-zero on fatal
errors).
- Around line 187-307: handleExecute creates pendingToolCalls as Map<CallId,
(result)=>void> and blindly overwrites sessions without disposal, causing
orphaned enclaves, unresolved timeouts and incorrect tool_result_applied
emissions; change pendingToolCalls to store objects {resolve, reject, timeout}
so timeouts can be cleared and rejects used on cancel, add a collision check
before sessions.set(sessionId, session) that disposes/returns early if a session
exists (or reject new request), update handleCancel to clear timeout and call
reject(...) (not resolve) to avoid emitting tool_result_applied, update
handleToolResult to clear the stored timeout, call resolve(result) and emit the
tool_result_applied event from handleToolResult (not inside the resolver), and
replace callId generation with crypto.randomUUID() (and if event seq ordering
matters, add seq to ActiveSession so handleToolResult can increment/send events
consistently).
In @docs/examples/tool-calling.mdx:
- Around line 211-214: The example inside enclave.run uses an undefined variable
userId when calling callTool('users:get', { id: userId });; fix by either
declaring and initializing userId before the enclave.run invocation or by
passing the id via the enclave context (as shown in "Context-Aware Tools"),
e.g., set a context object with userId and reference that inside the script or
replace userId with an explicit literal/example value so the snippet is
runnable; update the enclave.run example to include the chosen approach and
ensure the callTool invocation references the defined symbol.
- Around line 288-314: The example incorrectly declares the toolHandler with
three parameters (name, args, context: ExecutionContext) whereas the Enclave API
only passes two; update the Enclave instantiation to remove the third parameter
from the toolHandler signature (use toolHandler: async (name, args) => { ... })
and stop referencing context inside that function; if execution context data is
needed inside tools, pass it via the Enclave globals option or attach it to args
before calling executeTool, and ensure references to ExecutionContext in the
example are removed or replaced with a note about using globals and enclave.run
for supplying context.
🧹 Nitpick comments (1)
docs/examples/encrypted-sessions.mdx (1)
486-500: React example should include cleanup and error handling.The
useEffecthook is missing a cleanup function, which could leave the connection open if the component unmounts. Additionally, the example lacks error handling forconnect()andexecute(), which could mislead readers implementing this pattern.Suggested improvement
useEffect(() => { const newClient = new EncryptedEnclaveClient('https://api.example.com'); setClient(newClient); - newClient.connect().then(() => { - setConnected(true); - }); - }, []); + newClient.connect() + .then(() => setConnected(true)) + .catch((err) => console.error('Connection failed:', err)); + + return () => { + // Cleanup connection on unmount + newClient.disconnect?.(); + }; + }, []); const execute = async () => { if (!client || !connected) return; - const result = await client.execute(code); - setResult(result); + try { + const result = await client.execute(code); + setResult(result); + } catch (err) { + console.error('Execution failed:', err); + } };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
apps/streaming-demo/src/client-server.tsapps/streaming-demo/src/runtime-server.tsdocs/examples/encrypted-sessions.mdxdocs/examples/streaming-ui.mdxdocs/examples/tool-calling.mdxlibs/enclave-vm/src/__tests__/perf/enclave.perf.spec.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/examples/streaming-ui.mdx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use ESLint flat config with unused parameters and variables prefixed with underscore_
Prefer workspace import paths (e.g.,import { validate } from 'ast-guard') over deep cross-library relative imports
Files:
apps/streaming-demo/src/runtime-server.tsapps/streaming-demo/src/client-server.ts
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/examples/tool-calling.mdxdocs/examples/encrypted-sessions.mdx
🧬 Code graph analysis (1)
apps/streaming-demo/src/runtime-server.ts (2)
libs/enclavejs-runtime/src/runtime-worker.ts (3)
handleExecute(172-213)handleToolResult(232-247)handleCancel(218-227)libs/enclave-vm/src/sidecar/reference-resolver.ts (1)
resolve(170-216)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Performance
🔇 Additional comments (2)
docs/examples/tool-calling.mdx (2)
382-458: Well-structured comprehensive example.This complete example effectively ties together validation, tool execution, logging, and proper cleanup with
enclave.dispose(). The inclusion ofsecurityLevelandmaxToolCallsconfiguration options provides a good reference for production usage.
460-464: All referenced documentation pages in the Next Steps section exist and are correctly linked.
docs/examples/streaming-ui.mdx✓docs/core-libraries/enclave-vm/tool-system.mdx✓docs/guides/tool-integration.mdx✓
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/streaming-demo/src/client-server.ts:
- Around line 302-346: The message handler registered with ws.on('message') is
async and can produce unhandled promise rejections (e.g., from await
handleToolLocally(...) or from code after the await) and res.end()/ws.close()
may be raced from both the 'message' and 'close' handlers; to fix, make the ws
message callback non-async and explicitly run the async work inside a
self-invoked promise with a terminal .catch(...) to surface errors (e.g.,
replace ws.on('message', async (data) => { ... }) with ws.on('message', (data)
=> { (async () => { ... await handleToolLocally(...) ... })().catch(err =>
console.error(...)) });), and centralize connection teardown by adding a single
cleanup function (e.g., function safeClose() { if (ended) return; ended = true;
ws.close(); res.end(); }) guarded by a boolean flag (ended) and call safeClose()
instead of calling res.end()/ws.close() directly from both the final event
handling and the ws.on('close') handler.
🧹 Nitpick comments (3)
apps/streaming-demo/src/client-server.ts (3)
209-261: Consider extracting shared logic between embedded and lambda modes.The lambda handler shares nearly identical code with
handleEmbeddedExecute- only the broker URL and log prefix differ. For a demo app this is acceptable, but extracting the common pattern could reduce maintenance overhead.♻️ Optional refactor to reduce duplication
+async function handleBrokerExecute( + req: Request, + res: Response, + mode: 'embedded' | 'lambda', +): Promise<void> { + const { code } = req.body; + if (!code || typeof code !== 'string') { + res.status(400).json({ error: 'Code is required' }); + return; + } + + const logPrefix = mode === 'embedded' + ? '[Client → Embedded]' + : '[Client → Broker → Lambda]'; + const endpoint = mode === 'embedded' + ? '/sessions/embedded' + : '/sessions/lambda'; + + console.log(`\x1b[34m${logPrefix}\x1b[0m Executing: ${code.slice(0, 50)}...`); + // ... rest of shared logic +} + +app.post('/api/execute/embedded', (req, res) => handleBrokerExecute(req, res, 'embedded')); +app.post('/api/execute/lambda', (req, res) => handleBrokerExecute(req, res, 'lambda'));
362-368: Minor: Event listener not cleaned up on normal completion.The
req.on('close')listener (line 363) isn't removed when the WebSocket completes normally. ThereadyStatecheck on line 364 prevents issues, but removing the listener explicitly would be cleaner.
377-408: FunctionhandleToolLocallyis markedasyncbut contains noawait.The function can be simplified to a synchronous function returning
unknowndirectly, since it has no asynchronous operations.♻️ Simplify to synchronous function
-async function handleToolLocally(toolName: string, args: Record<string, unknown>): Promise<unknown> { +function handleToolLocally(toolName: string, args: Record<string, unknown>): unknown {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/codeql/codeql-config.ymlapps/streaming-demo/src/broker-server.tsapps/streaming-demo/src/client-server.tsapps/streaming-demo/src/runtime-server.tsdocs/examples/encrypted-sessions.mdxdocs/examples/tool-calling.mdx
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/codeql/codeql-config.yml
- docs/examples/encrypted-sessions.mdx
- apps/streaming-demo/src/runtime-server.ts
- docs/examples/tool-calling.mdx
- apps/streaming-demo/src/broker-server.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use ESLint flat config with unused parameters and variables prefixed with underscore_
Prefer workspace import paths (e.g.,import { validate } from 'ast-guard') over deep cross-library relative imports
Files:
apps/streaming-demo/src/client-server.ts
🧬 Code graph analysis (1)
apps/streaming-demo/src/client-server.ts (2)
libs/enclavejs-broker/src/http/express-adapter.ts (1)
options(169-172)libs/enclave-vm/src/sidecar/reference-resolver.ts (1)
resolve(170-216)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Performance
- GitHub Check: Test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/streaming-demo/src/client-server.ts (6)
1-24: LGTM!Clear documentation of the three execution modes and their architectural differences. The imports are appropriate for the server's functionality.
26-43: LGTM!CORS middleware is correctly configured for a demo application. The preflight handling is appropriate.
61-145: LGTM!The NDJSON streaming implementation handles abort, cleanup, and buffered parsing correctly. The
finallyblock properly removes event listeners and cancels the reader.
150-202: LGTM!Solid implementation with proper input validation, streaming headers, abort handling, and error recovery. The defensive
res.writablecheck before writing errors is good practice.
410-423: LGTM!Default route and health check are well-implemented. Good use of underscore prefix for the unused
_reqparameter as per coding guidelines.
425-442: LGTM!Clear startup logging with helpful documentation of available endpoints and execution modes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/streaming-demo/src/client-server.ts:
- Around line 373-377: The catch block in client-server.ts logs and writes an
error response without checking whether the HTTP response is still writable;
wrap the res.write/ res.end logic in a guard that checks res.writable (e.g., if
(res.writable) { res.write(...); res.end(); }) and otherwise only log the error,
keeping the existing console.error call; ensure you reference the existing
variables used there (res and error) so the fix applies to that catch block.
🧹 Nitpick comments (4)
apps/streaming-demo/src/client-server.ts (4)
121-123: Consider logging parse errors in debug mode.Silently swallowing all JSON parse errors could mask issues with malformed broker responses. While ignoring incomplete chunks is intentional, a debug-level log would help troubleshooting.
♻️ Optional: Add debug logging
} catch { - // Ignore parse errors for incomplete JSON + // Incomplete JSON lines are expected during streaming + // Uncomment for debugging: console.debug('[NDJSON] Incomplete line:', trimmed.slice(0, 50)); }
209-261: Opportunity to reduce duplication with embedded handler.The lambda and embedded handlers are nearly identical, differing only in the broker endpoint path and log prefix. Consider extracting a shared handler.
♻️ Optional: Extract shared handler
async function handleBrokerExecute( req: Request, res: Response, mode: 'embedded' | 'lambda' ): Promise<void> { const { code } = req.body; if (!code || typeof code !== 'string') { res.status(400).json({ error: 'Code is required' }); return; } const label = mode === 'embedded' ? 'Client → Embedded' : 'Client → Broker → Lambda'; console.log(`\x1b[34m[${label}]\x1b[0m Executing: ${code.slice(0, 50)}...`); // ... rest of shared logic } app.post('/api/execute/embedded', (req, res) => handleBrokerExecute(req, res, 'embedded')); app.post('/api/execute/lambda', (req, res) => handleBrokerExecute(req, res, 'lambda'));
281-378: Consider adding an execution timeout for direct mode.Unlike the broker-proxied modes (which benefit from broker-side timeouts), direct mode has no timeout after the WebSocket connects. If the Lambda never sends a
finalevent, the response will hang indefinitely.♻️ Suggested execution timeout
// After WebSocket connection established const executionTimeout = setTimeout(() => { console.log(`\x1b[31m[Client → Lambda Direct]\x1b[0m Execution timeout`); if (ws.readyState === WebSocket.OPEN) { ws.send(JSON.stringify({ type: 'cancel', sessionId, reason: 'Execution timeout' })); ws.close(); } if (res.writable) { res.write(JSON.stringify({ type: 'client_error', error: 'Execution timeout' }) + '\n'); res.end(); } }, 30000); // 30 second timeout // Clear timeout when final event received or on close // In the 'final' handler: clearTimeout(executionTimeout); // In the 'close' handler: clearTimeout(executionTimeout);
387-397: Type assertions onargslack runtime validation.The type assertions (
args as { a: number; b: number }) assume the caller sends correct argument shapes. If malformed args are received, this could cause runtime errors (e.g.,undefined + undefinedfor addNumbers).For a demo, this is acceptable. For robustness, consider basic validation.
♻️ Optional: Add basic validation
case 'addNumbers': { - const { a, b } = args as { a: number; b: number }; + const a = typeof args.a === 'number' ? args.a : 0; + const b = typeof args.b === 'number' ? args.b : 0; return { result: a + b, operation: `${a} + ${b} = ${a + b}`, calculatedBy: 'client-direct' }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/streaming-demo/src/client-server.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use ESLint flat config with unused parameters and variables prefixed with underscore_
Prefer workspace import paths (e.g.,import { validate } from 'ast-guard') over deep cross-library relative imports
Files:
apps/streaming-demo/src/client-server.ts
🧬 Code graph analysis (1)
apps/streaming-demo/src/client-server.ts (2)
libs/enclavejs-broker/src/http/express-adapter.ts (1)
options(169-172)libs/enclave-vm/src/sidecar/reference-resolver.ts (1)
resolve(170-216)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test
- GitHub Check: Performance
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/streaming-demo/src/client-server.ts (3)
22-24: Configuration is hardcoded but acceptable for demo purposes.For a demo application, hardcoded values are fine. If this grows into a production-ready component, consider using environment variables with fallbacks.
150-202: LGTM!The embedded mode handler has proper input validation, streaming headers, abort handling, and error paths. The defensive
res.writablecheck prevents writing to closed streams.
414-446: LGTM!The default route reuse, health check, and startup logging are well-implemented for a demo server. The console output provides clear guidance for users.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.