Conversation
An approval is a screenshot, a sentence and two answers, which is a chat message — and 0.4.0 left it in a browser tab. `channels` is the seam an adapter sits on: one array of in-process objects, notified once each as soon as there is something worth sending, and in approval mode handed the JPEG the phone is looking at plus a way to answer. `answer()` goes through the same settle path a relay `approve` takes, so the first answer wins whoever gives it and the loser is told with a `false` rather than a throw — an approval sent to two places at once is meant to be answerable twice. The relay still receives `ended`, so an open phone shows the ending, and nothing else about a settled handoff changes. The wide event grows `answeredVia`. A channel cannot break the handoff: `notify` is not awaited and a throw or a rejection is one `channel_failed` warning. Why not let the adapter be a second human WebSocket client — the relay accepts one human peer and replaces it, so it would throw the phone off — is in ADR 0007, along with why `answer()` returns a boolean.
Review round 1 (Opus), findings 11 and 12. Taking the approval screenshot is a round trip to the browser, and the page can close or the wait can run out while it is in flight. The frame was already withheld in that case; the channel was not, so a chat could end up holding live Approve/Deny buttons under a request that no longer exists. `announce` now checks the same `over` flag `sendApprovalFrame` checks. The test settles a handoff mid-screenshot and asserts nobody was told. `channelHandoff` had an `|| !shot` arm that fell back to a takeover announcement. It was unreachable, but its consequence was not a degraded message — it was the wrong one: a bearer link and "drive the browser" in place of a yes or no. Replaced by two builders, so the state cannot be expressed rather than being handled wrongly. Also: a takeover handback now has a test proving the wide event carries no `answeredVia`. The review read that guard as redundant; it is not. Handback and abort settle through the same `answerHandoff`, so without the outcome check every takeover event would claim it was "answered via relay". The README's channels snippet reads its environment through a destructuring default, because `process.env.X` is `string | undefined` and the old snippet did not compile.
The channels hook is the 0.5.0 feature (CHANGELOG: Unreleased — 0.5.0), and handraise-telegram declares a peer range of >=0.5.0, so the version has to say so before the adapter can install this branch as its dev dependency.
Round 2 (GPT-5.6 Sol), finding 9. Every one of these describes a promise ADR 0007 makes and nothing enforced. - A late `answer()` after a timeout, and after the browser session died, returns false and emits no second wide event. - A session that dies while `page.screenshot()` is still in flight notifies no channel — the disconnect half of the window commit 09b484c closed for the timeout half. - Mutating the Buffer a channel was handed does not change the frame the phone already has. An adapter that watermarks in place is a plausible thing to write. - A `notify` that returns a never-settling promise does not delay the handoff. A regression that awaited it hangs to the test timeout, which is the loudest failure this boundary can have. - `answer` and `screenshot` do not exist on a takeover ChannelHandoff at compile time. `tsc --noEmit` covers the test file, and an unused `@ts-expect-error` is itself an error, so a union that quietly grew those members fails the typecheck. The fake browser can now die on demand (`killSession()`), which is what made the two disconnect cases drivable rather than assumed. Proved red first: with the `over` guard removed from `announce`, both "no channel notified" tests fail; with `answer` added to TakeoverChannelHandoff, `tsc` reports the unused directive.
ADR 0007 left this as a known gap and said it should wait for a second adapter. The first one settled it: with no signal that the handoff is over, an adapter that waits for a reply can only stop on its own clock. Measured on handraise-telegram — a handoff answered on the phone 500 ms in left its timer and its in-flight poll alive, and the Node process exited 20.5 s later with a 20 s budget. At the default that is a script that prints its result and then sits there for five minutes fifty, holding the bot's single getUpdates slot, so a second run inside that window is refused outright. `ChannelHandoff.settled` is the existing settle promise, exposed: resolved once with the outcome, on every path, never rejecting, the same promise for every channel of one handoff. It carries `finalOutcome`, not the outcome the human gave — a handback that becomes `disconnected` because the session died during the cookie capture must not reach a chat as `resolved`, and there is a test that fails if it does. It resolves before teardown, so a channel releases its connection while the relay is still shutting down rather than after. Additive: nothing existing changes shape.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
channels?: HandoffChannel[]toraiseHand: a place to announce a handoff that can also answer it. A channel gets the link and the reason; in approval mode it also gets the screenshot the phone sees and ananswer("approve" | "deny")that settles the handoff in-process. First answer wins between the phone and any channel; the second is told so withfalse. The relay still receivesended, so the phone shows the outcome either way. The wide event gainsansweredVia: "relay" | "channel"on approved and denied outcomes.Why an in-process hook rather than the adapter joining the relay as a second human: the relay allows one human peer and replaces the previous one, so an adapter socket would push the phone off; and the adapter would have neither the screenshot bytes nor the handoff id without a round trip. ADR 0007 has the alternatives.
Guarantees, each with a test against the real relay process:
notifyis called once per channel, only after the URL exists and, in approval mode, after the screenshot was captured; never after the handoff is over; never awaited; a throw, a rejection or a promise that never settles cannot delay or end the handoff (onechannel_failedwarning). A lateanswer()after timeout or disconnect returnsfalseand emits nothing.answerandscreenshotdo not exist on a takeover handoff, at the type level (@ts-expect-errorfixture) and at runtime. Mutating the delivered screenshot buffer cannot change what the phone received.Verification: lint, typecheck, 199 tests, build, dist smoke; live e2e against Solari including an approval answered by a channel (
answeredVia: "channel", one frame, relay destroyed). Complexity ratchet unchanged. Two independent reviews (Opus, GPT-5.6 Sol) and two fix rounds; every finding on this branch is fixed. The first adapter,handraise-telegram, is built against this branch and lands as its own repo once this is released as 0.5.0.Version bumped to 0.5.0 here because the adapter declares a peer range of
>=0.5.0. CHANGELOG entry is still under Unreleased; it gets its date at tag time.🤖 Generated with Claude Code