feat(sdk/plugin-claude): surface incoming contact requests for approval#207
Conversation
The daemon only polled /messages, so an incoming contact request never reached the other agent's session — it had to be pulled manually. Now the listener also polls /contacts/requests (every 15s) and, on a NEW incoming request, pushes it into the session as a channel event and exposes it in whoami.pendingContactRequests and inbox.contactRequests. The agent approves with contact_accept (or the new /tinyplace:contacts command). Never auto-accepted — accepting a contact is a trust decision (and the DM gate depends on it), unlike the auto-responder which only answers existing contacts. Verified live on staging: alice's contact request surfaces in bob's whoami/inbox. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@sanil-23 is attempting to deploy a commit to the Vezures Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
👮 Files not reviewed due to content moderation or server errors (1)
📝 Walkthrough🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sdk/plugin-claude/mcp/server.mjs (1)
924-931: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
inboxnow blocks on an extra network round-trip every call.
await drainContacts()performs a fullcontacts.requests()fetch synchronously on everyinboxinvocation, in addition to the 15s background poll. This adds latency/failure surface to a hot path tool without a visible timeout on the underlying HTTP client. Consider relying on the cachedpendingContactRequestspopulated by the background timer (or only re-fetching if the cache is older than some threshold) rather than fetching inline on everyinboxcall.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/plugin-claude/mcp/server.mjs` around lines 924 - 931, The inbox flow in server.mjs is doing an inline contacts.requests() fetch via drainContacts on every call, adding unnecessary latency and failure risk to a hot path. Update the inbox handling to rely on the cached pendingContactRequests maintained by the background poll, or only refresh it when the cache is stale beyond a threshold, and remove the unconditional await drainContacts() from the inbox path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/plugin-claude/mcp/server.mjs`:
- Around line 511-525: The `drainContacts` flow in `server.mjs` leaves
`seenContactRequests` permanently populated, so withdrawn or later re-sent
requests from the same requester never call `pushContactRequest` again. Update
`drainContacts` to reconcile `session.seenContactRequests` against the current
`session.pendingContactRequests` on each tick: keep only requesters still
present, remove any no-longer-pending entries, and then continue pushing
notifications for newly pending requesters. Use the existing
`pendingContactRequests`, `seenContactRequests`, and `pushContactRequest`
symbols to keep the notification state consistent.
---
Nitpick comments:
In `@sdk/plugin-claude/mcp/server.mjs`:
- Around line 924-931: The inbox flow in server.mjs is doing an inline
contacts.requests() fetch via drainContacts on every call, adding unnecessary
latency and failure risk to a hot path. Update the inbox handling to rely on the
cached pendingContactRequests maintained by the background poll, or only refresh
it when the cache is stale beyond a threshold, and remove the unconditional
await drainContacts() from the inbox path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6acee24-2a7d-4209-868a-13c91a9bcd1b
📒 Files selected for processing (2)
sdk/plugin-claude/commands/contacts.mdsdk/plugin-claude/mcp/server.mjs
| async function drainContacts() { | ||
| if (!session) return; | ||
| try { | ||
| const { incoming } = await session.client.contacts.requests(); | ||
| const list = Array.isArray(incoming) ? incoming : []; | ||
| session.pendingContactRequests = list.map((r) => String(r.agentId ?? r.contact?.requester ?? "")).filter(Boolean); | ||
| for (const requester of session.pendingContactRequests) { | ||
| if (session.seenContactRequests.has(requester)) continue; | ||
| session.seenContactRequests.add(requester); | ||
| void pushContactRequest(requester); | ||
| } | ||
| } catch { | ||
| // best-effort — pending list simply doesn't update this tick | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stale seenContactRequests blocks re-notification after withdraw/resend.
seenContactRequests entries are never removed, so if a requester is withdrawn (or accepted then a new request from the same peer arrives later), it won't re-trigger pushContactRequest — it will only silently reappear in pendingContactRequests/inbox/whoami without the channel push. Consider reconciling the set with the current pendingContactRequests each tick so requesters no longer pending get removed and can trigger a fresh notification if they re-request.
🔧 Suggested fix
const { incoming } = await session.client.contacts.requests();
const list = Array.isArray(incoming) ? incoming : [];
session.pendingContactRequests = list.map((r) => String(r.agentId ?? r.contact?.requester ?? "")).filter(Boolean);
+ const current = new Set(session.pendingContactRequests);
+ for (const seen of session.seenContactRequests) {
+ if (!current.has(seen)) session.seenContactRequests.delete(seen);
+ }
for (const requester of session.pendingContactRequests) {
if (session.seenContactRequests.has(requester)) continue;
session.seenContactRequests.add(requester);
void pushContactRequest(requester);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function drainContacts() { | |
| if (!session) return; | |
| try { | |
| const { incoming } = await session.client.contacts.requests(); | |
| const list = Array.isArray(incoming) ? incoming : []; | |
| session.pendingContactRequests = list.map((r) => String(r.agentId ?? r.contact?.requester ?? "")).filter(Boolean); | |
| for (const requester of session.pendingContactRequests) { | |
| if (session.seenContactRequests.has(requester)) continue; | |
| session.seenContactRequests.add(requester); | |
| void pushContactRequest(requester); | |
| } | |
| } catch { | |
| // best-effort — pending list simply doesn't update this tick | |
| } | |
| } | |
| async function drainContacts() { | |
| if (!session) return; | |
| try { | |
| const { incoming } = await session.client.contacts.requests(); | |
| const list = Array.isArray(incoming) ? incoming : []; | |
| session.pendingContactRequests = list.map((r) => String(r.agentId ?? r.contact?.requester ?? "")).filter(Boolean); | |
| const current = new Set(session.pendingContactRequests); | |
| for (const seen of session.seenContactRequests) { | |
| if (!current.has(seen)) session.seenContactRequests.delete(seen); | |
| } | |
| for (const requester of session.pendingContactRequests) { | |
| if (session.seenContactRequests.has(requester)) continue; | |
| session.seenContactRequests.add(requester); | |
| void pushContactRequest(requester); | |
| } | |
| } catch { | |
| // best-effort — pending list simply doesn't update this tick | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/plugin-claude/mcp/server.mjs` around lines 511 - 525, The `drainContacts`
flow in `server.mjs` leaves `seenContactRequests` permanently populated, so
withdrawn or later re-sent requests from the same requester never call
`pushContactRequest` again. Update `drainContacts` to reconcile
`session.seenContactRequests` against the current
`session.pendingContactRequests` on each tick: keep only requesters still
present, remove any no-longer-pending entries, and then continue pushing
notifications for newly pending requesters. Use the existing
`pendingContactRequests`, `seenContactRequests`, and `pushContactRequest`
symbols to keep the notification state consistent.
There was a problem hiding this comment.
Fixed in bfe6730: drainContacts now reconciles seenContactRequests against the live pending set each tick, so a withdrawn/re-sent request from the same peer re-triggers pushContactRequest instead of silently reappearing.
There was a problem hiding this comment.
Oops, something went wrong! Please try again later. 🐰 💔
- drainContacts: reconcile seenContactRequests against the live pending set each tick so a withdrawn/re-sent request from the same peer re-triggers a channel notification instead of silently reappearing in whoami/inbox. - inbox: drop the inline drainContacts() network round-trip; read the cache maintained by the 15s background poll (matching whoami) to keep the hot path off an extra fetch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the CodeRabbit review in bfe6730:
Verified with |
…e C) Give each agent a single process that owns the relay drain + Signal ratchet, so N sessions never race the mailbox or corrupt the shared ratchet. - hooks/agent-daemon.mjs: single owner via a CAS lock (mcp/daemon-lock.mjs) with takeover on death and idle-exit when the agent has no live sessions. It is the sole decryptor — drains the mailbox once, routes each message to the target session's inbox/ queue (to_session live → that session; dead → _unrouted/ held until it appears; none → primary/broadcast/drop per TINYPLACE_UNROUTED_POLICY), sends outbound jobs from _outbox/ (sole ratchet writer), and drives the auto-responder. - mcp/routing.mjs (routeTarget/enqueueRouted/drainInbox/redeliverUnrouted) and mcp/outbox.mjs (writeOutboxJob/claimOutboxJobs) — atomic-rename file queues. - server.mjs thin-client mode: on use, register presence + ensure a daemon; if one is live, stop touching the relay — send/auto_reply write outbox jobs, inbox/check_reply/await_reply/send_and_wait read the session's inbox/ queue (correlating on the in-body envelope message.id). Falls back to the original self-drain if the daemon is disabled (TINYPLACE_SESSION_DAEMON=off) or can't start — no regression for single-session use. whoami reports mode + daemon. - format.mjs: buildEnvelope returns the message id; decode surfaces messageId. - Tests: routing-test.mjs, lock-test.mjs (incl. a 3-way process race), and daemon-test.mjs (daemon boot + idle-exit + thin-client send/inbox/check_reply) wired into npm test. Existing adopt tests pin TINYPLACE_SESSION_DAEMON=off. Follow-up: contact-request surfacing (drainContacts / /tinyplace:contacts) is not in this base (PR #207), so moving contact-polling into the daemon is deferred. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to #204 (merged). That squash landed everything through the review fixes, but this commit — incoming contact-request surfacing — was pushed just after the merge, so it's not in
mainyet.What
The plugin daemon previously only polled
/messages, so an incoming contact request never reached the other agent's Claude session — it had to be pulled manually withcontact_requests. Now the listener also polls/contacts/requests(every 15s) and, on a new incoming request:<channel source="tinyplace">event (meta.kind="contact_request"),whoami.pendingContactRequestsandinbox.contactRequests,contact_accepttool or the new/tinyplace:contactscommand.Never auto-accepted — accepting a contact is a trust decision (and the DM gate depends on it), unlike the auto-responder which only answers existing contacts.
Verified (staging)
alicesends a contact request → it surfaces inbob'swhoami.pendingContactRequestsandinbox.contactRequests(bob does not auto-accept).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes