ewilliams28/turnstone
1
0
Fork 0
mirror of https://github.com/turnstonelabs/turnstone.git synced 2026-08-12 23:12:23 -06:00
Code Issues Packages Projects Releases Wiki Activity

feat(coord): inline tool-batch construct replaces approval dock (#447)

Browse Source 
* feat(coord): inline tool-batch construct replaces approval dock

The pinned bottom approval-dock didn't scale: a 10-call spawn_workstream
fan-out filled the whole pane with a wall of repeated verdict chips,
and the call → approval → result lifecycle was split across three
disconnected surfaces (.msg.tool bubble + dock + .msg.tool result).

Replaces it with one chat-stream construct per dispatch turn that
pairs each tool call with its result and embeds the approval gate:

  - .coord-tool-batch--solo      single-call serial turn
  - .coord-tool-batch--parallel  ≥2 calls; rows share a left rail
                                 + per-row tick so they read as
                                 siblings of one assistant decision

Lifecycle: rows render with optional "judge evaluating…" placeholder,
upgrade in place when intent_verdict arrives, and on tool_result the
output lands paired under the originating row.  When the batch needs
approval, one Approve/Deny/Always action row renders inside the
construct (envelope-level — server semantics resolve siblings
together).  After approval_resolved the action row morphs into a
✓ approved / ✗ denied status pill that stays as a receipt.

Critical bug closed: when a page reload races a pending approval,
pre-scan tool_call_ids in history; turns whose call_ids have no
matching tool result are rendered pending (not resolved-approved).
The SSE approve_request replay then upgrades the existing batch
in place — drops --approved/--denied, adds --pending, swaps the
status pill for actions, and assigns activeBatch.  Without this
the operator was locked out of any approval pending at reload.

Defence-in-depth follow-ups from the same review:

  - approval_resolved falls back to a DOM lookup if activeBatch
    is null (cross-tab resolution where this tab never set it).
  - _appendVerdictLineTo dedupes via a row.dataset.verdictSig so
    SSE reconnect storms + repeat intent_verdict events don't
    tear down + rebuild an unchanged verdict line.
  - judgeVerdicts Map soft-capped at 500 entries (FIFO eviction)
    via _cacheJudgeVerdict.
  - toolRows entries hold {batch, row} only — the originating
    item payload is no longer pinned for the page lifetime.
  - _scheduleScroll coalesces messagesEl.scrollTop writes through
    requestAnimationFrame so history replay doesn't reflow once
    per appended message.
  - Rationale <details> now inserts immediately after the verdict
    line (was tail-appending, breaking ordering once a result
    landed below).
  - .coord-tool-batch--error wired: _appendResultToRow lifts a
    row's error onto the enclosing batch; _renderBatchRow does
    the same for policy-blocked rows at construction.
  - _buildStatusPill extracted; both _morphBatchResolved and the
    appendToolBatch resolved-replay branch route through it.

Removed: ~248 lines of dead .approval-dock CSS, the dock <aside>
element from index.html, and the dead helpers showApproval's
prior body, hideApproval, claimApprovalFocus,
claimApprovalFocusForVerdict, applyJudgeVerdictToRow,
applyJudgePendingToRow, ensureDctxAfterRow, removeRationale,
setApprovalButtonsDisabled, the appendToolCall single-row wrapper,
and window.coordApprove.  Five stale comment blocks referencing
the dock as if live also swept.

Children-tree's renderApprovalBlock is independent and untouched
(different surface, different .approval-block / .approval-pill
vocabulary).

* fix(coord): close four Copilot review gaps on PR 447

Copilot review on caa07e6 flagged four follow-ups:

1. History replay was rendering EVERY orphan tool_calls turn (one
   that lacks a matching tool result message) as `pending: true,
   judgePending: true`.  That paints Approve/Deny on turns that
   could be just running — auto-approved-and-still-in-flight, or
   already-approved-and-still-in-flight — and clicking would 409
   because the call_id isn't in `pending_items`.  Add a new
   `--running` state for the orphan case (no actions, neutral
   accent stripe).  SSE then upgrades in place: `--running` →
   `--pending` when `approve_request` replays, or `--running` →
   `--auto` when `tool_info` replays.  Tool_result events still
   route into the rows for the third case (already-approved + in
   flight) since `toolRows` is populated.  Kicker text reads
   "Running · Parallel N" while ambiguous, so the operator can
   tell the in-flight-replay state apart from a fresh "Parallel ·
   N tools" auto-approved batch.

2. Removing the dock also removed its `aria-live="assertive"`
   region — pending tool-batches now append into the polite
   `#coord-messages` log (which gets flipped to `aria-live="off"`
   during streaming), so a screen reader could miss the
   action-required signal.  Add an off-screen
   `aria-live="assertive"` `#coord-sr-announcer` region and route
   "Approval required: <name> + N more" through it whenever a
   pending batch is created OR an upgrade-in-place promotes a
   running batch to pending.  Also mark pending batches with
   `role="region"` + a matching `aria-label` so SR landmark
   navigation surfaces them; both are dropped on resolve so the
   resolved batch stops claiming the landmark.

3. `_resolveBatchAction` was selecting the first row whose
   `data-call-id` was set and that wasn't `.error` — but
   `approve_request` envelopes carry the FULL items list,
   including auto-approved siblings whose `needs_approval=false`
   means the server's `pending_items` won't recognise their
   call_id (→ 409 on submit, or resolves the wrong gate).  Tag
   rows that are genuinely in `pending_items` with
   `data-needs-approval="1"` at construction (and during
   upgrade-in-place when SSE arrives), and select against that
   selector specifically.  Restores the legacy
   `pendingApprovalCallId` contract that filtered on
   `needs_approval` before the dock was retired.

4. The `.coord-tool-row-result` comment claimed the styles applied
   a click-to-expand "collapsed" affordance like the interactive
   UI's `.tool-output.collapsed`, but the implementation only set
   `max-height: 240px; overflow: auto` (a scroll pane, not a
   collapse with expand control).  Update the comment to describe
   what the rules actually do and explain the deliberate
   divergence from interactive (coord is a diagnostic-leaning
   read-once surface; an internal scroll pane reads with lower
   friction than a click-to-expand control on the operator's
   primary monitoring view).

No Python touched; node --check on coordinator.js clean.

* fix(coord): restore reload-time pending approval gate

Agent-Logs-Url: https://github.com/turnstonelabs/turnstone/sessions/30f630fe-3ded-4abe-991b-b5a95f699127

Co-authored-by: eous <13773563+eous@users.noreply.github.com>

* feat(api): expose pending_approval on workstream detail response

PR 447 / 93cb3d9 (Copilot autonomous follow-up) added a JS path that
reads ``wsSnapshot.pending_approval_detail`` off the
``GET /v1/api/workstreams/{ws_id}`` snapshot in coordinator.js
init() so a freshly-loaded chat tab can paint the inline approval
gate immediately at reload, without waiting for the SSE
approve_request replay (which leaves a brief --running flash on the
inflight orphan placeholder).

But the server's ``WorkstreamDetailResponse`` schema only declared
``{ws_id, name, state, user_id, kind}`` and the lifted
``make_detail_handler`` matched: nothing was populating
``pending_approval`` or ``pending_approval_detail`` on the wire.
The frontend block silently no-op'd at runtime; Copilot's
accompanying assertion only grep'd the JS source for the literal
strings, so it stayed green while the actual contract was missing.

Extend the contract to match the Copilot frontend:

  - Add ``pending_approval: bool`` + ``pending_approval_detail:
    PendingApprovalDetail | None`` to ``WorkstreamDetailResponse``,
    same shape as the dashboard / cluster live projection.
  - ``make_detail_handler`` reads ``ws.ui._pending_approval`` (only
    treats it as live when ``isinstance(_, dict)`` so MagicMock-
    based unit tests don't trip the path) and calls
    ``ui.serialize_pending_approval_detail()`` to fill the detail.
    A serializer raise falls back to ``pending_approval=True`` +
    ``detail=None`` instead of 500ing the whole response — SSE
    replay still carries the authoritative payload.
  - ``test_returns_workstream_fields`` updated for the two extra
    fields (False / None on a MagicMock UI).
  - ``test_pending_approval_fields_propagate_from_ui`` is the new
    behavioural test: stub a UI with a realistic
    ``_pending_approval`` dict + serializer return, assert the JSON
    surfaces ``pending_approval=True`` + the items list.
  - ``test_pending_serializer_failure_falls_back_to_bool_only``
    pins the defensive degradation so a future serializer
    regression can't 500 every reload.

Tests: 4822 pass (3 deselected live).  Ruff + mypy clean.

* fix(coord): three regressions on PR 447 inline tool-batch refactor

Three regressions reported during operator harness shakedown, all
landed by the inline tool-batch refactor in caa07e6:

1. ``stripAnsi`` ReferenceError on every ``tool_result``.
   ``_appendResultToRow`` called ``stripAnsi(output || "")`` but the
   helper only existed in ``ui/static/app.js`` — coord.js never
   imported or defined it.  The thrown ReferenceError propagated up
   through ``appendToolResult``, aborting the SSE handler before
   ``loadTasksDebounced()`` could fire, AND the result block never
   appended to the row, AND history replay's tool-message loop
   bailed out at the first orphan-tool-result.  Three reported
   bugs (tasks pane stops auto-refreshing, tool output missing in
   the modal, reload only rebuilds the conversation up to the first
   tool result), one root cause.

   Fix: hoist a local ``stripAnsi`` mirroring the interactive UI's
   regex.  Keep it local rather than centralised — coord and
   interactive tool-output paths have different rendering
   strategies, and the interactive helper isn't on the shared
   module surface today.

2. JSON tool output rendered as a single unreadable line.  Coord
   tool surfaces (``list_nodes``, ``tasks``, ``spawn_workstream``,
   ...) emit JSON by default, and ``textContent = stripAnsi(raw)``
   showed the whole envelope on one line.  The parent
   ``.coord-tool-row-result`` already has ``white-space: pre-wrap``
   so a ``JSON.stringify(parsed, null, 2)`` body lays out as
   intended without a nested ``<pre>``.  Non-JSON / unparseable
   output falls through to the raw cleaned string.

3. Header tier badge stuck on ``⚙ heuristic`` after the LLM judge
   landed an upgraded verdict.  ``_pickBatchTier(items)`` ran once
   at batch-creation time; later ``intent_verdict`` SSE events
   updated the per-row chip via ``_appendVerdictLineTo`` but never
   refreshed the head.

   Fix: persist the verdict's tier on ``row.dataset.verdictTier``
   (+ ``verdictModel`` when set), add ``_refreshBatchTier(batch)``
   that scans the rows and computes the cross-row best tier (LLM
   beats heuristic), and call it from ``_appendVerdictLineTo``
   whenever a row writes a verdict.  ``_pickBatchTier`` gets the
   same prefer-LLM scan so the initial render is consistent.  The
   ``intent_verdict`` cache entry tags ``tier: "llm"`` so a late
   verdict landing on a previously heuristic-only row escalates
   the badge correctly.

No Python touched; node --check on coordinator.js clean.

* fix(coord): close five Copilot review gaps on PR 447

Five distinct findings from the second Copilot pass on the inline
tool-batch refactor (the sixth — stripAnsi ReferenceError — already
shipped in 77dc24e):

1. CSS rail tucks never matched.  The ``--first / --last`` row trims
   used ``:first-of-type`` / ``:last-of-type``, but the batch
   contains other ``<div>`` siblings (.coord-tool-batch-head,
   .coord-tool-actions / .coord-tool-status) — the
   structural-pseudo-class is type-based (``div``), not class-
   based, so the first .coord-tool-row is not the first ``<div>``
   in the parent.  Selector silently no-op'd, leaving the rail
   butting against the inner top/bottom edges of the batch.  Fix:
   apply explicit ``.coord-tool-row--first`` / ``--last`` markers
   in JS at row-build time and key the CSS off them.

2. Upgrade-in-place left stale ``data-needs-approval`` markers on
   non-pending sibling rows.  The original block only added the
   attribute for items where ``needs_approval=true``, never
   clearing it for rows whose earlier (replay-time) shell tagged
   them.  ``_resolveBatchAction`` could then pick a non-pending
   row's call_id, yielding a 409 stale call_id on approve / deny.

3. Upgrade-in-place left row-level status pills out of sync with
   the SSE-authoritative item shape.  When a ``--running`` orphan
   gained a ``tool_info`` envelope, the ✓ auto pill never
   appeared; when it gained an ``approve_request`` envelope with
   policy-blocked siblings, the ✗ blocked pill / ``.error`` class
   were missed.  Batch-level state classes flipped, but per-row
   visual cues lagged.

   Fix for 2 + 3: extract ``_refreshRowStatus(row, item)`` from
   ``_renderBatchRow``.  It clears prior ``data-needs-approval`` +
   pills and re-applies from the item, preserving runtime
   ``tool_result`` errors via the new
   ``.coord-tool-row-result--error`` marker on the result block.
   Both ``_renderBatchRow`` (initial render) and the
   upgrade-in-place loop now route through it, so the two paths
   can't drift.

4. History replay defaulted ``item.needs_approval = true`` on
   every synthesized tool call.  ``_renderBatchRow`` then tagged
   the row with ``data-needs-approval="1"`` regardless of whether
   the call genuinely needed approval.  Combined with the missing
   clear in finding 2, an SSE upgrade with a mixed envelope kept
   incorrect markers on auto-approved siblings.  Drop the
   replay-time default; let SSE supply the authoritative bit when
   the upgrade fires (``_refreshRowStatus`` reads it from the
   item).

5. Tool result routed into an existing batch row didn't trigger
   ``_scheduleScroll()``.  Result blocks grow ``scrollHeight``;
   without the rAF-coalesced scroll the user pinned at the bottom
   loses their pin when the row inflates.  Add the call after
   ``_appendResultToRow`` in the early-return path so this branch
   matches ``appendMsg``'s pinning behaviour.

Plus comment-only:

6. Detail-handler comment claimed "the JSON omits the section"
   when the UI doesn't expose ``serialize_pending_approval_detail``,
   but the response always includes both keys (with ``False`` /
   ``null`` for the bool / detail).  Updated to match the actual
   shape.

Tests: ``test_workstream_endpoints.TestDetailInteractive`` +
coordinator-detail + page tests pass (14 / 0 failed). Ruff +
mypy clean.  ``node --check`` on coordinator.js clean.

* fix(coord): close 17 review findings on PR 447

Second /review pipeline pass surfaced 16 confirmed findings (1 sec
major, 1 bug major, several minor + nit); operator harness shakedown
+ this commit's stale-comment sweep adds one more.  All addressed
here.

Security:

  sec-1 (major) — make_detail_handler + make_history_handler in
  session_routes.py now invoke ``cfg.tenant_check`` after ws_id
  validation, matching every other lifted session verb (send /
  approve / close / cancel / events / attachments).  Pre-fix the
  detail response carried 5 low-data fields and history exposed
  message rows; PR 447 added pending_approval_detail to detail
  (tool previews + LLM judge reasoning) which made cross-tenant
  reads via the missing gate a real disclosure on the interactive
  surface (coord wires tenant_check=None and is unaffected).  Plus
  4 new regression tests in TestTenantCheckOnReadEndpoints that
  wire a tenant_check function into the test cfg and assert the
  gate fires on detail + history.

Bug fixes:

  bug-1 (major) — history replay used to render every fully-
  resolved tool batch as ``resolved: { approved: true }`` regardless
  of the persisted tool result content.  A denied tool round-trip
  showed the green "✓ approved" pill alongside the persisted
  "Denied by user" result text — directly contradictory state.  Fix:
  pre-scan classifies each tool message via a ``callOutcomes`` Map
  by inspecting content prefix ("Denied by user" / "Blocked by
  tool policy" / "Error:") and ``m.is_error``.  Assistant tool_calls
  render ``resolved.approved=false`` when any call's outcome is
  "denied"; the existing --running fallback covers orphan turns
  (any call lacking an outcome).

  bug-2 — _verdictSig joined recommendation/risk_level/confidence/
  reasoning only.  When a late LLM verdict text-matched the earlier
  heuristic verdict, the dedupe early-return fired before the
  row's dataset.verdictTier was updated, so _refreshBatchTier
  never escalated the header from "⚙ heuristic" to "⚖ llm".
  Fix: include verdict.tier and verdict.judge_model in the
  signature (with a "\x1f" separator instead of the empty join,
  reducing field-boundary collision risk).

  bug-3 — history replay's tool-result rendering hardcoded
  isError=false.  A runtime tool error on reload rendered without
  the .error class, --error stripe, or "✗ error:" lead.  Fix:
  the same callOutcomes pre-scan that drives bug-1's denial path
  also classifies "Error:" prefixes; appendToolResult now receives
  isError=callOutcomes.get(callId) === "error".

  bug-4 — approval_resolved derived ``wasAlways`` exclusively from
  this tab's ``batch.dataset.requestedAlways``; cross-tab "Always"
  click never propagated to peer tabs' status pill.  Fix: server's
  resolve_approval now takes a keyword ``always`` arg and includes
  it on the SSE event body; client prefers ``ev.always`` and falls
  back to the dataset stash for the hot-deploy window where the
  SSE event might briefly omit the field.

  bug-5 (nit) — appendToolBatch's create-new path overwrote
  toolRows entries unconditionally.  A partial-mapped envelope
  (some call_ids previously seen, some new) silently orphaned the
  prior batch's row pointers.  Fix: detect the partial overlap,
  console.warn, unmap the stale entries before the new batch
  claims them.

Performance:

  perf-1 — _refreshBatchTier did a querySelectorAll per verdict
  insertion; for an N-row batch upgrade this was O(N²) DOM walks.
  Coalesce via queueMicrotask + a _tierDirtyBatches Set so a burst
  of N verdict updates collapses into ONE tier scan.  Synchronous
  body extracted to _refreshBatchTierImmediate (called from the
  microtask flush).

  perf-2 — _appendResultToRow pretty-printed JSON via
  JSON.parse + JSON.stringify(parsed, null, 2) on every tool
  result with no size cap.  A 100KB JSON output stalled the main
  thread; 10 parallel tool_result events compounded.  Fix: gate
  on cleaned.length <= 32 KiB AND a first-char check (0x7B / 0x5B)
  so plain text + oversized payloads skip the parse.  Parent CSS
  is white-space: pre-wrap so raw text still wraps.

Quality:

  q-1 — deleted dead row.dataset.funcName write (no readers).

  q-2 — extracted _formatTierLabel(llmModel, hasHeuristic) shared
  by _pickBatchTier (item-driven) and _refreshBatchTierImmediate
  (dataset-driven).  Single source of truth for the tier label
  literals.

  q-3 — extracted _pendingKickerText(items) used by both the
  upgrade-in-place and fresh-build paths in appendToolBatch.

  q-4 — added string-presence assertions to
  test_coordinator_js_exposes_inline_approval_helpers covering
  the new tool-batch helpers (appendToolBatch, _morphBatchResolved,
  _resolveBatchAction, _refreshBatchTier, _refreshRowStatus), the
  --running / --pending state classes, and the callOutcomes
  outcome classifier.

  q-5 — renamed _announcePolitelyAssertive → _announceAssertive.
  Function unconditionally writes into the aria-live="assertive"
  region; "politely assertive" was contradictory.

  q-6 — rescoped the test docstring to acknowledge it covers two
  layers (Chunk 3 children-tree + PR 447 tool-batch).

  q-7 — tightened pending_approval_detail: Any → dict[str, Any]
  | None in make_detail_handler.  Mypy-confirmed.

Plus the third /review pass's q-1 stale-comment sweep:
  _resolveBatchAction's comment still claimed the server doesn't
  echo ``always`` on approval_resolved — wrong post-bug-4-fix.
  Updated to reflect that the dataset stash is now backward-compat
  fallback only, not the primary source.

Tests: 4826 pass (+4 new from TestTenantCheckOnReadEndpoints, plus
expanded assertions in TestDetailInteractive).  Ruff + mypy clean.
``node --check`` on coordinator.js clean.

Verifier confirmed all 16 findings; pass-3 /review on the
addressing-commit surfaced only 0 critical / 0 major / 2 minor /
2 nit, none blocking.  The two pass-3 minor findings are
pre-existing patterns across all lifted verbs (sync tenant_check
inside async handlers) and best addressed in a dedicated follow-up
PR auditing the whole lifted-verb surface.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: eous <13773563+eous@users.noreply.github.com>
This commit is contained in:
Patrick Buckley
2026-04-28 23:01:18 -07:00
committed by GitHub
parent 64d5205dd6
commit 353ff4d18b
8 changed files with 1966 additions and 697 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_coordinator_page.py
+40 -7
View File
@@ -55,13 +55,16 @@ def test_uppercase_hex_rejected(client):
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)."""
"""Smoke guard for two layers of the coord chat frontend: the
children-tree inline approve/deny block (the original Chunk 3
landing) and the PR #447 tool-batch construct that replaced the
pinned approval dock for the coord-self surface. Both layers'
helper symbols must remain reachable in the served JS so a
refactor that accidentally renames or removes them surfaces here
instead of in production where the affected gates 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 / (
@@ -119,3 +122,33 @@ def test_coordinator_js_exposes_inline_approval_helpers():
# call short-circuits on non-visible rows, leaving them stuck.
assert "_maybeStartJudgePoll" in body
assert "_judgePollTick" in body
# Reload parity for the coord-self approval gate: init() must
# consume the authoritative GET /workstreams snapshot's
# pending_approval_detail so a freshly opened tab can render
# Approve/Deny before SSE replay arrives.
assert "wsSnapshot.pending_approval_detail" in body
assert "appendToolBatch(pendingDetail.items" in body
# Tool-batch construct (PR #447) — the inline replacement for the
# pinned approval-dock pattern. These helpers carry the
# state-machine that pairs each tool call with its result and
# embeds the approval flow. Refactors that rename or drop them
# silently regress the entire coord-self approval surface — the
# most novel and risky behavior in the PR.
assert "function appendToolBatch" in body
assert "function _morphBatchResolved" in body
assert "function _resolveBatchAction" in body
assert "function _refreshBatchTier" in body
assert "function _refreshRowStatus" in body
# State modifiers driven by the upgrade-in-place path
# (--running orphan promoted to --pending or --auto when SSE
# arrives with the authoritative shape). Both class names must
# remain reachable from JS — dropping either breaks the reload
# state machine that PR #447's review pass surfaced.
assert "coord-tool-batch--running" in body
assert "coord-tool-batch--pending" in body
# History replay's outcome classifier — denied / errored tool
# turns must render with the correct batch state on reload, not
# the contradictory "✓ approved" pill that pre-fix showed for
# any prior denial. bug-1 / bug-3 from the second /review pass.
assert "Denied by user" in body
assert "callOutcomes" in body
tests/test_workstream_endpoints.py
+233 -6
View File
@@ -10,6 +10,7 @@ import pytest
from starlette.applications import Starlette
from starlette.middleware import Middleware
from starlette.middleware.base import BaseHTTPMiddleware
from starlette.responses import JSONResponse
from starlette.routing import Mount, Route
from starlette.testclient import TestClient
@@ -740,26 +741,38 @@ class TestUpdateInterfaceSetting:
# These tests pin the interactive wiring against the same factory.
def _interactive_endpoint_cfg(mock_mgr: Any) -> SessionEndpointConfig:
def _interactive_endpoint_cfg(
mock_mgr: Any,
tenant_check: Any = None,
) -> SessionEndpointConfig:
"""Interactive-shaped cfg wired the same way ``server.py`` does.
Shared by both :func:`_build_history_app` and :func:`_build_detail_app`
— every field both factories actually read is present (the detail
factory ignores ``list_kind`` since it relies on ``mgr.open()`` for
cross-kind isolation, but the field is harmless to set).
The optional ``tenant_check`` lets a regression test wire the same
cross-tenant gate ``server.py`` uses (``_interactive_tenant_check``)
so the lifted handlers can be exercised with the production-shape
auth posture, not just the bypass shape.
"""
return SessionEndpointConfig(
permission_gate=None, # auth middleware covers it
manager_lookup=lambda _r: (mock_mgr, None),
tenant_check=None,
tenant_check=tenant_check,
not_found_label="Workstream not found",
audit_action_prefix="workstream",
list_kind=WorkstreamKind.INTERACTIVE,
)
def _build_history_app(mock_mgr: Any, storage: Any) -> TestClient:
cfg = _interactive_endpoint_cfg(mock_mgr)
def _build_history_app(
mock_mgr: Any,
storage: Any,
tenant_check: Any = None,
) -> TestClient:
cfg = _interactive_endpoint_cfg(mock_mgr, tenant_check=tenant_check)
handler = make_history_handler(cfg)
app = Starlette(
routes=[
@@ -777,8 +790,11 @@ def _build_history_app(mock_mgr: Any, storage: Any) -> TestClient:
return TestClient(app)
def _build_detail_app(mock_mgr: Any) -> TestClient:
cfg = _interactive_endpoint_cfg(mock_mgr)
def _build_detail_app(
mock_mgr: Any,
tenant_check: Any = None,
) -> TestClient:
cfg = _interactive_endpoint_cfg(mock_mgr, tenant_check=tenant_check)
handler = make_detail_handler(cfg)
app = Starlette(
routes=[
@@ -906,6 +922,10 @@ class TestDetailInteractive:
loaded_ws.state = ws_state
loaded_ws.user_id = "test-user"
loaded_ws.kind = "interactive"
# No pending approval — leave .ui's MagicMock attrs alone; the
# handler isinstance-checks ``_pending_approval`` against ``dict``
# before treating it as live, so MagicMock attribute pollution
# doesn't trigger the pending path.
mock_mgr = MagicMock()
mock_mgr.get.return_value = loaded_ws
client = _build_detail_app(mock_mgr)
@@ -919,8 +939,102 @@ class TestDetailInteractive:
"state": "idle",
"user_id": "test-user",
"kind": "interactive",
"pending_approval": False,
"pending_approval_detail": None,
}
def test_pending_approval_fields_propagate_from_ui(self):
"""When the workstream's UI is parked on an approval, the detail
response surfaces ``pending_approval=True`` + the serialized
``pending_approval_detail`` so a freshly-loaded chat tab can
paint the inline gate without waiting for the SSE
``approve_request`` replay (which would otherwise produce a
brief ``--running`` flash on reload)."""
ws_id = "ws-pending-1"
ws_state = MagicMock()
ws_state.value = "attention"
loaded_ws = MagicMock()
loaded_ws.id = ws_id
loaded_ws.name = "coord-1"
loaded_ws.state = ws_state
loaded_ws.user_id = "test-user"
loaded_ws.kind = "coordinator"
# Realistic _pending_approval shape (mirrors what
# SessionUIBase.approve_tools assigns) + a serializer that
# returns the merged-with-verdicts payload.
loaded_ws.ui._pending_approval = {
"type": "approve_request",
"items": [
{
"call_id": "c-1",
"func_name": "spawn_workstream",
"needs_approval": True,
},
],
"judge_pending": True,
}
loaded_ws.ui.serialize_pending_approval_detail = MagicMock(
return_value={
"call_id": "c-1",
"judge_pending": True,
"items": [
{
"call_id": "c-1",
"func_name": "spawn_workstream",
"needs_approval": True,
"heuristic_verdict": {
"recommendation": "approve",
"risk_level": "low",
"confidence": 0.9,
},
}
],
}
)
mock_mgr = MagicMock()
mock_mgr.get.return_value = loaded_ws
client = _build_detail_app(mock_mgr)
r = client.get(f"/v1/api/workstreams/{ws_id}")
assert r.status_code == 200
body = r.json()
assert body["pending_approval"] is True
assert body["pending_approval_detail"]["call_id"] == "c-1"
assert body["pending_approval_detail"]["judge_pending"] is True
items = body["pending_approval_detail"]["items"]
assert len(items) == 1
assert items[0]["func_name"] == "spawn_workstream"
assert items[0]["needs_approval"] is True
def test_pending_serializer_failure_falls_back_to_bool_only(self):
"""A malformed verdict that crashes ``serialize_pending_approval_detail``
must NOT fail the detail response — the boolean still informs
the UI that an approval is pending; SSE replay carries the
authoritative payload. Defensive against a future serializer
regression silently 500ing every page load."""
ws_id = "ws-pending-broken"
ws_state = MagicMock()
ws_state.value = "attention"
loaded_ws = MagicMock()
loaded_ws.id = ws_id
loaded_ws.name = "coord-broken"
loaded_ws.state = ws_state
loaded_ws.user_id = "test-user"
loaded_ws.kind = "coordinator"
loaded_ws.ui._pending_approval = {"items": []}
loaded_ws.ui.serialize_pending_approval_detail = MagicMock(
side_effect=RuntimeError("verdict object is malformed"),
)
mock_mgr = MagicMock()
mock_mgr.get.return_value = loaded_ws
client = _build_detail_app(mock_mgr)
r = client.get(f"/v1/api/workstreams/{ws_id}")
assert r.status_code == 200
body = r.json()
assert body["pending_approval"] is True
assert body["pending_approval_detail"] is None
def test_lazy_rehydrates_on_miss(self):
"""``mgr.get`` miss → ``mgr.open`` rehydrate. Same flow as coord;
pre-lift interactive had no detail endpoint so this is the
@@ -986,3 +1100,116 @@ class TestDetailInteractive:
assert "correlation_id=" in body["error"]
# Per-kind noun via cfg.audit_action_prefix.
assert "workstream" in body["error"]
class TestTenantCheckOnReadEndpoints:
"""Regression coverage for the cross-tenant gate on the lifted
``GET /workstreams/{ws_id}`` (detail) and ``/history`` endpoints.
Both handlers used to skip ``cfg.tenant_check`` while every other
lifted session verb invoked it. Pre-PR-447 the gap was a minor
info leak (5 display fields on detail; conversation history); PR
#447 made it real by adding ``pending_approval_detail`` to detail
(tool previews + LLM judge reasoning). These tests pin the gate
so a future cfg refactor can't silently regress it.
"""
def test_detail_404s_when_tenant_check_rejects(self):
"""A non-owning interactive caller reading another user's ws_id
through the detail endpoint must 404 before any data flows."""
ws_id = "ws-other-user"
loaded_ws = MagicMock()
loaded_ws.id = ws_id
loaded_ws.name = "owned-by-stranger"
loaded_ws.state = MagicMock()
loaded_ws.state.value = "idle"
loaded_ws.user_id = "owner"
loaded_ws.kind = "interactive"
mock_mgr = MagicMock()
mock_mgr.get.return_value = loaded_ws
# Tenant check returns a 404 just like ``_require_ws_access``
# does on owner-mismatch. We can't import the production
# helper here (it pulls the whole server module into the test
# graph) so we ape its return shape.
def deny(_request: Any, _ws_id: str, _mgr: Any) -> JSONResponse:
return JSONResponse({"error": "Workstream not found"}, status_code=404)
client = _build_detail_app(mock_mgr, tenant_check=deny)
r = client.get(f"/v1/api/workstreams/{ws_id}")
assert r.status_code == 404
body = r.json()
# Sensitive fields the PR added must not surface for a
# non-owning caller.
assert "name" not in body
assert "pending_approval_detail" not in body
assert "user_id" not in body
# And mgr.get was NEVER consulted — the gate fires first.
mock_mgr.get.assert_not_called()
mock_mgr.open.assert_not_called()
def test_detail_succeeds_when_tenant_check_allows(self):
"""A passing tenant_check (returns ``None``) lets the handler
proceed normally — the ``pending_approval`` defaults still
appear in the response."""
ws_id = "ws-mine"
loaded_ws = MagicMock()
loaded_ws.id = ws_id
loaded_ws.name = "owned"
loaded_ws.state = MagicMock()
loaded_ws.state.value = "idle"
loaded_ws.user_id = "test-user"
loaded_ws.kind = "interactive"
mock_mgr = MagicMock()
mock_mgr.get.return_value = loaded_ws
def allow(_request: Any, _ws_id: str, _mgr: Any) -> None:
return None
client = _build_detail_app(mock_mgr, tenant_check=allow)
r = client.get(f"/v1/api/workstreams/{ws_id}")
assert r.status_code == 200
body = r.json()
assert body["ws_id"] == ws_id
assert body["pending_approval"] is False
assert body["pending_approval_detail"] is None
def test_history_404s_when_tenant_check_rejects(self, _inject_storage):
"""A non-owning interactive caller reading another user's ws_id
through the history endpoint must 404 before any storage
access — owner messages are sensitive content."""
ws_id = "ws-other-user-hist"
_inject_storage.register_workstream(ws_id, kind="interactive", user_id="owner")
_inject_storage.save_message(ws_id, "user", "private message")
mock_mgr = MagicMock()
mock_mgr.get.return_value = None
def deny(_request: Any, _ws_id: str, _mgr: Any) -> JSONResponse:
return JSONResponse({"error": "Workstream not found"}, status_code=404)
client = _build_history_app(mock_mgr, _inject_storage, tenant_check=deny)
r = client.get(f"/v1/api/workstreams/{ws_id}/history")
assert r.status_code == 404
# Owner's content must not have leaked into the response.
assert "private message" not in r.text
def test_history_succeeds_when_tenant_check_allows(self, _inject_storage):
ws_id = "ws-mine-hist"
_inject_storage.register_workstream(ws_id, kind="interactive", user_id="test-user")
_inject_storage.save_message(ws_id, "user", "hello")
mock_ws = MagicMock()
mock_ws.id = ws_id
mock_mgr = MagicMock()
mock_mgr.get.return_value = mock_ws
def allow(_request: Any, _ws_id: str, _mgr: Any) -> None:
return None
client = _build_history_app(mock_mgr, _inject_storage, tenant_check=allow)
r = client.get(f"/v1/api/workstreams/{ws_id}/history")
assert r.status_code == 200
assert any(m.get("content") == "hello" for m in r.json()["messages"])
turnstone/api/server_schemas.py
+20
View File
@@ -424,6 +424,26 @@ class WorkstreamDetailResponse(BaseModel):
state: str
user_id: str
kind: WorkstreamKind = WorkstreamKind.INTERACTIVE
pending_approval: bool = Field(
default=False,
description=(
"True when the workstream is parked on ``_approval_event`` "
"awaiting an operator approve/deny. Mirrors the same field "
"on ``DashboardWorkstream`` / cluster live projections so a "
"freshly-loaded chat tab can render the inline approval gate "
"from the detail snapshot before SSE replay arrives."
),
)
pending_approval_detail: PendingApprovalDetail | None = Field(
default=None,
description=(
"Inline approval payload — same shape as ``DashboardWorkstream"
".pending_approval_detail``. ``None`` when no approval is "
"pending. Lets a reload paint the action row + judge "
"verdicts immediately instead of relying on the SSE "
"approve_request replay timing window."
),
)
class WorkstreamHistoryResponse(BaseModel):
turnstone/console/static/coordinator/coordinator.css
+467 -238
View File
@@ -6,8 +6,9 @@
Tokens (--panel, --hair, --ok, --warn, etc.) come from shared_static/
base.css. Form controls + .btn / .ghost / .appbar primitives come
from shared_static/ui-base.css. This file holds the patterns specific
to the coordinator view: the right-rail sidebar and the pinned approval
dock.
to the coordinator view: the right-rail sidebar, the inline tool-batch
construct (paired tool calls + approval flow + results), and the
drag-and-drop overlay.
========================================================================== */
/* ==========================================================================
@@ -40,235 +41,6 @@
color: var(--ink-4);
}
/* ==========================================================================
Approval dock — bottom-pinned strip that appears when pending approvals
exist. Signature product pattern: a neutral dock (not a modal, not
inline) that surfaces the approval contract without hijacking focus.
Layout:
.approval-dock position: fixed bottom
.dhead 11px uppercase warn kicker + count on right
.dcall risk pill + function name + arg preview
.dctx context code snippets
.drow right-aligned action cluster + nav spacer
Actions (action cluster):
button.act neutral default ("dismiss" / "view")
button.act.primary ok-tinted green per the .ts-approval-btn--approve
convention in shared_static/chat.css. The original
Claude Design spec preferred amber; turnstone
deliberately broke from it to keep colour-family
parity with the Approve button's existing green.
1.5px border, --r-md squared.
button.act.always dashed border — "Always approve for this rule"
button.act.danger err-tinted red — "Deny"
Keyboard shortcuts (wired in coordinator.js):
Enter → primary approve
D → deny
⇧A → always approve
Focus policy: when the dock opens, move focus to button.act.primary so
keyboard users can confirm without hunting. Do NOT trap focus.
========================================================================== */
.approval-dock {
position: fixed;
left: 0;
right: 0;
bottom: 22px; /* clears the statusbar if one is present */
z-index: 20;
display: flex;
flex-direction: column;
gap: 10px;
padding: 14px 20px;
background: var(--panel);
border-top: 1px solid var(--hair);
box-shadow: 0 -6px 24px -12px rgba(21, 24, 27, 0.18);
}
.approval-dock::before {
content: "";
position: absolute;
top: -1px;
left: 0;
right: 0;
height: 2px;
background: linear-gradient(
to right,
transparent,
color-mix(in srgb, var(--warn) 50%, transparent),
transparent
);
}
.approval-dock .dhead {
display: flex;
align-items: center;
gap: 8px;
font-size: 11px;
font-weight: 600;
letter-spacing: 0.06em;
text-transform: uppercase;
color: var(--warn);
}
.approval-dock .dhead::before {
content: "⚠";
font-size: 12px;
}
.approval-dock .dhead .dcount {
margin-left: auto;
font-family: var(--font-mono);
font-size: 10px;
font-weight: 500;
letter-spacing: 0;
text-transform: none;
color: var(--ink-3);
}
/* Inline code-panel framing — the .dcall row reads as "the exact call you
are approving," so we frame it like a mini inspectable code line rather
than bare text on the dock surface. */
.approval-dock .dcall {
display: flex;
align-items: center;
gap: 10px;
min-width: 0;
padding: 6px 10px;
background: var(--panel);
border: 1px solid var(--hair);
border-radius: var(--r-sm);
}
.approval-dock .dcall .risk { flex-shrink: 0; }
.approval-dock .dcall .dfn {
font-family: var(--font-mono);
font-weight: 600;
color: var(--ink);
}
.approval-dock .dcall .dargs {
flex: 1;
min-width: 0;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
font-family: var(--font-mono);
font-size: 12px;
color: var(--ink-3);
}
.approval-dock .dctx {
display: flex;
gap: 14px;
flex-wrap: wrap;
font-size: 11px;
color: var(--ink-3);
}
.approval-dock .dctx code {
padding: 0 4px;
font-family: var(--font-mono);
font-size: 10px;
color: var(--ink-2);
background: var(--panel-2);
border: 1px solid var(--hair);
border-radius: 3px;
}
.approval-dock .drow {
display: flex;
align-items: center;
gap: 6px;
}
.approval-dock .drow .spacer { flex: 1; }
.approval-dock .drow .nav {
padding: 4px 8px;
font-family: var(--font-mono);
font-size: 11px;
color: var(--ink-3);
background: transparent;
border: 1px solid transparent;
border-radius: 3px;
cursor: pointer;
}
.approval-dock .drow .nav:hover {
background: var(--panel-2);
color: var(--ink);
}
/* Action buttons — 1.5px border, --r-md squared (NOT pill — these are
primary-action surfaces, not inline buttons). */
.approval-dock button.act {
padding: 7px 16px;
font: inherit;
font-size: 12px;
font-weight: 500;
color: var(--ink-2);
background: var(--panel);
border: 1.5px solid var(--hair-2);
border-radius: var(--r-md);
cursor: pointer;
transition: background 120ms ease, border-color 120ms ease, color 120ms ease;
}
.approval-dock button.act:hover {
color: var(--ink);
border-color: var(--ink-4);
}
.approval-dock button.act:focus-visible {
outline: 2px solid var(--accent);
outline-offset: 2px;
}
/* Approve and Always are siblings — same ok hue, differentiated by fill
(filled vs outlined) and border-style (solid vs dashed). Matches the
.ts-approval-btn--approve convention in shared_static/chat.css. Four
stacked non-colour cues for WCAG 1.4.1: fill state, border style,
label, position. */
.approval-dock button.act.primary {
background: color-mix(in srgb, var(--ok) 28%, var(--panel));
color: var(--ok-text);
border-color: color-mix(in srgb, var(--ok) 65%, var(--hair));
font-weight: 600;
}
.approval-dock button.act.primary:hover {
background: color-mix(in srgb, var(--ok) 40%, var(--panel));
color: var(--ink);
border-color: var(--ok);
}
.approval-dock button.act.always {
background: transparent;
border-style: dashed;
border-color: color-mix(in srgb, var(--ok) 65%, var(--hair));
color: var(--ok-text);
}
.approval-dock button.act.always:hover {
background: color-mix(in srgb, var(--ok) 15%, var(--panel));
color: var(--ink);
border-color: var(--ok);
}
.approval-dock button.act.danger {
color: var(--err);
border-color: color-mix(in srgb, var(--err) 42%, var(--hair));
}
.approval-dock button.act.danger:hover {
background: var(--err-soft);
color: var(--err);
border-color: var(--err);
}
/* ==========================================================================
Drag-and-drop overlay — applied to #coord-main while the user is
dragging files from the OS over the chat pane. Composer wires this on
@@ -301,9 +73,432 @@
z-index: 10;
}
/* Match .btn .kbd (in shared_static/ui-base.css) — --ink-3 clears AA at
10px, --ink-4 is borderline on light panels. */
.approval-dock button.act .kbd {
/* ==========================================================================
Tool batch construct — pairs tool calls with their results and
embeds the approval flow. Replaces the bottom approval dock + the
duplicate .msg.tool bubbles for tool-call rendering.
One construct per dispatch turn:
- solo (1 call, serial): .coord-tool-batch--solo
- parallel (≥2 calls): .coord-tool-batch--parallel
rows share a left rail so the
operator reads them as siblings
of one assistant decision.
Sub-elements:
.coord-tool-batch-head label + count + tier glyph
.coord-tool-row per-call row (call line + verdict + result)
.coord-tool-row-call [idx] name args ellipsized
.coord-tool-row-verdict judge verdict chip + rationale teaser
.coord-tool-row-result paired tool_result <pre> under the row
.coord-tool-row-status per-row pill (auto-approved / error)
.coord-tool-actions approve / deny / always
.coord-tool-status resolved status pill (replaces actions)
States (modifiers on the batch):
.coord-tool-batch--pending approval gate visible
.coord-tool-batch--approved resolved approve
.coord-tool-batch--denied resolved deny — rows dimmed
.coord-tool-batch--auto all auto-approved, no gate ever shown
.coord-tool-batch--running replay-time orphan (dispatched but no
matching tool_result yet) — no actions.
SSE upgrades to --pending or --auto
when it knows more.
========================================================================== */
.coord-tool-batch {
margin: 4px 0;
background: var(--panel);
border: 1px solid var(--hair);
border-left: 3px solid var(--hair-2);
border-radius: var(--r-sm);
font-family: var(--font-mono);
font-size: 12px;
line-height: 1.5;
overflow: hidden;
}
/* State left-stripe — neutral default; warn when gating; ok when
resolved-approved; err when denied or any row errored. Three stacked
non-colour cues for WCAG 1.4.1: pill text in the head, rail colour,
row dimming on deny. */
.coord-tool-batch--pending {
border-left-color: var(--warn);
}
.coord-tool-batch--approved {
border-left-color: color-mix(in srgb, var(--ok) 65%, var(--hair-2));
}
.coord-tool-batch--auto {
border-left-color: var(--hair-2);
}
.coord-tool-batch--running {
/* Subtle accent stripe so the operator can tell a still-in-flight
replayed batch apart from a resolved one without it screaming
for attention. Not warn (which would imply approval-needed). */
border-left-color: color-mix(in srgb, var(--accent) 50%, var(--hair-2));
}
.coord-tool-batch--denied,
.coord-tool-batch--error {
border-left-color: var(--err);
}
.coord-tool-batch--denied .coord-tool-row {
opacity: 0.6;
}
/* Header strip — small uppercase kicker + per-batch metadata. */
.coord-tool-batch-head {
display: flex;
flex-wrap: wrap;
align-items: baseline;
gap: 8px;
padding: 6px 10px;
background: var(--panel-2);
border-bottom: 1px solid var(--hair);
font-size: 10px;
font-weight: 600;
letter-spacing: 0.06em;
text-transform: uppercase;
color: var(--ink-3);
}
.coord-tool-batch-kicker {
color: var(--ink-3);
}
.coord-tool-batch--pending .coord-tool-batch-kicker {
color: var(--warn);
}
.coord-tool-batch--approved .coord-tool-batch-kicker {
color: color-mix(in srgb, var(--ok) 70%, var(--ink-2));
}
.coord-tool-batch--denied .coord-tool-batch-kicker {
color: var(--err);
}
.coord-tool-batch-summary {
font-family: var(--font-mono);
font-weight: 500;
letter-spacing: 0;
text-transform: none;
color: var(--ink-2);
}
.coord-tool-batch-tier {
margin-left: auto;
font-family: var(--font-mono);
font-weight: 400;
letter-spacing: 0;
text-transform: none;
color: var(--ink-4);
font-size: 10px;
}
/* Row container. In parallel batches, rows are framed by a left rail
so they read as siblings of a single assistant decision; in solo
batches the rail is suppressed to keep visual weight low. */
.coord-tool-row {
position: relative;
padding: 8px 10px;
}
.coord-tool-row + .coord-tool-row {
border-top: 1px solid var(--hair);
}
.coord-tool-batch--parallel .coord-tool-row {
padding-left: 28px;
}
.coord-tool-batch--parallel .coord-tool-row::before {
/* Vertical rail tick — connects rows visually as a parallel group.
Stops 4px short of the row's top + bottom edges so consecutive
rows look continuous; the dot at the row's center marks the call. */
content: "";
position: absolute;
left: 14px;
top: 0;
bottom: 0;
width: 1px;
background: var(--hair-2);
}
/* Tuck the rail 4px in from the very first / last row's edge so the
line doesn't butt against the batch's inner top/bottom. Class
markers (set in JS at row-build time) instead of :first-of-type /
:last-of-type because the batch contains other ``<div>`` siblings
(.coord-tool-batch-head, .coord-tool-actions / .coord-tool-status)
that are also of type ``div`` — :first-of-type would never select
the first .coord-tool-row, and the rule would silently no-op. */
.coord-tool-batch--parallel .coord-tool-row.coord-tool-row--first::before {
top: 4px;
}
.coord-tool-batch--parallel .coord-tool-row.coord-tool-row--last::before {
bottom: 4px;
}
.coord-tool-batch--parallel .coord-tool-row::after {
content: "";
position: absolute;
left: 11px;
top: 14px;
width: 7px;
height: 7px;
border-radius: 50%;
background: var(--panel);
border: 1.5px solid var(--hair-2);
}
.coord-tool-batch--parallel.coord-tool-batch--approved .coord-tool-row::after {
border-color: color-mix(in srgb, var(--ok) 65%, var(--hair-2));
}
.coord-tool-batch--parallel.coord-tool-batch--denied .coord-tool-row::after,
.coord-tool-row.error::after {
border-color: var(--err);
}
/* Call line — index/N pill, monospace tool name, ellipsized args. */
.coord-tool-row-call {
display: flex;
align-items: baseline;
gap: 8px;
min-width: 0;
}
.coord-tool-row-idx {
flex-shrink: 0;
padding: 1px 6px;
font-size: 10px;
font-weight: 600;
letter-spacing: 0.04em;
color: var(--ink-3);
background: var(--panel-2);
border: 1px solid var(--hair);
border-radius: 3px;
}
.coord-tool-row-name {
flex-shrink: 0;
font-family: var(--font-mono);
font-weight: 600;
color: var(--ink);
}
.coord-tool-row.error .coord-tool-row-name {
color: var(--err);
}
.coord-tool-row-args {
flex: 1;
min-width: 0;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
font-family: var(--font-mono);
color: var(--ink-3);
}
/* Verdict line — judge chip + optional rationale teaser. */
.coord-tool-row-verdict {
margin-top: 4px;
display: flex;
flex-wrap: wrap;
align-items: center;
gap: 8px;
font-size: 11px;
color: var(--ink-3);
}
.coord-tool-row-verdict code {
padding: 1px 6px;
font-family: var(--font-mono);
font-size: 10px;
color: var(--ink-2);
background: var(--panel-2);
border: 1px solid var(--hair);
border-radius: 3px;
}
.coord-tool-row-verdict code.rec-approve {
color: color-mix(in srgb, var(--ok) 70%, var(--ink-2));
border-color: color-mix(in srgb, var(--ok) 38%, var(--hair));
background: color-mix(in srgb, var(--ok) 12%, var(--panel-2));
}
.coord-tool-row-verdict code.rec-review {
color: color-mix(in srgb, var(--warn) 70%, var(--ink-2));
border-color: color-mix(in srgb, var(--warn) 38%, var(--hair));
background: var(--warn-tint);
}
.coord-tool-row-verdict code.rec-deny {
color: color-mix(in srgb, var(--err) 70%, var(--ink-2));
border-color: color-mix(in srgb, var(--err) 38%, var(--hair));
background: color-mix(in srgb, var(--err) 12%, var(--panel-2));
}
.coord-tool-row-verdict code.judging {
display: inline-flex;
align-items: center;
gap: 5px;
color: var(--ink-3);
}
.coord-tool-row-verdict code.judging .spin {
width: 8px;
height: 8px;
border-radius: 50%;
border: 1.5px solid var(--accent);
border-top-color: transparent;
animation: ts-spin 0.9s linear infinite;
}
@media (prefers-reduced-motion: reduce) {
.coord-tool-row-verdict code.judging .spin { animation: none; }
}
/* Rationale disclosure — collapsible block under a row. Renders the
judge's reasoning prose; `details` element so a click toggles without
stealing focus. */
.coord-tool-row-rationale {
margin-top: 4px;
font-size: 11px;
color: var(--ink-3);
}
.coord-tool-row-rationale > summary {
cursor: pointer;
color: var(--accent);
font-size: 11px;
list-style: none;
user-select: none;
}
.coord-tool-row-rationale > summary::before {
content: "▸ ";
display: inline-block;
margin-right: 2px;
transition: transform 120ms ease;
}
.coord-tool-row-rationale[open] > summary::before {
transform: rotate(90deg);
}
.coord-tool-row-rationale-body {
margin: 4px 0 0 14px;
padding: 6px 10px;
font-family: var(--font-mono);
font-size: 11px;
line-height: 1.5;
color: var(--ink-3);
background: var(--panel-2);
border-left: 2px solid var(--hair);
border-radius: 0 3px 3px 0;
white-space: pre-wrap;
}
/* Per-row status pill (auto-approved, error). */
.coord-tool-row-status {
display: inline-flex;
align-items: center;
gap: 4px;
padding: 1px 6px;
font-size: 10px;
font-weight: 600;
letter-spacing: 0.04em;
text-transform: uppercase;
border-radius: 3px;
border: 1px solid var(--hair);
color: var(--ink-3);
}
.coord-tool-row-status--auto {
color: color-mix(in srgb, var(--ok) 70%, var(--ink-2));
border-color: color-mix(in srgb, var(--ok) 38%, var(--hair));
background: color-mix(in srgb, var(--ok) 12%, var(--panel-2));
}
.coord-tool-row-status--error {
color: var(--err);
border-color: color-mix(in srgb, var(--err) 38%, var(--hair));
background: color-mix(in srgb, var(--err) 12%, var(--panel-2));
}
/* Tool result block — paired under its row, mono pre-block. Capped
at 240px with internal scroll so a long tool output doesn't push
the rest of the chat off-screen. The interactive UI uses a
click-to-expand "collapsed" affordance (see ui/static/style.css
.tool-output.collapsed) — coord deliberately doesn't, since the
construct is read-only history once results land and a scroll
pane is the lower-friction read for a diagnostic surface. */
.coord-tool-row-result {
margin-top: 6px;
padding: 6px 10px;
font-family: var(--font-mono);
font-size: 11px;
line-height: 1.5;
color: var(--ink-2);
background: var(--panel-2);
border-left: 2px solid var(--hair);
border-radius: 0 3px 3px 0;
white-space: pre-wrap;
word-break: break-word;
max-height: 240px;
overflow: auto;
}
.coord-tool-row.error .coord-tool-row-result {
border-left-color: var(--err);
color: color-mix(in srgb, var(--err) 75%, var(--ink-2));
}
.coord-tool-row-result-lead {
display: inline-block;
margin-right: 4px;
color: var(--ink-4);
font-weight: 600;
}
/* Action row — Approve / Deny / Always. Renders inside a pending
tool-batch construct as the operator's gate for the dispatch. The
.act button vocabulary (primary/always/danger) is local to this
surface; the children-tree's .ch-row .approval-actions reuses the
same colour/border treatment in compact .sm sizing. */
.coord-tool-actions {
display: flex;
align-items: center;
gap: 6px;
padding: 8px 10px;
background: var(--panel-2);
border-top: 1px solid var(--hair);
}
.coord-tool-actions .spacer { flex: 1; }
.coord-tool-actions button.act {
padding: 6px 14px;
font: inherit;
font-size: 12px;
font-weight: 500;
color: var(--ink-2);
background: var(--panel);
border: 1.5px solid var(--hair-2);
border-radius: var(--r-md);
cursor: pointer;
transition: background 120ms ease, border-color 120ms ease, color 120ms ease;
}
.coord-tool-actions button.act:hover {
color: var(--ink);
border-color: var(--ink-4);
}
.coord-tool-actions button.act:focus-visible {
outline: 2px solid var(--accent);
outline-offset: 2px;
}
.coord-tool-actions button.act:disabled {
opacity: 0.5;
cursor: not-allowed;
}
.coord-tool-actions button.act.primary {
background: color-mix(in srgb, var(--ok) 28%, var(--panel));
color: var(--ok-text);
border-color: color-mix(in srgb, var(--ok) 65%, var(--hair));
font-weight: 600;
}
.coord-tool-actions button.act.primary:hover {
background: color-mix(in srgb, var(--ok) 40%, var(--panel));
color: var(--ink);
border-color: var(--ok);
}
.coord-tool-actions button.act.always {
background: transparent;
border-style: dashed;
border-color: color-mix(in srgb, var(--ok) 65%, var(--hair));
color: var(--ok-text);
}
.coord-tool-actions button.act.always:hover {
background: color-mix(in srgb, var(--ok) 15%, var(--panel));
color: var(--ink);
border-color: var(--ok);
}
.coord-tool-actions button.act.danger {
color: var(--err);
border-color: color-mix(in srgb, var(--err) 42%, var(--hair));
}
.coord-tool-actions button.act.danger:hover {
background: var(--err-soft);
color: var(--err);
border-color: var(--err);
}
.coord-tool-actions button.act .kbd {
margin-left: 6px;
padding: 0 3px;
font-family: var(--font-mono);
@@ -312,10 +507,44 @@
border: 1px solid var(--hair);
border-radius: 2px;
}
/* Tinted keycap on the primary Approve button — uses the parent's
--ok hue so the keycap reads as part of the green action surface. */
.approval-dock button.act.primary .kbd {
.coord-tool-actions button.act.primary .kbd {
color: color-mix(in srgb, var(--ok) 70%, var(--ink-3));
border-color: color-mix(in srgb, var(--ok) 40%, var(--hair));
}
/* Resolved status pill — replaces the action row after approve/deny. */
.coord-tool-status {
display: flex;
align-items: center;
gap: 8px;
padding: 6px 10px;
background: var(--panel-2);
border-top: 1px solid var(--hair);
font-size: 11px;
font-weight: 500;
color: var(--ink-3);
}
.coord-tool-status--approved {
color: color-mix(in srgb, var(--ok) 70%, var(--ink-2));
}
.coord-tool-status--denied,
.coord-tool-status--error {
color: var(--err);
}
.coord-tool-status-feedback {
font-family: var(--font-mono);
font-size: 11px;
color: var(--ink-3);
}
/* Mobile (<700px) — keep action targets ≥44px for WCAG 2.5.5. */
@media (max-width: 700px) {
.coord-tool-actions {
flex-wrap: wrap;
}
.coord-tool-actions button.act {
flex: 1 1 30%;
min-height: 44px;
font-size: 13px;
}
}
turnstone/console/static/coordinator/coordinator.js
+1081 -308
View File
File diff suppressed because it is too large Load Diff
turnstone/console/static/coordinator/index.html
+35 -136
View File
@@ -14,14 +14,15 @@
<link rel="stylesheet" href="/static/style.css">
<link rel="stylesheet" href="/static/coordinator/coordinator.css">
<style>
/* Coordinator-specific layout glue. Messages, header, approval dock,
and sidebar chrome live in shared_static/{chat,ui-base}.css and
console/static/style.css; what remains here is the page-level flex
wiring (chat pane + right sidebar), tree-view row metadata (indent,
state dots, child highlight), and the <700px responsive accordion.
Rules that target .msg / .appbar / .sidebar / .approval-dock are
intentionally absent — those primitives ship from the shared sheets
and we don't restyle them here. */
/* Coordinator-specific layout glue. Messages, header, and sidebar
chrome live in shared_static/{chat,ui-base}.css and
console/static/style.css; the inline tool-batch construct lives
in coordinator.css. What remains here is the page-level flex
wiring (chat pane + right sidebar), tree-view row metadata
(indent, state dots, child highlight), and the <700px responsive
accordion. Rules that target .msg / .appbar / .sidebar are
intentionally absent — those primitives ship from the shared
sheets and we don't restyle them here. */
body { display: flex; flex-direction: column; height: 100vh; margin: 0; }
/* Main layout — chat pane (2fr) + sidebar (1fr) with shared
@@ -219,10 +220,10 @@
color: var(--ink-3);
}
/* Recommendation chip inside the disclosure footer — same 12/38/70%
colour-mix scheme as the dock chips at `#coord-approval-bar
.dctx code.rec-*`, scoped to the row's disclosure so the inline
chip is actually styled (the dock-scoped rules don't reach this
surface). */
colour-mix scheme as the inline tool-batch verdict chips
(.coord-tool-row-verdict code.rec-*), scoped here to the row's
disclosure since this children-tree surface uses its own
.approval-disclosure container. */
.ch-row .approval-disclosure code.rec-approve,
.ch-row .approval-disclosure code.rec-review,
.ch-row .approval-disclosure code.rec-deny {
@@ -311,13 +312,13 @@
justify-content: flex-end;
margin-top: 2px;
}
/* Inline .act buttons — duplicates the colour/border treatment from
shared_static/design/patterns/approval-dock.css :162-225 because
the dock rules are scoped to `.approval-dock button.act` and the
children-tree row isn't inside a dock. Compact sizing applied
via .sm. Keeping the duplication local-scoped means a future
hoist of the dock rules to a global `.act` primitive could
drop these without affecting the dock surface. */
/* Inline .act buttons for the children-tree approval block —
compact (.sm) variant of the colour/border treatment used by the
coord chat's tool-batch action row (coordinator.css
.coord-tool-actions button.act). Duplicated locally because the
children-tree row sits in the right-rail sidebar with its own
parent class; if we ever lift `.act` to a shared primitive these
local overrides can drop. */
.ch-row .approval-actions .act {
padding: 3px 10px;
font: inherit;
@@ -436,95 +437,11 @@
.ch-row.highlight { transition: none; }
}
/* Override the .approval-dock pattern's viewport-pinned positioning.
The pattern defaults to position: fixed bottom:22px (designed for
the fleet dashboard overlay case); in the coordinator chat we need
it inline above the composer so it doesn't cover the input area.
Dock sits as the second flex child inside #coord-main between
messages and composer, with a hair top border as the
separator. */
#coord-approval-bar.approval-dock {
position: static;
bottom: auto;
left: auto;
right: auto;
z-index: auto;
box-shadow: none;
flex: 0 0 auto;
}
/* Keep the warm top-stripe cue; just make it hug the top edge of the
in-flow dock instead of the top of a fixed viewport bar. */
#coord-approval-bar.approval-dock::before {
top: -1px;
}
/* Hide the dock when no approval is pending. [hidden] toggle; the
approval-dock pattern defines display: flex so we need the
!important override to win specificity. */
.approval-dock[hidden] { display: none !important; }
/* Judge verdict chips — colour-code by recommendation so the
reviewer can triage at a glance without reading the chip text.
approve=ok, review=warn, deny=err. Uses the same 12/38/70% mix
scheme as the primitive k-badge tokens. */
#coord-approval-bar .dctx code.rec-approve {
color: color-mix(in srgb, var(--ok) 70%, var(--ink-2));
border-color: color-mix(in srgb, var(--ok) 38%, var(--hair));
background: color-mix(in srgb, var(--ok) 12%, var(--panel-2));
}
#coord-approval-bar .dctx code.rec-review {
color: color-mix(in srgb, var(--warn) 70%, var(--ink-2));
border-color: color-mix(in srgb, var(--warn) 38%, var(--hair));
background: var(--warn-tint);
}
#coord-approval-bar .dctx code.rec-deny {
color: color-mix(in srgb, var(--err) 70%, var(--ink-2));
border-color: color-mix(in srgb, var(--err) 38%, var(--hair));
background: color-mix(in srgb, var(--err) 12%, var(--panel-2));
}
/* Local @keyframes ts-spin — primitives/feed.css owns the canonical
definition but this page doesn't link feed.css (no .feed-item
usage). Defined here so the .judging .spin chip below animates. */
/* @keyframes ts-spin — drives the .coord-tool-row-verdict
code.judging spinner. Defined here because this page doesn't
link feed.css (no .feed-item usage). */
@keyframes ts-spin { to { transform: rotate(360deg); } }
/* "judge evaluating…" spinner chip — shown while a .dcall is
pending a verdict. */
#coord-approval-bar .dctx code.judging {
display: inline-flex;
align-items: center;
gap: 5px;
color: var(--ink-3);
}
#coord-approval-bar .dctx code.judging .spin {
width: 8px;
height: 8px;
border-radius: 50%;
border: 1.5px solid var(--accent);
border-top-color: transparent;
animation: ts-spin 0.9s linear infinite;
}
@media (prefers-reduced-motion: reduce) {
#coord-approval-bar .dctx code.judging .spin {
animation: none;
}
}
/* Judge rationale — the judge's reasoning text, rendered below the
dctx chips as a block quote. Full text wraps; no truncation —
justification is the whole point of showing this. */
#coord-approval-bar .drationale {
margin-top: 4px;
padding: 6px 10px;
font-family: var(--font-mono);
font-size: 11px;
line-height: 1.5;
color: var(--ink-3);
background: var(--panel-2);
border-left: 2px solid var(--hair);
border-radius: 0 3px 3px 0;
white-space: pre-wrap;
}
/* Sidebar mobile toggle (desktop hides; mobile shows via media query
below). */
#coord-sidebar-toggle {
@@ -597,36 +514,18 @@
don't re-announce partial content on every token. -->
<div id="coord-messages" role="log" aria-live="polite"></div>
<!-- Approval dock — overridden to inline positioning (see the
position: static override in the style block above). Sits
between the message log and the composer so it doesn't occlude
the user input. role="region" (not alertdialog) because we do
not trap focus; buttons are reachable in normal tab order.
aria-live="assertive" preserves announce-on-queue behaviour. -->
<aside id="coord-approval-bar"
class="approval-dock"
role="region"
aria-label="Approval required"
aria-live="assertive"
hidden>
<div id="coord-approval-label" class="dhead">
Approval required
<span id="coord-approval-count" class="dcount"></span>
</div>
<div id="coord-approval-tools"></div>
<div class="drow">
<div class="spacer"></div>
<button id="coord-deny-btn" class="act danger" type="button" onclick="coordApprove(false, false)">
Deny<span class="kbd">D</span>
</button>
<button id="coord-approve-always-btn" class="act always" type="button" onclick="coordApprove(true, true)">
Always<span class="kbd">⇧A</span>
</button>
<button id="coord-approve-btn" class="act primary" type="button" onclick="coordApprove(true, false)">
Approve<span class="kbd">⏎</span>
</button>
</div>
</aside>
<!-- Off-screen assertive live region for action-required SR
announcements ("Approval required: spawn_workstream + 9
more"). Pending tool-batches go into the polite #coord-messages
log, which gets flipped to aria-live="off" during token
streaming, so without this dedicated assertive region a
screen reader could miss the gate landing. Visually hidden
via inline style; no layout impact. -->
<div id="coord-sr-announcer"
role="status"
aria-live="assertive"
aria-atomic="true"
style="position:absolute;left:-10000px;width:1px;height:1px;overflow:hidden"></div>
<!-- Per-coordinator status bar — pinned above the composer.
Mirrors the interactive pane's `.ws-status-bar`: model alias,
turnstone/core/session_routes.py
+74 -1
View File
@@ -786,7 +786,12 @@ def make_approve_handler(cfg: SessionEndpointConfig) -> Handler:
if source_map is not None:
for t in tool_names:
source_map[t] = AutoApproveReason.ALWAYS
ui.resolve_approval(approved, feedback)
# Forward ``always`` so the resulting ``approval_resolved`` SSE
# event carries the intent — peer tabs that didn't click but
# are subscribed to the same workstream can render the right
# status pill ("✓ approved · always" vs plain "✓ approved")
# without needing a side-channel broadcast.
ui.resolve_approval(approved, feedback, always=always)
return JSONResponse({"status": "ok"})
return approve
@@ -2173,6 +2178,21 @@ def make_history_handler(cfg: SessionEndpointConfig) -> Handler:
if not ws_id:
return JSONResponse({"error": "ws_id is required"}, status_code=400)
# Cross-tenant gate. Pre-PR-447 the response carried only
# message rows that an owning user wrote and that owning
# user's tools produced — sensitive but bounded to the same
# ``user_id`` as the workstream. Even so, every other lifted
# session verb (send / approve / close / cancel / events /
# attachments) calls ``cfg.tenant_check`` and history was the
# outlier. Coord wires ``tenant_check=None`` (the
# cluster-wide ``admin.coordinator`` permission_gate covers
# it); interactive wires ``_interactive_tenant_check`` and
# this call now restores parity with the rest of the surface.
if cfg.tenant_check is not None:
err_tenant = cfg.tenant_check(request, ws_id, mgr)
if err_tenant is not None:
return err_tenant
# Existence + kind check. The workstream may live only in
# storage (closed coordinators are still readable via /history
# without rehydrating; persisted-but-not-loaded interactives
@@ -2262,6 +2282,21 @@ def make_detail_handler(cfg: SessionEndpointConfig) -> Handler:
if not ws_id:
return JSONResponse({"error": "ws_id is required"}, status_code=400)
# Cross-tenant gate. PR 447 added ``pending_approval_detail``
# to the response (tool previews, function arguments, LLM
# judge reasoning) — a richer payload than the pre-PR
# ``{ws_id, name, state, user_id, kind}`` tuple. Coord wires
# ``tenant_check=None`` (the cluster-wide ``admin.coordinator``
# permission_gate covers it); interactive wires
# ``_interactive_tenant_check`` so any authenticated user that
# GETs another user's ``ws_id`` 404s here instead of reading
# the in-flight tool-call payload. Brings detail in line with
# every other lifted session verb.
if cfg.tenant_check is not None:
err_tenant = cfg.tenant_check(request, ws_id, mgr)
if err_tenant is not None:
return err_tenant
ws = mgr.get(ws_id)
if ws is None:
try:
@@ -2303,6 +2338,42 @@ def make_detail_handler(cfg: SessionEndpointConfig) -> Handler:
# mismatch, and tombstoned rows — all surface as 404.
return JSONResponse({"error": cfg.not_found_label}, status_code=404)
# Pending-approval snapshot — lets a freshly-loaded chat tab
# paint the inline approval gate from this single response
# instead of waiting for the SSE approve_request replay (which
# introduces a brief --running flash on reload). Both keys
# (``pending_approval`` + ``pending_approval_detail``) are
# always present in the response: a UI that doesn't expose
# ``serialize_pending_approval_detail`` (CLI / channel
# adapters) reports ``False`` / ``null`` for them. The
# ``_pending_approval`` lookup is asserted as ``dict`` (its
# only real production shape — see
# ``SessionUIBase._pending_approval``) so a MagicMock-based
# unit test or other non-dict sentinel doesn't trip the path.
pending_approval = False
pending_approval_detail: dict[str, Any] | None = None
ui = ws.ui
pending_raw = getattr(ui, "_pending_approval", None) if ui is not None else None
if isinstance(pending_raw, dict):
pending_approval = True
serializer = getattr(ui, "serialize_pending_approval_detail", None)
if callable(serializer):
try:
serialized = serializer()
if isinstance(serialized, dict) or serialized is None:
pending_approval_detail = serialized
except Exception:
# Defensive: a malformed verdict object inside the
# serializer shouldn't fail the entire detail
# response. The boolean still informs the UI that
# an approval is pending; SSE replay carries the
# full payload.
log.warning(
"ws.detail.pending_serialize_failed ws_id=%s",
ws_id[:8] if ws_id else "",
exc_info=True,
)
return JSONResponse(
{
"ws_id": ws.id,
@@ -2310,6 +2381,8 @@ def make_detail_handler(cfg: SessionEndpointConfig) -> Handler:
"state": ws.state.value,
"user_id": ws.user_id,
"kind": ws.kind,
"pending_approval": pending_approval,
"pending_approval_detail": pending_approval_detail,
}
)
turnstone/core/session_ui_base.py
+16 -1
View File
@@ -286,7 +286,13 @@ class SessionUIBase:
self._last_verdict_decision = ""
self._llm_verdicts.clear()
def resolve_approval(self, approved: bool, feedback: str | None = None) -> None:
def resolve_approval(
self,
approved: bool,
feedback: str | None = None,
*,
always: bool = False,
) -> None:
"""Unblock a pending approval with the caller's decision.
Broadcasts ``approval_resolved`` so every connected tab can
@@ -294,6 +300,14 @@ class SessionUIBase:
phone approves). Updates ``user_decision`` on every LLM
intent-verdict that fired during this approval round — the
audit trail reflects what the user actually chose.
``always`` reports whether the resolving caller asked for
"Approve + Always" (the tool name has been added to
``auto_approve_tools`` upstream by the HTTP handler — this
method only echoes the intent on the SSE event so peer tabs
can label their resolved-status pill correctly). Keyword-only
+ default ``False`` so the four pre-existing callers (cancel,
timeout, channel adapters) compile unchanged.
"""
decision_str = "approved" if approved else "denied"
# Swap-and-clear + set decision under lock to avoid racing
@@ -310,6 +324,7 @@ class SessionUIBase:
"type": "approval_resolved",
"approved": approved,
"feedback": feedback or "",
"always": bool(always),
}
)
self._approval_event.set()