feat(canvas): add linked agent inbox notifications - #98
Conversation
|
Two notes from a batch review of the open PRs. The rate limit is bypassable by concurrent requests. The link check rests on an asserted identity. Worth deciding alongside #113: that PR covers the same ground with arbitrary-text Verified by reading |
652da19 to
8d3b00b
Compare
|
Thank you for this, and sorry it sat open. I am not going to merge it, but not because there is anything wrong with it. The agent-messaging design I am implementing right now covers the same ground, and it folds in both of the ideas that make this PR work:
Both are going in with credit to you. I would rather do that than have you rebase a 111-line PR onto a design that is about to move underneath it. If you want to stay involved in the messaging work, say so and I will loop you in on the design before it lands — you clearly thought about the failure modes here. |
The control surface has never had a throttle. What it has is timeouts (SLOWLORIS_MS -> CONTROL_CEILING_MS, the 120s pendingControl bound) and a one-at-a-time confirmation serializer. A timeout is not a throttle and a modal is not a budget — and messaging skips the modal entirely once the per-project switch is on, so even the accidental brake is gone. Two limits, both PRE-PROBE, both refusing with the existing `rateLimited` member of the outcome union rather than a new shape: - Per PAIR, 10s (eneskirca#98's number, now enforced). Keyed by sender alone, an orchestrator fanning out to four workers starves itself; keyed by target alone, one node's window is held shut by whoever wrote to it last and every other conversation with it pays. Keyed by the pair, the budget belongs to the conversation — and B->A is a different pair, so a reply is never throttled by the message it answers. - Per TURN, 4 per sender. A turn is identified by the budget entry itself, created by the turn's first send and destroyed by the sender's own `newTurn` — the same edge the renderer's per-turn fan-out already uses. A state transition would reset it many times inside one turn; a session id would never reset it at all. State is process memory, swept on read. A LOST limiter is one extra message per pair after a restart; a STUCK one is a permanently silent agent, so every path that could produce one fails open: entries are evicted rather than filtered, a backwards clock jump drops an entry instead of parking it in the future, and a fan-out budget whose reset signal never arrives expires after TURN_STALE_MS. Ships inert: nothing calls any of it until the verbs land. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…h the one delivery path PR eneskirca#98 (parsa222) designed `notify` as a fixed, app-authored prompt a canvas agent can send a linked agent, with the rule that made it safe: "The app owns the entire prompt so the source cannot inject instructions through command arguments." This commit SUPERSEDES eneskirca#98 rather than rejecting it: the verb, its `--node` requirement and its exact `--text` refusal ('notify does not accept --text') survive verbatim, and its 10-second per-pair throttle survives as the flow module's PAIR_MIN_INTERVAL_MS (eneskirca#208) — the same number, now shared with send/reply so one conversation has one window. What changed versus eneskirca#98's implementation: the body is substituted in MAIN (NOTIFY_BODY), so even a forged IPC request cannot put caller text into the envelope — the renderer's --text refusal is UX, the substitution is the boundary, and the test proves a hostile body never reaches the pane. Delivery goes through the identical deliverAgentMessage pipeline (identity gates, idle gate, pane probes, receipt, trace) instead of a bare sendText, notify is verified-only like its siblings, and the per-project switch governs it. eneskirca#98's "check your configured inbox" wording is dropped — the product has no inbox concept — in favour of pointing at the linked context the notified agent can actually read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ere refused Appends the post-convergence session to the existing handoff rather than replacing it — the earlier document is an accurate pre-convergence snapshot and stays. The part worth reading is the three shapes a bulk merge used to lose working code, because each one looked exactly like success: symbols that existed in a parent commit and were simply gone; two branches each adding a same-named interface, which TypeScript MERGES rather than rejects, so the effective type became the union and every producer failed with errors pointing away from the cause; and a destructive safety barrier whose outer scaffolding survived while its inner body was replaced by the shallower branch's version, so the guard read as present and was not. All three passed a green typecheck. That is the sentence to remember. Also records three deliberate PR exclusions with their reasoning, so nobody "finishes" them later without revisiting it: psmux is a second Windows persistence backend competing with the session host this fork already ships; eneskirca#98 is superseded by richer inter-agent messaging already on main; eneskirca#149 is a 9-file registry refactor that collides with a shortcut landed the same day, aborted rather than half-merged. And it states plainly what is not verified: the suite has not been run this session. 份 handoff 唔係報喜——最有用嗰段係講一個睇落成功嘅 merge 點樣靜靜雞整走咗 啲做緊嘢嘅 code,而且三次都過到 typecheck。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@eneskirca, thank you for the reply! Glad to see the feature implemented, and I was happy to contribute. I have thoroughly enjoyed using nodeterm thus far and am excited for things to come. Keep up the great work! |
Summary
notify --node <id>canvas-control verb for linked agent coordinationwriteandclosecommandsValidation
npm test(4,220 passed, 8 skipped)npm run typechecknpm run build