Compare commits
11 Commits
fix/fallba
...
fix/google
| Author | SHA1 | Date | |
|---|---|---|---|
|
4c7b81986a | ||
|
|
fec75535ba | ||
|
|
e5a0ab4034 | ||
|
31dc65e9ac | ||
|
|
86cfa06aef | ||
|
|
3c2ccba62b | ||
|
|
e0f2952659 | ||
|
d556937c8e | ||
|
73d9e1f847 | ||
|
|
6d5d250f8f | ||
|
|
b6c433dae0 |
@@ -1,357 +0,0 @@
|
||||
# Issue #1501 분석 보고서: ULW Mode PLAN AGENT 무한루프
|
||||
|
||||
## 📋 이슈 요약
|
||||
|
||||
**증상:**
|
||||
- ULW (ultrawork) mode에서 PLAN AGENT가 무한루프에 빠짐
|
||||
- 분석/탐색 완료 후 plan만 계속 생성
|
||||
- 1분마다 매우 작은 토큰으로 요청 발생
|
||||
|
||||
**예상 동작:**
|
||||
- 탐색 완료 후 solution document 생성
|
||||
|
||||
---
|
||||
|
||||
## 🔍 근본 원인 분석
|
||||
|
||||
### 파일: `src/tools/delegate-task/constants.ts`
|
||||
|
||||
#### 문제의 핵심
|
||||
|
||||
`PLAN_AGENT_SYSTEM_PREPEND` (constants.ts 234-269행)에 구조적 결함이 있었습니다:
|
||||
|
||||
1. **Interactive Mode 가정**
|
||||
```
|
||||
2. After gathering context, ALWAYS present:
|
||||
- Uncertainties: List of unclear points
|
||||
- Clarifying Questions: Specific questions to resolve uncertainties
|
||||
|
||||
3. ITERATE until ALL requirements are crystal clear:
|
||||
- Do NOT proceed to planning until you have 100% clarity
|
||||
- Ask the user to confirm your understanding
|
||||
```
|
||||
|
||||
2. **종료 조건 없음**
|
||||
- "100% clarity" 요구는 객관적 측정 불가능
|
||||
- 사용자 확인 요청은 ULW mode에서 불가능
|
||||
- 무한루프로 이어짐
|
||||
|
||||
3. **ULW Mode 미감지**
|
||||
- Subagent로 실행되는 경우를 구분하지 않음
|
||||
- 항상 interactive mode로 동작 시도
|
||||
|
||||
### 왜 무한루프가 발생했는가?
|
||||
|
||||
```
|
||||
ULW Mode 시작
|
||||
→ Sisyphus가 Plan Agent 호출 (subagent)
|
||||
→ Plan Agent: "100% clarity 필요"
|
||||
→ Clarifying questions 생성
|
||||
→ 사용자 없음 (subagent)
|
||||
→ 다시 plan 생성 시도
|
||||
→ "여전히 unclear"
|
||||
→ 무한루프 반복
|
||||
```
|
||||
|
||||
**핵심:** Plan Agent는 사용자와 대화하도록 설계되었지만, ULW mode에서는 사용자가 없는 subagent로 실행됨.
|
||||
|
||||
---
|
||||
|
||||
## ✅ 적용된 수정 방안
|
||||
|
||||
### 수정 내용 (constants.ts)
|
||||
|
||||
#### 1. SUBAGENT MODE DETECTION 섹션 추가
|
||||
|
||||
```typescript
|
||||
SUBAGENT MODE DETECTION (CRITICAL):
|
||||
If you received a detailed prompt with gathered context from a parent orchestrator (e.g., Sisyphus):
|
||||
- You are running as a SUBAGENT
|
||||
- You CANNOT directly interact with the user
|
||||
- DO NOT ask clarifying questions - proceed with available information
|
||||
- Make reasonable assumptions for minor ambiguities
|
||||
- Generate the plan based on the provided context
|
||||
```
|
||||
|
||||
#### 2. Context Gathering Protocol 수정
|
||||
|
||||
```diff
|
||||
- 1. Launch background agents to gather context:
|
||||
+ 1. Launch background agents to gather context (ONLY if not already provided):
|
||||
```
|
||||
|
||||
**효과:** 이미 Sisyphus가 context를 수집한 경우 중복 방지
|
||||
|
||||
#### 3. Clarifying Questions → Assumptions
|
||||
|
||||
```diff
|
||||
- 2. After gathering context, ALWAYS present:
|
||||
- - Uncertainties: List of unclear points
|
||||
- - Clarifying Questions: Specific questions
|
||||
+ 2. After gathering context, assess clarity:
|
||||
+ - User Request Summary: Concise restatement
|
||||
+ - Assumptions Made: List any assumptions for unclear points
|
||||
```
|
||||
|
||||
**효과:** 질문 대신 가정 사항 문서화
|
||||
|
||||
#### 4. 무한루프 방지 - 명확한 종료 조건
|
||||
|
||||
```diff
|
||||
- 3. ITERATE until ALL requirements are crystal clear:
|
||||
- - Do NOT proceed to planning until you have 100% clarity
|
||||
- - Ask the user to confirm your understanding
|
||||
- - Resolve every ambiguity before generating the work plan
|
||||
+ 3. PROCEED TO PLAN GENERATION when:
|
||||
+ - Core objective is understood (even if some details are ambiguous)
|
||||
+ - You have gathered context via explore/librarian (or context was provided)
|
||||
+ - You can make reasonable assumptions for remaining ambiguities
|
||||
+
|
||||
+ DO NOT loop indefinitely waiting for perfect clarity.
|
||||
+ DOCUMENT assumptions in the plan so they can be validated during execution.
|
||||
```
|
||||
|
||||
**효과:**
|
||||
- "100% clarity" 요구 제거
|
||||
- 객관적인 진입 조건 제공
|
||||
- 무한루프 명시적 금지
|
||||
- Assumptions를 plan에 문서화하여 실행 중 검증 가능
|
||||
|
||||
#### 5. 철학 변경
|
||||
|
||||
```diff
|
||||
- REMEMBER: Vague requirements lead to failed implementations.
|
||||
+ REMEMBER: A plan with documented assumptions is better than no plan.
|
||||
```
|
||||
|
||||
**효과:** Perfectionism → Pragmatism
|
||||
|
||||
---
|
||||
|
||||
## 🎯 해결 메커니즘
|
||||
|
||||
### Before (무한루프)
|
||||
|
||||
```
|
||||
Plan Agent 시작
|
||||
↓
|
||||
Context gathering
|
||||
↓
|
||||
Requirements 명확한가?
|
||||
↓ NO
|
||||
Clarifying questions 생성
|
||||
↓
|
||||
사용자 응답 대기 (없음)
|
||||
↓
|
||||
다시 plan 시도
|
||||
↓
|
||||
(무한 반복)
|
||||
```
|
||||
|
||||
### After (정상 종료)
|
||||
|
||||
```
|
||||
Plan Agent 시작
|
||||
↓
|
||||
Subagent mode 감지?
|
||||
↓ YES
|
||||
Context 이미 있음? → YES
|
||||
↓
|
||||
Core objective 이해? → YES
|
||||
↓
|
||||
Reasonable assumptions 가능? → YES
|
||||
↓
|
||||
Plan 생성 (assumptions 문서화)
|
||||
↓
|
||||
완료 ✓
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 📊 영향 분석
|
||||
|
||||
### 해결되는 문제
|
||||
|
||||
1. **ULW mode 무한루프** ✓
|
||||
2. **Sisyphus에서 Plan Agent 호출 시 블로킹** ✓
|
||||
3. **작은 토큰 반복 요청** ✓
|
||||
4. **1분마다 재시도** ✓
|
||||
|
||||
### 부작용 없음
|
||||
|
||||
- Interactive mode (사용자와 직접 대화)는 여전히 작동
|
||||
- Subagent mode일 때만 다르게 동작
|
||||
- Backward compatibility 유지
|
||||
|
||||
### 추가 개선사항
|
||||
|
||||
- Assumptions를 plan에 명시적으로 문서화
|
||||
- Execution 중 validation 가능
|
||||
- 더 pragmatic한 workflow
|
||||
|
||||
---
|
||||
|
||||
## 🧪 검증 방법
|
||||
|
||||
### 테스트 시나리오
|
||||
|
||||
1. **ULW mode에서 Plan Agent 호출**
|
||||
```bash
|
||||
oh-my-opencode run "Complex task requiring planning. ulw"
|
||||
```
|
||||
- 예상: Plan 생성 후 정상 종료
|
||||
- 확인: 무한루프 없음
|
||||
|
||||
2. **Interactive mode (변경 없어야 함)**
|
||||
```bash
|
||||
oh-my-opencode run --agent prometheus "Design X"
|
||||
```
|
||||
- 예상: Clarifying questions 여전히 가능
|
||||
- 확인: 사용자와 대화 가능
|
||||
|
||||
3. **Subagent context 제공 케이스**
|
||||
- 예상: Context gathering skip
|
||||
- 확인: 중복 탐색 없음
|
||||
|
||||
---
|
||||
|
||||
## 📝 수정된 파일
|
||||
|
||||
```
|
||||
src/tools/delegate-task/constants.ts
|
||||
```
|
||||
|
||||
### Diff Summary
|
||||
|
||||
```diff
|
||||
@@ -234,22 +234,32 @@ export const PLAN_AGENT_SYSTEM_PREPEND = `<system>
|
||||
+SUBAGENT MODE DETECTION (CRITICAL):
|
||||
+[subagent 감지 및 처리 로직]
|
||||
+
|
||||
MANDATORY CONTEXT GATHERING PROTOCOL:
|
||||
-1. Launch background agents to gather context:
|
||||
+1. Launch background agents (ONLY if not already provided):
|
||||
|
||||
-2. After gathering context, ALWAYS present:
|
||||
- - Uncertainties
|
||||
- - Clarifying Questions
|
||||
+2. After gathering context, assess clarity:
|
||||
+ - Assumptions Made
|
||||
|
||||
-3. ITERATE until ALL requirements are crystal clear:
|
||||
- - Do NOT proceed until 100% clarity
|
||||
- - Ask user to confirm
|
||||
+3. PROCEED TO PLAN GENERATION when:
|
||||
+ - Core objective understood
|
||||
+ - Context gathered
|
||||
+ - Reasonable assumptions possible
|
||||
+
|
||||
+ DO NOT loop indefinitely.
|
||||
+ DOCUMENT assumptions.
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🚀 권장 사항
|
||||
|
||||
### Immediate Actions
|
||||
|
||||
1. ✅ **수정 적용 완료** - constants.ts 업데이트됨
|
||||
2. ⏳ **테스트 수행** - ULW mode에서 동작 검증
|
||||
3. ⏳ **PR 생성** - code review 요청
|
||||
|
||||
### Future Improvements
|
||||
|
||||
1. **Subagent context 표준화**
|
||||
- Subagent로 호출 시 명시적 플래그 전달
|
||||
- `is_subagent: true` 파라미터 추가 고려
|
||||
|
||||
2. **Assumptions validation workflow**
|
||||
- Plan 실행 중 assumptions 검증 메커니즘
|
||||
- Incorrect assumptions 감지 시 재계획
|
||||
|
||||
3. **Timeout 메커니즘**
|
||||
- Plan Agent가 X분 이상 걸리면 강제 종료
|
||||
- Fallback plan 생성
|
||||
|
||||
4. **Monitoring 추가**
|
||||
- Plan Agent 실행 시간 측정
|
||||
- Iteration 횟수 로깅
|
||||
- 무한루프 조기 감지
|
||||
|
||||
---
|
||||
|
||||
## 📖 관련 코드 구조
|
||||
|
||||
### Call Stack
|
||||
|
||||
```
|
||||
Sisyphus (ULW mode)
|
||||
↓
|
||||
task(category="deep", ...)
|
||||
↓
|
||||
executor.ts: executeBackgroundContinuation()
|
||||
↓
|
||||
prompt-builder.ts: buildSystemContent()
|
||||
↓
|
||||
constants.ts: PLAN_AGENT_SYSTEM_PREPEND (문제 위치)
|
||||
↓
|
||||
Plan Agent 실행
|
||||
```
|
||||
|
||||
### Key Functions
|
||||
|
||||
1. **executor.ts:587** - `isPlanAgent()` 체크
|
||||
2. **prompt-builder.ts:11** - Plan Agent prepend 주입
|
||||
3. **constants.ts:234** - PLAN_AGENT_SYSTEM_PREPEND 정의
|
||||
|
||||
---
|
||||
|
||||
## 🎓 교훈
|
||||
|
||||
### Design Lessons
|
||||
|
||||
1. **Dual Mode Support**
|
||||
- Interactive vs Autonomous mode 구분 필수
|
||||
- Context 전달 방식 명확히
|
||||
|
||||
2. **Avoid Perfectionism in Agents**
|
||||
- "100% clarity" 같은 주관적 조건 지양
|
||||
- 명확한 객관적 종료 조건 필요
|
||||
|
||||
3. **Document Uncertainties**
|
||||
- 불확실성을 숨기지 말고 문서화
|
||||
- 실행 중 validation 가능하게
|
||||
|
||||
4. **Infinite Loop Prevention**
|
||||
- 모든 반복문에 명시적 종료 조건
|
||||
- Timeout 또는 max iteration 설정
|
||||
|
||||
---
|
||||
|
||||
## 🔗 참고 자료
|
||||
|
||||
- **Issue:** #1501 - [Bug]: ULW mode will 100% cause PLAN AGENT to get stuck
|
||||
- **Files Modified:** `src/tools/delegate-task/constants.ts`
|
||||
- **Related Concepts:** Ultrawork mode, Plan Agent, Subagent delegation
|
||||
- **Agent Architecture:** Sisyphus → Prometheus → Atlas workflow
|
||||
|
||||
---
|
||||
|
||||
## ✅ Conclusion
|
||||
|
||||
**Root Cause:** Plan Agent가 interactive mode를 가정했으나 ULW mode에서는 subagent로 실행되어 사용자 상호작용 불가능. "100% clarity" 요구로 무한루프 발생.
|
||||
|
||||
**Solution:** Subagent mode 감지 로직 추가, clarifying questions 제거, 명확한 종료 조건 제공, assumptions 문서화 방식 도입.
|
||||
|
||||
**Result:** ULW mode에서 Plan Agent가 정상적으로 plan 생성 후 종료. 무한루프 해결.
|
||||
|
||||
---
|
||||
|
||||
**Status:** ✅ Fixed
|
||||
**Tested:** ⏳ Pending
|
||||
**Deployed:** ⏳ Pending
|
||||
|
||||
**Analyst:** Sisyphus (oh-my-opencode ultrawork mode)
|
||||
**Date:** 2026-02-05
|
||||
**Session:** fast-ember
|
||||
@@ -1,4 +1,5 @@
|
||||
import { resolveModelPipeline } from "../../shared"
|
||||
import { transformModelForProvider } from "../../shared/provider-model-id-transform"
|
||||
|
||||
export function applyModelResolution(input: {
|
||||
uiSelectedModel?: string
|
||||
@@ -20,8 +21,10 @@ export function getFirstFallbackModel(requirement?: {
|
||||
}) {
|
||||
const entry = requirement?.fallbackChain?.[0]
|
||||
if (!entry || entry.providers.length === 0) return undefined
|
||||
const provider = entry.providers[0]
|
||||
const transformedModel = transformModelForProvider(provider, entry.model)
|
||||
return {
|
||||
model: `${entry.providers[0]}/${entry.model}`,
|
||||
model: `${provider}/${transformedModel}`,
|
||||
provenance: "provider-fallback" as const,
|
||||
variant: entry.variant,
|
||||
}
|
||||
|
||||
@@ -334,48 +334,48 @@ exports[`generateModelConfig single native provider uses Gemini models when only
|
||||
"model": "opencode/minimax-m2.5-free",
|
||||
},
|
||||
"metis": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"momus": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"multimodal-looker": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"oracle": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"prometheus": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
},
|
||||
},
|
||||
"categories": {
|
||||
"artistry": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"quick": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"ultrabrain": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"unspecified-high": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"unspecified-low": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"visual-engineering": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"writing": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
},
|
||||
}
|
||||
@@ -395,48 +395,48 @@ exports[`generateModelConfig single native provider uses Gemini models with isMa
|
||||
"model": "opencode/minimax-m2.5-free",
|
||||
},
|
||||
"metis": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"momus": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"multimodal-looker": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"oracle": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"prometheus": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
},
|
||||
},
|
||||
"categories": {
|
||||
"artistry": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"quick": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"ultrabrain": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"unspecified-high": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
},
|
||||
"unspecified-low": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"visual-engineering": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"writing": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
},
|
||||
}
|
||||
@@ -468,7 +468,7 @@ exports[`generateModelConfig all native providers uses preferred models from fal
|
||||
"variant": "medium",
|
||||
},
|
||||
"multimodal-looker": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"oracle": {
|
||||
"model": "openai/gpt-5.2",
|
||||
@@ -485,7 +485,7 @@ exports[`generateModelConfig all native providers uses preferred models from fal
|
||||
},
|
||||
"categories": {
|
||||
"artistry": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"deep": {
|
||||
@@ -506,11 +506,11 @@ exports[`generateModelConfig all native providers uses preferred models from fal
|
||||
"model": "anthropic/claude-sonnet-4-6",
|
||||
},
|
||||
"visual-engineering": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"writing": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
},
|
||||
}
|
||||
@@ -542,7 +542,7 @@ exports[`generateModelConfig all native providers uses preferred models with isM
|
||||
"variant": "medium",
|
||||
},
|
||||
"multimodal-looker": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"oracle": {
|
||||
"model": "openai/gpt-5.2",
|
||||
@@ -559,7 +559,7 @@ exports[`generateModelConfig all native providers uses preferred models with isM
|
||||
},
|
||||
"categories": {
|
||||
"artistry": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"deep": {
|
||||
@@ -581,11 +581,11 @@ exports[`generateModelConfig all native providers uses preferred models with isM
|
||||
"model": "anthropic/claude-sonnet-4-6",
|
||||
},
|
||||
"visual-engineering": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"writing": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
},
|
||||
}
|
||||
@@ -1230,10 +1230,10 @@ exports[`generateModelConfig mixed provider scenarios uses Gemini + Claude combi
|
||||
"variant": "max",
|
||||
},
|
||||
"multimodal-looker": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
"oracle": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"prometheus": {
|
||||
@@ -1247,14 +1247,14 @@ exports[`generateModelConfig mixed provider scenarios uses Gemini + Claude combi
|
||||
},
|
||||
"categories": {
|
||||
"artistry": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"quick": {
|
||||
"model": "anthropic/claude-haiku-4-5",
|
||||
},
|
||||
"ultrabrain": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"unspecified-high": {
|
||||
@@ -1264,11 +1264,11 @@ exports[`generateModelConfig mixed provider scenarios uses Gemini + Claude combi
|
||||
"model": "anthropic/claude-sonnet-4-6",
|
||||
},
|
||||
"visual-engineering": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"writing": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
},
|
||||
}
|
||||
@@ -1391,7 +1391,7 @@ exports[`generateModelConfig mixed provider scenarios uses all providers togethe
|
||||
},
|
||||
"categories": {
|
||||
"artistry": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"deep": {
|
||||
@@ -1412,11 +1412,11 @@ exports[`generateModelConfig mixed provider scenarios uses all providers togethe
|
||||
"model": "anthropic/claude-sonnet-4-6",
|
||||
},
|
||||
"visual-engineering": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"writing": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
},
|
||||
}
|
||||
@@ -1465,7 +1465,7 @@ exports[`generateModelConfig mixed provider scenarios uses all providers with is
|
||||
},
|
||||
"categories": {
|
||||
"artistry": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"deep": {
|
||||
@@ -1487,11 +1487,11 @@ exports[`generateModelConfig mixed provider scenarios uses all providers with is
|
||||
"model": "anthropic/claude-sonnet-4-6",
|
||||
},
|
||||
"visual-engineering": {
|
||||
"model": "google/gemini-3-pro",
|
||||
"model": "google/gemini-3-pro-preview",
|
||||
"variant": "high",
|
||||
},
|
||||
"writing": {
|
||||
"model": "google/gemini-3-flash",
|
||||
"model": "google/gemini-3-flash-preview",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
191
src/cli/provider-model-id-transform.test.ts
Normal file
191
src/cli/provider-model-id-transform.test.ts
Normal file
@@ -0,0 +1,191 @@
|
||||
import { describe, expect, test } from "bun:test"
|
||||
|
||||
import { transformModelForProvider } from "./provider-model-id-transform"
|
||||
|
||||
describe("transformModelForProvider", () => {
|
||||
describe("github-copilot provider", () => {
|
||||
test("transforms claude-opus-4-6 to claude-opus-4.6", () => {
|
||||
// #given github-copilot provider and claude-opus-4-6 model
|
||||
const provider = "github-copilot"
|
||||
const model = "claude-opus-4-6"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should transform to claude-opus-4.6
|
||||
expect(result).toBe("claude-opus-4.6")
|
||||
})
|
||||
|
||||
test("transforms claude-sonnet-4-5 to claude-sonnet-4.5", () => {
|
||||
// #given github-copilot provider and claude-sonnet-4-5 model
|
||||
const provider = "github-copilot"
|
||||
const model = "claude-sonnet-4-5"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should transform to claude-sonnet-4.5
|
||||
expect(result).toBe("claude-sonnet-4.5")
|
||||
})
|
||||
|
||||
test("transforms claude-haiku-4-5 to claude-haiku-4.5", () => {
|
||||
// #given github-copilot provider and claude-haiku-4-5 model
|
||||
const provider = "github-copilot"
|
||||
const model = "claude-haiku-4-5"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should transform to claude-haiku-4.5
|
||||
expect(result).toBe("claude-haiku-4.5")
|
||||
})
|
||||
|
||||
test("transforms gemini-3-pro to gemini-3-pro-preview", () => {
|
||||
// #given github-copilot provider and gemini-3-pro model
|
||||
const provider = "github-copilot"
|
||||
const model = "gemini-3-pro"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should transform to gemini-3-pro-preview
|
||||
expect(result).toBe("gemini-3-pro-preview")
|
||||
})
|
||||
|
||||
test("transforms gemini-3-flash to gemini-3-flash-preview", () => {
|
||||
// #given github-copilot provider and gemini-3-flash model
|
||||
const provider = "github-copilot"
|
||||
const model = "gemini-3-flash"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should transform to gemini-3-flash-preview
|
||||
expect(result).toBe("gemini-3-flash-preview")
|
||||
})
|
||||
|
||||
test("prevents double transformation of gemini-3-pro-preview", () => {
|
||||
// #given github-copilot provider and gemini-3-pro-preview model (already transformed)
|
||||
const provider = "github-copilot"
|
||||
const model = "gemini-3-pro-preview"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should NOT become gemini-3-pro-preview-preview
|
||||
expect(result).toBe("gemini-3-pro-preview")
|
||||
})
|
||||
|
||||
test("prevents double transformation of gemini-3-flash-preview", () => {
|
||||
// #given github-copilot provider and gemini-3-flash-preview model (already transformed)
|
||||
const provider = "github-copilot"
|
||||
const model = "gemini-3-flash-preview"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should NOT become gemini-3-flash-preview-preview
|
||||
expect(result).toBe("gemini-3-flash-preview")
|
||||
})
|
||||
})
|
||||
|
||||
describe("google provider", () => {
|
||||
test("transforms gemini-3-flash to gemini-3-flash-preview", () => {
|
||||
// #given google provider and gemini-3-flash model
|
||||
const provider = "google"
|
||||
const model = "gemini-3-flash"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should transform to gemini-3-flash-preview
|
||||
expect(result).toBe("gemini-3-flash-preview")
|
||||
})
|
||||
|
||||
test("transforms gemini-3-pro to gemini-3-pro-preview", () => {
|
||||
// #given google provider and gemini-3-pro model
|
||||
const provider = "google"
|
||||
const model = "gemini-3-pro"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should transform to gemini-3-pro-preview
|
||||
expect(result).toBe("gemini-3-pro-preview")
|
||||
})
|
||||
|
||||
test("passes through other gemini models unchanged", () => {
|
||||
// #given google provider and gemini-2.5-flash model
|
||||
const provider = "google"
|
||||
const model = "gemini-2.5-flash"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should pass through unchanged
|
||||
expect(result).toBe("gemini-2.5-flash")
|
||||
})
|
||||
|
||||
test("prevents double transformation of gemini-3-flash-preview", () => {
|
||||
// #given google provider and gemini-3-flash-preview model (already transformed)
|
||||
const provider = "google"
|
||||
const model = "gemini-3-flash-preview"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should NOT become gemini-3-flash-preview-preview
|
||||
expect(result).toBe("gemini-3-flash-preview")
|
||||
})
|
||||
|
||||
test("prevents double transformation of gemini-3-pro-preview", () => {
|
||||
// #given google provider and gemini-3-pro-preview model (already transformed)
|
||||
const provider = "google"
|
||||
const model = "gemini-3-pro-preview"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should NOT become gemini-3-pro-preview-preview
|
||||
expect(result).toBe("gemini-3-pro-preview")
|
||||
})
|
||||
|
||||
test("does not transform claude models for google provider", () => {
|
||||
// #given google provider and claude-opus-4-6 model
|
||||
const provider = "google"
|
||||
const model = "claude-opus-4-6"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should pass through unchanged (google doesn't use claude)
|
||||
expect(result).toBe("claude-opus-4-6")
|
||||
})
|
||||
})
|
||||
|
||||
describe("unknown provider", () => {
|
||||
test("passes model through unchanged for unknown provider", () => {
|
||||
// #given unknown provider and any model
|
||||
const provider = "unknown-provider"
|
||||
const model = "some-model"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should pass through unchanged
|
||||
expect(result).toBe("some-model")
|
||||
})
|
||||
|
||||
test("passes gemini-3-flash through unchanged for unknown provider", () => {
|
||||
// #given unknown provider and gemini-3-flash model
|
||||
const provider = "unknown-provider"
|
||||
const model = "gemini-3-flash"
|
||||
|
||||
// #when transformModelForProvider is called
|
||||
const result = transformModelForProvider(provider, model)
|
||||
|
||||
// #then should pass through unchanged (no transformation for unknown provider)
|
||||
expect(result).toBe("gemini-3-flash")
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -1,12 +1 @@
|
||||
export function transformModelForProvider(provider: string, model: string): string {
|
||||
if (provider === "github-copilot") {
|
||||
return model
|
||||
.replace("claude-opus-4-6", "claude-opus-4.6")
|
||||
.replace("claude-sonnet-4-6", "claude-sonnet-4.6")
|
||||
.replace("claude-haiku-4-5", "claude-haiku-4.5")
|
||||
.replace("claude-sonnet-4", "claude-sonnet-4")
|
||||
.replace("gemini-3-pro", "gemini-3-pro-preview")
|
||||
.replace("gemini-3-flash", "gemini-3-flash-preview")
|
||||
}
|
||||
return model
|
||||
}
|
||||
export { transformModelForProvider } from "../shared/provider-model-id-transform"
|
||||
|
||||
@@ -1,50 +1,262 @@
|
||||
import type { Hooks, PluginInput } from "@opencode-ai/plugin"
|
||||
|
||||
import { existsSync } from "fs"
|
||||
import { resolve, isAbsolute, join, normalize, sep } from "path"
|
||||
import { existsSync, realpathSync } from "fs"
|
||||
import { basename, dirname, isAbsolute, join, normalize, relative, resolve, sep } from "path"
|
||||
|
||||
import { log } from "../../shared"
|
||||
|
||||
type GuardArgs = {
|
||||
filePath?: string
|
||||
path?: string
|
||||
file_path?: string
|
||||
overwrite?: boolean | string
|
||||
}
|
||||
|
||||
const MAX_TRACKED_SESSIONS = 256
|
||||
export const MAX_TRACKED_PATHS_PER_SESSION = 1024
|
||||
const OUTSIDE_SESSION_MESSAGE = "Path must be inside session directory."
|
||||
|
||||
function asRecord(value: unknown): Record<string, unknown> | undefined {
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) {
|
||||
return undefined
|
||||
}
|
||||
|
||||
return value as Record<string, unknown>
|
||||
}
|
||||
|
||||
function getPathFromArgs(args: GuardArgs | undefined): string | undefined {
|
||||
return args?.filePath ?? args?.path ?? args?.file_path
|
||||
}
|
||||
|
||||
function resolveInputPath(ctx: PluginInput, inputPath: string): string {
|
||||
return normalize(isAbsolute(inputPath) ? inputPath : resolve(ctx.directory, inputPath))
|
||||
}
|
||||
|
||||
function isPathInsideDirectory(pathToCheck: string, directory: string): boolean {
|
||||
const relativePath = relative(directory, pathToCheck)
|
||||
return relativePath === "" || (!relativePath.startsWith("..") && !isAbsolute(relativePath))
|
||||
}
|
||||
|
||||
function toCanonicalPath(absolutePath: string): string {
|
||||
let canonicalPath = absolutePath
|
||||
|
||||
if (existsSync(absolutePath)) {
|
||||
try {
|
||||
canonicalPath = realpathSync.native(absolutePath)
|
||||
} catch {
|
||||
canonicalPath = absolutePath
|
||||
}
|
||||
} else {
|
||||
const absoluteDir = dirname(absolutePath)
|
||||
const resolvedDir = existsSync(absoluteDir) ? realpathSync.native(absoluteDir) : absoluteDir
|
||||
canonicalPath = join(resolvedDir, basename(absolutePath))
|
||||
}
|
||||
|
||||
// Preserve canonical casing from the filesystem to avoid collapsing distinct
|
||||
// files on case-sensitive volumes (supported on all major OSes).
|
||||
return normalize(canonicalPath)
|
||||
}
|
||||
|
||||
function isOverwriteEnabled(value: boolean | string | undefined): boolean {
|
||||
if (value === true) {
|
||||
return true
|
||||
}
|
||||
|
||||
if (typeof value === "string") {
|
||||
return value.toLowerCase() === "true"
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
export function createWriteExistingFileGuardHook(ctx: PluginInput): Hooks {
|
||||
return {
|
||||
"tool.execute.before": async (input, output) => {
|
||||
const toolName = input.tool?.toLowerCase()
|
||||
if (toolName !== "write") {
|
||||
const readPermissionsBySession = new Map<string, Set<string>>()
|
||||
const sessionLastAccess = new Map<string, number>()
|
||||
const canonicalSessionRoot = toCanonicalPath(resolveInputPath(ctx, ctx.directory))
|
||||
const sisyphusRoot = join(canonicalSessionRoot, ".sisyphus") + sep
|
||||
|
||||
const touchSession = (sessionID: string): void => {
|
||||
sessionLastAccess.set(sessionID, Date.now())
|
||||
}
|
||||
|
||||
const evictLeastRecentlyUsedSession = (): void => {
|
||||
let oldestSessionID: string | undefined
|
||||
let oldestSeen = Number.POSITIVE_INFINITY
|
||||
|
||||
for (const [sessionID, lastSeen] of sessionLastAccess.entries()) {
|
||||
if (lastSeen < oldestSeen) {
|
||||
oldestSeen = lastSeen
|
||||
oldestSessionID = sessionID
|
||||
}
|
||||
}
|
||||
|
||||
if (!oldestSessionID) {
|
||||
return
|
||||
}
|
||||
|
||||
readPermissionsBySession.delete(oldestSessionID)
|
||||
sessionLastAccess.delete(oldestSessionID)
|
||||
}
|
||||
|
||||
const ensureSessionReadSet = (sessionID: string): Set<string> => {
|
||||
let readSet = readPermissionsBySession.get(sessionID)
|
||||
if (!readSet) {
|
||||
if (readPermissionsBySession.size >= MAX_TRACKED_SESSIONS) {
|
||||
evictLeastRecentlyUsedSession()
|
||||
}
|
||||
|
||||
readSet = new Set<string>()
|
||||
readPermissionsBySession.set(sessionID, readSet)
|
||||
}
|
||||
|
||||
touchSession(sessionID)
|
||||
return readSet
|
||||
}
|
||||
|
||||
const trimSessionReadSet = (readSet: Set<string>): void => {
|
||||
while (readSet.size > MAX_TRACKED_PATHS_PER_SESSION) {
|
||||
const oldestPath = readSet.values().next().value
|
||||
if (!oldestPath) {
|
||||
return
|
||||
}
|
||||
|
||||
const args = output.args as
|
||||
| { filePath?: string; path?: string; file_path?: string }
|
||||
| undefined
|
||||
const filePath = args?.filePath ?? args?.path ?? args?.file_path
|
||||
readSet.delete(oldestPath)
|
||||
}
|
||||
}
|
||||
|
||||
const registerReadPermission = (sessionID: string, canonicalPath: string): void => {
|
||||
const readSet = ensureSessionReadSet(sessionID)
|
||||
if (readSet.has(canonicalPath)) {
|
||||
readSet.delete(canonicalPath)
|
||||
}
|
||||
|
||||
readSet.add(canonicalPath)
|
||||
trimSessionReadSet(readSet)
|
||||
}
|
||||
|
||||
const consumeReadPermission = (sessionID: string, canonicalPath: string): boolean => {
|
||||
const readSet = readPermissionsBySession.get(sessionID)
|
||||
if (!readSet || !readSet.has(canonicalPath)) {
|
||||
return false
|
||||
}
|
||||
|
||||
readSet.delete(canonicalPath)
|
||||
touchSession(sessionID)
|
||||
return true
|
||||
}
|
||||
|
||||
const invalidateOtherSessions = (canonicalPath: string, writingSessionID?: string): void => {
|
||||
for (const [sessionID, readSet] of readPermissionsBySession.entries()) {
|
||||
if (writingSessionID && sessionID === writingSessionID) {
|
||||
continue
|
||||
}
|
||||
|
||||
readSet.delete(canonicalPath)
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
"tool.execute.before": async (input, output) => {
|
||||
const toolName = input.tool?.toLowerCase()
|
||||
if (toolName !== "write" && toolName !== "read") {
|
||||
return
|
||||
}
|
||||
|
||||
const argsRecord = asRecord(output.args)
|
||||
const args = argsRecord as GuardArgs | undefined
|
||||
const filePath = getPathFromArgs(args)
|
||||
if (!filePath) {
|
||||
return
|
||||
}
|
||||
|
||||
const resolvedPath = normalize(
|
||||
isAbsolute(filePath) ? filePath : resolve(ctx.directory, filePath)
|
||||
)
|
||||
const resolvedPath = resolveInputPath(ctx, filePath)
|
||||
const canonicalPath = toCanonicalPath(resolvedPath)
|
||||
const isInsideSessionDirectory = isPathInsideDirectory(canonicalPath, canonicalSessionRoot)
|
||||
|
||||
if (existsSync(resolvedPath)) {
|
||||
const sisyphusRoot = join(ctx.directory, ".sisyphus") + sep
|
||||
const isSisyphusMarkdown =
|
||||
resolvedPath.startsWith(sisyphusRoot) && resolvedPath.endsWith(".md")
|
||||
if (isSisyphusMarkdown) {
|
||||
log("[write-existing-file-guard] Allowing .sisyphus/*.md overwrite", {
|
||||
sessionID: input.sessionID,
|
||||
filePath,
|
||||
})
|
||||
if (!isInsideSessionDirectory) {
|
||||
if (toolName === "read") {
|
||||
return
|
||||
}
|
||||
|
||||
log("[write-existing-file-guard] Blocking write to existing file", {
|
||||
log("[write-existing-file-guard] Blocking write outside session directory", {
|
||||
sessionID: input.sessionID,
|
||||
filePath,
|
||||
resolvedPath,
|
||||
})
|
||||
|
||||
throw new Error("File already exists. Use edit tool instead.")
|
||||
throw new Error(OUTSIDE_SESSION_MESSAGE)
|
||||
}
|
||||
|
||||
if (toolName === "read") {
|
||||
if (!existsSync(resolvedPath) || !input.sessionID) {
|
||||
return
|
||||
}
|
||||
|
||||
registerReadPermission(input.sessionID, canonicalPath)
|
||||
return
|
||||
}
|
||||
|
||||
const overwriteEnabled = isOverwriteEnabled(args?.overwrite)
|
||||
|
||||
if (argsRecord && "overwrite" in argsRecord) {
|
||||
// Intentionally mutate output args so overwrite bypass remains hook-only.
|
||||
delete argsRecord.overwrite
|
||||
}
|
||||
|
||||
if (!existsSync(resolvedPath)) {
|
||||
return
|
||||
}
|
||||
|
||||
const isSisyphusPath = canonicalPath.startsWith(sisyphusRoot)
|
||||
if (isSisyphusPath) {
|
||||
log("[write-existing-file-guard] Allowing .sisyphus/** overwrite", {
|
||||
sessionID: input.sessionID,
|
||||
filePath,
|
||||
})
|
||||
invalidateOtherSessions(canonicalPath, input.sessionID)
|
||||
return
|
||||
}
|
||||
|
||||
if (overwriteEnabled) {
|
||||
log("[write-existing-file-guard] Allowing overwrite flag bypass", {
|
||||
sessionID: input.sessionID,
|
||||
filePath,
|
||||
resolvedPath,
|
||||
})
|
||||
invalidateOtherSessions(canonicalPath, input.sessionID)
|
||||
return
|
||||
}
|
||||
|
||||
if (input.sessionID && consumeReadPermission(input.sessionID, canonicalPath)) {
|
||||
log("[write-existing-file-guard] Allowing overwrite after read", {
|
||||
sessionID: input.sessionID,
|
||||
filePath,
|
||||
resolvedPath,
|
||||
})
|
||||
invalidateOtherSessions(canonicalPath, input.sessionID)
|
||||
return
|
||||
}
|
||||
|
||||
log("[write-existing-file-guard] Blocking write to existing file", {
|
||||
sessionID: input.sessionID,
|
||||
filePath,
|
||||
resolvedPath,
|
||||
})
|
||||
|
||||
throw new Error("File already exists. Use edit tool instead.")
|
||||
},
|
||||
event: async ({ event }: { event: { type: string; properties?: unknown } }) => {
|
||||
if (event.type !== "session.deleted") {
|
||||
return
|
||||
}
|
||||
|
||||
const props = event.properties as { info?: { id?: string } } | undefined
|
||||
const sessionID = props?.info?.id
|
||||
if (!sessionID) {
|
||||
return
|
||||
}
|
||||
|
||||
readPermissionsBySession.delete(sessionID)
|
||||
sessionLastAccess.delete(sessionID)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,305 +1,551 @@
|
||||
import { describe, test, expect, beforeEach, afterEach } from "bun:test"
|
||||
import { afterEach, beforeEach, describe, expect, test } from "bun:test"
|
||||
import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { dirname, join, resolve } from "node:path"
|
||||
|
||||
import { MAX_TRACKED_PATHS_PER_SESSION } from "./hook"
|
||||
import { createWriteExistingFileGuardHook } from "./index"
|
||||
import * as fs from "fs"
|
||||
import * as path from "path"
|
||||
import * as os from "os"
|
||||
|
||||
const BLOCK_MESSAGE = "File already exists. Use edit tool instead."
|
||||
const OUTSIDE_SESSION_MESSAGE = "Path must be inside session directory."
|
||||
|
||||
type Hook = ReturnType<typeof createWriteExistingFileGuardHook>
|
||||
|
||||
function isCaseInsensitiveFilesystem(directory: string): boolean {
|
||||
const probeName = `CaseProbe_${Date.now()}_A.txt`
|
||||
const upperPath = join(directory, probeName)
|
||||
const lowerPath = join(directory, probeName.toLowerCase())
|
||||
|
||||
writeFileSync(upperPath, "probe")
|
||||
try {
|
||||
return existsSync(lowerPath)
|
||||
} finally {
|
||||
rmSync(upperPath, { force: true })
|
||||
}
|
||||
}
|
||||
|
||||
describe("createWriteExistingFileGuardHook", () => {
|
||||
let tempDir: string
|
||||
let ctx: { directory: string }
|
||||
let hook: ReturnType<typeof createWriteExistingFileGuardHook>
|
||||
let tempDir = ""
|
||||
let hook: Hook
|
||||
let callCounter = 0
|
||||
|
||||
const createFile = (relativePath: string, content = "existing content"): string => {
|
||||
const absolutePath = join(tempDir, relativePath)
|
||||
mkdirSync(dirname(absolutePath), { recursive: true })
|
||||
writeFileSync(absolutePath, content)
|
||||
return absolutePath
|
||||
}
|
||||
|
||||
const invoke = async (args: {
|
||||
tool: string
|
||||
sessionID?: string
|
||||
outputArgs: Record<string, unknown>
|
||||
}): Promise<{ args: Record<string, unknown> }> => {
|
||||
callCounter += 1
|
||||
const output = { args: args.outputArgs }
|
||||
|
||||
await hook["tool.execute.before"]?.(
|
||||
{
|
||||
tool: args.tool,
|
||||
sessionID: args.sessionID ?? "ses_default",
|
||||
callID: `call_${callCounter}`,
|
||||
} as never,
|
||||
output as never
|
||||
)
|
||||
|
||||
return output
|
||||
}
|
||||
|
||||
const emitSessionDeleted = async (sessionID: string): Promise<void> => {
|
||||
await hook.event?.({ event: { type: "session.deleted", properties: { info: { id: sessionID } } } })
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "write-guard-test-"))
|
||||
ctx = { directory: tempDir }
|
||||
hook = createWriteExistingFileGuardHook(ctx as any)
|
||||
tempDir = mkdtempSync(join(tmpdir(), "write-existing-file-guard-"))
|
||||
hook = createWriteExistingFileGuardHook({ directory: tempDir } as never)
|
||||
callCounter = 0
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true })
|
||||
rmSync(tempDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
describe("tool.execute.before", () => {
|
||||
test("allows write to non-existing file", async () => {
|
||||
//#given
|
||||
const nonExistingFile = path.join(tempDir, "new-file.txt")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: nonExistingFile, content: "hello" } }
|
||||
test("#given non-existing file #when write executes #then allows", async () => {
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
outputArgs: { filePath: join(tempDir, "new-file.txt"), content: "new content" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
})
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
test("#given existing file without read or overwrite #when write executes #then blocks", async () => {
|
||||
const existingFile = createFile("existing.txt")
|
||||
|
||||
//#then
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
outputArgs: { filePath: existingFile, content: "new content" },
|
||||
})
|
||||
).rejects.toThrow(BLOCK_MESSAGE)
|
||||
})
|
||||
|
||||
test("#given same-session read #when write executes #then allows once and consumes permission", async () => {
|
||||
const existingFile = createFile("consume-once.txt")
|
||||
const sessionID = "ses_consume"
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
|
||||
test("blocks write to existing file", async () => {
|
||||
//#given
|
||||
const existingFile = path.join(tempDir, "existing-file.txt")
|
||||
fs.writeFileSync(existingFile, "existing content")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: existingFile, content: "new content" } }
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile, content: "first overwrite" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile, content: "second overwrite" },
|
||||
})
|
||||
).rejects.toThrow(BLOCK_MESSAGE)
|
||||
})
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
test("#given same-session concurrent writes #when only one read permission exists #then allows only one write", async () => {
|
||||
const existingFile = createFile("concurrent-consume.txt")
|
||||
const sessionID = "ses_concurrent"
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
|
||||
test("blocks write tool (lowercase) to existing file", async () => {
|
||||
//#given
|
||||
const existingFile = path.join(tempDir, "existing-file.txt")
|
||||
fs.writeFileSync(existingFile, "existing content")
|
||||
const input = { tool: "write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: existingFile, content: "new content" } }
|
||||
const results = await Promise.allSettled([
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile, content: "first attempt" },
|
||||
}),
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile, content: "second attempt" },
|
||||
}),
|
||||
])
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
const successCount = results.filter((result) => result.status === "fulfilled").length
|
||||
const failures = results.filter(
|
||||
(result): result is PromiseRejectedResult => result.status === "rejected"
|
||||
)
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
expect(successCount).toBe(1)
|
||||
expect(failures).toHaveLength(1)
|
||||
expect(String(failures[0]?.reason)).toContain(BLOCK_MESSAGE)
|
||||
})
|
||||
|
||||
test("#given read in another session #when write executes #then blocks", async () => {
|
||||
const existingFile = createFile("cross-session.txt")
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID: "ses_reader",
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
|
||||
test("ignores non-write tools", async () => {
|
||||
//#given
|
||||
const existingFile = path.join(tempDir, "existing-file.txt")
|
||||
fs.writeFileSync(existingFile, "existing content")
|
||||
const input = { tool: "Edit", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: existingFile, content: "new content" } }
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID: "ses_writer",
|
||||
outputArgs: { filePath: existingFile, content: "new content" },
|
||||
})
|
||||
).rejects.toThrow(BLOCK_MESSAGE)
|
||||
})
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
test("#given overwrite true boolean #when write executes #then bypasses guard and strips overwrite", async () => {
|
||||
const existingFile = createFile("overwrite-boolean.txt")
|
||||
|
||||
//#then
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
const output = await invoke({
|
||||
tool: "write",
|
||||
outputArgs: {
|
||||
filePath: existingFile,
|
||||
content: "new content",
|
||||
overwrite: true,
|
||||
},
|
||||
})
|
||||
|
||||
test("ignores tools without any file path arg", async () => {
|
||||
//#given
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { command: "ls" } }
|
||||
expect(output.args.overwrite).toBeUndefined()
|
||||
})
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
test("#given overwrite true string #when write executes #then bypasses guard and strips overwrite", async () => {
|
||||
const existingFile = createFile("overwrite-string.txt")
|
||||
|
||||
//#then
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
const output = await invoke({
|
||||
tool: "write",
|
||||
outputArgs: {
|
||||
filePath: existingFile,
|
||||
content: "new content",
|
||||
overwrite: "true",
|
||||
},
|
||||
})
|
||||
|
||||
describe("alternative arg names", () => {
|
||||
test("blocks write using 'path' arg to existing file", async () => {
|
||||
//#given
|
||||
const existingFile = path.join(tempDir, "existing-file.txt")
|
||||
fs.writeFileSync(existingFile, "existing content")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { path: existingFile, content: "new content" } }
|
||||
expect(output.args.overwrite).toBeUndefined()
|
||||
})
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
test("#given overwrite falsy values #when write executes #then does not bypass guard", async () => {
|
||||
const existingFile = createFile("overwrite-falsy.txt")
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
})
|
||||
for (const overwrite of [false, "false"] as const) {
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
outputArgs: {
|
||||
filePath: existingFile,
|
||||
content: "new content",
|
||||
overwrite,
|
||||
},
|
||||
})
|
||||
).rejects.toThrow(BLOCK_MESSAGE)
|
||||
}
|
||||
})
|
||||
|
||||
test("blocks write using 'file_path' arg to existing file", async () => {
|
||||
//#given
|
||||
const existingFile = path.join(tempDir, "existing-file.txt")
|
||||
fs.writeFileSync(existingFile, "existing content")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { file_path: existingFile, content: "new content" } }
|
||||
test("#given two sessions read same file #when one writes #then other session is invalidated", async () => {
|
||||
const existingFile = createFile("invalidate.txt")
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
})
|
||||
|
||||
test("allows write using 'path' arg to non-existing file", async () => {
|
||||
//#given
|
||||
const nonExistingFile = path.join(tempDir, "new-file.txt")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { path: nonExistingFile, content: "hello" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
test("allows write using 'file_path' arg to non-existing file", async () => {
|
||||
//#given
|
||||
const nonExistingFile = path.join(tempDir, "new-file.txt")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { file_path: nonExistingFile, content: "hello" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
})
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID: "ses_a",
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID: "ses_b",
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
|
||||
describe("relative path resolution using ctx.directory", () => {
|
||||
test("blocks write to existing file using relative path", async () => {
|
||||
//#given
|
||||
const existingFile = path.join(tempDir, "existing-file.txt")
|
||||
fs.writeFileSync(existingFile, "existing content")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: "existing-file.txt", content: "new content" } }
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID: "ses_b",
|
||||
outputArgs: { filePath: existingFile, content: "updated by B" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID: "ses_a",
|
||||
outputArgs: { filePath: existingFile, content: "updated by A" },
|
||||
})
|
||||
).rejects.toThrow(BLOCK_MESSAGE)
|
||||
})
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
test("#given existing file under .sisyphus #when write executes #then always allows", async () => {
|
||||
const existingFile = createFile(".sisyphus/plans/plan.txt")
|
||||
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
outputArgs: { filePath: existingFile, content: "new plan" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
})
|
||||
|
||||
test("#given file arg variants #when read then write executes #then supports all variants", async () => {
|
||||
const existingFile = createFile("variants.txt")
|
||||
const variants: Array<"filePath" | "path" | "file_path"> = [
|
||||
"filePath",
|
||||
"path",
|
||||
"file_path",
|
||||
]
|
||||
|
||||
for (const variant of variants) {
|
||||
const sessionID = `ses_${variant}`
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { [variant]: existingFile },
|
||||
})
|
||||
|
||||
test("allows write to non-existing file using relative path", async () => {
|
||||
//#given
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: "new-file.txt", content: "hello" } }
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { [variant]: existingFile, content: `overwrite via ${variant}` },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
}
|
||||
})
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
test("#given tools without file path arg #when write and read execute #then ignores safely", async () => {
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
outputArgs: { content: "no path" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
|
||||
test("blocks write to nested relative path when file exists", async () => {
|
||||
//#given
|
||||
const subDir = path.join(tempDir, "subdir")
|
||||
fs.mkdirSync(subDir)
|
||||
const existingFile = path.join(subDir, "existing.txt")
|
||||
fs.writeFileSync(existingFile, "existing content")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: "subdir/existing.txt", content: "new content" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "read",
|
||||
outputArgs: {},
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
})
|
||||
|
||||
test("uses ctx.directory not process.cwd for relative path resolution", async () => {
|
||||
//#given
|
||||
const existingFile = path.join(tempDir, "test-file.txt")
|
||||
fs.writeFileSync(existingFile, "content")
|
||||
const differentCtx = { directory: tempDir }
|
||||
const differentHook = createWriteExistingFileGuardHook(differentCtx as any)
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: "test-file.txt", content: "new" } }
|
||||
test("#given non-read-write tool #when it executes #then does not grant write permission", async () => {
|
||||
const existingFile = createFile("ignored-tool.txt")
|
||||
const sessionID = "ses_ignored_tool"
|
||||
|
||||
//#when
|
||||
const result = differentHook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
})
|
||||
|
||||
describe(".sisyphus/*.md exception", () => {
|
||||
test("allows write to existing .sisyphus/plans/plan.md", async () => {
|
||||
//#given
|
||||
const sisyphusDir = path.join(tempDir, ".sisyphus", "plans")
|
||||
fs.mkdirSync(sisyphusDir, { recursive: true })
|
||||
const planFile = path.join(sisyphusDir, "plan.md")
|
||||
fs.writeFileSync(planFile, "# Existing Plan")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: planFile, content: "# Updated Plan" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
test("allows write to existing .sisyphus/notes.md", async () => {
|
||||
//#given
|
||||
const sisyphusDir = path.join(tempDir, ".sisyphus")
|
||||
fs.mkdirSync(sisyphusDir, { recursive: true })
|
||||
const notesFile = path.join(sisyphusDir, "notes.md")
|
||||
fs.writeFileSync(notesFile, "# Notes")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: notesFile, content: "# Updated Notes" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
test("allows write to existing .sisyphus/*.md using relative path", async () => {
|
||||
//#given
|
||||
const sisyphusDir = path.join(tempDir, ".sisyphus")
|
||||
fs.mkdirSync(sisyphusDir, { recursive: true })
|
||||
const planFile = path.join(sisyphusDir, "plan.md")
|
||||
fs.writeFileSync(planFile, "# Plan")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: ".sisyphus/plan.md", content: "# Updated" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
test("blocks write to existing .sisyphus/file.txt (non-markdown)", async () => {
|
||||
//#given
|
||||
const sisyphusDir = path.join(tempDir, ".sisyphus")
|
||||
fs.mkdirSync(sisyphusDir, { recursive: true })
|
||||
const textFile = path.join(sisyphusDir, "file.txt")
|
||||
fs.writeFileSync(textFile, "content")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: textFile, content: "new content" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
})
|
||||
|
||||
test("blocks write when .sisyphus is in parent path but not under ctx.directory", async () => {
|
||||
//#given
|
||||
const fakeSisyphusParent = path.join(os.tmpdir(), ".sisyphus", "evil-project")
|
||||
fs.mkdirSync(fakeSisyphusParent, { recursive: true })
|
||||
const evilFile = path.join(fakeSisyphusParent, "plan.md")
|
||||
fs.writeFileSync(evilFile, "# Evil Plan")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: evilFile, content: "# Hacked" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
|
||||
// cleanup
|
||||
fs.rmSync(path.join(os.tmpdir(), ".sisyphus"), { recursive: true, force: true })
|
||||
})
|
||||
|
||||
test("blocks write to existing regular file (not in .sisyphus)", async () => {
|
||||
//#given
|
||||
const regularFile = path.join(tempDir, "regular.md")
|
||||
fs.writeFileSync(regularFile, "# Regular")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: regularFile, content: "# Updated" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
})
|
||||
await invoke({
|
||||
tool: "edit",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile, oldString: "old", newString: "new" },
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile, content: "should block" },
|
||||
})
|
||||
).rejects.toThrow(BLOCK_MESSAGE)
|
||||
})
|
||||
|
||||
test("#given relative read and absolute write #when same session writes #then allows", async () => {
|
||||
createFile("relative-absolute.txt")
|
||||
const sessionID = "ses_relative_absolute"
|
||||
const relativePath = "relative-absolute.txt"
|
||||
const absolutePath = resolve(tempDir, relativePath)
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { filePath: relativePath },
|
||||
})
|
||||
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: absolutePath, content: "updated" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
})
|
||||
|
||||
test("#given existing file outside session directory #when write executes #then blocks", async () => {
|
||||
const outsideDir = mkdtempSync(join(tmpdir(), "write-existing-file-guard-outside-"))
|
||||
|
||||
try {
|
||||
const outsideFile = join(outsideDir, "outside.txt")
|
||||
writeFileSync(outsideFile, "outside")
|
||||
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
outputArgs: { filePath: outsideFile, content: "attempted overwrite" },
|
||||
})
|
||||
).rejects.toThrow(OUTSIDE_SESSION_MESSAGE)
|
||||
} finally {
|
||||
rmSync(outsideDir, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test("#given session read permission #when session deleted #then permission is cleaned up", async () => {
|
||||
const existingFile = createFile("session-cleanup.txt")
|
||||
const sessionID = "ses_cleanup"
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
|
||||
await emitSessionDeleted(sessionID)
|
||||
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile, content: "after cleanup" },
|
||||
})
|
||||
).rejects.toThrow(BLOCK_MESSAGE)
|
||||
})
|
||||
|
||||
test("#given case-different read path #when writing canonical path #then follows platform behavior", async () => {
|
||||
const canonicalFile = createFile("CaseFile.txt")
|
||||
const lowerCasePath = join(tempDir, "casefile.txt")
|
||||
const sessionID = "ses_case"
|
||||
const isCaseInsensitiveFs = isCaseInsensitiveFilesystem(tempDir)
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { filePath: lowerCasePath },
|
||||
})
|
||||
|
||||
const writeAttempt = invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: canonicalFile, content: "updated" },
|
||||
})
|
||||
|
||||
if (isCaseInsensitiveFs) {
|
||||
await expect(writeAttempt).resolves.toBeDefined()
|
||||
return
|
||||
}
|
||||
|
||||
await expect(writeAttempt).rejects.toThrow(BLOCK_MESSAGE)
|
||||
})
|
||||
|
||||
test("#given read via symlink #when write via real path #then allows overwrite", async () => {
|
||||
const targetFile = createFile("real/target.txt")
|
||||
const symlinkPath = join(tempDir, "linked-target.txt")
|
||||
const sessionID = "ses_symlink"
|
||||
|
||||
try {
|
||||
symlinkSync(targetFile, symlinkPath)
|
||||
} catch (error) {
|
||||
console.warn(
|
||||
"Skipping symlink test: symlinks are not supported or cannot be created in this environment.",
|
||||
error
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { filePath: symlinkPath },
|
||||
})
|
||||
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: targetFile, content: "updated via symlink read" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
})
|
||||
|
||||
test("#given session reads beyond path cap #when writing oldest and newest #then only newest is authorized", async () => {
|
||||
const sessionID = "ses_path_cap"
|
||||
const oldestFile = createFile("path-cap/0.txt")
|
||||
let newestFile = oldestFile
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { filePath: oldestFile },
|
||||
})
|
||||
|
||||
for (let index = 1; index <= MAX_TRACKED_PATHS_PER_SESSION; index += 1) {
|
||||
newestFile = createFile(`path-cap/${index}.txt`)
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { filePath: newestFile },
|
||||
})
|
||||
}
|
||||
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: oldestFile, content: "stale write" },
|
||||
})
|
||||
).rejects.toThrow(BLOCK_MESSAGE)
|
||||
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: newestFile, content: "fresh write" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
})
|
||||
|
||||
test("#given recently active session #when lru evicts #then keeps recent session permission", async () => {
|
||||
const existingFile = createFile("lru.txt")
|
||||
const hotSession = "ses_hot"
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID: hotSession,
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
|
||||
for (let index = 0; index < 255; index += 1) {
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID: `ses_${index}`,
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
}
|
||||
|
||||
await new Promise((resolvePromise) => setTimeout(resolvePromise, 2))
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID: hotSession,
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID: "ses_overflow",
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID: hotSession,
|
||||
outputArgs: { filePath: existingFile, content: "hot session write" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
})
|
||||
|
||||
test("#given session permissions #when session deleted #then subsequent writes are blocked", async () => {
|
||||
const existingFile = createFile("cleanup.txt")
|
||||
const sessionID = "ses_cleanup"
|
||||
|
||||
// establish permission by reading the existing file
|
||||
await invoke({
|
||||
tool: "read",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile },
|
||||
})
|
||||
|
||||
// sanity check: write should be allowed while the session is active
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile, content: "first write" },
|
||||
})
|
||||
).resolves.toBeDefined()
|
||||
|
||||
// delete the session to trigger cleanup of any stored permissions/state
|
||||
await invoke({
|
||||
tool: "session.deleted",
|
||||
sessionID,
|
||||
outputArgs: {},
|
||||
})
|
||||
|
||||
// after session deletion, the previous permissions must no longer apply
|
||||
await expect(
|
||||
invoke({
|
||||
tool: "write",
|
||||
sessionID,
|
||||
outputArgs: { filePath: existingFile, content: "second write after delete" },
|
||||
})
|
||||
).rejects.toThrow(BLOCK_MESSAGE)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -383,3 +383,55 @@ type EventInput = { event: { type: string; properties?: Record<string, unknown>
|
||||
expect(dispatchCalls[1].event.type).toBe("session.idle")
|
||||
})
|
||||
})
|
||||
|
||||
describe("createEventHandler - event forwarding", () => {
|
||||
it("forwards session.deleted to write-existing-file-guard hook", async () => {
|
||||
//#given
|
||||
const forwardedEvents: EventInput[] = []
|
||||
const disconnectedSessions: string[] = []
|
||||
const deletedSessions: string[] = []
|
||||
const eventHandler = createEventHandler({
|
||||
ctx: {} as never,
|
||||
pluginConfig: {} as never,
|
||||
firstMessageVariantGate: {
|
||||
markSessionCreated: () => {},
|
||||
clear: () => {},
|
||||
},
|
||||
managers: {
|
||||
skillMcpManager: {
|
||||
disconnectSession: async (sessionID: string) => {
|
||||
disconnectedSessions.push(sessionID)
|
||||
},
|
||||
},
|
||||
tmuxSessionManager: {
|
||||
onSessionCreated: async () => {},
|
||||
onSessionDeleted: async ({ sessionID }: { sessionID: string }) => {
|
||||
deletedSessions.push(sessionID)
|
||||
},
|
||||
},
|
||||
} as never,
|
||||
hooks: {
|
||||
writeExistingFileGuard: {
|
||||
event: async (input: EventInput) => {
|
||||
forwardedEvents.push(input)
|
||||
},
|
||||
},
|
||||
} as never,
|
||||
})
|
||||
const sessionID = "ses_forward_delete_event"
|
||||
|
||||
//#when
|
||||
await eventHandler({
|
||||
event: {
|
||||
type: "session.deleted",
|
||||
properties: { info: { id: sessionID } },
|
||||
},
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(forwardedEvents.length).toBe(1)
|
||||
expect(forwardedEvents[0]?.event.type).toBe("session.deleted")
|
||||
expect(disconnectedSessions).toEqual([sessionID])
|
||||
expect(deletedSessions).toEqual([sessionID])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -127,6 +127,7 @@ export function createEventHandler(args: {
|
||||
await Promise.resolve(hooks.ralphLoop?.event?.(input))
|
||||
await Promise.resolve(hooks.stopContinuationGuard?.event?.(input))
|
||||
await Promise.resolve(hooks.compactionTodoPreserver?.event?.(input))
|
||||
await Promise.resolve(hooks.writeExistingFileGuard?.event?.(input))
|
||||
await Promise.resolve(hooks.atlasHook?.handler?.(input))
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import { log } from "./logger"
|
||||
import * as connectedProvidersCache from "./connected-providers-cache"
|
||||
import { fuzzyMatchModel } from "./model-availability"
|
||||
import type { FallbackEntry } from "./model-requirements"
|
||||
import { transformModelForProvider } from "./provider-model-id-transform"
|
||||
|
||||
export type ModelResolutionRequest = {
|
||||
intent?: {
|
||||
@@ -85,10 +86,13 @@ export function resolveModelPipeline(
|
||||
if (parts.length >= 2) {
|
||||
const provider = parts[0]
|
||||
if (connectedProviders.includes(provider)) {
|
||||
const modelName = parts.slice(1).join("/")
|
||||
const transformedModel = `${provider}/${transformModelForProvider(provider, modelName)}`
|
||||
log("Model resolved via category default (connected provider)", {
|
||||
model: normalizedCategoryDefault,
|
||||
model: transformedModel,
|
||||
original: normalizedCategoryDefault,
|
||||
})
|
||||
return { model: normalizedCategoryDefault, provenance: "category-default", attempted }
|
||||
return { model: transformedModel, provenance: "category-default", attempted }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -108,10 +112,11 @@ export function resolveModelPipeline(
|
||||
for (const entry of fallbackChain) {
|
||||
for (const provider of entry.providers) {
|
||||
if (connectedSet.has(provider)) {
|
||||
const model = `${provider}/${entry.model}`
|
||||
const transformedModelId = transformModelForProvider(provider, entry.model)
|
||||
const model = `${provider}/${transformedModelId}`
|
||||
log("Model resolved via fallback chain (connected provider)", {
|
||||
provider,
|
||||
model: entry.model,
|
||||
model: transformedModelId,
|
||||
variant: entry.variant,
|
||||
})
|
||||
return {
|
||||
|
||||
@@ -543,7 +543,8 @@ describe("resolveModelWithFallback", () => {
|
||||
const result = resolveModelWithFallback(input)
|
||||
|
||||
// then - should use github-copilot (second provider) since google not connected
|
||||
expect(result!.model).toBe("github-copilot/gemini-3-pro")
|
||||
// model name is transformed to preview variant for github-copilot provider
|
||||
expect(result!.model).toBe("github-copilot/gemini-3-pro-preview")
|
||||
expect(result!.source).toBe("provider-fallback")
|
||||
cacheSpy.mockRestore()
|
||||
})
|
||||
@@ -795,8 +796,82 @@ describe("resolveModelWithFallback", () => {
|
||||
// when
|
||||
const result = resolveModelWithFallback(input)
|
||||
|
||||
// then - should use categoryDefaultModel since google is connected
|
||||
expect(result!.model).toBe("google/gemini-3-pro")
|
||||
// then - should use transformed categoryDefaultModel since google is connected
|
||||
expect(result!.model).toBe("google/gemini-3-pro-preview")
|
||||
expect(result!.source).toBe("category-default")
|
||||
cacheSpy.mockRestore()
|
||||
})
|
||||
|
||||
test("transforms gemini-3-flash in categoryDefaultModel for google connected provider", () => {
|
||||
// given - google connected, category default uses gemini-3-flash
|
||||
const cacheSpy = spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(["google"])
|
||||
const input: ExtendedModelResolutionInput = {
|
||||
categoryDefaultModel: "google/gemini-3-flash",
|
||||
availableModels: new Set(),
|
||||
systemDefaultModel: "anthropic/claude-sonnet-4-5",
|
||||
}
|
||||
|
||||
// when
|
||||
const result = resolveModelWithFallback(input)
|
||||
|
||||
// then - gemini-3-flash should be transformed to gemini-3-flash-preview
|
||||
expect(result!.model).toBe("google/gemini-3-flash-preview")
|
||||
expect(result!.source).toBe("category-default")
|
||||
cacheSpy.mockRestore()
|
||||
})
|
||||
|
||||
test("does not double-transform categoryDefaultModel already containing -preview", () => {
|
||||
// given - category default already has -preview suffix
|
||||
const cacheSpy = spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(["google"])
|
||||
const input: ExtendedModelResolutionInput = {
|
||||
categoryDefaultModel: "google/gemini-3-pro-preview",
|
||||
availableModels: new Set(),
|
||||
systemDefaultModel: "anthropic/claude-sonnet-4-5",
|
||||
}
|
||||
|
||||
// when
|
||||
const result = resolveModelWithFallback(input)
|
||||
|
||||
// then - should NOT become gemini-3-pro-preview-preview
|
||||
expect(result!.model).toBe("google/gemini-3-pro-preview")
|
||||
expect(result!.source).toBe("category-default")
|
||||
cacheSpy.mockRestore()
|
||||
})
|
||||
|
||||
test("transforms gemini-3-pro in fallback chain for google connected provider", () => {
|
||||
// given - google connected, fallback chain has gemini-3-pro
|
||||
const cacheSpy = spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(["google"])
|
||||
const input: ExtendedModelResolutionInput = {
|
||||
fallbackChain: [
|
||||
{ providers: ["google", "github-copilot"], model: "gemini-3-pro" },
|
||||
],
|
||||
availableModels: new Set(),
|
||||
systemDefaultModel: "anthropic/claude-sonnet-4-5",
|
||||
}
|
||||
|
||||
// when
|
||||
const result = resolveModelWithFallback(input)
|
||||
|
||||
// then - should transform to preview variant for google provider
|
||||
expect(result!.model).toBe("google/gemini-3-pro-preview")
|
||||
expect(result!.source).toBe("provider-fallback")
|
||||
cacheSpy.mockRestore()
|
||||
})
|
||||
|
||||
test("passes through non-gemini-3 models for google connected provider", () => {
|
||||
// given - google connected, category default uses gemini-2.5-flash (no transform needed)
|
||||
const cacheSpy = spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(["google"])
|
||||
const input: ExtendedModelResolutionInput = {
|
||||
categoryDefaultModel: "google/gemini-2.5-flash",
|
||||
availableModels: new Set(),
|
||||
systemDefaultModel: "anthropic/claude-sonnet-4-5",
|
||||
}
|
||||
|
||||
// when
|
||||
const result = resolveModelWithFallback(input)
|
||||
|
||||
// then - should pass through unchanged
|
||||
expect(result!.model).toBe("google/gemini-2.5-flash")
|
||||
expect(result!.source).toBe("category-default")
|
||||
cacheSpy.mockRestore()
|
||||
})
|
||||
|
||||
18
src/shared/provider-model-id-transform.ts
Normal file
18
src/shared/provider-model-id-transform.ts
Normal file
@@ -0,0 +1,18 @@
|
||||
export function transformModelForProvider(provider: string, model: string): string {
|
||||
if (provider === "github-copilot") {
|
||||
return model
|
||||
.replace("claude-opus-4-6", "claude-opus-4.6")
|
||||
.replace("claude-sonnet-4-6", "claude-sonnet-4.6")
|
||||
.replace("claude-sonnet-4-5", "claude-sonnet-4.5")
|
||||
.replace("claude-haiku-4-5", "claude-haiku-4.5")
|
||||
.replace("claude-sonnet-4", "claude-sonnet-4")
|
||||
.replace(/gemini-3-pro(?!-)/g, "gemini-3-pro-preview")
|
||||
.replace(/gemini-3-flash(?!-)/g, "gemini-3-flash-preview")
|
||||
}
|
||||
if (provider === "google") {
|
||||
return model
|
||||
.replace(/gemini-3-pro(?!-)/g, "gemini-3-pro-preview")
|
||||
.replace(/gemini-3-flash(?!-)/g, "gemini-3-flash-preview")
|
||||
}
|
||||
return model
|
||||
}
|
||||
@@ -78,7 +78,7 @@ export function createBackgroundOutput(manager: BackgroundOutputManager, client:
|
||||
}
|
||||
|
||||
const isActive = task.status === "pending" || task.status === "running"
|
||||
const fullSession = args.full_session ?? isActive
|
||||
const fullSession = args.full_session ?? false
|
||||
const includeThinking = isActive || (args.include_thinking ?? false)
|
||||
const includeToolResults = isActive || (args.include_tool_results ?? false)
|
||||
|
||||
|
||||
@@ -232,7 +232,7 @@ describe("background_output full_session", () => {
|
||||
expect(output).toContain("Has more: true")
|
||||
})
|
||||
|
||||
test("defaults to full_session when task is running", async () => {
|
||||
test("defaults to compact status when task is running", async () => {
|
||||
// #given
|
||||
const task = createTask({ status: "running" })
|
||||
const manager = createMockManager(task)
|
||||
@@ -242,6 +242,21 @@ describe("background_output full_session", () => {
|
||||
// #when
|
||||
const output = await tool.execute({ task_id: "task-1" }, mockContext)
|
||||
|
||||
// #then
|
||||
expect(output).toContain("# Task Status")
|
||||
expect(output).not.toContain("# Full Session Output")
|
||||
})
|
||||
|
||||
test("returns full session when explicitly requested for running task", async () => {
|
||||
// #given
|
||||
const task = createTask({ status: "running" })
|
||||
const manager = createMockManager(task)
|
||||
const client = createMockClient({})
|
||||
const tool = createBackgroundOutput(manager, client)
|
||||
|
||||
// #when
|
||||
const output = await tool.execute({ task_id: "task-1", full_session: true }, mockContext)
|
||||
|
||||
// #then
|
||||
expect(output).toContain("# Full Session Output")
|
||||
})
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { FallbackEntry } from "../../shared/model-requirements"
|
||||
import { fuzzyMatchModel } from "../../shared/model-availability"
|
||||
import { transformModelForProvider } from "../../shared/provider-model-id-transform"
|
||||
|
||||
function normalizeModel(model?: string): string | undefined {
|
||||
const trimmed = model?.trim()
|
||||
@@ -38,7 +39,8 @@ export function resolveModelForDelegateTask(input: {
|
||||
const first = fallbackChain[0]
|
||||
const provider = first?.providers?.[0]
|
||||
if (provider) {
|
||||
return { model: `${provider}/${first.model}`, variant: first.variant }
|
||||
const transformedModelId = transformModelForProvider(provider, first.model)
|
||||
return { model: `${provider}/${transformedModelId}`, variant: first.variant }
|
||||
}
|
||||
} else {
|
||||
for (const entry of fallbackChain) {
|
||||
|
||||
Reference in New Issue
Block a user