-
Notifications
You must be signed in to change notification settings - Fork 0
feat(workspace): multi-directory Location.Ref + workspace-aware prompt (opencode#215, draft) #219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: local/amicode
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { Effect } from "effect" | ||
| import type { DatabaseMigration } from "../migration" | ||
|
|
||
| export default { | ||
| id: "20260820000001_add_session_directories", | ||
| up(tx) { | ||
| return Effect.gen(function* () { | ||
| yield* tx.run(`ALTER TABLE session ADD COLUMN directories TEXT;`) | ||
| }) | ||
| }, | ||
| } satisfies DatabaseMigration.Migration |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ export const SessionTable = sqliteTable( | |
| parent_id: text().$type<SessionSchema.ID>(), | ||
| slug: text().notNull(), | ||
| directory: DatabasePath.directoryColumn().notNull(), | ||
| directories: text({ mode: "json" }).$type<string[]>(), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Persist and hydrate The new column is never written by A session with multiple workspace folders will lose them after persistence. Add the serialization mapping and reconstruct 🤖 Prompt for AI Agents |
||
| path: DatabasePath.pathColumn(), | ||
| title: text().notNull(), | ||
| version: text().notNull(), | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,9 +13,14 @@ const builtIns = Layer.effectDiscard( | |||||||||||||||||||||||||||
| Effect.gen(function* () { | ||||||||||||||||||||||||||||
| const location = yield* Location.Service | ||||||||||||||||||||||||||||
| const registry = yield* SystemContextRegistry.Service | ||||||||||||||||||||||||||||
| const workspaceFolders = location.directories?.length ? location.directories : [location.directory] | ||||||||||||||||||||||||||||
| const workspaceEnv = | ||||||||||||||||||||||||||||
| workspaceFolders.length > 1 | ||||||||||||||||||||||||||||
| ? ` Working directory: ${location.directory}\n Workspace folders:\n${workspaceFolders.map((d, i) => ` - ${d}${i === 0 ? " (primary)" : ""}`).join("\n")}` | ||||||||||||||||||||||||||||
| : ` Working directory: ${location.directory}` | ||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Normalize workspace folders before marking the primary folder. This code assumes Build Proposed fix- const workspaceFolders = location.directories?.length ? location.directories : [location.directory]
+ const workspaceFolders = [
+ location.directory,
+ ...(location.directories ?? []).filter((directory) => directory !== location.directory),
+ ]
const workspaceEnv =
workspaceFolders.length > 1
- ? ` Working directory: ${location.directory}\n Workspace folders:\n${workspaceFolders.map((d, i) => ` - ${d}${i === 0 ? " (primary)" : ""}`).join("\n")}`
+ ? ` Working directory: ${location.directory}\n Workspace folders:\n${workspaceFolders.map((d) => ` - ${d}${d === location.directory ? " (primary)" : ""}`).join("\n")}`📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| const environment = [ | ||||||||||||||||||||||||||||
| "<env>", | ||||||||||||||||||||||||||||
| ` Working directory: ${location.directory}`, | ||||||||||||||||||||||||||||
| workspaceEnv, | ||||||||||||||||||||||||||||
| ` Workspace root folder: ${location.project.directory}`, | ||||||||||||||||||||||||||||
| ` Is directory a git repo: ${location.vcs?.type === "git" ? "yes" : "no"}`, | ||||||||||||||||||||||||||||
| ` Platform: ${process.platform}`, | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ import * as Project from "./project" | |
|
|
||
| export interface LoadInput { | ||
| directory: string | ||
| directories?: string[] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift Include workspace directories in the instance identity.
Use a normalized workspace-directory set in the cache identity, or isolate the context per session. This prevents instruction and workspace state from leaking between sessions. Also applies to: 52-59 🤖 Prompt for AI Agents |
||
| worktree?: string | ||
| project?: Project.Info | ||
| } | ||
|
|
@@ -48,12 +49,14 @@ const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Ser | |
| input.project && input.worktree | ||
| ? { | ||
| directory: input.directory, | ||
| directories: input.directories, | ||
| worktree: input.worktree, | ||
| project: input.project, | ||
| } | ||
| : yield* project.fromDirectory(input.directory).pipe( | ||
| Effect.map((result) => ({ | ||
| directory: input.directory, | ||
| directories: input.directories, | ||
| worktree: result.sandbox, | ||
| project: result.project, | ||
| })), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,16 +119,26 @@ const layer: Layer.Layer< | |
| } | ||
| } | ||
|
|
||
| // The first project-level match wins so we don't stack AGENTS.md/CLAUDE.md from every ancestor. | ||
| // Multi-root: run findUp from each workspace directory, dedup by resolved path. | ||
| // Primary first, then remaining in workspace order; each walks to its own worktree root. | ||
| if (!Flag.OPENCODE_DISABLE_PROJECT_CONFIG) { | ||
| const allDirs: string[] = (ctx as unknown as { directories?: string[] }).directories?.length | ||
| ? (ctx as unknown as { directories: string[] }).directories | ||
| : [ctx.directory] | ||
| // Preserve order but ensure primary is first | ||
| const ordered = allDirs[0] === ctx.directory ? allDirs : [ctx.directory, ...allDirs.filter((d) => d !== ctx.directory)] | ||
| for (const file of instructionFiles) { | ||
| const matches = yield* fs | ||
| .findUp(file, ctx.directory, ctx.worktree) | ||
| .pipe(Effect.catch(() => Effect.succeed([]))) | ||
| if (matches.length > 0) { | ||
| matches.forEach((item) => paths.add(path.resolve(item))) | ||
| break | ||
| let foundAny = false | ||
| for (const dir of ordered) { | ||
| const matches = yield* fs | ||
| .findUp(file, dir, ctx.worktree) | ||
| .pipe(Effect.catch(() => Effect.succeed([]))) | ||
| if (matches.length > 0) { | ||
| matches.forEach((item) => paths.add(path.resolve(item))) | ||
| foundAny = true | ||
| } | ||
| } | ||
| if (foundAny) break | ||
|
Comment on lines
+122
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Use the correct root for each workspace directory. Lines 133-135 pass the primary Also update Store or derive a root for each workspace directory and use the matching root in both paths. 🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Canonicalize every workspace root before the escape check.
locationRootis canonical, but each value inlocation.directoriesis not. If a secondary workspace folder is a symlink, a path below that folder can pass the lexical check and then fail Line 135 afterresolvePath()returns its real path.Resolve the configured workspace directories once during layer initialization. Use those canonical roots for the canonical containment check.
🤖 Prompt for AI Agents