Skip to content

fix(server): require the bootstrap envelope before startup - #17568

Open
voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:t3/fix-issue-17367
Open

voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:t3/fix-issue-17367

Conversation

@voltcrash

Copy link
Copy Markdown
Contributor

Problem

A desktop backend launched with --bootstrap-fd can miss its bootstrap envelope and silently start in web mode without the desktop credential. The desktop then receives invalid_credential and cannot recover through the renderer's retry controls.

Fixes #17367.

Change

Require a decoded envelope whenever --bootstrap-fd or T3CODE_BOOTSTRAP_FD is supplied. Give the read a bounded 30-second deadline instead of one second. An unavailable fd, empty input, read/decode failure, or timeout now fails startup with a typed error, so the existing desktop supervisor can restart the backend. Release the reader on completion and interruption, and handle errors forwarded by readline.

The shared server startup path covers the primary desktop backend's fd 3 and the WSL backend's stdin. Launches without a bootstrap fd retain their existing defaults. No client UI, provider adapter, or wire-contract changes are needed; local, remote, and tunnel clients all benefit from the backend starting with its intended configuration.

Scope and approval

This fixes the single bootstrap-loss defect established in maintainer triage, using the suggested server-side failure and longer-wait directions. It does not change the error-screen recovery flow or the separate WSL port-collision issue.

Verification

  • Added config regressions for both the CLI flag and environment variable. Before the fix, both tests returned a web-mode config without a desktop credential; after the fix, both fail before creating the server home.
  • vp test run apps/server/src/bootstrap.test.ts apps/server/src/cli/config.test.ts — all 32 tests passed. A controlled Node stream and virtual clock verify delivery after the former one-second deadline, the bounded timeout, EOF without an envelope, read errors, and interruption cleanup. Existing real-fd reads and config precedence checks also pass.
  • Direct CLI checks on macOS: an invalid explicit fd exits with code 1 and BootstrapFdStatError; empty --bootstrap-fd 0 input exits with code 1 and BootstrapEnvelopeMissingError. Neither launch creates its isolated server home.
  • Scoped lint and formatting checks on the four changed files, server-only typecheck (vp run --filter t3 typecheck), server-only export check (vp exec knip --workspace apps/server --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints), and git diff --check passed.
  • The nondeterministic Windows post-update cold start was not reproduced on this macOS host. The suspected Defender/threadpool trigger remains unconfirmed. This is a backend-only change, so UI screenshots are not applicable.

Implemented with gpt-6.1-sol (xhigh) 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 9, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The change alters the production startup contract: supplied bootstrap descriptors now must produce a valid envelope, and failures prevent server initialization instead of falling back. It also changes the default bootstrap wait from 1 second to 30 seconds, making this a product-default and runtime behavior change.

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

@coderabbitai

coderabbitai Bot commented Oct 9, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9d7dab9d-3383-464f-be0f-749ab6eacf61

📥 Commits

Reviewing files that changed from the base of the PR and between 6497246 and 6c080b3.


📒 Files selected for processing (4)
  • apps/server/src/bootstrap.test.ts
  • apps/server/src/bootstrap.ts
  • apps/server/src/cli/config.test.ts
  • apps/server/src/cli/config.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

Bootstrap envelope reads now return decoded values or typed errors instead of optional values. The default timeout is 30 seconds. Server configuration uses the decoded envelope directly when a descriptor is provided.

Changes

Bootstrap envelope startup handling

Layer / File(s) Summary
Envelope read contract and stream lifecycle
apps/server/src/bootstrap.ts, apps/server/src/bootstrap.test.ts
Reads fail with typed errors when the descriptor is unavailable, the stream closes without an envelope, or the read times out. The default timeout changes to 30 seconds. Tests cover successful reads, errors, timeout, and stream cleanup.
Configuration loading
apps/server/src/cli/config.ts, apps/server/src/cli/config.test.ts
Configuration assigns the decoded envelope directly when a descriptor is provided. Tests cover unavailable descriptors from the CLI flag and environment variable.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge


Merge Risk

Merge Risk: ⚪ Minimal · up to 6c080

No actionable startup issue remains identified; the change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6c080

Missing bootstrap input now stops startup instead of allowing unintended defaults. The change remains confined to local process startup and does not expand remote access or credential authority. No introduced security concern was established; remaining uncertainty concerns asynchronous cleanup and descriptor ownership.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed authority boundary is the parent-to-backend startup handoff. Controlling this input requires control over process launch configuration or its inherited descriptor; the inspected call chain provides no remote request path to the reader. The 30-second wait occurs before listening.

Trust Boundaries and Controls

  • observed — The desktop remains the owner of bootstrap identity generation. The server validates the supplied envelope before consuming its identity fields. Ordinary local configuration precedence remains unchanged, and neither timeout nor supervisor restart bypasses envelope validation when a descriptor is supplied.

Resilience and Maintainability Implications

  • observed — The POSIX path can reopen the supplied descriptor and destroy the resulting stream without closing the original descriptor. This ownership arrangement predates the PR and is not an introduced concern. Production ownership of the original descriptor and dependency-level competing-event settlement were not independently established.



Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the main change: requiring the bootstrap envelope before server startup.
Description check Passed The description includes all required sections. It explains the problem, change, scope and approval, focused verification results, limitations, and the agent and harness used.
Linked Issues check Passed The PR addresses the coding requirements in #17367. resolveServerConfig loads an envelope when --bootstrap-fd or T3CODE_BOOTSTRAP_FD supplies a descriptor, and it fails before server-directory c…
Out of Scope Changes check Passed The changed files are limited to shared bootstrap reading, server configuration, and related tests. These changes directly support #17367. They do not modify the client UI, provider adapters, wire con…


✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@juliusmarminge juliusmarminge mentioned this pull request Oct 10, 2026
2 tasks done

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

1 participant