Drop juspay/xterm.js fork — both fixes are upstream#979
Conversation
Both fork fixes are merged upstream:
- #5817 (CursorBlinkStateManager dispose leak) — merged 2026-04-21
- #5821 (WeakRef variant) — closed, superseded by #5831
(observer-nullify on dispose), merged 2026-05-26
Swap pnpm.overrides:
@xterm/xterm github:juspay/xterm.js#fix/kolu-xterm-fixes-built
-> 6.1.0-beta.225
@xterm/addon-webgl same fork URL + path:/addons/addon-webgl
-> 0.20.0-beta.224
Both betas are built from xtermjs/xterm.js master commit afaed108,
so this is "build off master" without the fork's prebuilt-lib detour.
Address hickey finding F1: the published blog post still showed the juspay/xterm.js fork github URL as the present-tense consumption mechanism. Update to reflect the override now pinning npm betas built from xtermjs/xterm.js master (#5817 merged, #5821 superseded by #5831).
pnpm-lock.yaml regenerated when overrides moved from the juspay/xterm.js fork URL to npm betas. The fetchPnpmDeps pre-image follows.
Hickey/Lowy Analysis
Hickey rationale
Lowy rationale
|
EvidenceThis PR has no Kolu-side code change — it swaps the The empirical evidence is
These suites drive a real Kolu binary in Chromium and exercise the surfaces most sensitive to an xterm swap: terminal rendering (themes, theme shuffle, theme per terminal), command palette, clipboard, file-drop into the terminal, kitty keyboard protocol, image addon, and dock/workspace switching. All scenarios passed against |
|
| Step | Status | Duration | Verification |
|---|---|---|---|
| sync | ✓ | 1s | git fetch ok; forge=github; noGit=false |
| research | ✓ | 3m 53s | Confirmed both fork patches landed upstream (#5817 + #5831 supersedes #5821); current upstream master shipped as @xterm/xterm@6.1.0-beta.225 and @xterm/addon-webgl@0.20.0-beta.224. |
| branch | ✓ | 8s | On feature branch chore/xterm-upstream-master, created from origin/master. |
| implement | ✓ | 41s | Two pnpm.overrides lines swapped fork URLs for pinned betas; pnpm install refreshed the lockfile. |
| check | ✓ | 49s | just check exit 0 — pnpm typecheck clean across all 22 workspaces. |
| docs | ✓ | 19s | README + packages/surface/README.md reference xterm architecturally — no fork/version mentions to update. |
| fmt | ✓ | 10s | just fmt clean — no rewrites. |
| commit | ✓ | 25s | Primary commit 5f07cf1a pushed; branch tracking origin. |
| hickey+lowy | ✓ | 2m 47s | Hickey F1: stale fork URL in xtermjs-perf.md → fixed as 25dfb790. Lowy: No-op (override receptacle is the correct boundary). |
| police | — | 1m 9s | Skipped — diff is config + docs only; no JS/TS code patterns for /code-police to review. |
| test | ✓ | 1m 1s | just test-unit: 469 passed across opencode, codex, git, claude-code, client, server. |
| create-pr | ✓ | 1m 58s | Draft PR #979 + Hickey/Lowy analysis comment. |
| ci | ✓ | 7m 59s | All 26 checks green on HEAD e536cf84. First run flaked ci::unit@aarch64-darwin (SIGKILL in 4s before tests ran); single-node retry passed cleanly. |
| evidence | ✓ | 51s | Evidence comment ties to ci::e2e on both platforms — terminal rendering exercised live against the new xterm. |
| done | ✓ | 0s | — |
| Total | 22m 35s |
Slowest step: ci (7m 59s, 35% of total)
Optimization suggestions
cidominated and got doubled by a darwin flake.ci::unit@aarch64-darwinSIGKILLed in 4s on the first attempt with onlypackages/integrations/io test:unit: Failedin the log — process tree torn down before any test produced output. If this is a recurring sincereintent shape it might be worth raising as its own host-health issue; for now the retry pattern (justci run ci::unit@aarch64-darwin) closes the status without re-running the whole pipeline. For follow-up pushes on this branch,/do --from ci-onlyskips straight to CI.researchwas the second-longest step (3m 53s) — most of it was walking thejuspay/xterm.jsPR timeline / upstreamxtermjs/xterm.jsPR cross-references. Future PRs that bump xterm could shorten this by linking the upstream PR (e.g.Refs xtermjs/xterm.js#5831) directly in the task description.hickey+lowy(2m 47s) is at the floor for parallel sub-agent spin-up — both ran in parallel; only one structural finding emerged (the stale blog snippet), which is the kind of doc-coherence issue that's easier to fix once with hickey than to remember to update by hand.
Workflow completed at $(date -u +%Y-%m-%dT%H:%M:%SZ).
Perf measurement: PR vs
|
| Bucket | PR (upstream) | master (fork) |
Δ (master − PR) |
|---|---|---|---|
| Total Δ heap | +1,940,607 B | +1,970,490 B | +29,883 (1.5%) |
| code (V8 JIT) | +1,357,416 B | +1,364,420 B | +7,004 (0.5%) |
| native | +54,375 B | +81,730 B | +27,355 (50%) |
| array | +328,388 B | +327,600 B | −788 (−0.2%) |
| closure | +58,116 B | +58,280 B | +164 (0.3%) |
| object | +77,188 B | +75,384 B | −1,804 (−2.3%) |
JSArrayBufferData |
+31 / +49,568 B | +31 / +49,568 B | 0 (identical) |
| 30-toggle wall | 5,724 ms | 5,754 ms | +30 (0.5%) |
The JSArrayBufferData row is the diagnostic one — those are xterm BufferLine Uint32Array backings, the exact retainer chain the dispose-leak fixes target. Both arms allocate +31 fresh ArrayBuffers per 30 toggles (one per dispose cycle) and release them before the next iteration. WeakRef and observer-nullify drop the load equally when measured against the canonical reproducer.
N=1 was misleading — methodology note
The first iteration showed master growing ~2.6× more than PR (+1,653 KB vs +623 KB). That gap turned out to be a confounding warmup artifact: the PR arm had a 5-toggle dry-run during methodology validation, so its baseline snapshot was already past the V8 JIT churn (FeedbackVector / LoadHandler / code:* allocations) that the master arm absorbed during its measured 30 toggles. Reloading both arms cold and re-measuring (N=2 above) collapsed the gap to ~30 KB.
The lesson is the perf-diagnose skill's existing one: facts over guesses, and one snapshot isn't a fact yet.
What this measurement doesn't cover
The 30× canvas-maximize toggle exercises the mount/dispose lifecycle, not the parse/render hot loop. The headline upstream-master perf work — #5825 CSI fast-path (+42% throughput), #5864 BufferLine translateToString cache, #5902 decoration-at-cell cache, #5867 const-enum conversions — all live in code paths a static-toggle test never hits.
To measure those, the right reproducer is a heavy-stdout stream (multi-MB cat large.log, a find /nix/store-class dump, or yes | head -c 100M) with a performance trace capturing parser time per byte. That's a separate measurement — not in scope for this PR's perf signoff, but worth filing if the upstream claims need empirical confirmation downstream.
Conclusion
Memory-wise, the swap is safe to ship. No regression on the canonical dispose-leak reproducer; the upstream observer-nullify behaves identically to the fork's WeakRef. Toggle wall-clock is unchanged (≈5.7 s / 30 toggles on both). The wider parse-throughput / render-hotpath claims attached to the 138-commit upstream delta would land or fail under a different reproducer this PR doesn't unblock or block.
1 participant
Both Kolu-maintained xterm.js patches landed upstream, so
pnpm.overridesno longer needs to point at thejuspay/xterm.jsfork. The override now pins the per-commit npm betas auto-published fromxtermjs/xterm.js@master, giving us a release channel that is byte-for-byte the unreleased upstream master.Patch status
CursorBlinkStateManager+_pausedResizeTaskregistrationWeakRefinIntersectionObserver(variant A)Both fixes ship in upstream master commit
afaed108, which is the source of@xterm/xterm@6.1.0-beta.225and@xterm/addon-webgl@0.20.0-beta.224.pnpm.overridesdiffThe two packages are co-versioned upstream (
addon-webgl@beta.224's peerDep is@xterm/xterm: ^6.1.0-beta.225), so they always move in lockstep. When xterm 6.1.0 stable releases, the override collapses to a plain^6.1.0declaration and thesepnpm.overridesentries can be dropped entirely.Side updates
website/src/content/blog/xtermjs-perf.mdshowed the old fork URL in present tense — now describes the move to upstream betas.default.nixpnpmDeps.hashrefreshed to match the regeneratedpnpm-lock.yaml.Try it locally
Generated by
/doon Claude Code (modelclaude-opus-4-7).