diff --git a/.archon/workflows/e2e-worktree-disabled.yaml b/.archon/workflows/e2e-worktree-disabled.yaml new file mode 100644 index 00000000..4c1948e6 --- /dev/null +++ b/.archon/workflows/e2e-worktree-disabled.yaml @@ -0,0 +1,34 @@ +# E2E smoke test — workflow-level worktree.enabled: false +# Verifies: when a workflow pins worktree.enabled: false, runs happen in the +# live repo checkout (no worktree created, cwd == repo root). Zero AI calls. +name: e2e-worktree-disabled +description: "Pinned-isolation-off smoke. Asserts cwd is the repo root rather than a worktree path, regardless of how the workflow is invoked." + +worktree: + enabled: false + +nodes: + # Print cwd so the operator can eyeball it, and capture for the assertion node. + - id: print-cwd + bash: "pwd" + + # Assertion: cwd must NOT contain '/.archon/workspaces/' — if it does, the + # policy was ignored and a worktree was created anyway. We also assert the + # cwd ends with a git repo (has a .git directory or file visible). + - id: assert-live-checkout + bash: | + cwd="$(pwd)" + echo "assert-live-checkout cwd=$cwd" + case "$cwd" in + */.archon/workspaces/*/worktrees/*) + echo "FAIL: workflow ran inside a worktree ($cwd) despite worktree.enabled: false" + exit 1 + ;; + esac + if [ ! -e "$cwd/.git" ]; then + echo "FAIL: cwd $cwd is not a git checkout root (.git missing)" + exit 1 + fi + echo "PASS: ran in live checkout (no worktree created by policy)" + depends_on: [print-cwd] + trigger_rule: all_success diff --git a/.archon/workflows/repo-triage.yaml b/.archon/workflows/repo-triage.yaml index 30d1520b..0dac0b55 100644 --- a/.archon/workflows/repo-triage.yaml +++ b/.archon/workflows/repo-triage.yaml @@ -8,6 +8,12 @@ description: >- runs; safe to re-run; idempotent. interactive: false +# Read-only triage runs directly in the live checkout. Creating a worktree +# every run would be wasted work (nothing is mutated) and would scatter stale +# branches under ~/.archon/workspaces///worktrees/. +worktree: + enabled: false + nodes: # --------------------------------------------------------------------------- # Issue triage — runs concurrently with pr-link (no depends_on between them). diff --git a/CHANGELOG.md b/CHANGELOG.md index d49c5b5e..34e4d9ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`'global'` variant on `WorkflowSource`** — workflows at `~/.archon/workflows/` and commands at `~/.archon/commands/` now render with a distinct source label (no longer coerced to `'project'`). Web UI badges updated. - **`getHomeWorkflowsPath()`, `getHomeCommandsPath()`, `getHomeScriptsPath()`, `getLegacyHomeWorkflowsPath()`** helpers in `@archon/paths`, exported for both internal discovery and external callers that want to target the home scope directly. - **`discoverScriptsForCwd(cwd)`** in `@archon/workflows/script-discovery` — merges home-scoped + repo-scoped scripts with repo winning on name collisions. Used by the DAG executor and validator; callers no longer need to know about the two-scope shape. +- **Workflow-level worktree policy (`worktree.enabled` in workflow YAML).** A workflow can now pin whether its runs use isolation regardless of how they were invoked: `worktree.enabled: false` always runs in the live checkout (CLI `--branch` / `--from` hard-error; web/chat/orchestrator short-circuits `validateAndResolveIsolation`), `worktree.enabled: true` requires isolation (CLI `--no-worktree` hard-errors). Omit the block to let the caller decide (current default). First consumer: `.archon/workflows/repo-triage.yaml` pinned to `enabled: false` since it's read-only. +- **Per-project worktree path (`worktree.path` in `.archon/config.yaml`).** Opt-in repo-relative directory (e.g. `.worktrees`) where Archon places worktrees for that repo, instead of the default `~/.archon/workspaces///worktrees/`. Co-locates worktrees with the project so they appear in the IDE file tree. Validated as a safe relative path (no absolute, no `..`); malformed values fail loudly at worktree creation. Users opting in are responsible for `.gitignore`ing the directory themselves — no automatic file mutation. Credits @joelsb for surfacing the need in #1117. - **Three-path env model with operator-visible log lines.** The CLI and server now load env vars from `~/.archon/.env` (user scope) and `/.archon/.env` (repo scope, overrides user) at boot, both with `override: true`. A new `[archon] loaded N keys from ` line is emitted per source (only when N > 0). `[archon] stripped N keys from (...)` now also prints when stripCwdEnv removes target-repo env keys, replacing the misleading `[dotenv@17.3.1] injecting env (0) from .env` preamble that always reported 0. The `quiet: true` flag suppresses dotenv's own output. (#1302) - **`archon setup --scope home|project` and `--force` flags.** Default is `--scope home` (writes `~/.archon/.env`). `--scope project` targets `/.archon/.env` instead. `--force` overwrites the target wholesale rather than merging; a timestamped backup is still written. (#1303) - **Merge-only setup writes with timestamped backups.** `archon setup` now reads the existing target file, preserves non-empty values, carries user-added custom keys forward, and writes a `.archon-backup-` before every rewrite. Fixes silent PostgreSQL→SQLite downgrade and silent token loss on re-run. (#1303) diff --git a/packages/cli/src/commands/workflow.test.ts b/packages/cli/src/commands/workflow.test.ts index 6eb2aed5..996ce991 100644 --- a/packages/cli/src/commands/workflow.test.ts +++ b/packages/cli/src/commands/workflow.test.ts @@ -865,6 +865,146 @@ describe('workflowRunCommand', () => { expect(createCallsAfter).toBe(createCallsBefore); }); + // ------------------------------------------------------------------------- + // Workflow-level `worktree.enabled` policy + // ------------------------------------------------------------------------- + + it('skips isolation when workflow YAML pins worktree.enabled: false', 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 isolation = await import('@archon/isolation'); + + const getIsolationProviderMock = isolation.getIsolationProvider as ReturnType; + const providerBefore = getIsolationProviderMock.mock.results.at(-1)?.value as + | { create: ReturnType } + | undefined; + const createCallsBefore = providerBefore?.create.mock.calls.length ?? 0; + + (discoverWorkflowsWithConfig as ReturnType).mockResolvedValueOnce({ + workflows: [ + makeTestWorkflowWithSource({ + name: 'triage', + description: 'Read-only triage', + worktree: { enabled: false }, + }), + ], + errors: [], + }); + (conversationDb.getOrCreateConversation as ReturnType).mockResolvedValueOnce({ + id: 'conv-123', + }); + (codebaseDb.findCodebaseByDefaultCwd as ReturnType).mockResolvedValueOnce({ + id: 'cb-123', + default_cwd: '/test/path', + }); + (conversationDb.updateConversation as ReturnType).mockResolvedValueOnce(undefined); + (executeWorkflow as ReturnType).mockResolvedValueOnce({ + success: true, + workflowRunId: 'run-123', + }); + + // No flags — policy alone should disable isolation + await workflowRunCommand('/test/path', 'triage', 'go', {}); + + const providerAfter = getIsolationProviderMock.mock.results.at(-1)?.value as + | { create: ReturnType } + | undefined; + const createCallsAfter = providerAfter?.create.mock.calls.length ?? 0; + expect(createCallsAfter).toBe(createCallsBefore); + }); + + it('throws when workflow pins worktree.enabled: false but caller passes --branch', async () => { + const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery'); + + (discoverWorkflowsWithConfig as ReturnType).mockResolvedValueOnce({ + workflows: [ + makeTestWorkflowWithSource({ + name: 'triage', + description: 'Read-only triage', + worktree: { enabled: false }, + }), + ], + errors: [], + }); + + await expect( + workflowRunCommand('/test/path', 'triage', 'go', { branchName: 'feat-x' }) + ).rejects.toThrow(/worktree\.enabled: false/); + }); + + it('throws when workflow pins worktree.enabled: false but caller passes --from', async () => { + const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery'); + + (discoverWorkflowsWithConfig as ReturnType).mockResolvedValueOnce({ + workflows: [ + makeTestWorkflowWithSource({ + name: 'triage', + description: 'Read-only triage', + worktree: { enabled: false }, + }), + ], + errors: [], + }); + + await expect( + workflowRunCommand('/test/path', 'triage', 'go', { fromBranch: 'dev' }) + ).rejects.toThrow(/worktree\.enabled: false/); + }); + + it('accepts worktree.enabled: false + --no-worktree as redundant (no error)', 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'); + + (discoverWorkflowsWithConfig as ReturnType).mockResolvedValueOnce({ + workflows: [ + makeTestWorkflowWithSource({ + name: 'triage', + description: 'Read-only triage', + worktree: { enabled: false }, + }), + ], + errors: [], + }); + (conversationDb.getOrCreateConversation as ReturnType).mockResolvedValueOnce({ + id: 'conv-123', + }); + (codebaseDb.findCodebaseByDefaultCwd as ReturnType).mockResolvedValueOnce({ + id: 'cb-123', + default_cwd: '/test/path', + }); + (conversationDb.updateConversation as ReturnType).mockResolvedValueOnce(undefined); + (executeWorkflow as ReturnType).mockResolvedValueOnce({ + success: true, + workflowRunId: 'run-123', + }); + + // Should not throw — redundant, not contradictory + await workflowRunCommand('/test/path', 'triage', 'go', { noWorktree: true }); + }); + + it('throws when workflow pins worktree.enabled: true but caller passes --no-worktree', async () => { + const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery'); + + (discoverWorkflowsWithConfig as ReturnType).mockResolvedValueOnce({ + workflows: [ + makeTestWorkflowWithSource({ + name: 'build', + description: 'Requires a worktree', + worktree: { enabled: true }, + }), + ], + errors: [], + }); + + await expect( + workflowRunCommand('/test/path', 'build', 'go', { noWorktree: true }) + ).rejects.toThrow(/worktree\.enabled: true/); + }); + it('throws when isolation cannot be created due to missing codebase', async () => { const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery'); const conversationDb = await import('@archon/core/db/conversations'); diff --git a/packages/cli/src/commands/workflow.ts b/packages/cli/src/commands/workflow.ts index 7e281db7..22130b55 100644 --- a/packages/cli/src/commands/workflow.ts +++ b/packages/cli/src/commands/workflow.ts @@ -261,6 +261,37 @@ export async function workflowRunCommand( ); } + // Reconcile workflow-level worktree policy with invocation flags. + // The workflow YAML's `worktree.enabled` pins isolation regardless of caller — + // a mismatch between policy and flags is a user error we surface loudly + // rather than silently applying one side and ignoring the other. + const pinnedEnabled = workflow.worktree?.enabled; + if (pinnedEnabled === false) { + if (options.branchName !== undefined) { + throw new Error( + `Workflow '${workflow.name}' sets worktree.enabled: false (runs in live checkout).\n` + + ' --branch requires an isolated worktree.\n' + + " Drop --branch or change the workflow's worktree.enabled." + ); + } + if (options.fromBranch !== undefined) { + throw new Error( + `Workflow '${workflow.name}' sets worktree.enabled: false (runs in live checkout).\n` + + ' --from/--from-branch only applies when a worktree is created.\n' + + " Drop --from or change the workflow's worktree.enabled." + ); + } + // --no-worktree is redundant but not contradictory — silently accept. + } else if (pinnedEnabled === true) { + if (options.noWorktree) { + throw new Error( + `Workflow '${workflow.name}' sets worktree.enabled: true (requires a worktree).\n` + + ' --no-worktree conflicts with the workflow policy.\n' + + " Drop --no-worktree or change the workflow's worktree.enabled." + ); + } + } + console.log(`Running workflow: ${workflowName}`); console.log(`Working directory: ${cwd}`); console.log(''); @@ -403,8 +434,14 @@ export async function workflowRunCommand( console.log(''); } - // Default to worktree isolation unless --no-worktree or --resume - const wantsIsolation = !options.resume && !options.noWorktree; + // Default to worktree isolation unless --no-worktree or --resume. + // Workflow YAML `worktree.enabled` pins the decision — mismatches with CLI + // flags are rejected above, so by this point the policy (if set) and flags + // agree. `--resume` reuses an existing worktree and takes precedence over + // the pinned policy to avoid disturbing a paused run. + const flagWantsIsolation = !options.resume && !options.noWorktree; + const wantsIsolation = + !options.resume && pinnedEnabled !== undefined ? pinnedEnabled : flagWantsIsolation; if (wantsIsolation && codebase) { // Auto-generate branch identifier from workflow name + timestamp when --branch not provided diff --git a/packages/core/src/config/config-types.ts b/packages/core/src/config/config-types.ts index 6e3611c5..63dd1359 100644 --- a/packages/core/src/config/config-types.ts +++ b/packages/core/src/config/config-types.ts @@ -176,6 +176,29 @@ export interface RepoConfig { * @default true */ initSubmodules?: boolean; + + /** + * Per-project worktree directory (relative to repo root). When set, + * worktrees are created at `//` instead of under + * `~/.archon/worktrees/` or the workspaces layout. + * + * Opt-in — co-locates worktrees with the repo so they appear in the IDE + * file tree. The user is responsible for adding the directory to their + * `.gitignore` (no automatic file mutation). + * + * Path resolution precedence (highest to lowest): + * 1. this `worktree.path` (repo-local) + * 2. global `paths.worktrees` (absolute override in `~/.archon/config.yaml`) + * 3. auto-detected project-scoped (`~/.archon/workspaces/owner/repo/...`) + * 4. default global (`~/.archon/worktrees/`) + * + * Must be a safe relative path: no leading `/`, no `..` segments. Absolute + * or escaping values fail loudly at worktree creation (Fail Fast — no silent + * fallback). + * + * @example '.worktrees' + */ + path?: string; }; /** diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index af748846..ba24331b 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -228,31 +228,43 @@ async function dispatchOrchestratorWorkflow( codebase_id: codebase.id, }); - // Validate and resolve isolation + // Validate and resolve isolation. + // A workflow with `worktree.enabled: false` short-circuits the resolver entirely + // and runs in the live checkout — no worktree creation, no env row. This is the + // declarative equivalent of CLI `--no-worktree` for workflows that should always + // run live (e.g. read-only triage, docs generation on the main checkout). let cwd: string; - try { - const result = await validateAndResolveIsolation( - { ...conversation, codebase_id: codebase.id }, - codebase, - platform, - conversationId, - isolationHints + if (workflow.worktree?.enabled === false) { + getLog().info( + { workflowName: workflow.name, conversationId, codebaseId: codebase.id }, + 'workflow.worktree_disabled_by_policy' ); - cwd = result.cwd; - } catch (error) { - if (error instanceof IsolationBlockedError) { - getLog().warn( - { - reason: error.reason, - conversationId, - codebaseId: codebase.id, - workflowName: workflow.name, - }, - 'isolation_blocked' + cwd = codebase.default_cwd; + } else { + try { + const result = await validateAndResolveIsolation( + { ...conversation, codebase_id: codebase.id }, + codebase, + platform, + conversationId, + isolationHints ); - return; + cwd = result.cwd; + } catch (error) { + if (error instanceof IsolationBlockedError) { + getLog().warn( + { + reason: error.reason, + conversationId, + codebaseId: codebase.id, + workflowName: workflow.name, + }, + 'isolation_blocked' + ); + return; + } + throw error; } - throw error; } // Dispatch workflow diff --git a/packages/docs-web/src/content/docs/guides/authoring-workflows.md b/packages/docs-web/src/content/docs/guides/authoring-workflows.md index 55f1a64d..5caea999 100644 --- a/packages/docs-web/src/content/docs/guides/authoring-workflows.md +++ b/packages/docs-web/src/content/docs/guides/authoring-workflows.md @@ -120,6 +120,12 @@ model: sonnet modelReasoningEffort: medium # Codex only webSearchMode: live # Codex only interactive: true # Web only: run in foreground instead of background +worktree: # Optional: pin isolation behavior regardless of caller + enabled: false # false = always run in the live checkout (CLI --no-worktree + # and web both honor it). Use for read-only workflows + # like triage/reporting. true = must use a worktree; + # CLI --no-worktree hard-errors. Omit to let the + # caller decide (current default = worktree). # Required for DAG-based nodes: diff --git a/packages/docs-web/src/content/docs/reference/configuration.md b/packages/docs-web/src/content/docs/reference/configuration.md index 11506517..a29d13f2 100644 --- a/packages/docs-web/src/content/docs/reference/configuration.md +++ b/packages/docs-web/src/content/docs/reference/configuration.md @@ -127,6 +127,10 @@ worktree: - .vscode # Copy entire directory initSubmodules: true # Optional: default true — auto-detects .gitmodules and runs # `git submodule update --init --recursive`. Set false to opt out. + path: .worktrees # Optional: co-locate worktrees with the repo at + # /.worktrees/ instead of under + # ~/.archon/workspaces///worktrees/. + # Must be relative; no absolute, no `..` segments. # Documentation directory docs: @@ -180,6 +184,8 @@ This is useful when you maintain coding style or identity preferences in `~/.cla **Docs path behavior:** The `docs.path` setting controls where the `$DOCS_DIR` variable points. When not configured, `$DOCS_DIR` defaults to `docs/`. Unlike `$BASE_BRANCH`, this variable always has a safe default and never throws an error. Configure it when your documentation lives outside the standard `docs/` directory (e.g., `packages/docs-web/src/content/docs`). +**Worktree path behavior:** By default, every repo's worktrees live under `~/.archon/workspaces///worktrees/` — outside the repo, invisible to the IDE. Set `worktree.path` to opt in to a **repo-local** layout instead: worktrees are created at `//` so they show up in the file tree and editor workspace. A common choice is `.worktrees`. Because worktrees now live inside the repository tree, you should add the directory to your `.gitignore` (Archon does not modify user-owned files). The configured path must be relative to the repo root; absolute paths and paths containing `..` segments fail loudly at worktree creation rather than silently falling back. + ## Environment Variables Environment variables override all other configuration. They are organized by category below. diff --git a/packages/git/src/git.test.ts b/packages/git/src/git.test.ts index 8f59d3b4..518a0132 100644 --- a/packages/git/src/git.test.ts +++ b/packages/git/src/git.test.ts @@ -194,79 +194,78 @@ describe('git utilities', () => { } }); - test('returns ~/.archon/worktrees by default for local (non-Docker)', () => { + test('returns workspace-scoped base for a local non-workspace repo (via path fallback)', () => { + // New-model invariant: every repo resolves to workspace-scoped. For a repo + // living outside ~/.archon/workspaces/, owner/repo is derived from the last + // two path segments (extractOwnerRepo) so the worktree base is still stable. delete process.env.WORKTREE_BASE; delete process.env.WORKSPACE_PATH; delete process.env.ARCHON_HOME; delete process.env.ARCHON_DOCKER; const result = git.getWorktreeBase('/workspace/my-repo'); - expect(result).toBe(join(homedir(), '.archon', 'worktrees')); + expect(result).toEqual({ + base: join(homedir(), '.archon', 'workspaces', 'workspace', 'my-repo', 'worktrees'), + layout: 'workspace-scoped', + }); }); - test('returns /.archon/worktrees for Docker environment', () => { - delete process.env.WORKTREE_BASE; - delete process.env.ARCHON_HOME; - process.env.WORKSPACE_PATH = '/workspace'; - const result = git.getWorktreeBase('/workspace/my-repo'); - expect(result).toBe(join('/', '.archon', 'worktrees')); - }); - - test('detects Docker by HOME=/root + WORKSPACE_PATH', () => { - delete process.env.WORKTREE_BASE; - delete process.env.ARCHON_HOME; - delete process.env.ARCHON_DOCKER; - process.env.HOME = '/root'; - process.env.WORKSPACE_PATH = '/app/workspace'; - const result = git.getWorktreeBase('/workspace/my-repo'); - expect(result).toBe(join('/', '.archon', 'worktrees')); - }); - - test('uses ARCHON_HOME for local (non-Docker)', () => { + test('uses ARCHON_HOME for the workspace-scoped base (local non-Docker)', () => { delete process.env.WORKSPACE_PATH; delete process.env.WORKTREE_BASE; delete process.env.ARCHON_DOCKER; process.env.ARCHON_HOME = '/custom/archon'; const result = git.getWorktreeBase('/workspace/my-repo'); - expect(result).toBe(join('/custom/archon', 'worktrees')); + expect(result).toEqual({ + base: join('/custom/archon', 'workspaces', 'workspace', 'my-repo', 'worktrees'), + layout: 'workspace-scoped', + }); }); - test('uses fixed path in Docker', () => { + test('uses the Docker archon home for the workspace-scoped base', () => { delete process.env.ARCHON_HOME; process.env.ARCHON_DOCKER = 'true'; const result = git.getWorktreeBase('/workspace/my-repo'); - expect(result).toBe(join('/', '.archon', 'worktrees')); + expect(result).toEqual({ + base: join('/', '.archon', 'workspaces', 'workspace', 'my-repo', 'worktrees'), + layout: 'workspace-scoped', + }); }); - test('returns project-scoped worktrees path when repo is under workspaces', () => { + test('returns workspace-scoped path when repo is already under workspaces/', () => { delete process.env.WORKSPACE_PATH; delete process.env.ARCHON_DOCKER; delete process.env.ARCHON_HOME; const workspacesPath = join(homedir(), '.archon', 'workspaces'); const repoPath = join(workspacesPath, 'acme', 'widget', 'source'); const result = git.getWorktreeBase(repoPath); - expect(result).toBe(join(workspacesPath, 'acme', 'widget', 'worktrees')); + expect(result).toEqual({ + base: join(workspacesPath, 'acme', 'widget', 'worktrees'), + layout: 'workspace-scoped', + }); }); - test('returns project-scoped path with ARCHON_HOME override', () => { + test('workspace-scoped path honors ARCHON_HOME override', () => { delete process.env.WORKSPACE_PATH; delete process.env.ARCHON_DOCKER; process.env.ARCHON_HOME = join('/', 'custom', 'archon'); const repoPath = join('/', 'custom', 'archon', 'workspaces', 'acme', 'widget', 'source'); const result = git.getWorktreeBase(repoPath); - expect(result).toBe( - join('/', 'custom', 'archon', 'workspaces', 'acme', 'widget', 'worktrees') - ); + expect(result).toEqual({ + base: join('/', 'custom', 'archon', 'workspaces', 'acme', 'widget', 'worktrees'), + layout: 'workspace-scoped', + }); }); - test('uses codebaseName to resolve project-scoped path for local repo', () => { + test('uses codebaseName to resolve workspace-scoped path for a local repo', () => { delete process.env.WORKSPACE_PATH; delete process.env.ARCHON_DOCKER; delete process.env.ARCHON_HOME; const localRepoPath = '/Users/rasmus/Projects/sasha-demo'; const result = git.getWorktreeBase(localRepoPath, 'Widinglabs/sasha-demo'); - expect(result).toBe( - join(homedir(), '.archon', 'workspaces', 'Widinglabs', 'sasha-demo', 'worktrees') - ); + expect(result).toEqual({ + base: join(homedir(), '.archon', 'workspaces', 'Widinglabs', 'sasha-demo', 'worktrees'), + layout: 'workspace-scoped', + }); }); test('codebaseName takes priority over workspaces path detection', () => { @@ -276,19 +275,52 @@ describe('git utilities', () => { const workspacesPath = join(homedir(), '.archon', 'workspaces'); const repoPath = join(workspacesPath, 'old-owner', 'old-repo', 'source'); const result = git.getWorktreeBase(repoPath, 'new-owner/new-repo'); - expect(result).toBe(join(workspacesPath, 'new-owner', 'new-repo', 'worktrees')); + expect(result).toEqual({ + base: join(workspacesPath, 'new-owner', 'new-repo', 'worktrees'), + layout: 'workspace-scoped', + }); }); - test('ignores invalid codebaseName and falls back to path detection', () => { + test('ignores invalid codebaseName and falls back to path-derived owner/repo', () => { + // "invalid-no-slash" doesn't parse as owner/repo; the layout still resolves + // to workspace-scoped using the last two segments of the repoPath. delete process.env.WORKSPACE_PATH; delete process.env.ARCHON_DOCKER; delete process.env.ARCHON_HOME; const result = git.getWorktreeBase('/local/repo', 'invalid-no-slash'); - expect(result).toBe(join(homedir(), '.archon', 'worktrees')); + expect(result).toEqual({ + base: join(homedir(), '.archon', 'workspaces', 'local', 'repo', 'worktrees'), + layout: 'workspace-scoped', + }); + }); + + test('repoLocal override wins over workspace-scoped default', () => { + delete process.env.WORKSPACE_PATH; + delete process.env.ARCHON_DOCKER; + delete process.env.ARCHON_HOME; + const repoPath = '/Users/rasmus/Projects/myapp'; + const result = git.getWorktreeBase(repoPath, undefined, { repoLocal: '.worktrees' }); + expect(result).toEqual({ + base: join(repoPath, '.worktrees'), + layout: 'repo-local', + }); + }); + + test('repoLocal override wins even for repos under workspaces/', () => { + delete process.env.WORKSPACE_PATH; + delete process.env.ARCHON_DOCKER; + delete process.env.ARCHON_HOME; + const workspacesPath = join(homedir(), '.archon', 'workspaces'); + const repoPath = join(workspacesPath, 'acme', 'widget', 'source'); + const result = git.getWorktreeBase(repoPath, 'acme/widget', { repoLocal: '.wt' }); + expect(result).toEqual({ + base: join(repoPath, '.wt'), + layout: 'repo-local', + }); }); }); - describe('isProjectScopedWorktreeBase', () => { + describe('isProjectScopedWorktreeBase (deprecated)', () => { const originalArchonHome = process.env.ARCHON_HOME; const originalWorkspacePath = process.env.WORKSPACE_PATH; const originalArchonDocker = process.env.ARCHON_DOCKER; @@ -321,19 +353,14 @@ describe('git utilities', () => { ).toBe(true); }); - test('returns false for path outside workspaces', () => { + test('returns true for a local non-workspace path (new two-layout model)', () => { + // In the pre-refactor three-layout model, this returned false (legacy global). + // Under the two-layout model every repo is workspace-scoped unless a + // `repoLocal` override is supplied, which this helper does not accept. delete process.env.WORKSPACE_PATH; delete process.env.ARCHON_DOCKER; delete process.env.ARCHON_HOME; - expect(git.isProjectScopedWorktreeBase('/workspace/my-repo')).toBe(false); - }); - - test('returns false for path under workspaces with only owner (no repo)', () => { - delete process.env.WORKSPACE_PATH; - delete process.env.ARCHON_DOCKER; - delete process.env.ARCHON_HOME; - const workspacesPath = join(homedir(), '.archon', 'workspaces'); - expect(git.isProjectScopedWorktreeBase(join(workspacesPath, 'acme'))).toBe(false); + expect(git.isProjectScopedWorktreeBase('/workspace/my-repo')).toBe(true); }); test('returns true when codebaseName is provided (local repo)', () => { @@ -345,11 +372,13 @@ describe('git utilities', () => { ); }); - test('returns false when codebaseName is invalid', () => { + test('returns true when codebaseName is invalid (falls back to path-derived)', () => { + // Under the two-layout model the helper always returns true for any resolvable + // owner/repo. Invalid codebaseName + valid repo path → still workspace-scoped. delete process.env.WORKSPACE_PATH; delete process.env.ARCHON_DOCKER; delete process.env.ARCHON_HOME; - expect(git.isProjectScopedWorktreeBase('/local/repo', 'invalid')).toBe(false); + expect(git.isProjectScopedWorktreeBase('/local/repo', 'invalid')).toBe(true); }); }); diff --git a/packages/git/src/index.ts b/packages/git/src/index.ts index adfac78b..39252ce4 100644 --- a/packages/git/src/index.ts +++ b/packages/git/src/index.ts @@ -26,6 +26,7 @@ export { getCanonicalRepoPath, verifyWorktreeOwnership, } from './worktree'; +export type { WorktreeLayout, WorktreeBaseOverride } from './worktree'; // Branch operations export { diff --git a/packages/git/src/worktree.ts b/packages/git/src/worktree.ts index 62f6d141..32ad2dbb 100644 --- a/packages/git/src/worktree.ts +++ b/packages/git/src/worktree.ts @@ -1,11 +1,6 @@ import { readFile, access } from 'fs/promises'; import { join, resolve } from 'path'; -import { - createLogger, - getArchonWorktreesPath, - getArchonWorkspacesPath, - getProjectWorktreesPath, -} from '@archon/paths'; +import { createLogger, getArchonWorkspacesPath, getProjectWorktreesPath } from '@archon/paths'; import { execFileAsync } from './exec'; import type { RepoPath, BranchName, WorktreePath, WorktreeInfo } from './types'; import { toRepoPath, toBranchName, toWorktreePath } from './types'; @@ -18,60 +13,111 @@ function getLog(): ReturnType { } /** - * Get the base directory for worktrees. + * Layout of a worktree base relative to the repository. * - * Resolution order: - * 1. If `codebaseName` is provided in "owner/repo" format, returns the project-scoped - * path directly: ~/.archon/workspaces/owner/repo/worktrees/ - * 2. For paths under ~/.archon/workspaces/owner/repo/..., extracts owner/repo from path - * and returns the project-scoped path. - * 3. Otherwise, returns the legacy global path: ~/.archon/worktrees/ + * Two layouts only — worktrees live either co-located with the repo (opt-in) + * or inside the user's archon workspace area (default for every repo): + * + * - `repo-local` — `//` (opt-in per repo config) + * - `workspace-scoped` — `~/.archon/workspaces///worktrees/` (default) + * + * In both layouts the base already includes all repo context, so callers append + * only the branch name to compose the final worktree path — there is no layout + * where owner/repo gets tacked on as a separate path segment. */ -export function getWorktreeBase(repoPath: RepoPath, codebaseName?: string): string { - // If codebase name is known, use project-scoped path directly +export type WorktreeLayout = 'repo-local' | 'workspace-scoped'; + +/** + * Override inputs for `getWorktreeBase()`. All fields are optional. + */ +export interface WorktreeBaseOverride { + /** + * Repo-relative path where worktrees should live (e.g. `.worktrees`). + * Only supported override today. Must be validated as a safe relative path + * by the caller before reaching this layer. + */ + repoLocal?: string; +} + +/** + * Resolve the `{ owner, repo }` identity used to scope archon-managed worktrees. + * + * Precedence: + * 1. Explicit `codebaseName` in `owner/repo` format (from the database / web UI) + * 2. Path segments when `repoPath` is already under `~/.archon/workspaces/owner/repo/` + * 3. Last two path segments of `repoPath` (works for any local checkout) + * + * The third fallback is what lets non-cloned / locally-registered repos still + * land in the workspace-scoped layout — every repo gets a stable owner/repo + * identity derived from its filesystem path. + */ +function resolveOwnerRepo( + repoPath: RepoPath, + codebaseName?: string +): { owner: string; repo: string } { if (codebaseName) { const parts = codebaseName.split('/'); if (parts.length === 2 && parts[0] && parts[1]) { - return getProjectWorktreesPath(parts[0], parts[1]); + return { owner: parts[0], repo: parts[1] }; } - // codebaseName present but not "owner/repo" format — fall through to path detection. - // This is intentional: safe degradation to legacy global path. getLog().warn({ codebaseName }, 'worktree.invalid_codebase_name_format'); } - // Existing path-prefix detection (cloned repos under workspaces/) const workspacesPath = getArchonWorkspacesPath(); if (repoPath.startsWith(workspacesPath)) { const relative = repoPath.substring(workspacesPath.length + 1); const parts = relative.split(/[/\\]/).filter(p => p.length > 0); if (parts.length >= 2) { - return getProjectWorktreesPath(parts[0], parts[1]); + return { owner: parts[0], repo: parts[1] }; } } - // Legacy global fallback (no codebase name, no workspace path match) - return getArchonWorktreesPath(); + // Fallback: derive from path basename/parent-basename — covers local-registered + // repos that never lived under workspaces/. Delegates to extractOwnerRepo() + // which throws on pathologically short paths. + return extractOwnerRepo(repoPath); } /** - * Check if the worktree base for a given repo path is project-scoped - * (under ~/.archon/workspaces/owner/repo/worktrees/) vs legacy global. + * Get the base directory for worktrees and the resolved layout. * - * When project-scoped, the worktree base already includes the owner/repo context, - * so callers should NOT append owner/repo again. + * Resolution (highest to lowest priority): + * 1. `override.repoLocal` → `//` (layout: `repo-local`) + * 2. Otherwise → `~/.archon/workspaces///worktrees/` + * (layout: `workspace-scoped`) * - * Resolution order mirrors `getWorktreeBase`: codebaseName → path detection → legacy. + * The `/` identity is resolved via `resolveOwnerRepo()` — see its + * docstring for the precedence. Every repo ends up with a stable workspace-scoped + * base; there is no `~/.archon/worktrees/owner/repo/` fallback layout. + */ +export function getWorktreeBase( + repoPath: RepoPath, + codebaseName?: string, + override?: WorktreeBaseOverride +): { base: string; layout: WorktreeLayout } { + if (override?.repoLocal) { + return { base: join(repoPath, override.repoLocal), layout: 'repo-local' }; + } + const { owner, repo } = resolveOwnerRepo(repoPath, codebaseName); + return { + base: getProjectWorktreesPath(owner, repo), + layout: 'workspace-scoped', + }; +} + +/** + * Check if the worktree base for a given repo path is workspace-scoped. + * + * Kept for backward compatibility with callers outside this package; prefer + * reading `layout` from `getWorktreeBase()` in new code. This helper is unaware + * of `override.repoLocal`, so it does not reflect per-repo overrides — use + * `getWorktreeBase(...).layout === 'workspace-scoped'` in override-aware code. + * + * @deprecated Use `getWorktreeBase(...).layout === 'workspace-scoped'` instead. + * This helper returned `false` for pre-workspace registered repos in the old + * two-layout model; in the current model every repo resolves to workspace-scoped + * when no override is set, so this always returns `true`. */ export function isProjectScopedWorktreeBase(repoPath: RepoPath, codebaseName?: string): boolean { - // If codebase name is known, it's always project-scoped - if (codebaseName) { - const parts = codebaseName.split('/'); - if (parts.length === 2 && parts[0] && parts[1]) return true; - // Invalid format — fall through to path detection (same safe degradation as getWorktreeBase). - } - const workspacesPath = getArchonWorkspacesPath(); - if (!repoPath.startsWith(workspacesPath)) return false; - const relative = repoPath.substring(workspacesPath.length + 1); - const parts = relative.split(/[/\\]/).filter(p => p.length > 0); - return parts.length >= 2; + return getWorktreeBase(repoPath, codebaseName).layout === 'workspace-scoped'; } /** diff --git a/packages/isolation/src/factory.ts b/packages/isolation/src/factory.ts index fa559478..73ac5666 100644 --- a/packages/isolation/src/factory.ts +++ b/packages/isolation/src/factory.ts @@ -14,7 +14,7 @@ let configuredLoader: RepoConfigLoader = () => Promise.resolve(null); /** * Configure the isolation system with a repo config loader. * Must be called before getIsolationProvider() for full functionality. - * If not called, WorktreeProvider uses a no-op loader (no custom baseBranch or copyFiles). + * If not called, WorktreeProvider uses a no-op loader (no custom baseBranch, copyFiles, or path). */ export function configureIsolation(loader: RepoConfigLoader): void { configuredLoader = loader; diff --git a/packages/isolation/src/providers/worktree.test.ts b/packages/isolation/src/providers/worktree.test.ts index f1339622..329717d3 100644 --- a/packages/isolation/src/providers/worktree.test.ts +++ b/packages/isolation/src/providers/worktree.test.ts @@ -2462,6 +2462,93 @@ describe('WorktreeProvider', () => { }); }); + // --------------------------------------------------------------------------- + // Per-repo `worktree.path` override (co-located worktrees opt-in) — #1117 successor + // --------------------------------------------------------------------------- + describe('worktree.path repo-local override', () => { + const baseRequest: IsolationRequest = { + codebaseId: 'cb-local-1', + codebaseName: 'owner/myapp', + canonicalRepoPath: '/Users/dev/Projects/myapp', + workflowType: 'task', + identifier: 'add-feature', + }; + + test('uses // when worktree.path is set', () => { + const branch = provider.generateBranchName(baseRequest); + const result = provider.getWorktreePath(baseRequest, branch, { path: '.worktrees' }); + expect(result).toBe(join('/Users/dev/Projects/myapp', '.worktrees', branch)); + }); + + test('empty / whitespace-only path is ignored and default layout applies', () => { + const branch = provider.generateBranchName(baseRequest); + const expectedDefault = join( + TEST_ARCHON_HOME, + 'workspaces', + 'owner', + 'myapp', + 'worktrees', + branch + ); + expect(provider.getWorktreePath(baseRequest, branch, { path: '' })).toBe(expectedDefault); + expect(provider.getWorktreePath(baseRequest, branch, { path: ' ' })).toBe(expectedDefault); + }); + + test('null / undefined config falls back to workspace-scoped default', () => { + const branch = provider.generateBranchName(baseRequest); + const expected = join(TEST_ARCHON_HOME, 'workspaces', 'owner', 'myapp', 'worktrees', branch); + expect(provider.getWorktreePath(baseRequest, branch, null)).toBe(expected); + expect(provider.getWorktreePath(baseRequest, branch, undefined)).toBe(expected); + expect(provider.getWorktreePath(baseRequest, branch)).toBe(expected); + }); + + test('override wins even when repo lives under ~/.archon/workspaces/', () => { + // Precedence contract: per-repo `worktree.path` is the highest layer. + // A repo that would normally land in workspaces/owner/repo/worktrees/ + // still gets a repo-local worktree when the config opts in. + const request: IsolationRequest = { + codebaseId: 'cb-local-2', + codebaseName: 'owner/repo', + canonicalRepoPath: join(TEST_ARCHON_HOME, 'workspaces', 'owner', 'repo'), + workflowType: 'task', + identifier: 'my-task', + }; + const branch = provider.generateBranchName(request); + const result = provider.getWorktreePath(request, branch, { path: 'worktrees-local' }); + expect(result).toBe( + join(TEST_ARCHON_HOME, 'workspaces', 'owner', 'repo', 'worktrees-local', branch) + ); + }); + + test('rejects an absolute worktree.path with a clear error', () => { + const branch = provider.generateBranchName(baseRequest); + expect(() => + provider.getWorktreePath(baseRequest, branch, { path: '/tmp/worktrees' }) + ).toThrow(/must be relative to the repo root/); + }); + + test('rejects a worktree.path that escapes the repo root via `..`', () => { + const branch = provider.generateBranchName(baseRequest); + expect(() => provider.getWorktreePath(baseRequest, branch, { path: '../worktrees' })).toThrow( + /must stay within the repo/ + ); + expect(() => provider.getWorktreePath(baseRequest, branch, { path: '..' })).toThrow( + /must stay within the repo/ + ); + expect(() => + provider.getWorktreePath(baseRequest, branch, { path: 'nested/../../escape' }) + ).toThrow(/must stay within the repo/); + }); + + test('accepts a nested relative path without `..`', () => { + const branch = provider.generateBranchName(baseRequest); + const result = provider.getWorktreePath(baseRequest, branch, { + path: '.archon/worktrees', + }); + expect(result).toBe(join('/Users/dev/Projects/myapp', '.archon/worktrees', branch)); + }); + }); + // --------------------------------------------------------------------------- // Additional lifecycle method tests // --------------------------------------------------------------------------- diff --git a/packages/isolation/src/providers/worktree.ts b/packages/isolation/src/providers/worktree.ts index 9d15196f..4d76c721 100644 --- a/packages/isolation/src/providers/worktree.ts +++ b/packages/isolation/src/providers/worktree.ts @@ -6,16 +6,14 @@ import { createHash } from 'crypto'; import { access, rm } from 'fs/promises'; -import { join, resolve } from 'path'; +import { isAbsolute, join, normalize as normalizePath, resolve, sep } from 'path'; import { createLogger } from '@archon/paths'; import { execFileAsync, - extractOwnerRepo, findWorktreeByBranch, getCanonicalRepoPath, getWorktreeBase, - isProjectScopedWorktreeBase, listWorktrees, mkdirAsync, removeWorktree, @@ -26,6 +24,7 @@ import { toWorktreePath, toBranchName, } from '@archon/git'; +import type { WorktreeBaseOverride } from '@archon/git'; import { getArchonWorkspacesPath } from '@archon/paths'; import type { RepoPath, WorktreeInfo } from '@archon/git'; import { copyWorktreeFiles } from '../worktree-copy'; @@ -56,18 +55,94 @@ function getLog(): ReturnType { */ const GIT_OPERATION_TIMEOUT_MS = 5 * 60 * 1000; +/** + * Validate a user-supplied `worktree.path` from `.archon/config.yaml` and return + * it as a safe relative path for `getWorktreeBase()`, or `undefined` to fall + * through to default path resolution. + * + * Rules (Fail Fast — malformed values throw; empty/whitespace values are ignored): + * - `undefined` / empty-after-trim → `undefined` (no override; default resolution applies) + * - Absolute path → throw (users must configure globally, not per-repo) + * - Contains `..` segment → throw (escapes repo root) + * - Resolved path escapes repoRoot → throw (covers symlink / nested `../` edge cases) + * + * The path is returned trimmed. The caller composes it via `join(repoRoot, result)`. + */ +function resolveRepoLocalOverride( + rawPath: string | undefined, + repoRoot: string +): string | undefined { + if (rawPath === undefined) return undefined; + const trimmed = rawPath.trim(); + if (!trimmed) return undefined; + + if (isAbsolute(trimmed)) { + throw new Error( + `.archon/config.yaml worktree.path must be relative to the repo root (got absolute: ${trimmed}). ` + + 'For an absolute location, set ~/.archon/config.yaml paths.worktrees instead.' + ); + } + + const normalized = normalizePath(trimmed); + // A plain `..` or anything that starts with `../` or contains `/../` escapes the repo. + if ( + normalized === '..' || + normalized.startsWith('../') || + normalized.startsWith('..\\') || + normalized.includes('/../') || + normalized.includes('\\..\\') + ) { + throw new Error( + `.archon/config.yaml worktree.path must stay within the repo (got: ${trimmed}). ` + + 'Remove any `..` segments.' + ); + } + + // Double-check via resolved absolute paths — catches edge cases like a path that + // normalizes clean but still escapes when joined (e.g. leading `./../` on some platforms). + // Uses `path.sep` so the "is inside repoRoot" check works on Windows (\\) as well as POSIX (/). + const resolved = resolve(repoRoot, normalized); + const repoRootResolved = resolve(repoRoot); + if (resolved !== repoRootResolved && !resolved.startsWith(repoRootResolved + sep)) { + throw new Error( + `.archon/config.yaml worktree.path resolves outside the repo root (got: ${trimmed} → ${resolved}).` + ); + } + + return normalized; +} + export class WorktreeProvider implements IIsolationProvider { readonly providerType = 'worktree'; constructor(private loadConfig: RepoConfigLoader = () => Promise.resolve(null)) {} /** - * Create an isolated environment using git worktrees + * Create an isolated environment using git worktrees. + * + * Config is loaded exactly once here and threaded through the rest of the + * `create()` call. A malformed `.archon/config.yaml` fails loudly at this + * boundary rather than being swallowed — see CLAUDE.md "Fail Fast + Explicit + * Errors". Downstream helpers assume they receive either a valid config + * object or `null`, never a second chance to reload. */ async create(request: IsolationRequest): Promise { + let repoConfig: WorktreeCreateConfig | null; + try { + repoConfig = await this.loadConfig(request.canonicalRepoPath); + } catch (error) { + const err = error as Error; + getLog().error({ err, repoPath: request.canonicalRepoPath }, 'repo_config_load_failed'); + throw new Error(`Failed to load config: ${err.message}`); + } + const branchName = toBranchName(this.generateBranchName(request)); - const worktreePath = this.getWorktreePath(request, branchName); - const envId = this.generateEnvId(request); + const worktreePath = this.getWorktreePath(request, branchName, repoConfig); + // envId is, by contract, the worktree filesystem path (see `destroy()` docstring). + // Assign directly from the resolved path to keep the invariant in sync with + // the actual directory created below — computing it via a separate helper would + // risk divergence if resolution rules change. + const envId = worktreePath; // Check for existing worktree (adoption) const existing = await this.findExisting(request, branchName, worktreePath); @@ -75,8 +150,8 @@ export class WorktreeProvider implements IIsolationProvider { return existing; } - // Create new worktree - const { warnings } = await this.createWorktree(request, worktreePath, branchName); + // Create new worktree (re-uses the already-loaded repoConfig — no double load). + const { warnings } = await this.createWorktree(request, worktreePath, branchName, repoConfig); return { id: envId, @@ -498,34 +573,29 @@ export class WorktreeProvider implements IIsolationProvider { } /** - * Generate unique environment ID - */ - generateEnvId(request: IsolationRequest): string { - const branchName = this.generateBranchName(request); - return this.getWorktreePath(request, branchName); - } - - /** - * Get worktree path for request. + * Get worktree path for a request, honoring the per-repo override if set. * - * Path format depends on the worktree base layout: - * - Project-scoped: `~/.archon/workspaces/{owner}/{repo}/worktrees/{branch}` - * - Legacy global: `~/.archon/worktrees/{owner}/{repo}/{branch}` + * Layouts (see `getWorktreeBase()` in `@archon/git` for resolution): + * - `repo-local` → `//{branch}` (opt-in) + * - `workspace-scoped` → `~/.archon/workspaces/{owner}/{repo}/worktrees/{branch}` (default) * - * 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. + * In both layouts the resolved base already carries full repo context, so the + * caller simply appends the branch name — no owner/repo namespacing here. + * + * The per-repo `config.path` is validated via `resolveRepoLocalOverride()`; + * unsafe values (absolute, `..` segments, escape-from-repoRoot) throw rather + * than silently falling back to the default layout. */ - getWorktreePath(request: IsolationRequest, branchName: string): string { - const worktreeBase = getWorktreeBase(request.canonicalRepoPath, request.codebaseName); - - if (isProjectScopedWorktreeBase(request.canonicalRepoPath, request.codebaseName)) { - return join(worktreeBase, branchName); - } - - const { owner, repo } = this.extractOwnerRepo(request.canonicalRepoPath); - return join(worktreeBase, owner, repo, branchName); + getWorktreePath( + request: IsolationRequest, + branchName: string, + config?: WorktreeCreateConfig | null + ): string { + const override: WorktreeBaseOverride = { + repoLocal: resolveRepoLocalOverride(config?.path, request.canonicalRepoPath), + }; + const { base } = getWorktreeBase(request.canonicalRepoPath, request.codebaseName, override); + return join(base, branchName); } /** @@ -621,35 +691,30 @@ export class WorktreeProvider implements IIsolationProvider { /** * Create the actual worktree. * Returns warnings that should be surfaced to the user (non-fatal issues). + * + * `repoConfig` is the already-loaded config from `create()`. Receiving it here + * keeps the work of each public entrypoint tied to exactly one config load — + * see the "Fail Fast" comment on `create()`. */ private async createWorktree( request: IsolationRequest, worktreePath: string, - branchName: string + branchName: string, + worktreeConfig: 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}`); - } - // 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 }); - } else { - const { owner, repo } = this.extractOwnerRepo(repoPath); - await mkdirAsync(join(worktreeBase, owner, repo), { recursive: true }); - } + const override: WorktreeBaseOverride = { + repoLocal: resolveRepoLocalOverride(worktreeConfig?.path, repoPath), + }; + const { base: worktreeBase } = getWorktreeBase(repoPath, request.codebaseName, override); + // In both layouts the base already carries repo context — creating it + // recursively is enough. + await mkdirAsync(worktreeBase, { recursive: true }); if (isPRIsolationRequest(request)) { // For PRs: fetch and checkout the PR branch (actual or synthetic) @@ -1141,14 +1206,6 @@ export class WorktreeProvider implements IIsolationProvider { } } - /** - * Extract owner and repo name from a repository path. - * Used for legacy global worktree base layout where owner/repo must be appended. - */ - private extractOwnerRepo(repoPath: string): { owner: string; repo: string } { - return extractOwnerRepo(toRepoPath(repoPath)); - } - /** * Generate short hash for thread identifiers */ diff --git a/packages/isolation/src/types.ts b/packages/isolation/src/types.ts index 2a3d0cb2..b369ffd7 100644 --- a/packages/isolation/src/types.ts +++ b/packages/isolation/src/types.ts @@ -248,6 +248,19 @@ export interface WorktreeCreateConfig { * Set to `false` to opt out. No-op when `.gitmodules` is absent. */ initSubmodules?: boolean; + /** + * Per-project relative path (from repo root) where worktrees should be created. + * When set, worktrees live at `//` with `repo-local` layout. + * Highest priority in path resolution — overrides project-scoped and global defaults. + * + * Must be a safe relative path: no leading `/`, no `..` segments, non-empty after trim. + * Validation is enforced in `WorktreeProvider.getWorktreePath()` (fails fast with a + * clear error rather than silently falling back). + * + * Sourced from `.archon/config.yaml > worktree.path` in the repo. + * @example '.worktrees' + */ + path?: string; } export type RepoConfigLoader = (repoPath: string) => Promise; diff --git a/packages/workflows/src/loader.test.ts b/packages/workflows/src/loader.test.ts index eff8a6d8..127b2690 100644 --- a/packages/workflows/src/loader.test.ts +++ b/packages/workflows/src/loader.test.ts @@ -93,6 +93,33 @@ describe('Workflow Loader', () => { expect(result.workflows[0].workflow.interactive).toBeUndefined(); }); + it('should parse worktree.enabled: false', async () => { + const workflowDir = join(testDir, '.archon', 'workflows'); + await mkdir(workflowDir, { recursive: true }); + const yaml = `name: triage\ndescription: read-only\nworktree:\n enabled: false\nnodes:\n - id: n\n prompt: p\n`; + await writeFile(join(workflowDir, 'triage.yaml'), yaml); + const result = await discoverWorkflows(testDir, { loadDefaults: false }); + expect(result.workflows[0].workflow.worktree).toEqual({ enabled: false }); + }); + + it('should parse worktree.enabled: true', async () => { + const workflowDir = join(testDir, '.archon', 'workflows'); + await mkdir(workflowDir, { recursive: true }); + const yaml = `name: build\ndescription: needs worktree\nworktree:\n enabled: true\nnodes:\n - id: n\n prompt: p\n`; + await writeFile(join(workflowDir, 'build.yaml'), yaml); + const result = await discoverWorkflows(testDir, { loadDefaults: false }); + expect(result.workflows[0].workflow.worktree).toEqual({ enabled: true }); + }); + + it('should omit worktree block when not present (policy is caller-decides)', async () => { + const workflowDir = join(testDir, '.archon', 'workflows'); + await mkdir(workflowDir, { recursive: true }); + const yaml = `name: normal\ndescription: no policy\nnodes:\n - id: n\n prompt: p\n`; + await writeFile(join(workflowDir, 'normal.yaml'), yaml); + const result = await discoverWorkflows(testDir, { loadDefaults: false }); + expect(result.workflows[0].workflow.worktree).toBeUndefined(); + }); + it('should parse valid DAG workflow YAML', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); await mkdir(workflowDir, { recursive: true }); diff --git a/packages/workflows/src/loader.ts b/packages/workflows/src/loader.ts index d238bed1..e4d53bfd 100644 --- a/packages/workflows/src/loader.ts +++ b/packages/workflows/src/loader.ts @@ -339,6 +339,28 @@ export function parseWorkflow(content: string, filename: string): ParseResult { } } + // Parse workflow-level worktree policy. Same warn-and-ignore pattern used + // for `interactive` / `modelReasoningEffort` — invalid values are dropped + // rather than rejected, so a typo in one workflow doesn't nuke the whole + // discovery pass. Only `worktree.enabled` is recognised today. + let worktreePolicy: { enabled?: boolean } | undefined; + if (raw.worktree !== undefined) { + if ( + typeof raw.worktree === 'object' && + raw.worktree !== null && + !Array.isArray(raw.worktree) + ) { + const rawEnabled = (raw.worktree as Record).enabled; + if (typeof rawEnabled === 'boolean') { + worktreePolicy = { enabled: rawEnabled }; + } else if (rawEnabled !== undefined) { + getLog().warn({ filename, value: rawEnabled }, 'invalid_worktree_enabled_value_ignored'); + } + } else { + getLog().warn({ filename, value: raw.worktree }, 'invalid_worktree_block_ignored'); + } + } + return { workflow: { name: raw.name, @@ -350,6 +372,7 @@ export function parseWorkflow(content: string, filename: string): ParseResult { additionalDirectories, interactive, nodes: dagNodes, + ...(worktreePolicy ? { worktree: worktreePolicy } : {}), }, error: null, }; diff --git a/packages/workflows/src/schemas/workflow.ts b/packages/workflows/src/schemas/workflow.ts index 589c6a0b..40771af5 100644 --- a/packages/workflows/src/schemas/workflow.ts +++ b/packages/workflows/src/schemas/workflow.ts @@ -22,6 +22,33 @@ export const webSearchModeSchema = z.enum(['disabled', 'cached', 'live']); export type WebSearchMode = z.infer; +// --------------------------------------------------------------------------- +// Workflow-level worktree policy +// --------------------------------------------------------------------------- + +/** + * Per-workflow worktree policy. Pins whether a run uses isolation regardless of + * how it was invoked (CLI flags, web UI, chat). When the field is omitted the + * caller's default applies — worktree for task/issue/pr, etc. + * + * Currently one field (`enabled`). Other worktree-shaped settings (copyFiles, + * initSubmodules, path, baseBranch) live in repo-level `.archon/config.yaml` + * because they are repo-wide, not per-workflow. This block is deliberately + * narrow to avoid re-expressing the repo-level knobs here. + */ +export const workflowWorktreePolicySchema = z.object({ + /** + * Pin worktree isolation on or off for this workflow. + * - `true` — always run inside a worktree; CLI `--no-worktree` hard-errors + * - `false` — always run in the live checkout; CLI `--branch` / `--from` + * hard-error, orchestrator skips isolation resolution + * - omitted — caller decides (current default = worktree for most types) + */ + enabled: z.boolean().optional(), +}); + +export type WorkflowWorktreePolicy = z.infer; + // --------------------------------------------------------------------------- // WorkflowBase — common fields shared by all workflow types // --------------------------------------------------------------------------- @@ -40,6 +67,7 @@ export const workflowBaseSchema = z.object({ fallbackModel: z.string().min(1).optional(), betas: z.array(z.string().min(1)).nonempty("'betas' must be a non-empty array").optional(), sandbox: sandboxSettingsSchema.optional(), + worktree: workflowWorktreePolicySchema.optional(), }); export type WorkflowBase = z.infer;