fix: close 4 findings from Oracle audit wave 6 (key-files + resilience)

Two read-only Oracles (resilience spine: overflow/fallback/retry; key-files
subsystem). NO P0s — the periphery is converging. Cores verified safe (no builtin
fallback fan-out, retryable-only iteration, OpenCode+Pi subagent termination
parity, storage/versioning/validator/atomic-commit, XML escaping, AFT detection).
4 source-confirmed fixes; design/cache-adjacent items banked (D19-D21).

- Key-files child session deleted before validation (identify-key-files.ts).
  Completes the wave-3 retention-on-failure fix, which Oracle Q found incomplete:
  runKeyFilesLlm set succeeded=true on text-extraction and deleted the child
  session — BEFORE validateLlmOutput runs. An LLM that returned text but failed
  validation (bad JSON / doc / out-of-candidate path) lost its session, defeating
  debugging. Moved cleanup ownership to runKeyFilesTask: delete only after
  validation + commit succeed (taskCompleted), retain on any failure.

- Stale pinned key-file persists forever (identify-key-files.ts). The no_change
  short-circuit only checked every candidate is pinned, NOT that every pinned row
  is still a valid candidate — so a file that dropped out of the candidate set
  (renamed / below min_reads / doc-lockfile under an older policy) kept being
  injected. Added the reverse-membership clause so a stale row falls through to
  the LLM re-run, which validates against the candidate allow-set and drops it.

- abortChildRun could hang and mask the original error (model-suggestion-retry.ts).
  The best-effort child-session abort awaited client.session.abort() unbounded; a
  wedged abort endpoint would hang the caller and hide the timeout/abort error we
  need to surface. Race it against a 3s timer (the abort still proceeds
  server-side).

- Documented the overflow-detection plausible-limit floor (overflow-detection.ts):
  evaluated raising it to the [20k,3M] trusted-limit band but kept 1024 — overflow
  detection honors a provider EXPLICITLY stating its limit, and real 4096/8192
  local models exist (a test asserts 4096 is accepted); the band would discard a
  legitimate small-model signal. Comment-only.

Banked: detected-limit not model-keyed + missing Anthropic ">N maximum" extractor
(P-1/design), validation-aware fallback for dreamer/sidekick empty-output (P-2,
touches fallback design), transform-wrapper snapshot-restore (P-6 — its fix is the
full-array clone-per-pass the user rejects; needs a targeted-catch design),
persistent-storage fail-open-vs-closed (P), key-files injection-cache project key
+ Pi project-switch m1 replay (Q — cache-adjacent). See oracle-loop-findings.md.

Gate: plugin 2176/0, Pi tsc clean, tsc+biome clean.

Author: ualtinok

packages/plugin/src/features/magic-context/key-files/identify-key-files.ts

@@ -464,7 +464,7 @@ async function runKeyFilesLlm(args: {
     prompt: string;
     deadline: number;
     fallbackModels?: readonly string[];
-}): Promise<{ text: string; messages: unknown[] }> {
+}): Promise<{ text: string; messages: unknown[]; agentSessionId: string }> {
     const createResponse = await args.client.session.create({
         body: {
             ...(args.parentSessionId ? { parentID: args.parentSessionId } : {}),
@@ -477,46 +477,39 @@ async function runKeyFilesLlm(args: {
     });
     const agentSessionId = typeof created?.id === "string" ? created.id : null;
     if (!agentSessionId) throw new Error("Could not create key-file identification session.");
-    let succeeded = false;
-    try {
-        await shared.promptSyncWithModelSuggestionRetry(
-            args.client,
-            {
-                path: { id: agentSessionId },
-                query: { directory: args.projectPath },
-                body: {
-                    agent: DREAMER_AGENT,
-                    system: KEY_FILES_SYSTEM_PROMPT,
-                    parts: [{ type: "text", text: args.prompt, synthetic: true }],
-                },
-            },
-            {
-                timeoutMs: Math.min(Math.max(0, args.deadline - Date.now()), 5 * 60 * 1000),
-                fallbackModels: args.fallbackModels,
-                callContext: "dreamer:key-files-v6",
-            },
-        );
-        const messagesResponse = await args.client.session.messages({
+    // NOTE: child-session cleanup is OWNED BY THE CALLER (runKeyFilesTask), not
+    // here. Deleting on text-extraction success would discard the session BEFORE
+    // validateLlmOutput runs — so an LLM that returned text but failed validation
+    // (bad JSON, doc/out-of-candidate path) would lose its session, defeating
+    // retention-on-failure debugging. The caller deletes only after validation +
+    // commit succeed, and retains on any failure.
+    await shared.promptSyncWithModelSuggestionRetry(
+        args.client,
+        {
             path: { id: agentSessionId },
-            query: { directory: args.projectPath, limit: 50 },
-        });
-        const messages = shared.normalizeSDKResponse(messagesResponse, [] as unknown[], {
-            preferResponseOnMissingData: true,
-        });
-        const text = extractLatestAssistantText(messages);
-        if (!text) throw new Error("Dreamer returned no key-files output.");
-        succeeded = true;
-        return { text, messages };
-    } finally {
-        // Keep the child session on failure (debugging) — mirrors the main-task
-        // cleanup rule; this try/finally has no catch, so a throw leaves
-        // succeeded=false and the session is retained.
-        if (succeeded && !shouldKeepSubagents()) {
-            await args.client.session
-                .delete({ path: { id: agentSessionId } })
-                .catch(() => undefined);
-        }
-    }
+            query: { directory: args.projectPath },
+            body: {
+                agent: DREAMER_AGENT,
+                system: KEY_FILES_SYSTEM_PROMPT,
+                parts: [{ type: "text", text: args.prompt, synthetic: true }],
+            },
+        },
+        {
+            timeoutMs: Math.min(Math.max(0, args.deadline - Date.now()), 5 * 60 * 1000),
+            fallbackModels: args.fallbackModels,
+            callContext: "dreamer:key-files-v6",
+        },
+    );
+    const messagesResponse = await args.client.session.messages({
+        path: { id: agentSessionId },
+        query: { directory: args.projectPath, limit: 50 },
+    });
+    const messages = shared.normalizeSDKResponse(messagesResponse, [] as unknown[], {
+        preferResponseOnMissingData: true,
+    });
+    const text = extractLatestAssistantText(messages);
+    if (!text) throw new Error("Dreamer returned no key-files output.");
+    return { text, messages, agentSessionId };
 }
 
 export async function runKeyFilesTask(args: {
@@ -557,10 +550,18 @@ export async function runKeyFilesTask(args: {
             }
         });
     const currentPaths = new Set(currentRows.map((row) => row.path));
-    if (
-        allRowsFreshAndCurrent &&
-        candidates.every((candidate) => currentPaths.has(candidate.path))
-    ) {
+    const candidatePaths = new Set(candidates.map((candidate) => candidate.path));
+    // Short-circuit ONLY when the pinned set exactly matches the current candidate
+    // set: every candidate is already pinned AND every pinned row is still a valid
+    // candidate. The second clause is the fix — without it, a pinned file that
+    // dropped out of the candidate set (renamed, fell below min_reads, or a doc/
+    // lockfile under an older policy) would be treated as "no change" and injected
+    // forever. When a stale row exists we fall through to the LLM re-run, which
+    // validates against the candidate allow-set and drops it.
+    const pinnedMatchesCandidates =
+        candidates.every((candidate) => currentPaths.has(candidate.path)) &&
+        currentRows.every((row) => candidatePaths.has(row.path));
+    if (allRowsFreshAndCurrent && pinnedMatchesCandidates) {
         log(`key-files: no_change short-circuit (${currentRows.length} rows fresh)`);
         return { committedVersion: null, candidates: candidates.length, noChange: true };
     }
@@ -585,6 +586,11 @@ export async function runKeyFilesTask(args: {
     // pushed in the dreamer runner ("key files") so the dashboard maps tokens
     // to the right row.
     let invocationRecorded = false;
+    // Child-session cleanup is owned here (not in runKeyFilesLlm) so it happens
+    // only AFTER validation + commit succeed — a validation failure retains the
+    // session for debugging (retention-on-failure parity with the main task).
+    let childSessionId: string | null = null;
+    let taskCompleted = false;
     const llmStartedAt = Date.now();
     const recordKeyFilesInvocation = (params: {
         status: "completed" | "failed";
@@ -607,14 +613,19 @@ export async function runKeyFilesTask(args: {
     };
     try {
         try {
-            const { text: raw, messages: llmMessages } = await runKeyFilesLlm({
+            const {
+                text: raw,
+                messages: llmMessages,
+                agentSessionId,
+            } = await runKeyFilesLlm({
                 client: args.client,
                 parentSessionId: args.parentSessionId,
                 projectPath,
                 prompt,
                 deadline: args.deadline,
                 fallbackModels: args.fallbackModels,
             });
+            childSessionId = agentSessionId;
             // The LLM call completed (tokens spent) — record before validation,
             // which is post-processing that can fail independently.
             recordKeyFilesInvocation({ status: "completed", messages: llmMessages });
@@ -629,8 +640,10 @@ export async function runKeyFilesTask(args: {
             log(`[key-files] LLM validation failed: ${getErrorMessage(error)}`);
             throw error;
         }
-        if (validated.no_change)
+        if (validated.no_change) {
+            taskCompleted = true;
             return { committedVersion: null, candidates: candidates.length, noChange: true };
+        }
         const committedVersion = commitKeyFiles({
             db: args.db,
             projectPath,
@@ -640,8 +653,16 @@ export async function runKeyFilesTask(args: {
             leaseHolderId: args.holderId,
         });
         renewLease(args.db, args.holderId);
+        taskCompleted = true;
         return { committedVersion, candidates: candidates.length, noChange: false };
     } finally {
         clearInterval(leaseInterval);
+        // Delete the child session only on full success; retain on any failure
+        // (validation/commit) for debugging, unless keep_subagents is set.
+        if (childSessionId && taskCompleted && !shouldKeepSubagents()) {
+            await args.client.session
+                .delete({ path: { id: childSessionId } })
+                .catch(() => undefined);
+        }
     }
 }

packages/plugin/src/features/magic-context/overflow-detection.ts

@@ -69,7 +69,10 @@ const LIMIT_EXTRACTION_PATTERNS: ReadonlyArray<RegExp> = [
 ];
 
 /** Minimum plausible context limit. Anything smaller is probably a match
- *  against an unrelated number in the error (e.g., error code). */
+ *  against an unrelated number in the error (e.g., error code). Kept at 1024
+ *  (NOT the [20k,3M] trusted-limit band): overflow detection honors a provider
+ *  EXPLICITLY stating its limit, and real small-context models exist (4096/8192
+ *  local llama.cpp) — raising this floor would discard a legitimate signal. */
 const MIN_PLAUSIBLE_LIMIT = 1024;
 /** Maximum plausible context limit. Anything larger is very likely a false
  *  match against a token-count field rather than a limit. */

packages/plugin/src/shared/model-suggestion-retry.ts

@@ -6,6 +6,11 @@ import { parseProviderModel } from "./resolve-fallbacks";
 
 type Client = ReturnType<typeof createOpencodeClient>;
 
+/** Max time to wait for the best-effort child-session abort HTTP call before
+ *  giving up on its response (the abort still proceeds server-side). Keeps a
+ *  wedged abort endpoint from masking the original timeout/abort error. */
+const ABORT_CALL_TIMEOUT_MS = 3000;
+
 type PromptBody = {
     model?: { providerID: string; modelID: string };
     [key: string]: unknown;
@@ -171,7 +176,15 @@ async function promptWithTimeout(
  */
 async function abortChildRun(client: Client, sessionId: string): Promise<void> {
     try {
-        await client.session.abort({ path: { id: sessionId } });
+        // Bound the abort call: it's best-effort cleanup, and if the abort
+        // endpoint itself stalls (the runner is wedged) an unbounded await here
+        // would hang the caller and MASK the original timeout/abort error that we
+        // still need to surface. Race against a short timer; the abort keeps
+        // running server-side regardless of whether we wait for its response.
+        await Promise.race([
+            client.session.abort({ path: { id: sessionId } }),
+            new Promise<void>((resolve) => setTimeout(resolve, ABORT_CALL_TIMEOUT_MS)),
+        ]);
     } catch (error) {
         log(`[model-retry] child session abort failed for ${sessionId}: ${String(error)}`);
     }