diff --git a/apps/claw-frontend/src/app/globals.css b/apps/claw-frontend/src/app/globals.css index b0364061f..44762d870 100644 --- a/apps/claw-frontend/src/app/globals.css +++ b/apps/claw-frontend/src/app/globals.css @@ -1239,6 +1239,27 @@ line-height: 1.6; } + /* The CTA is an INVERTED panel — its ground is `--editorial-ink`, not + * `--editorial-paper`. So any child that re-sets `color` has to take that + * colour from the inverted set too. `--editorial-signal` and + * `--editorial-graphite` are both tuned for a paper ground, and on ink they + * land at 3.4:1 and 3.0:1 in light mode, 2.3:1 and 1.8:1 in dark. Both + * themes fail WCAG AA; dark merely fails louder, which is why Lighthouse + * reported it first. + * + * The compare pages never hit this: they put bare text in the panel and let + * it inherit the panel's own colour. Only the learn and integration topic + * pages nest `__verdict-label` + `__body` inside, so only those 32 URLs + * failed the gate. + */ + .editorial-comparison__cta .editorial-comparison__verdict-label { + color: var(--editorial-paper); + } + + .editorial-comparison__cta .editorial-comparison__body { + color: color-mix(in srgb, var(--editorial-paper) 85%, var(--editorial-ink)); + } + .editorial-comparison__cta-actions { display: flex; flex-wrap: wrap; diff --git a/apps/claw-routing-service/src/modules/routing/__tests__/routing-education.repository.spec.ts b/apps/claw-routing-service/src/modules/routing/__tests__/routing-education.repository.spec.ts index a791a8223..313679160 100644 --- a/apps/claw-routing-service/src/modules/routing/__tests__/routing-education.repository.spec.ts +++ b/apps/claw-routing-service/src/modules/routing/__tests__/routing-education.repository.spec.ts @@ -71,6 +71,7 @@ const buildRepo = (): { routerModelProfile: { deleteMany: jest.Mock; createMany: jest.Mock }; routerTopicProfile: { deleteMany: jest.Mock; createMany: jest.Mock }; routerWorkspacePrior: { findUnique: jest.Mock; upsert: jest.Mock }; + $queryRaw: jest.Mock; $transaction: jest.Mock; }; } => { @@ -106,6 +107,9 @@ const buildRepo = (): { routerModelProfile, routerTopicProfile, routerWorkspacePrior, + // The advisory lock that serialises the replace. Returns a resolved value + // so it can sit inside the $transaction array like any PrismaPromise. + $queryRaw: jest.fn().mockResolvedValue([{ pg_advisory_xact_lock: '' }]), // Mirrors Prisma's array-form $transaction: resolve each already-created // PrismaPromise and return the results in order, so commitCalibrationBatch // can destructure the created snapshot out of the batch. @@ -154,6 +158,37 @@ describe('RoutingEducationRepository.commitCalibrationBatch', () => { expect(prisma.routerTopicProfile.deleteMany).toHaveBeenCalledTimes(1); expect(snapshot.version).toBe('calibration-2'); }); + + it('takes the advisory lock as the FIRST statement in the transaction', async () => { + // Regression guard. Without this lock, two concurrent rebuilds interleave: + // the second transaction's DELETE snapshot predates the first's COMMIT, so + // it removes only the rows it can see and its INSERT then violates + // `router_model_profiles_provider_model_task_family_topic_key_key`. The + // P2002 is an unhandled rejection that kills routing-service outright, and + // `rebuildCalibrationSnapshot()` runs after EVERY routing outcome, so the + // exposure is every pair of concurrent generations. Observed twice in a + // long-thread stress run on 2026-08-30. + // + // Order is the whole point: a lock taken after the DELETE would let the + // race happen before it ever blocks anything. + const { repository, prisma } = buildRepo(); + + await repository.commitCalibrationBatch({ + version: 'calibration-3', + windowDays: 30, + summary: { decisionsAnalyzed: 1 }, + promptHints: { bestModelsByTaskFamily: [] }, + modelProfiles: [modelProfileRow()], + topicProfiles: [topicProfileRow()], + modelProfileRows: [modelProfileRow()], + topicProfileRows: [topicProfileRow()], + }); + + expect(prisma.$queryRaw).toHaveBeenCalledTimes(1); + const [operations] = prisma.$transaction.mock.calls[0] as [unknown[]]; + expect(operations).toHaveLength(7); + expect(operations[0]).toBe(prisma.$queryRaw.mock.results[0]?.value); + }); }); describe('RoutingEducationRepository.restoreCalibrationSnapshot', () => { @@ -167,6 +202,9 @@ describe('RoutingEducationRepository.restoreCalibrationSnapshot', () => { }); expect(prisma.$transaction).toHaveBeenCalledTimes(1); + // The rollback path replaces the same tables the same way, so it is exposed + // to the same race and takes the same lock first. + expect(prisma.$queryRaw).toHaveBeenCalledTimes(1); expect(prisma.routingCalibrationSnapshot.updateMany).toHaveBeenNthCalledWith(1, { data: { active: false }, where: { active: true }, diff --git a/apps/claw-routing-service/src/modules/routing/constants/routing-education.constants.ts b/apps/claw-routing-service/src/modules/routing/constants/routing-education.constants.ts index b674f57ee..8fefe43cb 100644 --- a/apps/claw-routing-service/src/modules/routing/constants/routing-education.constants.ts +++ b/apps/claw-routing-service/src/modules/routing/constants/routing-education.constants.ts @@ -65,3 +65,37 @@ export const WORKSPACE_PRIOR_BLEND_WEIGHT = 0.15; * never touches at all). */ export const MAX_WORKSPACE_PRIOR_NUDGE = 0.1; + +/** + * Advisory-lock key that serialises calibration commits. + * + * `commitCalibrationBatch` and `restoreCalibrationSnapshot` both replace the + * whole live profile set with `deleteMany()` followed by `createMany()`. That + * pair is not safe to run concurrently under READ COMMITTED: if the second + * transaction's DELETE takes its snapshot before the first COMMITs, it removes + * only the rows it can see, never observes the rows the first is inserting, and + * its own INSERT then violates + * `router_model_profiles_provider_model_task_family_topic_key_key`. + * + * That is not hypothetical. `rebuildCalibrationSnapshot()` runs on EVERY + * routing outcome, so every pair of concurrent generations is an opportunity, + * and the resulting P2002 is an unhandled rejection that kills the process — + * routing-service down, `/routing/models` 502, no message routable at all. + * Reproduced twice on 2026-08-30 under a long-thread stress run. + * + * The window widens as history accumulates: `findEducationWindow` returns a + * growing set of decisions, so the transaction takes longer the more the system + * has been used. It survived 24 concurrent generations on a young database and + * crashed reliably after ~1,400, which is why load testing a fresh install does + * not surface it. + * + * A transaction-scoped advisory lock makes the replace atomic against other + * writers and is released automatically on COMMIT or ROLLBACK, so a crashed + * transaction cannot strand it. + * + * Next in routing-service's 740_040_00N advisory-lock block (001 = deployment + * seed, 002 = router chain seed, 003 = model cost seed). The value must stay + * stable; it only has to be distinct from the other advisory locks held against + * this database. + */ +export const CALIBRATION_COMMIT_LOCK_KEY = 740_040_004; diff --git a/apps/claw-routing-service/src/modules/routing/repositories/routing-education.repository.ts b/apps/claw-routing-service/src/modules/routing/repositories/routing-education.repository.ts index 1ff9c726b..3f06ab38d 100644 --- a/apps/claw-routing-service/src/modules/routing/repositories/routing-education.repository.ts +++ b/apps/claw-routing-service/src/modules/routing/repositories/routing-education.repository.ts @@ -1,5 +1,6 @@ import { Injectable } from '@nestjs/common'; import { + Prisma, type RouterModelProfile, type RouterTopicProfile, type RouterWorkspacePrior, @@ -8,6 +9,7 @@ import { type RoutingOutcomeRecord, } from '../../../generated/prisma'; import { PrismaService } from '../../../infrastructure/database/prisma/prisma.service'; +import { CALIBRATION_COMMIT_LOCK_KEY } from '../constants/routing-education.constants'; import type { CommitCalibrationBatchInput, CreateRoutingFeedbackInput, @@ -143,7 +145,14 @@ export class RoutingEducationRepository { input: CommitCalibrationBatchInput, ): Promise { const now = new Date(); - const [, snapshot] = await this.prisma.$transaction([ + // Serialise the whole replace. See CALIBRATION_COMMIT_LOCK_KEY: the + // delete-then-insert below races itself under concurrent rebuilds and the + // resulting P2002 takes the process down. The lock is transaction-scoped, + // so it is released on COMMIT or ROLLBACK without any cleanup path. + const [, , snapshot] = await this.prisma.$transaction([ + this.prisma.$queryRaw( + Prisma.sql`SELECT pg_advisory_xact_lock(${CALIBRATION_COMMIT_LOCK_KEY})::text`, + ), this.prisma.routingCalibrationSnapshot.updateMany({ data: { active: false }, where: { active: true }, @@ -184,7 +193,12 @@ export class RoutingEducationRepository { topicProfileRows: RouterTopicProfileRecord[]; }): Promise { const now = new Date(); + // Same lock as the commit path: a rollback replaces the same live tables + // the same way, so it must not interleave with a concurrent rebuild either. await this.prisma.$transaction([ + this.prisma.$queryRaw( + Prisma.sql`SELECT pg_advisory_xact_lock(${CALIBRATION_COMMIT_LOCK_KEY})::text`, + ), this.prisma.routingCalibrationSnapshot.updateMany({ data: { active: false }, where: { active: true }, diff --git a/scripts/qa-lab/client.mjs b/scripts/qa-lab/client.mjs index 9d9a78509..7e6f20acc 100644 --- a/scripts/qa-lab/client.mjs +++ b/scripts/qa-lab/client.mjs @@ -20,8 +20,14 @@ export const ALLOW_METERED = false; let token = null; let tokenAt = 0; +// Whoever the process is currently acting as. Kept separate from EMAIL / +// PASSWORD so the silent token refresh below renews the ACTIVE identity rather +// than snapping back to the lab account mid-run. +let activeEmail = EMAIL; +let activePassword = PASSWORD; + export async function login() { - if (EMAIL.length === 0 || PASSWORD.length === 0) { + if (activeEmail.length === 0 || activePassword.length === 0) { throw new Error( 'QA_LAB_EMAIL and QA_LAB_PASSWORD must be set. See skills/audit-conversational-context.md.', ); @@ -29,7 +35,7 @@ export async function login() { const res = await fetch(`${BASE}/auth/login`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ email: EMAIL, password: PASSWORD }), + body: JSON.stringify({ email: activeEmail, password: activePassword }), }); if (!res.ok) throw new Error(`login failed ${res.status} ${await res.text()}`); const body = await res.json(); @@ -38,6 +44,47 @@ export async function login() { return body.user; } +/** + * Act as a different user for every subsequent call. + * + * Memories are USER-scoped and are injected into every thread that user owns, + * so an account that has accumulated fixtures from earlier runs quietly + * contaminates later ones: a stored "the internal project codename is ORCHID-…" + * competes with the codename the scenario seeds in the thread, and the model + * picks one. Measured on the shared lab account — three recall probes at + * distances 4, 24 and 56 all failed at exactly the same rate, which is the + * signature of a constant competing fact rather than of context loss. + * + * A long run therefore belongs on its own fresh account, where the only thing + * in scope is the conversation being measured. + */ +export async function loginAs(email, password) { + activeEmail = email; + activePassword = password; + token = null; + tokenAt = 0; + return login(); +} + +/** Mint a brand-new empty account (admin credentials required). */ +export async function createIsolatedUser(tag) { + const suffix = `${tag}${Date.now().toString(36)}`; + const account = { + email: `qa-${suffix}@claw.local`, + username: `qa${suffix}`.replace(/[^a-z0-9]/gi, '').slice(0, 28), + password: 'QaIsolated123!', + firstName: 'QA', + lastName: 'Isolated', + }; + await login(); + const created = await api('POST', '/users', account); + if (!created.ok) { + throw new Error(`createIsolatedUser failed ${created.status} ${JSON.stringify(created.body)}`); + } + const user = await loginAs(account.email, account.password); + return { ...account, id: user.id }; +} + async function ensureToken() { // access token lives 900s; refresh well before the edge if (token === null || Date.now() - tokenAt > 600_000) await login(); diff --git a/scripts/qa-lab/memory-experiment.mjs b/scripts/qa-lab/memory-experiment.mjs index ba4269a51..6d5bcd22e 100644 --- a/scripts/qa-lab/memory-experiment.mjs +++ b/scripts/qa-lab/memory-experiment.mjs @@ -88,8 +88,17 @@ for (const row of rows) { } console.log(`\n${rows.filter((r) => r.pass).length}/${rows.length} passed`); +// Forgetting a memory is confirmation-gated (`FORGET_CONFIRMATION_REQUIRED`), +// and this cleanup used to omit the flag and ignore the result. The 400 was +// silent, so every run left its standing INSTRUCTION behind — and the NEXT run +// then had two conflicting "always end your reply with …" memories live at +// once, obeyed the older one, and reported a product failure that was really +// its own litter. A cleanup whose failure is invisible is not a cleanup. if (memoryId !== null) { - await api('DELETE', `/memories/${memoryId}`); + const forgotten = await api('DELETE', `/memories/${memoryId}?confirm=FORGET`); + if (!forgotten.ok) { + console.log(`WARNING: could not forget ${memoryId} (${String(forgotten.status)}) — next run may see it`); + } await sleep(200); } writeJson(`${OUT}/summary.json`, { runId: RUN_ID, threadId: thread.id, memoryId, rows, answer }); diff --git a/scripts/qa-lab/stress-context.mjs b/scripts/qa-lab/stress-context.mjs new file mode 100644 index 000000000..98a123442 --- /dev/null +++ b/scripts/qa-lab/stress-context.mjs @@ -0,0 +1,326 @@ +// Long-thread stress: does the whole conversation still reach the model when a +// thread runs to hundreds of messages? +// +// The earlier suites prove the CONTRACT at ordinary lengths (20-60 messages). +// This one runs threads far past the point where the token budget must start +// evicting, because that is where a context system either degrades honestly or +// lies. Two different things are measured, and conflating them is how the +// original defect survived so long: +// +// DELIVERY — did the composer hand the model every message it had budget +// for, and evict ONLY for budget? This is the product's promise +// and it must hold at every length. +// RECALL — did the model then use what it was given? This is the model's +// job, and a weak model failing it is not a context bug. +// +// So every probe records the manifest alongside the answer. A recall failure on +// a turn where the fact was demonstrably IN the prompt is reported as a model +// result, never as a context regression. +// +// Memories are user-scoped and inject into every thread, so a shared account's +// leftovers can compete with the facts a scenario plants. `--isolated` mints a +// fresh account to rule that out; see the note above `createIsolatedUser` below +// for why this particular harness does not need it, and what it costs. +import { + login, createIsolatedUser, loadAllowedModels, createThread, sendMessage, awaitAssistant, + getReceipt, appendJsonl, writeJson, pool, assertFree, +} from './client.mjs'; + +const args = process.argv.slice(2).reduce((acc, cur, i, arr) => { + if (cur.startsWith('--')) acc[cur.slice(2)] = arr[i + 1] ?? 'true'; + return acc; +}, {}); + +/** Threads to run in parallel families. */ +const THREADS = Number(args.threads ?? 12); +/** User turns per thread. Each turn produces a user + an assistant message. */ +const TURNS = Number(args.turns ?? 120); +/** Simultaneous in-flight generations across the whole run. */ +const WORKERS = Number(args.workers ?? 4); + +const RUN_ID = `STRESS-${Date.now().toString(36)}`; +const OUT = `./results/${RUN_ID}`; + +const log = (msg) => { + const t = new Date().toISOString().slice(11, 19); + console.log(`[${t}] ${msg}`); +}; + +// Filler is deliberately boring and self-contained: it must add LENGTH without +// adding facts that could be confused with the planted ones. +const FILLER = [ + 'In one short sentence, what is a bloom filter?', + 'In one short sentence, what is a write-ahead log?', + 'In one short sentence, what is consistent hashing?', + 'In one short sentence, what is a merkle tree?', + 'In one short sentence, what is backpressure?', + 'In one short sentence, what is a vector clock?', + 'In one short sentence, what is quorum consensus?', + 'In one short sentence, what is copy-on-write?', +]; + +// `--verbose` swaps in filler that asks for real prose. +// +// One-sentence answers are cheap, and a 432-message thread of them came to +// 12,780 tokens against a 24,515 budget — so the eviction path never ran and +// "0 delivery violations" only ever proved the easy case. Long answers push a +// thread past its own window, which is where the composer has to start +// choosing and where its choices are worth checking. +const VERBOSE_FILLER = [ + 'Explain bloom filters in about 200 words, including false-positive behaviour.', + 'Explain write-ahead logging in about 200 words, including crash recovery.', + 'Explain consistent hashing in about 200 words, including virtual nodes.', + 'Explain merkle trees in about 200 words, including anti-entropy repair.', + 'Explain backpressure in about 200 words, including bounded queues.', + 'Explain vector clocks in about 200 words, including concurrent updates.', + 'Explain quorum consensus in about 200 words, including R + W > N.', + 'Explain copy-on-write in about 200 words, including snapshot isolation.', +]; + +/** + * An upstream provider refusing on ITS OWN account limits, returned as a normal + * assistant message. Matched on the shape every such notice shares rather than + * on one vendor's wording, so a second provider's phrasing is still caught. + */ +const PROVIDER_LIMIT_PATTERN = /usage limit|rate limit|quota exceeded|upgrade for higher|insufficient (credit|balance)/i; + +const VERBOSE = args.verbose === 'true'; +const fillerAt = (turn) => + VERBOSE ? VERBOSE_FILLER[turn % VERBOSE_FILLER.length] : FILLER[turn % FILLER.length]; + +/** + * A thread of `TURNS` user turns that plants a uniquely-named fact every tenth + * turn and re-asks for earlier ones at ever-growing distance. + * + * Facts carry a per-thread nonce. Without one, a probe can be satisfied by + * another thread's value (or by a previous run's), which reads as a pass and + * hides a real failure. + */ +function buildPlan(nonce) { + const plan = []; + const facts = []; + for (let turn = 0; turn < TURNS; turn += 1) { + if (turn % 10 === 0) { + const index = facts.length + 1; + const value = `${nonce}-${String(index).padStart(2, '0')}`; + facts.push({ index, value, plantedAtTurn: turn }); + plan.push({ + kind: 'plant', + factIndex: index, + content: `Record this: parameter ${String(index)} is ${value}. Acknowledge in one short sentence.`, + }); + continue; + } + // Probe on turns ending in 5. ALTERNATE between the very first fact and a + // recent one. + // + // The obvious design — cycle through the fact list — is what this used to + // do, and it quietly never tests anything: in a 432-message thread it + // asked at a maximum distance of 31 messages, because the cycle keeps + // landing on a recent fact. A distance test whose distance is bounded by + // its own indexing measures nothing about distance. Always re-asking + // fact #1 is what makes the probe distance grow with the thread. + if (turn % 5 === 0 && facts.length > 0) { + const askOldest = (turn / 5) % 2 === 1; + const target = askOldest ? facts[0] : facts[facts.length - 1]; + plan.push({ + kind: 'probe', + factIndex: target.index, + expect: target.value, + plantedAtTurn: target.plantedAtTurn, + content: `What is the value of parameter ${String(target.index)}? Reply with the value only, nothing else.`, + }); + continue; + } + plan.push({ kind: 'filler', content: fillerAt(turn) }); + } + return plan; +} + +// `--isolated` mints a brand-new account, which is the right call whenever a +// scenario plants a fact under a GENERIC name ("the project codename") that a +// leftover memory could also claim. It costs quota headroom though: a fresh +// account lands on the Free plan and its daily token quota is exhausted inside +// ~15 turns of a long thread, so a run of this size cannot finish there. +// +// This harness does not need it. Every planted value carries a per-thread nonce +// (`P