Skip to content

Fix/ai response delivery durability - #188

Closed
NathanTarbert wants to merge 100 commits into
stagingfrom
fix/ai-response-delivery-durability
Closed

Fix/ai response delivery durability#188
NathanTarbert wants to merge 100 commits into
stagingfrom
fix/ai-response-delivery-durability

Conversation

@NathanTarbert

Copy link
Copy Markdown
Collaborator

No description provided.

NathanTarbert and others added 30 commits July 13, 2026 21:42
Two stubbed endpoints behind the already-built /sync dashboard
(mapping config PUT, bulk force-sync) never got finished. Specs
closing both using existing SystemConfig table and TRACKER_SYNC job
- no new schema or job types needed.
5 tasks, each red-green tested: SystemConfig-backed mapping persistence,
loadStatusMap wiring, initializeSyncEngine override support, worker
adapter registration fix (Linear was never wired up), bulk force-sync.
loadStatusMap only truthiness-checked entry.outpostStatus before casting
it straight into the StatusMap config. A persisted row with a typo'd or
corrupted outpostStatus (e.g. 'GARBAGE') passed through untouched, so
toOutpost() later returned that literal garbage string instead of the
safe TicketStatus.OPEN fallback -- silently corrupting downstream ticket
status logic and, via the sync push path, risking a bad status write to
Linear.

Fix: reject any entry whose outpostStatus is not a real TicketStatus
enum member (Object.values(TicketStatus).includes(...)) before adding
it to the config. If every entry for a plugin is invalid, the existing
"entries present but map empty -> fallback" logic already covers it.

Call-site enumeration (grep -rn "loadStatusMap" packages/outpost apps):
- packages/outpost/shared/src/sync/index.ts:4 -- re-export only, signature
  unchanged, unaffected.
- packages/outpost/shared/src/sync/__tests__/status-map.test.ts -- updated
  with 2 new tests (invalid entry skipped / all-invalid fallback), all 7
  tests pass.
- apps/worker/src/build-sync-engine.ts:14 -- consumes the returned
  StatusMap object only; return type and success-path behavior unchanged,
  stricter filtering only improves correctness for it.
- apps/worker/src/__tests__/build-sync-engine.test.ts:7 -- mocks
  loadStatusMap entirely, never exercises real implementation, unaffected.
Two CR findings on the same function, fixed together (both live in
POST /api/sync/force):

- Narrowed try/catch to only cover request.json() parsing, so a
  mid-loop DB/queue failure propagates instead of being mislabeled
  "Invalid request body" (400).
- The "known plugin" gate now also accepts a plugin with a real
  TicketExternalLink, not just a prior SyncEvent -- previously a
  plugin's very first force-sync (real links, zero sync history)
  404'd.

Call-site enumeration: POST is framework-invoked by Next.js; no other
code calls it directly. Signature/return type unchanged.
PUT /api/sync/mappings only truthiness-checked statusMappings and
priorityMappings, so a caller could persist a garbage-shaped value
(wrong type, unknown outpostStatus/outpostPriority) and GET would
echo it back as valid config. Adds isValidMappingShape() checking
both fields are { [plugin]: Array<{external*, outpost*}> } with
outpost* values restricted to the real TicketStatus/TicketPriority
enum members, imported from @copilotkit/outpost/shared (not /db,
which only exports the prisma client).

Call-site enumeration: PUT is framework-invoked; no other code calls
it directly. isValidMappingShape is new, no other call sites.
- force/route.ts: "per changed field" implied change detection;
  the handler unconditionally enqueues both jobs.
- build-sync-engine.ts: presented Linear registration as
  unconditional; it's env-gated (LINEAR_API_KEY + LINEAR_TEAM_ID).
- mappings/route.ts: top-of-file comment said mappings are "kept as
  code defaults," directly contradicted by the PUT/GET persistence
  logic added right below it.

Comment-only, no behavior change, no test required.
…nfig

isValidMappingShape({}, ...) returned true because Object.values({}).every(...)
is vacuously true. A PUT with {statusMappings: {}, priorityMappings: {}} passed
shape validation, got persisted, and GET then returned {} for both fields
instead of falling back to defaults -- silently wiping any real config.

Now require at least one own key on the top-level object before checking
entries, so a fully-empty mapping object is rejected (400) while a config that
legitimately maps some plugins and omits others (e.g. {linear: [...]} with no
github key) still passes.

Call-site enumeration (grep -n "isValidMappingShape" route.ts):
  114: function isValidMappingShape(...)
  161: if (!isValidMappingShape(body.statusMappings, Object.values(TicketStatus)))
  168: if (!isValidMappingShape(body.priorityMappings, Object.values(TicketPriority)))
Exactly two call sites (statusMappings, priorityMappings), both unaffected for
non-empty inputs -- only a fully-empty {} now fails at each site.
PUT /api/sync/mappings validated statusMappings/priorityMappings shape
but persisted body.labelRules verbatim with zero validation, so a
caller sending labelRules: "garbage" or labelRules: 42 would have it
stored and later echoed back by GET as if it matched
Record<string, Array<{externalPrefix, outpostPrefix}>>.

Added isValidLabelRulesShape (mirrors isValidMappingShape's object/array
checks but drops the enum constraint since outpostPrefix can be any
string, including empty). PUT now returns 400 without calling
prisma.systemConfig.upsert when labelRules is present but malformed.

Call-site enumeration: grepped labelRules across apps/web/src and
packages/outpost. Only other reads are mapping-editor.tsx (reads the
already-typed GET response into client state) and mock-sync.ts (unrelated
mock/dev data). Nothing else reads body.labelRules directly, so nothing
depended on the previous unvalidated pass-through.

Tests: added a red/green case (garbage labelRules -> 400, upsert not
called; confirmed failing against old code, passing after) and a
green-only case establishing missing coverage for the valid labelRules
persistence path.
The PUT handler in apps/web/src/app/api/sync/mappings/route.ts wrapped
the entire body — including prisma.systemConfig.upsert — in a single
try/catch that mapped any failure to a 400 "Invalid request body".
A DB failure (connection issue, constraint violation) was therefore
mislabeled as a client error instead of propagating as a 500. Same bug
class already fixed in the sibling apps/web/src/app/api/sync/force/route.ts
(narrowed try/catch to only wrap request.json() parsing) but not
mirrored here.

Narrowed the try/catch to cover only `await request.json()`; the shape
validation checks and the prisma.systemConfig.upsert call now run
outside that catch, so a DB failure propagates as an unhandled error
(Next.js turns it into a 500) instead of a misleading 400.

Added a covering test mirroring the equivalent force-route test:
mocks systemConfig.upsert to reject with an Error and asserts the PUT
handler rejects/throws rather than returning a 400. Verified red
(failed against the old code, which returned a 400 response) then
green (passes with the fix) before committing.

Call-site enumeration: PUT is a framework-invoked Next.js route
handler. Only call site in the codebase is the frontend fetch('/api/sync/mappings',
{ method: 'PUT', ... }) in apps/web/src/app/sync/mappings/page.tsx —
no direct code call-sites to the exported PUT function besides the
test file. Trivially clean.
Round-3 CR finding: the docstring claimed TRACKER_SYNC jobs "no-op"
when no adapter is registered, but the handler returns
{success:false, error:'Plugin "..." is not registered'} — the job
fails and retries per the queue's normal policy, it doesn't silently
do nothing. Comment-only, no behavior change.
…iorityMappings

isValidMappingShape already rejected {} to stop empty config from
silently wiping stored statusMappings/priorityMappings. isValidLabelRulesShape
never got the same guard, so Object.values({}).every(...) vacuously returned
true and PUT with labelRules: {} passed validation and persisted an empty
labelRules object. Flagged independently by 3 reviewers across two CR rounds.

Call-site enumeration: isValidLabelRulesShape is called exactly once
(route.ts:212), guarded by `body.labelRules !== undefined` — undefined
(optional labelRules) still bypasses validation entirely, unaffected.
A non-empty valid labelRules object still passes since the new check
only rejects zero-key objects.
Standing rule going forward, not case-by-case: run
copilotkit-internal:cr-loop on the diff before pushing any
non-trivial change. Prompted by this branch's CR loop catching
real bugs across 3 rounds that would otherwise have shipped.
Cosmetic only -- embedded code-fence reflow in the plan/spec docs to
satisfy prettier --check. No content change.
The /sync dashboard persists statusMappings, priorityMappings, and
labelRules, but the worker only loaded the status map — priority/label
edits saved + displayed yet had zero effect on sync (init always used
the hardcoded createLinearPriorityMap/createLinearLabelMapper).

Mirror the status pattern:
- loadPriorityMap(plugin, db) in priority-map.ts (+ PriorityMapDb)
- loadLabelMapper(plugin, db) in label-map.ts (+ LabelMapperDb)
- priorityMapOverride / labelMapperOverride options on initializeSyncEngine
- buildSyncEngine loads all three (Promise.all) and passes the overrides
- export the loaders + Db types from sync/index

Same fallback ladder as loadStatusMap (missing row / malformed JSON /
no plugin entry / invalid entries -> hardcoded default). Priority
validates the TicketPriority enum; label validates string prefixes.

Tests: loadPriorityMap + loadLabelMapper (mirroring loadStatusMap),
init priority/label overrides, build-sync-engine wiring. 762 outpost +
1 worker test pass; typecheck clean.

Stacked on #95 (sync-mapping-persistence-bulk-force-sync); depends on
its loadStatusMap/statusMapOverride pattern.

Closes #96
…pings

fix(sync): apply persisted priority + label mappings in worker (#96)
… linked reporters, supervisor + loom rules

Workflow changes to the Weekly Community Signal routine, mirrored in the skills
(source of truth) and the Notion Playbook.

- Reddit Pulse via Composio REST + a write-scoped API key (Tools=Write in
  COMPOSIO_API_KEY), not the composio MCP (entity mismatch + read-only keys).
- Every card's Source line (and prospect Issue lines) carries `· opened
  YYYY-MM-DD` (created date, from gh issue view --json createdAt).
- Rule: the report lives ONLY in Notion, never in the codebase. Removed
  commercial-surfaces.json; product-surface-scan diffs against last week's
  Notion report. Kept reddit-pulse-seen.json as labeled dedup state.
- Reporter handles + company badges are hyperlinks on every card; enrich-reporter
  returns profile_url + company_url.
- Supervisor rule: the orchestrator verifies every subagent's output against
  source and reconciles cross-agent contradictions before publishing.
- Loom script spec: <=10 min, dive straight in, walk the report top to bottom
  (CopilotKit then AG-UI, same section order), explicit no-blame framing.
Enable lint in CI:
- turbo.json declares ESLINT_USE_FLAT_CONFIG under the lint task's env.
  turbo sanitizes the environment, so 'turbo run lint' failed with
  "couldn't find eslint.config.js" even with the variable set in the
  parent shell. The CI job already set the variable; it never reached
  eslint.
- add the Lint step to ci.yml, replacing the skip comment. The required
  check is named "Lint, Typecheck & Test" but lint never ran.
- fix the 4 errors this surfaced:
  - queue/src/types.ts: three empty interfaces (SlaCheckPayload,
    JobCleanupPayload, GithubReactionPollPayload) accepted any
    non-nullish value, defeating payload typing. Now
    Record<string, never>, which still accepts {}.
  - shared/src/platforms/slack.ts: 'blocks as any' cast removed by
    typing blocks as KnownBlock[] from @slack/web-api.

Call-site enumeration:
- SlaCheckPayload / JobCleanupPayload / GithubReactionPollPayload:
  referenced by their handlers and JobPayloadMap in queue/src/types.ts;
  all call sites pass {} or a typed payload object -> assumptions hold
  (Record<string, never> accepts {}). Verified by pnpm typecheck.
- blocks (local const, slack.ts postResponse): single push site and one
  chat.postMessage consumer, both in the same function -> no external
  call sites.

Secret hygiene: .gitignore now covers .env.*, .env.bak*, and
.railway-config-pull-*/ while keeping .env.example tracked. An
untracked .env.bak holding live Discord/Slack/Anthropic credentials was
one 'git add .' from being committed.

Docs: deployment.md said to run 'pnpm db:push', but apps/web/start.sh
runs 'prisma migrate deploy' on every boot and versioned migrations
exist. db push against a migration-managed database drifts the schema
and breaks the next migrate deploy. Documents the real mechanism and
records the absent backup/restore procedure as a known gap.
a4 pnpm lint was broken outside CI. ESLINT_USE_FLAT_CONFIG was set only in
the CI job env, so every local 'pnpm lint' died with "couldn't find
eslint.config.js". Set it once in the root lint script; turbo passes it
through via the lint task's env declaration. Verified with the variable
absent from the environment: 10/10 tasks pass.

a6 the worker was exempt from the gate this branch adds. Its lint script
was "echo 'no eslint config for worker yet'", so the service that owns the
SHADOW_MODE post-back path was never linted. Now 'eslint src/', which
surfaced two no-explicit-any errors on one line: the SyncEngine
construction cast prisma and createJob through 'any'. SyncEngineDeps
describes only the slice of Prisma the engine needs with loose
Record<string, unknown> args, so the real narrower signatures are not
assignable — the coercion is deliberate. Assert to SyncEngineDeps['prisma']
and SyncEngineDeps['createJob'] instead, so the cast is tied to a named
contract and breaks loudly if that contract changes.

a7 turbo served stale lint/typecheck passes after config edits. Added
globalDependencies for .eslintrc.cjs, tsconfig.json, vitest.config.ts,
.prettierrc. Verified: a content change to .eslintrc.cjs takes the run from
10 cached to 0 cached.

a3/a8 secret coverage gaps in .gitignore. .claude/settings.local.json holds
SLACK_WEBHOOK_URL_1 but was ignored only by a personal global gitignore, so
it was unprotected in every other clone. Added it, plus *.pem/*.key/*.p12/
*.pfx — GITHUB_PRIVATE_KEY arrives as a downloaded .pem. Collapsed the
redundant .env.bak patterns into .env.*, keeping .env.example tracked.
Verified each path with git check-ignore.

a1/a2 the worker health-port docs were inverted. apps/worker/Dockerfile sets
ENV HEALTH_PORT=3005 and exposes/probes 3005, so 3005 is the image value,
not a local-only override. The worker also resolves PORT ?? HEALTH_PORT ??
3003, so an injected PORT shadows HEALTH_PORT; the Teams bot reads only
HEALTH_PORT. Corrected the table, the note, and the health-check list.

a5 migration docs named only web. apps/worker/start.sh:4 also runs
'prisma migrate deploy', so a deploy restarting both gives two concurrent
migrators on one database. Documented that, and that Prisma's advisory lock
makes the second wait rather than corrupt.

Call-site enumeration:
- SyncEngineDeps (type-only import, apps/worker/src/index.ts): already
  exported from packages/outpost/shared/src/sync/index.ts:3, so no new
  public surface. Used only in the indexed-access positions added here.
- SyncEngine constructor: single construction site, unchanged arity and
  runtime arguments — assertions are type-level only. Verified by
  pnpm typecheck + pnpm build.
- apps/worker lint script: consumed only by 'turbo run lint'; no other
  caller references it.
- root lint script: consumed by CI's Lint step and by developers; CI keeps
  its own job-level env var, so the two are independent.
NathanTarbert and others added 26 commits August 11, 2026 17:42
Create stored the ticket's sourceId one way and reply looked it up another:
`message.threadId ?? null` on create vs `message.threadId ?? ''` on lookup.
With no threadId the two could never agree, so every message in that
conversation looked like a brand-new ticket and drew its own AI answer --
defeating the one-answer-per-ticket rule this branch enforces. The Slack
composite key was asymmetric the same way (create required BOTH channelId
and threadId; the lookup built "channelId:" from channelId alone), and
apps/slack-bot/src/events/message.ts built a third variant,
"C123:undefined".

Root cause was three-way duplication of the key-building logic, so the fix
is one definition -- shared/src/platforms/source-id.ts's
buildTicketSourceId(source, threadId, channelId) -- that both the writer and
every reader derive the key from. findTicketBySourceAndThread(source,
threadId, channelId) becomes findTicketBySourceId(source, sourceId): the
reader takes the finished key and has no key-building code left to disagree
with.

Deliberate decision for "no threadId": the helper returns null, not a
placeholder. A ticket with no thread key cannot be found again by any
lookup, so writers store null (the ticket is still created -- dropping a
report is worse) and readers treat null as not-lookup-able and skip the
query entirely rather than searching for '' or "C123:", which can only be a
false miss or a false hit on a malformed row. Slack with no channelId is
also null now instead of a bare thread_ts: a thread_ts is only unique within
a channel, and both Slack post paths already threw for those tickets because
ticket.channel is null in exactly that case. Documented in the module's doc
comment along with the "never inline `${channelId}:${threadId}`" rule.

Tests, red-green verified: reverting inbound.ts to the two inline paths gives
5 failed / 37 passed, widened to 8 failed / 38 passed once the unaddressable
rows join the write/read symmetry table; breaking the helper to return
placeholders gives 3 failed / 3 passed in the new unit test; restored, all
green. Covers the previously-untested Slack empty-threadId lookup arm, the
Slack no-channelId arms, and the no-threadId case for a non-Slack source
(create stores null, reply issues no query, and a threadId-less reply is not
matched against a sourceId: null ticket).

Verified: packages/outpost `vitest run` 984 passed / `tsc -p
shared/tsconfig.json --noEmit` clean; slack-bot 52, discord-bot 57,
teams-bot 56, github-app 37 passed; `tsc --noEmit` clean in all four apps.
eslint cannot run repo-wide (no flat eslint.config.*) -- pre-existing.

Out of scope, untouched: the (source, sourceId) uniqueness constraint / any
Prisma migration, and the Slack subtype filter -- both filed as separate
follow-ups.

Call-Site Enumeration
---------------------
ADDED buildTicketSourceId -- grep -rn "buildTicketSourceId" --include="*.ts",
all 7 references:
- shared/platforms/inbound.ts handleNewTicket: holds. Wants the key to store;
  null is a valid Ticket.sourceId (nullable column, PrismaLike.create already
  types it string|null).
- shared/platforms/inbound.ts handleReply: holds. Wants the key to search for;
  narrows null before calling findTicketBySourceId(...: string).
- shared/platforms/index.ts re-export: holds. Value export from a module that
  imports only ../types.js, so no adapter runtime is pulled in.
- shared/index.ts re-export: holds. Same reasoning -- the barrel's
  "platform types only, browser-safe" contract is preserved; justified inline.
- apps/slack-bot/src/events/message.ts: holds. source is always
  TicketSource.SLACK there (SlackAdapter.parseInboundEvent sets it), so the
  Slack arm applies; a null key returns early instead of querying.
- apps/slack-bot/src/lib/tickets.ts findTicketByThreadTs: holds. Its callers
  (events/actions.ts:22,:74, commands/assign.ts) were written against findFirst
  and already treat null as "not found", so an early null is indistinguishable.
- shared/__tests__/platforms-source-id.test.ts: holds -- unit tests of it.

REMOVED findTicketBySourceAndThread -- 2 hits before (definition + its single
call, both in inbound.ts), 0 hits repo-wide after. It was private, so no
external consumer was even representable. Negative finding: nothing dangles.
Near-miss namesake apps/github-app/src/lib/tickets.ts:11 exports a different
findTicketBySourceId(sourceId) -- separate package, not imported here, not
touched, no collision.

CHANGED the sourceId value written on create -- every writer/reader in the repo:
- apps/web/.../webhooks/postmark/route.ts:112 writes body.MessageID: holds.
  Not routed through InboundHandler, and the helper is the identity for EMAIL,
  so the formats already agree if it ever is.
- apps/discord-bot/src/lib/shadow-mode.ts:76 writes thread.id: holds --
  identical to buildTicketSourceId(DISCORD, thread.id).
- apps/discord-bot/src/lib/tickets.ts:10 and apps/teams-bot/src/lib/tickets.ts:10
  read the bare threadId/conversationId: holds. Non-Slack keys are verbatim, so
  these match what create stores. Left inline deliberately -- for non-Slack the
  builder is the identity function, there is no format to duplicate.
- apps/github-app/src/lib/tickets.ts:11,28 read a prebuilt owner/repo#n: holds,
  identity for GITHUB_ISSUE/GITHUB_DISCUSSION, unchanged.
- shared/platforms/slack.ts:178 extractThreadTs splits on the first ':': holds.
  The composite format is byte-identical; only would-be bare Slack keys became
  null, and those tickets already threw on both post paths.
- shared/platforms/github.ts:418 parseSourceId regex: holds, GitHub keys
  unchanged. queue/handlers/github-reaction-poll.ts:22,87 likewise, and it
  already null-guards ticket.sourceId.
- queue/handlers/ai-response.ts:276 passes ticket.sourceId to the adapter:
  holds -- already string|null per PlatformAdapter.postResponse, and adapters
  throw loudly on null, the correct outcome for an unaddressable ticket.
- shared/platforms/{discord,teams,email-postmark}.ts post paths: hold. All
  already null-guard sourceId and throw a clear error -- no silent misroute.
- TicketSource is still imported in inbound.ts (used by toPlatformTarget), so
  the import is not dead; confirmed by clean tsc.

(cherry picked from commit 33fd91ca68ca1387f29d87e30d0a8afec79a626c)
handleReply stopped enqueuing AI_RESPONSE, but on a ticket-lookup MISS it
fell back to `handleNewTicket({ ...message, isThreadStart: true })` — and
handleNewTicket enqueues. A mid-thread reply became a brand-new "ticket"
titled with the follow-up text, and the bot answered it. The one-answer-per-
ticket rule was routed around by the same file that documents it.

Reachability was not theoretical:
- apps/slack-bot/src/events/message.ts pre-filters replies whose thread is
  untracked, so Slack was shielded.
- apps/teams-bot/src/handlers/message.ts has NO such pre-filter and its
  monitored-channel gate only runs for thread starts, so a Teams reply
  reached the fallback.
- Discord and GitHub reached it for any thread predating Outpost.

handleNewTicket now takes an explicit `{ answer: boolean }` decision from its
caller instead of inferring one. handle() passes `{ answer: true }` for a
genuine thread start; the orphaned-reply fallback passes `{ answer: false }`.
No duplication of handleNewTicket's body, and InboundResult.aiJobEnqueued
stays truthful (false on that path).

ASSUMPTION, stated in the code comment so it is reviewable: an orphaned reply
still CREATES a ticket and persists the message — dropping a customer's
message is worse than filing an oddly-titled ticket — but it is never
answered, because the message that opened the real conversation was never
seen by us. A human picks it up from the dashboard.

Also corrects the now-falsified claim in queue/src/handlers/ai-response.ts
that the ticket-history gate makes the invariant "unroutable-around". That
gate only sees messages on the ticket, so a ticket freshly minted around a
mid-thread message has no prior AI response and sails through it. The gate is
a re-answer guard, not a total gate; the orphaned-reply case must be refused
at the enqueue site, and the comment now says so and points at it.

Tests: both pre-existing fallback tests asserted nothing about createJob,
which is exactly why this shipped. Both now assert it, plus new coverage for
Teams/Discord/GitHub/Slack orphaned replies, a non-team-member sender (the
case that WOULD have been answered), and a genuine thread start still being
answered so the fix is not a blanket mute.

Red-green verified: with `{ answer: false }` flipped to `{ answer: true }`,
7 tests fail in packages/outpost (2 files) and 1 in apps/teams-bot; restored,
all green.

Call-Site Enumeration
---------------------
Symbols changed: `InboundHandler.handleNewTicket` (signature — added required
second param `{ answer: boolean }`).
Symbols added: none exported. Symbols removed: none.

`handleNewTicket` — private; grep over the repo (excluding node_modules)
returns exactly three hits, all in shared/src/platforms/inbound.ts:
  - :147 the declaration.
  - :133 `handle()` thread-start branch → passes `{ answer: true }`.
    Assumption holds: isThreadStart=true is the opening message, the one
    message Outpost may answer. Behavior byte-identical to before.
  - :252 `handleReply` orphan fallback → passes `{ answer: false }`.
    Assumption deliberately INVERTED here; that is the fix.
  No external caller exists and none can be added by accident: the method is
  `private` and is not re-exported from platforms/index.ts or shared/index.ts
  (verified — those export the class, InboundHandlerConfig, CreateJobFn only).

`handleReply` — private; hits at :135 (sole caller, the non-thread-start
branch of handle()) and :226 (declaration). Signature unchanged; its return
type and every non-orphan path are untouched. Third hit is a comment
reference in queue/src/handlers/ai-response.ts:100 (prose, no call).

`InboundResult.aiJobEnqueued` — non-test consumers:
  - shared/src/platforms/types.ts:165 — the declaration, `boolean`, unchanged.
  - apps/discord-bot/src/events/message-create.ts:68 — logs
    " (AI job enqueued)" when true. Assumption still holds and is now MORE
    accurate: on an orphaned Discord reply the log no longer claims an
    enqueue that would have happened. Log text only, no control flow.
  NEGATIVE FINDING: no other runtime code reads aiJobEnqueued — not the Slack,
  Teams, or GitHub bots, not the queue, not the dashboard. grep for the
  identifier over apps/ and packages/ returns only the above plus test files.
  So no caller branches on it and no caller can be broken by it flipping to
  false on this path.

`isTeamMember` — still called exactly where it was, now guarded by `answer &&`
short-circuit so the lookup is skipped when no answer is possible. NEGATIVE
FINDING: the returned value was used for nothing but the enqueue decision in
handleNewTicket (the reply path calls it separately for its own status
transitions, untouched), so skipping the query cannot change any other
observable behavior. The reply path's own isTeamMember call at :281 is
unaffected.

`handle` (public entry point) — signature and return type unchanged; all bot
call sites (apps/discord-bot thread-create.ts + message-create.ts,
apps/slack-bot events/message.ts, apps/teams-bot handlers/message.ts,
apps/github-app webhooks/issues-opened.ts + discussion-created.ts) compile and
pass unchanged. NEGATIVE FINDING: no bot needed a per-platform change; the fix
is entirely in the shared handler, so all four platforms are covered at once
and no bot can opt out.

`ai-response.ts` change — comments only, zero code touched, so it has no call
sites and no type surface.

Verification
------------
- packages/outpost: `npx vitest run --reporter=dot` → 60 files, 972 tests, all
  passing. (`npx prisma generate` was needed first in this fresh worktree;
  without it queue/src/__tests__/scheduler.test.ts fails to load on a missing
  .prisma/client — pre-existing env setup, not a code failure.)
- `npx tsc --project shared/tsconfig.json --noEmit` → clean.
- apps/teams-bot 57, apps/discord-bot 57, apps/slack-bot 50,
  apps/github-app 37 — all passing.

(cherry picked from commit 6f14b00703f954280922af044bc2500337ef41f9)
The one-response-per-ticket guard reads the BOT Message row, which is
committed BEFORE the platform post-back. That made two pre-existing
soft-failure paths permanent:

1. The `suggestedResponse` ticket.update ran unguarded between the BOT row
   and post-back. A throw there aborted the job; every retry then hit the
   guard, returned success with skipped: true, and nothing was posted or
   escalated.
2. `adapter.postResponse` throwing was logged and the job still reported
   success. Before the guard a manual re-enqueue could still deliver the
   answer; after it, that door is closed.

Either way the reporter is silent forever while the DB says they were
answered. The guard is the requirement, so it is untouched — instead every
path where the answer failed to reach the reporter now hands the thread to a
human in the same run:

- `suggestedResponse` write is non-fatal (logged); it can no longer strand
  the job before delivery is attempted.
- postResponse throwing, and getAdapter throwing, record a deliveryFailure.
- deliveryFailure enqueues ESCALATION with a reason naming the failure, and
  wins the reason slot over suppression / low confidence (most actionable).
- If that ESCALATION enqueue also fails, the job returns success: false so
  the worker records a failed attempt with the reason on the job row — the
  one outcome with neither delivery nor a human must not look like success.
  Escalation-enqueue failures for low confidence / suppression keep the
  historical success result: there the response did reach the reporter.
- The externalCommentId write moved out of the post-back try so bookkeeping
  failure is not misread as delivery failure.
- Sources with no adapter escalate only if the suggestedResponse write
  failed, since there suggestedResponse IS the delivery path. SHADOW_MODE
  posts nothing by design and never escalates on that basis.

Out of scope (filed separately): the guard's check-then-write race under
AI_RESPONSE concurrency 4, whose fix is a uniqueness constraint plus a
migration. Nothing here narrows or widens that window — no ordering of the
guard read or the Message create changed, and no transaction was added.

Call-Site Enumeration
---------------------
`handleAiResponse` (exported; signature unchanged)
- packages/outpost/queue/src/index.ts:4 re-export — holds, same signature.
- apps/worker/src/index.ts:82 `worker.on(JobType.AI_RESPONSE, ...)` — holds.
  The new success: false path is a JobResult the worker already handles at
  worker.ts:351 via handleFailure (retry ladder + error text persisted on the
  job row). A retry after that failure is skipped by the guard and returns
  success, which completes the job; the recorded error text remains on the
  row, so the operator signal survives. That is intended: the answer is
  undeliverable, so retrying generation is pointless.
- queue/src/__tests__/ai-response.test.ts — updated, all call sites reviewed.
- No other callers (grepped `handleAiResponse` across packages/ and apps/).

JobResult.data key `escalated` (semantics widened: now also true on delivery failure)
- Grepped `escalated` across packages/ and apps/: every hit is unrelated
  (AI disclaimer copy, escalation handler log lines, bot escalate commands)
  except ai-response.test.ts. NEGATIVE FINDING: no production reader of
  AI_RESPONSE's result.data exists — worker.ts only inspects `success` and
  stores nothing from `data` — so widening it breaks nothing.

JobResult.data key `deliveryFailed` (new)
- Only readers are the new tests. No dashboard/API code reads AI_RESPONSE
  job result payloads (grepped `result.data` under queue/src and apps/).

ESCALATION payload `reason` (new value shape for the delivery case)
- queue/src/handlers/escalation.ts:33,66,108,130 — holds. `reason` is used
  only as free text: interpolated into the routing reason, the assignment
  note and a SYSTEM message. No parsing, no enum matching, no length limit.
- apps/* escalate commands construct their own reasons; unaffected.

New locals (`deliveryFailure`, `escalationEnqueueError`, `suggestedResponseError`,
`escalationReason`, `escalated`) and the widened scope of `externalCommentId`
- All function-local to handleAiResponse; no exports, no references outside
  the handler body. `externalCommentId` changed from a `const` inside the
  post-back try to a `let` in the enclosing block; its only consumer is the
  message.update below it, which now runs on the delivered path only.

Tests
-----
Red-green verified for each behaviour: dropping the postResponse
deliveryFailure assignment (3 red), restoring the unguarded suggestedResponse
write (2 red), disabling the success: false return (1 red), and moving the
externalCommentId write back inside the post-back try (1 red).

`npx vitest run` — 973 passed (60 files).
`npx tsc --project queue/tsconfig.json --noEmit` — clean.

(cherry picked from commit 71dd6a9702f1b7811cee7ae2237bfd3233481cb0)
Outpost answers exactly one message per ticket -- the one that opened it
(the invariant the handler already documents and enforces at step 1b).
The question selection contradicted that: it scanned ticket.messages in
REVERSE and took the LATEST type: 'USER' row.

Replies are still persisted as USER messages, and that is correct -- they
belong in the thread's history. The consequence was that a reporter who
split a thought across two Discord messages in the seconds between ticket
creation and the job dequeuing had the ticket's one and only answer aimed
at the follow-up fragment ("btw I'm on the app router") instead of the
question that opened the thread. One shot, spent on the wrong sentence.

ticket.messages is loaded orderBy: { createdAt: 'asc' }, so the fix is a
forward .find() for the first USER row. The ?? ticket.description ??
ticket.title fallback chain is unchanged.

Judgement call -- conversation history stays FULL
------------------------------------------------
conversationHistory still carries every non-SYSTEM message, including any
follow-up that landed after the opening message, even though the QUESTION
is now the opening message. The two inputs answer different questions:
`question` is what to respond to, `conversationHistory` is what the
responder knows. A Discord follow-up is usually the same thought continued
-- a stack trace, a version number, "on Next 15" -- and it is exactly the
detail that makes the single allowed answer good, so truncating history
would trade a targeting bug for a worse answer. Truncation would also
require inventing a second policy for the non-USER rows after the opening,
with no evidence behind it. Consecutive same-role turns are not a new
condition: the generator already appends `question` after the history, so
the base single-message case has always produced two user turns in a row.

Call-Site Enumeration
---------------------
- `latestUserMessage` (removed, function-local const): repo-wide grep over
  *.ts/*.tsx/*.md (excluding node_modules and dist) returns ZERO remaining
  references. Nothing outside the handler could see it; it never escaped
  the function body. Negative finding: no docs or comments named it either.
- `openingUserMessage` (added, function-local const): 1 reference, the very
  next line (line 144). Not exported, not part of any type. No other call
  site can be affected.
- `question` (unchanged name, changed VALUE): 1 consumer inside the handler
  -- `pipeline.generateSupportResponse(question, ...)` at line 175. Its
  contract is "a support question as a string"; still a string, still
  non-empty via the same fallback chain, so the assumption holds. The
  fallback ordering and the SUPPRESSED/groundedness paths downstream are
  value-agnostic and unaffected.
- `pipeline.generateSupportResponse` (signature untouched): other callers
  are packages/outpost/ai/src/pipeline{,-groundedness}.test.ts (pass their
  own literal questions) and apps/web/src/app/api/qa/route.ts (passes the
  user's typed QA question, no ticket involved). Negative finding: none of
  them reads a Ticket's messages, so none inherits this behaviour change.
- `conversationHistory` (unchanged shape and value): consumed by
  AIPipeline.generateSupportResponse -> generator.buildMessages
  (ai/src/generator.ts:222). Unchanged, so its assumptions hold by
  construction.
- `ticket.messages` ordering assumption: the only producer is the
  `orderBy: { createdAt: 'asc' }` include at line 65 of this same file --
  same function, no other loader feeds this variable. The step-1b
  already-answered guard also uses a forward `.find()` and is unaffected.

Tests
-----
New describe block "answers the message that opened the ticket" in
queue/src/__tests__/ai-response.test.ts: the key case (opening USER +
later USER -> generates against the opening, asserted on
mockGenerateSupportResponse.mock.calls[0][0]), the companion assertion
that the interim follow-up still arrives as conversationHistory, a
leading-SYSTEM-row case, and the no-USER-message description fallback.
The pre-existing "filters SYSTEM messages from conversation history" test
encoded the bug in its expected question ('Follow up question') and now
expects the opening ('Hello'); its history assertion is unchanged, which
is what pins the judgement call above.

Red-green verified: with the source reverted to the reverse().find() form,
3 tests fail (the two new targeting tests plus the corrected SYSTEM-filter
test); restored, 968 tests pass.

Gates: `npx vitest run --reporter=dot` in packages/outpost -> 60 files,
968 tests, all passing. `npx tsc --project queue/tsconfig.json --noEmit`
-> clean (sibling packages built first so the workspace `dist` type
entrypoints resolve). Prettier clean on both touched files.

(cherry picked from commit 297e555ac62bd88b519a7fb90d7825b3965f43f4)
The one-response-per-ticket guard returned right after reportProgress(20),
so a job that succeeded by deciding to do nothing persisted progress=20 on
its Job row and read as hung to anything watching job progress. Walk the
ladder to 100 before returning, like every other successful exit.

Failure exits are left as-is on purpose: the Job row carries status FAILED
next to the number, so a partial progress value is the honest reading there
and 100 would falsely claim completion.

(cherry picked from commit a65c2eaef1e088b24719d622468840415bc0a1e0)
The guard's predicate is `m.type === 'BOT' && m.isAiGenerated`, but the
tests only pinned the first half. Deleting `&& m.isAiGenerated` left the
whole suite green, because no fixture carried a BOT row with
isAiGenerated: false — the shape a human reply sent from the dashboard
persists as.

Two changes, tests only:

- Spell out isAiGenerated on every message fixture. The DB column is
  non-nullable, so rows that omit it are a shape the handler never sees,
  and the omission let the guard be satisfied by `undefined`.
- Add the BOT + isAiGenerated: false case: a teammate's reply goes out
  over the bot channel but is not Outpost's one answer, so the AI's
  single response must still be generated and posted.

Mutation-verified: dropping `m.type === 'BOT'` fails the SYSTEM
shadow-mode-log test; dropping `&& m.isAiGenerated` fails the new
human-BOT-reply test (and the SYSTEM-history filter test). Each half is
now independently pinned.

(cherry picked from commit 35610d3fa08c9c37921508de21f36ecf02b765c4)
The one-response-per-ticket change made replies never enqueue an AI_RESPONSE
for any sender, which quietly turned three "team member reply gets no AI
response" tests tautological -- they would keep passing with team-member
detection deleted outright.

Team-member detection still decides aiJobEnqueued on the NEW-TICKET path, so
that is where the assertion belongs.

- packages/outpost/shared/.../platforms-inbound.test.ts: deleted 'skips
  AI_RESPONSE for team member reply'. The reply rule is already covered by two
  explicit tests, and the new-ticket path already has 'skips AI job when sender
  is a team member' plus the whole 'team member detection' block. A comment
  records why no team-member reply test lives there.
- apps/discord-bot: deleted the reply-path test in message-create.test.ts and
  repointed it into thread-create.test.ts (discord's new-ticket path), which had
  no team-member coverage at all.
- apps/slack-bot: repointed in place -- the same handler serves both paths, so
  the test moved from 'threaded replies' to 'new top-level messages'.

Red-green verified with `return false` at the top of InboundHandler.isTeamMember:
both repointed tests fail, and the three surviving shared-package team-member
tests fail with them. Restored -> all green. No production code touched.

(cherry picked from commit ad620a45f82648dff0f408a54b7daff186636d46)
The prose around the one-answer invariant made claims the code does not
support, and framed the enforcement backwards.

Corrections:

- inbound.ts `handle()` / module header: path 1 enqueues AI_RESPONSE only
  when the sender is not a team member, and it is the only path here that
  enqueues at all. Team status no longer gates the reply path in any way.
- ai-response.ts header: the step list omitted step 1b (already-answered
  gate), step 5b (platform post-back), and the SHADOW_MODE branch that
  replaces post-back with a SYSTEM message.
- ai-response.ts gate comment: "Five separate code paths could enqueue
  AI_RESPONSE" was both miscounted and wrong about which. Three enqueue
  sites exist -- InboundHandler.handleNewTicket (Discord, Slack and Teams
  all funnel through it), handleShadowThreadCreate in the Discord
  shadow-mode path, and the Postmark new-email branch.
- The gate is a RE-ANSWER guard, not the enforcement point, and the
  comments now say so everywhere they mention it. A ticket freshly minted
  around a mid-thread reply carries no prior AI response and passes the
  gate untouched; likewise a reply on a ticket Outpost never answered. The
  enqueue-site refusals are what actually hold one-answer-per-ticket, so
  calling them a cost optimisation or "the cheap arm" was backwards.
- source-id.ts: a writer/reader key mismatch no longer means "every reply
  gets its own AI answer" -- an unmatched reply is filed as an untracked
  ticket with no answer. The real damage is the lost reply and the
  duplicate stub.
- shadow-mode.ts: the enqueue comment said the worker calls
  logShadowResponse; it writes the shadow row inline and never calls it.
  The logShadowResponse docstring said NOTE-type; the row is SYSTEM.

Also trimmed the "this used to..." narrative, which was repeated in four
places, down to the two sites where it genuinely stops the bug being
reintroduced.

Comments only -- no behavior change, so no test accompanies it.
Verified with `npx turbo typecheck` and `npx turbo test` (all green).

(cherry picked from commit 69b4c941a8d8ec18e4b70f65c6b214a3c66bf14d)
Call sites audited:
- handleMessage dispatch: apps/teams-bot/src/index.ts -> handlers/message.ts
- TeamsAdapter.parseInboundEvent: handlers/message.ts; teams-adapter.test.ts; shared platforms-adapters.test.ts
- orphan reply path: InboundHandler.handleReply -> handleNewTicket(answer=false) -> Teams isNewTicket acknowledgment branch
- policy tests: message.test.ts; teams-adapter.test.ts; inbound-handler.test.ts
Review of #170 caught a regression this branch introduced. The orphaned-reply
fallback creates a ticket with `{ answer: false }` so no AI response is
enqueued, but it routes through `handleNewTicket`, which returns
`isNewTicket: true` unconditionally. The Teams handler branches on exactly that
flag to write a conversationReference and post an acknowledgement card.

So a mid-thread "thanks, that worked!" -- Teams sets isThreadStart from the
absence of replyToId, and no ticket matches -- made the bot post a card into a
conversation it was never part of. That is the unwanted-chatter class this
branch exists to remove, reintroduced on the platform most reachable for the
orphan path.

`InboundResult` now carries `isOrphanedReply`, set true only on that fallback,
and `handleNewTicket` takes `{ answer, orphanedReply }` as two independent
decisions rather than inferring one from the other. The Teams handler gates
both the card and the additionalInfo write on it. `isNewTicket` stays true: a
ticket really was created, and other consumers may reasonably care.

The existing handleMessage orphan test asserted the bug -- it expected
sendActivity to have been called once -- so it would have kept this green
forever. It now asserts the ticket and message are still persisted while
createJob, sendActivity and ticket.update are 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.

Call-site enumeration for isOrphanedReply -- 1 consumer changed, 7 cleared:

  - apps/teams-bot/src/handlers/message.ts -- CHANGED. The only consumer with a
    reporter-visible side effect on the new-ticket branch.
  - apps/slack-bot/src/events/message.ts -- cleared: pre-filters replies whose
    thread has no ticket, so it cannot reach the fallback.
  - apps/discord-bot/src/events/message-create.ts -- cleared: same pre-filter.
  - apps/github-app/src/webhooks/issue-comment.ts -- cleared: returns when no
    ticket matches, and never uses InboundHandler.
  - apps/github-app/src/webhooks/issues-opened.ts and discussion-created.ts --
    cleared: isThreadStart is always true, and neither has a reporter-visible
    side effect on that branch.
  - apps/discord-bot/src/events/thread-create.ts -- cleared: no ack post since
    this branch removed it.
  - apps/web -- cleared: never uses InboundHandler; Postmark has its own path
    and already implements the orphan rule locally.

Two further review findings, same shape as the above:

Route the last three inlined `Ticket.sourceId` sites through
`buildTicketSourceId` -- shadow-mode.ts on the write side, discord-bot and
teams-bot lib/tickets.ts on the read side. Identity for both platforms today,
so this fixes no live bug; it closes the drift surface the helper exists for,
which already produced one write/read mismatch on this branch. Readers return
early on an unaddressable key instead of querying `sourceId: null`, which would
match unrelated keyless rows. Adds tickets.test.ts for discord-bot, which had
none, and stops shadow-mode.test.ts stubbing the whole shared module -- the
helper was mocked away, so no test there could have caught a drift.

Correct comment prose that overstated what the code does. The literal —
escape was in four files, not the two previously fixed -- github-app's
issues-opened and discussion-created carried it too, plus a mangled ellipsis.
The orphaned-reply rationale in inbound.ts read as a universal principle, but
Discord, GitHub and Slack all pre-filter before reaching it and Postmark
implements it locally, so Teams is the only caller that arrives there; the
comment now says so. The Teams ack card's divergence from Discord and Slack,
which post nothing, is recorded as deliberate rather than left to look like an
oversight.

Not addressed here, both belonging to the email-threading follow-up:
postmark/route.ts inlines the EMAIL key as body.MessageID in three places, and
a plain reply with no plus-address is still treated as new and answered,
because sourceId is the per-email MessageID and there is no In-Reply-To or
References fallback.
…ures legible

Production could not deploy from 2026-08-07 to 2026-08-12. Every attempt built
and pushed an image, then failed the healthcheck eleven times over five minutes
with "1/1 replicas never became healthy". The replica serving production was the
2026-08-03 image the whole time.

Root cause: the production database was missing the SystemConfig table.
_prisma_migrations recorded 0001_init as applied, but the table that migration
declares did not exist. Prisma tracks migrations by name, so `migrate deploy`
reported "No pending migrations to apply" on every deploy and never revisited it.
Every other model and enum in schema.prisma was present -- SystemConfig was the
only gap, confirmed by comparing all 22 models against pg_tables.

The worker reads SystemConfig at boot through buildSyncEngine (the persisted
status / priority / label mapping configs). That read threw, and because it ran
at module scope ABOVE the health server, the process exited before anything bound
the port. Railway had nothing to report but a timeout, which looks exactly like a
broken image. Nine days of a five-minute silence.

Three changes, in the order they matter:

1. A repair migration creating SystemConfig. IF NOT EXISTS, because environments
   whose 0001_init did create the table must no-op rather than fail. Column
   definitions copied verbatim from the SystemConfig block in 0001_init.

   The table has already been created directly in production to end the outage,
   so this migration no-ops there. It matters for staging -- whose worker last
   deployed 2026-07-24, before this code existed, and would otherwise hit the
   same wall -- and for any environment created from here.

2. A schema-drift guard in apps/worker/start.sh. `migrate deploy` only compares
   the migrations directory against _prisma_migrations; it never inspects the
   real schema, so a migration recorded as applied but never executed is
   invisible to it. `migrate diff --from-schema-datasource --to-schema-datamodel
   --exit-code` compares the LIVE DATABASE against schema.prisma and exits
   non-zero on any difference, which is exactly this class. Verified against the
   production database: it reports "No difference detected" now that the table
   exists, and would have named the missing table before.

   Deliberately fatal rather than a warning. The worker's sync mappings come from
   the database, and one running against a schema it does not match would write
   wrong statuses to Linear. Failing the deploy keeps the previous replica up.

3. /health now binds before anything touches the database. Fail-fast is retained
   -- a worker that could not read its mappings still must not be reported
   healthy -- but failing is no longer silent. The port binds first, boot state is
   tracked, and /health answers 503 with the phase and the error message while
   boot is unfinished or failed. Railway still fails the deploy and keeps the
   previous replica, so the outcome is unchanged; the difference is that the
   reason is now visible from a single probe.

   The payload logic is extracted to apps/worker/src/health.ts because index.ts
   is a top-level-await module that binds a port and starts polling on import, so
   its boot path cannot be exercised from a test. buildHealthResponse is pure and
   pinned by 5 tests: ready reports 200 with the worker snapshot; failed reports
   503 carrying the reason; starting reports 503; phase 'ready' with no worker
   still reports 503; and a failed boot never returns 200 even with a worker
   snapshot present. Red-green verified -- reverting to the old always-200
   behaviour fails 4 of them.

Call-site enumeration:

  - buildHealthResponse / BootState (new, apps/worker/src/health.ts) -- 1 consumer,
    the /health handler in index.ts. Not exported from a package entry point; the
    worker app is not a library.
  - worker and scheduler in index.ts changed from `const` to nullable `let`,
    since both are now constructed after the health server binds. Two reads:
    the health handler (guards on `worker` being non-null) and shutdown() (uses
    optional calls). No other module imports them -- index.ts is the app entry
    point and exports nothing.
  - start.sh -- one caller, the Dockerfile CMD. Syntax checked with `sh -n`.

Verification: turbo typecheck 10/10, turbo test 19/19 (1770 tests). The drift
guard was run against the live production database rather than only reasoned
about.

Not addressed here: why 0001_init was recorded as applied without creating that
table. The likeliest explanation is that the schema was created with `db push` at
some point and the migration marked applied, which means the same drift may exist
in staging. The guard in (2) turns that from an invisible failure into a loud one,
but confirming staging needs a run from inside Railway -- its Postgres has no
public URL.
Review of the previous commit turned up four issues, one of which made its
headline change inert.

1. The catch around buildSyncEngine rethrew. This module is a top-level-await
   entry module, so an exception escaping evaluation rejects the module's
   evaluation promise, Node reports it as uncaught, and the process exits 1 --
   a listening HTTP server does not keep it alive. /health never answered a
   single probe, and the log line promising "will report 503 with this reason"
   was false. Verified A/B against an unreachable DATABASE_URL: before, the
   process exits 1 and curl gets connection refused; after, it stays up and
   serves 503.

   The whole boot sequence now lives in startWorker() and is awaited in a
   try/catch that records the reason instead of rethrowing. Worker construction,
   scheduler.start() and worker.start() sat outside the old try and would have
   died the same silent way; they are inside it now.

   Fail-fast is unchanged in outcome: the healthcheck still fails, Railway still
   fails the deploy and keeps the previous replica. It just says why now.

2. /health echoed the raw exception. Prisma's connectivity errors quote the
   database host, port and user (P1001, P1000), and the endpoint is
   unauthenticated. summarizeBootError keeps the message only for the
   schema-shape codes that name the missing object (P2021/P2022) -- the actual
   diagnostic payload -- and reduces everything else to error class plus code,
   leaving the full text to the logs. The live failure turns out to be
   PrismaClientInitializationError with errorCode undefined, hence the
   class-name fallback.

3. The drift guard treated `migrate diff` exit 1 (CLI or connectivity failure)
   the same as exit 2 (drift detected), so a database blip during deploy printed
   "the database does not match schema.prisma" with no diff above it. It now
   captures the status and reports the two cases separately.

4. The health test fixture asserted on `activeJobs`, which is not a field of
   WorkerHealthStatus, and index.ts cast the real snapshot through `unknown`, so
   nothing checked the shape actually served. buildHealthResponse now takes
   WorkerHealthStatus | null and the fixture is built from that type. The spread
   is reordered so the envelope's own `status` cannot be shadowed by a future
   field of the same name, with a test pinning it.

turbo typecheck 10/10, turbo test 10/10, start.sh drift branches exercised at
exit 0/1/2 with a stubbed prisma.
Follow-up to the boot restructure. Two problems, the second found by testing
the first.

Signal handlers were registered after the boot await, so a SIGTERM arriving
while module evaluation was still suspended found no handler and killed the
process outright. That window used to be short; now that a failed boot parks
the process alive on a 503, it is exactly when Railway tears a bad deploy down.
Registration moves above the boot block. worker/scheduler are still null in
that window, so the optional calls no-op and shutdown reduces to closing the
port and dropping the Prisma connection.

Moving it up surfaced a latent bug: `process.on('SIGTERM', () => shutdown(...))`
never handled shutdown's returned promise. Signalled mid-boot, $disconnect()
rejects with P2024 while tearing down a pool that never filled, and the
unhandled rejection killed the process with a stack trace mid-shutdown --
replacing a clean stop with a crash. shutdown() now catches, reports the failed
stop and exits non-zero rather than claiming success, guards against a second
signal re-entering, and carries a 10s unref'd watchdog so a hung stop() or
$disconnect() cannot outlive Railway's grace period.

Verified against a hanging database (240.0.0.1, connect never completes):
SIGTERM mid-boot now logs "Shutdown failed: <reason>" and exits 1 in 7s with no
unhandled rejection, where before it dumped a PrismaClientInitializationError
stack. SIGTERM after a failed boot shuts down cleanly in 1s and exits 0.

turbo typecheck 10/10, turbo test 10/10.
Round 1 of the 7-agent CR on the two prior commits. Findings converged hard:
six of six agents that looked at /health found the same 200-when-not-healthy
bug, five of five found the port and bind-error crashes. Most of what follows
is a defect in the two commits I added today, not in the original hotfix.

BOOT / SHUTDOWN LIFECYCLE

- Boot could resume AFTER shutdown began. shutdown() and startWorker() are
  independent promise chains and nothing sequenced them, so a SIGTERM during
  buildSyncEngine() let the boot come back behind it: Scheduler.start() ticks
  every definition immediately and Worker.start() begins claiming jobs, and the
  pending process.exit then stranded freshly-claimed rows in PROCESSING.
  startWorker() now checks the shutdown flag after the await.
- Boot failure was permanent. Railway's healthcheckPath gates a NEW DEPLOYMENT
  and does not restart a running service; restartPolicyType="ALWAYS" is
  restart-on-exit and can never fire on a process that never exits. So a 20s
  Postgres failover during an ordinary container restart wedged the worker at
  zero jobs until a human noticed -- strictly worse than the crash-loop it
  replaced. The reason is now published for BOOT_FAILURE_LINGER_MS and then the
  process exits 1 so the restart policy retries. Diagnosable and self-healing.
- watchdog.unref() disabled the watchdog in exactly its motivating case: with
  the server closed and $disconnect() hung, no referenced handle remains, so
  Node exited 0 reporting a clean stop for a shutdown that never finished.
  Verified with a timer that never fires unref'd and fires ref'd.
- The 10s watchdog was shorter than jobTimeouts (300s) that Worker.stop() waits
  on, so any deploy landing mid-job was guaranteed a forced exit(1). Sized above
  the drain it guards.
- healthServer.close() ran after the drain, advertising a healthy worker for up
  to 300s after it had committed to dying. Closed first now.
- A partially-started boot left the scheduler's setInterval timers running.
  Torn down through locals in startWorker().

PATHS THAT STILL DIED BEFORE /health COULD ANSWER

- PORT="" (a cleared platform variable) parsed to NaN and listen(NaN) throws
  ERR_SOCKET_BAD_PORT synchronously at module scope -- reproducing the exact
  opaque failure this PR exists to remove. Extracted resolvePort() with
  validation and a logged fallback; unit-tested.
- healthServer had no 'error' listener, so EADDRINUSE/EACCES was an uncaught
  exception. It is the one failure that genuinely cannot be reported over the
  port, so it is now loud in the logs and exits deliberately.
- The /health handler was unguarded; worker.healthCheck() or JSON.stringify
  throwing would have let a probe kill the process it exists to observe.
- No unhandledRejection/uncaughtException handlers, so one stray rejection
  reverted the whole stay-alive design. Added as last-resort handlers.
- The BOOT ORDER comment claimed /health binds before anything touches the
  database. The `prisma` import constructs a PrismaClient at module scope, so
  that was only ever true of query-time failures. Comment corrected.

/health HONESTY

- 200 was gated on the snapshot EXISTING, never on running. Worker.stop() sets
  running=false without exiting (including from Worker's own signal handlers),
  and a poll blocked on a hung database call freezes lastPollTime -- both left a
  worker processing nothing while answering 200 {"status":"ok"}. Now requires
  the worker to actually be polling, with distinct stopped/stalled reasons.
- BootState made {phase:'failed', error:null} representable -- a reasonless 503,
  the exact bug being fixed -- unreachable only by assignment ordering. Now a
  discriminated union. Every non-200 carries a reason; no body says "ready" on
  a failure response.
- P2021/P2022 messages were echoed raw. Prisma prefixes them with an invocation
  preamble carrying an absolute container path and a source code frame, so the
  unauthenticated probe published internal layout. Only the backticked object
  name is lifted out now, length-bounded.

DRIFT GUARD

- migrate diff is bidirectional and emitted DROP for anything in the database
  that schema.prisma does not declare -- routinely true while a sibling service
  is mid-rollout. Combined with restartPolicyType="ALWAYS" that turned any
  normal schema-advancing deploy into a worker crash-loop. Now fatal only on
  objects the schema declares and the database lacks (the outage class), with a
  NOTE for extras.
- Added OUTPOST_ALLOW_SCHEMA_DRIFT=1 as an incident escape hatch, [worker] log
  prefixes (web and worker interleave in Railway logs), and explicit framing for
  a migrate deploy failure, which was left to bare set -e.
- Pinned the Prisma CLI to 6.19.3. It now gates whether the worker may boot, and
  `prisma@6` floated; npx resolving 7.9.1 during verification rejected the
  schema outright, which is precisely the failure mode.
- Dockerfile HEALTHCHECK hardcoded 3005 while the code prefers PORT; it now
  follows the same precedence, over 127.0.0.1 rather than localhost.
- railway.toml: added healthcheckTimeout so a known-failed boot fails fast
  instead of waiting out the 300s default.

Verified against a throwaway Postgres: all six drift-guard branches (clean,
missing object, escape hatch, extra object, unreachable, exit codes); happy path
serves 200 with an intact snapshot and shuts down cleanly in 1s; invalid PORT no
longer kills the process; a failed boot lingers then exits 1; /health?probe=1
and /health/ answer rather than 404. Tests 12 -> 24. turbo typecheck 10/10,
turbo test 10/10.

The boot-after-shutdown window could not be manufactured live -- shutdown
reached process.exit before boot resumed in both attempts -- but the outcome is
pinned: zero jobs enqueued where the unguarded path would have ticked the
scheduler.
Two CR rounds (7 agents each) converged on the same verdict: the repair
migration is correct and stable, and the boot//health rework is not ready.

Round 1 found ~15 defects. Round 2 found ~20 more -- in round 1's fixes, not in
the original code. Three of them would have caused incidents:

- STALE_POLL_MS=60s answered 503 "stalled" for a HEALTHY worker. Worker.poll()
  stamps lastPollTime then awaits the job batch, and jobTimeouts allow 300s, so
  any long job froze the timestamp past the bound. The Docker healthcheck marks
  the container dead after 90s. A working worker would have been killed mid-job.
  The same commit had explicitly sized SHUTDOWN_WATCHDOG_MS *above* the 300s
  timeout for exactly this reason.
- The drift guard's regex classifier silently passed real drift. Missing enum
  values (ALTER TYPE ... ADD VALUE) and wrong column types (ALTER COLUMN) match
  neither pattern, so the script printed "schema matches" against a drifted
  database -- the identical failure mode to `migrate deploy` reporting "no
  pending migrations", which is the bug this guard exists to compensate for.
- Number(process.env.X ?? default) collapsed to 0 on a cleared platform
  variable, silently disabling the boot-failure linger window and forcing every
  SIGTERM to exit(1) mid-drain. Written one file away from a docblock explaining
  why ?? is insufficient for exactly this.

The meta-pattern, as one reviewer put it: the module hardened its inputs in one
place and trusted them everywhere else. That is a design that needs time and
tests, not another round inside a hotfix.

So this PR is reduced to what is verified:

- The repair migration. Reviewed by every agent in both rounds; column
  definitions confirmed byte-for-byte against 0001_init and schema.prisma each
  time. This is what unblocks staging, whose worker last deployed 2026-07-24 and
  would otherwise hit the same missing table.
- The schema-drift guard, in its plain `migrate diff --exit-code` form. ANY
  difference is fatal. The classifier that tried to be cleverer about sibling
  skew is gone; OUTPOST_ALLOW_SCHEMA_DRIFT (1/true/yes) is the release valve
  instead. A guard that can be wrong in the reassuring direction is worse than
  no guard.
- Framing for a migrate deploy failure, [worker] log prefixes (web and worker
  interleave in Railway logs), and a startup signal trap so a SIGTERM during
  migration is not discarded by PID 1.
- The Prisma CLI pinned to the lockfile's 6.19.3. It now gates whether the
  worker boots, and `prisma@6` floated -- resolving that range during review
  picked up 7.9.1, which rejects this schema outright.

apps/worker/src/index.ts returns to main's behaviour: boot failures are fatal
and opaque, exactly as they are in production today. That is a known quantity.
Shipping a rework that answers 503 for healthy workers is not.

The rework and its full finding list are preserved on
wip/worker-health-boot-rework for a follow-up PR.

Verified against a throwaway Postgres: clean DB starts; a missing table, a
missing enum value, and a wrong column type each fail with exit 1 (the last two
are the classes the deleted classifier passed); OUTPOST_ALLOW_SCHEMA_DRIFT=1,
true and yes each start with warnings; an unreachable database reports a tool
failure rather than confirmed drift. turbo typecheck 10/10, turbo test 10/10,
sh -n clean.
…caught this

Round 3 (7 agents) on the reduced diff found no behavioural bugs in the shipped
logic -- the remaining findings were claim accuracy, input robustness, and one
real operational hazard.

The hazard: OUTPOST_ALLOW_SCHEMA_DRIFT only covered the drift branch. A CLI-level
failure (a schema the pinned CLI rejects, a missing engine binary, an unreadable
/opt/prisma) exited 1 with no override at all, which under
restartPolicyType="ALWAYS" is an unbreakable crash loop whose only recourse is a
code change and a rebuild. A guard added to unblock deploys must not become the
thing that blocks them, so the override now covers both fatal branches -- a check
that could not RUN is a strictly weaker guarantee than one that ran and found
drift, so accepting the latter implies accepting the former.

CI now runs the check that was missing. `prisma validate` parses the schema and
`migrate deploy` consults only the _prisma_migrations table, so a migration
recorded as applied but never effective passed both -- which is why production
carried that state for months. The job gets a real Postgres and asserts:

  - migrations reproduce schema.prisma exactly from empty (migrate diff == 0)
  - re-applying them is a no-op
  - with SystemConfig dropped and the repair un-recorded, the guard DETECTS it
  - the repair migration then fixes it
  - against a mis-shaped SystemConfig the migration FAILS rather than no-ops
  - the Dockerfile's Prisma pin matches the lockfile

The last two are new behaviour. The migration now asserts its own postcondition
with a DO $$ block: CREATE TABLE IF NOT EXISTS silently no-ops on a table with
wrong columns and gets recorded as applied, reproducing "recorded but not
effective" one level up. Deferring that to the worker's drift guard was not
enough either, since apps/web/start.sh runs a bare migrate deploy against the
same database with no guard and would close the repair window silently. Failing
records P3009 and demands a human -- deliberately louder, because a stopped
deploy is recoverable and a silently-ineffective one cost nine days.

Also from round 3:

- Both prisma invocations are now bounded (timeout, busybox applet; falls back to
  unbounded with a NOTE if absent). A black-holing endpoint previously hung with
  no log line at all -- indistinguishable from a slow migration and from a
  healthy-but-slow boot, defeating the point of putting the reason in the log.
- Every FATAL/WARNING goes to stderr. web and worker interleave in Railway logs,
  so severity has to be machine-distinguishable, not merely prefixed.
- Unrecognised override values are echoed back instead of failing closed with
  output byte-identical to the variable never being set.
- The trap comment claimed it aborts a SIGTERM during migrate deploy. Three
  agents independently reproduced that POSIX sh defers traps behind foreground
  commands, and the platform signals PID 1 only, so it does not. Narrowed to what
  it actually buys, with a note that interrupting a migration is deliberately not
  attempted.
- The Dockerfile pin's justification was impossible as written (`prisma@6` cannot
  resolve 7.9.1; the 7.9.1 sighting was a bare npx). Replaced with the durable
  reason. apps/web is pinned to 6.19.3 to match, closing the apply-with-one-CLI /
  verify-with-another loop.
- HEALTHCHECK follows the same PORT precedence as index.ts instead of hardcoding
  3005, over 127.0.0.1 not localhost, and start-period is 120s because this
  script now runs two prisma invocations before anything binds /health -- at 10s
  an orchestrator could restart mid-migration and manufacture the P3009 state.
- The migration's scope claim now cites the production-wide migrate diff (which
  covers indexes and constraints) rather than "every table and enum", and records
  that staging was not checked. Plus a note that updatedAt has no SQL DEFAULT, so
  a hand-repair INSERT must pass it explicitly.
- OUTPOST_ALLOW_SCHEMA_DRIFT documented in .env.example; start.sh is +x in git.

Verified: all nine start.sh branches (clean, drift with/without override in four
spellings, tool failure with/without override, deploy failure) exit and start as
intended, with FATAL on stderr and unrecognised values echoed; the CI sequence
run end-to-end against a throwaway Postgres including the mis-shaped-table
failure. turbo typecheck 10/10, turbo test 10/10.
Round 4 (7 agents) found no behavioural bugs in the migration or the guard's
decision logic, but three defects in the machinery around them -- two of which
would have made the guard lie in the reassuring direction, which is the failure
mode this whole change argues is worse than no guard.

BUSYBOX TIMEOUT SEMANTICS

Both timeout branches were dead code. The runtime is node:20-alpine, whose
busybox timeout reports 128+SIGTERM = 143, not GNU coreutils' 124 -- verified in
the actual image by four agents independently. So a black-holed database (the
exact case the timeout was added for) fell through to the generic branch and was
reported as "migrate deploy failed ... P3009 / P3006 / unreachable database",
none of which was the cause. Without -k, busybox also signals and then waits for
the child anyway, so a step that traps SIGTERM (the prisma CLI does) had no bound
at all. run_step now uses -k and normalises 143/137 to 124.

migrate deploy is no longer bounded. Killing it mid-migration leaves
_prisma_migrations holding finished_at = NULL -- the P3009 state this script
exists to diagnose -- and an agent reproduced the full sequence: killed deploy,
orphaned pg_advisory_lock, then P1002 on the next attempt and P3009 after that,
blocking BOTH services. The bound only makes sense on the read-only drift check,
and the signal traps already document that interrupting a migration is
deliberately not attempted. The two policies now agree.

THE MIGRATION'S ASSERTION WAS MUCH WEAKER THAN ITS COMMENT

It counted column NAMES. A SystemConfig with all three names but wrong types, or
nullable, or carrying an extra column, or missing its primary key (which Prisma's
upsert on @id key requires) passed, no-opped, and was recorded as applied -- the
"recorded but not effective" state one level up, again. It now checks types,
nullability, exact column count and the PK.

It also queried information_schema with a hardcoded 'public'. Both were wrong:
information_schema is privilege-filtered, so a table created by another role --
which is exactly how production's was created -- could read as absent and
hard-fail into P3009 over a database that was fine; and the CREATE is unqualified
so it follows search_path, meaning any ?schema= environment would inspect the
wrong schema. Now pg_catalog and current_schema().

CI NOW RUNS THE GUARD ITSELF

Nothing executed start.sh. CI re-implemented its prisma calls in bash on ubuntu,
which is precisely why an alpine-only exit-code difference sat in it unnoticed.
PRISMA is overridable via OUTPOST_PRISMA_CMD so the script runs against a stub,
and eleven cases now assert exit status and whether the worker boots: clean,
drift blocked, drift overridden in two spellings, unknown and falsey overrides
blocked, tool failure blocked and overridden, deploy failure, plus a non-numeric
and a zero timeout. Two more assert diagnostics do not leak to stdout and that an
overridden boot does not emit FATAL.

Also fixed, all from round 4:

- The pin check read only apps/worker/Dockerfile while apps/web/Dockerfile's
  comment claimed both were covered. It loops over both now, anchored on ^RUN,
  with pipefail and a guard on the parsed version.
- Both CI negative controls accepted any non-zero exit, so a broken CLI or an
  unreachable database would have passed as "the guard works". They assert exit 2
  specifically, and the mis-shaped case greps for the migration's own message.
- The idempotency step was vacuous: migrate deploy skips recorded migrations, so
  it ran zero SQL and would have passed with a body of SELECT 1/0. It now deletes
  the row first, which also covers the majority path (table present, repair
  unrecorded) that nothing tested.
- CI restores the database after the destructive assertions, which previously
  left a mis-shaped table and a failed P3009 row for every later step.
- FATAL was emitted before the override was consulted, so an overridden boot
  paged. And the success line claimed "schema verified" on both override paths,
  including the one where nothing was compared.
- OUTPOST_STARTUP_STEP_TIMEOUT is validated; 0 now means unbounded (the GNU
  reading an operator intends) rather than busybox's kill-immediately. Default
  lowered to 120s and documented in .env.example.
- Corrected the index count in the migration's audit comment (27, not 45).

Verified against a throwaway Postgres: wrong types, nullable columns, an extra
column, and a missing primary key each fail the migration; absent and correct
tables both succeed; a fresh deploy into a non-public schema succeeds. The CI
guard-test block was run verbatim -- all eleven cases pass. turbo typecheck
10/10, turbo test 10/10.
The idempotency fix added a psql call to the first verification step, but
PGPASSWORD was only set on the second — so the step failed with
'no password supplied' before reaching anything it was meant to assert.
Also pins psql to 127.0.0.1: the runner resolved localhost to ::1.
pnpm exec remaps any non-zero child status to 1
(ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL), which collapses migrate diff's
0/2/other contract into pass/fail. The previous run proved it: the guard
correctly reported real drift as exit 2 and the assertion saw 1, failing
with 'it never compared the database' when it had.

apps/worker/start.sh was never affected — it calls the CLI directly — but
the CI that verifies that guard has to invoke it the same way. Adds a
positive control asserting a deliberate exit 2 arrives as 2.
The mis-shaped-table assertion greps the deploy output to prove the failure
came from the migration's own guard rather than, say, an unreachable
database. The guard's message was reworded when it grew type/nullability/PK
checks; the pattern was not. CI then reported a correctly-firing guard as
'failed, but not via the migration's own guard'. Anchors on a stable
fragment now.
The step runs under `bash -e`, and over half the guard cases are supposed
to exit non-zero — so `out=$(...)` on the first blocked-boot case aborted
the step before it could compare anything. The two stream checks had the
same problem inverted: `... | grep -q FATAL && { exit 1; }` returns
non-zero precisely when the assertion PASSES.

Verified by extracting the block from the workflow and running it under
bash -e: all eleven cases pass, exit 0.
…onfig-table

fix(worker): repair the missing SystemConfig table, guard against schema drift, and gate it in CI
…t-core

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

Rebuilt from main rather than rebased. PR #170 carried both the customer-facing
fix and this infrastructure work; #172 took the fix and shipped, so what remains
is the half that was never reviewed. A rebase would have replayed 24 commits that
predate the #180 hotfix and, as the two-dot diff showed, would have DELETED the
SystemConfig repair migration, the schema-drift guard in start.sh, and its CI
gate. Rebuilding on current main keeps all three.

#172 shipped two layers of the rule: reply paths no longer enqueue, and a
re-answer gate reads the ticket's history. Both are check-then-act against a
snapshot, so neither survives two jobs running at once. Two ways that happens
today, neither needing a duplicate inbound event:

  - runWithTimeout in worker.ts is a Promise.race that does not cancel the
    handler. AI_RESPONSE has a 120s timeout over a deliberately slow pipeline, so
    on timeout attempt 1 keeps running while attempt 2 starts a second later,
    reads messages before attempt 1's message.create, and both post.
  - AI_RESPONSE runs at concurrency 4.

The database now arbitrates. Message gains a nullable responseKey with a unique
index on (ticketId, responseKey); exactly one PRIMARY_AI_RESPONSE row can exist
per ticket, and the row is claimed BEFORE any platform post-back, so the loser
catches a narrowly-identified P2002 and returns without posting. Nullable on
purpose: historical BOT rows stay valid and still prove a ticket was answered.

Delivery is a state machine rather than a hope. responseState is PENDING ->
DELIVERED | ESCALATED, every transition a compare-and-set via updateMany guarded
on responseKey and current state, never a blind update by id. A response starts
PENDING before any external post; successful delivery marks DELIVERED; one
needing a human stays PENDING until that escalation 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, so the one-post rule
holds without racing the original handler.

Job gains claimToken so a timed-out attempt cannot write back over the attempt
that replaced it. Stale PROCESSING jobs are reclaimed after a grace period, and
graceful shutdown shares one drain promise so signal handlers and the app cannot
each half-drain.

Also here:

  - generator.ts read only content[0], silently discarding text from multi-block
    model responses, and now concatenates all text blocks in order. It also
    throws on empty text rather than publishing nothing — unrequested scope,
    filed as #178 for an explicit decision.
  - Postmark ticket creation is idempotent, backed by a partial unique index on
    Ticket(sourceId) WHERE source = 'EMAIL'. Reply detection falls back to
    In-Reply-To and References so a reply without a plus-address stops becoming a
    second ticket with its own answer.
  - The Teams acknowledgement card no longer claims an AI is reviewing the
    question when no AI job was enqueued.

Restores the /health rework that #180 deferred (d0c6f99). #180's own start.sh
guard catches the SystemConfig cause; it does not catch the class. Any other
boot-time failure — unreachable database, bad credentials, a throw inside
buildSyncEngine unrelated to schema — still exits before the port binds, leaving
Railway nothing to report but a five-minute timeout indistinguishable from a
broken image. The port now binds first, boot state is tracked, and /health
answers 503 with the phase and reason while boot is unfinished or failed.
Fail-fast is retained: an unbooted worker is never reported healthy, because its
sync mappings come from the database and one running against a schema it does not
match would write wrong statuses to Linear. railway.toml drops healthcheckTimeout
from Railway's 300s default to 180s, above BOOT_FAILURE_LINGER_MS, so the probe
reads the reason before the process exits to be restarted.

This is the reviewed version of that work recovered from d0c6f99~1, not a
rewrite — it carries the review rounds that made the boot-failure path reachable
and shutdown survive a signal during boot. Worker tests go 2 -> 24.

Verification: turbo typecheck 10/10, turbo test 10/10 (1924 tests). Confirmed by
diff that start.sh, both Dockerfiles, .github/workflows/ci.yml, .env.example and
the 20260812 SystemConfig migration are byte-identical to main.

Three findings from the split brief are NOT fixed here and block merge:
escalationEnqueued is assigned and never read in the non-recovery path; the
In-Reply-To/References resolution trusts attacker-controlled headers, so anyone
holding a Message-ID from a thread can append to that ticket; and responseError
carries DELIVERY_CONFIRMED / ESCALATION_REQUIRED control state in a free-text
error column, in the same change that introduced an enum for exactly that.
…elta

The split brief named three findings that had to be fixed before this work could
merge. All three lived in the half of PR #170 that never had a review pass.

1. escalationEnqueued was assigned and never read on the main path.

enqueueEscalationAtomically returns false when its compare-and-set found the
response row in a state other than PENDING, meaning no escalation was queued.
That false was discarded: nothing logged, and the handler returned success with
`escalated` computed from local conditions rather than from what actually
happened. A response that reached nobody and summoned nobody reported itself
handled.

A no-op CAS now reads the row's actual responseState and splits the two cases
that were previously identical. DELIVERED means no escalation was needed, so the
job succeeds honestly with escalated: false. ESCALATED means someone else already
queued it, so escalated: true. PENDING, missing, or unreadable means the handoff
did not happen and the job fails loudly. `escalated` is now derived from the
durable handoff instead of inferred.

2. Header-based email reply resolution trusted attacker-controlled input.

findTicketByReplyMessageIds matched candidate IDs from In-Reply-To and References
against Ticket.sourceId and Message.attachments.postmarkMessageId. Those headers
are sender-supplied, and a Message-ID from a thread is KNOWN to every participant
who was ever CC'd rather than having to be guessed. Anyone holding one could
append a message to that ticket and reopen it. No AI response is spent on a reply
and nothing is echoed back, so this was integrity and noise rather than
disclosure — but materially wider than the MailboxHash path it supplements.

Header matching now additionally requires the sender to be an existing
participant: ticket.user.email, or an address parsed out of an existing
Message.author on that ticket, or anyone at ticket.account.domain.

The Message.author path is load-bearing rather than defensive — email tickets
never set userId, so the opening sender is identified only by the "Name <email>"
message label. Deriving a domain from user.email or a message author was
considered and rejected: a ticket opened from an address at a freemail provider
would make every address at that provider a participant, which reopens the hole
while looking like a fix. Account.domain is opt-in CRM data and cannot silently
widen to a public provider. normalizeParticipantEmail also keeps free-form
author values that are not addresses (System, slack:U123, Outpost AI) from
counting as identities.

An unauthorized candidate is treated as unresolved, so the mail falls through to
the orphaned-reply path and is filed as its own ticket with no AI job. A
legitimate sender writing from an unrecognised address never loses their message.
MailboxHash is deliberately NOT gated — it is a plus-addressed token we generate,
and gating it breaks the legitimate reply path.

Two supporting changes the fix required: findFirst became findMany so candidates
are scanned oldest-first for the first AUTHORIZED match, since otherwise a chain
naming somebody else's older ticket would refuse the sender's own legitimate
reply; and MAX_REPLY_MESSAGE_IDS caps the sender-supplied IN (...) fan-out that
requiring participants necessarily widens.

3. responseError carried lifecycle state in a free-text error column.

DELIVERY_CONFIRMED: and ESCALATION_REQUIRED: string prefixes encoded control
state in the column an operator reads to find out what went wrong — so any ops
surface would render DELIVERY_CONFIRMED as an error. In the same change that
introduced MessageResponseState for exactly this purpose.

Both moved to dedicated additive columns: deliveryConfirmed Boolean
@default(false) and escalationRequiredReason String?. Columns rather than new
enum values, deliberately: both markers describe a response that is still
PENDING, and MessageResponseState records the OUTCOME, so adding them would have
destroyed the outcome they are sub-states of and silently broken every
responseState === 'PENDING' comparison in the handler. The escalation signal is
one nullable text column rather than a boolean plus a reason, so the fact and its
payload cannot drift apart. responseError now holds only real error text.

Migration 20260813000000_add_message_response_substates is two ADD COLUMNs, no
index, and needs no backfill: DEFAULT false makes existing rows correct by
construction, and a row that somehow did hold a marker reads as "not confirmed",
which is the safe direction. Verified against the merged CI drift gate by having
Prisma 6.19.3 emit the required SQL via migrate diff and comparing it to the
hand-written file; they are identical. No Postgres in the sandbox, so CI's own
migrate deploy round-trip was not executed locally.

One deliberate behaviour change falls out of (3): independent columns can both be
set, which a single string prefix made impossible. The re-answer gate therefore
checks the owed escalation BEFORE the confirmed-delivery repair — dropping a
promised human handoff is the worse failure. Neither branch reposts.

Verification: turbo typecheck 10/10, turbo test 10/10 (1949 tests, up from 1924).
Every fix carries red-green verification; 16 separate mutations were each
observed RED and restored to GREEN, including one confirming that gating the
MailboxHash path turns 10 tests red, which pins that boundary. Existing
assertions that referenced the old markers were updated and each re-checked for
degenerating to always-pass.

Confirmed by diff that start.sh, both Dockerfiles, .github/workflows/ci.yml,
.env.example and the 20260812 SystemConfig migration remain byte-identical to
main.
@NathanTarbert

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #187, which carries the same head branch (fix/ai-response-delivery-durability) against main.

These were not duplicates — the difference is the base branch. #187 targets main and is 2 commits, +4073/-352: the actual delivery-durability change. This PR targets staging and is 100 commits, +13307/-1102, because origin/staging is currently 100 commits behind main with 0 ahead, so the base-branch delta got swept into the diff and made the real change unreviewable.

The staging drift is tracked separately as #194staging needs a fast-forward from main regardless of this PR, since Railway deploys staging from that branch and is therefore validating code from before the groundedness gate (#143), the one-answer-per-ticket arbitration (#172), and the schema-drift guard (#180).

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