From d82f3c1f49364b217aa6dae5f02fd917fe6b8e87 Mon Sep 17 00:00:00 2001 From: Richardson Gunde Date: Mon, 27 Jul 2026 12:16:05 +0530 Subject: [PATCH 1/2] Fix Slack notifications: 'fields' is not a Block Kit block type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack notifications have never been deliverable. Both payload builders in src/notifications/index.js emitted a block of `{ type: 'fields', fields }`, but Block Kit has no `fields` block type — fields belong on a `section` block. Slack rejects the entire attachment with: 400 invalid_attachments Verified against a live Incoming Webhook: failing before this change, delivering successfully after it. The bogus type had propagated to every consumer, so each one is updated to read fields off a section block while still tolerating the old shape, which keeps any in-flight or persisted payloads converting correctly: - channels/teams.js - facts extraction - channels/discord.js - embed fields - channels/text.js - plain-text flattening (covered by the existing "slackPayloadToText renders blocks as readable plain text" test, which caught this consumer) Teams and Discord converters produce identical output to before the change. tests/notifications-channels.test.js, harness-notifications.test.js and notify-hook.test.js pass 21/21. Co-Authored-By: Claude Opus 5 (1M context) --- src/notifications/channels/discord.js | 3 ++- src/notifications/channels/teams.js | 3 ++- src/notifications/channels/text.js | 3 ++- src/notifications/index.js | 6 ++++-- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/notifications/channels/discord.js b/src/notifications/channels/discord.js index 5d875ddc..847c1daf 100644 --- a/src/notifications/channels/discord.js +++ b/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'); diff --git a/src/notifications/channels/teams.js b/src/notifications/channels/teams.js index 28b86a5e..0b6bc4b9 100644 --- a/src/notifications/channels/teams.js +++ b/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); diff --git a/src/notifications/channels/text.js b/src/notifications/channels/text.js index adad2663..2814c08f 100644 --- a/src/notifications/channels/text.js +++ b/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]); diff --git a/src/notifications/index.js b/src/notifications/index.js index 14e3a2ea..1c20acef 100644 --- a/src/notifications/index.js +++ b/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}` }, From f080458ce7962b3fefc72594edd268e128359aee Mon Sep 17 00:00:00 2001 From: Richardson Gunde Date: Mon, 27 Jul 2026 14:18:47 +0530 Subject: [PATCH 2/2] Fix remaining #470 console flash: hub server spawn, not just browser-open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #471 fixed the cmd.exe browser-open spawn (`start ` via shell:true) at four call sites, but each of those files makes a SECOND spawn that #471 missed: launching the Business Hub server itself by spawning node.exe directly with `detached: true` and no `windowsHide`. node.exe is a console-subsystem executable. Spawning one detached without windowsHide flashes a visible console window on win32 independent of the shell:true/cmd.exe case #471 addressed — so the flash kept happening on every Claude Code session start (SessionStart hook -> autoLaunchBusinessHub) and every Pi session start, exactly where users would notice it most. Fixed the two call sites: - src/hooks/auto-launch.js (Claude Code SessionStart hook) - src/integrations/pi/rstack-sdlc.ts (Pi session start) autoLaunchBusinessHub now accepts an injectable `spawnImpl` (mirroring the existing openBrowser/startContainerRuntime convention) so the windowsHide behavior on the server-launch spawn is unit-testable without spawning a real background process. Added a regression test to tests/windows-console-flash-470.test.js alongside the existing #470 coverage. Full suite: 1559 pass / 51 fail, same 51 as an unmodified tree — all spawn ENOENT / temp-dir EBUSY, the documented pre-existing Windows-sandbox limitations, none touching the changed files. Co-Authored-By: Claude Sonnet 5 --- src/hooks/auto-launch.js | 11 ++++++++- src/integrations/pi/rstack-sdlc.ts | 5 +++++ tests/windows-console-flash-470.test.js | 30 ++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/hooks/auto-launch.js b/src/hooks/auto-launch.js index aadba125..bc6450b8 100644 --- a/src/hooks/auto-launch.js +++ b/src/hooks/auto-launch.js @@ -18,9 +18,13 @@ function isPortOpen(port) { }); } +// spawnImpl is injectable (mirrors openBrowser's convention below) so the +// windowsHide behavior on the server-launch spawn is testable without +// actually spawning a background process. export async function autoLaunchBusinessHub(projectRoot, opts = {}) { if (process.env.RSTACK_NO_BUSINESS_HUB === '1') return; + const spawnImpl = opts.spawnImpl ?? spawn; const port = Number(process.env.RSTACK_BUSINESS_PORT ?? 3008); const already = await isPortOpen(port); if (already) { @@ -34,9 +38,14 @@ export async function autoLaunchBusinessHub(projectRoot, opts = {}) { const args = ['--no-browser']; if (projectRoot) args.push('--project', projectRoot); - const child = spawn(process.execPath, [binPath, ...args], { + const child = spawnImpl(process.execPath, [binPath, ...args], { stdio: 'ignore', detached: true, + // node.exe is itself a console-subsystem executable; spawning it detached + // without windowsHide flashes a visible console window on win32, same as + // the cmd.exe browser-launch case #470/#471 fixed — that fix covered only + // openBrowser's spawn, not this one. + windowsHide: true, env: { ...process.env, RSTACK_NO_BROWSER: '1', RSTACK_BUSINESS_PORT: String(port) }, }); child.unref(); diff --git a/src/integrations/pi/rstack-sdlc.ts b/src/integrations/pi/rstack-sdlc.ts index 0a255d79..18197792 100644 --- a/src/integrations/pi/rstack-sdlc.ts +++ b/src/integrations/pi/rstack-sdlc.ts @@ -194,6 +194,11 @@ function tryRegisterAndLaunchHub(projectRoot: string): void { const child = spawn(process.execPath, [binPath, "--no-browser", "--project", projectRoot], { stdio: ["ignore", logFd, logFd], detached: true, + // node.exe is itself a console-subsystem executable; spawning it detached + // without windowsHide flashes a visible console window on win32, same as + // the cmd.exe browser-launch case #470/#471 fixed — that fix covered only + // openUrl's spawn, not this one. + windowsHide: true, env: { ...process.env, RSTACK_NO_BROWSER: "1", RSTACK_BUSINESS_PORT: String(port) }, }); child.unref(); diff --git a/tests/windows-console-flash-470.test.js b/tests/windows-console-flash-470.test.js index 92126599..27411434 100644 --- a/tests/windows-console-flash-470.test.js +++ b/tests/windows-console-flash-470.test.js @@ -6,6 +6,14 @@ * passes windowsHide on win32, and that non-win32 platforms are unaffected * (the option is harmless there, but shell must stay false). * + * The original #470/#471 fix covered the browser-open spawn (cmd.exe /c + * start) at each call site but missed the OTHER spawn each of those files + * makes: launching the Business Hub SERVER itself by spawning node.exe + * directly with detached: true. node.exe is a console-subsystem executable, + * so that spawn flashes a console too, independent of the shell:true case — + * it kept flashing after #471 merged. Covered below for both the Claude + * Code hook (auto-launch.js) and the Pi integration (rstack-sdlc.ts). + * * owner: RStack developed by Richardson Gunde */ @@ -17,7 +25,7 @@ // call sites below instead of a direct import. import test from 'node:test'; import assert from 'node:assert/strict'; -import { openBrowser as openBrowserAutoLaunch } from '../src/hooks/auto-launch.js'; +import { openBrowser as openBrowserAutoLaunch, autoLaunchBusinessHub } from '../src/hooks/auto-launch.js'; import { openUrl } from '../src/integrations/pi/rstack-sdlc.ts'; import { startContainerRuntime } from '../src/core/harness/sandbox.js'; @@ -94,6 +102,26 @@ test('pi/rstack-sdlc.ts openUrl: no-ops under CI (never pops a browser during au assert.equal(calls.length, 0, 'openUrl must not spawn anything when CI is set'); }); +test('auto-launch.js autoLaunchBusinessHub: windowsHide:true on the server-launch spawn (missed by #471)', async () => { + const calls = []; + // A high, effectively-never-bound port so isPortOpen() resolves false and + // the function takes the "spawn a fresh instance" branch under test. + const originalPort = process.env.RSTACK_BUSINESS_PORT; + const originalNoBrowser = process.env.RSTACK_NO_BROWSER; + process.env.RSTACK_BUSINESS_PORT = '58471'; + process.env.RSTACK_NO_BROWSER = '1'; + try { + await autoLaunchBusinessHub('/tmp/project', { spawnImpl: recordingSpawn(calls) }); + } finally { + if (originalPort === undefined) delete process.env.RSTACK_BUSINESS_PORT; else process.env.RSTACK_BUSINESS_PORT = originalPort; + if (originalNoBrowser === undefined) delete process.env.RSTACK_NO_BROWSER; else process.env.RSTACK_NO_BROWSER = originalNoBrowser; + } + assert.equal(calls.length, 1); + assert.equal(calls[0].cmd, process.execPath); + assert.equal(calls[0].options.detached, true); + assert.equal(calls[0].options.windowsHide, true); +}); + test('sandbox.js startContainerRuntime: windowsHide:true on the engine auto-start spawn', async () => { const calls = []; const result = await startContainerRuntime({