Issue Station
Sign In
Tools hereToolbox

openclaw - ✅(Solved) Fix auto-reply queue: drain finally deletes FOLLOWUP_QUEUES entry without identity check, orphaning post-/stop queue [4 pull requests, 1 participants]

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…
GitHub stats
openclaw/openclaw#68838•Fetched 2026-04-19 15:06:54
View on GitHub
Comments
0
Participants
1
Timeline
8
Reactions
0
Author
Feelw00Feelw00
Participants
Feelw00Feelw00
Timeline (top)
cross-referenced ×4referenced ×4

Root Cause

  • 외부 lock: rg "Mutex|Semaphore|AsyncLock" 매치 0건. queue 보호 락 없음.
  • AbortController: rg "AbortController|AbortSignal" 매치 0건. abort 전파 없음.
  • identity check: rg "FOLLOWUP_QUEUES\.get\(key\)\s*===|map\.get\(key\)\s*===" 매치 0건. finally 의 delete 에서 자신의 queue 가 여전히 map 의 entry 인지 확인하는 로직 부재.
  • 기존 테스트: queue.drain-restart.test.ts:236-260 은 callback 이 clearSessionQueues 이후 유지되지 않음을 검증하나, Q2 orphan 시나리오는 커버하지 않음.
  • 주변 코드 맥락: subagent-announce-queue.ts:62-72 resetAnnounceQueuesForTests 주석 ("Clearing the map alone isn't enough because drain loops capture queue by reference") 에서 maintainer 가 dangling reference 문제를 인지. 그러나 test-reset 경로에만 대응했을 뿐 production finally 의 identity 보호는 없음.
  • primary-path inversion: identity check (FOLLOWUP_QUEUES.get(key) === queue) 가 unconditional 하게 finally 에 존재해야 race 가 차단됨. 현재 부재 → race 재현 가능.

Fix Action

Fixed

PR fix notes

PR #68839: fix(auto-reply): guard FOLLOWUP_QUEUES delete against late drain finally

Description (problem / solution / changelog)

Summary

  • Problem: drain IIFE finally (drain.ts:263-271) unconditionally calls FOLLOWUP_QUEUES.delete(key) + clearFollowupDrainCallback(key) based on the key alone, without checking whether the captured queue still matches the map entry. Under the /stop + immediate followup sequence, a late-returning D1 finally deletes the map entry for a fresh Q2 and orphans it.
  • Why it matters: in production the /stop flow (commands-session-abort.ts:146 → clearSessionQueues) clears items in-place, but D1 may still be parked on await effectiveRunFollowup(...) at drain.ts:216/256. If a new message arrives before D1 resumes, Q2 is registered in the map and then silently dropped when D1's finally runs.
  • What changed: the finally now only removes the map entry and drain callback when FOLLOWUP_QUEUES.get(key) === queue. Mirrors the identity pattern noted in subagent-announce-queue.ts:62-64.
  • What did NOT change: drain flow, drop counting, re-schedule semantics, or map deletion by clearFollowupQueue / clearSessionQueues (those remain the authoritative map mutators).

Change Type

  • Bug fix

Scope

  • auto-reply

Linked Issue

Closes #68838

Root Cause

The finally block used key-only deletion, so it could not distinguish between "my queue still owns this slot" (delete is correct) and "someone replaced the slot while I was awaiting" (delete is wrong). Missing guardrail: identity check before the mutation.

Regression Test Plan

Added src/auto-reply/reply/queue/drain.identity-guard.test.ts:

  • Uses real enqueueFollowupRun / scheduleFollowupDrain / clearSessionQueues (no module mocks).
  • Parks D1 inside runFollowup on a Deferred gate (same pattern as queue.drain-restart.test.ts:207-234).
  • T2 enqueue uses restartIfIdle=false so D2 is NOT kicked — D1's finally is the only mutator that can touch the map entry.
  • Deterministic pre/post-fix differentiation:
    • pre-fix: FOLLOWUP_QUEUES.get(key) === undefined (Q2 orphaned), getFollowupQueueDepth === 0.
    • post-fix: FOLLOWUP_QUEUES.get(key) === Q2, getFollowupQueueDepth === 1.

Security Impact

None. No new permissions, secrets, network calls, or data scope change.

Repro + Verification

Environment: Node 22, macOS, pnpm 10.33.0.

Steps:

  1. `pnpm test src/auto-reply/reply/queue/drain.identity-guard.test.ts`

Expected (post-fix): 1 passed. Actual (pre-fix): `FOLLOWUP_QUEUES.get(key)` returned `undefined` when expected to be Q2 — confirms the orphan.

Evidence

  • Pre-fix run (fix stashed): 1 failed.
  • Post-fix run: 1 passed.
  • Full `src/auto-reply/reply/` suite: 100 files / 1082 tests passed.
  • `pnpm check` + `pnpm build` clean.

Human Verification

  • Verified drain.ts 263-271 identity guard via Read, confirmed line matches in current upstream HEAD.
  • Confirmed `subagent-announce-queue.ts:62-64` uses the same identity pattern as intentional design.
  • Confirmed `restartIfIdle=false` path (6th positional arg of `enqueueFollowupRun`) has a real production caller at `agent-runner.ts:1002-1009` (not synthetic).

Review Conversations

Greptile + Codex reviews will run on the PR; will respond to any flagged items.

Compatibility / Migration

None. Internal behavioral fix; no public API or config surface touched.

Risks and Mitigations

  • Risk: identity check could skip deletion in a legitimate case where the queue finished but was replaced by an identical-looking queue (same reference). Mitigation: JavaScript reference equality is strict — same reference means same queue instance, so this cannot be a false negative.
  • Risk: stale entries accumulate if the identity guard always fails. Mitigation: clearFollowupQueue (state.ts:86) and clearSessionQueues remain the primary map mutators and both operate by key; subsequent drains will naturally reach the finally with a matching identity once the replaced queue drains.

AI-assisted (fully tested). Generated via openclaw-audit pipeline (gatekeeper approve + post-harness cross-review 5/5 real-problem-real-fix + pre-pr cross-review 3/3 real-problem-real-fix).

Changed files

  • src/auto-reply/reply/queue/drain.identity-guard.test.ts (added, +114/-0)
  • src/auto-reply/reply/queue/drain.ts (modified, +7/-2)

PR #68908: fix(auto-reply): add identity guard in drain finally to prevent queue orphaning

Description (problem / solution / changelog)

Summary

The drain finally block in scheduleFollowupDrain deletes FOLLOWUP_QUEUES entries by key only, without verifying the current map entry is the same queue reference. If /stop or session reset clears the original queue and a new one is registered under the same key, the original drain finally will delete the new queue, orphaning it.

Fix

Add an identity check before deleting: if (FOLLOWUP_QUEUES.get(key) === queue). Only delete and clear callback if the current map entry matches the queue reference being drained.

Reproduction

  1. Message comes in → Q1 created, D1 drain starts
  2. /stop → clearFollowupQueue removes Q1 from map
  3. New message → Q2 registered under same key
  4. D1 finishes → finally deletes Q2 (wrong queue!)
  5. Q2 is now orphaned

Testing

  • /stop followed by new message no longer orphans the new queue
  • Normal drain operation unchanged when no race condition
  • /status queue depth reporting remains accurate

Closes #68838

Changed files

PR #68914: fix(auto-reply): add identity guard in drain finally to prevent queue orphaning

Description (problem / solution / changelog)

The drain finally block deletes FOLLOWUP_QUEUES entries by key only, without verifying the current map entry is the same queue reference. If /stop or session reset clears the original queue and a new one is registered under the same key, the original drain finally will delete the new queue. Add identity check: if (FOLLOWUP_QUEUES.get(key) === queue).

Closes #68838

Changed files

  • src/auto-reply/reply/queue/drain.ts (modified, +4/-2)

PR #68976: fix(auto-reply): add identity guard in drain finally

Description (problem / solution / changelog)

Add identity check before deleting queue.

Closes #68838

Changed files

  • src/auto-reply/reply/queue/drain.ts (modified, +4/-2)
RAW_BUFFERClick to expand / collapse

요약

auto-reply queue: drain finally 가 identity 검증 없이 map entry 를 삭제해 후속 queue 를 orphan 시킴

공통 패턴

단일 FIND 기반 single CAND. scheduleFollowupDrain 의 drain IIFE 는 finally 블록 (src/auto-reply/reply/queue/drain.ts:263-270) 에서 FOLLOWUP_QUEUES.delete(key) + clearFollowupDrainCallback(key) 를 호출한다. 이 두 호출은 key 만으로 제거하며 자신이 들고 있는 queue 참조가 현재 map 의 entry 와 동일한지 검증하지 않는다.

관련 FIND

  • FIND-auto-reply-concurrency-001: /stop 또는 session reset 이 drain 의 await 지점에서 clearSessionQueues → clearFollowupQueue(key) 를 호출하고, 이후 enqueueFollowupRun 이 새 Q2 를 등록하면, 원래 D1 의 finally 가 Q2 의 map entry 와 callback 까지 지워 Q2 가 orphan 되는 시나리오.

재현 시나리오 요약

  • T0: msg1 → Q1 생성 + D1 drain 시작 (await effectiveRunFollowup 에서 대기).
  • T1: /stop → clearFollowupQueue 가 Q1.items 를 in-place 비우고 map 에서 Q1 제거.
  • T2: msg2 → 새 Q2 를 map 에 등록, D2 kick.
  • T3-T4: D1 의 await 반환 → finally 가 L266 FOLLOWUP_QUEUES.delete(key) 실행 → 현재 map entry 인 Q2 가 삭제됨.
  • T5: getFollowupQueueDepth(key) → 0 반환 (실제 Q2.items 는 비어있지 않음).

영향

  • impact_hypothesis: wrong-output
  • /status observability 오염 (queue depth 0 으로 보고)
  • 동일 session key 에서 D2/D3 병렬 실행 가능 (per-session serialization 위반)
  • production caller (agent-runner.ts:1002) 는 runFollowup 을 항상 전달하므로 callback 손실은 일반 흐름에서 발생 안 함. 그러나 invariant 깨짐.

대응 방향 (제안만)

drain finally 에서 if (FOLLOWUP_QUEUES.get(key) === queue) { delete; clearCallback; } 형태의 identity guard. 구체는 SOL 단계에서 결정.

참고

  • 동일 패턴이 src/agents/subagent-announce-queue.ts:204 에도 존재하며 maintainer 주석 (L63-64) 이 dangling reference 문제를 인지함 (테스트 reset 경로에만 대응). 이 CAND 의 hot-path 는 production finally 경로.

관련 Finding 상세

1. drain finally 의 FOLLOWUP_QUEUES.delete 가 identity 검증 없어 후속 queue 를 orphan 시킴

  • 파일: src/auto-reply/reply/queue/drain.ts:263-271
  • 증상 유형: concurrency-race
  • 예상 영향: wrong-output — 정성: /stop 직후 다음 메시지가 들어오는 흔한 시나리오에서 발생.
  • 관찰 지표 오염: /status 명령이 읽는 getFollowupQueueDepth (status-text.ts:247) 가 실제 pending 수 (Q2.items) 를 0 으로 보고. 유저가 "queue 비어 있음" 으로 인식.
  • 콜백 손실 윈도우: D1.finally 가 clearFollowupDrainCallback(key) 한 후, 그 사이에 들어온 enqueue 가 runFollowup 을 생략하면 (이론상; production caller 인 agent-runner.ts:1002 는 항상 제공) 재시작할 callback 이 없어 메시지 drop 가능.
  • Double-drain: Q2 에 대한 D2 와 Q3 에 대한 D3 가 동일 session key 로 동시에 실행되어 agent run 이 병렬화됨 (intended 는 per-key serialization). 정량: 재현 조건 = (drain 내 최소 1회 await) AND (await 중 clearSessionQueues 호출) AND (clearSessionQueues 이후 enqueueFollowupRun). drain.ts 는 L151, L161, L216 등 여러 await 존재. agent 실행은 초~분 단위이므로 window 가 넓음.
<details><summary>증거 / 메커니즘 / 근본 원인</summary>

drain finally 의 FOLLOWUP_QUEUES.delete 가 identity 검증 없어 후속 queue 를 orphan 시킴

문제

scheduleFollowupDrain 의 drain IIFE 는 종료 시 finally 에서 FOLLOWUP_QUEUES.delete(key)

  • clearFollowupDrainCallback(key) 를 호출한다. 이는 "drain 이 처리 중이던 queue == 현재 map 에 등록된 queue" 라는 암묵 전제에 의존한다. 그러나 drain 의 await 경로 중 /stop 이나 session reset 이 clearSessionQueues → clearFollowupQueue 를 호출하면 map entry 가 제거되고, 이어진 enqueue 가 새 queue (Q2) 를 등록하면 원래 drain (D1) 의 finally 가 Q2 의 map entry 와 callback 을 지워버린다. Q2 는 orphan 되어 map 관찰자 관점에서 사라지고, D2 는 분리된 경로로 실행된다.

발현 메커니즘

위 frontmatter mechanism 참조 (타임라인 T0-T7).

핵심은 3-way interleaving:

  • D1 await 중 (agent 실행 ~ 수 초 ~ 수 분)
  • clearSessionQueues (map.delete + in-place clear)
  • enqueueFollowupRun (new Q2 map.set + D2 kick)
  • D1 await 복귀 → finally 가 Q2 map entry 를 지움

근본 원인 분석

  1. drain.ts:266 의 FOLLOWUP_QUEUES.delete(key) 는 key 만으로 삭제. 현재 map entry 가 자신이 캡처한 queue 인지 확인하지 않음.
  2. clearFollowupQueue (state.ts:80-86) 는 queue.items 를 in-place 로 비우고 map 에서 제거 하므로, D1 이 "정상적인 drain 완료" 와 "외부 clear" 를 구분할 수 없다.
  3. enqueueFollowupRun + kickFollowupDrainIfIdle 조합은 "map 에 queue 가 없거나 !draining" 이면 새 drain 을 시작하므로, D1 의 await 중에도 D2 가 병렬 실행될 수 있다.
  4. 기존 테스트 (queue.drain-restart.test.ts) 는 debounceMs=0 + OPENCLAW_TEST_FAST 환경에서 await window 가 거의 0 이라 세 이벤트가 겹치는 시퀀스를 자연스럽게 만들지 못한다.

영향

  • impact_hypothesis: wrong-output — queue depth 관찰 오류, 동일 key 에 대한 의도치 않은 double-drain.
  • /status 명령이 getFollowupQueueDepth 로 읽는 pending 수가 실제 Q2 에 남은 항목을 반영 하지 못함 (map 조회 기반이므로 orphan Q2 는 0 으로 보고).
  • Q2 에 대한 D2 와 이후 enqueue 로 생성된 Q3 에 대한 D3 가 동일 session key 로 동시에 agent run 을 트리거 → per-session serialization 위반.
  • production caller (agent-runner.ts:1002) 는 runFollowup 을 항상 전달하므로 callback 손실로 인한 메시지 drop 은 일반 흐름에서는 발생하지 않음. 그러나 관찰 지표/직렬화 invariant 는 깨짐.
  • 재현 조건: /stop 직후 다음 메시지라는 가장 흔한 사용 시나리오. agent 실행이 수 초 이상 걸릴 때 자연스럽게 트리거.

반증 탐색

  • 외부 lock: rg "Mutex|Semaphore|AsyncLock" 매치 0건. queue 보호 락 없음.
  • AbortController: rg "AbortController|AbortSignal" 매치 0건. abort 전파 없음.
  • identity check: rg "FOLLOWUP_QUEUES\.get\(key\)\s*===|map\.get\(key\)\s*===" 매치 0건. finally 의 delete 에서 자신의 queue 가 여전히 map 의 entry 인지 확인하는 로직 부재.
  • 기존 테스트: queue.drain-restart.test.ts:236-260 은 callback 이 clearSessionQueues 이후 유지되지 않음을 검증하나, Q2 orphan 시나리오는 커버하지 않음.
  • 주변 코드 맥락: subagent-announce-queue.ts:62-72 resetAnnounceQueuesForTests 주석 ("Clearing the map alone isn't enough because drain loops capture queue by reference") 에서 maintainer 가 dangling reference 문제를 인지. 그러나 test-reset 경로에만 대응했을 뿐 production finally 의 identity 보호는 없음.
  • primary-path inversion: identity check (FOLLOWUP_QUEUES.get(key) === queue) 가 unconditional 하게 finally 에 존재해야 race 가 차단됨. 현재 부재 → race 재현 가능.

Self-check

내가 확실한 근거

  • drain.ts:266-267 은 key 만으로 map.delete + callback clear 를 수행. identity check 없음.
  • state.ts:80-86 clearFollowupQueue 는 in-place items 비우기 + map.delete. D1 의 array reference 는 동일하므로 즉시 length=0 으로 관찰.
  • enqueue.ts:60-110 enqueueFollowupRun 은 map 에 key 가 없으면 새 queue 를 만들어 set 하고 !draining 이면 drain 을 kick. D1 의 await 중에도 D2 가 시작됨.
  • status-text.ts:247 은 getFollowupQueueDepth(key) 를 map 조회에 의존.

내가 한 가정

  • /stop 직후 다음 메시지가 들어오는 시나리오가 실제 user 행동에 흔하다고 가정. 빈도 측정 데이터 없음.
  • agent 실행 시간 (effectiveRunFollowup 의 await) 이 수 초~분 단위라고 가정. 짧은 agent run 일수록 window 가 좁아 race 확률 감소.
  • production caller 가 항상 runFollowup 을 enqueue 에 전달한다고 가정. agent-runner.ts:1002 확인했으나 다른 경로가 있을 수 있음 (allowed_paths 밖).

확인 안 한 것 중 영향 가능성

  • drainCollectQueueStep 경로에서 isCrossChannel 재계산 여부 (별도 FIND-auto-reply-concurrency-002).
  • subagent-announce-queue.ts:204 의 동일 패턴 (out-of-scope: src/agents/).
  • refreshQueuedFollowupSession (state.ts:90-152) 이 drain 의 await 중 호출되었을 때의 영향.
  • status-text.ts 이외에 getFollowupQueueDepth 를 사용하는 다른 관찰 지표.
</details>

<sub>이 이슈는 openclaw-audit 로컬 신뢰성 감사 파이프라인에서 생성됨. 재현 테스트와 수정은 별도 PR 에 포함됩니다.</sub>

<!-- openclaw-audit: cand=CAND-012 fingerprints=35ed2ee868e7 at=2026-04-19T06:23:27+00:00 -->

extent analysis

TL;DR

Add an identity guard to the drain finally block to prevent deleting the wrong queue entry.

Guidance

  • Verify that the FOLLOWUP_QUEUES.get(key) returns the same queue object as the one being drained before deleting it.
  • Consider adding a check to ensure that the queue being deleted is the same one that was being processed by the drain.
  • Review the clearFollowupQueue function to ensure it correctly handles the removal of queue entries.
  • Test the fix with various scenarios, including concurrent executions and different queue sizes.

Example

if (FOLLOWUP_QUEUES.get(key) === queue) {
  FOLLOWUP_QUEUES.delete(key);
  clearFollowupDrainCallback(key);
}

Notes

The provided code snippet assumes that the queue variable refers to the queue object being drained. The fix should be applied to the drain finally block in src/auto-reply/reply/queue/drain.ts.

Recommendation

Apply the workaround by adding the identity guard to the drain finally block, as it will prevent the deletion of the wrong queue entry and ensure the correct functionality of the queue system.

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…
#api#ssr#installation#API versioning#request timeout

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING

openclaw - ✅(Solved) Fix auto-reply queue: drain finally deletes FOLLOWUP_QUEUES entry without identity check, orphaning post-/stop queue [4 pull requests, 1 participants]