feat(proxy): wire interactive approval into proxy and proxy-http

[dgenio]

Summary

Makes ask decisions usable in both live proxies instead of the hardwired deny-all behavior, and unifies the approver contract the three call sites share. This delivers a coherent group of four related issues that all live in the approval/proxy code path.

What changed:

• Unify the two duplicate Approver interfaces behind one shared contract #137 — unify the Approver contract. proxy.Approver / httpproxy.Approver and their DenyAllApprover structs are now type aliases for the single approval.Approver / approval.DenyAllApprover, so one approver implementation wires into both proxies and check (no adapters, no import cycle).
• Wire the interactive TTY approver and approval timeout into proxy and proxy-http #126 — wire the TTY approver + timeout into the proxies. runProxy/runProxyHTTP now build a real approval.TTYApprover by default and accept a new --approval-timeout; --no-interactive keeps the fail-closed DenyAllApprover. A new shared approval.Resolve is the single ask→decision mapping (the CLI's resolveAsk now delegates to it). The proxies audit the resolved decision with the approval outcome appended to the engine's reason (preserving taint-escalation context) and return the canonical reason to the agent without leaking internal approver error text.
• Add regression tests covering proxy ask approval paths #129 — regression tests. Table-driven coverage across both proxies for approve, deny, timeout, I/O error, non-interactive, and ask-on-notification, asserting both the agent-facing response and the audit decision/reason.
• Correct stale documentation that describes shipped features as unimplemented #127 — correct stale docs. docs/integration-guide.md and docs/architecture.md no longer describe proxy-http/the interactive approver as roadmap/unimplemented or reference closed issues [Approval] Implement interactive TTY approval for ask decisions #29/[Approval] Add approval timeout with default-deny fallback #30.

Related issue

Closes #126
Closes #127
Closes #129
Closes #137

How verified

• make ci (fmt-check, go vet ./..., go test -race ./...) — all packages pass, race-clean, total coverage 80.1%.
• New/updated tests: internal/proxy (TestProcessAgentLineAskApproved/Denied/Errors/Timeout/NonInteractive/NotificationDropped) and internal/httpproxy (TestHTTPAskApprovedForwards/ApproverErrorBlocks/Timeout + reason assertion on the existing deny test) — each fails without the fix (deny-all) and passes with it.
• Smoke: agentfence proxy --help lists --approval-timeout; an invalid --approval-timeout value is rejected with a parse error.

Tradeoffs / risks

• Behavior change (intentional, Mode B / no back-compat constraint): the proxies previously audited the unresolved ask; they now audit the resolved allow/deny. The agent-facing blocked message now uses canonical reasons (denied interactively, approval timeout, approval I/O error) instead of "... (denied via ask)" / "approval error: <raw>", and the raw approver error text is no longer exposed to the agent. Documented in the CHANGELOG.
• The stdio proxy's interactive prompt reads /dev/tty; where that is unavailable (e.g. Windows/CI) interactive approval isn't meaningful — use --no-interactive there. The CLI's resolveAsk reason text is unchanged (an existing test pins non-interactive: ask auto-denied).

Checklist

• Tests added or updated
• Documentation updated if needed
• CI passes
• Issue number included

https://claude.ai/code/session_01K8hWMifivsN6RMKqzXV3gX

---

Generated by Claude Code

[Copilot]

Pull request overview

This PR wires interactive ask approval into both live proxies (stdio and HTTP) by unifying the Approver contract across packages and centralizing ask→decision reason mapping in internal/approval. It also updates docs/changelog and adds regression coverage to lock the behavior in.

Changes:

• Aliases proxy.Approver/httpproxy.Approver (and deny-all implementations) to the shared approval.Approver contract.
• Adds approval.Resolve and uses it in check, proxy, and proxy-http to apply approval timeout + consistent canonical reasons.
• Expands proxy/httpproxy tests and updates documentation to reflect shipped interactive approval behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/proxy/proxy.go	Uses shared approver + resolves ask before auditing/forwarding; adds timeout/non-interactive options.
internal/proxy/proxy_test.go	Adds/updates regression tests for approve/deny/error/timeout/non-interactive/notification behavior.
internal/httpproxy/httpproxy.go	Mirrors stdio proxy: resolves ask via shared resolver before audit/response/forward.
internal/httpproxy/httpproxy_test.go	Adds approval-path tests for allow/deny/error/timeout and audit assertions.
internal/approval/approval.go	Introduces Outcome + Resolve as canonical ask→reason mapping used by all call sites.
cmd/agentfence/main.go	Wires TTY approver + --approval-timeout into proxy and proxy-http; delegates check ask resolution to approval.Resolve.
docs/integration-guide.md	Updates approval guidance and removes stale “unimplemented/roadmap” statements.
docs/architecture.md	Updates architecture description to match interactive approval + timeout behavior.
CHANGELOG.md	Documents the new proxy approval behavior, unified approver contract, and reason/audit behavior changes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dgenio]

Good catch — fixed in e5918b4. Both comments are the same root cause: the proxies must not fall back to stdin for prompts.

• Added approval.NewTTYApproverStrict() which opens /dev/tty and returns an error instead of falling back to os.Stdin.
• newProxyApprover now uses it; with no controlling terminal the proxy exits with a message to re-run with --no-interactive rather than silently reading approvals off the JSON-RPC channel. (check keeps the stdin fallback, since its stdin is the user terminal and calls come from --call.)
• Updated the integration-guide paragraph to state the "no /dev/tty → use --no-interactive" behavior, and added tests for the deny-all and require-TTY paths.

---

Generated by Claude Code