feat(security): DEFER authorization decision (closes #211)

[naveen-kurra]

Closes #211. Governance R4c — the fifth and final `PolicyDecision` from the R4 taxonomy. On a high-risk tool call, the executor pauses, an external approver POSTs a decision, and the executor resumes where it left off. Distinct from DENY (refuses) and STEP_UP (re-authenticates) — the tool call is not terminated, only paused.

Design

• In-process pause-and-resume via goroutine block. The hook goroutine holding the tasks/send HTTP request blocks on the deferral's decision channel; the goroutine's stack IS the persisted state. Process restart abandons pending deferrals (documented; cross-restart persistence is a follow-up seam).
• Per-tool config — security.defer.tools.<name>: {to, timeout, context_template}. Fast-path for unconfigured tools.
• Task status transitions — flips to `deferred` in the a2a store while blocked so parallel `GET /tasks/{id}` polls see the pause; restored on approve, remains `working` for the caller to see the failed response on reject/timeout.
• Three audit events: `task_deferred` (on entry), `task_deferred_decision` (approve/reject with approver + note + wait_ms), `task_deferred_timeout` (auto-DENY on window close).

Config

```yaml
security:
defer:
enabled: true
default_timeout: 10m
tools:
cli_execute:
to: channel:slack:#oncall
timeout: 10m
context_template: "Agent wants to run {tool}: {args}"
```

Endpoint

```
POST /tasks/{id}/decisions
Content-Type: application/json

{"decision":"approve","approver":"alice@example.com","note":"one-off"}
```

200 on resolve, 404 on unknown task, 400 on invalid decision string, 409 on race with timeout.

Acceptance criteria (from #211)

• `TaskStateDeferred` recognized in A2A responses
• `DecisionDefer` returnable from any guardrail hook (`runtime.Defer(to, timeout, context, reason)` constructor)
• Executor persists step state on Defer; ctx returns without terminating the task (in-process pause; goroutine stack IS state)
• `POST /tasks/{id}/decisions` accepts `{decision, approver, note}` and resumes the executor
• Timeout auto-DENYs and audits (via `time.AfterFunc` in the engine; emits `task_deferred_timeout`)
• Slack + Telegram notify handlers wired to receive deferred-context payloads — follow-up. The `to` field is free-form today; operators tail the audit stream to forward. First-party channel adapter with approve/reject buttons is tracked separately.
• Tests: pause → external approve → resume; pause → reject → deny; pause → timeout → deny
• Docs: `docs/security/defer-decisions.md`

Test coverage

• `forge-core/security/deferpolicy/defer_test.go` — 12 tests: Register uniqueness, concurrent tasks, Resolve approve/reject unblocks waiter, timeout unblocks with `DecisionTimeout`, resolve-before-timeout cancels timer with no spurious value, timeout-before-resolve leaves nothing to resolve, unknown-task 404 shape, ctx-cancel behaviour, Peek, default 10m.
• `forge-cli/runtime/defer_test.go` — 8 tests: hook approve / reject / timeout paths, unconfigured-tool fast path, ctx-cancel cleanup, context-template rendering, decisions endpoint happy path / 404 / 400 for bad decision.

Live-verified end-to-end

Built the binary from the branch tip. `forge run` startup log:
```
{"level":"info","msg":"defer engine wired","time":"...","tools":1}
```
Driver in `/tmp/r4c-demo/verify/` exercised all four scenarios against a live decisions endpoint:

• Scenario A (approve) — POST → hook unblocked with approve + approver + note
• Scenario B (reject) — POST → hook unblocked with reject
• Scenario C (timeout) — no POST in 40ms → hook unblocked with auto-DENY
• Scenario D (unknown task) — POST to non-existent task → 404

Full `go test ./forge-core/... ./forge-cli/...` sweep green; `gofmt -w` + `golangci-lint run` clean.

[initializ-mk]

Request changes — this is the strongest PR in the governance batch: the pause/resume concurrency (the part most likely to harbor bugs) is genuinely well-engineered and passes -race. The changes requested are small — an inaccurate status-code comment and validation-consistency with the sibling R4 PRs — plus one design limitation worth documenting before merge.

Concurrency — verified correct

I traced every ordering of the Resolve-vs-timeout race:

• Register arms a time.AfterFunc; both Resolve and autoTimeout delete from pending under e.mu, so whichever takes the lock first wins and the other finds !ok and no-ops. Handle.resolved sync.Once is a second guard so wait is sent to exactly once.
• wait is buffered(1), so a resolver never blocks even if the waiter already left (ctx-cancel) or hasn't arrived yet — no goroutine leak, no send-on-closed panic.
• Timers are cleaned on all three paths (Resolve, autoTimeout, ctx-cancel) — no timer leak.
• ctx-cancel is cleaned up by the hook itself: on WaitCtx returning ctx.Err(), it restores task status and calls engine.Resolve(...Timeout) to deregister, rather than leaking the handle until the timer fires.

Both security/deferpolicy and the runtime defer/decision tests pass under -race.

Reject/timeout -> failed task is correct here (unlike #247)

On reject/timeout the hook returns an error -> the loop aborts -> executeTask converts it to a failed task. That's the same swallow I flagged as a blocker on #247, but here it is the right behavior: DEFER has no machine-readable challenge to emit, so a rejected/timed-out approval legitimately ends the task as failed, and the a2a async model conveys that via task status (the caller reads the returned task or polls GET /tasks/{id}). Approve -> restore working -> tool proceeds. All three audit events fire.

Changes requested

1. Status-code comment is wrong. registerDecisionsEndpoint / makeDecisionsHandler comments say "409 for tasks not in a deferred state," but the code returns 404 for that case — a Peek miss can't distinguish unknown-task from not-deferred, so both are 404. 409 fires only on the narrow Peek-hit-then-Resolve race. The user-facing docs/security/defer-decisions.md is correct; just fix the two inline comments so an integrator building the approve/reject webhook isn't misled about the contract.

2. Validation consistency with the R4 siblings. Step-up (#247) and intent (#245) fail loud when enabled: true but no tools are declared; defer silently no-ops (len(cfg.Tools) == 0 -> return) while still logging "defer engine wired," so an operator who enables defer but forgets tools: gets silent inaction. Add the same startup rejection (and a small DeferConfig.Validate — today Register clamps timeout <= 0 to 10m, which is safe but happens deep at first use rather than at startup).

Design limitation (document before merge)

The hook blocks the executor/request goroutine synchronously for up to timeout (default 10m). The caller must hold the connection open — or use tasks/sendSubscribe (SSE) — for the whole approval window; a shorter client or server write-timeout cancels the ctx (cleaned up correctly) and abandons the approval. Worth calling out explicitly in the docs that synchronous tasks/send isn't the right transport for long approval windows. Cross-restart persistence is absent, but the Engine seam is called out — fine as a follow-up.

Nice work — the double-guard on the resolve/timeout race and the buffered-channel-never-blocks-a-resolver design are exactly right. Fix the comment + the validation-consistency nit and I'm happy to approve.

[initializ-mk]

Approving — commit 05f311c addresses all three requested changes. Re-verified on the branch: build clean, forge-core/security/deferpolicy green under -race, runtime defer/decision tests green, types config test green.

1. Status-code comment fixed. registerDecisionsEndpoint / makeDecisionsHandler now document it accurately: 404 for no-pending-deferral (unknown task and not-currently-deferred collapse into one case — Peek can't distinguish them), 409 only for the narrow Peek-hit-then-Resolve race, 400 for malformed bodies/decisions, 200 on resolve. The user-facing docs were already right.

2. DeferConfig.Validate added and called at runner construction, so enabled: true with no defer.tools: fails startup (with a message that names the block) instead of logging "defer engine wired" and enforcing nothing — consistent with the fail-loud posture of the R4 siblings (#245 / #247). TestDeferConfig_Validate pins the matrix.

3. Design-limitation documented. The new "Choose your transport for the approval window" callout in docs/security/defer-decisions.md explains that the pause blocks the executor goroutine for up to defer.timeout, so the client read-timeout / server write-timeout / proxy idle must exceed that window, and steers long approvals to tasks/sendSubscribe (SSE) or the async poll pattern.

This was already the strongest PR in the governance batch — the resolve/timeout double-guard (pending-delete-under-lock + resolved sync.Once), the buffered-channel-never-blocks-a-resolver design, and the correct ctx-cancel cleanup all verified under -race in the first pass. The three surface items are now closed cleanly. Nice work.

[initializ-mk]

Re-approving — my earlier approval was auto-dismissed by the rebase, not by any code change. The new head 68d1866 is a pure rebase of the previously-approved 05f311c onto the current main (now carrying the merged #245/#246/#247): I diffed the two — only blob hashes and line-number positions moved, the added content is byte-identical (the Defer.Validate() call at Runner startup, DeferConfig.Validate, the 404/409 comment fix, and the transport/timeout doc callout).

Re-verified on the rebased base: both modules build clean, security/deferpolicy green under -race, runtime defer/decision tests green, and the types defer-config test green.

All three requested changes remain in place and correct. No new concerns. Approving.