diff --git a/docs/openapi/schemas.yaml b/docs/openapi/schemas.yaml index 4b17f06e6..e3d785c87 100644 --- a/docs/openapi/schemas.yaml +++ b/docs/openapi/schemas.yaml @@ -12188,12 +12188,23 @@ SuggestionReview: tier: type: string enum: [paid, free] + feedbackSubjectId: + type: [string, 'null'] + description: >- + SpaceCat-internal id of the sub-item this review is about within the + suggestion (e.g. a CWV issue id), used to group per-issue feedback. Null + for whole-suggestion reviews. previousFix: type: object - description: Patch snapshot at review time (only when ?include=...,patches). + description: AI-generated patch snapshot at review time (only when ?include=...,patches). editedFix: type: object description: ESE-edited patch (only when ?include=...,patches). + guidanceMarkdown: + type: [string, 'null'] + description: >- + AI-generated issue context (title + description) for the reviewed issue + (only when ?include=...,patches). Null when not set. required: - eventId - eventTime diff --git a/docs/openapi/site-opportunities.yaml b/docs/openapi/site-opportunities.yaml index 7d88f2572..68c6d0d94 100644 --- a/docs/openapi/site-opportunities.yaml +++ b/docs/openapi/site-opportunities.yaml @@ -1264,6 +1264,21 @@ site-opportunity-suggestion-backoffice-reviews: type: string maxLength: 8192 description: Optional ESE rationale (markdown). Required on reject by the UI; sanitised + 8 KB cap server-side. + guidanceMarkdown: + type: string + maxLength: 65536 + description: >- + AI-generated issue context (title + description) for the reviewed issue. + Gives the Learning Agent "what the issue was" alongside the generated + patch (previousFix) and the review. Sanitised + 64 KB cap server-side. + feedbackSubjectId: + type: string + maxLength: 200 + description: >- + Optional SpaceCat-internal id of the sub-item this review is about + within the suggestion (e.g. a CWV issue id), so the Backoffice can group + per-issue "previous feedback" when issues share one suggestionId. + Independent of the Fix-entity issue id. Not exported to S3. rejectionCategory: type: string enum: [product_bug, bad_recommendation, other] diff --git a/package-lock.json b/package-lock.json index ce7cb6655..7f8d3533a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,7 +23,7 @@ "@adobe/spacecat-shared-brand-client": "1.4.0", "@adobe/spacecat-shared-cloudflare-client": "1.2.1", "@adobe/spacecat-shared-content-client": "1.8.24", - "@adobe/spacecat-shared-data-access": "4.8.0", + "@adobe/spacecat-shared-data-access": "4.9.0", "@adobe/spacecat-shared-drs-client": "1.13.0", "@adobe/spacecat-shared-gpt-client": "1.6.23", "@adobe/spacecat-shared-http-utils": "1.34.0", @@ -3811,9 +3811,9 @@ } }, "node_modules/@adobe/spacecat-shared-data-access": { - "version": "4.8.0", - "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-data-access/-/spacecat-shared-data-access-4.8.0.tgz", - "integrity": "sha512-FUUIfrnOIG36Jnm+byG91MJPioTZJGLfKxO3zODangn3Dt+ZCr+2DpppqkazcUdxj24N6EFApRSPC2NE0I1e6Q==", + "version": "4.9.0", + "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-data-access/-/spacecat-shared-data-access-4.9.0.tgz", + "integrity": "sha512-yZPVZAUsMNOCmhGPFUK5822u8UGO4VpF8TTmYoAbhlCAqJiOJmxCBY+2cDunAMS1aS1P2dhjcp3btGcJrGxh1Q==", "license": "Apache-2.0", "dependencies": { "@adobe/fetch": "^4.2.3", diff --git a/package.json b/package.json index ce40c57ff..f48d1d724 100644 --- a/package.json +++ b/package.json @@ -88,7 +88,7 @@ "@adobe/spacecat-shared-brand-client": "1.4.0", "@adobe/spacecat-shared-cloudflare-client": "1.2.1", "@adobe/spacecat-shared-content-client": "1.8.24", - "@adobe/spacecat-shared-data-access": "4.8.0", + "@adobe/spacecat-shared-data-access": "4.9.0", "@adobe/spacecat-shared-drs-client": "1.13.0", "@adobe/spacecat-shared-gpt-client": "1.6.23", "@adobe/spacecat-shared-http-utils": "1.34.0", diff --git a/src/controllers/suggestions.js b/src/controllers/suggestions.js index df30c8faf..78576511d 100644 --- a/src/controllers/suggestions.js +++ b/src/controllers/suggestions.js @@ -2872,8 +2872,8 @@ function SuggestionsController(ctx, sqs, env) { const body = isNonEmptyObject(context.data) ? context.data : {}; const { - eventId, verdict, detailMarkdown, rejectionCategory, - stateTransition, previousFix, editedFix, + eventId, verdict, detailMarkdown, guidanceMarkdown, rejectionCategory, + stateTransition, previousFix, editedFix, feedbackSubjectId, } = body; // FR-09: event_id is MANDATORY and client-supplied (no server fallback). @@ -2902,6 +2902,24 @@ function SuggestionsController(ctx, sqs, env) { return createResponse({ message: 'detail_markdown exceeds the 8 KB limit' }, 413); } } + // guidance_markdown is the AI-generated issue context (title + description). + // Larger cap than detail_markdown (64 KB) because issue descriptions + + // implementation guidance run long. + if (guidanceMarkdown != null) { + if (typeof guidanceMarkdown !== 'string') { + return badRequest('guidance_markdown must be a string'); + } + if (Buffer.byteLength(guidanceMarkdown, 'utf8') > 65536) { + return createResponse({ message: 'guidance_markdown exceeds the 64 KB limit' }, 413); + } + } + // feedback_subject_id is an opaque grouping id (e.g. a CWV issue id) — a short + // string, not free text. Bounded to guard against abuse. + if (feedbackSubjectId != null) { + if (typeof feedbackSubjectId !== 'string' || feedbackSubjectId.length > 200) { + return badRequest('feedback_subject_id must be a string of at most 200 characters'); + } + } const site = await Site.findById(siteId); if (!site) { @@ -2939,10 +2957,13 @@ function SuggestionsController(ctx, sqs, env) { const { detailMarkdown: cleanMarkdown, + guidanceMarkdown: cleanGuidance, previousFix: cleanPreviousFix, editedFix: cleanEditedFix, scrubHits, - } = redactFeedbackContent({ detailMarkdown, previousFix, editedFix }); + } = redactFeedbackContent({ + detailMarkdown, guidanceMarkdown, previousFix, editedFix, + }); const scrubbed = Object.entries(scrubHits); if (scrubbed.length > 0) { @@ -2959,6 +2980,8 @@ function SuggestionsController(ctx, sqs, env) { signal, reviewer_id: reviewerId, detail_markdown: cleanMarkdown ?? null, + guidance_markdown: cleanGuidance ?? null, + feedback_subject_id: feedbackSubjectId ?? null, previous_fix: cleanPreviousFix ?? null, edited_fix: cleanEditedFix ?? null, state_transition: hasText(stateTransition) ? stateTransition : null, diff --git a/src/support/feedback-redaction.js b/src/support/feedback-redaction.js index 5ba45a08d..f79e9a399 100644 --- a/src/support/feedback-redaction.js +++ b/src/support/feedback-redaction.js @@ -129,12 +129,16 @@ export function scrubDeep(value, hits) { * * @param {object} input * @param {string} [input.detailMarkdown] + * @param {string} [input.guidanceMarkdown] * @param {*} [input.previousFix] * @param {*} [input.editedFix] - * @returns {{ detailMarkdown: (string|undefined), previousFix: *, editedFix: *, + * @returns {{ detailMarkdown: (string|undefined), + * guidanceMarkdown: (string|undefined), previousFix: *, editedFix: *, * scrubHits: Record }} */ -export function redactFeedbackContent({ detailMarkdown, previousFix, editedFix } = {}) { +export function redactFeedbackContent({ + detailMarkdown, guidanceMarkdown, previousFix, editedFix, +} = {}) { const scrubHits = {}; let cleanMarkdown = detailMarkdown; @@ -142,8 +146,17 @@ export function redactFeedbackContent({ detailMarkdown, previousFix, editedFix } cleanMarkdown = scrubString(sanitizeMarkdown(detailMarkdown), scrubHits); } + // guidance_markdown is AI-generated issue context, but we secret-scrub it too + // (defence in depth — it can quote customer HTML/config). Sanitised like the + // reviewer's rationale. + let cleanGuidance = guidanceMarkdown; + if (typeof guidanceMarkdown === 'string') { + cleanGuidance = scrubString(sanitizeMarkdown(guidanceMarkdown), scrubHits); + } + return { detailMarkdown: cleanMarkdown, + guidanceMarkdown: cleanGuidance, previousFix: previousFix === undefined ? undefined : scrubDeep(previousFix, scrubHits), editedFix: editedFix === undefined ? undefined : scrubDeep(editedFix, scrubHits), scrubHits, diff --git a/test/controllers/suggestions-reviews.test.js b/test/controllers/suggestions-reviews.test.js index 3f026d420..7a2ca10a5 100644 --- a/test/controllers/suggestions-reviews.test.js +++ b/test/controllers/suggestions-reviews.test.js @@ -249,6 +249,58 @@ describe('Suggestions Controller - backoffice reviews', () => { expect(response.status).to.equal(413); }); + it('captures guidance_markdown (issue context) alongside the generated patch', async () => { + const context = makeContext({ + postgrest: { onInsert: () => ({ data: { event_id: EVENT_ID, signal: 'negative', tier: 'free' }, error: null }) }, + }); + context.data.guidanceMarkdown = '# Slow hero image\n\nLCP element is a 1.2 MB PNG.'; + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(201); + const inserted = context.dataAccess.services.postgrestClient.capturedInserts[0]; + expect(inserted.guidance_markdown).to.equal('# Slow hero image\n\nLCP element is a 1.2 MB PNG.'); + // the generated patch is stored too (the LA maps the review to it) + expect(inserted.previous_fix).to.deep.equal({ patch: 'before' }); + }); + + it('rejects oversize guidance_markdown with 413 (64 KB cap)', async () => { + const context = makeContext(); + context.data.guidanceMarkdown = 'x'.repeat(65537); + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(413); + }); + + it('rejects a non-string guidance_markdown with 400', async () => { + const context = makeContext(); + context.data.guidanceMarkdown = { not: 'a string' }; + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(400); + }); + + it('stores feedback_subject_id (per-issue grouping key) on the row', async () => { + const context = makeContext({ + postgrest: { onInsert: () => ({ data: { event_id: EVENT_ID, signal: 'negative', tier: 'free' }, error: null }) }, + }); + context.data.feedbackSubjectId = 'issue-42'; + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(201); + const inserted = context.dataAccess.services.postgrestClient.capturedInserts[0]; + expect(inserted.feedback_subject_id).to.equal('issue-42'); + }); + + it('rejects a non-string feedback_subject_id with 400', async () => { + const context = makeContext(); + context.data.feedbackSubjectId = { not: 'a string' }; + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(400); + }); + + it('rejects an over-long feedback_subject_id with 400', async () => { + const context = makeContext(); + context.data.feedbackSubjectId = 'x'.repeat(201); + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(400); + }); + it('returns 404 when the site is not found', async () => { mockDataAccess.Site.findById.resolves(null); const response = await controller.createBackofficeReview(makeContext()); diff --git a/test/support/feedback-redaction.test.js b/test/support/feedback-redaction.test.js index 9f1b566f9..838f0547c 100644 --- a/test/support/feedback-redaction.test.js +++ b/test/support/feedback-redaction.test.js @@ -129,12 +129,16 @@ describe('feedback-redaction', () => { it('sanitises + scrubs markdown and scrubs both patches, aggregating hits', () => { const result = redactFeedbackContent({ detailMarkdown: ' key AKIAABCDEFGHIJKLMNOP me@adobe.com', + guidanceMarkdown: ' issue at foo.corp.adobe.com', previousFix: { code: 'Bearer abcdefghijklmnop' }, editedFix: { code: `ghp_${'z'.repeat(25)}` }, }); expect(result.detailMarkdown).to.not.contain('