feat(hitl): approval binds to the resolved request, not the tool name

.gitleaksignore

@@ -58,3 +58,13 @@
 # introduced the line stays in history and still needs an entry. Not a real key:
 # it never authenticated against anything.
 eebfe898b34858918d5cbbe11e60336d1b6a916e:src/test/java/ai/labs/eddi/secrets/sanitize/SecretScrubberTest.java:generic-api-key:238
+
+# ResolvedRequestTest: the same mistake as the SecretScrubberTest entry above,
+# made again. Tests proving that a credential in a request BODY is redacted need
+# a literal carrying SecretRedactionFilter's `sk-` + 20-char shape, and the first
+# version used a realistic-looking one. Replaced in a follow-up commit with a
+# zero-entropy value (repeated characters — same shape, nothing for the scanner
+# to flag), but gitleaks scans a PR's whole commit range, so the commit that
+# introduced it stays in history and still needs an entry. Never a real key: it
+# never authenticated against anything.
+96df3c83fa449e02250c08c1aa420a11617f2ebb:src/test/java/ai/labs/eddi/modules/apicalls/impl/ResolvedRequestTest.java:generic-api-key:193

docs/hitl.md

@@ -366,6 +366,41 @@ A **REJECTED** call is not executed; instead the LLM receives a structured rejec
 
 `GET /agents/{id}/approval-status` returns a `TOOL_CALL` `pauseDetails` object (computed at read time; see the [`pauseDetails` shape](#pausedetails-shape) reference above for the full JSON). Its `calls[].arguments` is **always** the redacted, size-capped value (`argumentsRedacted`) — the raw arguments never appear. `executedUngatedCalls` names ungated calls in the same batch that already ran (see decision 4). `outcomeUnknown` lists callIds with an `EXECUTING` journal entry — a prior approval that crashed mid-execution — and is empty in the common case.
 
+### Request pinning — approval binds to a request, not a tool name
+
+For an `http`-sourced call, the tool name alone tells an approver little: it comes from the endpoint's `operationId` (or a generated slug) and says nothing about which resource is targeted or with what body. So at gate time each gated httpcall tool is **resolved** — `IApiCallExecutor.resolve` builds the method, URL, query, headers and body it would send, without sending it — and both a **redacted preview** and a **SHA-256 fingerprint** of that resolved request are persisted on the pause (`PendingToolCall.requestPreview`, `.requestFingerprint`). The preview is what an approver should actually look at, not the raw tool arguments.
+
+`GET .../approval-status` surfaces this: each entry in `pauseDetails.calls[]` carries `requestPinned` (boolean) and, when the call could be resolved at all, `requestPreview` — `{method, uri, queryParams, headers, body, bodyTruncated}`, all already redacted. The raw fingerprint itself is never exposed; it is an internal comparison value with no meaning to a human.
+
+**The two fields are independent, and a client must not infer one from the other.** `requestPinned` says whether a fingerprint will be *enforced* before execution — not whether a preview exists. An `http` call carrying `preRequest.propertyInstructions` is previewed best-effort but deliberately left unpinnable, so it arrives with `requestPinned: false` and a non-null `requestPreview`: show it, but do not present it as guaranteed to be what runs. `requestPreview: null` means there was nothing to resolve (every non-`http` tool), which is not a resolution failure a caller should treat as an error.
+
+On resume, an **approved** call is re-resolved and its fingerprint re-compared immediately before execution. A mismatch refuses the call — a synthetic `{"status":"NOT_EXECUTED","reason":"the request changed after it was approved"}` result, an audit line (`hitl.tool.request_changed`, tool + callId + reason only — never the request itself), and the rest of the batch proceeds normally. This is what makes the approval bind to *the request that runs*, not to the name of the tool that was called.
+
+**Headers are fingerprinted redacted; the body is fingerprinted raw.** For **headers** the fingerprint deliberately covers the redacted form: `ApiCallExecutor` resolves `${caller:token}` into `Authorization` at execution time, and the approver is routinely a different person than whoever's turn raised the pause — fingerprinting the live header would mismatch on every cross-user approval (the normal case) and the check would have to be disabled. Redacting first makes the fingerprint answer what approval is actually about — *what the request does* — and leaves *whose credentials carry it* to authentication, which the fingerprint does not participate in.
+
+The **body** has no such legitimate variance (`${caller:token}` is rejected outside headers, and a `${vault:…}` reference resolves identically at gate time and at execution), so it is hashed as resolved and only the *stored* copy is redacted. Redacting before hashing would collapse two different credentials to one marker and therefore to one fingerprint, letting a swapped secret pass the pre-execution re-check unnoticed.
+
+The fingerprint is never returned through the client API — but it is a SHA-256 digest, not encryption, and it *is* persisted on the pause record. Treat the stored value as sensitive internal data: for a predictable body it supports offline guessing, and equal digests reveal that two requests were identical. It is excluded from every client-facing projection for that reason, not merely because it is meaningless to a human.
+
+Query parameters get the same treatment as the body — hashed as resolved, redacted for display — because `?api_key=…` is a conventional way to pass a credential and the query string is shown to the approver too. A repeated parameter (`?tag=a&tag=b`) is canonicalised as one length-prefixed field *per value*, so a single value containing the display separator cannot impersonate two.
+
+Body and query redaction are by **value shape**, not field name — a body is caller-defined JSON (or another format entirely) with no fixed key vocabulary to match on the way headers have. `SecretRedactionFilter` (the same filter behind `argumentsRedacted`) removes OpenAI/Anthropic-style keys, bearer tokens and vault references wherever they appear. A hand-rolled secret in a generically named field, matching none of those shapes, is not caught — the same limitation the redacted tool arguments already carry, and the reason a config write that must carry a credential belongs behind a vault reference rather than a literal.
+
+**A call can be unpinned**, and that is a deliberate degrade, not a bug. A call is left unpinnable whenever `execute()` could legitimately build a request that `resolve()` did not — the guard is "never pin what cannot be honoured", so the set is defined by that property rather than by a list of features:
+
+| Unpinnable when | Why `execute()` can diverge from `resolve()` |
+| --- | --- |
+| The tool is not `http` (builtin/mcp/a2a) | There is no HTTP request on this side of the boundary to pin. |
+| `preRequest.propertyInstructions` is set | Those write to conversation memory, so resolving them ahead of execution would apply them twice. |
+| `fireAndForget` **and** `preRequest.batchRequests` | The batch expands at execution time into N requests, none of them the single one that was previewed. |
+| `postResponse.retryApiCallInstruction` with `maxRetries >= 1` | `buildRequest` sits inside the retry loop, and each attempt re-renders templates against a memory that the previous attempt wrote to (`{…Error}`, `{…HttpCode}`, `{responseObjectName}`). Attempt 2 is a request nobody previewed. |
+
+The retry row is the easy one to trip over: `RetryApiCallInstruction.maxRetries` **defaults to 3**, so `"postResponse": {"retryApiCallInstruction": {}}` is by itself enough to unpin an otherwise-pinnable write — and a retry can fire on a **2xx** response when `responseValuePathMatchers` matches, not only on `retryOnHttpCodes`. Read `requestPinned` per call; do not infer it from the endpoint.
+
+An unpinned call (`requestFingerprint == null`) is approved on name and arguments alone, exactly as before pinning existed — nothing is ever refused on a comparison that was never sound. An **amended** call (`amendedArguments` set) is likewise never fingerprint-checked: the approver rewrote the request themselves, so the pin describes the request they replaced.
+
+Three situations fail **closed** instead — refused, not silently allowed — because "cannot verify" is a different answer than "unchanged": the tool disappeared from the workflow between pause and resume, re-resolution throws, or the call's config gained any of the unpinnable properties above mid-pause (a pinned call becoming unpinnable — adding `propertyInstructions`, or a `retryApiCallInstruction`, while a human is deciding). Treating any of these as "unchanged" would make reconfiguring an agent while a human is deciding the way around the guard.
+
 ### The execution journal (at-most-once)
 
 Approved tool executions are protected by a write-ahead journal (`IHitlToolJournalStore`) so a human approval is executed **at most once**, across pod crashes and re-approvals:
@@ -433,7 +468,8 @@ Config: `eddi.hitl.crash-recovery.enabled` (default `true`), `eddi.hitl.crash-re
 
 ## Operations
 
-- **Metrics** (`/q/metrics`): `eddi_hitl_pause_count`, `eddi_hitl_resume_count`, `eddi_hitl_timeout_count`, each tagged `surface=regular|group`; `eddi_group_member_pause_skipped_count` for auto-cancelled member pauses inside groups.
+- **Metrics** (`/q/metrics`): `eddi_hitl_pause_count`, `eddi_hitl_resume_count`, `eddi_hitl_timeout_count`, each tagged `surface=regular|group`; `eddi_group_member_pause_skipped_count` for auto-cancelled member pauses inside groups; `eddi.operator.write.approval{decision=approved|rejected|timeout}`, one per gated call the instant its verdict is resolved (`timeout` is a distinct bucket from `approved`/`rejected` — see [Request pinning](#request-pinning--approval-binds-to-a-request-not-a-tool-name) above; not operator-specific despite the name, it fires for any gated call regardless of which agent).
+- **Operator canary/gate metrics** (client-reported): the write canary and gate-installed check both run entirely client-side (there is no server-side notion of "the operator", just an agent with a particular `hitlConfig`), so the Manager reports outcomes via `POST /administration/operator/{canary-result,gate-status}` (`eddi-admin` only) purely so they show up on `/q/metrics` without a Manager tab open. This is a relay, not a verification — a report is trusted at face value. Produces `eddi.operator.canary{outcome=pass|fail|unknown}`, `eddi.operator.canary.duration`, and the gauge `eddi.operator.gate.verified` (1 only while every provisioned version last read back with a sound gate; defaults to 0, including on a deployment that has never activated an operator — that ambiguity is real and unresolved by this signal alone).
 - **Undeploy**: paused conversations do **not** count as active — an agent version with pending approvals can be undeployed. Resuming afterwards returns `409 agent not deployed` and the pause is restored (redeploy, then retry). The idle-conversation cleanup sweep likewise **spares** `AWAITING_HUMAN` conversations — a pending approval is never silently force-ended by maintenance.
 - **Cancel semantics (regular)**: cancels a paused conversation, or signals a turn executing on the same pod to stop at the next task boundary. `CANCEL_IMMEDIATE` currently degrades to graceful on the regular surface. Cancelling an idle conversation returns `409` (use `endConversation`).
 - **Timeout schedules are not manually operable**: HITL timeout schedules live in the schedule store but the schedule REST surface refuses to fire them manually (`409`, use `/resume` or `/cancel` — manual firing would bypass the approval authz), restricts update/delete/enable/disable to `eddi-admin` (`403` otherwise, so an editor cannot disarm an ABORT/AUTO_REJECT safety timeout), and redacts them from non-admin listings.

planning/operator-write-scope-plan.md

@@ -1,5 +1,19 @@
 # Implementation Plan — Approval-Gated Write Capability for the Platform Operator
 
+> **Superseded, kept as historical record.** This plan scoped `WRITE_ENDPOINTS` to
+> four narrow operational verbs and explicitly excluded any agent-authoring
+> endpoint (§5) — correct reasoning for what existed at the time (no `hitlConfig`
+> support on the standard agent-setup path, no escalation-flag mechanism, no
+> request-pinning). Both landed later, and the operator now has real
+> create-and-modify capability over agents, agent groups, and every
+> workflow-extension store, each still individually approval-gated. The
+> authoritative reference for what is actually granted today is the doc comment
+> on `WRITE_ENDPOINTS` in `EDDI-Manager/src/lib/operator/tool-scopes.ts` — treat
+> this document as explaining the ORIGINAL reasoning, not the current state; §5's
+> stale exclusion is struck through below rather than silently deleted, since the
+> reasoning it once carried still explains why the later change was carefully
+> scoped rather than done casually.
+
 ## 0. Premise
 
 The HITL gate itself is complete and fires before execution (`AgentOrchestrator.java:1169-1253`; gated requests never reach `executeSingleToolCall`). Nothing in the gate needs changing. The blocker is **provisioning** plus **verification**: `setup-api` cannot install a gate, and `tool-scopes.ts` is a provisioning-time constant, not a runtime boundary — so "writes are gated" must be an *asserted, read-back fact*, not an assumption.
@@ -91,7 +105,7 @@ Because `requireApproval` is `["http:*"]`, **anything later added to `WRITE_ENDP
 
 ## 3. Pause UX
 
-**Where.** Inline in the operator chat (`components/operator/operator-chat.tsx`), after the last message — the precedent is `discussion-transcript.tsx:579-599`, which already renders `ApprovalBanner` inside a live transcript. Not the approvals page: `pages/approvals.tsx:332-344` deliberately refuses to decide `TOOL_CALL` pauses and links out instead. That page remains the correct *someone else's queue* fallback and needs no change.
+**Where.** Inline in the operator chat (`components/operator/operator-chat.tsx`), after the last message — the precedent is `discussion-transcript.tsx:579-599`, which already renders `ApprovalBanner` inside a live transcript. `pages/approvals.tsx` deliberately refused to decide `TOOL_CALL` pauses at the time this was written, for the same reason `WRITE_ENDPOINTS` stayed empty: the approver had nothing but a client-side `operationId` guess to review. That reason no longer holds once request pinning ships (§3 note below, and EDDI#627) — the inbox now expands a `TOOL_CALL` row in place into the same `ApprovalBanner`/`RequestPreview` the operator chat uses, so any `eddi-admin`, not only whoever is at the operator screen, can decide a gated write.
 
 **Detecting the pause.** There is no SSE pause event on the 1:1 surface (`RestAgentEngineStreaming.java:66-138`). Two paths, both already proven in `use-chat.ts`:
 1. `use-operator-chat.ts:218` currently does `if (event.type === "done") break;` and discards the payload. Parse it: `conversationState === "AWAITING_HUMAN"`, plus `hitlPauseType` and the names-only `hitlPendingToolCalls` (`ConversationMemoryUtilities.java:268-307`) which ride on the snapshot for free.
@@ -173,12 +187,12 @@ The gauge is the one worth alerting on: it is the machine-readable form of "writ
 
 ## 5. What I would NOT do
 
-- **Not populate `WRITE_ENDPOINTS` beyond the four.** Specifically never bind, regardless of approval: `setup-api`/`setup` (one call provisions a *new* agent with an arbitrary `endpoints` filter and no gate — complete escape from the allow-list); `POST /agents/{id}/resume` (self-approval — `HitlAccessGuard` has no "not the requester" check); `PATCH /agents/{id}/state` and `/cancel` (clears `AWAITING_HUMAN` under the gate); `PUT /variablestore/variables/...` (the operator's own config blob lives at key `platform.operator`, `operator.ts:74`); all `/secretstore` writes; `/backup/import*`; `apicallstore`/`mcpcallsstore`/`channelstore` writes; `/ragstore/.../ingest`; `usermemorystore` writes; `AgentTriggerStore` writes; `/administration/quotas`; `DELETE /administration/orphans`.
+- ~~Not populate `WRITE_ENDPOINTS` beyond the four. Specifically never bind, regardless of approval: `setup-api`/`setup` (one call provisions a *new* agent with an arbitrary `endpoints` filter and no gate — complete escape from the allow-list)~~ — superseded (see the banner at the top). `SetupAgentRequest` gained the same `hitlConfig` field `CreateApiAgentRequest` already had, both are now bound, and `escalation-flags.ts`'s `agentCreatedWithoutGate`/`agentCreatedWithBroadEndpoints` checks surface exactly the two risks named here (no gate; unbounded `endpoints`) to the approver above the raw JSON. `apicallstore`/`mcpcallsstore` writes are bound too, for the same "modify an existing agent's tool wiring" reason the other workflow-extension stores are — narrower than blanket "never," and still gated like everything else. `POST /agents/{id}/resume` (self-approval), `PATCH /agents/{id}/state` and `/cancel`, `PUT /variablestore/variables/...` (the operator's own config), all `/secretstore` writes, `/backup/import*`, `channelstore` writes, `/ragstore/.../ingest`, `usermemorystore` writes, `AgentTriggerStore` writes, `/administration/quotas`, and `DELETE /administration/orphans` remain excluded — this correction is scoped to exactly the two items it names, not a blanket reopening.
 - **Not upgrade an existing read-only operator in place.** Changing scope **re-provisions** a new agent (fresh `setup-api`, gate on v1) and resets the old one via `resetOperator`. An in-place `PUT` would leave an older, ungated version of the same agent that a bound `deployAgent` call could roll back to. This is why "every version carries the gate" is the read-back invariant rather than "the current version does".
 - **Not change backend `AUTO_APPROVE` semantics.** Explicit `toolApprovals.timeoutPolicy: AUTO_APPROVE` is honored (`ConversationService.java:2242-2247`) and existing agents may rely on it. Refuse it Manager-side for the operator only.
 - **Not enable Slack approvals for operator writes.** `SlackHitlSupport.java:69,75` truncates to 5 calls and 300 chars of arguments while keeping the same buttons — the realistic rubber-stamping surface.
 - **Not add `POST /agents/{id}/resume/stream`.** Real gap (`ConversationService.resumeConversation` already accepts a handler; only the REST adapter passes `null`), but a separate backend PR. A post-decision re-read is adequate.
-- **Not touch `pages/approvals.tsx`.** Its refusal to decide TOOL_CALL pauses is correct.
+- ~~Not touch `pages/approvals.tsx`.~~ Superseded once request pinning shipped — see §3.
 - **Not build "approve all".**
 - **Not treat `tool-scopes.ts` as a security boundary.** It is applied at provisioning time only. Say so in the file comment.
 

src/main/java/ai/labs/eddi/engine/api/IConversationService.java

@@ -308,6 +308,16 @@ CancelOutcome cancelConversation(String conversationId,
      *            the human approval/rejection decision
      * @param responseHandler
      *            optional callback — may be null for fire-and-forget
+     * @throws IllegalArgumentException
+     *             {@code decision} is null, carries no top-level {@code verdict},
+     *             or its {@code toolDecisions} fail validation — maps to HTTP 400;
+     *             checked before the AWAITING_HUMAN->IN_PROGRESS CAS, so the
+     *             pause is never consumed by a malformed request. Every current
+     *             caller (REST, Slack, MCP, timeout auto-resolution) already
+     *             guarantees a non-null verdict before calling this method; this is
+     *             the one place that guarantee is enforced rather than assumed, so
+     *             a future caller that forgets fails loudly here instead of
+     *             silently reaching the tool-execution gate with nothing to check.
      * @throws IllegalStateException
      *             wrong-state conflict (not AWAITING_HUMAN, or agent not deployed)
      *             — maps to HTTP 409; the pause is preserved/restored