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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ops/NEEDS_HUMAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Git metadata required

The worktree's `.git` file points to `/home/daytona/.project-git`, but that
directory does not exist. Please either restore that Git directory mount or
provide the correct `gitdir` path. Without the repository metadata, the
definition-of-done command `git status --porcelain` cannot run, and creating a
new repository here would fabricate the baseline rather than verify the real
worktree.
136 changes: 76 additions & 60 deletions ops/NEXT.md
Original file line number Diff line number Diff line change
@@ -1,87 +1,103 @@
# NEXT — work package for this tick
# Work package — Gate 3: hn-monitor runner (sub-PR A)

**Scope:** Build a minimal agent worker in the SDK. CODE task, SDK-side.
**Pinned to:** Gate 3

This run is pinned to **gate 3** and must not work on any other gate.
**Scope (quoted from ops/TARGET.md):**

## Objective
> Build sub-PR A of the Gate 2 push: a real `hn-monitor` polling runner in the SDK. CODE task, `sdk/src/`-side. This is a scaffolding PR — proof that the workload EXECUTES end-to-end is deliberately deferred to sub-PR B (integration test). Do not conflate the two.

**Context:**

RFC-0001 §3 gate 2 is done when "hn-monitor runs as a relayflow in production, triggered by its real events, with zero bespoke persistence." Every primitive already exists in this repo:
- Event triggers (PR #14, `kernel/relayflowd/tests/event_wake.rs`)
- The flow spec (`testdata/hn-monitor.flow.yaml`)
- The poller (`sdk/src/hn-poller.ts`)
- The agent worker (`sdk/src/worker.ts` from PR #53)
- A one-shot demo (`sdk/src/demo-hn-monitor.ts`)

Promote the throwaway worker the tests already build into a real SDK component
that can execute agent steps by running their declared CLI as a subprocess.
Nothing has run them together as a continuous workload. This PR fixes that.

## Context
**Prior attempt (PR #83, closed):** produced a functional runner but was rejected by the swarm on five real findings. This attempt must address them all:

Nothing in this repo can execute an agent step. Searching for `workerAttach` /
`step.complete` finds only TESTS (`sdk/tests/live-kernel.test.ts`,
`journal-client.test.ts`, `journal-client-loopback.ts`) and the protocol
definitions. `sdk/src/cli/run.ts` only OBSERVES worker leases and waits for one
that never arrives.
1. **Fail-closed on journal errors.** #83's `catch (err) { onPollError(err) }` swallowed EVERY error including `eventSubmit` journal failures — violates covenant 2 (fail-closed) and RFC-0001 §1. Only fetch-level errors (network flakiness, HN API rate limits) may be swallowed; a journal write failure MUST throw and terminate the runner. Split: `try { fetch } catch { onFetchError }` around the network call, `try { eventSubmit } catch { rethrow }` around the journal call.

The kernel's dispatch, lease and claim machinery is real and tested. The worker
side of the protocol is simply unimplemented, and that is what blocks gate 2
("a workload RUNS as a relayflow" — today a run can only be shown CREATED) and
gate 3 ("every claim/lease/retry served by the kernel").
2. **AgentWorker.close() must release the worker (or explicitly document it does not).** #83 added `await worker.close()` to shutdown but the current `close()` only drains local promises — it does NOT tell the kernel to release the worker registration. Either:
- Add a `workerRelease` verb to `sdk/src/protocol.ts` and call it from `close()` (preferred — completes the shutdown contract), OR
- Add a one-line comment on `close()` naming exactly what shutdown intentionally does NOT do

`sdk/tests/live-kernel.test.ts` around the `live-manual-agent` case (line 288)
shows the whole shape: connect, `hello`, `workerAttach` with pins, receive
`step.dispatch`, act, complete. The protocol is already proven there.
3. **Class field declaration order.** #83 declared `private readonly fetcher` AFTER the constructor. Works today because of ES2022 hoisting semantics but breaks silently if someone adds `= someDefault` to a declaration. Declare ALL fields at the top of the class body, before the constructor.

4. **Signal handlers must be opt-in via AbortSignal.** #83 registered `SIGTERM`/`SIGINT` handlers on the process directly with no opt-out. A library user embedding this can't cancel one runner without affecting others. Accept `signal?: AbortSignal` in options; the CLI wrapper (sub-PR C) can create + wire a process-signal-driven AbortController.

5. **Test coverage for pollError branch.** #83's tests never asserted the loop survives a fetcher throw AND the loop TERMINATES on a journal throw. Add both cases; without them, someone regresses `onPollError` to a no-op and every test still passes.

## Objective

Implement `sdk/src/hn-monitor-runner.ts` that composes existing primitives into a continuous polling runner that:
- Constructs a `JournalClient` connected to the running `relayflowd` socket
- Constructs an `AgentWorker` and calls `workerAttach()` for `agent` steps — attach BEFORE first poll
- Loops: `pollHackerNewsOnce(spec, sink)` → sleep `POLL_INTERVAL_MS` (env-configurable, default 60000 = 60s) → repeat
- Exits cleanly on `AbortSignal.abort` (drain in-flight steps, close client, release worker per finding #2)
- Exported from `sdk/src/index.ts`

Keep it small and honest:
- The worker must attach BEFORE the first poll
- The poller layer handles single-fetch failures with a typed error; the loop just moves to the next tick — but journal errors MUST fail the runner (finding #1)
- No scheduling logic beyond the sleep (the kernel owns retry and dedupe policy)
- No LLM calls; the runner is glue, not a reviewer

## Files in scope

- `sdk/src/worker.ts` — new file, the worker implementation
- `sdk/src/index.ts` — export the worker
- `sdk/tests/live-kernel.test.ts` OR a new test file — add a test that runs a
real flow with an agent step end to end against a live `relayflowd`, with
this worker attached, and asserts the step reaches `done`.
- `sdk/src/hn-monitor-runner.ts` (NEW)
- `sdk/src/worker.ts` (modify `close()` per finding #2)
- `sdk/src/protocol.ts` (add `workerRelease` request/response if we choose that option for finding #2)
- `sdk/src/index.ts` (export `HnMonitorRunner`)
- `sdk/tests/hn-monitor-runner.test.ts` (NEW)

## Definition of done

ALL of the following must hold:
All of the following must be true and demonstrated with captured output:

1. `sdk/src/hn-monitor-runner.ts` exists and exports `HnMonitorRunner` from `sdk/src/index.ts`

2. `sdk/src/worker.ts` — either `close()` calls `workerRelease` (add to protocol.ts if missing), OR a one-line comment names what close() intentionally does NOT do

1. The worker in `sdk/src/worker.ts`, exported from `sdk/src/index.ts`
3. `sdk/src/protocol.ts` — if `workerRelease` is added, matching request/response definitions

2. A test that runs a real flow with an agent step end to end against a live
`relayflowd`, with this worker attached, and asserts the step reaches
`done`. `sdk/tests/live-kernel.test.ts` already starts a daemon — follow
that pattern.
4. `sdk/tests/hn-monitor-runner.test.ts` covers ALL of these:
- Fake fetch + mock journal client → runner submits an event on each tick
- Abort signal triggers clean shutdown within one tick (worker released or documented)
- Worker attach happens before first poll
- **Fetch throw → loop survives** (onPollError called, next tick still runs)
- **Journal throw → loop TERMINATES** (runner.run() rejects with the error)

3. **The worker must attach BEFORE the run starts.** A run that finds no worker
parks, and attaching afterwards does not re-drive it — `run.resume` is what
picks a parked run back up. That contract is pinned in the live-kernel
suite; do not fight it.
5. `cd sdk && npm test` green (pretest hook builds the kernel automatically)

4. The worker must:
- attach for `agent` steps with the pins it holds
- on `step.dispatch`, run the step's declared `cli` as a subprocess
- report the result back through the existing protocol (`step.complete`, and
the failure path when the CLI exits nonzero)
- nothing speculative: no retries of its own, no scheduling, no LLM calls.
The kernel owns retry and lease policy — do not reimplement it.
6. EVERY new test confirmed to FAIL against current code (comment out the source; the test fails), with the literal failing output pasted in this summary

5. `cd sdk && npm test` must be green. Run it and paste the literal command and
output tail showing test counts.
7. PR body explicitly names the non-goals:
- Test-actually-runs is sub-PR B (integration test with real relayflowd)
- CLI wrapper is sub-PR C
- Gate-2 declaration is sub-PR D

6. `cd kernel && sh ../ops/cargo.sh test` must be green. Run it and paste the
literal command and output tail showing test counts.
8. As the LAST action, run `git status --porcelain` and paste it

7. EVERY new test confirmed to FAIL against current code, with the literal
failing output quoted in the summary.
## Explicit non-goals for THIS PR (belongs to later sub-PRs)

8. As your LAST action, run `git status --porcelain` and paste it.
- Proving the workload actually executes end-to-end (dispatch → step complete). That is sub-PR B (integration test with real relayflowd + fake HN fetch + assert step reaches `done`). This PR ONLY proves the runner assembles and its unit tests hold.
- CLI wrapper (`flows hn-monitor start`). That is sub-PR C.
- Ops/STATE.md gate-2 GREEN declaration. That is sub-PR D.

## Explicitly OUT of scope
## Out of scope for THIS tick — DO NOT TOUCH

- LLM steps — not in the gate 3 scope
- Retry logic in the worker — the kernel owns retry policy
- Scheduling or lease management — the kernel owns lease policy
- Optimizations, abstractions, or speculative features
- Changes to the kernel
- Changes to existing tests (except adding new test cases)
- Work on any gate other than gate 3
- `.github/workflows/*` — no GHA changes
- `kernel/*` — the kernel side of gate 2 already works via PR #14
- `workflows/*.yaml` — those are for later sub-PRs
- `ops/AUTODRIVE_BRIEF.md` — chief owns this file, not the drive loop
- CLI wrapper — sub-PR C, separate PR
- End-to-end integration test with real relayflowd — sub-PR B, separate PR
- ops/STATE.md gate-2 declaration — sub-PR D, separate PR

## If blocked

If gate 3 is genuinely unreachable from the current state, write
ops/NEEDS_HUMAN.md saying exactly why and still end with ASSESS_DONE. Do not
silently substitute different work: a run that reports progress on the wrong
gate is worse than one that reports it is blocked.
If genuinely blocked on a decision only a human can make, write ops/NEEDS_HUMAN.md with the exact question and the options — then still end with ASSESS_DONE. Do not silently substitute different work.
133 changes: 133 additions & 0 deletions sdk/src/hn-monitor-runner.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import type { Pins } from './protocol.js';
import { JournalClient } from './journal-client.js';
import { AgentWorker } from './worker.js';
import {
pollHackerNewsOnce,
type EventSink,
type Fetcher,
} from './hn-poller.js';

const DEFAULT_POLL_INTERVAL_MS = 60_000;

interface RunnerClient extends EventSink {
connect(): Promise<void>;
hello(client: string): Promise<unknown>;
close(): void;
}

interface RunnerWorker {
attach(): Promise<void>;
close(): Promise<void>;
}

export interface HnMonitorRunnerOptions {
socketPath: string;
spec: unknown;
workerId: string;
pins: Pins;
signal?: AbortSignal;
pollIntervalMs?: number;
storyLimit?: number;
fetcher?: Fetcher;
onPollError?: (error: HnFetchError) => void;
client?: RunnerClient;
worker?: RunnerWorker;
}

export class HnFetchError extends Error {
readonly cause: unknown;

constructor(cause: unknown) {
super(`HN fetch failed: ${errorMessage(cause)}`);
this.name = 'HnFetchError';
this.cause = cause;
}
}

/** Continuously submits Hacker News events and hosts their agent worker. */
export class HnMonitorRunner {
private readonly client: RunnerClient;
private readonly worker: RunnerWorker;
private readonly fetcher: Fetcher;
private readonly pollIntervalMs: number;
private readonly options: HnMonitorRunnerOptions;

constructor(options: HnMonitorRunnerOptions) {
this.options = options;
const client = options.client ?? new JournalClient(options.socketPath);
this.client = client;
this.worker = options.worker ?? new AgentWorker(client as JournalClient, {
workerId: options.workerId,
pins: options.pins,
});
this.fetcher = wrapFetcher(options.fetcher ?? fetchText);
this.pollIntervalMs = options.pollIntervalMs ?? intervalFromEnvironment();
}

async run(): Promise<void> {
await this.client.connect();
try {
await this.client.hello('hn-monitor-runner');
await this.worker.attach();
while (!this.options.signal?.aborted) {
try {
await pollHackerNewsOnce(this.options.spec, this.client, {
fetcher: this.fetcher,
storyLimit: this.options.storyLimit,
});
} catch (error) {
if (!(error instanceof HnFetchError)) throw error;
this.options.onPollError?.(error);
}
await waitForNextTick(this.pollIntervalMs, this.options.signal);
}
} finally {
await this.worker.close();
this.client.close();
}
}
}

function wrapFetcher(fetcher: Fetcher): Fetcher {
return async (url) => {
try {
return await fetcher(url);
} catch (error) {
throw new HnFetchError(error);
}
};
}

async function fetchText(url: string): Promise<string> {
const response = await fetch(url);
if (!response.ok) throw new Error(`HTTP ${response.status}`);
return response.text();
}

function intervalFromEnvironment(): number {
const configured = process.env.POLL_INTERVAL_MS;
if (configured === undefined) return DEFAULT_POLL_INTERVAL_MS;
const interval = Number(configured);
if (!Number.isFinite(interval) || interval < 0) {
throw new Error('POLL_INTERVAL_MS must be a non-negative number');
}
return interval;
}

function waitForNextTick(ms: number, signal?: AbortSignal): Promise<void> {
if (signal?.aborted) return Promise.resolve();
return new Promise((resolve) => {
const timer = setTimeout(done, ms);
signal?.addEventListener('abort', done, { once: true });

function done(): void {
clearTimeout(timer);
signal?.removeEventListener('abort', done);
resolve();
}
});
}

function errorMessage(error: unknown): string {
return error instanceof Error ? error.message : String(error);
}
5 changes: 5 additions & 0 deletions sdk/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ export { JOURNAL_WRITE_FAILED, PROTOCOL_VERSION } from './protocol.js';

export { JournalClient, type JournalClientOptions } from './journal-client.js';
export { AgentWorker, type AgentWorkerOptions } from './worker.js';
export {
HnFetchError,
HnMonitorRunner,
type HnMonitorRunnerOptions,
} from './hn-monitor-runner.js';

export {
validateWorkPackage,
Expand Down
11 changes: 9 additions & 2 deletions sdk/src/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ interface CliResult {
/** Executes dispatched agent steps using their declared CLI. */
export class AgentWorker extends EventEmitter {
private attached = false;
private readonly executions = new Set<Promise<void>>();

constructor(
private readonly client: JournalClient,
Expand All @@ -38,14 +39,20 @@ export class AgentWorker extends EventEmitter {
}
}

close(): void {
/** Drains local executions; intentionally does not release the server registration. */
async close(): Promise<void> {
this.client.off('step.dispatch', this.onDispatch);
this.attached = false;
await Promise.allSettled(this.executions);
}

private readonly onDispatch = (dispatch: StepDispatchEvent): void => {
if (dispatch.step_type !== 'agent') return;
void this.execute(dispatch).catch((error: unknown) => this.emit('error', error));
const execution = this.execute(dispatch);
this.executions.add(execution);
void execution
.catch((error: unknown) => this.emit('error', error))
.finally(() => this.executions.delete(execution));
};

private async execute(dispatch: StepDispatchEvent): Promise<void> {
Expand Down
Loading