From cedfa5747783a33df4230091c707aece29aced04 Mon Sep 17 00:00:00 2001 From: Sanchit Rk <0xsanchit@gmail.com> Date: Tue, 27 Oct 2026 02:42:29 +0530 Subject: [PATCH 1/2] refactor(harness): tidy loop lifecycle seams after review - A committed turn is `{ messageId, reason }` or nothing; `needsContinuation` was always `reason === "toolUse"`, so it is gone from `Hooks.Completed`. - Docs that still said plugin setup and config run per exchange now say per turn, including the plugin SDK's shared-context docs. - New loop tests: a `prepareNextTurn` snapshot reaches the next request, with a steer queued while it ran; `prepareRequest` sees promoted input and does not promote a steer queued during it; `continue` after a tool batch adds no request; `end` skips next-turn preparation. - Comments and test names describe behaviour without naming other projects. Co-Authored-By: Claude Opus 5.5 --- packages/harness/README.md | 2 +- packages/harness/src/effect/harness.ts | 8 +- packages/harness/src/event/list.ts | 2 +- packages/harness/src/plugin/builtin.ts | 2 +- .../harness/src/plugin/builtin/tool/read.ts | 2 +- packages/harness/src/plugin/catalog.ts | 20 +-- .../harness/src/plugin/prompt/registry.ts | 2 +- packages/harness/src/plugin/store.ts | 4 +- packages/harness/src/runner/hooks.ts | 2 +- packages/harness/src/runner/llm.ts | 2 +- packages/harness/src/runner/loop.ts | 38 ++---- packages/harness/src/settings/settings.ts | 2 +- packages/harness/src/state/state.ts | 16 +-- packages/harness/src/tool/binary.ts | 4 +- .../test/__artifacts__/runner.hooks.json | 7 +- packages/harness/test/fixtures/search.ts | 14 +- packages/harness/test/runner.loop.test.ts | 120 +++++++++++++++++- packages/harness/test/tools.edit.test.ts | 2 +- packages/harness/test/tools.read.test.ts | 2 +- packages/harness/test/tools.write.test.ts | 2 +- packages/plugin/src/plugin/context.ts | 2 +- packages/plugin/src/plugin/plugin.ts | 2 +- 22 files changed, 176 insertions(+), 81 deletions(-) diff --git a/packages/harness/README.md b/packages/harness/README.md index b5822184..5ac6d2e0 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -150,7 +150,7 @@ A plugin is installed into its own directory, so its Effect is a separate module Package sources install with pnpm, with lifecycle scripts disabled, under the harness home cache. The installer inherits stderr, so a first install prints pnpm's own progress and errors to the terminal — and a `plugins` entry in a settings file means that can happen during `Harness.layer` construction, before any session exists. An omitted version means `latest` on the first installation; subsequent constructions reuse that completed installation. The selection is read once per `Harness.layer`, so an edited `plugins` block applies at the next construction; hot reload and daemon lifecycles are not implemented. -Failures are attributed: a bad reference, unreadable module, or malformed plugin fails `Harness.layer` construction with `PluginPreparationError`, which carries the failing phase (`source`, `install`, `import`, or `definition`) and the index of the offending reference. A failing package install reports `PluginInstallError`; a plugin's `setup` failure becomes `Plugin.SetupError` with the plugin id, surfacing as a `SnapshotError` for that exchange. Plugins are trusted in-process code — local paths and `file:` references import whatever they point at, so only load sources you trust. +Failures are attributed: a bad reference, unreadable module, or malformed plugin fails `Harness.layer` construction with `PluginPreparationError`, which carries the failing phase (`source`, `install`, `import`, or `definition`) and the index of the offending reference. A failing package install reports `PluginInstallError`; a plugin's `setup` failure becomes `Plugin.SetupError` with the plugin id, surfacing as a `SnapshotError` for that turn. Plugins are trusted in-process code — local paths and `file:` references import whatever they point at, so only load sources you trust. ## Pluggable Sandboxes diff --git a/packages/harness/src/effect/harness.ts b/packages/harness/src/effect/harness.ts index e7e2649f..5957d621 100644 --- a/packages/harness/src/effect/harness.ts +++ b/packages/harness/src/effect/harness.ts @@ -90,11 +90,11 @@ export const layer = (options: Options = {}) => // has none. Boot reads the user layer and the explicit override only -- a server can // start anywhere, so it must never walk ancestors of the directory it happened to // launch in for a `.codework/`, which would hand it a stranger's project settings. - // A project's plugins still load: lazily, at the session's first exchange, through + // A project's plugins still load: lazily, at the session's first turn, through // `follow`. const config = yield* Settings.load({ ...settingsOptions, home: paths.home }); /* - * Which entries this process configures from, per exchange. + * Which entries this process configures from, per turn. * * Settings entries extend the built-in selection rather than standing in for it, so * naming a plugin cannot silently drop Bash or the default prompt. A built-in is turned @@ -122,7 +122,7 @@ export const layer = (options: Options = {}) => /* * What the store already holds for a reference, and nothing else. * - * Resolve-only by construction: `Store.resolve` never opens a socket, so an exchange + * Resolve-only by construction: `Store.resolve` never opens a socket, so a turn * asking this question cannot block on a registry, and `plugin install` stays the only * verb that puts bytes on disk. A reference that does not parse, or names a local * path, is simply not filed. @@ -158,7 +158,7 @@ export const layer = (options: Options = {}) => const resolveOnly = { ...catalogOptions, install: PluginStore.required }; // The load pass, once, at boot: a store lookup and an ESM import per module, never a - // fetch. Every later exchange runs only the config pass over what this produced. + // fetch. Every later turn runs only the config pass over what this produced. const pool = yield* Ref.make( yield* load(references(config), { ...resolveOnly, declared: declaredIn(config) }), ); diff --git a/packages/harness/src/event/list.ts b/packages/harness/src/event/list.ts index aa96755d..46607344 100644 --- a/packages/harness/src/event/list.ts +++ b/packages/harness/src/event/list.ts @@ -296,7 +296,7 @@ export const PluginUpdated = EventSchema.define({ file: optional(Schema.String), error: optional(Schema.String), /** - * The session whose exchange triggered the event, when one did -- `failed` from a + * The session whose turn triggered the event, when one did -- `failed` from a * session's own `follow` names it. Absent on `reload` and boot, which no session owns: * the pool is process-wide, so this is the trigger, never an attribution of ownership. */ diff --git a/packages/harness/src/plugin/builtin.ts b/packages/harness/src/plugin/builtin.ts index e1c79ec0..e7246954 100644 --- a/packages/harness/src/plugin/builtin.ts +++ b/packages/harness/src/plugin/builtin.ts @@ -12,7 +12,7 @@ import { writePlugin } from "./builtin/tool/write.ts"; /** * Every plugin the harness ships, and the selection a caller who passes no `plugins` gets. Tool * contributors come first; within the prompt domain, declaration order is run order. The search - * tools (grep, find, ls) are `optIn`, as Pi ships them off: settings turn each on with + * tools (grep, find, ls) are `optIn` and ship off: settings turn each on with * `{ "plugin": "codework.tool.grep", "enabled": true }`. */ export const builtins = Object.freeze([ diff --git a/packages/harness/src/plugin/builtin/tool/read.ts b/packages/harness/src/plugin/builtin/tool/read.ts index d7b6dd98..0eed4e0e 100644 --- a/packages/harness/src/plugin/builtin/tool/read.ts +++ b/packages/harness/src/plugin/builtin/tool/read.ts @@ -78,7 +78,7 @@ export const readDef = Tool.define({ const fail = (message: string) => Effect.fail(new ReadFailed({ message })); /** - * Filesystem failures are the model's to see, as Pi shows them: a missing path in + * Filesystem failures are the model's to see: a missing path in * one wording on every backend, anything else (EACCES, …) as the backend reported it. */ const fsFailure = (path: string) => (error: SandboxFileSystem.FileSystemError) => diff --git a/packages/harness/src/plugin/catalog.ts b/packages/harness/src/plugin/catalog.ts index b1964340..7ab2f342 100644 --- a/packages/harness/src/plugin/catalog.ts +++ b/packages/harness/src/plugin/catalog.ts @@ -91,9 +91,9 @@ const emptyPool: Pool = { /** * The **load pass**: store lookup and ESM import, never the network. * - * Runs at boot, at `reload`, and at any exchange that finds a reference it cannot satisfy. It + * Runs at boot, at `reload`, and at any turn that finds a reference it cannot satisfy. It * deliberately knows nothing about `enabled`, options or order -- those are data over modules that - * are already loaded, they cost a walk of a short array, and they belong to every exchange. + * are already loaded, they cost a walk of a short array, and they belong to every turn. * * Loading is keyed by what a reference resolves to, so naming one module twice imports it once. */ @@ -186,7 +186,7 @@ export interface Selected { } /** - * The **config pass**: pure data over modules that are already loaded, run at every exchange. + * The **config pass**: pure data over modules that are already loaded, run at every turn. * * An array entry is a **module** — a definition object, a path, a `file:` URL, or a package * spec. It takes the position it is written at, so naming a module again moves it. @@ -285,9 +285,9 @@ export const select = Effect.fn("PluginCatalog.select")(function* (references: R }); /** - * The exchange-boundary check: has the store moved under this pool? + * The turn-boundary check: has the store moved under this pool? * - * Every exchange already re-reads every settings layer from disk, and the config pass already + * Every turn already re-reads every settings layer from disk, and the config pass already * walks those entries against the loaded set. Two things can be settled right there, and they are * the same question asked twice: * @@ -305,15 +305,15 @@ export const select = Effect.fn("PluginCatalog.select")(function* (references: R * * **This is not a watcher.** Nothing polls and no fiber exists; it is the observation that we were * already paying for the I/O that would tell us, and discarding the answer. Three bounds keep it - * safe: the store only and never the network, so an exchange cannot block on a registry; - * resolve-only, so it cannot install; and once, at the top of a snapshot, so an exchange already + * safe: the store only and never the network, so a turn cannot block on a registry; + * resolve-only, so it cannot install; and once, at the top of a snapshot, so a turn already * running is unaffected. */ export const follow = Effect.fn("PluginCatalog.follow")(function* ( references: ReadonlyArray, pool: Pool, /** - * `install` is required and must be resolve-only. Making it non-optional is how "an exchange + * `install` is required and must be resolve-only. Making it non-optional is how "a turn * follows the store, it does not fill it" becomes a fact of the signature rather than a rule * someone has to remember: the default installer fetches, and there is no way to reach it here. */ @@ -327,7 +327,7 @@ export const follow = Effect.fn("PluginCatalog.follow")(function* ( * exists and covers only it. * * `from` is the `.npmrc` anchor the lookup computes the store key under: the project that - * declared the reference, so an exchange answers about the same artifact `plugin install` + * declared the reference, so a turn answers about the same artifact `plugin install` * filed rather than the one the server's own directory would name. */ filed: (reference: string, from: string) => Effect.Effect>, @@ -382,7 +382,7 @@ export const follow = Effect.fn("PluginCatalog.follow")(function* ( // // A retained origin's source may have vanished since it loaded -- a file deleted, a store // entry removed. It is nobody's declaration now, so it drops out of the pool with a warning - // rather than failing the exchange for plugins nothing was wrong with. A reference the + // rather than failing the turn for plugins nothing was wrong with. A reference the // session itself still configures stays strict: the drift check above covered it, and `load` // still fails on it. const retained: Origin[] = []; diff --git a/packages/harness/src/plugin/prompt/registry.ts b/packages/harness/src/plugin/prompt/registry.ts index f8c73c55..d5df862d 100644 --- a/packages/harness/src/plugin/prompt/registry.ts +++ b/packages/harness/src/plugin/prompt/registry.ts @@ -6,7 +6,7 @@ import type { PromptRegistry } from "./schema.ts"; * * A selection with no prompt plugin is a legitimate configuration -- an embedder * passing `plugins: []`, or a user disabling `codework.prompt.default` before - * their own plugin is ready -- and failing the exchange over it buys nothing the + * their own plugin is ready -- and failing the turn over it buys nothing the * assembled prompt would not already tell them. The real foundation lives in * `codework.prompt.default`; this is only the floor. */ diff --git a/packages/harness/src/plugin/store.ts b/packages/harness/src/plugin/store.ts index 1cfaab9a..8acd85d8 100644 --- a/packages/harness/src/plugin/store.ts +++ b/packages/harness/src/plugin/store.ts @@ -24,7 +24,7 @@ * * **A generation is immutable.** Installing never mutates a published one: it stages elsewhere and * renames into place. Three problems that solves, and nothing simpler solves all three -- a - * running exchange holding file URLs into generation *N* is not disturbed by *N+1*; Node's ESM + * running turn holding file URLs into generation *N* is not disturbed by *N+1*; Node's ESM * registry is keyed by URL and never evicts, so an in-place update would hand back the *old* * module on re-import; and rollback is a `rename`, because a generation that fails validation * never gets a marker. @@ -341,7 +341,7 @@ export const add = Effect.fn("PluginStore.add")(function* ( * Expire what is superseded. * * The two newest generations are kept regardless of age: the newest is live, and the one before it - * may still be imported by an exchange in flight. Runs at the end of an `add` that published + * may still be imported by a turn in flight. Runs at the end of an `add` that published * something -- no timer and no sweeper, because installs are the only thing that creates garbage. */ export const collect = Effect.fn("PluginStore.collect")(function* (directory: string) { diff --git a/packages/harness/src/runner/hooks.ts b/packages/harness/src/runner/hooks.ts index 3ccb3d3d..72644b3c 100644 --- a/packages/harness/src/runner/hooks.ts +++ b/packages/harness/src/runner/hooks.ts @@ -17,8 +17,8 @@ export interface Completed { /** The snapshot the turn ran on. */ readonly snapshot: State.Snapshot; readonly messageId: string; + /** `toolUse` means tool results wait for a reply. */ readonly reason: "stop" | "length" | "toolUse"; - readonly needsContinuation: boolean; /** * What the next run would promote, read without promoting it: the queued steers, or the oldest * follow-up when there are none. As of the commit; input admitted later is not in it. diff --git a/packages/harness/src/runner/llm.ts b/packages/harness/src/runner/llm.ts index 18a4cae6..8edc8b31 100644 --- a/packages/harness/src/runner/llm.ts +++ b/packages/harness/src/runner/llm.ts @@ -139,7 +139,7 @@ export type Resolve = ( input: ResolutionInput, ) => Effect.Effect; -/** Resolve once before exchange setup; execution reuses this exact instance. */ +/** Resolve once before turn setup; execution reuses this exact instance. */ export const resolve: Resolve = Effect.fn("LLM.resolve")(function* (input) { const overrides = resolveOverrides(input.settings ?? {}); const declared = ModelCatalog.entry(input.models ?? [], input.provider, input.model); diff --git a/packages/harness/src/runner/loop.ts b/packages/harness/src/runner/loop.ts index 83808a11..f90132b0 100644 --- a/packages/harness/src/runner/loop.ts +++ b/packages/harness/src/runner/loop.ts @@ -26,12 +26,8 @@ import { Runner } from "./run.ts"; export type Promotion = "steer" | "followUp" | undefined; -export interface TurnResult { - readonly needsContinuation: boolean; - /** Set with `reason` when the turn committed an assistant message. */ - readonly messageId?: string; - readonly reason?: Hooks.Completed["reason"]; -} +/** A turn that committed an assistant message; `undefined` when it sent no request. */ +export type TurnResult = Pick | undefined; export interface Options { /** Deterministic provider seam for tests. */ @@ -281,14 +277,14 @@ export const layer = (options: Options = {}) => const steers = yield* inputs.promoteSteers(sessionId, cutoff); // The pending input this turn was started for was cancelled before promotion: with no // earlier work to continue there is nothing to answer, so no provider request. - if (!continuing && !followUp && steers === 0) return { needsContinuation: false } satisfies TurnResult; + if (!continuing && !followUp && steers === 0) return undefined; } const assembled = yield* context.assemble(sessionId); const leaf = assembled.lastAssistant?.entryId === assembled.leafEntryId ? assembled.lastAssistant.message : undefined; if (leaf !== undefined && (leaf.stopReason === "stop" || leaf.stopReason === "length")) { - return { needsContinuation: false } satisfies TurnResult; + return undefined; } // The latest attempt's publisher; a retried turn has one per provider request. @@ -400,11 +396,7 @@ export const layer = (options: Options = {}) => committed = true; }); if (interrupted !== undefined) return yield* Effect.failCause(interrupted); - return { - needsContinuation: terminal.reason === "toolUse", - messageId: terminal.message.messageId, - reason: terminal.reason, - } satisfies TurnResult; + return { messageId: terminal.message.messageId, reason: terminal.reason } satisfies TurnResult; }); // `onError`, not `catchCause`: a catch handler never runs on an interrupted fiber, and @@ -485,28 +477,22 @@ export const layer = (options: Options = {}) => // Tool results and steers continue first; a follow-up only once both are exhausted. while (true) { const result: TurnResult = yield* runTurn(promotion, snapshot, continuing); + const toolUse = result?.reason === "toolUse"; const completed: Hooks.Completed | undefined = - result.messageId === undefined || result.reason === undefined + result === undefined ? undefined - : { - snapshot, - messageId: result.messageId, - reason: result.reason, - needsContinuation: result.needsContinuation, - pending: yield* peekPending(input.sessionId), - }; + : { ...result, snapshot, pending: yield* peekPending(input.sessionId) }; const decision = completed === undefined ? undefined : yield* hooks.finishTurn(completed); // Queued steers and follow-ups stay for the next run. if (decision === "end") return; // A forced continuation after a finished reply sends nothing: the leaf guard in // `runTurnAttempt` ends the turn unless something was appended after it. - continuing = result.needsContinuation || decision === "continue"; + continuing = toolUse || decision === "continue"; const steer = yield* inputs.hasPending(input.sessionId, "steer"); - // Only tool results outrank a follow-up. A forced continuation rides along with it, as in - // Pi, where the follow-up is the natural next request and the continuation its fallback. - const followUp = - !result.needsContinuation && !steer && (yield* inputs.hasPending(input.sessionId, "followUp")); + // Only tool results outrank a follow-up. A forced continuation rides along with it: the + // follow-up is the natural next request, and the continuation only its fallback. + const followUp = !toolUse && !steer && (yield* inputs.hasPending(input.sessionId, "followUp")); if (!continuing && !steer && !followUp) return; promotion = followUp ? "followUp" : "steer"; diff --git a/packages/harness/src/settings/settings.ts b/packages/harness/src/settings/settings.ts index 7802e2a2..667f0375 100644 --- a/packages/harness/src/settings/settings.ts +++ b/packages/harness/src/settings/settings.ts @@ -217,7 +217,7 @@ export const parse = Effect.fn("Settings.parse")(function* (path: string, source export interface Interface { /** - * Await fresh host files at each exchange. No cache, mutation, or reload API. Fails when a + * Await fresh host files at each turn. No cache, mutation, or reload API. Fails when a * file exists and cannot be used, including on an edit made mid-session. * * The discovery root is an argument rather than a property of the layer because it is a diff --git a/packages/harness/src/state/state.ts b/packages/harness/src/state/state.ts index 357f535c..7e988730 100644 --- a/packages/harness/src/state/state.ts +++ b/packages/harness/src/state/state.ts @@ -80,7 +80,7 @@ export interface Options extends RequestOptions { } /** - * Exchange plugin setup failed. + * A plugin's setup failed while capturing a turn's snapshot. * * Typed rather than a defect: a caller's callback failing is a caller bug, but it * is one the session should report and survive rather than crash on. The turn @@ -97,7 +97,7 @@ export class SnapshotError extends Schema.TaggedError()("State.Sn } /** - * Immutable runtime state for one exchange. + * Immutable runtime state for one turn. * * `tools.handle` is executable, and that is deliberate: the definitions the model * is shown, the wire schemas it is given, and the handlers that run all come from @@ -142,7 +142,7 @@ export interface Interface { /** * Re-import every configured plugin, and swap the loaded set if it works. * - * This covers exactly one case the exchange boundary cannot: a **local** plugin whose contents + * This covers exactly one case the turn boundary cannot: a **local** plugin whose contents * changed while its path did not. Nothing in settings changed and no generation moved -- there * is none, because a local source is never copied into the store -- so neither boundary check * fires, and the module registry holds the old module against an unchanged URL. @@ -196,7 +196,7 @@ const resolver = (input: string | PromptResolver): PromptResolver => (typeof inp * * The split it enables: **loading** a module costs a store lookup and an ESM import, so it happens * at boot and at `reload`; **configuring** one -- `enabled`, `options`, order -- is pure data over - * modules already in memory, so it happens at every exchange. That is why editing `options` in a + * modules already in memory, so it happens at every turn. That is why editing `options` in a * settings file takes effect at the next turn without anything being re-imported. * * `snapshot` reads the `Ref` once, at the top, so a reload lands on the next turn and never @@ -204,7 +204,7 @@ const resolver = (input: string | PromptResolver): PromptResolver => (typeof inp * a genuinely different URL, so the module registry cannot hand back the old one. * * `references` says which entries the config pass walks. It is a function of the settings that - * exchange read, so a caller that pinned an explicit list keeps it and everyone else follows the + * turn read, so a caller that pinned an explicit list keeps it and everyone else follows the * files. * * `pool` is the **process view**: what boot loaded from the user layers. A session in a project @@ -218,7 +218,7 @@ export const layer = ( pool: Ref.Ref, references: (settings: Info) => ReadonlyArray, /** - * The exchange-boundary check: the new pool when the store has moved, nothing when it has not. + * The turn-boundary check: the new pool when the store has moved, nothing when it has not. * State does not know what a store is, so whoever built the pool answers this. */ follow: ( @@ -254,7 +254,7 @@ export const layer = ( /* * One load pass at a time, for the whole process. * - * Two sessions can reach an exchange boundary together and both find the same entry + * Two sessions can reach a turn boundary together and both find the same entry * unloaded -- someone ran `plugin add` in a directory they share. Without this they * both run a load pass and both swap the pool, so the work happens twice and the * loser's result is discarded. @@ -431,7 +431,7 @@ export const layer = ( }).pipe(loading.withPermits(1), Effect.mapError(failed)); // The config pass: pure data over what is already loaded, so it runs every - // exchange. + // turn. const chosen = yield* select(refs, loaded).pipe(Effect.mapError(failed)); // An entry naming a module the pool does not hold is the one case the pass // cannot satisfy, so it is free to report -- which is what a watcher was diff --git a/packages/harness/src/tool/binary.ts b/packages/harness/src/tool/binary.ts index 710ab2be..cad020ac 100644 --- a/packages/harness/src/tool/binary.ts +++ b/packages/harness/src/tool/binary.ts @@ -125,7 +125,7 @@ const platformOf = (shell: ISandboxExe) => return { os: mappedOs, arch: mappedArch } as const; }); -/** Unpacks with the sandbox's own `tar`, as Pi does on its host; the archive layout varies by release. */ +/** Unpacks with the sandbox's own `tar`; the archive layout varies by release. */ const unpack = [ "set -e", 'dir=$1; archive=$2; name=$3; tmp="$dir/.extract-$name-$$"', @@ -142,7 +142,7 @@ const install = (name: Name, shell: ISandboxExe, fs: SandboxFileSystem.Interface Effect.gen(function* () { const spec = specs[name]; const { os, arch } = yield* platformOf(shell); - // Pi pins fd on Intel macs, where later releases ship no build. + // fd is pinned on Intel macs, where later releases ship no build. const version = name === "fd" && os === "darwin" && arch === "x86_64" ? "10.3.0" : yield* latestVersion(spec.repo); const asset = spec.asset(version, os, arch); diff --git a/packages/harness/test/__artifacts__/runner.hooks.json b/packages/harness/test/__artifacts__/runner.hooks.json index b1229a17..830f017b 100644 --- a/packages/harness/test/__artifacts__/runner.hooks.json +++ b/packages/harness/test/__artifacts__/runner.hooks.json @@ -6,13 +6,11 @@ }, { "hook": "finishTurn", - "reason": "toolUse", - "needsContinuation": true + "reason": "toolUse" }, { "hook": "prepareNextTurn", "completed": "toolUse", - "needsContinuation": true, "tools": [ "echo", "echo_late" @@ -24,8 +22,7 @@ }, { "hook": "finishTurn", - "reason": "stop", - "needsContinuation": false + "reason": "stop" } ], "requestTools": [ diff --git a/packages/harness/test/fixtures/search.ts b/packages/harness/test/fixtures/search.ts index d3ef445a..0b4eac80 100644 --- a/packages/harness/test/fixtures/search.ts +++ b/packages/harness/test/fixtures/search.ts @@ -54,13 +54,13 @@ export const grepCases: Cases = { "glob-basename": { pattern: "needle", glob: "*.ts" }, "glob-path": { pattern: "needle", glob: "**/*.spec.ts" }, context: { pattern: "console.log\\(needle", context: 1 }, - // Pi: the limit is global, and the context of the one match shown still comes with it. + // The limit is global, and the context of the one match shown still comes with it. "limit-context": { pattern: "match", path: "context.txt", limit: 1, context: 1 }, limit: { pattern: "needle", limit: 2 }, "single-file": { pattern: "needle", path: "src/util.ts" }, subdir: { pattern: "needle", path: "src" }, "dash-pattern": { pattern: "-x" }, - // Pi: a pattern that looks like a flag is searched for, never obeyed. + // A pattern that looks like a flag is searched for, never obeyed. "flag-pattern": { pattern: "--pre=./payload.sh" }, "no-match": { pattern: "absent-token" }, "long-line": { pattern: "match x" }, @@ -77,7 +77,7 @@ export const grepCases: Cases = { export const VARIANCE = "glob-basename"; export const lsCases: Cases = { - // Pi: dotfiles and dot-directories are listed. + // Dotfiles and dot-directories are listed. root: {}, subdir: { path: "src" }, limit: { limit: 2 }, @@ -87,14 +87,14 @@ export const lsCases: Cases = { }; export const findCases: Cases = { - // Pi: hidden files are found unless ignored. + // Hidden files are found unless ignored. basename: { pattern: "*.ts" }, "with-path": { pattern: "src/**/*.ts" }, - // Pi #3302: a pattern with a directory in it matches the path, not the basename. + // A pattern with a directory in it matches the path, not the basename. "spec-path": { pattern: "src/**/*.spec.ts" }, subtree: { pattern: "src/nested/**" }, "leading-globstar": { pattern: "**/nested/*" }, - // Pi #3303: each .gitignore applies to its own subtree only. + // Each .gitignore applies to its own subtree only. "nested-gitignore": { pattern: "*.txt", path: "a" }, "sibling-gitignore": { pattern: "*.txt", path: "b" }, negated: { pattern: "*.log" }, @@ -102,7 +102,7 @@ export const findCases: Cases = { // fd stops at whichever match a thread finds first, so the limit is hit by the only match. limit: { pattern: "*.log", limit: 1 }, none: { pattern: "*.nothing" }, - // Pi: a flag-like pattern is a pattern; a glob fd cannot parse is reported. + // A flag-like pattern is a pattern; a glob fd cannot parse is reported. "flag-pattern": { pattern: "--help" }, "bad-glob": { pattern: "[" }, missing: { pattern: "*.ts", path: "nope" }, diff --git a/packages/harness/test/runner.loop.test.ts b/packages/harness/test/runner.loop.test.ts index dbfaee49..0c3a9170 100644 --- a/packages/harness/test/runner.loop.test.ts +++ b/packages/harness/test/runner.loop.test.ts @@ -1199,7 +1199,7 @@ describe("runner loop — resume after restart", () => { }), ); - // Pi resends the cut-off request as it was; a follow-up queued behind it waits for the answer. + // The cut-off request is resent as it was; a follow-up queued behind it waits for the answer. it( "answers the reply a shutdown cut off before a queued follow-up goes in", Effect.gen(function* () { @@ -1400,7 +1400,6 @@ describe("runner loop — lifecycle hooks", () => { calls.push({ hook: "prepareNextTurn", completed: turn.reason, - needsContinuation: turn.needsContinuation, tools: next.tools.defs.map((def) => def.name).filter((name) => name.startsWith("echo")), }); return next; @@ -1412,7 +1411,7 @@ describe("runner loop — lifecycle hooks", () => { }), finishTurn: (turn) => Effect.sync(() => { - calls.push({ hook: "finishTurn", reason: turn.reason, needsContinuation: turn.needsContinuation }); + calls.push({ hook: "finishTurn", reason: turn.reason }); return undefined; }), }; @@ -1491,10 +1490,14 @@ describe("runner loop — lifecycle hooks", () => { describe("finishTurn end", () => { const contexts: Message.Context[] = []; + let prepared = 0; const { effect: it } = testEffect( runtime({ open: recording(contexts), - hooks: hooksOf({ finishTurn: (turn) => Effect.succeed(turn.reason === "toolUse" ? "end" : undefined) }), + hooks: hooksOf({ + finishTurn: (turn) => Effect.succeed(turn.reason === "toolUse" ? "end" : undefined), + prepareNextTurn: (_turn, next) => Effect.sync(() => ((prepared += 1), next)), + }), plugins: [...plugins, { id: "test.tool.echo", kind: "tool", setup: (ctx) => ctx.plugin.tools.add(echo) }], }), ); @@ -1510,6 +1513,7 @@ describe("runner loop — lifecycle hooks", () => { yield* execution.resume(sessionId); expect(contexts).toHaveLength(1); + expect(prepared).toBe(0); const sessions = yield* Session.Service; const path = yield* sessions.path(sessionId); expect(path.map((item) => [item.entry.type, item.entry.state])).toEqual([ @@ -1604,6 +1608,114 @@ describe("runner loop — lifecycle hooks", () => { ); }); + describe("finishTurn continue after a tool batch", () => { + const contexts: Message.Context[] = []; + const completed: string[] = []; + const { effect: it } = testEffect( + runtime({ + open: recording(contexts), + hooks: hooksOf({ + finishTurn: () => Effect.succeed("continue"), + prepareNextTurn: (turn, next) => Effect.sync(() => (completed.push(turn.reason), next)), + }), + plugins: [...plugins, { id: "test.tool.echo", kind: "tool", setup: (ctx) => ctx.plugin.tools.add(echo) }], + }), + ); + + it( + "is met by the tool-result request and adds none of its own", + Effect.gen(function* () { + const execution = yield* RunnerExecution.Service; + const sessionId = yield* seedSession(); + yield* admit({ id: "msg_tool", sessionId, delivery: "steer" }); + + yield* execution.resume(sessionId); + + expect(completed).toEqual(["toolUse", "stop"]); + expect(contexts).toHaveLength(2); + }), + ); + }); + + describe("prepareNextTurn", () => { + const contexts: Message.Context[] = []; + const during: { run: Effect.Effect } = { run: Effect.void }; + const { effect: it } = testEffect( + runtime({ + open: recording(contexts), + hooks: hooksOf({ + prepareNextTurn: (_turn, next) => during.run.pipe(Effect.as({ ...next, systemPrompt: "replaced" })), + }), + plugins: [...plugins, { id: "test.tool.echo", kind: "tool", setup: (ctx) => ctx.plugin.tools.add(echo) }], + }), + ); + + it( + "its snapshot reaches the next request, with a steer queued while it ran", + Effect.gen(function* () { + const execution = yield* RunnerExecution.Service; + const sessionId = yield* seedSession(); + yield* admit({ id: "msg_first", sessionId, delivery: "steer" }); + const services = yield* Effect.context>>(); + during.run = admit({ id: "msg_during", sessionId, delivery: "steer" }).pipe( + Effect.andThen( + Effect.sync(() => { + during.run = Effect.void; + }), + ), + Effect.provideContext(services), + ); + + yield* execution.resume(sessionId); + + expect(contexts.map((context) => context.systemPrompt === "replaced")).toEqual([false, true]); + expect(contexts.map(texts)).toEqual([["msg_first"], ["msg_first", "msg_during"]]); + }), + ); + }); + + describe("prepareRequest and queued input", () => { + const contexts: Message.Context[] = []; + const seen: Array> = []; + const during: { run: Effect.Effect } = { run: Effect.void }; + const { effect: it } = testEffect( + runtime({ + open: recording(contexts), + hooks: hooksOf({ + prepareRequest: (request) => + Effect.sync(() => seen.push(texts(request.context))).pipe( + Effect.andThen(during.run), + Effect.as(request), + ), + }), + plugins: [...plugins, { id: "test.tool.echo", kind: "tool", setup: (ctx) => ctx.plugin.tools.add(echo) }], + }), + ); + + it( + "sees promoted input, and a steer queued while it runs waits for the next turn", + Effect.gen(function* () { + const execution = yield* RunnerExecution.Service; + const sessionId = yield* seedSession(); + yield* admit({ id: "msg_first", sessionId, delivery: "steer" }); + const services = yield* Effect.context>>(); + during.run = admit({ id: "msg_during", sessionId, delivery: "steer" }).pipe( + Effect.andThen( + Effect.sync(() => { + during.run = Effect.void; + }), + ), + Effect.provideContext(services), + ); + + yield* execution.resume(sessionId); + + expect(seen).toEqual([["msg_first"], ["msg_first", "msg_during"]]); + expect(contexts.map(texts)).toEqual(seen); + }), + ); + }); + describe("prepareRequest on a retry", () => { const models: string[] = []; const attempts: Array<{ readonly retry: number; readonly failed?: string }> = []; diff --git a/packages/harness/test/tools.edit.test.ts b/packages/harness/test/tools.edit.test.ts index bd766def..a244ab8a 100644 --- a/packages/harness/test/tools.edit.test.ts +++ b/packages/harness/test/tools.edit.test.ts @@ -267,7 +267,7 @@ const editAll = (root: string, custom: string) => ); describe("edit plugin through the local harness", () => { - it("replaces, matches fuzzily, keeps line endings and reports failures like Pi", () => + it("replaces, matches fuzzily, keeps line endings and reports failures", () => withSettings(async ({ root, custom }) => { const work = join(root, "work"); await fs.mkdir(work); diff --git a/packages/harness/test/tools.read.test.ts b/packages/harness/test/tools.read.test.ts index 2089a313..0417ca6d 100644 --- a/packages/harness/test/tools.read.test.ts +++ b/packages/harness/test/tools.read.test.ts @@ -171,7 +171,7 @@ const summarise = (content: ReadonlyArray<{ type: string; text?: string; data?: ); describe("read plugin through the local harness", () => { - it("reads text, pages, images and failures like Pi", () => + it("reads text, pages, images and failures", () => withSettings(async ({ root, custom }) => { for (const { file } of cases) { if (file === undefined) continue; diff --git a/packages/harness/test/tools.write.test.ts b/packages/harness/test/tools.write.test.ts index 9ebabd61..ef95c368 100644 --- a/packages/harness/test/tools.write.test.ts +++ b/packages/harness/test/tools.write.test.ts @@ -75,7 +75,7 @@ const tree = async (dir: string, prefix = ""): Promise> = }; describe("write plugin through the local harness", () => { - it("creates, overwrites and reports failures like Pi", () => + it("creates, overwrites and reports failures", () => withSettings(async ({ root, custom }) => { const work = join(root, "work"); await fs.mkdir(work); diff --git a/packages/plugin/src/plugin/context.ts b/packages/plugin/src/plugin/context.ts index 4b9dd00b..9e0e12da 100644 --- a/packages/plugin/src/plugin/context.ts +++ b/packages/plugin/src/plugin/context.ts @@ -41,7 +41,7 @@ export interface Paths { readonly log: string; } -/** One object, shared by every plugin in one exchange. */ +/** One object, shared by every plugin in one turn. */ export interface SharedPluginContext { readonly sessionId: SessionID; readonly sandbox: SandboxIO.Identity; diff --git a/packages/plugin/src/plugin/plugin.ts b/packages/plugin/src/plugin/plugin.ts index 8c99ebbd..6151eb3b 100644 --- a/packages/plugin/src/plugin/plugin.ts +++ b/packages/plugin/src/plugin/plugin.ts @@ -55,7 +55,7 @@ export interface Plugin { * `options` is this plugin's own configuration block, `{}` when its entry carried none. * Unvalidated: the harness never looks inside it, so a plugin checks whatever shape it * documents. It is a second argument rather than a context field so `ctx` stays one object - * shared by every plugin in the exchange. + * shared by every plugin in the turn. */ readonly setup: ( ctx: SharedPluginContext, From 977ae834d21c20e767448621c739eed81ebe4b98 Mon Sep 17 00:00:00 2001 From: Sanchit Rk <0xsanchit@gmail.com> Date: Tue, 27 Oct 2026 03:16:39 +0530 Subject: [PATCH 2/2] test(harness): check a mid-chain change lands on the next live request One real tool chain per provider: the first tool edits the host settings file and unlocks a second tool, and the very next request of the same run must carry the new header revision and the new tool. The copilot settings test now names gemini-3.8-flash, since models.dev no longer lists 3.6. Co-Authored-By: Claude Opus 5.5 --- .../runner.turns.live.anthropic.json | 29 ++++ .../runner.turns.live.deepseek.json | 29 ++++ .../runner.turns.live.gemini.json | 29 ++++ .../__artifacts__/runner.turns.live.muse.json | 29 ++++ .../runner.turns.live.openai-codex.json | 29 ++++ .../runner.turns.live.openai.json | 29 ++++ .../harness/test/runner.turns.live.test.ts | 140 ++++++++++++++++++ packages/harness/test/settings.llm.test.ts | 4 +- 8 files changed, 316 insertions(+), 2 deletions(-) create mode 100644 packages/harness/test/__artifacts__/runner.turns.live.anthropic.json create mode 100644 packages/harness/test/__artifacts__/runner.turns.live.deepseek.json create mode 100644 packages/harness/test/__artifacts__/runner.turns.live.gemini.json create mode 100644 packages/harness/test/__artifacts__/runner.turns.live.muse.json create mode 100644 packages/harness/test/__artifacts__/runner.turns.live.openai-codex.json create mode 100644 packages/harness/test/__artifacts__/runner.turns.live.openai.json create mode 100644 packages/harness/test/runner.turns.live.test.ts diff --git a/packages/harness/test/__artifacts__/runner.turns.live.anthropic.json b/packages/harness/test/__artifacts__/runner.turns.live.anthropic.json new file mode 100644 index 00000000..192be833 --- /dev/null +++ b/packages/harness/test/__artifacts__/runner.turns.live.anthropic.json @@ -0,0 +1,29 @@ +{ + "requests": [ + { + "revision": "a", + "tools": [ + "first" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + } + ], + "calls": [ + "first:completed", + "second:completed" + ], + "reply": true +} diff --git a/packages/harness/test/__artifacts__/runner.turns.live.deepseek.json b/packages/harness/test/__artifacts__/runner.turns.live.deepseek.json new file mode 100644 index 00000000..192be833 --- /dev/null +++ b/packages/harness/test/__artifacts__/runner.turns.live.deepseek.json @@ -0,0 +1,29 @@ +{ + "requests": [ + { + "revision": "a", + "tools": [ + "first" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + } + ], + "calls": [ + "first:completed", + "second:completed" + ], + "reply": true +} diff --git a/packages/harness/test/__artifacts__/runner.turns.live.gemini.json b/packages/harness/test/__artifacts__/runner.turns.live.gemini.json new file mode 100644 index 00000000..192be833 --- /dev/null +++ b/packages/harness/test/__artifacts__/runner.turns.live.gemini.json @@ -0,0 +1,29 @@ +{ + "requests": [ + { + "revision": "a", + "tools": [ + "first" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + } + ], + "calls": [ + "first:completed", + "second:completed" + ], + "reply": true +} diff --git a/packages/harness/test/__artifacts__/runner.turns.live.muse.json b/packages/harness/test/__artifacts__/runner.turns.live.muse.json new file mode 100644 index 00000000..192be833 --- /dev/null +++ b/packages/harness/test/__artifacts__/runner.turns.live.muse.json @@ -0,0 +1,29 @@ +{ + "requests": [ + { + "revision": "a", + "tools": [ + "first" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + } + ], + "calls": [ + "first:completed", + "second:completed" + ], + "reply": true +} diff --git a/packages/harness/test/__artifacts__/runner.turns.live.openai-codex.json b/packages/harness/test/__artifacts__/runner.turns.live.openai-codex.json new file mode 100644 index 00000000..192be833 --- /dev/null +++ b/packages/harness/test/__artifacts__/runner.turns.live.openai-codex.json @@ -0,0 +1,29 @@ +{ + "requests": [ + { + "revision": "a", + "tools": [ + "first" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + } + ], + "calls": [ + "first:completed", + "second:completed" + ], + "reply": true +} diff --git a/packages/harness/test/__artifacts__/runner.turns.live.openai.json b/packages/harness/test/__artifacts__/runner.turns.live.openai.json new file mode 100644 index 00000000..192be833 --- /dev/null +++ b/packages/harness/test/__artifacts__/runner.turns.live.openai.json @@ -0,0 +1,29 @@ +{ + "requests": [ + { + "revision": "a", + "tools": [ + "first" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + }, + { + "revision": "b", + "tools": [ + "first", + "second" + ] + } + ], + "calls": [ + "first:completed", + "second:completed" + ], + "reply": true +} diff --git a/packages/harness/test/runner.turns.live.test.ts b/packages/harness/test/runner.turns.live.test.ts new file mode 100644 index 00000000..0ba2f799 --- /dev/null +++ b/packages/harness/test/runner.turns.live.test.ts @@ -0,0 +1,140 @@ +import { Effect, Schema } from "effect"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { describe, expect, it } from "vite-plus/test"; +import { ContextCodec } from "../src/context/codec.ts"; +import { Harness } from "../src/effect/harness.ts"; +import { Session } from "../src/effect/session.ts"; +import { define } from "../src/plugin/plugin.ts"; +import { SessionRuntime } from "../src/session/runtime.ts"; +import type { State } from "../src/state/state.ts"; +import * as Tool from "../src/tool/tool.ts"; +import { withSettings } from "./fixtures/settings.ts"; +import { available, LIVE } from "./utils/live.ts"; + +/** + * One run, one tool chain, against a real provider. The first tool's handler edits the host + * settings file and unlocks a second tool; both land on the very next request of the same chain, + * because every turn takes a fresh snapshot. The second tool's token is only reachable through + * that tool, so a right reply means the mid-chain declaration was really callable. + */ + +const FIRST = "alpha-91"; +const SECOND = "omega-27"; + +/** The request as sent, reduced to what a mid-chain change can move. */ +type Sent = { readonly revision?: unknown; readonly tools: ReadonlyArray }; + +describe.each(LIVE)("turn snapshots against live $name", (live) => { + it.skipIf(!available(live))( + "lands a settings edit and a tool declared during a tool batch on the next request", + { timeout: 300_000 }, + () => + withSettings(async ({ root, custom }) => { + const write = (revision: string) => + writeFile( + join(custom, "settings.jsonc"), + JSON.stringify({ + model: { + provider: live.provider, + id: live.id, + providerOptions: { + [live.provider]: { [live.id]: { headers: { "x-settings-revision": revision } } }, + }, + }, + }), + ); + let unlocked = false; + const first = Tool.register( + Tool.make({ + name: "first", + description: "Returns the first token and names the next step. Takes no arguments.", + parameters: Schema.Struct({}), + success: Schema.String, + encodeContent: (value) => [{ type: "text", text: value }], + handler: () => + Effect.gen(function* () { + yield* Effect.promise(() => write("b")); + unlocked = true; + return `${FIRST}. Now call the tool named second.`; + }), + }), + ); + const second = Tool.register( + Tool.make({ + name: "second", + description: "Returns the second token. Takes no arguments.", + parameters: Schema.Struct({}), + success: Schema.String, + encodeContent: (value) => [{ type: "text", text: value }], + handler: () => Effect.succeed(SECOND), + }), + ); + const relay = define({ + id: "acme.tool.relay", + kind: "tool", + setup(ctx) { + ctx.plugin.tools.add(first); + if (unlocked) ctx.plugin.tools.add(second); + }, + }); + + const sent: Sent[] = []; + const onPayload: NonNullable = async (payload) => { + const params = payload as Record; + sent.push({ + revision: (params.headers as Record | undefined)?.["x-settings-revision"], + tools: Object.keys((params.tools as Record | undefined) ?? {}).filter( + (name) => name === "first" || name === "second", + ), + }); + return payload; + }; + + await write("a"); + const { calls, reply } = await Effect.gen(function* () { + const handle = yield* Session.create({ directory: root }); + const runtime = yield* SessionRuntime.Service; + yield* runtime.update(handle.id, { onPayload }); + yield* handle.run( + "Call the first tool and follow its instruction. Then reply with exactly the two tokens " + + "the tools returned, separated by one space, and nothing else.", + ); + const messages = yield* Effect.forEach(yield* handle.path(), ContextCodec.decodeMessage); + const last = messages.findLast((message) => message.parts.some((part) => part.type === "text")); + return { + calls: messages + .flatMap((message) => message.parts) + .flatMap((part) => (part.type === "toolCall" ? [`${part.name}:${part.status}`] : [])), + reply: (last?.parts ?? []).flatMap((part) => (part.type === "text" ? [part.text] : [])).join(""), + }; + }).pipe( + Effect.provide( + Harness.layer({ + home: join(root, "home"), + hostCwd: root, + database: ":memory:", + userConfigDir: custom, + plugins: [relay], + }), + ), + Effect.scoped, + Effect.timeout("240 seconds"), + Effect.runPromise, + ); + + // The chain: first → second → reply, three requests in one run. + expect(calls).toEqual(["first:completed", "second:completed"]); + expect(sent).toHaveLength(3); + const [before, after] = sent as [Sent, Sent, Sent]; + expect(before).toEqual({ revision: "a", tools: ["first"] }); + expect(after).toEqual({ revision: "b", tools: ["first", "second"] }); + expect(reply).toContain(FIRST); + expect(reply).toContain(SECOND); + + await expect( + `${JSON.stringify({ requests: sent, calls, reply: [FIRST, SECOND].every((token) => reply.includes(token)) }, null, "\t")}\n`, + ).toMatchFileSnapshot(`./__artifacts__/runner.turns.live.${live.name}.json`); + }), + ); +}); diff --git a/packages/harness/test/settings.llm.test.ts b/packages/harness/test/settings.llm.test.ts index 119a9a37..895f6996 100644 --- a/packages/harness/test/settings.llm.test.ts +++ b/packages/harness/test/settings.llm.test.ts @@ -142,7 +142,7 @@ describe("settings at the LLM boundary", () => { id: "1", object: "chat.completion.chunk", created: 1, - model: "gemini-3.6-flash", + model: "gemini-3.8-flash", choices: [{ index: 0, delta, finish_reason: finish }], })}\n\n`; return new Response( @@ -153,7 +153,7 @@ describe("settings at the LLM boundary", () => { ); }; - const model = await llm("github-copilot", "gemini-3.6-flash"); + const model = await llm("github-copilot", "gemini-3.8-flash"); expect(model).toBeDefined(); if (!model) return;