fix(coding-agent): bounded mnemopi consolidate-on-dispose so /quit returns within ~2 s
/quit and /exit hung for many seconds because AgentSession.dispose() awaited MnemopiSessionState.dispose() unconditionally, and that path runs consolidate() (state.ts:421) which fires a fresh LLM fact extraction for the just-retained transcript and then awaits flushExtractions() per owned bank. One LLM round-trip per shutdown, no upper bound, no visible status. - Add a timeoutMs option to MnemopiSessionState.dispose. When the cap fires the in-flight consolidate is detached to the background and the SQLite handles close once it settles, so writes never race a closed handle. - AgentSession.dispose passes SHUTDOWN_CONSOLIDATE_BUDGET_MS = 1_500 on the user-visible shutdown path. Per-turn maybeRetainOnAgentEnd has already retained earlier turns, so the worst case is losing episodic promotion for the last few turns. State-replacement disposes (mnemopiBackend.start) stay unbounded. - InteractiveMode.shutdown surfaces a 'Closing session…' status before dispose runs so the brief pause is explained rather than mysterious. Two regression tests in memory-tools.test.ts cover (1) dispose returns within the budget when flushExtractions stalls and the deferred close still runs once consolidate settles, and (2) unbounded dispose still runs the full #2320 consolidate-then-close pipeline. Fixes #3641
This commit is contained in:
@@ -361,6 +361,12 @@
|
||||
- Fixed marketplace-installed plugins incorrectly appearing in both the npm plugin list and the extension-package status provider.
|
||||
- Fixed inconsistent OpenRouter prompt-cache hits on `/advisor` turns by ensuring advisor agents inherit the same provider-shaping options, hooks, and settings as the main agent.
|
||||
- Fixed path-scoped TTSR (Targeted Tool Safety Rules) evaluation for `hashline` and `apply_patch` edit streams, ensuring rules are correctly applied to file paths parsed from section headers and envelope markers without leaking across file scopes.
|
||||
- Prevented auto-generated session titles from accidentally re-shouting user all-caps text
|
||||
- Fixed auto-generated session titles re-shouting emphatic ALL-CAPS from the user's message. `reconcileTitleCasing` (`packages/coding-agent/src/tiny/text.ts`) restored any source token with interior/repeated uppercase, so shouting like "unify ALL ERROR HANDLING" turned the model's clean sentence case ("Unify error handling…") back into "Unify ERROR HANDLING…". Casing is now restored only from mixed-case identifiers the user typed deliberately (`TinyVMM`, `iOS`, `IDs`); pure all-caps is left to the model's own output.
|
||||
- Fixed Tavily web search with recency filters to retry once without `time_range` when Tavily returns HTTP 200 with no renderable content. ([#3633](https://github.com/can1357/oh-my-pi/issues/3633))
|
||||
- Fixed TUI thought stream stalling and `ui.loop-blocked` warnings during subagent-heavy runs by replacing the mid-run compaction persistence check's O(n²) branch rebuild + per-pair `JSON.stringify` content compare with a one-shot persistence-key snapshot. Content equality is preserved as the rare collision tiebreaker. ([#3629](https://github.com/can1357/oh-my-pi/issues/3629))
|
||||
- Fixed marketplace-installed plugins appearing in both the npm plugin list and the OMP extension-package status provider. ([#3628](https://github.com/can1357/oh-my-pi/issues/3628))
|
||||
- Fixed `/quit` and `/exit` blocking for many seconds before the session closed. `AgentSession.dispose()` (`packages/coding-agent/src/session/agent-session.ts`) awaited `MnemopiSessionState.dispose()` (`packages/coding-agent/src/mnemopi/state.ts`), which always ran the full consolidate pass — and consolidate fires a fresh LLM fact extraction for the just-retained transcript, so even minimal-activity sessions stalled on a 1–3 s round-trip with no visible status. Dispose now passes a 1 500 ms shutdown budget; anything still in flight is detached to the background, the SQLite handles close once it settles (so writes never race a closed handle), and `InteractiveMode.shutdown()` surfaces a `Closing session…` line while the dispose runs. Per-turn `maybeRetainOnAgentEnd` still retains earlier turns, so the worst case is losing episodic promotion for the last few turns. ([#3641](https://github.com/can1357/oh-my-pi/issues/3641))
|
||||
|
||||
## [16.2.1] - 2026-06-27
|
||||
|
||||
|
||||
@@ -433,19 +433,55 @@ export class MnemopiSessionState {
|
||||
* e.g. `mnemopiBackend.clear` — pass `{ consolidate: false }` to skip the
|
||||
* extraction/sleep pass, since spending tokens on memories that will be
|
||||
* wiped on the next line is wasted work (PR #2327 review).
|
||||
*
|
||||
* `timeoutMs` caps how long the consolidate await blocks the caller
|
||||
* (the user-visible `/quit` / `/exit` shutdown path passes this so
|
||||
* dispose returns within a UX budget — issue #3641). When the cap is
|
||||
* hit, dispose returns immediately and detaches the still-in-flight
|
||||
* consolidate; the SQLite handles are closed in the background once
|
||||
* the consolidate settles so writes never race a closed handle, and
|
||||
* any pending embeddings are SIGKILL'd along with the embed worker
|
||||
* (a tolerable loss — working memory rows are durable; only the
|
||||
* episodic promotion / embedding for the LAST few turns is skipped,
|
||||
* and `maybeRetainOnAgentEnd` has already retained earlier turns).
|
||||
*/
|
||||
async dispose(options: { consolidate?: boolean } = {}): Promise<void> {
|
||||
async dispose(options: { consolidate?: boolean; timeoutMs?: number } = {}): Promise<void> {
|
||||
this.unsubscribe?.();
|
||||
this.unsubscribe = undefined;
|
||||
if (this.aliasOf) return;
|
||||
if (options.consolidate !== false) {
|
||||
try {
|
||||
await this.consolidate();
|
||||
} catch (error) {
|
||||
logger.warn("Mnemopi: consolidation on dispose failed.", { error: String(error) });
|
||||
}
|
||||
const closeOwned = (): void => {
|
||||
for (const memory of this.scoped.owned) memory.close();
|
||||
};
|
||||
if (options.consolidate === false) {
|
||||
closeOwned();
|
||||
return;
|
||||
}
|
||||
for (const memory of this.scoped.owned) memory.close();
|
||||
const consolidatePromise = this.consolidate().catch((error: unknown) => {
|
||||
logger.warn("Mnemopi: consolidation on dispose failed.", { error: String(error) });
|
||||
});
|
||||
const { timeoutMs } = options;
|
||||
if (timeoutMs !== undefined && timeoutMs > 0) {
|
||||
const TIMED_OUT = Symbol("mnemopi.dispose.timedOut");
|
||||
const winner = await Promise.race([
|
||||
consolidatePromise.then(() => undefined as unknown),
|
||||
Bun.sleep(timeoutMs).then(() => TIMED_OUT as unknown),
|
||||
]);
|
||||
if (winner === TIMED_OUT) {
|
||||
logger.warn("Mnemopi: consolidate-on-dispose exceeded shutdown budget; detaching to background.", {
|
||||
timeoutMs,
|
||||
});
|
||||
// Defer close until the in-flight consolidate settles so SQLite
|
||||
// writes don't race a closed handle. The process is on the way
|
||||
// to `postmortem.quit(0)`; if it exits first, the OS reclaims
|
||||
// the handles (and a still-pending embed() goes down with the
|
||||
// embed worker the caller is about to SIGKILL).
|
||||
void consolidatePromise.finally(closeOwned);
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
await consolidatePromise;
|
||||
}
|
||||
closeOwned();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3303,6 +3303,13 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
this.#omfgController.dispose();
|
||||
this.#focusController.dispose();
|
||||
|
||||
// Surface an explicit "Closing session…" line so the user sees a reason
|
||||
// for the pause while `session.dispose()` flushes memory consolidate and
|
||||
// other cleanups (issue #3641). The await on the next line yields the
|
||||
// event loop, giving requestRender() a tick to paint the status before
|
||||
// dispose blocks.
|
||||
this.showStatus("Closing session…");
|
||||
|
||||
// Persist the draft and dispose the session through the shared teardown
|
||||
// so a signal that arrives mid-shutdown cannot fire a second dispose.
|
||||
// The teardown is a promise-memoized singleton; whichever path calls it
|
||||
|
||||
@@ -445,6 +445,18 @@ const UNEXPECTED_STOP_MAX_RETRIES = 3;
|
||||
const UNEXPECTED_STOP_TIMEOUT_MS = 4000;
|
||||
const EMPTY_STOP_MAX_RETRIES = 3;
|
||||
const RETRY_BACKOFF_MAX_DELAY_MS = 8_000;
|
||||
/**
|
||||
* Cap how long {@link AgentSession.dispose} waits for
|
||||
* `MnemopiSessionState.dispose()` to finish its consolidate pass on the
|
||||
* user-visible `/quit` / `/exit` shutdown path. Consolidate fires fresh
|
||||
* LLM fact extractions, each a 1–3 s round-trip, so an unbounded await
|
||||
* stalled `/quit` for many seconds even on minimal-activity sessions
|
||||
* (issue #3641). Anything still in flight when the budget elapses is
|
||||
* detached to the background; the SQLite handles close once it settles.
|
||||
* Per-turn `maybeRetainOnAgentEnd` already retained earlier turns, so
|
||||
* the worst case is losing episodic promotion for the LAST few turns.
|
||||
*/
|
||||
const SHUTDOWN_CONSOLIDATE_BUDGET_MS = 1_500;
|
||||
|
||||
type CompactionCheckResult = Readonly<{
|
||||
deferredHandoff: boolean;
|
||||
@@ -5437,7 +5449,7 @@ export class AgentSession {
|
||||
this.setHindsightSessionState(undefined);
|
||||
hindsightState?.dispose();
|
||||
const mnemopiState = setMnemopiSessionState(this, undefined);
|
||||
await mnemopiState?.dispose();
|
||||
await mnemopiState?.dispose({ timeoutMs: SHUTDOWN_CONSOLIDATE_BUDGET_MS });
|
||||
// Tear down the embeddings subprocess AFTER mnemopi state.dispose:
|
||||
// consolidate-on-dispose may still call `embed()` to store the final
|
||||
// memories, and that round-trips through the worker we are about to
|
||||
|
||||
@@ -574,6 +574,61 @@ describe("Mnemopi backend lifecycle", () => {
|
||||
registeredMnemopiState = undefined;
|
||||
});
|
||||
|
||||
it("dispose({ timeoutMs }) returns within the budget when consolidate stalls (#3641)", async () => {
|
||||
const state = registerMnemopiState();
|
||||
const retainMemory = state.getScopedRetainTarget().memory;
|
||||
// Hold flushExtractions hostage longer than any reasonable shutdown budget
|
||||
// so the race exclusively settles via the timeout branch.
|
||||
const flushStall = Promise.withResolvers<void>();
|
||||
let flushCalls = 0;
|
||||
const flushSpy = vi.spyOn(retainMemory, "flushExtractions").mockImplementation(async () => {
|
||||
flushCalls++;
|
||||
await flushStall.promise;
|
||||
});
|
||||
const closeSpy = vi.spyOn(retainMemory, "close");
|
||||
|
||||
const BUDGET_MS = 100;
|
||||
const start = Bun.nanoseconds();
|
||||
await state.dispose({ timeoutMs: BUDGET_MS });
|
||||
const elapsedMs = (Bun.nanoseconds() - start) / 1_000_000;
|
||||
|
||||
// Dispose must surrender within the budget (plus a generous slack); the
|
||||
// in-flight consolidate is detached, not awaited.
|
||||
expect(elapsedMs).toBeLessThan(BUDGET_MS * 5);
|
||||
expect(elapsedMs).toBeGreaterThanOrEqual(BUDGET_MS - 10);
|
||||
expect(flushSpy).toHaveBeenCalled();
|
||||
expect(flushCalls).toBe(1);
|
||||
// `close()` is deferred so SQLite writes don't race a closed handle.
|
||||
expect(closeSpy).not.toHaveBeenCalled();
|
||||
|
||||
// Release the stall and confirm the deferred close runs once consolidate
|
||||
// settles — i.e. the SQLite handle still ends up released eventually.
|
||||
flushStall.resolve();
|
||||
await Bun.sleep(50);
|
||||
expect(closeSpy).toHaveBeenCalledTimes(1);
|
||||
|
||||
registeredMnemopiState = undefined;
|
||||
});
|
||||
|
||||
it("dispose with no timeoutMs awaits consolidate to completion (#3641 — preserves #2320 contract)", async () => {
|
||||
const state = registerMnemopiState();
|
||||
const retainMemory = state.getScopedRetainTarget().memory;
|
||||
const flushSpy = vi.spyOn(retainMemory, "flushExtractions").mockResolvedValue();
|
||||
const sleepSpy = vi.spyOn(retainMemory, "sleepAllSessions");
|
||||
const closeSpy = vi.spyOn(retainMemory, "close");
|
||||
|
||||
await state.dispose();
|
||||
|
||||
// Unbounded dispose still runs the full consolidate-then-close pipeline,
|
||||
// matching the #2320 contract for non-shutdown callers (state replacement
|
||||
// during `mnemopiBackend.start`, etc.).
|
||||
expect(flushSpy).toHaveBeenCalledTimes(1);
|
||||
expect(sleepSpy).toHaveBeenCalledTimes(1);
|
||||
expect(closeSpy).toHaveBeenCalledTimes(1);
|
||||
|
||||
registeredMnemopiState = undefined;
|
||||
});
|
||||
|
||||
it("skips consolidation when disposing an aliased subagent state (#2320)", async () => {
|
||||
const settings = Settings.isolated({ "memory.backend": "mnemopi" });
|
||||
const parentState = registerMnemopiState();
|
||||
|
||||
Reference in New Issue
Block a user