Skip to content

fix(web): update everything updates remote servers again - #18931

Closed
t3dotgg wants to merge 1 commit into
mainfrom
t3/fix-update-all-remote-envs
Closed

t3dotgg wants to merge 1 commit into
mainfrom
t3/fix-update-all-remote-envs

Conversation

@t3dotgg

@t3dotgg t3dotgg commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

The sidebar "update everything" button skipped every remote server. It updated providers and this app, but no remote T3 Connect server got an update request.

Before it offers a server that installs from npm, the sidebar asks npm whether that version is published. Effect's HttpClient adds b3 and traceparent headers by default. In a browser, those headers force a CORS preflight. npm answers the preflight with a 404 and no CORS headers, so the check always failed and every npm-installed server was dropped. Settings "Update all" does not run this check, so it still worked.

The fix turns off trace propagation for this one request. A test runs the check against a fake fetch that fails like a browser when trace headers are present.

Reviewed with sol-loop: 1 round with GPT-6.1-Sol.

Created with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

Closes #18690

Effect's HttpClient adds b3 and traceparent headers, which turn the npm
publish check into a CORS preflight that registry.npmjs.org rejects. The
check always failed, so the sidebar run dropped every server that
installs from npm.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 13, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 13, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at a92eb11

Macroscope's review found this PR approvable — This is a focused fix to an existing npm publication check: disabling trace headers prevents the browser CORS preflight failure, allowing the established update-all flow to include npm-installed remote servers again. The added test covers the request-header behavior, and no defaults, schemas, infrastructure, or static-analysis suppressions are changed.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

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

@coderabbitai

coderabbitai Bot commented Oct 13, 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: Team
  • Run ID: 56c6e877-f858-4704-9e1c-574c61fdd19f
📥 Commits

Reviewing files that changed from the base of the PR and between e9221e4 and a92eb11.

📒 Files selected for processing (2)
  • apps/web/src/state/npmRelease.test.ts
  • apps/web/src/state/npmRelease.ts

Included review availability: This review used your included allowance. 9 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.

📝 Walkthrough

The npm registry check is now an exported Effect. The release atom uses the helper and provides a fetch client. The check disables tracer propagation and retains its 404 and version-match behavior.

Changes: npm release check

Layer / File(s) Summary
Registry check and atom integration
apps/web/src/state/npmRelease.ts, apps/web/src/state/npmRelease.test.ts
The exported Effect checks the registry response against the requested version and disables tracer propagation. The atom delegates to the helper and provides a fetch client. The test mocks fetch and checks that requests without trace headers return the matching version.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a92eb

No actionable merge-blocking risk is identified; the change is ready for normal checks.

Pre-merge checks · Passed 4
Passed Passed checks (4 passed)
Check name Status Explanation
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.
Description check Passed The description explains the problem, identifies the cause, describes the fix, references issue #18690, and documents the test approach. It does not use the template headings or state the observed tes…
Title check Passed The title clearly and concisely describes the primary fix: restoring remote server updates in the web app's "update everything" flow. It also follows the repository's conventional commit style.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • 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 added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 13, 2026
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 13, 2026 05:07

Dismissing prior approval to re-evaluate a92eb11

@juliusmarminge

Copy link
Copy Markdown
Member

Note

Grok responding on behalf of Julius.

Superseded by #18709, which already landed on main and closed #18690.

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: e9221e4 · PR result: a92eb11 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

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:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Sidebar "update everything" never updates CLI/npm servers (npm check blocked by CORS preflight)

2 participants