Skip to content

fix(server): explain incompatible CLI executables during discovery - #14594

Closed
camptx wants to merge 2 commits into
pingdotgg:mainfrom
camptx:fix/gh-bad-architecture
Closed

camptx wants to merge 2 commits into
pingdotgg:mainfrom
camptx:fix/gh-bad-architecture

Conversation

@camptx

@camptx camptx commented Oct 1, 2026 •

Copy link
Copy Markdown

An Intel-only gh on an Apple Silicon Mac is marked missing during source-control discovery, but the installation hint does not explain which executable cannot run or why. This fixes the remaining diagnostic from #14590. The synchronous spawn failure and connection handling are already addressed on current main.

The rebuilt fix recognizes raw and platform-wrapped macOS architecture errors, names the executable and the server architecture, and preserves the installation guidance. It uses the existing installHint consumed by Settings, Git actions, and shared project-readiness checks. Git and Jujutsu use the same diagnostic. No client or contract changes are needed.

Fixes #14590. Scope approved in the maintainer reply. Related open PR #21606 addresses the same remaining diagnostic.

Verification

  • The new discovery regression tests failed on main: incompatible executables returned only Install GitHub CLI. They pass with this fix for synchronous errno -86, symbolic EBADARCH, and platform-wrapped errors. Ordinary missing and permission errors retain their hints; Linux errno -86 is not misclassified.
  • Focused discovery and process-runner tests passed.
  • Changed-file checks passed for formatting, lint, unused code, generated GraphQL operations, affected-package typechecks, and adjacent tests.
  • Checked on Linux. The macOS kernel failure and native desktop/mobile clients were not exercised in this rebuild. Earlier Apple Silicon reproduction evidence remains in the original verification comment; it is historical evidence, not a current-head run.

Rebuilt with GPT-6.1 Sol (gpt-6.1-sol, high reasoning effort) 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:L 100-499 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 4a09f4e

Macroscope's review found this PR approvable — This is a localized bug fix that makes incompatible optional CLI executables fail gracefully, preserves unrelated defects, and surfaces a useful architecture diagnostic through existing discovery and UI fields. The production behavior change is narrow and supported by targeted regression tests, with no schema or deployment impact.

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 →

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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: 80ef2fb3-fa30-4728-ab3d-82ab79d630a3

📥 Commits

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

📒 Files selected for processing (8)
  • apps/server/src/processRunner.test.ts
  • apps/server/src/processRunner.ts
  • apps/server/src/processSpawnError.ts
  • apps/server/src/sourceControl/GitHubCli.test.ts
  • apps/server/src/sourceControl/GitHubCli.ts
  • apps/server/src/sourceControl/SourceControlProviderDiscovery.test.ts
  • apps/server/src/sourceControl/SourceControlProviderDiscovery.ts
  • apps/web/src/components/settings/SourceControlSettings.tsx

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

The server now identifies bad-CPU-type spawn errors and reports them as unavailable GitHub CLI probes with executable-specific details. Source-control discovery passes those details to auth status, and settings display them for unavailable items.

Changes

Spawn error conversion

Layer / File(s) Summary
Spawn error conversion
apps/server/src/processSpawnError.ts, apps/server/src/processRunner.ts, apps/server/src/processRunner.test.ts
New helpers identify spawn system errors and bad-CPU-type errors and format architecture details. Process execution converts recognized spawn errors to platform errors, preserves unrelated defects, and tests both cases.

GitHub CLI error classification

Layer / File(s) Summary
GitHub CLI error classification
apps/server/src/sourceControl/GitHubCli.ts, apps/server/src/sourceControl/GitHubCli.test.ts
Bad-CPU-type spawn errors classify the GitHub CLI as unavailable. The error detail names the executable and reports incompatible CPU architecture.

Discovery detail propagation

Layer / File(s) Summary
Discovery and settings detail propagation
apps/server/src/sourceControl/SourceControlProviderDiscovery.ts, apps/server/src/sourceControl/SourceControlProviderDiscovery.test.ts, apps/web/src/components/settings/SourceControlSettings.tsx
Discovery uses available spawn-error details for unavailable CLI and auth status. Settings display the discovery detail when present and otherwise use the installation hint.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: ⚪ Minimal · up to b4c95

No actionable merge-blocking issue is established. Incompatible GitHub CLI executables are reported as unavailable with useful diagnostics while other process requests remain usable. Merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b4c95

The fix keeps incompatible executables unavailable rather than treating them as authenticated or granting additional execution privileges. The diagnostic is displayed as plain text. No introduced security vulnerability was established, but production access enforcement and interruption cleanup were not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The new exposure is executable-path metadata in source-control diagnostics for the selected server environment. The inspected change does not use that metadata to execute another command, disclose credential values, or expand service authority.

Trust Boundaries and Controls

  • observed — The diagnostic reaches an existing RPC whose payload has no command input and whose authorization map declares orchestration-read scope. Text rendering prevents the diagnostic from becoming HTML in the inspected settings consumer. Effective production authorization enforcement remains unverified.

Resilience and Maintainability Implications

  • inferred — The newly recovered synchronous failure occurs before obtaining a child handle, so this tested transition has no returned process handle to strand. Normal execution retains the scoped runner boundary; concrete live-child finalizer behavior was not verified.
🚥 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 8 functions across 8 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: handling incompatible optional CLI executables on the server.
Description check ✅ Passed The description covers the problem, implementation, approved scope, verification results, limitations, and issue reference. It omits the template headings and UI screenshots, but these omissions are n…
Linked Issues check ✅ Passed The changes satisfy the coding requirements in [#14590]. processRunner converts recognized synchronous bad-architecture spawn failures into typed process errors and preserves unrelated defects. GitH…
Out of Scope Changes check ✅ Passed The changed server and web files support [#14590]. They handle spawn-error classification, preserve defect behavior, expose the failed executable and cause, keep discovery usable, and display the diag…
✨ 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.

@camptx

camptx commented Oct 1, 2026

Copy link
Copy Markdown
Author

Verified locally on Apple Silicon with the same Intel gh 2.97.0 binary in both instances, using isolated T3 state.

Before the fix, source-control discovery fails with spawn Unknown system error -86. The active config subscription fails and a subsequent request on the same connection fails. Settings shows the environment as disconnected.

Before: environment disconnected

With the fix, the environment stays connected and Git remains available. GitHub is marked unavailable with the resolved gh path, the incompatible CPU architecture error, and guidance to install a compatible version. A subsequent config request on the same connection succeeds.

After: environment connected with architecture diagnostic

This reproduction uses the real Intel binary, rather than an injected spawn error.

@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:40

Dismissing prior approval to re-evaluate b4c95d2

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 2, 2026
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 6, 2026 09:40

Dismissing prior approval to re-evaluate 4a09f4e

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one appears to have been created with an older model (gpt-6). If this change is really important, we recommend rebuilding the PR with a newer model if possible.

@maria-rcks maria-rcks closed this Oct 11, 2026
@camptx camptx changed the title fix(server): recover from incompatible optional CLI executables fix(server): explain incompatible CLI executables during discovery Oct 31, 2026
@camptx

camptx commented Oct 31, 2026 •

Copy link
Copy Markdown
Author

Rebuilt this PR with GPT-6.1 Sol (gpt-6.1-sol, high reasoning effort) through the Codex harness in T3 Code, following the request to use a newer model.

Commit 604c728d2cdebe06bbe0e03f96b8181b80780cd0 merges current main and replaces the superseded implementation with the remaining architecture diagnostic. The existing branch history is preserved. The rebuilt branch changes three files relative to the fetched main baseline and uses the existing installation-hint field, so no client or schema changes are required.

The new regression tests fail on main and pass with the fix for raw and platform-wrapped architecture errors. Focused process-runner and discovery tests passed. vp run check:changed bf8e1324391a570fbca298cfd0c97d39381666a3 --concurrency 2 passed formatting, lint, unused-code checks, generated GraphQL checks, all 18 affected-package typechecks, and eight adjacent tests.

This rebuild was checked on Linux. The original Apple Silicon captures remain historical evidence; I have not claimed a fresh macOS or native-client run. The description now states these limits and links the overlapping open implementation in #21606.

The rebuild is ready, but GitHub reports viewerCanReopen: false for camptx and rejects both GraphQL and REST reopen requests. Please could a maintainer reopen this PR for reconsideration? The fork branch is at the commit above; the closed PR snapshot still records the old head.

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:L 100-499 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.

[Bug]: Intel-only gh makes local Apple Silicon environment appear disconnected

3 participants