fix(workbench): keep git checkouts within Git for Windows' GIT_DIR limit

README.md

@@ -692,7 +692,7 @@ Supported values:
 | `review_policy.major_event` | `comment`, `request_changes`. Sets the review event when the highest severity is major. Blocking findings always request changes, a major finding never approves, and the rollup model's proposed event cannot override either rule. Under `request_changes`, the rollup model may soften a major-only review to a comment. |
 | `review_policy.resolve_threads` | `auto`, `never` |
 | `data.retention.enforcement` | `at_write` applies review-time pruning before each `cr review`; `manual_only` disables review-time pruning and leaves `cr data prune` as the explicit maintenance path. Review-time pruning runs only when no other cr instance is mid-run (it requires the data-root active-runs lock exclusively) and is skipped, not queued, on contention. |
-| `data.keep_workbench` | `true` retains the run checkout under `workbench/` after a successful run. Failed and errored runs always retain it. |
+| `data.keep_workbench` | `true` retains the run workbench (`workbench/` and its checkout under the data root's `ws/`) after a successful run. Failed and errored runs always retain it. |
 
 `subscription` LLM auth means the adapter owns its own credentials, such as a
 logged-in CLI or local runtime. `api_key` LLM auth requires `llm.credential`
@@ -1716,8 +1716,11 @@ command remains the safe preview path for manual maintenance, and no-selector
 `cr data prune` uses its built-in live 90-day and dry-run 7-day windows
 independent from profile config.
 
-Successful runs delete their run checkout (`workbench/`) once the run reaches a
-successful terminal state. Failed and errored runs retain it for inspection, and
+Successful runs delete their run checkout (`workbench/`, plus the git checkout
+kept under the data root's `ws/` directory) once the run reaches a successful
+terminal state. Git checkouts live in `ws/` rather than inside the run directory
+so their `.git` paths fit Git for Windows' 220-character limit; `cr data prune`
+also removes any checkout whose run is gone. Failed and errored runs retain it for inspection, and
 `data.keep_workbench: true` retains it for successful runs too. `cr review
 --keep-workbench` overrides the config for one invocation; an explicit
 `--keep-workbench=false` forces deletion even when `data.keep_workbench: true`.

docs/checkout-native-review-contract.md

@@ -43,9 +43,10 @@ dossier assembly, and the dossier is assembled before orchestrator selection.
 
 ## Artifact Model
 
-All checkout-native review artifacts are owned by the review run and must live
+All checkout-native review artifacts are owned by the review run and live
 under the run artifact directory so existing retention, `cr data prune`, and
-`cr data purge` remove them automatically.
+`cr data purge` remove them automatically. The one exception is git checkouts,
+which live in the flat workspaces root described below.
 
 The target layout is:
 
@@ -63,12 +64,6 @@ runs/<run-id>/
     final/
     index.json
   workbench/
-    repo/
-    reviewers/
-      <reviewer-id>/
-        repo/
-      <reviewer-id>-coverage-repair/
-        repo/
     scratch/
       <reviewer-id>/
         cache/
@@ -77,8 +72,38 @@ runs/<run-id>/
         cache/
         tmp/
     metadata.json
+
+ws/<key>/
+  r/            git checkout
+  owner.json    owning run directory, role, and agent ID
 ```
 
+Git checkouts (the run workbench repo and each reviewer clone) live under
+`<data>/ws/<key>/r` beside `runs/`, not inside the run directory. Git for
+Windows refuses a `GIT_DIR` longer than 220 characters (its `PATH_MAX` of 260
+minus the 40 that `setup.c` reserves), and a transport clone hands `GIT_DIR` to
+its `index-pack` child, so a checkout nested in the run tree failed with
+`'$GIT_DIR' too big` for longer repository or reviewer names. A workspace's
+`.git` path is the data root plus a fixed 23 characters, whatever the PR or
+reviewer. `<key>` is a 12-character hash of the run directory, role
+(`workbench` or `reviewer`), and agent ID, so a resumed run finds the same
+workspaces and no two runs or reviewers share one. On Windows, workspace
+preparation checks the `.git` length before running git and fails with the fix
+when the data root itself is too long. Checkouts set `core.longpaths=true` so
+Git for Windows can also check out files whose full path passes 260
+characters; other platforms ignore the setting.
+
+`cr data prune` removes a workspace whose `owner.json` names a run directory
+that no longer exists (including runs that prune itself deletes), and one with
+no `owner.json` at all once it is older than the orphan grace period. An
+`owner.json` that exists but cannot be read or parsed proves nothing about
+ownership: prune keeps that workspace and reports a warning rather than risk
+deleting a live run's checkout.
+`cr data purge` removes the whole data root, workspaces included. Run
+directories outside the standard `<data>/runs/.../run-<id>` layout, such as
+caller-owned benchmark directories, keep their checkouts under `workbench/` as
+before.
+
 Notes:
 
 - `llm-tasks/` keeps the existing durable structured-task artifacts.
@@ -87,24 +112,24 @@ Notes:
 - `dossier/summary/` holds durable normalized discussion artifacts.
 - `dossier/final/` holds the reviewer-facing dossier files used by the
   orchestrator and specialists.
-- `workbench/repo/` is a clean pinned checkout at the PR head SHA, carrying
-  `refs/heads/cr-review-head` at that SHA. The ref is load-bearing: the
+- The workbench checkout (`ws/<key>/r`, role `workbench`) is a clean pinned
+  checkout at the PR head SHA, carrying `refs/heads/cr-review-head` at that SHA. The ref is load-bearing: the
   per-reviewer workspace is created with `git clone` from this directory, and
   git does not treat a directory without `refs/` as a repository. Only the head
   is given a ref, so the base commit is not transferred into reviewer
   workspaces when base is not an ancestor of head; reviewers are handed the
   provider-generated `diff.patch` and nothing in the pipeline resolves the base
   SHA inside the workspace. That is a decision, not an oversight -- add
   `refs/heads/cr-review-base` if a reviewer ever needs `git log base..HEAD`.
-- `workbench/reviewers/<reviewer-id>/repo/` is a disposable reviewer checkout.
-  Clone it through Git transport (`--no-local`) rather than copying the
+- Each reviewer's `ws/<key>/r` (role `reviewer`) is a disposable reviewer
+  checkout. Clone it through Git transport (`--no-local`) rather than copying the
   canonical object directory: background repacking can replace pack files
   during a local-copy clone. Each checkout has its own object database and
   receives only history reachable from the canonical refs.
 - `workbench/scratch/<reviewer-id>/` holds reviewer-owned scratch, temp, and
   cache roots.
 - A coverage-repair pass runs under the derived identity
-  `<reviewer-id>-coverage-repair`, so a repaired run also has that reviewer
+  `<reviewer-id>-coverage-repair`, so a repaired run also has its own reviewer
   checkout, that scratch root, and its own encoded agent log. Workspace
   preparation clears the directory it is given, so the repair must never reuse
   the primary reviewer's identity.
@@ -117,8 +142,8 @@ Notes:
 The workbench is run-owned, not cache-owned. Shared clone or fetch caches are a
 possible future optimization but are not part of the correctness contract.
 
-A successful run removes its `workbench/` tree when it reaches a successful
-terminal state, after rollup and plan build, so retention no longer pins a full
+A successful run removes its `workbench/` tree and its workbench checkout when
+it reaches a successful terminal state, after rollup and plan build, so retention no longer pins a full
 checkout per run. A live run reaches that state when its outbox post succeeds;
 failed, aborted, and incomplete runs retain the workbench for inspection, and
 `data.keep_workbench: true` opts a run back into retention on success. The

docs/review-guidance.md

@@ -108,16 +108,21 @@ review run artifact root:
 runs/<run-id>/
   dossier/
   workbench/
+ws/<key>/
+  r/
+  owner.json
 ```
 
-Because those directories live under the run artifact root, existing data
-lifecycle commands clean them up automatically:
+Git checkouts live in `ws/` rather than the run directory so their `.git` paths
+stay within Git for Windows' 220-character limit; each records its owning run.
+Data lifecycle commands clean both up:
 
-- `cr data prune` removes dossier and workbench directories for pruned runs
-- `cr data purge` removes the entire data root, including dossier and workbench
-  artifacts
+- `cr data prune` removes dossier and workbench directories for pruned runs,
+  and any workspace whose owning run directory is gone
+- `cr data purge` removes the entire data root, including dossier, workbench,
+  and workspace artifacts
 
-A successful run also deletes its own `workbench/` directory once the run
+A successful run also deletes its own `workbench/` directory and checkout once the run
 reaches a successful terminal state, so dossier data is the durable artifact that
 remains. Failed and errored runs always retain the workbench for inspection, and
 `data.keep_workbench: true` opts successful runs back into retention.

internal/datalifecycle/datalifecycle.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/open-cli-collective/codereview-cli/internal/ledger"
+	"github.com/open-cli-collective/codereview-cli/internal/runartifact"
 	"github.com/open-cli-collective/codereview-cli/internal/runlock"
 	"github.com/open-cli-collective/codereview-cli/internal/statepaths"
 )
@@ -135,6 +136,11 @@ func Show(ctx context.Context, opts Options) (Stats, error) {
 	if err != nil {
 		return Stats{}, err
 	}
+	workspaceOrphans, _, err := orphanWorkspaces(opts.Layout, opts.now().Add(-OrphanGrace), nil)
+	if err != nil {
+		return Stats{}, err
+	}
+	orphanItems = append(orphanItems, workspaceOrphans...)
 	stats := Stats{
 		DataRoot:      opts.Layout.DataRoot,
 		LedgerPath:    opts.Layout.LedgerDB(),
@@ -165,6 +171,11 @@ func Show(ctx context.Context, opts Options) (Stats, error) {
 	if err != nil {
 		return Stats{}, err
 	}
+	workspaceBytes, err := dirBytes(opts.Layout.WorkspacesRoot())
+	if err != nil {
+		return Stats{}, err
+	}
+	stats.ArtifactBytes += workspaceBytes
 	for _, orphan := range orphanItems {
 		stats.OrphanCount++
 		stats.OrphanBytes += orphan.Bytes
@@ -232,8 +243,22 @@ func Prune(ctx context.Context, opts Options, prune PruneOptions) (PruneResult,
 			orphanSpan.End(err)
 			return result, err
 		}
+		// Workspaces whose run this prune would delete are reported with it.
+		removing := map[string]bool{}
+		for _, run := range selected {
+			removing[filepath.Clean(run.ArtifactPath)] = true
+		}
+		for _, orphan := range orphanItems {
+			removing[filepath.Clean(orphan.Path)] = true
+		}
+		workspaceOrphans, warnings, err := orphanWorkspaces(opts.Layout, opts.now().Add(-OrphanGrace), removing)
+		if err != nil {
+			orphanSpan.End(err)
+			return result, err
+		}
+		result.Warnings = append(result.Warnings, warnings...)
 		orphanSpan.End(nil)
-		result.OrphansRemoved = orphanItems
+		result.OrphansRemoved = append(orphanItems, workspaceOrphans...)
 		return result, nil
 	}
 
@@ -275,6 +300,21 @@ func Prune(ctx context.Context, opts Options, prune PruneOptions) (PruneResult,
 		}
 		result.OrphansRemoved = append(result.OrphansRemoved, orphan)
 	}
+	// Run directories are gone by now, so this also catches the workspaces of
+	// every run deleted above.
+	workspaceOrphans, warnings, err := orphanWorkspaces(opts.Layout, opts.now().Add(-OrphanGrace), nil)
+	if err != nil {
+		orphanSpan.End(err)
+		return result, err
+	}
+	result.Warnings = append(result.Warnings, warnings...)
+	for _, orphan := range workspaceOrphans {
+		if err := opts.removeAll()(orphan.Path); err != nil && !errors.Is(err, fs.ErrNotExist) {
+			result.Warnings = append(result.Warnings, fmt.Sprintf("failed to remove orphan workspace at %s: %v", orphan.Path, err))
+			continue
+		}
+		result.OrphansRemoved = append(result.OrphansRemoved, orphan)
+	}
 	orphanSpan.End(nil)
 	_ = removeEmptyParents(runsRoot(opts.Layout))
 	return result, nil
@@ -462,6 +502,74 @@ func orphanItems(layout statepaths.Layout, runs []ledger.Run, graceCutoff time.T
 	return items, nil
 }
 
+// orphanWorkspaces lists git workspaces whose owning run directory no longer
+// exists or is in removing. A workspace with no owner record (a run that
+// stopped between creating the directory and recording its owner) is swept
+// once it is older than graceCutoff. An owner record that exists but cannot be
+// read or parsed proves nothing about ownership, so that workspace is kept and
+// reported in the returned warnings. Symlinked entries are never followed or
+// removed.
+func orphanWorkspaces(layout statepaths.Layout, graceCutoff time.Time, removing map[string]bool) ([]OrphanItem, []string, error) {
+	root := layout.WorkspacesRoot()
+	if err := rejectSymlink(root); err != nil {
+		return nil, nil, err
+	}
+	entries, err := os.ReadDir(root)
+	if errors.Is(err, fs.ErrNotExist) {
+		return nil, nil, nil
+	}
+	if err != nil {
+		return nil, nil, err
+	}
+	var items []OrphanItem
+	var warnings []string
+	for _, entry := range entries {
+		if !entry.IsDir() || entry.Type()&fs.ModeSymlink != 0 {
+			continue
+		}
+		path := filepath.Join(root, entry.Name())
+		orphaned, err := workspaceOrphaned(path, entry, graceCutoff, removing)
+		if err != nil {
+			warnings = append(warnings, fmt.Sprintf("kept workspace %s: cannot determine its owner: %v", path, err))
+			continue
+		}
+		if !orphaned {
+			continue
+		}
+		bytes, err := dirBytes(path)
+		if err != nil {
+			return nil, nil, err
+		}
+		items = append(items, OrphanItem{Path: path, Bytes: bytes})
+	}
+	sort.Slice(items, func(i, j int) bool { return items[i].Path < items[j].Path })
+	return items, warnings, nil
+}
+
+func workspaceOrphaned(path string, entry fs.DirEntry, graceCutoff time.Time, removing map[string]bool) (bool, error) {
+	owner, err := runartifact.ReadWorkspaceOwner(path)
+	if errors.Is(err, fs.ErrNotExist) {
+		info, infoErr := entry.Info()
+		if infoErr != nil {
+			return false, infoErr
+		}
+		return !info.ModTime().After(graceCutoff), nil
+	}
+	if err != nil {
+		return false, err
+	}
+	runDir := filepath.Clean(owner.RunDir)
+	if removing[runDir] {
+		return true, nil
+	}
+	if _, err := os.Stat(runDir); errors.Is(err, fs.ErrNotExist) {
+		return true, nil
+	} else if err != nil {
+		return false, err
+	}
+	return false, nil
+}
+
 func referencedArtifacts(layout statepaths.Layout, runs []ledger.Run) (map[string]bool, error) {
 	referenced := map[string]bool{}
 	for _, run := range runs {