diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..1ce1b6b --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,133 @@ +# Copilot PR Review Instructions + +## 1. Review Goals and Priorities + +Your primary purpose is to identify **behavior-breaking defects**, **security/auth gaps**, and **missing tests for changed behavior**. +When such issues are present, prioritize them above all other considerations (performance, cost, style). + +Use **three severities**: + +* **Critical** – Bugs, regressions, security issues, missing required validation, or missing tests for changed behavior. + *Respond with:* “This PR should not be merged until this is fixed.” +* **Major** – Missing documentation, missing but non-blocking tests, realistic performance or cost concerns. +* **Minor** – Stylistic suggestions or optional improvements. + *Only list Minor issues if no Critical issues exist.* + +If you find any Critical issue, list it first and deprioritize all other feedback. + +--- + +## 2. Output Format (Always Required) + +Respond using the following structure: + +### Summary + +1–3 sentences describing the overall health of the PR. + +### Issues + +#### Critical + +* List each issue, quoting relevant code and suggesting a concrete fix. + +#### Major + +* As above. + +#### Minor + +* As above. Only include if there are no Critical issues. + +### Suggested Tests + +* Describe which tests should be added or updated (if applicable). If no test changes are needed, state that clearly. + +--- + +## 3. Core Checks (Apply to Every PR) + +### 3.1 Bug & Regression Scan + +Look for defects including: + +* Missing or incorrect null/undefined checks. +* Incorrect async/await handling. +* Logic changes without corresponding test updates. + +**If you see changed behavior without new or updated tests, mark as Critical.** + +--- + +### 3.2 Use of shared utility functions + +Check that utility functions available here: https://github.com/adobe/spacecat-shared/blob/main/packages/spacecat-shared-utils/src/index.js are used where appropriate and instead of self-made checks. + +--- + +### 3.3 Required Tests + +For any non-trivial code change: + +* Require unit tests under `test/**` using Mocha/Chai/Sinon/esmock. +* Integration tests where relevant. +* Tests must assert behavior, not just shallow coverage. +* Fixtures and helpers must be updated consistently. + +**If behavior changes but tests do not → Critical.** + +If a PR is documentation-only or comment-only, explicitly mark tests as not required. + +--- + +## 4. Performance Scan (Secondary Priority) + +Raise **Major** issues for realistic performance risks: + +* Repeated DAO calls inside loops. +* Redundant fetches or HTTP calls. +* Blocking or synchronous operations where async or batching exists. +* Unbounded payload handling without streaming. + +Do **not** speculate without evidence. + +--- + +## 5. Cost Impact Scan (Secondary Priority) + +Flag potential cost increases only when the diff clearly adds: + +* New SQS calls, queue consumers, cron jobs. +* Large CSV/JSON generation. +* Long-running processing. +* Removal of rate limits such as `SANDBOX_AUDIT_RATE_LIMIT_HOURS`. + +Tie comments to specific code, not general assumptions. + +--- + +## 6. Config, Documentation, and Change Control + +For any new: + +* Env var +* Queue +* Feature flag +* Controller surface area + +Require updates to: + +* `README.md` + +Missing required docs → **Major**. + +--- + +## 7. Final Quality Pass + +Once all Critical and Major issues are addressed: + +* Ensure handlers, tests, routing, and docs are consistent. +* Ensure no lint rules are violated. +* Ensure logging is structured and avoids PII. +* Only then offer stylistic suggestions (Minor). \ No newline at end of file diff --git a/README.md b/README.md index 3e0f985..aeff5b4 100644 --- a/README.md +++ b/README.md @@ -107,9 +107,9 @@ $ npm run lint ## Message Body Formats -Task processor consumes the `SPACECAT-TASK-PROCESSOR-JOBS` queue, performs the requested task and sends a notification to slack as needed. +Task processor consumes the `SPACECAT-TASK-PROCESSOR-JOBS` queue, performs the requested task and sends a notification to Slack as needed. -Expected message body format in `SPACECAT-TASK-PROCESSOR-JOBS` is: +### SQS (legacy) envelope ```json { @@ -117,3 +117,35 @@ Expected message body format in `SPACECAT-TASK-PROCESSOR-JOBS` is: "siteId": "string" } ``` + +### Agent workflow (direct invoke) payload + +When the AWS Step Functions Agent Workflow invokes the Lambda directly, it sends the same top-level envelope but without SQS metadata. The payload **must** include `type: "agent-executor"` plus the following fields: + +```json +{ + "type": "agent-executor", + "agentId": "brand-profile", + "siteId": "123e4567-e89b-12d3-a456-426614174000", + "context": { + "baseURL": "https://example.com", + "params": { + "crawlDepth": 2 + } + }, + "slackContext": { + "channelId": "C123456", + "threadTs": "1731111111.000200" + }, + "idempotencyKey": "brand-profile-123e4567-e89b-12d3-a456-426614174000-1731111111000" +} +``` + +Field descriptions: +- `agentId` *(required)* – must match a registered agent (e.g., `brand-profile`). +- `siteId` *(required)* – kept at the envelope level for logging/metrics. Agents can still read it from the message passed into `agent.persist`. +- `context` *(required)* – forwarded to `agent.run`. At minimum it must include `baseURL`; additional agent-specific params live here. +- `slackContext` *(optional)* – when present, the workflow sends pre-/post-execution Slack notifications using `channelId` and `threadTs`. Provide an empty object `{}` if no Slack context is available. +- `idempotencyKey` *(required by workflow)* – generated by the caller to deduplicate executions. The task processor treats it as opaque metadata but logs it for traceability. + +Any additional properties are passed through to the agent and appear in the executor response body so the workflow can inspect them. diff --git a/src/agents/brand-profile/index.js b/src/agents/brand-profile/index.js index 8518e18..7fc33e9 100644 --- a/src/agents/brand-profile/index.js +++ b/src/agents/brand-profile/index.js @@ -11,7 +11,12 @@ */ import { Config } from '@adobe/spacecat-shared-data-access/src/models/site/config.js'; import { AzureOpenAIClient } from '@adobe/spacecat-shared-gpt-client'; -import { isNonEmptyObject, isValidUrl, isValidUUID } from '@adobe/spacecat-shared-utils'; +import { + hasText, + isNonEmptyObject, + isValidUrl, + isValidUUID, +} from '@adobe/spacecat-shared-utils'; import { readPromptFile, renderTemplate } from '../base.js'; @@ -57,21 +62,22 @@ async function persist(message, context, result) { if (!isValidUUID(siteId)) { log.warn(`brand-profile persist: invalid siteId ${siteId}`); - return; + return {}; } if (!isNonEmptyObject(result)) { log.warn(`brand-profile persist: empty result for site ${siteId}`); - return; + return {}; } const { Site } = dataAccess; const site = await Site.findById(siteId); if (!site) { log.warn(`brand-profile persist: site not found ${siteId}`); - return; + return {}; } const cfg = site.getConfig(); + const baseURL = site.getBaseURL(); const before = cfg.getBrandProfile?.() || {}; const beforeHash = before?.contentHash || null; cfg.updateBrandProfile(result); @@ -82,11 +88,11 @@ async function persist(message, context, result) { await site.save(); // Emit concise summary for observability/Slack step consumers via logs - const baseURL = message?.context?.baseURL; const version = after?.version; + const isDev = context.env.AWS_ENV === 'dev'; const summary = changed - ? `Brand profile updated to v${version} for site ${siteId}${baseURL}.` - : `Brand profile unchanged (v${version}) for site ${siteId}${baseURL}.`; + ? `:white_check_mark: Brand profile updated to v${version} for ${baseURL}.` + : `:information_source: Brand profile already up to date (v${version}) for ${baseURL}.`; log.info('brand-profile persist:', { siteId, version, @@ -95,6 +101,70 @@ async function persist(message, context, result) { baseURL, summary, }); + + const primaryVoice = Array.isArray(after?.main_profile?.tone_attributes?.primary) + ? after.main_profile.tone_attributes.primary.slice(0, 3) + : []; + const highlightLines = []; + if (primaryVoice.length > 0) { + highlightLines.push(`*Primary voice:* ${primaryVoice.join(', ')}`); + } + if (hasText(after?.main_profile?.communication_style)) { + highlightLines.push(`*Style:* ${after.main_profile.communication_style}`); + } + if (hasText(after?.main_profile?.target_audience)) { + highlightLines.push(`*Audience:* ${after.main_profile.target_audience}`); + } + const highlightText = highlightLines.join('\n'); + const blocks = [ + { + type: 'section', + text: { + type: 'mrkdwn', + text: `${summary}\n*Site:* ${baseURL}`, + }, + }, + ]; + if (hasText(highlightText)) { + blocks.push({ + type: 'section', + text: { + type: 'mrkdwn', + text: highlightText, + }, + }); + } + const contextElements = [ + { type: 'mrkdwn', text: `*Site ID:* ${siteId}` }, + ]; + if (version) { + contextElements.push({ type: 'mrkdwn', text: `*Version:* ${version}` }); + } + if (afterHash) { + contextElements.push({ type: 'mrkdwn', text: `*Hash:* \`${afterHash}\`` }); + } + contextElements.push({ + type: 'mrkdwn', + text: `https://spacecat.experiencecloud.live/api/${isDev ? 'ci' : 'v1'}/sites/${siteId}/brand-profile`, + }); + blocks.push({ + type: 'context', + elements: contextElements, + }); + + return { + siteId, + version, + changed, + contentHash: afterHash, + summary, + notifications: { + success: { + text: summary, + blocks, + }, + }, + }; } export default { diff --git a/src/tasks/agent-executor/handler.js b/src/tasks/agent-executor/handler.js index af451bd..3467d17 100644 --- a/src/tasks/agent-executor/handler.js +++ b/src/tasks/agent-executor/handler.js @@ -10,10 +10,43 @@ * governing permissions and limitations under the License. */ import { ok, badRequest } from '@adobe/spacecat-shared-http-utils'; -import { hasText, isNonEmptyObject } from '@adobe/spacecat-shared-utils'; +import { hasText, isNonEmptyArray, isNonEmptyObject } from '@adobe/spacecat-shared-utils'; import { getAgent } from '../../agents/registry.js'; +const NOTIFICATION_KEYS = ['success', 'failure']; + +function normalizeNotifications(source) { + if (!isNonEmptyObject(source)) { + return undefined; + } + + const normalized = {}; + NOTIFICATION_KEYS.forEach((key) => { + const value = source[key]; + if (!isNonEmptyObject(value)) { + return; + } + + const entry = {}; + if (hasText(value.text)) { + entry.text = value.text; + } + if (isNonEmptyArray(value.blocks)) { + entry.blocks = value.blocks; + } + if (isNonEmptyArray(value.attachments)) { + entry.attachments = value.attachments; + } + + if (Object.keys(entry).length > 0) { + normalized[key] = entry; + } + }); + + return Object.keys(normalized).length > 0 ? normalized : undefined; +} + /** * Message shape: * { @@ -41,6 +74,7 @@ export async function runAgentExecutor(message, context) { const result = await agent.run(agentContext, context.env, log); // Optionally persist if the agent supports persistence let persistMeta; + let notifications; if (typeof agent.persist === 'function') { try { persistMeta = await agent.persist(message, context, result); @@ -55,7 +89,17 @@ export async function runAgentExecutor(message, context) { result, }; if (isNonEmptyObject(persistMeta)) { - payload.persistMeta = persistMeta; + const { notifications: persistNotifications, ...rest } = persistMeta; + if (isNonEmptyObject(rest)) { + payload.persistMeta = rest; + } + const normalized = normalizeNotifications(persistNotifications); + if (normalized) { + notifications = normalized; + } + } + if (notifications) { + payload.notifications = notifications; } return ok(payload); } diff --git a/src/tasks/slack-notify/handler.js b/src/tasks/slack-notify/handler.js index 4cf0bcb..c8363f1 100644 --- a/src/tasks/slack-notify/handler.js +++ b/src/tasks/slack-notify/handler.js @@ -11,7 +11,7 @@ */ import { ok, badRequest } from '@adobe/spacecat-shared-http-utils'; import { BaseSlackClient, SLACK_TARGETS } from '@adobe/spacecat-shared-slack-client'; -import { hasText } from '@adobe/spacecat-shared-utils'; +import { hasText, isNonEmptyObject } from '@adobe/spacecat-shared-utils'; import { say } from '../../utils/slack-utils.js'; @@ -26,25 +26,46 @@ import { say } from '../../utils/slack-utils.js'; */ export async function runSlackNotify(message, context) { const slackContext = message?.slackContext; - const text = message?.text; - const blocks = message?.blocks || []; + const payload = isNonEmptyObject(message?.message) ? message.message : {}; + const hasPayloadText = typeof payload.text === 'string' && payload.text.length > 0; + const rawText = hasPayloadText ? payload.text : message?.text; + const text = hasText(rawText) ? rawText : ''; + const blocksSource = Array.isArray(payload?.blocks) && payload.blocks.length > 0 + ? payload.blocks + : message?.blocks; + const attachmentsSource = Array.isArray(payload?.attachments) && payload.attachments.length > 0 + ? payload.attachments + : message?.attachments; + const blocks = Array.isArray(blocksSource) ? blocksSource : []; + const attachments = Array.isArray(attachmentsSource) ? attachmentsSource : []; + const hasStructuredPayload = blocks.length > 0 || attachments.length > 0; if (!hasText(slackContext?.channelId)) { return badRequest('slackContext.channelId is required'); } + if (!hasStructuredPayload && !hasText(text)) { + return badRequest('text is required when no blocks or attachments are provided'); + } + // Prefer the shared say() utility for simple text notifications - if (blocks.length === 0) { + if (!hasStructuredPayload) { await say(context.env, context.log, slackContext, text); } else { // Fallback for advanced block messages const client = BaseSlackClient.createFrom(context, SLACK_TARGETS.WORKSPACE_INTERNAL); - await client.postMessage({ + const messageBody = { channel: slackContext.channelId, thread_ts: slackContext.threadTs, - text, - blocks, - }); + text: hasText(text) ? text : ' ', + }; + if (blocks.length > 0) { + messageBody.blocks = blocks; + } + if (attachments.length > 0) { + messageBody.attachments = attachments; + } + await client.postMessage(messageBody); } return ok({ status: 'sent' }); } diff --git a/test/agents/brand-profile/index.test.js b/test/agents/brand-profile/index.test.js index b0c44e4..c3652cb 100644 --- a/test/agents/brand-profile/index.test.js +++ b/test/agents/brand-profile/index.test.js @@ -191,6 +191,7 @@ describe('agents/brand-profile', () => { getConfig: () => cfg, setConfig, save, + getBaseURL: () => 'https://example.com', }); context.dataAccess.Site = { findById }; @@ -226,6 +227,7 @@ describe('agents/brand-profile', () => { getConfig: () => cfg, setConfig, save, + getBaseURL: () => 'https://example.com', }); context.dataAccess.Site = { findById }; @@ -248,7 +250,7 @@ describe('agents/brand-profile', () => { expect(save).to.have.been.calledOnce; expect(log.info).to.have.been.calledWith( 'brand-profile persist:', - sinon.match.has('summary', sinon.match('Brand profile unchanged')), + sinon.match.has('summary', sinon.match(':information_source: Brand profile already up to date (v5) for https://example.com')), ); }); @@ -263,6 +265,7 @@ describe('agents/brand-profile', () => { getConfig: () => cfg, setConfig, save, + getBaseURL: () => 'https://example.com', }); context.dataAccess.Site = { findById }; @@ -287,4 +290,50 @@ describe('agents/brand-profile', () => { sinon.match.object, ); }); + + it('persist() includes highlight blocks when main profile data is present', async () => { + let currentProfile = { version: 1, contentHash: 'old' }; + const cfg = { + getBrandProfile: () => currentProfile, + updateBrandProfile: (profile) => { + currentProfile = { ...profile, version: 2, contentHash: 'new' }; + }, + }; + const setConfig = sinon.stub(); + const save = sinon.stub().resolves(); + const findById = sandbox.stub().resolves({ + getConfig: () => cfg, + setConfig, + save, + getBaseURL: () => 'https://example.com', + }); + context.dataAccess.Site = { findById }; + + const toDynamoItem = sandbox.stub().callsFake((c) => c); + const mod = await esmock('../../../src/agents/brand-profile/index.js', { + '@adobe/spacecat-shared-data-access/src/models/site/config.js': { + Config: { toDynamoItem }, + }, + }); + + const result = await mod.default.persist( + { siteId: '123e4567-e89b-12d3-a456-426614174000' }, + context, + { + main_profile: { + tone_attributes: { primary: ['confident', 'warm', 'pragmatic', 'extra'] }, + communication_style: 'Conversational expert guidance', + target_audience: 'Digital leaders', + }, + }, + ); + + expect(result.notifications.success.blocks[1]).to.deep.equal({ + type: 'section', + text: { + type: 'mrkdwn', + text: '*Primary voice:* confident, warm, pragmatic\n*Style:* Conversational expert guidance\n*Audience:* Digital leaders', + }, + }); + }); }); diff --git a/test/tasks/agent-executor/agent-executor.test.js b/test/tasks/agent-executor/agent-executor.test.js index 6c8859b..dc66d9b 100644 --- a/test/tasks/agent-executor/agent-executor.test.js +++ b/test/tasks/agent-executor/agent-executor.test.js @@ -202,4 +202,73 @@ describe('agent-executor handler', () => { expect(body.result).to.deep.equal({ ok: true }); expect(body).to.not.have.property('persistMeta'); }); + + it('normalizes notification payloads when provided', async () => { + const runStub = sandbox.stub().resolves({ ok: true }); + const persistStub = sandbox.stub().resolves({ + stored: true, + notifications: { + success: { + text: 'Success!', + blocks: [{ type: 'section', text: { type: 'mrkdwn', text: 'Success block' } }], + }, + failure: { + text: 'Failure :(', + attachments: [{ text: 'details' }], + }, + ignored: {}, + }, + }); + const handlerModule = await esmock('../../../src/tasks/agent-executor/handler.js', { + '../../../src/agents/registry.js': { + getAgent: sandbox.stub().returns({ run: runStub, persist: persistStub }), + }, + '@adobe/spacecat-shared-utils': { + hasText: (s) => typeof s === 'string' && s.length > 0, + isNonEmptyObject: (obj) => !!obj && typeof obj === 'object' && Object.keys(obj).length > 0, + }, + }); + runAgentExecutor = handlerModule.runAgentExecutor; + + const message = { agentId: 'brand-profile', context: { baseURL: 'https://example.com' } }; + const resp = await runAgentExecutor(message, context); + const body = await resp.json(); + expect(body.notifications).to.deep.equal({ + success: { + text: 'Success!', + blocks: [{ type: 'section', text: { type: 'mrkdwn', text: 'Success block' } }], + }, + failure: { + text: 'Failure :(', + attachments: [{ text: 'details' }], + }, + }); + expect(body.persistMeta).to.deep.equal({ stored: true }); + }); + + it('omits notifications when payload is empty or invalid', async () => { + const runStub = sandbox.stub().resolves({ ok: true }); + const persistStub = sandbox.stub().resolves({ + notifications: { + success: {}, + failure: { blocks: 'invalid' }, + }, + }); + const handlerModule = await esmock('../../../src/tasks/agent-executor/handler.js', { + '../../../src/agents/registry.js': { + getAgent: sandbox.stub().returns({ run: runStub, persist: persistStub }), + }, + '@adobe/spacecat-shared-utils': { + hasText: (s) => typeof s === 'string' && s.length > 0, + isNonEmptyObject: (obj) => !!obj && typeof obj === 'object' && Object.keys(obj).length > 0, + }, + }); + runAgentExecutor = handlerModule.runAgentExecutor; + + const message = { agentId: 'brand-profile', context: { baseURL: 'https://example.com' } }; + const resp = await runAgentExecutor(message, context); + const body = await resp.json(); + expect(body.result).to.deep.equal({ ok: true }); + expect(body).to.not.have.property('notifications'); + }); }); diff --git a/test/tasks/slack-notify/slack-notify.test.js b/test/tasks/slack-notify/slack-notify.test.js index a23f9ed..a1bb321 100644 --- a/test/tasks/slack-notify/slack-notify.test.js +++ b/test/tasks/slack-notify/slack-notify.test.js @@ -45,6 +45,7 @@ describe('slack-notify handler', () => { }, '@adobe/spacecat-shared-utils': { hasText: (s) => !!s && s.length > 0, + isNonEmptyObject: (obj) => !!obj && typeof obj === 'object' && Object.keys(obj).length > 0, }, }); runSlackNotify = handlerModule.runSlackNotify; @@ -66,6 +67,7 @@ describe('slack-notify handler', () => { }, '@adobe/spacecat-shared-utils': { hasText: () => true, + isNonEmptyObject: (obj) => !!obj && typeof obj === 'object' && Object.keys(obj).length > 0, }, }); runSlackNotify = handlerModule.runSlackNotify; @@ -97,6 +99,7 @@ describe('slack-notify handler', () => { }, '@adobe/spacecat-shared-utils': { hasText: () => true, + isNonEmptyObject: (obj) => !!obj && typeof obj === 'object' && Object.keys(obj).length > 0, }, }); runSlackNotify = handlerModule.runSlackNotify; @@ -117,4 +120,191 @@ describe('slack-notify handler', () => { blocks, }); }); + + it('prefers message payloads for simple notifications', async () => { + const sayStub = sandbox.stub().resolves(); + const handlerModule = await esmock('../../../src/tasks/slack-notify/handler.js', { + '@adobe/spacecat-shared-slack-client': { + BaseSlackClient: { createFrom: sandbox.stub() }, + SLACK_TARGETS: { WORKSPACE_INTERNAL: 'workspace_internal' }, + }, + '../../../src/utils/slack-utils.js': { + say: sayStub, + }, + '@adobe/spacecat-shared-utils': { + hasText: () => true, + isNonEmptyObject: (obj) => !!obj && typeof obj === 'object' && Object.keys(obj).length > 0, + }, + }); + runSlackNotify = handlerModule.runSlackNotify; + + const resp = await runSlackNotify({ + slackContext: { channelId: 'C123', threadTs: 'T1' }, + message: { text: 'Inner message' }, + }, context); + + expect(resp.status).to.equal(200); + expect(sayStub).to.have.been.calledOnceWithExactly( + context.env, + context.log, + { channelId: 'C123', threadTs: 'T1' }, + 'Inner message', + ); + }); + + it('uses payload blocks when provided inside message object', async () => { + const postMessage = sandbox.stub().resolves(); + const baseCreateFrom = sandbox.stub().returns({ postMessage }); + const handlerModule = await esmock('../../../src/tasks/slack-notify/handler.js', { + '@adobe/spacecat-shared-slack-client': { + BaseSlackClient: { createFrom: baseCreateFrom }, + SLACK_TARGETS: { WORKSPACE_INTERNAL: 'workspace_internal' }, + }, + '../../../src/utils/slack-utils.js': { + say: sandbox.stub().resolves(), + }, + '@adobe/spacecat-shared-utils': { + hasText: () => true, + isNonEmptyObject: (obj) => !!obj && typeof obj === 'object' && Object.keys(obj).length > 0, + }, + }); + runSlackNotify = handlerModule.runSlackNotify; + + const blocks = [{ type: 'section', text: { type: 'mrkdwn', text: 'Payload block' } }]; + const resp = await runSlackNotify({ + slackContext: { channelId: 'C321', threadTs: '321.0' }, + text: 'outer text', + message: { + text: 'payload text', + blocks, + }, + }, context); + + expect(resp.status).to.equal(200); + expect(postMessage).to.have.been.calledOnceWith({ + channel: 'C321', + thread_ts: '321.0', + text: 'payload text', + blocks, + }); + }); + + it('uses payload attachments when provided inside message object', async () => { + const postMessage = sandbox.stub().resolves(); + const baseCreateFrom = sandbox.stub().returns({ postMessage }); + const handlerModule = await esmock('../../../src/tasks/slack-notify/handler.js', { + '@adobe/spacecat-shared-slack-client': { + BaseSlackClient: { createFrom: baseCreateFrom }, + SLACK_TARGETS: { WORKSPACE_INTERNAL: 'workspace_internal' }, + }, + '../../../src/utils/slack-utils.js': { + say: sandbox.stub().resolves(), + }, + '@adobe/spacecat-shared-utils': { + hasText: () => true, + isNonEmptyObject: (obj) => !!obj && typeof obj === 'object' && Object.keys(obj).length > 0, + }, + }); + runSlackNotify = handlerModule.runSlackNotify; + + const attachments = [{ text: 'payload attachment' }]; + const resp = await runSlackNotify({ + slackContext: { channelId: 'C654', threadTs: '654.0' }, + text: 'outer', + message: { + text: 'payload', + attachments, + }, + }, context); + + expect(resp.status).to.equal(200); + expect(postMessage).to.have.been.calledOnceWith({ + channel: 'C654', + thread_ts: '654.0', + text: 'payload', + attachments, + }); + }); + + it('sends attachments even without blocks', async () => { + const postMessage = sandbox.stub().resolves(); + const baseCreateFrom = sandbox.stub().returns({ postMessage }); + const handlerModule = await esmock('../../../src/tasks/slack-notify/handler.js', { + '@adobe/spacecat-shared-slack-client': { + BaseSlackClient: { createFrom: baseCreateFrom }, + SLACK_TARGETS: { WORKSPACE_INTERNAL: 'workspace_internal' }, + }, + '../../../src/utils/slack-utils.js': { + say: sandbox.stub().resolves(), + }, + '@adobe/spacecat-shared-utils': { + hasText: () => true, + isNonEmptyObject: (obj) => !!obj && typeof obj === 'object' && Object.keys(obj).length > 0, + }, + }); + runSlackNotify = handlerModule.runSlackNotify; + + const resp = await runSlackNotify({ + slackContext: { channelId: 'C777', threadTs: '777.1' }, + text: 'fallback', + attachments: [{ text: 'details' }], + }, context); + + expect(resp.status).to.equal(200); + expect(postMessage).to.have.been.calledOnceWith({ + channel: 'C777', + thread_ts: '777.1', + text: 'fallback', + attachments: [{ text: 'details' }], + }); + }); + + it('returns badRequest when no text or structured content is provided', async () => { + const handlerModule = await esmock('../../../src/tasks/slack-notify/handler.js', { + '@adobe/spacecat-shared-slack-client': { + BaseSlackClient: { createFrom: sandbox.stub() }, + SLACK_TARGETS: { WORKSPACE_INTERNAL: 'workspace_internal' }, + }, + '../../../src/utils/slack-utils.js': { + say: sandbox.stub().resolves(), + }, + }); + runSlackNotify = handlerModule.runSlackNotify; + + const resp = await runSlackNotify({ + slackContext: { channelId: 'C001', threadTs: 'T0' }, + text: '', + }, context); + + expect(resp.status).to.equal(400); + }); + + it('uses fallback text when sending blocks without explicit text', async () => { + const postMessage = sandbox.stub().resolves(); + const baseCreateFrom = sandbox.stub().returns({ postMessage }); + const handlerModule = await esmock('../../../src/tasks/slack-notify/handler.js', { + '@adobe/spacecat-shared-slack-client': { + BaseSlackClient: { createFrom: baseCreateFrom }, + SLACK_TARGETS: { WORKSPACE_INTERNAL: 'workspace_internal' }, + }, + '../../../src/utils/slack-utils.js': { + say: sandbox.stub().resolves(), + }, + }); + runSlackNotify = handlerModule.runSlackNotify; + + const blocks = [{ type: 'section', text: { type: 'mrkdwn', text: '*Hello*' } }]; + const resp = await runSlackNotify({ + slackContext: { channelId: 'C555', threadTs: '555.0' }, + blocks, + }, context); + + expect(resp.status).to.equal(200); + expect(postMessage).to.have.been.calledOnceWith({ + channel: 'C555', + thread_ts: '555.0', + text: ' ', + blocks, + }); + }); });