From d8e7e4f170e14e8ca775a1ba11f07aad33891865 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 13:30:00 +0900 Subject: [PATCH 1/2] refactor: extract git worktree parser from atlas hook --- src/hooks/atlas/index.ts | 113 +----------------- .../git-worktree/collect-git-diff-stats.ts | 29 +++++ .../git-worktree/format-file-changes.ts | 46 +++++++ src/shared/git-worktree/git-worktree.test.ts | 51 ++++++++ src/shared/git-worktree/index.ts | 5 + src/shared/git-worktree/parse-diff-numstat.ts | 27 +++++ .../git-worktree/parse-status-porcelain.ts | 25 ++++ src/shared/git-worktree/types.ts | 8 ++ src/shared/index.ts | 1 + 9 files changed, 195 insertions(+), 110 deletions(-) create mode 100644 src/shared/git-worktree/collect-git-diff-stats.ts create mode 100644 src/shared/git-worktree/format-file-changes.ts create mode 100644 src/shared/git-worktree/git-worktree.test.ts create mode 100644 src/shared/git-worktree/index.ts create mode 100644 src/shared/git-worktree/parse-diff-numstat.ts create mode 100644 src/shared/git-worktree/parse-status-porcelain.ts create mode 100644 src/shared/git-worktree/types.ts diff --git a/src/hooks/atlas/index.ts b/src/hooks/atlas/index.ts index ffad0459..b2608187 100644 --- a/src/hooks/atlas/index.ts +++ b/src/hooks/atlas/index.ts @@ -1,5 +1,4 @@ import type { PluginInput } from "@opencode-ai/plugin" -import { execSync } from "node:child_process" import { existsSync, readdirSync } from "node:fs" import { join } from "node:path" import { @@ -12,6 +11,7 @@ import { findNearestMessageWithFields, MESSAGE_STORAGE } from "../../features/ho import { log } from "../../shared/logger" import { createSystemDirective, SYSTEM_DIRECTIVE_PREFIX, SystemDirectiveTypes } from "../../shared/system-directive" import { isCallerOrchestrator, getMessageDir } from "../../shared/session-utils" +import { collectGitDiffStats, formatFileChanges } from "../../shared/git-worktree" import type { BackgroundManager } from "../../features/background-agent" export const HOOK_NAME = "atlas" @@ -269,113 +269,6 @@ function extractSessionIdFromOutput(output: string): string { return match?.[1] ?? "" } -interface GitFileStat { - path: string - added: number - removed: number - status: "modified" | "added" | "deleted" -} - -function getGitDiffStats(directory: string): GitFileStat[] { - try { - const output = execSync("git diff --numstat HEAD", { - cwd: directory, - encoding: "utf-8", - timeout: 5000, - stdio: ["pipe", "pipe", "pipe"], - }).trim() - - if (!output) return [] - - const statusOutput = execSync("git status --porcelain", { - cwd: directory, - encoding: "utf-8", - timeout: 5000, - stdio: ["pipe", "pipe", "pipe"], - }).trim() - - const statusMap = new Map() - for (const line of statusOutput.split("\n")) { - if (!line) continue - const status = line.substring(0, 2).trim() - const filePath = line.substring(3) - if (status === "A" || status === "??") { - statusMap.set(filePath, "added") - } else if (status === "D") { - statusMap.set(filePath, "deleted") - } else { - statusMap.set(filePath, "modified") - } - } - - const stats: GitFileStat[] = [] - for (const line of output.split("\n")) { - const parts = line.split("\t") - if (parts.length < 3) continue - - const [addedStr, removedStr, path] = parts - const added = addedStr === "-" ? 0 : parseInt(addedStr, 10) - const removed = removedStr === "-" ? 0 : parseInt(removedStr, 10) - - stats.push({ - path, - added, - removed, - status: statusMap.get(path) ?? "modified", - }) - } - - return stats - } catch { - return [] - } -} - -function formatFileChanges(stats: GitFileStat[], notepadPath?: string): string { - if (stats.length === 0) return "[FILE CHANGES SUMMARY]\nNo file changes detected.\n" - - const modified = stats.filter((s) => s.status === "modified") - const added = stats.filter((s) => s.status === "added") - const deleted = stats.filter((s) => s.status === "deleted") - - const lines: string[] = ["[FILE CHANGES SUMMARY]"] - - if (modified.length > 0) { - lines.push("Modified files:") - for (const f of modified) { - lines.push(` ${f.path} (+${f.added}, -${f.removed})`) - } - lines.push("") - } - - if (added.length > 0) { - lines.push("Created files:") - for (const f of added) { - lines.push(` ${f.path} (+${f.added})`) - } - lines.push("") - } - - if (deleted.length > 0) { - lines.push("Deleted files:") - for (const f of deleted) { - lines.push(` ${f.path} (-${f.removed})`) - } - lines.push("") - } - - if (notepadPath) { - const notepadStat = stats.find((s) => s.path.includes("notepad") || s.path.includes(".sisyphus")) - if (notepadStat) { - lines.push("[NOTEPAD UPDATED]") - lines.push(` ${notepadStat.path} (+${notepadStat.added})`) - lines.push("") - } - } - - return lines.join("\n") -} - interface ToolExecuteAfterInput { tool: string sessionID?: string @@ -750,8 +643,8 @@ export function createAtlasHook( } if (output.output && typeof output.output === "string") { - const gitStats = getGitDiffStats(ctx.directory) - const fileChanges = formatFileChanges(gitStats) + const gitStats = collectGitDiffStats(ctx.directory) + const fileChanges = formatFileChanges(gitStats) const subagentSessionId = extractSessionIdFromOutput(output.output) const boulderState = readBoulderState(ctx.directory) diff --git a/src/shared/git-worktree/collect-git-diff-stats.ts b/src/shared/git-worktree/collect-git-diff-stats.ts new file mode 100644 index 00000000..158a09d8 --- /dev/null +++ b/src/shared/git-worktree/collect-git-diff-stats.ts @@ -0,0 +1,29 @@ +import { execSync } from "node:child_process" +import { parseGitStatusPorcelain } from "./parse-status-porcelain" +import { parseGitDiffNumstat } from "./parse-diff-numstat" +import type { GitFileStat } from "./types" + +export function collectGitDiffStats(directory: string): GitFileStat[] { + try { + const diffOutput = execSync("git diff --numstat HEAD", { + cwd: directory, + encoding: "utf-8", + timeout: 5000, + stdio: ["pipe", "pipe", "pipe"], + }).trim() + + if (!diffOutput) return [] + + const statusOutput = execSync("git status --porcelain", { + cwd: directory, + encoding: "utf-8", + timeout: 5000, + stdio: ["pipe", "pipe", "pipe"], + }).trim() + + const statusMap = parseGitStatusPorcelain(statusOutput) + return parseGitDiffNumstat(diffOutput, statusMap) + } catch { + return [] + } +} diff --git a/src/shared/git-worktree/format-file-changes.ts b/src/shared/git-worktree/format-file-changes.ts new file mode 100644 index 00000000..5afb58b8 --- /dev/null +++ b/src/shared/git-worktree/format-file-changes.ts @@ -0,0 +1,46 @@ +import type { GitFileStat } from "./types" + +export function formatFileChanges(stats: GitFileStat[], notepadPath?: string): string { + if (stats.length === 0) return "[FILE CHANGES SUMMARY]\nNo file changes detected.\n" + + const modified = stats.filter((s) => s.status === "modified") + const added = stats.filter((s) => s.status === "added") + const deleted = stats.filter((s) => s.status === "deleted") + + const lines: string[] = ["[FILE CHANGES SUMMARY]"] + + if (modified.length > 0) { + lines.push("Modified files:") + for (const f of modified) { + lines.push(` ${f.path} (+${f.added}, -${f.removed})`) + } + lines.push("") + } + + if (added.length > 0) { + lines.push("Created files:") + for (const f of added) { + lines.push(` ${f.path} (+${f.added})`) + } + lines.push("") + } + + if (deleted.length > 0) { + lines.push("Deleted files:") + for (const f of deleted) { + lines.push(` ${f.path} (-${f.removed})`) + } + lines.push("") + } + + if (notepadPath) { + const notepadStat = stats.find((s) => s.path.includes("notepad") || s.path.includes(".sisyphus")) + if (notepadStat) { + lines.push("[NOTEPAD UPDATED]") + lines.push(` ${notepadStat.path} (+${notepadStat.added})`) + lines.push("") + } + } + + return lines.join("\n") +} diff --git a/src/shared/git-worktree/git-worktree.test.ts b/src/shared/git-worktree/git-worktree.test.ts new file mode 100644 index 00000000..27183018 --- /dev/null +++ b/src/shared/git-worktree/git-worktree.test.ts @@ -0,0 +1,51 @@ +/// + +import { describe, expect, test } from "bun:test" +import { formatFileChanges, parseGitDiffNumstat, parseGitStatusPorcelain } from "./index" + +describe("git-worktree", () => { + test("#given status porcelain output #when parsing #then maps paths to statuses", () => { + const porcelain = [ + " M src/a.ts", + "A src/b.ts", + "?? src/c.ts", + "D src/d.ts", + ].join("\n") + + const map = parseGitStatusPorcelain(porcelain) + expect(map.get("src/a.ts")).toBe("modified") + expect(map.get("src/b.ts")).toBe("added") + expect(map.get("src/c.ts")).toBe("added") + expect(map.get("src/d.ts")).toBe("deleted") + }) + + test("#given diff numstat and status map #when parsing #then returns typed stats", () => { + const porcelain = [" M src/a.ts", "A src/b.ts"].join("\n") + const statusMap = parseGitStatusPorcelain(porcelain) + + const numstat = ["1\t2\tsrc/a.ts", "3\t0\tsrc/b.ts", "-\t-\tbin.dat"].join("\n") + const stats = parseGitDiffNumstat(numstat, statusMap) + + expect(stats).toEqual([ + { path: "src/a.ts", added: 1, removed: 2, status: "modified" }, + { path: "src/b.ts", added: 3, removed: 0, status: "added" }, + { path: "bin.dat", added: 0, removed: 0, status: "modified" }, + ]) + }) + + test("#given git file stats #when formatting #then produces grouped summary", () => { + const summary = formatFileChanges([ + { path: "src/a.ts", added: 1, removed: 2, status: "modified" }, + { path: "src/b.ts", added: 3, removed: 0, status: "added" }, + { path: "src/c.ts", added: 0, removed: 4, status: "deleted" }, + ]) + + expect(summary).toContain("[FILE CHANGES SUMMARY]") + expect(summary).toContain("Modified files:") + expect(summary).toContain("Created files:") + expect(summary).toContain("Deleted files:") + expect(summary).toContain("src/a.ts") + expect(summary).toContain("src/b.ts") + expect(summary).toContain("src/c.ts") + }) +}) diff --git a/src/shared/git-worktree/index.ts b/src/shared/git-worktree/index.ts new file mode 100644 index 00000000..0bc363d0 --- /dev/null +++ b/src/shared/git-worktree/index.ts @@ -0,0 +1,5 @@ +export type { GitFileStatus, GitFileStat } from "./types" +export { parseGitStatusPorcelain } from "./parse-status-porcelain" +export { parseGitDiffNumstat } from "./parse-diff-numstat" +export { collectGitDiffStats } from "./collect-git-diff-stats" +export { formatFileChanges } from "./format-file-changes" diff --git a/src/shared/git-worktree/parse-diff-numstat.ts b/src/shared/git-worktree/parse-diff-numstat.ts new file mode 100644 index 00000000..3ea2b0f6 --- /dev/null +++ b/src/shared/git-worktree/parse-diff-numstat.ts @@ -0,0 +1,27 @@ +import type { GitFileStat, GitFileStatus } from "./types" + +export function parseGitDiffNumstat( + output: string, + statusMap: Map +): GitFileStat[] { + if (!output) return [] + + const stats: GitFileStat[] = [] + for (const line of output.split("\n")) { + const parts = line.split("\t") + if (parts.length < 3) continue + + const [addedStr, removedStr, path] = parts + const added = addedStr === "-" ? 0 : parseInt(addedStr, 10) + const removed = removedStr === "-" ? 0 : parseInt(removedStr, 10) + + stats.push({ + path, + added, + removed, + status: statusMap.get(path) ?? "modified", + }) + } + + return stats +} diff --git a/src/shared/git-worktree/parse-status-porcelain.ts b/src/shared/git-worktree/parse-status-porcelain.ts new file mode 100644 index 00000000..0623de5d --- /dev/null +++ b/src/shared/git-worktree/parse-status-porcelain.ts @@ -0,0 +1,25 @@ +import type { GitFileStatus } from "./types" + +export function parseGitStatusPorcelain(output: string): Map { + const map = new Map() + if (!output) return map + + for (const line of output.split("\n")) { + if (!line) continue + + const status = line.substring(0, 2).trim() + const filePath = line.substring(3) + + if (!filePath) continue + + if (status === "A" || status === "??") { + map.set(filePath, "added") + } else if (status === "D") { + map.set(filePath, "deleted") + } else { + map.set(filePath, "modified") + } + } + + return map +} diff --git a/src/shared/git-worktree/types.ts b/src/shared/git-worktree/types.ts new file mode 100644 index 00000000..eb423699 --- /dev/null +++ b/src/shared/git-worktree/types.ts @@ -0,0 +1,8 @@ +export type GitFileStatus = "modified" | "added" | "deleted" + +export interface GitFileStat { + path: string + added: number + removed: number + status: GitFileStatus +} diff --git a/src/shared/index.ts b/src/shared/index.ts index d42be5a7..4ea34697 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -41,5 +41,6 @@ export * from "./tmux" export * from "./model-suggestion-retry" export * from "./opencode-server-auth" export * from "./port-utils" +export * from "./git-worktree" export * from "./safe-create-hook" export * from "./truncate-description" From 676ff513fa896152d25570e65fe555ab0a49d1e4 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 14:50:36 +0900 Subject: [PATCH 2/2] fix: detect completion tags in ralph/ULW loop to stop iteration (#1233) --- src/hooks/ralph-loop/index.test.ts | 92 ++++++++++++++++++++---------- src/hooks/ralph-loop/index.ts | 66 +++++++++++++++------ 2 files changed, 111 insertions(+), 47 deletions(-) diff --git a/src/hooks/ralph-loop/index.test.ts b/src/hooks/ralph-loop/index.test.ts index 9c7ce4f1..851e0ce1 100644 --- a/src/hooks/ralph-loop/index.test.ts +++ b/src/hooks/ralph-loop/index.test.ts @@ -511,6 +511,38 @@ describe("ralph-loop", () => { expect(messagesCalls[0].sessionID).toBe("session-123") }) + test("should detect completion promise in reasoning part via session messages API", async () => { + //#given - active loop with assistant reasoning containing completion promise + mockSessionMessages = [ + { info: { role: "user" }, parts: [{ type: "text", text: "Build something" }] }, + { + info: { role: "assistant" }, + parts: [ + { type: "reasoning", text: "I am done now. REASONING_DONE" }, + ], + }, + ] + const hook = createRalphLoopHook(createMockPluginInput(), { + getTranscriptPath: () => join(TEST_DIR, "nonexistent.jsonl"), + }) + hook.startLoop("session-123", "Build something", { + completionPromise: "REASONING_DONE", + }) + + //#when - session goes idle + await hook.event({ + event: { + type: "session.idle", + properties: { sessionID: "session-123" }, + }, + }) + + //#then - loop completed via API detection, no continuation + expect(promptCalls.length).toBe(0) + expect(toastCalls.some((t) => t.title === "Ralph Loop Complete!")).toBe(true) + expect(hook.getState()).toBeNull() + }) + test("should handle multiple iterations correctly", async () => { // given - active loop const hook = createRalphLoopHook(createMockPluginInput()) @@ -596,13 +628,14 @@ describe("ralph-loop", () => { expect(promptCalls.length).toBe(1) }) - test("should only check LAST assistant message for completion", async () => { - // given - multiple assistant messages, only first has completion promise + test("should check last 3 assistant messages for completion", async () => { + // given - multiple assistant messages, promise in recent (not last) assistant message mockSessionMessages = [ { info: { role: "user" }, parts: [{ type: "text", text: "Start task" }] }, - { info: { role: "assistant" }, parts: [{ type: "text", text: "I'll work on it. DONE" }] }, + { info: { role: "assistant" }, parts: [{ type: "text", text: "Working on it." }] }, { info: { role: "user" }, parts: [{ type: "text", text: "Continue" }] }, - { info: { role: "assistant" }, parts: [{ type: "text", text: "Working on more features..." }] }, + { info: { role: "assistant" }, parts: [{ type: "text", text: "Nearly there... DONE" }] }, + { info: { role: "assistant" }, parts: [{ type: "text", text: "(extra output after promise)" }] }, ] const hook = createRalphLoopHook(createMockPluginInput(), { getTranscriptPath: () => join(TEST_DIR, "nonexistent.jsonl"), @@ -614,35 +647,36 @@ describe("ralph-loop", () => { event: { type: "session.idle", properties: { sessionID: "session-123" } }, }) - // then - loop should continue (last message has no completion promise) - expect(promptCalls.length).toBe(1) - expect(hook.getState()?.iteration).toBe(2) - }) - - test("should detect completion only in LAST assistant message", async () => { - // given - last assistant message has completion promise - mockSessionMessages = [ - { info: { role: "user" }, parts: [{ type: "text", text: "Start task" }] }, - { info: { role: "assistant" }, parts: [{ type: "text", text: "Starting work..." }] }, - { info: { role: "user" }, parts: [{ type: "text", text: "Continue" }] }, - { info: { role: "assistant" }, parts: [{ type: "text", text: "Task complete! DONE" }] }, - ] - const hook = createRalphLoopHook(createMockPluginInput(), { - getTranscriptPath: () => join(TEST_DIR, "nonexistent.jsonl"), - }) - hook.startLoop("session-123", "Build something", { completionPromise: "DONE" }) - - // when - session goes idle - await hook.event({ - event: { type: "session.idle", properties: { sessionID: "session-123" } }, - }) - - // then - loop should complete (last message has completion promise) + // then - loop should complete (promise found within last 3 assistant messages) expect(promptCalls.length).toBe(0) expect(toastCalls.some((t) => t.title === "Ralph Loop Complete!")).toBe(true) expect(hook.getState()).toBeNull() }) + test("should NOT detect completion if promise is older than last 3 assistant messages", async () => { + // given - promise appears in an assistant message older than last 3 + mockSessionMessages = [ + { info: { role: "user" }, parts: [{ type: "text", text: "Start task" }] }, + { info: { role: "assistant" }, parts: [{ type: "text", text: "Promise early DONE" }] }, + { info: { role: "assistant" }, parts: [{ type: "text", text: "More work 1" }] }, + { info: { role: "assistant" }, parts: [{ type: "text", text: "More work 2" }] }, + { info: { role: "assistant" }, parts: [{ type: "text", text: "More work 3" }] }, + ] + const hook = createRalphLoopHook(createMockPluginInput(), { + getTranscriptPath: () => join(TEST_DIR, "nonexistent.jsonl"), + }) + hook.startLoop("session-123", "Build something", { completionPromise: "DONE" }) + + // when - session goes idle + await hook.event({ + event: { type: "session.idle", properties: { sessionID: "session-123" } }, + }) + + // then - loop should continue (promise is older than last 3 assistant messages) + expect(promptCalls.length).toBe(1) + expect(hook.getState()?.iteration).toBe(2) + }) + test("should allow starting new loop while previous loop is active (different session)", async () => { // given - active loop in session A const hook = createRalphLoopHook(createMockPluginInput()) @@ -928,7 +962,7 @@ Original task: Build something` const elapsed = Date.now() - startTime // then - should complete quickly (not hang for 10s) - expect(elapsed).toBeLessThan(2000) + expect(elapsed).toBeLessThan(6000) // then - loop should continue (API error = no completion detected) expect(promptCalls.length).toBe(1) expect(apiCallCount).toBeGreaterThan(0) diff --git a/src/hooks/ralph-loop/index.ts b/src/hooks/ralph-loop/index.ts index 3cc77edd..693ef37e 100644 --- a/src/hooks/ralph-loop/index.ts +++ b/src/hooks/ralph-loop/index.ts @@ -67,7 +67,7 @@ export interface RalphLoopHook { getState: () => RalphLoopState | null } -const DEFAULT_API_TIMEOUT = 3000 +const DEFAULT_API_TIMEOUT = 5000 export function createRalphLoopHook( ctx: PluginInput, @@ -80,6 +80,23 @@ export function createRalphLoopHook( const apiTimeout = options?.apiTimeout ?? DEFAULT_API_TIMEOUT const checkSessionExists = options?.checkSessionExists + async function withTimeout(promise: Promise, timeoutMs: number): Promise { + let timeoutId: ReturnType | undefined + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout(() => { + reject(new Error("API timeout")) + }, timeoutMs) + }) + + try { + return await Promise.race([promise, timeoutPromise]) + } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId) + } + } + } + function getSessionState(sessionID: string): SessionState { let state = sessions.get(sessionID) if (!state) { @@ -126,34 +143,44 @@ export function createRalphLoopHook( promise: string ): Promise { try { - const response = await Promise.race([ + const response = await withTimeout( ctx.client.session.messages({ path: { id: sessionID }, query: { directory: ctx.directory }, }), - new Promise((_, reject) => - setTimeout(() => reject(new Error("API timeout")), apiTimeout) - ), - ]) + apiTimeout + ) const messages = (response as { data?: unknown[] }).data ?? [] if (!Array.isArray(messages)) return false - const assistantMessages = (messages as OpenCodeSessionMessage[]).filter( - (msg) => msg.info?.role === "assistant" - ) - const lastAssistant = assistantMessages[assistantMessages.length - 1] - if (!lastAssistant?.parts) return false + const assistantMessages = (messages as OpenCodeSessionMessage[]).filter((msg) => msg.info?.role === "assistant") + if (assistantMessages.length === 0) return false const pattern = new RegExp(`\\s*${escapeRegex(promise)}\\s*`, "is") - const responseText = lastAssistant.parts - .filter((p) => p.type === "text") - .map((p) => p.text ?? "") - .join("\n") - return pattern.test(responseText) + const recentAssistants = assistantMessages.slice(-3) + for (const assistant of recentAssistants) { + if (!assistant.parts) continue + + const responseText = assistant.parts + .filter((p) => p.type === "text" || p.type === "reasoning") + .map((p) => p.text ?? "") + .join("\n") + + if (pattern.test(responseText)) { + return true + } + } + + return false } catch (err) { - log(`[${HOOK_NAME}] Session messages check failed`, { sessionID, error: String(err) }) + setTimeout(() => { + log(`[${HOOK_NAME}] Session messages check failed`, { + sessionID, + error: String(err), + }) + }, 0) return false } } @@ -343,7 +370,10 @@ export function createRalphLoopHook( let model: { providerID: string; modelID: string } | undefined try { - const messagesResp = await ctx.client.session.messages({ path: { id: sessionID } }) + const messagesResp = await withTimeout( + ctx.client.session.messages({ path: { id: sessionID } }), + apiTimeout + ) const messages = (messagesResp.data ?? []) as Array<{ info?: { agent?: string; model?: { providerID: string; modelID: string }; modelID?: string; providerID?: string } }>