feat(approvals): question-set approvals — decisions beyond yes/no (WO-52 phase 1) - #5372
Conversation
…-52 phase 1) Approvals were strictly binary approve/deny. This adds "question set" approvals end-to-end for the LOCAL product, with Claude Code's AskUserQuestion tool as the first source: * clawmetry/question_sets.py (new) — shared vocabulary: sanitize the 1-4-question set from the PreToolUse payload, strictly validate structured answers against it (unknown question / unknown option label are rejected; multiSelect takes label arrays; free text only when the question set says so), merge winning answers into updatedInput, and the shared decision-wall core (answers land in args["_cm_answers"] BEFORE the row flips to decision='answered', so a waiting hook never sees the status without the answers). Storage is migration-free: the set rides in args["_cm_questions"] (the _cm_risk precedent). * routes/hooks.py — the PreToolUse receiver intercepts AskUserQuestion before policy matching, parks a question-set row (on_timeout="ask"), and answers an 'answered' row with permissionDecision=allow + hookSpecificOutput.updatedInput = tool_input + answers. Every failure mode — window elapsed, expired row, malformed payload, unreadable answers, store down — falls back to "ask" (the terminal prompt), NEVER the binary on_timeout=deny default and NEVER a fabricated answer. CLAWMETRY_QUESTION_GATE=0 kills the intercept; CLAWMETRY_QUESTION_WINDOW_S overrides the window (default: the mirror window, 180 s). The hook client already forwards hookSpecificOutput verbatim, so no client change was needed. * routes/policy.py + routes/hitl.py — both local decision walls accept decision='answer' carrying the answers map (validated → 400 on unknown labels/questions; first-click-wins preserved via update_approval_decision). GET /api/approvals now serves the full question set per row; the audit summary counts 'answered'. * clawmetry/claude_code_gate.py — the installed PreToolUse matcher also watches AskUserQuestion whenever the gate is armed, and the hook timeout is floored at the question window + buffer so Claude Code never cancels the hook before the receiver's "ask" fallback lands. * clawmetry/templates/tabs/approvals.html — pending question rows render full fidelity (radios / checkboxes / free-text when flagged, count + headers chip) with a single "Send answers" button posting decision='answer'; binary rows are untouched. * tests/test_question_set_approvals.py (23 tests) — validation 400s, multiSelect round-trip, first-click-wins on structured answers, binary regression through both walls, gate intercept → updatedInput, expiry → "ask". Two exact-matcher assertions in tests/test_runtime_gates_and_hooks.py updated for the widened matcher. Excluded from this phase: cloud/pro decision walls, Telegram/phone question fidelity (phase 2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VCaSBpV1wBCKrU4z9MKKZ
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VCaSBpV1wBCKrU4z9MKKZ
|
![8090-software-factory[bot]](https://avatars.githubusercontent.com/in/1847834?s=60&v=4){kind=small}
There was a problem hiding this comment.
The _wait_on_row function in the PreToolUse hook receiver does not handle question-set approvals. WO-52 requires that when a row carries a question set, an "answered" status should invoke _answered_reply() to return allow+updatedInput with structured answers, and an expired question-set row should fall back to "ask" (never the binary deny default). The current code lacks both the status=="answered" check and the is_question&&status=="expired" special case.
There was a problem hiding this comment.
AC-RA-002.1 requires that when a pre-tool gate intercepts AskUserQuestion, "the runtime's native question prompt is intercepted, its full question set becomes an approval request, and the human's structured answers are returned to the runtime in the runtime's own answer contract." The hooks.py receiver does not contain the _park_question_set() function needed to intercept and park the question set, nor the _answered_reply() function needed to return the merged answers via updatedInput.
There was a problem hiding this comment.
The PreToolUse hook receiver skips the AskUserQuestion intercept entirely. WO-52 phase 1 requires intercepting tool_name=="AskUserQuestion" before policy matching and calling _park_question_set() to park the question set. The code goes directly from the "no tool_name" check to "fresh call: policy match" without the AskUserQuestion branch, so Claude Code's structured questions are never parked as question-set approvals.
Test plan & review notesRepo: vivekchand/clawmetry What changed
CI status The six Drift Bot findings on this PR all point to the same root cause: the WO-52 implementation appears incomplete in
If these paths are intentionally deferred to a later phase, that's fine — but a note in the PR description (or a Smoke commands make lint # catch missing-function NameErrors early
make test-api # approval flow integration testsWhat to look at visually
Issue link
Generated by Claude Code |
|
Drift Bot's findings are false positives — the code it reports missing is in this PR's tree, same resolution failure as on #5367 (the bot appears to analyze a ref other than the PR head; the Factory code index only knows merged code, so a spec-first PR cannot link its own additions):
Covered by |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return jsonify({"error": msg}), code | ||
| flag.unlink() | ||
| log.info("HITL answer: session=%s approval=%s operator=%s", | ||
| session_id, approval_id, operator) |
| return jsonify({"error": msg}), code | ||
| flag.unlink() | ||
| log.info("HITL answer: session=%s approval=%s operator=%s", | ||
| session_id, approval_id, operator) |
| return jsonify({"error": msg}), code | ||
| flag.unlink() | ||
| log.info("HITL answer: session=%s approval=%s operator=%s", | ||
| session_id, approval_id, operator) |
Visual diffComparing 40 of 66 comparison(s) flagged (>1% pixel diff).
Folder: e593a4d0a745. Full PNGs also attached as a workflow artefact. Generated by visual-diff bot. Pixel diffs >1% flagged; eyeball the table before merging. This check is non-blocking — fail = bot bug, not a code problem. |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
2 participants
Product record: https://factory.8090.ai/project/b415065f-ab2f-4f53-8864-0c009fd098cb/requirements/a8fd0512-9bee-4209-9cee-80b732d0f130
Factory requirement
"Approval Decisions Beyond Yes/No" (WO-52), phase 1. Approvals today are strictly binary approve/deny. This ships "question set" approvals end-to-end for the local product, with Claude Code's
AskUserQuestiontool as the first source: the runtime's structured question (1-4 questions, 2-4 labelled options each, optional multiSelect) is parked as an approval row, the dashboard renders it full-fidelity, and the human's structured answers resume the session via the PreToolUse hook'shookSpecificOutput.updatedInput(= originaltool_input+answersmap), exactly as if they had been picked in the terminal.What phase 1 covers
clawmetry/question_sets.py: sanitize the question set from the hook payload, strictly validate structured answers (unknown question → 400, unknown option label → 400, multiSelect takes label arrays, free text only when the question set says so), merge answers intoupdatedInput, and the shared decision-wall core. Storage is migration-free: the set rides inargs["_cm_questions"], the answers inargs["_cm_answers"](the_cm_riskprecedent). Answers are written intoargsbefore the row flips todecision='answered', so a hook polling the row never sees the answered status without the answers.routes/hooks.pyinterceptsAskUserQuestionin the PreToolUse receiver (before policy matching; no protection rule needed), waits like other gated calls, and answers anansweredrow withpermissionDecision=allow+updatedInput. Plain approve → allow unchanged (question appears in the terminal); deny → deny with reason. Every failure mode — window elapsed, expired row, malformed payload, unreadable answers, store down — falls back toask(Claude Code's own terminal prompt), never the binaryon_timeout=denydefault, and never a fabricated/defaulted answer.CLAWMETRY_QUESTION_GATE=0is the kill switch;CLAWMETRY_QUESTION_WINDOW_Soverrides the window (default: the mirror window, 180 s). The hook client (claude_code_gate.hook_main) already forwardshookSpecificOutputverbatim, so no client change was needed — verified, not assumed.POST /api/approvals/<id>/decideandPOST /api/hitl/decideacceptdecision='answer'+answers, validated against the stored set; first-click-wins is preserved by reusingupdate_approval_decision(decisionanswered, statusanswered).GET /api/approvalsnow serves the full question set per pending row; the audit summary countsanswered.AskUserQuestionwhenever the gate is armed, and the hook timeout is floored at question-window + buffer so Claude Code never cancels the hook before the receiver'saskfallback lands.clawmetry/templates/tabs/approvals.html: pending question rows render radios (single-choice), checkboxes (multiSelect), a free-text input when the set flags it, a question-count + headers chip, and a single Send answers button postingdecision='answer'. Binary rows render exactly as before. (static/js/app.jsuntouched.)Explicitly excluded (phase 2)
clawmetry-cloud) and pro (clawmetry-pro) decision walls / relay.Tests
tests/test_question_set_approvals.py— 23 passed (validation 400s incl. unknown label/question, multiSelect array round-trip, first-click-wins on structured answers, deny-beats-late-answer, binary regression through both walls, queue payload carries the set, gate intercept →updatedInput, dedup ontool_use_id, expiry →askwith row closed astimeout, already-expiredrow →ask, malformed payload →ask, env kill switch).tests/test_approvals_local_store.py tests/test_policy_replay.py tests/test_tool_policy_route_gate.py— 38 passed.test_runtime_gates_and_hooks.py, test_mirror_gate.py, test_hook_collision.py, test_hooks_claude_code.py, test_approval_seam.py, test_tool_risk_remember_flow.py, test_openclaw_exec_policy_gate.py, test_approvals_local_blocking.py, test_approvals_duckdb_watcher.py, test_approvals_deny_kill.py— 175 passed, 1 pre-existing failure (test_cc_gate_windowless_python_swap) that fails identically on a clean origin/main checkout (unrelated to this change).tests/test_runtime_gates_and_hooks.pywere updated for the deliberately widened installed matcher (Bash→Bash|AskUserQuestion, timeout floor 180 → 240); the matcher-derivation helper itself stays policy-only and its own pins are unchanged.ast.parseclean on all touched Python files; template inline JS passesnode --check.🤖 Generated with Claude Code
https://claude.ai/code/session_017VCaSBpV1wBCKrU4z9MKKZ