Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
the same way the GET routes do; the header still works, and `sameOrigin`
still blocks cross-origin browsers. Closes KNOWN-ISSUES §5.

- **An open dashboard tab was a watcher only 60% of the time, so a headless
run hung at its first gated tool call.** The standalone server registered a
watcher when an SSE client connected and then pinged every 25s — but the ping
never refreshed the flag, against a 15s TTL. An ask raised in the 10s gap was
delegated to the composer panel; with no UI tab attached there is no panel, so
the turn hung until the answer timeout and the run never left `research`.
Measured on a real headless run (2026-10-07): the ask sat unclaimed for 10+
minutes while an SSE connection was demonstrably open, and `GET /api/state`
reported `pending: []` the whole time. The ping now refreshes the watcher, the
TTL is 60s against a 25s ping, and both constants live in `src/watcher-ttl.ts`
so the interval cannot drift outside the window it refreshes. The new test
fails on the old code and on a 25s TTL.


### Changed

Expand Down
16 changes: 13 additions & 3 deletions src/dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ import type { ReviewSignal } from './signals.ts'
// in-UI page does this from `remote.ts`'s `live()`; this is the same fact for
// the loopback server.
import { clearWatcher, noteWatcher } from './approvals.ts'
// The keep-alive lives beside the watcher TTL it must stay under: an SSE
// stream's ping is the only proof of life that keeps the tab a watcher.
import { SSE_KEEPALIVE_MS } from './watcher-ttl.ts'
import type { ApprovalRegistry } from './approvals.ts'
import type { ApprovalDecision } from './approval-bridge.ts'
import { RunGrants, runGrantable } from './run-grants.ts'
Expand Down Expand Up @@ -345,8 +348,6 @@ const FEED_LIMIT = 200
const RUN_LIMIT = 50
/** Largest approval POST body we will read. */
const MAX_BODY = 8 * 1024
/** SSE keep-alive interval, under typical proxy idle timeouts. */
const KEEPALIVE_MS = 25_000

/** Human labels for the feed, keyed by outcome. */
const OUTCOME_LABEL: Record<ApprovalOutcome, string> = {
Expand Down Expand Up @@ -915,7 +916,16 @@ export function startDashboard(
// answer times out. Guarded by `cfg.answers` so `answers: false` really
// does mean observe-only.
if (cfg.answers) noteWatcher()
const keepalive = setInterval(() => res.write(': ping\n\n'), KEEPALIVE_MS)
// The ping IS this tab's heartbeat, so it refreshes the watcher as well as
// keeping the socket warm. Without that, "watching" was measured against
// the ping interval instead of against the stream being open: at a 25s
// ping against a 15s TTL the tab was a watcher 60% of the time, and an ask
// raised in the gap went to the composer panel — which on a headless run
// does not exist, so the turn hung instead of being claimable here.
const keepalive = setInterval(() => {
res.write(': ping\n\n')
if (cfg.answers) noteWatcher()
}, SSE_KEEPALIVE_MS)
keepalive.unref()
req.on('close', () => {
clearInterval(keepalive)
Expand Down
26 changes: 25 additions & 1 deletion src/watcher-ttl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,30 @@
* operator is watching, as "the gate is flaky". `test/dashboard.test.ts` pins
* that relationship.
*
* The second heartbeat is the standalone server's SSE stream, and it was
* WRONG until 2026-10-07: it registered a watcher on connect and then pinged
* every `SSE_KEEPALIVE_MS`, but the ping never refreshed the flag. At a 15s TTL
* against a 25s ping the tab was a watcher 60% of the time, and an ask raised
* in the gap went to the composer panel — which on a headless run does not
* exist, so the turn hung at its first gated tool call. The ping now refreshes
* the watcher, and `SSE_KEEPALIVE_MS` lives here so the two cannot drift: the
* test asserts the interval clears the TTL.
*
* ponytail: a heartbeat window is still an approximation of "is a tab open".
* The upgrade path is a streaming remote method that reports attach/detach
* exactly, which is what `docs/PLAN-close-open-issues.md` §1 already plans for
* the dashboard's own state.
*
* @module dsh-feature-loop/watcher-ttl
*/
export const WATCHER_TTL_MS = 15_000
export const WATCHER_TTL_MS = 60_000

/**
* SSE keep-alive interval, in milliseconds.
*
* Lives beside the TTL it must stay under, because the two are one contract:
* the stream's ping is the standalone server's heartbeat, and
* `test/dashboard.test.ts` fails if the ping stops arriving inside the window
* it refreshes.
*/
export const SSE_KEEPALIVE_MS = 25_000
51 changes: 50 additions & 1 deletion test/dashboard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
workspaceLabelOf,
} from '../src/dashboard.ts'
import type { DashboardHandle, DashboardSnapshot } from '../src/dashboard.ts'
import { clearWatcher, watcherActive } from '../src/approvals.ts'
import { clearWatcher, noteWatcher, watcherActive } from '../src/approvals.ts'
// Type-only, so these are erased at runtime: the measurement modules are
// authored concurrently, and this file must pin their shapes without
// loading their code.
Expand Down Expand Up @@ -1016,3 +1016,52 @@ test('the in-UI page polls well inside the watcher TTL', () => {
assert.ok(Math.round(ttl / 3) < ttl, 'the derived poll must clear the TTL')
}
})

// ── the SSE ping must refresh the watcher, not just keep the socket warm ────
// The regression this pins, measured on a real headless run (2026-10-07): the
// TTL was 15s and the ping 25s, so an open dashboard tab was a watcher only
// 60% of the time. An ask raised in the 10s gap was delegated to the composer
// panel; with no UI tab attached there is no panel, so the turn hung and the
// run stalled at its first gated tool call. The test drives the ping through
// its own interval rather than waiting 25 real seconds.
test('the SSE ping refreshes the watcher, so an open tab never falls out of the TTL', async (t) => {
const here = dirname(fileURLToPath(import.meta.url))
const source = readFileSync(join(here, '..', 'src', 'dashboard.ts'), 'utf8')
const keepaliveText = /SSE_KEEPALIVE_MS\s*=\s*([\d_]+)/.exec(
readFileSync(join(here, '..', 'src', 'watcher-ttl.ts'), 'utf8'),
)?.[1]
const ttlText = /WATCHER_TTL_MS\s*=\s*([\d_]+)/.exec(
readFileSync(join(here, '..', 'src', 'watcher-ttl.ts'), 'utf8'),
)?.[1]
assert.ok(keepaliveText !== undefined, 'the keep-alive must be a literal this test can read')
assert.ok(ttlText !== undefined, 'the watcher TTL must be a literal this test can read')
const keepalive = Number(keepaliveText.replace(/_/g, ''))
const ttl = Number(ttlText.replace(/_/g, ''))

// The interval must clear the TTL with room to spare — this is the arithmetic
// that was wrong, and it is the half a source-shape assertion cannot see.
assert.ok(keepalive * 2 <= ttl,
`the SSE ping every ${keepalive}ms cannot keep a ${ttl}ms watcher TTL alive — `
+ 'an ask raised between pings escapes to the composer panel')

// And the ping must actually call `noteWatcher`, or the interval is only a
// socket warmer and the arithmetic above is meaningless.
const handler = /const keepalive = setInterval\(\(\) => \{([\s\S]*?)\}, SSE_KEEPALIVE_MS\)/.exec(source)?.[1]
assert.ok(handler !== undefined, 'the keep-alive must be a handler block this test can read')
assert.match(handler, /noteWatcher\(\)/,
'the SSE ping must refresh the watcher — a ping that only writes bytes '
+ 'leaves an open tab outside its own TTL')

// Behaviourally: with a tab open, the watcher stays active across more than
// one TTL of wall clock once the ping refreshes it.
clearWatcher()
t.after(clearWatcher)
const { dash } = await started(t, { host: '127.0.0.1', answers: true })
const disconnect = await connectSse(dash)
t.after(disconnect)
assert.equal(watcherActive(), true, 'an open tab claims asks')
noteWatcher(Date.now() - (ttl - 1_000))
assert.equal(watcherActive(), true, 'a heartbeat inside the TTL keeps the claim')
noteWatcher(Date.now() - (ttl + 1_000))
assert.equal(watcherActive(), false, 'a heartbeat older than the TTL loses it')
})
Loading