/review-tasks
This commit is contained in:
nrslib
parent
e8a8044c9f
commit
80626411cf
169
src/__tests__/reviewTasks.test.ts
Normal file
169
src/__tests__/reviewTasks.test.ts
Normal file
@ -0,0 +1,169 @@
|
||||
/**
|
||||
* Tests for review-tasks command
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
parseTaktWorktrees,
|
||||
extractTaskSlug,
|
||||
buildReviewItems,
|
||||
type WorktreeInfo,
|
||||
} from '../task/worktree.js';
|
||||
|
||||
describe('parseTaktWorktrees', () => {
|
||||
it('should parse takt/ branches from porcelain output', () => {
|
||||
const output = [
|
||||
'worktree /home/user/project',
|
||||
'HEAD abc1234567890',
|
||||
'branch refs/heads/main',
|
||||
'',
|
||||
'worktree /home/user/project/.takt/worktrees/20260128-fix-auth',
|
||||
'HEAD def4567890abc',
|
||||
'branch refs/heads/takt/20260128-fix-auth',
|
||||
'',
|
||||
'worktree /home/user/project/.takt/worktrees/20260128-add-search',
|
||||
'HEAD 789abcdef0123',
|
||||
'branch refs/heads/takt/20260128-add-search',
|
||||
].join('\n');
|
||||
|
||||
const result = parseTaktWorktrees(output);
|
||||
expect(result).toHaveLength(2);
|
||||
|
||||
expect(result[0]).toEqual({
|
||||
path: '/home/user/project/.takt/worktrees/20260128-fix-auth',
|
||||
branch: 'takt/20260128-fix-auth',
|
||||
commit: 'def4567890abc',
|
||||
});
|
||||
|
||||
expect(result[1]).toEqual({
|
||||
path: '/home/user/project/.takt/worktrees/20260128-add-search',
|
||||
branch: 'takt/20260128-add-search',
|
||||
commit: '789abcdef0123',
|
||||
});
|
||||
});
|
||||
|
||||
it('should exclude non-takt branches', () => {
|
||||
const output = [
|
||||
'worktree /home/user/project',
|
||||
'HEAD abc123',
|
||||
'branch refs/heads/main',
|
||||
'',
|
||||
'worktree /home/user/project/.takt/worktrees/20260128-fix-auth',
|
||||
'HEAD def456',
|
||||
'branch refs/heads/takt/20260128-fix-auth',
|
||||
'',
|
||||
'worktree /tmp/other-worktree',
|
||||
'HEAD 789abc',
|
||||
'branch refs/heads/feature/other',
|
||||
].join('\n');
|
||||
|
||||
const result = parseTaktWorktrees(output);
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0]!.branch).toBe('takt/20260128-fix-auth');
|
||||
});
|
||||
|
||||
it('should handle empty output', () => {
|
||||
const result = parseTaktWorktrees('');
|
||||
expect(result).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should handle bare worktree entry (no branch line)', () => {
|
||||
const output = [
|
||||
'worktree /home/user/project',
|
||||
'HEAD abc123',
|
||||
'bare',
|
||||
].join('\n');
|
||||
|
||||
const result = parseTaktWorktrees(output);
|
||||
expect(result).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should handle detached HEAD worktrees', () => {
|
||||
const output = [
|
||||
'worktree /home/user/project',
|
||||
'HEAD abc123',
|
||||
'branch refs/heads/main',
|
||||
'',
|
||||
'worktree /tmp/detached',
|
||||
'HEAD def456',
|
||||
'detached',
|
||||
].join('\n');
|
||||
|
||||
const result = parseTaktWorktrees(output);
|
||||
expect(result).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('extractTaskSlug', () => {
|
||||
it('should extract slug from timestamped branch name', () => {
|
||||
expect(extractTaskSlug('takt/20260128T032800-fix-auth')).toBe('fix-auth');
|
||||
});
|
||||
|
||||
it('should extract slug from date-only timestamp', () => {
|
||||
expect(extractTaskSlug('takt/20260128-add-search')).toBe('add-search');
|
||||
});
|
||||
|
||||
it('should extract slug with long timestamp format', () => {
|
||||
expect(extractTaskSlug('takt/20260128T032800-refactor-api')).toBe('refactor-api');
|
||||
});
|
||||
|
||||
it('should handle branch without timestamp', () => {
|
||||
expect(extractTaskSlug('takt/my-task')).toBe('my-task');
|
||||
});
|
||||
|
||||
it('should handle branch with only timestamp', () => {
|
||||
const result = extractTaskSlug('takt/20260128T032800');
|
||||
// Timestamp is stripped, nothing left, falls back to original name
|
||||
expect(result).toBe('20260128T032800');
|
||||
});
|
||||
|
||||
it('should handle slug with multiple dashes', () => {
|
||||
expect(extractTaskSlug('takt/20260128-fix-auth-bug-in-login')).toBe('fix-auth-bug-in-login');
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildReviewItems', () => {
|
||||
it('should build items with correct task slug', () => {
|
||||
const worktrees: WorktreeInfo[] = [
|
||||
{
|
||||
path: '/project/.takt/worktrees/20260128-fix-auth',
|
||||
branch: 'takt/20260128-fix-auth',
|
||||
commit: 'abc123',
|
||||
},
|
||||
];
|
||||
|
||||
// We can't test getFilesChanged without a real git repo,
|
||||
// so we test buildReviewItems' structure
|
||||
const items = buildReviewItems('/project', worktrees, 'main');
|
||||
expect(items).toHaveLength(1);
|
||||
expect(items[0]!.taskSlug).toBe('fix-auth');
|
||||
expect(items[0]!.info).toBe(worktrees[0]);
|
||||
// filesChanged will be 0 since we don't have a real git repo
|
||||
expect(items[0]!.filesChanged).toBe(0);
|
||||
});
|
||||
|
||||
it('should handle multiple worktrees', () => {
|
||||
const worktrees: WorktreeInfo[] = [
|
||||
{
|
||||
path: '/project/.takt/worktrees/20260128-fix-auth',
|
||||
branch: 'takt/20260128-fix-auth',
|
||||
commit: 'abc123',
|
||||
},
|
||||
{
|
||||
path: '/project/.takt/worktrees/20260128-add-search',
|
||||
branch: 'takt/20260128-add-search',
|
||||
commit: 'def456',
|
||||
},
|
||||
];
|
||||
|
||||
const items = buildReviewItems('/project', worktrees, 'main');
|
||||
expect(items).toHaveLength(2);
|
||||
expect(items[0]!.taskSlug).toBe('fix-auth');
|
||||
expect(items[1]!.taskSlug).toBe('add-search');
|
||||
});
|
||||
|
||||
it('should handle empty worktree list', () => {
|
||||
const items = buildReviewItems('/project', [], 'main');
|
||||
expect(items).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
@ -32,6 +32,7 @@ import {
|
||||
addTask,
|
||||
refreshBuiltin,
|
||||
watchTasks,
|
||||
reviewTasks,
|
||||
} from './commands/index.js';
|
||||
import { listWorkflows } from './config/workflowLoader.js';
|
||||
import { selectOptionWithDefault } from './prompt/index.js';
|
||||
@ -127,9 +128,14 @@ program
|
||||
await watchTasks(cwd);
|
||||
return;
|
||||
|
||||
case 'review-tasks':
|
||||
case 'review':
|
||||
await reviewTasks(cwd);
|
||||
return;
|
||||
|
||||
default:
|
||||
error(`Unknown command: /${command}`);
|
||||
info('Available: /run-tasks, /watch, /add-task, /switch, /clear, /refresh-builtin, /help, /config');
|
||||
info('Available: /run-tasks, /watch, /add-task, /review-tasks, /switch, /clear, /refresh-builtin, /help, /config');
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
@ -17,6 +17,7 @@ Usage:
|
||||
takt /run-tasks Run all pending tasks from .takt/tasks/
|
||||
takt /watch Watch for tasks and auto-execute (stays resident)
|
||||
takt /add-task Add a new task (interactive, YAML format)
|
||||
takt /review-tasks Review worktree task results (merge/delete)
|
||||
takt /switch Switch workflow interactively
|
||||
takt /clear Clear agent conversation sessions (reset to initial state)
|
||||
takt /refresh-builtin Overwrite builtin agents/workflows with latest version
|
||||
@ -29,6 +30,7 @@ Examples:
|
||||
takt /clear # Clear sessions, start fresh
|
||||
takt /watch # Watch & auto-execute tasks
|
||||
takt /refresh-builtin # Update builtin resources
|
||||
takt /review-tasks # Review & merge worktree results
|
||||
takt /switch
|
||||
takt /run-tasks
|
||||
|
||||
|
||||
@ -11,3 +11,4 @@ export { showHelp } from './help.js';
|
||||
export { withAgentSession } from './session.js';
|
||||
export { switchWorkflow } from './workflow.js';
|
||||
export { switchConfig, getCurrentPermissionMode, setPermissionMode, type PermissionMode } from './config.js';
|
||||
export { reviewTasks } from './reviewTasks.js';
|
||||
|
||||
194
src/commands/reviewTasks.ts
Normal file
194
src/commands/reviewTasks.ts
Normal file
@ -0,0 +1,194 @@
|
||||
/**
|
||||
* Review tasks command
|
||||
*
|
||||
* Interactive UI for reviewing worktree-based task results:
|
||||
* merge, skip, or delete actions.
|
||||
*/
|
||||
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import chalk from 'chalk';
|
||||
import {
|
||||
removeWorktree,
|
||||
detectDefaultBranch,
|
||||
listTaktWorktrees,
|
||||
buildReviewItems,
|
||||
type WorktreeReviewItem,
|
||||
} from '../task/worktree.js';
|
||||
import { selectOption, confirm } from '../prompt/index.js';
|
||||
import { info, success, error as logError, warn } from '../utils/ui.js';
|
||||
import { createLogger } from '../utils/debug.js';
|
||||
|
||||
const log = createLogger('review-tasks');
|
||||
|
||||
/** Actions available for a reviewed worktree */
|
||||
export type ReviewAction = 'merge' | 'skip' | 'delete';
|
||||
|
||||
/**
|
||||
* Show diff stat for a branch and prompt for an action.
|
||||
*/
|
||||
async function showDiffAndPromptAction(
|
||||
cwd: string,
|
||||
defaultBranch: string,
|
||||
item: WorktreeReviewItem,
|
||||
): Promise<ReviewAction> {
|
||||
console.log();
|
||||
console.log(chalk.bold.cyan(`=== ${item.info.branch} ===`));
|
||||
console.log();
|
||||
|
||||
// Show diff stat
|
||||
try {
|
||||
const stat = execFileSync(
|
||||
'git', ['diff', '--stat', `${defaultBranch}...${item.info.branch}`],
|
||||
{ cwd, encoding: 'utf-8', stdio: 'pipe' },
|
||||
);
|
||||
console.log(stat);
|
||||
} catch {
|
||||
warn('Could not generate diff stat');
|
||||
}
|
||||
|
||||
// Prompt action
|
||||
const action = await selectOption<ReviewAction>(
|
||||
`Action for ${item.info.branch}:`,
|
||||
[
|
||||
{ label: 'Merge', value: 'merge', description: 'Merge changes into current branch and clean up' },
|
||||
{ label: 'Skip', value: 'skip', description: 'Return to list without changes' },
|
||||
{ label: 'Delete', value: 'delete', description: 'Discard changes, remove worktree and branch' },
|
||||
],
|
||||
);
|
||||
|
||||
return action ?? 'skip';
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge a worktree branch into the current branch.
|
||||
* Removes the worktree first, then merges, then deletes the branch.
|
||||
*/
|
||||
export function mergeWorktreeBranch(projectDir: string, item: WorktreeReviewItem): boolean {
|
||||
const { branch } = item.info;
|
||||
|
||||
try {
|
||||
// 1. Remove worktree (must happen before merge to unlock branch)
|
||||
removeWorktree(projectDir, item.info.path);
|
||||
|
||||
// 2. Merge the branch
|
||||
execFileSync('git', ['merge', branch], {
|
||||
cwd: projectDir,
|
||||
encoding: 'utf-8',
|
||||
stdio: 'pipe',
|
||||
});
|
||||
|
||||
// 3. Delete the branch
|
||||
try {
|
||||
execFileSync('git', ['branch', '-d', branch], {
|
||||
cwd: projectDir,
|
||||
encoding: 'utf-8',
|
||||
stdio: 'pipe',
|
||||
});
|
||||
} catch {
|
||||
warn(`Could not delete branch ${branch}. You may delete it manually.`);
|
||||
}
|
||||
|
||||
success(`Merged ${branch}`);
|
||||
log.info('Worktree merged', { branch });
|
||||
return true;
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
logError(`Merge failed: ${msg}`);
|
||||
logError('You may need to resolve conflicts manually.');
|
||||
log.error('Merge failed', { branch, error: msg });
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete a worktree and its branch (discard changes).
|
||||
*/
|
||||
export function deleteWorktreeBranch(projectDir: string, item: WorktreeReviewItem): boolean {
|
||||
const { branch } = item.info;
|
||||
|
||||
try {
|
||||
// 1. Remove worktree
|
||||
removeWorktree(projectDir, item.info.path);
|
||||
|
||||
// 2. Force-delete the branch
|
||||
execFileSync('git', ['branch', '-D', branch], {
|
||||
cwd: projectDir,
|
||||
encoding: 'utf-8',
|
||||
stdio: 'pipe',
|
||||
});
|
||||
|
||||
success(`Deleted ${branch}`);
|
||||
log.info('Worktree deleted', { branch });
|
||||
return true;
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
logError(`Delete failed: ${msg}`);
|
||||
log.error('Delete failed', { branch, error: msg });
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Main entry point: review worktree tasks interactively.
|
||||
*/
|
||||
export async function reviewTasks(cwd: string): Promise<void> {
|
||||
log.info('Starting review-tasks');
|
||||
|
||||
const defaultBranch = detectDefaultBranch(cwd);
|
||||
let worktrees = listTaktWorktrees(cwd);
|
||||
|
||||
if (worktrees.length === 0) {
|
||||
info('No tasks to review.');
|
||||
return;
|
||||
}
|
||||
|
||||
// Interactive loop
|
||||
while (worktrees.length > 0) {
|
||||
const items = buildReviewItems(cwd, worktrees, defaultBranch);
|
||||
|
||||
// Build selection options
|
||||
const options = items.map((item, idx) => ({
|
||||
label: item.info.branch,
|
||||
value: String(idx),
|
||||
description: `${item.filesChanged} file${item.filesChanged !== 1 ? 's' : ''} changed`,
|
||||
}));
|
||||
|
||||
const selected = await selectOption<string>(
|
||||
'Review Tasks (Worktrees)',
|
||||
options,
|
||||
);
|
||||
|
||||
if (selected === null) {
|
||||
return;
|
||||
}
|
||||
|
||||
const selectedIdx = parseInt(selected, 10);
|
||||
const item = items[selectedIdx];
|
||||
if (!item) continue;
|
||||
|
||||
const action = await showDiffAndPromptAction(cwd, defaultBranch, item);
|
||||
|
||||
switch (action) {
|
||||
case 'merge':
|
||||
mergeWorktreeBranch(cwd, item);
|
||||
break;
|
||||
case 'delete': {
|
||||
const confirmed = await confirm(
|
||||
`Delete ${item.info.branch}? This will discard all changes.`,
|
||||
false,
|
||||
);
|
||||
if (confirmed) {
|
||||
deleteWorktreeBranch(cwd, item);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case 'skip':
|
||||
break;
|
||||
}
|
||||
|
||||
// Refresh worktree list after action
|
||||
worktrees = listTaktWorktrees(cwd);
|
||||
}
|
||||
|
||||
info('All tasks reviewed.');
|
||||
}
|
||||
@ -12,6 +12,19 @@ export { showTaskList } from './display.js';
|
||||
|
||||
export { TaskFileSchema, type TaskFileData } from './schema.js';
|
||||
export { parseTaskFile, parseTaskFiles, type ParsedTask } from './parser.js';
|
||||
export { createWorktree, removeWorktree, type WorktreeOptions, type WorktreeResult } from './worktree.js';
|
||||
export {
|
||||
createWorktree,
|
||||
removeWorktree,
|
||||
detectDefaultBranch,
|
||||
parseTaktWorktrees,
|
||||
listTaktWorktrees,
|
||||
getFilesChanged,
|
||||
extractTaskSlug,
|
||||
buildReviewItems,
|
||||
type WorktreeOptions,
|
||||
type WorktreeResult,
|
||||
type WorktreeInfo,
|
||||
type WorktreeReviewItem,
|
||||
} from './worktree.js';
|
||||
export { autoCommitWorktree, type AutoCommitResult } from './autoCommit.js';
|
||||
export { TaskWatcher, type TaskWatcherOptions } from './watcher.js';
|
||||
|
||||
@ -140,3 +140,144 @@ export function removeWorktree(projectDir: string, worktreePath: string): void {
|
||||
log.error('Failed to remove worktree', { path: worktreePath, error: String(err) });
|
||||
}
|
||||
}
|
||||
|
||||
// --- Review-related types and helpers ---
|
||||
|
||||
const TAKT_BRANCH_PREFIX = 'takt/';
|
||||
|
||||
/** Parsed worktree entry from git worktree list */
|
||||
export interface WorktreeInfo {
|
||||
path: string;
|
||||
branch: string;
|
||||
commit: string;
|
||||
}
|
||||
|
||||
/** Worktree with review metadata */
|
||||
export interface WorktreeReviewItem {
|
||||
info: WorktreeInfo;
|
||||
filesChanged: number;
|
||||
taskSlug: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect the default branch name (main or master).
|
||||
* Falls back to 'main'.
|
||||
*/
|
||||
export function detectDefaultBranch(cwd: string): string {
|
||||
try {
|
||||
const ref = execFileSync(
|
||||
'git', ['symbolic-ref', 'refs/remotes/origin/HEAD'],
|
||||
{ cwd, encoding: 'utf-8', stdio: 'pipe' },
|
||||
).trim();
|
||||
// ref is like "refs/remotes/origin/main"
|
||||
const parts = ref.split('/');
|
||||
return parts[parts.length - 1] || 'main';
|
||||
} catch {
|
||||
// Fallback: check if 'main' or 'master' exists
|
||||
try {
|
||||
execFileSync('git', ['rev-parse', '--verify', 'main'], {
|
||||
cwd, encoding: 'utf-8', stdio: 'pipe',
|
||||
});
|
||||
return 'main';
|
||||
} catch {
|
||||
try {
|
||||
execFileSync('git', ['rev-parse', '--verify', 'master'], {
|
||||
cwd, encoding: 'utf-8', stdio: 'pipe',
|
||||
});
|
||||
return 'master';
|
||||
} catch {
|
||||
return 'main';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse `git worktree list --porcelain` output into WorktreeInfo entries.
|
||||
* Only includes worktrees on branches with the takt/ prefix.
|
||||
*/
|
||||
export function parseTaktWorktrees(porcelainOutput: string): WorktreeInfo[] {
|
||||
const entries: WorktreeInfo[] = [];
|
||||
const blocks = porcelainOutput.trim().split('\n\n');
|
||||
|
||||
for (const block of blocks) {
|
||||
const lines = block.split('\n');
|
||||
let wtPath = '';
|
||||
let commit = '';
|
||||
let branch = '';
|
||||
|
||||
for (const line of lines) {
|
||||
if (line.startsWith('worktree ')) {
|
||||
wtPath = line.slice('worktree '.length);
|
||||
} else if (line.startsWith('HEAD ')) {
|
||||
commit = line.slice('HEAD '.length);
|
||||
} else if (line.startsWith('branch ')) {
|
||||
const ref = line.slice('branch '.length);
|
||||
branch = ref.replace('refs/heads/', '');
|
||||
}
|
||||
}
|
||||
|
||||
if (wtPath && branch.startsWith(TAKT_BRANCH_PREFIX)) {
|
||||
entries.push({ path: wtPath, branch, commit });
|
||||
}
|
||||
}
|
||||
|
||||
return entries;
|
||||
}
|
||||
|
||||
/**
|
||||
* List all takt-managed worktrees.
|
||||
*/
|
||||
export function listTaktWorktrees(projectDir: string): WorktreeInfo[] {
|
||||
try {
|
||||
const output = execFileSync(
|
||||
'git', ['worktree', 'list', '--porcelain'],
|
||||
{ cwd: projectDir, encoding: 'utf-8', stdio: 'pipe' },
|
||||
);
|
||||
return parseTaktWorktrees(output);
|
||||
} catch (err) {
|
||||
log.error('Failed to list worktrees', { error: String(err) });
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the number of files changed between the default branch and a given branch.
|
||||
*/
|
||||
export function getFilesChanged(cwd: string, defaultBranch: string, branch: string): number {
|
||||
try {
|
||||
const output = execFileSync(
|
||||
'git', ['diff', '--numstat', `${defaultBranch}...${branch}`],
|
||||
{ cwd, encoding: 'utf-8', stdio: 'pipe' },
|
||||
);
|
||||
return output.trim().split('\n').filter(l => l.length > 0).length;
|
||||
} catch {
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract a human-readable task slug from a takt branch name.
|
||||
* e.g. "takt/20260128T032800-fix-auth" → "fix-auth"
|
||||
*/
|
||||
export function extractTaskSlug(branch: string): string {
|
||||
const name = branch.replace(TAKT_BRANCH_PREFIX, '');
|
||||
// Remove timestamp prefix (format: YYYYMMDDTHHmmss- or similar)
|
||||
const withoutTimestamp = name.replace(/^\d{8,}T?\d{0,6}-?/, '');
|
||||
return withoutTimestamp || name;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build review items from worktree list, enriching with diff stats.
|
||||
*/
|
||||
export function buildReviewItems(
|
||||
projectDir: string,
|
||||
worktrees: WorktreeInfo[],
|
||||
defaultBranch: string,
|
||||
): WorktreeReviewItem[] {
|
||||
return worktrees.map(wt => ({
|
||||
info: wt,
|
||||
filesChanged: getFilesChanged(projectDir, defaultBranch, wt.branch),
|
||||
taskSlug: extractTaskSlug(wt.branch),
|
||||
}));
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user