Fix Slack notifications: 'fields' is not a Block Kit block type

package.json

@@ -114,6 +114,7 @@
   "overrides": {
     "esbuild": ">=0.28.1",
     "protobufjs": ">=7.6.4 <8",
-    "ws": ">=8.21.0"
+    "ws": ">=8.21.0",
+    "brace-expansion": "5.0.8"
   }
 }

scripts/security-audit.mjs

@@ -13,13 +13,14 @@
 // fails the build. Tolerated advisories are printed so they stay visible.
 
 import { execFileSync } from 'node:child_process';
+import { fileURLToPath } from 'node:url';
 
 // Packages whose own subtree we cannot patch downstream: pi-coding-agent ships
 // an npm-shrinkwrap.json, which npm honors absolutely — root `overrides` are
 // ignored inside it exactly like a bundled tarball (verified empirically: a
 // fresh resolve with a matching override still installs the shrinkwrapped
 // version). Fixes there must come from an upstream pi release.
-const BUNDLED_ROOTS = [
+export const BUNDLED_ROOTS = [
   'node_modules/@earendil-works/pi-coding-agent/node_modules/',
 ];
 
@@ -28,22 +29,29 @@ const BUNDLED_ROOTS = [
 // per-advisory allowlist is deliberate: a brand-new high/critical for the
 // same package — let alone any other package — still fails the gate, forcing
 // a conscious decision rather than silent acceptance.
-// - brace-expansion GHSA-3jxr-9vmj-r5cp (DoS): pinned 5.0.6 by pi's
-//   shrinkwrap; every non-shrinkwrapped path in our tree is on the fixed line.
+// - brace-expansion GHSA-3jxr-9vmj-r5cp + GHSA-mh99-v99m-4gvg (both DoS):
+//   pinned 5.0.6 by pi's shrinkwrap, both advisories confined to that one
+//   bundled node (verified: the root `overrides` entry below forces every
+//   non-shrinkwrapped path in our tree onto the patched 5.0.8 line, which
+//   fixes both GHSAs — 5.0.8 itself declares `engines.node: "20 || >=22"`,
+//   dropping Node 18, but it's a devDependency-only lint/audit-tooling path
+//   with no `engine-strict` set, so npm installs it without error on the
+//   Node 18 CI lane; this is a deliberate, documented trade-off since no
+//   brace-expansion release fixes both GHSAs while keeping Node 18 support).
 // - protobufjs / ws: previously tolerated by name while their advisories were
 //   live in the bundled subtree; currently dormant, so no ids are listed — if
 //   one refires, the gate fails loudly and the specific GHSA gets re-added.
-const TOLERATED_BUNDLED_ADVISORIES = new Map([
-  ['brace-expansion', new Set(['GHSA-3jxr-9vmj-r5cp'])],
+export const TOLERATED_BUNDLED_ADVISORIES = new Map([
+  ['brace-expansion', new Set(['GHSA-3jxr-9vmj-r5cp', 'GHSA-mh99-v99m-4gvg'])],
 ]);
 
-function advisoryIds(info) {
+export function advisoryIds(info) {
   return (info.via ?? [])
     .filter((entry) => typeof entry === 'object' && entry?.url)
     .map((entry) => String(entry.url).split('/').pop());
 }
 
-const BLOCKING = new Set(['high', 'critical']);
+export const BLOCKING = new Set(['high', 'critical']);
 
 function runAudit() {
   try {
@@ -60,43 +68,61 @@ function runAudit() {
   }
 }
 
-function isFullyBundled(nodes) {
+export function isFullyBundled(nodes, bundledRoots = BUNDLED_ROOTS) {
   if (!nodes || !nodes.length) return false;
-  return nodes.every((path) => BUNDLED_ROOTS.some((root) => path.includes(root)));
+  return nodes.every((path) => bundledRoots.some((root) => path.includes(root)));
 }
 
-const report = runAudit();
-const vulns = report.vulnerabilities || {};
-const blocking = [];
-const tolerated = [];
+// Pure decision logic (no subprocess, no process.exit) — the part every
+// tolerate/block call actually has to get right. Kept separate from
+// runAudit()/main() so it can be unit-tested against a synthetic `npm audit
+// --json` shape without shelling out.
+export function evaluateAdvisories(
+  vulnerabilities,
+  { toleratedBundledAdvisories = TOLERATED_BUNDLED_ADVISORIES, bundledRoots = BUNDLED_ROOTS } = {},
+) {
+  const blocking = [];
+  const tolerated = [];
 
-for (const [name, info] of Object.entries(vulns)) {
-  if (!BLOCKING.has(info.severity)) continue;
-  const allowedIds = TOLERATED_BUNDLED_ADVISORIES.get(name);
-  const ids = advisoryIds(info);
-  const everyIdTolerated = Boolean(allowedIds)
-    && ids.length > 0
-    && ids.every((id) => allowedIds.has(id));
-  if (everyIdTolerated && isFullyBundled(info.nodes)) {
-    tolerated.push({ name, severity: info.severity, ids });
-  } else {
-    blocking.push({ name, severity: info.severity, nodes: info.nodes });
+  for (const [name, info] of Object.entries(vulnerabilities || {})) {
+    if (!BLOCKING.has(info.severity)) continue;
+    const allowedIds = toleratedBundledAdvisories.get(name);
+    const ids = advisoryIds(info);
+    const everyIdTolerated = Boolean(allowedIds)
+      && ids.length > 0
+      && ids.every((id) => allowedIds.has(id));
+    if (everyIdTolerated && isFullyBundled(info.nodes, bundledRoots)) {
+      tolerated.push({ name, severity: info.severity, ids });
+    } else {
+      blocking.push({ name, severity: info.severity, nodes: info.nodes });
+    }
   }
-}
 
-if (tolerated.length) {
-  console.log('Tolerated advisories (vendored/bundled, unpatchable downstream — track upstream):');
-  for (const t of tolerated) console.log(`  - ${t.name} (${t.severity}) [${t.ids.join(', ')}]`);
+  return { blocking, tolerated };
 }
 
-if (blocking.length) {
-  console.error('\nBlocking high/critical advisories in our dependency tree:');
-  for (const b of blocking) {
-    console.error(`  - ${b.name} (${b.severity})`);
-    for (const n of b.nodes || []) console.error(`      ${n}`);
+function main() {
+  const report = runAudit();
+  const { blocking, tolerated } = evaluateAdvisories(report.vulnerabilities || {});
+
+  if (tolerated.length) {
+    console.log('Tolerated advisories (vendored/bundled, unpatchable downstream — track upstream):');
+    for (const t of tolerated) console.log(`  - ${t.name} (${t.severity}) [${t.ids.join(', ')}]`);
+  }
+
+  if (blocking.length) {
+    console.error('\nBlocking high/critical advisories in our dependency tree:');
+    for (const b of blocking) {
+      console.error(`  - ${b.name} (${b.severity})`);
+      for (const n of b.nodes || []) console.error(`      ${n}`);
+    }
+    console.error('\nRun `npm audit` for details, then bump or override the offending package.');
+    process.exit(1);
   }
-  console.error('\nRun `npm audit` for details, then bump or override the offending package.');
-  process.exit(1);
+
+  console.log(`\nSecurity baseline OK — ${tolerated.length} tolerated bundled advisory(ies), 0 blocking.`);
 }
 
-console.log(`\nSecurity baseline OK — ${tolerated.length} tolerated bundled advisory(ies), 0 blocking.`);
+if (process.argv[1] === fileURLToPath(import.meta.url)) {
+  main();
+}

src/notifications/channels/discord.js

@@ -21,7 +21,8 @@ export function convertSlackToDiscord(slackPayload) {
       if (block.type === 'section' && block.text) {
         descriptionLines.push(block.text.text);
       }
-      if (block.type === 'fields' && block.fields) {
+      // Fields ride on a section block; 'fields' is tolerated for older payloads.
+      if ((block.type === 'section' || block.type === 'fields') && block.fields) {
         for (const f of block.fields) {
           const raw = f.text || '';
           const parts = raw.split('\n');

src/notifications/channels/teams.js

@@ -24,7 +24,8 @@ export function convertSlackToTeams(slackPayload) {
           facts: []
         });
       }
-      if (block.type === 'fields' && block.fields) {
+      // Fields ride on a section block; 'fields' is tolerated for older payloads.
+      if ((block.type === 'section' || block.type === 'fields') && block.fields) {
         const currentSection = sections[sections.length - 1] || { facts: [] };
         if (!sections.includes(currentSection)) {
           sections.push(currentSection);

src/notifications/channels/text.js

@@ -23,7 +23,8 @@ export function slackPayloadToText(slackPayload) {
       if (block.type === 'section' && block.text?.text) {
         lines.push(stripMrkdwn(block.text.text));
       }
-      if (block.type === 'fields' && Array.isArray(block.fields)) {
+      // Fields ride on a section block; 'fields' is tolerated for older payloads.
+      if ((block.type === 'section' || block.type === 'fields') && Array.isArray(block.fields)) {
         for (const field of block.fields) {
           const parts = String(field.text ?? '').split('\n');
           const name = stripMrkdwn(parts[0]);

src/notifications/index.js

@@ -89,7 +89,8 @@ export function formatSlackStageMessage(runId, stageId, status, details = {}) {
       },
     },
     {
-      type: 'fields',
+      // Block Kit has no 'fields' block type — fields live on a section block.
+      type: 'section',
       fields,
     },
   ];
@@ -146,7 +147,8 @@ export function formatSlackTaskReportMessage(runId, taskId, trace) {
       },
     },
     {
-      type: 'fields',
+      // Block Kit has no 'fields' block type — fields live on a section block.
+      type: 'section',
       fields: [
         { type: 'mrkdwn', text: `*Run ID:*\n${runId}` },
         { type: 'mrkdwn', text: `*Task:*\n${taskId}` },

tests/notifications-channels.test.js

@@ -41,6 +41,30 @@ test('discord and teams converters keep their original behavior', () => {
   assert.ok(teams.sections[0].facts.some((fact) => fact.name === 'Run ID:'));
 });
 
+test('legacy block.type === "fields" payloads are still converted (backward compatibility)', () => {
+  const legacyPayload = {
+    attachments: [{
+      color: '#22c55e',
+      blocks: [
+        { type: 'section', text: { type: 'mrkdwn', text: 'Legacy stage update' } },
+        { type: 'fields', fields: [{ text: '*Run ID:*\nrun-legacy-001' }] },
+      ],
+    }],
+  };
+
+  const text = slackPayloadToText(legacyPayload);
+  assert.ok(text.includes('Legacy stage update'));
+  assert.ok(text.includes('Run ID: run-legacy-001'), 'legacy fields block flattened into text');
+
+  const discord = convertSlackToDiscord(legacyPayload);
+  assert.equal(discord.embeds[0].fields.length, 1);
+  assert.equal(discord.embeds[0].fields[0].name, 'Run ID:');
+  assert.equal(discord.embeds[0].fields[0].value, 'run-legacy-001');
+
+  const teams = convertSlackToTeams(legacyPayload);
+  assert.ok(teams.sections[0].facts.some((fact) => fact.name === 'Run ID:' && fact.value === 'run-legacy-001'));
+});
+
 test('resolveChannels: env vars, URL sniffing, and file config precedence', () => {
   const root = mkdtempSync(join(tmpdir(), 'rstack-notify-'));
   mkdirSync(join(root, '.rstack'), { recursive: true });