Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,7 @@ _Avoid_: Allow-listed app, multi-line app, safe app
**Break placement**:
The choice of where paragraph breaks belong in one dictation. The rewrite model server decides it, and answers with sentence numbers rather than with text.
_Avoid_: Paragraph inference, auto-formatting, LLM cleanup

**Post-processing hook** (#259):
An optional, user-supplied script the finished text is piped through (stdin in, stdout out) as the very last step before delivery - after rules cleanup and break placement, not instead of them. Unlike those, it is off by default, arbitrary (the script can do anything, including call out to a local model), and never blocks delivery: a failure, a timeout, or a missing script falls back to the text as it stood before the hook ran. Distinct from Voice edit, which transforms text the user already selected on request, not a dictation in flight.
_Avoid_: Post-processing plugin, middleware, filter
54 changes: 54 additions & 0 deletions docs/adr/0004-shell-script-post-processing-hook.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
---
status: accepted
---

# A shell script, not an LLM prompt, for the post-processing hook

[#259](https://github.com/Nabzx/openstream/issues/259) asked for a hook between
Rules-cleanup and delivery, and named two candidate shapes: a user-supplied shell
script piped the finished text, or a user-supplied prompt sent to the existing
rewrite model server. Building both was on the table too.

**Shell script only, configured as a path to an existing executable file, not an
inline command typed into Settings.** A script subsumes the LLM case rather than
sitting beside it - anyone who wants an LLM in their post-processing step can write a
script that calls one, local or otherwise, and OpenStream needs no second
process-execution surface, no prompt-template UI, and no dependency on the rewrite
model server's quality (already found unreliable at following instructions -
[#222](https://github.com/Nabzx/openstream/issues/222)'s spike). A script path also
keeps Settings from becoming a place where a full shell command is typed and run
directly from a text box; the user writes and maintains the script in their own
editor, and OpenStream only ever points at it.

This is a deliberate acceptance of an arbitrary-code-execution feature, not an
oversight. The script runs with the same access the OpenStream process has, on text
the user just spoke - there is no sandboxing beyond a timeout, and none was built. The
mitigations are: **off by default** (`postProcessScriptPath: null`), a file picker
rather than a pasted command (the user already had to have the script on disk), and an
explicit warning in Settings that the script runs with full access and should only
point at something the user trusts. Anyone using this feature is knowingly running
their own code on their own machine, the same trust boundary as any other script they
write and run themselves - this is not a plugin marketplace or a shared-script
feature, and should not become one without revisiting this decision.

## Consequences

- **`electron/postProcessHook.js` owns the mechanism** (spawn, pipe stdin, collect
stdout, enforce a timeout, kill on expiry); `dictationCoordinator.js` owns the
policy (a failure or timeout falls back to the un-hooked text, never blocks
delivery, logged not surfaced). The split mirrors every other adapter in the
coordinator - the pure decision logic is what's unit-tested against a fake, the
real child-process mechanics are what's tested against real scripts.
- **No LLM-prompt hook exists.** If model-driven post-processing is wanted later
(semantic voice edits per #259's own framing, or a "make this shorter" prompt).
it can either be its own script (calling out to `llama-server`'s existing
`/v1/chat/completions` endpoint, or any other local model) or a genuine second
hook type - revisit this ADR if a real need for the latter shows up.
- **No inline shell-command field, and no plan to add one.** If that convenience is
wanted later, it changes this ADR's trust framing (a copy-pasted one-liner is a
weaker signal of "the user wrote and understands this" than a file they authored)
and should be a deliberate reopening, not a quiet UI addition.
- **The timeout (10s default, `postProcessHook.js`'s `DEFAULT_TIMEOUT_MS`) is not
exposed in Settings.** A user who wants a slower hook (one that calls out to a
local LLM, say) is out of luck until this is revisited - kept minimal for v1
rather than guessed at.
27 changes: 27 additions & 0 deletions electron/dictationCoordinator.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ function createDictationIntake(options) {
// "paste" command. Callers/tests that don't wire it get no paste
// command, everything else unchanged.
clipboard = null,
// #259: an optional user-supplied post-processing hook, the last
// transform before delivery. Defaults to a no-op passthrough so
// callers/tests that don't wire it see today's behaviour.
postProcess = { run: async (text) => text },
} = options;

assertAdapter("transcription", transcription, "transcribe");
Expand All @@ -75,6 +79,7 @@ function createDictationIntake(options) {
assertAdapter("vocabulary", vocabulary, "getPrompt");
assertAdapter("corrections", corrections, "getEntries");
assertAdapter("language", language, "get");
assertAdapter("postProcess", postProcess, "run");

let queue = Promise.resolve();

Expand Down Expand Up @@ -308,6 +313,28 @@ function createDictationIntake(options) {
}
}

// #259: the last transform before delivery, and optional - most callers
// have no hook configured and postProcess.run() is a no-op passthrough.
// A failure or timeout never blocks delivery: it falls back to the text
// as it stood before the hook ran, logged but not surfaced to the user.
try {
const hooked = await postProcess.run(finishedText);
if (typeof hooked !== "string") {
throw new Error("post-process hook adapter returned a non-string result");
}
emitDiagnostic("postProcess.applied", hooked !== finishedText);
finishedText = hooked;
} catch (error) {
emitDiagnostic("postProcess.failure", errorMessage(error));
}

// A hook can deliberately return empty text (a redaction filter that
// decides there's nothing left to say) - treat that the same as the
// no-speech check earlier, rather than delivering nothing.
if (!finishedText) {
return { status: "no-speech" };
}

try {
// #355: pass the bundle just confirmed as frontmost, so the helper's
// own abort check (which watches for the app changing again, this
Expand Down
91 changes: 91 additions & 0 deletions electron/dictationCoordinator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ function createIntake({
listDetection,
// #375: undefined = no clipboard adapter wired at all.
clipboardText,
// #259: undefined = no post-processing hook adapter wired (the default
// no-op passthrough).
postProcess,
} = {}) {
const diagnostics = [];
const delivered = [];
Expand Down Expand Up @@ -56,6 +59,7 @@ function createIntake({
onDiagnostic: onDiagnostic || ((name, value) => diagnostics.push([name, value])),
listDetection,
clipboard: clipboardText !== undefined ? { readText: () => clipboardText } : null,
...(postProcess !== undefined ? { postProcess } : {}),
});

return { intake, diagnostics, delivered, breakCalls };
Expand Down Expand Up @@ -397,6 +401,7 @@ test("repairs malformed break indices without retrying and records format and re
["paragraphBreaks.repairUsed", true],
["listBoundaries.formatValid", true],
["listBoundaries.repairUsed", false],
["postProcess.applied", false],
]);
});

Expand Down Expand Up @@ -457,6 +462,7 @@ test("a flagged spoken list renders as bullets set off from the surrounding pros
["paragraphBreaks.repairUsed", false],
["listBoundaries.formatValid", true],
["listBoundaries.repairUsed", false],
["postProcess.applied", false],
]);
});

Expand Down Expand Up @@ -486,6 +492,7 @@ test("an out-of-range list range is clamped into the text and recorded as repair
["paragraphBreaks.repairUsed", false],
["listBoundaries.formatValid", true],
["listBoundaries.repairUsed", true],
["postProcess.applied", false],
]);
});

Expand Down Expand Up @@ -515,6 +522,7 @@ test("a malformed LIST line fails closed to prose without dropping paragraph bre
["paragraphBreaks.repairUsed", false],
["listBoundaries.formatValid", false],
["listBoundaries.repairUsed", false],
["postProcess.applied", false],
]);
});

Expand Down Expand Up @@ -543,6 +551,7 @@ test("list detection is off by default: a valid range is parsed and reported but
["paragraphBreaks.repairUsed", false],
["listBoundaries.formatValid", true],
["listBoundaries.repairUsed", false],
["postProcess.applied", false],
]);
});

Expand Down Expand Up @@ -805,6 +814,88 @@ test("held delivery preserves the complete finished text without retrying delive
assert.deepEqual(delivered, ["Hello world."]);
});

test("#259: with no post-process hook configured, text is delivered unchanged", async () => {
const harness = createIntake({ transcript: "hello world" });

const result = await harness.intake.complete(completedWav);

assert.deepEqual(result, { status: "delivered", text: "Hello world." });
assert.deepEqual(harness.delivered, ["Hello world."]);
});

test("#259: a post-process hook's output is what gets delivered", async () => {
const harness = createIntake({
transcript: "hello world",
postProcess: { run: async (text) => text.toUpperCase() },
});

const result = await harness.intake.complete(completedWav);

assert.deepEqual(result, { status: "delivered", text: "HELLO WORLD." });
assert.deepEqual(harness.delivered, ["HELLO WORLD."]);
assert.ok(harness.diagnostics.some(([name, value]) => name === "postProcess.applied" && value === true));
});

test("#259: a hook that throws falls back to the un-hooked text, delivery still proceeds", async () => {
const harness = createIntake({
transcript: "hello world",
postProcess: {
run: async () => {
throw new Error("script exited with code 1");
},
},
});

const result = await harness.intake.complete(completedWav);

assert.deepEqual(result, { status: "delivered", text: "Hello world." });
assert.deepEqual(harness.delivered, ["Hello world."]);
assert.ok(
harness.diagnostics.some(([name, value]) => name === "postProcess.failure" && value === "script exited with code 1"),
);
});

test("#259: a hook returning a non-string is treated as a failure, falls back", async () => {
const harness = createIntake({
transcript: "hello world",
postProcess: { run: async () => 42 },
});

const result = await harness.intake.complete(completedWav);

assert.deepEqual(result, { status: "delivered", text: "Hello world." });
assert.ok(harness.diagnostics.some(([name]) => name === "postProcess.failure"));
});

test("#259: a hook that deliberately empties the text results in no-speech, not an empty delivery", async () => {
const harness = createIntake({
transcript: "hello world",
postProcess: { run: async () => "" },
});

const result = await harness.intake.complete(completedWav);

assert.deepEqual(result, { status: "no-speech" });
assert.deepEqual(harness.delivered, []);
});

test("#259: a held delivery carries the hook's output, not the pre-hook text", async () => {
const delivered = [];
const harness = createIntake({
transcript: "hello world",
postProcess: { run: async (text) => text.toUpperCase() },
deliver: async (text) => {
delivered.push(text);
return { kind: "held", reason: "unverified target" };
},
});

const result = await harness.intake.complete(completedWav);

assert.deepEqual(result, { status: "held", text: "HELLO WORLD.", reason: "unverified target" });
assert.deepEqual(delivered, ["HELLO WORLD."]);
});

test("completed recordings are processed and delivered in FIFO order", async () => {
const firstTranscription = {};
const firstTranscriptionReady = new Promise((resolve) => {
Expand Down
25 changes: 25 additions & 0 deletions electron/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const hotkeyHelper = require("./hotkeyHelper");
const accessibilityHelper = require("./accessibilityHelper");
const { createSettingsStore } = require("./settingsStore");
const { createRecordingHistoryStore } = require("./recordingHistoryStore");
const { createPostProcessHook } = require("./postProcessHook");
const { createIdleUnloader } = require("./idleUnloader");
const { createMediaPause } = require("./mediaPause");
const { createSoundCues } = require("./soundCues");
Expand Down Expand Up @@ -205,6 +206,13 @@ function soundCuesEnabled() {
return Boolean(settingsStore && settingsStore.get().soundCues);
}

// #259: reads the configured script path fresh on every dictation, same
// reasoning as corrections/language below - a change in Settings applies
// to the very next utterance, no restart needed.
const postProcessHook = createPostProcessHook({
getScriptPath: () => (settingsStore ? settingsStore.get().postProcessScriptPath : null),
});

const dictationIntake = createDictationIntake({
transcription,
// #375: a spoken "paste" reads from here.
Expand All @@ -218,6 +226,7 @@ const dictationIntake = createDictationIntake({
corrections: { getEntries: () => (settingsStore ? settingsStore.get().termCorrections : []) },
// #252: the transcription input language, same fresh read.
language: { get: () => (settingsStore ? settingsStore.get().inputLanguage : "en") },
postProcess: postProcessHook,
onDiagnostic: recordDictationDiagnostic,
});

Expand Down Expand Up @@ -1111,6 +1120,22 @@ ipcMain.handle("settings:set-history-max-entries", (event, count) => {
return settingsStore.setHistoryMaxEntries(count);
});

// #259: clears the hook when passed null; picking a new script goes through
// settings:pick-post-process-script instead, which sets it in one round trip.
ipcMain.handle("settings:set-post-process-script", (event, scriptPath) => {
return settingsStore.setPostProcessScriptPath(scriptPath);
});

ipcMain.handle("settings:pick-post-process-script", async () => {
if (!win) return null;
const result = await dialog.showOpenDialog(win, {
properties: ["openFile"],
message: "Choose a script to run the finished text through before it's delivered",
});
if (result.canceled || result.filePaths.length === 0) return null;
return settingsStore.setPostProcessScriptPath(result.filePaths[0]);
});

// #19: pick an app from disk instead of hunting down its bundle id by
// hand. Returns { bundleId, name } for the renderer to add, or null if the
// dialog was cancelled; a bundle with no readable identifier rejects.
Expand Down
87 changes: 87 additions & 0 deletions electron/postProcessHook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
const { spawn } = require("child_process");

// #259: an optional, user-supplied script the cleaned, finished text is
// piped through (stdin) right before delivery - a shell hook rather than a
// second LLM path, since a script can call out to anything (a local LLM
// CLI included) and needs no new process-execution surface of its own.
//
// This module only knows how to run one script and enforce a timeout - it
// never decides what a failure means for the dictation. That policy
// ("fall back to the un-hooked text, never block delivery") lives in
// dictationCoordinator.js, which is what actually gets unit-tested against
// a fake; this file is what gets tested against real child processes.
const DEFAULT_TIMEOUT_MS = 10_000;

function createPostProcessHook({ getScriptPath, timeoutMs = DEFAULT_TIMEOUT_MS, spawnFn = spawn }) {
async function run(text) {
const scriptPath = getScriptPath();
if (!scriptPath) return text;
return runScript(spawnFn, scriptPath, text, timeoutMs);
}

return { run };
}

function runScript(spawnFn, scriptPath, text, timeoutMs) {
return new Promise((resolve, reject) => {
let child;
try {
child = spawnFn(scriptPath, [], { stdio: ["pipe", "pipe", "pipe"] });
} catch (error) {
reject(error);
return;
}

let stdout = "";
let stderr = "";
let settled = false;

const timer = setTimeout(() => {
if (settled) return;
settled = true;
child.kill("SIGKILL");
reject(new Error(`post-process hook timed out after ${timeoutMs}ms`));
}, timeoutMs);

child.stdout.on("data", (chunk) => {
stdout += chunk;
});
child.stderr.on("data", (chunk) => {
stderr += chunk;
});

// A script that isn't executable, or doesn't exist, fires this rather
// than "close" - covers the "the script got moved or deleted" case.
child.on("error", (error) => {
if (settled) return;
settled = true;
clearTimeout(timer);
reject(error);
});

child.on("close", (code) => {
if (settled) return;
settled = true;
clearTimeout(timer);
if (code !== 0) {
const detail = stderr.trim();
reject(new Error(`post-process hook exited with code ${code}${detail ? `: ${detail}` : ""}`));
return;
}
// A CLI convention, not part of the dictation - a script that prints
// with `echo`/`puts`/`print` adds exactly one trailing newline that
// was never something the user said.
resolve(stdout.replace(/\n$/, ""));
});

child.stdin.on("error", () => {
// A script that closes stdin early (reads nothing, e.g. `cat /dev/null`)
// would otherwise throw EPIPE on this write - "close" above still
// fires and decides the outcome from the exit code.
});
child.stdin.write(text);
child.stdin.end();
});
}

module.exports = { createPostProcessHook, DEFAULT_TIMEOUT_MS };
Loading
Loading