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

docs/changelog.md

@@ -3,6 +3,47 @@
 > **Purpose:** Living document tracking all changes, decisions, and reasoning during implementation. Updated as work progresses for easy reference and review.
 
 
+---
+
+## 🔒 feat(hitl): approval binds to the resolved request, not the tool name (2026-08-03)
+
+**Repo:** EDDI (`feat/operator-request-fingerprint`, branched from `main` after PR #625 merged — the per-endpoint-friction entry below plus setup-api gate provisioning, docs-over-REST, and `mcpServerUrls`; builds on the foundation laid in [#622](#-featoperator-the-foundation-for-an-agent-that-can-safely-write-2026-07-29))
+
+Closes the gap the operator write-scope plan (`planning/operator-write-scope-plan.md` §3) flagged as the reason `WRITE_ENDPOINTS` had to stay empty: an approver of a gated `http` call saw the tool's name and the model's raw arguments, never the actual request. Method, path, query and body are only produced inside `ApiCallExecutor#execute`, **after** approval — so what an approver signed off on and what ran could, in principle, differ.
+
+**Four commits, one seam apiece:**
+
+1. `IApiCallExecutor#resolve` — builds the request `execute` would send, without sending it. Deliberately weaker than `execute`: it skips `preRequest.propertyInstructions` because those write to conversation memory and previewing a call must never do that, so a call that has them comes back with no fingerprint rather than one that doesn't match what execution will actually build. Shares one redaction definition (`RequestRedactor`, extracted from `ApiCallExecutor`'s private scrub) between the conversation-memory debug record and the approval preview, so the two paths cannot drift apart on what counts as a credential.
+2. Gate time: each gated httpcall tool is resolved, and a redacted preview plus a SHA-256 fingerprint are persisted on the pause (`PendingToolCall.requestPreview` / `.requestFingerprint`). The fingerprint deliberately hashes the **redacted** request, not the live one — `ApiCallExecutor` resolves `${caller:token}` into `Authorization`, the approver is routinely a different person than whoever's turn raised the pause, and fingerprinting the live header would mismatch on every cross-user approval (the normal case), which would just get the check disabled. Canonicalization is length-prefixed rather than delimiter-joined, so a body containing a crafted newline cannot impersonate an extra header field and collide.
+3. Resume time: an approved, pinned call is re-resolved and refused — synthetic `NOT_EXECUTED`, audited as `hitl.tool.request_changed` (tool + callId + reason, never the request) — if the fingerprint moved. This is the actual enforcement; everything before it was groundwork. Three situations fail *closed* rather than being waved through: the tool vanished from the workflow across the pause, re-resolution throws, or the call's config gained `preRequest.propertyInstructions` mid-pause. "Cannot verify" is a different answer than "unchanged" — treating it as the latter would make reconfiguring an agent while a human decides the way around the guard.
+4. `eddi.operator.write.approval{decision=approved|rejected|timeout}` — the rubber-stamping signal the plan's metrics table calls for, emitted the instant a gated call's verdict is resolved regardless of what happens to it afterwards. `timeout` is its own bucket (`decidedBy == "system:timeout"`, from `HitlTimeoutHandler`) rather than folded into `approved`/`rejected` — an unattended auto-approval inflating "approved" would defeat the point of the metric.
+
+**Two metrics the backend cannot honestly emit itself.** `eddi.operator.canary` (+`.duration`) and `eddi.operator.gate.verified` describe facts the Manager establishes client-side — the write canary is a synthetic conversation it drives in the browser, gate verification is it re-reading every version of the operator agent document — and this codebase has no first-class notion of "the operator" to hang a server-side event on. `POST /administration/operator/{canary-result,gate-status}` (`eddi-admin`) exists purely to relay those already-established facts onto `/q/metrics`, so on-call doesn't need a Manager tab open. **Not a verification endpoint** — a report is trusted at face value, which is why it sits behind the same tier that can provision the operator at all. The gauge defaults to 0 before any report arrives, which is indistinguishable from "activated, and broken"; that ambiguity is real and this signal alone doesn't resolve it.
+
+**Verification.** Full `mvnw validate` + `mvnw test` run checked against the documented environmental baseline (no-network loopback failures in `Web*ToolTest`); none of the touched classes appear in the failure list. The fingerprint discrimination properties (method/URI/query/body/header changes each move the hash; header casing, ordering, and redacted-credential values do not) and the enforcement decision (pinned+changed → refused; unpinned, amended, or matching → proceeds; unresolvable → fails closed) are both covered with dedicated unit tests. Four mutations applied against the enforcement path, each confirmed to kill exactly the tests guarding that branch; one applied against the timeout-tagging logic, confirmed to kill only the two timeout tests and leave approved/rejected untouched.
+
+Documented in [`docs/hitl.md`](hitl.md) (new §"Request pinning — approval binds to a request, not a tool name"; Operations metrics list extended).
+
+### Two more commits on the same branch: the preview reaches the wire, and everything above gets a real metric (2026-08-03)
+
+The pinning above persisted `requestPreview`/`requestFingerprint` on the pause record, but nothing external ever read them back — `RestAgentEngine.buildToolCallPauseDetails` builds its response as an explicit field-by-field map, so a new model field is invisible to a caller until something puts it there. `GET .../approval-status` now surfaces `requestPinned` and, when pinned, the redacted `requestPreview` (`method`/`uri`/`queryParams`/`headers`/`body`/`bodyTruncated`) per call — this is what an approver actually reads, replacing what the Manager previously had to guess by reconstructing an `operationId` against a separately-fetched spec. The raw fingerprint stays internal; it means nothing to a human. `namesOnlyPendingToolCalls` — the security-motivated projection for the generic (non-approver) read surfaces — needed no code change, since it's an explicit allow-list and a field it was never told to copy is absent by construction; only its doc comment needed the two new field names added.
+
+Also lands `eddi.operator.write.approval{decision=approved|rejected|timeout}` (a real backend-native metric — the orchestrator observes every gated-call verdict directly) and the relay endpoints `POST /administration/operator/{canary-result,gate-status}` for the two metrics the backend cannot honestly emit itself.
+
+**Verification note worth recording**: this repo's `@Nested`-only JUnit classes report `Tests run: 0` in the plain-text surefire report even when every test inside passed — already documented in memory from a prior session, and it still cost real time to rediscover mid-session before the XML `<testsuite tests="…">` attribute was checked. Both touched test classes' real results: `RestAgentEngineToolPauseDetailsTest` 11/11, `ConversationMemoryUtilitiesHitlTest` 8/8.
+
+### The preview leaked the body it was supposed to protect (2026-08-03)
+
+Found while scoping the operator's authoring UI, and the reason that scope changed: `RequestRedactor` only ever touched `headers`. Both consumers of a resolved request — the debug record persisted to the conversation document and the approval preview shown to a human — passed the **body** through verbatim. A config write carries its credential in the body, not a header, so a `POST` creating an agent with a provider key would have shown that key in plaintext to whoever approved the pause — routinely a different admin than the one whose turn raised it.
+
+Fixed by giving `RequestRedactor` a `redactBody` (delegating to `SecretRedactionFilter`, the same value-shape scan already behind `argumentsRedacted` — one filter for one class of data, rather than a second scheme that would drift), wired into both `redactRequestMap` and `ResolvedRequest#of`.
+
+The ordering matters more than the redaction. Headers stay fingerprinted **redacted** for the cross-user-approval reason documented above; the body is fingerprinted **raw** and only the stored copy is redacted, because a body has no equivalent legitimate variance (`${caller:token}` is header-only; `${vault:…}` resolves identically both times). Redacting first would hash two *different* credentials to one marker and so to one fingerprint — a swapped secret would pass the pre-execution re-check as an unchanged request. `ResolvedRequest#of` does the redaction itself so no call site can get that order wrong; a test asserts two distinct keys produce distinct fingerprints, and it fails if the redaction is hoisted above the hash. The fingerprint is never exposed to a client, so hashing raw reveals nothing.
+
+The limitation is stated rather than papered over: value-shape matching catches `sk-…`, `sk-ant-…`, bearer tokens and vault refs, not a hand-rolled secret in a generically named field. That is the same limitation `argumentsRedacted` already carries.
+
+**What's left before `WRITE_ENDPOINTS` can actually be populated:** it already has been, on the Manager side — see that repo's own changelog for the write canary, the four curated endpoints, and real `read_write` scope selection. What remains is Manager-side only: render this backend's `requestPreview` in the approval banner in place of the client-side `operationId` reconstruction it was always labelled as a stand-in for, and the agent/group authoring UI (iteration 7).
+
 ---
 
 ## 🎚️ feat(hitl): per-endpoint approval friction (2026-08-01)

docs/hitl.md

@@ -366,6 +366,24 @@ 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 pinned, `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. `requestPinned: false` with `requestPreview: null` means exactly what it says — nothing to preview, not a resolution failure the caller should treat as an error — see the unpinned/fail-closed cases above.
+
+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 exposed to any client, so hashing the raw body reveals nothing.
+
+Body redaction is 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: every non-`http` tool (builtin/mcp/a2a — there is no HTTP request on this side of the boundary to pin), and any `http` call whose config carries `preRequest.propertyInstructions` (those write to conversation memory, so resolving them ahead of execution would apply them twice). 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 `preRequest.propertyInstructions` mid-pause (a pinned call becoming unpinnable). 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 +451,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.