diff --git a/src/support/serenity/handlers/markets-subworkspace.js b/src/support/serenity/handlers/markets-subworkspace.js index 7ba4b111c..dc93522df 100644 --- a/src/support/serenity/handlers/markets-subworkspace.js +++ b/src/support/serenity/handlers/markets-subworkspace.js @@ -224,7 +224,7 @@ function validateCreateBody(body) { async function generateAndAttachPrompts(transport, workspaceId, projectId, { domain, country, topicCap = 0, brandNames = [], provisioned, env, writeDeadline = computeWriteDeadline(), -}, log) { +}, log, headroom) { const raw = await transport.getBrandTopics(workspaceId, { domain, country }); let topics = []; if (Array.isArray(raw)) { @@ -313,6 +313,13 @@ async function generateAndAttachPrompts(transport, workspaceId, projectId, { } } + // PROMPT metering seam (Rainer, live-verified LLMO-6190): front headroom sized + // on the real generated prompt count (`texts.size`) BEFORE the metered + // `createPromptsByIds` writes below — the disguised-quota 405 fires there, not + // at publish. No-op when the flag is OFF; NOT optional-chained, a caller that + // forgets to thread the guard must fail loud. + await headroom.ensure({ prompts: texts.size }, { includeDrafted: true }); + for (const { items, tagIds } of byTagSet.values()) { // eslint-disable-next-line no-await-in-loop await transport.createPromptsByIds(workspaceId, projectId, items, tagIds); diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index 839c5a5c3..a350ec6e6 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -242,20 +242,6 @@ export async function handleCreatePromptsSubworkspace( invalidateTagCacheForProject(workspaceId, pid); } - // PROMPT metering seam: the just-created prompts are drafted synchronously across the affected - // projects of THIS child; size headroom from `used + drafted` (includeDrafted, staleness-immune) - // before the publish. One workspace-level top-up covers all affected projects (the allocation is - // per sub-workspace, not per project). No-op when the flag is OFF; skipped when nothing was - // created so the OFF path and the empty path issue zero headroom reads. - if (affectedProjectIds.length > 0) { - const headroom = createHeadroomGuard( - transport, - { enabled: dynamicAllocation, subWorkspaceId: workspaceId, parentWorkspaceId }, - log, - ); - await headroom.ensure({}, { includeDrafted: true }); - } - if (deferPublish) { log?.info?.('serenity create-prompts (subworkspace): deferPublish set — prompts written as draft, publish skipped', { workspaceId, created: created.length, skipped: skipped.length, failed: failed.length, @@ -265,7 +251,16 @@ export async function handleCreatePromptsSubworkspace( }; } - const publishErrors = await publishAffected(transport, workspaceId, affectedProjectIds, log); + // Route each project's publish through the headroom guard's retryOnQuota (LLMO-6190 item 4): + // a disguised metered-405 gets ONE bounded top-up+retry per project before being recorded as a + // failure. No-op passthrough when the flag is OFF (the guard's retryOnQuota is a plain call). + const publishErrors = await publishAffected( + transport, + workspaceId, + affectedProjectIds, + log, + (fn) => headroom.retryOnQuota(fn), + ); // publishAffected returns { projectId, message } records whose message is // ALREADY redacted (redactUpstreamMessage) — pubErr is a record, not a raw error. for (const pubErr of publishErrors) { diff --git a/src/support/serenity/handlers/prompts.js b/src/support/serenity/handlers/prompts.js index 284d6ed69..c1cbc3b0f 100644 --- a/src/support/serenity/handlers/prompts.js +++ b/src/support/serenity/handlers/prompts.js @@ -533,7 +533,18 @@ export function parseUpdatePromptBody(body) { }, }; } - const text = String(body.text); + // Mirror the create contract (`normalizePromptInput`): empty or whitespace-only + // text is rejected here rather than passed on to `renamePrompt`, where it would + // be classified and written as a blank prompt. `|| ''` also coerces a falsy + // non-string (`null`, `0`, `false`) to empty, matching create exactly. + const text = String(body.text || '').trim(); + if (!text) { + return { + ok: false, + status: 400, + body: { error: 'invalidRequest', message: 'text must be a non-empty string' }, + }; + } const tagIds = sanitizeTagIds(body.tagIds); if (tagIds.length === 0) { return { @@ -819,10 +830,19 @@ export async function handleUpdatePrompt( } const projectId = project.getSemrushProjectId(); - // Recompute the type AND intent tags from the NEW text BEFORE the delete: the - // unified layer (tree read / on-demand tag create / LLM classify) must not run - // between delete and create, so a classification failure aborts cleanly with + // Recompute the type AND intent tags from the NEW text BEFORE the rename: the + // unified layer (tree read / on-demand tag create / LLM classify) must run + // before any upstream write, so a classification failure aborts cleanly with // the old prompt still present (serenity-docs#31, #32). + // + // This runs UNCONDITIONALLY, even when the PATCH does not change the text. The + // upstream provider has no GET-by-id and the handler is not sent the old text + // (the body is the full next state — see the docblock above), so it cannot + // know whether the text actually changed. `renamePrompt`'s `is_updated: false` + // reports a no-op only AFTER the rename, too late to gate a classify that has + // to run first for failure-safety. Skipping the reclassification would require + // the client to send the old text — a contract change deliberately out of + // scope here (keep the edit path a single straight line). const injectComputedType = makeTypeInjector( transport, semrushWorkspaceId, diff --git a/test/support/serenity/dynamic-allocation-fronting.test.js b/test/support/serenity/dynamic-allocation-fronting.test.js index 2dd8e95fe..38dd001b4 100644 --- a/test/support/serenity/dynamic-allocation-fronting.test.js +++ b/test/support/serenity/dynamic-allocation-fronting.test.js @@ -546,6 +546,8 @@ describe('dynamic-allocation fronting — retryOnQuota wiring', () => { }, log, undefined, + undefined, + undefined, { dynamicAllocation: true, parentWorkspaceId: MASTER }, ); // The retry succeeded, so the create is NOT recorded as a publish failure. diff --git a/test/support/serenity/handlers/prompts-subworkspace.test.js b/test/support/serenity/handlers/prompts-subworkspace.test.js index 4ea861799..d19e2642b 100644 --- a/test/support/serenity/handlers/prompts-subworkspace.test.js +++ b/test/support/serenity/handlers/prompts-subworkspace.test.js @@ -202,7 +202,9 @@ describe('prompts-subworkspace handlers', () => { prompts: [{ text: 'p', tagIds: ['tag-1'], geoTargetId: 2840, languageCode: 'en', }], - }, log, undefined, { dynamicAllocation: true, parentWorkspaceId: 'parent-ws' }); + }, log, undefined, undefined, undefined, { + dynamicAllocation: true, parentWorkspaceId: 'parent-ws', + }); expect(result.created).to.have.length(1); expect(transport.getWorkspaceResources).to.have.been.calledOnceWith(WS); expect(transport.getWorkspaceResources) @@ -339,7 +341,7 @@ describe('prompts-subworkspace handlers', () => { expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( WS, 'p-us-en', - [{ id: 'old-id', references: ['tag-1'], replace: true }], + [{ id: 'old-id', references: ['tag-1', TAG_IDS.intentInformational], replace: true }], ); expect(transport.deletePromptsByIds).to.not.have.been.called; expect(transport.createPromptsByIds).to.not.have.been.called; @@ -398,10 +400,15 @@ describe('prompts-subworkspace handlers', () => { text: 'new', tagIds: ['tag-cat-1', '', undefined], geoTargetId: 2840, languageCode: 'en', }, log); expect(result.status).to.equal(200); - expect(result.body.semrushPromptId).to.equal('new-prompt-by-id'); + // The id is preserved — the edit is in place, never a re-create. + expect(result.body.semrushPromptId).to.equal('old-id'); expect(result.body.tagIds).to.deep.equal(['tag-cat-1', TAG_IDS.intentInformational]); - expect(transport.deletePromptsByIds).to.have.been.calledWith(WS, 'p-us-en', ['old-id']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['new'], ['tag-cat-1', TAG_IDS.intentInformational]); + expect(transport.renamePrompt).to.have.been.calledOnceWithExactly(WS, 'p-us-en', 'old-id', 'new'); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WS, + 'p-us-en', + [{ id: 'old-id', references: ['tag-cat-1', TAG_IDS.intentInformational], replace: true }], + ); }); it('400s when the slice key is invalid', async () => { @@ -436,8 +443,15 @@ describe('prompts-subworkspace handlers', () => { expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( WS, 'p-us-en', - ['now mentions Acme'], - [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.intentInformational], + [{ + id: 'old-id', + references: [ + TAG_IDS.categoryRunningShoes, + TAG_IDS.typeBranded, + TAG_IDS.intentInformational, + ], + replace: true, + }], ); }); diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index e08421742..959b250f2 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -1004,6 +1004,49 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { expect(result.body.error).to.equal('invalidRequest'); }); + // Mirror of the create-path contract (normalizePromptInput trims + rejects + // empty): an edit must not slip an empty prompt past validation into the + // rename, where it would be classified and written blank. + it('400s when text is an empty string', async () => { + const dataAccess = makeDataAccess([]); + + const result = await handleUpdatePrompt( + {}, + dataAccess, + BRAND, + WORKSPACE, + 'sem-1', + { + geoTargetId: 2840, languageCode: 'en', text: '', tagIds: ['tag-1'], + }, + fakeLog(), + ); + + expect(result.status).to.equal(400); + expect(result.body.error).to.equal('invalidRequest'); + expect(result.body.message).to.match(/non-empty/); + }); + + it('400s when text is whitespace-only', async () => { + const dataAccess = makeDataAccess([]); + + const result = await handleUpdatePrompt( + {}, + dataAccess, + BRAND, + WORKSPACE, + 'sem-1', + { + geoTargetId: 2840, languageCode: 'en', text: ' ', tagIds: ['tag-1'], + }, + fakeLog(), + ); + + expect(result.status).to.equal(400); + expect(result.body.error).to.equal('invalidRequest'); + expect(result.body.message).to.match(/non-empty/); + }); + it('edits text+tagIds in place (rename + replace-mode tag write), id unchanged', async () => { const project = makeProject({ semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', @@ -1013,10 +1056,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().resolves({ - page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'next' }], existing_count: 0, - }), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'next', is_updated: true }), + updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), }; @@ -1041,8 +1082,53 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { text: 'next', tagIds: ['tag-cat-1', TAG_IDS.intentInformational], }); - expect(transport.deletePromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['sem-1']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['tag-cat-1', TAG_IDS.intentInformational]); + expect(transport.renamePrompt).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', 'sem-1', 'next'); + // Replace-mode tag write with the injector's full output (caller tag + intent). + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ id: 'sem-1', references: ['tag-cat-1', TAG_IDS.intentInformational], replace: true }], + ); + }); + + // Guards the documented always-reclassify invariant: an unchanged-text edit + // comes back from rename as `is_updated: false`, but the handler still writes + // the (re-classified) tag set and publishes — it has no old text and upstream + // has no GET-by-id, so it cannot short-circuit. A future "optimization" that + // skips the tag write on is_updated:false would break this and fail here. + it('still writes tags + publishes when rename reports is_updated:false (no-op text)', async () => { + const project = makeProject({ + semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', + }); + const dataAccess = makeDataAccess([]); + dataAccess.BrandSemrushProject.findBySlice.resolves(project); + + const transport = { + listProjectTags: makeListProjectTagsStub(), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'same', is_updated: false }), + updatePromptTagsByIds: sinon.stub().resolves(null), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleUpdatePrompt( + transport, + dataAccess, + BRAND, + WORKSPACE, + 'sem-1', + { + geoTargetId: 2840, languageCode: 'en', text: 'same', tagIds: ['tag-cat-1'], + }, + fakeLog(), + ); + + expect(result.status).to.equal(200); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ id: 'sem-1', references: ['tag-cat-1', TAG_IDS.intentInformational], replace: true }], + ); + expect(transport.publishProject).to.have.been.called; }); it('drops falsy tagIds entries on PATCH before the tag write', async () => { @@ -1054,10 +1140,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().resolves({ - page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'next' }], existing_count: 0, - }), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'next', is_updated: true }), + updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), }; @@ -1074,7 +1158,11 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { ); expect(result.body.tagIds).to.deep.equal(['keep', TAG_IDS.intentInformational]); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['keep', TAG_IDS.intentInformational]); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ id: 'sem-1', references: ['keep', TAG_IDS.intentInformational], replace: true }], + ); }); it('drops malformed tagIds entries on PATCH like validateParentIdFormat does for parentId', async () => { @@ -1086,10 +1174,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().resolves({ - page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'next' }], - }), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'next', is_updated: true }), + updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), }; const tooLong = 'x'.repeat(201); @@ -1110,7 +1196,11 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { ); expect(result.body.tagIds).to.deep.equal(['keep', TAG_IDS.intentInformational]); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['keep', TAG_IDS.intentInformational]); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ id: 'sem-1', references: ['keep', TAG_IDS.intentInformational], replace: true }], + ); }); it('400s when tagIds sanitizes to empty (every entry malformed)', async () => { @@ -1220,14 +1310,14 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const dataAccess = makeDataAccess([]); dataAccess.BrandSemrushProject.findBySlice.resolves(project); - // The prompt-gone 404 is gated by isUpstreamGone which requires - // SerenityTransportError specifically. A generic Error with .status=404 - // must NOT trip the promptNotFound path. + // The prompt-gone 404 is gated by isUpstreamGone, which requires a + // SerenityTransportError. A rename that reports the prompt is gone maps to + // promptNotFound and never reaches the tag write. const err = new SerenityTransportError(404, 'not found'); const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().rejects(err), - createPromptsByIds: sinon.stub(), + renamePrompt: sinon.stub().rejects(err), + updatePromptTagsByIds: sinon.stub(), }; const result = await handleUpdatePrompt( @@ -1247,6 +1337,37 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { expect(transport.updatePromptTagsByIds).to.have.callCount(0); }); + // The negative of the case above: isUpstreamGone requires a + // SerenityTransportError, so a generic Error carrying .status=404 must NOT be + // treated as promptNotFound — it propagates like any other upstream failure. + it('generic Error with status 404 → throws (not promptNotFound)', async () => { + const project = makeProject({ + semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', + }); + const dataAccess = makeDataAccess([]); + dataAccess.BrandSemrushProject.findBySlice.resolves(project); + + const err = Object.assign(new Error('plain 404'), { status: 404 }); + const transport = { + listProjectTags: makeListProjectTagsStub(), + renamePrompt: sinon.stub().rejects(err), + updatePromptTagsByIds: sinon.stub(), + }; + + await expect(handleUpdatePrompt( + transport, + dataAccess, + BRAND, + WORKSPACE, + 'sem-1', + { + geoTargetId: 2840, languageCode: 'en', text: 'x', tagIds: ['tag-1'], + }, + fakeLog(), + )).to.be.rejectedWith(/plain 404/); + expect(transport.updatePromptTagsByIds).to.have.callCount(0); + }); + // The collision contract (serenity-docs#63 decision 2): a rename onto a // sibling prompt's exact text is refused upstream with 409 and NOTHING has // mutated — the handler propagates it untouched so the controller's mapError @@ -1261,8 +1382,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const err = new SerenityTransportError(409, 'conflict'); const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().resolves({}), // no items + renamePrompt: sinon.stub().rejects(err), + updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), }; @@ -1291,8 +1412,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const err = Object.assign(new Error('upstream 503'), { status: 503 }); const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().rejects(err), - createPromptsByIds: sinon.stub().resolves({ items: [{ id: 'should-not-happen' }] }), + renamePrompt: sinon.stub().rejects(err), + updatePromptTagsByIds: sinon.stub().resolves(null), }; await expect(handleUpdatePrompt( @@ -1322,8 +1443,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const tagErr = Object.assign(new Error('tag write boom'), { status: 500 }); const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().rejects(createErr), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'x', is_updated: true }), + updatePromptTagsByIds: sinon.stub().rejects(tagErr), publishProject: sinon.stub().resolves(), }; @@ -1989,7 +2110,11 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) 'proj-us-en', [{ id: 'old-id', - references: [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded], + references: [ + TAG_IDS.categoryRunningShoes, + TAG_IDS.typeBranded, + TAG_IDS.intentInformational, + ], replace: true, }], );