From 5d2cfda3bca8222bfff0467414c4ef1fb6e4640d Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Sun, 6 Sep 2026 11:56:16 +0530 Subject: [PATCH 1/2] fix(workspace): bound in-memory workspace cache --- docs/chatgpt-coding-workflow.md | 3 +- docs/gotchas.md | 5 +-- src/server.ts | 5 ++- src/skills.test.ts | 10 ++---- src/skills.ts | 14 ++------- src/workspaces.test.ts | 56 +++++++++++++++++++++++++++++++++ src/workspaces.ts | 31 ++++++++++-------- 7 files changed, 84 insertions(+), 40 deletions(-) diff --git a/docs/chatgpt-coding-workflow.md b/docs/chatgpt-coding-workflow.md index f6826617c..a90f3b859 100644 --- a/docs/chatgpt-coding-workflow.md +++ b/docs/chatgpt-coding-workflow.md @@ -137,8 +137,7 @@ advertised `SKILL.md` before following that skill. Skill paths may be outside the workspace. DevSpace only permits reading: -- advertised `SKILL.md` files -- files under a skill directory after that skill's `SKILL.md` has been read +- files within advertised skill directories Set `skills.enabled` to `false` to hide skills from workspace output. Enable Subagents and choose providers through `devspace init` or the persisted provider diff --git a/docs/gotchas.md b/docs/gotchas.md index 3bf7dd2d8..842cf301f 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -252,8 +252,9 @@ Copy or adapt them into one of the active profile directories before use. Legacy project paths such as `.pi/skills` can be added to `skills.paths` when needed. -If a skill appears in `open_workspace`, the model must read that skill's -`SKILL.md` before reading other files inside the skill directory. +If a skill appears in `open_workspace`, the model should read that skill's +`SKILL.md` before following it. DevSpace permits reads within advertised skill +directories without tracking whether `SKILL.md` was read first. ## Review Card Does Not Appear diff --git a/src/server.ts b/src/server.ts index 8945d303d..03982a866 100644 --- a/src/server.ts +++ b/src/server.ts @@ -130,7 +130,7 @@ function serverInstructions( const showChangesInstruction = " If the turn successfully modifies files by creating, editing, overwriting, deleting, moving, or applying patches, call show_changes exactly once for that workspace after the final related file change and before your final response so the user can inspect the aggregate diff for that turn. Do not call it after every individual file change."; const skills = config.skillsEnabled - ? `When ${toolNames.openWorkspace} returns available skills and a task matches a skill, use ${toolNames.read} to read that skill's path before proceeding. Skill paths may be outside the workspace, but ${toolNames.read} only permits advertised SKILL.md files and files under already-loaded skill directories. ` + ? `When ${toolNames.openWorkspace} returns available skills and a task matches a skill, use ${toolNames.read} to read that skill's path before proceeding. Skill paths may be outside the workspace, and ${toolNames.read} permits files within advertised skill directories. ` : ""; const agents = `Follow instructions returned by ${toolNames.openWorkspace}. Before working under a path listed in availableAgentsFiles, use ${toolNames.read} to inspect that instruction file and follow it. `; const common = `Use DevSpace for coding work. Call ${toolNames.openWorkspace} once for each project folder or isolated worktree, then keep using its workspaceId. During continued work in the same project or worktree, do not call ${toolNames.openWorkspace} again. Open another workspace only when changing projects, switching checkout/worktree mode, creating another isolated worktree, or when the current workspaceId is rejected.`; @@ -597,7 +597,7 @@ function registerMcpSurface( "Read a file in a workspace. Use this for file inspection instead of shell commands like cat or sed.", "Use this tool to inspect relevant AGENTS.md or CLAUDE.md files listed by open_workspace before working in nested directories.", config.skillsEnabled - ? "If available skills were returned and a task matches one, read that skill's path before proceeding. Skill paths may be outside the workspace; only advertised SKILL.md files and files under already-loaded skill directories are readable." + ? "If available skills were returned and a task matches one, read that skill's path before proceeding. Skill paths may be outside the workspace; files within advertised skill directories are readable." : "", ] .filter(Boolean) @@ -650,7 +650,6 @@ function registerMcpSurface( }, response.content, startedAt); return response; } - workspaces.markReadPathLoaded(workspace, readPath); logToolCall(config, { tool: toolNames.read, diff --git a/src/skills.test.ts b/src/skills.test.ts index 41556b3ad..d758dee81 100644 --- a/src/skills.test.ts +++ b/src/skills.test.ts @@ -232,18 +232,12 @@ try { assert.ok(projectSkill); assert.match(formatPathForPrompt(projectSkill.filePath), /SKILL\.md$/); - const skillFileRead = resolveSkillReadPath(loaded.skills, new Set(), projectSkill.filePath); - assert.equal(skillFileRead?.isSkillFile, true); + const skillFileRead = resolveSkillReadPath(loaded.skills, projectSkill.filePath); assert.equal(skillFileRead?.absolutePath, projectSkill.filePath); const resourcePath = join(projectSkill.baseDir, "references.md"); await writeFile(resourcePath, "reference\n"); - assert.equal(resolveSkillReadPath(loaded.skills, new Set(), resourcePath), undefined); - assert.equal( - resolveSkillReadPath(loaded.skills, new Set([projectSkill.baseDir]), resourcePath) - ?.isSkillFile, - false, - ); + assert.equal(resolveSkillReadPath(loaded.skills, resourcePath)?.absolutePath, resourcePath); } finally { if (originalHome === undefined) delete process.env.HOME; else process.env.HOME = originalHome; diff --git a/src/skills.ts b/src/skills.ts index cf4fa3322..74c77344d 100644 --- a/src/skills.ts +++ b/src/skills.ts @@ -18,7 +18,6 @@ export interface LoadedSkills { export interface SkillReadResolution { absolutePath: string; skill: Skill; - isSkillFile: boolean; } const SUBAGENTS_SKILL_NAME = "subagents"; @@ -84,7 +83,6 @@ export function loadWorkspaceSkills(config: ServerConfig, cwd: string): LoadedSk export function resolveSkillReadPath( skills: Skill[], - activatedSkillDirs: Set, inputPath: string, ): SkillReadResolution | undefined { const absolutePath = resolve(expandHomePath(inputPath)); @@ -92,28 +90,20 @@ export function resolveSkillReadPath( for (const skill of skills) { const skillFilePath = resolve(skill.filePath); if (absolutePath === skillFilePath) { - return { absolutePath, skill, isSkillFile: true }; + return { absolutePath, skill }; } } for (const skill of skills) { const baseDir = resolve(skill.baseDir); - if (!activatedSkillDirs.has(baseDir)) continue; if (!isPathInsideRoot(absolutePath, baseDir)) continue; - return { absolutePath, skill, isSkillFile: false }; + return { absolutePath, skill }; } return undefined; } -export function markSkillActivated( - activatedSkillDirs: Set, - skill: Skill, -): void { - activatedSkillDirs.add(resolve(skill.baseDir)); -} - export function formatPathForPrompt(path: string): string { const home = resolve(homedir()); const resolvedPath = resolve(path); diff --git a/src/workspaces.test.ts b/src/workspaces.test.ts index b31e108db..9ab74158c 100644 --- a/src/workspaces.test.ts +++ b/src/workspaces.test.ts @@ -156,6 +156,62 @@ test("persisted checkout and worktree sessions restore after recreating the regi } }); +test("workspace cache evicts old contexts without losing advertised skill reads", async (t) => { + const context = await fixture(t); + const stateDir = join(context.root, ".bounded-state"); + const agentDir = join(context.outsideRoot, "agent"); + const skillDir = join(agentDir, "skills", "cache-skill"); + const skillFile = join(skillDir, "SKILL.md"); + const resourceFile = join(skillDir, "reference.md"); + await mkdir(skillDir, { recursive: true }); + await writeFile( + skillFile, + [ + "---", + "name: cache-skill", + "description: Cache eviction regression skill.", + "---", + "", + "Read the reference when needed.", + "", + ].join("\n"), + ); + await writeFile(resourceFile, "reference\n"); + + const config = loadConfig(writeTestDevspaceConfig( + join(context.root, ".bounded-home"), + { + server: { port: 1 }, + workspaces: { + allowedRoots: [context.root], + worktreeRoot: join(context.root, ".devspace", "bounded-worktrees"), + }, + skills: { agentDir }, + subagents: { enabled: true, providers: [] }, + }, + )); + + const store = new SqliteWorkspaceStore(stateDir); + t.after(() => store.close()); + const registry = new WorkspaceRegistry(config, store); + const first = await registry.openWorkspace(context.root); + assert.equal( + registry.resolveReadPath(first.workspace, resourceFile).absolutePath, + resourceFile, + ); + + for (let index = 0; index < 32; index += 1) { + await registry.openWorkspace(context.root); + } + + const restored = registry.getWorkspace(first.workspace.id); + assert.notEqual(restored, first.workspace); + assert.equal( + registry.resolveReadPath(restored, resourceFile).absolutePath, + resourceFile, + ); +}); + test("workspace paths outside the allowed roots are rejected", async (t) => { const context = await fixture(t); diff --git a/src/workspaces.ts b/src/workspaces.ts index 3c83b343a..6b339dd76 100644 --- a/src/workspaces.ts +++ b/src/workspaces.ts @@ -18,7 +18,6 @@ import { } from "./roots.js"; import { loadWorkspaceSkills, - markSkillActivated, resolveSkillReadPath, type LoadedSkills, type SkillReadResolution, @@ -55,7 +54,6 @@ export interface Workspace { skills: LoadedSkills["skills"]; skillDiagnostics: LoadedSkills["diagnostics"]; agentProfiles: LocalAgentProfile[]; - activatedSkillDirs: Set; } export interface WorkspaceContext { @@ -90,6 +88,8 @@ type DirectoryOps = { mkdir: (path: string, options: { recursive: true }) => Promise; }; +const MAX_CACHED_WORKSPACES = 32; + export class WorkspaceRegistry { private readonly workspaces = new Map(); private readonly pendingCheckoutOpens = new Map>(); @@ -247,6 +247,8 @@ export class WorkspaceRegistry { getWorkspace(workspaceId: string): Workspace { const workspace = this.workspaces.get(workspaceId); if (workspace) { + this.workspaces.delete(workspaceId); + this.workspaces.set(workspaceId, workspace); this.store?.touchSession(workspaceId); return workspace; } @@ -277,10 +279,9 @@ export class WorkspaceRegistry { : undefined, ...this.loadSkillsForWorkspace(root), agentProfiles: [], - activatedSkillDirs: new Set(), }; this.store?.touchSession(workspaceId); - this.workspaces.set(restoredWorkspace.id, restoredWorkspace); + this.rememberWorkspace(restoredWorkspace); return restoredWorkspace; } @@ -303,7 +304,6 @@ export class WorkspaceRegistry { } catch (workspaceError) { const skillRead = resolveSkillReadPath( workspace.skills, - workspace.activatedSkillDirs, inputPath, ); if (!skillRead) throw workspaceError; @@ -316,12 +316,6 @@ export class WorkspaceRegistry { } } - markReadPathLoaded(workspace: Workspace, readPath: WorkspaceReadPath): void { - if (readPath.skillRead?.isSkillFile) { - markSkillActivated(workspace.activatedSkillDirs, readPath.skillRead.skill); - } - } - resolveWorkingDirectory(workspace: Workspace, workingDirectory: string | undefined): string { const directory = workingDirectory ? this.resolvePath(workspace, workingDirectory) : workspace.root; return assertAllowedPath(directory, [workspace.root]); @@ -366,7 +360,6 @@ export class WorkspaceRegistry { worktree: input.worktree, ...this.loadSkillsForWorkspace(input.root), agentProfiles: await loadLocalAgentProfiles(this.config, input.root), - activatedSkillDirs: new Set(), }; this.store?.createSession({ @@ -378,7 +371,7 @@ export class WorkspaceRegistry { baseSha: workspace.worktree?.baseSha, managed: workspace.worktree?.managed, }); - this.workspaces.set(workspace.id, workspace); + this.rememberWorkspace(workspace); const agentsFiles = await this.loadInitialAgentsFiles(workspace.root); const availableAgentsFiles = await this.findAvailableAgentsFiles(workspace.root, agentsFiles); @@ -391,6 +384,18 @@ export class WorkspaceRegistry { }; } + private rememberWorkspace(workspace: Workspace): void { + this.workspaces.delete(workspace.id); + this.workspaces.set(workspace.id, workspace); + + if (!this.store) return; + while (this.workspaces.size > MAX_CACHED_WORKSPACES) { + const oldestWorkspaceId = this.workspaces.keys().next().value as string | undefined; + if (!oldestWorkspaceId) break; + this.workspaces.delete(oldestWorkspaceId); + } + } + private loadSkillsForWorkspace(root: string): Pick { const result = loadWorkspaceSkills(this.config, root); return { From af27987b3e07b66d8b0755479ecc8637dba7e0b1 Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Sun, 6 Sep 2026 12:06:26 +0530 Subject: [PATCH 2/2] test(workspace): close store before fixture cleanup --- src/workspaces.test.ts | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/workspaces.test.ts b/src/workspaces.test.ts index 9ab74158c..e3506937f 100644 --- a/src/workspaces.test.ts +++ b/src/workspaces.test.ts @@ -192,24 +192,27 @@ test("workspace cache evicts old contexts without losing advertised skill reads" )); const store = new SqliteWorkspaceStore(stateDir); - t.after(() => store.close()); - const registry = new WorkspaceRegistry(config, store); - const first = await registry.openWorkspace(context.root); - assert.equal( - registry.resolveReadPath(first.workspace, resourceFile).absolutePath, - resourceFile, - ); - - for (let index = 0; index < 32; index += 1) { - await registry.openWorkspace(context.root); + try { + const registry = new WorkspaceRegistry(config, store); + const first = await registry.openWorkspace(context.root); + assert.equal( + registry.resolveReadPath(first.workspace, resourceFile).absolutePath, + resourceFile, + ); + + for (let index = 0; index < 32; index += 1) { + await registry.openWorkspace(context.root); + } + + const restored = registry.getWorkspace(first.workspace.id); + assert.notEqual(restored, first.workspace); + assert.equal( + registry.resolveReadPath(restored, resourceFile).absolutePath, + resourceFile, + ); + } finally { + store.close(); } - - const restored = registry.getWorkspace(first.workspace.id); - assert.notEqual(restored, first.workspace); - assert.equal( - registry.resolveReadPath(restored, resourceFile).absolutePath, - resourceFile, - ); }); test("workspace paths outside the allowed roots are rejected", async (t) => {