Skip to content

feat(live): agent-facing long-poll endpoint for proceed signals - #169

Merged
zpyoung merged 3 commits into
mainfrom
claude/agent-events-long-poll-8f0496
Jul 9, 2026
Merged

zpyoung merged 3 commits into
mainfrom
claude/agent-events-long-poll-8f0496

Conversation

@zpyoung

@zpyoung zpyoung commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Summary

Implements docs/plans/agent-events-long-poll.md — an agent-facing long-poll endpoint on the isles live server so the Quirk bridge can replace its state/events file-poll loop with a boring one-request/one-response GET (no poll latency, no file races, server-push on click).

Node server only; strictly additive. Rust parity (crates/isles-server) and the bridge's wait switch are follow-ups per the plan's Non-goals.

What changed (src/live.mjs)

  • GET /__agent-isles/agent/events — returns the newest matching proceed record (latest-wins, at-most-once, removed on delivery) or parks up to hold seconds (default 100, clamped [0,110]; hold=0 = check once) then 204s. screen/since filters mirror the bridge's lenient _screen_matches (matches screen slug or screen_file; unstamped records match any filter) and latest_proceed_event; since uses >=. Body is the verbatim appendSignalEvent record.
  • GET /__agent-isles/agent/health — {"ok":true,"pid":<pid>} readiness probe.
  • In-memory proceed queue — only type === "proceed" is queued; bounded to the newest 64; cleared on new-screen advance so a stale click can't satisfy a wait for the next screen (with screen/since as second/third guards).
  • Auth — per-session crypto.randomBytes(16) bearer token published in state/server-info (now written 0600). Both agent routes require it (401 on missing/wrong) and reject any request bearing an Origin header (403).
  • Idle interplay — an open hold counts as an active client (won't idle-shutdown mid-brainstorm); held requests are released with 503 at shutdown.
  • Bonus — fire-and-forget live:signal SSE broadcast on every click (browsers ignore unknown events; nothing depends on it).

Back-compat (all hard requirements met)

  • appendSignalEvent still writes the events file; record shape unchanged.
  • No changes to /, /events, /__agent-isles/{screens,signal,tree,raw}, or reader mode.
  • token is an additive server-info field; the bridge's parser ignores unknown fields.

Test plan

  • 18 new tests in tests/live.test.mjs covering every bullet of the plan's Testing section: queue delivery/drain, park-and-resolve (with push-immediacy assertion), hold-expiry retention across invocations, since >= edge, screen slug/file/unstamped matching, two-clicks-newest-wins, new-screen queue clear, auth 401/403, server-info token, live:signal broadcast, non-proceed-not-queued, idle-vs-hold, 503-on-shutdown, WebSocket signal-twin ingress, and queue-bound eviction.
  • Adversarial multi-agent review (5 dimensions → verify): 8 findings refuted, 4 confirmed — all test-coverage gaps (now closed), zero correctness defects.
  • npm test green: 392/394 unit (the 2 cli-packs failures are pre-existing/environmental, confirmed via stash) + 28/28 browser.
  • Manual curl loop against a real isles live dir: click → poll returns the record instantly with screen/screen_file stamps; re-poll → 204; server-info is -rw-------; no-token → 401; Origin → 403.

Adds an agent long-poll channel to the live server so the Quirk bridge
can replace its events-file poll loop with one-request/one-response GETs.

- GET /__agent-isles/agent/events: returns the newest matching `proceed`
  record (latest-wins, at-most-once) or parks up to `hold` seconds
  (clamped [0,110]) then 204s; `screen`/`since` filters mirror the
  bridge's lenient `_screen_matches`/`latest_proceed_event`.
- GET /__agent-isles/agent/health: cheap authed readiness probe.
- In-memory proceed queue (bounded 64), cleared on new-screen advance so
  a stale click cannot satisfy a wait for the next screen.
- Per-session bearer token published in state/server-info (0600); both
  agent routes require it and reject any request bearing an Origin header.
- Held holds count as active clients against idle shutdown and are
  released with 503 at shutdown.
- Bonus fire-and-forget live:signal SSE broadcast on every click.

Strictly additive: the events file, existing routes, and record shape
are unchanged. Rust parity (crates/isles-server) and the bridge switch
are follow-ups per the plan.
Copilot AI review requested due to automatic review settings July 9, 2026 12:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR introduces an authenticated agent long-poll endpoint to replace the bridge’s file-poll loop, backed by an in-memory bounded queue of proceed events, and adds integration tests to validate the new behavior.

Changes:

  • Added /__agent-isles/agent/events (long-poll with hold, since, screen) and /__agent-isles/agent/health, protected by a per-session bearer token.
  • Enqueued/broadcast signal records on both POST and WebSocket paths; cleared in-memory agent queue on screen advance; prevented idle shutdown while agent holds are parked.
  • Added extensive test coverage for queueing, draining, hold behavior, filtering, auth/origin gate, idle shutdown, and WS signal ingestion.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tests/live.test.mjs Adds integration tests and small HTTP/WS helpers for the new agent long-poll + auth/token behavior.
src/live.mjs Implements token auth, agent event queue + hold handling, broadcasts signals, and updates idle/close behavior accordingly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/live.test.mjs
Comment thread tests/live.test.mjs
Comment thread src/live.mjs Outdated
Follow-up to the Fable-model red-team of the agent long-poll endpoint.

- Redact the bearer token from `isles live` stdout: it lands in scrollback
  / CI logs / agent transcripts where the 0600 file mode can't protect it.
  The bridge reads the token from the file, so stdout never needed it.
- Select the newest queued record by insertion order (last matching), not
  wall-clock-max timestamp — this is exactly the bridge's
  `latest_proceed_event` ("last matching line") and stays correct if the
  clock steps backwards between two clicks. Also simpler.
- Harden the server-info 0600 write: remove any stale tmp first so the
  token is always *created* at 0600, never carried over at a looser mode.
- Add a test for held-request cleanup on client abort mid-hold (the
  plan's `req.close` path); mutation-verified it fails if cleanup regresses.
@zpyoung

zpyoung commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

Adversarial review (Fable-model subagent) — follow-up in b5650fb

Ran a second, independent adversarial red-team on a Fable model (distinct from the earlier Opus multi-agent review). Verdict: safe to ship, no contract or lifecycle break found. It probed since >= vs >, hold clamp edges, equal-timestamp tie-breaks, at-most-once with concurrent parked requests, filtered-consumption not dropping non-matching records, DNS-rebinding, duplicate Origin/Authorization headers, timingSafeEqual length handling, and keepAliveTimeout vs long holds — none broke it.

Four low-severity findings, all addressed:

  1. Token echoed to stdout — isles live printed the full server-info (now incl. token) to stdout, undercutting the 0600 file mode. Now redacted from stdout; the bridge still reads the token from the file.
  2. "Newest" = wall-clock-max, not insertion order — diverges from the bridge's latest_proceed_event ("last matching line") under a backward clock step. Now selects the last matching record in insertion order (also simpler).
  3. server-info 0600 tmp-reuse window — stale tmp could carry the token at a looser mode. Now removes any stale tmp first so it's always created at 0600.
  4. No test for req.close abort cleanup — added one; mutation-verified it fails if the cleanup regresses.

Full suite still green: 393/395 unit (2 failures are pre-existing/environmental cli-packs) + 28/28 browser.

Addresses Copilot review feedback on PR #169.

- openSignalWs: reject on a non-101 response or timeout instead of
  hanging forever if the WS route ever stops upgrading.
- agentGet: add a settled guard (resolve once) and a request timeout so
  a stalled response fails fast rather than blocking the runner; keep the
  deliberate resolve-on-close so the shutdown-503 status survives a
  force-closed socket (now documented).
- Move parseHoldSeconds/parseSinceSeconds/agentScreenMatches off the
  public export surface into an explicit `__internal` namespace; they are
  exposed only for unit tests, not as supported API.
@zpyoung
zpyoung merged commit ba14ac4 into main Jul 9, 2026
3 checks passed
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.

2 participants