diff --git a/docs/PRD.md b/docs/PRD.md index 82932f8..6b443e7 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -109,16 +109,30 @@ Many runs may reference one contract. This is what makes A/B comparison across m The harness exposes a session identifier but **not** the model. Resolution order: -1. `CLAUDE_CODE_SESSION_ID` from the environment → locate +1. `TDD_EXECUTOR_MODEL` environment variable — set by the launching harness when it knows + the answer (e.g. a subagent harness that inherits its parent's `CLAUDE_CODE_SESSION_ID` + and would be mis-attributed). Recorded with `source: declared`. Wins over transcript. +2. `CLAUDE_CODE_SESSION_ID` from the environment → locate `~/.claude/projects//.jsonl` → read the `model` field. -2. Failing that, a `--executor` label supplied by a **human** at `run start`. -3. Failing that, `unknown`, and the run is excluded from model-comparison metrics. + Recorded with `source: transcript`. +3. Failing that, a `--executor` label supplied by a **human** at `run start`. + Recorded with `source: human`. +4. Failing that, `unknown` (`source: unknown`), and the run is excluded from model-comparison + metrics. `Executor.reason` records why: `CLAUDE_CODE_SESSION_ID` not set; no transcript + found for the session; the transcript contains no model records. + +When resolution yields `unknown`, `run start` emits an `executor_unknown` integrity event +(detail = the reason) and includes `executor_warning` in the success envelope so the gap is +visible at the moment it can still be fixed. + +`tdd doctor` reports an informational `executor identity` check (always `ok: true`) showing +`: `, plus the reason when the source is `unknown`. - **R5.1** The transcript lookup is isolated behind a single resolver so an undocumented format change breaks one function, not the tool. -- **R5.2** Agents never supply executor identity by any path. Step 2 is a human affordance. +- **R5.2** Agents never supply executor identity by any path. Step 3 is a human affordance. - **R5.3** Resolution requires the tool to run on the same host as the agent. Remote or CI - execution falls through to step 2. + execution can set `TDD_EXECUTOR_MODEL` to declare the answer explicitly. ### Cycle | Field | Notes | @@ -161,7 +175,7 @@ Reserved per-run keys: `plan_quality_score` (per plan phase, with rationale), `c ### IntegrityEvent Typed: `test_removed`, `test_weakened`, `undeclared_file_touched`, `restore_mismatch`, -`off_protocol_invocation`, `stale_artifact`, `plan_blob_changed`. +`off_protocol_invocation`, `stale_artifact`, `plan_blob_changed`, `executor_unknown`. ### Blocker Typed: `regression`, `target_unfixable`, `bad_red`, `plan_defect`, `tooling`, `context_exhausted`, diff --git a/docs/harness-integration.md b/docs/harness-integration.md index e4ec0f4..a63d279 100644 --- a/docs/harness-integration.md +++ b/docs/harness-integration.md @@ -104,16 +104,27 @@ and auditors should read the two accordingly. ## Executor identity and subagents -`tdd run start` records which model is executing by reading the harness session id -(`CLAUDE_CODE_SESSION_ID`) and resolving the model from that session's transcript. -This is trustworthy only when the executor is a **top-level session** — its own -terminal, worktree, or cloud session. +`tdd run start` records which model is executing. Resolution order (first match wins): + +1. **`TDD_EXECUTOR_MODEL`** — set this env var when the launching harness knows the answer + (e.g. a CI pipeline or a subagent harness). Recorded with `source: declared`. Takes + precedence over transcript detection. +2. **Transcript** — reads `CLAUDE_CODE_SESSION_ID` and resolves the model from + `~/.claude/projects//.jsonl`. Recorded with `source: transcript`. + Trustworthy only for top-level sessions (see below). +3. **`--executor`** — human-supplied label at `run start`. Recorded with `source: human`. +4. **`unknown`** — `Executor.reason` records why: env var not set; no transcript for the + session; transcript contains no model records. An `executor_unknown` integrity event is + emitted and `result.executor_warning` is set in the `run start` envelope. + +`tdd doctor` always reports an informational `executor identity` check (`ok: true`) showing +the resolved `: `, plus the reason when the source is `unknown`. An in-process subagent (Claude Code's Agent/Task tool) inherits the parent's session id and has no transcript of its own in the location the resolver reads, so a run -started by a subagent is attributed to the **parent's** model. The run itself is -unaffected — but if you are comparing models across runs, dispatch executors as -separate top-level sessions, not as subagents, or the comparison is silently wrong. +started by a subagent would be attributed to the **parent's** model. Set +`TDD_EXECUTOR_MODEL` in the subagent's environment to declare the correct model +explicitly — the declared path was added precisely to fix this case. ## Concurrent-command refusals diff --git a/src/tddcli/cli.py b/src/tddcli/cli.py index ce1ee15..7348f59 100644 --- a/src/tddcli/cli.py +++ b/src/tddcli/cli.py @@ -304,6 +304,12 @@ def cmd_doctor(args) -> Envelope: "ledger outside worktree", not str(ledger.path).startswith(str(worktree)), str(ledger.path) ) + ex = identity.resolve(worktree) + ex_detail = f"{ex.source}: {ex.model}" + if ex.source == "unknown": + ex_detail += f" — {ex.reason}" + check("executor identity", True, ex_detail) + projects: dict[str, dict] = {} for name, project in cfg.projects.items(): before = len(checks) @@ -798,6 +804,8 @@ def cmd_run_start(args) -> Envelope: ledger.event(run_id, None, "baseline_reused", json.dumps(sorted(reused))) if implausible: ledger.event(run_id, None, "baseline_accepted", json.dumps(implausible)) + if executor.source == "unknown": + ledger.event(run_id, None, "executor_unknown", executor.reason or "") # Baselines and the collection snapshot, per project (R9.5, R8.9) — from the # probe above, so the suite is not run twice. @@ -850,12 +858,15 @@ def cmd_run_start(args) -> Envelope: verb, opening = engine.opening_action(cycle) detail = f"Run {run_id} started ({executor.model}, via {executor.source}). {opening}" + result: dict = { + "baselines": {n: len(v) for n, v in ledger.baselines(run_id).items()}, + "executor_source": executor.source, + } + if executor.source == "unknown": + result["executor_warning"] = executor.reason or "" return Envelope( run=engine.run_state(cycle), - result={ - "baselines": {n: len(v) for n, v in ledger.baselines(run_id).items()}, - "executor_source": executor.source, - }, + result=result, next_action=NextAction(verb, detail), ) finally: diff --git a/src/tddcli/identity.py b/src/tddcli/identity.py index 671f326..476cce8 100644 --- a/src/tddcli/identity.py +++ b/src/tddcli/identity.py @@ -26,7 +26,8 @@ class Executor: model: str session: str | None - source: str # transcript | human | unknown + source: str # transcript | human | declared | unknown + reason: str | None = None def _slug(path: Path) -> str: @@ -69,14 +70,25 @@ def _model_from_transcript(path: Path) -> str | None: def resolve(project_path: Path | None = None, human_label: str | None = None) -> Executor: session = os.environ.get("CLAUDE_CODE_SESSION_ID") - if session: + + declared = os.environ.get("TDD_EXECUTOR_MODEL") + if declared: + return Executor(model=declared, session=session, source="declared") + + reason: str | None = None + if not session: + reason = "CLAUDE_CODE_SESSION_ID is not set" + else: transcript = _find_transcript(session, project_path) - if transcript is not None: + if transcript is None: + reason = f"no transcript for session {session} under {TRANSCRIPT_ROOT}" + else: model = _model_from_transcript(transcript) if model: return Executor(model=model, session=session, source="transcript") + reason = f"no model records in transcript {transcript}" if human_label: return Executor(model=human_label, session=session, source="human") - return Executor(model="unknown", session=session, source="unknown") + return Executor(model="unknown", session=session, source="unknown", reason=reason) diff --git a/tasks/friction-logs/issue-74-executor-attribution-friction.md b/tasks/friction-logs/issue-74-executor-attribution-friction.md new file mode 100644 index 0000000..6006fd1 --- /dev/null +++ b/tasks/friction-logs/issue-74-executor-attribution-friction.md @@ -0,0 +1,89 @@ +# Implementation Friction Log: tasks/issue-74-executor-attribution.md + +- Run: 15 +- Executor: claude-sonnet-4-6 (source: transcript) +- Plan blob: `a66891f7adb8b6ea80f680ac92727388eff9ed8a` (declared) +- Started: 2026-08-29T07:15:49.931037+00:00 Ended: 2026-08-29T08:03:42.177931+00:00 Outcome: complete +- Baseline failures at start: tddcli=0 + +## Plan fidelity + +- Declared cycles: 8 +- Delivered: 8 Skipped: 0 +- Never reached: none +- Human interventions: 0 + +### Cycle 8: doctor reports executor identity and the failure reason informationally _(standard)_ +- **Target:** `tddcli::tests/test_executor_attribution.py::test_doctor_reports_executor_identity` +- **Projects:** `tddcli` +- **Suite runs by phase:** {'AWAITING_TEST': 1, 'AWAITING_IMPL': 1, 'CLOSE_SWEEP': 1} +- **First run outcome:** failed (as expected) +- **Commits:** + - `241591a22` [red] test: doctor names the executor-identity diagnosis (1 files) + - `c658611f1` [green] feat: informational executor identity check in doctor (1 files) + +### Cycle 7: the run start envelope surfaces the attribution warning _(standard)_ +- **Target:** `tddcli::tests/test_executor_attribution.py::test_run_start_envelope_carries_executor_warning` +- **Projects:** `tddcli` +- **Suite runs by phase:** {'AWAITING_TEST': 1, 'AWAITING_IMPL': 1, 'CLOSE_SWEEP': 1} +- **First run outcome:** failed (as expected) +- **Commits:** + - `86541dc3d` [red] test: run start result warns when the executor is unknown (1 files) + - `2d33e0d0e` [green] feat: executor_warning in the run start envelope (1 files) + +### Cycle 6: run start records an executor_unknown event with the reason _(standard)_ +- **Target:** `tddcli::tests/test_executor_attribution.py::test_run_start_records_executor_unknown_event` +- **Projects:** `tddcli` +- **Suite runs by phase:** {'AWAITING_TEST': 1, 'AWAITING_IMPL': 1, 'CLOSE_SWEEP': 1} +- **First run outcome:** failed (as expected) +- **Commits:** + - `726ea02f2` [red] test: an unattributed run leaves an executor_unknown event (1 files) + - `daf7f7f97` [green] feat: run start logs executor_unknown with the detection reason (1 files) + +### Cycle 5: resolve records why detection failed: transcript has no model line _(standard)_ +- **Target:** `tddcli::tests/test_executor_attribution.py::test_reason_names_the_model_less_transcript` +- **Projects:** `tddcli` +- **Suite runs by phase:** {'AWAITING_TEST': 1, 'AWAITING_IMPL': 1, 'CLOSE_SWEEP': 1} +- **First run outcome:** failed (as expected) +- **Commits:** + - `6764fa319` [red] test: unknown executor carries the no-model-record reason (1 files) + - `34029594a` [green] feat: reason distinguishes a model-less transcript from a missing one (1 files) + +### Cycle 4: resolve records why detection failed: transcript not found _(standard)_ +- **Target:** `tddcli::tests/test_executor_attribution.py::test_reason_names_the_missing_transcript` +- **Projects:** `tddcli` +- **Suite runs by phase:** {'AWAITING_TEST': 1, 'AWAITING_IMPL': 1, 'CLOSE_SWEEP': 1} +- **First run outcome:** failed (as expected) +- **Commits:** + - `cd3685c78` [red] test: unknown executor carries the no-transcript reason (1 files) + - `8e0a82601` [green] feat: reason names the session whose transcript was not found (1 files) + +### Cycle 3: resolve records why detection failed: session env missing _(standard)_ +- **Target:** `tddcli::tests/test_executor_attribution.py::test_reason_names_the_missing_session_env` +- **Projects:** `tddcli` +- **Suite runs by phase:** {'AWAITING_TEST': 1, 'AWAITING_IMPL': 1, 'CLOSE_SWEEP': 1} +- **First run outcome:** failed (as expected) +- **Commits:** + - `603318a7e` [red] test: unknown executor carries the missing-env reason (1 files) + - `f648879ae` [green] feat: Executor.reason — CLAUDE_CODE_SESSION_ID not set (1 files) + +### Cycle 2: the declared override wins over a readable transcript _(standard)_ +- **Target:** `tddcli::tests/test_executor_attribution.py::test_declared_override_beats_transcript` +- **Projects:** `tddcli` +- **Suite runs by phase:** {'AWAITING_TEST': 1, 'AWAITING_IMPL': 1, 'CLOSE_SWEEP': 2} +- **First run outcome:** failed (as expected) +- **Commits:** + - `325b820e4` [red] test: declared identity overrides transcript detection (1 files) + - `1a2d64046` [green] feat: declared executor identity takes precedence (1 files) + - `369b18d35` [refactor] refactor: the declared override wins over a readable transcript (1 files) + +### Cycle 1: TDD_EXECUTOR_MODEL resolves with source declared _(standard)_ +- **Target:** `tddcli::tests/test_executor_attribution.py::test_env_override_resolves_as_declared` +- **Projects:** `tddcli` +- **Suite runs by phase:** {'AWAITING_TEST': 1, 'AWAITING_IMPL': 1, 'CLOSE_SWEEP': 2} +- **First run outcome:** failed (as expected) +- **Commits:** + - `dffa231ed` [red] test: TDD_EXECUTOR_MODEL yields source declared (1 files) + - `63a3fa815` [green] feat: harness-declared executor identity via TDD_EXECUTOR_MODEL (1 files) + - `6c606917c` [refactor] refactor: TDD_EXECUTOR_MODEL resolves with source declared (1 files) + diff --git a/tests/conftest.py b/tests/conftest.py index 6864f9c..3ebc2d8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,6 +20,15 @@ def _isolated_lease_dir(tmp_path, monkeypatch): monkeypatch.setenv("TDD_LEASE_DIR", str(tmp_path / "worker-leases")) +@pytest.fixture(autouse=True) +def _pinned_executor_identity(monkeypatch): + """A developer's shell resolves identity from its live Claude session; CI + resolves nothing and every run logs executor_unknown. Pin a declared + identity so both behave the same; the attribution tests delenv this to + exercise the unknown paths.""" + monkeypatch.setenv("TDD_EXECUTOR_MODEL", "pytest-executor") + + @pytest.fixture def ledger_home(tmp_path, monkeypatch): home = tmp_path / "ledger-home" diff --git a/tests/test_executor_attribution.py b/tests/test_executor_attribution.py new file mode 100644 index 0000000..3d3b823 --- /dev/null +++ b/tests/test_executor_attribution.py @@ -0,0 +1,133 @@ +from __future__ import annotations + +import json + +from conftest import run_cli, write_plan +from tddcli import identity +from tddcli.ledger import Ledger + +MINIMAL_PLAN = """\ +--- +cycles: + - n: 1 + project: backend + title: "placeholder" + test: "tests/test_smoke.py::test_smoke" + commit_red: "test: placeholder" + commit_green: "feat: placeholder" +--- +# Minimal plan for executor attribution tests +""" + + +def test_env_override_resolves_as_declared(tmp_path, monkeypatch): + monkeypatch.setenv("TDD_EXECUTOR_MODEL", "harness-model") + monkeypatch.delenv("CLAUDE_CODE_SESSION_ID", raising=False) + monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", tmp_path / "nowhere") + + e = identity.resolve(None) + assert e.model == "harness-model" + assert e.source == "declared" + + +def test_declared_override_beats_transcript(tmp_path, monkeypatch): + slug = str(tmp_path / "proj").replace("/", "-") + transcripts = tmp_path / "projects" / slug + transcripts.mkdir(parents=True) + (transcripts / "sess-99.jsonl").write_text( + json.dumps({"type": "assistant", "model": "claude-transcript-model"}) + "\n" + ) + monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", tmp_path / "projects") + monkeypatch.setenv("CLAUDE_CODE_SESSION_ID", "sess-99") + monkeypatch.setenv("TDD_EXECUTOR_MODEL", "harness-model") + + e = identity.resolve(tmp_path / "proj") + assert e.source == "declared" + assert e.model == "harness-model" + + +def test_reason_names_the_missing_session_env(tmp_path, monkeypatch): + monkeypatch.delenv("CLAUDE_CODE_SESSION_ID", raising=False) + monkeypatch.delenv("TDD_EXECUTOR_MODEL", raising=False) + monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", tmp_path / "nowhere") + + e = identity.resolve(None) + assert e.model == "unknown" + assert "CLAUDE_CODE_SESSION_ID" in e.reason + + +def test_reason_names_the_missing_transcript(tmp_path, monkeypatch): + empty_root = tmp_path / "projects" + empty_root.mkdir() + monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", empty_root) + monkeypatch.setenv("CLAUDE_CODE_SESSION_ID", "sess-gone") + monkeypatch.delenv("TDD_EXECUTOR_MODEL", raising=False) + + e = identity.resolve(None) + assert e.model == "unknown" + assert e.reason and "sess-gone" in e.reason + + +def test_reason_names_the_model_less_transcript(tmp_path, monkeypatch): + root = tmp_path / "projects" + (root / "slug").mkdir(parents=True) + (root / "slug" / "sess-empty.jsonl").write_text( + json.dumps({"type": "user", "content": "hello"}) + "\n" + ) + monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", root) + monkeypatch.setenv("CLAUDE_CODE_SESSION_ID", "sess-empty") + monkeypatch.delenv("TDD_EXECUTOR_MODEL", raising=False) + + e = identity.resolve(None) + assert e.model == "unknown" + assert e.reason and "no model" in e.reason.lower() + + +def test_run_start_records_executor_unknown_event(repo, tmp_path, monkeypatch): + monkeypatch.delenv("CLAUDE_CODE_SESSION_ID", raising=False) + monkeypatch.delenv("TDD_EXECUTOR_MODEL", raising=False) + monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", tmp_path / "empty-transcripts") + + plan = write_plan(repo, MINIMAL_PLAN) + run_cli(repo, "plan", "register", plan) + out = run_cli(repo, "run", "start", "--plan", plan) + assert out["ok"], out + + ledger = Ledger(repo) + rows = ledger.all( + "SELECT detail FROM integrity_event WHERE kind = 'executor_unknown'" + ) + assert len(rows) == 1 + assert "CLAUDE_CODE_SESSION_ID" in rows[0]["detail"] + + +def test_run_start_envelope_carries_executor_warning(repo, tmp_path, monkeypatch): + monkeypatch.delenv("CLAUDE_CODE_SESSION_ID", raising=False) + monkeypatch.delenv("TDD_EXECUTOR_MODEL", raising=False) + monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", tmp_path / "empty-transcripts") + + plan = write_plan(repo, MINIMAL_PLAN) + run_cli(repo, "plan", "register", plan) + out = run_cli(repo, "run", "start", "--plan", plan) + assert out["ok"], out + + warning = out["result"].get("executor_warning") + assert warning and "CLAUDE_CODE_SESSION_ID" in warning + + +def test_doctor_reports_executor_identity(repo, tmp_path, monkeypatch): + monkeypatch.delenv("CLAUDE_CODE_SESSION_ID", raising=False) + monkeypatch.delenv("TDD_EXECUTOR_MODEL", raising=False) + monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", tmp_path / "empty-transcripts") + + out = run_cli(repo, "doctor") + assert out["result"]["healthy"] is True + + check = next( + (c for c in out["result"]["checks"] if c["check"] == "executor identity"), + None, + ) + assert check is not None, "executor identity check not found" + assert check["ok"] is True + assert "unknown" in check["detail"] + assert "CLAUDE_CODE_SESSION_ID" in check["detail"] diff --git a/tests/test_snapshot_and_identity.py b/tests/test_snapshot_and_identity.py index abc4a03..6c66327 100644 --- a/tests/test_snapshot_and_identity.py +++ b/tests/test_snapshot_and_identity.py @@ -81,6 +81,7 @@ def test_model_is_read_from_the_session_transcript(tmp_path, monkeypatch): identity, "TRANSCRIPT_ROOT", tmp_path / "home" / ".claude" / "projects" ) monkeypatch.setenv("CLAUDE_CODE_SESSION_ID", "sess-1") + monkeypatch.delenv("TDD_EXECUTOR_MODEL", raising=False) executor = identity.resolve(project) assert executor.model == "claude-sonnet-4-6" @@ -90,6 +91,7 @@ def test_model_is_read_from_the_session_transcript(tmp_path, monkeypatch): def test_human_label_is_the_fallback_not_the_default(tmp_path, monkeypatch): monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", tmp_path / "nowhere") monkeypatch.delenv("CLAUDE_CODE_SESSION_ID", raising=False) + monkeypatch.delenv("TDD_EXECUTOR_MODEL", raising=False) assert identity.resolve(None, "opus-by-hand").source == "human" assert identity.resolve(None).model == "unknown" @@ -104,6 +106,7 @@ def test_last_model_wins_when_a_session_switches(tmp_path, monkeypatch): ) monkeypatch.setattr(identity, "TRANSCRIPT_ROOT", root) monkeypatch.setenv("CLAUDE_CODE_SESSION_ID", "s") + monkeypatch.delenv("TDD_EXECUTOR_MODEL", raising=False) assert identity.resolve(None).model == "claude-sonnet-4-6"