-
Notifications
You must be signed in to change notification settings - Fork 16
feat(serenity): derive + inject prompt origin server-side — WP-O2b (LLMO-6275) #2866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b6f5fe2
103b413
4f0c923
19652ee
4ec38c5
de5a02b
88b6f69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| 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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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). | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (blocking): The auth middleware ( Consider defaulting to user-principal when auth type is indeterminate: const isUserPrincipal = !authType || authType === 'jwt' || authType === 'ims';Or add an explicit guard that returns an error when |
||
| // | ||
| // 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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| 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, | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (blocking): Removing The PR correctly notes zero NULLs in 265,980 production rows. However, the other fields in the same function ( Consider using nullish coalescing instead: |
||
| 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, | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update-path preserve: |
||
| }; | ||
| 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); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix (suggestion):
originnowrequiredon the V2Prompt response schema.originis NOT NULL in production andmapRowToPromptreturns it verbatim with no fallback (origin-dimension.md §2.3 / §3 item 4), so SDK-generated types now reflect it as always-present rather than optional.