diff --git a/tests/test_app_js.py b/tests/test_app_js.py index 2b56bf41..b7e610d0 100644 --- a/tests/test_app_js.py +++ b/tests/test_app_js.py @@ -1567,16 +1567,19 @@ def test_risk_level_normalized_before_dom_interpolation() -> None: straight into the class string and silently break selector targeting — guard that the chokepoint exists and no site skips it.""" body = _INTERACTIVE_JS.read_text(encoding="utf-8") - assert "function normalizeRiskLevel(" in body - assert "VALID_RISK_LEVELS" in body - for level in ("low", "medium", "high", "critical"): - assert f'"{level}"' in body + # The canonical normalizer + its enum moved to the shared conversation.js + # (step 5e.1b); the pane imports it and routes all three sites through it. + assert "normalizeRiskLevel," in body and 'from "./conversation.js"' in body # The raw fallback antipattern must be gone from every interpolation site. assert 'risk_level || "medium"' not in body assert 'risk_level) || "medium"' not in body - # The three known sites (updateVerdictBadge / _buildOutputWarningEl / + # The three sites (updateVerdictBadge / _buildOutputWarningEl / # renderVerdictBadge) all route through the chokepoint. - assert body.count("normalizeRiskLevel(") >= 4 # 1 def + 3 call sites + assert body.count("normalizeRiskLevel(") >= 3 # 3 call sites (def is shared) + shared = (_INTERACTIVE_JS.parent / "conversation.js").read_text(encoding="utf-8") + assert "export function normalizeRiskLevel(" in shared + for level in ("low", "medium", "high", "critical"): + assert f'"{level}"' in shared def test_announced_rail_outspecifies_inline_cyan_hold() -> None: diff --git a/tests/test_conversation_js.py b/tests/test_conversation_js.py index 13b73c46..32601fce 100644 --- a/tests/test_conversation_js.py +++ b/tests/test_conversation_js.py @@ -65,3 +65,24 @@ def test_no_inner_html() -> None: """House style: programmatic DOM only — no innerHTML *usage* in the shared module (the header comment names it; guard the access pattern).""" assert ".innerHTML" not in _body() + + +def test_normalize_risk_level_unknown_to_medium() -> None: + """Unified canonical fallback (step 5e.1b): an unknown / unrecognized risk + normalizes to "medium" (the user's decision; the coordinator's old rank used + "high"). The crit/med abbreviations alias to critical/medium so a 'crit' + verdict no longer renders as medium (the latent interactive bug).""" + body = _body() + assert 'return RISK_LEVELS.indexOf(s) >= 0 ? s : "medium";' in body + assert 'crit: "critical"' in body and 'med: "medium"' in body + + +def test_risk_rank_and_max_severity_exported() -> None: + """riskRank + maxSeverityItem (lifted from the coordinator's _riskRank / + _maxSeverityItem) are exported and build on the canonical normalize, so the + rank and the display can't disagree on the fallback. An item with no verdict + ranks below low so it never wins the max-severity pick.""" + body = _body() + assert "export function riskRank(" in body + assert "export function maxSeverityItem(" in body + assert "? riskRank(v.risk_level) : -1;" in body diff --git a/tests/test_coordinator_page.py b/tests/test_coordinator_page.py index f13e1a43..9a44b214 100644 --- a/tests/test_coordinator_page.py +++ b/tests/test_coordinator_page.py @@ -73,7 +73,7 @@ def test_coordinator_js_exposes_inline_approval_helpers(): body = coord_js.read_text(encoding="utf-8") # Approval-block rendering helpers assert "function renderApprovalBlock" in body - assert "function _maxSeverityItem" in body + assert "maxSeverityItem," in body # imported from conversation.js (5e.1b) assert "function _renderSubItem" in body # The submit + 409 race-handling path assert "function submitChildApproval" in body or "submitChildApproval(" in body @@ -103,11 +103,14 @@ def test_coordinator_js_exposes_inline_approval_helpers(): # 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 + # Critical-risk handling: bug-1 was that risk_level='critical' rendered as + # low because the old severity table only mapped 'crit'. The crit/critical + # alias moved to the shared conversation.js (step 5e.1b); verify it there so + # a 'critical' verdict still ranks like 'crit'. + shared = Path(__file__).resolve().parent.parent / ( + "turnstone/shared_static/conversation.js" + ) + assert 'crit: "critical"' in shared.read_text(encoding="utf-8") # 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 diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js index c41a2daa..90c588e4 100644 --- a/turnstone/console/static/coordinator/coordinator.js +++ b/turnstone/console/static/coordinator/coordinator.js @@ -30,6 +30,7 @@ import { stripAnsi, buildWatchResultCard, buildSystemNudgeMarker, + maxSeverityItem, } from "/shared/conversation.js"; function buildCoordChrome(root, opts) { @@ -3222,50 +3223,10 @@ function createCoordinatorPane(root, wsId, opts) { // null if the detail is unusable (defensive \u2014 server is supposed to // emit None when no items). Stays DOM-method-only to match the // zero-innerHTML XSS posture of the rest of the row template. - // Risk-level → numeric severity for max-across-items computation. - // Values are ordinal: higher integer = higher severity. Both "crit" - // and "critical" map to 3 because production emitters disagree: - // turnstone/core/judge.py validates against ('low','medium','high', - // 'critical') and the heuristic seeds emit the full word, but - // earlier dock UI history used the abbreviation. Accept both. - // Unknown / malformed risk_level falls back to "high" rank so a - // schema drift fails *safe* (over-alert) rather than silently - // downgrading to a green pill — fixes the failure mode where - // "critical" was treated as unknown and rendered as low. - const RISK_SEVERITY = { - low: 0, - medium: 1, - med: 1, - high: 2, - crit: 3, - critical: 3, - }; - const UNKNOWN_RISK_RANK = 2; // fail-safe: treat unknown as "high" - - function _riskRank(verdict) { - if (!verdict) return -1; - const risk = (verdict.risk_level || "").toLowerCase(); - return RISK_SEVERITY[risk] != null - ? RISK_SEVERITY[risk] - : UNKNOWN_RISK_RANK; - } - - // Pick the item carrying the highest risk_level — pill colour and - // body display follow the worst tool in the envelope so a low-risk - // item[0] can't visually mask a crit item[2]. - function _maxSeverityItem(items) { - let best = items[0]; - let bestRank = _riskRank(best.judge_verdict || best.heuristic_verdict); - for (let i = 1; i < items.length; i += 1) { - const v = items[i].judge_verdict || items[i].heuristic_verdict; - const r = _riskRank(v); - if (r > bestRank) { - best = items[i]; - bestRank = r; - } - } - return best; - } + // Risk-level severity ranking moved to the shared conversation.js + // (maxSeverityItem / riskRank, imported above) so the coordinator and + // interactive panes can't drift on the fallback. Unknown / malformed + // risk_level ranks "medium" (step 5e.1b: this pane's old rank used "high"). function _evidenceLineText(line) { if (typeof line === "string") return line; @@ -3350,7 +3311,7 @@ function createCoordinatorPane(root, wsId, opts) { // them all so leading with [0] keeps the operator's mental model // anchored on "what the LLM dispatched first"). const primary = items[0]; - const severityItem = _maxSeverityItem(items); + const severityItem = maxSeverityItem(items); const judge = severityItem.judge_verdict || null; const heuristic = severityItem.heuristic_verdict || null; const verdict = judge || heuristic; diff --git a/turnstone/shared_static/conversation.js b/turnstone/shared_static/conversation.js index 922d8344..1d426cca 100644 --- a/turnstone/shared_static/conversation.js +++ b/turnstone/shared_static/conversation.js @@ -83,3 +83,47 @@ export function buildSystemNudgeMarker() { el.textContent = "system nudge"; return el; } + +// Canonical risk-level vocabulary shared by both panes. RISK_LEVELS is ordinal +// (index == severity rank); the aliases cover emitters that abbreviate. An +// unknown / unrecognized level normalizes to "medium": not "low" (an unlabelled +// risk must not render benign) and not "high" (medium is the neutral default +// both panes already displayed; the coordinator's old rank used "high", which +// step 5e.1b brings to "medium" per the unify decision). +const RISK_LEVELS = ["low", "medium", "high", "critical"]; +const RISK_ALIASES = { med: "medium", crit: "critical" }; + +export function normalizeRiskLevel(raw) { + let s = String(raw == null ? "" : raw) + .trim() + .toLowerCase(); + s = RISK_ALIASES[s] || s; + return RISK_LEVELS.indexOf(s) >= 0 ? s : "medium"; +} + +// Ordinal rank (low=0 .. critical=3) via the canonical normalize, so an alias or +// unknown value ranks consistently with how it displays. Unknown -> medium (1). +export function riskRank(raw) { + return RISK_LEVELS.indexOf(normalizeRiskLevel(raw)); +} + +// Pick the item carrying the highest risk_level (judge verdict preferred over +// heuristic) so a low-risk item[0] can't visually mask a higher-risk item[2]. +// An item with NO verdict ranks below "low" (-1) so it never wins; a verdict +// with an unknown level ranks "medium" via riskRank. +export function maxSeverityItem(items) { + function rank(it) { + const v = it && (it.judge_verdict || it.heuristic_verdict); + return v ? riskRank(v.risk_level) : -1; + } + let best = items[0]; + let bestRank = rank(best); + for (let i = 1; i < items.length; i += 1) { + const r = rank(items[i]); + if (r > bestRank) { + best = items[i]; + bestRank = r; + } + } + return best; +} diff --git a/turnstone/shared_static/interactive.js b/turnstone/shared_static/interactive.js index a66b65ad..6328b0bc 100644 --- a/turnstone/shared_static/interactive.js +++ b/turnstone/shared_static/interactive.js @@ -27,6 +27,7 @@ import { stripAnsi, buildWatchResultCard, buildSystemNudgeMarker, + normalizeRiskLevel, } from "./conversation.js"; let _paneCounter = 0; @@ -3182,21 +3183,6 @@ function buildToolDiv(item) { return div; } -// Server-supplied ``risk_level`` is constrained to this enum, but it lands -// in ``className`` / ``data-risk`` strings that the verdict + output-warning -// CSS and the ``badge.nextElementSibling`` / ``data-risk`` selectors rely on. -// Funnel every interpolation through one chokepoint so whitespace, a stray -// case, or a future relaxed-validation server value can't break selector -// targeting silently (issue #562) — unknown / blank → the neutral default. -const VALID_RISK_LEVELS = new Set(["low", "medium", "high", "critical"]); - -function normalizeRiskLevel(raw) { - const s = String(raw || "") - .trim() - .toLowerCase(); - return VALID_RISK_LEVELS.has(s) ? s : "medium"; -} - function renderVerdictBadge(verdict, judgePending) { if (!verdict) return document.createDocumentFragment(); const risk = normalizeRiskLevel(verdict.risk_level);