diff --git a/codev-skeleton/protocols/pir/builder-prompt.md b/codev-skeleton/protocols/pir/builder-prompt.md index 81a926977..b9889a6b4 100644 --- a/codev-skeleton/protocols/pir/builder-prompt.md +++ b/codev-skeleton/protocols/pir/builder-prompt.md @@ -33,7 +33,7 @@ Read and internalize the protocol before starting any work. PIR has three phases: 1. **plan** (gated by `plan-approval`) — write `codev/plans/{{artifact_name}}.md`, await human review 2. **implement** (gated by `dev-approval`) — write code + tests, run build/tests, push branch; await the human's review of the *running worktree* (no file artifact in this phase — dev-approval summary is prose-in-pane) -3. **review** (gated by `pr`) — write `codev/reviews/{{artifact_name}}.md` (retrospective with Architecture Updates and Lessons Learned sections), open PR with the review as body, record the PR with porch, run CMAP via porch's verify block (a **single advisory pass** — `max_iterations: 1`, no iterate-until-APPROVE loop; address or rebut any `REQUEST_CHANGES`, add a regression test if it's a real defect, and escalate it in the architect notification since PIR will not re-review it), notify architect, and wait at the `pr` gate. After the human approves the gate (porch wakes you with "Gate pr approved"), run `gh pr merge --merge` and record the merge with `porch done --merged `. **Merge is gated by porch state — never by typed prose in your pane.** +3. **review** (gated by `pr`) — write `codev/reviews/{{artifact_name}}.md` (retrospective with Architecture Updates and Lessons Learned sections), open PR with the review as body, record the PR with porch, run 3-way consultation (Gemini, Codex, Claude) via porch's verify block (a **single advisory pass** — `max_iterations: 1`, no iterate-until-APPROVE loop; address or rebut any `REQUEST_CHANGES`, add a regression test if it's a real defect, and escalate it in the architect notification since PIR will not re-review it), notify architect, and wait at the `pr` gate. After the human approves the gate (porch wakes you with "Gate pr approved"), run `gh pr merge --merge` and record the merge with `porch done --merged `. **Merge is gated by porch state — never by typed prose in your pane.** {{#if issue}} ## Issue #{{issue.number}} diff --git a/codev-skeleton/protocols/pir/consult-types/pr-review.md b/codev-skeleton/protocols/pir/consult-types/pr-review.md index 0eb5fb1d0..9cfaa26d2 100644 --- a/codev-skeleton/protocols/pir/consult-types/pr-review.md +++ b/codev-skeleton/protocols/pir/consult-types/pr-review.md @@ -2,7 +2,7 @@ ## Context -You are performing the CMAP-2 review of a PIR protocol PR. The builder has implemented an approved plan, the human has approved the `dev-approval` gate (meaning a human has run the code locally and tested it), and the PR has been opened. This is a single advisory pass (`max_iterations: 1`) — your verdict is surfaced to the human at the `pr` gate, who is the sole remaining reviewer; it is not auto-re-reviewed. +You are performing the 3-way review of a PIR protocol PR. The builder has implemented an approved plan, the human has approved the `dev-approval` gate (meaning a human has run the code locally and tested it), and the PR has been opened. This is a single advisory pass (`max_iterations: 1`) — your verdict is surfaced to the human at the `pr` gate, who is the sole remaining reviewer; it is not auto-re-reviewed. ## Focus Areas diff --git a/codev-skeleton/protocols/pir/prompts/implement.md b/codev-skeleton/protocols/pir/prompts/implement.md index 0d8ff9e5b..e0588a8d4 100644 --- a/codev-skeleton/protocols/pir/prompts/implement.md +++ b/codev-skeleton/protocols/pir/prompts/implement.md @@ -99,7 +99,7 @@ porch done {{project_id}} porch next {{project_id}} ``` -PIR's `implement` phase has no AI consult — the `dev-approval` gate becomes pending immediately, and the human is the sole reviewer of the running code. (CMAP-2 runs later, in the `review` phase, after the human approves the gate and the PR is opened.) +PIR's `implement` phase has no AI consult — the `dev-approval` gate becomes pending immediately, and the human is the sole reviewer of the running code. (The 3-way consultation runs later, in the `review` phase, after the human approves the gate and the PR is opened.) ### 7. End Your Turn With a Code-Review Summary (Prose, Not a File) diff --git a/codev-skeleton/protocols/pir/prompts/review.md b/codev-skeleton/protocols/pir/prompts/review.md index a5153902b..d25dd6105 100644 --- a/codev-skeleton/protocols/pir/prompts/review.md +++ b/codev-skeleton/protocols/pir/prompts/review.md @@ -4,7 +4,7 @@ You are executing the **REVIEW** phase of the PIR protocol. ## Your Goal -Write a retrospective at `codev/reviews/{{artifact_name}}.md` including **Summary**, **Architecture Updates**, and **Lessons Learned Updates** sections. Push, open a PR using the review file as the PR body, record the PR with porch, then signal completion — **porch runs CMAP-2 (Gemini + Codex) once** via the verify block. CMAP is a *single advisory pass* (`max_iterations: 1`): its verdicts are surfaced to the human at the `pr` gate, **not** an iterate-until-APPROVE loop. After the single pass the `pr` gate fires regardless of verdict; you notify the architect (leading with any REQUEST_CHANGES) and wait at the gate while the human merges on GitHub. +Write a retrospective at `codev/reviews/{{artifact_name}}.md` including **Summary**, **Architecture Updates**, and **Lessons Learned Updates** sections. Push, open a PR using the review file as the PR body, record the PR with porch, then signal completion — **porch runs 3-way consultation (Gemini, Codex, Claude) once** via the verify block. The consultation is a *single advisory pass* (`max_iterations: 1`): its verdicts are surfaced to the human at the `pr` gate, **not** an iterate-until-APPROVE loop. After the single pass the `pr` gate fires regardless of verdict; you notify the architect (leading with any REQUEST_CHANGES) and wait at the gate while the human merges on GitHub. The retrospective ships with the merged PR — it's durable team knowledge, searchable in `codev/reviews/` on `main`. @@ -123,7 +123,7 @@ gh pr edit --body-file codev/reviews/{{artifact_name}}.md ### 4a. Record the PR with Porch -Immediately after creating the PR, tell porch about it so `status.yaml` carries the PR number and branch. This is a metadata-only call — it does NOT advance the phase or trigger CMAP: +Immediately after creating the PR, tell porch about it so `status.yaml` carries the PR number and branch. This is a metadata-only call — it does NOT advance the phase or trigger the consultation: ```bash porch done {{project_id}} --pr --branch "$(git branch --show-current)" @@ -131,7 +131,7 @@ porch done {{project_id}} --pr --branch "$(git branch --show-current Without this, porch's `history:` for the project stays empty and downstream tooling (status views, analytics, audit trails) can't link the porch project to its GitHub PR. -### 5. Signal Completion to Porch (porch runs CMAP-2) +### 5. Signal Completion to Porch (porch runs 3-way consultation) ```bash porch done {{project_id}} @@ -139,14 +139,14 @@ porch done {{project_id}} Porch will: 1. Run the `pr_exists` / `review_has_arch_updates` / `review_has_lessons_updates` checks. -2. **Execute CMAP-2 (Gemini + Codex) automatically** via the protocol's `verify` block. Outputs land in `codev/projects/{{project_id}}-/{{project_id}}-gemini.txt` and `-codex.txt`. -3. CMAP runs **exactly once** (`max_iterations: 1`). Whatever the verdicts, porch records them in `status.yaml` and advances to the `pr` gate — there is **no automated re-review pass and no "stays in the review phase" loop**. `APPROVE` and `REQUEST_CHANGES` differ only in what you must surface to the human (steps 6–7), not in whether the gate fires. The output of `porch done` surfaces the verdicts. +2. **Run 3-way consultation (Gemini, Codex, Claude) automatically** via the protocol's `verify` block. Outputs land in `codev/projects/{{project_id}}-/{{project_id}}-gemini.txt`, `-codex.txt`, and `-claude.txt`. +3. The consultation runs **exactly once** (`max_iterations: 1`). Whatever the verdicts, porch records them in `status.yaml` and advances to the `pr` gate — there is **no automated re-review pass and no "stays in the review phase" loop**. `APPROVE` and `REQUEST_CHANGES` differ only in what you must surface to the human (steps 6–7), not in whether the gate fires. The output of `porch done` surfaces the verdicts. -> **Why CMAP-2, not CMAP-3?** PIR's design parallels BUGFIX/AIR's consult footprint. The human already approved the *running* implementation at the `dev-approval` gate; CMAP at PR is a pre-merge hygiene + code-quality pass, not a functional review. +> **Why consult after the human already approved the running code?** The human approved the *running* implementation at the `dev-approval` gate; the 3-way consultation at the PR is a pre-merge hygiene + code-quality pass, not a functional review. ### 6. Handle a REQUEST_CHANGES Verdict (single-pass — no automated re-review) -PIR's CMAP is one advisory pass (`max_iterations: 1`). If a reviewer returns `REQUEST_CHANGES`, porch does **not** loop or re-run CMAP — it records the verdict and proceeds to the `pr` gate. There is no iter-2. The correctness backstop for a CMAP-flagged issue is therefore **(a)** your fix + a regression test and **(b)** the human's `pr`-gate review — *not* an independent model re-review. Treat that as load-bearing: a substantive finding you "address and rebut" gets no second AI opinion. +PIR's consultation is one advisory pass (`max_iterations: 1`). If a reviewer returns `REQUEST_CHANGES`, porch does **not** loop or re-run it — it records the verdict and proceeds to the `pr` gate. There is no iter-2. The correctness backstop for a consultation-flagged issue is therefore **(a)** your fix + a regression test and **(b)** the human's `pr`-gate review — *not* an independent model re-review. Treat that as load-bearing: a substantive finding you "address and rebut" gets no second AI opinion. For any `REQUEST_CHANGES`: @@ -154,22 +154,23 @@ For any `REQUEST_CHANGES`: 2. **Assess it honestly:** - **Real defect** (correctness / cancellation / security / data-loss): fix it in code, add a regression test that fails without the fix, commit + push (the PR updates automatically — no new `gh pr create`). Then document the finding, your fix, and the pinning test in the review file's **"Things to Look At During PR Review"** section. - **False positive / out of scope**: write a brief rebuttal in that same section explaining why no change is warranted. -3. Do **not** re-run `porch done` expecting another CMAP — `max_iterations: 1` means it will not re-review. Proceed to step 7. +3. Do **not** re-run `porch done` expecting another consultation pass — `max_iterations: 1` means it will not re-review. Proceed to step 7. Whether you fixed it or rebutted it, a `REQUEST_CHANGES` that PIR will never re-check **must be escalated to the human at the `pr` gate** (step 7) — they are the only remaining reviewer of that decision. -### 7. Notify the Architect (after the single CMAP pass — gate is now pending) +### 7. Notify the Architect (after the single consultation pass — gate is now pending) -After the one CMAP pass + structural checks, porch fires the **`pr` gate** (pending) **regardless of the verdicts**. Read the verdicts from porch state and notify — and if any verdict is `REQUEST_CHANGES`, **lead with it and state the disposition**, because PIR will not re-review it and the human at the `pr` gate is the only remaining check: +After the one consultation pass + structural checks, porch fires the **`pr` gate** (pending) **regardless of the verdicts**. Read the verdicts from porch state and notify — and if any verdict is `REQUEST_CHANGES`, **lead with it and state the disposition**, because PIR will not re-review it and the human at the `pr` gate is the only remaining check: ```bash GEMINI_VERDICT=$(grep -m1 -i '^\(approve\|request_changes\|comment\)' "codev/projects/{{project_id}}-"*/"{{project_id}}-gemini.txt" || echo UNKNOWN) CODEX_VERDICT=$(grep -m1 -i '^\(approve\|request_changes\|comment\)' "codev/projects/{{project_id}}-"*/"{{project_id}}-codex.txt" || echo UNKNOWN) +CLAUDE_VERDICT=$(grep -m1 -i '^\(approve\|request_changes\|comment\)' "codev/projects/{{project_id}}-"*/"{{project_id}}-claude.txt" || echo UNKNOWN) -if echo "$GEMINI_VERDICT $CODEX_VERDICT" | grep -qi request_changes; then - afx send architect "⚠️ PR # (PIR #{{issue.number}}): CMAP returned REQUEST_CHANGES (gemini=$GEMINI_VERDICT, codex=$CODEX_VERDICT). Disposition: + regression test | rebutted, see review 'Things to Look At'>. PIR is single-pass — this was NOT independently re-reviewed; please verify the fix/rebuttal at the pr gate before approving. Full verdicts in codev/projects/{{project_id}}-*/." +if echo "$GEMINI_VERDICT $CODEX_VERDICT $CLAUDE_VERDICT" | grep -qi request_changes; then + afx send architect "⚠️ PR # (PIR #{{issue.number}}): 3-way consultation returned REQUEST_CHANGES (gemini=$GEMINI_VERDICT, codex=$CODEX_VERDICT, claude=$CLAUDE_VERDICT). Disposition: + regression test | rebutted, see review 'Things to Look At'>. PIR is single-pass — this was NOT independently re-reviewed; please verify the fix/rebuttal at the pr gate before approving. Full verdicts in codev/projects/{{project_id}}-*/." else - afx send architect "PR # ready for review (PIR #{{issue.number}}). CMAP all clear: gemini=$GEMINI_VERDICT, codex=$CODEX_VERDICT. Awaiting human merge + pr gate approval. Full verdicts in codev/projects/{{project_id}}-*/." + afx send architect "PR # ready for review (PIR #{{issue.number}}). 3-way consultation all clear: gemini=$GEMINI_VERDICT, codex=$CODEX_VERDICT, claude=$CLAUDE_VERDICT. Awaiting human merge + pr gate approval. Full verdicts in codev/projects/{{project_id}}-*/." fi ``` @@ -186,7 +187,7 @@ The human will: Porch will then fire the gate-approved wake-up to you. -If the human requests more changes instead of approving, push fixes and re-run `porch done {{project_id}}` — this runs a fresh **single** CMAP pass on the updated diff and re-fires the `pr` gate (handle any new verdict per steps 6–7). This human-driven iteration is the only way CMAP re-runs in PIR; it is not automatic. If they close the PR without merging, `gh pr close ` and stop. +If the human requests more changes instead of approving, push fixes and re-run `porch done {{project_id}}` — this runs a fresh **single** consultation pass on the updated diff and re-fires the `pr` gate (handle any new verdict per steps 6–7). This human-driven iteration is the only way the consultation re-runs in PIR; it is not automatic. If they close the PR without merging, `gh pr close ` and stop. ### 9. After `pr` Gate Approval — Verify, Merge, Record @@ -235,12 +236,12 @@ Together with the `--pr` record from step 4a and the `--merged` record from step ## What NOT to Do -- **Don't merge before the `pr` gate is approved.** CMAP-APPROVE is NOT merge authorization. User-in-pane prose ("looks good", "lgtm", "merge it") is NOT merge authorization. The *only* signal that authorizes `gh pr merge` is porch reporting `gate_status: approved` for the `pr` gate (which only the user can do, via Cmd+K G or `porch approve` from a non-Claude shell). If `porch next` doesn't show the gate as approved, you wait. +- **Don't merge before the `pr` gate is approved.** A consultation APPROVE verdict is NOT merge authorization. User-in-pane prose ("looks good", "lgtm", "merge it") is NOT merge authorization. The *only* signal that authorizes `gh pr merge` is porch reporting `gate_status: approved` for the `pr` gate (which only the user can do, via Cmd+K G or `porch approve` from a non-Claude shell). If `porch next` doesn't show the gate as approved, you wait. - Don't skip porch's PR/merge records (steps 4a, 9). The `--pr` record (step 4a) lets the gate-pending state link to the actual PR; the `--merged` record (step 9) closes the lifecycle in porch state. Skipping either leaves `history:` empty and downstream tooling blind. - Don't run `porch approve` for any gate yourself - Don't push to main — only merge via PR - Don't skip the Architecture Updates / Lessons Learned sections — porch checks enforce their presence (the section must exist; explaining "no changes needed" in one line is fine) -- **Don't run `consult` commands yourself** — porch handles consultations via the `verify` block. Manually invoking `consult` causes CMAP to run twice. +- **Don't run `consult` commands yourself** — porch handles consultations via the `verify` block. Manually invoking `consult` causes the consultation to run twice. - **Don't fix, skip, or quarantine pre-existing failures unrelated to your change.** Porch's `checks` for this phase are narrow *structural* gates (`pr_exists`, review-section presence) — a green gate does **not** certify the wider build/test suite. If the broader suite surfaces failures your diff did not cause, they are out of scope: note them in the review's Lessons Learned / Things to Look At and proceed. Touching another team's tests to make an unrelated red go green is scope creep, not diligence. ## Handling Problems @@ -251,7 +252,7 @@ Together with the `--pr` record from step 4a and the `--merged` record from step - Force-push with lease: `git push --force-with-lease` - Re-run `gh pr create` -**If porch's CMAP consults fail (e.g., model unavailable):** +**If porch's consultation fails (e.g., model unavailable):** - `porch done` will report the failure. Inspect `codev/projects/{{project_id}}-*/{{project_id}}-.txt` for the failure details. - Re-run `porch done {{project_id}}` once — porch will retry the consult. - If the model is persistently unavailable, notify the architect and ask whether to proceed without that model's verdict. They may direct you to skip via a manual override. diff --git a/codev-skeleton/protocols/pir/protocol.json b/codev-skeleton/protocols/pir/protocol.json index e2812d036..067e64765 100644 --- a/codev-skeleton/protocols/pir/protocol.json +++ b/codev-skeleton/protocols/pir/protocol.json @@ -70,7 +70,7 @@ { "id": "review", "name": "Review", - "description": "Write retrospective (arch + lessons), push, open PR with review as body, run CMAP, then wait at pr gate while the human merges on GitHub", + "description": "Write retrospective (arch + lessons), push, open PR with review as body, run 3-way consultation, then wait at pr gate while the human merges on GitHub", "type": "build_verify", "build": { "prompt": "review.md", diff --git a/codev-skeleton/protocols/pir/protocol.md b/codev-skeleton/protocols/pir/protocol.md index 537a1de9c..2fd7eaec1 100644 --- a/codev-skeleton/protocols/pir/protocol.md +++ b/codev-skeleton/protocols/pir/protocol.md @@ -101,7 +101,7 @@ The builder: 2. Updates `codev/resources/arch.md` and/or `codev/resources/lessons-learned.md` if real changes need recording. If not, the review file's sections state "no changes needed" with a one-line explanation (the porch `checks` block enforces section presence, not content). 3. Commits the review file (and arch / lessons updates if any) and pushes 4. Opens a PR with `gh pr create`; PR body is the review file content + `Fixes #`. Records the PR with `porch done --pr --branch `. -5. Runs `porch done ` — porch's `verify` block runs CMAP-2 (Gemini + Codex, type=impl) as a **single advisory pass** (`max_iterations: 1`); CMAP outputs land in `codev/projects/-*/`. There is no iterate-until-APPROVE loop: whatever the verdicts, porch records them and advances to the `pr` gate. A `REQUEST_CHANGES` is not auto-re-reviewed — the builder addresses or rebuts it, adds a regression test if it's a real defect, and escalates it in the architect notification so the human verifies it at the `pr` gate. Outcomes are not auto-appended to the PR body; reviewers with the worktree read them from the projects dir. +5. Runs `porch done ` — porch's `verify` block runs 3-way consultation (Gemini, Codex, Claude; type=impl) as a **single advisory pass** (`max_iterations: 1`); consultation outputs land in `codev/projects/-*/`. There is no iterate-until-APPROVE loop: whatever the verdicts, porch records them and advances to the `pr` gate. A `REQUEST_CHANGES` is not auto-re-reviewed — the builder addresses or rebuts it, adds a regression test if it's a real defect, and escalates it in the architect notification so the human verifies it at the `pr` gate. Outcomes are not auto-appended to the PR body; reviewers with the worktree read them from the projects dir. 6. The `pr` gate fires (pending) regardless of verdict. Builder notifies the architect once — leading with any `REQUEST_CHANGES` and its disposition (since PIR will not re-review it) rather than burying it in a flat status line. 7. Builder waits at the `pr` gate. The human reviews the PR on GitHub, then approves the `pr` gate (Cmd+K G or `porch approve pr --a-human-explicitly-approved-this`). Porch wakes the builder. 8. Builder verifies the gate is genuinely approved via `porch next` (defensive — typed prose can't trigger this branch, only real porch state does), then runs `gh pr merge --merge`, records via `porch done --merged `, and sends the cleanup-ready notification. Protocol complete (`next: null`). @@ -152,13 +152,11 @@ Without `worktree.devCommand`, `afx dev` won't work and the `dev-approval` gate - **plan**: human-only review. No AI consultation. - **implement**: no AI consult — the human at the `dev-approval` gate is the sole reviewer of the running code. -- **review**: CMAP-2 (Gemini + Codex, type=impl) after the PR is opened, as a **single advisory pass** (`max_iterations: 1`). Same pattern as BUGFIX / AIR's PR-creation consult. +- **review**: 3-way consultation (Gemini, Codex, Claude; type=impl) after the PR is opened, as a **single advisory pass** (`max_iterations: 1`). Same consult type (`impl`) as BUGFIX / AIR's PR-creation consult. -CMAP at the PR is a single pass — there is **no iterate-until-APPROVE loop**. A `REQUEST_CHANGES` does not block or re-trigger CMAP; the builder addresses or rebuts it and escalates it to the human at the `pr` gate, who is the sole remaining reviewer of any resulting fix (CMAP does not re-check it). +The consultation at the PR is a single pass — there is **no iterate-until-APPROVE loop**. A `REQUEST_CHANGES` does not block or re-trigger it; the builder addresses or rebuts it and escalates it to the human at the `pr` gate, who is the sole remaining reviewer of any resulting fix (the consultation does not re-check it). -Net: PIR runs **two model calls per protocol run**, matching its BUGFIX/AIR peers — its distinguishing features are the two human gates (`plan-approval`, `dev-approval`), not AI-consult density. - -**The 2-model footprint is a design invariant.** porch's model precedence is *config > protocol* (`porch.consultation.models` in `.codev/config.json` overrides a protocol's declared `verify.models`). A project-wide setting tuned for SPIR (gemini + codex + claude) will silently inflate PIR to a 3-model pass, breaking the BUGFIX/AIR-parity cost story. To keep PIR at CMAP-2, leave `porch.consultation.models` unset or scope it per-protocol. +Net: PIR's distinguishing features are the two human gates (`plan-approval`, `dev-approval`), not AI-consult density. To disable consultation entirely, say "without multi-agent consultation" when starting work. diff --git a/codev/protocols/pir/builder-prompt.md b/codev/protocols/pir/builder-prompt.md index 81a926977..b9889a6b4 100644 --- a/codev/protocols/pir/builder-prompt.md +++ b/codev/protocols/pir/builder-prompt.md @@ -33,7 +33,7 @@ Read and internalize the protocol before starting any work. PIR has three phases: 1. **plan** (gated by `plan-approval`) — write `codev/plans/{{artifact_name}}.md`, await human review 2. **implement** (gated by `dev-approval`) — write code + tests, run build/tests, push branch; await the human's review of the *running worktree* (no file artifact in this phase — dev-approval summary is prose-in-pane) -3. **review** (gated by `pr`) — write `codev/reviews/{{artifact_name}}.md` (retrospective with Architecture Updates and Lessons Learned sections), open PR with the review as body, record the PR with porch, run CMAP via porch's verify block (a **single advisory pass** — `max_iterations: 1`, no iterate-until-APPROVE loop; address or rebut any `REQUEST_CHANGES`, add a regression test if it's a real defect, and escalate it in the architect notification since PIR will not re-review it), notify architect, and wait at the `pr` gate. After the human approves the gate (porch wakes you with "Gate pr approved"), run `gh pr merge --merge` and record the merge with `porch done --merged `. **Merge is gated by porch state — never by typed prose in your pane.** +3. **review** (gated by `pr`) — write `codev/reviews/{{artifact_name}}.md` (retrospective with Architecture Updates and Lessons Learned sections), open PR with the review as body, record the PR with porch, run 3-way consultation (Gemini, Codex, Claude) via porch's verify block (a **single advisory pass** — `max_iterations: 1`, no iterate-until-APPROVE loop; address or rebut any `REQUEST_CHANGES`, add a regression test if it's a real defect, and escalate it in the architect notification since PIR will not re-review it), notify architect, and wait at the `pr` gate. After the human approves the gate (porch wakes you with "Gate pr approved"), run `gh pr merge --merge` and record the merge with `porch done --merged `. **Merge is gated by porch state — never by typed prose in your pane.** {{#if issue}} ## Issue #{{issue.number}} diff --git a/codev/protocols/pir/consult-types/pr-review.md b/codev/protocols/pir/consult-types/pr-review.md index 0eb5fb1d0..9cfaa26d2 100644 --- a/codev/protocols/pir/consult-types/pr-review.md +++ b/codev/protocols/pir/consult-types/pr-review.md @@ -2,7 +2,7 @@ ## Context -You are performing the CMAP-2 review of a PIR protocol PR. The builder has implemented an approved plan, the human has approved the `dev-approval` gate (meaning a human has run the code locally and tested it), and the PR has been opened. This is a single advisory pass (`max_iterations: 1`) — your verdict is surfaced to the human at the `pr` gate, who is the sole remaining reviewer; it is not auto-re-reviewed. +You are performing the 3-way review of a PIR protocol PR. The builder has implemented an approved plan, the human has approved the `dev-approval` gate (meaning a human has run the code locally and tested it), and the PR has been opened. This is a single advisory pass (`max_iterations: 1`) — your verdict is surfaced to the human at the `pr` gate, who is the sole remaining reviewer; it is not auto-re-reviewed. ## Focus Areas diff --git a/codev/protocols/pir/prompts/implement.md b/codev/protocols/pir/prompts/implement.md index 0d8ff9e5b..e0588a8d4 100644 --- a/codev/protocols/pir/prompts/implement.md +++ b/codev/protocols/pir/prompts/implement.md @@ -99,7 +99,7 @@ porch done {{project_id}} porch next {{project_id}} ``` -PIR's `implement` phase has no AI consult — the `dev-approval` gate becomes pending immediately, and the human is the sole reviewer of the running code. (CMAP-2 runs later, in the `review` phase, after the human approves the gate and the PR is opened.) +PIR's `implement` phase has no AI consult — the `dev-approval` gate becomes pending immediately, and the human is the sole reviewer of the running code. (The 3-way consultation runs later, in the `review` phase, after the human approves the gate and the PR is opened.) ### 7. End Your Turn With a Code-Review Summary (Prose, Not a File) diff --git a/codev/protocols/pir/prompts/review.md b/codev/protocols/pir/prompts/review.md index a5153902b..d25dd6105 100644 --- a/codev/protocols/pir/prompts/review.md +++ b/codev/protocols/pir/prompts/review.md @@ -4,7 +4,7 @@ You are executing the **REVIEW** phase of the PIR protocol. ## Your Goal -Write a retrospective at `codev/reviews/{{artifact_name}}.md` including **Summary**, **Architecture Updates**, and **Lessons Learned Updates** sections. Push, open a PR using the review file as the PR body, record the PR with porch, then signal completion — **porch runs CMAP-2 (Gemini + Codex) once** via the verify block. CMAP is a *single advisory pass* (`max_iterations: 1`): its verdicts are surfaced to the human at the `pr` gate, **not** an iterate-until-APPROVE loop. After the single pass the `pr` gate fires regardless of verdict; you notify the architect (leading with any REQUEST_CHANGES) and wait at the gate while the human merges on GitHub. +Write a retrospective at `codev/reviews/{{artifact_name}}.md` including **Summary**, **Architecture Updates**, and **Lessons Learned Updates** sections. Push, open a PR using the review file as the PR body, record the PR with porch, then signal completion — **porch runs 3-way consultation (Gemini, Codex, Claude) once** via the verify block. The consultation is a *single advisory pass* (`max_iterations: 1`): its verdicts are surfaced to the human at the `pr` gate, **not** an iterate-until-APPROVE loop. After the single pass the `pr` gate fires regardless of verdict; you notify the architect (leading with any REQUEST_CHANGES) and wait at the gate while the human merges on GitHub. The retrospective ships with the merged PR — it's durable team knowledge, searchable in `codev/reviews/` on `main`. @@ -123,7 +123,7 @@ gh pr edit --body-file codev/reviews/{{artifact_name}}.md ### 4a. Record the PR with Porch -Immediately after creating the PR, tell porch about it so `status.yaml` carries the PR number and branch. This is a metadata-only call — it does NOT advance the phase or trigger CMAP: +Immediately after creating the PR, tell porch about it so `status.yaml` carries the PR number and branch. This is a metadata-only call — it does NOT advance the phase or trigger the consultation: ```bash porch done {{project_id}} --pr --branch "$(git branch --show-current)" @@ -131,7 +131,7 @@ porch done {{project_id}} --pr --branch "$(git branch --show-current Without this, porch's `history:` for the project stays empty and downstream tooling (status views, analytics, audit trails) can't link the porch project to its GitHub PR. -### 5. Signal Completion to Porch (porch runs CMAP-2) +### 5. Signal Completion to Porch (porch runs 3-way consultation) ```bash porch done {{project_id}} @@ -139,14 +139,14 @@ porch done {{project_id}} Porch will: 1. Run the `pr_exists` / `review_has_arch_updates` / `review_has_lessons_updates` checks. -2. **Execute CMAP-2 (Gemini + Codex) automatically** via the protocol's `verify` block. Outputs land in `codev/projects/{{project_id}}-/{{project_id}}-gemini.txt` and `-codex.txt`. -3. CMAP runs **exactly once** (`max_iterations: 1`). Whatever the verdicts, porch records them in `status.yaml` and advances to the `pr` gate — there is **no automated re-review pass and no "stays in the review phase" loop**. `APPROVE` and `REQUEST_CHANGES` differ only in what you must surface to the human (steps 6–7), not in whether the gate fires. The output of `porch done` surfaces the verdicts. +2. **Run 3-way consultation (Gemini, Codex, Claude) automatically** via the protocol's `verify` block. Outputs land in `codev/projects/{{project_id}}-/{{project_id}}-gemini.txt`, `-codex.txt`, and `-claude.txt`. +3. The consultation runs **exactly once** (`max_iterations: 1`). Whatever the verdicts, porch records them in `status.yaml` and advances to the `pr` gate — there is **no automated re-review pass and no "stays in the review phase" loop**. `APPROVE` and `REQUEST_CHANGES` differ only in what you must surface to the human (steps 6–7), not in whether the gate fires. The output of `porch done` surfaces the verdicts. -> **Why CMAP-2, not CMAP-3?** PIR's design parallels BUGFIX/AIR's consult footprint. The human already approved the *running* implementation at the `dev-approval` gate; CMAP at PR is a pre-merge hygiene + code-quality pass, not a functional review. +> **Why consult after the human already approved the running code?** The human approved the *running* implementation at the `dev-approval` gate; the 3-way consultation at the PR is a pre-merge hygiene + code-quality pass, not a functional review. ### 6. Handle a REQUEST_CHANGES Verdict (single-pass — no automated re-review) -PIR's CMAP is one advisory pass (`max_iterations: 1`). If a reviewer returns `REQUEST_CHANGES`, porch does **not** loop or re-run CMAP — it records the verdict and proceeds to the `pr` gate. There is no iter-2. The correctness backstop for a CMAP-flagged issue is therefore **(a)** your fix + a regression test and **(b)** the human's `pr`-gate review — *not* an independent model re-review. Treat that as load-bearing: a substantive finding you "address and rebut" gets no second AI opinion. +PIR's consultation is one advisory pass (`max_iterations: 1`). If a reviewer returns `REQUEST_CHANGES`, porch does **not** loop or re-run it — it records the verdict and proceeds to the `pr` gate. There is no iter-2. The correctness backstop for a consultation-flagged issue is therefore **(a)** your fix + a regression test and **(b)** the human's `pr`-gate review — *not* an independent model re-review. Treat that as load-bearing: a substantive finding you "address and rebut" gets no second AI opinion. For any `REQUEST_CHANGES`: @@ -154,22 +154,23 @@ For any `REQUEST_CHANGES`: 2. **Assess it honestly:** - **Real defect** (correctness / cancellation / security / data-loss): fix it in code, add a regression test that fails without the fix, commit + push (the PR updates automatically — no new `gh pr create`). Then document the finding, your fix, and the pinning test in the review file's **"Things to Look At During PR Review"** section. - **False positive / out of scope**: write a brief rebuttal in that same section explaining why no change is warranted. -3. Do **not** re-run `porch done` expecting another CMAP — `max_iterations: 1` means it will not re-review. Proceed to step 7. +3. Do **not** re-run `porch done` expecting another consultation pass — `max_iterations: 1` means it will not re-review. Proceed to step 7. Whether you fixed it or rebutted it, a `REQUEST_CHANGES` that PIR will never re-check **must be escalated to the human at the `pr` gate** (step 7) — they are the only remaining reviewer of that decision. -### 7. Notify the Architect (after the single CMAP pass — gate is now pending) +### 7. Notify the Architect (after the single consultation pass — gate is now pending) -After the one CMAP pass + structural checks, porch fires the **`pr` gate** (pending) **regardless of the verdicts**. Read the verdicts from porch state and notify — and if any verdict is `REQUEST_CHANGES`, **lead with it and state the disposition**, because PIR will not re-review it and the human at the `pr` gate is the only remaining check: +After the one consultation pass + structural checks, porch fires the **`pr` gate** (pending) **regardless of the verdicts**. Read the verdicts from porch state and notify — and if any verdict is `REQUEST_CHANGES`, **lead with it and state the disposition**, because PIR will not re-review it and the human at the `pr` gate is the only remaining check: ```bash GEMINI_VERDICT=$(grep -m1 -i '^\(approve\|request_changes\|comment\)' "codev/projects/{{project_id}}-"*/"{{project_id}}-gemini.txt" || echo UNKNOWN) CODEX_VERDICT=$(grep -m1 -i '^\(approve\|request_changes\|comment\)' "codev/projects/{{project_id}}-"*/"{{project_id}}-codex.txt" || echo UNKNOWN) +CLAUDE_VERDICT=$(grep -m1 -i '^\(approve\|request_changes\|comment\)' "codev/projects/{{project_id}}-"*/"{{project_id}}-claude.txt" || echo UNKNOWN) -if echo "$GEMINI_VERDICT $CODEX_VERDICT" | grep -qi request_changes; then - afx send architect "⚠️ PR # (PIR #{{issue.number}}): CMAP returned REQUEST_CHANGES (gemini=$GEMINI_VERDICT, codex=$CODEX_VERDICT). Disposition: + regression test | rebutted, see review 'Things to Look At'>. PIR is single-pass — this was NOT independently re-reviewed; please verify the fix/rebuttal at the pr gate before approving. Full verdicts in codev/projects/{{project_id}}-*/." +if echo "$GEMINI_VERDICT $CODEX_VERDICT $CLAUDE_VERDICT" | grep -qi request_changes; then + afx send architect "⚠️ PR # (PIR #{{issue.number}}): 3-way consultation returned REQUEST_CHANGES (gemini=$GEMINI_VERDICT, codex=$CODEX_VERDICT, claude=$CLAUDE_VERDICT). Disposition: + regression test | rebutted, see review 'Things to Look At'>. PIR is single-pass — this was NOT independently re-reviewed; please verify the fix/rebuttal at the pr gate before approving. Full verdicts in codev/projects/{{project_id}}-*/." else - afx send architect "PR # ready for review (PIR #{{issue.number}}). CMAP all clear: gemini=$GEMINI_VERDICT, codex=$CODEX_VERDICT. Awaiting human merge + pr gate approval. Full verdicts in codev/projects/{{project_id}}-*/." + afx send architect "PR # ready for review (PIR #{{issue.number}}). 3-way consultation all clear: gemini=$GEMINI_VERDICT, codex=$CODEX_VERDICT, claude=$CLAUDE_VERDICT. Awaiting human merge + pr gate approval. Full verdicts in codev/projects/{{project_id}}-*/." fi ``` @@ -186,7 +187,7 @@ The human will: Porch will then fire the gate-approved wake-up to you. -If the human requests more changes instead of approving, push fixes and re-run `porch done {{project_id}}` — this runs a fresh **single** CMAP pass on the updated diff and re-fires the `pr` gate (handle any new verdict per steps 6–7). This human-driven iteration is the only way CMAP re-runs in PIR; it is not automatic. If they close the PR without merging, `gh pr close ` and stop. +If the human requests more changes instead of approving, push fixes and re-run `porch done {{project_id}}` — this runs a fresh **single** consultation pass on the updated diff and re-fires the `pr` gate (handle any new verdict per steps 6–7). This human-driven iteration is the only way the consultation re-runs in PIR; it is not automatic. If they close the PR without merging, `gh pr close ` and stop. ### 9. After `pr` Gate Approval — Verify, Merge, Record @@ -235,12 +236,12 @@ Together with the `--pr` record from step 4a and the `--merged` record from step ## What NOT to Do -- **Don't merge before the `pr` gate is approved.** CMAP-APPROVE is NOT merge authorization. User-in-pane prose ("looks good", "lgtm", "merge it") is NOT merge authorization. The *only* signal that authorizes `gh pr merge` is porch reporting `gate_status: approved` for the `pr` gate (which only the user can do, via Cmd+K G or `porch approve` from a non-Claude shell). If `porch next` doesn't show the gate as approved, you wait. +- **Don't merge before the `pr` gate is approved.** A consultation APPROVE verdict is NOT merge authorization. User-in-pane prose ("looks good", "lgtm", "merge it") is NOT merge authorization. The *only* signal that authorizes `gh pr merge` is porch reporting `gate_status: approved` for the `pr` gate (which only the user can do, via Cmd+K G or `porch approve` from a non-Claude shell). If `porch next` doesn't show the gate as approved, you wait. - Don't skip porch's PR/merge records (steps 4a, 9). The `--pr` record (step 4a) lets the gate-pending state link to the actual PR; the `--merged` record (step 9) closes the lifecycle in porch state. Skipping either leaves `history:` empty and downstream tooling blind. - Don't run `porch approve` for any gate yourself - Don't push to main — only merge via PR - Don't skip the Architecture Updates / Lessons Learned sections — porch checks enforce their presence (the section must exist; explaining "no changes needed" in one line is fine) -- **Don't run `consult` commands yourself** — porch handles consultations via the `verify` block. Manually invoking `consult` causes CMAP to run twice. +- **Don't run `consult` commands yourself** — porch handles consultations via the `verify` block. Manually invoking `consult` causes the consultation to run twice. - **Don't fix, skip, or quarantine pre-existing failures unrelated to your change.** Porch's `checks` for this phase are narrow *structural* gates (`pr_exists`, review-section presence) — a green gate does **not** certify the wider build/test suite. If the broader suite surfaces failures your diff did not cause, they are out of scope: note them in the review's Lessons Learned / Things to Look At and proceed. Touching another team's tests to make an unrelated red go green is scope creep, not diligence. ## Handling Problems @@ -251,7 +252,7 @@ Together with the `--pr` record from step 4a and the `--merged` record from step - Force-push with lease: `git push --force-with-lease` - Re-run `gh pr create` -**If porch's CMAP consults fail (e.g., model unavailable):** +**If porch's consultation fails (e.g., model unavailable):** - `porch done` will report the failure. Inspect `codev/projects/{{project_id}}-*/{{project_id}}-.txt` for the failure details. - Re-run `porch done {{project_id}}` once — porch will retry the consult. - If the model is persistently unavailable, notify the architect and ask whether to proceed without that model's verdict. They may direct you to skip via a manual override. diff --git a/codev/protocols/pir/protocol.json b/codev/protocols/pir/protocol.json index e2812d036..067e64765 100644 --- a/codev/protocols/pir/protocol.json +++ b/codev/protocols/pir/protocol.json @@ -70,7 +70,7 @@ { "id": "review", "name": "Review", - "description": "Write retrospective (arch + lessons), push, open PR with review as body, run CMAP, then wait at pr gate while the human merges on GitHub", + "description": "Write retrospective (arch + lessons), push, open PR with review as body, run 3-way consultation, then wait at pr gate while the human merges on GitHub", "type": "build_verify", "build": { "prompt": "review.md", diff --git a/codev/protocols/pir/protocol.md b/codev/protocols/pir/protocol.md index 537a1de9c..2fd7eaec1 100644 --- a/codev/protocols/pir/protocol.md +++ b/codev/protocols/pir/protocol.md @@ -101,7 +101,7 @@ The builder: 2. Updates `codev/resources/arch.md` and/or `codev/resources/lessons-learned.md` if real changes need recording. If not, the review file's sections state "no changes needed" with a one-line explanation (the porch `checks` block enforces section presence, not content). 3. Commits the review file (and arch / lessons updates if any) and pushes 4. Opens a PR with `gh pr create`; PR body is the review file content + `Fixes #`. Records the PR with `porch done --pr --branch `. -5. Runs `porch done ` — porch's `verify` block runs CMAP-2 (Gemini + Codex, type=impl) as a **single advisory pass** (`max_iterations: 1`); CMAP outputs land in `codev/projects/-*/`. There is no iterate-until-APPROVE loop: whatever the verdicts, porch records them and advances to the `pr` gate. A `REQUEST_CHANGES` is not auto-re-reviewed — the builder addresses or rebuts it, adds a regression test if it's a real defect, and escalates it in the architect notification so the human verifies it at the `pr` gate. Outcomes are not auto-appended to the PR body; reviewers with the worktree read them from the projects dir. +5. Runs `porch done ` — porch's `verify` block runs 3-way consultation (Gemini, Codex, Claude; type=impl) as a **single advisory pass** (`max_iterations: 1`); consultation outputs land in `codev/projects/-*/`. There is no iterate-until-APPROVE loop: whatever the verdicts, porch records them and advances to the `pr` gate. A `REQUEST_CHANGES` is not auto-re-reviewed — the builder addresses or rebuts it, adds a regression test if it's a real defect, and escalates it in the architect notification so the human verifies it at the `pr` gate. Outcomes are not auto-appended to the PR body; reviewers with the worktree read them from the projects dir. 6. The `pr` gate fires (pending) regardless of verdict. Builder notifies the architect once — leading with any `REQUEST_CHANGES` and its disposition (since PIR will not re-review it) rather than burying it in a flat status line. 7. Builder waits at the `pr` gate. The human reviews the PR on GitHub, then approves the `pr` gate (Cmd+K G or `porch approve pr --a-human-explicitly-approved-this`). Porch wakes the builder. 8. Builder verifies the gate is genuinely approved via `porch next` (defensive — typed prose can't trigger this branch, only real porch state does), then runs `gh pr merge --merge`, records via `porch done --merged `, and sends the cleanup-ready notification. Protocol complete (`next: null`). @@ -152,13 +152,11 @@ Without `worktree.devCommand`, `afx dev` won't work and the `dev-approval` gate - **plan**: human-only review. No AI consultation. - **implement**: no AI consult — the human at the `dev-approval` gate is the sole reviewer of the running code. -- **review**: CMAP-2 (Gemini + Codex, type=impl) after the PR is opened, as a **single advisory pass** (`max_iterations: 1`). Same pattern as BUGFIX / AIR's PR-creation consult. +- **review**: 3-way consultation (Gemini, Codex, Claude; type=impl) after the PR is opened, as a **single advisory pass** (`max_iterations: 1`). Same consult type (`impl`) as BUGFIX / AIR's PR-creation consult. -CMAP at the PR is a single pass — there is **no iterate-until-APPROVE loop**. A `REQUEST_CHANGES` does not block or re-trigger CMAP; the builder addresses or rebuts it and escalates it to the human at the `pr` gate, who is the sole remaining reviewer of any resulting fix (CMAP does not re-check it). +The consultation at the PR is a single pass — there is **no iterate-until-APPROVE loop**. A `REQUEST_CHANGES` does not block or re-trigger it; the builder addresses or rebuts it and escalates it to the human at the `pr` gate, who is the sole remaining reviewer of any resulting fix (the consultation does not re-check it). -Net: PIR runs **two model calls per protocol run**, matching its BUGFIX/AIR peers — its distinguishing features are the two human gates (`plan-approval`, `dev-approval`), not AI-consult density. - -**The 2-model footprint is a design invariant.** porch's model precedence is *config > protocol* (`porch.consultation.models` in `.codev/config.json` overrides a protocol's declared `verify.models`). A project-wide setting tuned for SPIR (gemini + codex + claude) will silently inflate PIR to a 3-model pass, breaking the BUGFIX/AIR-parity cost story. To keep PIR at CMAP-2, leave `porch.consultation.models` unset or scope it per-protocol. +Net: PIR's distinguishing features are the two human gates (`plan-approval`, `dev-approval`), not AI-consult density. To disable consultation entirely, say "without multi-agent consultation" when starting work. diff --git a/packages/core/src/tower-client.ts b/packages/core/src/tower-client.ts index 40efda734..fdc438b10 100644 --- a/packages/core/src/tower-client.ts +++ b/packages/core/src/tower-client.ts @@ -367,6 +367,64 @@ export class TowerClient { }); } + /** + * Upload a clipboard image to Tower and get back a temp file path. + * + * Deliberately does NOT route through request(): that helper force-sets + * `Content-Type: application/json` after spreading options.headers, so a + * binary content-type can't pass through. This mirrors request()'s auth + * (codev-web-key), timeout, and error-normalization for a raw binary body. + */ + async pasteImage( + workspacePath: string, + bytes: Buffer, + mime: string, + ): Promise<{ ok: boolean; path?: string; error?: string }> { + try { + const authKey = this.getAuthKey(); + const headers: Record = { 'Content-Type': mime }; + if (authKey) { + headers['codev-web-key'] = authKey; + } + // Buffer isn't reliably assignable to fetch's BodyInit across + // @types/node lib versions; an ArrayBuffer slice always is. + const body = bytes.buffer.slice( + bytes.byteOffset, bytes.byteOffset + bytes.byteLength, + ) as ArrayBuffer; + // The paste-image handler is workspace-scoped (same router as + // /workspace//api/state) — a global /api/paste-image has no route. + const encoded = encodeWorkspacePath(workspacePath); + const response = await fetch(`${this.baseUrl}/workspace/${encoded}/api/paste-image`, { + method: 'POST', + headers, + body, + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }); + if (!response.ok) { + const text = await response.text(); + let error: string; + try { + const json = JSON.parse(text); + error = json.error || json.message || text; + } catch { + error = text; + } + return { ok: false, error }; + } + const data = (await response.json()) as { path: string }; + return { ok: true, path: data.path }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (message.includes('ECONNREFUSED')) { + return { ok: false, error: 'Tower not running' }; + } + if (message.includes('timeout')) { + return { ok: false, error: 'Request timeout' }; + } + return { ok: false, error: message }; + } + } + getWorkspaceUrl(workspacePath: string): string { const encoded = encodeWorkspacePath(workspacePath); return `${this.baseUrl}/workspace/${encoded}/`; diff --git a/packages/vscode/CHANGELOG.md b/packages/vscode/CHANGELOG.md index 0b6b1faae..bc60d1705 100644 --- a/packages/vscode/CHANGELOG.md +++ b/packages/vscode/CHANGELOG.md @@ -2,6 +2,17 @@ What's changed in the Codev VS Code extension, version by version, written for the developers who use it. +## [Unreleased] + +### What's new + +- **Per-builder changed files, inline.** A builder row in the Builders view is now expandable to the list of files it changed vs the default branch; clicking a file opens its 2-way diff (read-only merge-base base ↔ worktree file). Rows use VSCode's native Source Control look — the file-type icon plus a colored one-letter status badge (A/M/D/R/C/T, plus `!` for unmerged) and tinted label, driven by your theme's `gitDecoration.*` tokens (the built-in Git decorator can't see the gitignored `.builders/` worktrees, so Codev supplies its own). Git is throttled to ~1 spawn / 15s per expanded builder; collapsed builders never run git. +- **Codev: View Diff** — a command + builder right-click that opens a builder worktree's whole delta vs the default branch in VSCode's Multi Diff Editor. Base content is served by a read-only `codev-diff:` provider running `git -C ` directly, so it works for the gitignored `.builders/` worktrees the Git extension can't discover; handles add/modify/delete/rename/copy and binary files. Coexists with Open Worktree in New Window. +- **Accordion mode for the Builders tree** — expanding one builder auto-collapses the others, so a reviewer never has diffs from unrelated worktrees open at once. Toggle via the new `codev.buildersAutoCollapse` setting (default on), a Builders title-bar button (fold/unfold icon reflects state), or the Command Palette. Builder rows now carry a stable id, fixing expansion being reset on every overview poll. +- **Live-status dot for active builders** — an active builder row shows a green filled dot (the running-process idiom) instead of the old `play` icon that read like a press-to-start button; blocked builders keep the amber bell. +- **Image paste into Codev terminals** — `Cmd+Alt+V` (macOS) / `Ctrl+Alt+V` (Windows/Linux) in a focused Codev terminal uploads a clipboard image to Tower and injects the saved file path into the terminal — the same path-injection UX as the web dashboard and VSCode's own built-in terminal. Codev terminals are `Pseudoterminal`-backed, so VSCode's built-in image-paste bridge never fires for them; this reimplements it (per-OS clipboard read: macOS `osascript`, Linux `wl-paste`/`xclip`, Windows PowerShell). Cmd+V is untouched — normal text paste stays fully native. +- **Backlog hides issues that already have a builder** — an issue with an active builder no longer appears in the Backlog (nor counts toward "Backlog (N)"), so you can't accidentally spawn a second builder for it. Matches the web dashboard. Note `hasBuilder` is machine-local — it reflects this machine's builders, not a teammate's on another machine. + ## [3.0.6] - 2026-05-18 ### What's new diff --git a/packages/vscode/package.json b/packages/vscode/package.json index 31de2eb34..bec23e73b 100644 --- a/packages/vscode/package.json +++ b/packages/vscode/package.json @@ -84,6 +84,16 @@ "title": "Codev: Refresh Overview", "icon": "$(refresh)" }, + { + "command": "codev.enableBuildersAutoCollapse", + "title": "Codev: Turn On Builders Auto-Collapse", + "icon": "$(unfold)" + }, + { + "command": "codev.disableBuildersAutoCollapse", + "title": "Codev: Turn Off Builders Auto-Collapse", + "icon": "$(fold)" + }, { "command": "codev.refreshTeam", "title": "Codev: Refresh Team", @@ -110,6 +120,14 @@ "command": "codev.addReviewComment", "title": "Codev: Add Review Comment" }, + { + "command": "codev.viewDiff", + "title": "Codev: View Diff" + }, + { + "command": "codev.openBuilderFileDiff", + "title": "Codev: Open Builder File Diff" + }, { "command": "codev.openWorktreeWindow", "title": "Codev: Open Worktree in New Window" @@ -130,6 +148,11 @@ "command": "codev.stopWorkspaceDev", "title": "Codev: Stop Dev Server (this workspace)" }, + { + "command": "codev.pasteImage", + "title": "Codev: Paste Image into Terminal", + "icon": "$(file-media)" + }, { "command": "codev.openBuilderById", "title": "Codev: Open Builder Terminal" @@ -187,6 +210,10 @@ "command": "codev.copyBacklogIssueNumber", "when": "false" }, + { + "command": "codev.openBuilderFileDiff", + "when": "false" + }, { "command": "codev.addReviewComment", "when": "editorLangId == 'markdown'" @@ -221,6 +248,11 @@ "when": "view == codev.builders && viewItem =~ /^(builder|blocked-builder)-pir$/", "group": "1_primary@3" }, + { + "command": "codev.viewDiff", + "when": "view == codev.builders && viewItem =~ /^(builder|blocked-builder)-/", + "group": "2_worktree@0" + }, { "command": "codev.openWorktreeWindow", "when": "view == codev.builders && viewItem =~ /^(builder|blocked-builder)-/", @@ -283,6 +315,16 @@ "when": "view == codev.builders", "group": "navigation" }, + { + "command": "codev.disableBuildersAutoCollapse", + "when": "view == codev.builders && codev.buildersAutoCollapse", + "group": "navigation" + }, + { + "command": "codev.enableBuildersAutoCollapse", + "when": "view == codev.builders && !codev.buildersAutoCollapse", + "group": "navigation" + }, { "command": "codev.refreshOverview", "when": "view == codev.pullRequests", @@ -367,6 +409,12 @@ "command": "codev.stopWorkspaceDev", "key": "ctrl+alt+s", "mac": "cmd+alt+s" + }, + { + "command": "codev.pasteImage", + "key": "ctrl+alt+v", + "mac": "cmd+alt+v", + "when": "codev.terminalFocused && terminalFocus" } ], "viewsContainers": { @@ -449,6 +497,11 @@ "default": 60, "minimum": 0, "markdownDescription": "Auto-refresh the Builders / Pull Requests / Backlog / Recently Closed views every N seconds while the Codev sidebar is visible. `0` disables periodic refresh (event-only, the previous behavior). A shared Tower-side 30s cache throttles GitHub calls across all windows." + }, + "codev.buildersAutoCollapse": { + "type": "boolean", + "default": true, + "description": "Builders view accordion: expanding one builder auto-collapses the others, so only one builder's changed-files diff is open at a time." } } } diff --git a/packages/vscode/src/clipboard-image.ts b/packages/vscode/src/clipboard-image.ts new file mode 100644 index 000000000..e90d5fc7a --- /dev/null +++ b/packages/vscode/src/clipboard-image.ts @@ -0,0 +1,178 @@ +/** + * Read a bitmap image off the system clipboard (#736). + * + * VSCode's clipboard API is text-only and a Pseudoterminal never receives + * image bytes, so we shell out per-OS. There is no single cross-platform + * binary-clipboard API available to an extension — this module IS the + * cross-platform layer (macOS + Windows + Linux X11/Wayland behind one + * function). Every shim normalises to PNG, which Tower's /api/paste-image + * accepts directly. + * + * `deps` is injectable purely for unit tests (spawn / platform / env). + */ + +import { spawn as realSpawn } from 'node:child_process'; +import type { ChildProcessByStdio, SpawnOptions } from 'node:child_process'; +import { readFile, unlink } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { randomUUID } from 'node:crypto'; + +export type ClipboardImageResult = + | { kind: 'image'; bytes: Buffer; mime: 'image/png' } + | { kind: 'no-image' } + | { kind: 'tool-missing'; tool: string } + | { kind: 'error'; message: string }; + +export interface ClipboardDeps { + spawn: typeof realSpawn; + platform: () => NodeJS.Platform; + env: NodeJS.ProcessEnv; +} + +const DEFAULT_DEPS: ClipboardDeps = { + spawn: realSpawn, + platform: () => process.platform, + env: process.env, +}; + +const SHIM_TIMEOUT_MS = 5000; + +interface CaptureResult { + spawnError?: NodeJS.ErrnoException; + code: number | null; + stdout: Buffer; + stderr: string; + timedOut: boolean; +} + +/** Spawn a command, capture binary stdout, with a hard timeout. No shell. */ +function runCapture( + deps: ClipboardDeps, + cmd: string, + args: string[], +): Promise { + return new Promise((resolve) => { + const opts: SpawnOptions = { stdio: ['ignore', 'pipe', 'pipe'] }; + let child: ChildProcessByStdio; + try { + child = deps.spawn(cmd, args, opts) as typeof child; + } catch (err) { + resolve({ spawnError: err as NodeJS.ErrnoException, code: null, stdout: Buffer.alloc(0), stderr: '', timedOut: false }); + return; + } + + const out: Buffer[] = []; + let err = ''; + let settled = false; + const finish = (r: CaptureResult) => { if (!settled) { settled = true; resolve(r); } }; + + const timer = setTimeout(() => { + child.kill('SIGKILL'); + finish({ code: null, stdout: Buffer.concat(out), stderr: err, timedOut: true }); + }, SHIM_TIMEOUT_MS); + + child.stdout.on('data', (c: Buffer) => out.push(c)); + child.stderr.on('data', (c: Buffer) => { err += c.toString(); }); + child.on('error', (e: NodeJS.ErrnoException) => { + clearTimeout(timer); + finish({ spawnError: e, code: null, stdout: Buffer.concat(out), stderr: err, timedOut: false }); + }); + child.on('close', (code) => { + clearTimeout(timer); + finish({ code, stdout: Buffer.concat(out), stderr: err, timedOut: false }); + }); + }); +} + +function isENOENT(e: NodeJS.ErrnoException | undefined): boolean { + return !!e && (e.code === 'ENOENT' || /not found|no such file/i.test(e.message)); +} + +/** macOS / Windows shims write a temp PNG; read it then always clean up. */ +async function readTempPng(file: string): Promise { + try { + const buf = await readFile(file); + return buf.length > 0 ? buf : null; + } catch { + return null; + } finally { + void unlink(file).catch(() => { /* best-effort */ }); + } +} + +async function readMac(deps: ClipboardDeps): Promise { + const file = join(tmpdir(), `codev-paste-clip-${randomUUID()}.png`); + // osascript is always present on macOS. Returns "OK" / "NO_IMAGE"; + // any clipboard-has-no-PNG case is caught and reported, never thrown. + const script = [ + 'try', + ' set theData to (the clipboard as «class PNGf»)', + 'on error', + ' return "NO_IMAGE"', + 'end try', + `set fh to open for access (POSIX file ${JSON.stringify(file)}) with write permission`, + 'write theData to fh', + 'close access fh', + 'return "OK"', + ].join('\n'); + const r = await runCapture(deps, 'osascript', ['-e', script]); + if (isENOENT(r.spawnError)) { return { kind: 'tool-missing', tool: 'osascript' }; } + if (r.spawnError || r.timedOut) { + return { kind: 'error', message: r.timedOut ? 'clipboard read timed out' : String(r.spawnError) }; + } + if (r.stdout.toString().includes('NO_IMAGE')) { return { kind: 'no-image' }; } + if (r.code !== 0) { return { kind: 'error', message: r.stderr.trim() || `osascript exited ${r.code}` }; } + const bytes = await readTempPng(file); + return bytes ? { kind: 'image', bytes, mime: 'image/png' } : { kind: 'no-image' }; +} + +async function readLinux(deps: ClipboardDeps): Promise { + const wayland = !!deps.env.WAYLAND_DISPLAY; + const cmd = wayland ? 'wl-paste' : 'xclip'; + const args = wayland + ? ['-t', 'image/png'] + : ['-selection', 'clipboard', '-t', 'image/png', '-o']; + const r = await runCapture(deps, cmd, args); + if (isENOENT(r.spawnError)) { + return { kind: 'tool-missing', tool: wayland ? 'wl-clipboard' : 'xclip' }; + } + if (r.spawnError || r.timedOut) { + return { kind: 'error', message: r.timedOut ? 'clipboard read timed out' : String(r.spawnError) }; + } + // Both tools exit non-zero / empty when the clipboard has no PNG. + if (r.code !== 0 || r.stdout.length === 0) { return { kind: 'no-image' }; } + return { kind: 'image', bytes: r.stdout, mime: 'image/png' }; +} + +async function readWindows(deps: ClipboardDeps): Promise { + const file = join(tmpdir(), `codev-paste-clip-${randomUUID()}.png`); + const ps = [ + 'Add-Type -AssemblyName System.Windows.Forms;', + 'Add-Type -AssemblyName System.Drawing;', + '$img = [System.Windows.Forms.Clipboard]::GetImage();', + 'if ($img -eq $null) { Write-Output "NO_IMAGE"; exit 0 }', + `$img.Save(${JSON.stringify(file)}, [System.Drawing.Imaging.ImageFormat]::Png);`, + 'Write-Output "OK"', + ].join(' '); + const r = await runCapture(deps, 'powershell.exe', ['-NoProfile', '-NonInteractive', '-Command', ps]); + if (isENOENT(r.spawnError)) { return { kind: 'tool-missing', tool: 'PowerShell' }; } + if (r.spawnError || r.timedOut) { + return { kind: 'error', message: r.timedOut ? 'clipboard read timed out' : String(r.spawnError) }; + } + if (r.stdout.toString().includes('NO_IMAGE')) { return { kind: 'no-image' }; } + if (r.code !== 0) { return { kind: 'error', message: r.stderr.trim() || `powershell exited ${r.code}` }; } + const bytes = await readTempPng(file); + return bytes ? { kind: 'image', bytes, mime: 'image/png' } : { kind: 'no-image' }; +} + +export async function readClipboardImage( + deps: ClipboardDeps = DEFAULT_DEPS, +): Promise { + switch (deps.platform()) { + case 'darwin': return readMac(deps); + case 'linux': return readLinux(deps); + case 'win32': return readWindows(deps); + default: return { kind: 'tool-missing', tool: 'a supported platform (macOS, Linux, or Windows)' }; + } +} diff --git a/packages/vscode/src/commands/paste-image.ts b/packages/vscode/src/commands/paste-image.ts new file mode 100644 index 000000000..8fdc4835e --- /dev/null +++ b/packages/vscode/src/commands/paste-image.ts @@ -0,0 +1,80 @@ +/** + * Codev: Paste Image into Terminal (#736). + * + * Bound to a DEDICATED shortcut (Cmd+Alt+V / Ctrl+Alt+V), scoped + * `when: codev.terminalFocused && terminalFocus`. It never touches Cmd+V — + * normal text paste stays 100% native VSCode Pseudoterminal paste (no + * interception, no async detour, no re-dispatch). That is why the earlier + * multi-line text-paste corruption is gone *by construction*: we no longer + * sit in the text-paste path at all. + * + * Codev terminals are `Pseudoterminal`-backed, so VSCode's built-in image + * paste bridge never fires for them (it only fires for terminals VSCode + * owns). This command reimplements it: if the clipboard holds an image, + * upload it to Tower's /api/paste-image and inject the returned temp-file + * path into the focused Codev PTY — the same path-injection UX as the web + * dashboard and VSCode's own built-in terminal. For anything else (no image + * / clipboard tool missing / read error / Tower down) we surface a toast; we + * do NOT paste text here — that is Cmd+V's job and we leave it untouched. + */ + +import * as vscode from 'vscode'; +import type { ConnectionManager } from '../connection-manager.js'; +import type { TerminalManager } from '../terminal-manager.js'; +import { readClipboardImage } from '../clipboard-image.js'; + +export async function pasteImage( + connectionManager: ConnectionManager, + terminalManager: TerminalManager, +): Promise { + const pty = terminalManager.getActiveManagedPty(); + if (!pty) { + // `when` should prevent this. If it ever fires outside a Codev terminal, + // no-op — never shadow normal paste (that's Cmd+V, which we don't bind). + return; + } + + const client = connectionManager.getClient(); + if (!client || connectionManager.getState() !== 'connected') { + vscode.window.showWarningMessage( + 'Codev: not connected to Tower — image paste needs Tower running.', + ); + return; + } + + const result = await readClipboardImage(); + + if (result.kind === 'no-image') { + vscode.window.showInformationMessage('Codev: no image on the clipboard.'); + return; + } + if (result.kind === 'tool-missing') { + vscode.window.showErrorMessage( + `Codev: image paste needs ${result.tool} installed.`, + ); + return; + } + if (result.kind === 'error') { + vscode.window.showErrorMessage( + `Codev: couldn't read clipboard image (${result.message}).`, + ); + return; + } + + // result.kind === 'image' + const workspacePath = connectionManager.getWorkspacePath(); + if (!workspacePath) { + pty.writeNotice('\r\n\x1b[31m[Image upload failed: no workspace]\x1b[0m\r\n'); + return; + } + pty.writeNotice('\r\n\x1b[90m[Uploading image...]\x1b[0m'); + const up = await client.pasteImage(workspacePath, result.bytes, result.mime); + if (!up.ok || !up.path) { + pty.writeNotice( + `\r\x1b[2K\x1b[31m[Image upload failed${up.error ? `: ${up.error}` : ''}]\x1b[0m\r\n`, + ); + return; + } + pty.writeNotice('\r\x1b[2K'); + pty.handleInput(up.path); // no newline — mirrors the dashboard's term.paste(path) +} diff --git a/packages/vscode/src/commands/view-diff.ts b/packages/vscode/src/commands/view-diff.ts new file mode 100644 index 000000000..7d865c53e --- /dev/null +++ b/packages/vscode/src/commands/view-diff.ts @@ -0,0 +1,385 @@ +/** + * Codev: View Diff — open a builder worktree's delta vs the default branch + * as a single in-window multi-file diff editor (file list on the left, + * matching VSCode's built-in "Working Tree" view). + * + * This replaces the previously-removed `review-diff.ts`, which drove the + * same `vscode.changes` editor but resolved file content through `git:` + * URIs. The `git:` scheme is owned by VSCode's built-in Git extension, and + * that extension never discovers a worktree living inside the host repo's + * `.gitignore`d `.builders//` directory — so it returned empty content + * ("(N files)" in the title, "No Changed Files" in the body). + * + * The fix: never touch the `git:` scheme. The *base* (left) side is backed + * by our own read-only `codev-diff:` TextDocumentContentProvider (the same + * pattern as `view-issue.ts`'s `codev-issue:` scheme), populated by running + * `git` directly against the worktree path; the *worktree* (right) side is + * a plain `file:` URI. Nothing depends on the Git extension's repository + * discovery. + * + * Base content is keyed by the immutable merge-base SHA, so VSCode's + * content cache stays correct without a manual `onDidChange`. The right + * side reads live from disk, so it always reflects committed + uncommitted + * tracked changes (the full builder delta a reviewer wants mid-flight). + */ + +import * as vscode from 'vscode'; +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import * as path from 'node:path'; +import type { ConnectionManager } from '../connection-manager.js'; + +const execFileAsync = promisify(execFile); + +export const DIFF_SCHEME = 'codev-diff'; + +// ── Pure helpers (exported for unit testing — no vscode/git dependency) ── + +export type ChangeStatus = 'A' | 'M' | 'D' | 'R' | 'C' | 'T' | 'U'; + +export interface ChangeEntry { + status: ChangeStatus; + /** Source path for renames/copies; null otherwise. */ + oldPath: string | null; + /** Current path (the file as it lives in the worktree). */ + path: string; +} + +/** + * Parse `git diff --name-status -M ` output. Rename/copy lines are + * `R\told\tnew`; everything else is `\tpath`. + */ +export function parseNameStatus(stdout: string): ChangeEntry[] { + return stdout + .split('\n') + .map(l => l.replace(/\r$/, '')) + .filter(Boolean) + .map(line => { + const parts = line.split('\t'); + const status = parts[0]![0]! as ChangeStatus; + if ((status === 'R' || status === 'C') && parts.length >= 3) { + return { status, oldPath: parts[1]!, path: parts[parts.length - 1]! }; + } + return { status, oldPath: null, path: parts[parts.length - 1]! }; + }); +} + +/** + * Strip git's rename rendering from a `--numstat` path field: + * `src/{old => new}/f.ts` → `src/new/f.ts` + * `old/path => new/path` → `new/path` + */ +export function normalizeNumstatPath(raw: string): string { + const brace = raw.replace(/\{[^}]*? => ([^}]*?)\}/g, '$1'); + const arrow = brace.includes(' => ') ? brace.split(' => ').pop()! : brace; + return arrow.trim(); +} + +/** + * Paths whose `git diff --numstat -M ` row is `-\t-\t…` (binary). + */ +export function parseBinaryPaths(numstat: string): Set { + const out = new Set(); + for (const line of numstat.split('\n')) { + const parts = line.split('\t'); + if (parts.length >= 3 && parts[0] === '-' && parts[1] === '-') { + out.add(normalizeNumstatPath(parts.slice(2).join('\t'))); + } + } + return out; +} + +export interface SideSpec { + /** `base` → git blob at merge-base; `file` → on-disk worktree file; + * `empty` → empty doc; `binary` → "not shown" placeholder. */ + kind: 'base' | 'file' | 'empty' | 'binary'; + /** Repo-relative path for `base`/`file` sides. */ + path?: string; +} + +export interface ResourcePlan { + /** New path — drives the file-list icon/label. */ + resourcePath: string; + left: SideSpec; + right: SideSpec; +} + +/** + * Decide, per changed file, what the left (original) and right (modified) + * sides of the diff should be. Pure — no vscode/git. + */ +export function planResources( + changes: ChangeEntry[], + binaryPaths: Set, +): ResourcePlan[] { + return changes.map(c => { + const resourcePath = c.path; + if (binaryPaths.has(c.path) || (c.oldPath && binaryPaths.has(c.oldPath))) { + return { resourcePath, left: { kind: 'binary' }, right: { kind: 'binary' } }; + } + switch (c.status) { + case 'A': + return { resourcePath, left: { kind: 'empty' }, right: { kind: 'file', path: c.path } }; + case 'D': + return { resourcePath, left: { kind: 'base', path: c.path }, right: { kind: 'empty' } }; + case 'R': + case 'C': + return { + resourcePath, + left: { kind: 'base', path: c.oldPath ?? c.path }, + right: { kind: 'file', path: c.path }, + }; + default: // M, T, U + return { + resourcePath, + left: { kind: 'base', path: c.path }, + right: { kind: 'file', path: c.path }, + }; + } + }); +} + +interface DiffQuery { + wt: string; + ref: string; + path: string; + empty?: true; + binary?: true; +} + +export function encodeDiffQuery(q: DiffQuery): string { + return JSON.stringify(q); +} + +export function decodeDiffQuery(query: string): DiffQuery { + return JSON.parse(query) as DiffQuery; +} + +// ── codev-diff: content provider ──────────────────────────────────────── + +class DiffContentProvider implements vscode.TextDocumentContentProvider { + async provideTextDocumentContent(uri: vscode.Uri): Promise { + let q: DiffQuery; + try { + q = decodeDiffQuery(uri.query); + } catch { + return ''; + } + if (q.empty) { return ''; } + if (q.binary) { return '// Binary file — not shown\n'; } + try { + const { stdout } = await execFileAsync( + 'git', + ['-C', q.wt, 'show', `${q.ref}:${q.path}`], + { maxBuffer: 64 * 1024 * 1024 }, + ); + return stdout; + } catch { + // File didn't exist at the base ref (e.g. added file) → empty side. + return ''; + } + } +} + +const provider = new DiffContentProvider(); + +export function activateDiffView(context: vscode.ExtensionContext): void { + context.subscriptions.push( + vscode.workspace.registerTextDocumentContentProvider(DIFF_SCHEME, provider), + ); +} + +// ── URI construction (thin wrappers over the pure encoder) ────────────── + +function baseUri(wt: string, ref: string, repoPath: string): vscode.Uri { + return vscode.Uri.from({ + scheme: DIFF_SCHEME, + path: `/${repoPath}`, + query: encodeDiffQuery({ wt, ref, path: repoPath }), + }); +} + +function placeholderUri(repoPath: string, kind: 'empty' | 'binary'): vscode.Uri { + const q: DiffQuery = { wt: '', ref: '', path: repoPath, [kind]: true }; + return vscode.Uri.from({ + scheme: DIFF_SCHEME, + path: `/${repoPath}`, + query: encodeDiffQuery(q), + }); +} + +function sideUri(side: SideSpec, ctx: { wt: string; ref: string; resourcePath: string }): vscode.Uri { + switch (side.kind) { + case 'file': + return vscode.Uri.file(path.join(ctx.wt, side.path!)); + case 'base': + return baseUri(ctx.wt, ctx.ref, side.path!); + case 'binary': + return placeholderUri(ctx.resourcePath, 'binary'); + default: + return placeholderUri(ctx.resourcePath, 'empty'); + } +} + +/** + * Left/right URIs for one changed file — the shared seam used by both the + * multi-file `vscode.changes` editor and the per-file `vscode.diff` opened + * from the Builders tree. Left = base blob (or empty/binary placeholder); + * right = the on-disk worktree file (or placeholder for deletes/binary). + */ +export function diffUrisForChange( + plan: ResourcePlan, + ctx: { wt: string; ref: string }, +): { left: vscode.Uri; right: vscode.Uri } { + const sideCtx = { wt: ctx.wt, ref: ctx.ref, resourcePath: plan.resourcePath }; + return { + left: sideUri(plan.left, sideCtx), + right: sideUri(plan.right, sideCtx), + }; +} + +// ── Git: a builder's delta vs the default branch ──────────────────────── + +export interface BuilderChanges { + /** Branch name resolved from origin/HEAD (e.g. `main`); for titles. */ + defaultBranch: string; + /** Immutable merge-base SHA used as the base-content cache key. */ + baseRef: string; + changes: ChangeEntry[]; + binaryPaths: Set; +} + +/** + * Resolve a builder worktree's full delta vs its default branch: + * default branch (origin/HEAD, fallback `main`) → merge-base SHA (fallback + * branch name) → `git diff --name-status -M` + `--numstat` against the + * working tree (committed + uncommitted tracked changes). + * + * Throws on git failure so each caller picks its own UX — the command + * surfaces a toast; the Builders tree shows a placeholder row. + */ +export async function getBuilderChanges(wt: string): Promise { + // Default branch from origin/HEAD; fall back to `main`. + let defaultBranch = 'main'; + try { + const { stdout } = await execFileAsync('git', [ + '-C', wt, 'symbolic-ref', '--short', 'refs/remotes/origin/HEAD', + ]); + const ref = stdout.trim().replace(/^origin\//, ''); + if (ref) { defaultBranch = ref; } + } catch { + // origin/HEAD not set — keep `main`. + } + + // Merge-base SHA → immutable cache key for base content. Fall back to the + // branch name if merge-base fails (e.g. unrelated histories). + let baseRef = defaultBranch; + try { + const { stdout } = await execFileAsync('git', [ + '-C', wt, 'merge-base', defaultBranch, 'HEAD', + ]); + const sha = stdout.trim(); + if (sha) { baseRef = sha; } + } catch { + // keep defaultBranch + } + + const [nameStatus, numstat] = await Promise.all([ + execFileAsync('git', ['-C', wt, 'diff', '--name-status', '-M', baseRef], { maxBuffer: 64 * 1024 * 1024 }), + execFileAsync('git', ['-C', wt, 'diff', '--numstat', '-M', baseRef], { maxBuffer: 64 * 1024 * 1024 }), + ]); + return { + defaultBranch, + baseRef, + changes: parseNameStatus(nameStatus.stdout), + binaryPaths: parseBinaryPaths(numstat.stdout), + }; +} + +// ── Command ───────────────────────────────────────────────────────────── + +export async function viewDiff( + connectionManager: ConnectionManager, + builderIdArg: string | undefined, +): Promise { + const client = connectionManager.getClient(); + const workspacePath = connectionManager.getWorkspacePath(); + if (!client || !workspacePath || connectionManager.getState() !== 'connected') { + vscode.window.showErrorMessage('Codev: Not connected to Tower'); + return; + } + + const overview = await client.getOverview(workspacePath); + const builders = overview?.builders ?? []; + if (builders.length === 0) { + vscode.window.showInformationMessage('Codev: No builders to diff'); + return; + } + + const builder = builderIdArg + ? builders.find(b => b.id === builderIdArg) + : await pickBuilder(builders); + if (!builder) { + if (builderIdArg) { + vscode.window.showErrorMessage(`Codev: No builder found for "${builderIdArg}"`); + } + return; + } + if (!builder.worktreePath) { + vscode.window.showErrorMessage(`Codev: Builder ${builder.id} has no worktree on record`); + return; + } + const wt = builder.worktreePath; + + let delta: BuilderChanges; + try { + delta = await getBuilderChanges(wt); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + vscode.window.showErrorMessage(`Codev: git diff failed — ${message}`); + return; + } + const { defaultBranch, baseRef, changes, binaryPaths } = delta; + + if (changes.length === 0) { + vscode.window.showInformationMessage( + `Codev: No changes to review yet for #${builder.issueId ?? builder.id}`, + ); + return; + } + + const resources: Array<[vscode.Uri, vscode.Uri, vscode.Uri]> = planResources( + changes, + binaryPaths, + ).map(plan => { + const { left, right } = diffUrisForChange(plan, { wt, ref: baseRef }); + return [ + vscode.Uri.file(path.join(wt, plan.resourcePath)), + left, + right, + ]; + }); + + await vscode.commands.executeCommand( + 'vscode.changes', + `Reviewing #${builder.issueId ?? builder.id} (${defaultBranch} ↔ HEAD)`, + resources, + ); +} + +interface BuilderLike { + id: string; + issueId: string | null; + issueTitle: string | null; +} + +async function pickBuilder(builders: T[]): Promise { + const picked = await vscode.window.showQuickPick( + builders.map(b => ({ + label: `#${b.issueId ?? b.id} ${b.issueTitle ?? ''}`, + builder: b, + })), + { placeHolder: 'Select builder to diff against the default branch' }, + ); + return picked?.builder; +} diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 902fdce09..bc392866b 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -7,9 +7,11 @@ import { sendMessage } from './commands/send.js'; import { approveGate } from './commands/approve.js'; import { cleanupBuilder } from './commands/cleanup.js'; import { openWorktreeWindow } from './commands/open-worktree-window.js'; +import { viewDiff, activateDiffView, diffUrisForChange } from './commands/view-diff.js'; import { runWorktreeDev } from './commands/run-worktree-dev.js'; import { stopWorktreeDev } from './commands/stop-worktree-dev.js'; import { runWorkspaceDev, stopWorkspaceDev } from './commands/run-workspace-dev.js'; +import { pasteImage } from './commands/paste-image.js'; import { openWorktreeFolder } from './commands/open-worktree-folder.js'; import { runWorktreeSetup } from './commands/run-worktree-setup.js'; import { viewPlanFile } from './commands/view-artifact.js'; @@ -24,12 +26,15 @@ import { BuilderSpawnHandler } from './builder-spawn-handler.js'; import { BuilderTerminalLinkProvider } from './terminal-link-provider.js'; import { BuildersProvider } from './views/builders.js'; import { PullRequestsProvider } from './views/pull-requests.js'; -import { BacklogProvider } from './views/backlog.js'; +import { BacklogProvider, spawnableBacklog } from './views/backlog.js'; import { RecentlyClosedProvider } from './views/recently-closed.js'; import { TeamProvider } from './views/team.js'; import { StatusProvider } from './views/status.js'; import { WorkspaceProvider } from './views/workspace.js'; import { BuilderTreeItem } from './views/builder-tree-item.js'; +import { BuilderFileTreeItem } from './views/builder-file-tree-item.js'; +import { BuilderDiffCache } from './views/builder-diff-cache.js'; +import { BuilderFileDecorationProvider } from './views/builder-file-decoration.js'; import { BacklogTreeItem } from './views/backlog-tree-item.js'; let connectionManager: ConnectionManager | null = null; @@ -111,6 +116,17 @@ export async function activate(context: vscode.ExtensionContext) { terminalManager = new TerminalManager(connectionManager, outputChannel, context.extensionUri, overviewCache); context.subscriptions.push({ dispose: () => terminalManager?.dispose() }); + // Drive the `codev.terminalFocused` context key so the Cmd/Ctrl+V image + // paste binding (#736) only applies when a Codev terminal is focused — + // it must never shadow Cmd+V anywhere else. + const syncTerminalFocusContext = () => + vscode.commands.executeCommand( + 'setContext', 'codev.terminalFocused', + terminalManager?.isCodevTerminalActive() ?? false); + context.subscriptions.push( + vscode.window.onDidChangeActiveTerminal(syncTerminalFocusContext)); + syncTerminalFocusContext(); // seed initial state + // Update status bar with builder/gate counts const updateStatusBarCounts = () => { if (!statusBarItem || connectionManager?.getState() !== 'connected') { return; } @@ -137,7 +153,7 @@ export async function activate(context: vscode.ExtensionContext) { typeof n === 'number' ? `${base} (${n})` : base; if (buildersView) { buildersView.title = withCount('Builders', data?.builders.length); } if (pullRequestsView) { pullRequestsView.title = withCount('Pull Requests', data?.pendingPRs.length); } - if (backlogView) { backlogView.title = withCount('Backlog', data?.backlog.length); } + if (backlogView) { backlogView.title = withCount('Backlog', data ? spawnableBacklog(data.backlog).length : undefined); } if (recentlyClosedView) { recentlyClosedView.title = withCount('Recently Closed', data?.recentlyClosed.length); } }; @@ -186,9 +202,17 @@ export async function activate(context: vscode.ExtensionContext) { updateListViewTitles(); }); + // Shared across the Builders tree (second-level changed files) and the + // SCM-style file decorations so both read one TTL-guarded git result. + const builderDiffCache = new BuilderDiffCache(); + context.subscriptions.push( + { dispose: () => builderDiffCache.dispose() }, + vscode.window.registerFileDecorationProvider(new BuilderFileDecorationProvider(builderDiffCache)), + ); + // List views use createTreeView so their title can carry a live item // count; the rest stay on registerTreeDataProvider. - buildersView = vscode.window.createTreeView('codev.builders', { treeDataProvider: new BuildersProvider(overviewCache) }); + buildersView = vscode.window.createTreeView('codev.builders', { treeDataProvider: new BuildersProvider(overviewCache, builderDiffCache) }); pullRequestsView = vscode.window.createTreeView('codev.pullRequests', { treeDataProvider: new PullRequestsProvider(overviewCache) }); backlogView = vscode.window.createTreeView('codev.backlog', { treeDataProvider: new BacklogProvider(overviewCache) }); recentlyClosedView = vscode.window.createTreeView('codev.recentlyClosed', { treeDataProvider: new RecentlyClosedProvider(overviewCache) }); @@ -203,6 +227,44 @@ export async function activate(context: vscode.ExtensionContext) { vscode.window.registerTreeDataProvider('codev.status', new StatusProvider(connectionManager)), ); + // Builders accordion: expanding one builder auto-collapses the others so a + // reviewer can't have diffs from unrelated worktrees open at once. The + // deterministic collapseAll+reveal pair (vs fighting VSCode's expansion + // reconciliation) is guarded against the expand/collapse events it itself + // generates. Toggle via the header button / `codev.buildersAutoCollapse`. + const readAccordion = () => + vscode.workspace.getConfiguration('codev').get('buildersAutoCollapse', true); + let accordionOn = readAccordion(); + let reconciling = false; + // The builder we've made (or are making) the single open one. The id check + // is the real guard: `reveal({expand:true})` re-fires onDidExpandElement + // for the same builder, and that re-fire can land *after* the await chain + // (so `reconciling` is already false). Matching builderId makes the + // re-fire a no-op regardless of timing — `reconciling` only debounces + // rapid expands of *different* builders. + let openBuilderId: string | undefined; + vscode.commands.executeCommand('setContext', 'codev.buildersAutoCollapse', accordionOn); + context.subscriptions.push( + buildersView.onDidExpandElement(async (e) => { + if (!accordionOn) { return; } + if (!(e.element instanceof BuilderTreeItem)) { return; } + if (e.element.builderId === openBuilderId || reconciling) { return; } + openBuilderId = e.element.builderId; + reconciling = true; + try { + await vscode.commands.executeCommand('workbench.actions.treeView.codev.builders.collapseAll'); + await buildersView!.reveal(e.element, { expand: true, select: false, focus: false }); + } finally { + reconciling = false; + } + }), + vscode.workspace.onDidChangeConfiguration((e) => { + if (!e.affectsConfiguration('codev.buildersAutoCollapse')) { return; } + accordionOn = readAccordion(); + vscode.commands.executeCommand('setContext', 'codev.buildersAutoCollapse', accordionOn); + }), + ); + // Periodic overview refresh. VSCode has no timer-based refresh (event-only), // so an idle workspace never sees externally-merged PRs / new issues. Mirror // the dashboard's poll idiom: refresh on a cadence while the Codev sidebar is @@ -355,6 +417,14 @@ export async function activate(context: vscode.ExtensionContext) { vscode.commands.registerCommand('codev.cleanupBuilder', () => cleanupBuilder(connectionManager!, overviewCache)), vscode.commands.registerCommand('codev.openWorktreeWindow', (arg: vscode.TreeItem | string | undefined) => openWorktreeWindow(connectionManager!, extractBuilderId(arg))), + vscode.commands.registerCommand('codev.viewDiff', (arg: vscode.TreeItem | string | undefined) => + viewDiff(connectionManager!, extractBuilderId(arg))), + vscode.commands.registerCommand('codev.openBuilderFileDiff', async (arg: unknown) => { + if (!(arg instanceof BuilderFileTreeItem)) { return; } + const { left, right } = diffUrisForChange(arg.plan, { wt: arg.worktreePath, ref: arg.baseRef }); + const title = `${arg.plan.resourcePath} (#${arg.builderId})`; + await vscode.commands.executeCommand('vscode.diff', left, right, title); + }), vscode.commands.registerCommand('codev.runWorktreeDev', (arg: vscode.TreeItem | string | undefined) => runWorktreeDev(connectionManager!, terminalManager!, extractBuilderId(arg))), vscode.commands.registerCommand('codev.stopWorktreeDev', () => @@ -363,6 +433,8 @@ export async function activate(context: vscode.ExtensionContext) { runWorkspaceDev(connectionManager!, terminalManager!)), vscode.commands.registerCommand('codev.stopWorkspaceDev', () => stopWorkspaceDev(connectionManager!, terminalManager!)), + vscode.commands.registerCommand('codev.pasteImage', () => + pasteImage(connectionManager!, terminalManager!)), vscode.commands.registerCommand('codev.refreshTeam', () => teamProvider.refresh()), vscode.commands.registerCommand('codev.openWorktreeFolder', (arg: vscode.TreeItem | string | undefined) => openWorktreeFolder(connectionManager!, extractBuilderId(arg))), @@ -371,6 +443,10 @@ export async function activate(context: vscode.ExtensionContext) { vscode.commands.registerCommand('codev.viewPlanFile', (arg: vscode.TreeItem | string | undefined) => viewPlanFile(connectionManager!, extractBuilderId(arg))), vscode.commands.registerCommand('codev.refreshOverview', () => overviewCache.refresh()), + vscode.commands.registerCommand('codev.enableBuildersAutoCollapse', () => + vscode.workspace.getConfiguration('codev').update('buildersAutoCollapse', true, vscode.ConfigurationTarget.Global)), + vscode.commands.registerCommand('codev.disableBuildersAutoCollapse', () => + vscode.workspace.getConfiguration('codev').update('buildersAutoCollapse', false, vscode.ConfigurationTarget.Global)), vscode.commands.registerCommand('codev.reconnect', () => connectionManager?.reconnect()), vscode.commands.registerCommand('codev.connectTunnel', () => connectTunnel(connectionManager!)), vscode.commands.registerCommand('codev.disconnectTunnel', () => disconnectTunnel(connectionManager!)), @@ -382,6 +458,11 @@ export async function activate(context: vscode.ExtensionContext) { // backlog action — renders issue body + comments as markdown preview. activateIssueView(context); + // Read-only `codev-diff:` content provider backing the "View Diff" + // builder action — serves base-branch blob content for the diff editor + // without relying on the Git extension's worktree discovery. + activateDiffView(context); + // Review comment decorations activateReviewDecorations(context); diff --git a/packages/vscode/src/terminal-adapter.ts b/packages/vscode/src/terminal-adapter.ts index 5f703dc8c..d2f531479 100644 --- a/packages/vscode/src/terminal-adapter.ts +++ b/packages/vscode/src/terminal-adapter.ts @@ -71,6 +71,17 @@ export class CodevPseudoterminal implements vscode.Pseudoterminal { this.ws.send(frame); } + /** + * Write text straight to the VSCode terminal renderer (local only — this + * goes through writeEmitter, NOT handleInput, so it is never sent to the + * PTY / agent). Used for transient status notices like "[Uploading + * image...]" during image paste (#736). + */ + writeNotice(text: string): void { + if (this.disposed) { return; } + this.writeEmitter.fire(text); + } + setDimensions(dimensions: vscode.TerminalDimensions): void { this.lastDimensions = { cols: dimensions.columns, rows: dimensions.rows }; if (this.replaying) { diff --git a/packages/vscode/src/terminal-manager.ts b/packages/vscode/src/terminal-manager.ts index 1b0a120e1..a39062cb7 100644 --- a/packages/vscode/src/terminal-manager.ts +++ b/packages/vscode/src/terminal-manager.ts @@ -261,6 +261,25 @@ export class TerminalManager { return this.terminals.size; } + /** + * Resolve the currently-focused VSCode terminal to its Codev PTY, or null + * if the active terminal is not a Codev-managed one. Linear scan over the + * (≤ MAX_TERMINALS) map — no reverse index to keep in sync. Used by the + * image-paste command (#736) and the `codev.terminalFocused` context key. + */ + getActiveManagedPty(): CodevPseudoterminal | null { + const active = vscode.window.activeTerminal; + if (!active) { return null; } + for (const entry of this.terminals.values()) { + if (entry.terminal === active) { return entry.pty; } + } + return null; + } + + isCodevTerminalActive(): boolean { + return this.getActiveManagedPty() !== null; + } + // ── Internal ───────────────────────────────────────────────── private async openTerminal( diff --git a/packages/vscode/src/test/backlog.test.ts b/packages/vscode/src/test/backlog.test.ts new file mode 100644 index 000000000..2f1fa0ff4 --- /dev/null +++ b/packages/vscode/src/test/backlog.test.ts @@ -0,0 +1,37 @@ +import * as assert from 'assert'; +import type { OverviewBacklogItem } from '@cluesmith/codev-types'; +import { spawnableBacklog } from '../views/backlog.js'; + +function item(id: string, hasBuilder: boolean): OverviewBacklogItem { + // Only the fields spawnableBacklog reads matter; cast the rest. + return { id, title: `t${id}`, hasBuilder } as unknown as OverviewBacklogItem; +} + +suite('spawnableBacklog', () => { + test('drops items that already have an active builder', () => { + const out = spawnableBacklog([ + item('1', false), + item('2', true), + item('3', false), + ]); + assert.deepStrictEqual(out.map(i => i.id), ['1', '3']); + }); + + test('empty in -> empty out', () => { + assert.deepStrictEqual(spawnableBacklog([]), []); + }); + + test('all have builders -> empty', () => { + assert.deepStrictEqual(spawnableBacklog([item('1', true), item('2', true)]), []); + }); + + test('preserves input order of the kept items', () => { + const out = spawnableBacklog([ + item('a', false), + item('b', true), + item('c', false), + item('d', false), + ]); + assert.deepStrictEqual(out.map(i => i.id), ['a', 'c', 'd']); + }); +}); diff --git a/packages/vscode/src/test/clipboard-image.test.ts b/packages/vscode/src/test/clipboard-image.test.ts new file mode 100644 index 000000000..98a97f9c7 --- /dev/null +++ b/packages/vscode/src/test/clipboard-image.test.ts @@ -0,0 +1,110 @@ +import * as assert from 'assert'; +import { EventEmitter } from 'node:events'; +import { readClipboardImage, type ClipboardDeps } from '../clipboard-image.js'; + +/** + * Fake child process matching the shape clipboard-image's runCapture uses: + * `.stdout`/`.stderr` emit 'data'; the child emits 'error' | 'close'. + */ +class FakeChild extends EventEmitter { + stdout = new EventEmitter(); + stderr = new EventEmitter(); + kill(): void { /* no-op */ } +} + +interface Scenario { + stdout?: Buffer; + stderr?: string; + code?: number; + enoent?: boolean; +} + +interface SpawnCall { cmd: string; args: string[]; } + +function makeDeps( + platform: NodeJS.Platform, + env: NodeJS.ProcessEnv, + scenario: Scenario, +): { deps: ClipboardDeps; calls: SpawnCall[] } { + const calls: SpawnCall[] = []; + const spawn = ((cmd: string, args: string[]) => { + calls.push({ cmd, args }); + const child = new FakeChild(); + setImmediate(() => { + if (scenario.enoent) { + child.emit('error', Object.assign(new Error(`spawn ${cmd} ENOENT`), { code: 'ENOENT' })); + return; + } + if (scenario.stdout && scenario.stdout.length) { child.stdout.emit('data', scenario.stdout); } + if (scenario.stderr) { child.stderr.emit('data', Buffer.from(scenario.stderr)); } + child.emit('close', scenario.code ?? 0); + }); + return child; + }) as unknown as ClipboardDeps['spawn']; + return { deps: { spawn, platform: () => platform, env }, calls }; +} + +suite('readClipboardImage', () => { + test('unsupported platform → tool-missing', async () => { + const { deps } = makeDeps('freebsd' as NodeJS.Platform, {}, {}); + const r = await readClipboardImage(deps); + assert.strictEqual(r.kind, 'tool-missing'); + }); + + test('linux X11 (no WAYLAND_DISPLAY) spawns xclip with clipboard target', async () => { + const { deps, calls } = makeDeps('linux', {}, { stdout: Buffer.from('PNGDATA'), code: 0 }); + const r = await readClipboardImage(deps); + assert.strictEqual(calls[0].cmd, 'xclip'); + assert.ok(calls[0].args.join(' ').includes('-selection clipboard -t image/png -o')); + assert.strictEqual(r.kind, 'image'); + if (r.kind === 'image') { + assert.strictEqual(r.mime, 'image/png'); + assert.strictEqual(r.bytes.toString(), 'PNGDATA'); + } + }); + + test('linux Wayland (WAYLAND_DISPLAY set) spawns wl-paste', async () => { + const { deps, calls } = makeDeps('linux', { WAYLAND_DISPLAY: 'wayland-0' }, { stdout: Buffer.from('X'), code: 0 }); + await readClipboardImage(deps); + assert.strictEqual(calls[0].cmd, 'wl-paste'); + assert.ok(calls[0].args.includes('image/png')); + }); + + test('linux ENOENT → tool-missing names the right tool per session', async () => { + const x11 = makeDeps('linux', {}, { enoent: true }); + const rx = await readClipboardImage(x11.deps); + assert.deepStrictEqual(rx, { kind: 'tool-missing', tool: 'xclip' }); + + const way = makeDeps('linux', { WAYLAND_DISPLAY: ':0' }, { enoent: true }); + const rw = await readClipboardImage(way.deps); + assert.deepStrictEqual(rw, { kind: 'tool-missing', tool: 'wl-clipboard' }); + }); + + test('linux clean exit with empty stdout → no-image', async () => { + const { deps } = makeDeps('linux', {}, { code: 0 }); + assert.deepStrictEqual(await readClipboardImage(deps), { kind: 'no-image' }); + }); + + test('linux non-zero exit → no-image', async () => { + const { deps } = makeDeps('linux', {}, { code: 1, stderr: 'target not available' }); + assert.deepStrictEqual(await readClipboardImage(deps), { kind: 'no-image' }); + }); + + test('macOS osascript reports NO_IMAGE → no-image', async () => { + const { deps, calls } = makeDeps('darwin', {}, { stdout: Buffer.from('NO_IMAGE\n'), code: 0 }); + assert.deepStrictEqual(await readClipboardImage(deps), { kind: 'no-image' }); + assert.strictEqual(calls[0].cmd, 'osascript'); + }); + + test('macOS osascript OK but temp file unwritable → no-image (graceful)', async () => { + // Fake osascript "succeeds" but no temp file was actually written. + const { deps } = makeDeps('darwin', {}, { stdout: Buffer.from('OK\n'), code: 0 }); + assert.deepStrictEqual(await readClipboardImage(deps), { kind: 'no-image' }); + }); + + test('windows PowerShell NO_IMAGE → no-image; spawns powershell.exe', async () => { + const { deps, calls } = makeDeps('win32', {}, { stdout: Buffer.from('NO_IMAGE'), code: 0 }); + assert.deepStrictEqual(await readClipboardImage(deps), { kind: 'no-image' }); + assert.strictEqual(calls[0].cmd, 'powershell.exe'); + }); +}); diff --git a/packages/vscode/src/test/view-diff.test.ts b/packages/vscode/src/test/view-diff.test.ts new file mode 100644 index 000000000..58c3e4155 --- /dev/null +++ b/packages/vscode/src/test/view-diff.test.ts @@ -0,0 +1,184 @@ +import * as assert from 'assert'; +import { + parseNameStatus, + normalizeNumstatPath, + parseBinaryPaths, + planResources, + encodeDiffQuery, + decodeDiffQuery, + diffUrisForChange, + DIFF_SCHEME, + type ChangeEntry, +} from '../commands/view-diff.js'; + +suite('view-diff parseNameStatus', () => { + test('parses modified / added / deleted', () => { + const out = parseNameStatus('M\tsrc/a.ts\nA\tsrc/b.ts\nD\tsrc/c.ts\n'); + assert.deepStrictEqual(out, [ + { status: 'M', oldPath: null, path: 'src/a.ts' }, + { status: 'A', oldPath: null, path: 'src/b.ts' }, + { status: 'D', oldPath: null, path: 'src/c.ts' }, + ]); + }); + + test('splits rename into old + new path', () => { + const [r] = parseNameStatus('R096\told/name.ts\tnew/name.ts'); + assert.deepStrictEqual(r, { status: 'R', oldPath: 'old/name.ts', path: 'new/name.ts' }); + }); + + test('splits copy into old + new path', () => { + const [c] = parseNameStatus('C100\tsrc/orig.ts\tsrc/copy.ts'); + assert.deepStrictEqual(c, { status: 'C', oldPath: 'src/orig.ts', path: 'src/copy.ts' }); + }); + + test('ignores blank lines and trailing CR', () => { + const out = parseNameStatus('M\tsrc/a.ts\r\n\nA\tsrc/b.ts\n'); + assert.strictEqual(out.length, 2); + assert.strictEqual(out[0].path, 'src/a.ts'); + }); +}); + +suite('view-diff normalizeNumstatPath', () => { + test('passes through a plain path', () => { + assert.strictEqual(normalizeNumstatPath('src/a.ts'), 'src/a.ts'); + }); + + test('resolves the braced rename form to the new path', () => { + assert.strictEqual(normalizeNumstatPath('src/{old => new}/f.ts'), 'src/new/f.ts'); + }); + + test('resolves the bare arrow rename form to the new path', () => { + assert.strictEqual(normalizeNumstatPath('old/path.ts => new/path.ts'), 'new/path.ts'); + }); +}); + +suite('view-diff parseBinaryPaths', () => { + test('collects only the -\\t- (binary) rows', () => { + const numstat = [ + '12\t3\tsrc/a.ts', + '-\t-\tassets/logo.png', + '0\t0\tsrc/empty.ts', + '-\t-\tsrc/{old => new}/icon.ico', + ].join('\n'); + const bin = parseBinaryPaths(numstat); + assert.ok(bin.has('assets/logo.png')); + assert.ok(bin.has('src/new/icon.ico')); + assert.ok(!bin.has('src/a.ts')); + assert.strictEqual(bin.size, 2); + }); +}); + +suite('view-diff planResources', () => { + const empty = new Set(); + + test('modified → base ↔ file', () => { + const c: ChangeEntry = { status: 'M', oldPath: null, path: 'src/a.ts' }; + const [p] = planResources([c], empty); + assert.deepStrictEqual(p, { + resourcePath: 'src/a.ts', + left: { kind: 'base', path: 'src/a.ts' }, + right: { kind: 'file', path: 'src/a.ts' }, + }); + }); + + test('added → empty ↔ file', () => { + const [p] = planResources([{ status: 'A', oldPath: null, path: 'n.ts' }], empty); + assert.deepStrictEqual(p.left, { kind: 'empty' }); + assert.deepStrictEqual(p.right, { kind: 'file', path: 'n.ts' }); + }); + + test('deleted → base ↔ empty', () => { + const [p] = planResources([{ status: 'D', oldPath: null, path: 'gone.ts' }], empty); + assert.deepStrictEqual(p.left, { kind: 'base', path: 'gone.ts' }); + assert.deepStrictEqual(p.right, { kind: 'empty' }); + }); + + test('renamed → old@base ↔ new file', () => { + const [p] = planResources( + [{ status: 'R', oldPath: 'old.ts', path: 'new.ts' }], + empty, + ); + assert.deepStrictEqual(p.left, { kind: 'base', path: 'old.ts' }); + assert.deepStrictEqual(p.right, { kind: 'file', path: 'new.ts' }); + assert.strictEqual(p.resourcePath, 'new.ts'); + }); + + test('binary file → binary placeholder on both sides', () => { + const [p] = planResources( + [{ status: 'M', oldPath: null, path: 'logo.png' }], + new Set(['logo.png']), + ); + assert.deepStrictEqual(p.left, { kind: 'binary' }); + assert.deepStrictEqual(p.right, { kind: 'binary' }); + }); + + test('renamed binary detected via old path', () => { + const [p] = planResources( + [{ status: 'R', oldPath: 'a.png', path: 'b.png' }], + new Set(['a.png']), + ); + assert.strictEqual(p.left.kind, 'binary'); + assert.strictEqual(p.right.kind, 'binary'); + }); +}); + +suite('view-diff diff-query codec', () => { + test('round-trips through encode/decode', () => { + const q = { wt: '/repo/.builders/0042', ref: 'abc123', path: 'src/x.ts' }; + assert.deepStrictEqual(decodeDiffQuery(encodeDiffQuery(q)), q); + }); + + test('preserves the empty/binary sentinels', () => { + assert.strictEqual(decodeDiffQuery(encodeDiffQuery({ wt: '', ref: '', path: 'p', empty: true })).empty, true); + assert.strictEqual(decodeDiffQuery(encodeDiffQuery({ wt: '', ref: '', path: 'p', binary: true })).binary, true); + }); +}); + +suite('view-diff diffUrisForChange', () => { + const ctx = { wt: '/repo/.builders/0042', ref: 'abc123' }; + + test('modified → codev-diff base ↔ file: worktree', () => { + const [plan] = planResources( + [{ status: 'M', oldPath: null, path: 'src/a.ts' }], + new Set(), + ); + const { left, right } = diffUrisForChange(plan, ctx); + assert.strictEqual(left.scheme, DIFF_SCHEME); + assert.deepStrictEqual(decodeDiffQuery(left.query), { + wt: ctx.wt, ref: ctx.ref, path: 'src/a.ts', + }); + assert.strictEqual(right.scheme, 'file'); + assert.strictEqual(right.fsPath, '/repo/.builders/0042/src/a.ts'); + }); + + test('added → empty-sentinel base ↔ file: worktree', () => { + const [plan] = planResources( + [{ status: 'A', oldPath: null, path: 'n.ts' }], + new Set(), + ); + const { left, right } = diffUrisForChange(plan, ctx); + assert.strictEqual(decodeDiffQuery(left.query).empty, true); + assert.strictEqual(right.scheme, 'file'); + }); + + test('deleted → base ↔ empty-sentinel', () => { + const [plan] = planResources( + [{ status: 'D', oldPath: null, path: 'gone.ts' }], + new Set(), + ); + const { left, right } = diffUrisForChange(plan, ctx); + assert.strictEqual(left.scheme, DIFF_SCHEME); + assert.strictEqual(decodeDiffQuery(left.query).path, 'gone.ts'); + assert.strictEqual(decodeDiffQuery(right.query).empty, true); + }); + + test('renamed → old@base ↔ new file', () => { + const [plan] = planResources( + [{ status: 'R', oldPath: 'old.ts', path: 'new.ts' }], + new Set(), + ); + const { left, right } = diffUrisForChange(plan, ctx); + assert.strictEqual(decodeDiffQuery(left.query).path, 'old.ts'); + assert.strictEqual(right.fsPath, '/repo/.builders/0042/new.ts'); + }); +}); diff --git a/packages/vscode/src/views/backlog.ts b/packages/vscode/src/views/backlog.ts index 217620a34..8e45f4a91 100644 --- a/packages/vscode/src/views/backlog.ts +++ b/packages/vscode/src/views/backlog.ts @@ -3,6 +3,16 @@ import type { OverviewBacklogItem } from '@cluesmith/codev-types'; import type { OverviewCache } from './overview-data.js'; import { BacklogTreeItem } from './backlog-tree-item.js'; +/** + * Backlog rows the user can act on — exclude issues that already have an + * active builder. Mirrors the dashboard's BacklogList + * (`items.filter(i => !i.hasBuilder)`) so the extension and web show the + * same "available work" set and you can't double-spawn from the Backlog. + */ +export function spawnableBacklog(items: OverviewBacklogItem[]): OverviewBacklogItem[] { + return items.filter(i => !i.hasBuilder); +} + /** * Backlog view: open GitHub issues with no PR yet. Issues assigned to the * current user (auto-detected via OverviewData.currentUser) sort to the @@ -34,8 +44,9 @@ export class BacklogProvider implements vscode.TreeDataProvider const isMine = (item: OverviewBacklogItem) => !!me && !!item.assignees?.some(a => a.toLowerCase() === me); - const mine = data.backlog.filter(isMine); - const rest = data.backlog.filter(item => !isMine(item)); + const items = spawnableBacklog(data.backlog); + const mine = items.filter(isMine); + const rest = items.filter(item => !isMine(item)); return [...mine, ...rest].map(item => { const assigned = mine.includes(item); diff --git a/packages/vscode/src/views/builder-diff-cache.ts b/packages/vscode/src/views/builder-diff-cache.ts new file mode 100644 index 000000000..4a48b6f02 --- /dev/null +++ b/packages/vscode/src/views/builder-diff-cache.ts @@ -0,0 +1,117 @@ +import * as vscode from 'vscode'; +import * as path from 'node:path'; +import { + getBuilderChanges, + planResources, + type ChangeEntry, + type ChangeStatus, + type ResourcePlan, +} from '../commands/view-diff.js'; + +/** + * A changed file paired with its diff plan. `planResources` maps + * `changes` 1:1 in order, so zipping by index is safe. + */ +export interface BuilderFileChange { + change: ChangeEntry; + plan: ResourcePlan; +} + +export interface BuilderDiffResult { + /** Merge-base SHA (or branch name) — passed to `diffUrisForChange`. */ + baseRef: string; + /** Empty when there are no changes. */ + files: BuilderFileChange[]; + /** Present when `git` failed; the tree shows a placeholder row. */ + error?: string; +} + +/** + * TTL cache around `getBuilderChanges` keyed by builder id, plus a global + * `fsPath → git status` registry that backs the SCM-style file decorations + * (colored status letter on each changed-file row). + * + * Why the TTL: VSCode re-queries an *expanded* tree node's children on + * every `onDidChangeTreeData` — which `BuildersProvider` fires on each SSE + * event and the 60s overview poll. Without a cache, every tick would spawn + * `git` for every expanded builder. The TTL caps that to ~1 spawn / + * interval / expanded builder; collapsed builders never call `getChildren`. + */ +export class BuilderDiffCache { + private readonly cache = new Map(); + + /** fsPath → status for every file currently shown across builders. */ + private readonly decorations = new Map(); + /** builderId → the fsPaths it contributed, so a recompute can replace them. */ + private readonly builderPaths = new Map(); + + private readonly _onDidChangeDecorations = new vscode.EventEmitter(); + /** Fires the affected file URIs whenever the decoration map changes. */ + readonly onDidChangeDecorations = this._onDidChangeDecorations.event; + + constructor(private readonly ttlMs = 15_000) {} + + /** Status for a file URI, or undefined if it isn't a tracked change. */ + decorationFor(uri: vscode.Uri): ChangeStatus | undefined { + return this.decorations.get(uri.fsPath); + } + + async getDiff(builderId: string, worktreePath: string): Promise { + const hit = this.cache.get(builderId); + if (hit && Date.now() - hit.ts < this.ttlMs) { + return hit.result; + } + + let result: BuilderDiffResult; + try { + const { baseRef, changes, binaryPaths } = await getBuilderChanges(worktreePath); + const plans = planResources(changes, binaryPaths); + result = { + baseRef, + files: changes.map((change, i) => ({ change, plan: plans[i]! })), + }; + } catch (error) { + result = { + baseRef: '', + files: [], + error: error instanceof Error ? error.message : String(error), + }; + } + + this.cache.set(builderId, { ts: Date.now(), result }); + this.syncDecorations(builderId, worktreePath, result); + return result; + } + + /** Drop a builder's cached diff + decorations (e.g. after cleanup). */ + invalidate(builderId: string): void { + this.cache.delete(builderId); + this.syncDecorations(builderId, '', { baseRef: '', files: [] }); + } + + /** + * Replace this builder's contribution to the decoration map and notify + * listeners of every affected URI (old ∪ new) so VSCode re-queries them. + */ + private syncDecorations(builderId: string, worktreePath: string, result: BuilderDiffResult): void { + const oldPaths = this.builderPaths.get(builderId) ?? []; + for (const p of oldPaths) { this.decorations.delete(p); } + + const newPaths: string[] = []; + for (const f of result.files) { + const fsPath = vscode.Uri.file(path.join(worktreePath, f.change.path)).fsPath; + this.decorations.set(fsPath, f.change.status); + newPaths.push(fsPath); + } + this.builderPaths.set(builderId, newPaths); + + const affected = [...new Set([...oldPaths, ...newPaths])]; + if (affected.length > 0) { + this._onDidChangeDecorations.fire(affected.map(p => vscode.Uri.file(p))); + } + } + + dispose(): void { + this._onDidChangeDecorations.dispose(); + } +} diff --git a/packages/vscode/src/views/builder-file-decoration.ts b/packages/vscode/src/views/builder-file-decoration.ts new file mode 100644 index 000000000..213ecc4f3 --- /dev/null +++ b/packages/vscode/src/views/builder-file-decoration.ts @@ -0,0 +1,47 @@ +import * as vscode from 'vscode'; +import type { ChangeStatus } from '../commands/view-diff.js'; +import type { BuilderDiffCache } from './builder-diff-cache.js'; + +/** + * SCM-style decorations for builder changed-file rows: a colored one-letter + * status badge plus a label tint, exactly like VSCode's built-in Git + * decorator (which can't see the gitignored `.builders/` worktrees, so we + * provide our own). The native file-type icon is preserved because a + * `FileDecoration` only adds a badge/color — it never replaces the icon. + * + * Status → letter / theme color follows the Git extension's convention so + * the user's theme drives the exact colors. + */ + +interface Deco { badge: string; color: string; label: string; } + +const DECO: Record = { + A: { badge: 'A', color: 'gitDecoration.addedResourceForeground', label: 'Added' }, + M: { badge: 'M', color: 'gitDecoration.modifiedResourceForeground', label: 'Modified' }, + D: { badge: 'D', color: 'gitDecoration.deletedResourceForeground', label: 'Deleted' }, + R: { badge: 'R', color: 'gitDecoration.renamedResourceForeground', label: 'Renamed' }, + C: { badge: 'C', color: 'gitDecoration.renamedResourceForeground', label: 'Copied' }, + T: { badge: 'T', color: 'gitDecoration.modifiedResourceForeground', label: 'Type changed' }, + U: { badge: '!', color: 'gitDecoration.conflictingResourceForeground', label: 'Unmerged' }, +}; + +export class BuilderFileDecorationProvider implements vscode.FileDecorationProvider { + readonly onDidChangeFileDecorations: vscode.Event; + + constructor(private readonly cache: BuilderDiffCache) { + this.onDidChangeFileDecorations = cache.onDidChangeDecorations; + } + + provideFileDecoration(uri: vscode.Uri): vscode.FileDecoration | undefined { + const status = this.cache.decorationFor(uri); + if (!status) { return undefined; } + const d = DECO[status] ?? DECO.M; + return { + badge: d.badge, + color: new vscode.ThemeColor(d.color), + tooltip: d.label, + // Don't tint the parent builder row / ancestor folders. + propagate: false, + }; + } +} diff --git a/packages/vscode/src/views/builder-file-tree-item.ts b/packages/vscode/src/views/builder-file-tree-item.ts new file mode 100644 index 000000000..a33b8e8f7 --- /dev/null +++ b/packages/vscode/src/views/builder-file-tree-item.ts @@ -0,0 +1,58 @@ +import * as vscode from 'vscode'; +import * as path from 'node:path'; +import type { ChangeEntry, ChangeStatus, ResourcePlan } from '../commands/view-diff.js'; + +/** + * Second-level tree row: one changed file under a builder in the Builders + * view. Carries the typed fields the `codev.openBuilderFileDiff` handler + * needs (it receives the item itself, like the backlog/builder rows, and + * narrows via `instanceof`). + * + * `plan` (left/right `SideSpec`) feeds `diffUrisForChange`; `change` + * carries the git status (used for the tooltip + rename source). The + * status *icon* is deliberately NOT set here — `resourceUri` lets the + * file-type icon show, and `BuilderFileDecorationProvider` adds the + * SCM-style colored status-letter badge, mirroring VSCode's Git decorator. + * + * Used by views/builders.ts. + */ + +const STATUS_LABEL: Record = { + A: 'Added', + D: 'Deleted', + R: 'Renamed', + C: 'Copied', + M: 'Modified', + T: 'Type changed', + U: 'Unmerged', +}; + +export class BuilderFileTreeItem extends vscode.TreeItem { + constructor( + public readonly builderId: string, + public readonly worktreePath: string, + public readonly baseRef: string, + public readonly change: ChangeEntry, + public readonly plan: ResourcePlan, + ) { + const rel = plan.resourcePath; + super(path.basename(rel)); + + const dir = path.dirname(rel); + const dirLabel = dir === '.' ? '' : dir; + this.description = + change.status === 'R' && change.oldPath + ? `${dirLabel ? dirLabel + ' ' : ''}↤ ${change.oldPath}` + : dirLabel; + + // resourceUri → native file-type icon + the decoration-provider badge. + this.resourceUri = vscode.Uri.file(path.join(worktreePath, rel)); + this.tooltip = `${STATUS_LABEL[change.status] ?? 'Changed'} · ${rel}`; + this.contextValue = 'builder-file'; + this.command = { + command: 'codev.openBuilderFileDiff', + title: 'Open Diff', + arguments: [this], + }; + } +} diff --git a/packages/vscode/src/views/builders.ts b/packages/vscode/src/views/builders.ts index 28c4f5ca3..b41567e98 100644 --- a/packages/vscode/src/views/builders.ts +++ b/packages/vscode/src/views/builders.ts @@ -2,6 +2,8 @@ import * as vscode from 'vscode'; import type { OverviewBuilder } from '@cluesmith/codev-types'; import type { OverviewCache } from './overview-data.js'; import { BuilderTreeItem } from './builder-tree-item.js'; +import { BuilderFileTreeItem } from './builder-file-tree-item.js'; +import type { BuilderDiffCache } from './builder-diff-cache.js'; /** * Order builders for the Builders tree: blocked first with the longest- @@ -29,7 +31,10 @@ export class BuildersProvider implements vscode.TreeDataProvider(); readonly onDidChangeTreeData = this.changeEmitter.event; - constructor(private cache: OverviewCache) { + constructor( + private cache: OverviewCache, + private readonly diffCache: BuilderDiffCache, + ) { cache.onDidChange(() => this.changeEmitter.fire()); } @@ -37,7 +42,23 @@ export class BuildersProvider implements vscode.TreeDataProvider { + // Second level: a builder's changed files. VSCode only calls this for + // an *expanded* builder, so collapsed builders cost no git. + if (element instanceof BuilderTreeItem) { + return this.fileChildren(element.builderId); + } + // File rows are leaves. + if (element instanceof BuilderFileTreeItem) { + return []; + } + // Root: the builder list. const data = this.cache.getData(); if (!data) { return []; } @@ -48,15 +69,26 @@ export class BuildersProvider implements vscode.TreeDataProvider { + const builder = this.cache.getData()?.builders.find(b => b.id === builderId); + if (!builder?.worktreePath) { + return [placeholder('No worktree on record')]; + } + + const result = await this.diffCache.getDiff(builderId, builder.worktreePath); + if (result.error) { + const row = placeholder('Diff unavailable'); + row.tooltip = result.error; + return [row]; + } + if (result.files.length === 0) { + return [placeholder('No changes yet')]; + } + return result.files.map( + f => new BuilderFileTreeItem(builderId, builder.worktreePath, result.baseRef, f.change, f.plan), + ); + } +} + +/** Non-clickable informational leaf (no worktree / no changes / error). */ +function placeholder(label: string): vscode.TreeItem { + const item = new vscode.TreeItem(label); + item.contextValue = 'builder-file-none'; + item.iconPath = new vscode.ThemeIcon('circle-slash', new vscode.ThemeColor('disabledForeground')); + return item; } function timeSince(isoDate: string): string {