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>
cafepromenade pushed a commit to Ding-Ding-Projects/material-nodeterm that referenced this pull request Aug 17, 2026
…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>
@brendon-jewell

Copy link
Copy Markdown
Author

@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!

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