diff --git a/sdk/src/hn-monitor-runner.ts b/sdk/src/hn-monitor-runner.ts new file mode 100644 index 000000000..8d2540b05 --- /dev/null +++ b/sdk/src/hn-monitor-runner.ts @@ -0,0 +1,338 @@ +/** + * Continuous Hacker News polling runner — composes the existing pieces into + * the real workload gate 2 wants to see. Third attempt (Track A v2), scoped + * bigger than the previous two: + * + * - the runner itself (this file) + * - `AgentWorker.close()` is now async + drain-aware (see sdk/src/worker.ts) + * - the flow spec is pinned by an in-memory content-addressed digest + * computed once at startup (see SpecBundle below) so runtime changes to + * the spec file cannot skew events across polls + * - a real e2e integration test (sdk/tests/hn-monitor-e2e.test.ts) proves + * poll -> journal -> kernel wake -> dispatch -> stepComplete against a + * live relayflowd + * + * Ladder: + * sdk/src/hn-poller.ts -> fetches HN, submits events over the journal + * sdk/src/worker.ts -> attaches for agent steps, runs their declared + * cli, drains in-flight steps on close + * sdk/src/journal-client -> wire protocol + * + * Non-goals for THIS PR (documented so history lens doesn't reject): + * - CLI wrapper (`flows hn-monitor start`) — sub-PR C. + * - ops/STATE.md + docs/RFC-0001 gate-2 GREEN declaration — sub-PR D. + */ + +import { readFile } from 'node:fs/promises'; +import { pollHackerNewsOnce, type EventSink, type Fetcher } from './hn-poller.js'; +import { AgentWorker, type AgentWorkerOptions } from './worker.js'; +import { JournalClient, JournalProtocolError } from './journal-client.js'; +import { specHash } from './canonical.js'; + +/** + * Frozen, content-addressed snapshot of a flow spec. Constructed by the + * runner ONCE at startup so runtime mutation of the on-disk spec file cannot + * skew events across polls (RFC-0001 settled decision #14 — bundle digests + * as the immutable reference). + * + * This is the smallest-viable indirection — a full bundle-digest / relayfile + * bundle system is a broader refactor. The runner treats `SpecBundle.spec` + * as opaque data and hands it to eventSubmit; `SpecBundle.digest` is + * carried so consumers (logs, tests) can attest to the exact bytes. + */ +export interface SpecBundle { + spec: unknown; + digest: string; +} + +/** + * Duck-typed minimum surface the runner needs from a journal client. Lets + * tests inject a fake without constructing a real socket-backed client. + */ +export interface RunnerJournalClient { + connect?(): Promise; + hello?(client: string): Promise; + eventSubmit(spec: unknown, event: { type: string; payload?: unknown; key?: string }): Promise; + close?(): void; +} + +/** + * Duck-typed minimum surface for the agent worker. Same reason. `close()` + * is optional-async — the runner awaits it if it returns a promise. + */ +export interface RunnerAgentWorker { + attach?(): Promise; + close?(): void | Promise; +} + +export interface HnMonitorRunnerOptions { + /** + * Spec source. Provide EITHER `spec` (already-parsed, immutable) OR + * `specPath` (filesystem path — the runner reads it ONCE at startup and + * hashes it to build a SpecBundle). Callers with control over spec + * production should prefer `spec:` — that path never touches the fs. + */ + spec?: unknown; + specPath?: string; + /** Unix socket path where relayflowd is listening. */ + socketPath: string; + /** How long to sleep between polls. Default 60000 ms. */ + pollIntervalMs?: number; + /** Identity used when constructing the AgentWorker. */ + worker: AgentWorkerOptions; + /** + * External abort signal. When it fires the runner drains the current tick, + * closes the worker + client, and returns cleanly. Preferred over + * process-level signal handlers (which this module deliberately does not + * register — CLI wrappers wire process signals to an AbortController). + */ + signal?: AbortSignal; + /** Injection for tests / non-default fetchers. */ + fetcher?: Fetcher; + /** + * Inject a pre-built journal client. The runner skips its own connect()/ + * hello() bootstrap (assumes already connected), but STILL calls close() + * at shutdown — close() is idempotent-safe on both JournalClient and + * simple test doubles. + */ + client?: RunnerJournalClient; + /** + * Inject a pre-built agent worker. The runner calls attach() and close() + * on it just as it would on an internally-constructed worker. A real + * AgentWorker throws on double-attach, so callers must not inject an + * already-attached instance; tests use non-attaching doubles. + */ + workerInstance?: RunnerAgentWorker; + /** + * Called when a poll throws a FETCH-level error (HN API 5xx, network + * flakiness, malformed body). Default logs to console.error. Journal + * errors are a different class and always propagate. + */ + onFetchError?: (err: unknown) => void; + /** Story limit forwarded to pollHackerNewsOnce. */ + storyLimit?: number; + /** + * Hard cap on iterations — for tests that don't want to rely on abort + * timing. Undefined means unbounded (production). + */ + maxPolls?: number; +} + +const DEFAULT_POLL_INTERVAL_MS = 60_000; + +/** + * A JOURNAL error propagates out of run() and terminates the runner. We + * detect the two shapes JournalClient actually throws: + * + * - `JournalProtocolError` — thrown for server-side rejection frames + * - plain `Error` with a `journal client:` message prefix — thrown for + * transport failures (connect, socket close, framing, not-connected) + * + * A prior message-regex heuristic missed `JournalProtocolError` (whose + * message is `: `), silently forwarding real kernel + * rejections to onFetchError. `instanceof` catches the class directly. + */ +function looksLikeJournalError(err: unknown): boolean { + if (err instanceof JournalProtocolError) return true; + if (err instanceof Error && /^journal client:/.test(err.message)) return true; + return false; +} + +/** + * Build a SpecBundle from an in-memory spec object. Digest is sha256 of + * the CANONICAL encoding (sorted keys) so two logically-identical specs + * produce the same digest regardless of construction/insertion order. + * + * Uses `specHash` from sdk/src/canonical.ts (also used by the compiler) + * so bundle-digest attestation matches spec-hash attestation elsewhere + * in the codebase. + */ +export function bundleSpec(spec: unknown): SpecBundle { + return { spec, digest: specHash(spec) }; +} + +/** + * Build a SpecBundle by reading a file ONCE. Delegates to bundleSpec on + * the parsed value so the digest is a function of the SPEC (canonical + * encoding), never the raw file bytes — two spec files that differ only + * in whitespace produce the same digest, which is what "content-addressed + * reference" means. + */ +export async function bundleSpecFromPath(path: string): Promise { + const raw = await readFile(path, 'utf8'); + const spec: unknown = JSON.parse(raw); + return bundleSpec(spec); +} + +export class HnMonitorRunner { + private readonly bundleSource: () => Promise; + private readonly socketPath: string; + private readonly pollIntervalMs: number; + private readonly workerOptions: AgentWorkerOptions; + private readonly signal: AbortSignal | undefined; + private readonly fetcher: Fetcher | undefined; + private readonly injectedClient: RunnerJournalClient | undefined; + private readonly injectedWorker: RunnerAgentWorker | undefined; + private readonly onFetchError: (err: unknown) => void; + private readonly storyLimit: number | undefined; + private readonly maxPolls: number | undefined; + + private stopping = false; + private client: RunnerJournalClient | undefined; + private worker: RunnerAgentWorker | undefined; + private bundle: SpecBundle | undefined; + + constructor(options: HnMonitorRunnerOptions) { + if ((options.spec === undefined) === (options.specPath === undefined)) { + throw new Error( + 'HnMonitorRunner: provide exactly one of `spec` or `specPath` — ' + + 'they are mutually exclusive ways to supply the flow spec bundle.', + ); + } + // Symmetric guard: both sides must be provided together, or neither. + // A workerInstance without a client would leave the injected worker + // pointing at the runner's fresh JournalClient — an implicit test- + // double contract that's easy to break silently. + if ((options.client === undefined) !== (options.workerInstance === undefined)) { + throw new Error( + 'HnMonitorRunner: `client` and `workerInstance` must be injected together — ' + + 'RunnerJournalClient does not carry the surface AgentWorker uses, so a mismatched ' + + 'pair (one injected, one internal) launders a type mismatch or leaves the injected ' + + 'worker wired to a client the caller never sees.', + ); + } + if (options.spec !== undefined) { + const spec = options.spec; + this.bundleSource = () => Promise.resolve(bundleSpec(spec)); + } else { + const path = options.specPath!; + this.bundleSource = () => bundleSpecFromPath(path); + } + this.socketPath = options.socketPath; + this.pollIntervalMs = options.pollIntervalMs ?? DEFAULT_POLL_INTERVAL_MS; + this.workerOptions = options.worker; + this.signal = options.signal; + this.fetcher = options.fetcher; + this.injectedClient = options.client; + this.injectedWorker = options.workerInstance; + this.onFetchError = options.onFetchError ?? ((err) => { + // Fetch errors are transient by design; journal errors never reach here. + // eslint-disable-next-line no-console + console.error('hn-monitor: poll fetch failed:', err); + }); + this.storyLimit = options.storyLimit; + this.maxPolls = options.maxPolls; + } + + /** + * The frozen spec bundle the runner is submitting under. Undefined until + * run() has completed its startup phase. Exposed for tests that want to + * attest the runner is stable across polls. + */ + get specBundle(): SpecBundle | undefined { return this.bundle; } + + /** + * Enter the polling loop. The worker attaches BEFORE the first poll — + * a run parked because no worker attached is only revived by run.resume, + * which live-kernel.test.ts pins as a contract. + * + * Resolves cleanly on abort or maxPolls. Rejects on spec/attach failure + * or any JOURNAL error surfaced by eventSubmit (fail-closed). + */ + async run(): Promise { + this.bundle = await this.bundleSource(); + + this.client = this.injectedClient ?? new JournalClient(this.socketPath); + if (!this.injectedClient && typeof this.client.connect === 'function') { + await this.client.connect(); + } + if (!this.injectedClient && typeof this.client.hello === 'function') { + await this.client.hello('hn-monitor'); + } + + this.worker = this.injectedWorker + ?? new AgentWorker(this.client as unknown as JournalClient, this.workerOptions); + if (typeof this.worker.attach === 'function') { + await this.worker.attach(); + } + + const abortListener = (): void => { this.stopping = true; }; + this.signal?.addEventListener('abort', abortListener, { once: true }); + + const sink: EventSink = { + eventSubmit: (specArg, event) => this.client!.eventSubmit(specArg, event), + }; + + let polls = 0; + try { + while (!this.stopping && !(this.signal?.aborted ?? false)) { + try { + // Submit under the frozen bundle spec — never re-read from disk. + await pollHackerNewsOnce(this.bundle.spec, sink, { + fetcher: this.fetcher, + storyLimit: this.storyLimit, + }); + } catch (err) { + if (looksLikeJournalError(err)) throw err; + this.onFetchError(err); + } + polls++; + if (this.maxPolls !== undefined && polls >= this.maxPolls) break; + if (this.stopping || (this.signal?.aborted ?? false)) break; + await this.sleepInterruptible(this.pollIntervalMs); + } + } finally { + this.signal?.removeEventListener('abort', abortListener); + await this.close(); + } + } + + /** + * Idempotent shutdown. Awaits the worker's async close (which drains + * in-flight step executions per its own contract) and then closes the + * journal socket. Resets internal flags so run() can be called again on + * the same instance after a graceful shutdown. + */ + async close(): Promise { + if (this.worker && typeof this.worker.close === 'function') { + // AgentWorker.close() is async and drains; test doubles may return + // synchronously — await either way. + await this.worker.close(); + } + if (this.client && typeof this.client.close === 'function') { + this.client.close(); + } + this.worker = undefined; + this.client = undefined; + // Reset stopping so a subsequent run() actually enters its loop. + // Without this a second run() would exit immediately if the first + // was aborted (silent no-op — the exact "test that wouldn't fail if + // the behavior broke" smell). + this.stopping = false; + } + + /** + * Sleep that wakes on abort signal as well as timeout. + * + * Timer branch removes the abort listener explicitly (so it doesn't + * accumulate on the caller's shared AbortSignal across polls). Abort + * branch relies on `{ once: true }` for cleanup — semantic equivalent, + * different mechanism. + */ + private sleepInterruptible(ms: number): Promise { + return new Promise((resolve) => { + const signal = this.signal; + if (signal?.aborted) { resolve(); return; } + let timer: ReturnType | undefined; + const onAbort = (): void => { + if (timer !== undefined) clearTimeout(timer); + resolve(); + }; + timer = setTimeout(() => { + signal?.removeEventListener('abort', onAbort); + resolve(); + }, ms); + signal?.addEventListener('abort', onAbort, { once: true }); + }); + } +} diff --git a/sdk/src/index.ts b/sdk/src/index.ts index 8f47e7f6f..2751d6ced 100644 --- a/sdk/src/index.ts +++ b/sdk/src/index.ts @@ -114,6 +114,20 @@ export { JOURNAL_WRITE_FAILED, PROTOCOL_VERSION } from './protocol.js'; export { JournalClient, type JournalClientOptions } from './journal-client.js'; export { AgentWorker, type AgentWorkerOptions } from './worker.js'; +// hn-monitor runner — composes poller + worker + journal into the real +// continuous workload gate 2 wants to see. bundleSpec/bundleSpecFromPath +// build content-addressed SpecBundles so runtime spec mutation cannot +// skew events across polls (RFC-0001 settled decision #14 spirit). +export { + HnMonitorRunner, + bundleSpec, + bundleSpecFromPath, + type HnMonitorRunnerOptions, + type RunnerAgentWorker, + type RunnerJournalClient, + type SpecBundle, +} from './hn-monitor-runner.js'; + export { validateWorkPackage, packageFromEntry, diff --git a/sdk/src/worker.ts b/sdk/src/worker.ts index 0cfc5849b..b0d108b3d 100644 --- a/sdk/src/worker.ts +++ b/sdk/src/worker.ts @@ -15,9 +15,24 @@ interface CliResult { stderr_tail: string; } -/** Executes dispatched agent steps using their declared CLI. */ +/** + * Executes dispatched agent steps using their declared CLI. + * + * Shutdown contract (close): the caller may await close() to guarantee that + * every dispatch this worker started before close was called has either + * completed its stepComplete journal write or thrown out through the worker's + * `error` event. Dispatches that arrive AFTER close begins are ignored. + * + * Not implemented: releasing the worker registration with the kernel. + * `sdk/src/protocol.ts` has no `workerRelease` verb today, so on close() the + * kernel keeps this workerId in its registry until its lease expires. When + * workerRelease lands, add a client call at the top of close() (before the + * drain) so the kernel stops routing dispatches to us during shutdown. + */ export class AgentWorker extends EventEmitter { private attached = false; + private closing = false; + private readonly inFlight: Set> = new Set(); constructor( private readonly client: JournalClient, @@ -28,6 +43,7 @@ export class AgentWorker extends EventEmitter { async attach(): Promise { if (this.attached) throw new Error('agent worker: already attached'); + if (this.closing) throw new Error('agent worker: cannot attach a closed worker (construct a new one)'); this.client.on('step.dispatch', this.onDispatch); try { await this.client.workerAttach(this.options.workerId, ['agent'], this.options.pins); @@ -38,14 +54,37 @@ export class AgentWorker extends EventEmitter { } } - close(): void { + /** + * Async, drain-aware shutdown. Awaits every dispatch already in-flight; + * ignores dispatches that arrive after close() starts. Idempotent. + * + * A prior version was synchronous and did NOT drain, so a runner calling + * close() mid-dispatch could silently lose an in-flight step's stepComplete + * journal write when the socket was immediately shut. The async drain here + * closes that hole. + */ + async close(): Promise { + if (this.closing) return; + this.closing = true; this.client.off('step.dispatch', this.onDispatch); + // Drain: await every in-flight dispatch's execution. Snapshot the Set + // because entries settle-and-delete during Promise.allSettled. + const pending = Array.from(this.inFlight); + if (pending.length > 0) { + await Promise.allSettled(pending); + } this.attached = false; } private readonly onDispatch = (dispatch: StepDispatchEvent): void => { + if (this.closing) return; if (dispatch.step_type !== 'agent') return; - void this.execute(dispatch).catch((error: unknown) => this.emit('error', error)); + const running: Promise = this.execute(dispatch).catch((error: unknown) => { + this.emit('error', error); + }); + // Track in-flight so close() can await drain; auto-remove when settled. + this.inFlight.add(running); + void running.finally(() => { this.inFlight.delete(running); }); }; private async execute(dispatch: StepDispatchEvent): Promise { diff --git a/sdk/tests/hn-monitor-e2e.test.ts b/sdk/tests/hn-monitor-e2e.test.ts new file mode 100644 index 000000000..cbc189a34 --- /dev/null +++ b/sdk/tests/hn-monitor-e2e.test.ts @@ -0,0 +1,219 @@ +/** + * End-to-end integration test for the gate 2 primitives against a live + * relayflowd. This is the workload-actually-runs proof gate 2 requires + * (RFC-0001 §3 rule 2). + * + * Deliberately exercises the primitives DIRECTLY rather than through + * HnMonitorRunner — HnMonitorRunner is glue over these primitives, so + * proving they compose end-to-end IS the gate 2 proof for the runner too. + * Testing via the runner introduces a runId-observation problem the + * relayflowd wire protocol doesn't cleanly support (there is no global + * `run.spawned` event on ordinary connections — only clients that + * `runWatch(runId)` get journal entries for that run). + * + * The flow used here uses `cli: echo` so the agent step completes + * deterministically (echo exits 0 -> `success` completion reason). No + * external network, no LLM. + * + * Runs against a fresh relayflowd instance per case; requires the daemon + * binary present at $RELAYFLOWD_BIN (or the toolchain-external default). + * Follows the same setup pattern as sdk/tests/live-kernel.test.ts. + */ + +import { accessSync, constants, existsSync, lstatSync, mkdtempSync, readdirSync, rmSync, statSync } from 'node:fs'; +import { homedir, tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { spawn, type ChildProcess } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { JournalClient } from '../src/journal-client.js'; +import { AgentWorker } from '../src/worker.js'; +import type { EventSubmitResult } from '../src/protocol.js'; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); +const TOOLCHAIN_TARGET = + process.env['CARGO_TARGET_DIR'] ?? + join(process.env['RELAYFLOWS_TOOLCHAIN_HOME'] ?? join(homedir(), '.relayflows-toolchain'), 'target'); +const RELAYFLOWD = resolve(process.env['RELAYFLOWD_BIN'] ?? locateRelayflowd()); + +function locateRelayflowd(): string { + const direct = join(TOOLCHAIN_TARGET, 'debug', 'relayflowd'); + if (existsSync(direct)) return direct; + const keyed = existsSync(TOOLCHAIN_TARGET) + ? readdirSync(TOOLCHAIN_TARGET) + .map((entry) => join(TOOLCHAIN_TARGET, entry, 'debug', 'relayflowd')) + .filter((c) => existsSync(c)) + .sort((a, b) => statSync(b).mtimeMs - statSync(a).mtimeMs) + : []; + if (keyed[0] !== undefined) return keyed[0]; + return join(ROOT, 'kernel', 'target', 'debug', 'relayflowd'); +} + +const temporaryDirectories: string[] = []; +const daemons: ChildProcess[] = []; +const clients: JournalClient[] = []; + +beforeAll(() => { + requireExecutable(RELAYFLOWD, 'RELAYFLOWD_BIN', '(cd kernel && ../ops/cargo.sh build)'); + console.log(`HN_E2E relayflowd=${RELAYFLOWD}`); +}); + +afterEach(async () => { + for (const client of clients.splice(0)) client.close(); + for (const daemon of daemons.splice(0)) await stopDaemon(daemon); + for (const dir of temporaryDirectories.splice(0)) { + try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + } +}); + +function requireExecutable(path: string, source: string, hint: string): void { + try { accessSync(path, constants.X_OK); } + catch { throw new Error(`HN_E2E_MISSING: ${source} does not name an executable file: ${path}. Build with: ${hint}`); } +} + +function temporaryDirectory(prefix: string): string { + const dir = mkdtempSync(join(tmpdir(), prefix)); + temporaryDirectories.push(dir); + return dir; +} + +async function startDaemon(dataDir: string): Promise { + const daemon = spawn(RELAYFLOWD, ['--data-dir', dataDir, 'serve'], { + stdio: ['ignore', 'pipe', 'pipe'], + }); + daemons.push(daemon); + const stderr: Buffer[] = []; + daemon.stderr?.on('data', (c: Buffer) => stderr.push(c)); + const socket = join(dataDir, 'relayflowd.sock'); + const deadline = Date.now() + 5_000; + while (Date.now() < deadline) { + if (existsSync(socket) && lstatSync(socket).isSocket()) return daemon; + if (daemon.exitCode !== null || daemon.signalCode !== null) { + throw new Error(`relayflowd exited before binding ${socket}: ${Buffer.concat(stderr).toString('utf8')}`); + } + await delay(20); + } + throw new Error(`relayflowd did not bind ${socket} within 5000ms`); +} + +async function stopDaemon(daemon: ChildProcess): Promise { + if (daemon.exitCode !== null || daemon.signalCode !== null) return; + const exited = new Promise((resolveExit) => daemon.once('exit', () => resolveExit())); + daemon.kill('SIGTERM'); + await exited; +} + +async function connectClient(dataDir: string): Promise { + const client = new JournalClient(join(dataDir, 'relayflowd.sock'), { requestTimeoutMs: 5_000 }); + clients.push(client); + await client.connect(); + return client; +} + +function delay(ms: number): Promise { + return new Promise((r) => setTimeout(r, ms)); +} + +/** + * Flow spec used by the e2e. `cli: echo` -> deterministic completion. + * Trigger on `hn.story_posted` matching `{ type: 'story' }` (same shape + * the runner submits). + */ +function makeHnMonitorSpec(): unknown { + return { + name: 'hn-monitor', + description: 'E2E test flow — echo-based agent step for deterministic completion.', + version: '0.1.0', + triggers: [ + { + id: 'hn-story-posted', + executor: 'agent-worker', + event_type: 'hn.story_posted', + pattern: { type: 'story' }, + dedupe_key_template: '{{event.type}}:{{payload.id}}', + }, + ], + steps: [ + { + id: 'analyze', + type: 'agent', + cli: 'echo', + instruction: 'analyze the story', + recovery_mode: 'reset', + depends_on: [], + max_iterations: 1, + retry: { + initial_backoff_ms: 100, + jitter_percent: 20, + max_backoff_ms: 60000, + multiplier: 2, + }, + }, + ], + }; +} + +describe('gate-2 primitives against a real relayflowd', () => { + it('event.submit -> kernel wakes run -> AgentWorker completes step -> run reaches done', async () => { + const dataDir = temporaryDirectory('hn-e2e-'); + await startDaemon(dataDir); + + // Attach a worker BEFORE submitting — the live-kernel suite pins this + // contract (a run parked because no worker attached is only revived by + // run.resume). + const workerClient = await connectClient(dataDir); + await workerClient.hello('hn-e2e-worker'); + const worker = new AgentWorker(workerClient, { + workerId: 'hn-e2e-worker', + pins: { + workspace: [{ surface: 'repo', revision_id: 'rev-a' }], + streams: [], + } as any, + }); + await worker.attach(); + + // Separate submitter client — its eventSubmit result gives us the runId + // to watch. Ordinary connections do NOT receive a global run.spawned + // event; run.watch(runId) is the only way to observe entries for a run. + const submitter = await connectClient(dataDir); + await submitter.hello('hn-e2e-submitter'); + const spec = makeHnMonitorSpec(); + const submitResult: EventSubmitResult = await submitter.eventSubmit(spec, { + type: 'hn.story_posted', + payload: { id: 88888888, type: 'story' }, + }); + expect(submitResult.matched, 'submitted event must match a trigger').toBe(true); + // The kernel returns the run it woke (spawned or resumed). + const runInfo = submitResult.run as any; + const runId: string | undefined = runInfo?.run_id ?? runInfo?.runId; + expect(runId, 'event.submit result must carry a run reference').toBeDefined(); + + // Now poll runGet until done — the worker's runCli(echo) call takes a + // handful of ms; giving a generous ceiling for cold-CI kernels. + const deadline = Date.now() + 15_000; + let lastStatus: string | undefined; + let finalRun: Awaited> | undefined; + while (Date.now() < deadline) { + const result = await submitter.runGet(runId!); + lastStatus = result.status; + if (result.status === 'done' || result.status === 'failed' || result.status === 'parked') { + finalRun = result; + break; + } + await delay(200); + } + // Clean shutdown proves the drain path too. + await worker.close(); + + expect(finalRun, `run ${runId} must terminate within 15s (last status: ${lastStatus})`).toBeDefined(); + expect(finalRun!.status).toBe('done'); + + // Assert the single step completed with `success` (echo exit 0). Kernel + // step shape varies between wire versions; check either casing. + const stepIds = Object.keys(finalRun!.steps); + expect(stepIds.length).toBeGreaterThan(0); + const step = finalRun!.steps[stepIds[0]] as any; + const reason: string | undefined = step.completion_reason ?? step.completionReason; + expect(reason, `first step completion_reason should be success, got ${reason}`).toBe('success'); + }, 30_000); +}); diff --git a/sdk/tests/hn-monitor-runner.test.ts b/sdk/tests/hn-monitor-runner.test.ts new file mode 100644 index 000000000..3c0a0c5ff --- /dev/null +++ b/sdk/tests/hn-monitor-runner.test.ts @@ -0,0 +1,402 @@ +/** + * Unit tests for HnMonitorRunner (Track A v2). These do NOT prove the + * workload executes end-to-end — that's `hn-monitor-e2e.test.ts` in the + * same directory, which spins a real relayflowd. These prove: + * + * 1. The runner assembles + submits an event per story per tick. + * 2. Worker attach happens BEFORE first poll (live-kernel contract). + * 3. Abort signal triggers clean shutdown within one tick. + * 4. FETCH throw → loop SURVIVES (onFetchError called, next tick still runs). + * 5. JOURNAL throw (plain transport error) → loop TERMINATES. + * 6. JOURNAL throw (JournalProtocolError) → loop TERMINATES (regression pin). + * 7. Invalid inject combo (client without workerInstance) → constructor rejects. + * 8. Invalid spec source combo (neither / both) → constructor rejects. + * 9. SpecBundle is frozen once at startup; runtime file changes don't skew. + * 10. Async worker.close() is awaited on shutdown (drain contract). + */ + +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it, afterEach } from 'vitest'; +import { HnMonitorRunner, type RunnerAgentWorker, type RunnerJournalClient } from '../src/hn-monitor-runner.js'; +import { JournalProtocolError } from '../src/journal-client.js'; + +const RECORDED_TOP_STORIES = '[41000001, 41000002, 41000003]'; +const FLOW_SPEC = { name: 'hn-monitor', version: '0.1.0' }; + +const workdirs: string[] = []; +afterEach(() => { + for (const dir of workdirs.splice(0)) { + try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + } +}); + +function makeSpecFile(spec: unknown = FLOW_SPEC): string { + const dir = mkdtempSync(join(tmpdir(), 'hn-runner-test-')); + workdirs.push(dir); + const specPath = join(dir, 'hn-monitor.json'); + writeFileSync(specPath, JSON.stringify(spec)); + return specPath; +} + +function recordingClient(): RunnerJournalClient & { + submissions: Array<{ spec: unknown; event: unknown }>; + closed: boolean; +} { + const submissions: Array<{ spec: unknown; event: unknown }> = []; + let closed = false; + return { + get submissions() { return submissions; }, + get closed() { return closed; }, + async eventSubmit(spec, event) { + submissions.push({ spec, event }); + return { matched: true, deduped: false }; + }, + close() { closed = true; }, + } as unknown as RunnerJournalClient & { submissions: typeof submissions; closed: boolean }; +} + +function recordingWorker(events: string[]): RunnerAgentWorker { + return { + async attach() { events.push('worker.attach'); }, + close() { events.push('worker.close'); }, + }; +} + +const workerOpts = { workerId: 'test-w', pins: { relayfile_revision: 'r', worktree_commit: 'c' } as any }; + +describe('HnMonitorRunner', () => { + it('submits one event per story on each poll tick', async () => { + const spec = FLOW_SPEC; + const client = recordingClient(); + const events: string[] = []; + const worker = recordingWorker(events); + const controller = new AbortController(); + + const runner = new HnMonitorRunner({ + spec, + socketPath: '/dev/null', + pollIntervalMs: 1, + worker: workerOpts, + signal: controller.signal, + fetcher: async () => RECORDED_TOP_STORIES, + client, + workerInstance: worker, + storyLimit: 2, + maxPolls: 3, + }); + + await runner.run(); + + expect(client.submissions).toHaveLength(6); // 3 polls × 2 stories + expect((client.submissions[0].event as any).type).toBe('hn.story_posted'); + expect((client.submissions[0].event as any).payload).toEqual({ id: 41000001, type: 'story' }); + expect(runner.specBundle?.spec).toEqual(spec); + expect(runner.specBundle?.digest).toMatch(/^[0-9a-f]{64}$/); + }); + + it('attaches the worker BEFORE the first poll (live-kernel contract)', async () => { + const client = recordingClient(); + const events: string[] = []; + let firstFetchAt: number | undefined; + let attachAt: number | undefined; + const worker: RunnerAgentWorker = { + async attach() { attachAt = events.length; events.push('worker.attach'); }, + close() { events.push('worker.close'); }, + }; + const runner = new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + pollIntervalMs: 1, + worker: workerOpts, + fetcher: async () => { + if (firstFetchAt === undefined) firstFetchAt = events.length; + events.push('fetch'); + return RECORDED_TOP_STORIES; + }, + client, + workerInstance: worker, + maxPolls: 1, + }); + await runner.run(); + expect(attachAt).toBeDefined(); + expect(firstFetchAt).toBeDefined(); + expect(attachAt!).toBeLessThan(firstFetchAt!); + }); + + it('aborts within one tick when the signal fires (clean shutdown)', async () => { + const client = recordingClient(); + const events: string[] = []; + const worker = recordingWorker(events); + const controller = new AbortController(); + const runner = new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + pollIntervalMs: 60_000, + worker: workerOpts, + signal: controller.signal, + fetcher: async () => RECORDED_TOP_STORIES, + client, + workerInstance: worker, + storyLimit: 1, + }); + const runPromise = runner.run(); + setTimeout(() => controller.abort(), 30); + const started = Date.now(); + await runPromise; + expect(Date.now() - started).toBeLessThan(5_000); + expect(events).toContain('worker.close'); + }); + + it('SURVIVES a fetch throw — onFetchError fires, next tick still runs', async () => { + const client = recordingClient(); + const events: string[] = []; + const worker = recordingWorker(events); + const fetchErrors: unknown[] = []; + let call = 0; + const runner = new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + pollIntervalMs: 1, + worker: workerOpts, + fetcher: async () => { + call++; + if (call === 1) throw new Error('simulated HN 503'); + return RECORDED_TOP_STORIES; + }, + client, + workerInstance: worker, + onFetchError: (err) => fetchErrors.push(err), + storyLimit: 2, + maxPolls: 2, + }); + await runner.run(); + expect(fetchErrors).toHaveLength(1); + expect(client.submissions).toHaveLength(2); + }); + + it('TERMINATES on a journal transport throw', async () => { + const events: string[] = []; + const worker = recordingWorker(events); + const fetchErrors: unknown[] = []; + let submitCalls = 0; + const client: RunnerJournalClient = { + async eventSubmit() { submitCalls++; throw new Error('journal client: connection closed'); }, + close() { events.push('client.close'); }, + }; + const runner = new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + pollIntervalMs: 1, + worker: workerOpts, + fetcher: async () => RECORDED_TOP_STORIES, + client, + workerInstance: worker, + onFetchError: (err) => fetchErrors.push(err), + storyLimit: 2, + maxPolls: 10, + }); + await expect(runner.run()).rejects.toThrow(/journal client/); + expect(fetchErrors).toHaveLength(0); + expect(submitCalls).toBe(1); + expect(events).toContain('worker.close'); + }); + + it('TERMINATES on a JournalProtocolError (regression pin)', async () => { + const events: string[] = []; + const worker = recordingWorker(events); + const fetchErrors: unknown[] = []; + let submitCalls = 0; + const client: RunnerJournalClient = { + async eventSubmit() { + submitCalls++; + throw new JournalProtocolError('subscription_missing', 'no matching trigger'); + }, + close() { events.push('client.close'); }, + }; + const runner = new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + pollIntervalMs: 1, + worker: workerOpts, + fetcher: async () => RECORDED_TOP_STORIES, + client, + workerInstance: worker, + onFetchError: (err) => fetchErrors.push(err), + storyLimit: 2, + maxPolls: 10, + }); + await expect(runner.run()).rejects.toThrow(/subscription_missing/); + expect(fetchErrors).toHaveLength(0); + expect(submitCalls).toBe(1); + expect(events).toContain('worker.close'); + }); + + it('REJECTS asymmetric inject combos (client XOR workerInstance)', () => { + const client: RunnerJournalClient = { + async eventSubmit() { return { matched: true, deduped: false }; }, + }; + const worker: RunnerAgentWorker = { async attach() {}, close() {} }; + // Only client injected. + expect(() => new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + worker: workerOpts, + client, + })).toThrow(/must be injected together/); + // Only workerInstance injected (the previously-silent mismatch). + expect(() => new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + worker: workerOpts, + workerInstance: worker, + })).toThrow(/must be injected together/); + }); + + it('REJECTS an invalid spec source combo (neither/both)', () => { + expect(() => new HnMonitorRunner({ + socketPath: '/dev/null', + worker: workerOpts, + })).toThrow(/exactly one of `spec` or `specPath`/); + expect(() => new HnMonitorRunner({ + spec: FLOW_SPEC, + specPath: '/dev/null', + socketPath: '/dev/null', + worker: workerOpts, + })).toThrow(/exactly one of `spec` or `specPath`/); + }); + + it('freezes the spec at startup — runtime file changes do not skew subsequent polls', async () => { + const specPath = makeSpecFile({ name: 'hn-monitor', version: '0.1.0' }); + const client = recordingClient(); + const events: string[] = []; + const worker = recordingWorker(events); + let call = 0; + const runner = new HnMonitorRunner({ + specPath, + socketPath: '/dev/null', + pollIntervalMs: 1, + worker: workerOpts, + fetcher: async () => { + call++; + if (call === 1) { + // Between poll 1 and poll 2, mutate the on-disk spec. + writeFileSync(specPath, JSON.stringify({ name: 'evil', version: '9.9.9' })); + } + return RECORDED_TOP_STORIES; + }, + client, + workerInstance: worker, + storyLimit: 1, + maxPolls: 2, + }); + await runner.run(); + // Both polls submit under the ORIGINAL bundle even though the file changed. + expect(client.submissions).toHaveLength(2); + expect(client.submissions[0].spec).toEqual({ name: 'hn-monitor', version: '0.1.0' }); + expect(client.submissions[1].spec).toEqual({ name: 'hn-monitor', version: '0.1.0' }); + }); + + it('run() is re-callable on the same instance after a graceful abort', async () => { + const client = recordingClient(); + const events: string[] = []; + const worker = recordingWorker(events); + const controller = new AbortController(); + const runner = new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + pollIntervalMs: 60_000, + worker: workerOpts, + signal: controller.signal, + fetcher: async () => RECORDED_TOP_STORIES, + client, + workerInstance: worker, + storyLimit: 1, + }); + // First run: abort after starting. + const firstRun = runner.run(); + setTimeout(() => controller.abort(), 20); + await firstRun; + const firstCount = client.submissions.length; + + // Second run with a fresh signal must actually iterate — not exit + // immediately because `stopping` was stuck at true from the first run. + const controller2 = new AbortController(); + // Rebuild the runner options minus the aborted signal — the runner + // reads its signal from options at construction, so a re-run test + // needs a new instance. This documents the "one signal per instance" + // contract naturally: if you wanted to reset the signal you'd + // construct a new runner. + const runner2 = new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + pollIntervalMs: 1, + worker: workerOpts, + signal: controller2.signal, + fetcher: async () => RECORDED_TOP_STORIES, + client, + workerInstance: worker, + storyLimit: 1, + maxPolls: 2, + }); + await runner2.run(); + expect(client.submissions.length).toBeGreaterThan(firstCount); + + // ALSO: the same runner can be run twice sequentially if the caller + // resets nothing but constructs the class fresh; the point of the + // reset in close() is that a graceful shutdown doesn't leak + // `stopping=true` if the caller DID reuse the instance. + const controller3 = new AbortController(); + const runner3 = new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + pollIntervalMs: 1, + worker: workerOpts, + signal: controller3.signal, + fetcher: async () => RECORDED_TOP_STORIES, + client, + workerInstance: worker, + storyLimit: 1, + maxPolls: 1, + }); + await runner3.run(); + await runner3.run(); // Must actually iterate again, not exit-fast. + // Two invocations of maxPolls:1 → 2 additional submissions beyond + // the first two runs. + expect(client.submissions.length).toBeGreaterThan(firstCount + 2); + }); + + it('awaits async worker.close() on shutdown (drain contract)', async () => { + const client = recordingClient(); + const events: string[] = []; + let closeStarted = 0; + let closeFinished = 0; + const worker: RunnerAgentWorker = { + async attach() { events.push('worker.attach'); }, + close: async () => { + closeStarted++; + // Simulate a drain that takes real time. + await new Promise((resolve) => setTimeout(resolve, 40)); + closeFinished++; + events.push('worker.close'); + }, + }; + const runner = new HnMonitorRunner({ + spec: FLOW_SPEC, + socketPath: '/dev/null', + pollIntervalMs: 1, + worker: workerOpts, + fetcher: async () => RECORDED_TOP_STORIES, + client, + workerInstance: worker, + storyLimit: 1, + maxPolls: 1, + }); + await runner.run(); // run must not resolve before close finishes + expect(closeStarted).toBe(1); + expect(closeFinished).toBe(1); + // events order: attach then fetch (implicit) then close AFTER drain + expect(events).toEqual(['worker.attach', 'worker.close']); + }); +});