feat(operator): foundation for an agent that can safely write

docs/changelog.md

@@ -3,6 +3,43 @@
 > **Purpose:** Living document tracking all changes, decisions, and reasoning during implementation. Updated as work progresses for easy reference and review.
 
 
+---
+
+## 🔑 feat(operator): the foundation for an agent that can safely write (2026-07-29)
+
+**Repo:** EDDI (`feat/operator-write-foundation`)
+
+Groundwork for a workspace operator agent that manages a deployment — creating and updating agents and groups — while acting as the person chatting to it. Five commits, no new capability granted: the operator's endpoint allow-list is untouched and still read-only. What changes is that widening it is now safe, where before it was neither safe nor functional.
+
+**Generated writes did not work at all.** `McpApiToolBuilder.buildBodyTemplate` emitted Qute variables for a request body but registered none of them, and `AgentOrchestrator` builds the tool schema from `ApiCall.getParameters()` alone. So the model had no documented way to fill a body; with strict rendering off the variables rendered empty and every generated `POST`/`PUT`/`PATCH` went out structurally valid and semantically empty. Adding a write endpoint before this would have produced garbage requests that fail at the far end rather than at the config.
+
+**The body is now one model-written variable.** A per-property template looked more helpful and was worse three ways: every variable became a *required* tool parameter (a `Map<String,String>` has nowhere to record optionality, so a `PATCH` of one field forced the model to restate all the others); values were substituted into the JSON unescaped, since the templating engine runs in `TEXT` mode, so a value containing a quote could break the body or add fields the schema never declared; and the HITL card shows tool *arguments*, so "the arguments are the request" only holds if the body is one of them. The shape a decomposed template implied now lives in the parameter description, which names each property with its type and marks which are required.
+
+**Approval patterns can address the endpoint a tool calls.** Names come from `operationId` or a slug and drift when a spec changes, and `ToolApprovalGate` allows an unmatched call — so a renamed or newly generated write arrived ungated and silently. Method and path were available and discarded one line into registration; they now travel alongside, so a pattern may match a bare name, `source:name`, or `source.method:path`:
+
+```json
+{ "requireApproval": ["http.post:*", "http.put:*", "http.patch:*", "http.delete:*"],
+  "exempt": ["http.get:*"] }
+```
+
+gates every mutation without naming a tool, while `http.post:/agentstore/agents` addresses exactly one. Both speak the same `METHOD /path` vocabulary as the endpoint allow-list, so the two can be generated from one source instead of maintained in two. Documented in [`docs/hitl.md`](hitl.md).
+
+**Design decision — enumerate downward, never upward.** Whether something is gated stays in `requireApproval`/`exempt`; per-endpoint tuning may only *lower* friction. A missed exemption costs an approval prompt; a missed requirement is an ungated write. The same reasoning restricts the method qualifier to `http`: it is the only source whose tools record an endpoint, so `mcp.post:` is rejected at save time rather than saved as a pattern nothing could ever match.
+
+**MCP tool calls can now run as the chatting user.** They previously ran as whatever static credential the config named, and a `${caller:token}` there passed through the global-variable and secret resolvers untouched and was sent as the literal placeholder — failing silently rather than closed. The transport supported this all along: `customHeaders` has three overloads and EDDI used the constant one. The per-request `McpHeadersSupplier` overload makes the credential per-call while the client stays cached.
+
+**Discovery must be told apart from invocation explicitly.** The first version of this decided by asking whether a caller was bound to the thread — but discovery runs *inside* the turn, on a thread that is bound, so `initialize` and `tools/list` went out with the first caller's token. Since the client is cached, that session was then reused by everyone after them, and the tool list reflected one user's permissions while being offered to the next. langchain4j distinguishes the two — `DefaultMcpClient.listTools()` delegates with a null `InvocationContext` while `McpToolExecutor` always builds one — but it does not enforce anything; EDDI has to read `McpCallContext.invocationContext()` and act on it, which it now does. Only a tool call carries the caller; discovery goes unauthenticated on a caller-bound config and the server decides.
+
+**Fixed in passing — a privilege bug.** MCP clients were cached by URL alone, so two agents naming the same server with *different* credentials silently shared whichever client was constructed first. The key now includes a digest of the configured credential: a digest so a literal key never becomes a map key, taken unresolved so configs sharing a vault reference still share a client. This does not multiply clients per user — a caller-bound config yields one client whose supplier reads the caller per request.
+
+**Not solved, deliberately.** On an expired MCP session the transport retries `initialize()` on an HTTP callback thread where the caller binding does not exist. That path now sends the request unauthenticated rather than falling back to the static key under the caller's intent — a visible failure instead of the wrong authority.
+
+**Corrections to earlier analysis, recorded because they changed decisions.** `McpCallsConfiguration.toolsWhitelist` does *not* fail closed — `AgentOrchestrator` skips filtering when the list is empty, the same shape as the approval gate. `toolApprovals` is not agent-only: `LlmConfiguration` carries a per-task override that fully replaces the agent-level block. And a `PUT` on a config does not reach a running agent, because `HistorizedResourceStore.update` creates `version + 1` while agents pin a version — so self-modification takes a chain of write, re-point, redeploy, each independently gated. The thing to guard is whatever can re-point a version reference, not `PUT` in general.
+
+**Verification.** 566 tests pass on a clean build across the affected suites; checkstyle clean. Each fix mutation-checked by reverting it. Four tests written during this work were found vacuous by that check and rewritten — three asserted against a helper or state seeded through the very wrapper under test, one seeded a thread binding that the fix then restored, so it passed either way.
+
+**Next:** per-endpoint approval friction (`timeoutPolicy`, `approvalTimeout` and the pause message are still single scalars for every gated tool), agent-readable documentation (EDDI's MCP client never reads MCP *resources*, so `eddi://docs/*` is reachable from a desktop client but not from an agent), widening the allow-list, and the Manager scope picker and approval surface.
+
 ---
 
 ## 🐳 fix(demo): the Open WebUI seeder did nothing on a second run (2026-07-29)

docs/hitl.md

@@ -262,9 +262,11 @@ Patterns are matched by `ToolApprovalPatterns` / `ToolApprovalGate`:
 
 - **`*` is the only wildcard** — it matches any run of characters (including empty). Every other character is a quoted literal, so compilation is **ReDoS-safe**.
 - **Source-qualified or bare.** A pattern may carry a known source prefix (`mcp:read_*`, `http:*`) or match the bare tool name (`delete_account`). Each call is tested against `source:name` **first**, then the bare dispatch name — a tool with an unknown source still matches bare-name patterns (fail-safe).
-- **Known sources** (the only accepted prefixes): `builtin`, `http`, `mcp`, `a2a`, `dynamic`, `memory`, `recall`. An unknown prefix is rejected at save time with a typo suggestion (Levenshtein ≤ 2).
-- **Case-sensitive.** Patterns match tool names exactly; `Delete_*` does not match `delete_account`.
-- Patterns may not start or end with `:`, and are capped at 256 characters. Allowed characters: `A-Za-z0-9_-.:*`.
+- **Endpoint-qualified** (`http` tools only). A pattern may also address *what a tool calls* rather than what it is named: `http.post:*` matches every POST, `http.post:/agentstore/agents` matches exactly one endpoint. This is the robust form for tools generated from an OpenAPI spec, whose names come from `operationId` or a slug and change when the spec does — `http.post:*` keeps gating every mutation even when a name changes or a new endpoint appears. The path is matched **as the httpcall config declares it**, normalised to a leading slash (an absolute URL contributes only its path).
+- **Known sources** (the only accepted prefixes): `builtin`, `http`, `mcp`, `a2a`, `dynamic`, `memory`, `recall`. An unknown prefix is rejected at save time with a typo suggestion (Levenshtein ≤ 2). Only `http` may carry a method qualifier: it is the only source whose tools record an endpoint, so `mcp.post:` is rejected rather than saved as a pattern nothing could match.
+- **Allowed characters:** `A-Za-z0-9_-.:*` plus `/` `{` `}` for endpoint path templates. Braces are matched literally, not as regex quantifiers.
+- **Case-sensitive.** Patterns match tool names exactly; `Delete_*` does not match `delete_account`. An endpoint path is matched as the httpcall config declares it, so its case must match too; the method half is normalised to lower case.
+- Patterns may not start or end with `:`, and are capped at 256 characters.
 
 ### Precedence
 

src/main/java/ai/labs/eddi/engine/hitl/tools/ToolApprovalGate.java

@@ -32,6 +32,26 @@ public record GateResult(List<ToolExecutionRequest> gated,
 
     public GateResult classify(List<ToolExecutionRequest> batch, Map<String, String> toolSources,
                                ToolApprovalsConfig cfg, Set<String> clearedCallIds) {
+        return classify(batch, toolSources, Map.of(), cfg, clearedCallIds);
+    }
+
+    /**
+     * As {@link #classify(List, Map, ToolApprovalsConfig, Set)}, additionally
+     * matching patterns against what an http tool actually calls.
+     * <p>
+     * A pattern may address a tool three ways: its bare name, {@code source:name},
+     * or — for httpcall tools — {@code source.method:path}, e.g.
+     * {@code http.post:/agentstore/agents}. The third form is the useful one for a
+     * generated API client: names come from {@code operationId} or a slug and drift
+     * when the spec changes, whereas {@code http.post:*} gates every mutation
+     * whether or not anyone maintained a list, and
+     * {@code http.post:/agentstore/agents} says exactly which one needs more care.
+     *
+     * @param toolEndpoints
+     *            tool name to {@code method:path}; empty for non-http tools
+     */
+    public GateResult classify(List<ToolExecutionRequest> batch, Map<String, String> toolSources,
+                               Map<String, String> toolEndpoints, ToolApprovalsConfig cfg, Set<String> clearedCallIds) {
         if (cfg == null || cfg.getRequireApproval() == null || cfg.getRequireApproval().isEmpty()) {
             return new GateResult(List.of(), List.copyOf(batch), Map.of());
         }
@@ -57,11 +77,14 @@ public GateResult classify(List<ToolExecutionRequest> batch, Map<String, String>
             }
             String source = toolSources.get(request.name());
             String qualified = source != null ? source + ":" + request.name() : null;
-            if (firstMatch(exempt, qualified, request.name()) != null) {
+            // e.g. "http" + "." + "post:/agentstore/agents"
+            String endpoint = toolEndpoints.get(request.name());
+            String endpointQualified = source != null && endpoint != null ? source + "." + endpoint : null;
+            if (firstMatch(exempt, qualified, endpointQualified, request.name()) != null) {
                 allowed.add(request);
                 continue;
             }
-            CompiledPattern match = firstMatch(require, qualified, request.name());
+            CompiledPattern match = firstMatch(require, qualified, endpointQualified, request.name());
             if (match != null) {
                 gated.add(request);
                 if (request.id() != null) {
@@ -84,8 +107,11 @@ private static List<CompiledPattern> compileAll(List<String> globs) {
         return globs.stream().map(g -> new CompiledPattern(g, ToolApprovalPatterns.compile(g))).toList();
     }
 
-    private static CompiledPattern firstMatch(List<CompiledPattern> patterns, String qualified, String bare) {
+    private static CompiledPattern firstMatch(List<CompiledPattern> patterns, String qualified, String endpointQualified, String bare) {
         for (CompiledPattern cp : patterns) {
+            if (endpointQualified != null && cp.pattern().matcher(endpointQualified).matches()) {
+                return cp;
+            }
             // bare (= request.name()) can be null: langchain4j's ToolExecutionRequest
             // does not guarantee a non-null name (some providers emit malformed tool
             // calls). Guard both matchers so a null name matches nothing and the call

src/main/java/ai/labs/eddi/engine/hitl/tools/ToolApprovalPatterns.java

@@ -18,7 +18,20 @@
  */
 public final class ToolApprovalPatterns {
     public static final List<String> KNOWN_SOURCES = List.of("builtin", "http", "mcp", "a2a", "dynamic", "memory", "recall");
-    private static final Pattern LEGAL_CHARS = Pattern.compile("[A-Za-z0-9_\\-.:*]+");
+    /**
+     * Also permits {@code / { }} so a pattern can name an endpoint path template
+     * such as {@code http.post:/agentstore/agents/{id}}. Braces are safe in the
+     * compiled regex because {@link #compile} quotes every non-wildcard segment.
+     */
+    private static final Pattern LEGAL_CHARS = Pattern.compile("[A-Za-z0-9_\\-.:*/{}]+");
+
+    /**
+     * The only source whose tools carry an endpoint, so the only one qualifiable.
+     */
+    private static final String HTTP_SOURCE = "http";
+
+    /** HTTP methods that may qualify the http source, as {@code http.post:…}. */
+    private static final List<String> KNOWN_METHODS = List.of("get", "post", "put", "patch", "delete", "head", "options");
     private static final int MAX_LENGTH = 256;
 
     private ToolApprovalPatterns() {
@@ -50,7 +63,8 @@ public static Optional<String> validate(String pattern) {
             return Optional.of("pattern exceeds " + MAX_LENGTH + " characters");
         }
         if (!LEGAL_CHARS.matcher(pattern).matches()) {
-            return Optional.of("pattern '" + pattern + "' contains illegal characters — allowed: A-Za-z0-9_-.:* (tool names never contain spaces)");
+            return Optional.of("pattern '" + pattern + "' contains illegal characters — allowed: A-Za-z0-9_-.:* plus / { } for endpoint paths "
+                    + "(tool names never contain spaces)");
         }
         if (pattern.startsWith(":") || pattern.endsWith(":")) {
             return Optional.of("pattern '" + pattern
@@ -59,14 +73,36 @@ public static Optional<String> validate(String pattern) {
         int colon = pattern.indexOf(':');
         if (colon > 0) {
             String prefix = pattern.substring(0, colon);
-            if (!prefix.contains("*") && !KNOWN_SOURCES.contains(prefix)) {
+            if (!prefix.contains("*") && !KNOWN_SOURCES.contains(prefix) && !isMethodQualifiedSource(prefix)) {
                 return Optional.of("unknown tool source prefix '" + prefix + ":' in pattern '" + pattern + "'"
-                        + suggestionFor(prefix) + " — known sources: " + String.join(", ", KNOWN_SOURCES));
+                        + suggestionFor(prefix) + " — known sources: " + String.join(", ", KNOWN_SOURCES)
+                        + "; only the http source may be qualified by method, e.g. 'http.post:'");
             }
         }
         return Optional.empty();
     }
 
+    /**
+     * Whether a prefix is {@code http.<httpMethod>}, e.g. {@code http.post}.
+     * <p>
+     * Deliberately restricted to {@code http}: it is the only source whose tools
+     * carry an endpoint (see {@code AgentOrchestrator} — the mcp and a2a branches
+     * register a source and no endpoint). Accepting {@code mcp.post:} would let a
+     * pattern save cleanly and then match nothing at runtime, and since the gate
+     * allows an unmatched call, that is an ungated write — the very failure this
+     * validation exists to prevent. Widen this only together with whatever
+     * populates endpoints for the other source.
+     * <p>
+     * A typo like {@code http.pots:} is reported for the same reason.
+     */
+    private static boolean isMethodQualifiedSource(String prefix) {
+        int dot = prefix.indexOf('.');
+        if (dot <= 0) {
+            return false;
+        }
+        return HTTP_SOURCE.equals(prefix.substring(0, dot)) && KNOWN_METHODS.contains(prefix.substring(dot + 1));
+    }
+
     private static String suggestionFor(String prefix) {
         for (String known : KNOWN_SOURCES) {
             if (levenshtein(prefix, known) <= 2) {