Let the boundary ask a person, not only say yes or no - #15
Conversation
The action policy had two answers, so every action a deployment was unsure about had to be permanently forbidden or permanently permitted. The cases people actually have are in between: it may do this, but I want to see the first one; it may spend money, but ask me over fifty pounds. The only approximation was to deny the action and have somebody take the wheel, which throws away the Bot's turn and everything it had worked out to reach it. Adds a third list. `ask` is evaluated after `deny` and before `allow`, and both halves of that position are load-bearing. It must not soften a deny, because a thing a deployment has forbidden is not up for renegotiation at a prompt. It must beat allow, because the shipped policy is `allow: ["true"]` and an ask checked afterwards would be unreachable, so the first rule anybody wrote would silently do nothing. A broken ask expression asks, the same way a broken deny denies. An answer is bound to a fingerprint of the exact action it was given for: the Bot, the tool, the ref, the key, the file path and the page. That binding is what makes this more than a dialog box, because without it an id granted for "click Place order" is spendable on "click Delete account". Approvals are single use and expire after ten minutes, and they live in memory: a pending question is about a live browser session and a live turn, and an approval that outlived the process would be a grant nobody remembers giving. Answering is audited as its own act by its own actor. The person who approves is usually not the one whose turn raised the question, the two happen minutes apart, and an approval that is given and never spent leaves no action row at all, so the request, the answer and the action are three rows joined by an approval id rather than one row with a flag on it. The surface holds the tool call open, polls for the answer, and re-issues the identical request with the approval attached, so an approved action costs the Bot some seconds rather than its turn. A declined one comes back as a refusal and Stop still works out of the wait. In dry-run an ask interrupts nobody and is only recorded. The whole promise of dry-run is that switching a policy on changes nothing, and a mode that started stopping people to ask questions would be a mode nobody dares switch on.
…ed it The ask list was only half wired. `evaluateActionPolicy` judges a Bot's calls to other people's MCP servers as well as what it does in a browser, and that call site only knew about yes and no: an ask verdict does not forward, so it was thrown as a permanent refusal, with a sentence that read "The Bot wants to call ." because the neutral blanks a tool call is judged against were mistaken for a file path. Nobody was asked, no id existed to grant, and the third list quietly became a second deny list for every tool a deployment has added. That is the outcome the list exists to prevent, and it lands on the first rule most people write, which is about somebody else's system rather than about a button. The MCP path now asks the same way the computer does: the question is opened against the same registry, the route reports it as 409 with the same body the acting routes use, and the surface holds the call open and re-issues it with the answer attached. Its binding covers the arguments as well as the tool, because a call to somebody else's server is identified by what it says rather than by what it lands on, and "post the release note in the team channel" is not permission to post something else somewhere else. Answering moved to `/api/approvals`, its own surface rather than a pair of handlers under the computer. A deployment can run plugins without a browser, and a question raised where nobody can answer it is worse than a rule that never fired: the Bot waits out the full ten minutes and then reports that nobody answered, having never asked anybody. The one projection lives beside the registry now, so the fingerprint cannot leave the process through one handler while its sibling four lines away is careful to keep it in, and an answer names the Bot the question was actually about rather than the address it arrived at: otherwise the trail holds a grant filed under one Bot and the action it paid for filed under another. The three rows are `approval.requested`, `approval.granted` and `approval.denied`, no longer named for the computer, each filed against the thing the question was about. The card in the transcript is handed its question by the tool call that raised it. It used to poll the Bot's list and show the oldest unanswered entry, and nothing withdraws a question when a wait ends, so pressing Stop or reloading a tab left one open for the rest of its ten minutes: the next turn's card offered a person a stale question, recorded their Allow against it, and left the action they were actually looking at waiting for an answer that never came. `submit` is a policy attribute now. The type tool takes a flag meaning "and press Enter", the computer presses it itself, and no keypress ever arrives as an action of its own, so a boundary written about clicking and about `key` watched the one call that submits a single-field form go straight past it. Both form presets say it, and it is in the binding, because "fill the postcode in" is not "fill it in and send the form".
davidmckayv
left a comment
davidmckayv
left a comment
There was a problem hiding this comment.
Approving. Reviewed against the code and run locally: 641 pass / 0 fail on a clean database.
I went looking for the two ways this could be talked into yes and both are closed. An approval is bound to a hash of the exact action with the Bot id first, so "yes, click Place order" cannot be spent on "click Delete account" or reused against another Bot, and consume(id, fingerprint) makes it single-use with a TTL.
I did briefly think "any signed-in user can answer" was a hole and started restricting it to the asker. Your own test corrected me: /** The person whose turn raised the question. Not the person who answers it. */. Separation of duties is the point, and my change broke six tests, which is the right outcome. Reverted.
One for later, not this PR: with no tenancy yet, "a person" is everyone in the deployment. Worth revisiting when I4 lands.
Merge-order note: this adds 0001_gigantic_sumo.sql, and #18 and #20 each add a different 0001. Whichever lands first takes the number; the other two need renumbering and a regenerated journal.
Both sides added to the same four places and both belong. `PolicyContext` now carries `repeat.count` and `submit` together, and both stay required for the same reason: an identifier CEL cannot resolve throws, and a deny rule that throws denies, so one optional field turns one rule about repetition or about form submission into a deployment that refuses everything. In `govern()` the count is still taken before the context is built and the row about it is still written before the policy is asked, which is the whole point of counting there: a rule about the tenth identical click decides the tenth. The ask path runs after the decision, where it always did, so a question a person is put in front of is a question about an attempt that has already been counted. The audit page keeps both new filters and both new sets of words, the boundaries page keeps all three presets, and `.env.example` names `submit`, the MCP fields and `repeat.count` in one attribute list. The gateway and policy test files keep every case from both sides; the repetition fixtures gained the `ask` list the policy type now requires.
One interaction the merge created that neither side had
Practical effect: under a rule like Left as it stands, deliberately. Removing it means either changing the repeat fingerprint or moving |
Migration numbering, for whoever mergesMain holds They do not conflict against main today, so each merges clean on its own — but the first one merged
|
davidmckayv
left a comment
davidmckayv
left a comment
There was a problem hiding this comment.
Re-approving after the rebase onto #17. 697 pass / 0 fail on a clean database.
Merging this one next, ahead of #18 and #20, because it moves the contracts they sit on: ActionPolicy gains ask and PolicyContext gains submit. Landing it first means the other two rebase onto the final shape once rather than twice.
Good catch on the interaction with #17 that neither branch had alone: an approved action is counted twice against repeat.count, once for the attempt that raised the question and once for the approved retry. Agree with leaving it and erring toward over-counting; worth knowing before anyone writes a repeat rule with a tight threshold.
I verified the security properties live rather than by reading: granted an approval for "open example.com", tried to spend that exact id on another host and got 409 with a fresh question, then spent it on the granted action and it went through. The mismatched attempt did not burn the grant, and replaying the spent id raised a new question. The trail showed 3 requested, 1 granted, 1 allowed.
Both branches added to the same audit row renderer. CopilotKit#19 colours a stalled turn as not-carried-out and prints how long the silence was; this branch prints the rule a question was asked under. Neither displaces the other.
davidmckayv
left a comment
davidmckayv
left a comment
There was a problem hiding this comment.
Re-approving after resolving the conflict #19 created in the audit row renderer. 738 pass / 0 fail locally, CI green.
Both features kept: a stalled turn still colours as not-carried-out and prints its silence, and an approval row still prints the rule it was asked under. Neither displaces the other.
CopilotKit#15 took 0001 when it merged. Regenerated with db:generate on top of the new main rather than renaming by hand, so the file, the journal entry and the snapshot are consistent. Also merges two import-block conflicts with the approvals work, both additive.
The approval registry from #15 kept its pending questions in a Map in the process, and the repetition detector from #17 kept its counts the same way. Both are correct on a laptop and both stop being correct the moment a deployment runs a second server process, which is the deployment every part of this is aimed at: several processes behind a load balancer, serving a company. The failure mode is the reason to take them back rather than leave them and fix them later. Neither one breaks loudly. A question raised on one process and answered on another is reported as no longer open, which reads exactly like an expiry. Counts split across processes mean a rule written as `repeat.count >= 10` never fires, which reads exactly like a Bot behaving itself. A boundary that silently stops enforcing is worse than one that was never advertised, because the deployment is relying on it. The ask lists, the approval surface, the repetition context and the two audit event kinds go with them. What stays is everything that was already right: the stall watchdog, which tracks streams in the process that holds them and belongs there, and the client-side work from #16 and #19. The rule is now on the pull request template, stated before the work rather than at review. Our own gateway snapshot cache has the same problem and is next.
2 participants
The problem
The action policy has exactly two answers: forbidden forever, or permitted forever. That fits the ends — nobody wants a Bot typing into a password field, everybody wants it reading a page — but not the boundary most deployments actually want, which sounds like "it may submit forms, but I want to see the first few", or "it may use Jira, but ask me before anything changes there".
The only approximation is to deny the action and have a person take the wheel, and that throws away the turn: the run ended at the refusal, so everything the model had worked out to reach that button went with it. The rule is therefore either too loose to be worth writing or too expensive to keep. A policy that only says yes or no also leaves a trail that never records anybody agreeing to anything.
The approach
ActionPolicygains a third list,ask, evaluated afterdenyand beforeallow, and both halves of that position are load-bearing. An ask must not soften a deny: what a deployment has forbidden is not up for renegotiation at a prompt. It must beat allow, because the shipped policy isallow: ["true"]— an ask checked afterwards would be unreachable, so the first rule anybody wrote would silently do nothing. A broken ask expression asks, as a broken deny denies.The same lists judge a Bot's MCP tool calls as well as its browsing, and
askis wired through both: "ask me before anything changes in Jira" is the rule operators reach for first, and a call site that knew only yes and no would throw an ask verdict as a permanent refusal, making the third list a second deny list. Answering has its own surface,POST /api/approvals/:botId/:approvalId, mounted with or without a computer.The binding is what makes this governance rather than a confirmation dialog. An approval carries a fingerprint over the Bot, tool, ref, key, file path, page URL, whether the call presses Enter, and a tool call's canonicalised arguments; spending it requires that fingerprint to match, or an id granted for "click Place order" is spendable on "click Delete account". Approvals are single use, expire after ten minutes, and survive a mismatched replay rather than being burned by it. Enter is in there because
submitis a policy attribute now: the type tool presses it itself, so no keypress reaches the gateway and a preset named "never submit a form" missed the one call that submits a single-field form.The registry is in memory and per process, deliberately: a question is about a live session and a live turn, and a restart takes the snapshot, the page and the model with it, so a persisted approval would return as a grant nobody remembers giving. The cost is that restarting mid-decision loses the question. Answering is audited as its own act by its own actor, in
approval.granted/approval.deniedrows filed against a computer or a tool rather than folded into the action row, because the approver is often not the person whose turn raised the question.approval.requestedmakes the case that matters visible: a question asked and never answered.Over HTTP the ask reports as 409 carrying
awaitingApproval: true, not 403: 403 already means a boundary refused you and it is final, so reusing it would make every ask rule read to a Bot as a deny rule. The surface holds the tool call open, polls once a second, and re-issues the identical request withapprovalIdattached — waiting insidecallComputerandcallPluginToolrather than in each handler, so a tool cannot be added without it, and retrying once rather than recursively, because a second ask means the approval did not fit. Stop is honoured out of the wait. The card is drawn on the line of the call that raised the question and handed it by that call: nothing withdraws a question when a wait ends, so a card keyed on the Bot would offer a stale one on an unrelated line. Indry-runan ask interrupts nobody and is only recorded.What is not covered
e9could name a different element without the fingerprint noticing.computer_typecall types is not bound. Deliberately: the question names the field, never the value, so binding to a value nobody was shown would only produce repeat questions.APPROVAL_TTL_MSandWAIT_FOR_ANSWER_MS, which can drift.Merge notes
This branch moves contracts the other in-flight branches sit on.
ActionPolicygains a requiredask: string[]andPolicyContextgainssubmit, so every policy literal conflicts; three server test files grewask: []fixtures, nothing deleted or skipped.createAppgains a trailingapprovalsparameter,createPluginStorea requiredapprovalsoption,auditEventTypesthree entries.server/drizzle/0001_gigantic_sumo.sqlwas generated, so any branch that also randb:generatecollides on_journal.jsonand the0001snapshot and needs renumbering.Verification
All gates run from a clean tree at the pushed commit.
bun run formatandbun run format:check— no fixes appliedbun run lint— exit 0, 24noTemplateCurlyInStringwarnings, all pre-existing in five test files this branch does not touchbun run typecheck— app, server and worker all exit 0bun run test— 646 pass, 5 skip, 0 fail across 71 filesbun run build— exit 0That is 52 tests more than upstream main's 594. Two files are new:
server/tests/computer-approvals.test.tson the registry — fingerprint binding, the refused replay that does not burn the grant, single use, expiry, cross-Bot isolation, argument canonicalisation — andserver/tests/approval-routes.test.tson the real routes: the three-row sequence, attribution to the answering person, the wrong-Bot 409, the projection that keeps the fingerprint inside the process, a decline, single-use binding. Existing suites gained ask precedence and question phrasing (computer-policy), stop-and-ask and the type tool's Enter (computer-gateway), an asked-about MCP call bound to its arguments (plugin-store.integration), and ask rules surviving a restart (policy-durability.integration). The schema change was applied against a real database first.