Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions docs/chatgpt-coding-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions docs/gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 2 additions & 3 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.`;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -650,7 +650,6 @@ function registerMcpSurface(
}, response.content, startedAt);
return response;
}
workspaces.markReadPathLoaded(workspace, readPath);

logToolCall(config, {
tool: toolNames.read,
Expand Down
10 changes: 2 additions & 8 deletions src/skills.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 2 additions & 12 deletions src/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export interface LoadedSkills {
export interface SkillReadResolution {
absolutePath: string;
skill: Skill;
isSkillFile: boolean;
}

const SUBAGENTS_SKILL_NAME = "subagents";
Expand Down Expand Up @@ -84,36 +83,27 @@ export function loadWorkspaceSkills(config: ServerConfig, cwd: string): LoadedSk

export function resolveSkillReadPath(
skills: Skill[],
activatedSkillDirs: Set<string>,
inputPath: string,
): SkillReadResolution | undefined {
const absolutePath = resolve(expandHomePath(inputPath));

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<string>,
skill: Skill,
): void {
activatedSkillDirs.add(resolve(skill.baseDir));
}

export function formatPathForPrompt(path: string): string {
const home = resolve(homedir());
const resolvedPath = resolve(path);
Expand Down
59 changes: 59 additions & 0 deletions src/workspaces.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,65 @@ 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);
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();
}
});

test("workspace paths outside the allowed roots are rejected", async (t) => {
const context = await fixture(t);

Expand Down
31 changes: 18 additions & 13 deletions src/workspaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
} from "./roots.js";
import {
loadWorkspaceSkills,
markSkillActivated,
resolveSkillReadPath,
type LoadedSkills,
type SkillReadResolution,
Expand Down Expand Up @@ -55,7 +54,6 @@ export interface Workspace {
skills: LoadedSkills["skills"];
skillDiagnostics: LoadedSkills["diagnostics"];
agentProfiles: LocalAgentProfile[];
activatedSkillDirs: Set<string>;
}

export interface WorkspaceContext {
Expand Down Expand Up @@ -90,6 +88,8 @@ type DirectoryOps = {
mkdir: (path: string, options: { recursive: true }) => Promise<unknown>;
};

const MAX_CACHED_WORKSPACES = 32;

export class WorkspaceRegistry {
private readonly workspaces = new Map<string, Workspace>();
private readonly pendingCheckoutOpens = new Map<string, Promise<WorkspaceContext>>();
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -303,7 +304,6 @@ export class WorkspaceRegistry {
} catch (workspaceError) {
const skillRead = resolveSkillReadPath(
workspace.skills,
workspace.activatedSkillDirs,
inputPath,
);
if (!skillRead) throw workspaceError;
Expand All @@ -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]);
Expand Down Expand Up @@ -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({
Expand All @@ -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);

Expand All @@ -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<Workspace, "skills" | "skillDiagnostics"> {
const result = loadWorkspaceSkills(this.config, root);
return {
Expand Down
Loading