fix(session): accept directory when creating sessions#21131
fix(session): accept directory when creating sessions#21131EZotoff wants to merge 3 commits intoanomalyco:devfrom
Conversation
github-actions
bot
added
the
needs:compliance
|
The following comment was made by an LLM, it may be inaccurate: Based on my search, I found the following related PRs: Potentially Related:
Other Potentially Relevant:
The current PR (21131) appears to be addressing issue #12918 and is a narrow, backward-compatible fix that enables the |
There was a problem hiding this comment.
Pull request overview
This PR makes session creation directory-aware by allowing callers to override the working directory at create time, aligning POST /session with existing directory-based session listing behavior and enabling correct session directory assignment when switching projects/worktrees.
Changes:
- Add optional
directoryparameter toSession.create(schema + service implementation) and persist it to the session. - Extend
POST /sessionto accept?directory=...and forward it intoSession.create. - Add a regression test covering the route-level directory override.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/opencode/src/session/index.ts | Adds optional directory support to the Session.create API and schema. |
| packages/opencode/src/server/routes/session.ts | Accepts directory via query params for session creation and passes it through. |
| packages/opencode/test/server/session-actions.test.ts | Adds a test verifying the create route honors ?directory=.... |
Comments suppressed due to low confidence (1)
packages/opencode/src/session/index.ts:507
Session.createnow accepts a caller-supplieddirectoryand persists it as-is. Unlike the default path (fromInstanceState.directory), this can be relative, contain..segments, or be an empty string, which risks creating sessions with a non-canonical/invalid working directory and later tool execution or path checks behaving inconsistently. Consider normalizing/validatinginput.directory(e.g., resolve to an absolute canonical path and reject empty strings) before passing it tocreateNext.
const create = Effect.fn("Session.create")(function* (input?: {
directory?: string
parentID?: SessionID
title?: string
permission?: Permission.Ruleset
workspaceID?: WorkspaceID
}) {
const directory = input?.directory ?? (yield* InstanceState.directory)
return yield* createNext({
parentID: input?.parentID,
directory,
title: input?.title,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| validator("query", z.object({ directory: z.string().optional() }).optional()), | ||
| async (c) => { | ||
| const body = c.req.valid("json") ?? {} | ||
| const session = await Session.create(body) | ||
| const query = c.req.valid("query") | ||
| const session = await Session.create({ ...body, ...query }) |
There was a problem hiding this comment.
The new route-level directory override is persisted to the session, but the value is taken directly from the query string rather than the already-canonicalized Instance.directory that WorkspaceRouterMiddleware computes for routing. This can lead to sessions storing a different path representation than the active instance (e.g., relative paths, unnormalized casing on Windows, or paths with ..). Prefer resolving the incoming directory (or using Instance.directory) before passing it to Session.create.
| validator("query", z.object({ directory: z.string().optional() }).optional()), | ||
| async (c) => { | ||
| const body = c.req.valid("json") ?? {} | ||
| const session = await Session.create(body) | ||
| const query = c.req.valid("query") | ||
| const session = await Session.create({ ...body, ...query }) |
There was a problem hiding this comment.
The query validator is defined as z.object({ directory: z.string().optional() }).optional(). Since the object already has only-optional fields, making the entire query object optional is redundant and makes c.req.valid("query") typed as possibly undefined, which complicates downstream handling. Prefer a non-optional object schema here (consistent with the other query validators in this file) and treat the validated query as { directory?: string }.
github-actions
bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
Addressed the review points in follow-up commit
Validation:
|
|
I tracked the Windows-only failure down to the test assertion rather than the route implementation. On Windows, Pushed follow-up commit
Validation:
|
Issue for this PR
Closes #12918
Type of change
What does this PR do?
This makes session creation honor an explicit directory override.
Specifically:
Session.createnow accepts an optionaldirectoryPOST /sessionnow accepts?directory=...and forwards it into session creationFilesystem.resolve(...)before being persistedWhy this works:
createNextalready persists an explicit directorySession.createand the create routeInstanceState.directorywhen no override is providedThis is intentionally narrower than #9365, which tackles broader per-session working-directory behavior across the system.
How did you verify your code works?
bun test ./test/server/session-actions.test.tsfrompackages/opencodeScreenshots / recordings
Not a UI change.
Checklist