From 031c10e6216526a436e8036a620bab46900120da Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 10 Nov 2026 04:51:29 -0500 Subject: [PATCH 1/3] Queue Sidekick follow-ups behind the live automatic-feedback request A follow-up sent while the owner is still asking the provider for automatic feedback was refused by the dispatch recovery guard, because the feedback record saved with turn:null reads as unresolved until its canonical Prompt completes. The UI also showed [review recovery] for that ordinary in-flight state. The owner now records the exact position of the feedback record its running exchange wrote, before the refresh that exposes it, and releases it by identity when that exchange records its outcome, fails or unwinds. SidekickOwner.recoveryPending is the retained recovery answer with only that position set aside; the dispatch guard and the recovery presentation read it. The reading itself still reports the record as unresolved, cold owners hold no exception, and earlier or mismatched feedback keeps the guard. Queue admission starts nothing, and the guard is rechecked before a waiting message starts. Fixes #1183. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/repl/program.ts | 5 +- packages/cli/src/repl/sidekick/dispatch.ts | 7 +- packages/cli/src/repl/sidekick/model.ts | 57 ++- packages/cli/src/repl/sidekick/owner.ts | 59 ++- .../cli/src/repl/sidekick/recovery-state.ts | 5 +- .../cli/tests/repl-sidekick-dispatch.test.ts | 278 ++++++++--- .../cli/tests/repl-sidekick-journey.test.ts | 57 +++ .../cli/tests/repl-sidekick-recovery.test.ts | 433 +++++++++++------- specs/repl-spec.md | 15 +- 9 files changed, 676 insertions(+), 240 deletions(-) diff --git a/packages/cli/src/repl/program.ts b/packages/cli/src/repl/program.ts index f324d91eb..09a7f92d3 100644 --- a/packages/cli/src/repl/program.ts +++ b/packages/cli/src/repl/program.ts @@ -2922,10 +2922,7 @@ function sidekickReadingOf( // current association, which is what a person who has not chosen is reading. const reading = selected ?? active; return Object.freeze({ - recovery: - read.value.recoveryPending || - (read.value.recoveries.length === 0 && - read.value.unestablished.some((one) => one.conversation === active)), + recovery: owner.recoveryPending, chat: sidekickChatRows(read.value, owner.previewing), // What the replies of this history actually did to this execution's // entries, as the coordinates their records hold. Detached from the chat diff --git a/packages/cli/src/repl/sidekick/dispatch.ts b/packages/cli/src/repl/sidekick/dispatch.ts index 76ce8c0e1..c2ee9ac86 100644 --- a/packages/cli/src/repl/sidekick/dispatch.ts +++ b/packages/cli/src/repl/sidekick/dispatch.ts @@ -204,12 +204,7 @@ export function* useSidekickDispatch(options: { if (!reading.ok) { return reading; } - if ( - !eligible(head) && - (reading.value.recoveryPending || - (reading.value.recoveries.length === 0 && - reading.value.unestablished.some((one) => one.conversation === reading.value.active))) - ) { + if (!eligible(head) && owner.recoveryPending) { return Err( new Error("This conversation holds feedback whose acceptance cannot be established."), ); diff --git a/packages/cli/src/repl/sidekick/model.ts b/packages/cli/src/repl/sidekick/model.ts index 8f793bf26..9c020613e 100644 --- a/packages/cli/src/repl/sidekick/model.ts +++ b/packages/cli/src/repl/sidekick/model.ts @@ -890,6 +890,19 @@ function slot(conversation: string, response: string, at: number): string { * the nearest readable shape would show somebody a conversation they never had. */ export function projectSidekick(records: readonly SidekickRecord[]): Result { + const projected = projectHistory(records); + return projected.ok ? Ok(projected.value.reading) : projected; +} + +/** A reading, with where each of its unestablished feedback facts was read from. */ +export interface ProjectedHistory { + readonly reading: SidekickReading; + /** The record position of each `reading.unestablished` entry, index for index. */ + readonly unestablishedAt: readonly number[]; +} + +/** {@link projectSidekick}, keeping the record positions a live owner's exception needs. */ +export function projectHistory(records: readonly SidekickRecord[]): Result { // The request boundaries first, by the same rules the live owner reads them // by. Everything below that associates a message, a reply or an orientation // with a saved turn asks these rather than comparing numbers. @@ -1060,7 +1073,7 @@ export function projectSidekick(records: readonly SidekickRecord[]): Result(); let preferences: SidekickPreferences | undefined; const delivered = new Map(); - const unestablished: { conversation: string; turn: number | null }[] = []; + const unestablished: { conversation: string; turn: number | null; at: number }[] = []; /** * Feedback text this history retained and never acknowledged, in record order. * @@ -1335,7 +1348,7 @@ export function projectSidekick(records: readonly SidekickRecord[]): Result { const held = conversations.get(id)!; @@ -1529,8 +1542,40 @@ export function projectSidekick(records: readonly SidekickRecord[]): Result Object.freeze({ conversation, turn })), ), - unestablished: Object.freeze(unestablished.map((held) => Object.freeze(held))), + unestablished: Object.freeze( + unestablished.map((held) => + Object.freeze({ conversation: held.conversation, turn: held.turn }), + ), + ), answers: Object.freeze(answers.map((held) => Object.freeze(held))), }), + unestablishedAt: Object.freeze(unestablished.map((held) => held.at)), + }); +} + +/** + * Whether the active conversation needs recovery before anything else is sent. + * + * `live` names the one feedback record a live owner is still asking the + * provider about. Only that exact record is set aside, and only by its position: + * earlier feedback with identical text, a mismatched acknowledgement and an + * unused recovery decision each keep the answer `true`. + */ +export function recoveryRequired( + records: readonly SidekickRecord[], + history: ProjectedHistory, + live: number | undefined, +): boolean { + const { reading } = history; + const active = reading.active; + if (active === undefined) { + return false; + } + return ( + recoveryBlocked(records, reading.requests, reading.recoveries, active, undefined, live) || + (reading.recoveries.length === 0 && + reading.unestablished.some( + (one, index) => one.conversation === active && history.unestablishedAt[index] !== live, + )) ); } diff --git a/packages/cli/src/repl/sidekick/owner.ts b/packages/cli/src/repl/sidekick/owner.ts index d60c852c8..858348b6a 100644 --- a/packages/cli/src/repl/sidekick/owner.ts +++ b/packages/cli/src/repl/sidekick/owner.ts @@ -126,8 +126,14 @@ import { ReplHost } from "../host.ts"; import { API } from "@executablemd/runtime"; import { conversationStream, failedOutcome } from "./history.ts"; import type { SidekickRecord, SidekickWriter } from "./history.ts"; -import { applicableHumanStop, projectSidekick, requestsFromHistory } from "./model.ts"; -import type { SidekickReading } from "./model.ts"; +import { + applicableHumanStop, + projectHistory, + projectSidekick, + recoveryRequired, + requestsFromHistory, +} from "./model.ts"; +import type { ProjectedHistory, SidekickReading } from "./model.ts"; import { controlJournal } from "./admission.ts"; import { responseSurface, @@ -505,6 +511,15 @@ export interface SidekickOwner { * holds one is the one answer that cannot be corrected later. */ readonly reading: Result; + /** + * Whether the active conversation needs recovery before anything else is sent. + * + * {@link SidekickOwner.reading} with one exception: the feedback record this + * owner's running exchange is still asking the provider about. That record + * stays unresolved in the reading, and stops being excepted once its + * exchange ends, fails or is stopped. An unreadable history answers `true`. + */ + readonly recoveryPending: boolean; /** Whether a cycle is running, so a second one refuses rather than queueing. */ readonly busy: boolean; /** @@ -796,6 +811,29 @@ export function useSidekickOwner(input: SidekickOwnerInput): Operation = Ok(EMPTY); let applicable: HumanStopFact | undefined; + /** + * The feedback record this owner's running exchange is asking about, by position. + * + * Set before the refresh that first shows the record and cleared by its + * own exchange, by identity. Nothing durable holds it, so a reopened owner + * has none. + */ + let inFlight: { readonly at: number } | undefined; + let recovering = false; + /** What the last refresh read, kept so a change to `inFlight` needs no reread. */ + let projected: { records: readonly SidekickRecord[]; history: ProjectedHistory } | undefined; + function reckon(): void { + recovering = + projected === undefined || + recoveryRequired(projected.records, projected.history, inFlight?.at); + } + function relinquish(held: { readonly at: number } | undefined): void { + if (held !== undefined && inFlight === held) { + inFlight = undefined; + reckon(); + moved(); + } + } /** * The response whose generation this process is watching, where one is. * @@ -1020,7 +1058,10 @@ export function useSidekickOwner(input: SidekickOwnerInput): Operation { const records = storage === undefined ? [] : yield* storage.writer.readAll(); - reading = projectSidekick(records); + const history = projectHistory(records); + reading = history.ok ? Ok(history.value.reading) : history; + projected = history.ok ? { records, history: history.value } : undefined; + reckon(); applicable = reading.ok ? applicableHumanStop(reading.value, records) : undefined; // Every re-read is a change somebody may be looking at: a message landing, // a reply, a failed turn and a configuration all come through here. @@ -2461,6 +2502,7 @@ export function useSidekickOwner(input: SidekickOwnerInput): Operation { @@ -2473,9 +2515,12 @@ export function useSidekickOwner(input: SidekickOwnerInput): Operation { return reading; }, + get recoveryPending(): boolean { + return recovering; + }, get busy(): boolean { return busy; }, diff --git a/packages/cli/src/repl/sidekick/recovery-state.ts b/packages/cli/src/repl/sidekick/recovery-state.ts index be03ea835..e47d684e4 100644 --- a/packages/cli/src/repl/sidekick/recovery-state.ts +++ b/packages/cli/src/repl/sidekick/recovery-state.ts @@ -254,6 +254,7 @@ export function recoveryBlocked( dispositions: readonly RecoveryDisposition[], conversation: string, permission?: string, + live?: number, ): boolean { const retired = new Set( dispositions @@ -271,5 +272,7 @@ export function recoveryBlocked( ) { return true; } - return uncertainRequests(records, requests, conversation).some((at) => !retired.has(at)); + return uncertainRequests(records, requests, conversation).some( + (at) => at !== live && !retired.has(at), + ); } diff --git a/packages/cli/tests/repl-sidekick-dispatch.test.ts b/packages/cli/tests/repl-sidekick-dispatch.test.ts index a7e81196c..9b88bb52f 100644 --- a/packages/cli/tests/repl-sidekick-dispatch.test.ts +++ b/packages/cli/tests/repl-sidekick-dispatch.test.ts @@ -878,62 +878,63 @@ describe("987 — held recovery admission and queue", () => { } }); +function reconciling(held: Harness): () => Operation> { + return function* () { + if (held.execution.chat.form !== "paired") { + throw new Error("Expected paired history."); + } + yield* held.execution.stream.settled(); + const entries = held.session.reading(undefined); + if (!entries.ok) { + throw entries.error; + } + const findings = yield* coldReservations( + yield* held.execution.chat.writer.readAll(), + entries.value.model.entries, + held.execution.chat.location.entries, + ); + if (!findings.ok) { + return findings; + } + for (const finding of findings.value) { + yield* retainResolution(held.execution.chat.writer, finding); + } + return Ok(undefined); + }; +} +function stopping(held: Harness): Operation { + return useSidekickDispatch({ + owner: held.owner, + admission: createSidekickAdmission(), + execution: held.execution.id, + workspace: held.workspace, + changed() {}, + reconcile: reconciling(held), + }); +} +function* counted(): Operation<{ held: Harness; effects(): number }> { + let effects = 0; + const held = yield* useHarness(undefined, { + entryInstallation: { + components: [ + { + name: "CountedEffect", + origin: "test", + props: { type: "object", additionalProperties: false }, + factory: () => + function* () { + effects++; + return "counted once"; + }, + }, + ], + }, + }); + return { held, effects: () => effects }; +} +const SUBMIT = fills("") + "\n\nWorking."; + describe("1151 — ordinary Send after a joined human Stop", () => { - function reconciling(held: Harness): () => Operation> { - return function* () { - if (held.execution.chat.form !== "paired") { - throw new Error("Expected paired history."); - } - yield* held.execution.stream.settled(); - const entries = held.session.reading(undefined); - if (!entries.ok) { - throw entries.error; - } - const findings = yield* coldReservations( - yield* held.execution.chat.writer.readAll(), - entries.value.model.entries, - held.execution.chat.location.entries, - ); - if (!findings.ok) { - return findings; - } - for (const finding of findings.value) { - yield* retainResolution(held.execution.chat.writer, finding); - } - return Ok(undefined); - }; - } - function stopping(held: Harness): Operation { - return useSidekickDispatch({ - owner: held.owner, - admission: createSidekickAdmission(), - execution: held.execution.id, - workspace: held.workspace, - changed() {}, - reconcile: reconciling(held), - }); - } - function* counted(): Operation<{ held: Harness; effects(): number }> { - let effects = 0; - const held = yield* useHarness(undefined, { - entryInstallation: { - components: [ - { - name: "CountedEffect", - origin: "test", - props: { type: "object", additionalProperties: false }, - factory: () => - function* () { - effects++; - return "counted once"; - }, - }, - ], - }, - }); - return { held, effects: () => effects }; - } - const SUBMIT = fills("") + "\n\nWorking."; function promptEvents(all: readonly SidekickRecord[]): number { return all.filter((one) => { if (one.kind !== "event") { @@ -1070,3 +1071,170 @@ describe("1151 — ordinary Send after a joined human Stop", () => { ]); }); }); + +function conversationOf(held: Harness): string | undefined { + const reading = held.owner.reading; + if (!reading.ok) { + throw reading.error; + } + return reading.value.active; +} + +describe("1183 — admission behind live automatic feedback", () => { + const FOLLOW_UP = "When that finishes, explain the changes."; + function* feedbackHeld(held: Harness, dispatch: SidekickDispatch): Operation { + const first = yield* send(dispatch, "Run the counted effect."); + yield* held.fake.holdingTurns(1); + expect(held.fake.prompts).toHaveLength(2); + expect(held.fake.prompts[1]).toContain("counted once"); + return first; + } + + it("A5: UI and ACP work wait in FIFO behind the exact live request, and a withdrawn one never starts", function* () { + const { held, effects } = yield* counted(); + held.fake.script({ reply: SUBMIT }); + held.fake.script({ reply: "Feedback answer.", held: true }); + held.fake.script({ reply: "UI answer." }); + held.fake.script({ reply: "ACP answer." }); + const dispatch = yield* stopping(held); + const first = yield* feedbackHeld(held, dispatch); + const conversation = conversationOf(held); + const raw = held.owner.reading; + if (!raw.ok) { + throw raw.error; + } + expect(raw.value.unestablished).toEqual([{ conversation, turn: null }]); + expect(raw.value.recoveryPending).toBe(true); + expect(held.owner.recoveryPending).toBe(false); + + const ui = yield* send(dispatch, FOLLOW_UP); + const withdrawn = yield* send(dispatch, "Withdrawn before it starts."); + const acp = yield* scoped(function* () { + yield* SidekickOrigin.set({ kind: "acp", attachment: "attachment" }); + return yield* send(dispatch, "ACP follow-up."); + }); + expect([ui, withdrawn, acp].map((one) => one.snapshot.state)).toEqual([ + "queued", + "queued", + "queued", + ]); + expect(dispatch.queue.held).toBe(undefined); + yield* withdrawn.cancel(); + expect((yield* withdrawn.completion).ok).toBe(false); + expect(dispatch.queue.pending.map((one) => one.text)).toEqual([FOLLOW_UP, "ACP follow-up."]); + expect(dispatch.current).toBe(first); + expect(held.fake.prompts).toHaveLength(2); + + held.fake.releaseTurn(); + for (const exchange of [first, ui, acp]) { + const done = yield* exchange.completion; + if (!done.ok) { + throw done.error; + } + } + expect(held.fake.prompts).toHaveLength(4); + expect(held.fake.prompts[2]).toContain(FOLLOW_UP); + expect(held.fake.prompts[3]).toContain("ACP follow-up."); + expect(held.fake.prompts.some((one) => one.includes("Withdrawn before it starts."))).toBe( + false, + ); + const after = yield* records(held.execution); + expect( + after.flatMap((one) => + one.kind === "message" ? [[one.text, one.origin.kind, one.conversation]] : [], + ), + ).toEqual([ + ["Run the counted effect.", "ui", conversation], + [FOLLOW_UP, "ui", conversation], + ["ACP follow-up.", "acp", conversation], + ]); + expect( + after.flatMap((one) => (one.kind === "feedback" ? [one.turn === null ? null : "named"] : [])), + ).toEqual([null, "named"]); + const settled = held.owner.reading; + if (!settled.ok) { + throw settled.error; + } + expect(settled.value.unestablished).toEqual([]); + expect(settled.value.recoveryPending).toBe(false); + expect(held.owner.recoveryPending).toBe(false); + expect(held.session.model.entries).toHaveLength(1); + expect(effects()).toBe(1); + }); + + it("A3: a failed live request holds the waiting message and keeps its recovery", function* () { + const { held, effects } = yield* counted(); + held.fake.script({ reply: SUBMIT }); + held.fake.script({ + reply: "Unfinished", + held: true, + failure: "The provider failed the feedback.", + }); + const dispatch = yield* stopping(held); + const first = yield* feedbackHeld(held, dispatch); + const next = yield* send(dispatch, FOLLOW_UP); + expect(next.snapshot.state).toBe("queued"); + held.fake.releaseTurn(); + const failed = yield* first.completion; + expect(failed.ok ? "" : failed.error.message).toContain("The provider failed the feedback."); + expect(dispatch.queue.held?.kind).toBe("failure"); + expect(dispatch.queue.pending.map((one) => one.text)).toEqual([FOLLOW_UP]); + expect(next.snapshot.state).toBe("queued"); + expect(held.owner.recoveryPending).toBe(true); + const reading = held.owner.reading; + if (!reading.ok) { + throw reading.error; + } + expect(reading.value.unestablished).toEqual([ + { conversation: reading.value.active, turn: null }, + ]); + const resumed = yield* dispatch.resume(); + expect(resumed.ok ? "" : resumed.error.message).toContain("cannot be established"); + const another = yield* dispatch.send("Another follow-up."); + expect(another.ok ? "" : another.error.message).toContain("cannot be established"); + expect(dispatch.queue.pending.map((one) => one.text)).toEqual([FOLLOW_UP]); + expect(held.fake.prompts).toHaveLength(2); + expect(held.session.model.entries).toHaveLength(1); + expect(effects()).toBe(1); + }); + + it("A3: a human Stop of the live request joins and lends no consent to the waiting message", function* () { + const { held, effects } = yield* counted(); + held.fake.script({ reply: SUBMIT }); + held.fake.script({ deltas: ["Thinking", " further."], holdAfter: 1 }); + const dispatch = yield* stopping(held); + const first = yield* send(dispatch, "Run the counted effect."); + yield* held.fake.streamingTurns(1); + const older = yield* send(dispatch, FOLLOW_UP); + expect(older.snapshot.state).toBe("queued"); + expect(held.owner.recoveryPending).toBe(false); + dispatch.hold(); + yield* first.cancel("human"); + expect((yield* first.completion).ok).toBe(false); + const stopped = yield* records(held.execution); + expect(stopped.filter((one) => one.kind === "human-stop")).toHaveLength(1); + expect(held.owner.recoveryPending).toBe(true); + expect(dispatch.queue.held?.kind).toBe("stopped"); + const refused = yield* dispatch.resume(); + expect(refused.ok ? "" : refused.error.message).toContain("cannot be established"); + expect(dispatch.queue.pending.map((one) => one.text)).toEqual([FOLLOW_UP]); + expect(held.fake.prompts).toHaveLength(2); + expect(yield* records(held.execution)).toEqual(stopped); + + const fresh = yield* send(dispatch, "The post-Stop instruction."); + yield* older.cancel(); + held.fake.script({ reply: "The post-Stop answer." }); + expect((yield* dispatch.resume()).ok).toBe(true); + const sent = yield* fresh.completion; + if (!sent.ok) { + throw sent.error; + } + expect(held.fake.prompts).toHaveLength(3); + expect(held.fake.prompts[2]).toContain("The post-Stop instruction."); + expect(held.fake.prompts.some((one) => one.includes(FOLLOW_UP))).toBe(false); + const after = yield* records(held.execution); + expect(after.filter((one) => one.kind === "recovery-decision")).toHaveLength(1); + expect(held.session.model.entries).toHaveLength(1); + expect(effects()).toBe(1); + }); +}); diff --git a/packages/cli/tests/repl-sidekick-journey.test.ts b/packages/cli/tests/repl-sidekick-journey.test.ts index b0a56e143..c4777de05 100644 --- a/packages/cli/tests/repl-sidekick-journey.test.ts +++ b/packages/cli/tests/repl-sidekick-journey.test.ts @@ -95,6 +95,7 @@ import type { MountedRepl } from "./support/mounted-repl.ts"; import { useReplPage } from "./support/repl-page.ts"; import type { ReplPage, ScreenReading } from "./support/repl-page.ts"; import { + COMPOSER_VALUE, composerHolds, draftHolds, draftText, @@ -5129,3 +5130,59 @@ describe("protected Sidekick question feedback", () => { expect(held.fake.prompts.some((prompt) => prompt.includes("useful"))).toBe(true); }); }); + +describe("1183 — a follow-up sent during live automatic feedback", () => { + it("A1/A2: queues visibly behind the held feedback and starts once on the same conversation", function* () { + const page = yield* useReplPage(); + const FOLLOW_UP = "When that finishes, explain the changes."; + page.repl.fake.script({ reply: followSubmission("COUNTED-ENTRY") }); + page.repl.fake.script({ reply: "Feedback answer.", held: true }); + page.repl.fake.script({ reply: "Follow-up answer." }); + yield* sendMessage(page, "run the counted entry"); + yield* page.repl.fake.holdingTurns(1); + expect(page.repl.fake.prompts).toHaveLength(2); + expect(page.repl.fake.prompts[1]).toContain("Entry entry-1 completed."); + const held = yield* settledReading(page.repl); + expect(held.model.entries.map((entry) => entry.terminal?.status)).toEqual(["ok"]); + expect(held.chat.filter((record) => record.kind === "feedback")).toHaveLength(1); + + yield* page.press("QUEUE"); + yield* sendMessage(page, FOLLOW_UP); + yield* page.until( + "the queued follow-up row and the cleared composer", + (one) => + one.shows(FOLLOW_UP) && + one.found(FOLLOW_UP, COMPOSER_VALUE) === 0 && + one.shows("QUEUE 1/16"), + ); + yield* intoComposer(page); + yield* typed(page, "newer unsent text"); + const queued = page.screen(); + expect(queued.shows("[review recovery]")).toBe(false); + expect(queued.shows("cannot be established")).toBe(false); + expect(page.repl.fake.prompts).toHaveLength(2); + const before = yield* settledReading(page.repl); + expect(before.chat.filter((record) => record.kind === "message")).toHaveLength(1); + + page.repl.fake.releaseTurn(); + yield* page.until("the follow-up's answer", (one) => one.shows("Follow-up answer.")); + yield* savedReplies(page.repl, 3); + yield* page.until("the follow-up joined", sidekickIdle); + expect(page.repl.fake.prompts).toHaveLength(3); + expect(page.repl.fake.prompts[2]).toContain(FOLLOW_UP); + expect(page.screen().found("newer unsent text", COMPOSER_VALUE)).toBe(1); + expect(page.screen().shows("[review recovery]")).toBe(false); + const after = yield* settledReading(page.repl); + const messages = after.chat.flatMap((record) => (record.kind === "message" ? [record] : [])); + expect(messages.map((record) => record.text)).toEqual(["run the counted entry", FOLLOW_UP]); + expect(new Set(messages.map((record) => record.conversation)).size).toBe(1); + expect( + after.chat.filter((record) => record.kind === "feedback" && record.turn === null), + ).toHaveLength(1); + expect(after.model.entries.map((entry) => entry.terminal?.status)).toEqual(["ok"]); + expect(after.model.entries[0]?.terminal?.output).toContain("COUNTED-ENTRY"); + const known = yield* projected(after.chat); + expect(known.unestablished).toEqual([]); + expect(known.recoveryPending).toBe(false); + }); +}); diff --git a/packages/cli/tests/repl-sidekick-recovery.test.ts b/packages/cli/tests/repl-sidekick-recovery.test.ts index 29cddb1cb..e69a8d07a 100644 --- a/packages/cli/tests/repl-sidekick-recovery.test.ts +++ b/packages/cli/tests/repl-sidekick-recovery.test.ts @@ -51,6 +51,7 @@ import type { SidekickDispatch, SidekickExchange } from "../src/repl/sidekick/di import type { SidekickOwner } from "../src/repl/sidekick/owner.ts"; import type { PairedChatStorage } from "../src/repl/sidekick/repository.ts"; import type { Harness } from "./fixtures/repl/issue-883/sidekick-harness.ts"; +import type { ScriptedTurn } from "./support/fake-acp.ts"; import { useTempFileCompiler, Elicitation } from "@executablemd/core"; import { parseDurableEvent, serializeDurableEvent } from "@executablemd/durable-streams"; import { AGENT_PROMPT, parsePromptRecord } from "@executablemd/core/host"; @@ -68,7 +69,7 @@ import { } from "../src/repl/sidekick/history.ts"; import type { ControlRecord, SidekickRecord } from "../src/repl/sidekick/history.ts"; import { designatedResults } from "../src/repl/sidekick/feedback.ts"; -import { projectSidekick } from "../src/repl/sidekick/model.ts"; +import { projectHistory, projectSidekick, recoveryRequired } from "../src/repl/sidekick/model.ts"; import { sidekickChatRows } from "../src/repl/program.ts"; import type { ReplSidekickChatRow } from "../src/repl/application.ts"; import type { SidekickReading } from "../src/repl/sidekick/model.ts"; @@ -3709,177 +3710,179 @@ describe("987 correction — cold prepared replacement", () => { }); }); -describe("1151 — cold ordinary Send after a retained human Stop", () => { - type Administration = "clean" | "active" | "forced" | "missing"; - function administered(mode: Administration): AgentSessionOwnershipAdministration | undefined { - if (mode === "missing") { - return undefined; - } - return { - *inspectOwnership(keyDigest) { - return Ok({ - keyDigest, - recordRevision: mode, - record: { - keyDigest, - state: mode === "active" ? "active" : "idle", - release: mode === "clean" ? "quiesced" : mode === "forced" ? "forced" : "none", - ownerKind: "session", - operationId: "retained-owner", - lastForcedRecovery: null, - }, - }); - }, - *forceRecoverOwnership() { - throw new Error("Recovery must not force ownership."); - }, - }; +type Administration = "clean" | "active" | "forced" | "missing"; +function administered(mode: Administration): AgentSessionOwnershipAdministration | undefined { + if (mode === "missing") { + return undefined; } - function reconciling( - chat: PairedChatStorage, - stream: { settled(): Operation }, - session: Harness["session"], - ): () => Operation> { - return function* () { - yield* stream.settled(); - const entries = session.reading(undefined); - if (!entries.ok) { - throw entries.error; - } - const findings = yield* coldReservations( - yield* chat.writer.readAll(), - entries.value.model.entries, - chat.location.entries, - ); - if (!findings.ok) { - return findings; - } - for (const finding of findings.value) { - yield* retainResolution(chat.writer, finding); - } - return Ok(undefined); - }; - } - /** Reply 1 submits one counted entry, and its automatic feedback is held streaming. */ - function* counted(): Operation<{ held: Harness; chat: PairedChatStorage; effects(): number }> { - let effects = 0; - const held = yield* useHarness(undefined, { - entryInstallation: { - components: [ - { - name: "CountedEffect", - origin: "test", - props: { type: "object", additionalProperties: false }, - factory: () => - function* () { - effects++; - return "counted once"; - }, - }, - ], - }, - }); - if (held.execution.chat.form !== "paired") { - throw new Error("Expected paired history."); + return { + *inspectOwnership(keyDigest) { + return Ok({ + keyDigest, + recordRevision: mode, + record: { + keyDigest, + state: mode === "active" ? "active" : "idle", + release: mode === "clean" ? "quiesced" : mode === "forced" ? "forced" : "none", + ownerKind: "session", + operationId: "retained-owner", + lastForcedRecovery: null, + }, + }); + }, + *forceRecoverOwnership() { + throw new Error("Recovery must not force ownership."); + }, + }; +} +function reconciling( + chat: PairedChatStorage, + stream: { settled(): Operation }, + session: Harness["session"], +): () => Operation> { + return function* () { + yield* stream.settled(); + const entries = session.reading(undefined); + if (!entries.ok) { + throw entries.error; } - held.fake.script({ - reply: - '\n```xmd\n\n```\n\n' + - "{code}\n\nWorking.", - }); - held.fake.script({ deltas: ["Thinking", " further."], holdAfter: 1 }); - return { held, chat: held.execution.chat, effects: () => effects }; - } - function* liveFeedback( - held: Harness, - chat: PairedChatStorage, - ): Operation<{ dispatch: SidekickDispatch; first: SidekickExchange }> { - const dispatch = yield* useSidekickDispatch({ - owner: held.owner, - admission: createSidekickAdmission(), - execution: held.execution.id, - workspace: held.workspace, - changed() {}, - reconcile: reconciling(chat, held.execution.stream, held.session), - }); - const first = yield* dispatch.send("Run the counted effect."); - if (!first.ok) { - throw first.error; + const findings = yield* coldReservations( + yield* chat.writer.readAll(), + entries.value.model.entries, + chat.location.entries, + ); + if (!findings.ok) { + return findings; } - yield* held.fake.streamingTurns(1); - dispatch.hold(); - return { dispatch, first: first.value }; + for (const finding of findings.value) { + yield* retainResolution(chat.writer, finding); + } + return Ok(undefined); + }; +} +/** Reply 1 submits one counted entry, and its automatic feedback is held streaming. */ +function* counted( + feedback: ScriptedTurn = { deltas: ["Thinking", " further."], holdAfter: 1 }, +): Operation<{ held: Harness; chat: PairedChatStorage; effects(): number }> { + let effects = 0; + const held = yield* useHarness(undefined, { + entryInstallation: { + components: [ + { + name: "CountedEffect", + origin: "test", + props: { type: "object", additionalProperties: false }, + factory: () => + function* () { + effects++; + return "counted once"; + }, + }, + ], + }, + }); + if (held.execution.chat.form !== "paired") { + throw new Error("Expected paired history."); } - function* stoppedFeedback(): Operation<{ held: Harness; effects(): number }> { - const { held, chat, effects } = yield* counted(); - yield* scoped(function* () { - const { first } = yield* liveFeedback(held, chat); - yield* first.cancel("human"); - }); - return { held, effects }; + held.fake.script({ + reply: + '\n```xmd\n\n```\n\n' + + "{code}\n\nWorking.", + }); + held.fake.script(feedback); + return { held, chat: held.execution.chat, effects: () => effects }; +} +function* liveFeedback( + held: Harness, + chat: PairedChatStorage, +): Operation<{ dispatch: SidekickDispatch; first: SidekickExchange }> { + const dispatch = yield* useSidekickDispatch({ + owner: held.owner, + admission: createSidekickAdmission(), + execution: held.execution.id, + workspace: held.workspace, + changed() {}, + reconcile: reconciling(chat, held.execution.stream, held.session), + }); + const first = yield* dispatch.send("Run the counted effect."); + if (!first.ok) { + throw first.error; } - function failing(kind: SidekickRecord["kind"], failure: Error, text?: string): Operation { - return ReplHost.around( - { - *appendRecord([path, line], next) { - const record = parseSidekickRecord(line.trim()); - if ( - record?.kind === kind && - (text === undefined || (record.kind === "message" && record.text === text)) - ) { - throw failure; - } - yield* next(path, line); - }, + yield* held.fake.streamingTurns(1); + dispatch.hold(); + return { dispatch, first: first.value }; +} +function* stoppedFeedback(): Operation<{ held: Harness; effects(): number }> { + const { held, chat, effects } = yield* counted(); + yield* scoped(function* () { + const { first } = yield* liveFeedback(held, chat); + yield* first.cancel("human"); + }); + return { held, effects }; +} +function failing(kind: SidekickRecord["kind"], failure: Error, text?: string): Operation { + return ReplHost.around( + { + *appendRecord([path, line], next) { + const record = parseSidekickRecord(line.trim()); + if ( + record?.kind === kind && + (text === undefined || (record.kind === "message" && record.text === text)) + ) { + throw failure; + } + yield* next(path, line); }, - { at: "max" }, - ); + }, + { at: "max" }, + ); +} +function* coldOwner( + held: Harness, + administration: AgentSessionOwnershipAdministration | undefined, +): Operation<{ owner: SidekickOwner; chat: PairedChatStorage; dispatch: SidekickDispatch }> { + const reopened = yield* held.repository.open(held.execution.id); + if (reopened.chat.form !== "paired") { + throw new Error("Expected paired history."); } - function* coldOwner( - held: Harness, - administration: AgentSessionOwnershipAdministration | undefined, - ): Operation<{ owner: SidekickOwner; chat: PairedChatStorage; dispatch: SidekickDispatch }> { - const reopened = yield* held.repository.open(held.execution.id); - if (reopened.chat.form !== "paired") { - throw new Error("Expected paired history."); - } - const chat = reopened.chat; - const owner = yield* useSidekickOwner({ - assembly: { ...held.profile.sidekickAssembly, ownershipAdministration: administration }, - chat, - *establish() { - return Ok(chat); - }, - root: { - ...rootReads(held.session, held.profile), - *fill() { - return Err(new Error("This witness admits no draft fill.")); - }, - }, - *submit() { - return Err(new Error("This witness admits no entry.")); + const chat = reopened.chat; + const owner = yield* useSidekickOwner({ + assembly: { ...held.profile.sidekickAssembly, ownershipAdministration: administration }, + chat, + *establish() { + return Ok(chat); + }, + root: { + ...rootReads(held.session, held.profile), + *fill() { + return Err(new Error("This witness admits no draft fill.")); }, - revision: () => 0, - *acknowledge() {}, - draft: () => "", - }); - const dispatch = yield* useSidekickDispatch({ - owner, - admission: createSidekickAdmission(), - execution: held.execution.id, - workspace: held.workspace, - changed() {}, - reconcile: reconciling(chat, reopened.stream, held.session), - }); - return { owner, chat, dispatch }; - } - function* bytes(held: Harness): Operation { - if (held.execution.chat.form !== "paired") { - throw new Error("Expected paired history."); - } - return yield* until(readFile(held.execution.chat.location.sidekick, "utf8")); + }, + *submit() { + return Err(new Error("This witness admits no entry.")); + }, + revision: () => 0, + *acknowledge() {}, + draft: () => "", + }); + const dispatch = yield* useSidekickDispatch({ + owner, + admission: createSidekickAdmission(), + execution: held.execution.id, + workspace: held.workspace, + changed() {}, + reconcile: reconciling(chat, reopened.stream, held.session), + }); + return { owner, chat, dispatch }; +} +function* bytes(held: Harness): Operation { + if (held.execution.chat.form !== "paired") { + throw new Error("Expected paired history."); } + return yield* until(readFile(held.execution.chat.location.sidekick, "utf8")); +} +describe("1151 — cold ordinary Send after a retained human Stop", () => { for (const mode of ["clean", "active", "forced", "missing"] as const) { it(`A2: a cold eligible Send ${mode === "clean" ? "binds clean retained ownership" : `refuses ${mode} ownership`}`, function* () { const { held, effects } = yield* stoppedFeedback(); @@ -4269,3 +4272,109 @@ describe("1151 — cold ordinary Send after a retained human Stop", () => { expect(messages[1]?.id).toBe(decisions[0]?.message); }); }); + +function required(records: readonly SidekickRecord[], live: number | undefined): boolean { + const projected = projectHistory(records); + if (!projected.ok) { + throw projected.error; + } + return recoveryRequired(records, projected.value, live); +} + +describe("1183 — the exact live feedback request is the only exception", () => { + function* waiting(held: Harness, chat: PairedChatStorage): Operation { + const dispatch = yield* useSidekickDispatch({ + owner: held.owner, + admission: createSidekickAdmission(), + execution: held.execution.id, + workspace: held.workspace, + changed() {}, + reconcile: reconciling(chat, held.execution.stream, held.session), + }); + const first = yield* dispatch.send("Run the counted effect."); + if (!first.ok) { + throw first.error; + } + return dispatch; + } + + it("A4: an identical earlier request, a mismatched acknowledgement and no owner keep the guard", function* () { + const { held, chat } = yield* counted(); + yield* waiting(held, chat); + yield* held.fake.streamingTurns(1); + const prefix = yield* chat.writer.readAll(); + const live = prefix.findIndex((one) => one.kind === "feedback" && one.turn === null); + const pending = prefix[live]; + if (pending?.kind !== "feedback") { + throw new Error("Expected the live feedback record."); + } + const reading = held.owner.reading; + if (!reading.ok) { + throw reading.error; + } + expect(reading.value.unestablished).toEqual([ + { conversation: pending.conversation, turn: null }, + ]); + expect(reading.value.recoveryPending).toBe(true); + expect(held.owner.recoveryPending).toBe(false); + expect(required(prefix, live)).toBe(false); + expect(required(prefix, undefined)).toBe(true); + + const repeated = [...prefix, pending]; + expect(required(repeated, repeated.length - 1)).toBe(true); + expect(required(repeated, live)).toBe(true); + const mismatched: SidekickRecord[] = [ + ...prefix, + { + kind: "feedback", + conversation: pending.conversation, + response: pending.response, + turn: 1, + text: "Something turn 1 never carried.", + }, + ]; + expect(required(mismatched, live)).toBe(true); + expect(held.fake.prompts).toHaveLength(2); + }); + + it("A4: a fresh owner of the held and of the failed prefix still requires recovery and sends nothing", function* () { + const { held, chat, effects } = yield* counted({ + reply: "Unfinished", + held: true, + failure: "The provider failed the feedback.", + }); + const dispatch = yield* waiting(held, chat); + yield* held.fake.holdingTurns(1); + expect(held.owner.recoveryPending).toBe(false); + const before = yield* bytes(held); + const during = yield* coldOwner(held, administered("clean")); + expect(during.owner.recoveryPending).toBe(true); + const refused = yield* during.dispatch.send("Explain the counted result."); + expect(refused.ok ? "" : refused.error.message).toContain("cannot be established"); + const queued = yield* dispatch.send("When that finishes, explain the changes."); + expect(queued.ok && queued.value.snapshot.state).toBe("queued"); + expect(held.fake.prompts).toHaveLength(2); + expect(yield* bytes(held)).toBe(before); + + held.fake.releaseTurn(); + yield* awaitedFact("the held failure", function* () { + return dispatch.queue.held?.kind === "failure"; + }); + expect(held.owner.recoveryPending).toBe(true); + const after = yield* coldOwner(held, administered("clean")); + expect(after.owner.recoveryPending).toBe(true); + const again = yield* after.dispatch.send("Explain the counted result."); + expect(again.ok ? "" : again.error.message).toContain("cannot be established"); + const read = after.owner.reading; + if (!read.ok) { + throw read.error; + } + expect(read.value.unestablished).toEqual([{ conversation: read.value.active, turn: null }]); + expect(dispatch.queue.pending.map((one) => one.text)).toEqual([ + "When that finishes, explain the changes.", + ]); + expect(held.fake.prompts).toHaveLength(2); + expect(held.session.model.entries).toHaveLength(1); + expect(effects()).toBe(1); + }); +}); diff --git a/specs/repl-spec.md b/specs/repl-spec.md index 978b4a6a4..5d6e674fb 100644 --- a/specs/repl-spec.md +++ b/specs/repl-spec.md @@ -2953,8 +2953,11 @@ history position is selected, or while Sidekick is active. Arrival wakes the screen without moving selection, focus, Follow or either editable draft. Ordinary `xmd run` retains its existing foreground forwarding. -When feedback has no exact completed canonical input, `[review recovery]` shows -the original request, saved failure, established entries and unfinished controls. +When feedback has no exact completed canonical input and is not the exact +in-flight request owned by the live exchange, `[review recovery]` shows the +original request, saved failure, established entries and unfinished controls. +Ordinary live feedback alone does not expose the recovery action. Losing the live +owner leaves the retained uncertainty subject to the existing recovery rules. The action sits with the conversation's fixed controls, directly above the tray and outside the scrolling chat, so neither a long conversation nor its scroll position hides it at any supported size. It belongs to the Message keyboard @@ -3293,6 +3296,14 @@ oversize message refuses before clearing text, with no eviction or truncation. Successful composer Send clears only its accepted Message revision. Newer typing survives; Retry consumes no composer text and ACP changes neither field. +An otherwise eligible message also enters the queue while this live owner is +asking the provider for automatic feedback. The exception names only the exact +pending feedback request owned by that exchange; other unresolved feedback and +pending recovery decisions keep their existing guard. Queue admission starts no +additional prompt, evaluation or writer. Before a waiting message starts, the +owner checks eligibility again. A failed or interrupted feedback exchange retains +the applicable queue hold and recovery behavior. + Waiting work has no ordinary saved message record. When it starts, ordinary saved acknowledgement releases only matching Draft takeover before response controls run. The entire exchange and its cleanup finish before the next starts. Human From bb340a6114aac324d1d834c84873b244ce4b5ade Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 10 Nov 2026 05:30:31 -0500 Subject: [PATCH 2/3] fix: retain recovery guard for interrupted-work messages --- packages/cli/src/repl/sidekick/owner.ts | 6 ++++-- specs/repl-spec.md | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/repl/sidekick/owner.ts b/packages/cli/src/repl/sidekick/owner.ts index 858348b6a..b2c30fc9f 100644 --- a/packages/cli/src/repl/sidekick/owner.ts +++ b/packages/cli/src/repl/sidekick/owner.ts @@ -2519,8 +2519,10 @@ export function useSidekickOwner(input: SidekickOwnerInput): Operation Date: Tue, 10 Nov 2026 12:17:10 -0500 Subject: [PATCH 3/3] Specify exact Sidekick queue admission during live feedback Address the maintainer review on #1225. Define one live feedback exception for feedback-only requests and queued messages carrying completed round results. Use it for admission and recovery presentation. Describe how a start-time refusal holds the queued head and how attachment cancellation differs from human Stop. Evidence: 47 tests and 174 steps passed in the dispatch, recovery and Sidekick journey suites. The four child-reference steps blocked by local socket permissions passed with local listeners permitted. --- specs/repl-spec.md | 55 +++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/specs/repl-spec.md b/specs/repl-spec.md index c97de30aa..ab35903d4 100644 --- a/specs/repl-spec.md +++ b/specs/repl-spec.md @@ -2953,11 +2953,11 @@ history position is selected, or while Sidekick is active. Arrival wakes the screen without moving selection, focus, Follow or either editable draft. Ordinary `xmd run` retains its existing foreground forwarding. -When feedback has no exact completed canonical input and is not the exact -in-flight automatic-feedback request owned by the live exchange, `[review recovery]` -shows the original request, saved failure, established entries and unfinished controls. -Ordinary live feedback alone does not expose the recovery action. Losing the live -owner leaves the retained uncertainty subject to the existing recovery rules. +The recovery guard presents `[review recovery]` for unresolved feedback or a +pending recovery decision, using the same +[live feedback exception](#live-feedback-exception) as queue admission. The +review shows the original request, saved failure, established entries and +unfinished controls. The action sits with the conversation's fixed controls, directly above the tray and outside the scrolling chat, so neither a long conversation nor its scroll position hides it at any supported size. It belongs to the Message keyboard @@ -3296,23 +3296,36 @@ oversize message refuses before clearing text, with no eviction or truncation. Successful composer Send clears only its accepted Message revision. Newer typing survives; Retry consumes no composer text and ACP changes neither field. -An otherwise eligible message also enters the queue while this live owner is -asking the provider for automatic feedback. The exception names only the exact -pending feedback request owned by that exchange; other unresolved feedback and -pending recovery decisions keep their existing guard. Queue admission starts no -additional prompt, evaluation or writer. Before a waiting message starts, the -owner checks eligibility again. A failed or interrupted feedback exchange retains -the applicable queue hold and recovery behavior. + + +The **live feedback exception** sets aside one unresolved feedback record, +identified by its position in the conversation record. The live owner binds +that position to the current exchange's exact provider request. +This can be a feedback-only request or a queued message's request carrying the +completed round's results. An otherwise eligible message can enter the FIFO +while either request is live. An earlier record with the same text, a mismatched +acknowledgement and a pending recovery decision remain subject to the guard. +The exception ends with the request's ownership and is absent on cold reopen. + +Queue admission starts no additional prompt, evaluation or writer. Before a +waiting head starts, the owner checks eligibility again. A refusal leaves the +head queued without sending it and shows the reason in the queue hold. +**Remove message** withdraws that exact item while leaving the feedback guard +and hold in force. **Resume queue** rechecks the guard, and +[explicit recovery](#who-owns-what) uses a distinct new instruction to address +unresolved feedback. Waiting work has no ordinary saved message record. When it starts, ordinary saved acknowledgement releases only matching Draft takeover before response controls run. The entire exchange and its cleanup finish before the next starts. Human Sidekick Stop holds remaining work until explicit guarded Resume, which passes the -unknown-feedback guard only for a head bound to a retained human Stop; attachment -cancellation affects only its exact exchange and adds no hold. Ordinary failure -holds without automatic retry; fatal failures and unknown-feedback guards retain -their authority. Pending work must be removed before agent replacement. Waiting -work ends with this live owner and has no cold recovery or automatic replay. +unknown-feedback guard only for a head bound to a retained human Stop. +Attachment cancellation ends only its exact exchange and supplies no Stop +binding. If acceptance remains unknown, the independent eligibility recheck +holds the waiting head as described above. Ordinary failure holds without +automatic retry. Fatal failures retain their authority. Pending work must be +removed before ordinary agent replacement. Waiting work ends with this live +owner and has no cold recovery or automatic replay. A reply may ask a question, and it is answered in the Sidekick's own pane. When one is waiting, the pane says so and offers one control that opens its form; the @@ -3882,9 +3895,11 @@ completed, failed or interrupted, each its own — and its actual output, as two separate facts. The Submit answer is still the receipt described above; the output comes through the continuation, never through the receipt's wording. With Follow on, the next eligible queued message goes first and another autonomous -round only after it, under the queue's own rules (#964); no origin is preferred -and no held queue is restarted by it. Turning Follow off ends automatic -continuation once the current round is over and does not interrupt that round. +round only after it, under the queue's own rules (#964). That message carries +the settled round's results and its request uses the +[live feedback exception](#live-feedback-exception). FIFO applies across +origins and the handoff leaves a held queue in place. Turning Follow off lets +the current round finish and ends automatic continuation at that point. The receipt and control results remain retained without a delivery marker for feedback that was not sent. Ordinary read-only designated feedback still receives its answer with Follow off; a refused Submit creates no round.