Skip to content

fix(updates): respect configured npm registries for version checks - #27812

Closed
svanvliet wants to merge 1 commit into
pingdotgg:mainfrom
svanvliet:fix/npm-registry-version-checks
Closed

svanvliet wants to merge 1 commit into
pingdotgg:mainfrom
svanvliet:fix/npm-registry-version-checks

Conversation

@svanvliet

Copy link
Copy Markdown
Contributor

Problem

Automatic provider-version and T3 release checks contact registry.npmjs.org directly instead of honoring the server's configured npm registry. In a managed environment this triggers network protection and prevents update discovery, even though npm itself is configured to use the corporate proxy.

Related: #24369. Overlaps the provider-lookup fix in #24372; this draft also addresses browser-side T3 release checks.

Change

  • Resolve npm-based provider versions through configured npm view --global, including native/manual installations and fallback after a failed installer-specific lookup. Preserve installer-authoritative results and existing provider behavior when npm is absent. A configured-npm lookup failure does not retry public npm.
  • Move T3 release metadata behind an authenticated, read-scoped server RPC. Preserve database-migration metadata, validate release specs, bound subprocess output/time, and share a one-minute cache across WebSocket connections.
  • Check publication against each target environment's registry. Advertise the RPC as a capability: older servers remain eligible for their existing updater instead of being permanently excluded by an unsupported precheck.
  • Surface safe lookup errors in track-switch and update planning, and document registry behavior.

Scope and approval

This addresses one underlying problem: release discovery bypassing configured npm registries. No maintainer approval of the broader scope or the compatibility tradeoff below is claimed. Submitting as a draft for feedback; draft status does not waive contribution requirements.

Unresolved behavior needing direction: T3 release-track switching currently requires npm on the primary server's PATH, including standalone/desktop installations. Missing npm now produces an actionable error, but this is still a functional regression from the old browser lookup. This is separate from provider-version resolution, which retains its existing npm-absent behavior.

Possible directions are a narrowly scoped public lookup when npm is genuinely absent, or publishing database-compatibility metadata with first-party release artifacts. This draft does not choose either approach or silently fall back after a configured-registry failure.

Verification

Using the already-installed partial toolchain:

  • Added regression cases first: the older-server update case and two missing-npm cases failed before their fixes.
  • Final focused suites: 20 web tests (release state, update planning, track-switch planning) and 34 server tests (release service and RPC authorization) passed.
  • Contracts, provider-core, and client-runtime typechecks passed; targeted formatting, lint, and git diff --check passed.
  • Corrected a missing exhaustive RPC instrumentation entry identified during review. A full server compiler rerun removed exactly its two diagnostics and added none; remaining server diagnostics persist. The web compiler's normalized diagnostics are unchanged from the prior dependency-blocked run.
  • Earlier focused checks passed for provider resolution (62 tests), shared client state/runtime (54 tests), and RPC contracts (2 tests).
  • Earlier controlled network probes passed 34 service cases plus a guard positive control, with real npm subprocesses against isolated registries. These did not observe public-registry attempts in the exercised service paths. The client probe used IPC and virtual time, not the full app; it predates the final capability/error/cache-wiring fixes.

Before merge, confirm all required checks run and pass in CI. Local full-app verification is blocked by dependency availability and quarantine in the corporate registry—the same environment that exposed this bug, not the same code defect. The locked electron@44.10.0 was confirmed quarantined until November 3, 2026, 10:06:18 AM Pacific.

vp run check:changed, complete builds/typechecks, and full-app verification have not passed. Actual WebSocket/multi-connection behavior, mounted UI/network behavior, genuine pnpm behavior, and Windows integration remain unverified. No integrated UI captures are available. Green CI would cover automated checks; integrated app/network verification and the npm-less compatibility decision remain outstanding.

Implemented with GPT-6 Astra in T3 Code through the acpRegistry harness. Static review by Claude Opus 5.5.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@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 30, 2026
t3dotgg added a commit that referenced this pull request Oct 30, 2026
Based on #27812 by Scott Van Vliet.

Co-Authored-By: Scott Van Vliet <782486+svanvliet@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@t3dotgg

t3dotgg commented Oct 30, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Superseded by #27847, which is merged. It keeps your design: npm lookups on the server, the server.getNpmRelease RPC, and the npmReleaseLookup capability. You are credited as co-author.

Changes made on top of this PR:

  • Fixed the CI failures: ws.test.ts was missing the new service, and the Codex driver tests ran the host's real npm.
  • Public npm is used only when npm is not installed. If npm is installed and fails, the check fails and does not fall back. This resolves the open question about desktop installs without npm.
  • npm view requests only version and t3code. The full t3 document is 76 KB and grows with each release, so it would soon pass the 256 KB output cap.
  • The "Update everything" npm precheck is removed. Servers that support the lookup update from release archives and check the release before they restart.
  • At most two npm lookups run at once.

Thank you for the fix and the clear write-up.

@t3dotgg t3dotgg closed this Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

2 participants