Skip to content

fix(browser): connect the live view once, not once per postMessage - #1186

Merged
philmerrell merged 1 commit into
developfrom
fix/browser-live-view-duplicate-connect
Sep 19, 2026
Merged

philmerrell merged 1 commit into
developfrom
fix/browser-live-view-duplicate-connect

Conversation

@philmerrell

Copy link
Copy Markdown
Contributor

What broke

The browser sign-in viewer never streamed on dev. It framed correctly, then showed "Could not start the session. It may have ended." — the dcv.authenticate error path (code 10), behind a WebSocket closing with Close received after close.

Found while testing whether the Chromium URL blocklist holds under human control during a takeover. That question is still unanswered — this was in the way.

What it wasn't

Probed against real AgentCore before changing anything, because the obvious hypothesis was wrong:

probe result
signature minted for /live-view, GET /live-view 404 WebSocket endpoint not found
signature minted for /live-view, GET /live-view/auth 400 Expected a websocket subprotocol of dcv ← signature valid
signature minted for /live-view/auth, GET /live-view/auth 403 Authentication failed
/live-view/auth + subprotocol dcv OPEN
same, after take_control (stream DISABLED) OPEN

So SigV4 base-path signing is correct as-is, the endpoint is healthy, and take_control is not implicated. My first hypothesis — that signing /live-view couldn't validate for /live-view/auth — is disproven by row 2, and signing the sub-path directly would actively break it.

What it was

The SPA posts the minted URL up to three times — after minting, on the iframe's load, and in reply to the viewer's ready — because it cannot know which arrives first.

The viewer guarded with if (connection && currentUrl), but connection is assigned only after dcv.connect() resolves. Two posts milliseconds apart both reached dcv.authenticate; the second socket's open closed the first.

Fix: guard on the connection attempt, and release the latch on every terminal outcome so one failed mint can't lock the viewer shut for the rest of the session.

The SPA comment claiming the later posts "win and the other is a no-op" was false. Corrected in place rather than deleted — it is the assumption that caused this.

Tests

live-view.js had no tests at all, which is why this shipped. Adds the first, running the real file in a vm context against a DOM stub (no jsdom — the page touches only a handful of DOM APIs, and a stub keeps that dependency honest).

Mutation-checked: reverting the guard to its original form fails 2 of the 5. Full suite: 861 pass, tsc --noEmit clean.

Risk

Viewer asset + one comment. No infrastructure, no IAM, no backend. Deploys via platform.yml (CDK stages assets/mcp-sandbox).

🤖 Generated with Claude Code

The sign-in viewer never streamed on dev. It framed, then failed with
"Could not start the session. It may have ended." — the `dcv.authenticate`
error path, code 10, behind a WebSocket that closed with
`Close received after close`.

The service was not at fault. Probed against real AgentCore, the live-view
auth socket opens cleanly: the SigV4 query signature minted for the base
`/live-view` path DOES validate for the `/live-view/auth` sub-path (signing
`/auth` itself is what 403s), the required `dcv` subprotocol negotiates, and
it opens with the automation stream both ENABLED and DISABLED — so
`take_control` is not implicated either.

The viewer was. The SPA posts the minted URL up to three times — after
minting, on the iframe's `load`, and in reply to the viewer's `ready` —
because it cannot know which fires first. The viewer guarded on an
established `connection`, but that is assigned only after `dcv.connect()`
RESOLVES. Two posts milliseconds apart therefore both reached
`dcv.authenticate`, and the second socket's open closed the first.

Guard on the connection ATTEMPT instead, and release the latch on every
terminal outcome so a failed mint cannot lock the viewer shut for the
session. The comment in the SPA claiming the later posts were no-ops was
simply wrong, and is corrected rather than deleted.

Adds the first test for `live-view.js`, which had none — it runs the real
file in a `vm` context against a DOM stub. Mutation-checked: reverting the
guard to its original form fails 2 of the 5.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@philmerrell
philmerrell merged commit f89bd70 into develop Sep 19, 2026
6 checks passed
philmerrell added a commit that referenced this pull request Sep 19, 2026
…1187)

* fix(browser): connect the live view once, not once per postMessage

The sign-in viewer never streamed on dev. It framed, then failed with
"Could not start the session. It may have ended." — the `dcv.authenticate`
error path, code 10, behind a WebSocket that closed with
`Close received after close`.

The service was not at fault. Probed against real AgentCore, the live-view
auth socket opens cleanly: the SigV4 query signature minted for the base
`/live-view` path DOES validate for the `/live-view/auth` sub-path (signing
`/auth` itself is what 403s), the required `dcv` subprotocol negotiates, and
it opens with the automation stream both ENABLED and DISABLED — so
`take_control` is not implicated either.

The viewer was. The SPA posts the minted URL up to three times — after
minting, on the iframe's `load`, and in reply to the viewer's `ready` —
because it cannot know which fires first. The viewer guarded on an
established `connection`, but that is assigned only after `dcv.connect()`
RESOLVES. Two posts milliseconds apart therefore both reached
`dcv.authenticate`, and the second socket's open closed the first.

Guard on the connection ATTEMPT instead, and release the latch on every
terminal outcome so a failed mint cannot lock the viewer shut for the
session. The comment in the SPA claiming the later posts were no-ops was
simply wrong, and is corrected rather than deleted.

Adds the first test for `live-view.js`, which had none — it runs the real
file in a `vm` context against a DOM stub. Mutation-checked: reverting the
guard to its original form fails 2 of the 5.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(ci): trigger the Platform Stack deploy on the assets it deploys

`infrastructure/assets/**` is staged into the mcp-sandbox bucket at synth,
and `scripts/build/fetch-dcv-sdk.sh` runs inside platform.yml before synth.
Neither was in the workflow's `paths:` filter.

So a change to either deployed NOWHERE. frontend-deploy.yml does not ship
those assets, and no workflow failed — the push went green, the merge looked
clean, and the old artifact stayed live. That is exactly what happened to the
browser sign-in viewer fix in #1186: it merged, CI passed, and the broken
viewer remained deployed until the workflow was dispatched by hand.

The same hole covers a `fetch-dcv-sdk.sh` version, SHA256 or GPG-fingerprint
bump — a supply-chain pin change that would silently not take effect.

Tests derive the expectation from what the workflow and the CDK tree actually
do (scripts it invokes; directories the constructs stage) rather than
restating the `paths:` list back at itself, so a newly staged directory is
covered the day it is added. Mutation-checked: removing either filter fails
all three.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
philmerrell added a commit that referenced this pull request Sep 19, 2026
…ew (#1189)

#1186 removed a real duplicate-connect bug but the viewer still never
streamed: one `dcv.authenticate`, still failing with code 10 behind a socket
that closed immediately.

Measured against real AgentCore, one probe at a time:

  correct single query  -> socket OPEN, held
  DOUBLED query         -> HTTP 403

That is what we were sending. The SDK APPENDS `httpExtraSearchParams` to
whatever URL it is given, and we handed `authenticate` the full signed URL
with its query still attached AND the extras callback — so every SigV4
parameter went twice and the service refused it.

`connect()` has always stripped the query for exactly this reason; its
comment even says "the transport appends to whatever URL it is given".
`authenticate()` was simply missed, and the failure mode (a socket that
opens then closes, surfacing as "Failed to communicate with server") reads
like a service problem rather than a malformed request.

Also ruled out by probe, so the next person does not re-walk them: base-path
SigV4 signing IS valid for the `/auth` sub-path (signing `/auth` itself
403s), the `dcv` subprotocol is requested correctly by the SDK, an `Origin`
header changes nothing, and `take_control` is not implicated.

Mutation-checked: restoring the signed URL fails the new assertion.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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