feat(redis-lock): 分散式 Redis lock 實作，支援 file-lock fallback (PR739 rebuild)#781
feat(redis-lock): 分散式 Redis lock 實作，支援 file-lock fallback (PR739 rebuild)#781jlin53882 wants to merge 3 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b8de59b23
ℹ️ 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".
| mode = "bm25"; | ||
| results = await this.bm25OnlyRetrieval( | ||
| query, tagTokens, safeLimit, scopeFilter, category, trace, | ||
| query, tagTokens, safeLimit, scopeFilter, category, trace, diagnostics, |
There was a problem hiding this comment.
When retrieveWithTrace takes the tag/auto-recall path, this assigns to mode and passes diagnostics even though neither variable is declared in this method; because the package is ESM/strict mode, the assignment throws ReferenceError before any retrieval result or debug trace is returned. The same pattern appears in the other branches, so declaring/initializing these locals as in retrieve() is needed before calling the memory_debug trace path.
Useful? React with 👍 / 👎.
| this.redis = new Redis({ | ||
| host: parsed.host, | ||
| port: parsed.port, | ||
| db: parsed.db, | ||
| lazyConnect: true, |
There was a problem hiding this comment.
For password-protected Redis URLs such as redis://:secret@host:6379/1, parseRedisUrl() extracts only host/port/db and this constructor never passes username/password to ioredis. In those deployments the lock manager cannot authenticate and will disable/fail Redis locking even though REDIS_URL is valid; pass the original URL or include the auth fields in the options.
Useful? React with 👍 / 👎.
| constructor(private readonly config?: LockConfig) { | ||
| // Issue 4 fix: namespace key with dbPath hash，避免不同 dbPath 的 store 互相 blocking | ||
| this._lockNamespace = config?.dbPath ? hashString(config.dbPath) : "default"; |
There was a problem hiding this comment.
When callers provide ttl, maxWait, or retryDelay in LockConfig, the constructor only computes the namespace and leaves the class defaults in place, so a contended lock still waits up to 60s and uses the default TTL/retry cadence. That makes the public config ineffective for tests or deployments that need shorter acquisition timeouts.
Useful? React with 👍 / 👎.
PR 739 重建（原本 PR 有 merge conflict）： - 新增 src/redis-lock.ts（340行） - dynamic import 避免 ioredis 未安裝時 crash - RedisUnavailableError + Symbol.for marker（跨 module ESM-safe） - isRedisConnectionError 含 wrapped error 遞迴檢查 - parseRedisUrl() 正確解析 URL（含密碼、DB selection） - RedisLockManager + createRedisLockManager 工廠 - dbPath hash namespace 避免跨 store lock interference - 修改 src/store.ts（+50行） - import RedisLockManager/createRedisLockManager/RedisUnavailableError - runWithFileLock() 新增 Redis lock 優先實作 - runWithFileLockCore() 保留既有 file lock 實作 - getRedisLockManager() 模組層級 cache（Option E） - 修正 CortexReach#670：realpath:false 避免 stale lock cleanup 造成 ENOENT - 新增 3 個測試檔（40 個測試，全部通過） - test/redis-lock-error-types.test.mjs - test/redis-lock-url-parse.test.mjs - test/redis-lock-concurrent-init.test.mjs - package.json：新增 ioredis devDependency 注意：batch accumulator（CortexReach#690）完整保留，不受影響。
jlin53882
force-pushed
the
james/pr739-redis-lock-rebuild
branch
from
2b8de59 to
926586a
Compare
1 participant
PR 739 重建說明
原本的 PR #739（分支 fix/pr704-redis-url-parsing-v2）已關閉，有 merge conflict。
此 PR 以最新 master 為基礎重新實作，完整保留 #690 batch accumulator 功能。
實作內容
新增 src/redis-lock.ts（340 行）
修改 src/store.ts（+50 行）
新增測試（40 tests，全部通過）
修復的 Issue
測試結果
tests: 40, pass: 40, fail: 0
cross-process-lock.test.mjs、reflection-bypass-hook.test.mjs 皆通過。