Skip to content

fix(web): retry automatic machine selection after resource failures - #14603

Closed
dominic-r wants to merge 1 commit into
pingdotgg:mainfrom
d5cr:dominic/fix-auto-machine-selection-verified
Closed

dominic-r wants to merge 1 commit into
pingdotgg:mainfrom
d5cr:dominic/fix-auto-machine-selection-verified

Conversation

@dominic-r

Copy link
Copy Markdown
Contributor

Problem

An automatic draft stays blocked if its first machine-resource check fails or reports a busy host, even after the host recovers. The mounted query does not refresh itself when its stale time expires.

Change

Retry resource checks every five seconds while an automatic draft has no selected machine. Stop after selection, disabling automatic routing, or closing the draft. Preserve the existing eligibility, preference, CPU, memory, and freshness limits. Blocked sends now distinguish checking, no eligible machines, unavailable readings, and busy machines.

Scope and approval

Replacement for #12774, which Julius closed for missing UI verification evidence. The requested before/after screenshots and recovery recording are included below. GitHub reports viewerCanReopen: false for the original PR, so this is submitted for reconsideration as a replacement.

This fixes the existing automatic machine-selection flow getting stuck after a rejected resource sample. It preserves the eligibility rules, resource thresholds, and default settings.

Rebased onto upstream main at 5cc99e1c2. Head d71f82adf is one commit directly on that base. Its complete Git tree is identical to 27e24dcc7, the revision used for the UI evidence and prior typecheck/build verification.

Verification

  • 162 focused tests passed again after the rebase: useLoadBalancedEnvironment.test.tsx, ChatView.logic.test.ts, and projectGrouping.test.ts.
  • Web and client-runtime typechecks passed. Targeted lint passed with existing React compiler warnings. Web production build passed using the repository's installed Vite CLI.
  • Chromium ran the real web app against two isolated local servers. Only server.getHostResources replies were simulated. Busy uses 98% CPU; unavailable uses a null CPU reading; healthy uses 25% CPU. Memory remains 75% available. Setting both machine preference weights to zero exercises no eligible machines.
  • Before captures use the three affected production modules from upstream main at 5cc99e1c2. The original hook remained blocked for eight seconds after healthy readings became available, with no new resource requests.
  • The fixed code requested fresh readings about five seconds after the busy sample. It selected a machine 4.1 seconds after the fixture became healthy, kept the same draft URL, and accepted the message when Send was clicked. No draft reopen or page reload occurred during recovery.
  • Web and desktop share this UI. Mobile has no automatic load-balancing caller. Native desktop/mobile shells and relay/tunnel connections were not exercised. No wire schemas or provider adapters changed.

UI evidence

Case Before After
Busy machine Before: Busy machine After: Busy machine
Unavailable resource reading Before: Unavailable resource reading After: Unavailable resource reading
No eligible machines Before: No eligible machines After: No eligible machines

Watch automatic recovery without reopening the draft. The recording starts with the busy warning, then shows the same draft sending successfully after the next resource check.

Capture scripts and measured results are attached as GitHub release assets on the fork. No screenshots or recordings are committed to the PR.

Implemented and verified with GPT-6.0 Astra through the Codex harness in T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 1, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 1, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d71f82a

Macroscope's review found this PR approvable — This is a focused bug fix to recover existing automatic machine selection after resource failures or capacity changes. The retry is bounded to unresolved drafts, preserves existing eligibility rules, has cleanup behavior and targeted coverage, and introduces no schema, security, infrastructure, or default-setting changes.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ca17f975-a26a-4335-8d6d-a58fa601ba19

📥 Commits

Reviewing files that changed from the base of the PR and between 5cc99e1 and d71f82a.

📒 Files selected for processing (5)
  • apps/web/src/components/ChatView.tsx
  • apps/web/src/hooks/useLoadBalancedEnvironment.test.tsx
  • apps/web/src/hooks/useLoadBalancedEnvironment.ts
  • packages/client-runtime/src/load-balancing.ts
  • packages/client-runtime/src/state/projectGrouping.test.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.


📝 Walkthrough

Walkthrough

Automatic environment balancing now returns explicit selection statuses. The hook retries host-resource reads under defined conditions, and ChatView displays status-specific labels and warnings.

Changes

Automatic environment balancing

Layer / File(s) Summary
Selection results and status classification
packages/client-runtime/src/load-balancing.ts, packages/client-runtime/src/state/projectGrouping.test.ts
The chooser returns an environment ID with a selected, no-candidates, unavailable, or at-capacity status. Tests cover candidate eligibility, resource data validity, capacity limits, and selection results.
Resource refresh and selection status
apps/web/src/hooks/useLoadBalancedEnvironment.ts, apps/web/src/hooks/useLoadBalancedEnvironment.test.tsx
The hook exposes the chooser status and refreshes host resources every five seconds when resources exist, none are pending, and no environment is selected. Tests cover retries, recovery, and stopping retries.
Status-specific ChatView guidance
apps/web/src/components/ChatView.tsx
ChatView labels unavailable and no-candidate states and displays send-block warning titles and guidance based on load-balancing status. The at-capacity guidance names the CPU and available-memory thresholds.

Priority: ⬇️ Low

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Hook as useLoadBalancedEnvironment
  participant Chooser as chooseLoadBalancedEnvironment
  participant Chat as ChatView
  Hook->>Chooser: Evaluate candidate resources
  Chooser-->>Hook: Return environmentId and status
  Hook->>Hook: Refresh host resources every five seconds when no selection is available
  Hook-->>Chat: Provide environmentId and status
Loading

Suggested reviewers: maria-rcks

Merge Risk: ⚪ Minimal · up to d71f8

Automatic selection now retries unavailable resource readings and provides status-specific guidance. No actionable merge-blocking risk remains on the supplied evidence; merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d71f8

The change retries existing resource checks while preserving machine eligibility and selection safeguards. No new privilege or trust-boundary expansion was identified in the examined flow, but wider runtime behavior was not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The added activity repeats reads across the active draft’s eligible candidate environments, not only the eventual selected machine. The examined change increases request repetition through the existing operation rather than adding a new environment source, RPC payload, or execution privilege.

Trust Boundaries and Controls

  • observed — Recovery still passes through connection and provider eligibility checks, resource-validity checks, and current-project environment resolution. The host-resource RPC continues to declare an authorization failure. These are inspected controls, not proof of complete server-side enforcement.

Resilience and Maintainability Implications

  • inferred — The examined retry, assignment, manual-routing, and draft-cleanup guards contain recovery within current draft ownership. Resource failure does not introduce a fallback that bypasses eligibility or capacity checks, and sending prevents concurrent selection persistence in the inspected path.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. 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 main change: retrying automatic machine selection after resource failures.
Description check ✅ Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the affected behavior, implementation, approval context, focused test results, UI evidence…
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 2, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 2, 2026 05:43

Dismissing prior approval to re-evaluate d71f82a

@d5cr d5cr closed this by deleting the head repository Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants