Skip to content

perf: skip DTS for local launcher rebuilds#85142

Merged
RomneyDa merged 1 commit into
mainfrom
long-tui-build-times
May 22, 2026
Merged

perf: skip DTS for local launcher rebuilds#85142
RomneyDa merged 1 commit into
mainfrom
long-tui-build-times

Conversation

@RomneyDa
Copy link
Copy Markdown
Member

Summary

  • Skip DTS generation only for the tsdown rebuild launched by scripts/run-node.mjs, which is the source-checkout launcher behind commands like pnpm tui and pnpm openclaw.
  • Keep production/package builds on the existing path: pnpm build, build:docker, build:strict-smoke, release/prepack, and CI build artifact jobs call scripts/build-all.mjs or scripts/tsdown-build.mjs directly and do not set this local-launcher flag.
  • Cover the boundary in run-node tests so bundled plugin asset builds and the final CLI command do not inherit the DTS-skip env var.

Build Timing

Measured on this worktree with the same tsdown rebuild step:

  • Before: /usr/bin/time -p node scripts/tsdown-build.mjs --no-clean aborted with V8 OOM after real 57.68s at about a 6 GB heap.
  • After: /usr/bin/time -p env OPENCLAW_RUN_NODE_SKIP_DTS_BUILD=1 node scripts/tsdown-build.mjs --no-clean completed in real 2.62s.

Production Boundary

This does not change shipped or production build outputs. The npm package bin is openclaw.mjs, scripts/run-node.mjs is not included in the package files, and package/release build scripts use scripts/build-all.mjs or scripts/tsdown-build.mjs directly without OPENCLAW_RUN_NODE_SKIP_DTS_BUILD.

Verification

  • node scripts/run-vitest.mjs src/infra/run-node.test.ts - 61 passed
  • git diff --check
  • Inspected package scripts, release scripts, CI workflow build calls, and package files/bin entries for scripts/run-node.mjs production usage

@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts size: S maintainer Maintainer-authored PR labels May 22, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 22, 2026

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The PR adds an internal run-node env flag for the tsdown child build, disables tsdown DTS generation under that flag, and adds a run-node env-scoping regression test.

Reproducibility: not applicable. this is a scripts performance PR rather than a bug report. The changed boundary is clear from source inspection, and the PR body includes terminal timing evidence for the tsdown rebuild path.

PR rating
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Summary: Focused scripts patch with clear boundary coverage and no blocking findings from source review.

Rank-up moves:

  • none
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Real behavior proof
Not applicable: The external contributor proof gate does not apply to this maintainer-labeled member PR, though the body includes terminal timing evidence and targeted test output.

Risk before merge

  • This read-only review did not rerun the reported run-node test or timing build; merge should still rely on normal CI or maintainer validation for execution proof.

Maintainer options:

  1. Decide the mitigation before merge
    Merge after routine maintainer and CI validation, keeping DTS generation skipped only for local launcher rebuilds while package, release, and direct build paths retain declaration output.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No automated repair is needed; the remaining action is ordinary maintainer and CI merge validation for a protected maintainer-labeled PR.

Security
Cleared: The diff only changes local build-script env scoping, tsdown config, and a regression test; it does not add dependencies, workflows, permissions, or secret handling.

Review details

Best possible solution:

Merge after routine maintainer and CI validation, keeping DTS generation skipped only for local launcher rebuilds while package, release, and direct build paths retain declaration output.

Do we have a high-confidence way to reproduce the issue?

Not applicable; this is a scripts performance PR rather than a bug report. The changed boundary is clear from source inspection, and the PR body includes terminal timing evidence for the tsdown rebuild path.

Is this the best way to solve the issue?

Yes; scoping the env override to the launcher-triggered tsdown child is the narrow maintainable path, and direct build/package/release callers remain outside that injected environment.

Label justifications:

  • P3: This is a low-risk scripts performance and local developer ergonomics improvement with limited runtime blast radius.
  • rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🐚 platinum hermit, patch quality is 🐚 platinum hermit, and Focused scripts patch with clear boundary coverage and no blocking findings from source review.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external contributor proof gate does not apply to this maintainer-labeled member PR, though the body includes terminal timing evidence and targeted test output.

What I checked:

  • Live PR state: The live PR is open, non-draft, mergeable, at head 08932d6, authored by a MEMBER account, and labeled maintainer. (08932d6b63dc)
  • Diff scope: The patch sets OPENCLAW_RUN_NODE_SKIP_DTS_BUILD only in a copied env for the scripts/tsdown-build.mjs --no-clean spawn, leaving the asset build and final CLI command on the original env unless the user already set that variable externally. (scripts/run-node.mjs:1355, 08932d6b63dc)
  • DTS toggle: The tsdown config reads the new env flag and maps only the exact value 1 to dts: false; the default path still leaves the option unset. (tsdown.config.ts:335, 08932d6b63dc)
  • Production boundary: Current package.json exposes openclaw.mjs as the package bin, does not include scripts/run-node.mjs in package files, and production build scripts call scripts/build-all.mjs or scripts/tsdown-build.mjs directly. (package.json:16, 4faeb378ee49)
  • Build script path: Current scripts/build-all.mjs invokes scripts/tsdown-build.mjs as its own build step, so normal build-all callers do not inherit the launcher-only env injection from run-node. (scripts/build-all.mjs:13, 4faeb378ee49)
  • Dependency contract: The tsdown v0.22.0 docs state that declaration generation is enabled by default when package.json has types/typings, and can also be controlled by the dts config option, supporting the PR's explicit dts: false skip path.

Likely related people:

  • steipete: Local blame for the current run-node build path, tsdown config export, and run-node test scaffolding points to aef8d17, and remote history shows several recent run-node/test maintenance commits. (role: recent area contributor; confidence: high; commits: aef8d1771dc8, 12debcb05e60, e19f05b79b2e; files: scripts/run-node.mjs, src/infra/run-node.test.ts, tsdown.config.ts)
  • RomneyDa: Remote history for tsdown.config.ts shows recent merged work on rolldown-plugin-dts warning handling, directly adjacent to the DTS behavior this PR changes. (role: recent build/DTS contributor; confidence: high; commits: cd019cfa412b; files: tsdown.config.ts)
  • rubencu: Remote history for scripts/run-node.mjs includes a cluster of runtime output and rebuild freshness commits that are adjacent to the local launcher rebuild path. (role: adjacent runtime build contributor; confidence: medium; commits: 9ce359b370bc, 31f74259cbcb, 8a9f14294229; files: scripts/run-node.mjs)
  • vincentkoc: Remote history shows recent touches to src/infra/run-node.test.ts and scripts/run-node.mjs, making this a useful routing candidate for local launcher test behavior. (role: recent adjacent test/build contributor; confidence: medium; commits: 58e13518633f, 8bef5d0d622b; files: src/infra/run-node.test.ts, scripts/run-node.mjs)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4faeb378ee49.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels May 22, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 22, 2026

ClawSweeper PR egg

✨ Hatched: 🥚 common Gilded Review Wisp

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: finds missing screenshots.
Image traits: location review cove; accessory commit compass; palette sunrise gold and clean white; mood patient; pose stepping out of a freshly hatched shell; shell matte ceramic shell; lighting bright celebratory glints; background gentle dashboard dots.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Gilded Review Wisp in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@RomneyDa RomneyDa force-pushed the long-tui-build-times branch from c434de3 to 08932d6 Compare May 22, 2026 00:50
@RomneyDa RomneyDa merged commit d391434 into main May 22, 2026
99 checks passed
@RomneyDa RomneyDa deleted the long-tui-build-times branch May 22, 2026 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant