Skip to content

fix(connect): return without redundant host hello - #1879

Merged
khaliqgant merged 5 commits into
mainfrom
fix/connect-join-no-blocking-hello
Oct 3, 2026
Merged

khaliqgant merged 5 commits into
mainfrom
fix/connect-join-no-blocking-hello

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Summary

  • return connect join output immediately after the probe admits the participant
  • rely on Cloud’s deferred idempotent join announcement instead of sending a second synchronous host hello
  • render the probe’s connect_join_timeout as an explicit safe-retry instruction

Validation

  • @agent-relay/connect test suite passes (36 tests)
  • package build passes
  • Prettier check passes for changed files

Cloud announcement change: AgentWorkforce/cloud#4128
Probe retry/timeout change: AgentWorkforce/relay-desktop#201

🤖 Generated with Claude Code


Note

Low Risk
CLI-only join flow and error messaging; removes an extra synchronous send and adds retry guidance for timeout/pending probe responses.

Overview
@agent-relay/connect join now finishes as soon as the desktop probe accepts the join, without a blocking follow-up /connect/send “hello” to the host; join announcements are left to Cloud’s deferred idempotent path instead.

Probe errors connect_join_timeout and connect_join_pending are mapped to exit code 8 with stderr text that tells callers to retry the same link and options safely. Changelog and CLI tests were updated to match (no post-join send; timeout/pending behavior).

Reviewed by Cursor Bugbot for commit 9d5e0d6. Bugbot is set up for automated code reviews on this repo. Configure here.


Agent Relay sessions

  • codex session 01a0fc4a · opened via gh pr create · last active 2026-10-02
  • claude session 02580a3c · contributor · pushed commits · last active 2026-10-02

Let the Cloud join announcement notify the host once, removing the synchronous post-join send and mapping the probe's retry-safe timeout code.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@devin-ai-integration devin-ai-integration 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9f126aed-dcde-4f3b-af32-1a3e38f87ddc

📥 Commits

Reviewing files that changed from the base of the PR and between ea940b0 and 9d5e0d6.

📒 Files selected for processing (1)
  • CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

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


📝 Walkthrough

Walkthrough

The guest join path no longer sends a readiness message to the host. The CLI maps pending and timed-out joins to exit code 8 and displays retry guidance.

Changes

Connect join completion

Layer / File(s) Summary
Guest join completion
packages/connect/src/cli.js, packages/connect/tests/connect.test.ts, CHANGELOG.md
The guest join path no longer sends a host-ready notification. Tests reject send requests and expect no sends. The changelog states that the command returns when the probe admits the participant.
Join timeout handling
packages/connect/src/cli.js, packages/connect/tests/connect.test.ts, CHANGELOG.md
The CLI maps connect_join_pending and connect_join_timeout to exit code 8 with retry guidance. Tests check both errors, and the changelog records the retry guidance.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 9d5e0

The changed join behavior and retry guidance are consistent with the inspected responses and tests. No issue identified here prevents merging.

Architecture Summary

Architecture risk: 🔵 Low · up to 61500

The change affects 2 systems.

Changed systems: packages/connect, CHANGELOG.md

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — packages/connect (library) was modified; 2 changed files map to changed impact.
  • observed — CHANGELOG.md (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in CHANGELOG.md: The unreleased heading is now labeled as a patch release. A new Fixed entry documents the join command’s return timing and retry guidance for timed-out or pending joins.
  • observed — Modified behavior in packages/connect/src/cli.js: errorDetails adds mappings for connect_join_pending and connect_join_timeout, each with retry guidance and exit code 8.
  • observed — Modified behavior in packages/connect/src/cli.js: Removed the post-join notification for non-host agents: the CLI no longer validates that the response identifies the agent and host, sends a readiness message to the host, or catches notification failures to print a warning.
  • observed — Modified behavior in packages/connect/tests/connect.test.ts: Renamed the join test to specify that the host claim stays private and joining makes no blocking follow-up send, replacing its former expectation of one hello per join.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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 3 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: removing the redundant host hello after connect join.
Description check ✅ Passed The description clearly summarizes the behavior change and includes validation results. It is mostly complete, although it uses a Validation section instead of the template's Test Plan section and doe…
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.
Full details: Docstring Coverage

Explanation

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 3 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

A rabbit joins and waits no more,
No host-ready note goes out the door.
If pending or timeout appears,
Retry the request; the path is clear.
I nibble clover, pleased to see
The join flow finish quietly.

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

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread CHANGELOG.md Outdated
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 3 files (changes from recent commits).

Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Re-trigger cubic

Comment thread packages/connect/src/cli.js Outdated
khaliqgant and others added 3 commits October 2, 2026 05:04
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Keep both sets of Unreleased changelog entries.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@khaliqgant
khaliqgant merged commit bca6fbf into main Oct 3, 2026
135 of 137 checks passed
@khaliqgant
khaliqgant deleted the fix/connect-join-no-blocking-hello branch October 3, 2026 06:08
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