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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ undefined/
coverage

worktrees
.worktrees
screenshots/
test-screenshots/
.claude/worktree-registry.json
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/config/config-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,16 @@ function mergeRepoConfig(merged: MergedConfig, repo: RepoConfig): MergedConfig {
result.baseBranch = repo.worktree.baseBranch.trim();
}

// Propagate per-project worktree path for isolation providers
if (repo.worktree?.path !== undefined) {
const trimmed = repo.worktree.path.trim();
if (trimmed) {
result.worktreePath = trimmed;
} else {
getLog().warn({ rawValue: repo.worktree.path }, 'config.worktree_path_whitespace_ignored');
}
Comment on lines +389 to +395
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard repo.worktree.path type before calling .trim().

This branch can throw if YAML contains a non-string value (e.g., path: null), because !== undefined still passes and .trim() is invoked on a non-string.

Suggested fix
   // Propagate per-project worktree path for isolation providers
   if (repo.worktree?.path !== undefined) {
-    const trimmed = repo.worktree.path.trim();
+    if (typeof repo.worktree.path !== 'string') {
+      throw new Error('Invalid .archon/config.yaml: worktree.path must be a string');
+    }
+    const trimmed = repo.worktree.path.trim();
     if (trimmed) {
       result.worktreePath = trimmed;
     } else {
       getLog().warn({ rawValue: repo.worktree.path }, 'config.worktree_path_whitespace_ignored');
     }
   }

As per coding guidelines, “Throw early with a clear error for unsupported or unsafe states” and “keep unsupported paths explicit (error out) rather than adding partial fake support.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (repo.worktree?.path !== undefined) {
const trimmed = repo.worktree.path.trim();
if (trimmed) {
result.worktreePath = trimmed;
} else {
getLog().warn({ rawValue: repo.worktree.path }, 'config.worktree_path_whitespace_ignored');
}
if (repo.worktree?.path !== undefined) {
if (typeof repo.worktree.path !== 'string') {
throw new Error('Invalid .archon/config.yaml: worktree.path must be a string');
}
const trimmed = repo.worktree.path.trim();
if (trimmed) {
result.worktreePath = trimmed;
} else {
getLog().warn({ rawValue: repo.worktree.path }, 'config.worktree_path_whitespace_ignored');
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/config/config-loader.ts` around lines 389 - 395, The code
calls repo.worktree.path.trim() after checking !== undefined which will throw if
path is non-string (e.g., null); update the branch in config-loader.ts to first
assert the type is string (e.g., typeof repo.worktree.path === 'string') before
calling .trim(), set result.worktreePath when trimmed is non-empty, and
otherwise either log the whitespace-warning using getLog().warn or throw a clear
error for unsupported non-string values (per guidelines) so the unsafe state is
rejected explicitly; reference symbols: repo.worktree.path, result.worktreePath,
and getLog().warn.

}

// Propagate docs path for $DOCS_DIR substitution in workflow commands
if (repo.docs?.path !== undefined) {
const trimmed = repo.docs.path.trim();
Expand Down
16 changes: 16 additions & 0 deletions packages/core/src/config/config-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,14 @@ export interface RepoConfig {
* @example [".env", ".archon", "data/fixtures/"]
*/
copyFiles?: string[];

/**
* Per-project worktree directory path (relative to repo root).
* When set, worktrees are created at `<repoRoot>/<path>/<branchName>`
* instead of the global `~/.archon/worktrees/` directory.
* @example '.worktrees'
*/
path?: string;
};

/**
Expand Down Expand Up @@ -251,6 +259,14 @@ export interface MergedConfig {
* When undefined, workflows referencing $BASE_BRANCH will fail with an error.
*/
baseBranch?: string;
/**
* Per-project worktree directory path from repo config (worktree.path).
* When set, worktrees are created at `<repoRoot>/<worktreePath>/<branchName>`
* instead of the global `paths.worktrees` directory.
* Undefined when not configured (uses global paths.worktrees).
* @example '.worktrees'
*/
worktreePath?: string;
/**
* Docs directory path from repo config (docs.path).
* Used for $DOCS_DIR substitution in workflow commands.
Expand Down
67 changes: 67 additions & 0 deletions packages/isolation/src/providers/worktree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2161,6 +2161,73 @@ describe('WorktreeProvider', () => {
});
});

describe('per-project worktree.path', () => {
test('getWorktreePath uses config.path when set', () => {
const request: IsolationRequest = {
codebaseId: 'cb-123',
canonicalRepoPath: '/Users/joel/Projects/myapp',
workflowType: 'task',
identifier: 'add-feature',
};
const branchName = provider.generateBranchName(request);
const config = { path: '.worktrees' };
const path = provider.getWorktreePath(request, branchName, config);
expect(path).toBe(join('/Users/joel/Projects/myapp', '.worktrees', branchName));
});

test('getWorktreePath ignores empty or whitespace-only path', () => {
const request: IsolationRequest = {
codebaseId: 'cb-123',
canonicalRepoPath: '/Users/joel/Projects/myapp',
workflowType: 'task',
identifier: 'add-feature',
};
const branchName = provider.generateBranchName(request);

// Empty string
const path1 = provider.getWorktreePath(request, branchName, { path: '' });
expect(path1).not.toContain('.worktrees');

// Whitespace-only
const path2 = provider.getWorktreePath(request, branchName, { path: ' ' });
expect(path2).not.toContain('.worktrees');
});

test('getWorktreePath falls back to default when config is null', () => {
const request: IsolationRequest = {
codebaseId: 'cb-123',
canonicalRepoPath: '/Users/joel/Projects/myapp',
workflowType: 'task',
identifier: 'add-feature',
};
const branchName = provider.generateBranchName(request);
const pathWithNull = provider.getWorktreePath(request, branchName, null);
const pathWithUndefined = provider.getWorktreePath(request, branchName, undefined);
const pathDefault = provider.getWorktreePath(request, branchName);
expect(pathWithNull).toBe(pathDefault);
expect(pathWithUndefined).toBe(pathDefault);
});

test('getWorktreePath config.path overrides project-scoped and legacy paths', () => {
// Even for repos under workspaces/, config.path should win
const request: IsolationRequest = {
codebaseId: 'cb-123',
codebaseName: 'owner/repo',
canonicalRepoPath: join(TEST_ARCHON_HOME, 'workspaces', 'owner', 'repo'),
workflowType: 'task',
identifier: 'my-task',
};
const branchName = provider.generateBranchName(request);
const config = { path: 'worktrees-local' };
const path = provider.getWorktreePath(request, branchName, config);
expect(path).toBe(
join(TEST_ARCHON_HOME, 'workspaces', 'owner', 'repo', 'worktrees-local', branchName)
);
// Should NOT use the project-scoped worktrees/ path
expect(path).not.toContain('/worktrees/archon/');
});
});

// ---------------------------------------------------------------------------
// Additional lifecycle method tests
// ---------------------------------------------------------------------------
Expand Down
82 changes: 61 additions & 21 deletions packages/isolation/src/providers/worktree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,37 @@ export class WorktreeProvider implements IIsolationProvider {
* Create an isolated environment using git worktrees
*/
async create(request: IsolationRequest): Promise<IsolatedEnvironment> {
// Load config early so worktree.path can influence path resolution.
// On success, pass to createWorktree to avoid double loading.
// On failure, let createWorktree reload (so config errors propagate correctly).
let earlyConfig: WorktreeCreateConfig | null = null;
let earlyConfigLoaded = false;
try {
earlyConfig = await this.loadConfig(request.canonicalRepoPath);
earlyConfigLoaded = true;
} catch {
// Non-fatal here: fall back to default path resolution.
// createWorktree will re-attempt the load and throw if needed.
}

const branchName = toBranchName(this.generateBranchName(request));
const worktreePath = this.getWorktreePath(request, branchName);
const envId = this.generateEnvId(request);
const worktreePath = this.getWorktreePath(request, branchName, earlyConfig);
const envId = worktreePath;

// Check for existing worktree (adoption)
const existing = await this.findExisting(request, branchName, worktreePath);
if (existing) {
return existing;
}

// Create new worktree
const { warnings } = await this.createWorktree(request, worktreePath, branchName);
// Create new worktree. Pass pre-loaded config only when early load succeeded;
// otherwise pass undefined so createWorktree reloads (and throws on error).
const { warnings } = await this.createWorktree(
request,
worktreePath,
branchName,
earlyConfigLoaded ? earlyConfig : undefined
);

return {
id: envId,
Expand Down Expand Up @@ -454,16 +473,26 @@ export class WorktreeProvider implements IIsolationProvider {
/**
* Get worktree path for request.
*
* Path format depends on the worktree base layout:
* - Project-scoped: `~/.archon/workspaces/{owner}/{repo}/worktrees/{branch}`
* - Legacy global: `~/.archon/worktrees/{owner}/{repo}/{branch}`
* Path format depends on configuration:
* - Per-project path: `<repoRoot>/<worktree.path>/<branch>` (when repo config sets worktree.path)
* - Project-scoped: `~/.archon/workspaces/{owner}/{repo}/worktrees/{branch}`
* - Legacy global: `~/.archon/worktrees/{owner}/{repo}/{branch}`
*
* When the worktree base is project-scoped (under workspaces/owner/repo/worktrees/),
* only append the branch name since the base already includes owner/repo.
* When using the legacy global worktrees path, append owner/repo/branch to
* avoid collisions between repos.
*/
getWorktreePath(request: IsolationRequest, branchName: string): string {
getWorktreePath(
request: IsolationRequest,
branchName: string,
config?: WorktreeCreateConfig | null
): string {
// Per-project worktree path takes highest priority
if (config?.path?.trim()) {
return join(request.canonicalRepoPath, config.path.trim(), branchName);
}

const worktreeBase = getWorktreeBase(request.canonicalRepoPath, request.codebaseName);

if (isProjectScopedWorktreeBase(request.canonicalRepoPath, request.codebaseName)) {
Expand Down Expand Up @@ -530,30 +559,41 @@ export class WorktreeProvider implements IIsolationProvider {
private async createWorktree(
request: IsolationRequest,
worktreePath: string,
branchName: string
branchName: string,
preloadedConfig?: WorktreeCreateConfig | null
): Promise<{ warnings: string[] }> {
const repoPath = request.canonicalRepoPath;

let worktreeConfig: WorktreeCreateConfig | null;
try {
worktreeConfig = await this.loadConfig(repoPath);
} catch (error) {
const err = error as Error;
getLog().error({ err, repoPath }, 'repo_config_load_failed');
throw new Error(`Failed to load config: ${err.message}`);
if (preloadedConfig !== undefined) {
// Use pre-loaded config (avoids double loading from create())
worktreeConfig = preloadedConfig;
} else {
try {
worktreeConfig = await this.loadConfig(repoPath);
} catch (error) {
const err = error as Error;
getLog().error({ err, repoPath }, 'repo_config_load_failed');
throw new Error(`Failed to load config: ${err.message}`);
}
}

// Sync uses only the configured base branch (or auto-detects via getDefaultBranch).
// request.fromBranch is the start-point for worktree creation, not a sync target.
const baseBranch = await this.syncWorkspaceBeforeCreate(repoPath, worktreeConfig?.baseBranch);

const worktreeBase = getWorktreeBase(repoPath, request.codebaseName);

if (isProjectScopedWorktreeBase(repoPath, request.codebaseName)) {
await mkdirAsync(worktreeBase, { recursive: true });
// Ensure parent directory for worktree exists
if (worktreeConfig?.path?.trim()) {
// Per-project path: create the configured directory under the repo root
await mkdirAsync(join(repoPath, worktreeConfig.path.trim()), { recursive: true });
} else {
const { owner, repo } = this.extractOwnerRepo(repoPath);
await mkdirAsync(join(worktreeBase, owner, repo), { recursive: true });
const worktreeBase = getWorktreeBase(repoPath, request.codebaseName);
if (isProjectScopedWorktreeBase(repoPath, request.codebaseName)) {
await mkdirAsync(worktreeBase, { recursive: true });
} else {
const { owner, repo } = this.extractOwnerRepo(repoPath);
await mkdirAsync(join(worktreeBase, owner, repo), { recursive: true });
}
}

if (isPRIsolationRequest(request)) {
Expand Down
7 changes: 7 additions & 0 deletions packages/isolation/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,13 @@ export interface IsolationEnvironmentRow {
export interface WorktreeCreateConfig {
baseBranch?: string;
copyFiles?: string[];
/**
* Per-project worktree directory path (relative to repo root).
* When set, worktrees are created at `<repoRoot>/<path>/<branchName>`
* instead of the global `~/.archon/worktrees/` directory.
* @example '.worktrees'
*/
path?: string;
}

export type RepoConfigLoader = (repoPath: string) => Promise<WorktreeCreateConfig | null>;
Expand Down