From 7c9176fcc8042718553c1ec7f16ba543547af0d8 Mon Sep 17 00:00:00 2001 From: James Maes Date: Thu, 12 Nov 2026 10:37:13 -0600 Subject: [PATCH] fix(tools): the tool-call service owns the policy, the identity and the confirmation An independent security review of engine.ToolCallService (after PR 106) found what a caller could still hand it. The chat's turn engine is the only caller today and builds everything correctly, so none of it was reachable; it is closed before an MCP endpoint calls the service. The rule: every tool call runs as the signed-in person with their current permissions and budget, never as the assistant; a refusal happens before any data action; every call that ran has a log row with redacted input. Numbered as in the review: 1. A caller cannot supply a policy. The service reads it (PolicyService.current(false).tools(), narrowed for a scheduled task's conversation) and builds the tool Context itself; a caller hands it a tools.registry.ToolCall (person, id, name, input). Every change always shows a confirmation card (owner, 2026-11-12): ToolPolicy.writesRequireConfirmation is removed and UiApiToolbox always proposes. The assistant_policy column stays and is no longer read; no migration. 2. New SignedInPerson binds a session to the credentials forwarded to the UI API. No public constructor: ofRequest(Context), only for AssistantSpecs (ArchUnit), or ofSession(session). The context's gateway is always runtime.getUiApiGateway(). AssistantApi and TurnEngine lose withCredentials. 3. The hard budget is read on every call, not once per scope. executeAndLog(call, conversationId, hipaaRoute) makes the scope. 4. confirm(person, pendingActionId) replaces executeConfirmed, confirmationRefusal and logConfirmed: the service loads the stored action of that person, checks it, claims it once, runs it, marks it and writes the row. AssistantApi.confirmAction is a thin caller and the only one (ArchUnit). 5. The toolboxes cannot be called around the service: their entry points are package-private, the runtime has no getter for a toolbox (each family is handed its own), and ArchUnit forbids calls into tools.. from outside it except through the registry. The assistant's own reads (starter prompts, knowledge re-read) use the new read-only UiApiToolbox.read. 6. TurnEngine.executeTools writes each started call's row in a per-call finally: after an interruption of the engine, after a runtime exception while a result is processed or logged, and for a call cut short by its time limit, with the real outcome when the worker finished. A row that cannot be written for a call that changed data is logged at ERROR. 7. Whatever fails after a confirmation's claim ends in FAILED with a row, never in RUNNING. 8. The service checks that the scope's conversation and turn belong to the call's person and works out the row's client channel itself (ToolCallScope.clientChannel is gone). hipaaRoute stays the caller's; the javadoc says why. 9. AssistantQBitConfig.withToolFamily throws once the QBit is produced. Tests: ToolCallServiceTest (one per item, on the service directly), ToolCallRowsTest (item 6; all three fail on develop), and ToolCallArchitectureTest (items 2, 4, 5). ChatToolSetTest and ToolCallChecksTest pass with unchanged expectations. UiApiToolsSecurityTest's two tests that ran writes with confirmation off now propose and confirm. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 18 + CLAUDE.md | 13 +- README.md | 6 +- UPGRADE.md | 18 + docs/DESIGN.md | 6 +- .../qbits/assistant/AssistantQBitConfig.java | 10 +- .../qbits/assistant/AssistantRuntime.java | 37 +- .../qbits/assistant/SignedInPerson.java | 133 +++ .../assistant/access/DataAccessService.java | 17 + .../qbits/assistant/api/AssistantApi.java | 148 ++- .../assistant/api/specs/AssistantSpecs.java | 12 +- .../qbits/assistant/engine/PolicyService.java | 13 +- .../assistant/engine/StarterPrompts.java | 6 +- .../assistant/engine/ToolCallService.java | 954 +++++++++++++----- .../qbits/assistant/engine/TurnEngine.java | 369 ++++--- .../assistant/home/AssistantSetupStatus.java | 2 +- .../metadata/AssistantRecordScreens.java | 2 +- .../assistant/model/AssistantPolicy.java | 2 +- .../qbits/assistant/rag/RagService.java | 4 +- .../scheduled/ScheduledTaskRunner.java | 4 +- .../assistant/scheduled/ScheduledTurn.java | 2 +- .../qbits/assistant/tools/KnowledgeTool.java | 2 +- .../assistant/tools/KnowledgeToolFamily.java | 32 +- .../assistant/tools/PresentResultsTool.java | 2 +- .../assistant/tools/UiApiToolFamily.java | 38 +- .../qbits/assistant/tools/UiApiToolbox.java | 126 ++- .../tools/bulkload/BulkLoadToolFamily.java | 43 +- .../tools/bulkload/BulkLoadTools.java | 4 +- .../tools/registry/ConfirmedToolCall.java | 3 + .../assistant/tools/registry/ToolCall.java | 38 + .../tools/registry/ToolCallRequest.java | 12 +- .../tools/registry/ToolCallScope.java | 22 +- .../main/resources/help/qbit-assistant.yaml | 4 +- .../assistant/AssistantChannelWallTest.java | 3 +- .../AssistantDefaultsChangelogTest.java | 3 +- .../assistant/engine/StarterPromptsTest.java | 15 +- .../assistant/engine/ToolCallRowsTest.java | 289 ++++++ .../assistant/engine/ToolCallServiceTest.java | 749 +++++++++----- .../qbits/assistant/rag/RagTest.java | 21 +- .../qbits/assistant/testing/ToolCalls.java | 151 +++ .../assistant/tools/AnalysisSaveTest.java | 11 +- .../assistant/tools/AnalysisToolsTest.java | 6 +- .../tools/PresentResultsToolTest.java | 2 +- .../qbits/assistant/tools/Toolboxes.java | 97 ++ .../tools/UiApiToolsSecurityTest.java | 85 +- .../tools/bulkload/BulkLoadToolsTest.java | 25 +- .../tools/registry/ChatToolSetTest.java | 2 +- .../registry/ToolCallArchitectureTest.java | 215 +++- .../tools/registry/ToolRegistryTest.java | 53 +- 49 files changed, 2858 insertions(+), 971 deletions(-) create mode 100644 qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/SignedInPerson.java create mode 100644 qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCall.java create mode 100644 qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/ToolCallRowsTest.java create mode 100644 qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/testing/ToolCalls.java create mode 100644 qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/Toolboxes.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 27aa4b9..ab3edcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## Unreleased - The tool-call service owns the policy, the identity and the confirmation + +Closes what an independent security review found after "The tool-call service makes its own checks". None of it was reachable through the chat, which built everything correctly; it is closed before a second caller (an MCP route) uses the service. The rule: every tool call runs as the signed-in person with their current permissions and budget, never as the assistant; a refusal happens before any data action; every call that ran has a log row with redacted input. + +- **Every change always shows a confirmation card, and nothing can switch that off** (owner decision, 2026-11-12). `PolicyService.ToolPolicy` no longer has `writesRequireConfirmation`, and `UiApiToolbox` always proposes a create, update or delete. Behaviour change for a host that had set **Writes require confirmation** to off on the policy row: writes now wait for the card like everywhere else. The `assistant_policy.writesRequireConfirmation` column and entity field stay in place and are no longer read (no migration; nothing for a host to apply); the field's help text says so. +- **A caller cannot supply a policy** (review 1). The service reads the policy itself (`PolicyService.current(false).tools()`, narrowed for a scheduled task's conversation exactly as the chat narrows it) and builds the tool `Context` itself. A caller hands it a new `tools.registry.ToolCall` (person, call id, tool name, input) and nothing else. Before, a caller could pass `new ToolPolicy(false, Set.of(), 200, false)` and get every tool, 200 rows, no PII redaction and writes without confirmation. +- **A session cannot be mixed with another person's credentials** (review 2). New `SignedInPerson` binds a session to the credentials the tools forward to the UI API. It has no public constructor: `SignedInPerson.ofRequest(Context)` takes the session the route's session setup resolved and the credentials of that same request (only `AssistantSpecs` may call it, ArchUnit), and `SignedInPerson.ofSession(session)` forwards nothing but that session's own id. `AssistantApi.withCredentials` and `TurnEngine.withCredentials` are gone (`new AssistantApi(runtime, person)`, `TurnEngine.withPerson`, which refuses a person of another session); `new AssistantApi(runtime, session)` is unchanged. The gateway in the tool context is always `runtime.getUiApiGateway()`. +- **The hard budget is read on every call** (review 3). It was remembered for the life of a scope, and the scope was whatever object the caller passed. The check only reads (no budget event, no alert, the chat's warning notice stays with the model call that reports it). `executeAndLog(call, conversationId, hipaaRoute)` now makes the scope itself. +- **A write runs only from its stored pending action** (review 4, 7). `ToolCallService.executeConfirmed(ConfirmedToolCall)`, `confirmationRefusal` and `logConfirmed` are replaced by one entry, `confirm(person, pendingActionId)`: the service loads the action for that person, checks expiry, status and that the proposing turn has finished, refuses while the kill switch is on or the policy no longer allows the tool (the action stays pending), claims it once, runs it, marks it and writes the log row itself. The caller chooses no table, record or values. Whatever fails after the claim (a tool that throws, stored values that are not JSON) ends in `FAILED` with a row; before, the action stayed `RUNNING` forever with no row. `AssistantApi.confirmAction` is a thin caller and the only one (ArchUnit). The route's answers are unchanged. The confirm route never re-read the record to compare it with the proposal, and still does not: the UI API decides with the record as it is at confirmation. +- **The toolboxes cannot be called around the service** (review 5). `UiApiToolbox.execute`, `executeConfirmedWrite`, `executeConfirmedAnalysis`, `BulkLoadTools.execute` and `executeConfirmed`, `KnowledgeTool.execute` and `PresentResultsTool.execute` are package-private. `AssistantRuntime.getToolbox()`, `getKnowledgeTool()` and `getBulkLoadTools()` are gone; each family is handed its toolbox (`AssistantRuntime.ownToolFamilies()` is now an instance method). New ArchUnit rules: no class outside `tools..` calls into it except through the registry (allow-list: the runtime's wiring, the value types, `UiApiToolbox.notAWriteTool`, and the new `UiApiToolbox.read`); only the service constructs a `ToolCallRequest` or a `ConfirmedToolCall`; only `AssistantApi` calls `confirm`; only `RagService` and the knowledge tool call `RagService.read`. +- New `UiApiToolbox.read(context, callId, toolName, input)`: the assistant's own reads (starter prompts, re-reading a knowledge hit). It runs `list_tables`, `describe_table`, `count_records` and `get_record` and refuses every other name, so it cannot propose or change anything. `StarterPrompts` and `RagService` use it; nothing else may (ArchUnit). `AssistantApi` reads "is any table writable" from the new `DataAccessService.anyWritableTable()`. +- **A call that ran always has its row** (review 6). `TurnEngine.executeTools` writes each started call's row in a per-call `finally`. An interruption of the engine while calls run is passed on only after the rows are written (before, no row was written). A runtime exception while a result is classified, shown or logged (for example a tool's `inputForLog`) costs neither that row nor the later ones; the result is then withheld from the model. A call that was cut short gets up to one second, in all, to hand back what it did: when it finished after all, the row is of its real outcome, with the operation, status, audit link and pending action it left behind (before: "Timed out" and nothing else); the model and the panel are still told it took too long. `ToolCallService.log` no longer throws and returns whether the row was written; a row that cannot be written for a call that left something behind (a proposed write, an audited change, a confirmed write) is logged at ERROR, not WARN. +- **The scope is checked, not believed** (review 8). Before a tool runs, the service checks that the conversation and the turn a scope names belong to the call's person (and the turn to that conversation); a call that names someone else's is refused (`assistant:request`) and its row does not carry them. The row's client channel is worked out by the service; `ToolCallScope.clientChannel()` is gone. `hipaaRoute` stays the caller's to say, because only the caller knows which model or client receives the result (javadoc of `ToolCallService` and `ToolCallScope.hipaaRoute`). +- `AssistantQBitConfig.withToolFamily` throws `IllegalStateException` when it is called after the QBit was produced (review 9); it was silently ignored. +- For a host or another QBit with its own `ToolFamily`: `AssistantTool.execute(ToolCallRequest)` and `executeConfirmed(ConfirmedToolCall)` are unchanged. A tool must not construct either record itself. +- Stricter than before, in the chat: the policy a tool call runs under is the administrator's row as it is when the call starts, not the copy the turn read when it began (a tool switched off mid-turn is refused at once); a turn whose person goes over a hard budget while its tools run has the remaining calls refused (`assistant:budget`). +- Tests: `ToolCallServiceTest` (one per review item, calling the service directly), new `ToolCallRowsTest` (rows of a call that ran and timed out, of a runtime exception while logging, of an interrupted engine; all three fail on the code before), `ToolCallArchitectureTest` (the rules above). `ChatToolSetTest` and `ToolCallChecksTest` pass with their expectations unchanged (`ChatToolSetTest` builds a `ToolPolicy` without the removed argument). `UiApiToolsSecurityTest.testWriteDeniedIsTheSameDenial` and `testDenialMatrix` ran writes with confirmation off; they now propose and confirm, and compare the confirmed write's status and message with the UI's. + ## Unreleased - The tool-call service makes its own checks, for any caller - `engine.ToolCallService` no longer trusts its caller for a check it can make itself. On every call, before the tool is reached, it checks that the request is whole (a session, a context made for that same session, a policy), that the person has assistant access, the kill switch (read fresh), the hard budget, and that the tool is offered to this person now under the request's policy. The chat's turn engine made these checks around the service; a caller with no chat turn would have skipped them. The service's javadoc lists what a caller still has to supply: rate limiting, how many calls a request may make and for how long, and classification. diff --git a/CLAUDE.md b/CLAUDE.md index bf77629..c53b3ae 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,7 @@ This file provides guidance to Claude Code when working with code in this reposi The panel UI lives in `qqq-frontend-next`. Data access goes only through the **QQQ UI API** (`/qqq/v1`, spec'd operations, the user's own session) via `uiapi/UiApiGateway` and `tools/UiApiToolbox`; only `navigate` and `search_help` (no records) use the core AI tool kernel. - Design spec: `QRun-IO/qqq` `docs/specs/2026-10-11-qqq-assistant-qbit-design.md`. The owner decisions D1 to D6 at its top are binding. -- Writes (create/update/delete) are offered only for tables an admin set to READ_WRITE in `assistant_data_access`, and by default wait for the user's confirmation (pending action card). +- Writes (create/update/delete) are offered only for tables an admin set to READ_WRITE in `assistant_data_access`, and always wait for the user's confirmation (pending action card); nothing switches that off. ## Build Commands @@ -64,12 +64,17 @@ Always use the wrapper: Maven 3.10 cannot read the parent POM. The committed def - The vector index stores no record content: only source/table/key/field/chunk, content hash and embedding. Retrieval re-reads every hit through the UI API as the asking user, then applies the existing get_record exposure/masking path. - Only `persistence/` calls QueryAction, InsertAction and the other data actions (ArchUnit). - Every tool is registered once in `tools/registry/ToolRegistry` (name, kind, checks, definition, execute) by a registered `ToolFamily`: the QBit's own are the explicit list `AssistantRuntime.ownToolFamilies()`, a host's or another QBit's come from `AssistantQBitConfig.withToolFamily`. Nothing is found on the classpath. A new family of the QBit is new files and one entry in that list. - - A tool is looked up only in the registry. Outside `tools/`, only `engine/ToolCallService` runs a tool or writes an `assistant_tool_call` row (ArchUnit); the turn engine and the confirm route both go through it. A tool call runs as the signed-in person, never as the assistant. - - `ToolCallService` trusts no caller: on every call it checks itself that the request is whole (a session, a context made for that session, a policy), assistant access, the kill switch, the hard budget (per scope) and that the tool is offered to this person now, before the tool is reached; a confirmed write gets the first three and the policy's tool switch. A caller with no turn uses `executeAndLog`; only the turn engine may use `execute` and `log` separately (ArchUnit). What a caller still supplies (rate limiting, per-request call limits and timeouts, classification) is listed in the service's javadoc. + - A tool is looked up only in the registry. Outside `tools/`, only `engine/ToolCallService` runs a tool or writes an `assistant_tool_call` row (ArchUnit); the turn engine and the confirm route both go through it. A tool call runs as the signed-in person, with their current permissions and budget, never as the assistant; a refusal happens before any data action; every call that ran has a log row with redacted input. + - `ToolCallService` trusts no caller and takes nothing it can find out itself. A caller hands it a `tools.registry.ToolCall` (a `SignedInPerson`, the call's id, name and input) and nothing else: the service reads the policy (`PolicyService.current(false).tools()`, narrowed for a scheduled task's conversation), builds the tool `Context` with `runtime.getUiApiGateway()` and keeps the proposed writes itself. Only the service constructs a `ToolCallRequest` or a `ConfirmedToolCall` (ArchUnit). + - Its entry points: `executeAndLog(call, conversationId, hipaaRoute)` for a caller with no turn (the service makes the scope); `confirm(person, pendingActionId)`, called only by `AssistantApi` (ArchUnit); `execute(call, scope)` and `log(call, scope, outcome)` for the turn engine only (ArchUnit). + - On every call, before the tool is reached, it checks: a signed-in person, assistant access, the kill switch, that the scope's conversation and turn are that person's, the policy (unreadable refuses), the hard budget (read on every call, never remembered per scope) and that the tool is offered to this person now. What a caller still supplies (rate limiting, per-request call limits and timeouts, classification, the HIPAA route) is listed in the service's javadoc. + - `SignedInPerson` binds a session to the credentials forwarded to the UI API. It has no public constructor: `ofRequest(Context)` (only `AssistantSpecs`, ArchUnit) or `ofSession(session)` (no forwarded credentials). Never pass a session and credentials separately. + - The toolboxes cannot be called around the service: their entry points that run a tool are package-private, the runtime has no getter for a toolbox (each family is handed its own), and no class outside `tools/` calls into it except through the registry (ArchUnit, with a short allow-list: the runtime's wiring, the value types, and `UiApiToolbox.read`, the four read-only tools the starter prompts and the knowledge index use). + - The turn engine writes one row per started call in a per-call `finally`: a call that ran past its time limit, one whose result could not be processed, and a batch the engine was interrupted in all keep their rows, of the real outcome when the worker finished. A row that cannot be written for a call that changed data is logged at ERROR. - A missing policy refuses; it never means "every tool". - Data tools run only through `UiApiToolbox` → `UiApiGateway` (spec'd `/qqq/v1` operations with the user's `sessionUUID`); never core data actions for user data. Only `navigate` and `search_help` use `AiToolKernel.execute`. - Data access (`DataAccessService`) and the policy only ever narrow what the UI API allows; a table whose AI exposure is NONE stays NONE. - - A write runs only after the owner confirms its pending action (unless the policy turns confirmation off), with the confirming request's session, once. + - A write runs only after the owner confirms its pending action, with the confirming request's session, once; no policy or caller can turn confirmation off (`assistant_policy.writesRequireConfirmation` is no longer read). `ToolCallService.confirm` loads the stored action for that person, claims it, runs it and writes its row; whatever fails after the claim ends in FAILED with a row, never in RUNNING. - Panel API queries also filter `userId = me`, so an admin's all-access key never mixes other users' conversations into the panel. - **Caching:** the request prefix stays byte-stable. The order is tools (kernel-sorted), then system block 1 (rules + primer + app digest, LONG/1h), then system block 2 (user digest + date, SHORT/5m), then messages. Exactly one SHORT hint goes on the newest message. Don't put timestamps or ids in the system prompt. - **Tool results:** diff --git a/README.md b/README.md index 4ec7395..e8e9c0e 100644 --- a/README.md +++ b/README.md @@ -289,7 +289,7 @@ The nav lists the home and the main tables: **Providers**, **Conversations**, ** | `customRedactionPatterns` | none | Extra regular expressions the kernel redacts from tool results. | | `killSwitchPollMillis` | 2000 | How often a running turn re-checks the kill switch while it waits on the model. | | `providerTypes` | none | `withProviderType(...)`: the types the Providers screen offers (`AnthropicProviderType`). | -| `toolFamilies` | none | `withToolFamily(...)`: a tool family (`tools.registry.ToolFamily`) of the host or of another QBit. Its tools join the assistant's own in the tool registry when the QBit is produced. This is the only way a tool gets in from outside the QBit: a family class that is only on the classpath is never registered. The runtime refuses to start when two tools share a name, and logs the registered tool names once. | +| `toolFamilies` | none | `withToolFamily(...)`: a tool family (`tools.registry.ToolFamily`) of the host or of another QBit. Its tools join the assistant's own in the tool registry when the QBit is produced, so call it before that: a call after the QBit is produced throws `IllegalStateException` (it used to be ignored). This is the only way a tool gets in from outside the QBit: a family class that is only on the classpath is never registered. The runtime refuses to start when two tools share a name, and logs the registered tool names once. | | `allowedApiKeyEnvNames` | none | Extra environment variable names a provider row may name as its key. Each type's default is always allowed. | | `allowedBaseUrls` | none | Base URL overrides a provider row may use. The type's own URL is always allowed. | | `secretResolver` | `System::getenv` | How a key variable name becomes the key (a host secret store, or tests). | @@ -746,8 +746,8 @@ the task table. This feature does not provision an account or token. **Run now**, **Approve write mode** and **Revoke write mode** are Review/Result record actions. Write approval names the designated user and displays the exact prompt SHA-256. Changing prompt, user, credential reference, profile or write-mode -request clears approval. Scheduled writes always wait for that user's panel -confirmation, even if normal policy disables confirmation. +request clears approval. Scheduled writes wait for that user's panel +confirmation, as every write does. Each run keeps an owner-locked audit row and a normal conversation. RECORD adds no notification; NOTIFICATION and BOTH also notify the designated user through diff --git a/UPGRADE.md b/UPGRADE.md index 92d342b..cbe6176 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,23 @@ # Upgrading qbit-assistant +## Every write waits for confirmation; the tool-call service owns the policy and the identity (Unreleased) + +No schema change and nothing to apply to the database. What a host may notice: + +- **Writes require confirmation** on the `assistant_policy` row no longer does + anything: every create, update or delete the assistant proposes waits for the + user's confirmation card. The column `writes_require_confirmation` stays in + place and is not read. A host that had switched it off gets the card again. +- `AssistantQBitConfig.withToolFamily` throws when it is called after the QBit + was produced (it was ignored). Register tool families before producing it. +- Java API, for a host that called these directly: `AssistantApi.withCredentials` + is gone (`new AssistantApi(runtime, session)` is unchanged; a route uses + `new AssistantApi(runtime, SignedInPerson.ofRequest(context))`); + `AssistantRuntime.getToolbox()`, `getKnowledgeTool()` and `getBulkLoadTools()` + are gone; `PolicyService.ToolPolicy` has three components + (`allowedTools`, `maxRowsPerCall`, `redactPii`); `ToolCallScope.clientChannel()` + is gone. A host's own `ToolFamily` and its tools compile unchanged. + ## Conversation management (Unreleased, UI API extension assistant 1.6.0) No schema change. `assistant_conversation` is now usable by every user holding diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 97c14ad..f0705bc 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -120,7 +120,7 @@ Owner: *"the agent should have to go through the same API that the UIUX does. th 2. **Data access is configured and verifiable in the UI.** - Each table and process has an AI data access setting: NONE, READ or READ_WRITE. Sensitive tables default to NONE. - Admins can see what the assistant could do for a given user, and why. -3. **Writes need confirmation.** A write tool only *proposes* the change, as a pending action card. Nothing changes until the user confirms in the panel, unless the policy turns confirmation off. +3. **Writes need confirmation.** A write tool only *proposes* the change, as a pending action card. Nothing changes until the user confirms in the panel. No policy, setting or caller can turn confirmation off (owner, 2026-11-12). 4. **Persona and skills, never more access** (owner, 2026-10-13): *"in the config for the AI settings, we should be able to set a name and personality and extra skills etc for the agent - and again - the agent should be limited to only using our UIUX API and being able to only do the things that the currently logged in user could do with the same UIUX API via the UIUX"*. - **Profiles** (`assistant_profile`, admin-edited): display name, icon, personality (tone instructions, at most 2,000 characters), greeting and up to 6 starter prompts. - One profile is the default. Others are scoped to an app, to users holding a permission (a role), or to both. The most specific profile wins. @@ -176,7 +176,7 @@ This matches core's design (`docs/UI-API-EXTENSIONS.md` §5, `UiApiInvoker`, QRu `analyze_records` answers grouped questions (a count, sum, average, minimum, maximum or distinct count per group, for one or two group-by fields). The user needs what the Analyses screens need: read on the table and the table's analyze permission. The assistant adds its own limits first: data access READ on the table, and group-by, measure and filter fields whose values it may see (a hidden field is refused as an unknown one, before any request). It returns at most the policy's rows per call. `save_analysis` is the first write that creates more than one record. It follows W below (create and schedule a report through the saved-analysis and schedule tables' own insert operations) with these fixed points: -- One proposal, one card: the pending action stores the whole plan (the analysis definition and the schedule); the card lists what will be created and the numbers as they are now. It is never auto-confirmed, also where the policy turns confirmation off for record writes. +- One proposal, one card: the pending action stores the whole plan (the analysis definition and the schedule); the card lists what will be created and the numbers as they are now. It is never auto-confirmed (no write is). - The confirming user's session creates both records, the saved analysis first. If the schedule's insert is refused, the outcome says exactly that: the analysis exists, the schedule does not. - The schedule is DAILY at a local time in an IANA time zone, emailed to the confirming user. A time zone is never guessed: the tool refuses a schedule without one and tells the model to ask. - A schedule is refused, with the reason, while it could not run: the assistant reads the instance and asks the host (`withScheduledDeliveryCheck`). Whether a scheduled run can get the owner's current access when they are signed out is the host's to answer (a service principal, QQQ `docs/SERVICE-IDENTITY.md`). @@ -194,7 +194,7 @@ Running processes and reports through the assistant (`/processes/...`, `/reports - processes are NONE. The write tools are offered only when some table is READ_WRITE. 3. **Policy** (`assistant_policy`): - - `writesRequireConfirmation` (default true); + - every write waits for the user's confirmation card; no policy switches that off (the `writesRequireConfirmation` column is no longer read); - `allowedTools` (empty allows all); - `maxRowsPerCall` (default 50, ceiling 200); - `redactPii` (default true: secrets, card numbers and SSNs are redacted, with the core `AiRedactor`). diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantQBitConfig.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantQBitConfig.java index b1a7c68..3490a73 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantQBitConfig.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantQBitConfig.java @@ -1699,14 +1699,20 @@ public class AssistantQBitConfig implements QBitConfig ** is the only way a tool gets in from outside the QBit; a family class that ** is merely on the classpath is never registered. ** - ** Call it before the QBit is produced. The runtime refuses to start when a - ** registered tool has the name of another tool. A registered tool is + ** Call it before the QBit is produced: once the config has its runtime the + ** registry is complete, and a later call throws instead of being silently + ** ignored. The runtime refuses to start when a registered + ** tool has the name of another tool. A registered tool is ** offered and run like the assistant's own: as the signed-in person, after ** the checks of engine.ToolCallService, and it must apply its own checks to ** what it reads or writes (see tools.registry.ToolCheck). *******************************************************************************/ public AssistantQBitConfig withToolFamily(ToolFamily toolFamily) { + if(runtime != null) + { + throw (new IllegalStateException("withToolFamily was called after the assistant's runtime was built: the family would never be registered. Call it before the QBit is produced.")); + } if(toolFamily != null) { toolFamilies.add(toolFamily); diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantRuntime.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantRuntime.java index ef394b1..8ea6215 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantRuntime.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantRuntime.java @@ -327,24 +327,18 @@ public class AssistantRuntime - /******************************************************************************* - ** Getter for toolbox (the UI API data tools) - *******************************************************************************/ - public UiApiToolbox getToolbox() - { - return (toolbox); - } - - - /******************************************************************************* ** The QBit's own tool families: the complete list. A new family of the ** QBit is added here; one of a host or another QBit is registered with ** AssistantQBitConfig.withToolFamily. + ** + ** Each family is handed the toolbox its tools run. The runtime has no + ** getter for a toolbox: outside the tools packages a tool is reached only + ** through the registry, and run only by engine.ToolCallService. *******************************************************************************/ - public static List ownToolFamilies() + public List ownToolFamilies() { - return (List.of(new UiApiToolFamily(), new BulkLoadToolFamily(), new KnowledgeToolFamily(), new PresentResultsToolFamily(), new KernelToolFamily())); + return (List.of(new UiApiToolFamily(toolbox), new BulkLoadToolFamily(bulkLoadTools, toolbox), new KnowledgeToolFamily(knowledgeTool), new PresentResultsToolFamily(), new KernelToolFamily())); } @@ -624,14 +618,6 @@ public class AssistantRuntime - /******************************************************************************* - ** Permission-filtered retrieval tool. - *******************************************************************************/ - public KnowledgeTool getKnowledgeTool() - { - return (knowledgeTool); - } - /******************************************************************************* ** Server-owned scheduled execution context. *******************************************************************************/ @@ -651,17 +637,6 @@ public class AssistantRuntime - /******************************************************************************* - ** The bulk-load tools (saved profiles, attached files, dry runs, loads). - *******************************************************************************/ - public BulkLoadTools getBulkLoadTools() - { - return (bulkLoadTools); - } - - - - /******************************************************************************* ** Stop accepting work, cancel, wait a bounded grace, then force finalization. *******************************************************************************/ diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/SignedInPerson.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/SignedInPerson.java new file mode 100644 index 0000000..87f4459 --- /dev/null +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/SignedInPerson.java @@ -0,0 +1,133 @@ +/* + * Copyright 2026 Kingsrook, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.kingsrook.qbits.assistant; + + +import com.kingsrook.qqq.backend.core.context.QContext; +import com.kingsrook.qqq.backend.core.model.session.QSession; +import com.kingsrook.qqq.middleware.javalin.specs.invoke.UiApiCredentials; +import io.javalin.http.Context; + + +/******************************************************************************* + ** The signed-in person a request is made for: their own session and the + ** credentials the tools forward to the UI API, bound together when the + ** request's session is set up and never changed afterwards. A tool call, a + ** turn and a confirmation are all made for one of these, so none of them can + ** be handed one person's session with another person's credentials. + ** + ** There are two ways to get one, and no public constructor: + ** - {@link #ofRequest(Context)}, for a route: the session the route's session + ** setup put on this thread, with the credentials of that same HTTP request. + ** Only the assistant's route classes may call it (ArchUnit). + ** - {@link #ofSession(QSession)}, for code that has a session and no HTTP + ** request (a process, a scheduled run): no forwarded credentials at all, so + ** the UI API is called with that session's own id and nothing else. + ** + ** The credentials are never logged; toString leaves them out. + *******************************************************************************/ +public final class SignedInPerson +{ + private final QSession session; + private final UiApiCredentials credentials; + + + + /******************************************************************************* + ** + *******************************************************************************/ + private SignedInPerson(QSession session, UiApiCredentials credentials) + { + if(session == null) + { + throw (new IllegalArgumentException("A signed-in person needs a session.")); + } + this.session = session; + this.credentials = credentials; + } + + + + /******************************************************************************* + ** The person of the HTTP request being served on this thread: the session + ** the route's session setup resolved for it (QContext), and that request's + ** own credentials exactly as they came in. Both come from the one request, + ** so they are the same person's; neither is a value the caller chose. + *******************************************************************************/ + public static SignedInPerson ofRequest(Context request) + { + if(request == null) + { + throw (new IllegalArgumentException("A request is needed to take its credentials.")); + } + return (new SignedInPerson(QContext.getQSession(), UiApiCredentials.fromContext(request))); + } + + + + /******************************************************************************* + ** A person known by their session alone, with no HTTP request to take + ** credentials from: the UI API is called with this session's own id. + *******************************************************************************/ + public static SignedInPerson ofSession(QSession session) + { + return (new SignedInPerson(session, null)); + } + + + + /******************************************************************************* + ** The person's own session. + *******************************************************************************/ + public QSession session() + { + return (session); + } + + + + /******************************************************************************* + ** The credentials of the person's request, to forward to the UI API; null + ** when the session's own id is the credential. Never log them. + *******************************************************************************/ + public UiApiCredentials credentials() + { + return (credentials); + } + + + + /******************************************************************************* + ** The person's user id (the owner of their assistant rows); null when the + ** session names nobody. + *******************************************************************************/ + public String userId() + { + return (AssistantSecurity.userId(session)); + } + + + + /******************************************************************************* + ** Without the credentials. + *******************************************************************************/ + @Override + public String toString() + { + return ("SignedInPerson[" + userId() + "]"); + } +} diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/access/DataAccessService.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/access/DataAccessService.java index 5c1e2aa..15b9b45 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/access/DataAccessService.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/access/DataAccessService.java @@ -84,6 +84,23 @@ public class DataAccessService + /******************************************************************************* + ** Whether any table's data access allows writes (so write tools are offered). + *******************************************************************************/ + public boolean anyWritableTable() + { + for(String tableName : qInstance.getTables().keySet()) + { + if(forTable(tableName).mode().allowsWrite()) + { + return (true); + } + } + return (false); + } + + + /******************************************************************************* ** The mode for a table (unknown table: NONE). *******************************************************************************/ diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/api/AssistantApi.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/api/AssistantApi.java index 039fbef..963b234 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/api/AssistantApi.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/api/AssistantApi.java @@ -27,10 +27,10 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; -import com.fasterxml.jackson.core.type.TypeReference; import com.kingsrook.qbits.assistant.AssistantQBitConfig; import com.kingsrook.qbits.assistant.AssistantRuntime; import com.kingsrook.qbits.assistant.AssistantSecurity; +import com.kingsrook.qbits.assistant.SignedInPerson; import com.kingsrook.qbits.assistant.access.AccessExplainer; import com.kingsrook.qbits.assistant.access.DataAccessMode; import com.kingsrook.qbits.assistant.attachments.AttachmentService; @@ -58,7 +58,8 @@ import com.kingsrook.qbits.assistant.persistence.AssistantStore; import com.kingsrook.qbits.assistant.persistence.TranscriptRedactor; import com.kingsrook.qbits.assistant.spi.ModelCapabilities; import com.kingsrook.qbits.assistant.tools.UiApiToolbox; -import com.kingsrook.qbits.assistant.tools.registry.ConfirmedToolCall; +import com.kingsrook.qbits.assistant.tools.registry.AssistantTool; +import com.kingsrook.qbits.assistant.tools.registry.ToolOffer; import com.kingsrook.qqq.backend.core.actions.ai.AiRecordRef; import com.kingsrook.qqq.backend.core.actions.ai.AiToolOutcome; import com.kingsrook.qqq.backend.core.actions.ai.AiToolResult; @@ -72,7 +73,6 @@ import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.session.QSession; import com.kingsrook.qqq.backend.core.utils.JsonUtils; import com.kingsrook.qqq.backend.core.utils.StringUtils; -import com.kingsrook.qqq.middleware.javalin.specs.invoke.UiApiCredentials; import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; @@ -106,37 +106,51 @@ public class AssistantApi private final AssistantStore store; private final QSession session; private final String userId; - private UiApiCredentials credentials; + private final SignedInPerson person; /******************************************************************************* - ** Binds the API to a request's session and gives the session its own - ** owner key (so the record locks admit the user's own rows). + ** Binds the API to a session with no HTTP request behind it (a process, a + ** scheduled run, a test) and gives the session its own owner key (so the + ** record locks admit the user's own rows). The tools call the UI API with + ** that session's own id. *******************************************************************************/ public AssistantApi(AssistantRuntime runtime, QSession session) { - this.runtime = runtime; - this.config = runtime.getConfig(); - this.store = runtime.getStore(); - this.session = session; - this.userId = AssistantSecurity.userId(session); - if(session != null) - { - AssistantSecurity.ownerKey(session, config); - } + this(runtime, session, session == null ? null : SignedInPerson.ofSession(session)); } /******************************************************************************* - ** The credentials of the request this API serves, exactly as they came in; - ** the tools forward them to the UI API. Never logged. + ** Binds the API to the signed-in person of a request: their session and the + ** request's credentials, bound together by the route's session setup. The + ** tools forward those credentials to the UI API. There is no way to give + ** the API a session and credentials separately. *******************************************************************************/ - public AssistantApi withCredentials(UiApiCredentials credentials) + public AssistantApi(AssistantRuntime runtime, SignedInPerson person) { - this.credentials = credentials; - return (this); + this(runtime, person.session(), person); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + private AssistantApi(AssistantRuntime runtime, QSession session, SignedInPerson person) + { + this.runtime = runtime; + this.config = runtime.getConfig(); + this.store = runtime.getStore(); + this.session = session; + this.person = person; + this.userId = AssistantSecurity.userId(session); + if(session != null) + { + AssistantSecurity.ownerKey(session, config); + } } @@ -194,7 +208,7 @@ public class AssistantApi limits.put("attachmentTypes", AttachmentService.typeLabels(reason == null && visionModel())); } result.put("limits", limits); - result.put("features", Map.of("writes", runtime.getToolbox().anyWritableTable(), "pendingActions", true, "attachments", attachments)); + result.put("features", Map.of("writes", runtime.getDataAccessService().anyWritableTable(), "pendingActions", true, "attachments", attachments)); ProfileService.Persona persona = runtime.getProfileService().persona(session, null); Map profile = new LinkedHashMap<>(); profile.put("displayName", persona.displayName()); @@ -259,8 +273,9 @@ public class AssistantApi try { PolicyService.EffectivePolicy policy = runtime.getPolicyService().current(false); - UiApiToolbox.Context context = new UiApiToolbox.Context(session.getUuid(), session.getValue(QSession.VALUE_KEY_USER_TIMEZONE), runtime.getUiApiGateway(), policy.tools(), null, credentials); - boolean historyOffered = runtime.getToolbox().definitions(policy.tools(), false).stream().anyMatch(d -> UiApiToolbox.RECORD_HISTORY.equals(d.getName())); + UiApiToolbox.Context context = new UiApiToolbox.Context(session.getUuid(), session.getValue(QSession.VALUE_KEY_USER_TIMEZONE), runtime.getUiApiGateway(), policy.tools(), null, person.credentials()); + AssistantTool history = runtime.getToolRegistry().find(UiApiToolbox.RECORD_HISTORY); + boolean historyOffered = history != null && history.definition(new ToolOffer(policy.tools(), session, null)) != null; return (runtime.getStarterPrompts().forUser(context, userId, persona.starterPrompts(), historyOffered)); } catch(QException | RuntimeException e) @@ -590,7 +605,7 @@ public class AssistantApi RunningTurn runningTurn = new RunningTurn(turn.getId(), userId); TurnEngine engine = new TurnEngine(runtime, session, userId, conversation, userMessage, assistantMessage, turn, text, pageContext, policy, runningTurn, - budget.softExceeded() ? budget.message() : null).withCredentials(credentials).withTranscript(transcript); + budget.softExceeded() ? budget.message() : null).withPerson(person).withTranscript(transcript); runtime.register(runningTurn); if(!runningTurn.isFinalized()) { @@ -982,80 +997,37 @@ public class AssistantApi /******************************************************************************* - ** POST /actions/{id}/confirm: run the proposed write through the UI API - ** with THIS request's session (the user's own), then record the result and - ** a tool-call log row. A write is never run twice. + ** POST /actions/{id}/confirm: have the tool-call service run the proposed + ** write with THIS request's person (the user's own session), then put the + ** result into the conversation. The service does the rest itself: it loads + ** the stored action of this user, checks it, claims it so it never runs + ** twice, runs it, marks it and writes its tool-call log row. *******************************************************************************/ public Map confirmAction(Integer id) throws QException, AssistantApiException { requireAccess(); - AssistantPendingAction action = expireIfDue(requireAction(id)); - if(!AssistantPendingAction.STATUS_PENDING.equals(action.getStatus())) - { - throw (new AssistantApiException(409, "This action is already " + action.getStatus().toLowerCase() + ".")); - } - - AssistantTurn proposingTurn = store.getTurn(userId, action.getTurnId()); - if(proposingTurn != null && isRunning(proposingTurn)) - { - throw (new AssistantApiException(409, "Wait for the assistant to finish before confirming this action.")); - } - - /////////////////////////////////////////////////////////////////////// - // the kill switch and the policy's switch for this tool refuse a // - // confirmation as they refuse a call: asked before the claim, so a // - // refused action stays pending // - /////////////////////////////////////////////////////////////////////// - PolicyService.EffectivePolicy policy = runtime.getPolicyService().current(true); - UiApiToolbox.Context context = new UiApiToolbox.Context(session.getUuid(), session.getValue(QSession.VALUE_KEY_USER_TIMEZONE), runtime.getUiApiGateway(), policy.tools(), null, credentials, action.getTurnId() == null ? null : String.valueOf(action.getTurnId())); - AiToolResult refused = runtime.getToolCalls().confirmationRefusal(session, context, action.getToolCallId(), action.getToolName()); - if(refused != null) - { - throw (ToolCallService.CHECK_KILL_SWITCH.equals(refused.getPermissionCheck()) - ? new AssistantApiException(503, "KILL_SWITCH", KILL_SWITCH_MESSAGE) - : new AssistantApiException(403, "NOT_ALLOWED", refused.getModelContent())); - } - - /////////////////////////////////////////////////////////////////////// - // claim it first, so a double click cannot run the write twice // - /////////////////////////////////////////////////////////////////////// - Instant now = runtime.getClock().instant(); - store.updateFields(AssistantPendingAction.TABLE_NAME, new QRecord().withValue("id", id).withValue("status", "RUNNING").withValue("decidedAt", now)); - AssistantPendingAction claimed = store.getPendingAction(userId, id); - if(claimed == null || !"RUNNING".equals(claimed.getStatus())) - { - throw (new AssistantApiException(409, "This action is already being handled.")); - } - - Map values; + ToolCallService.Confirmation confirmation; try { - values = action.getParamsJson() == null ? Map.of() : JsonUtils.toObject(action.getParamsJson(), new TypeReference>() {}); + confirmation = runtime.getToolCalls().confirm(person, id); } - catch(Exception e) + catch(ToolCallService.ConfirmationRefused e) { - throw (new QException("The stored values of action " + id + " are not valid JSON", e)); + throw (switch(e.getReason()) + { + case NOT_FOUND -> new AssistantApiException(404, e.getMessage()); + case NOT_PENDING, TURN_RUNNING, ALREADY_HANDLED -> new AssistantApiException(409, e.getMessage()); + case KILL_SWITCH -> new AssistantApiException(503, "KILL_SWITCH", KILL_SWITCH_MESSAGE); + case NO_ACCESS, NOT_ALLOWED -> new AssistantApiException(403, "NOT_ALLOWED", e.getMessage()); + }); } - /////////////////////////////////////////////////////////////////////// - // the registry's tool runs its confirmed write, as the user who // - // confirms (this request's session) // - /////////////////////////////////////////////////////////////////////// - UiApiToolbox.Outcome outcome = runtime.getToolCalls().executeConfirmed( - new ConfirmedToolCall(session, context, action.getToolCallId(), action.getToolName(), action.getTableName(), action.getRecordId(), values, userId)); - AiToolResult result = outcome.getResult(); - boolean ok = result.getOutcome() == AiToolOutcome.OK; - store.updateFields(AssistantPendingAction.TABLE_NAME, new QRecord() - .withValue("id", id) - .withValue("status", ok ? AssistantPendingAction.STATUS_EXECUTED : AssistantPendingAction.STATUS_FAILED) - .withValue("httpStatus", outcome.getHttpStatus()) - .withValue("resultSummary", truncate(result.getSummary(), 500))); + AssistantPendingAction action = confirmation.action(); + UiApiToolbox.Outcome outcome = confirmation.outcome(); + AiToolResult result = outcome.getResult(); + reportDecision(action, result, result.getOutcome() == AiToolOutcome.OK); - runtime.getToolCalls().logConfirmed(action, userId, outcome); - - reportDecision(action, result, ok); - - Map rs = toJson(store.getPendingAction(userId, id)); + Map rs = toJson(action); rs.put("outcome", result.getOutcome() == null ? null : result.getOutcome().name()); rs.put("message", result.getModelContent()); if(outcome.getAuditHref() != null) @@ -1222,7 +1194,7 @@ public class AssistantApi requireTurn(turnId); } List> tables = new ArrayList<>(); - for(AccessExplainer.TableAccess access : runtime.getAccessExplainer().explain(runtime.getUiApiGateway(), session.getUuid(), credentials, runtime.getPolicyService().current(false).tools(), table, prompt, turnId == null ? null : String.valueOf(turnId))) + for(AccessExplainer.TableAccess access : runtime.getAccessExplainer().explain(runtime.getUiApiGateway(), session.getUuid(), person.credentials(), runtime.getPolicyService().current(false).tools(), table, prompt, turnId == null ? null : String.valueOf(turnId))) { if(!access.uiVisible() && access.mode() == DataAccessMode.NONE) { diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/api/specs/AssistantSpecs.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/api/specs/AssistantSpecs.java index 3c6bd30..945eb28 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/api/specs/AssistantSpecs.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/api/specs/AssistantSpecs.java @@ -22,12 +22,14 @@ import java.util.List; import java.util.Map; import com.fasterxml.jackson.annotation.JsonInclude; import com.kingsrook.qbits.assistant.AssistantRuntime; +import com.kingsrook.qbits.assistant.SignedInPerson; import com.kingsrook.qbits.assistant.api.AssistantApi; import com.kingsrook.qbits.assistant.api.AssistantApiException; import com.kingsrook.qbits.assistant.api.TurnStreams; import com.kingsrook.qbits.assistant.engine.RunningTurn; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QBadRequestException; +import com.kingsrook.qqq.backend.core.model.session.QSession; import com.kingsrook.qqq.backend.core.utils.ExceptionUtils; import com.kingsrook.qqq.backend.core.utils.JsonUtils; import com.kingsrook.qqq.backend.core.utils.StringUtils; @@ -36,7 +38,6 @@ import com.kingsrook.qqq.middleware.javalin.specs.BasicOperation; import com.kingsrook.qqq.middleware.javalin.specs.BasicResponse; import com.kingsrook.qqq.middleware.javalin.specs.extensions.AbstractJsonEndpointSpec; import com.kingsrook.qqq.middleware.javalin.specs.extensions.MiddlewareSpecExtension; -import com.kingsrook.qqq.middleware.javalin.specs.invoke.UiApiCredentials; import com.kingsrook.qqq.middleware.javalin.specs.v1.responses.BasicErrorResponseV1; import com.kingsrook.qqq.openapi.model.HttpMethod; import com.kingsrook.qqq.openapi.model.In; @@ -428,7 +429,14 @@ public final class AssistantSpecs //////////////////////////////////// } } - AssistantApi api = new AssistantApi(runtime, QContext.getQSession()).withCredentials(context == null ? null : UiApiCredentials.fromContext(context)); + /////////////////////////////////////////////////////////////////////// + // the one place a request's session and its credentials are bound: // + // the session this route's session setup resolved, with the // + // credentials of that same request // + /////////////////////////////////////////////////////////////////////// + QSession session = QContext.getQSession(); + AssistantApi api = session == null ? new AssistantApi(runtime, (QSession) null) + : new AssistantApi(runtime, context == null ? SignedInPerson.ofSession(session) : SignedInPerson.ofRequest(context)); return (handle(api, input)); } diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/PolicyService.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/PolicyService.java index 5b90aef..749e34f 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/PolicyService.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/PolicyService.java @@ -74,14 +74,17 @@ public class PolicyService /******************************************************************************* ** The policy row's tool settings, with their defaults. ** - ** @param writesRequireConfirmation every write waits for the user's - ** confirmation card (default true) + ** Whether a write waits for the person's confirmation is not one of them: + ** every change always shows a confirmation card, and no policy can switch + ** that off (the assistant_policy.writesRequireConfirmation column is still + ** there and is no longer read). + ** ** @param allowedTools tool names allowed; empty allows every tool ** @param maxRowsPerCall most rows one tool call returns to the model ** @param redactPii redact secrets and personal identifiers from tool ** results (default true) *******************************************************************************/ - public record ToolPolicy(boolean writesRequireConfirmation, Set allowedTools, int maxRowsPerCall, boolean redactPii) + public record ToolPolicy(Set allowedTools, int maxRowsPerCall, boolean redactPii) { public static final int DEFAULT_MAX_ROWS_PER_CALL = 50; public static final int MAX_ROWS_PER_CALL_CEILING = 200; @@ -93,7 +96,7 @@ public class PolicyService *******************************************************************************/ public static ToolPolicy defaults() { - return (new ToolPolicy(true, Set.of(), DEFAULT_MAX_ROWS_PER_CALL, true)); + return (new ToolPolicy(Set.of(), DEFAULT_MAX_ROWS_PER_CALL, true)); } @@ -129,7 +132,7 @@ public class PolicyService } } int maxRows = row.getMaxRowsPerCall() == null || row.getMaxRowsPerCall() < 1 ? DEFAULT_MAX_ROWS_PER_CALL : Math.min(MAX_ROWS_PER_CALL_CEILING, row.getMaxRowsPerCall()); - return (new ToolPolicy(!Boolean.FALSE.equals(row.getWritesRequireConfirmation()), allowed, maxRows, !Boolean.FALSE.equals(row.getRedactPii()))); + return (new ToolPolicy(allowed, maxRows, !Boolean.FALSE.equals(row.getRedactPii()))); } } diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/StarterPrompts.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/StarterPrompts.java index 465ceec..abe988d 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/StarterPrompts.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/StarterPrompts.java @@ -263,7 +263,7 @@ public class StarterPrompts *******************************************************************************/ private List readableTables(UiApiToolbox.Context context) { - UiApiToolbox.Outcome outcome = toolbox.execute(context, "starter-list", UiApiToolbox.LIST_TABLES, Map.of()); + UiApiToolbox.Outcome outcome = toolbox.read(context, "starter-list", UiApiToolbox.LIST_TABLES, Map.of()); Map data = json(outcome); List rs = new ArrayList<>(); if(data != null && data.get("tables") instanceof List tables) @@ -290,7 +290,7 @@ public class StarterPrompts Map filter = new LinkedHashMap<>(); filter.put("booleanOperator", "AND"); filter.put("criteria", List.of()); - UiApiToolbox.Outcome outcome = toolbox.execute(context, "starter-count", UiApiToolbox.COUNT_RECORDS, Map.of("table", table.name(), "filter", filter)); + UiApiToolbox.Outcome outcome = toolbox.read(context, "starter-count", UiApiToolbox.COUNT_RECORDS, Map.of("table", table.name(), "filter", filter)); if(outcome.getResult().getOutcome() != AiToolOutcome.OK || outcome.getResult().getRender() == null) { return (false); @@ -306,7 +306,7 @@ public class StarterPrompts *******************************************************************************/ private String dateField(UiApiToolbox.Context context, TableRef table) { - UiApiToolbox.Outcome outcome = toolbox.execute(context, "starter-describe", UiApiToolbox.DESCRIBE_TABLE, Map.of("table", table.name())); + UiApiToolbox.Outcome outcome = toolbox.read(context, "starter-describe", UiApiToolbox.DESCRIBE_TABLE, Map.of("table", table.name())); Map data = json(outcome); if(data == null || !(data.get("fields") instanceof List fields)) { diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/ToolCallService.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/ToolCallService.java index 5e14c6b..93e3502 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/ToolCallService.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/ToolCallService.java @@ -17,19 +17,30 @@ package com.kingsrook.qbits.assistant.engine; +import java.time.Instant; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; +import com.fasterxml.jackson.core.type.TypeReference; import com.kingsrook.qbits.assistant.AssistantQBitConfig; import com.kingsrook.qbits.assistant.AssistantRuntime; import com.kingsrook.qbits.assistant.AssistantSecurity; +import com.kingsrook.qbits.assistant.SignedInPerson; +import com.kingsrook.qbits.assistant.classification.Classifications; +import com.kingsrook.qbits.assistant.model.AssistantConversation; import com.kingsrook.qbits.assistant.model.AssistantPendingAction; +import com.kingsrook.qbits.assistant.model.AssistantScheduledTask; import com.kingsrook.qbits.assistant.model.AssistantToolCall; +import com.kingsrook.qbits.assistant.model.AssistantTurn; import com.kingsrook.qbits.assistant.persistence.AssistantStore; +import com.kingsrook.qbits.assistant.scheduled.ScheduledTurn; import com.kingsrook.qbits.assistant.tools.UiApiToolbox; import com.kingsrook.qbits.assistant.tools.registry.AssistantTool; import com.kingsrook.qbits.assistant.tools.registry.ConfirmedToolCall; +import com.kingsrook.qbits.assistant.tools.registry.ToolCall; import com.kingsrook.qbits.assistant.tools.registry.ToolCallOutcome; import com.kingsrook.qbits.assistant.tools.registry.ToolCallRequest; import com.kingsrook.qbits.assistant.tools.registry.ToolCallScope; @@ -37,10 +48,12 @@ import com.kingsrook.qbits.assistant.tools.registry.ToolOffer; import com.kingsrook.qbits.assistant.tools.registry.ToolRegistry; import com.kingsrook.qqq.backend.core.actions.ai.AiToolOutcome; import com.kingsrook.qqq.backend.core.actions.ai.AiToolResult; +import com.kingsrook.qqq.backend.core.actions.ai.DataClassification; import com.kingsrook.qqq.backend.core.actions.ai.output.AiRedactor; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.logging.QLogger; +import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.session.QSession; import com.kingsrook.qqq.backend.core.utils.JsonUtils; @@ -51,39 +64,59 @@ import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; /******************************************************************************* ** "Run one tool call for this person, and log it", for every caller: the ** chat's turn engine and the confirm route today, a caller with no chat turn - ** around it later. The service trusts no caller for a check it can make - ** itself. + ** around it (an MCP route) later. The service trusts no caller for anything + ** it can find out itself, and a caller cannot hand it a policy, a gateway, + ** loose credentials, a place to keep proposed writes, or the write to run. ** - ** What the service enforces on every call, in this order, before the tool is - ** reached (so a refusal happens before any data action): - ** 1. the request is whole: a session, a context made for that same session, - ** and a policy. A request without a policy is refused, never run - ** unnarrowed. - ** 2. assistant access: the assistant is enabled and the session's person - ** holds assistant.hasAccess. + ** The rule: every tool call runs as the signed-in person, with their own + ** session, current permissions and budget, never as the assistant; a refusal + ** happens before any data action; every call that ran has a log row with its + ** input redacted. + ** + ** The entry points: + ** - {@link #executeAndLog(ToolCall, Integer, boolean)}: one call for a + ** caller with no turn. The service makes the scope itself, from the person + ** and a conversation it checks is theirs, runs the call and writes its row. + ** - {@link #confirm(SignedInPerson, Integer)}: a write the person confirmed. + ** The service loads the stored pending action of that person, checks it, + ** claims it once, runs it, marks it and writes its row. Only the assistant + ** API calls it (ArchUnit). + ** - {@link #execute(ToolCall, ToolCallScope)} and + ** {@link #log(ToolCall, ToolCallScope, ToolCallOutcome)}: the two halves of + ** executeAndLog for the turn engine, which runs its calls in parallel under + ** its own time limit and writes the rows in call order afterwards. No other + ** class may use them (ArchUnit). + ** + ** What the service does itself on every call, in this order, before the tool + ** is reached (so a refusal happens before any data action): + ** 1. the call is whole: it names a signed-in person. A person is a + ** {@link SignedInPerson}: the session and the credentials forwarded to the + ** UI API are bound when the request's session is set up, so a call cannot + ** carry one person's session and another's credentials. + ** 2. assistant access: the assistant is enabled and the person holds + ** assistant.hasAccess. ** 3. the kill switch, read fresh; a policy that cannot be read counts as on. - ** 4. the hard budget (the policy's hard limits, and a budget that has stopped - ** the person), when the first call of a scope arrives. It only reads: no - ** budget event, no alert, nothing charged. - ** 5. the offer: the tool is one this person is offered now under the - ** request's policy (not switched off by the policy and, for a write or a - ** bulk-load tool, with the writable table or the profiles it needs). A - ** name the registry does not have is refused the same way. - ** Then the registry's tool runs with the person's own session and applies its - ** own checks (the policy's tool switch again, data access, field exposure, - ** confirmation, what the person may do in the UI API). - ** - ** A confirmed write ({@link #executeConfirmed}) goes through 1 to 3 and the - ** policy's switch for its tool. It spends no model budget, so the hard budget - ** does not stop it. - ** - ** {@link #executeAndLog} is the one call a caller without a turn makes: it - ** runs the call and writes its assistant_tool_call row, redacting the input - ** itself, for a refused call as for one that ran. The turn engine runs its - ** calls in parallel under its own time limit and writes the rows in call - ** order afterwards, so it uses the two halves of that same path, - ** {@link #execute} and {@link #log}; no other class may - ** (ToolCallArchitectureTest). + ** 4. the scope is the person's: a conversation or a turn it names belongs to + ** that person (and the turn to that conversation). The client channel of + ** the log row is worked out here, not taken from the caller. + ** 5. the policy: the administrator's current tool settings, read by the + ** service (narrowed for a scheduled task's conversation, as the chat + ** does). A policy that cannot be read refuses. + ** 6. the hard budget (the policy's hard limits, and a budget that has stopped + ** the person), on every call. It only reads: no budget event, no alert, + ** nothing charged, and the chat's warning notice is left for the model + ** call that reports it. + ** 7. the offer: the tool is one this person is offered now under that policy + ** (not switched off and, for a write or a bulk-load tool, with the + ** writable table or the profiles it needs). A name the registry does not + ** have is refused the same way. + ** Then the registry's tool runs with a context the service built (the + ** runtime's gateway, the person's own session and credentials, the policy + ** from step 5, the service's own pending-action store) and applies its own + ** checks (the policy's tool switch again, data access, field exposure, what + ** the person may do in the UI API). A tool that changes something only ever + ** proposes it: the person confirms it on a card, and no policy or caller can + ** switch that off. ** ** What a caller still has to supply, because only it knows its own requests: ** - rate limiting (the chat: RateLimiter, turns per minute and concurrent @@ -94,18 +127,15 @@ import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; ** stopped itself ({@link #log} with the result it gave instead); ** - classification: keeping the scope's data classification up to date from ** every result and persisting it (the chat: TurnClassification.observe and - ** persist), and a truthful {@link ToolCallScope#hipaaRoute()}; - ** - the scope itself: one per request, never shared by two people, with the - ** client channel its calls are logged under; - ** - the session: the signed-in person's own, from the route's session setup, - ** never one the caller made. Request credentials the context forwards to - ** the UI API must be that same person's: the service compares the context's - ** session id with the session, but it cannot check forwarded credentials; - ** - the policy's tool settings in force for the request (the service refuses - ** a missing policy, but does not compare the one it is given with the - ** administrator's row). + ** persist); + ** - the HIPAA route ({@link ToolCallScope#hipaaRoute()}, or the hipaaRoute + ** argument of executeAndLog): whether whoever receives the result is HIPAA + ** eligible. The service cannot know which model or client the caller will + ** send the result to, so it cannot check this; a caller that cannot vouch + ** for its route passes false; + ** - the person: a {@link SignedInPerson} from the route's session setup, + ** never a session the caller made. ** - ** A call never runs as the assistant: the session is always the request's. ** The log row is written through the store, as the same person. *******************************************************************************/ public class ToolCallService @@ -121,6 +151,8 @@ public class ToolCallService public static final String CHECK_BUDGET = "assistant:budget"; public static final String CHECK_TOOL_SWITCH = "policy:tool"; + private static final String CHANNEL = "assistant"; + private static final AiRedactor INPUT_REDACTOR = new AiRedactor(List.of()); private final AssistantRuntime runtime; @@ -143,29 +175,167 @@ public class ToolCallService /******************************************************************************* - ** What the service remembers for one scope: the offer its calls are checked - ** against, and that the hard budget let the scope's first call through. It - ** belongs to one session and one policy; a request with another is checked - ** afresh. + ** Why a confirmation was not run. Nothing was changed, and the pending + ** action is as it was (still pending, unless it had expired). *******************************************************************************/ - private static final class Admission + public static final class ConfirmationRefused extends Exception { - private QSession session; - private PolicyService.ToolPolicy policy; - private ToolOffer offer; - private boolean withinBudget; + /******************************************************************************* + ** What stopped the confirmation. + *******************************************************************************/ + public enum Reason + { + NO_ACCESS, + NOT_FOUND, + NOT_PENDING, + TURN_RUNNING, + KILL_SWITCH, + NOT_ALLOWED, + ALREADY_HANDLED + } + + + + private final Reason reason; /******************************************************************************* - ** Bind a new admission to the request's person and policy. + ** *******************************************************************************/ - private Admission bind(ToolCallRequest request) + private ConfirmationRefused(Reason reason, String message) { - session = request.session(); - policy = request.context().policy(); - offer = new ToolOffer(policy, session, request.context().turnId()); - return (this); + super(message); + this.reason = reason; + } + + + + /******************************************************************************* + ** Getter for reason + *******************************************************************************/ + public Reason getReason() + { + return (reason); + } + } + + + + /******************************************************************************* + ** A confirmation that was run: the pending action as it is now (EXECUTED or + ** FAILED) and what the write gave back. + ** + ** @param action the pending action, re-read after it was marked + ** @param outcome the write's result, with its UI API operation detail + *******************************************************************************/ + public record Confirmation(AssistantPendingAction action, UiApiToolbox.Outcome outcome) + { + } + + + + /******************************************************************************* + ** What the service has worked out for one scope and one person: that the + ** scope's conversation and turn are theirs, the channel its rows are logged + ** under, and the offer its calls are checked against under the policy the + ** service read. The hard budget is not remembered here: it is read on every + ** call. An admission belongs to one person, one conversation, one turn and + ** one policy; a call with another is checked afresh. + *******************************************************************************/ + private static final class Admission + { + private QSession session; + private Integer conversationId; + private Integer turnId; + private boolean bound; + private boolean owned; + private String channel = CHANNEL; + private AssistantScheduledTask task; + private PolicyService.ToolPolicy basePolicy; + private PolicyService.ToolPolicy policy; + private ToolOffer offer; + } + + + + /******************************************************************************* + ** The scope of a call that has no turn: made by the service from what it + ** checked. The classification is the conversation's stored one once the + ** conversation is known to be the person's. + *******************************************************************************/ + private static final class OwnScope implements ToolCallScope + { + private final Integer conversationId; + private final boolean hipaaRoute; + private final Map, Object> state = new ConcurrentHashMap<>(); + + private volatile DataClassification classification = DataClassification.PUBLIC; + + + + /******************************************************************************* + ** + *******************************************************************************/ + private OwnScope(Integer conversationId, boolean hipaaRoute) + { + this.conversationId = conversationId; + this.hipaaRoute = hipaaRoute; + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public DataClassification classification() + { + return (classification); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public Integer conversationId() + { + return (conversationId); + } + + + + /******************************************************************************* + ** A call with no turn. + *******************************************************************************/ + @Override + public Integer turnId() + { + return (null); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public boolean hipaaRoute() + { + return (hipaaRoute); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public T state(Class type, Supplier create) + { + return (type.cast(state.computeIfAbsent(type, key -> create.get()))); } } @@ -223,21 +393,44 @@ public class ToolCallService /******************************************************************************* - ** Run one call as the request's person, after every check of the service. - ** A refusal is a result, and nothing ran. The turn engine logs the outcome - ** with {@link #log}; every other caller uses {@link #executeAndLog}. + ** Run one call as its person, after every check of the service. A refusal + ** is a result, and nothing ran. Only the turn engine calls this, and logs + ** the outcome with {@link #log}; every other caller uses + ** {@link #executeAndLog}. *******************************************************************************/ - public ToolCallOutcome execute(ToolCallRequest request) + public ToolCallOutcome execute(ToolCall call, ToolCallScope scope) { - AiToolResult incomplete = incomplete(request); - if(incomplete != null) + if(call == null || call.person() == null || call.person().userId() == null) { - return (ToolCallOutcome.of(incomplete)); + return (ToolCallOutcome.of(denied(result(call), CHECK_REQUEST, "The tool call has no signed-in person."))); } - try(PersonsContext ignored = new PersonsContext(request.session())) + if(scope == null) { - AiToolResult refused = refusal(request); - return (refused != null ? ToolCallOutcome.of(refused) : registry.find(request.toolName()).execute(request)); + return (ToolCallOutcome.of(denied(result(call), CHECK_REQUEST, "The tool call has no scope."))); + } + + QSession session = call.person().session(); + try(PersonsContext ignored = new PersonsContext(session)) + { + AssistantSecurity.ownerKey(session, runtime.getConfig()); + AiToolResult closed = accessOrKillSwitch(session, result(call)); + if(closed != null) + { + return (ToolCallOutcome.of(closed)); + } + + Admission admission = admission(call.person(), scope); + ToolCallRequest request; + synchronized(admission) + { + AiToolResult refused = refusal(call, admission); + if(refused != null) + { + return (ToolCallOutcome.of(refused)); + } + request = new ToolCallRequest(session, context(call.person(), admission), call.toolCallId(), call.toolName(), call.input(), scope); + } + return (registry.find(call.toolName()).execute(request)); } } @@ -245,55 +438,86 @@ public class ToolCallService /******************************************************************************* ** Run one call and write its tool-call log row: the whole of a tool call - ** for a caller with no turn. A refused call is logged like one that ran, - ** and a tool that throws gives a fixed ERROR result and its row. When the - ** row cannot be written the caller gets the exception, not the result: a - ** result is never handed out unlogged. Only a call that names no person at - ** all (which was refused) has no row. + ** for a caller with no turn. The scope is the service's own: the call + ** belongs to the conversation named (which must be the person's; null for + ** none) and to no turn. hipaaRoute is the caller's to say, because only the + ** caller knows whether whoever receives the result is HIPAA eligible; pass + ** false when that is not known. + ** + ** A refused call is logged like one that ran, and a tool that throws gives + ** a fixed ERROR result and its row. When the row cannot be written the + ** caller gets the exception, not the result: a result is never handed out + ** unlogged. Only a call that names no person at all (which was refused) has + ** no row. *******************************************************************************/ - public ToolCallOutcome executeAndLog(ToolCallRequest request) throws QException + public ToolCallOutcome executeAndLog(ToolCall call, Integer conversationId, boolean hipaaRoute) throws QException { + ToolCallScope scope = new OwnScope(conversationId, hipaaRoute); ToolCallOutcome outcome; try { - outcome = execute(request); + outcome = execute(call, scope); } catch(RuntimeException e) { - LOG.warn("Assistant tool call threw", logPair("tool", request.toolName()), logPair("errorClass", e.getClass().getSimpleName())); - outcome = ToolCallOutcome.of(result(request).withOutcome(AiToolOutcome.ERROR).withSummary("Failed") + LOG.warn("Assistant tool call threw", logPair("tool", call.toolName()), logPair("errorClass", e.getClass().getSimpleName())); + outcome = ToolCallOutcome.of(result(call).withOutcome(AiToolOutcome.ERROR).withSummary("Failed") .withModelContent("The tool failed unexpectedly. Answer with what you have, or try another way.")); } - if(request == null || AssistantSecurity.userId(request.session()) == null) + if(call == null || call.person() == null || call.person().userId() == null) { - LOG.warn("An assistant tool call that names no signed-in person was refused; it has no log row", logPair("tool", request == null ? null : request.toolName())); + LOG.warn("An assistant tool call that names no signed-in person was refused; it has no log row", logPair("tool", call == null ? null : call.toolName())); return (outcome); } - log(request, outcome); + try + { + writeRow(call, scope, outcome); + } + catch(QException | RuntimeException e) + { + reportUnlogged(call, scope, outcome, e); + throw (e instanceof QException q ? q : new QException("The tool call could not be logged.", e)); + } return (outcome); } /******************************************************************************* - ** Why a request cannot be checked at all, or null when it is whole: it - ** names a person (a session), carries a context made for that same session, - ** and a policy. + ** The refusal of a call by the scope, the policy, the hard budget or the + ** offer, or null when its tool may run. The admission is the caller's lock. *******************************************************************************/ - private static AiToolResult incomplete(ToolCallRequest request) + private AiToolResult refusal(ToolCall call, Admission admission) { - if(request == null || request.session() == null || request.context() == null) + AiToolResult result = result(call); + if(!admission.owned) { - return (denied(result(request), CHECK_REQUEST, "The tool call has no signed-in person.")); + return (denied(result, CHECK_REQUEST, "The tool call names a conversation or a turn that is not this person's.")); } - if(!Objects.equals(request.session().getUuid(), request.context().sessionUUID())) + + PolicyService.EffectivePolicy effective; + try { - return (denied(result(request), CHECK_REQUEST, "The tool call's session and its context are not the same person's.")); + effective = runtime.getPolicyService().current(false); + policy(admission, effective.tools()); } - if(request.context().policy() == null) + catch(QException | RuntimeException e) { - return (denied(result(request), CHECK_REQUEST, "The tool call has no policy.")); + LOG.warn("Could not read the assistant policy; refusing the tool call (fail closed)", logPair("errorClass", e.getClass().getSimpleName())); + return (denied(result, CHECK_REQUEST, "The assistant's policy could not be read. Try again in a moment.")); + } + + String reason = hardBudget(admission.session, effective); + if(reason != null) + { + return (denied(result, CHECK_BUDGET, reason)); + } + + AssistantTool tool = registry.find(call.toolName()); + if(tool == null || tool.definition(admission.offer) == null) + { + return (notOffered(call.toolCallId(), call.toolName())); } return (null); } @@ -301,38 +525,62 @@ public class ToolCallService /******************************************************************************* - ** The refusal of a whole request, or null when its tool may run: assistant - ** access, the kill switch, the hard budget, then the offer. + ** The policy a scope's calls run under: the administrator's current tool + ** settings, narrowed for a scheduled task's conversation. The offer is made + ** again only when those settings changed since the scope's last call. *******************************************************************************/ - private AiToolResult refusal(ToolCallRequest request) + private void policy(Admission admission, PolicyService.ToolPolicy current) throws QException { - AiToolResult result = result(request); - AiToolResult closed = accessOrKillSwitch(request.session(), result); - if(closed != null) + if(admission.offer != null && current.equals(admission.basePolicy)) { - return (closed); + return; } + admission.basePolicy = current; + admission.policy = admission.task == null ? current : ScheduledTurn.tools(runtime, admission.task, current); + admission.offer = new ToolOffer(admission.policy, admission.session, admission.turnId == null ? null : String.valueOf(admission.turnId)); + } - Admission admission = admission(request); - synchronized(admission) - { - if(!admission.withinBudget) - { - String reason = hardBudget(request.session()); - if(reason != null) - { - return (denied(result, CHECK_BUDGET, reason)); - } - admission.withinBudget = true; - } - AssistantTool tool = registry.find(request.toolName()); - if(tool == null || tool.definition(admission.offer) == null) - { - return (notOffered(request.toolCallId(), request.toolName())); - } - } - return (null); + + /******************************************************************************* + ** The context a tool runs with, made here and nowhere else: the person's + ** own session and credentials, the runtime's gateway, the policy the + ** service read, and the service's own store of proposed writes. + *******************************************************************************/ + private UiApiToolbox.Context context(SignedInPerson person, Admission admission) + { + QSession session = person.session(); + String userId = person.userId(); + Integer conversationId = admission.conversationId; + Integer turnId = admission.turnId; + return (new UiApiToolbox.Context(session.getUuid(), session.getValue(QSession.VALUE_KEY_USER_TIMEZONE), runtime.getUiApiGateway(), admission.policy, + write -> propose(userId, conversationId, turnId, write), person.credentials(), turnId == null ? null : String.valueOf(turnId))); + } + + + + /******************************************************************************* + ** Record a write the person must confirm; nothing runs until they do + ** ({@link #confirm}). + *******************************************************************************/ + private Integer propose(String userId, Integer conversationId, Integer turnId, UiApiToolbox.ProposedWrite write) throws QException + { + Instant now = runtime.getClock().instant(); + Integer ttl = runtime.getConfig().getPendingActionTtlSeconds(); + AssistantPendingAction action = store.insert(new AssistantPendingAction() + .withConversationId(conversationId) + .withTurnId(turnId) + .withUserId(userId) + .withToolCallId(write.toolCallId()) + .withToolName(write.toolName()) + .withUiApiOperation(write.operation()) + .withTableName(write.tableName()) + .withRecordId(write.recordId()) + .withParamsJson(JsonUtils.toJson(write.values())) + .withSummary(write.summary()) + .withStatus(AssistantPendingAction.STATUS_PENDING) + .withExpiresAt(now.plusSeconds(ttl == null ? 900 : ttl))); + return (action.getId()); } @@ -344,13 +592,10 @@ public class ToolCallService *******************************************************************************/ private AiToolResult accessOrKillSwitch(QSession session, AiToolResult result) { - if(!runtime.getConfig().isEffectivelyEnabled()) + AiToolResult noAccess = noAccess(session, result); + if(noAccess != null) { - return (denied(result, CHECK_ACCESS, "The assistant is not enabled.")); - } - if(AssistantSecurity.userId(session) == null || !session.hasPermission(AssistantQBitConfig.ACCESS_PERMISSION)) - { - return (denied(result, CHECK_ACCESS, "You do not have access to the assistant.")); + return (noAccess); } if(runtime.getPolicyService().isKillSwitchOn()) { @@ -362,16 +607,33 @@ public class ToolCallService /******************************************************************************* - ** What the person is told when a hard budget stops them, or null. It only - ** reads. A budget that cannot be read stops the call. + ** The refusal of a person without assistant access, or null. *******************************************************************************/ - private String hardBudget(QSession session) + private AiToolResult noAccess(QSession session, AiToolResult result) + { + if(!runtime.getConfig().isEffectivelyEnabled()) + { + return (denied(result, CHECK_ACCESS, "The assistant is not enabled.")); + } + if(AssistantSecurity.userId(session) == null || !session.hasPermission(AssistantQBitConfig.ACCESS_PERMISSION)) + { + return (denied(result, CHECK_ACCESS, "You do not have access to the assistant.")); + } + return (null); + } + + + + /******************************************************************************* + ** What the person is told when a hard budget stops them, or null. It only + ** reads, on every call. A budget that cannot be read stops the call. + *******************************************************************************/ + private String hardBudget(QSession session, PolicyService.EffectivePolicy policy) { try { - String userId = AssistantSecurity.userId(session); - PolicyService.EffectivePolicy policy = runtime.getPolicyService().current(false); - PolicyService.BudgetStatus configured = runtime.getPolicyService().checkConfiguredBudget(policy, userId, 0, 0); + String userId = AssistantSecurity.userId(session); + PolicyService.BudgetStatus configured = runtime.getPolicyService().checkConfiguredBudget(policy, userId, 0, 0); if(configured.hardExceeded()) { return (configured.message()); @@ -388,30 +650,74 @@ public class ToolCallService /******************************************************************************* - ** The scope's admission when it was made for this request's session and - ** policy; one of the request's own otherwise (also for a call without a - ** scope), so one person's offer never answers for another. + ** The scope's admission when it was made for this person, conversation and + ** turn; one of the call's own otherwise, so one person's offer or checked + ** conversation never answers for another. Called with the person's session + ** as the thread's context (the ownership check reads the store). *******************************************************************************/ - private static Admission admission(ToolCallRequest request) + private Admission admission(SignedInPerson person, ToolCallScope scope) { - if(request.scope() == null) + Admission admission = scope.state(Admission.class, Admission::new); + if(admission == null) { - return (new Admission().bind(request)); + admission = new Admission(); } - Admission admission = request.scope().state(Admission.class, Admission::new); synchronized(admission) { - if(admission.offer == null) + if(!admission.bound) { - return (admission.bind(request)); + return (bind(admission, person, scope)); } - boolean same = admission.session == request.session() && admission.policy.equals(request.context().policy()); - return (same ? admission : new Admission().bind(request)); + boolean same = admission.session == person.session() && Objects.equals(admission.conversationId, scope.conversationId()) && Objects.equals(admission.turnId, scope.turnId()); + return (same ? admission : bind(new Admission(), person, scope)); } } + /******************************************************************************* + ** Bind a new admission to the person and to what the scope names, and find + ** out whether that is theirs: a conversation the person owns, a turn the + ** person owns in that conversation. A scheduled login session's calls + ** belong to its task's conversation and to nothing else. When the store + ** cannot be read the scope is not the person's (fail closed). + *******************************************************************************/ + private Admission bind(Admission admission, SignedInPerson person, ToolCallScope scope) + { + admission.bound = true; + admission.session = person.session(); + admission.conversationId = scope.conversationId(); + admission.turnId = scope.turnId(); + try + { + String userId = person.userId(); + AssistantConversation conversation = admission.conversationId == null ? null : store.getConversation(userId, admission.conversationId); + AssistantTurn turn = admission.turnId == null ? null : store.getTurn(userId, admission.turnId); + boolean conversationOwned = admission.conversationId == null || conversation != null; + boolean turnOwned = admission.turnId == null || (turn != null && admission.conversationId != null && Objects.equals(turn.getConversationId(), admission.conversationId)); + + admission.task = admission.conversationId == null ? null : runtime.getScheduledTurn().task(admission.conversationId); + boolean scheduledSession = runtime.getScheduledTurn().channel(person.session().getUuid()) != null; + admission.owned = conversationOwned && turnOwned && (!scheduledSession || admission.task != null); + if(admission.owned) + { + admission.channel = admission.task != null ? CHANNEL + "/scheduled/" + admission.task.getId() : (admission.turnId == null ? CHANNEL : CHANNEL + "/" + admission.turnId); + if(scope instanceof OwnScope own && conversation != null) + { + own.classification = Classifications.stored(conversation.getMaxClassification()); + } + } + } + catch(QException | RuntimeException e) + { + LOG.warn("Could not check whose conversation a tool call names; refusing it (fail closed)", logPair("errorClass", e.getClass().getSimpleName())); + admission.owned = false; + } + return (admission); + } + + + /******************************************************************************* ** The refusal of a call to a tool the person is not offered (a name the ** model made up, or a tool the policy or the host switched off). @@ -425,11 +731,11 @@ public class ToolCallService /******************************************************************************* - ** An empty result for the request's call. + ** An empty result for the call. *******************************************************************************/ - private static AiToolResult result(ToolCallRequest request) + private static AiToolResult result(ToolCall call) { - return (new AiToolResult().withToolCallId(request == null ? null : request.toolCallId()).withToolName(request == null ? null : request.toolName())); + return (new AiToolResult().withToolCallId(call == null ? null : call.toolCallId()).withToolName(call == null ? null : call.toolName())); } @@ -445,103 +751,257 @@ public class ToolCallService /******************************************************************************* - ** Why a write the person confirmed may not run now, or null when it may: - ** the confirming session and its context are the same person's and carry a - ** policy, that person has assistant access, the kill switch is off, and the - ** policy (the confirming request's and the administrator's current one) - ** allows the tool. The confirm route asks before it claims the pending - ** action, so a refused confirmation leaves the action pending; - ** {@link #executeConfirmed} asks again itself. + ** Run a write the person confirmed: the whole of a confirmation. The caller + ** names the person and the pending action's id and nothing else; the table, + ** the record and the values are the stored proposal's. + ** + ** Refused (nothing changed, the action stays as it was) when: the person has + ** no assistant access; the action is not that person's (not found); it has + ** expired or is no longer pending; the turn that proposed it is still + ** running; the kill switch is on; the administrator's policy no longer + ** allows the tool; or another request claimed it first. + ** + ** Otherwise the action is claimed (RUNNING) so it can run only once, the + ** registry's tool runs its confirmed write with this person's session (the + ** UI API decides, with the person's permissions as they are now, and + ** refuses a record that is gone), the action is marked EXECUTED or FAILED + ** and the service writes the tool-call log row. Whatever goes wrong after + ** the claim (stored values that cannot be read, a tool that throws) ends in + ** FAILED with a row, never in an action left RUNNING. A confirmed write + ** spends no model budget, so the hard budget does not stop it. *******************************************************************************/ - public AiToolResult confirmationRefusal(QSession session, UiApiToolbox.Context context, String toolCallId, String toolName) + public Confirmation confirm(SignedInPerson person, Integer pendingActionId) throws QException, ConfirmationRefused { - AiToolResult result = new AiToolResult().withToolCallId(toolCallId).withToolName(toolName); - if(session == null || context == null) + if(person == null || person.userId() == null) { - return (denied(result, CHECK_REQUEST, "The confirmation has no signed-in person.")); - } - if(!Objects.equals(session.getUuid(), context.sessionUUID())) - { - return (denied(result, CHECK_REQUEST, "The confirmation's session and its context are not the same person's.")); - } - if(context.policy() == null) - { - return (denied(result, CHECK_REQUEST, "The confirmation has no policy.")); + throw (new ConfirmationRefused(ConfirmationRefused.Reason.NO_ACCESS, "The confirmation has no signed-in person.")); } + QSession session = person.session(); + String userId = person.userId(); try(PersonsContext ignored = new PersonsContext(session)) { - AiToolResult closed = accessOrKillSwitch(session, result); - if(closed != null) + AssistantSecurity.ownerKey(session, runtime.getConfig()); + AiToolResult noAccess = noAccess(session, new AiToolResult()); + if(noAccess != null) { - return (closed); + throw (new ConfirmationRefused(ConfirmationRefused.Reason.NO_ACCESS, noAccess.getModelContent())); } - if(!context.policy().allows(toolName) || !runtime.getPolicyService().current(false).tools().allows(toolName)) + + AssistantPendingAction action = pendingActionId == null ? null : store.getPendingAction(userId, pendingActionId); + if(action == null) { - return (denied(result, CHECK_TOOL_SWITCH, "The administrator has not allowed the " + toolName + " tool.")); + throw (new ConfirmationRefused(ConfirmationRefused.Reason.NOT_FOUND, "Action not found.")); } + Instant now = runtime.getClock().instant(); + if(AssistantPendingAction.STATUS_PENDING.equals(action.getStatus()) && action.getExpiresAt() != null && action.getExpiresAt().isBefore(now)) + { + store.updateFields(AssistantPendingAction.TABLE_NAME, new QRecord().withValue("id", action.getId()).withValue("status", AssistantPendingAction.STATUS_EXPIRED) + .withValue("resultSummary", "Expired before it was confirmed; nothing changed.")); + action.setStatus(AssistantPendingAction.STATUS_EXPIRED); + } + if(!AssistantPendingAction.STATUS_PENDING.equals(action.getStatus())) + { + throw (new ConfirmationRefused(ConfirmationRefused.Reason.NOT_PENDING, "This action is already " + String.valueOf(action.getStatus()).toLowerCase() + ".")); + } + AssistantTurn proposingTurn = action.getTurnId() == null ? null : store.getTurn(userId, action.getTurnId()); + if(proposingTurn != null && ("RUNNING".equals(proposingTurn.getStatus()) || "CANCELLING".equals(proposingTurn.getStatus()))) + { + throw (new ConfirmationRefused(ConfirmationRefused.Reason.TURN_RUNNING, "Wait for the assistant to finish before confirming this action.")); + } + + /////////////////////////////////////////////////////////////////////// + // the kill switch and the policy's switch for this tool refuse a // + // confirmation as they refuse a call: asked before the claim, so a // + // refused action stays pending // + /////////////////////////////////////////////////////////////////////// + PolicyService.ToolPolicy policy; + try + { + PolicyService.EffectivePolicy effective = runtime.getPolicyService().current(true); + if(effective.killSwitch()) + { + throw (new ConfirmationRefused(ConfirmationRefused.Reason.KILL_SWITCH, "The assistant has been turned off by an administrator.")); + } + policy = effective.tools(); + } + catch(QException | RuntimeException e) + { + LOG.warn("Could not read the assistant policy; refusing the confirmation (fail closed)", logPair("errorClass", e.getClass().getSimpleName())); + throw (new ConfirmationRefused(ConfirmationRefused.Reason.KILL_SWITCH, "The assistant's policy could not be read. Try again in a moment.")); + } + if(!policy.allows(action.getToolName())) + { + throw (new ConfirmationRefused(ConfirmationRefused.Reason.NOT_ALLOWED, "The administrator has not allowed the " + action.getToolName() + " tool.")); + } + + /////////////////////////////////////////////////////////////////////// + // claim it first, so a double click cannot run the write twice // + /////////////////////////////////////////////////////////////////////// + store.updateFields(AssistantPendingAction.TABLE_NAME, new QRecord().withValue("id", action.getId()).withValue("status", "RUNNING").withValue("decidedAt", now)); + AssistantPendingAction claimed = store.getPendingAction(userId, action.getId()); + if(claimed == null || !"RUNNING".equals(claimed.getStatus())) + { + throw (new ConfirmationRefused(ConfirmationRefused.Reason.ALREADY_HANDLED, "This action is already being handled.")); + } + + UiApiToolbox.Outcome outcome = runClaimed(person, action, policy); + finishClaimed(action, userId, outcome); + AssistantPendingAction decided = store.getPendingAction(userId, action.getId()); + return (new Confirmation(decided == null ? action : decided, outcome)); } - catch(QException e) - { - LOG.warn("Could not read the assistant policy; refusing the confirmation (fail closed)", logPair("errorClass", e.getClass().getSimpleName())); - return (denied(result, CHECK_KILL_SWITCH, "The assistant's policy could not be read. Try again in a moment.")); - } - return (null); } /******************************************************************************* - ** Run a write the person confirmed, with the confirming request's session, - ** after {@link #confirmationRefusal}'s checks. A confirmation named for - ** another person than its session's is refused; so is a name that proposes - ** no write. + ** Run a claimed action's write as the confirming person. Never throws: a + ** tool that throws, or stored values that cannot be read, is a fixed ERROR + ** outcome (what went wrong is only in the log). *******************************************************************************/ - public UiApiToolbox.Outcome executeConfirmed(ConfirmedToolCall call) + private UiApiToolbox.Outcome runClaimed(SignedInPerson person, AssistantPendingAction action, PolicyService.ToolPolicy policy) { - AiToolResult refused = confirmationRefusal(call.session(), call.context(), call.toolCallId(), call.toolName()); - if(refused == null && !Objects.equals(call.userId(), AssistantSecurity.userId(call.session()))) + try { - refused = denied(new AiToolResult().withToolCallId(call.toolCallId()).withToolName(call.toolName()), CHECK_REQUEST, "The confirmation's session and its user are not the same person."); + Map values = action.getParamsJson() == null ? Map.of() : JsonUtils.toObject(action.getParamsJson(), new TypeReference>() {}); + QSession session = person.session(); + UiApiToolbox.Context context = new UiApiToolbox.Context(session.getUuid(), session.getValue(QSession.VALUE_KEY_USER_TIMEZONE), runtime.getUiApiGateway(), policy, null, + person.credentials(), action.getTurnId() == null ? null : String.valueOf(action.getTurnId())); + AssistantTool tool = registry.find(action.getToolName()); + UiApiToolbox.Outcome outcome = tool == null ? UiApiToolbox.notAWriteTool(action.getToolCallId(), action.getToolName(), action.getTableName()) + : tool.executeConfirmed(new ConfirmedToolCall(session, context, action.getToolCallId(), action.getToolName(), action.getTableName(), action.getRecordId(), values, person.userId())); + if(outcome != null && outcome.getResult() != null) + { + return (outcome); + } + LOG.warn("A confirmed assistant action's tool gave no result", logPair("pendingActionId", action.getId()), logPair("tool", action.getToolName())); } - if(refused != null) + catch(Exception e) { - return (UiApiToolbox.Outcome.of(refused.withTableName(call.tableName()), null, null, null)); + LOG.warn("A confirmed assistant action threw", logPair("pendingActionId", action.getId()), logPair("tool", action.getToolName()), logPair("errorClass", e.getClass().getSimpleName())); } - - AssistantTool tool = registry.find(call.toolName()); - if(tool == null) - { - return (UiApiToolbox.notAWriteTool(call.toolCallId(), call.toolName(), call.tableName())); - } - return (tool.executeConfirmed(call)); + AiToolResult failed = new AiToolResult().withToolCallId(action.getToolCallId()).withToolName(action.getToolName()).withTableName(action.getTableName()) + .withOutcome(AiToolOutcome.ERROR).withSummary("Failed").withModelContent("The change failed unexpectedly. Check the record before trying again."); + failed.setDurationMs(0L); + return (UiApiToolbox.Outcome.of(failed, null, null, null)); } /******************************************************************************* - ** The tool-call log row of a call: of {@link #execute}'s outcome, or of the - ** result a caller gave instead of running or finishing the call (its own - ** time limit, a stop). The service takes the person, the turn, the - ** conversation and the channel from the request and its scope, and redacts - ** the input itself; the caller passes nothing it has prepared. + ** Mark a claimed action EXECUTED or FAILED and write its tool-call log row. + ** The row is written even when the mark fails. A confirmed write may have + ** changed data, so a mark or a row that cannot be written is an ERROR in + ** the log for the operator, never only a warning; the person still gets the + ** outcome of their change. *******************************************************************************/ - public void log(ToolCallRequest request, ToolCallOutcome outcome) throws QException + private void finishClaimed(AssistantPendingAction action, String userId, UiApiToolbox.Outcome outcome) { - AssistantTool tool = registry.find(request.toolName()); - AiToolResult result = outcome.result(); - UiApiToolbox.Outcome extras = outcome.extras(); - ToolCallScope scope = request.scope(); - Map input = tool == null ? request.input() : tool.inputForLog(request.input(), result); - String inputJson = INPUT_REDACTOR.redact(JsonUtils.toJson(input), new HashSet<>()); - try(PersonsContext ignored = new PersonsContext(request.session())) + AiToolResult result = outcome.getResult(); + boolean ok = result.getOutcome() == AiToolOutcome.OK; + try { + store.updateFields(AssistantPendingAction.TABLE_NAME, new QRecord() + .withValue("id", action.getId()) + .withValue("status", ok ? AssistantPendingAction.STATUS_EXECUTED : AssistantPendingAction.STATUS_FAILED) + .withValue("httpStatus", outcome.getHttpStatus()) + .withValue("resultSummary", truncate(result.getSummary(), 500))); + } + catch(QException | RuntimeException e) + { + LOG.error("A confirmed assistant action ran but its status could not be recorded; it is left RUNNING", logPair("pendingActionId", action.getId()), + logPair("tool", action.getToolName()), logPair("outcome", outcomeName(result)), logPair("errorClass", e.getClass().getSimpleName())); + } + finally + { + try + { + store.insert(new AssistantToolCall() + .withTurnId(action.getTurnId()) + .withClientChannel(CHANNEL + (action.getTurnId() == null ? "" : "/" + action.getTurnId())) + .withConversationId(action.getConversationId()) + .withUserId(userId) + .withToolCallId(action.getToolCallId()) + .withToolName(action.getToolName()) + .withInputJson(truncate(INPUT_REDACTOR.redact(action.getParamsJson() == null ? "{}" : action.getParamsJson(), new HashSet<>()), 4000)) + .withOutcome(outcomeName(result)) + .withPermissionCheck(result.getPermissionCheck()) + .withTableName(action.getTableName()) + .withRowCount(result.getRowCount()) + .withDurationMs(result.getDurationMs()) + .withResultSummary(truncate("Confirmed: " + result.getSummary(), 500)) + .withResultJson(result.getModelContent()) + .withUiApiOperation(outcome.getOperation()) + .withHttpStatus(outcome.getHttpStatus()) + .withAuditHref(outcome.getAuditHref()) + .withPendingActionId(action.getId())); + } + catch(QException | RuntimeException e) + { + LOG.error("A confirmed assistant action ran but its tool-call log row could not be written", logPair("pendingActionId", action.getId()), + logPair("tool", action.getToolName()), logPair("outcome", outcomeName(result)), logPair("errorClass", e.getClass().getSimpleName())); + } + } + } + + + + /******************************************************************************* + ** The tool-call log row of a call, for the turn engine: of + ** {@link #execute}'s outcome, or of the result the engine gave instead of + ** running or finishing the call (its own time limit, a stop). The service + ** takes the person from the call, checks the scope's conversation and turn + ** are theirs (a row never names another person's), works out the channel + ** and redacts the input itself; the caller passes nothing it has prepared. + ** + ** It never throws, so one row that cannot be written does not cost the + ** rows after it: the answer says whether the row was written. A row that + ** could not be written is reported here, as an ERROR when the call left + ** something behind (a proposed write, an audited change). + *******************************************************************************/ + public boolean log(ToolCall call, ToolCallScope scope, ToolCallOutcome outcome) + { + try + { + writeRow(call, scope, outcome); + return (true); + } + catch(QException | RuntimeException e) + { + reportUnlogged(call, scope, outcome, e); + return (false); + } + } + + + + /******************************************************************************* + ** Write the tool-call log row of a call, as its person. + *******************************************************************************/ + private void writeRow(ToolCall call, ToolCallScope scope, ToolCallOutcome outcome) throws QException + { + AssistantTool tool = registry.find(call.toolName()); + AiToolResult result = outcome.result(); + UiApiToolbox.Outcome extras = outcome.extras(); + QSession session = call.person().session(); + try(PersonsContext ignored = new PersonsContext(session)) + { + AssistantSecurity.ownerKey(session, runtime.getConfig()); + Admission admission = admission(call.person(), scope); + boolean owned; + String channel; + synchronized(admission) + { + owned = admission.owned; + channel = admission.channel; + } + String inputJson = INPUT_REDACTOR.redact(JsonUtils.toJson(inputForLog(tool, call, result)), new HashSet<>()); store.insert(new AssistantToolCall() - .withTurnId(scope == null ? null : scope.turnId()) - .withConversationId(scope == null ? null : scope.conversationId()) - .withUserId(AssistantSecurity.userId(request.session())) - .withToolCallId(request.toolCallId()) - .withToolName(request.toolName()) - .withClientChannel(scope == null ? "assistant" : scope.clientChannel()) + .withTurnId(owned ? scope.turnId() : null) + .withConversationId(owned ? scope.conversationId() : null) + .withUserId(call.person().userId()) + .withToolCallId(call.toolCallId()) + .withToolName(call.toolName()) + .withClientChannel(channel) .withInputJson(tool == null ? inputJson : tool.forLog(inputJson)) .withOutcome(outcomeName(result)) .withPermissionCheck(result.getPermissionCheck()) @@ -550,7 +1010,7 @@ public class ToolCallService .withWidgetName(result.getWidgetName()) .withRowCount(result.getRowCount()) .withDurationMs(result.getDurationMs()) - .withResultSummary(summary(request.toolName(), result)) + .withResultSummary(summary(call.toolName(), result)) .withResultJson(tool == null ? result.getModelContent() : tool.forLog(result.getModelContent())) .withRedactionsJson(result.getRedactions() == null || result.getRedactions().isEmpty() ? null : JsonUtils.toJson(result.getRedactions())) .withUiApiOperation(extras == null ? (tool == null ? null : tool.operationWithoutDetail()) : extras.getOperation()) @@ -562,6 +1022,54 @@ public class ToolCallService + /******************************************************************************* + ** What the row keeps as the call's input, before it is redacted. A tool + ** that throws while saying what to keep costs the input, not the row. + *******************************************************************************/ + private static Map inputForLog(AssistantTool tool, ToolCall call, AiToolResult result) + { + if(tool == null) + { + return (call.input() == null ? Map.of() : call.input()); + } + try + { + Map input = tool.inputForLog(call.input(), result); + return (input == null ? Map.of() : input); + } + catch(RuntimeException e) + { + LOG.warn("A tool could not say what its log row keeps as input; the row keeps none", logPair("tool", call.toolName()), logPair("errorClass", e.getClass().getSimpleName())); + return (Map.of()); + } + } + + + + /******************************************************************************* + ** Say that a call has no log row. A call that left something behind (a + ** proposed write, an audited change) is an ERROR for the operator; a call + ** that changed nothing is a warning. + *******************************************************************************/ + private static void reportUnlogged(ToolCall call, ToolCallScope scope, ToolCallOutcome outcome, Exception e) + { + UiApiToolbox.Outcome extras = outcome == null ? null : outcome.extras(); + boolean changed = extras != null && (extras.getPendingActionId() != null || extras.getAuditHref() != null); + String message = changed ? "An assistant tool call that changed data has no tool-call log row: the row could not be written" : "Could not record assistant tool call"; + if(changed) + { + LOG.error(message, logPair("tool", call.toolName()), logPair("toolCallId", call.toolCallId()), logPair("turnId", scope == null ? null : scope.turnId()), + logPair("pendingActionId", extras.getPendingActionId()), logPair("audited", extras.getAuditHref() != null), logPair("errorClass", e.getClass().getSimpleName())); + } + else + { + LOG.warn(message, logPair("tool", call.toolName()), logPair("toolCallId", call.toolCallId()), logPair("turnId", scope == null ? null : scope.turnId()), + logPair("errorClass", e.getClass().getSimpleName())); + } + } + + + /******************************************************************************* ** The outcome's name as the log and the panel show it (none is an ERROR). *******************************************************************************/ @@ -582,36 +1090,6 @@ public class ToolCallService - /******************************************************************************* - ** The tool-call log row of a confirmed write: the pending action's call, - ** run by the person who confirmed it. - *******************************************************************************/ - public void logConfirmed(AssistantPendingAction action, String userId, UiApiToolbox.Outcome outcome) throws QException - { - AiToolResult result = outcome.getResult(); - store.insert(new AssistantToolCall() - .withTurnId(action.getTurnId()) - .withClientChannel("assistant" + (action.getTurnId() == null ? "" : "/" + action.getTurnId())) - .withConversationId(action.getConversationId()) - .withUserId(userId) - .withToolCallId(action.getToolCallId()) - .withToolName(action.getToolName()) - .withInputJson(truncate(INPUT_REDACTOR.redact(action.getParamsJson() == null ? "{}" : action.getParamsJson(), new HashSet<>()), 4000)) - .withOutcome(outcomeName(result)) - .withPermissionCheck(result.getPermissionCheck()) - .withTableName(action.getTableName()) - .withRowCount(result.getRowCount()) - .withDurationMs(result.getDurationMs()) - .withResultSummary(truncate("Confirmed: " + result.getSummary(), 500)) - .withResultJson(result.getModelContent()) - .withUiApiOperation(outcome.getOperation()) - .withHttpStatus(outcome.getHttpStatus()) - .withAuditHref(outcome.getAuditHref()) - .withPendingActionId(action.getId())); - } - - - /******************************************************************************* ** Getter for registry *******************************************************************************/ diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/TurnEngine.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/TurnEngine.java index da5acdb..97db8ea 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/TurnEngine.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/TurnEngine.java @@ -39,11 +39,13 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicIntegerArray; import java.util.concurrent.atomic.AtomicLongArray; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; import com.kingsrook.qbits.assistant.AssistantQBitConfig; import com.kingsrook.qbits.assistant.AssistantRuntime; +import com.kingsrook.qbits.assistant.SignedInPerson; import com.kingsrook.qbits.assistant.attachments.AttachmentService; import com.kingsrook.qbits.assistant.model.AssistantConversation; import com.kingsrook.qbits.assistant.model.AssistantMessage; @@ -67,8 +69,8 @@ import com.kingsrook.qbits.assistant.spi.ModelUsage; import com.kingsrook.qbits.assistant.spi.StreamHandle; import com.kingsrook.qbits.assistant.tools.PresentResultsTool; import com.kingsrook.qbits.assistant.tools.UiApiToolbox; +import com.kingsrook.qbits.assistant.tools.registry.ToolCall; import com.kingsrook.qbits.assistant.tools.registry.ToolCallOutcome; -import com.kingsrook.qbits.assistant.tools.registry.ToolCallRequest; import com.kingsrook.qbits.assistant.tools.registry.ToolCallScope; import com.kingsrook.qqq.backend.core.actions.ai.AiRecordRef; import com.kingsrook.qqq.backend.core.actions.ai.AiToolCall; @@ -83,7 +85,6 @@ import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.session.QSession; import com.kingsrook.qqq.backend.core.utils.JsonUtils; import com.kingsrook.qqq.backend.core.utils.StringUtils; -import com.kingsrook.qqq.middleware.javalin.specs.invoke.UiApiCredentials; import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; @@ -119,6 +120,13 @@ public class TurnEngine implements Runnable private static final int TEXT_FLUSH_CHARS = 48; private static final long TEXT_FLUSH_MILLIS = 120; + /////////////////////////////////////////////////////////////////////////// + // how long a batch waits, in all, for tool calls it stopped waiting for // + // (a time limit, a stop) to say what they did, so their log rows are of // + // the real outcome // + /////////////////////////////////////////////////////////////////////////// + private static final long CUT_SHORT_GRACE_MILLIS = 1000; + private static final Set ERROR_OUTCOMES = Set.of(AiToolOutcome.DENIED, AiToolOutcome.NOT_FOUND, AiToolOutcome.INVALID_INPUT, AiToolOutcome.ERROR); private final ToolCallScope toolScope = new TurnToolScope(); @@ -176,7 +184,7 @@ public class TurnEngine implements Runnable /////////////////////////////////////////////////////////////////////////// private final AtomicReference housekeeper = new AtomicReference<>(); private volatile long housekeeperStartedNanos; - private UiApiCredentials credentials; + private SignedInPerson person; private volatile TranscriptRedactor transcript; private boolean engineRunning; @@ -202,6 +210,7 @@ public class TurnEngine implements Runnable this.policy = policy; this.running = running; this.softBudgetNotice = softBudgetNotice; + this.person = SignedInPerson.ofSession(session); this.startedAt = turn.getStartedAt() == null ? runtime.getClock().instant() : turn.getStartedAt(); this.deadline = startedAt.plusSeconds(config.getMaxTurnSeconds()); running.setStartedAt(startedAt); @@ -211,13 +220,18 @@ public class TurnEngine implements Runnable /******************************************************************************* - ** The credentials of the request that started the turn, forwarded to the - ** UI API by the tools (session cookies, Authorization, API key as they came - ** in). Without them the session UUID is used. Never logged. + ** The signed-in person the turn runs for, as the route's session setup + ** bound them: this turn's session with the credentials of the request that + ** started it, which the tools forward to the UI API. Without it the turn + ** runs with the session alone. A person of another session is refused. *******************************************************************************/ - public TurnEngine withCredentials(UiApiCredentials credentials) + public TurnEngine withPerson(SignedInPerson person) { - this.credentials = credentials; + if(person == null || person.session() != session) + { + throw (new IllegalArgumentException("The turn's person must be the person of the turn's session.")); + } + this.person = person; return (this); } @@ -1051,20 +1065,28 @@ public class TurnEngine implements Runnable ** each one (a tool that is not offered is refused there) and runs it as the ** user. The limits of a turn are this engine's: how many calls, how many at ** once, how long each may take. + ** + ** Every call that was started has its tool-call log row when this method + ** is left, however it is left: a call that ran past its time limit, one + ** whose result could not be shown, and a batch the engine was interrupted + ** in (the rows are written, then the interruption is passed on). The row + ** is of what the call in fact did when its worker has finished. *******************************************************************************/ private ToolBatch executeTools(List toolUses) throws QException, InterruptedException { - int allowed = Math.max(0, config.getMaxToolCallsPerTurn() - toolCallCount); - boolean limitReached = false; - AiToolResult[] results = new AiToolResult[toolUses.size()]; - UiApiToolbox.Outcome[] outcomes = new UiApiToolbox.Outcome[toolUses.size()]; - ToolCallRequest[] requests = new ToolCallRequest[toolUses.size()]; - List> futures = new ArrayList<>(); - List futureIndexes = new ArrayList<>(); - AtomicLongArray startedNanos = new AtomicLongArray(toolUses.size()); - boolean[] failed = new boolean[toolUses.size()]; - boolean[] timedOut = new boolean[toolUses.size()]; - Semaphore permits = new Semaphore(Math.max(1, config.getMaxParallelToolCalls())); + int allowed = Math.max(0, config.getMaxToolCallsPerTurn() - toolCallCount); + boolean limitReached = false; + AiToolResult[] results = new AiToolResult[toolUses.size()]; + UiApiToolbox.Outcome[] outcomes = new UiApiToolbox.Outcome[toolUses.size()]; + ToolCall[] calls = new ToolCall[toolUses.size()]; + List> futures = new ArrayList<>(); + List futureIndexes = new ArrayList<>(); + AtomicLongArray startedNanos = new AtomicLongArray(toolUses.size()); + AtomicIntegerArray begun = new AtomicIntegerArray(toolUses.size()); + CountDownLatch[] finished = new CountDownLatch[toolUses.size()]; + boolean[] failed = new boolean[toolUses.size()]; + boolean[] timedOut = new boolean[toolUses.size()]; + Semaphore permits = new Semaphore(Math.max(1, config.getMaxParallelToolCalls())); for(int i = 0; i < toolUses.size(); i++) { @@ -1079,74 +1101,96 @@ public class TurnEngine implements Runnable phase("tool", use.getToolName()); emit(TurnEvent.TOOL_START, map("messageId", assistantMessage.getId(), "toolCallId", use.getToolUseId(), "name", use.getToolName(), "summary", ToolLabels.running(use.getToolName()))); - requests[i] = new ToolCallRequest(session, toolContext(), use.getToolUseId(), use.getToolName(), use.getInput(), toolScope); + calls[i] = new ToolCall(person, use.getToolUseId(), use.getToolName(), use.getInput()); + finished[i] = new CountDownLatch(1); final int index = i; AiToolCall call = new AiToolCall().withId(use.getToolUseId()).withName(use.getToolName()).withInput(use.getInput()); futureIndexes.add(index); futures.add(runtime.getExecutor().submit(() -> { - if(!transcript.retainWorker()) - { - return; - } - transcript.attach(); + begun.set(index, 1); try { - QContext.init(runtime.getQInstance(), session); - permits.acquire(); - startedNanos.set(index, System.nanoTime()); - LOG.info("Assistant tool call started", logPair("turnId", turn.getId()), logPair("tool", call.getName())); + if(!transcript.retainWorker()) + { + return; + } + transcript.attach(); try { - if(!running.isCancelled()) + QContext.init(runtime.getQInstance(), session); + permits.acquire(); + startedNanos.set(index, System.nanoTime()); + LOG.info("Assistant tool call started", logPair("turnId", turn.getId()), logPair("tool", call.getName())); + try { - /////////////////////////////////////////////////////////////// - // the service checks the call, then the registry's tool // - // runs it, as the user: with the user's own session, policy // - // and pending-action store // - /////////////////////////////////////////////////////////////// - ToolCallOutcome done = runtime.getToolCalls().execute(requests[index]); - outcomes[index] = done.extras(); - results[index] = done.result(); + if(!running.isCancelled()) + { + /////////////////////////////////////////////////////////////// + // the service checks the call, then the registry's tool // + // runs it, as the user: with the user's own session, the // + // policy the service read and its pending-action store // + /////////////////////////////////////////////////////////////// + ToolCallOutcome done = runtime.getToolCalls().execute(calls[index], toolScope); + outcomes[index] = done.extras(); + results[index] = done.result(); + } } + finally + { + permits.release(); + LOG.info("Assistant tool call finished", logPair("turnId", turn.getId()), logPair("tool", call.getName()), + logPair("outcome", results[index] == null || results[index].getOutcome() == null ? null : results[index].getOutcome().name()), + logPair("durationMs", TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startedNanos.get(index)))); + } + } + catch(InterruptedException e) + { + Thread.currentThread().interrupt(); + } + catch(RuntimeException | Error e) + { + /////////////////////////////////////////////////////////////////// + // internal details go to the log only; the model and the user // + // get a fixed, safe text // + /////////////////////////////////////////////////////////////////// + LOG.warn("Tool kernel threw", hipaaSafe(e), logPair("turnId", turn.getId()), logPair("tool", call.getName()), logPair("errorClass", e.getClass().getSimpleName())); + failed[index] = true; + results[index] = failedResult(call.getId(), call.getName()); } finally { - permits.release(); - LOG.info("Assistant tool call finished", logPair("turnId", turn.getId()), logPair("tool", call.getName()), - logPair("outcome", results[index] == null || results[index].getOutcome() == null ? null : results[index].getOutcome().name()), - logPair("durationMs", TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startedNanos.get(index)))); + transcript.releaseWorker(); + TranscriptRedactor.detach(); + QContext.clear(); } } - catch(InterruptedException e) - { - Thread.currentThread().interrupt(); - } - catch(RuntimeException | Error e) - { - /////////////////////////////////////////////////////////////////// - // internal details go to the log only; the model and the user // - // get a fixed, safe text // - /////////////////////////////////////////////////////////////////// - LOG.warn("Tool kernel threw", hipaaSafe(e), logPair("turnId", turn.getId()), logPair("tool", call.getName()), logPair("errorClass", e.getClass().getSimpleName())); - failed[index] = true; - results[index] = failedResult(call.getId(), call.getName()); - } finally { - transcript.releaseWorker(); - TranscriptRedactor.detach(); - QContext.clear(); + /////////////////////////////////////////////////////////////// + // whatever the worker did is now readable by the engine // + /////////////////////////////////////////////////////////////// + finished[index].countDown(); } })); } persistPendingEvents(); + InterruptedException interrupted = null; long toolTimeoutNanos = TimeUnit.SECONDS.toNanos(config.getToolCallTimeoutSeconds() == null ? 60 : config.getToolCallTimeoutSeconds()); for(int f = 0; f < futures.size(); f++) { Future future = futures.get(f); int index = futureIndexes.get(f); + if(interrupted != null) + { + //////////////////////////////////////////////////////////////// + // the engine was told to stop: the calls it has not waited // + // for yet are stopped too // + //////////////////////////////////////////////////////////////// + future.cancel(true); + continue; + } /////////////////////////////////////////////////////////////////////// // each call has its own limit (from when it got a permit), and no // // call outlives the turn's deadline // @@ -1176,6 +1220,41 @@ public class TurnEngine implements Runnable { LOG.debug("tool future cancelled", logPair("turnId", turn.getId())); } + catch(InterruptedException e) + { + //////////////////////////////////////////////////////////////// + // not passed on yet: the calls that ran still get their rows // + //////////////////////////////////////////////////////////////// + interrupted = e; + future.cancel(true); + LOG.info("Assistant turn interrupted while its tool calls ran", logPair("turnId", turn.getId())); + } + } + + /////////////////////////////////////////////////////////////////////// + // a call that was cut short (its time limit, a stop) has been // + // interrupted; give it a bounded moment to hand back what it did, // + // so its row can say so // + /////////////////////////////////////////////////////////////////////// + long graceUntil = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(CUT_SHORT_GRACE_MILLIS); + for(int index : futureIndexes) + { + if(begun.get(index) == 0) + { + continue; + } + try + { + if(!finished[index].await(Math.max(0, graceUntil - System.nanoTime()), TimeUnit.NANOSECONDS)) + { + LOG.warn("Assistant tool call did not stop when it was cut short; its log row cannot say what it did", logPair("turnId", turn.getId()), logPair("tool", toolUses.get(index).getToolName())); + } + } + catch(InterruptedException e) + { + interrupted = interrupted == null ? e : interrupted; + break; + } } List modelResults = new ArrayList<>(); @@ -1188,33 +1267,72 @@ public class TurnEngine implements Runnable continue; } - AiToolResult result = timedOut[i] ? null : results[i]; - UiApiToolbox.Outcome extras = timedOut[i] ? null : outcomes[i]; + /////////////////////////////////////////////////////////////////////// + // what the worker in fact gave back, if it has finished // + /////////////////////////////////////////////////////////////////////// + boolean workerFinished = finished[i].getCount() == 0; + AiToolResult real = workerFinished ? results[i] : null; + UiApiToolbox.Outcome realExtras = workerFinished ? outcomes[i] : null; + boolean cutShort = timedOut[i] || (interrupted != null && real == null); + + AiToolResult result = timedOut[i] ? null : real; + UiApiToolbox.Outcome extras = timedOut[i] ? null : realExtras; if(result == null && failed[i]) { result = failedResult(use.getToolUseId(), use.getToolName()); } else if(result == null) { - boolean stopped = running.isCancelled() && !timedOut[i]; + boolean stopped = (running.isCancelled() || interrupted != null) && !timedOut[i]; result = new AiToolResult().withToolCallId(use.getToolUseId()).withToolName(use.getToolName()).withOutcome(AiToolOutcome.ERROR) .withSummary(stopped ? "Not run (stopped)" : "Timed out") .withModelContent(stopped ? "Not run: the user stopped the answer." : "The tool did not finish in time. Answer with what you have, or try a narrower request."); timedOut[i] = !stopped; } - classification.observe(use, result); - recordToolResult(use, requests[i], result, extras); - if(timedOut[i]) - { - notice("error", "A step took too long and was stopped (" + ToolLabels.running(use.getToolName()) + ")."); - } - else if(failed[i] || (extras != null && extras.isFailed())) - { - notice("error", "A step failed (" + ToolLabels.running(use.getToolName()) + ")."); - } - String content = result.getModelContent() == null ? "" : result.getModelContent(); - modelResults.add(ModelPart.toolResult(use.getToolUseId(), content, result.getOutcome() != null && ERROR_OUTCOMES.contains(result.getOutcome()))); + /////////////////////////////////////////////////////////////////////// + // the row: the call's real outcome when it finished after all (a // + // call that ran and then ran out of time), and in any case what it // + // left behind (the operation, a proposed write, an audited change); // + // the model and the panel were told it took too long // + /////////////////////////////////////////////////////////////////////// + AiToolResult rowResult = cutShort && real != null && AiToolOutcome.OK.equals(real.getOutcome()) ? real : result; + UiApiToolbox.Outcome rowExtras = cutShort ? realExtras : extras; + + ModelPart part = null; + try + { + if(interrupted == null) + { + part = showToolResult(use, result, timedOut[i], failed[i] || (extras != null && extras.isFailed())); + } + } + catch(RuntimeException e) + { + //////////////////////////////////////////////////////////////////// + // a result that could not be classified or shown is not sent to // + // the model; the call still has its row, and so have the next // + //////////////////////////////////////////////////////////////////// + LOG.warn("Assistant tool result could not be processed", hipaaSafe(e), logPair("turnId", turn.getId()), logPair("tool", use.getToolName()), logPair("errorClass", e.getClass().getSimpleName())); + part = ModelPart.toolResult(use.getToolUseId(), failedResult(use.getToolUseId(), use.getToolName()).getModelContent(), true); + } + finally + { + ////////////////////////////////////////////////////////////////////// + // the service writes the row: it redacts the input itself, takes // + // the person from the call and checks the turn is theirs. It does // + // not throw, and reports a row it could not write // + ////////////////////////////////////////////////////////////////////// + runtime.getToolCalls().log(calls[i], toolScope, new ToolCallOutcome(rowResult, rowExtras)); + } + if(part != null) + { + modelResults.add(part); + } + } + if(interrupted != null) + { + throw (interrupted); } persistClassification(); persistPendingEvents(); @@ -1223,6 +1341,30 @@ public class TurnEngine implements Runnable + /******************************************************************************* + ** Take a tool result into the turn: its classification, its events and + ** message parts, a notice when the step failed or took too long; the answer + ** is what goes back to the model. + *******************************************************************************/ + private ModelPart showToolResult(ModelPart use, AiToolResult result, boolean tookTooLong, boolean stepFailed) + { + classification.observe(use, result); + recordToolResult(use, result); + if(tookTooLong) + { + notice("error", "A step took too long and was stopped (" + ToolLabels.running(use.getToolName()) + ")."); + } + else if(stepFailed) + { + notice("error", "A step failed (" + ToolLabels.running(use.getToolName()) + ")."); + } + + String content = result.getModelContent() == null ? "" : result.getModelContent(); + return (ModelPart.toolResult(use.getToolUseId(), content, result.getOutcome() != null && ERROR_OUTCOMES.contains(result.getOutcome()))); + } + + + /******************************************************************************* ** The result of a tool call that threw: an ERROR (is_error) result with a ** fixed text; what went wrong is only in the log. @@ -1236,11 +1378,12 @@ public class TurnEngine implements Runnable /******************************************************************************* - ** Audit row, events and message parts of one tool result. The render is - ** checked first (RenderShapes): one that cannot be shown is left out and an - ** error notice says so. + ** Events and message parts of one tool result (its audit row is written by + ** executeTools, whatever happens here). The render is checked first + ** (RenderShapes): one that cannot be shown is left out and an error notice + ** says so. *******************************************************************************/ - private void recordToolResult(ModelPart use, ToolCallRequest request, AiToolResult result, UiApiToolbox.Outcome extras) + private void recordToolResult(ModelPart use, AiToolResult result) { String outcome = ToolCallService.outcomeName(result); String summary = ToolCallService.summary(use.getToolName(), result); @@ -1299,19 +1442,6 @@ public class TurnEngine implements Runnable emit(TurnEvent.UI_NAVIGATE, map("messageId", assistantMessage.getId(), "href", href)); parts.addNavigate(href); } - - try - { - ////////////////////////////////////////////////////////////////////// - // the service writes the row: it redacts the input itself and // - // takes the person, the turn and the channel from the request // - ////////////////////////////////////////////////////////////////////// - runtime.getToolCalls().log(request, new ToolCallOutcome(result, extras)); - } - catch(QException e) - { - LOG.warn("Could not record assistant tool call", hipaaSafe(e), logPair("turnId", turn.getId()), logPair("tool", use.getToolName())); - } } @@ -1360,18 +1490,6 @@ public class TurnEngine implements Runnable - /******************************************************************************* - ** The channel this turn's requests carry: the turn's, or the scheduled - ** task's. - *******************************************************************************/ - @Override - public String clientChannel() - { - return (scheduledTask == null ? "assistant/" + turn.getId() : "assistant/scheduled/" + scheduledTask.getId()); - } - - - /******************************************************************************* ** The X-QQQ-AI-Route the bulk-load tools send. *******************************************************************************/ @@ -1395,51 +1513,6 @@ public class TurnEngine implements Runnable - /******************************************************************************* - ** The UI API tool context: the user's session, the gateway, the policy's - ** tools and the pending-action hook. - *******************************************************************************/ - private UiApiToolbox.Context toolContext() - { - return (new UiApiToolbox.Context(session.getUuid(), userTimezone(), runtime.getUiApiGateway(), toolPolicy, this::proposeWrite, credentials, turn.getId() == null ? null : String.valueOf(turn.getId()))); - } - - - - /******************************************************************************* - ** The browser's timezone the session carries, if any. - *******************************************************************************/ - private String userTimezone() - { - return (session.getValue(QSession.VALUE_KEY_USER_TIMEZONE)); - } - - - - /******************************************************************************* - ** Record a write the user must confirm; nothing runs until they do. - *******************************************************************************/ - private Integer proposeWrite(UiApiToolbox.ProposedWrite write) throws QException - { - Instant now = runtime.getClock().instant(); - AssistantPendingAction action = store.insert(new AssistantPendingAction() - .withConversationId(conversation.getId()) - .withTurnId(turn.getId()) - .withUserId(userId) - .withToolCallId(write.toolCallId()) - .withToolName(write.toolName()) - .withUiApiOperation(write.operation()) - .withTableName(write.tableName()) - .withRecordId(write.recordId()) - .withParamsJson(JsonUtils.toJson(write.values())) - .withSummary(write.summary()) - .withStatus(AssistantPendingAction.STATUS_PENDING) - .withExpiresAt(now.plusSeconds(config.getPendingActionTtlSeconds() == null ? 900 : config.getPendingActionTtlSeconds()))); - return (action.getId()); - } - - - /******************************************************************************* ** Only same-origin application links are passed on. *******************************************************************************/ diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/home/AssistantSetupStatus.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/home/AssistantSetupStatus.java index 932c3ec..79f191c 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/home/AssistantSetupStatus.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/home/AssistantSetupStatus.java @@ -142,7 +142,7 @@ public final class AssistantSetupStatus } else { - items.add(new Item("Policy", AssistantPolicy.TABLE_NAME, State.OK, "Writes " + (Boolean.FALSE.equals(policies.get(0).getWritesRequireConfirmation()) ? "do not wait" : "wait") + " for confirmation.", null)); + items.add(new Item("Policy", AssistantPolicy.TABLE_NAME, State.OK, "Writes wait for confirmation.", null)); } //////////////////////// diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/metadata/AssistantRecordScreens.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/metadata/AssistantRecordScreens.java index 070b005..36387d7 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/metadata/AssistantRecordScreens.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/metadata/AssistantRecordScreens.java @@ -731,7 +731,7 @@ public final class AssistantRecordScreens help(table, "mainModel", "The model id answers come from; empty uses the model configured for the application."); help(table, "killSwitch", "On: the assistant takes no new requests from anyone."); help(table, "killSwitchReason", "Shown to users while the kill switch is on."); - help(table, "writesRequireConfirmation", "On (or empty): every create, update or delete the assistant proposes waits for the user to confirm it in the panel. Off: writes run at once, still with the user's own permissions."); + help(table, "writesRequireConfirmation", "No longer used. Every create, update or delete the assistant proposes waits for the user to confirm it in the panel; this cannot be switched off."); help(table, "allowedTools", "Comma-separated tool names the assistant may use (e.g. list_tables, query_records, count_records). Empty allows every tool."); help(table, "maxRowsPerCall", "Most rows one tool call returns to the model. Empty: 50; at most 200."); help(table, "redactPii", "On (or empty): API keys, tokens, private keys, card numbers and social security numbers are redacted from tool results before the model sees them."); diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/model/AssistantPolicy.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/model/AssistantPolicy.java index eb146bc..368e326 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/model/AssistantPolicy.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/model/AssistantPolicy.java @@ -617,7 +617,7 @@ public class AssistantPolicy extends QRecordEntity /******************************************************************************* - ** Getter for writesRequireConfirmation (whether every write waits for the user to confirm a card (default true)) + ** Getter for writesRequireConfirmation (no longer read: every write always waits for the user to confirm a card; the column stays) *******************************************************************************/ public Boolean getWritesRequireConfirmation() { diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/rag/RagService.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/rag/RagService.java index 0f256b6..bb3f3ca 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/rag/RagService.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/rag/RagService.java @@ -145,8 +145,8 @@ public class RagService { PolicyService.ToolPolicy policy = context.policy(); UiApiToolbox.Context internal = new UiApiToolbox.Context(context.sessionUUID(), context.userTimezone(), context.gateway(), - new PolicyService.ToolPolicy(true, Set.of(), policy == null ? 50 : policy.maxRowsPerCall(), policy == null || policy.redactPii()), null, context.credentials(), context.turnId()); - return (reader.execute(internal, "knowledge-read", UiApiToolbox.GET_RECORD, Map.of("table", table, "id", pk))); + new PolicyService.ToolPolicy(Set.of(), policy == null ? 50 : policy.maxRowsPerCall(), policy == null || policy.redactPii()), null, context.credentials(), context.turnId()); + return (reader.read(internal, "knowledge-read", UiApiToolbox.GET_RECORD, Map.of("table", table, "id", pk))); } diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/scheduled/ScheduledTaskRunner.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/scheduled/ScheduledTaskRunner.java index f6d64a0..2a5b922 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/scheduled/ScheduledTaskRunner.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/scheduled/ScheduledTaskRunner.java @@ -24,6 +24,7 @@ package com.kingsrook.qbits.assistant.scheduled; import java.time.Instant; import java.util.Map; import com.kingsrook.qbits.assistant.AssistantRuntime; +import com.kingsrook.qbits.assistant.SignedInPerson; import com.kingsrook.qbits.assistant.api.AssistantApi; import com.kingsrook.qbits.assistant.api.AssistantApiException; import com.kingsrook.qbits.assistant.engine.MessageParts; @@ -40,7 +41,6 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterOrderBy; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.session.QSession; -import com.kingsrook.qqq.middleware.javalin.specs.invoke.UiApiCredentials; /******************************************************************************* @@ -119,7 +119,7 @@ public class ScheduledTaskRunner { session = login.login(runtime, task); QContext.init(runtime.getQInstance(), session); - AssistantApi api = new AssistantApi(runtime, session).withCredentials(UiApiCredentials.ofSessionUUID(session.getUuid())); + AssistantApi api = new AssistantApi(runtime, SignedInPerson.ofSession(session)); Integer conversationId = ((Number) api.createConversation(Map.of("title", task.getName())).get("id")).intValue(); run.setConversationId(conversationId); store.updateFields(AssistantConversation.TABLE_NAME, new QRecord().withValue("id", conversationId) diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/scheduled/ScheduledTurn.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/scheduled/ScheduledTurn.java index 6e82463..cc9b777 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/scheduled/ScheduledTurn.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/scheduled/ScheduledTurn.java @@ -110,7 +110,7 @@ public class ScheduledTurn { allowed = Set.of("__no_tools__"); } - return (new PolicyService.ToolPolicy(true, allowed, policy.maxRowsPerCall(), policy.redactPii())); + return (new PolicyService.ToolPolicy(allowed, policy.maxRowsPerCall(), policy.redactPii())); } diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/KnowledgeTool.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/KnowledgeTool.java index f0ae9da..659db60 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/KnowledgeTool.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/KnowledgeTool.java @@ -127,7 +127,7 @@ public class KnowledgeTool /******************************************************************************* ** Expected denials disclose only a filtered count, never hit identifiers. *******************************************************************************/ - public AiToolResult execute(UiApiToolbox.Context context, String callId, Map input, DataClassification classification, Integer conversationId, Integer turnId) + AiToolResult execute(UiApiToolbox.Context context, String callId, Map input, DataClassification classification, Integer conversationId, Integer turnId) { AiToolResult result = new AiToolResult().withToolName(NAME).withToolCallId(callId); Long started = System.nanoTime(); diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/KnowledgeToolFamily.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/KnowledgeToolFamily.java index 8731e3a..e5bdc17 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/KnowledgeToolFamily.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/KnowledgeToolFamily.java @@ -37,6 +37,29 @@ public class KnowledgeToolFamily implements ToolFamily { private static final List CHECKS = List.of(ToolCheck.TOOL_SWITCH, ToolCheck.DATA_ACCESS, ToolCheck.CLASSIFICATION, ToolCheck.PERSON_PERMISSIONS, ToolCheck.FIELD_EXPOSURE); + private final KnowledgeTool knowledgeTool; + + + + /******************************************************************************* + ** The knowledge tool is handed over by the runtime that made it; nothing + ** outside this package can reach it to run it around the service. + *******************************************************************************/ + public KnowledgeToolFamily(KnowledgeTool knowledgeTool) + { + this.knowledgeTool = knowledgeTool; + } + + + + /******************************************************************************* + ** The knowledge tool, for this package (and its tests). + *******************************************************************************/ + KnowledgeTool knowledgeTool() + { + return (knowledgeTool); + } + /******************************************************************************* @@ -45,7 +68,7 @@ public class KnowledgeToolFamily implements ToolFamily @Override public List tools(AssistantRuntime runtime) { - return (List.of(new Tool(runtime))); + return (List.of(new Tool(runtime, knowledgeTool))); } @@ -56,15 +79,17 @@ public class KnowledgeToolFamily implements ToolFamily private static final class Tool implements AssistantTool { private final AssistantRuntime runtime; + private final KnowledgeTool tool; /******************************************************************************* ** *******************************************************************************/ - private Tool(AssistantRuntime runtime) + private Tool(AssistantRuntime runtime, KnowledgeTool tool) { this.runtime = runtime; + this.tool = tool; } @@ -108,7 +133,6 @@ public class KnowledgeToolFamily implements ToolFamily @Override public ModelToolDefinition definition(ToolOffer offer) { - KnowledgeTool tool = runtime.getKnowledgeTool(); return (tool.offered(offer.policy(), offer.session(), offer.turnId()) ? tool.definition() : null); } @@ -120,7 +144,7 @@ public class KnowledgeToolFamily implements ToolFamily @Override public ToolCallOutcome execute(ToolCallRequest request) { - return (request.inPersonsContext(runtime.getQInstance(), () -> ToolCallOutcome.of(runtime.getKnowledgeTool().execute(request.context(), request.toolCallId(), request.input(), + return (request.inPersonsContext(runtime.getQInstance(), () -> ToolCallOutcome.of(tool.execute(request.context(), request.toolCallId(), request.input(), request.scope().classification(), request.scope().conversationId(), request.scope().turnId())))); } diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/PresentResultsTool.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/PresentResultsTool.java index 87c3da6..f1ebe92 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/PresentResultsTool.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/PresentResultsTool.java @@ -117,7 +117,7 @@ public class PresentResultsTool /******************************************************************************* ** Preserve valid items when others are dropped; never echo rejected values. *******************************************************************************/ - public synchronized AiToolResult execute(UiApiToolbox.Context context, String callId, Map input) + synchronized AiToolResult execute(UiApiToolbox.Context context, String callId, Map input) { AiToolResult result = new AiToolResult().withToolCallId(callId).withToolName(NAME); try diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/UiApiToolFamily.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/UiApiToolFamily.java index 9182a68..ea72846 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/UiApiToolFamily.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/UiApiToolFamily.java @@ -71,6 +71,29 @@ public class UiApiToolFamily implements ToolFamily Map.entry(AnalysisTools.ANALYZE_RECORDS, EXPOSE_THEN_READ), Map.entry(AnalysisTools.SAVE_ANALYSIS, List.of(TOOL_SWITCH, SPECD_OPERATION, DATA_ACCESS, FIELD_EXPOSURE, PERSON_PERMISSIONS, CONFIRMATION))); + private final UiApiToolbox toolbox; + + + + /******************************************************************************* + ** The toolbox is handed over by the runtime that made it; nothing outside + ** this package can reach it to run a tool around the service. + *******************************************************************************/ + public UiApiToolFamily(UiApiToolbox toolbox) + { + this.toolbox = toolbox; + } + + + + /******************************************************************************* + ** The toolbox, for this package (and its tests). + *******************************************************************************/ + UiApiToolbox toolbox() + { + return (toolbox); + } + /******************************************************************************* @@ -86,7 +109,7 @@ public class UiApiToolFamily implements ToolFamily List tools = new ArrayList<>(); for(String name : names) { - tools.add(new Tool(runtime, name)); + tools.add(new Tool(runtime, toolbox, name)); } return (tools); } @@ -99,6 +122,7 @@ public class UiApiToolFamily implements ToolFamily private static final class Tool implements AssistantTool { private final AssistantRuntime runtime; + private final UiApiToolbox toolbox; private final String name; @@ -106,9 +130,10 @@ public class UiApiToolFamily implements ToolFamily /******************************************************************************* ** *******************************************************************************/ - private Tool(AssistantRuntime runtime, String name) + private Tool(AssistantRuntime runtime, UiApiToolbox toolbox, String name) { this.runtime = runtime; + this.toolbox = toolbox; this.name = name; } @@ -157,8 +182,7 @@ public class UiApiToolFamily implements ToolFamily { Map offered = offer.once(UiApiToolFamily.class, () -> { - Map byName = new HashMap<>(); - UiApiToolbox toolbox = runtime.getToolbox(); + Map byName = new HashMap<>(); for(ModelToolDefinition definition : toolbox.definitions(offer.policy(), toolbox.anyWritableTable())) { byName.put(definition.getName(), definition.withStrict(ToolRegistry.isStrict(runtime.getConfig(), definition.getName(), true))); @@ -177,7 +201,7 @@ public class UiApiToolFamily implements ToolFamily public ToolCallOutcome execute(ToolCallRequest request) { return (request.inPersonsContext(runtime.getQInstance(), () -> - ToolCallOutcome.of(runtime.getToolbox().execute(request.context(), request.toolCallId(), request.toolName(), request.input())))); + ToolCallOutcome.of(toolbox.execute(request.context(), request.toolCallId(), request.toolName(), request.input())))); } @@ -192,9 +216,9 @@ public class UiApiToolFamily implements ToolFamily { if(AnalysisTools.WRITE_TOOLS.contains(name)) { - return (runtime.getToolbox().executeConfirmedAnalysis(call.context(), call.toolCallId(), call.toolName(), call.values(), call.userId())); + return (toolbox.executeConfirmedAnalysis(call.context(), call.toolCallId(), call.toolName(), call.values(), call.userId())); } - return (runtime.getToolbox().executeConfirmedWrite(call.context(), call.toolCallId(), call.toolName(), call.tableName(), call.recordId(), call.values())); + return (toolbox.executeConfirmedWrite(call.context(), call.toolCallId(), call.toolName(), call.tableName(), call.recordId(), call.values())); } } } diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/UiApiToolbox.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/UiApiToolbox.java index 30800ad..7ea8508 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/UiApiToolbox.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/UiApiToolbox.java @@ -136,6 +136,12 @@ public class UiApiToolbox Map.entry(AnalysisTools.ANALYZE_RECORDS, new String[] {"POST", AnalysisTools.PIVOT_PATH}), Map.entry(AnalysisTools.SAVE_ANALYSIS, new String[] {"POST", AnalysisTools.PIVOT_PATH})); + ///////////////////////////////////////////////////////////////////////// + // what the assistant may read for itself through read(): no tool that // + // proposes or changes anything // + ///////////////////////////////////////////////////////////////////////// + public static final Set ASSISTANT_READS = Set.of(LIST_TABLES, DESCRIBE_TABLE, COUNT_RECORDS, GET_RECORD); + public static final int DEFAULT_LIMIT = 20; private final QInstance qInstance; @@ -458,22 +464,37 @@ public class UiApiToolbox *******************************************************************************/ public boolean anyWritableTable() { - for(String tableName : qInstance.getTables().keySet()) - { - if(dataAccess.forTable(tableName).mode().allowsWrite()) - { - return (true); - } - } - return (false); + return (dataAccess.anyWritableTable()); } /******************************************************************************* - ** Run one tool call. Never throws: failures become error outcomes. + ** A read the assistant makes for itself, with the person's session: the + ** tables a person may read for their starter prompts, a knowledge hit read + ** again before it is shown. It is not a tool call of a model (those go + ** through engine.ToolCallService and are logged), and it is the only way + ** into the toolbox from outside this package: it runs the four tools named + ** in {@link #ASSISTANT_READS} and nothing else, so it can never propose or + ** make a change. Never throws: failures become error outcomes. *******************************************************************************/ - public Outcome execute(Context context, String toolCallId, String toolName, Map input) + public Outcome read(Context context, String callId, String toolName, Map input) + { + if(!ASSISTANT_READS.contains(toolName)) + { + String message = "Not one of the assistant's own reads: " + toolName; + return (new Outcome(new AiToolResult().withToolCallId(callId).withToolName(toolName).withOutcome(AiToolOutcome.INVALID_INPUT).withSummary(shortText(message)).withModelContent(message))); + } + return (execute(context, callId, toolName, input)); + } + + + + /******************************************************************************* + ** Run one tool call. Never throws: failures become error outcomes. Only + ** this package calls it (the tool family, for engine.ToolCallService). + *******************************************************************************/ + Outcome execute(Context context, String toolCallId, String toolName, Map input) { AiToolResult result = new AiToolResult().withToolCallId(toolCallId).withToolName(toolName); Outcome outcome = new Outcome(result); @@ -544,9 +565,10 @@ public class UiApiToolbox /******************************************************************************* ** Run a confirmed write (from a pending action) with the confirming user's - ** session. Same mapping as a direct write. + ** session. Only this package calls it (the tool family, for the service's + ** confirm). *******************************************************************************/ - public Outcome executeConfirmedWrite(Context context, String toolCallId, String toolName, String tableName, String recordId, Map values) + Outcome executeConfirmedWrite(Context context, String toolCallId, String toolName, String tableName, String recordId, Map values) { AiToolResult result = new AiToolResult().withToolCallId(toolCallId).withToolName(toolName).withTableName(tableName); Outcome outcome = new Outcome(result); @@ -605,7 +627,7 @@ public class UiApiToolbox ** user's session: the saved analysis and, when one was proposed, its ** schedule, emailed to that user. *******************************************************************************/ - public Outcome executeConfirmedAnalysis(Context context, String toolCallId, String toolName, Map values, String userId) + Outcome executeConfirmedAnalysis(Context context, String toolCallId, String toolName, Map values, String userId) { AiToolResult result = new AiToolResult().withToolCallId(toolCallId).withToolName(toolName).withTableName(AnalysisTools.SAVED_TABLE); Outcome outcome = new Outcome(result); @@ -1297,8 +1319,8 @@ public class UiApiToolbox /******************************************************************************* ** create / update / delete: checked against data access and the user's - ** table permissions as the UI reports them, then proposed (pending action) - ** or run. + ** table permissions as the UI reports them, then proposed (pending action). + ** A write runs only from executeConfirmedWrite, once the person confirmed. *******************************************************************************/ private void write(Context context, Outcome outcome, String toolName, Map in) throws QException { @@ -1335,47 +1357,45 @@ public class UiApiToolbox } } - if(context.policy() == null || context.policy().writesRequireConfirmation()) + /////////////////////////////////////////////////////////////////////// + // a change is always proposed: the person confirms it on a card, and // + // nothing (no policy, no caller) can make it run without that // + /////////////////////////////////////////////////////////////////////// + String summary = describeWrite(toolName, table, recordId, values); + Integer actionId = context.pendingActions() == null ? null + : context.pendingActions().propose(new ProposedWrite(outcome.result.getToolCallId(), toolName, outcome.operation, tableName, recordId, values, summary)); + if(actionId == null) { - String summary = describeWrite(toolName, table, recordId, values); - Integer actionId = context.pendingActions() == null ? null - : context.pendingActions().propose(new ProposedWrite(outcome.result.getToolCallId(), toolName, outcome.operation, tableName, recordId, values, summary)); - if(actionId == null) - { - throw (new QException("The write could not be proposed.")); - } - outcome.pendingActionId = actionId; - - List> changes = new ArrayList<>(); - values.forEach((name, value) -> - { - QFieldMetaData field = table.getFields().get(name); - Map change = new LinkedHashMap<>(); - change.put("field", name); - change.put("label", field == null || field.getLabel() == null ? name : field.getLabel()); - change.put("value", value == null ? null : String.valueOf(value)); - changes.add(change); - }); - Map render = new LinkedHashMap<>(); - render.put("type", "pending_action"); - render.put("actionId", actionId); - render.put("toolName", toolName); - render.put("tableName", tableName); - render.put("tableLabel", label(table)); - render.put("recordId", recordId); - render.put("summary", summary); - render.put("changes", changes); - if(recordId != null) - { - render.put("href", AiLinkBuilder.recordHref(tableName, recordId)); - } - outcome.result.withOutcome(AiToolOutcome.OK).withTableName(tableName).withRender(render).withSummary("Waiting for confirmation: " + summary) - .withPermissionCheck("pending:" + actionId) - .withModelContent("Proposed, not done yet: " + summary + ". The user must confirm it in the panel; nothing has changed. Tell the user what will change and ask them to confirm. Do not call the tool again for this change."); - return; + throw (new QException("The write could not be proposed.")); } + outcome.pendingActionId = actionId; - runWrite(context, outcome, toolName, tableName, recordId, values); + List> changes = new ArrayList<>(); + values.forEach((name, value) -> + { + QFieldMetaData field = table.getFields().get(name); + Map change = new LinkedHashMap<>(); + change.put("field", name); + change.put("label", field == null || field.getLabel() == null ? name : field.getLabel()); + change.put("value", value == null ? null : String.valueOf(value)); + changes.add(change); + }); + Map render = new LinkedHashMap<>(); + render.put("type", "pending_action"); + render.put("actionId", actionId); + render.put("toolName", toolName); + render.put("tableName", tableName); + render.put("tableLabel", label(table)); + render.put("recordId", recordId); + render.put("summary", summary); + render.put("changes", changes); + if(recordId != null) + { + render.put("href", AiLinkBuilder.recordHref(tableName, recordId)); + } + outcome.result.withOutcome(AiToolOutcome.OK).withTableName(tableName).withRender(render).withSummary("Waiting for confirmation: " + summary) + .withPermissionCheck("pending:" + actionId) + .withModelContent("Proposed, not done yet: " + summary + ". The user must confirm it in the panel; nothing has changed. Tell the user what will change and ask them to confirm. Do not call the tool again for this change."); } diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadToolFamily.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadToolFamily.java index 8aba9fc..9358377 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadToolFamily.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadToolFamily.java @@ -47,6 +47,32 @@ public class BulkLoadToolFamily implements ToolFamily private static final List READ_CHECKS = List.of(ToolCheck.TOOL_SWITCH, ToolCheck.DATA_ACCESS, ToolCheck.PERSON_PERMISSIONS); private static final List PROPOSE_CHECKS = List.of(ToolCheck.TOOL_SWITCH, ToolCheck.DATA_ACCESS, ToolCheck.CONFIRMATION, ToolCheck.PERSON_PERMISSIONS); + private final BulkLoadTools bulkLoadTools; + private final UiApiToolbox toolbox; + + + + /******************************************************************************* + ** The bulk-load tools, and the data toolbox (which knows whether any table + ** is writable), are handed over by the runtime that made them; nothing + ** outside this package can reach them to run a tool around the service. + *******************************************************************************/ + public BulkLoadToolFamily(BulkLoadTools bulkLoadTools, UiApiToolbox toolbox) + { + this.bulkLoadTools = bulkLoadTools; + this.toolbox = toolbox; + } + + + + /******************************************************************************* + ** The bulk-load tools, for this package (and its tests). + *******************************************************************************/ + BulkLoadTools bulkLoadTools() + { + return (bulkLoadTools); + } + /******************************************************************************* @@ -58,7 +84,7 @@ public class BulkLoadToolFamily implements ToolFamily List tools = new ArrayList<>(); for(String name : NAMES) { - tools.add(new Tool(runtime, name)); + tools.add(new Tool(runtime, bulkLoadTools, toolbox, name)); } return (tools); } @@ -71,6 +97,8 @@ public class BulkLoadToolFamily implements ToolFamily private static final class Tool implements AssistantTool { private final AssistantRuntime runtime; + private final BulkLoadTools bulkLoadTools; + private final UiApiToolbox toolbox; private final String name; @@ -78,9 +106,11 @@ public class BulkLoadToolFamily implements ToolFamily /******************************************************************************* ** *******************************************************************************/ - private Tool(AssistantRuntime runtime, String name) + private Tool(AssistantRuntime runtime, BulkLoadTools bulkLoadTools, UiApiToolbox toolbox, String name) { this.runtime = runtime; + this.bulkLoadTools = bulkLoadTools; + this.toolbox = toolbox; this.name = name; } @@ -130,10 +160,9 @@ public class BulkLoadToolFamily implements ToolFamily Map offered = offer.once(BulkLoadToolFamily.class, () -> { Map byName = new HashMap<>(); - BulkLoadTools tools = runtime.getBulkLoadTools(); - if(tools.offered()) + if(bulkLoadTools.offered()) { - for(ModelToolDefinition definition : tools.definitions(offer.policy(), runtime.getToolbox().anyWritableTable())) + for(ModelToolDefinition definition : bulkLoadTools.definitions(offer.policy(), toolbox.anyWritableTable())) { byName.put(definition.getName(), definition); } @@ -153,7 +182,7 @@ public class BulkLoadToolFamily implements ToolFamily public ToolCallOutcome execute(ToolCallRequest request) { return (request.inPersonsContext(runtime.getQInstance(), () -> - ToolCallOutcome.of(runtime.getBulkLoadTools().execute(request.context(), request.toolCallId(), request.toolName(), request.input(), request.scope().hipaaRoute())))); + ToolCallOutcome.of(bulkLoadTools.execute(request.context(), request.toolCallId(), request.toolName(), request.input(), request.scope().hipaaRoute())))); } @@ -168,7 +197,7 @@ public class BulkLoadToolFamily implements ToolFamily { return (AssistantTool.super.executeConfirmed(call)); } - return (runtime.getBulkLoadTools().executeConfirmed(call.context(), call.toolCallId(), call.toolName(), call.tableName(), call.values())); + return (bulkLoadTools.executeConfirmed(call.context(), call.toolCallId(), call.toolName(), call.tableName(), call.values())); } diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadTools.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadTools.java index cd92565..07bd7a9 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadTools.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadTools.java @@ -194,7 +194,7 @@ public class BulkLoadTools ** Run a tool. hipaaRoute is the turn's provider's HIPAA eligibility (sent ** as X-QQQ-AI-Route; the model cannot set it). *******************************************************************************/ - public UiApiToolbox.Outcome execute(UiApiToolbox.Context context, String toolCallId, String toolName, Map input, boolean hipaaRoute) + UiApiToolbox.Outcome execute(UiApiToolbox.Context context, String toolCallId, String toolName, Map input, boolean hipaaRoute) { AiToolResult result = new AiToolResult().withToolCallId(toolCallId).withToolName(toolName); Map in = input == null ? Map.of() : input; @@ -252,7 +252,7 @@ public class BulkLoadTools ** Run a confirmed write (from a pending action) with the confirming user's ** session. *******************************************************************************/ - public UiApiToolbox.Outcome executeConfirmed(UiApiToolbox.Context context, String toolCallId, String toolName, String tableName, Map values) + UiApiToolbox.Outcome executeConfirmed(UiApiToolbox.Context context, String toolCallId, String toolName, String tableName, Map values) { AiToolResult result = new AiToolResult().withToolCallId(toolCallId).withToolName(toolName).withTableName(tableName); Call call = new Call(context, false); diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ConfirmedToolCall.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ConfirmedToolCall.java index 675223b..600eaf4 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ConfirmedToolCall.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ConfirmedToolCall.java @@ -25,6 +25,9 @@ import com.kingsrook.qqq.backend.core.model.session.QSession; /******************************************************************************* ** A proposed write the person has confirmed: the pending action's tool call, ** to be run once with the confirming request's own session (the context). + ** Only engine.ToolCallService makes one (ArchUnit), from the stored pending + ** action it loaded for that person and claimed; a caller chooses none of the + ** table, the record or the values. ** ** @param session the confirming person's own session ** @param context that same session for the UI API, with the gateway and diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCall.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCall.java new file mode 100644 index 0000000..1f6e572 --- /dev/null +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCall.java @@ -0,0 +1,38 @@ +/* + * Copyright 2026 Kingsrook, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.kingsrook.qbits.assistant.tools.registry; + + +import java.util.Map; +import com.kingsrook.qbits.assistant.SignedInPerson; + + +/******************************************************************************* + ** One tool call as a caller hands it to engine.ToolCallService: who it is + ** for, and what the model (or another client) asked for. It carries nothing + ** the service decides for itself: no policy, no gateway, no credentials apart + ** from the person's own, no place to keep a proposed write. The service makes + ** those, and the {@link ToolCallRequest} a tool is run with. + ** + ** @param person the signed-in person the call runs as + ** @param toolCallId the caller's id of the call + ** @param toolName the tool called + ** @param input the call's input + *******************************************************************************/ +public record ToolCall(SignedInPerson person, String toolCallId, String toolName, Map input) +{ +} diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallRequest.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallRequest.java index dff5db0..993fd97 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallRequest.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallRequest.java @@ -26,14 +26,16 @@ import com.kingsrook.qqq.backend.core.model.session.QSession; /******************************************************************************* - ** One tool call, for one person. The service that runs it refuses a request - ** whose context was not made for its session (two identities in one call) - ** or carries no policy. + ** One tool call as a tool is run with it, for one person. Only + ** engine.ToolCallService makes one (ArchUnit), from the {@link ToolCall} a + ** caller handed it: the context, with the gateway, the policy in force and + ** where a proposed write is kept, is the service's own, never a caller's. ** ** @param session the signed-in person's own session ** @param context that same session for the UI API (its sessionUUID is the - ** session's), with the gateway, the policy's tool settings - ** and where a proposed write is kept + ** session's, its credentials the person's own), with the + ** runtime's gateway, the administrator's policy as the + ** service read it and where a proposed write is kept ** @param toolCallId the model's id of the call ** @param toolName the tool called ** @param input the model's input diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallScope.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallScope.java index fb43dbd..0ae53e5 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallScope.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallScope.java @@ -24,6 +24,12 @@ import com.kingsrook.qqq.backend.core.actions.ai.DataClassification; /******************************************************************************* ** What a tool call may know about where it runs (in chat: the turn). All of ** it is decided by the server; nothing here comes from the model. + ** + ** engine.ToolCallService does not take a scope's word for whose it is: it + ** checks that the conversation and the turn belong to the call's person + ** before a tool runs, and works out the client channel of the log row + ** itself. Whether the route is HIPAA eligible stays the scope's to say (see + ** {@link #hipaaRoute()}). *******************************************************************************/ public interface ToolCallScope { @@ -49,23 +55,15 @@ public interface ToolCallScope /******************************************************************************* - ** Whether the model that asked for the call is HIPAA eligible. + ** Whether the model that asked for the call is HIPAA eligible. This is the + ** one value the service cannot check: only the caller knows which model (or + ** which client outside the chat) will receive the result. A caller that + ** cannot vouch for its route answers false. *******************************************************************************/ boolean hipaaRoute(); - /******************************************************************************* - ** The client channel label the scope's calls are logged under: assistant, - ** with the turn when there is one. - *******************************************************************************/ - default String clientChannel() - { - return (turnId() == null ? "assistant" : "assistant/" + turnId()); - } - - - /******************************************************************************* ** State a tool keeps for as long as the scope lives (in chat: one turn), ** created on first use and then shared by the scope's calls. diff --git a/qbit-assistant/src/main/resources/help/qbit-assistant.yaml b/qbit-assistant/src/main/resources/help/qbit-assistant.yaml index 2d52369..4650587 100644 --- a/qbit-assistant/src/main/resources/help/qbit-assistant.yaml +++ b/qbit-assistant/src/main/resources/help/qbit-assistant.yaml @@ -459,9 +459,9 @@ entries: content: | Values, from least to most: **Low**, **Medium**, **High**, **Xhigh** and **Max**. Empty uses your application’s configured default effort. Models that do not support reasoning effort ignore it. - key: table:assistant_policy;field:writesRequireConfirmation - summary: When on, or empty, every change the assistant proposes waits for the user to confirm it in the panel. + summary: No longer used. Every change the assistant proposes waits for the user to confirm it in the panel. content: | - Off lets proposed creates, updates and deletes run at once. They still use only the asking user's own permissions and record locks, and tables set to read only in data access rules stay read only. Leave this on unless you have a clear reason. + This setting has no effect any more: a proposed create, update or delete always shows a confirmation card, and nothing can switch that off. A confirmed change still uses only the confirming user's own permissions and record locks, and tables set to read only in data access rules stay read only. - key: table:assistant_policy;field:instanceMonthlyTokensHard summary: The monthly token total for the whole site at which new questions are refused; empty means no token limit. content: | diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantChannelWallTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantChannelWallTest.java index 40505a8..a7b2446 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantChannelWallTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantChannelWallTest.java @@ -30,6 +30,7 @@ import com.kingsrook.qbits.assistant.metadata.AssistantChannelWall; import com.kingsrook.qbits.assistant.testing.AssistantTestFixture; import com.kingsrook.qbits.assistant.testing.BaseAssistantTest; import com.kingsrook.qbits.assistant.testing.UiApiTestServer; +import com.kingsrook.qbits.assistant.tools.Toolboxes; import com.kingsrook.qbits.assistant.tools.UiApiToolbox; import com.kingsrook.qbits.assistant.uiapi.UiApiCall; import com.kingsrook.qbits.assistant.uiapi.UiApiResult; @@ -111,7 +112,7 @@ class AssistantChannelWallTest extends BaseAssistantTest UiApiResult uiMetadata = ui(base, "GET", "/metaData", null); assertThat(uiMetadata.status()).isEqualTo(200); QContext.init(qInstance, AssistantTestFixture.ownerSession(AssistantTestFixture.ADMIN, config)); - UiApiToolbox.Outcome listed = config.getRuntime().getToolbox().execute(new UiApiToolbox.Context(AssistantTestFixture.ADMIN, null, + UiApiToolbox.Outcome listed = Toolboxes.execute(config.getRuntime(), new UiApiToolbox.Context(AssistantTestFixture.ADMIN, null, config.getRuntime().getUiApiGateway(), PolicyService.ToolPolicy.defaults(), null, null, "42"), "list", UiApiToolbox.LIST_TABLES, Map.of()); for(String table : qInstance.getTables().keySet().stream().filter(name -> name.startsWith("assistant")).toList()) { diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantDefaultsChangelogTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantDefaultsChangelogTest.java index f5bcea4..a1fa049 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantDefaultsChangelogTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantDefaultsChangelogTest.java @@ -252,11 +252,10 @@ class AssistantDefaultsChangelogTest extends BaseAssistantTest assertThat(decision.notices()).singleElement().asString().contains("80%", "Monthly spend (whole instance)"); //////////////////////////////////////////////// - // the seeded policy: writes wait, 50 rows max // + // the seeded policy: 50 rows max // //////////////////////////////////////////////// PolicyService.EffectivePolicy policy = config.getRuntime().getPolicyService().current(true); assertThat(policy.killSwitch()).isFalse(); - assertThat(policy.tools().writesRequireConfirmation()).isTrue(); assertThat(policy.tools().maxRowsPerCall()).isEqualTo(50); } diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/StarterPromptsTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/StarterPromptsTest.java index 69fc81b..3f34370 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/StarterPromptsTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/StarterPromptsTest.java @@ -24,18 +24,19 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; -import com.kingsrook.qbits.assistant.testing.AssistantTestFixture; import com.kingsrook.qbits.assistant.api.AssistantApi; import com.kingsrook.qbits.assistant.model.AssistantDataAccess; +import com.kingsrook.qbits.assistant.testing.AssistantTestFixture; import com.kingsrook.qbits.assistant.testing.BaseAssistantTest; import com.kingsrook.qbits.assistant.testing.UiApiTestServer; +import com.kingsrook.qbits.assistant.tools.Toolboxes; import com.kingsrook.qbits.assistant.tools.UiApiToolbox; import com.kingsrook.qbits.assistant.uiapi.UiApiCall; import com.kingsrook.qbits.assistant.uiapi.UiApiGateway; import com.kingsrook.qbits.assistant.uiapi.UiApiResult; -import com.kingsrook.qqq.backend.core.actions.tables.DeleteAction; import com.kingsrook.qqq.backend.core.actions.ai.AiExposure; import com.kingsrook.qqq.backend.core.actions.ai.AiFieldMetaData; +import com.kingsrook.qqq.backend.core.actions.tables.DeleteAction; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.model.actions.tables.delete.DeleteInput; @@ -83,7 +84,7 @@ class StarterPromptsTest extends BaseAssistantTest private StarterPrompts prompts(Instant now) { UiApiTestServer.serve(qInstance, config); - return (new StarterPrompts(config.getRuntime().getToolbox(), Clock.fixed(now, ZoneOffset.UTC))); + return (new StarterPrompts(Toolboxes.data(config.getRuntime()), Clock.fixed(now, ZoneOffset.UTC))); } @@ -126,7 +127,7 @@ class StarterPromptsTest extends BaseAssistantTest UiApiGateway gateway = config.getRuntime().getUiApiGateway(); for(int day = 0; day < 20; day++) { - StarterPrompts daily = new StarterPrompts(config.getRuntime().getToolbox(), Clock.fixed(NOW.plusSeconds(86400L * day), ZoneOffset.UTC)); + StarterPrompts daily = new StarterPrompts(Toolboxes.data(config.getRuntime()), Clock.fixed(NOW.plusSeconds(86400L * day), ZoneOffset.UTC)); List rs = daily.forUser(context(AssistantTestFixture.EDITOR, gateway), AssistantTestFixture.EDITOR, List.of(), true); assertThat(rs).isNotEmpty(); assertThat(String.join("|", rs)).doesNotContain("pages"); @@ -218,7 +219,7 @@ class StarterPromptsTest extends BaseAssistantTest return (delegate.invoke(call)); } }; - StarterPrompts starter = new StarterPrompts(config.getRuntime().getToolbox(), Clock.fixed(NOW, ZoneOffset.UTC)); + StarterPrompts starter = new StarterPrompts(Toolboxes.data(config.getRuntime()), Clock.fixed(NOW, ZoneOffset.UTC)); starter.forUser(context(AssistantTestFixture.EDITOR, counting), AssistantTestFixture.EDITOR, List.of(), false); int firstCalls = calls.get(); @@ -304,7 +305,7 @@ class StarterPromptsTest extends BaseAssistantTest UiApiToolbox.Context allowed = context(AssistantTestFixture.ANALYST, gateway); assertThat(starter.forUser(allowed, "same-user", List.of(), false)).isNotEmpty(); UiApiToolbox.Context denied = new UiApiToolbox.Context(allowed.sessionUUID(), null, gateway, - new PolicyService.ToolPolicy(true, Set.of(UiApiToolbox.DESCRIBE_TABLE), 50, true), null); + new PolicyService.ToolPolicy(Set.of(UiApiToolbox.DESCRIBE_TABLE), 50, true), null); assertThat(starter.forUser(denied, "same-user", List.of(), false)).isEmpty(); } @@ -332,7 +333,7 @@ class StarterPromptsTest extends BaseAssistantTest { StarterPrompts starter = prompts(NOW); UiApiToolbox.Context context = context(AssistantTestFixture.ANALYST, config.getRuntime().getUiApiGateway()); - String described = config.getRuntime().getToolbox().execute(context, "describe-date-fixture", UiApiToolbox.DESCRIBE_TABLE, + String described = Toolboxes.execute(config.getRuntime(), context, "describe-date-fixture", UiApiToolbox.DESCRIBE_TABLE, Map.of("table", "analytics_daily_stat")).getResult().getModelContent(); assertThat(described).contains("\"name\":\"statDate\"", "\"type\":\"DATE\""); assertThat(String.join("|", starter.forUser(context, AssistantTestFixture.ANALYST, List.of(), false))).contains("recent"); diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/ToolCallRowsTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/ToolCallRowsTest.java new file mode 100644 index 0000000..5ab9360 --- /dev/null +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/ToolCallRowsTest.java @@ -0,0 +1,289 @@ +/* + * Copyright 2026 Kingsrook, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.kingsrook.qbits.assistant.engine; + + +import java.util.List; +import java.util.Map; +import com.kingsrook.qbits.assistant.AssistantQBitConfig; +import com.kingsrook.qbits.assistant.AssistantRuntime; +import com.kingsrook.qbits.assistant.model.AssistantToolCall; +import com.kingsrook.qbits.assistant.spi.ModelToolDefinition; +import com.kingsrook.qbits.assistant.testing.BaseAssistantTest; +import com.kingsrook.qbits.assistant.testing.TurnRunner; +import com.kingsrook.qbits.assistant.tools.UiApiToolbox; +import com.kingsrook.qbits.assistant.tools.registry.AssistantTool; +import com.kingsrook.qbits.assistant.tools.registry.ToolCallOutcome; +import com.kingsrook.qbits.assistant.tools.registry.ToolCallRequest; +import com.kingsrook.qbits.assistant.tools.registry.ToolCheck; +import com.kingsrook.qbits.assistant.tools.registry.ToolFamily; +import com.kingsrook.qbits.assistant.tools.registry.ToolKind; +import com.kingsrook.qbits.assistant.tools.registry.ToolOffer; +import com.kingsrook.qqq.backend.core.actions.ai.AiToolOutcome; +import com.kingsrook.qqq.backend.core.actions.ai.AiToolResult; +import org.junit.jupiter.api.Test; +import static com.kingsrook.qbits.assistant.testing.AssistantTestFixture.EDITOR; +import static org.assertj.core.api.Assertions.assertThat; + + +/******************************************************************************* + ** Review fix 6: a tool call of a turn that ran has its tool-call log row, + ** with what it in fact did, however the turn's batch of calls ended: the + ** call ran past its time limit, its row could not be prepared the usual way, + ** or the engine was interrupted while it ran. The three tools of the family + ** registered here each do their work (a result that names an operation, its + ** status and a pending action) and then make one of those happen. + *******************************************************************************/ +class ToolCallRowsTest extends BaseAssistantTest +{ + private static final String LATE_TOOL = "late_tool"; + private static final String BAD_LOG_TOOL = "bad_log_tool"; + private static final String STOPPING_TOOL = "stopping_tool"; + + ///////////////////////////////////////////////////////////////////////// + // what each tool leaves behind: an obviously made-up pending action // + ///////////////////////////////////////////////////////////////////////// + private static final Integer LEFT_BEHIND = 424_242; + + + + /******************************************************************************* + ** A one-second tool limit and the family of this test. + *******************************************************************************/ + @Override + protected void customizeConfig(AssistantQBitConfig config) + { + config.withToolCallTimeoutSeconds(1); + config.withToolFamily(new RowsFamily()); + } + + + + /******************************************************************************* + ** A call that did its work and then ran out of time: the model and the + ** panel are told it took too long, and the row says what it did (before: + ** "Timed out", with no operation, status or pending action). + *******************************************************************************/ + @Test + void aCallThatRanAndThenTimedOutHasARowOfWhatItDid() throws Exception + { + provider.thenToolUse(LATE_TOOL, Map.of()).thenText("That took too long."); + TurnRunner.Outcome outcome = runner.run(EDITOR, "do the slow thing"); + + assertThat(outcome.end()).containsEntry("status", "COMPLETE"); + assertThat(outcome.events(TurnEvent.TOOL_RESULT)).singleElement().satisfies(result -> assertThat(result).containsEntry("outcome", "ERROR").containsEntry("summary", "Timed out")); + + assertThat(outcome.toolCalls()).singleElement().satisfies(row -> + { + assertThat(row.getToolName()).isEqualTo(LATE_TOOL); + assertThat(row.getOutcome()).isEqualTo("OK"); + assertThat(row.getResultSummary()).isEqualTo("Did it"); + assertDidItsWork(row); + }); + } + + + + /******************************************************************************* + ** A runtime exception while a row is prepared (here the tool's own + ** inputForLog throws) costs neither that row nor the rows of the calls + ** after it, and the turn goes on (before: the exception left the batch, so + ** that call and every later one had no row). + *******************************************************************************/ + @Test + void aRuntimeExceptionWhileLoggingCostsNoRow() throws Exception + { + provider.thenToolUses("Two steps.", List.of(new Object[] {BAD_LOG_TOOL, Map.of("note", "kept nowhere")}, new Object[] {UiApiToolbox.LIST_TABLES, Map.of()})).thenText("Done."); + TurnRunner.Outcome outcome = runner.run(EDITOR, "do both"); + + assertThat(outcome.end()).containsEntry("status", "COMPLETE"); + List rows = outcome.toolCalls().stream().sorted((a, b) -> a.getId().compareTo(b.getId())).toList(); + assertThat(rows).extracting(AssistantToolCall::getToolName).containsExactly(BAD_LOG_TOOL, UiApiToolbox.LIST_TABLES); + + assertThat(rows.get(0).getOutcome()).isEqualTo("OK"); + assertThat(rows.get(0).getResultSummary()).isEqualTo("Did it"); + assertThat(rows.get(0).getInputJson()).as("the input the tool could not prepare is not kept").isEqualTo("{}"); + assertDidItsWork(rows.get(0)); + assertThat(rows.get(1).getOutcome()).isEqualTo("OK"); + } + + + + /******************************************************************************* + ** The engine is interrupted while a call runs (the person stops the + ** answer): the call that ran still has its row, of what it did, and the + ** turn ends as stopped (before: the interruption left the batch at once + ** and no row was written). + *******************************************************************************/ + @Test + void aCallThatRanWhileTheEngineWasInterruptedHasARow() throws Exception + { + provider.thenToolUse(STOPPING_TOOL, Map.of()).thenText("Never said."); + TurnRunner.Outcome outcome = runner.run(EDITOR, "do it, then I stop you"); + + assertThat(outcome.end()).containsEntry("status", "CANCELLED"); + assertThat(outcome.toolCalls()).singleElement().satisfies(row -> + { + assertThat(row.getToolName()).isEqualTo(STOPPING_TOOL); + assertThat(row.getOutcome()).isEqualTo("OK"); + assertThat(row.getResultSummary()).isEqualTo("Did it"); + assertDidItsWork(row); + }); + } + + + + /******************************************************************************* + ** The row carries what the call left behind. + *******************************************************************************/ + private static void assertDidItsWork(AssistantToolCall row) + { + assertThat(row.getUserId()).isEqualTo(EDITOR); + assertThat(row.getTurnId()).isNotNull(); + assertThat(row.getUiApiOperation()).isEqualTo("POST /somewhere"); + assertThat(row.getHttpStatus()).isEqualTo(201); + assertThat(row.getPendingActionId()).isEqualTo(LEFT_BEHIND); + } + + + + /******************************************************************************* + ** The three tools. + *******************************************************************************/ + private static final class RowsFamily implements ToolFamily + { + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public List tools(AssistantRuntime runtime) + { + return (List.of(new RowsTool(runtime, LATE_TOOL), new RowsTool(runtime, BAD_LOG_TOOL), new RowsTool(runtime, STOPPING_TOOL))); + } + } + + + + /******************************************************************************* + ** A tool that does its work, then makes its call end badly. + *******************************************************************************/ + private static final class RowsTool implements AssistantTool + { + private final AssistantRuntime runtime; + private final String name; + + + + /******************************************************************************* + ** + *******************************************************************************/ + private RowsTool(AssistantRuntime runtime, String name) + { + this.runtime = runtime; + this.name = name; + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public String name() + { + return (name); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public ToolKind kind() + { + return (ToolKind.READ); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public List checks() + { + return (List.of(ToolCheck.TOOL_SWITCH)); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public ModelToolDefinition definition(ToolOffer offer) + { + return (offer.allows(name) ? new ModelToolDefinition().withName(name).withDescription("For tests.").withStrict(false).withInputSchema(Map.of("type", "object")) : null); + } + + + + /******************************************************************************* + ** The work is done first; then the late tool outlives its time limit (and + ** hands its outcome back when it is interrupted), and the stopping tool + ** stops its own turn, as the person's Stop does. + *******************************************************************************/ + @Override + public ToolCallOutcome execute(ToolCallRequest request) + { + AiToolResult result = new AiToolResult().withToolCallId(request.toolCallId()).withToolName(name).withOutcome(AiToolOutcome.OK).withSummary("Did it").withModelContent("Done."); + ToolCallOutcome done = ToolCallOutcome.of(UiApiToolbox.Outcome.of(result, "POST /somewhere", 201, LEFT_BEHIND)); + if(LATE_TOOL.equals(name)) + { + try + { + Thread.sleep(30_000); + } + catch(InterruptedException e) + { + Thread.currentThread().interrupt(); + } + } + if(STOPPING_TOOL.equals(name)) + { + runtime.getRunningTurn(request.scope().turnId()).cancel("USER"); + } + return (done); + } + + + + /******************************************************************************* + ** One tool cannot say what its row keeps as input. + *******************************************************************************/ + @Override + public Map inputForLog(Map input, AiToolResult result) + { + if(BAD_LOG_TOOL.equals(name)) + { + throw (new IllegalStateException("boom")); + } + return (input); + } + } +} diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/ToolCallServiceTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/ToolCallServiceTest.java index 77f49bc..a7800df 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/ToolCallServiceTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/ToolCallServiceTest.java @@ -17,13 +17,16 @@ package com.kingsrook.qbits.assistant.engine; -import java.util.HashMap; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.RecordComponent; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Supplier; import com.kingsrook.qbits.assistant.AssistantQBitConfig; import com.kingsrook.qbits.assistant.AssistantRuntime; +import com.kingsrook.qbits.assistant.SignedInPerson; import com.kingsrook.qbits.assistant.access.DataAccessMode; import com.kingsrook.qbits.assistant.api.AssistantApi; import com.kingsrook.qbits.assistant.api.AssistantApiException; @@ -34,12 +37,14 @@ import com.kingsrook.qbits.assistant.model.AssistantToolCall; import com.kingsrook.qbits.assistant.spi.ModelToolDefinition; import com.kingsrook.qbits.assistant.testing.AssistantTestFixture; import com.kingsrook.qbits.assistant.testing.BaseAssistantTest; +import com.kingsrook.qbits.assistant.testing.ToolCalls; import com.kingsrook.qbits.assistant.testing.TurnRunner; import com.kingsrook.qbits.assistant.testing.UiApiTestServer; +import com.kingsrook.qbits.assistant.tools.Toolboxes; import com.kingsrook.qbits.assistant.tools.UiApiToolbox; -import com.kingsrook.qbits.assistant.tools.bulkload.BulkLoadTools; import com.kingsrook.qbits.assistant.tools.registry.AssistantTool; import com.kingsrook.qbits.assistant.tools.registry.ConfirmedToolCall; +import com.kingsrook.qbits.assistant.tools.registry.ToolCall; import com.kingsrook.qbits.assistant.tools.registry.ToolCallOutcome; import com.kingsrook.qbits.assistant.tools.registry.ToolCallRequest; import com.kingsrook.qbits.assistant.tools.registry.ToolCallScope; @@ -47,12 +52,16 @@ import com.kingsrook.qbits.assistant.tools.registry.ToolCheck; import com.kingsrook.qbits.assistant.tools.registry.ToolFamily; import com.kingsrook.qbits.assistant.tools.registry.ToolKind; import com.kingsrook.qbits.assistant.tools.registry.ToolOffer; +import com.kingsrook.qbits.assistant.uiapi.UiApiGateway; import com.kingsrook.qqq.backend.core.actions.ai.AiToolOutcome; import com.kingsrook.qqq.backend.core.actions.ai.AiToolResult; import com.kingsrook.qqq.backend.core.actions.ai.DataClassification; import com.kingsrook.qqq.backend.core.context.QContext; +import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; +import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.session.QSession; +import com.kingsrook.qqq.middleware.javalin.specs.invoke.UiApiCredentials; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static com.kingsrook.qbits.assistant.testing.AssistantTestFixture.ADMIN; @@ -66,12 +75,14 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; /******************************************************************************* ** The tool-call service is safe for a caller that has no chat turn around - ** it: every check a call needs is made by the service itself, before the - ** tool is reached, and the call is logged by the service with its input - ** redacted. Each test calls the service directly, the way a later caller - ** will, with nothing of the turn engine in between; the UI API test server - ** sees every request the assistant makes, so "nothing ran" is what it did - ** not see. + ** it: everything a call needs is found out by the service itself (the policy, + ** the gateway, whose the conversation is, the budget), before the tool is + ** reached, and the call is logged by the service with its input redacted. A + ** caller hands it a person and a call, or a person and a pending action's + ** id, and nothing else. Each test calls the service directly, the way a later + ** caller will, with nothing of the turn engine in between; the UI API test + ** server sees every request the assistant makes, so "nothing ran" is what it + ** did not see. *******************************************************************************/ class ToolCallServiceTest extends BaseAssistantTest { @@ -80,12 +91,14 @@ class ToolCallServiceTest extends BaseAssistantTest ///////////////////////////////////////////////////////////////////////// private static final String FAKE_SSN = "123-45-6789"; - private static final String THROWING_TOOL = "throwing_tool"; + private static final String THROWING_TOOL = "throwing_tool"; + private static final String PROPOSING_TOOL = "proposing_tool"; /******************************************************************************* - ** A registered family whose tool always throws. + ** A registered family with a tool that always throws, and one that + ** proposes a write whose confirmation throws. *******************************************************************************/ @Override protected void customizeConfig(AssistantQBitConfig config) @@ -107,24 +120,23 @@ class ToolCallServiceTest extends BaseAssistantTest /******************************************************************************* - ** Fix 1: the service refuses a tool the person is not offered now, whoever - ** calls: a tool the policy switched off (the kernel's navigate has no policy - ** check of its own, and ran before), and a write tool while no table is - ** writable. The refusal is the one of a name without a tool, and nothing is + ** The service refuses a tool the person is not offered now, whoever calls: + ** a write tool while no table is writable, and a tool the administrator's + ** policy switched off (the kernel's navigate has no policy check of its + ** own). The refusal is the one of a name without a tool, and nothing is ** requested. A tool the same policy allows runs. *******************************************************************************/ @Test - void aToolThatIsNotOfferedIsRefusedByTheServiceItself() + void aToolThatIsNotOfferedIsRefusedByTheServiceItself() throws Exception { - PolicyService.ToolPolicy onlyListTables = new PolicyService.ToolPolicy(true, Set.of(UiApiToolbox.LIST_TABLES), 50, true); + assertUnknownTool(execute(ADMIN, UiApiToolbox.CREATE_RECORD, input("table", "blog_post", "values", Map.of("title", "Never proposed"))), UiApiToolbox.CREATE_RECORD); - assertUnknownTool(service().execute(request(EDITOR, "navigate", input("target", "blog_post"), onlyListTables)), "navigate"); - assertUnknownTool(service().execute(request(EDITOR, UiApiToolbox.COUNT_RECORDS, input("table", "blog_post", "filter", filter("AND")), onlyListTables)), UiApiToolbox.COUNT_RECORDS); - assertUnknownTool(service().execute(request(ADMIN, UiApiToolbox.CREATE_RECORD, input("table", "blog_post", "values", Map.of("title", "Never proposed")), PolicyService.ToolPolicy.defaults())), - UiApiToolbox.CREATE_RECORD); + policy(new AssistantPolicy().withName("default").withAllowedTools(UiApiToolbox.LIST_TABLES)); + assertUnknownTool(execute(EDITOR, "navigate", input("target", "blog_post")), "navigate"); + assertUnknownTool(execute(EDITOR, UiApiToolbox.COUNT_RECORDS, input("table", "blog_post", "filter", filter("AND"))), UiApiToolbox.COUNT_RECORDS); assertThat(UiApiTestServer.SCHEDULED_REQUESTS).as("nothing was requested").isEmpty(); - ToolCallOutcome allowed = service().execute(request(EDITOR, UiApiToolbox.LIST_TABLES, Map.of(), onlyListTables)); + ToolCallOutcome allowed = execute(EDITOR, UiApiToolbox.LIST_TABLES, Map.of()); assertThat(allowed.result().getOutcome()).isEqualTo(AiToolOutcome.OK); assertThat(UiApiTestServer.SCHEDULED_REQUESTS).isNotEmpty().allSatisfy(seen -> assertThat(seen).containsEntry("session", EDITOR)); } @@ -132,88 +144,141 @@ class ToolCallServiceTest extends BaseAssistantTest /******************************************************************************* - ** Fix 2: no policy is not "every tool". The service refuses a request - ** without one, and the three tools that read the switch themselves refuse - ** too when they are handed a context without a policy. + ** Review fix 1: a caller cannot supply a policy. What a caller hands the + ** service has no place for one, and no entry point of the service takes a + ** policy, a tool context or a gateway; the policy a call runs under is the + ** administrator's row, read by the service. *******************************************************************************/ @Test - void aCallWithoutAPolicyIsRefused() + void aCallerCannotSupplyAPolicy() throws Exception { - ToolCallOutcome outcome = service().execute(request(EDITOR, UiApiToolbox.LIST_TABLES, Map.of(), null)); - assertRefused(outcome, ToolCallService.CHECK_REQUEST); - assertThat(outcome.result().getModelContent()).contains("no policy"); + assertThat(Arrays.stream(ToolCall.class.getRecordComponents()).map(RecordComponent::getName)).containsExactly("person", "toolCallId", "toolName", "input"); + for(Method method : ToolCallService.class.getDeclaredMethods()) + { + if(Modifier.isPublic(method.getModifiers())) + { + assertThat(method.getParameterTypes()).as(method.getName() + " takes nothing the service decides itself") + .doesNotContain(PolicyService.ToolPolicy.class, PolicyService.EffectivePolicy.class, UiApiToolbox.Context.class, UiApiGateway.class, UiApiCredentials.class, QSession.class, + ToolCallRequest.class, ConfirmedToolCall.class, UiApiToolbox.PendingActions.class); + } + } - UiApiToolbox.Context noPolicy = context(EDITOR, null); + /////////////////////////////////////////////////////////////////////// + // the administrator's row decides: one row per call, two tools // + /////////////////////////////////////////////////////////////////////// + policy(new AssistantPolicy().withName("default").withAllowedTools(UiApiToolbox.LIST_TABLES + "," + UiApiToolbox.QUERY_RECORDS).withMaxRowsPerCall(1)); + ToolCallOutcome queried = execute(EDITOR, UiApiToolbox.QUERY_RECORDS, input("table", "blog_post", "filter", filter("AND"))); + assertThat(queried.result().getOutcome()).as(queried.result().getModelContent()).isEqualTo(AiToolOutcome.OK); + assertThat(queried.result().getRowCount()).as("the administrator's row cap").isEqualTo(1); + assertUnknownTool(execute(EDITOR, UiApiToolbox.COUNT_RECORDS, input("table", "blog_post", "filter", filter("AND"))), UiApiToolbox.COUNT_RECORDS); + } + + + + /******************************************************************************* + ** Review fix 1: a write always produces a proposal. Nothing can switch the + ** confirmation off: the administrator's old setting (the column is still + ** there) is not read any more, so the write tool only records a pending + ** action and the record is not created. + *******************************************************************************/ + @Test + void aWriteAlwaysProducesAProposal() throws Exception + { + writable("blog_post"); + policy(new AssistantPolicy().withName("default").withWritesRequireConfirmation(false)); + + ToolCallOutcome outcome = service().executeAndLog(ToolCalls.call(config, ADMIN, UiApiToolbox.CREATE_RECORD, input("table", "blog_post", "values", Map.of("title", "Only proposed", "status", "DRAFT"))), null, false); + assertThat(outcome.result().getOutcome()).as(outcome.result().getModelContent()).isEqualTo(AiToolOutcome.OK); + assertThat(outcome.extras().getPendingActionId()).as("a proposal").isNotNull(); + assertThat(outcome.extras().getHttpStatus()).as("no write was made").isNull(); + assertThat(outcome.result().getModelContent()).contains("Proposed, not done yet"); + assertThat(requestsFor("blog_post")).as("nothing was written").isEmpty(); + + assertThat(pendingAction(ADMIN, outcome.extras().getPendingActionId()).getStatus()).isEqualTo(AssistantPendingAction.STATUS_PENDING); + assertThat(everyToolCall()).singleElement().satisfies(row -> assertThat(row.getPendingActionId()).isEqualTo(outcome.extras().getPendingActionId())); + } + + + + /******************************************************************************* + ** The tools that read the policy's switch themselves still refuse when they + ** are handed a context without a policy (no policy is not "every tool"). + *******************************************************************************/ + @Test + void aToolHandedNoPolicyRefuses() + { + UiApiToolbox.Context noPolicy = new UiApiToolbox.Context(EDITOR, "UTC", config.getRuntime().getUiApiGateway(), null, null); AssistantRuntime runtime = config.getRuntime(); QContext.init(qInstance, AssistantTestFixture.ownerSession(EDITOR, config)); - AiToolResult data = runtime.getToolbox().execute(noPolicy, "call-1", UiApiToolbox.LIST_TABLES, Map.of()).getResult(); + AiToolResult data = Toolboxes.execute(runtime, noPolicy, "call-1", UiApiToolbox.LIST_TABLES, Map.of()).getResult(); assertThat(data.getOutcome()).isEqualTo(AiToolOutcome.DENIED); assertThat(data.getPermissionCheck()).isEqualTo("policy:tool"); - AiToolResult knowledge = runtime.getKnowledgeTool().execute(noPolicy, "call-2", Map.of("query", "posts"), DataClassification.PUBLIC, null, null); + AiToolResult knowledge = Toolboxes.search(runtime, noPolicy, "call-2", Map.of("query", "posts"), DataClassification.PUBLIC, null, null); assertThat(knowledge.getOutcome()).isEqualTo(AiToolOutcome.DENIED); - AiToolResult bulkLoad = runtime.getBulkLoadTools().execute(noPolicy, "call-3", BulkLoadTools.LIST, Map.of("tableName", "blog_post"), false).getResult(); - assertThat(bulkLoad.getOutcome()).isEqualTo(AiToolOutcome.DENIED); - assertThat(bulkLoad.getPermissionCheck()).isEqualTo("policy:tool"); - assertThat(UiApiTestServer.SCHEDULED_REQUESTS).as("nothing was requested").isEmpty(); } /******************************************************************************* - ** Fix 3: one call cannot mix two people. A session with a context made for - ** another session is refused, for a call and for a confirmation, and so is - ** a confirmation named for another person than its session's. + ** Review fix 2: one call cannot mix two people. A person is made from a + ** session alone (no forwarded credentials: the UI API gets that session's + ** own id) or by the route from one HTTP request; there is no constructor + ** that takes a session and credentials, and the service takes neither + ** loose (see aCallerCannotSupplyAPolicy). Every request of a call is made + ** as the call's person. *******************************************************************************/ @Test - void aCallWhoseSessionAndContextAreTwoPeopleIsRefused() + void aSessionWithAnotherPersonsCredentialsCannotBeMade() throws Exception { - QSession analyst = AssistantTestFixture.ownerSession(ANALYST, config); + assertThat(SignedInPerson.class.getConstructors()).as("no public constructor").isEmpty(); + assertThat(Arrays.stream(SignedInPerson.class.getDeclaredMethods()).filter(method -> Modifier.isPublic(method.getModifiers()) && Modifier.isStatic(method.getModifiers()))) + .as("the only ways to get a person").allSatisfy(factory -> + { + assertThat(factory.getParameterCount()).as(factory.getName() + " takes one thing: a session, or a request").isEqualTo(1); + assertThat(factory.getParameterTypes()).doesNotContain(UiApiCredentials.class); + }); - ToolCallOutcome mixed = service().execute(new ToolCallRequest(analyst, context(EDITOR, PolicyService.ToolPolicy.defaults()), "call-1", UiApiToolbox.COUNT_RECORDS, - input("table", "blog_post", "filter", filter("AND")), new Scope())); - assertRefused(mixed, ToolCallService.CHECK_REQUEST); - assertThat(mixed.result().getModelContent()).contains("not the same person"); + QSession analyst = AssistantTestFixture.ownerSession(ANALYST, config); + SignedInPerson person = SignedInPerson.ofSession(analyst); + assertThat(person.session()).isSameAs(analyst); + assertThat(person.credentials()).as("the session's own id is the credential").isNull(); + assertThat(person.userId()).isEqualTo(ANALYST); + assertThat(person.toString()).contains(ANALYST); + assertThatThrownBy(() -> SignedInPerson.ofSession(null)).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> SignedInPerson.ofRequest(null)).isInstanceOf(IllegalArgumentException.class); - UiApiToolbox.Outcome mixedConfirmation = service().executeConfirmed(new ConfirmedToolCall(analyst, context(EDITOR, PolicyService.ToolPolicy.defaults()), "call-2", - UiApiToolbox.CREATE_RECORD, "blog_post", null, Map.of("title", "Never created"), ANALYST)); - assertThat(mixedConfirmation.getResult().getOutcome()).isEqualTo(AiToolOutcome.DENIED); - assertThat(mixedConfirmation.getResult().getPermissionCheck()).isEqualTo(ToolCallService.CHECK_REQUEST); - - UiApiToolbox.Outcome namedForAnother = service().executeConfirmed(new ConfirmedToolCall(analyst, context(ANALYST, PolicyService.ToolPolicy.defaults()), "call-3", - UiApiToolbox.CREATE_RECORD, "blog_post", null, Map.of("title", "Never created"), EDITOR)); - assertThat(namedForAnother.getResult().getOutcome()).isEqualTo(AiToolOutcome.DENIED); - assertThat(namedForAnother.getResult().getPermissionCheck()).isEqualTo(ToolCallService.CHECK_REQUEST); - - assertThat(UiApiTestServer.SCHEDULED_REQUESTS).as("nothing was requested").isEmpty(); + ToolCallOutcome outcome = service().execute(new ToolCall(person, "call-1", UiApiToolbox.LIST_TABLES, Map.of()), ToolCalls.scope()); + assertThat(outcome.result().getOutcome()).isEqualTo(AiToolOutcome.OK); + assertThat(UiApiTestServer.SCHEDULED_REQUESTS).isNotEmpty().allSatisfy(seen -> assertThat(seen).containsEntry("session", ANALYST)); } /******************************************************************************* - ** Fix 4: executeAndLog runs the call and writes its row itself, with the - ** input redacted, for a call that ran, for one that was refused and for one - ** whose tool threw. The caller prepares nothing. + ** executeAndLog runs the call and writes its row itself, with the input + ** redacted, for a call that ran, for one that was refused and for one whose + ** tool threw. The caller prepares nothing. *******************************************************************************/ @Test void executeAndLogWritesTheRowWithRedactedInputAlsoForARefusedCall() throws Exception { Map input = input("table", "blog_post", "filter", filter("AND", "title", "EQUALS", List.of(FAKE_SSN))); - ToolCallOutcome ran = service().executeAndLog(request(EDITOR, UiApiToolbox.COUNT_RECORDS, input, PolicyService.ToolPolicy.defaults())); + ToolCallOutcome ran = service().executeAndLog(ToolCalls.call(config, EDITOR, UiApiToolbox.COUNT_RECORDS, input), null, false); assertThat(ran.result().getOutcome()).isEqualTo(AiToolOutcome.OK); - PolicyService.ToolPolicy onlyListTables = new PolicyService.ToolPolicy(true, Set.of(UiApiToolbox.LIST_TABLES), 50, true); - ToolCallOutcome refused = service().executeAndLog(request(EDITOR, UiApiToolbox.COUNT_RECORDS, input, onlyListTables)); - assertUnknownTool(refused, UiApiToolbox.COUNT_RECORDS); - - ToolCallOutcome threw = service().executeAndLog(request(EDITOR, THROWING_TOOL, input("note", FAKE_SSN), PolicyService.ToolPolicy.defaults())); + ToolCallOutcome threw = service().executeAndLog(ToolCalls.call(config, EDITOR, THROWING_TOOL, input("note", FAKE_SSN)), null, false); assertThat(threw.result().getOutcome()).isEqualTo(AiToolOutcome.ERROR); assertThat(threw.result().getModelContent()).doesNotContain("boom"); + policy(new AssistantPolicy().withName("default").withAllowedTools(UiApiToolbox.LIST_TABLES)); + ToolCallOutcome refused = service().executeAndLog(ToolCalls.call(config, EDITOR, UiApiToolbox.COUNT_RECORDS, input), null, false); + assertUnknownTool(refused, UiApiToolbox.COUNT_RECORDS); + List rows = everyToolCall(); assertThat(rows).hasSize(3).allSatisfy(row -> { @@ -230,34 +295,35 @@ class ToolCallServiceTest extends BaseAssistantTest assertThat(ranRow.getUiApiOperation()).isEqualTo("POST /table/{tableName}/count"); assertThat(ranRow.getHttpStatus()).isEqualTo(200); - AssistantToolCall refusedRow = rows.get(1); + AssistantToolCall threwRow = rows.get(1); + assertThat(threwRow.getToolName()).isEqualTo(THROWING_TOOL); + assertThat(threwRow.getOutcome()).isEqualTo("ERROR"); + assertThat(threwRow.getResultSummary()).isEqualTo("Failed"); + + AssistantToolCall refusedRow = rows.get(2); assertThat(refusedRow.getToolName()).isEqualTo(UiApiToolbox.COUNT_RECORDS); assertThat(refusedRow.getOutcome()).isEqualTo("INVALID_INPUT"); assertThat(refusedRow.getResultSummary()).isEqualTo("Unknown tool"); assertThat(refusedRow.getInputJson()).contains("blog_post"); assertThat(refusedRow.getHttpStatus()).isNull(); - AssistantToolCall threwRow = rows.get(2); - assertThat(threwRow.getToolName()).isEqualTo(THROWING_TOOL); - assertThat(threwRow.getOutcome()).isEqualTo("ERROR"); - assertThat(threwRow.getResultSummary()).isEqualTo("Failed"); - assertThat(requestsFor("blog_post")).as("only the call that was offered reached the UI API").hasSize(1); } /******************************************************************************* - ** Fix 4: a call that names no person at all is refused and, having nobody - ** to belong to, leaves no row. + ** A call that names no person at all is refused and, having nobody to + ** belong to, leaves no row; so is a call with no scope. *******************************************************************************/ @Test void aCallThatNamesNoPersonIsRefusedAndHasNoRow() throws Exception { - ToolCallOutcome nobody = service().executeAndLog(new ToolCallRequest(null, context(EDITOR, PolicyService.ToolPolicy.defaults()), "call-1", UiApiToolbox.LIST_TABLES, Map.of(), new Scope())); + ToolCallOutcome nobody = service().executeAndLog(new ToolCall(null, "call-1", UiApiToolbox.LIST_TABLES, Map.of()), null, false); assertRefused(nobody, ToolCallService.CHECK_REQUEST); - assertThat(service().executeAndLog(null).result().getOutcome()).isEqualTo(AiToolOutcome.DENIED); + assertThat(service().executeAndLog(null, null, false).result().getOutcome()).isEqualTo(AiToolOutcome.DENIED); + assertRefused(service().execute(ToolCalls.call(config, EDITOR, UiApiToolbox.LIST_TABLES, Map.of()), null), ToolCallService.CHECK_REQUEST); assertThat(everyToolCall()).isEmpty(); assertThat(UiApiTestServer.SCHEDULED_REQUESTS).isEmpty(); } @@ -265,10 +331,144 @@ class ToolCallServiceTest extends BaseAssistantTest /******************************************************************************* - ** Fix 6: a write the person confirms while the kill switch is on is refused - ** before the pending action is claimed, so it stays pending and can be - ** confirmed once the switch is off. The service refuses it too when it is - ** asked directly. + ** Review fix 3: the hard budget stops a call that no turn admitted, for the + ** person who is over it and for nobody else, and it is read on every call: + ** a scope that was let through before the limit is refused after it. + *******************************************************************************/ + @Test + void theHardBudgetIsCheckedByTheServiceOnEveryCall() throws Exception + { + provider.thenText("Hello."); + runner.run(EDITOR, "Hello"); + UiApiTestServer.SCHEDULED_REQUESTS.clear(); + + ToolCall call = ToolCalls.call(config, EDITOR, UiApiToolbox.LIST_TABLES, Map.of()); + ToolCallScope admitted = ToolCalls.scope(); + assertThat(service().execute(call, admitted).result().getOutcome()).isEqualTo(AiToolOutcome.OK); + UiApiTestServer.SCHEDULED_REQUESTS.clear(); + + config.getBudgets().withUserDailyTokensHard(1L); + ToolCallOutcome afterTheLimit = service().execute(call, admitted); + assertRefused(afterTheLimit, ToolCallService.CHECK_BUDGET); + assertThat(afterTheLimit.result().getModelContent()).contains("daily assistant usage limit"); + + assertRefused(execute(EDITOR, UiApiToolbox.LIST_TABLES, Map.of()), ToolCallService.CHECK_BUDGET); + assertThat(UiApiTestServer.SCHEDULED_REQUESTS).as("nothing was requested").isEmpty(); + + assertThat(execute(LIMITED, UiApiToolbox.LIST_TABLES, Map.of()).result().getPermissionCheck()) + .as("a person who has used nothing").isNotEqualTo(ToolCallService.CHECK_BUDGET); + } + + + + /******************************************************************************* + ** Review fix 4: a write with no stored pending action cannot be executed. + ** The one way to run a write is confirm(person, id): an id that names no + ** action of that person (none at all, or another person's) is refused, and + ** the other person's action stays pending and unwritten. + *******************************************************************************/ + @Test + void aWriteWithNoStoredPendingActionOfThePersonCannotBeExecuted() throws Exception + { + Integer actionId = proposeAPost(); + UiApiTestServer.SCHEDULED_REQUESTS.clear(); + + assertConfirmationRefused(ADMIN, 987_654, ToolCallService.ConfirmationRefused.Reason.NOT_FOUND); + assertConfirmationRefused(ADMIN, null, ToolCallService.ConfirmationRefused.Reason.NOT_FOUND); + assertConfirmationRefused(EDITOR, actionId, ToolCallService.ConfirmationRefused.Reason.NOT_FOUND); + + assertThat(pendingAction(ADMIN, actionId).getStatus()).isEqualTo(AssistantPendingAction.STATUS_PENDING); + assertThat(requestsFor("blog_post")).as("nothing was written").isEmpty(); + assertThat(everyToolCall()).as("only the proposal's row").hasSize(1); + } + + + + /******************************************************************************* + ** Review fix 4: a confirm runs the stored proposal (the caller names only + ** the person and the id), marks the action and writes its own row; it runs + ** once. + *******************************************************************************/ + @Test + void aConfirmRunsTheStoredProposalOnceAndWritesItsOwnRow() throws Exception + { + Integer actionId = proposeAPost(); + UiApiTestServer.SCHEDULED_REQUESTS.clear(); + + ToolCallService.Confirmation confirmation = service().confirm(SignedInPerson.ofSession(AssistantTestFixture.sessionFor(ADMIN)), actionId); + assertThat(confirmation.outcome().getResult().getOutcome()).as(confirmation.outcome().getResult().getModelContent()).isEqualTo(AiToolOutcome.OK); + assertThat(confirmation.action().getStatus()).isEqualTo(AssistantPendingAction.STATUS_EXECUTED); + assertThat(requestsFor("blog_post")).as("the write, as the confirming person").isNotEmpty().allSatisfy(seen -> assertThat(seen).containsEntry("session", ADMIN)); + + List rows = everyToolCall(); + assertThat(rows).as("the proposal's row and the confirmation's").hasSize(2); + AssistantToolCall row = rows.get(1); + assertThat(row.getPendingActionId()).isEqualTo(actionId); + assertThat(row.getUserId()).isEqualTo(ADMIN); + assertThat(row.getToolName()).isEqualTo(UiApiToolbox.CREATE_RECORD); + assertThat(row.getTableName()).isEqualTo("blog_post"); + assertThat(row.getOutcome()).isEqualTo("OK"); + assertThat(row.getResultSummary()).startsWith("Confirmed: "); + assertThat(row.getUiApiOperation()).isEqualTo("POST /table/{tableName}"); + assertThat(row.getHttpStatus()).isBetween(200, 299); + assertThat(row.getInputJson()).contains("Proposed in a turn"); + + assertConfirmationRefused(ADMIN, actionId, ToolCallService.ConfirmationRefused.Reason.NOT_PENDING); + assertThat(everyToolCall()).as("no second write, no second row").hasSize(2); + } + + + + /******************************************************************************* + ** Review fix 7: a confirmed action never sticks in RUNNING. A tool that + ** throws, and stored values that cannot be read, both end in FAILED with a + ** log row; what went wrong is not shown. + *******************************************************************************/ + @Test + void aConfirmWhoseToolThrowsEndsFailedWithARow() throws Exception + { + ToolCallOutcome proposed = service().executeAndLog(ToolCalls.call(config, EDITOR, PROPOSING_TOOL, Map.of()), null, false); + Integer actionId = proposed.extras().getPendingActionId(); + assertThat(actionId).isNotNull(); + + ToolCallService.Confirmation confirmation = service().confirm(SignedInPerson.ofSession(AssistantTestFixture.sessionFor(EDITOR)), actionId); + assertThat(confirmation.outcome().getResult().getOutcome()).isEqualTo(AiToolOutcome.ERROR); + assertThat(confirmation.outcome().getResult().getModelContent()).doesNotContain("boom"); + assertThat(confirmation.action().getStatus()).isEqualTo(AssistantPendingAction.STATUS_FAILED); + assertThat(pendingAction(EDITOR, actionId).getStatus()).isEqualTo(AssistantPendingAction.STATUS_FAILED); + + List rows = everyToolCall(); + assertThat(rows).hasSize(2); + assertThat(rows.get(1).getPendingActionId()).isEqualTo(actionId); + assertThat(rows.get(1).getOutcome()).isEqualTo("ERROR"); + assertThat(rows.get(1).getResultSummary()).isEqualTo("Confirmed: Failed"); + assertThat(rows.get(1).getResultJson()).doesNotContain("boom"); + + /////////////////////////////////////////////////////////////////////// + // stored values that are not JSON any more // + /////////////////////////////////////////////////////////////////////// + Integer unreadable = service().executeAndLog(ToolCalls.call(config, EDITOR, PROPOSING_TOOL, Map.of()), null, false).extras().getPendingActionId(); + QContext.init(qInstance, AssistantTestFixture.ownerSession(EDITOR, config)); + try + { + config.getRuntime().getStore().updateFields(AssistantPendingAction.TABLE_NAME, new QRecord().withValue("id", unreadable).withValue("paramsJson", "{not json")); + } + finally + { + QContext.clear(); + } + ToolCallService.Confirmation second = service().confirm(SignedInPerson.ofSession(AssistantTestFixture.sessionFor(EDITOR)), unreadable); + assertThat(second.outcome().getResult().getOutcome()).isEqualTo(AiToolOutcome.ERROR); + assertThat(pendingAction(EDITOR, unreadable).getStatus()).isEqualTo(AssistantPendingAction.STATUS_FAILED); + assertThat(everyToolCall()).filteredOn(row -> unreadable.equals(row.getPendingActionId()) && "ERROR".equals(row.getOutcome())).hasSize(1); + } + + + + /******************************************************************************* + ** A write the person confirms while the kill switch is on is refused before + ** the pending action is claimed, so it stays pending and can be confirmed + ** once the switch is off; through the route and by the service itself. *******************************************************************************/ @Test void aConfirmationWhileTheKillSwitchIsOnIsRefusedAndTheActionStaysPending() throws Exception @@ -290,19 +490,18 @@ class ToolCallServiceTest extends BaseAssistantTest assertThat(api.getAction(actionId)).containsEntry("status", AssistantPendingAction.STATUS_PENDING); assertThat(requestsFor("blog_post")).as("nothing was written").isEmpty(); assertThat(everyToolCall()).as("only the proposal's row").hasSize(1); + QContext.clear(); - UiApiToolbox.Outcome direct = service().executeConfirmed(new ConfirmedToolCall(admin, context(ADMIN, PolicyService.ToolPolicy.defaults()), "call-1", UiApiToolbox.CREATE_RECORD, - "blog_post", null, Map.of("title", "Never created"), ADMIN)); - assertThat(direct.getResult().getOutcome()).isEqualTo(AiToolOutcome.DENIED); - assertThat(direct.getResult().getPermissionCheck()).isEqualTo(ToolCallService.CHECK_KILL_SWITCH); + assertConfirmationRefused(ADMIN, actionId, ToolCallService.ConfirmationRefused.Reason.KILL_SWITCH); + assertThat(pendingAction(ADMIN, actionId).getStatus()).isEqualTo(AssistantPendingAction.STATUS_PENDING); assertThat(requestsFor("blog_post")).as("nothing was written").isEmpty(); } /******************************************************************************* - ** Fix 6: so is a confirmation of a tool the policy has switched off since - ** the write was proposed. + ** So is a confirmation of a tool the policy has switched off since the + ** write was proposed. *******************************************************************************/ @Test void aConfirmationOfAToolThePolicySwitchedOffIsRefusedAndTheActionStaysPending() throws Exception @@ -319,31 +518,30 @@ class ToolCallServiceTest extends BaseAssistantTest QContext.init(qInstance, admin); assertThat(api.getAction(actionId)).containsEntry("status", AssistantPendingAction.STATUS_PENDING); assertThat(requestsFor("blog_post")).as("nothing was written").isEmpty(); + QContext.clear(); - UiApiToolbox.Outcome direct = service().executeConfirmed(new ConfirmedToolCall(admin, context(ADMIN, PolicyService.ToolPolicy.defaults()), "call-1", UiApiToolbox.CREATE_RECORD, - "blog_post", null, Map.of("title", "Never created"), ADMIN)); - assertThat(direct.getResult().getOutcome()).isEqualTo(AiToolOutcome.DENIED); - assertThat(direct.getResult().getPermissionCheck()).isEqualTo(ToolCallService.CHECK_TOOL_SWITCH); + assertConfirmationRefused(ADMIN, actionId, ToolCallService.ConfirmationRefused.Reason.NOT_ALLOWED); + assertThat(pendingAction(ADMIN, actionId).getStatus()).isEqualTo(AssistantPendingAction.STATUS_PENDING); } /******************************************************************************* - ** Fix 9: with no chat turn around the call, the service itself requires - ** assistant access, and checks it before the kill switch: a person without - ** access is told so even while the switch is on. + ** With no chat turn around the call, the service itself requires assistant + ** access, and checks it before the kill switch: a person without access is + ** told so even while the switch is on. A confirmation needs it too. *******************************************************************************/ @Test void assistantAccessIsRequiredByTheService() throws Exception { - assertRefused(service().execute(request(NO_ACCESS, UiApiToolbox.LIST_TABLES, Map.of(), PolicyService.ToolPolicy.defaults())), ToolCallService.CHECK_ACCESS); + assertRefused(execute(NO_ACCESS, UiApiToolbox.LIST_TABLES, Map.of()), ToolCallService.CHECK_ACCESS); policy(new AssistantPolicy().withName("default").withKillSwitch(true)); - assertRefused(service().execute(request(NO_ACCESS, UiApiToolbox.LIST_TABLES, Map.of(), PolicyService.ToolPolicy.defaults())), ToolCallService.CHECK_ACCESS); + assertRefused(execute(NO_ACCESS, UiApiToolbox.LIST_TABLES, Map.of()), ToolCallService.CHECK_ACCESS); - UiApiToolbox.Outcome confirmation = service().executeConfirmed(new ConfirmedToolCall(AssistantTestFixture.sessionFor(NO_ACCESS), context(NO_ACCESS, PolicyService.ToolPolicy.defaults()), - "call-1", UiApiToolbox.CREATE_RECORD, "blog_post", null, Map.of("title", "Never created"), NO_ACCESS)); - assertThat(confirmation.getResult().getPermissionCheck()).isEqualTo(ToolCallService.CHECK_ACCESS); + assertConfirmationRefused(NO_ACCESS, 1, ToolCallService.ConfirmationRefused.Reason.NO_ACCESS); + assertThatThrownBy(() -> service().confirm(null, 1)).isInstanceOfSatisfying(ToolCallService.ConfirmationRefused.class, + e -> assertThat(e.getReason()).isEqualTo(ToolCallService.ConfirmationRefused.Reason.NO_ACCESS)); assertThat(UiApiTestServer.SCHEDULED_REQUESTS).as("nothing was requested").isEmpty(); } @@ -351,16 +549,16 @@ class ToolCallServiceTest extends BaseAssistantTest /******************************************************************************* - ** Fix 9: the kill switch stops a call that no turn admitted. + ** The kill switch stops a call that no turn admitted. *******************************************************************************/ @Test void theKillSwitchIsCheckedByTheService() throws Exception { - assertThat(service().execute(request(EDITOR, UiApiToolbox.LIST_TABLES, Map.of(), PolicyService.ToolPolicy.defaults())).result().getOutcome()).isEqualTo(AiToolOutcome.OK); + assertThat(execute(EDITOR, UiApiToolbox.LIST_TABLES, Map.of()).result().getOutcome()).isEqualTo(AiToolOutcome.OK); UiApiTestServer.SCHEDULED_REQUESTS.clear(); policy(new AssistantPolicy().withName("default").withKillSwitch(true)); - ToolCallOutcome outcome = service().execute(request(EDITOR, UiApiToolbox.LIST_TABLES, Map.of(), PolicyService.ToolPolicy.defaults())); + ToolCallOutcome outcome = execute(EDITOR, UiApiToolbox.LIST_TABLES, Map.of()); assertRefused(outcome, ToolCallService.CHECK_KILL_SWITCH); assertThat(UiApiTestServer.SCHEDULED_REQUESTS).as("nothing was requested").isEmpty(); } @@ -368,30 +566,52 @@ class ToolCallServiceTest extends BaseAssistantTest /******************************************************************************* - ** Fix 9: the hard budget stops a call that no turn admitted, for the person - ** who is over it and for nobody else; a scope that was let through is not - ** asked again (the check is made when a scope's first call arrives). + ** Review fix 8: the scope is checked, not believed. A call that names + ** another person's conversation, or a turn that is not the person's, is + ** refused before anything is requested, and its row does not carry the + ** other person's conversation. The person's own conversation is accepted + ** and is on the row. *******************************************************************************/ @Test - void theHardBudgetIsCheckedByTheService() throws Exception + void aScopeNamingSomeoneElsesConversationIsRefused() throws Exception { - provider.thenText("Hello."); - runner.run(EDITOR, "Hello"); - UiApiTestServer.SCHEDULED_REQUESTS.clear(); + Integer editors = conversationOf(EDITOR); + Integer analysts = conversationOf(ANALYST); - ToolCallRequest admitted = request(EDITOR, UiApiToolbox.LIST_TABLES, Map.of(), PolicyService.ToolPolicy.defaults()); - assertThat(service().execute(admitted).result().getOutcome()).isEqualTo(AiToolOutcome.OK); - UiApiTestServer.SCHEDULED_REQUESTS.clear(); + ToolCallOutcome someoneElses = service().executeAndLog(ToolCalls.call(config, ANALYST, UiApiToolbox.LIST_TABLES, Map.of()), editors, false); + assertRefused(someoneElses, ToolCallService.CHECK_REQUEST); + assertThat(someoneElses.result().getModelContent()).contains("not this person's"); - config.getBudgets().withUserDailyTokensHard(1L); - ToolCallOutcome outcome = service().execute(request(EDITOR, UiApiToolbox.LIST_TABLES, Map.of(), PolicyService.ToolPolicy.defaults())); - assertRefused(outcome, ToolCallService.CHECK_BUDGET); - assertThat(outcome.result().getModelContent()).contains("daily assistant usage limit"); + assertRefused(service().execute(ToolCalls.call(config, ANALYST, UiApiToolbox.LIST_TABLES, Map.of()), ToolCalls.scope(editors, null)), ToolCallService.CHECK_REQUEST); + assertRefused(service().execute(ToolCalls.call(config, ANALYST, UiApiToolbox.LIST_TABLES, Map.of()), ToolCalls.scope(analysts, 987_654)), ToolCallService.CHECK_REQUEST); + assertRefused(service().execute(ToolCalls.call(config, ANALYST, UiApiToolbox.LIST_TABLES, Map.of()), ToolCalls.scope(null, 987_654)), ToolCallService.CHECK_REQUEST); assertThat(UiApiTestServer.SCHEDULED_REQUESTS).as("nothing was requested").isEmpty(); - assertThat(service().execute(request(LIMITED, UiApiToolbox.LIST_TABLES, Map.of(), PolicyService.ToolPolicy.defaults())).result().getPermissionCheck()) - .as("a person who has used nothing").isNotEqualTo(ToolCallService.CHECK_BUDGET); - assertThat(service().execute(admitted).result().getOutcome()).as("the scope admitted before the limit").isEqualTo(AiToolOutcome.OK); + ToolCallOutcome own = service().executeAndLog(ToolCalls.call(config, ANALYST, UiApiToolbox.LIST_TABLES, Map.of()), analysts, false); + assertThat(own.result().getOutcome()).isEqualTo(AiToolOutcome.OK); + + List rows = everyToolCall(); + assertThat(rows).hasSize(2).allSatisfy(row -> assertThat(row.getUserId()).isEqualTo(ANALYST)); + assertThat(rows.get(0).getConversationId()).as("a refused call's row never names another person's conversation").isNull(); + assertThat(rows.get(0).getOutcome()).isEqualTo("DENIED"); + assertThat(rows.get(1).getConversationId()).isEqualTo(analysts); + assertThat(rows.get(1).getClientChannel()).isEqualTo("assistant"); + } + + + + /******************************************************************************* + ** Review fix 9: a tool family registered after the QBit was produced would + ** never be in the registry; the config says so instead of ignoring it. + *******************************************************************************/ + @Test + void aToolFamilyRegisteredAfterTheQBitIsProducedIsRefused() + { + assertThat(config.getRuntime()).isNotNull(); + assertThatThrownBy(() -> config.withToolFamily(new ThrowingFamily())).isInstanceOf(IllegalStateException.class).hasMessageContaining("before the QBit is produced"); + assertThat(config.getToolFamilies()).hasSize(1); + + assertThat(new AssistantQBitConfig().withToolFamily(new ThrowingFamily()).getToolFamilies()).hasSize(1); } @@ -407,21 +627,11 @@ class ToolCallServiceTest extends BaseAssistantTest /******************************************************************************* - ** A call of a person's, in a scope of its own, with no turn. + ** A call of a person's, in a scope of its own, with no turn and no row. *******************************************************************************/ - private ToolCallRequest request(String person, String toolName, Map input, PolicyService.ToolPolicy policy) + private ToolCallOutcome execute(String person, String toolName, Map input) { - return (new ToolCallRequest(AssistantTestFixture.ownerSession(person, config), context(person, policy), "call-1", toolName, input, new Scope())); - } - - - - /******************************************************************************* - ** A person's tool context, outside a turn. - *******************************************************************************/ - private UiApiToolbox.Context context(String person, PolicyService.ToolPolicy policy) - { - return (new UiApiToolbox.Context(person, "UTC", config.getRuntime().getUiApiGateway(), policy, null)); + return (service().execute(ToolCalls.call(config, person, toolName, input), ToolCalls.scope())); } @@ -452,20 +662,41 @@ class ToolCallServiceTest extends BaseAssistantTest /******************************************************************************* - ** A write the administrator's turn proposed: the pending action's id. + ** A confirmation the service refuses, for that reason. *******************************************************************************/ - private Integer proposeAPost() throws Exception + private void assertConfirmationRefused(String person, Integer actionId, ToolCallService.ConfirmationRefused.Reason reason) + { + assertThatThrownBy(() -> service().confirm(SignedInPerson.ofSession(AssistantTestFixture.sessionFor(person)), actionId)) + .isInstanceOfSatisfying(ToolCallService.ConfirmationRefused.class, e -> assertThat(e.getReason()).isEqualTo(reason)); + } + + + + /******************************************************************************* + ** Let the assistant write to a table. + *******************************************************************************/ + private void writable(String tableName) throws Exception { QContext.init(qInstance, AssistantTestFixture.sessionFor(ADMIN)); try { - config.getRuntime().getStore().insert(new AssistantDataAccess().withObjectType(AssistantDataAccess.TYPE_TABLE).withObjectName("blog_post").withMode(DataAccessMode.READ_WRITE.name())); + config.getRuntime().getStore().insert(new AssistantDataAccess().withObjectType(AssistantDataAccess.TYPE_TABLE).withObjectName(tableName).withMode(DataAccessMode.READ_WRITE.name())); config.getRuntime().getDataAccessService().invalidate(); } finally { QContext.clear(); } + } + + + + /******************************************************************************* + ** A write the administrator's turn proposed: the pending action's id. + *******************************************************************************/ + private Integer proposeAPost() throws Exception + { + writable("blog_post"); provider.thenToolUse(UiApiToolbox.CREATE_RECORD, input("table", "blog_post", "values", Map.of("title", "Proposed in a turn", "status", "DRAFT"))).thenText("Please confirm."); TurnRunner.Outcome outcome = runner.run(ADMIN, "Create a draft post"); assertThat(outcome.toolCalls()).hasSize(1); @@ -475,6 +706,43 @@ class ToolCallServiceTest extends BaseAssistantTest + /******************************************************************************* + ** A new conversation of a person's: its id. + *******************************************************************************/ + private Integer conversationOf(String person) throws Exception + { + QSession session = AssistantTestFixture.sessionFor(person); + QContext.init(qInstance, session); + try + { + return (((Number) new AssistantApi(config.getRuntime(), session).createConversation(Map.of("title", "Of " + person)).get("id")).intValue()); + } + finally + { + QContext.clear(); + } + } + + + + /******************************************************************************* + ** A person's pending action, as it is stored now. + *******************************************************************************/ + private AssistantPendingAction pendingAction(String person, Integer actionId) throws Exception + { + QContext.init(qInstance, AssistantTestFixture.ownerSession(person, config)); + try + { + return (config.getRuntime().getStore().getPendingAction(person, actionId)); + } + finally + { + QContext.clear(); + } + } + + + /******************************************************************************* ** The requests the UI API server saw for a table. *******************************************************************************/ @@ -524,72 +792,8 @@ class ToolCallServiceTest extends BaseAssistantTest /******************************************************************************* - ** A scope with no turn: what a caller outside the chat gives the service. - *******************************************************************************/ - private static final class Scope implements ToolCallScope - { - private final Map, Object> state = new HashMap<>(); - - - - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public DataClassification classification() - { - return (DataClassification.PUBLIC); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public Integer conversationId() - { - return (null); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public Integer turnId() - { - return (null); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public boolean hipaaRoute() - { - return (false); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public T state(Class type, Supplier create) - { - return (type.cast(state.computeIfAbsent(type, key -> create.get()))); - } - } - - - - /******************************************************************************* - ** A registered family with one tool, which throws. + ** A registered family with two tools: one that throws, and one that + ** proposes a write whose confirmation throws. *******************************************************************************/ private static final class ThrowingFamily implements ToolFamily { @@ -599,61 +803,106 @@ class ToolCallServiceTest extends BaseAssistantTest @Override public List tools(AssistantRuntime runtime) { - return (List.of(new AssistantTool() + return (List.of(new TestTool(THROWING_TOOL), new TestTool(PROPOSING_TOOL))); + } + } + + + + /******************************************************************************* + ** One of the family's tools. + *******************************************************************************/ + private static final class TestTool implements AssistantTool + { + private final String name; + + + + /******************************************************************************* + ** + *******************************************************************************/ + private TestTool(String name) + { + this.name = name; + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public String name() + { + return (name); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public ToolKind kind() + { + return (PROPOSING_TOOL.equals(name) ? ToolKind.PROPOSE : ToolKind.READ); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public List checks() + { + return (PROPOSING_TOOL.equals(name) ? List.of(ToolCheck.TOOL_SWITCH, ToolCheck.CONFIRMATION) : List.of(ToolCheck.TOOL_SWITCH)); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public ModelToolDefinition definition(ToolOffer offer) + { + return (offer.allows(name) ? new ModelToolDefinition().withName(name).withDescription("For tests.").withStrict(false).withInputSchema(Map.of("type", "object")) : null); + } + + + + /******************************************************************************* + ** One throws; the other proposes a write in the store the service gave it. + *******************************************************************************/ + @Override + public ToolCallOutcome execute(ToolCallRequest request) + { + if(THROWING_TOOL.equals(name)) { - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public String name() - { - return (THROWING_TOOL); - } + throw (new IllegalStateException("boom")); + } + try + { + Integer actionId = request.context().pendingActions().propose(new UiApiToolbox.ProposedWrite(request.toolCallId(), name, "POST /nowhere", "blog_post", null, Map.of("title", "x"), "A write whose confirmation throws")); + AiToolResult result = new AiToolResult().withToolCallId(request.toolCallId()).withToolName(name).withOutcome(AiToolOutcome.OK).withSummary("Proposed").withModelContent("Proposed."); + return (ToolCallOutcome.of(UiApiToolbox.Outcome.of(result, "POST /nowhere", null, actionId))); + } + catch(QException e) + { + throw (new IllegalStateException(e)); + } + } - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public ToolKind kind() - { - return (ToolKind.READ); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public List checks() - { - return (List.of(ToolCheck.TOOL_SWITCH)); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public ModelToolDefinition definition(ToolOffer offer) - { - return (offer.allows(THROWING_TOOL) ? new ModelToolDefinition().withName(THROWING_TOOL).withDescription("Throws.").withStrict(false).withInputSchema(Map.of("type", "object")) : null); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - @Override - public ToolCallOutcome execute(ToolCallRequest request) - { - throw (new IllegalStateException("boom")); - } - })); + /******************************************************************************* + ** The confirmation of the proposing tool's write throws. + *******************************************************************************/ + @Override + public UiApiToolbox.Outcome executeConfirmed(ConfirmedToolCall call) + { + throw (new IllegalStateException("boom")); } } } diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/rag/RagTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/rag/RagTest.java index 110e16a..c56bfca 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/rag/RagTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/rag/RagTest.java @@ -45,6 +45,7 @@ import com.kingsrook.qbits.assistant.testing.AssistantTestFixture; import com.kingsrook.qbits.assistant.testing.BaseAssistantTest; import com.kingsrook.qbits.assistant.testing.UiApiTestServer; import com.kingsrook.qbits.assistant.tools.KnowledgeTool; +import com.kingsrook.qbits.assistant.tools.Toolboxes; import com.kingsrook.qbits.assistant.tools.UiApiToolbox; import com.kingsrook.qbits.assistant.uiapi.UiApiCall; import com.kingsrook.qqq.backend.core.actions.ai.AiExposure; @@ -61,8 +62,8 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryInput; import com.kingsrook.qqq.backend.core.model.actions.tables.update.UpdateInput; import com.kingsrook.qqq.backend.core.model.data.QRecord; -import com.kingsrook.qqq.middleware.javalin.specs.invoke.UiApiCredentials; import com.kingsrook.qqq.backend.core.utils.JsonUtils; +import com.kingsrook.qqq.middleware.javalin.specs.invoke.UiApiCredentials; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -244,7 +245,7 @@ class RagTest extends BaseAssistantTest private AiToolResult search(String user, DataClassification classification) throws Exception { as(user); - return (config.getRuntime().getKnowledgeTool().execute(config.getRuntime().getRagService().context(), "search-1", Map.of("query", "Drain upgrade verify", "limit", 8), classification, null, null)); + return (Toolboxes.search(config.getRuntime(), config.getRuntime().getRagService().context(), "search-1", Map.of("query", "Drain upgrade verify", "limit", 8), classification, null, null)); } @@ -361,14 +362,14 @@ class RagTest extends BaseAssistantTest assertThat(allowed.getOutcome()).as(allowed.getModelContent()).isEqualTo(AiToolOutcome.OK); assertThat(allowed.getRowCount()).isPositive(); assertThat(allowed.getModelContent()).contains("Drain, upgrade, verify.", "/app/blog_post/").doesNotContain(AssistantTestFixture.HIDDEN_SENTINEL, AssistantTestFixture.PASSWORD_SENTINEL); - assertThat(config.getRuntime().getKnowledgeTool().offered(PolicyService.ToolPolicy.defaults(), QContext.getQSession())).isTrue(); + assertThat(Toolboxes.knowledge(config.getRuntime()).offered(PolicyService.ToolPolicy.defaults(), QContext.getQSession())).isTrue(); AiToolResult denied = search(AssistantTestFixture.ANALYST, DataClassification.PUBLIC); assertThat(denied.getOutcome()).isEqualTo(AiToolOutcome.OK); assertThat(denied.getRowCount()).isZero(); assertThat(denied.getSummary()).doesNotEndWith("filtered 0"); assertThat(denied.getRecordRefs()).isEmpty(); assertThat(denied.getModelContent()).doesNotContain("blog_post", "Drain", "/app/"); - assertThat(config.getRuntime().getKnowledgeTool().offered(PolicyService.ToolPolicy.defaults(), QContext.getQSession())).isFalse(); + assertThat(Toolboxes.knowledge(config.getRuntime()).offered(PolicyService.ToolPolicy.defaults(), QContext.getQSession())).isFalse(); assertThat(config.getRuntime().getUiApiGateway().invoke(new UiApiCall("GET", "/table/{tableName}/{primaryKey}") .withPathParam("tableName", "blog_post").withPathParam("primaryKey", "1").withSessionUUID(AssistantTestFixture.ANALYST)).status()).isEqualTo(403); } @@ -393,7 +394,7 @@ class RagTest extends BaseAssistantTest as(AssistantTestFixture.ADMIN); config.getRuntime().getStore().insert(new AssistantDataAccess().withObjectType("TABLE").withObjectName("blog_post").withMode("NONE")); assertThat(rag.validate(source)).anyMatch(s -> s.contains("NONE")); - assertThat(config.getRuntime().getKnowledgeTool().offered(PolicyService.ToolPolicy.defaults(), QContext.getQSession())).isFalse(); + assertThat(Toolboxes.knowledge(config.getRuntime()).offered(PolicyService.ToolPolicy.defaults(), QContext.getQSession())).isFalse(); assertThat(search(AssistantTestFixture.EDITOR, DataClassification.PUBLIC).getRowCount()).isZero(); } @@ -418,7 +419,7 @@ class RagTest extends BaseAssistantTest config.getRuntime().getStore().updateFields(AssistantProvider.TABLE_NAME, new QRecord().withValue("id", embeddingProvider.getId()).withValue("hipaaEligible", true)); assertThat(config.getRuntime().getRagService().index(patient).errors()).isZero(); as(AssistantTestFixture.EDITOR); - AiToolResult result = config.getRuntime().getKnowledgeTool().execute(config.getRuntime().getRagService().context(), "phi", Map.of("query", "diagnosis", "tables", List.of("patient")), DataClassification.PUBLIC, null, null); + AiToolResult result = Toolboxes.search(config.getRuntime(), config.getRuntime().getRagService().context(), "phi", Map.of("query", "diagnosis", "tables", List.of("patient")), DataClassification.PUBLIC, null, null); assertThat(result.getRowCount()).isPositive(); TurnClassification classification = new TurnClassification(config.getClassificationSource(), new AssistantConversation().withMaxClassification("PUBLIC"), null); classification.observe(ModelPart.toolUse("phi", KnowledgeTool.NAME, Map.of()), result); @@ -615,11 +616,11 @@ class RagTest extends BaseAssistantTest KnowledgeIndexProcess.ClearReview clearReview = new KnowledgeIndexProcess.ClearReview(); clearReview.runOnePage(new RunBackendStepInput().withRecords(List.of(source.toQRecord())), clearOutput); assertThat(clearReview.getProcessSummary(clearOutput, false)).hasSize(1); - var policy = new PolicyService.ToolPolicy(true, Set.of("search_help"), 50, true); - assertThat(config.getRuntime().getKnowledgeTool().offered(policy, QContext.getQSession())).isFalse(); + var policy = new PolicyService.ToolPolicy(Set.of("search_help"), 50, true); + assertThat(Toolboxes.knowledge(config.getRuntime()).offered(policy, QContext.getQSession())).isFalse(); var context = new UiApiToolbox.Context(AssistantTestFixture.ADMIN, null, config.getRuntime().getUiApiGateway(), policy, null); - assertThat(config.getRuntime().getKnowledgeTool().execute(context, "denied", Map.of("query", "x"), DataClassification.PUBLIC, null, null).getOutcome()).isEqualTo(AiToolOutcome.DENIED); - assertThat(config.getRuntime().getKnowledgeTool().execute(config.getRuntime().getRagService().context(), "invalid", Map.of(), DataClassification.PUBLIC, null, null).getOutcome()).isEqualTo(AiToolOutcome.INVALID_INPUT); + assertThat(Toolboxes.search(config.getRuntime(), context, "denied", Map.of("query", "x"), DataClassification.PUBLIC, null, null).getOutcome()).isEqualTo(AiToolOutcome.DENIED); + assertThat(Toolboxes.search(config.getRuntime(), config.getRuntime().getRagService().context(), "invalid", Map.of(), DataClassification.PUBLIC, null, null).getOutcome()).isEqualTo(AiToolOutcome.INVALID_INPUT); config.getBudgets().withUserDailyTokensHard(1L); assertThatThrownBy(() -> new EmbeddingService(config.getRuntime()).embed(model.getId(), List.of("blocked"), DataClassification.PUBLIC, "blog_post", null, null)).hasMessageContaining("budget"); } diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/testing/ToolCalls.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/testing/ToolCalls.java new file mode 100644 index 0000000..bec1c03 --- /dev/null +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/testing/ToolCalls.java @@ -0,0 +1,151 @@ +/* + * Copyright 2026 Kingsrook, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.kingsrook.qbits.assistant.testing; + + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; +import com.kingsrook.qbits.assistant.AssistantQBitConfig; +import com.kingsrook.qbits.assistant.SignedInPerson; +import com.kingsrook.qbits.assistant.tools.registry.ToolCall; +import com.kingsrook.qbits.assistant.tools.registry.ToolCallScope; +import com.kingsrook.qqq.backend.core.actions.ai.DataClassification; + + +/******************************************************************************* + ** Tool calls as a caller hands them to the tool-call service, for tests that + ** call the service directly with nothing of the turn engine in between. + *******************************************************************************/ +public final class ToolCalls +{ + /******************************************************************************* + ** Utility class. + *******************************************************************************/ + private ToolCalls() + { + } + + + + /******************************************************************************* + ** A call ("call-1") of a fixture person, with a session of their own. + *******************************************************************************/ + public static ToolCall call(AssistantQBitConfig config, String person, String toolName, Map input) + { + return (new ToolCall(SignedInPerson.ofSession(AssistantTestFixture.ownerSession(person, config)), "call-1", toolName, input)); + } + + + + /******************************************************************************* + ** A scope that names no conversation and no turn. + *******************************************************************************/ + public static ToolCallScope scope() + { + return (new Scope(null, null)); + } + + + + /******************************************************************************* + ** A scope that names a conversation and a turn (either may be null), as a + ** caller would claim them: the service checks whose they are. + *******************************************************************************/ + public static ToolCallScope scope(Integer conversationId, Integer turnId) + { + return (new Scope(conversationId, turnId)); + } + + + + /******************************************************************************* + ** A scope a caller made. + *******************************************************************************/ + private static final class Scope implements ToolCallScope + { + private final Integer conversationId; + private final Integer turnId; + private final Map, Object> state = new ConcurrentHashMap<>(); + + + + /******************************************************************************* + ** + *******************************************************************************/ + private Scope(Integer conversationId, Integer turnId) + { + this.conversationId = conversationId; + this.turnId = turnId; + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public DataClassification classification() + { + return (DataClassification.PUBLIC); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public Integer conversationId() + { + return (conversationId); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public Integer turnId() + { + return (turnId); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public boolean hipaaRoute() + { + return (false); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public T state(Class type, Supplier create) + { + return (type.cast(state.computeIfAbsent(type, key -> create.get()))); + } + } +} diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/AnalysisSaveTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/AnalysisSaveTest.java index 4264080..c85719a 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/AnalysisSaveTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/AnalysisSaveTest.java @@ -163,15 +163,14 @@ class AnalysisSaveTest extends BaseAssistantTest /******************************************************************************* - ** Never without the confirmation, even where the administrator turned - ** confirmation off for record writes; and saving alone needs no schedule. + ** Never without the confirmation (no policy can switch it off, for this + ** tool or any other); and saving alone needs no schedule. *******************************************************************************/ @Test void alwaysAProposalAndSavingAloneWorks() throws Exception { - PolicyService.ToolPolicy noConfirmation = new PolicyService.ToolPolicy(false, Set.of(), 50, true); - Map in = lastDayByFormat(null, null); - UiApiToolbox.Outcome proposed = tool(AnalysisTestFixture.REPORTER, noConfirmation, in); + Map in = lastDayByFormat(null, null); + UiApiToolbox.Outcome proposed = tool(AnalysisTestFixture.REPORTER, PolicyService.ToolPolicy.defaults(), in); assertThat(proposed.getResult().getOutcome()).as(proposed.getResult().getModelContent()).isEqualTo(AiToolOutcome.OK); assertThat(proposed.getPendingActionId()).isNotNull(); assertThat(count(SAVED)).isZero(); @@ -386,7 +385,7 @@ class AnalysisSaveTest extends BaseAssistantTest QContext.init(qInstance, AssistantTestFixture.ownerSession(user, config)); try { - return (config.getRuntime().getToolbox().execute(new UiApiToolbox.Context(user, null, config.getRuntime().getUiApiGateway(), policy == null ? PolicyService.ToolPolicy.defaults() : policy, + return (Toolboxes.execute(config.getRuntime(), new UiApiToolbox.Context(user, null, config.getRuntime().getUiApiGateway(), policy == null ? PolicyService.ToolPolicy.defaults() : policy, write -> config.getRuntime().getStore().insert(new AssistantPendingAction().withUserId(user).withToolCallId(write.toolCallId()).withToolName(write.toolName()).withTableName(write.tableName()) .withRecordId(write.recordId()).withUiApiOperation(write.operation()).withParamsJson(com.kingsrook.qqq.backend.core.utils.JsonUtils.toJson(write.values())) .withSummary(write.summary()).withStatus(AssistantPendingAction.STATUS_PENDING) diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/AnalysisToolsTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/AnalysisToolsTest.java index 8bdc62c..44c4b2d 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/AnalysisToolsTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/AnalysisToolsTest.java @@ -204,13 +204,13 @@ class AnalysisToolsTest extends BaseAssistantTest assertThat(outcome.getHttpStatus()).as("no request is made").isNull(); } - PolicyService.ToolPolicy oneRow = new PolicyService.ToolPolicy(true, Set.of(), 1, true); + PolicyService.ToolPolicy oneRow = new PolicyService.ToolPolicy(Set.of(), 1, true); UiApiToolbox.Outcome capped = tool(AnalysisTestFixture.REPORTER, oneRow, input("table", AnalysisTestFixture.TABLE, "groupBy", List.of(Map.of("field", "format")), "filter", filter("AND"), "limit", 100)); assertThat(capped.getResult().getOutcome()).as(capped.getResult().getModelContent()).isEqualTo(AiToolOutcome.TRUNCATED); assertThat(capped.getResult().getRowCount()).isEqualTo(1); assertThat(capped.getResult().getModelContent()).contains("\"hasMore\":true"); - PolicyService.ToolPolicy onlyCount = new PolicyService.ToolPolicy(true, Set.of(UiApiToolbox.COUNT_RECORDS), 50, true); + PolicyService.ToolPolicy onlyCount = new PolicyService.ToolPolicy(Set.of(UiApiToolbox.COUNT_RECORDS), 50, true); UiApiToolbox.Outcome refused = tool(AnalysisTestFixture.REPORTER, onlyCount, input("table", AnalysisTestFixture.TABLE, "groupBy", List.of(Map.of("field", "format")), "filter", filter("AND"))); assertThat(refused.getResult().getOutcome()).isEqualTo(AiToolOutcome.DENIED); assertThat(refused.getHttpStatus()).isNull(); @@ -286,7 +286,7 @@ class AnalysisToolsTest extends BaseAssistantTest QContext.init(qInstance, AssistantTestFixture.ownerSession(user, config)); try { - return (config.getRuntime().getToolbox().execute(new UiApiToolbox.Context(user, null, config.getRuntime().getUiApiGateway(), policy == null ? PolicyService.ToolPolicy.defaults() : policy, null), "call-1", AnalysisTools.ANALYZE_RECORDS, in)); + return (Toolboxes.execute(config.getRuntime(), new UiApiToolbox.Context(user, null, config.getRuntime().getUiApiGateway(), policy == null ? PolicyService.ToolPolicy.defaults() : policy, null), "call-1", AnalysisTools.ANALYZE_RECORDS, in)); } finally { diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/PresentResultsToolTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/PresentResultsToolTest.java index 1eba20e..8ad4bd0 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/PresentResultsToolTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/PresentResultsToolTest.java @@ -254,7 +254,7 @@ class PresentResultsToolTest extends BaseAssistantTest @Test void testInvalidSummaryAndAlwaysOffered() throws Exception { - assertThat(runner.runtime().getContextBuilder().tools(new PolicyService.ToolPolicy(true, java.util.Set.of("navigate"), 20, true))) + assertThat(runner.runtime().getContextBuilder().tools(new PolicyService.ToolPolicy(java.util.Set.of("navigate"), 20, true))) .anyMatch(tool -> PresentResultsTool.NAME.equals(tool.getName()) && !Boolean.TRUE.equals(tool.getStrict())); assertThat(execute(input("summary", "x".repeat(301))).getOutcome()).isEqualTo(AiToolOutcome.INVALID_INPUT); provider.thenToolUse(PresentResultsTool.NAME, Map.of()).thenText("Unable to present."); diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/Toolboxes.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/Toolboxes.java new file mode 100644 index 0000000..bd2ce47 --- /dev/null +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/Toolboxes.java @@ -0,0 +1,97 @@ +/* + * Copyright 2026 Kingsrook, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.kingsrook.qbits.assistant.tools; + + +import java.util.Map; +import com.kingsrook.qbits.assistant.AssistantRuntime; +import com.kingsrook.qbits.assistant.tools.registry.ToolFamily; +import com.kingsrook.qqq.backend.core.actions.ai.AiToolResult; +import com.kingsrook.qqq.backend.core.actions.ai.DataClassification; + + +/******************************************************************************* + ** The toolboxes of a runtime, for tests. Main code outside the tools + ** packages cannot reach a toolbox (the runtime has no getter for one, and + ** the entry points that run a tool are package-private); a test that checks + ** what a toolbox does on its own, without the tool-call service in front of + ** it, gets it here. + *******************************************************************************/ +public final class Toolboxes +{ + /******************************************************************************* + ** Utility class. + *******************************************************************************/ + private Toolboxes() + { + } + + + + /******************************************************************************* + ** The runtime's UI API data toolbox. + *******************************************************************************/ + public static UiApiToolbox data(AssistantRuntime runtime) + { + for(ToolFamily family : runtime.ownToolFamilies()) + { + if(family instanceof UiApiToolFamily dataTools) + { + return (dataTools.toolbox()); + } + } + throw (new IllegalStateException("The runtime has no UI API tool family.")); + } + + + + /******************************************************************************* + ** The runtime's knowledge tool. + *******************************************************************************/ + public static KnowledgeTool knowledge(AssistantRuntime runtime) + { + for(ToolFamily family : runtime.ownToolFamilies()) + { + if(family instanceof KnowledgeToolFamily knowledgeTools) + { + return (knowledgeTools.knowledgeTool()); + } + } + throw (new IllegalStateException("The runtime has no knowledge tool family.")); + } + + + + /******************************************************************************* + ** Run a data tool straight on the toolbox, with no service in front of it. + *******************************************************************************/ + public static UiApiToolbox.Outcome execute(AssistantRuntime runtime, UiApiToolbox.Context context, String toolCallId, String toolName, Map input) + { + return (data(runtime).execute(context, toolCallId, toolName, input)); + } + + + + /******************************************************************************* + ** Run the knowledge search straight on the tool, with no service in front + ** of it. + *******************************************************************************/ + public static AiToolResult search(AssistantRuntime runtime, UiApiToolbox.Context context, String callId, Map input, DataClassification classification, Integer conversationId, Integer turnId) + { + return (knowledge(runtime).execute(context, callId, input, classification, conversationId, turnId)); + } +} diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/UiApiToolsSecurityTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/UiApiToolsSecurityTest.java index 78a3454..e54305a 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/UiApiToolsSecurityTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/UiApiToolsSecurityTest.java @@ -98,7 +98,7 @@ class UiApiToolsSecurityTest extends BaseAssistantTest .withPathParam("tableName", table).withJsonBody(Map.of()).withSessionUUID(AssistantTestFixture.EDITOR)); assertThat(result.status()).isEqualTo(403); } - assertThat(config.getRuntime().getToolbox().definitions()).extracting(definition -> definition.getName()).doesNotContain(UiApiToolbox.RECORD_HISTORY); + assertThat(Toolboxes.data(config.getRuntime()).definitions()).extracting(definition -> definition.getName()).doesNotContain(UiApiToolbox.RECORD_HISTORY); assertThat(tool(AssistantTestFixture.EDITOR, null, UiApiToolbox.RECORD_HISTORY, input("table", "page", "id", "12")).getHttpStatus()).isEqualTo(403); } @@ -112,7 +112,7 @@ class UiApiToolsSecurityTest extends BaseAssistantTest { config.getRuntime().close(); qInstance = AssistantTestFixture.build(config.withRecordHistory(true)); - assertThat(config.getRuntime().getToolbox().definitions()).extracting(definition -> definition.getName()).contains(UiApiToolbox.RECORD_HISTORY); + assertThat(Toolboxes.data(config.getRuntime()).definitions()).extracting(definition -> definition.getName()).contains(UiApiToolbox.RECORD_HISTORY); UiApiToolbox.Outcome outcome = tool(AssistantTestFixture.EDITOR, null, UiApiToolbox.RECORD_HISTORY, input("table", "page", "id", "12", "since", "THIS_WEEK")); assertThat(outcome.getResult().getOutcome()).isEqualTo(AiToolOutcome.OK); assertThat(outcome.getHttpStatus()).isEqualTo(200); @@ -132,13 +132,13 @@ class UiApiToolsSecurityTest extends BaseAssistantTest qInstance = AssistantTestFixture.build(config.withRecordHistory(true)); ClientChannelsMetaData channels = ClientChannelsMetaData.ofOrWithNew(qInstance); channels.withDeniedObject("assistant", name); - assertThat(config.getRuntime().getToolbox().definitions()).extracting(definition -> definition.getName()).doesNotContain(UiApiToolbox.RECORD_HISTORY); + assertThat(Toolboxes.data(config.getRuntime()).definitions()).extracting(definition -> definition.getName()).doesNotContain(UiApiToolbox.RECORD_HISTORY); channels.getDeniedObjects().get("assistant").remove(name); qInstance.getTable(name).getPermissionRules().withDeniedClients(Set.of("assistant")); - assertThat(config.getRuntime().getToolbox().definitions()).extracting(definition -> definition.getName()).doesNotContain(UiApiToolbox.RECORD_HISTORY); + assertThat(Toolboxes.data(config.getRuntime()).definitions()).extracting(definition -> definition.getName()).doesNotContain(UiApiToolbox.RECORD_HISTORY); qInstance.setDefaultPermissionRules(qInstance.getTable(name).getPermissionRules()); qInstance.getTable(name).setPermissionRules(null); - assertThat(config.getRuntime().getToolbox().definitions()).extracting(definition -> definition.getName()).doesNotContain(UiApiToolbox.RECORD_HISTORY); + assertThat(Toolboxes.data(config.getRuntime()).definitions()).extracting(definition -> definition.getName()).doesNotContain(UiApiToolbox.RECORD_HISTORY); } @@ -184,7 +184,7 @@ class UiApiToolsSecurityTest extends BaseAssistantTest QContext.init(qInstance, AssistantTestFixture.ownerSession(user, config)); try { - return (config.getRuntime().getToolbox().execute(new UiApiToolbox.Context(user, null, config.getRuntime().getUiApiGateway(), policy == null ? PolicyService.ToolPolicy.defaults() : policy, + return (Toolboxes.execute(config.getRuntime(), new UiApiToolbox.Context(user, null, config.getRuntime().getUiApiGateway(), policy == null ? PolicyService.ToolPolicy.defaults() : policy, write -> config.getRuntime().getStore().insert(new AssistantPendingAction().withUserId(user).withToolName(write.toolName()).withTableName(write.tableName()) .withRecordId(write.recordId()).withUiApiOperation(write.operation()).withParamsJson(com.kingsrook.qqq.backend.core.utils.JsonUtils.toJson(write.values())) .withSummary(write.summary()).withStatus(AssistantPendingAction.STATUS_PENDING) @@ -218,11 +218,27 @@ class UiApiToolsSecurityTest extends BaseAssistantTest /******************************************************************************* - ** + ** A write the way it always runs now: the tool call only proposes it (no + ** request is made), and the person's confirmation runs it through the UI + ** API with their own session. The answer is the confirm route's: the + ** action's status, the HTTP status of the write, the outcome and message. *******************************************************************************/ - private static PolicyService.ToolPolicy noConfirmation() + private Map proposedThenConfirmed(String user, String toolName, Map in) throws Exception { - return (new PolicyService.ToolPolicy(false, Set.of(), 50, true)); + UiApiToolbox.Outcome proposed = tool(user, null, toolName, in); + assertThat(proposed.getResult().getOutcome()).as(toolName + " proposes").isEqualTo(AiToolOutcome.OK); + assertThat(proposed.getPendingActionId()).as(toolName + " proposes").isNotNull(); + assertThat(proposed.getHttpStatus()).as("nothing runs before confirmation").isNull(); + + QContext.init(qInstance, AssistantTestFixture.sessionFor(user)); + try + { + return (new AssistantApi(config.getRuntime(), QContext.getQSession()).confirmAction(proposed.getPendingActionId())); + } + finally + { + QContext.clear(); + } } @@ -264,8 +280,8 @@ class UiApiToolsSecurityTest extends BaseAssistantTest /******************************************************************************* - ** No insert permission: even with data access READ_WRITE and no - ** confirmation, the write gets the UI's 403. + ** No insert permission: even with data access READ_WRITE, the write the + ** person confirms gets the UI's 403. *******************************************************************************/ @Test void testWriteDeniedIsTheSameDenial() throws Exception @@ -274,13 +290,14 @@ class UiApiToolsSecurityTest extends BaseAssistantTest HttpResponse ui = ui("POST", "/table/blog_post", AssistantTestFixture.EDITOR, "{\"title\":\"From the UI\"}"); assertThat(ui.statusCode()).isEqualTo(403); - UiApiToolbox.Outcome outcome = tool(AssistantTestFixture.EDITOR, noConfirmation(), UiApiToolbox.CREATE_RECORD, + Map outcome = proposedThenConfirmed(AssistantTestFixture.EDITOR, UiApiToolbox.CREATE_RECORD, input("table", "blog_post", "values", Map.of("title", "From the assistant"))); - assertThat(outcome.getHttpStatus()).isEqualTo(403); - assertThat(outcome.getResult().getOutcome()).isEqualTo(AiToolOutcome.DENIED); - assertThat(outcome.getResult().getModelContent()).contains(new UiApiResult(ui.statusCode(), ui.body()).errorMessage()); + assertThat(outcome.get("httpStatus")).isEqualTo(403); + assertThat(outcome.get("status")).isEqualTo(AssistantPendingAction.STATUS_FAILED); + assertThat(outcome.get("outcome")).isEqualTo(AiToolOutcome.DENIED.name()); + assertThat(String.valueOf(outcome.get("message"))).contains(new UiApiResult(ui.statusCode(), ui.body()).errorMessage()); - assertThat(tool(AssistantTestFixture.EDITOR, noConfirmation(), UiApiToolbox.DELETE_RECORD, input("table", "blog_post", "id", "1")).getHttpStatus()) + assertThat(proposedThenConfirmed(AssistantTestFixture.EDITOR, UiApiToolbox.DELETE_RECORD, input("table", "blog_post", "id", "1")).get("httpStatus")) .isEqualTo(ui("DELETE", "/table/blog_post/1", AssistantTestFixture.EDITOR, null).statusCode()); } @@ -306,7 +323,7 @@ class UiApiToolsSecurityTest extends BaseAssistantTest // the admin may insert in the UI, but blog_post is READ (the // // default): the assistant refuses before any request // ////////////////////////////////////////////////////////////////// - UiApiToolbox.Outcome readOnly = tool(AssistantTestFixture.ADMIN, noConfirmation(), UiApiToolbox.CREATE_RECORD, input("table", "blog_post", "values", Map.of("title", "x"))); + UiApiToolbox.Outcome readOnly = tool(AssistantTestFixture.ADMIN, null, UiApiToolbox.CREATE_RECORD, input("table", "blog_post", "values", Map.of("title", "x"))); assertThat(readOnly.getResult().getOutcome()).isEqualTo(AiToolOutcome.DENIED); assertThat(readOnly.getHttpStatus()).isNull(); assertThat(readOnly.getResult().getModelContent()).contains("READ"); @@ -435,11 +452,11 @@ class UiApiToolsSecurityTest extends BaseAssistantTest @Test void testPolicy() { - PolicyService.ToolPolicy onlyCount = new PolicyService.ToolPolicy(true, Set.of(UiApiToolbox.COUNT_RECORDS), 2, true); + PolicyService.ToolPolicy onlyCount = new PolicyService.ToolPolicy(Set.of(UiApiToolbox.COUNT_RECORDS), 2, true); assertThat(tool(AssistantTestFixture.EDITOR, onlyCount, UiApiToolbox.QUERY_RECORDS, input("table", "blog_post", "filter", filter("AND"))).getResult().getOutcome()) .isEqualTo(AiToolOutcome.DENIED); - PolicyService.ToolPolicy twoRows = new PolicyService.ToolPolicy(true, Set.of(), 2, true); + PolicyService.ToolPolicy twoRows = new PolicyService.ToolPolicy(Set.of(), 2, true); UiApiToolbox.Outcome capped = tool(AssistantTestFixture.EDITOR, twoRows, UiApiToolbox.QUERY_RECORDS, input("table", "blog_post", "filter", filter("AND"), "limit", 50)); assertThat(capped.getResult().getRowCount()).isEqualTo(2); assertThat(capped.getResult().getModelContent()).contains("\"hasMore\":true").doesNotContain(AssistantTestFixture.HIDDEN_SENTINEL).doesNotContain(AssistantTestFixture.PASSWORD_SENTINEL); @@ -451,8 +468,9 @@ class UiApiToolsSecurityTest extends BaseAssistantTest ** The owner's matrix: a user without permission on a table gets the ** identical denial (status and message) from the UI API and through the ** assistant, for query, get, insert, update and delete. The table is - ** READ_WRITE and confirmation is off, so nothing on the assistant's side - ** stops the call first: the UI API decides. Processes: the assistant has + ** READ_WRITE, so nothing on the assistant's side stops a read first, and a + ** write is proposed and then confirmed (a write never runs without that): + ** the UI API decides. Processes: the assistant has ** no process tool and process data access defaults to NONE, so it can run ** none (the matrix row asserts that). *******************************************************************************/ @@ -474,14 +492,31 @@ class UiApiToolsSecurityTest extends BaseAssistantTest { @SuppressWarnings("unchecked") HttpResponse direct = (HttpResponse) row[1]; - UiApiToolbox.Outcome assistant = tool(user, noConfirmation(), (String) row[2], (Map) row[3]); + String toolName = (String) row[2]; + Integer httpStatus; + String outcome; + String content; + if(UiApiToolbox.WRITE_TOOLS.contains(toolName)) + { + Map confirmed = proposedThenConfirmed(user, toolName, (Map) row[3]); + httpStatus = (Integer) confirmed.get("httpStatus"); + outcome = (String) confirmed.get("outcome"); + content = String.valueOf(confirmed.get("message")); + } + else + { + UiApiToolbox.Outcome assistant = tool(user, null, toolName, (Map) row[3]); + httpStatus = assistant.getHttpStatus(); + outcome = assistant.getResult().getOutcome().name(); + content = assistant.getResult().getModelContent(); + } assertThat(direct.statusCode()).as(row[0] + " via the UI").isBetween(400, 499); - assertThat(assistant.getHttpStatus()).as(row[0] + " via the assistant").isEqualTo(direct.statusCode()); - assertThat(assistant.getResult().getOutcome()).as(String.valueOf(row[0])).isIn(AiToolOutcome.DENIED, AiToolOutcome.NOT_FOUND); + assertThat(httpStatus).as(row[0] + " via the assistant").isEqualTo(direct.statusCode()); + assertThat(outcome).as(String.valueOf(row[0])).isIn(AiToolOutcome.DENIED.name(), AiToolOutcome.NOT_FOUND.name()); String message = new UiApiResult(direct.statusCode(), direct.body()).errorMessage(); if(message != null) { - assertThat(assistant.getResult().getModelContent()).as(row[0] + " message").contains(message); + assertThat(content).as(row[0] + " message").contains(message); } } diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadToolsTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadToolsTest.java index 50c4986..98bb75f 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadToolsTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/bulkload/BulkLoadToolsTest.java @@ -36,6 +36,7 @@ import com.kingsrook.qbits.assistant.testing.AssistantTestFixture; import com.kingsrook.qbits.assistant.testing.BaseAssistantTest; import com.kingsrook.qbits.assistant.testing.TurnRunner; import com.kingsrook.qbits.assistant.tools.UiApiToolbox; +import com.kingsrook.qbits.assistant.tools.registry.ToolFamily; import com.kingsrook.qbits.assistant.uiapi.UiApiCall; import com.kingsrook.qqq.backend.core.actions.ai.AiToolOutcome; import com.kingsrook.qqq.backend.core.context.QContext; @@ -122,6 +123,24 @@ class BulkLoadToolsTest extends BaseAssistantTest + /******************************************************************************* + ** The runtime's bulk-load tools: main code outside the tools packages has no + ** way to them, the family of this package has. + *******************************************************************************/ + private BulkLoadTools bulkLoadTools() + { + for(ToolFamily family : config.getRuntime().ownToolFamilies()) + { + if(family instanceof BulkLoadToolFamily bulkLoad) + { + return (bulkLoad.bulkLoadTools()); + } + } + throw (new IllegalStateException("The runtime has no bulk-load tool family.")); + } + + + /******************************************************************************* ** One tool call as the turn engine makes it. *******************************************************************************/ @@ -130,7 +149,7 @@ class BulkLoadToolsTest extends BaseAssistantTest QContext.init(qInstance, AssistantTestFixture.ownerSession(AssistantTestFixture.ADMIN, config)); try { - return (config.getRuntime().getBulkLoadTools().execute(new UiApiToolbox.Context("session-1", null, server, PolicyService.ToolPolicy.defaults(), + return (bulkLoadTools().execute(new UiApiToolbox.Context("session-1", null, server, PolicyService.ToolPolicy.defaults(), write -> config.getRuntime().getStore().insert(new AssistantPendingAction().withUserId(AssistantTestFixture.ADMIN).withToolName(write.toolName()) .withTableName(write.tableName()).withUiApiOperation(write.operation()).withParamsJson(JsonUtils.toJson(write.values())) .withSummary(write.summary()).withStatus(AssistantPendingAction.STATUS_PENDING).withExpiresAt(java.time.Instant.now().plusSeconds(600))).getId()), @@ -152,9 +171,9 @@ class BulkLoadToolsTest extends BaseAssistantTest { List names = config.getRuntime().getContextBuilder().tools().stream().map(d -> d.getName()).toList(); assertThat(names).contains(BulkLoadTools.LIST, BulkLoadTools.DESCRIBE, BulkLoadTools.ATTACH, BulkLoadTools.SUGGEST, BulkLoadTools.DRY_RUN); - assertThat(config.getRuntime().getBulkLoadTools().definitions(PolicyService.ToolPolicy.defaults(), true)) + assertThat(bulkLoadTools().definitions(PolicyService.ToolPolicy.defaults(), true)) .extracting(d -> d.getName()).containsExactlyInAnyOrderElementsOf(BulkLoadTools.NAMES); - assertThat(config.getRuntime().getBulkLoadTools().definitions(PolicyService.ToolPolicy.defaults(), false)) + assertThat(bulkLoadTools().definitions(PolicyService.ToolPolicy.defaults(), false)) .extracting(d -> d.getName()).doesNotContain(BulkLoadTools.SAVE, BulkLoadTools.RUN); } diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ChatToolSetTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ChatToolSetTest.java index c265a2e..69d0865 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ChatToolSetTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ChatToolSetTest.java @@ -251,7 +251,7 @@ class ChatToolSetTest extends BaseAssistantTest { scenarios.put("everything", describe(config.getRuntime().getContextBuilder().tools())); scenarios.put("everythingNoPolicy", describe(config.getRuntime().getContextBuilder().tools(null))); - scenarios.put("restrictedByPolicy", describe(config.getRuntime().getContextBuilder().tools(new PolicyService.ToolPolicy(true, RESTRICTED, 20, true)))); + scenarios.put("restrictedByPolicy", describe(config.getRuntime().getContextBuilder().tools(new PolicyService.ToolPolicy(RESTRICTED, 20, true)))); config.setStrictToolNames(STRICT); scenarios.put("strictToolNames", describe(config.getRuntime().getContextBuilder().tools())); diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallArchitectureTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallArchitectureTest.java index f95248c..322fc94 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallArchitectureTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ToolCallArchitectureTest.java @@ -17,12 +17,26 @@ package com.kingsrook.qbits.assistant.tools.registry; +import java.util.Set; +import java.util.regex.Pattern; +import com.kingsrook.qbits.assistant.AssistantRuntime; +import com.kingsrook.qbits.assistant.SignedInPerson; +import com.kingsrook.qbits.assistant.api.AssistantApi; +import com.kingsrook.qbits.assistant.api.specs.AssistantSpecs; +import com.kingsrook.qbits.assistant.engine.StarterPrompts; import com.kingsrook.qbits.assistant.engine.ToolCallService; import com.kingsrook.qbits.assistant.engine.TurnEngine; import com.kingsrook.qbits.assistant.model.AssistantToolCall; +import com.kingsrook.qbits.assistant.rag.RagService; +import com.kingsrook.qbits.assistant.tools.FilterLinkBuilder; +import com.kingsrook.qbits.assistant.tools.UiApiToolbox; +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.JavaCall; +import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.core.importer.ImportOption; +import io.javalin.http.Context; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses; @@ -42,6 +56,25 @@ class ToolCallArchitectureTest private static final String BASE = "com.kingsrook.qbits.assistant.."; private static final String TOOLS = "com.kingsrook.qbits.assistant.tools.."; + private static final String TOOLS_PACKAGE = "com.kingsrook.qbits.assistant.tools"; + private static final String REGISTRY_PACKAGE = "com.kingsrook.qbits.assistant.tools.registry"; + + /////////////////////////////////////////////////////////////////////////// + // what a class outside the tools packages may use of them, apart from // + // the registry: the value types a result is read from, and the link // + // builder (no tool is run through any of them) // + /////////////////////////////////////////////////////////////////////////// + private static final Set VALUE_TYPES = Set.of( + UiApiToolbox.Context.class.getName(), UiApiToolbox.Outcome.class.getName(), UiApiToolbox.ProposedWrite.class.getName(), UiApiToolbox.PendingActions.class.getName(), + FilterLinkBuilder.class.getName()); + + /////////////////////////////////////////////////////////////////////////// + // the only classes that make a tool context: the service, for a tool // + // call; the API and the starter prompts, and the knowledge index, for // + // the assistant's own reads // + /////////////////////////////////////////////////////////////////////////// + private static final Set CONTEXT_MAKERS = Set.of(ToolCallService.class.getName(), AssistantApi.class.getName(), StarterPrompts.class.getName(), RagService.class.getName()); + private static JavaClasses classes; @@ -98,10 +131,188 @@ class ToolCallArchitectureTest void onlyTheTurnEngineRunsACallAndLogsItInTwoSteps() { noClasses().that().resideInAPackage(BASE).and().doNotHaveFullyQualifiedName(ToolCallService.class.getName()).and().doNotHaveFullyQualifiedName(TurnEngine.class.getName()) - .should().callMethod(ToolCallService.class, "execute", ToolCallRequest.class) - .orShould().callMethod(ToolCallService.class, "log", ToolCallRequest.class, ToolCallOutcome.class) + .should().callMethod(ToolCallService.class, "execute", ToolCall.class, ToolCallScope.class) + .orShould().callMethod(ToolCallService.class, "log", ToolCall.class, ToolCallScope.class, ToolCallOutcome.class) .because("a tool call that is run is logged: callers other than the turn engine use ToolCallService.executeAndLog") .check(classes); } + + + /******************************************************************************* + ** What a tool is run with (the context, with its policy, gateway and + ** credentials) and what a confirmed write is run with (the table, the + ** record, the values) are made by the service, from what it found out + ** itself. No caller can make one and hand it over. + *******************************************************************************/ + @Test + void onlyTheServiceMakesWhatAToolRunsWith() + { + noClasses().that().resideInAPackage(BASE).and().doNotHaveFullyQualifiedName(ToolCallService.class.getName()) + .should().callConstructorWhere(target(owner(ToolCallRequest.class.getName()).or(owner(ConfirmedToolCall.class.getName())))) + .because("a tool's request and a confirmed write are built by ToolCallService, never by its caller") + .check(classes); + } + + + + /******************************************************************************* + ** A confirmed write is run for the confirm route, by nothing else. + *******************************************************************************/ + @Test + void onlyTheAssistantApiConfirms() + { + noClasses().that().resideInAPackage(BASE).and().doNotHaveFullyQualifiedName(AssistantApi.class.getName()) + .should().callMethod(ToolCallService.class, "confirm", SignedInPerson.class, Integer.class) + .because("a pending action is confirmed by the person, through POST /actions/{id}/confirm") + .check(classes); + } + + + + /******************************************************************************* + ** A request's session and its credentials are bound in one place: the + ** assistant's route handler, where the session setup has just run. + *******************************************************************************/ + @Test + void onlyTheRouteBindsARequestsCredentialsToItsSession() + { + noClasses().that().resideInAPackage(BASE).and().haveNameNotMatching(Pattern.quote(AssistantSpecs.class.getName()) + "(\\$.*)?") + .should().callMethod(SignedInPerson.class, "ofRequest", Context.class) + .because("only the route's session setup knows that a session and a request's credentials are the same person's") + .check(classes); + } + + + + /******************************************************************************* + ** The toolboxes cannot be called around the service. Outside the tools + ** packages a tool is reached through the registry's interfaces and through + ** nothing else: no class calls a method or constructor of a toolbox, a tool + ** or a family, except + ** - the runtime, which builds them and hands them to their families; + ** - the value types a result or a proposed write is read from (a tool + ** context is made only by the service and by the assistant's own reads); + ** - UiApiToolbox.read, the assistant's own reads (four read tools, never a + ** change), for the starter prompts and the knowledge index; + ** - UiApiToolbox.notAWriteTool, a refusal the service builds. + *******************************************************************************/ + @Test + void nothingOutsideTheToolsCallsIntoThemExceptThroughTheRegistry() + { + noClasses().that().resideInAPackage(BASE).and().resideOutsideOfPackage(TOOLS) + .should().callCodeUnitWhere(new DescribedPredicate>("a member of the tools packages that is not the registry's and not allowed to that caller") + { + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public boolean test(JavaCall call) + { + String targetPackage = call.getTargetOwner().getPackageName(); + boolean inTools = targetPackage.equals(TOOLS_PACKAGE) || targetPackage.startsWith(TOOLS_PACKAGE + "."); + boolean inRegistry = targetPackage.equals(REGISTRY_PACKAGE) || targetPackage.startsWith(REGISTRY_PACKAGE + "."); + return (inTools && !inRegistry && !allowed(call)); + } + }) + .because("a tool is run by ToolCallService through the registry; a toolbox called directly would skip its checks and its log row") + .check(classes); + } + + + + /******************************************************************************* + ** A knowledge hit is read again (get_record with the asking person's + ** session, with no log row of its own) for the knowledge tool and the + ** index, and for nothing else: it is not a way to read a record around the + ** service. + *******************************************************************************/ + @Test + void onlyTheKnowledgeToolReadsAHitAgain() + { + noClasses().that().resideInAPackage(BASE).and().resideOutsideOfPackage(TOOLS).and().doNotHaveFullyQualifiedName(RagService.class.getName()) + .should().callMethod(RagService.class, "read", UiApiToolbox.Context.class, String.class, String.class) + .because("RagService.read serves the knowledge tool, whose call the service checked and logs") + .check(classes); + } + + + + /******************************************************************************* + ** Whether a call from outside the tools packages into them is one of the + ** few that run no tool call of a model. + *******************************************************************************/ + private static boolean allowed(JavaCall call) + { + String owner = call.getTargetOwner().getName(); + String name = call.getTarget().getName(); + String origin = call.getOriginOwner().getName(); + if(owner.equals(UiApiToolbox.Context.class.getName()) && "".equals(name)) + { + return (CONTEXT_MAKERS.stream().anyMatch(maker -> origin.equals(maker) || origin.startsWith(maker + "$"))); + } + if(VALUE_TYPES.contains(owner)) + { + return (true); + } + if(origin.equals(AssistantRuntime.class.getName()) || origin.startsWith(AssistantRuntime.class.getName() + "$")) + { + /////////////////////////////////////////////////////////////////// + // the runtime builds the toolboxes and their families // + /////////////////////////////////////////////////////////////////// + return ("".equals(name) || "withScheduledDeliveryCheck".equals(name)); + } + if(owner.equals(UiApiToolbox.class.getName())) + { + return (switch(name) + { + case "read" -> origin.startsWith(StarterPrompts.class.getName()) || origin.startsWith(RagService.class.getName()); + case "" -> origin.startsWith(RagService.class.getName()); + case "notAWriteTool" -> origin.equals(ToolCallService.class.getName()); + default -> false; + }); + } + return (false); + } + + + + /******************************************************************************* + ** A call's target, by the class that declares it. + *******************************************************************************/ + private static DescribedPredicate> target(DescribedPredicate owner) + { + return (new DescribedPredicate<>("target owner " + owner.getDescription()) + { + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public boolean test(JavaCall call) + { + return (owner.test(call.getTargetOwner())); + } + }); + } + + + + /******************************************************************************* + ** A class by its name. + *******************************************************************************/ + private static DescribedPredicate owner(String className) + { + return (new DescribedPredicate<>(className) + { + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public boolean test(JavaClass javaClass) + { + return (javaClass.getName().equals(className)); + } + }); + } + } diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ToolRegistryTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ToolRegistryTest.java index 702283c..12ed7fd 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ToolRegistryTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/tools/registry/ToolRegistryTest.java @@ -32,6 +32,7 @@ import com.kingsrook.qbits.assistant.provider.fake.ScriptedModelProvider; import com.kingsrook.qbits.assistant.spi.ModelToolDefinition; import com.kingsrook.qbits.assistant.testing.AssistantTestFixture; import com.kingsrook.qbits.assistant.testing.BaseAssistantTest; +import com.kingsrook.qbits.assistant.testing.ToolCalls; import com.kingsrook.qbits.assistant.tools.AnalysisTools; import com.kingsrook.qbits.assistant.tools.ClasspathOnlyToolFamily; import com.kingsrook.qbits.assistant.tools.KnowledgeTool; @@ -115,7 +116,7 @@ class ToolRegistryTest extends BaseAssistantTest @Test void onlyTheFamiliesGivenAreRegistered() { - ToolRegistry own = ToolRegistry.of(config.getRuntime(), AssistantRuntime.ownToolFamilies()); + ToolRegistry own = ToolRegistry.of(config.getRuntime(), config.getRuntime().ownToolFamilies()); assertThat(own.tools()).extracting(AssistantTool::name).containsExactlyElementsOf(KINDS.keySet()); ToolRegistry kernelOnly = ToolRegistry.of(config.getRuntime(), List.of(new KernelToolFamily())); @@ -139,7 +140,7 @@ class ToolRegistryTest extends BaseAssistantTest assertThat(config.getRuntime().getToolRegistry().tools()).extracting(AssistantTool::name).containsExactlyElementsOf(KINDS.keySet()); assertThat(offeredToTheEditor(qInstance, config)).doesNotContain(ClasspathOnlyToolFamily.NAME); - ToolCallOutcome outcome = config.getRuntime().getToolCalls().execute(request(config, ClasspathOnlyToolFamily.NAME)); + ToolCallOutcome outcome = config.getRuntime().getToolCalls().execute(ToolCalls.call(config, AssistantTestFixture.EDITOR, ClasspathOnlyToolFamily.NAME, Map.of()), ToolCalls.scope()); assertThat(outcome.result().getOutcome()).isEqualTo(AiToolOutcome.INVALID_INPUT); assertThat(outcome.result().getSummary()).isEqualTo("Unknown tool"); } @@ -163,7 +164,7 @@ class ToolRegistryTest extends BaseAssistantTest assertThat(hostConfig.getRuntime().getToolRegistry().tools()).extracting(AssistantTool::name).contains(ClasspathOnlyToolFamily.NAME).containsAll(KINDS.keySet()).hasSize(KINDS.size() + 1); assertThat(offeredToTheEditor(host, hostConfig)).contains(ClasspathOnlyToolFamily.NAME); - ToolCallOutcome outcome = hostConfig.getRuntime().getToolCalls().execute(request(hostConfig, ClasspathOnlyToolFamily.NAME)); + ToolCallOutcome outcome = hostConfig.getRuntime().getToolCalls().execute(ToolCalls.call(hostConfig, AssistantTestFixture.EDITOR, ClasspathOnlyToolFamily.NAME, Map.of()), ToolCalls.scope()); assertThat(outcome.result().getOutcome()).isEqualTo(AiToolOutcome.OK); assertThat(outcome.result().getModelContent()).isEqualTo("ran"); } @@ -257,17 +258,6 @@ class ToolRegistryTest extends BaseAssistantTest - /******************************************************************************* - ** A call of the editor's to a tool, outside a turn. - *******************************************************************************/ - private static ToolCallRequest request(AssistantQBitConfig config, String toolName) - { - UiApiToolbox.Context context = new UiApiToolbox.Context(AssistantTestFixture.EDITOR, "UTC", config.getRuntime().getUiApiGateway(), PolicyService.ToolPolicy.defaults(), null); - return (new ToolCallRequest(AssistantTestFixture.ownerSession(AssistantTestFixture.EDITOR, config), context, "call-1", toolName, Map.of(), null)); - } - - - /******************************************************************************* ** Two tools of one name, or a tool without a name, kind or checks, stop the ** runtime from starting. @@ -288,7 +278,7 @@ class ToolRegistryTest extends BaseAssistantTest assertThatThrownBy(() -> new ToolRegistry(List.of())).isInstanceOf(IllegalStateException.class).hasMessageContaining("no tools"); assertThatThrownBy(() -> ToolRegistry.of(config.getRuntime(), List.of())).isInstanceOf(IllegalStateException.class).hasMessageContaining("no tools"); - List clash = new ArrayList<>(AssistantRuntime.ownToolFamilies()); + List clash = new ArrayList<>(config.getRuntime().ownToolFamilies()); clash.add(new KernelToolFamily()); assertThatThrownBy(() -> ToolRegistry.of(config.getRuntime(), clash)).isInstanceOf(IllegalStateException.class).hasMessageContaining("navigate"); } @@ -303,7 +293,7 @@ class ToolRegistryTest extends BaseAssistantTest void aCallToANameWithoutAToolIsRefused() { ToolCallService calls = config.getRuntime().getToolCalls(); - ToolCallOutcome outcome = calls.execute(new ToolCallRequest(AssistantTestFixture.sessionFor(AssistantTestFixture.EDITOR), context(), "call-1", "no_such_tool", Map.of(), null)); + ToolCallOutcome outcome = calls.execute(ToolCalls.call(config, AssistantTestFixture.EDITOR, "no_such_tool", Map.of()), ToolCalls.scope()); assertThat(outcome.extras()).isNull(); assertThat(outcome.result().getOutcome()).isEqualTo(AiToolOutcome.INVALID_INPUT); @@ -317,27 +307,28 @@ class ToolRegistryTest extends BaseAssistantTest /******************************************************************************* ** A confirmation for a tool that proposes nothing (of any family, or of ** none) gets the refusal the confirm route always gave: the record write's - ** own "not a write tool". + ** own "not a write tool". The service is the only main code that asks a + ** tool this, and it does so for the stored pending action it claimed. *******************************************************************************/ @Test void aToolThatProposesNothingCannotBeConfirmed() { for(String name : List.of(UiApiToolbox.LIST_TABLES, AnalysisTools.ANALYZE_RECORDS, BulkLoadTools.LIST, KnowledgeTool.NAME, PresentResultsTool.NAME, "navigate", "no_such_tool")) { - UiApiToolbox.Outcome before = config.getRuntime().getToolbox().executeConfirmedWrite(context(), "call-1", name, "blog_post", "1", Map.of("title", "x")); - UiApiToolbox.Outcome now = config.getRuntime().getToolCalls().executeConfirmed(new ConfirmedToolCall(AssistantTestFixture.sessionFor(AssistantTestFixture.EDITOR), context(), "call-1", name, "blog_post", "1", Map.of("title", "x"), AssistantTestFixture.EDITOR)); + AssistantTool tool = config.getRuntime().getToolRegistry().find(name); + UiApiToolbox.Outcome now = tool == null ? UiApiToolbox.notAWriteTool("call-1", name, "blog_post") + : tool.executeConfirmed(new ConfirmedToolCall(AssistantTestFixture.sessionFor(AssistantTestFixture.EDITOR), context(), "call-1", name, "blog_post", "1", Map.of("title", "x"), AssistantTestFixture.EDITOR)); - assertThat(now.getResult().getOutcome()).as(name).isEqualTo(before.getResult().getOutcome()).isEqualTo(AiToolOutcome.INVALID_INPUT); - assertThat(now.getResult().getSummary()).as(name).isEqualTo(before.getResult().getSummary()); - assertThat(now.getResult().getModelContent()).as(name).isEqualTo(before.getResult().getModelContent()).isEqualTo("Not a write tool: " + name); - assertThat(now.getResult().getToolCallId()).isEqualTo(before.getResult().getToolCallId()); - assertThat(now.getResult().getToolName()).isEqualTo(before.getResult().getToolName()); - assertThat(now.getResult().getTableName()).isEqualTo(before.getResult().getTableName()); - assertThat(now.getResult().getPermissionCheck()).isEqualTo(before.getResult().getPermissionCheck()); - assertThat(now.getOperation()).as(name).isEqualTo(before.getOperation()).isNull(); - assertThat(now.getHttpStatus()).isEqualTo(before.getHttpStatus()).isNull(); - assertThat(now.getPendingActionId()).isEqualTo(before.getPendingActionId()).isNull(); - assertThat(now.isFailed()).isEqualTo(before.isFailed()); + assertThat(now.getResult().getOutcome()).as(name).isEqualTo(AiToolOutcome.INVALID_INPUT); + assertThat(now.getResult().getModelContent()).as(name).isEqualTo("Not a write tool: " + name); + assertThat(now.getResult().getToolCallId()).isEqualTo("call-1"); + assertThat(now.getResult().getToolName()).isEqualTo(name); + assertThat(now.getResult().getTableName()).isEqualTo("blog_post"); + assertThat(now.getResult().getPermissionCheck()).isNull(); + assertThat(now.getOperation()).as(name).isNull(); + assertThat(now.getHttpStatus()).isNull(); + assertThat(now.getPendingActionId()).isNull(); + assertThat(now.isFailed()).isFalse(); } } @@ -361,7 +352,7 @@ class ToolRegistryTest extends BaseAssistantTest } assertThat(asked).hasSize(1); assertThat(offer.allows("anything")).isTrue(); - assertThat(new ToolOffer(new PolicyService.ToolPolicy(true, java.util.Set.of("navigate"), 20, true), null, null).allows("anything")).isFalse(); + assertThat(new ToolOffer(new PolicyService.ToolPolicy(java.util.Set.of("navigate"), 20, true), null, null).allows("anything")).isFalse(); assertThat(offer.turnId()).isEqualTo("turn-1"); } -- 2.54.0