UX audit fixes: accurate is_new on run tools, approver reason capture, human-readable approval pages

.hoplite/settings.json

@@ -0,0 +1,14 @@
+{
+	"version": 1,
+	"ports": {"preview": 3000, "additional": {}},
+	"scripts": {
+		"setup": {
+			"enabled": true,
+			"command": "apt-get update -qq && apt-get install -y -qq git-lfs >/dev/null 2>&1; uv sync --all-extras"
+		},
+		"run": {"enabled": true, "command": null},
+		"archive": {"enabled": true, "command": null},
+		"check": {"enabled": true, "command": "uv run pytest -q && uv run ruff check src tests && uv run mypy src/cam"}
+	},
+	"mcpServers": []
+}

src/cam/core/orchestrator/gates.py

@@ -146,6 +146,7 @@ async def resolve_gate(
     signing_key: bytes,
     store: Any,  # RunStore protocol
     audit_fn: Any | None = None,  # AuditService.record — optional
+    reason: str | None = None,
 ) -> ApprovalDecision:
     """Verify token → expiry → single-use → authz → record → resume/reject.
 
@@ -224,6 +225,7 @@ async def resolve_gate(
         channel=channel,
         token_id=token_id,
         decided_at=decided_at,
+        reason=reason,
     )
     await store.save_approval_decision(approval_decision)
 
@@ -235,7 +237,8 @@ async def resolve_gate(
     action = "gate.approved" if decision == "approve" else "gate.rejected"
     await _maybe_audit(
         audit_fn, actor, action,
-        {"run_id": run_id, "step": step, "channel": channel, "token_id": token_id}
+        {"run_id": run_id, "step": step, "channel": channel, "token_id": token_id,
+         "reason": reason}
     )
 
     new_run_status = RunStatus.RUNNING if decision == "approve" else RunStatus.REJECTED

src/cam/core/orchestrator/triggers.py

@@ -31,6 +31,7 @@ async def start_run(
     trigger: TriggerRef,
     store: Any,
     idem_key: str | None = None,
+    run_id: str | None = None,
 ) -> WorkflowRun:
     """Create and enqueue a new workflow run — idempotent on dedupe_key.
 
@@ -43,6 +44,9 @@ async def start_run(
         trigger: How the run was triggered.
         store: RunStore (in-memory for tests, SQLAlchemy for prod).
         idem_key: Optional explicit dedupe key (overrides trigger.dedupe_key).
+        run_id: Optional caller-supplied run id.  Callers that need to report
+            whether a fresh run was created can supply one and compare it to
+            the returned run's id (a dedupe hit returns the existing run).
     """
     defn = get_workflow_latest(workflow_name)
     defn.validate_context(context)
@@ -52,7 +56,7 @@ async def start_run(
         trigger = trigger.model_copy(update={"dedupe_key": dedupe_key})
 
     now = datetime.now(tz=UTC)
-    run_id = str(uuid.uuid4())
+    run_id = run_id or str(uuid.uuid4())
 
     run = WorkflowRun(
         id=run_id,

src/cam/core/workflows/intake/workflow.py

@@ -15,6 +15,7 @@
 from __future__ import annotations
 
 import hashlib
+import uuid
 from dataclasses import dataclass
 from typing import Any
 
@@ -218,11 +219,13 @@ async def tool_intake_run(
         idempotency_key = hashlib.sha256(base.encode()).hexdigest()[:16]
 
     trigger = make_agent_trigger(actor, idem_key=idempotency_key)
+    candidate_id = str(uuid.uuid4())
     context = {"lead": lead.model_dump(mode="json"), "case_type_hint": case_type_hint}
     run = await start_run(
         workflow_name="intake", context=context,
         trigger=trigger, store=run_store, idem_key=idempotency_key,
+        run_id=candidate_id,
     )
     return {"run_id": run.id, "status": run.status, "current_step": run.current_step,
-            "is_new": run.trigger.dedupe_key == idempotency_key}
+            "is_new": run.id == candidate_id}
 

src/cam/mcp_server/workflow_tools.py

@@ -54,21 +54,25 @@ async def tool_workflow_run(
 
     Risk tier: write (confirm).  Starting a run never bypasses inner gates.
     """
+    import uuid
+
     from cam.core.orchestrator.triggers import make_agent_trigger, start_run
 
     trigger = make_agent_trigger(actor, idem_key=inp.idem_key)
+    candidate_id = str(uuid.uuid4())
     run = await start_run(
         workflow_name=inp.workflow,
         context=inp.context,
         trigger=trigger,
         store=store,
         idem_key=inp.idem_key,
+        run_id=candidate_id,
     )
     return WorkflowRunOutput(
         run_id=run.id,
         status=run.status,
         current_step=run.current_step,
-        is_new=True,  # simplified — the store dedup handles idempotency
+        is_new=run.id == candidate_id,
     )
 
 
@@ -196,6 +200,7 @@ async def tool_approval_decide(
         signing_key=signing_key,
         store=store,
         audit_fn=audit_fn,
+        reason=inp.reason,
     )
     return ApprovalDecideOutput(
         gate_request_id=decision_record.gate_request_id,

src/cam/sidecar/approvals.py

@@ -1,12 +1,18 @@
-"""Web UI approval endpoint — GET/POST /approvals/{token} — spec G8.3."""
+"""Web UI approval endpoint — GET/POST /approvals/{token} — spec G8.3.
+
+Human-facing pages: the approver arrives from an email/web link and must see
+what they are approving, record a reason, and get a readable confirmation —
+never raw JSON.
+"""
 
 from __future__ import annotations
 
-from datetime import UTC
+from datetime import UTC, datetime
+from html import escape
 from typing import Literal
 
 from fastapi import APIRouter, HTTPException, Request
-from fastapi.responses import HTMLResponse, JSONResponse
+from fastapi.responses import HTMLResponse
 from pydantic import BaseModel
 
 router = APIRouter(prefix="/approvals", tags=["approvals"])
@@ -17,6 +23,36 @@ class ApprovalSubmission(BaseModel):
     reason: str | None = None
 
 
+_PAGE_STYLE = """body{font-family:sans-serif;max-width:600px;margin:4rem auto;padding:0 1rem}
+.card{background:#f8f9fa;border-radius:8px;padding:1.5rem;margin-bottom:1.5rem}
+.approve{background:#198754;color:#fff;border:none;padding:.75rem 2rem;
+  border-radius:4px;cursor:pointer;font-size:1rem;margin-right:1rem}
+.reject{background:#dc3545;color:#fff;border:none;padding:.75rem 2rem;
+  border-radius:4px;cursor:pointer;font-size:1rem}
+textarea{width:100%;box-sizing:border-box;margin:1rem 0;padding:.5rem;
+  border:1px solid #ccc;border-radius:4px;font-family:inherit}
+.ok{color:#198754}.err{color:#dc3545}"""
+
+
+def _page(title: str, body: str, status_code: int = 200) -> HTMLResponse:
+    html = f"""<!DOCTYPE html>
+<html lang="en">
+<head><meta charset="UTF-8"><title>{escape(title)}</title>
+<style>{_PAGE_STYLE}</style></head>
+<body>
+{body}
+</body></html>"""
+    return HTMLResponse(html, status_code=status_code)
+
+
+def _error_page(reason: str, status_code: int) -> HTMLResponse:
+    body = f"""<h1 class="err">Approval not recorded</h1>
+<div class="card"><p>{escape(reason)}</p>
+<p>This link is single-use and expires 24 hours after it was issued.
+Ask the requester to re-trigger the approval if you still need to act on it.</p></div>"""
+    return _page("Approval not recorded", body, status_code=status_code)
+
+
 @router.get("/{raw_token}", response_class=HTMLResponse)
 async def approval_page(raw_token: str, request: Request) -> HTMLResponse:
     """Render the gate context for the approver."""
@@ -27,44 +63,47 @@ async def approval_page(raw_token: str, request: Request) -> HTMLResponse:
 
     payload, err = verify_token(raw_token, signing_key)
     if err:
-        raise HTTPException(status_code=401, detail=f"Invalid token: {err}")
+        return _error_page(f"This approval link is not valid ({err}).", 401)
 
     assert payload is not None  # verified above
-    from datetime import datetime
-
+    run_id = str(payload.get("rid", ""))
+    step = str(payload.get("step", ""))
     exp = payload.get("exp", 0)
-    expires_at = datetime.fromtimestamp(exp, tz=UTC).isoformat()
+    expires_at = datetime.fromtimestamp(exp, tz=UTC).strftime("%Y-%m-%d %H:%M UTC")
 
-    html = f"""<!DOCTYPE html>
-<html lang="en">
-<head><meta charset="UTF-8"><title>Approval Required</title>
-<style>body{{font-family:sans-serif;max-width:600px;margin:4rem auto;padding:0 1rem}}
-.gate{{background:#f8f9fa;border-radius:8px;padding:1.5rem;margin-bottom:1.5rem}}
-.approve{{background:#198754;color:#fff;border:none;padding:.75rem 2rem;
-  border-radius:4px;cursor:pointer;font-size:1rem;margin-right:1rem}}
-.reject{{background:#dc3545;color:#fff;border:none;padding:.75rem 2rem;
-  border-radius:4px;cursor:pointer;font-size:1rem}}</style></head>
-<body>
-<h1>Workflow approval required</h1>
-<div class="gate">
-  <p><strong>Run:</strong> {payload.get("rid","")}</p>
-  <p><strong>Step:</strong> {payload.get("step","")}</p>
-  <p><strong>Expires:</strong> {expires_at}</p>
+    # Enrich with the workflow name when a run store is wired (fail-soft).
+    workflow_name = ""
+    store = getattr(app_state, "run_store", None)
+    if store is not None and run_id:
+        try:
+            run = await store.get_run(run_id)
+            if run is not None:
+                workflow_name = str(run.workflow)
+        except Exception:
+            workflow_name = ""
+
+    workflow_line = (
+        f'<p><strong>Workflow:</strong> {escape(workflow_name)}</p>' if workflow_name else ""
+    )
+    body = f"""<h1>Workflow approval required</h1>
+<div class="card">
+  {workflow_line}
+  <p><strong>Run:</strong> {escape(run_id)}</p>
+  <p><strong>Step:</strong> {escape(step)}</p>
+  <p><strong>Expires:</strong> {escape(expires_at)}</p>
 </div>
-<form method="POST" action="/approvals/{raw_token}">
+<form method="POST" action="/approvals/{escape(raw_token)}">
+  <label for="reason">Reason (optional, recorded with your decision):</label>
+  <textarea id="reason" name="reason" rows="3"
+    placeholder="e.g. Reviewed the draft; ready to send"></textarea>
   <button class="approve" name="decision" value="approve" type="submit">Approve</button>
   <button class="reject" name="decision" value="reject" type="submit">Reject</button>
-</form>
-</body></html>"""
-    return HTMLResponse(html)
+</form>"""
+    return _page("Approval required", body)
 
 
 @router.post("/{raw_token}")
-async def submit_approval(
-    raw_token: str,
-    request: Request,
-    decision: Literal["approve", "reject"] | None = None,
-) -> JSONResponse:
+async def submit_approval(raw_token: str, request: Request) -> HTMLResponse:
     """Submit an approval decision via the web channel."""
     from cam.core.orchestrator.gates import GateResolutionError, resolve_gate
 
@@ -75,12 +114,17 @@ async def submit_approval(
         raise HTTPException(status_code=503, detail="Run store unavailable.")
 
     # Support both form POST and query param
+    decision: str | None = request.query_params.get("decision")
     if decision is None:
         form = await request.form()
         decision = form.get("decision")  # type: ignore[assignment]
     if decision not in ("approve", "reject"):
         raise HTTPException(status_code=400, detail="decision must be 'approve' or 'reject'.")
-    _decision: Literal["approve", "reject"] = decision
+    _decision: Literal["approve", "reject"] = decision  # type: ignore[assignment]
+
+    form = await request.form()
+    reason_value = form.get("reason")
+    reason = str(reason_value).strip() or None if isinstance(reason_value, str) else None
 
     actor = getattr(request.state, "user_identity", "web_anonymous")
 
@@ -92,10 +136,19 @@ async def submit_approval(
             channel="web",
             signing_key=signing_key,
             store=store,
+            reason=reason,
         )
     except GateResolutionError as exc:
-        raise HTTPException(status_code=exc.status_code, detail=exc.reason) from exc
-
-    return JSONResponse(
-        {"decision": result.decision, "run_id": result.run_id, "gate": result.gate_request_id}
-    )
+        return _error_page(exc.reason, exc.status_code)
+
+    outcome = "approved" if result.decision == "approve" else "rejected"
+    reason_line = f"<p><strong>Reason recorded:</strong> {escape(reason)}</p>" if reason else ""
+    body = f"""<h1 class="ok">Decision recorded</h1>
+<div class="card">
+  <p>The workflow step was <strong>{escape(outcome)}</strong>.</p>
+  <p><strong>Run:</strong> {escape(result.run_id)}</p>
+  <p><strong>Gate:</strong> {escape(result.gate_request_id)}</p>
+  {reason_line}
+</div>
+<p>You can close this window — the workflow continues automatically.</p>"""
+    return _page("Decision recorded", body)

tests/test_e2e_workflows.py

@@ -257,3 +257,26 @@ async def test_e2e_05_duplicate_lead_produces_single_run() -> None:
 
     assert r1["run_id"] == r2["run_id"], "Duplicate trigger must return the same run"
     assert len(run_store._runs) == 1, "Only one run should exist"
+    assert r1["is_new"] is True, "First submission must report a new run"
+    assert r2["is_new"] is False, "Duplicate submission must report the existing run"
+
+
+async def test_e2e_06_workflow_run_reports_is_new() -> None:
+    """workflow.run reports is_new=False when the idem key dedupes to an existing run."""
+    from cam.mcp_server.workflow_tools import WorkflowRunInput, tool_workflow_run
+
+    services = _make_services()
+    register_intake_workflow(services)
+
+    run_store = InMemoryRunStore()
+    inp = WorkflowRunInput(
+        workflow="intake",
+        context={"lead": _make_lead().model_dump(mode="json")},
+        idem_key="e2e-06-idem-001",
+    )
+    out1 = await tool_workflow_run(inp, run_store)
+    out2 = await tool_workflow_run(inp, run_store)
+
+    assert out1.is_new is True
+    assert out2.is_new is False
+    assert out1.run_id == out2.run_id

tests/test_intake.py

@@ -361,6 +361,8 @@ async def test_duplicate_lead_same_run() -> None:
     r1 = await tool_intake_run(lead, store, idempotency_key="idem-abc")
     r2 = await tool_intake_run(lead, store, idempotency_key="idem-abc")
     assert r1["run_id"] == r2["run_id"]
+    assert r1["is_new"] is True
+    assert r2["is_new"] is False
 
 
 # ──────────────────────────────────────────────────────────────

tests/test_orchestrator_gates.py

@@ -2,6 +2,8 @@
 
 from __future__ import annotations
 
+from typing import Any
+
 import pytest
 
 from cam.core.orchestrator.dsl import GateConfig, StepContext, clear_registry, workflow
@@ -130,3 +132,29 @@ def test_token_verify_wrong_key() -> None:
     raw_token, _ = issue_token("gid", "rid", "GATE:step", "mcp", KEY)
     _, err = verify_token(raw_token, b"wrong-key-32-bytes-padded-00000")
     assert err == "tampered"
+
+
+# h. Approver reason is recorded on the decision and in the audit stream
+async def test_reason_recorded_on_decision() -> None:
+    store, run_id, raw_token, _ = await _build_gated_run("wf_reason")
+    audits: list[dict[str, Any]] = []
+
+    async def _audit(**kwargs: Any) -> None:
+        audits.append(kwargs)
+
+    await resolve_gate(
+        raw_token, "approve", "attorney", "mcp", KEY, store,
+        audit_fn=_audit, reason="Reviewed draft; ready to send",
+    )
+    decision = store.all_decisions()[0]
+    assert decision.reason == "Reviewed draft; ready to send"
+    approved = [a for a in audits if a.get("action") == "gate.approved"]
+    assert approved, "gate.approved must be audited"
+    assert approved[0]["inputs"]["reason"] == "Reviewed draft; ready to send"
+
+
+# i. Omitted reason stays None (backwards compatible)
+async def test_reason_defaults_to_none() -> None:
+    store, run_id, raw_token, _ = await _build_gated_run("wf_no_reason")
+    await resolve_gate(raw_token, "approve", "attorney", "mcp", KEY, store)
+    assert store.all_decisions()[0].reason is None