feat(bridge): Phase 3 proactive scheduler and async Expo Push completion#494
Merged
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
…ions Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Fail fast when the extension is loaded without esbuild env injection instead of falling back to REPLACE_* placeholders. Co-authored-by: Cursor <cursoragent@cursor.com>
Align security rules with the Phase 2 approval flow where mobile writes approvalToken alongside status and approvedAt. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
cloud-agent/src/handlers/wsLiveAgentHandler.test.ts (1)
1017-1019: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the propagated
sessionIdandtaskIdin these fallback tests.These tests exercise the new bridge signature, but they currently pass even if
sessionIdis dropped or swapped because they only check token/body. Pinning those fields here would protect the main contract change in this PR.Also applies to: 1098-1099
🤖 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.test.ts` around lines 1017 - 1019, The fallback tests around sendTaskComplete currently only assert token and message text, so they can miss regressions where sessionId or taskId are dropped or swapped. Update the relevant assertions in wsLiveAgentHandler.test.ts for the sendTaskComplete bridge cases to explicitly check the propagated sessionId and taskId on expoPushCalls, using the same call objects already referenced by these tests, so the contract change is protected even if the payload body still matches.cloud-agent/src/handlers/schedulerTriggerHandler.test.ts (1)
121-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the failed spend attempt explicitly.
The override throws before incrementing
creditCalls.spend, so Line 131 only proves no successful debit was recorded; it does not prove the handler attemptedspendCredit. Track a separate attempt counter.Test refactor
test('returns 402 when user has insufficient credits', async () => { + let spendAttempts = 0 const { app, creditCalls } = buildApp({ - spendCredit: async () => { throw new Error('INSUFFICIENT_CREDITS') }, + spendCredit: async () => { + spendAttempts++ + throw new Error('INSUFFICIENT_CREDITS') + }, }) @@ assert.equal(res.status, 402) assert.match(res.body.error, /insufficient credits/i) + assert.equal(spendAttempts, 1) assert.equal(creditCalls.spend, 0) })🤖 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/schedulerTriggerHandler.test.ts` around lines 121 - 131, The test in schedulerTriggerHandler.test.ts only checks that no successful debit was recorded, but it does not explicitly prove the handler attempted spendCredit when credits are insufficient. Update the buildApp test harness and the insufficient-credits case to track a separate spend attempt counter inside the spendCredit override, then assert that the attempt happened even though the call threw and creditCalls.spend stayed unchanged.
🤖 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/schedulerTriggerHandler.ts`:
- Around line 18-20: The scheduler trigger payload currently treats action as an
arbitrary record, which lets invalid actions pass into downstream checks and
persistence. Update schedulerTriggerHandler to validate action with the shared
DSL schemas, using singleActionSchema and taskIntentSchema instead of a broad
z.record and unsafe SingleAction | SequenceAction casting. Keep the parsing
localized in the schedulerBodySchema flow so the handler only accepts actions
that match the shared contract.
In `@cloud-agent/src/handlers/wsLiveAgentHandler.test.ts`:
- Around line 963-1030: The websocket integration test leaves the server running
if the async flow fails before the final cleanup, which can leak handles and
cause later tests to flake. Wrap the test body that uses createLiveTestServer
and listen in a try/finally so close() is always awaited even when the Promise
rejects. Apply the same cleanup pattern to the other affected websocket test
block as well, and keep the assertions inside the try so failures still
propagate correctly.
In `@cloud-agent/src/handlers/wsLiveAgentHandler.ts`:
- Around line 410-413: The Expo fallback promise chain in getToken.then
currently discards the promise returned by fwd.sendTaskComplete, so delivery
failures bypass the existing catch and logging. Update the getToken(fbUid) flow
in wsLiveAgentHandler so the sendTaskComplete call is returned or awaited within
the then callback, allowing any rejection to propagate into the existing
.catch((err) => console.error('[pushToLive Expo fallback]', err)) handler.
In `@cloud-agent/src/index.test.ts`:
- Around line 443-457: The scheduler-trigger test is restoring SCHEDULER_SECRET
incorrectly, which can leave a stringified undefined value behind and leak env
state on failures. In cloud-agent/src/index.test.ts, update the restore logic
around the createApp/request flow so the original value is put back with delete
when SCHEDULER_SECRET was unset, and ensure the second restoration is also
protected by its own finally block. Use the existing savedSecret variable and
the scheduler-trigger test block to locate the change.
In `@docs/superpowers/plans/2026-06-29-phase3-proactive-scheduler.md`:
- Line 7: Sync the plan to the shipped scheduler contract: update the
`phase3-proactive-scheduler` document to describe the shared `SCHEDULER_SECRET`
flow used by `cloud-agent/src/index.ts` and
`cloud-agent/src/handlers/schedulerTriggerHandler.ts`, including the current
`createSchedulerTriggerHandler` signature with `creditService` and
`resolveUserId`, and the fact that completion returns `200`. Remove the outdated
OIDC bearer-secret wording, the old 4-argument handler description, and the
`202` success path so the plan matches the implemented `schedulerTriggerHandler`
and `index` wiring.
- Around line 938-940: The fenced infra snippets in this document are missing
language labels, which triggers markdownlint MD040. Update the unlabeled fenced
blocks around the SCHEDULER_SECRET example and the other infra snippet section
so each fence includes an appropriate language tag such as bash or text. Use the
surrounding snippet content to choose the best label and keep the existing text
otherwise unchanged.
In `@firestore.rules`:
- Around line 17-19: The approval flow currently allows approved updates without
validating approvalToken, so tighten both the Firestore rule and the observer
logic. In firestore.rules, keep approvalToken restricted to a non-empty string
for allowed updates, and in cloud-agent/src/handlers/authApprovalObserver.ts
update the approved-handling path so it only resumes the extension after
verifying the token matches a valid expected value before calling the resume
logic.
---
Nitpick comments:
In `@cloud-agent/src/handlers/schedulerTriggerHandler.test.ts`:
- Around line 121-131: The test in schedulerTriggerHandler.test.ts only checks
that no successful debit was recorded, but it does not explicitly prove the
handler attempted spendCredit when credits are insufficient. Update the buildApp
test harness and the insufficient-credits case to track a separate spend attempt
counter inside the spendCredit override, then assert that the attempt happened
even though the call threw and creditCalls.spend stayed unchanged.
In `@cloud-agent/src/handlers/wsLiveAgentHandler.test.ts`:
- Around line 1017-1019: The fallback tests around sendTaskComplete currently
only assert token and message text, so they can miss regressions where sessionId
or taskId are dropped or swapped. Update the relevant assertions in
wsLiveAgentHandler.test.ts for the sendTaskComplete bridge cases to explicitly
check the propagated sessionId and taskId on expoPushCalls, using the same call
objects already referenced by these tests, so the contract change is protected
even if the payload body still matches.
🪄 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: 5c931bd5-a518-4d49-82eb-4d0ef605838e
📒 Files selected for processing (14)
cloud-agent/src/handlers/schedulerTriggerHandler.test.tscloud-agent/src/handlers/schedulerTriggerHandler.tscloud-agent/src/handlers/wsLiveAgentHandler.test.tscloud-agent/src/handlers/wsLiveAgentHandler.tscloud-agent/src/index.test.tscloud-agent/src/index.tscloud-agent/src/services/fcmDispatcher.test.tscloud-agent/src/services/fcmDispatcher.tscloud-agent/src/services/liveToolAdapter.tscloud-agent/src/tools/browserAction.test.tscloud-agent/src/tools/browserAction.tsdocs/superpowers/plans/2026-06-29-phase3-proactive-scheduler.mdextension/src/env.tsfirestore.rules
Validate scheduler actions with shared DSL schemas, verify approval tokens before resuming, and harden test cleanup and error propagation. Co-authored-by: Cursor <cursoragent@cursor.com>
Owner
Author
/fix-pr follow-upCommit: Review resolution
Verification
|
This was referenced Jun 30, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Phase 3 of the MV3 browser extension bridge: Cloud Scheduler–triggered proactive browser tasks, Expo Push delivery when results arrive after the voice session closes, and supporting hardening (billing, auth, rules).
Spec: MV3 Browser Extension Bridge Design (Phase 3)
Type of Change
Related Issue
Fixes #
Changes Made
sendProactivetofcmDispatcherforPROACTIVE_TASKExpo Push payloadspushToLivewithsessionIdand fall back tosendTaskCompletewhen the voice WS closes before a browser task result arrivesPOST /agent/browser/scheduler-triggerwith Bearer secret auth, rate limiting, billing, and 60s synchronous waitREQUIRES_AUTH422) — read-only monitoring only for Phase 3 gateapprovalTokenin auth doc client updates (Phase 2 alignment)REPLACE_*placeholders)docs/superpowers/plans/Testing
Platforms Tested
Test Steps
cd cloud-agent && npm test— 191 pass, 0 failcd cloud-agent && npm run typecheck— cleanSCHEDULER_SECRET, create Cloud Scheduler job, force-run, verify extension wake +PROACTIVE_TASKExpo Push +/talkdeep linkScreenshots
Before
N/A — server-side feature
After
N/A — pending manual GCP E2E verification
Checklist
Additional Notes
SCHEDULER_SECRETon Cloud Run; not configured locally (returns 503).awaiting_authscheduled flows are intentionally out of scope; destructive scheduled actions return 422.Made with Cursor