Skip to content

Internal cleanup - #33670

Merged
Jinwoo-H merged 10 commits into
mainfrom
jinwoo/connector-contract
Oct 26, 2026
Merged

Jinwoo-H merged 10 commits into
mainfrom
jinwoo/connector-contract

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

No description provided.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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.

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

Adds a private ES-module package that exports connector routes, limits, wire scalars, and versioned schemas for authentication, polling, deliveries, events, and replies. Adds matching machine-side contract constants. Adds schema tests and host-proof vector tests.

Priority: Normal

Merge Risk: Moderate · up to f1e4c

Mixed-version poll compatibility remains unsafe, and direct connector package validation can fail in a fresh checkout until the dependency build prerequisite is added.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 14 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title is specific and accurately describes the main change: a wire contract between Orca and the cloud connector service, added as inert code. It uses the conventional feat(connector) prefix and i…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The pull request description is complete and follows the repository template. It explains the change, motivation, compatibility approach, testing, review history, security impact, lack of UI changes, …
Full details: Title check

Explanation

The title is specific and accurately describes the main change: a wire contract between Orca and the cloud connector service, added as inert code. It uses the conventional feat(connector) prefix and is one sentence. It is 83 characters, which is slightly long but still clear.

Full details: Description check

Explanation

The pull request description is complete and follows the repository template. It explains the change, motivation, compatibility approach, testing, review history, security impact, lack of UI changes, and checklist status. The maintainer correctly marks the linked issue as not applicable. Manual testing was not reported, but automated validation is documented.

Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • 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 coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2


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: 04edd59e-746d-44e8-b36c-df89ba53f19a
Commits

Reviewing files that changed from the base of the PR and between 23798ef and 3e43652.

Files ignored due to path filters: 1
  • cloud/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
Files selected for processing: 18
  • cloud/packages/connector-contract/package.json
  • cloud/packages/connector-contract/src/connector-endpoints.ts
  • cloud/packages/connector-contract/src/connector-host-proof-vector.json
  • cloud/packages/connector-contract/src/connector-host-proof-vector.test.ts
  • cloud/packages/connector-contract/src/connector-limits.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
  • cloud/packages/connector-contract/src/hello-messages.ts
  • cloud/packages/connector-contract/src/host-auth-messages.ts
  • cloud/packages/connector-contract/src/index.ts
  • cloud/packages/connector-contract/src/sign-in-messages.ts
  • cloud/packages/connector-contract/tsconfig.build.json
  • cloud/packages/connector-contract/tsconfig.json
  • config/scripts/check-test-file-runners.mjs
  • src/main/runtime/connector-host-proof-vector.test.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; 8 remain after this review.

unsettled: z
.array(z.object({ deliveryId: OpaqueIdSchema, conversation: ConnectorConversationSchema }))
.optional(),
deliveries: z.array(ConnectorDeliverySchema).max(CONNECTOR_LIMITS.maxDeliveriesPerPoll),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Major · Data Integrity & Integration · Heavy lift

🔎 Supported by static analysis

Script executed:

cat cloud/packages/connector-contract/src/delivery-messages.ts
rg -n 'unsupported|unknown|kind' cloud/packages/connector-contract/src/contract.test.ts

Repository: stablyai/orca

Length of output: 4365


Stop unknown delivery kinds from rejecting the whole poll.

ConnectorPollResponseSchema validates deliveries as z.array(ConnectorDeliverySchema). A delivery whose kind is not start, send, answer, or cancel fails the discriminated union. That failure rejects the whole poll response. The comment above ConnectorDeliverySchema says the machine settles such a delivery as {outcome:'unsupported'}. The schema does not do this, so the machine cannot acknowledge the unknown entry or process the other deliveries in the same poll.

Parse the poll envelope first. Then validate each delivery separately. Settle an unreadable delivery as unsupported when its deliveryId is readable, and keep the other deliveries. This keeps mixed-version clients compatible when the cloud adds a new kind.

The package is inert today, so this defect has no runtime effect yet. It becomes active when a consumer wires the contract.

.array(z.object({ deliveryId: OpaqueIdSchema, conversation: ConnectorConversationSchema }))
.optional(),
deliveries: z.array(ConnectorDeliverySchema).max(CONNECTOR_LIMITS.maxDeliveriesPerPoll),
control: PollControlSchema.optional()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor · Data Integrity & Integration · Quick win

🔎 Supported by static analysis

Script executed:

sed -n '1,90p' cloud/packages/connector-contract/src/delivery-messages.ts
rg -n 'control|Control' cloud/packages/connector-contract/src/contract.test.ts

Repository: stablyai/orca

Length of output: 3672


Script executed:

set -o pipefail
printf '%s\n' '--- contract.test.ts ---'
nl -ba cloud/packages/connector-contract/src/contract.test.ts | sed -n '1,110p'
printf '%s\n' '--- schema references ---'
rg -n -F --glob '*.ts' -e 'ConnectorPollResponseSchema' -e 'PollControlSchema' -e 'connector-contract' cloud src || test "$?" -eq 1
printf '%s\n' '--- package files ---'
rg --files cloud/packages/connector-contract | sort

Repository: stablyai/orca

Length of output: 6699


Ignore controls without a recognized shape.

PollControlSchema accepts only three control shapes. .optional() permits an absent control, but a present control with no recognized shape still causes ConnectorPollResponseSchema to fail. This can hide valid deliveries from mixed-version clients. The contract is not currently consumed, so the impact is limited to future interoperability.

Suggested fix
--- "a/cloud/packages/connector-contract/src/delivery-messages.ts"
+++ "b/cloud/packages/connector-contract/src/delivery-messages.ts"
@@ -47,7 +47,7 @@
     .array(z.object({ deliveryId: OpaqueIdSchema, conversation: ConnectorConversationSchema }))
     .optional(),
   deliveries: z.array(ConnectorDeliverySchema).max(CONNECTOR_LIMITS.maxDeliveriesPerPoll),
-  control: PollControlSchema.optional()
+  control: PollControlSchema.optional().catch(undefined)
 })
 
 export const ConnectorDeliveryResultSchema = z.discriminatedUnion('outcome', [

Add a regression test that supplies an unrecognized control and asserts that parsing succeeds with data.control undefined.

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
control: PollControlSchema.optional()
control: PollControlSchema.optional().catch(undefined)

@Jinwoo-H Jinwoo-H mentioned this pull request Oct 26, 2026
@Jinwoo-H Jinwoo-H closed this Oct 26, 2026
@Jinwoo-H Jinwoo-H reopened this Oct 26, 2026
This was referenced Oct 26, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

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

⚠️ Outside diff range comments (1)

Minor · Build @orca-cloud/push-contract before connector-contract typecheck and… · package.json:14-17

cloud/packages/connector-contract/package.json:14-17
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Build @orca-cloud/push-contract before connector-contract typecheck and build.

A direct pnpm typecheck or pnpm build in cloud/packages/connector-contract can fail in a fresh checkout. @orca-cloud/push-contract exposes dist/index.d.ts, but its dist directory is not tracked. Only pretest builds that dependency. The workspace-level build supplies the prerequisite, but the package scripts do not.

Suggested fix
     "build": "tsc -p tsconfig.build.json",
+    "prebuild": "pnpm --filter @orca-cloud/push-contract build",
     "typecheck": "tsc -p tsconfig.json --noEmit",
+    "pretypecheck": "pnpm --filter @orca-cloud/push-contract build",

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: d9aa9c9b-3570-4727-9acb-255929f845dd
Commits

Reviewing files that changed from the base of the PR and between 5b7d32c and f1e4c42.

Files selected for processing: 2
  • cloud/packages/connector-contract/src/contract.test.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; 6 remain after this review.

@Jinwoo-H Jinwoo-H mentioned this pull request Oct 26, 2026
@Jinwoo-H
Jinwoo-H merged commit 869ae49 into main Oct 26, 2026
50 checks passed
@Jinwoo-H Jinwoo-H mentioned this pull request Oct 26, 2026
@Jinwoo-H Jinwoo-H changed the title feat(connector): wire contract between Orca and the cloud connector service (inert) Internal cleanup Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant