feat: NOUS_CAMPAIGN_PARENT env var relocates campaign artifacts outside target repo (#239) (#240)

* feat: NOUS_CAMPAIGN_PARENT env var relocates campaign artifacts outside target repo (#239)

Closes #239.

Today's friction: campaign work_dirs default to `<target_repo>/.nous/<run_id>/`,
where they live as **untracked files** in the target repo's working tree.
This causes:

- `git stash -u` silently captures multi-GB of campaign output (recently
  caused 4.7 GB of ea-control-stack JSONs to be stashed; recovery required
  digging through `stash@{0}^3` and unstaging 60+ files)
- `git status` clutter from thousands of untracked entries
- `git add .` and `git checkout <commit> -- <path>` silently stage
  campaign content
- PR review noise

Fix:
1. Add `NOUS_CAMPAIGN_PARENT` env var. When set, work_dir lives at
   `$NOUS_CAMPAIGN_PARENT/<run_id>/`, fully outside the target.
2. Add `work_dir` field to state.json — per-campaign source of truth
   for location, robust to env var changes between runs.
3. Worktrees are NOT affected — they still live at
   `<target>/.nous-experiments/<run>/<arm>/` per #133, because they
   ARE code FOR the target repo and must share its git history.

Changes:
- New `orchestrator/work_dir_resolver.py` (73 LOC): single source of
  truth for "where would this run_id live, given today's environment?"
- `orchestrator/iteration.py:setup_work_dir` uses the resolver and
  records the resolved absolute path in state.json's new `work_dir`
  field.
- `orchestrator/cli.py:resolve_work_dir` and `_cmd_run` use the same
  resolver — keeps all three call sites consistent (avoids the silent
  "work_dir mismatch" trap from #184).
- `orchestrator/templates/state.json` adds `work_dir: null` (set on
  setup).
- `orchestrator/schemas/state.schema.json` extends to permit `work_dir`.
- `orchestrator/create_campaign.py:_TEMPLATE` documents the env var
  for new campaigns.
- `README.md` adds a recommended-setup section.
- `tests/test_work_dir_resolver.py`: 12 behavioral tests covering both
  env-var-set and env-var-unset paths, state.json record robustness,
  expanduser, idempotency. No live LLM calls (per CLAUDE.md).

Backward-compat: when `NOUS_CAMPAIGN_PARENT` is unset, behavior is
byte-identical to today's legacy `<repo>/.nous/<run_id>/`. All 1,195
existing tests still pass.

Out of scope (per-user / per-campaign one-time work):
- Migrating existing campaigns: `mv <target>/.nous/<name> $NOUS_CAMPAIGN_PARENT/<name>`
- Updating per-campaign internal scripts to self-locate via __file__

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Address PR review: harden NOUS_CAMPAIGN_PARENT resolver + close in-progress detection trap (#239)

Comprehensive fixup addressing all 12 issues identified by review agents.

CRITICAL fixes:
- C1 / silent-failure-hunter C3: `_cmd_run` now uses
  `find_existing_work_dir` to check BOTH legacy and env-var locations
  for in-progress detection. Fixes the regression where toggling
  NOUS_CAMPAIGN_PARENT between runs could silently allow a parallel
  run that corrupts shared worktrees.
- C2 / silent-failure-hunter C1: empty/whitespace `NOUS_CAMPAIGN_PARENT`
  now raises ValueError instead of silently falling through to legacy.
  Bash `export NOUS_CAMPAIGN_PARENT=$UNSET` is a common typo that the
  user explicitly tried to opt out of.
- C3 / pr-test-analyzer gap #2: `tests/conftest.py` autouse fixture
  now strips `NOUS_CAMPAIGN_PARENT` so existing tests don't fail
  for contributors who follow README's recommendation to export it.
- C4 / comment-analyzer #1: README "byte-identical" claim corrected;
  scope tightened to "the resolved work_dir is unchanged".

IMPORTANT fixes:
- I1 / pr-test-analyzer gap #1: 4 new CLI tests cover yaml + bare-run-id
  with env-var, including the test that would have caught C1.
- I2 / comment-analyzer #3 / silent-failure-hunter C4: README adds
  migration command (`mv <repo>/.nous/<run> $NOUS_CAMPAIGN_PARENT/<run>`).
  `_cmd_run` emits a migration hint when a campaign is found at the
  legacy path while env var is set.
- I3 / silent-failure-hunter I5: state.json's `work_dir` field is now
  read by `find_existing_work_dir` — prefers the recorded path when
  it points to an existing campaign elsewhere (handles post-creation
  `mv`).
- I4 / comment-analyzer #10: `cli.resolve_work_dir` legacy branch
  delegates to the resolver via `find_existing_work_dir` instead of
  reimplementing path logic.
- I5 / silent-failure-hunter I1: `repo_path` validated for existence
  in resolver; raises FileNotFoundError on typos.
- I6 / code-reviewer #2: explicit `Path(...)` wrap restored at CLI
  boundary for type robustness.
- I7 / pr-test-analyzer gap #3: schema validator regression test
  added; state.json shape verified post-setup.
- I8 / silent-failure-hunter D1: collision detection. `setup_work_dir`
  refuses to clobber a same-named campaign at the env-var path that
  records a different `repo_path`. Adds `repo_path` field to state.json
  schema.

Suggestion fixes:
- comment-analyzer #2: Resolution rules consolidated in
  `work_dir_resolver.py:RESOLUTION RULES` marker comment; other
  docstrings reference it. Reduces drift across 5 places.
- comment-analyzer #5: schema description tightened (drop
  implementation detail; mark fields as machine-local).
- comment-analyzer #7: `_TEMPLATE` bullet structure restructured for
  unambiguous conditional reading.
- comment-analyzer #4: cross-machine portability story documented
  in schema descriptions (machine-local with caveat).
- comment-analyzer #8: README line 166 (post-#239) updated to
  reflect the conditional default.
- silent-failure-hunter I2: setup_work_dir wraps mkdir with OSError
  surfacing env-var context.
- silent-failure-hunter I4: `_find_repo_root` failure message now
  mentions `NOUS_CAMPAIGN_PARENT` as an alternative.
- pr-test-analyzer suggestions: edge-case tests added (whitespace
  env var, trailing slash, env-var-change scenario, corrupt state.json,
  recorded-path-wins, recorded-path-nonexistent-falls-back).

Schema additions:
- `state.json` gains `work_dir` and `repo_path` fields (both
  nullable strings; absolute paths recorded at setup_work_dir).
- `state.schema.json` extends with both fields and machine-local
  caveats in descriptions.

Test count:
- Before: 12 tests (work_dir_resolver only)
- After: 32 tests covering resolver + setup + CLI + schema validation
- Full suite: 1,215 pass, 2 skip (was 1,195/2/0 — 20 new tests, 0 regressions).

Backward compatibility preserved:
- `NOUS_CAMPAIGN_PARENT` unset → resolved path is unchanged
- Pre-#239 campaigns at `<repo>/.nous/<run>/` continue to be found
  by `find_existing_work_dir` even when env var is set (migration
  grace).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: sriumcp

README.md

@@ -79,6 +79,30 @@ export OPENAI_BASE_URL=https://your-litellm-proxy.example.com  # or any OpenAI-c
 
 If you're using Anthropic directly via a LiteLLM proxy, point both vars at the proxy. If these aren't set, gate summaries and report generation are skipped (non-fatal). The campaign still runs — you just won't get LLM-generated summaries at the gates or a final report.
 
+**Recommended: relocate campaign artifacts outside the target repo (#239).**
+
+By default, Nous creates each campaign's working directory at `<target_repo>/.nous/<run_id>/`. That puts campaign output (state, ledger, principles, JSON results, findings) inside the target repo's working tree as untracked files — which means `git stash -u` silently captures them, `git status` shows thousands of unrelated entries, and `git add .` accidentally stages campaign content. To avoid this, set:
+
+```bash
+# Add to your shell rc (.zshrc / .bashrc):
+export NOUS_CAMPAIGN_PARENT=~/Documents/Projects/nous-campaigns
+```
+
+When `NOUS_CAMPAIGN_PARENT` is set, campaign artifacts live at `$NOUS_CAMPAIGN_PARENT/<run_id>/` — wholly outside the target repo. Code worktrees (per-arm BLIS branches, #133) continue to live at `<target>/.nous-experiments/<run_id>/<arm>/` because they ARE code FOR the target. The target repo's working tree stays clean. Backward-compat: when the env var is unset, the resolved `work_dir` is unchanged — `<repo_path>/.nous/<run_id>/`, exactly as today.
+
+The resolved absolute `work_dir` and `repo_path` are recorded in each campaign's `state.json` so the campaign location and target survive env-var changes between runs. `nous resume` and `nous status` look up campaigns at both the env-var location AND the legacy location, so existing pre-#239 campaigns continue to work without immediate migration.
+
+**Migrating existing campaigns** to the new location is a one-time operation per campaign:
+
+```bash
+# 1. Set the env var first (in your shell rc).
+# 2. Move existing campaign(s):
+mv <target_repo>/.nous/<run_id> $NOUS_CAMPAIGN_PARENT/<run_id>
+# 3. Continue using the campaign as before. Nous will find it at the new location.
+```
+
+state.json's recorded `work_dir` will be stale until the campaign's next setup; that's fine — `find_existing_work_dir` checks the actual directory existence, not just state.json's recorded path.
+
 ### 1. Install Nous
 
 ```bash
@@ -139,7 +163,7 @@ target_system:
   repo_path: /path/to/your/repo
 ```
 
-When `repo_path` is set, the campaign directory is created inside the target repo at `.nous/<run_id>/`. All artifacts live there.
+When `repo_path` is set, the campaign directory is created at `$NOUS_CAMPAIGN_PARENT/<run_id>/` if you've set that env var (recommended — see [Environment setup](#environment-setup) above), or otherwise at the legacy `<target_repo>/.nous/<run_id>/`. All artifacts live there.
 
 To discover the full schema (required vs optional fields, descriptions
 verbatim from the schema source), run:

orchestrator/cli.py

@@ -14,11 +14,33 @@ def _find_repo_root(start=None):
         if parent == current:
             break
         current = parent
-    print("Could not find .nous/ directory in any parent", file=sys.stderr)
+    print(
+        f"Could not find .nous/ directory in any parent of {Path.cwd()}. "
+        f"Either run from inside the target repo, pass an explicit "
+        f"work_dir path, or set NOUS_CAMPAIGN_PARENT (#239).",
+        file=sys.stderr,
+    )
     sys.exit(1)
 
 
 def resolve_work_dir(target):
+    """Resolve a CLI ``target`` (yaml path | dir | run_id) to a work_dir.
+
+    Honors NOUS_CAMPAIGN_PARENT (#239) and finds existing campaigns
+    that may live at the legacy ``<repo>/.nous/<run_id>/`` path even
+    when the env var is set (so users with pre-#239 campaigns can still
+    run ``nous status`` / ``nous resume`` without first migrating).
+
+    For an explicit dir target with state.json, the dir is taken at
+    face value.
+    """
+    import os
+
+    from orchestrator.work_dir_resolver import (
+        ENV_VAR,
+        find_existing_work_dir,
+    )
+
     if target.endswith(".yaml") or target.endswith(".yml"):
         p = Path(target)
         if not p.exists():
@@ -33,12 +55,32 @@ def resolve_work_dir(target):
             print(f"Campaign file {target} is empty or not a YAML mapping", file=sys.stderr)
             sys.exit(1)
         try:
-            repo_path = Path(data["target_system"]["repo_path"])
+            # Wrap in Path() for type-robustness: surfaces a clear error
+            # immediately if repo_path is the wrong type (e.g. an int
+            # from a hand-edited yaml) rather than failing later in the
+            # resolver with a less helpful message.
+            repo_path = (
+                Path(data["target_system"]["repo_path"])
+                if data["target_system"].get("repo_path") is not None
+                else None
+            )
             run_id = data["run_id"]
         except (KeyError, TypeError) as exc:
             print(f"Campaign file {target} missing required field: {exc}", file=sys.stderr)
             sys.exit(1)
-        work_dir = repo_path / ".nous" / run_id
+        try:
+            work_dir = find_existing_work_dir(run_id, repo_path)
+        except (ValueError, FileNotFoundError) as exc:
+            print(f"Campaign location resolution failed: {exc}", file=sys.stderr)
+            sys.exit(1)
+        if work_dir is None:
+            print(
+                f"Work directory not found for run_id={run_id!r} "
+                f"(checked {ENV_VAR} location and "
+                f"{repo_path!s}/.nous/{run_id}/ if applicable).",
+                file=sys.stderr,
+            )
+            sys.exit(1)
         return work_dir
 
     p = Path(target)
@@ -50,10 +92,29 @@ def resolve_work_dir(target):
         sys.exit(1)
 
     run_id = target
-    root = _find_repo_root()
-    work_dir = root / ".nous" / run_id
-    if not work_dir.is_dir():
-        print(f"Work directory not found: {work_dir}", file=sys.stderr)
+    # Bare run_id: prefer find_existing_work_dir (env-var path), then
+    # fall back to CWD-walk for the legacy invocation pattern.
+    work_dir = None
+    try:
+        work_dir = find_existing_work_dir(run_id, repo_path=None)
+    except ValueError as exc:
+        print(f"Campaign location resolution failed: {exc}", file=sys.stderr)
+        sys.exit(1)
+    if work_dir is None and not os.environ.get(ENV_VAR):
+        # No env var set: fall through to legacy CWD-walk for
+        # backward-compat with `nous status <run_id>` invoked from
+        # inside the target repo.
+        root = _find_repo_root()
+        candidate = root / ".nous" / run_id
+        if candidate.is_dir():
+            work_dir = candidate
+    if work_dir is None:
+        print(
+            f"Work directory not found for run_id={run_id!r}. "
+            f"Either run from inside the target repo, pass an explicit "
+            f"work_dir path, or set {ENV_VAR}.",
+            file=sys.stderr,
+        )
         sys.exit(1)
     return work_dir
 
@@ -88,20 +149,44 @@ def _cmd_run(args):
     run_id = args.run_id or campaign.get("run_id") or (campaign_path.parent.name + "-run")
     repo_path = campaign["target_system"].get("repo_path")
 
-    if repo_path:
-        state_path = Path(repo_path) / ".nous" / run_id / "state.json"
-        if state_path.exists():
-            state = json.loads(state_path.read_text())
-            # #236: read via helper so legacy ``phase`` keys still resolve.
-            from orchestrator.engine import read_phase_field
-            phase = read_phase_field(state)
-            if phase != "INIT":
-                print(
-                    f"Run '{run_id}' already in progress (phase={phase}). "
-                    f"Use 'nous resume' to continue.",
-                    file=sys.stderr,
-                )
-                sys.exit(1)
+    # #239: in-progress detection must check BOTH the legacy and
+    # env-var locations — otherwise toggling NOUS_CAMPAIGN_PARENT
+    # between runs would silently allow a parallel run that corrupts
+    # shared worktrees. find_existing_work_dir consults all candidates
+    # plus state.json's recorded work_dir.
+    import os as _os
+
+    from orchestrator.work_dir_resolver import (
+        ENV_VAR,
+        find_existing_work_dir,
+    )
+    try:
+        existing = find_existing_work_dir(run_id, repo_path)
+    except (ValueError, FileNotFoundError) as exc:
+        print(f"Campaign location resolution failed: {exc}", file=sys.stderr)
+        sys.exit(1)
+    if existing is not None:
+        state = json.loads((existing / "state.json").read_text())
+        # #236: read via helper so legacy ``phase`` keys still resolve.
+        from orchestrator.engine import read_phase_field
+        phase = read_phase_field(state)
+        if phase != "INIT":
+            print(
+                f"Run '{run_id}' already in progress at {existing} "
+                f"(phase={phase}). Use 'nous resume' to continue.",
+                file=sys.stderr,
+            )
+            sys.exit(1)
+        # Migration hint: if env var is set but the existing campaign
+        # lives at the legacy location, point the user at `mv`.
+        if _os.environ.get(ENV_VAR) and ".nous" in existing.parts:
+            print(
+                f"Note: campaign found at legacy location {existing}. "
+                f"To migrate to {ENV_VAR}: "
+                f"`mv {existing} ${ENV_VAR}/{run_id}` and re-run. "
+                f"Continuing at the legacy location for now.",
+                file=sys.stderr,
+            )
 
     work_dir = setup_work_dir(run_id, repo_path=repo_path)
 

orchestrator/create_campaign.py

@@ -119,11 +119,27 @@
     - "TODO: replace with a real knob name"
     - "TODO: replace with a real knob name"
 
-  # Path to the target system's git repo. When set, the orchestrator
-  # creates the campaign directory at <repo_path>/.nous/<run_id>/ and
-  # uses worktree isolation per arm (#133). Set to null only if you
-  # plan to override on the CLI; running `nous run` from a different
-  # CWD will silently land artifacts in the wrong place (#184).
+  # Path to the target system's git repo. Used for two distinct things
+  # (#239 keeps them cleanly separated):
+  #
+  #   1. Code worktrees per arm (#133) live at
+  #      <repo_path>/.nous-experiments/<run_id>/<arm>/. Always —
+  #      they ARE code FOR the target repo.
+  #
+  #   2. Campaign artifacts (state, ledger, principles, findings, JSON
+  #      results) live at $NOUS_CAMPAIGN_PARENT/<run_id>/ if you've
+  #      set that env var (recommended — see below); otherwise at the
+  #      legacy <repo_path>/.nous/<run_id>/, which pollutes the
+  #      target's git status (#239).
+  #
+  # Recommended setup: export NOUS_CAMPAIGN_PARENT=~/Documents/Projects/nous-campaigns
+  # in your shell rc. Campaign artifacts then live outside the target,
+  # cleanly separated from regular development. The target's git status
+  # stays clean; `git stash -u` won't capture campaign output.
+  #
+  # Set repo_path to null only if you plan to override on the CLI;
+  # running `nous run` from a different CWD will silently land artifacts
+  # in the wrong place (#184).
   repo_path: {repo_path}
 
 prompts:

orchestrator/iteration.py

@@ -21,6 +21,7 @@
 import argparse
 import json
 import logging
+import os
 import re
 import shutil
 import sys
@@ -726,31 +727,85 @@ def _merge_principles(work_dir: Path, iter_dir: Path) -> None:
 def setup_work_dir(run_id: str, repo_path: str | None = None) -> Path:
     """Create and initialize a working directory from templates.
 
-    If repo_path is provided, the campaign directory is created inside
-    the target repo at .nous/<run_id>/. Otherwise falls back to creating
-    <run_id>/ in the current directory.
+    See ``orchestrator/work_dir_resolver.py`` for the canonical
+    RESOLUTION RULES — this function delegates path resolution there.
+    Briefly: ``$NOUS_CAMPAIGN_PARENT/<run_id>/`` if env var set, else
+    legacy ``<repo_path>/.nous/<run_id>/``, else ``<run_id>/`` relative
+    to CWD (#239).
+
+    The resolved absolute work_dir AND repo_path are recorded in
+    state.json — this is the per-campaign source of truth for location.
+    Used by collision detection (refuses to clobber a same-named
+    campaign that targets a different repo, #239 D1) and by future
+    resume/discovery tooling.
+
+    Worktrees are NOT affected: they continue to live at
+    ``<repo_path>/.nous-experiments/<run_id>/<arm>/`` because they are
+    code FOR the target repo and must share its git history. See
+    ``orchestrator/worktree.py``.
 
     Also writes a per-campaign ``.claude/settings.json`` permission policy
     (issue #135) so dispatchers can pass ``--settings <path>`` instead of
     ``--dangerously-skip-permissions``.
+
+    Raises:
+        ValueError: ``NOUS_CAMPAIGN_PARENT`` set to empty/whitespace, OR
+            an existing state.json at the resolved path records a
+            different ``repo_path`` (run_id collision under env var).
+        FileNotFoundError: ``repo_path`` provided but doesn't exist.
+        OSError: filesystem error creating the work_dir (wrapped with
+            env-var context if ``NOUS_CAMPAIGN_PARENT`` is set).
     """
     from orchestrator.settings_template import (
         render_campaign_settings,
         settings_path_for,
         write_campaign_settings,
     )
+    from orchestrator.work_dir_resolver import ENV_VAR, resolve_work_dir
+
+    work_dir = resolve_work_dir(run_id, repo_path)
+
+    # #239 D1: collision detection. If state.json already exists with a
+    # different recorded repo_path, the user has accidentally collided
+    # two campaigns under the same run_id. Refuse loudly rather than
+    # silently corrupt the existing campaign.
+    existing_state = work_dir / "state.json"
+    if existing_state.exists():
+        try:
+            prior = json.loads(existing_state.read_text())
+        except (json.JSONDecodeError, OSError):
+            prior = {}
+        prior_repo = prior.get("repo_path")
+        new_repo = str(Path(repo_path).resolve()) if repo_path else None
+        if prior_repo and new_repo and prior_repo != new_repo:
+            raise ValueError(
+                f"run_id collision at {work_dir}: existing state.json "
+                f"records repo_path={prior_repo!r} but this run targets "
+                f"repo_path={new_repo!r}. Run_ids must be globally unique "
+                f"under {ENV_VAR}; rename one campaign's run_id."
+            )
+
+    try:
+        work_dir.mkdir(parents=True, exist_ok=True)
+    except OSError as exc:
+        env_hint = ""
+        if os.environ.get(ENV_VAR):
+            env_hint = f" (resolved via {ENV_VAR}={os.environ[ENV_VAR]!r})"
+        raise OSError(
+            f"could not create campaign work_dir at {work_dir}{env_hint}: {exc}"
+        ) from exc
 
-    if repo_path:
-        work_dir = Path(repo_path) / ".nous" / run_id
-    else:
-        work_dir = Path(run_id)
-    work_dir.mkdir(parents=True, exist_ok=True)
     for t in ["state.json", "ledger.json", "principles.json"]:
         dest = work_dir / t
         if not dest.exists():
             shutil.copy(TEMPLATES_DIR / t, dest)
     state = json.loads((work_dir / "state.json").read_text())
     state["run_id"] = run_id
+    # #239: record resolved paths as per-campaign source of truth.
+    # work_dir survives env var changes; repo_path enables collision
+    # detection and future cross-machine discovery.
+    state["work_dir"] = str(work_dir.resolve())
+    state["repo_path"] = str(Path(repo_path).resolve()) if repo_path else None
     atomic_write(work_dir / "state.json", json.dumps(state, indent=2) + "\n")
 
     # Per-campaign permission policy. Idempotent: don't overwrite a settings

orchestrator/schemas/state.schema.json

@@ -37,6 +37,14 @@
     "config_ref": {
       "type": ["string", "null"],
       "description": "Path to the campaign configuration file (campaign.yaml). Null before setup."
+    },
+    "work_dir": {
+      "type": ["string", "null"],
+      "description": "Absolute filesystem path to the campaign's work_dir, recorded at setup_work_dir (#239). Per-campaign source of truth for location, robust to NOUS_CAMPAIGN_PARENT changes between runs. **Machine-local**: tools that travel state.json across machines must validate Path(work_dir).exists() before trusting it (or rederive from the local environment). Null before setup."
+    },
+    "repo_path": {
+      "type": ["string", "null"],
+      "description": "Absolute path to the target system's repo, recorded at setup_work_dir (#239). Used for collision detection (refusing to clobber a same-named campaign that targets a different repo when NOUS_CAMPAIGN_PARENT is set) and to identify which target a campaign belongs to. **Machine-local**, same caveats as work_dir. Null before setup or when the campaign was authored without a target_system.repo_path."
     }
   }
 }

orchestrator/templates/state.json

@@ -4,5 +4,7 @@
   "run_id": "TODO-SET-RUN-ID",
   "family": null,
   "timestamp": "1970-01-01T00:00:00Z",
-  "config_ref": null
+  "config_ref": null,
+  "work_dir": null,
+  "repo_path": null
 }