feat(review-tutor): always register the Claude Code and Codex connectors

packages/review-tutor/README.md

@@ -46,7 +46,7 @@ The model dialog lists the session's scoped models when `--models` or the settin
 
 ## Harness connectors
 
-A harness connector owns model discovery, isolated invocation, and stream parsing while the shared runner owns process lifetime, bounds, and cancellation. Pi is registered by default; the Claude Code and Codex connectors are available behind `reviewTutorHarnessConnectors`. The flag defaults off. For local testing on main, set `REVIEW_TUTOR_FLAGS=reviewTutorHarnessConnectors` before starting Pi. Child processes receive only the shared environment allowlist plus keys explicitly declared by their connector, and runner failures redact common API keys, bearer credentials, and tokens before leaving the process boundary.
+A harness connector owns model discovery, isolated invocation, and stream parsing while the shared runner owns process lifetime, bounds, and cancellation. Pi, Claude Code, and Codex are registered; a connector appears in the Harness picker only when its executable is found and supported at startup, and the helper line explains any that are not. Child processes receive only the shared environment allowlist plus keys explicitly declared by their connector, and runner failures redact common API keys, bearer credentials, and tokens before leaving the process boundary.
 
 The Claude Code connector forwards `CLAUDE_CONFIG_DIR` when present, but never forwards `ANTHROPIC_API_KEY`; users who rely on that environment key must sign in through Claude Code instead.
 

packages/review-tutor/extensions/review-tutor.ts

@@ -6,7 +6,6 @@ import type {
   ExtensionCommandContext,
 } from "@earendil-works/pi-coding-agent";
 import { createConnectorRegistry } from "../src/connectors/registry.ts";
-import { createReviewTutorFlags } from "../src/flags.ts";
 import type { ExecFile } from "../src/inputs.ts";
 import type { ModelChoice, SourceRequest } from "../src/protocol.ts";
 import {
@@ -260,8 +259,7 @@ export default function reviewTutorExtension(pi: ExtensionAPI): void {
               "model snapshot failed: expected at least one available model; configure a Pi model and retry",
             );
           }
-          const flags = createReviewTutorFlags();
-          const registry = createConnectorRegistry({ flags, piModels });
+          const registry = createConnectorRegistry({ piModels });
           return startReviewTutorServer({
             cwd,
             canonicalRepo,

packages/review-tutor/src/connectors/registry.ts

@@ -1,5 +1,4 @@
 import { execFile } from "node:child_process";
-import type { ReviewTutorFlags } from "../flags.ts";
 import { ClaudeCodeConnector } from "./claude-code.ts";
 import { CodexConnector } from "./codex.ts";
 import { PiConnector } from "./pi.ts";
@@ -28,25 +27,20 @@ function which(command: string): Promise<string | undefined> {
 }
 
 export function createConnectorRegistry(options: {
-  flags: ReviewTutorFlags;
   piModels: ModelChoice[];
   piVersion?: string;
   which?: DiscoveryDeps["which"];
   execFile?: DiscoveryExecFile;
 }): ConnectorRegistry {
-  const pi = new PiConnector();
-  const optionalConnectors: HarnessConnector[] = [new ClaudeCodeConnector(), new CodexConnector()];
+  const registered: HarnessConnector[] = [new PiConnector(), new ClaudeCodeConnector(), new CodexConnector()];
   const dependencies: DiscoveryDeps = {
     piModels: options.piModels,
     ...(options.piVersion ? { piVersion: options.piVersion } : {}),
     which: options.which ?? which,
     ...(options.execFile ? { execFile: options.execFile } : {}),
   };
 
-  const connectors = (): HarnessConnector[] => [
-    pi,
-    ...(options.flags.isEnabled("reviewTutorHarnessConnectors") ? optionalConnectors : []),
-  ];
+  const connectors = (): HarnessConnector[] => registered;
 
   return {
     connectors,

packages/review-tutor/test/connector-codex.test.ts

@@ -6,7 +6,6 @@ import { describe, expect, it, vi } from "vitest";
 import { CodexConnector, discoveryEnvironment } from "../src/connectors/codex.ts";
 import { createConnectorRegistry } from "../src/connectors/registry.ts";
 import type { DiscoveryDeps, ParseSink } from "../src/connectors/types.ts";
-import { createReviewTutorFlags } from "../src/flags.ts";
 import { TutorRunner } from "../src/runner.ts";
 
 class FakeChild extends EventEmitter {
@@ -406,19 +405,14 @@ describe("Codex runner integration", () => {
 });
 
 describe("Codex registry", () => {
-  it("registers Codex only with the flag and preserves unavailable discovery", async () => {
-    const off = createConnectorRegistry({
-      flags: createReviewTutorFlags({ get: () => false, set: () => {} }),
-      piModels,
-    });
+  it("registers Codex and preserves unavailable discovery", async () => {
     const on = createConnectorRegistry({
-      flags: createReviewTutorFlags({ get: () => true, set: () => {} }),
       piModels,
+      which: async () => undefined,
       execFile: fakeDiscovery({
         "--version": Object.assign(new Error("spawn codex ENOENT"), { code: "ENOENT" }),
       }).execFile,
     });
-    expect(off.connectors().map(({ id }) => id)).toEqual(["pi"]);
     expect(on.connectors().map(({ id }) => id)).toEqual(["pi", "claude-code", "codex"]);
     const discoveries = await on.discoveries();
     expect(discoveries.find(({ connector }) => connector.id === "codex")).toEqual({

packages/review-tutor/test/connectors.test.ts

@@ -9,7 +9,6 @@ import {
   type HarnessConnector,
   type ParseSink,
 } from "../src/connectors/types.ts";
-import { createReviewTutorFlags } from "../src/flags.ts";
 import { TutorRunner } from "../src/runner.ts";
 
 class FakeChild extends EventEmitter {
@@ -22,15 +21,8 @@ class FakeChild extends EventEmitter {
 
 const models = [{ id: "anthropic/model", label: "Model", thinkingLevels: ["low"] }];
 
-function flags(enabled: boolean) {
-  return createReviewTutorFlags({
-    get: () => enabled,
-    set: () => {},
-  });
-}
-
-function registry(enabled = false) {
-  return createConnectorRegistry({ flags: flags(enabled), piModels: models });
+function registry() {
+  return createConnectorRegistry({ piModels: models });
 }
 
 const request = {
@@ -42,22 +34,8 @@ const request = {
 };
 
 describe("connector registry", () => {
-  it("reads the environment override once when flags are created", () => {
-    const previous = process.env.REVIEW_TUTOR_FLAGS;
-    try {
-      process.env.REVIEW_TUTOR_FLAGS = "reviewTutorHarnessConnectors";
-      const snapshot = createReviewTutorFlags();
-      process.env.REVIEW_TUTOR_FLAGS = "";
-      expect(snapshot.isEnabled("reviewTutorHarnessConnectors")).toBe(true);
-    } finally {
-      if (previous === undefined) delete process.env.REVIEW_TUTOR_FLAGS;
-      else process.env.REVIEW_TUTOR_FLAGS = previous;
-    }
-  });
-
-  it("registers optional connectors only when the flag is on", () => {
-    expect(registry(false).connectors().map((connector) => connector.id)).toEqual(["pi"]);
-    expect(registry(true).connectors().map((connector) => connector.id)).toEqual(["pi", "claude-code", "codex"]);
+  it("registers Pi, Claude Code, and Codex in that order", () => {
+    expect(registry().connectors().map((connector) => connector.id)).toEqual(["pi", "claude-code", "codex"]);
   });
 
   it("resolves namespaced and legacy Pi ids and rejects unknown harnesses", () => {
@@ -71,8 +49,7 @@ describe("connector registry", () => {
       connector: { id: "pi" },
       model: "ollama/qwen3:8b",
     });
-    expect(registry().resolve("codex:x")).toBeUndefined();
-    expect(registry(true).resolve("codex:x")).toMatchObject({ model: "x" });
+    expect(registry().resolve("codex:x")).toMatchObject({ model: "x" });
     expect(registry().resolve("unknown:x")).toBeUndefined();
   });
 

packages/review-tutor/test/server-extension.test.ts

@@ -11,7 +11,6 @@ import {
 } from "../extensions/review-tutor.ts";
 import { createConnectorRegistry } from "../src/connectors/registry.ts";
 import type { HarnessConnector } from "../src/connectors/types.ts";
-import { createReviewTutorFlags } from "../src/flags.ts";
 import { pageHtml } from "../src/page.ts";
 import { resolveStatePaths } from "../src/paths.ts";
 import type { AskRequest } from "../src/protocol.ts";
@@ -21,12 +20,13 @@ const skillPath = fileURLToPath(
   new URL("../skills/review-tutor/SKILL.md", import.meta.url),
 );
 const temporaryRoots: string[] = [];
+const absentExecFile = async () => { throw Object.assign(new Error("spawn ENOENT"), { code: "ENOENT" }); };
 
 function registry(models = [
   { id: "provider/model", label: "Model", thinkingLevels: ["low"] },
   { id: "ollama/qwen3:8b", label: "Qwen", thinkingLevels: ["low"] },
 ]) {
-  return createConnectorRegistry({ flags: createReviewTutorFlags(), piModels: models });
+  return createConnectorRegistry({ piModels: models, which: async () => undefined, execFile: absentExecFile });
 }
 
 interface DeferredResult {
@@ -322,7 +322,6 @@ describe("local server security", () => {
 describe("connector protocol boundary", () => {
   it("reports an unavailable Claude Code harness without offering its models when flagged on", async () => {
     const connectorRegistry = createConnectorRegistry({
-      flags: createReviewTutorFlags({ get: () => true, set: () => {} }),
       piModels: [{ id: "provider/model", label: "Model", thinkingLevels: ["low"] }],
       which: async () => undefined,
       execFile: async () => { throw Object.assign(new Error("spawn codex ENOENT"), { code: "ENOENT" }); },
@@ -359,7 +358,11 @@ describe("connector protocol boundary", () => {
         "pi:provider/model",
         "pi:ollama/qwen3:8b",
       ]);
-      expect(state.harnesses).toEqual([{ id: "pi", label: "Pi", available: true, models: state.models }]);
+      expect(state.harnesses).toEqual([
+        { id: "pi", label: "Pi", available: true, models: state.models },
+        { id: "claude-code", label: "Claude Code", available: false, reason: "Claude Code is not installed (claude not found on PATH).", models: [] },
+        { id: "codex", label: "Codex", available: false, reason: "Codex is not installed (codex not found on PATH).", models: [] },
+      ]);
 
       const source = await loadSource(server.port, server.token);
       expect((await ask(server.port, server.token, source.id)).status).toBe(202);
@@ -387,12 +390,20 @@ describe("connector protocol boundary", () => {
 
       const unknown = await call(server.port, server.token, "/api/ask", {
         method: "POST",
-        body: JSON.stringify({ ...askBody(source.id), modelId: "codex:x" }),
+        body: JSON.stringify({ ...askBody(source.id), modelId: "unknown:x" }),
       });
       expect(unknown.status).toBe(400);
       await expect(unknown.json()).resolves.toEqual({
         error: "model selection failed: unknown harness; refresh state and retry",
       });
+      const foreign = await call(server.port, server.token, "/api/ask", {
+        method: "POST",
+        body: JSON.stringify({ ...askBody(source.id), modelId: "codex:x" }),
+      });
+      expect(foreign.status).toBe(400);
+      await expect(foreign.json()).resolves.toEqual({
+        error: "model selection failed: expected an available model and thinking level; refresh state and retry",
+      });
 
       await waitFor(async () => ((await (await call(server.port, server.token, "/api/log")).json()) as unknown[]).length === 2);
       const exported = await (await call(server.port, server.token, "/api/export")).text();

packages/review-tutor/test/storage-export-sse.test.ts

@@ -5,7 +5,6 @@ import { join } from "node:path";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { createConnectorRegistry } from "../src/connectors/registry.ts";
 import { exportLearningHtml } from "../src/export-html.ts";
-import { createReviewTutorFlags } from "../src/flags.ts";
 import {
   appendEntry,
   foldLog,
@@ -18,7 +17,6 @@ import { SseHub } from "../src/sse.ts";
 
 const dirs: string[] = [];
 const exportRegistry = createConnectorRegistry({
-  flags: createReviewTutorFlags(),
   piModels: [],
 });
 afterEach(async () => {
@@ -132,7 +130,7 @@ describe("standalone export", () => {
       { ...entry(), id: "colon-model", modelId: "ollama/qwen3:8b" },
       { ...entry(), id: "namespaced-colon-model", modelId: "pi:ollama/qwen3:8b" },
       { ...entry(), id: "invalid-model", modelId: 42 } as unknown as LearningEntry,
-      { ...entry(), id: "disabled-harness", modelId: "codex:gpt-5.6-sol" },
+      { ...entry(), id: "unknown-harness", modelId: "future:gpt-9" },
     ], exportRegistry);
     expect(html).toContain("Private code warning");
     expect(html).toContain("GitHub is the source of truth");
@@ -145,8 +143,8 @@ describe("standalone export", () => {
     expect(html.match(/<dt>Harness<\/dt><dd>Pi<\/dd>/g)).toHaveLength(6);
     expect(html.match(/<dt>Model<\/dt><dd>ollama\/qwen3:8b<\/dd>/g)).toHaveLength(2);
     expect(html).toContain("<dt>Model</dt><dd>42</dd>");
-    expect(html).toContain("<dt>Harness</dt><dd>codex</dd>");
-    expect(html).toContain("<dt>Model</dt><dd>gpt-5.6-sol</dd>");
+    expect(html).toContain("<dt>Harness</dt><dd>future</dd>");
+    expect(html).toContain("<dt>Model</dt><dd>gpt-9</dd>");
     expect(html).toContain("&lt;script&gt;x&lt;/script&gt;");
     expect(html).toContain("&amp; hostile &lt;img src=x&gt;");
     expect(html).not.toContain("<script");

packages/review-tutor/test/structure.test.ts

@@ -4,7 +4,6 @@ import { join } from "node:path";
 import { fileURLToPath } from "node:url";
 import { afterEach, describe, expect, it } from "vitest";
 import { createConnectorRegistry } from "../src/connectors/registry.ts";
-import { createReviewTutorFlags } from "../src/flags.ts";
 import type { ExecFile } from "../src/inputs.ts";
 import type { InputSnapshot, StructureEdge, StructureSnapshot } from "../src/protocol.ts";
 import { startReviewTutorServer } from "../src/server.ts";
@@ -15,7 +14,8 @@ import {
 const skillPath = fileURLToPath(new URL("../skills/review-tutor/SKILL.md", import.meta.url));
 const temporaryRoots: string[] = [];
 const registry = () => createConnectorRegistry({
-  flags: createReviewTutorFlags(),
+  which: async () => undefined,
+  execFile: async () => { throw Object.assign(new Error("spawn ENOENT"), { code: "ENOENT" }); },
   piModels: [{ id: "provider/model", label: "Model", thinkingLevels: ["low"] }],
 });