From 52e43cafe5bd291ce0e983deadc61c03473e32f3 Mon Sep 17 00:00:00 2001 From: memtomem <118508877+memtomem@users.noreply.github.com> Date: Mon, 9 Nov 2026 19:53:41 +0900 Subject: [PATCH 1/2] Repair one room candidate with one provider call (#173 PR 1) `room repair CAND --prompt ... --preview` shows what would run and prints a token; `--confirm TOKEN` runs it only if a plan rebuilt at that moment is identical. Only the member that produced the candidate runs, from that candidate, and the result is a new candidate, verified again, that the user compares and selects. Peers, their candidates and the source stay as they were. The claim's transaction re-derives the revision, the parent rules and the stored session, so a change after confirmation refuses before a round exists. A task that would only overflow once rendered is refused at the preview, where it cannot freeze the room. Repair rounds write provenance v3; all-member rounds keep writing v2. `round_expected_members` narrows conversation records and `_missing_members` to the target of a believed repair, so a damaged record keeps today's warnings. This departs from the approved design in three places, because #273 merged after it. A retained worktree now holds its candidate, so the checkout is rebuilt only when it differs, which keeps ignored files in the common case. No hook flags (#269 owns hooks). `rebuild_worktree` already existed. Measured with real Claude, Codex and Kimi Code: resume survives a forced rebuild, and an unknown session fails rather than starting a new conversation. Refs #173 Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 1 + docs/architecture/persisted-contracts.md | 2 +- docs/room-targeted-repair-plan.md | 113 ++- docs/user-guide.ko.md | 26 + docs/user-guide.md | 28 + src/syncmill/cli.py | 165 ++++ src/syncmill/room_compare.py | 79 +- src/syncmill/rooms.py | 552 ++++++++++++- tests/test_room_repair.py | 958 +++++++++++++++++++++++ tests/test_round_provenance.py | 144 ++++ 10 files changed, 1967 insertions(+), 101 deletions(-) create mode 100644 tests/test_room_repair.py diff --git a/CLAUDE.md b/CLAUDE.md index 1ba35ab2..b0a63cfe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,6 +29,7 @@ uv run syncmill room export [--round N | --all-rounds] [--format markd uv run syncmill room export --include patch: [--include output:|gate-output:|prompt: ...] (--preview | --confirm TOKEN) # opt-in raw content (room_raw.py, #189): preview prints sizes/digests/first lines + a token over every item descriptor; confirm re-collects and writes only on an equal token; reads bound to the comparison's room/round/digests; gate-output accepted only when room_compare._gate_verdict accepts the record; markdown = escaped display rendering, json adds `raw_content` (base64, ensure_ascii); never redacted # room rounds also record each member's conversation (session cold/resume/fork, checkpoint candidate ids + omitted, memory delivery; v2 adds lessons.selected ids + per-member lessons delivery, readers accept v1+v2) in rounds.conversation_json, written once under the lease before the fan-out (rooms.round_conversation_status binds it to SQLite rows); compare shows per-candidate facts and round counts only (plus the round's selected lesson ids) # room rounds record their effective starting tree + gate settings (v2 adds the gate's env_allowlist: null = inherited, [] distinct) in rounds.provenance_json (rooms.round_provenance_status, writer and readers share it, readers accept v1+v2; NULL = legacy); compare/export/TUI show them as starting tree / gate settings beside the report-derived gate profile, whose JSON fields are unchanged; a v1 round's environment reads not recorded and never groups as same settings; `room apply --verify` still runs the current `[gate]` and prints a `verify:` notice (`rooms.verify_settings_notice`) unless the round's v2 record equals it +uv run syncmill room repair --prompt TEXT (--preview | --confirm TOKEN) [--cold] # #173 PR 1: one member runs from its newest completed candidate; the token is `RepairPlan.token` (rooms.py) over a plan rebuilt at confirm; `start_round(repair_guard=)` re-derives revision, parent rules and session in the claim's transaction; provenance v3 (repair rounds only, all-member rounds keep v2; `round_expected_members` narrows conversation records and `_missing_members`); the checkout is rebuilt only when it differs from the parent's patch; foreground only; design: docs/room-targeted-repair-plan.md uv run syncmill room select [--expected-revision N --expected-digest SHA256 --rationale TEXT] # verifies the artifact, records a bound selection (rooms.selection_json); apply refuses an inconsistent record uv run syncmill room judge accepted|rejected [--expected-digest SHA256 --rationale TEXT] # #174: append-only human verdict on the APPLIED candidate (room_judgments, bound to candidate + patch digest + applied run; never inferred from select/apply, never touches memory, no state/revision change); rooms.room_outcome_status is the one read model (not_selected / selected_not_applied / applied_unreviewed / accepted / rejected / unknown — a damaged latest record is unknown, never an older verdict) shown by compare/export as room.outcome uv run syncmill room promote --lesson TEXT [--scope GOAL --supersedes ID] # #174 PR 2: needs the board; only a judged room (accepted/rejected); a board decision bound to project_id with provenance_json (room/judgment/verdict/candidate/digest/run) and a unique source_key (retry returns the existing decision, never reactivates a superseded one); `run` shows then injects Board.project_lessons (project-wide scope; a room created with `room create --goal G` (rooms.goal, verbatim, fixed) also gets scope==G lessons via project_lessons(goal=), marked `(goal G)`; board.select_lessons budget; --skip-lesson/--no-lessons, forwarded in _inner_argv) via Supervisor.run(lessons=) AFTER routing classified the raw prompt; board items get unbound + own-project decisions; room rounds: `room round`/`start` show them (cli._show_round_lessons, rooms.round_lessons_fit) and pass RoundLesson pairs to run_round(lessons=) (detached: a second anonymous fd, --lessons-fd); the coordinator never opens the board; lessons sit after the checkpoint, before the task, excerpts budgeted around them; TUI (Rooms `j` verdict / `p` promote, `tui.py` `_confirm_room_judge`/`_confirm_lesson_promote`): the round dialog reads `project_lessons` once on the Textual thread (`RoundLessonsView`), recomputes `select_lessons(skip=unchecked)` live, refuses (never drops) when `round_lessons_fit` fails on `round_prompt`'s fallback rule, and launches with exactly those pairs; promote passes `expected_judgment_id` (checked on the same outcome read as the provenance); the round/verdict/promote dialog callbacks act on the captured room id + store root diff --git a/docs/architecture/persisted-contracts.md b/docs/architecture/persisted-contracts.md index 8c13bccb..cd4b955b 100644 --- a/docs/architecture/persisted-contracts.md +++ b/docs/architecture/persisted-contracts.md @@ -52,7 +52,7 @@ A golden over the current writer does not protect these. A reader can stop accep | Family | Documents | Readers | Frozen by | |---|---|---|---| | Artifacts | run `manifest.json`, `result.json`, `winner.patch`, apply receipts (`applications/*.json`), in every state a standalone `run` leaves them (room-, session- and workflow-origin artifacts are frozen with those families) | `runs list` / `show` / `prune`, `conversations list` / `show`, `diff`, `apply`, `recover` (`_bound_receipt`), `continue`, the TUI artifact view | `test_frozen_artifacts.py` (v0.1.2, v0.3.0) | -| Rooms | the room database (`rooms.sqlite3`, with the WAL a killed process leaves): rooms, rounds, candidates, members, workspaces, events, `sessions_json`, `selection_json` v1, `provenance_json` v1/v2, `conversation_json` v1; candidate `manifest.json`, patch and output; room-bound run artifacts. No release wrote `conversation_json` v2 or judgment rows | `room list` / `rounds` / `show` / `candidates` / `diff` / `compare` / `export`, `select`, `apply`, `judge`, `recover`, `close`, `prune` `RoomStore.member_sessions` (and through these `RoomCandidateStore.load`, `_parse_sessions_json`, `selection_record_status`, `room_outcome_status`, `round_provenance_status`, `round_conversation_status`); the standalone `apply` / `recover` refusal and skip for room-bound artifacts. Not yet: a HEAD round continuing a released room, the room viewer's log paths, `apply --verify` settings notices, raw export, the TUI room views | `test_frozen_rooms.py` (v0.1.2, v0.2.0, v0.3.0), partly | +| Rooms | the room database (`rooms.sqlite3`, with the WAL a killed process leaves): rooms, rounds, candidates, members, workspaces, events, `sessions_json`, `selection_json` v1, `provenance_json` v1/v2, `conversation_json` v1; candidate `manifest.json`, patch and output; room-bound run artifacts. No release wrote `provenance_json` v3, `conversation_json` v2 or judgment rows | `room list` / `rounds` / `show` / `candidates` / `diff` / `compare` / `export`, `select`, `apply`, `judge`, `recover`, `close`, `prune` `RoomStore.member_sessions` (and through these `RoomCandidateStore.load`, `_parse_sessions_json`, `selection_record_status`, `room_outcome_status`, `round_provenance_status`, `round_conversation_status`); the standalone `apply` / `recover` refusal and skip for room-bound artifacts. Not yet: a HEAD round continuing a released room, the room viewer's log paths, `apply --verify` settings notices, raw export, the TUI room views | `test_frozen_rooms.py` (v0.1.2, v0.2.0, v0.3.0), partly | | Workflows | stored `spec_json`, manifest `schema_version` 1 and 3 with accounting, node `result.json` and `commit.json`; workflow-origin run artifacts (v0.2.0 on) | `WorkflowStore.load`, bundle verification; for workflow-origin artifacts, the approval check at `apply` | not yet | | Sessions and project | isolated-session documents and their checkpoint run artifacts, project marker `.syncmill/project.json` | `IsolatedSessionStore.load` / `list`, `_marker_project_id`; the artifact readers for checkpoint runs | not yet | | Board | item rows and their evidence and inputs, decisions | `_row_to_item`, `_load_evidence`, `_row_to_decision` | not yet | diff --git a/docs/room-targeted-repair-plan.md b/docs/room-targeted-repair-plan.md index 6739fb0c..3077b56f 100644 --- a/docs/room-targeted-repair-plan.md +++ b/docs/room-targeted-repair-plan.md @@ -1,7 +1,9 @@ # Targeted room repair: design for #173 -Status: **APPROVED** 2026-11-09, not implemented. Baseline: main @ `132e6c4`. The proposed -answers in section 14 are the approved ones. Implementation follows #171 on the roadmap. +Status: **APPROVED** 2026-11-09. PR 1 (section 9) is implemented; the rest is not. Baseline: +main @ `132e6c4`, before #273 changed what a retained worktree holds. Where that moved a +decision, the section says so. The proposed answers in section 14 are the approved ones, except +where decision 5 says otherwise. Issue: [#173](https://github.com/memtomem/syncmill/issues/173). Semantic boundary: [ADR-0005](adr/adr-0005-evidence-first-review.md). @@ -24,18 +26,29 @@ Non-goals: All of these ran on fake agents in a sandbox, on the locked environment. -**A retained worktree does not hold its candidate's bytes.** `run_round` captures the -patch and then runs the gate in the same worktree. With a gate that writes a file: +**A retained worktree did not hold its candidate's bytes, at this baseline.** `run_round` +captured the patch and then ran the gate in the same worktree. With a gate that writes a file: | Round | Patch digest | `files_changed` | Gate | | --- | --- | --- | --- | | 1 | `e04af41017` | `calc.py` | passed | | 2 (the agent changed nothing) | `f1a6bd1257` | `calc.py`, `gate-debris.txt` | passed | -Round 2's candidate carries a file no agent wrote, and applying it put that file in the -source. This is an existing defect, [#268](https://github.com/memtomem/syncmill/issues/268). -A `python3 -m unittest` gate did not leak `__pycache__` in the same setup, because macOS's -system Python writes bytecode to `sys.pycache_prefix`. +Round 2's candidate carried a file no agent wrote, and applying it put that file in the +source. This was [#268](https://github.com/memtomem/syncmill/issues/268). A `python3 -m unittest` +gate did not leak `__pycache__` in the same setup, because macOS's system Python writes bytecode +to `sys.pycache_prefix`. + +**#273 fixed that, and it changes what a repair must do.** Measured after it, with fake agents, one +member, and an agent writing `calc.py`, `.gitignore` (`.venv/`) and `.venv/agent-dep`: + +| The gate writes | Tree digest equals the candidate's | `.venv/agent-dep` | `workspace.rebuilt` | +| --- | --- | --- | --- | +| only an ignored file | yes | kept | no | +| `gate-debris.txt` | yes | gone | yes, at the end of that round | + +So a retained worktree now holds its candidate unless a later failed or timed-out attempt changed +it, and section 5.2 rebuilds only then. **A candidate's tree can be rebuilt at the same path.** The worktree held an agent commit, a binary change, a mode change, a delete, a rename, a symlink change, a file with no @@ -61,8 +74,25 @@ round 2 asks for the codeword without stating it. The control round passes `--fr | Codex | `resume`, codeword returned | unchanged | `UNKNOWN` | In both resumed runs, `note.txt` held both rounds' lines, a file left in the old checkout was -gone, and the rebuilt digest equalled the candidate's. Kimi Code was not measured, and this -used existing `room round` with the rebuild run by a script, since no targeted round exists yet. +gone, and the rebuilt digest equalled the candidate's. This used existing `room round` with the +rebuild run by a script, since no targeted round existed yet. + +**`room repair` itself, with real providers.** Claude Code 2.1.319, Codex CLI 0.167.0 and Kimi Code 2.5.2, +in one-member rooms with a gate, on macOS, 8 provider launches in all. Round 1's prompt holds a +random codeword. A file is then left in the checkout, so the repair must rebuild it: + +| Provider | Session | `workspace.rebuilt` | That file at launch, or in the patch | Codeword | Time | +| --- | --- | --- | --- | --- | --- | +| Claude | `resume` | 1 | no | returned | 12.1 s | +| Codex | `resume` | 1 | no | returned | 21.4 s | +| Kimi Code | `resume` | 1 | no | returned | 24.4 s | + +Each repair was one launch of that provider, and its candidate passed the gate. + +- A further Claude repair with a matching checkout: no `workspace.rebuilt`, and an ignored file + left in the checkout was still there. +- Codex with an unknown stored session id: a `failed` candidate in 2.5 s and no fallback to a new + conversation. The room stayed `awaiting_decision`, and `--cold --preview` on the parent worked. **Hooks.** With `post-index-change` and `post-checkout` hooks in the control repository: @@ -203,33 +233,37 @@ Checked in `preflight_round` and again inside `start_round`'s transaction: ### 5.2 Starting tree -After the lease is held and before any prompt is built, the target's checkout is rebuilt -from the parent's patch, at the path and branch its `room_workspaces` row records: +After the lease is held and before any prompt is built, the target's checkout is compared with +the parent's patch through `capture_changes`. If they match, the checkout is kept. If they differ, +which is what a failed attempt since the parent leaves, or if the capture fails, `rebuild_worktree` +replaces it at the path and branch its `room_workspaces` row records: ```text +rehearse the patch on a scratch checkout, then remove it cleanup_worktree(control, worktree) -git -c core.hooksPath=/dev/null worktree add -b -git -c core.hooksPath=/dev/null apply --binary - (skipped for an empty patch) -capture_changes(worktree, base) must have the parent's digest +git worktree add -b +git apply --binary - (skipped for an empty patch) +capture_changes(worktree, base) must have the parent's patch ``` -The `worktree add` and `apply` steps move out of `WorktreePool.acquire` into one helper that -both call, so repair and `branch-from` share a recipe. A mismatch or any failure goes to the -existing recovery path before a provider starts. Nothing is lost, since the parent's patch is -in its artifact. +`branch-from` shares the `worktree add` and `apply` steps through `_checkout_from_patch`. A +mismatch or any failure goes to the existing recovery path before a provider starts. Nothing is +lost, since the parent's patch is in its artifact. -**The cost: ignored files do not survive.** Installed dependencies and caches an agent or gate -left in the checkout are gone, as they are after `branch-from`. A gate that relies on them can -fail on the repair until the agent reinstalls them. The preview states this. +**The cost of a rebuild: ignored files do not survive it.** Installed dependencies and caches an +agent or gate left in the checkout are gone, as they are after `branch-from`. A gate that relies on +them can fail on the repair until the agent reinstalls them. A kept checkout holds them, so the +preview and the prompt say these files may be missing, and neither says which: the prompt's +digest is bound by the plan, so it stays one string. **What the digest check proves.** The tree is patch-equivalent to the parent as `capture_changes` sees it, and that is all `apply` ever consumes. It does not prove byte equality for content a clean or smudge filter hides from Git. Filters come from repository configuration an agent could write, and the rebuild does not defend against that, matching the stated trust model: the state root blocks traversal and redirection, not a same-credential -actor. The flag stops hooks during `worktree add` and `apply` only. It does not make the control -repository trusted, and the final `capture_changes` still runs hooks until that is fixed -separately. +actor. An earlier draft passed `-c core.hooksPath=/dev/null` to `worktree add` and `apply`. #273 +dropped that: `capture_changes` still runs hooks, so partial coverage only looks safer, and all +hook work belongs to #269. Only the target's checkout is touched, and its path is unchanged, which keeps provider sessions affine to it. Every member's workspace metadata is still validated, but only the target must be @@ -264,8 +298,8 @@ of memory: - The agent's task section is rendered at round start from the plan's feedback records: ```text -Repair your candidate CAND. Your working tree was rebuilt from exactly that -candidate's patch. Ignored files, such as installed dependencies, are not there. +Repair your candidate CAND. Your working tree holds exactly that candidate's +patch. Ignored files, such as installed dependencies, may be missing. Change requests: 1. [calc.py] ... @@ -275,8 +309,9 @@ Questions (answer these in your response): ``` The rendering is deterministic, and its digest is recorded (section 5.5), so a preview, -its confirmation and the worker agree on the bytes. A batch that does not fit -`_ROOM_PROMPT_MAX_BYTES` beside the selected lessons is refused at preview, never cut. +its confirmation and the worker agree on the bytes. A task or batch that does not fit +`_ROOM_PROMPT_MAX_BYTES` beside the selected lessons is refused at preview, never cut, including +with no lessons at all: such a task would otherwise first fail after the lease. Lessons work as they do in `room round`, including `--skip-lesson` and `--no-lessons`. By default the batch is the candidate's `open` requests. `--feedback ID` (repeatable) @@ -340,7 +375,9 @@ SHA-256 of the canonical JSON, following `room_raw.preview_token`: - candidate id, patch and output digests - member, provider, and the provider's effective execution settings as a digest of its `agent_config`, which covers timeouts and turn limits -- session mode and a digest of the stored session id +- session mode, a digest of the stored session id, `--cold`, and whether conversations are on. + With conversations off and a stored session, `--cold` and no `--cold` share a mode and a digest + but differ in whether the session is dropped - each feedback id with its record digest and derived status - the rendered task digest, lesson ids, and gate settings - a provider call count of 1 @@ -408,7 +445,8 @@ Each PR is usable alone and gets the implementation cross-review. detached path with its `repair.refused` event (section 6). 5. **Hunk anchors,** only if #190's evidence asks for them. -Implementation stays behind #171 under the roadmap's order; this design need not. +The maintainer started implementation on 2026-11-09, once #171's first delivery had shipped. What +is left on #171 (a Linux smoke, the human pilot, a TUI entry) does not block this. ## 10. Compatibility @@ -422,11 +460,10 @@ Implementation stays behind #171 under the roadmap's order; this design need not ## 11. Separate from this design -Both are existing behaviour, and this design fixes neither. +This design fixes neither. -- **[#268](https://github.com/memtomem/syncmill/issues/268): gate debris.** A gate's files enter - a `continue-own` round's next patch, and every `run` strategy's winner patch. Repair is immune - because it rebuilds first. +- **[#268](https://github.com/memtomem/syncmill/issues/268): gate debris.** Fixed by #273, after + this design was written. - **[#269](https://github.com/memtomem/syncmill/issues/269): repository-local hooks run during capture.** `isolated_git_env` strips ambient configuration but not hooks in the control repository, so `capture_changes` runs them after an agent's turn. @@ -487,6 +524,8 @@ Both are existing behaviour, and this design fixes neither. `recovery_required` like any round, so a good earlier candidate can no longer be selected. A lease-safe recovery back to `awaiting_decision` would help every round type, which makes it its own change rather than part of this one. -5. **Ignored files on repair.** Proposed: accept the loss, matching `branch-from`, and state it +5. **Ignored files on repair.** Approved: accept the loss, matching `branch-from`, and state it in the preview and the agent's prompt. Claude and Codex resume across the rebuild (section 2). - Kimi Code still needs measuring before PR 1 merges. + So does Kimi Code, measured for PR 1. #273 narrowed this: the loss now happens only when a + rebuild does (section 5.2), so a smoke of the resume must force a mismatch first, since an + ordinary retained tree is kept. diff --git a/docs/user-guide.ko.md b/docs/user-guide.ko.md index 429f50cd..8a83ba26 100644 --- a/docs/user-guide.ko.md +++ b/docs/user-guide.ko.md @@ -749,6 +749,7 @@ SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room cand SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room compare [--round N | --all-rounds] [--json] SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room export [--round N | --all-rounds] [--format markdown|json] [--include KIND:TARGET ... (--preview | --confirm TOKEN)] [--output FILE] SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room diff <후보-id> +SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room repair <후보-id> --prompt "고칠 내용" (--preview | --confirm TOKEN) [--cold] SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room select <후보-id> [--expected-revision N] [--expected-digest SHA256] [--rationale TEXT | --rationale-file PATH] SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room apply --verify [--expected-revision N] # 적용한 변경을 써 본 뒤: 받아들이는지 기록 @@ -986,6 +987,31 @@ resume 자체가 없으므로 모든 멤버가 델타가 아니라 전체 체크 `room round --fresh-session`은 저장된 세션 id를 버리고 새 대화로 시작하며, `room round --fork`(claude 멤버 전용)는 대화를 이어 가는 대신 fork합니다. 세션 id는 크래시와 `room recover`를 거쳐도 살아남습니다. provider가 토큰 사용량을 보고하면 +다른 멤버를 다시 돌리지 않고 후보 하나만 고치려면 `room repair`를 씁니다. 먼저 +미리 보고, 미리보기가 출력한 토큰으로 확정해요. + +```bash +syncmill room repair <후보-id> --prompt "고칠 내용" --preview +syncmill room repair <후보-id> --prompt "고칠 내용" --confirm <토큰> +``` + +미리보기는 아무것도 실행하지 않아요. 실행될 멤버(그 후보를 만든 멤버, provider 호출 +1번), 대화를 이어 가는지 새로 시작하는지, 검증 명령을 보여 줍니다. `--confirm`은 방, +후보, 세션, 설정, 옵션이 미리보기 때와 그대로일 때만 실행하고, 달라졌으면 다시 +미리 보라고 안내해요. 결과는 새 라운드의 새 후보이고, 이 라운드도 `max_rounds`에 +포함됩니다. 다시 검증하며 이전 게이트 결과를 물려받지 않고, 비교, 선택, 적용은 직접 +해야 해요(`room compare --round N`). 다른 멤버와 그 후보는 그대로이고 원본도 +건드리지 않습니다. + +- 멤버의 가장 최근 완료 후보만, 방이 `awaiting_decision`일 때만 고칠 수 있어요. 게이트에 + 실패한 후보도 됩니다. 내장 저장소가 든 후보는 안 돼요. 패치로 복원할 수 없기 때문입니다. +- 수리가 실패해도 원래 후보는 다시 고칠 수 있어요. provider가 대화를 이어 가지 못하면 + 조용히 새로 시작하지 않고 실패합니다. 이때는 `--cold`로 다시 미리 보세요. +- 체크아웃은 보통 그대로 둡니다. 실패한 시도가 바꿔 놓았다면 먼저 후보의 패치로 다시 + 만들고, 이때 설치된 의존성 같은 무시 파일이 사라져요. 그 파일에 기대는 게이트는 + 에이전트가 다시 설치할 때까지 실패할 수 있습니다. +- `--detach`와 TUI에서는 아직 수리를 쓸 수 없어요. + `room candidates`와 TUI 후보 화면에 `cache 85% (34.2k/40.1k)`처럼 캐시 적중률이 표시됩니다. diff --git a/docs/user-guide.md b/docs/user-guide.md index ed421437..13d44f34 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -792,6 +792,7 @@ SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room cand SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room compare [--round N | --all-rounds] [--json] SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room export [--round N | --all-rounds] [--format markdown|json] [--include KIND:TARGET ... (--preview | --confirm TOKEN)] [--output FILE] SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room diff +SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room repair --prompt "what to change" (--preview | --confirm TOKEN) [--cold] SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room select [--expected-revision N] [--expected-digest SHA256] [--rationale TEXT | --rationale-file PATH] SYNCMILL_ROOMS__ENABLED=true uv run --project "$SYNCMILL_SRC" syncmill room apply --verify [--expected-revision N] # after you have lived with the applied change: record whether you accept it @@ -1082,6 +1083,33 @@ every member then receives the full checkpoint rather than a delta. `room round --fresh-session` drops the stored session ids for one round, and `room round --fork` (claude members only) forks the conversations instead of continuing them in place. Session ids survive crashes and `room recover`. +To fix one candidate without running the other members again, use `room repair`. +Preview it, then confirm with the token the preview prints: + +```bash +syncmill room repair --prompt "what to change" --preview +syncmill room repair --prompt "what to change" --confirm +``` + +The preview starts nothing. It shows the member that will run (the one that produced +the candidate, one provider call), whether it resumes its conversation or starts cold, +and the verification commands. `--confirm` runs only if the room, the candidate, the +session, the settings and your options are still what the preview showed; otherwise it +asks you to preview again. The result is a new candidate in a new round, which counts +toward `max_rounds`. It is verified again and inherits no gate result, and you compare, +select and apply it yourself (`room compare --round N`). Other members and +their candidates stay as they were, and your source is not touched. + +- Only a member's newest completed candidate can be repaired, and only while the room + is `awaiting_decision`. A candidate whose gate failed qualifies. A candidate that holds + an embedded repository does not, because a patch cannot restore one. +- A failed repair leaves the original candidate repairable. A provider that cannot resume + its conversation fails instead of quietly starting a new one: preview again with `--cold`. +- The checkout usually stays as it was. If a failed attempt changed it, it is rebuilt from + the candidate's patch first, and that loses its ignored files, such as installed + dependencies. A gate that relies on them can fail until the agent reinstalls them. +- `--detach` and the TUI do not offer repair yet. + When a provider reports token usage, `room candidates` and the TUI candidate views show it — e.g. `cache 85% (34.2k/40.1k)` — so you can see the cache hit-rate per member per round. diff --git a/src/syncmill/cli.py b/src/syncmill/cli.py index 3308e4bf..fb97eec0 100644 --- a/src/syncmill/cli.py +++ b/src/syncmill/cli.py @@ -129,6 +129,7 @@ from syncmill.rooms import ( _ROOM_PROMPT_MAX_BYTES, JUDGMENT_VERDICTS, + RepairPlan, RoomCoordinator, RoomError, RoundLesson, @@ -3100,6 +3101,135 @@ def room_round( coordinator.close() +@room_group.command("repair") +@click.argument("candidate_id") +@click.option("--prompt", default=None, help="What to change in this candidate.") +@click.option( + "--prompt-file", + default=None, + type=click.Path(path_type=Path, allow_dash=True), + help=_PROMPT_FILE_HELP, +) +@click.option( + "--preview", + is_flag=True, + help="Show what the repair would run and print its confirmation token. " + "Starts nothing.", +) +@click.option( + "--confirm", + default=None, + metavar="TOKEN", + help="Run the repair, only if it is still exactly what the preview showed. " + "Pass the same options as the preview.", +) +@click.option( + "--cold", + is_flag=True, + help="Start this member's provider without its earlier conversation. Its stored " + "session id is dropped; other members' are kept.", +) +@click.option( + "--repo", default=".", type=click.Path(path_type=Path), help=_PROJECT_REPO_HELP +) +@click.option( + "--skip-lesson", + "skip_lesson", + multiple=True, + metavar="ID", + help="Leave this approved project lesson out of this repair (repeatable). " + "A resumed conversation keeps what earlier rounds sent it.", +) +@click.option( + "--no-lessons", + is_flag=True, + help="Run this repair without any approved project lessons. A resumed " + "conversation keeps what earlier rounds sent it.", +) +def room_repair( + candidate_id: str, + prompt: str | None, + prompt_file: Path | None, + preview: bool, + confirm: str | None, + cold: bool, + repo: Path, + skip_lesson: tuple[str, ...] = (), + no_lessons: bool = False, +) -> None: + """Run only the member that produced CANDIDATE_ID, from that candidate. + + The result is a new candidate, verified again, that you compare and select + yourself. Other members, their candidates and the source are not touched.""" + if preview == (confirm is not None): + raise click.UsageError("room repair needs exactly one of --preview and --confirm") + prompt = _resolve_prompt(prompt, prompt_file, required=True) + _, config, coordinator = _room_coordinator(repo) + try: + parent = coordinator.store.get_candidate(candidate_id) + room_id = str(parent["room_id"]) + lessons = _show_repair_lessons( + config, + coordinator, + room_id, + str(parent["member_id"]), + list(skip_lesson), + no_lessons, + cold=cold, + ) + plan = coordinator.build_repair_plan( + candidate_id, str(prompt), cold=cold, lessons=lessons + ) + if preview: + _print_repair_plan(plan) + return + if plan.token != confirm: + raise _MultilineError( + "This repair is not what that token confirmed: the room, the candidate, " + "the session, the settings or these options changed.", + "Run it with --preview again.", + ) + room = coordinator.run_round(room_id, plan.prompt, lessons=lessons, target=plan) + echo(f"Room {_shown(room_id)}: {_shown(room['state'])}") + number = int(room["round_count"]) + for row in coordinator.store.list_candidates(room_id): + if row.get("round_number") != number: + continue + echo( + f"{_shown(row['id'])} {_shown(row['member_id']):10} " + f"{_shown(row['state']):9} eligible:{bool(row['eligible'])}" + ) + if row["state"] != "completed": + echo( + f"{candidate_id} is still repairable: preview it again, with --cold " + "if the provider could not resume its conversation." + ) + echo(f"Review: syncmill room compare {_shown(room_id)} --round {number}") + except (RoomError, WorkspaceError, ProjectError, OSError, ValueError) as exc: + raise click.ClickException(str(exc)) from exc + finally: + coordinator.close() + + +def _print_repair_plan(plan: RepairPlan) -> None: + echo(f"Repair candidate {_shown(plan.candidate_id)} of room {_shown(plan.room_id)}") + echo(f"Runs: {_shown(plan.member_id)} alone, {plan.provider_calls} provider call") + if plan.session_mode == "resume": + echo("Session: resumes the member's stored conversation") + else: + echo("Session: cold; the provider starts without the earlier conversation") + if plan.gate["kind"] == "none": + echo("Verification: none; the result will not be verified") + else: + for command in plan.gate["commands"]: + echo(f"Verification: {_shown(command)}") + echo(f"Prompt: {len(plan.agent_task.encode('utf-8', 'surrogateescape'))} bytes") + echo("Starts from: this candidate's patch. The checkout is rebuilt only if it differs,") + echo(" which loses its ignored files, such as installed dependencies.") + echo(f"Token: {plan.token}") + echo("To run it, repeat this command with --confirm TOKEN in place of --preview.") + + @room_group.command("candidates") @click.argument("room_id") @click.option( @@ -4253,6 +4383,41 @@ def _show_round_lessons( return tuple(RoundLesson(lesson.id, lesson.text) for lesson in used) +def _show_repair_lessons( + config: SyncmillConfig, + coordinator: RoomCoordinator, + room_id: str, + member_id: str, + skip: list[str], + no_lessons: bool, + *, + cold: bool, +) -> tuple[RoundLesson, ...]: + """The approved lessons a repair sends to its one member. No fit check here: + leaving a set out would change what the repair runs, so the plan refuses a + set that does not fit.""" + used: list[Decision] = [] + if config.board.enabled: + used = _show_lessons( + config, + coordinator.project_id, + skip, + no_lessons, + into="this member's prompt", + goal=coordinator.store.get_room(room_id).get("goal"), + ) + if coordinator.resumes_with_lesson_history( + room_id, fresh_session=cold, member_id=member_id + ): + secho( + "[syncmill] note: a resumed conversation may still hold lessons sent in " + "earlier rounds; --cold starts it without that history", + fg="yellow", + err=True, + ) + return tuple(RoundLesson(lesson.id, lesson.text) for lesson in used) + + def _show_lessons( config: SyncmillConfig, project: str, diff --git a/src/syncmill/room_compare.py b/src/syncmill/room_compare.py index 270c9687..6140f9db 100644 --- a/src/syncmill/room_compare.py +++ b/src/syncmill/room_compare.py @@ -44,6 +44,7 @@ RoomStore, SelectionCheck, round_conversation_status, + round_expected_members, round_provenance_status, room_outcome_status, selection_record_status, @@ -179,15 +180,17 @@ class LineageView: #: recorded | not_recorded | malformed status: str reason: str | None = None - #: the lineage the user asked for: continue-own | fresh | branch-from + #: the lineage the user asked for: continue-own | fresh | branch-from | repair requested: str | None = None - #: snapshot | branch | continued + #: snapshot | branch | continued | repair tree: str | None = None + #: branch: the candidate seeded into every member; repair: the target's parent. base_candidate_id: str | None = None base_patch_sha256: str | None = None base_round_number: int | None = None - #: continued only: member id -> (parent candidate id, parent round), or - #: None when the previous round held no single candidate for the member. + #: continued: member id -> (parent candidate id, parent round), or None when + #: the previous round held no single candidate for the member. repair: the + #: target's entry alone. parents: dict[str, tuple[str, int] | None] = field(default_factory=dict) @@ -755,13 +758,18 @@ def _provenance_views( ) for member_id, parent in tree["members"].items() } + repair = tree["kind"] == "repair" + if repair: + parents = { + tree["member_id"]: (tree["parent_candidate_id"], tree["parent_round_number"]) + } lineage = LineageView( "recorded", requested=record["requested_lineage"], tree=tree["kind"], - base_candidate_id=tree.get("base_candidate_id"), - base_patch_sha256=tree.get("base_patch_sha256"), - base_round_number=tree.get("base_round_number"), + base_candidate_id=tree["parent_candidate_id"] if repair else tree.get("base_candidate_id"), + base_patch_sha256=tree["parent_patch_sha256"] if repair else tree.get("base_patch_sha256"), + base_round_number=tree["parent_round_number"] if repair else tree.get("base_round_number"), parents=parents, ) gate = record["gate"] @@ -951,12 +959,13 @@ def _missing_members( round_row: dict[str, Any], rows: list[CandidateRow], now: float, + provenance: ProvenanceCheck, ) -> tuple[list[MissingMember], list[str]]: warnings: list[str] = [] members = room.get("members") expected: list[tuple[str, str]] = [] if isinstance(members, list): - for member in members: + for member in round_expected_members(members, provenance): if isinstance(member, dict) and isinstance(member.get("member_id"), str): provider = member.get("provider_id") expected.append( @@ -971,6 +980,14 @@ def _missing_members( for m in store.list_round_members(str(round_row["id"])) } have = {r.member_id for r in rows} + if provenance.record is not None and provenance.record["tree"]["kind"] == "repair": + target = provenance.record["tree"]["member_id"] + strays = sorted((have | set(progress)) - {target}) + if strays: + warnings.append( + f"this repair of {_p(target)} also holds records for " + + ", ".join(_p(member_id) for member_id in strays) + ) lease = round_row.get("lease_expires") lease_live = isinstance(lease, (int, float)) and lease > now missing: list[MissingMember] = [] @@ -1107,25 +1124,25 @@ def build_room_comparison( ] try: profile, warnings = _gate_profile(rows) - lineage, settings, provenance_warnings = _provenance_views( - round_provenance_status( - round_row.get("provenance_json"), - round_number=int(round_row["number"]), - member_ids=member_ids, - candidates=candidate_rows, - ), - rows, + provenance = round_provenance_status( + round_row.get("provenance_json"), + round_number=int(round_row["number"]), + member_ids=member_ids, + candidates=candidate_rows, ) + lineage, settings, provenance_warnings = _provenance_views(provenance, rows) conversation, conversation_warnings = _conversation_view( round_conversation_status( round_row.get("conversation_json"), round_number=int(round_row["number"]), - members=members if isinstance(members, list) else [], + members=round_expected_members( + members if isinstance(members, list) else [], provenance + ), candidates=candidate_rows, ) ) missing, member_warnings = _missing_members( - store, room, round_row, rows, now + store, room, round_row, rows, now, provenance ) groups.append( RoundGroup( @@ -1404,6 +1421,13 @@ def starts_from(group: RoundGroup, row: CandidateRow) -> dict[str, Any] | None: "base_candidate_id": lineage.base_candidate_id, "base_patch_sha256": lineage.base_patch_sha256, } + if lineage.tree == "repair": + return { + "kind": "repair", + "parent_candidate_id": lineage.base_candidate_id, + "parent_patch_sha256": lineage.base_patch_sha256, + "parent_round_number": lineage.base_round_number, + } return {"kind": "snapshot"} @@ -1562,17 +1586,29 @@ def starting_tree_label(group: RoundGroup) -> str: "each member continues its own retained worktree; " "a common starting tree is not established" + requested ) + if lineage.tree == "repair": + members = ", ".join(_p(member_id) for member_id in lineage.parents) + return ( + f"candidate {_p(lineage.base_candidate_id)}'s patch " + f"(round {lineage.base_round_number}, sha256 {_p(lineage.base_patch_sha256)}), " + f"for {members} alone" + requested + ) return "snapshot" + requested def row_start_label(group: RoundGroup, row: CandidateRow) -> str | None: """Where one candidate's worktree started, when that differs per candidate (a round that continued each member's own worktree); else None.""" - if group.lineage.status != "recorded" or group.lineage.tree != "continued": + if group.lineage.status != "recorded" or group.lineage.tree not in { + "continued", + "repair", + }: return None parent = group.lineage.parents.get(row.member_id) if parent is None: return "parent not recorded" + if group.lineage.tree == "repair": + return f"a repair of candidate {_p(parent[0])} (round {parent[1]})" return f"the worktree left by candidate {_p(parent[0])} (round {parent[1]})" @@ -2120,7 +2156,10 @@ def render_round_table( patch += " large" files = "? files" if row.files_changed is None else f"{len(row.files_changed)} files" start = [] - if group.lineage.status == "recorded" and group.lineage.tree == "continued": + if group.lineage.status == "recorded" and group.lineage.tree in { + "continued", + "repair", + }: parent = group.lineage.parents.get(row.member_id) start = [ " from: parent not recorded" diff --git a/src/syncmill/rooms.py b/src/syncmill/rooms.py index dde58fd4..83f81c51 100644 --- a/src/syncmill/rooms.py +++ b/src/syncmill/rooms.py @@ -303,6 +303,110 @@ class RoundLesson: text: str +@dataclasses.dataclass(frozen=True) +class RepairPlan: + """One ``room repair``, fixed by its preview: which member runs, from which + candidate, with which settings. Its token is the digest of + :meth:`document`, so a confirmation proceeds only when a plan rebuilt at + that moment is identical. The bodies stay out of the document; the task's + digest covers them.""" + + room_id: str + revision: int + candidate_id: str + patch_sha256: str + output_sha256: str + parent_round_number: int + member_id: str + provider_id: str + agent_settings_sha256: str + conversations_enabled: bool + cold: bool + #: resume | cold + session_mode: str + session_sha256: str | None + prompt: str + agent_task: str + agent_task_sha256: str + lesson_ids: tuple[str, ...] + gate: dict[str, Any] + #: Each entry binds a request's id, record digest and status. Requests + #: arrive with the feedback store, so a plan holds none yet. + feedback: tuple[dict[str, str], ...] = () + provider_calls: int = 1 + + def document(self) -> dict[str, Any]: + return { + "v": 1, + "room": self.room_id, + "revision": self.revision, + "candidate": self.candidate_id, + "patch_sha256": self.patch_sha256, + "output_sha256": self.output_sha256, + "parent_round": self.parent_round_number, + "member": self.member_id, + "provider": self.provider_id, + "agent_settings_sha256": self.agent_settings_sha256, + "conversations_enabled": self.conversations_enabled, + "cold": self.cold, + "session_mode": self.session_mode, + "session_sha256": self.session_sha256, + "agent_task_sha256": self.agent_task_sha256, + "lessons": list(self.lesson_ids), + "gate": self.gate, + "feedback": list(self.feedback), + "provider_calls": self.provider_calls, + } + + @property + def token(self) -> str: + canonical = json.dumps( + self.document(), sort_keys=True, separators=(",", ":"), ensure_ascii=True + ) + return sha256(canonical.encode("ascii")).hexdigest() + + +def _text_sha256(value: str) -> str: + return sha256(value.encode("utf-8", errors="surrogateescape")).hexdigest() + + +def render_repair_task(candidate_id: str, prompt: str) -> str: + """What a repair asks of its member. Deterministic, because the plan binds + its digest and a preview and its confirmation must agree on the bytes.""" + return ( + f"Repair your candidate {candidate_id}. Your working tree holds exactly " + "that candidate's patch. Ignored files, such as installed dependencies, " + f"may be missing.\n\n{prompt}" + ) + + +_GITLINK_HEADER = re.compile( + rb"^(?:(?:new file mode|deleted file mode|old mode|new mode) 160000" + rb"|index [0-9a-f]+\.\.[0-9a-f]+ 160000)$", + re.MULTILINE, +) + + +def patch_has_gitlink(patch: bytes) -> bool: + """Whether a captured patch records an embedded repository. A patch holds + only the commit id of one, so it cannot rebuild that repository's files.""" + return _GITLINK_HEADER.search(patch) is not None + + +def _gate_settings_record(gate: GateConfig) -> dict[str, Any]: + """The ``gate`` part of a round's provenance record.""" + if not gate.commands: + return {"kind": "none"} + return { + "kind": "commands", + "commands": list(gate.commands), + "timeout_seconds": gate.timeout_seconds, + "env_allowlist": ( + None if gate.env_allowlist is None else list(gate.env_allowlist) + ), + } + + def _canonical_uuid(value: object) -> bool: if not isinstance(value, str): return False @@ -662,13 +766,28 @@ def outcome(status: str, reason: str | None = None, **extra: Any) -> OutcomeChec _PROVENANCE_SCHEMA_VERSION = 2 -_PROVENANCE_READABLE_VERSIONS = frozenset({1, 2}) -ROUND_LINEAGES = frozenset({"continue-own", "fresh", "branch-from"}) +#: Written only by a repair round, which names one member and one parent. +#: All-member rounds keep writing v2, whatever came before them in the room. +_PROVENANCE_REPAIR_SCHEMA_VERSION = 3 +_PROVENANCE_READABLE_VERSIONS = frozenset({1, 2, 3}) +ROUND_LINEAGES = frozenset({"continue-own", "fresh", "branch-from", "repair"}) _LINEAGE_TREES = { "fresh": frozenset({"snapshot"}), "branch-from": frozenset({"branch"}), "continue-own": frozenset({"snapshot", "continued"}), + "repair": frozenset({"repair"}), } +_REPAIR_TREE_KEYS = frozenset( + { + "kind", + "member_id", + "parent_candidate_id", + "parent_patch_sha256", + "parent_round_number", + "feedback_ids", + "agent_task_sha256", + } +) _HEX = frozenset("0123456789abcdef") @@ -724,6 +843,8 @@ def malformed(reason: str) -> ProvenanceCheck: lineage = record["requested_lineage"] if not isinstance(lineage, str) or lineage not in ROUND_LINEAGES: return malformed(f"provenance lineage {lineage!r} is unknown") + if (lineage == "repair") != (version == _PROVENANCE_REPAIR_SCHEMA_VERSION): + return malformed(f"lineage {lineage} cannot be recorded in version {version}") gate = record["gate"] if not isinstance(gate, dict): @@ -763,7 +884,12 @@ def malformed(reason: str) -> ProvenanceCheck: kind = tree.get("kind") # A str check first: an unhashable kind ([] / {}) must read malformed, # never raise out of a validator its readers expect to return. - if not isinstance(kind, str) or kind not in {"snapshot", "branch", "continued"}: + if not isinstance(kind, str) or kind not in { + "snapshot", + "branch", + "continued", + "repair", + }: return malformed(f"provenance tree kind {kind!r} is unknown") if kind not in _LINEAGE_TREES[lineage]: return malformed(f"tree {kind} cannot result from lineage {lineage}") @@ -789,6 +915,10 @@ def malformed(reason: str) -> ProvenanceCheck: digest = tree["base_patch_sha256"] if not _sha256_hex(digest) or digest != base.get("patch_sha256"): return malformed("branch base digest differs from the candidate's") + elif kind == "repair": + reason = _repair_tree_problem(tree, round_number, member_ids, candidates) + if reason is not None: + return malformed(reason) else: if set(tree) != {"kind", "members"}: return malformed("provenance tree keys are not the v1 set") @@ -821,6 +951,69 @@ def malformed(reason: str) -> ProvenanceCheck: return ProvenanceCheck("recorded", record=record) +def _repair_tree_problem( + tree: dict[str, Any], + round_number: int, + member_ids: list[str], + candidates: dict[str, dict[str, Any]], +) -> str | None: + """Why a repair tree cannot be believed, or ``None``. It must name a member + of the room and that member's newest completed candidate as of this round.""" + if set(tree) != _REPAIR_TREE_KEYS: + return "provenance tree keys are not the v3 repair set" + member_id = tree["member_id"] + if not isinstance(member_id, str) or member_id not in member_ids: + return "repair member is not a member of this room" + parent_id = tree["parent_candidate_id"] + parent = candidates.get(parent_id) if isinstance(parent_id, str) else None + if parent is None or parent.get("member_id") != member_id: + return "repair parent is not that member's candidate" + if parent.get("state") != "completed": + return "repair parent is not a completed candidate" + parent_round = tree["parent_round_number"] + if not _plain_int(parent_round) or parent_round != parent.get("round_number"): + return "repair parent round differs from the candidate's" + if parent_round >= round_number: + return "repair parent is not from an earlier round" + digest = tree["parent_patch_sha256"] + if not _sha256_hex(digest) or digest != parent.get("patch_sha256"): + return "repair parent digest differs from the candidate's" + for row in candidates.values(): + number = row.get("round_number") + if ( + row.get("member_id") == member_id + and row.get("state") == "completed" + and _plain_int(number) + and parent_round < number < round_number + ): + return "repair parent is not that member's newest completed candidate" + feedback = tree["feedback_ids"] + if not isinstance(feedback, list) or not all(_canonical_uuid(f) for f in feedback): + return "repair feedback ids are not canonical uuids" + if len(set(feedback)) != len(feedback): + return "repair feedback ids list a request twice" + if not _sha256_hex(tree["agent_task_sha256"]): + return "repair task digest is malformed" + return None + + +def round_expected_members( + members: list[dict[str, Any]], provenance: ProvenanceCheck +) -> list[dict[str, Any]]: + """The members a round was meant to run: the target alone for a recorded + repair, the room's members otherwise. A malformed record keeps the room's + members, so its damage stays visible as missing candidates.""" + record = provenance.record + if ( + provenance.status != "recorded" + or record is None + or record["tree"]["kind"] != "repair" + ): + return members + target = record["tree"]["member_id"] + return [m for m in members if isinstance(m, dict) and m.get("member_id") == target] + + def verify_settings_notice(check: ProvenanceCheck, gate: GateConfig) -> str | None: """Why ``room apply --verify`` cannot claim the round's gate settings. @@ -1786,6 +1979,7 @@ def start_round( lease_timeout_seconds: int, max_rounds: int, provenance: dict[str, Any] | None = None, + repair_guard: RepairPlan | None = None, ) -> dict[str, Any]: """Insert the next round and claim its lease. @@ -1794,6 +1988,10 @@ def start_round( read inside this transaction and written by the same INSERT, so a round never exists without the record its writer meant. ``None`` leaves the legacy NULL (direct store callers); the coordinator always passes one. + + ``repair_guard`` is a confirmed repair plan. What can change between its + confirmation and this claim is derived again here, and any difference + refuses before a round exists. """ now = _now() round_id = str(uuid4()) @@ -1816,6 +2014,10 @@ def start_round( self._event(conn, room_id, "room.stale", {}) stale = True else: + if repair_guard is not None: + problem = self._repair_guard_problem(conn, row, repair_guard) + if problem is not None: + raise RoomError(problem) encoded: str | None = None if provenance is not None: encoded = _encode_provenance(provenance) @@ -1873,6 +2075,105 @@ def start_round( self._conn.execute("SELECT * FROM rounds WHERE id=?", (round_id,)).fetchone() ) + def repair_parent_problem(self, room_id: str, candidate_id: str) -> str | None: + """Why this candidate cannot be repaired now, or ``None``.""" + row = self._conn.execute( + "SELECT * FROM rooms WHERE id=?", (room_id,) + ).fetchone() + if row is None: + return f"unknown room: {room_id}" + return self._repair_parent_problem(self._conn, row, candidate_id) + + @classmethod + def _repair_parent_problem( + cls, conn: sqlite3.Connection, room: sqlite3.Row, candidate_id: str + ) -> str | None: + state = room["state"] + if state == "selected": + return ( + "a candidate is already selected, so this room cannot be repaired; " + "`room close`, then `room create`" + ) + if state != "awaiting_decision": + return f"room cannot be repaired from {state}" + if room["recovery_path"] or room["workspace_owner"]: + return "room has workspace recovery pending; recover or close it" + rows = cls._candidate_rows(conn, room["id"]) + parent = rows.get(candidate_id) + if parent is None: + return "candidate does not belong to this room" + if parent["state"] != "completed": + return ( + f"candidate ended {parent['state']}, so it holds no finished work to " + "repair; only a completed candidate can be repaired" + ) + number = parent["round_number"] + if not _plain_int(number): + return "candidate's round is not recorded" + newer = [ + value + for value in rows.values() + if value["member_id"] == parent["member_id"] + and value["state"] == "completed" + and _plain_int(value["round_number"]) + and value["round_number"] > number + ] + if newer: + newest = max(newer, key=lambda value: value["round_number"]) + return ( + f"candidate {newest['id']} is its member's newer completed candidate; " + "repair that one, or use `room round --lineage branch-from` for this one" + ) + workspace = conn.execute( + "SELECT base_commit FROM room_workspaces WHERE room_id=? AND member_id=?", + (room["id"], parent["member_id"]), + ).fetchone() + if workspace is None: + return "the candidate's member has no retained workspace; `room create` a new room" + if parent["base_commit"] != workspace["base_commit"]: + return ( + "the member's workspace moved to another snapshot after this candidate; " + "use `room round --lineage branch-from` for this one" + ) + return None + + @classmethod + def _repair_guard_problem( + cls, conn: sqlite3.Connection, room: sqlite3.Row, plan: RepairPlan + ) -> str | None: + again = "preview the repair again" + if int(room["revision"]) != plan.revision: + return f"the room changed since this repair was confirmed; {again}" + problem = cls._repair_parent_problem(conn, room, plan.candidate_id) + if problem is not None: + return problem + parent = cls._candidate_rows(conn, room["id"])[plan.candidate_id] + if ( + parent["patch_sha256"] != plan.patch_sha256 + or parent["output_sha256"] != plan.output_sha256 + or parent["member_id"] != plan.member_id + ): + return f"the candidate differs from the one confirmed; {again}" + stored = cls._stored_session(conn, room, plan.member_id) + if (None if stored is None else _text_sha256(stored)) != plan.session_sha256: + return f"the member's provider session changed; {again}" + return None + + @staticmethod + def _stored_session( + conn: sqlite3.Connection, room: sqlite3.Row, member_id: str + ) -> str | None: + """One member's stored session id, read as :meth:`member_sessions` does: + the live workspace row wins over the durable copy.""" + row = conn.execute( + "SELECT provider_session_id FROM room_workspaces WHERE room_id=? AND member_id=?", + (room["id"], member_id), + ).fetchone() + live = row["provider_session_id"] if row is not None else None + if isinstance(live, str) and live: + return live + return _parse_sessions_json(room["sessions_json"]).get(member_id) + def record_round_conversation( self, room_id: str, @@ -1891,18 +2192,29 @@ def record_round_conversation( encoded = json.dumps(record, sort_keys=True, separators=(",", ":")) with self._tx() as conn: round_row = conn.execute( - "SELECT room_id, number FROM rounds WHERE id=?", (round_id,) + "SELECT room_id, number, provenance_json FROM rounds WHERE id=?", + (round_id,), ).fetchone() if round_row is None or round_row["room_id"] != room_id: raise RoomError(f"round {round_id} is not a round of room {room_id}") room = conn.execute( "SELECT members_json FROM rooms WHERE id=?", (room_id,) ).fetchone() + members = json.loads(room["members_json"]) + candidates = self._candidate_rows(conn, room_id) check = round_conversation_status( encoded, round_number=int(round_row["number"]), - members=json.loads(room["members_json"]), - candidates=self._candidate_rows(conn, room_id), + members=round_expected_members( + members, + round_provenance_status( + round_row["provenance_json"], + round_number=int(round_row["number"]), + member_ids=[m["member_id"] for m in members], + candidates=candidates, + ), + ), + candidates=candidates, ) if check.status != "recorded": raise RoomError(f"invalid round conversation record: {check.reason}") @@ -3516,25 +3828,40 @@ def resumes_with_lesson_history( *, lineage: str = "continue-own", fresh_session: bool = False, + member_id: str | None = None, ) -> bool: """Whether a round launched now would resume at least one member's provider conversation in a room where an earlier round selected approved lessons (#174). Such a conversation may still hold lessons this round leaves out: exclusion stops new injection, it cannot retract what a conversation was sent. Read-only and as of now; it - names no lesson and claims nothing about what a conversation holds.""" + names no lesson and claims nothing about what a conversation holds. + ``member_id`` asks about that member's conversation alone (a repair).""" if fresh_session or lineage == "fresh" or not self.config.conversations.enabled: return False room = self.store.get_room(room_id) sessions = self.store.member_sessions(room_id) - if not any(sessions.get(member["member_id"]) for member in room["members"]): + if not any( + sessions.get(member["member_id"]) + for member in room["members"] + if member_id is None or member["member_id"] == member_id + ): return False candidates = self.store.candidate_rows(room_id) + member_ids = [member["member_id"] for member in room["members"]] for row in self.store.list_rounds(room_id): check = round_conversation_status( row.get("conversation_json"), round_number=int(row["number"]), - members=room["members"], + members=round_expected_members( + room["members"], + round_provenance_status( + row.get("provenance_json"), + round_number=int(row["number"]), + member_ids=member_ids, + candidates=candidates, + ), + ), candidates=candidates, ) if ( @@ -3545,6 +3872,76 @@ def resumes_with_lesson_history( return True return False + def build_repair_plan( + self, + candidate_id: str, + prompt: str, + *, + cold: bool = False, + lessons: list[RoundLesson] | tuple[RoundLesson, ...] | None = None, + ) -> RepairPlan: + """The plan for repairing one candidate as of now. Read-only: it starts + nothing, and a repair that could only fail after the round's lease is + refused here instead, so such a repair cannot freeze the room.""" + room_id = str(self.store.get_candidate(candidate_id)["room_id"]) + problem = self.store.repair_parent_problem(room_id, candidate_id) + if problem is not None: + raise RoomError(problem) + if not prompt.strip(): + raise RoomError("a repair needs a prompt saying what to change") + room = self.preflight_round(room_id, prompt) + parent = self.store.candidate_rows(room_id)[candidate_id] + member_id = parent["member_id"] + try: + member = next(m for m in room["members"] if m["member_id"] == member_id) + except StopIteration: + raise RoomError("candidate's member is not a member of this room") from None + provider_id = member["provider_id"] + if provider_id not in {agent.agent_id for agent in self.agents}: + raise RoomError(f"room provider is no longer enabled: {provider_id}") + self.store.verify_candidate_artifact(candidate_id) + artifact = self.store.load_candidate(candidate_id) + if patch_has_gitlink(artifact["patch"] or b""): + raise RoomError( + "candidate's patch records an embedded repository, which a rebuild " + "cannot restore, so it cannot be repaired" + ) + agent_task = render_repair_task(candidate_id, prompt) + # round_lessons_fit answers True when there are no lessons, and an + # oversized task would otherwise first fail after the lease is held. + _validate_prompt_text(agent_task) + selected = round_lessons(lessons) + if not round_lessons_fit(agent_task, [lesson.text for lesson in selected]): + raise RoomError( + "the selected lessons do not fit beside this repair's prompt; " + "leave some out with --skip-lesson or --no-lessons" + ) + conversations = self.config.conversations.enabled + stored = self.store.member_sessions(room_id).get(member_id) + settings = self.config.agent_config(provider_id).model_dump(mode="json") + return RepairPlan( + room_id=room_id, + revision=int(room["revision"]), + candidate_id=candidate_id, + patch_sha256=parent["patch_sha256"], + output_sha256=parent["output_sha256"], + parent_round_number=int(parent["round_number"]), + member_id=member_id, + provider_id=provider_id, + agent_settings_sha256=_text_sha256( + json.dumps(settings, sort_keys=True, separators=(",", ":")) + ), + conversations_enabled=conversations, + cold=cold, + session_mode="resume" if conversations and stored and not cold else "cold", + session_sha256=None if stored is None else _text_sha256(stored), + prompt=prompt, + agent_task=agent_task, + agent_task_sha256=_text_sha256(agent_task), + lesson_ids=tuple(lesson.id for lesson in selected), + gate=_gate_settings_record(self.config.gate), + ) + def _continued_parents(self, room: dict[str, Any]) -> dict[str, dict[str, Any] | None]: """Each member's candidate in the previous round: the attempt whose retained worktree a continue-own round continues. ``None`` when that @@ -3574,12 +3971,29 @@ def run_round( fresh_session: bool = False, fork: bool = False, lessons: list[RoundLesson] | tuple[RoundLesson, ...] | None = None, + target: RepairPlan | None = None, ) -> dict[str, Any]: """Run one round. ``lessons`` are the approved project lessons the launcher showed and chose (#174); the coordinator never opens the board. They go to every member this round, after the checkpoint and - before the task, and the round's conversation record names them.""" + before the task, and the round's conversation record names them. + + ``target`` makes this a repair: one member runs, from one candidate of + its own, as its confirmed plan says. Without it every member runs.""" selected_lessons = round_lessons(lessons) + if target is not None: + if ( + lineage != "continue-own" + or base_candidate_id + or fresh_session + or fork + or prompt != target.prompt + ): + raise RoomError("a repair takes its prompt and session from its plan") + if tuple(lesson.id for lesson in selected_lessons) != target.lesson_ids: + raise RoomError("a repair's lessons differ from its plan") + if target.room_id != room_id: + raise RoomError("a repair's plan names another room") room = self.preflight_round( room_id, prompt, @@ -3633,8 +4047,23 @@ def run_round( fresh_session or lineage == "fresh" or not self.config.conversations.enabled + or (target is not None and target.session_mode == "cold") ) seed_sessions = {} if cold_sessions else self.store.member_sessions(room_id) + parent_patch: str | None = None + if target is not None: + # Peers' stored sessions are not this round's: only the target runs. + seed_sessions = { + member_id: session + for member_id, session in seed_sessions.items() + if member_id == target.member_id + } + if not existing: + raise RoomError("the room has no retained workspace to repair") + artifact = self.store.load_candidate(target.candidate_id) + parent_patch = (artifact["patch"] or b"").decode( + "utf-8", errors="surrogateescape" + ) # One private copy of the gate settings: it is what the round records # AND what build_gate receives after the fan-out, so the two cannot # diverge. @@ -3645,7 +4074,10 @@ def run_round( tree: dict[str, Any] if lineage == "continue-own" and existing: _, validated = self._validated_workspace_records( - room_id, existing, require_live=True + room_id, + existing, + require_live=True, + live_members=None if target is None else {target.member_id}, ) retained_worktrees = { value["member_id"]: Worktree( @@ -3659,10 +4091,22 @@ def run_round( } source_fingerprint = room["source_fingerprint"] snapshot_commit = room["snapshot_commit"] - tree = { - "kind": "continued", - "members": self._continued_parents(room), - } + tree = ( + { + "kind": "continued", + "members": self._continued_parents(room), + } + if target is None + else { + "kind": "repair", + "member_id": target.member_id, + "parent_candidate_id": target.candidate_id, + "parent_patch_sha256": target.patch_sha256, + "parent_round_number": target.parent_round_number, + "feedback_ids": [item["id"] for item in target.feedback], + "agent_task_sha256": target.agent_task_sha256, + } + ) else: seed_patch: bytes | None = None tree = {"kind": "snapshot"} @@ -3719,24 +4163,16 @@ def run_round( lease_timeout_seconds=self.config.rooms.lease_timeout_seconds, max_rounds=self.config.rooms.max_rounds, provenance={ - "schema_version": _PROVENANCE_SCHEMA_VERSION, - "requested_lineage": lineage, - "tree": tree, - "gate": ( - { - "kind": "commands", - "commands": list(gate_config.commands), - "timeout_seconds": gate_config.timeout_seconds, - "env_allowlist": ( - None - if gate_config.env_allowlist is None - else list(gate_config.env_allowlist) - ), - } - if gate_config.commands - else {"kind": "none"} + "schema_version": ( + _PROVENANCE_SCHEMA_VERSION + if target is None + else _PROVENANCE_REPAIR_SCHEMA_VERSION ), + "requested_lineage": lineage if target is None else "repair", + "tree": tree, + "gate": _gate_settings_record(gate_config), }, + repair_guard=target, ) except BaseException: if snapshot is not None: @@ -3754,7 +4190,14 @@ def run_round( # those rounds. A successful cold turn repopulates them below. if fresh_session or lineage == "fresh": self.store.clear_member_sessions(room_id) - members = room["members"] + if target is not None and target.cold: + # One member's, never clear_member_sessions: peers sat this out. + self.store.clear_member_session(room_id, target.member_id) + members = ( + room["members"] + if target is None + else [m for m in room["members"] if m["member_id"] == target.member_id] + ) by_id = {agent.agent_id: agent for agent in self.agents} try: selected_agents = [by_id[member["provider_id"]] for member in members] @@ -3786,6 +4229,19 @@ def run_round( existing = self.store.list_workspaces(room_id) else: worktrees = [retained_worktrees[member["member_id"]] for member in members] + if target is not None and parent_patch is not None: + # A failed attempt since the parent can have left changes here. + self._keep_candidate_tree( + room_id, + target.member_id, + worktrees[0], + parent_patch, + existing, + reason="the worktree differed from the repair's parent", + ) + # The text every member is asked to act on. A repair's stays out of + # rounds.prompt, which `apply` shares to memory. + round_task = round_prompt if target is None else target.agent_task # Per-member prompts: a RESUMING member's provider conversation # already holds its own prior outputs and every candidate published # before its last round's prompt was built — re-sending them is pure @@ -3807,13 +4263,15 @@ def run_round( # round, strictly BEFORE the fan-out (the shared-loop rule), and # apply it only to the AGENT prompts (the stored rounds.prompt row # keeps the user-authored text). - memory_context = self._memory_context(room_id, round_prompt) + memory_context = self._memory_context( + room_id, round_prompt if target is None else room["prompt"] + ) # The lessons are member-independent and so is their fit: only # the task can leave too little room (excerpts and memory make way # for them). The launcher's preview ran the same check; a set that # still does not fit is recorded as dropped, never cut mid-text. lesson_texts = [lesson.text for lesson in selected_lessons] - if round_lessons_fit(round_prompt, lesson_texts): + if round_lessons_fit(round_task, lesson_texts): delivered_lessons = lesson_texts lesson_delivery = "included" if lesson_texts else "none" else: @@ -3828,14 +4286,14 @@ def run_round( if seeded and last_seen is not None: agent_prompt, included, omitted = self._checkpoint( room_id, - round_prompt, + round_task, exclude_member=member_id, min_round_number=last_seen, lessons=delivered_lessons, ) else: agent_prompt, included, omitted = self._checkpoint( - room_id, round_prompt, lessons=delivered_lessons + room_id, round_task, lessons=delivered_lessons ) composed, delivered = self._memory_prefixed(memory_context, agent_prompt) # Final-argv guarantee: the checkpoint embeds candidate OUTPUTS @@ -4006,13 +4464,16 @@ def _keep_candidate_tree( worktree: Worktree, patch: str, workspaces: list[dict[str, Any]], + *, + reason: str = "the gate changed the worktree", ) -> None: - """Keep what a gate wrote out of this member's later rounds (#268). + """Make this member's worktree hold exactly ``patch``. The worktree outlives the candidate, so a file the gate left would enter - the next round's patch. Only a tree the gate actually changed is rebuilt, - which costs its ignored files. A failure propagates to the round's - recovery path, so a tree that may still hold gate output is never reused. + the next round's patch (#268), and a repair must start from its parent. + Only a tree that differs is rebuilt, which costs its ignored files. A + failure propagates to the round's recovery path, so a tree that may hold + something else is never reused. """ try: unchanged = capture_changes(worktree.path, worktree.base_commit).patch == patch @@ -4031,7 +4492,7 @@ def _keep_candidate_tree( self.store.record_event( room_id, "workspace.rebuilt", - {"member_id": member_id, "reason": "the gate changed the worktree"}, + {"member_id": member_id, "reason": reason}, ) def _memory_context(self, room_id: str, prompt: str) -> str: @@ -4261,11 +4722,16 @@ def _validated_workspace_records( values: list[dict[str, Any]], *, require_live: bool, + live_members: set[str] | None = None, ) -> tuple[dict[str, Any], list[tuple[dict[str, Any], Path, Path]]]: + """Validate workspace records. ``live_members`` narrows ``require_live`` + to those members, so a repair is not blocked by a peer's damaged + workspace; the next all-member round still fails on it.""" room = self.store.get_room(room_id) members = {member["member_id"]: member for member in room["members"]} observed_members = {value["member_id"] for value in values} - if (require_live and observed_members != set(members)) or not ( + must_be_live = set(members) if live_members is None else live_members + if (require_live and not must_be_live <= observed_members) or not ( observed_members <= set(members) ): raise RoomError("room workspace metadata does not match room members") @@ -4283,7 +4749,7 @@ def _validated_workspace_records( or value["provider_id"] != member["provider_id"] ): raise RoomError("room workspace metadata contains unsafe paths") - if require_live: + if require_live and value["member_id"] in must_be_live: control_git = control_repo / ".git" self._resolved_owned_path(control_git) git_marker = worktree_path / ".git" diff --git a/tests/test_room_repair.py b/tests/test_room_repair.py new file mode 100644 index 00000000..10ace2d8 --- /dev/null +++ b/tests/test_room_repair.py @@ -0,0 +1,958 @@ +"""Targeted repair: one member runs, from one of its own candidates.""" + +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest +from click.testing import CliRunner + +from conftest import FakeMemory +from syncmill.agents import _PROVIDER_SESSIONS +from syncmill.cli import cli +from syncmill.config import SyncmillConfig +from syncmill.models import TaskResult, TaskStatus +from syncmill.room_compare import ( + build_room_comparison, + comparison_to_dict, + row_start_label, + starting_tree_label, + starts_from, +) +from syncmill.rooms import ( + RoomCoordinator, + RoomError, + RoundLesson, + patch_has_gitlink, + render_repair_task, + round_conversation_status, + round_expected_members, + round_provenance_status, +) +from syncmill.worktree import capture_changes + +from test_rooms import WritingAgent + + +LESSON_ID = "11111111-1111-4111-8111-111111111111" + + +def _config(tmp_path: Path, **overrides) -> SyncmillConfig: + return SyncmillConfig( + project={"id": "repair-tests"}, + runs={"root": str(tmp_path / "runs")}, + workspace={"root": str(tmp_path / "workspaces")}, + rooms={"enabled": True, "max_members": 3, "max_parallel": 3}, + **overrides, + ) + + +class Member(WritingAgent): + """Writes ``.txt`` naming its call number, and reports a new session id + per call. ``on_call(number, workdir)`` runs before the write; ``fail_calls`` + end as a failed attempt that still leaves the write behind.""" + + def __init__(self, agent_id: str, on_call=None, fail_calls=(), raise_calls=()): + super().__init__(agent_id) + self.scopes: list[dict] = [] + self.on_call = on_call + self.fail_calls = set(fail_calls) + self.raise_calls = set(raise_calls) + + async def aexecute(self, prompt: str, workdir: Path, timeout: int) -> TaskResult: + state = _PROVIDER_SESSIONS.get() + number = len(self.scopes) + 1 + self.scopes.append( + {"seed": dict(state["seed"]) if state else None, "prompt": prompt, "workdir": workdir} + ) + if number in self.raise_calls: + raise RuntimeError("the worker went away") + if self.on_call is not None: + self.on_call(number, workdir) + (workdir / f"{self.agent_id}.txt").write_text(f"call {number}\n", encoding="utf-8") + failed = number in self.fail_calls + self._status = TaskStatus.FAILED if failed else TaskStatus.COMPLETED + self._error = "scripted failure" if failed else None + self._exit_code = 1 if failed else 0 + result = self.execute(prompt, workdir, timeout) + result.provider_session_id = None if failed else f"{self.agent_id}-sid-{number}" + return result + + +class Harness: + def __init__(self, git_repo: Path, tmp_path: Path, agents: list[Member], **config): + self.repo = git_repo + self.agents = {agent.agent_id: agent for agent in agents} + self.config = _config(tmp_path, **config) + self.coordinator = RoomCoordinator(self.config, git_repo, agents=agents) + self.store = self.coordinator.store + self.room_id = self.coordinator.create("the room prompt", [a.agent_id for a in agents])["id"] + + def round(self, prompt: str = "a round", **kwargs) -> dict: + return self.coordinator.run_round(self.room_id, prompt, **kwargs) + + def candidate(self, member: str, round_number: int | None = None) -> dict: + rows = [ + row + for row in self.store.list_candidates(self.room_id) + if row["member_id"] == member + and (round_number is None or row["round_number"] == round_number) + ] + return max(rows, key=lambda row: row["round_number"]) + + def workspace(self, member: str) -> dict: + return next( + value + for value in self.store.list_workspaces(self.room_id) + if value["member_id"] == member + ) + + def worktree(self, member: str) -> Path: + return Path(self.workspace(member)["path"]) + + def live_patch(self, member: str) -> str: + value = self.workspace(member) + return capture_changes(Path(value["path"]), value["base_commit"]).patch + + def repair(self, candidate_id: str, prompt: str = "fix it", *, cold=False, lessons=None): + plan = self.coordinator.build_repair_plan(candidate_id, prompt, cold=cold, lessons=lessons) + room = self.coordinator.run_round( + self.room_id, plan.prompt, lessons=lessons, target=plan + ) + return plan, room + + def rounds(self) -> list[dict]: + return self.store.list_rounds(self.room_id) + + def provenance(self, row: dict): + room = self.store.get_room(self.room_id) + return round_provenance_status( + row["provenance_json"], + round_number=int(row["number"]), + member_ids=[m["member_id"] for m in room["members"]], + candidates=self.store.candidate_rows(self.room_id), + ) + + def events(self, kind: str) -> list[dict]: + return [e for e in self.store.events(self.room_id) if e["kind"] == kind] + + def close(self) -> None: + self.coordinator.close() + + +@pytest.fixture +def make(git_repo, tmp_path): + made: list[Harness] = [] + + def build(*agents: Member, **config) -> Harness: + harness = Harness(git_repo, tmp_path, list(agents), **config) + made.append(harness) + return harness + + yield build + for harness in made: + harness.close() + + +def _both(): + return Member("claude"), Member("codex") + + +# ------------------------------------------------------------------ one member + + +def test_a_repair_runs_only_its_member_and_leaves_its_peer_untouched(make): + claude, codex = _both() + room = make(claude, codex) + room.round() + peer = room.candidate("codex") + peer_dir = Path(peer["artifact_path"]) + peer_bytes = {p.name: p.read_bytes() for p in peer_dir.iterdir() if p.is_file()} + peer_status = room.live_patch("codex") + parent = room.candidate("claude") + + plan, state = room.repair(parent["id"], "make it better") + + assert state["state"] == "awaiting_decision" + assert len(claude.scopes) == 2 + assert len(codex.scopes) == 1 + new = room.candidate("claude") + assert new["id"] != parent["id"] and new["round_number"] == 2 + assert [c["round_number"] for c in room.store.list_candidates(room.room_id) if c["member_id"] == "codex"] == [1] + assert {p.name: p.read_bytes() for p in peer_dir.iterdir() if p.is_file()} == peer_bytes + assert room.live_patch("codex") == peer_status + assert room.workspace("codex")["provider_session_id"] == "codex-sid-1" + assert room.workspace("claude")["provider_session_id"] == "claude-sid-2" + # The original candidate stays as it was, and selectable by its own row. + assert room.store.get_candidate(parent["id"])["patch_sha256"] == parent["patch_sha256"] + + +def test_a_repair_records_its_target_and_parent(make): + room = make(*_both()) + room.round() + parent = room.candidate("claude") + plan, _ = room.repair(parent["id"], "make it better") + + row = room.rounds()[-1] + assert row["prompt"] == "make it better" + check = room.provenance(row) + assert check.status == "recorded", check.reason + assert check.record["schema_version"] == 3 + assert check.record["requested_lineage"] == "repair" + assert check.record["tree"] == { + "kind": "repair", + "member_id": "claude", + "parent_candidate_id": parent["id"], + "parent_patch_sha256": parent["patch_sha256"], + "parent_round_number": 1, + "feedback_ids": [], + "agent_task_sha256": plan.agent_task_sha256, + } + room_row = room.store.get_room(room.room_id) + conversation = round_conversation_status( + row["conversation_json"], + round_number=2, + members=round_expected_members(room_row["members"], check), + candidates=room.store.candidate_rows(room.room_id), + ) + assert conversation.status == "recorded", conversation.reason + assert set(conversation.record["members"]) == {"claude"} + assert [m["member_id"] for m in room.store.list_round_members(row["id"])] == ["claude"] + + +def test_a_repair_hands_its_member_the_task_and_resumes_its_session_alone(make): + claude, codex = _both() + room = make(claude, codex) + room.round() + parent = room.candidate("claude") + room.repair(parent["id"], "make it better") + + scope = claude.scopes[-1] + assert scope["seed"] == {"claude": "claude-sid-1"} + assert render_repair_task(parent["id"], "make it better") in scope["prompt"] + # Resuming: its own output is in the conversation, its peer's is new. + assert "result from codex" in scope["prompt"] + + +def test_a_cold_repair_clears_only_its_members_session(make): + claude, codex = _both() + room = make(claude, codex) + room.round() + plan, _ = room.repair(room.candidate("claude")["id"], cold=True) + + assert plan.session_mode == "cold" + assert claude.scopes[-1]["seed"] == {} + assert room.workspace("codex")["provider_session_id"] == "codex-sid-1" + assert room.store.member_sessions(room.room_id)["codex"] == "codex-sid-1" + assert room.events("sessions.cleared") == [] + + +def test_with_conversations_off_only_cold_drops_the_stored_session(make): + claude = Member("claude", fail_calls={2, 3}) + room = make(claude) + room.round() + room.coordinator.config.conversations.enabled = False + parent = room.candidate("claude")["id"] + + without = room.coordinator.build_repair_plan(parent, "fix it") + with_cold = room.coordinator.build_repair_plan(parent, "fix it", cold=True) + assert without.session_mode == with_cold.session_mode == "cold" + assert without.session_sha256 == with_cold.session_sha256 + assert without.token != with_cold.token + + # Failed attempts report no session, so what remains is what the run left. + room.coordinator.run_round(room.room_id, "fix it", target=without) + assert room.store.member_sessions(room.room_id) == {"claude": "claude-sid-1"} + with_cold = room.coordinator.build_repair_plan(parent, "fix it", cold=True) + room.coordinator.run_round(room.room_id, "fix it", target=with_cold) + assert room.store.member_sessions(room.room_id) == {} + + +# --------------------------------------------------------------- starting tree + + +def test_a_tree_that_matches_its_parent_is_kept_with_its_ignored_files(make): + def write_ignored(number, workdir): + if number == 1: + (workdir / ".gitignore").write_text(".venv/\n", encoding="utf-8") + (workdir / ".venv").mkdir() + (workdir / ".venv" / "dep").write_text("dep\n", encoding="utf-8") + + seen = {} + + def observe(number, workdir): + write_ignored(number, workdir) + if number == 2: + seen["dep"] = (workdir / ".venv" / "dep").exists() + + room = make(Member("claude", on_call=observe)) + room.round() + room.repair(room.candidate("claude")["id"]) + + assert seen["dep"] is True + assert room.events("workspace.rebuilt") == [] + + +def test_a_failed_attempts_changes_are_gone_when_the_repair_is_retried(make): + seen = {} + room_box = {} + + def observe(number, workdir): + if number == 2: + (workdir / "partial.txt").write_text("half done\n", encoding="utf-8") + if number == 3: + seen["partial"] = (workdir / "partial.txt").exists() + seen["patch"] = capture_changes( + workdir, room_box["room"].workspace("claude")["base_commit"] + ).patch + + claude = Member("claude", on_call=observe, fail_calls={2}) + room = make(claude) + room_box["room"] = room + room.round() + parent = room.candidate("claude") + parent_patch = room.store.load_candidate(parent["id"])["patch"] + + _, state = room.repair(parent["id"]) + assert state["state"] == "awaiting_decision" + assert room.candidate("claude", 2)["state"] == "failed" + assert room.candidate("claude", 2)["patch_sha256"] != parent["patch_sha256"] + + # The failed attempt did not disqualify the parent. + room.repair(parent["id"]) + + assert seen["partial"] is False + assert seen["patch"].encode("utf-8", "surrogateescape") == parent_patch + assert [e["payload"]["reason"] for e in room.events("workspace.rebuilt")] == [ + "the worktree differed from the repair's parent" + ] + assert room.candidate("claude", 3)["state"] == "completed" + + +def test_a_gate_that_writes_a_file_does_not_reach_the_repair(make): + seen = {} + room_box = {} + + def observe(number, workdir): + if number == 2: + seen["patch"] = capture_changes( + workdir, room_box["room"].workspace("claude")["base_commit"] + ).patch + + room = make( + Member("claude", on_call=observe), + gate={"commands": ["python3 -c \"open('gate-debris.txt','w').write('x')\""]}, + ) + room_box["room"] = room + room.round() + parent = room.candidate("claude") + room.repair(parent["id"]) + + assert "gate-debris" not in seen["patch"] + assert "gate-debris" not in room.store.load_candidate(room.candidate("claude")["id"])["patch"].decode() + + +def test_a_repair_can_start_from_an_empty_patch(make): + class Idle(Member): + async def aexecute(self, prompt, workdir, timeout): + self.scopes.append({"seed": None, "prompt": prompt, "workdir": workdir}) + result = self.execute(prompt, workdir, timeout) + result.provider_session_id = None + return result + + idle = Idle("claude") + room = make(idle) + room.round() + parent = room.candidate("claude") + assert not room.store.load_candidate(parent["id"])["patch"] + room.repair(parent["id"]) + assert room.candidate("claude")["round_number"] == 2 + + +@pytest.mark.parametrize( + "line,expected", + [ + (b"new file mode 160000", True), + (b"deleted file mode 160000", True), + (b"old mode 160000", True), + (b"new mode 160000", True), + (b"index abc123..def456 160000", True), + (b"new file mode 100644", False), + (b"index abc123..def456 100644", False), + (b"+new file mode 160000", False), + (b" index abc123..def456 160000", False), + ], +) +def test_a_gitlink_is_read_from_the_patch_headers(line, expected): + assert patch_has_gitlink(b"diff --git a/x b/x\n" + line + b"\n") is expected + + +def test_a_candidate_holding_an_embedded_repository_is_refused(make): + def embed(number, workdir): + inner = workdir / "vendored" + inner.mkdir() + for args in ( + ["init", "-q"], + ["-c", "user.email=t@t.test", "-c", "user.name=T", "commit", "-q", "--allow-empty", "-m", "x"], + ): + subprocess.run(["git", *args], cwd=inner, check=True, capture_output=True) + + room = make(Member("claude", on_call=embed)) + room.round() + parent = room.candidate("claude") + assert patch_has_gitlink(room.store.load_candidate(parent["id"])["patch"]) + before = room.store.get_room(room.room_id)["revision"] + + with pytest.raises(RoomError, match="embedded repository"): + room.coordinator.build_repair_plan(parent["id"], "fix it") + assert room.store.get_room(room.room_id)["revision"] == before + + +def test_a_damaged_peer_workspace_does_not_block_a_repair(make): + room = make(*_both()) + room.round() + shutil.rmtree(room.worktree("codex")) + + room.repair(room.candidate("claude")["id"]) + assert room.candidate("claude")["round_number"] == 2 + + with pytest.raises(RoomError, match="missing or invalid"): + room.round() + + +# ------------------------------------------------------------------- refusals + + +def _no_change(room: Harness, revision: int, rounds: int) -> None: + assert room.store.get_room(room.room_id)["revision"] == revision + assert len(room.rounds()) == rounds + + +@pytest.mark.parametrize("prompt", ["", " \n"]) +def test_a_repair_with_nothing_to_do_is_refused(make, prompt): + room = make(Member("claude")) + room.round() + with pytest.raises(RoomError, match="needs a prompt"): + room.coordinator.build_repair_plan(room.candidate("claude")["id"], prompt) + + +def test_a_prompt_that_only_overflows_as_a_repair_is_refused_at_the_preview(make): + room = make(Member("claude")) + room.round() + parent = room.candidate("claude") + state = room.store.get_room(room.room_id) + from syncmill.rooms import _ROOM_PROMPT_MAX_BYTES + + # Fits as a prompt on its own, not with the repair's opening lines. + prompt = "x" * (_ROOM_PROMPT_MAX_BYTES - 10) + room.coordinator.preflight_round(room.room_id, prompt) + with pytest.raises(RoomError, match="exceeds"): + room.coordinator.build_repair_plan(parent["id"], prompt) + _no_change(room, state["revision"], 1) + assert room.store.get_room(room.room_id)["state"] == "awaiting_decision" + + +def test_lessons_that_do_not_fit_are_refused_not_dropped(make): + room = make(Member("claude")) + room.round() + from syncmill.rooms import _ROOM_PROMPT_MAX_BYTES + + lesson = RoundLesson(LESSON_ID, "x" * _ROOM_PROMPT_MAX_BYTES) + with pytest.raises(RoomError, match="lessons do not fit"): + room.coordinator.build_repair_plan( + room.candidate("claude")["id"], "fix it", lessons=[lesson] + ) + + +def test_a_failed_candidate_is_refused(make): + room = make(Member("claude", fail_calls={1})) + room.round() + with pytest.raises(RoomError, match="only a completed candidate"): + room.coordinator.build_repair_plan(room.candidate("claude")["id"], "fix it") + + +def test_a_candidate_with_a_newer_completed_one_is_refused(make): + room = make(Member("claude")) + room.round() + older = room.candidate("claude") + room.round("again") + newer = room.candidate("claude") + with pytest.raises(RoomError, match=f"{newer['id']}.*newer completed"): + room.coordinator.build_repair_plan(older["id"], "fix it") + + +def test_a_selected_room_is_refused_with_what_to_do(make): + room = make(Member("claude")) + room.round() + parent = room.candidate("claude") + room.store.select_candidate(room.room_id, parent["id"]) + with pytest.raises(RoomError, match="room close"): + room.coordinator.build_repair_plan(parent["id"], "fix it") + + +def test_a_disabled_provider_is_refused_before_any_state_changes(make): + room = make(Member("claude")) + room.round() + room.coordinator.agents = [] + with pytest.raises(RoomError, match="no longer enabled"): + room.coordinator.build_repair_plan(room.candidate("claude")["id"], "fix it") + + +def test_a_damaged_artifact_is_refused(make): + room = make(Member("claude")) + room.round() + parent = room.candidate("claude") + patch = Path(parent["artifact_path"]) / "candidate.patch" + patch.write_bytes(patch.read_bytes() + b"tampered\n") + with pytest.raises(RoomError): + room.coordinator.build_repair_plan(parent["id"], "fix it") + + +def test_max_rounds_bounds_repeated_repair(make): + room = make(Member("claude")) + room.coordinator.config.rooms.max_rounds = 2 + room.round() + room.repair(room.candidate("claude")["id"]) + with pytest.raises(RoomError, match="max_rounds=2"): + room.coordinator.build_repair_plan(room.candidate("claude")["id"], "fix it") + + +# ----------------------------------------------------------------- the token + + +def _token_of(room: Harness, candidate_id: str, **kwargs) -> str: + return room.coordinator.build_repair_plan(candidate_id, kwargs.pop("prompt", "fix it"), **kwargs).token + + +def test_every_bound_field_changes_the_token(make): + room = make(Member("claude")) + room.round() + parent = room.candidate("claude")["id"] + base = _token_of(room, parent) + assert base == _token_of(room, parent), "a preview is deterministic" + + tokens = { + "prompt": _token_of(room, parent, prompt="fix it differently"), + "cold": _token_of(room, parent, cold=True), + "lessons": _token_of(room, parent, lessons=[RoundLesson(LESSON_ID, "a lesson")]), + } + room.coordinator.config.claude.timeout_seconds += 1 + tokens["agent settings"] = _token_of(room, parent) + room.coordinator.config.claude.timeout_seconds -= 1 + room.coordinator.config.gate.commands = ["true"] + tokens["gate"] = _token_of(room, parent) + room.coordinator.config.gate.commands = [] + room.coordinator.config.conversations.enabled = False + tokens["conversations"] = _token_of(room, parent) + room.coordinator.config.conversations.enabled = True + room.store.update_workspace_session(room.room_id, "claude", "another-session") + tokens["session"] = _token_of(room, parent) + + assert base not in tokens.values() + assert len(set(tokens.values())) == len(tokens) + + +def test_a_used_plan_is_refused_the_second_time(make): + room = make(Member("claude")) + room.round() + parent = room.candidate("claude") + plan = room.coordinator.build_repair_plan(parent["id"], "fix it") + room.coordinator.run_round(room.room_id, plan.prompt, target=plan) + rounds = len(room.rounds()) + + with pytest.raises(RoomError, match="changed since this repair was confirmed"): + room.coordinator.run_round(room.room_id, plan.prompt, target=plan) + assert len(room.rounds()) == rounds + + +def test_a_session_change_between_confirm_and_claim_starts_nothing(make): + claude = Member("claude") + room = make(claude) + room.round() + plan = room.coordinator.build_repair_plan(room.candidate("claude")["id"], "fix it") + room.store.update_workspace_session(room.room_id, "claude", "another-session") + state = room.store.get_room(room.room_id) + + with pytest.raises(RoomError, match="provider session changed"): + room.coordinator.run_round(room.room_id, plan.prompt, target=plan) + + _no_change(room, state["revision"], 1) + assert len(claude.scopes) == 1 + assert room.store.get_room(room.room_id)["state"] == "awaiting_decision" + + +def test_a_plan_cannot_be_mixed_with_other_round_options(make): + room = make(Member("claude")) + room.round() + plan = room.coordinator.build_repair_plan(room.candidate("claude")["id"], "fix it") + for kwargs in ( + {"lineage": "fresh"}, + {"fresh_session": True}, + {"fork": True}, + {"base_candidate_id": room.candidate("claude")["id"]}, + ): + with pytest.raises(RoomError, match="from its plan"): + room.coordinator.run_round(room.room_id, plan.prompt, target=plan, **kwargs) + with pytest.raises(RoomError, match="from its plan"): + room.coordinator.run_round(room.room_id, "another prompt", target=plan) + with pytest.raises(RoomError, match="lessons differ"): + room.coordinator.run_round( + room.room_id, plan.prompt, target=plan, lessons=[RoundLesson(LESSON_ID, "x")] + ) + + +# ------------------------------------------------------------- what comes next + + +def test_the_next_all_member_round_keeps_writing_v2_and_peers_see_the_repair(make): + claude, codex = _both() + room = make(claude, codex) + room.round() + room.repair(room.candidate("claude")["id"], "make it better") + repaired = room.candidate("claude") + + room.round("everyone again") + + row = room.rounds()[-1] + check = room.provenance(row) + assert check.status == "recorded", check.reason + assert check.record["schema_version"] == 2 + assert check.record["tree"]["members"] == { + "claude": {"parent_candidate_id": repaired["id"], "parent_round_number": 2}, + "codex": None, + } + assert "result from claude" in codex.scopes[-1]["prompt"] + assert set(codex.scopes[-1]["seed"]) == {"claude", "codex"} + room_row = room.store.get_room(room.room_id) + conversation = round_conversation_status( + row["conversation_json"], + round_number=3, + members=round_expected_members(room_row["members"], check), + candidates=room.store.candidate_rows(room.room_id), + ) + assert conversation.status == "recorded", conversation.reason + + +def test_a_repair_candidate_can_itself_be_selected_and_repaired(make): + room = make(Member("claude")) + room.round() + room.repair(room.candidate("claude")["id"], "first fix") + second = room.candidate("claude") + room.repair(second["id"], "second fix") + third = room.candidate("claude") + assert third["round_number"] == 3 + state = room.store.select_candidate(room.room_id, third["id"]) + assert state["selected_candidate_id"] == third["id"] + + +def test_memory_is_searched_with_the_rooms_prompt(git_repo, tmp_path): + class RecordingMemory(FakeMemory): + def __init__(self): + super().__init__() + self.queries: list[str] = [] + + async def search_context(self, query, *args, **kwargs): + self.queries.append(query) + return await super().search_context(query, *args, **kwargs) + + memory = RecordingMemory() + agent = Member("claude") + coordinator = RoomCoordinator(_config(tmp_path), git_repo, agents=[agent], memory=memory) + try: + room_id = coordinator.create("the room prompt", ["claude"])["id"] + coordinator.run_round(room_id, "a round") + parent = coordinator.store.list_candidates(room_id)[0] + plan = coordinator.build_repair_plan(parent["id"], "the repair prompt") + before = len(memory.queries) + coordinator.run_round(room_id, plan.prompt, target=plan) + assert memory.queries[before:] == ["the room prompt"] + finally: + coordinator.close() + + +def test_a_worker_failure_during_a_repair_freezes_the_room_with_the_parent_readable(make): + room = make(Member("claude", raise_calls={2})) + room.round() + parent = room.candidate("claude") + plan = room.coordinator.build_repair_plan(parent["id"], "fix it") + + with pytest.raises(RuntimeError, match="the worker went away"): + room.coordinator.run_round(room.room_id, plan.prompt, target=plan) + + assert room.store.get_room(room.room_id)["state"] == "recovery_required" + assert room.store.load_candidate(parent["id"])["patch"] + with pytest.raises(RoomError): + room.coordinator.build_repair_plan(parent["id"], "fix it") + + +def test_source_drift_after_the_preview_stales_the_room_and_starts_nothing(make): + claude = Member("claude") + room = make(claude) + room.round() + plan = room.coordinator.build_repair_plan(room.candidate("claude")["id"], "fix it") + (room.repo / "README.md").write_text("changed\n", encoding="utf-8") + + with pytest.raises(RoomError, match="create a new room"): + room.coordinator.run_round(room.room_id, plan.prompt, target=plan) + + assert room.store.get_room(room.room_id)["state"] == "stale" + assert len(claude.scopes) == 1 + + +def test_a_killed_repair_waits_for_its_lease_and_then_recovers(make): + room = make(Member("claude")) + room.round() + parent = room.candidate("claude") + plan = room.coordinator.build_repair_plan(parent["id"], "fix it") + state = room.store.get_room(room.room_id) + # What a killed worker leaves: the round claimed, nothing after it. + row = room.store.start_round( + room.room_id, + prompt=plan.prompt, + source_fingerprint=state["source_fingerprint"], + snapshot_commit=state["snapshot_commit"], + lease_owner="99999999-dead", + lease_timeout_seconds=900, + max_rounds=20, + repair_guard=plan, + ) + assert room.store.get_room(room.room_id)["state"] == "running" + with pytest.raises(RoomError): + room.coordinator.close_room(room.room_id) + + room.store._conn.execute("UPDATE rounds SET lease_expires=1 WHERE id=?", (row["id"],)) + room.store._conn.commit() + assert room.room_id in room.coordinator.recover(room.room_id) + assert room.store.get_room(room.room_id)["state"] == "recovery_required" + assert room.store.load_candidate(parent["id"])["patch"] + + +def test_a_one_entry_conversation_on_an_all_member_round_stays_malformed(make): + room = make(*_both()) + room.round() + row = room.rounds()[-1] + record = json.loads(row["conversation_json"]) + del record["members"]["codex"] + room_row = room.store.get_room(room.room_id) + check = round_conversation_status( + json.dumps(record), + round_number=1, + members=round_expected_members(room_row["members"], room.provenance(row)), + candidates=room.store.candidate_rows(room.room_id), + ) + assert check.status == "malformed" + + +# ---------------------------------------------------------------- read models + + +def _comparison(room: Harness, number: int): + return build_room_comparison( + room.store, room.room_id, round_number=number, source_check=lambda recorded: True + ) + + +def test_compare_expects_only_the_target_in_a_repair_round(make): + room = make(*_both()) + room.round() + parent = room.candidate("claude") + room.repair(parent["id"]) + + group = _comparison(room, 2).groups[0] + assert group.missing == () + assert group.warnings == () + assert group.lineage.tree == "repair" + assert group.lineage.requested == "repair" + assert group.conversation.status == "recorded" + assert starts_from(group, group.rows[0]) == { + "kind": "repair", + "parent_candidate_id": parent["id"], + "parent_patch_sha256": parent["patch_sha256"], + "parent_round_number": 1, + } + label = starting_tree_label(group) + assert parent["id"] in label and "for claude alone" in label + assert "requested lineage: repair" in label + assert row_start_label(group, group.rows[0]) == f"a repair of candidate {parent['id']} (round 1)" + document = comparison_to_dict(_comparison(room, 2)) + assert document["schema_version"] == 1 + assert document["rounds"][0]["lineage"]["members"] == { + "claude": {"parent_candidate_id": parent["id"], "parent_round_number": 1} + } + assert "fix it" not in json.dumps(document) + + +def test_a_damaged_repair_record_keeps_every_member_expected(make): + room = make(*_both()) + room.round() + room.repair(room.candidate("claude")["id"]) + row = room.rounds()[-1] + record = json.loads(row["provenance_json"]) + record["tree"]["member_id"] = "someone-else" + room.store._conn.execute( + "UPDATE rounds SET provenance_json=? WHERE id=?", (json.dumps(record), row["id"]) + ) + room.store._conn.commit() + + group = _comparison(room, 2).groups[0] + assert [m.member_id for m in group.missing] == ["codex"] + assert group.lineage.status == "malformed" + + +def test_a_repair_round_holding_another_members_row_warns(make): + room = make(*_both()) + room.round() + room.repair(room.candidate("claude")["id"]) + row = room.rounds()[-1] + room.store.init_round_members( + room.room_id, row["id"], [{"member_id": "codex", "provider_id": "codex"}] + ) + group = _comparison(room, 2).groups[0] + assert any("also holds records for codex" in warning for warning in group.warnings) + + +def test_a_repair_in_progress_expects_only_its_member(make): + seen = {} + room_box = {} + + def observe(number, workdir): + if number == 2: + seen["group"] = _comparison(room_box["room"], 2).groups[0] + + room = make(Member("claude", on_call=observe), Member("codex")) + room_box["room"] = room + room.round() + room.repair(room.candidate("claude")["id"]) + + group = seen["group"] + assert [(m.member_id, m.status) for m in group.missing] == [("claude", "in_progress")] + + +def test_lesson_history_is_asked_about_the_target_alone(make): + claude, codex = _both() + room = make(claude, codex) + room.round(lessons=[RoundLesson(LESSON_ID, "a lesson")]) + assert room.coordinator.resumes_with_lesson_history(room.room_id, member_id="claude") + room.store.clear_member_session(room.room_id, "claude") + assert not room.coordinator.resumes_with_lesson_history(room.room_id, member_id="claude") + assert room.coordinator.resumes_with_lesson_history(room.room_id, member_id="codex") + + +# ------------------------------------------------------------------------ CLI + + +@pytest.fixture +def cli_room(monkeypatch, git_repo, tmp_path): + claude, codex = _both() + config = _config(tmp_path) + agents = [claude, codex] + + def build() -> RoomCoordinator: + return RoomCoordinator(config, git_repo, agents=agents) + + monkeypatch.setattr("syncmill.cli._room_coordinator", lambda repo: (git_repo, config, build())) + coordinator = build() + room_id = coordinator.create("the room prompt", ["claude", "codex"])["id"] + coordinator.run_round(room_id, "a round") + parent = next( + c for c in coordinator.store.list_candidates(room_id) if c["member_id"] == "claude" + ) + yield SimpleNamespace( + coordinator=coordinator, room_id=room_id, parent=parent, claude=claude, codex=codex, config=config + ) + coordinator.close() + + +def _invoke(*args: str): + return CliRunner().invoke(cli, ["room", "repair", *args]) + + +def _token(output: str) -> str: + return next(line.split()[1] for line in output.splitlines() if line.startswith("Token:")) + + +@pytest.mark.parametrize( + "flags", [[], ["--preview", "--confirm", "abc"]], ids=["neither", "both"] +) +def test_the_cli_needs_exactly_one_of_preview_and_confirm(cli_room, flags): + res = _invoke(cli_room.parent["id"], "--prompt", "fix it", *flags) + assert res.exit_code == 2 + assert "exactly one of --preview and --confirm" in res.output + assert len(cli_room.claude.scopes) == 1 + + +def test_the_preview_starts_nothing_and_says_what_would_run(cli_room): + state = cli_room.coordinator.store.get_room(cli_room.room_id) + res = _invoke(cli_room.parent["id"], "--prompt", "fix it", "--preview") + assert res.exit_code == 0, res.output + for expected in ( + "claude alone, 1 provider call", + "resumes the member's stored conversation", + "none; the result will not be verified", + "ignored files", + "--confirm TOKEN", + ): + assert expected in res.output + assert len(cli_room.claude.scopes) == 1 + after = cli_room.coordinator.store.get_room(cli_room.room_id) + assert after["revision"] == state["revision"] + assert after["round_count"] == state["round_count"] + + +def test_the_preview_says_when_the_provider_starts_cold(cli_room): + res = _invoke(cli_room.parent["id"], "--prompt", "fix it", "--preview", "--cold") + assert "starts without the earlier conversation" in res.output + + +def test_confirm_runs_exactly_what_the_preview_showed(cli_room): + token = _token(_invoke(cli_room.parent["id"], "--prompt", "fix it", "--preview").output) + res = _invoke(cli_room.parent["id"], "--prompt", "fix it", "--confirm", token) + assert res.exit_code == 0, res.output + assert len(cli_room.claude.scopes) == 2 + assert len(cli_room.codex.scopes) == 1 + assert f"room compare {cli_room.room_id} --round 2" in res.output + + again = _invoke(cli_room.parent["id"], "--prompt", "fix it", "--confirm", token) + assert again.exit_code == 1 + assert len(cli_room.claude.scopes) == 2 + + +@pytest.mark.parametrize( + "other", + [["--prompt", "another prompt"], ["--prompt", "fix it", "--cold"]], + ids=["prompt", "cold"], +) +def test_confirm_refuses_options_the_preview_did_not_have(cli_room, other): + token = _token(_invoke(cli_room.parent["id"], "--prompt", "fix it", "--preview").output) + res = _invoke(cli_room.parent["id"], *other, "--confirm", token) + assert res.exit_code == 1 + assert "not what that token confirmed" in res.output + assert "--preview again" in res.output + assert len(cli_room.claude.scopes) == 1 + + +def test_lessons_that_do_not_fit_stop_the_cli_before_a_round_exists(cli_room, monkeypatch): + from syncmill.rooms import _ROOM_PROMPT_MAX_BYTES + + cli_room.config.board.enabled = True + monkeypatch.setattr( + "syncmill.cli._show_lessons", + lambda *a, **k: [SimpleNamespace(id=LESSON_ID, text="x" * _ROOM_PROMPT_MAX_BYTES)], + ) + res = _invoke(cli_room.parent["id"], "--prompt", "fix it", "--preview") + assert res.exit_code == 1 + assert "lessons do not fit" in res.output + assert len(cli_room.coordinator.store.list_rounds(cli_room.room_id)) == 1 + + +def test_a_failed_repair_says_the_parent_can_still_be_repaired(cli_room): + cli_room.claude.fail_calls = {2} + token = _token(_invoke(cli_room.parent["id"], "--prompt", "fix it", "--preview").output) + res = _invoke(cli_room.parent["id"], "--prompt", "fix it", "--confirm", token) + assert res.exit_code == 0, res.output + assert "is still repairable" in res.output + assert "--cold" in res.output diff --git a/tests/test_round_provenance.py b/tests/test_round_provenance.py index 6d40b02e..b57d4037 100644 --- a/tests/test_round_provenance.py +++ b/tests/test_round_provenance.py @@ -14,6 +14,7 @@ RoomCoordinator, RoomError, RoomStore, + round_expected_members, round_provenance_status, ) import syncmill.rooms as rooms_module @@ -443,3 +444,146 @@ def test_no_configured_gate_is_recorded_as_none(git_repo, tmp_path): assert _provenance(coordinator, room["id"], 1)["gate"] == {"kind": "none"} finally: coordinator.close() + + +# ----------------------------------------------------------------- repair (v3) + +_REPAIR_ROWS = { + "parent": { + "member_id": "a", + "state": "completed", + "round_number": 1, + "patch_sha256": DIGEST, + "eligible": 1, + }, + "failed-later": {"member_id": "a", "state": "failed", "round_number": 2, "patch_sha256": "b" * 64}, + "peer": {"member_id": "b", "state": "completed", "round_number": 1, "patch_sha256": "c" * 64}, + "not-completed": {"member_id": "a", "state": "timeout", "round_number": 1, "patch_sha256": DIGEST}, +} +_FEEDBACK = "11111111-1111-4111-8111-111111111111" + + +def _repair_tree(**changes) -> dict: + tree = { + "kind": "repair", + "member_id": "a", + "parent_candidate_id": "parent", + "parent_patch_sha256": DIGEST, + "parent_round_number": 1, + "feedback_ids": [], + "agent_task_sha256": "d" * 64, + } + tree.update(changes) + return tree + + +def _repair_check(tree=None, *, version=3, lineage="repair", round_number=3, candidates=None): + return _check( + _record(tree or _repair_tree(), lineage=lineage, version=version), + round_number=round_number, + candidates=_REPAIR_ROWS if candidates is None else candidates, + ) + + +def test_a_repair_record_is_believed_past_a_later_failed_attempt(): + check = _repair_check() + assert check.status == "recorded", check.reason + assert _repair_check(_repair_tree(feedback_ids=[_FEEDBACK])).status == "recorded" + + +def test_a_repair_record_with_a_commands_gate_is_believed(): + gate = {"kind": "commands", "commands": ["true"], "timeout_seconds": 5, "env_allowlist": None} + check = _check( + _record(_repair_tree(), lineage="repair", version=3, gate=gate), + round_number=3, + candidates=_REPAIR_ROWS, + ) + assert check.status == "recorded", check.reason + + +@pytest.mark.parametrize( + "changes", + [ + {"member_id": "stranger"}, + {"member_id": 1}, + {"member_id": "b"}, + {"parent_candidate_id": "unknown"}, + {"parent_candidate_id": ["parent"]}, + {"parent_candidate_id": "peer"}, + {"parent_candidate_id": "not-completed"}, + {"parent_patch_sha256": "e" * 64}, + {"parent_patch_sha256": "short"}, + {"parent_round_number": 2}, + {"parent_round_number": True}, + {"feedback_ids": "x"}, + {"feedback_ids": ["not-a-uuid"]}, + {"feedback_ids": [_FEEDBACK, _FEEDBACK]}, + {"agent_task_sha256": "short"}, + {"agent_task_sha256": None}, + {"extra": 1}, + ], + ids=lambda changes: next(iter(changes)) + "=" + repr(next(iter(changes.values())))[:16], +) +def test_a_malformed_repair_tree_is_not_believed(changes): + check = _repair_check(_repair_tree(**changes)) + assert check.status == "malformed" + + +def test_a_repair_tree_missing_a_key_is_not_believed(): + tree = _repair_tree() + del tree["feedback_ids"] + assert _repair_check(tree).status == "malformed" + + +def test_a_repair_parent_must_come_before_its_round(): + assert _repair_check(round_number=1).status == "malformed" + same_round = {"parent": {**_REPAIR_ROWS["parent"], "round_number": 3}} + check = _repair_check(_repair_tree(parent_round_number=3), candidates=same_round) + assert check.status == "malformed" + assert "earlier round" in check.reason + + +def test_a_repair_parent_must_be_its_members_newest_completed_candidate(): + rows = { + **_REPAIR_ROWS, + "newer": {"member_id": "a", "state": "completed", "round_number": 2, "patch_sha256": "f" * 64}, + } + check = _repair_check(candidates=rows) + assert check.status == "malformed" + assert "newest completed" in check.reason + # A later round's candidate does not change what this round's record meant. + later = { + **_REPAIR_ROWS, + "later": {"member_id": "a", "state": "completed", "round_number": 3, "patch_sha256": "f" * 64}, + } + assert _repair_check(candidates=later).status == "recorded" + + +@pytest.mark.parametrize("version", [1, 2]) +def test_a_repair_cannot_be_recorded_below_version_3(version): + check = _repair_check(version=version) + assert check.status == "malformed" + assert "cannot be recorded" in check.reason + + +@pytest.mark.parametrize("lineage", ["continue-own", "fresh", "branch-from"]) +def test_version_3_is_only_for_a_repair(lineage): + check = _check( + _record({"kind": "snapshot"}, lineage=lineage, version=3), candidates=_REPAIR_ROWS + ) + assert check.status == "malformed" + + +def test_a_repair_tree_cannot_hide_under_another_lineage(): + assert _repair_check(lineage="continue-own").status == "malformed" + assert _repair_check(tree={"kind": "snapshot"}).status == "malformed" + + +def test_expected_members_are_the_target_only_for_a_believed_repair(): + members = [{"member_id": "a", "provider_id": "x"}, {"member_id": "b", "provider_id": "y"}] + assert round_expected_members(members, _repair_check()) == members[:1] + malformed = _repair_check(_repair_tree(member_id="stranger")) + assert round_expected_members(members, malformed) == members + assert round_expected_members(members, _check(None)) == members + all_members = _check(_record({"kind": "snapshot"}), round_number=1) + assert round_expected_members(members, all_members) == members From be7fd9f20a7c4e00342abc6ddf248118fe04c7d3 Mon Sep 17 00:00:00 2001 From: memtomem <118508877+memtomem@users.noreply.github.com> Date: Mon, 9 Nov 2026 20:56:21 +0900 Subject: [PATCH 2/2] fix(tui): keep a repair's prompt out of the next all-member round (#173) The round dialog prefilled the latest round's prompt. After a repair that is an instruction written for one member about its own candidate, and accepting the default sent it to every member. Offer the last all-member round's prompt, and say so. Co-Authored-By: Claude Opus 5.5 --- docs/user-guide.ko.md | 2 +- docs/user-guide.md | 2 +- src/syncmill/rooms.py | 23 +++++++++++++++++++++++ src/syncmill/tui.py | 13 +++++++++---- src/syncmill/tui_collaboration.py | 2 +- tests/test_room_repair.py | 11 +++++++++++ tests/test_tui.py | 4 ++++ tests/test_tui_room_review.py | 27 +++++++++++++++++++++++++++ 8 files changed, 77 insertions(+), 7 deletions(-) diff --git a/docs/user-guide.ko.md b/docs/user-guide.ko.md index 8a83ba26..e6e44a9b 100644 --- a/docs/user-guide.ko.md +++ b/docs/user-guide.ko.md @@ -1019,7 +1019,7 @@ syncmill room repair <후보-id> --prompt "고칠 내용" --confirm <토큰> `--prompt-file`/`-`). 생략하면 방 생성 프롬프트를 재사용하는데 이제 **노란 경고**가 보이고, 실행된 프롬프트 첫 줄이 에코됩니다. `room rounds `는 라운드별 프롬프트 목록을 보여 주고, TUI 방 상세에는 생성 프롬프트가, 후보 Details에는 그 라운드의 -프롬프트가 표시돼요. TUI의 `x`는 이제 라운드 모달(직전 라운드 프롬프트로 미리 채워진 +프롬프트가 표시돼요. TUI의 `x`는 이제 라운드 모달(수리가 아닌 직전 라운드 프롬프트로 미리 채워진 입력 + lineage + base 후보)을 엽니다. 라운드 프롬프트에는 `run`/`board`와 같은 베스트-에포트 공유 메모리 컨텍스트가 주입되고(메모리가 고장나도 라운드는 계속), **적용된** 후보는 `room-result` 태그로 공유 메모리에 기록되어 이후 실행이 참조할 수 diff --git a/docs/user-guide.md b/docs/user-guide.md index 13d44f34..de90f2d8 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -1120,7 +1120,7 @@ prompt **with a visible warning**, and the round output echoes the prompt that actually ran. `room rounds ` lists every round with its prompt, the TUI room detail shows the creation prompt, and the candidate Details view shows the round's prompt. In the TUI, `x` now opens a round modal — prompt (prefilled -with the previous round's), lineage, and base candidate — instead of replaying +with the previous all-member round's), lineage, and base candidate — instead of replaying the creation prompt. Round prompts are also enriched with bounded shared-memory context (the same best-effort enrichment `run`/`board` get — a broken memory backend never blocks a round), and an **applied** candidate is written back to diff --git a/src/syncmill/rooms.py b/src/syncmill/rooms.py index 83f81c51..ae6303aa 100644 --- a/src/syncmill/rooms.py +++ b/src/syncmill/rooms.py @@ -3872,6 +3872,29 @@ def resumes_with_lesson_history( return True return False + def next_round_prompt(self, room_id: str) -> tuple[str, bool]: + """The prompt to offer for the room's next all-member round, and whether + the latest round was a repair and so was passed over: a repair's prompt + was written for one member about its own candidate.""" + room = self.store.get_room(room_id) + rows = self.store.list_rounds(room_id) + candidates = self.store.candidate_rows(room_id) + member_ids = [member["member_id"] for member in room["members"]] + + def is_repair(row: dict[str, Any]) -> bool: + check = round_provenance_status( + row.get("provenance_json"), + round_number=int(row["number"]), + member_ids=member_ids, + candidates=candidates, + ) + return check.record is not None and check.record["tree"]["kind"] == "repair" + + for row in reversed(rows): + if not is_repair(row): + return str(row["prompt"]), row is not rows[-1] + return str(room["prompt"]), bool(rows) + def build_repair_plan( self, candidate_id: str, diff --git a/src/syncmill/tui.py b/src/syncmill/tui.py index 83f00ee5..49dd5e73 100644 --- a/src/syncmill/tui.py +++ b/src/syncmill/tui.py @@ -3063,10 +3063,10 @@ def _confirm_room_round(self) -> None: room_id, store_root = target try: room = self._rooms.store.get_room(room_id) - # Prefill with the PREVIOUS round's prompt (creation prompt on a - # fresh room) — the modal is where per-round instructions land now. - rounds = self._rooms.store.list_rounds(room_id) - default_prompt = rounds[-1]["prompt"] if rounds else room["prompt"] + # Prefill with the PREVIOUS all-member round's prompt (creation + # prompt on a fresh room) — the modal is where per-round + # instructions land now. + default_prompt, passed_over_repair = self._rooms.next_round_prompt(room_id) eligible = [ row for row in self._rooms.store.list_candidates(room_id) @@ -3076,6 +3076,11 @@ def _confirm_room_round(self) -> None: except (*_ROOM_POLL_ERRORS, OSError) as exc: self.notify(str(exc), severity="error") return + if passed_over_repair: + self.notify( + "The last round repaired one member, so this shows the last prompt " + "written for every member." + ) self.push_screen( RoomRoundModal(room, default_prompt, eligible, lessons), lambda fields, room_id=room_id, store_root=store_root, shown=lessons.lessons: ( diff --git a/src/syncmill/tui_collaboration.py b/src/syncmill/tui_collaboration.py index 005c601a..ea61d30d 100644 --- a/src/syncmill/tui_collaboration.py +++ b/src/syncmill/tui_collaboration.py @@ -163,7 +163,7 @@ class RoomRoundModal(_CollabModal): """Per-round prompt + lineage input for the Rooms tab. Replaces the bare confirm the round used to run through: the TextArea is - prefilled with the PREVIOUS round's prompt (creation prompt on a fresh + prefilled with the PREVIOUS all-member round's prompt (creation prompt on a fresh room) so every round can carry a new instruction, and lineage/base-candidate are selectable in place. The review checkbox safety gate is preserved. Dumb like the other modals — dismisses raw strings; parsing/validation diff --git a/tests/test_room_repair.py b/tests/test_room_repair.py index 10ace2d8..d2c0eaa2 100644 --- a/tests/test_room_repair.py +++ b/tests/test_room_repair.py @@ -649,6 +649,17 @@ def test_a_repair_candidate_can_itself_be_selected_and_repaired(make): assert state["selected_candidate_id"] == third["id"] +def test_the_next_rounds_prompt_passes_over_a_repair(make): + room = make(*_both()) + assert room.coordinator.next_round_prompt(room.room_id) == ("the room prompt", False) + room.round("every member") + assert room.coordinator.next_round_prompt(room.room_id) == ("every member", False) + room.repair(room.candidate("claude")["id"], "fix your own file") + assert room.coordinator.next_round_prompt(room.room_id) == ("every member", True) + room.round("every member again") + assert room.coordinator.next_round_prompt(room.room_id) == ("every member again", False) + + def test_memory_is_searched_with_the_rooms_prompt(git_repo, tmp_path): class RecordingMemory(FakeMemory): def __init__(self): diff --git a/tests/test_tui.py b/tests/test_tui.py index 97558516..55aa3039 100644 --- a/tests/test_tui.py +++ b/tests/test_tui.py @@ -3059,6 +3059,10 @@ def preflight_round(self, room_id, prompt, *, lineage, base_candidate_id): self.preflights.append((room_id, prompt, lineage, base_candidate_id)) return {} + def next_round_prompt(self, room_id): + rounds = self.store.list_rounds(room_id) + return (rounds[-1]["prompt"] if rounds else "creation prompt"), False + def resumes_with_lesson_history(self, room_id, *, lineage): self.history_asked.append((room_id, lineage)) value = self.history.get(lineage, False) diff --git a/tests/test_tui_room_review.py b/tests/test_tui_room_review.py index 299eee5c..186318c2 100644 --- a/tests/test_tui_room_review.py +++ b/tests/test_tui_room_review.py @@ -1011,3 +1011,30 @@ async def scenario(): assert "visible words" in shown drive(scenario()) + + +def test_the_round_dialog_does_not_offer_a_repairs_prompt_to_every_member(git_repo, tmp_path): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("the room prompt", ["claude", "codex"]) + coordinator.run_round(room["id"], "for every member") + parent = next( + row for row in coordinator.store.list_candidates(room["id"]) if row["member_id"] == "claude" + ) + plan = coordinator.build_repair_plan(parent["id"], "fix your own file") + coordinator.run_round(room["id"], plan.prompt, target=plan) + + async def scenario(): + app = _app(tmp_path, git_repo, board=None) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + app._confirm_room_round() + await pilot.pause() + assert isinstance(app.screen, RoomRoundModal) + assert app.screen._default_prompt == "for every member" + assert any("repaired one member" in n.message for n in app._notifications) + + asyncio.run(scenario()) + coordinator.close() +