diff --git a/AGENTS.md b/AGENTS.md index df30005d..9d72dc04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -208,8 +208,17 @@ what it finds. Off by default. Worth knowing before touching it: or deleting it there breaks the operator screen. - **Its capability boundary is the allow-list** in `src/lib/operator/tool-scopes.ts` — an allow-list, never a deny-list, because a deny-list silently grants any - endpoint the backend adds later. Writes are unreachable until an approval - handler exists (`isWriteScopeAvailable`). + endpoint the backend adds later. `WRITE_ENDPOINTS` holds 22 entries — read the + constant rather than this line, and read its doc comment before adding to it: + what is excluded (every `DELETE`, the full agent and group document PUTs, and + every `llmstore` write — each because that document carries an approval gate + of its own) is as deliberate as what is included. + Offering any of them is additionally gated by `isWriteScopeAvailable` + (backend HITL support, an already-verified gate, caller-identity auth, and a + mounted approval surface all have to hold — see `operator-activation.tsx`). + Granting `read_write` runs a write canary (`write-canary.ts`) that provokes + a real gated write and rolls the whole activation back on anything but a + clean pause. - **Config is one atomic JSON blob** in the `platform.operator` global variable. Activation writes several values that must land together and the variable store has no transaction. diff --git a/HANDOFF.md b/HANDOFF.md index 08bf5a44..6b1eadc9 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -98,6 +98,21 @@ - **Tests**: 119 unit tests across debug panel + workforce components, audit test fix (camelCase assertion). - **Commits**: 6 commits on `test/debug-workforce-coverage` from `e05391bf` to `feeb5aef`. +- **Platform Operator — Approval Gate + Write Capability** (branch: `feat/operator-write-scope`, stacked on the P1 read-only operator): Takes the operator from read-only to an admin-grantable `read_write` scope, gated end-to-end by a real human-approval workflow rather than a config toggle. Includes: + - **HITL approval gate (read-only)**: `useVerifyOperatorGate` builds and deploys a throwaway agent config against the backend to confirm the HITL tool-approval gate is actually enforced on this deployment before anything write-capable is ever offered — a version bump alone proves nothing. `gate.verified` is the seam every later write-scope check reads. + - **Approval surface**: `ApprovalBanner`'s `renderCallExtra` render-prop lets the operator screen show gated-call context inline in the operator chat itself (`operator-chat.tsx`) rather than sending the admin to a separate approvals page — resolving a pause is `requireExplicitPerCall` so a swept-in call can't inherit a top-level Approve unreviewed. + - **System prompt from the granted endpoint set**: the operator's system prompt is derived from `tool-scopes.ts`'s allow-list rather than hand-maintained prose, so the model is never told about more than it can actually call. + - **Fixes along the way**: reconciling a resumed turn by placeholder identity (not output-count comparison) and handling a re-pause mid-resume; per-tool `toolApprovals.rules` verification, not just the top-level gate; the pause placeholder id moved from a ref into state (a ref update the resume handler read before React committed it raced the very state it was meant to protect). + - **`WRITE_ENDPOINTS` populated**: 22 entries, previously an intentionally empty allow-list stub. Four operational verbs (descriptor PATCH, deploy, undeploy, schedule disable), agent and group creation (`setup`, `setup-api`, `POST /groupstore/groups`), the workflow-extension authoring stores (rules/outputs/property-setters/dictionaries/apicalls/mcpcalls/workflows), and `PUT /agentstore/agents/{id}/updateResourceUri` — the hop that makes an edit actually take effect, since EDDI writes version+1 rather than mutating in place. **No `DELETE`, no full `PUT /agentstore/agents/{id}`, and no `llmstore` write** — that document carries a per-task `toolApprovals` which FULLY REPLACES the agent-level gate, so writing it could strip the operator's own oversight (which also means the operator cannot edit a prompt or model). `tool-scopes.ts` is the source of truth; its doc comment justifies every inclusion and exclusion. + - **Write canary**: `write-canary.ts` provokes one real gated write and asserts it actually pauses (rejecting the pause so nothing executes) before `read_write` is ever granted; anything but a clean pause rolls the whole activation back (`resetOperator` — undeploy + delete + clear config), since the agent is already deployed with live write tools by that point. + - **Real scope selection**: activation form's scope radio was previously pinned to `read_only`. `isWriteScopeAvailable` (`operator-activation.tsx`) requires backend HITL support, an already-verified gate, `authMode: "caller-identity"`, and a mounted approval surface to ALL hold before `read_write` is offered — which means it can only appear on a *reconfigure* of an already-verified operator, never a first activation (nothing to have verified yet). A `scope`/`effectiveScope` split keeps the UI from showing a choice as active that a lost precondition would silently not submit. + - **Server-verified request preview**: the backend now resolves and fingerprints the exact HTTP request a gated call will make (EDDI-side: `IApiCallExecutor#resolve`, pinned at gate time, re-checked immediately before execution) and returns a redacted preview through `approval-status`. The operator screen's `RequestPreview` component renders that preview — method/uri/query/headers/body, with a `verified`/`preview` badge depending on whether the backend could pin it — demoting the old client-side `reconstructEndpoint` (`operationId` → guessed path) to a fallback for calls the backend could not resolve ahead of execution. + - **i18n**: `operator.readWriteChip`, `operator.activation.scope.*`, `operator.stage["write-canary"]`, `operator.toast.activatedReadWrite`, `operator.approval.{verified,verifiedTitle,previewOnly,previewOnlyTitle,query,headers,bodyTruncated}` — all 11 locales. + - **Backend companion PR** (`labsai/EDDI`, branch `feat/operator-request-fingerprint`): request-fingerprint pinning (`ResolvedRequest`, `RequestRedactor`), `eddi.operator.write.approval{decision}` metric plus the two `/administration/operator/{canary-result,gate-status}` relay endpoints this branch's canary and gate-verification calls report to. + - **Commits**: 11 commits on `feat/operator-write-scope` from `fa793a04` to `6f47e835`; substantially more has landed on this branch since (request pinning/redaction hardening, self-guard, escalation flags, the approvals inbox, full agent/group create+modify, and the docked drawer below) — see `git log feat/operator-write-scope` rather than trusting this range. + - **Context-aware side-chat drawer**: the operator was Manager-only and full-page-only — unreachable from Workforce, and always a full navigation away. `operator-drawer.tsx` is a floating launcher mounted once in `AppLayout` and once in each of `WorkforceLayout`'s three viewport branches (self-positioned `fixed`, since those four layouts share no common chrome slot), reusing `useOperatorChat`/`useOperatorConfig` directly — same react-query cache, same conversation, not a second copy. Getting there required promoting `use-operator-chat.ts` off local `useState` onto a Zustand store (`useOperatorChatStore`) so the drawer and the full page render the identical live conversation; the full page's own call sites are unchanged. A pause opens to a compact notice + link to `/manage/operator` rather than a shrunk `ApprovalBanner` — that component is security-reviewed for one full-width surface, not a duplicate. `useCurrentScreenContext()` (route → `{screen, agentId, workflowId, groupId, boardId}` via `matchPath`, since the drawer sits above the routed ``) threads into `send()` as a `context` argument, which a new unconditional, Qute-conditional section of the system prompt (`BODY_APP_CONTEXT`) reads as `{context.screen}` etc. — existing, previously-unused transport (`InputData.context`), zero backend changes. i18n: `operator.chat.pauseCompact{Fallback,Review}`, `operator.drawer.{title,notActivated,activate}` — all 11 locales. + - **Still open**: iteration 7 (agent/group authoring UI) and a final whole-branch review have both since landed — nothing tracked as open from this line as of the drawer above; check `git log` for what came after. + ### Boardroom v2 Design Decisions **Decisions confirmed by user:** @@ -136,7 +151,8 @@ - 112 Backend tenancy tests passing (`mvn test`) ### Last Commit Focus -- Frontend: `feat(operator): add the Platform Operator agent (P1, read-only)` on `feat/platform-operator-agent` +- Frontend: `feat(operator): render the backend's resolved-request preview in the approval banner` (`6f47e835`) on `feat/operator-write-scope` — see the "Platform Operator — Approval Gate + Write Capability" phase above for the full arc. Not yet pushed; ask before pushing. +- Earlier frontend: `feat(operator): add the Platform Operator agent (P1, read-only)` on `feat/platform-operator-agent` - Opt-in, admin-activated agent that inspects this EDDI deployment through its own REST API, exposed as tools via `setup-api`. Read-only allow-list (`src/lib/operator/tool-scopes.ts`), non-editable safety preamble, single-blob `platform.operator` config, activation flow with a post-provision spec check, operator screen with a live tool-activity trace, dashboard discovery card, kill switch, `operator.*` i18n across 11 locales. - **Design correction:** the design assumed EDDI forwards the caller's token to an API agent's tool calls. It did not. That gap is now closed in the backend (labsai/EDDI#613) by a `${caller:token}` resolver, and the operator's `authMode: "caller-identity"` uses it: EDDI substitutes the token while building the request, releasing it only for a same-origin call, only into a header, and never persisting it. `"none"` remains the default and is blocked at activation when OIDC is on, since every tool call would 401. - **Review pass:** added the post-deploy canary the design asked for (one probe read; it counts tool calls and detects 401s, since a READY badge proves nothing about whether the generated tools can authenticate), made activation honour setup-api's `deployed`/`deploymentStatus` and reject the `"unknown"` agent-id fallback, added a redeploy-in-place path so re-enabling a paused operator no longer rebuilds it, fixed the activation form's accessibility (not one control had an accessible name), and stopped `{{count}}` triggering i18next pluralization. diff --git a/src/components/hitl/__tests__/approval-banner.test.tsx b/src/components/hitl/__tests__/approval-banner.test.tsx index a9104b6a..69849222 100644 --- a/src/components/hitl/__tests__/approval-banner.test.tsx +++ b/src/components/hitl/__tests__/approval-banner.test.tsx @@ -9,8 +9,8 @@ function toolPause(overrides: Partial = {}): ToolCallPause return { type: "TOOL_CALL", calls: [ - { callId: "c1", toolName: "sendEmail", source: "mcp", arguments: '{"to":"[REDACTED]"}', argsTruncated: false, gateReason: "mcp:*" }, - { callId: "c2", toolName: "transfer_funds", source: "builtin", arguments: '{"amount":100}', argsTruncated: false, gateReason: "transfer_*" }, + { callId: "c1", toolName: "sendEmail", source: "mcp", arguments: '{"to":"[REDACTED]"}', argsTruncated: false, gateReason: "mcp:*", requestPinned: false }, + { callId: "c2", toolName: "transfer_funds", source: "builtin", arguments: '{"amount":100}', argsTruncated: false, gateReason: "transfer_*", requestPinned: false }, ], executedUngatedCalls: [], outcomeUnknown: [], @@ -301,7 +301,7 @@ describe("ApprovalBanner", () => { it("does not offer amendment for a call whose arguments were truncated", () => { const details = toolPause({ calls: [ - { callId: "c1", toolName: "bulkUpdate", source: "http", arguments: "{…}", argsTruncated: true, gateReason: "http:*" }, + { callId: "c1", toolName: "bulkUpdate", source: "http", arguments: "{…}", argsTruncated: true, gateReason: "http:*", requestPinned: false }, ], }); renderWithProviders( @@ -314,7 +314,7 @@ describe("ApprovalBanner", () => { it("surfaces executedUngatedCalls and the outcome-unknown warning", () => { const details = toolPause({ calls: [ - { callId: "c1", toolName: "sendEmail", source: "mcp", arguments: "{}", argsTruncated: false, gateReason: "mcp:*" }, + { callId: "c1", toolName: "sendEmail", source: "mcp", arguments: "{}", argsTruncated: false, gateReason: "mcp:*", requestPinned: false }, ], executedUngatedCalls: ["getCurrentDateTime"], outcomeUnknown: ["c1"], @@ -337,5 +337,215 @@ describe("ApprovalBanner", () => { expect(screen.queryByTestId("tool-call-approvals")).not.toBeInTheDocument(); expect(screen.getByTestId("approval-banner")).toHaveAttribute("data-pause-type", "RULE"); }); + + it("always shows the redaction caveat, regardless of requireExplicitPerCall", () => { + renderWithProviders( + , + ); + // The marker must be the one the backend actually emits + // (`RequestRedactor.REDACTED` / `SecretRedactionFilter` = ""). + // This asserted "[REDACTED]", pinning the wrong string in place: the + // caveat tells an approver which text to look for, and that text never + // appears in any payload. + expect(screen.getByTestId("redaction-caveat")).toHaveTextContent(""); + }); + + it("renders renderCallExtra content for each call", () => { + renderWithProviders( + POST /agentstore/agents} + />, + ); + expect(screen.getByTestId("extra-c1")).toBeInTheDocument(); + expect(screen.getByTestId("extra-c2")).toBeInTheDocument(); + }); + }); + + describe("blockedCalls — a refusal, not a warning", () => { + const blocked = [{ callId: "c1", reason: "This modifies the operator's own agent." }]; + + it("disables Approve outright while a call is blocked", () => { + // The distinction from every other signal on this banner: an escalation + // flag informs, this one takes the decision away. It exists for the single + // write that would remove the approval gate itself. + renderWithProviders( + , + ); + expect(screen.getByTestId("approve-button")).toBeDisabled(); + }); + + it("says why, as an alert rather than styled text", () => { + // An approver who loses Approve without explanation assumes the UI is + // broken and goes looking for another way to do the same thing. + renderWithProviders( + , + ); + const alert = screen.getByTestId("approval-blocked"); + expect(alert).toHaveAttribute("role", "alert"); + expect(alert).toHaveTextContent("This modifies the operator's own agent."); + }); + + it("still allows Reject, so a blocked pause is never a dead end", () => { + renderWithProviders( + , + ); + expect(screen.getByTestId("reject-button")).not.toBeDisabled(); + }); + + it("blocks the whole batch, not just the offending call", () => { + // Per-call verdicts are submitted together; approving "the rest" would + // still run the batch containing the write being refused. + const onDecide = vi.fn(); + renderWithProviders( + , + ); + fireEvent.click(screen.getByTestId("approve-button")); + expect(onDecide).not.toHaveBeenCalled(); + }); + + it("leaves Approve available when nothing is blocked", () => { + // The mirror direction, so the pairing is not vacuous: a regression that + // blocked everything would pass every test above. + renderWithProviders( + , + ); + expect(screen.getByTestId("approve-button")).not.toBeDisabled(); + expect(screen.queryByTestId("approval-blocked")).not.toBeInTheDocument(); + }); + }); + + describe("requireExplicitPerCall", () => { + it("disables Approve until every call has an explicit verdict", () => { + renderWithProviders( + , + ); + expect(screen.getByTestId("approve-button")).toBeDisabled(); + expect(screen.getByTestId("explicit-review-missing")).toBeInTheDocument(); + }); + + it("enables Approve once EVERY call has been explicitly toggled", () => { + const onDecide = vi.fn(); + renderWithProviders( + , + ); + fireEvent.click(screen.getByTestId("tool-approve-c1")); + // Still missing c2 — must stay disabled. + expect(screen.getByTestId("approve-button")).toBeDisabled(); + + fireEvent.click(screen.getByTestId("tool-approve-c2")); + expect(screen.getByTestId("approve-button")).not.toBeDisabled(); + expect(screen.queryByTestId("explicit-review-missing")).not.toBeInTheDocument(); + + fireEvent.click(screen.getByTestId("approve-button")); + confirmInDialog("Approve"); + expect(onDecide).toHaveBeenCalledWith("APPROVED", undefined, undefined, { + c1: { verdict: "APPROVED" }, + c2: { verdict: "APPROVED" }, + }); + }); + + it("un-reviewing a call (toggling it back off) re-disables Approve", () => { + renderWithProviders( + , + ); + fireEvent.click(screen.getByTestId("tool-approve-c1")); + fireEvent.click(screen.getByTestId("tool-approve-c2")); + expect(screen.getByTestId("approve-button")).not.toBeDisabled(); + + // Toggling an already-selected verdict off is the existing "un-toggle" + // behavior (see the onToggle handler) — Approve must track it live. + fireEvent.click(screen.getByTestId("tool-approve-c2")); + expect(screen.getByTestId("approve-button")).toBeDisabled(); + }); + + it("Reject stays available regardless — rejecting the batch needs no per-call review", () => { + const onDecide = vi.fn(); + renderWithProviders( + , + ); + expect(screen.getByTestId("reject-button")).not.toBeDisabled(); + fireEvent.click(screen.getByTestId("reject-button")); + confirmInDialog("Reject"); + expect(onDecide).toHaveBeenCalledWith("REJECTED", undefined, undefined, undefined); + }); + + it("does not affect a RULE pause, which has no per-call state to require", () => { + const onDecide = vi.fn(); + renderWithProviders( + , + ); + expect(screen.getByTestId("approve-button")).not.toBeDisabled(); + }); + + it("defaults to off — existing callers (conversation-detail, discussion-transcript) keep sweep-approve", () => { + renderWithProviders( + , + ); + expect(screen.getByTestId("approve-button")).not.toBeDisabled(); + expect(screen.queryByTestId("explicit-review-missing")).not.toBeInTheDocument(); + }); + }); +}); + +describe("a failed pause-details read must not be approvable", () => { + // The regression this pins: when the read FAILS, pauseDetails is null — so + // blockedCalls is empty, explicitReviewMissing is false, and every other + // guard in the disabled list evaluates to "nothing to object to" precisely + // BECAUSE we know nothing. Approve was left enabled, under a message saying + // it couldn't be approved, and resumed the whole batch with no per-call + // decisions — executing calls nobody had seen. + it("disables Approve when the details failed to load", () => { + const onDecide = vi.fn(); + renderWithProviders( + , + ); + + expect(screen.getByTestId("approval-details-error")).toBeInTheDocument(); + expect(screen.getByTestId("approve-button")).toBeDisabled(); + }); + + it("still allows Reject, which is safe without knowing the detail", () => { + renderWithProviders( + , + ); + expect(screen.getByTestId("reject-button")).not.toBeDisabled(); + }); + + it("offers a retry that calls back", () => { + const onRetry = vi.fn(); + renderWithProviders( + , + ); + fireEvent.click(screen.getByTestId("approval-details-retry")); + expect(onRetry).toHaveBeenCalledTimes(1); }); }); diff --git a/src/components/hitl/approval-banner.tsx b/src/components/hitl/approval-banner.tsx index 74eb2526..8568f581 100644 --- a/src/components/hitl/approval-banner.tsx +++ b/src/components/hitl/approval-banner.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useEffect, useState, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; import { CheckCircle2, @@ -45,11 +45,25 @@ interface ApprovalBannerProps { pauseDetails?: PauseDetails | null; /** Whether the mutation is in-flight. */ isSubmitting?: boolean; - /** True while the structured pause details are still loading (or failed to - * load) for a paused conversation. Blocks Approve so the reviewer can't - * blind approve-all before knowing whether this is a RULE or TOOL_CALL pause; - * Reject/Cancel stay available (both are safe with details unknown). */ + /** True while the structured pause details are still loading for a paused + * conversation. Blocks Approve so the reviewer can't blind approve-all + * before knowing whether this is a RULE or TOOL_CALL pause; Reject/Cancel + * stay available (both are safe with details unknown). */ pauseDetailsPending?: boolean; + /** + * True when loading those details FAILED, as distinct from still loading. + * + * Both block Approve, and must — but they are not the same thing to a human. + * Folding a failure into `pauseDetailsPending` (as every caller here used to) + * left a pulsing "Loading approval details…" on screen forever with Approve + * permanently disabled, no error, and no way to retry: a dead end on the one + * surface the whole feature exists for. Callers pass their query's own error + * flag, and `onRetryPauseDetails` to offer a way out. + */ + pauseDetailsError?: boolean; + /** Retries the pause-details read. Renders a Retry action when provided + * alongside `pauseDetailsError`. */ + onRetryPauseDetails?: () => void; /** Called when the user submits a decision. `toolDecisions` is populated only * for a TOOL_CALL pause (per-call verdicts / amended arguments). */ onDecide: ( @@ -60,6 +74,35 @@ interface ApprovalBannerProps { ) => void; /** Called when the user cancels. */ onCancel?: () => void; + /** + * On a TOOL_CALL pause, disables top-level Approve until every gated call has + * an explicit per-call verdict — an untouched call otherwise inherits the + * top-level verdict silently (see the hint text below the call list). Regular + * conversation and group review intentionally keep that default; this exists + * for a surface where a swept-in call executes an unreviewed write against the + * platform itself (the Platform Operator), so "I clicked Approve" must mean + * "I looked at every call", not "I looked at the batch". + */ + requireExplicitPerCall?: boolean; + /** + * Call ids that must NOT be approvable, with the reason shown to the + * approver. Unlike every other signal on this banner these are a refusal, not + * a warning: Approve is disabled outright while any is present. + * + * Used by the operator surfaces for a write the operator aimed at its own + * agent document — see `self-guard.ts` for why that specific write ends with + * an operator that no longer needs approval for anything. Reject stays + * available, so the pause is never a dead end. + */ + blockedCalls?: readonly { callId: string; reason: string }[]; + /** + * Rendered per call, above the redacted arguments — e.g. the reconstructed + * "METHOD /path" a generated tool actually calls, which `PendingToolCallView` + * does not carry (only the operationId-derived tool name does). Kept as a + * render prop rather than a new required field so this stays optional for + * every other caller. + */ + renderCallExtra?: (call: PendingToolCallView) => ReactNode; } /** Whether a date string parses to a real instant. */ @@ -123,8 +166,13 @@ export function ApprovalBanner({ pauseDetails, isSubmitting, pauseDetailsPending, + pauseDetailsError, + onRetryPauseDetails, onDecide, onCancel, + requireExplicitPerCall = false, + blockedCalls, + renderCallExtra, }: ApprovalBannerProps) { const { t } = useTranslation(); const [note, setNote] = useState(""); @@ -154,6 +202,21 @@ export function ApprovalBanner({ const toolPause = pauseDetails?.type === "TOOL_CALL" ? pauseDetails : null; const isToolCall = !!toolPause; + // Every gated call must carry an explicit verdict before Approve is offered. + // Checked against callStates directly (not toolDecisions, which is built only + // at submit time) so the button reflects the CURRENT review state live. + const explicitReviewMissing = + requireExplicitPerCall && + isToolCall && + toolPause!.calls.some((call) => callStates[call.callId]?.verdict === undefined); + + // A refusal, not a nudge. Any blocked call disables Approve for the whole + // batch rather than only for itself: the per-call verdicts are submitted + // together, and letting the rest through would still run the batch that + // contains the write we are refusing to authorise. + const blocked = blockedCalls ?? []; + const approvalBlocked = blocked.length > 0; + const setCall = (callId: string, patch: Partial) => { setCallStates((prev) => ({ ...prev, [callId]: { ...prev[callId], ...patch } })); setSubmitError(null); @@ -269,12 +332,30 @@ export function ApprovalBanner({ variant: "warning" as const, }; case "REJECTED": - return { - title: t("hitl.confirmRejectTitle", "Reject request?"), - description: t("hitl.confirmRejectDescription", "Reject this request? The conversation will not proceed."), - confirmLabel: t("hitl.reject", "Reject"), - variant: "destructive" as const, - }; + // Two different outcomes, and conflating them pushed the wrong way. A + // TOOL_CALL rejection does NOT end the conversation: the backend + // (Conversation.java — `verdict == REJECTED && !toolPause`) short-circuits + // only RULE pauses; a rejected tool call becomes a synthetic rejection + // result and the model answers without it. Telling an approver their + // conversation dies is pressure toward Approve — precisely the + // rubber-stamping this whole flow exists to avoid. + return isToolCall + ? { + title: t("hitl.confirmRejectToolTitle", "Reject tool execution?"), + description: t( + "hitl.confirmRejectToolDescription", + "Nothing will run. The conversation continues and the agent answers without {{toolNames}}.", + { toolNames: gatedToolNames.join(", ") }, + ), + confirmLabel: t("hitl.reject", "Reject"), + variant: "destructive" as const, + } + : { + title: t("hitl.confirmRejectTitle", "Reject request?"), + description: t("hitl.confirmRejectDescription", "Reject this request? The conversation will not proceed."), + confirmLabel: t("hitl.reject", "Reject"), + variant: "destructive" as const, + }; case "CANCEL": return surface === "group" ? { @@ -373,12 +454,26 @@ export function ApprovalBanner({

{t("hitl.toolCallsAwaiting", "Tool calls awaiting approval")}

+ {/* The backend never sends raw arguments to a client — only the + redacted, size-capped form. A secret inside the payload reads as + the literal text "" here — the marker must match + `RequestRedactor.REDACTED` / `SecretRedactionFilter` exactly, + because this sentence is telling an approver what string to look + for. It said "[REDACTED]" until a review caught it, which sent + them hunting for something that never appears; an + approver who doesn't know that can mistake it for "nothing + sensitive was in this call" rather than "something was, and it was + hidden from you". */} +

+ {t("hitl.redactionCaveat", "Arguments shown below are redacted — a secret value appears as \"\", not omitted.")} +

{toolPause.calls.map((call) => ( setCall(call.callId, { verdict: callStates[call.callId]?.verdict === verdict ? undefined : verdict, @@ -400,8 +495,36 @@ export function ApprovalBanner({

)}

- {t("hitl.toolApprovalHint", "Approve applies your per-call choices (calls you didn't change are approved). Reject rejects the whole batch.")} + {requireExplicitPerCall + ? t("hitl.toolApprovalHintExplicit", "Every call needs its own Approve or Reject before you can approve the batch. Reject rejects the whole batch.") + : t("hitl.toolApprovalHint", "Approve applies your per-call choices (calls you didn't change are approved). Reject rejects the whole batch.")}

+ {explicitReviewMissing && ( +

+

+ )} + {approvalBlocked && ( + // role="alert" rather than a styled paragraph: Approve has just been + // taken away, and an approver who does not know why will assume the + // UI is broken and go looking for another way to do it. +
+

+

+
    + {blocked.map((entry) => ( +
  • {entry.reason}
  • + ))} +
+
+ )} )} @@ -510,18 +633,58 @@ export function ApprovalBanner({

)} - {pauseDetailsPending && ( + {pauseDetailsPending && !pauseDetailsError && (

)} + {pauseDetailsError && ( +
+
+ )} + {/* Action buttons */}
+ {extra} {call.arguments && (
       
       
+      
       
       
       
diff --git a/src/components/layout/sidebar.tsx b/src/components/layout/sidebar.tsx
index 6a52f6ee..18cc13cc 100644
--- a/src/components/layout/sidebar.tsx
+++ b/src/components/layout/sidebar.tsx
@@ -1,5 +1,6 @@
 import { NavLink } from "react-router-dom";
 import { useTranslation } from "react-i18next";
+import { usePendingApprovals } from "@/hooks/use-hitl";
 import {
   LayoutDashboard,
   Bot,
@@ -124,6 +125,13 @@ export function Sidebar({ collapsed, onToggle }: SidebarProps) {
     retry: 1, // Don't retry infinitely if offline
   });
 
+  // Shares its query key with the approvals page, so mounting this in the
+  // sidebar adds an observer rather than a second poll. The endpoint allows
+  // eddi-approver alongside admin/editor/user, so every role that can act on
+  // an approval can also see that one is waiting.
+  const { data: pendingApprovals } = usePendingApprovals();
+  const pendingApprovalCount = pendingApprovals?.length ?? 0;
+
   /** User initials for avatar */
   const initials = showUser
     ? [user.firstName, user.lastName]
@@ -246,7 +254,7 @@ export function Sidebar({ collapsed, onToggle }: SidebarProps) {
                     end={item.path === "/manage" || item.path === "/manage/conversations"}
                     className={({ isActive }) =>
                       cn(
-                        "flex items-center gap-3 rounded-lg px-3 py-2 text-sm font-medium transition-all",
+                        "relative flex items-center gap-3 rounded-lg px-3 py-2 text-sm font-medium transition-all",
                         "hover:bg-sidebar-accent/10 hover:text-sidebar-accent",
                         isActive
                           ? "border-s-2 border-sidebar-accent bg-sidebar-accent/10 text-sidebar-accent"
@@ -259,6 +267,27 @@ export function Sidebar({ collapsed, onToggle }: SidebarProps) {
                   >