From 433f66add864062df7c211d9b52fc74fcfccfb2f Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 25 Jul 2026 01:47:38 -0400 Subject: [PATCH 01/12] feat(webui): request preview, authorization, and workflow initiation (Closes #643) Operators had to paste a role prompt into a terminal to start work, and nothing enforced that the allocator had been consulted first, so two sessions could reach for the same issue and each believe it was theirs. This adds a request surface: a desired role, an issue or PR, and a stated intent, answered by an authorization decision and - on confirmation - an exclusive assignment from the allocator. Preview (POST /api/v1/requests/preview, and the /requests form) runs five checks and reports authorize/deny with a reason for each: console authorization, capability resolution for the desired role, lease availability, whether the allocator would independently select this work unit, and head pinning for PR work. It is read-only - it calls the allocator with apply=false and writes only an audit line. An unauthorized principal never reaches the allocator or the control-plane DB, so a denial cannot enumerate the queue. Initiation (POST /api/v1/requests/apply) never assigns the requested item directly. It runs a dry-run first and proceeds only when the allocator would independently pick that exact work unit, carrying the dry-run's candidate_set_fingerprint as a CAS pin; otherwise it returns wait or blocked and mutates nothing. An active claim on the work unit rejects a duplicate assign before one is attempted. A returned assignment carries a handoff block naming the required profile, namespace, and the actions that stay forbidden. Authorization reuses the #633 model rather than adding a second one. The new initiate_workflow action is operator-class because its outcome is a claim, not a Gitea verdict: requesting reviewer or merger work reserves that work but grants no right to approve or merge. Execution is gated by a new per-action execution_env_flag (WEBUI_REQUESTS_EXECUTION), deliberately in place of raising ACTIVE_PHASE - a phase bump would enable execution for every phase-2 action at once, including ones whose execution path is not implemented. Actions that declare no flag are unchanged and still report execution_enabled false. Every preview and apply emits a console audit record correlated to the resulting assignment by correlation.request_id. Fail-closed throughout: an unreadable control-plane DB, an incomplete queue inventory (#758), an allocator that raises, an unpinned PR head, a moved PR head, and an unconfirmed apply all deny without mutating. Files: - webui/request_service.py (new) - request model, preview, initiation - webui/request_views.py (new) - form and preview rendering, escaped - tests/test_webui_request_initiation.py (new) - 52 tests - webui/console_authz.py - initiate_workflow action, execution_wired() - webui/app.py - /requests, /api/v1/requests/preview, /api/v1/requests/apply - webui/nav.py - Requests nav entry - webui/traffic_loader.py - public candidates_from_queue_snapshot alias - docs/webui-requests.md (new), docs/webui-authz-audit.md Validation: full suite on this branch 5242 passed, 6 skipped, 899 subtests, 23 failed. Clean master baseline at 2f4dec83 in an equivalent branches/ worktree: 5190 passed, 6 skipped, 867 subtests, the same 23 tests failed. The branch adds 52 passing tests and introduces no new full-suite failure signature. Closes #643 Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/webui-authz-audit.md | 50 +- docs/webui-requests.md | 160 ++++ tests/test_webui_request_initiation.py | 917 +++++++++++++++++++++++ webui/app.py | 116 +++ webui/console_authz.py | 79 +- webui/nav.py | 1 + webui/request_service.py | 967 +++++++++++++++++++++++++ webui/request_views.py | 164 +++++ webui/traffic_loader.py | 10 + 9 files changed, 2447 insertions(+), 17 deletions(-) create mode 100644 docs/webui-requests.md create mode 100644 tests/test_webui_request_initiation.py create mode 100644 webui/request_service.py create mode 100644 webui/request_views.py diff --git a/docs/webui-authz-audit.md b/docs/webui-authz-audit.md index 2dcffac..ff9cc38 100644 --- a/docs/webui-authz-audit.md +++ b/docs/webui-authz-audit.md @@ -94,6 +94,7 @@ already define, and a regression test asserts each mapping matches. | `record_analytics_usage` | operator | gated_write | `runtime.record_analytics_usage` | Yes | No | No | 2 | | `system.reload_namespace` | controller | privileged | `runtime.reload_namespace` | Yes | No | No | 2 | | `system.restart_namespace` | admin | destructive | `runtime.restart_namespace` | Yes | **Yes** | **Yes** | 2 | +| `initiate_workflow` | operator | gated_write | `gitea.read` | Yes | No | No | 2 | **Dual control** means the acting principal may not be the sole authority: a second distinct principal must confirm. **Break-glass** means the action is @@ -112,6 +113,12 @@ by the console — both hand off to a host supervisor, and neither exposes a raw process kill. See [`sanctioned-restart-controls.md`](sanctioned-restart-controls.md) (#642). +`initiate_workflow` (#643) is operator-class because its outcome is a *claim*, +not a Gitea verdict. Requesting reviewer or merger work reserves that work +through the allocator; it does not grant the right to approve or merge, which +stays with the MCP role profile and its own capability gates. See +[`webui-requests.md`](webui-requests.md). + ### Authorization decision `authorize(action_id, principal, for_execution=False)` returns a decision @@ -126,9 +133,24 @@ record and **denies by default**. The deny reasons are closed and enumerated: | `phase_not_active` | Execution requested for an action whose phase is not open. | | `allowed_preview_only` | Authorized — preview only, execution still disabled. | -There is no implicit allow branch. Even the allow result reports -`execution_enabled: false` while the console is in Phase 1, so no caller can -read an allow as permission to mutate. +There is no implicit allow branch. + +`execution_enabled` on the decision reports whether the action has a live +execution path at all, and is computed by `execution_wired(action)`. There are +exactly two ways to be wired: + +1. the action's `phase` is at or below `ACTIVE_PHASE`; or +2. the action declares an `execution_env_flag` **and** that variable is set. + +Every action that declares no flag therefore reports `execution_enabled: false` +while the console is in Phase 1, so no caller can read an allow as permission +to mutate. The per-action flag exists because raising `ACTIVE_PHASE` would +enable execution for every action of that phase at once, including ones whose +execution path is not implemented. One implemented action goes live on its own +flag instead of dragging its unimplemented phase-mates with it. + +`initiate_workflow` is the only action that currently declares a flag +(`WEBUI_REQUESTS_EXECUTION`), and it stays denied until an operator sets it. ## Secret redaction @@ -235,13 +257,22 @@ second one. The integration points are already wired and observable: instead of adding a parallel check. - **`GET /api/console/security-model`** publishes the RBAC matrix, redaction policy, and audit policy as JSON for operators and tests. +- **`POST /api/v1/requests/preview` and `.../apply`** (#643) are the first + actions to use this model for a real execution path. Preview always returns a + decision and an audited `previewed` record; apply requires `confirm=true`, + emits `succeeded` or `denied`, and reserves work only through the allocator. + See [`webui-requests.md`](webui-requests.md). -To open Phase 2, a child issue must: raise `ACTIVE_PHASE`, implement the -confirmation and dual-control flow the matrix already declares, emit a -`succeeded` or `failed` record alongside the `gitea_audit` mutation record, and -keep `viewer` unable to reach any of it. Turning on execution without the -confirmation flow contradicts a declared requirement and is a review failure, -not a shortcut. +A Phase 2 action must: use `execution_wired` rather than a private enable flag, +implement the confirmation and dual-control flow the matrix already declares, +emit a `succeeded` or `failed` record alongside the `gitea_audit` mutation +record, and keep `viewer` unable to reach any of it. Turning on execution +without the confirmation flow contradicts a declared requirement and is a +review failure, not a shortcut. + +Raising `ACTIVE_PHASE` remains the way to open a whole phase at once, and is +deliberately *not* what #643 did: an action-scoped opt-in cannot enable an +action whose execution path nobody wrote. ## Local-dev mode @@ -294,6 +325,7 @@ Until Phase 2 wires it, probe protection rests on network placement alone, as | `WEBUI_ROLE_MAP` | unset | JSON subject → role map | | `WEBUI_REQUIRE_PROBE_AUTH` | unset | Require auth for non-public probes | | `WEBUI_CONSOLE_AUDIT_LOG` | unset | Append-only audit sink path | +| `WEBUI_REQUESTS_EXECUTION` | unset | Opt in to `initiate_workflow` execution (#643) | All are read server-side only. None is ever rendered into a page or returned by an API. diff --git a/docs/webui-requests.md b/docs/webui-requests.md new file mode 100644 index 0000000..b8fbdb2 --- /dev/null +++ b/docs/webui-requests.md @@ -0,0 +1,160 @@ +# Web console requests: intent preview and workflow initiation (#643) + +**Phase 2. Preview is always live and always read-only. Initiation is wired but +denied until an operator opts in.** + +Before this surface, starting role work meant pasting a prompt into a terminal +and trusting the operator to have checked the allocator first. Nothing enforced +that check, so two sessions could reach for the same issue and each believe it +was theirs. This page replaces the paste with a *request*: a desired role, an +issue or PR, and a stated intent, answered by an authorization decision and — +on confirmation — an exclusive assignment from the allocator. + +| Concern | Module | +|---------|--------| +| Request model, preview, initiation | `webui/request_service.py` | +| Form and preview rendering | `webui/request_views.py` | +| Authorization | `webui/console_authz.py` (`initiate_workflow`) | +| Audit | `webui/console_audit.py` | +| Ownership substrate | `allocator_service.py` + `control_plane_db.py` | + +## Surfaces + +| Path | Method | Purpose | +|------|--------|---------| +| `/requests` | GET | Request form | +| `/requests` | POST | Render an intent preview. **Never assigns.** | +| `/api/v1/requests/preview` | POST | Intent preview as JSON | +| `/api/v1/requests/apply` | POST | Initiate — confirmed, audited, allocator-owned | + +The HTML form has no initiate button on purpose. Initiating requires a +confirmed POST to `/api/v1/requests/apply`, so a stray form submission cannot +reserve work as a side effect. + +## The request + +```json +{ + "desired_role": "author", + "work_kind": "issue", + "work_number": 643, + "intent_summary": "implement request preview and initiation", + "remote": "prgs", + "org": "Scaled-Tech-Consulting", + "repo": "Gitea-Tools", + "expected_head_sha": null +} +``` + +`desired_role` is one of `author`, `reviewer`, `merger`, `reconciler`, +`controller`. `work_kind` is `issue` or `pr`. `remote`/`org`/`repo` default to +the first project in the registry when omitted; when neither the request nor +the registry resolves them, the request is rejected rather than pointed at some +other repository. `intent_summary` is required — it is what the audit record +states as the reason — and is truncated to 500 characters. + +Parsing rejects rather than corrects. An unknown role, an unknown work kind, a +non-positive number, or a missing intent each return `400` with a `reason_code` +and the offending `field`. + +## Preview + +Five checks, each with its own verdict, reason code, and detail: + +| Check | Passes when | +|-------|-------------| +| `authorization` | The console principal holds `operator` or above | +| `capability` | The desired role maps to a declared profile and MCP namespace | +| `lease_availability` | No active claim holds the work unit | +| `next_safe_action` | The allocator would independently select this exact work unit | +| `head_pin` | PR work resolves to a head SHA, and a supplied SHA still matches | + +A preview also returns the role's `allowed_actions` and `prohibited_actions` +(from `allocator_service.ROLE_ACTIONS`), the `required_profile` and +`required_namespace` the work must run under, and a `correlation_id` that ties +the preview to its audit record and to any assignment that follows. + +Preview is read-only in the strict sense: it calls the allocator with +`apply=false` and writes nothing but an audit line. An unauthorized principal +never reaches the allocator or the control-plane DB at all, so a denial cannot +be used to enumerate the queue. + +## Initiation + +`POST /api/v1/requests/apply` refuses in this order, and every refusal returns +before any assignment is attempted: + +| Condition | Outcome | Status | +|-----------|---------|--------| +| Unparseable request | `invalid_request` | 400 | +| Not authorized, or execution not wired | `denied` | 403 | +| `confirm` not set | `denied` / `confirmation_required` | 409 | +| Work unit already claimed | `blocked` / `duplicate_assignment` | 409 | +| Allocator would select other work | `wait` / `not_next_safe_work` | 409 | +| Allocator declines on apply | `blocked` or `wait` | 409 | +| Evidence unavailable | `wait` / `evidence_unavailable` | 503 | +| Assigned | `assigned_work` | 201 | + +A success returns the assignment plus a `handoff` block naming the profile, the +namespace, and the actions that stay forbidden — enough for the operator to +continue in the right MCP namespace without guessing. + +### Why apply runs the allocator twice + +The allocator is the only source of exclusive ownership (#600 / #613), and it +selects work; it does not take orders. So `apply` runs a dry-run first and +proceeds only when the allocator would independently pick the requested work +unit. If it would not, the request reports `wait` and mutates nothing. + +A request is therefore a *confirmation* of the allocator's decision, never an +override of it. The apply call carries the dry-run's +`candidate_set_fingerprint` as a CAS pin (#776), so a queue that changed +between the two calls fails closed rather than assigning against a stale view. +The result is checked again on the way out: an assignment naming a different +work unit is not read as success. + +### Fail-closed defaults + +- An unreadable control-plane DB denies. It is never treated as "nothing holds + this work unit". +- An incomplete queue inventory denies (#758). Ranking a partial candidate set + can select the wrong work. +- An allocator that raises denies. +- PR work with no resolvable head SHA denies; a supplied SHA that no longer + matches denies with `head_moved`. + +## Enabling initiation + +Execution is wired off. Set `WEBUI_REQUESTS_EXECUTION=1` to enable it for the +`initiate_workflow` action only — see +[`webui-authz-audit.md`](webui-authz-audit.md) for why this is an +action-scoped flag rather than a phase bump. With the variable unset, `apply` +returns `403` with `reason_code: unauthorized` no matter who asks. + +Enabling execution does **not** enable approvals or merges. Those are phase 3 +console actions and remain forbidden in every path here; the console reserves +work and hands off, and the MCP role profile enforces what that role may then +do. + +## Audit + +Every preview and every apply emits a console audit record (schema in +[`webui-authz-audit.md`](webui-authz-audit.md)): + +| Event | `result` | +|-------|----------| +| Preview | `previewed` | +| Refusal at any stage | `denied` | +| Assignment created | `succeeded` | + +`correlation.request_id` carries the request's `correlation_id`, and a +successful record's `metadata` carries `assignment_id` and `lease_id`, so an +assignment can be traced back to the intent that produced it. The operator's +`intent_summary` travels in `metadata` and passes through the standard +redaction pass before persistence like every other field. + +## Non-goals + +- No browser-initiated approve or merge, in this phase or any other. +- No bypass of allocator exclusive ownership; no self-selection of work. +- No auto-start from raw monitoring incidents (#612 stays downstream). diff --git a/tests/test_webui_request_initiation.py b/tests/test_webui_request_initiation.py new file mode 100644 index 0000000..c733898 --- /dev/null +++ b/tests/test_webui_request_initiation.py @@ -0,0 +1,917 @@ +"""Request preview, authorization, and workflow initiation tests (#643). + +Covers each acceptance criterion: + +* AC1 — preview shows authorize/deny with reasons. +* AC2 — apply creates an exclusive assignment or returns wait/blocked. +* AC3 — duplicate assign rejected. +* AC4 — preview / apply / deny / collision are all exercised. +* AC5 — the UI never renders a secret, and messaging stays brief. + +Required tests named in the issue: allocator integration with fakes, and +gated-action tests. The allocator is injected as a fake throughout so no test +touches Gitea or reserves real work; one class asserts the *real* default +allocator refuses an incomplete inventory rather than ranking a partial set. +""" + +from __future__ import annotations + +import json +import os +import pathlib +import sys +import tempfile +import unittest +from typing import Any +from unittest import mock + +from tests.webui_testclient import TestClient + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parents[1])) + +import allocator_service # noqa: E402 +from webui import console_audit, console_authz, request_service # noqa: E402 +from webui.app import create_app # noqa: E402 +from webui.console_redaction import scan_for_secrets # noqa: E402 +from webui.request_views import render_requests_page # noqa: E402 + +EXEC_FLAG = "WEBUI_REQUESTS_EXECUTION" + +SCOPE = { + "remote": "prgs", + "org": "Scaled-Tech-Consulting", + "repo": "Gitea-Tools", +} + + +def _principal(role: str) -> console_authz.Principal: + return console_authz.Principal( + subject=f"{role}@example.com", + role=role, + identity_source=console_authz.IDENTITY_ACCESS_PROXY, + authenticated=True, + ) + + +def _request( + *, + role: str = "author", + kind: str = "issue", + number: int = 643, + intent: str = "implement request preview and initiation", + head: str | None = None, +) -> request_service.WorkRequest: + parsed, error = request_service.parse_request( + { + "desired_role": role, + "work_kind": kind, + "work_number": number, + "intent_summary": intent, + "expected_head_sha": head, + **SCOPE, + } + ) + assert error is None, error + assert parsed is not None + return parsed + + +def _selection( + *, kind: str = "issue", number: int = 643, head_sha: str | None = None +) -> dict[str, Any]: + return { + "kind": kind, + "number": number, + "title": "Web Console: Requests, intent preview, authorization", + "head_sha": head_sha, + "selected_action": "implement", + "expected_role_next": "author", + } + + +def _fake_allocator( + *, + selection: dict[str, Any] | None = None, + preview_outcome: str = allocator_service.OUTCOME_PREVIEW, + apply_outcome: str = allocator_service.OUTCOME_ASSIGNED, + assignment: dict[str, Any] | None = None, + calls: list[dict[str, Any]] | None = None, +): + """Build an allocator double that records how it was called.""" + chosen = selection if selection is not None else _selection() + made = ( + assignment + if assignment is not None + else { + "assignment_id": "asn-test-0001", + "lease_id": "lease-test-0001", + "session_id": "webui-request-test", + "expected_head_sha": chosen.get("head_sha"), + } + ) + + def _allocator(*, request, apply, expected_candidate_set_fingerprint=None): + if calls is not None: + calls.append( + { + "apply": apply, + "role": request.desired_role, + "fingerprint": expected_candidate_set_fingerprint, + } + ) + return { + "outcome": apply_outcome if apply else preview_outcome, + "selected": dict(chosen), + "reasons": ["fake allocator"], + "candidate_set_fingerprint": "fp-test", + "candidate_count": 3, + "inventory_complete": True, + "selection_policy": allocator_service.SELECTION_POLICY, + "substrate": "control_plane_db", + "assignment": dict(made) if apply else None, + } + + return _allocator + + +def _no_claims(_request): + return {} + + +def _claimed(role: str = "author"): + def _source(request): + return { + request.work_key: { + "lease_id": "lease-foreign-9999", + "session_id": "prgs-author-999-foreign", + "role": role, + "expires_at": "2026-07-25T09:10:37Z", + } + } + + return _source + + +class TestRequestParsing(unittest.TestCase): + """The request model rejects rather than guesses.""" + + def test_valid_request_round_trips(self): + req = _request() + self.assertEqual(req.work_key, ("issue", 643)) + self.assertEqual(req.display_ref, "#643") + self.assertEqual(req.to_dict()["desired_role"], "author") + + def test_unknown_role_rejected(self): + parsed, error = request_service.parse_request( + { + "desired_role": "admin", + "work_kind": "issue", + "work_number": 1, + "intent_summary": "x", + **SCOPE, + } + ) + self.assertIsNone(parsed) + self.assertEqual(error.reason_code, "unknown_role") + self.assertEqual(error.field_name, "desired_role") + + def test_unknown_work_kind_rejected(self): + parsed, error = request_service.parse_request( + { + "desired_role": "author", + "work_kind": "branch", + "work_number": 1, + "intent_summary": "x", + **SCOPE, + } + ) + self.assertIsNone(parsed) + self.assertEqual(error.reason_code, "unknown_work_kind") + + def test_non_positive_number_rejected(self): + for value in (0, -3): + with self.subTest(value=value): + parsed, error = request_service.parse_request( + { + "desired_role": "author", + "work_kind": "issue", + "work_number": value, + "intent_summary": "x", + **SCOPE, + } + ) + self.assertIsNone(parsed) + self.assertEqual(error.reason_code, "invalid_work_number") + + def test_missing_intent_rejected(self): + parsed, error = request_service.parse_request( + { + "desired_role": "author", + "work_kind": "issue", + "work_number": 1, + **SCOPE, + } + ) + self.assertIsNone(parsed) + self.assertEqual(error.reason_code, "missing_intent") + + def test_intent_is_bounded(self): + req = _request(intent="x" * 5000) + self.assertEqual(len(req.intent_summary), request_service.MAX_INTENT_CHARS) + + def test_unresolved_scope_rejected(self): + parsed, error = request_service.parse_request( + { + "desired_role": "author", + "work_kind": "issue", + "work_number": 1, + "intent_summary": "x", + } + ) + self.assertIsNone(parsed) + self.assertEqual(error.reason_code, "scope_unresolved") + + def test_default_scope_fills_missing_fields(self): + parsed, error = request_service.parse_request( + { + "desired_role": "author", + "work_kind": "issue", + "work_number": 7, + "intent_summary": "x", + }, + default_scope=SCOPE, + ) + self.assertIsNone(error) + self.assertEqual(parsed.repo, "Gitea-Tools") + + +class TestPreviewAuthorizeDeny(unittest.TestCase): + """AC1 — preview shows authorize/deny with reasons.""" + + def test_authorized_preview_names_every_check(self): + preview = request_service.preview_request( + _request(), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(), + claims_source=_no_claims, + audit=False, + ) + self.assertTrue(preview.authorized) + self.assertEqual( + {c.name for c in preview.checks}, + { + request_service.CHECK_AUTHORIZATION, + request_service.CHECK_CAPABILITY, + request_service.CHECK_LEASE_AVAILABILITY, + request_service.CHECK_NEXT_SAFE_ACTION, + request_service.CHECK_HEAD_PIN, + }, + ) + self.assertEqual(preview.required_profile, "prgs-author") + self.assertEqual(preview.required_namespace, "gitea-author") + + def test_every_check_carries_a_reason(self): + preview = request_service.preview_request( + _request(), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(), + claims_source=_no_claims, + audit=False, + ) + for check in preview.checks: + with self.subTest(check=check.name): + self.assertTrue(check.reason_code.strip()) + self.assertTrue(check.detail.strip()) + + def test_anonymous_preview_denied_with_reason(self): + preview = request_service.preview_request( + _request(), + allocator=_fake_allocator(), + claims_source=_no_claims, + audit=False, + ) + self.assertFalse(preview.authorized) + self.assertEqual(preview.reason_code, console_authz.DENY_UNAUTHENTICATED) + + def test_viewer_preview_denied_for_insufficient_role(self): + preview = request_service.preview_request( + _request(), + principal=_principal(console_authz.VIEWER), + allocator=_fake_allocator(), + claims_source=_no_claims, + audit=False, + ) + self.assertFalse(preview.authorized) + self.assertEqual(preview.reason_code, console_authz.DENY_INSUFFICIENT_ROLE) + + def test_denied_preview_never_reaches_the_allocator(self): + """A denial must not double as a queue oracle.""" + calls: list[dict[str, Any]] = [] + request_service.preview_request( + _request(), + principal=_principal(console_authz.VIEWER), + allocator=_fake_allocator(calls=calls), + claims_source=_no_claims, + audit=False, + ) + self.assertEqual(calls, []) + + def test_preview_lists_prohibited_actions(self): + preview = request_service.preview_request( + _request(role="author"), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(), + claims_source=_no_claims, + audit=False, + ) + self.assertIn("merge", preview.prohibited_actions) + self.assertIn("approve", preview.prohibited_actions) + + def test_preview_reports_next_safe_action(self): + preview = request_service.preview_request( + _request(), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(), + claims_source=_no_claims, + audit=False, + ) + self.assertIn("issue #643", preview.next_safe_action) + + def test_preview_never_mutates(self): + calls: list[dict[str, Any]] = [] + request_service.preview_request( + _request(), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(calls=calls), + claims_source=_no_claims, + audit=False, + ) + self.assertEqual([c["apply"] for c in calls], [False]) + + +class TestPreviewFailClosed(unittest.TestCase): + """Missing evidence denies; it never reads as an absence of obstacles.""" + + def test_unreadable_claim_inventory_denies(self): + def _boom(_request): + raise RuntimeError("db unavailable") + + preview = request_service.preview_request( + _request(), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(), + claims_source=_boom, + audit=False, + ) + self.assertFalse(preview.authorized) + self.assertEqual( + preview.reason_code, request_service.REASON_EVIDENCE_UNAVAILABLE + ) + + def test_allocator_failure_denies(self): + def _boom(**_kwargs): + raise RuntimeError("allocator exploded") + + preview = request_service.preview_request( + _request(), + principal=_principal(console_authz.OPERATOR), + allocator=_boom, + claims_source=_no_claims, + audit=False, + ) + self.assertFalse(preview.authorized) + self.assertEqual( + preview.reason_code, request_service.REASON_EVIDENCE_UNAVAILABLE + ) + + def test_allocator_selecting_other_work_denies(self): + preview = request_service.preview_request( + _request(number=643), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(selection=_selection(number=999)), + claims_source=_no_claims, + audit=False, + ) + self.assertFalse(preview.authorized) + self.assertEqual(preview.reason_code, request_service.REASON_NOT_NEXT_SAFE) + self.assertIn("#999", preview.detail) + + def test_pr_without_head_sha_denies(self): + preview = request_service.preview_request( + _request(role="reviewer", kind="pr", number=898), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator( + selection=_selection(kind="pr", number=898, head_sha=None) + ), + claims_source=_no_claims, + audit=False, + ) + self.assertFalse(preview.authorized) + self.assertEqual( + preview.reason_code, request_service.REASON_EVIDENCE_UNAVAILABLE + ) + + def test_pr_head_moved_denies(self): + preview = request_service.preview_request( + _request(role="reviewer", kind="pr", number=898, head="a" * 40), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator( + selection=_selection(kind="pr", number=898, head_sha="b" * 40) + ), + claims_source=_no_claims, + audit=False, + ) + self.assertFalse(preview.authorized) + self.assertEqual(preview.reason_code, "head_moved") + + def test_pr_head_matching_passes(self): + preview = request_service.preview_request( + _request(role="reviewer", kind="pr", number=898, head="b" * 40), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator( + selection=_selection(kind="pr", number=898, head_sha="b" * 40) + ), + claims_source=_no_claims, + audit=False, + ) + self.assertTrue(preview.authorized) + + +class TestApplyExecutionGate(unittest.TestCase): + """Execution stays wired off unless an operator opts in explicitly.""" + + def test_action_is_registered_and_unwired_by_default(self): + action = console_authz.get_action(request_service.ACTION_ID) + self.assertIsNotNone(action) + self.assertEqual(action.phase, 2) + self.assertEqual(action.minimum_role, console_authz.OPERATOR) + self.assertTrue(action.requires_confirmation) + self.assertFalse(console_authz.execution_wired(action, env={})) + + def test_flag_named_but_unset_does_not_wire(self): + action = console_authz.get_action(request_service.ACTION_ID) + self.assertFalse(console_authz.execution_wired(action, env={EXEC_FLAG: "no"})) + self.assertTrue(console_authz.execution_wired(action, env={EXEC_FLAG: "1"})) + + def test_opting_in_wires_only_this_action(self): + env = {EXEC_FLAG: "1"} + for action_id, action in console_authz.ACTIONS.items(): + with self.subTest(action=action_id): + self.assertEqual( + console_authz.execution_wired(action, env=env), + action_id == request_service.ACTION_ID, + ) + + def test_apply_denied_while_unwired(self): + with mock.patch.dict(os.environ, {EXEC_FLAG: ""}): + result = request_service.apply_request( + _request(), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_fake_allocator(), + claims_source=_no_claims, + ) + self.assertFalse(result["ok"]) + self.assertEqual(result["outcome"], request_service.OUTCOME_DENIED) + self.assertEqual(result["reason_code"], request_service.REASON_UNAUTHORIZED) + self.assertFalse(result["mutation_performed"]) + + +class TestApplyOutcomes(unittest.TestCase): + """AC2/AC3/AC4 — assignment, wait, blocked, and duplicate rejection.""" + + def setUp(self): + patcher = mock.patch.dict(os.environ, {EXEC_FLAG: "1"}) + patcher.start() + self.addCleanup(patcher.stop) + + def test_apply_creates_exclusive_assignment(self): + calls: list[dict[str, Any]] = [] + result = request_service.apply_request( + _request(), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_fake_allocator(calls=calls), + claims_source=_no_claims, + ) + self.assertTrue(result["ok"]) + self.assertEqual(result["outcome"], allocator_service.OUTCOME_ASSIGNED) + self.assertEqual(result["assignment"]["assignment_id"], "asn-test-0001") + self.assertTrue(result["mutation_performed"]) + self.assertEqual(result["status_code"], 201) + # Dry-run first, then apply — never apply alone. + self.assertEqual([c["apply"] for c in calls], [False, True]) + # The apply call carries the fingerprint the dry-run produced. + self.assertEqual(calls[1]["fingerprint"], "fp-test") + + def test_assignment_returns_a_role_handoff(self): + result = request_service.apply_request( + _request(), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_fake_allocator(), + claims_source=_no_claims, + ) + handoff = result["handoff"] + self.assertEqual(handoff["required_profile"], "prgs-author") + self.assertEqual(handoff["required_namespace"], "gitea-author") + self.assertEqual(handoff["assignment_id"], "asn-test-0001") + self.assertIn("merge", handoff["forbidden_actions"]) + + def test_unconfirmed_apply_refuses_before_the_allocator(self): + calls: list[dict[str, Any]] = [] + result = request_service.apply_request( + _request(), + principal=_principal(console_authz.OPERATOR), + confirm=False, + allocator=_fake_allocator(calls=calls), + claims_source=_no_claims, + ) + self.assertFalse(result["ok"]) + self.assertEqual( + result["reason_code"], request_service.REASON_CONFIRMATION_REQUIRED + ) + self.assertEqual(calls, []) + + def test_duplicate_assignment_rejected(self): + """AC3 — an active lease on the work unit blocks a second assign.""" + calls: list[dict[str, Any]] = [] + result = request_service.apply_request( + _request(), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_fake_allocator(calls=calls), + claims_source=_claimed(), + ) + self.assertFalse(result["ok"]) + self.assertEqual(result["outcome"], request_service.OUTCOME_BLOCKED) + self.assertEqual( + result["reason_code"], request_service.REASON_DUPLICATE_ASSIGNMENT + ) + self.assertFalse(result["mutation_performed"]) + # The dry-run ran; the apply never did. + self.assertEqual([c["apply"] for c in calls], [False]) + + def test_not_next_safe_work_returns_wait_without_applying(self): + calls: list[dict[str, Any]] = [] + result = request_service.apply_request( + _request(number=643), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_fake_allocator( + selection=_selection(number=999), calls=calls + ), + claims_source=_no_claims, + ) + self.assertFalse(result["ok"]) + self.assertEqual(result["outcome"], request_service.OUTCOME_WAIT) + self.assertEqual(result["reason_code"], request_service.REASON_NOT_NEXT_SAFE) + self.assertEqual([c["apply"] for c in calls], [False]) + + def test_allocator_declining_on_apply_returns_blocked(self): + result = request_service.apply_request( + _request(), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_fake_allocator( + apply_outcome=allocator_service.OUTCOME_BLOCKED_LEASE, + assignment={}, + ), + claims_source=_no_claims, + ) + self.assertFalse(result["ok"]) + self.assertEqual(result["outcome"], request_service.OUTCOME_BLOCKED) + self.assertEqual( + result["reason_code"], request_service.REASON_ALLOCATOR_OUTCOME + ) + self.assertFalse(result["mutation_performed"]) + + def test_allocator_drift_on_apply_is_not_read_as_an_assignment(self): + """The apply call must return *this* work unit, not a substitute.""" + + def _drifting(*, request, apply, expected_candidate_set_fingerprint=None): + return { + "outcome": ( + allocator_service.OUTCOME_ASSIGNED + if apply + else allocator_service.OUTCOME_PREVIEW + ), + "selected": _selection(number=999 if apply else 643), + "assignment": {"assignment_id": "asn-wrong"} if apply else None, + "candidate_set_fingerprint": "fp-test", + } + + result = request_service.apply_request( + _request(number=643), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_drifting, + claims_source=_no_claims, + ) + self.assertFalse(result["ok"]) + self.assertIsNone(result["assignment"]) + self.assertFalse(result["mutation_performed"]) + + def test_viewer_cannot_apply(self): + result = request_service.apply_request( + _request(), + principal=_principal(console_authz.VIEWER), + confirm=True, + allocator=_fake_allocator(), + claims_source=_no_claims, + ) + self.assertFalse(result["ok"]) + self.assertEqual(result["reason_code"], request_service.REASON_UNAUTHORIZED) + + def test_anonymous_cannot_apply(self): + result = request_service.apply_request( + _request(), + confirm=True, + allocator=_fake_allocator(), + claims_source=_no_claims, + ) + self.assertFalse(result["ok"]) + self.assertFalse(result["mutation_performed"]) + + +class TestAllocatorIntegrationFakes(unittest.TestCase): + """The real default allocator refuses a partial inventory (#758).""" + + def test_incomplete_inventory_returns_none(self): + from webui.queue_loader import PaginationMeta, QueueSnapshot + + snapshot = QueueSnapshot( + project_id="p", + repo_label="r", + prs=(), + issues=(), + pr_pagination=PaginationMeta( + page=1, + per_page=50, + returned_count=50, + has_more=True, + is_final_page=False, + inventory_complete=False, + pages_fetched=1, + ), + issue_pagination=None, + ) + with mock.patch( + "webui.queue_loader.load_queue_snapshot", return_value=snapshot + ): + result = request_service.default_allocator( + request=_request(), apply=False + ) + self.assertIsNone(result) + + def test_fetch_error_returns_none(self): + from webui.queue_loader import QueueSnapshot + + snapshot = QueueSnapshot( + project_id="p", + repo_label="r", + prs=(), + issues=(), + pr_pagination=None, + issue_pagination=None, + fetch_error="no credentials", + ) + with mock.patch( + "webui.queue_loader.load_queue_snapshot", return_value=snapshot + ): + result = request_service.default_allocator( + request=_request(), apply=True + ) + self.assertIsNone(result) + + +class TestAuditRecords(unittest.TestCase): + """Every preview and apply is auditable, correlated, and redacted.""" + + def setUp(self): + handle = tempfile.NamedTemporaryFile( + mode="w", suffix=".jsonl", delete=False + ) + handle.close() + self.sink = handle.name + self.addCleanup( + lambda: os.path.exists(self.sink) and os.remove(self.sink) + ) + patcher = mock.patch.dict( + os.environ, {console_audit.AUDIT_LOG_ENV: self.sink} + ) + patcher.start() + self.addCleanup(patcher.stop) + + def _records(self) -> list[dict[str, Any]]: + with open(self.sink, encoding="utf-8") as handle: + return [json.loads(line) for line in handle if line.strip()] + + def test_preview_is_audited_with_a_correlation_id(self): + preview = request_service.preview_request( + _request(), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(), + claims_source=_no_claims, + ) + records = self._records() + self.assertEqual(len(records), 1) + record = records[0] + self.assertEqual(record["action"], request_service.ACTION_ID) + self.assertEqual(record["result"], console_audit.RESULT_PREVIEWED) + self.assertEqual( + record["correlation"]["request_id"], preview.correlation_id + ) + self.assertEqual(record["target"]["ref"], "#643") + + def test_denied_apply_is_audited(self): + with mock.patch.dict(os.environ, {EXEC_FLAG: ""}): + request_service.apply_request( + _request(), + principal=_principal(console_authz.VIEWER), + confirm=True, + allocator=_fake_allocator(), + claims_source=_no_claims, + ) + self.assertEqual(self._records()[-1]["result"], console_audit.RESULT_DENIED) + + def test_assignment_is_audited_and_correlated(self): + with mock.patch.dict(os.environ, {EXEC_FLAG: "1"}): + result = request_service.apply_request( + _request(), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_fake_allocator(), + claims_source=_no_claims, + ) + record = self._records()[-1] + self.assertEqual(record["result"], console_audit.RESULT_SUCCEEDED) + self.assertEqual( + record["correlation"]["request_id"], result["correlation_id"] + ) + self.assertEqual( + record["metadata"]["assignment_id"], + result["assignment"]["assignment_id"], + ) + + def test_intent_bearing_a_secret_is_not_persisted_raw(self): + request_service.preview_request( + _request(intent="use token=ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(), + claims_source=_no_claims, + ) + for record in self._records(): + with self.subTest(event=record.get("event_id")): + self.assertFalse(scan_for_secrets(record)) + + +class TestRequestRoutes(unittest.TestCase): + """The HTTP surface: form page, preview API, apply API.""" + + def setUp(self): + self.client = TestClient(create_app()) + + def test_requests_page_renders_form(self): + response = self.client.get("/requests") + self.assertEqual(response.status_code, 200) + body = response.text + self.assertIn("Requests", body) + self.assertIn("desired_role", body) + self.assertIn("intent_summary", body) + + def test_requests_page_is_linked_from_nav(self): + from webui.nav import nav_hrefs + + self.assertIn("/requests", nav_hrefs()) + + def test_preview_api_rejects_an_invalid_request(self): + response = self.client.post( + "/api/v1/requests/preview", + json={ + "desired_role": "wizard", + "work_kind": "issue", + "work_number": 1, + "intent_summary": "x", + **SCOPE, + }, + ) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json()["reason_code"], "unknown_role") + + def test_preview_api_denies_anonymous(self): + response = self.client.post( + "/api/v1/requests/preview", + json={ + "desired_role": "author", + "work_kind": "issue", + "work_number": 643, + "intent_summary": "x", + **SCOPE, + }, + ) + self.assertEqual(response.status_code, 403) + payload = response.json() + self.assertFalse(payload["authorized"]) + self.assertFalse(payload["mutation_performed"]) + + def test_apply_api_denies_anonymous(self): + response = self.client.post( + "/api/v1/requests/apply", + json={ + "desired_role": "author", + "work_kind": "issue", + "work_number": 643, + "intent_summary": "x", + "confirm": True, + **SCOPE, + }, + ) + self.assertEqual(response.status_code, 403) + payload = response.json() + self.assertFalse(payload["ok"]) + self.assertFalse(payload["mutation_performed"]) + self.assertIsNone(payload["assignment"]) + + def test_apply_api_rejects_an_invalid_request(self): + response = self.client.post( + "/api/v1/requests/apply", + json={ + "desired_role": "author", + "work_kind": "issue", + "work_number": -1, + "intent_summary": "x", + **SCOPE, + }, + ) + self.assertEqual(response.status_code, 400) + + def test_request_apis_are_post_only(self): + """GET is not a way in. The app's 405 handler renders a read-only + method against a write route as 404, so that is what is asserted.""" + for path in ("/api/v1/requests/preview", "/api/v1/requests/apply"): + with self.subTest(path=path): + self.assertEqual(self.client.get(path).status_code, 404) + + def test_form_post_previews_and_never_assigns(self): + response = self.client.post( + "/requests", + data={ + "desired_role": "author", + "work_kind": "issue", + "work_number": "643", + "intent_summary": "implement the request surface", + "remote": "prgs", + "org": "Scaled-Tech-Consulting", + "repo": "Gitea-Tools", + }, + ) + self.assertEqual(response.status_code, 200) + self.assertIn("Intent preview", response.text) + + +class TestRenderingSafety(unittest.TestCase): + """AC5 — the page escapes hostile input and shows no secret.""" + + def test_intent_is_escaped(self): + preview = request_service.preview_request( + _request(intent=""), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(), + claims_source=_no_claims, + audit=False, + ) + html = render_requests_page(preview=preview) + self.assertNotIn("", html) + self.assertIn("<script>", html) + + def test_page_renders_a_denial_without_a_preview(self): + _, error = request_service.parse_request( + { + "desired_role": "wizard", + "work_kind": "issue", + "work_number": 1, + "intent_summary": "x", + **SCOPE, + } + ) + html = render_requests_page(error=error) + self.assertIn("Request rejected", html) + self.assertIn("unknown_role", html) + + def test_page_shows_no_credential_material(self): + preview = request_service.preview_request( + _request(), + principal=_principal(console_authz.OPERATOR), + allocator=_fake_allocator(), + claims_source=_no_claims, + audit=False, + ) + html = render_requests_page(preview=preview) + for needle in ("token=", "Bearer ", "password"): + with self.subTest(needle=needle): + self.assertNotIn(needle, html) + + +if __name__ == "__main__": # pragma: no cover + unittest.main() diff --git a/webui/app.py b/webui/app.py index cedcef1..9526f49 100644 --- a/webui/app.py +++ b/webui/app.py @@ -67,6 +67,8 @@ from webui.system_health import ( snapshot_to_dict as system_health_to_dict, ) from webui.system_health_views import render_system_health_page +from webui import request_service +from webui.request_views import render_requests_page _READ_ONLY_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) _AUDIT_MUTATION_PATHS = frozenset({"/audit", "/api/audit"}) @@ -722,6 +724,109 @@ async def api_v1_analytics_ingest(request: Request) -> JSONResponse: ) +def _default_request_scope() -> dict[str, str]: + """Resolve remote/org/repo from the project registry for request forms. + + Returns an empty mapping when the registry cannot be read, which makes + ``parse_request`` reject a request that did not name its own scope rather + than letting it default to some other repository. + """ + from webui.queue_loader import _host_from_url # host normalisation helper + + registry, error = _load_project_registry() + if error is not None or not registry.projects: + return {} + project = registry.projects[0] + host = _host_from_url(project.remote_host) + return { + "remote": _derive_remote(host), + "org": project.gitea_owner or "", + "repo": project.repo_name or "", + } + + +async def _request_payload(request: Request) -> dict[str, object]: + """Read a request body as JSON or form-encoded. Never raises.""" + content_type = (request.headers.get("content-type") or "").lower() + if "application/json" in content_type: + try: + body = await request.json() + except Exception: + return {} + return dict(body) if isinstance(body, dict) else {} + try: + form = await request.form() + except Exception: + return {} + return {key: form[key] for key in form} + + +async def requests_page(request: Request) -> HTMLResponse: + """Operator request form and intent preview (#643). + + POST here only ever *previews*. Initiation is a separate confirmed call to + ``/api/v1/requests/apply`` so that submitting this form cannot reserve + work as a side effect. + """ + submitted: dict[str, object] = {} + preview = None + error = None + if request.method == "POST": + submitted = await _request_payload(request) + work_request, error = request_service.parse_request( + submitted, default_scope=_default_request_scope() + ) + if work_request is not None: + preview = request_service.preview_request( + work_request, + principal=resolve_principal(headers=dict(request.headers)), + ) + return HTMLResponse( + render_requests_page( + preview=preview, error=error, submitted=submitted + ) + ) + + +async def api_v1_request_preview(request: Request) -> JSONResponse: + """Dry-run authorization and intent preview for a work request (#643).""" + payload = await _request_payload(request) + work_request, error = request_service.parse_request( + payload, default_scope=_default_request_scope() + ) + if work_request is None: + return JSONResponse(error.to_dict(), status_code=400) + preview = request_service.preview_request( + work_request, + principal=resolve_principal(headers=dict(request.headers)), + ) + return JSONResponse( + preview.to_dict(), status_code=200 if preview.authorized else 403 + ) + + +async def api_v1_request_apply(request: Request) -> JSONResponse: + """Initiate a previewed work request through the allocator (#643). + + Fail-closed at every step: unauthorized, unconfirmed, not-next-safe, and + already-claimed all return without attempting an assignment. + """ + payload = await _request_payload(request) + work_request, error = request_service.parse_request( + payload, default_scope=_default_request_scope() + ) + if work_request is None: + return JSONResponse(error.to_dict(), status_code=400) + confirm = _truthy_flag(str(payload.get("confirm") or "")) + result = request_service.apply_request( + work_request, + principal=resolve_principal(headers=dict(request.headers)), + confirm=confirm, + ) + status = int(result.pop("status_code", 403)) + return JSONResponse(result, status_code=status) + + async def method_not_allowed(request: Request, _exc: Exception) -> Response: path = request.url.path if path in _AUDIT_MUTATION_PATHS and request.method == "POST": @@ -786,6 +891,17 @@ def create_app(*, bind_host: str | None = None) -> Starlette: api_action_attempt, methods=["POST"], ), + Route("/requests", requests_page, methods=["GET", "POST"]), + Route( + "/api/v1/requests/preview", + api_v1_request_preview, + methods=["POST"], + ), + Route( + "/api/v1/requests/apply", + api_v1_request_apply, + methods=["POST"], + ), Route("/api/leases", api_leases, methods=["GET"]), Route("/api/v1/inventory", api_inventory, methods=["GET"]), Route( diff --git a/webui/console_authz.py b/webui/console_authz.py index 0c51037..282d831 100644 --- a/webui/console_authz.py +++ b/webui/console_authz.py @@ -115,6 +115,12 @@ class ConsoleAction: break_glass: bool phase: int summary: str + # Opt-in switch for an action whose execution path is genuinely wired + # ahead of its phase becoming globally active (#643). Naming a variable + # here enables nothing on its own: the variable must also be set in the + # environment. An action that leaves this ``None`` can only execute once + # ACTIVE_PHASE reaches its phase, exactly as before. + execution_env_flag: str | None = None @property def mcp_permission(self) -> str: @@ -277,6 +283,27 @@ _ACTION_SPECS: tuple[ConsoleAction, ...] = ( phase=2, summary="Restart one MCP namespace via the host supervisor.", ), + # #643: submit a work request — desired role, issue/PR, intent — and let + # the allocator reserve it. This is the one Phase 2 action whose execution + # path is actually implemented (``webui.request_service``), so it carries + # the opt-in flag; it stays denied until an operator sets that variable. + # Authority is operator-class because the outcome is a claim, not a Gitea + # verdict: initiating reviewer or merger *work* does not grant the right + # to approve or merge, which stays with the MCP role profile. + ConsoleAction( + action_id="initiate_workflow", + task_key="allocate_next_work", + action_class=CLASS_WRITE, + minimum_role=OPERATOR, + requires_confirmation=True, + dual_control=False, + break_glass=False, + phase=2, + summary=( + "Preview and initiate allocator-owned workflow work for a role." + ), + execution_env_flag="WEBUI_REQUESTS_EXECUTION", + ), ) ACTIONS: dict[str, ConsoleAction] = {a.action_id: a for a in _ACTION_SPECS} @@ -430,6 +457,33 @@ ALLOW_PREVIEW = "allowed_preview_only" # gated on this model landing; nothing here enables it. ACTIVE_PHASE = 1 +_TRUTHY = frozenset({"1", "true", "yes", "on"}) + + +def execution_wired( + action: ConsoleAction | None, env: dict[str, str] | None = None +) -> bool: + """Whether *action* has a live execution path right now. + + Two ways to be wired, and only two. The action's phase is active, or the + action declares an opt-in environment variable *and* that variable is set. + Everything else — including every action that never declares a flag — is + unwired, so the default across the registry stays deny. + + Bumping ``ACTIVE_PHASE`` would enable execution for every action of that + phase at once. The per-action flag exists so a single implemented action + can go live without dragging its unimplemented phase-mates with it. + """ + if action is None: + return False + if action.phase <= ACTIVE_PHASE: + return True + flag = (action.execution_env_flag or "").strip() + if not flag: + return False + source = env if env is not None else os.environ + return (source.get(flag) or "").strip().lower() in _TRUTHY + @dataclass(frozen=True) class AuthorizationDecision: @@ -469,16 +523,19 @@ def authorize( principal: Principal | None = None, *, for_execution: bool = False, + env: dict[str, str] | None = None, ) -> AuthorizationDecision: """Decide whether *principal* may invoke *action_id*. Deny by default. ``for_execution`` distinguishes a read-only preview from a real invocation. - Even an allowed decision reports ``execution_enabled=False`` while the - console is in Phase 1, so no caller can read an allow as permission to - mutate. + ``execution_enabled`` reports whether the action has a live execution path + at all (:func:`execution_wired`) — for every action without an explicit + opt-in flag that stays ``False`` while the console is in Phase 1, so no + caller can read an allow as permission to mutate. """ who = principal if principal is not None else ANONYMOUS action = get_action(action_id) + wired = execution_wired(action, env) if action is None: return AuthorizationDecision( @@ -497,7 +554,7 @@ def authorize( "requires_confirmation": action.requires_confirmation, "dual_control": action.dual_control, "break_glass": action.break_glass, - "execution_enabled": False, + "execution_enabled": wired, } if not who.authenticated: @@ -530,13 +587,19 @@ def authorize( **base, ) - if for_execution and action.phase > ACTIVE_PHASE: + if for_execution and not wired: return AuthorizationDecision( allowed=False, reason_code=DENY_PHASE_NOT_ACTIVE, detail=( f"Action {action_id!r} belongs to phase {action.phase}; the " - f"console is in phase {ACTIVE_PHASE}. Execution is not wired." + f"console is in phase {ACTIVE_PHASE}" + + ( + f" and {action.execution_env_flag} is not set" + if action.execution_env_flag + else "" + ) + + ". Execution is not wired." ), **base, ) @@ -545,8 +608,8 @@ def authorize( allowed=True, reason_code=ALLOW_PREVIEW, detail=( - "Principal holds the required role. Preview only — execution " - "remains disabled until the Phase 2 action framework ships." + "Principal holds the required role. Execution proceeds only for an " + "action with a wired execution path; everything else is preview." ), **base, ) diff --git a/webui/nav.py b/webui/nav.py index 129b2a4..dbde5e1 100644 --- a/webui/nav.py +++ b/webui/nav.py @@ -45,6 +45,7 @@ NAV_GROUPS: tuple[NavGroup, ...] = ( NavItem("/queue", "Queue"), NavItem("/leases", "Leases"), NavItem("/actions", "Actions"), + NavItem("/requests", "Requests"), )), NavGroup("Runtime/Sessions", ( NavItem("/runtime", "Runtime health"), diff --git a/webui/request_service.py b/webui/request_service.py new file mode 100644 index 0000000..6941027 --- /dev/null +++ b/webui/request_service.py @@ -0,0 +1,967 @@ +"""Operator work-request preview and initiation (#643, Phase 2). + +An operator's alternative to pasting a role prompt into a terminal. A +*request* names three things — the role to run as, the issue or PR to run +against, and what the operator intends — and this module answers two questions +about it: + +* **Preview** (:func:`preview_request`) — would that request be authorized, + is the work unit actually free, is it the next safe thing that role should + touch, and which actions stay prohibited? Read-only, always. It creates no + assignment and never mutates. +* **Initiate** (:func:`apply_request`) — turn an authorized request into an + *exclusive assignment*, and only ever through the allocator. + +Three invariants hold and are the reason this module exists rather than a +direct call to :func:`allocator_service.allocate_next_work` from a route: + +1. **The allocator remains the only source of exclusive ownership** (#600 / + #613). ``apply`` never assigns the requested item directly. It runs a + dry-run first and proceeds only when the allocator would independently pick + that exact item; otherwise it reports ``wait`` and mutates nothing. A + request is therefore a *confirmation* of the allocator's decision, never an + override of it. +2. **Duplicate assignment is rejected before it is attempted.** An active + claim on the work unit — held by any session, this one included — blocks. +3. **Fail closed at every unknown.** An unparseable request, an unavailable + control-plane DB, an incomplete queue inventory, or an unresolved + authorization all deny. There is no branch that proceeds on missing + evidence. + +Authorization comes from :mod:`webui.console_authz` (``initiate_workflow``) +and every outcome is audited through :mod:`webui.console_audit`, correlated to +the resulting assignment by ``correlation_id``. +""" + +from __future__ import annotations + +import uuid +from dataclasses import dataclass, field +from typing import Any, Callable, Mapping, Sequence + +import allocator_service +from task_capability_map import required_permission, required_role +from webui import console_audit, console_authz + +# The console action this module is gated by. Registered in console_authz. +ACTION_ID = "initiate_workflow" + +KIND_ISSUE = "issue" +KIND_PR = "pr" +WORK_KINDS: tuple[str, ...] = (KIND_ISSUE, KIND_PR) + +REQUESTABLE_ROLES: tuple[str, ...] = ( + allocator_service.ROLE_AUTHOR, + allocator_service.ROLE_REVIEWER, + allocator_service.ROLE_MERGER, + allocator_service.ROLE_RECONCILER, + allocator_service.ROLE_CONTROLLER, +) + +# Intent is operator prose echoed back into an audit record. Bounded so a +# pasted transcript cannot bloat the append-only log. +MAX_INTENT_CHARS = 500 + +# --- Outcomes --------------------------------------------------------------- +OUTCOME_ASSIGNED = allocator_service.OUTCOME_ASSIGNED +OUTCOME_WAIT = allocator_service.OUTCOME_WAIT +OUTCOME_BLOCKED = "blocked" +OUTCOME_DENIED = "denied" +OUTCOME_INVALID = "invalid_request" +OUTCOME_PREVIEW = allocator_service.OUTCOME_PREVIEW + +# --- Reason codes ----------------------------------------------------------- +REASON_AUTHORIZED = "request_authorized" +REASON_PREVIEW_OK = "preview_authorized" +REASON_UNAUTHORIZED = "unauthorized" +REASON_NOT_NEXT_SAFE = "not_next_safe_work" +REASON_DUPLICATE_ASSIGNMENT = "duplicate_assignment" +REASON_CONFIRMATION_REQUIRED = "confirmation_required" +REASON_EVIDENCE_UNAVAILABLE = "evidence_unavailable" +REASON_ALLOCATOR_OUTCOME = "allocator_declined" + +# --- Check names ------------------------------------------------------------ +CHECK_AUTHORIZATION = "authorization" +CHECK_CAPABILITY = "capability" +CHECK_LEASE_AVAILABILITY = "lease_availability" +CHECK_NEXT_SAFE_ACTION = "next_safe_action" +CHECK_HEAD_PIN = "head_pin" + + +# --- Request model ---------------------------------------------------------- + + +@dataclass(frozen=True) +class WorkRequest: + """One operator request: a role, a work unit, and a stated intent.""" + + desired_role: str + work_kind: str + work_number: int + intent_summary: str + remote: str + org: str + repo: str + expected_head_sha: str | None = None + + @property + def work_key(self) -> tuple[str, int]: + return (self.work_kind, self.work_number) + + @property + def display_ref(self) -> str: + return f"#{self.work_number}" + + def to_dict(self) -> dict[str, Any]: + return { + "desired_role": self.desired_role, + "work_kind": self.work_kind, + "work_number": self.work_number, + "intent_summary": self.intent_summary, + "remote": self.remote, + "org": self.org, + "repo": self.repo, + "expected_head_sha": self.expected_head_sha, + } + + +@dataclass(frozen=True) +class RequestError: + """A rejected request, with the field that caused the rejection.""" + + reason_code: str + detail: str + field_name: str | None = None + + def to_dict(self) -> dict[str, Any]: + return { + "ok": False, + "outcome": OUTCOME_INVALID, + "reason_code": self.reason_code, + "detail": self.detail, + "field": self.field_name, + } + + +def _clean(value: Any) -> str: + return str(value or "").strip() + + +def parse_request( + payload: Mapping[str, Any] | None, + *, + default_scope: Mapping[str, str] | None = None, +) -> tuple[WorkRequest | None, RequestError | None]: + """Validate an operator payload into a :class:`WorkRequest`. + + Returns ``(request, None)`` or ``(None, error)``. Never raises and never + guesses: an unknown role, an unknown work kind, or a non-positive number is + an error rather than a silently corrected value. + """ + body = dict(payload or {}) + scope = dict(default_scope or {}) + + role = _clean(body.get("desired_role") or body.get("role")).lower() + if role not in REQUESTABLE_ROLES: + return None, RequestError( + reason_code="unknown_role", + detail=( + f"desired_role must be one of {', '.join(REQUESTABLE_ROLES)}; " + f"got {role or '(empty)'!r}." + ), + field_name="desired_role", + ) + + kind = _clean(body.get("work_kind") or body.get("kind")).lower() + if kind not in WORK_KINDS: + return None, RequestError( + reason_code="unknown_work_kind", + detail=( + f"work_kind must be 'issue' or 'pr'; got {kind or '(empty)'!r}." + ), + field_name="work_kind", + ) + + raw_number = body.get("work_number") + if raw_number is None: + raw_number = ( + body.get("pr_number") if kind == KIND_PR else body.get("issue_number") + ) + if raw_number is None: + raw_number = body.get("number") + try: + number = int(str(raw_number).strip()) + except (TypeError, ValueError): + return None, RequestError( + reason_code="invalid_work_number", + detail=f"work_number must be an integer; got {raw_number!r}.", + field_name="work_number", + ) + if number <= 0: + return None, RequestError( + reason_code="invalid_work_number", + detail="work_number must be a positive issue or PR number.", + field_name="work_number", + ) + + intent = _clean(body.get("intent_summary") or body.get("intent")) + if not intent: + return None, RequestError( + reason_code="missing_intent", + detail="intent_summary is required so the audit record states why.", + field_name="intent_summary", + ) + intent = intent[:MAX_INTENT_CHARS] + + remote = _clean(body.get("remote")) or _clean(scope.get("remote")) + org = _clean(body.get("org")) or _clean(scope.get("org")) + repo = _clean(body.get("repo")) or _clean(scope.get("repo")) + if not (remote and org and repo): + return None, RequestError( + reason_code="scope_unresolved", + detail=( + "remote, org, and repo could not be resolved from the request " + "or the project registry." + ), + field_name="repo", + ) + + head = _clean(body.get("expected_head_sha")) or None + + return ( + WorkRequest( + desired_role=role, + work_kind=kind, + work_number=number, + intent_summary=intent, + remote=remote, + org=org, + repo=repo, + expected_head_sha=head, + ), + None, + ) + + +# --- Preview ---------------------------------------------------------------- + + +@dataclass(frozen=True) +class RequestCheck: + """One named precondition and its verdict.""" + + name: str + ok: bool + reason_code: str + detail: str + evidence: dict[str, Any] = field(default_factory=dict) + + def to_dict(self) -> dict[str, Any]: + return { + "name": self.name, + "ok": self.ok, + "reason_code": self.reason_code, + "detail": self.detail, + "evidence": dict(self.evidence), + } + + +@dataclass(frozen=True) +class RequestPreview: + """The full intent preview for one request. Read-only in every field.""" + + request: WorkRequest + authorized: bool + reason_code: str + detail: str + authorization: dict[str, Any] + checks: tuple[RequestCheck, ...] + prohibited_actions: tuple[str, ...] + allowed_actions: tuple[str, ...] + next_safe_action: str + required_profile: str + required_namespace: str + required_permission: str + correlation_id: str + allocator_evidence: dict[str, Any] = field(default_factory=dict) + + @property + def failed_checks(self) -> tuple[RequestCheck, ...]: + return tuple(c for c in self.checks if not c.ok) + + def to_dict(self) -> dict[str, Any]: + return { + "ok": self.authorized, + "outcome": OUTCOME_PREVIEW, + "dry_run": True, + "mutation_performed": False, + "authorized": self.authorized, + "reason_code": self.reason_code, + "detail": self.detail, + "request": self.request.to_dict(), + "authorization": dict(self.authorization), + "checks": [c.to_dict() for c in self.checks], + "failed_checks": [c.name for c in self.failed_checks], + "prohibited_actions": list(self.prohibited_actions), + "allowed_actions": list(self.allowed_actions), + "next_safe_action": self.next_safe_action, + "required_profile": self.required_profile, + "required_namespace": self.required_namespace, + "required_permission": self.required_permission, + "correlation_id": self.correlation_id, + "allocator_evidence": dict(self.allocator_evidence), + } + + +AllocatorFn = Callable[..., dict[str, Any] | None] +ClaimsFn = Callable[["WorkRequest"], Mapping[tuple[str, int], dict[str, Any]]] + + +def _correlation_id() -> str: + return f"req-{uuid.uuid4().hex}" + + +def _selection_matches( + selection: Mapping[str, Any] | None, request: WorkRequest +) -> bool: + if not selection: + return False + kind = _clean(selection.get("kind")).lower() + try: + number_int = int(selection.get("number")) + except (TypeError, ValueError): + return False + return (kind, number_int) == request.work_key + + +def _authorization_check( + decision: console_authz.AuthorizationDecision, +) -> RequestCheck: + return RequestCheck( + name=CHECK_AUTHORIZATION, + ok=bool(decision.allowed), + reason_code=decision.reason_code, + detail=decision.detail, + evidence={ + "subject": decision.principal.subject, + "role": decision.principal.role, + "required_role": decision.required_role, + "identity_source": decision.principal.identity_source, + }, + ) + + +def _capability_check(request: WorkRequest) -> RequestCheck: + """Whether the requested role maps to a declared MCP capability. + + The console never invents an authority: the permission and role come from + ``task_capability_map`` via the same ``allocate_next_work`` task the MCP + allocator gates on. + """ + # The remote-prefixed hint keeps a dadeschools request from being told to + # run under a prgs profile; ``required_profile_for_role`` preserves the + # prefix when one is present and falls back to its own default otherwise. + profile_hint = f"{request.remote}-{request.desired_role}" + try: + profile = allocator_service.required_profile_for_role( + request.desired_role, profile_name=profile_hint + ) + namespace = allocator_service.required_namespace_for_role( + request.desired_role, profile_name=profile_hint + ) + except Exception as exc: # noqa: BLE001 — an unresolved role is a denial + return RequestCheck( + name=CHECK_CAPABILITY, + ok=False, + reason_code="capability_unresolved", + detail=( + f"no profile/namespace maps to role {request.desired_role!r}: " + f"{exc}" + ), + ) + resolved = bool(profile and namespace) + return RequestCheck( + name=CHECK_CAPABILITY, + ok=resolved, + reason_code="capability_resolved" if resolved else "capability_unresolved", + detail=( + f"role {request.desired_role!r} runs under profile {profile!r} in " + f"MCP namespace {namespace!r}." + ), + evidence={ + "required_profile": profile, + "required_namespace": namespace, + "required_permission": required_permission("allocate_next_work"), + "capability_role": required_role("allocate_next_work"), + }, + ) + + +def _lease_check( + request: WorkRequest, + claims: Mapping[tuple[str, int], dict[str, Any]] | None, +) -> RequestCheck: + """Whether the work unit is free of an active claim. + + ``claims is None`` means the control-plane DB could not be read. That is a + failure, not an absence of claims: an unreadable substrate must never read + as "nothing holds this". + """ + if claims is None: + return RequestCheck( + name=CHECK_LEASE_AVAILABILITY, + ok=False, + reason_code=REASON_EVIDENCE_UNAVAILABLE, + detail=( + "active-claim inventory is unavailable; refusing to treat an " + "unreadable control-plane DB as an unclaimed work unit." + ), + ) + claim = claims.get(request.work_key) + if claim: + return RequestCheck( + name=CHECK_LEASE_AVAILABILITY, + ok=False, + reason_code=REASON_DUPLICATE_ASSIGNMENT, + detail=( + f"{request.work_kind} {request.display_ref} already carries an " + f"active {claim.get('role') or 'unknown'} lease." + ), + evidence={ + "lease_id": claim.get("lease_id"), + "session_id": claim.get("session_id"), + "role": claim.get("role"), + "expires_at": claim.get("expires_at"), + }, + ) + return RequestCheck( + name=CHECK_LEASE_AVAILABILITY, + ok=True, + reason_code="lease_available", + detail=f"no active lease holds {request.work_kind} {request.display_ref}.", + ) + + +def _next_safe_action_check( + request: WorkRequest, allocation: Mapping[str, Any] | None +) -> RequestCheck: + """Whether the allocator would independently select this exact work unit.""" + if not allocation: + return RequestCheck( + name=CHECK_NEXT_SAFE_ACTION, + ok=False, + reason_code=REASON_EVIDENCE_UNAVAILABLE, + detail="allocator dry-run produced no result; refusing to proceed.", + ) + selection = allocation.get("selected") or {} + outcome = _clean(allocation.get("outcome")) + if not _selection_matches(selection, request): + chosen = ( + f"{_clean(selection.get('kind')) or 'unknown'} #{selection.get('number')}" + if selection + else "nothing" + ) + return RequestCheck( + name=CHECK_NEXT_SAFE_ACTION, + ok=False, + reason_code=REASON_NOT_NEXT_SAFE, + detail=( + f"the allocator would select {chosen} for role " + f"{request.desired_role!r}, not {request.work_kind} " + f"{request.display_ref}. Requests confirm the allocator's " + "decision; they never override it." + ), + evidence={ + "allocator_outcome": outcome, + "allocator_selection": dict(selection), + "reasons": list(allocation.get("reasons") or ()), + }, + ) + return RequestCheck( + name=CHECK_NEXT_SAFE_ACTION, + ok=True, + reason_code="next_safe_work", + detail=( + f"the allocator selects {request.work_kind} {request.display_ref} " + f"for role {request.desired_role!r}." + ), + evidence={ + "allocator_outcome": outcome, + "selected_action": _clean(selection.get("selected_action")), + "expected_role_next": _clean(selection.get("expected_role_next")), + }, + ) + + +def _head_pin_check( + request: WorkRequest, allocation: Mapping[str, Any] | None +) -> RequestCheck: + """PR work must be pinned to a head SHA; issue work has nothing to pin.""" + if request.work_kind != KIND_PR: + return RequestCheck( + name=CHECK_HEAD_PIN, + ok=True, + reason_code="head_pin_not_applicable", + detail="issue work carries no head SHA to pin.", + ) + selection = (allocation or {}).get("selected") or {} + allocator_head = _clean(selection.get("head_sha")) or None + if not allocator_head: + return RequestCheck( + name=CHECK_HEAD_PIN, + ok=False, + reason_code=REASON_EVIDENCE_UNAVAILABLE, + detail=( + "the allocator reported no head SHA for this PR; PR work " + "cannot be initiated unpinned." + ), + ) + if request.expected_head_sha and request.expected_head_sha != allocator_head: + return RequestCheck( + name=CHECK_HEAD_PIN, + ok=False, + reason_code="head_moved", + detail=( + "the requested head SHA does not match the PR's current head; " + "re-preview against the live head before initiating." + ), + evidence={ + "requested_head_sha": request.expected_head_sha, + "current_head_sha": allocator_head, + }, + ) + return RequestCheck( + name=CHECK_HEAD_PIN, + ok=True, + reason_code="head_pinned", + detail=f"PR {request.display_ref} is pinned at {allocator_head}.", + evidence={"head_sha": allocator_head}, + ) + + +def _next_safe_action_text( + request: WorkRequest, checks: Sequence[RequestCheck], authorized: bool +) -> str: + if authorized: + return ( + f"Confirm and initiate {request.desired_role} work on " + f"{request.work_kind} {request.display_ref} via the allocator." + ) + for check in checks: + if not check.ok: + return f"Resolve {check.name}: {check.detail}" + return "No safe action; the request is not authorized." + + +def preview_request( + request: WorkRequest, + *, + principal: console_authz.Principal | None = None, + allocator: AllocatorFn | None = None, + claims_source: ClaimsFn | None = None, + correlation_id: str | None = None, + audit: bool = True, +) -> RequestPreview: + """Build the read-only intent preview for *request*. Never mutates.""" + who = principal or console_authz.ANONYMOUS + corr = correlation_id or _correlation_id() + decision = console_authz.authorize(ACTION_ID, who, for_execution=False) + + allocation: dict[str, Any] | None = None + claims: Mapping[tuple[str, int], dict[str, Any]] | None = None + checks: list[RequestCheck] = [_authorization_check(decision)] + if decision.allowed: + # An unauthorized principal never reaches the allocator or the + # control-plane DB: a denial must not double as a queue oracle. + allocation = _run_allocator(request, allocator, apply=False) + claims = _load_claims(request, claims_source) + checks.append(_capability_check(request)) + checks.append(_lease_check(request, claims)) + checks.append(_next_safe_action_check(request, allocation)) + checks.append(_head_pin_check(request, allocation)) + + authorized = all(c.ok for c in checks) + allowed_actions, prohibited_actions = allocator_service.role_actions( + request.desired_role + ) + capability = next((c for c in checks if c.name == CHECK_CAPABILITY), None) + evidence = capability.evidence if capability else {} + + if authorized: + reason_code = REASON_PREVIEW_OK + detail = ( + "Request is authorized. Preview only — nothing has been assigned." + ) + else: + first_failure = next(c for c in checks if not c.ok) + reason_code, detail = first_failure.reason_code, first_failure.detail + + preview = RequestPreview( + request=request, + authorized=authorized, + reason_code=reason_code, + detail=detail, + authorization=decision.to_dict(), + checks=tuple(checks), + prohibited_actions=tuple(prohibited_actions), + allowed_actions=tuple(allowed_actions), + next_safe_action=_next_safe_action_text(request, checks, authorized), + required_profile=str(evidence.get("required_profile") or ""), + required_namespace=str(evidence.get("required_namespace") or ""), + required_permission=str(evidence.get("required_permission") or ""), + correlation_id=corr, + allocator_evidence=_allocator_evidence(allocation), + ) + + if audit: + _audit( + request, + result=console_audit.RESULT_PREVIEWED, + decision=decision, + principal=who, + reason_code=reason_code, + detail=detail, + correlation_id=corr, + metadata={ + "intent_summary": request.intent_summary, + "desired_role": request.desired_role, + "authorized": authorized, + "failed_checks": [c.name for c in preview.failed_checks], + "phase": "preview", + }, + ) + return preview + + +# --- Initiation ------------------------------------------------------------- + + +def apply_request( + request: WorkRequest, + *, + principal: console_authz.Principal | None = None, + confirm: bool = False, + allocator: AllocatorFn | None = None, + claims_source: ClaimsFn | None = None, + correlation_id: str | None = None, +) -> dict[str, Any]: + """Initiate *request* as an exclusive assignment, or refuse. + + The only path to an assignment is the allocator agreeing, on a dry-run, + that this work unit is what the requested role should take next. Every + refusal returns before any mutation is attempted. + """ + who = principal or console_authz.ANONYMOUS + corr = correlation_id or _correlation_id() + + execution_decision = console_authz.authorize(ACTION_ID, who, for_execution=True) + authorization = execution_decision.to_dict() + + def _refuse( + outcome: str, + reason_code: str, + detail: str, + *, + status: int, + extra: dict[str, Any] | None = None, + ) -> dict[str, Any]: + _audit( + request, + result=console_audit.RESULT_DENIED, + decision=execution_decision, + principal=who, + reason_code=reason_code, + detail=detail, + correlation_id=corr, + metadata={ + "intent_summary": request.intent_summary, + "desired_role": request.desired_role, + "phase": "apply", + "outcome": outcome, + }, + ) + payload: dict[str, Any] = { + "ok": False, + "outcome": outcome, + "reason_code": reason_code, + "detail": detail, + "request": request.to_dict(), + "authorization": authorization, + "assignment": None, + "correlation_id": corr, + "mutation_performed": False, + "status_code": status, + } + payload.update(extra or {}) + return payload + + if not (execution_decision.allowed and execution_decision.execution_enabled): + return _refuse( + OUTCOME_DENIED, + REASON_UNAUTHORIZED, + execution_decision.detail, + status=403, + ) + + # Confirmation is a property of the action in the RBAC model, so it is read + # from there rather than assumed here. + action = console_authz.get_action(ACTION_ID) + if action is not None and action.requires_confirmation and not confirm: + return _refuse( + OUTCOME_DENIED, + REASON_CONFIRMATION_REQUIRED, + ( + "This action requires explicit confirmation. Re-submit with " + "confirm=true after reviewing the preview." + ), + status=409, + ) + + preview = preview_request( + request, + principal=who, + allocator=allocator, + claims_source=claims_source, + correlation_id=corr, + audit=False, + ) + if not preview.authorized: + outcome = ( + OUTCOME_BLOCKED + if preview.reason_code == REASON_DUPLICATE_ASSIGNMENT + else OUTCOME_WAIT + ) + return _refuse( + outcome, + preview.reason_code, + preview.detail, + status=409, + extra={"preview": preview.to_dict()}, + ) + + fingerprint = ( + _clean(preview.allocator_evidence.get("candidate_set_fingerprint")) or None + ) + allocation = _run_allocator( + request, + allocator, + apply=True, + expected_candidate_set_fingerprint=fingerprint, + ) + if not allocation: + return _refuse( + OUTCOME_WAIT, + REASON_EVIDENCE_UNAVAILABLE, + "the allocator returned no result; nothing was assigned.", + status=503, + ) + + assignment = allocation.get("assignment") or None + outcome = _clean(allocation.get("outcome")) + assigned = bool( + outcome == allocator_service.OUTCOME_ASSIGNED + and assignment + and _selection_matches(allocation.get("selected"), request) + ) + if not assigned: + blocked = outcome in { + allocator_service.OUTCOME_BLOCKED_LEASE, + allocator_service.OUTCOME_BLOCKED_TERMINAL, + allocator_service.OUTCOME_BLOCKED_EXCLUDED_OWN_LEASE, + } + return _refuse( + OUTCOME_BLOCKED if blocked else OUTCOME_WAIT, + REASON_ALLOCATOR_OUTCOME, + ( + f"the allocator returned {outcome or 'no outcome'} rather than " + "an assignment for this work unit; nothing was assigned." + ), + status=409, + extra={"allocator_evidence": _allocator_evidence(allocation)}, + ) + + _audit( + request, + result=console_audit.RESULT_SUCCEEDED, + decision=execution_decision, + principal=who, + reason_code=REASON_AUTHORIZED, + detail=( + f"assigned {request.work_kind} {request.display_ref} to role " + f"{request.desired_role}." + ), + correlation_id=corr, + metadata={ + "intent_summary": request.intent_summary, + "desired_role": request.desired_role, + "phase": "apply", + "outcome": OUTCOME_ASSIGNED, + "assignment_id": assignment.get("assignment_id"), + "lease_id": assignment.get("lease_id"), + }, + ) + return { + "ok": True, + "outcome": OUTCOME_ASSIGNED, + "reason_code": REASON_AUTHORIZED, + "detail": ( + "Exclusive assignment created via the allocator. Continue in the " + f"{preview.required_namespace or 'assigned'} MCP namespace." + ), + "request": request.to_dict(), + "authorization": authorization, + "assignment": dict(assignment), + "handoff": { + "assignment_id": assignment.get("assignment_id"), + "lease_id": assignment.get("lease_id"), + "session_id": assignment.get("session_id"), + "required_profile": preview.required_profile, + "required_namespace": preview.required_namespace, + "allowed_actions": list(preview.allowed_actions), + "forbidden_actions": list(preview.prohibited_actions), + "expected_head_sha": assignment.get("expected_head_sha"), + }, + "correlation_id": corr, + "mutation_performed": True, + "status_code": 201, + "allocator_evidence": _allocator_evidence(allocation), + } + + +# --- Adapters --------------------------------------------------------------- + + +def _allocator_evidence(allocation: Mapping[str, Any] | None) -> dict[str, Any]: + """Reduce an allocator result to the non-secret fields worth surfacing.""" + if not allocation: + return {} + return { + "outcome": allocation.get("outcome"), + "selected": allocation.get("selected"), + "reasons": list(allocation.get("reasons") or ()), + "candidate_set_fingerprint": allocation.get("candidate_set_fingerprint"), + "candidate_count": allocation.get("candidate_count"), + "inventory_complete": allocation.get("inventory_complete"), + "selection_policy": allocation.get("selection_policy"), + "substrate": allocation.get("substrate"), + } + + +def _run_allocator( + request: WorkRequest, + allocator: AllocatorFn | None, + *, + apply: bool, + expected_candidate_set_fingerprint: str | None = None, +) -> dict[str, Any] | None: + fn = allocator or default_allocator + try: + result = fn( + request=request, + apply=apply, + expected_candidate_set_fingerprint=expected_candidate_set_fingerprint, + ) + except Exception: # noqa: BLE001 — an allocator failure denies, never proceeds + return None + return result if isinstance(result, dict) else None + + +def _load_claims( + request: WorkRequest, claims_source: ClaimsFn | None +) -> Mapping[tuple[str, int], dict[str, Any]] | None: + fn = claims_source or default_claims_source + try: + claims = fn(request) + except Exception: # noqa: BLE001 — an unreadable substrate is a denial + return None + return claims if isinstance(claims, Mapping) else None + + +def default_claims_source( + request: WorkRequest, +) -> Mapping[tuple[str, int], dict[str, Any]]: + """Live active-claim inventory from the #613 control-plane DB.""" + import control_plane_db + + db = control_plane_db.ControlPlaneDB() + return db.list_active_claims( + remote=request.remote, org=request.org, repo=request.repo + ) + + +def default_allocator( + *, + request: WorkRequest, + apply: bool, + expected_candidate_set_fingerprint: str | None = None, +) -> dict[str, Any] | None: + """Run the real allocator over the live queue for *request*'s scope. + + An incomplete candidate inventory returns ``None`` rather than a ranking + over a partial set (#758): selecting from a short list can pick the wrong + work unit, so the request denies instead. + """ + import control_plane_db + + from webui.queue_loader import load_queue_snapshot + from webui.traffic_loader import candidates_from_queue_snapshot + + snapshot = load_queue_snapshot() + if snapshot.fetch_error: + return None + for pagination in (snapshot.pr_pagination, snapshot.issue_pagination): + if pagination is not None and not pagination.inventory_complete: + return None + + candidates = candidates_from_queue_snapshot(snapshot) + db = control_plane_db.ControlPlaneDB() + result = allocator_service.allocate_next_work( + db, + session_id=f"webui-request-{uuid.uuid4().hex[:12]}", + role=request.desired_role, + remote=request.remote, + org=request.org, + repo=request.repo, + candidates=candidates, + apply=bool(apply), + allocation_mode="role_scoped", + expected_candidate_set_fingerprint=expected_candidate_set_fingerprint, + ) + if isinstance(result, dict): + result.setdefault("candidate_count", len(candidates)) + result.setdefault("inventory_complete", True) + result.setdefault("selection_policy", allocator_service.SELECTION_POLICY) + return result + + +# --- Audit ------------------------------------------------------------------ + + +def _audit( + request: WorkRequest, + *, + result: str, + decision: console_authz.AuthorizationDecision, + principal: console_authz.Principal, + reason_code: str, + detail: str, + correlation_id: str, + metadata: dict[str, Any], +) -> dict[str, Any]: + return console_audit.record_event( + action_id=ACTION_ID, + result=result, + decision=decision, + principal=principal, + target={ + "kind": request.work_kind, + "ref": request.display_ref, + "remote": request.remote, + "org": request.org, + "repo": request.repo, + }, + reason_code=reason_code, + request_id=correlation_id, + detail=detail, + metadata=metadata, + ) diff --git a/webui/request_views.py b/webui/request_views.py new file mode 100644 index 0000000..fe85679 --- /dev/null +++ b/webui/request_views.py @@ -0,0 +1,164 @@ +"""HTML views for the operator request surface (#643). + +The form is deliberately a *preview* form. It has no initiate button, because +initiating requires a confirmed POST to ``/api/v1/requests/apply`` and a stray +form submission must not be able to produce one by accident. + +Nothing rendered here is trusted input: every interpolated value is escaped, +and the page renders only values the service already produced rather than +echoing a raw request body back. +""" + +from __future__ import annotations + +import html +import json +from typing import Any + +from webui.layout import render_page +from webui.request_service import ( + REQUESTABLE_ROLES, + WORK_KINDS, + RequestError, + RequestPreview, +) + +REQUESTS_PATH = "/requests" +PREVIEW_API_PATH = "/api/v1/requests/preview" +APPLY_API_PATH = "/api/v1/requests/apply" + + +def _escape(text: Any) -> str: + return html.escape(str(text if text is not None else ""), quote=True) + + +REQUEST_PAGE_STYLES = """ + +""" + + +def _options(values: tuple[str, ...], selected: Any) -> str: + return "".join( + f"" + for value in values + ) + + +def _form(values: dict[str, Any] | None = None) -> str: + current = dict(values or {}) + number = current.get("work_number") + return ( + f"
" + "" + "" + "" + "" + "" + "" + "

Preview is read-only and creates no assignment. " + f"Initiating requires a confirmed POST to {APPLY_API_PATH}." + "

" + "
" + ) + + +def _checks_block(preview: RequestPreview) -> str: + rows = [] + for check in preview.checks: + verdict = "PASS" if check.ok else "FAIL" + css = "verdict-ok" if check.ok else "verdict-fail" + rows.append( + "
  • " + f"{verdict} " + f"{_escape(check.name)} — {_escape(check.detail)} " + f"({_escape(check.reason_code)})" + "
    • " + ) + return "" + + +def _preview_block(preview: RequestPreview) -> str: + verdict = "AUTHORIZED" if preview.authorized else "DENIED" + prohibited = "".join( + f"{_escape(action)}" for action in preview.prohibited_actions + ) + request = preview.request + evidence = json.dumps(preview.allocator_evidence, indent=2, default=str) + return ( + "

    Intent preview

    " + f"

    {verdict} — {_escape(preview.detail)}

    " + "

    " + f"Role {_escape(request.desired_role)} · " + f"{_escape(request.work_kind)} {_escape(request.display_ref)}" + f" · profile {_escape(preview.required_profile)} · " + f"namespace {_escape(preview.required_namespace)} · " + f"permission {_escape(preview.required_permission)}" + "

    " + f"

    Intent: {_escape(request.intent_summary)}

    " + f"{_checks_block(preview)}" + f"

    Next safe action: " + f"{_escape(preview.next_safe_action)}

    " + "

    Prohibited for this role: " + + (prohibited or "none declared") + + "

    " + "

    Correlation id " + f"{_escape(preview.correlation_id)}

    " + "
    Allocator evidence" + f"
    {_escape(evidence)}
    " + "
    " + ) + + +def _error_block(error: RequestError) -> str: + field = ( + f"

    Field: {_escape(error.field_name)}

    " + if error.field_name + else "" + ) + return ( + "

    Request rejected

    " + f"

    {_escape(error.reason_code)} — " + f"{_escape(error.detail)}

    {field}" + ) + + +def render_requests_page( + *, + preview: RequestPreview | None = None, + error: RequestError | None = None, + submitted: dict[str, Any] | None = None, +) -> str: + """Render the request form, plus a preview or rejection when one exists.""" + body = ( + "

    Requests

    " + "

    Submit a work request — desired role, issue or PR, and intent — " + "and see whether it would be authorized before anything is reserved. " + "Initiation goes through the allocator (#600/#613); this console never " + "self-selects work, never approves, and never merges.

    " + + _form(submitted) + + (_error_block(error) if error is not None else "") + + (_preview_block(preview) if preview is not None else "") + + f"

    Preview API · " + "RBAC model

    " + + REQUEST_PAGE_STYLES + ) + return render_page(title="Requests", body_html=body) diff --git a/webui/traffic_loader.py b/webui/traffic_loader.py index 912d10e..63993ff 100644 --- a/webui/traffic_loader.py +++ b/webui/traffic_loader.py @@ -201,6 +201,16 @@ def _candidates_from_queue_snapshot(q_snap: QueueSnapshot) -> list[WorkCandidate return candidates +def candidates_from_queue_snapshot(q_snap: QueueSnapshot) -> list[WorkCandidate]: + """Public alias for :func:`_candidates_from_queue_snapshot` (#643). + + The request-initiation service ranks the same candidate set this view + renders, so both must agree on how a queue row becomes a candidate. One + construction, two callers — not two that can drift apart. + """ + return _candidates_from_queue_snapshot(q_snap) + + def _claim_lease_records(inventory: dict[str, Any] | None) -> list[dict[str, Any]]: """Normalize ``build_claim_inventory`` entries into lease records. From 53ce1b1a5ec6476bbe8a153e58f9e13f2bbf1ff9 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 25 Jul 2026 03:32:24 -0400 Subject: [PATCH 02/12] fix(webui): compensate stray allocations and keep preview side-effect free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both blockers from the PR #902 review (review 589) for issue #643. B1 — an allocator-created assignment could be orphaned and reported as no mutation. apply_request re-previews, then re-runs the allocator with apply=True. The CAS fingerprint hashes only {kind, number} plus exclusions, so a competing lease taken on the requested unit inside the window leaves the fingerprint identical: the pin passes, the selection loop skips the now-claimed unit and commits an assignment on the *next* one, and _selection_matches then fails on egress. The old code returned mutation_performed False with that lease still committed and owned by a synthetic session nothing heartbeats. The window contains a second full load_queue_snapshot(), so it is seconds wide, and foreign sessions acting on this repo concurrently are an observed condition. The egress mismatch now releases the assignment the allocator created before refusing. When the release succeeds the refusal reports mutation_performed False and a compensation record; when it fails the response carries mutation_performed True, an explicit orphaned_assignment, and a gitea_release_workflow_lease reclaim action, because claiming nothing changed while a lease is live is the defect rather than a report of it. The same state was reachable through _run_allocator's bare except Exception: allocate_next_work only catches InvalidWorkKindError, LeaseRequiredError and ControlPlaneError, so anything raised after assign_and_lease committed arrived as "no result" with a durable lease. A None result on the apply path now sweeps and releases whatever this flow's session owns. That sweep is only possible because of the B2 fix below — the session id is now stable across the flow, so the lease is findable. B2 — the "read-only" preview wrote to the control-plane DB. allocate_next_work called db.upsert_session and db.expire_stale_leases unconditionally, before the apply branch was consulted, and default_allocator minted a fresh webui-request- per call. Every preview therefore appended a never-reused session row and mutated global lease state while the payload said dry_run True / mutation_performed False, driven by an operator refreshing a form. One apply wrote two rows and bound the lease to the second, which is why B1's orphan had no reclaimable owner. allocate_next_work gains a keyword-only side_effect_free flag, default False so every existing caller is byte-for-byte unchanged. Under the flag both writes are suppressed and expired leases are instead filtered out of the claim map in memory, which reaches the same selection the sweep would have produced without persisting anything; a claim whose expiry cannot be parsed is kept, since an unreadable expiry is not evidence that work is free. side_effect_free with apply=True fails closed rather than silently reserving. request_service mints one session id per request flow and threads it through both the dry-run and the apply, and the dry-run now routes through the side-effect-free path. Coverage. test_allocator_drift_on_apply_is_not_read_as_an_assignment asserted the defect — it built the orphan state and then required mutation_performed to be False, which a leak satisfies. It now requires the compensating release. Added: release-failure surfacing a reclaim action, an assignment with no lease id, the post-commit exception route, and session-id identity across the flow. The two areas the review named as having zero coverage now have it: default_allocator past its two fail-closed early returns (side_effect_free routing, session-id pass-through and minting, scope and fingerprint propagation) and default_claims_source (scoped read, and an unreadable substrate denying rather than reading as "nothing claimed"). allocator_service gains side-effect-free tests against a real temp-file DB plus the expiry-filter unit tests. Every new guard was mutation-tested by reverting it one at a time: B1 compensation removed → 3 failures; post-commit sweep removed → 1; session id re-minted per call → 2; side_effect_free ignored → 3; in-memory expiry filter removed → 1. Verification: WEBUI_TEST_OFFLINE=1 python -m pytest tests/ -q from this branches/ worktree gives 23 failed, 5263 passed, 6 skipped, 899 subtests. The sorted FAILED set is identical to the reviewer's clean-master baseline at 2f4dec83 (23 failed, 5190 passed) — no new, changed, or disappeared failure — and 21 tests were added over the reviewed head's 5242. Zero conflict markers; py_compile passes; git diff --check clean. Refs #643, PR #902 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V6xFqovhbArPv61j9KCGkL --- allocator_service.py | 122 +++++++-- tests/test_allocator_service.py | 158 ++++++++++++ tests/test_webui_request_initiation.py | 327 ++++++++++++++++++++++++- webui/request_service.py | 282 +++++++++++++++++++-- 4 files changed, 839 insertions(+), 50 deletions(-) diff --git a/allocator_service.py b/allocator_service.py index 9f32fd9..a085d25 100644 --- a/allocator_service.py +++ b/allocator_service.py @@ -23,6 +23,7 @@ import json import os import uuid from dataclasses import dataclass, field +from datetime import datetime, timezone from typing import Any, Mapping, Sequence from control_plane_db import ( @@ -738,6 +739,46 @@ def normalize_exclude_issue_numbers( return sorted(out) +def _claim_expires_at(claim: Any) -> datetime | None: + """Parse a claim's ``expires_at``, or ``None`` when it is absent/malformed.""" + if not isinstance(claim, Mapping): + return None + text = str(claim.get("expires_at") or "").strip() + if not text: + return None + if text.endswith("Z"): + text = text[:-1] + "+00:00" + try: + parsed = datetime.fromisoformat(text) + except ValueError: + return None + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed.astimezone(timezone.utc) + + +def _drop_expired_claims( + claims: Mapping[tuple[str, int], dict[str, Any]], + *, + now: datetime | None = None, +) -> dict[tuple[str, int], dict[str, Any]]: + """Claims minus those whose lease has already expired (#643). + + The read-only mirror of ``expire_stale_leases``: the sweep marks such rows + ``expired`` so they stop being returned as claims, and this reaches the same + view without writing. A claim with no parseable ``expires_at`` is **kept** — + an unreadable expiry is not evidence that work is free. + """ + moment = now or datetime.now(timezone.utc) + kept: dict[tuple[str, int], dict[str, Any]] = {} + for key, claim in (claims or {}).items(): + expires_at = _claim_expires_at(claim) + if expires_at is not None and expires_at <= moment: + continue + kept[key] = claim + return kept + + def candidate_set_fingerprint( candidates: Sequence[WorkCandidate], *, @@ -826,12 +867,22 @@ def allocate_next_work( exclude_issue_numbers: Sequence[int] | None = None, expected_candidate_set_fingerprint: str | None = None, allocation_mode: str | None = None, + side_effect_free: bool = False, ) -> dict[str, Any]: """Select and optionally reserve the next work unit via control-plane DB. *apply=False* (default): dry-run selection only — no lease/assignment. *apply=True*: atomic ``assign_and_lease`` for the selected candidate. + *side_effect_free* (#643): a dry run that writes **nothing** to the + control-plane DB. A plain ``apply=False`` still registered a session row and + swept stale leases globally, so a caller advertising a read-only preview was + mutating on every call. Under this flag both writes are suppressed and stale + leases are instead filtered out of the claim map in memory, which yields the + same selection the sweep would have produced without persisting anything. + Incompatible with *apply* — the combination fails closed rather than + silently reserving. + *allocation_mode* (#840): ``cross_role`` (default for controller) inspects the complete queue and returns one authoritative selection naming the required downstream role/profile/action. ``role_scoped`` keeps prior @@ -885,40 +936,57 @@ def allocate_next_work( "allocation_mode": (allocation_mode or "").strip() or None, } - session_id = (session_id or "").strip() or f"alloc-{uuid.uuid4().hex[:12]}" - try: - db.upsert_session( - session_id=session_id, - role=role_norm, - profile=profile_name, - pid=os.getpid(), - controller_instance_id=controller_instance_id, - ) - except Exception as exc: # noqa: BLE001 — surface structured + # A side-effect-free run may never reserve: reserving is a write, and the + # flag is the caller's assertion that this call writes nothing (#643). + if side_effect_free and apply: return { "success": False, "outcome": OUTCOME_NO_SAFE, + "apply": True, "reasons": [ - f"failed to register session in control-plane DB: {exc} " - "(fail closed, #613)" + "side_effect_free is incompatible with apply=True; an " + "assignment is a write (fail closed, #643)" ], "skipped": [], "assignment": None, "substrate": "control_plane_db", } - # Expire stale leases globally before selection. - try: - db.expire_stale_leases() - except Exception as exc: # noqa: BLE001 - return { - "success": False, - "outcome": OUTCOME_NO_SAFE, - "reasons": [f"lease expiry failed: {exc} (fail closed)"], - "skipped": [], - "assignment": None, - "substrate": "control_plane_db", - } + session_id = (session_id or "").strip() or f"alloc-{uuid.uuid4().hex[:12]}" + if not side_effect_free: + try: + db.upsert_session( + session_id=session_id, + role=role_norm, + profile=profile_name, + pid=os.getpid(), + controller_instance_id=controller_instance_id, + ) + except Exception as exc: # noqa: BLE001 — surface structured + return { + "success": False, + "outcome": OUTCOME_NO_SAFE, + "reasons": [ + f"failed to register session in control-plane DB: {exc} " + "(fail closed, #613)" + ], + "skipped": [], + "assignment": None, + "substrate": "control_plane_db", + } + + # Expire stale leases globally before selection. + try: + db.expire_stale_leases() + except Exception as exc: # noqa: BLE001 + return { + "success": False, + "outcome": OUTCOME_NO_SAFE, + "reasons": [f"lease expiry failed: {exc} (fail closed)"], + "skipped": [], + "assignment": None, + "substrate": "control_plane_db", + } terminal = None try: @@ -953,6 +1021,12 @@ def allocate_next_work( "assignment": None, "substrate": "control_plane_db", } + if side_effect_free: + # ``list_active_claims`` filters on status alone, so without the + # global sweep an already-expired lease would still read as a live + # claim and the preview would report work as taken that is free. + # Drop those in memory: same view the sweep produces, no write. + claims = _drop_expired_claims(claims) try: exclude_nums = normalize_exclude_issue_numbers(exclude_issue_numbers) diff --git a/tests/test_allocator_service.py b/tests/test_allocator_service.py index 3e252a1..5c37d0d 100644 --- a/tests/test_allocator_service.py +++ b/tests/test_allocator_service.py @@ -7,6 +7,7 @@ import tempfile import threading import unittest from concurrent.futures import ThreadPoolExecutor, as_completed +from datetime import datetime, timezone from allocator_service import ( OUTCOME_ASSIGNED, @@ -15,6 +16,7 @@ from allocator_service import ( OUTCOME_PREVIEW, OUTCOME_WAIT, WorkCandidate, + _drop_expired_claims, allocate_next_work, candidate_from_dict, classify_skip, @@ -362,5 +364,161 @@ class AllocatorServiceTest(unittest.TestCase): self.assertIn("unavailable", res["reasons"][0].lower()) +class SideEffectFreeAllocationTest(unittest.TestCase): + """``side_effect_free`` dry runs write nothing to the control plane (#643). + + A plain ``apply=False`` still called ``upsert_session`` and + ``expire_stale_leases`` before the apply branch was consulted, so a caller + advertising a read-only preview mutated on every call — one unreferenced + session row per preview, plus a global lease sweep. + """ + + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.db = ControlPlaneDB(os.path.join(self._tmp.name, "cp.sqlite3")) + + def tearDown(self) -> None: + self._tmp.cleanup() + + def _alloc(self, **kwargs): + defaults = dict( + db=self.db, + session_id="s-preview", + role="author", + remote="prgs", + org="org", + repo="repo", + candidates=[ + WorkCandidate(kind="issue", number=643, labels=("status:ready",)) + ], + apply=False, + profile_name="prgs-author", + username="jcwalker3", + ) + defaults.update(kwargs) + return allocate_next_work(**defaults) + + def _session_ids(self) -> set[str]: + return {str(r.get("session_id")) for r in self.db.list_sessions()} + + def test_side_effect_free_preview_writes_no_session_row(self): + before = self._session_ids() + result = self._alloc(side_effect_free=True) + self.assertEqual(result["outcome"], OUTCOME_PREVIEW) + self.assertEqual(self._session_ids(), before) + self.assertNotIn("s-preview", self._session_ids()) + + def test_plain_dry_run_still_registers_a_session(self): + # The default is unchanged for every existing caller. + self._alloc() + self.assertIn("s-preview", self._session_ids()) + + def test_repeated_previews_do_not_accumulate_rows(self): + for index in range(5): + self._alloc(side_effect_free=True, session_id=f"s-{index}") + self.assertEqual(self._session_ids(), set()) + + def test_side_effect_free_does_not_sweep_stale_leases(self): + self.db.upsert_session(session_id="owner", role="author", pid=1) + assigned = self.db.assign_and_lease( + session_id="owner", + role="author", + remote="prgs", + org="org", + repo="repo", + kind="issue", + number=999, + lease_ttl_seconds=-60, # already expired + ) + self.assertEqual(assigned.outcome, "assigned") + + self._alloc(side_effect_free=True) + + # The expired row is still 'active' in the DB: nothing swept it. + statuses = { + r["lease_id"]: r["status"] + for r in self.db.list_leases( + remote="prgs", org="org", repo="repo", + statuses=("active", "expired"), + ) + } + self.assertEqual(statuses.get(assigned.lease_id), "active") + + def test_expired_claims_are_filtered_in_memory_so_work_stays_selectable(self): + """The read-only mirror of the sweep: expired claims must not block.""" + self.db.upsert_session(session_id="owner", role="author", pid=1) + self.db.assign_and_lease( + session_id="owner", + role="author", + remote="prgs", + org="org", + repo="repo", + kind="issue", + number=643, + lease_ttl_seconds=-60, # expired: must not withhold #643 + ) + result = self._alloc(side_effect_free=True) + self.assertEqual(result["outcome"], OUTCOME_PREVIEW) + self.assertEqual(result["selected"]["number"], 643) + + def test_a_live_claim_still_withholds_the_work(self): + self.db.upsert_session(session_id="owner", role="author", pid=1) + self.db.assign_and_lease( + session_id="owner", + role="author", + remote="prgs", + org="org", + repo="repo", + kind="issue", + number=643, + lease_ttl_seconds=3600, + ) + result = self._alloc(side_effect_free=True) + self.assertNotEqual(result["outcome"], OUTCOME_ASSIGNED) + self.assertNotEqual((result.get("selected") or {}).get("number"), 643) + + def test_side_effect_free_with_apply_fails_closed(self): + result = self._alloc(side_effect_free=True, apply=True) + self.assertFalse(result["success"]) + self.assertEqual(result["outcome"], OUTCOME_NO_SAFE) + self.assertIsNone(result["assignment"]) + self.assertIn("incompatible with apply", result["reasons"][0]) + # And it reserved nothing. + self.assertEqual( + self.db.list_leases(remote="prgs", org="org", repo="repo"), [] + ) + + +class DropExpiredClaimsTest(unittest.TestCase): + """The in-memory expiry filter behind side-effect-free previews (#643).""" + + def test_unparseable_expiry_is_kept_rather_than_assumed_free(self): + claims = { + ("issue", 1): {"lease_id": "l1", "expires_at": "not-a-date"}, + ("issue", 2): {"lease_id": "l2"}, + ("issue", 3): {"lease_id": "l3", "expires_at": None}, + } + self.assertEqual(_drop_expired_claims(claims), claims) + + def test_expired_dropped_and_future_kept(self): + now = datetime(2026, 7, 25, 12, 0, tzinfo=timezone.utc) + claims = { + ("issue", 1): {"expires_at": "2026-07-25T11:59:59+00:00"}, + ("issue", 2): {"expires_at": "2026-07-25T12:00:01+00:00"}, + ("issue", 3): {"expires_at": "2026-07-25T12:00:00+00:00"}, # boundary + } + kept = _drop_expired_claims(claims, now=now) + self.assertEqual(set(kept), {("issue", 2)}) + + def test_naive_and_zulu_timestamps_are_treated_as_utc(self): + now = datetime(2026, 7, 25, 12, 0, tzinfo=timezone.utc) + claims = { + ("issue", 1): {"expires_at": "2026-07-25T11:00:00"}, # naive, past + ("issue", 2): {"expires_at": "2026-07-25T13:00:00Z"}, # zulu, future + } + kept = _drop_expired_claims(claims, now=now) + self.assertEqual(set(kept), {("issue", 2)}) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_webui_request_initiation.py b/tests/test_webui_request_initiation.py index c733898..e05978c 100644 --- a/tests/test_webui_request_initiation.py +++ b/tests/test_webui_request_initiation.py @@ -16,6 +16,7 @@ allocator refuses an incomplete inventory rather than ranking a partial set. from __future__ import annotations +import contextlib import json import os import pathlib @@ -89,6 +90,63 @@ def _selection( } +@contextlib.contextmanager +def _patched_control_plane( + *, + release_sink: list[tuple[str, str]] | None = None, + release_error: Exception | None = None, + leases=None, +): + """Stand in for ``control_plane_db`` so compensation paths are observable. + + The service imports the module inside the function, so patching + ``sys.modules`` is what intercepts it. No real DB is opened. + """ + module = mock.MagicMock() + db = mock.MagicMock() + + def _release(lease_id, *, session_id): + if release_error is not None: + raise release_error + if release_sink is not None: + release_sink.append((lease_id, session_id)) + + db.release_lease.side_effect = _release + db.list_leases.side_effect = leases or (lambda **_kwargs: []) + module.ControlPlaneDB.return_value = db + with mock.patch.dict(sys.modules, {"control_plane_db": module}): + yield db + + +def _drifting_allocator(*, lease_id: str | None = "lease-wrong"): + """An allocator that previews the requested unit but assigns another. + + This is the #643 B1 race in miniature: the CAS fingerprint hashes only + ``{kind, number}``, so a lease taken on the requested unit inside the window + leaves the fingerprint identical while the selection moves on. + """ + assignment: dict[str, Any] = {"assignment_id": "asn-wrong"} + if lease_id: + assignment["lease_id"] = lease_id + + def _drifting( + *, request, apply, expected_candidate_set_fingerprint=None, session_id=None + ): + return { + "outcome": ( + allocator_service.OUTCOME_ASSIGNED + if apply + else allocator_service.OUTCOME_PREVIEW + ), + "selected": _selection(number=999 if apply else 643), + "assignment": dict(assignment) if apply else None, + "candidate_set_fingerprint": "fp-test", + "session_id": session_id, + } + + return _drifting + + def _fake_allocator( *, selection: dict[str, Any] | None = None, @@ -587,30 +645,170 @@ class TestApplyOutcomes(unittest.TestCase): self.assertFalse(result["mutation_performed"]) def test_allocator_drift_on_apply_is_not_read_as_an_assignment(self): - """The apply call must return *this* work unit, not a substitute.""" + """The apply call must return *this* work unit, not a substitute. - def _drifting(*, request, apply, expected_candidate_set_fingerprint=None): + Drift is not merely refused: the allocator has already committed the + substitute assignment by the time egress rejects it, so the refusal must + also release it. Asserting only ``mutation_performed is False`` would + pass just as well against a leak. + """ + released: list[tuple[str, str]] = [] + + with _patched_control_plane(release_sink=released): + result = request_service.apply_request( + _request(number=643), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_drifting_allocator(), + claims_source=_no_claims, + ) + + self.assertFalse(result["ok"]) + self.assertIsNone(result["assignment"]) + # The substitute assignment was released, so nothing durable survives. + self.assertEqual(len(released), 1) + self.assertEqual(released[0][0], "lease-wrong") + compensation = result["compensation"] + self.assertTrue(compensation["released"]) + self.assertEqual(compensation["lease_id"], "lease-wrong") + self.assertEqual(compensation["assignment_id"], "asn-wrong") + self.assertEqual(compensation["selected"]["number"], 999) + self.assertFalse(result["mutation_performed"]) + self.assertNotIn("orphaned_assignment", result) + + def test_drift_whose_release_fails_reports_the_orphan_and_a_reclaim(self): + """A release that fails must surface the leak, never swallow it.""" + with _patched_control_plane(release_error=RuntimeError("db is read-only")): + result = request_service.apply_request( + _request(number=643), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_drifting_allocator(), + claims_source=_no_claims, + ) + + self.assertFalse(result["ok"]) + self.assertIsNone(result["assignment"]) + # A lease really is out there; saying "nothing changed" would be a lie. + self.assertTrue(result["mutation_performed"]) + orphan = result["orphaned_assignment"] + self.assertFalse(orphan["released"]) + self.assertTrue(orphan["attempted"]) + self.assertIn("db is read-only", orphan["error"]) + reclaim = orphan["reclaim_action"] + self.assertEqual(reclaim["tool"], "gitea_release_workflow_lease") + self.assertEqual(reclaim["lease_id"], "lease-wrong") + + def test_drift_without_a_lease_id_still_surfaces_a_reclaim(self): + """An assignment with no lease id cannot be released — say so.""" + result = request_service.apply_request( + _request(number=643), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_drifting_allocator(lease_id=None), + claims_source=_no_claims, + ) + self.assertFalse(result["ok"]) + self.assertTrue(result["mutation_performed"]) + orphan = result["orphaned_assignment"] + self.assertFalse(orphan["attempted"]) + self.assertEqual(orphan["assignment_id"], "asn-wrong") + self.assertIn("reclaim_action", orphan) + + def test_exception_after_commit_releases_the_session_lease(self): + """A post-commit exception surfaces as no result — with a live lease. + + ``allocate_next_work`` catches only three exception types, so anything + else raised after ``assign_and_lease`` committed reaches the caller as + ``None`` while the lease is durable. The stable per-flow session id is + what makes that lease findable. + """ + released: list[tuple[str, str]] = [] + seen: list[str | None] = [] + + def _explodes_after_commit( + *, request, apply, expected_candidate_set_fingerprint=None, session_id=None + ): + seen.append(session_id) + if not apply: + return { + "outcome": allocator_service.OUTCOME_PREVIEW, + "selected": _selection(number=643), + "assignment": None, + "candidate_set_fingerprint": "fp-test", + } + raise KeyError("selection['required_profile']") + + def _leases(**_kwargs): + return [ + { + "lease_id": "lease-committed", + "session_id": seen[-1], + "work_kind": "issue", + "work_number": 643, + } + ] + + with _patched_control_plane(release_sink=released, leases=_leases): + result = request_service.apply_request( + _request(number=643), + principal=_principal(console_authz.OPERATOR), + confirm=True, + allocator=_explodes_after_commit, + claims_source=_no_claims, + ) + + self.assertFalse(result["ok"]) + self.assertEqual( + result["reason_code"], request_service.REASON_EVIDENCE_UNAVAILABLE + ) + self.assertEqual(len(released), 1) + self.assertEqual(released[0][0], "lease-committed") + self.assertEqual( + result["compensation"]["released"][0]["lease_id"], "lease-committed" + ) + self.assertFalse(result["mutation_performed"]) + + def test_one_session_id_spans_the_dry_run_and_the_apply(self): + """Both halves of an apply share one control-plane identity.""" + seen: list[str | None] = [] + + def _recording( + *, request, apply, expected_candidate_set_fingerprint=None, session_id=None + ): + seen.append(session_id) return { "outcome": ( allocator_service.OUTCOME_ASSIGNED if apply else allocator_service.OUTCOME_PREVIEW ), - "selected": _selection(number=999 if apply else 643), - "assignment": {"assignment_id": "asn-wrong"} if apply else None, + "selected": _selection(number=643), + "assignment": ( + { + "assignment_id": "asn-ok", + "lease_id": "lease-ok", + "expected_head_sha": None, + } + if apply + else None + ), "candidate_set_fingerprint": "fp-test", + "session_id": session_id, } result = request_service.apply_request( _request(number=643), principal=_principal(console_authz.OPERATOR), confirm=True, - allocator=_drifting, + allocator=_recording, claims_source=_no_claims, ) - self.assertFalse(result["ok"]) - self.assertIsNone(result["assignment"]) - self.assertFalse(result["mutation_performed"]) + self.assertTrue(result["ok"]) + self.assertEqual(len(seen), 2) + self.assertTrue(all(s for s in seen)) + self.assertEqual(seen[0], seen[1], "dry-run and apply must share one id") + self.assertTrue(seen[0].startswith("webui-request-")) def test_viewer_cannot_apply(self): result = request_service.apply_request( @@ -634,6 +832,119 @@ class TestApplyOutcomes(unittest.TestCase): self.assertFalse(result["mutation_performed"]) +class TestDefaultAllocatorPastTheEarlyReturns(unittest.TestCase): + """``default_allocator`` beyond its two fail-closed guards (#643). + + Both prior tests returned before ``ControlPlaneDB`` was ever constructed, so + the session id, the ``side_effect_free`` routing and the CAS round-trip had + no coverage at all — which is how a preview that writes session rows shipped. + """ + + def setUp(self): + from webui.queue_loader import QueueSnapshot + + self.snapshot = QueueSnapshot( + project_id="p", + repo_label="r", + prs=(), + issues=(), + pr_pagination=None, + issue_pagination=None, + ) + + @contextlib.contextmanager + def _harness(self): + """Run the real ``default_allocator`` against a recorded allocator call.""" + calls: dict[str, Any] = {} + + def _allocate(db, **kwargs): + calls.update(kwargs) + return {"outcome": allocator_service.OUTCOME_PREVIEW} + + module = mock.MagicMock() + with mock.patch.dict(sys.modules, {"control_plane_db": module}), \ + mock.patch( + "webui.queue_loader.load_queue_snapshot", + return_value=self.snapshot, + ), \ + mock.patch( + "webui.traffic_loader.candidates_from_queue_snapshot", + return_value=[], + ), \ + mock.patch.object( + allocator_service, "allocate_next_work", side_effect=_allocate + ): + yield calls + + def test_preview_runs_side_effect_free_and_never_applies(self): + with self._harness() as calls: + result = request_service.default_allocator( + request=_request(), apply=False + ) + self.assertIsNotNone(result) + self.assertTrue(calls["side_effect_free"]) + self.assertFalse(calls["apply"]) + + def test_apply_is_not_side_effect_free(self): + with self._harness() as calls: + request_service.default_allocator(request=_request(), apply=True) + self.assertFalse(calls["side_effect_free"]) + self.assertTrue(calls["apply"]) + + def test_caller_session_id_is_passed_through_verbatim(self): + with self._harness() as calls: + request_service.default_allocator( + request=_request(), apply=True, session_id="webui-request-fixed" + ) + self.assertEqual(calls["session_id"], "webui-request-fixed") + + def test_absent_session_id_is_minted_with_the_expected_shape(self): + with self._harness() as calls: + request_service.default_allocator(request=_request(), apply=False) + self.assertTrue(str(calls["session_id"]).startswith("webui-request-")) + + def test_scope_and_fingerprint_reach_the_allocator(self): + with self._harness() as calls: + request_service.default_allocator( + request=_request(number=664), + apply=False, + expected_candidate_set_fingerprint="fp-pinned", + ) + self.assertEqual(calls["expected_candidate_set_fingerprint"], "fp-pinned") + self.assertEqual(calls["remote"], SCOPE["remote"]) + self.assertEqual(calls["org"], SCOPE["org"]) + self.assertEqual(calls["repo"], SCOPE["repo"]) + self.assertEqual(calls["allocation_mode"], "role_scoped") + + +class TestDefaultClaimsSource(unittest.TestCase): + """``default_claims_source`` had no test at all (#643).""" + + def test_claims_are_read_for_the_request_scope(self): + db = mock.MagicMock() + db.list_active_claims.return_value = {("issue", 643): {"lease_id": "l1"}} + module = mock.MagicMock() + module.ControlPlaneDB.return_value = db + with mock.patch.dict(sys.modules, {"control_plane_db": module}): + claims = request_service.default_claims_source(_request()) + self.assertEqual(claims, {("issue", 643): {"lease_id": "l1"}}) + db.list_active_claims.assert_called_once_with( + remote=SCOPE["remote"], org=SCOPE["org"], repo=SCOPE["repo"] + ) + + def test_an_unreadable_substrate_denies_rather_than_returning_empty(self): + module = mock.MagicMock() + module.ControlPlaneDB.side_effect = RuntimeError("no db") + with mock.patch.dict(sys.modules, {"control_plane_db": module}): + # _load_claims converts the failure into None, which fails the + # lease check closed; an empty mapping would read as "nothing + # claimed" and wrongly authorize. + claims = request_service._load_claims( + _request(), request_service.default_claims_source + ) + self.assertIsNone(claims) + + class TestAllocatorIntegrationFakes(unittest.TestCase): """The real default allocator refuses a partial inventory (#758).""" diff --git a/webui/request_service.py b/webui/request_service.py index 6941027..5926971 100644 --- a/webui/request_service.py +++ b/webui/request_service.py @@ -35,6 +35,7 @@ the resulting assignment by ``correlation_id``. from __future__ import annotations +import inspect import uuid from dataclasses import dataclass, field from typing import Any, Callable, Mapping, Sequence @@ -321,6 +322,17 @@ def _correlation_id() -> str: return f"req-{uuid.uuid4().hex}" +def _new_session_id() -> str: + """Mint the control-plane session id for one request flow (#643). + + Minted once per flow and reused by the dry-run and the apply, so a lease the + apply creates is owned by an id the caller still holds and can release. When + each call minted its own id, an apply wrote two session rows and bound the + lease to the second — leaving it with no owner able to reclaim it. + """ + return f"webui-request-{uuid.uuid4().hex[:12]}" + + def _selection_matches( selection: Mapping[str, Any] | None, request: WorkRequest ) -> bool: @@ -561,8 +573,15 @@ def preview_request( claims_source: ClaimsFn | None = None, correlation_id: str | None = None, audit: bool = True, + session_id: str | None = None, ) -> RequestPreview: - """Build the read-only intent preview for *request*. Never mutates.""" + """Build the read-only intent preview for *request*. Never mutates. + + "Never mutates" is now literal on the default path: the allocator runs + ``side_effect_free``, so no control-plane session row is written and no lease + sweep runs. *session_id* is threaded from an enclosing apply so both halves + of that flow share one control-plane identity (#643). + """ who = principal or console_authz.ANONYMOUS corr = correlation_id or _correlation_id() decision = console_authz.authorize(ACTION_ID, who, for_execution=False) @@ -573,7 +592,9 @@ def preview_request( if decision.allowed: # An unauthorized principal never reaches the allocator or the # control-plane DB: a denial must not double as a queue oracle. - allocation = _run_allocator(request, allocator, apply=False) + allocation = _run_allocator( + request, allocator, apply=False, session_id=session_id + ) claims = _load_claims(request, claims_source) checks.append(_capability_check(request)) checks.append(_lease_check(request, claims)) @@ -650,9 +671,20 @@ def apply_request( The only path to an assignment is the allocator agreeing, on a dry-run, that this work unit is what the requested role should take next. Every refusal returns before any mutation is attempted. + + One exception is unavoidable and is compensated rather than prevented: the + allocator can commit an assignment and only then reveal, on egress, that it + selected a different work unit than the one requested. That assignment is + released before refusing, and ``mutation_performed`` reports whether any + durable control-plane state survives this call — ``False`` once the stray + assignment is gone, ``True`` with an explicit ``orphaned_assignment`` and + reclaim action when the release did not succeed (#643). """ who = principal or console_authz.ANONYMOUS corr = correlation_id or _correlation_id() + # One identity for the whole flow, so a lease the apply creates is owned by + # an id this function still holds and can release. + session_id = _new_session_id() execution_decision = console_authz.authorize(ACTION_ID, who, for_execution=True) authorization = execution_decision.to_dict() @@ -664,6 +696,7 @@ def apply_request( *, status: int, extra: dict[str, Any] | None = None, + mutation_performed: bool = False, ) -> dict[str, Any]: _audit( request, @@ -689,7 +722,9 @@ def apply_request( "authorization": authorization, "assignment": None, "correlation_id": corr, - "mutation_performed": False, + # True only when durable control-plane state survives this refusal — + # an assignment the allocator created that could not be released. + "mutation_performed": bool(mutation_performed), "status_code": status, } payload.update(extra or {}) @@ -724,6 +759,7 @@ def apply_request( claims_source=claims_source, correlation_id=corr, audit=False, + session_id=session_id, ) if not preview.authorized: outcome = ( @@ -747,21 +783,37 @@ def apply_request( allocator, apply=True, expected_candidate_set_fingerprint=fingerprint, + session_id=session_id, ) if not allocation: + # "No result" is not proof of "no write": allocate_next_work only catches + # three exception types, so anything raised after assign_and_lease + # committed lands here with a lease already durable. Release whatever + # this flow's session owns before refusing. + sweep = _sweep_session_leases(request, session_id=session_id) + extra: dict[str, Any] = {} + detail = "the allocator returned no result; nothing was assigned." + if sweep.get("released"): + extra["compensation"] = sweep + detail = ( + "the allocator returned no result after creating an assignment; " + "the assignment was released and nothing remains assigned." + ) + elif sweep.get("error"): + extra["compensation"] = sweep return _refuse( OUTCOME_WAIT, REASON_EVIDENCE_UNAVAILABLE, - "the allocator returned no result; nothing was assigned.", + detail, status=503, + extra=extra or None, ) assignment = allocation.get("assignment") or None outcome = _clean(allocation.get("outcome")) + committed = bool(outcome == allocator_service.OUTCOME_ASSIGNED and assignment) assigned = bool( - outcome == allocator_service.OUTCOME_ASSIGNED - and assignment - and _selection_matches(allocation.get("selected"), request) + committed and _selection_matches(allocation.get("selected"), request) ) if not assigned: blocked = outcome in { @@ -769,15 +821,45 @@ def apply_request( allocator_service.OUTCOME_BLOCKED_TERMINAL, allocator_service.OUTCOME_BLOCKED_EXCLUDED_OWN_LEASE, } + extra = {"allocator_evidence": _allocator_evidence(allocation)} + detail = ( + f"the allocator returned {outcome or 'no outcome'} rather than " + "an assignment for this work unit; nothing was assigned." + ) + if committed: + # The allocator assigned a *different* unit than the one requested. + # That assignment is real and durable; releasing it is the whole + # difference between a refusal and a silent leak. + compensation = _release_stray_assignment( + request, allocation, session_id=session_id + ) + extra["compensation"] = compensation + if compensation.get("released"): + detail = ( + "the allocator assigned a different work unit than the one " + "requested; that assignment was released and nothing " + "remains assigned." + ) + else: + extra["orphaned_assignment"] = compensation + return _refuse( + OUTCOME_BLOCKED if blocked else OUTCOME_WAIT, + REASON_ALLOCATOR_OUTCOME, + ( + "the allocator assigned a different work unit than the " + "one requested and it could not be released; it must be " + "reclaimed explicitly." + ), + status=409, + extra=extra, + mutation_performed=True, + ) return _refuse( OUTCOME_BLOCKED if blocked else OUTCOME_WAIT, REASON_ALLOCATOR_OUTCOME, - ( - f"the allocator returned {outcome or 'no outcome'} rather than " - "an assignment for this work unit; nothing was assigned." - ), + detail, status=409, - extra={"allocator_evidence": _allocator_evidence(allocation)}, + extra=extra, ) _audit( @@ -853,19 +935,175 @@ def _run_allocator( *, apply: bool, expected_candidate_set_fingerprint: str | None = None, + session_id: str | None = None, ) -> dict[str, Any] | None: fn = allocator or default_allocator + kwargs: dict[str, Any] = { + "request": request, + "apply": apply, + "expected_candidate_set_fingerprint": expected_candidate_set_fingerprint, + } + if session_id: + # Injected allocators in tests predate this argument; only pass it to + # callables that accept it so a fake signature is never broken. + if _accepts_session_id(fn): + kwargs["session_id"] = session_id try: - result = fn( - request=request, - apply=apply, - expected_candidate_set_fingerprint=expected_candidate_set_fingerprint, - ) + result = fn(**kwargs) except Exception: # noqa: BLE001 — an allocator failure denies, never proceeds return None return result if isinstance(result, dict) else None +def _accepts_session_id(fn: AllocatorFn) -> bool: + try: + params = inspect.signature(fn).parameters + except (TypeError, ValueError): # builtins / C callables + return False + if "session_id" in params: + return True + return any(p.kind is inspect.Parameter.VAR_KEYWORD for p in params.values()) + + +def _release_stray_assignment( + request: WorkRequest, + allocation: Mapping[str, Any], + *, + session_id: str | None, +) -> dict[str, Any]: + """Release an assignment the allocator committed for the wrong work unit. + + The apply path can commit an assignment and then discover, on egress, that + the allocator selected a *different* unit than the one requested — the CAS + fingerprint hashes only ``{kind, number}`` plus exclusions, so a lease taken + on the requested unit inside the window leaves the fingerprint identical and + the pin passes while the selection moves on. Without compensation that lease + is orphaned: owned by a session nothing heartbeats, and invisible because the + response says nothing was mutated. + + Returns a record describing what was attempted so the caller can report it, + including a reclaim action when the release itself did not succeed. + """ + assignment = allocation.get("assignment") or {} + lease_id = _clean(assignment.get("lease_id")) or None + assignment_id = _clean(assignment.get("assignment_id")) or None + owner = _clean(allocation.get("session_id")) or session_id or None + selected = allocation.get("selected") or {} + record: dict[str, Any] = { + "attempted": False, + "released": False, + "lease_id": lease_id, + "assignment_id": assignment_id, + "owner_session_id": owner, + "selected": { + "kind": _clean(selected.get("kind")).lower() or None, + "number": selected.get("number"), + }, + } + if not lease_id: + # Nothing durable to release (or the allocator reported no lease id); + # still surface the assignment id so a leak is never silent. + record["detail"] = ( + "the allocator reported an assignment with no lease id; nothing " + "could be released automatically." + ) + if assignment_id: + record["reclaim_action"] = _reclaim_action(record) + return record + if not owner: + record["detail"] = ( + "the owning session id is unknown; the assignment cannot be " + "released automatically." + ) + record["reclaim_action"] = _reclaim_action(record) + return record + + record["attempted"] = True + try: + import control_plane_db + + control_plane_db.ControlPlaneDB().release_lease(lease_id, session_id=owner) + except Exception as exc: # noqa: BLE001 — a failed release must be reported + record["error"] = str(exc) + record["detail"] = ( + "the allocator created an assignment for a different work unit and " + "releasing it failed; it must be reclaimed explicitly." + ) + record["reclaim_action"] = _reclaim_action(record) + return record + + record["released"] = True + record["detail"] = ( + "the allocator created an assignment for a different work unit; it was " + "released, so no assignment persists from this request." + ) + return record + + +def _reclaim_action(record: Mapping[str, Any]) -> dict[str, Any]: + """The explicit operator action for an assignment that outlived its request.""" + lease_id = record.get("lease_id") + return { + "tool": "gitea_release_workflow_lease", + "lease_id": lease_id, + "session_id": record.get("owner_session_id"), + "assignment_id": record.get("assignment_id"), + "instructions": ( + "A control-plane assignment was created for a work unit other than " + "the requested one and could not be released automatically. Call " + f"gitea_release_workflow_lease(lease_id={lease_id!r}, " + f"session_id={record.get('owner_session_id')!r}) to reclaim it, or " + "wait for the lease TTL to expire." + ), + } + + +def _sweep_session_leases( + request: WorkRequest, *, session_id: str | None +) -> dict[str, Any]: + """Release any lease this flow's session owns after an indeterminate apply. + + ``_run_allocator`` returns ``None`` for *any* exception, and + ``allocate_next_work`` only catches ``InvalidWorkKindError``, + ``LeaseRequiredError`` and ``ControlPlaneError`` — so an unexpected error + raised *after* ``assign_and_lease`` committed surfaces as "no result" with a + lease already written. Because the session id is now stable across the flow, + that lease is findable: anything this session owns after a failed apply is by + definition unclaimed by anyone, so it is released. + """ + record: dict[str, Any] = {"attempted": False, "released": [], "session_id": session_id} + if not session_id: + return record + record["attempted"] = True + try: + import control_plane_db + + db = control_plane_db.ControlPlaneDB() + leases = db.list_leases( + remote=request.remote, + org=request.org, + repo=request.repo, + statuses=("active",), + ) + for row in leases: + if _clean(row.get("session_id")) != session_id: + continue + lease_id = _clean(row.get("lease_id")) or None + if not lease_id: + continue + db.release_lease(lease_id, session_id=session_id) + record["released"].append( + { + "lease_id": lease_id, + "work_kind": row.get("work_kind"), + "work_number": row.get("work_number"), + } + ) + except Exception as exc: # noqa: BLE001 — best-effort; never mask the refusal + record["error"] = str(exc) + return record + + def _load_claims( request: WorkRequest, claims_source: ClaimsFn | None ) -> Mapping[tuple[str, int], dict[str, Any]] | None: @@ -894,12 +1132,19 @@ def default_allocator( request: WorkRequest, apply: bool, expected_candidate_set_fingerprint: str | None = None, + session_id: str | None = None, ) -> dict[str, Any] | None: """Run the real allocator over the live queue for *request*'s scope. An incomplete candidate inventory returns ``None`` rather than a ranking over a partial set (#758): selecting from a short list can pick the wrong work unit, so the request denies instead. + + *session_id* is supplied by the caller so the dry-run and the apply share one + control-plane identity; the lease an apply creates is then owned by an id the + request flow still holds. A dry run additionally goes through the allocator's + ``side_effect_free`` path, so a preview writes no session row and sweeps no + leases (#643). """ import control_plane_db @@ -917,7 +1162,7 @@ def default_allocator( db = control_plane_db.ControlPlaneDB() result = allocator_service.allocate_next_work( db, - session_id=f"webui-request-{uuid.uuid4().hex[:12]}", + session_id=session_id or _new_session_id(), role=request.desired_role, remote=request.remote, org=request.org, @@ -926,6 +1171,7 @@ def default_allocator( apply=bool(apply), allocation_mode="role_scoped", expected_candidate_set_fingerprint=expected_candidate_set_fingerprint, + side_effect_free=not apply, ) if isinstance(result, dict): result.setdefault("candidate_count", len(candidates)) From 1c88b87ec5030256fe5573bdc711792bf5bd5d37 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 25 Jul 2026 08:19:06 -0400 Subject: [PATCH 03/12] feat(webui): implement Phase 2 recovery controls & playbooks (#644) --- docs/sanctioned-recovery-playbooks.md | 62 +++ docs/webui-authz-audit.md | 3 + task_capability_map.py | 13 + tests/test_webui_console_recovery.py | 182 ++++++++ webui/app.py | 83 ++++ webui/console_authz.py | 34 ++ webui/console_recovery.py | 573 ++++++++++++++++++++++++++ webui/gated_actions.py | 7 + webui/system_health_views.py | 61 ++- 9 files changed, 998 insertions(+), 20 deletions(-) create mode 100644 docs/sanctioned-recovery-playbooks.md create mode 100644 tests/test_webui_console_recovery.py create mode 100644 webui/console_recovery.py diff --git a/docs/sanctioned-recovery-playbooks.md b/docs/sanctioned-recovery-playbooks.md new file mode 100644 index 0000000..819de2c --- /dev/null +++ b/docs/sanctioned-recovery-playbooks.md @@ -0,0 +1,62 @@ +# Sanctioned Recovery Playbooks & Controls (Phase 2 #644) + +## Overview + +Stale runtimes, worktree binding mismatches, and un-reconciled merged branches previously required expert manual shell recovery. Manual process kills (`pkill -f mcp_server.py`) are strictly forbidden and classified as runtime contamination ([#630](file:///Users/jasonwalker/Development/Gitea-Tools/docs/sanctioned-restart-controls.md)). + +Phase 2 introduces **sanctioned recovery playbooks and controls** into the Web Console: +- **Diagnose**: Surface stale runtimes, worktree binding errors, contamination markers, and worktree anomalies via health & inventory APIs. +- **Preview**: Render mutation ledgers and exact confirmation phrases for recovery playbooks. +- **Confirm & Apply**: Execute sanctioned recovery actions through gated, audited paths. +- **Verify**: Revalidate control-plane state post-recovery before claiming clean status. + +--- + +## Recovery Playbook Taxonomy + +| Playbook ID | Action ID | Minimum Role | Target / Scope | Description | +|---|---|---|---|---| +| `clear_stale_binding` | `system.clear_stale_binding` | Operator | Active worktree binding | Clear provably missing or superseded `GITEA_ACTIVE_WORKTREE` binding ([#702](file:///Users/jasonwalker/Development/Gitea-Tools/stale_binding_recovery.py)). | +| `rebind_session_worktree` | `system.rebind_session_worktree` | Operator | Session worktree | Rebind or synchronize session worktree to verified lease worktree ([#864](file:///Users/jasonwalker/Development/Gitea-Tools/dirty_same_claimant_session_rebind.py)). | +| `reconcile_cleanups` | `system.reconcile_cleanups` | Controller | Worktree hygiene | Execute reconciler cleanup preview and apply for merged/superseded PR branches. | +| `sanctioned_restart` | `system.restart_namespace` | Admin | MCP Namespace | Restart MCP daemon gracefully via host supervisor ([#642](file:///Users/jasonwalker/Development/Gitea-Tools/docs/sanctioned-restart-controls.md)). | + +--- + +## Wizard Workflow (Diagnose → Preview → Confirm → Verify) + +### 1. Diagnose (`GET /api/v1/system/recovery/diagnose`) +Runs control-plane diagnostics: +- **Stale Runtime**: Mismatch between running daemon HEAD, local checkout HEAD, and remote-tracking HEAD. +- **Worktree Binding**: Missing path (`provably_stale_missing_path`), unverified inherited binding (`unverified_inherited`), or superseded binding (`superseded_by_session_lease`). +- **Contamination**: Checks for live contamination markers from unmanaged process kills. +- **Worktree Anomalies**: Scans `branches/` directory for un-reconciled cleanups or missing preserved worktrees. + +Returns `RecoveryDiagnosis` with eligible playbooks. + +### 2. Preview (`POST /api/v1/system/recovery/preview`) +Takes `playbook_id` and optional `target`/`params`. +Returns: +- **Mutation Ledger**: Step-by-step sequence of actions. +- **Confirmation Phrase**: Exact phrase required to authorize execution (e.g., `confirm clear_stale_binding`). +- **Authorization Decision**: RBAC check against the operator's principal. + +### 3. Apply (`POST /api/v1/system/recovery/apply`) +Requires `playbook_id` and matching `confirmation` phrase. +- Validates RBAC permissions (`console_authz`). +- Verifies confirmation phrase (`confirmation_matches`). +- Enforces contamination rules ([#630](file:///Users/jasonwalker/Development/Gitea-Tools/docs/sanctioned-restart-controls.md)): A contaminated runtime must be cleared through reconciler cleanup before other playbooks run. +- Enforces master parity ([#610](file:///Users/jasonwalker/Development/Gitea-Tools/master_parity_gate.py)). +- Applies sanctioned recovery logic. +- Logs audit record in `console_audit`. + +### 4. Verify (`POST /api/v1/system/recovery/verify`) +Re-evaluates control-plane diagnostics post-recovery. Asserts `clean: true` before transitioning out of recovery mode. + +--- + +## Safety & Governance Principles + +1. **No Manual `pkill`**: Direct process killing remains forbidden and is recorded as contamination. +2. **Auditability**: Every recovery preview and execution is logged in the console audit trail. +3. **Master Parity & Dual Control**: High-privilege recovery actions require controller/admin roles and explicit confirmation phrases. diff --git a/docs/webui-authz-audit.md b/docs/webui-authz-audit.md index 2dcffac..9b9575e 100644 --- a/docs/webui-authz-audit.md +++ b/docs/webui-authz-audit.md @@ -94,6 +94,9 @@ already define, and a regression test asserts each mapping matches. | `record_analytics_usage` | operator | gated_write | `runtime.record_analytics_usage` | Yes | No | No | 2 | | `system.reload_namespace` | controller | privileged | `runtime.reload_namespace` | Yes | No | No | 2 | | `system.restart_namespace` | admin | destructive | `runtime.restart_namespace` | Yes | **Yes** | **Yes** | 2 | +| `system.clear_stale_binding` | operator | gated_write | `gitea.read` | Yes | No | No | 2 | +| `system.rebind_session_worktree` | operator | gated_write | `gitea.read` | Yes | No | No | 2 | +| `system.reconcile_cleanups` | controller | privileged | `gitea.pr.close` | Yes | No | No | 2 | **Dual control** means the acting principal may not be the sole authority: a second distinct principal must confirm. **Break-glass** means the action is diff --git a/task_capability_map.py b/task_capability_map.py index 878cf0a..3a8efd1 100644 --- a/task_capability_map.py +++ b/task_capability_map.py @@ -142,6 +142,19 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = { "permission": "gitea.read", "role": "author", }, + # #644: Phase 2 Web Console recovery tasks. + "clear_stale_binding": { + "permission": "gitea.read", + "role": "author", + }, + "rebind_session_worktree": { + "permission": "gitea.read", + "role": "author", + }, + "reconcile_cleanups": { + "permission": "gitea.pr.close", + "role": "reconciler", + }, # PR synchronization lifecycle: assess is read-only (any role with gitea.read); # update-by-merge is author-only and mutates the PR head via Gitea API. "assess_pr_sync_status": { diff --git a/tests/test_webui_console_recovery.py b/tests/test_webui_console_recovery.py new file mode 100644 index 0000000..343a61b --- /dev/null +++ b/tests/test_webui_console_recovery.py @@ -0,0 +1,182 @@ +"""Unit and integration tests for Phase 2 Web Console recovery controls (#644).""" + +from __future__ import annotations + +import json +import os +import unittest +from unittest.mock import patch + +from starlette.testclient import TestClient + +from webui import console_audit, console_authz, console_recovery +from webui.app import create_app + + +class TestConsoleRecovery(unittest.TestCase): + + def test_diagnose_recovery_healthy(self) -> None: + diag = console_recovery.diagnose_recovery() + self.assertIn(diag.status, {console_recovery.STATUS_HEALTHY, console_recovery.STATUS_ACTION_REQUIRED}) + self.assertIsInstance(diag.playbooks, tuple) + self.assertGreaterEqual(len(diag.playbooks), 4) + + playbook_ids = {pb.playbook_id for pb in diag.playbooks} + self.assertIn(console_recovery.PLAYBOOK_CLEAR_STALE_BINDING, playbook_ids) + self.assertIn(console_recovery.PLAYBOOK_REBIND_SESSION, playbook_ids) + self.assertIn(console_recovery.PLAYBOOK_RECONCILE_CLEANUPS, playbook_ids) + self.assertIn(console_recovery.PLAYBOOK_SANCTIONED_RESTART, playbook_ids) + + def test_confirmation_phrase_generation_and_matching(self) -> None: + phrase = console_recovery.confirmation_phrase("clear_stale_binding") + self.assertEqual(phrase, "confirm clear_stale_binding") + self.assertTrue(console_recovery.confirmation_matches("clear_stale_binding", "confirm clear_stale_binding")) + self.assertFalse(console_recovery.confirmation_matches("clear_stale_binding", "wrong phrase")) + + phrase_target = console_recovery.confirmation_phrase("sanctioned_restart", "gitea-author") + self.assertEqual(phrase_target, "confirm sanctioned_restart gitea-author") + self.assertTrue(console_recovery.confirmation_matches("sanctioned_restart", "confirm sanctioned_restart gitea-author", "gitea-author")) + + def test_build_recovery_preview(self) -> None: + principal = console_authz.Principal("dev@example.com", console_authz.OPERATOR, console_authz.IDENTITY_LOCAL_DEV, True) + preview = console_recovery.build_recovery_preview( + playbook_id=console_recovery.PLAYBOOK_CLEAR_STALE_BINDING, + target="test-worktree", + principal=principal, + ) + self.assertEqual(preview["playbook_id"], console_recovery.PLAYBOOK_CLEAR_STALE_BINDING) + self.assertEqual(preview["action_id"], console_recovery.ACTION_CLEAR_STALE_BINDING) + self.assertEqual(preview["confirmation_phrase"], "confirm clear_stale_binding test-worktree") + self.assertTrue(len(preview["mutation_ledger"]) >= 3) + self.assertTrue(preview["authorization"]["allowed"]) + + def test_build_recovery_preview_unknown_playbook(self) -> None: + preview = console_recovery.build_recovery_preview("unknown_playbook") + self.assertFalse(preview.get("allowed")) + self.assertEqual(preview.get("error"), "unknown_playbook") + + def test_execute_recovery_playbook_confirmation_mismatch(self) -> None: + principal = console_authz.Principal("dev@example.com", console_authz.OPERATOR, console_authz.IDENTITY_LOCAL_DEV, True) + result = console_recovery.execute_recovery_playbook( + playbook_id=console_recovery.PLAYBOOK_CLEAR_STALE_BINDING, + confirmation="invalid confirmation", + principal=principal, + ) + self.assertFalse(result["success"]) + self.assertFalse(result["allowed"]) + self.assertEqual(result["error"], "confirmation_mismatch") + + def test_execute_recovery_playbook_unauthorized(self) -> None: + # Anonymous principal has viewer role -> should be denied + result = console_recovery.execute_recovery_playbook( + playbook_id=console_recovery.PLAYBOOK_CLEAR_STALE_BINDING, + confirmation="confirm clear_stale_binding", + principal=console_authz.ANONYMOUS, + ) + self.assertFalse(result["success"]) + self.assertFalse(result["allowed"]) + self.assertEqual(result["error"], console_authz.DENY_UNAUTHENTICATED) + + def test_execute_recovery_playbook_clear_stale_binding_success(self) -> None: + principal = console_authz.Principal("dev@example.com", console_authz.OPERATOR, console_authz.IDENTITY_LOCAL_DEV, True) + phrase = console_recovery.confirmation_phrase(console_recovery.PLAYBOOK_CLEAR_STALE_BINDING) + + result = console_recovery.execute_recovery_playbook( + playbook_id=console_recovery.PLAYBOOK_CLEAR_STALE_BINDING, + confirmation=phrase, + principal=principal, + ) + self.assertTrue(result["allowed"]) + self.assertIn("applied_result", result) + self.assertIn("post_recovery_verification", result) + + self.assertIn("audit", result) + self.assertEqual(result["audit"]["event"]["action"], console_recovery.ACTION_CLEAR_STALE_BINDING) + + def test_execute_recovery_playbook_rebind_session_success(self) -> None: + principal = console_authz.Principal("dev@example.com", console_authz.OPERATOR, console_authz.IDENTITY_LOCAL_DEV, True) + phrase = console_recovery.confirmation_phrase(console_recovery.PLAYBOOK_REBIND_SESSION, "branches/feat-issue-644") + + result = console_recovery.execute_recovery_playbook( + playbook_id=console_recovery.PLAYBOOK_REBIND_SESSION, + confirmation=phrase, + target="branches/feat-issue-644", + principal=principal, + ) + self.assertTrue(result["allowed"]) + self.assertTrue(result["success"]) + self.assertEqual(result["applied_result"]["rebound_worktree"], "branches/feat-issue-644") + + def test_verify_post_recovery(self) -> None: + verification = console_recovery.verify_post_recovery() + self.assertIn("clean", verification) + self.assertIn("status", verification) + self.assertIn("reasons", verification) + + +class TestConsoleRecoveryApi(unittest.TestCase): + def setUp(self) -> None: + self.app = create_app() + self.client = TestClient(self.app) + + def test_api_recovery_diagnose(self) -> None: + res = self.client.get("/api/v1/system/recovery/diagnose") + self.assertEqual(res.status_code, 200) + data = res.json() + self.assertIn("status", data) + self.assertIn("clean", data) + self.assertIn("playbooks", data) + self.assertTrue(len(data["playbooks"]) >= 4) + + def test_api_recovery_preview(self) -> None: + res = self.client.post( + "/api/v1/system/recovery/preview", + json={"playbook_id": "clear_stale_binding", "target": "active"}, + ) + self.assertEqual(res.status_code, 200) + data = res.json() + self.assertEqual(data["playbook_id"], "clear_stale_binding") + self.assertEqual(data["confirmation_phrase"], "confirm clear_stale_binding active") + self.assertIn("mutation_ledger", data) + + def test_api_recovery_apply_denied_without_auth(self) -> None: + res = self.client.post( + "/api/v1/system/recovery/apply", + json={"playbook_id": "clear_stale_binding", "confirmation": "confirm clear_stale_binding"}, + ) + self.assertEqual(res.status_code, 400) + data = res.json() + self.assertFalse(data["success"]) + self.assertFalse(data["allowed"]) + + def test_api_recovery_apply_with_dev_auth(self) -> None: + env = { + "WEBUI_AUTH_MODE": "local_dev", + "WEBUI_DEV_SUBJECT": "dev@example.com", + "WEBUI_DEV_ROLE": "operator", + } + with patch.dict(os.environ, env): + res = self.client.post( + "/api/v1/system/recovery/apply", + json={ + "playbook_id": "rebind_session_worktree", + "target": "branches/feat-issue-644", + "confirmation": "confirm rebind_session_worktree branches/feat-issue-644", + }, + ) + self.assertEqual(res.status_code, 200) + data = res.json() + self.assertTrue(data["success"]) + self.assertTrue(data["allowed"]) + self.assertEqual(data["playbook_id"], "rebind_session_worktree") + + def test_api_recovery_verify(self) -> None: + res = self.client.get("/api/v1/system/recovery/verify") + self.assertEqual(res.status_code, 200) + data = res.json() + self.assertIn("clean", data) + self.assertIn("status", data) + + +if __name__ == "__main__": + unittest.main() diff --git a/webui/app.py b/webui/app.py index 560ec75..6fece41 100644 --- a/webui/app.py +++ b/webui/app.py @@ -200,6 +200,84 @@ async def system_health(request: Request) -> HTMLResponse: ) +async def api_recovery_diagnose(_request: Request) -> JSONResponse: + from webui.console_recovery import diagnose_recovery + diag = diagnose_recovery() + return JSONResponse({ + "status": diag.status, + "clean": diag.clean, + "stale_runtime": diag.stale_runtime, + "master_parity": diag.master_parity, + "stale_binding": diag.stale_binding, + "contamination": diag.contamination, + "worktree_anomalies": list(diag.worktree_anomalies), + "reasons": list(diag.reasons), + "playbooks": [ + { + "playbook_id": pb.playbook_id, + "label": pb.label, + "action_id": pb.action_id, + "description": pb.description, + "eligible": pb.eligible, + "requires_confirmation": pb.requires_confirmation, + "reason": pb.reason, + "params_schema": pb.params_schema, + } + for pb in diag.playbooks + ], + }) + + +async def api_recovery_preview(request: Request) -> JSONResponse: + from webui.console_recovery import build_recovery_preview + body = {} + try: + body = await request.json() + except Exception: + pass + playbook_id = body.get("playbook_id") or request.query_params.get("playbook_id") or "" + target = body.get("target") or request.query_params.get("target") + principal = resolve_principal(request.headers) + preview = build_recovery_preview( + playbook_id=playbook_id, + target=target, + params=body, + principal=principal, + ) + status = 200 if preview.get("playbook_id") else 400 + return JSONResponse(preview, status_code=status) + + +async def api_recovery_apply(request: Request) -> JSONResponse: + from webui.console_recovery import execute_recovery_playbook + body = {} + try: + body = await request.json() + except Exception: + pass + playbook_id = body.get("playbook_id", "") + confirmation = body.get("confirmation", "") + target = body.get("target") + principal = resolve_principal(request.headers) + request_id = getattr(request.state, "request_id", None) + result = execute_recovery_playbook( + playbook_id=playbook_id, + confirmation=confirmation, + target=target, + params=body, + principal=principal, + request_id=request_id, + ) + status_code = 200 if result.get("success") else 400 + return JSONResponse(result, status_code=status_code) + + +async def api_recovery_verify(_request: Request) -> JSONResponse: + from webui.console_recovery import verify_post_recovery + verification = verify_post_recovery() + return JSONResponse(verification, status_code=200) + + async def queue(_request: Request) -> HTMLResponse: snapshot = load_queue_snapshot() return HTMLResponse(render_page(title="Queue", body_html=render_queue_page(snapshot))) @@ -818,6 +896,11 @@ def create_app(*, bind_host: str | None = None) -> Starlette: api_console_security_model, methods=["GET"], ), + # #644 Phase 2 Recovery API routes + Route("/api/v1/system/recovery/diagnose", api_recovery_diagnose, methods=["GET"]), + Route("/api/v1/system/recovery/preview", api_recovery_preview, methods=["POST", "GET"]), + Route("/api/v1/system/recovery/apply", api_recovery_apply, methods=["POST"]), + Route("/api/v1/system/recovery/verify", api_recovery_verify, methods=["POST", "GET"]), *[ Route(path, phase_stub, methods=["GET"]) for path in STUB_PAGES diff --git a/webui/console_authz.py b/webui/console_authz.py index 0c51037..e8de56b 100644 --- a/webui/console_authz.py +++ b/webui/console_authz.py @@ -277,6 +277,40 @@ _ACTION_SPECS: tuple[ConsoleAction, ...] = ( phase=2, summary="Restart one MCP namespace via the host supervisor.", ), + # #644: Phase 2 recovery controls & playbooks. + ConsoleAction( + action_id="system.clear_stale_binding", + task_key="clear_stale_binding", + action_class=CLASS_WRITE, + minimum_role=OPERATOR, + requires_confirmation=True, + dual_control=False, + break_glass=False, + phase=2, + summary="Clear provably stale or superseded GITEA_ACTIVE_WORKTREE binding.", + ), + ConsoleAction( + action_id="system.rebind_session_worktree", + task_key="rebind_session_worktree", + action_class=CLASS_WRITE, + minimum_role=OPERATOR, + requires_confirmation=True, + dual_control=False, + break_glass=False, + phase=2, + summary="Rebind session worktree context to verified lease worktree.", + ), + ConsoleAction( + action_id="system.reconcile_cleanups", + task_key="reconcile_cleanups", + action_class=CLASS_PRIVILEGED, + minimum_role=CONTROLLER, + requires_confirmation=True, + dual_control=False, + break_glass=False, + phase=2, + summary="Run reconciler cleanup for merged or superseded PR branches.", + ), ) ACTIONS: dict[str, ConsoleAction] = {a.action_id: a for a in _ACTION_SPECS} diff --git a/webui/console_recovery.py b/webui/console_recovery.py new file mode 100644 index 0000000..61783b0 --- /dev/null +++ b/webui/console_recovery.py @@ -0,0 +1,573 @@ +"""Web Console Phase 2 Recovery Controls & Playbooks (#644). + +Provides canonical recovery controls for the web console: +1. Diagnosis: Surfaces stale runtimes, worktree binding errors, contamination markers, + and un-reconciled cleanups. +2. Gated Actions & Playbooks: Guided recovery (rebind session worktree, clear stale + binding, trigger reconciler cleanups, sanctioned restart). +3. RBAC, Contamination (#630), and Master Parity (#610) integration. +4. Audit trail via ``console_audit`` and mandatory post-recovery revalidation. +""" + +from __future__ import annotations + +import os +from dataclasses import asdict, dataclass, field +from pathlib import Path +from typing import Any + +import master_parity_gate +import merged_cleanup_reconcile +import runtime_recovery_guard +import stale_binding_recovery +from webui import console_audit, console_authz, sanctioned_restart, system_health, worktree_scanner + +# --- Recovery Statuses ------------------------------------------------------ +STATUS_HEALTHY = "healthy" +STATUS_ACTION_REQUIRED = "action_required" +STATUS_BLOCKED_CONTAMINATION = "blocked_contamination" +STATUS_RECONNECT_REQUIRED = "reconnect_required" + +# --- Playbook Identifiers --------------------------------------------------- +PLAYBOOK_CLEAR_STALE_BINDING = "clear_stale_binding" +PLAYBOOK_REBIND_SESSION = "rebind_session_worktree" +PLAYBOOK_RECONCILE_CLEANUPS = "reconcile_cleanups" +PLAYBOOK_SANCTIONED_RESTART = "sanctioned_restart" + +KNOWN_PLAYBOOKS: tuple[str, ...] = ( + PLAYBOOK_CLEAR_STALE_BINDING, + PLAYBOOK_REBIND_SESSION, + PLAYBOOK_RECONCILE_CLEANUPS, + PLAYBOOK_SANCTIONED_RESTART, +) + +# --- Console Action Mapping ------------------------------------------------- +ACTION_CLEAR_STALE_BINDING = "system.clear_stale_binding" +ACTION_REBIND_SESSION = "system.rebind_session_worktree" +ACTION_RECONCILE_CLEANUPS = "system.reconcile_cleanups" + +PLAYBOOK_ACTIONS: dict[str, str] = { + PLAYBOOK_CLEAR_STALE_BINDING: ACTION_CLEAR_STALE_BINDING, + PLAYBOOK_REBIND_SESSION: ACTION_REBIND_SESSION, + PLAYBOOK_RECONCILE_CLEANUPS: ACTION_RECONCILE_CLEANUPS, + PLAYBOOK_SANCTIONED_RESTART: sanctioned_restart.ACTION_RESTART_NAMESPACE, +} + + +@dataclass(frozen=True) +class RecoveryLedgerEntry: + """One planned recovery step displayed before execution.""" + + sequence: int + step: str + summary: str + executes_process_kill: bool = False + + +@dataclass(frozen=True) +class PlaybookDescriptor: + """Structured recovery playbook option returned during diagnosis.""" + + playbook_id: str + label: str + action_id: str + description: str + eligible: bool + requires_confirmation: bool + reason: str + params_schema: dict[str, Any] = field(default_factory=dict) + + +@dataclass(frozen=True) +class RecoveryDiagnosis: + """Complete diagnostic snapshot of control-plane recovery needs.""" + + status: str + clean: bool + stale_runtime: dict[str, Any] + master_parity: dict[str, Any] + stale_binding: dict[str, Any] + contamination: dict[str, Any] + worktree_anomalies: tuple[str, ...] + playbooks: tuple[PlaybookDescriptor, ...] + reasons: tuple[str, ...] + + +def _repo_root(custom_path: Path | str | None = None) -> Path: + if custom_path: + return Path(custom_path).resolve() + override = (os.environ.get("WEBUI_REPO_ROOT") or "").strip() + if override: + return Path(override).resolve() + return Path(__file__).resolve().parent.parent + + +def confirmation_phrase(playbook_id: str, target: str | None = None) -> str: + """Construct exact confirmation phrase required for a recovery playbook.""" + clean_target = (target or "").strip() + if clean_target: + return f"confirm {playbook_id} {clean_target}" + return f"confirm {playbook_id}" + + +def confirmation_matches( + playbook_id: str, confirmation: str | None, target: str | None = None +) -> bool: + expected = confirmation_phrase(playbook_id, target) + return str(confirmation or "").strip() == expected + + +def _build_ledger( + playbook_id: str, target: str | None = None +) -> tuple[RecoveryLedgerEntry, ...]: + if playbook_id == PLAYBOOK_CLEAR_STALE_BINDING: + return ( + RecoveryLedgerEntry(1, "quiesce", "Stop admitting new gated mutations."), + RecoveryLedgerEntry( + 2, + "clear_env", + f"Remove stale env binding GITEA_ACTIVE_WORKTREE ({target or 'active'}).", + ), + RecoveryLedgerEntry( + 3, "audit", "Record clear_stale_binding event in console audit log." + ), + RecoveryLedgerEntry( + 4, "revalidate", "Re-run diagnosis to verify clean binding state." + ), + ) + if playbook_id == PLAYBOOK_REBIND_SESSION: + return ( + RecoveryLedgerEntry(1, "quiesce", "Stop admitting new gated mutations."), + RecoveryLedgerEntry( + 2, + "rebind_worktree", + f"Rebind session worktree context safely to {target or 'target worktree'}.", + ), + RecoveryLedgerEntry( + 3, "audit", "Record rebind_session_worktree event in console audit log." + ), + RecoveryLedgerEntry( + 4, "revalidate", "Re-run diagnosis to verify worktree binding state." + ), + ) + if playbook_id == PLAYBOOK_RECONCILE_CLEANUPS: + return ( + RecoveryLedgerEntry(1, "quiesce", "Stop admitting new gated mutations."), + RecoveryLedgerEntry( + 2, + "reconcile_cleanups", + "Execute sanctioned reconciler cleanup for merged or superseded PRs.", + ), + RecoveryLedgerEntry( + 3, "audit", "Record reconcile_cleanups event in console audit log." + ), + RecoveryLedgerEntry( + 4, "revalidate", "Re-run worktree scanner to verify clean tree." + ), + ) + if playbook_id == PLAYBOOK_SANCTIONED_RESTART: + restart_ledger = sanctioned_restart._mutation_ledger( + target or "gitea-author", sanctioned_restart.MODE_RESTART + ) + return tuple( + RecoveryLedgerEntry( + sequence=e.sequence, + step=e.step, + summary=e.summary, + executes_process_kill=e.executes_process_kill, + ) + for e in restart_ledger + ) + return ( + RecoveryLedgerEntry(1, "unspecified", f"Execute recovery playbook {playbook_id}."), + ) + + +def diagnose_recovery( + repo_path: Path | str | None = None, + env: dict[str, str] | None = None, + active_worktree_val: str | None = None, + session_lease_wt: str | None = None, + role_kind: str | None = None, +) -> RecoveryDiagnosis: + """Run full control-plane diagnostics to determine recovery needs and options.""" + root = _repo_root(repo_path) + source_env = dict(env) if env is not None else dict(os.environ) + reasons: list[str] = [] + + # 1. Stale runtime assessment + stale_runtime_obj = system_health.assess_stale_runtime(root) + stale_runtime_dict = { + "daemon_head": stale_runtime_obj.daemon_head, + "checkout_head": stale_runtime_obj.checkout_head, + "remote_head": stale_runtime_obj.remote_head, + "stale": stale_runtime_obj.stale, + "determinable": stale_runtime_obj.determinable, + "mutation_safe": stale_runtime_obj.mutation_safe, + "reasons": list(stale_runtime_obj.reasons), + } + if stale_runtime_obj.stale: + reasons.append("Runtime HEAD disagrees with checkout/remote HEAD.") + + # 2. Master parity assessment + checkout_head = stale_runtime_obj.checkout_head + startup_dict = master_parity_gate.capture_startup_parity(str(root), head=checkout_head) + parity_dict = master_parity_gate.assess_master_parity(startup_dict, checkout_head) + if not parity_dict.get("in_parity", True): + reasons.append("Repository is not in master parity.") + + # 3. Worktree binding classification + boot_bindings = stale_binding_recovery.snapshot_boot_bindings(source_env) + active_val = ( + active_worktree_val + if active_worktree_val is not None + else source_env.get(stale_binding_recovery.ACTIVE_WORKTREE_ENV) + ) + boot_inherited = bool(boot_bindings.get("active_worktree") and active_val == boot_bindings.get("active_worktree")) + + path_exists = None + if active_val: + path_exists = os.path.exists(os.path.realpath(active_val)) + + binding_class = stale_binding_recovery.classify_active_worktree_binding( + active_value=active_val, + session_lease_worktree=session_lease_wt, + boot_inherited=boot_inherited, + path_exists=path_exists, + role_kind=role_kind, + ) + + if binding_class.get("clear_eligible"): + reasons.append( + f"Active worktree binding is stale ({binding_class.get('classification')})." + ) + elif binding_class.get("classification") == stale_binding_recovery.CLASSIFICATION_UNVERIFIED_INHERITED: + reasons.append("Inherited worktree binding is unverified.") + + # 4. Contamination assessment (#630) + contamination_dict = runtime_recovery_guard.assess_contamination_gate( + marker=None, task=None, actual_role=role_kind + ) + if contamination_dict.get("block"): + reasons.append("Runtime is contaminated by manual process kill (#630).") + + # 5. Worktree scanner hygiene & anomalies + hygiene = worktree_scanner.load_hygiene_snapshot(project_root=str(root)) + worktree_anomalies = hygiene.anomalies + + # Determine status & eligible playbooks + playbooks: list[PlaybookDescriptor] = [] + + # Playbook 1: Clear Stale Binding + clear_eligible = bool(binding_class.get("clear_eligible")) + playbooks.append( + PlaybookDescriptor( + playbook_id=PLAYBOOK_CLEAR_STALE_BINDING, + label="Clear Stale Worktree Binding", + action_id=ACTION_CLEAR_STALE_BINDING, + description="Clear provably stale or superseded GITEA_ACTIVE_WORKTREE environment binding.", + eligible=clear_eligible, + requires_confirmation=True, + reason=( + f"Binding classified as {binding_class.get('classification')}; clear is authorized." + if clear_eligible + else "Active worktree binding is clean, corroborated, or absent." + ), + ) + ) + + # Playbook 2: Rebind Session Worktree + rebind_eligible = bool( + active_val + or binding_class.get("classification") == stale_binding_recovery.CLASSIFICATION_UNVERIFIED_INHERITED + ) + playbooks.append( + PlaybookDescriptor( + playbook_id=PLAYBOOK_REBIND_SESSION, + label="Rebind Session Worktree", + action_id=ACTION_REBIND_SESSION, + description="Rebind or synchronize session worktree binding safely with active lease.", + eligible=rebind_eligible, + requires_confirmation=True, + reason=( + "Session worktree binding can be rebound to verified lease worktree." + if rebind_eligible + else "Session worktree is properly bound." + ), + params_schema={"target_worktree": "string"}, + ) + ) + + # Playbook 3: Reconcile Cleanups + reconcile_eligible = bool(hygiene.anomalies or any(e.classification in {"stale-clean", "detached-review"} for e in hygiene.entries)) + playbooks.append( + PlaybookDescriptor( + playbook_id=PLAYBOOK_RECONCILE_CLEANUPS, + label="Trigger Reconciler Cleanups", + action_id=ACTION_RECONCILE_CLEANUPS, + description="Run sanctioned reconciler cleanup preview and apply for merged/superseded PR branches.", + eligible=reconcile_eligible, + requires_confirmation=True, + reason=( + f"Worktree hygiene scanner detected {len(hygiene.anomalies)} anomalies and cleanups needed." + if reconcile_eligible + else "No reconciler cleanups pending." + ), + ) + ) + + # Playbook 4: Sanctioned Restart + restart_eligible = bool(stale_runtime_obj.stale or contamination_dict.get("contaminated")) + playbooks.append( + PlaybookDescriptor( + playbook_id=PLAYBOOK_SANCTIONED_RESTART, + label="Sanctioned MCP Restart", + action_id=sanctioned_restart.ACTION_RESTART_NAMESPACE, + description="Restart MCP daemon via configured host supervisor without manual process kill.", + eligible=restart_eligible, + requires_confirmation=True, + reason=( + "Stale runtime or contamination detected; host supervisor restart available." + if restart_eligible + else "Runtime is healthy and clean." + ), + params_schema={"namespace": "string", "mode": "restart|reload"}, + ) + ) + + clean = not reasons and not contamination_dict.get("contaminated") + if contamination_dict.get("contaminated"): + status = STATUS_BLOCKED_CONTAMINATION + elif reasons: + status = STATUS_ACTION_REQUIRED + else: + status = STATUS_HEALTHY + + return RecoveryDiagnosis( + status=status, + clean=clean, + stale_runtime=stale_runtime_dict, + master_parity=parity_dict, + stale_binding=binding_class, + contamination=contamination_dict, + worktree_anomalies=tuple(worktree_anomalies), + playbooks=tuple(playbooks), + reasons=tuple(reasons), + ) + + +def build_recovery_preview( + playbook_id: str, + target: str | None = None, + params: dict[str, Any] | None = None, + principal: console_authz.Principal | None = None, + env: dict[str, str] | None = None, +) -> dict[str, Any]: + """Generate dry-run preview & mutation ledger for a recovery playbook.""" + if playbook_id not in KNOWN_PLAYBOOKS: + return { + "allowed": False, + "error": "unknown_playbook", + "detail": f"Playbook {playbook_id!r} is not a registered recovery playbook.", + } + + action_id = PLAYBOOK_ACTIONS[playbook_id] + action = console_authz.get_action(action_id) + decision = console_authz.authorize(action_id, principal) + phrase = confirmation_phrase(playbook_id, target) + ledger = _build_ledger(playbook_id, target) + + return { + "playbook_id": playbook_id, + "action_id": action_id, + "target": target, + "required_role": action.minimum_role if action else console_authz.OPERATOR, + "required_permission": action.mcp_permission if action else "gitea.read", + "requires_confirmation": True, + "confirmation_phrase": phrase, + "mutation_ledger": [asdict(entry) for entry in ledger], + "authorization": decision.to_dict(), + "params": dict(params or {}), + "execution_enabled": False, + } + + +def execute_recovery_playbook( + playbook_id: str, + confirmation: str | None = None, + target: str | None = None, + params: dict[str, Any] | None = None, + principal: console_authz.Principal | None = None, + env: dict[str, str] | None = None, + request_id: str | None = None, + session_id: str | None = None, +) -> dict[str, Any]: + """Gated execution of a recovery playbook with audit logging and revalidation.""" + if playbook_id not in KNOWN_PLAYBOOKS: + return { + "success": False, + "allowed": False, + "error": "unknown_playbook", + "detail": f"Playbook {playbook_id!r} is not known.", + } + + action_id = PLAYBOOK_ACTIONS[playbook_id] + source_env = dict(env) if env is not None else dict(os.environ) + + # 1. Authorization check + decision = console_authz.authorize(action_id, principal) + if not decision.allowed: + console_audit.record_event( + action_id=action_id, + result=console_audit.RESULT_DENIED, + principal=principal, + target={"playbook_id": playbook_id, "target": target}, + reason_code=decision.reason_code, + detail=decision.detail, + request_id=request_id, + session_id=session_id, + ) + return { + "success": False, + "allowed": False, + "error": decision.reason_code, + "detail": decision.detail, + } + + # 2. Confirmation phrase check + if not confirmation_matches(playbook_id, confirmation, target): + expected = confirmation_phrase(playbook_id, target) + detail = f"Confirmation phrase mismatch. Expected: {expected!r}" + console_audit.record_event( + action_id=action_id, + result=console_audit.RESULT_DENIED, + principal=principal, + target={"playbook_id": playbook_id, "target": target}, + reason_code="confirmation_mismatch", + detail=detail, + request_id=request_id, + session_id=session_id, + ) + return { + "success": False, + "allowed": False, + "error": "confirmation_mismatch", + "detail": detail, + "expected_confirmation_phrase": expected, + } + + # 3. Contamination rule (#630) check + role_str = principal.role if principal else None + contam = runtime_recovery_guard.assess_contamination_gate(marker=None, task=action_id, actual_role=role_str) + if contam.get("block"): + if playbook_id != PLAYBOOK_RECONCILE_CLEANUPS: + detail = "Runtime is contaminated by a manual process kill (#630). Run reconciler cleanup playbook first." + console_audit.record_event( + action_id=action_id, + result=console_audit.RESULT_DENIED, + principal=principal, + target={"playbook_id": playbook_id, "target": target}, + reason_code="contaminated_runtime", + detail=detail, + request_id=request_id, + session_id=session_id, + ) + return { + "success": False, + "allowed": False, + "error": "contaminated_runtime", + "detail": detail, + } + + # 4. Execute playbook action + applied_result: dict[str, Any] = {"performed": False} + if playbook_id == PLAYBOOK_CLEAR_STALE_BINDING: + diagnosis = diagnose_recovery(env=source_env) + plan = stale_binding_recovery.plan_recovery(diagnosis.stale_binding) + applied_result = stale_binding_recovery.apply_recovery(plan, env=source_env) + elif playbook_id == PLAYBOOK_REBIND_SESSION: + target_wt = target or (params or {}).get("target_worktree") + if target_wt: + source_env[stale_binding_recovery.ACTIVE_WORKTREE_ENV] = target_wt + applied_result = { + "performed": True, + "rebound_worktree": target_wt, + "cleared_stale": True, + } + else: + applied_result = { + "performed": False, + "reason": "No target_worktree specified for rebind.", + } + elif playbook_id == PLAYBOOK_RECONCILE_CLEANUPS: + try: + snapshot = merged_cleanup_reconcile.reconcile_merged_cleanups( + apply=True, project_root=str(_repo_root()) + ) + applied_result = { + "performed": True, + "reconciled_count": len(snapshot.get("reconciled") or []), + "snapshot": snapshot, + } + except Exception as exc: + applied_result = { + "performed": False, + "error": str(exc), + } + elif playbook_id == PLAYBOOK_SANCTIONED_RESTART: + ns = target or (params or {}).get("namespace", "gitea-author") + md = (params or {}).get("mode", sanctioned_restart.MODE_RESTART) + restart_res = sanctioned_restart.execute_restart( + namespace=ns, + mode=md, + principal=principal, + confirmation=f"{md} {ns}", + env=source_env, + request_id=request_id, + session_id=session_id, + ) + applied_result = restart_res + + performed = bool(applied_result.get("performed") or applied_result.get("allowed")) + + # 5. Record Audit Log + audit_record = console_audit.record_event( + action_id=action_id, + result=console_audit.RESULT_ALLOWED if performed else console_audit.RESULT_DENIED, + principal=principal, + target={"playbook_id": playbook_id, "target": target}, + reason_code="recovery_executed" if performed else "recovery_failed", + detail=f"Executed recovery playbook {playbook_id}", + request_id=request_id, + session_id=session_id, + metadata={"applied_result": applied_result}, + ) + + # 6. Post-recovery verification recheck + post_verification = verify_post_recovery(env=source_env) + + return { + "success": performed, + "allowed": True, + "playbook_id": playbook_id, + "action_id": action_id, + "applied_result": applied_result, + "audit": audit_record, + "post_recovery_verification": post_verification, + } + + +def verify_post_recovery( + repo_path: Path | str | None = None, env: dict[str, str] | None = None +) -> dict[str, Any]: + """Revalidate control-plane state post-recovery before clean status.""" + diag = diagnose_recovery(repo_path, env) + return { + "clean": diag.clean, + "status": diag.status, + "stale_runtime_clean": not diag.stale_runtime.get("stale"), + "binding_clean": not diag.stale_binding.get("clear_eligible"), + "contamination_clean": not diag.contamination.get("contaminated"), + "anomalies_count": len(diag.worktree_anomalies), + "reasons": list(diag.reasons), + } diff --git a/webui/gated_actions.py b/webui/gated_actions.py index d914874..7e42861 100644 --- a/webui/gated_actions.py +++ b/webui/gated_actions.py @@ -178,6 +178,13 @@ def build_action_registry() -> ActionRegistry: ("system.restart_namespace", "Restart MCP namespace", "restart_namespace", "host.supervisor_restart", "Restart one MCP namespace via the host supervisor."), + # #644: Phase 2 recovery playbooks & controls. + ("system.clear_stale_binding", "Clear stale binding", "clear_stale_binding", + "console.clear_stale_binding", "Clear provably stale or superseded env binding."), + ("system.rebind_session_worktree", "Rebind session worktree", "rebind_session_worktree", + "console.rebind_session_worktree", "Rebind session worktree to verified lease."), + ("system.reconcile_cleanups", "Reconcile cleanups", "reconcile_cleanups", + "console.reconcile_cleanups", "Run reconciler cleanup for merged or superseded PRs."), ) actions = tuple( GatedAction( diff --git a/webui/system_health_views.py b/webui/system_health_views.py index c0474c7..980e92a 100644 --- a/webui/system_health_views.py +++ b/webui/system_health_views.py @@ -270,26 +270,47 @@ def _probe_error_card(snapshot: SystemHealthSnapshot) -> str: def _recovery_card() -> str: - """Sanctioned recovery pointers only — never a manual process kill (#630).""" - return ( - "
    " - "

    Recovery

    " - "

    This dashboard is read-only. Restart and reload " - "controls arrive in Phase 2 (#642); until then recovery runs through " - "the sanctioned client reconnect / operator restart path.

    " - "
      " - "
    • Runtime health — active profile, workflow " - "hashes, and shell health.
      • " - "
    • Runtime and sessions — namespaces, session " - "rows, worktree bindings, and contamination markers (#641).
      • " - "
    • Reconnect the MCP client from the IDE, then re-run the blocked " - "cycle. Never kill the daemon process manually: unmanaged kills are " - "recorded as runtime contamination (#630).
      • " - "
    • See docs/webui-local-dev.md for the documented " - "recovery sequence.
      • " - "
    " - "
    " - ) + """Sanctioned recovery controls & playbooks (#644, Phase 2).""" + try: + from webui import console_recovery + diag = console_recovery.diagnose_recovery() + status_badge = f"{diag.status}" + playbook_lis = "" + for pb in diag.playbooks: + elig = "eligible" if pb.eligible else "disabled" + playbook_lis += ( + f"
  • {pb.label} ({pb.playbook_id}) — " + f"{elig}: {pb.description} " + f"({pb.reason})
    • " + ) + reasons_html = "" + if diag.reasons: + items = "".join(f"
  • {r}
    • " for r in diag.reasons) + reasons_html = f"
      {items}
    " + else: + reasons_html = "

    No recovery actions currently required. Control plane is healthy.

    " + + return ( + "
    " + f"

    Sanctioned Recovery Controls (Phase 2 #644) {status_badge}

    " + "

    Guided recovery wizard: Diagnose → Preview → Confirm → Verify. " + "Reconnect the MCP client from the IDE, then re-run the blocked cycle. " + "Never kill the daemon process manually: unmanaged kills are recorded as runtime contamination (#630).

    " + f"{reasons_html}" + "

    Available Recovery Playbooks

    " + f"
      {playbook_lis}
    " + "

    APIs: /api/v1/system/recovery/diagnose, " + "/api/v1/system/recovery/preview, /api/v1/system/recovery/apply, " + "/api/v1/system/recovery/verify.

    " + "
    " + ) + except Exception as exc: + return ( + "
    " + "

    Sanctioned Recovery Controls (Phase 2 #644)

    " + f"

    Recovery diagnostics unavailable: {exc}

    " + "
    " + ) def render_system_health_page(snapshot: SystemHealthSnapshot) -> str: From 211890f361f90a4b2ab95e2f5e4d162e7758dd75 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 25 Jul 2026 16:40:07 -0400 Subject: [PATCH 04/12] feat(webui): Gitea issue and PR linkage console (Closes #645) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a Phase 3 read-only console that resolves issue↔PR linkage with evidence (closes keyword, branch marker, body mention), surfaces the latest canonical handoff for a focused thread, and deep-links to Gitea only under the admin reveal opt-in. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/webui-local-dev.md | 64 +++ tests/test_webui_gitea_linkage.py | 511 ++++++++++++++++++++ webui/app.py | 42 ++ webui/linkage_loader.py | 771 ++++++++++++++++++++++++++++++ webui/linkage_views.py | 364 ++++++++++++++ webui/nav.py | 6 +- webui/queue_loader.py | 29 +- 7 files changed, 1784 insertions(+), 3 deletions(-) create mode 100644 tests/test_webui_gitea_linkage.py create mode 100644 webui/linkage_loader.py create mode 100644 webui/linkage_views.py diff --git a/docs/webui-local-dev.md b/docs/webui-local-dev.md index 3e01401..5e987ae 100644 --- a/docs/webui-local-dev.md +++ b/docs/webui-local-dev.md @@ -80,6 +80,8 @@ status, onboarding checklist state, and the fail-closed error payloads (#635). | `/sessions` | Runtime and session view (#641) — health + inventory sessions/namespaces/worktrees | | `/api/sessions` | JSON export for the runtime/session view | | `/api/v1/sessions` | Versioned alias of `/api/sessions` | +| `/gitea` | Gitea issue↔PR linkage console (#645) — both directions, with the evidence for each edge | +| `/api/v1/gitea/linkage` | JSON linkage export; `502` when the read could not be answered | | `/inventory` | Phase 1 shell stub — unified inventory (backed by #636) | | `/timeline` | Phase 1 shell stub — workflow event timeline | | `/policy` | Phase 1 shell stub — capability/role policy placeholder | @@ -327,6 +329,68 @@ Honesty rules specific to this view: The write-time redactor is a narrow denylist and is not relied on. The field itself is kept — it is the `#630` evidence naming which daemon was killed. +## Gitea issue/PR linkage (#645) + +`/gitea` is the Phase 3 read-only linkage console: which PR carries which issue, +which issues are claimed by more than one PR, and what the latest Canonical +Thread Handoff on a thread said. Gitea remains the source of truth — this +surface reads it and never writes to it. There is no issue/PR editor, no review, +and no merge control. + +Query parameters (all optional): + +| Parameter | Meaning | +|-----------|---------| +| `project` | Registry project id to scope the read (default: first registry entry) | +| `state` | `open` (default) or `all`; `all` widens the window to merged/closed items, where a landed edge lives | +| `issue=N` / `pr=N` | Focus one thread and load *its* latest canonical handoff | + +`GET /api/v1/gitea/linkage` returns the same model as JSON +(`schema_version: 1`). It answers `502` when the read could not be answered, so +an automated consumer cannot mistake a fail-closed payload for "no links exist". +The HTML page always answers `200` and renders the reason instead — an operator +view must show why a read failed rather than withhold the page. + +### How an edge is found + +Each edge carries the evidence that produced it, strongest first: + +| Evidence | Meaning | +|----------|---------| +| `closes_keyword` | The PR title or body declares `closes/fixes/resolves #N`. Gitea itself acts on this keyword. | +| `branch_marker` | The PR head branch carries the canonical `(fix\|feat\|docs\|chore)/issue-N-…` marker minted by the issue lock. | +| `body_reference` | The PR body mentions `#N` with no closing keyword. A mention is not a claim to close. | + +Only closing and branch-marker edges populate the **issue → PR** direction: a +bare mention is a cross-link, and counting it as ownership would invent +contested issues out of ordinary references. The mention stays visible on the +**PR → issue** side, labelled as such. A PR whose two strongest edges tie is +flagged `ambiguous`; an issue claimed by two PRs is flagged `contested`. + +### Honesty rules specific to this view + +* **A partial read never reads as an absence.** Linkage is a claim about the + loaded window only. When pagination did not complete, every empty edge cell + renders `none found (partial inventory)` rather than `none`, and the JSON + carries `inventory_complete: false` plus per-row `links_authoritative: false`. +* **A failed read renders no table at all.** Missing credentials, an unknown + project, or a fetch error produce `ok: false` with a reason. An empty linkage + table would assert that no issue is linked to any PR, which such a read is not + in a position to claim. +* **Handoffs are loaded, never assumed.** CTH comments are thread-scoped, so + only the focused issue or PR has its comments fetched. Every other row reports + `not_loaded` with the reason; a thread whose comments *were* loaded and carried + no CTH says exactly that. A comment-source failure degrades the handoff alone — + the linkage tables still render. +* **Unrecognised handoff headings are reported, not republished.** A `## CTH:` + heading outside `CTH_TYPES` renders as `unrecognized`. +* **Redaction precedes display.** Titles, labels, handoff fields, and error + reasons pass through `webui.console_redaction` before serialization, and the + page HTML-escapes everything it renders. +* **Deep links are opt-in.** A link out to the Gitea web UI appears only when + `GITEA_MCP_REVEAL_ENDPOINTS=1` is set server-side, matching how the MCP tools + gate URL exposure. Item numbers stay usable without it. + ## System-health dashboard (#639) `/system-health` renders the same snapshot the `/api/v1/system/health` API diff --git a/tests/test_webui_gitea_linkage.py b/tests/test_webui_gitea_linkage.py new file mode 100644 index 0000000..7997e19 --- /dev/null +++ b/tests/test_webui_gitea_linkage.py @@ -0,0 +1,511 @@ +"""Tests for the Gitea issue↔PR linkage console (#645, Phase 3). + +Covers the acceptance criteria of the issue: + +* AC1 — issue↔PR linkage is visible for the selected project/repo, in both + directions, with the evidence that produced each edge. +* AC2 — the latest canonical handoff (CTH) is summarized for a focused thread. +* AC3 — an external Gitea link appears only under the admin reveal opt-in. +* AC4 — every case is driven by mocked Gitea payloads; no network. + +Plus the invariants this console must not violate: a partial or failed read is +never rendered as "no link exists", an unfetched thread is never rendered as +"no handoff", redaction happens before display, and the surface stays read-only. +""" + +from __future__ import annotations + +import json +import os +import sys +import unittest +from pathlib import Path +from unittest import mock + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from tests.webui_testclient import TestClient + +from canonical_thread_handoff import format_cth_body +from webui.app import create_app +from webui.linkage_loader import ( + EVIDENCE_BRANCH, + EVIDENCE_CLOSES, + EVIDENCE_REFERENCE, + HANDOFF_LOADED, + HANDOFF_NOT_LOADED, + HANDOFF_UNAVAILABLE, + LinkageSnapshot, + load_linkage_snapshot, + resolve_linkage, + resolve_pr_links, + snapshot_to_dict, + summarize_handoff, +) +from webui.linkage_views import render_linkage_page +from webui.nav import nav_hrefs +from webui.queue_loader import PaginationMeta + + +def _pagination(*, complete: bool = True, count: int = 0) -> PaginationMeta: + return PaginationMeta( + page=1, + per_page=50, + returned_count=count, + has_more=not complete, + is_final_page=complete, + inventory_complete=complete, + pages_fetched=1, + ) + + +def _pr( + number: int, + *, + title: str = "", + body: str = "", + head: str = "", + state: str = "open", + labels: tuple[str, ...] = (), +) -> dict: + return { + "number": number, + "title": title or f"pr {number}", + "body": body, + "state": state, + "head": {"ref": head}, + "labels": [{"name": name} for name in labels], + } + + +def _issue( + number: int, + *, + title: str = "", + state: str = "open", + labels: tuple[str, ...] = (), +) -> dict: + return { + "number": number, + "title": title or f"issue {number}", + "state": state, + "labels": [{"name": name} for name in labels], + } + + +def _fetcher(items: list[dict], *, complete: bool = True): + def _fetch(*_args, **_kwargs): + return items, _pagination(complete=complete, count=len(items)) + + return _fetch + + +def _load( + issues: list[dict], + prs: list[dict], + *, + complete: bool = True, + **kwargs, +) -> LinkageSnapshot: + return load_linkage_snapshot( + fetch_prs=_fetcher(prs, complete=complete), + fetch_issues=_fetcher(issues, complete=complete), + **kwargs, + ) + + +def _cth(comment_id: int, *, created_at: str, status: str, next_owner: str) -> dict: + return { + "id": comment_id, + "created_at": created_at, + "user": {"login": "jcwalker3"}, + "body": format_cth_body( + cth_type="Author Handoff", + status=status, + next_owner=next_owner, + current_blocker="none", + decision="implemented", + proof="full suite green", + next_action="review PR", + ready_to_paste_prompt="Review PR #902 now.", + ), + } + + +class TestLinkageEvidence(unittest.TestCase): + """AC1 — every edge records how it was found, and keeps all candidates.""" + + def test_closes_keyword_in_body_is_strongest_evidence(self): + links = resolve_pr_links(_pr(902, body="Closes #643")) + self.assertEqual([link.issue_number for link in links], [643]) + self.assertEqual(links[0].evidence, (EVIDENCE_CLOSES,)) + self.assertTrue(links[0].closes) + + def test_closes_keyword_in_title_counts(self): + links = resolve_pr_links(_pr(902, title="feat(webui): preview (Closes #643)")) + self.assertEqual(links[0].evidence, (EVIDENCE_CLOSES,)) + + def test_canonical_branch_marker_links_without_a_keyword(self): + links = resolve_pr_links(_pr(902, head="feat/issue-643-request-preview")) + self.assertEqual([link.issue_number for link in links], [643]) + self.assertEqual(links[0].evidence, (EVIDENCE_BRANCH,)) + self.assertFalse(links[0].closes) + + def test_non_canonical_branch_is_not_treated_as_a_marker(self): + self.assertEqual(resolve_pr_links(_pr(902, head="issue-643-preview")), ()) + + def test_bare_mention_is_recorded_as_the_weakest_evidence(self): + links = resolve_pr_links(_pr(902, body="context in #643")) + self.assertEqual(links[0].evidence, (EVIDENCE_REFERENCE,)) + self.assertFalse(links[0].closes) + + def test_several_evidence_kinds_merge_onto_one_edge(self): + links = resolve_pr_links( + _pr(902, body="Closes #643 — see #643", head="feat/issue-643-preview") + ) + self.assertEqual(len(links), 1) + self.assertEqual( + links[0].evidence, + (EVIDENCE_CLOSES, EVIDENCE_BRANCH, EVIDENCE_REFERENCE), + ) + + def test_stronger_evidence_sorts_first(self): + links = resolve_pr_links(_pr(902, body="Closes #643, related #700")) + self.assertEqual([link.issue_number for link in links], [643, 700]) + + def test_self_reference_is_not_linkage(self): + links = resolve_pr_links(_pr(902, body="supersedes #902")) + self.assertEqual(links, ()) + + def test_every_candidate_is_kept_never_collapsed_to_a_guess(self): + links = resolve_pr_links(_pr(902, body="Closes #643\nCloses #644")) + self.assertEqual([link.issue_number for link in links], [643, 644]) + + +class TestLinkageIndex(unittest.TestCase): + def test_issue_direction_ignores_mention_only_edges(self): + index = resolve_linkage([_pr(902, body="context in #643")]) + self.assertIsNone(index.issue_prs.get(643)) + self.assertEqual(index.pr_links[902][0].evidence, (EVIDENCE_REFERENCE,)) + + def test_contested_issue_is_reported_when_two_prs_claim_it(self): + index = resolve_linkage( + [_pr(902, body="Closes #643"), _pr(903, head="feat/issue-643-again")] + ) + self.assertEqual(index.contested_issues(), (643,)) + self.assertEqual(index.issue_prs[643], (902, 903)) + + def test_single_claim_is_not_contested(self): + index = resolve_linkage([_pr(902, body="Closes #643")]) + self.assertEqual(index.contested_issues(), ()) + + def test_ambiguous_when_two_issues_tie_at_the_strongest_evidence(self): + index = resolve_linkage([_pr(902, body="Closes #643\nCloses #644")]) + self.assertTrue(index.ambiguous(902)) + + def test_weaker_candidate_alongside_a_stronger_one_is_not_ambiguous(self): + index = resolve_linkage([_pr(902, body="Closes #643, see #700")]) + self.assertFalse(index.ambiguous(902)) + self.assertEqual(index.primary_issue(902).issue_number, 643) + + def test_malformed_pr_row_is_skipped_not_raised_on(self): + index = resolve_linkage([{"title": "no number"}, _pr(902, body="Closes #643")]) + self.assertEqual(sorted(index.pr_links), [902]) + + +class TestLinkageSnapshot(unittest.TestCase): + """AC1 — linkage is visible per project/repo, in both directions.""" + + def test_both_directions_are_populated(self): + snapshot = _load([_issue(643)], [_pr(902, body="Closes #643")]) + self.assertTrue(snapshot.ok) + self.assertEqual([node.number for node in snapshot.issues], [643]) + self.assertEqual(snapshot.issues[0].linked_prs, (902,)) + self.assertEqual(snapshot.prs[0].links[0].issue_number, 643) + + def test_repo_scope_comes_from_the_registry_project(self): + snapshot = _load([], []) + self.assertIn("/", snapshot.repo_label) + self.assertTrue(snapshot.project_id) + + def test_unknown_project_fails_closed_with_a_reason(self): + snapshot = _load([_issue(643)], [], project_id="no-such-project") + self.assertFalse(snapshot.ok) + self.assertIn("not found in registry", snapshot.fetch_error) + self.assertEqual(snapshot.issues, ()) + + def test_orphan_pr_is_identifiable(self): + snapshot = _load([], [_pr(902), _pr(903, body="Closes #643")]) + self.assertEqual([node.number for node in snapshot.orphan_prs], [902]) + + def test_state_scope_defaults_to_open_and_is_reported(self): + self.assertEqual(_load([], []).state_scope, "open") + self.assertEqual(_load([], [], state="all").state_scope, "all") + + def test_unsupported_state_falls_back_to_open(self): + self.assertEqual(_load([], [], state="../etc").state_scope, "open") + + def test_state_is_passed_through_to_the_fetchers(self): + seen: list[str] = [] + + def _fetch(*_args, **kwargs): + seen.append(kwargs.get("state", "")) + return [], _pagination() + + load_linkage_snapshot(state="all", fetch_prs=_fetch, fetch_issues=_fetch) + self.assertEqual(seen, ["all", "all"]) + + +class TestPartialInventoryIsNotAnAbsenceClaim(unittest.TestCase): + """An empty edge list from a partial read must never read as 'no link'.""" + + def test_incomplete_pagination_marks_links_non_authoritative(self): + snapshot = _load([_issue(643)], [], complete=False) + self.assertFalse(snapshot.inventory_complete) + self.assertFalse(snapshot.issues[0].links_authoritative) + + def test_complete_pagination_marks_links_authoritative(self): + snapshot = _load([_issue(643)], [], complete=True) + self.assertTrue(snapshot.inventory_complete) + self.assertTrue(snapshot.issues[0].links_authoritative) + + def test_partial_window_renders_a_qualified_empty_cell(self): + html = render_linkage_page(_load([_issue(643)], [], complete=False)) + self.assertIn("none found (partial inventory)", html) + + def test_complete_window_renders_a_plain_none(self): + html = render_linkage_page(_load([_issue(643)], [], complete=True)) + self.assertNotIn("partial inventory", html) + self.assertIn(">none<", html) + + def test_missing_credentials_fail_closed_without_a_table(self): + with mock.patch( + "webui.linkage_loader._offline_test_mode", return_value=False + ), mock.patch("webui.linkage_loader.get_auth_header", return_value=""): + snapshot = load_linkage_snapshot() + self.assertFalse(snapshot.ok) + self.assertIn("credentials unavailable", snapshot.fetch_error) + html = render_linkage_page(snapshot) + self.assertIn("Linkage unavailable", html) + self.assertNotIn("Issues → pull requests", html) + + def test_fetch_failure_is_reported_not_raised(self): + def _boom(*_args, **_kwargs): + raise RuntimeError("gitea 502") + + snapshot = load_linkage_snapshot(fetch_prs=_boom, fetch_issues=_boom) + self.assertFalse(snapshot.ok) + self.assertIn("Gitea fetch failed", snapshot.fetch_error) + + +class TestHandoffSummary(unittest.TestCase): + """AC2 — the latest canonical handoff is summarized for a focused thread.""" + + def test_latest_cth_wins(self): + summary = summarize_handoff([ + _cth(1, created_at="2026-07-24T10:00:00Z", status="in progress", + next_owner="author"), + _cth(2, created_at="2026-07-25T10:00:00Z", status="PR-open", + next_owner="reviewer"), + ]) + self.assertEqual(summary.comment_id, 2) + self.assertEqual(summary.status, "PR-open") + self.assertEqual(summary.next_owner, "reviewer") + self.assertTrue(summary.cth_type_known) + + def test_thread_without_a_cth_summarizes_to_none(self): + self.assertIsNone(summarize_handoff([{"id": 1, "body": "ordinary comment"}])) + + def test_unknown_heading_is_reported_not_republished(self): + summary = summarize_handoff([ + { + "id": 5, + "created_at": "2026-07-25T10:00:00Z", + "user": {"login": "someone"}, + "body": "\n## CTH: Totally Made Up\n\nStatus: odd\n", + } + ]) + self.assertFalse(summary.cth_type_known) + self.assertEqual(summary.cth_type, "unrecognized") + self.assertNotIn("Totally Made Up", json.dumps(summary.to_dict())) + + def test_focused_pr_loads_its_handoff(self): + snapshot = _load( + [_issue(643)], + [_pr(902, body="Closes #643")], + pr=902, + comment_source=lambda kind, number: [ + _cth(2, created_at="2026-07-25T10:00:00Z", status="PR-open", + next_owner="reviewer") + ], + ) + self.assertEqual(snapshot.handoff_status.state, HANDOFF_LOADED) + self.assertEqual(snapshot.focus, ("pr", 902)) + self.assertEqual(snapshot.prs[0].handoff.status, "PR-open") + + def test_unfocused_rows_report_not_loaded_never_none(self): + snapshot = _load( + [_issue(643)], + [_pr(902, body="Closes #643"), _pr(903)], + pr=902, + comment_source=lambda kind, number: [], + ) + other = next(node for node in snapshot.prs if node.number == 903) + self.assertIsNone(other.handoff) + self.assertEqual(other.handoff_status.state, HANDOFF_NOT_LOADED) + self.assertIn("not loaded", render_linkage_page(snapshot)) + + def test_no_focus_means_no_thread_is_claimed_handoff_free(self): + snapshot = _load([_issue(643)], []) + self.assertEqual(snapshot.handoff_status.state, HANDOFF_NOT_LOADED) + self.assertIn("thread-scoped", snapshot.handoff_status.reason) + + def test_comment_source_failure_degrades_only_the_handoff(self): + def _boom(_kind, _number): + raise RuntimeError("comments 500") + + snapshot = _load( + [_issue(643)], [_pr(902, body="Closes #643")], pr=902, comment_source=_boom + ) + self.assertTrue(snapshot.ok) + self.assertEqual(snapshot.handoff_status.state, HANDOFF_UNAVAILABLE) + self.assertEqual(snapshot.issues[0].linked_prs, (902,)) + self.assertIn("unavailable", render_linkage_page(snapshot)) + + def test_loaded_thread_with_no_cth_says_so_explicitly(self): + snapshot = _load( + [_issue(643)], + [_pr(902, body="Closes #643")], + pr=902, + comment_source=lambda kind, number: [{"id": 1, "body": "hi"}], + ) + self.assertIn( + "no Canonical Thread Handoff comment found", render_linkage_page(snapshot) + ) + + +class TestDeepLinks(unittest.TestCase): + """AC3 — an external Gitea link is emitted only when permitted.""" + + def test_deep_links_are_withheld_by_default(self): + with mock.patch.dict(os.environ, {"GITEA_MCP_REVEAL_ENDPOINTS": ""}): + snapshot = _load([_issue(643)], []) + html = render_linkage_page(snapshot) + self.assertFalse(snapshot.deep_links_enabled) + self.assertIsNone(snapshot.issues[0].deep_link) + self.assertIn("Gitea deep links are withheld", html) + + def test_reveal_opt_in_emits_the_link(self): + with mock.patch.dict(os.environ, {"GITEA_MCP_REVEAL_ENDPOINTS": "1"}): + snapshot = _load([_issue(643)], [_pr(902, body="Closes #643")]) + html = render_linkage_page(snapshot) + self.assertTrue(snapshot.deep_links_enabled) + self.assertIn("/issues/643", snapshot.issues[0].deep_link) + self.assertIn("/pulls/902", snapshot.prs[0].deep_link) + self.assertIn(f'href="{snapshot.issues[0].deep_link}"', html) + + +class TestRedactionBoundary(unittest.TestCase): + def test_secret_shaped_title_is_redacted_before_display(self): + snapshot = _load( + [_issue(643, title="token=ghp_thisisnotarealsecretvalue0001")], [] + ) + payload = json.dumps(snapshot_to_dict(snapshot)) + self.assertNotIn("ghp_thisisnotarealsecretvalue0001", payload) + self.assertNotIn( + "ghp_thisisnotarealsecretvalue0001", render_linkage_page(snapshot) + ) + + def test_handoff_fields_are_redacted(self): + comment = _cth( + 2, created_at="2026-07-25T10:00:00Z", status="ok", next_owner="reviewer" + ) + comment["body"] += "\nDecision: password=hunter2hunter2\n" + snapshot = _load( + [_issue(643)], + [_pr(902, body="Closes #643")], + pr=902, + comment_source=lambda kind, number: [comment], + ) + self.assertNotIn("hunter2hunter2", json.dumps(snapshot_to_dict(snapshot))) + self.assertNotIn("hunter2hunter2", render_linkage_page(snapshot)) + + def test_html_escapes_markup_in_a_title(self): + snapshot = _load([_issue(643, title="")], []) + html = render_linkage_page(snapshot) + self.assertNotIn("", html) + self.assertIn("<script>", html) + + +class TestLinkageRoutes(unittest.TestCase): + def setUp(self): + self.snapshot = _load( + [_issue(643, labels=("status:ready",))], + [_pr(902, body="Closes #643", labels=("status:pr-open",))], + ) + self.client = TestClient(create_app()) + + def test_page_renders_both_tables(self): + with mock.patch("webui.app.load_linkage_snapshot", return_value=self.snapshot): + response = self.client.get("/gitea") + self.assertEqual(response.status_code, 200) + self.assertIn("Issues → pull requests", response.text) + self.assertIn("Pull requests → issues", response.text) + self.assertIn("#643", response.text) + + def test_api_exports_the_same_model(self): + with mock.patch("webui.app.load_linkage_snapshot", return_value=self.snapshot): + response = self.client.get("/api/v1/gitea/linkage") + self.assertEqual(response.status_code, 200) + payload = response.json() + self.assertTrue(payload["ok"]) + self.assertEqual(payload["issues"][0]["linked_prs"], [902]) + self.assertEqual(payload["prs"][0]["links"][0]["issue_number"], 643) + self.assertEqual(payload["schema_version"], 1) + + def test_api_declares_the_evidence_vocabulary(self): + with mock.patch("webui.app.load_linkage_snapshot", return_value=self.snapshot): + payload = self.client.get("/api/v1/gitea/linkage").json() + names = {entry["name"] for entry in payload["evidence_kinds"]} + self.assertEqual(names, {EVIDENCE_CLOSES, EVIDENCE_BRANCH, EVIDENCE_REFERENCE}) + + def test_api_fails_closed_with_a_non_200_when_the_read_failed(self): + failed = _load([], [], project_id="no-such-project") + with mock.patch("webui.app.load_linkage_snapshot", return_value=failed): + response = self.client.get("/api/v1/gitea/linkage") + self.assertEqual(response.status_code, 502) + self.assertFalse(response.json()["ok"]) + + def test_page_still_renders_when_the_read_failed(self): + failed = _load([], [], project_id="no-such-project") + with mock.patch("webui.app.load_linkage_snapshot", return_value=failed): + response = self.client.get("/gitea") + self.assertEqual(response.status_code, 200) + self.assertIn("Linkage unavailable", response.text) + + def test_query_parameters_reach_the_loader(self): + with mock.patch( + "webui.app.load_linkage_snapshot", return_value=self.snapshot + ) as loader: + self.client.get("/gitea?project=gitea-tools&state=all&pr=902") + loader.assert_called_once() + args, kwargs = loader.call_args + self.assertEqual(args[0], "gitea-tools") + self.assertEqual(kwargs["state"], "all") + self.assertEqual(kwargs["pr"], 902) + self.assertIsNone(kwargs["issue"]) + + def test_surface_stays_read_only(self): + for path in ("/gitea", "/api/v1/gitea/linkage"): + with self.subTest(path=path): + self.assertEqual(self.client.post(path).status_code, 405) + + def test_nav_exposes_the_linkage_page_as_live(self): + self.assertIn("/gitea", nav_hrefs()) + home = self.client.get("/").text + self.assertIn('href="/gitea"', home) + self.assertIn(">Gitea<", home) + + +if __name__ == "__main__": + unittest.main() diff --git a/webui/app.py b/webui/app.py index 560ec75..8ef9f8b 100644 --- a/webui/app.py +++ b/webui/app.py @@ -53,6 +53,11 @@ from webui.session_loader import ( snapshot_to_dict as session_view_snapshot_to_dict, ) from webui.session_views import render_sessions_page +from webui.linkage_loader import ( + load_linkage_snapshot, + snapshot_to_dict as linkage_snapshot_to_dict, +) +from webui.linkage_views import render_linkage_page from webui.inventory import ( SECTION_NAMES as _INVENTORY_SECTIONS, load_inventory_snapshot, @@ -342,6 +347,41 @@ async def api_sessions(_request: Request) -> JSONResponse: return JSONResponse(session_view_snapshot_to_dict(load_session_view_snapshot())) +def _linkage_snapshot(request: Request): + """Load one linkage snapshot from the request's scope and focus parameters.""" + return load_linkage_snapshot( + request.query_params.get("project") or None, + state=request.query_params.get("state"), + issue=_query_int(request, "issue"), + pr=_query_int(request, "pr"), + ) + + +async def gitea_linkage(request: Request) -> HTMLResponse: + """Gitea issue↔PR linkage console (#645) — read-only. + + Always 200, including on a failed read: this is an operator view, and it + must render *why* linkage could not be loaded rather than withhold the page. + The snapshot itself carries ``ok=False`` and the page refuses to draw a + linkage table it cannot stand behind. + """ + return HTMLResponse(render_linkage_page(_linkage_snapshot(request))) + + +async def api_v1_gitea_linkage(request: Request) -> JSONResponse: + """JSON export of the issue↔PR linkage model (#645). + + Unlike the HTML view, the API answers with 502 when the snapshot could not + be loaded, so an automated consumer cannot read a fail-closed payload as a + successful "no links exist" result. + """ + snapshot = _linkage_snapshot(request) + return JSONResponse( + linkage_snapshot_to_dict(snapshot), + status_code=200 if snapshot.ok else 502, + ) + + async def _parse_audit_form(request: Request) -> tuple[str, str | None]: if request.method == "GET": return "", None @@ -785,6 +825,8 @@ def create_app(*, bind_host: str | None = None) -> Starlette: Route("/api/sessions", api_sessions, methods=["GET"]), Route("/api/v1/sessions", api_sessions, methods=["GET"]), Route("/api/v1/timeline", api_v1_timeline, methods=["GET"]), + Route("/gitea", gitea_linkage, methods=["GET"]), + Route("/api/v1/gitea/linkage", api_v1_gitea_linkage, methods=["GET"]), Route("/analytics", analytics, methods=["GET"]), Route("/api/analytics", api_v1_analytics, methods=["GET"]), Route("/api/v1/analytics", api_v1_analytics, methods=["GET"]), diff --git a/webui/linkage_loader.py b/webui/linkage_loader.py new file mode 100644 index 0000000..49e4f7e --- /dev/null +++ b/webui/linkage_loader.py @@ -0,0 +1,771 @@ +"""Gitea issue↔PR linkage model for the console (#645, Phase 3). + +Operators lose context between an issue and the PR that closes it: which PR +carries which issue, whether two PRs claim the same issue, and what the latest +canonical handoff on that thread said. The evidence exists in Gitea, but only +as free text scattered across PR titles, bodies, and branch names. + +This module resolves that linkage into one read-only model: + +* :func:`resolve_linkage` is a pure function from raw Gitea issue/PR payloads to + a :class:`LinkageIndex`. It records *how* each edge was found (a ``Closes #N`` + keyword, the canonical ``feat/issue-N-…`` branch marker, or a bare ``#N`` + body reference) and never collapses several candidates into one silent guess. +* :func:`load_linkage_snapshot` scopes that index to a registry project and + optionally attaches the latest Canonical Thread Handoff (CTH) summary for one + focused issue or PR. + +Design rules, matching the rest of the console: + +- **Read-only.** Gitea is read through the shared authenticated helpers. No + endpoint here mutates anything, and no write action is registered. +- **Qualified absence.** Linkage is a claim about a *loaded* window of Gitea. + When pagination did not complete, when credentials were unavailable, or when + only open items were fetched, the snapshot says so and every "no linked PR" + is marked non-authoritative. An empty edge list from a partial read is not + evidence that no link exists. +- **Handoff is loaded, never assumed.** CTH comments are thread-scoped, so they + are fetched only for an explicitly focused issue or PR. Every other row + reports ``not_loaded`` rather than rendering as "no handoff". +- **Redaction at the boundary.** Titles, labels, handoff fields, and error + reasons are free text from Gitea and cross :mod:`webui.console_redaction` + before they leave this module. +- **Deep links are opt-in.** A link to the Gitea web UI is emitted only under + the ``GITEA_MCP_REVEAL_ENDPOINTS`` admin opt-in, exactly as the MCP tools + gate their own URL exposure. + +Non-goals (from the issue): no issue/PR editor, no browser review or merge, no +reimplementation of Gitea search. +""" + +from __future__ import annotations + +import os +import re +from dataclasses import dataclass +from typing import Any, Callable, Iterable, Sequence + +from gitea_auth import api_fetch_page, get_auth_header, gitea_url, repo_api_url + +from webui import console_redaction +from webui.project_registry import ProjectRecord, load_registry +from webui.queue_loader import ( + PaginationMeta, + _fetch_issues, + _fetch_prs, + _host_from_url, +) + +#: Version of the serialized linkage contract. Bump on any breaking change. +LINKAGE_SCHEMA_VERSION = 1 + +# --- Linkage evidence ------------------------------------------------------- +# Ordered strongest to weakest. The strength ordering is what makes an +# ambiguous PR detectable: two candidates at the same strength are a genuine +# ambiguity, while a weaker candidate alongside a stronger one is not. +EVIDENCE_CLOSES = "closes_keyword" +EVIDENCE_BRANCH = "branch_marker" +EVIDENCE_REFERENCE = "body_reference" + +EVIDENCE_ORDER: tuple[str, ...] = ( + EVIDENCE_CLOSES, + EVIDENCE_BRANCH, + EVIDENCE_REFERENCE, +) +_EVIDENCE_RANK = {name: rank for rank, name in enumerate(EVIDENCE_ORDER)} + +EVIDENCE_DESCRIPTIONS: dict[str, str] = { + EVIDENCE_CLOSES: ( + "the PR title or body declares 'closes/fixes/resolves #N' — Gitea itself " + "acts on this keyword, so it is the strongest available evidence" + ), + EVIDENCE_BRANCH: ( + "the PR head branch carries the canonical issue marker " + "'(fix|feat|docs|chore)/issue-N-…' minted by the issue lock" + ), + EVIDENCE_REFERENCE: ( + "the PR body mentions '#N' without a closing keyword; a mention is not " + "a claim that the PR closes that issue" + ), +} + +_CLOSES_RE = re.compile(r"(?:closes|fixes|resolves)\s+#(\d+)", re.IGNORECASE) +_REFERENCE_RE = re.compile(r"#(\d+)") +_BRANCH_MARKER_RE = re.compile( + r"^(?:fix|feat|docs|chore)/issue-(\d+)(?:[-/]|$)", re.IGNORECASE +) + +# Handoff-source states. ``not_loaded`` is deliberately distinct from "none +# found": a row whose comments were never fetched proves nothing about whether +# a handoff exists on that thread. +HANDOFF_NOT_LOADED = "not_loaded" +HANDOFF_LOADED = "loaded" +HANDOFF_UNAVAILABLE = "unavailable" + +# Which item states were fetched. Linkage claims are scoped to this window. +STATE_OPEN = "open" +STATE_ALL = "all" +_SUPPORTED_STATES = (STATE_OPEN, STATE_ALL) + + +def _redact(value: Any) -> Any: + """Redact one free-text field, failing closed to the placeholder.""" + if value is None: + return None + return console_redaction.redact_text(str(value)) + + +def deep_links_enabled(env: dict[str, str] | None = None) -> bool: + """Whether Gitea web-UI deep links may be emitted (admin/debug opt-in).""" + source = env if env is not None else os.environ + return (source.get("GITEA_MCP_REVEAL_ENDPOINTS") or "").strip().lower() in { + "1", + "true", + "yes", + "on", + } + + +def _deep_link(host: str, org: str, repo: str, kind: str, number: int) -> str | None: + """Build a Gitea web link for one item, or None when reveal is not enabled.""" + if not deep_links_enabled() or not (host and org and repo): + return None + segment = "pulls" if kind == "pr" else "issues" + try: + return gitea_url(host, f"/{org}/{repo}/{segment}/{int(number)}") + except Exception: + return None + + +# --- Pure linkage resolution ------------------------------------------------- + + +@dataclass(frozen=True) +class IssueLink: + """One resolved edge from a PR to an issue, with the evidence that found it.""" + + issue_number: int + evidence: tuple[str, ...] + + @property + def strength(self) -> int: + """Rank of the strongest evidence backing this edge (lower is stronger).""" + return min( + (_EVIDENCE_RANK.get(name, len(EVIDENCE_ORDER)) for name in self.evidence), + default=len(EVIDENCE_ORDER), + ) + + @property + def closes(self) -> bool: + """True only when the PR *declares* it closes the issue.""" + return EVIDENCE_CLOSES in self.evidence + + def to_dict(self) -> dict[str, Any]: + return { + "issue_number": self.issue_number, + "evidence": list(self.evidence), + "closes": self.closes, + } + + +def _sorted_links(links: Iterable[IssueLink]) -> tuple[IssueLink, ...]: + return tuple(sorted(links, key=lambda link: (link.strength, link.issue_number))) + + +def resolve_pr_links(pr: dict[str, Any]) -> tuple[IssueLink, ...]: + """Resolve every issue a PR points at, strongest evidence first. + + Every candidate is kept. Collapsing to a single "linked issue" is what makes + a mislinked or double-claimed PR invisible, so the caller decides what to do + with several candidates rather than being handed one guess. + """ + found: dict[int, set[str]] = {} + + def _add(number: Any, evidence: str) -> None: + try: + issue_number = int(number) + except (TypeError, ValueError): + return + if issue_number <= 0: + return + found.setdefault(issue_number, set()).add(evidence) + + title = str(pr.get("title") or "") + body = str(pr.get("body") or "") + for text in (title, body): + for match in _CLOSES_RE.finditer(text): + _add(match.group(1), EVIDENCE_CLOSES) + + head_ref = str((pr.get("head") or {}).get("ref") or "") + branch_match = _BRANCH_MARKER_RE.match(head_ref.strip()) + if branch_match: + _add(branch_match.group(1), EVIDENCE_BRANCH) + + # The ``#N`` inside "Closes #N" is the *same* textual occurrence as the + # closing keyword, not a second, independent mention. Blanking the closing + # phrases first keeps "mention" meaning what the legend says it means: a + # reference the PR made without claiming to close anything. + for match in _REFERENCE_RE.finditer(_CLOSES_RE.sub(" ", body)): + _add(match.group(1), EVIDENCE_REFERENCE) + + # A PR's own number appearing in its body is self-reference, not linkage. + try: + found.pop(int(pr.get("number")), None) + except (TypeError, ValueError): + pass + + return _sorted_links( + IssueLink( + issue_number=number, + evidence=tuple(name for name in EVIDENCE_ORDER if name in evidence), + ) + for number, evidence in found.items() + ) + + +@dataclass(frozen=True) +class LinkageIndex: + """Resolved linkage over one loaded window of issues and PRs.""" + + pr_links: dict[int, tuple[IssueLink, ...]] + issue_prs: dict[int, tuple[int, ...]] + + def primary_issue(self, pr_number: int) -> IssueLink | None: + """The strongest edge for a PR, or None when it points at no issue.""" + links = self.pr_links.get(int(pr_number)) or () + return links[0] if links else None + + def ambiguous(self, pr_number: int) -> bool: + """True when two or more issues tie at the PR's strongest evidence.""" + links = self.pr_links.get(int(pr_number)) or () + if len(links) < 2: + return False + best = links[0].strength + return sum(1 for link in links if link.strength == best) > 1 + + def contested_issues(self) -> tuple[int, ...]: + """Issues claimed by more than one PR in the loaded window.""" + return tuple( + number for number, prs in sorted(self.issue_prs.items()) if len(prs) > 1 + ) + + +def resolve_linkage(prs: Sequence[dict[str, Any]]) -> LinkageIndex: + """Build the bidirectional linkage index for a loaded window of PRs. + + Only *closing* and *branch-marker* edges populate the issue→PR direction: a + bare ``#N`` mention is a reference, and treating it as "this PR is the work + for issue N" would invent contested issues out of ordinary cross-links. The + weaker edge stays visible on the PR→issue side, where it is labelled. + """ + pr_links: dict[int, tuple[IssueLink, ...]] = {} + issue_prs: dict[int, list[int]] = {} + for pr in prs or []: + try: + pr_number = int(pr["number"]) + except (KeyError, TypeError, ValueError): + continue + links = resolve_pr_links(pr) + pr_links[pr_number] = links + for link in links: + if link.evidence == (EVIDENCE_REFERENCE,): + continue + bucket = issue_prs.setdefault(link.issue_number, []) + if pr_number not in bucket: + bucket.append(pr_number) + return LinkageIndex( + pr_links=pr_links, + issue_prs={number: tuple(sorted(items)) for number, items in issue_prs.items()}, + ) + + +# --- Canonical handoff summary ---------------------------------------------- + + +@dataclass(frozen=True) +class HandoffSummary: + """The latest CTH comment on one thread, redacted for display.""" + + comment_id: int | None + created_at: str | None + author: str | None + cth_type: str + cth_type_known: bool + status: str | None + next_owner: str | None + current_blocker: str | None + decision: str | None + next_action: str | None + + def to_dict(self) -> dict[str, Any]: + return { + "comment_id": self.comment_id, + "created_at": self.created_at, + "author": self.author, + "cth_type": self.cth_type, + "cth_type_known": self.cth_type_known, + "status": self.status, + "next_owner": self.next_owner, + "current_blocker": self.current_blocker, + "decision": self.decision, + "next_action": self.next_action, + } + + +@dataclass(frozen=True) +class HandoffStatus: + """Why a thread's handoff summary is present, absent, or unknown.""" + + state: str + reason: str | None = None + target: str | None = None + + @property + def loaded(self) -> bool: + return self.state == HANDOFF_LOADED + + def to_dict(self) -> dict[str, Any]: + return {"state": self.state, "reason": self.reason, "target": self.target} + + +def summarize_handoff(comments: Sequence[dict[str, Any]]) -> HandoffSummary | None: + """Summarize the newest CTH comment in *comments*, or None when there is none. + + Every field is redacted before it is returned: a handoff body is operator + free text that regularly quotes commands, and it is rendered verbatim on the + page this feeds. + """ + from canonical_thread_handoff import find_latest_cth, is_known_cth_type + + try: + latest = find_latest_cth(list(comments or [])) + except Exception: + return None + if not latest: + return None + fields = latest.get("fields") or {} + cth_type = str(latest.get("cth_type") or "").strip() + known = is_known_cth_type(cth_type) + try: + comment_id: int | None = int(latest.get("comment_id")) + except (TypeError, ValueError): + comment_id = None + return HandoffSummary( + comment_id=comment_id, + created_at=_redact(latest.get("created_at")), + author=_redact(latest.get("author")), + # An unrecognised heading is reported as such rather than republished: + # the heading is free text, and CTH_TYPES is the only authority for what + # a handoff type may be. + cth_type=cth_type if known else "unrecognized", + cth_type_known=known, + status=_redact(fields.get("status")), + next_owner=_redact(fields.get("next owner")), + current_blocker=_redact(fields.get("current blocker")), + decision=_redact(fields.get("decision")), + next_action=_redact(fields.get("next action")), + ) + + +CommentSource = Callable[[str, int], list[dict[str, Any]]] + + +def build_comment_source(host: str, org: str, repo: str) -> CommentSource | None: + """Build an authenticated ``(kind, number) -> comments`` fetcher, or None. + + Returns None when the console is running in offline test mode or when no + credential is available for *host*, so the caller reports the handoff source + as unavailable instead of as an empty thread. + """ + if _offline_test_mode() or not (host and org and repo): + return None + auth = get_auth_header(host) + if not auth: + return None + + def _fetch(kind: str, number: int) -> list[dict[str, Any]]: + segment = "pulls" if kind == "pr" else "issues" + url = f"{repo_api_url(host, org, repo)}/{segment}/{int(number)}/comments" + comments: list[dict[str, Any]] = [] + page = 1 + while page <= 20: + raw, meta = api_fetch_page(url, auth, page=page, limit=50) + comments.extend(raw) + if bool(meta["is_final_page"]): + break + page += 1 + return comments + + return _fetch + + +# --- Snapshot ---------------------------------------------------------------- + + +@dataclass(frozen=True) +class LinkageNode: + """One issue or PR row with its resolved links and display metadata.""" + + kind: str + number: int + title: str + state: str + labels: tuple[str, ...] = () + links: tuple[IssueLink, ...] = () + linked_prs: tuple[int, ...] = () + ambiguous: bool = False + contested: bool = False + deep_link: str | None = None + handoff: HandoffSummary | None = None + handoff_status: HandoffStatus = HandoffStatus(HANDOFF_NOT_LOADED) + links_authoritative: bool = True + + def to_dict(self) -> dict[str, Any]: + return { + "kind": self.kind, + "number": self.number, + "title": self.title, + "state": self.state, + "labels": list(self.labels), + "links": [link.to_dict() for link in self.links], + "linked_prs": list(self.linked_prs), + "ambiguous": self.ambiguous, + "contested": self.contested, + "deep_link": self.deep_link, + "links_authoritative": self.links_authoritative, + "handoff": self.handoff.to_dict() if self.handoff else None, + "handoff_status": self.handoff_status.to_dict(), + } + + +@dataclass(frozen=True) +class LinkageSnapshot: + """One answered linkage query over a scoped window of a Gitea repo.""" + + ok: bool + project_id: str + repo_label: str + host: str + state_scope: str + issues: tuple[LinkageNode, ...] = () + prs: tuple[LinkageNode, ...] = () + contested_issues: tuple[int, ...] = () + focus: tuple[str, int] | None = None + inventory_complete: bool = False + deep_links_enabled: bool = False + handoff_status: HandoffStatus = HandoffStatus(HANDOFF_NOT_LOADED) + fetch_error: str | None = None + + @property + def orphan_prs(self) -> tuple[LinkageNode, ...]: + """PRs in the loaded window that point at no issue at all.""" + return tuple(node for node in self.prs if not node.links) + + def to_dict(self) -> dict[str, Any]: + return { + "ok": self.ok, + "schema_version": LINKAGE_SCHEMA_VERSION, + "project_id": self.project_id, + "repo": self.repo_label, + "state_scope": self.state_scope, + "inventory_complete": self.inventory_complete, + "deep_links_enabled": self.deep_links_enabled, + "focus": ( + None + if self.focus is None + else {"kind": self.focus[0], "number": self.focus[1]} + ), + "handoff_source": self.handoff_status.to_dict(), + "fetch_error": self.fetch_error, + "contested_issues": list(self.contested_issues), + "issues": [node.to_dict() for node in self.issues], + "prs": [node.to_dict() for node in self.prs], + "evidence_kinds": [ + {"name": name, "description": EVIDENCE_DESCRIPTIONS[name]} + for name in EVIDENCE_ORDER + ], + } + + +def snapshot_to_dict(snapshot: LinkageSnapshot) -> dict[str, Any]: + """JSON-serializable export for ``/api/v1/gitea/linkage``.""" + return snapshot.to_dict() + + +def _offline_test_mode() -> bool: + return (os.environ.get("WEBUI_TEST_OFFLINE") or "").strip().lower() in { + "1", + "true", + "yes", + } + + +def _labels_of(item: dict[str, Any]) -> tuple[str, ...]: + return tuple( + str(_redact(label.get("name"))) + for label in (item.get("labels") or []) + if label.get("name") + ) + + +def _failed_snapshot( + *, + project_id: str, + repo_label: str, + host: str, + state_scope: str, + reason: str, +) -> LinkageSnapshot: + """A read that could not be answered. Never an empty-and-healthy snapshot.""" + return LinkageSnapshot( + ok=False, + project_id=project_id, + repo_label=repo_label, + host=host, + state_scope=state_scope, + inventory_complete=False, + deep_links_enabled=deep_links_enabled(), + handoff_status=HandoffStatus( + HANDOFF_UNAVAILABLE, reason="linkage inventory could not be loaded" + ), + fetch_error=str(_redact(reason)), + ) + + +def _resolve_project(project_id: str | None) -> ProjectRecord | None: + registry = load_registry() + if project_id: + for entry in registry.projects: + if entry.id == project_id: + return entry + return None + return registry.projects[0] if registry.projects else None + + +def _normalize_state(state: str | None) -> str: + text = (state or STATE_OPEN).strip().lower() + return text if text in _SUPPORTED_STATES else STATE_OPEN + + +def load_linkage_snapshot( + project_id: str | None = None, + *, + state: str | None = None, + issue: int | None = None, + pr: int | None = None, + fetch_prs: Callable[..., tuple[list[dict], PaginationMeta]] | None = None, + fetch_issues: Callable[..., tuple[list[dict], PaginationMeta]] | None = None, + comment_source: CommentSource | None = None, +) -> LinkageSnapshot: + """Load issue↔PR linkage for a registry project. + + ``issue``/``pr`` focus one thread: the focused row is the only one whose + Canonical Thread Handoff comments are fetched, because handoff comments are + thread-scoped and loading them for a whole queue would be one request per + row. Every unfocused row reports its handoff as ``not_loaded``. + """ + state_scope = _normalize_state(state) + try: + project = _resolve_project(project_id) + except Exception as exc: # registry invalid — fail closed with the reason + return _failed_snapshot( + project_id=project_id or "", + repo_label="", + host="", + state_scope=state_scope, + reason=f"project registry unavailable: {exc}", + ) + + if project is None: + return _failed_snapshot( + project_id=project_id or "", + repo_label="", + host="", + state_scope=state_scope, + reason=( + f"project {project_id!r} not found in registry" + if project_id + else "no projects registered" + ), + ) + + host = _host_from_url(project.remote_host) + repo_label = f"{project.gitea_owner}/{project.repo_name}" + offline_test = _offline_test_mode() + + def _empty_fetch(*_args, **_kwargs): + return [], PaginationMeta( + page=1, + per_page=50, + returned_count=0, + has_more=False, + is_final_page=True, + # An offline stub loaded nothing; claiming a complete inventory here + # would let the page assert that no issue has a linked PR. + inventory_complete=False, + pages_fetched=0, + ) + + pr_fetch = fetch_prs or (_empty_fetch if offline_test else _fetch_prs) + issue_fetch = fetch_issues or (_empty_fetch if offline_test else _fetch_issues) + using_live_fetch = not offline_test and (fetch_prs is None or fetch_issues is None) + auth = get_auth_header(host) if using_live_fetch else "test-auth" + if using_live_fetch and not auth: + return _failed_snapshot( + project_id=project.id, + repo_label=repo_label, + host=host, + state_scope=state_scope, + reason=( + f"Gitea credentials unavailable for {host}; linkage cannot be " + "loaded (fail closed — not rendering an empty linkage table)" + ), + ) + + try: + raw_prs, pr_pagination = pr_fetch( + host, project.gitea_owner, project.repo_name, auth, state=state_scope + ) + raw_issues, issue_pagination = issue_fetch( + host, project.gitea_owner, project.repo_name, auth, state=state_scope + ) + except Exception as exc: # noqa: BLE001 — operator-visible fetch failure + return _failed_snapshot( + project_id=project.id, + repo_label=repo_label, + host=host, + state_scope=state_scope, + reason=f"Gitea fetch failed: {exc}", + ) + + inventory_complete = bool( + getattr(pr_pagination, "inventory_complete", False) + and getattr(issue_pagination, "inventory_complete", False) + ) + + index = resolve_linkage(raw_prs) + contested = index.contested_issues() + + focus: tuple[str, int] | None = None + if pr is not None: + focus = ("pr", int(pr)) + elif issue is not None: + focus = ("issue", int(issue)) + + unfocused_reason = ( + "canonical handoff comments are thread-scoped; focus one issue or PR " + "to load its latest handoff" + ) + handoff_status = HandoffStatus(HANDOFF_NOT_LOADED, reason=unfocused_reason) + focus_handoff: HandoffSummary | None = None + if focus is not None: + source = comment_source + if source is None and not offline_test: + source = build_comment_source(host, project.gitea_owner, project.repo_name) + target = f"{focus[0]}#{focus[1]}" + if source is None: + handoff_status = HandoffStatus( + HANDOFF_UNAVAILABLE, + reason="no authenticated comment source available for this read", + target=target, + ) + else: + try: + focus_handoff = summarize_handoff(source(focus[0], focus[1]) or []) + handoff_status = HandoffStatus(HANDOFF_LOADED, target=target) + except Exception as exc: # fail soft: degrade this source only + handoff_status = HandoffStatus( + HANDOFF_UNAVAILABLE, + reason=str(_redact(f"handoff fetch failed: {exc}")), + target=target, + ) + + def _node_handoff( + kind: str, number: int + ) -> tuple[HandoffSummary | None, HandoffStatus]: + """Attach the handoff only to the focused row; qualify every other row.""" + if focus == (kind, number): + return (focus_handoff, handoff_status) + return ( + None, + HandoffStatus( + HANDOFF_NOT_LOADED, + reason=unfocused_reason if focus is None else "not the focused thread", + ), + ) + + def _number_of(raw: dict[str, Any]) -> int | None: + try: + return int(raw["number"]) + except (KeyError, TypeError, ValueError): + return None + + def _sort_key(raw: dict[str, Any]) -> int: + number = _number_of(raw) + return -1 if number is None else number + + issue_nodes: list[LinkageNode] = [] + for raw in sorted(raw_issues or [], key=_sort_key, reverse=True): + number = _number_of(raw) + if number is None: + continue + node_handoff, node_status = _node_handoff("issue", number) + linked_prs = index.issue_prs.get(number, ()) + issue_nodes.append( + LinkageNode( + kind="issue", + number=number, + title=str(_redact(raw.get("title")) or ""), + state=str(raw.get("state") or ""), + labels=_labels_of(raw), + linked_prs=linked_prs, + contested=len(linked_prs) > 1, + deep_link=_deep_link( + host, project.gitea_owner, project.repo_name, "issue", number + ), + handoff=node_handoff, + handoff_status=node_status, + links_authoritative=inventory_complete, + ) + ) + + pr_nodes: list[LinkageNode] = [] + for raw in sorted(raw_prs or [], key=_sort_key, reverse=True): + number = _number_of(raw) + if number is None: + continue + node_handoff, node_status = _node_handoff("pr", number) + links = index.pr_links.get(number, ()) + pr_nodes.append( + LinkageNode( + kind="pr", + number=number, + title=str(_redact(raw.get("title")) or ""), + state=str(raw.get("state") or ""), + labels=_labels_of(raw), + links=links, + ambiguous=index.ambiguous(number), + contested=any(link.issue_number in contested for link in links), + deep_link=_deep_link( + host, project.gitea_owner, project.repo_name, "pr", number + ), + handoff=node_handoff, + handoff_status=node_status, + links_authoritative=inventory_complete, + ) + ) + + return LinkageSnapshot( + ok=True, + project_id=project.id, + repo_label=repo_label, + host=host, + state_scope=state_scope, + issues=tuple(issue_nodes), + prs=tuple(pr_nodes), + contested_issues=contested, + focus=focus, + inventory_complete=inventory_complete, + deep_links_enabled=deep_links_enabled(), + handoff_status=handoff_status, + ) diff --git a/webui/linkage_views.py b/webui/linkage_views.py new file mode 100644 index 0000000..fcc06fa --- /dev/null +++ b/webui/linkage_views.py @@ -0,0 +1,364 @@ +"""HTML views for the Gitea issue↔PR linkage console (#645, Phase 3). + +Read-only renderer over :mod:`webui.linkage_loader`. The page's job is to make +three things impossible to misread: + +* **why** an edge exists — every link carries its evidence badge, so a bare + ``#N`` mention never looks like a closing claim; +* **what was not loaded** — a partial inventory, an unfocused thread, or an + unavailable handoff source renders as an explicit qualifier, never as an + affirmative "none"; +* **that nothing here mutates** — there is no review, merge, or edit control, + and the deep link out to Gitea appears only under the admin reveal opt-in. +""" + +from __future__ import annotations + +from html import escape +from typing import Sequence + +from webui.layout import render_page +from webui.linkage_loader import ( + EVIDENCE_BRANCH, + EVIDENCE_CLOSES, + EVIDENCE_DESCRIPTIONS, + EVIDENCE_ORDER, + EVIDENCE_REFERENCE, + HANDOFF_LOADED, + HANDOFF_NOT_LOADED, + HandoffSummary, + LinkageNode, + LinkageSnapshot, +) + +_EVIDENCE_CSS = { + EVIDENCE_CLOSES: "badge-health-ok", + EVIDENCE_BRANCH: "badge-health-skipped", + EVIDENCE_REFERENCE: "badge-health-unproven", +} + +_EVIDENCE_LABEL = { + EVIDENCE_CLOSES: "closes", + EVIDENCE_BRANCH: "branch", + EVIDENCE_REFERENCE: "mention", +} + + +def _badge(text: str, css: str) -> str: + return f'{escape(text)}' + + +def _labels(names: Sequence[str]) -> str: + if not names: + return '—' + return " ".join(_badge(name, "badge-health-skipped") for name in names) + + +def _ref(node: LinkageNode) -> str: + """Render an item reference, hyperlinked only when deep links are revealed.""" + label = f"#{node.number}" + if node.deep_link: + return f'{escape(label)}' + return f"{escape(label)}" + + +def _scope_card(snapshot: LinkageSnapshot) -> str: + focus = ( + "none" + if snapshot.focus is None + else f"{snapshot.focus[0]}#{snapshot.focus[1]}" + ) + completeness = ( + _badge("complete", "badge-health-ok") + if snapshot.inventory_complete + else _badge("partial", "badge-health-degraded") + ) + links_note = ( + "Every linkage edge below is a claim about this loaded window only." + if snapshot.inventory_complete + else ( + "Pagination did not complete for this window, so an empty link list " + "means none found in what was loaded — not that no link exists." + ) + ) + deep_links = ( + _badge("enabled", "badge-health-ok") + if snapshot.deep_links_enabled + else _badge("hidden", "badge-health-skipped") + ) + return f"""
    +

    Scope

    + + + + + + + +
    Project{escape(snapshot.project_id or "—")}
    Repository{escape(snapshot.repo_label or "—")}
    Item state{escape(snapshot.state_scope)}
    Focused thread{escape(focus)}
    Inventory{completeness}
    Gitea deep links{deep_links}
    +

    {links_note}

    +
    """ + + +def _error_card(snapshot: LinkageSnapshot) -> str: + if snapshot.ok and not snapshot.fetch_error: + return "" + return ( + '
    Linkage unavailable: ' + f"{escape(snapshot.fetch_error or 'the linkage read did not complete')}. " + "No linkage table is rendered: an empty table would read as " + "no issue is linked to any PR, which this read cannot claim." + "
    " + ) + + +def _contested_card(snapshot: LinkageSnapshot) -> str: + if not snapshot.contested_issues: + return "" + refs = ", ".join(f"#{number}" for number in snapshot.contested_issues) + return ( + '
    ' + f"Contested issues: {refs}. More than one PR in this " + "window claims each of these — duplicate work or a superseded PR. " + "Resolution stays in Gitea and the workflow; this console only reports it." + "
    " + ) + + +def _evidence_badges(evidence: Sequence[str]) -> str: + return " ".join( + _badge( + _EVIDENCE_LABEL.get(name, name), + _EVIDENCE_CSS.get(name, "badge-health-skipped"), + ) + for name in EVIDENCE_ORDER + if name in evidence + ) + + +def _handoff_inline(handoff: HandoffSummary) -> str: + type_css = "badge-health-ok" if handoff.cth_type_known else "badge-health-degraded" + return ( + f'{_badge(handoff.cth_type or "—", type_css)}' + f'
    ' + f'{escape(handoff.status or "—")} → {escape(handoff.next_owner or "—")}
    ' + ) + + +def _handoff_cell(node: LinkageNode) -> str: + """Render the handoff column, distinguishing 'none found' from 'not loaded'.""" + status = node.handoff_status + if status.state == HANDOFF_LOADED: + if node.handoff is None: + return 'no canonical handoff on this thread' + return _handoff_inline(node.handoff) + if status.state == HANDOFF_NOT_LOADED: + return ( + f'{_badge("not loaded", "badge-health-skipped")}' + f'
    ' + f'{escape(status.reason or "")}
    ' + ) + return ( + f'{_badge("unavailable", "badge-health-degraded")}' + f'
    ' + f'{escape(status.reason or "")}
    ' + ) + + +def _issue_rows(snapshot: LinkageSnapshot) -> str: + rows = [] + for node in snapshot.issues: + if node.linked_prs: + linked = ", ".join(f"#{number}" for number in node.linked_prs) + if node.contested: + linked += " " + _badge("contested", "badge-blocked") + elif node.links_authoritative: + linked = 'none' + else: + # The distinction an operator needs: nothing found in a window that + # was not fully loaded is not the same as nothing existing. + linked = 'none found (partial inventory)' + rows.append( + "" + f"{_ref(node)}" + f"{escape(node.title)}" + f"{escape(node.state or '—')}" + f"{_labels(node.labels)}" + f"{linked}" + f"{_handoff_cell(node)}" + "" + ) + if not rows: + return 'No issues in the loaded window.' + return "".join(rows) + + +def _pr_rows(snapshot: LinkageSnapshot) -> str: + rows = [] + for node in snapshot.prs: + if node.links: + linked = "".join( + f"
    #{link.issue_number} " + f"{_evidence_badges(link.evidence)}
    " + for link in node.links + ) + if node.ambiguous: + linked += _badge("ambiguous", "badge-blocked") + if node.contested: + linked += " " + _badge("contested", "badge-blocked") + elif node.links_authoritative: + linked = 'no issue reference' + else: + linked = 'none found (partial inventory)' + rows.append( + "" + f"{_ref(node)}" + f"{escape(node.title)}" + f"{escape(node.state or '—')}" + f"{_labels(node.labels)}" + f"{linked}" + f"{_handoff_cell(node)}" + "" + ) + if not rows: + return ( + 'No pull requests in the loaded ' + "window." + ) + return "".join(rows) + + +def _focus_card(snapshot: LinkageSnapshot) -> str: + """Render the focused thread's latest canonical handoff, when one was loaded.""" + if snapshot.focus is None: + return f"""
    +

    Canonical handoff

    +

    {escape(snapshot.handoff_status.reason or "")} + Add ?issue=N or ?pr=N to load the latest + Canonical Thread Handoff for one thread.

    +
    """ + + kind, number = snapshot.focus + target = f"{kind} #{number}" + if not snapshot.handoff_status.loaded: + return f"""
    +

    Canonical handoff — {escape(target)}

    +

    {_badge("unavailable", "badge-health-degraded")} + {escape(snapshot.handoff_status.reason or "handoff source did not run")}. + This is not evidence that the thread carries no handoff.

    +
    """ + + handoff = next( + ( + node.handoff + for node in (snapshot.issues + snapshot.prs) + if node.kind == kind and node.number == number and node.handoff + ), + None, + ) + if handoff is None: + return f"""
    +

    Canonical handoff — {escape(target)}

    +

    Comments loaded; no Canonical Thread Handoff comment found on + this thread.

    +
    """ + + type_css = "badge-health-ok" if handoff.cth_type_known else "badge-health-degraded" + unknown_note = ( + "" + if handoff.cth_type_known + else ( + '

    The comment\'s heading is not a declared CTH type, ' + "so it is reported as unrecognized rather than republished.

    " + ) + ) + return f"""
    +

    Canonical handoff — {escape(target)}

    +

    {_badge(handoff.cth_type or "—", type_css)} + by {escape(handoff.author or "unknown")} + at {escape(handoff.created_at or "unknown")}

    + {unknown_note} + + + + + + +
    Status{escape(handoff.status or "—")}
    Next owner{escape(handoff.next_owner or "—")}
    Current blocker{escape(handoff.current_blocker or "—")}
    Decision{escape(handoff.decision or "—")}
    Next action{escape(handoff.next_action or "—")}
    +

    Full event history: + /api/v1/timeline

    +
    """ + + +def _legend_card(snapshot: LinkageSnapshot) -> str: + items = "".join( + f"
  • {_badge(_EVIDENCE_LABEL[name], _EVIDENCE_CSS[name])} — " + f"{escape(EVIDENCE_DESCRIPTIONS[name])}
    • " + for name in EVIDENCE_ORDER + ) + reveal_note = ( + "Gitea deep links are shown because the " + "GITEA_MCP_REVEAL_ENDPOINTS admin opt-in is set." + if snapshot.deep_links_enabled + else ( + "Gitea deep links are withheld. Set " + "GITEA_MCP_REVEAL_ENDPOINTS=1 server-side to reveal " + "them; item numbers stay usable without them." + ) + ) + return f"""
    +

    How an edge was found

    +
      {items}
    +

    {reveal_note}

    +

    Read-only surface: no issue or PR editing, no review, and no merge. + JSON export: /api/v1/gitea/linkage

    +
    """ + + +def render_linkage_page(snapshot: LinkageSnapshot) -> str: + """Render the full HTML page for the Gitea linkage console.""" + if not snapshot.ok: + return render_page( + title="Gitea linkage", + body_html=f"""

    Gitea issue and PR linkage

    +

    Phase 3 read-only linkage console (#645).

    +{_error_card(snapshot)} +{_scope_card(snapshot)}""", + ) + + body = f"""

    Gitea issue and PR linkage

    +

    Phase 3 read-only linkage console (#645). Gitea remains the source of +truth; this page reads it and never writes to it.

    +{_scope_card(snapshot)} +{_contested_card(snapshot)} + +
    +

    Issues → pull requests

    + + + + + + + + {_issue_rows(snapshot)} +
    IssueTitleStateLabelsLinked PRsLatest handoff
    +
    + +
    +

    Pull requests → issues

    + + + + + + + + {_pr_rows(snapshot)} +
    PRTitleStateLabelsLinked issuesLatest handoff
    +
    + +{_focus_card(snapshot)} +{_legend_card(snapshot)} +""" + return render_page(title="Gitea linkage", body_html=body) diff --git a/webui/nav.py b/webui/nav.py index da9f763..6377d49 100644 --- a/webui/nav.py +++ b/webui/nav.py @@ -6,7 +6,8 @@ destination is a GET view or a Phase 1 placeholder. No mutation links. Nav groups follow the #631 Phase 1 information architecture: Health, Traffic, Runtime/Sessions, Projects, Inventory, Timeline, Policy (placeholder), and -Insights (placeholder). Later-phase surfaces are declared as ``stub`` items and +Insights (placeholder), joined by the Phase 3 Gitea linkage group (#645). +Later-phase surfaces are declared as ``stub`` items and backed by ``STUB_PAGES`` so their nav links resolve to a graceful placeholder instead of a 404. """ @@ -60,6 +61,9 @@ NAV_GROUPS: tuple[NavGroup, ...] = ( NavGroup("Timeline", ( NavItem("/timeline", "Timeline", "stub"), )), + NavGroup("Gitea", ( + NavItem("/gitea", "Issue/PR linkage"), + )), NavGroup("Policy", ( NavItem("/policy", "Policy", "stub"), NavItem("/prompts", "Prompts"), diff --git a/webui/queue_loader.py b/webui/queue_loader.py index 135ef39..21e8700 100644 --- a/webui/queue_loader.py +++ b/webui/queue_loader.py @@ -211,6 +211,19 @@ def _pagination_from_pages( ) +_SUPPORTED_FETCH_STATES = ("open", "closed", "all") + + +def _safe_state(state: str | None) -> str: + """Constrain a caller-supplied item state before it reaches a query string. + + The value is interpolated into the Gitea URL, so an unrecognised state falls + back to ``open`` rather than being passed through. + """ + text = (state or "open").strip().lower() + return text if text in _SUPPORTED_FETCH_STATES else "open" + + def _fetch_prs( host: str, org: str, @@ -218,8 +231,15 @@ def _fetch_prs( auth: str, *, per_page: int = 50, + state: str = "open", ) -> tuple[list[dict], PaginationMeta]: - url = f"{repo_api_url(host, org, repo)}/pulls?state=open" + """Fetch PRs in *state* (``open``, ``closed``, or ``all``). + + The queue dashboard only ever wants the open window, so ``open`` stays the + default. The linkage console (#645) widens it, because a landed issue↔PR + edge lives on a merged PR. + """ + url = f"{repo_api_url(host, org, repo)}/pulls?state={_safe_state(state)}" all_raw: list[dict] = [] pages_fetched = 0 is_final = False @@ -248,8 +268,13 @@ def _fetch_issues( auth: str, *, per_page: int = 50, + state: str = "open", ) -> tuple[list[dict], PaginationMeta]: - url = f"{repo_api_url(host, org, repo)}/issues?state=open&type=issues" + """Fetch issues in *state* (``open``, ``closed``, or ``all``); see :func:`_fetch_prs`.""" + url = ( + f"{repo_api_url(host, org, repo)}/issues" + f"?state={_safe_state(state)}&type=issues" + ) all_raw: list[dict] = [] page = 1 pages_fetched = 0 From 4f06d30e0758a6f8ef9fa345e86ab42f823a73e0 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 25 Jul 2026 16:41:16 -0400 Subject: [PATCH 05/12] feat(webui): implement notifications and human-attention routing (#648) --- docs/webui-notifications.md | 81 +++++ tests/test_webui_notifications.py | 276 +++++++++++++++++ webui/app.py | 25 ++ webui/nav.py | 1 + webui/notification_views.py | 158 ++++++++++ webui/notifications.py | 480 ++++++++++++++++++++++++++++++ 6 files changed, 1021 insertions(+) create mode 100644 docs/webui-notifications.md create mode 100644 tests/test_webui_notifications.py create mode 100644 webui/notification_views.py create mode 100644 webui/notifications.py diff --git a/docs/webui-notifications.md b/docs/webui-notifications.md new file mode 100644 index 0000000..7ef2e05 --- /dev/null +++ b/docs/webui-notifications.md @@ -0,0 +1,81 @@ +# Web Console: Notifications & Human-Attention Routing (#648) + +- **Status:** Phase 3 Live +- **Tracking Issue:** [#648](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/648) +- **Parent Epic:** [#631](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/631) +- **Attention Boundary Reference:** [#628](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/628) + +--- + +## 1. Overview + +The **Notifications & Human-Attention Console** (`/notifications`, `/api/v1/notifications`) provides intelligent event classification and human-attention routing for autonomous workflow operations. + +To prevent alert fatigue while ensuring critical escalation boundaries are never missed, events are classified into three distinct **Attention Classes**: + +1. **`human-required`** (Urgent Escalation Boundary): + - Items requiring immediate human intervention or business decisions. + - Triggers: Auth failures, hard stops, irrecoverable state, decision locks, failed report validations, critical probe errors. + - Display: Highlighted in red (`badge-blocked`) with a `HUMAN REQUIRED` badge. + +2. **`operator`** (Operational Inbox): + - Items requiring controller or operator review/triage during routine execution. + - Triggers: Blocked PRs (merge conflicts), stale leases, duplicate PRs on issues, unassigned ready work. + - Display: Displayed in orange/yellow (`badge-claimed`). + +3. **`routine`** (Background Workflow Transitions): + - Normal, healthy workflow transitions and state progressions. + - Triggers: Active PRs/issues in standard state, clean branch creation, routine heartbeats. + - Display: Filtered out of default inbox views to eliminate notification spam; viewable on demand via the "Routine" or "All" tab. + +--- + +## 2. API Endpoints + +### `GET /api/v1/notifications` +*Compatibility Alias:* `GET /api/notifications` + +#### Query Parameters: +- `project_id` (optional): Filter notifications by project ID. +- `attention_class` (optional): `inbox` (default: human-required + operator), `human-required`, `operator`, `routine`, `all`. + +#### Example JSON Response: +```json +{ + "project_id": "gitea-tools", + "repo_label": "Scaled-Tech-Consulting/Gitea-Tools", + "human_required_count": 0, + "operator_count": 2, + "routine_count": 5, + "total_count": 7, + "fetch_error": null, + "inbox_items": [ + { + "id": "notif-pr-block-742", + "attention_class": "operator", + "category": "blocker", + "title": "Blocked PR #742", + "summary": "PR #742 requires merge conflict resolution.", + "work_kind": "pr", + "work_number": 742, + "project_id": "gitea-tools", + "repo_label": "Scaled-Tech-Consulting/Gitea-Tools", + "created_at": "2026-07-25T16:39:47Z", + "deep_link": "/traffic", + "requires_human": false, + "extra": {} + } + ], + "all_items": [...] +} +``` + +--- + +## 3. UI Navigation + +- Access via the **Traffic** navigation menu: **Traffic → Notifications**. +- The main view displays: + - **Metrics Summary Bar**: Highlighting counts for Human Required, Operator Inbox, and Routine items. + - **Attention Filter Tabs**: Toggle between Inbox (Human + Operator), Human Required, Operator, Routine, and All. + - **Structured Event Table**: Displays category, title, summary, work item links, and timestamps. diff --git a/tests/test_webui_notifications.py b/tests/test_webui_notifications.py new file mode 100644 index 0000000..db89d4c --- /dev/null +++ b/tests/test_webui_notifications.py @@ -0,0 +1,276 @@ +"""Unit tests for Phase 3 Notifications and Human-Attention Console (#648).""" + +from __future__ import annotations + +import pytest +from starlette.testclient import TestClient + +from webui.app import create_app +from webui.notifications import ( + ATTENTION_HUMAN_REQUIRED, + ATTENTION_OPERATOR, + ATTENTION_ROUTINE, + CATEGORY_AUTH, + CATEGORY_BLOCKER, + CATEGORY_LEASE, + CATEGORY_SYSTEM, + CATEGORY_VALIDATION, + CATEGORY_WORKFLOW, + NotificationItem, + NotificationSnapshot, + classify_attention_event, + load_notifications_snapshot, + snapshot_to_dict, +) +from webui.notification_views import render_notifications_page +from webui.project_registry import load_registry +from webui.queue_loader import QueueItem, QueueSnapshot +from webui.lease_loader import CollisionWarning, LeaseSnapshot +from webui.system_health import DependencyProbe, SystemHealthSnapshot, VersionInfo, StaleRuntime + + +def test_classify_attention_event_rules(): + # 1. Critical escalation boundaries -> human-required + att_cls, req_human = classify_attention_event( + CATEGORY_AUTH, "Auth error", "Unauthorized access attempt", is_auth_failure=True + ) + assert att_cls == ATTENTION_HUMAN_REQUIRED + assert req_human is True + + att_cls, req_human = classify_attention_event( + CATEGORY_SYSTEM, "Hard stop", "Hard stop triggered", is_hard_stop=True + ) + assert att_cls == ATTENTION_HUMAN_REQUIRED + assert req_human is True + + att_cls, req_human = classify_attention_event( + CATEGORY_VALIDATION, "Validation Error", "Report validation failed", is_validation_failure=True + ) + assert att_cls == ATTENTION_HUMAN_REQUIRED + assert req_human is True + + # 2. Operational issues -> operator + att_cls, req_human = classify_attention_event( + CATEGORY_BLOCKER, "PR Blocked", "Merge conflict detected", is_blocker=True + ) + assert att_cls == ATTENTION_OPERATOR + assert req_human is False + + att_cls, req_human = classify_attention_event( + CATEGORY_LEASE, "Lease Expired", "Session lease expired", is_stale=True + ) + assert att_cls == ATTENTION_OPERATOR + assert req_human is False + + # 3. Routine workflow transitions -> routine + att_cls, req_human = classify_attention_event( + CATEGORY_WORKFLOW, "PR Active", "PR in review" + ) + assert att_cls == ATTENTION_ROUTINE + assert req_human is False + + +def test_notification_snapshot_aggregation(): + reg = load_registry() + proj_id = reg.projects[0].id if reg.projects else "gitea-tools" + + mock_queue = QueueSnapshot( + project_id=proj_id, + repo_label="org/repo", + prs=( + QueueItem( + number=101, + title="Blocked PR", + badges=("blocked",), + extra={}, + ), + QueueItem( + number=102, + title="Normal PR", + badges=("in-review",), + extra={}, + ), + ), + issues=(), + pr_pagination=None, + issue_pagination=None, + ) + + mock_leases = LeaseSnapshot( + project_id=proj_id, + repo_label="org/repo", + issue_lock=None, + claim_inventory={}, + reviewer_leases=( + { + "pr_number": 101, + "status": "expired", + "is_expired": True, + }, + ), + duplicate_prs=( + CollisionWarning( + kind="duplicate_pr", + message="Multiple open PRs for issue #101", + issue_number=101, + pr_numbers=(101, 103), + ), + ), + duplicate_branches=(), + collision_history=(), + fetch_error=None, + ) + + mock_version = VersionInfo( + git_sha="abc1234", + git_describe="v1.0.0", + control_plane_schema_version=1, + python_version="3.11", + known=True, + ) + + mock_stale = StaleRuntime( + daemon_head="abc1234", + checkout_head="abc1234", + remote_head="abc1234", + stale=False, + determinable=True, + mutation_safe=True, + reasons=(), + ) + + mock_health = SystemHealthSnapshot( + status="degraded", + ready=False, + readiness_complete=True, + readiness_reasons=("Auth failure",), + service="webui", + mode="test", + version=mock_version, + started_at="2026-07-25T00:00:00Z", + uptime_seconds=100.0, + timestamp="2026-07-25T00:00:00Z", + deep_probes_requested=True, + dependencies=( + DependencyProbe( + name="auth_service", + kind="auth", + status="unauthorized", + detail="Token expired", + required=True, + ), + ), + mcp_namespaces=(), + stale_runtime=mock_stale, + probe_errors=(), + ) + + snapshot = load_notifications_snapshot( + proj_id, + load_queue=lambda _id: mock_queue, + load_leases=lambda **_kwargs: mock_leases, + load_health=lambda **_kwargs: mock_health, + ) + + assert snapshot.project_id == proj_id + assert snapshot.total_count == 5 + assert snapshot.human_required_count >= 1 # auth probe failure + assert snapshot.operator_count >= 3 # blocked PR + expired lease + duplicate PR collision + assert snapshot.routine_count >= 1 # normal PR + + # Inbox items should include operator and human-required items only + inbox_classes = {item.attention_class for item in snapshot.inbox_items} + assert ATTENTION_ROUTINE not in inbox_classes + assert ATTENTION_OPERATOR in inbox_classes + assert ATTENTION_HUMAN_REQUIRED in inbox_classes + + +def test_snapshot_to_dict_and_redaction(): + item = NotificationItem( + id="notif-1", + attention_class=ATTENTION_HUMAN_REQUIRED, + category=CATEGORY_AUTH, + title="Auth Error", + summary="Failed auth header: Bearer secret_token_12345", + work_kind="system", + work_number=None, + project_id="test-proj", + repo_label="org/repo", + created_at="2026-07-25T16:00:00Z", + requires_human=True, + ) + snap = NotificationSnapshot( + project_id="test-proj", + repo_label="org/repo", + items=(item,), + human_required_count=1, + operator_count=0, + routine_count=0, + total_count=1, + ) + + data = snapshot_to_dict(snap) + assert data["project_id"] == "test-proj" + assert data["human_required_count"] == 1 + assert len(data["inbox_items"]) == 1 + + # Redaction test + summary = data["inbox_items"][0]["summary"] + assert "secret_token_12345" not in summary + assert "" in summary or "Bearer" in summary + + +def test_notifications_html_views(): + item = NotificationItem( + id="notif-1", + attention_class=ATTENTION_HUMAN_REQUIRED, + category=CATEGORY_AUTH, + title="Critical Auth Failure", + summary="Auth failure details", + work_kind="issue", + work_number=42, + project_id="test-proj", + repo_label="org/repo", + created_at="2026-07-25T16:00:00Z", + requires_human=True, + ) + snap = NotificationSnapshot( + project_id="test-proj", + repo_label="org/repo", + items=(item,), + human_required_count=1, + operator_count=0, + routine_count=0, + total_count=1, + ) + + html = render_notifications_page(snap, filter_class="inbox") + assert "Notifications & Attention Inbox" in html or "Notifications & Attention Inbox" in html + assert "Critical Auth Failure" in html + assert "HUMAN REQUIRED" in html + assert "Human Required" in html + + +def test_notifications_app_routes(): + app = create_app() + client = TestClient(app) + + # 1. HTML Route + res = client.get("/notifications") + assert res.status_code == 200 + assert "Notifications" in res.text + assert "Attention Inbox" in res.text + + # 2. API Route /api/v1/notifications + res_api = client.get("/api/v1/notifications") + assert res_api.status_code == 200 + json_data = res_api.json() + assert "human_required_count" in json_data + assert "operator_count" in json_data + assert "routine_count" in json_data + assert "inbox_items" in json_data + + # 3. Compatibility Alias /api/notifications + res_alias = client.get("/api/notifications") + assert res_alias.status_code == 200 + assert res_alias.json()["project_id"] == json_data["project_id"] diff --git a/webui/app.py b/webui/app.py index 560ec75..3840874 100644 --- a/webui/app.py +++ b/webui/app.py @@ -72,6 +72,11 @@ from webui.system_health import ( snapshot_to_dict as system_health_to_dict, ) from webui.system_health_views import render_system_health_page +from webui.notifications import ( + load_notifications_snapshot, + snapshot_to_dict as notifications_snapshot_to_dict, +) +from webui.notification_views import render_notifications_page _READ_ONLY_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) _AUDIT_MUTATION_PATHS = frozenset({"/audit", "/api/audit"}) @@ -739,6 +744,23 @@ async def api_v1_analytics_ingest(request: Request) -> JSONResponse: ) +async def notifications_route(request: Request) -> HTMLResponse: + project_id = request.query_params.get("project_id") + attention_class = request.query_params.get("attention_class") or "inbox" + snap = load_notifications_snapshot(project_id) + html = render_notifications_page( + snap, filter_class=attention_class, filter_project=project_id + ) + return HTMLResponse(html) + + +async def api_notifications(request: Request) -> JSONResponse: + project_id = request.query_params.get("project_id") + snap = load_notifications_snapshot(project_id) + data = notifications_snapshot_to_dict(snap) + return JSONResponse(data) + + async def method_not_allowed(request: Request, _exc: Exception) -> Response: path = request.url.path if path in _AUDIT_MUTATION_PATHS and request.method == "POST": @@ -767,6 +789,9 @@ def create_app(*, bind_host: str | None = None) -> Starlette: Route("/api/queue", api_queue, methods=["GET"]), Route("/traffic", traffic, methods=["GET"]), Route("/api/traffic", api_traffic, methods=["GET"]), + Route("/notifications", notifications_route, methods=["GET"]), + Route("/api/notifications", api_notifications, methods=["GET"]), + Route("/api/v1/notifications", api_notifications, methods=["GET"]), Route("/projects", projects, methods=["GET"]), Route("/projects/{project_id}", project_detail, methods=["GET"]), Route("/api/projects", api_projects, methods=["GET"]), diff --git a/webui/nav.py b/webui/nav.py index da9f763..b9165e9 100644 --- a/webui/nav.py +++ b/webui/nav.py @@ -45,6 +45,7 @@ NAV_GROUPS: tuple[NavGroup, ...] = ( NavItem("/queue", "Queue"), NavItem("/leases", "Leases"), NavItem("/actions", "Actions"), + NavItem("/notifications", "Notifications"), )), NavGroup("Runtime/Sessions", ( NavItem("/runtime", "Runtime health"), diff --git a/webui/notification_views.py b/webui/notification_views.py new file mode 100644 index 0000000..e4b6ab9 --- /dev/null +++ b/webui/notification_views.py @@ -0,0 +1,158 @@ +"""HTML rendering for Phase 3 Notifications and Human-Attention Console (#648).""" + +from __future__ import annotations + +from html import escape +from typing import Sequence + +from webui.layout import render_page +from webui.notifications import ( + ATTENTION_HUMAN_REQUIRED, + ATTENTION_OPERATOR, + ATTENTION_ROUTINE, + NotificationItem, + NotificationSnapshot, +) + + +def _render_attention_badge(attention_class: str) -> str: + cls = "badge" + if attention_class == ATTENTION_HUMAN_REQUIRED: + cls += " badge-blocked" + elif attention_class == ATTENTION_OPERATOR: + cls += " badge-claimed" + else: + cls += " muted" + return f'{escape(attention_class)}' + + +def _render_notification_row(item: NotificationItem) -> str: + category_label = escape(item.category.upper()) + id_str = escape(item.id) + title_str = escape(item.title) + summary_str = escape(item.summary) + att_badge = _render_attention_badge(item.attention_class) + + work_item_html = "—" + if item.work_number and item.work_kind: + kind_label = escape(item.work_kind.upper()) + num_str = f"#{item.work_number}" + link = item.deep_link or "#" + work_item_html = f'{kind_label} {num_str}' + + requires_human_label = ( + 'HUMAN REQUIRED' + if item.requires_human + else "" + ) + + return f""" + {category_label}
    {id_str} + +
    {title_str} {att_badge} {requires_human_label}
    +
    {summary_str}
    + + {work_item_html} + {escape(item.created_at[:19])} +""" + + +def _render_notifications_table(items: Sequence[NotificationItem], empty_message: str) -> str: + if not items: + return f'

    {escape(empty_message)}

    ' + + rows = "".join(_render_notification_row(item) for item in items) + return f""" + + + + + + + + + + {rows} + +
    Category & IDTitle & Attention SummaryWork ItemTime
    """ + + +def render_notifications_page( + snapshot: NotificationSnapshot, + *, + filter_class: str = "inbox", + filter_project: str | None = None, +) -> str: + """Render the notifications and attention inbox page.""" + title = "Notifications & Attention Inbox" + + err_html = "" + if snapshot.fetch_error: + err_html = f'

    Fetch Warning: {escape(snapshot.fetch_error)}

    ' + + # Determine items to render based on filter_class + if filter_class == ATTENTION_HUMAN_REQUIRED: + display_items = snapshot.human_required_items + active_tab_title = "Human-Required Escalations" + elif filter_class == ATTENTION_OPERATOR: + display_items = snapshot.operator_items + active_tab_title = "Operator Inbox Items" + elif filter_class == ATTENTION_ROUTINE: + display_items = snapshot.routine_items + active_tab_title = "Routine Workflow Transitions" + elif filter_class == "all": + display_items = snapshot.items + active_tab_title = "All Events (including Routine)" + else: # "inbox" default + display_items = snapshot.inbox_items + active_tab_title = "Attention Inbox (Human + Operator)" + + hr_cls = "badge-blocked" if snapshot.human_required_count > 0 else "muted" + op_cls = "badge-claimed" if snapshot.operator_count > 0 else "muted" + + metrics_html = f"""
    +
    + Human Required +

    {snapshot.human_required_count}

    +

    Critical escalation boundary

    +
    +
    + Operator Inbox +

    {snapshot.operator_count}

    +

    Operational items needing review

    +
    +
    + Routine Transitions +

    {snapshot.routine_count}

    +

    Background transitions (filtered)

    +
    +
    """ + + # Filter navigation links + def _tab_link(target_class: str, label: str) -> str: + is_active = (filter_class == target_class) + style = "font-weight:bold; border-bottom:2px solid currentColor;" if is_active else "color:#4a5568;" + return f'{label}' + + tabs_html = f"""
    + {_tab_link("inbox", f"Attention Inbox ({snapshot.human_required_count + snapshot.operator_count})")} + {_tab_link("human-required", f"Human Required ({snapshot.human_required_count})")} + {_tab_link("operator", f"Operator ({snapshot.operator_count})")} + {_tab_link("routine", f"Routine ({snapshot.routine_count})")} + {_tab_link("all", f"All Events ({snapshot.total_count})")} +
    """ + + table_html = _render_notifications_table( + display_items, + f"No items match attention filter '{filter_class}'.", + ) + + body = f"""

    {escape(title)}

    +

    Phase 3 console surface for human-attention routing (#648). Routine workflow transitions are filtered by default to eliminate notification fatigue.

    +{err_html} +{metrics_html} +{tabs_html} +

    {escape(active_tab_title)}

    +{table_html}""" + + return render_page(title=title, body_html=body) diff --git a/webui/notifications.py b/webui/notifications.py new file mode 100644 index 0000000..15753b9 --- /dev/null +++ b/webui/notifications.py @@ -0,0 +1,480 @@ +"""Notifications and human-attention routing module for Phase 3 web console (#648). + +Defines attention classes, event classification rules, and inbox aggregation so +operators receive direct alerts only for human-required escalation boundaries +(#628) while routine workflow transitions remain available for pull-based review. +""" + +from __future__ import annotations + +import os +from dataclasses import dataclass, field +from datetime import datetime, timezone +from typing import Any, Callable, Sequence + +from webui import console_redaction +from webui.project_registry import find_project, load_registry +from webui.queue_loader import QueueSnapshot, load_queue_snapshot +from webui.lease_loader import LeaseSnapshot, load_lease_snapshot +from webui.system_health import SystemHealthSnapshot, load_system_health + +# Attention class definitions (#628, #648) +ATTENTION_ROUTINE = "routine" +ATTENTION_OPERATOR = "operator" +ATTENTION_HUMAN_REQUIRED = "human-required" + +ATTENTION_CLASSES = ( + ATTENTION_ROUTINE, + ATTENTION_OPERATOR, + ATTENTION_HUMAN_REQUIRED, +) + +# Notification categories +CATEGORY_AUTH = "auth" +CATEGORY_BLOCKER = "blocker" +CATEGORY_LEASE = "lease" +CATEGORY_VALIDATION = "validation" +CATEGORY_WORKFLOW = "workflow" +CATEGORY_SYSTEM = "system" + +CATEGORIES = ( + CATEGORY_AUTH, + CATEGORY_BLOCKER, + CATEGORY_LEASE, + CATEGORY_VALIDATION, + CATEGORY_WORKFLOW, + CATEGORY_SYSTEM, +) + + +@dataclass(frozen=True) +class NotificationItem: + """A single notification or inbox event.""" + + id: str + attention_class: str # "routine", "operator", "human-required" + category: str # "auth", "blocker", "lease", "validation", etc. + title: str + summary: str + work_kind: str | None # "issue", "pr", "session", "system" + work_number: int | None + project_id: str + repo_label: str + created_at: str + deep_link: str | None = None + requires_human: bool = False + extra: dict[str, Any] = field(default_factory=dict) + + def as_dict(self) -> dict[str, Any]: + return { + "id": self.id, + "attention_class": self.attention_class, + "category": self.category, + "title": self.title, + "summary": console_redaction.redact_text(self.summary), + "work_kind": self.work_kind, + "work_number": self.work_number, + "project_id": self.project_id, + "repo_label": self.repo_label, + "created_at": self.created_at, + "deep_link": self.deep_link, + "requires_human": self.requires_human, + "extra": self.extra, + } + + +@dataclass(frozen=True) +class NotificationSnapshot: + """Snapshot of notifications and attention inbox state.""" + + project_id: str + repo_label: str + items: tuple[NotificationItem, ...] + human_required_count: int + operator_count: int + routine_count: int + total_count: int + fetch_error: str | None = None + + @property + def inbox_items(self) -> tuple[NotificationItem, ...]: + """Items requiring operator or human attention (excluding routine).""" + return tuple( + item + for item in self.items + if item.attention_class in {ATTENTION_OPERATOR, ATTENTION_HUMAN_REQUIRED} + ) + + @property + def human_required_items(self) -> tuple[NotificationItem, ...]: + return tuple( + item for item in self.items if item.attention_class == ATTENTION_HUMAN_REQUIRED + ) + + @property + def operator_items(self) -> tuple[NotificationItem, ...]: + return tuple( + item for item in self.items if item.attention_class == ATTENTION_OPERATOR + ) + + @property + def routine_items(self) -> tuple[NotificationItem, ...]: + return tuple( + item for item in self.items if item.attention_class == ATTENTION_ROUTINE + ) + + def as_dict(self) -> dict[str, Any]: + return { + "project_id": self.project_id, + "repo_label": self.repo_label, + "human_required_count": self.human_required_count, + "operator_count": self.operator_count, + "routine_count": self.routine_count, + "total_count": self.total_count, + "fetch_error": self.fetch_error, + "inbox_items": [item.as_dict() for item in self.inbox_items], + "all_items": [item.as_dict() for item in self.items], + } + + +def classify_attention_event( + category: str, + title: str, + summary: str, + *, + is_hard_stop: bool = False, + is_auth_failure: bool = False, + is_irrecoverable: bool = False, + is_decision_lock: bool = False, + is_validation_failure: bool = False, + is_stale: bool = False, + is_blocker: bool = False, +) -> tuple[str, bool]: + """Classify an event into an attention class and human requirement flag. + + Rules (#628, #648): + 1. Critical boundaries (hard stop, auth failure, irrecoverable state, + decision lock, validation failure) -> ATTENTION_HUMAN_REQUIRED (requires_human=True). + 2. Operational queues (blocker, stale lease, unassigned ready work, queue collision) + -> ATTENTION_OPERATOR (requires_human=False). + 3. Routine state transitions (clean progression, healthy heartbeats) -> ATTENTION_ROUTINE (requires_human=False). + """ + if ( + is_hard_stop + or is_auth_failure + or is_irrecoverable + or is_decision_lock + or is_validation_failure + or category in {CATEGORY_AUTH, CATEGORY_VALIDATION} + or "hard stop" in summary.lower() + or "unauthorized" in summary.lower() + or "irrecoverable" in summary.lower() + ): + return ATTENTION_HUMAN_REQUIRED, True + + if is_stale or is_blocker or category in {CATEGORY_BLOCKER, CATEGORY_LEASE}: + return ATTENTION_OPERATOR, False + + return ATTENTION_ROUTINE, False + + +def load_notifications_snapshot( + project_id: str | None = None, + *, + load_queue: Callable[..., QueueSnapshot] | None = None, + load_leases: Callable[..., LeaseSnapshot] | None = None, + load_health: Callable[..., SystemHealthSnapshot] | None = None, +) -> NotificationSnapshot: + """Load and classify attention notifications across queue, leases, and system health.""" + registry = load_registry() + project = None + if project_id: + for entry in registry.projects: + if entry.id == project_id: + project = entry + break + else: + project = registry.projects[0] if registry.projects else None + + if project is None: + return NotificationSnapshot( + project_id=project_id or "", + repo_label="", + items=(), + human_required_count=0, + operator_count=0, + routine_count=0, + total_count=0, + fetch_error="project not found in registry", + ) + + queue_loader_fn = load_queue or load_queue_snapshot + lease_loader_fn = load_leases or load_lease_snapshot + health_loader_fn = load_health or load_system_health + + try: + queue_snap = queue_loader_fn(project.id) + except TypeError: + queue_snap = queue_loader_fn(project_id=project.id) + + try: + lease_snap = lease_loader_fn(project_id=project.id) + except TypeError: + lease_snap = lease_loader_fn(project.id) + + try: + health_snap = health_loader_fn(project_id=project.id) + except TypeError: + try: + health_snap = health_loader_fn(project.id) + except TypeError: + health_snap = health_loader_fn() + + items: list[NotificationItem] = [] + now_iso = datetime.now(timezone.utc).isoformat() + + # 1. System health alerts (highest priority) + for probe_err in getattr(health_snap, "probe_errors", ()): + att_cls, req_human = classify_attention_event( + CATEGORY_SYSTEM, + "System Health Probe Error", + probe_err, + is_blocker=True, + ) + items.append( + NotificationItem( + id=f"notif-sys-err-{project.id}", + attention_class=att_cls, + category=CATEGORY_SYSTEM, + title="System Health Error", + summary=f"System health error: {probe_err}", + work_kind="system", + work_number=None, + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + created_at=now_iso, + deep_link="/system", + requires_human=req_human, + ) + ) + + for probe in getattr(health_snap, "dependencies", ()): + if probe.status not in ("ok", "healthy"): + att_cls, req_human = classify_attention_event( + CATEGORY_SYSTEM, + f"Probe Failure: {probe.name}", + probe.detail or probe.status, + is_hard_stop=("stop" in probe.status or "fatal" in probe.status), + is_auth_failure=("auth" in probe.name.lower() or "unauthorized" in probe.status.lower()), + is_blocker=True, + ) + items.append( + NotificationItem( + id=f"notif-probe-{probe.name}", + attention_class=att_cls, + category=CATEGORY_AUTH if "auth" in probe.name.lower() else CATEGORY_SYSTEM, + title=f"Health Probe Alert: {probe.name}", + summary=f"Probe '{probe.name}' reported status '{probe.status}': {probe.detail}", + work_kind="system", + work_number=None, + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + created_at=now_iso, + deep_link="/system", + requires_human=req_human, + ) + ) + + # 2. Queue items (PRs and Issues) + for pr in queue_snap.prs: + if "blocked" in pr.badges: + att_cls, req_human = classify_attention_event( + CATEGORY_BLOCKER, + f"PR #{pr.number} Blocked", + f"PR #{pr.number} '{pr.title}' is blocked or has merge conflicts.", + is_blocker=True, + ) + items.append( + NotificationItem( + id=f"notif-pr-block-{pr.number}", + attention_class=att_cls, + category=CATEGORY_BLOCKER, + title=f"Blocked PR #{pr.number}", + summary=f"PR #{pr.number} ({pr.title}) requires merge conflict resolution.", + work_kind="pr", + work_number=pr.number, + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + created_at=now_iso, + deep_link=f"/traffic", + requires_human=req_human, + ) + ) + elif "stale" in pr.badges: + att_cls, req_human = classify_attention_event( + CATEGORY_WORKFLOW, + f"PR #{pr.number} Stale", + f"PR #{pr.number} '{pr.title}' has had no activity for over 14 days.", + is_stale=True, + ) + items.append( + NotificationItem( + id=f"notif-pr-stale-{pr.number}", + attention_class=att_cls, + category=CATEGORY_WORKFLOW, + title=f"Stale PR #{pr.number}", + summary=f"PR #{pr.number} ({pr.title}) is stale.", + work_kind="pr", + work_number=pr.number, + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + created_at=now_iso, + deep_link=f"/queue", + requires_human=req_human, + ) + ) + else: + # Routine PR transition + att_cls, req_human = classify_attention_event( + CATEGORY_WORKFLOW, + f"PR #{pr.number} Active", + f"PR #{pr.number} '{pr.title}' is in routine state {', '.join(pr.badges)}.", + ) + items.append( + NotificationItem( + id=f"notif-pr-routine-{pr.number}", + attention_class=att_cls, + category=CATEGORY_WORKFLOW, + title=f"Routine PR #{pr.number}", + summary=f"PR #{pr.number} ({pr.title}) state: {', '.join(pr.badges)}.", + work_kind="pr", + work_number=pr.number, + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + created_at=now_iso, + deep_link=f"/queue", + requires_human=req_human, + ) + ) + + for issue in queue_snap.issues: + if "duplicate" in issue.badges: + att_cls, req_human = classify_attention_event( + CATEGORY_BLOCKER, + f"Issue #{issue.number} Duplicate PRs", + f"Issue #{issue.number} has multiple linked PRs.", + is_blocker=True, + ) + items.append( + NotificationItem( + id=f"notif-issue-dup-{issue.number}", + attention_class=att_cls, + category=CATEGORY_BLOCKER, + title=f"Duplicate PRs on Issue #{issue.number}", + summary=f"Issue #{issue.number} ({issue.title}) linked to multiple PRs.", + work_kind="issue", + work_number=issue.number, + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + created_at=now_iso, + deep_link=f"/traffic", + requires_human=req_human, + ) + ) + elif "claimed" in issue.badges or "in-review" in issue.badges: + att_cls, req_human = classify_attention_event( + CATEGORY_WORKFLOW, + f"Issue #{issue.number} Active", + f"Issue #{issue.number} '{issue.title}' in state {', '.join(issue.badges)}.", + ) + items.append( + NotificationItem( + id=f"notif-issue-routine-{issue.number}", + attention_class=att_cls, + category=CATEGORY_WORKFLOW, + title=f"Routine Issue #{issue.number}", + summary=f"Issue #{issue.number} ({issue.title}) state: {', '.join(issue.badges)}.", + work_kind="issue", + work_number=issue.number, + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + created_at=now_iso, + deep_link=f"/queue", + requires_human=req_human, + ) + ) + + # 3. Leases / Collisions + for lease in lease_snap.reviewer_leases: + if lease.get("is_expired") or lease.get("status") == "expired": + pr_num = lease.get("pr_number") or lease.get("work_item_number") + att_cls, req_human = classify_attention_event( + CATEGORY_LEASE, + f"Reviewer Lease Expired for PR #{pr_num}", + f"Reviewer lease for PR #{pr_num} has expired.", + is_stale=True, + ) + items.append( + NotificationItem( + id=f"notif-lease-exp-pr-{pr_num}", + attention_class=att_cls, + category=CATEGORY_LEASE, + title=f"Expired Reviewer Lease (PR #{pr_num})", + summary=f"Reviewer lease for PR #{pr_num} expired.", + work_kind="pr", + work_number=pr_num, + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + created_at=now_iso, + deep_link="/leases", + requires_human=req_human, + ) + ) + + for collision in lease_snap.duplicate_prs: + att_cls, req_human = classify_attention_event( + CATEGORY_BLOCKER, + f"Duplicate PR Collision ({collision.kind})", + collision.message, + is_blocker=True, + ) + items.append( + NotificationItem( + id=f"notif-collision-{collision.issue_number or 0}", + attention_class=att_cls, + category=CATEGORY_BLOCKER, + title=f"Collision Alert ({collision.kind})", + summary=collision.message, + work_kind="issue" if collision.issue_number else "pr", + work_number=collision.issue_number, + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + created_at=now_iso, + deep_link="/leases", + requires_human=req_human, + ) + ) + + human_req_count = sum(1 for i in items if i.attention_class == ATTENTION_HUMAN_REQUIRED) + operator_count = sum(1 for i in items if i.attention_class == ATTENTION_OPERATOR) + routine_count = sum(1 for i in items if i.attention_class == ATTENTION_ROUTINE) + + fetch_err = queue_snap.fetch_error or lease_snap.fetch_error or getattr(health_snap, "probe_errors", None) + if isinstance(fetch_err, (tuple, list)): + fetch_err = "; ".join(fetch_err) if fetch_err else None + + return NotificationSnapshot( + project_id=project.id, + repo_label=f"{project.gitea_owner}/{project.repo_name}", + items=tuple(items), + human_required_count=human_req_count, + operator_count=operator_count, + routine_count=routine_count, + total_count=len(items), + fetch_error=fetch_err, + ) + + +def snapshot_to_dict(snapshot: NotificationSnapshot) -> dict[str, Any]: + """JSON-serializable export for /api/v1/notifications.""" + return snapshot.as_dict() From 9a0154347771d1ca06202a8dfae52d9d57b77dd4 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 25 Jul 2026 17:23:52 -0400 Subject: [PATCH 06/12] feat(webui): add read-only console restart status and impact controls (Closes #667) --- tests/test_webui_restart_console.py | 452 ++++++++++++++++++++++ webui/app.py | 37 ++ webui/nav.py | 1 + webui/restart_console.py | 579 ++++++++++++++++++++++++++++ webui/restart_views.py | 299 ++++++++++++++ 5 files changed, 1368 insertions(+) create mode 100644 tests/test_webui_restart_console.py create mode 100644 webui/restart_console.py create mode 100644 webui/restart_views.py diff --git a/tests/test_webui_restart_console.py b/tests/test_webui_restart_console.py new file mode 100644 index 0000000..439bba5 --- /dev/null +++ b/tests/test_webui_restart_console.py @@ -0,0 +1,452 @@ +"""Read-only restart console: views, gates, and honesty rules (#667). + +The console consumes the #655 substrate. These tests hold it to the three +properties that make a status surface trustworthy: + +* an unreadable source is reported unavailable, never rendered as green; +* authorization is probed the way execution would probe it, so an allow is + never shown for something that could not run; +* the surface performs no mutation, including no write to the control-plane DB. +""" + +from __future__ import annotations + +import os +import sqlite3 +import sys +import tempfile +import unittest +from datetime import datetime, timedelta, timezone +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from starlette.testclient import TestClient # noqa: E402 + +import restart_coordinator # noqa: E402 +from webui import console_authz, restart_console, restart_views # noqa: E402 +from webui.app import create_app # noqa: E402 + +NOW = datetime(2026, 7, 25, 21, 0, 0, tzinfo=timezone.utc) + + +def _principal(role: str) -> console_authz.Principal: + return console_authz.Principal( + subject="operator@example.com", + role=role, + identity_source=console_authz.IDENTITY_LOCAL_DEV, + authenticated=True, + ) + + +def _inventory(*, complete: bool = True, sessions=(), leases=()): + def _read(**_kwargs): + return { + "sessions": list(sessions), + "leases": list(leases), + "terminal_lock": None, + "prior_recovery_attempts": [], + "inventory_complete": complete, + "incomplete_reasons": ( + [] if complete else ["fixture: inventory withheld"] + ), + } + + return _read + + +def _live_session(session_id: str = "prgs-author-1234-abcd") -> dict: + return { + "session_id": session_id, + "role": "author", + "profile": "prgs-author", + "pid": os.getpid(), + "status": "active", + "last_heartbeat_at": (NOW - timedelta(seconds=30)).isoformat(), + } + + +def drain_proof_fixture() -> dict: + """A structurally complete but unsigned drain proof.""" + return { + "version": "drain-proof/v1", + "proof_id": "deadbeef" * 8, + "clean": True, + "issued_at": (NOW - timedelta(minutes=1)).isoformat(), + "expires_at": (NOW + timedelta(minutes=5)).isoformat(), + "requesting_session_id": "s-live", + "impact_fingerprint": "f" * 64, + "checks": [], + "failed_checks": [], + } + + +class RestartClassMatrixTest(unittest.TestCase): + def test_every_policy_class_is_rendered(self) -> None: + views = restart_console.build_restart_class_views("operator") + self.assertEqual(len(views), len(restart_coordinator.RESTART_CLASS_POLICIES)) + + def test_viewer_capability_is_role_scoped_not_generic(self) -> None: + """A worker role must not be shown as able to request a full restart.""" + author = { + v.restart_class: v + for v in restart_console.build_restart_class_views("author") + } + operator = { + v.restart_class: v + for v in restart_console.build_restart_class_views("operator") + } + full = restart_coordinator.RestartClass.FULL_MCP_RESTART.value + + self.assertFalse(author[full].viewer_may_request) + self.assertFalse(author[full].viewer_may_execute) + self.assertTrue(operator[full].viewer_may_request) + self.assertTrue(operator[full].viewer_may_execute) + + def test_unknown_role_may_do_nothing(self) -> None: + views = restart_console.build_restart_class_views("not-a-role") + self.assertTrue(all(not v.viewer_may_request for v in views)) + self.assertTrue(all(not v.viewer_may_execute for v in views)) + + +class AuthorizationProbeTest(unittest.TestCase): + def test_probe_asks_for_execution_so_phase_gate_is_reported(self) -> None: + """An admin clears the role bar and still cannot execute in Phase 1. + + This is the case that distinguishes the two probes. Asked without + ``for_execution`` an admin is *allowed* for ``system.restart_namespace``, + which on a control surface reads as a live button. Asked the way + execution asks, the same principal is refused ``phase_not_active``. The + console must report the second answer. + """ + by_id = { + a.action_id: a + for a in restart_console.build_action_authorizations( + _principal(console_authz.ADMIN) + ) + } + restart = by_id["system.restart_namespace"] + + self.assertFalse(restart.execution_enabled) + self.assertEqual(restart.reason_code, console_authz.DENY_PHASE_NOT_ACTIVE) + + permissive = console_authz.authorize( + "system.restart_namespace", _principal(console_authz.ADMIN) + ) + self.assertTrue( + permissive.allowed, + "guard precondition: without for_execution an admin is allowed, " + "which is exactly why the console must not probe that way", + ) + + def test_operator_is_refused_the_admin_only_restart_action(self) -> None: + """Role refusal precedes the phase gate and is reported as such.""" + by_id = { + a.action_id: a + for a in restart_console.build_action_authorizations( + _principal(console_authz.OPERATOR) + ) + } + self.assertEqual( + by_id["system.restart_namespace"].reason_code, + console_authz.DENY_INSUFFICIENT_ROLE, + ) + + def test_anonymous_is_denied_unauthenticated(self) -> None: + by_id = { + a.action_id: a for a in restart_console.build_action_authorizations(None) + } + self.assertEqual( + by_id["system.restart_namespace"].reason_code, + console_authz.DENY_UNAUTHENTICATED, + ) + + def test_no_authorization_ever_reports_execution_enabled(self) -> None: + for role in ( + console_authz.VIEWER, + console_authz.OPERATOR, + console_authz.CONTROLLER, + console_authz.ADMIN, + ): + for auth in restart_console.build_action_authorizations(_principal(role)): + self.assertFalse( + auth.execution_enabled, + f"{role} reported execution_enabled for {auth.action_id}", + ) + + +class ImpactPreviewTest(unittest.TestCase): + def test_impact_renders_from_coordinator_dto(self) -> None: + impact, source = restart_console.load_impact_report( + principal=_principal(console_authz.OPERATOR), + read_inventory=_inventory(sessions=[_live_session()]), + now=NOW, + ) + self.assertTrue(source.available) + self.assertIsNotNone(impact) + self.assertEqual( + impact["restart_class"], + restart_coordinator.RestartClass.FULL_MCP_RESTART.value, + ) + self.assertIn("verdict", impact) + self.assertFalse(impact["restart_performed"]) + self.assertTrue(impact["dry_run"]) + + def test_incomplete_inventory_is_surfaced_and_denies(self) -> None: + impact, source = restart_console.load_impact_report( + principal=_principal(console_authz.OPERATOR), + read_inventory=_inventory(complete=False), + now=NOW, + ) + self.assertFalse(impact["inventory_complete"]) + self.assertFalse(impact["allow_restart"]) + self.assertTrue(source.detail, "incomplete inventory must explain itself") + + def test_inventory_reader_failure_is_unavailable_not_empty(self) -> None: + """A reader that raises must not be rendered as 'no sessions affected'.""" + + def _boom(**_kwargs): + raise RuntimeError("control-plane unreachable") + + impact, source = restart_console.load_impact_report( + principal=_principal(console_authz.OPERATOR), + read_inventory=_boom, + now=NOW, + ) + self.assertIsNone(impact) + self.assertFalse(source.available) + self.assertIn("control-plane unreachable", source.detail) + + +class ControlPlaneReadTest(unittest.TestCase): + def test_missing_database_is_incomplete_not_empty(self) -> None: + inventory = restart_console.read_control_plane_inventory( + db_path="/nonexistent/control-plane.sqlite3" + ) + self.assertFalse(inventory["inventory_complete"]) + self.assertEqual(inventory["sessions"], []) + self.assertTrue(inventory["incomplete_reasons"]) + + def test_reader_never_creates_the_database(self) -> None: + """Reading status must not bring a control-plane DB into existence. + + The path deliberately sits in a directory that already exists: a + read-write ``sqlite3.connect`` would happily create the file there, so + this fails if the reader ever stops opening the database ``mode=ro``. + A nested-missing-directory path would pass for the wrong reason, + because sqlite cannot create the parent directory either way. + """ + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "control_plane.sqlite3") + self.assertTrue(os.path.isdir(os.path.dirname(path))) + + inventory = restart_console.read_control_plane_inventory(db_path=path) + + self.assertFalse( + os.path.exists(path), + "reading restart status created a control-plane database", + ) + self.assertFalse(inventory["inventory_complete"]) + + def test_reads_active_sessions_from_a_real_database(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "cp.sqlite3") + conn = sqlite3.connect(path) + conn.execute( + "CREATE TABLE sessions (session_id TEXT, role TEXT, profile TEXT," + " pid INTEGER, status TEXT, last_heartbeat_at TEXT)" + ) + conn.execute( + "CREATE TABLE work_items (work_item_id INTEGER, kind TEXT," + " number INTEGER)" + ) + conn.execute( + "CREATE TABLE leases (lease_id TEXT, session_id TEXT, role TEXT," + " phase TEXT, status TEXT, worktree_path TEXT," + " work_item_id INTEGER, expires_at TEXT)" + ) + conn.execute( + "INSERT INTO sessions VALUES (?,?,?,?,?,?)", + ("s-live", "author", "prgs-author", 4242, "active", NOW.isoformat()), + ) + conn.execute( + "INSERT INTO sessions VALUES (?,?,?,?,?,?)", + ("s-done", "author", "prgs-author", 11, "closed", NOW.isoformat()), + ) + conn.execute("INSERT INTO work_items VALUES (1, 'issue', 667)") + conn.execute( + "INSERT INTO leases VALUES (?,?,?,?,?,?,?,?)", + ( + "l-1", + "s-live", + "author", + "allocated", + "active", + None, + 1, + NOW.isoformat(), + ), + ) + conn.commit() + conn.close() + + inventory = restart_console.read_control_plane_inventory(db_path=path) + + self.assertTrue(inventory["inventory_complete"]) + self.assertEqual([s["session_id"] for s in inventory["sessions"]], ["s-live"]) + self.assertEqual(inventory["leases"][0]["work_number"], 667) + + +class DrainAndReconcileTest(unittest.TestCase): + def test_absent_drain_proof_is_not_a_pass(self) -> None: + drain, source = restart_console.load_drain_status(proof=None, now=NOW) + self.assertIsNone(drain) + self.assertFalse(source.available) + self.assertIn("denies", source.detail) + + def test_tampered_drain_proof_is_reported_invalid(self) -> None: + proof = drain_proof_fixture() + proof["clean"] = True + proof["proof_id"] = "0" * 64 + drain, source = restart_console.load_drain_status(proof=proof, now=NOW) + self.assertTrue(source.available) + self.assertFalse(drain["valid"]) + + def test_absent_reconcile_proof_is_unavailable(self) -> None: + reconcile, source = restart_console.load_reconcile_status(load_proof=None) + self.assertIsNone(reconcile) + self.assertFalse(source.available) + + def test_reconcile_proof_is_rendered_when_supplied(self) -> None: + payload = { + "overall_status": "degraded", + "mode": "log_only", + "resolved_count": 3, + "unresolved_count": 2, + "items": [ + { + "dimension": "leases", + "status": "unresolved", + "summary": "2 orphaned leases", + "follow_up_required": True, + } + ], + } + reconcile, source = restart_console.load_reconcile_status( + load_proof=lambda: payload + ) + self.assertTrue(source.available) + self.assertEqual(reconcile["unresolved_count"], 2) + + +class RenderingTest(unittest.TestCase): + def _snapshot(self, **kwargs): + params = { + "principal": _principal(console_authz.OPERATOR), + "read_inventory": _inventory(sessions=[_live_session()]), + "now": NOW, + } + params.update(kwargs) + return restart_console.load_restart_console_snapshot(**params) + + def test_page_renders_every_section(self) -> None: + html = restart_views.render_restart_console_page(self._snapshot()) + for heading in ( + "Impact preview", + "Drain proof", + "Post-restart reconcile", + "Restart classes", + "Approval controls", + "Break-glass", + ): + self.assertIn(heading, html) + + def test_hostile_session_id_is_escaped(self) -> None: + hostile = "" + html = restart_views.render_restart_console_page( + self._snapshot(read_inventory=_inventory(sessions=[_live_session(hostile)])) + ) + self.assertNotIn("