Skip to content

fix(assistant): message people by name; repair feedback filing and first-message push - #49

Open
tmad4000 wants to merge 2 commits into
mainfrom
fix/assistant-send-to-person
Open

tmad4000 wants to merge 2 commits into
mainfrom
fix/assistant-send-to-person

Conversation

@tmad4000

@tmad4000 tmad4000 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes the four failures from Jacob's OpenChat Assistant session on 2026-09-08, each traced against prod data.

1. "Feedback service is down right now" — feedback filing was completely broken

WIT_API_BASE defaulted to the sthqnyjniclvnflfkyio Supabase project, which is paused — requests fail at connect (curl returns 000). This killed both the Assistant's submit_feedback tool and POST /api/feedback, so no user feedback reached the tracker at all. Repointed to the live qmzopiburflputowkuhu base, plus .env.example and the prod compose default.

2. "I don't have access to that conversation" when sending to Robert

The confirmation gate is a two-turn handshake, but each assistant turn rebuilds its context from persisted chat messages only — tool calls and results are never stored. The conversationId found in turn A was gone by the time Jacob said "yes" in turn B, so the model guessed an id and failed the participation check. The DM was fine the whole time.

Now the unconfirmed send is remembered server-side (15 min TTL, one slot per user) and re-injected into the next turn's system prompt, so the handshake can actually complete.

3. No way to message a person — the requested feature

The only send tool took an opaque conversationId, forcing the model to dig one out of list_conversations and carry it across turns. Added find_person and send_message_to_person, which work in the terms users actually speak — a name — resolve the DM via the shared ensureDirectConversation path, and refuse to guess when a name is ambiguous or unreachable.

Discovery scope mirrors GET /api/chat/contacts, so the Assistant can never surface someone the user couldn't already find themselves: existing contacts by name, anyone by complete email, plus substring search for trusted directory users. Confirmation and rate limiting apply to the new path exactly as before.

4. "Robert's first message didn't work, only the second did"

The message was never lost — it's in the graph 4s after the conversation was created. The notification was missing: fanoutPushForMessage was private to chatHandler.ts and only called from the socket send path. POST /conversations/:id/messages did everything else (broadcast, webhooks, previews, embeddings, assistant trigger) but never pushed. So any REST-sent message arrived silently — exactly the first message in a new conversation, before the sender's socket is up. Guarded on wasCreated so a client retry doesn't notify twice.

Testing

  • tsc --noEmit clean; eslint clean; 93 tests pass, no regressions.
  • New assistantSendToPerson.integration.test.ts covers the ephemeral-Neo4j path: name resolution scoped to real contacts, non-contacts reachable only by complete email, never resolving the user themselves, the confirm → pending-memory → send handshake landing in the existing DM, refusal to guess between same-named people, and unreachable-person errors.

Still open

The Assistant's own persistMessage path doesn't fan out push either, so a message the Assistant sends on the user's behalf still arrives without a notification. Filed separately rather than widened into this PR.

🤖 Generated with Claude Code

Jacob Cole and others added 2 commits September 8, 2026 16:45
Three failures from Jacob's 2026-09-08 Assistant session, all traced in
prod data:

1. "Feedback service is down right now" — WIT_API_BASE defaulted to the
   `sthqnyjniclvnflfkyio` Supabase project, which is PAUSED. Requests fail
   at connect, so every submit_feedback and POST /api/feedback died. Point
   both at the live `qmzopiburflputowkuhu` base (verified: create-issue
   returns success). Also updated .env.example and the prod compose default.

2. "I don't have access to that conversation" when sending to Robert —
   the confirmation gate is a two-turn handshake, but each assistant turn
   rebuilds context from persisted chat messages only; tool results are
   never stored. So the conversationId found in turn A was gone by the time
   the user said "yes" in turn B, and the model guessed. Remember the
   unconfirmed send server-side (15 min TTL, one slot per user) and re-inject
   it into the next turn's system prompt so the handshake can complete.

3. No way to message a person — the only send tool took a conversationId,
   forcing the model to dig one out of list_conversations and carry it
   across turns. Add find_person and send_message_to_person, which work in
   the terms users actually speak (a name), resolve the DM via the shared
   ensureDirectConversation path, and refuse to guess when a name is
   ambiguous or unreachable. Discovery scope mirrors GET /api/chat/contacts,
   so the Assistant can never surface someone the user couldn't find
   themselves: existing contacts by name, anyone by complete email, plus
   substring search for trusted directory users.

Confirmation and rate limiting apply to the new path exactly as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Jacob's fourth report from the 2026-09-08 session: "the first message from
Robert to Jacob didn't work and it only worked on the second message."

The message itself was never lost — Robert's "hi" is in the graph at
22:01:12, four seconds after the conversation was created. What was missing
was the notification. fanoutPushForMessage was private to chatHandler.ts and
only called from the socket send path; POST /conversations/:id/messages did
everything else (broadcast, webhooks, previews, embeddings, assistant
trigger) but never notified anyone. A message sent over REST — which is what
happens when the sender's socket isn't up yet, exactly the case for the first
message in a brand-new conversation — arrived silently.

Guarded on wasCreated so a client retrying the same message id doesn't
notify twice, and fire-and-forget so push latency never blocks the send.

Co-Authored-By: Claude Opus 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.

1 participant