fix(bridge): 4 race condition fixes for approval lifecycle
This commit is contained in:
@@ -464,3 +464,16 @@
|
|||||||
- **원인**: Discord Gateway가 WebSocket 불안정 시 `MESSAGE_CREATE` 이벤트를 중복 전달 (known discord.py issue). 봇 프로세스 1개, 코드상 `on_message` 1회 실행 로직이지만 이벤트 자체가 2번 도착
|
- **원인**: Discord Gateway가 WebSocket 불안정 시 `MESSAGE_CREATE` 이벤트를 중복 전달 (known discord.py issue). 봇 프로세스 1개, 코드상 `on_message` 1회 실행 로직이지만 이벤트 자체가 2번 도착
|
||||||
- **해결**: `on_message`에 `_processed_message_ids: set[int]` (bounded 200개) 중복 방지 추가
|
- **해결**: `on_message`에 `_processed_message_ids: set[int]` (bounded 200개) 중복 방지 추가
|
||||||
- **주의**: Gateway reconnection, RESUME 실패 시 발생 빈도 증가. message ID 기반 dedup이 가장 확실한 방어
|
- **주의**: Gateway reconnection, RESUME 실패 시 발생 빈도 증가. message ID 기반 dedup이 가장 확실한 방어
|
||||||
|
|
||||||
|
### [2026-03-15] HTML 패치 멀티 인스턴스 race condition — 화면 파괴
|
||||||
|
- **증상**: Extension 패치 후 AG 재시작 시 전체 화면 날아감 (빈 화면/깨진 레이아웃)
|
||||||
|
- **원인**: 2+ Extension 인스턴스가 `setupApprovalObserver()`에서 동시에 같은 `workbench.html`/`workbench-jetski-agent.html`에 `readFileSync`/`writeFileSync` → 0-byte 파일 또는 부분 데이터 → 영구 손상
|
||||||
|
- **해결**: `.patch-lock` 파일 기반 cross-instance lock 추가 (30초 stale 판정). Lock 취득 실패 시 패치 skip
|
||||||
|
- **주의**: Lock은 "방지"에 해당. 기존 `.orig` 백업은 "복구"에 해당. 둘 다 유지해야 함. Lock 파일 경로 = `scriptDir/.patch-lock`
|
||||||
|
|
||||||
|
### [2026-03-15] 로컬 승인 ↔ Discord 승인 교차 race condition
|
||||||
|
- **증상**: AG에서 직접 Run 클릭 후 Discord 승인 요청이 "완료됨" 표시 안 됨. Discord에서도 뒤늦게 클릭 시 이미 완료된 step에 RPC 실행 → 에러 스팸
|
||||||
|
- **원인 1**: auto_resolve가 pending 상태만 변경하고 Discord에 알림 없음 → `writeChatSnapshot()` 추가
|
||||||
|
- **원인 2**: `processResponseFile()`이 pending의 `auto_resolved`/`expired` 상태를 체크하지 않음 → 상태 확인 후 skip 로직 추가
|
||||||
|
- **원인 3**: Bot의 auto_resolved 스캐너가 `discord_message_id`에만 의존 — Extension은 이 값을 모름 → `_approval_messages` dict (rid→msg_id) 추가, fallback 조회
|
||||||
|
- **주의**: `processResponseFile` L2534의 `lastPendingStepIndex = -1` 리셋이 Discord 승인 경로에서 auto_resolve 중복 진입을 방지하는 핵심 gate. 이 줄을 삭제하면 중복 알림 발생
|
||||||
|
|||||||
8
bot.py
8
bot.py
@@ -181,6 +181,7 @@ class GravityBot(commands.Bot):
|
|||||||
self.guild: discord.Guild | None = None
|
self.guild: discord.Guild | None = None
|
||||||
self.auto_approve_projects: set[str] = set() # projects with auto-approve enabled
|
self.auto_approve_projects: set[str] = set() # projects with auto-approve enabled
|
||||||
self._processed_message_ids: set[int] = set() # dedup for Gateway event replay
|
self._processed_message_ids: set[int] = set() # dedup for Gateway event replay
|
||||||
|
self._approval_messages: dict[str, int] = {} # FIX #4: request_id → discord message_id (for auto_resolved lookup)
|
||||||
self.gateway = None # Set by main.py in gateway mode
|
self.gateway = None # Set by main.py in gateway mode
|
||||||
|
|
||||||
def _write_command(self, project: str, text: str, **kwargs):
|
def _write_command(self, project: str, text: str, **kwargs):
|
||||||
@@ -615,7 +616,9 @@ class GravityBot(commands.Bot):
|
|||||||
try:
|
try:
|
||||||
data = json.loads(f.read_text(encoding="utf-8-sig"))
|
data = json.loads(f.read_text(encoding="utf-8-sig"))
|
||||||
if data.get("status") == "auto_resolved":
|
if data.get("status") == "auto_resolved":
|
||||||
msg_id = data.get("discord_message_id", 0)
|
rid = data.get("request_id", "")
|
||||||
|
# FIX #5: Use _approval_messages as fallback when discord_message_id is 0
|
||||||
|
msg_id = data.get("discord_message_id", 0) or self._approval_messages.get(rid, 0)
|
||||||
project = data.get("project_name", Config.PROJECT_NAME)
|
project = data.get("project_name", Config.PROJECT_NAME)
|
||||||
if msg_id:
|
if msg_id:
|
||||||
channel = await self._get_channel(project)
|
channel = await self._get_channel(project)
|
||||||
@@ -634,6 +637,8 @@ class GravityBot(commands.Bot):
|
|||||||
f.unlink()
|
f.unlink()
|
||||||
self._deferred_ids.pop(data.get("request_id", ""), None)
|
self._deferred_ids.pop(data.get("request_id", ""), None)
|
||||||
self._sent_commands.pop(data.get("request_id", ""), None)
|
self._sent_commands.pop(data.get("request_id", ""), None)
|
||||||
|
self._approval_messages.pop(data.get("request_id", ""), None)
|
||||||
|
self._sent_approval_ids.discard(data.get("request_id", ""))
|
||||||
except (json.JSONDecodeError, OSError):
|
except (json.JSONDecodeError, OSError):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
@@ -765,6 +770,7 @@ class GravityBot(commands.Bot):
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
logger.info(f"Sent approval request: {request.request_id[:12]}")
|
logger.info(f"Sent approval request: {request.request_id[:12]}")
|
||||||
|
self._approval_messages[request.request_id] = msg.id # FIX #4: Track msg_id for auto_resolved lookup
|
||||||
|
|
||||||
# ─── Discord → IDE Text Relay ─────────────────────────────────────
|
# ─── Discord → IDE Text Relay ─────────────────────────────────────
|
||||||
|
|
||||||
|
|||||||
@@ -4,3 +4,4 @@
|
|||||||
|---|------|------|------|------|
|
|---|------|------|------|------|
|
||||||
| 001 | 07:00~08:16 | 승인 신호 누락 진단 & 5건 버그 수정 (DEDUP collision, fs.watch fail, default 보호, auto 확인, msg dedup) | `40e3cd5` | ✅ |
|
| 001 | 07:00~08:16 | 승인 신호 누락 진단 & 5건 버그 수정 (DEDUP collision, fs.watch fail, default 보호, auto 확인, msg dedup) | `40e3cd5` | ✅ |
|
||||||
| 002 | 08:25~08:31 | Extension v0.3.10 버전 범프 & VSIX 빌드 | `10caae1` | ✅ |
|
| 002 | 08:25~08:31 | Extension v0.3.10 버전 범프 & VSIX 빌드 | `10caae1` | ✅ |
|
||||||
|
| 003 | 10:00~10:41 | 승인 라이프사이클 race condition 4건 수정 (HTML lock, pending status skip, auto_resolve Discord 알림, Bot approval_messages) | `015aa79` | ✅ |
|
||||||
|
|||||||
BIN
extension/gravity-bridge-0.3.10.zip
Normal file
BIN
extension/gravity-bridge-0.3.10.zip
Normal file
Binary file not shown.
@@ -481,6 +481,24 @@ async function setupApprovalObserver() {
|
|||||||
requiredScript: 'jetskiAgent.js', // JS entry point
|
requiredScript: 'jetskiAgent.js', // JS entry point
|
||||||
},
|
},
|
||||||
];
|
];
|
||||||
|
// ── FIX #1: File lock to prevent multi-instance HTML patching race ──
|
||||||
|
const lockFile = path.join(scriptDir, '.patch-lock');
|
||||||
|
let lockAcquired = false;
|
||||||
|
try {
|
||||||
|
if (fs.existsSync(lockFile)) {
|
||||||
|
const lockAge = Date.now() - fs.statSync(lockFile).mtimeMs;
|
||||||
|
if (lockAge < 30_000) {
|
||||||
|
logToFile(`[OBSERVER] another instance is patching (lock age=${Math.round(lockAge/1000)}s) — skipping`);
|
||||||
|
return; // Exit setupApprovalObserver entirely
|
||||||
|
}
|
||||||
|
logToFile(`[OBSERVER] stale lock (age=${Math.round(lockAge/1000)}s) — force-acquiring`);
|
||||||
|
}
|
||||||
|
fs.writeFileSync(lockFile, JSON.stringify({ pid: process.pid, ts: Date.now() }), 'utf-8');
|
||||||
|
lockAcquired = true;
|
||||||
|
} catch (lockErr: any) {
|
||||||
|
logToFile(`[OBSERVER] lock acquire error: ${lockErr.message} — proceeding anyway`);
|
||||||
|
}
|
||||||
|
|
||||||
for (const spec of htmlFileSpecs) {
|
for (const spec of htmlFileSpecs) {
|
||||||
const htmlPath = path.join(scriptDir, spec.name);
|
const htmlPath = path.join(scriptDir, spec.name);
|
||||||
const backupPath = htmlPath + '.orig';
|
const backupPath = htmlPath + '.orig';
|
||||||
@@ -584,6 +602,12 @@ async function setupApprovalObserver() {
|
|||||||
logToFile(`[OBSERVER] ${spec.name} patch error: ${e.message}`);
|
logToFile(`[OBSERVER] ${spec.name} patch error: ${e.message}`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Release patch lock
|
||||||
|
if (lockAcquired) {
|
||||||
|
try { fs.unlinkSync(lockFile); } catch { }
|
||||||
|
logToFile('[OBSERVER] patch lock released');
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// 4. Update product.json checksums so vscode-file:// serves our patched files
|
// 4. Update product.json checksums so vscode-file:// serves our patched files
|
||||||
@@ -1918,6 +1942,8 @@ function setupMonitor() {
|
|||||||
pd.status = 'auto_resolved';
|
pd.status = 'auto_resolved';
|
||||||
fs.writeFileSync(pfPath, JSON.stringify(pd, null, 2), 'utf-8');
|
fs.writeFileSync(pfPath, JSON.stringify(pd, null, 2), 'utf-8');
|
||||||
logToFile(`[AUTO-RESOLVE] step=${lastPendingStepIndex} progressed → marked ${pf} (age=${Math.round(ageMs/1000)}s)`);
|
logToFile(`[AUTO-RESOLVE] step=${lastPendingStepIndex} progressed → marked ${pf} (age=${Math.round(ageMs/1000)}s)`);
|
||||||
|
// FIX #3: Notify Discord that user approved locally
|
||||||
|
writeChatSnapshot(`✅ **AG에서 직접 승인됨** (step ${lastPendingStepIndex})\n\n\`${(pd.command || '').substring(0, 200)}\``);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (e: any) { logToFile(`[AUTO-RESOLVE] error: ${e.message}`); }
|
} catch (e: any) { logToFile(`[AUTO-RESOLVE] error: ${e.message}`); }
|
||||||
@@ -2452,6 +2478,14 @@ async function processResponseFile(filePath: string) {
|
|||||||
if (fs.existsSync(pendingFile)) {
|
if (fs.existsSync(pendingFile)) {
|
||||||
try {
|
try {
|
||||||
const pending = JSON.parse(fs.readFileSync(pendingFile, 'utf-8'));
|
const pending = JSON.parse(fs.readFileSync(pendingFile, 'utf-8'));
|
||||||
|
|
||||||
|
// FIX #2: Skip if pending was already resolved locally (auto_resolve or expired)
|
||||||
|
if (pending.status === 'auto_resolved' || pending.status === 'expired') {
|
||||||
|
logToFile(`[RESPONSE] SKIP — pending already ${pending.status} (rid=${resp.request_id})`);
|
||||||
|
try { fs.unlinkSync(filePath); } catch { }
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
sessionId = pending.conversation_id || '';
|
sessionId = pending.conversation_id || '';
|
||||||
isDomObserver = pending.auto_detected === true
|
isDomObserver = pending.auto_detected === true
|
||||||
|| pending.source === 'dom_observer';
|
|| pending.source === 'dom_observer';
|
||||||
|
|||||||
Reference in New Issue
Block a user