Skip to content

Commit a59ca89

Browse files
jwaldripclaude
jwaldrip
and
claude
authored
fix(review): remote review works end-to-end via HTTP+heartbeat (#213)
* fix(review): remote review works end-to-end via HTTP+heartbeat Remote review was broken top-to-bottom: - `HAIKU_REVIEW_SITE_URL=` empty string fell through `config.ts:str()`'s `??` check and produced `review.siteUrl = ""`, so `buildReviewUrl` generated tokens starting at `/review/#…` instead of `https://haikumethod.ai/review/#…`. `str()` now treats empty as "use default". - `withE2E` crashed on any 204/205/304 response with "Invalid response status code 204" from Node's Response constructor. E2E wrapping now short-circuits for null-body statuses. - CORS only allowed `Content-Type` in request headers and nothing in response headers. The SPA's `bypass-tunnel-reminder` request header tripped preflight rejection, and the browser couldn't read `X-E2E-Encrypted` / `X-Original-Content-Type` from responses. Added `HEAD`, `bypass-tunnel-reminder` to allowed headers, and the two E2E headers to `Access-Control-Expose-Headers`. - Custom-subdomain requests to loca.lt hang indefinitely when the upstream reservation service is unhappy. Dropped the per-process subdomain entirely — stable subdomains aren't worth fighting for. - WebSocket presence detection never worked through loca.lt (browser UA sniffing serves the interstitial on WS upgrade, and browsers can't set custom headers on `new WebSocket()`). Replaced WS with a HEAD heartbeat every 10s from the SPA to `/api/session/:id/heartbeat`. A per-session sweep on the server marks sessions as `presenceLost` when no heartbeat for 25s and wakes the waiting `_openReviewAndWait` so it can reopen the browser without burning a retry attempt. Simpler client code, no tunnel-specific workarounds, presence detection within ~20s of tab close. - Review SPA now hides site header/footer and renders full-pane with zero chrome. Scoped via a `data-haiku-review` attribute on `<html>` that's torn down on unmount, so no impact on any other route. Also adds `scripts/smoke-remote-review.ts` for end-to-end local testing of the review transport without driving a full intent flow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(review): StatusBadge crashes when status is undefined Frontmatter fields like `status` / `discipline` / `stage` are optional on some units and intents. The SPA was calling `.toLowerCase()` on whatever came through, crashing the whole review render with "Cannot read properties of undefined (reading 'toLowerCase')" on any intent/unit where a referenced field was missing. StatusBadge now treats undefined/null/empty as "unknown" and keeps rendering. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style(http): biome formatting for multi-line condition CI's biome check wanted the 204/205/304 guard broken across lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(review): address code review comments - sessions.ts: evictSessions now clears heartbeat state on both TTL and cap-based eviction, and sweepPresence cleans up presenceLost entries when the backing session is gone. - http.ts: withE2E skips encryption for null-body responses (HEAD endpoints), removing redundant crypto + allocation on every heartbeat. OPTIONS preflight no longer double-wraps withCors since the network-layer middleware covers it. - server.ts: _openReviewAndWait's timeout throw path now calls clearHeartbeat alongside clearE2EKey. - useReviewSession.ts: heartbeat fetch has an 8s AbortSignal timeout so a hung tunnel can't stack up concurrent requests. `isConnected` starts as `null` (not-yet-checked) so the "Reconnecting…" banner doesn't flash on first render while the initial beat is in flight. - ReviewShell.tsx / ReviewSidebar.tsx: only show the reconnecting indicator when `isConnected === false`, so the `null` initial state renders as "connected" until proven otherwise. - smoke-remote-review.ts: SIGINT/SIGTERM handlers close the tunnel before exit so Ctrl-C doesn't leave dangling resources. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 67b832f commit a59ca89

10 files changed

Lines changed: 307 additions & 158 deletions

File tree

‎packages/haiku/scripts/smoke-remote-review.ts‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#!/usr/bin/env tsx
2+
// Standalone smoke test for H·AI·K·U remote review.
3+
// Starts HTTP server, opens tunnel, creates a fake review session, builds the
4+
// signed review URL, and prints it. Run with:
5+
//
6+
// HAIKU_REMOTE_REVIEW=1 npx tsx scripts/smoke-remote-review.ts [--open]
7+
8+
import { spawn } from "node:child_process"
9+
import { review } from "../src/config.ts"
10+
import { startHttpServer } from "../src/http.ts"
11+
import { createSession } from "../src/sessions.ts"
12+
import {
13+
buildReviewUrl,
14+
closeTunnel,
15+
isRemoteReviewEnabled,
16+
openTunnel,
17+
} from "../src/tunnel.ts"
18+
19+
console.error("HAIKU_REMOTE_REVIEW :", process.env.HAIKU_REMOTE_REVIEW)
20+
console.error("HAIKU_REVIEW_SITE_URL :", process.env.HAIKU_REVIEW_SITE_URL)
21+
console.error("isRemoteReviewEnabled :", isRemoteReviewEnabled())
22+
console.error("review.siteUrl :", review.siteUrl)
23+
24+
const session = createSession({
25+
intent_dir: "/tmp/fake-intent",
26+
intent_slug: "smoke-test",
27+
review_type: "intent",
28+
target: "smoke-test",
29+
html: "",
30+
})
31+
console.error("session_id :", session.session_id)
32+
33+
const port = await startHttpServer()
34+
console.error("http port :", port)
35+
36+
let url: string
37+
if (isRemoteReviewEnabled()) {
38+
const tunnelUrl = await openTunnel(port)
39+
console.error("tunnel url :", tunnelUrl)
40+
url = buildReviewUrl(session.session_id, tunnelUrl, "review")
41+
} else {
42+
url = `http://127.0.0.1:${port}/review/${session.session_id}`
43+
}
44+
45+
console.log("")
46+
console.log("REVIEW URL:")
47+
console.log(url)
48+
console.log("")
49+
50+
if (process.argv.includes("--open")) {
51+
const cmd = process.platform === "darwin" ? "open" : "xdg-open"
52+
spawn(cmd, [url], { stdio: "ignore", detached: true }).unref()
53+
}
54+
55+
const holdMs = Number(process.env.SMOKE_HOLD_MS ?? 600_000)
56+
console.error(`holding open for ${Math.round(holdMs / 1000)}s — curl or browse the URL`)
57+
58+
function shutdown(): void {
59+
try {
60+
closeTunnel()
61+
} catch {
62+
/* best-effort */
63+
}
64+
process.exit(0)
65+
}
66+
67+
process.on("SIGINT", shutdown)
68+
process.on("SIGTERM", shutdown)
69+
70+
setTimeout(shutdown, holdMs)

‎packages/haiku/src/config.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ function flag(name: string, defaultValue: boolean): boolean {
1919
}
2020

2121
function str(name: string, defaultValue: string): string {
22-
return process.env[name] ?? defaultValue
22+
const raw = process.env[name]
23+
if (raw === undefined || raw === "") return defaultValue
24+
return raw
2325
}
2426

2527
/** Feature flags. */

‎packages/haiku/src/http.ts‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { z } from "zod"
1111
import { REVIEW_APP_HTML } from "./review-app-html.js"
1212
import {
1313
getSession,
14+
recordHeartbeat,
1415
updateDesignDirectionSession,
1516
updateQuestionSession,
1617
updateSession,
@@ -199,8 +200,21 @@ function withCors(response: Response): Response {
199200
if (!isRemoteReviewEnabled()) return response
200201
const headers = new Headers(response.headers)
201202
headers.set("Access-Control-Allow-Origin", "*")
202-
headers.set("Access-Control-Allow-Methods", "GET, POST, OPTIONS")
203-
headers.set("Access-Control-Allow-Headers", "Content-Type")
203+
headers.set("Access-Control-Allow-Methods", "GET, HEAD, POST, OPTIONS")
204+
// `bypass-tunnel-reminder` is the custom header the SPA sends to skip
205+
// loca.lt's interstitial reminder page. Custom headers trigger a CORS
206+
// preflight and must be explicitly allowed.
207+
headers.set(
208+
"Access-Control-Allow-Headers",
209+
"Content-Type, bypass-tunnel-reminder",
210+
)
211+
// `X-E2E-Encrypted` / `X-Original-Content-Type` are read by the SPA's
212+
// e2eFetch helper to decide whether to decrypt. Custom response headers
213+
// are only visible to cross-origin JS when exposed here.
214+
headers.set(
215+
"Access-Control-Expose-Headers",
216+
"X-E2E-Encrypted, X-Original-Content-Type",
217+
)
204218
return new Response(response.body, {
205219
status: response.status,
206220
statusText: response.statusText,
@@ -227,6 +241,17 @@ async function withE2E(
227241
): Promise<Response> {
228242
if (!sessionId || !isE2EActive(sessionId) || response.status >= 400)
229243
return response
244+
// Null-body responses (HEAD endpoints, 204/205/304) have no payload to
245+
// encrypt, and constructing `new Response(body, { status: 204 })` throws
246+
// "Invalid response status code 204" in modern Node. Skip encryption
247+
// and the associated crypto + allocation cost on every call.
248+
if (response.body === null) return response
249+
if (
250+
response.status === 204 ||
251+
response.status === 205 ||
252+
response.status === 304
253+
)
254+
return response
230255

231256
const originalContentType =
232257
response.headers.get("Content-Type") ?? "application/octet-stream"
@@ -841,9 +866,11 @@ function handleRequest(req: Request): Response | Promise<Response> {
841866
const url = new URL(req.url)
842867
const path = url.pathname
843868

844-
// CORS preflight (handled before E2E — preflight is plaintext)
869+
// CORS preflight (handled before E2E — preflight is plaintext).
870+
// The network-layer middleware (see `listenOnPort`) already applies
871+
// `withCors` to every response, so returning a bare 204 here is enough.
845872
if (req.method === "OPTIONS" && isRemoteReviewEnabled()) {
846-
return withCors(new Response(null, { status: 204 }))
873+
return new Response(null, { status: 204 })
847874
}
848875

849876
// GET /files/:sessionId/*path — consolidated file serving
@@ -858,6 +885,13 @@ function handleRequest(req: Request): Response | Promise<Response> {
858885
return handleSessionApi(apiSessionMatch[1])
859886
}
860887

888+
// HEAD /api/session/:sessionId/heartbeat — client presence ping
889+
const heartbeatMatch = path.match(/^\/api\/session\/([^/]+)\/heartbeat$/)
890+
if (heartbeatMatch && req.method === "HEAD") {
891+
const ok = recordHeartbeat(heartbeatMatch[1])
892+
return new Response(null, { status: ok ? 200 : 404 })
893+
}
894+
861895
// GET /review/:sessionId
862896
const reviewMatch = path.match(/^\/review\/([^/]+)$/)
863897
if (reviewMatch && req.method === "GET") {

‎packages/haiku/src/server.ts‎

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@ import {
3636
reportFeedback,
3737
} from "./sentry.js"
3838
import {
39+
clearHeartbeat,
3940
createDesignDirectionSession,
4041
createQuestionSession,
4142
createSession,
4243
getSession,
44+
hasPresenceLost,
4345
waitForSession,
4446
} from "./sessions.js"
4547
import type {
@@ -859,32 +861,41 @@ setOpenReviewHandler(
859861
openBrowser()
860862

861863
// Retry loop: wait → check → reopen if needed → wait again
862-
for (let attempt = 0; attempt < 3; attempt++) {
863-
try {
864-
await waitForSession(session.session_id, 10 * 60 * 1000) // 10 min per attempt
865-
} catch {
866-
// Timeout — check if session is still pending (user might still be looking)
867-
const check = getSession(session.session_id)
864+
attempts: for (let attempt = 0; attempt < 3; attempt++) {
865+
// Inner loop: handle heartbeat-driven wakeups without burning an attempt
866+
while (true) {
867+
let timedOut = false
868+
try {
869+
await waitForSession(session.session_id, 10 * 60 * 1000) // 10 min per attempt
870+
} catch {
871+
timedOut = true
872+
}
873+
874+
const updated = getSession(session.session_id)
868875
if (
869-
check &&
870-
check.session_type === "review" &&
871-
check.status === "decided"
876+
updated &&
877+
updated.session_type === "review" &&
878+
updated.status === "decided"
872879
) {
880+
clearHeartbeat(session.session_id)
873881
if (useRemote) {
874882
clearE2EKey(session.session_id)
875883
closeTunnel()
876884
}
877885
return {
878-
decision: check.decision,
879-
feedback: check.feedback,
880-
annotations: check.annotations,
886+
decision: updated.decision,
887+
feedback: updated.feedback,
888+
annotations: updated.annotations,
881889
}
882890
}
883-
// Still pending — try reopening the browser
884-
if (attempt < 2) {
891+
892+
if (hasPresenceLost(session.session_id)) {
893+
// Heartbeat gap detected — user closed the tab. Reopen without
894+
// consuming a retry attempt; the user may have just refreshed.
885895
console.error(
886-
`[haiku] Review session timeout (attempt ${attempt + 1}/3) — reopening browser`,
896+
`[haiku] Review session ${session.session_id} lost presence — reopening browser`,
887897
)
898+
clearHeartbeat(session.session_id)
888899
if (useRemote) {
889900
const tunnelUrl = await openTunnel(port)
890901
const newUrl = buildReviewUrl(
@@ -898,26 +909,33 @@ setOpenReviewHandler(
898909
}
899910
continue
900911
}
901-
}
902912

903-
const updated = getSession(session.session_id)
904-
if (
905-
updated &&
906-
updated.session_type === "review" &&
907-
updated.status === "decided"
908-
) {
909-
if (useRemote) {
910-
clearE2EKey(session.session_id)
911-
closeTunnel()
912-
}
913-
return {
914-
decision: updated.decision,
915-
feedback: updated.feedback,
916-
annotations: updated.annotations,
913+
if (timedOut) {
914+
if (attempt < 2) {
915+
console.error(
916+
`[haiku] Review session timeout (attempt ${attempt + 1}/3) — reopening browser`,
917+
)
918+
if (useRemote) {
919+
const tunnelUrl = await openTunnel(port)
920+
const newUrl = buildReviewUrl(
921+
session.session_id,
922+
tunnelUrl,
923+
reviewType,
924+
)
925+
openBrowser(newUrl)
926+
} else {
927+
openBrowser()
928+
}
929+
continue attempts
930+
}
931+
break attempts
917932
}
933+
934+
// Spurious wakeup — loop and wait again.
918935
}
919936
}
920937

938+
clearHeartbeat(session.session_id)
921939
if (useRemote) {
922940
clearE2EKey(session.session_id)
923941
closeTunnel()

‎packages/haiku/src/sessions.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,70 @@ const sessionEvents = new EventEmitter()
44
// Prevent warnings when many sessions are active concurrently
55
sessionEvents.setMaxListeners(200)
66

7+
// ─── Presence / heartbeat tracking ───────────────────────────────────
8+
// Browser clients HEAD /api/session/:id/heartbeat every 10s. If no
9+
// heartbeat arrives within HEARTBEAT_GRACE_MS we mark the session as
10+
// disconnected and wake up any waiting handler so it can reopen the
11+
// browser or bail out. This replaces the WebSocket-based presence
12+
// detection that didn't work across tunnel proxies.
13+
const HEARTBEAT_GRACE_MS = 25_000
14+
const HEARTBEAT_SWEEP_INTERVAL = 5_000
15+
const lastHeartbeatAt = new Map<string, number>()
16+
const presenceLost = new Set<string>()
17+
18+
export function recordHeartbeat(sessionId: string): boolean {
19+
if (!sessions.has(sessionId)) return false
20+
lastHeartbeatAt.set(sessionId, Date.now())
21+
if (presenceLost.delete(sessionId)) {
22+
console.error(`[haiku] Presence restored for session ${sessionId}`)
23+
}
24+
return true
25+
}
26+
27+
export function hasPresenceLost(sessionId: string): boolean {
28+
return presenceLost.has(sessionId)
29+
}
30+
31+
export function clearHeartbeat(sessionId: string): void {
32+
lastHeartbeatAt.delete(sessionId)
33+
presenceLost.delete(sessionId)
34+
}
35+
36+
function sweepPresence(): void {
37+
const now = Date.now()
38+
for (const [id, ts] of lastHeartbeatAt) {
39+
if (now - ts <= HEARTBEAT_GRACE_MS) continue
40+
const session = sessions.get(id)
41+
if (!session) {
42+
lastHeartbeatAt.delete(id)
43+
presenceLost.delete(id)
44+
continue
45+
}
46+
// Only interesting while a handler is still blocking on the session
47+
if (
48+
(session.session_type === "review" && session.status !== "pending") ||
49+
(session.session_type === "question" && session.status !== "pending") ||
50+
(session.session_type === "design_direction" &&
51+
session.status !== "pending")
52+
) {
53+
continue
54+
}
55+
if (!presenceLost.has(id)) {
56+
presenceLost.add(id)
57+
console.error(
58+
`[haiku] Presence lost for session ${id} — no heartbeat in ${Math.round(
59+
(now - ts) / 1000,
60+
)}s`,
61+
)
62+
sessionEvents.emit(`session:${id}`)
63+
}
64+
}
65+
}
66+
67+
// Watchdog sweeps every HEARTBEAT_SWEEP_INTERVAL. unref() so the timer
68+
// never prevents the MCP process from exiting cleanly.
69+
setInterval(sweepPresence, HEARTBEAT_SWEEP_INTERVAL).unref()
70+
771
/**
872
* Notify that a session's status has been updated.
973
* Tool handlers awaiting waitForSession() will resolve.
@@ -159,6 +223,7 @@ function evictSessions(): void {
159223
if (now - ts > SESSION_TTL_MS) {
160224
sessions.delete(id)
161225
sessionCreatedAt.delete(id)
226+
clearHeartbeat(id)
162227
}
163228
}
164229
// If still over cap, evict oldest
@@ -167,6 +232,7 @@ function evictSessions(): void {
167232
if (!oldest) break
168233
sessions.delete(oldest[0])
169234
sessionCreatedAt.delete(oldest[0])
235+
clearHeartbeat(oldest[0])
170236
}
171237
}
172238

0 commit comments

Comments
 (0)