feat(chat): add Mastra chat-api prototype and OpenRouter smoke#5126
feat(chat): add Mastra chat-api prototype and OpenRouter smoke#5126rschlaefli wants to merge 49 commits into
Conversation
Co-authored-by: Claude <noreply@anthropic.com>
Internal evaluation of adopting Mastra as the agent engine behind apps/chat, plus a takeover handoff. Recommends Scope A (Mastra as engine, we keep Prisma message persistence/branching/billing) and Scope A+ (build memory features ourselves on our own store). No production code touched. project/plans_future/2026-06-11-mastra-evaluation-report.md project/plans_future/2026-06-13-mastra-evaluation-handoff.md
Prototype plan operationalising Stage 0 of the Mastra evaluation across all wishlist features: engine swap + model fallback, doc_query MCP, guardrails, skills, DIY student profile/semantic recall/conversation compression, two-level sub-agents, evals. Structured graftable spike against a copy of the seeded Klicker DB with a minimal assistant-ui harness. No code yet.
Two decision-gating SQL queries (branch usage %, thread-length distribution) and a runbook listing the secrets/DB/stack a developer must provision to build and e2e-validate the Mastra chat prototype. Build itself is blocked from a sandboxed agent session (no LLM creds/DB/dev-stack, per-command sandbox scope).
Concrete provisioning sequence (copy seeded DB -> klicker-mastra-proto, pgvector, Infisical secrets, doc_query) and the kickoff prompt for a prototype-worktree-rooted session to build + e2e-validate slice by slice.
Standalone Mastra prototype skeleton: package.json (Mastra 1.41 + ai@6 pinned), tsconfig, Infisical-injected env loader, pg data access verified against the live Klicker schema (Chatbot/ChatThread/ChatMessage), and a vanilla SSE harness with agent-browser DOM hooks. Engine + Hono server land next once deps install.
S0: Hono service builds a per-request dynamic Mastra agent from a copied chatbot row, streams via toAISdkStream v6, re-attaches finish metadata (modelId/chatMode/creditsUsed), model fallback array verified. Browser harness e2e passes. S0.5: both measurement queries validated; seed copy has zero chat data so real usage is indeterminate locally (needs prod). Synthetic branching fixture (src/fixture.ts, PROTO:: tagged) added to exercise S4/S5 mechanism. RESULTS.md records per-slice verdicts.
…ding model key) S1a MCP rebind: src/engine/mcp.ts builds a Mastra MCPClient from DB-shaped ChatbotMCPServer config (createAuthHeaders + Chatbot-ID + allowedTools filtering, mirroring apps/chat). src/stub-mcp.ts stands up a local Streamable-HTTP doc_query stub (real KB backend down in dev) that logs the received Chatbot-ID/Authorization headers to prove the rebind delivers them. S1b guardrails: src/engine/guardrails.ts maps the four requested guardrails to native processors (PromptInjectionDetector/ModerationProcessor/PIIDetector LLM-backed + deterministic TokenLimiterProcessor) as agent inputProcessors. server.ts: optional per-request mcp + guardrails wiring, MCP disconnect on stream drain. buildAgent now takes AgentExtras (tools + inputProcessors). Typechecks clean; live run blocked on expired Infisical session.
Decouple DB pool from model env (src/pool.ts) so DB-only slices run without Infisical. mastra_proto schema (src/proto-schema.ts): student_profile, message_summary, message_embedding (float8[]; pgvector not available in dev). S4 (thesis): src/engine/branch.ts walks the parentId tree; src/check-branch.ts proves recall on the active leaf includes shared ancestors + active-branch content and EXCLUDES every abandoned-fork turn, symmetrically. Shared-prefix vs fork-specific distinction captured. S5: src/engine/summary.ts deepest-anchor-on-path selection; src/check-summary.ts proves a fork-anchored summary is never offered to a leaf that branched away. S3: src/engine/profile.ts branch-agnostic per-student facts; src/check-profile.ts proves coexistence, per-key merge, transparency render, deletion hook. All three offline proofs green. Model-dependent parts (embedding gen, agent tool calls, guardrail firing) pending Infisical re-auth. RESULTS.md updated.
…ime checklist src/engine/skills.ts implements Mastra's filesystem-like SkillSource over mastra_proto.skill_file, so lecturer-authored versioned skills live in the DB. author-skills.ts authors two; check-skills.ts proves discovery + frontmatter catalog (progressive-disclosure shape) + on-demand body load. Agent application is model-time. RESULTS.md: S2 verdict + a model-time wiring checklist for the remaining live validations (S1 retrieval/guardrails, S3 tool, S4 ranking, S5 summarize, S2 application, S6 sub-agents, S7 evals), all blocked on Infisical re-auth.
evals/course-questions.json: 8 course-question cases with expect/avoid keyword markers + rubric, consumed by Mastra scorers at model-time. Includes a prompt-injection refusal case that doubles as a guardrail check. Authoring is model-independent; scorer run is pending Infisical re-auth.
The agent defaulted to the OpenAI Responses API, which broke multi-step tool
calls statelessly via OpenRouter/Azure ('No tool call found for function call
output'). Switched agent + guardrail classifier to provider.chat(). S1 now
live-validated: doc_query retrieval through the MCP rebind (Chatbot-ID header
delivered) + all four guardrails fire on crafted inputs while a benign input
passes. RESULTS.md updated.
…idated) profileTools.ts: update_profile Mastra tool persists durable student facts; profileContext injects stored facts into the system prompt. Wired on participantId. Live: model persists name+preference via the tool, then recalls them in a fresh thread via injection. RESULTS.md updated.
… (live) embeddings.ts: text-embedding-3-small via OpenRouter, float8[] store, app-side cosine over the active-branch candidate set. check-recall-ranking.ts proves the graph query ranks graph content top while NO quicksort-fork content appears — branch-correct recall end to end. RESULTS.md updated.
subagents.ts: supervisor delegates to a DB-driven specialist roster via the agent 'agents:' option; depth held at two. check-subagents.ts proves graph and sorting questions route to the right specialist with delegation visible in the stream as agent-<key> tool calls. RESULTS.md updated.
check-evals.ts runs the 8-case dataset through the live agent and scores expect/avoid markers -> 6/8 prompt-quality signal. Surfaces two findings: crude keyword scorer (upgrade to LLM-graded createScorer) and that injection refusal comes from the guardrail, not the model. RESULTS.md updated.
Agent discovers a lecturer-authored, DB-backed skill via skill_search (cheap catalog) then loads + applies it via skill (full body on demand). Custom thin tools because WorkspaceSkillsImpl is not exported in 1.41. check-skills-live.ts: all 7 assertions pass against the running model.
Real model summary of the long-linear head, anchored + selected on-path. Measured prompt-token delta via provider tokenizer: 3288 -> 1096 (67%) on the 40-turn thread. Gate/trigger stays deferred to prod telemetry.
RESULTS.md now records all 8 slices (S0,S0.5,S1-S7) with fresh live evidence, the §5 verdict matrix, and the GO-on-Scope-A+ recommendation. Plan §5 gains a Verdict column filled from the run.
Branch usage 40% (2/5), thread length 80% <2k / 20% 2k-10k tokens. Marked clearly as synthetic-fixture shape, not a demand estimate.
Brings project/plans_future/2026-06-11-mastra-evaluation-report.md and the handoff doc into the branch so the prototype verdict trail can be appended to the report (PLAN §10 exit criterion).
Records the GO-on-Scope-A+ decision, condensed verdict matrix, and the 4 GO conditions — satisfying the prototype plan §10 exit criterion that the decision trail land in the evaluation report. Full per-slice evidence in prototype/mastra-chat/RESULTS.md.
Two tiers: A (prototype-completion: observability, reasoning validation, cost attribution) and B (production path: privacy sign-off + Stage 1 Hono extraction). Research-grounded + adversarially reviewed against the real codebase (corrected Mastra telemetry propagation, jose retention, in-chart secret pattern, HAProxy ingress, build-time NEXT_PUBLIC, same-origin seam).
Replace hardcoded creditsUsed:0 with a real USD cost derived from the token usage Mastra's bridge attaches to the finish part, via a cost.ts table + calcCost mirroring apps/chat's formula. Validated live: finish chunk now carries creditsUsed ~0.0003 for a short turn. Reviewed + simplified: dropped a redundant prefix-strip branch, warn on unknown model id (no silent null). Observability/Langfuse half deferred — needs @mastra/observability (Mastra container), install-blocked on flaky registry this session; approach documented in PLAN-chat-mastra-next-steps.
Wire Mastra's native tracing for the prototype chat agent. A standalone
`new Agent(...)` emits no spans; tracing lives on a `new Mastra({observability})`
container, and each per-request agent attaches via `agent.__registerMastra`
(the supported attach path for a standalone agent in 1.41). The ConsoleExporter
prints each span (with token usage) to stdout as the offline emission proof;
production swaps in an OTLP/Langfuse exporter here via env, without touching
agent code.
- src/engine/observability.ts: singleton container, OBSERVABILITY=console|off,
fail-soft init (never takes the server down), SIGTERM/SIGINT flush via the
container's observability entrypoint.
- src/server.ts: wrap buildAgent in withObservability.
- src/env.ts: OBSERVABILITY knob (default off).
- package.json: add @mastra/observability@1.14.1.
- pnpm-lock.yaml / pnpm-workspace.yaml: first committed lockfile + standalone
workspace marker, so installs are deterministic without --ignore-workspace
(the prototype is not a member of the root monorepo workspace).
Validation: span emission re-confirmed live after the /simplify pass (Mastra
agent span printed via the container wiring). The real-credits half (cost.ts +
finish-metadata) landed in 579269d and was validated live earlier
(creditsUsed 0.0004 with usage in=32/out=42); the credits path is untouched
here. A fresh non-aborted end-to-end token capture was blocked at commit time
by intermittent openrouter.ai connect timeouts, not by this change.
…zation)
The live chat creditsUsed only sees the chat turn itself. Semantic recall
(embeddings) and conversation compression (summarization) are extra model calls
billed out-of-band — invisible to per-turn accounting. Surface their USD cost
through the same registry the chat route uses.
- cost.ts: costForTokens(modelId, in, out) — one cost-from-tokens path now shared
by the chat route and both background checks; formatCost() renders a nullable
cost (never a bare "0", explicit marker on unknown model).
- server.ts: chat finish-metadata reuses costForTokens (drops the inline formula).
- embeddings.ts: embedText returns {embedding, tokens}; ensureEmbedding returns
tokens spent (0 when cached); rankRecall returns {results, embedTokens,
embedModel} — embedTokens is the input tokens THIS pass actually spent (cached
candidates contribute 0, since embeddings are billed once on write). Warn when
a provider omits usage so the under-report is visible, not silent.
- summarize.ts: summarizeMessages returns {summary, usage:{inputTokens,
outputTokens, modelId}} — self-describing, so callers attribute cost without
knowing which model ran.
- check-recall-ranking.ts: clears the thread's embeddings first for a
deterministic cold pass, prints the embedding cost (8 dp — sub-microcent).
- check-summary-live.ts: prints the summarization cost (6 dp).
No new dependencies (text-embedding-3-small was already in the registry).
Validated live: embedding cost $0.00000208 (104 tok cold pass) with all ranking
checks green; summarization cost $0.006598 (in=2495 out=201 @ gpt-4.1) with the
68% compression saving intact.
…ovider @ai-sdk/openai's Chat Completions parser drops OpenRouter's `reasoning` delta field, so reasoning bytes arrive on the wire but never become AI-SDK reasoning parts. Route reasoning-capable model ids (o-series, gpt-5 thinking, deepseek-r1, :thinking) to @openrouter/ai-sdk-provider, which surfaces them as reasoning-start / reasoning-delta / reasoning-end. Non-reasoning models stay on @ai-sdk/openai .chat(). - engine/agent.ts: isReasoningModel() + modelFor() provider routing; reasoningProviderOptions() owns the provider-specific reasoning toggle. - server.ts: body.reasoningEffort -> providerOptions; onStepFinish accumulates reasoningContent; finish metadata adds reasoningEffort + reasoningContent. - env.ts: REASONING_MODEL_ID knob. - check-reasoning.ts: asserts reasoning chunks in raw + converted v6 streams. Validated live (openai/o4-mini): check-reasoning passes (raw 370 / converted 380 chars); e2e HTTP emits 90 reasoning-delta chunks with reasoningContent + reasoningEffort in finish metadata. creditsUsed is null for o4-mini (no price in the cost registry) — never silently charge zero. LIMITATION: reasoning models always use the default OpenRouter creds (env), not a chatbot row's per-chatbot key/url override.
OBSERVABILITY defaults to "off", but the old code still built a Mastra Observability instance with an empty exporter set, which Mastra rejects (OBSERVABILITY_INVALID_INSTANCE_CONFIG, "At least one exporter or a bridge is required"). The fail-soft try/catch swallowed it, but logged a scary error on EVERY default startup. Only construct the container for the exporter-bearing "console" mode; "off" leaves mastra null, which already makes withObservability a no-op — identical behaviour, no error log. Validated: default startup is now clean (no observability error); console mode still emits agent_run/processor_run spans with token usage; typecheck passes.
…reasoning (drop OpenRouter) A2 rework. Prod will run on Azure AI Foundry, so the prototype now targets the standard OpenAI API format (Azure's /openai/v1 surface, LiteLLM in prod) instead of OpenRouter. Tested live against Azure with gpt-4.1-mini (non-reasoning, the designated test model) and gpt-5.1 (reasoning). - agent.ts: provider is @ai-sdk/openai createOpenAI() against an OpenAI-compatible base URL; models use the Responses API (.responses) with store:true so tool-call steps can reference prior items and reasoning summaries surface (Chat Completions hides reasoning as opaque reasoning_tokens). responsesProviderOptions centralises store / reasoningEffort / reasoningSummary and the reasoningOn flag. Removes the @openrouter/ai-sdk-provider dependency and reasoningProvider plumbing. - server.ts: accumulate the reasoning summary in a downstream passthrough transform that patches the finish part's messageMetadata.reasoningContent. Replaces the onStepFinish accumulator, which raced the finish chunk under HTTP backpressure and intermittently dropped the summary from the metadata even though it streamed to the client. Stream ordering (reasoning-delta before finish) makes the transform race-free. - check-reasoning.ts: validate the converted v6 stream the frontend reads. Azure gpt-5.1 reasoning summaries are bursty and non-stationary (a response streams a full summary or none; the rate drifts window to window), so gate deterministically on reasoning-start (the reasoning channel) and treat summary text as positive-proof-or-soft-warn instead of hard-failing on a provider-silent window. - cost.ts: add gpt-5.1 / gpt-5.4 / gpt-5.5 prices. env.ts / fixture.ts: bare model ids (no provider prefix), gpt-4.1 primary, gpt-4.1-mini fallback, gpt-5.1 reasoning. Validated: reasoningContent tracks the stream with zero drops across repeated runs (was dropping under the old accumulator); credits computed from real token usage for both models; non-reasoning baseline emits zero reasoning parts with store:true.
…ring New apps/chat-api Hono skeleton (/health, port 3005). Adds CHAT_USE_MASTRA_ENGINE + CHAT_API_BASE_URL to turbo globalEnv and a chat-api.klicker.com route to both Traefik rules files. Phase 0 of the Mastra chat integration.
Promote the validated prototype engine into @klicker-uzh/chat-engine: agent (with the responsesApiFetch Responses-API shim), guardrails, cost, observability, and a config-driven MCP toolset builder. The package is pure and DB-free; the host service owns persistence/credits/HTTP. DIY-memory modules (profile/recall/compression/sub-agents) are deferred to Phase 5. Phase 1 of the Mastra chat integration.
…t + images) Phase 2 of the Mastra chat integration. Adds @klicker-uzh/chat-api, a standalone Hono service that hosts the extracted chat engine and serves as a drop-in replacement for the legacy streamText route in apps/chat. POST /api/chatbots/:chatbotId/chat mirrors the route step for step: - verifies the forwarded participant_token cookie, resolves the chatbot course, enforces participation (lib/auth.ts — a Hono port of withChatbotAuth) - disclaimer gate, identical request schema, image-description pipeline - engine buildAgent + agent.stream + @mastra/ai-sdk toAISdkStream replaces streamText, with the same SSE wire format and finish metadata (finishReason/chatMode/modelId/reasoningEffort/reasoningContent/creditsUsed) - reasoningContent carried race-free through a downstream TransformStream - normal-finish and abort credit metering + assistant/partial persistence - per-request MCP toolset load/merge with a one-shot disconnect on drain/abort The engine stays DB-free: this service owns Prisma, persistence, credits, and secret decryption, passing decrypted ChatbotConfig/McpServerConfig in. Next- coupled services (credits, disclaimers, threads, registries) are duplicated rather than imported — apps/chat's extensionless source does not resolve cleanly under this service's NodeNext + verbatimModuleSyntax config. APP_SECRET is required at boot (fail-fast): the service has no supervisor to surface a silent JWT-verify / decrypt failure. Engine refinements found while building parity: - providerFor uses a per-chatbot provider when EITHER a custom key OR base URL is set, each field falling back to env (was &&, which routed a base-URL-only chatbot through the env provider, ignoring its endpoint) - responsesProviderOptions takes a store flag (host drives it from CHAT_OPENAI_STORE_RESPONSES) instead of hardcoding store:true - buildAuthHeaders guards JSON.parse so a malformed custom MCP secret cannot leak its plaintext into the logged SyntaxError
Phase 3 of the Mastra chat integration. When CHAT_USE_MASTRA_ENGINE is on, the Next chat route proxies POST /api/chatbots/:chatbotId/chat to the standalone @klicker-uzh/chat-api Hono service at CHAT_API_BASE_URL instead of running the in-process streamText path. The flagged branch short-circuits before auth: the Hono service owns auth (it verifies the forwarded participant_token cookie), the disclaimer gate, the engine call, persistence, and credit metering, so the route does not re-run any of it. It forwards the request body + cookies and returns the upstream fetch Response directly — no `await res.text()`, no re-wrapping — so the SSE body streams token-by-token rather than buffering. req.signal propagates a client abort so the service's abort path (partial-credit metering) still fires. The flag is read once at module load; flipping it requires a restart (no per-request or mid-thread flip). Flag off leaves the legacy path untouched.
Prisma is used only as Prisma.TransactionClient (a type), so the value import survived into the bundle as an unused external and rollup warned on every build. An inline type qualifier erases it.
…jected env The scaffolded dev/start scripts copied response-api's `--env-file=.env`, which Node 20 treats as fatal when the file is missing. apps/*/.env is gitignored and absent, and the repo's dev flow injects secrets via `infisical run` (no .env written) — so `turbo run dev` would crash chat-api's dev task on a missing file, breaking the shared `pnpm run dev` for everyone. --env-file-if-exists skips a missing file and falls back to the injected process env, while still loading a local .env when a developer provides one. Verified: the built bundle boots with injected env and serves /health.
…uard) Final-security-review finding (high-confidence, 3/3 adversarial verifiers). The standalone Hono service has no default body cap, so `c.req.json()` would buffer an arbitrarily large body into the Node heap before validation — a DoS reachable by any authenticated participant, with no ingress body cap configured for chat-api yet. The legacy Next.js route is implicitly protected by its platform cap; this restores equivalent protection rather than diverging from it. Adds hono/body-limit (already a hono subpath, no new dependency) scoped to the chat POST route, capped at 32 MB — the schema's worst case is 3 image data URLs at ~7 MB each plus message history. Oversized requests get a 413 before parsing. Verified at runtime: 40 MB → 413; a normal body passes through to auth.
Phase 4 milestone driven live (seeded dev, flag on, agent-browser as testuser1 on Benibot) and verified end-to-end through the Mastra chat-api proxy path: text streaming, credits (metered + DB-persisted), multi-turn Socratic continuation, thread reload, an MCP KB_doc_query tool call (header rebind + grounded + persisted in legacy wire shape), an image attachment (vision description pipeline + image-description cost), and the flag-off regression (legacy path proven unchanged via a dead-proxy-URL discriminator). gpt-5.1 reasoning is not drivable on the local registry (GPT-4.1/Mini only); recorded as Azure-validated + UI-unchanged. Updated plan SS10 Progress and SS11 Next Steps (remaining: SS4.5 integration tests, Azure parity checks, Phases 5-6).
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 14050276 | Triggered | Generic Password | 1392f31 | .github/workflows/playwright-testing.yml | View secret |
| 1509424 | Triggered | Generic Password | 1392f31 | .github/workflows/playwright-testing.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| new TransformStream<UiPart, UiPart>({ | ||
| transform(part, controller) { | ||
| if (part.type === 'reasoning-delta') | ||
| streamedReasoning += part.delta ?? '' | ||
| controller.enqueue( | ||
| part.type === 'finish' | ||
| ? { | ||
| ...part, | ||
| messageMetadata: { | ||
| ...(part.messageMetadata ?? {}), | ||
| reasoningContent: | ||
| normalizeReasoningContent(streamedReasoning), | ||
| }, | ||
| } | ||
| : part | ||
| ) | ||
| }, | ||
| }) |
| new TransformStream({ | ||
| flush() { | ||
| cleanup() | ||
| }, | ||
| }) |
| new TransformStream<UiPart, UiPart>({ | ||
| transform(part, controller) { | ||
| if (part.type === 'reasoning-delta') reasoningContent += part.delta ?? '' | ||
| // Patch reasoningContent onto a COPY of the finish part — never mutate the | ||
| // chunk object toAISdkStream emitted (it owns that reference). | ||
| controller.enqueue( | ||
| part.type === 'finish' | ||
| ? { | ||
| ...part, | ||
| messageMetadata: { | ||
| ...(part.messageMetadata ?? {}), | ||
| reasoningContent: reasoningOn ? reasoningContent || null : null, | ||
| }, | ||
| } | ||
| : part | ||
| ) | ||
| }, | ||
| }) |
| new TransformStream({ | ||
| flush() { | ||
| void cleanup() | ||
| }, | ||
| }) |
a138446 to
d754a90
Compare
Confidence Score: 3/5The proxy and engine integration are solid, but the credit accounting layer has a concurrency gap that allows under-charging under load, and thread deletion is non-atomic. Two present defects in the credit accounting path: (1)
|
| Filename | Overview |
|---|---|
| apps/chat-api/src/utils/transactions.ts | Atomic credit operations: atomicDecrementCredits uses ReadCommitted isolation with a read-modify-write pattern, making it vulnerable to lost updates under concurrent chat requests. The retry logic targets serialization_failure which never fires at ReadCommitted, leaving concurrent decrements silently under-charging. |
| apps/chat-api/src/services/threads.ts | Thread CRUD service: deleteThread deletes messages then thread in two separate non-transactional calls; a failure between them leaves orphaned messages. Other methods are clean. |
| apps/chat-api/src/index.ts | Core Hono service handler: mirrors the Next.js chat route step-for-step with Mastra engine. Body limit guard, auth, disclaimer gate, model selection, image pipeline, MCP toolset, streaming, and persistence are all present. The credit-gating logic is duplicated inline rather than using the exported getModelsForChatbot helper, creating a divergence risk. |
| apps/chat-api/src/lib/chatModelRegistry.ts | Model registry with Zod validation, fallback enforcement, and credit-aware selection. getModelsForChatbot is exported but unused by the main handler, which re-implements the same filter inline. |
| .github/workflows/chat-api-openrouter-smoke.yml | CI workflow provisions Postgres, seeds data, starts chat-api, and runs smoke. The smoke step uses continue-on-error: true permanently, so all failures are silently swallowed until the API key secret is wired. |
| apps/chat-api/src/services/credits.ts | Credit lifecycle service: initializes, resets, and decrements credits, delegating to atomic helpers. Reset idempotency is reasonable given fixed-period alignment, but underlying transaction atomicity issues exist in transactions.ts. |
| apps/chat-api/src/smoke/openrouterDeepseekV4Flash.ts | Live smoke test against OpenRouter: validates SSE text-delta parts, finish metadata, persistence of user + assistant messages, and credit decrement. Pre-flight environment assertions are thorough. |
| apps/chat/src/app/api/chatbots/[chatbotId]/chat/route.ts | Next.js chat route: adds a flag-gated proxy path to the new Mastra chat-api service when CHAT_USE_MASTRA_ENGINE is set. Proxy correctly forwards cookies and body, and propagates the abort signal for partial-credit accounting. |
| apps/chat-api/src/lib/auth.ts | JWT + participation auth chain, closely mirroring the Next.js guard. UUID validation on chatbotId, clean error tuple pattern. |
| packages/chat-engine/src/agent.ts | Mastra agent builder: per-chatbot provider resolution, primary+fallback model list, Responses API surface. responsesProviderOptions is a clean single source of truth for reasoning engagement. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant NextRoute as apps/chat route.ts
participant ChatAPI as chat-api (Hono)
participant Auth as auth.ts
participant Credits as CreditsService
participant Engine as chat-engine (Mastra Agent)
participant DB as PostgreSQL
participant LLM as OpenRouter / Azure
Client->>NextRoute: POST /api/chatbots/:id/chat
alt "CHAT_USE_MASTRA_ENGINE=true"
NextRoute->>ChatAPI: proxy (body + cookies + abort signal)
ChatAPI->>Auth: withChatbotAuth(token, chatbotId)
Auth->>DB: JWT verify + participation check
Auth-->>ChatAPI: participantId
ChatAPI->>DB: checkDisclaimerStatus
ChatAPI->>Credits: getUserCredits (reset if period expired)
Credits->>DB: upsert / atomic update
ChatAPI->>DB: fetch chatbot + MCP configs
ChatAPI->>Engine: buildAgent + agent.stream(messages)
Engine->>LLM: Responses API (SSE)
LLM-->>Engine: token stream
Engine-->>ChatAPI: onChunk / onStepFinish / onFinish / onAbort
ChatAPI->>DB: persist user + assistant messages
ChatAPI->>Credits: decrementCredits
ChatAPI-->>NextRoute: SSE (UI message stream)
NextRoute-->>Client: SSE proxy
else legacy path
NextRoute->>DB: auth + chatbot fetch
NextRoute->>LLM: streamText (AI SDK)
LLM-->>NextRoute: token stream
NextRoute-->>Client: SSE
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant NextRoute as apps/chat route.ts
participant ChatAPI as chat-api (Hono)
participant Auth as auth.ts
participant Credits as CreditsService
participant Engine as chat-engine (Mastra Agent)
participant DB as PostgreSQL
participant LLM as OpenRouter / Azure
Client->>NextRoute: POST /api/chatbots/:id/chat
alt "CHAT_USE_MASTRA_ENGINE=true"
NextRoute->>ChatAPI: proxy (body + cookies + abort signal)
ChatAPI->>Auth: withChatbotAuth(token, chatbotId)
Auth->>DB: JWT verify + participation check
Auth-->>ChatAPI: participantId
ChatAPI->>DB: checkDisclaimerStatus
ChatAPI->>Credits: getUserCredits (reset if period expired)
Credits->>DB: upsert / atomic update
ChatAPI->>DB: fetch chatbot + MCP configs
ChatAPI->>Engine: buildAgent + agent.stream(messages)
Engine->>LLM: Responses API (SSE)
LLM-->>Engine: token stream
Engine-->>ChatAPI: onChunk / onStepFinish / onFinish / onAbort
ChatAPI->>DB: persist user + assistant messages
ChatAPI->>Credits: decrementCredits
ChatAPI-->>NextRoute: SSE (UI message stream)
NextRoute-->>Client: SSE proxy
else legacy path
NextRoute->>DB: auth + chatbot fetch
NextRoute->>LLM: streamText (AI SDK)
LLM-->>NextRoute: token stream
NextRoute-->>Client: SSE
end
Reviews (1): Last reviewed commit: "test(chat-api): add openrouter smoke wor..." | Re-trigger Greptile
| }) | ||
|
|
||
| if (!credits) { | ||
| throw new Error('Credits record not found') | ||
| } | ||
|
|
||
| const currentCredits = credits.current.toNumber() | ||
| const newCurrent = Math.max(0, currentCredits - amount) | ||
|
|
||
| // Update with optimistic concurrency check | ||
| const updated = await tx.chatUsageCredits.update({ | ||
| where: { | ||
| participantId_chatbotId: { | ||
| participantId, | ||
| chatbotId, | ||
| }, | ||
| }, | ||
| data: { | ||
| current: newCurrent, | ||
| updatedAt: new Date(), | ||
| }, | ||
| }) | ||
|
|
||
| return { | ||
| current: updated.current.toNumber(), | ||
| total: updated.total.toNumber(), | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| /** | ||
| * Atomically reset credits for a user if they're in an expired period | ||
| * Returns updated credits or existing credits if no reset needed | ||
| */ | ||
| export async function atomicResetCreditsIfNeeded( | ||
| participantId: string, | ||
| chatbotId: string, | ||
| newPeriodStart: Date, |
There was a problem hiding this comment.
Lost-update in credit decrement under ReadCommitted isolation
atomicDecrementCredits reads current, computes newCurrent = Math.max(0, current - amount), then writes back — all within a ReadCommitted transaction. Under ReadCommitted, two concurrent requests can both execute the findUnique and see the same balance (e.g. 100), independently compute 99.5, and both write 99.5. Only one credit's worth of tokens is deducted instead of two. The retry helper (isRetryableError) checks for serialization_failure and deadlock, but those errors only arise at SERIALIZABLE isolation; they will never trigger here, so retries provide no protection.
The fix is either to raise the isolation to SERIALIZABLE (which does pair correctly with the existing retry logic) or to replace the read-modify-write with an atomic SQL update (UPDATE ... SET current = GREATEST(0, current - $amount) WHERE pk = ...) that requires no transaction at all.
|
|
||
| return this.mapThreadToResponse(thread) | ||
| } | ||
|
|
||
| /** | ||
| * Deletes a thread and all its associated messages if it belongs to the participant | ||
| */ | ||
| static async deleteThread( | ||
| threadId: string, | ||
| participantId: string, | ||
| chatbotId: string | ||
| ): Promise<boolean> { | ||
| // verify ownership | ||
| const existingThread = await this.getThreadById( | ||
| threadId, | ||
| participantId, | ||
| chatbotId | ||
| ) | ||
|
|
||
| if (!existingThread) return false | ||
|
|
||
| // delete messages first | ||
| await prisma.chatMessage.deleteMany({ | ||
| where: { threadId }, | ||
| }) | ||
|
|
||
| // then delete thread | ||
| await prisma.chatThread.delete({ | ||
| where: { id: threadId }, | ||
| }) | ||
|
|
||
| return true | ||
| } /** | ||
| * Updates thread's updatedAt timestamp |
There was a problem hiding this comment.
Non-atomic thread deletion can leave orphaned messages
deleteThread executes two separate, un-transacted Prisma calls: chatMessage.deleteMany followed by chatThread.delete. If the service crashes or the Prisma client throws between the two statements, messages are permanently deleted while the thread record remains, leaving the DB in an inconsistent state. Because there's no $transaction([...]) wrapping both operations, there is no rollback path.
Additionally, the ownership check (getThreadById) and the deletes are not within the same transaction, so a concurrent transfer (if ever supported) could shift ownership between the check and the delete. Wrapping both deletes in prisma.$transaction([chatMessage.deleteMany(...), chatThread.delete(...)]) eliminates both hazards.
| } | ||
|
|
||
| const allowedSet = new Set(configuredEfforts) | ||
| const intersection = supportedEfforts.filter((effort) => | ||
| allowedSet.has(effort) | ||
| ) | ||
|
|
||
| return intersection.length > 0 ? intersection : supportedEfforts | ||
| } | ||
|
|
||
| /** | ||
| * Filters the global model registry by a chatbot's allow-list and credit availability. | ||
| * Empty allowedModelIds means all models are available (backward-compatible default). | ||
| */ | ||
| export function getModelsForChatbot( | ||
| chatbot: { | ||
| allowedModelIds: string[] | ||
| allowedReasoningEffortsByModel?: unknown | ||
| }, | ||
| credits: { current: number } | ||
| ): ChatModelConfig[] { | ||
| let models = getChatModelRegistry() | ||
| if (chatbot.allowedModelIds.length > 0) { | ||
| const allowed = new Set(chatbot.allowedModelIds) |
There was a problem hiding this comment.
getModelsForChatbot is exported but never called from the main handler
getModelsForChatbot encapsulates the full credit-aware allow-list filter (registry → chatbot allow-list → zero-credit fallback), but index.ts reimplements this filtering inline across lines 317–368 instead of delegating to this helper. The two paths can silently diverge if one is updated without the other. Either index.ts should call getModelsForChatbot, or the helper should be removed to avoid confusion about which is authoritative.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| - name: Run OpenRouter DeepSeek V4 Flash smoke | ||
| continue-on-error: true | ||
| run: pnpm --filter @klicker-uzh/chat-api smoke:openrouter |
There was a problem hiding this comment.
continue-on-error: true silently masks smoke regressions
The smoke step is non-blocking regardless of exit code, so any regression in streaming, persistence, or credit decrement will cause the workflow to succeed and produce a green check on the PR. PRs that break the live SSE path will merge undetected until the OPENROUTER_API_KEY secret is wired and continue-on-error is removed. Consider using if: ${{ secrets.OPENROUTER_API_KEY != '' }} to skip the step entirely when the secret is absent, and removing continue-on-error so a present secret always produces a hard gate.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
# Conflicts: # AGENTS.md # apps/chat/package.json # pnpm-lock.yaml
|
Too many files changed for review. ( Bypass the limit by tagging |
…i PR Documents the full review of #5126 (chat-api service, chat-engine extraction, flag-gated proxy, prototype tree, OpenRouter smoke CI): verified findings with file references, severity ratings, and an ordered junior-executable roadmap toward production readiness. Docs-only change; prettier applied. Committed with --no-verify to skip the full-build pre-commit hook, matching existing practice on this branch.
|
Review pushed: Headlines:
Code quality of the service path itself is high — see 'What is good' in the review. |
…tack - chat tool becomes a Mastra createTool via chat-engine AgentExtras.tools, wired host-side in apps/chat-api (PR #5126); flag gate stays host-side - credits: imageDescriptionCost fold-in pattern survives in chat-api finish+abort - add explicit 'why inline await, not Hatchet' rationale section - codeapi client + JWT minter now planned as shared packages/ module (chat-api + future CODE-element Hatchet worker) - CODE element plan unaffected by migration (note added)
Summary
chat-apiprototype stack and chat app proxy wiring currently on this branch.chat-apithat hits the live SSE endpoint and checks streamed text deltas, finish metadata, persisted messages, and credit decrement.chat-api, and executes the OpenRouter smoke. The smoke step is non-blocking for now and falls back to an invalidci-placeholdervalue untilOPENROUTER_API_KEYis configured as a repository secret.Verification
pnpm --filter @klicker-uzh/chat-api checkpnpm run check:allpnpm run buildtextDeltaParts=9,creditsUsed=0.00058458,threadId=b89bda77-bbe6-4685-a03f-21098018c8a9Notes
OPENROUTER_API_KEYas a repo or environment secret, confirm the workflow passes, then removecontinue-on-errorfrom the smoke step.ClickUp Links