takt: fix-original-instruction-diff (#181)

This commit is contained in:
nrs 2026-02-09 20:55:57 +09:00 committed by GitHub
parent 3df83eb7f8
commit a481346945
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 521 additions and 29 deletions

View File

@ -0,0 +1,141 @@
import { execFileSync } from 'node:child_process';
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { afterEach, describe, expect, it } from 'vitest';
import { getFilesChanged, getOriginalInstruction } from '../infra/task/branchList.js';
function runGit(cwd: string, args: string[]): string {
return execFileSync('git', args, { cwd, encoding: 'utf-8', stdio: 'pipe' }).trim();
}
function isUnsupportedInitBranchOptionError(error: unknown): boolean {
if (!(error instanceof Error)) {
return false;
}
return /unknown switch [`'-]?b/.test(error.message);
}
function writeAndCommit(repo: string, fileName: string, content: string, message: string): void {
writeFileSync(join(repo, fileName), content, 'utf-8');
runGit(repo, ['add', fileName]);
runGit(repo, ['commit', '-m', message]);
}
function setupRepoForIssue167(options?: { disableReflog?: boolean; firstBranchCommitMessage?: string }): { repoDir: string; branch: string } {
const repoDir = mkdtempSync(join(tmpdir(), 'takt-branchlist-'));
try {
runGit(repoDir, ['init', '-b', 'main']);
} catch (error) {
if (!isUnsupportedInitBranchOptionError(error)) {
throw error;
}
runGit(repoDir, ['init']);
}
if (options?.disableReflog) {
runGit(repoDir, ['config', 'core.logallrefupdates', 'false']);
}
runGit(repoDir, ['config', 'user.name', 'takt-test']);
runGit(repoDir, ['config', 'user.email', 'takt-test@example.com']);
writeAndCommit(repoDir, 'main.txt', 'main\n', 'main base');
runGit(repoDir, ['branch', '-M', 'main']);
runGit(repoDir, ['checkout', '-b', 'develop']);
writeAndCommit(repoDir, 'develop-a.txt', 'develop a\n', 'develop commit A');
writeAndCommit(repoDir, 'develop-takt.txt', 'develop takt\n', 'takt: old instruction on develop');
writeAndCommit(repoDir, 'develop-b.txt', 'develop b\n', 'develop commit B');
const taktBranch = 'takt/#167/fix-original-instruction';
runGit(repoDir, ['checkout', '-b', taktBranch]);
const firstBranchCommitMessage = options?.firstBranchCommitMessage ?? 'takt: github-issue-167-fix-original-instruction';
writeAndCommit(repoDir, 'task-1.txt', 'task1\n', firstBranchCommitMessage);
writeAndCommit(repoDir, 'task-2.txt', 'task2\n', 'follow-up implementation');
return { repoDir, branch: taktBranch };
}
describe('branchList regression for issue #167', () => {
const tempDirs: string[] = [];
afterEach(() => {
while (tempDirs.length > 0) {
const dir = tempDirs.pop();
if (dir) {
rmSync(dir, { recursive: true, force: true });
}
}
});
it('should resolve originalInstruction correctly even when HEAD is main', () => {
const fixture = setupRepoForIssue167();
tempDirs.push(fixture.repoDir);
runGit(fixture.repoDir, ['checkout', 'main']);
const instruction = getOriginalInstruction(fixture.repoDir, 'main', fixture.branch);
expect(instruction).toBe('github-issue-167-fix-original-instruction');
});
it('should keep filesChanged non-zero even when HEAD is target branch', () => {
const fixture = setupRepoForIssue167();
tempDirs.push(fixture.repoDir);
runGit(fixture.repoDir, ['checkout', fixture.branch]);
const changed = getFilesChanged(fixture.repoDir, 'main', fixture.branch);
expect(changed).toBe(2);
});
it('should ignore takt commits that exist only on base branch history', () => {
const fixture = setupRepoForIssue167();
tempDirs.push(fixture.repoDir);
runGit(fixture.repoDir, ['checkout', 'main']);
const instruction = getOriginalInstruction(fixture.repoDir, 'main', fixture.branch);
const changed = getFilesChanged(fixture.repoDir, 'main', fixture.branch);
expect(instruction).toBe('github-issue-167-fix-original-instruction');
expect(changed).toBe(2);
});
it('should keep original instruction and changed files after merging branch into develop', () => {
const fixture = setupRepoForIssue167();
tempDirs.push(fixture.repoDir);
runGit(fixture.repoDir, ['checkout', 'develop']);
runGit(fixture.repoDir, ['merge', '--no-ff', fixture.branch, '-m', 'merge takt branch']);
runGit(fixture.repoDir, ['checkout', 'main']);
const instruction = getOriginalInstruction(fixture.repoDir, 'main', fixture.branch);
const changed = getFilesChanged(fixture.repoDir, 'main', fixture.branch);
expect(instruction).toBe('github-issue-167-fix-original-instruction');
expect(changed).toBe(2);
});
it('should resolve correctly without branch reflog by inferring base from refs', () => {
const fixture = setupRepoForIssue167({ disableReflog: true });
tempDirs.push(fixture.repoDir);
runGit(fixture.repoDir, ['checkout', 'main']);
const instruction = getOriginalInstruction(fixture.repoDir, 'main', fixture.branch);
const changed = getFilesChanged(fixture.repoDir, 'main', fixture.branch);
expect(instruction).toBe('github-issue-167-fix-original-instruction');
expect(changed).toBe(2);
});
it('should use inferred branch base when first branch commit has no takt prefix and reflog is unavailable', () => {
const fixture = setupRepoForIssue167({
disableReflog: true,
firstBranchCommitMessage: 'Initial branch implementation',
});
tempDirs.push(fixture.repoDir);
runGit(fixture.repoDir, ['checkout', 'main']);
const instruction = getOriginalInstruction(fixture.repoDir, 'main', fixture.branch);
expect(instruction).toBe('Initial branch implementation');
});
});

View File

@ -0,0 +1,68 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
vi.mock('node:child_process', () => ({
execFileSync: vi.fn(),
}));
import { execFileSync } from 'node:child_process';
const mockExecFileSync = vi.mocked(execFileSync);
import { getFilesChanged } from '../infra/task/branchList.js';
beforeEach(() => {
vi.clearAllMocks();
});
describe('getFilesChanged', () => {
it('should count changed files from branch entry base commit via reflog', () => {
mockExecFileSync
.mockReturnValueOnce('f00dbabe\nfeedface\nabc123\n')
.mockReturnValueOnce('1\t0\tfile1.ts\n2\t1\tfile2.ts\n');
const result = getFilesChanged('/project', 'main', 'takt/20260128-fix-auth');
expect(result).toBe(2);
expect(mockExecFileSync).toHaveBeenNthCalledWith(
2,
'git',
['diff', '--numstat', 'abc123..takt/20260128-fix-auth'],
expect.objectContaining({ cwd: '/project', encoding: 'utf-8' }),
);
});
it('should infer base from refs when reflog is unavailable', () => {
mockExecFileSync
.mockImplementationOnce(() => {
throw new Error('reflog unavailable');
})
.mockReturnValueOnce('develop\n')
.mockReturnValueOnce('base999\n')
.mockReturnValueOnce('1\n')
.mockReturnValueOnce('takt: fix auth\n')
.mockReturnValueOnce('1\t0\tfile1.ts\n');
const result = getFilesChanged('/project', 'develop', 'takt/20260128-fix-auth');
expect(result).toBe(1);
expect(mockExecFileSync).toHaveBeenCalledWith(
'git',
['for-each-ref', '--format=%(refname:short)', 'refs/heads', 'refs/remotes'],
expect.objectContaining({ cwd: '/project', encoding: 'utf-8' }),
);
expect(mockExecFileSync).toHaveBeenCalledWith(
'git',
['merge-base', 'develop', 'takt/20260128-fix-auth'],
expect.objectContaining({ cwd: '/project', encoding: 'utf-8' }),
);
});
it('should return 0 when base commit resolution fails', () => {
mockExecFileSync.mockImplementation(() => {
throw new Error('base resolution failed');
});
const result = getFilesChanged('/project', 'main', 'takt/20260128-fix-auth');
expect(result).toBe(0);
});
});

View File

@ -19,29 +19,61 @@ beforeEach(() => {
});
describe('getOriginalInstruction', () => {
it('should extract instruction from takt-prefixed commit message', () => {
mockExecFileSync.mockReturnValue('takt: 認証機能を追加する\ntakt: fix-auth\n');
it('should extract instruction from branch entry commit via reflog', () => {
mockExecFileSync
.mockReturnValueOnce('last789\nfirst456\nbase123\n')
.mockReturnValueOnce('takt: 認証機能を追加する\n');
const result = getOriginalInstruction('/project', 'main', 'takt/20260128-fix-auth');
expect(result).toBe('認証機能を追加する');
expect(mockExecFileSync).toHaveBeenCalledWith(
'git',
['log', '--format=%s', '--reverse', 'main..takt/20260128-fix-auth'],
['reflog', 'show', '--format=%H', 'takt/20260128-fix-auth'],
expect.objectContaining({ cwd: '/project', encoding: 'utf-8' }),
);
expect(mockExecFileSync).toHaveBeenCalledWith(
'git',
['show', '-s', '--format=%s', 'first456'],
expect.objectContaining({ cwd: '/project', encoding: 'utf-8' }),
);
});
it('should return first commit message without takt prefix if not present', () => {
mockExecFileSync.mockReturnValue('Initial implementation\n');
it('should infer base from refs when reflog is unavailable', () => {
mockExecFileSync
.mockImplementationOnce(() => {
throw new Error('reflog unavailable');
})
.mockReturnValueOnce('develop\n')
.mockReturnValueOnce('base123\n')
.mockReturnValueOnce('2\n')
.mockReturnValueOnce('takt: Initial implementation\nfollow-up\n')
.mockReturnValueOnce('first456\ttakt: Initial implementation\n');
const result = getOriginalInstruction('/project', 'main', 'takt/20260128-fix-auth');
expect(result).toBe('Initial implementation');
expect(mockExecFileSync).toHaveBeenCalledWith(
'git',
['for-each-ref', '--format=%(refname:short)', 'refs/heads', 'refs/remotes'],
expect.objectContaining({ cwd: '/project', encoding: 'utf-8' }),
);
expect(mockExecFileSync).toHaveBeenCalledWith(
'git',
['merge-base', 'develop', 'takt/20260128-fix-auth'],
expect.objectContaining({ cwd: '/project', encoding: 'utf-8' }),
);
});
it('should return empty string when no commits on branch', () => {
mockExecFileSync.mockReturnValue('');
mockExecFileSync
.mockImplementationOnce(() => {
throw new Error('reflog unavailable');
})
.mockReturnValueOnce('abc123\n')
.mockReturnValueOnce('')
.mockReturnValueOnce('abc123\n')
.mockReturnValueOnce('');
const result = getOriginalInstruction('/project', 'main', 'takt/20260128-fix-auth');
@ -59,7 +91,9 @@ describe('getOriginalInstruction', () => {
});
it('should handle multi-line commit messages (use only first line)', () => {
mockExecFileSync.mockReturnValue('takt: Fix the login bug\ntakt: follow-up fix\n');
mockExecFileSync
.mockReturnValueOnce('f00dbabe\ndeadbeef\nbase123\n')
.mockReturnValueOnce('takt: Fix the login bug\n');
const result = getOriginalInstruction('/project', 'main', 'takt/20260128-fix-login');
@ -67,8 +101,9 @@ describe('getOriginalInstruction', () => {
});
it('should return empty string when takt prefix has no content', () => {
// "takt: \n" trimmed → "takt:", starts with "takt:" → slice + trim → ""
mockExecFileSync.mockReturnValue('takt: \n');
mockExecFileSync
.mockReturnValueOnce('cafebabe\nbase123\n')
.mockReturnValueOnce('takt:\n');
const result = getOriginalInstruction('/project', 'main', 'takt/20260128-task');
@ -76,22 +111,22 @@ describe('getOriginalInstruction', () => {
});
it('should return instruction text when takt prefix has content', () => {
mockExecFileSync.mockReturnValue('takt: add search feature\n');
mockExecFileSync
.mockReturnValueOnce('beadface\nbase123\n')
.mockReturnValueOnce('takt: add search feature\n');
const result = getOriginalInstruction('/project', 'main', 'takt/20260128-task');
expect(result).toBe('add search feature');
});
it('should use correct git range with custom default branch', () => {
mockExecFileSync.mockReturnValue('takt: Add search feature\n');
it('should return original subject when branch entry commit has no takt prefix', () => {
mockExecFileSync
.mockReturnValueOnce('last789\nfirst456\nbase123\n')
.mockReturnValueOnce('Initial implementation\n');
getOriginalInstruction('/project', 'master', 'takt/20260128-add-search');
const result = getOriginalInstruction('/project', 'main', 'takt/20260128-fix-auth');
expect(mockExecFileSync).toHaveBeenCalledWith(
'git',
['log', '--format=%s', '--reverse', 'master..takt/20260128-add-search'],
expect.objectContaining({ cwd: '/project' }),
);
expect(result).toBe('Initial implementation');
});
});

View File

@ -0,0 +1,223 @@
import { execFileSync } from 'node:child_process';
import { existsSync } from 'node:fs';
import { createLogger } from '../../shared/utils/index.js';
type BranchEntryPoint = {
baseCommit: string;
firstCommit: string;
};
type FirstTaktCommit = {
subject: string;
};
type BaseRefCandidate = {
baseRef: string;
baseCommit: string;
firstSubject: string;
distance: number;
};
const TAKT_COMMIT_PREFIX = 'takt:';
const log = createLogger('branchGitResolver');
function runGit(gitCwd: string, args: string[]): string {
return execFileSync('git', args, {
cwd: gitCwd,
encoding: 'utf-8',
stdio: 'pipe',
}).trim();
}
function parseDistinctHashes(output: string): string[] {
const hashes = output
.split('\n')
.map(line => line.trim())
.filter(line => line.length > 0);
const distinct: string[] = [];
for (const hash of hashes) {
if (distinct[distinct.length - 1] !== hash) {
distinct.push(hash);
}
}
return distinct;
}
export function resolveGitCwd(cwd: string, worktreePath?: string): string {
return worktreePath && existsSync(worktreePath) ? worktreePath : cwd;
}
export function resolveMergeBase(gitCwd: string, baseRef: string, branch: string): string {
return runGit(gitCwd, ['merge-base', baseRef, branch]);
}
function listCandidateRefs(gitCwd: string, branch: string): string[] {
const output = runGit(gitCwd, [
'for-each-ref',
'--format=%(refname:short)',
'refs/heads',
'refs/remotes',
]);
const refs = output
.split('\n')
.map(line => line.trim())
.filter(line => line.length > 0)
.filter(ref => ref !== branch)
.filter(ref => !ref.endsWith(`/${branch}`))
.filter(ref => !ref.endsWith('/HEAD'));
return Array.from(new Set(refs));
}
function getFirstParentDistance(gitCwd: string, baseCommit: string, branch: string): number {
const output = runGit(gitCwd, ['rev-list', '--count', '--first-parent', `${baseCommit}..${branch}`]);
return Number.parseInt(output, 10);
}
function getFirstParentFirstSubject(gitCwd: string, baseCommit: string, branch: string): string {
const output = runGit(gitCwd, ['log', '--format=%s', '--reverse', '--first-parent', `${baseCommit}..${branch}`]);
return output.split('\n')[0]?.trim() ?? '';
}
function resolveBaseCandidate(gitCwd: string, baseRef: string, branch: string): BaseRefCandidate | null {
try {
const baseCommit = resolveMergeBase(gitCwd, baseRef, branch);
if (!baseCommit) {
return null;
}
const distance = getFirstParentDistance(gitCwd, baseCommit, branch);
if (!Number.isFinite(distance) || distance <= 0) {
return null;
}
const firstSubject = getFirstParentFirstSubject(gitCwd, baseCommit, branch);
return { baseRef, baseCommit, firstSubject, distance };
} catch (error) {
log.debug('Failed to resolve base candidate', { error: String(error), gitCwd, baseRef, branch });
return null;
}
}
function chooseBestBaseCandidate(candidates: BaseRefCandidate[]): BaseRefCandidate | null {
if (candidates.length === 0) {
return null;
}
const sorted = [...candidates].sort((a, b) => {
const aTakt = a.firstSubject.startsWith(TAKT_COMMIT_PREFIX);
const bTakt = b.firstSubject.startsWith(TAKT_COMMIT_PREFIX);
if (aTakt !== bTakt) {
return aTakt ? -1 : 1;
}
if (a.distance !== b.distance) {
return a.distance - b.distance;
}
const aRemote = a.baseRef.includes('/');
const bRemote = b.baseRef.includes('/');
if (aRemote !== bRemote) {
return aRemote ? 1 : -1;
}
return a.baseRef.localeCompare(b.baseRef);
});
return sorted[0] ?? null;
}
function resolveBranchBaseCommitFromRefs(gitCwd: string, branch: string): string | null {
const refs = listCandidateRefs(gitCwd, branch);
const candidates: BaseRefCandidate[] = [];
for (const ref of refs) {
const candidate = resolveBaseCandidate(gitCwd, ref, branch);
if (candidate) {
candidates.push(candidate);
}
}
const best = chooseBestBaseCandidate(candidates);
return best?.baseCommit ?? null;
}
function resolveBranchEntryPointFromReflog(gitCwd: string, branch: string): BranchEntryPoint | null {
try {
const output = runGit(gitCwd, ['reflog', 'show', '--format=%H', branch]);
const hashes = parseDistinctHashes(output).reverse();
if (hashes.length < 2) {
return null;
}
return {
baseCommit: hashes[0]!,
firstCommit: hashes[1]!,
};
} catch (error) {
log.debug('Failed to resolve branch entry point from reflog', { error: String(error), gitCwd, branch });
return null;
}
}
function readCommitSubject(gitCwd: string, commit: string): string {
return runGit(gitCwd, ['show', '-s', '--format=%s', commit]);
}
function parseFirstCommitLine(output: string): FirstTaktCommit | null {
if (!output) {
return null;
}
const firstLine = output.split('\n')[0];
if (!firstLine) {
return null;
}
const tabIndex = firstLine.indexOf('\t');
if (tabIndex === -1) {
return null;
}
return {
subject: firstLine.slice(tabIndex + 1),
};
}
export function findFirstTaktCommit(
gitCwd: string,
defaultBranch: string,
branch: string,
): FirstTaktCommit | null {
const entryPoint = resolveBranchEntryPointFromReflog(gitCwd, branch);
if (entryPoint) {
const subject = readCommitSubject(gitCwd, entryPoint.firstCommit);
return {
subject,
};
}
const baseCommit = resolveBranchBaseCommitFromRefs(gitCwd, branch) ?? resolveMergeBase(gitCwd, defaultBranch, branch);
const output = runGit(gitCwd, [
'log',
'--format=%H\t%s',
'--reverse',
'--first-parent',
'--grep=^takt:',
`${baseCommit}..${branch}`,
]);
return parseFirstCommitLine(output);
}
export function resolveBranchBaseCommit(gitCwd: string, defaultBranch: string, branch: string): string {
const entryPoint = resolveBranchEntryPointFromReflog(gitCwd, branch);
if (entryPoint) {
return entryPoint.baseCommit;
}
return resolveBranchBaseCommitFromRefs(gitCwd, branch) ?? resolveMergeBase(gitCwd, defaultBranch, branch);
}

View File

@ -7,8 +7,12 @@
*/
import { execFileSync } from 'node:child_process';
import { existsSync } from 'node:fs';
import { createLogger } from '../../shared/utils/index.js';
import {
findFirstTaktCommit,
resolveBranchBaseCommit,
resolveGitCwd,
} from './branchGitResolver.js';
import type { BranchInfo, BranchListItem } from './types.js';
@ -31,19 +35,22 @@ export class BranchManager {
).trim();
const prefix = 'refs/remotes/origin/';
return ref.startsWith(prefix) ? ref.slice(prefix.length) : ref;
} catch {
} catch (error) {
log.debug('detectDefaultBranch symbolic-ref failed', { error: String(error), cwd });
try {
execFileSync('git', ['rev-parse', '--verify', 'main'], {
cwd, encoding: 'utf-8', stdio: 'pipe',
});
return 'main';
} catch {
} catch (mainError) {
log.debug('detectDefaultBranch main lookup failed', { error: String(mainError), cwd });
try {
execFileSync('git', ['rev-parse', '--verify', 'master'], {
cwd, encoding: 'utf-8', stdio: 'pipe',
});
return 'master';
} catch {
} catch (masterError) {
log.debug('detectDefaultBranch master lookup failed', { error: String(masterError), cwd });
return 'main';
}
}
@ -110,16 +117,19 @@ export class BranchManager {
return entries;
}
/** Get the number of files changed between the default branch and a given branch */
/** Get the number of files changed between a branch and its inferred base commit */
getFilesChanged(cwd: string, defaultBranch: string, branch: string, worktreePath?: string): number {
try {
// If worktreePath is provided, use it for git diff (for worktree-sessions branches)
const gitCwd = worktreePath && existsSync(worktreePath) ? worktreePath : cwd;
const gitCwd = resolveGitCwd(cwd, worktreePath);
const baseCommit = resolveBranchBaseCommit(gitCwd, defaultBranch, branch);
if (!baseCommit) {
throw new Error(`Failed to resolve base commit for branch: ${branch}`);
}
log.debug('getFilesChanged', { gitCwd, defaultBranch, branch, worktreePath });
log.debug('getFilesChanged', { gitCwd, baseCommit, branch, worktreePath });
const output = execFileSync(
'git', ['diff', '--numstat', `${defaultBranch}...${branch}`],
'git', ['diff', '--numstat', `${baseCommit}..${branch}`],
{ cwd: gitCwd, encoding: 'utf-8', stdio: 'pipe' },
);
@ -150,9 +160,23 @@ export class BranchManager {
branch: string,
): string {
try {
const firstTaktCommit = findFirstTaktCommit(cwd, defaultBranch, branch);
if (firstTaktCommit) {
const TAKT_COMMIT_PREFIX = 'takt:';
if (firstTaktCommit.subject.startsWith(TAKT_COMMIT_PREFIX)) {
return firstTaktCommit.subject.slice(TAKT_COMMIT_PREFIX.length).trim();
}
return firstTaktCommit.subject;
}
const baseCommit = resolveBranchBaseCommit(cwd, defaultBranch, branch);
if (!baseCommit) {
throw new Error(`Failed to resolve base commit for branch: ${branch}`);
}
const output = execFileSync(
'git',
['log', '--format=%s', '--reverse', `${defaultBranch}..${branch}`],
['log', '--format=%s', '--reverse', `${baseCommit}..${branch}`],
{ cwd, encoding: 'utf-8', stdio: 'pipe' },
).trim();
@ -165,7 +189,8 @@ export class BranchManager {
}
return firstLine;
} catch {
} catch (error) {
log.debug('getOriginalInstruction failed', { error: String(error), cwd, defaultBranch, branch });
return '';
}
}