[Jinwoo-H]

No description provided.

[coderabbitai]

Walkthrough

The change adds a desktop connector-cloud client with authenticated requests, project and conversation data exchange, delivery dispatch, polling, and persisted event handling. It adds shared request and delivery contracts, account-scoped token minting, retrying outboxes, and receipt lookup. Startup and profile lifecycle hooks install, connect, stop, and reconcile the connector. Tests cover payload limits, retries, delivery handling, account changes, sign-out, and shutdown.

Priority: Normal

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 51 functions across 44 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title names the main change: the desktop app holds the Orca Cloud connector poll and authenticates it with relay-style connector tokens. This matches the changeset, which adds the desktop connecto…
Description check		The description follows the repository template closely. It has the ELI5, What Changed, Why, Testing, and AI Disclosure sections, plus a Review section and a checklist. The Linked Issue section is fil…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: the desktop app holds the Orca Cloud connector poll and authenticates it with relay-style connector tokens. This matches the changeset, which adds the desktop connector-cloud poll client, token minting, and lifecycle wiring. The title is long (99 characters), but it is specific and not vague.

Full details: Description check

Explanation

The description follows the repository template closely. It has the ELI5, What Changed, Why, Testing, and AI Disclosure sections, plus a Review section and a checklist. The Linked Issue section is filled with 'N/A (maintainer...)' and cites a stacked PR rather than an issue, and 'Fixes #' is left blank. The Visual Proof section gives an N/A reason. The AI Disclosure section is blank, which fits the internal-team exemption. The Testing section lists automated coverage and says manual testing is not done. Some template items are unchecked or only partly filled: the manual-testing checkbox and the Notes and Checklist items (cross-platform, SSH, and lint/typecheck confirmation). These gaps are minor.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Walkthrough

The change adds a desktop connector-cloud client with authenticated requests, project and conversation data exchange, delivery dispatch, polling, and persisted event handling. It adds shared request and delivery contracts, account-scoped token minting, retrying outboxes, and receipt lookup. Startup and profile lifecycle hooks install, connect, stop, and reconcile the connector. Tests cover payload limits, retries, delivery handling, account changes, sign-out, and shutdown.

Priority: Normal

Merge Risk: Low · up to d0f89

The desktop connector is mostly sound. A few error paths could leave sign-out or delivery acknowledgments stuck, or add unnecessary disk writes under a backlog. These are low-severity fixes and should be addressed soon, though they need not block the change. A real staging check of the auth route and connector poll is still outstanding.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 51 functions across 44 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title names the main change: the desktop app holds the Orca Cloud connector poll and authenticates it with relay-style connector tokens. This matches the changeset, which adds the desktop connecto…
Description check		The description follows the repository template closely. It has the ELI5, What Changed, Why, Testing, and AI Disclosure sections, plus a Review section and a checklist. The Linked Issue section is fil…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: the desktop app holds the Orca Cloud connector poll and authenticates it with relay-style connector tokens. This matches the changeset, which adds the desktop connector-cloud poll client, token minting, and lifecycle wiring. The title is long (99 characters), but it is specific and not vague.

Full details: Description check

Explanation

The description follows the repository template closely. It has the ELI5, What Changed, Why, Testing, and AI Disclosure sections, plus a Review section and a checklist. The Linked Issue section is filled with 'N/A (maintainer...)' and cites a stacked PR rather than an issue, and 'Fixes #' is left blank. The Visual Proof section gives an N/A reason. The AI Disclosure section is blank, which fits the internal-team exemption. The Testing section lists automated coverage and says manual testing is not done. Some template items are unchecked or only partly filled: the manual-testing checkbox and the Notes and Checklist items (cross-platform, SSH, and lint/typecheck confirmation). These gaps are minor.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments: 1

src/main/connector-cloud/connector-turn-events.ts: 1

223-230: Trivial · Performance & Scalability · Quick win

Write the turn-end file only when the set of turn ends changes.

onChange runs on every outbox change. That includes every prompt, linked and turn-started push, and every accepted ack-free batch. Each call re-serializes and synchronously rewrites the full turn-end list with writeSecureJsonFile on the main process.

During an outage or a 503 backoff, the outbox can hold up to 200 turn ends. Each one can carry an 8 KB lastAssistantMessage. Every live agent event then does a ~1.6 MB synchronous write on the Electron main thread, even though no turn end changed.

Skip the write when the kept turn ends are the same objects as the last stored set.

♻️ Proposed change

-  private store(events: readonly ConnectorEvent[]): void {
-    const turnEnds = events.filter(isTurnEnded)
-    if (turnEnds.length === 0) {
+  private store(events: readonly ConnectorEvent[]): void {
+    const turnEnds = events.filter(isTurnEnded)
+    const last = this.storedTurnEnds
+    if (last && last.length === turnEnds.length && turnEnds.every((event, i) => event === last[i])) {
+      return
+    }
+    this.storedTurnEnds = turnEnds
+    if (turnEnds.length === 0) {
       rmSync(this.filePath, { force: true })
     } else {
       writeSecureJsonFile(this.filePath, { account: this.account, turnEnds })
     }
   }

Declare private storedTurnEnds: TurnEnded[] | null = null on the class. The first call still writes, because the field starts as null.

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 15ea262d-b588-4a6c-b211-89bfcc435267

Commits

Reviewing files that changed from the base of the PR and between 22fd6f5 and d0f89c3.

Files selected for processing: 44

• src/main/connector-cloud/connector-cloud-install.ts
• src/main/connector-cloud/connector-cloud-request.ts
• src/main/connector-cloud/connector-cloud.test.ts
• src/main/connector-cloud/connector-cloud.ts
• src/main/connector-cloud/connector-deliveries.ts
• src/main/connector-cloud/connector-hello.test.ts
• src/main/connector-cloud/connector-hello.ts
• src/main/connector-cloud/connector-machine.ts
• src/main/connector-cloud/connector-origin.ts
• src/main/connector-cloud/connector-outbox.test.ts
• src/main/connector-cloud/connector-outbox.ts
• src/main/connector-cloud/connector-places.test.ts
• src/main/connector-cloud/connector-places.ts
• src/main/connector-cloud/connector-poll-client.test.ts
• src/main/connector-cloud/connector-poll-client.ts
• src/main/connector-cloud/connector-snapshot.test.ts
• src/main/connector-cloud/connector-snapshot.ts
• src/main/connector-cloud/connector-token-mint.test.ts
• src/main/connector-cloud/connector-token-mint.ts
• src/main/connector-cloud/connector-transport.ts
• src/main/connector-cloud/connector-turn-events.test.ts
• src/main/connector-cloud/connector-turn-events.ts
• src/main/connector-cloud/fake-connector-cloud.test-fixture.ts
• src/main/connector-cloud/fake-connector-seam.test-fixture.ts
• src/main/external-conversations/external-conversation-delivery-receipts.test.ts
• src/main/external-conversations/external-conversation-delivery-receipts.ts
• src/main/external-conversations/external-conversation-events.ts
• src/main/external-conversations/external-conversation-prompts.test.ts
• src/main/external-conversations/external-conversation-seam.ts
• src/main/ipc/orca-profile-id-arg.ts
• src/main/ipc/orca-profiles.ts
• src/main/ipc/register-core-handlers/register-core-handlers.ts
• src/main/orca-profiles/profile-cloud-auth-config.test.ts
• src/main/orca-profiles/profile-cloud-auth-config.ts
• src/main/orca-profiles/profile-cloud-client.test.ts
• src/main/orca-profiles/profile-cloud-org-members-client.test.ts
• src/main/orca-profiles/profile-cloud-pkce.test.ts
• src/main/startup/main-process-connector-startup.ts
• src/main/startup/main-process-quit.ts
• src/main/startup/main-process-runtime-launch-activation.test.ts
• src/main/startup/main-process-runtime-launch.ts
• src/main/startup/main-process-state.ts
• src/main/startup/main-window-core-services.ts
• src/shared/connector-cloud-contract.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

[coderabbitai]

Minor · Stability & Availability · Quick win

Isolate outbox cleanup failures so they cannot abort the sign-out.

signOutStarted() runs from onBeforeSignOut, before signOutCurrentOrcaProfile. this.events.clear() and ConnectorTurnEvents.clearStored() do file I/O: rmSync and writeSecureJsonFile in store(). On Windows these can throw EBUSY or EPERM when antivirus or another process holds the file.

If either call throws, three things follow:

• The exception propagates out of the orcaProfiles:signOutCurrent handler.
• The sign-out never starts.
• signingOut stays true. onAfterSignOut runs only in the finally of the signOutCurrentOrcaProfile call, which never executes. The connector then stays stopped until restart.

The outbox file is tagged with its account, and readStored ignores a file that belongs to another account. A failed cleanup therefore does not expose turn ends to another account. Log the failure and continue.

Proposed fix

--- "a/src/main/connector-cloud/connector-cloud.ts"
+++ "b/src/main/connector-cloud/connector-cloud.ts"
@@ -43,16 +43,20 @@
   }
 
   /** Just before the desktop signs out: stop now, and drop turn ends no other account can report. */
   signOutStarted(): void {
     this.signingOut = true
     this.stopClient()
-    if (this.events) {
-      this.events.clear()
-      this.events = null
-    } else {
-      ConnectorTurnEvents.clearStored(this.options.userDataPath)
-    }
+    try {
+      if (this.events) {
+        this.events.clear()
+      } else {
+        ConnectorTurnEvents.clearStored(this.options.userDataPath)
+      }
+    } catch (error) {
+      console.warn('[connector] could not clear pending turn ends before sign-out', error)
+    }
+    this.events = null
   }
 
   /** Quit or relaunch: stop now; pending turn ends stay on disk for the next launch of this account. */
   fence(): void {

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	signOutStarted(): void {
	this.signingOut = true
	this.stopClient()
	if (this.events) {
	this.events.clear()
	this.events = null
	} else {
	ConnectorTurnEvents.clearStored(this.options.userDataPath)
	}
	}
	signOutStarted(): void {
	this.signingOut = true
	this.stopClient()
	try {
	if (this.events) {
	this.events.clear()
	} else {
	ConnectorTurnEvents.clearStored(this.options.userDataPath)
	}
	} catch (error) {
	console.warn('[connector] could not clear pending turn ends before sign-out', error)
	}
	this.events = null
	}

[coderabbitai]

Minor · Stability & Availability · Quick win

🔎 Supported by static analysis

Script executed:

set -o pipefail
printf '%s\n' '--- token mint ---'
nl -ba src/main/connector-cloud/connector-token-mint.ts
printf '%s\n' '--- bound symbols and transport callers ---'
rg -n -F --glob '*.ts' -e 'function connectorAccount' -e 'ensureActiveOrcaProfile' -e 'class ConnectorAccountTransport' -e 'ConnectorAccountTransport' src || test "$?" -eq 1
printf '%s\n' '--- transport definitions/callers ---'
rg -n -F --glob '*.ts' -e 'class ConnectorAccountTransport' -e 'ensure(' -e '.call(' src/main/connector-cloud src/main | head -200

Repository: stablyai/orca

Length of output: 38126

---

Script executed:

printf '%s\n' '--- profile index store ---'
nl -ba src/main/orca-profiles/profile-index-store.ts | sed -n '1,270p'
printf '%s\n' '--- connector transport ---'
nl -ba src/main/connector-cloud/connector-transport.ts
printf '%s\n' '--- connector cloud construction ---'
nl -ba src/main/connector-cloud/connector-cloud.ts | sed -n '55,105p'

Repository: stablyai/orca

Length of output: 16040

---

Move the profile reads inside the try block.

desktopConnectorTokenMint should return a ConnectorResult instead of rejecting. Both profile reads can perform synchronous filesystem operations and throw before the current try block. Move them inside the block so the existing catch converts those errors into ConnectorFailure.

🛡️ Suggested fix

--- "a/src/main/connector-cloud/connector-token-mint.ts"
+++ "b/src/main/connector-cloud/connector-token-mint.ts"
@@ -35,13 +35,13 @@
   account: string,
   fetchImpl: typeof globalThis.fetch = globalThis.fetch
 ): ConnectorTokenMint {
   return async () => {
-    if (connectorAccount(userDataPath) !== account) {
-      return SIGNED_OUT
-    }
-    const active = ensureActiveOrcaProfile(userDataPath)
     try {
+      if (connectorAccount(userDataPath) !== account) {
+        return SIGNED_OUT
+      }
+      const active = ensureActiveOrcaProfile(userDataPath)
       // The per-install id the host descriptor already keeps; the cloud keys this machine on it.
       const machineId = await loadOrCreateHostInstallationId(userDataPath)
       const minted = await runWithFreshOrcaCloudSession(
         authConfig,

[coderabbitai]

Minor · Stability & Availability · Quick win

Return a rejected promise from recorded instead of throwing synchronously.

recorded returns Promise<Delivery | null>, but it can throw synchronously in two places:

• this.settledDb(), if this.db() or settlePendingMutationReceipts throws.
• JSON.parse(row.receipt ?? 'null'), if the receipt is corrupt.

ConnectorPollClient.reconcile (src/main/connector-cloud/connector-poll-client.ts Line 251-259) adds the delivery ID to active first. Then it calls this.seam.recorded(...).then(..., onError). A synchronous throw skips onError, so the void this.reconcile(...) promise rejects unhandled. The delivery ID stays in active. Later hellos list the delivery as unsettled again, and reconcile returns early because of the active.has guard. The cloud never receives a done for that delivery until the process restarts.

Make the method async so every failure becomes a rejection. reconcile already maps a rejection to unknown.

🐛 Proposed fix

--- "a/src/main/external-conversations/external-conversation-delivery-receipts.ts"
+++ "b/src/main/external-conversations/external-conversation-delivery-receipts.ts"
@@ -95,20 +95,20 @@
   }
 
   /** Read-only: what a received delivery came to, or null when nothing was recorded (refusals and answers leave none). */
-  recorded(key: ConversationKey, deliveryId: string): Promise<Delivery | null> {
+  async recorded(key: ConversationKey, deliveryId: string): Promise<Delivery | null> {
     const requestId = JSON.stringify([key.externalId, deliveryId])
     const fingerprint = callerFingerprint(key.source)
     const running = this.inFlight.get(JSON.stringify([fingerprint, requestId]))
     if (running) {
       return running
     }
     const row = this.settledDb().getMutationReceipt(fingerprint, requestId)
     if (row?.state !== 'completed') {
-      return Promise.resolve(null)
+      return null
     }
     const recorded = RecordedDelivery.safeParse(JSON.parse(row.receipt ?? 'null'))
-    return Promise.resolve(recorded.success ? recorded.data : LOST)
+    return recorded.success ? recorded.data : LOST
   }
 
   private settledDb(): DeliveryReceiptDb {
     const db = this.db()

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	recorded(key: ConversationKey, deliveryId: string): Promise<Delivery | null> {
	const requestId = JSON.stringify([key.externalId, deliveryId])
	const fingerprint = callerFingerprint(key.source)
	const running = this.inFlight.get(JSON.stringify([fingerprint, requestId]))
	if (running) {
	return running
	}
	const row = this.settledDb().getMutationReceipt(fingerprint, requestId)
	if (row?.state !== 'completed') {
	return Promise.resolve(null)
	}
	const recorded = RecordedDelivery.safeParse(JSON.parse(row.receipt ?? 'null'))
	return Promise.resolve(recorded.success ? recorded.data : LOST)
	}
	async recorded(key: ConversationKey, deliveryId: string): Promise<Delivery | null> {
	const requestId = JSON.stringify([key.externalId, deliveryId])
	const fingerprint = callerFingerprint(key.source)
	const running = this.inFlight.get(JSON.stringify([fingerprint, requestId]))
	if (running) {
	return running
	}
	const row = this.settledDb().getMutationReceipt(fingerprint, requestId)
	if (row?.state !== 'completed') {
	return null
	}
	const recorded = RecordedDelivery.safeParse(JSON.parse(row.receipt ?? 'null'))
	return recorded.success ? recorded.data : LOST
	}

[coderabbitai]

Minor · Functional Correctness · Quick win

🔎 Supported by static analysis

Script executed:

set -eu
printf '%s\n' '--- contract diff and current definitions ---'
git diff --no-ext-diff 22fd6f505f7cde2a05cf58b5bf342276d4eaad32 d0f89c33f0d28bdb02a61e4b1766068f1623e702 -- src/shared/connector-cloud-contract.ts
nl -ba src/shared/connector-cloud-contract.ts | sed -n '1,180p'
printf '%s\n' '--- references ---'
rg -n -F --glob '*.{ts,tsx}' -e 'requestConnector' -e 'unsettled' -e 'ConnectorPollResponse' -e 'ConnectorConversation' src test tests 2>/dev/null || true
printf '%s\n' '--- poll client candidates ---'
rg -n -F --glob '*.{ts,tsx}' -e 'class ConnectorPollClient' -e 'cycle(' -e 'safeParse' src/main src/shared 2>/dev/null || true

Repository: stablyai/orca

Length of output: 40843

---

Script executed:

set -eu
printf '%s\n' '--- connector-cloud-request ---'
nl -ba src/main/connector-cloud/connector-cloud-request.ts | sed -n '1,150p'
printf '%s\n' '--- connector-poll-client cycle and reconciliation ---'
nl -ba src/main/connector-cloud/connector-poll-client.ts | sed -n '130,310p'
printf '%s\n' '--- delivery parsing ---'
nl -ba src/main/connector-cloud/connector-deliveries.ts | sed -n '1,130p'
printf '%s\n' '--- poll client tests around unsettled and failures ---'
nl -ba src/main/connector-cloud/connector-poll-client.test.ts | sed -n '170,235p'
nl -ba src/main/connector-cloud/connector-poll-client.test.ts | sed -n '450,625p'

Repository: stablyai/orca

Length of output: 25004

---

Script executed:

set -eu
printf '%s\n' '--- connector files and source discriminants ---'
rg -n -F --glob '*.{ts,tsx,js,json,md}' -e 'connector-cloud' -e 'CONNECTOR_PATHS' -e 'source: '\''slack'\''' -e 'source: "slack"' -e 'source: '\''discord'\''' -e 'source: "discord"' -e 'ConnectorConversation' -e '/v1/connector/poll' . 2>/dev/null || true
printf '%s\n' '--- connector directory ---'
find src/main/connector-cloud src/shared -maxdepth 3 -type f \( -iname '*connector*' -o -iname '*conversation*' \) -print | sort
printf '%s\n' '--- repository top-level and cloud-like paths ---'
find . -maxdepth 3 -type d \( -iname '*cloud*' -o -iname '*connector*' \) -print | sort

Repository: stablyai/orca

Length of output: 22453

---

Script executed:

set -eu
printf '%s\n' '--- cloud connector contract ---'
nl -ba cloud/packages/connector-contract/src/connector-wire-scalars.ts | sed -n '1,80p'
nl -ba cloud/packages/connector-contract/src/delivery-messages.ts | sed -n '1,100p'
printf '%s\n' '--- cloud inbox implementation ---'
nl -ba cloud/apps/connector/src/connector-inbox.ts | sed -n '1,280p'
printf '%s\n' '--- cloud poll route ---'
nl -ba cloud/apps/connector/src/machine-routes.ts | sed -n '45,115p'
printf '%s\n' '--- relevant cloud tests ---'
nl -ba cloud/apps/connector/src/connector-inbox.test.ts | sed -n '145,245p'
nl -ba cloud/apps/connector/src/connector-events.test.ts | sed -n '170,205p'

Repository: stablyai/orca

Length of output: 28916

---

Make unsettled forward-compatible.

The cloud contract accepts slack, discord, and linear conversations. The desktop contract accepts only slack. A non-Slack unsettled entry can therefore make the complete poll response unreadable. Parse unsettled entries independently. A valid delivery ID with an unsupported conversation should receive an unsupported completion acknowledgment; skip only entries that have no usable ID.

♻️ Suggested fix

 const ConnectorConversation = z.object({
   source: z.literal('slack'),
   externalId: z.string().min(1)
 })
 export type ConnectorConversation = z.infer<typeof ConnectorConversation>
 
+export const ConnectorUnsettled = z.object({
+  deliveryId: z.string().min(1),
+  conversation: ConnectorConversation
+})
+
 ...
-  unsettled: z
-    .array(z.object({ deliveryId: z.string().min(1), conversation: ConnectorConversation }))
-    .optional(),
+  unsettled: z.array(z.unknown()).optional(),

Call ConnectorUnsettled.safeParse for each entry in ConnectorPollClient.cycle. Send { outcome: 'unsupported' } for entries with a valid deliveryId that fail the per-entry parse.

Source: Coding guidelines