Skip to content

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

Closed
voltcrash wants to merge 4 commits into
pingdotgg:mainfrom
voltcrash:t3/fix-issue-17367
Closed

voltcrash wants to merge 4 commits into
pingdotgg:mainfrom
voltcrash:t3/fix-issue-17367

Conversation

@voltcrash

@voltcrash voltcrash commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A desktop-spawned backend can miss its bootstrap envelope and silently start in web mode without the desktop credential. On Linux, launching the real CLI with --bootstrap-fd 0 and either closing the pipe empty or delaying the envelope until 12 seconds reproduced a running backend that rejected the bootstrap credential with 401 invalid_credential.

Change

Require a decoded envelope whenever a bootstrap descriptor is explicitly supplied by flag or environment. Allow up to 30 seconds for input; propagate typed errors for unavailable input, EOF, read/decode failures and timeout before creating server state. Preserve ordinary web defaults when no bootstrap descriptor is supplied and existing platform-specific descriptor handling.

Scope and approval

Fixes https://git.cafe/pingdotgg/t3code/issues/17367
GitCafe: https://git.cafe/pingdotgg/t3code/issues/17367
Triaged bug via the automated issue flow. Existing contributor commits and implementation are preserved.

Verification

  • Re-ran the existing flag/env configuration regressions against main at 49e97d48f9: both fail because server state is created despite the unavailable descriptor.
  • At PR head d52b1cc7fb, all 33 focused bootstrap/config tests pass. Scoped lint, formatting and server typecheck pass.
  • Real Linux CLI: empty input, invalid flag/env descriptors and malformed input exit 1 with the expected typed error and no userdata. Valid desktop input delivered at 12 seconds reaches readiness and exchanges its credential with HTTP 200. Ordinary no-fd web startup reaches readiness.
  • Withheld input logs the 30-second timeout error without creating userdata. The harness terminated that process after its observation deadline; natural timeout process exit was not established.
  • Visual fixture matrix is empty: no render paths changed. Runtime transcripts and red/green test output:

17367-runtime-evidence.txt
17367-red-test.txt
17367-green-test.txt

Native Windows/Electron post-update startup, WSL and native macOS were not verified. The Linux injection reproduces the missing-envelope failure mode, not the original nondeterministic Windows update trigger.

Implementation and fresh verification: GPT-6.1-Sol, Codex harness in T3 Code. Existing implementation and contributor commits preserved.

Fixes #17367

@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: Needs Human Approval

Macroscope's review found this PR requires human approval — Bootstrap-configured launches now require an envelope and wait up to 30 seconds rather than 1 second before timing out. This changes the default startup wait and replaces fallback startup with failure, requiring human confirmation of the intended startup behavior.

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

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

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, stream-open, stat, decode, timeout, missing-envelope, and read failures, plus 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, and verify that the server home directory is not created.

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · previous review 37dad

No merge-blocking issue is established. Native Windows startup remains unverified, but that alone does not indicate a failure.

Security architecture risk: Minimal · up to aa7dc

The change strengthens startup authentication guarantees: missing or invalid bootstrap input now stops startup instead of silently selecting web defaults. The reviewed changes introduce no material security concern.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The material boundary is a backend process consuming locally supplied bootstrap input and turning it into server configuration. Failure now prevents that process from proceeding without its requested envelope; the changed test helpers add no independently reachable production entrypoint.

Trust Boundaries and Controls

  • observed — A supplied descriptor must pass stat/open handling and schema decoding before configuration proceeds. Missing input, read failure, malformed input, and timeout produce failures rather than credential-free fallback. Configuration tests assert that unavailable flag and environment descriptors leave the isolated server home absent.

Resilience and Maintainability Implications

  • inferred — The one-shot line listener, terminal cleanup, and cancellation finalizer establish first-result ownership and prevent continued envelope consumption after completion. Delayed input is accepted within the bounded wait without making timeout a successful startup outcome. There is no dedicated repeated-delivery test, so that conclusion rests on the control flow.
Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title identifies the main change: require a bootstrap envelope before startup.
Linked Issues check Passed [#17367] The server requires an envelope for an explicit flag or environment descriptor. It waits up to 30 seconds and reports typed errors instead of starting in web mode. Without a descriptor, web d…
Out of Scope Changes check Passed [#17367] The four changed server files implement the required bootstrap behavior and its tests. The typed errors, cleanup, and configuration changes support that objective. The summary shows no unrela…
Description check Passed The description includes all required sections. It explains the problem and change, links the triaged issue, reports focused tests and runtime results, and identifies unverified platforms and timeout …
Full details: Linked Issues check

Explanation

[#17367] The server requires an envelope for an explicit flag or environment descriptor. It waits up to 30 seconds and reports typed errors instead of starting in web mode. Without a descriptor, web defaults remain. Bootstrap and configuration tests cover delayed input, timeout, missing input, read errors, and unavailable descriptors. The reported test run predates the reviewed head. Natural process exit after timeout and native Windows behavior were not verified; the issue does not require those manual checks.

Full details: Out of Scope Changes check

Explanation

[#17367] The four changed server files implement the required bootstrap behavior and its tests. The typed errors, cleanup, and configuration changes support that objective. The summary shows no unrelated changes.

Full details: Description check

Explanation

The description includes all required sections. It explains the problem and change, links the triaged issue, reports focused tests and runtime results, and identifies unverified platforms and timeout behavior. It also names the model and harness used.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 0ab0db6 and 37dad3c.

Files selected for processing: 3

  • 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.

@juliusmarminge juliusmarminge mentioned this pull request Oct 10, 2026
2 tasks done
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 11, 2026 — with ChatGPT Codex Connector
@juliusmarminge juliusmarminge self-assigned this Oct 19, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one as maintainer for batch server-persistence-02. I will preserve the contributor commits, merge current main, reduce the tests to focused startup behavior, and run independent implementation and adversarial reviews. No merge is planned.

@juliusmarminge juliusmarminge added the via-triage Filed through npx t3 triage label Oct 22, 2026 — with GitCafe Production

Copy link
Copy Markdown
Member

Fresh review found no implementation defects, but current main conflicts in the two test files. Prepared a merge of main that preserves contributor commits and the bootstrap regressions in main’s trimmed test layout: 8c46a3d392, pushed to origin branch fix/issue-17367. The existing cross-fork head is unchanged: cafe checkout cannot resolve its source repository. The prepared branch needs to be applied to the original PR source before landing. Native Windows/Electron startup remains unverified.

@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:needs-julius Needs Julius: route gate, unverifiable surface, or blocked and removed via-triage Filed through npx t3 triage labels Oct 22, 2026
@voltcrash
voltcrash force-pushed the t3/fix-issue-17367 branch 3 times, most recently from 70c9855 to 0ab0db6 Compare November 1, 2026 06:39
@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Nov 11, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Evidence

Evidence: real built Linux server CLI, fresh isolated homes and synthetic credentials. The same driver runs empty input, a 12-second delayed envelope, held-open timeout, unavailable fd 57, nonblocking fd 9, directory read failure, and an ordinary web launch. Recordings show a live CLI observation dashboard, not Electron. Transcripts are unchanged driver output, retaining sessionCookieName and traceId.

Before After
a03d9bce02 4946281074
Recording · Transcript Recording · Transcript
Empty input starts with loopback-browser authentication; synthetic desktop credential returns HTTP 401 invalid_credential. Empty input reports BootstrapEnvelopeMissingError and exits 1 in 0.4 seconds without creating server state.
Delayed-envelope case becomes ready at 2.6 seconds, before delivery; credential exchange returns HTTP 401. Waits for delivery at 12.1 seconds, becomes ready at 12.8 seconds with desktop-managed-local authentication; credential exchange returns HTTP 200 authenticated=true.
Held-open stdin starts without the credential and remains alive through the 38-second observation window. BootstrapEnvelopeTimeoutError; naturally exits 1 at 30.4 seconds while the writer remains open; no server state created.
Unavailable fd 57 starts with web authentication and rejects the credential. BootstrapFdStatError; exits 1 in 0.4 seconds without creating server state.
Nonblocking fd 9 starts with web authentication. Naturally exits 1 after a 30.4-second timeout while the writer remains open; no abort or server state.
Directory read failure exits 1. Reports BootstrapEnvelopeReadError and exits 1 in 0.4 seconds; no abort or server state.
Ordinary launch retains loopback-browser authentication. Ordinary launch retains loopback-browser authentication.

Reproduction and recording driver. Requires Pillow and ffmpeg; usage is documented in the script. AI-assisted; verified with the tests and tools listed above.

Not shown:

  • Native Windows/Electron update-and-restart behavior was not reproduced: this environment is Linux. The controlled real-server reproduction verifies bootstrap startup and authentication, not Windows supervisor recovery.
  • The previous ad-hoc fd 9 EAGAIN/SIGABRT conditions were not recreated exactly because its original descriptor setup was unspecified. A controlled nonblocking fd 9 now times out cleanly, and a real directory read failure exercises BootstrapEnvelopeReadError cleanup.
  • Windows and macOS descriptor-specific behavior, and selection through T3CODE_BOOTSTRAP_FD rather than the CLI flag, were not exercised.

@freddy-d

Copy link
Copy Markdown
Contributor

Tested this on native Windows 11 with the desktop app: it fixes the late-envelope case (12 s delay → /oauth/token 200), but when the envelope never arrives the backend logs BootstrapEnvelopeTimeoutError and then hangs instead of exiting, so the desktop never restarts it. Cause: Node hardwires Stats.isFIFO() to false on Windows, so the fd3 pipe falls back to fs.createReadStream and the pending threadpool read keeps the process alive. (The real-pipe test that covers this is skipIf(win32).)

Fix and full Windows verification, stacked on this branch: #34384. Only the top commit is new; it checks the S_IFIFO mode bits and runs the real-pipe test on Windows too. With it, the real Electron dev app self-heals in about 39 s (timeout → exit → respawn in desktop mode → login OK).

Via T3 Code triage with Claude Code (Claude Opus 5.5).

@t3dotgg

t3dotgg commented Nov 12, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Closing. This PR has been open for more than 3 weeks, and main has moved a long way since then. If the change still matters, open a fresh PR from current main.

@t3dotgg t3dotgg closed this Nov 12, 2026
@voltcrash
voltcrash deleted the t3/fix-issue-17367 branch November 12, 2026 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews pr:needs-julius Needs Julius: route gate, unverifiable surface, or blocked size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

4 participants