Tell somebody when a Bot is blocked on them - #20
Closed
jerelvelarde wants to merge 7 commits into
Closed
Conversation
A Bot that hits a login wall calls computer_request_help, the gateway writes the row, the control state flips, and nothing else happens. If the person is not on that Bot's screen at that moment, nothing tells them, and the Bot waits until its ten minutes run out. The same is true of a secret request. It is the one event in this product worth interrupting somebody for and it was the one event that announced itself nowhere. notifications.ts holds the rule and nothing else: a Bot blocked on you earns an interruption, a Bot merely working does not, and every kind states its answer in one table so a new kind has to argue with the ones already there rather than being added quietly somewhere else. It also flattens what the model said into one line, because a model asked for a sentence sends back three paragraphs and a fenced code block, and a notification is one line by definition. Delivery is the channel event hub's, on a topic of its own. In process would have been fewer moving parts and silently wrong: a deployment runs more than one server process, the browser holds its socket to whichever one answered the upgrade, and the handover is served by whichever one answered that request. Nothing arranges for those to be the same process, so an in-memory event reaches the person only by luck, which works on a laptop and stops working in production without saying so. The events on that socket are now tagged, so one connection carries both kinds and a tab left open across a deploy skips what it cannot read instead of guessing. The gateway raises it where it already writes the audit row, after the row rather than before it: the trail is the record and the notification is a message about it. It is raised and not awaited, because telling somebody must never be able to delay, or fail, the handover that lets them unblock the Bot. The cost is that a notification that cannot be delivered is gone rather than retried. Silencing one Bot is a column on agent_preferences beside hidden_at, which is the same kind of thing: an opinion one person holds about one Bot that changes nothing for anybody else. The server reads it before it raises, so the switch is not decoration. It silences the notification only; the Bot still asks, its screen still shows the prompt, and the trail still records the handover.
Three surfaces, in order of how much they are allowed to be missed. A toast in the corner catches an eye that is elsewhere and goes away on its own. A marker on the Bot's row in the sidebar is the trace the toast leaves, and it stays until somebody opens the Bot, because a toast nobody read has not answered anything. The operating system's own notification is behind a switch, for the case where OpenBot is not the window at all. Clicking the toast goes to the Bot with its screen open, because the prompt that unblocks it lives in that pane; sending somebody to the transcript would be the right conversation and the wrong half of it. The marker is in localStorage rather than in React state, so refreshing a tab does not answer a Bot's question. It costs being per browser, which is honest: the socket does not replay either, so a person signed in elsewhere sees only what arrives while they are there. What has already been announced is seeded from what was outstanding at mount, or every reload would greet somebody with a toast, and a desktop notification, for every Bot that ever went unanswered. The desktop switch is the only thing in the product that asks the browser for permission, and it asks only when somebody turns it on. A prompt on load is usually answered with Block, and Block is permanent enough that the feature is then gone for good. It stores its answer in this browser, not on the account, because the permission is granted per browser and a server-side yes this browser has never granted would be a promise the product cannot keep. Silencing one Bot sits on that Bot's profile next to Hide, which is the same kind of decision, and the copy says what it does not do: the Bot still asks, its screen still shows the prompt, and the trail still records the handover.
Three ways the corner of the screen was not doing what it says it does. The auto-dismiss timer belonged to the list of cards rather than to a card, so it was torn down and rebuilt every time the waiting store changed at all. The common case is the worst one: a Bot on the channel somebody already has open asks for help, the marker is set and then immediately cleared by that screen, the rebuilt effect finds nothing newly arrived and schedules nothing, and the card stays in the corner until the tab is reloaded. Two Bots asking inside the same twelve seconds stranded the first the same way. Each card now counts its own time from when it appeared, which is the only clock a store somewhere else cannot stop. Stranded cards were not merely untidy: they take pointer events, so they sat over whatever was underneath them. The socket and the cards were mounted in the app shell, and Settings and Admin are siblings of that shell rather than screens inside it, while the provider that owns the Bot's request-help tool sits above all three. A Bot could be mid-run and ask for a person who was reading the preferences page, and that ask reached nothing at all: no card, no marker, no desktop notification, and no way to recover it afterwards. The page that offers the desktop switch was a page the switch could not deliver on. Both now hang off the authed boundary, which is where the tool that raises them already is, and it is still one socket for the application. Opening a channel cleared the marker for its first coworker only, while the sidebar sets one for any of them. The API creates channels with more than one, so a marker set by the second could survive being opened and had nowhere left to go. Both ends now mean the same thing by a row. The name on the card came from the channel, which is named after everybody in it, so a two-Bot channel announced both names and said one of them was waiting. It comes from the roster now. The marker store also gains a storage listener. It kept its copy in a module variable and wrote the whole map back on every change, so a second tab whose copy predated another tab clearing a marker put that marker back, and a Bot somebody had already dealt with reappeared. The module argued the cost of this store was per browser; without the listener it was per tab. What is still not fixed is stated where it is relied on rather than left to be found: nothing on the server holds an outstanding notification and nothing re-derives the marker from control state, so a notification raised while a browser is disconnected is not delivered late. The header of the socket hook and the store's own docblock now say so instead of implying a recovery that does not exist.
The switch read its position from this browser's stored preference alone and consulted the permission only while somebody was pressing it. A browser grant can be withdrawn long after it was given, in site settings or by a profile clear, and none of that comes back through the tab that asked for it. So a person who turned the switch on and later revoked notifications for OpenBot reopened Settings to a switch sitting at On, with nothing said, while every notification was being dropped at the permission check. That is the exact failure the component's own comment claims it prevents, and it is worse than the switch being off, because somebody trusts it and misses the Bot. The rule now lives in a function that takes the stored preference and the permission API and returns where the switch may honestly sit, so it can be argued with in a test rather than only in a browser. The page settles the two when it opens and again whenever the tab is focused, because revoking a grant is done in the browser's own settings and the way back from there is to this tab. A withdrawal is written back to storage rather than only displayed, so nothing downstream goes on reading a yes this browser will not honour, and the sentence about a blocked browser is shown only to somebody who had asked to be notified.
The control-character strip in summarize was added because the only source of that text is model output shaped by whatever page the Bot is on. The same threat walks straight past it as characters that are invisible rather than unprintable: a right-to-left override survives both the strip and the whitespace collapse, so a page could hand the model a reason that renders in the toast and in the operating system's notification reading backwards from the audit row describing the same handover. Zero-width joiners and spaces and the byte order mark go for the same reason. Removed rather than replaced with a space, because a character of no width is not a word boundary. Silencing a Bot had no test anywhere. The two routes, the store's write and the read the notification path makes were each only exercised by hand, and the profile field was projected into the DTO alongside rows that were all false, so a field that was never read would have passed. The integration test now walks the round trip the feature actually depends on: the switch writes, the thing that decides whether to interrupt somebody reads, it holds for one person and not another, and hiding the same Bot afterwards does not undo it, which is the failure the shared row invites. One assertion in the notification tests could not fail. Asserting that isWorthInterrupting returns a boolean for a key that exists by construction reads as coverage of "every kind states its answer out loud" and is none. It now checks the thing that can actually go wrong: that the rule and the frame, two functions reading one table, cannot disagree about a kind, and that a kind which earns an interruption has words to show for it.
davidmckayv
previously approved these changes
Aug 19, 2026
davidmckayv
left a comment
davidmckayv
left a comment
Contributor
There was a problem hiding this comment.
Approving. Reviewed against the code and run locally: 625 pass / 0 fail on a clean database.
summarize() is the part I'd have expected to be missing and isn't. Stripping control characters, zero-width joiners and BiDi overrides before model text reaches a toast and the OS, with code-point-safe truncation, is a prompt-injection defence most of this kind of code skips. The reasoning about a page putting an RTL override in front of the model so the notification reads backwards from the audit row is exactly right.
Gateway integration is correct too: the notification is raised rather than awaited, and absent notify leaves every handover behaving as it did. Telling somebody must never be able to delay or fail the thing that unblocks the Bot.
Merge-order note: this adds 0001_clammy_crystal.sql, and #15 and #18 each add a different 0001. Whichever lands first takes the number; the other two need renumbering and a regenerated journal.
jerelvelarde
marked this pull request as ready for review
August 19, 2026 23:48
jerelvelarde
requested review from
MikeRyanDev,
guidovizoso and
tylerslaton
as code owners
August 19, 2026 23:48
Two features that touch the same gateway from opposite ends: main counts a Bot repeating itself and hands the count to the policy, and this branch announces a Bot that has stopped and is waiting for somebody. The count still lands in `PolicyContext` before `evaluateActionPolicy` is asked, so a rule about repetition still decides the very attempt that crossed the line. The notification still goes out after the audit row for the handover and is still not awaited, so telling somebody cannot delay or fail the handover that unblocks the Bot. `ComputerGatewayOptions` carries both `repeat` and `notify`, and the gateway tests keep both sets of cases.
Contributor
Author
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
|
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.
Contributor
|
Closing this one for sequencing, not for its design. This branch is the only one in the current batch that got the deployment model right, and it got it right deliberately. Putting delivery on a Postgres The reason it is closed is that it lands on top of the approvals work from #15, which is being reverted: that registry keeps its pending questions in a process Please open it again against main after the reverts land. Nothing here needs to change on the merits. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
A Bot that hits a login wall calls
computer_request_help. The gateway writes thecomputer.help_requestedrow, the control state flips, the Bot's screen shows a take-the-wheel prompt, and the tool call holds open for ten minutes. If the person is not looking at that Bot's panel — another channel, Settings, another tab, coffee — nothing tells them, the ten minutes run out, and the run ends having achieved nothing. A secret request behaves the same way. It is the only state where the product has stopped and cannot start again on its own, and it announced itself nowhere outside that pane. There was nothing to build on either:useNeedsYoupolls every three seconds and only beside an open channel screen, and a deployment runs more than one server process, which rules out the obvious in-process fix.The approach
server/src/notifications.tsholds the rule: a Bot blocked on you earns an interruption, a Bot merely working does not. Kinds sit in one table, each stating itsblockinganswer out loud, so a new one has to argue with the two already there — a product that announces six things a person cannot act on has stopped announcing the seventh, which they could have. The module also flattens the model's words to one line, unwrapping fences instead of dropping what is inside them, and strips control characters together with the invisible ones: a right-to-left override survives a control-character strip, and the toast would then read backwards from the audit row for the same handover.Delivery goes on a new Postgres
LISTEN/NOTIFYtopic, through the hub that already fans channel activity over the app's WebSocket. In-process would have been fewer moving parts and silently wrong: the browser holds its socket to whichever process answered the upgrade and the handover is served by whichever answered that request, so an in-memory event works on a laptop and stops working in production without saying so. Events are now tagged, so one connection carries both and a stale tab skips a kind it has never heard of.The gateway raises it beside the audit row and after it: the trail is the record, the notification a message about it. It is never awaited, because telling somebody must not delay or fail the handover that unblocks the Bot, so an undeliverable notification is lost rather than retried; the mute preference fails the same way, reading as not muted when its store is unreachable. The recipient is the person whose session the Bot runs under, not everybody who can see it. That preference is
notifications_muted_atbesidehidden_at, written by an upsert that touches only that column so muting cannot unhide a Bot; a muted Bot's handover still succeeds and is still audited.Three surfaces, in order of how much each is allowed to be missed. A toast keeps its own countdown, since one owned by the list of cards was cancelled whenever any other marker changed, stranding the commonest card of all: the Bot on the channel somebody already has open. The marker it leaves sits on the channel row, lives in
localStorageso that a reload is not an answer, and is cleared for every coworker in a channel when somebody opens it. Both the socket and the toasts hang off the authed boundary rather than the app shell, because Settings and Admin are siblings of that shell and a Bot does not stop needing somebody who has walked into the preferences page. The desktop notification is behind a switch in Settings, the only thing here that asks the browser for permission, and that switch re-checks the permission at mount and on focus so it cannot sit at On while every notification is being dropped.What is not covered
localStorage./bot.useNeedsYouis untouched. This works when that poll cannot; it does not replace it.help_requestedandsecret_requested.Merge notes
This branch adds
server/drizzle/0001_clammy_crystal.sqland claims index 1 in the drizzle journal, as do policy-ask-a-person and routines-and-webhook-triggers, so whichever lands second must renumber and regeneratemeta/0001_snapshot.json. It adds an optionalnotifytoComputerGatewayOptionsplus two call sites incomputer/gateway.ts, which three other branches edit, and composes the raiser inserver/src/index.ts, which four of them touch. Two contract changes travel further than their files:ChannelActivityEventgainedtypeand itsmemberIdsis nowrecipientIds, andAgentProfilegainednotificationsMuted.startChannelActivityListeneris nowstartLiveEventListener.Verification
All gates run from the repository root against a dedicated database.
formatandformat:checkare clean;lintexits 0, leaving the repository's 24 pre-existing warnings in files this change does not touch;typecheckexits 0 for app, server and worker;buildexits 0.bun run testreports 625 pass, 5 skip, 0 fail across 71 files — 630 in all against upstream main's 594, so this branch adds 36.Two test files are new.
server/tests/notifications.test.tsholds the rule and the frame, which cannot disagree about a kind, andsummarize: fences unwrapped, control and invisible characters removed, clipping counted in code points.app/tests/notifications.test.tsholds the marker store and the reconcile that refuses to leave the desktop switch at On against a withdrawn grant. Five existing suites were extended: the gateway raise for both kinds and a handover that works with nonotifypassed, delivery only to the person named over a real Postgres round trip, the mute round trip, the two routes, and the new column.By hand, a fenced multi-line help request produced one audit row and one notification flattened to a line, the marker appeared on that Bot's channel row and survived a reload without re-announcing, the toast landed on
/channel/{id}?watch=trueand cleared it, and muting left the handover working with no notification. Reproducing that needsCOMPUTER_TOKENset, which the checked-in.envleaves empty.