feat(serenity): derive + inject prompt origin server-side — WP-O2b (LLMO-6275)#2866
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
59cf1ae to
4d66cd6
Compare
|
This PR will trigger a minor release when merged. |
Review note — principal breadth of the
|
|
🔎 LLMO-6275 → In Review. Code is complete and this PR is up for review. It remains a draft and must not merge until its base, WP-O2a #2862 (LLMO-6274), lands — at which point GitHub retargets this PR to |
…e (LLMO-6275) WP-O2b of the origin rename (serenity-docs#46), stacked on WP-O2a (claude/wp-o2a-origin, #2862). Generalizes the prompt-tag injector and the v2 prompts write surface so `origin` is derived from the write path, never asserted by a user, with the create/update asymmetry origin-dimension.md §3 requires. Serenity tag path (handlers/prompts.js + prompts-subworkspace.js): - makeTypeInjector -> makePromptTagInjector: on CREATE strips any caller-supplied tag id beneath the origin root (by resolved id, never by name) and injects the derived `human`; on UPDATE leaves origin untouched -- the stored value the caller echoes rides through, never re-derived (gates 7, 8; create arm of 5). - resolveClosedValueInjection generalizes resolveTypeValueInjection. v2 prompts API (controllers/brands.js + prompts-storage.js): - createPromptsByBrand derives origin from the principal: IMS/JWT user -> `human` (body ignored, never rejected); service principal (admin x-api-key, e.g. DRS) -> validated body value (gates 5, 6 -- the DRS origin:'ai' contract). - upsertPrompts preserves the stored origin on a match-update; updatePromptById no longer patches origin; mapRowToPrompt returns row.origin verbatim. - OpenAPI: origin documented as service-principal-only / read-only for users and not patchable on update; the origin query filter + sort enum stay. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
4d66cd6 to
b6f5fe2
Compare
|
♻️ Rebased onto |
aliciadriani
left a comment
aliciadriani
left a comment
There was a problem hiding this comment.
Reviewed WP-O2b (LLMO-6275), the CANONICAL O2b on claude/wp-o2b-derivation, base=main (O2a merged). The salvage #2864 is closed/superseded. This is the version #2867 (WP-S2) already built cleanly on, and it carries the gate-5/6 controller coverage #2864 lacked. Requesting MysticatBot (base=main). DO-NOT-MERGE banner expected.
All lens invariants CONFIRMED:
Create-inject / update-preserve asymmetry — CONFIRMED. makePromptTagInjector(transport, ws, classify, log, { originValue }): CREATE passes { originValue: ORIGIN_VALUE.HUMAN } (handleCreatePrompts + subworkspace twin), so the origin block strips any caller origin id and injects the derived value; UPDATE passes NO options, so the if (originValue) block is skipped entirely — origin is neither stripped nor re-injected, and the stored origin tag the caller round-trips rides through the replace-mode write untouched. Re-deriving would relabel an edited ai prompt human; this path never re-derives. (type is still recomputed from text on every write, as before.)
Strip by resolved tag id, never by name — CONFIRMED. Both the type and origin strips are tagIds.filter((id) => !valueTagIds.includes(id)) where valueTagIds is every id under the resolved root (via resolveClosedValueInjection), so a customer category legitimately named ai/branded is not under a server root and is left untouched.
mapRowToPrompt returns row.origin — CONFIRMED (no || 'human'): origin is NOT NULL in prod, and a fallback would silently mislabel a model-written prompt were a NULL ever inserted. upsertPrompts preserves the stored origin on both reactivate and update (match.origin ?? row.origin), so a user-principal derive of human can never relabel an existing ai prompt on a later write; updatePromptById patches origin never (fixed at creation).
origin REMAINS on V2PromptInput — CONFIRMED (schemas.yaml): kept with enum [ai, human], default human, plus a service-principal-only description — NOT deleted. This is the DRS origin:'ai' guard: deleting the field would strip the service principal's ability to assert authorship. (source is also still present here, as expected pre-S2.)
Gate 5/6 controller coverage — CONFIRMED present in test/controllers/brands.test.js (the gap I flagged on #2864). Four controller-level createPromptsByBrand cases exercise the real context.attributes.authInfo.getType() → isUserPrincipal mapping, not just the storage unit: a user principal (ims) with body origin:'ai' stores human (gate 5, 201 — body IGNORED, never rejected); a service principal (apikey / legacyApiKey) with origin:'ai' stores ai (gate 6, 201 — honoured). deriveV2PromptOrigin is also unit-tested directly in prompts-storage.test.js. The isUserPrincipal = authType === 'jwt' || authType === 'ims' derivation (service = everything else) matches these cases and the DRS admin-x-api-key contract.
CI note: build / type-check / codecov / lint / protect-nyc-config / validate-pr-title are green. it-postgres is red — the it/-gated integration tests that wait on the WP-S1 mock release, not a finding per the lens. branch-deploy / deploy-stage / semantic-release show red at ~0s: these are the deploy/release-pipeline jobs (main-only, secret-gated), not the unit build — expected on a draft PR, not a code-quality signal. Flagging for visibility; not blocking review.
No Must-Fix, no Should-Fix — review-clean on correctness. This resolves the gate-5/6 coverage gap I raised on #2864. Requesting MysticatBot below (base=main).
|
|
||
| // origin — CREATE only. `originValue` unset means UPDATE: leave origin alone | ||
| // (the stored value the caller echoes rides through the replace-mode write). | ||
| if (originValue) { |
There was a problem hiding this comment.
The origin block is CREATE-only, keyed on originValue being set — so UPDATE (which passes no options) skips it entirely and leaves the stored origin tag the caller echoes untouched. The strip is by resolved id (valueTagIds), never by name. This is the create-inject/update-preserve asymmetry the WP requires, achieved by leave-alone-on-update rather than an active re-inject — simpler and correct: it never re-derives on edit.
| // value on insert; on update the stored origin is preserved (upsertPrompts) | ||
| // and never patched (updatePromptById). | ||
| const authType = context.attributes?.authInfo?.getType?.(); | ||
| const isUserPrincipal = authType === 'jwt' || authType === 'ims'; |
There was a problem hiding this comment.
isUserPrincipal = authType === 'jwt' || authType === 'ims' (service principal = any other auth type, e.g. DRS admin x-api-key) is the crux of gate 5/6, and it reads the auth type from the same per-request context as updatedBy. Confirmed this exact mapping is exercised end-to-end by the four brands.test.js controller cases (ims→human, apikey/legacyApiKey→honoured). Good — this is the coverage #2864 was missing.
| * @param {boolean} isUserPrincipal - true for an IMS/JWT user request. | ||
| * @returns {string} the origin to store (`ai` or `human`). | ||
| */ | ||
| export function deriveV2PromptOrigin(bodyOrigin, isUserPrincipal) { |
There was a problem hiding this comment.
deriveV2PromptOrigin: user principal → always human (body ignored, never rejected); service principal → body value honoured, validated against V2_PROMPT_ORIGINS, defaulting to human only when absent/out-of-vocabulary. Correct DRS contract — an out-of-vocab service value coerces rather than 400s, and a user's assertion is silently overridden. Directly unit-tested in prompts-storage.test.js.
| // NOT NULL in production (zero NULLs in 265,980 rows, origin-dimension.md | ||
| // §2.3), and a fallback would silently mislabel a model-written prompt as | ||
| // human-authored were a NULL ever inserted. | ||
| origin: row.origin, |
There was a problem hiding this comment.
origin: row.origin with no || 'human' fallback — correct: the column is NOT NULL in prod, and a fallback would mislabel a model-written row as human were a NULL ever present. Paired with the match.origin ?? row.origin preserve on update (line 921) so a later user-principal write can never relabel a stored ai.
| ...row, | ||
| id: match.id, | ||
| source: match.source ?? source, | ||
| origin: match.origin ?? row.origin, |
There was a problem hiding this comment.
Update-path preserve: origin: match.origin ?? row.origin keeps the stored origin across a reactivation/update — origin is fixed by the writer that created the row and never re-derived. The ?? row.origin is a defensive in-memory/test fallback, not a backfill (prod has zero NULL origins). Consistent with updatePromptById not patching origin at all.
| enum: [ai, human] | ||
| default: human | ||
| description: >- | ||
| Who authored the prompt's text. SERVICE-PRINCIPAL-ONLY and read-only for |
There was a problem hiding this comment.
origin retained on V2PromptInput (enum [ai,human], default human) with the service-principal-only description — NOT removed. This is the DRS guard: the service principal must still be able to assert origin:'ai', which deriveV2PromptOrigin honours. Correct per the lens.
| // 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 () => { |
There was a problem hiding this comment.
Gate 6 controller case: a service principal (legacyApiKey) with origin:'ai' → stored ai, 201. Together with the gate-5 case (line 1136, ims → coerced human, 201, body not rejected) and the two DRS/user cases above, this is the controller-boundary coverage of the s2sConsumer/auth-type → derive mapping that #2864 lacked. Confirmed all assert 201 (never a rejection) and the stored origin.
There was a problem hiding this comment.
Hey @aliciadriani,
Verdict: Request changes - three defense-in-depth and coverage gaps to address.
Complexity: HIGH - large diff; API surface + auth-context derivation.
Changes: Generalizes makeTypeInjector to makePromptTagInjector stamping both type and origin, derives origin from the request principal, and removes origin patchability on update (11 files).
Note: Recommend a human read before merge - this change modifies a shared contract (OpenAPI schema files). The bot review is a complement to, not a replacement for, a human read here.
Note: CI it-postgres is failing (expected per PR body - blocked on WP-O1 mock release).
Must fix before merge
- [Important] Auth type defaults to service-principal when
authInfois absent -src/controllers/brands.js:571(details inline) - [Important]
mapRowToPromptfallback removal can return null/undefined origin to API consumers -src/support/prompts-storage.js:452(details inline) - [Important] Missing test for origin preservation on the reactivation (soft-delete) upsert path -
test/support/prompts-storage.test.js(details inline)
Non-blocking (4): minor issues and suggestions
- nit: MAX_TAG_IDS cap (50) is applied before server tag injection, so a prompt can carry 51 tags (50 user + 1 origin). Confirm upstream accepts this or document that server-derived tags are exempt from the user-facing cap -
src/support/serenity/handlers/prompts.js - nit: Controller gate tests in
brands.test.jsrebuild identicalthenable/insertStub/fromfixtures four times - a shared helper would reduce copy-paste drift - suggestion: Consider deprecation-marking
resolveTypeValueInjection(JSDoc@deprecated) so new callers go directly toresolveClosedValueInjection-src/support/serenity/tag-tree.js - suggestion: Add a one-line comment in
makePromptTagInjectornoting the origin cache assumes a singleoriginValueper instance (it keys by project only, not by value) -src/support/serenity/handlers/prompts.js
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 8s | Cost: $7.84 | Commit: b6f5fe2a8840cd44fcb15b70dc7fd31dbc2d7b38
If this code review was useful, please react with 👍. Otherwise, react with 👎.
| // 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.
issue (blocking): getType?.() optional chaining means that if authInfo is absent or getType is not a function, authType evaluates to undefined, making isUserPrincipal = false. This treats the request as a service principal, granting it the ability to assert arbitrary origin values.
The auth middleware (authWrapper) currently prevents unauthenticated requests from reaching this handler, so this is not exploitable today. However, the fail-open stance is a defense-in-depth concern: any future code path that calls this controller without the auth wrapper (e.g. an internal queue consumer, a new middleware ordering) would silently grant service-principal privilege.
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 authType is falsy, making the security boundary self-documenting and fail-closed regardless of upstream wrapper behavior.
| // NOT NULL in production (zero NULLs in 265,980 rows, origin-dimension.md | ||
| // §2.3), and a fallback would silently mislabel a model-written prompt as | ||
| // human-authored were a NULL ever inserted. | ||
| origin: row.origin, |
There was a problem hiding this comment.
issue (blocking): Removing row.origin || 'human' in favor of row.origin means that if the DB ever returns a row with NULL origin (race with a migration, test DB, staging env with stale data), the API response will surface null/undefined to consumers. The OpenAPI schema documents origin with enum: [ai, human] and default: human - consumers may depend on this always being a non-null string.
The PR correctly notes zero NULLs in 265,980 production rows. However, the other fields in the same function (source: row.source || 'config', status: row.status || 'active') retain defensive fallbacks for the same reason.
Consider using nullish coalescing instead: origin: row.origin ?? 'human'. This preserves the "no silent mislabeling" goal (since 'ai' is truthy and passes through both || and ??) while honoring the response schema contract on edge-case NULLs. Alternatively, add a NOT NULL database constraint (if not already present) to make the invariant enforceable at the storage layer.
| @@ -1105,6 +1140,80 @@ describe('prompts-storage', () => { | |||
| expect(result.prompts[0].source).to.equal('gsc'); | |||
There was a problem hiding this comment.
issue (blocking): upsertPrompts has three match-update paths: (a) active match, (b) deleted match (reactivation), and (c) fresh insert. The PR adds origin: match.origin ?? row.origin to both (a) and (b), and tests path (a) ("preserves the stored origin on an id-match update") and path (c) ("writes the provided origin on a fresh insert"). But there is no test for path (b) - the reactivation of a deleted row with a conflicting origin.
This is analogous to the existing test "reactivating a deleted row by prompt_id keeps the stored source". Add a test that reactivates a deleted ai-origin prompt via a user-principal write (which would carry origin: 'human') and asserts the stored 'ai' is preserved. A regression in the reactivation path would silently relabel reactivated prompts.
MysticatBot
added
ai-reviewed
Must-fix: - brands.js: derive the write principal by POSITIVE identification — an absent or unknown auth type now falls to the USER branch (origin `human`, body ignored), never the service branch. Closes the hole where an unattributable request could assert its own origin. - prompts-storage.js mapRowToPrompt: coalesce a missing `origin` to `human` on READ so the non-nullable V2Prompt contract never sees null/undefined. Correctness stays on the write path (origin derived from the principal); this is only the contract default for a prod-nonexistent NULL column. - test: add origin-preservation coverage for the reactivation (soft-delete) upsert path, and an absent-principal derivation test (must-fix #1). Non-blocking: note that server-derived tags are injected on top of the MAX_TAG_IDS user cap; document the per-project origin-cache single-value assumption; mark resolveTypeValueInjection @deprecated in favour of resolveClosedValueInjection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks @MysticatBot — addressed the review in Must-fix
Non-blocking
Verification: |
There was a problem hiding this comment.
Hey @aliciadriani,
Verdict: Approve - prior findings addressed cleanly; no remaining blockers.
Complexity: HIGH - large diff; API surface + auth-context derivation.
Changes: Addresses prior review findings - auth derivation now fail-safe to user principal, mapRowToPrompt deliberate no-fallback documented, reactivation origin test added (11 files).
Note: Recommend a human read before merge - this change modifies a shared contract (OpenAPI schema files). The bot review is a complement to, not a replacement for, a human read here.
Note: CI it-postgres is failing (expected per PR body - blocked on WP-O1 mock release).
Previously flagged, now resolved
- Auth type defaults fixed: absent/indeterminate auth now falls to user principal (fail-safe, least privilege)
mapRowToPromptfallback removal rationale documented: fail-loud over fabricatedhumanfor correctness-critical signal- Reactivation origin test added: deleted
aiprompt reactivated by user write retains storedai
Non-blocking (3): minor issues and suggestions
- suggestion: Add
originto therequiredarray in theV2Promptresponse schema (OpenAPI) so SDK-generated types reflect the non-nullable contract -docs/openapi/schemas.yaml - nit: No controller-level test for the unattributable auth case (
attributes: {}orauthInfowithoutgetType) - the!authTypefail-safe branch is only tested indirectly viaderiveV2PromptOriginunit tests -test/controllers/brands.test.js - nit:
resolveClosedValueInjectionhas no direct unit test; exercised only indirectly through the wrapper andmakePromptTagInjectorintegration tests -src/support/serenity/tag-tree.js
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 3s | Cost: $7.06 | Commit: 103b413fafd669ae926c7461d5af96a6e0f76e74
If this code review was useful, please react with 👍. Otherwise, react with 👎.
… (LLMO-6275) WP-O2b now stamps the derived `origin` tag (`human`) on a user-authenticated create alongside the computed `type` tag, so the id-based create integration test carries 4 tags (2 supplied + type + origin), not 3. Update the length assertion and the comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
Clears the three non-blocking items from the MysticatBot approval review: - suggestion: mark `origin` required (non-nullable) on the V2Prompt response schema so SDK-generated types reflect the NOT-NULL contract. - nit: add direct controller coverage of the auth fail-safe branch (`!authType`/no-getType → user principal) — a body-asserted `origin: 'ai'` from an unattributable caller is coerced to `human`. - nit: add direct unit tests for `resolveClosedValueInjection` (the generalized resolver, previously covered only via the `resolveTypeValueInjection` wrapper), including the gate-8 strip-by-root-id collision case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aliciadriani
left a comment
aliciadriani
left a comment
There was a problem hiding this comment.
Addressed the three non-blocking items from your approval review in 19652ee6. Inline comments below mark each fix on its line.
| # 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 |
There was a problem hiding this comment.
Fix (suggestion): origin now required on the V2Prompt response schema. origin 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 now reflect it as always-present rather than optional.
| // 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. | ||
| [ |
There was a problem hiding this comment.
Fix (nit): direct controller coverage of the auth fail-safe branch. Parameterized over both unattributable cases — attributes: {} (no authInfo) and an authInfo without getType — asserting a body-asserted origin: 'ai' is coerced to human (!authType → user, brands.js:584). Previously exercised only indirectly via the deriveV2PromptOrigin unit tests.
| // wrapper and `makePromptTagInjector`; these tests hit it straight, for the | ||
| // `origin` dimension, and pin gate 8 (strip-by-root-id, never by name). | ||
| describe('resolveClosedValueInjection', () => { | ||
| it('resolves an `origin` value id plus EVERY id under the origin root (strip set)', async () => { |
There was a problem hiding this comment.
Fix (nit): direct unit tests for resolveClosedValueInjection. The generalized resolver was previously covered only through the resolveTypeValueInjection wrapper; these hit it straight for the origin dimension and pin the gate-8 collision (a customer sub-category named human under the category root is NOT in the origin strip set).
GitHub Actions
Inactive
|
@MysticatBot re-review please — addressed the three non-blocking items from your approval in
Inline comments mark each fix on its line. |
There was a problem hiding this comment.
Hey @aliciadriani,
Verdict: Approve - prior non-blocking suggestions addressed cleanly; no remaining blockers.
Complexity: HIGH - large diff; API surface + auth-context derivation.
Changes: Addresses all three non-blocking suggestions from the prior approval - adds origin to V2Prompt required, adds controller-level fail-safe auth test, adds direct resolveClosedValueInjection unit tests (4 files).
Note: Recommend a human read before merge - this change modifies a shared contract (OpenAPI schema files). The bot review is a complement to, not a replacement for, a human read here.
Note: CI it-postgres is failing (expected per PR body - blocked on WP-O1 mock release).
Previously flagged, now resolved
originadded torequiredarray in V2Prompt response schema (OpenAPI) - SDK-generated types now reflect the non-nullable contract- Controller-level test for unattributable auth case added - parameterized coverage of fail-safe branch (absent authInfo, non-function getType)
resolveClosedValueInjectiondirect unit tests added - exercises both origin values and pins gate-8 cross-root isolation
Non-blocking (1): minor issues and suggestions
- nit: The gate-8 assertion (
valueTagIdsdoes not includesubCategoryHuman) passes by construction of the stub (the origin resolution never reads category-root children), not by exercising filtering logic in the SUT. Consider a brief comment noting it is a regression guard rather than active validation -test/support/serenity/tag-tree.test.js:508
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 1s | Cost: $3.78 | Commit: 19652ee62a1bcb1ac3d3e9072918d0f130f857bf
If this code review was useful, please react with 👍. Otherwise, react with 👎.
…6275) Addresses MysticatBot: the `valueTagIds` does-not-include `subCategoryHuman` assertion passes BY CONSTRUCTION — `resolveClosedValueInjection` only reads the ORIGIN root's children, so a same-named category id is never in the returned set. The actual strip-by-id lives in `makePromptTagInjector` (covered by the injector tests). Comment now states this is a root-scoping regression guard, not active filter validation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aliciadriani
left a comment
aliciadriani
left a comment
There was a problem hiding this comment.
@MysticatBot good catch — addressed in 4ec38c5e. You're right that the assertion passes by construction: resolveClosedValueInjection only reads the ORIGIN root's children, so the same-named category id is never in the returned set, and the real strip-by-id logic lives in makePromptTagInjector (covered by the injector tests in prompts.test.js). The comment now labels it a root-scoping regression guard rather than active filter validation.
| 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 |
There was a problem hiding this comment.
Fix (MysticatBot): reframed this as a REGRESSION GUARD. The comment now states the subCategoryHuman id is excluded by construction (the resolver reads only the ORIGIN root's children), not by a filter in the SUT — the assertion locks in that the strip set stays root-scoped and never widens to a name match. The actual strip behaviour is validated by the makePromptTagInjector tests.
There was a problem hiding this comment.
Hey @aliciadriani,
Verdict: Approve - prior nit addressed cleanly; no remaining findings.
Complexity: HIGH - large diff; API surface + auth-context derivation.
Changes: Adds clarifying test comments labeling the gate-8 assertion as a regression guard rather than active filter validation (1 file).
Note: Recommend a human read before merge - this change modifies a shared contract (OpenAPI schema files). The bot review is a complement to, not a replacement for, a human read here.
Note: CI it-postgres is failing (expected per PR body - blocked on WP-O1 mock release).
Previously flagged, now resolved
- Gate-8 test assertion reframed as a regression guard with explanatory comment noting root-scoped strip set lives in the resolver, active strip logic in the injector
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 0m 52s | Cost: $1.81 | Commit: 4ec38c5e1cf2eb2a8e1b910ccdb32099c0509a6f
If this code review was useful, please react with 👍. Otherwise, react with 👎.
GitHub Actions
Inactive
# Conflicts: # src/support/serenity/handlers/prompts-subworkspace.js
GitHub Actions
Inactive
# [1.674.0](v1.673.3...v1.674.0) (2026-07-21) ### Bug Fixes * **deps:** trigger prod deploy for spacecat-shared-data-access 4.11.0 (LLMO-6401) ([#2879](#2879)) ([038e4da](038e4da)) ### Features * **serenity:** derive + inject prompt origin server-side — WP-O2b (LLMO-6275) ([#2866](#2866)) ([331071f](331071f)), closes [#2825](#2825) [#2867](#2867)
|
🎉 This PR is included in version 1.674.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 participants
WP-O2b (LLMO-6275) — origin derivation + injector generalization
Canonical WP-O2b implementation (serenity dimension-root program, epic 46). Stacked on
claude/wp-o2a-origin.Generalizes
makeTypeInjector→makePromptTagInjectorto strip-and-inject bothtypeandoriginwith the spec §3 item-3 asymmetry (CREATE injects the derived value; UPDATE re-injects the prompt's STORED origin, never re-derives), strips by resolved tag id beneath the root (never by name), derivesoriginfrom the request principal (service principal → asserted body value validated against the vocabulary; user principal →human, body ignored, never rejected), stops patchingoriginon update,mapRowToPromptreturnsrow.origin(no|| 'human'fallback), and KEEPSoriginonV2PromptInput(the DRSorigin:'ai'upsert guard, item 6). Spec gates 5–8, including the gate5/6 controller coverage intest/controllers/brands.test.js.Verify: node24 type-check + lint + npm test (unit) green.
it/-postgres waits the WP-O1 mock release (not a finding).Supersedes #2864 (the earlier "salvage" implementation from #2825), which lacked the gate5/6 controller coverage. This branch is also the confirmed base for WP-S2 (#2867). History preserved in #2864.
WP-O2b — derive & inject the
origindimension server-sideThe derivation half of WP-O2 in the origin rename (serenity-docs#46). Tracks LLMO-6275 (epic LLMO-6270).
origin(who authored a prompt's text:human/ai) becomes a function of the write path, never a caller-supplied field, with the create/update asymmetry origin-dimension.md §3 requires.Serenity tag path —
handlers/prompts.js+prompts-subworkspace.jsmakeTypeInjector→makePromptTagInjector, now stamping bothtypeandorigin:originroot (by resolved id, never by name — a customer category namedaisurvives) and injects the derivedhuman.originalone — the stored value the caller echoes rides through the replace-mode write, never re-derived (re-deriving would relabel an editedaiprompthuman; stripping-without-injecting would make it invisible — both illegal).resolveClosedValueInjectiongeneralizesresolveTypeValueInjection(the origin root is resolved tolerantly, so it addresses whichever physical root —originor a legacysource— the project carries).v2 prompts API —
controllers/brands.js+prompts-storage.jscreatePromptsByBrandderivesoriginfrom the principal: an IMS/JWT user →human(body ignored, never rejected); a service principal (adminx-api-key, e.g. DRS) → validated body value — the DRSorigin:'ai'contract stays honoured.upsertPromptspreserves the storedoriginon a match-update;updatePromptByIdno longer patchesorigin;mapRowToPromptreturnsrow.originverbatim (no|| 'human').origindocumented as service-principal-only / read-only for users and not patchable on update; theoriginquery filter + sort enum stay.The user-facing elmo write surface (Source picker, CSV import) is WP-O3b, a separate WP — not here.
Gates (origin-dimension.md §7)
origin:'ai'honoured ✅aiprompt staysaiacross an edit ✅aisurvives a create carrying an origin id ✅Verification
npm testexit 0 — 15285 passing, coverage thresholds met.npm run type-checkclean;eslintclean on all touched paths;docs:lintvalid (only pre-existing warnings, incl. the unrelated/configurations/ambiguity).test/it/integration is out of scope — it needs WP-O1 (spacecat-shared#1831) released for the PE-client mock image; not run here.