fix(ai): answer each ticket once, and stop showing reporters internal ticket IDs

[NathanTarbert]

The rule

Outpost answers exactly one message per ticket — the message that opened it — and never posts in that thread again, regardless of who speaks next. Not the original reporter, not a teammate, not a third party. The agent is a first line of defence; a human owns the thread from the first response onward.

Team-member detection is irrelevant to this decision and no longer gates it.

Why

Two real threads in the CopilotKit Discord:

• 1535447155735789708 — a maintainer posted the real solution; the bot replied 14 seconds later summarising that answer back at the thread.
• 1531971013791711342 — a maintainer asked a community member a follow-up question; the bot answered the maintainer, explaining it "can't see other users' replies".

The same thread also opened with 🎫 Ticket TKT-4HS8SRR3 created, publishing an internal identifier into a public server.

How the rule is enforced

Three layers, because no single one is sufficient:

1. Enqueue sites refuse. Replies no longer enqueue an AI_RESPONSE job on any platform — shared/platforms/inbound.ts handleReply (Discord, Slack, Teams, GitHub issue bodies), github-app/webhooks/issue-comment.ts (its own path, which bypassed InboundHandler entirely), web/api/webhooks/postmark/route.ts, and discord-bot/lib/shadow-mode.ts so shadow mirrors production. Only three enqueue sites remain, all new-ticket-only.

2. The database elects one primary response per ticket. Message gained a nullable responseKey with a unique index on (ticketId, responseKey). Exactly one PRIMARY_AI_RESPONSE row can exist per ticket, so concurrent jobs cannot both pass a check-then-write — the loser catches a narrowly-identified P2002 and stops. This matters because AI_RESPONSE runs at concurrency 4. The column is nullable specifically so historical BOT rows stay valid and still count as proof a ticket was answered.

3. A re-answer gate in the handler. Reads the ticket's own message history, so a caller added later inherits the rule.

Layer 1 is the load-bearing one. This is the correction I most want reviewed: the first version of this branch described the handler gate as making the invariant "unroutable-around", with the enqueue-site removals as a cost optimisation. That was backwards — a ticket freshly minted around a mid-thread reply carries no prior AI response and sails straight through the gate. The gate prevents re-answering; the enqueue refusals are what enforce one-answer-per-ticket. The comments now say so.

Delivery is now a state machine, not a hope

The original guard had a hole: the BOT marker was written before the platform post-back, so a failed post left a ticket that looked answered while the reporter got silence — and the guard then blocked the retry. Strictly worse than the over-responding it fixed.

Message.responseState is now PENDING → DELIVERED | ESCALATED:

• A response starts PENDING before any external post.
• Successful delivery with no human handoff marks it DELIVERED.
• A response needing escalation stays PENDING until that job is durable, then becomes ESCALATED.
• If delivery itself ends PENDING, a retry schedules a delayed check that pulls in a human only if the response is still pending — preserving the one-post rule without racing the original handler.

Every transition is a compare-and-set (updateMany guarded on responseKey + responseState), never a blind update by id.

Internal ticket IDs no longer reach reporters

• Discord and Slack 🎫 Ticket TKT-… created acknowledgment posts removed entirely — they leaked an internal identifier and spent a bot message on nothing the reporter could act on.
• All three Teams Adaptive Cards no longer render a displayId. It was also dead payload in the response card's Action.Submit data — card-actions.ts ignores data and resolves by conversation id — so it's gone from there too.
• The Teams ack card no longer claims "our AI assistant is reviewing your question" when no AI job was enqueued.
• displayId stays in the dashboard, team slash commands, and logs.

Also fixed here

Area	Fix
inbound.ts	The orphaned-reply fallback called handleNewTicket, which enqueues — so a mid-thread reply became a "new ticket" and got answered. Reported independently by 11 of 20 review agents; reachable on Teams in particular. handleNewTicket now takes an explicit { answer: boolean }.
inbound.ts	sourceId was written as threadId ?? null but looked up as threadId ?? '' — a threadId-less message could never match, so every message minted a new ticket. Now one shared buildTicketSourceId helper across four call sites.
ai-response.ts	Answered the latest USER message, not the opening one, so a follow-up racing the job stole the ticket's single answer.
reopen sets	The three reply paths disagreed on which statuses a customer reply reopens — GitHub omitted CLOSED, email omitted WAITING_ON_CUSTOMER. Load-bearing because of this change: reopening is the only human signal left. Now one shared predicate.
generator.ts	Only read content[0], silently discarding text from multi-block model responses. Now concatenates all text blocks in order and throws on an empty result rather than publishing nothing.
worker.ts	Stale PROCESSING jobs are reclaimed after a grace period, with Job.claimToken fencing so a reclaimed job's original owner cannot write back over the new one. Graceful shutdown shares one drain promise, so signal handlers and the app can't each half-drain.
Teams	Replies are gated by monitored channel — the filter previously only ran for thread starts, so a reply in an excluded channel reached the handler.
Postmark	Reply detection used only the plus-address MailboxHash, so a reply without one became a second ticket and spent an AI answer on a mid-conversation message. Now falls back to In-Reply-To/References, resolving against Ticket.sourceId and Message.attachments.postmarkMessageId. References is what makes this work at all: replies to mail we sent name an ID we never stored, since nothing outbound has ever been sent.
Postmark	Ticket creation is idempotent, backed by a partial unique index on Ticket(sourceId) WHERE source = 'EMAIL'. Orphaned email replies are kept out of the AI queue.

Tests

1902 pass across all 10 packages (was 1778 before this branch); typecheck green 10/10; CI green.

Every behavioural fix carries red-green verification — test written, source broken, RED confirmed, restored, GREEN confirmed. Eleven pre-existing tests asserted the old behaviour and were rewritten, including github-app and discord-bot cases that actively locked in "answer every follow-up".

Notable test repairs, because they were passing while proving nothing:

• The Teams leak guards were mutation-proven unable to fail — the helper read only top-level body[].text, skipping nested containers and action titles. Now a recursive walker fed leak-carrying inputs, five mutations verified RED.
• Deleting && m.isAiGenerated from the gate predicate left the whole suite green. Both halves are now independently pinned.
• Three team-member tests this change silently turned tautological were deleted or repointed onto the new-ticket path.

The gate tests mock the AI pipeline at the class seam rather than driving LLMock, because the assertion they exist to make is that no model call happens at all — generateSupportResponse not being called is the direct expression of that.

Deliberately not in this PR

• #169 — inbound idempotency, rescoped. The per-ticket claim race and Postmark MessageID idempotency are done here. Still open: no generic (source, sourceId) uniqueness (only EMAIL has an index), no find-first-or-upsert in handleNewTicket, and no dedupe on GitHub comment.id, Slack thread starts, or Discord ThreadCreate.
• #171 — whether email auto-answers at all. EMAIL is a registered delivery platform but EmailPostmarkAdapter.postResponse throws not yet implemented, so an email ticket generates an answer, stores it, fails to deliver, and escalates. Either unregister EMAIL or implement sending — a product call, and option two starts mailing customers, so it is not being decided inside this PR. Reply detection for email is fixed here (see below).
• The escalation copy. Responses open with "we've escalated this to our engineering team" and nothing enforces it. Deferred by request.
• Six clusters recorded during review: Discord shadow-mode parity, Teams card-action correctness (unordered findFirst can mutate the wrong ticket), GitHub webhook robustness, the Postmark email door, Slack subtype coverage, Teams card markdown injection.

Reviewer notes

Provenance, stated plainly. ~22 of these commits were authored in a separate session from the first 13. I have since read their diffs and the description above reflects the branch as it actually stands. The full agent review round ran against the first 13 commits only; the later work has not had an equivalent adversarial pass, so treat CI-green as "verified", not "reviewed".

Where I would look hardest:

1. The orphaned-reply assumption — it creates a ticket but never answers it. Dropping a customer's message seemed worse than an unanswered ticket a human picks up from the dashboard, but that's a product call.
2. conversationHistory deliberately still includes an interim follow-up even though the question is the opening message. Question = what to answer, history = what is known.
3. apps/teams-bot/src/__tests__/cards.test.ts was a hand-merge of two agents that both rewrote it; the first two merge attempts had brace errors. A hand-merged test file is exactly where a guard quietly stops guarding.
4. The migrations. All additive — nullable columns, a new enum, partial unique indexes where NULLs stay legal, no backfill. Plain CREATE UNIQUE INDEX rather than CONCURRENTLY, which is correct since Prisma wraps migrations in a transaction, but it does take a brief write lock.
5. Neither Teams card builder sanitizes caller-supplied title/reason, so call-site discipline is all that keeps a TKT- out of card copy. Both current call sites are safe. Filed, not fixed.

[jerelvelarde]

Review: one blocker, one note to correct, rest as follow-ups

Core change is right and the reasoning in the diff is unusually careful — the self-correction about the guard being a backstop rather than the enforcement is the correct read. Two things I'd want before merge, then this is a go.

Verified locally: PR-touched packages/outpost suites pass (130 tests). App-package suites don't load in my worktree on uninstalled deps (@octokit/rest, @hubspot/api-client) — unrelated to the diff, and CI is green on them.

---

Blocking: the Teams orphan path posts a bot card into a thread we were never part of

The new orphan branch at platforms/inbound.ts:270 routes into handleNewTicket, which returns isNewTicket: true unconditionally (inbound.ts:226). apps/teams-bot/src/handlers/message.ts:67 branches on exactly that flag to post buildTicketCreatedCard.

So a mid-thread "thanks, that worked!" — isThreadStart = !activity.replyToId, no ticket found — now produces a "🎫 We've got your question" card posted into the conversation. And because the monitored-channel gate at message.ts:53 runs only if (message.isThreadStart), it fires in unmonitored channels too.

That's the unwanted-bot-chatter class this PR exists to remove, reintroduced on the platform the PR itself identifies as the most reachable for the orphan path. The new orphan test drives InboundHandler directly, so it can't see this.

Suggested fix, small: distinguish the orphan case in InboundResult (isOrphanedReply: true, or return isNewTicket: false) and gate both the ack card and the additionalInfo write on it. Worth moving the channel gate above the handle() call in the same pass.

Also before merge: the #169 deferral rationale is wrong

The PR body says #169's inbound-uniqueness constraint "also closes the guard's check-then-write race." It doesn't. A unique key on the inbound platform key prevents duplicate tickets; it does not serialize two AI_RESPONSE jobs against the same ticket. That needs a different constraint — a unique partial index on Message(ticketId) where type='BOT' and isAiGenerated, or an atomic conditional update.

Fine to leave the race deferred. But as written, the note points #169 at a fix that won't close it, so whoever picks up #169 will believe this is handled.

Worth recording while you're editing that section: the race is reachable without any duplicate inbound event. worker.ts:374 runWithTimeout is a Promise.race and does not cancel the handler; AI_RESPONSE has a 120s timeout (apps/worker/src/index.ts:74) over a deliberately slow pipeline. On timeout, attempt 1 keeps running while attempt 2 starts ~1s later, reads messages before attempt 1's step-5 message.create, passes the guard, and both post.

---

Follow-ups — not blocking

The guard flips a deliberately-failed job to COMPLETED on retry. ai-response.ts:450 returns success: false for "undelivered and escalation not enqueued" so an operator sees it. But worker.ts:413 marks the job PENDING; attempt 2 hits the guard (attempt 1's BOT row exists), returns success: true, and worker.ts:341 writes status: COMPLETED, progress: 100. The error column is left stale but the status reads green. The fail-loud return from 508e904 is neutralized by the guard added in the same PR. Fix would be to verify a delivered response (e.g. externalCommentId present) before the skip path declares success.

Postmark still answers an unresolvable reply. At postmark/route.ts:59, a present-but-unresolvable MailboxHash (deleted ticket, stale hash) falls through to the new-ticket branch and enqueues at line 139. Same for any reply arriving without the plus-address — sourceId: body.MessageID is unique per email and there's no In-Reply-To/References fallback, so a plain reply is always "new" and always answered. Minimum fix: when ticketIdFromHash was present but unresolvable, treat it as an orphaned reply.

Orphaned-reply behavior is Teams-only. inbound.ts:255 states the principle — dropping a customer's words is worse than an oddly-titled ticket — but Discord returns at message-create.ts:54, GitHub at issue-comment.ts:33, and Slack pre-filters at message.ts:56. All three still drop. Either adopt it everywhere or scope it to Teams explicitly in the comment.

Ack UX now diverges. Discord and Slack post nothing; Teams still posts a card. The rationale for removing the Discord/Slack posts applies equally to Teams. Intentional either way, just worth a line.

shadow-mode.ts:76 writes sourceId: thread.id inline, bypassing buildTicketSourceId, in a file this PR touches and against source-id.ts's own "don't inline" contract. Identity for Discord today, so harmless — but it's the drift surface the helper exists to close. Same on the read side in discord-bot/src/lib/tickets.ts:10 and teams-bot/src/lib/tickets.ts:10.

Nit: literal — escapes left in comments at slack-bot/src/events/message.ts:61 and discord-bot/src/events/thread-create.ts:88, where the rest of the codebase uses a real em dash.

---

On the tests

Genuinely strong, and the red-green discipline shows. Two things I want to call out as the right calls: self-testing the visibleCardStrings walker is exactly the response a mutation-proven-dead guard deserves, and mocking at the generateSupportResponse seam is correct when the assertion is "no model call at all."

Gaps all trace to the findings above: nothing drives handleMessage on the Teams orphan path, nothing covers guard-skip-after-failure, nothing covers the timeout double-post.

On the skipped confirmation round — given nine of twelve findings were defects in the first commit, I'd read that as evidence the round earns its cost rather than evidence human review substitutes for it. Not a merge condition; the fixes here are individually verified.

[NathanTarbert]

Thanks — this was a genuinely useful review, and the blocker was a real regression. Fixed in 0bc5900.

Blocking: the Teams orphan card

You were right, and it was worse than you could see from the diff: the existing handleMessage orphan test asserted the bug. It expected sendActivity to have been called once, so it would have kept this green indefinitely.

Fixed as you suggested. InboundResult now carries isOrphanedReply, set true only on the fallback, and handleNewTicket takes { answer, orphanedReply } as two independent decisions rather than inferring one from the other. The Teams handler gates both the ack card and the additionalInfo write on it. I kept isNewTicket: true per your preference — a ticket really was created.

The replacement test asserts the ticket and message still persist while createJob, sendActivity and ticket.update are all not called. Verified red twice: reverting the gate fails it, and gating only the card while leaving the additionalInfo write ungated also fails it, so each half is independently covered.

One correction on the channel gate: that half was already fixed in 91bc0cd — it now runs above the handle() call and covers replies. I verified before touching it rather than fixing it twice.

The #169 rationale — you're right about the reasoning, and it's since been implemented

Your point is correct and worth stating plainly: a unique key on the inbound platform key prevents duplicate tickets and does nothing to serialize two AI_RESPONSE jobs against one ticket. My original note pointed #169 at a fix that wouldn't have closed the race, and someone picking it up would have believed it was handled.

The reason I'm not changing code here is that the mechanism you describe as the real fix — "a unique partial index on Message ... or an atomic conditional update" — is now in the branch, both halves:

• Message.responseKey with a unique index on (ticketId, responseKey) (20260811193000_add_message_response_key).
• ai-response.ts:564 claims that slot via message.create before any platform post-back; the loser catches a narrowly-scoped P2002 (isPrimaryAiResponseConflict, deliberately narrow so an unrelated P2002 still fails the job) and returns at :575 without posting.
• Every subsequent state transition is a compare-and-set via updateMany guarded on responseKey + responseState, never a blind update by id.

So the timeout scenario you traced — runWithTimeout being a Promise.race that doesn't cancel, attempt 1 still running while attempt 2 passes the history gate — now ends with attempt 2 colliding on the create and returning. Only one post. Thanks for spelling out that path; it's the clearest statement of the race anyone has written down, and it's in the rescoped #169 as the reason the constraint had to live on Message rather than Ticket.

The PR body and #169 were both corrected before this reply. #169 is now scoped to what's genuinely left: no generic (source, sourceId) uniqueness (only EMAIL has an index), no find-first-or-upsert in handleNewTicket, and no dedupe on GitHub comment.id, Slack thread starts, or Discord ThreadCreate.

Follow-ups

Guard flipping a failed job to COMPLETED — already closed, by 5a9c98d. An undelivered response with no durable escalation writes an ESCALATION_REQUIRED marker into responseError (ai-response.ts:732), and the retry checks requiredEscalationReason at :387 — before the generic already-answered skip at :414 — routing into recoverRequiredEscalation instead of returning success. Your diagnosis of the shape was right; it landed between your read and this reply.

Postmark unresolvable hash — already closed, by 4d93868. isOrphanedReply = ticketIdFromHash !== null (route.ts:~127) treats a present-but-unresolvable hash as an orphaned reply and keeps it out of the AI queue.

Your second sub-point there is still live and is the largest remaining hole in the rule: a plain reply with no plus-address is always "new" and always answered, since sourceId is the per-email MessageID and there's no In-Reply-To/References fallback. Left with the email-threading follow-up rather than folded in here — happy to pull it forward if you'd rather it block.

Orphaned-reply behavior is Teams-only — documented rather than universalized, as you offered. Verified all three of your pre-filter citations, and found two more facts worth recording: the GitHub comment webhook never uses InboundHandler at all, and Postmark honors the principle but implements it locally. Teams is the only caller that reaches the branch, and the comment now says exactly that.

Ack UX divergence — recorded as deliberate at the Teams ack site, so it doesn't read as an oversight. Worth flagging that a first draft of that comment justified it by the card's "buttons" — buildTicketCreatedCard emits body only, no actions. Caught and rewritten before commit, but it's the same class of error as the displayId-resolves-clicks comment you'd expect me to have learned from.

shadow-mode.ts bypassing buildTicketSourceId — fixed, along with the read sides in discord-bot/src/lib/tickets.ts and teams-bot/src/lib/tickets.ts. Readers return early on an unaddressable key rather than querying sourceId: null, which would match unrelated keyless rows. Two things surfaced while doing it: discord-bot had no coverage for lib/tickets.ts at all, and shadow-mode.test.ts was stubbing the entire @copilotkit/outpost/shared module — so buildTicketSourceId was mocked away and no test there could have caught a drift. Both fixed.

One residual of the same class, not fixed: postmark/route.ts inlines the EMAIL key as body.MessageID at :133, :157 and :194.

Nit — the literal escape was in four files, not two. github-app's issues-opened.ts and discussion-created.ts had it as well, plus a mangled ellipsis. My earlier pass fixed two of six occurrences and reported it closed.

On the confirmation round

Fair, and I'd rather take the correction than defend the shortcut. You're right that nine-of-twelve findings being defects in the first commit argues for the round, not that human review substitutes for it — and this review is a second data point, since it found a regression that twelve fix agents and I all read past.

Current state: typecheck 10/10, 1881 tests pass (11 new), CI green.

[NathanTarbert]

Following up on one thing from your review that I'd left filed rather than fixed — the product owner asked for it to be pulled forward, so it's now in the PR at 76fb78d.

Your second Postmark sub-point is fixed

You wrote:

Same for any reply arriving without the plus-address — sourceId: body.MessageID is unique per email and there's no In-Reply-To/References fallback, so a plain reply is always "new" and always answered.

That was the largest remaining hole in the rule, and it's closed. Detection now falls back to In-Reply-To and References — both already typed on PostmarkInboundPayload.Headers and read by nothing — resolving candidate IDs against Ticket.sourceId and Message.attachments.postmarkMessageId.

Behaviour is now:

Incoming	Result
New email	new ticket, one AI job
Reply, MailboxHash present	appended, no job (already worked)
Reply, resolved via headers	appended, reopened if reopensOnCustomerReply, no job
Reply, resolves to nothing	ticket filed so the words aren't lost, no job

MailboxHash stays the primary path when present — exact ticket reference, cheaper than header matching.

Two things worth surfacing because they shaped the fix:

References is doing the real work, not In-Reply-To. When a customer replies to something we sent, In-Reply-To names a Message-ID we never stored — postResponse throws, so no outbound mail has ever been sent and no outbound ID recorded. References accumulates the whole chain and therefore still contains the customer's own original Message-ID, which is exactly what Ticket.sourceId holds.

One supporting change beyond detection: postmarkMessageId was only persisted on messages that carried a file attachment, so the mid-thread half of the lookup would have had an empty store and been dead code that tests could still fake. Now persisted on every message. Additive; existing rows aren't backfilled, so replies into pre-existing threads resolve via Ticket.sourceId but not via a mid-thread message.

23 new tests, 9 mutations each verified RED then GREEN — including In-Reply-To matching sourceId, References matching where In-Reply-To doesn't, a mid-thread attachments match, headers resolving to nothing, and a genuinely new email still being answered so this isn't a blanket mute. Bracket/whitespace normalisation gets its own direct test: Postmark's MessageID is bare while header values are angle-bracketed and space-separated, and one stray bracket would make every lookup miss and turn every reply back into a new ticket — the exact bug, with a green suite.

Ticket.sourceId semantics for EMAIL and the Ticket_email_sourceId_key index are untouched.

Deliberately still out of scope

While in there I found that EMAIL is registered as a delivery platform but cannot deliver — EmailPostmarkAdapter.postResponse throws not yet implemented, so an email ticket generates an answer, stores it, fails to post, and (since this PR's delivery-failure escalation) pulls in a human for a channel that was never able to deliver.

Filed as #171 rather than fixed here, because the fix is a product decision: unregister EMAIL, or implement sending. The second starts mailing customers automatically and needs threading headers, persisted outbound IDs, and a verified sending domain — not something to decide inside this PR, and not easily undone.

Also captured there, both yours in origin: EmailPostmarkAdapter.parseInboundEvent still detects replies by In-Reply-To alone, so the deferred adapter refactor inherits the original bug; and the email door bypassing InboundHandler means no team-member gate, no userId link, no truncate. Plus one of mine: no expression index behind the new JSON-path lookup on attachments.postmarkMessageId — fine at current volume, a sequential scan later.

The remaining inline-key residual I mentioned last time is unchanged: postmark/route.ts still inlines the EMAIL key as body.MessageID at :133, :157 and :194.

Current state: 1902 tests pass, typecheck 10/10, CI green at 76fb78d. Your blocker and every follow-up is either fixed or filed with a reason.

[NathanTarbert]

Splitting this per the plan in the split brief.

The reviewed core — the actual bot-interrupting fix plus the ticket-ID removal — is now #172: 43 files, 2,133 insertions, zero database changes, based on e02f620 (the reviewed 13 commits) with 91bc0cd and 0bc5900 lifted on top. That ships independently.

This PR stays open and will be reduced to the remainder: (ticketId, responseKey) uniqueness, the response state machine, job claim-token fencing, stale-job reclaim, shutdown drain, Postmark idempotency and header threading, plus four migrations. That half has not had a review pass.

Rationale for the split: the safe fix was trapped behind the risky one, and rolling back the job-queue work would have rolled back the bot fix with it. The two must not be recombined — independent rollback is the point.

[NathanTarbert]

Closing in favour of #187.

Per the split brief: the customer-facing fix merged as #172, and the remaining infrastructure work has been rebuilt on current main as #187.

Why closed rather than reduced in place. This PR's diff still showed the pre-hotfix state, its title described work that has already merged, and CHANGES_REQUESTED was anchored to commits that no longer exist. Reducing it would have meant a force-push that detached every review anchor anyway.

Rebuilt rather than rebased, and that turned out to matter: this branch predates the #180 hotfix, so replaying it would have deleted the SystemConfig repair migration (−93), the schema-drift guard in start.sh (−210), and its CI gate (−257). #187 is built on main with all six hotfix files verified byte-identical.

@jerelvelarde — both blockers you raised are fixed and shipped in #172 (0bc5900 for the Teams orphan ack card; the concurrency race closed by the (ticketId, responseKey) unique claim, which is in #187). The three findings from the brief's technical section are also fixed in #187: escalationEnqueued being write-only, the In-Reply-To/References thread hijack, and responseError carrying control state. The review state moved because the scope changed, not because anything you found persists.

#187 has not had a full review round — that was the reason for the split.