Skip to content

fix(channels/connect): remove 5-min CONNECT_TIMEOUT, poll until Ctrl+C (AIT-334) - #43

Merged
ord669 merged 1 commit into
mainfrom
ait-334-cli-connect-no-timeout
Aug 5, 2026
Merged

fix(channels/connect): remove 5-min CONNECT_TIMEOUT, poll until Ctrl+C (AIT-334)#43
ord669 merged 1 commit into
mainfrom
ait-334-cli-connect-no-timeout

Conversation

@ord669

@ord669 ord669 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Fixes AIT-334.

Problem

channels connect gave up after a hard 5-minute poll timeout. Sentry HOOKMYAPP-CLI-15 hit 5 new users on 2026-08-04: two had OAuth complete 8–11 min after poll start (CLI reported failure, connect then succeeded), two re-ran connect when the channel already existed (snapshot-diff poll can never satisfy → guaranteed timeout), one abandoned.

Change

  • No hard timeout. The poll waits until channels appear or Ctrl+C. A Still waiting (N min) hint prints every 30s so the wait is visibly alive.
  • Already-connected notice. If the workspace already has a connected channel of the requested type at start, the CLI says so before waiting for a NEW connection.

Tests

  • Replaced the two CONNECT_TIMEOUT tests: late channel at 6 min resolves; legacy no-updatedAt backend keeps polling instead of rejecting.
  • New test: onStillWaiting fires every ~30s while nothing appears.
  • Full suite: 131 files / 997 tests green; tsc + lint clean.

Summary by CodeRabbit

  • New Features

    • Channel connection setup now continues waiting beyond five minutes until a new connection is detected.
    • Added periodic progress updates while waiting, including elapsed-time guidance and cancellation instructions.
    • Existing connections are identified and reported before starting the authorization flow.
  • Bug Fixes

    • Fixed polling behavior when existing channels lack update timestamps.
    • Improved detection and handling of newly connected channels.

…C (AIT-334)

- pollForNewChannels no longer rejects after 5 minutes: real Meta OAuth
  (login + 2FA + permission screens) routinely exceeds it, and the timeout
  reported failure to users whose connect then succeeded (HOOKMYAPP-CLI-15).
- onStillWaiting hook fires every 30s so the open-ended wait stays visible;
  runChannelsConnect prints elapsed minutes + Ctrl+C hint.
- Up-front notice when the workspace already has a connected channel of the
  requested type: the poll only reports NEW connections, so a re-run on an
  already-connected workspace previously hit a guaranteed timeout with no
  clue the connect had already succeeded.
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3e5765ef-711d-4427-92b3-356d16a803ef

📥 Commits

Reviewing files that changed from the base of the PR and between e4b6478 and 30f5bff.

📒 Files selected for processing (3)
  • src/commands/__tests__/channels-connect-poll.test.ts
  • src/commands/channels-connect-poll.ts
  • src/commands/channels.ts

📝 Walkthrough

Walkthrough

pollForNewChannels no longer times out after five minutes. It supports 30-second waiting callbacks. runChannelsConnect reports existing channels, waits for a new connection, and displays cancellation and elapsed-time guidance.

Changes

Channel connection polling

Layer / File(s) Summary
Indefinite polling and callback behavior
src/commands/channels-connect-poll.ts, src/commands/__tests__/channels-connect-poll.test.ts
Polling no longer throws CONNECT_TIMEOUT. It waits for channel stabilization and invokes onStillWaiting every 30 seconds. Tests cover timestamp-less channels, waits beyond six minutes, eventual resolution, and callback timing.
Connect flow status and snapshot handling
src/commands/channels.ts
The connect flow parses the initial snapshot once, reports matching existing channels, and waits for a new connection with cancellation and elapsed-time guidance.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • hookmyapp/cli#1: Introduces or renames the channel polling functions changed here.

Suggested reviewers: ordvir

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: removing the five-minute timeout and polling until the user cancels with Ctrl+C.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ait-334-cli-connect-no-timeout

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

@ord669
ord669 merged commit dc2e52d into main Aug 5, 2026
3 checks passed
@ord669
ord669 deleted the ait-334-cli-connect-no-timeout branch August 5, 2026 06:51
ord669 added a commit that referenced this pull request Aug 12, 2026
…C (AIT-334) (#43)

- pollForNewChannels no longer rejects after 5 minutes: real Meta OAuth
  (login + 2FA + permission screens) routinely exceeds it, and the timeout
  reported failure to users whose connect then succeeded (HOOKMYAPP-CLI-15).
- onStillWaiting hook fires every 30s so the open-ended wait stays visible;
  runChannelsConnect prints elapsed minutes + Ctrl+C hint.
- Up-front notice when the workspace already has a connected channel of the
  requested type: the poll only reports NEW connections, so a re-run on an
  already-connected workspace previously hit a guaranteed timeout with no
  clue the connect had already succeeded.
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