diff --git a/.specgit.yaml b/.specgit.yaml index 5c6d893ac..78c488e2c 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,8 +1,10 @@ version: 1 -delivery: issue389 +delivery: memory-topic-creation context: kind: branch - branch: feat/todo-step-reminders + branch: fix/395-memory-topic-creation issues: - - 389 -pr: 394 + - 395 + - 396 + - 397 +pr: 398 diff --git a/packages/opencode/src/memory/memory.ts b/packages/opencode/src/memory/memory.ts index 232983391..f5d767746 100644 --- a/packages/opencode/src/memory/memory.ts +++ b/packages/opencode/src/memory/memory.ts @@ -60,6 +60,10 @@ export interface Interface { * project passes every activation gate. Surface this wherever a silent * "remains off" would leave the user guessing (e.g. /memory on). */ readonly statusReason: () => Effect.Effect + /** Truthful one-line state for /memory status surfaces: the statusReason + * blocker when a gate (identity, init, model availability) holds Memory + * inert, else the actual on/off state. */ + readonly status: () => Effect.Effect } export class Service extends Context.Service()("@opencode/Memory") {} @@ -774,7 +778,9 @@ export const layer: Layer.Layer< // #350: the why-is-Memory-inert companion of configuration()'s fail-closed // gates. Mirrors their order; only the gates a user can act on produce a // reason (identity retirement and admission repair stay log-only — they - // are operator concerns, not /memory on guidance). + // are operator concerns, not /memory on guidance). #397: an enabled + // config whose model no longer resolves is equally inert — active() gates + // on resolveModel() — so it gets its own actionable reason. const statusReason = Effect.fn("Memory.statusReason")(function* () { const ctx = yield* InstanceState.context const current = yield* project.get(ctx.project.id) @@ -784,9 +790,29 @@ export const layer: Layer.Layer< if (current.vcs !== "git") return "Memory requires a git repository." if (!current.time.initialized) return "Memory is unavailable until the project is initialized — run /init first, then /memory on." + // An unreadable config/store answers "cannot determine" rather than + // failing the status surface. + const optioned = yield* Effect.option(configuration()) + const loaded = Option.isSome(optioned) ? optioned.value?.loaded : undefined + if (loaded?.config.enabled) { + // resolveModel answers undefined (not a failure) when the model is + // absent from the provider list; Effect.option only catches the + // torn-read ModelNotFoundError edge — both mean unavailable here. + const model = yield* Effect.option(resolveModel(loaded.config)) + if (!Option.isSome(model) || model.value === undefined) + return "Memory is enabled but its configured model is unavailable — run /memory on to reselect a replacement, or set `model` in .opencode/memory.jsonc to an installed provider/model." + } return undefined }) + const status: Interface["status"] = Effect.fn("Memory.status")(function* () { + const reason = yield* statusReason() + if (reason) return reason + const optioned = yield* Effect.option(configuration()) + const loaded = Option.isSome(optioned) ? optioned.value?.loaded : undefined + return loaded?.config.enabled ? "Memory on" : "Memory remains off" + }) + const setEnabledUnsafe = Effect.fn("Memory.setEnabledUnsafe")(function* (enabled: boolean) { const initial = yield* configuration() if (!initial) { @@ -827,13 +853,16 @@ export const layer: Layer.Layer< Effect.catchCause((cause) => Effect.gen(function* () { yield* Effect.logWarning("MEMORY command failed", { cause }) - return "Memory remains off" + // #397: a failure that statusReason can explain (e.g. no + // installed model to reselect) surfaces the actionable reason + // instead of a bare "remains off". + return (yield* statusReason()) ?? "Memory remains off" }), ), ), ) - return Service.of({ init, prepare, context, search, checkpoint, setEnabled, statusReason }) + return Service.of({ init, prepare, context, search, checkpoint, setEnabled, statusReason, status }) }), ) diff --git a/packages/opencode/src/memory/model.ts b/packages/opencode/src/memory/model.ts index f8d7c2285..f36241238 100644 --- a/packages/opencode/src/memory/model.ts +++ b/packages/opencode/src/memory/model.ts @@ -5,11 +5,13 @@ import { Context, Duration, Effect, Layer, Schema } from "effect" import { streamObject } from "ai" import { Provider } from "@/provider/provider" -// Liveness is judged per-chunk, never by a whole-call wall clock: a stream -// that keeps delivering parts is alive, however long the reasoning runs. -// CONNECT_TIMEOUT bounds the wait for the FIRST part; IDLE_TIMEOUT bounds the -// silence BETWEEN parts and is re-armed by every arriving part. Generation -// still terminates on its own via max_output_tokens / a natural stop. +// Liveness is judged per-part, never by a whole-call wall clock: a stream +// that keeps delivering parts is alive, however long the call runs. Two +// caveats: streamObject's fullStream DROPS reasoning-only parts (the ai SDK +// forwards text-delta/finish/error only), so a model that reasons silently +// past these windows still trips the timers — and CONNECT_TIMEOUT bounds the +// wait for the FIRST part while IDLE_TIMEOUT bounds the silence BETWEEN +// parts, re-armed by every arriving part. const CONNECT_TIMEOUT = Duration.seconds(60) const IDLE_TIMEOUT = Duration.seconds(60) @@ -37,7 +39,10 @@ export class GenerateError extends Schema.TaggedErrorClass()("Mem cause: Schema.Defect(), }) { override get message() { - return `MEMORY model call failed: ${String(this.cause)}` + // openai-compatible flattens a provider SSE error event to its bare + // message string, which can be empty — keep the failure identifiable. + const cause = String(this.cause) + return `MEMORY model call failed: ${cause === "" ? "(provider stream error with an empty message)" : cause}` } } @@ -76,8 +81,9 @@ function requireJsonToken(request: Request): Request { // Signals that the stream went silent past the liveness window. export class Stalled extends Error {} -// Drains `parts`, re-arming the idle watchdog on EVERY part (so a live stream -// that keeps delivering — reasoning deltas included — never trips the timer). +// Drains `parts`, re-arming the idle watchdog on every part the consumer +// sees. NOTE: for streamObject that excludes reasoning-only parts (they are +// filtered out upstream), so silent reasoning does NOT count as liveness. // Arms `connectTimeout` until the first part and `idleTimeout` between parts; // a silent window invokes `onStall` (abort the request) and fails with // `Stalled`, while an `errorOf` hit fails with that part's error. @@ -123,6 +129,48 @@ export const drainWithLiveness = (input: { })() }) +// Providers without structured-outputs support (every openai-compatible model +// today) downgrade response_format to bare {"type":"json_object"} and never +// see the schema passed to streamObject — the model then free-styles a +// different shape every call and client-side validation always rejects +// (issue #395). The schema therefore rides in the system prompt: the draft-07 +// document with every $ref inlined, since "#/definitions/..." pointers are +// meaningless to the model. Optional fields arrive as anyOf [T, null]; the +// null arm is dropped so the schema reads "provide T or omit the key", +// matching what the decoder actually accepts. +function jsonSchemaText(schema: Schema.Decoder) { + const root = Schema.toStandardJSONSchemaV1(schema)["~standard"].jsonSchema.input({ target: "draft-07" }) + const defs = { ...recordOf(root.definitions), ...recordOf(root.$defs) } + const walk = (node: unknown, refs: ReadonlySet): unknown => { + if (Array.isArray(node)) return node.map((item) => walk(item, refs)) + if (!isRecord(node)) return node + const ref = typeof node.$ref === "string" ? /^#\/(?:\$defs|definitions)\/(.+)$/.exec(node.$ref)?.[1] : undefined + if (ref !== undefined) { + if (refs.has(ref)) return {} + return walk(defs[ref] ?? {}, new Set([...refs, ref])) + } + if (Array.isArray(node.anyOf) && Object.keys(node).length === 1) { + const kept = node.anyOf.filter(isRecord).filter((arm) => arm.type !== "null") + if (kept.length === 1) return walk(kept[0], refs) + } + const out: Record = {} + for (const [key, value] of Object.entries(node)) { + if (key === "definitions" || key === "$defs" || key === "$id" || key === "$schema") continue + out[key] = walk(value, refs) + } + return out + } + return JSON.stringify(walk(root, new Set())) +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value) +} + +function recordOf(value: unknown): Record { + return isRecord(value) ? value : {} +} + const streamGenerate = (input: { language: Parameters[0]["model"] system: string @@ -132,8 +180,9 @@ const streamGenerate = (input: { maxOutputTokens: number connectTimeout: Duration.Duration idleTimeout: Duration.Duration -}) => - Effect.tryPromise({ +}): Effect.Effect => { + const system = `${input.system}\n\nThe response must be a single JSON object that validates against this JSON Schema:\n${jsonSchemaText(input.schema)}` + return Effect.tryPromise({ try: (signal) => (async () => { const controller = new AbortController() @@ -142,7 +191,7 @@ const streamGenerate = (input: { try { const result = streamObject({ model: input.language, - system: input.system, + system, prompt: input.prompt, schema: Object.assign( Schema.toStandardSchemaV1(input.schema), @@ -167,6 +216,7 @@ const streamGenerate = (input: { })(), catch: (cause) => (cause instanceof Stalled ? new TimeoutError() : new GenerateError({ cause })), }) +} export const layer = Layer.effect( Service, diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 2139e4412..ea7aa6db4 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -1887,12 +1887,14 @@ export const layer = Layer.effect( if (input.command === "memory") { const memory = Option.getOrUndefined(yield* Effect.serviceOption(Memory.Service)) const argument = input.arguments.trim() + // #396: anything that is not an exact on/off is a status query — + // report the true state instead of a hardcoded "remains off". const result = memory ? argument === "on" ? yield* memory.setEnabled(true) : argument === "off" ? yield* memory.setEnabled(false) - : "Memory remains off" + : yield* memory.status() : "Memory remains off" const model = yield* currentModel(input.sessionID) const agentName = input.agent ?? (yield* agents.defaultAgent()) diff --git a/packages/opencode/test/memory/memory.test.ts b/packages/opencode/test/memory/memory.test.ts index 9b711d389..f6e85eb5d 100644 --- a/packages/opencode/test/memory/memory.test.ts +++ b/packages/opencode/test/memory/memory.test.ts @@ -2128,6 +2128,113 @@ describe("memory enablement", () => { ) }) +function statusFixture() { + const replacement = ProviderTest.model({ + providerID: ProviderV2.ID.make("test"), + id: ModelV2.ID.make("replacement"), + }) + const state: { config: MemorySchema.Config; available: boolean } = { + config: { ...config, enabled: true, model: "removed/model" }, + available: true, + } + const providerLayer = Layer.mock(Provider.Service, { + list: () => + Effect.succeed( + state.available + ? { [replacement.providerID]: ProviderTest.info({ id: replacement.providerID, models: { [replacement.id]: replacement } }) } + : {}, + ), + getModel: (providerID, modelID) => + Effect.succeed( + ProviderTest.model({ + providerID, + id: modelID, + }), + ), + }) + const layer = Memory.layer.pipe( + Layer.provide( + Layer.mergeAll( + emptyConfigLayer, + EffectFlock.defaultLayer, + MemoryHome.defaultLayer, + MemoryIdentityFence.defaultLayer, + providerLayer, + Layer.mock(Project.Service, { + get: (id) => + Effect.succeed({ + id, + worktree: "/unused", + vcs: "git" as const, + time: { created: 0, updated: 0, initialized: 1 }, + sandboxes: [], + }), + }), + Layer.mock(MemoryConfig.Service, { + load: (directory) => + Effect.succeed({ config: state.config, path: directory, level: "project" as const }), + loadGlobal: () => Effect.succeed(undefined), + writeGlobal: () => Effect.succeed(true), + writeProject: () => Effect.void, + }), + readyAdmissionLayer, + MemoryLock.defaultLayer, + Layer.mock(MemoryModel.Service, { + generate: () => Effect.die(new Error("status surfaces must not call a model")), + }), + Layer.mock(MemoryStore.Service, { + readTopics: () => Effect.succeed([]), + }), + ), + ), + ) + return { state, it: testEffect(layer) } +} + +describe("memory status truthfulness (issues #396 #397)", () => { + const status = statusFixture() + + status.it.instance( + "reports why an enabled config is inert when its model is gone", + () => + Effect.gen(function* () { + const memory = yield* Memory.Service + const reason = yield* memory.statusReason() + if (reason === undefined) return yield* Effect.fail(new Error("expected a model-unavailability reason")) + expect(reason).toContain("model is unavailable") + expect(yield* memory.status()).toBe(reason) + expect(yield* memory.setEnabled(true)).toContain("model is unavailable") + }), + { git: true }, + ) + + status.it.instance( + "reports the true on/off state once the model resolves", + () => + Effect.gen(function* () { + const memory = yield* Memory.Service + status.state.config = { ...config, enabled: true, model: "test/replacement" } + expect(yield* memory.statusReason()).toBeUndefined() + expect(yield* memory.status()).toBe("Memory on") + status.state.config = { ...config, enabled: false, model: "test/replacement" } + expect(yield* memory.status()).toBe("Memory remains off") + }), + { git: true }, + ) + + status.it.instance( + "surfaces the model reason when /memory on cannot reselect any model", + () => + Effect.gen(function* () { + status.state.config = { ...config, enabled: true, model: "removed/model" } + status.state.available = false + const memory = yield* Memory.Service + expect(yield* memory.setEnabled(true)).toContain("model is unavailable") + }), + { git: true }, + ) +}) + function assistant( parentID: MessageID, sessionID: SessionID, diff --git a/packages/opencode/test/memory/model-wire.test.ts b/packages/opencode/test/memory/model-wire.test.ts new file mode 100644 index 000000000..9c8a3bed0 --- /dev/null +++ b/packages/opencode/test/memory/model-wire.test.ts @@ -0,0 +1,210 @@ +import { describe, expect } from "bun:test" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { SessionV1 } from "@opencode-ai/core/v1/session" +import { ModelV2 } from "@opencode-ai/core/model" +import { ProviderV2 } from "@opencode-ai/core/provider" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import fs from "node:fs/promises" +import path from "node:path" +import { Effect, Schema } from "effect" +import { InstanceState } from "@/effect/instance-state" +import { Memory } from "@/memory/memory" +import { MemoryModel } from "@/memory/model" +import { MemoryPrompts } from "@/memory/prompts" +import { MemorySchema } from "@/memory/schema" +import { MemoryStore } from "@/memory/store" +import { Project } from "@/project/project" +import { Provider } from "@/provider/provider" +import { MessageID, PartID, SessionID } from "@/session/schema" +import { provideTmpdirServer } from "../fixture/fixture" +import { pollWithTimeout, testEffect } from "../lib/effect" +import { raw, reply, TestLLMServer } from "../lib/llm-server" +import { testProviderConfig } from "../lib/test-provider" + +const ref = { providerID: ProviderV2.ID.make("test"), modelID: ModelV2.ID.make("test-model") } + +const memoryConfig = { + schema_version: 1, + enabled: true, + model: "test/test-model", + topic_limit: 10, + turn_interval: 5, + injection: { max_topics: 3, max_tokens: 1_200 }, +} satisfies MemorySchema.Config + +const createTopicReply = reply() + .text( + JSON.stringify({ + actions: [ + { + type: "create_topic", + name: "Reply style", + summary: "The user confirmed a durable preference for concise replies.", + categories: ["preference"], + keywords: ["replies", "concise"], + related_topics: [], + item: { + kind: "preference", + content: "User prefers concise replies.", + rationale: "The user confirmed this preference and it is long-term.", + }, + }, + ], + }), + ) + .stop() + +// #395 regression: openai-compatible providers downgrade response_format to +// bare {"type":"json_object"} and never receive the streamObject schema, so +// the model free-styles a fresh shape every call and validation rejects it. +// The matchers below only answer requests that visibly carry the schema, so +// every test in this file fails the moment the schema leaves the wire again. +const wireIt = testEffect( + LayerNode.buildLayer( + LayerNode.group([ + Provider.node, + MemoryModel.node, + CrossSpawnSpawner.node, + LayerNode.make(TestLLMServer.layer, []), + ]), + ), +) + +const wireGenerate = (schema: Schema.Decoder, system: string) => + Effect.gen(function* () { + const provider = yield* Provider.Service + const model = yield* provider.getModel(ref.providerID, ref.modelID) + return yield* (yield* MemoryModel.Service).generate({ + model, + system, + prompt: "User confirmed: replies stay concise.", + schema, + maxOutputTokens: 2_048, + }) + }) + +describe("memory model wire schema (issue #395)", () => { + wireIt.live("carries the maintenance schema on the wire so the model can conform", () => + provideTmpdirServer( + ({ llm }) => + Effect.gen(function* () { + yield* llm.pushMatch( + (hit) => JSON.stringify(hit.body).includes("create_topic"), + reply().text('{"actions":[{"type":"no_change"}]}').stop(), + ) + const result = yield* wireGenerate(MemorySchema.MaintenanceResponse, MemoryPrompts.MAINTAIN_SYSTEM) + expect(result).toEqual({ actions: [{ type: "no_change" }] }) + const inputs = yield* llm.inputs + const maintenance = inputs.find((input) => JSON.stringify(input.messages).includes("create_topic")) + expect(maintenance).toBeDefined() + expect(maintenance?.response_format).toEqual({ type: "json_object" }) + }), + { config: (url) => testProviderConfig(url) }, + ), + ) + + wireIt.live("keeps a provider error with an empty message legible", () => + provideTmpdirServer( + ({ llm }) => + Effect.gen(function* () { + yield* llm.push(raw({ head: [{ error: { message: "" } }] })) + const failure = yield* wireGenerate(MemorySchema.MaintenanceResponse, MemoryPrompts.MAINTAIN_SYSTEM).pipe( + Effect.flip, + ) + if (!(failure instanceof MemoryModel.GenerateError)) + return yield* Effect.fail(new Error(`expected GenerateError, got: ${String(failure)}`)) + expect(failure.message).toBe("MEMORY model call failed: (provider stream error with an empty message)") + }), + { config: (url) => testProviderConfig(url) }, + ), + ) +}) + +const stackIt = testEffect( + LayerNode.buildLayer( + LayerNode.group([ + Memory.node, + Project.node, + MemoryStore.node, + CrossSpawnSpawner.node, + LayerNode.make(TestLLMServer.layer, []), + ]), + ), +) + +describe("memory maintenance end to end (issue #395)", () => { + stackIt.live("creates a topic when the wire schema lets the model conform", () => + provideTmpdirServer( + ({ dir, llm }) => + Effect.gen(function* () { + const project = yield* Project.Service + const registered = yield* project.fromDirectory(dir) + yield* project.setInitialized(registered.project.id) + const configDir = path.join(dir, ".opencode") + yield* Effect.promise(() => fs.mkdir(configDir, { recursive: true })) + yield* Effect.promise(() => fs.writeFile(path.join(configDir, "memory.jsonc"), JSON.stringify(memoryConfig))) + + yield* llm.pushMatch( + (hit) => JSON.stringify(hit.body).includes("topic_ids"), + reply().text('{"topic_ids":[]}').stop(), + ) + yield* llm.pushMatch((hit) => JSON.stringify(hit.body).includes("create_topic"), createTopicReply) + + const sessionID = SessionID.make("ses_memory_wire") + const userID = MessageID.ascending() + const messages: SessionV1.WithParts[] = [ + { + info: { + id: userID, + role: "user", + sessionID, + time: { created: Date.now() }, + agent: "build", + model: ref, + }, + parts: [ + { + id: PartID.ascending(), + messageID: userID, + sessionID, + type: "text", + text: "以后回复保持简洁，这点长期有效", + }, + ], + }, + { + info: { + id: MessageID.ascending(), + role: "assistant", + sessionID, + parentID: userID, + mode: "build", + agent: "build", + path: { cwd: dir, root: dir }, + cost: 0, + tokens: { total: 0, input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + providerID: ref.providerID, + modelID: ref.modelID, + time: { created: Date.now() }, + finish: "end_turn", + }, + parts: [], + }, + ] + + yield* (yield* Memory.Service).checkpoint({ sessionID, messages }) + + const store = yield* MemoryStore.Service + const projectID = (yield* InstanceState.context).project.id + const topics = yield* pollWithTimeout( + Effect.suspend(() => store.readTopics(projectID)).pipe( + Effect.map((all) => (all.length > 0 ? all : undefined)), + ), + "maintenance never committed a topic", + ) + expect(topics[0]?.metadata.categories).toEqual(["preference"]) + }), + { config: (url) => testProviderConfig(url), git: true }, + ), + ) +}) diff --git a/packages/opencode/test/session/prompt.test.ts b/packages/opencode/test/session/prompt.test.ts index 0139cc10c..f181c97f0 100644 --- a/packages/opencode/test/session/prompt.test.ts +++ b/packages/opencode/test/session/prompt.test.ts @@ -248,6 +248,8 @@ function makePrompt(input?: PromptLayerOptions) { context: () => Effect.succeed(input?.memoryContext ?? []), checkpoint: () => Effect.succeed(input?.memoryContext ?? []), setEnabled: (enabled) => Effect.succeed(enabled ? ("Memory on" as const) : ("Memory off" as const)), + statusReason: () => Effect.succeed(undefined), + status: () => Effect.succeed("Memory on"), }) const deps = Layer.mergeAll( hookRecorderLayer, @@ -2372,12 +2374,15 @@ it.instance("stores the slash invocation as visible text and hides the expanded }), ) -noLLMServer.instance("dispatches /memory on and off without running a model turn", () => +noLLMServer.instance("dispatches /memory on, off, and status without running a model turn", () => Effect.gen(function* () { const { prompt, sessions, chat } = yield* boot() const off = yield* prompt.command({ sessionID: chat.id, command: "memory", arguments: "off" }) const on = yield* prompt.command({ sessionID: chat.id, command: "memory", arguments: "on" }) + // #396: a non-on/off argument is a status query — the reply must reflect + // the service's true state instead of the old hardcoded "remains off". + const status = yield* prompt.command({ sessionID: chat.id, command: "memory", arguments: "" }) const unsupported = yield* prompt.command({ sessionID: chat.id, command: "memory", arguments: "topic 20" }) expect(off.parts.filter((part) => part.type === "text").map((part) => part.text)).toEqual([ @@ -2388,9 +2393,13 @@ noLLMServer.instance("dispatches /memory on and off without running a model turn "/memory on", "Memory on", ]) + expect(status.parts.filter((part) => part.type === "text").map((part) => part.text)).toEqual([ + "/memory", + "Memory on", + ]) expect(unsupported.parts.filter((part) => part.type === "text").map((part) => part.text)).toEqual([ "/memory topic 20", - "Memory remains off", + "Memory on", ]) expect((yield* sessions.messages({ sessionID: chat.id })).every((message) => message.info.role === "user")).toBe(true) }),