[#395] github-issue-395-add-pull-from (#397)

* takt: github-issue-395-add-pull-from

* ci: trigger CI checks

* fix: taskDiffActions のコンフリクトマーカーを解消

origin/main でリネームされた「Merge from root」ラベル(PR #394)と、
このPR (#395) で追加した「Pull from remote」行を統合する。

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: trigger CI checks

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: masanobu-naruse <m_naruse@codmon.co.jp>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
nrs 2026-02-28 14:13:06 +09:00 committed by GitHub
parent 2d0dc127d0
commit 9ba05d8598
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 509 additions and 23 deletions

View File

@ -0,0 +1,307 @@
import { describe, expect, it, vi, beforeEach } from 'vitest';
vi.mock('node:fs', () => ({
existsSync: vi.fn(),
}));
vi.mock('node:child_process', () => ({
execFileSync: vi.fn(),
}));
vi.mock('../shared/ui/index.js', () => ({
success: vi.fn(),
error: vi.fn(),
}));
vi.mock('../shared/utils/index.js', () => ({
createLogger: vi.fn(() => ({
info: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
})),
getErrorMessage: vi.fn((err) => String(err)),
}));
vi.mock('../infra/task/index.js', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
pushBranch: vi.fn(),
}));
import * as fs from 'node:fs';
import { execFileSync } from 'node:child_process';
import { error as logError, success } from '../shared/ui/index.js';
import { pushBranch } from '../infra/task/index.js';
import { pullFromRemote } from '../features/tasks/list/taskPullAction.js';
import type { TaskListItem } from '../infra/task/index.js';
const mockExistsSync = vi.mocked(fs.existsSync);
const mockExecFileSync = vi.mocked(execFileSync);
const mockLogError = vi.mocked(logError);
const mockSuccess = vi.mocked(success);
const mockPushBranch = vi.mocked(pushBranch);
const PROJECT_DIR = '/project';
const ORIGIN_URL = 'git@github.com:user/repo.git';
function makeTask(overrides: Partial<TaskListItem> = {}): TaskListItem {
return {
kind: 'completed',
name: 'test-task',
branch: 'task/test-task',
createdAt: '2026-01-01T00:00:00Z',
filePath: '/project/.takt/tasks.yaml',
content: 'Implement feature X',
worktreePath: '/project-worktrees/test-task',
...overrides,
};
}
describe('pullFromRemote', () => {
beforeEach(() => {
vi.clearAllMocks();
mockExistsSync.mockReturnValue(true);
mockExecFileSync.mockReturnValue('' as never);
});
it('should throw when called with a non-task BranchActionTarget', () => {
const branchTarget = {
info: { branch: 'some-branch', commit: 'abc123' },
originalInstruction: 'Do something',
};
expect(
() => pullFromRemote(PROJECT_DIR, branchTarget as never),
).toThrow('Pull requires a task target.');
});
it('should return false and log error when worktreePath is missing', () => {
const task = makeTask({ worktreePath: undefined });
const result = pullFromRemote(PROJECT_DIR, task);
expect(result).toBe(false);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Worktree directory does not exist'),
);
expect(mockExecFileSync).not.toHaveBeenCalled();
});
it('should return false and log error when worktreePath does not exist on disk', () => {
const task = makeTask();
mockExistsSync.mockReturnValue(false);
const result = pullFromRemote(PROJECT_DIR, task);
expect(result).toBe(false);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Worktree directory does not exist'),
);
});
it('should get origin URL from projectDir', () => {
const task = makeTask();
mockExecFileSync.mockImplementation((cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'config') return `${ORIGIN_URL}\n` as never;
return '' as never;
});
pullFromRemote(PROJECT_DIR, task);
expect(mockExecFileSync).toHaveBeenCalledWith(
'git', ['config', '--get', 'remote.origin.url'],
expect.objectContaining({ cwd: PROJECT_DIR }),
);
});
it('should add temporary origin, pull, and remove origin', () => {
const task = makeTask();
const calls: string[][] = [];
mockExecFileSync.mockImplementation((cmd, args) => {
const argsArr = args as string[];
calls.push(argsArr);
if (argsArr[0] === 'config') return `${ORIGIN_URL}\n` as never;
return '' as never;
});
const result = pullFromRemote(PROJECT_DIR, task);
expect(result).toBe(true);
expect(mockSuccess).toHaveBeenCalledWith('Pulled & pushed.');
// Verify git remote add was called on worktree
expect(mockExecFileSync).toHaveBeenCalledWith(
'git', ['remote', 'add', 'origin', ORIGIN_URL],
expect.objectContaining({ cwd: task.worktreePath }),
);
// Verify git pull --ff-only was called on worktree
expect(mockExecFileSync).toHaveBeenCalledWith(
'git', ['pull', '--ff-only', 'origin', 'task/test-task'],
expect.objectContaining({ cwd: task.worktreePath }),
);
// Verify git remote remove was called on worktree
expect(mockExecFileSync).toHaveBeenCalledWith(
'git', ['remote', 'remove', 'origin'],
expect.objectContaining({ cwd: task.worktreePath }),
);
});
it('should push to projectDir then to origin after successful pull', () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'config') return `${ORIGIN_URL}\n` as never;
return '' as never;
});
pullFromRemote(PROJECT_DIR, task);
// worktree → project push
expect(mockExecFileSync).toHaveBeenCalledWith(
'git', ['push', PROJECT_DIR, 'HEAD'],
expect.objectContaining({ cwd: task.worktreePath }),
);
// project → origin push
expect(mockPushBranch).toHaveBeenCalledWith(PROJECT_DIR, 'task/test-task');
});
it('should return false and suggest sync when pull fails (not fast-forwardable)', () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'config') return `${ORIGIN_URL}\n` as never;
if (argsArr[0] === 'pull') throw new Error('fatal: Not possible to fast-forward');
return '' as never;
});
const result = pullFromRemote(PROJECT_DIR, task);
expect(result).toBe(false);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Pull failed'),
);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Sync with root'),
);
// Should NOT push when pull fails
expect(mockPushBranch).not.toHaveBeenCalled();
});
it('should remove temporary remote even when pull fails', () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'config') return `${ORIGIN_URL}\n` as never;
if (argsArr[0] === 'pull') throw new Error('fatal: Not possible to fast-forward');
return '' as never;
});
pullFromRemote(PROJECT_DIR, task);
// Verify remote remove was still called (cleanup in finally)
expect(mockExecFileSync).toHaveBeenCalledWith(
'git', ['remote', 'remove', 'origin'],
expect.objectContaining({ cwd: task.worktreePath }),
);
});
it('should not throw when git remote remove itself fails', () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'config') return `${ORIGIN_URL}\n` as never;
if (argsArr[0] === 'pull') throw new Error('pull failed');
if (argsArr[0] === 'remote' && argsArr[1] === 'remove') throw new Error('remove failed');
return '' as never;
});
const result = pullFromRemote(PROJECT_DIR, task);
expect(result).toBe(false);
});
it('should return false when getOriginUrl fails (root repo has no origin)', () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'config') throw new Error('fatal: No such remote \'origin\'');
return '' as never;
});
const result = pullFromRemote(PROJECT_DIR, task);
expect(result).toBe(false);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Failed to get origin URL'),
);
// Should not attempt remote add or pull
expect(mockExecFileSync).not.toHaveBeenCalledWith(
'git', expect.arrayContaining(['remote', 'add']),
expect.anything(),
);
});
it('should return false when git remote add fails', () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'config') return `${ORIGIN_URL}\n` as never;
if (argsArr[0] === 'remote' && argsArr[1] === 'add') throw new Error('fatal: remote origin already exists');
return '' as never;
});
const result = pullFromRemote(PROJECT_DIR, task);
expect(result).toBe(false);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Failed to add temporary remote'),
);
// Should still attempt remote remove (finally block)
expect(mockExecFileSync).toHaveBeenCalledWith(
'git', ['remote', 'remove', 'origin'],
expect.objectContaining({ cwd: task.worktreePath }),
);
// Should not push
expect(mockPushBranch).not.toHaveBeenCalled();
});
it('should return false when git push to projectDir fails after pull', () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'config') return `${ORIGIN_URL}\n` as never;
if (argsArr[0] === 'push') throw new Error('push failed');
return '' as never;
});
const result = pullFromRemote(PROJECT_DIR, task);
expect(result).toBe(false);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Push failed after pull'),
);
expect(mockSuccess).not.toHaveBeenCalled();
});
it('should return false when pushBranch fails after pull', () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'config') return `${ORIGIN_URL}\n` as never;
return '' as never;
});
mockPushBranch.mockImplementation(() => {
throw new Error('push to origin failed');
});
const result = pullFromRemote(PROJECT_DIR, task);
expect(result).toBe(false);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Push failed after pull'),
);
expect(mockSuccess).not.toHaveBeenCalled();
});
});

View File

@ -219,6 +219,42 @@ describe('syncBranchWithRoot', () => {
expect(result).toBe(false); expect(result).toBe(false);
}); });
it('returns false when push fails after successful merge', async () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'push') throw new Error('push failed');
return '' as never;
});
const result = await syncBranchWithRoot(PROJECT_DIR, task);
expect(result).toBe(false);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Push failed after sync'),
);
expect(mockSuccess).not.toHaveBeenCalledWith('Synced & pushed.');
});
it('returns false when push fails after AI conflict resolution', async () => {
const task = makeTask();
mockExecFileSync.mockImplementation((_cmd, args) => {
const argsArr = args as string[];
if (argsArr[0] === 'merge' && !argsArr.includes('--abort')) throw new Error('CONFLICT');
if (argsArr[0] === 'push') throw new Error('push failed');
return '' as never;
});
mockAgentCall.mockResolvedValue(makeAgentResponse({ status: 'done' }));
const result = await syncBranchWithRoot(PROJECT_DIR, task);
expect(result).toBe(false);
expect(mockLogError).toHaveBeenCalledWith(
expect.stringContaining('Push failed after sync'),
);
expect(mockSuccess).not.toHaveBeenCalledWith('Conflicts resolved & pushed.');
});
it('fetches from projectDir using local path ref', async () => { it('fetches from projectDir using local path ref', async () => {
const task = makeTask(); const task = makeTask();
mockExecFileSync.mockReturnValue('' as never); mockExecFileSync.mockReturnValue('' as never);

View File

@ -13,6 +13,7 @@ import {
mergeBranch, mergeBranch,
instructBranch, instructBranch,
syncBranchWithRoot, syncBranchWithRoot,
pullFromRemote,
} from './taskActions.js'; } from './taskActions.js';
import { deletePendingTask, deleteFailedTask, deleteCompletedTask, deleteAllTasks } from './taskDeleteActions.js'; import { deletePendingTask, deleteFailedTask, deleteCompletedTask, deleteAllTasks } from './taskDeleteActions.js';
import { retryFailedTask } from './taskRetryActions.js'; import { retryFailedTask } from './taskRetryActions.js';
@ -171,6 +172,9 @@ export async function listTasks(
case 'sync': case 'sync':
await syncBranchWithRoot(cwd, task); await syncBranchWithRoot(cwd, task);
break; break;
case 'pull':
pullFromRemote(cwd, task);
break;
case 'try': case 'try':
tryMergeBranch(cwd, task); tryMergeBranch(cwd, task);
break; break;

View File

@ -1,6 +1,13 @@
import * as fs from 'node:fs';
import { execFileSync } from 'node:child_process';
import { error as logError } from '../../../shared/ui/index.js';
import { createLogger } from '../../../shared/utils/index.js';
import { pushBranch } from '../../../infra/task/index.js';
import type { BranchListItem, TaskListItem } from '../../../infra/task/index.js'; import type { BranchListItem, TaskListItem } from '../../../infra/task/index.js';
export type ListAction = 'diff' | 'instruct' | 'sync' | 'try' | 'merge' | 'delete'; const log = createLogger('list-tasks');
export type ListAction = 'diff' | 'instruct' | 'sync' | 'pull' | 'try' | 'merge' | 'delete';
export type BranchActionTarget = TaskListItem | Pick<BranchListItem, 'info' | 'originalInstruction'>; export type BranchActionTarget = TaskListItem | Pick<BranchListItem, 'info' | 'originalInstruction'>;
@ -27,3 +34,36 @@ export function resolveTargetInstruction(target: BranchActionTarget): string {
} }
return target.originalInstruction; return target.originalInstruction;
} }
/**
* Validates that the target is a task target with a valid worktree path.
* Returns `false` with an error log if validation fails.
* Throws if the target is not a task target (programming error).
*/
export function validateWorktreeTarget(
target: BranchActionTarget,
actionName: string,
): target is TaskListItem & { worktreePath: string } {
if (!('kind' in target)) {
throw new Error(`${actionName} requires a task target.`);
}
if (!target.worktreePath || !fs.existsSync(target.worktreePath)) {
logError(`Worktree directory does not exist for task: ${target.name}`);
return false;
}
return true;
}
/** Push worktree → project dir, then project dir → origin */
export function pushWorktreeToOrigin(worktreePath: string, projectDir: string, branch: string): void {
execFileSync('git', ['push', projectDir, 'HEAD'], {
cwd: worktreePath,
encoding: 'utf-8',
stdio: 'pipe',
});
log.info('Pushed to main repo', { worktreePath, projectDir });
pushBranch(projectDir, branch);
log.info('Pushed to origin', { projectDir, branch });
}

View File

@ -19,3 +19,5 @@ export {
export { instructBranch } from './taskInstructionActions.js'; export { instructBranch } from './taskInstructionActions.js';
export { syncBranchWithRoot } from './taskSyncAction.js'; export { syncBranchWithRoot } from './taskSyncAction.js';
export { pullFromRemote } from './taskPullAction.js';

View File

@ -67,6 +67,7 @@ export async function showDiffAndPromptActionForTask(
{ label: 'View diff', value: 'diff', description: 'Show full diff in pager' }, { label: 'View diff', value: 'diff', description: 'Show full diff in pager' },
{ label: 'Instruct', value: 'instruct', description: 'Craft additional instructions and requeue this task' }, { label: 'Instruct', value: 'instruct', description: 'Craft additional instructions and requeue this task' },
{ label: 'Merge from root', value: 'sync', description: 'Merge root HEAD into worktree branch; auto-resolve conflicts with AI' }, { label: 'Merge from root', value: 'sync', description: 'Merge root HEAD into worktree branch; auto-resolve conflicts with AI' },
{ label: 'Pull from remote', value: 'pull', description: 'Pull latest changes from remote origin (fast-forward only)' },
{ label: 'Try merge', value: 'try', description: 'Squash merge (stage changes without commit)' }, { label: 'Try merge', value: 'try', description: 'Squash merge (stage changes without commit)' },
{ label: 'Merge & cleanup', value: 'merge', description: 'Merge and delete branch' }, { label: 'Merge & cleanup', value: 'merge', description: 'Merge and delete branch' },
{ label: 'Delete', value: 'delete', description: 'Discard changes, delete branch' }, { label: 'Delete', value: 'delete', description: 'Discard changes, delete branch' },

View File

@ -0,0 +1,95 @@
import { execFileSync } from 'node:child_process';
import { success, error as logError } from '../../../shared/ui/index.js';
import { createLogger, getErrorMessage } from '../../../shared/utils/index.js';
import {
type BranchActionTarget,
resolveTargetBranch,
validateWorktreeTarget,
pushWorktreeToOrigin,
} from './taskActionTarget.js';
const log = createLogger('list-tasks');
const TEMP_REMOTE_NAME = 'origin';
function getOriginUrl(projectDir: string): string {
return execFileSync(
'git', ['config', '--get', 'remote.origin.url'],
{ cwd: projectDir, encoding: 'utf-8', stdio: 'pipe' },
).trim();
}
export function pullFromRemote(
projectDir: string,
target: BranchActionTarget,
): boolean {
if (!validateWorktreeTarget(target, 'Pull')) {
return false;
}
const worktreePath = target.worktreePath;
const branch = resolveTargetBranch(target);
let originUrl: string;
try {
originUrl = getOriginUrl(projectDir);
} catch (err) {
logError(`Failed to get origin URL: ${getErrorMessage(err)}`);
log.error('getOriginUrl failed', { projectDir, error: getErrorMessage(err) });
return false;
}
log.info('Retrieved origin URL from root repo', { projectDir, originUrl });
try {
execFileSync('git', ['remote', 'add', TEMP_REMOTE_NAME, originUrl], {
cwd: worktreePath,
encoding: 'utf-8',
stdio: 'pipe',
});
log.info('Added temporary origin remote', { worktreePath, originUrl });
try {
execFileSync('git', ['pull', '--ff-only', TEMP_REMOTE_NAME, branch], {
cwd: worktreePath,
encoding: 'utf-8',
stdio: 'pipe',
});
log.info('Pull succeeded', { worktreePath, branch });
} catch (err) {
const msg = getErrorMessage(err);
logError(`Pull failed (not fast-forwardable?): ${msg}`);
logError('If the branch has diverged, use "Sync with root" instead.');
log.error('git pull --ff-only failed', { worktreePath, branch, error: msg });
return false;
}
} catch (err) {
logError(`Failed to add temporary remote: ${getErrorMessage(err)}`);
log.error('git remote add failed', { worktreePath, originUrl, error: getErrorMessage(err) });
return false;
} finally {
removeTemporaryRemote(worktreePath);
}
try {
pushWorktreeToOrigin(worktreePath, projectDir, branch);
} catch (err) {
logError(`Push failed after pull: ${getErrorMessage(err)}`);
log.error('pushWorktreeToOrigin failed', { worktreePath, projectDir, branch, error: getErrorMessage(err) });
return false;
}
success('Pulled & pushed.');
return true;
}
function removeTemporaryRemote(worktreePath: string): void {
try {
execFileSync('git', ['remote', 'remove', TEMP_REMOTE_NAME], {
cwd: worktreePath,
encoding: 'utf-8',
stdio: 'pipe',
});
log.info('Removed temporary origin remote', { worktreePath });
} catch (err) {
logError(`Failed to remove temporary remote: ${getErrorMessage(err)}`);
log.error('git remote remove failed', { worktreePath, error: getErrorMessage(err) });
}
}

View File

@ -1,13 +1,17 @@
import * as fs from 'node:fs';
import { execFileSync } from 'node:child_process'; import { execFileSync } from 'node:child_process';
import { success, error as logError, StreamDisplay } from '../../../shared/ui/index.js'; import { success, error as logError, StreamDisplay } from '../../../shared/ui/index.js';
import { createLogger, getErrorMessage } from '../../../shared/utils/index.js'; import { createLogger, getErrorMessage } from '../../../shared/utils/index.js';
import { getProvider, type ProviderType } from '../../../infra/providers/index.js'; import { getProvider, type ProviderType } from '../../../infra/providers/index.js';
import { resolveConfigValues } from '../../../infra/config/index.js'; import { resolveConfigValues } from '../../../infra/config/index.js';
import { pushBranch } from '../../../infra/task/index.js';
import { loadTemplate } from '../../../shared/prompts/index.js'; import { loadTemplate } from '../../../shared/prompts/index.js';
import { getLanguage } from '../../../infra/config/index.js'; import { getLanguage } from '../../../infra/config/index.js';
import { type BranchActionTarget, resolveTargetBranch, resolveTargetInstruction } from './taskActionTarget.js'; import {
type BranchActionTarget,
resolveTargetBranch,
resolveTargetInstruction,
validateWorktreeTarget,
pushWorktreeToOrigin,
} from './taskActionTarget.js';
const log = createLogger('list-tasks'); const log = createLogger('list-tasks');
@ -17,12 +21,7 @@ export async function syncBranchWithRoot(
projectDir: string, projectDir: string,
target: BranchActionTarget, target: BranchActionTarget,
): Promise<boolean> { ): Promise<boolean> {
if (!('kind' in target)) { if (!validateWorktreeTarget(target, 'Sync')) {
throw new Error('Sync requires a task target.');
}
if (!target.worktreePath || !fs.existsSync(target.worktreePath)) {
logError(`Worktree directory does not exist for task: ${target.name}`);
return false; return false;
} }
const worktreePath = target.worktreePath; const worktreePath = target.worktreePath;
@ -58,7 +57,9 @@ export async function syncBranchWithRoot(
} }
if (!mergeConflict) { if (!mergeConflict) {
pushSynced(worktreePath, projectDir, target); if (!pushSynced(worktreePath, projectDir, target)) {
return false;
}
success('Synced & pushed.'); success('Synced & pushed.');
log.info('Merge succeeded without conflicts', { worktreePath }); log.info('Merge succeeded without conflicts', { worktreePath });
return true; return true;
@ -86,7 +87,9 @@ export async function syncBranchWithRoot(
}); });
if (response.status === 'done') { if (response.status === 'done') {
pushSynced(worktreePath, projectDir, target); if (!pushSynced(worktreePath, projectDir, target)) {
return false;
}
success('Conflicts resolved & pushed.'); success('Conflicts resolved & pushed.');
log.info('AI conflict resolution succeeded', { worktreePath }); log.info('AI conflict resolution succeeded', { worktreePath });
return true; return true;
@ -102,18 +105,16 @@ async function autoApproveBash(request: { toolName: string; input: Record<string
return { behavior: 'allow' as const, updatedInput: request.input }; return { behavior: 'allow' as const, updatedInput: request.input };
} }
/** Push worktree → project dir, then project dir → origin */ function pushSynced(worktreePath: string, projectDir: string, target: BranchActionTarget): boolean {
function pushSynced(worktreePath: string, projectDir: string, target: BranchActionTarget): void {
execFileSync('git', ['push', projectDir, 'HEAD'], {
cwd: worktreePath,
encoding: 'utf-8',
stdio: 'pipe',
});
log.info('Pushed to main repo', { worktreePath, projectDir });
const branch = resolveTargetBranch(target); const branch = resolveTargetBranch(target);
pushBranch(projectDir, branch); try {
log.info('Pushed to origin', { projectDir, branch }); pushWorktreeToOrigin(worktreePath, projectDir, branch);
} catch (err) {
logError(`Push failed after sync: ${getErrorMessage(err)}`);
log.error('pushWorktreeToOrigin failed', { worktreePath, projectDir, branch, error: getErrorMessage(err) });
return false;
}
return true;
} }
function abortMerge(worktreePath: string): void { function abortMerge(worktreePath: string): void {