feat(review-tutor): add harness connector boundary

packages/review-tutor/README.md

@@ -1,6 +1,6 @@
 # @pickforge/review-tutor
 
-Review Tutor is a local-first, browser-based companion for guided reviews of PRs, diffs, and code. Questions run in an isolated tutor child, never in the main Pi conversation. GitHub remains the source of truth. Review Tutor never approves, comments on, or edits a PR. Answers, notes, and quiz results stay local.
+Review Tutor is a local-first, browser-based companion for guided reviews of PRs, diffs, and code. Questions run in an isolated tutor child, never in the main Pi conversation. GitHub remains the source of truth. Review Tutor never approves, comments on, or edits a PR. Answers, notes, and quiz results stay local. Harness connectors are tracked in [#63](https://github.com/pickforge/pickforge-platform/issues/63).
 
 ## Install
 
@@ -44,6 +44,10 @@ With no argument, choose a source in the browser. The browser supports worktree,
 
 The model dialog lists the session's scoped models when `--models` or the settings scope configures them. Otherwise, it lists all available models. Thinking levels are offered only for reasoning models. A scope entry with an explicit level, such as `gpt-5.6-sol:high`, pins the tutor to that level.
 
+## 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 the only registered connector today; Claude Code and Codex support is tracked in [#63](https://github.com/pickforge/pickforge-platform/issues/63). The `reviewTutorHarnessConnectors` 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.
+
 ## Local data
 
 By default, data is stored under:

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

@@ -5,6 +5,8 @@ import type {
   ExtensionAPI,
   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 {
@@ -252,16 +254,18 @@ export default function reviewTutorExtension(pi: ExtensionAPI): void {
         const server = await lifecycle.start(async (startupSignal) => {
           const cwd = await realpath(ctx.cwd);
           const canonicalRepo = await repositoryRoot(pi, cwd);
-          const models = modelChoices(ctx);
-          if (!models.length) {
+          const piModels = modelChoices(ctx);
+          if (!piModels.length) {
             throw new Error(
               "model snapshot failed: expected at least one available model; configure a Pi model and retry",
             );
           }
+          const flags = createReviewTutorFlags();
+          const registry = createConnectorRegistry({ flags, piModels });
           return startReviewTutorServer({
             cwd,
             canonicalRepo,
-            models,
+            registry,
             skillPath,
             initialSource: sourceFromArgument(args),
             startupSignal,

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

@@ -0,0 +1,102 @@
+import type {
+  ConnectorRequest,
+  Discovery,
+  DiscoveryDeps,
+  HarnessConnector,
+  ParseSink,
+  ParsedAnswer,
+  SpawnSpec,
+} from "./types.ts";
+import { ConnectorError } from "./types.ts";
+
+interface PiContent {
+  type?: unknown;
+  text?: unknown;
+}
+
+interface PiMessage {
+  role?: unknown;
+  content?: unknown;
+  usage?: unknown;
+}
+
+interface PiEvent {
+  type?: unknown;
+  message?: PiMessage;
+  messages?: PiMessage[];
+  assistantMessageEvent?: {
+    type?: unknown;
+    delta?: unknown;
+  };
+}
+
+function parseEvent(line: string): PiEvent {
+  try {
+    return JSON.parse(line) as PiEvent;
+  } catch {
+    throw new ConnectorError(
+      "Pi JSON parsing failed: expected one valid JSON object per LF-delimited line; inspect child output and retry",
+    );
+  }
+}
+
+function finalAnswer(messages: PiMessage[] | undefined): string | undefined {
+  if (!Array.isArray(messages)) return undefined;
+  const final = messages.filter((message) => message?.role === "assistant").at(-1);
+  if (!Array.isArray(final?.content)) {
+    return typeof final?.content === "string" ? final.content : "";
+  }
+  return (final.content as PiContent[])
+    .filter((content) => content?.type === "text")
+    .map((content) => typeof content.text === "string" ? content.text : "")
+    .join("");
+}
+
+export class PiConnector implements HarnessConnector {
+  readonly id = "pi" as const;
+  readonly label = "Pi";
+
+  async discover(deps: DiscoveryDeps): Promise<Discovery> {
+    return {
+      available: true,
+      version: deps.piVersion ?? "unknown",
+      models: deps.piModels.map((model) => ({ ...model, id: `pi:${model.id}` })),
+    };
+  }
+
+  spawnSpec(request: ConnectorRequest): SpawnSpec {
+    const [provider, ...modelParts] = request.model.split("/");
+    return {
+      command: "pi",
+      args: [
+        "--mode", "json", "--no-extensions", "--no-skills", "--no-prompt-templates",
+        "--no-context-files", "--no-session", "-p", "--tools", "read,grep,find,ls",
+        "--provider", provider!, "--model", modelParts.join("/"), "--thinking", request.thinking,
+      ],
+    };
+  }
+
+  parseLine(line: string, sink: ParseSink): void {
+    const event = parseEvent(line);
+    const update = event.type === "message_update" ? event.assistantMessageEvent : undefined;
+    if (update?.type === "text_delta" && typeof update.delta === "string") {
+      sink.delta(update.delta);
+    }
+    if (event.type === "message_end" && event.message?.usage && typeof event.message.usage === "object") {
+      sink.usage(event.message.usage as Record<string, number>);
+    }
+    if (event.type === "agent_end" && Array.isArray(event.messages)) {
+      const candidate = finalAnswer(event.messages);
+      if (candidate !== undefined) sink.final(candidate);
+    }
+  }
+
+  finish(sink: ParseSink): ParsedAnswer {
+    if (!sink.answer?.trim()) {
+      throw new ConnectorError(
+        "Pi answer failed: expected a non-empty final assistant message in agent_end; choose another question or model and retry",
+      );
+    }
+    return { answer: sink.answer, ...(sink.answerUsage ? { usage: sink.answerUsage } : {}) };
+  }
+}

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

@@ -0,0 +1,12 @@
+const PATTERNS = [
+  /sk-[A-Za-z0-9_-]{8,}/g,
+  /Bearer\s+\S+/gi,
+  /(?:ANTHROPIC|OPENAI|OPENROUTER|XAI)_API_KEY=\S+/g,
+  /token=\S+/gi,
+  /ghp_\w+/g,
+  /gho_\w+/g,
+] as const;
+
+export function redact(value: string): string {
+  return PATTERNS.reduce((redacted, pattern) => redacted.replace(pattern, "[redacted]"), value);
+}

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

@@ -0,0 +1,62 @@
+import type { ReviewTutorFlags } from "../flags.ts";
+import { PiConnector } from "./pi.ts";
+import type {
+  Discovery,
+  DiscoveryDeps,
+  HarnessConnector,
+  HarnessId,
+  ModelChoice,
+} from "./types.ts";
+
+export interface ConnectorRegistry {
+  connectors(): HarnessConnector[];
+  byId(id: string): HarnessConnector | undefined;
+  resolve(modelId: string): { connector: HarnessConnector; model: string } | undefined;
+  discoveries(): Promise<Array<{ connector: HarnessConnector; discovery: Discovery }>>;
+}
+
+export function createConnectorRegistry(options: {
+  flags: ReviewTutorFlags;
+  piModels: ModelChoice[];
+  piVersion?: string;
+}): ConnectorRegistry {
+  const pi = new PiConnector();
+  const optionalConnectors: HarnessConnector[] = [];
+  const dependencies: DiscoveryDeps = {
+    piModels: options.piModels,
+    ...(options.piVersion ? { piVersion: options.piVersion } : {}),
+  };
+
+  const connectors = (): HarnessConnector[] => [
+    pi,
+    ...(options.flags.isEnabled("reviewTutorHarnessConnectors") ? optionalConnectors : []),
+  ];
+
+  return {
+    connectors,
+    byId(id) {
+      return connectors().find((connector) => connector.id === id);
+    },
+    resolve(modelId) {
+      const { harness, model } = splitModelId(modelId);
+      const connector = this.byId(harness as HarnessId);
+      return connector ? { connector, model } : undefined;
+    },
+    async discoveries() {
+      return Promise.all(connectors().map(async (connector) => ({
+        connector,
+        discovery: await connector.discover(dependencies),
+      })));
+    },
+  };
+}
+
+// A harness namespace is the text before the first ":" only when that ":" precedes any "/"; provider ids may contain ":".
+export function splitModelId(modelId: string): { harness: string; model: string } {
+  const separator = modelId.indexOf(":");
+  const slash = modelId.indexOf("/");
+  const namespaced = separator >= 0 && (slash < 0 || separator < slash);
+  return namespaced
+    ? { harness: modelId.slice(0, separator), model: modelId.slice(separator + 1) }
+    : { harness: "pi", model: modelId };
+}

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

@@ -0,0 +1,52 @@
+export type HarnessId = "pi" | "claude-code" | "codex";
+
+export interface HarnessConnector {
+  readonly id: HarnessId;
+  readonly label: string;
+  discover(deps: DiscoveryDeps): Promise<Discovery>;
+  spawnSpec(request: ConnectorRequest): SpawnSpec;
+  parseLine(line: string, sink: ParseSink): void;
+  finish(sink: ParseSink): ParsedAnswer;
+  readonly envKeys?: readonly string[];
+}
+
+export interface DiscoveryDeps {
+  piModels: ModelChoice[];
+  piVersion?: string;
+}
+
+export type Discovery =
+  | { available: true; version: string; models: ModelChoice[] }
+  | { available: false; reason: string };
+
+export interface ModelChoice {
+  id: string;
+  label: string;
+  thinkingLevels: string[];
+}
+
+export interface ConnectorRequest {
+  model: string;
+  thinking: string;
+  cwd: string;
+}
+
+export interface SpawnSpec {
+  command: string;
+  args: string[];
+}
+
+export interface ParseSink {
+  readonly answer?: string;
+  readonly answerUsage?: Record<string, number>;
+  delta(text: string): void;
+  usage(u: Record<string, number>): void;
+  final(answer: string): void;
+}
+
+export interface ParsedAnswer {
+  answer: string;
+  usage?: Record<string, number>;
+}
+
+export class ConnectorError extends Error {}

packages/review-tutor/src/export-html.ts

@@ -1,3 +1,4 @@
+import { splitModelId, type ConnectorRegistry } from "./connectors/registry.ts";
 import type { LearningEntry, QuizOutcome } from "./protocol.ts";
 
 const QUIZ_LABELS: Record<QuizOutcome, string> = {
@@ -16,7 +17,15 @@ function escapeHtml(value: unknown): string {
   })[character]!);
 }
 
-function card(entry: LearningEntry): string {
+function modelDetails(entry: LearningEntry, registry: ConnectorRegistry): { harness: string; model: string } {
+  if (typeof entry.modelId !== "string") return { harness: "Pi", model: String(entry.modelId) };
+  const resolved = registry.resolve(entry.modelId);
+  if (resolved) return { harness: resolved.connector.label, model: resolved.model };
+  const { harness, model } = splitModelId(entry.modelId);
+  return { harness: harness === "pi" ? "Pi" : harness, model };
+}
+
+function card(entry: LearningEntry, registry: ConnectorRegistry): string {
   const sourceUrl = entry.source.githubUrl
     ? `<dt>GitHub source URL</dt><dd>${escapeHtml(entry.source.githubUrl)}</dd>`
     : "";
@@ -33,6 +42,7 @@ function card(entry: LearningEntry): string {
     ? entry.preferences.comparisonLanguages.map(escapeHtml).join(", ")
     : "None";
   const outcome = entry.quizOutcome ? QUIZ_LABELS[entry.quizOutcome] : "Not recorded";
+  const details = modelDetails(entry, registry);
 
   return `<article>
 <h2>${escapeHtml(entry.source.label)}</h2>
@@ -41,7 +51,8 @@ function card(entry: LearningEntry): string {
 <dt>Source label</dt><dd>${escapeHtml(entry.source.label)}</dd>
 <dt>Source digest</dt><dd><code>${escapeHtml(entry.source.digest)}</code></dd>
 ${sourceUrl}${head}${file}${range}
-<dt>Model</dt><dd>${escapeHtml(entry.modelId)}</dd>
+<dt>Harness</dt><dd>${escapeHtml(details.harness)}</dd>
+<dt>Model</dt><dd>${escapeHtml(details.model)}</dd>
 <dt>Explanation language</dt><dd>${escapeHtml(entry.preferences.explanationLanguage)}</dd>
 <dt>Comparison languages</dt><dd>${comparisons}</dd>
 <dt>Created</dt><dd>${escapeHtml(entry.createdAt)}</dd>
@@ -57,8 +68,8 @@ ${entry.source.kind === "pr" ? "<p>GitHub is the source of truth.</p>" : ""}
 </article>`;
 }
 
-export function exportLearningHtml(entries: LearningEntry[]): string {
-  const cards = entries.length ? entries.map(card).join("") : "<p>No learning entries yet.</p>";
+export function exportLearningHtml(entries: LearningEntry[], registry: ConnectorRegistry): string {
+  const cards = entries.length ? entries.map((entry) => card(entry, registry)).join("") : "<p>No learning entries yet.</p>";
   return `<!doctype html>
 <html lang="en">
 <head>

packages/review-tutor/src/flags.ts

@@ -0,0 +1,34 @@
+import {
+  createFlags,
+  type FlagOverrideStore,
+  type Flags,
+} from "../../flags/src/index.ts";
+
+const definitions = {
+  reviewTutorHarnessConnectors: {
+    description: "Show Claude Code and Codex harness connectors in Review Tutor",
+    default: false,
+  },
+} as const;
+
+export type ReviewTutorFlag = keyof typeof definitions;
+export type ReviewTutorFlags = Flags<ReviewTutorFlag>;
+
+function environmentStore(value = process.env.REVIEW_TUTOR_FLAGS): FlagOverrideStore {
+  const enabled = new Set((value ?? "").split(",").map((key) => key.trim()).filter(Boolean));
+  const overrides = new Map<string, boolean>();
+  for (const key of Object.keys(definitions)) {
+    if (enabled.has(key)) overrides.set(key, true);
+  }
+  return {
+    get: (key) => overrides.get(key),
+    set(key, next) {
+      if (next === undefined) overrides.delete(key);
+      else overrides.set(key, next);
+    },
+  };
+}
+
+export function createReviewTutorFlags(store?: FlagOverrideStore): ReviewTutorFlags {
+  return createFlags(definitions, { store: store ?? environmentStore() });
+}