Skip to content

fix(core): respect worktree.baseBranch in cleanup service #1424

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Closed
truffle-dev wants to merge 1 commit into from
Closed

fix(core): respect worktree.baseBranch in cleanup service #1424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 134 additions & 0 deletions packages/core/src/services/cleanup-service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ mock.module('../db/codebases', () => ({
getCodebase: mockGetCodebase,
}));

// Mock repo config loader (cleanup service consults `.archon/config.yaml`
// for `worktree.baseBranch` before falling back to git auto-detection)
const mockLoadRepoConfig = mock(() => Promise.resolve({} as Record<string, unknown>));
mock.module('../config/config-loader', () => ({
loadRepoConfig: mockLoadRepoConfig,
}));

import {
runScheduledCleanup,
startCleanupScheduler,
Expand All @@ -118,12 +125,14 @@ describe('cleanup-service', () => {
mockUpdateStatus.mockClear();
mockGetById.mockClear();
mockGetCodebase.mockClear();
mockLoadRepoConfig.mockClear();
// Reset defaults
mockHasUncommittedChanges.mockResolvedValue(false);
mockWorktreeExists.mockResolvedValue(false);
mockGetDefaultBranch.mockResolvedValue('main');
mockIsBranchMerged.mockResolvedValue(false);
mockGetLastCommitDate.mockResolvedValue(null);
mockLoadRepoConfig.mockResolvedValue({});
});

describe('removeEnvironment', () => {
Expand Down Expand Up @@ -457,12 +466,14 @@ describe('runScheduledCleanup', () => {
mockGetById.mockClear();
mockGetCodebase.mockClear();
mockDeleteOldSessions.mockClear();
mockLoadRepoConfig.mockClear();
// Reset defaults
mockHasUncommittedChanges.mockResolvedValue(false);
mockWorktreeExists.mockResolvedValue(false);
mockGetDefaultBranch.mockResolvedValue('main');
mockIsBranchMerged.mockResolvedValue(false);
mockGetLastCommitDate.mockResolvedValue(null);
mockLoadRepoConfig.mockResolvedValue({});
});

test('returns empty report when no environments exist', async () => {
Expand Down Expand Up @@ -776,6 +787,86 @@ describe('runScheduledCleanup', () => {
error: 'database locked',
});
});

test('uses worktree.baseBranch from repo config and skips git auto-detection', async () => {
mockLoadRepoConfig.mockResolvedValueOnce({ worktree: { baseBranch: 'master' } });
mockListAllActiveWithCodebase.mockResolvedValueOnce([
{
id: 'env-config',
codebase_id: 'codebase-1',
workflow_type: 'issue',
workflow_id: '42',
provider: 'worktree',
codebase_default_cwd: '/workspace/repo',
working_path: '/path/repo-issue-42',
branch_name: 'issue-42',
status: 'active',
created_at: new Date(),
created_by_platform: 'github',
metadata: {},
},
]);
mockWorktreeExists.mockResolvedValueOnce(true);

await runScheduledCleanup();

expect(mockLoadRepoConfig).toHaveBeenCalledWith('/workspace/repo');
expect(mockGetDefaultBranch).not.toHaveBeenCalled();
expect(mockIsBranchMerged).toHaveBeenCalledWith('/workspace/repo', 'issue-42', 'master');
});

test('falls back to getDefaultBranch when repo config has no baseBranch', async () => {
mockLoadRepoConfig.mockResolvedValueOnce({});
mockListAllActiveWithCodebase.mockResolvedValueOnce([
{
id: 'env-default',
codebase_id: 'codebase-1',
workflow_type: 'issue',
workflow_id: '43',
provider: 'worktree',
codebase_default_cwd: '/workspace/repo',
working_path: '/path/repo-issue-43',
branch_name: 'issue-43',
status: 'active',
created_at: new Date(),
created_by_platform: 'github',
metadata: {},
},
]);
mockWorktreeExists.mockResolvedValueOnce(true);

await runScheduledCleanup();

expect(mockLoadRepoConfig).toHaveBeenCalledWith('/workspace/repo');
expect(mockGetDefaultBranch).toHaveBeenCalledWith('/workspace/repo');
expect(mockIsBranchMerged).toHaveBeenCalledWith('/workspace/repo', 'issue-43', 'main');
});

test('treats whitespace-only baseBranch as unset and falls back to getDefaultBranch', async () => {
mockLoadRepoConfig.mockResolvedValueOnce({ worktree: { baseBranch: ' ' } });
mockListAllActiveWithCodebase.mockResolvedValueOnce([
{
id: 'env-blank',
codebase_id: 'codebase-1',
workflow_type: 'issue',
workflow_id: '44',
provider: 'worktree',
codebase_default_cwd: '/workspace/repo',
working_path: '/path/repo-issue-44',
branch_name: 'issue-44',
status: 'active',
created_at: new Date(),
created_by_platform: 'github',
metadata: {},
},
]);
mockWorktreeExists.mockResolvedValueOnce(true);

await runScheduledCleanup();

expect(mockGetDefaultBranch).toHaveBeenCalledWith('/workspace/repo');
expect(mockIsBranchMerged).toHaveBeenCalledWith('/workspace/repo', 'issue-44', 'main');
});
});

describe('SESSION_RETENTION_DAYS', () => {
Expand Down Expand Up @@ -831,9 +922,11 @@ describe('getWorktreeStatusBreakdown', () => {
mockGetDefaultBranch.mockClear();
mockIsBranchMerged.mockClear();
mockListByCodebaseWithAge.mockClear();
mockLoadRepoConfig.mockClear();
// Reset defaults
mockGetDefaultBranch.mockResolvedValue('main');
mockIsBranchMerged.mockResolvedValue(false);
mockLoadRepoConfig.mockResolvedValue({});
});

test('returns correct breakdown with mixed environments', async () => {
Expand Down Expand Up @@ -917,6 +1010,26 @@ describe('getWorktreeStatusBreakdown', () => {
expect(breakdown.stale).toBe(0);
expect(breakdown.active).toBe(0);
});

test('uses worktree.baseBranch from repo config for merge detection', async () => {
mockLoadRepoConfig.mockResolvedValueOnce({ worktree: { baseBranch: 'master' } });
mockListByCodebaseWithAge.mockResolvedValueOnce([
{
id: 'env-cfg',
branch_name: 'feature-branch',
created_by_platform: 'github',
days_since_activity: 1,
working_path: '/path/feature',
status: 'active',
},
]);

await getWorktreeStatusBreakdown('codebase-1', '/workspace/repo');

expect(mockLoadRepoConfig).toHaveBeenCalledWith('/workspace/repo');
expect(mockGetDefaultBranch).not.toHaveBeenCalled();
expect(mockIsBranchMerged).toHaveBeenCalledWith('/workspace/repo', 'feature-branch', 'master');
});
});

describe('cleanupMergedWorktrees', () => {
Expand All @@ -932,6 +1045,7 @@ describe('cleanupMergedWorktrees', () => {
mockWorktreeExists.mockClear();
mockGetCodebase.mockClear();
mockUpdateStatus.mockClear();
mockLoadRepoConfig.mockClear();
// Reset defaults
mockGetDefaultBranch.mockResolvedValue('main');
mockIsBranchMerged.mockResolvedValue(false);
Expand All @@ -941,6 +1055,7 @@ describe('cleanupMergedWorktrees', () => {
mockGetPrState.mockResolvedValue('NONE');
mockHasUncommittedChanges.mockResolvedValue(false);
mockWorktreeExists.mockResolvedValue(false);
mockLoadRepoConfig.mockResolvedValue({});
});

test('removes merged branches without uncommitted changes', async () => {
Expand Down Expand Up @@ -1188,6 +1303,25 @@ describe('cleanupMergedWorktrees', () => {
})
);
});

test('uses worktree.baseBranch from repo config when comparing merge state', async () => {
mockLoadRepoConfig.mockResolvedValueOnce({ worktree: { baseBranch: 'master' } });
mockListByCodebase.mockResolvedValueOnce([
{
id: 'env-cfg',
branch_name: 'feature-branch',
working_path: '/workspace/repo/worktrees/feature-branch',
status: 'active',
},
]);
mockIsBranchMerged.mockResolvedValueOnce(false);

await cleanupMergedWorktrees('codebase-1', '/workspace/repo');

expect(mockLoadRepoConfig).toHaveBeenCalledWith('/workspace/repo');
expect(mockGetDefaultBranch).not.toHaveBeenCalled();
expect(mockIsBranchMerged).toHaveBeenCalledWith('/workspace/repo', 'feature-branch', 'master');
});
});

describe('onConversationClosed', () => {
Expand Down
22 changes: 19 additions & 3 deletions packages/core/src/services/cleanup-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,22 @@ import type { RepoPath, BranchName } from '@archon/git';
import { createLogger } from '@archon/paths';
import type { IsolationEnvironmentRow } from '@archon/isolation';
import { ConversationNotFoundError } from '../types';
import { loadRepoConfig } from '../config/config-loader';

/**
* Resolve the base branch for a repo. If `.archon/config.yaml` sets
* `worktree.baseBranch`, use that and skip git auto-detection (which fails
* loudly when origin/HEAD is unset and the default branch is not main).
* Falls back to `getDefaultBranch` for repos without explicit config.
*/
async function resolveBaseBranch(repoPath: RepoPath): Promise<BranchName> {
const repoConfig = await loadRepoConfig(repoPath);
const configured = repoConfig?.worktree?.baseBranch?.trim();
if (configured) {
return toBranchName(configured);
}
return await getDefaultBranch(repoPath);
}

/** Lazy-initialized logger (deferred so test mocks can intercept createLogger) */
let cachedLog: ReturnType<typeof createLogger> | undefined;
Expand Down Expand Up @@ -308,7 +324,7 @@ export async function runScheduledCleanup(): Promise<CleanupReport> {

// Check if branch is merged
const mainRepoPath = toRepoPath(env.codebase_default_cwd);
const mainBranch = await getDefaultBranch(mainRepoPath);
const mainBranch = await resolveBaseBranch(mainRepoPath);
const merged = await isBranchMerged(
mainRepoPath,
toBranchName(env.branch_name),
Expand Down Expand Up @@ -462,7 +478,7 @@ export async function getWorktreeStatusBreakdown(
activeEnvs: [],
};

const mainBranch = await getDefaultBranch(repoPath);
const mainBranch = await resolveBaseBranch(repoPath);

for (const env of environments) {
// Skip Telegram (never shown as stale)
Expand Down Expand Up @@ -590,7 +606,7 @@ export async function cleanupMergedWorktrees(
const result: CleanupOperationResult = { removed: [], skipped: [] };
const environments = await isolationEnvDb.listByCodebase(codebaseId);
const repoPath = toRepoPath(mainRepoPath);
const mainBranch = await getDefaultBranch(repoPath);
const mainBranch = await resolveBaseBranch(repoPath);
const includeClosed = options.includeClosed ?? false;
const prStateCache = new Map<string, PrState>();

Expand Down