refactor(router): isolate API-key selection capture

devlog/_plan/260907_router_selection_capture/010_implementation.md

@@ -0,0 +1,32 @@
+# Issue #3894: implementation plan
+
+Satisfy-spec work, triggered by issue #3894 and the request to implement separate draft PRs. Goal: remove the direct router/selection mutation dependency. Non-goals: changing key resolution/failover or eliminating all transitive router cycles. Stop after verified draft PR; report unresolved gates. Escalate if extraction requires behavioral changes. This file records plan and evidence.
+
+Class C2: one pure helper extracted in the existing provider module convention. Independent branch from 522ce5f8c.
+
+Current map: router imports api-key-selection for capture; api-key-selection imports router for route resolution. Existing direct helper callers: router and matchesSelection. No package exports change.
+Chosen map: both modules import api-key-selection-capture; the old api-key-selection export forwards the same function. New leaf uses only existing OcxProviderConfig and ProviderApiKeySelection type imports.
+Rejected alternative: extracting routedProviderConfig would move broad routing dependencies. Other existing transitive cycles stay outside scope.
+
+File map:
+- NEW src/providers/api-key-selection-capture.ts: the existing function body unchanged, plus the two type-only imports.
+- MODIFY src/providers/api-key-selection.ts: replace local implementation with named import and compatibility re-export.
+- MODIFY src/router.ts: change capture import to the leaf.
+- NEW tests/providers/api-key-selection-capture.test.ts: selected/unmatched/missing/duplicate pool cases, immutable snapshot, old-export identity, and Bun-parsed runtime import boundary for leaf and router. Parse actual source, exclude erased type imports; no whole-router acyclicity assertion.
+- MODIFY scripts/test-layout/layout.json and tests/fixtures/test-layout-expected.json: register the new test using existing providers domain entries.
+- MODIFY structure/01_runtime.md: document the helper ownership and preserved stateful selection direction.
+
+Verification: helper tests; key-failover, provider-key-store and core-lab-boundary tests; test-layout guards; typecheck; privacy scan. Baseline focused run on unchanged code: 49 passed. Conditional test cases have concrete provider objects; boundary regression returns offending imports rather than scanning prose.
+
+Audit: extraction is a functional dependency with no mutable globals and no changed auth behavior. Old export is preserved. A boundary test targets this exact scope; broad graph cycles are not called fixed.
+
+## Verification before draft publication
+
+- `bun install --frozen-lockfile`: passed; lockfile unchanged.
+- Baseline key-failover/provider-key-store/Lab-boundary run: 49 passed.
+- Restoring the old router import made the new boundary regression fail; restoring the extraction returned it to green.
+- `bun test tests/providers/api-key-selection-capture.test.ts tests/adapters/key-failover.test.ts tests/providers/provider-key-store.test.ts tests/lab/core-lab-boundary.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts`: 73 passed, 0 failed.
+- `bun run typecheck`: passed.
+- `bun run privacy:scan`: passed.
+- Review scope covers the 10-line pure helper, two consumers, compatibility re-export, 7 new tests, two layout entries, and the runtime ownership row. No other router cycle is claimed resolved.
+- Whole-suite/maintainer approval is not attested; this is a draft handoff.

scripts/test-layout/layout.json

@@ -222,6 +222,7 @@
     "api-codex-log-guard.test.ts": "server",
     "api-debug.test.ts": "server",
     "api-key-attribution.test.ts": "server",
+    "api-key-selection-capture.test.ts": "providers",
     "api-keys-routes.test.ts": "server",
     "api-storage-cleanup.test.ts": "storage",
     "api-storage-policy-already-running.test.ts": "storage",

src/providers/api-key-selection-capture.ts

@@ -0,0 +1,10 @@
+import type { OcxProviderConfig } from "../types";
+import type { ProviderApiKeySelection } from "../types/provider";
+
+export function captureProviderApiKeySelection(provider: OcxProviderConfig): ProviderApiKeySelection {
+  return {
+    entryId: provider.apiKeyPool?.find(entry => entry.key === provider.apiKey)?.id,
+    reference: provider.apiKey,
+    revision: provider.apiKeySelectionRevision,
+  };
+}

src/providers/api-key-selection.ts

@@ -6,14 +6,9 @@ import type { ProviderApiKeySelection } from "../types/provider";
 import { routedProviderConfig } from "../router";
 import { OPENCODE_GO_SESSION_HEADER } from "./opencode-go-transport";
 import { resolveProviderTransport, XAI_GROK_COMPATIBILITY, type OcxProviderTransport } from "./xai-transport";
+import { captureProviderApiKeySelection } from "./api-key-selection-capture";
 
-export function captureProviderApiKeySelection(provider: OcxProviderConfig): ProviderApiKeySelection {
-  return {
-    entryId: provider.apiKeyPool?.find(entry => entry.key === provider.apiKey)?.id,
-    reference: provider.apiKey,
-    revision: provider.apiKeySelectionRevision,
-  };
-}
+export { captureProviderApiKeySelection } from "./api-key-selection-capture";
 
 function matchesSelection(provider: OcxProviderConfig, expected: ProviderApiKeySelection): boolean {
   const current = captureProviderApiKeySelection(provider);

src/router.ts

@@ -10,7 +10,7 @@ import {
 import type { NormalizedComboConfig } from "./combos/types";
 import { hasOwnProvider } from "./config/provider-name";
 import { providerUsesKeyAuthOverride, resolveProviderApiKey } from "./providers/key-store";
-import { captureProviderApiKeySelection } from "./providers/api-key-selection";
+import { captureProviderApiKeySelection } from "./providers/api-key-selection-capture";
 import { assertProviderDestinationAllowed } from "./lib/destination-policy";
 import { redactSecretString, redactUrlForLog } from "./lib/redact";
 import {

structure/01_runtime.md

@@ -16,6 +16,7 @@
 | `src/server/ports.ts` | Owns bind availability and ephemeral-port selection. Temporary probes dispose accepted peers and wait for listener close before reporting success. |
 | `src/cli/status.ts` / `src/cli/status-probes.ts` | Status snapshot assembly and the shared read-only health/stale-process probes used by status and doctor. Probe evidence keeps recorded-port choice, before/after snapshots and per-call timer cleanup together. |
 | `src/router.ts` | Provider/model selection before adapter dispatch. Policy execution and ordinary management dry-run share effective-provider capability evidence; unresolved, missing, and disabled providers are excluded before scoring. |
+| `src/providers/api-key-selection-capture.ts` | Pure request-owned snapshot of the configured key entry, reference, and revision. The router and stateful selection module share this leaf with type-only dependencies; `api-key-selection.ts` retains the compatibility export and owns persisted selection changes and route resolution. |
 | `src/types.ts` | Shared config, parsed request, adapter, and event types. |
 | `src/reasoning-effort.ts` | Codex reasoning-level definitions (`low`/`medium`/`high`/`xhigh`), per-model effort mapping, and catalog effort sanitization. |
 | `src/codex/shim.ts` | Codex autostart shim: replaces the `codex` binary with a wrapper that auto-starts the proxy on demand. It skips startup for management subcommands even when value-taking global flags precede the subcommand, and transactionally restores complete, stable external launcher replacements without a watcher or PATH rediscovery. |

tests/fixtures/test-layout-expected.json

@@ -57,6 +57,7 @@
   "api-codex-log-guard.test.ts": "server",
   "api-debug.test.ts": "server",
   "api-key-attribution.test.ts": "server",
+  "api-key-selection-capture.test.ts": "providers",
   "api-keys-routes.test.ts": "server",
   "api-storage-cleanup.test.ts": "storage",
   "api-storage-policy-already-running.test.ts": "storage",

tests/providers/api-key-selection-capture.test.ts

@@ -0,0 +1,69 @@
+import { describe, expect, test } from "bun:test";
+import { readFileSync } from "node:fs";
+import { captureProviderApiKeySelection } from "../../src/providers/api-key-selection-capture";
+import { captureProviderApiKeySelection as legacyCapture } from "../../src/providers/api-key-selection";
+import type { OcxProviderConfig } from "../../src/types";
+import { repoPath } from "../helpers/repo-root";
+
+describe("API-key selection snapshot", () => {
+  const base: OcxProviderConfig = { adapter: "openai-chat", baseUrl: "https://example.test/v1" };
+
+  test("captures the selected entry and revision without resolving its reference", () => {
+    const provider: OcxProviderConfig = {
+      ...base,
+      apiKey: "${OCX_CAPTURE_FIXTURE}",
+      apiKeySelectionRevision: "revision-before",
+      apiKeyPool: [
+        { id: "other", key: "keychain:other" },
+        { id: "selected", key: "${OCX_CAPTURE_FIXTURE}" },
+      ],
+    };
+    const before = structuredClone(provider);
+    const snapshot = captureProviderApiKeySelection(provider);
+    expect(snapshot).toEqual({ entryId: "selected", reference: "${OCX_CAPTURE_FIXTURE}", revision: "revision-before" });
+    expect(provider).toEqual(before);
+    provider.apiKey = "keychain:other";
+    provider.apiKeySelectionRevision = "revision-after";
+    provider.apiKeyPool![1]!.id = "changed";
+    expect(snapshot).toEqual({ entryId: "selected", reference: "${OCX_CAPTURE_FIXTURE}", revision: "revision-before" });
+  });
+
+  test("retains an unmatched reference and absent optional fields", () => {
+    expect(captureProviderApiKeySelection(base)).toEqual({ entryId: undefined, reference: undefined, revision: undefined });
+    expect(captureProviderApiKeySelection({ ...base, apiKey: "keychain:unpooled", apiKeyPool: [] })).toEqual({
+      entryId: undefined, reference: "keychain:unpooled", revision: undefined,
+    });
+  });
+
+  test("preserves first-match semantics when a pool repeats the same reference", () => {
+    expect(captureProviderApiKeySelection({
+      ...base,
+      apiKey: "keychain:shared",
+      apiKeyPool: [{ id: "first", key: "keychain:shared" }, { id: "second", key: "keychain:shared" }],
+    })).toEqual({ entryId: "first", reference: "keychain:shared", revision: undefined });
+  });
+
+  test("preserves the existing export", () => {
+    expect(legacyCapture).toBe(captureProviderApiKeySelection);
+  });
+});
+
+describe("selection capture dependency boundary", () => {
+  const transpiler = new Bun.Transpiler({ loader: "ts" });
+  const runtimeImports = (source: string) => transpiler.scanImports(transpiler.transformSync(source)).map(entry => entry.path);
+
+  test("the leaf has no runtime imports", () => {
+    expect(runtimeImports(readFileSync(repoPath("src/providers/api-key-selection-capture.ts"), "utf8"))).toEqual([]);
+  });
+
+  test("the router consumes capture without a direct import of the stateful selection module", () => {
+    const imports = runtimeImports(readFileSync(repoPath("src/router.ts"), "utf8"));
+    expect(imports).toContain("./providers/api-key-selection-capture");
+    expect(imports).not.toContain("./providers/api-key-selection");
+  });
+
+  test("the boundary scanner distinguishes erased types from a runtime dependency", () => {
+    expect(runtimeImports('import type { T } from "../router"; export const value = 1;')).toEqual([]);
+    expect(runtimeImports('import "../router"; export const value = 1;')).toEqual(["../router"]);
+  });
+});