ESD-1584-docs(wasm): document interactive-mode host contract by Phil-Browne · Pull Request #486 · megaport/megaport-cli

Phil-Browne · 2026-07-06T13:59:40Z

Documents the WASM interactive-mode host contract and makes the legacy sync entrypoint fail loudly instead of hanging.

The browser CLI has no stdin, so interactive commands ask the host page for input through JS callbacks. That contract was undocumented, and running an interactive command through the legacy synchronous executeMegaportCommand deadlocked the event loop (the host could never deliver a response).

What changed:

Guard the sync entrypoint: a command that prompts under executeMegaportCommand now fails fast. Value prompts return a clear "interactive mode requires the async entrypoint" error; confirmations are treated as declined. No more deadlock.
Document the contract in WASM_README.md and frontend-integration/INTEGRATION_GUIDE.md: async-only requirement, registerPromptHandler / submitPromptResponse / cancelPrompt, the prompt-request shape, and password masking (with a caveat that some secrets currently arrive as type "resource").
Extend interactive.test.ts with password-masking, cancel, and async-only assertions, and add a Go test for the sync-path guard.

Streamed-output docs are deferred to the live-output-streaming ticket.

Closes ESD-1584.

Document the WASM interactive-mode host contract (async-only requirement, registerPromptHandler/submitPromptResponse/cancelPrompt, prompt-request shape, and password masking) in WASM_README.md and the frontend INTEGRATION_GUIDE.md. Guard the legacy synchronous executeMegaportCommand entrypoint: a command that requests a prompt while running under it now returns a clear error instead of deadlocking the JS event loop. Extend interactive.test.ts with password-masking, cancel, and async-only assertions, and add a Go test for the sync-path guard. ESD-1584

- Reconcile docs/CHANGELOG with actual behavior: value prompts return the guard error, but confirmations are treated as declined (wasmConfirmPrompt swallows the error to false), so the sync entrypoint no longer deadlocks either way. - Fix cancel error string in docs to match code ("prompt cancelled by user"). - Add "password" to the PromptType field comment; note the guard counter is process-global and fails safe. - Drop the tautological secret-resource TS test that only asserted its own fixture.

Some secret-bearing inputs (partner auth/service/shared keys, MVE registration/secret keys) are prompted as type "resource", not "password". Drop the inaccurate "covers every secret" claim and tell integrators not to rely on the password type alone to mask every secret. Note secret-resource prompts also carry a resourceType.

codecov · 2026-07-06T14:01:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.54%. Comparing base (b5d1e4e) to head (7492582).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #486      +/-   ##
==========================================
+ Coverage   77.96%   78.54%   +0.58%     
==========================================
  Files         192      192              
  Lines       18391    18507     +116     
==========================================
+ Hits        14339    14537     +198     
+ Misses       2989     2904      -85     
- Partials     1063     1066       +3

see 33 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR documents the browser/WASM interactive-mode host contract (prompt callback lifecycle and prompt request shape) and adds a guard so the legacy synchronous WASM entrypoint no longer deadlocks the browser event loop when a command tries to prompt.

Changes:

Documented the interactive-mode JS host contract and clarified that interactive commands must use the async entrypoint.
Added a sync-entrypoint guard (tracking sync execution depth) so value prompts fail fast with a clear error instead of hanging.
Extended WASM (Go) and frontend integration (Vitest) tests to cover async-only behavior, cancel lifecycle, and password-masking expectations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`WASM_README.md`	Documents the interactive-mode host contract and async-only requirement.
`frontend-integration/INTEGRATION_GUIDE.md`	Adds integration guidance for wiring prompt handlers and using the async entrypoint.
`internal/wasm/prompts.go`	Introduces sync-entrypoint tracking and prompt guard to fail fast under sync execution.
`main_wasm.go`	Marks the legacy sync entrypoint as “sync execution” so prompt requests are rejected instead of deadlocking.
`internal/wasm/prompts_test.go`	Adds coverage for the sync-path prompt guard in Go WASM tests.
`frontend-integration/__tests__/interactive.test.ts`	Adds tests for password masking, cancel behavior, and async-only routing/guard surfacing.
`CHANGELOG.md`	Notes the deadlock fix and documentation updates for WASM interactive mode.

Clamp EndSyncExecution at 0 with a CAS loop so a stray or duplicate call can't drive the counter negative and re-enable prompting under the sync entrypoint. Reset pendingPrompts and release the JS function wrapper in the sync-guard test to avoid cross-test state leaking. Shorten the hardcoded guard-error string in the frontend test to a stable substring instead of the full message.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Correct INTEGRATION_GUIDE.md: an unregistered prompt handler fails fast, it does not hang (the 5-minute timeout only applies once a handler is registered and a prompt goes unanswered). Unmount the test wrapper before throwing in createReadyComposable so a WASM-not-ready failure doesn't leave a mounted component behind. Restore fetch/WebAssembly/Go globals and the executeMegaportCommand mocks in an afterEach for the Async-Only Entrypoint tests so they can't leak into later tests in the file.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Match the production locking discipline for promptCallback so a future refactor or parallelized test run can't reintroduce a data race on this package-level field.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

wasmSecretResourcePrompt sends secret-resource prompts as PromptType "password" while still populating ResourceType, so the field isn't limited to "resource" prompts as the comment implied.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

The previous fix polled pendingPrompts for up to 1s, which still had a window where the test could hang if the prompt wasn't registered by then. Respond from the prompt callback itself, using the id it's called with, since PromptForInput always registers the prompt before invoking the callback.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

The cur <= 0 branch stored 0 unconditionally, so a concurrent BeginSyncExecution could increment the counter between the load and the store, and this call would clobber it back to 0, incorrectly re-enabling prompting while a sync execution is still in progress. Compute the clamped next value and commit it with the same CAS used for the normal decrement, so a concurrent change always aborts the attempt and retries against the fresh value.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

penzeliz-megaport · 2026-07-07T06:05:40Z

QA review — doc/contract accuracy

The code change (the sync-execution fast-fail guard) looks good. Two doc items I'd flag before merge, both about the docs matching the actual composable behaviour rather than code defects.

1. INTEGRATION_GUIDE.md (§Interactive Commands, ~L268-272) — the "no handler → fails immediately" claim is backwards for the execute() path this section tells integrators to use.

The guide says:

If no handler is registered, a value prompt fails immediately with a "prompt callback not registered" error and a confirmation is treated as declined; once a handler is registered, a prompt left unanswered times out after 5 minutes.

But useMegaportWASM.initDirect() (composables/useMegaportWASM.ts:204-226) unconditionally registers a default no-op prompt handler at init — it just console.warns and returns, logging ✅ Default prompt handler registered (does nothing - override required). So via the composable, the Go-side cb.IsUndefined() fast-fail branch is unreachable: an integrator who forgets to register their own handler does not get an immediate error. The no-op swallows the prompt and the command hangs until the 5-minute timeout, after which confirmations resolve to declined.

The immediate "prompt callback not registered" failure only applies when driving the raw WASM globals directly (no registerPromptHandler ever called). Suggest splitting the two cases so the described failure mode matches what an execute() caller actually hits:

Composable: a default no-op handler is registered at init, so forgetting to override it means prompts hang until the 5-minute timeout (not an immediate error).
Raw WASM globals: if registerPromptHandler was never called, a value prompt fails immediately with "prompt callback not registered".

2. types/megaport-wasm.d.ts:35 — stale relative to the contract this PR documents (outside the changed files, but adjacent).

type: string; // "text", "confirm", "resource"
resourceType?: string; // for resource prompts

This PR adds "password" to the Go struct comment and to the guide (§Lifecycle step 2 lists "text" | "confirm" | "password" | "resource"), but the TypeScript definition integrators import still omits it, and resourceType is also set for "password" (secret-resource) prompts. Worth updating both comments so the machine-readable contract stays in step:

type: string; // "text" | "confirm" | "password" | "resource"
resourceType?: string; // set for "resource" and secret ("password") prompts

Item 1 is the one I'd fix before merge — the whole value of a contract doc is that integrators can trust the failure-mode description.

The Interactive Commands section claimed a missing handler fails immediately, but useMegaportWASM registers a default no-op handler at init, so composable callers who forget to override it hit the 5-minute timeout instead. Split the two paths: composable (no-op default, hangs to timeout) vs raw WASM globals (immediate 'prompt callback not registered' error). Also bring the MegaportPromptRequest TypeScript definition in step with the Go struct and guide: add 'password' to the type union and note resourceType is set for both resource and secret (password) prompts.

Phil-Browne · 2026-07-07T13:17:58Z

Thanks, both fixed in 8b75b1b.

INTEGRATION_GUIDE.md §Interactive Commands: split the failure mode into the two paths. Composable (useMegaportWASM) registers a default no-op handler at init, so forgetting to override it hangs until the 5-minute timeout (confirmations then declined). Raw WASM globals with no registerPromptHandler still fail immediately with "prompt callback not registered".
types/megaport-wasm.d.ts: added "password" to the type union and noted resourceType is set for both resource and secret (password) prompts, matching the Go struct and the guide.

Changelog entries are now auto-generated at release time.

penzeliz-megaport · 2026-07-08T00:06:40Z

Both doc items resolved — thanks. Confirmed against 8b75b1b17:

Failure-mode split (the fix-before-merge item): the Interactive Commands section now separates the two paths correctly — composable registers a default no-op handler at init, so forgetting to override it hangs until the 5-minute timeout; raw WASM globals fail immediately with "prompt callback not registered". Matches the actual composable behaviour now.
megaport-wasm.d.ts: type comment now lists "password" and resourceType notes it's set for "resource" and secret "password" prompts.

One thing blocking merge, not a code issue: the PR is currently CONFLICTING against main and has no CI run. Merged #489 (output streaming) also touched INTEGRATION_GUIDE.md, megaport-wasm.d.ts, prompts_test.go and main_wasm.go. The good news: #489's edits to the two doc files are in different regions (its new Output Streaming section and the registerOutputHandler type decls), so the failure-mode split and the MegaportPromptRequest comment above won't be clobbered by the merge. The real conflict is in the wasm test/code files (prompts_test.go / main_wasm.go).

Needs a rebase onto main so CI can run and go green — I'll re-review and approve once it's mergeable. The doc review itself is done.

…tive-docs-fix # Conflicts: # WASM_README.md # internal/wasm/prompts.go # main_wasm.go

Phil-Browne · 2026-07-08T00:40:54Z

Both doc items resolved, thanks. Confirmed against 8b75b1b:

Failure-mode split (the fix-before-merge item): the Interactive Commands section now separates the two paths correctly. The composable registers a default no-op handler at init, so forgetting to override it hangs until the 5-minute timeout, while raw WASM globals fail immediately with "prompt callback not registered". Matches the actual composable behaviour now.
megaport-wasm.d.ts: the type comment now lists "password", and resourceType notes it's set for "resource" and secret "password" prompts.

Rebased onto main and resolved conflicts in WASM_README.md, internal/wasm/prompts.go, internal/wasm/prompts_test.go, and main_wasm.go. Note: the rebase surfaced that #492 (merged after this branch diverged) fully retired the synchronous entrypoint, so it no longer just fails fast on prompts, it never runs a command at all. I updated the Interactive Mode doc text (WASM_README.md and INTEGRATION_GUIDE.md) to match that, and dropped the now-dead sync-guard code (syncExecutionDepth, BeginSyncExecution/EndSyncExecution) and its test that this PR had added, since main's retirement supersedes it.

CI is running now that the branch is mergeable. Will re-review once it's green.

penzeliz-megaport · 2026-07-10T00:26:39Z

Doc/code mismatch worth fixing before merge: internal/wasm/prompts.go:17 has const CommandTimeout = 10 * time.Minute (bumped from 5→10 min earlier in this same branch, "align prompt timeout with the command budget"), but the docs weren't updated to match. Three places still say 5 minutes:

WASM_README.md:162 — "A prompt left unanswered times out after 5 minutes and the command receives an error."
frontend-integration/INTEGRATION_GUIDE.md:312 — "time out after 5 minutes (confirmations then resolve to declined)..."
frontend-integration/INTEGRATION_GUIDE.md:318 — "a prompt left unanswered times out after 5 minutes."
frontend-integration/INTEGRATION_GUIDE.md:363 — "An unanswered prompt times out after 5 minutes and the command receives an error."

Since this PR's whole purpose is documenting the host contract precisely, this is exactly the kind of detail an integrator would build against and get wrong. Otherwise the contract docs check out against the current implementation (composable default handler, resource-vs-secret prompt typing for partner/MVE keys, executeMegaportCommand guard behaviour) — just this timeout figure needs updating to 10 minutes in all four spots.

Phil-Browne · 2026-07-10T00:33:11Z

Fixed in 7492582. All four spots now say 10 minutes to match CommandTimeout = 10 * time.Minute: WASM_README.md:162 and INTEGRATION_GUIDE.md lines 312, 318, 363.

Copilot

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

penzeliz-megaport · 2026-07-13T01:30:04Z

#514 deletes frontend-integration/, including the three files this PR edits: INTEGRATION_GUIDE.md, __tests__/interactive.test.ts, and types/megaport-wasm.d.ts. If #514 is the direction, those edits no longer apply; the host-contract docs this PR adds to WASM_README.md survive regardless. Worth deciding whether this PR is still needed as-is against #514 rather than settling it in a merge.

Phil-Browne · 2026-07-13T14:21:37Z

Closing in favor of #514.

We settled on the deletion in #514 as the direction: frontend-integration/ is dead code the Portal never imports, so the INTEGRATION_GUIDE.md, interactive.test.ts, and types/megaport-wasm.d.ts edits here go away with the directory. The one piece worth keeping, the interactive-mode host contract, is folded into #514's WASM_README.md (with the 10-minute timeout and password-masking notes intact). ESD-1584 is delivered there.

Nothing unique is lost: the WASM_README.md version documents the raw window prompt contract (registerPromptHandler / submitPromptResponse / cancelPrompt), the host-agnostic form of the same contract the guide covered through the composable. The composable-specific behavior in the guide described frontend-integration/'s own wrapper, which is exactly what's being removed.

Phil-Browne added 3 commits July 6, 2026 06:39

Copilot AI review requested due to automatic review settings July 6, 2026 13:59

Phil-Browne requested review from a team and penzeliz-megaport as code owners July 6, 2026 13:59

Phil-Browne requested a review from mega-alex July 6, 2026 13:59

Copilot started reviewing on behalf of Phil-Browne July 6, 2026 14:00 View session

Phil-Browne changed the title ~~docs(wasm): document interactive-mode host contract~~ ESD-1584-docs(wasm): document interactive-mode host contract Jul 6, 2026