Skip to content

chore(server): keep the legacy service entry point to the npm package only - #11770

Merged
juliusmarminge merged 1 commit into
mainfrom
chore/trim-legacy-entry
Sep 14, 2026
Merged

chore(server): keep the legacy service entry point to the npm package only#11770
juliusmarminge merged 1 commit into
mainfrom
chore/trim-legacy-entry

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 14, 2026

Copy link
Copy Markdown
Member

What changes

Follow-up to #11732, which fixed a real regression: a boot-service launcher installed before the executable runs node <versions>/<v>/node_modules/t3/dist/bin.mjs, and #11607 had removed that file from the t3 npm package, so every fleet update from a pre-1700 nightly failed after a successful install.

That PR fixed it in two places. This keeps one and removes the other:

  • Kept: dist/bin.mjs in the t3 npm package, a forwarder to the platform executable that preserves arguments, stdio, the launcher's IPC channel, signals, and exit status. This is the transition an old launcher actually goes through: it npm-installs the new t3, runs the old path, the executable starts, and the first server started this way rewrites the service unit to run the executable directly. After that nothing touches the file. The doc comment now says when to delete it (after the first stable release with the executable).
  • Removed: ensureLegacyEntry in pinnedRuntime.ts, which wrote the same shim into every archive install, including cached ones on every status check, so an old launcher could start an archive-layout version through the old path. An old launcher never installs an archive (it predates archive installs), so that case does not occur, and it left a permanent fake node_modules/t3/dist in the runtime layout plus a write to cached installs on read. The "archive" variant of the script and its two tests go with it.

Net: 25 insertions, 92 deletions; legacyCliLauncherScript has no parameter now.

Verification

  • legacyCliLauncher.test.ts (IPC, args, termination through the npm shim), pinnedRuntime.test.ts, and build-npm-platform-packages.test.ts (which exercises dist/bin.mjs from the built tarball): 10 pass.
  • Typecheck clean for apps/server, packages/shared, and scripts; knip clean.

Claude Fable 5 via Claude Code.


Devin Review

Summary by CodeRabbit

  • Changes
    • Simplified runtime installation and validation for greater consistency across published runtimes.
    • Updated the command-line launcher to use platform-specific packages directly.
    • Removed support for launching legacy archive-based runtimes and related compatibility behavior.
    • Updated launcher documentation to clarify its compatibility role and planned future removal.

… only

#11732 restored dist/bin.mjs in the t3 npm package so a boot-service
launcher from before the executable can start a version it just installed.
It also wrote the same shim into every archive install, cached ones
included, so an old launcher could start an archive-layout version through
the old path. That case never occurs: an old launcher only ever installs
through npm, and the first server it starts rewrites the unit to the
executable, after which nothing runs the old path. The archive write meant a
permanent fake node_modules/t3 in the runtime layout and a rewrite of cached
installs on every check. Only the npm shim remains, with a note on when to
remove it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 14, 2026
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 77be07d1-ed93-4b53-9275-b8f8409c4145

📥 Commits

Reviewing files that changed from the base of the PR and between 9130d93 and 7ac4061.

📒 Files selected for processing (5)
  • apps/server/src/cloud/pinnedRuntime.test.ts
  • apps/server/src/cloud/pinnedRuntime.ts
  • packages/shared/src/legacyCliLauncher.test.ts
  • packages/shared/src/legacyCliLauncher.ts
  • scripts/build-npm-platform-packages.ts
💤 Files with no reviewable changes (1)
  • apps/server/src/cloud/pinnedRuntime.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The compatibility launcher now resolves executables from platform npm packages only. Pinned runtime installation and publication no longer create legacy entry points before validation. Tests were updated to remove archive-launch and cached-archive coverage.

Changes

Launcher and pinned runtime changes

Layer / File(s) Summary
Platform launcher contract
packages/shared/src/legacyCliLauncher.ts, packages/shared/src/legacyCliLauncher.test.ts, scripts/build-npm-platform-packages.ts
legacyCliLauncherScript() no longer accepts a distribution. It always resolves the platform executable from the sibling npm package. Tests and package generation use the updated call.
Pinned runtime validation
apps/server/src/cloud/pinnedRuntime.ts, apps/server/src/cloud/pinnedRuntime.test.ts
Pinned runtime installation and concurrent publication validate runtimes without generating legacy npm service entry points. Tests for archive launching and cached-archive reuse were removed.

Priority: ⬇️ Low

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

Change: Refactor

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 7ac40

The compatibility launcher and pinned-runtime validation changes are ready to merge.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: retaining the legacy service entry point only in the npm package.
Description check ✅ Passed The description explains what changed, why the archive shim was removed, the compatibility behavior that remains, and the verification performed. It does not use the exact template headings for Why or…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/trim-legacy-entry

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

@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 13.6 KiB 13.5 KiB −5 B (−0.0%) 15.1 KiB
Codex Thread snapshot wire 7.0 KiB 7.0 KiB +2 B (+0.0%) 7.3 KiB
Codex Live turn WebSocket wire 6.5 KiB 6.5 KiB −7 B (−0.1%) 7.8 KiB
Codex Live turn WebSocket decoded 57.0 KiB 57.0 KiB 0 B (0.0%) 66.4 KiB
Codex Live turn messages 8 8 0 (0.0%) 21
Claude Total thread wire 13.6 KiB 13.6 KiB −6 B (−0.0%) 15.1 KiB
Claude Thread snapshot wire 7.0 KiB 7.1 KiB +6 B (+0.1%) 7.3 KiB
Claude Live turn WebSocket wire 6.5 KiB 6.5 KiB −12 B (−0.2%) 7.8 KiB
Claude Live turn WebSocket decoded 57.8 KiB 57.8 KiB 0 B (0.0%) 66.4 KiB
Claude Live turn messages 9 9 0 (0.0%) 21

Baseline: 9130d93 · PR result: 7ac4061 · 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: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

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

@macroscopeapp

macroscopeapp Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 7ac4061

Macroscope's review found this PR approvable — This is a focused compatibility cleanup that retains and tests the legacy npm launcher while removing unused archive-layout shim generation and cached-runtime writes. Existing service startup remains on the pinned executable path, with no schema, security, billing, or deployment changes.

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

@juliusmarminge
juliusmarminge merged commit f328db3 into main Sep 14, 2026
21 checks passed
@juliusmarminge
juliusmarminge deleted the chore/trim-legacy-entry branch September 14, 2026 18:21
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: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.

1 participant