fix: agent executor slack notification (#137)

* fix: agent executor slack notification

* chore: copilot instructions

* fix: urls

* Update src/agents/brand-profile/index.js

Co-authored-by: Copilot <[email protected]>

* fix: newlines

---------

Co-authored-by: Copilot <[email protected]>

Author: solaris007

.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).

README.md

@@ -107,13 +107,45 @@ $ 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
 {
   "type": "string",
   "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.

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 {

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);
 }