Internal cleanup by Jinwoo-H · Pull Request #33862 · stablyai/orca

Jinwoo-H · 2026-10-26T09:14:45Z

No description provided.

… or Linear; a posted reply names its messageId

…r touch a stored Slack thread

…chine ignores it once that turn is over

coderabbitai · 2026-10-26T09:23:00Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Last reviewed commit: 1a4de8c
Latest commit, not reviewed: 6991f9b

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

Connector contracts now accept Slack, Discord, and Linear conversation sources, and machine hello requests include a source list. Machine selection requires Slack support. Connector event handling filters non-Slack conversations from event and snapshot processing. Posted reply responses use messageId instead of messageTs.

Priority: Pending latest changes

Merge Risk: Moderate · up to 1a4de

The change widens the connector contract to Discord and Linear, but several Slack paths still do not separate conversations by source. A non-Slack snapshot, reply, or turn-end could affect Slack thread state, and older machines that omit sources would be rejected. The feature is experimental and flag-gated, so impact is limited now. These gaps should be closed before Discord or Linear conversations are produced.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

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

Passed checks: 4 passed

Check name	Status	Explanation
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.
Description check		The description covers ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, Agent skill upstream boundary, Notes, and Review sections, and it explains the mechanism and alternatives. The auth…
Title check		The title clearly summarizes the main changes: multi-source conversations, machine source capabilities, and reply responses using messageId. It omits the optional cancel turnId detail, but the title r…

Full details: Description check

Explanation

The description covers ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, Agent skill upstream boundary, Notes, and Review sections, and it explains the mechanism and alternatives. The author fills most required parts. Gaps: the Linked Issue section says N/A (internal connector work) instead of linking an issue, and the Testing checklist's manual-testing box is left unchecked. The AI Disclosure section is absent, and the Checklist section from the template is missing. The description is mostly complete, so the check passes.

Full details: Title check

Explanation

The title clearly summarizes the main changes: multi-source conversations, machine source capabilities, and reply responses using messageId. It omits the optional cancel turnId detail, but the title remains specific and relevant.

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

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

coderabbitai

Actionable comments posted: 1

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: e127505b-d043-45df-a6ca-bea57a0c12c5

Commits

Reviewing files that changed from the base of the PR and between 196ca64 and b43cf5c.

Files selected for processing: 7

cloud/apps/connector/src/connector-events.test.ts
cloud/apps/connector/src/connector-inbox.ts
cloud/apps/connector/src/connector-threads.ts
cloud/packages/connector-contract/src/connector-wire-scalars.ts
cloud/packages/connector-contract/src/contract.test.ts
cloud/packages/connector-contract/src/delivery-messages.ts
cloud/packages/connector-contract/src/event-messages.ts

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

coderabbitai · 2026-10-26T09:23:03Z

+        continue
+      }
      if (event.kind === 'snapshot') {
+        const conversations = event.conversations.filter((entry) => isStored(entry.conversation))


Major · Data Integrity & Integration · Heavy lift

🔎 Supported by static analysis

Script executed:

set -e printf '%s\n' '--- changed implementation ---' nl -ba cloud/apps/connector/src/connector-threads.ts | sed -n '1,220p' printf '%s\n' '--- connector event symbols and types ---' rg -n -F --glob '*.{ts,tsx}' -e 'applyEvents' -e 'conversations:' -e 'conversationSnapshot' -e 'ConnectorConversation' cloud packages src 2>/dev/null || true printf '%s\n' '--- conversation event declarations ---' rg -n -F --glob '*.{ts,tsx}' -e 'type ConnectorEvent' -e 'interface ConnectorEvent' -e 'conversations' cloud/packages cloud/apps 2>/dev/null || true printf '%s\n' '--- tests for connector threads ---' rg -n -F --glob '*.{ts,tsx}' -e 'connector-threads' -e 'Discord' -e 'Linear' -e 'unverifiable' cloud/apps/connector 2>/dev/null || true

Repository: stablyai/orca

Length of output: 14392

Script executed:

printf '%s\n' 'pending source inspection'

Repository: stablyai/orca

Length of output: 177

Script executed:

set -e printf '%s\n' '--- changed implementation ---' nl -ba cloud/apps/connector/src/connector-threads.ts | sed -n '1,260p' printf '%s\n' '--- relevant symbols and callers ---' rg -n -F --glob '*.{ts,tsx}' -e 'applyEvents' -e 'ConnectorConversation' -e 'conversations:' -e 'unverifiable' cloud 2>/dev/null || true printf '%s\n' '--- event declarations ---' rg -n -F --glob '*.{ts,tsx}' -e 'type ConnectorEvent' -e 'interface ConnectorEvent' -e 'conversationSnapshot' -e 'ConversationSnapshot' cloud/packages cloud/apps 2>/dev/null || true printf '%s\n' '--- connector tests ---' rg -n -F --glob '*.{ts,tsx}' -e 'connector-threads' -e 'Discord' -e 'Linear' -e 'unverifiable' cloud/apps/connector 2>/dev/null || true

Repository: stablyai/orca

Length of output: 14605

Script executed:

set -e nl -ba cloud/apps/connector/src/connector-threads.ts | sed -n '1,260p' printf '%s\n' '--- refs ---' rg -n -F --glob '*.{ts,tsx}' -e 'applyEvents' -e 'ConnectorConversation' -e 'conversations:' -e 'unverifiable' cloud || true printf '%s\n' '--- contract names ---' rg -n -F --glob '*.{ts,tsx}' -e 'type ConnectorEvent' -e 'interface ConnectorEvent' -e 'ConversationSnapshot' -e 'conversationSnapshot' cloud || true

Repository: stablyai/orca

Length of output: 12762

Script executed:

set -e printf '%s\n' '--- connector event contract ---' nl -ba cloud/packages/connector-contract/src/event-messages.ts | sed -n '1,115p' printf '%s\n' '--- snapshot producer candidates ---' rg -n -F --glob '*.{ts,tsx}' -e "kind: 'snapshot'" -e 'kind: "snapshot"' -e 'conversations:' cloud/apps cloud/packages src/shared | head -n 120 printf '%s\n' '--- connector event tests ---' nl -ba cloud/apps/connector/src/connector-events.test.ts | sed -n '1,235p'

Repository: stablyai/orca

Length of output: 19887

Script executed:

nl -ba cloud/packages/connector-contract/src/event-messages.ts | sed -n '45,75p'

Repository: stablyai/orca

Length of output: 1535

Script executed:

nl -ba cloud/packages/connector-contract/src/connector-wire-scalars.ts | sed -n '1,45p'

Repository: stablyai/orca

Length of output: 1076

Do not treat filtered snapshots as complete Slack snapshots.

ConnectorEventSchema accepts Slack, Discord, and Linear conversations, but does not require a snapshot to contain every linked conversation. After a Slack turn or prompt is stored, a schema-valid snapshot containing only Discord or Linear entries leaves the filtered ID sets empty. The cleanup then marks the Slack turn unverifiable and clears its prompt.

The regression test does not detect this path because it creates no working Slack turn or Slack prompt before sending the non-Slack snapshot. If snapshots can be source-partial, apply absence-based cleanup only to snapshots authoritative for Slack.

…cancel's turnId

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Major · Propagate and enforce cancel.turnId in the runtime path. · delivery-messages.ts:33-40

cloud/packages/connector-contract/src/delivery-messages.ts:33-40
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Propagate and enforce cancel.turnId in the runtime path.

The cloud inbox preserves turnId, but ConnectorCancel and connector.cancel discard it before calling cancelConversation. That function then calls interruptWorkingAgent, which sends ESCAPE to whichever turn is currently working without comparing turn IDs.

A cancel for an older turn can therefore interrupt a newer active turn. Propagate turnId through the connector cancellation path and refuse mismatched turns. Preserve the existing conversation-wide behavior when turnId is absent.

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: bde35bb6-c3ab-4f12-90c3-c644973b2a14

Commits

Reviewing files that changed from the base of the PR and between b43cf5c and acf8e39.

Files selected for processing: 3

cloud/packages/connector-contract/src/contract.test.ts
cloud/packages/connector-contract/src/hello-messages.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; 9 remain after this review.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Major · Desktop cancel path drops turnId, so a turn-targeted cancel can… · delivery-messages.ts:33-40

cloud/packages/connector-contract/src/delivery-messages.ts:33-40
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Desktop cancel path drops turnId, so a turn-targeted cancel can stop the running turn.

The contract says a machine that advertises turnCancel honors cancel.turnId and refuses a cancel for any other turn. The desktop cancel path does not do this. A cancel that names an older turn can therefore stop the current turn. ConnectorCancel does not declare turnId, so parsing drops it. The handler and seam do not pass a turn target to cancelConversation.

Forwarding turnId through these layers would not change behavior by itself. cancelConversation must also compare the target with the live turn and refuse on mismatch. PR #33909 is reported to complete the desktop handling. This head does not include that change.

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: 96e9cc08-bde0-44c3-b7d5-376c53c6ba53

Commits

Reviewing files that changed from the base of the PR and between acf8e39 and fc8e97b.

Files selected for processing: 1

cloud/packages/connector-contract/src/contract.test.ts

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

…ract-sources # Conflicts: # cloud/apps/connector/src/connector-inbox.ts

…ct now does

…ract-sources

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (2)

Major · Filter non-Slack turn endings before queuing Slack fallbacks. · slack-edge.ts:160-164

cloud/apps/connector/src/slack-edge.ts:160-164
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Filter non-Slack turn endings before queuing Slack fallbacks.

ConnectorThreads.applyEvents skips non-Slack events only for thread-state updates. machine-routes.ts still passes the original events to slack.onEvents. The callback can therefore queue a non-Slack message for a Slack thread with the same host and external ID.
Suggested fix
-        if (event.kind === 'turn-ended' && event.lastAssistantMessage !== undefined) {
+        if (
+          event.conversation.source === 'slack' &&
+          event.kind === 'turn-ended' &&
+          event.lastAssistantMessage !== undefined
+        ) {

Minor · Reject non-Slack conversations before the reply lookup. · slack-edge.ts:119

cloud/apps/connector/src/slack-edge.ts:119
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Reject non-Slack conversations before the reply lookup.

The reply handler drops conversation.source and passes only conversation.externalId to queueAgentReply. The thread lookup then matches on external_id and host_id only, so a Discord or Linear reply can post to a Slack thread that has the same external ID. Because ConnectorConversationSchema accepts any source with an OpaqueIdSchema external ID, the request is not rejected before this lookup.

Reject non-Slack sources in the handler. For a non-Slack source, the response matches the existing "not this host's thread" result. Non-collision requests keep their current result.
🐛 Suggested fix
       const { replyId, conversation, turnId, text } = body.data
+      if (conversation.source !== 'slack') {
+        return context.json({ error: 'not_this_hosts_thread' }, 403)
+      }
       const sent = await database.transaction((transaction) =>

🔇 Additional comments: 2

cloud/apps/connector/src/connector-threads.ts: 1

70-70: The filtered-snapshot cleanup concern remains.

A non-Slack-only snapshot leaves conversations empty, but the following updates can mark Slack working turns unverifiable and clear Slack prompts. This is the same issue reported in the prior review comment on Line 70.

cloud/packages/connector-contract/src/event-messages.ts-98-98: 1

98-98: Major · Data Integrity & Integration · Quick win

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Keep version 1 posted responses compatible.

A posted response with the former messageTs field now fails parsing because messageId is required. The endpoint also stops returning messageTs, so existing clients that read that field lose the posted message identifier. Accept both fields when parsing, and return both until the wire version changes or clients negotiate the new shape. Based on learnings, versioned Zod message fields should be deprecated rather than removed.

Source: Learnings

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: 061c00c5-91db-4c73-8c79-c0b272c6d013

Commits

Reviewing files that changed from the base of the PR and between fc8e97b and e757af8.

Files selected for processing: 8

cloud/apps/connector/src/connector-events.test.ts
cloud/apps/connector/src/connector-inbox.ts
cloud/apps/connector/src/connector-threads.ts
cloud/apps/connector/src/slack-edge.ts
cloud/apps/connector/src/slack-posting.test.ts
cloud/packages/connector-contract/src/contract.test.ts
cloud/packages/connector-contract/src/event-messages.ts
cloud/packages/connector-contract/src/hello-messages.ts

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

… contract The cloud picks a machine for a Slack conversation only among those whose hello lists slack, so a newer source never lands on a machine that would refuse it. The turn-bound cancel (turnId on cancel, the turnCancel feature) goes with Stop, which is not in the first release.

…ck thread with the same id Events are narrowed to stored (Slack) conversations once, before both the thread state and the Slack posts read them, and a reply for another source is refused like one for a thread the machine doesn't own.

coderabbitai

Actionable comments posted: 1

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: ea8e2d3a-4d77-4b43-85ca-372092875adf

Commits

Reviewing files that changed from the base of the PR and between e757af8 and 1a4de8c.

Files selected for processing: 6

cloud/apps/connector/src/connector-server-harness.test-fixture.ts
cloud/apps/connector/src/slack-harness.test-fixture.ts
cloud/apps/connector/src/slack-machine-choice.test.ts
cloud/apps/connector/src/slack-machine-choice.ts
cloud/packages/connector-contract/src/contract.test.ts
cloud/packages/connector-contract/src/hello-messages.ts

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

coderabbitai · 2026-10-26T18:12:24Z

  promptKinds: CapabilityList,
  features: CapabilityList,
+  // The cloud sends a conversation only to a machine that lists its source.
+  sources: CapabilityList,


Minor · Data Integrity & Integration · Quick win

🔎 Supported by static analysis

Script executed:

sed -n '1,60p' cloud/packages/connector-contract/src/hello-messages.ts rg -n "sources|turnCancel|ConnectorHelloRequest" --glob '!**/node_modules/**' . | head -60

Repository: stablyai/orca

Length of output: 9122

Script executed:

# Locate hello senders, the schema consumers, and any sources population. rg -n -F -e 'ConnectorHelloRequest' -e 'promptKinds' -e 'turnCancel' -e 'hello_required' -- cloud src || test "$?" -eq 1 echo '---sources-producers---' rg -n -F -e 'sources:' -e 'sources,' -- src/main src/shared cloud/packages 2>/dev/null | head -60 || test "$?" -eq 1 echo '---connector-flags---' rg -n -i -e 'connector.*(flag|experimental|enabled)' -e '(flag|experimental).*connector' -- src cloud 2>/dev/null | head -40 || test "$?" -eq 1

Repository: stablyai/orca

Length of output: 13562

Keep sources optional for legacy hellos.

sources is required, but the contract still accepts only v: 1. A v: 1 hello that omits sources fails parsing and is rejected. The connector is gated behind experimentalConnectors, which defaults to off. An opted-in machine on an older build can therefore fail hello validation and lose Slack conversations until it sends sources. Defaulting an omitted list to slack keeps those hellos on the Slack-only behavior the PR widens.

🐛 Suggested fix

--- "a/cloud/packages/connector-contract/src/hello-messages.ts" +++ "b/cloud/packages/connector-contract/src/hello-messages.ts" @@ -33,7 +33,7 @@ promptKinds: CapabilityList, features: CapabilityList, // The cloud sends a conversation only to a machine that lists its source. - sources: CapabilityList, + sources: CapabilityList.default(['slack']), projects: z.array(ConnectorProjectSchema).max(CONNECTOR_LIMITS.maxHelloProjects) })

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

sources: CapabilityList,

sources: CapabilityList.default(['slack']),

…ract-sources

Jinwoo-H added 3 commits October 26, 2026 04:44

feat(connector-contract): a conversation can come from Slack, Discord…

4e58aca

… or Linear; a posted reply names its messageId

fix(connector): turn events for a Discord or Linear conversation neve…

3a05291

…r touch a stored Slack thread

feat(connector-contract): a cancel can name the turn it stops; the ma…

b43cf5c

…chine ignores it once that turn is over

coderabbitai Bot reviewed Oct 26, 2026

View reviewed changes

Jinwoo-H mentioned this pull request Oct 26, 2026

Internal cleanup #33909

Merged

Jinwoo-H added 2 commits October 26, 2026 08:30

feat(connector-contract): a machine can list turnCancel, honouring a …

acf8e39

…cancel's turnId

test(connector-contract): drop the feature-list inventory assertion

fc8e97b

coderabbitai Bot reviewed Oct 26, 2026

View reviewed changes

Jinwoo-H added 3 commits October 26, 2026 13:53

Merge remote-tracking branch 'origin/main' into jinwoo/connector-cont…

6822db3

…ract-sources # Conflicts: # cloud/apps/connector/src/connector-inbox.ts

fix(connector): the Slack reply answer names messageId, as the contra…

8f42deb

…ct now does

Merge remote-tracking branch 'origin/main' into jinwoo/connector-cont…

e757af8

…ract-sources

coderabbitai Bot reviewed Oct 26, 2026

View reviewed changes

Jinwoo-H added 2 commits October 26, 2026 14:05

coderabbitai Bot reviewed Oct 26, 2026

View reviewed changes

Jinwoo-H mentioned this pull request Oct 26, 2026

Internal cleanup #33945

Open

Jinwoo-H changed the title ~~feat(connector-contract): conversations from Slack, Discord or Linear; replies name a messageId; a cancel can name its turn~~ feat(connector-contract): conversations from Slack, Discord or Linear; machines list the sources they take; replies name a messageId Oct 26, 2026

Jinwoo-H mentioned this pull request Oct 26, 2026

Internal cleanup #33927

Merged

Merge remote-tracking branch 'origin/main' into jinwoo/connector-cont…

6991f9b

…ract-sources

Jinwoo-H merged commit 22fd6f5 into main Oct 26, 2026
27 checks passed

Jinwoo-H mentioned this pull request Oct 26, 2026

Internal cleanup #33855

Merged

Jinwoo-H changed the title ~~feat(connector-contract): conversations from Slack, Discord or Linear; machines list the sources they take; replies name a messageId~~ Internal cleanup Nov 4, 2026

Navigation Menu

Repository navigation

Internal cleanup - #33862

Internal cleanup#33862
Jinwoo-H merged 11 commits into
mainfrom
jinwoo/connector-contract-sources

Jinwoo-H commented Oct 26, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 26, 2026 •

edited

Loading

Reviews paused

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Oct 26, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Oct 26, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	sources: CapabilityList,
	sources: CapabilityList.default(['slack']),

Conversation

Jinwoo-H commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Jinwoo-H commented Oct 26, 2026 •

edited

Loading

coderabbitai Bot commented Oct 26, 2026 •

edited

Loading