From bba5a7f143c0fe208246eb1636c53a62e4038c32 Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Sat, 2 May 2026 14:08:08 +0200 Subject: [PATCH] fix(headless): ignore pasted prose on orchestrator stdin --- src/headless-ui.ts | 111 +++++++++++------- .../extensions/sf/bootstrap/register-hooks.ts | 5 +- src/resources/extensions/sf/self-feedback.ts | 15 +++ src/tests/headless-supervised-stdin.test.ts | 93 +++++++++++++++ 4 files changed, 178 insertions(+), 46 deletions(-) create mode 100644 src/tests/headless-supervised-stdin.test.ts diff --git a/src/headless-ui.ts b/src/headless-ui.ts index 69e0618e0..7790157f1 100644 --- a/src/headless-ui.ts +++ b/src/headless-ui.ts @@ -243,10 +243,7 @@ export function summarizeToolArgs( } /** Summarize SF extension tool args into a compact identifier string. */ -function summarizeSfTool( - name: string, - input: Record, -): string { +function summarizeSfTool(name: string, input: Record): string { const parts: string[] = []; if (input.milestoneId) parts.push(String(input.milestoneId)); if (input.sliceId) parts.push(String(input.sliceId)); @@ -603,46 +600,70 @@ export function startSupervisedStdinReader( onResponse: (id: string) => void, ): () => void { return attachJsonlLineReader(process.stdin as Readable, (line) => { - let msg: Record; - try { - msg = JSON.parse(line); - } catch { - process.stderr.write( - `[headless] Warning: invalid JSON from orchestrator stdin, skipping\n`, - ); - return; - } - - const type = String(msg.type ?? ""); - - switch (type) { - case "extension_ui_response": { - const id = String(msg.id ?? ""); - const value = msg.value !== undefined ? String(msg.value) : undefined; - const confirmed = - typeof msg.confirmed === "boolean" ? msg.confirmed : undefined; - const cancelled = - typeof msg.cancelled === "boolean" ? msg.cancelled : undefined; - client.sendUIResponse(id, { value, confirmed, cancelled }); - if (id) { - onResponse(id); - } - break; - } - case "prompt": - client.prompt(String(msg.message ?? "")); - break; - case "steer": - client.steer(String(msg.message ?? "")); - break; - case "follow_up": - client.followUp(String(msg.message ?? "")); - break; - default: - process.stderr.write( - `[headless] Warning: unknown message type "${type}" from orchestrator stdin\n`, - ); - break; - } + handleSupervisedStdinLine(line, client, onResponse); }); } + +function shouldWarnInvalidOrchestratorLine(line: string): boolean { + const trimmed = line.trimStart(); + return trimmed.startsWith("{") || trimmed.startsWith("["); +} + +/** + * Handle one supervised JSONL stdin frame. + * + * Purpose: accept real orchestrator control records while ignoring accidental + * pasted prose in headless terminals without flooding stderr. + * + * Consumer: startSupervisedStdinReader, which receives LF-framed stdin lines. + */ +export function handleSupervisedStdinLine( + line: string, + client: RpcClient, + onResponse: (id: string) => void, + stderr: Pick = process.stderr, +): void { + let msg: Record; + try { + msg = JSON.parse(line); + } catch { + if (shouldWarnInvalidOrchestratorLine(line)) { + stderr.write( + `[headless] Warning: invalid JSON from orchestrator stdin, skipping\n`, + ); + } + return; + } + + const type = String(msg.type ?? ""); + + switch (type) { + case "extension_ui_response": { + const id = String(msg.id ?? ""); + const value = msg.value !== undefined ? String(msg.value) : undefined; + const confirmed = + typeof msg.confirmed === "boolean" ? msg.confirmed : undefined; + const cancelled = + typeof msg.cancelled === "boolean" ? msg.cancelled : undefined; + client.sendUIResponse(id, { value, confirmed, cancelled }); + if (id) { + onResponse(id); + } + break; + } + case "prompt": + client.prompt(String(msg.message ?? "")); + break; + case "steer": + client.steer(String(msg.message ?? "")); + break; + case "follow_up": + client.followUp(String(msg.message ?? "")); + break; + default: + stderr.write( + `[headless] Warning: unknown message type "${type}" from orchestrator stdin\n`, + ); + break; + } +} diff --git a/src/resources/extensions/sf/bootstrap/register-hooks.ts b/src/resources/extensions/sf/bootstrap/register-hooks.ts index 57cdbc9a5..3102d93bb 100644 --- a/src/resources/extensions/sf/bootstrap/register-hooks.ts +++ b/src/resources/extensions/sf/bootstrap/register-hooks.ts @@ -260,8 +260,11 @@ export function registerHooks( const { runGapAudit } = await import("../gap-audit.js"); const filed = runGapAudit(process.cwd()); if (filed > 0) { + const { selfFeedbackDestinationLabel } = await import( + "../self-feedback.js" + ); ctx.ui?.notify?.( - `Gap audit filed ${filed} new finding${filed === 1 ? "" : "s"} in .sf/SELF-FEEDBACK.md`, + `Gap audit filed ${filed} new finding${filed === 1 ? "" : "s"} in ${selfFeedbackDestinationLabel(process.cwd())}`, "info", ); } diff --git a/src/resources/extensions/sf/self-feedback.ts b/src/resources/extensions/sf/self-feedback.ts index fda194eb9..649af8f95 100644 --- a/src/resources/extensions/sf/self-feedback.ts +++ b/src/resources/extensions/sf/self-feedback.ts @@ -184,6 +184,21 @@ function upstreamLogPath(): string { return join(sfHome, "agent", "upstream-feedback.jsonl"); } +/** + * Return the operator-facing destination for new self-feedback in `basePath`. + * + * Purpose: keep startup notices honest when sf is dogfooded in other repos; + * external-project findings are about sf but are persisted globally, not in + * the target repo's `.sf/` directory. + * + * Consumer: session_start notifications that summarize detector writes. + */ +export function selfFeedbackDestinationLabel(basePath: string): string { + return isForgeRepo(basePath) + ? ".sf/SELF-FEEDBACK.md" + : "~/.sf/agent/upstream-feedback.jsonl"; +} + /** * Migrate the legacy filename. Older sf versions wrote `BACKLOG.md`; the * canonical name is now `SELF-FEEDBACK.md` (matches `self-feedback.jsonl`). diff --git a/src/tests/headless-supervised-stdin.test.ts b/src/tests/headless-supervised-stdin.test.ts new file mode 100644 index 000000000..8e129e75e --- /dev/null +++ b/src/tests/headless-supervised-stdin.test.ts @@ -0,0 +1,93 @@ +import assert from "node:assert/strict"; +import { describe, it } from "vitest"; +import { handleSupervisedStdinLine } from "../headless-ui.js"; + +class MockRpcClient { + prompts: string[] = []; + steers: string[] = []; + followUps: string[] = []; + uiResponses: Array<{ + id: string; + response: { + value?: string; + confirmed?: boolean; + cancelled?: boolean; + }; + }> = []; + + prompt(message: string): void { + this.prompts.push(message); + } + + steer(message: string): void { + this.steers.push(message); + } + + followUp(message: string): void { + this.followUps.push(message); + } + + sendUIResponse( + id: string, + response: { + value?: string; + confirmed?: boolean; + cancelled?: boolean; + }, + ): void { + this.uiResponses.push({ id, response }); + } +} + +function stderrSink(): { lines: string[]; write: (line: string) => boolean } { + const lines: string[] = []; + return { + lines, + write(line: string): boolean { + lines.push(line); + return true; + }, + }; +} + +describe("handleSupervisedStdinLine", () => { + it("ignores pasted prose without warning spam", () => { + const client = new MockRpcClient(); + const stderr = stderrSink(); + + handleSupervisedStdinLine( + "rchestrator stdin, skipping", + client as never, + () => {}, + stderr, + ); + + assert.deepEqual(stderr.lines, []); + assert.deepEqual(client.prompts, []); + }); + + it("warns for malformed JSON-looking orchestrator frames", () => { + const client = new MockRpcClient(); + const stderr = stderrSink(); + + handleSupervisedStdinLine("{ bad", client as never, () => {}, stderr); + + assert.equal(stderr.lines.length, 1); + assert.match(stderr.lines[0] ?? "", /invalid JSON from orchestrator stdin/); + }); + + it("dispatches valid steer frames", () => { + const client = new MockRpcClient(); + const stderr = stderrSink(); + + handleSupervisedStdinLine( + JSON.stringify({ type: "steer", message: "continue" }), + client as never, + () => {}, + stderr, + ); + + assert.deepEqual(client.steers, ["continue"]); + assert.deepEqual(stderr.lines, []); + }); +});