Harden home connector observability and snapshot handling#269
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds unconditional Sentry bootstrap to Home Connector startup (including Docker), installs coordinated graceful-shutdown and process-level error handlers that flush/close Sentry, enriches websocket transport with breadcrumbs and error capture, and hardens worker session state handling and tests to clear persisted connection state on disconnects or snapshot failures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Process as Process
participant Server as Server
participant HTTP as HTTP_Server
participant Sentry as Sentry
Process->>Server: Start (node --import ./src/sentry-init.ts)
Server->>HTTP: Start listening
Note over Server: Register SIGINT/SIGTERM and fatal handlers
Process->>Server: SIGTERM / uncaughtException / unhandledRejection
Server->>Server: Run shutdown routine
Server->>Server: Stop connector worker
Server->>HTTP: Close HTTP server (5s watchdog)
Server->>Sentry: Flush/Close (timeout)
Sentry-->>Server: Complete or timeout
Server->>Process: Exit with code
sequenceDiagram
autonumber
participant Socket as WebSocket
participant Connector as Connector_Handler
participant Dispatch as JSON_RPC_Dispatch
participant Sentry as Sentry
Socket->>Connector: message (raw data)
Connector->>Connector: try { parse + handle }
alt Parse & Dispatch Success
Connector->>Dispatch: Dispatch JSON-RPC
Dispatch-->>Connector: Result
Connector->>Sentry: add breadcrumb (ack/hello/list_changed) with socket context
else Parse or Handler Failure
Connector->>Connector: set lastError / update state
Connector->>Sentry: capture exception with socket context + payload preview
Connector->>Socket: optional error response / close
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-269.kentcdodds.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/worker/src/home/session.ts (1)
271-317: Use the newcaptureSessionMessagehelper here for consistency.Both rejection paths (
handleHello) duplicate the tag/level pattern that was just centralized incaptureSessionMessage. Routing them through the helper keeps tagging consistent and shrinks the surface area.♻️ Proposed refactor
if (ingressSessionKey && ingressSessionKey !== expectedSessionKey) { - Sentry.captureMessage( + this.captureSessionMessage( 'Remote connector session rejected hello (session key mismatch).', { level: 'error', - tags: { - service: 'worker', - worker_component: 'home-connector-session', - }, extra: { connectorId: canonicalInstanceId, declaredKind, ingressSessionKey, expectedSessionKey, }, }, ) @@ if (!expectedSecret || message.sharedSecret !== expectedSecret) { - Sentry.captureMessage( + this.captureSessionMessage( 'Home connector session rejected websocket hello.', { level: 'error', - tags: { - service: 'worker', - worker_component: 'home-connector-session', - }, extra: { connectorId: canonicalInstanceId, declaredKind, hasExpectedSecret: Boolean(expectedSecret), }, }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/home/session.ts` around lines 271 - 317, Replace the two direct Sentry.captureMessage calls inside the handleHello logic with the centralized helper captureSessionMessage to ensure consistent tagging/leveling; call captureSessionMessage with the original descriptive message string ('Remote connector session rejected hello (session key mismatch).' and 'Home connector session rejected websocket hello.'), and pass the same extra objects (connectorId: canonicalInstanceId, declaredKind, ingressSessionKey/expectedSessionKey for the first, and connectorId: canonicalInstanceId, declaredKind, hasExpectedSecret: Boolean(expectedSecret) for the second) so all other behavior (ws.send, ws.close, return) remains unchanged.packages/home-connector/src/transport/worker-connector.ts (2)
432-462: Cap or omitrawMessagein the Sentry payload.
String(event.data)can be large (full JSON-RPC tool result payloads) and may include device-state that you don't want to retain in Sentry. Consider truncating to a few hundred chars and/or only including a fingerprint.- extra: { - rawMessage: String(event.data), - }, + extra: { + rawMessage: String(event.data).slice(0, 500), + rawMessageLength: String(event.data).length, + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home-connector/src/transport/worker-connector.ts` around lines 432 - 462, The Sentry payload currently includes rawMessage: String(event.data) which may be very large or contain sensitive device state; in captureHomeConnectorException (near updateConnectionState and createSocketEventContext usage) truncate or omit the raw message before sending to Sentry — for example, replace rawMessage with a capped string (e.g. first ~200–500 chars plus an indicator like "...[truncated]") or remove it and instead include a short fingerprint/metadata (length, type, and truncated preview) so the Sentry extra does not retain full JSON-RPC payloads or sensitive device data.
377-430: Drop redundant type guards inside theirswitchcases.Within
case 'server.ack':the discriminant is already'server.ack', soisAckMessage(value)is always true. Same forisJsonRpcEnvelope(value)insidecase 'connector.jsonrpc':. They're dead checks that obscure the actual condition (socket?.readyState === WebSocket.OPEN).♻️ Proposed refactor
- if (isAckMessage(value) && socket?.readyState === WebSocket.OPEN) { + if (socket?.readyState === WebSocket.OPEN) { captureHomeConnectorMessage( 'Sending home connector tools changed notification.', @@ case 'connector.jsonrpc': { const message = value.message - if (isJsonRpcEnvelope(value) && isJsonRpcResponse(message)) { + if (isJsonRpcResponse(message)) { updateConnectionState(input.state, { @@ if ( - isJsonRpcEnvelope(value) && isJsonRpcRequest(message) && socket?.readyState === WebSocket.OPEN ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home-connector/src/transport/worker-connector.ts` around lines 377 - 430, In the switch handling for 'server.ack' and 'connector.jsonrpc', remove the redundant discriminant checks (calls to isAckMessage(value) inside case 'server.ack' and isJsonRpcEnvelope(value) inside case 'connector.jsonrpc') and rely on the switch discriminant; keep the real runtime guards such as socket?.readyState === WebSocket.OPEN and the JSON-RPC specific checks (isJsonRpcRequest / isJsonRpcResponse) that determine behavior. Specifically, edit the blocks around the captureHomeConnectorMessage/socket.send using stringifyHomeConnectorMessage/createToolsChangedNotification in the 'server.ack' branch to drop isAckMessage(value) and only check socket state, and in the 'connector.jsonrpc' branch drop isJsonRpcEnvelope(value) while still using isJsonRpcResponse(message) and isJsonRpcRequest(message) to decide when to call updateConnectionState and handleJsonRpcRequest and to send responses.packages/home-connector/server/index.ts (2)
22-69:closeSentryparameter is alwaystrue— consider inlining.Every call site (signal handlers, uncaughtException, unhandledRejection) passes
true. The flush branch is unreachable from here; the onlyflushHomeConnectorSentrycall lives in the outer startupcatch(Line 131) and bypassesshutdownentirely. Drop the parameter to reduce indirection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home-connector/server/index.ts` around lines 22 - 69, The shutdown function always receives closeSentry=true, so remove the closeSentry parameter and make shutdown always call await closeHomeConnectorSentry(); update the shutdown implementation (function shutdown(reason: string)) to delete the conditional branch that calls flushHomeConnectorSentry and instead always await closeHomeConnectorSentry(), and update all call sites (the process.once handlers for 'SIGINT'/'SIGTERM', 'uncaughtException', 'unhandledRejection' that currently call shutdown(..., true)) to call shutdown(reason) and keep existing .finally(process.exit(...)) logic; leave the separate startup catch that uses flushHomeConnectorSentry intact so flushing remains explicit there.
47-69: Capture bothreasonandpromisefor unhandled rejections.The
unhandledRejectioncallback receives(reason, promise). Including the promise reference (or at least a marker) in the Sentry context makes it easier to correlate the rejection with the originating call site, especially when the same error type can come from multiple async chains.- process.once('unhandledRejection', (reason) => { + process.once('unhandledRejection', (reason, promise) => { captureHomeConnectorException(reason, { tags: { area: 'process', process_event: 'unhandledRejection', }, + extra: { + promise: String(promise), + }, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home-connector/server/index.ts` around lines 47 - 69, The unhandledRejection handler currently only captures the reason; update the listener signature in the process.once('unhandledRejection', ...) callback to accept (reason, promise) and include the promise reference in the Sentry capture call (captureHomeConnectorException) — e.g., add the promise to the event context as an extra field or tag so it’s logged alongside reason and process_event; keep the shutdown('unhandledRejection', true) call unchanged but ensure any typing for the listener reflects the two-argument signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/home-connector/server/index.ts`:
- Around line 22-37: The shutdown function can hang because input.server.close()
waits for all connections to drain; update shutdown (and keep
input.connector.workerConnector.stop usage) to add a watchdog that proactively
terminates idle/hung connections: first attempt graceful close via
input.server.close(), then if it doesn't finish within a short timeout (e.g.,
few seconds of orchestrator grace) call input.server.closeAllConnections() or
iterate and destroy sockets to forcibly terminate connections, then proceed to
flush/close Sentry by calling closeHomeConnectorSentry/flushHomeConnectorSentry
as before; also simplify the always-true closeSentry parameter by removing it
and using a constant or inlining the chosen Sentry-close behavior.
In `@packages/worker/src/home/session.ts`:
- Around line 363-379: The catch block in the refreshToolsSnapshot sequence
rethrows the error (in the try/catch around this.refreshToolsSnapshot), which
causes an unhandled rejection because handleJsonRpcMessage (called from
handleWebSocketMessage and invoked as void in webSocketMessage) doesn't await
errors; instead of rethrowing from the catch, stop propagation by returning
after calling this.clearConnectionState(), this.captureSessionMessage(...), and
await this.persistState(); modify the catch to remove "throw error" and simply
return so the error is logged/persisted but not bubbled to
handleJsonRpcMessage/handleWebSocketMessage.
---
Nitpick comments:
In `@packages/home-connector/server/index.ts`:
- Around line 22-69: The shutdown function always receives closeSentry=true, so
remove the closeSentry parameter and make shutdown always call await
closeHomeConnectorSentry(); update the shutdown implementation (function
shutdown(reason: string)) to delete the conditional branch that calls
flushHomeConnectorSentry and instead always await closeHomeConnectorSentry(),
and update all call sites (the process.once handlers for 'SIGINT'/'SIGTERM',
'uncaughtException', 'unhandledRejection' that currently call shutdown(...,
true)) to call shutdown(reason) and keep existing .finally(process.exit(...))
logic; leave the separate startup catch that uses flushHomeConnectorSentry
intact so flushing remains explicit there.
- Around line 47-69: The unhandledRejection handler currently only captures the
reason; update the listener signature in the process.once('unhandledRejection',
...) callback to accept (reason, promise) and include the promise reference in
the Sentry capture call (captureHomeConnectorException) — e.g., add the promise
to the event context as an extra field or tag so it’s logged alongside reason
and process_event; keep the shutdown('unhandledRejection', true) call unchanged
but ensure any typing for the listener reflects the two-argument signature.
In `@packages/home-connector/src/transport/worker-connector.ts`:
- Around line 432-462: The Sentry payload currently includes rawMessage:
String(event.data) which may be very large or contain sensitive device state; in
captureHomeConnectorException (near updateConnectionState and
createSocketEventContext usage) truncate or omit the raw message before sending
to Sentry — for example, replace rawMessage with a capped string (e.g. first
~200–500 chars plus an indicator like "...[truncated]") or remove it and instead
include a short fingerprint/metadata (length, type, and truncated preview) so
the Sentry extra does not retain full JSON-RPC payloads or sensitive device
data.
- Around line 377-430: In the switch handling for 'server.ack' and
'connector.jsonrpc', remove the redundant discriminant checks (calls to
isAckMessage(value) inside case 'server.ack' and isJsonRpcEnvelope(value) inside
case 'connector.jsonrpc') and rely on the switch discriminant; keep the real
runtime guards such as socket?.readyState === WebSocket.OPEN and the JSON-RPC
specific checks (isJsonRpcRequest / isJsonRpcResponse) that determine behavior.
Specifically, edit the blocks around the captureHomeConnectorMessage/socket.send
using stringifyHomeConnectorMessage/createToolsChangedNotification in the
'server.ack' branch to drop isAckMessage(value) and only check socket state, and
in the 'connector.jsonrpc' branch drop isJsonRpcEnvelope(value) while still
using isJsonRpcResponse(message) and isJsonRpcRequest(message) to decide when to
call updateConnectionState and handleJsonRpcRequest and to send responses.
In `@packages/worker/src/home/session.ts`:
- Around line 271-317: Replace the two direct Sentry.captureMessage calls inside
the handleHello logic with the centralized helper captureSessionMessage to
ensure consistent tagging/leveling; call captureSessionMessage with the original
descriptive message string ('Remote connector session rejected hello (session
key mismatch).' and 'Home connector session rejected websocket hello.'), and
pass the same extra objects (connectorId: canonicalInstanceId, declaredKind,
ingressSessionKey/expectedSessionKey for the first, and connectorId:
canonicalInstanceId, declaredKind, hasExpectedSecret: Boolean(expectedSecret)
for the second) so all other behavior (ws.send, ws.close, return) remains
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dbd42e5b-31c0-4358-bb7d-36105b7519e3
📒 Files selected for processing (8)
packages/home-connector/Dockerfilepackages/home-connector/index.tspackages/home-connector/server/index.tspackages/home-connector/src/sentry.node.test.tspackages/home-connector/src/sentry.tspackages/home-connector/src/transport/worker-connector.tspackages/worker/src/home/session.node.test.tspackages/worker/src/home/session.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/home-connector/src/transport/worker-connector.ts (2)
364-414: Ack telemetry loses the "reconnects-before-recovery" signal.
consecutiveReconnectsis reset to0on line 366 before being passed intocreateSocketEventContexton lines 386 and 402. That meanswebsocket.ackandtools.list_changed.sentalways reportconsecutiveReconnects: 0, which removes one of the most useful diagnostic signals (how many failed attempts preceded a successful ack).♻️ Proposed fix — preserve the pre-reset value for telemetry
case 'server.ack': hasReportedSocketIssue = false + const reconnectsBeforeAck = consecutiveReconnects consecutiveReconnects = 0 updateConnectionState(input.state, { connected: true, lastSyncAt: new Date().toISOString(), lastError: null, }) console.info( `Home connector websocket acknowledged connectorId=${value.connectorId}`, ) captureHomeConnectorMessage( 'Home connector websocket acknowledged.', { level: 'info', tags: { connector_event: 'websocket.ack', }, extra: { ...createSocketEventContext({ config: input.config, connectionAttempt, - consecutiveReconnects, + consecutiveReconnects: reconnectsBeforeAck, }), acknowledgedConnectorId: value.connectorId, }, }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home-connector/src/transport/worker-connector.ts` around lines 364 - 414, In the server.ack handler preserve the pre-reset consecutiveReconnects value for telemetry: store the current consecutiveReconnects into a temp (e.g. prevConsecutiveReconnects) before setting consecutiveReconnects = 0, then pass that temp into createSocketEventContext calls used by captureHomeConnectorMessage (the 'websocket.ack' and 'tools.list_changed.sent' events) so telemetry reflects how many reconnects preceded recovery; reset consecutiveReconnects after capturing/logging using the preserved value.
246-262: Consider breadcrumbs (or sampling) for info-level lifecycle events.Each successful connect now emits ≥4 info-level
captureMessageevents (websocket.connecting,websocket.open,websocket.hello_sent,websocket.ack) plustools.list_changed.sent, and every flap adds awebsocket.reconnect_scheduled. On a flaky network this becomes an unbounded stream of billable Sentry events with no actionable signal.Two options to keep observability without the noise:
- Switch the
info-level emissions toSentry.addBreadcrumb({...})so they ride along with subsequent error events for free.- Or keep
captureMessagebut gate viaSentry.withScope+ a short-window dedupe / sample rate so reconnect storms don't multiply.Errors/warnings (
server.error,websocket.message_error,websocket.close,websocket.error) should remain as full captures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home-connector/src/transport/worker-connector.ts` around lines 246 - 262, The info-level lifecycle emission using captureHomeConnectorMessage (e.g., the 'Scheduling home connector websocket reconnect.' call that builds context via createSocketEventContext and includes reconnectDelayMs) is generating high-volume Sentry events; change these non-actionable info emissions to Sentry.addBreadcrumb(...) instead of capture/exception-level captures (or alternatively wrap them in Sentry.withScope and apply a short-window dedupe/sample before calling capture) so reconnect storms don't create billable events—leave error/warn captures (server.error, websocket.message_error, websocket.close, websocket.error) as full captureHomeConnectorMessage calls unchanged.packages/worker/src/home/session.ts (1)
62-77: LGTM — Sentry tagging consolidation is a clean improvement.The
captureSessionMessagehelper standardizesservice/worker_componenttagging across the four prior inlineSentry.captureMessagesites and makes the call sites considerably more readable. One micro-nit: theextrais conditionally spread, which is fine, but Sentry accepts an emptyextraobject equivalently — you could drop the conditional with no behavioral change. Optional, ignore if you prefer the explicit form.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/home/session.ts` around lines 62 - 77, In captureSessionMessage, simplify the Sentry options by always passing an extra object instead of conditionally spreading it: ensure the Sentry.captureMessage call within captureSessionMessage (the helper that sets level, tags, and extra) uses a guaranteed extra field (e.g., extra: input.extra ?? {}) so the conditional spread can be removed; update the captureSessionMessage implementation accordingly.packages/home-connector/server/index.ts (1)
58-68: Fatal-path handlers should flush Sentry synchronously, not run full async shutdown.Node.js official guidance states that
uncaughtExceptionsignals an undefined process state—synchronous cleanup only, then exit immediately. The current code runs the full graceful path:closeServerWithWatchdog()waits up to 5 seconds, thencloseHomeConnectorSentry()waits up to 2 seconds, for a 7-second window before exit. If the exception originated from corrupted internal state, that delay can cause additional harm rather than clean recovery.Replace
shutdown()withflushHomeConnectorSentry()for bothuncaughtExceptionandunhandledRejectionhandlers:Suggested adjustment
process.once('uncaughtException', (error) => { captureHomeConnectorException(error, { tags: { area: 'process', process_event: 'uncaughtException', }, }) - void shutdown('uncaughtException').finally(() => { - process.exit(1) - }) + void flushHomeConnectorSentry().finally(() => process.exit(1)) })process.once('unhandledRejection', (reason, promise) => { captureHomeConnectorException(reason, { tags: { area: 'process', process_event: 'unhandledRejection', }, extra: { promise: String(promise), }, }) - void shutdown('unhandledRejection').finally(() => { - process.exit(1) - }) + void flushHomeConnectorSentry().finally(() => process.exit(1)) })The signal handlers (SIGINT, SIGTERM) are correct as-is—the process state is clean there, so the full graceful shutdown is appropriate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home-connector/server/index.ts` around lines 58 - 68, The uncaughtException and unhandledRejection handlers should not call the async graceful shutdown; instead replace the async shutdown('...') invocation with a synchronous Sentry flush by calling flushHomeConnectorSentry() and then immediately exit (process.exit(1)); update the handlers that currently call shutdown(...) to call flushHomeConnectorSentry() and then process.exit(1) (refer to the existing process.once('uncaughtException', ...) and process.once('unhandledRejection', ...) handlers and the shutdown() call to locate the change), leaving SIGINT/SIGTERM handlers and the graceful shutdown flow unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/home-connector/server/index.ts`:
- Around line 70-83: The current unhandledRejection handler passes
extra.promise: String(promise) which only yields "[object Promise]"; update the
handler in the process.once('unhandledRejection', ...) block that calls
captureHomeConnectorException and shutdown: remove the extra.promise field
(since reason is already captured) or replace it with actionable data such as
capturing String(reason) when reason is not an Error (e.g., set extra.reason =
typeof reason === 'object' ? undefined : String(reason)); ensure
captureHomeConnectorException(...) still receives useful context and keep
shutdown('unhandledRejection') behavior unchanged.
In `@packages/worker/src/home/session.ts`:
- Around line 355-371: On refreshToolsSnapshot() failure inside the try/catch,
do not call clearConnectionState(); instead remove or clear only the tools
snapshot in state (e.g., reset this.stateSnapshot.persisted.tools or equivalent)
and then call this.captureSessionMessage(...) and await this.persistState() so
the transient RPC error doesn't zero connectedAt; keep clearConnectionState()
reserved for the websocket close handler and leave connectedAt intact so
handleHello() / notifications/tools/list_changed semantics remain correct.
---
Nitpick comments:
In `@packages/home-connector/server/index.ts`:
- Around line 58-68: The uncaughtException and unhandledRejection handlers
should not call the async graceful shutdown; instead replace the async
shutdown('...') invocation with a synchronous Sentry flush by calling
flushHomeConnectorSentry() and then immediately exit (process.exit(1)); update
the handlers that currently call shutdown(...) to call
flushHomeConnectorSentry() and then process.exit(1) (refer to the existing
process.once('uncaughtException', ...) and process.once('unhandledRejection',
...) handlers and the shutdown() call to locate the change), leaving
SIGINT/SIGTERM handlers and the graceful shutdown flow unchanged.
In `@packages/home-connector/src/transport/worker-connector.ts`:
- Around line 364-414: In the server.ack handler preserve the pre-reset
consecutiveReconnects value for telemetry: store the current
consecutiveReconnects into a temp (e.g. prevConsecutiveReconnects) before
setting consecutiveReconnects = 0, then pass that temp into
createSocketEventContext calls used by captureHomeConnectorMessage (the
'websocket.ack' and 'tools.list_changed.sent' events) so telemetry reflects how
many reconnects preceded recovery; reset consecutiveReconnects after
capturing/logging using the preserved value.
- Around line 246-262: The info-level lifecycle emission using
captureHomeConnectorMessage (e.g., the 'Scheduling home connector websocket
reconnect.' call that builds context via createSocketEventContext and includes
reconnectDelayMs) is generating high-volume Sentry events; change these
non-actionable info emissions to Sentry.addBreadcrumb(...) instead of
capture/exception-level captures (or alternatively wrap them in Sentry.withScope
and apply a short-window dedupe/sample before calling capture) so reconnect
storms don't create billable events—leave error/warn captures (server.error,
websocket.message_error, websocket.close, websocket.error) as full
captureHomeConnectorMessage calls unchanged.
In `@packages/worker/src/home/session.ts`:
- Around line 62-77: In captureSessionMessage, simplify the Sentry options by
always passing an extra object instead of conditionally spreading it: ensure the
Sentry.captureMessage call within captureSessionMessage (the helper that sets
level, tags, and extra) uses a guaranteed extra field (e.g., extra: input.extra
?? {}) so the conditional spread can be removed; update the
captureSessionMessage implementation accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c63c357e-0f43-498e-8491-c9417838bcee
📒 Files selected for processing (3)
packages/home-connector/server/index.tspackages/home-connector/src/transport/worker-connector.tspackages/worker/src/home/session.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/worker/src/home/session.ts (3)
270-282:⚠️ Potential issue | 🟠 MajorAvoid forwarding raw session keys to Sentry.
ingressSessionKeyandexpectedSessionKeyare auth-bearing tokens used to admit a connector to this DO; landing them verbatim in Sentryextraexposes them to anyone with project access and to any downstream PII pipeline. Even with rotation, anyone reading the issue can replay the key against the worker before rotation completes.Prefer a non-reversible fingerprint (and/or a boolean for the comparison result):
🛡️ Proposed redaction
this.captureSessionMessage( 'Remote connector session rejected hello (session key mismatch).', { level: 'error', extra: { connectorId: canonicalInstanceId, declaredKind, - ingressSessionKey, - expectedSessionKey, + ingressSessionKeyPresent: Boolean(ingressSessionKey), + sessionKeyMatches: ingressSessionKey === expectedSessionKey, }, }, )A short hashed prefix (e.g. first 8 chars of
sha256(key)) is also fine if you need to correlate with logs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/home/session.ts` around lines 270 - 282, The code currently sends raw auth-bearing tokens ingressSessionKey and expectedSessionKey in the captureSessionMessage extra payload; instead, compute non-reversible fingerprints (e.g. SHA-256 and use a short prefix like first 8 chars) for both keys and send those fingerprint fields plus a boolean sessionKeyMatch (false) rather than the raw values, and update the captureSessionMessage call site that builds the extra object (referencing ingressSessionKey, expectedSessionKey, and captureSessionMessage) to include ingressSessionKeyFingerprint, expectedSessionKeyFingerprint, and sessionKeyMatch: false instead of the original raw tokens.
124-257:⚠️ Potential issue | 🟡 MinorWebsocket handler is not wrapped end-to-end yet.
PR objective for
#268explicitly calls for capturing “unexpected async exceptions with connector id/attempt metadata” in the websocket handler. Today, only two paths are protected here:
- the
parseHomeConnectorMessagefailure (Lines 226–243), and- the
refreshToolsSnapshot()failure insidehandleJsonRpcMessage(Lines 355–371).Anything else thrown from
handleHello(e.g.persistState()storage failure,ws.sendafter socket teardown) orhandleHeartbeatbecomes an unhandled rejection becausewebSocketMessage(Line 128) dispatches withvoid this.handleWebSocketMessage(...). Those will not surface to Sentry from this site.Consider wrapping the dispatcher tail in a single
try/catchso all post-parse failures are reported with consistent metadata:♻️ Proposed fix
- switch (parsed.type) { - case 'connector.hello': - await this.handleHello(ws, parsed) - return - case 'connector.heartbeat': - await this.handleHeartbeat() - return - case 'connector.jsonrpc': - await this.handleJsonRpcMessage(parsed.message) - return - } + try { + switch (parsed.type) { + case 'connector.hello': + await this.handleHello(ws, parsed) + return + case 'connector.heartbeat': + await this.handleHeartbeat() + return + case 'connector.jsonrpc': + await this.handleJsonRpcMessage(parsed.message) + return + } + } catch (error) { + this.captureSessionMessage( + 'Home connector session message handler threw.', + { + level: 'error', + extra: { + connectorId: this.stateSnapshot.persisted.connectorId, + messageType: parsed.type, + error: error instanceof Error ? error.message : String(error), + }, + }, + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/home/session.ts` around lines 124 - 257, handleWebSocketMessage currently only catches parse errors and a specific refreshToolsSnapshot path, so exceptions thrown by handleHello, handleHeartbeat, handleJsonRpcMessage, persistState, or ws.send become unhandled; wrap the dispatch/switch block in handleWebSocketMessage (the code after parseHomeConnectorMessage) in a single try/catch that catches any error, calls this.captureSessionMessage with a clear message and extra metadata (include this.stateSnapshot.persisted.connectorId and any attempt/session identifiers), include the error message and stack, attempt to send a server.error via ws.send if the socket is open, then perform cleanup (clearConnectionState()/persistState() as appropriate) and return to avoid unhandled rejections; reference the methods handleHello, handleHeartbeat, handleJsonRpcMessage, captureSessionMessage, persistState, clearConnectionState, and webSocketMessage when locating the code to change.
167-182:⚠️ Potential issue | 🟡 MinorReconnect path leaves
toolsempty until the connector sendstools/list_changed.
clearConnectionState()(line 59) emptiesthis.stateSnapshot.tools = []on every disconnect viawebSocketClose. WhenhandleHelloreconnects (lines 320–335), it restoresconnectorId/connectorKind/connectedAt/lastSeenAtbut does not callrefreshToolsSnapshot(), leaving tools empty. They only repopulate when the connector sends anotifications/tools/list_changednotification (line 353–356).During the reconnect window,
getSnapshot()will return a valid snapshot withtools: []. Downstream,home/index.tsshort-circuits onsnapshot.tools.length === 0, buthome/status.tsonly checksif (!snapshot)— so the status surface briefly shows a connected connector with zero tools.Two options worth considering:
- Call
void this.refreshToolsSnapshot()(with the same try/catch + Sentry handling) at the end ofhandleHelloafter the ack, so a freshly re-handshaked socket is immediately re-populated.- Or persist
toolsacross disconnects and only clear on hello if the connector identity changed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/home/session.ts` around lines 167 - 182, Reconnection currently clears this.stateSnapshot.tools in clearConnectionState and never repopulates it in handleHello, so getSnapshot returns tools: [] until the connector sends a tools/list_changed notification; fix by invoking refreshToolsSnapshot() at the end of handleHello (after the ack and within the same try/catch + Sentry handling used elsewhere) so tools are reloaded immediately upon reconnection; alternatively, modify clearConnectionState to preserve stateSnapshot.tools across disconnects and only clear them in handleHello if connector identity changed—use the functions clearConnectionState, handleHello, refreshToolsSnapshot and the stateSnapshot.tools field to locate where to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/home-connector/src/transport/worker-connector.ts`:
- Around line 365-418: Wrap the entire case 'server.ack' body in its own block
(change to case 'server.ack': { ... }) so the const
previousConsecutiveReconnects is lexically scoped to that case and cannot be
accessed by sibling cases; update the block to include the existing references
to previousConsecutiveReconnects, consecutiveReconnects, socket,
updateConnectionState and addHomeConnectorSentryBreadcrumb exactly as they are
now, then close the block and keep the final return; this mirrors the case
'connector.jsonrpc' style and will satisfy the Biome noSwitchDeclarations rule.
- Around line 247-263: All six calls to addHomeConnectorSentryBreadcrumb use the
wrong signature (two args) — change each to pass a single object with keys {
message, category, level?, data? }; set message to the string currently passed,
derive category from the connector_event value (e.g.,
'websocket.reconnect_scheduled'), and merge the current tags and extra payload
(including the result of createSocketEventContext(...) and reconnectDelayMs or
other extras) into the data field; keep level if present. Update call sites near
addHomeConnectorSentryBreadcrumb and any uses of createSocketEventContext to
ensure the combined map is passed as data.
---
Outside diff comments:
In `@packages/worker/src/home/session.ts`:
- Around line 270-282: The code currently sends raw auth-bearing tokens
ingressSessionKey and expectedSessionKey in the captureSessionMessage extra
payload; instead, compute non-reversible fingerprints (e.g. SHA-256 and use a
short prefix like first 8 chars) for both keys and send those fingerprint fields
plus a boolean sessionKeyMatch (false) rather than the raw values, and update
the captureSessionMessage call site that builds the extra object (referencing
ingressSessionKey, expectedSessionKey, and captureSessionMessage) to include
ingressSessionKeyFingerprint, expectedSessionKeyFingerprint, and
sessionKeyMatch: false instead of the original raw tokens.
- Around line 124-257: handleWebSocketMessage currently only catches parse
errors and a specific refreshToolsSnapshot path, so exceptions thrown by
handleHello, handleHeartbeat, handleJsonRpcMessage, persistState, or ws.send
become unhandled; wrap the dispatch/switch block in handleWebSocketMessage (the
code after parseHomeConnectorMessage) in a single try/catch that catches any
error, calls this.captureSessionMessage with a clear message and extra metadata
(include this.stateSnapshot.persisted.connectorId and any attempt/session
identifiers), include the error message and stack, attempt to send a
server.error via ws.send if the socket is open, then perform cleanup
(clearConnectionState()/persistState() as appropriate) and return to avoid
unhandled rejections; reference the methods handleHello, handleHeartbeat,
handleJsonRpcMessage, captureSessionMessage, persistState, clearConnectionState,
and webSocketMessage when locating the code to change.
- Around line 167-182: Reconnection currently clears this.stateSnapshot.tools in
clearConnectionState and never repopulates it in handleHello, so getSnapshot
returns tools: [] until the connector sends a tools/list_changed notification;
fix by invoking refreshToolsSnapshot() at the end of handleHello (after the ack
and within the same try/catch + Sentry handling used elsewhere) so tools are
reloaded immediately upon reconnection; alternatively, modify
clearConnectionState to preserve stateSnapshot.tools across disconnects and only
clear them in handleHello if connector identity changed—use the functions
clearConnectionState, handleHello, refreshToolsSnapshot and the
stateSnapshot.tools field to locate where to apply the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3680c6df-5eee-4b59-97d7-9941e3906c56
📒 Files selected for processing (5)
packages/home-connector/server/index.tspackages/home-connector/src/sentry.node.test.tspackages/home-connector/src/sentry.tspackages/home-connector/src/transport/worker-connector.tspackages/worker/src/home/session.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/home-connector/src/sentry.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
cursor
Bot
force-pushed
the
cursor/home-connector-sentry-issues-5e68
branch
from
221bd9c to
eab74f8
Compare
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/src/home/session.ts (1)
141-161:⚠️ Potential issue | 🟡 MinorReject in-flight
pendingRequestson websocket close.
webSocketCloseclears connection state but leaves thependingRequestsMap intact. Any RPC promise outstanding at the moment of disconnect (e.g., atools/callinitiated from/rpc/tools-callat lines 107–119, or atools/listfromrefreshToolsSnapshot) will now hang for the fullrpcTimeoutMs(15s) before itssetTimeoutfires — there’s no way for a response to arrive once the socket is gone. The associated HTTP caller blocks for that duration and the timer keeps the DO from sleeping.Reject pending requests immediately on close so callers fail fast:
🛡️ Suggested fix
webSocketClose( _ws: WebSocket, code: number, reason: string, wasClean: boolean, ): void { this.stateSnapshot.persisted.lastSeenAt = new Date().toISOString() this.clearConnectionState() + for (const [id, pending] of this.pendingRequests) { + clearTimeout(pending.timeout) + pending.reject( + new Error( + `Home connector websocket closed before RPC ${id} responded.`, + ), + ) + } + this.pendingRequests.clear() const closeMessage = `Home connector session websocket closed code=${code} wasClean=${wasClean}${reason ? ` reason=${reason}` : ''}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/home/session.ts` around lines 141 - 161, The webSocketClose handler currently clears connection state but leaves the pendingRequests Map unresolved causing RPC callers to hang; update webSocketClose to iterate over this.pendingRequests (or the Map used for in-flight RPCs), call each request's reject callback (or reject Promise) with an Error describing the websocket close (include code and reason), clear the Map, then continue with clearConnectionState() and persistState() as before so callers fail fast and resources are released; reference the webSocketClose method and the pendingRequests Map when making this change.
🧹 Nitpick comments (2)
packages/home-connector/server/index.ts (2)
70-95:promiseConstructoris essentially always'Promise'— drop it.The runtime always invokes the
unhandledRejectionlistener with an actualPromiseinstance, so the newpromiseConstructorextra resolves to the literal string'Promise'for all practical purposes. It’s the same low-signal field the past review flagged, just spelled differently. Either remove it or replace with something actionable.♻️ Suggested cleanup
captureHomeConnectorException(reason, { tags: { area: 'process', process_event: 'unhandledRejection', }, extra: { ...(typeof reason === 'string' || typeof reason === 'number' || typeof reason === 'boolean' ? { reason: String(reason) } : {}), - promiseConstructor: - promise && - typeof promise === 'object' && - 'constructor' in promise && - typeof promise.constructor === 'function' && - promise.constructor.name - ? promise.constructor.name - : 'Promise', }, })If you do want something useful for grouping, consider
reasonType: typeof reasonorreasonName: reason instanceof Error ? reason.name : undefined— both add diagnostic valuenormalizeErrordoesn’t already cover.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home-connector/server/index.ts` around lines 70 - 95, The extra field promiseConstructor added in the unhandledRejection handler is low-signal and effectively always "Promise"; update the process.once('unhandledRejection', ...) handler in this file by removing the promiseConstructor entry from the extra payload passed to captureHomeConnectorException, or replace it with a useful diagnostic such as reasonType: typeof reason or reasonName: reason instanceof Error ? reason.name : undefined; keep the rest of the captureHomeConnectorException call and the flushHomeConnectorSentry().finally(() => process.exit(1)) logic unchanged.
58-68: Minor:flushHomeConnectorSentryvscloseHomeConnectorSentrydivergence.The signal path uses
closeHomeConnectorSentry()while the fataluncaughtException/unhandledRejectionpaths useflushHomeConnectorSentry(). That's defensible —flushis the right primitive when you're about toprocess.exit(1)immediately and don't need the client to refuse further events — just flagging it so the inconsistency is intentional rather than incidental.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home-connector/server/index.ts` around lines 58 - 68, The handlers use two different shutdown primitives (flushHomeConnectorSentry vs closeHomeConnectorSentry); make the divergence explicit and consistent: keep using flushHomeConnectorSentry() in fatal handlers (captureHomeConnectorException in uncaughtException/unhandledRejection) because we immediately call process.exit(1), and keep using closeHomeConnectorSentry() in signal handlers where we want the client to stop accepting events; add a short inline comment near each handler (uncaughtException/unhandledRejection and the signal handlers) explaining why flush vs close is chosen so the intent is clear to future readers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/worker/src/home/session.ts`:
- Around line 141-161: The webSocketClose handler currently clears connection
state but leaves the pendingRequests Map unresolved causing RPC callers to hang;
update webSocketClose to iterate over this.pendingRequests (or the Map used for
in-flight RPCs), call each request's reject callback (or reject Promise) with an
Error describing the websocket close (include code and reason), clear the Map,
then continue with clearConnectionState() and persistState() as before so
callers fail fast and resources are released; reference the webSocketClose
method and the pendingRequests Map when making this change.
---
Nitpick comments:
In `@packages/home-connector/server/index.ts`:
- Around line 70-95: The extra field promiseConstructor added in the
unhandledRejection handler is low-signal and effectively always "Promise";
update the process.once('unhandledRejection', ...) handler in this file by
removing the promiseConstructor entry from the extra payload passed to
captureHomeConnectorException, or replace it with a useful diagnostic such as
reasonType: typeof reason or reasonName: reason instanceof Error ? reason.name :
undefined; keep the rest of the captureHomeConnectorException call and the
flushHomeConnectorSentry().finally(() => process.exit(1)) logic unchanged.
- Around line 58-68: The handlers use two different shutdown primitives
(flushHomeConnectorSentry vs closeHomeConnectorSentry); make the divergence
explicit and consistent: keep using flushHomeConnectorSentry() in fatal handlers
(captureHomeConnectorException in uncaughtException/unhandledRejection) because
we immediately call process.exit(1), and keep using closeHomeConnectorSentry()
in signal handlers where we want the client to stop accepting events; add a
short inline comment near each handler (uncaughtException/unhandledRejection and
the signal handlers) explaining why flush vs close is chosen so the intent is
clear to future readers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8df026c1-a7ef-4290-bbb4-ad963684cead
📒 Files selected for processing (8)
packages/home-connector/Dockerfilepackages/home-connector/index.tspackages/home-connector/server/index.tspackages/home-connector/src/sentry.node.test.tspackages/home-connector/src/sentry.tspackages/home-connector/src/transport/worker-connector.tspackages/worker/src/home/session.node.test.tspackages/worker/src/home/session.ts
✅ Files skipped from review due to trivial changes (2)
- packages/home-connector/index.ts
- packages/home-connector/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/home-connector/src/sentry.node.test.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/worker/src/home/session.node.test.ts (1)
91-171: Solid coverage of the new socket-presence branching.Tests 2 and 3 nicely exercise both the "last socket closing" and "stale socket among many" paths through
webSocketClose. Two small enhancements you could consider in follow-up tests (no need to block on these):
- Assert that pending RPC requests are rejected when the last socket closes (e.g., issue an
/rpc/tools-listPOST first, then close the socket and assert the response is a 502 with thebefore RPC responsereason). This would lock in the newrejectPendingRequestscontract at lines 161–163 ofsession.ts.- Assert that test 3 (stale-socket close) still emits the
captureSessionMessagewarning, since the close-warning capture at lines 167–175 ofsession.tsis unconditional even when other sockets remain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/home/session.node.test.ts` around lines 91 - 171, Add assertions to the two tests to cover the additional behaviors: in the "websocket close clears..." test, open a pending RPC (e.g., POST /rpc/tools-list) before closing the last socket and assert that the pending request is rejected with a 502 and reason "before RPC response" to exercise HomeConnectorSession.rejectPendingRequests via webSocketClose; in the "stale websocket close preserves..." test, assert that captureMessageMock was called with the websocket-close warning (the captureSessionMessage call inside HomeConnectorSession.webSocketClose) even though other sockets remain; locate these by referring to HomeConnectorSession.webSocketClose and the rejectPendingRequests/captureSessionMessage behavior in session.ts.packages/worker/src/home/session.ts (1)
250-270: Symmetry: wrapws.sendin the parse-error path with the same try/catch as the dispatch path.The dispatch error path at lines 296–305 defensively wraps
ws.sendin an inner try/catch with the comment "Ignore send failures while we're already handling a websocket error." The parse-error branch here doesn't, so if the socket is already half-closed when an invalid payload arrives,ws.sendcan throw and surface as an unhandled rejection (sincewebSocketMessageat line 146 fire-and-forgets viavoid). Low-impact edge case but trivial to harden for consistency.♻️ Suggested adjustment
- ws.send( - stringifyHomeConnectorMessage({ - type: 'server.error', - message: error instanceof Error ? error.message : String(error), - }), - ) + try { + ws.send( + stringifyHomeConnectorMessage({ + type: 'server.error', + message: error instanceof Error ? error.message : String(error), + }), + ) + } catch { + // Ignore send failures while we're already handling a websocket error. + } return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/home/session.ts` around lines 250 - 270, The parse-error branch that calls ws.send after parseHomeConnectorMessage throws should wrap that ws.send in the same defensive try/catch used in the dispatch path (see the inner try/catch around ws.send in the dispatch/error handling) to ignore send failures while already handling a websocket error; update the block right after captureSessionMessage so the stringifyHomeConnectorMessage(...) send is performed inside a try { ws.send(...) } catch { /* Ignore send failures while we're already handling a websocket error. */ } to avoid unhandled rejections from webSocketMessage fire-and-forget callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/worker/src/home/session.node.test.ts`:
- Around line 91-171: Add assertions to the two tests to cover the additional
behaviors: in the "websocket close clears..." test, open a pending RPC (e.g.,
POST /rpc/tools-list) before closing the last socket and assert that the pending
request is rejected with a 502 and reason "before RPC response" to exercise
HomeConnectorSession.rejectPendingRequests via webSocketClose; in the "stale
websocket close preserves..." test, assert that captureMessageMock was called
with the websocket-close warning (the captureSessionMessage call inside
HomeConnectorSession.webSocketClose) even though other sockets remain; locate
these by referring to HomeConnectorSession.webSocketClose and the
rejectPendingRequests/captureSessionMessage behavior in session.ts.
In `@packages/worker/src/home/session.ts`:
- Around line 250-270: The parse-error branch that calls ws.send after
parseHomeConnectorMessage throws should wrap that ws.send in the same defensive
try/catch used in the dispatch path (see the inner try/catch around ws.send in
the dispatch/error handling) to ignore send failures while already handling a
websocket error; update the block right after captureSessionMessage so the
stringifyHomeConnectorMessage(...) send is performed inside a try { ws.send(...)
} catch { /* Ignore send failures while we're already handling a websocket
error. */ } to avoid unhandled rejections from webSocketMessage fire-and-forget
callers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d78027d-37af-49c7-af28-83fd0c390e56
📒 Files selected for processing (3)
packages/home-connector/index.tspackages/worker/src/home/session.node.test.tspackages/worker/src/home/session.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/home-connector/index.ts
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b17f6c7. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
This was referenced Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
npm run test -- --run packages/home-connector/src/sentry.node.test.ts packages/worker/src/home/session.node.test.ts packages/worker/src/mcp/capabilities/meta/meta-get-home-connector-status.node.test.tsnpm run typechecknpm run dev:curl http://127.0.0.1:3742/healthcurl http://127.0.0.1:4040/healthcurl http://127.0.0.1:3742/home/connectors/default/snapshot(returned a populated live tool snapshot includingbond_shade_set_position)Closes #268
Summary by CodeRabbit
New Features
Bug Fixes
Tests