Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -837,8 +837,10 @@ Resolution is narrow and fails loudly rather than degrading quietly:
- **Same origin only** — released only to the exact `scheme://host:port` the
caller addressed (read from the inbound request, not config), so a config
naming a third-party host cannot exfiltrate the token.
- **Headers only** — `${caller:token}` in a query parameter is rejected.
`${caller:userId}` is allowed in headers and query parameters.
- **Headers only** — `${caller:token}` in a query parameter, request body or
path is rejected. `${caller:userId}` is allowed in headers and query
parameters. An MCP server's `apiKey` may also carry it, which sends the tool
call as the chatting user (see [`docs/mcp-server.md`](docs/mcp-server.md)).
Comment thread
ginccc marked this conversation as resolved.
Resolved
- **Authenticated turns only** — scheduled jobs and triggers cannot satisfy it.
- **Fails closed** — an unsatisfiable reference errors instead of sending
`"Bearer "` with an empty token.
Expand Down
39 changes: 39 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,45 @@
> **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(runtime): code-review findings wave 3 — concurrency, lifecycle, cancellation, graceful shutdown, Dream wiring (2026-07-28)
Expand Down
8 changes: 5 additions & 3 deletions docs/hitl.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions docs/httpcalls.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ headers are scrubbed before the request is recorded.

Set `eddi.caller-identity.enabled=false` to forbid the feature outright.

The same reference works in an **MCP server's `apiKey`**, so a tool call reaches
that server as the chatting user rather than as a standing service principal.
Only the tool call carries the caller — the handshake and `tools/list` do not,
because the client is cached and a session opened with one user's token would be
reused by everyone after them. See
[`mcp-server.md`](mcp-server.md#calling-an-mcp-server-as-the-chatting-user).

### Running behind a reverse proxy

The origin is taken from the inbound request as EDDI sees it. Behind a
Expand Down
53 changes: 50 additions & 3 deletions docs/mcp-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ EDDI uses **Streamable HTTP** transport, served by the Quarkus MCP Server extens
| Tool | Description |
| ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `setup_agent` | Create a fully working agent in one call: creates behavior rules, LangChain config, optional output/greeting, package, agent, and deploys. Supports built-in tools, quick replies, and sentiment analysis. Default: `anthropic`/`claude-sonnet-4-6` |
| `create_api_agent` | Create an agent from an OpenAPI 3.0/3.1 spec. Parses the spec, generates HttpCalls configs (grouped by API tag), creates the full pipeline, and deploys. Supports endpoint filtering, base URL override, and auth header propagation |
| `create_api_agent` | Create an agent from an OpenAPI 3.0/3.1 spec. Parses the spec, generates HttpCalls configs (grouped by API tag), creates the full pipeline, and deploys. Supports endpoint filtering, base URL override, and auth header propagation. A generated write tool takes the whole request body as one `requestBody` parameter — see below |

### Schedule Management Tools (6)

Expand Down Expand Up @@ -650,13 +650,60 @@ External MCP servers are configured as **`mcpcalls` workflow extensions** — a
| ---------------- | ---------- | -------- | --------- | ------------------------------------------------------------------------------------------------------------------------ |
| `mcpServerUrl` | string | **Yes** | — | MCP server URL |
| `name` | string | No | — | Human-readable name for logging |
| `transport` | string | No | `"http"` | Informational only today — `McpToolProviderManager` always connects via Streamable HTTP |
| `apiKey` | string | No | — | API key, sent as `Authorization: Bearer <key>`. Resolved through global variables and `${vault:key}` references |
| `transport` | string | No | `"http"` | Only Streamable HTTP is implemented; an unimplemented value is rejected as invalid configuration rather than silently substituted |
| `apiKey` | string | No | — | API key, sent as `Authorization: Bearer <key>`. Resolved through global variables and `${vault:key}` references, or `${caller:token}` to call as the chatting user (see below) |
| `timeoutMs` | long | No | `30000` | Connection and request timeout in milliseconds |
| `toolsWhitelist` | string[] | No | — | If non-empty, only these tool names are exposed (names as returned by the server's `tools/list`) |
| `toolsBlacklist` | string[] | No | — | Tool names to exclude. Applied *after* the whitelist |
| `mcpCalls` | object[] | No | — | Deterministic, action-triggered tool bindings (see *Pipeline mode* below). Omit for agent-mode-only servers |

#### How `create_api_agent` builds a write tool's body

A generated `POST`/`PUT`/`PATCH` tool takes the **entire request body as a single
`requestBody` parameter**, whose description names the schema's properties, their
types, and which are required. The model writes the JSON itself.

It is worth knowing why, because the obvious alternative is worse. Decomposing the
schema into one parameter per property means every one becomes *required* (an
`ApiCall`'s parameter map has nowhere to record optionality), so a `PATCH` of one
field forces the model to restate all the others and a partial update silently
becomes a full overwrite. It also substitutes model-written values into JSON
unescaped, so a value containing a quote can break the body or add fields the
schema never declared.

The whole-body form matters most under [HITL approval](hitl.md): the approval card
shows tool **arguments**, so "what the approver sees is what gets sent" only holds
while the body is one of them.

#### Calling an MCP server as the chatting user

Set `apiKey` to `${caller:token}` and the tool call carries the identity of the
person chatting, instead of a standing service credential:

```json
{ "mcpServerUrl": "https://eddi.example/mcp", "apiKey": "${caller:token}" }
```

The same guarantees apply as for apicall headers — same origin only, fails
closed rather than sending a placeholder, never persisted. See
[`httpcalls.md`](httpcalls.md#calling-as-the-signed-in-user).
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

Two behaviours worth knowing, because they are deliberate:

- **Only tool calls carry the caller.** The `initialize` handshake and
`tools/list` are sent unauthenticated, because the client is cached: a session
opened with one user's token would be reused by everyone after them, and a
tool list reflecting one user's permissions would be offered to the next. If
your server requires authentication to *list* tools, use a static key.
- **Clients are cached per credential, not per URL.** Two agents pointing at the
same server with different keys get separate clients. A caller-bound config
still yields one shared client — the credential is applied per request, so
there is no client per user.

A `${caller:token}` key with `eddi.caller-identity.enabled=false` is rejected as
invalid configuration when the server is validated, rather than failing on every
tool call.

**Step 2 — add an `mcpcalls` step to the agent's workflow**, before the LLM step:

```json
Expand Down
2 changes: 1 addition & 1 deletion docs/security.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ When an LLM is given access to tools, every argument it supplies must be treated

## Caller Identity Forwarding — `CallerIdentityResolver`

**Applies to:** apicall headers that reference `${caller:token}` / `${caller:userId}`.
**Applies to:** apicall headers, and an MCP server's `apiKey`, that reference `${caller:token}` / `${caller:userId}`.

An agent that calls an API needs a credential. Baking a static one into the
config is the wrong shape when the API is EDDI's own: an OIDC token expires
Expand Down
Loading
Loading