Files
turnstone/tests/test_coordinator_page.py
T
Patrick Buckley 1f271789b3 fix(approve): global judge poll + Copilot round-2 feedback
Bug: LLM judge verdicts stayed stuck on heuristic-only render.
Root cause: per-row poller called scheduleLiveFetch which
short-circuits on non-visible rows — invalidate cleared the
cache, no fetch fired, the row kept rendering its last-cached
heuristic indefinitely. The 12s attempt cap also gave up before
slow LLM judges (>15s with reasoning effort) could land.

Replaced with a single global poller _maybeStartJudgePoll /
_judgePollTick:
  - Walks the full childrenState (not just visible rows)
  - Bypasses scheduleLiveFetch's visibility + TTL gates by
    adding to pendingLiveIds directly + flushing
  - One bulk request covers every pending row per tick
  - Self-terminates when every verdict lands or 90s elapses
    (operator can hit Refresh to retry on a failed judge)
  - 90s cap is wall-clock, not attempt count, so an LLM that
    takes 60s no longer prematurely gives up

Copilot round-2 feedback:

- _proxy_sse with use_service_auth=True silently fell back to
  empty headers when proxy_token_mgr was None, producing a
  retry-storm 401/403 loop. Fail fast with a 503 + clear log
  so the misconfig surfaces immediately.

- Mobile <700px CSS comment claimed buttons "stretch to full
  row width" but the rule keeps flex-direction: row with
  flex: 1 on each, giving 50/50 side-by-side. Updated the
  comment to match the deliberate side-by-side layout
  (stacking would push the action row below preview/disclosure
  on tall envelopes; 50/50 keeps both verbs reachable).
2026-04-27 11:41:14 -07:00

122 lines
5.4 KiB
Python

"""Tests for the /coordinator/{ws_id} HTML page handler.
The handler serves the shared template with the ws_id injected as a
``data-ws-id`` attribute. It does NOT enforce auth on the page itself —
auth gating happens on the API endpoints the page calls (an unauthenticated
visitor lands on the page but all API calls fail).
"""
from __future__ import annotations
import pytest
from starlette.applications import Starlette
from starlette.routing import Route
from starlette.testclient import TestClient
from turnstone.console.server import coordinator_page
@pytest.fixture
def client():
app = Starlette(routes=[Route("/coordinator/{ws_id}", coordinator_page, methods=["GET"])])
return TestClient(app)
def test_valid_ws_id_injects_data_attr(client):
ws_id = "a" * 32
resp = client.get(f"/coordinator/{ws_id}")
assert resp.status_code == 200
assert "text/html" in resp.headers["content-type"]
body = resp.text
# ws_id is injected into the html data-ws-id attribute.
assert f'data-ws-id="{ws_id}"' in body
# Template placeholder is fully substituted.
assert "{{WS_ID}}" not in body
# Sanity: the shared static imports are wired.
assert "/shared/base.css" in body
assert "/static/coordinator/coordinator.js" in body
def test_non_hex_ws_id_returns_400(client):
"""Only hex chars are allowed to avoid HTML injection."""
resp = client.get("/coordinator/not-hex-chars-here")
assert resp.status_code == 400
def test_ws_id_too_long_returns_400(client):
resp = client.get("/coordinator/" + "a" * 65)
assert resp.status_code == 400
def test_uppercase_hex_rejected(client):
# Our ws_ids are lowercase hex; reject mixed/upper to avoid surprises.
resp = client.get("/coordinator/" + "A" * 32)
assert resp.status_code == 400
def test_coordinator_js_exposes_inline_approval_helpers():
"""Smoke guard for the Chunk 3 frontend wiring — the new helper
function names must remain reachable in the served JS so a refactor
accidentally renaming/removing them surfaces here instead of in
production where the children-tree's inline approve/deny buttons
silently stop rendering. Asserts string presence only — no DOM
parsing — since coord.js has no JS test framework today (per the
plan's testing notes)."""
from pathlib import Path
coord_js = Path(__file__).resolve().parent.parent / (
"turnstone/console/static/coordinator/coordinator.js"
)
body = coord_js.read_text(encoding="utf-8")
# Approval-block rendering helpers
assert "function renderApprovalBlock" in body
assert "function _maxSeverityItem" in body
assert "function _renderSubItem" in body
# The submit + 409 race-handling path
assert "function submitChildApproval" in body or "submitChildApproval(" in body
# The shared approve POST helper (parameterized for child ws_ids)
assert "function approveWorkstream" in body or "approveWorkstream(" in body
# The urgent live-bulk fetch option that fires on activity_state
# transitions in/out of "approval"
assert "{ urgent: true }" in body or "urgent: true" in body
# Server-side payload field — drift here means the JS reads stale keys
assert "pending_approval_detail" in body
# Reconnect parity (chunk 4): the SSE re-open handler must drop
# non-permanent entries from the live-badge cache so a stale
# pending_approval_detail (left from before the disconnect)
# can't render zombie approve/deny buttons on a row whose
# approval was resolved during the gap. The implementation
# iterates the cache and deletes only !permanent entries —
# asserting the literal Map iteration form keeps a refactor
# back to liveBadgeCache.clear() (which would re-pay 403s on
# every reconnect for denied ids) from sneaking in.
assert "liveBadgeCache.delete" in body
# Edge-case matrix sentinel labels — POLICY-BLOCKED renders when
# an item has error set + needs_approval=False (server-side
# tool policy already blocked the call); "(judge unavailable)"
# renders when no verdict (judge or heuristic) and no
# judge_pending. Refactors that drop either branch silently
# regress to a buttoned approve UI on the wrong state.
assert "POLICY-BLOCKED" in body
assert "judge unavailable" in body
# Critical-risk handling — bug-1 was that risk_level='critical'
# rendered as low because RISK_SEVERITY only mapped 'crit'.
# Both aliases must remain in the table so a 'critical' verdict
# ranks at 3 and renders with the .risk.crit pill.
assert "critical: 3" in body
# Child approves must round-trip through the routing proxy at
# /v1/api/route/workstreams/{ws_id}/approve — the bare
# /v1/api/workstreams/.../approve path only works for the
# coord-self ws_id (the coord lives on the console process).
# Children live on cluster nodes and 404 without the prefix.
assert "/v1/api/route/workstreams/" in body
# Late-judge polling — the LLM judge runs async on the child
# node and never pushes a signal that reaches the coord, so
# the row's pending_approval_detail with judge_pending=true
# would freeze on heuristic verdicts forever without this
# poll loop. The poller is GLOBAL (not per-row) so off-screen
# rows still refresh — a per-row poller's scheduleLiveFetch
# call short-circuits on non-visible rows, leaving them stuck.
assert "_maybeStartJudgePoll" in body
assert "_judgePollTick" in body