Skip to content

fix: prevent worktree reuse across different local clones #1186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
halindrome wants to merge 5 commits into from
Closed

fix: prevent worktree reuse across different local clones #1186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9d54ac
fix: prevent worktree reuse across different local clones (#1183)
shanemccarron-maker Apr 13, 2026
9f38502
fix(cli): address QA round 1 — add tests and warn on null root
shanemccarron-maker Apr 13, 2026
e92ee2d
test(cli): address QA round 2 — test null-root warning path
shanemccarron-maker Apr 13, 2026
e2af7a2
fix(cli): use getCanonicalRepoPath for worktree-stable comparison
shanemccarron-maker Apr 13, 2026
14c57f1
fix(cli): address CodeRabbit — resolve subdir before canonicalize, lo…
shanemccarron-maker Apr 13, 2026
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
195 changes: 195 additions & 0 deletions packages/cli/src/commands/workflow.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ mock.module('@archon/workflows/event-emitter', () => ({

mock.module('@archon/git', () => ({
findRepoRoot: mock(() => Promise.resolve(null)),
getCanonicalRepoPath: mock(() => Promise.reject(new Error('not a git repo'))),
getRemoteUrl: mock(() => Promise.resolve(null)),
checkout: mock(() => Promise.resolve()),
toRepoPath: mock((path: string) => path),
Expand Down Expand Up @@ -975,6 +976,200 @@ describe('workflowRunCommand', () => {
consoleWarnSpy.mockRestore();
}
});

it('skips reuse when existing env has different source_repo_root (GH-1183)', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const isolationDb = await import('@archon/core/db/isolation-environments');
const gitModule = await import('@archon/git');
const isolation = await import('@archon/isolation');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'cb-123',
default_cwd: '/clone-b',
});
// Existing env was created from /clone-a
(isolationDb.findActiveByWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'env-1',
working_path: '/worktrees/feat',
branch_name: 'my-feature',
workflow_type: 'task',
workflow_id: 'my-feature',
metadata: { source_repo_root: '/clone-a' },
});
// Current cwd resolves to /clone-b (findRepoRoot → getCanonicalRepoPath chain)
(gitModule.findRepoRoot as ReturnType<typeof mock>).mockResolvedValueOnce('/clone-b');
(gitModule.getCanonicalRepoPath as ReturnType<typeof mock>).mockResolvedValueOnce('/clone-b');
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);
(executeWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
success: true,
workflowRunId: 'run-123',
});

await workflowRunCommand('/clone-b', 'assist', 'hello', { branchName: 'my-feature' });

// Should have created a new worktree instead of reusing
const getIsolationProviderMock = isolation.getIsolationProvider as ReturnType<typeof mock>;
const provider = getIsolationProviderMock.mock.results.at(-1)?.value as {
create: ReturnType<typeof mock>;
};
expect(provider.create).toHaveBeenCalled();
});

it('reuses existing env when source_repo_root is absent (backward compat)', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const isolationDb = await import('@archon/core/db/isolation-environments');
const gitModule = await import('@archon/git');
const isolation = await import('@archon/isolation');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'cb-123',
default_cwd: '/clone-a',
});
// Existing env has no source_repo_root (pre-existing)
(isolationDb.findActiveByWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'env-1',
working_path: '/worktrees/feat',
branch_name: 'my-feature',
workflow_type: 'task',
workflow_id: 'my-feature',
metadata: {},
});
(gitModule.findRepoRoot as ReturnType<typeof mock>).mockResolvedValueOnce('/clone-a');
(gitModule.getCanonicalRepoPath as ReturnType<typeof mock>).mockResolvedValueOnce('/clone-a');
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);
(executeWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
success: true,
workflowRunId: 'run-123',
});

await workflowRunCommand('/clone-a', 'assist', 'hello', { branchName: 'my-feature' });

// Should NOT have created a new worktree — reuse path taken
const getIsolationProviderMock = isolation.getIsolationProvider as ReturnType<typeof mock>;
const provider = getIsolationProviderMock.mock.results.at(-1)?.value as {
create: ReturnType<typeof mock>;
};
expect(provider.create).not.toHaveBeenCalled();
});

it('reuses existing env when source_repo_root matches current root', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const isolationDb = await import('@archon/core/db/isolation-environments');
const gitModule = await import('@archon/git');
const isolation = await import('@archon/isolation');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'cb-123',
default_cwd: '/clone-a',
});
// Existing env was created from same checkout
(isolationDb.findActiveByWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'env-1',
working_path: '/worktrees/feat',
branch_name: 'my-feature',
workflow_type: 'task',
workflow_id: 'my-feature',
metadata: { source_repo_root: '/clone-a' },
});
(gitModule.findRepoRoot as ReturnType<typeof mock>).mockResolvedValueOnce('/clone-a');
(gitModule.getCanonicalRepoPath as ReturnType<typeof mock>).mockResolvedValueOnce('/clone-a');
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);
(executeWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
success: true,
workflowRunId: 'run-123',
});

await workflowRunCommand('/clone-a', 'assist', 'hello', { branchName: 'my-feature' });

// Should NOT have created a new worktree — same checkout, reuse is safe
const getIsolationProviderMock = isolation.getIsolationProvider as ReturnType<typeof mock>;
const provider = getIsolationProviderMock.mock.results.at(-1)?.value as {
create: ReturnType<typeof mock>;
};
expect(provider.create).not.toHaveBeenCalled();
});

it('allows reuse with warning when repo root is undetectable but env has source_repo_root', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const isolationDb = await import('@archon/core/db/isolation-environments');
const gitModule = await import('@archon/git');
const isolation = await import('@archon/isolation');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'cb-123',
default_cwd: '/clone-a',
});
// Env has a known source_repo_root
(isolationDb.findActiveByWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'env-1',
working_path: '/worktrees/feat',
branch_name: 'my-feature',
workflow_type: 'task',
workflow_id: 'my-feature',
metadata: { source_repo_root: '/clone-a' },
});
// findRepoRoot returns null by default (not a git repo) — currentRepoRoot stays null
mockLogger.warn.mockClear();
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);
(executeWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
success: true,
workflowRunId: 'run-123',
});

await workflowRunCommand('/clone-a', 'assist', 'hello', { branchName: 'my-feature' });

// Should NOT have created a new worktree — fallback allows reuse when root undetectable
const getIsolationProviderMock = isolation.getIsolationProvider as ReturnType<typeof mock>;
const provider = getIsolationProviderMock.mock.results.at(-1)?.value as {
create: ReturnType<typeof mock>;
};
expect(provider.create).not.toHaveBeenCalled();
// Verify the warning was actually logged
expect(mockLogger.warn).toHaveBeenCalledWith(
expect.objectContaining({ envSourceRoot: '/clone-a' }),
'worktree.reuse_root_undetectable'
);
});
});

describe('workflowStatusCommand', () => {
Expand Down
53 changes: 51 additions & 2 deletions packages/cli/src/commands/workflow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,48 @@ export async function workflowRunCommand(
? await isolationDb.findActiveByWorkflow(codebase.id, 'task', options.branchName)
: undefined;

if (existingEnv && (await provider.healthCheck(existingEnv.working_path))) {
// Guard: skip reuse if the existing environment was created from a different
// local clone of the same remote (GH-1183). Two clones share one codebase_id
// (derived from the remote URL), so findActiveByWorkflow can return an
// environment that belongs to a sibling checkout. Compare the canonical repo
// path (stable across linked worktrees) recorded at creation time with the
// current working directory's canonical path.
let currentRepoRoot: string | null = null;
try {
// Resolve subdirectory to repo top-level first, then canonicalize to handle
// linked worktrees (getCanonicalRepoPath doesn't resolve subdirs itself).
const repoRoot = await git.findRepoRoot(cwd);
currentRepoRoot = repoRoot ? await git.getCanonicalRepoPath(repoRoot) : null;
} catch (error) {
// findRepoRoot returns null for non-git dirs (expected), but both functions
// throw on unexpected errors (permissions, I/O). Log and fall through —
// the guard will allow reuse when currentRepoRoot is null.
getLog().debug({ err: error, cwd }, 'worktree.reuse_root_detection_failed');
}
const envSourceRoot =
existingEnv && typeof existingEnv.metadata?.source_repo_root === 'string'
? existingEnv.metadata.source_repo_root
: undefined;

// Intentional: when currentRepoRoot or envSourceRoot is unavailable, fall back
// to allowing reuse. This preserves backward compat for pre-existing envs and
// avoids blocking workflows when git is temporarily unavailable. Risk: could
// allow cross-checkout reuse in edge cases — logged as a warning below.
const reuseSameCheckout =
!existingEnv || !envSourceRoot || !currentRepoRoot || envSourceRoot === currentRepoRoot;
Comment on lines +432 to +454
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "workflow.ts changed block:"
sed -n '426,536p' packages/cli/src/commands/workflow.ts

echo
echo "findRepoRoot() implementation:"
sed -n '14,40p' packages/git/src/repo.ts

echo
echo "getCanonicalRepoPath() implementation:"
sed -n '232,257p' packages/git/src/worktree.ts

Repository: coleam00/Archon

Length of output: 6635

Resolve the repo root before canonicalizing it.

Line 434 calls getCanonicalRepoPath(cwd) directly, but that function returns non-worktree paths unchanged. If the command runs from a subdirectory like /repo/subdir, it returns /repo/subdir as-is, causing same-clone comparisons to fail across different directory invocations. This unstable value is then persisted into metadata.source_repo_root at line 535. Additionally, the empty catch block swallows all exceptions—including real worktree metadata failures—and silently falls back to reuse-allowed behavior.

Call findRepoRoot(cwd) first to resolve any subdirectory to the actual repo top-level (returning null only for non-repo cases), then canonicalize that root. Log and throw on real errors rather than silencing them.

🛠️ Proposed fix 
    let currentRepoRoot: string | null = null;
    try {
-     currentRepoRoot = await git.getCanonicalRepoPath(cwd);
-   } catch {
-     // Non-fatal: cwd may not be a git repo (handled later) or git unavailable
+     const repoRoot = await git.findRepoRoot(cwd);
+     currentRepoRoot = repoRoot ? await git.getCanonicalRepoPath(repoRoot) : null;
+   } catch (error) {
+     const err = error as Error;
+     getLog().error({ err, cwd }, 'worktree.reuse_root_detection_failed');
+     throw new Error(`Failed to resolve repository root for ${cwd}: ${err.message}`);
    }

Violates: "Never silently swallow errors related to unsupported or unsafe states — throw early with clear error messages."

Also applies to: 535

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/workflow.ts` around lines 432 - 448, The code
should resolve the repo top-level before canonicalizing and must not silently
swallow real errors: call findRepoRoot(cwd) first and if it returns a string
pass that value into getCanonicalRepoPath(...) to compute currentRepoRoot (treat
null as “not a git repo” but do not ignore thrown errors); replace the empty
catch with logic that logs the caught error via processLogger (or appropriate
logger) and rethrows unexpected errors, while only allowing a benign fallback
when findRepoRoot returned null; ensure the value persisted into
existingEnv.metadata.source_repo_root and the reuseSameCheckout comparison use
this canonicalized root.


if (existingEnv && envSourceRoot && !currentRepoRoot) {
getLog().warn(
{ path: existingEnv.working_path, envSourceRoot },
'worktree.reuse_root_undetectable'
);
}

if (
existingEnv &&
reuseSameCheckout &&
(await provider.healthCheck(existingEnv.working_path))
) {
if (options.fromBranch) {
getLog().warn(
{ path: existingEnv.working_path, fromBranch: options.fromBranch },
Expand Down Expand Up @@ -463,6 +504,14 @@ export async function workflowRunCommand(
workingCwd = existingEnv.working_path;
isolationEnvId = existingEnv.id;
} else {
// Log when skipping reuse due to cross-checkout mismatch (GH-1183)
if (existingEnv && !reuseSameCheckout) {
getLog().warn(
{ path: existingEnv.working_path, envSourceRoot, currentRepoRoot },
'worktree.reuse_different_checkout'
);
}

// Create new worktree
getLog().info(
{ branch: branchIdentifier, fromBranch: options.fromBranch },
Expand All @@ -489,7 +538,7 @@ export async function workflowRunCommand(
working_path: isolatedEnv.workingPath,
branch_name: isolatedEnv.branchName,
created_by_platform: 'cli',
metadata: {},
metadata: { ...(currentRepoRoot ? { source_repo_root: currentRepoRoot } : {}) },
});

workingCwd = isolatedEnv.workingPath;
Expand Down