diff --git a/packages/durable/CHANGELOG.md b/packages/durable/CHANGELOG.md index f5faab47b..6511187f0 100644 --- a/packages/durable/CHANGELOG.md +++ b/packages/durable/CHANGELOG.md @@ -110,6 +110,7 @@ - Continue submissions: `submit({ type: "continue" })` starts a run on the context as it is, without a user message, for example after a run that ended without an answer (`pi.live.runFailure`). Only an idle conversation admits one (a busy one throws `ConversationBusy`; it never queues); items a failed run left queued are placed first and join the run. A context that ends in an answer, also a text answer cut off at its output limit, has nothing to continue (`nothing_to_answer`); a cut-off answer with tool calls is continued, as its calls end in error results. Tasks and tools submit one through `ConversationHandle`. The `pi.durable.submit` span starts a root trace for it, as for an input ([#11043](https://github.com/earendil-works/pi/issues/11043)) - Per-conversation turn limits: `AgentState.maxTurns` (`configure({ maxTurns })`, `null` clears; a `RangeError` for anything but an integer 1 or more) bounds the tool rounds one run takes. The round that reaches it runs its tools, then the run ends `unanswered` with the new `RunFailureReason` `turn_limit`, like a failed run, so `settings.afterFailedRun` decides the queued inputs and a continue resumes it on the round's results. The count is `pi.live.run.turns`; every run starts at 0, and an `onYield` continuation starts it over. Task-owned children and forks inherit the limit; `Agent.maxTurns` exposes it ([#11043](https://github.com/earendil-works/pi/issues/11043)) - `whenIdle: "reject"` on input submissions: an idle conversation rejects the input with the new `ConversationIdle` and writes nothing, so it never starts a run. With `whenBusy: "steer"` it steers the current run or is refused, in one admission instead of a host's check and a separate submit that can race the run's end. A known request ID still returns its submission ([#11149](https://github.com/earendil-works/pi/issues/11149)) +- `Conversation.submit(tx, draft)` admits a submission inside the caller's commit (`Harness.commit()`, `Conversation.commit()`) with the same rules as `submit(draft, context)`, and returns its ID: a host checks its own documents and `pi.live`, admits, and writes its own state in one commit, which stores all of it or none. `ConversationHandle.submit(tx, draft)` does the same inside a task's or tool's own commit. The Harness supplies the clock, queue modes, the `pi.durable.submit` span (covering the admission only; its `result` is provisional until the commit is stored), and the request for progress. It must come before the commit's first table write, once per commit, and throws `ReadAfterWrite` otherwise on every path ([#11149](https://github.com/earendil-works/pi/issues/11149)) ### Changed diff --git a/packages/durable/README.md b/packages/durable/README.md index 283ccafbb..3403a4997 100644 --- a/packages/durable/README.md +++ b/packages/durable/README.md @@ -81,7 +81,7 @@ Every async call takes a Chord `Context`, which carries cancellation and its cur `HarnessOptions.telemetry` accepts a host-owned `TelemetryContext` from `@earendil-works/pi-telemetry`, with optional `propagation` and `metrics` capabilities; omitted, it is a no-op. `DURABLE_TELEMETRY_SCHEMA`, exported from the package root, documents its typed spans, events, and metrics. Attributes contain IDs and control metadata, never messages, arguments, or results; IDs point to the transcript. -- `pi.durable.submit` covers the public submit call and its commit. Each input starts a new root trace with a link to its caller's current span; a write is a child of its caller. Duplicate `requestId` preserves the original submission and carrier. Internal admissions, such as compaction writes, emit events on their phase span without a submit span. +- `pi.durable.submit` covers the public submit call and its commit; for `submit(tx, draft)` it covers the admission only, its `result` is provisional until your commit is stored, and the commit's events go to your commit's context. Each input starts a new root trace with a link to its caller's current span; a write is a child of its caller. Duplicate `requestId` preserves the original submission and carrier. Internal admissions, such as compaction writes, emit events on their phase span without a submit span. - `pi.durable.phase` covers one run or abort phase of every task, including its decision commit, never the next phase, a durable wait, or a restart. Its attributes identify the task, phase, attempt when present, `resumed`, and outcome. Tool phases also identify the tool and call. Nested tasks and background compactions inherit the creating phase; model requests receive that phase context. Hooks add no spans. - Failed phase outcomes set error status with a fixed category (`task_faulted`, `task_failed`, or `task_orphaned`) and the outcome's diagnostic message, truncated to 500 characters (the reason for an orphaned task). Diagnostics can contain caller content; no error detail or exception name is recorded. An `aborted` outcome keeps status `ok`, including interruption by close or cancellation. - Every `onReport` report also emits a `pi.durable.report` event on its current span. Without a span, the NOOP span drops the event; no root report span is created. Only `type` (the report's `kind`), optional fixed `reason`, `extension`, and `task.id` are recorded; no error payload is included. `pi.durable.reports` counts all reports by `type` only, including those without a span. Clock failures report without a phase context. The `onReport` callback is unchanged. @@ -541,6 +541,19 @@ await root.submit({ type: "write", entry: { kind: "app.note", data: "user opened - **Steers** are placed after the current tool round and join the running work. - **Follow-ups** are placed when the run answers, and start the next run. - **`whenIdle: "reject"`** refuses an input when no run is working, so it never starts a run of its own. With `whenBusy: "steer"`, it steers the current run or throws `ConversationIdle`. +- **`submit(tx, draft)`** admits inside your own commit, with the same rules, and returns the submission's ID. Your checks, the admission, and your writes are stored together or not at all: + + ```typescript + const id = await harness.commit(async (tx) => { + const chat = await tx.doc(MyChat, root.id); // your own conversation document + if (chat.muted.includes(userId)) throw new Error("Muted"); + const id = await root.submit(tx, { type: "input", content: text, whenBusy: "steer", whenIdle: "reject" }); + chat.sent.push({ id, userId }); // your own state, same commit + return id; + }, context); + ``` + + Call it before the commit's own table writes (`appendEntry()`, `createTask()`, another admission), once per commit: otherwise it throws `ReadAfterWrite`, on every path. Documents may be read and written before and after it. A rejection (`ConversationBusy`, `ConversationIdle`) throws before any write, so you may catch it. Until your commit is stored the admission is provisional: if your callback throws later, no submission exists. Tasks and tools do the same in their own commits through `api.conversation(id)`; entries the admission places carry the task's `byTaskId`. - **Writes** append an entry without asking the model anything. A write of only `kind` and `data` changes nothing the model sees, so it is appended at once, even mid-run, and is never queued: a chat message shows in the transcript as soon as it is sent. It can therefore appear before items submitted earlier that are still queued, except behind a queued reset, where it waits so the reset does not hide it. A write with `model`, `head`, or `edits` waits for the next boundary like a steer. - `await submission.abort(context)` withdraws a queued submission. - The [settings](#settings) `steeringMode: "all"` and `followUpMode: "all"` place every queued item at once instead of one per turn. diff --git a/packages/durable/docs/spec.md b/packages/durable/docs/spec.md index dbe7cb96a..3ef2b232f 100644 --- a/packages/durable/docs/spec.md +++ b/packages/durable/docs/spec.md @@ -709,6 +709,8 @@ type HooksOf = K extends Task interface Conversation { readonly id: ConversationId; submit(submission: SubmissionDraft, context: Context): Promise; + /** Admit inside the caller's commit, with the same rules; stored with that commit (section 6). */ + submit(tx: Tx, submission: SubmissionDraft): Promise; /** Resolved with the current registry snapshot and settings. */ agent(context: Context): Promise; /** `configure()` in its own commit. */ @@ -1921,6 +1923,10 @@ Table rules: - Any table read after the first table write throws `ReadAfterWrite`. - Document access and read-your-writes remain available after table writes. - Creation methods return their created ID/record; callers do not read it back. +- `Conversation.submit(tx, draft)` and `ConversationHandle.submit(tx, draft)` + come before the commit's first table write, another admission's included; + otherwise they throw `ReadAfterWrite`, whether or not the admission would read + a table (section 6). Storage ownership: @@ -2860,6 +2866,25 @@ the inbox holds queued items, so the input never starts a run: with `run === undefined`, the inverse of `busy` above. An idle input queues with mode `steer` when `whenBusy` is `steer` and `followUp` otherwise. +`Conversation.submit(tx, draft)` runs the same admission inside the caller's +commit instead of one of its own and returns the submission's ID; a task or tool +does so through `ConversationHandle.submit(tx, draft)` in its own commit. The +Harness supplies the clock, queue modes, the `pi.durable.submit` span (same +parent, links, and trace carrier as `submit(draft, context)`), and the request +for progress a started run needs. The admission's writes, a started run +included, are stored with the caller's other writes in that commit, or not at +all: a host can check its own documents and `pi.live`, admit, and record the +submission in one commit. A rejection throws before any write. Admission must +precede the commit's first table write, another admission's included: it throws +`ReadAfterWrite` otherwise, on every path, also where it would read no table (a +busy input without request ID or trace). Entries it appends inside a task's or +tool's commit carry that task's `byTaskId`, as the commit's own appends do. The +span covers the admission only: it ends before the caller's commit is stored, +its `result` is provisional (a later throw in the callback or a storage failure +leaves no submission), and the commit's events (`pi.durable.entry`, +`pi.durable.submission`) go to the caller's commit context. A first argument +that is not a draft is taken as a transaction; one of another Session throws. + A continue starts a run on the context as it is, without a user message, for example after a run that ended without an answer. It is admitted only while the conversation is idle and never queues: whether a run can continue depends on how @@ -3387,6 +3412,11 @@ interface ConversationHandle { readonly id: ConversationId; /** Any submission, as `Conversation.submit()` takes: a task may also continue a conversation it owns. */ submit(submission: SubmissionDraft, context: Context): Promise; + /** + * Inside the invocation's commit (`runtime.commit()`, a tool's `api.commit()`), as `Conversation.submit(tx, ...)`; + * appended entries carry the task's `byTaskId`. Rejects once the invocation is aborted or ended. + */ + submit(tx: Tx, submission: SubmissionDraft): Promise; abort(context: Context): Promise; waitForIdle(context: Context): Promise; } @@ -6007,6 +6037,12 @@ These are contracts, not invitations to add defensive machinery: unsupported. - **Read after write:** read every required table row before the first table write. Document drafts remain usable afterward; table reads do not. +- **Admission inside a commit:** `submit(tx, draft)` throws `ReadAfterWrite` + after the commit's first table write: one admission per commit, before the + commit's own `appendEntry()`, `createTask()`, or `createSubmission()`. The + admission is provisional until the commit is stored: a later throw in the + callback, or a storage failure, discards it with the rest of the commit, + though its `pi.durable.submit` span already ended with a `result`. - **Writing to a busy conversation:** only the Harness appends to a conversation with an active run. Raw entries appended by `Harness.commit()` or a custom task while a generation prepares its request can misplace its system prompt entries; diff --git a/packages/durable/src/errors.ts b/packages/durable/src/errors.ts index 796ae7e1f..622b8def6 100644 --- a/packages/durable/src/errors.ts +++ b/packages/durable/src/errors.ts @@ -13,10 +13,14 @@ export function errorMessage(error: unknown): string { } } -/** A transaction read a table after its first table write. Read every required row before writing. */ +/** + * A transaction read a table after its first table write, or admitted a submission (`submit(tx, draft)`) after it. Read + * every required row, and admit, before writing. + */ export class ReadAfterWrite extends Error { - constructor(method: string) { - super(`Tx.${method}() cannot read tables after the first table write`); + /** `operation` names the call, such as `Tx.task()`. */ + constructor(operation: string) { + super(`${operation} cannot read tables after the first table write`); this.name = "ReadAfterWrite"; } } diff --git a/packages/durable/src/harness/harness.ts b/packages/durable/src/harness/harness.ts index a82e40fe7..055c05bd7 100644 --- a/packages/durable/src/harness/harness.ts +++ b/packages/durable/src/harness/harness.ts @@ -123,8 +123,12 @@ class ConversationImpl implements Conversation { return this.#host.harness.commitWith((tx) => configure(tx, this.id, change), context); } - submit(submission: SubmissionDraft, context: Context): Promise { - return this.#host.submissions.submit(this.id, submission, context); + submit(submission: SubmissionDraft, context: Context): Promise; + submit(tx: Tx, submission: SubmissionDraft): Promise; + submit(first: SubmissionDraft | Tx, second: Context | SubmissionDraft): Promise { + // The overloads pair a draft with a context, and anything else (a transaction) with a draft. + if ("type" in first) return this.#host.submissions.submit(this.id, first, second as Context); + return this.#host.submissions.admit(first, this.id, second as SubmissionDraft); } async compact(instructions: string | undefined, context: Context): Promise> { @@ -536,18 +540,30 @@ function boundConversation( return operation(bind(context)); }; }; + function submit(draft: SubmissionDraft, context: Context): Promise; + function submit(tx: Tx, draft: SubmissionDraft): Promise; + async function submit( + first: SubmissionDraft | Tx, + second: Context | SubmissionDraft, + ): Promise { + binding.check(); + // The overloads pair a draft with a context, and anything else (a transaction) with a draft. + if (!("type" in first)) { + // The caller's commit carries its own context, so the invocation's abort is checked here. + binding.signal.throwIfAborted(); + return submissions.admit(first, id, second as SubmissionDraft); + } + const submission = await submissions.submit(id, first, bind(second as Context)); + return { + id: submission.id, + status: bound((callContext) => submission.status(callContext)), + wait: bound((callContext) => submission.wait(callContext)), + abort: bound((callContext) => submission.abort(callContext)), + }; + } return { id, - submit: async (draft, context) => { - binding.check(); - const submission = await submissions.submit(id, draft, bind(context)); - return { - id: submission.id, - status: bound((callContext) => submission.status(callContext)), - wait: bound((callContext) => submission.wait(callContext)), - abort: bound((callContext) => submission.abort(callContext)), - }; - }, + submit, abort: async (context, options) => { binding.check(); return tasks.abortConversation(id, options?.background === true, bind(context)); diff --git a/packages/durable/src/harness/submissions.ts b/packages/durable/src/harness/submissions.ts index 76379d1b2..df0363e67 100644 --- a/packages/durable/src/harness/submissions.ts +++ b/packages/durable/src/harness/submissions.ts @@ -6,7 +6,7 @@ import { type TelemetryContext, type TelemetrySpan, } from "@earendil-works/pi-telemetry"; -import { ConversationBusy, ConversationIdle } from "../errors.ts"; +import { ConversationBusy, ConversationIdle, ReadAfterWrite } from "../errors.ts"; import type { ObservedCommit, SessionImpl } from "../session/session.ts"; import type { ConversationId, JsonObject, Storage, SubmissionId, SubmissionRecord, Tx } from "../types.ts"; import { startRun } from "./generation.ts"; @@ -66,6 +66,36 @@ export class Submissions { /** Admit a submission in one commit; see `admitSubmission()`. */ async submit(conversationId: ConversationId, draft: SubmissionDraft, context: Context): Promise { + const { id } = await this.#traced(conversationId, draft, context, (span) => + this.#session.commitWith( + (tx) => admitSubmission(tx, conversationId, draft, this.#now(), this.#queueModes(), span), + withTelemetry(span, context), + ), + ); + return new SubmissionHandle(id, this); + } + + /** + * Admit a submission inside the caller's commit `tx`, with the rules of `submit()`. The admission is stored or lost + * with that commit. It must come before the commit's first table write, another admission's included. + */ + async admit(tx: Tx, conversationId: ConversationId, draft: SubmissionDraft): Promise { + if (!this.#session.ownsTransaction(tx)) throw new Error("The transaction belongs to another Session"); + // Enforced on every path: admission reads tables only on some (request IDs, an idle boundary, a steer's trace). + if (tx.hasTableWrite) throw new ReadAfterWrite("submit(tx, draft)"); + const admission = await this.#traced(conversationId, draft, tx.context, (span) => + admitSubmission(tx, conversationId, draft, this.#now(), this.#queueModes(), span), + ); + return admission.id; + } + + /** Run one admission in its `pi.durable.submit` span, asking for progress first, as the run it starts needs it. */ + #traced( + conversationId: ConversationId, + draft: SubmissionDraft, + context: Context, + admit: (span: TelemetrySpan) => Promise, + ): Promise { this.#resume(); // Inputs and continues start runs: root traces linked to their callers. const startsRun = draft.type !== "write"; @@ -79,12 +109,9 @@ export class Submissions { async (span) => { if (startsRun) span.addLink({ context: context.telemetry }); try { - const { id, status } = await this.#session.commitWith( - (tx) => admitSubmission(tx, conversationId, draft, this.#now(), this.#queueModes(), span), - withTelemetry(span, context), - ); - span.setAttributes({ "submission.id": id, result: status }); - return new SubmissionHandle(id, this); + const admission = await admit(span); + span.setAttributes({ "submission.id": admission.id, result: admission.status }); + return admission; } catch (error) { // Error text may contain caller content. Record only the status. span.setStatus({ status: "error" }); @@ -185,16 +212,16 @@ function checkInputEntry(entry: InputEntry): void { } /** - * Admit a submission inside a commit (spec §6); `Conversation.submit()` and conversation-owned compactions share it. A - * known request ID returns its existing submission without writing. A display-only write (only `kind` and `data`) is - * appended and settled `done` at once, busy or not, as it changes nothing the model sees; only behind a queued reset it - * queues, so it lands after the reset instead of before it in the context the reset ends. A busy conversation queues - * anything else in `pi.inbox`, or rejects `whenBusy: "reject"` input and any continue with `ConversationBusy`; an idle - * one rejects `whenIdle: "reject"` input with `ConversationIdle`. An idle continue places queued items and starts a run - * with them and itself, placing no entry of its own. An idle conversation with queued items queues it behind them and - * runs a final boundary. Otherwise idle input places its entry (`pi.user`, or the input's - * own) and starts a run, and an idle write appends its entry and settles `done`, or `stale` when its head reaches - * before the active range. + * Admit a submission inside a commit (spec §6); both `Conversation.submit()` forms and conversation-owned compactions + * share it. A known request ID returns its existing submission without writing. A display-only write (only `kind` and + * `data`) is appended and settled `done` at once, busy or not, as it changes nothing the model sees; only behind a + * queued reset it queues, so it lands after the reset instead of before it in the context the reset ends. A busy + * conversation queues anything else in `pi.inbox`, or rejects `whenBusy: "reject"` input and any continue with + * `ConversationBusy`; an idle one rejects `whenIdle: "reject"` input with `ConversationIdle`. An idle continue places + * queued items and starts a run with them and itself, placing no entry of its own. An idle conversation with queued + * items queues it behind them and runs a final boundary. Otherwise idle input places its entry (`pi.user`, or the + * input's own) and starts a run, and an idle write appends its entry and settles `done`, or `stale` when its head + * reaches before the active range. */ export async function admitSubmission( tx: Tx, diff --git a/packages/durable/src/harness/telemetry.ts b/packages/durable/src/harness/telemetry.ts index 0c5f60ec6..baebaae0d 100644 --- a/packages/durable/src/harness/telemetry.ts +++ b/packages/durable/src/harness/telemetry.ts @@ -52,7 +52,7 @@ export const DURABLE_TELEMETRY_SCHEMA = defineTelemetrySchema({ spans: { "pi.durable.submit": { description: - "Public submission admission and its commit. Inputs and continues start root traces linked to their callers; writes are children. Internal admissions emit phase events only.", + "Public submission admission: with its commit for submit(draft, context); the admission only for submit(tx, draft), whose result is provisional (a later throw or storage failure of the caller's commit leaves no submission) and whose commit events go to the caller's commit context. Inputs and continues start root traces linked to their callers; writes are children. Internal admissions emit phase events only.", parents: { kind: "any" }, startAttributes: { "submission.type": { diff --git a/packages/durable/src/harness/types.ts b/packages/durable/src/harness/types.ts index 8ec9b6c8b..3306ffe09 100644 --- a/packages/durable/src/harness/types.ts +++ b/packages/durable/src/harness/types.ts @@ -152,12 +152,15 @@ export type HooksOf = K extends Task ? /** * Invocation-bound conversation operations for tasks and tools. Rejects after the invocation ends. `submit` takes any - * submission, as `Conversation.submit()` does: a subagent's conversation is an ordinary one, so a tool may also continue - * it. + * submission, in both forms of `Conversation.submit()`: a subagent's conversation is an ordinary one, so a tool may also + * continue it, and a task may admit a submission in the same commit as its own writes (`runtime.commit()`); entries + * the admission appends there carry the task's `byTaskId`. The transaction form also rejects once the invocation is + * aborted. */ export interface ConversationHandle { readonly id: ConversationId; submit(submission: SubmissionDraft, context: Context): Promise; + submit(tx: Tx, submission: SubmissionDraft): Promise; /** `Conversation.abort()`: withdraw queued inputs, abort the ordinary ownership scope, and wait until it is idle. */ abort(context: Context, options?: ConversationAbortOptions): Promise; /** Resolve when the conversation's ordinary ownership scope has no live non-background task. */ @@ -801,6 +804,17 @@ export interface Conversation { * it is, after placing queued items; a busy conversation rejects it with `ConversationBusy`, as it never queues. */ submit(submission: SubmissionDraft, context: Context): Promise; + /** + * Admit a submission inside the caller's commit `tx` (`Harness.commit()`, `Conversation.commit()`, a task's + * `runtime.commit()`), with the same rules, and return its ID: the admission is stored with that commit, or not at all + * when the callback throws later or the commit fails. Call it before the commit's first table write (`appendEntry()`, + * `createTask()`, `createSubmission()`, another admission): otherwise it throws `ReadAfterWrite`, on every path. + * Documents may be read and written before and after it. A rejection (`ConversationBusy`, `ConversationIdle`) throws + * before any write, so the callback may catch it. Do not call `Submission` or other Harness reads inside the callback: + * they wait for the commit holding the line. Its `pi.durable.submit` span covers the admission only, so its `result` + * is provisional. Throws for a transaction of another Session. + */ + submit(tx: Tx, submission: SubmissionDraft): Promise; /** * Admit a write of a `pi.reset` entry that starts a new context, carrying `handoff` as a user message when given. * Resolves after admission; while busy, it is placed at the next boundary. diff --git a/packages/durable/src/session/session.ts b/packages/durable/src/session/session.ts index 36fd435d0..5f5df8778 100644 --- a/packages/durable/src/session/session.ts +++ b/packages/durable/src/session/session.ts @@ -344,6 +344,11 @@ export class SessionImpl implements Session { return ms; } + /** Internal: whether `tx` is the transaction of one of this Session's commits. */ + ownsTransaction(tx: Tx): tx is Transaction { + return Transaction.ownedBy(tx, this.#host); + } + /** Internal: run a read-only job on the mutation line so multi-read derivations observe one committed state. */ readOnLine(job: () => Promise): Promise { try { diff --git a/packages/durable/src/session/transaction.ts b/packages/durable/src/session/transaction.ts index 79f91ada8..857728fba 100644 --- a/packages/durable/src/session/transaction.ts +++ b/packages/durable/src/session/transaction.ts @@ -254,6 +254,24 @@ export class Transaction implements Tx { this.#scope = scope; } + /** Internal: the context of the commit this transaction belongs to. */ + get context(): Context { + return this.#context; + } + + /** Internal: whether a table write was staged, after which table reads throw `ReadAfterWrite`. */ + get hasTableWrite(): boolean { + return this.#hasTableWrite; + } + + /** + * Internal: whether `tx` is a transaction of a commit of the Session behind `host`. A brand check, so a proxy or + * another wrapper of a transaction is not one. + */ + static ownedBy(tx: Tx, host: TransactionHost): tx is Transaction { + return #host in tx && tx.#host === host; + } + // ─── Table reads ──────────────────────────────────────────────────────── conversation(id: ConversationId): Promise { @@ -1063,7 +1081,7 @@ export class Transaction implements Tx { #read(method: string, read: () => Promise): Promise { try { this.#assertOpen(); - if (this.#hasTableWrite) throw new ReadAfterWrite(method); + if (this.#hasTableWrite) throw new ReadAfterWrite(`Tx.${method}()`); return this.#track(read()); } catch (error) { return Promise.reject(error); diff --git a/packages/durable/test/harness-submit-in-commit.test.ts b/packages/durable/test/harness-submit-in-commit.test.ts index 8cd6997da..3d8ad2986 100644 --- a/packages/durable/test/harness-submit-in-commit.test.ts +++ b/packages/durable/test/harness-submit-in-commit.test.ts @@ -1,9 +1,35 @@ -// Admission for hosts that must not start runs: `whenIdle: "reject"` (#11149). -import { type AssistantMessage, fauxAssistantMessage, fauxText, fauxToolCall, Type } from "@earendil-works/pi-ai"; -import { type Conversation, ConversationIdle, defineTool, InboxDoc, LiveDoc } from "@earendil-works/pi-durable"; +// Admission for hosts that must not start runs: `whenIdle: "reject"`, and `submit(tx, draft)` inside the caller's +// commit (#11149). +import { withTelemetry } from "@earendil-works/chord/context"; +import { + type AssistantMessage, + type FauxResponseStep, + fauxAssistantMessage, + fauxText, + fauxToolCall, + Type, +} from "@earendil-works/pi-ai"; +import { + type Conversation, + ConversationBusy, + type ConversationHandle, + ConversationIdle, + defineDoc, + defineTask, + defineTool, + Harness, + InboxDoc, + LiveDoc, + MemoryStorage, + ReadAfterWrite, + type SubmissionId, + type Tx, + UserEntry, +} from "@earendil-works/pi-durable"; +import { InMemoryTelemetryContext } from "@earendil-works/pi-telemetry"; import { describe, expect, it } from "vitest"; -import { chatSetup, openChat, unanswered } from "./chat-support.ts"; -import { addTool } from "./harness-support.ts"; +import { type ChatSetup, chatSetup, openChat, unanswered, waitFor } from "./chat-support.ts"; +import { addTask, addTool } from "./harness-support.ts"; import { ControlledStorage, context } from "./session-support.ts"; import { deferred } from "./task-support.ts"; @@ -12,6 +38,17 @@ const MODEL = { provider: "faux", modelId: "faux-1" } as const; const failure = (): AssistantMessage => fauxAssistantMessage([], { stopReason: "error", errorMessage: "Invalid request" }); +/** A host's own conversation document. */ +const Sent = defineDoc<{ ids: number[] }>({ + checkpointWhen: () => false, + kind: "test.sent", + version: 1, + scope: "conversation", + history: "latest", + fork: "initial", + initial: () => ({ ids: [] }), +}); + /** Open a chat whose run hangs until aborted, and wait until it works on its first input. */ async function openBusy(storage = new ControlledStorage()) { const setup = chatSetup(); @@ -23,6 +60,48 @@ async function openBusy(storage = new ControlledStorage()) { return { ...opened, storage, first }; } +/** Open a chat whose Harness records telemetry. */ +async function openTraced(setup: ChatSetup, telemetry: InMemoryTelemetryContext) { + const harness = await Harness.open( + new MemoryStorage(), + { models: setup.models, registry: setup.registry, settings: setup.settings, telemetry }, + context, + ); + const root = await harness.root(context, { agent: { model: MODEL } }); + return { harness, root }; +} + +/** Install tool `hold`, which reports its call and waits for `release`; its first call answers `tool`. */ +function addHold(setup: ChatSetup) { + const reached = deferred(); + const release = deferred(); + addTool( + setup.registry, + defineTool({ + name: "hold", + description: "Holds until released", + parameters: Type.Object({}), + execute: async () => { + reached.resolve(); + await release.promise; + return { output: [] }; + }, + }), + ); + const call = fauxAssistantMessage([fauxToolCall("hold", {}, { id: "c1" })], { stopReason: "toolUse" }); + return { reached, release, call }; +} + +/** Settled status of submission `id`, polled without asking for progress (`Submission.wait()` would). */ +async function settledStatus(harness: Harness, id: SubmissionId) { + const submission = (await harness.submission(id, context))!; + await waitFor(async () => { + const { status } = await submission.status(context); + return status === "done" || status === "unanswered"; + }); + return submission.status(context); +} + describe("whenIdle reject", () => { // #11149 it("rejects input to an idle conversation without writing", async () => { @@ -86,6 +165,8 @@ describe("whenIdle reject", () => { await first.wait(context); const draft = { type: "input", content: "work", requestId: "r1", whenBusy: "steer", whenIdle: "reject" } as const; expect((await root.submit(draft, context)).id).toBe(first.id); + // Deduplication runs before the idle rejection in the caller's commit too. + expect(await harness.commit((tx) => root.submit(tx, draft), context)).toBe(first.id); await harness.close(context); }); @@ -120,3 +201,446 @@ describe("whenIdle reject", () => { await harness.close(context); }); }); + +describe("submit inside the caller's commit", () => { + // #11149 + it("admits with the host's own check and writes in one commit", async () => { + const { harness, root, storage, first } = await openBusy(); + const commits = storage.commits.length; + const id = await harness.commit(async (tx) => { + if ((await tx.doc(LiveDoc, root.id)).run === undefined) throw new Error("not working"); + // The host's own document write may come before the admission. + const sent = await tx.doc(Sent, root.id); + const admitted = await root.submit(tx, { type: "input", content: "steer", whenBusy: "steer" }); + sent.ids.push(admitted); + // The host's own table write comes after it. + await tx.appendEntry(root.id, { kind: "app.sent", data: { submission: admitted } }); + return admitted; + }, context); + expect(storage.commits.length).toBe(commits + 1); + const writes = storage.commits.at(-1)!; + expect(writes.some((write) => write.type === "submission" && write.value.id === id)).toBe(true); + expect(writes.some((write) => write.type === "entry" && write.value.kind === "app.sent")).toBe(true); + expect((await harness.snapshot(Sent, root.id, context))?.ids).toEqual([id]); + const inbox = await harness.snapshot(InboxDoc, root.id, context); + expect(inbox?.items).toEqual([{ id, mode: "steer", content: "steer" }]); + expect((await harness.snapshot(LiveDoc, root.id, context))?.run?.inputs).toEqual([first.id]); + await harness.close(context); + }); + + // #11149 + it("is rolled back with the commit when the callback throws later", async () => { + const storage = new ControlledStorage(); + const { harness, root } = await openChat(storage, chatSetup()); + const commits = storage.commits.length; + const failed = harness.commit(async (tx) => { + await root.submit(tx, { type: "input", content: "hi", requestId: "r1" }); + throw new Error("host check failed"); + }, context); + await expect(failed).rejects.toThrow("host check failed"); + expect(storage.commits.length).toBe(commits); + expect((await harness.snapshot(LiveDoc, root.id, context))?.run).toBeUndefined(); + expect(await harness.commit((tx) => tx.submissionByRequest(root.id, "r1"), context)).toBeUndefined(); + await harness.close(context); + }); + + // #11149 + it("leaves no submission and no run when Storage fails the commit, also after reopening", async () => { + const storage = new ControlledStorage(); + const setup = chatSetup(); + const { harness, root } = await openChat(storage, setup); + const failure = new Error("disk full"); + storage.failNextCommit(failure); + const failed = harness.commit(async (tx) => { + await root.submit(tx, { type: "input", content: "hi", requestId: "r1" }); + await tx.appendEntry(root.id, { kind: "app.sent" }); + }, context); + await expect(failed).rejects.toBe(failure); + await harness.closed; + const reopened = await openChat(storage.reopen(), setup); + expect(reopened.root.id).toBe(root.id); + const found = await reopened.harness.commit(async (tx) => { + const submission = await tx.submissionByRequest(root.id, "r1"); + const tasks = await tx.scanTasks({ conversationId: root.id }, 10); + const entries = await tx.scanEntries({ conversationId: root.id }, 10); + return { submission, tasks: tasks.items, entries: entries.items }; + }, context); + expect(found).toEqual({ submission: undefined, tasks: [], entries: [] }); + expect((await reopened.harness.snapshot(LiveDoc, root.id, context))?.run).toBeUndefined(); + await reopened.harness.close(context); + }); + + // #11149 + it("starts the run of an idle input once the commit is stored", async () => { + const setup = chatSetup(); + setup.faux.setResponses([fauxAssistantMessage([fauxText("answer")])]); + // No `resume()`: the admission asks for progress itself. + const { harness, root } = await openChat(new ControlledStorage(), setup); + const id = await harness.commit((tx) => root.submit(tx, { type: "input", content: "hi" }), context); + expect(await settledStatus(harness, id)).toMatchObject({ type: "input", status: "done" }); + await harness.close(context); + }); + + // #11149 + it("queues a model-visible write while busy and places it at the next boundary", async () => { + const setup = chatSetup(); + const hold = addHold(setup); + setup.faux.setResponses([hold.call, fauxAssistantMessage([fauxText("answer")])]); + const { harness, root } = await openChat(new ControlledStorage(), setup); + const first = await root.submit({ type: "input", content: "work" }, context); + await hold.reached.promise; + const note = { role: "user", content: "a note the model sees", timestamp: 1 } as const; + const id = await harness.commit( + (tx) => root.submit(tx, { type: "write", entry: { kind: "test.note", model: [note] } }), + context, + ); + expect((await harness.snapshot(InboxDoc, root.id, context))?.items.map((item) => item.id)).toEqual([id]); + hold.release.resolve(); + expect(await first.wait(context)).toMatchObject({ status: "done" }); + expect(await settledStatus(harness, id)).toMatchObject({ + type: "write", + status: "done", + entry: expect.any(Number), + }); + await harness.close(context); + }); + + // #11149 + it("deduplicates request IDs", async () => { + const { harness, root, storage } = await openBusy(); + const draft = { type: "input", content: "steer", whenBusy: "steer", requestId: "r1" } as const; + const first = await harness.commit((tx) => root.submit(tx, draft), context); + const commits = storage.commits.length; + const again = await harness.commit((tx) => root.submit(tx, draft), context); + expect(again).toBe(first); + expect(storage.commits.length).toBe(commits); + await harness.close(context); + }); + + // #11149 + it("rejects before any write, so the callback may catch the rejection", async () => { + const storage = new ControlledStorage(); + const { harness, root } = await openChat(storage, chatSetup()); + const commits = storage.commits.length; + const outcome = await harness.commit(async (tx) => { + try { + await root.submit(tx, { type: "input", content: "hi", whenBusy: "steer", whenIdle: "reject" }); + return "admitted"; + } catch (error) { + return error instanceof ConversationIdle ? "idle" : "other"; + } + }, context); + expect(outcome).toBe("idle"); + expect(storage.commits.length).toBe(commits); + expect((await harness.snapshot(LiveDoc, root.id, context))?.run).toBeUndefined(); + await harness.close(context); + }); + + // #11149 + it("throws ReadAfterWrite after a table write of the commit, on every path", async () => { + const { harness, root } = await openBusy(); + // A busy input without request ID or trace reads no table, and is still refused. + const afterAppend = harness.commit(async (tx) => { + await tx.appendEntry(root.id, { kind: "app.note" }); + await root.submit(tx, { type: "input", content: "steer", whenBusy: "steer" }); + }, context); + await expect(afterAppend).rejects.toBeInstanceOf(ReadAfterWrite); + await expect(afterAppend).rejects.toThrow("submit(tx, draft) cannot read tables after the first table write"); + const second = harness.commit(async (tx) => { + await root.submit(tx, { type: "input", content: "one", whenBusy: "steer" }); + await root.submit(tx, { type: "input", content: "two", whenBusy: "steer" }); + }, context); + await expect(second).rejects.toBeInstanceOf(ReadAfterWrite); + expect((await harness.snapshot(InboxDoc, root.id, context))?.items).toEqual([]); + + const idle = await openChat(new ControlledStorage(), chatSetup()); + const late = idle.harness.commit(async (tx) => { + await tx.appendEntry(idle.root.id, { kind: "app.note" }); + await idle.root.submit(tx, { type: "input", content: "hi" }); + }, context); + await expect(late).rejects.toBeInstanceOf(ReadAfterWrite); + await idle.harness.close(context); + await harness.close(context); + }); + + // #11149 + it("continues a failed run, and must precede the commit's table writes", async () => { + const setup = chatSetup(); + setup.faux.setResponses([failure, fauxAssistantMessage([fauxText("recovered")])]); + const { harness, root } = await openChat(new ControlledStorage(), setup); + const first = await root.submit({ type: "input", content: "Analyze this" }, context); + expect(await first.wait(context)).toMatchObject({ status: "unanswered", reason: "model_error" }); + + const late = harness.commit(async (tx) => { + await tx.appendEntry(root.id, { kind: "app.note" }); + await root.submit(tx, { type: "continue" }); + }, context); + await expect(late).rejects.toBeInstanceOf(ReadAfterWrite); + + const id = await harness.commit(async (tx) => { + const continued = await root.submit(tx, { type: "continue" }); + await tx.appendEntry(root.id, { kind: "app.note" }); + return continued; + }, context); + expect(await settledStatus(harness, id)).toMatchObject({ type: "continue", status: "done" }); + await harness.close(context); + }); + + // #11149 + it("rejects a busy continue and steers with whenIdle inside the commit", async () => { + const { harness, root } = await openBusy(); + const busy = harness.commit((tx) => root.submit(tx, { type: "continue" }), context); + await expect(busy).rejects.toBeInstanceOf(ConversationBusy); + const steer = await harness.commit( + (tx) => root.submit(tx, { type: "input", content: "steer", whenBusy: "steer", whenIdle: "reject" }), + context, + ); + expect((await harness.snapshot(InboxDoc, root.id, context))?.items.map((item) => item.id)).toEqual([steer]); + await harness.close(context); + }); + + // #11149 + it("throws for a transaction of another Session, or a wrapped one", async () => { + const a = await openChat(new ControlledStorage(), chatSetup()); + const b = await openChat(new ControlledStorage(), chatSetup()); + const draft = { type: "input", content: "hi" } as const; + const foreign = b.harness.commit((tx) => a.root.submit(tx, draft), context); + await expect(foreign).rejects.toThrow("The transaction belongs to another Session"); + const proxied = a.harness.commit((tx) => a.root.submit(new Proxy(tx, {}), draft), context); + await expect(proxied).rejects.toThrow("The transaction belongs to another Session"); + const plain = a.harness.commit((tx) => a.root.submit({ doc: tx.doc.bind(tx) } as unknown as Tx, draft), context); + await expect(plain).rejects.toThrow("The transaction belongs to another Session"); + expect((await a.harness.snapshot(LiveDoc, a.root.id, context))?.run).toBeUndefined(); + await a.harness.close(context); + await b.harness.close(context); + }); + + // #11149 + it("records the trace carrier and links a steer to its caller and the running trace", async () => { + const setup = chatSetup(); + const telemetry = new InMemoryTelemetryContext(); + const busy = unanswered(); + setup.faux.setResponses([busy.step]); + const { harness, root } = await openTraced(setup, telemetry); + await root.submit({ type: "input", content: "work" }, context); + await busy.reached; + const run = (await harness.inspect(context)).tasks.find((task) => task.record.kind === "pi.generation")!.record; + const id = await telemetry.startSpan({ name: "caller" }, (caller) => + harness.commit( + (tx) => root.submit(tx, { type: "input", content: "steer", whenBusy: "steer" }), + withTelemetry(caller, context), + ), + ); + const spans = telemetry.getSpans(); + const caller = spans.find((span) => span.name === "caller")!; + const submit = spans.find( + (span) => span.name === "pi.durable.submit" && span.attributes["submission.id"] === id, + )!; + expect(submit).toMatchObject({ parentId: null, attributes: { result: "queued" } }); + expect(submit.links).toEqual([ + { context: caller.context, attributes: {} }, + { context: run.trace, attributes: {} }, + ]); + expect((await (await harness.submission(id, context))!.status(context)).trace).toEqual(submit.context); + await harness.close(context); + }); + + // #11149 + it("starts an idle input's run in the admission's trace", async () => { + const setup = chatSetup(); + const telemetry = new InMemoryTelemetryContext(); + const busy = unanswered(); + setup.faux.setResponses([busy.step]); + const { harness, root } = await openTraced(setup, telemetry); + const id = await telemetry.startSpan({ name: "caller" }, (caller) => + harness.commit((tx) => root.submit(tx, { type: "input", content: "hi" }), withTelemetry(caller, context)), + ); + await busy.reached; + const submit = telemetry + .getSpans() + .find((span) => span.name === "pi.durable.submit" && span.attributes["submission.id"] === id)!; + const record = await (await harness.submission(id, context))!.status(context); + expect(record.trace).toEqual(submit.context); + const run = (await harness.inspect(context)).tasks.find((task) => task.record.kind === "pi.generation")!.record; + expect(run.trace).toEqual(record.trace); + await harness.close(context); + }); + + // #11149 + it("makes a write a child of its caller and leaves the commit's events to the caller's commit", async () => { + const setup = chatSetup(); + const telemetry = new InMemoryTelemetryContext(); + const { harness, root } = await openTraced(setup, telemetry); + const id = await telemetry.startSpan({ name: "caller" }, (caller) => + harness.commit( + (tx) => root.submit(tx, { type: "write", entry: { kind: "app.note" } }), + withTelemetry(caller, context), + ), + ); + const spans = telemetry.getSpans(); + const caller = spans.find((span) => span.name === "caller")!; + const submit = spans.find((span) => span.name === "pi.durable.submit")!; + expect(submit).toMatchObject({ + parentContext: caller.context, + settled: true, + attributes: { "submission.id": id, result: "done" }, + events: [], + }); + expect(caller.events.map((event) => event.name)).toEqual(["pi.durable.entry", "pi.durable.submission"]); + await harness.close(context); + }); + + // #11149 + it("ends its span before the commit, and a later throw leaves no submission", async () => { + const setup = chatSetup(); + const telemetry = new InMemoryTelemetryContext(); + const { harness, root } = await openTraced(setup, telemetry); + await telemetry.startSpan({ name: "caller" }, async (caller) => { + const failed = harness.commit( + async (tx) => { + await root.submit(tx, { type: "write", entry: { kind: "app.note" } }); + throw new Error("later"); + }, + withTelemetry(caller, context), + ); + await expect(failed).rejects.toThrow("later"); + }); + const spans = telemetry.getSpans(); + const submit = spans.find((span) => span.name === "pi.durable.submit")!; + // The span reported an admission the commit then discarded. + expect(submit).toMatchObject({ settled: true, attributes: { result: "done" } }); + const id = submit.attributes["submission.id"] as SubmissionId; + expect(await harness.submission(id, context)).toBeUndefined(); + expect(spans.find((span) => span.name === "caller")!.events).toEqual([]); + await harness.close(context); + }); + + // #11149 + it("admits atomically from a tool's commit through its conversation handle", async () => { + const setup = chatSetup(); + let other: Conversation | undefined; + let toolTask: number | undefined; + addTool( + setup.registry, + defineTool({ + name: "delegate", + description: "Delegates to another conversation", + parameters: Type.Object({}), + execute: async (_args, api, callContext) => { + toolTask = api.taskId; + const handle = (await api.conversation(other!.id, callContext))!; + const id = await api.commit(async (tx) => { + const admitted = await handle.submit(tx, { type: "input", content: "from the tool" }); + await tx.appendEntry(other!.id, { kind: "app.delegated", data: { submission: admitted } }); + return admitted; + }, callContext); + return { output: [{ type: "text", text: `submitted ${id}` }] }; + }, + }), + ); + const reply: FauxResponseStep = (request) => { + const last = request.messages.at(-1); + if (last?.role === "user" && last.content === "delegate") { + return fauxAssistantMessage([fauxToolCall("delegate", {}, { id: "c1" })], { stopReason: "toolUse" }); + } + return fauxAssistantMessage([fauxText("ok")]); + }; + setup.faux.setResponses([reply, reply, reply]); + const { harness, root } = await openChat(new ControlledStorage(), setup); + other = await harness.createConversation({ ownership: { kind: "ownerless" }, agent: { model: MODEL } }, context); + const run = await root.submit({ type: "input", content: "delegate" }, context); + expect(await run.wait(context)).toMatchObject({ status: "done" }); + await harness.waitForIdle(context); + const entries = [...(await other.entries({}, 100, undefined, context)).items].reverse(); + const delegated = entries.find((entry) => entry.kind === "app.delegated"); + const submission = (delegated?.data as { submission: SubmissionId }).submission; + const settled = await settledStatus(harness, submission); + if (settled.type !== "input" || settled.status !== "done") throw new Error(`Unexpected ${settled.status}`); + // The placed input carries the tool task's byTaskId, as the commit's own appends do. + const user = await other.commit((tx) => tx.entry(UserEntry, settled.entry), context); + expect(user?.byTaskId).toBe(toolTask); + await harness.close(context); + }); + + // #11149 + it("admits inside a task's terminal commit", async () => { + const setup = chatSetup(); + setup.faux.setResponses([fauxAssistantMessage([fauxText("answer")])]); + const Admitting = defineTask({ + name: "test.admitting", + version: 1, + initial: () => ({ phase: "run" }), + phases: { + run: async (_task, runtime, taskContext) => { + const handle = (await runtime.conversation(runtime.conversationId, taskContext))!; + await runtime.commit(async (tx) => { + const id = await handle.submit(tx, { type: "input", content: "from the task" }); + return { status: "terminal", outcome: { status: "completed", result: id } }; + }, taskContext); + }, + }, + abort: (_task, runtime, taskContext) => + runtime.commit(() => ({ status: "terminal", outcome: { status: "aborted" } }), taskContext), + }); + addTask(setup.registry, Admitting); + const { harness, root } = await openChat(new ControlledStorage(), setup); + const task = await root.commit( + (tx) => tx.createTask(Admitting, null, { ownership: { kind: "conversation" } }), + context, + ); + harness.resume(); + const settled = await harness.waitForTask(task, context); + if (settled.state.outcome.status !== "completed") throw new Error(settled.state.outcome.status); + const status = await settledStatus(harness, settled.state.outcome.result); + if (status.type !== "input" || status.status !== "done") throw new Error(`Unexpected ${status.status}`); + const user = await root.commit((tx) => tx.entry(UserEntry, status.entry), context); + expect(user?.byTaskId).toBe(task); + await harness.close(context); + }); + + // #11149 + it("rejects through a handle once its invocation is aborted, and once it ended", async () => { + const setup = chatSetup(); + let other: Conversation | undefined; + let handle: ConversationHandle | undefined; + const started = deferred(); + const abortSeen = deferred(); + const release = deferred(); + addTool( + setup.registry, + defineTool({ + name: "hold", + description: "Holds until released, also when aborted", + parameters: Type.Object({}), + execute: async (_args, api, callContext) => { + handle = (await api.conversation(other!.id, callContext))!; + callContext.abortSignal!.addEventListener("abort", () => abortSeen.resolve(), { once: true }); + started.resolve(); + await release.promise; + return { output: [] }; + }, + }), + ); + setup.faux.setResponses([ + fauxAssistantMessage([fauxToolCall("hold", {}, { id: "c1" })], { stopReason: "toolUse" }), + fauxAssistantMessage([fauxText("ok")]), + ]); + const { harness, root } = await openChat(new ControlledStorage(), setup); + other = await harness.createConversation({ ownership: { kind: "ownerless" }, agent: { model: MODEL } }, context); + const run = await root.submit({ type: "input", content: "go" }, context); + await started.promise; + const tool = (await harness.inspect(context)).tasks.find((task) => task.record.kind === "pi.tool")!.record; + const aborting = harness.abortTask(tool.id, context); + await abortSeen.promise; + const draft = { type: "input", content: "late" } as const; + await expect(handle!.submit(draft, context)).rejects.toThrow(); + await expect(harness.commit((tx) => handle!.submit(tx, draft), context)).rejects.toThrow(); + expect((await harness.snapshot(LiveDoc, other.id, context))?.run).toBeUndefined(); + release.resolve(); + await aborting; + await run.wait(context); + await expect(harness.commit((tx) => handle!.submit(tx, draft), context)).rejects.toThrow(); + expect((await harness.snapshot(LiveDoc, other.id, context))?.run).toBeUndefined(); + await harness.close(context); + }); +});