From b0c9f46fd2bcfb2f5cc24984585f3a8c265f3732 Mon Sep 17 00:00:00 2001 From: Prerit-112 Date: Wed, 22 Jul 2026 16:49:28 +0530 Subject: [PATCH 1/2] Add human-approval wait for durable graph side effects. Park an approval node until approve/deny, prove future work is not scheduled early, and reject late approve after deny. Co-authored-by: Cursor --- .gitignore | 4 + README.md | 57 ++++++++++++--- s13code/routes.py | 24 ++++++ s13code/runtime.py | 137 ++++++++++++++++++++++++++++++++++- tests/test_human_approval.py | 103 ++++++++++++++++++++++++++ 5 files changed, 312 insertions(+), 13 deletions(-) create mode 100644 tests/test_human_approval.py diff --git a/.gitignore b/.gitignore index b112038..63f58aa 100644 --- a/.gitignore +++ b/.gitignore @@ -14,4 +14,8 @@ htmlcov/ benchmark.json benchmark.md a2a-proof.json +part1_traces/ +assignment.txt +lecture.txt +Transcript.md .DS_Store diff --git a/README.md b/README.md index 7cf7872..d256b23 100644 --- a/README.md +++ b/README.md @@ -112,17 +112,56 @@ uv run pytest -q Fork the official [`theschoolofai/S13Code`](https://github.com/theschoolofai/S13Code) repository linked from Axiom, create a branch, implement one meaningful extension, and open one pull request against that repository. Do not open the Session 13 pull request against [`theschoolofai/glc_v3`](https://github.com/theschoolofai/glc_v3). -Add one subsection to this README in the same pull request. It must contain: +Do not commit `.env`, credentials, personal memory, generated databases, unrestricted local paths, benchmark output containing private data, or provider responses containing secrets. Use synthetic identities in every proof. -1. the user-visible capability, -2. the exact prompt or API request, -3. the graph and ordered event trace, -4. the actual final result, -5. evidence and provider/agent assignments, -6. the adversarial failure and its fix, -7. commands that reproduce the result from a fresh checkout. +### Human-approval wait (live graph) -Do not commit `.env`, credentials, personal memory, generated databases, unrestricted local paths, benchmark output containing private data, or provider responses containing secrets. Use synthetic identities in every proof. +**Capability.** Prompts that require human approval park a first-class `approval` node in `waiting`. No remember/answer (or other side-effect) nodes exist until a human calls the approval API. Approve resumes the node and expands the graph; deny cancels the gate and finishes the run with `status=denied`. Part 1 floor traces showed the planner had no approval surface for durable side effects; this extension adds that wait/resume path without dumping Part 1 artifacts into the PR. + +**Exact request.** + +```bash +curl -s http://127.0.0.1:8113/v1/agent/runs \ + -H 'Content-Type: application/json' \ + -d '{ + "tenant_id": "course", + "project_id": "s13-approval-live", + "user_id": "student-01", + "prompt": "Requires human approval: My mom'\''s birthday is 15 May 2026. Remember that." + }' +``` + +Parked response: `status=waiting`, `waiting_nodes=["approval"]`, graph nodes `{approval}` only. + +```bash +curl -s http://127.0.0.1:8113/v1/agent/runs//approvals/approval \ + -H 'Content-Type: application/json' \ + -d '{"decision":"approve","comment":"synthetic-reviewer-ok"}' +``` + +**Graph and ordered events (live proof `run-08d8c81cf7f4`).** + +- Nodes after approve: `approval → remember → answer` (edges `approval→remember`, `remember→answer`; answer also cites the approval decision). +- Event order: `run_started → graph_patched(wait=approval) → human_approval_granted → graph_patched(resume) → run_resumed → task_started/succeeded(approval) → graph_patched(add remember) → … → task_succeeded(answer) → graph_patched(finish)`. + +**Final result.** `status=completed`. Answer cited the approved, sourced fact (Gemini via `glc_v3`). Approval result: `{"decision":"approved","comment":"synthetic-reviewer-ok","agent":"human_approver"}`. Remember wrote `Mom's birthday is 15 May 2026.` with source `api://agent/runs`. + +**Evidence / assignments.** `trace.agents.approval.agent=human_approver`, `remember` / `answer` local skills; answer provider `gemini_1` / model `gemini-2.5-flash`. Evidence includes the human decision plus the promoted fact. + +**Adversarial failure and fix.** After `decision=deny`, a late `decision=approve` returns HTTP `409` (`node 'approval' is cancelled, not waiting`). The cancelled gate is not resumed; `remember`/`answer` never appear. Covered by `tests/test_human_approval.py::test_adversarial_approve_after_deny_is_rejected`. + +**Reproduce from a fresh checkout.** + +```bash +uv sync +uv run ruff check . +uv run pytest -q tests/test_human_approval.py + +# Live path (glc_v3 on :8111): +uv run s13code serve +# then the two curl requests above (park → approve). For deny + late-approve 409: +# POST .../approvals/approval {"decision":"deny"} then the same URL with approve. +``` ## License diff --git a/s13code/routes.py b/s13code/routes.py index 1570088..744c36a 100644 --- a/s13code/routes.py +++ b/s13code/routes.py @@ -32,6 +32,11 @@ class ResumeBody(BaseModel): run_id: str = Field(min_length=1, max_length=128) +class ApprovalBody(BaseModel): + decision: str = Field(min_length=1, max_length=16, description="approve or deny") + comment: str | None = Field(default=None, max_length=4_000) + + class FactBody(ScopeBody): text: str = Field(min_length=1, max_length=20_000) source_uri: str @@ -75,6 +80,25 @@ async def resume(run_id: str, request: Request): raise HTTPException(503, str(error)) from error +@router.post("/runs/{run_id}/approvals/{node_id}") +async def decide_approval(run_id: str, node_id: str, body: ApprovalBody, request: Request): + """Human decision for a parked human_approval node (approve resumes, deny cancels).""" + runtime = request.app.state.s13_runtime + try: + return await runtime.apply_human_decision( + run_id=run_id, node_id=node_id, decision=body.decision, comment=body.comment, + llm=lambda prompt, system: gateway_text_llm(request.app, prompt, system), + ) + except KeyError: + raise HTTPException(404, "run or approval node not found") from None + except PermissionError as error: + raise HTTPException(409, str(error)) from error + except ValueError as error: + raise HTTPException(400, str(error)) from error + except RuntimeError as error: + raise HTTPException(503, str(error)) from error + + @router.post("/facts") async def fact(body: FactBody, request: Request): return request.app.state.s13_runtime.remember_fact(text=body.text, scope=body.scope(), source_uri=body.source_uri, diff --git a/s13code/runtime.py b/s13code/runtime.py index 45c16e3..cc21014 100644 --- a/s13code/runtime.py +++ b/s13code/runtime.py @@ -23,6 +23,15 @@ TextLLM = Callable[[str, str], Awaitable[dict[str, Any]]] +def _needs_human_approval(prompt: str) -> bool: + """True when the user explicitly gates the request on a human decision.""" + return bool(re.search( + r"\b(?:requires?|needs?|ask(?:\s+me)?\s+for)\s+human\s+approval\b|" + r"\bonly after (?:my |human )?approval\b", + prompt, re.IGNORECASE, + )) + + def _work_intent(prompt: str) -> tuple[str, list[TaskSpec]]: """Choose the first useful frontier from the non-browser skill surface. @@ -31,6 +40,14 @@ def _work_intent(prompt: str) -> tuple[str, list[TaskSpec]]: network tools outside this registry. """ lower = prompt.lower() + if _needs_human_approval(prompt): + # Park only the gate. Future work is planned after human_approval succeeds, + # so speculative nodes do not exist before the decision lands. + return "human_approval", [TaskSpec( + "approval", "human_approval", + {"prompt": prompt, "question": "Approve this request before any further work runs?"}, + {"agent": "human_approver", "kind": "human_approval"}, + )] index_directory = re.search( r"\bindex every\s+(\.[a-z0-9]+)\s+file\s+under\s+[`'\"]?([^\s`'\",]+)", prompt, re.IGNORECASE, @@ -139,10 +156,30 @@ def answer_patch(graph, *, reason: str) -> GraphPatch: async def plan(self, graph, event): if event.kind == "run_started": first = list(initial_frontier) + if mode == "human_approval": + # Add+wait in one patch: the approval node never becomes ready. + return GraphPatch( + add=tuple(first), + wait=tuple(task.id for task in first), + reason="park until a human approves or denies the request", + ) if explicit_memory: first.append(TaskSpec("remember", "remember_explicit_fact", {"text": prompt})) return GraphPatch(add=tuple(first), reason=f"first frontier selected for {mode}") + if mode == "human_approval" and event.node_id == "approval" and event.kind == "task_succeeded": + # Only now may durable side effects and the answer exist. + follow: list[TaskSpec] = [] + if explicit_memory: + follow.append(TaskSpec("remember", "remember_explicit_fact", {"text": prompt})) + return GraphPatch( + add=tuple(follow), + connect=(("approval", "remember"),), + reason="human approved; promote the explicit fact", + ) + return self.answer_patch(graph, reason="human approved; produce the gated answer") + if mode == "human_approval" and event.node_id == "remember" and event.kind == "task_succeeded": + return self.answer_patch(graph, reason="approved fact is durable; answer with evidence") if event.node_id == "index_file" and event.kind == "task_succeeded": return GraphPatch(add=(TaskSpec("recall", "memory_recall", {"query": prompt}),), connect=(("index_file", "recall"),), @@ -272,6 +309,10 @@ async def answer(_: TaskSpec) -> dict[str, Any]: elif node["skill"] == "create_reminder" and result.get("artifacts"): evidence.append({"text": "Calendar reminders created: " + ", ".join(result["artifacts"]), "sources": result["artifacts"], "kind": "calendar_artifact"}) + elif node["skill"] == "human_approval" and result.get("decision"): + evidence.append({"text": f"Human decision: {result['decision']}" + + (f" ({result['comment']})" if result.get("comment") else ""), + "sources": [f"graph://{run_id}/{node_id}"], "kind": "human_approval"}) elif node["state"] == "failed": evidence.append({"text": result.get("error", "task failed"), "sources": [f"graph://{run_id}/{node_id}"], "kind": "failure"}) @@ -401,6 +442,23 @@ async def run_retriever(task: TaskSpec) -> dict[str, Any]: return {**hits, "text": result.get("text", ""), "provider": result.get("provider"), "model": result.get("model"), "agent": "retriever"} + async def run_human_approval(task: TaskSpec) -> dict[str, Any]: + """Materialise a granted decision that already landed in the journal. + + Deny cancels this node before it runs. A late approve after cancel is + rejected by the HTTP seam, so this skill only ever records approval. + """ + decision_event = None + for event in reversed(runtime.graph.events(run_id)): + if event.node_id == task.id and event.kind == "human_approval_granted": + decision_event = event + break + if decision_event is None: + raise RuntimeError("human_approval skill ran without a granted decision event") + comment = (decision_event.payload or {}).get("comment") or "" + return {"decision": "approved", "comment": comment, + "question": task.input.get("question", ""), "agent": "human_approver"} + deterministic = DeterministicPlanner() planner: Any = deterministic if os.getenv("S13_PLANNER_LLM", "0").lower() in {"1", "true", "yes"}: @@ -410,21 +468,92 @@ async def run_retriever(task: TaskSpec) -> dict[str, Any]: "memory_recall": recall, "remember_explicit_fact": remember_explicit, "web_search": run_search, "fetch_url": run_fetch, "index_file": run_index, "list_directory": list_directory, "read_file": run_read_file, "create_reminder": create_reminder, + "human_approval": run_human_approval, "answer_with_evidence": answer, "researcher": run_researcher, "retriever": run_retriever, **role_workers, }, max_workers=int(os.getenv("S13_MAX_WORKERS", "4"))).run(run_id, resume=resume) + return self._run_response(run_id, report, planner) + + def _run_response(self, run_id: str, report: Any, planner: Any) -> dict[str, Any]: snapshot = self.graph.snapshot(run_id) answer = snapshot.nodes.get("answer", {}).get("result", {}) or snapshot.nodes.get("formatter", {}).get("result", {}) answer_state = snapshot.nodes.get("answer", {}).get("state") or snapshot.nodes.get("formatter", {}).get("state") + waiting = list(report.waiting) if getattr(report, "waiting", None) else [ + node_id for node_id, node in snapshot.nodes.items() if node["state"] == "waiting" + ] + if waiting and not snapshot.finished: + status = "waiting" + elif answer_state == "succeeded": + status = "completed" + elif snapshot.finished and any( + node.get("metadata", {}).get("kind") == "human_approval" and node["state"] == "cancelled" + for node in snapshot.nodes.values() + ) and answer_state != "succeeded": + status = "denied" + else: + status = "failed" trace = {node_id: {"agent": node.get("metadata", {}).get("agent", node["skill"]), "skill": node["skill"], "state": node["state"], "provider": (node.get("result") or {}).get("provider"), "model": (node.get("result") or {}).get("model")} for node_id, node in snapshot.nodes.items()} - return {"run_id": run_id, "status": "completed" if answer_state == "succeeded" else "failed", - "answer": answer.get("answer", ""), "provider": answer.get("provider"), - "model": answer.get("model"), "graph": {"finished": report.finished, "nodes": snapshot.nodes, - "edges": snapshot.edges}, "trace": {"planner": getattr(planner, "last_selection", {"mode": "deterministic"}), + denied_answer = "" + if status == "denied": + denied_answer = "Request denied by human approver; no further graph work was scheduled." + return {"run_id": run_id, "status": status, + "answer": answer.get("answer", "") if status != "denied" else denied_answer, + "waiting_nodes": waiting, + "provider": answer.get("provider"), + "model": answer.get("model"), "graph": {"finished": report.finished if hasattr(report, "finished") else snapshot.finished, + "nodes": snapshot.nodes, "edges": snapshot.edges}, + "trace": {"planner": getattr(planner, "last_selection", {"mode": "deterministic"}), "agents": trace}, "events": [event.__dict__ for event in self.graph.events(run_id)]} + async def apply_human_decision(self, *, run_id: str, node_id: str, decision: str, + comment: str | None, llm: TextLLM) -> dict[str, Any]: + """Resume or cancel a parked human_approval node, then continue the run.""" + try: + snapshot = self.graph.snapshot(run_id) + except KeyError as error: + raise KeyError(run_id) from error + node = snapshot.nodes.get(node_id) + if node is None: + raise KeyError(node_id) + if node.get("skill") != "human_approval" and node.get("metadata", {}).get("kind") != "human_approval": + raise ValueError(f"node {node_id!r} is not a human-approval gate") + if node["state"] != "waiting": + raise PermissionError(f"node {node_id!r} is {node['state']}, not waiting") + if snapshot.finished: + raise PermissionError(f"run {run_id!r} is already finished") + + decision = decision.lower().strip() + if decision not in {"approve", "deny"}: + raise ValueError("decision must be approve or deny") + payload = {"decision": decision, "comment": comment or ""} + if decision == "approve": + event = self.graph.record_external_event(run_id, "human_approval_granted", node_id, payload) + if self.graph.node_state(run_id, node_id) == "waiting": + self.graph.apply_patch( + run_id, + GraphPatch(resume=(node_id,), reason="human approved the parked request"), + trigger_event=event.sequence, + ) + return await self.run(prompt=None, scope=None, llm=llm, source_uri=None, + source_author=None, run_id=run_id, resume=True) + + event = self.graph.record_external_event(run_id, "human_approval_denied", node_id, payload) + cancel_ids = tuple( + nid for nid, item in snapshot.nodes.items() + if item["state"] in {"pending", "waiting", "running"} + ) + if self.graph.node_state(run_id, node_id) == "waiting": + self.graph.apply_patch( + run_id, + GraphPatch(cancel=cancel_ids, finish=True, reason="human denied; cancel parked work"), + trigger_event=event.sequence, + ) + # Synthetic planner handle for response shape; deny never re-enters the executor. + return self._run_response(run_id, type("R", (), {"finished": True, "waiting": ()})(), + type("P", (), {"last_selection": {"mode": "deterministic"}})()) + def remember_fact(self, *, text: str, scope: MemoryScope, source_uri: str, source_author: str, principal: Principal, supersedes_id: str | None = None) -> dict[str, Any]: record = self.memory.write(MemoryRecord(MemoryKind.FACT, scope, text, diff --git a/tests/test_human_approval.py b/tests/test_human_approval.py new file mode 100644 index 0000000..7f2486f --- /dev/null +++ b/tests/test_human_approval.py @@ -0,0 +1,103 @@ +"""Human-approval wait: park, approve/deny, no early future nodes, adversarial late approve.""" +from __future__ import annotations + +import s13code.routes as agent_route +from s13code.core.memory.embeddings import DeterministicEmbedder + +SCOPE = {"tenant_id": "course", "project_id": "s13-approval", "user_id": "student-01"} +PROMPT = ( + "Requires human approval: My mom's birthday is 15 May 2026. Remember that." +) + + +def test_human_approval_parks_without_future_work(app_client, monkeypatch): + app_client.app.state.s13_runtime.memory.embedder = DeterministicEmbedder(128) + + async def fake_gateway(_app, prompt: str, _system: str): + raise AssertionError(f"LLM must not run while approval is parked: {prompt[:80]}") + + monkeypatch.setattr(agent_route, "gateway_text_llm", fake_gateway) + parked = app_client.post("/v1/agent/runs", json={**SCOPE, "prompt": PROMPT}) + assert parked.status_code == 200 + body = parked.json() + assert body["status"] == "waiting" + assert body["waiting_nodes"] == ["approval"] + assert body["graph"]["nodes"]["approval"]["state"] == "waiting" + # Future side effects do not exist before the human decides. + assert "remember" not in body["graph"]["nodes"] + assert "answer" not in body["graph"]["nodes"] + assert any(event["kind"] == "graph_patched" and "approval" in (event["payload"].get("wait") or []) + for event in body["events"]) + + +def test_human_approval_resume_expands_only_after_grant(app_client, monkeypatch): + app_client.app.state.s13_runtime.memory.embedder = DeterministicEmbedder(128) + + async def fake_gateway(_app, prompt: str, _system: str): + assert "15 May 2026" in prompt + assert "Human decision: approved" in prompt + return {"text": "You told me mom's birthday is 15 May 2026. [source: api://agent/runs]", + "provider": "fake", "model": "fake"} + + monkeypatch.setattr(agent_route, "gateway_text_llm", fake_gateway) + parked = app_client.post("/v1/agent/runs", json={**SCOPE, "prompt": PROMPT}).json() + run_id = parked["run_id"] + + done = app_client.post(f"/v1/agent/runs/{run_id}/approvals/approval", + json={"decision": "approve", "comment": "synthetic-reviewer-ok"}) + assert done.status_code == 200 + body = done.json() + assert body["status"] == "completed" + assert body["graph"]["nodes"]["approval"]["state"] == "succeeded" + assert body["graph"]["nodes"]["approval"]["result"]["decision"] == "approved" + assert body["graph"]["nodes"]["remember"]["state"] == "succeeded" + assert body["graph"]["nodes"]["answer"]["state"] == "succeeded" + assert "15 May 2026" in body["answer"] + kinds = [event["kind"] for event in body["events"]] + assert "human_approval_granted" in kinds + assert kinds.index("human_approval_granted") < kinds.index("task_started") + + +def test_human_approval_deny_cancels_without_side_effects(app_client, monkeypatch): + app_client.app.state.s13_runtime.memory.embedder = DeterministicEmbedder(128) + + async def fake_gateway(_app, prompt: str, _system: str): + raise AssertionError("deny must not call the answer LLM") + + monkeypatch.setattr(agent_route, "gateway_text_llm", fake_gateway) + parked = app_client.post("/v1/agent/runs", json={**SCOPE, "prompt": PROMPT}).json() + run_id = parked["run_id"] + + denied = app_client.post(f"/v1/agent/runs/{run_id}/approvals/approval", + json={"decision": "deny", "comment": "synthetic-reviewer-no"}) + assert denied.status_code == 200 + body = denied.json() + assert body["status"] == "denied" + assert body["graph"]["finished"] is True + assert body["graph"]["nodes"]["approval"]["state"] == "cancelled" + assert "remember" not in body["graph"]["nodes"] + assert "answer" not in body["graph"]["nodes"] + assert any(event["kind"] == "human_approval_denied" for event in body["events"]) + + +def test_adversarial_approve_after_deny_is_rejected(app_client, monkeypatch): + app_client.app.state.s13_runtime.memory.embedder = DeterministicEmbedder(128) + + async def fake_gateway(_app, prompt: str, _system: str): + raise AssertionError("no llm on deny path") + + monkeypatch.setattr(agent_route, "gateway_text_llm", fake_gateway) + + parked = app_client.post("/v1/agent/runs", json={**SCOPE, "prompt": PROMPT}).json() + run_id = parked["run_id"] + assert app_client.post(f"/v1/agent/runs/{run_id}/approvals/approval", + json={"decision": "deny"}).status_code == 200 + + # Late / duplicate decision after cancellation must not revive the graph. + late = app_client.post(f"/v1/agent/runs/{run_id}/approvals/approval", + json={"decision": "approve", "comment": "too-late"}) + assert late.status_code == 409 + snapshot = app_client.get(f"/v1/agent/runs/{run_id}").json() + assert snapshot["finished"] is True + assert snapshot["nodes"]["approval"]["state"] == "cancelled" + assert "remember" not in snapshot["nodes"] From c90bd052eeb1ea65251846bf6c50f2192792b713 Mon Sep 17 00:00:00 2001 From: Prerit-112 Date: Mon, 10 Aug 2026 17:24:15 +0530 Subject: [PATCH 2/2] Document adversarial before/after for human-approval evidence. Clarify Part 1 gap vs late-approve-after-deny 409 so the PR proof checklist is complete. Co-authored-by: Cursor --- README.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index d256b23..2e4f661 100644 --- a/README.md +++ b/README.md @@ -148,19 +148,26 @@ curl -s http://127.0.0.1:8113/v1/agent/runs//approvals/approval \ **Evidence / assignments.** `trace.agents.approval.agent=human_approver`, `remember` / `answer` local skills; answer provider `gemini_1` / model `gemini-2.5-flash`. Evidence includes the human decision plus the promoted fact. -**Adversarial failure and fix.** After `decision=deny`, a late `decision=approve` returns HTTP `409` (`node 'approval' is cancelled, not waiting`). The cancelled gate is not resumed; `remember`/`answer` never appear. Covered by `tests/test_human_approval.py::test_adversarial_approve_after_deny_is_rejected`. +**Adversarial failure before the fix / same attack after.** + +- **Before (Part 1 floor / no approval gate):** a remember prompt scheduled `remember`/`answer` immediately with no waiting node, so durable side effects ran without a human decision. There was also no terminal cancel path: a late second decision had nothing to reject against. +- **Attack after deny:** `POST .../approvals/approval` with `{"decision":"deny"}`, then the same URL with `{"decision":"approve","comment":"too-late"}`. +- **After this change:** deny finishes the run (`status=denied`, `approval` cancelled, no `remember`/`answer`). The late approve returns HTTP `409` with `node 'approval' is cancelled, not waiting` (live proof `04_late_approve.json`). The cancelled gate is not resumed. Covered by `tests/test_human_approval.py::test_adversarial_approve_after_deny_is_rejected`. **Reproduce from a fresh checkout.** ```bash uv sync uv run ruff check . +uv run pytest -q +# focused approval proof: uv run pytest -q tests/test_human_approval.py # Live path (glc_v3 on :8111): uv run s13code serve -# then the two curl requests above (park → approve). For deny + late-approve 409: -# POST .../approvals/approval {"decision":"deny"} then the same URL with approve. +# park → approve (happy path), or park → deny → late approve (expects HTTP 409): +# POST .../approvals/approval {"decision":"deny"} +# POST .../approvals/approval {"decision":"approve","comment":"too-late"} ``` ## License