From 4383134762ea0b8321c41244d3e8a3438bcac27b Mon Sep 17 00:00:00 2001 From: Csaba Fabian Date: Thu, 8 Oct 2026 20:44:23 +0000 Subject: [PATCH 1/4] feat(acp): store a prompt under the client's msg-prefixed message id Co-Authored-By: Claude Opus 5.5 --- documentor/FORK.md | 1 + packages/opencode/src/acp/service.ts | 10 ++++++ .../opencode/test/acp/service-session.test.ts | 34 +++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/documentor/FORK.md b/documentor/FORK.md index b491b1851e2d..efcb2c9526a7 100644 --- a/documentor/FORK.md +++ b/documentor/FORK.md @@ -22,6 +22,7 @@ files it changes, because those are what a rebase meets. | `packages/core/package.json` | Depends on `@effect/sql-pg`, `pg`, `@opencode-ai/effect-drizzle-pg`; tests run with `--timeout 30000` | Each test database is a real schema, about 110 ms to build | | `packages/opencode/src/cli/cmd/db.ts` | `db `, `db migrate [--all]`, `db provision`, `db grant-read`, all outside the application runtime | The runtime's database service refuses the schemas these commands exist to create and migrate | | `packages/opencode/src/session/processor.ts` | One lock per processor around each read-then-write of a tool part; `tool-input-delta` and `tool-input-end` do not read a part the processor already tracks | The processor and the running tool both update a tool part. SQLite made each update atomic; a network database lets them interleave, and the tool's metadata was lost. Each read is a round trip, and a large edit streams thousands of deltas | +| `packages/opencode/src/acp/service.ts` | `prompt` passes a `messageId` that starts with `msg` to v1 as `messageID`, for a normal prompt and a slash command; an empty prompt passes none | DocuMentor names the user message a prompt creates, so its optimistic, live and durable copies share one id. ACP clients that send UUIDs keep opencode-minted ids | | `packages/core/src/integration.ts` | An OAuth attempt's credential is stored before the attempt reads as complete, inside one update of the attempts map | The attempt was marked complete first and the credential written after. On SQLite nothing could look in between; on a network database a caller that saw "complete" could list no credential | | `patches/effect@4.0.0-beta.83.patch` | Adds Effect's own fix for re-entrant interruption, first released in `4.0.0-beta.100` | A fiber that woke another fiber which interrupted it synchronously had its continuation stack corrupted once its cleanup awaited anything. With Postgres that leaked a pooled connection. Drop these hunks when upstream moves to `beta.100` or later | | `turbo.json` | `OPENCODE_TEST_DATABASE_URL` passes through to every task; `@opencode-ai/effect-drizzle-pg#test` is a task | turbo strips variables it is not told about | diff --git a/packages/opencode/src/acp/service.ts b/packages/opencode/src/acp/service.ts index c542e2b75320..e0eb17e95e6d 100644 --- a/packages/opencode/src/acp/service.ts +++ b/packages/opencode/src/acp/service.ts @@ -517,6 +517,7 @@ export function make(input: { const modeId = current.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined) const parts = promptContentToParts(params.prompt) const command = detectSlashCommand(parts) + const messageID = parts.length > 0 ? opencodeMessageID(params.messageId) : undefined if (!command) { const response = yield* request( @@ -525,6 +526,7 @@ export function make(input: { input.sdk.session.prompt( { sessionID: current.id, + ...(messageID ? { messageID } : {}), model: { providerID: selected.providerID, modelID: selected.modelID, @@ -551,6 +553,7 @@ export function make(input: { input.sdk.session.command( { sessionID: current.id, + ...(messageID ? { messageID } : {}), command: known.name, arguments: command.args, model: `${selected.providerID}/${selected.modelID}`, @@ -836,6 +839,13 @@ function detectSlashCommand(parts: ReturnType) { return { name, args: rest.join(" ").trim() } } +// A client may name the user message its prompt creates, so that its own copy and opencode's share one id. +// Only an id that is already an opencode message id (prefix "msg") is used; ACP clients that send UUIDs get an +// opencode-minted id. An empty prompt creates no message (SessionPrompt.prompt), so it names none. +function opencodeMessageID(messageId: string | null | undefined) { + return typeof messageId === "string" && messageId.startsWith("msg") ? messageId : undefined +} + const promptResponse = Effect.fn("ACP.promptResponse")(function* ( info: AssistantInfo, messageId: string | null | undefined, diff --git a/packages/opencode/test/acp/service-session.test.ts b/packages/opencode/test/acp/service-session.test.ts index 82305381bd4f..9dca24acd341 100644 --- a/packages/opencode/test/acp/service-session.test.ts +++ b/packages/opencode/test/acp/service-session.test.ts @@ -1350,6 +1350,40 @@ describe("ACP service sessions", () => { expect(usageUpdates).toEqual([session.sessionId]) }) + it("documentor: uses a client message id with the msg prefix as the durable user message id", async () => { + const { service, prompts } = makeService() + const session = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] })) + + const result = await Effect.runPromise( + service.prompt({ + sessionId: session.sessionId, + messageId: "msg_user_ses_a_1700000000000_x1", + prompt: [{ type: "text", text: "hello" }], + }), + ) + + expect(prompts).toHaveLength(1) + expect(prompts[0]).toMatchObject({ + sessionID: session.sessionId, + messageID: "msg_user_ses_a_1700000000000_x1", + parts: [{ type: "text", text: "hello" }], + }) + expect(result.userMessageId).toBe("msg_user_ses_a_1700000000000_x1") + }) + + it("documentor: names no message when an empty prompt carries a message id", async () => { + const { service, prompts } = makeService() + const session = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] })) + + await Effect.runPromise( + service.prompt({ sessionId: session.sessionId, messageId: "msg_user_ses_a_1700000000000_x2", prompt: [] }), + ) + + expect(prompts).toHaveLength(1) + expect(prompts[0]).toMatchObject({ parts: [] }) + expect(prompts[0]).not.toHaveProperty("messageID") + }) + it("waits for queued session updates before returning end_turn", async () => { const called = deferred() const response = deferred<{ data: { info: ReturnType } }>() From 27b3dc1aa93966d3e9392744746ee199ecf818cc Mon Sep 17 00:00:00 2001 From: Csaba Fabian Date: Thu, 8 Oct 2026 20:47:41 +0000 Subject: [PATCH 2/4] feat(session): resume the loop on a prompt with no parts Co-Authored-By: Claude Opus 5.5 --- documentor/FORK.md | 33 +++---- packages/opencode/src/session/prompt.ts | 11 +++ packages/opencode/test/session/prompt.test.ts | 88 +++++++++++++++++++ 3 files changed, 116 insertions(+), 16 deletions(-) diff --git a/documentor/FORK.md b/documentor/FORK.md index efcb2c9526a7..1015b4f3e2ca 100644 --- a/documentor/FORK.md +++ b/documentor/FORK.md @@ -11,22 +11,23 @@ files it changes, because those are what a rebase meets. ## Upstream files changed -| File | What differs | Why | -| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| The 11 table files: `packages/core/src/{account,credential,event,permission,project,session,share}/sql.ts`, `control-plane/workspace.sql.ts`, `data-migration.sql.ts`, `database/schema.sql.ts`, `database/path.ts` | Postgres table builder; `count`, `jsonb` and `text` columns from `effect-drizzle-pg`; time columns are `double precision` | Written by `documentor/scripts/convert-schema.py`, never by hand | -| `packages/core/src/database/schema.gen.ts`, `packages/core/schema.json`, `packages/core/drizzle.config.ts` | The Postgres baseline, as one statement batch | Generated; see "Regenerating the baseline" | -| `packages/core/src/database/database.ts` | Connects with `@effect/sql-pg`; verifies collation and migrations and changes nothing; a throwaway schema per layer when `OPENCODE_DATABASE_EPHEMERAL=1` | The service itself | -| `packages/core/src/database/migration.ts` | `migrate`, `migrateAll`, `verify`, `pending`, `BASELINE_IDS`, `ported` | Schema work is a command, not a start-up side effect | -| `packages/core/src/project/directories.ts` | `Transaction` is `Database.Transaction` | It was typed against the SQLite client | -| `packages/core/src/flag/flag.ts` | No `OPENCODE_DB` | Nothing reads it | -| `packages/core/package.json` | Depends on `@effect/sql-pg`, `pg`, `@opencode-ai/effect-drizzle-pg`; tests run with `--timeout 30000` | Each test database is a real schema, about 110 ms to build | -| `packages/opencode/src/cli/cmd/db.ts` | `db `, `db migrate [--all]`, `db provision`, `db grant-read`, all outside the application runtime | The runtime's database service refuses the schemas these commands exist to create and migrate | -| `packages/opencode/src/session/processor.ts` | One lock per processor around each read-then-write of a tool part; `tool-input-delta` and `tool-input-end` do not read a part the processor already tracks | The processor and the running tool both update a tool part. SQLite made each update atomic; a network database lets them interleave, and the tool's metadata was lost. Each read is a round trip, and a large edit streams thousands of deltas | -| `packages/opencode/src/acp/service.ts` | `prompt` passes a `messageId` that starts with `msg` to v1 as `messageID`, for a normal prompt and a slash command; an empty prompt passes none | DocuMentor names the user message a prompt creates, so its optimistic, live and durable copies share one id. ACP clients that send UUIDs keep opencode-minted ids | -| `packages/core/src/integration.ts` | An OAuth attempt's credential is stored before the attempt reads as complete, inside one update of the attempts map | The attempt was marked complete first and the credential written after. On SQLite nothing could look in between; on a network database a caller that saw "complete" could list no credential | -| `patches/effect@4.0.0-beta.83.patch` | Adds Effect's own fix for re-entrant interruption, first released in `4.0.0-beta.100` | A fiber that woke another fiber which interrupted it synchronously had its continuation stack corrupted once its cleanup awaited anything. With Postgres that leaked a pooled connection. Drop these hunks when upstream moves to `beta.100` or later | -| `turbo.json` | `OPENCODE_TEST_DATABASE_URL` passes through to every task; `@opencode-ai/effect-drizzle-pg#test` is a task | turbo strips variables it is not told about | -| Test helpers: `packages/core/test/preload.ts`, `packages/opencode/test/preload.ts`, `packages/opencode/test/fixture/db.ts`, `packages/opencode/test/server/httpapi-exercise/environment.ts`, `packages/sdk-next/test/embedded.test.ts` | Ask for a throwaway schema; no database file to delete | | +| File | What differs | Why | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| The 11 table files: `packages/core/src/{account,credential,event,permission,project,session,share}/sql.ts`, `control-plane/workspace.sql.ts`, `data-migration.sql.ts`, `database/schema.sql.ts`, `database/path.ts` | Postgres table builder; `count`, `jsonb` and `text` columns from `effect-drizzle-pg`; time columns are `double precision` | Written by `documentor/scripts/convert-schema.py`, never by hand | +| `packages/core/src/database/schema.gen.ts`, `packages/core/schema.json`, `packages/core/drizzle.config.ts` | The Postgres baseline, as one statement batch | Generated; see "Regenerating the baseline" | +| `packages/core/src/database/database.ts` | Connects with `@effect/sql-pg`; verifies collation and migrations and changes nothing; a throwaway schema per layer when `OPENCODE_DATABASE_EPHEMERAL=1` | The service itself | +| `packages/core/src/database/migration.ts` | `migrate`, `migrateAll`, `verify`, `pending`, `BASELINE_IDS`, `ported` | Schema work is a command, not a start-up side effect | +| `packages/core/src/project/directories.ts` | `Transaction` is `Database.Transaction` | It was typed against the SQLite client | +| `packages/core/src/flag/flag.ts` | No `OPENCODE_DB` | Nothing reads it | +| `packages/core/package.json` | Depends on `@effect/sql-pg`, `pg`, `@opencode-ai/effect-drizzle-pg`; tests run with `--timeout 30000` | Each test database is a real schema, about 110 ms to build | +| `packages/opencode/src/cli/cmd/db.ts` | `db `, `db migrate [--all]`, `db provision`, `db grant-read`, all outside the application runtime | The runtime's database service refuses the schemas these commands exist to create and migrate | +| `packages/opencode/src/session/processor.ts` | One lock per processor around each read-then-write of a tool part; `tool-input-delta` and `tool-input-end` do not read a part the processor already tracks | The processor and the running tool both update a tool part. SQLite made each update atomic; a network database lets them interleave, and the tool's metadata was lost. Each read is a round trip, and a large edit streams thousands of deltas | +| `packages/opencode/src/acp/service.ts` | `prompt` passes a `messageId` that starts with `msg` to v1 as `messageID`, for a normal prompt and a slash command; an empty prompt passes none | DocuMentor names the user message a prompt creates, so its optimistic, live and durable copies share one id. ACP clients that send UUIDs keep opencode-minted ids | +| `packages/opencode/src/session/prompt.ts` | `prompt` with no parts and no `noReply`, on a session that already has a user message, stores nothing and runs `loop`; on a session with no user message it stores a zero-part row as upstream does | DocuMentor's abort-then-answer: after a cancel, an empty ACP prompt answers the user messages that arrived during the cancelled step, which opencode already holds. A v1 client sending `parts: []` gets the same resume; upstream clients send none | +| `packages/core/src/integration.ts` | An OAuth attempt's credential is stored before the attempt reads as complete, inside one update of the attempts map | The attempt was marked complete first and the credential written after. On SQLite nothing could look in between; on a network database a caller that saw "complete" could list no credential | +| `patches/effect@4.0.0-beta.83.patch` | Adds Effect's own fix for re-entrant interruption, first released in `4.0.0-beta.100` | A fiber that woke another fiber which interrupted it synchronously had its continuation stack corrupted once its cleanup awaited anything. With Postgres that leaked a pooled connection. Drop these hunks when upstream moves to `beta.100` or later | +| `turbo.json` | `OPENCODE_TEST_DATABASE_URL` passes through to every task; `@opencode-ai/effect-drizzle-pg#test` is a task | turbo strips variables it is not told about | +| Test helpers: `packages/core/test/preload.ts`, `packages/opencode/test/preload.ts`, `packages/opencode/test/fixture/db.ts`, `packages/opencode/test/server/httpapi-exercise/environment.ts`, `packages/sdk-next/test/embedded.test.ts` | Ask for a throwaway schema; no database file to delete | | Upstream tests that are changed or deleted are listed, with reasons, in [`upstream-test-exceptions.md`](upstream-test-exceptions.md). diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 0f85d44f209b..0d3898ac8b38 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -1054,6 +1054,12 @@ const layer = Layer.effect( )(function* (input: PromptInput) { const session = yield* sessions.get(input.sessionID).pipe(Effect.orDie) yield* revert.cleanup(session) + if (input.parts.length === 0 && input.noReply !== true && (yield* hasUserMessage(input.sessionID))) { + // A prompt with no parts resumes the session instead of storing an empty user message. The loop runs over + // the messages already stored, so user messages that arrived during a cancelled step are answered, and a + // session with nothing left to answer returns its last assistant without calling the model. + return yield* loop({ sessionID: input.sessionID }) + } const message = yield* createUserMessage(input) yield* sessions.touch(input.sessionID) @@ -1078,6 +1084,11 @@ const layer = Layer.effect( throw new Error("Impossible") }) + const hasUserMessage = Effect.fnUntraced(function* (sessionID: SessionID) { + const match = yield* sessions.findMessage(sessionID, (m) => m.info.role === "user").pipe(Effect.orDie) + return Option.isSome(match) + }) + const runLoop: (sessionID: SessionID) => Effect.Effect = Effect.fn("SessionPrompt.run")( function* (sessionID: SessionID) { const ctx = yield* InstanceState.context diff --git a/packages/opencode/test/session/prompt.test.ts b/packages/opencode/test/session/prompt.test.ts index da6e0f8d036f..14fd587c1154 100644 --- a/packages/opencode/test/session/prompt.test.ts +++ b/packages/opencode/test/session/prompt.test.ts @@ -1496,6 +1496,94 @@ it.instance("prompt submitted during an active run is included in the next LLM i }), ) +it.instance("documentor: an empty prompt saves no message and answers the newest user message", () => + Effect.gen(function* () { + const { llm } = yield* useServerConfig(providerCfg) + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const chat = yield* sessions.create({ title: "Pinned" }) + // An interrupted step: the assistant has no finish, as a cancel leaves it. + const seeded = yield* seed(chat.id) + const queued = yield* user(chat.id, "queued during the cancelled step") + yield* llm.text("answer to the queued message") + + const result = yield* prompt.prompt({ sessionID: chat.id, agent: "build", model: ref, parts: [] }) + + const msgs = yield* sessions.messages({ sessionID: chat.id }) + expect(msgs.filter((msg) => msg.info.role === "user").map((msg) => msg.info.id)).toEqual([ + seeded.user.id, + queued.id, + ]) + if (result.info.role !== "assistant") throw new Error("expected an assistant message") + expect(result.info.parentID).toBe(queued.id) + expect(yield* llm.calls).toBe(1) + }), +) + +it.instance("documentor: an empty prompt with nothing to answer calls no model", () => + Effect.gen(function* () { + const { llm } = yield* useServerConfig(providerCfg) + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const chat = yield* sessions.create({ title: "Pinned" }) + const seeded = yield* seed(chat.id, { finish: "stop" }) + // Queued so a regression returns instead of waiting on the server. + yield* llm.text("should not be requested") + + const result = yield* prompt.prompt({ sessionID: chat.id, agent: "build", model: ref, parts: [] }) + + expect(result.info.id).toBe(seeded.assistant.id) + expect(yield* llm.hits).toHaveLength(0) + expect(yield* sessions.messages({ sessionID: chat.id })).toHaveLength(2) + }), +) + +it.instance("documentor: an empty prompt on a fresh session keeps upstream behaviour", () => + Effect.gen(function* () { + const { llm } = yield* useServerConfig(providerCfg) + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const chat = yield* sessions.create({ title: "Pinned" }) + yield* llm.text("first reply") + + yield* prompt.prompt({ sessionID: chat.id, agent: "build", model: ref, parts: [] }) + + const users = (yield* sessions.messages({ sessionID: chat.id })).filter((msg) => msg.info.role === "user") + expect(users).toHaveLength(1) + expect(users[0]?.parts).toHaveLength(0) + expect(yield* llm.calls).toBe(1) + }), +) + +it.instance("documentor: an empty prompt while a loop runs joins that loop", () => + Effect.gen(function* () { + const { llm } = yield* useServerConfig(providerCfg) + const gate = yield* Deferred.make() + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const chat = yield* sessions.create({ title: "Pinned" }) + + yield* llm.hold("first", deferredAsPromise(gate)) + + const a = yield* prompt + .prompt({ sessionID: chat.id, agent: "build", model: ref, parts: [{ type: "text", text: "first" }] }) + .pipe(Effect.forkChild) + yield* llm.wait(1) + yield* waitForBusy(chat.id) + + const b = yield* prompt.prompt({ sessionID: chat.id, agent: "build", model: ref, parts: [] }).pipe(Effect.forkChild) + yield* Deferred.succeed(gate, void 0) + + const [ea, eb] = yield* Effect.all([Fiber.await(a), Fiber.await(b)]) + expect(Exit.isSuccess(ea)).toBe(true) + expect(Exit.isSuccess(eb)).toBe(true) + expect(yield* llm.calls).toBe(1) + const msgs = yield* sessions.messages({ sessionID: chat.id }) + expect(msgs.filter((msg) => msg.info.role === "user")).toHaveLength(1) + expect(msgs.filter((msg) => msg.info.role === "assistant")).toHaveLength(1) + }), +) + it.instance("assertNotBusy fails with BusyError when loop running", () => Effect.gen(function* () { const { llm } = yield* useServerConfig(providerCfg) From 390396db077adbb8f76e5572a4a95048f7a922b4 Mon Sep 17 00:00:00 2001 From: Csaba Fabian Date: Thu, 8 Oct 2026 20:53:19 +0000 Subject: [PATCH 3/4] feat(acp): name the assistant message and its parent on every content and tool update Co-Authored-By: Claude Opus 5.5 --- documentor/FORK.md | 40 +++--- documentor/upstream-test-exceptions.md | 3 + packages/opencode/src/acp/event.ts | 71 +++++++++- packages/opencode/test/acp/event.test.ts | 132 ++++++++++++++++++ .../opencode/test/acp/service-session.test.ts | 26 ++++ 5 files changed, 250 insertions(+), 22 deletions(-) diff --git a/documentor/FORK.md b/documentor/FORK.md index 1015b4f3e2ca..735620f74658 100644 --- a/documentor/FORK.md +++ b/documentor/FORK.md @@ -6,28 +6,31 @@ Postgres. It has no SQLite path. The design is in Everything the fork adds lives in `documentor/`, `packages/effect-drizzle-pg/`, `packages/core/test/pg/`, `packages/opencode/test/pg/` and the two -`documentor-*` workflows. The rest of this document is about the upstream +`documentor-*` workflows, except the `documentor: …` test cases listed in the +table below, which sit beside the upstream tests of the code they cover. The rest of this document is about the upstream files it changes, because those are what a rebase meets. ## Upstream files changed -| File | What differs | Why | -| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| The 11 table files: `packages/core/src/{account,credential,event,permission,project,session,share}/sql.ts`, `control-plane/workspace.sql.ts`, `data-migration.sql.ts`, `database/schema.sql.ts`, `database/path.ts` | Postgres table builder; `count`, `jsonb` and `text` columns from `effect-drizzle-pg`; time columns are `double precision` | Written by `documentor/scripts/convert-schema.py`, never by hand | -| `packages/core/src/database/schema.gen.ts`, `packages/core/schema.json`, `packages/core/drizzle.config.ts` | The Postgres baseline, as one statement batch | Generated; see "Regenerating the baseline" | -| `packages/core/src/database/database.ts` | Connects with `@effect/sql-pg`; verifies collation and migrations and changes nothing; a throwaway schema per layer when `OPENCODE_DATABASE_EPHEMERAL=1` | The service itself | -| `packages/core/src/database/migration.ts` | `migrate`, `migrateAll`, `verify`, `pending`, `BASELINE_IDS`, `ported` | Schema work is a command, not a start-up side effect | -| `packages/core/src/project/directories.ts` | `Transaction` is `Database.Transaction` | It was typed against the SQLite client | -| `packages/core/src/flag/flag.ts` | No `OPENCODE_DB` | Nothing reads it | -| `packages/core/package.json` | Depends on `@effect/sql-pg`, `pg`, `@opencode-ai/effect-drizzle-pg`; tests run with `--timeout 30000` | Each test database is a real schema, about 110 ms to build | -| `packages/opencode/src/cli/cmd/db.ts` | `db `, `db migrate [--all]`, `db provision`, `db grant-read`, all outside the application runtime | The runtime's database service refuses the schemas these commands exist to create and migrate | -| `packages/opencode/src/session/processor.ts` | One lock per processor around each read-then-write of a tool part; `tool-input-delta` and `tool-input-end` do not read a part the processor already tracks | The processor and the running tool both update a tool part. SQLite made each update atomic; a network database lets them interleave, and the tool's metadata was lost. Each read is a round trip, and a large edit streams thousands of deltas | -| `packages/opencode/src/acp/service.ts` | `prompt` passes a `messageId` that starts with `msg` to v1 as `messageID`, for a normal prompt and a slash command; an empty prompt passes none | DocuMentor names the user message a prompt creates, so its optimistic, live and durable copies share one id. ACP clients that send UUIDs keep opencode-minted ids | -| `packages/opencode/src/session/prompt.ts` | `prompt` with no parts and no `noReply`, on a session that already has a user message, stores nothing and runs `loop`; on a session with no user message it stores a zero-part row as upstream does | DocuMentor's abort-then-answer: after a cancel, an empty ACP prompt answers the user messages that arrived during the cancelled step, which opencode already holds. A v1 client sending `parts: []` gets the same resume; upstream clients send none | -| `packages/core/src/integration.ts` | An OAuth attempt's credential is stored before the attempt reads as complete, inside one update of the attempts map | The attempt was marked complete first and the credential written after. On SQLite nothing could look in between; on a network database a caller that saw "complete" could list no credential | -| `patches/effect@4.0.0-beta.83.patch` | Adds Effect's own fix for re-entrant interruption, first released in `4.0.0-beta.100` | A fiber that woke another fiber which interrupted it synchronously had its continuation stack corrupted once its cleanup awaited anything. With Postgres that leaked a pooled connection. Drop these hunks when upstream moves to `beta.100` or later | -| `turbo.json` | `OPENCODE_TEST_DATABASE_URL` passes through to every task; `@opencode-ai/effect-drizzle-pg#test` is a task | turbo strips variables it is not told about | -| Test helpers: `packages/core/test/preload.ts`, `packages/opencode/test/preload.ts`, `packages/opencode/test/fixture/db.ts`, `packages/opencode/test/server/httpapi-exercise/environment.ts`, `packages/sdk-next/test/embedded.test.ts` | Ask for a throwaway schema; no database file to delete | | +| File | What differs | Why | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| The 11 table files: `packages/core/src/{account,credential,event,permission,project,session,share}/sql.ts`, `control-plane/workspace.sql.ts`, `data-migration.sql.ts`, `database/schema.sql.ts`, `database/path.ts` | Postgres table builder; `count`, `jsonb` and `text` columns from `effect-drizzle-pg`; time columns are `double precision` | Written by `documentor/scripts/convert-schema.py`, never by hand | +| `packages/core/src/database/schema.gen.ts`, `packages/core/schema.json`, `packages/core/drizzle.config.ts` | The Postgres baseline, as one statement batch | Generated; see "Regenerating the baseline" | +| `packages/core/src/database/database.ts` | Connects with `@effect/sql-pg`; verifies collation and migrations and changes nothing; a throwaway schema per layer when `OPENCODE_DATABASE_EPHEMERAL=1` | The service itself | +| `packages/core/src/database/migration.ts` | `migrate`, `migrateAll`, `verify`, `pending`, `BASELINE_IDS`, `ported` | Schema work is a command, not a start-up side effect | +| `packages/core/src/project/directories.ts` | `Transaction` is `Database.Transaction` | It was typed against the SQLite client | +| `packages/core/src/flag/flag.ts` | No `OPENCODE_DB` | Nothing reads it | +| `packages/core/package.json` | Depends on `@effect/sql-pg`, `pg`, `@opencode-ai/effect-drizzle-pg`; tests run with `--timeout 30000` | Each test database is a real schema, about 110 ms to build | +| `packages/opencode/src/cli/cmd/db.ts` | `db `, `db migrate [--all]`, `db provision`, `db grant-read`, all outside the application runtime | The runtime's database service refuses the schemas these commands exist to create and migrate | +| `packages/opencode/src/session/processor.ts` | One lock per processor around each read-then-write of a tool part; `tool-input-delta` and `tool-input-end` do not read a part the processor already tracks | The processor and the running tool both update a tool part. SQLite made each update atomic; a network database lets them interleave, and the tool's metadata was lost. Each read is a round trip, and a large edit streams thousands of deltas | +| `packages/opencode/src/acp/service.ts` | `prompt` passes a `messageId` that starts with `msg` to v1 as `messageID`, for a normal prompt and a slash command; an empty prompt passes none | DocuMentor names the user message a prompt creates, so its optimistic, live and durable copies share one id. ACP clients that send UUIDs keep opencode-minted ids | +| `packages/opencode/src/session/prompt.ts` | `prompt` with no parts and no `noReply`, on a session that already has a user message, stores nothing and runs `loop`; on a session with no user message it stores a zero-part row as upstream does | DocuMentor's abort-then-answer: after a cancel, an empty ACP prompt answers the user messages that arrived during the cancelled step, which opencode already holds. A v1 client sending `parts: []` gets the same resume; upstream clients send none | +| `packages/opencode/src/acp/event.ts` | `agent_message_chunk`, `agent_thought_chunk`, `tool_call` and `tool_call_update`, live and replayed, carry `_meta.opencode = { messageId, parentId }`: the assistant message and the user message it answers. The subscription caches each message's `parentID` from `message.updated`, fetches it once on a miss, and forgets a session's cache when the session goes idle | DocuMentor detects from the ACP stream alone, in order, the moment the agent starts answering a message sent while the turn ran | +| Fork cases in `packages/opencode/test/acp/event.test.ts`, `test/acp/service-session.test.ts` and `test/session/prompt.test.ts` | Tests named `documentor: …` | They pin the three rows above; a rebase keeps them | +| `packages/core/src/integration.ts` | An OAuth attempt's credential is stored before the attempt reads as complete, inside one update of the attempts map | The attempt was marked complete first and the credential written after. On SQLite nothing could look in between; on a network database a caller that saw "complete" could list no credential | +| `patches/effect@4.0.0-beta.83.patch` | Adds Effect's own fix for re-entrant interruption, first released in `4.0.0-beta.100` | A fiber that woke another fiber which interrupted it synchronously had its continuation stack corrupted once its cleanup awaited anything. With Postgres that leaked a pooled connection. Drop these hunks when upstream moves to `beta.100` or later | +| `turbo.json` | `OPENCODE_TEST_DATABASE_URL` passes through to every task; `@opencode-ai/effect-drizzle-pg#test` is a task | turbo strips variables it is not told about | +| Test helpers: `packages/core/test/preload.ts`, `packages/opencode/test/preload.ts`, `packages/opencode/test/fixture/db.ts`, `packages/opencode/test/server/httpapi-exercise/environment.ts`, `packages/sdk-next/test/embedded.test.ts` | Ask for a throwaway schema; no database file to delete | | Upstream tests that are changed or deleted are listed, with reasons, in [`upstream-test-exceptions.md`](upstream-test-exceptions.md). @@ -116,6 +119,7 @@ must produce the same `migration` journal; `documentor/scripts/binary-smoke.sh`. 6. Read `upstream-test-exceptions.md` against the rebased tests. A test that upstream rewrote may no longer need its exception. +7. Run `bun test test/acp/event.test.ts test/acp/service-session.test.ts test/session/prompt.test.ts -t "documentor:"` in `packages/opencode`. These pin the ACP changes DocuMentor's queued messages depend on; a failure means upstream moved the code they patch. A new upstream test that fails only on Postgres is most often one of two things. Either it reads state the instant a call returns, before a background diff --git a/documentor/upstream-test-exceptions.md b/documentor/upstream-test-exceptions.md index 7c9847506755..fcddfeb58eea 100644 --- a/documentor/upstream-test-exceptions.md +++ b/documentor/upstream-test-exceptions.md @@ -14,3 +14,6 @@ make it pass on Postgres or keep it listed here. | `packages/core/test/integration.test.ts`: "completes auto OAuth in the background" | Changed | It yielded once and then expected the attempt to be complete. The test now waits for the attempt to leave "pending" and then asserts, with nothing in between, that it is complete and its credential is listed | — | | `packages/sdk-next/test/embedded.test.ts` | Changed | Each test pointed `Flag.OPENCODE_DB` at a temporary SQLite file. The file sets the throwaway-schema environment once instead | — | | `packages/opencode/test/lib/cli-process.ts` and `test/cli/run/run-process.test.ts` | Changed | The subprocess harness allowed 30 s per `opencode run`, and the file 60 s per test. Thirteen subprocesses compile the CLI from source at once on a four-core runner; on unmodified upstream three of them already exceed 30 s there, and each process here also opens a database schema. Now 90 s and 120 s; the "exits nonzero promptly" case allows 45 s instead of 15 s and still proves the process exits rather than hanging until the harness kills it | — | +| `packages/opencode/test/acp/event.test.ts`: "uses reasoning part ids as ACP thought message boundaries" | Changed | Each expected update also carries `_meta.opencode` (FORK.md, `acp/event.ts`) | — | +| `packages/opencode/test/acp/service-session.test.ts`: "replays loaded session transcript chunks", "replays reasoning parts as separate ACP thought messages" | Changed | Each expected assistant or thought update also carries `_meta.opencode` (FORK.md, `acp/event.ts`) | — | +| `packages/opencode/test/acp/event.test.ts`: "does not call sdk.session.message repeatedly when metadata is known" | Changed | Every content update now also names the message's parent, so "metadata is known" includes the parent. The test delivers the message's `message.updated` before its deltas, as opencode does, and still asserts no lookups and five updates (FORK.md, `acp/event.ts`) | — | diff --git a/packages/opencode/src/acp/event.ts b/packages/opencode/src/acp/event.ts index 1180d3f5b0f5..a9d2dfa88b3b 100644 --- a/packages/opencode/src/acp/event.ts +++ b/packages/opencode/src/acp/event.ts @@ -3,6 +3,7 @@ import type { Event, EventMessagePartDelta, EventMessagePartUpdated, + Message, OpencodeClient, Part, SessionMessageResponse, @@ -30,6 +31,14 @@ type GlobalEventStream = { stream: AsyncIterable } +/** + * `_meta.opencode` on every content and tool update: the assistant message the + * update belongs to, and the user message that assistant answers. A client that + * sends a prompt while a turn runs reads from it the moment the agent starts + * answering that prompt. + */ +export type OpencodeUpdateMeta = { opencode: { messageId: string; parentId: string | null } } + export function start(input: { sdk: OpencodeClient; connection: Connection; session: ACPSession.Interface }) { const subscription = new Subscription(input) subscription.start() @@ -42,6 +51,9 @@ export class Subscription { private readonly toolStarts = new Set() private readonly connectionWaiters = new Set<() => void>() private readonly idleWaiters = new Map>>() + // Per session: message id → that message's parentID (null for a user message). Filled from message.updated, + // which opencode publishes before any part of the message, and forgotten when the session goes idle. + private readonly messageParents = new Map>() private readonly permission: ACPPermission.Handler private connected = false private started = false @@ -95,6 +107,9 @@ export class Subscription { case "session.status": if (event.properties.status.type === "idle") this.idle(event.properties.sessionID) return + case "message.updated": + this.recordMessage(event.properties.info) + return case "permission.asked": this.permission.handle(event) return @@ -107,6 +122,7 @@ export class Subscription { async replayMessage(message: SessionMessageResponse) { if (message.info.role !== "assistant" && message.info.role !== "user") return + this.recordMessage(message.info) const cwd = message.info.role === "assistant" ? message.info.path?.cwd : undefined for (const part of message.parts) { @@ -136,6 +152,16 @@ export class Subscription { sessionUpdate, messageId: part.type === "reasoning" ? part.id : message.info.id, ...chunk, + ...(sessionUpdate === "user_message_chunk" + ? {} + : { + _meta: { + opencode: { + messageId: message.info.id, + parentId: message.info.role === "assistant" ? (message.info.parentID ?? null) : null, + }, + } satisfies OpencodeUpdateMeta, + }), }, }) } @@ -182,12 +208,40 @@ export class Subscription { } private idle(sessionId: string) { + this.messageParents.delete(sessionId) const waiters = this.idleWaiters.get(sessionId) if (!waiters) return this.idleWaiters.delete(sessionId) for (const waiter of waiters) waiter.resolve() } + private recordMessage(info: Message) { + const parents = this.messageParents.get(info.sessionID) ?? new Map() + parents.set(info.id, info.role === "assistant" ? (info.parentID ?? null) : null) + this.messageParents.set(info.sessionID, parents) + } + + private async messageMeta(sessionId: string, cwd: string, messageId: string): Promise { + const known = this.messageParents.get(sessionId)?.get(messageId) + if (known !== undefined) return { opencode: { messageId, parentId: known } } + // The subscription misses message.updated when it connects mid-turn or after the session went idle. Asking + // the server costs one request per message; a failed lookup is not cached, so the next update asks again. + const message = await Promise.resolve() + .then(() => + this.input.sdk.session.message( + { sessionID: sessionId, messageID: messageId, directory: cwd }, + { throwOnError: true }, + ), + ) + .then((response) => response.data) + .catch(() => undefined) + if (!message) return { opencode: { messageId, parentId: null } } + this.recordMessage(message.info) + return { + opencode: { messageId, parentId: message.info.role === "assistant" ? (message.info.parentID ?? null) : null }, + } + } + private async handlePartUpdated(event: EventMessagePartUpdated) { const part = event.properties.part const sessionId = part.sessionID || event.properties.sessionID @@ -238,6 +292,7 @@ export class Subscription { type: "text", text: props.delta, }, + _meta: await this.messageMeta(session.id, session.cwd, props.messageID), }, }) return @@ -253,6 +308,7 @@ export class Subscription { type: "text", text: props.delta, }, + _meta: await this.messageMeta(session.id, session.cwd, props.messageID), }, }) } @@ -271,6 +327,7 @@ export class Subscription { .then((response) => response.data) .catch(() => undefined) if (!message) return + this.recordMessage(message.info) const part = message.parts.find((item) => item.id === partId) if (!part) return @@ -293,7 +350,8 @@ export class Subscription { } private async handleToolPart(sessionId: string, part: ToolPart, cwd: string) { - await this.toolStart(sessionId, part, cwd) + const meta = await this.messageMeta(sessionId, cwd, part.messageID) + await this.toolStart(sessionId, part, cwd, meta) switch (part.state.status) { case "pending": @@ -301,7 +359,7 @@ export class Subscription { return case "running": - await this.runningTool(sessionId, part, cwd) + await this.runningTool(sessionId, part, cwd, meta) return case "completed": @@ -316,6 +374,7 @@ export class Subscription { state: part.state, cwd, }), + _meta: meta, }, }) return @@ -332,13 +391,14 @@ export class Subscription { state: part.state, cwd, }), + _meta: meta, }, }) return } } - private async runningTool(sessionId: string, part: ToolPart, cwd: string) { + private async runningTool(sessionId: string, part: ToolPart, cwd: string, meta: OpencodeUpdateMeta) { if (part.state.status !== "running") return const output = part.tool === "bash" ? shellOutputSnapshot(part.state) : undefined @@ -354,6 +414,7 @@ export class Subscription { state: part.state, cwd, }), + _meta: meta, }, }) return @@ -372,11 +433,12 @@ export class Subscription { output, cwd, }), + _meta: meta, }, }) } - private async toolStart(sessionId: string, part: ToolPart, cwd: string) { + private async toolStart(sessionId: string, part: ToolPart, cwd: string, meta: OpencodeUpdateMeta) { if (this.toolStarts.has(part.callID)) return this.toolStarts.add(part.callID) await this.input.connection.sessionUpdate({ @@ -389,6 +451,7 @@ export class Subscription { state: part.state, cwd, }), + _meta: meta, }, }) } diff --git a/packages/opencode/test/acp/event.test.ts b/packages/opencode/test/acp/event.test.ts index 1806c4a27af9..038bed59c90e 100644 --- a/packages/opencode/test/acp/event.test.ts +++ b/packages/opencode/test/acp/event.test.ts @@ -318,6 +318,35 @@ async function createKnownSession( ) } +function assistantInfoWithParent(sessionID: string, messageID: string, parentID: string) { + return { + id: messageID, + sessionID, + role: "assistant", + time: { created: Date.now() }, + parentID, + modelID: "model", + providerID: "provider", + mode: "build", + agent: "build", + path: { cwd: "/workspace", root: "/workspace" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + } satisfies Message +} + +function messageUpdated(info: Message): Event { + return { id: `evt_${info.id}_updated`, type: "message.updated", properties: { sessionID: info.sessionID, info } } +} + +function sessionIdle(sessionID: string): Event { + return { + id: `evt_${sessionID}_idle`, + type: "session.status", + properties: { sessionID, status: { type: "idle" } }, + } +} + describe("acp event routing", () => { it("routes message.part.delta by sessionID without cross-session pollution", async () => { const harness = createHarness() @@ -377,11 +406,13 @@ describe("acp event routing", () => { sessionUpdate: "agent_thought_chunk", messageId: "part_first", content: { type: "text", text: "First" }, + _meta: { opencode: { messageId: "msg_reasoning", parentId: null } }, }, { sessionUpdate: "agent_thought_chunk", messageId: "part_second", content: { type: "text", text: "Second" }, + _meta: { opencode: { messageId: "msg_reasoning", parentId: null } }, }, ]) }) @@ -434,6 +465,7 @@ describe("acp event routing", () => { it("does not call sdk.session.message repeatedly when metadata is known", async () => { const harness = createHarness() await createKnownSession(harness.session, "ses_a", { messageId: "msg_a", partId: "part_a", partType: "text" }) + await harness.subscription.handle(messageUpdated(assistantInfoWithParent("ses_a", "msg_a", "msg_parent"))) for (const delta of ["a", "b", "c", "d", "e"]) { await harness.subscription.handle(textDelta("ses_a", "msg_a", "part_a", delta)) @@ -783,3 +815,103 @@ describe("acp event routing", () => { ]) }) }) + +describe("documentor: acp update message meta", () => { + it("documentor: names the assistant message and the message it answers on text chunks", async () => { + const harness = createHarness() + await createKnownSession(harness.session, "ses_meta", { + messageId: "msg_answer", + partId: "part_text", + partType: "text", + }) + await harness.subscription.handle( + messageUpdated(assistantInfoWithParent("ses_meta", "msg_answer", "msg_user_queued")), + ) + await harness.subscription.handle(textDelta("ses_meta", "msg_answer", "part_text", "hello")) + + expect(harness.updates.map((update) => update.update)).toEqual([ + { + sessionUpdate: "agent_message_chunk", + messageId: "msg_answer", + content: { type: "text", text: "hello" }, + _meta: { opencode: { messageId: "msg_answer", parentId: "msg_user_queued" } }, + }, + ]) + expect(harness.calls.message).toBe(0) + }) + + it("documentor: names the assistant message, not the reasoning part, on thought chunks", async () => { + const harness = createHarness() + await createKnownSession(harness.session, "ses_meta", { + messageId: "msg_answer", + partId: "part_thought", + partType: "reasoning", + }) + await harness.subscription.handle( + messageUpdated(assistantInfoWithParent("ses_meta", "msg_answer", "msg_user_queued")), + ) + await harness.subscription.handle(textDelta("ses_meta", "msg_answer", "part_thought", "thinking")) + + expect(harness.updates.map((update) => update.update)).toEqual([ + { + sessionUpdate: "agent_thought_chunk", + messageId: "part_thought", + content: { type: "text", text: "thinking" }, + _meta: { opencode: { messageId: "msg_answer", parentId: "msg_user_queued" } }, + }, + ]) + }) + + it("documentor: names the message on every tool update", async () => { + const harness = createHarness() + await Effect.runPromise(harness.session.create({ id: "ses_tool_meta", cwd: "/workspace" })) + await harness.subscription.handle( + messageUpdated(assistantInfoWithParent("ses_tool_meta", "msg_call_1", "msg_user_queued")), + ) + await harness.subscription.handle(toolUpdated(runningTool("ses_tool_meta", "call_1", "hello"))) + await harness.subscription.handle(toolUpdated(completedTool("ses_tool_meta", "call_1"))) + + const meta = { opencode: { messageId: "msg_call_1", parentId: "msg_user_queued" } } + expect(toolUpdates(harness.updates).map((item) => [item.update.sessionUpdate, item.update._meta])).toEqual([ + ["tool_call", meta], + ["tool_call_update", meta], + ["tool_call_update", meta], + ]) + expect(harness.calls.message).toBe(0) + }) + + it("documentor: fetches the parent once when message.updated was missed", async () => { + const harness = createHarness({ msg_call_2: assistantToolMessage(completedTool("ses_fetch", "call_2")) }) + await Effect.runPromise(harness.session.create({ id: "ses_fetch", cwd: "/workspace" })) + await harness.subscription.handle(toolUpdated(runningTool("ses_fetch", "call_2", "x"))) + await harness.subscription.handle(toolUpdated(completedTool("ses_fetch", "call_2"))) + + const parents = toolUpdates(harness.updates).map((item) => item.update._meta) + expect(parents).toEqual([ + { opencode: { messageId: "msg_call_2", parentId: "msg_parent" } }, + { opencode: { messageId: "msg_call_2", parentId: "msg_parent" } }, + { opencode: { messageId: "msg_call_2", parentId: "msg_parent" } }, + ]) + expect(harness.calls.message).toBe(1) + }) + + it("documentor: forgets parents at idle and fetches them again", async () => { + const harness = createHarness({ msg_answer: assistantMessage("ses_idle", "msg_answer", "part_text", "text") }) + await createKnownSession(harness.session, "ses_idle", { + messageId: "msg_answer", + partId: "part_text", + partType: "text", + }) + await harness.subscription.handle(messageUpdated(assistantInfoWithParent("ses_idle", "msg_answer", "msg_parent"))) + await harness.subscription.handle(textDelta("ses_idle", "msg_answer", "part_text", "before")) + expect(harness.calls.message).toBe(0) + + await harness.subscription.handle(sessionIdle("ses_idle")) + await harness.subscription.handle(textDelta("ses_idle", "msg_answer", "part_text", "after")) + + expect(harness.calls.message).toBe(1) + expect(harness.updates.at(-1)?.update._meta).toEqual({ + opencode: { messageId: "msg_answer", parentId: "msg_parent" }, + }) + }) +}) diff --git a/packages/opencode/test/acp/service-session.test.ts b/packages/opencode/test/acp/service-session.test.ts index 9dca24acd341..094f63cc6ed6 100644 --- a/packages/opencode/test/acp/service-session.test.ts +++ b/packages/opencode/test/acp/service-session.test.ts @@ -571,6 +571,7 @@ describe("ACP service sessions", () => { sessionUpdate: "agent_message_chunk", messageId: "msg_assistant", content: { type: "text", text: "hi there" }, + _meta: { opencode: { messageId: "msg_assistant", parentId: null } }, }, ]) }) @@ -607,15 +608,40 @@ describe("ACP service sessions", () => { sessionUpdate: "agent_thought_chunk", messageId: "part_first", content: { type: "text", text: "First" }, + _meta: { opencode: { messageId: "msg_assistant", parentId: null } }, }, { sessionUpdate: "agent_thought_chunk", messageId: "part_second", content: { type: "text", text: "Second" }, + _meta: { opencode: { messageId: "msg_assistant", parentId: null } }, }, ]) }) + it("documentor: replays each assistant chunk with its message and parent", async () => { + const { service, updates } = makeService([ + { + info: { id: "msg_user", sessionID: "ses_loaded", role: "user" }, + parts: [{ id: "part_user", sessionID: "ses_loaded", messageID: "msg_user", type: "text", text: "hello" }], + }, + { + info: { id: "msg_assistant", sessionID: "ses_loaded", role: "assistant", parentID: "msg_user" }, + parts: [ + { id: "part_assistant", sessionID: "ses_loaded", messageID: "msg_assistant", type: "text", text: "hi there" }, + ], + }, + ]) + + await Effect.runPromise(service.loadSession({ cwd: "/workspace", sessionId: "ses_loaded", mcpServers: [] })) + + const replayed = updates.map((item) => item.update) + expect(replayed.find((item) => item.sessionUpdate === "user_message_chunk")).not.toHaveProperty("_meta") + expect(replayed.find((item) => item.sessionUpdate === "agent_message_chunk")).toMatchObject({ + _meta: { opencode: { messageId: "msg_assistant", parentId: "msg_user" } }, + }) + }) + it("lists sessions sorted by updated time with cursor support", async () => { const { service } = makeService() const first = await Effect.runPromise(service.listSessions({ cwd: "/workspace" })) From caadf434969d536752ebe0832a39090a09ca1270 Mon Sep 17 00:00:00 2001 From: Csaba Fabian Date: Thu, 8 Oct 2026 20:57:38 +0000 Subject: [PATCH 4/4] fix(acp): look up a message's parent once, failures included, and only for tracked sessions Co-Authored-By: Claude Opus 5.5 --- documentor/FORK.md | 43 +++++++------- documentor/upstream-test-exceptions.md | 2 +- packages/opencode/src/acp/event.ts | 57 ++++++++++++------- packages/opencode/test/acp/event.test.ts | 72 ++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 42 deletions(-) diff --git a/documentor/FORK.md b/documentor/FORK.md index 735620f74658..b71aedebed2b 100644 --- a/documentor/FORK.md +++ b/documentor/FORK.md @@ -7,30 +7,31 @@ Postgres. It has no SQLite path. The design is in Everything the fork adds lives in `documentor/`, `packages/effect-drizzle-pg/`, `packages/core/test/pg/`, `packages/opencode/test/pg/` and the two `documentor-*` workflows, except the `documentor: …` test cases listed in the -table below, which sit beside the upstream tests of the code they cover. The rest of this document is about the upstream -files it changes, because those are what a rebase meets. +table below, which sit beside the upstream tests of the code they cover. The +rest of this document is about the upstream files it changes, because those are +what a rebase meets. ## Upstream files changed -| File | What differs | Why | -| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| The 11 table files: `packages/core/src/{account,credential,event,permission,project,session,share}/sql.ts`, `control-plane/workspace.sql.ts`, `data-migration.sql.ts`, `database/schema.sql.ts`, `database/path.ts` | Postgres table builder; `count`, `jsonb` and `text` columns from `effect-drizzle-pg`; time columns are `double precision` | Written by `documentor/scripts/convert-schema.py`, never by hand | -| `packages/core/src/database/schema.gen.ts`, `packages/core/schema.json`, `packages/core/drizzle.config.ts` | The Postgres baseline, as one statement batch | Generated; see "Regenerating the baseline" | -| `packages/core/src/database/database.ts` | Connects with `@effect/sql-pg`; verifies collation and migrations and changes nothing; a throwaway schema per layer when `OPENCODE_DATABASE_EPHEMERAL=1` | The service itself | -| `packages/core/src/database/migration.ts` | `migrate`, `migrateAll`, `verify`, `pending`, `BASELINE_IDS`, `ported` | Schema work is a command, not a start-up side effect | -| `packages/core/src/project/directories.ts` | `Transaction` is `Database.Transaction` | It was typed against the SQLite client | -| `packages/core/src/flag/flag.ts` | No `OPENCODE_DB` | Nothing reads it | -| `packages/core/package.json` | Depends on `@effect/sql-pg`, `pg`, `@opencode-ai/effect-drizzle-pg`; tests run with `--timeout 30000` | Each test database is a real schema, about 110 ms to build | -| `packages/opencode/src/cli/cmd/db.ts` | `db `, `db migrate [--all]`, `db provision`, `db grant-read`, all outside the application runtime | The runtime's database service refuses the schemas these commands exist to create and migrate | -| `packages/opencode/src/session/processor.ts` | One lock per processor around each read-then-write of a tool part; `tool-input-delta` and `tool-input-end` do not read a part the processor already tracks | The processor and the running tool both update a tool part. SQLite made each update atomic; a network database lets them interleave, and the tool's metadata was lost. Each read is a round trip, and a large edit streams thousands of deltas | -| `packages/opencode/src/acp/service.ts` | `prompt` passes a `messageId` that starts with `msg` to v1 as `messageID`, for a normal prompt and a slash command; an empty prompt passes none | DocuMentor names the user message a prompt creates, so its optimistic, live and durable copies share one id. ACP clients that send UUIDs keep opencode-minted ids | -| `packages/opencode/src/session/prompt.ts` | `prompt` with no parts and no `noReply`, on a session that already has a user message, stores nothing and runs `loop`; on a session with no user message it stores a zero-part row as upstream does | DocuMentor's abort-then-answer: after a cancel, an empty ACP prompt answers the user messages that arrived during the cancelled step, which opencode already holds. A v1 client sending `parts: []` gets the same resume; upstream clients send none | -| `packages/opencode/src/acp/event.ts` | `agent_message_chunk`, `agent_thought_chunk`, `tool_call` and `tool_call_update`, live and replayed, carry `_meta.opencode = { messageId, parentId }`: the assistant message and the user message it answers. The subscription caches each message's `parentID` from `message.updated`, fetches it once on a miss, and forgets a session's cache when the session goes idle | DocuMentor detects from the ACP stream alone, in order, the moment the agent starts answering a message sent while the turn ran | -| Fork cases in `packages/opencode/test/acp/event.test.ts`, `test/acp/service-session.test.ts` and `test/session/prompt.test.ts` | Tests named `documentor: …` | They pin the three rows above; a rebase keeps them | -| `packages/core/src/integration.ts` | An OAuth attempt's credential is stored before the attempt reads as complete, inside one update of the attempts map | The attempt was marked complete first and the credential written after. On SQLite nothing could look in between; on a network database a caller that saw "complete" could list no credential | -| `patches/effect@4.0.0-beta.83.patch` | Adds Effect's own fix for re-entrant interruption, first released in `4.0.0-beta.100` | A fiber that woke another fiber which interrupted it synchronously had its continuation stack corrupted once its cleanup awaited anything. With Postgres that leaked a pooled connection. Drop these hunks when upstream moves to `beta.100` or later | -| `turbo.json` | `OPENCODE_TEST_DATABASE_URL` passes through to every task; `@opencode-ai/effect-drizzle-pg#test` is a task | turbo strips variables it is not told about | -| Test helpers: `packages/core/test/preload.ts`, `packages/opencode/test/preload.ts`, `packages/opencode/test/fixture/db.ts`, `packages/opencode/test/server/httpapi-exercise/environment.ts`, `packages/sdk-next/test/embedded.test.ts` | Ask for a throwaway schema; no database file to delete | | +| File | What differs | Why | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| The 11 table files: `packages/core/src/{account,credential,event,permission,project,session,share}/sql.ts`, `control-plane/workspace.sql.ts`, `data-migration.sql.ts`, `database/schema.sql.ts`, `database/path.ts` | Postgres table builder; `count`, `jsonb` and `text` columns from `effect-drizzle-pg`; time columns are `double precision` | Written by `documentor/scripts/convert-schema.py`, never by hand | +| `packages/core/src/database/schema.gen.ts`, `packages/core/schema.json`, `packages/core/drizzle.config.ts` | The Postgres baseline, as one statement batch | Generated; see "Regenerating the baseline" | +| `packages/core/src/database/database.ts` | Connects with `@effect/sql-pg`; verifies collation and migrations and changes nothing; a throwaway schema per layer when `OPENCODE_DATABASE_EPHEMERAL=1` | The service itself | +| `packages/core/src/database/migration.ts` | `migrate`, `migrateAll`, `verify`, `pending`, `BASELINE_IDS`, `ported` | Schema work is a command, not a start-up side effect | +| `packages/core/src/project/directories.ts` | `Transaction` is `Database.Transaction` | It was typed against the SQLite client | +| `packages/core/src/flag/flag.ts` | No `OPENCODE_DB` | Nothing reads it | +| `packages/core/package.json` | Depends on `@effect/sql-pg`, `pg`, `@opencode-ai/effect-drizzle-pg`; tests run with `--timeout 30000` | Each test database is a real schema, about 110 ms to build | +| `packages/opencode/src/cli/cmd/db.ts` | `db `, `db migrate [--all]`, `db provision`, `db grant-read`, all outside the application runtime | The runtime's database service refuses the schemas these commands exist to create and migrate | +| `packages/opencode/src/session/processor.ts` | One lock per processor around each read-then-write of a tool part; `tool-input-delta` and `tool-input-end` do not read a part the processor already tracks | The processor and the running tool both update a tool part. SQLite made each update atomic; a network database lets them interleave, and the tool's metadata was lost. Each read is a round trip, and a large edit streams thousands of deltas | +| `packages/opencode/src/acp/service.ts` | `prompt` passes a `messageId` that starts with `msg` to v1 as `messageID`, for a normal prompt and a slash command; an empty prompt passes none | DocuMentor names the user message a prompt creates, so its optimistic, live and durable copies share one id. ACP clients that send UUIDs keep opencode-minted ids | +| `packages/opencode/src/session/prompt.ts` | `prompt` with no parts and no `noReply`, on a session that already has a user message, stores nothing and runs `loop`; on a session with no user message it stores a zero-part row as upstream does | DocuMentor's abort-then-answer: after a cancel, an empty ACP prompt answers the user messages that arrived during the cancelled step, which opencode already holds. A v1 client sending `parts: []` gets the same resume; upstream clients send none | +| `packages/opencode/src/acp/event.ts` | `agent_message_chunk`, `agent_thought_chunk`, `tool_call` and `tool_call_update`, live and replayed, carry `_meta.opencode = { messageId, parentId }`: the assistant message and the user message it answers. The subscription caches each assistant message's `parentID` from `message.updated` for the sessions it tracks. On a miss it fetches the message once, shared by concurrent updates; a failed fetch is remembered until the session goes idle, and meanwhile the updates carry `parentId: null`. A session's cache is forgotten when it goes idle, and the whole cache when the event stream disconnects | DocuMentor detects from the ACP stream alone, in order, the moment the agent starts answering a message sent while the turn ran | +| Fork cases in `packages/opencode/test/acp/event.test.ts`, `test/acp/service-session.test.ts` and `test/session/prompt.test.ts` | Tests named `documentor: …` | They pin the three rows above; a rebase keeps them | +| `packages/core/src/integration.ts` | An OAuth attempt's credential is stored before the attempt reads as complete, inside one update of the attempts map | The attempt was marked complete first and the credential written after. On SQLite nothing could look in between; on a network database a caller that saw "complete" could list no credential | +| `patches/effect@4.0.0-beta.83.patch` | Adds Effect's own fix for re-entrant interruption, first released in `4.0.0-beta.100` | A fiber that woke another fiber which interrupted it synchronously had its continuation stack corrupted once its cleanup awaited anything. With Postgres that leaked a pooled connection. Drop these hunks when upstream moves to `beta.100` or later | +| `turbo.json` | `OPENCODE_TEST_DATABASE_URL` passes through to every task; `@opencode-ai/effect-drizzle-pg#test` is a task | turbo strips variables it is not told about | +| Test helpers: `packages/core/test/preload.ts`, `packages/opencode/test/preload.ts`, `packages/opencode/test/fixture/db.ts`, `packages/opencode/test/server/httpapi-exercise/environment.ts`, `packages/sdk-next/test/embedded.test.ts` | Ask for a throwaway schema; no database file to delete | | Upstream tests that are changed or deleted are listed, with reasons, in [`upstream-test-exceptions.md`](upstream-test-exceptions.md). diff --git a/documentor/upstream-test-exceptions.md b/documentor/upstream-test-exceptions.md index fcddfeb58eea..b13c7017e873 100644 --- a/documentor/upstream-test-exceptions.md +++ b/documentor/upstream-test-exceptions.md @@ -16,4 +16,4 @@ make it pass on Postgres or keep it listed here. | `packages/opencode/test/lib/cli-process.ts` and `test/cli/run/run-process.test.ts` | Changed | The subprocess harness allowed 30 s per `opencode run`, and the file 60 s per test. Thirteen subprocesses compile the CLI from source at once on a four-core runner; on unmodified upstream three of them already exceed 30 s there, and each process here also opens a database schema. Now 90 s and 120 s; the "exits nonzero promptly" case allows 45 s instead of 15 s and still proves the process exits rather than hanging until the harness kills it | — | | `packages/opencode/test/acp/event.test.ts`: "uses reasoning part ids as ACP thought message boundaries" | Changed | Each expected update also carries `_meta.opencode` (FORK.md, `acp/event.ts`) | — | | `packages/opencode/test/acp/service-session.test.ts`: "replays loaded session transcript chunks", "replays reasoning parts as separate ACP thought messages" | Changed | Each expected assistant or thought update also carries `_meta.opencode` (FORK.md, `acp/event.ts`) | — | -| `packages/opencode/test/acp/event.test.ts`: "does not call sdk.session.message repeatedly when metadata is known" | Changed | Every content update now also names the message's parent, so "metadata is known" includes the parent. The test delivers the message's `message.updated` before its deltas, as opencode does, and still asserts no lookups and five updates (FORK.md, `acp/event.ts`) | — | +| `packages/opencode/test/acp/event.test.ts`: "does not call sdk.session.message repeatedly when metadata is known" | Changed | Every content update also names the message's parent, so "metadata is known" includes the parent. The test delivers the message's `message.updated` before its deltas, as opencode does, and still asserts no lookups and five updates (FORK.md, `acp/event.ts`) | — | diff --git a/packages/opencode/src/acp/event.ts b/packages/opencode/src/acp/event.ts index a9d2dfa88b3b..abc90503d3f5 100644 --- a/packages/opencode/src/acp/event.ts +++ b/packages/opencode/src/acp/event.ts @@ -51,9 +51,10 @@ export class Subscription { private readonly toolStarts = new Set() private readonly connectionWaiters = new Set<() => void>() private readonly idleWaiters = new Map>>() - // Per session: message id → that message's parentID (null for a user message). Filled from message.updated, - // which opencode publishes before any part of the message, and forgotten when the session goes idle. - private readonly messageParents = new Map>() + // Per tracked session: assistant message id → its parentID. Filled from message.updated, which opencode publishes + // before any part of the message, or by one lookup on a miss; a failed lookup holds null. Forgotten when the + // session goes idle and when the event stream disconnects. + private readonly messageParents = new Map>>() private readonly permission: ACPPermission.Handler private connected = false private started = false @@ -108,8 +109,7 @@ export class Subscription { if (event.properties.status.type === "idle") this.idle(event.properties.sessionID) return case "message.updated": - this.recordMessage(event.properties.info) - return + return this.handleMessageUpdated(event.properties.info) case "permission.asked": this.permission.handle(event) return @@ -198,6 +198,7 @@ export class Subscription { } private disconnected() { + this.messageParents.clear() if (!this.connected) return this.connected = false const error = new Error("ACP event stream disconnected") @@ -215,31 +216,47 @@ export class Subscription { for (const waiter of waiters) waiter.resolve() } + private async handleMessageUpdated(info: Message) { + const session = await Effect.runPromise(this.input.session.tryGet(info.sessionID)) + if (!session) return + this.recordMessage(info) + } + + // Only assistant messages: updates name the assistant message they belong to, never a user message. private recordMessage(info: Message) { - const parents = this.messageParents.get(info.sessionID) ?? new Map() - parents.set(info.id, info.role === "assistant" ? (info.parentID ?? null) : null) - this.messageParents.set(info.sessionID, parents) + if (info.role !== "assistant") return + this.sessionParents(info.sessionID).set(info.id, Promise.resolve(info.parentID ?? null)) + } + + private sessionParents(sessionId: string) { + const existing = this.messageParents.get(sessionId) + if (existing) return existing + const parents = new Map>() + this.messageParents.set(sessionId, parents) + return parents } private async messageMeta(sessionId: string, cwd: string, messageId: string): Promise { - const known = this.messageParents.get(sessionId)?.get(messageId) - if (known !== undefined) return { opencode: { messageId, parentId: known } } - // The subscription misses message.updated when it connects mid-turn or after the session went idle. Asking - // the server costs one request per message; a failed lookup is not cached, so the next update asks again. - const message = await Promise.resolve() + const parents = this.sessionParents(sessionId) + const known = parents.get(messageId) + if (known) return { opencode: { messageId, parentId: await known } } + // The subscription misses message.updated when it connects mid-turn or after the session went idle. One lookup + // per message answers every update for it, concurrent ones included. A failed lookup is remembered as null until + // the session goes idle, so a server that cannot answer is not asked again for each chunk of the stream. + const lookup = Promise.resolve() .then(() => this.input.sdk.session.message( { sessionID: sessionId, messageID: messageId, directory: cwd }, { throwOnError: true }, ), ) - .then((response) => response.data) - .catch(() => undefined) - if (!message) return { opencode: { messageId, parentId: null } } - this.recordMessage(message.info) - return { - opencode: { messageId, parentId: message.info.role === "assistant" ? (message.info.parentID ?? null) : null }, - } + .then((response) => { + const info = response.data?.info + return info?.role === "assistant" ? (info.parentID ?? null) : null + }) + .catch(() => null) + parents.set(messageId, lookup) + return { opencode: { messageId, parentId: await lookup } } } private async handlePartUpdated(event: EventMessagePartUpdated) { diff --git a/packages/opencode/test/acp/event.test.ts b/packages/opencode/test/acp/event.test.ts index 038bed59c90e..935d498c07a3 100644 --- a/packages/opencode/test/acp/event.test.ts +++ b/packages/opencode/test/acp/event.test.ts @@ -914,4 +914,76 @@ describe("documentor: acp update message meta", () => { opencode: { messageId: "msg_answer", parentId: "msg_parent" }, }) }) + + it("documentor: asks once for a message whose lookup fails, until the session goes idle", async () => { + const harness = createHarness() + const session = harness.sdk.session as unknown as { message: () => Promise } + session.message = () => { + harness.calls.message++ + return Promise.reject(new Error("unavailable")) + } + await createKnownSession(harness.session, "ses_fail", { + messageId: "msg_answer", + partId: "part_text", + partType: "text", + }) + + for (const delta of ["a", "b", "c", "d", "e"]) { + await harness.subscription.handle(textDelta("ses_fail", "msg_answer", "part_text", delta)) + } + + expect(harness.calls.message).toBe(1) + expect(harness.updates.map((update) => update.update._meta)).toEqual( + Array.from({ length: 5 }, () => ({ opencode: { messageId: "msg_answer", parentId: null } })), + ) + + await harness.subscription.handle(sessionIdle("ses_fail")) + await harness.subscription.handle(textDelta("ses_fail", "msg_answer", "part_text", "f")) + + expect(harness.calls.message).toBe(2) + }) + + it("documentor: shares one lookup between concurrent updates of the same message", async () => { + const harness = createHarness() + const pending: Array<(value: { data: SessionMessageResponse }) => void> = [] + const session = harness.sdk.session as unknown as { message: () => Promise<{ data: SessionMessageResponse }> } + session.message = () => { + harness.calls.message++ + return new Promise((resolve) => pending.push(resolve)) + } + await createKnownSession(harness.session, "ses_race", { + messageId: "msg_answer", + partId: "part_text", + partType: "text", + }) + + const first = harness.subscription.handle(textDelta("ses_race", "msg_answer", "part_text", "a")) + const second = harness.subscription.handle(textDelta("ses_race", "msg_answer", "part_text", "b")) + await pollUntil(() => pending.length > 0, "lookup did not start") + await new Promise((resolve) => setTimeout(resolve, 20)) + for (const resolve of pending) resolve({ data: assistantMessage("ses_race", "msg_answer", "part_text", "text") }) + await Promise.all([first, second]) + + expect(harness.calls.message).toBe(1) + expect(harness.updates.map((update) => update.update._meta)).toEqual([ + { opencode: { messageId: "msg_answer", parentId: "msg_parent" } }, + { opencode: { messageId: "msg_answer", parentId: "msg_parent" } }, + ]) + }) + + it("documentor: ignores message.updated for a session this connection does not track", async () => { + const harness = createHarness({ msg_answer: assistantMessage("ses_late", "msg_answer", "part_text", "text") }) + await harness.subscription.handle(messageUpdated(assistantInfoWithParent("ses_late", "msg_answer", "msg_other"))) + await createKnownSession(harness.session, "ses_late", { + messageId: "msg_answer", + partId: "part_text", + partType: "text", + }) + await harness.subscription.handle(textDelta("ses_late", "msg_answer", "part_text", "hello")) + + expect(harness.calls.message).toBe(1) + expect(harness.updates.at(-1)?.update._meta).toEqual({ + opencode: { messageId: "msg_answer", parentId: "msg_parent" }, + }) + }) })