fix(store): proper-lockfile retries + ECOMPROMISED graceful handling (#415)

[jlin53882]

Fix Issue #415 — proper-lockfile ECOMPROMISED graceful handling

Problem

Error: Unable to update lock within the stale threshold causes OpenClaw Gateway to exit when under heavy load.

Root cause: The stale threshold (10s) is too short for high-load environments where the Node.js event loop can be blocked for >10s by synchronous I/O or heavy computation. When proper-lockfile's setTimeout callback is delayed beyond the stale threshold, it triggers ECOMPROMISED which by default crashes the process.

Additionally, the retries configuration (5 retries, max wait ~3.1s) was severely misaligned with stale=10000ms, causing competitor processes to give up waiting even when the lock holder was still alive but temporarily blocked.

Solution

Fix 1: Increase retries max wait from ~3.1s → ~151s (src/store.ts)

- retries: { retries: 5, factor: 2, minTimeout: 100, maxTimeout: 2000 }
+ retries: { retries: 10, factor: 2, minTimeout: 1000, maxTimeout: 30000 }

Exponential backoff sequence: 1s, 2s, 4s, 8s, 16s, 30s×5 = ~151 seconds total. This gives lock holders enough time to recover from temporary event loop blocking.

Fix 2: ECOMPROMISED graceful handling with synchronous flag mechanism

ECOMPROMISED is an ambiguous degradation signal — the mtime-based mechanism in proper-lockfile cannot distinguish between "holder crashed" vs "holder event loop is temporarily blocked". Instead of trying to distinguish these cases (which is impossible with the available signals), the code accepts this ambiguity and uses a state machine:

ECOMPROMISED triggered
    ↓
fn() completed?
    ├─ Yes + succeeded → Return result + warn "don't auto-retry"
    ├─ Yes + failed    → Throw fnError (fn's error takes priority)
    └─ Still running  → Throw compromisedErr (caller decides to retry)
    ↓
release() → ignore ERELEASED (expected after compromised)

Key implementation details:

1. Synchronous onCompromised callback: Must be synchronous. setLockAsCompromised() does NOT await the Promise — an async throw inside onCompromised becomes an unhandled rejection, not an error returned to runWithFileLock().

2. ERELEASED handling: After onCompromised fires, proper-lockfile sets lock.released = true, causing release() to immediately return ERELEASED. This must be caught and ignored — do not return from the catch block, otherwise the finally's return value overrides the successful result from try.

3. No lockfile.check() distinction: check() only tests stat.mtime < now - stale. Both "holder crashed" and "holder event loop blocked" produce identical stale mtimes. Using check() to distinguish them is无效.

State Machine

fn() outcome	Compromised?	Action
Succeeded	No	Return result, normal release
Failed	No	Throw fnError, normal release
Succeeded	Yes	Return result, ignore ERELEASED, warn
Failed	Yes	Throw fnError (takes priority)
Not finished	Yes	Throw compromisedErr

Changes

File	Change
src/store.ts	runWithFileLock(): retries + synchronous onCompromised flag + ERELEASED handling
test/cross-process-lock.test.mjs	Regression tests with ERELEASED mock simulation
test/lock-stress-test.mjs	Concurrent lock stress test

Testing

• cross-process-lock.test.mjs: 8/8 pass (including ERELEASED handling, fnError priority, error propagation)
• lock-stress-test.mjs: 3/3 pass
• CI: core-regression ✅, storage-and-schema ✅, llm-clients-and-auth ✅, packaging-and-workflow ✅

References

• Fixes [openclaw] Uncaught exception: Error: Unable to update lock within the stale threshold #415

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[AliceLJY]

Fix is correct and well-scoped. The stale threshold increase (10s to 60s) and retry parameter realignment make sense for event-loop-starvation scenarios. The ECOMPROMISED fallback is a reasonable best-effort recovery.

Minor suggestions (non-blocking):

1. Remove the unused fnCompleted variable (dead code)
2. Add a console.warn when ECOMPROMISED fallback activates -- operators need visibility into how often this degraded path fires
3. CI cli-smoke failure is pre-existing on master (strip-envelope-metadata tests), unrelated to this PR

Approved.

[jlin53882]

Author review 已處理

✅ 已移除未使用的 nCompleted 變數
✅ 已加入 console.warn 讓 operator 看得到 degraded path 觸發時機

感謝作者的 review！

[rwmjhb]

Review: fix(store): increase proper-lockfile stale threshold + ECOMPROMISED graceful fallback (#415)

高负载下 event loop 阻塞导致 ECOMPROMISED crash 是真实问题。但 stale 从 10s 改到 60s 有 trade-off：

Must Fix

1. 恢复延迟 6x: stale: 60_000 意味着真正 crash 的 lock holder 需要 60s 才能被回收（原来 10s）。对于生产环境的可用性影响需要权衡。

2. Stress test 未被执行: 新的 stress-test file 没有加入项目的 test script，CI 不会跑。

3. ECOMPROMISED catch 是死代码: proper-lockfile 的 ECOMPROMISED 通过 onCompromised callback 触发，不是 thrown error——catch block 永远不会执行。

Questions

• fallback lockfile.lock 的 retries: 3 vs primary path 的 retries: 10 是有意为之还是遗漏？
• ECOMPROMISED fallback 硬编码 3s wait，但新 stale 是 60s——3s retry 无法回收 fresh lock，fallback 是否有效？

请修复 stress test 执行问题和 ECOMPROMISED 处理逻辑后再 review。

[jlin53882]

Maintainer Review 回應（commit 2d3277f）

感謝 reviewer 的詳細審查，以下是所有 Must Fix 的處理結果：

✅ Must Fix 3：ECOMPROMISED catch 是死代碼

已修復。使用 onCompromised callback + flag 機制取代原本無效的 catch (err.code === \ECOMPROMISED)：

ypescript onCompromised: (err: any) => { isCompromised = true; compromisedErr = err; console.warn(...); },

n() 的錯誤優先於 lock compromised 錯誤（避免 error masking）。

✅ Must Fix 2：Stress test 未加入 CI

已修復：

1. lock-stress-test.mjs 已加入 package.json 的 test script
2. 第三個測試（30 個並發寫入）會 ELOCKED，已改為順序寫入 20 筆
3. stress test 結果：3/3 pass

✅ Questions 回答

• fallback retries: 3 vs primary retries: 10：已移除 fallback 機制（因為根本進不去 catch）。現在只有 primary 的統一參數。
• 3s wait fallback 是否有效：已移除 fallback，等同回答「不需要」。

關於 stale=60s 的 trade-off

這是預期的 trade-off：lock holder 崩潰後需要 60 秒才能回收。但相對於 Gateway 直接 crash，60 秒的等待代價更低，且多數情況下 holder 只是被 block 不是真的崩潰。

請再次 review 🙏

[rwmjhb]

Re-review on new commits

Thanks for addressing the previous feedback. The onCompromised callback with flag mechanism is the right fix — properly intercepts the compromised event that the catch block couldn't reach. Three items remain.

Verdict: request-changes (confidence 0.95)

Must Fix

1. Build failure + stale base

BUILD_FAILURE blocker persists. stale_base=true — AliceLJY noted strip-envelope-metadata failures are pre-existing on main, but without a rebase this can't be confirmed on your branch. Please rebase and verify.

2. Two test failures in strip-envelope-metadata.test.mjs

Lines 121, 132: subagent wrapper text not stripped. Even if pre-existing, merging with unattributed test failures normalizes broken CI. After rebase, confirm these fail identically on main.

3. stale=60s recovery trade-off needs documentation

Your response acknowledges this as an intentional trade-off (60s wait vs Gateway crash). That's reasonable — please add a code comment near stale: 60000 explaining the decision so future maintainers understand why it's 6x the default.

Nice to Have

• Stress test 2 is sequential — doesn't test retry-under-contention. Use Promise.all to launch concurrent store() calls.
• 3 explicit any type additions in error-handling path of src/store.ts — consider typed error interface.
• Stress test 3 (30 concurrent writers) has no test-level timeout — CI may hang.

Prior Items — Status

Item	Status
ECOMPROMISED catch is dead code	✅ Fixed (onCompromised callback)
Fallback lock mechanism	✅ Removed (not needed with onCompromised)
Stress test not in CI	✅ Added to package.json

[jlin53882]

PR #517 回覆（完整版）

---

Must Fix 1：ECOMPROMISED callback 機制

問題：proper-lockfile 的 ECOMPROMISED 是透過 onCompromised callback 觸發，不是同步 throw——原本的 catch block 是死代碼。

修復狀態：✅ 已完成

Commit 2d3277f 已將 try/catch 改為 onCompromised flag 機制。fn() 的錯誤優先級高於 lock compromised 錯誤，確保錯誤不被覆蓋。

---

Must Fix 2：Stress test 未加入 CI

問題：新加的 lock-stress-test.mjs 沒有加進 CI。

修復狀態：✅ 已完成

Commit 8b2f161 已將 lock-stress-test.mjs 加入 ci-test-manifest.mjs。

---

Must Fix 3：stale=60s 的設計取捨

問題：stale 從 10s 改到 60s，真正 crash 的 lock holder 需要 60s 才能被回收。

設計決策說明：

問題根源是 retries 的總等待時間與 stale 不匹配：

• 原始：stale=10000ms + retries=5(max wait~3100ms) — 當 event loop 偶發性阻塞，setTimeout 延遲 >3.1s，competitor 的所有 retry 耗盡，但仍收到 ECOMPROMISED crash

修復方向是讓 retry 的總等待時間覆蓋 event loop 可能阻塞的時間：

• retries=10, maxTimeout=30000, factor=2 → 總等待約 151 秒
• stale=60000 → 60 秒後判定為 stale

Stale=60s 的代價評估：

• 如果 lock holder 真的 crash（不是被 block）：需要 60s 才能回收 lock
• 在 OpenClaw Gateway 實際使用情境（long-running process，lock holder 極少真的 crash）中，這個 trade-off 是可接受的
• 若 lock holder 只是被 event loop 阻塞，60s 後它會自動恢復並正常 release lock

---

Question 1：fallback lock 的 retries: 3 vs primary 的 retries: 10

回答：PR 目前沒有獨立的 fallback lock 機制。

src/store.ts 只有一組 retries 參數（retries: 10）。onCompromised 觸發時，operation 直接失敗並 throw。這是 intentional 的設計選擇——避免 fallback lock 引入額外的複雜度和 race condition 風險。

---

Question 2：fallback 的 3s wait vs 60s stale

回答：PR 沒有實作任何 fallback 等待時間。

當 onCompromised 觸發時：competitor 的所有 retry 已耗盡，finally 直接 throw，caller 收到錯誤並知道需要 retry 或人工介入。

沒有 fallback 的理由：

• ECOMPROMISED 是罕見事件
• Fallback lock 無法從外部判斷「holder 是 block 了還是真的 crash 了」
• 自動 fallback 可能造成連鎖反應
• 讓 operation 失敗並回報給 caller，比靜默 fallback 更安全

---

附加說明：Plan C 的 cleanup 目標

經過對抗式分析，我們發現 finally 中的 unlinkSync(lockPath) 嘗試刪除的是初始化 block 創建的目錄（.memory-write.lock），而不是 proper-lockfile 的 actual lock file（.lock.lock）。在 Windows 上 unlinkSync 刪目錄會得到 EPERM（被 catch {} 靜默忽略）。

兩種處理方向，請 maintainer 確認偏好：

選項 A：維持現狀——依賴 competitor 在下次 acquire 時發現 stale 並自動清理

選項 B：在 finally 中移除無效的 cleanup，明確說明 cleanup 由 competitor 的 stale 機制處理

---

請確認是否還有其他需要調整的地方。

[jlin53882]

Fixes applied to this PR

Two issues were found and fixed in this PR:

---

Fix 1: src/store.ts — False-positive compromisedErr after successful write

Problem: The finally block in runWithFileLock() was throwing compromisedErr even when fn() completed successfully. This happened because onCompromised is a callback that fires asynchronously (when the stale timer expires), not a synchronous interruption of fn(). So the following scenario occurred:

1. fn() executes → table.add([entry]) commits successfully
2. fn() returns successfully
3. onCompromised fires later (stale threshold exceeded)
4. finally: isCompromised === true, fnError === null → throw compromisedErr
5. Caller receives error → retries → generates new UUID → duplicate write

Fix: When fn() succeeds (fnError === null) and isCompromised === true, simply return instead of throwing. The onCompromised signal means "lock ownership may be lost", not "fn() failed". For mutation callers like store(), importEntry(), update(), and delete(), this prevents false failures and duplicate writes.

} finally {
  if (isCompromised) {
    // onCompromised means lock ownership may be lost, NOT fn() failed.
    // If fn() succeeded (fnError === null), do not throw — the write is real.
    // If fn() itself threw an error, catch already rethrew it.
    // Lock is auto-released by proper-lockfile, no need to call release().
    return;
  }
  await release();
}

---

Fix 2: scripts/verify-ci-test-manifest.mjs — Missing baseline entry

Problem: The PR added test/lock-stress-test.mjs to CI_TEST_MANIFEST but did not update EXPECTED_BASELINE in verify-ci-test-manifest.mjs. The CI job packaging-and-workflow runs this verification script, which enforces that every entry in CI_TEST_MANIFEST must appear exactly once in EXPECTED_BASELINE. This caused the CI to fail with:

Error: unexpected manifest entry: test/lock-stress-test.mjs

Fix: Added test/lock-stress-test.mjs to EXPECTED_BASELINE in verify-ci-test-manifest.mjs, maintaining the same group (storage-and-schema) and runner (node) as the manifest entry.

---

Both fixes are on the fix/issue-415-stale-threshold branch pushed to jlin53882/memory-lancedb-pro.

[jlin53882]

重新看過 PR #517 最新 commit，聚焦 #415 的 lock 修復後，目前只看到 1 個 blocker，沒有發現其他同級 hidden bug：

src/store.ts 在 onCompromised 後會把已成功的寫入誤報成失敗，caller 重試時可能產生 duplicate writes。

具體影響鏈：fn() 執行成功 → table.add() 已 commit → fn() 回傳成功結果 → finally 因 isCompromised=true 改成 throw compromisedErr → caller 以為寫入失敗而重試 → 新 UUID 再次寫入 → 重複資料。

這是 #415 / PR #517 主範圍內唯一需要修的 P1。

[jlin53882]

Fixes applied to this PR

Two issues were found and fixed in this PR:

---

Fix 1: src/store.ts — False-positive compromisedErr after successful write

Problem: The finally block in runWithFileLock() was throwing compromisedErr even when fn() completed successfully. This happened because onCompromised is a callback that fires asynchronously (when the stale timer expires), not a synchronous interruption of fn(). So the following scenario occurred:

1. fn() executes → table.add([entry]) commits successfully
2. fn() returns successfully
3. onCompromised fires later (stale threshold exceeded)
4. finally: isCompromised === true, fnError === null → throw compromisedErr
5. Caller receives error → retries → generates new UUID → duplicate write

Fix: When fn() succeeds (fnError === null) and isCompromised === true, simply return instead of throwing. The onCompromised signal means "lock ownership may be lost", not "fn() failed". For mutation callers like store(), importEntry(), update(), and delete(), this prevents false failures and duplicate writes.

} finally {
  if (isCompromised) {
    // onCompromised means lock ownership may be lost, NOT fn() failed.
    // If fn() succeeded (fnError === null), do not throw — the write is real.
    // If fn() itself threw an error, catch already rethrew it.
    // Lock is auto-released by proper-lockfile, no need to call release().
    return;
  }
  await release();
}

---

Fix 2: scripts/verify-ci-test-manifest.mjs — Missing baseline entry

Problem: The PR added test/lock-stress-test.mjs to CI_TEST_MANIFEST but did not update EXPECTED_BASELINE in verify-ci-test-manifest.mjs. The CI job packaging-and-workflow runs this verification script, which enforces that every entry in CI_TEST_MANIFEST must appear exactly once in EXPECTED_BASELINE. This caused the CI to fail with:

Error: unexpected manifest entry: test/lock-stress-test.mjs

Fix: Added test/lock-stress-test.mjs to EXPECTED_BASELINE in verify-ci-test-manifest.mjs, maintaining the same group (storage-and-schema) and runner (node) as the manifest entry.

---

Both fixes are on the fix/issue-415-stale-threshold branch pushed to jlin53882/memory-lancedb-pro.

[jlin53882]

@AliceLJY 我剛剛已經有經過 codex 對抗，將一些隱藏bug 抓取出來重新修正，已推上的最新的 commit 515b332 ，再麻煩您有空的的時候 ，幫我重新review 一次，看看有沒有其他忽略的點。

[rwmjhb]

感谢这个 PR，event loop 阻塞导致 ECOMPROMISED 崩溃是真实的高频故障，方向正确。

必须修复（3 项）

MR2：stale 提高到 60s 同时延迟了真实崩溃后的 lock 恢复时间

原来 10s 的 stale 窗口意味着持锁进程崩溃后最多 10s 可以被其他 writer 接管；改为 60s 后，一个真实崩溃的 lock holder 会让所有等待者阻塞最多 60s。建议评估一个更小的中间值（如 20–30s），或者在 ECOMPROMISED fallback 路径里主动缩短 stale 窗口。

EF1 / EF2：strip-envelope-metadata.test.mjs 有 2 个测试失败

验证器报告了 BUILD_FAILURE，且 strip-envelope-metadata.test.mjs 中有两个测试未通过（subagent wrapper pattern 未被 strip）。请确认：在你的 branch 上，这两个失败是在你修改之前就已存在，还是本 PR 引入的回归？建议 rebase 后用干净 main 重新跑一次测试并截图确认。

---

建议修复（不阻塞合并）

• F2：ECOMPROMISED catch block 在实际崩溃场景下是死代码——proper-lockfile 的 onCompromised 回调才会被调用，catch 无法拦截
• F3：stress test 2 是顺序执行的，无法验证并发争抢下的 retry 行为
• EF3：src/store.ts 锁错误处理路径中新增了 3 处显式 any，高风险文件建议补充类型

一个问题

fallback lockfile.lock 用的是 retries:3，主路径是 retries:10，这个不对称是有意为之（快速失败 fallback）还是笔误？

[rwmjhb]

Re-review on 515b332

Thanks for the Codex self-review and the two fixes (false-positive compromisedErr in finally, and the retry-window realignment). Reviewed commit 515b332.

Must Fix

EF1 — Rebase required before merge
stale_base=true: the branch is behind origin/main. The build failure in CI (from strip-envelope-metadata.test.mjs) is claimed pre-existing, but can't be verified until this is rebased. Please rebase onto current main — if the build failure is truly pre-existing it will still show up after rebase and can be confirmed as noise; if it disappears after rebase, it was caused by a conflict.

MR2 — Stale threshold trade-off needs explicit acknowledgment
Raising stale from 10 s → 60 s fixes the false-positive ECOMPROMISED under event-loop lag, but it also means a genuinely crashed lock holder blocks all other writers for 6× longer before they can take over. This is a real trade-off. Please either:

• Add a code comment in runWithFileLock() explaining why 60 s is the right balance (e.g., "event-loop lag observed up to ~40 s in production under load — [openclaw] Uncaught exception: Error: Unable to update lock within the stale threshold #415"), or
• Consider a two-tier approach: a shorter stale threshold with the ECOMPROMISED fallback handling the gap

Nice to Have

• F2: The onCompromised callback is required by proper-lockfile — the ECOMPROMISED catch block in the fallback path is unreachable for the crash scenario. Consider removing the dead code or documenting why it's kept as defense-in-depth.
• F3: Stress test 2 runs lock operations sequentially inside a for loop — it doesn't actually exercise concurrent contention. Consider using Promise.all() to run writers in parallel.
• EF3: src/store.ts gains 3 explicit any casts in the lock error-handling path. Can these be typed as NodeJS.ErrnoException or { code?: string } instead?
• EF4: Stress test 3 runs 30 concurrent writers with a 151 s max retry window and no per-test timeout. This risks indefinitely hanging CI. Adding a jest.setTimeout / it.concurrent timeout guard would help.

---

Good direction overall — the core crash fix is sound. Address the must-fix items (rebase + stale trade-off comment) and this is ready to merge.

[jlin53882]

回覆：感謝所有審查意見

感謝 @rwmjhb 兩輪詳細審查。以下逐一回覆。

---

🔴 P1 Blocker：James 發現的「成功寫入被誤報為失敗」問題（已修復）

問題：當 fn() 執行成功（table.add() 已 commit）→ onCompromised 在 fn() 完成後觸發 → finally 仍拋 compromisedErr → Caller 以為寫入失敗 → 重試 → duplicate writes。

根因：finally { return; } 或 finally { throw X; } 會覆蓋 try 區塊的成功回傳值。Remote 的 515b332 修復（if (fnError === null) { return; }）也有同樣問題，仍會回傳 undefined 導致 caller 重試。

修復（commit f4db94c）：加入 fnSucceeded 布林標記，確保 fn() 成功時：

let fnSucceeded = false;
try {
  const result = await fn();
  fnSucceeded = true;
  return result;
} catch (e) {
  fnError = e;
  throw e;
} finally {
  if (isCompromised) {
    if (fnError !== null) throw fnError;   // fn() 失敗 → 拋原本錯誤
    if (!fnSucceeded) throw compromisedErr; // fn() 未開始 → 拋 compromisedErr
    // fn() 成功 → log warning + 回傳成功結果，不重試
    console.warn(`[memory-lancedb-pro] Returning successful result despite compromised lock...`);
  }
  await release();
}

驗證：Codex 對抗式審查確認，finally { return; } 確實會覆蓋 try 的回傳值。Inline 測試確認修復後正確回傳 SUCCESS_RESULT（非 undefined）。

---

MR2：stale=60s 延長了真實崩潰後的 Lock 恢復時間

感謝指出這個 trade-off。這是一個真實的取捨：

設計選擇	優點	缺點
stale=10s（舊）	真實崩潰後 10s 可恢復	高負載時 event loop 阻塞 → false ECOMPROMISED
stale=60s（新）	避免 false ECOMPROMISED	真實崩潰後需 60s 才釋放

我們的選擇：在 OpenClaw 實際使用情境下，event loop 阻塞導致的 false ECOMPROMISED 是高頻故障，而真實崩潰是低頻事件。60s 的等待時間對於記憶寫入的 SLA 是可接受的。

補充：competitor 的重試策略（minTimeout=1000, maxTimeout=30000, retries=10）在 holder 崩潰後，會在約 63s（minTimeout * factor^5 = 32s 級）成功取得 lock。Lock 恢復時間不會高達 60s。

---

EF1/EF2：strip-envelope-metadata.test.mjs 失敗

確認：這個測試在 master branch 上根本不存在，是 fix/envelope-stripping-phase2 feature 的產物，透過 PR #517 的 merge 帶入的。

在 fix/issue-415-stale-threshold branch 上執行：

node --test test/strip-envelope-metadata.test.mjs
✔ stripEnvelopeMetadata (3.0944ms)
ℹ tests 14 | ℹ pass 14 | ℹ fail 0

結論：EF1/EF2 不是 PR #517 的 regression，是 Phase 2 envelope stripping 的測試。如果 Phase 2 有問題，需要獨立的 PR 處理。

---

EF3：三個 any type

同意這是類型品質問題。目前 compromisedErr、fnError、err 都是 any。從嚴格類型安全角度，catch (e) 應該是 unknown 再轉換，onCompromised 的 err 也應該是 Error type。

這是 non-blocking 建議，不阻擋 merge。如需修復，我們可以在 follow-up PR 中統一改用 unknown / Error。

---

F2 / F3

感謝建議 Stress test 和 Dead code 清理。這些可以在 stale 修復 merge 後作為 follow-up PR 處理，確保不與當前 PR 的範圍Scope 混淆。

---

總結

問題	狀態
P1 Blocker（duplicate writes）	✅ 已修復（f4db94c）
MR2（stale=60s trade-off）	📝 設計選擇，已說明
EF1/EF2（strip-envelope 失敗）	✅ 已排除（非 regression）
EF3（any type）	📝 Non-blocking，可 follow-up
F2/F3（stress test / dead code）	📝 Non-blocking，可 follow-up

請求 @rwmjhb 重新 review。謝謝！

[jlin53882]

CI 修復說明（commit 9d1e28e）

修復了兩個 CI 失敗

1. packaging-and-workflow — ✅ 已修補

根因：scripts/ci-test-manifest.mjs 已將 test/lock-stress-test.mjs 加入 CI manifest，但 scripts/verify-ci-test-manifest.mjs 的 EXPECTED_BASELINE 少了這一行，導致 verifyExactOnceCoverage() 失敗。

修復：在 EXPECTED_BASELINE 加入對應 entry。

---

2. storage-and-schema — ✅ 已修補

根因：f4db94c 新增的 regression test（test/cross-process-lock.test.mjs）使用了 TypeScript annotation：

let compromiseCallback: ((err: any) => void) | undefined;
lock: async (_lockPath: string, options: any) => {

CI 以 node --test 直接執行 .mjs 檔案（繞過 jiti 轉譯），Node.js 不支援 TypeScript 語法，因此回報 SyntaxError: Unexpected token ':'。

修復：移除 TypeScript annotation，改為 plain JavaScript：

let compromiseCallback;
lock: async (_lockPath, options) => {

---

cli-smoke 失敗說明（預存問題，與本 PR 無關）

cli-smoke 失敗是 upstream/環境問題：

• cli-smoke.mjs 和 src/tools.ts 都不在 f4db94c 的修改範圍內
• 本地 Windows 環境跑 node test/cli-smoke.mjs → OK: CLI smoke test passed
• CI 環境（Linux, Node.js v22.22.2）特有的行為差異，與本 PR 的 lock 修復無因果關係

---

修復後 CI 狀態

Job	結論
version-sync	✅
core-regression	✅
packaging-and-workflow	✅
llm-clients-and-auth	✅
storage-and-schema	✅
cli-smoke	❌（預存問題，非本 PR 造成）

[rwmjhb]

感谢这次修复——lockfile stale 阈值和 ECOMPROMISED fallback 解决的是真实的高负载崩溃问题。有几个阻塞项：

Must Fix

EF1/EF2 — Build failure + 2 个测试失败

strip-envelope-metadata.test.mjs:121,132 断言 subagent wrapper 前缀未被 stripEnvelopeMetadata() 剥离。AliceLJY 认为这是 pre-existing 问题，但 stale_base=true 意味着你的分支没有 main 上可能已有的修复。请先 rebase，确认失败仍然是 pre-existing 而非你的分支引入，并在 PR 中说明——不能带 unattributed 测试失败合并。

MR2 — stale 从 10s 提高到 60s，同时把真实崩溃进程的锁恢复时间延长了 6 倍

这是一个 tradeoff 需要明确：事件循环卡顿场景受益（减少误触发），但真实进程崩溃后其他进程等待锁释放的时间从 10s 变成 60s。是否有比单纯调大阈值更精准的方案（比如结合 onCompromised 回调）？

---

Nice to Have

• F2 (src/store.ts:219): ECOMPROMISED 的 catch block 在崩溃场景下是死代码——onCompromised 默认行为是在 setTimeout 回调中 throw，这是 Node.js 级别的 uncaught exception，PR 的 try/catch 此时已不活跃（lockfile.lock() promise 早已 resolve）。Issue #415 的 stack trace 也印证了这一点。如果要实现 graceful fallback，需要把恢复逻辑放在 onCompromised 回调里。
• F3 (test/lock-stress-test.mjs:82): Stress test 2 是顺序执行的——第一个 await store.store() 完成后锁已释放，第二个才开始，完全没有测到竞争场景。建议用 Promise.all([store.store(...), store.store(...)]) 并发触发。
• EF3: catch (err: any) 和 let fnError: any 在高风险文件 src/store.ts 的错误处理路径中新增了 3 处 any，建议改为 unknown + 类型收窄。

方向正确，rebase 确认 build 状态后可以合并。

[jlin53882]

@rwmjhb 請確認以下修改：

MR2 回覆：比單純調大閾值更精準的方案

分析：為何 stale=60000 不夠好？

stale=60000（上一版）是被動方案：只是等更久才觸發 callback。問題是：

• 真實崩潰後，competitor 等待 lock 釋放的時間從 10s → 60s（代價 6 倍）
• 如果 event loop 阻塞超過 60s（高負載伺服器常見），問題依然存在

MR2 方案：stale=10000 + lockfile.check() 主動區分

核心思路：在 onCompromised callback 裡，用 lockfile.check(lockPath, {stale: 2000}) 主動確認 holder 是否還活著。

stale=10000ms 時 onCompromised 觸發
    ↓
lockfile.check(lockPath, {stale: 2000})
    ↓
├─ check() 成功（2s 內 mtime 有更新）
│   → holder 還活著，只是慢（event loop 阻塞）
│   → warn 不 throw，讓 fn() 結果正常回傳 ✓
│
└─ check() 失敗（mtime 超過 2s 未更新）
    → holder 真的崩潰了
    → throw err，讓 caller 知道要重試 ✓

同時達成：

• ✅ 快速崩潰偵測（10s 內，與舊版相同）
• ✅ event loop 阻塞不會造成 false positive（check() 成功）
• ✅ 真實崩潰第一時間 throw（check() 失敗）

其他修改（依據審查建議）

項目	修改內容
EF3	compromisedErr: any → unknown，fnError: any → unknown，catch (err: any) → catch (err: unknown) + 類型收窄
F2	if (fnError !== null) 保留並加 clarifying comment（理論上 unreachable，因為 onCompromised 是 async throw）
F3	Test 2 改為 Promise.all() 真正並發請求，測試 lock 競爭情境
Regression test	mock lock() 加入 check() 模擬，確保 callback 在測試環境可正常運作

驗證

• node --test test/cross-process-lock.test.mjs → 6/6 pass ✅
• node --test test/lock-stress-test.mjs → 3/3 pass ✅
• TypeScript 編譯乾淨 ✅

[jlin53882]

PR 更新說明（commit 5984a34）

為何更新 PR

經過多輪 Codex 對抗審查，發現 MR2 的實作方向有根本性錯誤，所以做了重大修正並更新了 PR 描述。

---

MR2 變更摘要

原本方案	最終方案
async onCompromised + lockfile.check() 區分 crash vs 阻塞	同步 onCompromised 只設 flag
throw err 在 callback 裡	移除（setLockAsCompromised 不等 Promise，throw 無法傳回 caller）
stale=60000 被動方案	stale=10000（維持快速偵測）
check() 區分 crash vs 阻塞	移除（check() 無法區分，mtime 對兩者觀測結果相同）

---

為何 check() 無法區分 crash vs 阻塞

lockfile.check() 只問：stat.mtime < now - stale？

• Holder event loop 阻塞 >10s：mtime停在T=0，now - mtime = 12s > 2s → check 失敗
• Holder crash：mtime也停在T=0，now - mtime = 12s > 2s → check 失敗

兩者觀測結果完全相同，無法區分。

---

最終設計原則

接受 ECOMPROMISED 為 ambiguous degradation 訊號：

fn() 完成 + compromised → 回傳成功（資料已寫入，別重試）
fn() 失敗 + compromised → throw fnError（fn 的錯誤優先）
fn() 未完成 + compromised → throw compromisedErr
非 compromised → 正常 release

關鍵 insight：mtime 是 lock continuity 訊號，不是 process liveness 訊號。兩種 failure mode 在 lock 層面無法區分，但對 OpenClaw 場景不重要——只要資料寫入了，回傳成功就是對的。

---

PR 描述已同步更新

• 標題改為準確描述：proper-lockfile retries + ECOMPROMISED graceful handling
• 正文更新為完整技術說明
• CI 核心測試全部通過 ✅

@rwmjhb 請確認是否有其他問題。