Repository navigation
Bound stalled remote connector snapshot reads - #847
Merged
Merged
Conversation
📝 WalkthroughWalkthroughRemote connector snapshot retrieval now times out stalled requests, retries after timeout, reports timeout status, and isolates failed connectors during dynamic capability registry construction. ChangesRemote connector snapshot resilience
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CapabilityRegistry
participant SnapshotCache
participant RemoteConnector
CapabilityRegistry->>SnapshotCache: load connector snapshots
SnapshotCache->>RemoteConnector: request getSnapshot()
alt snapshot resolves
RemoteConnector-->>SnapshotCache: return snapshot
else snapshot stalls
SnapshotCache-->>CapabilityRegistry: return timeout failure
CapabilityRegistry-->>CapabilityRegistry: exclude failed connector
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kody-bot
marked this pull request as ready for review
July 22, 2026 04:40
Contributor
|
🔎 Preview deployed: https://kody-pr-847.kody-a99.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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/remote-connector/snapshot-cache.ts`:
- Around line 18-39: Update getSnapshotWithTimeout and its callers to prevent
repeated snapshot RPCs after a timeout: either propagate cancellation to the
in-flight getSnapshot operation or add a short timeout-specific
negative-cache/backoff window before allowing another lookup. Preserve normal
successful and null snapshot behavior, and ensure timed-out connector requests
cannot trigger an immediate fresh stub.getSnapshot call.
🪄 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: 90a88c6e-4935-4673-81ec-e8b4a0e09ce0
📒 Files selected for processing (4)
packages/worker/src/mcp/capabilities/registry.node.test.tspackages/worker/src/mcp/capabilities/registry.tspackages/worker/src/remote-connector/snapshot-cache.node.test.tspackages/worker/src/remote-connector/snapshot-cache.ts
Comment on lines
+18
to
+39
| async function getSnapshotWithTimeout(input: { | ||
| instanceId: string | ||
| getSnapshot: () => Promise<RemoteConnectorSnapshot | null> | ||
| }) { | ||
| let timeoutId: ReturnType<typeof setTimeout> | undefined | ||
| try { | ||
| return await Promise.race([ | ||
| input.getSnapshot(), | ||
| new Promise<never>((_resolve, reject) => { | ||
| timeoutId = setTimeout( | ||
| () => | ||
| reject(new RemoteConnectorSnapshotTimeoutError(input.instanceId)), | ||
| remoteConnectorSnapshotTimeoutMs, | ||
| ) | ||
| }), | ||
| ]) | ||
| } finally { | ||
| if (timeoutId !== undefined) { | ||
| clearTimeout(timeoutId) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the pending-cache implementation to confirm eviction/backoff behavior on rejection.
sed -n '1,56p' packages/worker/src/remote-connector/snapshot-cache.tsRepository: kentcdodds/kody
Length of output: 1836
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== snapshot-cache usage =="
rg -n "getSnapshotWithTimeout|remoteConnectorSnapshotCache|createRemoteConnectorSnapshotCacheKey|RemoteConnectorSnapshotTimeoutError" packages/worker/src -S
echo
echo "== PromiseLruCache definition =="
fd -a "published-package-cache.ts|package-registry" packages/worker/src
rg -n "class PromiseLruCache|PromiseLruCache" packages/worker/src -S
echo
echo "== snapshot-cache tests =="
fd -a "snapshot-cache" packages/worker/src test packages
rg -n "RemoteConnectorSnapshotTimeoutError|snapshot cache|getSnapshotWithTimeout|PromiseLruCache" packages/worker/src test packages -SRepository: kentcdodds/kody
Length of output: 9327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/worker/src/package-registry/published-package-cache.ts =="
wc -l packages/worker/src/package-registry/published-package-cache.ts
sed -n '1,220p' packages/worker/src/package-registry/published-package-cache.ts
echo
echo "== packages/worker/src/remote-connector/snapshot-cache.ts =="
sed -n '1,160p' packages/worker/src/remote-connector/snapshot-cache.ts
echo
echo "== packages/worker/src/remote-connector/snapshot-cache.node.test.ts (relevant section) =="
sed -n '1,280p' packages/worker/src/remote-connector/snapshot-cache.node.test.tsRepository: kentcdodds/kody
Length of output: 11628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('packages/worker/src/package-registry/published-package-cache.ts')
print(p.read_text())
PYRepository: kentcdodds/kody
Length of output: 2336
Add backoff or cancellation for timed-out snapshot RPCs Timed-out entries are evicted, so a persistently stalled connector will start a fresh stub.getSnapshot() on each later lookup while the previous RPC can still remain open. Consider a short negative-cache/backoff window after timeout, or propagating cancellation into the DO call.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/src/remote-connector/snapshot-cache.ts` around lines 18 - 39,
Update getSnapshotWithTimeout and its callers to prevent repeated snapshot RPCs
after a timeout: either propagate cancellation to the in-flight getSnapshot
operation or add a short timeout-specific negative-cache/backoff window before
allowing another lookup. Preserve normal successful and null snapshot behavior,
and ensure timed-out connector requests cannot trigger an immediate fresh
stub.getSnapshot call.
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
3 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
Validation
npm run validatepassedSystem recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@711d7885· Head:3b3857c5Classification: extends — bounds remote-connector snapshot latency and makes capability discovery fail softly per connector.
Primitives touched
capability-registryconnector-ingressremote-connectorsSystem map
Remote connector snapshot reads feed capability discovery; this change bounds that edge and isolates failures to the affected connector.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Invariants
Connector snapshots and cache keys remain user-scoped; fail-soft behavior never substitutes another user's connector data.
Summary by CodeRabbit