fix(plugin): honor cancellation in host LLM fallback

[hahahahahayesyeseys]

Description

Fixes #2473

Caller-driven cancellation and expired deadlines were reported by the primary provider as LLM_TIMEOUT, so the client treated them like provider outages and started a Host LLM fallback. The stdio bridge then dropped the abort signal and extended the wait by five seconds, allowing an already-useless host request to outlive the caller's budget.

This change:

• skips Host fallback when the caller's signal is already aborted or its absolute deadline has passed, including the circuit-breaker path;
• carries deadlineAt and signal through the Host provider and both executable bridge entries;
• caps reverse-RPC timeout at the remaining caller deadline and removes the extra five-second padding;
• makes pending stdio server requests abortable and removes their timer and abort listener on every completion path;
• shares the ESM and CommonJS Host bridge adapter so both runtime entries keep identical budget semantics.

The existing behavior is preserved when a provider fails while the caller still has budget: Host fallback still runs once. The new deadlineAt field and stdio signal option are optional, so existing adapters and callers remain source-compatible. No new dependencies are required.

This is an internal runtime change with no user-interface output, so screenshots or recordings are not applicable. The in-repository LLM behavior documentation was updated.

Related Issue (Required): Fixes #2473

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (does not change functionality, e.g. code style improvements, linting)
• Documentation update

How Has This Been Tested?

Red/green regression evidence on upstream/main (a7367d0): the initial targeted run had 5 failures covering caller abort, expired deadline, missing deadline forwarding, and stdio abort handling. The final targeted run passes all 44 tests.

• Unit Test
  • npm exec vitest run tests/unit/llm/client.test.ts tests/unit/bridge/stdio.test.ts tests/unit/bridge/host-llm-budget.test.ts -- --reporter=dot — 44/44 passed
  • Includes 128 concurrently cancelled reverse RPCs; all reject and leave zero pending timers.
• Test Script Or Test Steps (please provide)
  • npm test — 187/187 test files passed; 1607 passed, 2 skipped
  • npm run lint — passed
  • npm run build — passed
  • npm run check:hermes-version — passed
  • git diff --check — passed
• Pipeline Automated API Test (not applicable; this path is local stdio RPC and has no external API dependency)

Risk and validation limits

The main compatibility risk is changing reverse-RPC timeout behavior for callers that supply an absolute deadline; those requests now end at that deadline instead of waiting an additional five seconds. That is the intended contract and is covered by regression tests. Validation used the real newline-delimited stdio transport with controlled providers/transports, but did not call a live Hermes Python adapter or paid external LLM.

Checklist

• I have performed a self-review of my own code | 我已自行检查了自己的代码 (pending submitter review before publication)
• I have commented my code in hard-to-understand areas | 我已在难以理解的地方对代码进行了注释
• I have added tests that prove my fix is effective or that my feature works | 我已添加测试以证明我的修复有效或功能正常
• I have created related documentation issue/PR in MemOS-Docs (if applicable) | 我已在 MemOS-Docs 中创建了相关的文档 issue/PR（如果适用） (N/A: internal runtime fix; in-repository LLM documentation updated)
• I have linked the issue to this PR (if applicable) | 我已将 issue 链接到此 PR（如果适用）
• I have mentioned the person who will review this PR | 我已提及将审查此 PR 的人 — @CarltonXiang

Reviewer Checklist

• closes [Bug] memos-local-plugin: host LLM fallback fires after the caller's own signal/deadline cancels the primary, then ignores that signal (runs timeoutMs + 5 s) #2473
• Made sure Checks passed
• Tests have been provided

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2478
Task: b0af02aada1000ee
Base: main
Head: fix/issue-2473-cancel-host-fallback

🔍 OpenCodeReview found 11 issue(s) in this PR.

---

1. apps/memos-local-plugin/bridge/host-llm.ts (L21)

The magic number 60_000 is also hardcoded with the same value in stdio.ts (const timeoutMs = options?.timeoutMs ?? 60_000). Having the same default defined independently in two places risks them drifting out of sync when the default is changed. Consider extracting a shared constant (e.g. DEFAULT_LLM_TIMEOUT_MS) into a shared constants file and importing it in both locations.

---

2. apps/memos-local-plugin/bridge/host-llm.ts (L43)

result?.text silently falls back to "" when the field is missing or not a string. An empty string is indistinguishable from a genuine (but empty) LLM response. Downstream consumers (summarizers, JSON parsers, etc.) will silently process an empty result rather than failing loudly. Consider throwing an error or returning undefined here to surface malformed host responses explicitly:

if (typeof result?.text !== "string") {
  throw new Error(`host LLM response missing required 'text' field`);
}

---

3. apps/memos-local-plugin/bridge/host-llm.ts (L45-L47)

result?.model falls back to input.model ?? "". If both are absent, the returned model silently becomes "". Callers that use the returned model for logging, billing attribution, or routing will receive a misleading empty string rather than an explicit error. Since HostLlmCompletion.model is declared as required string, a missing model indicates a malformed response — consider throwing instead of coercing to "":

model:
  typeof result?.model === "string"
    ? result.model
    : input.model ??
      (() => { throw new Error("host LLM response missing required 'model' field"); })()

Or more readably, validate result before the return statement.

---

4. apps/memos-local-plugin/core/llm/host-bridge.ts (L21-L22)

The JSDoc says 'adapters must not outlive it', but deadlineAt is consumed by both the bridge layer (host-llm.ts checks remaining budget before dispatch) and the retry/fallback gate in client.ts. The phrase 'adapters' is narrower than the actual contract. Consider wording like: 'Unix-epoch ms of the absolute call deadline. All layers — retry loops, fallbacks, and bridge adapters — must abort once this point is passed.' This keeps the contract self-documenting for future readers and is consistent with the comment on LlmProviderCtx.deadlineAt ('providers must not renew it per retry').

💡 Suggested Change

Before:

  /** Absolute caller deadline; adapters must not outlive it. */
  deadlineAt?: number;

After:

  /** Absolute call deadline (Unix epoch ms). Retry loops, fallbacks, and bridge adapters must all abort once this point is passed. */
  deadlineAt?: number;

---

5. apps/memos-local-plugin/bridge/stdio.ts (L290-L296)

The abort path only rejects the local Promise and removes the pending entry — no cancellation notification is sent to the peer. Since bridge/host-llm.ts issues a host.llm.complete RPC call, aborting here leaves that in-flight request running indefinitely on the host side (LLM inference continues, tokens are consumed). If the JSON-RPC protocol supports a cancel notification, it should be sent here. If not, consider documenting this limitation explicitly, because callers receive a cancelled rejection while the host continues working.

💡 Suggested Change

Before:

      const abortHandler = signal
        ? () => {
            takeServerPending(id)?.reject(
              signal.reason ?? new Error(`serverRequest ${method} aborted`),
            );
          }
        : undefined;

After:

      const abortHandler = signal
        ? () => {
            if (takeServerPending(id)) {
              // Notify the peer that this request is being cancelled so it can
              // stop work (e.g. LLM inference) rather than silently discarding
              // its eventual response.
              writeLine({ jsonrpc: "2.0", method: "rpc.cancel", params: { id } });
              reject(signal.reason ?? new Error(`serverRequest ${method} aborted`));
            }
          }
        : undefined;

---

6. apps/memos-local-plugin/bridge/stdio.ts (L275-L277)

The early-abort rejection uses signal.reason directly. AbortSignal.reason is typed any and may be a non-Error value (e.g. a string or undefined). When reason is undefined the fallback creates a plain Error, which callers checking err.name === 'AbortError' will misclassify as a genuine failure. Consider normalising the reason to a DOMException('...', 'AbortError') when reason is absent, which is what the Web/Node standard AbortSignal.abort() produces by default.

💡 Suggested Change

Before:

    if (signal?.aborted) {
      return Promise.reject(signal.reason ?? new Error(`serverRequest ${method} aborted`));
    }

After:

    if (signal?.aborted) {
      return Promise.reject(
        signal.reason ?? new DOMException(`serverRequest ${method} aborted`, "AbortError"),
      );
    }

---

7. apps/memos-local-plugin/bridge/stdio.ts (L293)

Same issue as the early-abort path: the fallback inside abortHandler creates a plain Error instead of an AbortError-shaped error. Callers that distinguish cancellation from genuine errors via err.name === 'AbortError' (a widely used convention) will treat this as an unexpected failure.

💡 Suggested Change

Before:

              signal.reason ?? new Error(`serverRequest ${method} aborted`),

After:

              signal.reason ?? new DOMException(`serverRequest ${method} aborted`, "AbortError"),

---

8. apps/memos-local-plugin/bridge/stdio.ts (L130-L132)

for (const id of serverPending.keys()) iterates the live Map while takeServerPending(id) calls serverPending.delete(id) inside the loop body. Although ES2015 Map iteration is defined to skip already-deleted entries, this pattern is fragile and will confuse future maintainers. Snapshot the keys first to make the intent clear and safe.

💡 Suggested Change

Before:

    for (const id of serverPending.keys()) {
      takeServerPending(id)?.reject(err ?? new Error("stdio bridge closed"));
    }

After:

    for (const id of [...serverPending.keys()]) {
      takeServerPending(id)?.reject(err ?? new Error("stdio bridge closed"));
    }

---

9. apps/memos-local-plugin/bridge/stdio.ts (L104-L106)

The removeEventListener call in takeServerPending is redundant because the listener was already registered with { once: true }, which auto-removes it after the first invocation. This creates a subtle inconsistency: the teardown logic looks like it has two responsibilities, but removeEventListener only matters for the non-fire paths (timeout/bridge-close). While harmless, consider adding a comment to clarify why both mechanisms are present, or rely solely on { once: true } and drop the explicit remove.

💡 Suggested Change

Before:

    if (entry.signal && entry.abortHandler) {
      entry.signal.removeEventListener("abort", entry.abortHandler);
    }

After:

    // Explicitly remove the listener for the timeout/bridge-close paths;
    // the abort-fired path already auto-removed it via { once: true }.
    if (entry.signal && entry.abortHandler) {
      entry.signal.removeEventListener("abort", entry.abortHandler);
    }

---

10. apps/memos-local-plugin/core/llm/client.ts (L358)

The fallback gate is now duplicated and drifting. canUseHostFallback(opts) checks fallbackToHost + non-host provider + bridge presence + !callerBudgetIsGone(opts), while shouldFallback(...) re-implements the same first three checks and lacks the budget check — which is exactly why the extra !callerBudgetIsGone(opts) && has to be bolted on at the call site. Any future change to the budget rule (e.g. requiring a minimum remaining-ms margin before a host attempt is worth it) must be applied in two places, and the one that is missed will silently re-introduce the wasted-RPC bug this change fixes.

Suggestion: hoist callerBudgetIsGone to module scope (it only depends on opts, not on closure state) and let shouldFallback take opts and apply it as its first check, so both call sites share a single gate.

---

11. apps/memos-local-plugin/core/llm/client.ts (L185-L190)

callerBudgetIsGone calls Date.now() directly for the deadline check, while every other time-sensitive path in this file uses the injected clock breakerNow (or passes now from LlmCircuitBreakerConfig). This makes the deadline guard non-deterministic in unit tests unless vi.useFakeTimers() is in scope — the existing test at line 295 works only because Date.now() - 1 is already in the past at the moment the call is made, but a test that needs to simulate the deadline expiring during the primary call cannot do so with controlled time.

Consider accepting the injected clock, e.g.:

function callerBudgetIsGone(opts: LlmCallOptions | undefined, now: () => number = Date.now): boolean {
  return (
    opts?.signal?.aborted === true ||
    (opts?.deadlineAt !== undefined && now() >= opts.deadlineAt)
  );
}

and threading breakerNow (which is already in scope) through the call site.

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Environment preparation failed before any gating tests executed. Failed scopes: memos_local_plugin
Branch: fix/issue-2473-cancel-host-fallback