Add isolated Claude Code and Codex connectors to Review Tutor

[ElbertePlinio]

Goal

Add honest, isolated Claude Code and Codex harness connectors to Review Tutor after #59 / PR #61 is human-approved.

Review Tutor currently has one real connector: Pi. The browser must continue to show only harnesses that are actually installed, available, and supported. No disabled placeholders or UI-only options.

Status

Deferred / planned. Do not start implementation until PR #61 is finished and human-approved.

This is multi-PR work and must remain dark on main until the complete flow is ready. Proposed feature flag: reviewTutorHarnessConnectors (default off). Label this issue flagged; flag removal is part of completion after the enabled release.

Product contract

• Keep harness and model as separate concepts.
• Preserve the local-first, loopback-only Review Tutor security model.
• Run every question in a new isolated child process; never reuse or attach to the parent coding-agent session.
• Keep tutor tools read-only and bounded. The browser receives no provider credentials.
• Show Claude Code or Codex only after connector discovery succeeds; explain unavailable connectors outside the primary flow rather than presenting fake choices.
• Namespace model identities by harness so equal provider model names cannot collide.
• Keep Pi behavior unchanged while the flag is off.

Required design work

Before implementation, freeze a connector boundary that owns:

1. executable and capability discovery;
2. harness-specific model discovery and thinking/reasoning options;
3. isolated invocation and prompt transport;
4. normalized progress, answer, error, and terminal events;
5. cancellation and process-tree cleanup;
6. bounded stdout/stderr parsing and actionable failure messages;
7. credential redaction and an explicit child-environment policy.

Do not add a harness field opportunistically to the existing rt/1 request. Define and test the protocol migration or compatibility strategy first.

Security and process requirements

• Preserve loopback binding, random bearer token, exact Origin/Host checks, CSP, and no credential exposure to page state or logs.
• Preserve detached POSIX process groups, bounded output, SIGTERM → SIGKILL escalation, server-shutdown cleanup, and one terminal outcome.
• Add equivalent proven Windows process-tree cancellation before claiming Windows support.
• Use a minimal documented environment policy; never print tokens, API keys, auth files, or provider command lines containing secrets.
• Restrict child tools to the Review Tutor read-only allowlist (read,grep,find,ls) or the closest enforceable provider-native equivalent. If a provider cannot enforce that boundary, mark the connector unavailable rather than weakening it silently.
• Use immutable input snapshots; do not grant source-editing authority.

Delivery slices

PR 1 — Connector boundary and protocol decision

• Add a harness connector registry with Pi implemented through the boundary and no behavior change.
• Freeze the request/state/event compatibility strategy and namespaced model identity.
• Add fake-executable contract tests for discovery, lifecycle events, malformed output, timeouts, cancellation races, process cleanup, and redaction.
• Add the default-off reviewTutorHarnessConnectors flag.

PR 2 — Claude Code connector

• Detect the real executable and supported non-interactive interface.
• Discover only models/options the local installation can honestly run.
• Enforce isolated, read-only execution and normalized cancellation.
• Cover absent binary, auth failure, unsupported version, empty answer, malformed output, timeout, cancellation, and successful answer.
• Complete one real local provider smoke without recording credentials.

PR 3 — Codex connector

• Apply the same discovery, isolation, normalization, cancellation, redaction, and test contract.
• Cover provider-specific auth/version/output failures without leaking provider internals to the browser.
• Complete one real local provider smoke without recording credentials.

PR 4 — Enabled UI and cross-harness acceptance

• Populate Harness only from available connectors.
• Refresh Model and reasoning choices from the selected connector without stale cross-harness state.
• Keep Pi first/default when it is the active parent harness and available.
• Preserve draft/selection state when switching harnesses; a running question retains its original harness/model identity.
• Prove cancel, answer, quiz, Learning Log, export, reload, and error recovery for each connector.
• Verify desktop, mobile, high zoom, keyboard, focus, and screen-reader behavior.
• Enable the flag only in the release that ships the completed feature.
• Remove the flag in the next follow-up PR.

Acceptance criteria

• Pi remains unchanged with the flag off.
• No harness appears unless its connector passes discovery.
• Browser state, JSONL, exported HTML, errors, and command output contain no credentials.
• Each question runs outside the parent agent session and cannot edit source.
• Cancellation terminates the full child process tree and reconciles terminal races.
• Harness/model attribution survives persistence and export.
• CI uses deterministic fake connectors; real-provider tests are explicit local smoke checks, not credential-dependent CI.
• Linux behavior is proven; macOS/Windows support is claimed only with platform-specific process and discovery proof.
• Build Review Tutor learning companion #59 and the Review Tutor docs link this issue once implementation begins.

[ElbertePlinio]

Agent plan

Goal: isolated Claude Code and Codex connectors behind reviewTutorHarnessConnectors (default off), Pi unchanged.

Scouting done 2026-08-25:

• Local CLIs: Claude Code 2.1.245 (claude -p --output-format stream-json; --bare drops auth, so isolation = --strict-mcp-config + --tools Read,Grep,Glob + --setting-sources), Codex 0.147.0 (codex exec --json -s read-only --ephemeral; honest model catalog via codex debug models).
• Current seam is RunnerLike (Pi-shaped); rt/1 ids are provider/model; entries persist modelId only; @pickforge/flags exists with no consumer yet.

Decisions frozen for PR 1 (contract in the PR body):

• Boundary: connector = discovery + spawn spec + line parser; the existing runner state machine stays shared (env allowlist, process group, bounds, SIGTERM→SIGKILL, single settle).
• Model identity namespaced by harness: pi:anthropic/…; legacy persisted ids read as Pi. No harness field on AskRequest; rt/1 stays, additive harnesses in state.
• Redaction in one place (runner) before any message leaves.

Checklist:

• PR 1 boundary + namespace + flag (in progress)
• PR 2 Claude Code connector
• PR 3 Codex connector
• PR 4 enabled UI + cross-harness acceptance (design-director contract)
• Human test on main with REVIEW_TUTOR_FLAGS=reviewTutorHarnessConnectors
• enable flag in a release → remove flag

Current status: In progress
Next action: PR 1 opened for review

[ElbertePlinio]

Progress update:

• PR 1 connector boundary + namespace + flag implemented
• Focused tests and repository check passed
• PR opened
• PR review

PR 1: #76
Blocked: independent local-review lanes were unavailable in this harness; full in-session review found no actionable issue
Next: review PR 1

[ElbertePlinio]

Progress update:

• PR 2 Claude Code connector implemented behind reviewTutorHarnessConnectors
• Typecheck, focused tests, repository check, real smoke, and serious-backend local review passed
• PR opened
• PR review

PR 2: #77
Depends on: PR #76
Blocked: none
Next: review PR 2

[ElbertePlinio]

Progress update:

• PR 3 Codex connector implemented behind reviewTutorHarnessConnectors
• Fake-only connector tests, full validation, serious-backend local review, and one permitted local smoke completed
• PR opened against feat/review-tutor-connector-boundary
• PR review

PR 3: #78
Smoke: exit 0, answer ok, no secret-like text detected in captured events
Blocked: none
Next: review PR 3

[ElbertePlinio]

Progress: PR 1 #76 review-clean at fb6a9cc (ready for Elberte). PR 2 #77 and PR 3 #78 opened, real smokes passed (Claude sonnet/low → ok; Codex gpt-5.6-sol/low → ok); review rounds in progress. Next: fix rounds on #77/#78, then PR 4 UI.

[ElbertePlinio]

Deferred P3s from the #77/#78 rounds are tracked in #79 (not a flag lifecycle item).