Skip to content

fix: continue PR #430 - hook dedup + resource singleton + backup timer guard#612

Closed
jlin53882 wants to merge 1 commit intoCortexReach:masterfrom
jlin53882:fix/pr430-phase1-3-singleton
Closed

fix: continue PR #430 - hook dedup + resource singleton + backup timer guard#612
jlin53882 wants to merge 1 commit intoCortexReach:masterfrom
jlin53882:fix/pr430-phase1-3-singleton

Conversation

@jlin53882
Copy link
Copy Markdown
Contributor

@jlin53882 jlin53882 commented Apr 14, 2026

PR #612 — Phase 1: Hook Event Deduplication

摘要

繼續 PR #430 的 Phase 1 內容。從 upstream/master (0988a46) 為基礎,實作鉤子事件去重機制,防止 memory leak。

變更內容(Phase 1 Only)

新增模組 (index.ts)

  • _hookEventDedup: module-level Set<string>,儲存已處理的事件鍵
  • _dedupHookEvent(handlerName, event): 去重函數,鍵格式 handlerName:sessionKey:timestamp
    • 若鍵已存在 → return true(skip)
    • 若 Set size > 200 → 自動裁剪,保留最新 100 筆

三個 Handler 的 Guard 順序

Handler Guard 順序
agent:bootstrap isInternalReflectionSessionKey:subagent: check → _dedupHookEvent
appendSelfImprovementNote !Array.isArray(messages)_dedupHookEvent
runMemoryReflection !sessionKey_dedupHookEvent

所有驗證都在 _dedupHookEvent 之前,確保 invalid sessions 不會佔用去重鍵。

未涵蓋(Phase 2 / 3)

  • ❌ Singleton state 管理 (_singletonState)
  • ❌ Backup timer 優化

測試覆蓋

24 個單元測試,全部通過 (test/hook-dedup-phase1.test.mjs)

測試群組 數量 覆蓋場景
Core logic 10 去重鍵唯一性、sessionKey fallback、timestamp 處理、bounded
Handler guard placement 13 3 個 handler 的 validation 都正確放在 dedup 之前
Bounded memory 1 1000 events 後 Set size ≤ 200

驗證方式

  • ✅ 多次 scope init 後,hooks 不會累積
  • /reset 只觸發一次對應的 hook handler
  • ✅ Subagent sessions 的 legitimate events 不會被錯誤 skip
  • _hookEventDedup Set size 保持 bounded(≤200)

Codex 對抗式審查

結果:Critical / High 問題 0 個

問題 嚴重程度 狀態
去重鍵碰撞(同一毫秒事件被錯誤 skip) Critical ✅ 已修復(validation 前移)
Handler order(validation 在 dedup 之後) Critical ✅ 已修復
Subagent session 被錯誤 skip High ✅ 已修復(validation 前移)
FIFO eviction 對 long-running sessions 不公平 High ✅ 已改用 newest-100 保留
Pruning iterator bug ✅ 已修復(slice(-100))

Commit History

  • 5064e0e — feat: Phase 1 - hook event dedup singleton (index.ts)
  • 9b18625 — fix: move sessionKey validation before dedup guard (Phase 1 Codex fix)
  • cb458ce — feat: add unit tests + fix pruning logic
  • 7f2f476 — test(phase1): add selfImprovement mock tests (24/24 pass)
  • 0c92567 — ci: register hook-dedup-phase1 tests in core-regression group

…exReach#603 cherry-pick)

- store.ts: Replace unbounded promise chain with tail-reset semaphore
- access-tracker.ts: Separate retry count map, cap at _maxRetries=5
- embedder.ts: TTL eviction on set() when near capacity
- retrieval-stats.ts: Ring buffer replaces O(n) Array.shift()
- noise-prototypes.ts: Lower DEDUP_THRESHOLD 0.95 -> 0.90

Cherry-picked from PR CortexReach#603 (jlin53882/memory-lancedb-pro)
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b687062112

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/access-tracker.ts
} else {
this._retryCount.set(id, retryCount);
// Requeue with the original delta only (NOT accumulated) for next flush.
this.pending.set(id, delta);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve concurrent deltas when requeueing failed writes

When a flush is in progress, recordAccess() can add new increments for the same ID into pending; if the write then fails, this assignment overwrites that newer value with the stale batch delta, so accesses recorded during the failed flush are silently lost. This undercounts accessCount and can bias downstream decay/reinforcement behavior for active memories.

Useful? React with 👍 / 👎.

Comment thread src/access-tracker.ts
Comment on lines +341 to +343
this.logger.error(
`access-tracker: dropping ${id.slice(0, 8)} after ${retryCount} failed retries`,
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid calling non-required logger.error in retry drop path

AccessTrackerOptions only requires warn (with optional info), but this branch unconditionally calls logger.error; integrations that provide a warn-only logger (which the current type allows) will throw TypeError after max retries are exceeded, aborting flush handling exactly on persistent failures. Use an optional call/fallback to warn to keep retry exhaustion non-fatal.

Useful? React with 👍 / 👎.

@jlin53882 jlin53882 changed the title fix: resolve 5 memory leak issues from issue #598 (PR #603 cherry-pick) fix: continue PR #430 - hook dedup + resource singleton + backup timer guard Apr 14, 2026
@jlin53882
Copy link
Copy Markdown
Contributor Author

替換為 PR #617

此 PR(#612)原本指向錯誤的 branch(fix/pr430-phase1-3-singleton,含 Phase 2/3)。

正確的 Phase 1 內容已移至 PR #617#617

差異說明

Issue #610

追蹤:#610

@jlin53882 jlin53882 closed this Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant