Merge pull request #5 from K8Harness/feat/approval-flow

feat(approval-flow): add human approval hold flow

Author: henryqingmo

.gitignore

@@ -53,6 +53,7 @@ Thumbs.db
 data/
 logs/
 *.log
+.compose-bin/
 postgres-data/
 redis-data/
 
@@ -74,4 +75,3 @@ v0.md
 agents.md
 
 
-

.kiro/specs/approval-flow/design.md

@@ -0,0 +1,75 @@
+# Requirements Document
+
+## Introduction
+
+The approval-flow feature enables human-in-the-loop oversight for tool calls that the policy gate classifies as `approvalRequired`. When such a call is received, the gateway holds the HTTP connection open, sends a Slack Block Kit notification with the tool details and decision buttons, and waits for a manager to approve or deny within a 5-minute window. An approved call is forwarded to the upstream MCP server and the result returned to the client; a denied or timed-out call receives a JSON-RPC error response.
+
+## Boundary Context
+
+- **In scope**: synchronous hold (HTTP connection open during approval wait), Slack Block Kit notification, `/slack/actions` webhook endpoint, Slack request signature verification, decision resume signaling, durable ticket status updates before signaling, 5-minute fixed timeout with deny-on-timeout, session mutex TTL extension during approval wait.
+- **Out of scope**: email, PagerDuty, Teams, or other notification channels; multi-approver quorum; approval history dashboard or query API; async approval (fire-and-return) — v0 uses synchronous hold only; approval state surviving a gateway restart — a restart during an approval wait results in a client-side timeout on retry.
+- **Adjacent expectations**: policy-gate creates ticket records with `pending` status for `approvalRequired` cases — this feature reads and updates those records, changing the prior behavior of returning a `pending` response immediately; session-mgmt provides a session mutex TTL extension mechanism — this feature calls it to keep the session lock alive during the approval wait; the Slack API delivers interactive action payloads via HTTP POST to the gateway's webhook endpoint.
+
+## Requirements
+
+### Requirement 1: Synchronous Approval Hold
+
+**Objective:** As a gateway operator, I want the gateway to hold the HTTP connection open while awaiting a human decision, so that the agent client receives the tool call result only after the decision is made.
+
+#### Acceptance Criteria
+1. When the policy gate classifies a tool call as `approvalRequired`, the Gateway shall suspend the response and hold the HTTP connection open until either a decision signal is received or the timeout fires.
+2. While an approval hold is active, the Gateway shall periodically extend the session mutex lock TTL to prevent it from expiring before the decision arrives.
+3. When a decision signal is received and the decision is `approved`, the Gateway shall forward the original tool call to the upstream MCP server and return its result to the client.
+4. When a decision signal is received and the decision is `denied`, the Gateway shall return a JSON-RPC error response with code `-32001` and message `"approval denied"`.
+5. If no decision is received within 5 minutes, the Gateway shall return a JSON-RPC error response with code `-32001` and message `"approval timeout"`.
+
+### Requirement 2: Slack Approval Notification
+
+**Objective:** As a manager, I want to receive a Slack message when a tool call requires my approval, so that I can make an informed, timely decision.
+
+#### Acceptance Criteria
+1. When a tool call is held for approval, the Gateway shall send a Slack Block Kit message to the configured Slack channel containing: tool name, method/operation, call arguments, and session ID.
+2. The Slack Block Kit message shall include an Approve button and a Deny button, each carrying the associated ticket ID as an action value.
+3. If sending the Slack notification fails, the Gateway shall log the failure with the ticket ID and continue the approval hold — the timeout path still applies.
+4. The Gateway shall send the Slack notification using credentials read exclusively from environment variables; credentials must not be hardcoded.
+
+### Requirement 3: Slack Webhook Endpoint and Signature Verification
+
+**Objective:** As a gateway operator, I want the gateway to receive and verify Slack interactive action payloads, so that only legitimate Slack-originated decisions are processed.
+
+#### Acceptance Criteria
+1. The Gateway shall expose a `POST /slack/actions` HTTP endpoint to receive Slack interactive component action payloads.
+2. When a request arrives at `/slack/actions`, the Gateway shall verify the Slack request signature using HMAC-SHA256 with the `X-Slack-Signature` and `X-Slack-Request-Timestamp` headers against the configured signing secret before processing the payload.
+3. If signature verification fails, the Gateway shall return HTTP 400 and take no further action on the payload.
+4. If the `X-Slack-Request-Timestamp` value is more than 5 minutes in the past, the Gateway shall reject the request with HTTP 400 to prevent replay attacks.
+5. The Gateway shall read the Slack signing secret from an environment variable; the secret must not be hardcoded.
+
+### Requirement 4: Decision Persistence and Resume Signaling
+
+**Objective:** As a gateway operator, I want approval decisions to be durably recorded before the waiting handler is notified, so that no decision is lost if the gateway restarts between recording and signaling.
+
+#### Acceptance Criteria
+1. When a valid `approve` action is received at `/slack/actions`, the Gateway shall update the ticket record status to `approved` with a decision timestamp, and then publish a resume signal for that ticket, before returning a response to Slack.
+2. When a valid `deny` action is received at `/slack/actions`, the Gateway shall update the ticket record status to `denied` with a decision timestamp, and then publish a resume signal for that ticket, before returning a response to Slack.
+3. The Gateway shall always update the ticket record before publishing the resume signal.
+4. When the Gateway acknowledges a Slack action, it shall return HTTP 200 to dismiss the Slack button interaction.
+
+### Requirement 5: Timeout and Failure Handling
+
+**Objective:** As an agent client, I want the gateway to close the held connection with a clear error when no decision arrives in time, so that the connection does not hang indefinitely.
+
+#### Acceptance Criteria
+1. If no decision signal is received within 5 minutes of the approval hold beginning, the Gateway shall return a JSON-RPC error response with code `-32001` and message `"approval timeout"`.
+2. When a timeout occurs, the Gateway shall update the ticket record status to `expired`.
+3. If the decision notification channel fails during the approval wait, the Gateway shall treat the failure as a timeout and return the same error response as criterion 1.
+4. The Gateway shall log all timeout and error events with the ticket ID, session ID, and failure cause.
+
+### Requirement 6: Configuration
+
+**Objective:** As a gateway operator, I want all Slack integration settings to be provided via environment variables, so that credentials and channel configuration are not embedded in the binary.
+
+#### Acceptance Criteria
+1. The Gateway shall read the Slack notification channel identifier from an environment variable.
+2. The Gateway shall read the Slack signing secret from an environment variable.
+3. The Gateway shall read the Slack notification credential (webhook URL or bot token) from an environment variable.
+4. If any required Slack environment variable is absent at startup, the Gateway shall log an error describing which variable is missing and refuse to start.

.kiro/specs/approval-flow/requirements.md

@@ -0,0 +1,153 @@
+# Research & Design Decisions: approval-flow
+
+## Summary
+
+- **Feature**: `approval-flow`
+- **Discovery Scope**: Complex Integration
+- **Key Findings**:
+  - The pipeline's `(nil, nil)` continue-contract means `PolicyGateHandler` can block inline for the approval wait and return `(nil, nil)` on approval — the pipeline naturally continues to `UpstreamForwarder` with no forwarder injection or pipeline restructuring
+  - Slack block_actions payloads arrive URL-encoded with a `payload` form field containing JSON; raw body must be read before any parsing for HMAC verification to work correctly
+  - `go-redis/v9` is already present in the codebase; `client.Subscribe().Channel()` returns a Go channel that closes on connection loss — the nil-message case directly maps to the timeout fallback path (req 5.3)
+
+---
+
+## Research Log
+
+### Slack Interactive Components Payload Format
+
+- **Context**: Need to parse Slack manager's Approve/Deny click to extract ticket ID and decision
+- **Sources**: Slack API block_actions interaction payload documentation
+- **Findings**:
+  - POST arrives as `application/x-www-form-urlencoded`; single field `payload` contains URL-encoded JSON
+  - Top-level `type = "block_actions"` (not `interactive_message` — that is the legacy format)
+  - Decision routing: `actions[0].action_id` (`"approval_approve"` / `"approval_deny"`)
+  - Ticket ID carrier: `actions[0].value` (arbitrary string set when composing the Block Kit message)
+  - No `callback_id` at root for block_actions — that belongs to the legacy payload type
+  - `user.id` available for audit logging of `decision_by`
+- **Implications**: Parse `payload` field after URL-decoding from raw body; route on `action_id`; extract ticketID from `value`
+
+### Slack Request Signature Verification
+
+- **Context**: Req 3.2–3.4 require HMAC-SHA256 verification with replay protection
+- **Sources**: Slack verifying-requests-from-slack documentation
+- **Findings**:
+  - Base string format: `"v0:" + X-Slack-Request-Timestamp + ":" + rawBody`
+  - HMAC key: Slack Signing Secret (from app's Basic Info panel)
+  - Expected signature format: `"v0=" + hex(HMAC-SHA256(key, basestring))`
+  - Compare with `X-Slack-Signature` header using constant-time comparison (`hmac.Equal`)
+  - Raw body must be read before any form parsing (body reader is consumed)
+- **Implications**: Buffer raw body first; timestamp check first (cheap) then HMAC check; use `hmac.Equal` not `==`
+
+### Redis Pub/Sub API (go-redis/v9)
+
+- **Context**: Need event-driven resume signal delivery from SlackWebhookHandler to waiting PolicyGateHandler
+- **Sources**: go-redis/v9 pkg.go.dev documentation
+- **Findings**:
+  - `client.Subscribe(ctx, channel)` returns `*redis.PubSub`
+  - `pubsub.Channel()` returns `<-chan *redis.Message`; closes when `pubsub.Close()` is called or connection drops
+  - `client.Publish(ctx, channel, payload)` publishes to all current subscribers
+  - Nil message received from a closed channel — select `case msg, ok := <-ch` where `!ok` indicates closure
+  - No persistence: if no subscriber is active when signal is published, signal is lost
+- **Implications**: Nil/closed channel case must be handled as a timeout fallback; always `defer pubsub.Close()`
+
+### Existing Pipeline Handler Contract
+
+- **Context**: Need to understand how to wire the approval hold without modifying the pipeline
+- **Sources**: `/Users/henry/Programming/ToolGate/core/mcp/pipeline.go`, `handler.go`
+- **Findings**:
+  - `(nil, nil)` → continue to next registered handler or terminal forwarder
+  - `(*JSONRPCResponse, nil)` → halt pipeline, return response to client
+  - `PolicyGateHandler` is the last `pipeline.Use()` middleware before the terminal `UpstreamForwarder`
+  - Returning `(nil, nil)` from `PolicyGateHandler` on approval naturally calls `UpstreamForwarder.Handle(ctx, req)` with the original in-scope `req`
+- **Implications**: No forwarder injection needed; eliminates a component from the design
+
+### Existing TicketStore and TicketRecord
+
+- **Context**: Need to understand the current ticket API before extending it
+- **Sources**: `/Users/henry/Programming/ToolGate/cmd/gateway/ticket.go`
+- **Findings**:
+  - `TicketRecord{SessionID, TurnID, ToolName, Arguments, ExpiresAt}` — no ID field
+  - `Insert(ctx, TicketRecord) (string, error)` — returns UUID as string
+  - `approved`, `denied`, `expired` status values exist in the DB schema (policy-gate defined them)
+  - No `UpdateStatus` or `GetByID` currently exists
+- **Implications**: Add `UpdateStatus(ctx, id, status, decidedBy string) error`; idempotent (WHERE status='pending'); no GetByID needed since decision is in pub/sub payload
+
+---
+
+## Architecture Pattern Evaluation
+
+| Option | Description | Strengths | Risks / Limitations | Decision |
+|--------|-------------|-----------|---------------------|----------|
+| Block inline in PolicyGateHandler | Block via `WaitForDecision`; return `(nil,nil)` on approve | No pipeline changes; original req stays in scope for free forwarding | Holds goroutine for 5 min; HTTP WriteTimeout must accommodate | **Selected** |
+| Separate ApprovalInterceptor middleware | Wrap PolicyGateHandler; detect pending response; run approval hold | No changes to PolicyGateHandler | Requires parsing response body to detect "pending" — fragile | Rejected |
+| Async resume (fire-and-return) | Return immediately; resume via callback when decision arrives | No long-held connections | Requires async request resumption infrastructure; explicitly out-of-scope for v0 | Rejected (v1+) |
+
+---
+
+## Design Decisions
+
+### Decision: `(nil, nil)` continue-contract eliminates forwarder injection
+
+- **Context**: On approval, the gateway must forward the original MCP request to the upstream server
+- **Alternatives Considered**:
+  1. Inject `UpstreamForwarder` into `PolicyGateHandler` and call it directly
+  2. Return `(nil, nil)` from the handler on approval; let the pipeline continue naturally
+- **Selected Approach**: Option 2 — return `(nil, nil)`; the pipeline calls the terminal forwarder
+- **Rationale**: The original `*JSONRPCRequest` stays in scope throughout the blocking wait; returning `(nil, nil)` invokes the forwarder with no additional wiring
+- **Trade-offs**: Subtle: the pipeline continues in the same goroutine that was blocked; context is still valid since the client is still connected
+- **Follow-up**: Verify that the upstream forwarder's HTTP client timeout starts fresh from the point it's called (not from the start of the approval wait)
+
+### Decision: Decision carried in pub/sub payload, not re-fetched from DB
+
+- **Context**: After the resume signal arrives, `WaitForDecision` needs to know the decision
+- **Alternatives Considered**:
+  1. Publish a signal; waiter re-fetches ticket status from Postgres (requires `GetByID`)
+  2. Publish the decision string in the signal payload (`"approved"` / `"denied"`)
+- **Selected Approach**: Option 2 — decision in payload
+- **Rationale**: Avoids a Postgres round-trip; simpler; the authoritative decision is already in DB (persisted before publish); the signal is just a notification
+- **Trade-offs**: Signal payload is trusted internal data (UUID-keyed channel, not user-controllable); no external trust issue
+- **Follow-up**: None
+
+### Decision: SlackNotifier and ApprovalBridge as interfaces
+
+- **Context**: The brief explicitly identifies both as boundary candidates for v1+ swap-in
+- **Alternatives Considered**:
+  1. Concrete types only (simpler, fewer files)
+  2. Interfaces (brief-specified extensibility)
+- **Selected Approach**: Interfaces — one implementation each for v0
+- **Rationale**: Brief explicitly states "Approval notifier interface (initially Slack; designed as interface so v1+ can swap in other channels)" and "the interface hides the [resume] mechanism". Synthesis principle allows interfaces when they are explicitly specified as boundary candidates, even with one implementation.
+- **Trade-offs**: Slightly more files; enables v1+ to add Teams/email without touching PolicyGateHandler
+- **Follow-up**: None
+
+### Decision: `UpdateStatus` must be idempotent (WHERE status='pending')
+
+- **Context**: Slack retries webhook delivery on 5xx responses; a duplicate action payload could double-update a ticket
+- **Selected Approach**: `UPDATE ticket SET ... WHERE id=$1 AND status='pending'` — no-op if already terminal
+- **Rationale**: Prevents double-approval; returns success even if no rows updated (Slack gets 200 on retry)
+- **Follow-up**: Verify Postgres `UPDATE` returning 0 rows is not treated as error in `UpdateStatus`
+
+---
+
+## Synthesis Outcomes
+
+- **Generalization**: `ApprovalBridge` and `SlackNotifier` as interfaces generalize the "decision wait" and "notification delivery" concepts without over-building (one implementation each)
+- **Build vs. Adopt**: `go-redis/v9` Pub/Sub (already in codebase), `crypto/hmac`+`crypto/sha256` (stdlib), `net/http` (stdlib) — no new dependencies
+- **Simplification**: Removed `GetByID` (not needed; decision in payload); removed forwarder injection (pipeline contract handles it); removed `TicketRecord.ID` field addition (Insert returns ID separately)
+
+---
+
+## Risks & Mitigations
+
+- **HTTP server WriteTimeout too short**: If `WriteTimeout < 5 minutes`, approval waits will be forcibly closed by the server before the timeout fires. Mitigation: document required server timeout configuration; add to deployment checklist.
+- **Gateway restart during approval wait**: In-flight approval waits are lost on restart; `WaitForDecision` context is cancelled and the client sees a connection close. The ticket remains `pending` until it expires naturally. Mitigation: document limitation; v0 spec explicitly scopes this out.
+- **Signal lost between UpdateStatus and Publish**: Ticket is persisted but signal is never received; waiter times out after 5 minutes. Mitigation: waiter timeout marks ticket `expired` (already terminal, no update); client sees timeout error. Acceptable for v0.
+- **Slack argument payload too long**: Approval messages with large arguments may exceed Slack block character limits. Mitigation: truncate arguments display in `SlackClient.SendApprovalRequest`.
+
+---
+
+## References
+
+- Slack Block Actions payload: https://api.slack.com/reference/interaction-payloads/block-actions
+- Slack request signature verification: https://api.slack.com/authentication/verifying-requests-from-slack
+- Slack Block Kit message format: https://api.slack.com/block-kit
+- go-redis/v9 Pub/Sub: https://pkg.go.dev/github.com/redis/go-redis/v9

.kiro/specs/approval-flow/research.md

@@ -0,0 +1,23 @@
+{
+  "feature_name": "approval-flow",
+  "created_at": "2026-05-15T00:00:00Z",
+  "updated_at": "2026-05-16T00:00:00Z",
+  "language": "en",
+  "phase": "implemented",
+  "approvals": {
+    "requirements": {
+      "generated": true,
+      "approved": true
+    },
+    "design": {
+      "generated": true,
+      "approved": true
+    },
+    "tasks": {
+      "generated": true,
+      "approved": true
+    }
+  },
+  "ready_for_implementation": true,
+  "implementation_complete": true
+}