fix(extension): diff_review steps=[] race condition — in-memory metadata cache (v0.3.13)

Root cause: Collector deletes pending file before Extension reads edit_step_indices. Fix: diffReviewMetadata Map caches step indices in Extension memory. Known issue added. Devlog entry 003.
2026-03-16 16:09:04 +09:00
parent 12a1cf8692
commit 9ef2c3f07c
9 changed files with 163 additions and 43 deletions
--- a/.agents/references/known-issues.md
+++ b/.agents/references/known-issues.md
@@ -563,3 +563,9 @@
 - **해결**: diff_review pending 생성을 `setTimeout(8000)`으로 지연. 변수는 closure로 캡처 (`capturedSessionId`, `capturedStepCount`, 파일 목록). tracking 배열은 즉시 리셋
 - **주의**: 8초는 snapshot_scanner + Collector + Gateway + Bot 전체 경로의 전파 시간을 고려한 값. 너무 짧으면 여전히 버튼이 먼저 도착

+### [2026-03-16] diff_review AcknowledgeCascadeCodeEdit steps=[] — Collector pending 삭제 race
+- **증상**: Discord에서 Accept all 클릭 → `AcknowledgeCascadeCodeEdit(steps=[])` → RPC SUCCESS `{}` 반환 → AG에서 diff review 바가 사라지지 않음 (실제 no-op)
+- **원인**: Collector가 Gateway response를 받으면 로컬 pending 파일을 즉시 삭제 (`collector.py L259`). Extension `processResponseFile`이 pending 파일에서 `edit_step_indices`를 읽으려 할 때 파일 이미 없음 → `trackedSteps=[]`
+- **해결**: `diffReviewMetadata` 인메모리 Map 추가 (extension.ts L57). `writePendingApproval`에서 diff_review pending 생성 시 `edit_step_indices` + `modified_files`를 메모리에 캐시. `processResponseFile`에서 메모리 먼저 조회, pending 파일은 fallback
+- **주의**: 인메모리 캐시는 Extension Reload 시 소실됨. 그러나 diff_review pending 자체가 RUNNING→IDLE 전환 시 새로 생성되므로, 리로드 후에도 새 diff_review는 정상 동작. `bridge.py write_response()` L461의 pending 삭제도 동일 문제를 유발하지만 remote 모드에서는 Collector 경로만 해당
+
--- a/docs/devlog/2026-03-16.md
+++ b/docs/devlog/2026-03-16.md
@@ -4,3 +4,4 @@
 |---|------|------|------|------|
 | 001 | 07:30~11:10 | 승인 상태 관리 근본 원인 분석 + v0.3.12 수정 (sawRunningAfterPending gate) + approval-flow.md 시스템 Flow 문서 + known-issues 2건 추가 | `2d9fe96` | ✅ |
 | 002 | 13:25~14:20 | diff_review 핸들러 2-strategy 리팩토링 + 배포 불일치 발견/수정 + pending 순서 8초 지연 + 1차 테스트 (버튼 OK, RPC 미배포→재배포) + known-issues 2건 | `f302984` | 🔧 |
+| 003 | 15:18~16:06 | diff_review steps=[] 근본 원인 분석 + diffReviewMetadata 인메모리 캐시 수정 (v0.3.13) + 2차 E2E 테스트 (원인 확인) + known-issues 1건 | `00b9491` | 🔧 |
--- a/docs/devlog/entries/20260316-003.md
+++ b/docs/devlog/entries/20260316-003.md
@@ -0,0 +1,31 @@
+# diff_review steps=[] 근본 원인 분석 + v0.3.13 수정
+
+- **시간**: 2026-03-16 15:18~16:06
+- **Commit**: `00b9491`
+- **Vikunja**: #384 diff_review 원격 승인 테스트 → 진행중
+
+## 분석 결과
+
+2차 E2E 테스트에서 `AcknowledgeCascadeCodeEdit(steps=[])` → `SUCCESS: {}` 반환 확인.
+RPC 자체는 성공하지만 빈 step 배열로 호출하면 no-op — AG의 diff review 바 유지됨.
+
+**근본 원인 체인:**
+1. Extension이 `writePendingApproval()`로 diff_review pending 생성 (edit_step_indices=[86] 포함)
+2. Collector가 Gateway에 전달 후 로컬 pending 파일을 삭제 (collector.py L259)
+3. Discord Accept 클릭 → Gateway → Collector가 response 파일 작성
+4. Extension의 `processResponseFile`이 pending 파일에서 `edit_step_indices` 읽으려 함 → **파일 이미 없음**
+5. `trackedSteps=[]` → `AcknowledgeCascadeCodeEdit(steps=[])` → no-op
+
+## 수정 (v0.3.13)
+
+- `diffReviewMetadata` 인메모리 Map 추가 (extension.ts L57)
+- `writePendingApproval`에서 diff_review pending 생성 시 메타데이터를 메모리에 캐시
+- `processResponseFile`에서 메모리 먼저 조회, pending 파일은 fallback
+- `modifiedFiles` 변수를 Strategy 1/2 공유 스코프로 호이스팅
+
+## 미완료
+
+- **AG 풀 재시작 후 3차 E2E 테스트** 필요
+  - `[DIFF-REVIEW-CACHE]` 로그 확인
+  - `[DIFF-REVIEW-RPC]` steps 배열에 실제 인덱스 포함 확인
+  - AG에서 diff review 바 사라지는지 확인
--- a/extension/out/extension.js
+++ b/extension/out/extension.js
@@ -87,6 +87,9 @@ const sentPendingIds = new Set();
 // Map<string, number> = `${conversationId}:${stepIndex}` → creation timestamp
 const recentPendingSteps = new Map();
 const PENDING_MEMORY_TTL_MS = 60_000; // 60 seconds memory retention
+// In-memory cache for diff_review metadata (survives pending file deletion by Collector).
+// Map<request_id, { edit_step_indices, modified_files }>
+const diffReviewMetadata = new Map();
 // ─── Project Detection ───
 function detectProjectName() {
    const config = vscode.workspace.getConfiguration('gravityBridge');
@@ -2622,12 +2625,22 @@ async function processResponseFile(filePath) {
            logToFile(`[RESPONSE] diff_review → ${isAccept ? 'ACCEPT' : 'REJECT'} (btnIdx=${btnIdx})`);
            let diffReviewDone = false;
            const targetSession = sessionId || activeSessionId;
+            let modifiedFiles = []; // shared between Strategy 1 and 2
            // ── Strategy 1: AcknowledgeCascadeCodeEdit RPC ──
            // Accept/reject all pending code edits via protocol (no UI interaction needed)
            if (sdk) {
                try {
-                    // Get tracked step indices from pending data (or use all recent edit steps)
+                    // Get tracked step indices from in-memory cache FIRST (pending file may be deleted by Collector)
                    const trackedSteps = [];
+                    const memMeta = diffReviewMetadata.get(resp.request_id);
+                    if (memMeta) {
+                        trackedSteps.push(...memMeta.edit_step_indices);
+                        modifiedFiles = memMeta.modified_files;
+                        diffReviewMetadata.delete(resp.request_id); // cleanup
+                        logToFile(`[DIFF-REVIEW-RPC] loaded from memory: steps=[${trackedSteps.join(',')}] files=${modifiedFiles.length}`);
+                    }
+                    else {
+                        // Fallback: try pending file (may already be deleted)
                        const pendingDir = path.join(bridgePath, 'pending');
                        try {
                            const pendingFile = path.join(pendingDir, `${resp.request_id}.json`);
@@ -2635,9 +2648,12 @@ async function processResponseFile(filePath) {
                                const pd = JSON.parse(fs.readFileSync(pendingFile, 'utf-8'));
                                if (pd.edit_step_indices)
                                    trackedSteps.push(...pd.edit_step_indices);
+                                if (pd.modified_files)
+                                    modifiedFiles = pd.modified_files;
                            }
                        }
                        catch { }
+                    }
                    // If no tracked steps, use the step_index from the pending
                    if (trackedSteps.length === 0 && pendingStepIndex > 0) {
                        trackedSteps.push(pendingStepIndex);
@@ -2665,17 +2681,7 @@ async function processResponseFile(filePath) {
                        await new Promise(r => setTimeout(r, 500));
                    }
                    catch { }
-                    // Step 2b: Find modified files from pending data
-                    let modifiedFiles = [];
-                    try {
-                        const pendingFile = path.join(bridgePath, 'pending', `${resp.request_id}.json`);
-                        if (fs.existsSync(pendingFile)) {
-                            const pd = JSON.parse(fs.readFileSync(pendingFile, 'utf-8'));
-                            if (pd.modified_files)
-                                modifiedFiles = pd.modified_files;
-                        }
-                    }
-                    catch { }
+                    // Step 2b: Use modifiedFiles from Strategy 1 (already loaded from memory/file above)
                    // Step 2c: Open and focus each modified file, then execute
                    if (modifiedFiles.length > 0) {
                        for (const filePath of modifiedFiles) {
@@ -2967,9 +2973,19 @@ function writePendingApproval(data) {
            ...(data.step_index !== undefined ? { step_index: data.step_index } : {}),
            ...(data.source ? { source: data.source } : {}),
            ...(buttons ? { buttons } : {}),
+            ...(data.modified_files ? { modified_files: data.modified_files } : {}),
+            ...(data.edit_step_indices && data.edit_step_indices.length > 0 ? { edit_step_indices: data.edit_step_indices } : {}),
        };
        fs.writeFileSync(path.join(pendingDir, `${id}.json`), JSON.stringify(payload, null, 2), 'utf-8');
        console.log(`Gravity Bridge: pending approval written → ${id}.json`);
+        // Cache diff_review metadata in-memory (survives pending file deletion by Collector/Bot)
+        if (data.step_type === 'diff_review' && (data.edit_step_indices?.length || data.modified_files?.length)) {
+            diffReviewMetadata.set(id, {
+                edit_step_indices: data.edit_step_indices || [],
+                modified_files: data.modified_files || [],
+            });
+            logToFile(`[DIFF-REVIEW-CACHE] stored metadata for rid=${id}: steps=[${(data.edit_step_indices || []).join(',')}] files=${(data.modified_files || []).length}`);
+        }
        // Record in memory dedup cache (survives file deletion by Collector/Bot)
        if (data.step_index !== undefined && data.conversation_id) {
            recentPendingSteps.set(`${data.conversation_id}:${data.step_index}`, nowMs);
--- a/extension/out/extension.js.map
+++ b/extension/out/extension.js.map
--- a/extension/package.json
+++ b/extension/package.json
@@ -2,7 +2,7 @@
    "name": "gravity-bridge",
    "displayName": "Gravity Bridge",
    "description": "Antigravity ↔ Discord 브리지 연동 확장",
-    "version": "0.3.12",
+    "version": "0.3.13",
    "publisher": "variet",
    "engines": {
        "vscode": "^1.100.0"
--- a/extension/src/extension.ts
+++ b/extension/src/extension.ts
@@ -53,6 +53,9 @@ const sentPendingIds = new Set<string>();
 // Map<string, number> = `${conversationId}:${stepIndex}` → creation timestamp
 const recentPendingSteps = new Map<string, number>();
 const PENDING_MEMORY_TTL_MS = 60_000; // 60 seconds memory retention
+// In-memory cache for diff_review metadata (survives pending file deletion by Collector).
+// Map<request_id, { edit_step_indices, modified_files }>
+const diffReviewMetadata = new Map<string, { edit_step_indices: number[]; modified_files: string[] }>();

 // ─── Project Detection ───

@@ -2431,7 +2434,7 @@ function setupMonitor() {
                        ],
                        modified_files: capturedModFiles,
                        edit_step_indices: capturedEditSteps,
-                    } as any);
+                    });
                }, 8000);
            }
            wasRunning = isRunning;
@@ -2592,21 +2595,32 @@ async function processResponseFile(filePath: string) {

            let diffReviewDone = false;
            const targetSession = sessionId || activeSessionId;
+            let modifiedFiles: string[] = [];  // shared between Strategy 1 and 2

            // ── Strategy 1: AcknowledgeCascadeCodeEdit RPC ──
            // Accept/reject all pending code edits via protocol (no UI interaction needed)
            if (sdk) {
                try {
-                    // Get tracked step indices from pending data (or use all recent edit steps)
+                    // Get tracked step indices from in-memory cache FIRST (pending file may be deleted by Collector)
                    const trackedSteps: number[] = [];
+                    const memMeta = diffReviewMetadata.get(resp.request_id);
+                    if (memMeta) {
+                        trackedSteps.push(...memMeta.edit_step_indices);
+                        modifiedFiles = memMeta.modified_files;
+                        diffReviewMetadata.delete(resp.request_id); // cleanup
+                        logToFile(`[DIFF-REVIEW-RPC] loaded from memory: steps=[${trackedSteps.join(',')}] files=${modifiedFiles.length}`);
+                    } else {
+                        // Fallback: try pending file (may already be deleted)
                        const pendingDir = path.join(bridgePath, 'pending');
                        try {
                            const pendingFile = path.join(pendingDir, `${resp.request_id}.json`);
                            if (fs.existsSync(pendingFile)) {
                                const pd = JSON.parse(fs.readFileSync(pendingFile, 'utf-8'));
                                if (pd.edit_step_indices) trackedSteps.push(...pd.edit_step_indices);
+                                if (pd.modified_files) modifiedFiles = pd.modified_files;
                            }
                        } catch { }
+                    }
                    
                    // If no tracked steps, use the step_index from the pending
                    if (trackedSteps.length === 0 && pendingStepIndex > 0) {
@@ -2636,15 +2650,7 @@ async function processResponseFile(filePath: string) {
                        await new Promise(r => setTimeout(r, 500));
                    } catch { }

-                    // Step 2b: Find modified files from pending data
-                    let modifiedFiles: string[] = [];
-                    try {
-                        const pendingFile = path.join(bridgePath, 'pending', `${resp.request_id}.json`);
-                        if (fs.existsSync(pendingFile)) {
-                            const pd = JSON.parse(fs.readFileSync(pendingFile, 'utf-8'));
-                            if (pd.modified_files) modifiedFiles = pd.modified_files;
-                        }
-                    } catch { }
+                    // Step 2b: Use modifiedFiles from Strategy 1 (already loaded from memory/file above)

                    // Step 2c: Open and focus each modified file, then execute
                    if (modifiedFiles.length > 0) {
@@ -2833,7 +2839,7 @@ function extractToolDescription(stepData: any, sessionTitle: string, stepIndex:
 }

 /** Write a pending approval file matching Bot's ApprovalRequest dataclass. */
-function writePendingApproval(data: { conversation_id: string; command: string; description: string; step_type?: string; step_index?: number; source?: string; buttons?: Array<{text: string; index: number}> }) {
+function writePendingApproval(data: { conversation_id: string; command: string; description: string; step_type?: string; step_index?: number; source?: string; buttons?: Array<{text: string; index: number}>; modified_files?: string[]; edit_step_indices?: number[] }) {
    try {
        const pendingDir = path.join(bridgePath, 'pending');
        if (!fs.existsSync(pendingDir)) { fs.mkdirSync(pendingDir, { recursive: true }); }
@@ -2922,9 +2928,19 @@ function writePendingApproval(data: { conversation_id: string; command: string;
            ...(data.step_index !== undefined ? { step_index: data.step_index } : {}),
            ...(data.source ? { source: data.source } : {}),
            ...(buttons ? { buttons } : {}),
+            ...(data.modified_files ? { modified_files: data.modified_files } : {}),
+            ...(data.edit_step_indices && data.edit_step_indices.length > 0 ? { edit_step_indices: data.edit_step_indices } : {}),
        };
        fs.writeFileSync(path.join(pendingDir, `${id}.json`), JSON.stringify(payload, null, 2), 'utf-8');
        console.log(`Gravity Bridge: pending approval written → ${id}.json`);
+        // Cache diff_review metadata in-memory (survives pending file deletion by Collector/Bot)
+        if (data.step_type === 'diff_review' && (data.edit_step_indices?.length || data.modified_files?.length)) {
+            diffReviewMetadata.set(id, {
+                edit_step_indices: data.edit_step_indices || [],
+                modified_files: data.modified_files || [],
+            });
+            logToFile(`[DIFF-REVIEW-CACHE] stored metadata for rid=${id}: steps=[${(data.edit_step_indices || []).join(',')}] files=${(data.modified_files || []).length}`);
+        }
        // Record in memory dedup cache (survives file deletion by Collector/Bot)
        if (data.step_index !== undefined && data.conversation_id) {
            recentPendingSteps.set(`${data.conversation_id}:${data.step_index}`, nowMs);
--- a/tests/test_diff_review.py
+++ b/tests/test_diff_review.py
@@ -0,0 +1,25 @@
+"""
+diff_review E2E 테스트용 임시 파일
+생성: 2026-03-16 14:44
+"""
+
+
+def hello(name: str = "World"):
+    """인사 메시지를 반환합니다."""
+    return f"Hello, {name}!"
+
+
+def add(a: int, b: int) -> int:
+    """두 숫자를 더합니다."""
+    return a + b
+
+
+def multiply(a: int, b: int) -> int:
+    """두 숫자를 곱합니다."""
+    return a * b
+
+
+if __name__ == "__main__":
+    print(hello("User"))
+    print(add(1, 2))
+    print(multiply(3, 4))
--- a/tests/test_diff_review_e2e.py
+++ b/tests/test_diff_review_e2e.py
@@ -0,0 +1,25 @@
+"""
+Diff Review E2E Test File
+Created to trigger diff_review flow in AG extension.
+This file write should be tracked by DIFF-TRACK in the extension.
+"""
+
+
+def test_diff_review():
+    """Test that diff review Accept/Reject works via Discord."""
+    # This is a dummy test file to verify the diff_review pipeline:
+    # 1. Extension detects this file write via step probe
+    # 2. On RUNNING→IDLE transition, creates diff_review pending
+    # 3. Discord shows Accept all / Reject all buttons
+    # 4. User clicks Accept → AcknowledgeCascadeCodeEdit RPC fires
+    assert True, "Diff review E2E test passed"
+
+
+def test_multi_file_tracking():
+    """Verify multiple file modifications are tracked together."""
+    results = {
+        "files_tracked": 0,
+        "pending_created": False,
+        "rpc_called": False,
+    }
+    return results