Skip to content

Fix integration scope save latency#90

Merged
khaliqgant merged 2 commits into
mainfrom
fix/integration-scope-save-latency
Jun 5, 2026
Merged

Fix integration scope save latency#90
khaliqgant merged 2 commits into
mainfrom
fix/integration-scope-save-latency

Conversation

@kjgbot
Copy link
Copy Markdown
Contributor

@kjgbot kjgbot commented Jun 5, 2026

User description

Summary

  • make integration scope saves return after local persistence instead of waiting on mount/subscription reconciliation
  • continue syncing mount paths, event subscriptions, and integration system messages in the background
  • treat project-scoped event stream refresh before broker startup as a no-op

Verification

  • npm exec tsc -- --noEmit
  • npm exec vitest -- run src/main/broker.test.ts
  • npm run build

CodeAnt-AI Description

Fix integration scope saves and ignore project refreshes before startup

What Changed

  • Saving an integration scope now finishes right after the change is stored, instead of waiting for background sync work to complete.
  • Background syncing still updates mount paths and subscriptions, but failures are logged without blocking the save.
  • Slack channel option lookup now keeps working through the legacy channel list when the newer options path fails.
  • Refreshing a project event stream before the broker session has started now does nothing instead of raising an error.

Impact

✅ Faster integration scope saves
✅ Fewer blocked settings updates
✅ Clearer Slack channel selection

🔄 Retrigger CodeAnt AI Review

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 5, 2026

CodeAnt AI is reviewing your PR.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 5, 2026

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: cb338180-7dbc-469d-85d4-14321d410078

📥 Commits

Reviewing files that changed from the base of the PR and between 3730c6a and ce79a7b.

📒 Files selected for processing (1)
  • src/main/integrations.test.ts

📝 Walkthrough

Walkthrough

Broker refresh now returns early for project-scoped calls when the Relay workspace isn't started; integration option fetching uses account workspace endpoints via fetchJson (with Slack legacy fallback); integration state sync is made non-blocking and failures are logged. Tests add hoisted mocks and cover these behaviors.

Changes

Error resilience and integration refactoring

Layer / File(s) Summary
Broker refresh early return on missing project session
src/main/broker.ts, src/main/broker.test.ts
refreshEventStream wraps getOrAwaitSessionsForProject in try/catch and returns early for the "Relay workspace not started — select the project first" error; project-scoped calls resolve to undefined when no session exists (verified by test).
Integration options fetching via account workspace
src/main/integrations.ts
listOptions resolves account workspaceId and calls the options endpoint via fetchJson; Slack channels errors route to a legacy helper refactored to accept workspaceId and paginate via fetchJson.
Integration state sync non-blocking helper
src/main/integrations.ts
Added syncAgentStateEventually(projectId, notifyAgent) which calls syncAgentState asynchronously and logs failures; updateScope emits integration-added then invokes the non-blocking helper rather than awaiting sync inline.
Integrations tests and hoisted mocks
src/main/integrations.test.ts
Adds a Vitest hoisted mock environment, stubs for auth/store/mounts/agent/broker, and tests: Gmail options via account workspace, Slack legacy channel pagination, and updateScope returning before mount reconcile completes.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 I nudged the sessions, gentle and spry,
Fetching options beneath the sky,
Sync hops off to run in the weeds,
Tests cheer softly for resilient deeds,
A tiny carrot-shaped log rolls by.


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai Bot added the size:M This PR changes 30-99 lines, ignoring generated files label Jun 5, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a no-op behavior for project-scoped event stream refreshes before a broker session starts, refactors integration option retrieval to use direct JSON fetching instead of workspace handles, and makes agent state synchronization asynchronous. Feedback suggests using the existing isWorkspaceNotStartedError helper in broker.ts instead of hardcoding error message strings, and emitting an error event when background agent state synchronization fails in integrations.ts so the UI can notify the user.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/main/broker.ts
Comment on lines +1730 to +1735
} catch (err) {
if (err instanceof Error && err.message === 'Relay workspace not started — select the project first') {
return
}
throw err
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of hardcoding the exact error message string comparison, use the existing isWorkspaceNotStartedError(err) helper function. This is more robust, handles non-Error thrown values safely, and maintains consistency with how this error is checked elsewhere in the codebase.

      } catch (err) {
        if (isWorkspaceNotStartedError(err)) {
          return
        }
        throw err
      }

Comment thread src/main/integrations.ts
Comment on lines +1502 to +1506
private syncAgentStateEventually(projectId: string, notifyAgent: boolean): void {
void this.syncAgentState(projectId, notifyAgent).catch((error) => {
console.warn('[integrations] Failed to sync integration state:', toErrorMessage(error))
})
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since syncAgentState is now executed asynchronously in the background via syncAgentStateEventually, any failures during the sync process (such as local mount failures or event subscription reconciliation errors) will only be logged to the console and won't be propagated to the user. Consider emitting an integration-error event on failure so the UI can display an appropriate warning banner to the user if the background sync fails.

Comment thread src/main/integrations.ts
Comment on lines +1502 to +1506
private syncAgentStateEventually(projectId: string, notifyAgent: boolean): void {
void this.syncAgentState(projectId, notifyAgent).catch((error) => {
console.warn('[integrations] Failed to sync integration state:', toErrorMessage(error))
})
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: This fire-and-forget sync introduces a race: multiple rapid updateScope calls can run syncAgentState concurrently for the same project, but downstream reconciliation is not serialized. That can interleave close/re-subscribe cycles and leave duplicate/stale event subscriptions active. Serialize background sync per project (for example with a per-project promise queue or lock) before launching another sync. [race condition]

Severity Level: Critical 🚨
- ❌ Duplicate integration events delivered to project agents.
- ⚠️ Stale subscriptions watch outdated mount paths after scope changes.
- ⚠️ Orphaned watchers leak resources in IntegrationEventBridge.reconcile.
Steps of Reproduction ✅
1. Start the app and broker for a project so integration events are active; `broker:start`
is handled in `src/main/ipc-handlers.ts:5-20` and, on success, calls
`integrationsManager.notifyAgentState(projectId)` at `src/main/ipc-handlers.ts:15-16`,
which in turn awaits `this.syncAgentState(projectId, true)` in
`src/main/integrations.ts:821-823`.

2. In the renderer, open Project Settings so that integration visibility UI is available;
actions like toggling visibility or Slack channel selections ultimately call
`pear.integrations.updateScope(...)` via the preload bridge at
`src/preload/index.ts:12-24`, which invokes the IPC handler `'integrations:update-scope'`
at `src/main/ipc-handlers.ts:15-19`.

3. From the renderer (e.g., devtools console or rapid UI interactions from multiple
windows), issue two `pear.integrations.updateScope(projectId, integrationId, scope,
mountPaths)` calls in quick succession without waiting between them; both IPC requests
reach `IntegrationsManager.updateScope` at `src/main/integrations.ts:955-963`, which
persists the integration, emits an event, and then calls the fire‑and‑forget
`this.syncAgentStateEventually(projectId, integration.notifyAgent)` at
`src/main/integrations.ts:45-46`.

4. Each `syncAgentStateEventually` invocation starts an overlapping
`syncAgentState(projectId, notifyAgent)` task (defined at
`src/main/integrations.ts:1508-1517`), and both tasks call
`this.syncEventSubscriptions(projectId)` at `src/main/integrations.ts:1513`, which
delegates to `integrationEventBridge.reconcile(projectId, integrations)` at
`src/main/integration-event-bridge.ts:203-235`. Because `reconcile` performs `await
this.close(projectId)` near the top and later overwrites
`this.subscriptions.set(projectId, { subscriptions, signature })` without any per‑project
locking, the two concurrent calls can interleave such that one call's `close()` sees no
current subscription (map already cleared) and the other call later overwrites the map
entry, leaving the first call's newly created `subscriptions` array with active watchers
that are never reachable for future `close()` calls. This yields duplicate or stale event
subscriptions for the same project (multiple active Relayfile subscriptions / FS watchers)
and can be observed as duplicated integration events being delivered to agents from
`IntegrationEventBridge.injectEvent` in `src/main/integration-event-bridge.ts:63-145`.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/main/integrations.ts
**Line:** 1502:1506
**Comment:**
	*Race Condition: This fire-and-forget sync introduces a race: multiple rapid `updateScope` calls can run `syncAgentState` concurrently for the same project, but downstream reconciliation is not serialized. That can interleave close/re-subscribe cycles and leave duplicate/stale event subscriptions active. Serialize background sync per project (for example with a per-project promise queue or lock) before launching another sync.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 5, 2026

CodeAnt AI finished reviewing your PR.

@agent-relay-code
Copy link
Copy Markdown
Contributor

Fixed the validated PR review items:

  • refreshEventStream now uses isWorkspaceNotStartedError(err) instead of matching one exact error string.
  • Background integration sync failures now emit integration-error, and the settings view surfaces that through its existing error banner.
  • Added regression coverage for async updateScope behavior and background sync error emission.
  • Updated the existing integration test expectation to match current normalized Relayfile mount paths.

Local validation passed:

  • npm exec tsc -- --noEmit
  • npm exec vitest -- run src/main/broker.test.ts src/main/integrations.test.ts
  • npm exec vitest -- run
  • npm test
  • npm run build
  • npm run verify:mcp-resources-drift

@agent-relay-code
Copy link
Copy Markdown
Contributor

⚠️ pr-reviewer push failed (exit 128) — fixes were not applied to the PR. The notes below are advisory and were not pushed.

Fixed the validated PR review items:

  • refreshEventStream now uses isWorkspaceNotStartedError(err) instead of matching one exact error string.
  • Background integration sync failures now emit integration-error, and the settings view surfaces that through its existing error banner.
  • Added regression coverage for async updateScope behavior and background sync error emission.
  • Updated the existing integration test expectation to match current normalized Relayfile mount paths.

Local validation passed:

  • npm exec tsc -- --noEmit
  • npm exec vitest -- run src/main/broker.test.ts src/main/integrations.test.ts
  • npm exec vitest -- run
  • npm test
  • npm run build
  • npm run verify:mcp-resources-drift

@agent-relay-code
Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed ce79a7b to this PR. The notes below describe what changed.

Added focused regression coverage in src/main/integrations.test.ts for the PR’s integration changes:

  • account-workspace API option lookup, including gmail -> google-mail
  • legacy Slack channel fallback with pagination
  • updateScope returning without waiting on slow integration state sync

Local verification passed:

  • npm exec vitest run src/main/integrations.test.ts src/main/broker.test.ts src/main/integration-mounts.test.ts src/main/cloud-agent.test.ts src/main/auth.test.ts
  • npm run build
  • npm test

@agent-relay-code
Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

Reviewed PR #90 against the checked-out diff and traced the touched broker/integration paths. I did not find a current, reproducible breakage that needed a code change.

Verification run:

  • npx vitest run src/main/integrations.test.ts src/main/broker.test.ts
  • npm install
  • npm run build
  • npm test
  • npx vitest run

All passed after installing dependencies from the lockfile.

@khaliqgant khaliqgant merged commit d77a23c into main Jun 5, 2026
3 checks passed
@khaliqgant khaliqgant deleted the fix/integration-scope-save-latency branch June 5, 2026 11:07
@agent-relay-code
Copy link
Copy Markdown
Contributor

Reviewed PR #90 against the checked-out diff and traced the affected broker/integration paths. I did not find a current, reproducible defect requiring a code change.

Verified locally:

  • npx vitest run src/main/broker.test.ts src/main/integrations.test.ts
  • npx vitest run
  • npm test
  • npm run build

@agent-relay-code
Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

Reviewed PR #90 against the checked-out diff and traced the affected broker/integration paths. I did not find a current, reproducible defect requiring a code change.

Verified locally:

  • npx vitest run src/main/broker.test.ts src/main/integrations.test.ts
  • npx vitest run
  • npm test
  • npm run build

@agent-relay-code
Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 933c861 to this PR. The notes below describe what changed.

Implemented the validated fixes from the PR review artifacts:

  • refreshEventStream now uses the existing isWorkspaceNotStartedError helper instead of matching one exact error string.
  • Background updateScope sync failures now emit the existing integration-error event.
  • Project settings surfaces that background error through its existing error banner.
  • Added regression coverage for background updateScope error emission and fixed the resolver typing in the async test.

Local validation passed:

  • npx vitest run src/main/integrations.test.ts src/main/broker.test.ts
  • npx vitest run
  • npm test
  • npm run verify:mcp-resources-drift
  • npm run build
  • npm exec tsc -- --noEmit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants