step.3.diff.txt -- narrative retro for step.3 (U1.2: first real adapter, claude-code) Executed: 2026-07-18, same day as the skeleton. Companion artifact: step.3.gdiff.txt (automated, scripts/step_gdiff). This is the first step in either repo touching a real, network-capable external provider -- the safety posture worked out at drafting (research BEFORE code, `$ANTHROPIC_API_KEY`-only auth, never rides an ambient session) held with zero deviation; the real findings this step surfaced were architectural, not safety-related. ## Plan vs actual ``` +---+----------------------------------+------------------------------------------+ | # | Planned (§) | Actual | +---+----------------------------------+------------------------------------------+ | 1 | §2.1: SUPPORTS_TOOL_CALLS class | DROPPED mid-build. Realized while wiring | | | flag alongside SUPPORTS_ | claude-code into the contract suite that | | | INTERLEAVED_ABORT | capabilities()["tools"] ALREADY exists | | | | in design.md §2.4's own contract shape | | | | -- adding a parallel class flag for the | | | | same fact would be a redundant, easy-to- | | | | drift-out-of-sync duplicate. Gated | | | | test_tool_calls_response AND (unplanned) | | | | test_multi_part_stream off the EXISTING | | | | ["tools"]/["streaming"] fields instead. | | | | SUPPORTS_INTERLEAVED_ABORT survives | | | | unchanged -- no existing contract field | | | | covers "can abort() interleave with a | | | | blocked run()", so it earns its own flag.| | 2 | (not planned at all -- found | test_unrecognized_request_fails_loud | | | live): the skeleton's §2.4 | (built at step.2, U1.1) turned out to be | | | assumed the U1.1 contract suite | MockAdapter-specific fixture-routing | | | was already fully adapter- | behavior, not a universal PAL contract | | | agnostic (step.2's own G2 dry- | property -- invisible with only ONE | | | run "proved" this) | adapter registered (step.2's dry-run | | | | used a SECOND MockAdapter instance, so | | | | it could never have surfaced this), | | | | obviously wrong once a REAL, differently | | | | -behaved adapter (claude-code) joined. | | | | Moved to tests/test_mock_adapter.py. | | | | Lesson folded into finding #2 below. | +---+----------------------------------+------------------------------------------+ ``` Every other §5 open question resolved exactly as LEANed: Q-3-A (plain role-tagged flattening), Q-3-B (no ambient-session fallback, not even tested), Q-3-C (honest skip, no thread-based abort harness built), Q-3-D (CLI subprocess, no SDK dependency), Q-3-E (documented-shape fixture, flagged for re-validation). ## Gate evidence ``` +------+---------------------------------------------------------------------+ | G1 | pytest -v -rs: claude-code's 7 parametrized tests ALL skip with | | | the exact reason "$ANTHROPIC_API_KEY is not set (required for | | | --bare headless auth; C-3-B: this adapter never falls back to an | | | ambient interactive session)" -- captured verbatim, this dev | | | environment's real (absent) key state, not simulated. | | G2 | grep -rn -e mock -e claude -e openai app/pal/ -> empty after this | | | step's edits to adapter.py + the new errors.py. | | G3 | Code review: zero `if name == "claude-code"` (or similar) string | | | checks anywhere in tests/test_pal_contract.py -- every per-adapter | | | behavior difference reads capabilities()["tools"/"streaming"] or | | | Adapter.SUPPORTS_INTERLEAVED_ABORT. | | G4 | ClaudeCodeAdapter().is_available() and .capabilities() called | | | directly, live, in this session -- confirmed to spawn NO | | | subprocess (shutil.which + a dict read only). The full test suite | | | spawns nothing either, by construction of the skip (run() is never | | | reached for the claude-code param). | | G5 | The verbose skip output (G1) and every log/assertion this step | | | touches state PRESENCE/ABSENCE of $ANTHROPIC_API_KEY only -- never | | | a value. Confirmed by reading the actual captured output, not | | | just asserting the property. | | G6 | grep for env=os.environ / env=None in adapters/claude/adapter.py -- | | | empty; `_subprocess_env()` builds an explicit | | | PATH/HOME/ANTHROPIC_API_KEY-only dict, confirmed by reading the | | | call site (Popen(..., env=self._subprocess_env())). | | G7 | rm -rf .venv work && make venv/smoke/test: 17 passed, 7 skipped, | | | zero regression on step.1's smoke check or step.2's original 8 | | | mock-leg contract tests (now split 7 contract + 1 relocated mock- | | | specific, same 8 assertions, none dropped). | +------+---------------------------------------------------------------------+ ``` ## Findings for later steps 1. **A capability flag should never duplicate an existing contract field.** SUPPORTS_TOOL_CALLS would have been a second source of truth for exactly what capabilities()["tools"] already states -- the two could drift (an adapter forgetting to keep them in sync). The rule going forward: before adding a NEW capability-signaling mechanism, check design.md §2.4's envelope/capabilities() shape first; only add a class-level flag for a property that shape genuinely doesn't cover (SUPPORTS_INTERLEAVED_ABORT qualifies; SUPPORTS_TOOL_CALLS didn't). 2. **A "shared" contract suite built against exactly ONE real implementation cannot prove it is actually adapter-agnostic -- only that it's mock-agnostic.** step.2's G2 dry-run (a second MOCK instance) proved the REGISTRATION mechanism works, which was real and worth proving, but it could never have caught test_unrecognized_request_fails_loud's mock-specific assumption, because both "adapters" in that dry-run shared the exact same fixture-routing implementation. The first REAL second adapter is what actually stress-tests "is this test universal or did I just write it against the one implementation I had in front of me." Worth remembering for U6.1 (openai, the second REAL adapter after this one): expect it to surface at least one more test that turns out to be claude-code-specific or mock-specific, not universal -- that's not a sign of a bad suite, it's what a THIRD real implementation is for. 3. **Safety-by-research, not safety-by-caution-after-the-fact, worked.** Every load-bearing safety property (API-key-only auth, no ambient-session fallback, tools hardcoded off, minimal subprocess env) was decided BEFORE writing adapter code, grounded in verified documentation rather than assumption -- and none of it needed revision once the code existed. The one real correction this step made (d12) was a DESIGN-DOCUMENT refinement (what "rides the operator's subscription" actually covers), not a safety walkback. 4. **The documented-JSON-shape fixture (Q-3-E) is real but unproven coverage.** `test_responses_from_json_*` genuinely exercises the parsing LOGIC and would catch a coding mistake in `_responses_from_json` today -- but it cannot catch a mismatch between the documented shape and what Claude Code's `--output- format json` ACTUALLY emits in practice. Flagged in-code and here: whoever configures `$ANTHROPIC_API_KEY` first and runs this adapter live for the first time should immediately diff a real captured response against `_DOCUMENTED_SHAPE_FIXTURE` and fix any mismatch before trusting a live G1 pass. ## Numbers ``` +--------------------------------------+----------------+ | new adapters registered | 1 (claude-code,| | | total now 2) | | new tests | 16 (9 adapter- | | | internal, 1 | | | mock-specific, | | | 7 skip in this | | | env x1 param) | | capability flags kept | 1 (down from | | | 2 planned) | | tests relocated (not new, not lost) | 1 | | live subprocess invocations this | 0 | | session | | | BUILD commit 96afa52 | 7 files, +410 | +--------------------------------------+----------------+ ``` U1.2 done -> unblocks U1.3 (the one-round-trip CLI) and U1.4 (P1 gates + retro, which closes the whole P1 phase). Ledger updated in this RETRO commit per the §0 execution contract. End of retro.