From b77040e26b76a6a76dbed6fb76c82cbf03f6a8d7 Mon Sep 17 00:00:00 2001 From: Aliou Diallo Date: Wed, 20 May 2026 12:45:27 +0200 Subject: [PATCH] refactor: remove manager alert policy --- src/manager/index.test.ts | 250 ------------------ src/manager/index.ts | 48 +--- src/manager/internal-types.ts | 117 +++++++-- src/manager/process-output-tracker.test.ts | 282 +-------------------- src/manager/process-output-tracker.ts | 221 +++------------- src/manager/process-registry.test.ts | 59 ++--- src/manager/process-registry.ts | 22 +- src/manager/process-runtime-controller.ts | 77 +----- src/protocol.ts | 4 +- src/types.ts | 40 --- tests/e2e/dynamic-log-watch.e2e.ts | 47 ---- tests/e2e/kill.e2e.ts | 5 +- tests/e2e/output-and-watches.e2e.ts | 37 +-- tests/e2e/process-crash.e2e.ts | 13 +- tests/e2e/stateful-test-watcher.e2e.ts | 56 ---- tests/e2e/utils.ts | 85 ------- 16 files changed, 185 insertions(+), 1178 deletions(-) delete mode 100644 tests/e2e/dynamic-log-watch.e2e.ts delete mode 100644 tests/e2e/stateful-test-watcher.e2e.ts diff --git a/src/manager/index.test.ts b/src/manager/index.test.ts index 86764b7..159e27a 100644 --- a/src/manager/index.test.ts +++ b/src/manager/index.test.ts @@ -205,9 +205,6 @@ describe("start/list/get basics", () => { success: null, exitCode: null, endTime: null, - alertOnSuccess: false, - alertOnFailure: true, - alertOnKill: false, }), ); expect(info.pid).toBeGreaterThan(0); @@ -235,23 +232,6 @@ describe("start/list/get basics", () => { using manager = new ProcessManager(); expect(manager.get("nonexistent")).toBeNull(); }); - - it("custom alert flags in StartOptions", () => { - using manager = new ProcessManager(); - const info = manager.start("test", "echo hi", "/tmp", { - alertOnSuccess: true, - alertOnFailure: false, - alertOnKill: true, - }); - - expect(info).toEqual( - expect.objectContaining({ - alertOnSuccess: true, - alertOnFailure: false, - alertOnKill: true, - }), - ); - }); }); // --- Process lifecycle events --- @@ -468,220 +448,6 @@ describe("killAll", () => { }); }); -// --- Watch matching --- - -describe("process_watch_matched", () => { - it("fires once by default on first matching line", async () => { - using manager = new ProcessManager(); - const events = collectEvents(manager); - - const info = manager.start( - "watch-once", - "bash -c 'echo ready; echo ready; echo ready'", - "/tmp", - { - logWatches: [{ pattern: "ready" }], - }, - ); - - await waitForEnd(manager, info.id); - - const matches = events.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(1); - - const first = matches[0]; - if (first.type === "process_watch_matched") { - expect(first.match).toEqual( - expect.objectContaining({ - processId: info.id, - source: "stdout", - line: "ready", - }), - ); - expect(first.match.watch.repeat).toBe(false); - } - }); - - it("supports repeat watches", async () => { - using manager = new ProcessManager(); - const events = collectEvents(manager); - - const info = manager.start( - "watch-repeat", - "bash -c 'echo done; echo done; echo done'", - "/tmp", - { - logWatches: [{ pattern: "done", repeat: true }], - }, - ); - - await waitForEnd(manager, info.id); - - const matches = events.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(3); - }); - - it("respects stream scoping", async () => { - using manager = new ProcessManager(); - const events = collectEvents(manager); - - const info = manager.start( - "watch-stream", - "bash -c 'echo out; echo err >&2'", - "/tmp", - { - logWatches: [{ pattern: "err", stream: "stderr" }], - }, - ); - - await waitForEnd(manager, info.id); - - const matches = events.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(1); - - const match = matches[0]; - if (match.type === "process_watch_matched") { - expect(match.match).toEqual( - expect.objectContaining({ source: "stderr", line: "err" }), - ); - } - }); - - it("stream both matches stdout and stderr", async () => { - using manager = new ProcessManager(); - const events = collectEvents(manager); - - const info = manager.start( - "watch-both", - "bash -c 'echo marker; echo marker >&2'", - "/tmp", - { - logWatches: [{ pattern: "marker", stream: "both", repeat: true }], - }, - ); - - await waitForEnd(manager, info.id); - - const matches = events.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(2); - - const sources = new Set( - matches - .filter( - (e): e is Extract => - e.type === "process_watch_matched", - ) - .map((e) => e.match.source), - ); - - expect(sources).toEqual(new Set(["stdout", "stderr"])); - }); - - it("matches trailing partial line at process end", async () => { - using manager = new ProcessManager(); - const events = collectEvents(manager); - - const info = manager.start("watch-trailing", "printf ready", "/tmp", { - logWatches: [{ pattern: "ready" }], - }); - - await waitForEnd(manager, info.id); - - const matches = events.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(1); - }); - - it("throws for invalid watch regex", () => { - using manager = new ProcessManager(); - - expect(() => - manager.start("bad-watch", "echo ok", "/tmp", { - logWatches: [{ pattern: "(", mode: "regex" }], - }), - ).toThrowError(/Invalid log watch pattern/); - }); - - it("uses literal watch matching by default", async () => { - using manager = new ProcessManager(); - const events = collectEvents(manager); - - const info = manager.start("literal-watch", "echo '('", "/tmp", { - logWatches: [{ pattern: "(" }], - }); - - await waitForEnd(manager, info.id); - - const matches = events.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(1); - }); - - it("adds log watches to a running process", () => { - using manager = new ProcessManager(); - const events = collectEvents(manager); - const info = manager.start("late-watch", "sleep 60", "/tmp"); - - expect(manager.addLogWatches(info.id, [{ pattern: "late ready" }])).toEqual( - { - ok: true, - added: 1, - }, - ); - - const child = fakeProcesses.get(info.pid); - assert(child, "fake child should exist"); - child.stdout.write("late ready\n"); - - const matches = events.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(1); - if (matches[0].type === "process_watch_matched") { - expect(matches[0].match.watch.index).toBe(0); - } - }); - - it("appends log watches after existing watches", () => { - using manager = new ProcessManager(); - const events = collectEvents(manager); - const info = manager.start("late-watch", "sleep 60", "/tmp", { - logWatches: [{ pattern: "first" }], - }); - - expect(manager.addLogWatches(info.id, [{ pattern: "second" }])).toEqual({ - ok: true, - added: 1, - }); - - const child = fakeProcesses.get(info.pid); - assert(child, "fake child should exist"); - child.stdout.write("second\n"); - - const matches = events.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(1); - if (matches[0].type === "process_watch_matched") { - expect(matches[0].match.watch.index).toBe(1); - } - }); - - it("returns not_found when adding watches to an unknown process", () => { - using manager = new ProcessManager(); - - expect(manager.addLogWatches("missing", [{ pattern: "late" }])).toEqual({ - ok: false, - reason: "not_found", - }); - }); - - it("returns process_exited when adding watches to a finished process", async () => { - using manager = new ProcessManager(); - const info = manager.start("finished", "echo hi", "/tmp"); - await waitForEnd(manager, info.id); - - expect(manager.addLogWatches(info.id, [{ pattern: "late" }])).toEqual({ - ok: false, - reason: "process_exited", - }); - }); -}); - // --- Kill --- describe("kill", () => { @@ -718,22 +484,6 @@ describe("kill", () => { const result = await manager.kill(info.id); expect(result).toEqual({ ok: true, info: expect.any(Object) }); }); - - it("sets alertOnKill to false on kill", async () => { - vi.useFakeTimers(); - using manager = new ProcessManager(); - const info = manager.start("test", "sleep 60", "/tmp", { - alertOnKill: true, - }); - - const resultPromise = manager.kill(info.id); - await vi.advanceTimersByTimeAsync(3000); - await resultPromise; - - const updated = manager.get(info.id); - assert(updated, "process should exist"); - expect(updated.alertOnKill).toBe(false); - }); }); // --- Write to stdin --- diff --git a/src/manager/index.ts b/src/manager/index.ts index 927220a..3156292 100644 --- a/src/manager/index.ts +++ b/src/manager/index.ts @@ -1,14 +1,12 @@ import { EventEmitter } from "node:events"; import type { - AddLogWatchesResult, KillResult, - LogWatch, ManagerEvent, ProcessInfo, - StartOptions, WriteResult, } from "../types"; +import { formatProcess } from "./internal-types"; import { OutputChangeNotifier } from "./output-change-notifier"; import { ProcessLogStore } from "./process-log-store"; import { ProcessOutputTracker } from "./process-output-tracker"; @@ -37,7 +35,6 @@ export class ProcessManager { this.logs = new ProcessLogStore(); this.outputTracker = new ProcessOutputTracker({ - emit, appendCombinedLine: (file, source, line) => this.logs.appendCombinedLine(file, source, line), }); @@ -64,39 +61,14 @@ export class ProcessManager { }); } - // --- Event subscription --- - onEvent(listener: (event: ManagerEvent) => void): () => void { this.events.on("event", listener); return () => this.events.off("event", listener); } - // --- Process lifecycle --- - - start( - name: string, - command: string, - cwd: string, - options?: StartOptions, - ): ProcessInfo { - const managed = this.runtime.start(name, command, cwd, options); - return { - id: managed.id, - name: managed.name, - pid: managed.pid, - command: managed.command, - cwd: managed.cwd, - startTime: managed.startTime, - endTime: managed.endTime, - status: managed.status, - exitCode: managed.exitCode, - success: managed.success, - stdoutFile: managed.stdoutFile, - stderrFile: managed.stderrFile, - alertOnSuccess: managed.alertOnSuccess, - alertOnFailure: managed.alertOnFailure, - alertOnKill: managed.alertOnKill, - }; + start(name: string, command: string, cwd: string): ProcessInfo { + const managed = this.runtime.start(name, command, cwd); + return formatProcess(managed); } list(): ProcessInfo[] { @@ -107,8 +79,6 @@ export class ProcessManager { return this.registry.getPublicInfo(id); } - // --- Output retrieval --- - getOutput( id: string, tailLines = 100, @@ -163,8 +133,6 @@ export class ProcessManager { }); } - // --- Kill operations --- - async kill( id: string, opts?: { signal?: NodeJS.Signals; timeoutMs?: number }, @@ -180,16 +148,10 @@ export class ProcessManager { return this.runtime.writeToStdin(id, data, opts); } - addLogWatches(id: string, watches: LogWatch[]): AddLogWatchesResult { - return this.runtime.addLogWatches(id, watches); - } - killAll(): void { this.runtime.killAll(); } - // --- Cleanup --- - clearFinished(): number { return this.runtime.clearFinished(); } @@ -211,9 +173,7 @@ export class ProcessManager { } export type { - AddLogWatchesResult, KillResult, - LogWatch, ManagerEvent, ProcessInfo, ProcessStatus, diff --git a/src/manager/internal-types.ts b/src/manager/internal-types.ts index bb057ae..27996cc 100644 --- a/src/manager/internal-types.ts +++ b/src/manager/internal-types.ts @@ -1,7 +1,7 @@ import type { ChildProcess } from "node:child_process"; import type { Writable } from "node:stream"; -import type { LogWatchMode, LogWatchStream, ProcessInfo } from "../types"; +import type { ProcessInfo, ProcessStatus } from "../types"; export interface ProcessLogPaths { stdoutFile: string; @@ -9,44 +9,107 @@ export interface ProcessLogPaths { combinedFile: string; } -export interface ResolvedWatch { - index: number; - pattern: string; - mode: LogWatchMode; - regex: RegExp; - stream: LogWatchStream; - repeat: boolean; - fired: boolean; +/** + * Stable process metadata and lifecycle status. + * + * This is the part of a managed process that can be safely projected into the + * public `ProcessInfo` API. It intentionally contains no Node handles, mutable + * stream buffers, or manager bookkeeping fields. + */ +interface ProcessPublicState { + id: string; + name: string; + pid: number; + command: string; + cwd: string; + startTime: number; + endTime: number | null; + status: ProcessStatus; + exitCode: number | null; + success: boolean | null; + stdoutFile: string; + stderrFile: string; } -export interface ManagedProcess extends ProcessInfo { +/** + * Process handles and signal bookkeeping used by runtime control. + * + * Only `ProcessRuntimeController` should need these fields. They are not safe + * to expose because they allow direct mutation/control outside manager methods. + */ +interface ProcessRuntimeState { process: ChildProcess; stdin: Writable | null; stdinClosed: boolean; lastSignalSent: NodeJS.Signals | null; +} + +/** + * Internal log storage details. + * + * `stdoutFile` and `stderrFile` are public because agents/users can inspect + * them. The combined log file is an implementation detail used by manager/UI + * read APIs, so it stays internal. + */ +interface ProcessLogState { combinedFile: string; +} + +/** + * Output parser state for incomplete line chunks. + * + * Node streams can split a single logical line across multiple chunks. These + * buffers let `ProcessOutputTracker` emit only completed lines in events/logs. + */ +interface ProcessLineBufferState { stdoutPendingLine: string; stderrPendingLine: string; - watches: ResolvedWatch[]; +} + +/** + * Output lines accumulated since the last `process_output_changed` event. + * + * `OutputChangeNotifier` drains this buffer when it emits an output event. This + * gives extension code live lines without re-reading log files or polling. + */ +interface ProcessOutputEventBufferState { appendedLines: Array<{ type: "stdout" | "stderr"; text: string }>; } -export function publicProcessInfo(managed: ManagedProcess): ProcessInfo { +/** + * Internal mutable record owned by `ProcessManager`. + * + * This is intentionally broader than public `ProcessInfo`: it combines public + * lifecycle state with runtime handles, log implementation details, and output + * parser buffers. Callers must receive `ProcessInfo` snapshots instead, created + * by `formatProcess()` below. + */ +export interface ManagedProcessRecord + extends ProcessPublicState, + ProcessRuntimeState, + ProcessLogState, + ProcessLineBufferState, + ProcessOutputEventBufferState {} + +/** + * Convert an internal mutable record into the public process snapshot. + * + * Keep this function explicit instead of using object spread so new internal + * fields cannot accidentally leak into the public manager API. + */ +export function formatProcess(record: ManagedProcessRecord): ProcessInfo { return { - id: managed.id, - name: managed.name, - pid: managed.pid, - command: managed.command, - cwd: managed.cwd, - startTime: managed.startTime, - endTime: managed.endTime, - status: managed.status, - exitCode: managed.exitCode, - success: managed.success, - stdoutFile: managed.stdoutFile, - stderrFile: managed.stderrFile, - alertOnSuccess: managed.alertOnSuccess, - alertOnFailure: managed.alertOnFailure, - alertOnKill: managed.alertOnKill, + id: record.id, + name: record.name, + pid: record.pid, + command: record.command, + cwd: record.cwd, + startTime: record.startTime, + endTime: record.endTime, + status: record.status, + exitCode: record.exitCode, + success: record.success, + stdoutFile: record.stdoutFile, + stderrFile: record.stderrFile, }; } diff --git a/src/manager/process-output-tracker.test.ts b/src/manager/process-output-tracker.test.ts index 4285ed8..067d281 100644 --- a/src/manager/process-output-tracker.test.ts +++ b/src/manager/process-output-tracker.test.ts @@ -1,7 +1,6 @@ import { createMock, type PartialFuncReturn } from "@golevelup/ts-vitest"; import { beforeEach, describe, expect, it } from "vitest"; -import type { ManagerEvent } from "../types"; -import type { ManagedProcess } from "./internal-types"; +import type { ManagedProcessRecord } from "./internal-types"; import { ProcessOutputTracker } from "./process-output-tracker"; const managedDefaults = { @@ -18,18 +17,14 @@ const managedDefaults = { stdoutFile: "/tmp/stdout.log", stderrFile: "/tmp/stderr.log", combinedFile: "/tmp/combined.log", - alertOnSuccess: false, - alertOnFailure: true, - alertOnKill: false, stdin: null, stdinClosed: false, lastSignalSent: null, stdoutPendingLine: "", stderrPendingLine: "", -} satisfies PartialFuncReturn; +} satisfies PartialFuncReturn; describe("ProcessOutputTracker", () => { - let emitted: ManagerEvent[]; let combinedLines: Array<{ file: string; source: "stdout" | "stderr"; @@ -37,162 +32,25 @@ describe("ProcessOutputTracker", () => { }>; beforeEach(() => { - emitted = []; combinedLines = []; }); function createTracker(): ProcessOutputTracker { return new ProcessOutputTracker({ - emit: (event) => emitted.push(event), appendCombinedLine: (file, source, line) => { combinedLines.push({ file, source, line }); }, }); } - // --- resolveLogWatches --- - - describe("resolveLogWatches", () => { - it("returns empty array for no input", () => { - using tracker = createTracker(); - expect(tracker.resolveLogWatches()).toEqual([]); - expect(tracker.resolveLogWatches([])).toEqual([]); - }); - - it("resolves a valid watch", () => { - using tracker = createTracker(); - const watches = tracker.resolveLogWatches([{ pattern: "ready" }]); - - expect(watches).toHaveLength(1); - expect(watches[0]).toEqual( - expect.objectContaining({ - index: 0, - pattern: "ready", - mode: "literal", - stream: "both", - repeat: false, - fired: false, - }), - ); - expect(watches[0].regex).toBeInstanceOf(RegExp); - }); - - it("uses startIndex for resolved watch indexes", () => { - using tracker = createTracker(); - const watches = tracker.resolveLogWatches([{ pattern: "ready" }], 3); - - expect(watches[0].index).toBe(3); - }); - - it("respects stream and repeat options", () => { - using tracker = createTracker(); - const watches = tracker.resolveLogWatches([ - { pattern: "err", stream: "stderr", repeat: true }, - ]); - - expect(watches[0]).toEqual( - expect.objectContaining({ - mode: "literal", - stream: "stderr", - repeat: true, - }), - ); - }); - - it("escapes literal watches by default", () => { - using tracker = createTracker(); - const watches = tracker.resolveLogWatches([{ pattern: "(" }]); - - expect(watches[0].regex.test("(")).toBe(true); - }); - - it("supports explicit regex watches", () => { - using tracker = createTracker(); - const watches = tracker.resolveLogWatches([ - { pattern: "r.*y", mode: "regex" }, - ]); - - expect(watches[0]).toEqual( - expect.objectContaining({ mode: "regex", pattern: "r.*y" }), - ); - expect(watches[0].regex.test("ready")).toBe(true); - }); - - it("throws for empty pattern", () => { - using tracker = createTracker(); - expect(() => tracker.resolveLogWatches([{ pattern: "" }])).toThrow( - /pattern is required/, - ); - }); - - it("throws for whitespace-only pattern", () => { - using tracker = createTracker(); - expect(() => tracker.resolveLogWatches([{ pattern: " " }])).toThrow( - /pattern is required/, - ); - }); - - it("throws for invalid regex", () => { - using tracker = createTracker(); - expect(() => - tracker.resolveLogWatches([{ pattern: "(", mode: "regex" }]), - ).toThrow(/Invalid log watch pattern/); - }); - - it("throws for invalid mode", () => { - using tracker = createTracker(); - expect(() => - tracker.resolveLogWatches([ - { pattern: "ok", mode: "invalid" as never }, - ]), - ).toThrow(/Invalid logWatches.*mode/); - }); - - it("throws for invalid stream", () => { - using tracker = createTracker(); - expect(() => - tracker.resolveLogWatches([ - { pattern: "ok", stream: "invalid" as never }, - ]), - ).toThrow(/Invalid logWatches.*stream/); - }); - - it("throws when too many watches are configured", () => { - using tracker = createTracker(); - expect(() => - tracker.resolveLogWatches( - Array.from({ length: 21 }, () => ({ pattern: "ok" })), - ), - ).toThrow(/at most 20/); - }); - - it("throws when added watches exceed the total watch limit", () => { - using tracker = createTracker(); - expect(() => - tracker.resolveLogWatches( - Array.from({ length: 2 }, () => ({ pattern: "ok" })), - 19, - ), - ).toThrow(/at most 20/); - }); - - it("throws when a watch pattern is too long", () => { - using tracker = createTracker(); - expect(() => - tracker.resolveLogWatches([{ pattern: "x".repeat(501) }]), - ).toThrow(/500 characters/); - }); - }); - // --- onStdoutChunk / onStderrChunk --- describe("chunk processing", () => { it("extracts complete stdout lines and appends to combined", () => { using tracker = createTracker(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, combinedFile: "/tmp/combined.log", - watches: [], appendedLines: [], }); @@ -210,10 +68,9 @@ describe("ProcessOutputTracker", () => { it("extracts complete stderr lines and appends to combined", () => { using tracker = createTracker(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, combinedFile: "/tmp/combined.log", - watches: [], appendedLines: [], }); @@ -229,9 +86,8 @@ describe("ProcessOutputTracker", () => { it("handles partial lines across chunks", () => { using tracker = createTracker(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, - watches: [], appendedLines: [], }); @@ -246,9 +102,8 @@ describe("ProcessOutputTracker", () => { it("handles multiple partial lines", () => { using tracker = createTracker(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, - watches: [], appendedLines: [], }); @@ -265,9 +120,8 @@ describe("ProcessOutputTracker", () => { it("keeps stderr pending separate from stdout pending", () => { using tracker = createTracker(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, - watches: [], appendedLines: [], }); @@ -284,10 +138,9 @@ describe("ProcessOutputTracker", () => { describe("flushPendingLines", () => { it("flushes pending stdout and stderr lines", () => { using tracker = createTracker(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, combinedFile: "/tmp/combined.log", - watches: [], appendedLines: [], }); managed.stdoutPendingLine = "leftover out"; @@ -309,9 +162,8 @@ describe("ProcessOutputTracker", () => { it("is no-op when no pending lines", () => { using tracker = createTracker(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, - watches: [], appendedLines: [], }); @@ -327,9 +179,8 @@ describe("ProcessOutputTracker", () => { describe("drainAppendedLines", () => { it("returns and clears appended lines", () => { using tracker = createTracker(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, - watches: [], appendedLines: [], }); managed.appendedLines = [ @@ -348,123 +199,12 @@ describe("ProcessOutputTracker", () => { it("returns undefined when no appended lines", () => { using tracker = createTracker(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, - watches: [], appendedLines: [], }); expect(tracker.drainAppendedLines(managed)).toBeUndefined(); }); }); - - // --- matchWatches --- - - describe("watch matching", () => { - it("fires watch event on matching stdout line", () => { - using tracker = createTracker(); - const managed = createMock({ - ...managedDefaults, - id: "p1", - name: "test", - command: "cmd", - watches: tracker.resolveLogWatches([{ pattern: "ready" }]), - appendedLines: [], - }); - - tracker.onStdoutChunk(managed, Buffer.from("ready\n")); - - expect(emitted).toEqual([ - expect.objectContaining({ - type: "process_watch_matched", - }), - ]); - if (emitted[0].type === "process_watch_matched") { - expect(emitted[0].match).toEqual( - expect.objectContaining({ - processId: "p1", - source: "stdout", - line: "ready", - }), - ); - } - }); - - it("fires only once by default (no repeat)", () => { - using tracker = createTracker(); - const managed = createMock({ - ...managedDefaults, - watches: tracker.resolveLogWatches([{ pattern: "go" }]), - appendedLines: [], - }); - - tracker.onStdoutChunk(managed, Buffer.from("go\ngo\ngo\n")); - - const matches = emitted.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(1); - }); - - it("fires multiple times with repeat=true", () => { - using tracker = createTracker(); - const managed = createMock({ - ...managedDefaults, - watches: tracker.resolveLogWatches([{ pattern: "go", repeat: true }]), - appendedLines: [], - }); - - tracker.onStdoutChunk(managed, Buffer.from("go\ngo\ngo\n")); - - const matches = emitted.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(3); - }); - - it("respects stream scoping (stderr only)", () => { - using tracker = createTracker(); - const managed = createMock({ - ...managedDefaults, - watches: tracker.resolveLogWatches([ - { pattern: "err", stream: "stderr" }, - ]), - appendedLines: [], - }); - - tracker.onStdoutChunk(managed, Buffer.from("err\n")); - tracker.onStderrChunk(managed, Buffer.from("err\n")); - - const matches = emitted.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(1); - if (matches[0].type === "process_watch_matched") { - expect(matches[0].match.source).toBe("stderr"); - } - }); - - it("does not run watch matching against oversized lines", () => { - using tracker = createTracker(); - const managed = createMock({ - ...managedDefaults, - watches: tracker.resolveLogWatches([{ pattern: "needle" }]), - appendedLines: [], - }); - - tracker.onStdoutChunk(managed, Buffer.from(`${"x".repeat(10_001)}\n`)); - - const matches = emitted.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(0); - }); - - it("flushPendingLines triggers watches on trailing partial", () => { - using tracker = createTracker(); - const managed = createMock({ - ...managedDefaults, - watches: tracker.resolveLogWatches([{ pattern: "partial" }]), - appendedLines: [], - }); - managed.stdoutPendingLine = "partial data"; - - tracker.flushPendingLines(managed); - - const matches = emitted.filter((e) => e.type === "process_watch_matched"); - expect(matches).toHaveLength(1); - }); - }); }); diff --git a/src/manager/process-output-tracker.ts b/src/manager/process-output-tracker.ts index e2bb285..cf66c4d 100644 --- a/src/manager/process-output-tracker.ts +++ b/src/manager/process-output-tracker.ts @@ -1,56 +1,6 @@ -import type { - LogWatch, - LogWatchMode, - LogWatchStream, - ManagerEvent, -} from "../types"; -import type { ManagedProcess, ResolvedWatch } from "./internal-types"; - -const MAX_LOG_WATCHES = 20; -const MAX_LOG_WATCH_PATTERN_LENGTH = 500; -const MAX_LOG_WATCH_MATCH_LINE_LENGTH = 10_000; - -/* - * Log watch ReDoS policy - * - * Log watches are LLM-provided input. They are not user-entered UI filters, and - * they can be evaluated against every completed stdout/stderr line for a - * long-running process. That makes native JavaScript RegExp a possible CPU sink: - * a single catastrophic pattern can stall the extension if it is tested against - * a hostile or simply unlucky log line. - * - * Current Phase 1 behavior is deliberately conservative without adding another - * dependency yet: - * - * - The default mode is "literal", not "regex". Literal mode escapes the - * pattern before compiling, so punctuation like "(" or ".*" is matched as - * text and cannot introduce backtracking behavior. - * - Regex mode is explicit: callers must pass `mode: "regex"` to request native - * RegExp semantics. - * - Pattern count, pattern length, and matched line length are bounded. These - * caps do not prove regex safety, but they reduce the blast radius until the - * stronger validator is added. - * - * Follow-up ReDoS hardening after Phase 1: - * - * 1. Add a validator in this file, at the regex-mode boundary below, before - * `new RegExp(pattern)` runs. - * 2. Preferred lightweight guard: `safe-regex2(pattern, { limit: 25 })`. - * It is a Fastify-maintained heuristic that catches common nested-repeat - * catastrophic patterns. It has false positives and false negatives, so keep - * the existing literal default and caps even after adding it. - * 3. If we need stronger diagnostics later, evaluate `redos-detector` either as - * a replacement or as a stricter/dev-only pass with explicit timeout/step - * limits. - * 4. Avoid native `re2` here unless the project accepts native build tooling. - * That is a bad fit for this repo's macOS/arm64 + Nix constraints. - * 5. Be careful with `re2js`: it gives safer linear-time matching, but it - * changes supported syntax and semantics. If adopted, it should be a - * deliberate API choice, not a silent replacement for JS regex mode. - */ +import type { ManagedProcessRecord } from "./internal-types"; interface ProcessOutputTrackerDeps { - emit: (event: ManagerEvent) => void; appendCombinedLine: ( combinedFile: string, source: "stdout" | "stderr", @@ -59,7 +9,6 @@ interface ProcessOutputTrackerDeps { } export class ProcessOutputTracker { - private emit: (event: ManagerEvent) => void; private appendCombinedLine: ( combinedFile: string, source: "stdout" | "stderr", @@ -67,195 +16,87 @@ export class ProcessOutputTracker { ) => void; constructor(deps: ProcessOutputTrackerDeps) { - this.emit = deps.emit; this.appendCombinedLine = deps.appendCombinedLine; } - resolveLogWatches(input?: LogWatch[], startIndex = 0): ResolvedWatch[] { - if (!input || input.length === 0) return []; - if (startIndex + input.length > MAX_LOG_WATCHES) { - throw new Error(`logWatches supports at most ${MAX_LOG_WATCHES} entries`); - } - - return input.map((watch, offset) => { - const index = startIndex + offset; - const pattern = watch.pattern?.trim(); - if (!pattern) { - throw new Error(`logWatches[${index}].pattern is required`); - } - if (pattern.length > MAX_LOG_WATCH_PATTERN_LENGTH) { - throw new Error( - `logWatches[${index}].pattern must be ${MAX_LOG_WATCH_PATTERN_LENGTH} characters or fewer`, - ); - } - - const mode: LogWatchMode = watch.mode ?? "literal"; - if (mode !== "literal" && mode !== "regex") { - throw new Error( - `Invalid logWatches[${index}].mode: ${mode}. Expected literal or regex`, - ); - } - - let regex: RegExp; - try { - regex = - mode === "literal" - ? new RegExp(escapeRegExp(pattern)) - : new RegExp(pattern); - } catch (error) { - const message = - error instanceof Error ? error.message : "invalid regular expression"; - throw new Error( - `Invalid log watch pattern at logWatches[${index}]: ${message}`, - ); - } - - const stream: LogWatchStream = watch.stream ?? "both"; - if (stream !== "stdout" && stream !== "stderr" && stream !== "both") { - throw new Error( - `Invalid logWatches[${index}].stream: ${stream}. Expected stdout, stderr, or both`, - ); - } - - return { - index, - pattern, - mode, - regex, - stream, - repeat: watch.repeat ?? false, - fired: false, - }; - }); - } - - onStdoutChunk(managed: ManagedProcess, data: Buffer): string[] { - const lines = this.extractCompleteLines(managed, "stdout", data); + onStdoutChunk(record: ManagedProcessRecord, data: Buffer): string[] { + const lines = this.extractCompleteLines(record, "stdout", data); for (const line of lines) { - this.appendCombinedLine(managed.combinedFile, "stdout", line); - managed.appendedLines.push({ type: "stdout", text: line }); + this.appendCombinedLine(record.combinedFile, "stdout", line); + record.appendedLines.push({ type: "stdout", text: line }); } - this.matchWatches(managed, "stdout", lines); return lines; } - onStderrChunk(managed: ManagedProcess, data: Buffer): string[] { - const lines = this.extractCompleteLines(managed, "stderr", data); + onStderrChunk(record: ManagedProcessRecord, data: Buffer): string[] { + const lines = this.extractCompleteLines(record, "stderr", data); for (const line of lines) { - this.appendCombinedLine(managed.combinedFile, "stderr", line); - managed.appendedLines.push({ type: "stderr", text: line }); + this.appendCombinedLine(record.combinedFile, "stderr", line); + record.appendedLines.push({ type: "stderr", text: line }); } - this.matchWatches(managed, "stderr", lines); return lines; } - flushPendingLines(managed: ManagedProcess): void { - if (managed.stdoutPendingLine) { + flushPendingLines(record: ManagedProcessRecord): void { + if (record.stdoutPendingLine) { this.appendCombinedLine( - managed.combinedFile, + record.combinedFile, "stdout", - managed.stdoutPendingLine, + record.stdoutPendingLine, ); - this.matchWatches(managed, "stdout", [managed.stdoutPendingLine]); - managed.appendedLines.push({ + record.appendedLines.push({ type: "stdout", - text: managed.stdoutPendingLine, + text: record.stdoutPendingLine, }); - managed.stdoutPendingLine = ""; + record.stdoutPendingLine = ""; } - if (managed.stderrPendingLine) { + if (record.stderrPendingLine) { this.appendCombinedLine( - managed.combinedFile, + record.combinedFile, "stderr", - managed.stderrPendingLine, + record.stderrPendingLine, ); - this.matchWatches(managed, "stderr", [managed.stderrPendingLine]); - managed.appendedLines.push({ + record.appendedLines.push({ type: "stderr", - text: managed.stderrPendingLine, + text: record.stderrPendingLine, }); - managed.stderrPendingLine = ""; + record.stderrPendingLine = ""; } } drainAppendedLines( - managed: ManagedProcess, + record: ManagedProcessRecord, ): Array<{ type: "stdout" | "stderr"; text: string }> | undefined { - if (managed.appendedLines.length === 0) return undefined; - const lines = managed.appendedLines; - managed.appendedLines = []; + if (record.appendedLines.length === 0) return undefined; + const lines = record.appendedLines; + record.appendedLines = []; return lines; } private extractCompleteLines( - managed: ManagedProcess, + record: ManagedProcessRecord, source: "stdout" | "stderr", data: Buffer, ): string[] { const chunk = data.toString(); const pending = - source === "stdout" - ? managed.stdoutPendingLine - : managed.stderrPendingLine; + source === "stdout" ? record.stdoutPendingLine : record.stderrPendingLine; const merged = pending + chunk; const parts = merged.split(/\r?\n/); const completeLines = parts.slice(0, -1); const nextPending = parts[parts.length - 1] ?? ""; if (source === "stdout") { - managed.stdoutPendingLine = nextPending; + record.stdoutPendingLine = nextPending; } else { - managed.stderrPendingLine = nextPending; + record.stderrPendingLine = nextPending; } return completeLines; } - private matchWatches( - managed: ManagedProcess, - source: "stdout" | "stderr", - lines: string[], - ): void { - if (managed.watches.length === 0 || lines.length === 0) return; - - for (const line of lines) { - if (line.length > MAX_LOG_WATCH_MATCH_LINE_LENGTH) continue; - - for (const watch of managed.watches) { - if (!watch.repeat && watch.fired) continue; - if (watch.stream !== "both" && watch.stream !== source) continue; - - if (!watch.regex.test(line)) continue; - - watch.fired = true; - - this.emit({ - type: "process_watch_matched", - match: { - processId: managed.id, - processName: managed.name, - processCommand: managed.command, - source, - line, - watch: { - index: watch.index, - pattern: watch.pattern, - mode: watch.mode, - stream: watch.stream, - repeat: watch.repeat, - }, - }, - }); - } - } - } - [Symbol.dispose](): void { - // No state to clean up -- watches are owned by ManagedProcess. + // No state to clean up. } } - -function escapeRegExp(pattern: string): string { - return pattern.replace(/[\\^$.*+?()[\]{}|]/g, "\\$&"); -} diff --git a/src/manager/process-registry.test.ts b/src/manager/process-registry.test.ts index 0546783..7aa0448 100644 --- a/src/manager/process-registry.test.ts +++ b/src/manager/process-registry.test.ts @@ -1,6 +1,6 @@ import { createMock, type PartialFuncReturn } from "@golevelup/ts-vitest"; import { assert, describe, expect, it } from "vitest"; -import type { ManagedProcess } from "./internal-types"; +import type { ManagedProcessRecord } from "./internal-types"; import { ProcessRegistry } from "./process-registry"; const managedDefaults = { @@ -17,17 +17,13 @@ const managedDefaults = { stdoutFile: "/tmp/stdout.log", stderrFile: "/tmp/stderr.log", combinedFile: "/tmp/combined.log", - alertOnSuccess: false, - alertOnFailure: true, - alertOnKill: false, stdin: null, stdinClosed: false, lastSignalSent: null, stdoutPendingLine: "", stderrPendingLine: "", - watches: [], appendedLines: [], -} satisfies PartialFuncReturn; +} satisfies PartialFuncReturn; describe("ProcessRegistry", () => { it("generates sequential IDs", () => { @@ -39,10 +35,9 @@ describe("ProcessRegistry", () => { it("add and getRecord", () => { using registry = new ProcessRegistry(); - const managed = createMock({ + const managed = createMock({ ...managedDefaults, id: "proc_1", - watches: [], appendedLines: [], }); registry.add(managed); @@ -54,12 +49,11 @@ describe("ProcessRegistry", () => { it("getPublicInfo returns ProcessInfo", () => { using registry = new ProcessRegistry(); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_1", name: "test", status: "running", - watches: [], appendedLines: [], }), ); @@ -83,10 +77,9 @@ describe("ProcessRegistry", () => { it("delete removes a process", () => { using registry = new ProcessRegistry(); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_1", - watches: [], appendedLines: [], }), ); @@ -100,32 +93,29 @@ describe("ProcessRegistry", () => { it("list returns processes in insertion order", () => { using registry = new ProcessRegistry(); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_1", name: "oldest", startTime: 100, - watches: [], appendedLines: [], }), ); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_2", name: "newest", startTime: 300, - watches: [], appendedLines: [], }), ); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_3", name: "middle", startTime: 200, - watches: [], appendedLines: [], }), ); @@ -140,22 +130,20 @@ describe("ProcessRegistry", () => { it("keeps insertion order when start times match", () => { using registry = new ProcessRegistry(); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_alpha", name: "first", startTime: 100, - watches: [], appendedLines: [], }), ); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_beta", name: "second", startTime: 100, - watches: [], appendedLines: [], }), ); @@ -166,10 +154,9 @@ describe("ProcessRegistry", () => { it("has checks for existence", () => { using registry = new ProcessRegistry(); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_1", - watches: [], appendedLines: [], }), ); @@ -181,11 +168,10 @@ describe("ProcessRegistry", () => { it("hasAliveishProcesses returns true when live processes exist", () => { using registry = new ProcessRegistry(); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_1", status: "running", - watches: [], appendedLines: [], }), ); @@ -196,20 +182,18 @@ describe("ProcessRegistry", () => { it("hasAliveishProcesses returns false when all are dead", () => { using registry = new ProcessRegistry(); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_1", status: "exited", - watches: [], appendedLines: [], }), ); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_2", status: "killed", - watches: [], appendedLines: [], }), ); @@ -225,29 +209,26 @@ describe("ProcessRegistry", () => { it("forEachAlive iterates only live processes", () => { using registry = new ProcessRegistry(); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_1", status: "running", - watches: [], appendedLines: [], }), ); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_2", status: "exited", - watches: [], appendedLines: [], }), ); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_3", status: "terminating", - watches: [], appendedLines: [], }), ); @@ -261,18 +242,16 @@ describe("ProcessRegistry", () => { it("values and entries iterate all processes", () => { using registry = new ProcessRegistry(); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_1", - watches: [], appendedLines: [], }), ); registry.add( - createMock({ + createMock({ ...managedDefaults, id: "proc_2", - watches: [], appendedLines: [], }), ); diff --git a/src/manager/process-registry.ts b/src/manager/process-registry.ts index b08db09..8691c05 100644 --- a/src/manager/process-registry.ts +++ b/src/manager/process-registry.ts @@ -1,26 +1,26 @@ import { LIVE_STATUSES, type ProcessInfo } from "../types"; -import type { ManagedProcess } from "./internal-types"; -import { publicProcessInfo } from "./internal-types"; +import type { ManagedProcessRecord } from "./internal-types"; +import { formatProcess } from "./internal-types"; export class ProcessRegistry { - private processes: Map = new Map(); + private processes: Map = new Map(); private counter = 0; nextId(): string { return `proc_${++this.counter}`; } - add(process: ManagedProcess): void { + add(process: ManagedProcessRecord): void { this.processes.set(process.id, process); } - getRecord(id: string): ManagedProcess | undefined { + getRecord(id: string): ManagedProcessRecord | undefined { return this.processes.get(id); } getPublicInfo(id: string): ProcessInfo | null { const managed = this.processes.get(id); - return managed ? publicProcessInfo(managed) : null; + return managed ? formatProcess(managed) : null; } delete(id: string): boolean { @@ -28,14 +28,14 @@ export class ProcessRegistry { } list(): ProcessInfo[] { - return Array.from(this.processes.values()).map((p) => publicProcessInfo(p)); + return Array.from(this.processes.values()).map((p) => formatProcess(p)); } - values(): IterableIterator { + values(): IterableIterator { return this.processes.values(); } - entries(): IterableIterator<[string, ManagedProcess]> { + entries(): IterableIterator<[string, ManagedProcessRecord]> { return this.processes.entries(); } @@ -50,7 +50,9 @@ export class ProcessRegistry { return false; } - forEachAlive(callback: (id: string, managed: ManagedProcess) => void): void { + forEachAlive( + callback: (id: string, managed: ManagedProcessRecord) => void, + ): void { for (const [id, managed] of this.processes) { if (LIVE_STATUSES.has(managed.status)) { callback(id, managed); diff --git a/src/manager/process-runtime-controller.ts b/src/manager/process-runtime-controller.ts index fdbe51c..78cba0b 100644 --- a/src/manager/process-runtime-controller.ts +++ b/src/manager/process-runtime-controller.ts @@ -1,17 +1,11 @@ import type { ChildProcess } from "node:child_process"; -import type { - AddLogWatchesResult, - KillResult, - LogWatch, - StartOptions, - WriteResult, -} from "../types"; +import type { KillResult, WriteResult } from "../types"; import { LIVE_STATUSES } from "../types"; import { isProcessGroupAlive, killProcessGroup } from "../utils"; import { spawnCommand } from "../utils/command-executor"; -import type { ManagedProcess } from "./internal-types"; -import { publicProcessInfo } from "./internal-types"; +import type { ManagedProcessRecord } from "./internal-types"; +import { formatProcess } from "./internal-types"; import type { OutputChangeNotifier } from "./output-change-notifier"; import type { ProcessLogStore } from "./process-log-store"; import type { ProcessOutputTracker } from "./process-output-tracker"; @@ -47,22 +41,14 @@ export class ProcessRuntimeController { this.getConfiguredShellPath = deps.getConfiguredShellPath; } - start( - name: string, - command: string, - cwd: string, - options?: StartOptions, - ): ManagedProcess { - const resolvedWatches = this.outputTracker.resolveLogWatches( - options?.logWatches, - ); + start(name: string, command: string, cwd: string): ManagedProcessRecord { const id = this.registry.nextId(); const logPaths = this.logs.createLogs(id); const child = spawnCommand(command, cwd, this.getConfiguredShellPath()); child.unref(); - const managed: ManagedProcess = { + const managed: ManagedProcessRecord = { id, name, pid: child.pid ?? -1, @@ -76,16 +62,12 @@ export class ProcessRuntimeController { stdoutFile: logPaths.stdoutFile, stderrFile: logPaths.stderrFile, combinedFile: logPaths.combinedFile, - alertOnSuccess: options?.alertOnSuccess ?? false, - alertOnFailure: options?.alertOnFailure ?? true, - alertOnKill: options?.alertOnKill ?? false, process: child, stdin: child.stdin, stdinClosed: false, lastSignalSent: null, stdoutPendingLine: "", stderrPendingLine: "", - watches: resolvedWatches, appendedLines: [], }; @@ -102,18 +84,18 @@ export class ProcessRuntimeController { this.wireStdioHandlers(managed, child); - this.emit({ type: "process_started", info: publicProcessInfo(managed) }); + this.emit({ type: "process_started", info: formatProcess(managed) }); this.ensureWatcherRunning(); return managed; } - transition(managed: ManagedProcess, next: typeof managed.status): void { + transition(managed: ManagedProcessRecord, next: typeof managed.status): void { if (managed.status === next) return; managed.status = next; if (next === "exited" || next === "killed") { - this.emit({ type: "process_ended", info: publicProcessInfo(managed) }); + this.emit({ type: "process_ended", info: formatProcess(managed) }); } this.ensureWatcherRunning(); @@ -141,9 +123,6 @@ export class ProcessRuntimeController { success: false, stdoutFile: "", stderrFile: "", - alertOnSuccess: false, - alertOnFailure: true, - alertOnKill: false, }, reason: "not_found", }; @@ -152,10 +131,8 @@ export class ProcessRuntimeController { const signal = opts?.signal ?? "SIGTERM"; const timeoutMs = opts?.timeoutMs ?? 3000; - managed.alertOnKill = false; - if (!LIVE_STATUSES.has(managed.status)) { - return { ok: true, info: publicProcessInfo(managed) }; + return { ok: true, info: formatProcess(managed) }; } this.transition(managed, "terminating"); @@ -168,7 +145,7 @@ export class ProcessRuntimeController { if (err.code !== "EPERM") { return { ok: false, - info: publicProcessInfo(managed), + info: formatProcess(managed), reason: "error", }; } @@ -183,7 +160,7 @@ export class ProcessRuntimeController { this.transition(managed, "terminate_timeout"); return { ok: false, - info: publicProcessInfo(managed), + info: formatProcess(managed), reason: "timeout", }; } @@ -197,7 +174,7 @@ export class ProcessRuntimeController { this.outputTracker.flushPendingLines(managed); this.outputNotifier.flush(id); this.transition(managed, "killed"); - return { ok: true, info: publicProcessInfo(managed) }; + return { ok: true, info: formatProcess(managed) }; } killAll(): void { @@ -255,34 +232,6 @@ export class ProcessRuntimeController { } } - addLogWatches(id: string, watches: LogWatch[]): AddLogWatchesResult { - const managed = this.registry.getRecord(id); - if (!managed) { - return { - ok: false, - reason: "not_found", - }; - } - - if (!LIVE_STATUSES.has(managed.status)) { - return { - ok: false, - reason: "process_exited", - }; - } - - const resolved = this.outputTracker.resolveLogWatches( - watches, - managed.watches.length, - ); - managed.watches.push(...resolved); - - return { - ok: true, - added: resolved.length, - }; - } - clearFinished(): number { let cleared = 0; for (const [id, managed] of this.registry.entries()) { @@ -336,7 +285,7 @@ export class ProcessRuntimeController { } private wireStdioHandlers( - managed: ManagedProcess, + managed: ManagedProcessRecord, child: ChildProcess, ): void { child.stdout?.on("data", (data: Buffer) => { diff --git a/src/protocol.ts b/src/protocol.ts index 50f3bb7..ebc9e3a 100644 --- a/src/protocol.ts +++ b/src/protocol.ts @@ -1,11 +1,10 @@ -import type { KillResult, LogWatchMatchEvent, ProcessInfo } from "./types"; +import type { KillResult, ProcessInfo } from "./types"; export const CHANNELS = { // Core broadcasts STARTED: "processes:started", ENDED: "processes:ended", OUTPUT_CHANGED: "processes:output_changed", - WATCH_MATCHED: "processes:watch_matched", CHANGED: "processes:changed", // Request channels (UI -> core, sync callback) @@ -35,7 +34,6 @@ export type ProcessesOutputChangedPayload = { id: string; appendedText?: Array<{ type: "stdout" | "stderr"; text: string }>; }; -export type ProcessesWatchMatchedPayload = LogWatchMatchEvent; export type ProcessesChangedPayload = { reason: "started" | "ended" | "cleared"; }; diff --git a/src/types.ts b/src/types.ts index d088cd3..1d1fb0b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -11,27 +11,6 @@ export const LIVE_STATUSES: ReadonlySet = new Set([ "terminate_timeout", ]); -export type LogWatchStream = "stdout" | "stderr" | "both"; -export type LogWatchMode = "literal" | "regex"; - -export interface LogWatch { - pattern: string; - mode?: LogWatchMode; - stream?: LogWatchStream; - repeat?: boolean; -} - -export interface StartOptions { - alertOnSuccess?: boolean; // default false - alertOnFailure?: boolean; // default true - alertOnKill?: boolean; // default false - logWatches?: LogWatch[]; -} - -export type AddLogWatchesResult = - | { ok: true; added: number } - | { ok: false; reason: "not_found" | "process_exited" }; - export interface ProcessInfo { id: string; name: string; @@ -45,24 +24,6 @@ export interface ProcessInfo { success: boolean | null; // null if running, true if exit code 0, false otherwise stdoutFile: string; stderrFile: string; - alertOnSuccess: boolean; - alertOnFailure: boolean; - alertOnKill: boolean; -} - -export interface LogWatchMatchEvent { - processId: string; - processName: string; - processCommand: string; - source: "stdout" | "stderr"; - line: string; - watch: { - index: number; - pattern: string; - mode: LogWatchMode; - stream: LogWatchStream; - repeat: boolean; - }; } export type ManagerEvent = @@ -73,7 +34,6 @@ export type ManagerEvent = id: string; appendedText?: Array<{ type: "stdout" | "stderr"; text: string }>; } - | { type: "process_watch_matched"; match: LogWatchMatchEvent } | { type: "processes_changed" }; export type KillResult = diff --git a/tests/e2e/dynamic-log-watch.e2e.ts b/tests/e2e/dynamic-log-watch.e2e.ts deleted file mode 100644 index 24c7a95..0000000 --- a/tests/e2e/dynamic-log-watch.e2e.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { expect } from "vitest"; -import { getManager } from "../../src/get-manager"; -import { test } from "./fixtures"; -import { collectEvents, waitForEnd, waitForWatchMatch } from "./utils"; - -test("adds log watches to a real process while it is running", async ({ - cwd, - addFile, - addScript, -}) => { - using manager = getManager(); - const events = collectEvents(manager); - addScript("wait-for-file.sh"); - - const info = manager.start( - "dynamic-watch", - './wait-for-file.sh release-output "dynamic ready"', - cwd, - ); - - expect( - manager.addLogWatches(info.id, [ - { pattern: "dynamic ready", stream: "stdout" }, - ]), - ).toEqual({ - ok: true, - added: 1, - }); - - addFile("release-output"); - - const match = await waitForWatchMatch( - manager, - events, - info.id, - (candidate) => candidate.line === "dynamic ready", - ); - const ended = await waitForEnd(manager, info.id); - - expect(match.watch).toEqual( - expect.objectContaining({ - pattern: "dynamic ready", - index: 0, - }), - ); - expect(ended.success).toBe(true); -}); diff --git a/tests/e2e/kill.e2e.ts b/tests/e2e/kill.e2e.ts index a94c3a1..5329a5e 100644 --- a/tests/e2e/kill.e2e.ts +++ b/tests/e2e/kill.e2e.ts @@ -13,9 +13,7 @@ test("kills a real running process and clears finished logs", async ({ const events = collectEvents(manager); addScript("wait-for-file.sh"); - const info = manager.start("kill-target", "./wait-for-file.sh never", cwd, { - alertOnKill: true, - }); + const info = manager.start("kill-target", "./wait-for-file.sh never", cwd); const files = manager.getLogFiles(info.id); assert(files, "log files should exist"); @@ -26,7 +24,6 @@ test("kills a real running process and clears finished logs", async ({ expect(result.ok).toBe(true); if (result.ok) { expect(result.info.status).toBe("killed"); - expect(result.info.alertOnKill).toBe(false); } expect(events.some((event) => event.type === "process_ended")).toBe(true); diff --git a/tests/e2e/output-and-watches.e2e.ts b/tests/e2e/output-and-watches.e2e.ts index 52f36fd..0d79887 100644 --- a/tests/e2e/output-and-watches.e2e.ts +++ b/tests/e2e/output-and-watches.e2e.ts @@ -6,7 +6,7 @@ import type { ManagerEvent } from "../../src/types"; import { test } from "./fixtures"; import { collectEvents, waitForEnd } from "./utils"; -test("runs a real process and records logs, events, output, and watches", async ({ +test("runs a real process and records logs, events, and output", async ({ cwd, addScript, }) => { @@ -14,16 +14,7 @@ test("runs a real process and records logs, events, output, and watches", async const events = collectEvents(manager); addScript("emit-output.sh"); - const info = manager.start("real-output", "./emit-output.sh", cwd, { - alertOnSuccess: true, - alertOnFailure: false, - alertOnKill: true, - logWatches: [ - { pattern: "server ready on http://localhost:3000" }, - { pattern: "TypeError|ReferenceError", mode: "regex", stream: "stderr" }, - { pattern: "job completed", stream: "stdout", repeat: true }, - ], - }); + const info = manager.start("real-output", "./emit-output.sh", cwd); const ended = await waitForEnd(manager, info.id); @@ -33,9 +24,6 @@ test("runs a real process and records logs, events, output, and watches", async status: "exited", exitCode: 0, success: true, - alertOnSuccess: true, - alertOnFailure: false, - alertOnKill: true, }), ); @@ -98,27 +86,6 @@ test("runs a real process and records logs, events, output, and watches", async ]), ); - const watchMatches = events.filter( - ( - event, - ): event is Extract => - event.type === "process_watch_matched", - ); - expect(watchMatches).toHaveLength(3); - expect(watchMatches.map((event) => event.match.watch.pattern)).toEqual( - expect.arrayContaining([ - "server ready on http://localhost:3000", - "TypeError|ReferenceError", - "job completed", - ]), - ); - expect( - watchMatches.find( - (event) => - event.match.watch.pattern === "server ready on http://localhost:3000", - )?.match.watch.mode, - ).toBe("literal"); - expect(events.some((event) => event.type === "process_started")).toBe(true); expect(events.some((event) => event.type === "process_ended")).toBe(true); }); diff --git a/tests/e2e/process-crash.e2e.ts b/tests/e2e/process-crash.e2e.ts index 70e3dfc..bbae1bb 100644 --- a/tests/e2e/process-crash.e2e.ts +++ b/tests/e2e/process-crash.e2e.ts @@ -1,7 +1,7 @@ import { assert, expect } from "vitest"; import { getManager } from "../../src/get-manager"; import { test } from "./fixtures"; -import { collectEvents, waitForEnd, waitForWatchMatch } from "./utils"; +import { waitForEnd } from "./utils"; test("records a real process that fails by itself", async ({ cwd, @@ -9,29 +9,18 @@ test("records a real process that fails by itself", async ({ addScript, }) => { using manager = getManager(); - const events = collectEvents(manager); addScript("crash-on-file.sh"); const info = manager.start( "self-failing-worker", "bash ./crash-on-file.sh crash-now", cwd, - { - logWatches: [{ pattern: "fatal: marker", stream: "stderr" }], - }, ); addFile("crash-now"); - const match = await waitForWatchMatch( - manager, - events, - info.id, - (candidate) => candidate.line === "fatal: marker crash-now detected", - ); const ended = await waitForEnd(manager, info.id); - expect(match.source).toBe("stderr"); expect(ended).toEqual( expect.objectContaining({ status: "exited", diff --git a/tests/e2e/stateful-test-watcher.e2e.ts b/tests/e2e/stateful-test-watcher.e2e.ts deleted file mode 100644 index 28236fe..0000000 --- a/tests/e2e/stateful-test-watcher.e2e.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { expect } from "vitest"; -import { getManager } from "../../src/get-manager"; -import { test } from "./fixtures"; -import { collectEvents } from "./utils"; - -test("tracks a stateful test watcher as files fix failures", async ({ - cwd, - addFile, - addScript, -}) => { - using manager = getManager(); - const events = collectEvents(manager); - addScript("stateful-test-watcher.mjs"); - - const info = manager.start( - "stateful-tests", - "node ./stateful-test-watcher.mjs", - cwd, - { - logWatches: [ - { pattern: "FAIL ", stream: "stdout", repeat: true }, - { pattern: "PASS ", stream: "stdout", repeat: true }, - ], - }, - ); - - await expect(manager).toHaveLine( - events, - info.id, - "FAIL missing table: customers", - ); - - addFile("01-migrated"); - await expect(manager).toHaveLine(events, info.id, "PASS 01-migrated"); - await expect(manager).toHaveLine( - events, - info.id, - "FAIL missing seed data: orders", - ); - - addFile("02-seeded"); - await expect(manager).toHaveLine(events, info.id, "PASS 02-seeded"); - await expect(manager).toHaveLine( - events, - info.id, - "FAIL missing shipping calculator", - ); - - addFile("03-shipping"); - await expect(manager).toHaveLine(events, info.id, "PASS 03-shipping"); - await expect(manager).toHaveLine(events, info.id, "PASS all watched tests"); - - const result = await manager.kill(info.id, { signal: "SIGKILL" }); - - expect(result.ok).toBe(true); -}); diff --git a/tests/e2e/utils.ts b/tests/e2e/utils.ts index 498afea..f3c119a 100644 --- a/tests/e2e/utils.ts +++ b/tests/e2e/utils.ts @@ -1,54 +1,10 @@ -import { expect } from "vitest"; import type { ProcessManager } from "../../src/manager"; import { LIVE_STATUSES, - type LogWatchMatchEvent, type ManagerEvent, type ProcessInfo, } from "../../src/types"; -declare module "vitest" { - interface Matchers { - toHaveLine( - events: ManagerEvent[], - processId: string, - line: string, - ): T extends Promise ? Promise : Promise; - } -} - -expect.extend({ - async toHaveLine( - manager: ProcessManager, - events: ManagerEvent[], - processId: string, - line: string, - ) { - try { - await waitForWatchMatch( - manager, - events, - processId, - (match) => match.line === line, - ); - - return { - pass: true, - message: () => - `expected process ${processId} not to emit watched line ${this.utils.printExpected(line)}`, - }; - } catch (error) { - return { - pass: false, - message: () => - error instanceof Error - ? error.message - : `expected process ${processId} to emit watched line ${this.utils.printExpected(line)}`, - }; - } - }, -}); - export function collectEvents(manager: ProcessManager): ManagerEvent[] { const events: ManagerEvent[] = []; manager.onEvent((event) => events.push(event)); @@ -112,44 +68,3 @@ export async function waitForEndedCount( }); }); } - -export async function waitForWatchMatch( - manager: ProcessManager, - events: ManagerEvent[], - processId: string, - predicate: (match: LogWatchMatchEvent) => boolean, -): Promise { - const existing = findWatchMatch(events, processId, predicate); - if (existing) return existing; - - return new Promise((resolve, reject) => { - const timeout = setTimeout(() => { - unsubscribe(); - reject(new Error(`Timed out waiting for watch match on ${processId}`)); - }, 5000); - - const unsubscribe = manager.onEvent((event) => { - if (event.type !== "process_watch_matched") return; - if (event.match.processId !== processId) return; - if (!predicate(event.match)) return; - - clearTimeout(timeout); - unsubscribe(); - resolve(event.match); - }); - }); -} - -function findWatchMatch( - events: ManagerEvent[], - processId: string, - predicate: (match: LogWatchMatchEvent) => boolean, -): LogWatchMatchEvent | null { - for (const event of events) { - if (event.type !== "process_watch_matched") continue; - if (event.match.processId !== processId) continue; - if (predicate(event.match)) return event.match; - } - - return null; -} -- 2.51.2