From f98768f0a0dc230c2e16a55f1bc23585ccf7e526 Mon Sep 17 00:00:00 2001 From: Lucas Tonon Date: Sun, 19 Jul 2026 08:48:35 -0300 Subject: [PATCH 1/2] feat: human-in-the-loop approval checkpoints before expensive stages (#108) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - New `request-approval` capability skill: the agent posts a consequential decision (target column, prep/leakage plan, model shortlist, expensive run) as an actionable card and BLOCKS on the user's Approve/Edit. Reuses services/clarifications.py's future registry (timeouts, session-cleanup cancellation); 30-min window, timeout tells the agent to proceed but flag the decision unapproved. - services/approvals.py: per-session opt-in flag + apply_approval_gate, an identity function when the flag is off (same list, same prompt string object) — default behavior provably unchanged. The runner applies it per agent run, so delegated sub-agents inherit the gate through the shared session_id without parameter threading. - POST /sessions/{id}/approvals/{approval_id} resolves the pending gate (approve | edit; edit requires non-empty edits). - MessageCreate.approvals (optional): each run-triggering message re-asserts the session flag; omitted/false keeps gates off. Frontend: - ShieldCheck toggle in the chat input bar (OFF by default); the toggle state rides along on sendMessage only when ON. - ApprovalCard in components/chat/: kind badge, proposed decision (markdown), Approve / Edit-with-revision actions, resolved states (approved / edited / timeout / cancelled). - `approval_request` / `approval_resolved` SSE events: reducer cases in useSessionStream plus restore-on-reload handling so a reload mid-gate brings the card back actionable instead of stalling the session. Tests: 13 new (gate identity when disabled, skill+prompt injection, send_message flag wiring incl. default-off, handler block/approve/edit/ timeout/validation, resolve endpoint 200/404/400). Closes #108 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4 --- backend/routers/sessions.py | 50 ++- backend/schemas.py | 12 + backend/services/agent/runner.py | 11 + backend/services/approvals.py | 80 +++++ backend/skills/request-approval/SKILL.md | 33 ++ backend/skills/request-approval/handler.py | 173 ++++++++++ backend/skills/request-approval/schema.yaml | 27 ++ backend/tests/test_approvals.py | 305 ++++++++++++++++++ frontend/src/app/page.tsx | 15 + frontend/src/components/chat/ApprovalCard.tsx | 212 ++++++++++++ frontend/src/components/chat/ChatItemView.tsx | 3 + frontend/src/components/chat/ChatPane.tsx | 27 ++ frontend/src/lib/api.ts | 20 ++ frontend/src/lib/chatItems.ts | 8 + frontend/src/lib/types.ts | 20 ++ frontend/src/lib/useSessionStream.ts | 85 +++++ 16 files changed, 1080 insertions(+), 1 deletion(-) create mode 100644 backend/services/approvals.py create mode 100644 backend/skills/request-approval/SKILL.md create mode 100644 backend/skills/request-approval/handler.py create mode 100644 backend/skills/request-approval/schema.yaml create mode 100644 backend/tests/test_approvals.py create mode 100644 frontend/src/components/chat/ApprovalCard.tsx diff --git a/backend/routers/sessions.py b/backend/routers/sessions.py index 33362b7..edac8d5 100644 --- a/backend/routers/sessions.py +++ b/backend/routers/sessions.py @@ -16,7 +16,15 @@ from db import async_session, get_db from models import Artifact, Experiment, LogEvent, Message, Metric, Task from models import Session as SessionModel -from schemas import ClarificationReply, MessageCreate, SessionResume, TaskCreate, TaskUpdate +from schemas import ( + ApprovalReply, + ClarificationReply, + MessageCreate, + SessionResume, + TaskCreate, + TaskUpdate, +) +from services import approvals as approvals_svc from services.agent import abort_agent, run_agent from services.agent.resume import ( build_resume_context, @@ -141,6 +149,11 @@ async def send_message( if session.experiment and session.experiment.project: sandbox_config = session.experiment.project.sandbox_config or {} + # HITL approval gates (issue #108): assert the per-session flag from + # the message's toggle state on every launch. Omitted/False keeps the + # default (gates off) — the runner's injection is a no-op then. + approvals_svc.set_enabled(session_id, bool(body.approvals)) + # Mark the session "running" eagerly so the sidebar spinner + the # restore-on-tab-switch path both see the live state. Completion / # failure / cancellation paths below overwrite this. @@ -358,6 +371,41 @@ async def reply_to_clarification( return {"status": "ok"} +@router.post("/sessions/{session_id}/approvals/{approval_id}") +async def reply_to_approval( + session_id: str, + approval_id: str, + body: ApprovalReply, +): + """User verdict on a pending approval gate — unblocks the waiting agent. + + The `approval_resolved` SSE/persistence event is published by the skill + handler once its future resolves, so this route only resolves the future. + """ + edits = body.edits.strip() + if body.decision == "edit" and not edits: + raise HTTPException( + status_code=400, + detail="decision='edit' requires non-empty edits.", + ) + answered = resolve_clarification( + session_id, + approval_id, + { + "decision": body.decision, + "answer": edits, + "answered_by": "user", + "timeout": False, + }, + ) + if not answered: + raise HTTPException( + status_code=404, + detail="No pending approval with that approval_id (already answered or expired).", + ) + return {"status": "ok", "decision": body.decision} + + @router.get("/sessions/{session_id}/artifacts") async def get_artifacts(session_id: str, db: AsyncSession = Depends(get_db)): result = await db.execute(select(Artifact).where(Artifact.session_id == session_id)) diff --git a/backend/schemas.py b/backend/schemas.py index 96ae2bb..2ad9b75 100644 --- a/backend/schemas.py +++ b/backend/schemas.py @@ -55,12 +55,24 @@ class MessageCreate(BaseModel): # services/llm/thinking.py. Ignored for models that don't support it. agent_thinking: Optional[dict[str, str]] = Field(default=None) mentions: Optional[list[Mention]] = Field(default=None, max_length=64) + # HITL approval gates (issue #108). Opt-in: when true, agents launched by + # this message gain the `request-approval` skill and block consequential + # decisions on user Approve/Edit. Omitted/None/False → default behavior. + approvals: Optional[bool] = Field(default=None) class ClarificationReply(BaseModel): answer: str = Field(..., max_length=_CLARIFICATION_MAX) +class ApprovalReply(BaseModel): + """User verdict on a pending approval gate (issue #108).""" + + decision: Literal["approve", "edit"] + # Required (non-empty) when decision == "edit" — enforced in the route. + edits: str = Field(default="", max_length=_CLARIFICATION_MAX) + + class SessionResume(BaseModel): """Body for POST /sessions/{id}/resume. Everything is optional — the default is 'relaunch with the same knobs the session already has'.""" diff --git a/backend/services/agent/runner.py b/backend/services/agent/runner.py index 0842552..00dce5a 100644 --- a/backend/services/agent/runner.py +++ b/backend/services/agent/runner.py @@ -1013,6 +1013,17 @@ async def _publish( if "delegate-task" in agent_skills and not can_delegate(agent_type, depth): agent_skills = [s for s in agent_skills if s != "delegate-task"] + # HITL approval gates (issue #108): opt-in per session. When the flag + # is off this is an identity call — skills and prompt come back + # unchanged, so the default path is untouched. Applied per agent run, + # so delegated sub-agents inherit the gate through the shared + # session_id without any parameter threading. + from services.approvals import apply_approval_gate + + agent_skills, system_prompt = apply_approval_gate( + session_id, agent_skills, system_prompt + ) + logger.info( "Starting agent=%s id=%s parent=%s stage=%s session=%s provider=%s model=%s depth=%d skills=%s", agent_type, diff --git a/backend/services/approvals.py b/backend/services/approvals.py new file mode 100644 index 0000000..c135afd --- /dev/null +++ b/backend/services/approvals.py @@ -0,0 +1,80 @@ +"""Human-in-the-loop approval gates (issue #108). + +Opt-in, per-session: when the user switches approvals on (a toggle in the +chat input, sent with each run-triggering message), every agent run in the +session gains the `request-approval` capability skill plus a system-prompt +block telling it to post consequential decisions (target column, prep / +leakage plan, model shortlist, expensive runs) as an actionable card and +BLOCK until the user approves or edits. + +The blocking future itself is services.clarifications — approvals reuse the +same (session_id, question_id) registry, timeout machinery, and +session-cleanup cancellation. This module only owns the enable flag and the +skill/prompt injection applied by the runner. + +Default behavior is byte-identical when the flag is off: `apply_approval_gate` +returns its inputs unchanged. +""" + +from __future__ import annotations + +APPROVAL_SKILL_SLUG = "request-approval" + +APPROVAL_GATE_PROMPT = """## Approval gates (ENABLED for this session) + +The user switched on human-in-the-loop approvals. Before COMMITTING to any +consequential decision, call `request-approval` with the decision and block +on the result. Consequential decisions are: + +- the prediction **target column** (and problem framing), +- the **data-prep / leakage-handling plan** (drops, imputations, split + strategy, leakage mitigations), +- the **model shortlist** or final model/hyperparameter-search choice, +- anything that starts a long or expensive run (training sweeps, GPU jobs). + +Rules: +- Call `request-approval` BEFORE acting on the decision, never after. +- If the result is `approved`, proceed exactly as proposed. +- If the result is `edited`, the user's revision REPLACES your proposal — + follow it exactly. +- If the approval times out with no response, proceed with your proposal but + clearly flag in your report that it was not explicitly approved. +- Don't gate trivial choices (plot styles, file names, obvious defaults) — + one gate per consequential decision, not per step.""" + +# Sessions with approval gates switched on. In-memory by design (mirrors the +# clarification futures it gates on): the flag is re-asserted by the frontend +# on every run-triggering message, so a backend restart just falls back to +# the default-off state until the next message. +_enabled_sessions: set[str] = set() + + +def set_enabled(session_id: str, enabled: bool) -> None: + """Assert the approval-gate flag for a session (called per run launch).""" + if enabled: + _enabled_sessions.add(session_id) + else: + _enabled_sessions.discard(session_id) + + +def is_enabled(session_id: str) -> bool: + return session_id in _enabled_sessions + + +def apply_approval_gate( + session_id: str, agent_skills: list[str], system_prompt: str +) -> tuple[list[str], str]: + """Inject the approval skill + prompt block when the session opts in. + + When the flag is off this is an identity function — the returned list has + the same contents and the returned prompt is the same string object, so + the default path is provably unchanged. + """ + if not is_enabled(session_id): + return agent_skills, system_prompt + skills = ( + agent_skills + if APPROVAL_SKILL_SLUG in agent_skills + else [*agent_skills, APPROVAL_SKILL_SLUG] + ) + return skills, system_prompt + "\n\n" + APPROVAL_GATE_PROMPT diff --git a/backend/skills/request-approval/SKILL.md b/backend/skills/request-approval/SKILL.md new file mode 100644 index 0000000..024f58e --- /dev/null +++ b/backend/skills/request-approval/SKILL.md @@ -0,0 +1,33 @@ +--- +name: request-approval +description: > + Post a consequential decision (target column, prep/leakage plan, model + shortlist, expensive run) to the user as an actionable approval card and + BLOCK until they Approve or Edit it. +when_to_use: > + Only available when the session has approval gates switched on. Call it + BEFORE committing to a consequential decision — never after acting on it. +version: '0.1' +kind: capability +--- + +# request-approval + +Human-in-the-loop approval gate. Renders an actionable card in the studio +chat with your proposed decision; your run blocks until the user clicks +**Approve** or submits an **Edit** (or the approval window times out). + +## Arguments + +- `title` — short label for the card, e.g. "Target column" or "Prep plan". +- `decision` — the concrete decision you propose, stated so the user can + approve it as-is. Markdown allowed. +- `kind` — one of `target_column`, `prep_plan`, `model_shortlist`, `other`. +- `context` (optional) — why you chose this, alternatives you rejected. + +## Result contract + +- `approved` — proceed exactly as proposed. +- `edited` — the user's revision replaces your proposal; follow it exactly. +- `timeout` — no response within the window; proceed with your proposal but + flag in your report that it was not explicitly approved. diff --git a/backend/skills/request-approval/handler.py b/backend/skills/request-approval/handler.py new file mode 100644 index 0000000..27a964d --- /dev/null +++ b/backend/skills/request-approval/handler.py @@ -0,0 +1,173 @@ +"""request-approval skill — HITL approval gate on consequential decisions. + +Flow: + 1. Agent calls the skill with its proposed decision. + 2. We register a blocking future in services.clarifications (same registry, + timeout, and session-cleanup semantics as request-clarification) and + publish an `approval_request` SSE event that renders the actionable card. + 3. The run BLOCKS on the future until the user clicks Approve / submits an + Edit via POST /sessions/{id}/approvals/{approval_id} — or the window + times out. + 4. We publish `approval_resolved` (persisted, so reloads restore the card's + final state) and return the outcome as the tool result. +""" + +from __future__ import annotations + +import asyncio +import logging + +from services.clarifications import register + +logger = logging.getLogger(__name__) + +# Approval gates exist to stop expensive work from launching unreviewed, so +# the window is generous. On timeout the agent is told to proceed with its +# proposal but flag that it wasn't explicitly approved. +_APPROVAL_TIMEOUT_S = 1800.0 + +_VALID_KINDS = {"target_column", "prep_plan", "model_shortlist", "other"} + + +def create_handler( + session_id: str, + publish_fn, + parent_agent_type: str, + parent_agent_id: str = "root", + parent_parent_agent_id: str | None = None, + current_depth: int = 0, + **kwargs, +): + asker_meta = { + "agent_id": parent_agent_id, + "agent_type": parent_agent_type, + "parent_agent_id": parent_parent_agent_id, + "depth": current_depth, + } + + async def handler(args: dict): + title = (args.get("title") or "").strip() + decision = (args.get("decision") or "").strip() + context = (args.get("context") or "").strip() + kind = args.get("kind") or "other" + if kind not in _VALID_KINDS: + kind = "other" + + if not title or not decision: + return { + "content": [ + {"type": "text", "text": "title and decision are required"} + ], + "is_error": True, + } + + approval_id, future = register( + session_id=session_id, + asker_agent_id=parent_agent_id, + parent_agent_id=parent_parent_agent_id, + question=f"[approval:{kind}] {title}: {decision}", + timeout_s=_APPROVAL_TIMEOUT_S, + ) + + # `content` carries the decision text so the persisted Message row + # (and the restore-on-reload path) has the card body; everything else + # lands in metadata. + await publish_fn( + session_id, + "approval_request", + { + "content": decision, + "approval_id": approval_id, + "title": title, + "kind": kind, + "context": context, + "asker_agent_id": parent_agent_id, + "asker_agent_type": parent_agent_type, + "depth": current_depth, + }, + role="system", + agent_meta=asker_meta, + ) + + try: + payload = await future + except asyncio.CancelledError: + payload = { + "decision": "cancelled", + "answer": "(session ended before the approval was answered)", + "answered_by": "session_ended", + "timeout": False, + } + + answer_text = (payload.get("answer") or "").strip() + outcome = payload.get("decision") + if outcome is None: + # Resolved through a path that didn't set an explicit decision + # (timeout, or the generic clarification endpoint): a timeout is + # a timeout; any real free-text reply is treated as an edit. + outcome = "timeout" if payload.get("timeout") else "edit" + + await publish_fn( + session_id, + "approval_resolved", + { + "approval_id": approval_id, + "decision": outcome, + "answer": answer_text, + "answered_by": payload.get("answered_by", "user"), + }, + role="system", + agent_meta=asker_meta, + ) + + if outcome == "approve": + return { + "content": [ + { + "type": "text", + "text": ( + f"APPROVED (approval_id={approval_id}) — the user " + "approved this decision. Proceed exactly as proposed." + ), + } + ] + } + if outcome == "edit": + return { + "content": [ + { + "type": "text", + "text": ( + f"EDITED (approval_id={approval_id}) — the user " + "revised the decision. Their revision REPLACES your " + f"proposal; follow it exactly:\n{answer_text}" + ), + } + ] + } + if outcome == "cancelled": + return { + "content": [ + { + "type": "text", + "text": f"CANCELLED (approval_id={approval_id}) — {answer_text}", + } + ], + "is_error": True, + } + # timeout + return { + "content": [ + { + "type": "text", + "text": ( + f"NO RESPONSE (approval_id={approval_id}) — the approval " + "window elapsed without an answer. Proceed with your " + "proposed decision, and clearly flag in your report that " + "it was not explicitly approved." + ), + } + ] + } + + return handler diff --git a/backend/skills/request-approval/schema.yaml b/backend/skills/request-approval/schema.yaml new file mode 100644 index 0000000..2f9cf1a --- /dev/null +++ b/backend/skills/request-approval/schema.yaml @@ -0,0 +1,27 @@ +type: object +properties: + title: + type: string + description: Short label for the approval card, e.g. "Target column". + decision: + type: string + description: > + The concrete decision you propose, stated so the user can approve it + as-is (e.g. "Predict `churned` as a binary target; drop `customer_id` + and `signup_date` as leakage."). Markdown allowed. + kind: + type: string + enum: + - target_column + - prep_plan + - model_shortlist + - other + description: What class of decision this gates. Drives the card's badge. + context: + type: string + description: > + Optional supporting context — why you chose this, alternatives you + considered and rejected. +required: + - title + - decision diff --git a/backend/tests/test_approvals.py b/backend/tests/test_approvals.py new file mode 100644 index 0000000..526a408 --- /dev/null +++ b/backend/tests/test_approvals.py @@ -0,0 +1,305 @@ +"""HITL approval-gate tests (issue #108).""" + +import asyncio +from unittest.mock import AsyncMock, patch + +import pytest + +from services import approvals as approvals_svc +from services import clarifications +from services.approvals import ( + APPROVAL_GATE_PROMPT, + APPROVAL_SKILL_SLUG, + apply_approval_gate, + is_enabled, + set_enabled, +) +from services.skills import get_skill, load_handler + + +async def _create_experiment(client, sample_csv, project_id): + with open(sample_csv, "rb") as f: + resp = await client.post( + "/api/experiments", + data={ + "project_id": project_id, + "name": "Approvals Test", + "description": "", + "instructions": "test", + }, + files={"files": ("data.csv", f, "text/csv")}, + ) + body = resp.json() + return body["id"], body["session_id"] + + +@pytest.fixture(autouse=True) +def _clean_approval_flags(): + """Approval flags are process-global — isolate every test.""" + approvals_svc._enabled_sessions.clear() + yield + approvals_svc._enabled_sessions.clear() + + +class _PublishRecorder: + def __init__(self): + self.events = [] + + async def __call__(self, session_id, event_type, data, role=None, **kwargs): + self.events.append({"type": event_type, "data": data, "role": role}) + + +# --------------------------------------------------------------------------- +# Flag + gate injection +# --------------------------------------------------------------------------- + + +def test_gate_is_identity_when_disabled(): + """Default path is byte-identical: same skills content, same prompt object.""" + skills = ["execute-code", "tasks"] + prompt = "You are an agent." + out_skills, out_prompt = apply_approval_gate("session-x", skills, prompt) + assert out_skills is skills # not even copied + assert out_prompt is prompt # same string object — provably unchanged + assert APPROVAL_SKILL_SLUG not in out_skills + + +def test_gate_injects_skill_and_prompt_when_enabled(): + set_enabled("session-y", True) + skills = ["execute-code", "tasks"] + prompt = "You are an agent." + out_skills, out_prompt = apply_approval_gate("session-y", skills, prompt) + assert out_skills == ["execute-code", "tasks", APPROVAL_SKILL_SLUG] + assert skills == ["execute-code", "tasks"] # input not mutated + assert out_prompt == prompt + "\n\n" + APPROVAL_GATE_PROMPT + # No double-append if the skill is somehow already present + again, _ = apply_approval_gate("session-y", out_skills, prompt) + assert again.count(APPROVAL_SKILL_SLUG) == 1 + + +def test_set_enabled_toggles(): + assert not is_enabled("s1") + set_enabled("s1", True) + assert is_enabled("s1") + set_enabled("s1", False) + assert not is_enabled("s1") + + +def test_skill_is_discoverable(): + skill = get_skill(APPROVAL_SKILL_SLUG) + assert skill.has_handler + assert skill.schema["required"] == ["title", "decision"] + + +# --------------------------------------------------------------------------- +# send_message wiring +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_send_message_default_leaves_gates_off( + client, sample_csv, default_project_id +): + """A message without the approvals field must not enable gates.""" + _, session_id = await _create_experiment(client, sample_csv, default_project_id) + with patch("routers.sessions.run_agent", new_callable=AsyncMock): + resp = await client.post( + f"/api/sessions/{session_id}/messages", + json={"content": "go", "run_agent": True}, + ) + assert resp.status_code == 200 + from services.agent.tasks import _running_tasks + + await asyncio.wait_for(_running_tasks[session_id], timeout=5) + assert not is_enabled(session_id) + + +@pytest.mark.asyncio +async def test_send_message_with_approvals_enables_and_disables( + client, sample_csv, default_project_id +): + _, session_id = await _create_experiment(client, sample_csv, default_project_id) + from services.agent.tasks import _running_tasks + + with patch("routers.sessions.run_agent", new_callable=AsyncMock): + resp = await client.post( + f"/api/sessions/{session_id}/messages", + json={"content": "go", "run_agent": True, "approvals": True}, + ) + assert resp.status_code == 200 + await asyncio.wait_for(_running_tasks[session_id], timeout=5) + assert is_enabled(session_id) + + # Toggle off on the next message → flag drops back to default. + resp = await client.post( + f"/api/sessions/{session_id}/messages", + json={"content": "go again", "run_agent": True}, + ) + assert resp.status_code == 200 + await asyncio.wait_for(_running_tasks[session_id], timeout=5) + assert not is_enabled(session_id) + + +# --------------------------------------------------------------------------- +# Skill handler — block / approve / edit / timeout +# --------------------------------------------------------------------------- + + +def _make_handler(session_id: str, recorder: _PublishRecorder): + create_handler = load_handler(APPROVAL_SKILL_SLUG) + return create_handler( + session_id=session_id, + publish_fn=recorder, + parent_agent_type="orchestrator", + parent_agent_id="root", + parent_parent_agent_id=None, + current_depth=0, + ) + + +@pytest.mark.asyncio +async def test_handler_blocks_until_approved(): + recorder = _PublishRecorder() + handler = _make_handler("sess-appr-1", recorder) + + task = asyncio.create_task( + handler( + { + "title": "Target column", + "decision": "Predict `churned` as a binary target.", + "kind": "target_column", + "context": "Only plausible label in the schema.", + } + ) + ) + await asyncio.sleep(0.05) + assert not task.done() # blocked on the future + + # The request card was published with the decision as content. + reqs = [e for e in recorder.events if e["type"] == "approval_request"] + assert len(reqs) == 1 + req = reqs[0]["data"] + assert req["title"] == "Target column" + assert req["kind"] == "target_column" + assert req["content"] == "Predict `churned` as a binary target." + approval_id = req["approval_id"] + + # Approve through the shared clarifications registry (what the route does). + assert clarifications.resolve( + "sess-appr-1", + approval_id, + {"decision": "approve", "answer": "", "answered_by": "user", "timeout": False}, + ) + result = await asyncio.wait_for(task, timeout=5) + assert "APPROVED" in result["content"][0]["text"] + assert not result.get("is_error") + + resolved = [e for e in recorder.events if e["type"] == "approval_resolved"] + assert len(resolved) == 1 + assert resolved[0]["data"]["decision"] == "approve" + + +@pytest.mark.asyncio +async def test_handler_edit_returns_revision(): + recorder = _PublishRecorder() + handler = _make_handler("sess-appr-2", recorder) + task = asyncio.create_task( + handler({"title": "Model shortlist", "decision": "XGBoost only."}) + ) + await asyncio.sleep(0.05) + approval_id = recorder.events[0]["data"]["approval_id"] + + clarifications.resolve( + "sess-appr-2", + approval_id, + { + "decision": "edit", + "answer": "Also include LightGBM and a logistic baseline.", + "answered_by": "user", + "timeout": False, + }, + ) + result = await asyncio.wait_for(task, timeout=5) + text = result["content"][0]["text"] + assert "EDITED" in text + assert "Also include LightGBM" in text + + +@pytest.mark.asyncio +async def test_handler_timeout_tells_agent_to_flag(): + recorder = _PublishRecorder() + handler = _make_handler("sess-appr-3", recorder) + # load_handler execs the module without registering it in sys.modules, so + # patch the constant through the closure's module globals instead. + with patch.dict(handler.__globals__, {"_APPROVAL_TIMEOUT_S": 0.05}): + result = await asyncio.wait_for( + handler({"title": "Prep plan", "decision": "Drop rows with NaN."}), + timeout=5, + ) + text = result["content"][0]["text"] + assert "NO RESPONSE" in text + assert "not explicitly approved" in text + resolved = [e for e in recorder.events if e["type"] == "approval_resolved"] + assert resolved[0]["data"]["decision"] == "timeout" + + +@pytest.mark.asyncio +async def test_handler_requires_title_and_decision(): + recorder = _PublishRecorder() + handler = _make_handler("sess-appr-4", recorder) + result = await handler({"title": "", "decision": ""}) + assert result["is_error"] + assert recorder.events == [] # nothing published, nothing pending + + +# --------------------------------------------------------------------------- +# Resolve endpoint +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_approval_endpoint_resolves_pending(client): + approval_id, future = clarifications.register( + session_id="sess-appr-5", + asker_agent_id="root", + parent_agent_id=None, + question="[approval:other] t: d", + timeout_s=30, + ) + resp = await client.post( + f"/api/sessions/sess-appr-5/approvals/{approval_id}", + json={"decision": "approve"}, + ) + assert resp.status_code == 200 + assert resp.json() == {"status": "ok", "decision": "approve"} + payload = await asyncio.wait_for(future, timeout=5) + assert payload["decision"] == "approve" + assert payload["answered_by"] == "user" + + +@pytest.mark.asyncio +async def test_approval_endpoint_404_when_unknown(client): + resp = await client.post( + "/api/sessions/sess-x/approvals/deadbeef", + json={"decision": "approve"}, + ) + assert resp.status_code == 404 + + +@pytest.mark.asyncio +async def test_approval_endpoint_edit_requires_edits(client): + approval_id, future = clarifications.register( + session_id="sess-appr-6", + asker_agent_id="root", + parent_agent_id=None, + question="q", + timeout_s=30, + ) + resp = await client.post( + f"/api/sessions/sess-appr-6/approvals/{approval_id}", + json={"decision": "edit", "edits": " "}, + ) + assert resp.status_code == 400 + assert not future.done() # still pending — bad request didn't consume it + # Clean up the pending future so it doesn't leak across tests. + clarifications.cancel_session("sess-appr-6") diff --git a/frontend/src/app/page.tsx b/frontend/src/app/page.tsx index de1d0aa..fe91211 100644 --- a/frontend/src/app/page.tsx +++ b/frontend/src/app/page.tsx @@ -55,6 +55,14 @@ function HomePageContent() { }, [agentThinking]); const [draft, setDraft] = useState([]); + // HITL approval gates (issue #108). Opt-in per session; OFF by default so + // legacy behavior is unchanged. Sent with every run-triggering message — + // the backend re-asserts the per-session flag from it on each launch. + const [approvalsEnabled, setApprovalsEnabled] = useState(false); + const approvalsEnabledRef = useRef(false); + useEffect(() => { + approvalsEnabledRef.current = approvalsEnabled; + }, [approvalsEnabled]); // Derive the displayed experiment name from the experiments list (single // source of truth) so a rename in the sidebar updates the header without // needing a session reload. @@ -158,6 +166,7 @@ function HomePageContent() { agentModelsRef.current, pending.mentions, agentThinkingRef.current, + approvalsEnabledRef.current, ) .catch((e: any) => { addItem({ type: 'error', content: e.message }); @@ -198,6 +207,7 @@ function HomePageContent() { agentModelsRef.current, undefined, agentThinkingRef.current, + approvalsEnabledRef.current, ); } catch (e: any) { addItem({ type: 'error', content: e.message }); @@ -340,6 +350,7 @@ function HomePageContent() { agentModelsRef.current, mentions, agentThinkingRef.current, + approvalsEnabledRef.current, ); } catch (e: any) { addItem({ type: 'error', content: e.message }); @@ -431,6 +442,7 @@ function HomePageContent() { agentModelsRef.current, draftMentions, agentThinkingRef.current, + approvalsEnabledRef.current, ); } catch (e: any) { addItem({ type: 'error', content: e.message }); @@ -494,6 +506,7 @@ function HomePageContent() { agentModelsRef.current, undefined, agentThinkingRef.current, + approvalsEnabledRef.current, ); } } @@ -676,6 +689,8 @@ function HomePageContent() { onStop={handleStop} sessionState={sessionState} onResume={handleResume} + approvalsEnabled={approvalsEnabled} + onToggleApprovals={() => setApprovalsEnabled((v) => !v)} attachedFiles={attachedFiles} onRemoveAttachedFile={removeAttachedFile} onClearAttachedFiles={() => setAttachedFiles([])} diff --git a/frontend/src/components/chat/ApprovalCard.tsx b/frontend/src/components/chat/ApprovalCard.tsx new file mode 100644 index 0000000..7b454b5 --- /dev/null +++ b/frontend/src/components/chat/ApprovalCard.tsx @@ -0,0 +1,212 @@ +'use client'; + +import { useEffect, useState } from 'react'; +import { Check, CheckCircle2, Clock, Loader2, Pencil, ShieldCheck, XCircle } from 'lucide-react'; +import ReactMarkdown from 'react-markdown'; +import remarkGfm from 'remark-gfm'; +import ErrorBoundary from '@/components/ErrorBoundary'; +import { api } from '@/lib/api'; +import type { ChatItem } from '@/lib/chatItems'; +import { AGENT_COLORS, AGENT_META } from '@/components/chat/agentMeta'; + +// --------------------------------------------------------------------------- +// ApprovalCard — HITL approval gate (issue #108). The agent proposed a +// consequential decision (target column, prep plan, model shortlist, …) and +// is BLOCKED until the user approves it as-is or submits a revision. +// --------------------------------------------------------------------------- + +const MARKDOWN_PLUGINS = [remarkGfm]; + +const KIND_LABEL: Record = { + target_column: 'Target column', + prep_plan: 'Prep plan', + model_shortlist: 'Model shortlist', + other: 'Decision', +}; + +export default function ApprovalCard({ + item, + sessionId, +}: { + item: ChatItem; + sessionId: string | null; +}) { + const [editing, setEditing] = useState(false); + const [edits, setEdits] = useState(''); + const [submitting, setSubmitting] = useState(false); + const [localStatus, setLocalStatus] = useState<'pending' | 'sent' | 'resolved'>( + item.meta?.status === 'resolved' ? 'resolved' : 'pending', + ); + + useEffect(() => { + if (item.meta?.status === 'resolved') setLocalStatus('resolved'); + }, [item.meta?.status]); + + const askerType = item.meta?.asker_agent_type || 'agent'; + const askerLabel = AGENT_META[askerType]?.label || askerType; + const askerColor = AGENT_COLORS[AGENT_META[askerType]?.color || 'teal']; + const kind = item.meta?.kind || 'other'; + const decision = item.meta?.decision; + + async function send(verdict: 'approve' | 'edit') { + if (!sessionId || !item.meta?.approval_id) return; + if (verdict === 'edit' && !edits.trim()) return; + setSubmitting(true); + try { + await api.replyApproval( + sessionId, + item.meta.approval_id, + verdict, + verdict === 'edit' ? edits.trim() : undefined, + ); + setLocalStatus('sent'); + } catch (e) { + console.error('Failed to send approval verdict', e); + } finally { + setSubmitting(false); + } + } + + const isResolved = localStatus === 'resolved'; + const isSent = localStatus === 'sent'; + + return ( +
+
+ {/* Header */} +
+
+ +
+
+ {askerLabel} + + needs your approval + + + {KIND_LABEL[kind] || KIND_LABEL.other} + +
+ {isResolved && + (decision === 'approve' ? ( + + ) : decision === 'edit' ? ( + + ) : decision === 'timeout' ? ( + + ) : ( + + ))} +
+ + {/* Proposed decision */} + {item.meta?.title && ( +
{item.meta.title}
+ )} +
+
{item.content}
} + > + {item.content} + +
+ {item.meta?.context && ( +
Why: {item.meta.context}
+ )} + + {/* Verdict / actions */} + {isResolved ? ( +
+ {decision === 'approve' ? ( + Approved — proceeding as proposed. + ) : decision === 'edit' ? ( + <> + Edited: + {item.meta?.answer || '(no revision text)'} + + ) : decision === 'timeout' ? ( + + No response — the agent proceeded with its proposal (unapproved). + + ) : ( + Cancelled with the session. + )} +
+ ) : isSent ? ( +
+ Sent — the agent is resuming… +
+ ) : ( +
+ {editing && ( +