Skip to content

feat(canvas): add linked agent inbox notifications - #98

Open
brendon-jewell wants to merge 3 commits into
eneskirca:mainfrom
brendon-jewell:feat/agent-notify-control
Open

feat(canvas): add linked agent inbox notifications#98
brendon-jewell wants to merge 3 commits into
eneskirca:mainfrom
brendon-jewell:feat/agent-notify-control

Conversation

@brendon-jewell

Copy link
Copy Markdown

Summary

  • add an opt-in notify --node <id> canvas-control verb for linked agent coordination
  • deliver only a fixed Nodeterm-authored inbox prompt to context-linked agent targets
  • rate-limit each source-target pair while retaining confirmations for arbitrary write and close commands

Validation

  • npm test (4,220 passed, 8 skipped)
  • npm run typecheck
  • npm run build

@eneskirca

Copy link
Copy Markdown
Owner

Two notes from a batch review of the open PRs.

The rate limit is bypassable by concurrent requests. Canvas.tsx:6400 writes the throttle timestamp after await api.pty.sendText(...) at :6395. Nothing serializes control requests — the hook server awaits its handler per HTTP request on a concurrent node:http server — so N requests fired together all read lastSentAt at :6389 before any of them records one, and all N pass. Each delivered line is a submitted turn in the target. A failed send also records nothing, so an unreachable target can be retried without limit. Moving the set above the await fixes both.

The link check rests on an asserted identity. sourceNodeId reaches the handler from the request body, and the hook token is one value shared by every spawned session, so an agent can name a source it is not. Combined with list (which returns every node id on the project), an unlinked agent can iterate ids as claimed sources until one satisfies the link test. The payload here is a fixed string, so this is noise rather than takeover — but it is the same gate #112 and #113 now lean on much harder. I have written the shared analysis and the fix (bind the hook token per node, resolve the acting node from the token instead of the body) on #112 and #113; this PR benefits from the same change rather than needing its own.

Worth deciding alongside #113: that PR covers the same ground with arbitrary-text write plus per-pair grants that are pruned when a node is deleted — which also solves the never-pruned agentNotifyAtRef map here. The two overlap enough that taking them together, on top of the token fix, is probably cleaner than landing both as-is.

Verified by reading Canvas.tsx:6366-6410, hook-server.ts:171/180/453; not reproduced with a crafted POST.

@brendon-jewell
brendon-jewell force-pushed the feat/agent-notify-control branch from 652da19 to 8d3b00b Compare August 12, 2026 20:59
@eneskirca

Copy link
Copy Markdown
Owner

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:

  • the notify verb as the shape for an agent telling a linked node something, rather than overloading write
  • the 10 second throttle, which is the right instinct — an inbox that can be flooded by a loop is worse than no inbox

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.

pull Bot pushed a commit to jasonkneen/nodeterm that referenced this pull request Aug 15, 2026
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>
pull Bot pushed a commit to jasonkneen/nodeterm that referenced this pull request Aug 15, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants