Skip to content

Add live run lists, detail pages and transcript tails - #149

Merged
artyomsv merged 2 commits into
masterfrom
feat/live-runs-and-accounts
Sep 11, 2026
Merged

Add live run lists, detail pages and transcript tails#149
artyomsv merged 2 commits into
masterfrom
feat/live-runs-and-accounts

Conversation

@artyomsv

@artyomsv artyomsv commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Runs previously required a refresh to show progress and had no detail screen. This adds a live list and linked run pages showing recorded phases, runtime, spend, task summary, delivery and a live transcript.

  • Add the nullable agent-start timestamp without backfilling historical runs. Broadcast committed run changes, including separate outcome, spend and PR updates, through /api/ws/runs.
  • Keep list filters local, disclose and enforce the newest-200 window, and refetch detail on relevant pushes. Preserve unknown cost, missing timestamps and unknown statuses.
  • Consume the existing transcript socket with sequence ordering/deduplication, All/Agent/System/Errors filters, scroll-aware tail control, session-aware reconnects and terminal policy-close handling.
  • Bound the live transcript to 2,000 events. Add an optional exclusive before cursor to the REST transcript so the earlier-history link retrieves the preceding page.

Validation:

  • CI service tests and packaging passed on implementation commit e228dea (the original local packaging attempts were inconclusive).
  • Initial ./gradlew testFast testServices on e228dea: passed; aggregate reports contain 2,938 tests across 331 suites, zero failures/errors and one existing skip. Unchanged tasks reused their results.
  • npm test: 617 tests across 75 files passed; npx tsc --noEmit: clean.
  • Required mutations fail their intended tests: first agent start, broadcast coverage, unknown/archived spend, expired-session reconnect, splat routing, unknown transcript kinds, the newest-200 cap and serialization warning level.
  • Playwright checked the list → detail → transcript flow against real orchestrator REST/WebSocket endpoints and PostgreSQL, including live queue/status/PR updates and desktop/narrow layouts. Sibling session/attention endpoints were stubbed; no real agent execution or remote PR creation is claimed. Nightly E2E was not run.

Implementation decisions and verification details: live-runs record.

Review follow-up validation:

  • All four human findings addressed: visible/bounded filter window; cheap existence queries at all three sites; warning-level serialization failures; explicit, documented spend read.
  • 349 factory tests and the local real-HTTP/WebSocket browser probe passed (350 including the probe).
  • The two Semgrep test-literal findings are removed. HTTP/HTTPS protocol behavior is now explicitly tested for both sockets; the CI-pinned scanner ran 329 applicable rules on eight affected files with zero findings.

Closes #147
Closes #133
Closes #134
Closes #135
Closes #136
Closes #137
Closes #138

Broadcast committed run and spend changes so operators can follow progress
without refreshing. Record the first observed agent start and expose run
definition and token spend while preserving unknown values.

Add encoded detail routes and bounded, filtered transcript tails with
session-aware reconnects and a REST cursor for earlier history. Cover the
read models, sockets and UI with regression tests and mutation checks.

Signed-off-by: Artjoms Stukans <artjoms.stukans@gmail.com>
Comment thread spire-ui/src/hooks/useRunTranscript.test.tsx Fixed
Comment thread spire-ui/src/useLiveRuns.test.tsx Fixed

@artyomsv artyomsv left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Review — the epic is delivered; one regression to settle before merge

All six sub-tasks are implemented and their named mutation checks are real. Verified locally on e228dea:

Check Result
npx vitest run 613 passed / 75 files — matches the claim
styles.contract.test.ts + App.routes.test.tsx green
:spire-orchestrator:test --tests '…factory.*' BUILD SUCCESSFUL
New suites EveryRunWriteIsBroadcastTest 1 · RunsBroadcasterTest 3 · RunAgentStartedTest 5 · RunDetailReadTest 9 · RunsSocketTest 11 · RunChargesTest 13 — 0 failures

What is notably well done

  • EveryRunWriteIsBroadcastTest is a real guard, not a ceremony. It parses the javac AST, resolves SQL held in field constants, builds the local call graph, and falls back to "every caller of this private writer must push". A writer with no callers fails. assertFalse(writers.isEmpty(), …) stops it passing on an unreadable source — that is the failure mode this project has been bitten by.
  • The cost-push gap was found and closed. RunResultSaga projects before it charges, so the outcome push carries no cost. RunCharges now pushes again after the ledger commits, which is the only thing that keeps a FIX run's cost live — a FIX run has no later PR write to piggyback on. The stale comment in ReviewProjection that predicted this was updated rather than left to rot.
  • RunSpend.cost() is honest. No charge lines → unknown. Any unpriced line → unknown. Never zero. ADR-023 survives to the screen.
  • The splat decoding is tested where it matters. RunDetail.test.tsx drives /runs/${encodeURIComponent(id)} with an id carrying both / and :, asserts getRun was called with the decoded id, and adds a case with a literal %41 that a second decode would corrupt. That is the discriminating case, not the obvious one.
  • queuedTime is correct. Date.parse ties on microsecond-precision PostgreSQL timestamps; the fraction slice recovers sub-millisecond nanoseconds, and the tiebreak (run_id descending) matches the server's ORDER BY r.started_at DESC, r.run_id DESC exactly.

Findings

# Where Severity
1 useLiveRuns.ts:33 / Runs.tsx:147 fix before merge — a filtered screen says "No matching runs" when matching runs exist
2 RunResource.java:317,567 + RunTranscriptSocket.onOpen worth fixing — three existence checks now pay for a spend aggregate
3 RunsBroadcaster.java:38 worth fixing — a permanently unsendable row is silent in production
4 FactoryRunProjection.java:970 nit

Detail is inline. Nothing here loses data and nothing blocks the feature; #1 is the only one that makes a screen state something untrue.

Epic status

#133 #134 #135 #137 #138 are satisfied. #136 is satisfied apart from finding 1, which is a consequence of the client-side filter that issue chose.

Comment thread spire-ui/src/useLiveRuns.ts Outdated
Disclose and enforce the newest-200 filter window, avoid spend reads for existence checks, and warn on permanent serialization failures. Cover the regressions and both WebSocket protocols without insecure URL test literals.

Signed-off-by: Artjoms Stukans <artjoms.stukans@gmail.com>

@artyomsv artyomsv left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Re-review of 2b6d159GO

All four findings are fixed, each with a test, and the two that mattered were mutation-verified here rather than taken on trust. Every review thread is resolved.

What was fixed

# Fix Test
1 MAX_LIVE_RUNS is one exported constant driving the REST limit, the trim and both strings. The window is disclosed beside the filters, the empty state reads No matching runs in the newest 200, and sortRuns ends .slice(0, MAX_LIVE_RUNS) — which also closes the unbounded-growth half. 300 rows, the only cancelled at index 249, asserts the new wording, asserts the old wording is gone with { exact: true }, pushes the out-of-window run and asserts it does not appear, pushes an in-window change and asserts it does, asserts one getRuns call. Plus a 202-row trim test across REST, snapshot, older update and new arrival.
2 exists(runId) is SELECT 1 …; all three existence checks moved to it — resolveDispatch, transcript, RunTranscriptSocket.onOpen. find() is left to the one caller wanting the view. existenceDistinguishesAnEmptyRunFromAnUnknownId
3 Catch split: JsonProcessingException warns, RuntimeException stays DEBUG. anUnserialisableRowWarnsWithoutFailingTheProjection attaches a handler and asserts the level, the run id in the message and the throwable type — it verifies the warning, not merely the absence of a throw.
4 Spend read hoisted to a local, with the PgJDBC guarantee written down. unchanged behaviour, so nothing new needed

Verified here, not read

Check Result
npx vitest run 617 passed / 75 files (613 → 617)
:spire-orchestrator:test factory + ws BUILD SUCCESSFUL — 349 tests, 0 failures, 0 errors
Mutation: drop .slice(0, MAX_LIVE_RUNS) 2 failed / 34 passed — exactly the two intended
Mutation: restore the combined JsonProcessingException | RuntimeException catch 4 tests, 1 failed — exactly anUnserialisableRowWarnsWithoutFailingTheProjection
Working tree after both mutations clean, re-run green

CI is green on every required check, including service tests + packaging (3m28s). That closes the one uncertainty the PR description left open — packaging was interrupted locally and is now proven on a runner, so no claim in this PR rests on an unfinished local build.

Two Semgrep findings on insecure ws:// literals in the new tests were also fixed in passing: both socket tests now parameterise over http:/https: and assert the derived ws:/wss: protocol, which is a better test than the literal it replaced.

Not a blocker, but say it out loud

  • The run detail page is read-only. Cancel, steer and resolve-dispatch stay curl. That was #137's explicit scope decision and it is tracked in techdebt/spire-ui/4-3-three-factory-surfaces-still-have-no-screen.md, which is now the only factory screen gap left.
  • Nightly testE2e was not run, as the description says. That tier drives a containerised GitLab and reaches none of this code, so it is the right thing to skip — noted only so the gap is on the record rather than assumed away.
  • agent_started_at is NULL for every existing run and always will be. The phase strip says not recorded, which is the honest answer; nobody should later "fix" it with a backfill.

Merge

mergeStateStatus is BLOCKED on REVIEW_REQUIRED — branch protection wants an approving review, and this review cannot supply it because the reviewing account is the author. A human approval is the only thing standing between this and merge. Everything reviewable is done.

@artyomsv
artyomsv merged commit 598f98f into master Sep 11, 2026
14 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

2 participants