From 49c933961e04667047518b569c0e2d80e4b78e6e Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 3 Feb 2026 16:56:40 +0900 Subject: [PATCH] fix(background-cancel): skip notification when user explicitly cancels tasks - Add skipNotification option to cancelTask method - Apply skipNotification to background_cancel tool - Prevents unwanted notifications when user cancels via tool --- .opencode/command/get-unpublished-changes.md | 78 ++++++++++---------- .opencode/command/publish.md | 6 +- src/features/background-agent/manager.ts | 10 ++- src/features/context-injector/injector.ts | 4 +- src/tools/background-task/tools.test.ts | 52 +++++++++++++ src/tools/background-task/tools.ts | 2 + src/tools/look-at/tools.test.ts | 58 +++++++-------- 7 files changed, 135 insertions(+), 75 deletions(-) diff --git a/.opencode/command/get-unpublished-changes.md b/.opencode/command/get-unpublished-changes.md index 6dd6fc49..8c2731b7 100644 --- a/.opencode/command/get-unpublished-changes.md +++ b/.opencode/command/get-unpublished-changes.md @@ -54,95 +54,95 @@ For each commit, you MUST: ### feat | Scope | What Changed | |-------|--------------| -| X | 실제 변경 내용 설명 | +| X | Description of actual changes | ### fix | Scope | What Changed | |-------|--------------| -| X | 실제 변경 내용 설명 | +| X | Description of actual changes | ### refactor | Scope | What Changed | |-------|--------------| -| X | 실제 변경 내용 설명 | +| X | Description of actual changes | ### docs | Scope | What Changed | |-------|--------------| -| X | 실제 변경 내용 설명 | +| X | Description of actual changes | ### Breaking Changes -None 또는 목록 +None or list ### Files Changed {diff-stat} ### Suggested Version Bump - **Recommendation**: patch|minor|major -- **Reason**: 이유 +- **Reason**: Reason for recommendation -## Oracle 배포 안전성 검토 (사용자가 명시적으로 요청 시에만) +## Oracle Deployment Safety Review (Only when user explicitly requests) -**트리거 키워드**: "배포 가능", "배포해도 될까", "안전한지", "리뷰", "검토", "oracle", "오라클" +**Trigger keywords**: "safe to deploy", "can I deploy", "is it safe", "review", "check", "oracle" -사용자가 위 키워드 중 하나라도 포함하여 요청하면: +When user includes any of the above keywords in their request: -### 1. 사전 검증 실행 +### 1. Pre-validation ```bash bun run typecheck bun test ``` -- 실패 시 → Oracle 소환 없이 즉시 "❌ 배포 불가" 보고 +- On failure → Report "❌ Cannot deploy" immediately without invoking Oracle -### 2. Oracle 소환 프롬프트 +### 2. Oracle Invocation Prompt -다음 정보를 수집하여 Oracle에게 전달: +Collect the following information and pass to Oracle: ``` -## 배포 안전성 검토 요청 +## Deployment Safety Review Request -### 변경사항 요약 -{위에서 분석한 변경사항 테이블} +### Changes Summary +{Changes table analyzed above} -### 주요 diff (기능별로 정리) -{각 feat/fix/refactor의 핵심 코드 변경 - 전체 diff가 아닌 핵심만} +### Key diffs (organized by feature) +{Core code changes for each feat/fix/refactor - only key parts, not full diff} -### 검증 결과 +### Validation Results - Typecheck: ✅/❌ - Tests: {pass}/{total} (✅/❌) -### 검토 요청사항 -1. **리그레션 위험**: 기존 기능에 영향을 줄 수 있는 변경이 있는가? -2. **사이드이펙트**: 예상치 못한 부작용이 발생할 수 있는 부분은? -3. **Breaking Changes**: 외부 사용자에게 영향을 주는 변경이 있는가? -4. **Edge Cases**: 놓친 엣지 케이스가 있는가? -5. **배포 권장 여부**: SAFE / CAUTION / UNSAFE +### Review Items +1. **Regression Risk**: Are there changes that could affect existing functionality? +2. **Side Effects**: Are there areas where unexpected side effects could occur? +3. **Breaking Changes**: Are there changes that affect external users? +4. **Edge Cases**: Are there missed edge cases? +5. **Deployment Recommendation**: SAFE / CAUTION / UNSAFE -### 요청 -위 변경사항을 깊이 분석하고, 배포 안전성에 대해 판단해주세요. -리스크가 있다면 구체적인 시나리오와 함께 설명해주세요. -배포 후 모니터링해야 할 키워드가 있다면 제안해주세요. +### Request +Please analyze the above changes deeply and provide your judgment on deployment safety. +If there are risks, explain with specific scenarios. +Suggest keywords to monitor after deployment if any. ``` -### 3. Oracle 응답 후 출력 포맷 +### 3. Output Format After Oracle Response -## 🔍 Oracle 배포 안전성 검토 결과 +## 🔍 Oracle Deployment Safety Review Result -### 판정: ✅ SAFE / ⚠️ CAUTION / ❌ UNSAFE +### Verdict: ✅ SAFE / ⚠️ CAUTION / ❌ UNSAFE -### 리스크 분석 -| 영역 | 리스크 레벨 | 설명 | -|------|-------------|------| +### Risk Analysis +| Area | Risk Level | Description | +|------|------------|-------------| | ... | 🟢/🟡/🔴 | ... | -### 권장 사항 +### Recommendations - ... -### 배포 후 모니터링 키워드 +### Post-deployment Monitoring Keywords - ... -### 결론 -{Oracle의 최종 판단} +### Conclusion +{Oracle's final judgment} diff --git a/.opencode/command/publish.md b/.opencode/command/publish.md index aad56059..945ec539 100644 --- a/.opencode/command/publish.md +++ b/.opencode/command/publish.md @@ -14,7 +14,7 @@ You are the release manager for oh-my-opencode. Execute the FULL publish workflo - `major`: Breaking changes (1.1.7 → 2.0.0) **If the user did not provide a bump type argument, STOP IMMEDIATELY and ask:** -> "배포를 진행하려면 버전 범프 타입을 지정해주세요: `patch`, `minor`, 또는 `major`" +> "To proceed with deployment, please specify a version bump type: `patch`, `minor`, or `major`" **DO NOT PROCEED without explicit user confirmation of bump type.** @@ -48,7 +48,7 @@ You are the release manager for oh-my-opencode. Execute the FULL publish workflo ## STEP 1: CONFIRM BUMP TYPE If bump type provided as argument, confirm with user: -> "버전 범프 타입: `{bump}`. 진행할까요? (y/n)" +> "Version bump type: `{bump}`. Proceed? (y/n)" Wait for user confirmation before proceeding. @@ -293,7 +293,7 @@ Report success to user with: ## LANGUAGE -Respond to user in Korean (한국어). +Respond to user in English. diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index 89b09860..7a0b30a7 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -832,7 +832,7 @@ export class BackgroundManager { async cancelTask( taskId: string, - options?: { source?: string; reason?: string; abortSession?: boolean } + options?: { source?: string; reason?: string; abortSession?: boolean; skipNotification?: boolean } ): Promise { const task = this.tasks.get(taskId) if (!task || (task.status !== "running" && task.status !== "pending")) { @@ -878,7 +878,6 @@ export class BackgroundManager { } this.cleanupPendingByParent(task) - this.markForNotification(task) if (abortSession && task.sessionID) { this.client.session.abort({ @@ -886,6 +885,13 @@ export class BackgroundManager { }).catch(() => {}) } + if (options?.skipNotification) { + log(`[background-agent] Task cancelled via ${source} (notification skipped):`, task.id) + return true + } + + this.markForNotification(task) + try { await this.notifyParentSession(task) log(`[background-agent] Task cancelled via ${source}:`, task.id) diff --git a/src/features/context-injector/injector.ts b/src/features/context-injector/injector.ts index 3a6ba27c..ca676a11 100644 --- a/src/features/context-injector/injector.ts +++ b/src/features/context-injector/injector.ts @@ -146,14 +146,14 @@ export function createContextInjectorMessagesTransformHook( return } - // synthetic part 패턴 (minimal fields) + // synthetic part pattern (minimal fields) const syntheticPart = { id: `synthetic_hook_${Date.now()}`, messageID: lastUserMessage.info.id, sessionID: (lastUserMessage.info as { sessionID?: string }).sessionID ?? "", type: "text" as const, text: pending.merged, - synthetic: true, // UI에서 숨겨짐 + synthetic: true, // hidden in UI } lastUserMessage.parts.splice(textPartIndex, 0, syntheticPart as Part) diff --git a/src/tools/background-task/tools.test.ts b/src/tools/background-task/tools.test.ts index 94e3cef3..6c3a2099 100644 --- a/src/tools/background-task/tools.test.ts +++ b/src/tools/background-task/tools.test.ts @@ -328,6 +328,58 @@ describe("background_cancel", () => { expect(output).toContain("| `task-a` | running task | running | `ses-a` |") expect(output).toContain("| `task-b` | pending task | pending | (not started) |") }) + + test("passes skipNotification: true to cancelTask to prevent deadlock", async () => { + // #given + const task = createTask({ id: "task-1", status: "running" }) + const cancelOptions: Array<{ taskId: string; options: unknown }> = [] + const manager = { + getTask: (id: string) => (id === task.id ? task : undefined), + getAllDescendantTasks: () => [task], + cancelTask: async (taskId: string, options?: unknown) => { + cancelOptions.push({ taskId, options }) + task.status = "cancelled" + return true + }, + } as unknown as BackgroundManager + const client = { session: { abort: async () => ({}) } } as BackgroundCancelClient + const tool = createBackgroundCancel(manager, client) + + // #when - cancel all tasks + await tool.execute({ all: true }, mockContext) + + // #then - skipNotification should be true to prevent self-deadlock + expect(cancelOptions).toHaveLength(1) + expect(cancelOptions[0].options).toEqual( + expect.objectContaining({ skipNotification: true }) + ) + }) + + test("passes skipNotification: true when cancelling single task", async () => { + // #given + const task = createTask({ id: "task-1", status: "running" }) + const cancelOptions: Array<{ taskId: string; options: unknown }> = [] + const manager = { + getTask: (id: string) => (id === task.id ? task : undefined), + getAllDescendantTasks: () => [task], + cancelTask: async (taskId: string, options?: unknown) => { + cancelOptions.push({ taskId, options }) + task.status = "cancelled" + return true + }, + } as unknown as BackgroundManager + const client = { session: { abort: async () => ({}) } } as BackgroundCancelClient + const tool = createBackgroundCancel(manager, client) + + // #when - cancel single task + await tool.execute({ taskId: task.id }, mockContext) + + // #then - skipNotification should be true + expect(cancelOptions).toHaveLength(1) + expect(cancelOptions[0].options).toEqual( + expect.objectContaining({ skipNotification: true }) + ) + }) }) type BackgroundOutputMessage = { id?: string diff --git a/src/tools/background-task/tools.ts b/src/tools/background-task/tools.ts index 01aa6f5a..8d25282d 100644 --- a/src/tools/background-task/tools.ts +++ b/src/tools/background-task/tools.ts @@ -642,6 +642,7 @@ export function createBackgroundCancel(manager: BackgroundManager, client: Backg const cancelled = await manager.cancelTask(task.id, { source: "background_cancel", abortSession: originalStatus === "running", + skipNotification: true, }) if (!cancelled) continue cancelledInfo.push({ @@ -690,6 +691,7 @@ Only running or pending tasks can be cancelled.` const cancelled = await manager.cancelTask(task.id, { source: "background_cancel", abortSession: task.status === "running", + skipNotification: true, }) if (!cancelled) { return `[ERROR] Failed to cancel task: ${task.id}` diff --git a/src/tools/look-at/tools.test.ts b/src/tools/look-at/tools.test.ts index 033cc503..ee6fcc38 100644 --- a/src/tools/look-at/tools.test.ts +++ b/src/tools/look-at/tools.test.ts @@ -4,9 +4,9 @@ import { normalizeArgs, validateArgs, createLookAt } from "./tools" describe("look-at tool", () => { describe("normalizeArgs", () => { - // given LLM이 file_path 대신 path를 사용할 수 있음 - // when path 파라미터로 호출 - // then file_path로 정규화되어야 함 + // given LLM might use `path` instead of `file_path` + // when called with path parameter + // then should normalize to file_path test("normalizes path to file_path for LLM compatibility", () => { const args = { path: "/some/file.png", goal: "analyze" } const normalized = normalizeArgs(args as any) @@ -14,18 +14,18 @@ describe("look-at tool", () => { expect(normalized.goal).toBe("analyze") }) - // given 정상적인 file_path 사용 - // when file_path 파라미터로 호출 - // then 그대로 유지 + // given proper file_path usage + // when called with file_path parameter + // then keep as-is test("keeps file_path when properly provided", () => { const args = { file_path: "/correct/path.pdf", goal: "extract" } const normalized = normalizeArgs(args) expect(normalized.file_path).toBe("/correct/path.pdf") }) - // given 둘 다 제공된 경우 - // when file_path와 path 모두 있음 - // then file_path 우선 + // given both parameters provided + // when file_path and path are both present + // then prefer file_path test("prefers file_path over path when both provided", () => { const args = { file_path: "/preferred.png", path: "/fallback.png", goal: "test" } const normalized = normalizeArgs(args as any) @@ -34,17 +34,17 @@ describe("look-at tool", () => { }) describe("validateArgs", () => { - // given 유효한 인자 - // when 검증 - // then null 반환 (에러 없음) + // given valid arguments + // when validated + // then return null (no error) test("returns null for valid args", () => { const args = { file_path: "/valid/path.png", goal: "analyze" } expect(validateArgs(args)).toBeNull() }) - // given file_path 누락 - // when 검증 - // then 명확한 에러 메시지 + // given file_path missing + // when validated + // then clear error message test("returns error when file_path is missing", () => { const args = { goal: "analyze" } as any const error = validateArgs(args) @@ -52,9 +52,9 @@ describe("look-at tool", () => { expect(error).toContain("required") }) - // given goal 누락 - // when 검증 - // then 명확한 에러 메시지 + // given goal missing + // when validated + // then clear error message test("returns error when goal is missing", () => { const args = { file_path: "/some/path.png" } as any const error = validateArgs(args) @@ -62,9 +62,9 @@ describe("look-at tool", () => { expect(error).toContain("required") }) - // given file_path가 빈 문자열 - // when 검증 - // then 에러 반환 + // given file_path is empty string + // when validated + // then return error test("returns error when file_path is empty string", () => { const args = { file_path: "", goal: "analyze" } const error = validateArgs(args) @@ -73,9 +73,9 @@ describe("look-at tool", () => { }) describe("createLookAt error handling", () => { - // given session.prompt에서 JSON parse 에러 발생 - // when LookAt 도구 실행 - // then 사용자 친화적 에러 메시지 반환 + // given JSON parse error occurs in session.prompt + // when LookAt tool executed + // then return user-friendly error message test("handles JSON parse error from session.prompt gracefully", async () => { const mockClient = { session: { @@ -115,9 +115,9 @@ describe("look-at tool", () => { expect(result).toContain("image/png") }) - // given session.prompt에서 일반 에러 발생 - // when LookAt 도구 실행 - // then 원본 에러 메시지 포함한 에러 반환 + // given generic error occurs in session.prompt + // when LookAt tool executed + // then return error including original error message test("handles generic prompt error gracefully", async () => { const mockClient = { session: { @@ -158,8 +158,8 @@ describe("look-at tool", () => { describe("createLookAt model passthrough", () => { // given multimodal-looker agent has resolved model info - // when LookAt 도구 실행 - // then session.prompt에 model 정보가 전달되어야 함 + // when LookAt tool executed + // then model info should be passed to session.prompt test("passes multimodal-looker model to session.prompt when available", async () => { let promptBody: any