feat(agent-email): one-command playground launcher + live hub launch card

[kovtcharov]

Why this matters

Trying the email agent meant a multi-step dance — fetch the binary, spawn the sidecar, find the localhost URL, open it. This makes it one command (npx @amd-gaia/agent-email playground) and surfaces it on the hub agent page with a copy-the-command card whose Open playground button goes live the moment the local sidecar is reachable.

A static web page can't start a local process, so this is the meet-in-the-middle: a one-command launcher in the CLI + a smart button on the page that detects when the sidecar is up.

What's in it

• npm CLI — new playground command: fetchBinary → startSidecar → open the default browser to /v1/email/playground → run until Ctrl+C (auto-reaped on exit). Flags: --port (default 8131), --out (binary cache dir), --no-open.
• Hub agent page — a Playground card: the one-liner with a copy button, plus an Open playground button gated by a live liveness probe of the local sidecar (a no-cors fetch resolves if it's up, rejects if not; 127.0.0.1 is a trustworthy origin so the HTTPS hub may probe/link it). Shown only for npm agents that declare a playground_url (from feat(hub): production agent docs, changelog channel & richer hub rendering #1839).
• Docs synced (README/SPEC/SKILL/CHANGELOG) per the doc-sync rule (docs(claude): functional changes must sync all bundled docs (README/SPEC/SKILL/CHANGELOG) #1842).

Test plan

• npm: cd hub/agents/npm/agent-email && npm run build && npm test (36); node dist/cli.js help lists playground
• Website: cd website && npx astro check (0 errors) + npx vitest run (19) + build
• Hub agent page: Playground card shows the command (copy works) + an Open button that activates only once the local sidecar responds
• End-to-end (needs the published binary): npx @amd-gaia/agent-email playground fetches, starts the sidecar, and opens the playground

[github-actions]

Verdict: Request changes — one small robustness fix, then this is good to merge.

This PR turns "try the email agent" into a single command (npx @amd-gaia/agent-email playground) and adds a hub-page card that copies the command and lights up an Open button once your local sidecar is reachable. The approach — a CLI launcher plus a localhost liveness probe on the page — is the right way to bridge "a web page can't start a local process," and the docs were synced across README/SPEC/SKILL/CHANGELOG exactly as the project's doc-sync rule asks.

The one thing to fix before merge: the "open the browser" helper claims to be non-fatal, but it isn't. When the OS browser-opener isn't installed — which is normal on headless servers, plain containers, and WSL without wslu — opening fails after the helper returns, and that failure isn't caught where the code thinks it is. The result is the whole launcher crashes and tears down the sidecar it just started, instead of harmlessly printing the URL. A one-line handler fixes it. Everything else below is optional polish.

🔍 Technical details

🟡 Important — openBrowser can crash the launcher and kill the sidecar (src/cli.ts:125)

spawn() does not throw synchronously when the opener (xdg-open / open / cmd start) is missing — it emits an asynchronous 'error' event. The try/catch here only guards the synchronous spawn(...).unref() call, so that event is never caught. With no 'error' listener on the child, Node re-throws it as an uncaughtException.

That's worse than a no-op: startSidecar (via spawnSidecar, autoCleanup defaults on) installs a process-wide uncaughtException handler that reaps the sidecar tree and calls process.exit(1) (lifecycle.ts:137). So a missing opener — common on headless Linux, containers, and WSL without wslu — doesn't "print the URL regardless," it kills the sidecar the user just launched. Note spawnSidecar itself already attaches an 'error' handler (lifecycle.ts:225); openBrowser should do the same:

/** Best-effort cross-platform "open this URL in the default browser". */
function openBrowser(url: string): void {
  try {
    const [cmd, args] =
      process.platform === "darwin"
        ? ["open", [url]]
        : process.platform === "win32"
          ? ["cmd", ["/c", "start", "", url]]
          : ["xdg-open", [url]];
    const child = spawn(cmd, args as string[], { stdio: "ignore", detached: true });
    // The opener may be absent (headless/container/WSL) — spawn reports that via an
    // async 'error' event, NOT a sync throw, so swallow it here or it becomes an
    // uncaughtException and the sidecar auto-reaper tears everything down.
    child.on("error", () => {
      /* non-fatal: the URL was already printed above */
    });
    child.unref();
  } catch {
    /* non-fatal: the URL is printed regardless */
  }
}

🟢 Minor — redundant signal handling (src/cli.ts:169)

startSidecar already auto-registers SIGINT/SIGTERM/SIGHUP reapers (autoCleanup on by default). Those handlers run before cmdPlayground's own stop, so by the time your graceful shutdown() sends SIGTERM, the auto-reaper has already SIGKILLed the tree — the "stopping the sidecar cleanly" path is effectively preempted. It still exits 0, so this is harmless, but if you want the clean SIGTERM path to actually run (and to drop the duplication), own the lifecycle explicitly:

  const sidecar = await startSidecar({ binaryPath, port, autoCleanup: false });

🟢 Minor — port bound + reserved-port check (src/cli.ts:142)

Number.isInteger(port) && port > 0 accepts 99999 (invalid) and 4001 (which spawnSidecar rejects with a RangeError → surfaces as a generic "unexpected error" rather than a clean exit 2). Tightening the guard keeps bad input on the friendly error path:

  if (!Number.isInteger(port) || port <= 0 || port > 65535 || port === 4001) {
    process.stderr.write(`error: --port must be a port in 1..65535 and not 4001 (got ${String(args.flags.port)})\n`);
    return 2;
  }

🟢 Minor — no test for the new command

The playground path adds real branching (port parse/validate, temp-dir default) but isn't unit-tested. There's no cli.test.ts today, so this is consistent with the current suite rather than a regression — but a small test asserting --port abc → exit 2 and the default outDir would lock in the contract. Low priority.

Strengths

• Doc-sync done right. README/SPEC/SKILL/CHANGELOG all updated together — exactly the bundled-doc rule in CLAUDE.md (docs(claude): functional changes must sync all bundled docs (README/SPEC/SKILL/CHANGELOG) #1842), and the canonical miss this repo just hit in feat(agent-email): auto-reap sidecar on parent exit/crash (no manual wiring) #1841.
• The liveness probe is a clean fit for the constraint. playground_url && isNpm gating + a no-cors localhost probe (with the trustworthy-origin reasoning spelled out in the comment) is the correct way to let an HTTPS page detect a local http://127.0.0.1 sidecar without claiming to read it. healthUrl correctly targets the root /health the standalone sidecar serves (client.ts:96), not the router-scoped one.
• Reuse over reimplementation. Composes fetchBinary / startSidecar / shutdown rather than re-rolling spawn logic; the temp-dir cache default avoids littering cwd and is a cache-hit on rerun.

[kovtcharov]

Thanks — all four addressed in dc382917:

• 🟡 openBrowser crashing the launcher. Spot on — spawn reports a missing opener via an async 'error' event, so it was escaping the try/catch → uncaughtException → the auto-reaper killed the sidecar. Added a child.on("error", …) listener (mirroring spawnSidecar), so a missing open/xdg-open/cmd start is now genuinely non-fatal — the URL is still printed.
• 🟢 Redundant signal handling. Switched to startSidecar({ autoCleanup: false }) and now own SIGINT/SIGTERM/SIGHUP explicitly (with a re-entry guard), so the graceful shutdown() path actually runs instead of being pre-empted by the SIGKILL reaper.
• 🟢 Port bounds + 4001. Extracted resolvePlaygroundPort and tightened it to 1..65535 and reject 4001, so bad input stays on the friendly exit 2 path instead of surfacing spawnSidecar's RangeError.
• 🟢 No test. Added test/cli.test.ts (10 cases: defaults, valid, abc/0/-1/70000/8131.5/4001 → error, and the temp-dir cache default). To make cli.ts importable, I guarded the entry so main() only runs when invoked directly — verified the bin still works (node dist/cli.js help → exit 0).

npm: 46 tests + build clean. Website: 0 errors / 19 tests.

[kovtcharov]

A second deep pass (now that the launcher owns its own lifecycle) found one more release-blocking leak — fixed in 372935c3.

Blocker: an orphaned sidecar in the autoCleanup:false window. cmdPlayground starts the sidecar with autoCleanup: false (so the graceful shutdown actually runs), but that means no reaper is installed until the SIGINT/SIGTERM/SIGHUP handlers are wired a few lines later. Any throw in that window — realistically an EPIPE from a stdout.write into a closed pipe (npx … playground | head) — escaped to main().catch → process.exit(1) and left the sidecar holding its port.

🔍 Fix + the other items

• cli.ts — guard the whole post-start region in try { … } catch (e) { await shutdown(sidecar); throw e }, so any throw after the sidecar is up still tears it down. Also .catch() the shutdown promise before .finally(resolve) (no unhandledRejection net with autoCleanup off).
• SPEC.md — dropped the "auto-reaped on exit" claim for the playground (false with autoCleanup:false, and it contradicted SKILL.md); now states the command owns the lifecycle and stops on the three signals or a startup error.

Verified clean: openBrowser async-error fix, resolvePlaygroundPort/reserved-4001, the invokedDirectly() entry guard (bin still runs), arg parsing, card gating + XSS-safety of the website probe. npm: 46 tests + build clean; bin verified.

Merge-order: this conflicts with #1843 in [id].astro (same anchor). Suggest merging #1843 first, then rebasing this one keeping both the package-download card and the playground card.

[kovtcharov-amd]

@claude review this PR.

[github-actions]

Verdict: Approve — every item from the prior round is genuinely fixed, the doc-sync holds, and the #1843 rebase kept both cards. This is good to merge.

The launcher now owns its own lifecycle correctly: a missing browser-opener is non-fatal, bad --port input stays on the friendly exit-2 path, and the orphan window you caught is closed — any throw after the sidecar starts now tears it down on the way out. The hub page keeps both the package-download card (#1843) and the new playground card, and the localhost liveness probe is sound. No new blockers; the points below are observations, not change requests.

🔍 Technical details

Re-reviewed against the four original findings + the self-reported blocker:

• 🟡 openBrowser async-error → uncaughtException → sidecar reap. Fixed — child.on("error", …) mirrors spawnSidecar's handler (src/cli.ts:139). A missing open/xdg-open/cmd start is now genuinely non-fatal; the URL is printed regardless.
• 🟢 Redundant signal handling. startSidecar({ autoCleanup: false }) + explicit SIGINT/SIGTERM/SIGHUP with a re-entry guard (stopping flag) means the graceful shutdown() actually runs instead of being pre-empted (src/cli.ts:200-218).
• 🟢 Port bounds + 4001. Extracted resolvePlaygroundPort (src/cli.ts:160), rejects ≤0 / >65535 / 4001 / non-integer. Unit-tested in test/cli.test.ts (abc/0/-1/70000/8131.5/4001 → error).
• 🟢 No test. test/cli.test.ts added; invokedDirectly() guard (src/cli.ts:264) makes cli.ts importable without running main().
• Blocker (orphan in the autoCleanup:false window). Closed — the whole post-start region is wrapped in try { … } catch (e) { await shutdown(sidecar).catch(…); throw e } (src/cli.ts:191-228), so an EPIPE (npx … playground | head) tears the sidecar down rather than orphaning it.

Doc-sync verified consistent: SPEC.md:136-138 correctly scopes the playground command to autoCleanup:false / "owns the sidecar lifecycle", which no longer contradicts the general-SDK auto-reap default described in README.md/SKILL.md/CHANGELOG.md.

Website: both [data-download]/Download .zip (#1843) and [data-playground] cards coexist in [id].astro; the no-cors probe builds healthUrl from new URL(pgUrl).origin + '/health' and Astro auto-escapes the data-playground-url/href attributes, so no XSS surface.

Couldn't run npm test/build here — this CI checkout has no installed node_modules for the package (TS node types + vitest resolve fail), which is an environment gap, not the PR. Relying on your reported 46 tests + clean build / website 19 tests.

Minor, non-blocking: the page's setInterval(probe, 3000) runs for the page's lifetime with no clearInterval; harmless on a single agent page, just noting it.