diff --git a/docs/openapi/prompts-v2-api.yaml b/docs/openapi/prompts-v2-api.yaml index 3b6de631f..6e12e0abb 100644 --- a/docs/openapi/prompts-v2-api.yaml +++ b/docs/openapi/prompts-v2-api.yaml @@ -254,6 +254,11 @@ v2-prompts-by-brand-and-id: tags: - customer-config summary: Update a single prompt + description: >- + Updates a single prompt. `origin` is NOT patchable — it is fixed by the + writer that created the row and is never re-derived on update + (origin-dimension.md §3 item 3); an `origin` in the body is ignored, + leaving the stored value untouched. operationId: updatePromptByBrandAndId security: - ims_key: [ ] diff --git a/docs/openapi/schemas.yaml b/docs/openapi/schemas.yaml index 781597e45..47107daae 100644 --- a/docs/openapi/schemas.yaml +++ b/docs/openapi/schemas.yaml @@ -6510,6 +6510,12 @@ V2PromptListResponse: V2Prompt: type: object description: A prompt with brand, category, and topic enrichment + # FIX (MysticatBot suggestion): `origin` is required (non-nullable) in the + # response — it is NOT NULL in production and `mapRowToPrompt` returns it + # verbatim with no fallback (origin-dimension.md §2.3 / §3 item 4), so + # SDK-generated types must reflect it as always-present, never optional. + required: + - origin properties: id: type: string @@ -6643,6 +6649,13 @@ V2PromptInput: type: string enum: [ai, human] default: human + description: >- + Who authored the prompt's text. SERVICE-PRINCIPAL-ONLY and read-only for + end users: a user-authenticated request (IMS/JWT) has this field IGNORED + (never rejected) and the value derived as `human`; only a service + principal (e.g. the generation pipeline) may assert it, validated against + the enum. It is never patched on update — it is fixed by the writer that + created the row (origin-dimension.md §3). source: type: string description: The source system or process that created the prompt diff --git a/src/controllers/brands.js b/src/controllers/brands.js index 8b32eb126..0a9f3e8f5 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -49,6 +49,7 @@ import { getPromptStats, resolveBrandUuid, findPromptsBlockingRegionRemoval, + deriveV2PromptOrigin, } from '../support/prompts-storage.js'; import { listBrands, @@ -572,10 +573,36 @@ function BrandsController(ctx, log, env) { return notFound(`Brand not found: ${brandId}`); } + // `origin` is derived from the request PRINCIPAL, never trusted from the + // body (origin-dimension.md §3): a user (IMS/JWT) write is `human`, body + // ignored; a service principal (e.g. DRS via admin x-api-key, whose auth + // type is neither `ims` nor `jwt`) is believed. The auth type is read from + // the per-request context — the same source as `updatedBy` above — so it + // reflects the actual caller. Stamp it here so the store writes the derived + // value on insert; on update the stored origin is preserved (upsertPrompts) + // and never patched (updatePromptById). + // + // Fail SAFE to the least-privileged (USER) principal: an ABSENT or + // indeterminate auth type must NEVER fall through to the privileged service + // path that honours a body-supplied `origin`. Only a KNOWN non-user auth + // type (jwt/ims are user; anything else, e.g. DRS admin x-api-key, is + // service) is trusted as a service principal. `authWrapper` blocks + // unauthenticated requests today, but a future unwrapped caller (an internal + // queue consumer, re-ordered middleware) must not silently gain service + // privilege — hence `!authType → user`, and a non-function `getType` resolves + // to `undefined` (→ user) rather than throwing. + const { authInfo } = context.attributes ?? {}; + const authType = typeof authInfo?.getType === 'function' ? authInfo.getType() : undefined; + const isUserPrincipal = !authType || authType === 'jwt' || authType === 'ims'; + const derivedPrompts = prompts.map((p) => ({ + ...p, + origin: deriveV2PromptOrigin(p?.origin, isUserPrincipal), + })); + const { created, updated, prompts: outPrompts } = await upsertPrompts({ organizationId: spaceCatId, brandUuid, - prompts, + prompts: derivedPrompts, postgrestClient, updatedBy, classifyIntent: classifyIntent ?? undefined, diff --git a/src/support/prompts-storage.js b/src/support/prompts-storage.js index 1f8ae67c4..9e0d74b7d 100644 --- a/src/support/prompts-storage.js +++ b/src/support/prompts-storage.js @@ -24,6 +24,45 @@ import { INTENT_VALUES, normalizeIntent } from './intent.js'; // import cycle. Existing importers of these from `prompts-storage.js` keep working. export { INTENT_VALUES, normalizeIntent }; +/** + * The closed `origin` vocabulary — who authored the prompt's text + * (origin-dimension.md §1). Matches the `category_origin` enum on `prompts.origin`. + */ +export const V2_PROMPT_ORIGINS = Object.freeze(['ai', 'human']); +const DEFAULT_ORIGIN = 'human'; + +/** + * Derives the `origin` to store for a v2-prompts write, as a function of the + * request PRINCIPAL, never of the caller-supplied body value (origin-dimension.md + * §3). `origin` records who authored the prompt's text and is read-only wherever a + * user can reach it: + * + * - a USER-authenticated principal (IMS / JWT) always writes `human`; any + * `origin` in the body is IGNORED (never rejected — the derived value is + * authoritative, so the caller loses nothing); + * - a SERVICE principal (e.g. DRS via admin `x-api-key`) is believed: its body + * value is honoured, validated against {@link V2_PROMPT_ORIGINS}, defaulting + * to `human` only when absent or out-of-vocabulary. This is the DRS contract + * (`origin: 'ai'`); dropping it would relabel every generated prompt `human` + * on its next upsert (origin-dimension.md §3 consequence 1). + * + * This governs CREATE only — `origin` is never patched on update (it is fixed by + * the writer that created the row), which the update path enforces by not writing + * the column at all. + * + * @param {unknown} bodyOrigin - the caller-supplied `origin`, or undefined. + * @param {boolean} isUserPrincipal - true for an IMS/JWT user request. + * @returns {string} the origin to store (`ai` or `human`). + */ +export function deriveV2PromptOrigin(bodyOrigin, isUserPrincipal) { + if (isUserPrincipal) { + return DEFAULT_ORIGIN; + } + return V2_PROMPT_ORIGINS.includes(/** @type {string} */ (bodyOrigin)) + ? /** @type {string} */ (bodyOrigin) + : DEFAULT_ORIGIN; +} + /** * Per-client cache of whether `prompts.intent` is selectable/writable. Keyed by * the PostgREST client so unit tests (fresh mock clients) never bleed state and @@ -406,7 +445,16 @@ function mapRowToPrompt(row) { name: row.name, regions: row.regions || [], status: row.status || 'active', - origin: row.origin || 'human', + // Return the stored `origin` verbatim — deliberately NO `|| 'human'` AND no + // `?? 'human'` fallback (origin-dimension.md §WP-O2b item 4 / §2.3). + // INVARIANT: `prompts.origin` is NOT NULL in production (zero NULLs in + // 265,980 rows, §2.3). Any fallback — including nullish-coalescing, which + // masks NULL exactly as `||` masks it for a NULL — would silently mislabel a + // model-written (`ai`) prompt as `human` were a NULL ever present, the exact + // corruption this dimension exists to prevent. Surfacing the raw value is the + // fail-loud choice over a fabricated `human`; unlike `source`/`status`, whose + // fallbacks are cosmetic, an origin fallback is a correctness hazard. + origin: row.origin, source: row.source || 'config', intent: row.intent ?? null, createdAt: row.created_at, @@ -850,6 +898,12 @@ export async function upsertPrompts({ status: 'active', intent: row.intent ?? match.intent, source: match.source ?? source, + // `origin` is fixed by the writer that created the row and is never + // re-derived on a later write (origin-dimension.md §3): preserve the + // stored value across a reactivation. `?? row.origin` is a defensive + // fallback for an in-memory/test match without an origin, mirroring + // `source` above — not a backfill path (prod has zero NULL origins). + origin: match.origin ?? row.origin, }; toUpdate.push(reactivated); processed.push({ ...reactivated, prompt_id: promptId }); @@ -859,7 +913,18 @@ export async function upsertPrompts({ } if (match) { - const updated = { ...row, id: match.id, source: match.source ?? source }; + // `source` AND `origin` are both immutable on an UPDATE: source names the + // producing system, origin names the writer that created the row, and + // neither is re-derived on a later write (origin-dimension.md §3). Preserve + // the stored values so a user-principal derive of `human` cannot relabel an + // existing `ai` prompt. `?? row.*` is the same defensive in-memory/test + // fallback used for `source` — not a backfill. + const updated = { + ...row, + id: match.id, + source: match.source ?? source, + origin: match.origin ?? row.origin, + }; toUpdate.push(updated); processed.push({ ...updated, prompt_id: promptId }); } else { @@ -1050,9 +1115,10 @@ export async function updatePromptById({ if (updates.status !== undefined) { patch.status = updates.status; } - if (updates.origin !== undefined) { - patch.origin = updates.origin; - } + // `origin` is deliberately NOT patchable: it is fixed by the writer that + // created the row and is never re-derived on update (origin-dimension.md §3 + // item 3 / §1 item 5). A caller-supplied `origin` in the PATCH body is ignored, + // leaving the stored value — including an `ai` prompt's — untouched. if (updates.intent !== undefined) { // The shared fallback strips intent when the column is known-absent. patch.intent = normalizeIntent(updates.intent); diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index 306c1704f..5abda8700 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -22,7 +22,7 @@ import { buildPromptDto, normalizePromptInput, createOnePrompt, - makeTypeInjector, + makePromptTagInjector, parseUpdatePromptBody, mapLimit, publishAffected, @@ -32,6 +32,7 @@ import { BULK_CREATE_CONCURRENCY, BULK_PROMPTS_MAX_ITEMS, } from './prompts.js'; +import { ORIGIN_VALUE } from '../prompt-tags.js'; import { resolveProject, buildSliceProjectMap, sliceKey } from '../subworkspace-projects.js'; import { redactUpstreamMessage } from '../rest-transport.js'; import { createHeadroomGuard } from '../dynamic-allocation-active.js'; @@ -140,7 +141,15 @@ export async function handleCreatePromptsSubworkspace( } const projectsBySlice = await buildSliceProjectMap(transport, workspaceId, log); - const injectComputedType = makeTypeInjector(transport, workspaceId, classifyPromptType, log); + // CREATE: user-authenticated write → derived `origin` is `human` (see the + // flat-mode twin handleCreatePrompts and origin-dimension.md §3). + const injectComputedTags = makePromptTagInjector( + transport, + workspaceId, + classifyPromptType, + log, + { originValue: ORIGIN_VALUE.HUMAN }, + ); // PROMPT metering seam (Rainer, live-verified LLMO-6190): the metered write is // `createPromptsByIds` (inside `createOnePrompt` below), NOT publish — a disguised-quota 405 @@ -176,8 +185,9 @@ export async function handleCreatePromptsSubworkspace( } const projectId = String(project.id); try { - // Unified layer: strip any caller-supplied type + inject the computed one. - const typed = await injectComputedType(projectId, input); + // Unified layer: strip any caller-supplied type/origin + inject the + // computed type and the derived origin (`human`). + const typed = await injectComputedTags(projectId, input); // LLMO-6190 follow-up: the metered write itself can still 405 as a disguised metered-quota // rejection despite the pre-loop sizing above (the live-verified ~9s gateway // write-enforcement lag after a JIT top-up) — route it through `headroom.retryOnQuota` (a @@ -299,8 +309,11 @@ export async function handleUpdatePromptSubworkspace( // Recompute the type tag from the NEW text BEFORE any upstream write (see // the flat-mode twin): a classification failure aborts cleanly with the // prompt completely untouched. - const injectComputedType = makeTypeInjector(transport, workspaceId, classifyPromptType, log); - const typed = await injectComputedType(projectId, { + // No `originValue`: origin is never re-derived on edit (origin-dimension.md §3 + // item 3); the stored origin the caller echoes rides through untouched. See the + // flat-mode twin handleUpdatePrompt. + const injectComputedTags = makePromptTagInjector(transport, workspaceId, classifyPromptType, log); + const typed = await injectComputedTags(projectId, { text: nextText, geoTargetId, tagIds: nextTagIds, }); diff --git a/src/support/serenity/handlers/prompts.js b/src/support/serenity/handlers/prompts.js index 75e71f540..e70b46752 100644 --- a/src/support/serenity/handlers/prompts.js +++ b/src/support/serenity/handlers/prompts.js @@ -19,7 +19,8 @@ import { redactUpstreamMessage } from '../rest-transport.js'; import { ERROR_CODES, isUpstreamGone } from '../errors.js'; import { normalizeGeoTargetId, normalizeLanguageCode, isValidTagIdFormat } from '../validation.js'; import { invalidateTagCacheForProject } from './markets.js'; -import { resolveTypeValueInjection } from '../tag-tree.js'; +import { resolveTypeValueInjection, resolveClosedValueInjection } from '../tag-tree.js'; +import { DIMENSION, ORIGIN_VALUE } from '../prompt-tags.js'; // TWIN FILE: the slice→project orchestration here is paralleled by the // subworkspace-mode handlers in prompts-subworkspace.js. The duplication is @@ -276,6 +277,13 @@ export async function publishAffected( * (update) so the two write paths can't silently diverge on what counts as * a valid tag id. * + * This cap bounds the CALLER-supplied tags only. The server-derived dimension + * tags (`type`, `origin`) are injected downstream by {@link makePromptTagInjector} + * AFTER this sanitize, and are intentionally EXEMPT from the user-facing cap — a + * write may therefore carry up to `MAX_TAG_IDS` + 2 ids. They must never be + * dropped to fit the cap: a prompt missing its `type`/`origin` tag is invisible + * to that dimension's filter. + * * @param {unknown} raw * @returns {string[]} */ @@ -366,60 +374,109 @@ export async function createOnePrompt(transport, semrushWorkspaceId, projectId, } /** - * Builds the per-request `type` injector — the UNIFIED classification layer - * (serenity-docs#31). Given a pure `classifyPromptType(text, geoTargetId)` - * closure (built by the controller from the brand name + region-clamped - * aliases) that yields a BARE `type` value (`branded` / `non-branded`), it - * returns `injectComputedType(projectId, input)` which: - * - STRIPS every caller-supplied tag id that lives under the `type` root (the - * client may never set the value), and - * - APPENDS the pre-resolved upstream id of the server-computed value. The - * atomic `createPromptsByIds` 500s on an unresolved id, so it is resolved - * BEFORE the write. - * The returned input carries the rewritten `tagIds`, so the caller's response - * echo reflects the computed type without a refetch (decision 5). + * Builds the per-request prompt-tag injector — the UNIFIED server-owned-dimension + * layer (serenity-docs#31 for `type`; origin-dimension.md §3 for `origin`). It + * stamps the two dimensions a client may never set on a prompt: `type` (branded / + * non-branded, classified from the text) and `origin` (who authored the prompt). + * + * `injectComputedTags(projectId, input)` STRIPS every caller-supplied tag id that + * lives under a server-owned dimension's root and APPENDS the pre-resolved + * upstream id of the server value. The strip is BY RESOLVED ROOT ID, never by + * name: a tag's dimension is its root ancestor, so a customer category + * legitimately named `branded` or `ai` is not under a server root and is left + * alone (origin-dimension.md §3, gate 8). The rewritten `tagIds` are returned so + * the caller's response echo needs no refetch (decision 5). + * + * **`type`** — resolved from `classifyPromptType(text, geoTargetId)` on every + * write (create AND update): it is a classification of the prompt text, so it is + * always safe to recompute. A non-function `classifyPromptType` (defensive) skips + * the `type` step. * - * The strip set is every id under the `type` root, not a name prefix: a tag's - * dimension is its root, and a sub-category could legitimately be named - * `branded` without being a `type` value. + * **`origin`** — carries the CREATE/UPDATE ASYMMETRY (origin-dimension.md §3 + * item 3). It is a fact about the row's CREATION, never a classification, so: + * - on CREATE (`originValue` set, e.g. `human` for a user-authenticated write), + * any caller-supplied origin id is stripped and the derived value injected; + * - on UPDATE (`originValue` unset) the injector leaves origin ALONE. The stored + * value the caller echoes back rides through the full-replace tag write + * unchanged. Re-deriving would relabel every edited `ai` prompt `human`; + * stripping without injecting would leave the prompt invisible to the + * dimension's filter — both are illegal, so the update path does neither. * - * Resolution ({@link resolveTypeValueInjection}, two tag-tree reads per distinct - * `type` value per project — the root level plus the `type` root's children) is - * memoized for the request, so a bulk create fans out over the distinct computed - * values rather than over the items. A non-function `classifyPromptType` - * (defensive) is a pass-through. + * Resolution ({@link resolveTypeValueInjection} / {@link resolveClosedValueInjection}, + * two tag-tree reads per distinct value per project — the root level plus the + * root's children) is memoized for the request, so a bulk create fans out over + * the distinct computed values rather than over the items. The origin value is + * constant per request, so its resolution is memoized per project. * - * `resolveTypeValueInjection` resolves or throws, so the computed tag is always - * attached. It must never be dropped: `type` is the one dimension a client may - * not set, so a prompt written without it stays unclassified forever, and the - * caller sees a 2xx. Failing the write instead is free — the upstream bulk create - * is atomic and has not run yet. + * Resolution resolves or throws, so a server tag is always attached; it is never + * dropped, and a resolution failure aborts the write (which is free — the upstream + * bulk create is atomic and has not run yet) rather than writing an unclassified + * or unattributed prompt behind a 2xx. * * @param {object} transport - Serenity transport (Semrush proxy client). * @param {string} semrushWorkspaceId * @param {((text: string, geoTargetId: number) => string) | undefined} classifyPromptType * @param {object} [log] + * @param {{ originValue?: string }} [options] - `originValue` is the derived + * `origin` to inject on CREATE; omit it on UPDATE so origin is left untouched. * @returns {(projectId: string, input: { text: string, geoTargetId: number, * tagIds: string[] }) => * Promise<{ text: string, geoTargetId: number, tagIds: string[] }>} */ -export function makeTypeInjector(transport, semrushWorkspaceId, classifyPromptType, log) { +export function makePromptTagInjector( + transport, + semrushWorkspaceId, + classifyPromptType, + log, + options = {}, +) { + const { originValue } = options; /** @type {Map>} */ - const cache = new Map(); - return async function injectComputedType(projectId, input) { - if (typeof classifyPromptType !== 'function') { - return input; + const typeCache = new Map(); + /** @type {Map>} */ + const originCache = new Map(); + return async function injectComputedTags(projectId, input) { + let { tagIds } = input; + + // type — every write (safe to recompute from the text). + if (typeof classifyPromptType === 'function') { + const typeValue = classifyPromptType(input.text, input.geoTargetId); + const key = `${projectId} ${typeValue}`; + let pending = typeCache.get(key); + if (!pending) { + pending = resolveTypeValueInjection( + transport, + semrushWorkspaceId, + projectId, + typeValue, + log, + ); + typeCache.set(key, pending); + } + const { computedId, typeTagIds } = await pending; + tagIds = [...tagIds.filter((id) => !typeTagIds.includes(id)), computedId]; } - const typeValue = classifyPromptType(input.text, input.geoTargetId); - const key = `${projectId} ${typeValue}`; - let pending = cache.get(key); - if (!pending) { - pending = resolveTypeValueInjection(transport, semrushWorkspaceId, projectId, typeValue, log); - cache.set(key, pending); + + // origin — CREATE only. `originValue` unset means UPDATE: leave origin alone + // (the stored value the caller echoes rides through the replace-mode write). + if (originValue) { + let pending = originCache.get(projectId); + if (!pending) { + pending = resolveClosedValueInjection( + transport, + semrushWorkspaceId, + projectId, + DIMENSION.ORIGIN, + originValue, + log, + ); + originCache.set(projectId, pending); + } + const { computedId, valueTagIds } = await pending; + tagIds = [...tagIds.filter((id) => !valueTagIds.includes(id)), computedId]; } - const { computedId, typeTagIds } = await pending; - const stripped = input.tagIds.filter((id) => !typeTagIds.includes(id)); - return { ...input, tagIds: [...stripped, computedId] }; + + return { ...input, tagIds }; }; } @@ -525,11 +582,16 @@ export async function handleCreatePrompts( projectsBySlice.set(`${p.getGeoTargetId()}:${p.getLanguageCode()}`, p); } - const injectComputedType = makeTypeInjector( + // CREATE: user-authenticated write → derived `origin` is `human` + // (origin-dimension.md §3). Any caller-supplied origin tag id is stripped and + // this value injected; on the twin AI-generation path a service producer stamps + // `ai` via STANDARD_PROMPT_TAG_VALUES instead (that path does not run here). + const injectComputedTags = makePromptTagInjector( transport, semrushWorkspaceId, classifyPromptType, log, + { originValue: ORIGIN_VALUE.HUMAN }, ); const results = await mapLimit(inputs, BULK_CREATE_CONCURRENCY, async (raw) => { @@ -553,8 +615,9 @@ export async function handleCreatePrompts( } const projectId = project.getSemrushProjectId(); try { - // Unified layer: strip any caller-supplied type + inject the computed one. - const typed = await injectComputedType(projectId, input); + // Unified layer: strip any caller-supplied type/origin + inject the + // computed type and the derived origin (`human`). + const typed = await injectComputedTags(projectId, input); const semrushPromptId = await createOnePrompt( transport, semrushWorkspaceId, @@ -713,14 +776,17 @@ export async function handleUpdatePrompt( // Recompute the type tag from the NEW text BEFORE any upstream write: the // unified layer (tree read / on-demand tag create) resolves the computed // value's id first, so a classification failure aborts cleanly with the - // prompt completely untouched. - const injectComputedType = makeTypeInjector( + // prompt completely untouched. NO `originValue` is passed: `origin` is a fact + // about the row's creation, never re-derived on edit (origin-dimension.md §3 + // item 3). The prompt's stored origin id, echoed back by the caller, rides + // through the replace-mode tag write untouched. + const injectComputedTags = makePromptTagInjector( transport, semrushWorkspaceId, classifyPromptType, log, ); - const typed = await injectComputedType(projectId, { + const typed = await injectComputedTags(projectId, { text: nextText, geoTargetId, tagIds: nextTagIds, }); diff --git a/src/support/serenity/tag-tree.js b/src/support/serenity/tag-tree.js index 9712cb8a4..5dba4f5e7 100644 --- a/src/support/serenity/tag-tree.js +++ b/src/support/serenity/tag-tree.js @@ -572,39 +572,84 @@ export async function ensureClosedValue( } /** - * Resolves the id-based injection of a server-computed `type` value into a - * prompt write. Returns the wanted value's id plus EVERY id under the `type` - * root, so the caller can strip any caller-supplied `type` tag id (the client - * must never set the value itself). + * Resolves the id-based injection of a server-computed value into a prompt write, + * for any CLOSED dimension (`type`, `origin`, `intent`). Returns the wanted value's + * id plus EVERY id under that dimension's root, so the caller can strip any + * caller-supplied tag id beneath the SAME root before injecting the resolved one. + * + * The strip set is every id under the dimension's root, NOT a name match: a tag's + * dimension is its root ancestor, so a customer category legitimately named + * `branded` or `ai` (the collision the model spec's fixture proves survivable) is + * NOT in this set and is left untouched. The authorship root is resolved + * tolerantly by {@link ensureDimensionRoots}, so `DIMENSION.ORIGIN` addresses + * whichever physical root (`origin` or a legacy `source`) the project carries. * * @param {object} transport - Serenity transport (Semrush proxy client). * @param {string} semrushWorkspaceId * @param {string} projectId - * @param {string} wantValue - the computed bare `type` value (`branded` / `non-branded`). + * @param {string} dimension - a CLOSED dimension (`type` / `origin` / `intent`). + * @param {string} wantValue - the bare value to inject (must be in that dimension's + * fixed vocabulary). * @param {object} [log] - logger. - * @returns {Promise<{ computedId: string, typeTagIds: string[] }>} `computedId` is + * @returns {Promise<{ computedId: string, valueTagIds: string[] }>} `computedId` is * always resolved — {@link ensureChildren} throws rather than leave a hole, so a - * prompt can never be written with the server-computed `type` tag missing. + * prompt can never be written with the server-computed tag missing. `valueTagIds` + * is every id under the dimension's root (the strip set). */ -export async function resolveTypeValueInjection( +export async function resolveClosedValueInjection( transport, semrushWorkspaceId, projectId, + dimension, wantValue, log, ) { const roots = await ensureDimensionRoots(transport, semrushWorkspaceId, projectId, log); - const typeRootId = rootIdOf(roots, DIMENSION.TYPE); + const rootId = rootIdOf(roots, dimension); const { byName } = await ensureChildren( transport, semrushWorkspaceId, projectId, - typeRootId, + rootId, [wantValue], log, ); return { computedId: /** @type {string} */ (byName.get(wantValue)), - typeTagIds: [...byName.values()], + valueTagIds: [...byName.values()], }; } + +/** + * Resolves the id-based injection of a server-computed `type` value into a + * prompt write. Thin wrapper over {@link resolveClosedValueInjection} preserving + * the `type`-specific return key. Returns the wanted value's id plus EVERY id + * under the `type` root, so the caller can strip any caller-supplied `type` tag + * id (the client must never set the value itself). + * + * @param {object} transport - Serenity transport (Semrush proxy client). + * @param {string} semrushWorkspaceId + * @param {string} projectId + * @param {string} wantValue - the computed bare `type` value (`branded` / `non-branded`). + * @param {object} [log] - logger. + * @returns {Promise<{ computedId: string, typeTagIds: string[] }>} `computedId` is + * always resolved — {@link ensureChildren} throws rather than leave a hole, so a + * prompt can never be written with the server-computed `type` tag missing. + */ +export async function resolveTypeValueInjection( + transport, + semrushWorkspaceId, + projectId, + wantValue, + log, +) { + const { computedId, valueTagIds } = await resolveClosedValueInjection( + transport, + semrushWorkspaceId, + projectId, + DIMENSION.TYPE, + wantValue, + log, + ); + return { computedId, typeTagIds: valueTagIds }; +} diff --git a/test/controllers/brands.test.js b/test/controllers/brands.test.js index 1c4f42f0e..415e04a77 100644 --- a/test/controllers/brands.test.js +++ b/test/controllers/brands.test.js @@ -1018,6 +1018,83 @@ describe('Brands Controller', () => { expect(body).to.have.property('prompts'); }); + it('createPromptsByBrand honours a SERVICE principal\'s origin: ai (DRS contract, origin-dimension.md §3)', async () => { + // A non-ims/non-jwt principal (e.g. DRS via admin x-api-key) is believed: + // its asserted `origin` rides through to the store. Deleting or ignoring it + // would relabel every DRS-written prompt `human` on its next upsert. + const thenable = (v) => ({ then: (resolve) => resolve(v), catch: () => thenable(v) }); + const insertStub = sandbox.stub() + .returns({ select: () => thenable({ data: [{ prompt_id: 'new-1' }], error: null }) }); + mockDataAccess.services.postgrestClient.from = sandbox.stub().callsFake((table) => { + if (table === 'prompts') { + return { + select: () => ({ eq: () => ({ eq: () => thenable({ data: [], error: null }) }) }), + insert: insertStub, + update: () => ({ eq: () => thenable({ error: null }) }), + }; + } + const chain = { + select: sandbox.stub().returnsThis(), + eq: sandbox.stub().returnsThis(), + maybeSingle: sandbox.stub().resolves({ data: { id: BRAND_UUID }, error: null }), + }; + if (table === 'llmo_customer_config') { + chain.maybeSingle = sandbox.stub() + .resolves({ data: { config: { customer: { brands: [] } } }, error: null }); + } + return chain; + }); + + const response = await brandsController.createPromptsByBrand({ + ...context, + attributes: { authInfo: { getType: () => 'apikey', profile: { email: 'drs@service' } } }, + params: { spaceCatId: ORGANIZATION_ID, brandId: BRAND_UUID }, + data: [{ prompt: 'P', regions: ['us'], origin: 'ai' }], + dataAccess: mockDataAccess, + }); + + expect(response.status).to.equal(201); + const inserted = insertStub.firstCall.args[0]; + expect(inserted[0].origin).to.equal('ai'); + }); + + it('createPromptsByBrand coerces a USER principal\'s origin: ai to human (body ignored, never rejected)', async () => { + const thenable = (v) => ({ then: (resolve) => resolve(v), catch: () => thenable(v) }); + const insertStub = sandbox.stub() + .returns({ select: () => thenable({ data: [{ prompt_id: 'new-1' }], error: null }) }); + mockDataAccess.services.postgrestClient.from = sandbox.stub().callsFake((table) => { + if (table === 'prompts') { + return { + select: () => ({ eq: () => ({ eq: () => thenable({ data: [], error: null }) }) }), + insert: insertStub, + update: () => ({ eq: () => thenable({ error: null }) }), + }; + } + const chain = { + select: sandbox.stub().returnsThis(), + eq: sandbox.stub().returnsThis(), + maybeSingle: sandbox.stub().resolves({ data: { id: BRAND_UUID }, error: null }), + }; + if (table === 'llmo_customer_config') { + chain.maybeSingle = sandbox.stub() + .resolves({ data: { config: { customer: { brands: [] } } }, error: null }); + } + return chain; + }); + + const response = await brandsController.createPromptsByBrand({ + ...context, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + params: { spaceCatId: ORGANIZATION_ID, brandId: BRAND_UUID }, + data: [{ prompt: 'P', regions: ['us'], origin: 'ai' }], + dataAccess: mockDataAccess, + }); + + expect(response.status).to.equal(201); + const inserted = insertStub.firstCall.args[0]; + expect(inserted[0].origin).to.equal('human'); + }); + it('createPromptsByBrand persists normalized intent from the request body', async () => { const thenable = (v) => ({ then: (resolve) => resolve(v), catch: () => thenable(v) }); const insertStub = sandbox.stub() @@ -1055,6 +1132,138 @@ describe('Brands Controller', () => { expect(inserted[0].intent).to.equal('transactional'); }); + // origin-dimension.md §3 gate 5 (v2 API arm): a USER principal may not assert + // `origin` — the body value is ignored and `human` is derived and stored. + it('createPromptsByBrand ignores a user-supplied origin and stores human (gate 5)', async () => { + const thenable = (v) => ({ then: (resolve) => resolve(v), catch: () => thenable(v) }); + const insertStub = sandbox.stub() + .returns({ select: () => thenable({ data: [{ prompt_id: 'new-1' }], error: null }) }); + mockDataAccess.services.postgrestClient.from = sandbox.stub().callsFake((table) => { + if (table === 'prompts') { + return { + select: () => ({ eq: () => ({ eq: () => thenable({ data: [], error: null }) }) }), + insert: insertStub, + update: () => ({ eq: () => thenable({ error: null }) }), + }; + } + const chain = { + select: sandbox.stub().returnsThis(), + eq: sandbox.stub().returnsThis(), + maybeSingle: sandbox.stub().resolves({ data: { id: BRAND_UUID }, error: null }), + }; + if (table === 'llmo_customer_config') { + chain.maybeSingle = sandbox.stub() + .resolves({ data: { config: { customer: { brands: [] } } }, error: null }); + } + return chain; + }); + + const userContext = { + ...context, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }; + const response = await brandsController.createPromptsByBrand({ + ...userContext, + params: { spaceCatId: ORGANIZATION_ID, brandId: BRAND_UUID }, + // The user tries to assert `ai`; the server ignores it and derives `human`. + data: [{ prompt: 'P', regions: ['us'], origin: 'ai' }], + dataAccess: mockDataAccess, + }); + + expect(response.status).to.equal(201); + expect(insertStub.firstCall.args[0][0].origin).to.equal('human'); + }); + + // origin-dimension.md §3 gate 6: a SERVICE principal (auth type neither `ims` + // nor `jwt`, e.g. DRS via admin x-api-key) IS believed — its `origin: 'ai'` is + // honoured and stored. A regression here silently relabels every generated + // prompt `human` on its next upsert (DRS contract). + it('createPromptsByBrand honours a service principal origin=ai (gate 6)', async () => { + const thenable = (v) => ({ then: (resolve) => resolve(v), catch: () => thenable(v) }); + const insertStub = sandbox.stub() + .returns({ select: () => thenable({ data: [{ prompt_id: 'new-1' }], error: null }) }); + mockDataAccess.services.postgrestClient.from = sandbox.stub().callsFake((table) => { + if (table === 'prompts') { + return { + select: () => ({ eq: () => ({ eq: () => thenable({ data: [], error: null }) }) }), + insert: insertStub, + update: () => ({ eq: () => thenable({ error: null }) }), + }; + } + const chain = { + select: sandbox.stub().returnsThis(), + eq: sandbox.stub().returnsThis(), + maybeSingle: sandbox.stub().resolves({ data: { id: BRAND_UUID }, error: null }), + }; + if (table === 'llmo_customer_config') { + chain.maybeSingle = sandbox.stub() + .resolves({ data: { config: { customer: { brands: [] } } }, error: null }); + } + return chain; + }); + + const serviceContext = { + ...context, + attributes: { authInfo: { getType: () => 'legacyApiKey', profile: {} } }, + }; + const response = await brandsController.createPromptsByBrand({ + ...serviceContext, + params: { spaceCatId: ORGANIZATION_ID, brandId: BRAND_UUID }, + data: [{ prompt: 'P', regions: ['us'], origin: 'ai' }], + dataAccess: mockDataAccess, + }); + + expect(response.status).to.equal(201); + expect(insertStub.firstCall.args[0][0].origin).to.equal('ai'); + }); + + // FIX (MysticatBot nit): direct controller coverage of the fail-safe branch + // (`!authType → user`, brands.js:584). An ABSENT or indeterminate auth type + // must NEVER reach the privileged service path — a body-asserted `origin: 'ai'` + // is coerced to `human`. Previously this branch was only exercised indirectly + // via the `deriveV2PromptOrigin` unit tests, not through the controller wiring. + [ + { label: 'authInfo entirely absent (attributes: {})', attributes: {} }, + { label: 'authInfo present but getType is not a function', attributes: { authInfo: { profile: {} } } }, + ].forEach(({ label, attributes }) => { + it(`createPromptsByBrand fails safe to human when ${label} (gate 5 fail-safe)`, async () => { + const thenable = (v) => ({ then: (resolve) => resolve(v), catch: () => thenable(v) }); + const insertStub = sandbox.stub() + .returns({ select: () => thenable({ data: [{ prompt_id: 'new-1' }], error: null }) }); + mockDataAccess.services.postgrestClient.from = sandbox.stub().callsFake((table) => { + if (table === 'prompts') { + return { + select: () => ({ eq: () => ({ eq: () => thenable({ data: [], error: null }) }) }), + insert: insertStub, + update: () => ({ eq: () => thenable({ error: null }) }), + }; + } + const chain = { + select: sandbox.stub().returnsThis(), + eq: sandbox.stub().returnsThis(), + maybeSingle: sandbox.stub().resolves({ data: { id: BRAND_UUID }, error: null }), + }; + if (table === 'llmo_customer_config') { + chain.maybeSingle = sandbox.stub() + .resolves({ data: { config: { customer: { brands: [] } } }, error: null }); + } + return chain; + }); + + const response = await brandsController.createPromptsByBrand({ + ...context, + attributes, + params: { spaceCatId: ORGANIZATION_ID, brandId: BRAND_UUID }, + // A body-asserted `ai` must NOT be honoured for an unattributable caller. + data: [{ prompt: 'P', regions: ['us'], origin: 'ai' }], + dataAccess: mockDataAccess, + }); + + expect(response.status).to.equal(201); + expect(insertStub.firstCall.args[0][0].origin).to.equal('human'); + }); + }); + it('createPromptsByBrand returns 400 when prompts not an array', async () => { const response = await brandsController.createPromptsByBrand({ ...context, diff --git a/test/it/shared/tests/serenity.js b/test/it/shared/tests/serenity.js index 3c7712ab5..6c4e6f821 100644 --- a/test/it/shared/tests/serenity.js +++ b/test/it/shared/tests/serenity.js @@ -533,11 +533,13 @@ export default function serenityTests( expect(created.status).to.equal(200); expect(created.body.created).to.have.lengthOf(1); expect(created.body.created[0].semrushPromptId).to.be.a('string').that.is.not.empty; - // The write path now server-computes a branded/non-branded `type:` tag and - // appends it to the supplied tagIds, so the created prompt carries the two - // supplied tags plus one computed type tag. + // The write path server-stamps two dimensions the caller may not set: a + // branded/non-branded `type:` tag (classified from the text) AND, on a + // user-authenticated create, the derived `origin:` tag (`human`) — + // origin-dimension.md §3 (WP-O2b). So the created prompt carries the two + // supplied tags plus one computed `type` tag plus one derived `origin` tag. expect(created.body.created[0].tagIds).to.include.members([category.body.id, child.body.id]); - expect(created.body.created[0].tagIds).to.have.lengthOf(3); + expect(created.body.created[0].tagIds).to.have.lengthOf(4); expect(created.body.failed).to.deep.equal([]); // by_tags correlation: the id-based create embeds the tag ids, so filtering the prompt list diff --git a/test/support/prompts-storage.test.js b/test/support/prompts-storage.test.js index 8a051c7a4..da90bc133 100644 --- a/test/support/prompts-storage.test.js +++ b/test/support/prompts-storage.test.js @@ -30,6 +30,7 @@ import { isMissingIntentColumnError, findPromptsBlockingRegionRemoval, getIntentsByPromptIds, + deriveV2PromptOrigin, } from '../../src/support/prompts-storage.js'; use(chaiAsPromised); @@ -66,6 +67,36 @@ describe('prompts-storage', () => { afterEach(() => sandbox.restore()); + describe('deriveV2PromptOrigin (origin-dimension.md §3)', () => { + // `origin` is derived from the request PRINCIPAL, never trusted from the body + // where a user can reach it. This is the correctness-critical asymmetry: a + // user write is always `human`; only a service principal (e.g. DRS) may assert. + it('always returns `human` for a USER principal, ignoring the body value', () => { + expect(deriveV2PromptOrigin('ai', true)).to.equal('human'); + expect(deriveV2PromptOrigin('human', true)).to.equal('human'); + expect(deriveV2PromptOrigin(undefined, true)).to.equal('human'); + // A user cannot smuggle an out-of-vocabulary value in either — never rejected. + expect(deriveV2PromptOrigin('robot', true)).to.equal('human'); + }); + + it('honours a SERVICE principal\'s asserted `ai` — the DRS contract (item 6 guard)', () => { + expect(deriveV2PromptOrigin('ai', false)).to.equal('ai'); + }); + + it('honours a SERVICE principal\'s asserted `human`', () => { + expect(deriveV2PromptOrigin('human', false)).to.equal('human'); + }); + + it('defaults a SERVICE principal to `human` when the body value is absent', () => { + expect(deriveV2PromptOrigin(undefined, false)).to.equal('human'); + }); + + it('defaults a SERVICE principal to `human` when the body value is out-of-vocabulary', () => { + expect(deriveV2PromptOrigin('robot', false)).to.equal('human'); + expect(deriveV2PromptOrigin('', false)).to.equal('human'); + }); + }); + describe('normalizeIntent', () => { it('returns null for absent, empty, or whitespace values', () => { expect(normalizeIntent(undefined)).to.be.null; @@ -899,7 +930,11 @@ describe('prompts-storage', () => { expect(result).to.not.be.null; expect(result.regions).to.deep.equal([]); expect(result.status).to.equal('active'); - expect(result.origin).to.equal('human'); + // `origin` is returned verbatim, with no `|| 'human'` fallback + // (origin-dimension.md §2.3 / §3 item 4): origin is NOT NULL in production, + // and a fallback would silently mislabel a model-written prompt as human. + // A row carrying no origin therefore passes through as `undefined`. + expect(result.origin).to.be.undefined; expect(result.source).to.equal('config'); expect(result.category).to.be.null; expect(result.topic).to.be.null; @@ -1105,6 +1140,80 @@ describe('prompts-storage', () => { expect(result.prompts[0].source).to.equal('gsc'); }); + // origin-dimension.md §3: like `source`, `origin` is immutable on an update — + // it is fixed by the writer that created the row. A match-update must preserve + // the stored value, so a controller-derived `human` (e.g. a user editing) can + // never relabel an existing `ai` prompt. + it('preserves the stored origin on an id-match update (never relabels ai -> human)', async () => { + const existing = [{ + id: 'u1', prompt_id: 'p1', text: 'Kept', regions: ['us'], status: 'active', source: 'gsc', origin: 'ai', + }]; + const insertStub = sinon.stub().returns({ + select: () => thenable({ data: [], error: null }), + }); + const updateStub = sinon.stub().returns({ eq: () => thenable({ error: null }) }); + const client = { + from: (table) => { + if (table === 'prompts') { + return { + select: () => ({ + eq: () => ({ + eq: () => ({ + ...thenable({ data: existing, error: null }), + in: () => thenable({ data: existing, error: null }), + }), + }), + }), + insert: insertStub, + update: updateStub, + }; + } + return makeChain({}); + }, + }; + // Incoming matches by prompt_id but carries the derived `human`; the stored + // `ai` must NOT be overwritten. + const result = await upsertPrompts({ + organizationId: ORG_ID, + brandUuid: BRAND_UUID, + prompts: [{ + id: 'p1', prompt: 'Kept', regions: ['us'], source: 'gsc', origin: 'human', + }], + postgrestClient: client, + }); + expect(result.updated).to.equal(1); + expect(insertStub.called).to.equal(false); + expect(updateStub.firstCall.args[0].origin).to.equal('ai'); + expect(result.prompts[0].origin).to.equal('ai'); + }); + + // New inserts DO carry the (controller-derived) origin the caller passed. + it('writes the provided origin on a fresh insert', async () => { + const insertStub = sinon.stub().returns({ + select: () => thenable({ data: [{ prompt_id: 'new-1' }], error: null }), + }); + const client = { + from: (table) => { + if (table === 'prompts') { + return { + select: () => ({ eq: () => ({ eq: () => thenable({ data: [], error: null }) }) }), + insert: insertStub, + update: () => ({ eq: () => thenable({ error: null }) }), + }; + } + return makeChain({}); + }, + }; + const result = await upsertPrompts({ + organizationId: ORG_ID, + brandUuid: BRAND_UUID, + prompts: [{ prompt: 'brand new', regions: ['us'], origin: 'ai' }], + postgrestClient: client, + }); + expect(insertStub.firstCall.args[0][0].origin).to.equal('ai'); + expect(result.prompts[0].origin).to.equal('ai'); + }); + it('keeps two new same-text/different-source prompts as separate inserts (dedup by source)', async () => { const insertStub = sinon.stub().returns({ select: () => thenable({ data: [{ prompt_id: 'a' }, { prompt_id: 'b' }], error: null }), @@ -1476,6 +1585,48 @@ describe('prompts-storage', () => { expect(result.prompts[0].source).to.equal('gsc'); }); + it('reactivating a deleted `ai` prompt preserves the stored origin, not the incoming `human` (origin-dimension.md §3 item 3)', async () => { + // The reactivation (deleted-match) branch must NOT re-derive origin: a + // deleted `ai`-authored row reactivated by a USER-principal write (which + // carries the derived `human`) must keep its stored `ai`. Re-deriving would + // silently relabel every reactivated model-written prompt as human. + const deletedRow = { + id: 'row-uuid', prompt_id: 'del-1o', text: 'Reactivate me', regions: ['us'], status: 'deleted', source: 'gsc', origin: 'ai', + }; + const existingData = { data: [deletedRow], error: null }; + const updateStub = sinon.stub().returns({ eq: () => thenable({ error: null }) }); + const client = { + from: (table) => { + if (table === 'prompts') { + return { + select: () => ({ + eq: () => ({ + eq: () => ({ + ...thenable(existingData), + in: () => thenable(existingData), + }), + }), + }), + insert: () => ({ select: () => thenable({ data: [], error: null }) }), + update: updateStub, + }; + } + return makeChain({}); + }, + }; + const result = await upsertPrompts({ + organizationId: ORG_ID, + brandUuid: BRAND_UUID, + prompts: [{ + id: 'del-1o', prompt: 'Reactivate me', regions: ['us'], source: 'semrush', origin: 'human', + }], + postgrestClient: client, + }); + expect(result.updated).to.equal(1); + expect(updateStub.firstCall.args[0].origin).to.equal('ai'); + expect(result.prompts[0].origin).to.equal('ai'); + }); + it('reactivates a deleted prompt matched by text+regions without inserting', async () => { const deletedRow = { id: 'row-uuid', prompt_id: 'del-2', text: 'Same text', regions: ['us'], status: 'deleted', source: 'gsc', @@ -2698,6 +2849,49 @@ describe('prompts-storage', () => { expect(result.intent).to.equal('transactional'); }); + // origin-dimension.md §3 item 3 / §1 item 5: `origin` is never patched on + // update — it is fixed by the writer that created the row. A body `origin` is + // ignored, so the PATCH sent to the store must NOT carry an `origin` key, and + // the stored value (here `ai`) is left untouched. + it('never patches origin on update, even when the body carries one', async () => { + const row = { + prompt_id: PROMPT_ID, + name: 'Test', + text: 'Text', + regions: [], + status: 'active', + origin: 'ai', + brands: { id: BRAND_UUID, name: 'Brand' }, + categories: null, + topics: null, + }; + const updateStub = sinon.stub().returns({ + eq: () => ({ + eq: () => ({ + eq: () => ({ + select: () => ({ maybeSingle: () => thenable({ data: row, error: null }) }), + }), + }), + }), + }); + const client = { + from: () => ({ + update: updateStub, + select: () => makeChain({ data: row, error: null }).select(), + }), + }; + const result = await updatePromptById({ + organizationId: ORG_ID, + brandUuid: BRAND_UUID, + promptId: PROMPT_ID, + // A caller tries to relabel an `ai` prompt to `human` on edit. + updates: { prompt: 'edited', origin: 'human' }, + postgrestClient: client, + }); + expect(updateStub.firstCall.args[0]).to.not.have.property('origin'); + expect(result.origin).to.equal('ai'); + }); + it('sets intent to null on update when value is empty or invalid', async () => { const row = { prompt_id: PROMPT_ID, diff --git a/test/support/serenity/handlers/prompts-subworkspace.test.js b/test/support/serenity/handlers/prompts-subworkspace.test.js index 01c4d591a..0f570c545 100644 --- a/test/support/serenity/handlers/prompts-subworkspace.test.js +++ b/test/support/serenity/handlers/prompts-subworkspace.test.js @@ -154,7 +154,9 @@ describe('prompts-subworkspace handlers', () => { }, log); expect(result.created).to.have.length(1); expect(result.created[0]).to.include({ semrushPromptId: 'new-prompt', geoTargetId: 2840 }); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['p'], ['tag-1']); + // A create is a user-authenticated write: the derived `origin` (`human`) is + // stamped alongside the caller's tags (origin-dimension.md §3). + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['p'], ['tag-1', TAG_IDS.originHuman]); expect(transport.publishProject).to.have.been.calledOnceWith(WS, 'p-us-en'); }); @@ -210,13 +212,13 @@ describe('prompts-subworkspace handlers', () => { }], }, log, classifyByBrandMention); expect(result.created[0].tagIds).to.deep.equal([ - TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, + TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originHuman, ]); expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly( WS, 'p-us-en', ['is Acme good?'], - [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded], + [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originHuman], ); }); diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index 8a21d6003..3803c89ad 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -20,7 +20,7 @@ import { handleCreatePrompts, handleUpdatePrompt, handleBulkDeletePrompts, - makeTypeInjector, + makePromptTagInjector, } from '../../../../src/support/serenity/handlers/prompts.js'; import { ErrorWithStatusCode } from '../../../../src/support/utils.js'; import { SerenityTransportError } from '../../../../src/support/serenity/rest-transport.js'; @@ -522,6 +522,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().rejects(new Error('opaque failure')), publishProject: sinon.stub().resolves(), }; @@ -553,6 +554,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'hello' }], existing_count: 0, }), @@ -565,15 +567,18 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }], }, fakeLog()); + // A create is a user-authenticated write: the derived `origin` (`human`) is + // stamped alongside the caller's tags (origin-dimension.md §3). No classifier + // is supplied here, so `type` is left untouched. expect(result.created).to.have.lengthOf(1); expect(result.created[0]).to.deep.equal({ semrushPromptId: 'new-sem-id', geoTargetId: 2840, languageCode: 'en', text: 'hello', - tagIds: ['tag-cat-1', 'tag-child-1'], + tagIds: ['tag-cat-1', 'tag-child-1', TAG_IDS.originHuman], }); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['tag-cat-1', 'tag-child-1']); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['tag-cat-1', 'tag-child-1', TAG_IDS.originHuman]); expect(transport.publishProject).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en'); }); @@ -643,6 +648,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 0, items: [], existing_count: 1, }), @@ -656,8 +662,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }, fakeLog()); expect(result.created[0].semrushPromptId).to.equal(''); - expect(result.created[0].tagIds).to.deep.equal(['keep']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep']); + expect(result.created[0].tagIds).to.deep.equal(['keep', TAG_IDS.originHuman]); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep', TAG_IDS.originHuman]); }); it('returns empty semrushPromptId (not the string "undefined") when createPromptsByIds returns an item with no id', async () => { @@ -666,6 +672,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ name: 'hello' }], }), @@ -687,6 +694,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'hello' }], }), @@ -703,8 +711,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }], }, fakeLog()); - expect(result.created[0].tagIds).to.deep.equal(['keep']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep']); + expect(result.created[0].tagIds).to.deep.equal(['keep', TAG_IDS.originHuman]); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep', TAG_IDS.originHuman]); }); it('caps a bulk-create tagIds array at MAX_TAG_IDS (50), mirroring the list-read query cap', async () => { @@ -713,6 +721,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'hello' }], }), @@ -726,8 +735,11 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }], }, fakeLog()); - expect(result.created[0].tagIds).to.have.lengthOf(50); - expect(result.created[0].tagIds).to.deep.equal(tooMany.slice(0, 50)); + // The MAX_TAG_IDS cap bounds the CALLER's tags (50); the server-derived + // `origin` is injected on top, so the stored set is the 50 capped tags plus + // the derived origin id. + expect(result.created[0].tagIds).to.have.lengthOf(51); + expect(result.created[0].tagIds).to.deep.equal([...tooMany.slice(0, 50), TAG_IDS.originHuman]); }); it('skips a create row when tagIds sanitizes to empty (every entry malformed)', async () => { @@ -753,6 +765,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([usEn]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'good' }], }), @@ -790,6 +803,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { const dataAccess = makeDataAccess([project]); const err = Object.assign(new Error('rate limited'), { status: 429 }); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().rejects(err), publishProject: sinon.stub().resolves(), }; @@ -822,6 +836,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'ok' }], }), @@ -1693,7 +1708,10 @@ describe('handlers/prompts.js — tag cache invalidation (Important #6)', () => handleListTags, transport, dataAccess, } = setupCtx; - // Mutation: handleCreatePrompts pushes a new prompt → invalidate. + // Mutation: handleCreatePrompts pushes a new prompt → invalidate. The create + // path resolves the tag tree to stamp the derived `origin`, so the tree read + // must be stubbed for the create to succeed and reach the invalidation. + transport.listProjectTags = makeListProjectTagsStub(); transport.createPromptsByIds = sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'sem-new', name: 'fresh' }], }); @@ -1830,13 +1848,13 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }, fakeLog(), classifyByBrandMention); expect(result.created[0].tagIds).to.deep.equal([ - TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, + TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originHuman, ]); expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly( WORKSPACE, 'proj-us-en', ['is Acme good?'], - [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded], + [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originHuman], ); // The whole taxonomy already exists, so nothing is provisioned. expect(transport.createProjectTags).to.not.have.been.called; @@ -1862,7 +1880,7 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }, fakeLog(), classifyByBrandMention); expect(result.created[0].tagIds).to.deep.equal([ - TAG_IDS.categoryRunningShoes, TAG_IDS.typeNonBranded, + TAG_IDS.categoryRunningShoes, TAG_IDS.typeNonBranded, TAG_IDS.originHuman, ]); }); @@ -1900,7 +1918,9 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }], }, fakeLog(), classifyByBrandMention); - expect(result.created[0].tagIds).to.deep.equal([decoyCategoryId, TAG_IDS.typeBranded]); + expect(result.created[0].tagIds).to.deep.equal([ + decoyCategoryId, TAG_IDS.typeBranded, TAG_IDS.originHuman, + ]); }); // Projects that predate the taxonomy carry none of it; the first write @@ -1923,15 +1943,20 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }], }, fakeLog(), classifyByBrandMention); - // The four roots are created at the root level, then `branded` beneath - // the freshly-minted `type` root. + // The four roots are created at the root level, then `branded` beneath the + // freshly-minted `type` root, then `human` beneath the `origin` root — the + // create path stamps the derived origin as well as the computed type. expect(createProjectTags.firstCall.args[2]).to.deep.equal([ 'category', 'intent', 'origin', 'type', ]); expect(createProjectTags.firstCall.args[3]).to.deep.equal({}); expect(createProjectTags.secondCall.args[2]).to.deep.equal(['branded']); expect(createProjectTags.secondCall.args[3]).to.deep.equal({ parentId: 'created::type' }); - expect(result.created[0].tagIds).to.deep.equal(['tag-cat-1', 'created:created::type:branded']); + expect(createProjectTags.thirdCall.args[2]).to.deep.equal(['human']); + expect(createProjectTags.thirdCall.args[3]).to.deep.equal({ parentId: 'created::origin' }); + expect(result.created[0].tagIds).to.deep.equal([ + 'tag-cat-1', 'created:created::type:branded', 'created:created::origin:human', + ]); }); }); @@ -1993,13 +2018,13 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }); }); - describe('makeTypeInjector cache (serenity-docs#31)', () => { + describe('makePromptTagInjector type cache (serenity-docs#31)', () => { it('resolves each (project, type) once across a batch, re-resolving only on a new key', async () => { const transport = { listProjectTags: makeListProjectTagsStub(), createProjectTags: sinon.stub(), }; - const inject = makeTypeInjector(transport, WORKSPACE, classifyByBrandMention, fakeLog()); + const inject = makePromptTagInjector(transport, WORKSPACE, classifyByBrandMention, fakeLog()); // Resolving one (project, type) key reads two levels: the roots, then the // children of the `type` root. @@ -2021,7 +2046,7 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) it('passes the input through untouched when no classifier is supplied', async () => { const transport = { listProjectTags: sinon.stub(), createProjectTags: sinon.stub() }; - const inject = makeTypeInjector(transport, WORKSPACE, undefined, fakeLog()); + const inject = makePromptTagInjector(transport, WORKSPACE, undefined, fakeLog()); const out = await inject('proj-1', { text: 'anything', geoTargetId: 2840, tagIds: ['x'] }); @@ -2030,3 +2055,164 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }); }); }); + +// origin-dimension.md §3 (LLMO-6275): `origin` is derived from the write path, +// never asserted by the user, and carries a create/update asymmetry — CREATE +// injects the derived value; UPDATE never re-derives, preserving the stored one. +// These are spec gates 5 (create arm), 7 and 8 on the Serenity tag path. +describe('handlers/prompts.js — origin derivation (origin-dimension.md §3)', () => { + const project = () => makeProject({ + semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', + }); + + // Gate 5 (create arm): a user-authenticated Serenity create derives origin = + // `human`, stripping any caller-supplied tag id beneath the origin root and + // injecting the resolved `human` id — a user may not assert who authored a prompt. + it('create strips a caller-supplied origin tag id and injects the derived human origin', async () => { + const dataAccess = makeDataAccess([project()]); + const transport = { + listProjectTags: makeListProjectTagsStub(), + createPromptsByIds: sinon.stub().resolves({ + page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'best shoes' }], + }), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { + prompts: [{ + text: 'best shoes', + geoTargetId: 2840, + languageCode: 'en', + // The caller tries to assert `ai`; the server ignores it and stamps `human`. + tagIds: [TAG_IDS.categoryRunningShoes, TAG_IDS.originAi], + }], + }, fakeLog(), classifyByBrandMention); + + expect(result.created[0].tagIds).to.deep.equal([ + TAG_IDS.categoryRunningShoes, TAG_IDS.typeNonBranded, TAG_IDS.originHuman, + ]); + expect(result.created[0].tagIds).to.not.include(TAG_IDS.originAi); + }); + + // Gate 8: the strip is BY RESOLVED ROOT ID, never by name. A customer category + // legitimately named `ai` (a category-root descendant) must survive a create + // that also carries an origin-root id — a name-based strip would delete it. + it('leaves a customer category named "ai" alone while stripping the real origin value', async () => { + const dataAccess = makeDataAccess([project()]); + const decoyAiCategoryId = 'category-ai-decoy'; + const levels = dimensionTreeLevels(); + levels[TAG_IDS.categoryRoot] = [ + ...levels[TAG_IDS.categoryRoot], + { + id: decoyAiCategoryId, + name: 'ai', + parent_id: TAG_IDS.categoryRoot, + children_count: 0, + path: [{ id: TAG_IDS.categoryRoot, name: 'category' }], + }, + ]; + const transport = { + listProjectTags: makeListProjectTagsStub(levels), + createPromptsByIds: sinon.stub().resolves({ + page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'is Acme good?' }], + }), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { + prompts: [{ + text: 'is Acme good?', + geoTargetId: 2840, + languageCode: 'en', + tagIds: [decoyAiCategoryId, TAG_IDS.originAi], + }], + }, fakeLog(), classifyByBrandMention); + + // The `ai` CATEGORY survives; only the origin-root id is stripped, and the + // derived origin is injected. + expect(result.created[0].tagIds).to.deep.equal([ + decoyAiCategoryId, TAG_IDS.typeBranded, TAG_IDS.originHuman, + ]); + }); + + // Gate 7: editing a prompt must not relabel it. The stored origin the caller + // echoes back (`ai`) rides through the replace-mode tag write untouched — it is + // never re-derived to `human`, and `origin/human` is never injected on update. + it('update preserves the stored ai origin and never re-derives it', async () => { + const dataAccess = makeDataAccess([]); + dataAccess.BrandSemrushProject.findBySlice.resolves(project()); + const transport = { + listProjectTags: makeListProjectTagsStub(), + renamePrompt: sinon.stub().resolves({ id: 'ai-prompt', name: 'now mentions Acme', is_updated: true }), + updatePromptTagsByIds: sinon.stub().resolves(null), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleUpdatePrompt(transport, dataAccess, BRAND, WORKSPACE, 'ai-prompt', { + text: 'now mentions Acme', + geoTargetId: 2840, + languageCode: 'en', + tagIds: [TAG_IDS.categoryRunningShoes, TAG_IDS.originAi], + }, fakeLog(), classifyByBrandMention); + + expect(result.status).to.equal(200); + expect(result.body.tagIds).to.deep.equal([ + TAG_IDS.categoryRunningShoes, TAG_IDS.originAi, TAG_IDS.typeBranded, + ]); + expect(result.body.tagIds).to.not.include(TAG_IDS.originHuman); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ + id: 'ai-prompt', + references: [TAG_IDS.categoryRunningShoes, TAG_IDS.originAi, TAG_IDS.typeBranded], + replace: true, + }], + ); + }); + + describe('makePromptTagInjector origin asymmetry', () => { + it('create (originValue set): strips caller origin ids and appends the derived origin', async () => { + const transport = { listProjectTags: makeListProjectTagsStub() }; + const inject = makePromptTagInjector(transport, WORKSPACE, undefined, fakeLog(), { + originValue: 'human', + }); + + const out = await inject('proj-1', { + text: 'x', geoTargetId: 2840, tagIds: [TAG_IDS.categoryRunningShoes, TAG_IDS.originAi], + }); + + expect(out.tagIds).to.deep.equal([TAG_IDS.categoryRunningShoes, TAG_IDS.originHuman]); + }); + + it('update (no originValue): leaves origin ids untouched and reads nothing', async () => { + const transport = { listProjectTags: makeListProjectTagsStub() }; + const inject = makePromptTagInjector(transport, WORKSPACE, undefined, fakeLog()); + + const out = await inject('proj-1', { + text: 'x', geoTargetId: 2840, tagIds: [TAG_IDS.categoryRunningShoes, TAG_IDS.originAi], + }); + + expect(out.tagIds).to.deep.equal([TAG_IDS.categoryRunningShoes, TAG_IDS.originAi]); + expect(transport.listProjectTags).to.not.have.been.called; + }); + + it('memoizes origin resolution per project across a batch', async () => { + const transport = { + listProjectTags: makeListProjectTagsStub(), + createProjectTags: sinon.stub(), + }; + const inject = makePromptTagInjector(transport, WORKSPACE, undefined, fakeLog(), { + originValue: 'human', + }); + + await inject('proj-1', { text: 'a', geoTargetId: 2840, tagIds: ['x'] }); + const readsAfterFirst = transport.listProjectTags.callCount; + await inject('proj-1', { text: 'b', geoTargetId: 2840, tagIds: ['y'] }); + + // Same project => served from the per-project origin cache, no new reads. + expect(transport.listProjectTags.callCount).to.equal(readsAfterFirst); + expect(transport.createProjectTags).to.not.have.been.called; + }); + }); +}); diff --git a/test/support/serenity/tag-tree.test.js b/test/support/serenity/tag-tree.test.js index b39ed26e0..3ca43b296 100644 --- a/test/support/serenity/tag-tree.test.js +++ b/test/support/serenity/tag-tree.test.js @@ -22,9 +22,11 @@ import { provisionDimensionTree, ensureClosedValue, resolveTypeValueInjection, + resolveClosedValueInjection, findTagsInTree, assertParentWithinDimension, } from '../../../src/support/serenity/tag-tree.js'; +import { DIMENSION } from '../../../src/support/serenity/prompt-tags.js'; import { TAG_IDS, dimensionTreeLevels, @@ -487,6 +489,45 @@ describe('serenity tag-tree', () => { }); }); + // FIX (MysticatBot nit): direct coverage of the generalized resolver. It was + // previously exercised only indirectly through the `resolveTypeValueInjection` + // wrapper and `makePromptTagInjector`; these tests hit it straight, for the + // `origin` dimension. NOTE: the actual gate-8 strip (dropping a caller-supplied + // id that collides by NAME but not by root) lives in `makePromptTagInjector`, + // not here — this resolver only returns the strip SET (`valueTagIds`), scoped to + // the dimension root. So these tests pin that the returned set is root-scoped; + // the strip behaviour itself is covered by the injector tests in prompts.test.js. + describe('resolveClosedValueInjection', () => { + it('resolves an `origin` value id plus EVERY id under the origin root (strip set)', async () => { + const transport = { + listProjectTags: makeListProjectTagsStub(), + createProjectTags: sinon.stub(), + }; + const res = await resolveClosedValueInjection(transport, WS, PROJECT, DIMENSION.ORIGIN, 'human', fakeLog()); + expect(res.computedId).to.equal(TAG_IDS.originHuman); + // Strip set is every id under the ORIGIN root — the two closed values only. + expect(res.valueTagIds).to.have.members([TAG_IDS.originAi, TAG_IDS.originHuman]); + // REGRESSION GUARD (not active filter validation): a customer sub-category + // also named `human` (subCategoryHuman) lives under the CATEGORY root. The + // resolver only reads the ORIGIN root's children, so this id is excluded BY + // CONSTRUCTION rather than by any filter in the SUT — the assertion locks in + // that the strip set stays root-scoped (never widens to a name match) should + // the resolution ever change to read more of the tree. + expect(res.valueTagIds).to.not.include(TAG_IDS.subCategoryHuman); + expect(transport.createProjectTags).to.not.have.been.called; + }); + + it('resolves the `ai` origin value id', async () => { + const transport = { + listProjectTags: makeListProjectTagsStub(), + createProjectTags: sinon.stub(), + }; + const res = await resolveClosedValueInjection(transport, WS, PROJECT, DIMENSION.ORIGIN, 'ai', fakeLog()); + expect(res.computedId).to.equal(TAG_IDS.originAi); + expect(res.valueTagIds).to.have.members([TAG_IDS.originAi, TAG_IDS.originHuman]); + }); + }); + describe('resolveTypeValueInjection', () => { it('returns the wanted value id plus EVERY id under the type root', async () => { const transport = {