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
This commit is contained in:
parent
1b7fd32bad
commit
49c933961e
@ -54,95 +54,95 @@ For each commit, you MUST:
|
|||||||
### feat
|
### feat
|
||||||
| Scope | What Changed |
|
| Scope | What Changed |
|
||||||
|-------|--------------|
|
|-------|--------------|
|
||||||
| X | 실제 변경 내용 설명 |
|
| X | Description of actual changes |
|
||||||
|
|
||||||
### fix
|
### fix
|
||||||
| Scope | What Changed |
|
| Scope | What Changed |
|
||||||
|-------|--------------|
|
|-------|--------------|
|
||||||
| X | 실제 변경 내용 설명 |
|
| X | Description of actual changes |
|
||||||
|
|
||||||
### refactor
|
### refactor
|
||||||
| Scope | What Changed |
|
| Scope | What Changed |
|
||||||
|-------|--------------|
|
|-------|--------------|
|
||||||
| X | 실제 변경 내용 설명 |
|
| X | Description of actual changes |
|
||||||
|
|
||||||
### docs
|
### docs
|
||||||
| Scope | What Changed |
|
| Scope | What Changed |
|
||||||
|-------|--------------|
|
|-------|--------------|
|
||||||
| X | 실제 변경 내용 설명 |
|
| X | Description of actual changes |
|
||||||
|
|
||||||
### Breaking Changes
|
### Breaking Changes
|
||||||
None 또는 목록
|
None or list
|
||||||
|
|
||||||
### Files Changed
|
### Files Changed
|
||||||
{diff-stat}
|
{diff-stat}
|
||||||
|
|
||||||
### Suggested Version Bump
|
### Suggested Version Bump
|
||||||
- **Recommendation**: patch|minor|major
|
- **Recommendation**: patch|minor|major
|
||||||
- **Reason**: 이유
|
- **Reason**: Reason for recommendation
|
||||||
</output-format>
|
</output-format>
|
||||||
|
|
||||||
<oracle-safety-review>
|
<oracle-safety-review>
|
||||||
## 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
|
```bash
|
||||||
bun run typecheck
|
bun run typecheck
|
||||||
bun test
|
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 (기능별로 정리)
|
### Key diffs (organized by feature)
|
||||||
{각 feat/fix/refactor의 핵심 코드 변경 - 전체 diff가 아닌 핵심만}
|
{Core code changes for each feat/fix/refactor - only key parts, not full diff}
|
||||||
|
|
||||||
### 검증 결과
|
### Validation Results
|
||||||
- Typecheck: ✅/❌
|
- Typecheck: ✅/❌
|
||||||
- Tests: {pass}/{total} (✅/❌)
|
- Tests: {pass}/{total} (✅/❌)
|
||||||
|
|
||||||
### 검토 요청사항
|
### Review Items
|
||||||
1. **리그레션 위험**: 기존 기능에 영향을 줄 수 있는 변경이 있는가?
|
1. **Regression Risk**: Are there changes that could affect existing functionality?
|
||||||
2. **사이드이펙트**: 예상치 못한 부작용이 발생할 수 있는 부분은?
|
2. **Side Effects**: Are there areas where unexpected side effects could occur?
|
||||||
3. **Breaking Changes**: 외부 사용자에게 영향을 주는 변경이 있는가?
|
3. **Breaking Changes**: Are there changes that affect external users?
|
||||||
4. **Edge Cases**: 놓친 엣지 케이스가 있는가?
|
4. **Edge Cases**: Are there missed edge cases?
|
||||||
5. **배포 권장 여부**: SAFE / CAUTION / UNSAFE
|
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
|
||||||
- ...
|
- ...
|
||||||
|
|
||||||
### 결론
|
### Conclusion
|
||||||
{Oracle의 최종 판단}
|
{Oracle's final judgment}
|
||||||
</oracle-safety-review>
|
</oracle-safety-review>
|
||||||
|
|||||||
@ -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)
|
- `major`: Breaking changes (1.1.7 → 2.0.0)
|
||||||
|
|
||||||
**If the user did not provide a bump type argument, STOP IMMEDIATELY and ask:**
|
**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.**
|
**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
|
## STEP 1: CONFIRM BUMP TYPE
|
||||||
|
|
||||||
If bump type provided as argument, confirm with user:
|
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.
|
Wait for user confirmation before proceeding.
|
||||||
|
|
||||||
@ -293,7 +293,7 @@ Report success to user with:
|
|||||||
|
|
||||||
## LANGUAGE
|
## LANGUAGE
|
||||||
|
|
||||||
Respond to user in Korean (한국어).
|
Respond to user in English.
|
||||||
|
|
||||||
</command-instruction>
|
</command-instruction>
|
||||||
|
|
||||||
|
|||||||
@ -832,7 +832,7 @@ export class BackgroundManager {
|
|||||||
|
|
||||||
async cancelTask(
|
async cancelTask(
|
||||||
taskId: string,
|
taskId: string,
|
||||||
options?: { source?: string; reason?: string; abortSession?: boolean }
|
options?: { source?: string; reason?: string; abortSession?: boolean; skipNotification?: boolean }
|
||||||
): Promise<boolean> {
|
): Promise<boolean> {
|
||||||
const task = this.tasks.get(taskId)
|
const task = this.tasks.get(taskId)
|
||||||
if (!task || (task.status !== "running" && task.status !== "pending")) {
|
if (!task || (task.status !== "running" && task.status !== "pending")) {
|
||||||
@ -878,7 +878,6 @@ export class BackgroundManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
this.cleanupPendingByParent(task)
|
this.cleanupPendingByParent(task)
|
||||||
this.markForNotification(task)
|
|
||||||
|
|
||||||
if (abortSession && task.sessionID) {
|
if (abortSession && task.sessionID) {
|
||||||
this.client.session.abort({
|
this.client.session.abort({
|
||||||
@ -886,6 +885,13 @@ export class BackgroundManager {
|
|||||||
}).catch(() => {})
|
}).catch(() => {})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (options?.skipNotification) {
|
||||||
|
log(`[background-agent] Task cancelled via ${source} (notification skipped):`, task.id)
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
this.markForNotification(task)
|
||||||
|
|
||||||
try {
|
try {
|
||||||
await this.notifyParentSession(task)
|
await this.notifyParentSession(task)
|
||||||
log(`[background-agent] Task cancelled via ${source}:`, task.id)
|
log(`[background-agent] Task cancelled via ${source}:`, task.id)
|
||||||
|
|||||||
@ -146,14 +146,14 @@ export function createContextInjectorMessagesTransformHook(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// synthetic part 패턴 (minimal fields)
|
// synthetic part pattern (minimal fields)
|
||||||
const syntheticPart = {
|
const syntheticPart = {
|
||||||
id: `synthetic_hook_${Date.now()}`,
|
id: `synthetic_hook_${Date.now()}`,
|
||||||
messageID: lastUserMessage.info.id,
|
messageID: lastUserMessage.info.id,
|
||||||
sessionID: (lastUserMessage.info as { sessionID?: string }).sessionID ?? "",
|
sessionID: (lastUserMessage.info as { sessionID?: string }).sessionID ?? "",
|
||||||
type: "text" as const,
|
type: "text" as const,
|
||||||
text: pending.merged,
|
text: pending.merged,
|
||||||
synthetic: true, // UI에서 숨겨짐
|
synthetic: true, // hidden in UI
|
||||||
}
|
}
|
||||||
|
|
||||||
lastUserMessage.parts.splice(textPartIndex, 0, syntheticPart as Part)
|
lastUserMessage.parts.splice(textPartIndex, 0, syntheticPart as Part)
|
||||||
|
|||||||
@ -328,6 +328,58 @@ describe("background_cancel", () => {
|
|||||||
expect(output).toContain("| `task-a` | running task | running | `ses-a` |")
|
expect(output).toContain("| `task-a` | running task | running | `ses-a` |")
|
||||||
expect(output).toContain("| `task-b` | pending task | pending | (not started) |")
|
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 = {
|
type BackgroundOutputMessage = {
|
||||||
id?: string
|
id?: string
|
||||||
|
|||||||
@ -642,6 +642,7 @@ export function createBackgroundCancel(manager: BackgroundManager, client: Backg
|
|||||||
const cancelled = await manager.cancelTask(task.id, {
|
const cancelled = await manager.cancelTask(task.id, {
|
||||||
source: "background_cancel",
|
source: "background_cancel",
|
||||||
abortSession: originalStatus === "running",
|
abortSession: originalStatus === "running",
|
||||||
|
skipNotification: true,
|
||||||
})
|
})
|
||||||
if (!cancelled) continue
|
if (!cancelled) continue
|
||||||
cancelledInfo.push({
|
cancelledInfo.push({
|
||||||
@ -690,6 +691,7 @@ Only running or pending tasks can be cancelled.`
|
|||||||
const cancelled = await manager.cancelTask(task.id, {
|
const cancelled = await manager.cancelTask(task.id, {
|
||||||
source: "background_cancel",
|
source: "background_cancel",
|
||||||
abortSession: task.status === "running",
|
abortSession: task.status === "running",
|
||||||
|
skipNotification: true,
|
||||||
})
|
})
|
||||||
if (!cancelled) {
|
if (!cancelled) {
|
||||||
return `[ERROR] Failed to cancel task: ${task.id}`
|
return `[ERROR] Failed to cancel task: ${task.id}`
|
||||||
|
|||||||
@ -4,9 +4,9 @@ import { normalizeArgs, validateArgs, createLookAt } from "./tools"
|
|||||||
|
|
||||||
describe("look-at tool", () => {
|
describe("look-at tool", () => {
|
||||||
describe("normalizeArgs", () => {
|
describe("normalizeArgs", () => {
|
||||||
// given LLM이 file_path 대신 path를 사용할 수 있음
|
// given LLM might use `path` instead of `file_path`
|
||||||
// when path 파라미터로 호출
|
// when called with path parameter
|
||||||
// then file_path로 정규화되어야 함
|
// then should normalize to file_path
|
||||||
test("normalizes path to file_path for LLM compatibility", () => {
|
test("normalizes path to file_path for LLM compatibility", () => {
|
||||||
const args = { path: "/some/file.png", goal: "analyze" }
|
const args = { path: "/some/file.png", goal: "analyze" }
|
||||||
const normalized = normalizeArgs(args as any)
|
const normalized = normalizeArgs(args as any)
|
||||||
@ -14,18 +14,18 @@ describe("look-at tool", () => {
|
|||||||
expect(normalized.goal).toBe("analyze")
|
expect(normalized.goal).toBe("analyze")
|
||||||
})
|
})
|
||||||
|
|
||||||
// given 정상적인 file_path 사용
|
// given proper file_path usage
|
||||||
// when file_path 파라미터로 호출
|
// when called with file_path parameter
|
||||||
// then 그대로 유지
|
// then keep as-is
|
||||||
test("keeps file_path when properly provided", () => {
|
test("keeps file_path when properly provided", () => {
|
||||||
const args = { file_path: "/correct/path.pdf", goal: "extract" }
|
const args = { file_path: "/correct/path.pdf", goal: "extract" }
|
||||||
const normalized = normalizeArgs(args)
|
const normalized = normalizeArgs(args)
|
||||||
expect(normalized.file_path).toBe("/correct/path.pdf")
|
expect(normalized.file_path).toBe("/correct/path.pdf")
|
||||||
})
|
})
|
||||||
|
|
||||||
// given 둘 다 제공된 경우
|
// given both parameters provided
|
||||||
// when file_path와 path 모두 있음
|
// when file_path and path are both present
|
||||||
// then file_path 우선
|
// then prefer file_path
|
||||||
test("prefers file_path over path when both provided", () => {
|
test("prefers file_path over path when both provided", () => {
|
||||||
const args = { file_path: "/preferred.png", path: "/fallback.png", goal: "test" }
|
const args = { file_path: "/preferred.png", path: "/fallback.png", goal: "test" }
|
||||||
const normalized = normalizeArgs(args as any)
|
const normalized = normalizeArgs(args as any)
|
||||||
@ -34,17 +34,17 @@ describe("look-at tool", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("validateArgs", () => {
|
describe("validateArgs", () => {
|
||||||
// given 유효한 인자
|
// given valid arguments
|
||||||
// when 검증
|
// when validated
|
||||||
// then null 반환 (에러 없음)
|
// then return null (no error)
|
||||||
test("returns null for valid args", () => {
|
test("returns null for valid args", () => {
|
||||||
const args = { file_path: "/valid/path.png", goal: "analyze" }
|
const args = { file_path: "/valid/path.png", goal: "analyze" }
|
||||||
expect(validateArgs(args)).toBeNull()
|
expect(validateArgs(args)).toBeNull()
|
||||||
})
|
})
|
||||||
|
|
||||||
// given file_path 누락
|
// given file_path missing
|
||||||
// when 검증
|
// when validated
|
||||||
// then 명확한 에러 메시지
|
// then clear error message
|
||||||
test("returns error when file_path is missing", () => {
|
test("returns error when file_path is missing", () => {
|
||||||
const args = { goal: "analyze" } as any
|
const args = { goal: "analyze" } as any
|
||||||
const error = validateArgs(args)
|
const error = validateArgs(args)
|
||||||
@ -52,9 +52,9 @@ describe("look-at tool", () => {
|
|||||||
expect(error).toContain("required")
|
expect(error).toContain("required")
|
||||||
})
|
})
|
||||||
|
|
||||||
// given goal 누락
|
// given goal missing
|
||||||
// when 검증
|
// when validated
|
||||||
// then 명확한 에러 메시지
|
// then clear error message
|
||||||
test("returns error when goal is missing", () => {
|
test("returns error when goal is missing", () => {
|
||||||
const args = { file_path: "/some/path.png" } as any
|
const args = { file_path: "/some/path.png" } as any
|
||||||
const error = validateArgs(args)
|
const error = validateArgs(args)
|
||||||
@ -62,9 +62,9 @@ describe("look-at tool", () => {
|
|||||||
expect(error).toContain("required")
|
expect(error).toContain("required")
|
||||||
})
|
})
|
||||||
|
|
||||||
// given file_path가 빈 문자열
|
// given file_path is empty string
|
||||||
// when 검증
|
// when validated
|
||||||
// then 에러 반환
|
// then return error
|
||||||
test("returns error when file_path is empty string", () => {
|
test("returns error when file_path is empty string", () => {
|
||||||
const args = { file_path: "", goal: "analyze" }
|
const args = { file_path: "", goal: "analyze" }
|
||||||
const error = validateArgs(args)
|
const error = validateArgs(args)
|
||||||
@ -73,9 +73,9 @@ describe("look-at tool", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("createLookAt error handling", () => {
|
describe("createLookAt error handling", () => {
|
||||||
// given session.prompt에서 JSON parse 에러 발생
|
// given JSON parse error occurs in session.prompt
|
||||||
// when LookAt 도구 실행
|
// when LookAt tool executed
|
||||||
// then 사용자 친화적 에러 메시지 반환
|
// then return user-friendly error message
|
||||||
test("handles JSON parse error from session.prompt gracefully", async () => {
|
test("handles JSON parse error from session.prompt gracefully", async () => {
|
||||||
const mockClient = {
|
const mockClient = {
|
||||||
session: {
|
session: {
|
||||||
@ -115,9 +115,9 @@ describe("look-at tool", () => {
|
|||||||
expect(result).toContain("image/png")
|
expect(result).toContain("image/png")
|
||||||
})
|
})
|
||||||
|
|
||||||
// given session.prompt에서 일반 에러 발생
|
// given generic error occurs in session.prompt
|
||||||
// when LookAt 도구 실행
|
// when LookAt tool executed
|
||||||
// then 원본 에러 메시지 포함한 에러 반환
|
// then return error including original error message
|
||||||
test("handles generic prompt error gracefully", async () => {
|
test("handles generic prompt error gracefully", async () => {
|
||||||
const mockClient = {
|
const mockClient = {
|
||||||
session: {
|
session: {
|
||||||
@ -158,8 +158,8 @@ describe("look-at tool", () => {
|
|||||||
|
|
||||||
describe("createLookAt model passthrough", () => {
|
describe("createLookAt model passthrough", () => {
|
||||||
// given multimodal-looker agent has resolved model info
|
// given multimodal-looker agent has resolved model info
|
||||||
// when LookAt 도구 실행
|
// when LookAt tool executed
|
||||||
// then session.prompt에 model 정보가 전달되어야 함
|
// then model info should be passed to session.prompt
|
||||||
test("passes multimodal-looker model to session.prompt when available", async () => {
|
test("passes multimodal-looker model to session.prompt when available", async () => {
|
||||||
let promptBody: any
|
let promptBody: any
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user