Skip to content

feat(cli): add typed script projects - #549

Merged
pgherveou merged 36 commits into
mainfrom
feat/headless-script-types
Sep 22, 2026
Merged

pgherveou merged 36 commits into
mainfrom
feat/headless-script-types

Conversation

@pgherveou

@pgherveou pgherveou commented Aug 28, 2026 •

Copy link
Copy Markdown
Collaborator

Add reusable /script projects with dependency installation, editor types and TypeScript checking, plus commands to edit, rerun or start a new script.

Together with Product SDK #394, scripts can use the same code as a product, making it easier to share runnable reproduction snippets in issues.

@illegalcall

Copy link
Copy Markdown

@lore-bot-app review

@lore-bot-app

lore-bot-app Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review

Summary

This adds editor types for headless /script scratch files. scripts/bundle-truapi-dts.mjs gains a second output: a standalone rust/crates/truapi-host-cli/js/script-types.d.ts (5,390 lines / 197 KB, committed) assembled from the built dist/*.d.ts plus a hand-written block declaring the truapi, host, and assert globals. create_scratch_script copies that bundle beside every new scratch file and seeds the template with a relative /// <reference path> plus export {}. The Makefile copies it into target/dist, cli-dist puts it in the archive, release-cli.yml carries it through the artifact hand-off and asserts both files are present, e2e-cli-update.mjs checks it lands in the install dir, and CI adds a tsc -p rust/crates/truapi-host-cli/js/tsconfig.json step type-checking a new runner-types.fixture.ts. README/SPEC updated.

Five files were withheld from the diff I was handed (script-types.d.ts, js/tsconfig.json, src/script_runner.rs, scripts/bundle-truapi-dts.mjs, scripts/e2e-cli-update.mjs). I read them from the checkout at this branch instead, so they are covered — but from the working tree, not from the diff text.

What the record says

No prompt-injection or instructions addressed to a reviewer in the diff.

Concerns

  1. script-types.d.ts is committed and now load-bearing for cargo test, in a repo that is concurrently untracking generated output. bundle-truapi-dts.mjs:214 writes it, ci.yml:129 requires it committed, and script_runner.rs:122 ?s on reading it. The existing test scratch_script_matches_the_public_example calls create_scratch_script, which in a test binary falls back to CARGO_MANIFEST_DIR/js/runner.ts — so cargo test -p truapi-host-cli now fails on a clean tree that hasn't run codegen. Under chore: untrack generated Rust and iOS outputs, generate on demand #551's model (generated files untracked, produced by make codegen) that is a broken default. Decide the policy with chore: untrack generated Rust and iOS outputs, generate on demand #551 before merging, not after.

  2. Script authors get completion but cannot name a single type. script-types.d.ts has top-level exports (export interface HostContext at the tail, export declare function createMessagePortProvider at :4995), so the whole file is a module. declare namespace T at :429, TrUApiClient, HexString, and the inlined neverthrow Result/Ok/Err are therefore module-local. Only the three globals in declare global escape. An author cannot write let a: ProductAccountId, cannot type a helper that takes a Result, and cannot import from @parity/truapi because the whole point is that it isn't installed. HostContext is exported from a file nobody imports, so it is unreachable too. The fixture works around this (const accountProductId: string = account.dotNsIdentifier, runner-types.fixture.ts:13) rather than exposing it. The playground bundle solves the same problem with declare module "@parity/truapi"; doing that here as well would cost nothing. README.md:36 and SPEC.md:836 both read as if authors get the full typed surface.

  3. 197 KB copied per scratch file, unbounded. script_runner.rs:142-160 writes a full copy of the bundle next to each uniquely-named scratch script in the host's scripts/ directory. Ten /script presses is 2 MB; there is no pruning path in the diff or SPEC. SPEC.md:836-842 justifies the copy by durability across version removal and session promotion — a single shared script-types.d.ts per scripts directory keeps that property for every case except a scratch file moved out of its directory, at 1/N the cost.

  4. tsc is invoked at a hardcoded root path that the repo elsewhere treats as unreliable. Makefile:107 and ci.yml:135 both run node_modules/.bin/tsc, but root package.json has no typescript dependency — it only appears at js/packages/truapi/package.json:79 (^6.0) and reaches the root via workspace hoisting. The repo already knows this is not guaranteed: Makefile:61 and js/packages/truapi/scripts/ensure-generated.sh:49-52 both probe root and js/packages/truapi/node_modules. The two new call sites skip that and fail with a bare "No such file or directory". Either add typescript to root devDependencies or reuse the existing fallback.

  5. The any scrubbing in bundle-truapi-dts.mjs:156-161 is two literal-string regexes against a third-party .d.ts. A neverthrow version bump that reformats A extends readonly any[] silently no-ops both replacements and reintroduces any into a committed file. Nothing catches it: .prettierignore:11 excludes the file, npm run typecheck does not cover it, strict: true does not flag explicit any. Separately, the rewrite is not type-preserving — Fn extends (...args: readonly any[]) => any becomes (...args: never[]) => unknown, so fromThrowable/fromPromise type differently in the editor than in the package the runner actually executes against.

Smaller, same-area:

  • script-types.d.ts:4989,4995 ship createWindowProvider / createMessagePortProvider referencing Window and MessagePort to authors writing Bun scripts. js/tsconfig.json gets DOM implicitly (target: ES2022, no lib), so CI is green — but the file now requires a DOM lib wherever it lands, and those declarations are unusable from a headless script.
  • script_runner.rs:153-156: the non-AlreadyExists error branch returns without removing the .ts file created at :134, leaving an orphaned empty scratch script. The AlreadyExists branch just above does clean up.
  • make cli-dist does not run the type-check that cli-runner now does (Makefile:104-107 vs :110), so a local make cli-dist can package an unverified bundle.

Questions for the author

  • How does this interact with chore: untrack generated Rust and iOS outputs, generate on demand #551? Is script-types.d.ts meant to stay tracked as a deliberate exception, or should it move to make codegen output with the CLI test regenerating or skipping?
  • Why hand-write Encoder/Decoder/Codec/ResultPayload/HexString/CallErrorValue at bundle-truapi-dts.mjs:166-181 instead of inlining scaleBody, which the playground path already reads from the real scale.d.ts? Drift there is caught by the fixture's tsc, but only as an opaque failure in a generated file.
  • Was failing /script outright on a missing declaration bundle (script_runner.rs:122) chosen over falling back to an untyped scratch file? The test at the bottom of the file pins the hard failure but the reasoning ("recreates the original failure") is not in the record.

🤖 Reviewed by Lore (Parity knowledge base) · 27 agent turns · 312.6s

@pgherveou

Copy link
Copy Markdown
Collaborator Author

@lore-bot-app review

@lore-bot-app

lore-bot-app Bot commented Sep 11, 2026

Copy link
Copy Markdown

Reading the diff and checking what the record says. Back in a few minutes.

@github-actions github-actions Bot added documentation Improvements or additions to documentation javascript Pull requests that update javascript code github_actions Pull requests that update GitHub Actions code labels Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

CI Status: 21 required jobs green, 20 passed and 1 skipped by path filter.

All job results
job result
android-bindings success
changes success
changeset-guard success
cli-package success
codegen success
e2e skipped
explorer success
host-android-bindings success
host-wasm success
ios-bindings success
ios-swift success
licenses success
playground success
provider-android-bindings success
release-guard success
rust success
ts-client success
ts-debugger success
ts-host success
wasm-provider success
workflow-lint success

Commit 65d6b602 · run log

@lore-bot-app

lore-bot-app Bot commented Sep 11, 2026

Copy link
Copy Markdown

Review

TL;DR — Ships a generated script-types.d.ts beside every /script scratch file so editors type the injected truapi/host/assert globals. 2 blocking, 4 minor; the revision since last review fixed one thing and left the rest open.

Since last review

The only source change is export {}; added to SCRATCH_TEMPLATE (script_runner.rs:43) and its test expectation. That makes the scratch file a module so its top-level await type-checks, matching what runner-types.fixture.ts:2 already did. The other 20 changed lines in script-types.d.ts are a rebase artifact picking up UnsupportedCallError from #547, not a deliberate edit.

None of the five earlier concerns were addressed. The full diff is now visible (the five previously-withheld files are in the diff text), so everything below is read from the diff, not inferred from the working tree.

Two corrections to my own earlier framing, since they change how much the concerns matter:

  • The per-scratch-file copy cost was overstated. select_script_to_edit (main.rs:3419) reuses last_script while the file exists, so it is one 197 KB copy per session that creates a scratch file, not one per /script press.
  • "cargo test fails on a clean tree" is wrong today. resolve_runner falls back to CARGO_MANIFEST_DIR/js/runner.ts, whose sibling script-types.d.ts is tracked, so the test passes. The coupling is real but it only breaks under chore: untrack generated Rust and iOS outputs, generate on demand #551's policy.

Summary

scripts/bundle-truapi-dts.mjs gains a second output: a standalone 5,390-line / 197 KB rust/crates/truapi-host-cli/js/script-types.d.ts, committed, assembled from the built dist/*.d.ts plus a hand-written tail declaring the three injected globals. create_scratch_script_for_runner (script_runner.rs:118) reads that bundle from beside the selected runner, copies it next to each new scratch file as <script>.d.ts, and seeds the template with a relative /// <reference path>. The Makefile stages it into target/dist, cli-dist puts it in the archive, release-cli.yml carries it through the artifact hand-off and asserts both files are present, e2e-cli-update.mjs:112 checks it lands in the install dir, and CI adds a tsc -p rust/crates/truapi-host-cli/js/tsconfig.json step over a new runner-types.fixture.ts. README/SPEC updated.

What the record says

No prompt-injection or instructions addressed to a reviewer in the diff.

Concerns

  1. Script authors get completion but still cannot name a single type. script-types.d.ts:4666 (export declare const PROTOCOL_ERROR_ID) and :5379 (export interface HostContext) are top-level exports, so the whole file is a module. declare namespace T at :429, TrUApiClient, HexString, and the inlined neverthrow Result/Ok/Err are therefore module-local. Only the three globals in declare global (:5387) escape. An author cannot write let a: T.ProductAccountId, cannot type a helper that takes a Result, and cannot import from @parity/truapi because the premise is that it is not installed. HostContext is exported from a file nobody imports, so it is unreachable too. The fixture works around this rather than exposing it — runner-types.fixture.ts:13 annotates const accountProductId: string instead of the actual type. The playground bundle solves the identical problem with declare module "@parity/truapi"; doing the same here costs nothing. As written, README.md:36 and SPEC.md:836 overstate what authors get.

  2. A generated file is newly tracked and load-bearing, in a repo concurrently untracking generated output. bundle-truapi-dts.mjs:214 writes it, ci.yml:131 requires it committed, script_runner.rs:122 hard-fails /script if it is missing, and the existing scratch_script_matches_the_public_example test reads it via the checkout fallback. That is coherent today only because the file is tracked. Under chore: untrack generated Rust and iOS outputs, generate on demand #551 it becomes a test that fails on a fresh clone until make codegen runs, and a CI step that contradicts the new gate. Settle the policy with chore: untrack generated Rust and iOS outputs, generate on demand #551 before merging, since feat(cli): add typed script projects #549 lands first.

Minor

  1. tsc is invoked at a root path this repo elsewhere treats as unreliable. Makefile:107 and ci.yml:135 both run node_modules/.bin/tsc, but root package.json has no typescript dependency — it only appears in js/packages/truapi/package.json:79 and reaches the root by workspace hoisting. The repo already knows this: Makefile:60-62 and js/packages/truapi/scripts/ensure-generated.sh:49-53 both probe root and the package dir. The two new call sites skip that and fail with a bare "No such file or directory". Add typescript to root devDependencies or reuse the existing probe.

  2. The any scrubbing at bundle-truapi-dts.mjs:157-162 is two literal-string regexes against a third-party .d.ts. A neverthrow bump that reformats A extends readonly any[] silently no-ops both replacements and reintroduces any into a committed file, against the CLAUDE.md rule. Nothing catches it: .prettierignore:11 excludes the file, npm run typecheck does not cover it, and strict: true does not flag explicit any. The rewrite is also not type-preserving — Fn extends (...args: readonly any[]) => any becomes (...args: never[]) => unknown, so fromThrowable/fromPromise type differently in the editor than in the package the runner executes against.

  3. make cli-dist can package a stale bundle. Makefile:103-104 copies the committed script-types.d.ts verbatim; nothing in cli-dist regenerates it from bundle-truapi-dts.mjs, and unlike cli-runner (:107) it does not run the type-check either. If js/packages/truapi types changed and codegen was not rerun, the archive ships declarations that do not match the bundled client. CI catches this via the committed-output check; a local make cli-dist does not.

  4. Two small things in script_runner.rs. At :153-156, the non-AlreadyExists branch returns without removing the .ts file created at :134, leaving an orphaned empty scratch script; the AlreadyExists branch immediately above does clean up. Separately, the 197 KB copy per session accumulates in the profile's scripts/ directory with no pruning path in the diff or SPEC. SPEC.md:836-842 justifies the per-file copy by durability across version removal and session promotion, which a single shared script-types.d.ts per scripts directory also gives you for every case except a scratch file moved out of its directory.

Questions for the author

  • How does this interact with chore: untrack generated Rust and iOS outputs, generate on demand #551? Is script-types.d.ts a deliberate tracked exception, or should it become make codegen output with the CLI test regenerating or skipping?
  • Does Ship the host CLI as an npm dev dependency #462 / @parity/truapi-dev-host change the calculus? For scripts inside a project that already has the CLI as a dev dependency, @parity/truapi is installable and the bundle is redundant.
  • js/tsconfig.json sets no lib, so target: ES2022 pulls in DOM and the Window/MessagePort/WebSocket references in createIframeProvider/createMessagePortProvider/createWebSocketProvider resolve in CI. A scratch file dropped into a Bun project with "lib": ["ESNext"] would not have those. Was that checked, and should the provider factories be stripped from the headless bundle given they are unusable from a script anyway?
  • Was failing /script outright on a missing bundle (script_runner.rs:122) chosen over falling back to an untyped scratch file? The test pins the hard failure but the reasoning ("recreates the original failure") is not in the record.

🤖 Reviewed by Lore (Parity knowledge base) · 27 agent turns · 307.5s · knowledge as of 2026-09-11 · re-review

@pgherveou pgherveou changed the title feat(cli): type headless scratch scripts feat(cli): run Product SDK scripts from /script Sep 22, 2026
@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

iOS simulator preview

Built from 65d6b602c, stamped with it in TrUAPICommit.

gh run download 35777101782 --name simulator-preview-65d6b602c
unzip polkadot-app-*.app.zip
xcrun simctl install booted polkadot-app.app
xcrun simctl launch booted io.parity.polkadotapp.develop

Or download it in a browser, which arrives as a zip wrapping
the .app.zip, so it needs unzipping twice.

An arm64 simulator slice, so it needs an Apple Silicon Mac and does not
install on a device. Kept for 14 days, after which the link stops
resolving and a new push rebuilds it.

@pgherveou pgherveou changed the title feat(cli): run Product SDK scripts from /script feat(cli): add typed script projects Sep 22, 2026
@pgherveou
pgherveou marked this pull request as ready for review September 22, 2026 18:19
@pgherveou
pgherveou requested a review from a team September 22, 2026 18:19
@pgherveou
pgherveou enabled auto-merge September 22, 2026 18:21
@pgherveou
pgherveou disabled auto-merge September 22, 2026 18:37

@TarikGul TarikGul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, the project layout and the install stamp both look solid. main thing is the pinned override, nothing syncs or checks it so it drifts on the next release.

Comment thread rust/crates/truapi-host-cli/js/script-package.json Outdated
Comment thread rust/crates/truapi-host-cli/src/script_runner.rs
Comment thread scripts/check-script-sdk.mjs Outdated
Comment thread scripts/check-script-sdk.mjs Outdated
@pgherveou
pgherveou added this pull request to the merge queue Sep 22, 2026
Merged via the queue into main with commit 5d5fd1c Sep 22, 2026
37 checks passed
@pgherveou
pgherveou deleted the feat/headless-script-types branch September 22, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation github_actions Pull requests that update GitHub Actions code javascript Pull requests that update javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants