Fix one-shot exit hangs by tearing down cached memory managers by Julbarth · Pull Request #40367 · openclaw/openclaw

Julbarth · 2026-03-08T23:51:01Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: One-shot CLI runs can complete (message_end / agent_end) but the process may hang instead of exiting.
Why it matters: Short-lived automation/benchmark workflows can stall and timeout even when task execution succeeded.
What changed: Added explicit teardown for globally cached memory managers (index + search) and invoked teardown in runCli() finally.
What did NOT change (scope boundary): No changes to model logic, tool behavior, prompt flow, or memory retrieval semantics during normal execution.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes [Bug]: One-shot CLI can hang after completion due to cached memory manager FSWatcher handles #40365
Related #

User-visible / Behavior Changes

One-shot CLI commands now exit cleanly after completion in cases where cached memory manager watcher handles previously kept the process alive.

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (No)
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS 14.0 (Sonoma, 23A344)
Runtime/container: Node.js + pnpm (local dev)
Model/provider: N/A (provider-independent; reproduced with successful one-shot completion)
Integration/channel (if any): CLI one-shot run
Relevant config (redacted): default memory settings (memory search enabled)

Steps

Run OpenClaw main before this fix with default memory enabled.
Execute a one-shot CLI command that completes successfully.
Observe process state after completion event.

Expected

Process exits promptly after successful completion.

Actual

Process can remain alive due to active FSWatcher handles from cached memory managers.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
- Reproduced hang before fix on one-shot run.
- Confirmed clean exit after fix.
- Ran targeted tests:
  pnpm exec vitest run src/memory/search-manager.test.ts src/cli/run-main.exit.test.ts
Edge cases checked:
- Cleanup path executed via finally (success/failure/interrupt paths).
- Teardown is best-effort and does not crash CLI if cleanup throws.
What you did not verify:
- Full matrix across all OSes/containers/providers.
- Long-duration stress testing.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly: Revert this PR commit.
Files/config to restore: src/cli/run-main.ts, src/memory/manager.ts, src/memory/search-manager.ts, related test updates.
Known bad symptoms reviewers should watch for: unexpected errors during shutdown/teardown path.

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk: Teardown could run while some async cleanup is still in-flight.
- Mitigation: Cleanup is called in CLI terminal lifecycle (finally) and is best-effort with error handling.
Risk: Regressions in memory manager cache behavior.
- Mitigation: Added/updated targeted tests for teardown and CLI exit behavior.

greptile-apps · 2026-03-08T23:53:36Z

Greptile Summary

This PR fixes one-shot CLI hangs by adding explicit teardown of globally-cached memory managers (QMD_MANAGER_CACHE and INDEX_CACHE) in the runCli() finally block, ensuring FSWatcher handles and SQLite connections held by those managers are released before the process exits.

Key changes:

src/cli/run-main.ts — wraps the full CLI execution in try/finally, calling closeCliMemoryManagers() (best-effort, errors silenced) on every exit path including early returns and thrown errors.
src/memory/search-manager.ts — adds closeAllMemorySearchManagers() which snapshots and clears QMD_MANAGER_CACHE before closing each FallbackMemoryManager, then delegates to closeAllMemoryIndexManagers() to handle builtin-only cached managers.
src/memory/manager.ts — adds closeAllMemoryIndexManagers() which iterates and closes all entries in INDEX_CACHE. One gap: INDEX_CACHE_PENDING is not cleared, so an in-flight MemoryIndexManager.get() promise that resolves after teardown could re-insert a new manager (with an active FSWatcher) into the cleared cache. The race window is narrow for the targeted use case but is worth closing for correctness.
Tests correctly use vi.hoisted() for ESM mocking compatibility and include a dedicated teardown scenario.

Confidence Score: 4/5

Safe to merge — the fix correctly addresses the described hang and is backward compatible with a narrow edge case in teardown logic.
The core approach is sound: clearing module-level caches and closing their handles in a finally block is the right pattern for one-shot CLI teardown. All three close paths (QMD primary, QMD fallback, builtin-only) are handled. The one deduction is for INDEX_CACHE_PENDING not being cleared in closeAllMemoryIndexManagers(), which leaves a theoretical re-population race, though it is very unlikely to trigger in the targeted one-shot scenario since all operations are typically complete before the finally block runs.
Pay close attention to src/memory/manager.ts — specifically the closeAllMemoryIndexManagers() function's handling (or lack thereof) of INDEX_CACHE_PENDING.

_{Last reviewed commit: 9b3dc23}

greptile-apps · 2026-03-08T23:53:40Z

+export async function closeAllMemoryIndexManagers(): Promise<void> {
+  const managers = Array.from(INDEX_CACHE.values());
+  if (managers.length === 0) {
+    return;
+  }
+  for (const manager of managers) {
+    try {
+      await manager.close();
+    } catch (err) {
+      log.warn(`failed to close memory index manager: ${String(err)}`);
+    }
+  }
+  INDEX_CACHE.clear();
+}


INDEX_CACHE_PENDING not cleared during teardown

closeAllMemoryIndexManagers() clears INDEX_CACHE but leaves INDEX_CACHE_PENDING intact. If a MemoryIndexManager.get() call is still in-flight when teardown runs, the pending promise will eventually resolve and re-populate INDEX_CACHE with a brand-new manager (including its FSWatcher) that was never closed — exactly the hang this PR aims to fix.

The race window is narrow for the targeted one-shot CLI use case (all work is typically finished before teardown), but it's worth plugging for correctness:

Suggested change

export async function closeAllMemoryIndexManagers(): Promise<void> {

const managers = Array.from(INDEX_CACHE.values());

if (managers.length === 0) {

return;

}

for (const manager of managers) {

try {

await manager.close();

} catch (err) {

log.warn(`failed to close memory index manager: ${String(err)}`);

}

}

INDEX_CACHE.clear();

}

export async function closeAllMemoryIndexManagers(): Promise<void> {

const managers = Array.from(INDEX_CACHE.values());

INDEX_CACHE_PENDING.clear();

if (managers.length === 0) {

return;

}

for (const manager of managers) {

try {

await manager.close();

} catch (err) {

log.warn(`failed to close memory index manager: ${String(err)}`);

}

}

INDEX_CACHE.clear();

}

Clearing INDEX_CACHE_PENDING first prevents any concurrently-creating manager from re-inserting itself into the cache. Note the INDEX_CACHE.clear() is moved to after the close() loop so that manager.close() (which internally calls INDEX_CACHE.delete(this.cacheKey)) doesn't race with the bulk clear.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/memory/manager.ts Line: 45-58 Comment: **`INDEX_CACHE_PENDING` not cleared during teardown** `closeAllMemoryIndexManagers()` clears `INDEX_CACHE` but leaves `INDEX_CACHE_PENDING` intact. If a `MemoryIndexManager.get()` call is still in-flight when teardown runs, the pending promise will eventually resolve and re-populate `INDEX_CACHE` with a brand-new manager (including its `FSWatcher`) that was never closed — exactly the hang this PR aims to fix. The race window is narrow for the targeted one-shot CLI use case (all work is typically finished before teardown), but it's worth plugging for correctness: ```suggestion export async function closeAllMemoryIndexManagers(): Promise<void> { const managers = Array.from(INDEX_CACHE.values()); INDEX_CACHE_PENDING.clear(); if (managers.length === 0) { return; } for (const manager of managers) { try { await manager.close(); } catch (err) { log.warn(`failed to close memory index manager: ${String(err)}`); } } INDEX_CACHE.clear(); } ``` Clearing `INDEX_CACHE_PENDING` first prevents any concurrently-creating manager from re-inserting itself into the cache. Note the `INDEX_CACHE.clear()` is moved to after the `close()` loop so that `manager.close()` (which internally calls `INDEX_CACHE.delete(this.cacheKey)`) doesn't race with the bulk clear. How can I resolve this? If you propose a fix, please make it concise.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9b3dc232af

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-08T23:56:29Z

@@ -2,6 +2,7 @@ import type { OpenClawConfig } from "../config/config.js";
 import { createSubsystemLogger } from "../logging/subsystem.js";
 import type { ResolvedQmdConfig } from "./backend-config.js";
 import { resolveMemoryBackendConfig } from "./backend-config.js";
+import { closeAllMemoryIndexManagers } from "./manager.js";


Keep memory-manager teardown on the lazy runtime boundary

Importing closeAllMemoryIndexManagers directly from ./manager.js makes search-manager.ts eagerly load the same module that loadManagerRuntime() is trying to lazy-load, which defeats the runtime boundary and can pull in heavy memory/index dependencies on code paths that never use memory. This mixed static+dynamic loading pattern is exactly what the repo’s dynamic-import guardrail is intended to prevent, and it risks startup/perf regressions for routed CLI commands.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-08T23:56:29Z

+  if (managers.length === 0) {
+    return;


Drain pending index-manager creations during global teardown

This teardown exits early when INDEX_CACHE is empty, but it does not account for in-flight creations tracked in INDEX_CACHE_PENDING. If shutdown runs while a MemoryIndexManager.get() creation promise is still pending, that promise can still resolve after teardown and repopulate the cache with a live manager (including watcher handles), so the new one-shot exit fix can still miss the exact hang it is trying to prevent in cancellation/error timing windows.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fcaf4f069f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-09T00:12:04Z

 import { createSubsystemLogger } from "../logging/subsystem.js";
 import type { ResolvedQmdConfig } from "./backend-config.js";
 import { resolveMemoryBackendConfig } from "./backend-config.js";
+import { closeAllMemoryIndexManagers } from "./manager.js";


Keep index-manager teardown behind lazy runtime boundary

The new top-level import { closeAllMemoryIndexManagers } from "./manager.js" eagerly loads manager.js whenever search-manager.ts is imported, which bypasses the existing loadManagerRuntime() lazy-loading boundary used for memory internals. Because runCli() now calls memory teardown in finally, this turns many non-memory CLI paths into eager memory-module loads and can introduce avoidable startup/perf regressions on commands that never touch memory.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-09T00:12:04Z

+  if (managers.length === 0) {
+    return;


Drain pending manager creations during global teardown

closeAllMemoryIndexManagers() only closes instances currently in INDEX_CACHE and returns immediately when that cache is empty, but it never handles in-flight creations in INDEX_CACHE_PENDING. If shutdown happens while MemoryIndexManager.get() is still resolving, the pending promise can complete after teardown and repopulate INDEX_CACHE with a live manager (including watcher/timer handles), so one-shot runs can still hang in this race window.

Useful? React with 👍 / 👎.

openclaw-barnacle · 2026-03-09T00:19:20Z