feat(access): writes follow the signed-in person; no answer without a spending limit; a fourth review of the tool boundary closed #109

Merged

james.maes merged 4 commits from feat/writes-follow-the-person into develop 2026-11-12 19:26:47 +00:00

Conversation 0 Commits 4 Files Changed 63

+5132 -658

james.maes commented 2026-11-12 19:18:50 +00:00

Owner

Copy link

Copy source

Package A2 of the actions design (docs/actions-and-mcp.md, sections 2, 3 and 12.2): the assistant may propose any change the signed-in person may make themselves, by default, with the defaults declared in versioned code. With it, two owner decisions of 2026-11-12 and five findings of the last independent review of the tool boundary, each closed by the guarantee it gives.

Two behaviour changes need an administrator before deploying. UPGRADE.md has the steps.

What changes

Writes follow the person

• The data access of a table or process is declared in code: AssistantQBitConfig.withDefaultTableAccess / withDefaultProcessAccess (both READ_WRITE) and a pin per object, withTableAccess / withProcessAccess. READ_WRITE grants nothing: the assistant may propose what the person may do themselves; every change still waits for the person's confirmation card and runs as that person through the UI API.
• An administrator's assistant_data_access row can only narrow: READ (the assistant never proposes a change to the table) or NONE (the assistant never sees it). A row never raises a code default, a pin, or a table whose metadata keeps it from AI.
• Every assistant_* table and every process of the QBit is NONE by a pin in code that no row and no host pin lifts (DataAccessService.isOwnTable), in addition to core's channel wall.
• The write tools are offered to a person who may create, edit or delete records of some table (asked of the UI API as that person, once per offer, within toolCallTimeoutSeconds; no answer means no write tool). GET /config features.writes is that same answer for the person.
• Settings that cannot be read, and were never read, allow nothing (the fallback used to be the old default).
• The table action "Add rows for every table" is retired with its help and tests. The Data access card already shows the effective mode and its source for every table without storing anything.

No answer without a spending limit

• A person for whom no spending limit is in force is refused: the turn before anything is stored and before any model call (429 BUDGET, "The assistant is not answering because no spending limit is in force. An administrator must set or switch on a budget (Assistant > Budgets) before it can be used."), every later model call of a running answer, every tool call, every embedding request, and a scheduled turn (its run records the sentence). Restoring a limit restores service on the next request.
• "In force" (BudgetService.limitInForce): a hard limit above zero in the code configuration or on the policy row, or a budget row that is switched on, has a limit above zero and applies to the person (shared: INSTANCE, or PROVIDER / MODEL in use; their own: USER, or a ROLE they hold). Not in force: a row that is switched off, a zero limit, a row for somebody else, a soft limit alone. A budget row has no end date. An override is a dated exception to a budget that stays in force (see "For the owner to confirm").

The migration

db/liquibase/qbit-assistant/assistant-data-access-follow-user.changelog.sql, one changeset, included once by the host:

DELETE FROM assistant_data_access
WHERE object_type = 'TABLE' AND mode = 'READ'
  AND notes = '<the retired action''s fixed note>'
  AND create_date IS NOT NULL AND modify_date IS NOT NULL
  AND modify_date < create_date + INTERVAL '1' SECOND;

It removes the unedited READ rows the retired action wrote, which would otherwise now read as restrictions, and no other row: not a row an administrator wrote or changed, not a NONE or READ_WRITE row, not a PROCESS row. Guarded by a precondition (a host without the table marks it ran); rollback empty. Without it nothing breaks: those tables stay read-only for the assistant until the rows are deleted.

Review findings

#	Guarantee	Test (fails with the old behaviour put back, passes now)
1	A pending action is never marked expired over its outcome. Expiry is a decision made through the same claim as confirming and cancelling (AssistantStore.expirePendingAction(userId, id, now); ToolCallService.expiredIfDue decides from the stored action and the runtime's clock).	PendingActionExpiryTest: a list that read the action before it was confirmed; a confirmation and an expiry at once on two threads, twenty rounds; expiry refused for a running, cancelled or expired action
2	No action stays RUNNING for ever. Retention first ends one left running for more than an hour as FAILED ("Outcome unknown ... Check the record before asking for it again."), with a tool-call row; it then goes with the rest after approvalRetentionDays.	PendingActionExpiryTest.anActionLeftRunningIsEndedAsFailedAndThenRemoved, theRetentionProcessEndsActionsLeftRunning
3	A person is the session the authentication module resolved, not the object passed in.	SignedInPersonTest: a permission added by hand gives nothing; a revoked permission is seen on the next call
4	The assistant's own reads get their context from the service (ToolCallService.readContext(person, timeout): person, assistant access, kill switch, the policy it reads itself, the runtime's gateway). Context.forRead(sessionUUID, ..., gateway, policy, ...) is gone.	AssistantOwnReadsTest; two new rules in ToolCallArchitectureTest
5	The published test-jar holds com/kingsrook/qbits/assistant/testing/** and nothing else.	TestJarContentsTest
small	AssistantStore.decidePendingAction, expirePendingAction and failAbandonedPendingAction are final. The store stays open to subclassing, which this repository's tests use to make a read or a write fail.	PendingActionExpiryTest.expiryIsADecisionLikeTheOthers

Finding 4, log rows: these reads have none, by decision. A starter-prompt read reaches no model (it decides which example questions are shown); a knowledge hit is read again inside a search_knowledge call that has its row; an index run reads every record of its source as the administrator who started it and has its process record. The UI API's access log names the assistant as the client for all of them.

Finding 5, the provider modules: they use AssistantTestFixture, TurnRunner, GoldenTasks, PresentationGoldenTask, EvalSupport and CloudProviderTestServer, all in the testing package, and are unchanged. None of them used the three helpers.

"A hand-built service still runs every check": no document on develop says it. The README and the constructor's javadoc now say what is true: the guarantees are those of the service the runtime built; a service other code constructs checks what its own registry, store and gateway let it check, and cannot be given the runtime's.

Tool lists, before and after

On a host with no optional extension and no settings row (WritesFollowThePersonTest):

	Before	After
A person who may change a table	count_records describe_table get_record list_tables navigate present_results query_records render_widget search_help search_records	the same, plus create_record delete_record update_record
A person who may only read	the same ten	the same ten

Before, the three write tools appeared for everybody, a person who may only read included, as soon as an administrator stored one READ_WRITE row.

Existing expectations that changed

• ChatToolSetTest: no definition and no line of the expected file changed except one. The four "everything" scenarios ask as a person who may change tables, with no settings row (they asked as a person who may only read, with READ_WRITE rows). The scenario of a person who may not save an analysis lost its save_analysis line.
• ToolCallChecksTest: unchanged.
• ToolCallServiceTest: "a write tool while no table is writable" is now "for a person who may change no table"; the person's session is the resolved one, not the object passed in.
• UiApiToolsSecurityTest: a table is read-only for the assistant because of an administrator's row, not by default; an application's processes have the code default, the assistant's own are NONE.
• ProfilesSkillsAndAccessTest, AnalysisSaveTest, PendingActionsTest, AssistantSetupTest, HostToolFamilyTest: the new default, the new refusal wording (it names what decided), and the service's new entry points.
• The fixture declares a shared monthly limit in code, because the assistant refuses with none in force.
• Test only, not an expectation: the random canaries of TranscriptRedactionTest and ScheduledTasksTest have no digits any more. The scan looks for every five characters of a canary, and in the first full build of this branch five digits of a random canary also occurred in a stored timestamp's fraction of a second. Nothing had leaked; the collision is now impossible.

Where the design and the code disagree (the code wins; the design is corrected in this PR)

This branch merges the design branch (draft #104, two commits) so the corrections could be made to the document; #104 can be closed once this is merged.

• Section 0 said a policy can turn confirmation off. It cannot; corrected.
• 3.1: an administrator's row narrows what the code says and never raises it, a code default included. A host that sets the default to READ opens single tables in code.
• 3.1: the write tools follow what the person may change, asked as the person, not "some table is not READ or NONE".
• 3.2: "never edited" is modify_date < create_date + 1 second. Core sets a row's two dates in one insert from two readings of the clock, so create_date = modify_date would match almost no row.
• A2's file list said it touches neither the tool classes nor the store. It does, for the offer that follows the person and for the findings.

For the owner to confirm

• An "unlimited until" override does not take its budget out of force. It is an administrator's dated, reasoned exception to a budget that stays in place, and it always ends. The stricter reading (a person whose only budget is lifted is refused) is one condition in limitInForce.
• The refusal is 429 BUDGET, the code a released panel already shows the message of. GET /config does not announce it up front.

Not closed here

• A change the person may not make is refused when the confirmed change runs, with the UI API's own status and message. For a create that means a card can be shown and then refused. Refusing at the proposal would need one more metadata request per proposal and a message of the assistant's own.
• The write tools' descriptions still end "unless the administrator turned confirmation off". They are part of the cached request prefix and of ChatToolSetTest's expected file; left for a change that touches the prefix on purpose.
• The offer's question to the UI API is one request per offer: two per turn that calls a tool (the tool list, and the service's own check).
• Administrator rows are cached for policyCacheSeconds on servers other than the one that saved them, as before.

Build

./mvnw --batch-mode --no-transfer-progress -s <settings> -T1C clean verify: BUILD SUCCESS on cc5fe2a (all eight modules; 564 tests in qbit-assistant, 687 in all; every coverage check met). The first full run of this branch failed on the random canary collision described above and on nothing else.

🤖 Generated with Claude Code

Package **A2** of the actions design (`docs/actions-and-mcp.md`, sections 2, 3 and 12.2): the assistant may propose any change the signed-in person may make themselves, by default, with the defaults declared in versioned code. With it, two owner decisions of 2026-11-12 and five findings of the last independent review of the tool boundary, each closed by the guarantee it gives. **Two behaviour changes need an administrator before deploying. `UPGRADE.md` has the steps.** ## What changes **Writes follow the person** - The data access of a table or process is declared in code: `AssistantQBitConfig.withDefaultTableAccess` / `withDefaultProcessAccess` (both `READ_WRITE`) and a pin per object, `withTableAccess` / `withProcessAccess`. `READ_WRITE` grants nothing: the assistant may propose what the person may do themselves; every change still waits for the person's confirmation card and runs as that person through the UI API. - An administrator's `assistant_data_access` row can only narrow: `READ` (the assistant never proposes a change to the table) or `NONE` (the assistant never sees it). A row never raises a code default, a pin, or a table whose metadata keeps it from AI. - Every `assistant_*` table and every process of the QBit is `NONE` by a pin in code that no row and no host pin lifts (`DataAccessService.isOwnTable`), in addition to core's channel wall. - The write tools are offered to a person who may create, edit or delete records of some table (asked of the UI API as that person, once per offer, within `toolCallTimeoutSeconds`; no answer means no write tool). `GET /config` `features.writes` is that same answer for the person. - Settings that cannot be read, and were never read, allow nothing (the fallback used to be the old default). - The table action "Add rows for every table" is retired with its help and tests. The Data access card already shows the effective mode and its source for every table without storing anything. **No answer without a spending limit** - A person for whom no spending limit is in force is refused: the turn before anything is stored and before any model call (429 `BUDGET`, "The assistant is not answering because no spending limit is in force. An administrator must set or switch on a budget (Assistant > Budgets) before it can be used."), every later model call of a running answer, every tool call, every embedding request, and a scheduled turn (its run records the sentence). Restoring a limit restores service on the next request. - "In force" (`BudgetService.limitInForce`): a hard limit above zero in the code configuration or on the policy row, or a budget row that is switched on, has a limit above zero and applies to the person (shared: INSTANCE, or PROVIDER / MODEL in use; their own: USER, or a ROLE they hold). Not in force: a row that is switched off, a zero limit, a row for somebody else, a soft limit alone. A budget row has no end date. An override is a dated exception to a budget that stays in force (see "For the owner to confirm"). ## The migration `db/liquibase/qbit-assistant/assistant-data-access-follow-user.changelog.sql`, one changeset, included once by the host: ```sql DELETE FROM assistant_data_access WHERE object_type = 'TABLE' AND mode = 'READ' AND notes = '<the retired action''s fixed note>' AND create_date IS NOT NULL AND modify_date IS NOT NULL AND modify_date < create_date + INTERVAL '1' SECOND; ``` It removes the unedited `READ` rows the retired action wrote, which would otherwise now read as restrictions, and no other row: not a row an administrator wrote or changed, not a `NONE` or `READ_WRITE` row, not a `PROCESS` row. Guarded by a precondition (a host without the table marks it ran); rollback empty. Without it nothing breaks: those tables stay read-only for the assistant until the rows are deleted. ## Review findings | # | Guarantee | Test (fails with the old behaviour put back, passes now) | |---|---|---| | 1 | A pending action is never marked expired over its outcome. Expiry is a decision made through the same claim as confirming and cancelling (`AssistantStore.expirePendingAction(userId, id, now)`; `ToolCallService.expiredIfDue` decides from the stored action and the runtime's clock). | `PendingActionExpiryTest`: a list that read the action before it was confirmed; a confirmation and an expiry at once on two threads, twenty rounds; expiry refused for a running, cancelled or expired action | | 2 | No action stays RUNNING for ever. Retention first ends one left running for more than an hour as `FAILED` ("Outcome unknown ... Check the record before asking for it again."), with a tool-call row; it then goes with the rest after `approvalRetentionDays`. | `PendingActionExpiryTest.anActionLeftRunningIsEndedAsFailedAndThenRemoved`, `theRetentionProcessEndsActionsLeftRunning` | | 3 | A person is the session the authentication module resolved, not the object passed in. | `SignedInPersonTest`: a permission added by hand gives nothing; a revoked permission is seen on the next call | | 4 | The assistant's own reads get their context from the service (`ToolCallService.readContext(person, timeout)`: person, assistant access, kill switch, the policy it reads itself, the runtime's gateway). `Context.forRead(sessionUUID, ..., gateway, policy, ...)` is gone. | `AssistantOwnReadsTest`; two new rules in `ToolCallArchitectureTest` | | 5 | The published test-jar holds `com/kingsrook/qbits/assistant/testing/**` and nothing else. | `TestJarContentsTest` | | small | `AssistantStore.decidePendingAction`, `expirePendingAction` and `failAbandonedPendingAction` are final. The store stays open to subclassing, which this repository's tests use to make a read or a write fail. | `PendingActionExpiryTest.expiryIsADecisionLikeTheOthers` | Finding 4, log rows: these reads have none, by decision. A starter-prompt read reaches no model (it decides which example questions are shown); a knowledge hit is read again inside a `search_knowledge` call that has its row; an index run reads every record of its source as the administrator who started it and has its process record. The UI API's access log names the assistant as the client for all of them. Finding 5, the provider modules: they use `AssistantTestFixture`, `TurnRunner`, `GoldenTasks`, `PresentationGoldenTask`, `EvalSupport` and `CloudProviderTestServer`, all in the `testing` package, and are unchanged. None of them used the three helpers. "A hand-built service still runs every check": no document on `develop` says it. The README and the constructor's javadoc now say what is true: the guarantees are those of the service the runtime built; a service other code constructs checks what its own registry, store and gateway let it check, and cannot be given the runtime's. ## Tool lists, before and after On a host with no optional extension and no settings row (`WritesFollowThePersonTest`): | | Before | After | |---|---|---| | A person who may change a table | `count_records describe_table get_record list_tables navigate present_results query_records render_widget search_help search_records` | the same, plus `create_record delete_record update_record` | | A person who may only read | the same ten | the same ten | Before, the three write tools appeared for everybody, a person who may only read included, as soon as an administrator stored one `READ_WRITE` row. ## Existing expectations that changed - `ChatToolSetTest`: no definition and no line of the expected file changed except one. The four "everything" scenarios ask as a person who may change tables, with no settings row (they asked as a person who may only read, with `READ_WRITE` rows). The scenario of a person who may not save an analysis lost its `save_analysis` line. - `ToolCallChecksTest`: unchanged. - `ToolCallServiceTest`: "a write tool while no table is writable" is now "for a person who may change no table"; the person's session is the resolved one, not the object passed in. - `UiApiToolsSecurityTest`: a table is read-only for the assistant because of an administrator's row, not by default; an application's processes have the code default, the assistant's own are `NONE`. - `ProfilesSkillsAndAccessTest`, `AnalysisSaveTest`, `PendingActionsTest`, `AssistantSetupTest`, `HostToolFamilyTest`: the new default, the new refusal wording (it names what decided), and the service's new entry points. - The fixture declares a shared monthly limit in code, because the assistant refuses with none in force. - Test only, not an expectation: the random canaries of `TranscriptRedactionTest` and `ScheduledTasksTest` have no digits any more. The scan looks for every five characters of a canary, and in the first full build of this branch five digits of a random canary also occurred in a stored timestamp's fraction of a second. Nothing had leaked; the collision is now impossible. ## Where the design and the code disagree (the code wins; the design is corrected in this PR) This branch merges the design branch (draft #104, two commits) so the corrections could be made to the document; #104 can be closed once this is merged. - Section 0 said a policy can turn confirmation off. It cannot; corrected. - 3.1: an administrator's row narrows what the code says and never raises it, a code default included. A host that sets the default to `READ` opens single tables in code. - 3.1: the write tools follow what the person may change, asked as the person, not "some table is not READ or NONE". - 3.2: "never edited" is `modify_date < create_date + 1 second`. Core sets a row's two dates in one insert from two readings of the clock, so `create_date = modify_date` would match almost no row. - A2's file list said it touches neither the tool classes nor the store. It does, for the offer that follows the person and for the findings. ## For the owner to confirm - **An "unlimited until" override does not take its budget out of force.** It is an administrator's dated, reasoned exception to a budget that stays in place, and it always ends. The stricter reading (a person whose only budget is lifted is refused) is one condition in `limitInForce`. - The refusal is `429 BUDGET`, the code a released panel already shows the message of. `GET /config` does not announce it up front. ## Not closed here - A change the person may not make is refused when the confirmed change runs, with the UI API's own status and message. For a create that means a card can be shown and then refused. Refusing at the proposal would need one more metadata request per proposal and a message of the assistant's own. - The write tools' descriptions still end "unless the administrator turned confirmation off". They are part of the cached request prefix and of `ChatToolSetTest`'s expected file; left for a change that touches the prefix on purpose. - The offer's question to the UI API is one request per offer: two per turn that calls a tool (the tool list, and the service's own check). - Administrator rows are cached for `policyCacheSeconds` on servers other than the one that saved them, as before. ## Build `./mvnw --batch-mode --no-transfer-progress -s <settings> -T1C clean verify`: BUILD SUCCESS on `cc5fe2a` (all eight modules; 564 tests in `qbit-assistant`, 687 in all; every coverage check met). The first full run of this branch failed on the random canary collision described above and on nothing else. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 4 commits 2026-11-12 19:18:51 +00:00

docs(design): actions and MCP, the assistant does what the signed-in person may do ... 798acd5541

Draft design and work plan for owner review. Nothing is implemented here.

- Writes follow the person's permissions by default: data access defaults
  declared in code, an administrator can only restrict, every write is
  confirmed on a card that shows the values before and after.
- New tools: run a process the person may run, create and edit saved
  reports and saved analyses, schedule delivery by email.
- An MCP endpoint that serves the same tool registry as chat: read-only
  first, then writes as proposals approved in the application.
- Test strategy, repositories and release order, risks, decisions for the
  owner, and work packages that can be built in parallel.

docs/DESIGN.md gets a pointer under "Next phases".

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(design): name the open write-mode draft the design would replace ... 694309f528

The write-mode switch is not on develop but exists as an open draft
pull request (#21). Say so, and say what happens to it if the owner
confirms that every write is confirmed.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

Merge the actions and MCP design (docs/actions-and-mcp-design) into feat/writes-follow-the-person ... f1cfea7734

The design this branch builds package A2 of. It is merged here so that the
sentences of the design that the code and the owner's decisions of 2026-11-12
have overtaken can be corrected in the same change.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

feat(access): writes follow the signed-in person; no answer without a spending limit; fix: a fourth review of the tool boundary ... cc5fe2a700

Package A2 of the actions design (docs/actions-and-mcp.md, sections 2 and 3),
with two owner decisions of 2026-11-12, and five findings of an independent
review of the tool boundary, each closed by the guarantee it gives.

Writes follow the person
- The data access of a table or process is declared in versioned code:
  AssistantQBitConfig.withDefaultTableAccess / withDefaultProcessAccess (both
  READ_WRITE) and a pin per object (withTableAccess, withProcessAccess).
  READ_WRITE grants nothing: the assistant may propose what the signed-in
  person may do themselves, and every change waits for their confirmation.
- An administrator's assistant_data_access row can only narrow (READ, NONE).
  It never raises a code default, a pin or a table kept from AI.
- Every assistant_* table and every process of the QBit is NONE by a pin in
  code that no row and no host pin lifts.
- The write tools are offered to a person who may create, edit or delete
  records of some table (asked of the UI API as that person, once per offer,
  within the tool-call time limit; no answer means no write tool). GET /config
  features.writes is that answer for the person.
- Settings that cannot be read, and were never read, allow nothing.
- The table action "Add rows for every table" is retired. The changeset
  assistant-data-access-follow-user removes the unedited READ rows it wrote,
  and no other row.

No answer without a spending limit
- A person for whom no spending limit is in force (BudgetService.limitInForce)
  is refused: the turn before anything is stored, every model call, every tool
  call, every embedding request, a scheduled turn. Restoring a limit restores
  service on the next request.

Review findings
- A pending action is never marked expired over its outcome: expiry is a
  decision made through the same claim as confirming and cancelling.
- No action stays RUNNING for ever: retention ends one left running for more
  than an hour as FAILED ("outcome unknown, check the record"), with a log row.
- A person is the session the authentication module resolved, not the object
  passed in: a hand-made permission gives nothing, a revoked one is gone.
- The assistant's own reads get their context from the tool-call service, for
  a person it checked; there is no public way to make a tool context from a
  session id, a gateway and a policy.
- The published test-jar holds the testing package only.
- AssistantStore's decision methods are final.

See CHANGELOG.md and UPGRADE.md (what a host must apply before deploying).

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

james.maes merged commit 1719147114 into develop 2026-11-12 19:26:47 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

architecture

Architecture review: coupling, cohesion, UI API

bug

Something isn't working

dependencies

documentation

Improvements or additions to documentation

duplicate

This issue or pull request already exists

enhancement

New feature or request

github_actions

Pull requests that update GitHub Actions code

good first issue

Good for newcomers

help wanted

Extra attention is needed

invalid

This doesn't seem right

java

Pull requests that update java code

question

Further information is requested

release:rc

Merging this develop PR cuts the next release candidate

secuurity

ux

Admin UX / information architecture

wontfix

This will not be worked on

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

forge-recovery (Forge Recovery) james.maes (James Maes)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QRun-IO/qbit-assistant#109