feat(operator): Platform Operator agent (P1, read-only) - #126
Conversation
An opt-in, admin-activated agent that inspects this EDDI deployment and
explains what it finds. Off by default. Provisioned through setup-api as an
API Agent whose tools are generated from EDDI's own OpenAPI spec and scoped
by a read-only allow-list.
What ships:
- src/lib/operator/tool-scopes.ts - allow-list of GET endpoints (never a
deny-list: a deny-list silently grants any endpoint added later). Write
scope is empty and unreachable until an approval handler exists.
- src/lib/operator/system-prompt.ts - non-editable safety preamble treating
all tool output as untrusted data, plus an editable body.
- src/lib/api/operator.ts - one atomic platform.operator config blob,
full-spec fetch, provisioning, version resolution, kill switch, reset.
- Operator screen with a live tool-activity trace, status panel and kill
switch; dashboard discovery card; nav entry; operator.* i18n in 11 locales.
Auth correction. The design assumed EDDI forwards the chatting user's token
to an API agent's tool calls. It does not: ApiCallExecutor builds headers
only from the ApiCall config, and the template model exposes only
userInfo.userId. Auth is therefore explicit on the config:
- none - no Authorization header. Blocked at activation when OIDC is on,
because the operator would deploy READY and then 401 on every lookup.
- caller-context - Bearer {context.eddiAuthToken}, resolved per turn from the
conversation context so calls run with the caller's permissions and audit
identity. Gated behind an acknowledged token-at-rest warning, since context
is persisted with the conversation.
Contract bug fixed. createApiAgent sent `name`; the backend requires
`agentName` and rejects a blank one, so the wizard's API-agent path was
broken. Fixed the type, the wizard, and the MSW mock, which now rejects a
blank agentName as the backend does. That exposed three tests passing for the
wrong reason, including one shadowed by the broad */agents/:conversationId
handler - the setup-api mock now sits above it.
Also excludes .claude/** from vitest so other branches' worktrees do not run
their tests against this branch's mocks.
Review pass over the Platform Operator. Each item is a defect the first pass shipped, not a refinement. Correctness - Activation now runs the post-deploy canary the design called for: one probe read through the deployed operator, counting tool calls and detecting 401s. A READY badge only proves the agent config loaded; it says nothing about whether the generated tools can authenticate - which is exactly the failure mode a wrong authMode produces. A failed probe is surfaced as a warning on the operator screen with a re-check control, not swallowed. - assertProvisioned rejects two results that were previously accepted as success: setup-api answers 201 even when the deploy step failed, and falls back to the literal agent id "unknown", which would have been persisted as enabled and then addressed by every status and undeploy call. - Re-enabling a paused operator redeploys the existing agent instead of building a new one and deleting the old, which orphaned resources and forced the admin to re-enter a model key the vault already held. - The OpenAPI spec was fetched twice per activation (400+ KB each). The caller now fetches once and passes it, so the document that is validated is the document that is sent. Accessibility - Not one control in the activation form had an accessible name: bare <label> elements with no htmlFor next to id-less controls. Native controls are now bound by id; the credential picker and auth-mode radios are named groups. - The chat transcript is a live region, the input and icon-only buttons have labels, and progress and errors announce. i18n - The tool-count label passed `count`, which i18next treats as the plural selector, so it resolved through plural keys that do not exist (six of them in Arabic). Renamed to `toolCount`. - New canary, paused and connection-check strings across all 11 locales. Also de-duplicates the endpoint-parsing regex and the caller-context builder.
The config records the LLM credential's vault key *name*, but the activation form never read it back, so every reconfigure - even one that only switches model - demanded the key again. Now the field is pre-filled with the stored vault reference, and cleared on a provider change, since a key belongs to the provider it was issued for.
…sation context
EDDI 6.2.0 resolves ${caller:token} in apicall headers server-side, which is
what the operator wanted all along. The workaround this replaces piped the
signed-in user's bearer through the per-turn conversation context, so the
token was written into conversation memory in MongoDB and was visible
wherever conversation detail is rendered.
- authMode "caller-context" becomes "caller-identity"; apiAuth is now the
literal "Bearer ${caller:token}", substituted by EDDI while building the
request and never persisted.
- The chat hook and the canary no longer send any context; buildCallerContext
and CALLER_TOKEN_CONTEXT_KEY are gone.
- The token-at-rest warning and its acknowledgement checkbox are removed:
there is no longer a trade-off to accept. In their place, a short note
saying the token is substituted at call time, never stored, and only ever
sent back to this deployment.
- i18n updated across all 11 locales.
Requires EDDI 6.2.0+. Against an older backend the placeholder would be sent
verbatim, so "No credentials" remains the default and stays blocked when OIDC
is on.
Five issues found reviewing the workflow, each a thing an admin would actually trip over. - The operator conversation is remembered per tab (sessionStorage), so navigating away mid-investigation and back no longer silently starts a new one. sessionStorage rather than localStorage: an investigation belongs to the tab you are in. - Each turn keeps its own tool-activity trace, rendered under the answer it belongs to. Previously the newest turn replaced the trace of every earlier one, which is a poor trade for a tool whose value is showing its work. - Reconfiguring now warns that saving builds a new agent and drops the current one, because setup-api only creates. The existing operator conversation does not carry over, and that was not signposted anywhere. - The operator's agent carries an "Operator" badge in the Agents list, saying it is provisioned and managed by the operator screen. Deleting it there left the operator config pointing at nothing, with no hint that it was special. - "Environment" now explains it means where the operator agent runs, not which environment it can see - it can read any of them, since environment is a tool parameter. i18n across all 11 locales.
Records the constraints that are not obvious from the code: the operator is a real agent visible in the Agents list, its capability boundary is an allow-list (never a deny-list), the config is one atomic blob because the variable store has no transaction, and activation runs a canary because a READY badge proves nothing about whether the tools can authenticate.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughAdds a Platform Operator feature with read-only provisioning, persisted configuration, canary validation, lifecycle controls, operator chat, navigation, localized UI, managed-agent labeling, and comprehensive tests. ChangesPlatform Operator
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant OperatorPage
participant OperatorHooks
participant OperatorAPI
participant AgentService
User->>OperatorPage: Open operator management
OperatorPage->>OperatorHooks: Read config and deployment status
OperatorHooks->>OperatorAPI: Fetch persisted config and status
User->>OperatorPage: Submit activation
OperatorPage->>OperatorHooks: Start activation mutation
OperatorHooks->>OperatorAPI: Validate spec and provision agent
OperatorAPI->>AgentService: Create and deploy read-only agent
AgentService-->>OperatorAPI: Return agent and deployment result
OperatorAPI-->>OperatorHooks: Return config and canary result
OperatorHooks-->>OperatorPage: Update active operator view
User->>OperatorPage: Send chat message
OperatorPage->>OperatorHooks: Stream operator conversation
OperatorHooks->>AgentService: Start or resume conversation
AgentService-->>OperatorHooks: Return tokens and task events
OperatorHooks-->>OperatorPage: Render response and traces
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
src/components/operator/operator-activation.tsx (1)
112-376: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd i18n fallback strings to
t()calls.None of the
t()calls in this file pass a fallback (e.g.t("operator.activation.title")instead oft("operator.activation.title", "...")). This applies to essentially every call in the component (title/subtitle/labels/hints/stage text/auth-mode copy).
As per coding guidelines,src/**/*.{ts,tsx}should "Use inline i18n fallbacks witht("key", "Fallback")".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/operator/operator-activation.tsx` around lines 112 - 376, Add inline fallback text to every translation call in the operator activation component, including labels, hints, buttons, notices, stage messages, and auth-mode strings. Update both the main component and AuthModeField, including dynamic keys such as operator.stage and operator.authMode, while preserving the existing translation keys and interpolation parameters.Source: Coding guidelines
src/lib/api/__tests__/operator.test.ts (1)
185-189: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer stubbing only
locationrather than replacingwindow.
{ ...globalThis.window }copies only own enumerable properties, so the stubbed global loses prototype members (addEventListener,HTMLElement, etc.). Anything in the code path under test that touches those would fail for a reason unrelated to the assertion. Stubbingwindow.locationalone keeps the rest of the environment intact.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/api/__tests__/operator.test.ts` around lines 185 - 189, Update the beforeEach setup around the window stub to stub only window.location with the required origin, preserving the existing window object and its prototype members; keep the openapi server setup unchanged.src/hooks/use-operator-chat.ts (1)
188-194: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTwo rapid sends can each create a conversation.
conversationIdis read from render-time state, so a secondsenddispatched before the firstsetStatecommits also seesnulland callsstartConversationagain — the second id overwrites the first insessionStorage, silently splitting the investigation. A ref written synchronously alongside the state update avoids this and also removesstate.conversationIdfrom theuseCallbackdeps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/use-operator-chat.ts` around lines 188 - 194, Update the conversation initialization in the send callback around startConversation to use a ref as the synchronous source of truth: read the existing conversation ID from the ref, and when creating one, write it to the ref immediately alongside storeConversationId and setState. Remove state.conversationId from the useCallback dependency list while preserving the existing conversation creation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@HANDOFF.md`:
- Around line 127-133: The “Last Commit Focus” Platform Operator summary in
HANDOFF.md still describes the obsolete caller-context token forwarding
mechanism. Update the authMode design note to document the shipped
caller-identity mode and EDDI server-side ${caller:token} resolver, matching the
OperatorAuthMode values in operator-activation.tsx and the AGENTS.md Platform
Operator section; remove the conversation-context eddiAuthToken and
token-at-rest wording.
In `@src/components/operator/operator-chat.tsx`:
- Around line 56-70: Replace direct Operator UI translation calls with inline
fallback strings: in src/components/operator/operator-chat.tsx lines 56-70,
change OPERATOR_STARTER_PROMPTS to key/fallback pairs and pass both values to t;
in src/components/operator/operator-status.tsx lines 54-76, add fallbacks for
static and dynamic operator labels, including
operator.authMode.${config.authMode}.label; in src/pages/operator.tsx lines
78-82, add fallbacks for activation, canary, lifecycle, configuration-error,
active-status, and toast messages; in src/components/layout/sidebar.tsx line 53,
provide the nav.operator fallback through the sidebar item fallback path.
In `@src/hooks/use-operator-chat.ts`:
- Around line 225-243: Guard the cleanup in the streaming operation’s finally
block so a stale turn cannot affect a newer one: only clear abortRef.current and
set isStreaming to false when the settling turn’s controller is still the
current controller. Preserve the existing event, trace, and message cleanup,
while ensuring a newer send remains active and abortable.
In `@src/hooks/use-operator.ts`:
- Around line 100-157: Update useActivateOperator so failures after
provisionOperator/assertProvisioned trigger best-effort retirement of the newly
provisioned result.agentId, then rethrow the original error; keep
superseded-agent cleanup targeting config.agentId separately. After
writeOperatorConfig(next) succeeds, immediately invalidate operatorKeys.all
through qc rather than relying only on the mutation-level onSuccess callback.
In `@src/lib/api/operator.ts`:
- Around line 100-114: Update readOperatorConfig to validate that the JSON.parse
result is a non-null object before returning it as OperatorConfig; return null
for valid JSON primitives, arrays, or null, while preserving the existing
handling for parse errors and missing variables.
- Around line 318-382: Update runOperatorCanary’s task_complete handling to
deduplicate overlapping toolTrace entries from multiple completion events before
incrementing toolCalls or evaluating tool results. Track each completed taskId,
or use an equivalent stable identifier, so each canary tool call is processed
only once while preserving stream and authentication error handling.
In `@src/pages/dashboard.tsx`:
- Around line 451-458: Add inline English fallbacks to every listed operator
translation lookup: update operator.discovery.title,
operator.discovery.description, and operator.discovery.action in
src/pages/dashboard.tsx (lines 451-458), plus operator.managedAgentHint and
operator.managedAgentBadge in src/components/agents/agent-card.tsx (lines
123-132), using the two-argument t(key, fallback) form.
In `@src/pages/operator.tsx`:
- Around line 76-82: Update the activation flow around the canary result
handling in the operator page so a failed canary is not lost when navigating or
reloading. Persist the failed result with the operator configuration, or re-run
the canary while loading an active operator, and ensure the resulting warning
prevents the deployment status from presenting the operator as READY when
platform access remains unavailable.
In `@src/test/mocks/handlers.ts`:
- Around line 1036-1050: Update the mock response in the API agent creation
handler to derive resources.agentLocation from the same generated agentId
returned in the payload, preserving the expected agent-store URL format and
ensuring both values identify the same agent.
---
Nitpick comments:
In `@src/components/operator/operator-activation.tsx`:
- Around line 112-376: Add inline fallback text to every translation call in the
operator activation component, including labels, hints, buttons, notices, stage
messages, and auth-mode strings. Update both the main component and
AuthModeField, including dynamic keys such as operator.stage and
operator.authMode, while preserving the existing translation keys and
interpolation parameters.
In `@src/hooks/use-operator-chat.ts`:
- Around line 188-194: Update the conversation initialization in the send
callback around startConversation to use a ref as the synchronous source of
truth: read the existing conversation ID from the ref, and when creating one,
write it to the ref immediately alongside storeConversationId and setState.
Remove state.conversationId from the useCallback dependency list while
preserving the existing conversation creation behavior.
In `@src/lib/api/__tests__/operator.test.ts`:
- Around line 185-189: Update the beforeEach setup around the window stub to
stub only window.location with the required origin, preserving the existing
window object and its prototype members; keep the openapi server setup
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 215b8c0f-44cf-4eb0-b946-a70db26044d1
📒 Files selected for processing (39)
AGENTS.mdHANDOFF.mdsrc/app.tsxsrc/components/agents/agent-card.tsxsrc/components/layout/sidebar.tsxsrc/components/operator/__tests__/operator-activation.test.tsxsrc/components/operator/operator-activation.tsxsrc/components/operator/operator-chat.tsxsrc/components/operator/operator-status.tsxsrc/hooks/__tests__/use-agent-setup.test.tsxsrc/hooks/__tests__/use-misc-hooks.test.tsxsrc/hooks/use-operator-chat.tssrc/hooks/use-operator.tssrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/lib/api/__tests__/agent-setup.test.tssrc/lib/api/__tests__/operator.test.tssrc/lib/api/agent-setup.tssrc/lib/api/operator.tssrc/lib/model-suggestions.tssrc/lib/operator/__tests__/tool-scopes.test.tssrc/lib/operator/system-prompt.tssrc/lib/operator/tool-scopes.tssrc/lib/operator/vault-ref.tssrc/pages/__tests__/operator.test.tsxsrc/pages/agent-wizard.tsxsrc/pages/dashboard.tsxsrc/pages/operator.tsxsrc/test/mocks/handlers.tsvitest.config.ts
- A stopped-then-resent turn could settle after its successor started and then clobber the newer turn: nulling the live AbortController (so the stop button vanished and abort stopped working), wiping the new turn's events, and filing them under the old message id. A turn that is no longer current now touches only its own message. - readOperatorConfig treated any parseable JSON as a config. JSON.parse also succeeds for null, a number, a bare string and an array, so an overwritten variable was cast straight to OperatorConfig and surfaced later as undefined property reads instead of "not configured". - Added the inline i18n fallbacks these two files use by convention, for the operator strings on the dashboard card and the agent-card badge. - HANDOFF.md still described the replaced conversation-context mechanism. Two consolidated sibling comments (use-operator.ts:157, operator.ts:382) had empty bodies, so there was nothing specific to act on.
Three of these make the operator unusable; the rest are traps an admin would hit. - The LLM provider's base URL was sent as apiBaseUrl, which is the target server of the generated tools. Choosing Ollama or Jlama — the two providers that make the field mandatory — pointed every operator tool at http://localhost:11434 instead of at EDDI. apiBaseUrl is now always this deployment's origin, and the LLM base URL travels in the new baseUrl field (labsai/EDDI#613). The test that asserted the old behaviour asserted the bug; it now asserts the contract. - Reconfigure seeded a bare "vault:KEY" via JS interpolation. The picker renders that as a valid chip, but the backend's isVaultReference requires ${vault:...} and falls through to the plaintext branch, storing the literal string as the provider credential — so the new operator 401s on every model call, after the working agent has already been hard-deleted. Both seeding sites now go through a single toVaultRef formatter. - "Delete operator" on the paused screen deleted the agent, cascade and permanent, with no confirmation — while the identical action on the active screen is gated by a dialog. Both now share one. - After reconfiguring, the chat kept posting into the just-deleted agent's conversation (the conversation id alone selects the agent). Activation success now resets the chat. - The canary had no timeout and leaked a conversation per run: a stalled stream left activation spinning with no way out but a reload. It now aborts after 60s and ends its probe conversation in a finally. - looksLikeAuthFailure regexed the whole tool result, so an agent whose description contains "forbidden" failed the check for a working operator. - The dashboard card advertised a paused (or unreadable) operator as never set up, and both confirm dialogs rendered an English "Cancel" in every locale. - RTL: three mr-2 replaced with me-2, per the repo's logical-property rule. - Field hints were stamped with an id nothing referenced, so screen readers never associated them with their control. 4128 tests pass.
- Renamed the LLM endpoint field to llmBaseUrl, matching the backend rename (labsai/EDDI#613). Next to apiBaseUrl — the generated tools' target server — a field called "baseUrl" gave a caller no way to tell the two apart. - Inline i18n fallbacks on all 92 single-argument t() calls across the operator components, per the repo's coding guideline. The keys are present in all 11 locales and verified by a parity check, so this only matters while a key is being propagated — but it is the convention here. - The operator API test replaced the whole window object to stub location, which drops every prototype member (addEventListener, HTMLElement, ...), so anything in the code path touching them would fail for an unrelated reason. It now stubs location alone. 4128 tests pass.
What
An opt-in, admin-activated Platform Operator: an agent that inspects this EDDI deployment and explains what it finds. Off by default, read-only, one click to activate, with a kill switch.
It is provisioned through
setup-apias an API Agent whose tools are generated from EDDI's own OpenAPI spec and scoped by a read-only allow-list.Depends on labsai/EDDI#613 for the
caller-identityauth mode.Design notes worth reviewing
src/lib/operator/tool-scopes.ts), never a deny-list — a deny-list silently grants any endpoint added to the backend later. Writes are literally unreachable until an approval handler exists (isWriteScopeAvailable), enforced by tests rather than convention.platform.operatorglobal variable. Activation writes several values that must land together and the variable store has no transaction.READYdeployment badge only proves the agent config loaded; it says nothing about whether the generated tools can authenticate — which is exactly the failure a wrong auth mode produces. The probe sends one read and counts tool calls, so "answered without calling any tool" and "tools returned 401" are both reported as failures.A design assumption that turned out to be wrong
The design assumed EDDI forwards the chatting user's token to an API agent's tool calls. It does not —
ApiCallExecutorbuilds headers only from the ApiCall config, and the template model exposes onlyuserInfo.userId. The Phase 0 spike read as green because that box ranauth=none; "noapiAuth→ empty headers" is evidence of no auth at all, not of pass-through.That is what labsai/EDDI#613 fixes. This PR uses it:
authMode: "caller-identity"provisionsBearer ${caller:token}, resolved server-side and never persisted."none"remains the default and is blocked at activation when OIDC is enabled, since every tool call would 401.Contract bug fixed along the way
createApiAgentsentname; the backend requiresagentNameand rejects a blank one — so the agent wizard's API-agent path was broken onmain. Fixed in the type, the wizard and the MSW mock (which now rejects a blankagentNameas the backend does). That exposed three tests passing for the wrong reason, including one shadowed by the broad*/agents/:conversationIdhandler — MSW's*spans path segments, sosetup-apiwas being answered with a conversation snapshot.UX
Dashboard discovery card, guided two-step activation, operator screen with a live tool-activity trace (each turn keeps its own), status panel, pause/resume that redeploys in place rather than rebuilding, and a full delete. Conversations survive a reload per tab. The operator's agent carries an "Operator" badge in the Agents list so deleting it there is not a silent footgun.
Accessibility: every control in the activation form had no accessible name in the first pass (bare
<label>with nohtmlForbeside id-less controls) — fixed, with a regression test. The chat transcript is a live region.operator.*i18n across all 11 locales (87 keys, parity verified).Testing
4123 tests / 281 files passing; typecheck and lint clean. Mutation-checked the OIDC gate, the canary's tool-call detection and the deploy-result guard — each fails its tests when disabled.
Also excludes
.claude/**from vitest so other branches' worktrees don't run their tests against this branch's mocks.Not verified
Nothing here has run live. Every test is against mocks; contracts were verified by reading backend source. The first real activation with a real model key — and
caller-identityagainst real Keycloak — is what mocks cannot tell you.Summary by CodeRabbit
/manage/operatorwith activation/setup, reconfiguration, and read-only operator chat.agentName(instead of the old name field).