Skip to content

build: migrate to TypeScript 7 - #387

Merged
steipete merged 2 commits into
mainfrom
chore/typescript-7-migration
Aug 10, 2026
Merged

build: migrate to TypeScript 7#387
steipete merged 2 commits into
mainfrom
chore/typescript-7-migration

Conversation

@steipete

Copy link
Copy Markdown
Owner

Summary

  • replace the TypeScript 6/compiler-preview split with stable TypeScript 7.0.2 and its tsc binary across every workspace package
  • port the import-cycle gate from the removed TypeScript compiler API to oxc-parser, preserving static import, re-export, dynamic import, and import-type detection; add a deliberate two-file cycle fixture
  • refresh policy-eligible undici and patch image-size zero-length box/ICNS loops because upstream has no fixed release; add a timeout-isolated regression test
  • make the existing shared-slide cleanup fixture deterministic by establishing its durable cache reference before the 10 ms expiring row

Proof

  • pnpm -s build
  • pnpm -s check — 556 files passed, 3,015 tests passed, 29 files / 43 tests skipped
  • pnpm -C apps/chrome-extension test:chrome — 3 native/slides tests passed, then 111 passed / 8 skipped in the full Chromium suite
  • pnpm -C apps/chrome-extension build
  • scripts/release.sh verify — packed core and CLI, installed both tarballs, ran CLI help, and extracted one slide through the FFmpeg WebAssembly fallback
  • focused cache-store test repeated under load after fixing the fixture ordering
  • structured autoreview clean for both commits

Audit note

pnpm audit --audit-level high still identifies the two image-size@2.0.2 advisories by package version because the registry lists no patched version. This PR fixes both vulnerable loops through pnpm's checked-in dependency patch without muting the advisories, and the regression probe fails closed on zero-length ISO boxes and ICNS entries.

@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. 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. labels Aug 10, 2026
@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 10, 2026, 2:16 PM ET / 18:16 UTC.

ClawSweeper review

What this changes

Migrates all workspace builds to TypeScript 7, replaces the import-cycle parser with oxc-parser, and patches image-size against zero-length parser loops.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open for explicit owner review. The branch is a coherent TypeScript toolchain migration with focused cycle-gate and dependency-patch coverage; no discrete correctness defect was found in the reviewed diff.

Priority: P2
Reviewed head: b0dab4e3e583e76acec609fe84330dcfd3b5854b
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The implementation is focused and defensively tested; the remaining confidence step is normal integration validation of the new compiler’s generated artifacts.
Proof confidence 🌊 off-meta tidepool Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its body nevertheless records build, test, extension, and packed-release validation.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its body nevertheless records build, test, extension, and packed-release validation.
Evidence reviewed 5 items Compiler migration coverage: The branch consistently replaces tsgo and the preview compiler dependency with TypeScript 7 and tsc in the root, core, and extension manifests.
Cycle gate preserves covered import forms: The replacement visitor records static imports, re-exports, dynamic imports without options, and import types; its fixture verifies those forms and a real two-file cycle.
Dependency patch is resolved on the affected path: The lockfile applies the image-size patch to the image-size instance consumed by addons-linter through web-ext, and the regression test resolves that same dependency path.
Findings None None.
Security None None.

How this fits together

Workspace scripts compile the CLI, core library, and browser extension, while an architecture test scans source imports to prevent cycles. The extension’s transitive image decoder processes image metadata, so its patched parsing behavior affects extension tooling.

flowchart LR
  A[Workspace source] --> B[TypeScript build scripts]
  A --> C[Import-cycle check]
  C --> D[Dependency graph]
  E[Extension dependency tree] --> F[Patched image decoder]
  B --> G[CLI core and extension artifacts]
  F --> G
Loading

Decision needed

Question Recommendation
Should TypeScript 7 become the workspace’s supported compiler once the pending integration checks validate the exact PR head? Approve after integration validation: Merge once the declared build, full check, extension, and packed-release validations pass for this exact head.

Why: This owner-authored toolchain migration changes the compiler that produces published CLI and core artifacts; source review cannot make the release-compatibility acceptance decision.

Before merge

  • Resolve merge risk (P1) - Changing the compiler used for published CLI and core artifacts can alter emitted output or diagnostics for existing builds, so the supported workspace and extension checks should complete on this head before merge.
  • Complete next step (P2) - This owner-authored compiler migration needs explicit release-compatibility acceptance after normal integration validation; no mechanical repair is identified.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 10 files, +902/-112 lines The migration spans three package manifests, the lockfile, an architecture test, and a vendored dependency patch.

Merge-risk options

Maintainer options:

  1. Validate generated artifacts on the PR head (recommended)
    Require the documented workspace build, check gate, extension suite, and packed-release verification to complete before accepting the compiler change.

Technical review

Best possible solution:

Adopt TypeScript 7 after the clean-head build, full check gate, Chrome extension suite, and release-package verification confirm unchanged published artifact behavior.

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

Not applicable: this PR performs build-toolchain and dependency maintenance rather than fixing a single user-reproducible report.

Is this the best way to solve the issue?

Yes, conditionally: replacing the removed compiler-specific traversal with a parser visitor is a narrow maintainable path, provided release-artifact compatibility is accepted after integration validation.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against de5f5d0f9d71.

Labels

Label changes:

  • add P2: This is a broad but bounded build and dependency maintenance change with no demonstrated active user outage.
  • add merge-risk: 🚨 compatibility: Replacing the workspace compiler can change emitted CLI/core artifacts and typecheck behavior for existing supported builds.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its body nevertheless records build, test, extension, and packed-release validation.

Label justifications:

  • P2: This is a broad but bounded build and dependency maintenance change with no demonstrated active user outage.
  • merge-risk: 🚨 compatibility: Replacing the workspace compiler can change emitted CLI/core artifacts and typecheck behavior for existing supported builds.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its body nevertheless records build, test, extension, and packed-release validation.

Evidence

What I checked:

  • Compiler migration coverage: The branch consistently replaces tsgo and the preview compiler dependency with TypeScript 7 and tsc in the root, core, and extension manifests. (package.json:40, f55b995eab95)
  • Cycle gate preserves covered import forms: The replacement visitor records static imports, re-exports, dynamic imports without options, and import types; its fixture verifies those forms and a real two-file cycle. (tests/architecture.import-cycles.test.ts:64, f55b995eab95)
  • Dependency patch is resolved on the affected path: The lockfile applies the image-size patch to the image-size instance consumed by addons-linter through web-ext, and the regression test resolves that same dependency path. (pnpm-lock.yaml:4931, f55b995eab95)
  • Current main ownership and release provenance: Current main is de5f5d0, authored by Peter Steinberger as a dependency/security refresh; the latest release v0.21.10 is f78eba1 and predates this branch. No tag or branch contains the PR head. (package.json:40, de5f5d0f9d71)
  • Feature-history routing: Blame attributes the changed import-cycle implementation to the migration commit, while the prior test surface and current main dependency work are attributed to Peter Steinberger. (tests/architecture.import-cycles.test.ts:1, f55b995eab95)

Likely related people:

  • steipete: Current-main dependency/security work, the preceding release, and the proposed compiler migration are all attributed to Peter Steinberger. (role: recent build and dependency contributor; confidence: high; commits: de5f5d0f9d71, f78eba1590e5, f55b995eab95; files: package.json, pnpm-workspace.yaml, tests/architecture.import-cycles.test.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Confirm the pending integration checks complete successfully for this exact head.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

@steipete
steipete merged commit e5290f7 into main Aug 10, 2026
4 checks passed
@steipete
steipete deleted the chore/typescript-7-migration branch August 10, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant