Skip to content

Fix desktop WebSocket livelock and temporarily pause Linux CI - #166

Merged
mergify[bot] merged 4 commits into
mainfrom
agent/fix-desktop-loopback-ws-admission
Jul 24, 2026
Merged

mergify[bot] merged 4 commits into
mainfrom
agent/fix-desktop-loopback-ws-admission

Conversation

@slashdevcorpse

@slashdevcorpse slashdevcorpse commented Jul 24, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Disable the pre-auth per-peer WebSocket bucket only for private desktop loopback (mode=desktop, no publicUrl, and the configured host passes the existing isLoopbackHost policy).
  • Preserve the global connection cap, origin/auth checks, per-connection message limits, public-URL proxy behavior, and peer throttling for web or non-loopback deployments.
  • Make the exact peer-rate retry assertion deterministic with a fixed test clock.
  • Temporarily pause the three backlogged Linux CI lanes (quality_linux, the Linux unit-matrix entry, and e2e_linux) while keeping Windows validation required and macOS skipped.

Root cause and security boundary

The desktop renderer opens bootstrap and feature sockets that share one loopback peer identity. Startup and reconnect cycles depleted the default 10-per-minute pre-auth peer bucket; subsequent retries received HTTP 429 and could remain in a reconnect livelock until the bucket refilled.

The exemption intentionally uses the existing shared loopback predicate. Packaged desktop binds canonical 127.0.0.1; noncanonical desktop bind configurations remain peer-throttled fail-safe. This avoids broadening shared auth, health, trusted-origin, and shutdown policy semantics to all of 127/8.

Temporary Windows-only CI

  • quality_linux and e2e_linux are explicitly skipped.
  • The bounded unit matrix contains only windows-2022.
  • The quality aggregate requires Windows lanes to succeed and the backlogged Linux/macOS lanes to be skipped.
  • Workflow contracts lock this temporary topology against accidental drift.

Validation

  • bun run --cwd apps/server test -- src/wsTransportAdmission.test.ts src/wsRpc.connectionLifecycle.test.ts
    • 2 files passed
    • 43 tests passed
  • bun run --cwd scripts test -- lib/workflow-contracts.test.ts
    • 1 file passed
    • 30 tests passed
  • bun run workflows:validate
    • Passed for 7 allowed and 4 disabled workflow paths

Summary by CodeRabbit

  • Bug Fixes
    • Improved WebSocket admission for “private desktop” loopback to prevent peer-bucket throttling, while still enforcing the global connection cap.
    • Ensured peer-rate limiting and proxy traffic behavior remain consistent across desktop (non-loopback) and direct-web modes.
    • Fixed peer tracking state across repeated private-desktop bootstrap/feature cycles.
  • Tests
    • Expanded and reshaped WebSocket admission tests to cover private desktop vs other modes and re-validate proxy/global-cap behavior.
  • CI / Chores
    • Backlogged Linux quality and E2E lanes (treated as skipped) and updated workflow contract expectations/matrices accordingly.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8d726d81-b629-4538-a9ad-bf2cd35472d3

📥 Commits

Reviewing files that changed from the base of the PR and between 4be39e4 and edb2c84.

📒 Files selected for processing (2)
  • scripts/lib/workflow-contracts.test.ts
  • scripts/lib/workflow-contracts.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • scripts/lib/workflow-contracts.test.ts
  • scripts/lib/workflow-contracts.ts

📝 Walkthrough

Walkthrough

WebSocket transport admission now distinguishes private desktop loopback configurations from other modes. CI workflow contracts and jobs model backlogged Linux lanes as disabled, remove Linux from the unit matrix, and expect skipped aggregate results.

Changes

WebSocket admission throttling

Layer / File(s) Summary
Loopback-aware admission options
apps/server/src/nodeHttpServer.ts
The admission helper accepts optional mode and host values and disables peer rate limiting only for private desktop loopback configurations.
Admission behavior coverage
apps/server/src/wsTransportAdmission.test.ts
Tests cover binding decisions, peer throttling, proxy tracking, lifecycle cleanup, and global connection-cap enforcement.

Linux CI backlogging

Layer / File(s) Summary
Backlogged CI contract rules
scripts/lib/workflow-contracts.ts
Workflow validation requires Linux backlogging conditions, updated unit matrix contents, and skipped Linux results in aggregate gates.
Disabled Linux workflow lanes
.github/workflows/ci.yml
Linux quality and E2E jobs are disabled, Linux is removed from the unit matrix, and aggregate assertions expect skipped results.
Workflow contract fixture coverage
scripts/lib/workflow-contracts.test.ts
Fixtures and tests verify disabled Linux lanes, required platforms, macOS toggling, and updated Windows drift expectations.

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

Possibly related PRs

Suggested labels: ready-to-merge

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the two main changes: fixing desktop WebSocket livelock and pausing Linux CI.
Description check ✅ Passed The description covers the change, rationale, CI impact, and validation, though it doesn’t use the exact template headings.
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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/server/src/wsTransportAdmission.test.ts`:
- Around line 150-165: Update the wsTransportAdmissionOptionsForServerConfig
overrides in this test to inject a fixed clock with now returning 0, ensuring
the retryAfterMs assertion remains exactly 30_000 across all connection
acquisitions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f6fe429-0413-4611-9759-149562f30b1a

📥 Commits

Reviewing files that changed from the base of the PR and between ebe2ff3 and c117c0d.

📒 Files selected for processing (2)
  • apps/server/src/nodeHttpServer.ts
  • apps/server/src/wsTransportAdmission.test.ts

Comment thread apps/server/src/wsTransportAdmission.test.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

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 2 files

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

Re-trigger cubic

Comment thread apps/server/src/nodeHttpServer.ts
Comment thread apps/server/src/wsTransportAdmission.test.ts
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@slashdevcorpse slashdevcorpse changed the title Fix desktop WebSocket reconnect rate livelock Fix desktop WebSocket livelock and temporarily pause Linux CI Jul 24, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

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 4 files (changes from recent commits).

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

Re-trigger cubic

Comment thread scripts/lib/workflow-contracts.ts

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai coderabbitai Bot added the ready-to-merge Approved for Mergify auto-merge after required checks pass label Jul 24, 2026
@mergify

mergify Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

Merge Queue Status

  • ✅ Entered queue — 2026-07-24 07:05 UTC · Rule: default · triggered by merge protections
  • ✅ Checks skipped · PR is already up-to-date
  • ✅ Merged — 2026-07-24 07:05 UTC · at edb2c846c0902c4f00d6e8e66aed2bc8366b4640 · squash

This pull request spent 8 seconds in the queue, including 1 second running CI.

Required conditions to merge
  • #review-threads-unresolved = 0 [🛡 GitHub repository ruleset rule Super Synara main protection]
  • base = main
  • github-review-approved [🛡 GitHub repository ruleset rule Super Synara main protection] (documentation)
  • any of [🛡 GitHub repository ruleset rule Super Synara main protection]:
    • check-success = quality_windows
    • check-neutral = quality_windows
    • check-skipped = quality_windows
  • any of [🛡 GitHub repository ruleset rule Super Synara main protection]:
    • check-success = unit_windows
    • check-neutral = unit_windows
    • check-skipped = unit_windows
  • any of [🛡 GitHub repository ruleset rule Super Synara main protection]:
    • check-success = browser_windows
    • check-neutral = browser_windows
    • check-skipped = browser_windows
  • any of [🛡 GitHub repository ruleset rule Super Synara main protection]:
    • check-success = windows_x64
    • check-neutral = windows_x64
    • check-skipped = windows_x64
  • any of [🛡 GitHub repository ruleset rule Super Synara main protection]:
    • check-success = e2e_windows
    • check-neutral = e2e_windows
    • check-skipped = e2e_windows
  • any of [🛡 GitHub repository ruleset rule Super Synara main protection]:
    • check-success = release_smoke
    • check-neutral = release_smoke
    • check-skipped = release_smoke
  • any of [🛡 GitHub repository ruleset rule Super Synara main protection]:
    • check-success = dependency-review
    • check-neutral = dependency-review
    • check-skipped = dependency-review
  • any of [🛡 GitHub repository ruleset rule Super Synara main protection]:
    • check-success = codeql-actions
    • check-neutral = codeql-actions
    • check-skipped = codeql-actions
  • any of [🛡 GitHub repository ruleset rule Super Synara main protection]:
    • check-success = codeql-javascript-typescript
    • check-neutral = codeql-javascript-typescript
    • check-skipped = codeql-javascript-typescript

@mergify
mergify Bot merged commit 6620c72 into main Jul 24, 2026
29 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Approved for Mergify auto-merge after required checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant