fix(#3456): render /gsd command output via ctx.ui.notify in pi (#3485)

* fix(#3456): render /gsd command output via ctx.ui.notify in pi

* chore(#3456): add changeset

* chore(#3456): point changeset at PR 3485

---------

Co-authored-by: sim <sim@local>

.changeset/sturdy-dogs-caper.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3485
+---
+The /gsd slash command in Pi now visibly renders its output (progress, errors) via Pi's ctx.ui.notify mechanism instead of a return value Pi silently discards.

pi/gsd.cjs

@@ -320,21 +320,35 @@ module.exports = function gsdPiExtension(pi) {
     getArgumentCompletions,
     handler: async (args, ctx) => {
       const cwd = (ctx && ctx.cwd) || process.cwd();
+      // #3456: Pi's command dispatcher awaits the handler and DISCARDS its
+      // return value (agent-session.ts: `await command.handler(args, ctx);
+      // return true;`) — #3097's `{ content: [...] }` return was silently
+      // dropped exactly like the pre-#3097 bare string. The ONLY command-
+      // output mechanism Pi consumes is `ctx.ui.notify(message, type)`
+      // (pi docs: extensions.md#piregistercommandname-options), so the output
+      // is pushed as a side effect on both the success and error paths.
+      // Guarded so a host ctx without `ui` degrades silently instead of
+      // crashing the command.
+      const notify = (ctx && ctx.ui && typeof ctx.ui.notify === 'function')
+        ? (message, type) => ctx.ui.notify(message, type)
+        : null;
       const { family, subcommand, args: rest } = parseGsdCommandArgs(args);
       let dispatchGsdCommand;
       try {
         ({ dispatchGsdCommand } = require(path.join(GSD_CORE, 'bin', 'lib', 'shell-command-projection.cjs')));
       } catch (e) {
-        // #2991: return the structured { content } shape Pi's ExtensionAPI
-        // displays, not a bare string (which Pi silently drops).
-        return { content: [{ type: 'text', text: `GSD engine unavailable: ${e && e.message ? e.message : String(e)}` }] };
+        if (notify) {
+          notify(`GSD engine unavailable: ${e && e.message ? e.message : String(e)}`, 'error');
+        }
+        return;
       }
       const result = dispatchGsdCommand({ family, subcommand, args: rest, cwd });
-      // #2991: match gsd_invoke's proven output shape so Pi actually displays it.
       const text = result.ok
         ? result.stdout
         : `GSD error: ${result.stderr || result.stdout || `dispatch failed (exit ${result.code})`}`;
-      return { content: [{ type: 'text', text }] };
+      if (notify) {
+        notify(text, result.ok ? 'info' : 'error');
+      }
     },
   });
 

tests/pi-extension-reachability.test.cjs

@@ -6,8 +6,9 @@
  * Proves the pi extension is keystone-WIRED: the registered /gsd command's
  * `handler(args, ctx)` (pi's REAL ExtensionAPI shape — NOT the `execute(ctx)`
  * shape the original #1944 cut used) dispatches through gsd-tools.cjs
- * (subprocess-reuse — dispatchGsdCommand) and returns real output, not just a
- * registration on a mock. This is the "user can invoke X" proof.
+ * (subprocess-reuse — dispatchGsdCommand) and renders real output through
+ * `ctx.ui.notify` (#3456 — Pi discards command-handler return values), not
+ * just a registration on a mock. This is the "user can invoke X" proof.
  *
  * Dispatch is exercised with a real read-only family/subcommand
  * (progress/json) against a real temp project, matching the sibling
@@ -59,7 +60,27 @@ test('REACHABILITY: empty args dispatch a working default (gsd-tools.cjs --help)
   assert.equal(parsed.subcommand, undefined);
 });
 
-test('REACHABILITY: the /gsd handler dispatches a real family through gsd-tools.cjs and returns real output (keystone wired)', async () => {
+// #3456: Pi's command dispatcher does `await command.handler(args, ctx);
+// return true;` — it DISCARDS the handler's return value in any shape. The
+// only command-output mechanism Pi consumes is `ctx.ui.notify(message, type)`
+// (earendil-works/pi docs: extensions.md#piregistercommandname-options). The
+// #3097 tests asserted on the discarded return object, so they passed while
+// `/gsd` stayed mute in real Pi. These tests invoke the handler exactly as
+// Pi's dispatcher does (await + discard) and assert the notify side effect.
+function makePiCtx(dir) {
+  const notifications = [];
+  return {
+    ctx: {
+      cwd: dir,
+      ui: {
+        notify(message, type) { notifications.push({ message, type }); },
+      },
+    },
+    notifications,
+  };
+}
+
+test('REACHABILITY: the /gsd handler dispatches a real family through gsd-tools.cjs and renders output via ctx.ui.notify (keystone wired)', async () => {
   const pi = mockPi();
   gsdPiExtension(pi);
   const dir = createTempDir();
@@ -70,29 +91,46 @@ test('REACHABILITY: the /gsd handler dispatches a real family through gsd-tools.
     // is withheld (null), breaking this reachability proxy.
     fs.mkdirSync(path.join(dir, '.planning'), { recursive: true });
     fs.writeFileSync(path.join(dir, '.planning', 'ROADMAP.md'), '# Roadmap\n');
-    const result = await pi._recorded.commands['gsd'].handler('progress json', { cwd: dir });
-    // #2991: handler returns { content: [{ type: 'text', text }] } (Pi's display shape), not a bare string.
-    assert.ok(result && Array.isArray(result.content) && result.content[0].type === 'text',
-      `/gsd handler must return Pi's display shape { content: [{ type: 'text', text }] }; got: ${JSON.stringify(result).slice(0, 200)}`);
-    const parsed = JSON.parse(result.content[0].text);
+    const { ctx, notifications } = makePiCtx(dir);
+    // Invoke EXACTLY as Pi's dispatcher does: await, then discard the return.
+    const returned = await pi._recorded.commands['gsd'].handler('progress json', ctx);
+    assert.equal(notifications.length, 1, `/gsd must push exactly one ui.notify; got ${notifications.length}`);
+    assert.equal(notifications[0].type, 'info', 'successful dispatch notifies with type "info"');
+    const parsed = JSON.parse(notifications[0].message);
     assert.equal(typeof parsed.percent, 'number', '/gsd dispatch reached gsd-tools.cjs for real (the engine was reached)');
+    assert.equal(returned, undefined, 'the handler return value is the boundary Pi discards — the output must travel via ctx.ui.notify, not a return (#3456)');
   } finally {
     cleanup(dir);
   }
 });
 
-test('REACHABILITY: an unknown family surfaces a clear GSD error, not a throw', async () => {
+test('REACHABILITY: an unknown family renders a clear GSD error via ctx.ui.notify, not a throw', async () => {
   const pi = mockPi();
   gsdPiExtension(pi);
   const dir = createTempDir();
   try {
-    const result = await pi._recorded.commands['gsd'].handler('no-such-family-8675309', { cwd: dir });
-    // #2991: handler returns { content: [{ type: 'text', text }] } (Pi's display shape).
-    assert.ok(result && Array.isArray(result.content) && result.content[0].type === 'text',
-      `error result must carry Pi's display shape; got: ${JSON.stringify(result).slice(0, 200)}`);
-    const text = result.content[0].text;
-    assert.match(text, /GSD error:/);
-    assert.match(text, /no-such-family-8675309|Unknown command/);
+    const { ctx, notifications } = makePiCtx(dir);
+    const returned = await pi._recorded.commands['gsd'].handler('no-such-family-8675309', ctx);
+    assert.equal(notifications.length, 1, `failed dispatch must push exactly one ui.notify; got ${notifications.length}`);
+    assert.equal(notifications[0].type, 'error', 'failed dispatch notifies with type "error"');
+    assert.match(notifications[0].message, /GSD error:/);
+    assert.match(notifications[0].message, /no-such-family-8675309|Unknown command/);
+    assert.equal(returned, undefined, 'the error text must travel via ctx.ui.notify, not the discarded return (#3456)');
+  } finally {
+    cleanup(dir);
+  }
+});
+
+test('REACHABILITY: the /gsd handler tolerates a ctx without ui (older Pi host) without throwing', async () => {
+  const pi = mockPi();
+  gsdPiExtension(pi);
+  const dir = createTempDir();
+  try {
+    fs.mkdirSync(path.join(dir, '.planning'), { recursive: true });
+    fs.writeFileSync(path.join(dir, '.planning', 'ROADMAP.md'), '# Roadmap\n');
+    // No assert on output — a host without ctx.ui has no renderable channel;
+    // this only proves the handler does not crash such a host.
+    await pi._recorded.commands['gsd'].handler('progress json', { cwd: dir });
   } finally {
     cleanup(dir);
   }