From c4cebc6147cdf3e031b096f139de53cf69848641 Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 12:31:07 +0200 Subject: [PATCH 01/12] fix(core): pass configured default_branch to syncWorkspace (#1273) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backports the not-yet-merged PR #1273 fix as a prerequisite for #1516. syncWorkspace was always called with baseBranch=undefined, causing auto-detection to origin/HEAD and git reset --hard on every message for managed clones. - Codebase.default_branch (nullable) + createCodebase persist - cloneRepository detects post-clone branch and stores it - syncWorkspace receives toBranchName(default_branch) with null→undefined fallback - getCurrentBranch() helper in @archon/git - handleRegisterProject detects current branch on register - Codebase OpenAPI schema (nullable) - Migration 022 (no DEFAULT — preserve NULL on upgrade) - SQLite migrateColumns entry - 000_combined.sql baseline updated --- migrations/000_combined.sql | 1 + .../022_add_default_branch_to_codebases.sql | 8 ++++++ packages/core/src/db/adapters/sqlite.ts | 14 ++++++++++ packages/core/src/db/codebases.ts | 6 ++-- packages/core/src/handlers/clone.ts | 28 +++++++++++++++++-- .../src/orchestrator/orchestrator-agent.ts | 19 ++++++++++--- packages/core/src/types/index.ts | 1 + packages/git/src/branch.ts | 12 ++++++++ packages/git/src/index.ts | 1 + .../src/routes/schemas/codebase.schemas.ts | 1 + 10 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 migrations/022_add_default_branch_to_codebases.sql diff --git a/migrations/000_combined.sql b/migrations/000_combined.sql index 176963b40e..db9c7511c4 100644 --- a/migrations/000_combined.sql +++ b/migrations/000_combined.sql @@ -29,6 +29,7 @@ CREATE TABLE IF NOT EXISTS remote_agent_codebases ( name VARCHAR(255) NOT NULL, repository_url VARCHAR(500), default_cwd VARCHAR(500) NOT NULL, + default_branch TEXT, ai_assistant_type VARCHAR(20) DEFAULT 'claude', allow_env_keys BOOLEAN NOT NULL DEFAULT FALSE, commands JSONB DEFAULT '{}'::jsonb, diff --git a/migrations/022_add_default_branch_to_codebases.sql b/migrations/022_add_default_branch_to_codebases.sql new file mode 100644 index 0000000000..5d1281898a --- /dev/null +++ b/migrations/022_add_default_branch_to_codebases.sql @@ -0,0 +1,8 @@ +-- Add default_branch column to remote_agent_codebases. +-- NULL means "not yet detected"; syncWorkspace falls back to auto-detection +-- (pre-existing behaviour). New clones set this via the branch-detect path in +-- clone.ts. Using no DEFAULT so existing rows stay NULL rather than being +-- silently set to 'main' (which could trigger an unwanted hard-reset for +-- managed clones on a non-main branch). +ALTER TABLE remote_agent_codebases + ADD COLUMN IF NOT EXISTS default_branch TEXT; diff --git a/packages/core/src/db/adapters/sqlite.ts b/packages/core/src/db/adapters/sqlite.ts index 485706d040..09e7b17b9a 100644 --- a/packages/core/src/db/adapters/sqlite.ts +++ b/packages/core/src/db/adapters/sqlite.ts @@ -215,6 +215,20 @@ export class SqliteAdapter implements IDatabase { } catch (e: unknown) { getLog().warn({ err: e as Error }, 'db.sqlite_migration_session_columns_failed'); } + + // Codebases columns + try { + const codebaseCols = this.db.prepare("PRAGMA table_info('remote_agent_codebases')").all() as { + name: string; + }[]; + const codebaseColNames = new Set(codebaseCols.map(c => c.name)); + + if (!codebaseColNames.has('default_branch')) { + this.db.run('ALTER TABLE remote_agent_codebases ADD COLUMN default_branch TEXT'); + } + } catch (e: unknown) { + getLog().warn({ err: e as Error }, 'db.sqlite_migration_codebases_columns_failed'); + } } /** diff --git a/packages/core/src/db/codebases.ts b/packages/core/src/db/codebases.ts index 27adc91557..69039d6513 100644 --- a/packages/core/src/db/codebases.ts +++ b/packages/core/src/db/codebases.ts @@ -16,12 +16,14 @@ export async function createCodebase(data: { name: string; repository_url?: string; default_cwd: string; + default_branch?: string | null; ai_assistant_type?: string; }): Promise { const assistantType = data.ai_assistant_type ?? 'claude'; + const defaultBranch = data.default_branch ?? null; const result = await pool.query( - 'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, ai_assistant_type) VALUES ($1, $2, $3, $4) RETURNING *', - [data.name, data.repository_url ?? null, data.default_cwd, assistantType] + 'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, default_branch, ai_assistant_type) VALUES ($1, $2, $3, $4, $5) RETURNING *', + [data.name, data.repository_url ?? null, data.default_cwd, defaultBranch, assistantType] ); if (!result.rows[0]) { throw new Error('Failed to create codebase: INSERT succeeded but no row returned'); diff --git a/packages/core/src/handlers/clone.ts b/packages/core/src/handlers/clone.ts index 366a951b8a..0da17f0bdd 100644 --- a/packages/core/src/handlers/clone.ts +++ b/packages/core/src/handlers/clone.ts @@ -40,7 +40,8 @@ export interface RegisterResult { async function registerRepoAtPath( targetPath: string, name: string, - repositoryUrl: string | null + repositoryUrl: string | null, + defaultBranch?: string ): Promise { // Auto-detect assistant type based on SDK folder conventions. // Built-in providers use well-known folders (.claude/, .codex/). @@ -125,6 +126,7 @@ async function registerRepoAtPath( name, repository_url: repositoryUrl ?? undefined, default_cwd: targetPath, + default_branch: defaultBranch ?? undefined, ai_assistant_type: suggestedAssistant, }); @@ -279,7 +281,29 @@ export async function cloneRepository(repoUrl: string): Promise await execFileAsync('git', ['config', '--global', '--add', 'safe.directory', targetPath]); getLog().debug({ path: targetPath }, 'safe_directory_added'); - const result = await registerRepoAtPath(targetPath, `${ownerName}/${repoName}`, workingUrl); + // Detect the actual branch checked out after clone (may differ from 'main' for repos with + // a non-default HEAD). Non-fatal: falls back to DB schema default if detection fails. + let detectedBranch: string | undefined; + try { + const { stdout } = await execFileAsync( + 'git', + ['-C', targetPath, 'rev-parse', '--abbrev-ref', 'HEAD'], + { timeout: 5000 } + ); + const branch = stdout.trim(); + if (branch && branch !== 'HEAD') { + detectedBranch = branch; + } + } catch { + // Non-fatal — leave undefined so DB default applies + } + + const result = await registerRepoAtPath( + targetPath, + `${ownerName}/${repoName}`, + workingUrl, + detectedBranch + ); getLog().info({ url: workingUrl, targetPath }, 'clone_completed'); return result; } diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index 943b0f0b58..eb698f900c 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -27,7 +27,7 @@ import { toError } from '../utils/error'; import { getAgentProvider, getProviderCapabilities } from '@archon/providers'; import { getArchonWorkspacesPath, ensureArchonWorkspacesPath } from '@archon/paths'; import { syncArchonToWorktree } from '../utils/worktree-sync'; -import { syncWorkspace, toRepoPath } from '@archon/git'; +import { syncWorkspace, getCurrentBranch, toRepoPath, toBranchName } from '@archon/git'; import type { WorkspaceSyncResult } from '@archon/git'; import { discoverWorkflowsWithConfig } from '@archon/workflows/workflow-discovery'; import { findWorkflow } from '@archon/workflows/router'; @@ -434,9 +434,13 @@ async function discoverAllWorkflows(conversation: Conversation): Promise; created_at: Date; diff --git a/packages/git/src/branch.ts b/packages/git/src/branch.ts index 7307da895c..ac162f937f 100644 --- a/packages/git/src/branch.ts +++ b/packages/git/src/branch.ts @@ -77,6 +77,18 @@ export async function getDefaultBranch(repoPath: RepoPath): Promise } } +/** + * Get the currently checked-out branch name + */ +export async function getCurrentBranch(repoPath: RepoPath): Promise { + const { stdout } = await execFileAsync( + 'git', + ['-C', repoPath, 'rev-parse', '--abbrev-ref', 'HEAD'], + { timeout: 10000 } + ); + return toBranchName(stdout.trim()); +} + /** * Checkout a branch (creating it if it doesn't exist) */ diff --git a/packages/git/src/index.ts b/packages/git/src/index.ts index 39252ce4d3..2c09639ac7 100644 --- a/packages/git/src/index.ts +++ b/packages/git/src/index.ts @@ -31,6 +31,7 @@ export type { WorktreeLayout, WorktreeBaseOverride } from './worktree'; // Branch operations export { getDefaultBranch, + getCurrentBranch, checkout, hasUncommittedChanges, commitAllChanges, diff --git a/packages/server/src/routes/schemas/codebase.schemas.ts b/packages/server/src/routes/schemas/codebase.schemas.ts index d2880a6be1..cbac16c6c7 100644 --- a/packages/server/src/routes/schemas/codebase.schemas.ts +++ b/packages/server/src/routes/schemas/codebase.schemas.ts @@ -15,6 +15,7 @@ export const codebaseSchema = z name: z.string(), repository_url: z.string().nullable(), default_cwd: z.string(), + default_branch: z.string().nullable(), ai_assistant_type: z.string(), commands: z.record(codebaseCommandSchema), created_at: z.string(), From b9ff50cf54be3743d2fb62e3ec683a43f62baec7 Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 13:21:07 +0200 Subject: [PATCH 02/12] fix(git,orchestrator): non-destructive sync default + explicit reset for worktree-creation (#1516) --- .../orchestrator/orchestrator-agent.test.ts | 34 ++-- .../src/orchestrator/orchestrator-agent.ts | 40 ++-- packages/git/src/git.test.ts | 37 +++- packages/git/src/index.ts | 1 + packages/git/src/repo.ts | 179 +++++++++++++----- packages/git/src/types.ts | 25 ++- .../isolation/src/providers/worktree.test.ts | 25 ++- packages/isolation/src/providers/worktree.ts | 14 +- 8 files changed, 245 insertions(+), 110 deletions(-) diff --git a/packages/core/src/orchestrator/orchestrator-agent.test.ts b/packages/core/src/orchestrator/orchestrator-agent.test.ts index 11ff15f0ea..220537c13b 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.test.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.test.ts @@ -24,6 +24,7 @@ const mockSyncWorkspace = mock(() => Promise.resolve({ branch: 'main', synced: true, + state: 'in_sync' as const, previousHead: 'abc12345', newHead: 'abc12345', updated: false, @@ -194,7 +195,9 @@ mock.module('../utils/worktree-sync', () => ({ mock.module('@archon/git', () => ({ syncWorkspace: mockSyncWorkspace, + getCurrentBranch: mock(() => Promise.resolve('main')), toRepoPath: mockToRepoPath, + toBranchName: (s: string) => s, })); mock.module('fs', () => ({ @@ -213,6 +216,7 @@ function makeCodebase(name: string, id = `id-${name}`): Codebase { name, repository_url: null, default_cwd: `/repos/${name}`, + default_branch: null, ai_assistant_type: 'claude', commands: {}, created_at: new Date(), @@ -836,6 +840,7 @@ function makeCodebaseForSync() { name: 'test-repo', repository_url: 'https://github.com/test/repo', default_cwd: '/repos/test-repo', + default_branch: null, ai_assistant_type: 'claude', commands: {}, created_at: new Date(), @@ -930,32 +935,25 @@ describe('discoverAllWorkflows — remote sync', () => { const platform = makePlatform(); await handleMessage(platform, 'conv-1', 'What is the latest commit?'); - // /repos/test-repo is NOT under ~/.archon/workspaces/ so resetAfterFetch=false - expect(mockSyncWorkspace).toHaveBeenCalledWith('/repos/test-repo', undefined, { - resetAfterFetch: false, - }); - // Regression guard: orchestrator must resolve cwd through the ensure variant - // so the workspaces dir is created before the AI provider spawn (issue #1528). + // Chat-tick uses default mode 'fast-forward' — non-destructive. + // No third arg passed; default kicks in inside syncWorkspace. + expect(mockSyncWorkspace).toHaveBeenCalledWith('/repos/test-repo', undefined); + // Regression guard from #1528: orchestrator must resolve cwd through the + // ensure variant so the workspaces dir is created before the AI provider + // spawn. expect(mockEnsureArchonWorkspacesPath).toHaveBeenCalled(); }); - test('passes resetAfterFetch=true for managed clones', async () => { + test('passes configured default_branch as second arg', async () => { const conversation = makeConversation({ codebase_id: 'codebase-1' }); - const codebase = { - ...makeCodebaseForSync(), - default_cwd: '/home/test/.archon/workspaces/owner/repo/source', - }; + const codebase = { ...makeCodebaseForSync(), default_branch: 'develop' }; mockGetOrCreateConversation.mockReturnValueOnce(Promise.resolve(conversation)); mockGetCodebase.mockReturnValueOnce(Promise.resolve(codebase)); const platform = makePlatform(); await handleMessage(platform, 'conv-1', 'What is the latest commit?'); - expect(mockSyncWorkspace).toHaveBeenCalledWith( - '/home/test/.archon/workspaces/owner/repo/source', - undefined, - { resetAfterFetch: true } - ); + expect(mockSyncWorkspace).toHaveBeenCalledWith('/repos/test-repo', 'develop'); }); test('proceeds without throwing when syncWorkspace rejects', async () => { @@ -970,9 +968,7 @@ describe('discoverAllWorkflows — remote sync', () => { await expect( handleMessage(platform, 'conv-1', 'What is the latest commit?') ).resolves.toBeUndefined(); - expect(mockSyncWorkspace).toHaveBeenCalledWith('/repos/test-repo', undefined, { - resetAfterFetch: false, - }); + expect(mockSyncWorkspace).toHaveBeenCalledWith('/repos/test-repo', undefined); }); test('does not call syncWorkspace when conversation has no codebase_id', async () => { diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index eb698f900c..f129a9bfa7 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -426,26 +426,20 @@ async function discoverAllWorkflows(conversation: Conversation): Promise { + test("hard-resets working tree to origin after fetch (mode: 'reset')", async () => { execSpy.mockResolvedValue({ stdout: '', stderr: '' }); - await git.syncWorkspace('/workspace/repo', 'main'); + await git.syncWorkspace('/workspace/repo', 'main', { mode: 'reset' }); const resetCalls = execSpy.mock.calls.filter((call: unknown[]) => { const args = call[1] as string[]; @@ -1368,7 +1371,7 @@ branch refs/heads/feature/auth expect(resetCalls[0][1]).toEqual(['-C', '/workspace/repo', 'reset', '--hard', 'origin/main']); }); - test('throws if reset fails after successful fetch', async () => { + test("throws if reset fails after successful fetch (mode: 'reset')", async () => { execSpy.mockImplementation(async (_cmd: string, args: string[]) => { if (args.includes('reset')) { throw new Error('fatal: Could not reset index file'); @@ -1376,7 +1379,7 @@ branch refs/heads/feature/auth return { stdout: '', stderr: '' }; }); - await expect(git.syncWorkspace('/workspace/repo', 'main')).rejects.toThrow( + await expect(git.syncWorkspace('/workspace/repo', 'main', { mode: 'reset' })).rejects.toThrow( 'Reset to origin/main failed' ); }); @@ -1428,6 +1431,7 @@ branch refs/heads/feature/auth expect(result).toEqual({ branch: 'develop', synced: true, + state: 'diverged', previousHead: '', newHead: '', updated: false, @@ -1466,16 +1470,19 @@ branch refs/heads/feature/auth await expect(git.syncWorkspace('/workspace/repo')).rejects.not.toThrow('worktree.baseBranch'); }); - test('skips reset when resetAfterFetch is false (fetch-only mode)', async () => { + test("default mode 'fast-forward' never runs reset", async () => { + // All git invocations return empty stdout (success). With empty SHAs the + // function falls through to the ancestor check; mocked execFile always + // succeeds, so both ancestor probes report true → state = 'diverged' → + // no-op (neither reset nor merge). The fetch must still happen. execSpy.mockResolvedValue({ stdout: '', stderr: '' }); - const result = await git.syncWorkspace('/workspace/repo', 'main', { - resetAfterFetch: false, - }); + const result = await git.syncWorkspace('/workspace/repo', 'main'); expect(result).toEqual({ branch: 'main', synced: true, + state: 'diverged', previousHead: '', newHead: '', updated: false, @@ -1488,13 +1495,25 @@ branch refs/heads/feature/auth }); expect(fetchCalls).toHaveLength(1); - // Reset should NOT have been called + // Reset must NOT have been called — fast-forward is non-destructive const resetCalls = execSpy.mock.calls.filter((call: unknown[]) => { const args = call[1] as string[]; return args.includes('reset'); }); expect(resetCalls).toHaveLength(0); }); + + test("explicit mode: 'reset' runs git reset --hard", async () => { + execSpy.mockResolvedValue({ stdout: '', stderr: '' }); + + await git.syncWorkspace('/workspace/repo', 'main', { mode: 'reset' }); + + const resetCalls = execSpy.mock.calls.filter((call: unknown[]) => { + const args = call[1] as string[]; + return args.includes('reset') && args.includes('--hard'); + }); + expect(resetCalls).toHaveLength(1); + }); }); describe('cloneRepository', () => { diff --git a/packages/git/src/index.ts b/packages/git/src/index.ts index 2c09639ac7..2587de9a3c 100644 --- a/packages/git/src/index.ts +++ b/packages/git/src/index.ts @@ -5,6 +5,7 @@ export type { WorktreePath, GitResult, GitError, + SyncMode, WorkspaceSyncResult, WorktreeInfo, } from './types'; diff --git a/packages/git/src/repo.ts b/packages/git/src/repo.ts index 21ae8d3571..0a06b6941e 100644 --- a/packages/git/src/repo.ts +++ b/packages/git/src/repo.ts @@ -1,7 +1,7 @@ import { createLogger } from '@archon/paths'; import { execFileAsync } from './exec'; import { getDefaultBranch } from './branch'; -import type { RepoPath, BranchName, GitResult, WorkspaceSyncResult } from './types'; +import type { RepoPath, BranchName, GitResult, SyncMode, WorkspaceSyncResult } from './types'; import { toRepoPath } from './types'; /** Lazy-initialized logger (deferred so test mocks can intercept createLogger) */ @@ -68,38 +68,47 @@ export async function getRemoteUrl(repoPath: RepoPath): Promise { /** * Sync workspace with remote origin. - * Fetches the base branch from origin, then optionally hard-resets the working tree - * to match `origin/`. * - * When `resetAfterFetch` is true (default), the working tree is hard-reset to match - * the remote. This is safe for Archon-managed clones in `~/.archon/workspaces/` but - * **destructive for user's local working directories** — callers must check the path - * before enabling reset. + * Always fetches `origin/` (a non-destructive ref update). What + * happens next depends on `mode`: * - * When `resetAfterFetch` is false, only `git fetch` runs — the local working tree is - * untouched. This is safe for locally-registered repos where the user may have - * uncommitted changes. + * - `mode: 'fast-forward'` (default): if HEAD is strictly behind origin and the + * working tree is clean, `git merge --ff-only` is applied. In every other + * case (in_sync / ahead / diverged / dirty) HEAD and working tree are left + * alone. **Cannot destroy local commits or uncommitted changes.** Suitable + * for any path where local work must be preserved (e.g. chat-tick refresh). + * + * - `mode: 'reset'`: `git reset --hard origin/` runs after fetch. + * **Destructive** — discards uncommitted changes and pulls HEAD to origin + * even if local commits exist. Suitable only where a known-clean base state + * is required (e.g. immediately before creating a new worktree). + * + * Both modes return a `state` field describing the observed relationship of + * HEAD to origin **before** the operation, plus `updated` indicating whether + * HEAD moved as a result. * * Branch resolution: - * - If baseBranch is provided: Uses that branch (from config). Fails with actionable - * error if the branch doesn't exist - no silent fallback. - * - If baseBranch is omitted: Auto-detects the default branch via git. + * - If baseBranch is provided: uses that branch. Fails with actionable error + * if the branch doesn't exist on origin — no silent fallback. + * - If baseBranch is omitted: auto-detects the default branch via git. * * @param workspacePath - Path to the workspace (canonical repo, not worktree) - * @param baseBranch - Optional base branch name (e.g., 'main', 'develop'). If omitted, auto-detects default branch - * @param options - Optional settings. `resetAfterFetch` (default true) controls whether `git reset --hard` runs after fetch. - * @returns Branch used plus whether sync was performed - * @throws Error with actionable message if configured branch doesn't exist + * @param baseBranch - Optional base branch name. If omitted, auto-detects. + * @param options - Optional. `mode` defaults to `'fast-forward'`. + * @returns Branch, observed state, head SHAs before/after, and whether HEAD moved. + * @throws Error if fetch fails, configured branch doesn't exist on remote, or + * a `'reset'`/ff-merge fails unexpectedly. */ export async function syncWorkspace( workspacePath: RepoPath, baseBranch?: BranchName, - options?: { resetAfterFetch?: boolean } + options?: { mode?: SyncMode } ): Promise { - const shouldReset = options?.resetAfterFetch ?? true; + const mode: SyncMode = options?.mode ?? 'fast-forward'; const branchToSync = baseBranch ?? (await getDefaultBranch(workspacePath)); - // Fetch from origin to ensure origin/ is up-to-date + // Fetch from origin to ensure origin/ is up-to-date. + // This only updates refs/remotes/origin — never HEAD or working tree. try { await execFileAsync('git', ['-C', workspacePath, 'fetch', 'origin', branchToSync], { timeout: 60000, @@ -115,61 +124,133 @@ export async function syncWorkspace( ) { throw new Error( `Configured base branch '${baseBranch}' not found on remote. ` + - 'Either create the branch, update worktree.baseBranch in .archon/config.yaml, ' + + 'Either create the branch, update default_branch on the codebase, ' + 'or remove the setting to use the auto-detected default branch.' ); } throw new Error(`Sync fetch from origin/${branchToSync} failed: ${err.message}`); } - if (!shouldReset) { - // Fetch-only mode: safe for locally-registered repos with uncommitted changes - return { branch: branchToSync, synced: true, previousHead: '', newHead: '', updated: false }; + // Observe HEAD vs origin/ + const previousHead = await readShortSha(workspacePath, 'HEAD'); + const originSha = await readShortSha(workspacePath, `origin/${branchToSync}`); + const dirty = await isDirty(workspacePath); + + // Compute state. `dirty` is reported as the dominant mode when present — + // callers that need both ancestor info and dirty info can re-query the repo. + let state: WorkspaceSyncResult['state']; + if (dirty) { + state = 'dirty'; + } else if (previousHead && originSha && previousHead === originSha) { + state = 'in_sync'; + } else { + const headAncestorOfOrigin = await isAncestor(workspacePath, 'HEAD', `origin/${branchToSync}`); + const originAncestorOfHead = await isAncestor(workspacePath, `origin/${branchToSync}`, 'HEAD'); + if (headAncestorOfOrigin && !originAncestorOfHead) state = 'behind'; + else if (originAncestorOfHead && !headAncestorOfOrigin) state = 'ahead'; + else state = 'diverged'; } - // Capture HEAD before reset so we can report whether anything changed - let previousHead = ''; + if (mode === 'reset') { + // Hard-reset working tree and HEAD to origin/. Destructive — + // legitimate only for known-clean-base contexts (worktree creation). + try { + await execFileAsync( + 'git', + ['-C', workspacePath, 'reset', '--hard', `origin/${branchToSync}`], + { timeout: 30000 } + ); + } catch (error) { + const err = error as Error; + throw new Error(`Reset to origin/${branchToSync} failed: ${err.message}`); + } + const newHead = await readShortSha(workspacePath, 'HEAD'); + return { + branch: branchToSync, + synced: true, + state, + previousHead, + newHead, + updated: previousHead !== newHead && previousHead !== '', + }; + } + + // mode === 'fast-forward': only safe if behind and tree clean + if (state === 'behind') { + try { + await execFileAsync( + 'git', + ['-C', workspacePath, 'merge', '--ff-only', `origin/${branchToSync}`], + { timeout: 30000 } + ); + } catch (error) { + const err = error as Error; + throw new Error(`Fast-forward merge to origin/${branchToSync} failed: ${err.message}`); + } + const newHead = await readShortSha(workspacePath, 'HEAD'); + return { + branch: branchToSync, + synced: true, + state, + previousHead, + newHead, + updated: previousHead !== newHead && previousHead !== '', + }; + } + + // No-op for in_sync, ahead, diverged, dirty — local state preserved. + return { + branch: branchToSync, + synced: true, + state, + previousHead, + newHead: previousHead, + updated: false, + }; +} + +/** Read short HEAD SHA, return '' on failure (fresh-clone / detached-HEAD edge cases) */ +async function readShortSha(workspacePath: RepoPath, ref: string): Promise { try { const { stdout } = await execFileAsync( 'git', - ['-C', workspacePath, 'rev-parse', '--short=8', 'HEAD'], + ['-C', workspacePath, 'rev-parse', '--short=8', ref], { timeout: 10000 } ); - previousHead = stdout.trim(); + return stdout.trim(); } catch { - // Non-fatal — fresh clone or detached HEAD edge case + return ''; } +} - // Hard-reset local working tree to match origin — only safe for Archon-managed - // clones, never for a user's local working directory. +/** Working tree has uncommitted modifications */ +async function isDirty(workspacePath: RepoPath): Promise { try { - await execFileAsync('git', ['-C', workspacePath, 'reset', '--hard', `origin/${branchToSync}`], { - timeout: 30000, + const { stdout } = await execFileAsync('git', ['-C', workspacePath, 'status', '--porcelain'], { + timeout: 10000, }); - } catch (error) { - const err = error as Error; - throw new Error(`Reset to origin/${branchToSync} failed: ${err.message}`); + return stdout.trim().length > 0; + } catch { + return false; } +} - let newHead = ''; +/** Ancestor check via `git merge-base --is-ancestor` (true if `ancestor` is reachable from `descendant`) */ +async function isAncestor( + workspacePath: RepoPath, + ancestor: string, + descendant: string +): Promise { try { - const { stdout } = await execFileAsync( + await execFileAsync( 'git', - ['-C', workspacePath, 'rev-parse', '--short=8', 'HEAD'], + ['-C', workspacePath, 'merge-base', '--is-ancestor', ancestor, descendant], { timeout: 10000 } ); - newHead = stdout.trim(); + return true; } catch { - // Non-fatal + return false; } - - return { - branch: branchToSync, - synced: true, - previousHead, - newHead, - updated: previousHead !== newHead && previousHead !== '', - }; } /** diff --git a/packages/git/src/types.ts b/packages/git/src/types.ts index 1434af39f1..d11c6aa694 100644 --- a/packages/git/src/types.ts +++ b/packages/git/src/types.ts @@ -36,13 +36,34 @@ export type GitError = | { code: 'no_space'; path: string } | { code: 'unknown'; message: string }; +/** + * Mode for `syncWorkspace`. + * + * - `fast-forward` (default): fetch + ff-only-merge if `HEAD` is strictly + * behind `origin/`. Never destructive — leaves HEAD unchanged when + * ahead, diverged, or working tree dirty. Suitable for any path where local + * work must be preserved (e.g. chat-tick refresh on the canonical clone). + * + * - `reset`: fetch + `git reset --hard origin/`. Destructive — discards + * uncommitted changes and pulls `HEAD` to `origin/` even if local + * commits exist. Suitable only where a known-clean base state is required + * (e.g. immediately before creating a new worktree). + */ +export type SyncMode = 'fast-forward' | 'reset'; + /** Result of a workspace sync operation */ export interface WorkspaceSyncResult { branch: BranchName; synced: boolean; - /** HEAD SHA before the reset (short, 8 chars) */ + /** + * Observed relationship between HEAD and origin/ before the + * operation. `dirty` means working tree had uncommitted changes (orthogonal + * to ancestor state but reported as dominant mode for simplicity). + */ + state: 'in_sync' | 'behind' | 'ahead' | 'diverged' | 'dirty'; + /** HEAD SHA before the operation (short, 8 chars) */ previousHead: string; - /** HEAD SHA after the reset (short, 8 chars) */ + /** HEAD SHA after the operation (short, 8 chars) */ newHead: string; /** True if the working tree was updated (HEAD changed) */ updated: boolean; diff --git a/packages/isolation/src/providers/worktree.test.ts b/packages/isolation/src/providers/worktree.test.ts index 329717d374..7b59660863 100644 --- a/packages/isolation/src/providers/worktree.test.ts +++ b/packages/isolation/src/providers/worktree.test.ts @@ -92,6 +92,7 @@ describe('WorktreeProvider', () => { syncWorkspaceSpy.mockResolvedValue({ branch: 'main', synced: true, + state: 'in_sync', previousHead: '', newHead: '', updated: false, @@ -2229,7 +2230,14 @@ describe('WorktreeProvider', () => { test('uses resolved base branch as worktree start-point', async () => { worktreeExistsSpy.mockResolvedValue(false); - syncWorkspaceSpy.mockResolvedValue({ branch: 'develop', synced: true }); + syncWorkspaceSpy.mockResolvedValue({ + branch: 'develop', + synced: true, + state: 'in_sync', + previousHead: '', + newHead: '', + updated: false, + }); const configLoader: RepoConfigLoader = async () => ({ baseBranch: 'develop' }); provider = new WorktreeProvider(configLoader); @@ -2257,10 +2265,11 @@ describe('WorktreeProvider', () => { await provider.create(baseRequest); - // syncWorkspace called with undefined → triggers auto-detect via getDefaultBranch - // resetAfterFetch: false because test path is not a managed clone under ~/.archon/workspaces + // syncWorkspace called with undefined → triggers auto-detect via getDefaultBranch. + // mode: 'fast-forward' because test path is not a managed clone under ~/.archon/workspaces + // (managed clones get mode: 'reset' for known-clean base; locally-registered preserves user work). expect(syncWorkspaceSpy).toHaveBeenCalledWith('/workspace/owner/repo', undefined, { - resetAfterFetch: false, + mode: 'fast-forward', }); }); @@ -2280,7 +2289,7 @@ describe('WorktreeProvider', () => { // fromBranch is the start-point for the branch, not for sync — sync auto-detects expect(syncWorkspaceSpy).toHaveBeenCalledWith('/workspace/owner/repo', undefined, { - resetAfterFetch: false, + mode: 'fast-forward', }); }); @@ -2299,7 +2308,7 @@ describe('WorktreeProvider', () => { await provider.create(request); expect(syncWorkspaceSpy).toHaveBeenCalledWith('/workspace/owner/repo', 'main', { - resetAfterFetch: false, + mode: 'fast-forward', }); }); @@ -2318,7 +2327,7 @@ describe('WorktreeProvider', () => { // fromBranch is ignored for non-task types, so syncWorkspace gets undefined → auto-detect expect(syncWorkspaceSpy).toHaveBeenCalledWith('/workspace/owner/repo', undefined, { - resetAfterFetch: false, + mode: 'fast-forward', }); }); @@ -2332,7 +2341,7 @@ describe('WorktreeProvider', () => { await provider.create(baseRequest); expect(syncWorkspaceSpy).toHaveBeenCalledWith('/workspace/owner/repo', 'develop', { - resetAfterFetch: false, + mode: 'fast-forward', }); expect(getDefaultBranchSpy).not.toHaveBeenCalled(); }); diff --git a/packages/isolation/src/providers/worktree.ts b/packages/isolation/src/providers/worktree.ts index 4d76c721a8..702bb4780f 100644 --- a/packages/isolation/src/providers/worktree.ts +++ b/packages/isolation/src/providers/worktree.ts @@ -24,7 +24,7 @@ import { toWorktreePath, toBranchName, } from '@archon/git'; -import type { WorktreeBaseOverride } from '@archon/git'; +import type { SyncMode, WorktreeBaseOverride } from '@archon/git'; import { getArchonWorkspacesPath } from '@archon/paths'; import type { RepoPath, WorktreeInfo } from '@archon/git'; import { copyWorktreeFiles } from '../worktree-copy'; @@ -775,17 +775,21 @@ export class WorktreeProvider implements IIsolationProvider { { repoPath, branch: configuredBaseBranch ?? 'auto-detect' }, 'workspace_sync_starting' ); - // Only hard-reset for Archon-managed clones (under ~/.archon/workspaces/). - // Locally-registered repos get fetch-only to avoid destroying uncommitted work. + // Worktree creation needs a known-clean base on the canonical clone — + // for Archon-managed clones (under ~/.archon/workspaces/) we hard-reset. + // Locally-registered repos may contain the user's uncommitted work in + // the canonical path, so we use the non-destructive fast-forward mode + // there and rely on the worktree-add itself to use origin/. const isManagedClone = repoPath .replace(/\\/g, '/') .startsWith(getArchonWorkspacesPath().replace(/\\/g, '/')); + const mode: SyncMode = isManagedClone ? 'reset' : 'fast-forward'; const { branch } = await syncWorkspace( repoPath, configuredBaseBranch ? toBranchName(configuredBaseBranch) : undefined, - { resetAfterFetch: isManagedClone } + { mode } ); - getLog().debug({ repoPath, branch }, 'workspace_synced'); + getLog().debug({ repoPath, branch, mode }, 'workspace_synced'); return branch; } catch (error) { const err = error as Error & { code?: string }; From f45f7b67e483a5f63715fdd02faf8821ea5723d3 Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 13:26:57 +0200 Subject: [PATCH 03/12] feat(orchestrator): post-message reminder for unpushed work in source/ (#1516) --- .../src/orchestrator/orchestrator-agent.ts | 12 +++ .../src/orchestrator/post-message-reminder.ts | 78 +++++++++++++++++++ packages/git/src/branch.ts | 32 ++++++++ packages/git/src/index.ts | 1 + 4 files changed, 123 insertions(+) create mode 100644 packages/core/src/orchestrator/post-message-reminder.ts diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index f129a9bfa7..37f0769b1f 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -42,6 +42,7 @@ import { loadConfig } from '../config/config-loader'; import type { MergedConfig } from '../config/config-types'; import { generateAndSetTitle } from '../services/title-generator'; import { validateAndResolveIsolation, dispatchBackgroundWorkflow } from './orchestrator'; +import { reportUnpushedWorkInSource } from './post-message-reminder'; import { IsolationBlockedError } from '@archon/isolation'; import { buildOrchestratorPrompt, @@ -912,6 +913,17 @@ export async function handleMessage( ); } + // Post-message advisory: warn if the agent (or anyone) left unpushed work + // in source/. Non-destructive sync default preserves such work, but it is + // still local-only and at risk if a /worktree create or external git op + // runs next. Only meaningful when a codebase is attached. + if (conversation.codebase_id) { + const attachedCodebase = codebases.find(c => c.id === conversation.codebase_id); + if (attachedCodebase) { + await reportUnpushedWorkInSource(platform, conversationId, attachedCodebase); + } + } + getLog().debug({ conversationId }, 'orchestrator_message_completed'); } catch (error) { const err = toError(error); diff --git a/packages/core/src/orchestrator/post-message-reminder.ts b/packages/core/src/orchestrator/post-message-reminder.ts new file mode 100644 index 0000000000..4b4e599120 --- /dev/null +++ b/packages/core/src/orchestrator/post-message-reminder.ts @@ -0,0 +1,78 @@ +/** + * Post-message reminder: warn the user about unpushed work in source/. + * + * Background: with the non-destructive sync default (#1516), local commits and + * uncommitted modifications in `source/` are now preserved across chat ticks + * (good). However, the chat-mode agent does not auto-push on completion the way + * workflow-mode does. Without an explicit reminder, users can accumulate + * unpushed work in `source/` and lose it later — for example via: + * - A subsequent `/worktree create` (which uses `mode: 'reset'` for managed + * clones to ensure a clean base, discarding local commits) + * - Manual git operations from another terminal + * - Re-cloning the codebase + * + * This helper runs at the end of `handleMessage` for codebase-attached + * conversations and emits a single non-blocking system_status SSE event when + * `source/` has unpushed commits or uncommitted modifications. + */ + +import { createLogger } from '@archon/paths'; +import { countCommitsAhead, hasUncommittedChanges, toRepoPath, toBranchName } from '@archon/git'; +import type { IPlatformAdapter } from '../types'; +import type { Codebase } from '../types'; + +let cachedLog: ReturnType | undefined; +function getLog(): ReturnType { + if (!cachedLog) cachedLog = createLogger('orchestrator.post-message-reminder'); + return cachedLog; +} + +/** + * Inspect `codebase.default_cwd` for unpushed work and emit a single advisory + * `system_status` event if anything is at risk of being lost on the next + * destructive sync (e.g. `/worktree create`). + * + * Non-fatal: any error is logged at debug and swallowed — this is an advisory + * UX feature, not a safety boundary. + */ +export async function reportUnpushedWorkInSource( + platform: IPlatformAdapter, + conversationId: string, + codebase: Codebase +): Promise { + if (!platform.sendStructuredEvent) return; + + const sourcePath = toRepoPath(codebase.default_cwd); + const branchName = codebase.default_branch ? toBranchName(codebase.default_branch) : null; + + try { + const dirty = await hasUncommittedChanges(sourcePath); + // If we don't know the branch, we can't compare to origin/. + // Skip the unpushed-commits check; only the dirty bit is informative. + const aheadCount = branchName ? await countCommitsAhead(sourcePath, branchName) : 0; + + if (aheadCount === 0 && !dirty) return; + + const parts: string[] = []; + if (aheadCount > 0) { + parts.push(`${String(aheadCount)} unpushed commit${aheadCount === 1 ? '' : 's'}`); + } else if (aheadCount === -1) { + parts.push('unpushed local branch'); + } + if (dirty) { + parts.push('uncommitted changes'); + } + + const summary = parts.join(' and '); + const branchLabel = branchName ? ` on ${branchName}` : ''; + await platform.sendStructuredEvent(conversationId, { + type: 'system', + content: `source/ has ${summary}${branchLabel}. Push or commit + push to preserve — local-only state may be lost on the next worktree creation, manual checkout, or re-clone.`, + }); + } catch (err) { + getLog().debug( + { err: err as Error, codebaseId: codebase.id }, + 'post_message_reminder.check_failed' + ); + } +} diff --git a/packages/git/src/branch.ts b/packages/git/src/branch.ts index ac162f937f..6c3060a46e 100644 --- a/packages/git/src/branch.ts +++ b/packages/git/src/branch.ts @@ -89,6 +89,38 @@ export async function getCurrentBranch(repoPath: RepoPath): Promise return toBranchName(stdout.trim()); } +/** + * Count commits in HEAD that are not in `origin/` (i.e. unpushed work). + * + * Returns: + * - `n >= 0`: HEAD has `n` commits not yet on origin/ + * - `-1`: origin/ does not exist locally (e.g. branch never pushed) — + * treat as "everything is unpushed" for warning purposes + * + * Non-throwing: callers use this for advisory warnings, not safety-critical + * decisions. If git fails for an unexpected reason, returns 0 (silent — better + * to under-warn than to spam on every message). + */ +export async function countCommitsAhead(repoPath: RepoPath, branch: BranchName): Promise { + try { + const { stdout } = await execFileAsync( + 'git', + ['-C', repoPath, 'rev-list', '--count', `origin/${branch}..HEAD`], + { timeout: 10000 } + ); + const count = parseInt(stdout.trim(), 10); + return Number.isFinite(count) ? count : 0; + } catch (error) { + const err = error as Error; + const msg = err.message.toLowerCase(); + // origin/ doesn't exist locally — branch never pushed + if (msg.includes('unknown revision') || msg.includes('bad revision')) { + return -1; + } + return 0; + } +} + /** * Checkout a branch (creating it if it doesn't exist) */ diff --git a/packages/git/src/index.ts b/packages/git/src/index.ts index 2587de9a3c..c8b05ea213 100644 --- a/packages/git/src/index.ts +++ b/packages/git/src/index.ts @@ -33,6 +33,7 @@ export type { WorktreeLayout, WorktreeBaseOverride } from './worktree'; export { getDefaultBranch, getCurrentBranch, + countCommitsAhead, checkout, hasUncommittedChanges, commitAllChanges, From 19e3424093835863ae6c0d6a721b36de72782db4 Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 13:47:22 +0200 Subject: [PATCH 04/12] test(git): align expectations with updated 'default_branch' error message --- packages/git/src/git.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/git/src/git.test.ts b/packages/git/src/git.test.ts index 7e1080af6e..c25bfa00f1 100644 --- a/packages/git/src/git.test.ts +++ b/packages/git/src/git.test.ts @@ -1451,7 +1451,7 @@ branch refs/heads/feature/auth "Configured base branch 'does-not-exist' not found on remote" ); await expect(git.syncWorkspace('/workspace/repo', 'does-not-exist')).rejects.toThrow( - 'update worktree.baseBranch' + 'update default_branch on the codebase' ); }); @@ -1467,7 +1467,9 @@ branch refs/heads/feature/auth await expect(git.syncWorkspace('/workspace/repo')).rejects.toThrow( 'Sync fetch from origin/main failed' ); - await expect(git.syncWorkspace('/workspace/repo')).rejects.not.toThrow('worktree.baseBranch'); + await expect(git.syncWorkspace('/workspace/repo')).rejects.not.toThrow( + 'update default_branch on the codebase' + ); }); test("default mode 'fast-forward' never runs reset", async () => { From 79d47038c90b1f4e7306f4eacab9fabe8443aa70 Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 14:03:36 +0200 Subject: [PATCH 05/12] test(codebases): align fixtures with default_branch column from #1273 backport --- packages/core/src/db/codebases.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/core/src/db/codebases.test.ts b/packages/core/src/db/codebases.test.ts index b9bdbb6f1f..517ba1a91d 100644 --- a/packages/core/src/db/codebases.test.ts +++ b/packages/core/src/db/codebases.test.ts @@ -35,6 +35,7 @@ describe('codebases', () => { name: 'test-project', repository_url: 'https://github.com/user/repo', default_cwd: '/workspace/test-project', + default_branch: null, ai_assistant_type: 'claude', commands: { plan: { path: '.claude/commands/plan.md', description: 'Plan feature' } }, created_at: new Date(), @@ -54,8 +55,8 @@ describe('codebases', () => { expect(result).toEqual(mockCodebase); expect(mockQuery).toHaveBeenCalledWith( - 'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, ai_assistant_type) VALUES ($1, $2, $3, $4) RETURNING *', - ['test-project', 'https://github.com/user/repo', '/workspace/test-project', 'claude'] + 'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, default_branch, ai_assistant_type) VALUES ($1, $2, $3, $4, $5) RETURNING *', + ['test-project', 'https://github.com/user/repo', '/workspace/test-project', null, 'claude'] ); }); @@ -73,8 +74,8 @@ describe('codebases', () => { expect(result).toEqual(codebaseWithoutOptional); expect(mockQuery).toHaveBeenCalledWith( - 'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, ai_assistant_type) VALUES ($1, $2, $3, $4) RETURNING *', - ['test-project', null, '/workspace/test-project', 'claude'] + 'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, default_branch, ai_assistant_type) VALUES ($1, $2, $3, $4, $5) RETURNING *', + ['test-project', null, '/workspace/test-project', null, 'claude'] ); }); From bf9b46bccbe0e0a2a23c120af26bd64f71acd7b2 Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 14:03:50 +0200 Subject: [PATCH 06/12] fix(db): drop DEFAULT 'main' from fresh-DB schemas (CodeRabbit review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both PostgreSQL baseline and SQLite createSchema declared DEFAULT 'main', diverging from migration 022 which uses NULL. Fresh installs got 'main', migrated installs got NULL — same hard-reset-to-wrong-branch risk this PR is trying to remove. --- migrations/000_combined.sql | 2 +- packages/core/src/db/adapters/sqlite.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/migrations/000_combined.sql b/migrations/000_combined.sql index db9c7511c4..805cf9872d 100644 --- a/migrations/000_combined.sql +++ b/migrations/000_combined.sql @@ -29,7 +29,7 @@ CREATE TABLE IF NOT EXISTS remote_agent_codebases ( name VARCHAR(255) NOT NULL, repository_url VARCHAR(500), default_cwd VARCHAR(500) NOT NULL, - default_branch TEXT, + default_branch TEXT, -- NULL means "not yet detected" (auto-detect at sync); see migration 022 ai_assistant_type VARCHAR(20) DEFAULT 'claude', allow_env_keys BOOLEAN NOT NULL DEFAULT FALSE, commands JSONB DEFAULT '{}'::jsonb, diff --git a/packages/core/src/db/adapters/sqlite.ts b/packages/core/src/db/adapters/sqlite.ts index 09e7b17b9a..69bb39227b 100644 --- a/packages/core/src/db/adapters/sqlite.ts +++ b/packages/core/src/db/adapters/sqlite.ts @@ -248,7 +248,7 @@ export class SqliteAdapter implements IDatabase { name TEXT NOT NULL, repository_url TEXT, default_cwd TEXT NOT NULL, - default_branch TEXT DEFAULT 'main', + default_branch TEXT, ai_assistant_type TEXT DEFAULT 'claude', commands TEXT DEFAULT '{}', created_at TEXT DEFAULT (datetime('now')), From c0137440c634b7a910c9ba91b1a3d926eaf9f45c Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 14:04:14 +0200 Subject: [PATCH 07/12] fix(reminder): keep toRepoPath/toBranchName inside try (CodeRabbit review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Non-fatal contract was broken when default_cwd is empty — toRepoPath throws RepoPath cannot be empty before the try block opens. --- packages/core/src/orchestrator/post-message-reminder.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/core/src/orchestrator/post-message-reminder.ts b/packages/core/src/orchestrator/post-message-reminder.ts index 4b4e599120..1f6ebb3c10 100644 --- a/packages/core/src/orchestrator/post-message-reminder.ts +++ b/packages/core/src/orchestrator/post-message-reminder.ts @@ -42,10 +42,12 @@ export async function reportUnpushedWorkInSource( ): Promise { if (!platform.sendStructuredEvent) return; - const sourcePath = toRepoPath(codebase.default_cwd); - const branchName = codebase.default_branch ? toBranchName(codebase.default_branch) : null; - try { + // toRepoPath/toBranchName throw on empty strings — keep them inside the + // try so the non-fatal contract holds even on degenerate codebase rows. + const sourcePath = toRepoPath(codebase.default_cwd); + const branchName = codebase.default_branch ? toBranchName(codebase.default_branch) : null; + const dirty = await hasUncommittedChanges(sourcePath); // If we don't know the branch, we can't compare to origin/. // Skip the unpushed-commits check; only the dirty bit is informative. From 3a369c4beba43a398d8f295a78ffc24f7c094b4f Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 14:05:31 +0200 Subject: [PATCH 08/12] fix(isolation): require directory boundary in managed-clone path check (CodeRabbit review) startsWith(workspacesPath) matched siblings like workspaces-old/. Normalize trailing slashes and require an explicit / separator (or exact match). --- packages/isolation/src/providers/worktree.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/isolation/src/providers/worktree.ts b/packages/isolation/src/providers/worktree.ts index 702bb4780f..f326db4fb0 100644 --- a/packages/isolation/src/providers/worktree.ts +++ b/packages/isolation/src/providers/worktree.ts @@ -780,9 +780,14 @@ export class WorktreeProvider implements IIsolationProvider { // Locally-registered repos may contain the user's uncommitted work in // the canonical path, so we use the non-destructive fast-forward mode // there and rely on the worktree-add itself to use origin/. - const isManagedClone = repoPath - .replace(/\\/g, '/') - .startsWith(getArchonWorkspacesPath().replace(/\\/g, '/')); + // + // Path comparison normalizes trailing slashes and requires a directory + // separator boundary so siblings like `/workspaces-old/...` are + // not misclassified as managed. + const normalizedRepoPath = repoPath.replace(/\\/g, '/').replace(/\/+$/, ''); + const managedRoot = getArchonWorkspacesPath().replace(/\\/g, '/').replace(/\/+$/, ''); + const isManagedClone = + normalizedRepoPath === managedRoot || normalizedRepoPath.startsWith(`${managedRoot}/`); const mode: SyncMode = isManagedClone ? 'reset' : 'fast-forward'; const { branch } = await syncWorkspace( repoPath, From ba542aea26b1786c9e782806580a128a152e6b26 Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 14:05:58 +0200 Subject: [PATCH 09/12] fix(git): require HEAD on target branch before fast-forward merge (CodeRabbit review) Without the branch check, a topic branch that happens to be an ancestor of origin/ would silently advance to origin's tip, violating the non-default-branches-preserved guarantee. Use getCurrentBranch and noop unless HEAD == branchToSync. --- packages/git/src/repo.ts | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/git/src/repo.ts b/packages/git/src/repo.ts index 0a06b6941e..38c0a2493e 100644 --- a/packages/git/src/repo.ts +++ b/packages/git/src/repo.ts @@ -1,6 +1,6 @@ import { createLogger } from '@archon/paths'; import { execFileAsync } from './exec'; -import { getDefaultBranch } from './branch'; +import { getCurrentBranch, getDefaultBranch } from './branch'; import type { RepoPath, BranchName, GitResult, SyncMode, WorkspaceSyncResult } from './types'; import { toRepoPath } from './types'; @@ -175,8 +175,31 @@ export async function syncWorkspace( }; } - // mode === 'fast-forward': only safe if behind and tree clean + // mode === 'fast-forward': only safe if behind, tree clean, AND HEAD is on + // the target branch. Without the branch check, a topic branch that happens + // to be an ancestor of `origin/` would silently advance to + // origin's tip — violating the "non-default branches are preserved" guarantee. if (state === 'behind') { + let currentBranch: string | undefined; + try { + currentBranch = await getCurrentBranch(workspacePath); + } catch { + // Detached HEAD or unreadable — treat as "not on target branch", noop. + } + + if (currentBranch !== branchToSync) { + // HEAD is on a different branch (or detached). Skip the merge to + // preserve user state; the agent still sees fresh remote refs via fetch. + return { + branch: branchToSync, + synced: true, + state, + previousHead, + newHead: previousHead, + updated: false, + }; + } + try { await execFileAsync( 'git', From a8239426b6808a679ebc88f3566c08ab0f083ed4 Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 14:06:33 +0200 Subject: [PATCH 10/12] fix(orchestrator): re-fetch conversation when codebase_id was just attached (CodeRabbit review) dispatchOrchestratorWorkflow may have just persisted codebase_id (auto-attach on first turn), so the in-memory conversation can be stale. Re-read by platform id when codebase_id isn't set in our snapshot. --- .../core/src/orchestrator/orchestrator-agent.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index 37f0769b1f..825f47aaaa 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -917,8 +917,18 @@ export async function handleMessage( // in source/. Non-destructive sync default preserves such work, but it is // still local-only and at risk if a /worktree create or external git op // runs next. Only meaningful when a codebase is attached. - if (conversation.codebase_id) { - const attachedCodebase = codebases.find(c => c.id === conversation.codebase_id); + // + // dispatchOrchestratorWorkflow may have just persisted codebase_id (auto- + // attach on first turn), so the in-memory `conversation` can be stale. Re- + // read by platform id when codebase_id isn't set in our snapshot. + const reminderCodebaseId = + conversation.codebase_id ?? + (await db + .getConversationByPlatformId(platform.getPlatformType(), conversationId) + .then(c => c?.codebase_id ?? null) + .catch(() => null)); + if (reminderCodebaseId) { + const attachedCodebase = codebases.find(c => c.id === reminderCodebaseId); if (attachedCodebase) { await reportUnpushedWorkInSource(platform, conversationId, attachedCodebase); } From 135065747fbb662bc5b54724aea62d0a54e3a169 Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 16:33:09 +0200 Subject: [PATCH 11/12] fix(git): exclude untracked files from dirty check (#1516) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Untracked files (e.g. .DS_Store on macOS, claude/skill caches) are safe across all syncWorkspace paths — neither git merge --ff-only nor git reset --hard origin/ ever touches untracked files. Treating them as 'dirty' was blocking fast-forward sync on every checkout where the user happens to have local-only files, despite there being no destructive risk to defend against. Add --untracked-files=no to the porcelain check; tracked modifications still block ff-merge as before. --- packages/git/src/repo.ts | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/git/src/repo.ts b/packages/git/src/repo.ts index 38c0a2493e..792dfd0c55 100644 --- a/packages/git/src/repo.ts +++ b/packages/git/src/repo.ts @@ -246,12 +246,27 @@ async function readShortSha(workspacePath: RepoPath, ref: string): Promise` ever touch untracked files. Treating + * untracked files as "dirty" would block fast-forward sync on every macOS + * checkout (`.DS_Store`) or every claude/skill cache write — false positives + * with no destructive risk to defend against. + * + * (Distinct from `branch.ts:hasUncommittedChanges`, which keeps the broader + * default for worktree-cleanup safety and intentionally refuses to delete + * worktrees that have any local files at all.) + */ async function isDirty(workspacePath: RepoPath): Promise { try { - const { stdout } = await execFileAsync('git', ['-C', workspacePath, 'status', '--porcelain'], { - timeout: 10000, - }); + const { stdout } = await execFileAsync( + 'git', + ['-C', workspacePath, 'status', '--porcelain', '--untracked-files=no'], + { timeout: 10000 } + ); return stdout.trim().length > 0; } catch { return false; From 90bcf0dda67286fed226502298837f0890b9472b Mon Sep 17 00:00:00 2001 From: Zolto Date: Sat, 2 May 2026 17:29:00 +0200 Subject: [PATCH 12/12] fix(orchestrator): treat detached HEAD as undetected branch in handleRegisterProject (CodeRabbit round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getCurrentBranch returns the literal string 'HEAD' on detached HEAD, not an exception. he catch block never fires for detached HEAD, so the codebase row ended up with default_branch='HEAD' — which later gets passed to syncWorkspace as a branch name and fails the fetch with 'Configured base branch HEAD not found on remote'. Mirror the existing guard in clone.ts:cloneRepository: only assign the branch when it's a real name. --- packages/core/src/orchestrator/orchestrator-agent.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index 825f47aaaa..cee1fc1efd 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -1392,9 +1392,16 @@ async function handleRegisterProject( const config = await loadConfig(); let detectedBranch: string | undefined; try { - detectedBranch = await getCurrentBranch(toRepoPath(projectPath)); + const branch = await getCurrentBranch(toRepoPath(projectPath)); + // Detached HEAD returns the literal string "HEAD" (not an exception) — + // treat as "not detected" so the codebase row gets NULL and syncWorkspace + // auto-detects the default branch on first sync. Mirrors the same guard + // already used in clone.ts:cloneRepository. + if (branch && branch !== 'HEAD') { + detectedBranch = branch; + } } catch { - // non-git directory or detached HEAD — fall back to DB default 'main' + // non-git directory — leave undefined so DB stores NULL (auto-detect at sync) } const codebase = await codebaseDb.createCodebase({ name: projectName,