Skip to content

Commit 7ae7b43

Browse files
recuu-pfegclaude
andcommitted
refactor: P2 dedup + dead code cleanup
New utils: - spawn-claude.ts: unified Claude CLI spawn (replaces 3 copies) - completion-parser.ts: COMPLETION_JSON extraction from stdout - constants.ts: TIMEOUTS, INTERVALS, LIMITS Improvements: - resolveRepoRoot in git-ops.ts (replaces 4 inline copies) - Removed MergeLock (instantiated but never used) - Removed ChatPoller (legacy, replaced by Manager) - Removed skills/index.ts (migrated to Playbook) All 71 tests pass. Build succeeds. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 993dab5 commit 7ae7b43

File tree

14 files changed

+235
-726
lines changed

14 files changed

+235
-726
lines changed

src/chat-poller.ts

Lines changed: 0 additions & 462 deletions
This file was deleted.

src/commands/review.ts

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
*/
44

55
import { createApiClient, type Task } from "../api-client.js";
6-
import { resolveModelForRole } from "../agent-engine.js";
76
import * as ui from "../ui.js";
87
import { parseTaskLabels } from "../utils/parse-labels.js";
8+
import { spawnClaudeOnce } from "../utils/spawn-claude.js";
9+
import { TIMEOUTS } from "../constants.js";
910

1011
export async function handleReview(apiUrl: string, apiKey: string, taskId?: string, skills?: string[]): Promise<void> {
1112
const api = createApiClient(apiUrl, apiKey);
12-
const { spawn } = await import("node:child_process");
1313
const { execSync: revExec } = await import("node:child_process");
1414

1515
ui.intro();
@@ -69,16 +69,8 @@ export async function handleReview(apiUrl: string, apiKey: string, taskId?: stri
6969
const prompt = `${reviewSystem}\n\nRun: git diff ${diffRef}\nRun: npm test 2>&1 | tail -20\n\nThen output verdict.\n\n${outputFormat}`;
7070

7171
s.start("Running Reviewer agent...");
72-
const result = await new Promise<string>((resolve) => {
73-
const env = { ...process.env };
74-
delete env.CLAUDECODE;
75-
const child = spawn("claude", ["--print", "--model", resolveModelForRole("reviewer"), "--max-turns", "5", prompt], {
76-
env, cwd, stdio: ["ignore", "pipe", "pipe"], timeout: 300_000,
77-
});
78-
let out = "";
79-
child.stdout?.on("data", (chunk: Buffer) => { out += chunk.toString(); });
80-
child.on("close", () => resolve(out));
81-
child.on("error", () => resolve(""));
72+
const result = await spawnClaudeOnce(prompt, {
73+
role: "reviewer", maxTurns: 5, timeout: TIMEOUTS.REVIEWER, cwd,
8274
});
8375
s.stop("Review complete");
8476

src/commands/run-loop.ts

Lines changed: 11 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import { execSync } from "node:child_process";
1919
import { handlePropose } from "./propose.js";
2020
import type { ShutdownState } from "./shutdown.js";
2121
import { parseTaskLabels } from "../utils/parse-labels.js";
22+
import { extractCompletionJson } from "../utils/completion-parser.js";
23+
import { TIMEOUTS, INTERVALS } from "../constants.js";
2224

2325
// ---------------------------------------------------------------------------
2426
// Helpers
@@ -62,7 +64,7 @@ Output ONLY a JSON array, no markdown:
6264
return new Promise((resolve) => {
6365
const child = spawn("claude", ["--print", "--model", resolveModel("claude-haiku"), "--max-turns", "1", prompt], {
6466
stdio: ["ignore", "pipe", "pipe"],
65-
timeout: 30_000,
67+
timeout: TIMEOUTS.SPLIT_TASK,
6668
});
6769
let out = "";
6870
child.stdout?.on("data", (chunk: Buffer) => { out += chunk.toString(); });
@@ -96,16 +98,14 @@ export async function runLoop(cliArgs: CliArgs, runner: AgentRunner, shutdownSta
9698
const { api, wsServer, tobanHome, repos, gitToken, gitUserInfo, credentialHelperPath } = ctx;
9799
let { sprintData } = ctx;
98100

99-
const POLL_INTERVAL_MS = 30_000;
101+
const POLL_INTERVAL_MS = INTERVALS.POLL;
100102

101103
// Parallel agent slots
102104
const { SlotScheduler } = await import("../slot-scheduler.js");
103-
const { MergeLock } = await import("../merge-lock.js");
104105
const scheduler = new SlotScheduler([
105106
{ role: "builder", maxConcurrency: 2 },
106107
{ role: "cloud-engineer", maxConcurrency: 1 },
107108
]);
108-
const mergeLock = new MergeLock();
109109

110110
while (!shutdownState.shuttingDown) {
111111
try {
@@ -456,87 +456,15 @@ export async function runLoop(cliArgs: CliArgs, runner: AgentRunner, shutdownSta
456456
});
457457
};
458458
// Extract COMPLETION_JSON from agent stdout → enrich post_action update_task
459-
// Extract COMPLETION_JSON or stream result from agent stdout
460-
// Fallback: if no COMPLETION_JSON found, check stream-json result event
461459
if (!actionCtx.completionJson) {
462-
for (const l of runningAgent.stdout) {
463-
try {
464-
const ev = JSON.parse(l);
465-
if (ev.type === "result" && ev.subtype === "success" && typeof ev.result === "string") {
466-
// Try to extract COMPLETION_JSON from result text
467-
const cjMatch = ev.result.match(/COMPLETION_JSON:(\{[\s\S]*\})/);
468-
let resultText: string;
469-
if (cjMatch) {
470-
try {
471-
const cj = JSON.parse(cjMatch[1]);
472-
actionCtx.completionJson = { review_comment: cj.review_comment, commits: cj.commits };
473-
resultText = cj.review_comment || ev.result.slice(0, 2000);
474-
} catch {
475-
resultText = ev.result.slice(0, 2000);
476-
actionCtx.completionJson = { review_comment: resultText, commits: "" };
477-
}
478-
} else {
479-
resultText = ev.result.slice(0, 2000);
480-
actionCtx.completionJson = { review_comment: resultText, commits: "" };
481-
}
482-
for (const action of agentTemplate.post_actions) {
483-
if (action.type === "update_task" && action.when === "success" && action.params?.status === "review") {
484-
action.params = { ...action.params, review_comment: resultText, commits: "" };
485-
break;
486-
}
487-
}
488-
ui.info(`[completion] Extracted completion from stream result (no COMPLETION_JSON)`);
489-
taskLog.event("completion_parse", { source: "stream_result", review_comment: resultText.slice(0, 200) });
490-
break;
491-
}
492-
} catch { /* skip */ }
493-
}
494-
}
495-
for (const line of runningAgent.stdout) {
496-
const completionLine = line.startsWith("COMPLETION_JSON:") ? line : null;
497-
if (completionLine) {
498-
try {
499-
const json = JSON.parse(completionLine.slice("COMPLETION_JSON:".length));
500-
// Inject review_comment and commits into the update_task post_action params
501-
for (const action of agentTemplate.post_actions) {
502-
if (action.type === "update_task" && action.when === "success" && action.params?.status === "review") {
503-
action.params = { ...action.params, review_comment: json.review_comment, commits: json.commits };
504-
break;
505-
}
506-
}
507-
actionCtx.completionJson = { review_comment: json.review_comment, commits: json.commits };
508-
ui.info(`[completion] Parsed COMPLETION_JSON: ${json.review_comment?.slice(0, 80)}...`);
509-
taskLog.event("completion_parse", { source: "completion_json", review_comment: json.review_comment?.slice(0, 200), commits: json.commits });
510-
// Broadcast review comment immediately for real-time dashboard update
511-
if (json.review_comment) {
512-
actionCtx.onReviewUpdate?.(task.id, "agent_submitted", json.review_comment);
513-
}
514-
break;
515-
} catch { /* skip */ }
460+
const parsed = extractCompletionJson(runningAgent.stdout, agentTemplate.post_actions, {
461+
onReviewUpdate: (tid, phase, comment) => actionCtx.onReviewUpdate?.(tid, phase, comment),
462+
taskId: task.id,
463+
taskLog,
464+
});
465+
if (parsed) {
466+
actionCtx.completionJson = parsed;
516467
}
517-
// Also try stream-json events
518-
try {
519-
const event = JSON.parse(line);
520-
if (event.type === "result" && typeof event.result === "string") {
521-
const match = event.result.match(/COMPLETION_JSON:(\{[\s\S]*\})/);
522-
if (match) {
523-
const json = JSON.parse(match[1]);
524-
for (const action of agentTemplate.post_actions) {
525-
if (action.type === "update_task" && action.when === "success" && action.params?.status === "review") {
526-
action.params = { ...action.params, review_comment: json.review_comment, commits: json.commits };
527-
break;
528-
}
529-
}
530-
actionCtx.completionJson = { review_comment: json.review_comment, commits: json.commits };
531-
ui.info(`[completion] Parsed COMPLETION_JSON from stream: ${json.review_comment?.slice(0, 80)}...`);
532-
// Broadcast review comment immediately for real-time dashboard update
533-
if (json.review_comment) {
534-
actionCtx.onReviewUpdate?.(task.id, "agent_submitted", json.review_comment);
535-
}
536-
break;
537-
}
538-
}
539-
} catch { /* not JSON */ }
540468
}
541469

542470
actionCtx.onRetro = async () => {

src/commands/shutdown.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,18 @@
33
*/
44

55
import type { AgentRunner } from "../runner.js";
6-
import type { ChatPoller } from "../chat-poller.js";
76
import type { WsChatServer } from "../ws-server.js";
87
import * as ui from "../ui.js";
98

109
export interface ShutdownState {
1110
shuttingDown: boolean;
12-
activeChatPoller: ChatPoller | null;
1311
activeManager: { stop: () => void } | null;
1412
activeWsServer: WsChatServer | null;
1513
}
1614

1715
export function createShutdownState(): ShutdownState {
1816
return {
1917
shuttingDown: false,
20-
activeChatPoller: null,
2118
activeManager: null,
2219
activeWsServer: null,
2320
};
@@ -29,7 +26,6 @@ export function setupShutdownHandlers(runner: AgentRunner, state: ShutdownState)
2926
state.shuttingDown = true;
3027
ui.warn("Shutting down...");
3128
state.activeWsServer?.stop().catch(() => {});
32-
state.activeChatPoller?.stop();
3329
state.activeManager?.stop();
3430
for (const agent of runner.status()) { ui.info(`Stopping agent: ${agent.name}`); runner.stop(agent.name); }
3531
setTimeout(() => { ui.shutdown(); process.exit(0); }, 3000);

src/constants.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* Shared constants — timeouts, intervals, and limits.
3+
*/
4+
5+
export const TIMEOUTS = {
6+
REVIEWER: 300_000, // 5 min
7+
REVIEW_LLM: 120_000, // 2 min
8+
CLAUDE_CLI: 180_000, // 3 min
9+
GIT_OPERATION: 30_000, // 30 sec
10+
SPLIT_TASK: 30_000, // 30 sec
11+
} as const;
12+
13+
export const INTERVALS = {
14+
POLL: 30_000, // 30 sec
15+
MESSAGE_POLL: 10_000, // 10 sec
16+
} as const;
17+
18+
export const LIMITS = {
19+
MAX_DIFF_LINES: 300,
20+
MAX_RETRIES: 3,
21+
LOG_BUFFER_SIZE: 200,
22+
} as const;

src/git-ops.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,20 @@ import { join } from "node:path";
88
import type { WorkspaceRepository } from "./api-client.js";
99
import * as ui from "./ui.js";
1010

11+
/**
12+
* Resolve the git repo root from a working directory (handles worktrees).
13+
* Falls back to the given directory if resolution fails.
14+
*/
15+
export function resolveRepoRoot(workingDir: string): string {
16+
if (!existsSync(workingDir)) return workingDir;
17+
try {
18+
return execSync("git rev-parse --path-format=absolute --git-common-dir", { cwd: workingDir, stdio: "pipe" })
19+
.toString().trim().replace(/\/.git$/, "");
20+
} catch {
21+
return workingDir;
22+
}
23+
}
24+
1125
/**
1226
* Create a git credential helper script that fetches fresh tokens from the Toban API.
1327
* GitHub App installation tokens expire after 1 hour, so we need to refresh on each git operation.

src/handlers/git-merge.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import type { TemplateAction, ActionContext } from "../agent-templates.js";
77
import * as ui from "../ui.js";
88
import { logError, CLI_ERR } from "../error-logger.js";
9+
import { resolveRepoRoot } from "../git-ops.js";
910

1011
export async function handleGitMerge(
1112
action: TemplateAction,
@@ -17,10 +18,8 @@ export async function handleGitMerge(
1718
const { existsSync: gitExists } = await import("node:fs");
1819
const { join: gitJoin } = await import("node:path");
1920
// workingDir is the worktree path — we need the main repo root
20-
// Worktree is at <repo>/.worktrees/<branch>/, so repo root is 2 levels up
2121
const worktreePath = ctx.config.workingDir;
22-
const repoDir = gitExec("git rev-parse --path-format=absolute --git-common-dir", { cwd: worktreePath, stdio: "pipe" })
23-
.toString().trim().replace(/\/.git$/, "");
22+
const repoDir = resolveRepoRoot(worktreePath);
2423
const baseBranch = ctx.config.baseBranch;
2524

2625
// Find the agent's worktree branch

src/handlers/git-push.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import type { TemplateAction, ActionContext } from "../agent-templates.js";
77
import * as ui from "../ui.js";
88
import { logError, CLI_ERR } from "../error-logger.js";
9+
import { resolveRepoRoot } from "../git-ops.js";
910

1011
export async function handleGitPush(
1112
action: TemplateAction,
@@ -16,8 +17,7 @@ export async function handleGitPush(
1617
if (ctx.mergeSkipped) { ui.info(`[${phase}] ${label}: skipped (no merge)`); return; }
1718
const { execSync: pushExec } = await import("node:child_process");
1819
// Resolve repo root (workingDir may be a worktree)
19-
const pushRepoDir = pushExec("git rev-parse --path-format=absolute --git-common-dir", { cwd: ctx.config.workingDir, stdio: "pipe" })
20-
.toString().trim().replace(/\/.git$/, "");
20+
const pushRepoDir = resolveRepoRoot(ctx.config.workingDir);
2121
// Stash any unstaged changes (e.g. inject_memory's CLAUDE.md modifications)
2222
try {
2323
pushExec("git stash --include-untracked", { cwd: pushRepoDir, stdio: "pipe" });

src/handlers/review-changes.ts

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ import type { Task } from "../api-client.js";
1010
import { fetchWithRetry } from "../api-client.js";
1111
import * as ui from "../ui.js";
1212
import { logError, CLI_ERR } from "../error-logger.js";
13-
import { resolveModelForRole } from "../agent-engine.js";
1413
import { parseTaskLabels } from "../utils/parse-labels.js";
14+
import { spawnClaudeOnce } from "../utils/spawn-claude.js";
15+
import { resolveRepoRoot } from "../git-ops.js";
16+
import { TIMEOUTS } from "../constants.js";
1517

1618
export async function handleReviewChanges(
1719
action: TemplateAction,
@@ -24,13 +26,8 @@ export async function handleReviewChanges(
2426
const { existsSync: revExists } = await import("node:fs");
2527
// workingDir may be a deleted worktree after git_merge — resolve repo root
2628
const revRepoDir = (() => {
27-
// Try workingDir first (may be worktree or repo root)
28-
if (revExists(ctx.config.workingDir)) {
29-
try {
30-
return revExec("git rev-parse --path-format=absolute --git-common-dir", { cwd: ctx.config.workingDir, stdio: "pipe" })
31-
.toString().trim().replace(/\/.git$/, "");
32-
} catch { /* fall through */ }
33-
}
29+
const resolved = resolveRepoRoot(ctx.config.workingDir);
30+
if (resolved !== ctx.config.workingDir) return resolved;
3431
// Worktree deleted — walk up to find the repo root
3532
// dirname imported at module top
3633
let dir = ctx.config.workingDir;
@@ -75,7 +72,6 @@ export async function handleReviewChanges(
7572
ctx.onReviewUpdate?.(ctx.task.id, "analyzing");
7673
let llmReview = "";
7774
try {
78-
const { spawn: reviewSpawn } = await import("node:child_process");
7975
// Keep full diff context but filter out test files for size reduction
8076
const diffLines = diffContent.split("\n");
8177
const filteredDiff: string[] = [];
@@ -120,30 +116,10 @@ If tests failed, verdict MUST be NEEDS_CHANGES.
120116
121117
${outputFormat}`;
122118

123-
const env = { ...process.env };
124-
delete env.CLAUDECODE;
125-
const LLM_TIMEOUT = 120_000; // 2 minutes
126-
llmReview = await new Promise<string>((resolve) => {
127-
const child = reviewSpawn("claude", ["--print", "--model", resolveModelForRole("reviewer"), "--max-turns", "1", reviewPrompt], {
128-
env, stdio: ["ignore", "pipe", "pipe"], timeout: LLM_TIMEOUT,
129-
});
130-
let out = "";
131-
let resolved = false;
132-
child.stdout?.on("data", (chunk: Buffer) => { out += chunk.toString(); });
133-
child.on("close", (code) => {
134-
if (!resolved) { resolved = true; resolve(out.trim()); }
135-
if (code !== 0) ui.warn(`[review] LLM exited with code ${code}`);
136-
});
137-
child.on("error", (err) => {
138-
if (!resolved) { resolved = true; resolve(""); }
139-
ui.warn(`[review] LLM spawn error: ${err.message}`);
140-
});
141-
setTimeout(() => {
142-
if (!resolved) { resolved = true; resolve(""); }
143-
try { child.kill(); } catch {}
144-
ui.warn("[review] LLM review timed out (120s)");
145-
}, LLM_TIMEOUT);
119+
llmReview = await spawnClaudeOnce(reviewPrompt, {
120+
role: "reviewer", maxTurns: 1, timeout: TIMEOUTS.REVIEW_LLM,
146121
});
122+
llmReview = llmReview.trim();
147123
} catch (llmErr) {
148124
logError(CLI_ERR.REVIEW_LLM_FAILED, `LLM review failed`, { taskId: ctx.task.id }, llmErr);
149125
ui.warn(`[review] LLM review failed: ${llmErr instanceof Error ? llmErr.message : llmErr}`);

0 commit comments

Comments
 (0)