fix(test): use withEnv()/emptyHome() in propose-takes no-key tests

check:test-isolation R1 flagged direct process.env mutation in the two
new no-key tests. Swap the hand-rolled save/mutate/restore for the
canonical withEnv() helper (+ emptyHome() for the hermetic GBRAIN_HOME).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/core/brainstorm/orchestrator.ts

@@ -485,9 +485,44 @@ const DEFAULT_PARALLELISM = 4;
  * src/core/errors.ts (the v0.19.0 envelope every new agent-facing
  * surface uses) rather than introducing a new BrainstormError class.
  */
+/** File-config slice the orchestrator reads (see loadConfig in core/config.ts). */
+export interface BrainstormRunConfig {
+  embedding_model?: string;
+  chat_model?: string;
+  emotional_weight?: { user_holder?: string };
+}
+
+/**
+ * Model used for the cost preview + hard cost ceiling. Mirrors what the
+ * gateway will actually run: explicit --model override, else the configured
+ * chat_model (gateway default), else the hardcoded gateway fallback. Before
+ * this resolved through config, a non-Sonnet chat_model got its preview
+ * priced against the wrong model. (Takeover of PR #1855 by @starm2010.)
+ */
+export function resolveBrainstormChatModel(
+  config: { chat_model?: string },
+  modelOverride?: string,
+): string {
+  return modelOverride ?? config.chat_model ?? 'anthropic:claude-sonnet-4-6';
+}
+
+/**
+ * Judge-phase model precedence: --judge-model flag, else the
+ * `models.brainstorm.judge` config key, else undefined (falls back to
+ * `modelOverride` then the gateway default at the runJudge callsite).
+ */
+export async function resolveBrainstormJudgeModel(
+  engine: BrainEngine,
+  judgeModelFlag?: string,
+): Promise<string | undefined> {
+  if (judgeModelFlag) return judgeModelFlag;
+  const configured = await engine.getConfig('models.brainstorm.judge');
+  return configured ?? undefined;
+}
+
 export async function runBrainstorm(
   engine: BrainEngine,
-  config: { embedding_model?: string; emotional_weight?: { user_holder?: string } },
+  config: BrainstormRunConfig,
   opts: BrainstormOptions
 ): Promise<BrainstormResult> {
   // v0.39.3.0 (Phase 5, CV11+T4): outer try/catch around the orchestrator
@@ -509,7 +544,7 @@ export async function runBrainstorm(
 
 async function runBrainstormImpl(
   engine: BrainEngine,
-  config: { embedding_model?: string; emotional_weight?: { user_holder?: string } },
+  config: BrainstormRunConfig,
   opts: BrainstormOptions,
 ): Promise<BrainstormResult> {
   // v0.39.0.0 T10: install a gateway-layer BudgetTracker scope around the
@@ -529,7 +564,7 @@ async function runBrainstormImpl(
 
 async function _runBrainstormInner(
   engine: BrainEngine,
-  config: { embedding_model?: string; emotional_weight?: { user_holder?: string } },
+  config: BrainstormRunConfig,
   opts: BrainstormOptions,
 ): Promise<BrainstormResult> {
   const profile = opts.profile ?? BRAINSTORM_PROFILE;
@@ -538,7 +573,7 @@ async function _runBrainstormInner(
   const embedFn = opts.embedQueryFn ?? embedQuery;
 
   // ---- Phase 0: cost preview + TTY grace ----
-  const modelStr = opts.modelOverride ?? 'anthropic:claude-sonnet-4-6';
+  const modelStr = resolveBrainstormChatModel(config, opts.modelOverride);
   const { aborted, estimate } = await previewCostAndWait({
     profile,
     model: modelStr,
@@ -847,7 +882,7 @@ async function _runBrainstormInner(
       far_slug: i.far_slug,
     }));
     const judgeResult = await runJudge(profile.judge_config, judgeInput, {
-      modelOverride: opts.judgeModel ?? opts.modelOverride,
+      modelOverride: (await resolveBrainstormJudgeModel(engine, opts.judgeModel)) ?? opts.modelOverride,
       chatFn: opts.chatFn,
       activeBiasTags: activeBiasTags ?? undefined,
       abortSignal: opts.abortSignal,

src/core/config.ts

@@ -904,6 +904,7 @@ export const KNOWN_CONFIG_KEYS: readonly string[] = [
   'models.subagent',
   'models.expansion',
   'models.chat',
+  'models.brainstorm.judge',
   'models.eval.longmemeval',
   'facts.extraction_model',
   // #2113: output-token cap for the per-turn facts extractor (default 4000).

src/core/cycle/propose-takes.ts

@@ -39,11 +39,11 @@
 
 import { randomUUID, createHash } from 'node:crypto';
 import { BaseCyclePhase, type ScopedReadOpts, type BasePhaseOpts } from './base-phase.ts';
-import { chat as gatewayChat, getChatModel } from '../ai/gateway.ts';
+import { chat as gatewayChat, getChatModel, probeChatModel } from '../ai/gateway.ts';
+import { normalizeModelId } from '../model-id.ts';
 import { writeReceipt } from '../extract/receipt-writer.ts';
 import { upsertExtractRollup } from '../extract/rollup-writer.ts';
 import { GBrainError } from '../types.ts';
-import type { Page, PageFilters } from '../types.ts';
 import type { OperationContext } from '../operations.ts';
 import type { BrainEngine } from '../engine.ts';
 import type { PhaseStatus, CyclePhase } from '../cycle.ts';
@@ -156,6 +156,48 @@ export interface ProposeTakesResult {
   warnings: string[];
 }
 
+/** Narrow projection of `pages` — the only columns this phase reads. */
+interface ProposeTakesPageRow {
+  slug: string;
+  source_id: string;
+  compiled_truth: string | null;
+}
+
+/**
+ * Load proposal candidates with a narrow projection instead of
+ * `engine.listPages` (`SELECT p.*`). The phase only reads slug, source_id
+ * and compiled_truth — skipping timeline/frontmatter/title keeps large
+ * toasted columns out of the hot path. Scope precedence mirrors
+ * `sourceScopeOpts`: federated array (`sourceIds`) beats scalar
+ * (`sourceId`); ordering matches `PAGE_SORT_SQL.updated_desc` with an id
+ * tiebreak for determinism. (Takeover of PR #1979's projection by
+ * @shawnduggan.)
+ */
+async function listCandidatePages(
+  engine: BrainEngine,
+  scope: ScopedReadOpts,
+  limit: number,
+): Promise<ProposeTakesPageRow[]> {
+  const where = ['deleted_at IS NULL'];
+  const params: unknown[] = [];
+  if (scope.sourceIds && scope.sourceIds.length > 0) {
+    params.push(scope.sourceIds);
+    where.push(`source_id = ANY($${params.length}::text[])`);
+  } else if (scope.sourceId) {
+    params.push(scope.sourceId);
+    where.push(`source_id = $${params.length}`);
+  }
+  params.push(limit);
+  return engine.executeRaw<ProposeTakesPageRow>(
+    `SELECT slug, source_id, compiled_truth
+       FROM pages
+      WHERE ${where.join(' AND ')}
+      ORDER BY updated_at DESC, id DESC
+      LIMIT $${params.length}`,
+    params,
+  );
+}
+
 /**
  * Compute the content_hash key for the idempotency cache. SHA-256 of the
  * page body suffices — page slug + prompt_version are separate columns in
@@ -309,6 +351,34 @@ class ProposeTakesPhase extends BaseCyclePhase {
     const skipPagesWithFence = opts.skipPagesWithFence ?? false;
     const proposalRunId = `propose-${new Date().toISOString().slice(0, 19).replace(/[-:T]/g, '')}-${randomUUID().slice(0, 8)}`;
 
+    const modelId = opts.model ?? getChatModel();
+
+    // With the default (gateway) extractor, skip cheaply when the resolved
+    // model's provider can't run — same probe semantics as patterns.ts /
+    // think/index.ts: unknown provider/model or Anthropic-without-key skips;
+    // other providers' auth surfaces lazily at chat() time. An injected
+    // extractor bypasses the gateway, so it is never gated. (Takeover of
+    // PR #1979's intent by @shawnduggan.)
+    if (!opts.extractor) {
+      const probe = probeChatModel(normalizeModelId(modelId));
+      if (!probe.ok) {
+        return {
+          summary: `propose_takes skipped: ${probe.detail}`,
+          details: {
+            reason: 'no_provider',
+            model: modelId,
+            pages_scanned: 0,
+            cache_hits: 0,
+            cache_misses: 0,
+            proposals_inserted: 0,
+            budget_exhausted: false,
+            warnings: [],
+          },
+          status: 'skipped',
+        };
+      }
+    }
+
     const result: ProposeTakesResult = {
       pages_scanned: 0,
       cache_hits: 0,
@@ -319,19 +389,12 @@ class ProposeTakesPhase extends BaseCyclePhase {
     };
 
     // Load pages eligible for proposal. Source-scoped per BaseCyclePhase.
-    const pageFilters: PageFilters = {
-      ...scope,
-      limit: pageLimit,
-      sort: 'updated_desc',
-    };
-    const pages: Page[] = await engine.listPages(pageFilters);
+    const pages = await listCandidatePages(engine, scope, pageLimit);
 
     if (opts.reporter) {
       opts.reporter.start('propose_takes.pages' as never, pages.length);
     }
 
-    const modelId = opts.model ?? getChatModel();
-
     for (const page of pages) {
       result.pages_scanned += 1;
       this.tick(opts);
@@ -473,4 +536,5 @@ export const __testing = {
   contentHash,
   hasCompleteFence,
   extractExistingTakesForDedup,
+  listCandidatePages,
 };

test/brainstorm/model-config.test.ts

@@ -0,0 +1,65 @@
+/**
+ * Brainstorm model configurability (takeover of PR #1855 by @starm2010).
+ *
+ * - The cost preview + hard cost ceiling price the model that will actually
+ *   run: --model override → configured chat_model → gateway fallback. Before
+ *   this, the preview always priced anthropic:claude-sonnet-4-6 even when
+ *   the configured chat_model was something else.
+ * - The judge phase honors the `models.brainstorm.judge` config key when no
+ *   --judge-model flag is passed.
+ */
+
+import { describe, test, expect } from 'bun:test';
+import {
+  resolveBrainstormChatModel,
+  resolveBrainstormJudgeModel,
+} from '../../src/core/brainstorm/orchestrator.ts';
+import type { BrainEngine } from '../../src/core/engine.ts';
+
+function mockEngine(configValues: Record<string, string>): { engine: BrainEngine; reads: string[] } {
+  const reads: string[] = [];
+  const engine = {
+    async getConfig(key: string): Promise<string | null> {
+      reads.push(key);
+      return configValues[key] ?? null;
+    },
+  } as unknown as BrainEngine;
+  return { engine, reads };
+}
+
+describe('resolveBrainstormChatModel', () => {
+  test('--model override wins over config', () => {
+    expect(resolveBrainstormChatModel({ chat_model: 'openai:gpt-5' }, 'anthropic:claude-opus-4-6'))
+      .toBe('anthropic:claude-opus-4-6');
+  });
+
+  test('configured chat_model wins over the hardcoded fallback', () => {
+    expect(resolveBrainstormChatModel({ chat_model: 'openai:gpt-5' }))
+      .toBe('openai:gpt-5');
+  });
+
+  test('falls back to the gateway default model when nothing is configured', () => {
+    expect(resolveBrainstormChatModel({})).toBe('anthropic:claude-sonnet-4-6');
+  });
+});
+
+describe('resolveBrainstormJudgeModel', () => {
+  test('--judge-model flag wins without touching config', async () => {
+    const { engine, reads } = mockEngine({ 'models.brainstorm.judge': 'openai:gpt-5' });
+    const out = await resolveBrainstormJudgeModel(engine, 'anthropic:claude-opus-4-6');
+    expect(out).toBe('anthropic:claude-opus-4-6');
+    expect(reads).toHaveLength(0);
+  });
+
+  test('models.brainstorm.judge config key is honored when no flag is passed', async () => {
+    const { engine, reads } = mockEngine({ 'models.brainstorm.judge': 'openai:gpt-5' });
+    const out = await resolveBrainstormJudgeModel(engine);
+    expect(out).toBe('openai:gpt-5');
+    expect(reads).toEqual(['models.brainstorm.judge']);
+  });
+
+  test('returns undefined (defer to modelOverride / gateway default) when unset', async () => {
+    const { engine } = mockEngine({});
+    expect(await resolveBrainstormJudgeModel(engine)).toBeUndefined();
+  });
+});

test/propose-takes.test.ts

@@ -15,6 +15,7 @@
  */
 
 import { describe, test, expect } from 'bun:test';
+import { withEnv, emptyHome } from './helpers/with-env.ts';
 import {
   runPhaseProposeTakes,
   parseExtractorOutput,
@@ -52,6 +53,14 @@ function buildMockEngine(opts: {
     },
     async executeRaw<T>(sql: string, params?: unknown[]): Promise<T[]> {
       captured.push({ sql, params: params ?? [] });
+      // Narrow candidate-page projection (replaces listPages in the phase).
+      if (sql.includes('SELECT slug, source_id, compiled_truth')) {
+        return opts.pages.map((p) => ({
+          slug: p.slug,
+          source_id: p.source_id,
+          compiled_truth: p.compiled_truth,
+        })) as T[];
+      }
       // SELECT idempotency check
       if (sql.includes('SELECT id FROM take_proposals')) {
         const [sourceId, slug, ch, pv] = params ?? [];
@@ -435,4 +444,72 @@ New prose appended here.`;
       resetGateway();
     }
   });
+
+  test('default extractor skips cleanly when the Anthropic chat model has no key', async () => {
+    // Empty GBRAIN_HOME so hasAnthropicKey's config-file fallback can't find
+    // the operator's real key.
+    await withEnv({ GBRAIN_HOME: emptyHome(), ANTHROPIC_API_KEY: undefined }, async () => {
+      configureGateway({ chat_model: 'anthropic:claude-sonnet-4-6', env: {} });
+      try {
+        const { engine, captured } = buildMockEngine({
+          pages: [buildPage({ slug: 'wiki/a', body: 'claim-ish prose' })],
+        });
+        const result = await runPhaseProposeTakes(buildCtx(engine));
+
+        expect(result.status).toBe('skipped');
+        expect((result.details as Record<string, unknown>).reason).toBe('no_provider');
+        // Skips BEFORE touching the engine — no page scan, no cache probes.
+        expect(captured).toHaveLength(0);
+      } finally {
+        resetGateway();
+      }
+    });
+  });
+
+  test('an injected extractor is never gated on provider availability', async () => {
+    await withEnv({ GBRAIN_HOME: emptyHome(), ANTHROPIC_API_KEY: undefined }, async () => {
+      configureGateway({ chat_model: 'anthropic:claude-sonnet-4-6', env: {} });
+      try {
+        const { engine } = buildMockEngine({
+          pages: [buildPage({ slug: 'wiki/b', body: 'still processed' })],
+        });
+        const extractor: ProposeTakesExtractor = async () => [];
+        const result = await runPhaseProposeTakes(buildCtx(engine), { extractor });
+
+        expect(result.status).toBe('ok');
+        expect((result.details as Record<string, unknown>).pages_scanned).toBe(1);
+      } finally {
+        resetGateway();
+      }
+    });
+  });
+
+  test('loads proposal candidates with a narrow page projection', async () => {
+    const pages = [buildPage({ slug: 'wiki/narrow', body: 'A narrow projection avoids unrelated page columns.' })];
+    const { engine, captured } = buildMockEngine({ pages });
+    const extractor: ProposeTakesExtractor = async () => [];
+    await runPhaseProposeTakes(buildCtx(engine), { extractor });
+
+    const pageSelect = captured.find(c => c.sql.includes('FROM pages'));
+    expect(pageSelect).toBeDefined();
+    expect(pageSelect!.sql).toContain('SELECT slug, source_id, compiled_truth');
+    expect(pageSelect!.sql).not.toContain('*');
+    // Scalar sourceId scope from ctx binds as a plain equality param.
+    expect(pageSelect!.params[0]).toBe('default');
+  });
+
+  test('narrow projection: federated sourceIds beat scalar sourceId', async () => {
+    const { engine, captured } = buildMockEngine({ pages: [] });
+    const extractor: ProposeTakesExtractor = async () => [];
+    const ctx = {
+      ...buildCtx(engine),
+      auth: { allowedSources: ['team-a', 'team-b'] },
+    } as OperationContext;
+    await runPhaseProposeTakes(ctx, { extractor });
+
+    const pageSelect = captured.find(c => c.sql.includes('FROM pages'));
+    expect(pageSelect).toBeDefined();
+    expect(pageSelect!.sql).toContain('source_id = ANY(');
+    expect(pageSelect!.params[0]).toEqual(['team-a', 'team-b']);
+  });
 });