Skip to content

feat(playground): block bundle upload while the editor reports type errors - #1548

Merged
joshunrau merged 3 commits into
DouglasNeuroInformatics:mainfrom
joshunrau:playground-errors
Sep 16, 2026
Merged

joshunrau merged 3 commits into
DouglasNeuroInformatics:mainfrom
joshunrau:playground-errors

Conversation

@joshunrau

Copy link
Copy Markdown
Collaborator

Summary

Two changes to the playground's Upload Bundle flow.

1. Type errors block the upload. The transpiler state was never a type check — esbuild strips types without checking them, so an instrument that fails tsc still reached status: 'built' and uploaded happily. Monaco already validates every instrument file (diagnostics: true, onlyVisible off), so a new diagnostics slice holds its error-severity markers owned by the typescript/javascript languages, useEditorErrorSync keeps it current, and UploadBundleDialog disables Upload and lists the offending file (line N): message while that list is non-empty. handleSubmit re-checks, so the programmatic path fails loudly too.

The hook asks for markers one file at a time rather than reading every marker monaco holds: monaco also holds models for the runtime declarations, globals.d.ts, and files the user has deleted (deleteFile never disposes a model), none of which should block an upload.

2. A failed request shows the API's own message. A failure reported only the status line, so a 409 said nothing about why the instrument was rejected. The status now titles the notification — 409 - Conflict — and the message is whatever the JSON body carries under message (string, or a string array joined), or nothing when the body has none or is not JSON. A request that never got a response keeps its previous HTTP Request Failed notification.

Verification

pnpm lint (34/34) and pnpm test (1160 tests) pass. No e2e test: the Playwright suite starts api, gateway and web only — the playground is not in playwright.config.ts — and there is no apps/playground/vitest.config.ts, so this app has no unit tier either.

Driven manually in a browser against the dev server:

  • Clean instrument → no error block, Upload enabled once logged in.
  • Appended const brokenValue: number = 'not a number'; → dialog showed index.ts (line 28): Type 'string' is not assignable to type 'number'., Upload disabled. Undo → block gone.
  • Against a stub API: 409 + {"message":"An instrument with the same title already exists"} → notification titled 409 - Conflict with that message; 409 + a text/plain body → title alone, no message line.

Known gaps

Both come from existing EditorPane behaviour, left alone here:

  • Files added by bulk upload (addFiles) get no monaco model until opened, so their errors do not register. Single-file adds are fine — addFile selects the new file.
  • Closing every tab unmounts EditorPane, whose existing cleanup disposes all models, so diagnostics genuinely disappear and the gate reports nothing.

apps/playground/AGENTS.md documents the gate under "Talking to an ODC instance".

🤖 Generated with Claude Code

joshunrau and others added 3 commits September 16, 2026 13:43
…rrors

The transpiler state was never a type check: esbuild strips types without
checking them, so an instrument that fails tsc still reaches status 'built'
and uploaded happily. Monaco already validates every instrument file, so a
new diagnostics slice holds the error-severity markers owned by the
typescript and javascript languages, and UploadBundleDialog disables Upload
and lists them while that list is non-empty.

useEditorErrorSync asks for markers one file at a time rather than reading
every marker monaco holds, because monaco also holds models for the runtime
declarations, globals.d.ts and files the user has deleted, none of which
should block an upload.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…fails

A failed upload reported only the status line as the notification message,
so a 409 said nothing about why the instrument was rejected. The status now
titles the notification as "409 - Conflict", and the message is whatever the
JSON body carries under `message`, or nothing when the body has none or is
not JSON at all. A request that never got a response keeps its previous
"HTTP Request Failed" notification.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@joshunrau
joshunrau merged commit 2ea3f0a into DouglasNeuroInformatics:main Sep 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant