fix(wallet): address mint recovery review and clarify scope

docs/wallet-mint-recovery.md

@@ -0,0 +1,71 @@
+# Mint quote recovery: scope and troubleshooting
+
+`routstrd wallet recover` explicitly retries mint operations through coco using
+**their existing stored outputs**. It can restore signatures when a quote is
+already issued, and reopen failed operations when explicitly requested:
+
+```sh
+routstrd history --json
+routstrd wallet recover --op <operationId> --include-failed
+```
+
+Failed operations require explicit IDs over both HTTP and the CLI. A successful
+re-run on an already finalized operation is a no-op. Requests that exceed their
+wait budget are not cancelled; explicit retries skip the operation while the
+underlying work is outstanding.
+
+## What this fixes—and what it does not
+
+Coco already checks pending quotes on startup and the daemon periodically
+refreshes them. A quote paid while the daemon was offline does not, by itself,
+require a new issuance implementation.
+
+This change makes normal cleanup confirm UNPAID with the mint before failing an
+expired quote. PAID, ISSUED and unverified quotes remain pending. It also gives
+operators a recovery path for operations previously marked failed.
+
+It does **not** replace rejected outputs with fresh outputs on an active keyset.
+An inactive-keyset rejection can therefore remain retryable with zero recovery.
+Recovery reports coco's persisted mint error when available, rather than only a
+generic “remains pending” error.
+
+Do not infer that the production incidents were caused by keyset retirement.
+Before claiming those incidents are fixed, collect:
+
+- The affected operation IDs, quote IDs, state and persisted `error`.
+- A fresh remote quote state and, where provided, paid/issued amounts.
+- The keyset IDs in the stored outputs and the mint's current keyset metadata.
+- A reproduction showing existing recovery fails and the proposed fix succeeds.
+
+Inspect persisted operation data through a read-only database copy; do not edit
+rows or run recovery scripts concurrently with a daemon against the same wallet.
+Never share the mnemonic, output secrets, or full wallet database in a PR.
+
+A future fresh-output path must preserve original outputs for uncertain issuance
+and NUT-09 restore, allocate fresh deterministic counters safely, and coordinate
+with coco's watcher/processor. It needs its own integration tests before handling
+real funds.
+
+## Cleanup preview and force
+
+`wallet cleanup --dry-run` is local-only: it reports `mintQuoteCandidates`, not
+confirmed failures. `failedMintQuotes` and `leftForRecovery` are zero because no
+mint check or cleanup transition was performed. Send/melt counts remain planned
+cleanup counts in dry-run mode.
+
+`--force` deliberately bypasses mint confirmation and can strand paid sats in a
+failed operation. Prefer normal cleanup. Forced operations can be retried with
+`--op <operationId> --include-failed`, but recovery still depends on the mint
+accepting their stored outputs or restoring their signatures.
+
+## Integration and release notes
+
+The reopen helper uses private coco-core 1.0.1 methods. Retain real-Manager and
+HTTP fake-mint coverage, use frozen dependency installs, and re-run integration
+tests on coco upgrades. A controlled low-value live-mint smoke test remains
+recommended before release.
+
+PR #118 removes `cocod-client.ts`. When integrating that change, move recovery
+and cleanup contracts into its replacement `wallet-client.ts`, rename HTTP error
+references accordingly, and make recovery mandatory for the in-process client.
+This follow-up does not pull in #118's unrelated removal.

src/cli.ts

@@ -2045,7 +2045,9 @@ walletCmd
         });
         const answer = await new Promise<string>((resolve) => {
           rl.question(
-            "This will fail expired mint quotes confirmed unpaid, reclaim old pending sends, and cancel prepared melts. Continue? [y/N] ",
+            options.force
+              ? "WARNING: --force fails expired mint quotes WITHOUT checking the mint and may strand paid sats. It also reclaims old pending sends and cancels prepared melts. Continue? [y/N] "
+              : "This will fail expired mint quotes confirmed unpaid, reclaim old pending sends, and cancel prepared melts. Continue? [y/N] ",
             (value: string) => {
               rl.close();
               resolve(value.trim().toLowerCase());
@@ -2080,6 +2082,7 @@ walletCmd
           | {
               dryRun?: boolean;
               failedMintQuotes?: number;
+              mintQuoteCandidates?: number;
               leftForRecovery?: number;
               reclaimedSends?: number;
               cancelledMelts?: number;
@@ -2091,9 +2094,13 @@ walletCmd
         if (output) {
           const prefix = output.dryRun ? "Would clean up:" : "Cleaned up:";
           console.log(prefix);
-          console.log(
+          if (output.dryRun) {
-            `  Expired mint quotes failed: ${output.failedMintQuotes ?? 0}`,
+            console.log(
-          );
+              `  Expired mint quote candidates (not checked with mint): ${output.mintQuoteCandidates ?? 0}`,
+            );
+          } else {
+            console.log(`  Expired mint quotes failed: ${output.failedMintQuotes ?? 0}`);
+          }
           console.log(
             `  Expired quotes kept for recovery (paid/issued/unverified): ${output.leftForRecovery ?? 0}`,
           );
@@ -2129,11 +2136,11 @@ walletCmd
 walletCmd
   .command("recover")
   .description(
-    "Re-issue PAID mint quotes whose sats were never claimed by checking each quote with its mint",
+    "Retry mint quotes using their stored outputs (does not replace rejected outputs)",
   )
   .option(
     "--op <id>",
-    "Recover only this operation id (repeatable; required to target failed operations)",
+    "Recover this operation id (repeatable; find IDs with routstrd history --json)",
     (value: string, previous: string[]) => [...previous, value],
     [] as string[],
   )

src/daemon/http/index.ts

@@ -289,10 +289,13 @@ function optionalStringArrayField(
 ): string[] | undefined {
   const value = body[field];
   if (value === undefined) return undefined;
-  if (!Array.isArray(value) || value.some((item) => typeof item !== "string")) {
+  if (
-    throw new CocodHttpError(400, `'${field}' must be an array of strings.`);
+    !Array.isArray(value) ||
+    value.some((item) => typeof item !== "string" || !item.trim())
+  ) {
+    throw new CocodHttpError(400, `'${field}' must be an array of non-empty strings.`);
   }
-  return value as string[];
+  return value.map((item: string) => item.trim());
 }
 
 function getCurrentMode(deps: DaemonDeps): ClientMode {
@@ -502,13 +505,27 @@ export function createDaemonRequestHandler(deps: {
         }
 
         const body = await readJsonBody(req);
+        const operationIds = optionalStringArrayField(body, "operationIds");
+        if (body.includeFailed === true && !operationIds?.length) {
+          throw new CocodHttpError(
+            400,
+            "'includeFailed' requires non-empty 'operationIds'.",
+          );
+        }
+        if (
+          body.timeoutMs !== undefined &&
+          (typeof body.timeoutMs !== "number" ||
+            !Number.isFinite(body.timeoutMs) || body.timeoutMs <= 0)
+        ) {
+          throw new CocodHttpError(
+            400,
+            "'timeoutMs' must be a positive finite number.",
+          );
+        }
         const result = await deps.walletClient.recoverMintQuotes({
-          operationIds: optionalStringArrayField(body, "operationIds"),
+          operationIds,
           includeFailed: body.includeFailed === true,
-          timeoutMs:
+          timeoutMs: body.timeoutMs as number | undefined,
-            typeof body.timeoutMs === "number" && Number.isFinite(body.timeoutMs)
-              ? body.timeoutMs
-              : undefined,
         });
         return { output: result };
       });

src/daemon/http/wallet-recovery.test.ts

@@ -0,0 +1,49 @@
+import { describe, expect, it, mock } from "bun:test";
+import { EventEmitter } from "node:events";
+import { createDaemonRequestHandler } from "./index";
+
+async function recover(body: unknown) {
+  const recoverMintQuotes = mock(async (_options: unknown) => ({ recovered: 0 }));
+  const handler = createDaemonRequestHandler({ walletClient: { recoverMintQuotes } } as never);
+  const req = new EventEmitter() as any;
+  Object.assign(req, { method: "POST", url: "/wallet/recover", headers: { host: "localhost" } });
+  const res = {
+    status: 0, body: "",
+    writeHead(status: number) { this.status = status; },
+    end(chunk: string) { this.body = chunk; },
+  };
+  setImmediate(() => {
+    req.emit("data", Buffer.from(JSON.stringify(body)));
+    req.emit("end");
+  });
+  await handler(req, res as never);
+  return { res, recoverMintQuotes };
+}
+
+describe("POST /wallet/recover validation", () => {
+  it.each([{}, { operationIds: [] }])("rejects includeFailed without explicit IDs: %j", async (body) => {
+    const { res, recoverMintQuotes } = await recover({ ...body, includeFailed: true });
+    expect(res.status).toBe(400);
+    expect(recoverMintQuotes).not.toHaveBeenCalled();
+  });
+  it.each([[""], ["  "], [42]])("rejects invalid operation IDs: %j", async (operationIds) => {
+    const { res, recoverMintQuotes } = await recover({ operationIds });
+    expect(res.status).toBe(400);
+    expect(recoverMintQuotes).not.toHaveBeenCalled();
+  });
+  it.each([0, -1, "1000"])("rejects invalid timeout %j", async (timeoutMs) => {
+    const { res, recoverMintQuotes } = await recover({ timeoutMs });
+    expect(res.status).toBe(400);
+    expect(recoverMintQuotes).not.toHaveBeenCalled();
+  });
+  it("passes normalized explicit IDs and a positive timeout", async () => {
+    const { res, recoverMintQuotes } = await recover({ operationIds: [" op-1 "], includeFailed: true, timeoutMs: 1000 });
+    expect(res.status).toBe(200);
+    expect(recoverMintQuotes).toHaveBeenCalledWith({ operationIds: ["op-1"], includeFailed: true, timeoutMs: 1000 });
+  });
+  it("still permits checking pending quotes without IDs", async () => {
+    const { res, recoverMintQuotes } = await recover({});
+    expect(res.status).toBe(200);
+    expect(recoverMintQuotes).toHaveBeenCalledTimes(1);
+  });
+});

src/daemon/wallet/cleanup.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "bun:test";
-import { selectCleanupOperations } from "./cleanup";
+import { selectCleanupOperations, summarizeMintCleanup } from "./cleanup";
 
 const NOW_MS = 1_800_000_000_000;
 const DAY_MS = 24 * 60 * 60 * 1000;
@@ -166,3 +166,14 @@ describe("selectCleanupOperations", () => {
     expect(result.meltsToCancel).toEqual([]);
   });
 });
+
+describe("mint cleanup reporting", () => {
+  it("reports dry-run candidates, not confirmed failures", () => {
+    expect(summarizeMintCleanup({ dryRun: true, candidates: 3, failed: 0, leftForRecovery: 0 }))
+      .toEqual({ mintQuoteCandidates: 3, failedMintQuotes: 0, leftForRecovery: 0 });
+  });
+  it("reports only actual failures in a real run", () => {
+    expect(summarizeMintCleanup({ dryRun: false, candidates: 3, failed: 1, leftForRecovery: 2 }))
+      .toEqual({ mintQuoteCandidates: 3, failedMintQuotes: 1, leftForRecovery: 2 });
+  });
+});

src/daemon/wallet/cleanup.ts

@@ -64,8 +64,8 @@ export interface CleanupSelection<
  *   can have happened before expiry while the daemon was down, leaving no
  *   local observation. Callers that fail quotes automatically at startup must
  *   therefore confirm UNPAID with the mint first (see
- *   settleExpiredMintQuotes in coco-client.ts); only the explicit,
+ *   failExpiredMintQuoteIfUnpaid in coco-client.ts). Explicit cleanup follows
- *   user-invoked cleanup command may fail candidates purely locally.
+ *   the same rule unless the operator opts into unsafe `--force` behaviour.
  * - Pending sends are reclaimed (rolled back) only when they are older than
  *   `minAgeMs`, so we never roll back a token that a receiver might still
  *   legitimately claim.
@@ -102,3 +102,17 @@ export function selectCleanupOperations<
 
   return { mintsToFail, sendsToReclaim, meltsToCancel };
 }
+
+/** Keep a local-only dry-run preview distinct from mint-confirmed outcomes. */
+export function summarizeMintCleanup(input: {
+  dryRun: boolean;
+  candidates: number;
+  failed: number;
+  leftForRecovery: number;
+}) {
+  return {
+    mintQuoteCandidates: input.candidates,
+    failedMintQuotes: input.dryRun ? 0 : input.failed,
+    leftForRecovery: input.dryRun ? 0 : input.leftForRecovery,
+  };
+}

src/daemon/wallet/coco-client.test.ts

@@ -1333,6 +1333,28 @@ describe("runMintQuoteRecovery", () => {
     expect(result.errors).toHaveLength(1);
   });
 
+  it("falls back to the original failure when diagnostic lookup fails", async () => {
+    const { source } = fakeSource([mintOp()], {
+      observe: async () => ({ category: "ready" }),
+      finalize: async () => { throw new Error("original failure"); },
+    });
+    source.ops.mint.get = async () => { throw new Error("lookup failed"); };
+    const result = await runMintQuoteRecovery(source);
+    expect(result).toMatchObject({ retryable: 1, recovered: 0 });
+    expect(result.errors).toEqual([{ operationId: "op-1", error: "original failure" }]);
+  });
+
+  it("bounds a hung diagnostic lookup after finalize fails", async () => {
+    const { source } = fakeSource([mintOp()], {
+      observe: async () => ({ category: "ready" }),
+      finalize: async () => { throw new Error("original failure"); },
+    });
+    source.ops.mint.get = () => new Promise(() => {});
+    const result = await runMintQuoteRecovery(source, { timeoutMs: 20 });
+    expect(result).toMatchObject({ retryable: 1, recovered: 0 });
+    expect(result.errors).toEqual([{ operationId: "op-1", error: "original failure" }]);
+  });
+
   it("bounds finalize so one hung mint cannot block recovery", async () => {
     const { source } = fakeSource([mintOp()], {
       observe: async () => ({ category: "ready" }),
@@ -1369,6 +1391,26 @@ describe("runMintQuoteRecovery", () => {
     expect(reopenFailedOperation).not.toHaveBeenCalled();
   });
 
+  it("requires explicit IDs when including failed operations", async () => {
+    const { source, reopenFailedOperation, observePendingOperation } = fakeSource([]);
+    await expect(runMintQuoteRecovery(source, { includeFailed: true })).rejects.toThrow(
+      "includeFailed requires explicit operationIds",
+    );
+    await expect(runMintQuoteRecovery(source, { includeFailed: true, operationIds: [] })).rejects.toThrow(
+      "includeFailed requires explicit operationIds",
+    );
+    expect(reopenFailedOperation).not.toHaveBeenCalled();
+    expect(observePendingOperation).not.toHaveBeenCalled();
+  });
+
+  it.each([0, -1, NaN, Infinity])("rejects invalid recovery timeout %s", async (timeoutMs) => {
+    const { source, observePendingOperation } = fakeSource([]);
+    await expect(runMintQuoteRecovery(source, { timeoutMs })).rejects.toThrow(
+      "timeoutMs must be a positive finite number",
+    );
+    expect(observePendingOperation).not.toHaveBeenCalled();
+  });
+
   it("re-opens a named failed operation, then mints it", async () => {
     const { source, reopenFailedOperation, finalize } = fakeSource(
       [mintOp({ state: "failed", lastObservedRemoteState: "PAID" })],

src/daemon/wallet/coco-client.ts

@@ -36,7 +36,7 @@ import type {
   WalletCleanupResult,
   WalletRecoveryProgress,
 } from "./cocod-client";
-import { selectCleanupOperations } from "./cleanup";
+import { selectCleanupOperations, summarizeMintCleanup } from "./cleanup";
 import {
   classifyMintQuoteObservation,
   selectMintQuotesForRecovery,
@@ -1032,7 +1032,13 @@ export async function runMintQuoteRecovery(
   options: MintQuoteRecoveryOptions = {},
   onProgress?: (message: string) => void,
 ): Promise<MintQuoteRecoveryResult> {
+  if (options.includeFailed && !options.operationIds?.length) {
+    throw new Error("includeFailed requires explicit operationIds");
+  }
   const timeoutMs = options.timeoutMs ?? MINT_QUOTE_RECOVERY_TIMEOUT_MS;
+  if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) {
+    throw new Error("timeoutMs must be a positive finite number");
+  }
   const outstanding =
     options.outstanding ?? new Map<string, Promise<unknown>>();
   const result: MintQuoteRecoveryResult = {
@@ -1215,7 +1221,18 @@ export async function runMintQuoteRecovery(
         onProgress?.(`${label}: another recovery is working on it; skipped`);
       } else {
         result.retryable++;
-        onProgress?.(`${label}: could not finish recovery: ${messageOf(error)}`);
+        // finalize can throw a generic "remains pending" error after coco has
+        // persisted the actionable mint rejection (for example inactive keyset).
+        const current = await withTimeout(
+          source.ops.mint.get(operationId),
+          remaining(),
+        ).catch(() => null);
+        const detail = current?.state === "pending" && current.error
+          ? current.error
+          : messageOf(error);
+        result.errors.push({ operationId, error: detail });
+        onProgress?.(`${label}: could not finish recovery: ${detail}`);
+        return;
       }
       result.errors.push({ operationId, error: messageOf(error) });
       return;
@@ -2337,11 +2354,14 @@ export async function createCocoClient(
         }
       }
 
-      const failedMintQuoteCount = dryRun
+      const mintSummary = summarizeMintCleanup({
-        ? selection.mintsToFail.length
+        dryRun,
-        : failedMintQuotes;
+        candidates: selection.mintsToFail.length,
+        failed: failedMintQuotes,
+        leftForRecovery,
+      });
       const actedOn =
-        failedMintQuoteCount +
+        (dryRun ? mintSummary.mintQuoteCandidates : mintSummary.failedMintQuotes) +
         leftForRecovery +
         selection.sendsToReclaim.length +
         selection.meltsToCancel.length;
@@ -2351,8 +2371,7 @@ export async function createCocoClient(
 
       return {
         dryRun,
-        failedMintQuotes: failedMintQuoteCount,
+        ...mintSummary,
-        leftForRecovery,
         reclaimedSends: selection.sendsToReclaim.length,
         cancelledMelts: selection.meltsToCancel.length,
         skipped,

src/daemon/wallet/cocod-client.ts

@@ -94,9 +94,11 @@ export interface WalletCleanupOptions {
 /** Summary of a wallet cleanup run. */
 export interface WalletCleanupResult {
   dryRun: boolean;
-  /** Number of expired pending mint quotes marked as failed. */
+  /** Expired quotes selected for checking; dry runs do not contact the mint. */
+  mintQuoteCandidates: number;
+  /** Number actually marked failed (always zero in a dry run). */
   failedMintQuotes: number;
-  /** Expired quotes whose mint reported PAID/ISSUED, left for recovery. */
+  /** Expired quotes kept pending because they are paid/issued or unverified. */
   leftForRecovery: number;
   /** Number of stale pending send operations reclaimed. */
   reclaimedSends: number;

src/daemon/wallet/mint-quote-recovery.fake-mint.test.ts

@@ -134,6 +134,36 @@ describe("PAID mint quote recovery with a real Manager and mint", () => {
     expect(await booted.spendable()).toBe(210_000);
   });
 
+  it("existing coco recovery already issues expired paid pending quotes", async () => {
+    booted = await boot({ quoteExpiry: -60 });
+    const op = await prepareQuote(booted, 100);
+    booted.mint.markPaid(op.quoteId as string);
+    await booted.manager.recoverPendingMintOperations();
+    expect(await booted.spendable()).toBe(100);
+    expect(booted.mint.getQuote(op.quoteId as string)?.state).toBe("ISSUED");
+  });
+
+  it("keeps rejected stored outputs and reports the actionable mint error", async () => {
+    booted = await boot({ quoteExpiry: -60 });
+    const op = await prepareQuote(booted, 100);
+    const outputs = outputsOf(op);
+    booted.mint.markPaid(op.quoteId as string);
+    // Model the mint refusing the stored outputs, not invoice expiry. This is
+    // not evidence that the production quotes used an inactive keyset.
+    booted.mint.mintError = { code: 12001, detail: "keyset id inactive." };
+    await booted.manager.recoverPendingMintOperations();
+    expect(await booted.spendable()).toBe(0);
+    const result = await runMintQuoteRecovery(booted.source() as never, {
+      operationIds: [op.id as string],
+    });
+    expect(result).toMatchObject({ recovered: 0, retryable: 1 });
+    expect(result.errors.some((entry) => entry.error.includes("keyset id inactive"))).toBe(true);
+    expect(await booted.spendable()).toBe(0);
+    expect(booted.mint.getQuote(op.quoteId as string)?.state).toBe("PAID");
+    expect(outputsOf(await booted.manager.ops.mint.get(op.id as string) as unknown as AnyRecord)).toEqual(outputs);
+    for (const request of booted.mint.requests) expect(request.outputs).toEqual(outputs);
+  });
+
   it("restores proofs for a quote already issued at the mint", async () => {
     booted = await boot({ quoteExpiry: null });
     const op = await prepareQuote(booted, 210_000);

src/daemon/wallet/mint-quote-recovery.ts

@@ -5,11 +5,14 @@
  * `pending` (the Lightning payment landed before expiry while the daemon was
  * down, so no local observation was ever recorded) or even terminally
  * `failed` (coco gives up when the mint refuses to sign, for example after the
- * invoice expiry). The paid sats are claimable either way: NUT-04 lets the
+ * invoice expiry). Claimability still depends on the mint accepting issuance.
- * holder submit outputs for any quote id while `amount_issued < amount_paid`.
+ * This feature retries the stored outputs or restores their signatures; it
+ * does not regenerate outputs rejected by the mint (for example an inactive
+ * keyset). coco already reconciles pending paid quotes at startup and in the
+ * periodic sweep. The new capability is operator-targeted recovery, including
+ * explicitly reopening failed operations, alongside safer cleanup.
  *
- * Recovery therefore has to ask the mint what it thinks, then re-issue the
+ * Recovery asks the mint what it thinks, then retries issuance or restore. These helpers decide *what* to do from a remote observation; the
- * quote. These helpers decide *what* to do from a remote observation; the
  * actual state transitions are applied by the in-process coco wallet client
  * so coco-core's operation services emit their normal events and release
  * proof reservations. Keeping the decisions here makes them unit testable