Merge pull request #3089 from code-yeongyu/fix/prepublish-background-regressions

fix(background-agent): bound abort waits and remove dead worktree sync
This commit is contained in:
YeonGyu-Kim
2026-04-04 01:46:43 +09:00
committed by GitHub
9 changed files with 102 additions and 134 deletions
@@ -0,0 +1,57 @@
import { afterAll, describe, expect, mock, test } from "bun:test"
const logMock = mock(() => {})
mock.module("../../shared", () => ({
log: logMock,
}))
import { abortWithTimeout } from "./abort-with-timeout"
import type { OpencodeClient } from "./opencode-client"
function createClient(abort: (...args: Array<unknown>) => Promise<unknown>): OpencodeClient {
return {
session: {
abort: abort as never,
},
} as never
}
describe("abortWithTimeout", () => {
afterAll(() => {
mock.restore()
})
test("#given abort resolves before timeout #when abortWithTimeout runs #then it returns true", async () => {
// given
const abort = mock(async () => ({}))
// when
const result = await abortWithTimeout(createClient(abort), "session-1", 10)
// then
expect(result).toBe(true)
expect(abort).toHaveBeenCalledWith({ path: { id: "session-1" } })
expect(logMock).not.toHaveBeenCalled()
})
test("#given abort hangs indefinitely #when abortWithTimeout runs #then it logs warning and continues", async () => {
// given
const abort = mock(() => new Promise<never>(() => {}))
// when
const result = await Promise.race([
abortWithTimeout(createClient(abort), "session-2", 1),
new Promise<never>((_, reject) => {
setTimeout(() => reject(new Error("abort timeout test exceeded wait budget")), 100)
}),
])
// then
expect(result).toBe(false)
expect(logMock).toHaveBeenCalledWith(
"[background-agent] Session abort timed out; continuing cleanup:",
{ sessionID: "session-2", timeoutMs: 1 },
)
})
})
@@ -0,0 +1,35 @@
import { log } from "../../shared"
import type { OpencodeClient } from "./opencode-client"
export async function abortWithTimeout(
client: OpencodeClient,
sessionID: string,
timeoutMs = 10_000,
): Promise<boolean> {
let timeoutHandle: ReturnType<typeof setTimeout> | undefined
try {
const result = await Promise.race([
client.session.abort({ path: { id: sessionID } }).then(() => "aborted" as const),
new Promise<"timed_out">((resolve) => {
timeoutHandle = setTimeout(() => {
resolve("timed_out")
}, timeoutMs)
}),
])
if (result === "timed_out") {
log("[background-agent] Session abort timed out; continuing cleanup:", {
sessionID,
timeoutMs,
})
return false
}
return true
} finally {
if (timeoutHandle) {
clearTimeout(timeoutHandle)
}
}
}
@@ -10,6 +10,7 @@ import {
selectFallbackProvider,
} from "../../shared/model-error-classifier"
import { transformModelForProvider } from "../../shared/provider-model-id-transform"
import { abortWithTimeout } from "./abort-with-timeout"
export async function tryFallbackRetry(args: {
task: BackgroundTask
@@ -123,7 +124,7 @@ export async function tryFallbackRetry(args: {
}
if (previousSessionID) {
await client.session.abort({ path: { id: previousSessionID } }).catch(() => {})
await abortWithTimeout(client, previousSessionID).catch(() => {})
}
queue.push({ task, input: retryInput })
@@ -238,7 +238,7 @@ function stubNotifyParentSession(manager: BackgroundManager): void {
}
async function flushBackgroundNotifications(): Promise<void> {
for (let i = 0; i < 6; i++) {
for (let i = 0; i < 12; i++) {
await Promise.resolve()
}
}
@@ -2570,7 +2570,7 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
abortCalled,
new Promise<never>((_, reject) => setTimeout(() => reject(new Error("timeout")), 100)),
])
await Promise.resolve()
await flushBackgroundNotifications()
// then
const updatedTask = manager.getTask(task.id)
+3 -6
View File
@@ -57,6 +57,7 @@ import { join } from "node:path"
import { pruneStaleTasksAndNotifications } from "./task-poller"
import { checkAndInterruptStaleTasks } from "./task-poller"
import { removeTaskToastTracking } from "./remove-task-toast-tracking"
import { abortWithTimeout } from "./abort-with-timeout"
import {
MIN_SESSION_GONE_POLLS,
verifySessionExists as verifySessionStillExists,
@@ -193,9 +194,7 @@ export class BackgroundManager {
private async abortSessionWithLogging(sessionID: string, reason: string): Promise<void> {
try {
await this.client.session.abort({
path: { id: sessionID },
})
await abortWithTimeout(this.client, sessionID)
} catch (error) {
log(`[background-agent] Failed to abort session during ${reason}:`, {
sessionID,
@@ -1985,9 +1984,7 @@ export class BackgroundManager {
if (task.status === "running" && task.sessionID) {
abortRequests.push({
sessionID: task.sessionID,
promise: this.client.session.abort({
path: { id: task.sessionID },
}),
promise: abortWithTimeout(this.client, task.sessionID),
})
}
}
+3 -2
View File
@@ -13,6 +13,7 @@ import {
TERMINAL_TASK_TTL_MS,
TASK_TTL_MS,
} from "./constants"
import { abortWithTimeout } from "./abort-with-timeout"
import { removeTaskToastTracking } from "./remove-task-toast-tracking"
import { MIN_SESSION_GONE_POLLS, verifySessionExists } from "./session-existence"
@@ -167,7 +168,7 @@ export async function checkAndInterruptStaleTasks(args: {
onTaskInterrupted(task)
abortPromises.push(client.session.abort({ path: { id: sessionID } }))
abortPromises.push(abortWithTimeout(client, sessionID))
log(`[background-agent] Task ${task.id} interrupted: no progress since start`)
try {
@@ -205,7 +206,7 @@ export async function checkAndInterruptStaleTasks(args: {
onTaskInterrupted(task)
abortPromises.push(client.session.abort({ path: { id: sessionID } }))
abortPromises.push(abortWithTimeout(client, sessionID))
log(`[background-agent] Task ${task.id} interrupted: stale timeout`)
try {
-1
View File
@@ -2,4 +2,3 @@ export * from "./types"
export * from "./constants"
export * from "./storage"
export * from "./top-level-task"
export * from "./worktree-sync"
@@ -1,88 +0,0 @@
import { describe, expect, test, beforeEach, afterEach } from "bun:test"
import { existsSync, mkdirSync, rmSync, writeFileSync, readFileSync } from "node:fs"
import { join } from "node:path"
import { tmpdir } from "node:os"
import { syncSisyphusStateFromWorktree } from "./worktree-sync"
describe("syncSisyphusStateFromWorktree", () => {
const BASE = join(tmpdir(), "worktree-sync-test-" + Date.now())
const WORKTREE = join(BASE, "worktree")
const MAIN_REPO = join(BASE, "main")
beforeEach(() => {
mkdirSync(WORKTREE, { recursive: true })
mkdirSync(MAIN_REPO, { recursive: true })
})
afterEach(() => {
if (existsSync(BASE)) {
rmSync(BASE, { recursive: true, force: true })
}
})
test("#given no .sisyphus in worktree #when syncing #then returns true without error", () => {
const result = syncSisyphusStateFromWorktree(WORKTREE, MAIN_REPO)
expect(result).toBe(true)
expect(existsSync(join(MAIN_REPO, ".sisyphus"))).toBe(false)
})
test("#given .sisyphus with boulder.json in worktree #when syncing #then copies to main repo", () => {
const worktreeSisyphus = join(WORKTREE, ".sisyphus")
mkdirSync(worktreeSisyphus, { recursive: true })
writeFileSync(join(worktreeSisyphus, "boulder.json"), '{"active_plan":"/plan.md","plan_name":"test"}')
const result = syncSisyphusStateFromWorktree(WORKTREE, MAIN_REPO)
expect(result).toBe(true)
const copied = readFileSync(join(MAIN_REPO, ".sisyphus", "boulder.json"), "utf-8")
expect(JSON.parse(copied).plan_name).toBe("test")
})
test("#given nested .sisyphus dirs in worktree #when syncing #then copies full tree recursively", () => {
const worktreePlans = join(WORKTREE, ".sisyphus", "plans")
const worktreeNotepads = join(WORKTREE, ".sisyphus", "notepads", "my-plan")
mkdirSync(worktreePlans, { recursive: true })
mkdirSync(worktreeNotepads, { recursive: true })
writeFileSync(join(worktreePlans, "my-plan.md"), "- [x] Task 1\n- [ ] Task 2")
writeFileSync(join(worktreeNotepads, "learnings.md"), "learned something")
const result = syncSisyphusStateFromWorktree(WORKTREE, MAIN_REPO)
expect(result).toBe(true)
expect(readFileSync(join(MAIN_REPO, ".sisyphus", "plans", "my-plan.md"), "utf-8")).toContain("Task 1")
expect(readFileSync(join(MAIN_REPO, ".sisyphus", "notepads", "my-plan", "learnings.md"), "utf-8")).toBe("learned something")
})
test("#given existing .sisyphus in main repo #when syncing #then worktree state overwrites stale state", () => {
const mainSisyphus = join(MAIN_REPO, ".sisyphus")
mkdirSync(mainSisyphus, { recursive: true })
writeFileSync(join(mainSisyphus, "boulder.json"), '{"plan_name":"old"}')
const worktreeSisyphus = join(WORKTREE, ".sisyphus")
mkdirSync(worktreeSisyphus, { recursive: true })
writeFileSync(join(worktreeSisyphus, "boulder.json"), '{"plan_name":"updated"}')
const result = syncSisyphusStateFromWorktree(WORKTREE, MAIN_REPO)
expect(result).toBe(true)
const content = readFileSync(join(mainSisyphus, "boulder.json"), "utf-8")
expect(JSON.parse(content).plan_name).toBe("updated")
})
test("#given pre-existing files in main .sisyphus #when syncing #then preserves files not in worktree", () => {
const mainSisyphus = join(MAIN_REPO, ".sisyphus", "rules")
mkdirSync(mainSisyphus, { recursive: true })
writeFileSync(join(mainSisyphus, "my-rule.md"), "existing rule")
const worktreeSisyphus = join(WORKTREE, ".sisyphus")
mkdirSync(worktreeSisyphus, { recursive: true })
writeFileSync(join(worktreeSisyphus, "boulder.json"), '{"plan_name":"new"}')
const result = syncSisyphusStateFromWorktree(WORKTREE, MAIN_REPO)
expect(result).toBe(true)
expect(readFileSync(join(MAIN_REPO, ".sisyphus", "rules", "my-rule.md"), "utf-8")).toBe("existing rule")
expect(existsSync(join(MAIN_REPO, ".sisyphus", "boulder.json"))).toBe(true)
})
})
@@ -1,34 +0,0 @@
import { existsSync, cpSync, mkdirSync } from "node:fs"
import { join } from "node:path"
import { BOULDER_DIR } from "./constants"
import { log } from "../../shared/logger"
export function syncSisyphusStateFromWorktree(worktreePath: string, mainRepoPath: string): boolean {
const srcDir = join(worktreePath, BOULDER_DIR)
const destDir = join(mainRepoPath, BOULDER_DIR)
if (!existsSync(srcDir)) {
log("[worktree-sync] No .sisyphus directory in worktree, nothing to sync", { worktreePath })
return true
}
try {
if (!existsSync(destDir)) {
mkdirSync(destDir, { recursive: true })
}
cpSync(srcDir, destDir, { recursive: true, force: true })
log("[worktree-sync] Synced .sisyphus state from worktree to main repo", {
worktreePath,
mainRepoPath,
})
return true
} catch (err) {
log("[worktree-sync] Failed to sync .sisyphus state", {
worktreePath,
mainRepoPath,
error: String(err),
})
return false
}
}