From 2e5785eddd464af94c33dfd6283d5eb3ac481284 Mon Sep 17 00:00:00 2001 From: nanw Date: Mon, 6 Jul 2026 06:00:19 +0800 Subject: [PATCH] fix(opencode): handle stale session.directory gracefully Fixes three related issues caused by stale session.directory values stored in the database after a project directory is moved or deleted: - #35427: HTTP 500 from POST /session/{id}/command when session.directory points to a missing directory. SystemPrompt.environment() propagated a Die defect that bypassed the BadRequest handler. - #33909: CLI opencode run --session --dir hangs at 'loop step=0'. The --dir flag was ignored because session loading always used session.directory from the DB. - #23248: Sessions under project_id=global with old directory paths become orphaned when the project directory is renamed. Changes: - session/system.ts: wrap references computation in catchCause so a failing location layer (ENOENT) degrades to empty references instead of crashing - project/project.ts: add relinkStaleSessions in fromDirectory() that updates session.directory for sessions under the same project whose stored directory no longer exists on disk - handlers/session.ts: add directory existence check in requireSession that returns BadRequest for sessions with stale directories - groups/session.ts: add HttpApiError.BadRequest to share/unshare error schemas - Add regression tests in test/session/system.test.ts (catchCause), test/project/migrate-global.test.ts (relinkStaleSessions), and test/server/httpapi-session.test.ts (requireSession BadRequest) --- packages/opencode/src/project/project.ts | 46 +++++++++++ .../routes/instance/httpapi/groups/session.ts | 4 +- .../instance/httpapi/handlers/session.ts | 23 +++++- packages/opencode/src/session/system.ts | 5 +- .../test/project/migrate-global.test.ts | 77 +++++++++++++++++++ .../test/server/httpapi-session.test.ts | 27 +++++++ packages/opencode/test/session/system.test.ts | 73 +++++++++++++++++- 7 files changed, 248 insertions(+), 7 deletions(-) diff --git a/packages/opencode/src/project/project.ts b/packages/opencode/src/project/project.ts index 9870377b2169..01efb4fc6cd5 100644 --- a/packages/opencode/src/project/project.ts +++ b/packages/opencode/src/project/project.ts @@ -210,6 +210,44 @@ const layer = Layer.effect( ) }) + // When a project is reopened at a new path, sessions whose stored + // `directory` no longer exists on disk are relinked to the new path. This + // is best-effort: only sessions under the same project_id are touched, and + // only when the old directory is unreachable. Sessions in intentional + // sandboxes or alternate roots are left alone. + const relinkStaleSessions = Effect.fn("Project.relinkStaleSessions")(function* (input: { + projectID: ProjectV2.ID + currentDirectory: string + }) { + const rows = yield* db + .select({ id: SessionTable.id, directory: SessionTable.directory }) + .from(SessionTable) + .where(eq(SessionTable.project_id, input.projectID)) + .all() + .pipe(Effect.orDie) + + let relinked = 0 + for (const row of rows) { + if (row.directory === input.currentDirectory) continue + const exists = yield* fs.existsSafe(row.directory) + if (exists) continue + yield* db + .update(SessionTable) + .set({ directory: input.currentDirectory, time_updated: Date.now() }) + .where(eq(SessionTable.id, row.id)) + .run() + .pipe(Effect.orDie) + relinked++ + } + if (relinked > 0) { + yield* Effect.logInfo("relinked stale sessions to current project directory", { + projectID: input.projectID, + currentDirectory: input.currentDirectory, + count: relinked, + }) + } + }) + const fromDirectory = Effect.fn("Project.fromDirectory")(function* (directory: string) { yield* Effect.logInfo("fromDirectory", { directory }) @@ -295,6 +333,14 @@ const layer = Layer.effect( .where(and(eq(SessionTable.project_id, ProjectV2.ID.global), eq(SessionTable.directory, data.directory))) .run() .pipe(Effect.orDie) + + // Auto-relink stale sessions: when a project's worktree moves, sessions + // whose stored `directory` no longer exists on disk are updated to the + // new worktree. This addresses issue #23248 (orphaned sessions after a + // project directory rename) by surfacing the session under the new + // project path. Sessions whose directory still exists (intentional + // sandboxes, alternate roots) are left untouched. + yield* relinkStaleSessions({ projectID, currentDirectory: data.directory }) } yield* saveProjectDirectory({ diff --git a/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts index 959a303dc964..556d6d6ccd19 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts @@ -280,7 +280,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, query: WorkspaceRoutingQuery, success: described(Session.Info, "Successfully shared session"), - error: [HttpApiError.InternalServerError, ApiNotFoundError], + error: [HttpApiError.InternalServerError, HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.share", @@ -292,7 +292,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, query: WorkspaceRoutingQuery, success: described(Session.Info, "Successfully unshared session"), - error: [HttpApiError.InternalServerError, ApiNotFoundError], + error: [HttpApiError.InternalServerError, HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.unshare", diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts index 662585020a64..b7ab0d73c8db 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts @@ -4,6 +4,7 @@ import { SessionV1 } from "@opencode-ai/core/v1/session" import { EventV2Bridge } from "@/event-v2-bridge" import { Command } from "@/command" import { Permission } from "@/permission" +import { FSUtil } from "@opencode-ai/core/fs-util" import { SessionShare } from "@/share/session" import { Session } from "@/session/session" import { SessionCompaction } from "@/session/compaction" @@ -59,6 +60,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", const todoSvc = yield* Todo.Service const summary = yield* SessionSummary.Service const events = yield* EventV2Bridge.Service + const fs = yield* FSUtil.Service const scope = yield* Scope.Scope const list = Effect.fn("SessionHttpApi.list")(function* (ctx: { query: typeof ListQuery.Type }) { @@ -79,7 +81,22 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", }) const requireSession = Effect.fn("SessionHttpApi.requireSession")(function* (sessionID: SessionID) { - return yield* SessionError.mapStorageNotFound(session.get(sessionID)) + const sess = yield* SessionError.mapStorageNotFound(session.get(sessionID)) + // Fail fast when the session's stored directory is unreachable. Without + // this check, downstream services (SystemPrompt.environment, file + // watcher, snapshot tracking) crash with internal errors because the + // Location services can't boot a FileSystem rooted at a missing path. + // Surfaces issues #33909 (silent hang) and #35427 (HTTP 500) clearly to + // the user instead of letting the request die mid-flight. + const exists = yield* fs.existsSafe(sess.directory) + if (!exists) { + yield* Effect.logWarning("session directory no longer exists", { + sessionID, + directory: sess.directory, + }) + return yield* new HttpApiError.BadRequest({}) + } + return sess }) const get = Effect.fn("SessionHttpApi.get")(function* (ctx: { params: { sessionID: SessionID } }) { @@ -380,7 +397,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", const deleteMessage = Effect.fn("SessionHttpApi.deleteMessage")(function* (ctx: { params: { sessionID: SessionID; messageID: MessageID } }) { - yield* requireSession(ctx.params.sessionID) + yield* requireSession(ctx.params.sessionID).pipe(Effect.asVoid) yield* SessionError.mapBusy(runState.assertNotBusy(ctx.params.sessionID)) yield* session.removeMessage(ctx.params) return true @@ -389,7 +406,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", const deletePart = Effect.fn("SessionHttpApi.deletePart")(function* (ctx: { params: { sessionID: SessionID; messageID: MessageID; partID: PartID } }) { - yield* requireSession(ctx.params.sessionID) + yield* requireSession(ctx.params.sessionID).pipe(Effect.asVoid) yield* session.removePart(ctx.params) return true }) diff --git a/packages/opencode/src/session/system.ts b/packages/opencode/src/session/system.ts index 6e5b83ec31f9..f2bd22b311ff 100644 --- a/packages/opencode/src/session/system.ts +++ b/packages/opencode/src/session/system.ts @@ -59,7 +59,10 @@ const layer = Layer.effect( const ctx = yield* InstanceState.context const references = yield* Effect.gen(function* () { return (yield* (yield* Reference.Service).list()).filter((reference) => reference.description !== undefined) - }).pipe(Effect.provide(locations.get(Location.Ref.make({ directory: AbsolutePath.make(ctx.directory) })))) + }).pipe( + Effect.provide(locations.get(Location.Ref.make({ directory: AbsolutePath.make(ctx.directory) }))), + Effect.catchCause(() => Effect.succeed([])), + ) return [ [ `You are powered by the model named ${model.api.id}. The exact model ID is ${model.providerID}/${model.api.id}`, diff --git a/packages/opencode/test/project/migrate-global.test.ts b/packages/opencode/test/project/migrate-global.test.ts index 24d8aaaaaf66..e7fd2b26fffc 100644 --- a/packages/opencode/test/project/migrate-global.test.ts +++ b/packages/opencode/test/project/migrate-global.test.ts @@ -166,3 +166,80 @@ describe("migrateFromGlobal", () => { }), ) }) + +// Regression test for #23248: when a project's worktree moves, sessions under +// the same project_id whose stored directory no longer exists on disk should +// be auto-relinked to the new worktree path. +describe("relinkStaleSessions", () => { + it.live("relinks sessions whose directory no longer exists", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped({ git: true }) + const projects = yield* Project.Service + const { project } = yield* projects.fromDirectory(tmp) + expect(project.id).not.toBe(ProjectV2.ID.global) + + // Seed a session under the real project_id pointing at a directory + // that does not exist on disk (simulating a moved/renamed project). + const id = legacySessionID() + yield* seed({ id, dir: "/nonexistent/old/path", project: project.id }) + + yield* projects.fromDirectory(tmp) + + const row = yield* Database.Service.use(({ db }) => + db.select().from(SessionTable).where(eq(SessionTable.id, id)).get().pipe(Effect.orDie), + ) + expect(row).toBeDefined() + expect(row!.directory).toBe(tmp) + expect(row!.project_id).toBe(project.id) + }), + ) + + it.live("does not relink sessions whose directory still exists", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped({ git: true }) + const projects = yield* Project.Service + const { project } = yield* projects.fromDirectory(tmp) + expect(project.id).not.toBe(ProjectV2.ID.global) + + // Create an alternate sandbox directory that DOES exist on disk. + const sandbox = yield* tmpdirScoped() + const id = legacySessionID() + yield* seed({ id, dir: sandbox, project: project.id }) + + yield* projects.fromDirectory(tmp) + + // Directory should be untouched — intentional alternate root. + const row = yield* Database.Service.use(({ db }) => + db.select().from(SessionTable).where(eq(SessionTable.id, id)).get().pipe(Effect.orDie), + ) + expect(row).toBeDefined() + expect(row!.directory).toBe(sandbox) + }), + ) + + it.live("only relinks sessions under the matching project_id", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped({ git: true }) + const projects = yield* Project.Service + const { project } = yield* projects.fromDirectory(tmp) + expect(project.id).not.toBe(ProjectV2.ID.global) + + yield* ensureGlobal() + + // Seed a session under "global" whose directory doesn't exist. It must + // not be touched by this project's relink pass (it belongs to a + // different project). + const otherId = legacySessionID() + yield* seed({ id: otherId, dir: "/nonexistent/other", project: ProjectV2.ID.global }) + + yield* projects.fromDirectory(tmp) + + const row = yield* Database.Service.use(({ db }) => + db.select().from(SessionTable).where(eq(SessionTable.id, otherId)).get().pipe(Effect.orDie), + ) + expect(row).toBeDefined() + expect(row!.directory).toBe("/nonexistent/other") + expect(row!.project_id).toBe(ProjectV2.ID.global) + }), + ) +}) diff --git a/packages/opencode/test/server/httpapi-session.test.ts b/packages/opencode/test/server/httpapi-session.test.ts index 9d7643cb3309..74b12e09f743 100644 --- a/packages/opencode/test/server/httpapi-session.test.ts +++ b/packages/opencode/test/server/httpapi-session.test.ts @@ -207,6 +207,12 @@ const clearSessionPath = (sessionID: SessionIDType) => yield* db.update(SessionTable).set({ path: null }).where(eq(SessionTable.id, sessionID)).run().pipe(Effect.orDie) }) +const setSessionDirectory = (sessionID: SessionIDType, directory: string) => + Effect.gen(function* () { + const { db } = yield* Database.Service + yield* db.update(SessionTable).set({ directory }).where(eq(SessionTable.id, sessionID)).run().pipe(Effect.orDie) + }) + function request(path: string, init?: RequestInit) { const url = new URL(path, "http://localhost") return HttpClientRequest.fromWeb(new Request(url, init)).pipe( @@ -1087,4 +1093,25 @@ describe("session HttpApi", () => { }), { git: true, config: { formatter: false, lsp: false } }, ) + + // Regression test for #33909 (silent hang) and #35427 (HTTP 500): a session + // whose stored directory no longer exists on disk must fail fast with a + // clear BadRequest at the handler boundary instead of crashing mid-request + // while downstream services try to boot a FileSystem at the missing path. + it.instance( + "returns 400 when session.directory no longer exists", + () => + Effect.gen(function* () { + const test = yield* TestInstance + const headers = { "x-opencode-directory": test.directory } + const session = yield* createSession({ title: "stale" }) + + // Move the session's directory column to a path that does not exist. + yield* setSessionDirectory(session.id, "/nonexistent/path/that/does/not/exist") + + const get = yield* request(pathFor(SessionPaths.get, { sessionID: session.id }), { headers }) + expect(get.status).toBe(400) + }), + { git: true, config: { formatter: false, lsp: false } }, + ) }) diff --git a/packages/opencode/test/session/system.test.ts b/packages/opencode/test/session/system.test.ts index 1484bfd631a3..d0e1edff6539 100644 --- a/packages/opencode/test/session/system.test.ts +++ b/packages/opencode/test/session/system.test.ts @@ -1,11 +1,15 @@ import { describe, expect } from "bun:test" import { LayerNode } from "@opencode-ai/core/effect/layer-node" -import { Effect, Layer } from "effect" +import { Effect, Layer, LayerMap } from "effect" +import { Location } from "@opencode-ai/core/location" +import { LocationServiceMap } from "@opencode-ai/core/location-service-map" +import type { LocationError, LocationServices } from "@opencode-ai/core/location-services" import type { Agent } from "../../src/agent/agent" import { NamedError } from "@opencode-ai/core/util/error" import { Skill } from "../../src/skill" import { Permission } from "../../src/permission" import { SystemPrompt } from "../../src/session/system" +import { Provider } from "../../src/provider/provider" import { MCP } from "../../src/mcp" import { testEffect } from "../lib/effect" @@ -140,3 +144,70 @@ describe("session.system", () => { }), ) }) + +// Regression test for #35427: SystemPrompt.environment() must not crash the +// request when the location-services layer fails to boot (e.g. when a session +// references a directory that no longer exists). The references block should +// degrade to empty rather than propagate the Die defect. +describe("session.system (degraded location layer)", () => { + // Build a LocationServiceMap layer where get(ref) returns a layer that fails + // to construct. This mimics what happens when a stored session directory no + // longer exists on disk — the location-service layer boots and hits ENOENT. + // A minimal LayerMap that makes get() always return a layer that dies. + // The Die defect propagates through the catchCause in environment(). + const failingLocationsLayer = Layer.effect( + LocationServiceMap.Service, + Effect.gen(function* () { + return yield* LayerMap.make( + (_ref: Location.Ref) => + Layer.effect( + Location.Service, + Effect.fail(new Error("simulated location layer boot failure")), + ), + { idleTimeToLive: "1 minute" }, + ) + }) as unknown as Effect.Effect>, + ) + + const it2 = testEffect( + LayerNode.compile(SystemPrompt.node, [ + [LocationServiceMap.node, failingLocationsLayer], + [ + MCP.node, + Layer.mock(MCP.Service, { + instructions: () => Effect.succeed([]), + }), + ], + [ + Skill.node, + Layer.succeed( + Skill.Service, + Skill.Service.of({ + get: (name) => Effect.succeed(skills.find((skill) => skill.name === name)), + require: (name) => { + const info = skills.find((skill) => skill.name === name) + if (info) return Effect.succeed(info) + return Effect.fail(new Skill.NotFoundError({ name, available: skills.map((skill) => skill.name) })) + }, + all: () => Effect.succeed(skills), + dirs: () => Effect.succeed([]), + available: () => Effect.succeed(skills), + }), + ), + ], + ]), + ) + + it2.instance("environment() succeeds when location layer fails to boot", () => + Effect.gen(function* () { + const prompt = yield* SystemPrompt.Service + // Cast: only providerID and api.id are read by environment() + const model = { providerID: "test", api: { id: "test-model" } } as unknown as Provider.Model + const parts = yield* prompt.environment(model) + + // Should still produce the env block, but skip references gracefully. + expect(parts.some((p) => p.includes(""))).toBe(true) + expect(parts.some((p) => p.includes(""))).toBe(false) + }), + ) +})