feat(serenity): stamp prompt authorship metadata + DTO + sort/order (LLMO-6289)

[aliciadriani]

⛔ DO NOT MERGE YET — merge window: after WP2 released.
(Serenity dimension-root program. See LLMO-6289 + serenity-fleet-handover/launch-checklist.md.)

WP1 — stamp write paths + DTO + sort/order (LLMO-6289)

Stamps created_at/by + updated_at/by on every native prompt write, surfaces the four values on the list DTO, and adds server-side sort/order on the (now metadata-carrying) by_tags read. Built to the v3 metadata contract (which overrides the plan's *-with-metadata/PUT names).

Caller identity (correctness-critical)

• resolveCallerId(ctx) = authInfo.getProfile()?.user_id ?? sub, fallback unknown, capped at 100 chars (the upstream created_by/updated_by CHECK bound).
• Never derived from the forwarded upstream bearer (post promise-token exchange its principal can differ from the caller).
• Resolved once per request in the controller and threaded into every write handler in both prompt twins, createOnePrompt, and generateAndAttachPrompts (markets-subworkspace + brand-provisioning).

Stamping

• One shared helper: buildCreateMetadata (all four keys, created_* = updated_*) / buildUpdateMetadata (updated pair only). RFC 3339 UTC. No read-before-write; the stamp rides the same request as the mutation.
• Tag edits stay two writes: the v2 tag PUT is metadata-free; a text edit's stamp rides the combined PATCH /v3 {id} (name+metadata) that always runs in the in-place handler. No combined metadata-carrying tag write.
• Bulk deletes stamp nothing.

Transport (v3 write surface over the regenerated client)

createPromptsWithMetadata (POST /v3 prompts), patchPrompt (combined PATCH /v3 {id}), patchPromptMetadata (single) + patchPromptsMetadataBatch (batch); by_tags list forwards sort/order.

DTO + read + sort

buildPromptDto maps metadata.* → createdAt/By/updatedAt/By; sort/order allow-listed to metadata.created_at / metadata.updated_at and forwarded upstream. OpenAPI: SerenityPrompt authorship fields + 2 query params + contract test.

⚠️ Blocked on WP2 (regenerated PE client)

The v3 facade methods do not exist in the installed @adobe/spacecat-shared-project-engine-client@1.14.0; WP2 re-vendors the client to add them. The transport boundary type-checks now via a local ProjectEngineTransportWithMetadata typedef that WP2's generated types supersede on the dep bump. Merge only after WP2 is released, then bump the client dependency. it-postgres runs post-WP2-release only (it pulls the WP2 mock image by client version).

Verification (node 24)

• type-check ✅ · lint ✅ · unit tests ✅ (15362 passing; the 2 timeout flakes in unrelated suites pass in isolation) · coverage 99.91% lines / 98.94% branches (gate 90%) · docs:lint ✅
• Diff ≈1149 LOC (17 files) — under the ~2000 MysticatBot ceiling.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[MysticatBot]

Hey @aliciadriani,

Verdict: Request changes - two test gaps need closing before this ships.
Complexity: HIGH - large diff; API surface (OpenAPI + transport layer).
Changes: Stamps prompt authorship metadata (created/updated at/by) on every write path, surfaces the fields on the list DTO, and adds server-side sort/order on the by_tags read (17 files).
Note: Recommend a human read before merge - this change modifies a shared contract (OpenAPI schema). The bot review is a complement to, not a replacement for, a human read here.

Must fix before merge

1. [Important] Missing sort/order tests for subworkspace list handler - test/support/serenity/handlers/prompts-subworkspace.test.js (details inline)
2. [Important] patchPrompt stub signature mismatch in subworkspace test transport - test/support/serenity/handlers/prompts-subworkspace.test.js:49 (details inline)

Non-blocking (2): minor issues and suggestions

• nit: Tests pass undefined as callerId, so metadata carries created_by: undefined instead of the 'unknown' sentinel that would occur in real execution (where resolveCallerId ensures a floor). Consider passing a real caller id in at least one integration-level test per handler to prove the full path. - test/support/serenity/handlers/prompts.test.js
• suggestion: The WP2 merge-gate is documented only in the PR body. Consider enforcing it with a CI label check or merge-queue gate so accidental merge before the dep bump is structurally impossible across team rotation.

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 3m 10s | Cost: $8.27 | Commit: c5cdd79a29735daaed8c1c239cbf88d1815cc1d7
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

issue (blocking): handleListPromptsSubworkspace calls resolveSort(query) and threads sort/order into the transport call, but this test file has no dedicated tests exercising that forwarding path. The flat-mode twin (handleListPrompts in prompts.test.js) has tests that verify valid sort/order is forwarded and that invalid sort throws 400 before any upstream call. Since the two code paths resolve the project differently (DB lookup vs live listing), a regression in either is independently possible.

Fix: add at minimum two tests here:

1. A valid sort/order is forwarded to transport.listPromptsByTags.
2. An invalid sort throws 400 before listPromptsByTags is called.

[aliciadriani]

Fixed in 927e8f3. Added dedicated sort/order tests for handleListPromptsSubworkspace mirroring the flat-mode twin: valid sort/order forwarded to transport.listPromptsByTags, invalid sort → 400 before any upstream call, invalid order → 400 before any upstream call, and an unsorted read forwards neither. Also added a metadata→DTO mapping test for this twin.

[MysticatBot]

issue (blocking): The patchPrompt stub is defined as (ws, pid, promptId, newName) => ... but production now passes an object { name, metadata } as the 4th argument (not a string). The stub resolves { name: newName } which will be { name: { name: 'next', metadata: {...} } }. No current assertion reads this, but it is semantically wrong and will mask bugs if a future test asserts on the resolved value.

Fix: update to destructure the body:

patchPrompt: sinon.stub().callsFake(
  (ws, pid, promptId, body) => Promise.resolve(
    { id: promptId, name: body?.name ?? '', is_updated: true },
  ),
),

[aliciadriani]

Fixed in 927e8f3. The patchPrompt fake now takes the combined v3 body as its 4th arg — (ws, pid, promptId, body) => Promise.resolve({ id: promptId, name: body?.name ?? '', is_updated: true }) — matching the production signature. Added an update test that asserts the handler passes { name, metadata } (name + the stamped updated_* pair, no created_*) through, so a future assertion on the resolved value can't be masked.

[github-actions]

This PR will trigger a minor release when merged.

[aliciadriani]

Reviewed WP1 (LLMO-6289): stamp authorship metadata on every write + caller identity + DTO/sort, to the delivered v3 contract. Base=main, rebased + MERGEABLE → requesting MysticatBot. The last 35-track API build. Review-clean — no Must-Fix, no Should-Fix.

Caller identity (correctness-critical) — CONFIRMED. resolveCallerId(ctx) reads ctx.attributes.authInfo.getProfile()?.user_id ?? sub — the AUTH PROFILE, never the forwarded upstream bearer (whose principal can differ after the promise-token exchange). Missing/blank → the literal unknown; result .slice(0, CALLER_ID_MAX_LENGTH=100) so a pathological claim can't trip the upstream ≤100 CHECK. Resolved ONCE per request/batch (POST prompts once, PATCH once, the activate loop once for the whole batch — not re-resolved per market/prompt) and threaded into: both create twins, createOnePrompt, generateAndAttachPrompts, handleCreateMarketSubworkspace, and provisionBrandSubworkspace. Verified it never touches a token/bearer.

One stamping helper — CONFIRMED. buildCreateMetadata(callerId) (all four keys, created_* = updated_* = now/caller) and buildUpdateMetadata(callerId) (only the updated_* pair, so RFC-7396 merge keeps created_* with no read-before-write). RFC-3339 UTC via toISOString(). Shared by both twins + the AI-generation create — never duplicated.

Separate tag/metadata writes (v3) — CONFIRMED, no combined metadata-carrying tag write. A text edit is the combined PATCH /v3 .../{id} (patchPrompt — {name, metadata: buildUpdateMetadata}) replacing the v2 rename, so the updated_* stamp rides the SAME request as the text mutation (no read-before-write); it runs FIRST (the one op that can 404/409). The subsequent v2 batch tag PUT (updatePromptTagsByIds) carries {id, references, replace:true} only — NO metadata. So the edit's stamp is covered entirely by the combined PATCH, created_* is never sent, and there is no combined tag+metadata write anywhere. Creates stamp via createPromptsWithMetadata ({name, metadata} items) on the same write that creates the prompt.

DTO / sort / OpenAPI — CONFIRMED. buildPromptDto maps metadata.created_at/created_by/updated_at/updated_by → createdAt/createdBy/updatedAt/updatedBy, null when the item has no metadata (stable shape for the UI em-dash). resolveSort allow-lists sort to metadata.created_at/metadata.updated_at (else 400 — and it notes the value is forwarded verbatim upstream, so the allow-list is the injection guard, not just hygiene), order∈{asc,desc} default desc (else 400). sort/order are sent upstream ONLY when a sort is requested, so an unsorted read is byte-for-byte the legacy call. OpenAPI: SerenityPrompt gains the four nullable fields (createdBy/updatedBy maxLength:100), the two query params are enum-restricted, and the contract test is updated.

Rebase-conflict resolution (LLMO-6190 drift) — CONFIRMED sound. In generateAndAttachPrompts the v3 metadata create is correctly nested inside headroom.retryOnQuota(() => transport.createPromptsWithMetadata(ws, pid, items.map(name => ({name, metadata})), [...standardIds, typeId]), {callSite:'createPromptsWithMetadata'}) — the retry thunk wraps the v3 write, callerId is threaded through, buildCreateMetadata is called once per batch. In handleCreatePromptsSubworkspace, createOnePrompt(..., typed, callerId) sits inside headroom.retryOnQuota(() => ..., {callSite:'createOnePrompt'}). No WP1 substance is lost and the retry wrapper still wraps correctly; the fronting test is updated to items[0].name + the callerId arg.

Not dinged (per the merge plan): the v3 facade methods (createPromptsWithMetadata/patchPrompt/…) throw against the installed PE-client 1.14.0 — the merge is gated on the WP2 release + dep bump, and the local ProjectEngineTransportWithMetadata typedef is an explicit stopgap cast at the one transport boundary (WP2's regenerated types supersede it); it-postgres runs post-WP2-release; the draft→live publish-carries-metadata assumption is unverifiable in-fleet (WP5 G1). All intended — no finding.

Clean, well-tested (15393 passing, 99.91%/98.96% cov). Requesting MysticatBot (base=main).

[aliciadriani]

CORRECTNESS-CRITICAL, confirmed correct: resolveCallerId reads ctx.attributes.authInfo.getProfile()?.user_id ?? sub — the auth PROFILE, never the forwarded upstream bearer (whose principal can differ after the promise-token exchange). unknown fallback for a missing/blank identity, and .slice(0, 100) caps it below the upstream ≤100 CHECK so no write path can 400/roll-a-batch-back on a pathological claim. Single resolution point.

[aliciadriani]

One stamping helper, not duplicated: buildCreateMetadata sets all four keys (created_* = updated_* = now/caller), buildUpdateMetadata sets only the updated_* pair so an edit's RFC-7396 merge keeps created_* untouched with no read-before-write. RFC-3339 UTC. Shared by both twins + the AI-generation create.

[aliciadriani]

The combined v3 patchPrompt ({name, metadata: buildUpdateMetadata(callerId)}) replaces the v2 rename, so the text edit and the updated_* stamp land in ONE request — runs first (the one op that can 404/409). The subsequent v2 tag PUT (updatePromptTagsByIds) carries references only, NO metadata: the edit's stamp is fully covered here, created_* is never sent, and there is no combined tag+metadata write. Correct per the v3 contract.

[aliciadriani]

resolveSort allow-lists sort to metadata.created_at/metadata.updated_at (else 400) and order∈{asc,desc} (default desc, else 400). Since the value is forwarded verbatim upstream, the allow-list is the injection guard, not just input hygiene — good that it's a hard 400 rather than a silent drop. Returns {} when unspecified so an unsorted read is unchanged.

[aliciadriani]

resolveCallerId(ctx) resolved ONCE for the whole activate batch here and threaded into the per-market loop — correctly NOT re-resolved per market. Same once-per-request discipline as the POST/PATCH handlers. Good.

[aliciadriani]

Rebase-conflict resolution confirmed sound: WP1's v3 metadata create is nested inside the upstream headroom.retryOnQuota(() => transport.createPromptsWithMetadata(...), {callSite:'createPromptsWithMetadata'}) (the LLMO-6190 drift), callerId threaded, buildCreateMetadata called once per batch. The retry thunk wraps the v3 write correctly — no WP1 substance lost.

[aliciadriani]

The subworkspace twin: createOnePrompt(transport, ws, pid, typed, callerId) sits inside headroom.retryOnQuota(() => ..., {callSite:'createOnePrompt'}) — callerId threaded, retry wrapper intact. Matches the flat-mode twin. Rebase merge is clean.

[aliciadriani]

The ProjectEngineTransportWithMetadata typedef + the single boundary cast is the intended stopgap: the installed PE-client 1.14.0 lacks these v3 methods, so this lets the transport type-check under // @ts-check until the WP2 release + dep bump (which regenerates and supersedes this verbatim). This is the documented merge gate, not a finding — flagging only to confirm I understand it's deliberate.

[aliciadriani]

Re-check at 927e8f31 — MysticatBot's two blocking test findings + the nit are addressed, and the fixes are sound. This is a TEST-ONLY commit (two test files; no product-code change from the WP1 diff I already reviewed clean). WP1 is review-clean from the /review-pr side; type-check + build green.

1. Subworkspace sort/order tests — CONFIRMED. prompts-subworkspace.test.js gains the twin-mirrored set: a valid sort/order is forwarded to transport.listPromptsByTags (body.sort==='metadata.updated_at', body.order==='asc'); a sort outside the allow-list (text) rejects /sort must be one of/ with listPromptsByTags NOT called; an invalid order (sideways) rejects /order must be one of/, not called; an unsorted read forwards neither (body.sort/order undefined); plus a metadata→DTO mapping test proving createdAt/By/updatedAt/By map for this twin. Matches the flat-mode coverage in prompts.test.js, and the subworkspace twin resolves its project via the live listing (not the DB) so its forwarding path is independently exercised.

2. patchPrompt stub fixed — CONFIRMED (a real test-correctness bug). The makeTransport stub was (ws, pid, promptId, newName) => ({ name: newName }), which would have bound the whole combined body {name, metadata} object to newName — masking a signature mismatch. It is now (ws, pid, promptId, body) => ({ id: promptId, name: body?.name ?? '', is_updated: true }), reading body.name off the real combined v3 shape. Good catch to fix — the stub now can't hide a wrong call shape.

3. Nit / real-callerId — CONFIRMED. Two new tests use a REAL resolved id (caller-42) rather than the undefined default: create asserts createPromptsWithMetadata is called with the item carrying created_* = caller-42; update asserts the combined patchPrompt stamps updated_* = caller-42 AND that its body.metadata has NO created_at/created_by (merge-patch keeps them) AND the tag PUT's item has NO metadata property (metadata-free tag write). That directly locks the create-stamps-created / edit-stamps-updated-only / metadata-free-tag-PUT contract.

The dynamic-allocation-fronting.test.js edits are the mechanical alignment of the fronting stubs to the v3 signature (createPromptsByIds→createPromptsWithMetadata, texts[0]→items[0].name, the added undefined // callerId positional arg) — no change to the retryOnQuota wiring assertions. Full npm test 15400 passing.

Merge remains gated on the WP2 release + PE-client dep bump (intended). From /review-pr this is clean; re-triggering MysticatBot for a fresh-head pass (its recorded CHANGES_REQUESTED is against the pre-fix commit).

[MysticatBot]

Hey @aliciadriani,

Verdict: Approve - prior findings addressed, no new blocking issues found.
Complexity: HIGH - large diff; API surface (OpenAPI + transport layer).
Changes: Stamps prompt authorship metadata (created/updated at/by) on every write path, surfaces the fields on the list DTO, and adds server-side sort/order on the by_tags read (17 files).
Note: Recommend a human read before merge - this change modifies a shared contract (OpenAPI schema). The bot review is a complement to, not a replacement for, a human read here.

Non-blocking (3): minor issues and suggestions

• nit: resolveSort returns {} which destructures as sort: undefined, order: undefined in the handler object literals. The transport's if (body?.sort) guard catches it, but ...(sort && { sort, order }) at the handler call sites would make the "byte-for-byte legacy call" claim precise at every boundary, not just the transport. - src/support/serenity/handlers/prompts-subworkspace.js:104
• nit: handleCreatePromptsSubworkspace calls buildCreateMetadata(callerId) per item (inside createOnePrompt), so items in one batch get slightly different timestamps, while generateAndAttachPrompts shares one metadata object across the group. Both are defensible (actual creation instant vs batch instant), but the inconsistency is worth a one-line comment explaining the intentional difference. - src/support/serenity/handlers/prompts-subworkspace.js:195
• suggestion: The unknown sentinel as callerId fallback is by-design per the spec, but consider documenting explicitly (in the ADR or a code comment at the controller boundary) whether unknown-attributed writes are acceptable policy or whether they should eventually be rejected, so future readers know the fallback is intentional, not overlooked. - src/support/serenity/handlers/prompts.js:82

Previously flagged, now resolved

• Sort/order tests for subworkspace list handler now added (commit 927e8f3)
• patchPrompt stub signature updated to match production's combined v3 body shape (commit 927e8f3)

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 30s | Cost: $8.69 | Commit: 927e8f315f9a4c9ce35fa65d3e7c85fe9485008b
If this code review was useful, please react with 👍. Otherwise, react with 👎.}