diff --git a/docs/chatgpt-coding-workflow.md b/docs/chatgpt-coding-workflow.md index 66062621..ac2bdc7c 100644 --- a/docs/chatgpt-coding-workflow.md +++ b/docs/chatgpt-coding-workflow.md @@ -17,12 +17,36 @@ ChatGPT should call `open_workspace` once for a project folder: The result includes a `workspaceId`. All later file, search, edit, show-changes, and shell calls should reuse that same `workspaceId`. -Do not reopen the same folder unless: +ChatGPT may support automatic checkout recovery through optional host +conversation metadata. This is an OpenAI-host adapter detail, not a standard MCP +conversation field. When that optional context is available, opening the same +checkout project again in the same conversation can continue in the existing +workspace, and the context already provided for that reused checkout is not +repeated. The portable workflow remains the same: keep using the `workspaceId` +returned by `open_workspace` for later operations. Hosts without supported +conversation context receive a normal new workspace and continue with that +explicit `workspaceId` workflow. +The model receives actionable workspace instructions; automatic-reuse +bookkeeping is not a model-facing choice. + +Worktree mode is deliberately different: every call creates a new managed +worktree and a new workspace session with complete context, even for the same +path and base ref. + +The first successful open of a checkout provides complete instructions and +coding context. A repeated open that reuses the same checkout workspace does +not repeat the model-visible context, but the workspace UI continues to show the +complete details. Every new worktree establishes and returns its own complete +context, even when the same project was already opened in checkout or another +worktree. Opening checkout after a worktree therefore provides the checkout's +own context. + +Do not call `open_workspace` again for the same checkout folder unless: - the `workspaceId` is rejected as unknown -- the user switches to another folder -- the user switches between checkout and worktree mode -- the user explicitly asks to reopen +- work moves to a different project folder +- work switches between checkout and worktree mode +- the user asks for a new isolated worktree ## Checkout Mode @@ -56,6 +80,11 @@ Managed worktrees are created under: Worktree mode requires a Git repository with at least one commit. It starts from `HEAD` unless `baseRef` is provided. +Each worktree-mode call creates a new managed worktree and returns a new +`workspaceId`. Reuse that ID for work inside that worktree; call +`open_workspace` in worktree mode again only when another isolated worktree is +actually required. + Uncommitted source checkout changes are not copied into the managed worktree. DevSpace reports when the source checkout was dirty so the model can decide how to proceed with the user. @@ -158,10 +187,10 @@ and shell tools. The aggregate `show_changes` tool is not exposed by default. Use `DEVSPACE_WIDGETS=off` to disable widget UI, or `DEVSPACE_WIDGETS=changes` to expose the aggregate show-changes flow. -When `show_changes` is exposed, models should call it exactly once after the -final file modification in any turn that changes files. The tool only requires -the `workspaceId`; DevSpace automatically compares against the last shown -checkpoint and advances that checkpoint after rendering the aggregate diff. +When `show_changes` is exposed, call it exactly once after the final file +modification in any turn that changes files. It shows the combined changes for +that turn and advances the review point automatically. Reusing a workspace does +not change this workflow. ## Shell Use diff --git a/docs/gotchas.md b/docs/gotchas.md index 779e37ef..639f54a9 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -136,8 +136,23 @@ npx @waishnav/devspace init --force client receives an unknown workspace error, call `open_workspace` again for that project. -Workspace session metadata is persisted, but clients should still treat -`open_workspace` as the way to begin a fresh working session. +Workspace session metadata is persisted. ChatGPT may provide optional +conversation metadata that lets DevSpace resume the same checkout workspace for +the same project in that conversation; repeated opens reuse the `workspaceId` +and do not repeat context already provided for that reused checkout. Worktree +mode always creates a new isolated workspace with its own complete context. +Hosts without supported conversation metadata receive a normal new workspace. +In all cases, continue passing the `workspaceId` returned by `open_workspace` to +later tools. Other MCP hosts use this explicit workspace workflow as well. + +To review work, call `show_changes` once after the final related file change. It +shows the combined changes and advances the review point automatically. + +## Data Retention + +DevSpace does not currently prune workspace sessions, conversation bindings, +or review refs. A future product retention policy will define safe cleanup for +these records; no automatic deletion is performed today. ## Workspace Path Rejected diff --git a/package.json b/package.json index 4489360e..59e23330 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "dev": "node scripts/dev-server.mjs", "postinstall": "node scripts/fix-node-pty-permissions.mjs", "start": "node dist/cli.js serve", - "test": "tsx src/config.test.ts && tsx src/incoming-artifacts.test.ts && tsx src/artifact-download.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/patch-display.test.ts && tsx src/ui/tool-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/review-checkpoints.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts", + "test": "tsx src/config.test.ts && tsx src/request-meta.test.ts && tsx src/incoming-artifacts.test.ts && tsx src/artifact-download.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/patch-display.test.ts && tsx src/ui/tool-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/workspace-conversation.test.ts && tsx src/review-checkpoints.test.ts && tsx src/server.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts", "typecheck": "tsc -p tsconfig.json --noEmit" }, "keywords": [], diff --git a/src/db/migrations.ts b/src/db/migrations.ts index 2bda20c2..1c5c3298 100644 --- a/src/db/migrations.ts +++ b/src/db/migrations.ts @@ -22,6 +22,11 @@ const migrations: Migration[] = [ name: "local-agent-sessions", up: migrateLocalAgentSessions, }, + { + version: 4, + name: "workspace-conversation-bindings", + up: migrateWorkspaceConversationBindings, + }, ]; export function migrateDatabase(sqlite: Database.Database): void { @@ -174,6 +179,25 @@ function migrateLocalAgentSessions(sqlite: Database.Database): void { addColumnIfMissing(sqlite, "local_agent_sessions", "thinking", "text"); } +function migrateWorkspaceConversationBindings(sqlite: Database.Database): void { + sqlite.exec(` + create table if not exists workspace_conversation_bindings ( + conversation_scope_id text not null, + target_key text not null, + workspace_session_id text not null, + created_at text not null, + last_used_at text not null, + primary key (conversation_scope_id, target_key), + foreign key (workspace_session_id) + references workspace_sessions(id) + on delete cascade + ); + + create index if not exists workspace_conversation_bindings_workspace_idx + on workspace_conversation_bindings(workspace_session_id); + `); +} + function addColumnIfMissing( sqlite: Database.Database, table: "workspace_sessions" | "local_agent_sessions", diff --git a/src/db/schema.ts b/src/db/schema.ts index 01d13fac..215c6c1a 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -38,6 +38,23 @@ export const loadedAgentFiles = sqliteTable( ], ); +export const workspaceConversationBindings = sqliteTable( + "workspace_conversation_bindings", + { + conversationScopeId: text("conversation_scope_id").notNull(), + targetKey: text("target_key").notNull(), + workspaceSessionId: text("workspace_session_id") + .notNull() + .references(() => workspaceSessions.id, { onDelete: "cascade" }), + createdAt: text("created_at").notNull(), + lastUsedAt: text("last_used_at").notNull(), + }, + (table) => [ + primaryKey({ columns: [table.conversationScopeId, table.targetKey] }), + index("workspace_conversation_bindings_workspace_idx").on(table.workspaceSessionId), + ], +); + export const oauthClients = sqliteTable( "oauth_clients", { @@ -101,5 +118,7 @@ export type WorkspaceSessionRow = typeof workspaceSessions.$inferSelect; export type NewWorkspaceSessionRow = typeof workspaceSessions.$inferInsert; export type LoadedAgentFileRow = typeof loadedAgentFiles.$inferSelect; export type NewLoadedAgentFileRow = typeof loadedAgentFiles.$inferInsert; +export type WorkspaceConversationBindingRow = typeof workspaceConversationBindings.$inferSelect; +export type NewWorkspaceConversationBindingRow = typeof workspaceConversationBindings.$inferInsert; export type LocalAgentSessionRow = typeof localAgentSessions.$inferSelect; export type NewLocalAgentSessionRow = typeof localAgentSessions.$inferInsert; diff --git a/src/oauth-store.test.ts b/src/oauth-store.test.ts index e1c00338..e47f8121 100644 --- a/src/oauth-store.test.ts +++ b/src/oauth-store.test.ts @@ -44,6 +44,7 @@ async function testDatabaseConfiguration(stateDir: string): Promise { { version: 1, name: "workspace-state" }, { version: 2, name: "oauth-state" }, { version: 3, name: "local-agent-sessions" }, + { version: 4, name: "workspace-conversation-bindings" }, ]); } finally { database.close(); diff --git a/src/request-meta.test.ts b/src/request-meta.test.ts new file mode 100644 index 00000000..effd3dd0 --- /dev/null +++ b/src/request-meta.test.ts @@ -0,0 +1,38 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { openAiConversationScopeId } from "./request-meta.js"; + +test("undefined request metadata has no conversation scope", () => { + assert.equal(openAiConversationScopeId(undefined), undefined); +}); + +test("missing session metadata has no conversation scope", () => { + assert.equal(openAiConversationScopeId({}), undefined); +}); + +test("an empty session string has no conversation scope", () => { + assert.equal(openAiConversationScopeId({ "openai/session": "" }), undefined); +}); + +test("a non-string session value has no conversation scope", () => { + assert.equal(openAiConversationScopeId({ "openai/session": 42 }), undefined); + assert.equal(openAiConversationScopeId({ "openai/session": {} }), undefined); +}); + +test("valid OpenAI session metadata returns the raw opaque session value", () => { + assert.equal( + openAiConversationScopeId({ "openai/session": "chat-session-opaque-value" }), + "chat-session-opaque-value", + ); +}); + +test("unrelated metadata fields do not alter the selected conversation scope", () => { + assert.equal( + openAiConversationScopeId({ + "openai/session": "chat-session-opaque-value", + "openai/subject": "user-1", + "openai/organization": "org-1", + }), + "chat-session-opaque-value", + ); +}); diff --git a/src/request-meta.ts b/src/request-meta.ts new file mode 100644 index 00000000..5b8cb1ea --- /dev/null +++ b/src/request-meta.ts @@ -0,0 +1,14 @@ +function metadataString( + meta: unknown, + key: string, +): string | undefined { + if (typeof meta !== "object" || meta === null) return undefined; + const value = (meta as Record)[key]; + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +export function openAiConversationScopeId( + meta: unknown, +): string | undefined { + return metadataString(meta, "openai/session"); +} diff --git a/src/review-checkpoints.test.ts b/src/review-checkpoints.test.ts index 3ec4676a..0c2aeb7b 100644 --- a/src/review-checkpoints.test.ts +++ b/src/review-checkpoints.test.ts @@ -1,56 +1,228 @@ +import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import test, { type TestContext } from "node:test"; import { promisify } from "node:util"; -import assert from "node:assert/strict"; import { createReviewCheckpointManager } from "./review-checkpoints.js"; const execFileAsync = promisify(execFile); -const root = await mkdtemp(join(tmpdir(), "devspace-review-checkpoints-test-")); - -try { - await git(root, ["init"]); - await git(root, ["config", "user.email", "devspace@example.com"]); - await git(root, ["config", "user.name", "DevSpace Test"]); - await writeFile(join(root, "README.md"), "hello\n"); - await git(root, ["add", "README.md"]); - await git(root, ["commit", "-m", "Initial commit"]); +test("a clean workspace reports no changes from the last-shown checkpoint", async (t) => { + const root = await committedRepository(t); const manager = createReviewCheckpointManager(); - await manager.initializeWorkspace({ workspaceId: "ws_review", root }); - const clean = await manager.reviewChanges({ workspaceId: "ws_review", root }); + await manager.initializeWorkspace({ workspaceId: "ws_clean", root }); + const clean = await manager.reviewChanges({ workspaceId: "ws_clean", root }); + assert.equal(clean.summary.files, 0); assert.equal(clean.patch, ""); - assert.match(clean.result, /No changes/); + assert.match(clean.result, /No changes since last shown changes/); +}); + +test("show_changes reports and advances the last-shown checkpoint", async (t) => { + const root = await committedRepository(t); + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_incremental", root }); await writeFile(join(root, "README.md"), "hello\nworld\n"); await writeFile(join(root, "new.txt"), "new\n"); - const firstReview = await manager.reviewChanges({ - workspaceId: "ws_review", + const unreviewed = await manager.reviewChanges({ + workspaceId: "ws_incremental", root, markReviewed: false, }); - assert.equal(firstReview.summary.files, 2); - assert.equal(firstReview.summary.additions, 2); - assert.equal(firstReview.summary.removals, 0); - assert.equal(firstReview.files.some((file) => file.path === "README.md"), true); - assert.equal(firstReview.files.some((file) => file.path === "new.txt"), true); - assert.match(firstReview.patch, /world/); + assert.deepEqual(unreviewed.files.map((file) => file.path).sort(), ["README.md", "new.txt"]); + assert.equal(unreviewed.summary.additions, 2); + assert.match(unreviewed.patch, /world/); - const stillUnreviewed = await manager.reviewChanges({ - workspaceId: "ws_review", + const markedReviewed = await manager.reviewChanges({ + workspaceId: "ws_incremental", root, markReviewed: true, }); - assert.equal(stillUnreviewed.summary.files, 2); + assert.equal(markedReviewed.summary.files, 2); - const afterReviewed = await manager.reviewChanges({ workspaceId: "ws_review", root }); + const afterReviewed = await manager.reviewChanges({ workspaceId: "ws_incremental", root }); assert.equal(afterReviewed.summary.files, 0); -} finally { - await rm(root, { recursive: true, force: true }); + assert.equal(afterReviewed.patch, ""); +}); + +test("review checkpoints survive a manager restart", async (t) => { + const root = await committedRepository(t); + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_restart", root }); + + await writeFile(join(root, "README.md"), "hello\nworld\n"); + await manager.reviewChanges({ workspaceId: "ws_restart", root, markReviewed: true }); + + const restartedManager = createReviewCheckpointManager(); + await restartedManager.initializeWorkspace({ workspaceId: "ws_restart", root }); + await writeFile(join(root, "later.txt"), "after restart\n"); + + const afterRestart = await restartedManager.reviewChanges({ + workspaceId: "ws_restart", + root, + markReviewed: false, + }); + assert.deepEqual(afterRestart.files.map((file) => file.path), ["later.txt"]); + assert.match(afterRestart.patch, /after restart/); + assert.doesNotMatch(afterRestart.patch, /world/); +}); + +test("concurrent initialization produces one usable checkpoint state", async (t) => { + const root = await committedRepository(t); + const manager = createReviewCheckpointManager(); + + const [, concurrentReview] = await Promise.all([ + manager.initializeWorkspace({ workspaceId: "ws_concurrent", root }), + manager.reviewChanges({ workspaceId: "ws_concurrent", root, markReviewed: false }), + ]); + assert.equal(concurrentReview.summary.files, 0); + + await writeFile(join(root, "later.txt"), "visible after initialization\n"); + const afterInitialization = await manager.reviewChanges({ + workspaceId: "ws_concurrent", + root, + markReviewed: false, + }); + assert.deepEqual(afterInitialization.files.map((file) => file.path), ["later.txt"]); +}); + +test("a missing last-shown checkpoint falls back after restart and can be re-established", async (t) => { + const root = await committedRepository(t); + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_missing_baseline", root }); + + await writeFile(join(root, "README.md"), "hello\nchanged\n"); + await deleteReviewRef(root, "ws_missing_baseline", "baseline"); + + const restartedManager = createReviewCheckpointManager(); + await restartedManager.initializeWorkspace({ workspaceId: "ws_missing_baseline", root }); + + const fallback = await restartedManager.reviewChanges({ + workspaceId: "ws_missing_baseline", + root, + markReviewed: false, + }); + assert.equal(fallback.summary.files, 1); + assert.match(fallback.result, /compared from workspace open/); + assert.match(fallback.patch, /changed/); + + const reestablished = await restartedManager.reviewChanges({ + workspaceId: "ws_missing_baseline", + root, + markReviewed: true, + }); + assert.equal(reestablished.summary.files, 1); + assert.match(reestablished.result, /baseline was re-established/); + + const afterReestablished = await restartedManager.reviewChanges({ + workspaceId: "ws_missing_baseline", + root, + markReviewed: false, + }); + assert.equal(afterReestablished.summary.files, 0); +}); + +test("a checkpoint workspace rejects a different root without changing its state", async (t) => { + const root = await committedRepository(t); + const otherRoot = await committedRepository(t); + const manager = createReviewCheckpointManager(); + + await manager.initializeWorkspace({ workspaceId: "ws_root_mismatch", root }); + + await assert.rejects( + () => manager.reviewChanges({ + workspaceId: "ws_root_mismatch", + root: otherRoot, + markReviewed: false, + }), + /workspace root mismatch/, + ); + + await writeFile(join(root, "only-first-root.txt"), "first root\n"); + const review = await manager.reviewChanges({ + workspaceId: "ws_root_mismatch", + root, + markReviewed: false, + }); + assert.deepEqual(review.files.map((file) => file.path), ["only-first-root.txt"]); +}); + +test("a concurrent review rejects a different root after initialization", async (t) => { + const root = await committedRepository(t); + const otherRoot = await committedRepository(t); + const manager = createReviewCheckpointManager(); + + const [initialization, review] = await Promise.allSettled([ + manager.initializeWorkspace({ workspaceId: "ws_concurrent_root_mismatch", root }), + manager.reviewChanges({ + workspaceId: "ws_concurrent_root_mismatch", + root: otherRoot, + markReviewed: false, + }), + ]); + + assert.equal(initialization.status, "fulfilled"); + assert.equal(review.status, "rejected"); + if (review.status === "rejected") { + assert.match(String(review.reason), /workspace root mismatch/); + } +}); + +test("an unborn repository becomes reviewable after its first commit", async (t) => { + const root = await unbornRepository(t); + const manager = createReviewCheckpointManager(); + + await manager.initializeWorkspace({ workspaceId: "ws_unborn", root }); + await assert.rejects( + () => manager.reviewChanges({ workspaceId: "ws_unborn", root }), + /repository has no HEAD commit/, + ); + + await writeFile(join(root, "README.md"), "first commit\n"); + await git(root, ["add", "README.md"]); + await git(root, ["commit", "-m", "Initial commit"]); + + const afterFirstCommit = await manager.reviewChanges({ + workspaceId: "ws_unborn", + root, + markReviewed: false, + }); + assert.equal(afterFirstCommit.summary.files, 0); + assert.equal(afterFirstCommit.patch, ""); +}); + +async function committedRepository(t: TestContext): Promise { + const root = await mkdtemp(join(tmpdir(), "devspace-review-checkpoints-test-")); + t.after(() => rm(root, { recursive: true, force: true })); + await git(root, ["init"]); + await git(root, ["config", "user.email", "devspace@example.com"]); + await git(root, ["config", "user.name", "DevSpace Test"]); + await writeFile(join(root, "README.md"), "hello\n"); + await git(root, ["add", "README.md"]); + await git(root, ["commit", "-m", "Initial commit"]); + return root; +} + +async function unbornRepository(t: TestContext): Promise { + const root = await mkdtemp(join(tmpdir(), "devspace-review-unborn-test-")); + t.after(() => rm(root, { recursive: true, force: true })); + await git(root, ["init"]); + await git(root, ["config", "user.email", "devspace@example.com"]); + await git(root, ["config", "user.name", "DevSpace Test"]); + return root; +} + +async function deleteReviewRef( + root: string, + workspaceId: string, + checkpoint: "open" | "baseline", +): Promise { + await git(root, ["update-ref", "-d", `refs/devspace/review/${workspaceId}/${checkpoint}`]); } async function git(cwd: string, args: string[]): Promise { diff --git a/src/review-checkpoints.ts b/src/review-checkpoints.ts index eaa04dfa..0fd8bf36 100644 --- a/src/review-checkpoints.ts +++ b/src/review-checkpoints.ts @@ -3,7 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { git, getGitEligibility, safeWorkspaceRefSegment } from "./git.js"; -export type ReviewSince = "last_shown" | "last_review" | "workspace_open"; +export type ReviewSince = "last_shown" | "workspace_open"; export interface ReviewSummary { files: number; @@ -31,6 +31,8 @@ interface WorkspaceReviewState { gitRoot?: string; openRef: string; baselineRef: string; + openRefAvailable: boolean; + baselineRefAvailable: boolean; diagnostic?: string; } @@ -48,41 +50,62 @@ const REVIEW_REF_PREFIX = "refs/devspace/review"; export function createReviewCheckpointManager(): ReviewCheckpointManager { const states = new Map(); + const initializations = new Map>(); return { async initializeWorkspace({ workspaceId, root }) { - const refs = reviewRefs(workspaceId); - const state: WorkspaceReviewState = { root, ...refs }; - states.set(workspaceId, state); + const existingState = states.get(workspaceId); + assertWorkspaceRoot(existingState, workspaceId, root); + if (existingState?.root === root && existingState.gitRoot !== undefined) { + return; + } + + const pending = initializations.get(workspaceId); + if (pending) { + await pending; + assertWorkspaceRoot(states.get(workspaceId), workspaceId, root); + return; + } + const initialize = initializeWorkspaceState(states, workspaceId, root); + initializations.set(workspaceId, initialize); try { - const eligibility = await getGitEligibility(root); - if (!eligibility.ok || !eligibility.gitRoot) { - state.diagnostic = eligibility.message ?? "show_changes requires a Git workspace in this version."; - return; + await initialize; + } finally { + if (initializations.get(workspaceId) === initialize) { + initializations.delete(workspaceId); } - - state.gitRoot = eligibility.gitRoot; - const commit = await createWorkingTreeSnapshot(eligibility.gitRoot); - await git(eligibility.gitRoot, ["update-ref", state.openRef, commit]); - await git(eligibility.gitRoot, ["update-ref", state.baselineRef, commit]); - } catch (error) { - state.diagnostic = error instanceof Error ? error.message : String(error); } }, async reviewChanges({ workspaceId, root, since = "last_shown", markReviewed = true }) { let state = states.get(workspaceId); - if (!state) { + assertWorkspaceRoot(state, workspaceId, root); + if (!isReadyState(state)) { await this.initializeWorkspace({ workspaceId, root }); state = states.get(workspaceId); } + assertWorkspaceRoot(state, workspaceId, root); if (!state?.gitRoot) { throw new Error(state?.diagnostic ?? "show_changes requires a Git workspace in this version."); } - const baselineRef = since === "workspace_open" ? state.openRef : state.baselineRef; + let effectiveSince = since; + let usedWorkspaceOpenFallback = false; + if (since === "last_shown" && !state.baselineRefAvailable) { + if (!state.openRefAvailable) { + throw new Error("Review checkpoints are missing; show_changes cannot reconstruct that history safely."); + } + effectiveSince = "workspace_open"; + usedWorkspaceOpenFallback = true; + } else if (since === "workspace_open" && !state.openRefAvailable) { + throw new Error( + "The workspace-open review checkpoint is missing; show_changes cannot reconstruct that history safely.", + ); + } + + const baselineRef = effectiveSince === "workspace_open" ? state.openRef : state.baselineRef; const baseline = (await git(state.gitRoot, ["rev-parse", "--verify", `${baselineRef}^{commit}`])).stdout.trim(); const current = await createWorkingTreeSnapshot(state.gitRoot); const patch = (await git(state.gitRoot, ["diff", "--binary", "--no-color", baseline, current], { @@ -96,13 +119,18 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { if (markReviewed) { await git(state.gitRoot, ["update-ref", state.baselineRef, current]); + state.baselineRefAvailable = true; } + const fallbackNote = usedWorkspaceOpenFallback + ? ` The last-shown checkpoint was missing, so changes were compared from workspace open${markReviewed ? " and the baseline was re-established" : ""}.` + : ""; return { - result: + result: `${ summary.files === 0 - ? `No changes since ${since === "workspace_open" ? "workspace open" : "last shown changes"}.` - : `Changed ${summary.files} ${summary.files === 1 ? "file" : "files"} (+${summary.additions} -${summary.removals}).`, + ? `No changes since ${effectiveSince === "workspace_open" ? "workspace open" : "last shown changes"}.` + : `Changed ${summary.files} ${summary.files === 1 ? "file" : "files"} (+${summary.additions} -${summary.removals}).` + }${fallbackNote}`, summary, files, patch, @@ -111,7 +139,75 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { }; } -function reviewRefs(workspaceId: string): Pick { +function assertWorkspaceRoot( + state: WorkspaceReviewState | undefined, + workspaceId: string, + root: string, +): void { + if (state && state.root !== root) { + throw new Error(`Review checkpoint workspace root mismatch for ${workspaceId}.`); + } +} + +async function initializeWorkspaceState( + states: Map, + workspaceId: string, + root: string, +): Promise { + const refs = reviewRefs(workspaceId); + const state: WorkspaceReviewState = { + root, + ...refs, + openRefAvailable: false, + baselineRefAvailable: false, + }; + + try { + const eligibility = await getGitEligibility(root); + if (!eligibility.ok || !eligibility.gitRoot) { + state.diagnostic = eligibility.message ?? "show_changes requires a Git workspace in this version."; + return; + } + + const [openCommit, baselineCommit] = await Promise.all([ + commitForRef(eligibility.gitRoot, state.openRef), + commitForRef(eligibility.gitRoot, state.baselineRef), + ]); + + if (!openCommit && !baselineCommit) { + const initialCommit = await createWorkingTreeSnapshot(eligibility.gitRoot); + await git(eligibility.gitRoot, ["update-ref", state.openRef, initialCommit]); + await git(eligibility.gitRoot, ["update-ref", state.baselineRef, initialCommit]); + state.openRefAvailable = true; + state.baselineRefAvailable = true; + } else { + state.openRefAvailable = openCommit !== undefined; + state.baselineRefAvailable = baselineCommit !== undefined; + } + + state.gitRoot = eligibility.gitRoot; + } catch (error) { + state.diagnostic = error instanceof Error ? error.message : String(error); + } finally { + states.set(workspaceId, state); + } +} + +function isReadyState(state: WorkspaceReviewState | undefined): boolean { + return state?.gitRoot !== undefined; +} + +async function commitForRef(gitRoot: string, ref: string): Promise { + try { + return (await git(gitRoot, ["rev-parse", "--verify", `${ref}^{commit}`])).stdout.trim(); + } catch { + return undefined; + } +} + +function reviewRefs( + workspaceId: string, +): Pick { const segment = safeWorkspaceRefSegment(workspaceId); return { openRef: `${REVIEW_REF_PREFIX}/${segment}/open`, diff --git a/src/server.test.ts b/src/server.test.ts new file mode 100644 index 00000000..33f2a871 --- /dev/null +++ b/src/server.test.ts @@ -0,0 +1,304 @@ +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test, { type TestContext } from "node:test"; +import { promisify } from "node:util"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { loadConfig, type ServerConfig } from "./config.js"; +import { createReviewCheckpointManager } from "./review-checkpoints.js"; +import { ProcessSessionManager } from "./process-sessions.js"; +import { createMcpServer } from "./server.js"; +import { SqliteWorkspaceStore } from "./workspace-store.js"; +import { WorkspaceRegistry } from "./workspaces.js"; + +const execFileAsync = promisify(execFile); + +test("open_workspace keeps lifecycle flags out of model output and preserves complete card metadata", async (t) => { + const context = await fixture(t); + const first = await callOpen(context.client, context.project, "chat-1"); + const repeated = await callOpen(context.client, context.project, "chat-1"); + + const tools = await context.client.listTools(); + const openTool = tools.tools.find((tool) => tool.name === "open_workspace"); + const outputProperties = (openTool?.outputSchema as { properties?: Record } | undefined)?.properties; + assert.equal(outputProperties && "workspaceReused" in outputProperties, false); + assert.equal(outputProperties && "includeBootstrapContext" in outputProperties, false); + + const firstStructured = structuredContent(first); + assert.equal(firstStructured.workspaceId, structuredContent(repeated).workspaceId); + assert.ok(Array.isArray(firstStructured.agentsFiles)); + assert.ok(Array.isArray(firstStructured.availableAgentsFiles)); + assert.ok(Array.isArray(firstStructured.skills)); + assert.ok(Array.isArray(firstStructured.agentProviders)); + assert.ok(Array.isArray(firstStructured.agents)); + assert.ok(Array.isArray(firstStructured.skillDiagnostics)); + assert.equal("workspaceReused" in firstStructured, false); + assert.equal("includeBootstrapContext" in firstStructured, false); + + const repeatedStructured = structuredContent(repeated); + assert.equal(repeatedStructured.agentsFiles, undefined); + assert.equal(repeatedStructured.availableAgentsFiles, undefined); + assert.equal(repeatedStructured.skills, undefined); + assert.equal(repeatedStructured.agentProviders, undefined); + assert.equal(repeatedStructured.agents, undefined); + assert.equal(repeatedStructured.skillDiagnostics, undefined); + assert.equal("workspaceReused" in repeatedStructured, false); + assert.equal("includeBootstrapContext" in repeatedStructured, false); + + const repeatedText = responseText(repeated); + assert.match(repeatedText, /Workspace already open as/); + assert.match(repeatedText, /same checkout previously opened/); + assert.match(repeatedText, /Reuse this workspaceId for subsequent tool calls/); + assert.match(repeatedText, /previously provided for this workspace/); + assert.match(repeatedText, /not repeated here/); + + const card = responseCard(repeated); + assert.equal(card.workspaceReused, true); + assert.equal(card.includeBootstrapContext, false); + assert.ok(Array.isArray(card.agentsFiles)); + assert.ok(Array.isArray(card.availableAgentsFiles)); + assert.ok(Array.isArray(card.skills)); + assert.ok(Array.isArray(card.agentProviders)); + assert.ok(Array.isArray(card.agents)); + assert.ok(Array.isArray(card.skillDiagnostics)); +}); + +test("concurrent checkout opens return one full context and one reuse instruction", async (t) => { + const context = await fixture(t); + const [first, second] = await Promise.all([ + callOpen(context.client, context.project, "chat-1"), + callOpen(context.client, context.project, "chat-1"), + ]); + + assert.equal(structuredContent(first).workspaceId, structuredContent(second).workspaceId); + assert.equal( + [first, second].filter((result) => Array.isArray(structuredContent(result).agentsFiles)).length, + 1, + ); + assert.equal( + [first, second].filter((result) => responseText(result).includes("Workspace already open as")).length, + 1, + ); +}); + +test("new worktrees always receive a fresh workspace and complete worktree context", async (t) => { + const context = await fixture(t, { git: true }); + const checkout = await callOpen(context.client, context.project, "chat-1"); + const firstWorktree = await callOpen(context.client, context.project, "chat-1", "worktree"); + const secondWorktree = await callOpen(context.client, context.project, "chat-1", "worktree"); + const checkoutAgain = await callOpen(context.client, context.project, "chat-1"); + + assert.notEqual(structuredContent(firstWorktree).workspaceId, structuredContent(secondWorktree).workspaceId); + assert.equal(structuredContent(checkoutAgain).workspaceId, structuredContent(checkout).workspaceId); + for (const result of [firstWorktree, secondWorktree]) { + const structured = structuredContent(result); + assert.equal(structured.mode, "worktree"); + assert.ok(Array.isArray(structured.agentsFiles)); + assert.ok(Array.isArray(structured.availableAgentsFiles)); + assert.ok(Array.isArray(structured.skills)); + assert.ok(Array.isArray(structured.agentProviders)); + assert.ok(Array.isArray(structured.agents)); + assert.ok(Array.isArray(structured.skillDiagnostics)); + assert.match(responseText(result), /Opened isolated worktree workspace/); + } + assert.equal(structuredContent(checkoutAgain).agentsFiles, undefined); + assert.match(responseText(checkoutAgain), /same checkout previously opened/); +}); + +test("checkout opened after a worktree receives its own complete context", async (t) => { + const context = await fixture(t, { git: true }); + const worktree = await callOpen(context.client, context.project, "chat-1", "worktree"); + const checkout = await callOpen(context.client, context.project, "chat-1"); + const checkoutAgain = await callOpen(context.client, context.project, "chat-1"); + + assert.equal(structuredContent(worktree).mode, "worktree"); + assert.ok(Array.isArray(structuredContent(worktree).agentsFiles)); + assert.equal(structuredContent(checkout).mode, "checkout"); + assert.ok(Array.isArray(structuredContent(checkout).agentsFiles)); + assert.equal(structuredContent(checkoutAgain).workspaceId, structuredContent(checkout).workspaceId); + assert.equal(structuredContent(checkoutAgain).agentsFiles, undefined); + assert.match(responseText(checkoutAgain), /same checkout previously opened/); +}); + +test("a host without conversation metadata receives normal explicit-workspace behavior", async (t) => { + const context = await fixture(t); + const first = await callOpen(context.client, context.project); + const second = await callOpen(context.client, context.project); + + assert.notEqual(structuredContent(first).workspaceId, structuredContent(second).workspaceId); + assert.ok(Array.isArray(structuredContent(first).agentsFiles)); + assert.ok(Array.isArray(structuredContent(second).agentsFiles)); + assert.doesNotMatch(responseText(first), /conversation metadata/i); + assert.doesNotMatch(responseText(second), /conversation metadata/i); +}); + +test("checkout reuse and context suppression survive a registry restart", async (t) => { + const context = await fixture(t); + const first = await callOpen(context.client, context.project, "chat-1"); + const firstWorkspaceId = structuredContent(first).workspaceId; + + await context.close(); + + const restoredStore = new SqliteWorkspaceStore(context.stateDir); + const restoredServer = createMcpServer( + context.config, + new WorkspaceRegistry(context.config, restoredStore), + createReviewCheckpointManager(), + new ProcessSessionManager(), + [], + [], + ); + const [restoredClientTransport, restoredServerTransport] = InMemoryTransport.createLinkedPair(); + const restoredClient = new Client({ name: "devspace-restored-test-client", version: "1.0.0" }); + let restoredClosed = false; + const closeRestored = async () => { + if (restoredClosed) return; + restoredClosed = true; + await restoredClient.close(); + await restoredServer.close(); + restoredStore.close(); + }; + t.after(closeRestored); + + try { + await Promise.all([ + restoredClient.connect(restoredClientTransport), + restoredServer.connect(restoredServerTransport), + ]); + + const restored = await callOpen(restoredClient, context.project, "chat-1"); + assert.equal(structuredContent(restored).workspaceId, firstWorkspaceId); + assert.equal(structuredContent(restored).agentsFiles, undefined); + assert.match(responseText(restored), /same checkout previously opened/); + } finally { + await closeRestored(); + } +}); + +interface ServerFixture { + client: Client; + project: string; + config: ServerConfig; + stateDir: string; + close: () => Promise; +} + +async function fixture(t: TestContext, options: { git?: boolean } = {}): Promise { + const root = await mkdtemp(join(tmpdir(), "devspace-server-test-")); + const project = join(root, "project"); + const agentDir = join(root, "agent"); + const stateDir = join(root, ".state"); + + await mkdir(join(project, ".devspace", "agents"), { recursive: true }); + await mkdir(agentDir, { recursive: true }); + await writeFile(join(agentDir, "AGENTS.md"), "global instructions\n"); + await writeFile(join(project, "AGENTS.md"), "project instructions\n"); + await writeFile(join(project, ".devspace", "agents", "reviewer.md"), [ + "---", + "name: reviewer", + "description: Reviews project changes.", + "provider: codex", + "---", + "Review changes.", + ].join("\n")); + + if (options.git) { + await writeFile(join(project, "README.md"), "hello\n"); + await git(project, ["init"]); + await git(project, ["config", "user.email", "devspace@example.com"]); + await git(project, ["config", "user.name", "DevSpace Test"]); + await git(project, ["add", "."]); + await git(project, ["commit", "-m", "Initial commit"]); + } + + const config = loadConfig({ + DEVSPACE_CONFIG_DIR: join(root, ".config"), + DEVSPACE_ALLOWED_ROOTS: root, + DEVSPACE_WORKTREE_ROOT: join(root, ".worktrees"), + DEVSPACE_AGENT_DIR: agentDir, + DEVSPACE_WIDGETS: "full", + DEVSPACE_TOOL_MODE: "full", + DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", + PORT: "1", + }); + const store = new SqliteWorkspaceStore(stateDir); + const workspaces = new WorkspaceRegistry(config, store); + const server = createMcpServer( + config, + workspaces, + createReviewCheckpointManager(), + new ProcessSessionManager(), + [], + [], + ); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "devspace-test-client", version: "1.0.0" }); + await Promise.all([ + client.connect(clientTransport), + server.connect(serverTransport), + ]); + + let closed = false; + const close = async () => { + if (closed) return; + closed = true; + await client.close(); + await server.close(); + store.close(); + }; + + t.after(async () => { + await close(); + await rm(root, { recursive: true, force: true }); + }); + + return { client, project, config, stateDir, close }; +} + +async function git(cwd: string, args: string[]): Promise { + await execFileAsync("git", args, { cwd }); +} + +async function callOpen( + client: Client, + path: string, + conversationScopeId?: string, + mode?: "checkout" | "worktree", +): Promise>> { + const params = { + name: "open_workspace", + arguments: { + path, + ...(mode ? { mode } : {}), + }, + ...(conversationScopeId + ? { _meta: { "openai/session": conversationScopeId } } + : {}), + } as Parameters[0]; + return client.callTool(params); +} + +function structuredContent(result: Awaited>): Record { + assert.ok(result.structuredContent); + return result.structuredContent as Record; +} + +function responseText(result: Awaited>): string { + const content = (result as { content?: unknown }).content; + assert.ok(Array.isArray(content)); + const first = content[0] as { type?: unknown; text?: unknown } | undefined; + assert.equal(first?.type, "text"); + assert.equal(typeof first?.text, "string"); + return first?.text as string; +} + +function responseCard(result: Awaited>): Record { + const metadata = result._meta; + assert.ok(metadata && typeof metadata === "object"); + const card = (metadata as Record).card; + assert.ok(card && typeof card === "object"); + return card as Record; +} diff --git a/src/server.ts b/src/server.ts index 91f5df61..bfb8fb4f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -50,6 +50,7 @@ import { } from "./mcp-sessions.js"; import { ProcessSessionManager, type ProcessSnapshot } from "./process-sessions.js"; import { createReviewCheckpointManager } from "./review-checkpoints.js"; +import { openAiConversationScopeId } from "./request-meta.js"; import { shutdownHttpServer } from "./server-shutdown.js"; import { formatPathForPrompt } from "./skills.js"; import { createWorkspaceStore } from "./workspace-store.js"; @@ -196,7 +197,7 @@ function serverInstructions(config: ServerConfig): string { : ""; if (config.toolMode === "codex") { - return `Use DevSpace as a local coding workspace. Call ${toolNames.openWorkspace} once per project folder or worktree and reuse its workspaceId. Use ${toolNames.read} for direct file reads, apply_patch for all file modifications, exec_command for inspection, tests, builds, and other commands, and write_stdin to poll or interact with running processes. Follow instructions returned by ${toolNames.openWorkspace}; read applicable instruction and skill files before working in their scope.${artifactInstruction}${showChangesInstruction}`; + return `Use DevSpace as a local coding workspace. Call ${toolNames.openWorkspace} once per project folder or worktree and reuse its workspaceId. Open it again when the workspaceId is invalid, the project changes, checkout/worktree mode changes, or another isolated worktree is needed. Use ${toolNames.read} for direct file reads, apply_patch for all file modifications, exec_command for inspection, tests, builds, and other commands, and write_stdin to poll or interact with running processes. Follow instructions returned by ${toolNames.openWorkspace}; read applicable instruction and skill files before working in their scope.${artifactInstruction}${showChangesInstruction}`; } const inspection = config.toolMode !== "full" @@ -209,7 +210,7 @@ function serverInstructions(config: ServerConfig): string { const agentsMd = `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. `; - return `Use DevSpace as a local coding workspace. Call ${toolNames.openWorkspace} once per project folder or worktree to obtain a workspaceId. Reuse that same workspaceId for all later file, search, edit, write, show-changes, and shell tools in that folder; do not call ${toolNames.openWorkspace} again unless switching folders/worktrees, changing checkout/worktree mode, the workspaceId is rejected as unknown, or the user explicitly asks to reopen. ${agentsMd}${skills}${inspection}Prefer ${toolNames.edit} for targeted modifications, ${toolNames.write} only for new files or complete rewrites, and ${toolNames.shell} for tests, builds, git inspection, package scripts, and commands that are better executed by the shell. Do not create or modify files with ${toolNames.shell}; avoid shell redirection, heredocs, tee, sed -i, perl -i, node/python/ruby scripts, or any command whose purpose is to write project files.${artifactInstruction}${showChangesInstruction}`; + return `Use DevSpace as a local coding workspace. Call ${toolNames.openWorkspace} once per project folder or worktree to obtain a workspaceId. Reuse that same workspaceId for all later file, search, edit, write, show-changes, and shell tools in that folder; do not call ${toolNames.openWorkspace} again unless switching to a different project folder, changing checkout/worktree mode, the workspaceId is rejected as unknown, or a new isolated worktree is requested. ${agentsMd}${skills}${inspection}Prefer ${toolNames.edit} for targeted modifications, ${toolNames.write} only for new files or complete rewrites, and ${toolNames.shell} for tests, builds, git inspection, package scripts, and commands that are better executed by the shell. Do not create or modify files with ${toolNames.shell}; avoid shell redirection, heredocs, tee, sed -i, perl -i, node/python/ruby scripts, or any command whose purpose is to write project files.${artifactInstruction}${showChangesInstruction}`; } function formatVisibleAgent(agent: { @@ -693,7 +694,7 @@ function registerCodexProcessTools( ); } -function createMcpServer( +export function createMcpServer( config: ServerConfig, workspaces: WorkspaceRegistry, reviewCheckpoints: ReturnType, @@ -751,7 +752,7 @@ function createMcpServer( { title: "Open workspace", description: - "Open a local project directory as a coding workspace. Call this once per project folder or worktree before reading, editing, searching, writing, showing changes, or running commands. Reuse the returned workspaceId for later calls in the same folder; do not call open_workspace again unless switching folders/worktrees, changing checkout/worktree mode, the workspaceId is rejected as unknown, or the user explicitly asks to reopen. By default this opens the actual checkout; set mode=\"worktree\" when the user asks for an isolated or parallel coding session. Returns a workspaceId, loaded root project instructions, and nested instruction file paths the model should read before working in those directories.", + "Open a local project directory as a coding workspace. Call this once before working in a project or worktree, then reuse the returned workspaceId for later file, search, edit, show-changes, and shell calls. By default this opens the actual checkout; set mode=\"worktree\" when you need isolated or parallel work. Open another workspace when changing projects, switching modes, or starting another isolated worktree.", inputSchema: { path: z .string() @@ -762,7 +763,7 @@ function createMcpServer( .enum(["checkout", "worktree"]) .optional() .describe( - "Defaults to checkout. Use checkout to work in the actual directory. Use worktree to create an isolated managed Git worktree for parallel work.", + "Defaults to checkout, which works in the actual directory. Use worktree for isolated or parallel Git work.", ), baseRef: z .string() @@ -784,58 +785,85 @@ function createMcpServer( managed: z.boolean(), }) .optional(), - agentsFiles: z.array(workspaceAgentsFileOutputSchema), - availableAgentsFiles: z.array(workspaceAvailableAgentsFileOutputSchema), - skills: z.array(workspaceSkillOutputSchema), - agentProviders: z.array(workspaceLocalAgentProviderOutputSchema), - agents: z.array(workspaceLocalAgentOutputSchema), - skillDiagnostics: z.array(z.unknown()), + agentsFiles: z.array(workspaceAgentsFileOutputSchema).optional(), + availableAgentsFiles: z.array(workspaceAvailableAgentsFileOutputSchema).optional(), + skills: z.array(workspaceSkillOutputSchema).optional(), + agentProviders: z.array(workspaceLocalAgentProviderOutputSchema).optional(), + agents: z.array(workspaceLocalAgentOutputSchema).optional(), + skillDiagnostics: z.array(z.unknown()).optional(), instruction: z.string(), }, ...toolWidgetDescriptorMeta(config, "workspace"), annotations: { readOnlyHint: true }, }, - async ({ path, mode, baseRef }) => { + async ({ path, mode, baseRef }, { _meta }) => { const startedAt = performance.now(); - const { workspace, agentsFiles, availableAgentsFiles } = await workspaces.openWorkspace({ path, mode, baseRef }); + const { + workspace, + agentsFiles, + availableAgentsFiles, + workspaceReused, + includeBootstrapContext, + } = await workspaces.openWorkspace( + { path, mode, baseRef }, + { conversationScopeId: openAiConversationScopeId(_meta) }, + ); if (config.widgets === "changes") { - void reviewCheckpoints.initializeWorkspace({ + await reviewCheckpoints.initializeWorkspace({ workspaceId: workspace.id, root: workspace.root, }); } - const visibleSkills = workspace.skills + const cardSkills = workspace.skills .filter((skill) => !skill.disableModelInvocation) .map((skill) => ({ name: skill.name, description: skill.description, path: formatPathForPrompt(skill.filePath), })); - const visibleAgentProviders = config.subagents ? localAgentProviders : []; - const visibleAgents = workspace.agentProfiles.map((profile) => { + const cardAgentProviders = config.subagents ? localAgentProviders : []; + const cardAgents = workspace.agentProfiles.map((profile) => { const summary = summarizeLocalAgentProfile(profile); - const availability = visibleAgentProviders.find((provider) => provider.name === summary.provider); + const availability = cardAgentProviders.find((provider) => provider.name === summary.provider); return { ...summary, providerAvailable: availability?.available, providerUnavailableReason: availability?.reason, }; }); - const loadedAgentsFiles = agentsFiles.map((file) => ({ + const cardAgentsFiles = agentsFiles.map((file) => ({ path: formatAgentsPath(file.path, workspace.root), content: file.content, })); - const availableAgentsFileOutputs = availableAgentsFiles.map((file) => ({ + const cardAvailableAgentsFiles = availableAgentsFiles.map((file) => ({ path: formatAgentsPath(file.path, workspace.root), })); - const instruction = config.skillsEnabled - ? "Use this workspaceId in all subsequent tool calls for this project. Do not call open_workspace again for this same folder unless this workspaceId stops working, the user asks to reopen, or you switch to a different folder/worktree. Follow loaded agentsFiles instructions. Before working under a path listed in availableAgentsFiles, read that instruction file. When a task matches an available skill in skills, read its path before proceeding." - : "Use this workspaceId in all subsequent tool calls for this project. Do not call open_workspace again for this same folder unless this workspaceId stops working, the user asks to reopen, or you switch to a different folder/worktree. Follow loaded agentsFiles instructions. Before working under a path listed in availableAgentsFiles, read that instruction file."; + const visibleSkills = includeBootstrapContext ? cardSkills : []; + const visibleAgentProviders = includeBootstrapContext ? cardAgentProviders : []; + const visibleAgents = includeBootstrapContext ? cardAgents : []; + const loadedAgentsFiles = includeBootstrapContext ? cardAgentsFiles : []; + const availableAgentsFileOutputs = includeBootstrapContext ? cardAvailableAgentsFiles : []; + const cardInstruction = config.skillsEnabled + ? "Use this workspaceId in all subsequent tool calls for this project. Do not call open_workspace again for this same folder unless this workspaceId stops working, you switch to a different project folder or checkout/worktree mode, or the user requests a new isolated worktree. Follow loaded agentsFiles instructions. Before working under a path listed in availableAgentsFiles, read that instruction file. When a task matches an available skill in skills, read its path before proceeding." + : "Use this workspaceId in all subsequent tool calls for this project. Do not call open_workspace again for this same folder unless this workspaceId stops working, you switch to a different project folder or checkout/worktree mode, or the user requests a new isolated worktree. Follow loaded agentsFiles instructions. Before working under a path listed in availableAgentsFiles, read that instruction file."; + const instruction = workspaceReused + ? [ + `Workspace already open as ${workspace.id}.`, + "Reuse this workspaceId for subsequent tool calls. This is the same checkout previously opened for this project in this conversation.", + "Continue following the project instructions, nested instruction files, skills, agent profiles, and diagnostics previously provided for this workspace. They remain the active workspace context and are not repeated here.", + ].join("\n\n") + : workspace.mode === "worktree" + ? "Use this workspaceId for subsequent tool calls. Follow the project instructions, nested instruction files, skills, agent profiles, and diagnostics returned for this isolated worktree." + : cardInstruction; const resultContent: ToolContent[] = [ { type: "text" as const, text: [ - `Opened workspace ${workspace.id}`, + workspaceReused + ? `Workspace already open as ${workspace.id}.` + : workspace.mode === "worktree" + ? `Opened isolated worktree workspace ${workspace.id}.` + : `Opened workspace ${workspace.id}.`, `Root: ${workspace.root}`, `Mode: ${workspace.mode}`, loadedAgentsFiles.length > 0 @@ -876,13 +904,25 @@ function createMcpServer( workspaceId: workspace.id, root: workspace.root, path: workspace.root, + mode: workspace.mode, + workspaceReused, + includeBootstrapContext, + sourceRoot: workspace.sourceRoot, + worktree: workspace.worktree, + agentsFiles: cardAgentsFiles, + availableAgentsFiles: cardAvailableAgentsFiles, + skills: cardSkills, + agentProviders: cardAgentProviders, + agents: cardAgents, + skillDiagnostics: workspace.skillDiagnostics, + instruction: cardInstruction, summary: { mode: workspace.mode, - agentsFiles: loadedAgentsFiles.length, - availableAgentsFiles: availableAgentsFileOutputs.length, - skills: visibleSkills.length, - agentProviders: visibleAgentProviders.length, - agents: visibleAgents.length, + agentsFiles: cardAgentsFiles.length, + availableAgentsFiles: cardAvailableAgentsFiles.length, + skills: cardSkills.length, + agentProviders: cardAgentProviders.length, + agents: cardAgents.length, skillDiagnostics: workspace.skillDiagnostics.length, }, }, @@ -893,12 +933,16 @@ function createMcpServer( mode: workspace.mode, sourceRoot: workspace.sourceRoot, worktree: workspace.worktree, - agentsFiles: loadedAgentsFiles, - availableAgentsFiles: availableAgentsFileOutputs, - skills: visibleSkills, - agentProviders: visibleAgentProviders, - agents: visibleAgents, - skillDiagnostics: workspace.skillDiagnostics, + ...(includeBootstrapContext + ? { + agentsFiles: loadedAgentsFiles, + availableAgentsFiles: availableAgentsFileOutputs, + skills: visibleSkills, + agentProviders: visibleAgentProviders, + agents: visibleAgents, + skillDiagnostics: workspace.skillDiagnostics, + } + : {}), instruction, }, }; @@ -1250,7 +1294,7 @@ function createMcpServer( { title: "Show changes", description: - "Show aggregate file changes for an open workspace. If the current turn successfully modified files, call this exactly once after the final related file change and before your final response so the user can inspect the combined diff for the turn. Do not call it after every individual file change, and do not skip it because prior file-change tools already displayed per-tool diffs.", + "Show the changes made in this turn for an open workspace. Call this once after the final related file change and before your final response so the user can review the combined diff. Do not call it after each individual file change.", inputSchema: { workspaceId: z .string() @@ -1266,7 +1310,6 @@ function createMcpServer( const review = await reviewCheckpoints.reviewChanges({ workspaceId, root: workspace.root, - since: "last_shown", markReviewed: true, }); diff --git a/src/ui/card-types.test.ts b/src/ui/card-types.test.ts index eb47e9a0..eb16ad87 100644 --- a/src/ui/card-types.test.ts +++ b/src/ui/card-types.test.ts @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import test from "node:test"; import { isEditTool, isExpandableCard, @@ -7,19 +8,59 @@ import { isToolName, } from "./card-types.js"; -for (const tool of ["apply_patch", "exec_command", "write_stdin"]) { - assert.equal(isToolName(tool), true, `${tool} should be a recognized card tool`); -} +test("the supported coding tools are recognized as card tools", () => { + for (const tool of ["apply_patch", "exec_command", "write_stdin"]) { + assert.equal(isToolName(tool), true, `${tool} should be a recognized card tool`); + } +}); -assert.equal(isPatchTool("apply_patch"), true); -assert.equal(isEditTool("apply_patch"), false); -assert.equal(isShellTool("exec_command"), true); -assert.equal(isShellTool("write_stdin"), true); -assert.equal(isEditTool("exec_command"), false); -assert.equal(isShellTool("apply_patch"), false); +test("tool classification distinguishes patch, edit, and shell operations", () => { + assert.equal(isPatchTool("apply_patch"), true); + assert.equal(isEditTool("apply_patch"), false); + assert.equal(isShellTool("apply_patch"), false); + assert.equal(isShellTool("exec_command"), true); + assert.equal(isShellTool("write_stdin"), true); + assert.equal(isEditTool("exec_command"), false); +}); -assert.equal( - isExpandableCard({ tool: "apply_patch", payload: { patch: "diff --git a/a b/a" } }), - true, -); -assert.equal(isExpandableCard({ tool: "apply_patch" }), false); +test("a patch card expands only when it contains patch content", () => { + assert.equal( + isExpandableCard({ tool: "apply_patch", payload: { patch: "diff --git a/a b/a" } }), + true, + ); + assert.equal(isExpandableCard({ tool: "apply_patch" }), false); +}); + +test("a workspace card expands when it contains provider metadata", () => { + assert.equal( + isExpandableCard({ + tool: "open_workspace", + agentProviders: [{ name: "codex", available: true }], + }), + true, + ); +}); + +test("a workspace card expands when it contains agent metadata", () => { + assert.equal( + isExpandableCard({ + tool: "open_workspace", + agents: [{ name: "reviewer", provider: "codex" }], + }), + true, + ); +}); + +test("a workspace card expands when it contains available instruction files", () => { + assert.equal( + isExpandableCard({ + tool: "open_workspace", + availableAgentsFiles: [{ path: "nested/AGENTS.md" }], + }), + true, + ); +}); + +test("an empty workspace card stays collapsed", () => { + assert.equal(isExpandableCard({ tool: "open_workspace" }), false); +}); diff --git a/src/ui/card-types.ts b/src/ui/card-types.ts index 1e3c9409..596e2e84 100644 --- a/src/ui/card-types.ts +++ b/src/ui/card-types.ts @@ -23,6 +23,18 @@ export interface ToolResultCard { workspaceId?: string; path?: string; root?: string; + workspaceReused?: boolean; + includeBootstrapContext?: boolean; + mode?: "checkout" | "worktree"; + sourceRoot?: string; + worktree?: { + path?: string; + baseRef?: string; + baseSha?: string; + dirtySource?: boolean; + detached?: boolean; + managed?: boolean; + }; status?: string; summary?: Record; files?: Array<{ @@ -46,6 +58,20 @@ export interface ToolResultCard { description?: string; path?: string; }>; + agentProviders?: Array<{ + name?: string; + available?: boolean; + reason?: string; + }>; + agents?: Array<{ + name?: string; + description?: string; + provider?: string; + model?: string; + thinking?: string; + providerAvailable?: boolean; + providerUnavailableReason?: string; + }>; skillDiagnostics?: unknown[]; instruction?: string; } @@ -137,10 +163,16 @@ export function isExpandableCard(card: ToolResultCard): boolean { return ( Number(card.summary?.agentsFiles ?? 0) > 0 || Number(card.summary?.skills ?? 0) > 0 || + Number(card.summary?.agentProviders ?? 0) > 0 || + Number(card.summary?.agents ?? 0) > 0 || Number(card.summary?.skillDiagnostics ?? 0) > 0 || Boolean(card.agentsFiles?.length) || Boolean(card.availableAgentsFiles?.length) || Boolean(card.skills?.length) || + Boolean(card.agentProviders?.length) || + Boolean(card.agents?.length) || + Boolean(card.worktree) || + Boolean(card.instruction) || Boolean(card.skillDiagnostics?.length) ); } diff --git a/src/ui/tool-display.test.ts b/src/ui/tool-display.test.ts index b9977ac8..86b67271 100644 --- a/src/ui/tool-display.test.ts +++ b/src/ui/tool-display.test.ts @@ -5,6 +5,8 @@ import { getToolDisplay, getToolHeaderSummary } from "./tool-display.js"; const displayCases: Array<[ToolResultCard, { title: string; tone: string }]> = [ [{ tool: "open_workspace", root: "/tmp/project" }, { title: "Opened workspace", tone: "workspace" }], + [{ tool: "open_workspace", root: "/tmp/project", workspaceReused: true }, { title: "Reused workspace", tone: "workspace" }], + [{ tool: "open_workspace", root: "/tmp/project", mode: "worktree" }, { title: "Opened worktree", tone: "workspace" }], [{ tool: "read", path: "src/read.ts" }, { title: "Read file", tone: "read" }], [{ tool: "write", path: "src/write.ts" }, { title: "Wrote file", tone: "write" }], [{ tool: "edit", path: "src/edit.ts" }, { title: "Edited file", tone: "edit" }], diff --git a/src/ui/tool-display.ts b/src/ui/tool-display.ts index 7a847631..f9706690 100644 --- a/src/ui/tool-display.ts +++ b/src/ui/tool-display.ts @@ -27,7 +27,11 @@ export function getToolDisplay(card: ToolResultCard): ToolDisplay { case "open_workspace": return { icon: toolIcons.folderOpen, - title: "Opened workspace", + title: card.workspaceReused + ? "Reused workspace" + : card.mode === "worktree" + ? "Opened worktree" + : "Opened workspace", label: card.root ?? card.path, tone: "workspace", }; @@ -197,4 +201,3 @@ function durationLabel(durationMs: number | undefined): string | undefined { if (durationMs < 1_000) return `${Math.round(durationMs)}ms`; return `${(durationMs / 1_000).toFixed(durationMs < 10_000 ? 1 : 0)}s`; } - diff --git a/src/ui/workspace-app.tsx b/src/ui/workspace-app.tsx index 78723864..da32cd32 100644 --- a/src/ui/workspace-app.tsx +++ b/src/ui/workspace-app.tsx @@ -449,23 +449,80 @@ function workspacePayloadText(card: ToolResultCard): string { const agentsFiles = card.agentsFiles ?? []; const availableAgentsFiles = card.availableAgentsFiles ?? []; const skills = card.skills ?? []; + const agentProviders = card.agentProviders ?? []; + const agents = card.agents ?? []; + const diagnostics = card.skillDiagnostics ?? []; const lines = [ card.workspaceId ? `Workspace: ${card.workspaceId}` : undefined, card.root ? `Root: ${card.root}` : undefined, + card.mode ? `Mode: ${card.mode}` : undefined, + card.sourceRoot ? `Source root: ${card.sourceRoot}` : undefined, + card.worktree ? formatWorktree(card.worktree) : undefined, skills.length > 0 ? `Skills: ${skills.map((skill) => skill.name ?? skill.path ?? "unnamed").join(", ")}` : "Skills: none", + agentProviders.length > 0 + ? `Agent providers: ${agentProviders.map(formatAgentProvider).join(", ")}` + : undefined, + agents.length > 0 + ? `Agents: ${agents.map(formatAgent).join(", ")}` + : undefined, + diagnostics.length > 0 + ? `Skill diagnostics: ${diagnostics.map(formatDiagnostic).join("; ")}` + : undefined, availableAgentsFiles.length > 0 ? `Nested instructions: ${availableAgentsFiles.map((file) => file.path ?? "unknown").join(", ")}` : undefined, agentsFiles.length > 0 ? `\n${formatAgentsFilesForPayload(agentsFiles)}` : "\nAGENTS.md: none loaded", + card.instruction ? `\nInstruction: ${card.instruction}` : undefined, ].filter((line): line is string => typeof line === "string"); return lines.join("\n"); } +function formatWorktree(worktree: NonNullable): string { + const details = [ + worktree.baseRef ? `base ${worktree.baseRef}` : undefined, + worktree.baseSha ? `at ${worktree.baseSha.slice(0, 12)}` : undefined, + worktree.managed === true ? "managed" : undefined, + worktree.detached === true ? "detached" : undefined, + worktree.dirtySource === true ? "dirty source" : undefined, + ].filter((detail): detail is string => Boolean(detail)); + return `Worktree: ${worktree.path ?? "unknown"}${details.length > 0 ? ` (${details.join(", ")})` : ""}`; +} + +function formatAgentProvider( + provider: NonNullable[number], +): string { + const name = provider.name ?? "unknown"; + if (provider.available !== false) return name; + return provider.reason ? `${name} unavailable: ${provider.reason}` : `${name} unavailable`; +} + +function formatAgent(agent: NonNullable[number]): string { + const details = [ + agent.provider, + agent.model, + agent.thinking ? `thinking ${agent.thinking}` : undefined, + agent.providerAvailable === false + ? agent.providerUnavailableReason ?? "provider unavailable" + : undefined, + ].filter((detail): detail is string => Boolean(detail)); + return `${agent.name ?? "unnamed"}${details.length > 0 ? ` (${details.join(", ")})` : ""}`; +} + +function formatDiagnostic(diagnostic: unknown): string { + if (typeof diagnostic === "string") return diagnostic; + if (diagnostic instanceof Error) return diagnostic.message; + try { + return JSON.stringify(diagnostic) ?? String(diagnostic); + } catch { + return String(diagnostic); + } +} + function formatAgentsFilesForPayload( agentsFiles: NonNullable, ): string { diff --git a/src/workspace-conversation.test.ts b/src/workspace-conversation.test.ts new file mode 100644 index 00000000..5af9f991 --- /dev/null +++ b/src/workspace-conversation.test.ts @@ -0,0 +1,480 @@ +import { execFile } from "node:child_process"; +import assert from "node:assert/strict"; +import { mkdtemp, mkdir, realpath, rename, rm, stat, symlink, writeFile } from "node:fs/promises"; +import { platform, tmpdir } from "node:os"; +import { join } from "node:path"; +import test, { type TestContext } from "node:test"; +import { promisify } from "node:util"; +import { loadConfig, type ServerConfig } from "./config.js"; +import { openDatabase } from "./db/client.js"; +import { SqliteWorkspaceStore } from "./workspace-store.js"; +import { WorkspaceRegistry } from "./workspaces.js"; + +const execFileAsync = promisify(execFile); + +test("a conversation reuses its checkout context", async (t) => { + const { project, registry } = await fixture(t); + + const first = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + const second = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + + assert.equal(second.workspace.id, first.workspace.id); + assert.deepEqual(second.agentsFiles, first.agentsFiles); + assert.deepEqual(second.availableAgentsFiles, first.availableAgentsFiles); + assert.deepEqual(second.workspace.skills, first.workspace.skills); + assert.deepEqual(second.workspace.skillDiagnostics, first.workspace.skillDiagnostics); + assert.deepEqual(second.workspace.agentProfiles, first.workspace.agentProfiles); +}); + +test("different conversations receive separate checkout workspaces", async (t) => { + const { project, registry } = await fixture(t); + + const first = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + const second = await registry.openWorkspace(project, { conversationScopeId: "chat-2" }); + + assert.notEqual(second.workspace.id, first.workspace.id); +}); + +test("conversation bindings distinguish canonical projects", async (t) => { + const { root, project, registry } = await fixture(t); + const otherProject = join(root, "other-project"); + await mkdir(otherProject); + await writeFile(join(otherProject, "AGENTS.md"), "other project instructions\n"); + + const firstProjectOpen = await registry.openWorkspace(project, { + conversationScopeId: "chat-1", + }); + const otherProjectOpen = await registry.openWorkspace(otherProject, { + conversationScopeId: "chat-1", + }); + const repeatedProjectOpen = await registry.openWorkspace(project, { + conversationScopeId: "chat-1", + }); + const repeatedOtherProjectOpen = await registry.openWorkspace(otherProject, { + conversationScopeId: "chat-1", + }); + + assert.equal(repeatedProjectOpen.workspace.id, firstProjectOpen.workspace.id); + assert.equal(repeatedOtherProjectOpen.workspace.id, otherProjectOpen.workspace.id); + assert.notEqual(otherProjectOpen.workspace.id, firstProjectOpen.workspace.id); +}); + +test("concurrent checkout opens reuse one workspace and return matching context", async (t) => { + const { project, registry } = await fixture(t); + + const opens = await Promise.all([ + registry.openWorkspace(project, { conversationScopeId: "chat-1" }), + registry.openWorkspace(project, { conversationScopeId: "chat-1" }), + ]); + + assert.equal(new Set(opens.map((open) => open.workspace.id)).size, 1); + assert.deepEqual(opens[0].agentsFiles, opens[1].agentsFiles); + assert.deepEqual(opens[0].availableAgentsFiles, opens[1].availableAgentsFiles); +}); + +test("a checkout without a conversation scope does not use conversation reuse", async (t) => { + const { project, registry } = await fixture(t); + + const first = await registry.openWorkspace(project); + const second = await registry.openWorkspace(project); + + assert.notEqual(second.workspace.id, first.workspace.id); +}); + +test("worktree requests remain fresh without replacing the reusable checkout", async (t) => { + const { project, registry } = await fixture(t, { git: true }); + const worktreeInput = { path: project, mode: "worktree" as const }; + + const checkout = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + const firstWorktree = await registry.openWorkspace(worktreeInput, { + conversationScopeId: "chat-1", + }); + const secondWorktree = await registry.openWorkspace(worktreeInput, { + conversationScopeId: "chat-1", + }); + const checkoutAgain = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + + assert.notEqual(firstWorktree.workspace.id, secondWorktree.workspace.id); + assert.notEqual(firstWorktree.workspace.root, secondWorktree.workspace.root); + assert.equal(checkoutAgain.workspace.id, checkout.workspace.id); +}); + +test("a worktree-first conversation creates and then reuses its checkout", async (t) => { + const { project, registry } = await fixture(t, { git: true }); + const worktreeInput = { path: project, mode: "worktree" as const }; + + const worktree = await registry.openWorkspace(worktreeInput, { + conversationScopeId: "chat-1", + }); + const checkout = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + const checkoutAgain = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + + assert.equal(checkout.workspace.mode, "checkout"); + assert.notEqual(checkout.workspace.id, worktree.workspace.id); + assert.equal(checkoutAgain.workspace.id, checkout.workspace.id); +}); + +test("concurrent worktree opens remain fresh and return complete context", async (t) => { + const { project, registry } = await fixture(t, { git: true }); + const worktreeInput = { path: project, mode: "worktree" as const }; + + const [first, second] = await Promise.all([ + registry.openWorkspace(worktreeInput, { conversationScopeId: "chat-1" }), + registry.openWorkspace(worktreeInput, { conversationScopeId: "chat-1" }), + ]); + + assert.notEqual(first.workspace.id, second.workspace.id); + assert.notEqual(first.workspace.root, second.workspace.root); + assert.deepEqual( + first.agentsFiles.map((file) => file.content), + second.agentsFiles.map((file) => file.content), + ); + assert.deepEqual( + first.availableAgentsFiles.map((file) => file.path.replace(first.workspace.root, "")), + second.availableAgentsFiles.map((file) => file.path.replace(second.workspace.root, "")), + ); +}); + +test("checkout reuse survives a registry restart", async (t) => { + const context = await fixture(t); + const first = await context.registry.openWorkspace(context.project, { + conversationScopeId: "chat-1", + }); + context.closeStore(context.store); + + const restoredStore = context.openStore(); + const restoredRegistry = new WorkspaceRegistry(context.config, restoredStore); + const restored = await restoredRegistry.openWorkspace(context.project, { + conversationScopeId: "chat-1", + }); + + assert.equal(restored.workspace.id, first.workspace.id); +}); + +test("a failed first context load does not consume bootstrap", async (t) => { + const { project, registry } = await fixture(t); + const agentsDir = join(project, ".devspace", "agents"); + const backupDir = join(project, ".devspace", "agents-backup"); + + await breakAgentsDirectory(agentsDir, backupDir); + try { + await assert.rejects( + () => registry.openWorkspace(project, { conversationScopeId: "chat-1" }), + /directory|ENOTDIR/i, + ); + } finally { + await restoreAgentsDirectory(agentsDir, backupDir); + } + + const successfulOpen = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); +}); + +test("a context-loading failure preserves a valid checkout binding", async (t) => { + const { project, registry } = await fixture(t); + const first = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + const agentsDir = join(project, ".devspace", "agents"); + const backupDir = join(project, ".devspace", "agents-backup"); + + await breakAgentsDirectory(agentsDir, backupDir); + try { + await assert.rejects( + () => registry.openWorkspace(project, { conversationScopeId: "chat-1" }), + /directory|ENOTDIR/i, + ); + } finally { + await restoreAgentsDirectory(agentsDir, backupDir); + } + + const recovered = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + assert.equal(recovered.workspace.id, first.workspace.id); +}); + +test("a deleted checkout is replaced with a new workspace", async (t) => { + const { project, registry } = await fixture(t); + const first = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + + await rm(project, { recursive: true, force: true }); + const replacement = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + + assert.notEqual(replacement.workspace.id, first.workspace.id); + assert.equal((await stat(project)).isDirectory(), true); +}); + +test("canonical checkout identity remains stable when the requested target starts missing", async (t) => { + const { project, registry } = await fixture(t); + const missingTarget = join(project, "generated", "checkout"); + + const first = await registry.openWorkspace(missingTarget, { conversationScopeId: "chat-1" }); + const second = await registry.openWorkspace(missingTarget, { conversationScopeId: "chat-1" }); + + assert.equal(first.workspace.root, missingTarget); + assert.equal(second.workspace.id, first.workspace.id); +}); + +test("canonical checkout identity survives equivalent path and symlink aliases", async (t) => { + const { root, project, registry } = await fixture(t); + + const direct = await registry.openWorkspace(project, { conversationScopeId: "chat-1" }); + const equivalent = await registry.openWorkspace(join(project, "..", "project"), { + conversationScopeId: "chat-1", + }); + + assert.equal(equivalent.workspace.id, direct.workspace.id); + + if (platform() === "win32") return; + + const alias = join(root, "project-alias"); + await symlink(project, alias, "dir"); + const aliased = await registry.openWorkspace(alias, { conversationScopeId: "chat-1" }); + + assert.equal(aliased.workspace.id, direct.workspace.id); +}); + +test("canonical checkout identity survives macOS var path aliases", { skip: platform() !== "darwin" }, async (t) => { + const context = await fixture(t); + const macAlias = context.root.startsWith("/private/var/") + ? `/var/${context.root.slice("/private/var/".length)}` + : context.root.startsWith("/var/") + ? `/private/var/${context.root.slice("/var/".length)}` + : undefined; + if (!macAlias) { + t.skip("temporary directory is not under /var"); + return; + } + + const aliasConfig = loadConfig({ + DEVSPACE_CONFIG_DIR: join(context.root, ".alias-config"), + DEVSPACE_ALLOWED_ROOTS: `${context.root},${macAlias}`, + DEVSPACE_WORKTREE_ROOT: join(context.root, ".worktrees"), + DEVSPACE_AGENT_DIR: join(context.root, "agent"), + DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", + PORT: "1", + }); + const aliasRegistry = new WorkspaceRegistry(aliasConfig, context.store); + + const direct = await context.registry.openWorkspace(context.project, { + conversationScopeId: "chat-1", + }); + const aliased = await aliasRegistry.openWorkspace( + `${macAlias}/${context.project.slice(context.root.length + 1)}`, + { conversationScopeId: "chat-1" }, + ); + + assert.equal(aliased.workspace.id, direct.workspace.id); +}); + +test("an invalid persisted checkout binding is not reused", async (t) => { + const context = await fixture(t); + const first = await context.registry.openWorkspace(context.project, { + conversationScopeId: "chat-1", + }); + context.closeStore(context.store); + + const database = openDatabase(context.stateDir); + try { + database.sqlite + .prepare("update workspace_sessions set mode = 'worktree' where id = ?") + .run(first.workspace.id); + } finally { + database.close(); + } + + const restoredStore = context.openStore(); + const restoredRegistry = new WorkspaceRegistry(context.config, restoredStore); + const replacement = await restoredRegistry.openWorkspace(context.project, { + conversationScopeId: "chat-1", + }); + + assert.notEqual(replacement.workspace.id, first.workspace.id); +}); + +test("an inactive persisted checkout binding is not reused", async (t) => { + const context = await fixture(t); + const first = await context.registry.openWorkspace(context.project, { + conversationScopeId: "chat-1", + }); + context.closeStore(context.store); + + const database = openDatabase(context.stateDir); + try { + database.sqlite + .prepare("update workspace_sessions set status = 'inactive' where id = ?") + .run(first.workspace.id); + } finally { + database.close(); + } + + const restoredRegistry = new WorkspaceRegistry(context.config, context.openStore()); + const replacement = await restoredRegistry.openWorkspace(context.project, { + conversationScopeId: "chat-1", + }); + + assert.notEqual(replacement.workspace.id, first.workspace.id); +}); + +test("a checkout replaced by a file reports the filesystem error", async (t) => { + const context = await fixture(t); + const target = join(context.root, "file-target"); + await context.registry.openWorkspace(target, { conversationScopeId: "chat-1" }); + await rm(target, { recursive: true, force: true }); + await writeFile(target, "not a directory\n"); + + await assert.rejects( + () => context.registry.openWorkspace(target, { conversationScopeId: "chat-1" }), + /Workspace root must be a directory/, + ); +}); + +test("unexpected storage errors are not mistaken for stale bindings", async (t) => { + const context = await fixture(t); + const first = await context.registry.openWorkspace(context.project, { conversationScopeId: "chat-1" }); + const targetKey = checkoutTargetKey(await realpath(context.project)); + context.closeStore(context.store); + + await assert.rejects( + () => context.registry.openWorkspace(context.project, { conversationScopeId: "chat-1" }), + ); + + const restoredStore = context.openStore(); + assert.equal( + restoredStore.getConversationBinding("chat-1", targetKey)?.workspaceSessionId, + first.workspace.id, + ); +}); + +test("unexpected filesystem errors are propagated without replacing the binding", { + skip: platform() === "win32", +}, async (t) => { + const context = await fixture(t); + const first = await context.registry.openWorkspace(context.project, { conversationScopeId: "chat-1" }); + const targetKey = checkoutTargetKey(await realpath(context.project)); + const loopA = join(context.root, "loop-a"); + const loopB = join(context.root, "loop-b"); + + await symlink(loopB, loopA, "dir"); + await symlink(loopA, loopB, "dir"); + context.closeStore(context.store); + + const database = openDatabase(context.stateDir); + try { + database.sqlite + .prepare("update workspace_sessions set root = ? where id = ?") + .run(loopA, first.workspace.id); + } finally { + database.close(); + } + + const restoredStore = context.openStore(); + const restoredRegistry = new WorkspaceRegistry(context.config, restoredStore); + await assert.rejects( + () => restoredRegistry.openWorkspace(context.project, { conversationScopeId: "chat-1" }), + (error: unknown) => + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ELOOP", + ); + + const binding = restoredStore.getConversationBinding("chat-1", targetKey); + assert.equal(binding?.workspaceSessionId, first.workspace.id); + assert.equal(restoredStore.getSession(first.workspace.id)?.root, loopA); +}); + +interface WorkspaceFixture { + root: string; + project: string; + stateDir: string; + config: ServerConfig; + store: SqliteWorkspaceStore; + registry: WorkspaceRegistry; + openStore: () => SqliteWorkspaceStore; + closeStore: (store: SqliteWorkspaceStore) => void; +} + +async function fixture( + t: TestContext, + options: { git?: boolean } = {}, +): Promise { + const root = await mkdtemp(join(tmpdir(), "devspace-workspace-conversation-test-")); + const project = join(root, "project"); + const agentDir = join(root, "agent"); + const stateDir = join(root, ".state"); + const stores = new Set(); + + await mkdir(join(project, ".devspace", "agents"), { recursive: true }); + await mkdir(agentDir, { recursive: true }); + await writeFile(join(agentDir, "AGENTS.md"), "global instructions\n"); + await writeFile(join(project, "AGENTS.md"), "project instructions\n"); + await writeFile(join(project, ".devspace", "agents", "reviewer.md"), [ + "---", + "name: reviewer", + "description: Reviews project changes.", + "provider: codex", + "---", + "Review changes.", + ].join("\n")); + + if (options.git) await initializeGitRepository(project); + + const config = loadConfig({ + DEVSPACE_CONFIG_DIR: join(root, ".config"), + DEVSPACE_ALLOWED_ROOTS: root, + DEVSPACE_WORKTREE_ROOT: join(root, ".worktrees"), + DEVSPACE_AGENT_DIR: agentDir, + DEVSPACE_SUBAGENTS: "1", + DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", + PORT: "1", + }); + const openStore = () => { + const store = new SqliteWorkspaceStore(stateDir); + stores.add(store); + return store; + }; + const closeStore = (store: SqliteWorkspaceStore) => { + if (stores.delete(store)) store.close(); + }; + const store = openStore(); + + t.after(async () => { + for (const openStore of stores) openStore.close(); + await rm(root, { recursive: true, force: true }); + }); + + return { + root, + project, + stateDir, + config, + store, + registry: new WorkspaceRegistry(config, store), + openStore, + closeStore, + }; +} + +async function breakAgentsDirectory(agentsDir: string, backupDir: string): Promise { + await rename(agentsDir, backupDir); + await writeFile(agentsDir, "not a directory\n"); +} + +async function restoreAgentsDirectory(agentsDir: string, backupDir: string): Promise { + await rm(agentsDir, { force: true }); + await rename(backupDir, agentsDir); +} + +async function initializeGitRepository(root: string): Promise { + await writeFile(join(root, "README.md"), "hello\n"); + await git(root, ["init"]); + await git(root, ["config", "user.email", "devspace@example.com"]); + await git(root, ["config", "user.name", "DevSpace Test"]); + await git(root, ["add", "."]); + await git(root, ["commit", "-m", "Initial commit"]); +} + +async function git(cwd: string, args: string[]): Promise { + await execFileAsync("git", args, { cwd }); +} + +function checkoutTargetKey(project: string): string { + return JSON.stringify(["checkout", project, null]); +} diff --git a/src/workspace-store.ts b/src/workspace-store.ts index 39c2ed09..88a70e2e 100644 --- a/src/workspace-store.ts +++ b/src/workspace-store.ts @@ -1,7 +1,9 @@ -import { eq } from "drizzle-orm"; +import { and, eq } from "drizzle-orm"; import { openDatabase, type DatabaseHandle } from "./db/client.js"; import { + workspaceConversationBindings, workspaceSessions, + type WorkspaceConversationBindingRow, type WorkspaceSessionRow, } from "./db/schema.js"; @@ -20,6 +22,14 @@ export interface WorkspaceSession { lastUsedAt: string; } +export interface WorkspaceConversationBinding { + conversationScopeId: string; + targetKey: string; + workspaceSessionId: string; + createdAt: string; + lastUsedAt: string; +} + export interface WorkspaceStore { createSession(input: { id: string; @@ -32,6 +42,17 @@ export interface WorkspaceStore { }): WorkspaceSession; getSession(id: string): WorkspaceSession | undefined; touchSession(id: string): void; + getConversationBinding( + conversationScopeId: string, + targetKey: string, + ): WorkspaceConversationBinding | undefined; + setConversationBinding(input: { + conversationScopeId: string; + targetKey: string; + workspaceSessionId: string; + }): WorkspaceConversationBinding; + touchConversationBinding(conversationScopeId: string, targetKey: string): void; + deleteConversationBinding(conversationScopeId: string, targetKey: string): void; close?(): void; } @@ -102,6 +123,84 @@ export class SqliteWorkspaceStore implements WorkspaceStore { .run(); } + getConversationBinding( + conversationScopeId: string, + targetKey: string, + ): WorkspaceConversationBinding | undefined { + const row = this.database.db + .select() + .from(workspaceConversationBindings) + .where( + and( + eq(workspaceConversationBindings.conversationScopeId, conversationScopeId), + eq(workspaceConversationBindings.targetKey, targetKey), + ), + ) + .get(); + + return row ? rowToWorkspaceConversationBinding(row) : undefined; + } + + setConversationBinding(input: { + conversationScopeId: string; + targetKey: string; + workspaceSessionId: string; + }): WorkspaceConversationBinding { + const now = new Date().toISOString(); + const row = this.database.db + .insert(workspaceConversationBindings) + .values({ + conversationScopeId: input.conversationScopeId, + targetKey: input.targetKey, + workspaceSessionId: input.workspaceSessionId, + createdAt: now, + lastUsedAt: now, + }) + .onConflictDoUpdate({ + target: [ + workspaceConversationBindings.conversationScopeId, + workspaceConversationBindings.targetKey, + ], + set: { + workspaceSessionId: input.workspaceSessionId, + lastUsedAt: now, + }, + }) + .returning() + .get(); + + if (!row) { + throw new Error("Conversation workspace binding upsert returned no row."); + } + + return rowToWorkspaceConversationBinding(row); + } + + touchConversationBinding(conversationScopeId: string, targetKey: string): void { + this.database.db + .update(workspaceConversationBindings) + .set({ lastUsedAt: new Date().toISOString() }) + .where( + and( + eq(workspaceConversationBindings.conversationScopeId, conversationScopeId), + eq(workspaceConversationBindings.targetKey, targetKey), + ), + ) + .run(); + } + + deleteConversationBinding(conversationScopeId: string, targetKey: string): void { + this.database.db + .delete(workspaceConversationBindings) + .where( + and( + eq(workspaceConversationBindings.conversationScopeId, conversationScopeId), + eq(workspaceConversationBindings.targetKey, targetKey), + ), + ) + .run(); + } + close(): void { this.database.close(); } @@ -126,3 +225,15 @@ function rowToWorkspaceSession(row: WorkspaceSessionRow): WorkspaceSession { lastUsedAt: row.lastUsedAt, }; } + +function rowToWorkspaceConversationBinding( + row: WorkspaceConversationBindingRow, +): WorkspaceConversationBinding { + return { + conversationScopeId: row.conversationScopeId, + targetKey: row.targetKey, + workspaceSessionId: row.workspaceSessionId, + createdAt: row.createdAt, + lastUsedAt: row.lastUsedAt, + }; +} diff --git a/src/workspaces.test.ts b/src/workspaces.test.ts index 4f3eb769..fac8fd81 100644 --- a/src/workspaces.test.ts +++ b/src/workspaces.test.ts @@ -1,28 +1,192 @@ +import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { mkdtemp, mkdir, rm, stat, symlink, writeFile } from "node:fs/promises"; import { platform, tmpdir } from "node:os"; import { join } from "node:path"; +import test, { type TestContext } from "node:test"; import { promisify } from "node:util"; -import assert from "node:assert/strict"; -import { loadConfig } from "./config.js"; +import { loadConfig, type ServerConfig } from "./config.js"; import { GitWorktreeError } from "./git-worktrees.js"; import { SqliteWorkspaceStore } from "./workspace-store.js"; -import { ensureCheckoutWorkspaceRoot, WorkspaceRegistry } from "./workspaces.js"; +import { WorkspaceRegistry } from "./workspaces.js"; const execFileAsync = promisify(execFile); -const root = await mkdtemp(join(tmpdir(), "devspace-workspace-test-")); -const outsideRoot = await mkdtemp(join(tmpdir(), "devspace-workspace-outside-test-")); -try { +test("a checkout exposes initial and nested instruction context while filtering outside symlinks", async (t) => { + const context = await fixture(t); + const opened = await context.registry.openWorkspace(context.root); + + assert.equal(opened.workspace.mode, "checkout"); + assert.deepEqual( + opened.agentsFiles.map((file) => file.content), + ["global instructions\n", "root instructions\n"], + ); + assert.deepEqual( + opened.availableAgentsFiles.map((file) => file.path), + [join(context.root, "nested", "AGENTS.md")], + ); + assert.deepEqual( + opened.workspace.agentProfiles.map((profile) => ({ + name: profile.name, + description: profile.description, + provider: profile.provider, + body: profile.body, + })), + [{ + name: "reviewer", + description: "Read-only project reviewer.", + provider: "codex", + body: "Review only.", + }], + ); + + if (platform() !== "win32") { + const unsafeAgentDir = join(context.root, ".pi", "unsafe-agent"); + await mkdir(unsafeAgentDir, { recursive: true }); + await writeFile(join(context.outsideRoot, "secret.txt"), "outside secret\n"); + await symlink(join(context.outsideRoot, "secret.txt"), join(unsafeAgentDir, "AGENTS.md")); + + const unsafeConfig = loadConfig({ + DEVSPACE_CONFIG_DIR: join(context.root, ".devspace-unsafe-home"), + DEVSPACE_ALLOWED_ROOTS: context.root, + DEVSPACE_WORKTREE_ROOT: join(context.root, ".devspace", "unsafe-worktrees"), + DEVSPACE_AGENT_DIR: unsafeAgentDir, + DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", + PORT: "1", + }); + const unsafeWorkspace = await new WorkspaceRegistry(unsafeConfig).openWorkspace(context.root); + + assert.deepEqual( + unsafeWorkspace.agentsFiles.map((file) => file.content), + ["root instructions\n"], + ); + } +}); + +test("opening a missing checkout creates its workspace root", async (t) => { + const context = await fixture(t); + const missingRoot = join(context.root, "missing", "workspace"); + + const opened = await context.registry.openWorkspace(missingRoot); + assert.equal(opened.workspace.root, missingRoot); + assert.equal((await stat(missingRoot)).isDirectory(), true); +}); + +test("worktree opens require Git and create an isolated managed workspace", async (t) => { + const context = await fixture(t); + + await assert.rejects( + () => context.registry.openWorkspace({ path: context.root, mode: "worktree" }), + (error: unknown) => + error instanceof GitWorktreeError && error.code === "GIT_REPOSITORY_NOT_FOUND", + ); + + const gitRoot = await createGitProject(context.root); + await writeFile(join(gitRoot, "dirty.txt"), "not copied\n"); + + const opened = await context.registry.openWorkspace({ path: gitRoot, mode: "worktree" }); + + assert.equal(opened.workspace.mode, "worktree"); + assert.notEqual(opened.workspace.root, gitRoot); + assert.equal(opened.workspace.sourceRoot, gitRoot); + assert.equal(opened.workspace.worktree?.baseRef, "HEAD"); + assert.equal(opened.workspace.worktree?.dirtySource, true); + assert.equal(opened.workspace.worktree?.managed, true); + assert.equal((await stat(opened.workspace.root)).isDirectory(), true); + assert.match(opened.agentsFiles.map((file) => file.content).join("\n"), /global instructions/); + assert.match(opened.agentsFiles.map((file) => file.content).join("\n"), /git root instructions/); + + const resolvedReadme = context.registry.resolvePath(opened.workspace, "README.md"); + assert.equal(resolvedReadme.startsWith(opened.workspace.root), true); +}); + +test("persisted checkout and worktree sessions restore after recreating the registry", async (t) => { + const context = await fixture(t); + const gitRoot = await createGitProject(context.root); + const stateDir = join(context.root, ".state"); + const firstStore = new SqliteWorkspaceStore(stateDir); + const firstRegistry = new WorkspaceRegistry(context.config, firstStore); + + const checkout = await firstRegistry.openWorkspace(context.root); + const worktree = await firstRegistry.openWorkspace({ path: gitRoot, mode: "worktree" }); + firstStore.close(); + + const secondStore = new SqliteWorkspaceStore(stateDir); + try { + const restoredRegistry = new WorkspaceRegistry(context.config, secondStore); + const restoredCheckout = restoredRegistry.getWorkspace(checkout.workspace.id); + const restoredWorktree = restoredRegistry.getWorkspace(worktree.workspace.id); + + assert.equal(restoredCheckout.root, context.root); + assert.equal(restoredCheckout.mode, "checkout"); + assert.equal(restoredWorktree.root, worktree.workspace.root); + assert.equal(restoredWorktree.mode, "worktree"); + assert.equal(restoredWorktree.sourceRoot, gitRoot); + assert.equal(restoredWorktree.worktree?.managed, true); + } finally { + secondStore.close(); + } +}); + +test("workspace paths outside the allowed roots are rejected", async (t) => { + const context = await fixture(t); + + await assert.rejects( + () => context.registry.openWorkspace(context.outsideRoot), + /outside allowed roots/, + ); +}); + +test("a symlinked allowed root preserves checkout and worktree path behavior", { skip: platform() === "win32" }, async (t) => { + const context = await fixture(t); + const aliasRoot = join(context.root, "alias-root"); + await symlink(context.root, aliasRoot, "dir"); + await createGitProject(context.root); + + const aliasConfig = loadConfig({ + DEVSPACE_ALLOWED_ROOTS: aliasRoot, + DEVSPACE_WORKTREE_ROOT: join(aliasRoot, ".devspace", "alias-worktrees"), + DEVSPACE_AGENT_DIR: context.agentDir, + DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", + PORT: "1", + }); + const aliasRegistry = new WorkspaceRegistry(aliasConfig); + + const worktree = await aliasRegistry.openWorkspace({ + path: join(aliasRoot, "git-project"), + mode: "worktree", + }); + const checkout = await aliasRegistry.openWorkspace(aliasRoot); + + assert.equal(worktree.workspace.sourceRoot, join(aliasRoot, "git-project")); + assert.deepEqual( + checkout.agentsFiles.map((file) => file.content), + ["global instructions\n", "root instructions\n"], + ); +}); + +interface WorkspaceFixture { + root: string; + outsideRoot: string; + agentDir: string; + config: ServerConfig; + registry: WorkspaceRegistry; +} + +async function fixture(t: TestContext): Promise { + const root = await mkdtemp(join(tmpdir(), "devspace-workspace-test-")); + const outsideRoot = await mkdtemp(join(tmpdir(), "devspace-workspace-outside-test-")); const agentDir = join(root, ".pi", "agent"); - await mkdir(agentDir, { recursive: true }); + if (platform() === "win32") { + await mkdir(agentDir, { recursive: true }); await writeFile(join(agentDir, "AGENTS.md"), "global instructions\n"); } else { await mkdir(join(agentDir, "skills"), { recursive: true }); await writeFile(join(agentDir, "skills", "AGENTS.md"), "global instructions\n"); await symlink("skills/AGENTS.md", join(agentDir, "AGENTS.md")); } + await writeFile(join(root, "AGENTS.md"), "root instructions\n"); await mkdir(join(root, ".devspace", "agents"), { recursive: true }); await writeFile( @@ -51,83 +215,23 @@ try { DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", PORT: "1", }); - const registry = new WorkspaceRegistry(config); - const { workspace, agentsFiles, availableAgentsFiles } = await registry.openWorkspace(root); - - assert.equal(workspace.mode, "checkout"); - assert.deepEqual( - agentsFiles.map((file) => file.content), - ["global instructions\n", "root instructions\n"], - ); - assert.deepEqual( - availableAgentsFiles.map((file) => file.path), - [join(root, "nested", "AGENTS.md")], - ); - assert.deepEqual( - workspace.agentProfiles.map((profile) => ({ - name: profile.name, - description: profile.description, - provider: profile.provider, - body: profile.body, - })), - [ - { - name: "reviewer", - description: "Read-only project reviewer.", - provider: "codex", - body: "Review only.", - }, - ], - ); - - if (platform() !== "win32") { - const unsafeAgentDir = join(root, ".pi", "unsafe-agent"); - await mkdir(unsafeAgentDir, { recursive: true }); - await writeFile(join(outsideRoot, "secret.txt"), "outside secret\n"); - await symlink(join(outsideRoot, "secret.txt"), join(unsafeAgentDir, "AGENTS.md")); - const unsafeConfig = loadConfig({ - DEVSPACE_CONFIG_DIR: join(root, ".devspace-unsafe-home"), - DEVSPACE_ALLOWED_ROOTS: root, - DEVSPACE_WORKTREE_ROOT: join(root, ".devspace", "unsafe-worktrees"), - DEVSPACE_AGENT_DIR: unsafeAgentDir, - DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", - PORT: "1", - }); - const unsafeWorkspace = await new WorkspaceRegistry(unsafeConfig).openWorkspace(root); - assert.deepEqual( - unsafeWorkspace.agentsFiles.map((file) => file.content), - ["root instructions\n"], - ); - } - const missingWorkspaceRoot = join(root, "missing", "workspace"); - const missingWorkspace = await registry.openWorkspace(missingWorkspaceRoot); - assert.equal(missingWorkspace.workspace.root, missingWorkspaceRoot); - assert.equal(missingWorkspace.workspace.mode, "checkout"); - assert.equal((await stat(missingWorkspaceRoot)).isDirectory(), true); - - { - let mkdirCalls = 0; - const existingStats = await ensureCheckoutWorkspaceRoot(root, { - stat: async (path) => { - assert.equal(path, root); - return await stat(path); - }, - mkdir: async () => { - mkdirCalls += 1; - }, - }); - assert.equal(existingStats.isDirectory(), true); - assert.equal(mkdirCalls, 0); - } + t.after(async () => { + await rm(root, { recursive: true, force: true }); + await rm(outsideRoot, { recursive: true, force: true }); + }); - await assert.rejects( - () => registry.openWorkspace({ path: root, mode: "worktree" }), - (error: unknown) => - error instanceof GitWorktreeError && error.code === "GIT_REPOSITORY_NOT_FOUND", - ); + return { + root, + outsideRoot, + agentDir, + config, + registry: new WorkspaceRegistry(config), + }; +} - const gitRoot = join(root, "git-project"); +async function createGitProject(parent: string): Promise { + const gitRoot = join(parent, "git-project"); await mkdir(gitRoot); await writeFile(join(gitRoot, "AGENTS.md"), "git root instructions\n"); await writeFile(join(gitRoot, "README.md"), "hello\n"); @@ -136,74 +240,7 @@ try { await git(gitRoot, ["config", "user.name", "DevSpace Test"]); await git(gitRoot, ["add", "."]); await git(gitRoot, ["commit", "-m", "Initial commit"]); - await writeFile(join(gitRoot, "dirty.txt"), "not copied\n"); - - const worktreeWorkspace = await registry.openWorkspace({ - path: gitRoot, - mode: "worktree", - }); - assert.equal(worktreeWorkspace.workspace.mode, "worktree"); - assert.notEqual(worktreeWorkspace.workspace.root, gitRoot); - assert.match(worktreeWorkspace.workspace.root, /git-project-[a-f0-9]{8}$/); - assert.equal(worktreeWorkspace.workspace.sourceRoot, gitRoot); - assert.equal(worktreeWorkspace.workspace.worktree?.baseRef, "HEAD"); - assert.equal(worktreeWorkspace.workspace.worktree?.dirtySource, true); - assert.equal(worktreeWorkspace.workspace.worktree?.managed, true); - assert.equal((await stat(worktreeWorkspace.workspace.root)).isDirectory(), true); - assert.match(worktreeWorkspace.agentsFiles.map((file) => file.content).join("\n"), /global instructions/); - assert.match(worktreeWorkspace.agentsFiles.map((file) => file.content).join("\n"), /git root instructions/); - - const worktreeReadmePath = registry.resolvePath(worktreeWorkspace.workspace, "README.md"); - assert.equal(worktreeReadmePath.startsWith(worktreeWorkspace.workspace.root), true); - - const stateDir = join(root, ".state"); - const firstStore = new SqliteWorkspaceStore(stateDir); - const persistentRegistry = new WorkspaceRegistry(config, firstStore); - const persistentWorkspace = await persistentRegistry.openWorkspace(root); - const persistentWorktree = await persistentRegistry.openWorkspace({ - path: gitRoot, - mode: "worktree", - }); - firstStore.close(); - - const secondStore = new SqliteWorkspaceStore(stateDir); - const restoredRegistry = new WorkspaceRegistry(config, secondStore); - const restoredWorkspace = restoredRegistry.getWorkspace(persistentWorkspace.workspace.id); - assert.equal(restoredWorkspace.root, root); - assert.equal(restoredWorkspace.mode, "checkout"); - - const restoredWorktree = restoredRegistry.getWorkspace(persistentWorktree.workspace.id); - assert.equal(restoredWorktree.mode, "worktree"); - assert.equal(restoredWorktree.sourceRoot, gitRoot); - assert.equal(restoredWorktree.root, persistentWorktree.workspace.root); - assert.equal(restoredWorktree.worktree?.managed, true); - secondStore.close(); - - if (platform() !== "win32") { - const aliasRoot = join(root, "alias-root"); - await symlink(root, aliasRoot, "dir"); - const aliasConfig = loadConfig({ - DEVSPACE_ALLOWED_ROOTS: aliasRoot, - DEVSPACE_WORKTREE_ROOT: join(aliasRoot, ".devspace", "alias-worktrees"), - DEVSPACE_AGENT_DIR: agentDir, - DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", - PORT: "1", - }); - const aliasWorkspace = await new WorkspaceRegistry(aliasConfig).openWorkspace({ - path: join(aliasRoot, "git-project"), - mode: "worktree", - }); - assert.equal(aliasWorkspace.workspace.sourceRoot, join(aliasRoot, "git-project")); - - const aliasCheckout = await new WorkspaceRegistry(aliasConfig).openWorkspace(aliasRoot); - assert.deepEqual( - aliasCheckout.agentsFiles.map((file) => file.content), - ["global instructions\n", "root instructions\n"], - ); - } -} finally { - await rm(root, { recursive: true, force: true }); - await rm(outsideRoot, { recursive: true, force: true }); + return gitRoot; } async function git(cwd: string, args: string[]): Promise { diff --git a/src/workspaces.ts b/src/workspaces.ts index e3c252f4..fa8374a0 100644 --- a/src/workspaces.ts +++ b/src/workspaces.ts @@ -1,12 +1,21 @@ import { randomUUID } from "node:crypto"; import type { Stats } from "node:fs"; -import type { WorkspaceMode, WorkspaceStore } from "./workspace-store.js"; +import type { + WorkspaceConversationBinding, + WorkspaceMode, + WorkspaceStore, +} from "./workspace-store.js"; import { mkdir, opendir, readFile, realpath, stat } from "node:fs/promises"; -import { dirname, join, relative, resolve, sep } from "node:path"; +import { basename, dirname, join, relative, resolve, sep } from "node:path"; import { loadProjectContextFiles } from "@earendil-works/pi-coding-agent"; import type { ServerConfig } from "./config.js"; import { createManagedWorktree } from "./git-worktrees.js"; -import { assertAllowedPath, isPathInsideRoot, resolveAllowedPath } from "./roots.js"; +import { + AccessDeniedError, + assertAllowedPath, + isPathInsideRoot, + resolveAllowedPath, +} from "./roots.js"; import { loadWorkspaceSkills, markSkillActivated, @@ -53,6 +62,8 @@ export interface WorkspaceContext { workspace: Workspace; agentsFiles: LoadedAgentsFile[]; availableAgentsFiles: AvailableAgentsFile[]; + workspaceReused: boolean; + includeBootstrapContext: boolean; } export interface WorkspaceReadPath { @@ -67,6 +78,10 @@ export interface OpenWorkspaceInput { baseRef?: string; } +export interface OpenWorkspaceOptions { + conversationScopeId?: string; +} + type PathStats = Stats; type DirectoryOps = { stat: (path: string) => Promise; @@ -75,14 +90,63 @@ type DirectoryOps = { export class WorkspaceRegistry { private readonly workspaces = new Map(); + private readonly pendingCheckoutOpens = new Map>(); constructor( private readonly config: ServerConfig, private readonly store?: WorkspaceStore, ) {} - async openWorkspace(input: string | OpenWorkspaceInput): Promise { - const options = typeof input === "string" ? { path: input } : input; + async openWorkspace( + input: string | OpenWorkspaceInput, + openOptions: OpenWorkspaceOptions = {}, + ): Promise { + const workspaceInput = typeof input === "string" ? { path: input } : input; + const conversationScopeId = openOptions.conversationScopeId; + if (!conversationScopeId || !this.store) { + return this.openNewWorkspace(workspaceInput); + } + + const projectKey = await this.conversationProjectKey(workspaceInput); + const mode = workspaceInput.mode ?? "checkout"; + if (mode === "worktree") { + const context = await this.openWorktreeWorkspace(workspaceInput.path, workspaceInput.baseRef); + return { + ...context, + // A new worktree always has its own workspace-specific context. + includeBootstrapContext: true, + }; + } + + const targetKey = this.conversationCheckoutTargetKey(projectKey); + const operationKey = JSON.stringify([conversationScopeId, targetKey]); + const pending = this.pendingCheckoutOpens.get(operationKey); + if (pending) { + const context = await pending; + return { + ...context, + workspaceReused: true, + includeBootstrapContext: false, + }; + } + + const open = this.openConversationCheckout( + workspaceInput, + conversationScopeId, + targetKey, + ); + this.pendingCheckoutOpens.set(operationKey, open); + + try { + return await open; + } finally { + if (this.pendingCheckoutOpens.get(operationKey) === open) { + this.pendingCheckoutOpens.delete(operationKey); + } + } + } + + private async openNewWorkspace(options: OpenWorkspaceInput): Promise { const mode = options.mode ?? "checkout"; if (mode === "worktree") { @@ -92,6 +156,92 @@ export class WorkspaceRegistry { return this.openCheckoutWorkspace(options.path); } + private async openConversationCheckout( + input: OpenWorkspaceInput, + conversationScopeId: string, + targetKey: string, + ): Promise { + const binding = this.store?.getConversationBinding(conversationScopeId, targetKey); + if (binding) { + const reusableWorkspace = await this.findReusableCheckoutWorkspace(binding); + + if (reusableWorkspace) { + const context = await this.reusedWorkspaceContext(reusableWorkspace); + this.store?.touchConversationBinding(conversationScopeId, targetKey); + return { + ...context, + includeBootstrapContext: false, + }; + } + + this.workspaces.delete(binding.workspaceSessionId); + this.store?.deleteConversationBinding(conversationScopeId, targetKey); + } + + const context = await this.openCheckoutWorkspace(input.path); + this.store?.setConversationBinding({ + conversationScopeId, + targetKey, + workspaceSessionId: context.workspace.id, + }); + return { + ...context, + includeBootstrapContext: true, + }; + } + + private async findReusableCheckoutWorkspace( + binding: WorkspaceConversationBinding, + ): Promise { + const session = this.store?.getSession(binding.workspaceSessionId); + if (!session || session.status !== "active" || session.mode !== "checkout") { + return undefined; + } + + let root: string; + try { + root = this.assertWorkspaceRootAllowed(session.root, session.mode, session.sourceRoot); + const rootStats = await stat(root); + if (!rootStats.isDirectory()) return undefined; + } catch (error) { + if ( + error instanceof AccessDeniedError || + (isErrnoException(error) && (error.code === "ENOENT" || error.code === "ENOTDIR")) + ) { + return undefined; + } + + throw error; + } + + const workspace = this.getWorkspace(binding.workspaceSessionId); + if (workspace.mode !== "checkout" || workspace.root !== root) return undefined; + return workspace; + } + + private async conversationProjectKey(input: OpenWorkspaceInput): Promise { + const path = assertAllowedPath(input.path, this.config.allowedRoots); + return canonicalPath(path); + } + + private conversationCheckoutTargetKey(projectKey: string): string { + return JSON.stringify(["checkout", projectKey, null]); + } + + private async reusedWorkspaceContext(workspace: Workspace): Promise { + workspace.agentProfiles = await loadLocalAgentProfiles(this.config, workspace.root); + const agentsFiles = await this.loadInitialAgentsFiles(workspace.root); + const availableAgentsFiles = await this.findAvailableAgentsFiles(workspace.root, agentsFiles); + + return { + workspace, + agentsFiles, + availableAgentsFiles, + workspaceReused: true, + includeBootstrapContext: true, + }; + } + getWorkspace(workspaceId: string): Workspace { const workspace = this.workspaces.get(workspaceId); if (workspace) { @@ -228,7 +378,13 @@ export class WorkspaceRegistry { const agentsFiles = await this.loadInitialAgentsFiles(workspace.root); const availableAgentsFiles = await this.findAvailableAgentsFiles(workspace.root, agentsFiles); - return { workspace, agentsFiles, availableAgentsFiles }; + return { + workspace, + agentsFiles, + availableAgentsFiles, + workspaceReused: false, + includeBootstrapContext: true, + }; } private loadSkillsForWorkspace(root: string): Pick { @@ -303,6 +459,26 @@ export class WorkspaceRegistry { } } +async function canonicalPath(path: string): Promise { + const missingSegments: string[] = []; + let candidate = path; + + while (true) { + try { + return resolve(await realpath(candidate), ...missingSegments.slice().reverse()); + } catch (error) { + if (!isErrnoException(error) || (error.code !== "ENOENT" && error.code !== "ENOTDIR")) { + throw error; + } + + const parent = dirname(candidate); + if (parent === candidate) return path; + missingSegments.push(basename(candidate)); + candidate = parent; + } + } +} + export async function ensureCheckoutWorkspaceRoot( path: string, ops: DirectoryOps = { stat, mkdir },