Staging#543
Merged
Merged
Conversation
Remove `.cjs` from Metro's assetExts configuration. It's already in Expo's default sourceExts, so registering it as an asset extension caused Metro to bundle CommonJS entry points (e.g., superstruct's dist/index.cjs) as static assets instead of code. Their exports resolved to undefined, crashing the app at module load with "undefined is not a function" → trailing "Cannot read property 'ErrorBoundary' of undefined" in expo-router. This affected OTA update 30.21.1 and any subsequent OTA until fixed. Never add `.cjs` to assetExts. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Create new zero-dependency module `src/auth/devSandboxFlag.ts` exporting `isDevSandboxEnabled()` to break synchronous circular import: bootstrapSession → ensureDevSandboxCharacter → characterDatabase → index → wikiService → wikiLlmProvider → apiClient → bootstrapSession Two fixes: 1. Extract pure isDevSandboxEnabled() logic to devSandboxFlag.ts 2. Make characterMachine and bootstrapSession imports of ensureDevSandboxCharacter lazy via `await import()` inside conditionals (only load when actually needed) Import cycles still exist in the static graph, but lazy imports break the synchronous TDZ (Temporal Dead Zone) trap at module initialization that caused the crash. Update test mocks to import isDevSandboxEnabled from devSandboxFlag. Regenerate docs/flowcharts to reflect new module structure. Add --clear-cache to `npm run ota` script for deterministic bundling. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
fix: resolve OTA 30.21.1 crash - metro .cjs assetExts + auth cycle
Implements /agent/desktop persistent WebSocket, pairing routes, five vault_* ADK tools, and Firestore-routed task queue per the approved spec. Wire protocol matches CT v1.9.0; includes generation-guarded desktopBridge, device-doc revoke listener, and cap decay on timeout. Co-authored-by: Cursor <cursoragent@cursor.com>
…t-bridge feat(cloud-agent): desktop vault bridge for Curated Thoughts
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds a Desktop Vault Bridge to the cloud agent: pairing tokens, an in-memory WebSocket connection registry, a desktop WebSocket handler, Firestore-backed desktop task orchestration, and five ChangesDesktop Vault Bridge Feature
Dev Sandbox Flag Refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Refine vault tool timeout/cap logic, simplify test fixtures for desktop pairing and task workflows. Update bridge documentation with error codes and failure modes. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the Clanker side of the Desktop Vault Bridge: a persistent /agent/desktop WebSocket (pairing-token auth) that routes Firestore desktopTasks to a socket-owning Cloud Run instance, plus five read-only vault_* ADK tools wired into /agent/run and /agent/live. This also includes supporting docs, Firestore security rules, tests, and a small client-side refactor to isolate the dev-sandbox enablement flag.
Changes:
- Implemented desktop vault bridge server components in
cloud-agent/: pairing + revoke,desktopBridgeregistry,wsDesktopAgentHandler, Firestore task/device helpers, andvault_*tools (incl. wiring + tests). - Updated Firestore rules and documentation/specs/plans (incl. architecture and testing plan updates).
- Refactored dev sandbox gating into
src/auth/devSandboxFlag.tsand updated call sites/tests; adjusted Metro config comment and OTA script flag.
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/wikiLlmProvider.ts | Switch isDevSandboxEnabled import to shared flag module |
| src/services/characterSyncService.ts | Switch isDevSandboxEnabled import to shared flag module |
| src/services/aiChatService.ts | Switch isDevSandboxEnabled import to shared flag module |
| src/machines/liveVoiceMachine.ts | Switch isDevSandboxEnabled import to shared flag module |
| src/machines/characterMachine.ts | Lazy-load ensureDevSandboxCharacter; switch flag import |
| src/hooks/useTabCharacterId.ts | Switch isDevSandboxEnabled import to shared flag module |
| src/hooks/useAIChat.ts | Switch isDevSandboxEnabled import to shared flag module |
| src/auth/ensureDevSandboxCharacter.ts | Move flag logic to devSandboxFlag |
| src/auth/devSandboxFlag.ts | New: centralized dev-sandbox enablement predicate |
| src/auth/bootstrapSession.ts | Lazy-load ensureDevSandboxCharacter; switch flag import |
| README.md | Add Desktop Vault Bridge doc link |
| package.json | Add --clear-cache to OTA update script |
| metro.config.js | Document why .cjs must not be treated as an asset |
| firestore.rules | Deny all client access to users/{uid}/desktopTasks |
| docs/superpowers/specs/2026-07-05-desktop-vault-bridge-design.md | Spec amendments and expanded lifecycle/testing notes |
| docs/superpowers/plans/2026-07-05-desktop-vault-bridge.md | Plan updated to incorporate architecture review amendments |
| docs/flowcharts/SERVICES.md | Flowchart updates for dev-sandbox flag module |
| docs/flowcharts/MACHINES.md | Flowchart updates for dev-sandbox flag module |
| docs/flowcharts/HOOKS.md | Flowchart updates for dev-sandbox flag module |
| docs/desktop-vault-bridge.md | New operator/system documentation for the vault bridge |
| cloud-agent/src/tools/vaultToolsWiring.test.ts | New: tests tool presence gating in buildAgent/buildLiveTools |
| cloud-agent/src/tools/vaultTools.ts | New: vault_* ADK tools + Firestore task dispatch logic |
| cloud-agent/src/tools/vaultTools.test.ts | New: unit tests for vault tools + cap/decay/billing behaviors |
| cloud-agent/src/services/liveToolAdapter.ts | Wire vault tools into live (voice) toolset when deps provided |
| cloud-agent/src/services/firestoreSession.ts | Add desktop device/task APIs and snapshot support |
| cloud-agent/src/services/firestoreSession.test.ts | Add tests for desktop device filtering and desktopTasks lifecycle |
| cloud-agent/src/services/desktopPairing.ts | New: pairing token generation, resolve, and revoke logic |
| cloud-agent/src/services/desktopPairing.test.ts | New: pairing service unit tests |
| cloud-agent/src/services/desktopBridge.ts | New: per-instance desktopBridge registry w/ generation guard |
| cloud-agent/src/services/desktopBridge.test.ts | New: desktopBridge generation/replace behavior tests |
| cloud-agent/src/services/agentCore.ts | Wire vault tools into text agent when deps provided |
| cloud-agent/src/index.ts | Add /agent/desktop routes (HTTP + WS) and vault deps creation |
| cloud-agent/src/handlers/wsLiveAgentHandler.ts | Provide vault deps for live sessions; pass through to tool builder |
| cloud-agent/src/handlers/wsDesktopAgentHandler.ts | New: /agent/desktop WS auth, liveness, task dispatch/results |
| cloud-agent/src/handlers/wsDesktopAgentHandler.test.ts | New: handler auth/dispatch/liveness/revoke/pause tests |
| cloud-agent/scripts/desktopBridgeSmoke.ts | New: manual smoke client for local desktop bridge testing |
| app/_layout.tsx | Switch isDevSandboxEnabled import to shared flag module |
| tests/wikiLlmProvider.test.ts | Update mocks/imports for new dev-sandbox flag module |
| tests/useAIChat.test.tsx | Update mocks/imports for new dev-sandbox flag module |
| tests/liveVoiceMachine.test.ts | Update mocks/imports for new dev-sandbox flag module |
| tests/ensureDevSandboxCharacter.test.ts | Split imports across ensure + devSandboxFlag modules |
| tests/characterSyncWiki.test.ts | Update mock path for isDevSandboxEnabled |
| tests/characterSyncIdempotentUpload.test.ts | Update mock path for isDevSandboxEnabled |
| tests/bootstrapSession.test.ts | Update mocks to match new devSandboxFlag + lazy ensure import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kurt VanDusen <info@equationalapplications.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kurt VanDusen <info@equationalapplications.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kurt VanDusen <info@equationalapplications.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kurt VanDusen <info@equationalapplications.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kurt VanDusen <info@equationalapplications.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kurt VanDusen <info@equationalapplications.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kurt VanDusen <info@equationalapplications.com>
- Use static WebSocket.OPEN constant instead of instance property - Always unsubscribe listeners on disconnect; only mark offline for current generation - Fix indentation and use proper constant in dispatchLocalIfConnected - Allow task results for same-instance shortcut (warn on reject) - Check device before applying per-turn cap - Fix cap decay to not exceed maxCallsPerTurn - Inline failDesktopTaskIfUnresolved to avoid this binding issues Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Owner
Author
/fix-pr follow-upCommit: Review resolution
Verification
🤖 Generated by /fix-pr |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cloud-agent/src/handlers/wsLiveAgentHandler.ts (1)
148-241: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset vault-call budget at turn boundaries
vaultDepsis created once per live WebSocket session, but the per-turn counters increateVaultToolDepsnever reset. If a timeout/disconnect triggerscapDecayin one model turn, later turns in the same voice session stay artificially capped. HandleserverContent.turnCompleteand clearcallsThisTurn/capDecaythere.🤖 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 `@cloud-agent/src/handlers/wsLiveAgentHandler.ts` around lines 148 - 241, Reset the per-turn vault call budget in handleGeminiMessage by handling serverContent.turnComplete, since vaultDeps persists across the live WebSocket session and createVaultToolDeps currently keeps callsThisTurn/capDecay across turns. Add turn-boundary handling alongside the existing serverContent processing so that when turnComplete is received, those counters are cleared before the next turn starts.
🧹 Nitpick comments (2)
cloud-agent/src/tools/vaultTools.ts (1)
82-133: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo error handling around Firestore calls in
dispatchVaultCall.
fs.getActiveDesktopDeviceandfs.createDesktopTaskaren't wrapped in try/catch; unlike the craftedNO_DEVICE_MSG/TIMEOUT_MSG/CAP_MSGfriendly errors, a transient Firestore failure here would reject the tool's promise instead of returning a graceful message to the model.🤖 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 `@cloud-agent/src/tools/vaultTools.ts` around lines 82 - 133, Add explicit error handling in dispatchVaultCall around the Firestore-dependent calls so transient failures return a friendly tool message instead of rejecting. Wrap fs.getActiveDesktopDevice and fs.createDesktopTask (and any immediate dependent dispatch setup) in try/catch, and on failure return a stable Vault error string similar to NO_DEVICE_MSG and TIMEOUT_MSG. Keep the existing control flow in dispatchVaultCall and preserve billing pause/resume in the finally block.cloud-agent/src/index.ts (1)
71-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
defaultFirestoreSession()call forvault.
bridgeandvaultare both gated onadmin.apps.length, butvaultcallsdefaultFirestoreSession()again instead of reusingbridge.firestoreSession(aswsLiveAgentHandler.tsdoes for itsvaultDeps). Reusing the instance avoids constructing a redundant session object per/agent/runcall.♻️ Suggested fix
const bridge = admin.apps.length ? { firebaseUid, userId, firestoreSession: defaultFirestoreSession(), fcmDispatcher: defaultFcmDispatcher(), creditService: createCreditService(db), instanceId: INSTANCE_ID, } : undefined - const vault = admin.apps.length ? createVaultToolDeps({ + const vault = bridge ? createVaultToolDeps({ firebaseUid, - firestoreSession: defaultFirestoreSession(), + firestoreSession: bridge.firestoreSession, desktopBridge, }) : undefined🤖 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 `@cloud-agent/src/index.ts` around lines 71 - 86, The `runAgentReal` setup is creating two separate `defaultFirestoreSession()` instances for `bridge` and `vault`, which is redundant. Reuse the existing session from `bridge` when building `vault` so the `createVaultToolDeps` call uses the same firestore session object, matching the `wsLiveAgentHandler` pattern and avoiding an extra session construction per request.
🤖 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 `@cloud-agent/src/handlers/wsDesktopAgentHandler.ts`:
- Around line 110-124: The task dispatch flow in wsDesktopAgentHandler’s
watchPendingDesktopTasks callback keeps task IDs in the dispatched set even when
fs.markDesktopTaskExecuting fails, which blocks retries on later watcher
emissions. Update the markDesktopTaskExecuting promise chain so that the taskId
is removed from dispatched inside the .catch path before logging the failure,
allowing the same socket to retry that task on subsequent updates.
In `@cloud-agent/src/services/firestoreSession.ts`:
- Around line 214-221: The instance guard in markDesktopDeviceOffline is still
using a non-atomic get() + update() flow, so a stale disconnect can overwrite a
newer markDesktopDeviceOnline state. Update the logic in firestoreSession.ts’s
markDesktopDeviceOffline to use an atomic Firestore precondition or transaction
based on connectedInstanceId, so the offline write only applies when the stored
instance still matches expectedInstanceId. Keep the existing guard semantics,
but ensure the final write cannot clobber a freshly updated online device.
- Around line 156-172: The terminal-state guard in writeDesktopTaskResult is
non-atomic and can be raced by failDesktopTaskIfUnresolved, causing a stale
failure or success to overwrite the other. Update writeDesktopTaskResult and the
matching unresolved-failure path to use a Firestore transaction via
db.runTransaction so the get/check/update happens atomically. You’ll likely need
to extend FirestoreLike and the test fake to support transactional reads and
writes for desktopTaskPath, while keeping the existing terminal-status checks in
place.
In `@cloud-agent/src/tools/vaultTools.ts`:
- Around line 68-115: The local dispatch path in dispatchLocalIfConnected and
dispatchVaultCall can race with watchPendingDesktopTasks because
createDesktopTask publishes a pending task before markDesktopTaskExecuting runs,
so the same task may be dispatched twice. Change the same-instance flow in
vaultTools to use a single deduped handoff or mark the task as executing before
it becomes visible as pending, and keep the logic anchored around
dispatchLocalIfConnected, dispatchVaultCall, and createDesktopTask so the local
bridge and watcher cannot both send it.
---
Outside diff comments:
In `@cloud-agent/src/handlers/wsLiveAgentHandler.ts`:
- Around line 148-241: Reset the per-turn vault call budget in
handleGeminiMessage by handling serverContent.turnComplete, since vaultDeps
persists across the live WebSocket session and createVaultToolDeps currently
keeps callsThisTurn/capDecay across turns. Add turn-boundary handling alongside
the existing serverContent processing so that when turnComplete is received,
those counters are cleared before the next turn starts.
---
Nitpick comments:
In `@cloud-agent/src/index.ts`:
- Around line 71-86: The `runAgentReal` setup is creating two separate
`defaultFirestoreSession()` instances for `bridge` and `vault`, which is
redundant. Reuse the existing session from `bridge` when building `vault` so the
`createVaultToolDeps` call uses the same firestore session object, matching the
`wsLiveAgentHandler` pattern and avoiding an extra session construction per
request.
In `@cloud-agent/src/tools/vaultTools.ts`:
- Around line 82-133: Add explicit error handling in dispatchVaultCall around
the Firestore-dependent calls so transient failures return a friendly tool
message instead of rejecting. Wrap fs.getActiveDesktopDevice and
fs.createDesktopTask (and any immediate dependent dispatch setup) in try/catch,
and on failure return a stable Vault error string similar to NO_DEVICE_MSG and
TIMEOUT_MSG. Keep the existing control flow in dispatchVaultCall and preserve
billing pause/resume in the finally block.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a568b151-5eb5-4654-af60-d854b3ab5b68
📒 Files selected for processing (44)
README.md__tests__/bootstrapSession.test.ts__tests__/characterSyncIdempotentUpload.test.ts__tests__/characterSyncWiki.test.ts__tests__/ensureDevSandboxCharacter.test.ts__tests__/liveVoiceMachine.test.ts__tests__/useAIChat.test.tsx__tests__/wikiLlmProvider.test.tsapp/_layout.tsxcloud-agent/scripts/desktopBridgeSmoke.tscloud-agent/src/handlers/wsDesktopAgentHandler.test.tscloud-agent/src/handlers/wsDesktopAgentHandler.tscloud-agent/src/handlers/wsLiveAgentHandler.tscloud-agent/src/index.tscloud-agent/src/services/agentCore.tscloud-agent/src/services/desktopBridge.test.tscloud-agent/src/services/desktopBridge.tscloud-agent/src/services/desktopPairing.test.tscloud-agent/src/services/desktopPairing.tscloud-agent/src/services/firestoreSession.test.tscloud-agent/src/services/firestoreSession.tscloud-agent/src/services/liveToolAdapter.tscloud-agent/src/tools/vaultTools.test.tscloud-agent/src/tools/vaultTools.tscloud-agent/src/tools/vaultToolsWiring.test.tsdocs/desktop-vault-bridge.mddocs/flowcharts/HOOKS.mddocs/flowcharts/MACHINES.mddocs/flowcharts/SERVICES.mddocs/superpowers/plans/2026-07-05-desktop-vault-bridge.mddocs/superpowers/specs/2026-07-05-desktop-vault-bridge-design.mdfirestore.rulesmetro.config.jspackage.jsonsrc/auth/bootstrapSession.tssrc/auth/devSandboxFlag.tssrc/auth/ensureDevSandboxCharacter.tssrc/hooks/useAIChat.tssrc/hooks/useTabCharacterId.tssrc/machines/characterMachine.tssrc/machines/liveVoiceMachine.tssrc/services/aiChatService.tssrc/services/characterSyncService.tssrc/services/wikiLlmProvider.ts
Comment on lines
+110
to
+124
| unsubPending = fs.watchPendingDesktopTasks(uid, deviceId, (tasks) => { | ||
| for (const task of tasks) { | ||
| if (dispatched.has(task.taskId)) continue | ||
| dispatched.add(task.taskId) | ||
| void fs.markDesktopTaskExecuting(uid!, task.taskId) | ||
| .then(() => { | ||
| if (socketOpen()) { | ||
| ws.send(JSON.stringify({ | ||
| type: 'task', taskId: task.taskId, tool: task.tool, params: task.params, | ||
| })) | ||
| } | ||
| }) | ||
| .catch((err) => console.error('[desktop-bridge] dispatch failed:', task.taskId, err)) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the handler file and inspect the relevant region with line numbers.
sed -n '1,260p' cloud-agent/src/handlers/wsDesktopAgentHandler.ts | cat -n
printf '\n---\n'
# Search for the watcher and dispatch helpers involved in retry behavior.
rg -n "watchPendingDesktopTasks|markDesktopTaskExecuting|dispatched|pendingDesktopTasks|DesktopTask" cloud-agent/src -SRepository: equationalapplications/clanker
Length of output: 20378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the surrounding implementation of the desktop task store/watchers if present.
fd -a ".*desktop.*|.*task.*" cloud-agent/src | sed -n '1,200p'Repository: equationalapplications/clanker
Length of output: 672
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== wsDesktopAgentHandler.ts (relevant section) ==="
nl -ba cloud-agent/src/handlers/wsDesktopAgentHandler.ts | sed -n '90,170p'
echo
echo "=== references to watchPendingDesktopTasks / markDesktopTaskExecuting / dispatched ==="
rg -n "watchPendingDesktopTasks|markDesktopTaskExecuting|dispatched" cloud-agent/src -S
echo
echo "=== candidate task-store files ==="
fd -a "desktop|task" cloud-agent/src | sed -n '1,200p'Repository: equationalapplications/clanker
Length of output: 260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== definitions/usages of watchPendingDesktopTasks and markDesktopTaskExecuting across repo ==="
rg -n "watchPendingDesktopTasks\(|markDesktopTaskExecuting\(" -S .
echo
echo "=== locate the module that provides fs in wsDesktopAgentHandler.ts ==="
sed -n '1,120p' cloud-agent/src/handlers/wsDesktopAgentHandler.ts | nl -ba | sed -n '1,120p'Repository: equationalapplications/clanker
Length of output: 2874
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the implementation of watchPendingDesktopTasks / markDesktopTaskExecuting.
rg -n "function watchPendingDesktopTasks|const watchPendingDesktopTasks|watchPendingDesktopTasks:|markDesktopTaskExecuting" cloud-agent -S
echo
# Show the file(s) containing those definitions, if any, with line numbers.
FILES="$(rg -l "watchPendingDesktopTasks|markDesktopTaskExecuting" cloud-agent -S || true)"
for f in $FILES; do
echo "--- $f ---"
nl -ba "$f" | sed -n '1,260p'
doneRepository: equationalapplications/clanker
Length of output: 1528
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== cloud-agent/src/services/firestoreSession.ts (relevant sections) ==="
sed -n '130,220p' cloud-agent/src/services/firestoreSession.ts | cat -n
echo
echo "=== cloud-agent/src/services/firestoreSession.test.ts (task-state tests) ==="
sed -n '330,430p' cloud-agent/src/services/firestoreSession.test.ts | cat -n
echo
echo "=== docs/superpowers/plans/2026-07-05-desktop-vault-bridge.md (dispatch sections) ==="
sed -n '400,470p' docs/superpowers/plans/2026-07-05-desktop-vault-bridge.md | cat -n
sed -n '970,1005p' docs/superpowers/plans/2026-07-05-desktop-vault-bridge.md | cat -nRepository: equationalapplications/clanker
Length of output: 16671
Release dispatched when markDesktopTaskExecuting fails. A rejected update leaves the task pending in Firestore, but the local dispatched guard prevents the same socket from trying it again on later watcher emissions. Delete the taskId in .catch so it can be retried.
🤖 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 `@cloud-agent/src/handlers/wsDesktopAgentHandler.ts` around lines 110 - 124,
The task dispatch flow in wsDesktopAgentHandler’s watchPendingDesktopTasks
callback keeps task IDs in the dispatched set even when
fs.markDesktopTaskExecuting fails, which blocks retries on later watcher
emissions. Update the markDesktopTaskExecuting promise chain so that the taskId
is removed from dispatched inside the .catch path before logging the failure,
allowing the same socket to retry that task on subsequent updates.
Comment on lines
+156
to
+172
| async writeDesktopTaskResult( | ||
| uid: string, | ||
| taskId: string, | ||
| outcome: { status: 'complete'; result: unknown } | { status: 'failed'; error: DesktopTaskError }, | ||
| ): Promise<boolean> { | ||
| const ref = db.doc(desktopTaskPath(uid, taskId)) | ||
| const snap = await ref.get() | ||
| if (!snap.exists) return false | ||
| const current = snap.data() as unknown as DesktopTaskDoc | ||
| if (current.status === 'complete' || current.status === 'failed') return false | ||
| await ref.update( | ||
| outcome.status === 'complete' | ||
| ? { status: 'complete', result: outcome.result, updatedAt: now() } | ||
| : { status: 'failed', error: outcome.error, updatedAt: now() }, | ||
| ) | ||
| return true | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Non-atomic terminal-state guard allows lost/overwritten task results.
Both writeDesktopTaskResult and failDesktopTaskIfUnresolved do a plain get() then conditionally update(). These are called from independent triggers that can legitimately race: CT's task_result/task_error frame (via writeDesktopTaskResult in the WS handler) vs. the caller's callTimeoutMs timer (via failDesktopTaskIfUnresolved in vaultTools.ts), often on different Cloud Run instances. If both read the doc while it's still executing, both pass the terminal check and both write — the last update() wins, so a real complete result can be silently overwritten by a stale DESKTOP_TIMEOUT/DESKTOP_DISCONNECTED failure (or vice versa). The in-flight caller may already have resolved via the first snapshot it observed, but the persisted Firestore doc — which the docs explicitly say should be monitored for DESKTOP_TIMEOUT frequency — ends up inconsistent.
🔧 Suggested direction
Wrap the read-check-write in a Firestore transaction (db.runTransaction), which requires extending the FirestoreLike abstraction (and the test fake) to expose transactional reads/writes for this doc path, so the terminal-state check is atomic with the write.
Also applies to: 180-188
🤖 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 `@cloud-agent/src/services/firestoreSession.ts` around lines 156 - 172, The
terminal-state guard in writeDesktopTaskResult is non-atomic and can be raced by
failDesktopTaskIfUnresolved, causing a stale failure or success to overwrite the
other. Update writeDesktopTaskResult and the matching unresolved-failure path to
use a Firestore transaction via db.runTransaction so the get/check/update
happens atomically. You’ll likely need to extend FirestoreLike and the test fake
to support transactional reads and writes for desktopTaskPath, while keeping the
existing terminal-status checks in place.
Comment on lines
+214
to
+221
| async markDesktopDeviceOffline(uid: string, deviceId: string, expectedInstanceId?: string): Promise<void> { | ||
| const ref = db.doc(`${devicesPath(uid)}/${deviceId}`) | ||
| const snap = await ref.get() | ||
| if (!snap.exists) return | ||
| const data = snap.data() as { connectedInstanceId?: string | null } | undefined | ||
| if (expectedInstanceId !== undefined && data?.connectedInstanceId !== expectedInstanceId) return | ||
| await ref.update({ online: false, connectedInstanceId: null }) | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
markDesktopDeviceOffline's instance-guard has the same TOCTOU gap.
The check-then-update here is exactly what the design spec's "Replacement race guard" (§5) is meant to prevent, but get()+update() isn't atomic: if a new connection's markDesktopDeviceOnline (writing the new connectedInstanceId) lands between this get() and update(), the stale disconnect's update() still fires unconditionally and clobbers the freshly-online device back to online: false, connectedInstanceId: null. Since nothing re-marks the device online until its next actual reconnect, getActiveDesktopDevice's online === true filter would incorrectly treat a live, functioning desktop as unavailable.
🤖 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 `@cloud-agent/src/services/firestoreSession.ts` around lines 214 - 221, The
instance guard in markDesktopDeviceOffline is still using a non-atomic get() +
update() flow, so a stale disconnect can overwrite a newer
markDesktopDeviceOnline state. Update the logic in firestoreSession.ts’s
markDesktopDeviceOffline to use an atomic Firestore precondition or transaction
based on connectedInstanceId, so the offline write only applies when the stored
instance still matches expectedInstanceId. Keep the existing guard semantics,
but ensure the final write cannot clobber a freshly updated online device.
Comment on lines
+68
to
+115
| async function dispatchLocalIfConnected( | ||
| deps: VaultToolDeps, | ||
| deviceId: string, | ||
| taskId: string, | ||
| wireTool: string, | ||
| params: Record<string, unknown>, | ||
| ): Promise<void> { | ||
| const conn = deps.desktopBridge?.get(deps.firebaseUid, deviceId) | ||
| const ws = conn?.ws | ||
| if (!ws || ws.readyState !== WebSocket.OPEN) return | ||
| await deps.firestoreSession.markDesktopTaskExecuting(deps.firebaseUid, taskId) | ||
| ws.send(JSON.stringify({ type: 'task', taskId, tool: wireTool, params })) | ||
| } | ||
|
|
||
| async function dispatchVaultCall( | ||
| deps: VaultToolDeps, | ||
| adkName: VaultAdkName, | ||
| params: Record<string, unknown>, | ||
| ): Promise<string> { | ||
| const fs = deps.firestoreSession | ||
| const device = await fs.getActiveDesktopDevice(deps.firebaseUid) | ||
| if (!device) return NO_DEVICE_MSG | ||
|
|
||
| const cap = effectiveCap(deps) | ||
| if (deps.callsThisTurn.count >= cap) return CAP_MSG | ||
| deps.callsThisTurn.count++ | ||
|
|
||
| const wireTool = VAULT_WIRE_TOOL[adkName] | ||
| try { | ||
| deps.pauseBilling?.() | ||
| const taskId = crypto.randomUUID() | ||
| await fs.createDesktopTask(deps.firebaseUid, taskId, device.deviceId, wireTool, params) | ||
| await dispatchLocalIfConnected(deps, device.deviceId, taskId, wireTool, params) | ||
|
|
||
| const task = await new Promise<DesktopTaskDoc | null>((resolve) => { | ||
| const timeout = setTimeout(() => { | ||
| unsub() | ||
| void fs.failDesktopTaskIfUnresolved(deps.firebaseUid, taskId, { | ||
| code: 'DESKTOP_TIMEOUT', message: `No result within ${deps.callTimeoutMs}ms`, | ||
| }).catch(() => { /* TTL backstop */ }) | ||
| resolve(null) | ||
| }, deps.callTimeoutMs) | ||
| const unsub = fs.watchDesktopTask(deps.firebaseUid, taskId, (t) => { | ||
| if (t.status === 'complete' || t.status === 'failed') { | ||
| clearTimeout(timeout); unsub(); resolve(t) | ||
| } | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the full desktop WS handler to confirm dedup coverage across
# the same-instance shortcut and the pending-task listener.
fd wsDesktopAgentHandler.ts -x cat -n {}Repository: equationalapplications/clanker
Length of output: 8804
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Firestore task lifecycle methods and watch behavior.
fd firestoreSession.ts -x sh -c '
echo "===== {} ====="
cat -n "{}" | sed -n "1,260p"
'
# Also inspect the vault tool file around the dispatch path for any local guards.
fd vaultTools.ts -x sh -c '
echo "===== {} ====="
cat -n "{}" | sed -n "1,220p"
'Repository: equationalapplications/clanker
Length of output: 22705
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any coordination between vault direct dispatch and the desktop handler's dispatched set.
rg -n "dispatched\.|watchPendingDesktopTasks|dispatchLocalIfConnected|markDesktopTaskExecuting|desktopBridge\.get" cloud-agent/srcRepository: equationalapplications/clanker
Length of output: 3301
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "dispatched\.|watchPendingDesktopTasks|dispatchLocalIfConnected|markDesktopTaskExecuting|desktopBridge\.get" cloud-agent/srcRepository: equationalapplications/clanker
Length of output: 3301
Avoid the local dispatch race in cloud-agent/src/tools/vaultTools.ts:68-100.
desktopBridge.get() puts this code on the same instance that runs watchPendingDesktopTasks, so createDesktopTask(...status: 'pending') can be picked up in the gap before markDesktopTaskExecuting(), causing the same task to be sent twice. Route same-instance dispatches through one dedup path or otherwise mark them before they become visible as pending.
🤖 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 `@cloud-agent/src/tools/vaultTools.ts` around lines 68 - 115, The local
dispatch path in dispatchLocalIfConnected and dispatchVaultCall can race with
watchPendingDesktopTasks because createDesktopTask publishes a pending task
before markDesktopTaskExecuting runs, so the same task may be dispatched twice.
Change the same-instance flow in vaultTools to use a single deduped handoff or
mark the task as executing before it becomes visible as pending, and keep the
logic anchored around dispatchLocalIfConnected, dispatchVaultCall, and
createDesktopTask so the local bridge and watcher cannot both send it.
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
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.
Description
Adds the Clanker side of the desktop vault bridge: persistent
/agent/desktopWebSocket with pairing-token auth,
desktopPairings+desktopTasksFirestorerouting, and five
vault_*ADK tools (vault_wiki_search,vault_get_ontology,vault_traverse_graph,vault_semantic_search,vault_related_chunks) for/agent/runand/agent/live. Wire protocol matches Curated Thoughts v1.9.0.Specs:
Type of Change
Related Issue
Fixes #
Changes Made
POST /agent/desktop/pairand/agent/desktop/revoke(hash-only token storage)wsDesktopAgentHandler: auth, pending-task listener, device-doc revoke/pauselistener, 40s
lastSeenAtrefresh, generation-guardeddesktopBridgefirestoreSession:getActiveDesktopDevice, desktop task queue, browsergetActiveDeviceexcludestype: 'desktop'vaultTools.ts: five ADK tools, shared dispatcher, 5-call cap withtimeout/disconnect decay, same-instance shortcut, no credit spend
buildAgent(text) andbuildLiveTools(voice)firestore.rules: deny client access tousers/{uid}/desktopTaskscloud-agent/scripts/desktopBridgeSmoke.tsTesting
Platforms Tested
Test Steps
cd cloud-agent && npm run typechecknpm run build && NODE_ENV=test node --test --test-reporter spec dist/cloud-agent/src/handlers/wsDesktopAgentHandler.test.js dist/cloud-agent/src/tools/vaultTools.test.js dist/cloud-agent/src/services/firestoreSession.test.js dist/cloud-agent/src/services/desktopBridge.test.js dist/cloud-agent/src/services/desktopPairing.test.js dist/cloud-agent/src/tools/vaultToolsWiring.test.js— 53/53 passdocker compose -f docker-compose.local.yml up -d, pair via/agent/desktop/pair, runPAIRING_TOKEN=… npx tsx cloud-agent/scripts/desktopBridgeSmoke.ts, then/agent/runwith a vault search promptvault_wiki_searchend to endScreenshots
Before
N/A — new subsystem.
After
N/A — server-side only; mobile Settings UI for pair/revoke is a follow-up.
Checklist
Additional Notes
gcloud firestore fields ttls update expiresAt --collection-group=desktopTasks --enable-ttlCloudBridgeClientis already shipped (v1.9.0); this PR is the matching Clanker contract.Made with Cursor