From 26cb7dc843bb1d5ca52a727e15bcceae7b192d7e Mon Sep 17 00:00:00 2001 From: Alex Ultra Date: Tue, 16 Jun 2026 09:34:29 +0200 Subject: [PATCH 1/2] feat(rig): provision a rig-managed .gitignore block at init/apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Machine-artifact ignores must be provisioned by rig, not hand-edited into ~/.gitignore. rig now maintains a marker-delimited block in the repo's .gitignore so harness artifacts are ignored declaratively, reconciled like every other category. - Default entry: .claude/worktrees/ (Claude Code's throwaway worktrees). .serena/ is intentionally NOT ignored — Serena state is committed shared project memory. - Marker-fenced block (BEGIN/END); rig edits only its own lines, every other line preserved byte-for-byte (CRLF, no-final-newline, trailing blanks via offset splicing, not splitlines/rejoin). - Idempotent: a correct block is a no-op; a missing block is appended (creating .gitignore if absent); a drifted block is replaced in place. - States shared by apply + drift via resolve_gitignore: ok / create / update / conflict (unbalanced markers, left untouched + surfaced) / io_error (unreadable path → error, never a silent skip). - Config: gitignore.{enabled,entries}; default ON, opt out with enabled:false. Fail-closed validation rejects a non-mapping block, non-bool enabled, unknown keys, non-string entries, and an entry colliding with a managed marker. - rig status reports drift on the block, including a leftover block after the category is disabled (check_disabled_gitignore, mirrors the dispatcher scan). - Tests cover every state, idempotency, verbatim preservation, drift parity, and config validation. Docs (config-schema.md) and the dogfooded rig.yaml updated. Co-Authored-By: Claude Opus 4.8 --- .gitignore | 4 + docs/config-schema.md | 70 ++++++- rig.yaml | 10 + riglib/actions/runner.py | 167 +++++++++++++++ riglib/cli.py | 8 + riglib/config.py | 51 +++++ riglib/drift.py | 64 ++++++ riglib/plan.py | 46 ++++- riglib/state.py | 6 + tests/test_catalog_plan.py | 5 +- tests/test_gitignore.py | 403 +++++++++++++++++++++++++++++++++++++ 11 files changed, 830 insertions(+), 4 deletions(-) create mode 100644 tests/test_gitignore.py diff --git a/.gitignore b/.gitignore index 5af39c3..4c49d86 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,7 @@ dist/ # uv writes a lockfile when run via `uv run`; the dev/CI path is pip, so the lock is a # local artifact — never commit it. uv.lock + +# >>> rig-managed (do not edit) >>> +.claude/worktrees/ +# <<< rig-managed (do not edit) <<< diff --git a/docs/config-schema.md b/docs/config-schema.md index d1c695e..bf388d7 100644 --- a/docs/config-schema.md +++ b/docs/config-schema.md @@ -38,6 +38,7 @@ harness: { ... } # agent harness auto/permission provisioning (auto models: { ... } # daily model-freshness checker schedule (launchd/crontab cron) agents_md: { ... } # AGENTS.md (canonical) + CLAUDE.md (symlink), default ON github: { ... } # GitHub repo branch ruleset via gh api, default ON (no-op without a github remote) +gitignore: { ... } # rig-managed .gitignore block (ignores .claude/worktrees/), default ON ``` If `agent_tools_source` is omitted, rig resolves it from `$RIG_AGENT_TOOLS_SOURCE`, then @@ -451,6 +452,71 @@ Set `RIG_GH_DRY_RUN=1` to compute what *would* change (the create/update is repo --- +## `gitignore` + +Maintains a **rig-managed block** in the repo's `.gitignore` (at the repo root) so harness +artifacts are ignored **declaratively by the tool** — reconciled like every other category, not +by a hand-edited global `~/.gitignore` ("fix tooling, not manual"). The motivating case: Claude +Code creates throwaway worktrees under each repo's `.claude/worktrees/`; those must be gitignored, +and rig owns that ignore. Default **ON** — on `rig init` AND `rig apply` rig converges the block; +idempotent (a re-apply that finds it correct is a no-op). + +```yaml +gitignore: + enabled: true # provision the managed block (default ON; false opts out) + entries: # the ignored paths inside the managed block + - .claude/worktrees/ # default: Claude Code's throwaway worktrees +``` + +| Key | Type | Default | Meaning | +| --- | --- | --- | --- | +| `enabled` | bool | `true` | provision the managed block (set `false` to leave `.gitignore` untouched) | +| `entries` | list[str] | `[.claude/worktrees/]` | the paths ignored inside the managed block; an empty/absent list uses the default | + +**`.serena/` is intentionally NOT ignored.** Serena state is **committed** shared project memory +(project memories travel with the repo), so it is never in the default entries — only throwaway +harness artifacts are. + +**The managed block.** rig fences its lines with explicit markers and touches **only** what is +between them: + +``` +# >>> rig-managed (do not edit) >>> +.claude/worktrees/ +# <<< rig-managed (do not edit) <<< +``` + +**On-disk cases (idempotent, surgical, never destructive):** + +- **no `.gitignore`** → create it with the managed block. +- **`.gitignore` without the block** → append the block after the existing content (one blank-line + separator); every existing line is preserved verbatim. +- **block present and correct** → no-op (in sync). +- **block present but its entries differ** → replace **just the block** in place; every line + **outside** the markers (above and below) is preserved verbatim, and the block stays where it sits. +- **unbalanced / duplicated markers** (a begin with no end, an end before a begin, two blocks) → + **conflict**: rig won't guess the block's extent, so it leaves the file completely untouched and + surfaces it via `rig status`. Reconcile by hand, then re-apply. +- **unreadable `.gitignore`** (no read permission, or a directory at the path) → **io_error**: + `rig apply` returns an `error` (it could not even inspect the file — never a silent exit 0), and + `rig status` surfaces a could-not-verify drift item rather than reporting in sync. + +Lines OUTSIDE the markers are preserved **byte-for-byte**: the block is located and spliced by raw +offset (the file is read with newline translation off), so a CRLF file, a file with no trailing +newline, and trailing blank lines all survive untouched. `entries` may not contain a marker line +(validation rejects it — it would make the file a permanent conflict). + +`resolve_gitignore` is the single classification `rig apply` and `rig status` share (so they can +never disagree): `create` (missing block → append/create), `update` (block differs → replace just +the block), `ok` (matches → no-op), `conflict` (unbalanced markers → untouched, surfaced), +`io_error` (unreadable → apply errors, status surfaces). There is no backup file — rig only ever +edits its own fenced lines, so there is no user data to preserve (`on_conflict` does not apply +here). Opting the category out (`enabled: false`) after a block was installed does NOT delete it +(apply never deletes); `rig status` then reports the leftover block as a disk→config extra so you +can remove it deliberately. + +--- + ## Validation `apply`/`status`/`init` validate before touching disk and **fail closed** on: @@ -461,5 +527,7 @@ a malformed/out-of-range `models.schedule.time` or unknown `models` key, a non-b `agents_md.enabled`/`agents_md.symlink` or unknown `agents_md` key, an unknown `github`/`github.ruleset` key, a non-bool `github.ruleset` boolean knob, a `github.ruleset.required_reviews` that is not an int ≥ 0, a `github.ruleset.required_status_checks` -that is not a list of strings, and an `agent_tools_source` that is not an agent-tools checkout. +that is not a list of strings, a non-mapping `gitignore` block / non-bool `gitignore.enabled` / +unknown `gitignore` key / a `gitignore.entries` that is not a list of strings or that contains a +rig-managed marker line, and an `agent_tools_source` that is not an agent-tools checkout. `--dry-run` prints the resolved plan and exits 0 without writing. diff --git a/rig.yaml b/rig.yaml index 6d4fcd0..474fb3c 100644 --- a/rig.yaml +++ b/rig.yaml @@ -81,3 +81,13 @@ harness: enabled: true kind: claude-code auto_mode: true + +# ── Gitignore — rig owns a marker-delimited block in this repo's .gitignore ──────── +# Claude Code creates throwaway worktrees under .claude/worktrees/; rig ignores them +# DECLARATIVELY (reconciled like every other category) rather than via a hand-edited global +# ignore. NB: .serena/ is intentionally NOT ignored — Serena state is committed shared memory. +# Default ON; this block makes the dogfooded config explicit. Opt out with enabled: false. +gitignore: + enabled: true + entries: + - .claude/worktrees/ diff --git a/riglib/actions/runner.py b/riglib/actions/runner.py index d87d72e..af2cf89 100644 --- a/riglib/actions/runner.py +++ b/riglib/actions/runner.py @@ -17,6 +17,7 @@ from pathlib import Path from typing import Callable +from ..config import GITIGNORE_BEGIN_MARKER, GITIGNORE_END_MARKER from ..github_ruleset import ( build_ruleset_body, find_managed_ruleset, @@ -1282,6 +1283,171 @@ def _do_provision_agents_symlink(action: Action, on_conflict: str) -> ActionResu return ActionResult(action, "error", f"agents-md: unhandled state {r.state!r}") +# ── rig-managed .gitignore block ─────────────────────────────────────────────────── +# rig maintains a marker-delimited block in the repo's `.gitignore` so harness artifacts +# (chiefly Claude Code's throwaway `.claude/worktrees/`) are ignored DECLARATIVELY by the tool, +# reconciled like every other category — not by a hand-edited global ignore ("fix tooling, not +# manual"). The markers fence ONLY rig's lines; every other line the user has in `.gitignore` +# is preserved verbatim. apply and drift BOTH go through `resolve_gitignore` so they can never +# disagree on what the desired block is or whether the file is in sync. The marker constants live +# in `config.py` (the schema layer) so validation can reject a marker-colliding entry without an +# import cycle; they are imported at module top and re-used here (drift/tests reach for them via +# `runner.GITIGNORE_*`, which still resolves through this module's namespace). + + +def gitignore_block_text(entries: list[str]) -> str: + """The exact marker-delimited block rig owns for ``entries`` (no trailing newline). + + Single source of truth shared by the install handler and drift, so both agree byte-for-byte + on what the managed block SHOULD contain. The block is the begin marker, one line per entry + (in the order given), then the end marker. + """ + return "\n".join([GITIGNORE_BEGIN_MARKER, *entries, GITIGNORE_END_MARKER]) + + +@dataclass(frozen=True) +class GitignoreResolution: + """The desired ``.gitignore`` outcome for a repo — the one source apply + drift share. + + ``state`` is the single discriminator both consumers switch on: + - ``ok`` — the managed block is already present and exactly correct: no-op. + - ``create`` — no ``.gitignore`` or no managed block: append the block (creating the file + if absent), preserving any existing content. + - ``update`` — a managed block exists but its body differs: replace JUST the block, + preserving every line outside the markers (verbatim — CRLF and trailing + blanks included). + - ``conflict`` — the file has unbalanced/duplicated managed markers (a begin with no end, an + end before a begin, two blocks): rig won't guess the block's extent, so it + leaves the file untouched and surfaces it. ``detail`` says why. + - ``io_error`` — the path could not be read (unreadable, or a directory sits there). Unlike a + marker ``conflict`` (the file is fine, the operator must reconcile) this is a + failure to even inspect the file: apply reports it as an ERROR, never a silent + skip. ``detail`` carries the OS error. + + ``desired_block`` is the canonical block text; ``new_content`` is the full desired file + content for ``create``/``update`` (``None`` for ``ok``/``conflict``/``io_error``). + """ + + path: Path + state: str + desired_block: str + new_content: str | None = None + detail: str = "" + + +def resolve_gitignore(path: Path, entries: list[str]) -> GitignoreResolution: + """Classify the on-disk ``.gitignore`` vs the desired managed block (pure, no writes). + + Idempotent + non-destructive: rig only ever (a) appends the block to a file that lacks it + (creating the file if absent), (b) replaces JUST the existing block in place, or (c) no-ops a + correct block. An unbalanced marker pair is a ``conflict`` rig never rewrites. Every line + OUTSIDE the markers is preserved byte-for-byte: the block is located and spliced by raw + character offset (not splitlines/rejoin), so a CRLF file, a file with no trailing newline, and + trailing blank lines all survive untouched. + """ + desired = gitignore_block_text(entries) + if not path.exists(): + return GitignoreResolution(path, "create", desired, new_content=desired + "\n") + try: + # newline="" disables universal-newline translation so a CRLF file is read (and later + # re-written) byte-for-byte outside the managed block — the documented verbatim guarantee. + with path.open(encoding="utf-8", newline="") as fh: + content = fh.read() + except OSError as exc: + # unreadable, or a directory at the path — a failure to inspect, not a marker conflict. + return GitignoreResolution(path, "io_error", desired, detail=f"cannot read {path}: {exc}") + + # Find each marker line by its raw [start, end_of_line] offsets so the splice preserves every + # other byte verbatim (line ending included). A marker is a line whose stripped text equals the + # marker constant — tolerant of trailing whitespace on the marker line itself. + begins = _find_marker_lines(content, GITIGNORE_BEGIN_MARKER) + ends = _find_marker_lines(content, GITIGNORE_END_MARKER) + if len(begins) != len(ends) or len(begins) > 1: + return GitignoreResolution( + path, "conflict", desired, + detail=f"{path} has unbalanced/duplicated rig-managed markers — reconcile by hand, then re-run", + ) + if not begins: + # no managed block: append it, keeping a single blank-line separator from prior content. + body = content.rstrip("\n") + sep = "\n\n" if body else "" + new_content = f"{body}{sep}{desired}\n" + return GitignoreResolution(path, "create", desired, new_content=new_content) + + (b_start, _b_line_end), (e_start, e_line_end) = begins[0], ends[0] + if e_start < b_start: + return GitignoreResolution( + path, "conflict", desired, + detail=f"{path} has a rig-managed end marker before its begin marker — reconcile by hand, then re-run", + ) + # the block spans from the begin marker's start to the end marker's end-of-line. Compare the + # raw on-disk block to the desired text; if identical it's in sync, else splice in the new block + # and keep everything before b_start and after e_line_end byte-for-byte. + current_block = content[b_start : e_line_end] + if current_block == desired: + return GitignoreResolution(path, "ok", desired) + new_content = content[:b_start] + desired + content[e_line_end:] + return GitignoreResolution(path, "update", desired, new_content=new_content) + + +def _find_marker_lines(content: str, marker: str) -> list[tuple[int, int]]: + """Return ``(line_start_offset, line_end_offset)`` for each line equal to ``marker``. + + ``line_end_offset`` is the offset of the newline that terminates the line (or ``len(content)`` + for an un-terminated final line) — so a splice on ``[start, end)`` drops the marker line's text + but not its line ending, letting the caller re-emit a clean block. A line MATCHES when its text + with surrounding whitespace stripped equals ``marker`` (tolerant of a marker line that picked up + trailing spaces), so such a line is still recognized and normalized on the next apply. + """ + out: list[tuple[int, int]] = [] + pos = 0 + n = len(content) + while pos <= n: + nl = content.find("\n", pos) + line_end = n if nl == -1 else nl + if content[pos:line_end].strip() == marker: + out.append((pos, line_end)) + if nl == -1: + break + pos = nl + 1 + return out + + +def _do_provision_gitignore(action: Action, on_conflict: str) -> ActionResult: + """Provision/reconcile rig's managed block in the repo's ``.gitignore``. + + Switches on the shared :func:`resolve_gitignore` ``state`` — apply and drift read the same + classification, so ``status`` never misreports the on-disk state. Idempotent: a correct block + is a no-op; a missing block is appended (creating ``.gitignore`` if absent); a drifted block is + replaced IN PLACE, preserving every other line verbatim. An unbalanced marker pair is a + ``conflict`` rig leaves untouched (``skipped``), and an unreadable path is an ``error`` (never a + silent skip — apply must not exit 0 having failed to even inspect the file). There is no backup: + rig only ever edits its OWN fenced lines, so there is no user data to preserve — ``on_conflict`` + is irrelevant here (consistent with the surgical hook-bridge upsert, which likewise rewrites + only its own marked entries). + """ + entries = [str(e) for e in action.options.get("entries", [])] + r = resolve_gitignore(action.target, entries) + if r.state == "ok": + return ActionResult(action, "skipped", f"gitignore: managed block already correct in {action.target.name}") + if r.state == "conflict": + return ActionResult(action, "skipped", f"gitignore: {r.detail}") + if r.state == "io_error": + return ActionResult(action, "error", f"gitignore: {r.detail}") + if r.new_content is None: # defensive — create/update always carry content + return ActionResult(action, "error", f"gitignore: unhandled state {r.state!r}") + existed = action.target.exists() + action.target.parent.mkdir(parents=True, exist_ok=True) + # newline="" so the bytes we computed (which may carry the user's CRLF outside the block) are + # written verbatim, with no platform newline translation. + with action.target.open("w", encoding="utf-8", newline="") as fh: + fh.write(r.new_content) + if r.state == "create": + verb = "added block to" if existed else "created" + return ActionResult(action, "created", f"gitignore: {verb} {action.target.name} ({len(entries)} entr{'y' if len(entries) == 1 else 'ies'})") + return ActionResult(action, "updated", f"gitignore: updated managed block in {action.target.name}") + + # ── GitHub repository ruleset (gh api) ───────────────────────────────────────────── # rig reconciles a branch ruleset on the repo's DEFAULT branch — the modern replacement for # branch protection — declaratively, the same way every other category is reconciled. The @@ -1471,4 +1637,5 @@ def _do_provision_github_ruleset(action: Action, on_conflict: str) -> ActionResu "provision_schedule": _do_provision_schedule, "provision_agents_symlink": _do_provision_agents_symlink, "provision_github_ruleset": _do_provision_github_ruleset, + "provision_gitignore": _do_provision_gitignore, } diff --git a/riglib/cli.py b/riglib/cli.py index 0d07961..dc2e56e 100644 --- a/riglib/cli.py +++ b/riglib/cli.py @@ -410,6 +410,14 @@ def cmd_status(args: argparse.Namespace) -> int: from .drift import check_disabled_dispatcher check_disabled_dispatcher(loaded.repo_root, report) + # disabled-but-installed gitignore block: config opted the category out, but a prior apply may + # have left the rig-managed block in .gitignore. apply won't remove it, so surface it as + # disk→config drift (mirrors the disabled-dispatcher scan above; the block is repo-local). + gi_cfg = loaded.data.get("gitignore") + if isinstance(gi_cfg, dict) and gi_cfg.get("enabled") is False: + from .drift import check_disabled_gitignore + + check_disabled_gitignore(loaded.repo_root, report) # surface the model-freshness schedule explicitly (installed / drifted / not configured), # so `rig status` answers "is the daily checker cron there?" at a glance. _print_schedule_status(plan, report) diff --git a/riglib/config.py b/riglib/config.py index 6f12102..c2cb13b 100644 --- a/riglib/config.py +++ b/riglib/config.py @@ -38,6 +38,7 @@ "models", "agents_md", "github", + "gitignore", } _VALID_CATEGORIES = {"skills", "agent_hooks", "git_hooks", "ci", "mcp"} _VALID_ON_CONFLICT = {"skip", "overwrite", "backup"} @@ -206,6 +207,7 @@ def validate(data: dict[str, Any]) -> None: _validate_models(data.get("models", {})) _validate_agents_md(data.get("agents_md", {})) _validate_github(data.get("github", {})) + _validate_gitignore(data.get("gitignore", {})) def _validate_ci(ci: dict[str, Any]) -> None: @@ -381,6 +383,55 @@ def _validate_agents_md(am: dict[str, Any]) -> None: raise ConfigError(f"agents_md.{knob} must be a bool, got {value!r}") +# The default entries rig's managed .gitignore block ignores. The harness (Claude Code) +# creates throwaway worktrees under each repo's ``.claude/worktrees/``; those must be +# gitignored declaratively by rig, not by a hand-edited global ignore. Listed once so the +# validator (default-fill) and the plan builder reference the SAME default — NB: ``.serena/`` +# is deliberately NOT here (Serena state is COMMITTED shared project memory). +GITIGNORE_DEFAULT_ENTRIES = (".claude/worktrees/",) + +# The markers that fence rig's managed block in a repo's .gitignore. Defined here (the schema +# layer, stdlib-only) so config validation can reject an entry that collides with a marker +# WITHOUT importing the actions runner (which would form a config→plan→config import cycle); the +# runner imports these from config so the two never drift. +GITIGNORE_BEGIN_MARKER = "# >>> rig-managed (do not edit) >>>" +GITIGNORE_END_MARKER = "# <<< rig-managed (do not edit) <<<" + + +def _validate_gitignore(gi: dict[str, Any]) -> None: + """Validate the ``gitignore`` block — rig's managed ``.gitignore`` block in the repo. + + rig maintains a marker-delimited block in the repo's ``.gitignore`` so harness artifacts + (chiefly ``.claude/worktrees/``) are ignored declaratively by the tool, reconciled like + every other category — not by a hand-edited global ignore. Default **ON**: an absent/empty + block means "provision the default entries". Fail-closed, consistent with every other + block, on: a non-mapping block, a non-bool ``enabled``, an unknown key (typo guard), and a + non-string-list ``entries``. + """ + if not isinstance(gi, dict): + raise ConfigError("gitignore must be a mapping") + if not gi: + return + unknown = set(gi) - {"enabled", "entries"} + if unknown: + raise ConfigError(f"unknown gitignore key(s): {', '.join(sorted(unknown))}") + enabled = gi.get("enabled") + if enabled is not None and not isinstance(enabled, bool): + raise ConfigError(f"gitignore.enabled must be a bool, got {enabled!r}") + entries = gi.get("entries") + if entries is not None: + if not isinstance(entries, list) or not all(isinstance(e, str) for e in entries): + raise ConfigError(f"gitignore.entries must be a list of strings, got {entries!r}") + # Reject an entry that carries one of rig's block markers: writing it inside the managed + # block would make every later resolve see a duplicated marker and classify the file as a + # permanent conflict (apply could never re-converge). Fail closed on the footgun. + for e in entries: + if GITIGNORE_BEGIN_MARKER in e or GITIGNORE_END_MARKER in e: + raise ConfigError( + f"gitignore.entries may not contain a rig-managed marker line, got {e!r}" + ) + + # The ruleset knobs that are plain booleans (typo + type guard). Listed once so the # validator and the action builder reference the SAME knob set. _GITHUB_RULESET_BOOL_KNOBS = ( diff --git a/riglib/drift.py b/riglib/drift.py index a8bce17..695e2c4 100644 --- a/riglib/drift.py +++ b/riglib/drift.py @@ -32,9 +32,11 @@ github_ruleset_state, harness_settings_file, hook_bridge_entries, + GITIGNORE_BEGIN_MARKER, parse_mcp_command, resolve_agents_md, resolve_ci_workflow, + resolve_gitignore, schedule_plan_from_action, skill_harness_link_target, ) @@ -117,6 +119,8 @@ def detect( _check_agents_symlink(action, report) elif action.kind == "provision_github_ruleset": _check_github_ruleset(action, report) + elif action.kind == "provision_gitignore": + _check_gitignore(action, report) _extras_skills(declared_skill_dirs, report) _extras_ci(declared_ci_dirs, report) @@ -269,6 +273,66 @@ def _check_agents_symlink(action: Action, report: DriftReport) -> None: ) +def _check_gitignore(action: Action, report: DriftReport) -> None: + """Flag drift between the configured rig-managed ``.gitignore`` block and the file on disk. + + Switches on the SAME :func:`resolve_gitignore` ``state`` apply uses (one classification, + shared via the runner), so status and apply can never disagree on "in sync": + + - ``create`` → ``missing``: no ``.gitignore`` or no managed block (apply appends it). + - ``update`` → ``modified``: a managed block exists but its entries differ (apply replaces + just the block). + - ``ok`` → no drift item (in sync). + - ``conflict`` → ``modified``: unbalanced markers rig won't rewrite — surfaced so the + operator reconciles by hand (apply leaves it untouched). + - ``io_error`` → ``modified``: the file couldn't be read (unreadable / a directory at the + path). NOT silently in-sync — rig couldn't even inspect it, so a green status + would mask a genuinely un-provisioned ignore (mirrors the github gh_error + could-not-verify item). + """ + entries = [str(e) for e in action.options.get("entries", [])] + r = resolve_gitignore(action.target, entries) + if r.state == "ok": + return + if r.state == "create": + report.items.append( + DriftItem("missing", "gitignore", "block", action.target, + "rig-managed .gitignore block not present (apply adds it)") + ) + elif r.state == "update": + report.items.append( + DriftItem("modified", "gitignore", "block", action.target, + "rig-managed .gitignore block differs from config (apply replaces just the block)") + ) + elif r.state in ("conflict", "io_error"): + report.items.append( + DriftItem("modified", "gitignore", "block", action.target, r.detail) + ) + + +def check_disabled_gitignore(repo_root: Path, report: DriftReport) -> None: + """Flag a still-installed rig-managed block when the config disables the ``gitignore`` category. + + apply never deletes; so a repo that previously had the managed block keeps it in + ``.gitignore`` even after the config turns the category off. With the action gone from the + plan, ``_check_gitignore`` never runs — so without this scan the leftover block would report as + "in sync". Detect a present begin marker in the repo's ``.gitignore`` and report it as + disk→config drift (mirrors :func:`check_disabled_dispatcher` for the global dispatcher). + """ + gi = repo_root / ".gitignore" + if not gi.is_file(): + return + try: + content = gi.read_text(encoding="utf-8") + except OSError: + return + if any(ln.strip() == GITIGNORE_BEGIN_MARKER for ln in content.splitlines()): + report.items.append( + DriftItem("extra", "gitignore", "block", gi, + "gitignore disabled in config but the rig-managed block is still in .gitignore") + ) + + def _check_github_ruleset(action: Action, report: DriftReport) -> None: """Flag drift between the configured GitHub branch ruleset and the live repo. diff --git a/riglib/plan.py b/riglib/plan.py index fbffc5e..c871a36 100644 --- a/riglib/plan.py +++ b/riglib/plan.py @@ -66,7 +66,7 @@ class PlanError(ValueError): class Action: """A single planned install step. ``kind`` selects the runner in ``actions/``.""" - kind: str # copy_skill | link_skill_harness | install_agent_hook | install_dispatcher | install_ci | register_mcp | apply_harness | register_hook_bridge | provision_schedule | provision_agents_symlink | provision_github_ruleset + kind: str # copy_skill | link_skill_harness | install_agent_hook | install_dispatcher | install_ci | register_mcp | apply_harness | register_hook_bridge | provision_schedule | provision_agents_symlink | provision_github_ruleset | provision_gitignore category: str item: str source: Path # carrier path in the agent-tools checkout @@ -500,6 +500,9 @@ def build(config: LoadedConfig, catalog: Catalog, *, project_type: str = "unknow # ── github (repository branch ruleset via gh api) ───────────────────────────── _build_github_ruleset(config, plan) + # ── gitignore (rig-managed block in the repo's .gitignore) ───────────────────── + _build_gitignore(config, plan) + return plan @@ -706,6 +709,47 @@ def _build_github_ruleset(config: LoadedConfig, plan: InstallPlan) -> None: ) +def _build_gitignore(config: LoadedConfig, plan: InstallPlan) -> None: + """Plan the rig-managed ``.gitignore`` block provisioning for the repo. + + Default **ON** (like ``agents_md``): rig maintains a marker-delimited block in the repo's + ``.gitignore`` (at ``config.repo_root``) so harness artifacts — chiefly Claude Code's + throwaway ``.claude/worktrees/`` — are ignored declaratively by the tool, reconciled like + every other category, not by a hand-edited global ignore. Opt out with + ``gitignore: { enabled: false }``. + + The ignored ``entries`` are configurable with a sensible default + (``GITIGNORE_DEFAULT_ENTRIES``); an empty/absent block uses that default. The + upsert-just-the-block logic lives in ``actions/`` (it depends on what is already on disk), + so the plan emits ONE idempotent action carrying the resolved entries, anchored at the + repo root; no carrier in agent-tools. + """ + from .config import GITIGNORE_DEFAULT_ENTRIES + + gi = config.data.get("gitignore") + if gi is None: + gi = {} + if not isinstance(gi, dict): + return # validate() already fail-closed on a non-mapping block + if gi.get("enabled") is False: + return + raw_entries = gi.get("entries") + if not isinstance(raw_entries, list) or not raw_entries: + entries = list(GITIGNORE_DEFAULT_ENTRIES) + else: + entries = [str(e) for e in raw_entries] + plan.actions.append( + Action( + kind="provision_gitignore", + category="gitignore", + item="block", + source=config.repo_root, + target=config.repo_root / ".gitignore", + options={"entries": entries}, + ) + ) + + def _build_models(config: LoadedConfig, catalog: Catalog, plan: InstallPlan) -> None: """Plan the daily model-freshness checker schedule, if a ``models`` block enables it. diff --git a/riglib/state.py b/riglib/state.py index eeb8334..6f42272 100644 --- a/riglib/state.py +++ b/riglib/state.py @@ -18,6 +18,7 @@ from pathlib import Path from typing import Any +from .config import GITIGNORE_DEFAULT_ENTRIES from .github_ruleset import GITHUB_RULESET_DEFAULTS @@ -127,6 +128,11 @@ def default_state( # with ruleset.enabled: false. Add status checks with required_status_checks: [names]. # The knobs come straight from the action's GITHUB_RULESET_DEFAULTS (one source). "github": {"ruleset": github_ruleset}, + # rig-managed block in the repo's .gitignore: ignores the harness's throwaway + # `.claude/worktrees/` declaratively (default ON), reconciled like every other category. + # `.serena/` is intentionally NOT ignored — Serena state is committed shared memory. + # Opt out with gitignore.enabled: false; the default entries come from one source. + "gitignore": {"enabled": True, "entries": list(GITIGNORE_DEFAULT_ENTRIES)}, } diff --git a/tests/test_catalog_plan.py b/tests/test_catalog_plan.py index cbb3f1b..0656017 100644 --- a/tests/test_catalog_plan.py +++ b/tests/test_catalog_plan.py @@ -195,8 +195,8 @@ def test_xdg_config_home_maps_dispatcher_dir(fake_agent_tools, tmp_path, monkeyp def test_plan_disabled_category(fake_agent_tools, tmp_path): cat = Catalog.scan(str(fake_agent_tools)) - # agents_md and the github ruleset are default-ON, so turn them off too to assert a truly - # empty plan. + # agents_md, the github ruleset, and the gitignore block are default-ON, so turn them off + # too to assert a truly empty plan. cfg = _cfg( { "skills": {"enabled": False}, @@ -205,6 +205,7 @@ def test_plan_disabled_category(fake_agent_tools, tmp_path): "mcp": {"enabled": False}, "agents_md": {"enabled": False}, "github": {"ruleset": {"enabled": False}}, + "gitignore": {"enabled": False}, }, tmp_path, ) diff --git a/tests/test_gitignore.py b/tests/test_gitignore.py new file mode 100644 index 0000000..f5c6430 --- /dev/null +++ b/tests/test_gitignore.py @@ -0,0 +1,403 @@ +"""rig-managed ``.gitignore`` block provisioning — the ``gitignore`` block. + +Covers every resolved ``state`` (create / update / ok / conflict / io_error), the default-ON + +opt-out plan gating, config validation, and drift parity. The guiding invariant: apply and drift +switch on the SAME ``resolve_gitignore`` state, so they can never disagree — and rig only ever +edits its OWN marker-fenced lines, preserving every other line in the file verbatim (CRLF and +trailing blanks included). + +The motivation (CTO ask): Claude Code creates throwaway worktrees under each repo's +``.claude/worktrees/`` — those must be gitignored DECLARATIVELY by rig (reconciled like every +other category), not by a hand-edited global ignore. ``.serena/`` is deliberately NOT ignored +(Serena state is committed shared project memory). +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from riglib.actions.runner import ( + GITIGNORE_BEGIN_MARKER, + GITIGNORE_END_MARKER, + _do_provision_gitignore, + gitignore_block_text, + resolve_gitignore, + run_plan, +) +from riglib.config import GITIGNORE_DEFAULT_ENTRIES, ConfigError, LoadedConfig, validate +from riglib.drift import DriftReport, check_disabled_gitignore, detect +from riglib.plan import Action, InstallPlan, build + +DEFAULT_ENTRIES = list(GITIGNORE_DEFAULT_ENTRIES) + + +def _action(repo: Path, entries: list[str] | None = None) -> Action: + return Action( + kind="provision_gitignore", + category="gitignore", + item="block", + source=repo, + target=repo / ".gitignore", + options={"entries": entries if entries is not None else DEFAULT_ENTRIES}, + ) + + +def _apply(repo: Path, entries: list[str] | None = None, on_conflict: str = "backup"): + return _do_provision_gitignore(_action(repo, entries), on_conflict) + + +def _plan_with_action(repo: Path, entries: list[str] | None = None) -> InstallPlan: + plan = InstallPlan() + plan.actions.append(_action(repo, entries)) + return plan + + +def _block(entries: list[str]) -> str: + return gitignore_block_text(entries) + + +# ── the default entry is the harness worktrees dir, never .serena/ ───────────────── +def test_default_entries_ignore_worktrees_not_serena(): + assert ".claude/worktrees/" in DEFAULT_ENTRIES + assert ".serena/" not in DEFAULT_ENTRIES # Serena state is committed, never ignored + block = gitignore_block_text(DEFAULT_ENTRIES) + assert block.startswith(GITIGNORE_BEGIN_MARKER) + assert block.endswith(GITIGNORE_END_MARKER) + assert ".claude/worktrees/" in block + + +# ── create: fresh file ───────────────────────────────────────────────────────────── +def test_fresh_create_when_no_gitignore(tmp_path): + gi = tmp_path / ".gitignore" + assert resolve_gitignore(gi, DEFAULT_ENTRIES).state == "create" + res = _apply(tmp_path) + assert res.status == "created" + assert gi.is_file() + text = gi.read_text() + assert _block(DEFAULT_ENTRIES) in text + assert text.endswith("\n") # trailing newline + + +def test_create_appends_to_existing_file_preserving_lines(tmp_path): + gi = tmp_path / ".gitignore" + gi.write_text("node_modules/\n*.log\n", encoding="utf-8") + assert resolve_gitignore(gi, DEFAULT_ENTRIES).state == "create" + res = _apply(tmp_path) + assert res.status == "created" + text = gi.read_text() + # the user's prior lines are preserved verbatim, the block is appended after a blank line + assert "node_modules/\n*.log\n" in text + assert _block(DEFAULT_ENTRIES) in text + assert text.index("node_modules/") < text.index(GITIGNORE_BEGIN_MARKER) + assert "*.log\n\n# >>> rig-managed" in text # single blank-line separator + + +# ── ok: idempotent re-apply ──────────────────────────────────────────────────────── +def test_idempotent_second_apply_skips(tmp_path): + assert _apply(tmp_path).status == "created" + before = (tmp_path / ".gitignore").read_text() + second = _apply(tmp_path) + assert resolve_gitignore(tmp_path / ".gitignore", DEFAULT_ENTRIES).state == "ok" + assert second.status == "skipped" and "already correct" in second.detail + assert (tmp_path / ".gitignore").read_text() == before # byte-identical, no churn + + +def test_idempotent_when_block_already_present_among_other_lines(tmp_path): + gi = tmp_path / ".gitignore" + gi.write_text(f"node_modules/\n\n{_block(DEFAULT_ENTRIES)}\n\n*.tmp\n", encoding="utf-8") + assert resolve_gitignore(gi, DEFAULT_ENTRIES).state == "ok" + res = _apply(tmp_path) + assert res.status == "skipped" + # user lines on BOTH sides of the block are preserved untouched + text = gi.read_text() + assert "node_modules/" in text and "*.tmp" in text + + +# ── update: block differs, just the block is replaced ────────────────────────────── +def test_update_replaces_just_the_block_preserving_other_lines(tmp_path): + gi = tmp_path / ".gitignore" + # a stale managed block (different entries) sandwiched between user lines. + stale = gitignore_block_text([".claude/old-cruft/"]) + gi.write_text(f"# top user line\nbuild/\n\n{stale}\n\n# bottom user line\ndist/\n", encoding="utf-8") + assert resolve_gitignore(gi, DEFAULT_ENTRIES).state == "update" + res = _apply(tmp_path) + assert res.status == "updated" + text = gi.read_text() + # the new block is present, the stale entry is gone + assert ".claude/worktrees/" in text + assert ".claude/old-cruft/" not in text + # every user line OUTSIDE the markers is preserved verbatim, on both sides + assert "# top user line" in text and "build/" in text + assert "# bottom user line" in text and "dist/" in text + # the block stayed in place (between the user lines), not moved to the end + assert text.index("build/") < text.index(GITIGNORE_BEGIN_MARKER) < text.index("# bottom user line") + + +def test_update_then_idempotent(tmp_path): + gi = tmp_path / ".gitignore" + gi.write_text(f"x/\n{gitignore_block_text(['.claude/old/'])}\ny/\n", encoding="utf-8") + assert _apply(tmp_path).status == "updated" + assert _apply(tmp_path).status == "skipped" # converged → idempotent + + +# ── custom entries (configurable) ────────────────────────────────────────────────── +def test_custom_entries_are_used(tmp_path): + entries = [".claude/worktrees/", ".cache/agent-tools/", "scratch/"] + res = _apply(tmp_path, entries=entries) + assert res.status == "created" + text = (tmp_path / ".gitignore").read_text() + for e in entries: + assert e in text + # entries appear in the given order, inside the markers + block = _block(entries) + assert block in text + + +# ── conflict: unbalanced markers left untouched ──────────────────────────────────── +def test_unbalanced_markers_is_conflict_left_untouched(tmp_path): + gi = tmp_path / ".gitignore" + # a begin marker with no matching end — rig won't guess where the block ends. + original = f"node_modules/\n{GITIGNORE_BEGIN_MARKER}\n.claude/worktrees/\n" + gi.write_text(original, encoding="utf-8") + assert resolve_gitignore(gi, DEFAULT_ENTRIES).state == "conflict" + res = _apply(tmp_path) + assert res.status == "skipped" and "unbalanced" in res.detail + assert gi.read_text() == original # untouched + + +def test_duplicate_begin_markers_is_conflict(tmp_path): + gi = tmp_path / ".gitignore" + block = _block(DEFAULT_ENTRIES) + gi.write_text(f"{block}\n{block}\n", encoding="utf-8") + assert resolve_gitignore(gi, DEFAULT_ENTRIES).state == "conflict" + assert _apply(tmp_path).status == "skipped" + + +def test_end_before_begin_is_conflict(tmp_path): + gi = tmp_path / ".gitignore" + original = f"{GITIGNORE_END_MARKER}\n.claude/worktrees/\n{GITIGNORE_BEGIN_MARKER}\n" + gi.write_text(original, encoding="utf-8") + assert resolve_gitignore(gi, DEFAULT_ENTRIES).state == "conflict" + res = _apply(tmp_path) + assert res.status == "skipped" + assert gi.read_text() == original # untouched + + +# ── plan gating: default ON, explicit opt-out ───────────────────────────────────── +def _cfg(data: dict, repo: Path) -> LoadedConfig: + return LoadedConfig(data={"version": 1, **data}, repo_root=repo) + + +def test_plan_includes_gitignore_by_default(fake_agent_tools, tmp_path): + from riglib.catalog import Catalog + + cat = Catalog.scan(str(fake_agent_tools)) + plan = build(_cfg({"skills": {"enabled": False}}, tmp_path), cat, project_type="cli") + actions = [a for a in plan.actions if a.kind == "provision_gitignore"] + assert len(actions) == 1 + # default entries land in the action options, anchored at the repo's .gitignore + assert actions[0].options["entries"] == DEFAULT_ENTRIES + assert actions[0].target == tmp_path / ".gitignore" + + +def test_plan_opt_out(fake_agent_tools, tmp_path): + from riglib.catalog import Catalog + + cat = Catalog.scan(str(fake_agent_tools)) + plan = build(_cfg({"gitignore": {"enabled": False}}, tmp_path), cat, project_type="cli") + assert not any(a.kind == "provision_gitignore" for a in plan.actions) + + +def test_plan_honors_custom_entries(fake_agent_tools, tmp_path): + from riglib.catalog import Catalog + + cat = Catalog.scan(str(fake_agent_tools)) + entries = [".claude/worktrees/", "tmp/"] + plan = build(_cfg({"gitignore": {"entries": entries}}, tmp_path), cat, project_type="cli") + action = next(a for a in plan.actions if a.kind == "provision_gitignore") + assert action.options["entries"] == entries + + +def test_plan_empty_entries_falls_back_to_default(fake_agent_tools, tmp_path): + from riglib.catalog import Catalog + + cat = Catalog.scan(str(fake_agent_tools)) + plan = build(_cfg({"gitignore": {"entries": []}}, tmp_path), cat, project_type="cli") + action = next(a for a in plan.actions if a.kind == "provision_gitignore") + assert action.options["entries"] == DEFAULT_ENTRIES + + +# ── config validation (fail-closed) ─────────────────────────────────────────────── +def test_validate_rejects_non_bool_enabled(): + with pytest.raises(ConfigError, match="gitignore.enabled must be a bool"): + validate({"version": 1, "gitignore": {"enabled": "yes"}}) + + +def test_validate_rejects_unknown_key(): + with pytest.raises(ConfigError, match="unknown gitignore key"): + validate({"version": 1, "gitignore": {"markers": "..."}}) + + +def test_validate_rejects_non_string_list_entries(): + with pytest.raises(ConfigError, match="gitignore.entries must be a list of strings"): + validate({"version": 1, "gitignore": {"entries": [".claude/worktrees/", 5]}}) + with pytest.raises(ConfigError, match="gitignore.entries must be a list of strings"): + validate({"version": 1, "gitignore": {"entries": ".claude/worktrees/"}}) + + +def test_validate_rejects_non_mapping_block(): + with pytest.raises(ConfigError, match="gitignore must be a mapping"): + validate({"version": 1, "gitignore": ["x"]}) + + +def test_validate_accepts_empty_and_valid_block(): + validate({"version": 1, "gitignore": {}}) + validate({"version": 1, "gitignore": {"enabled": True, "entries": [".claude/worktrees/"]}}) + + +# ── drift parity (apply and drift switch on the same state) ─────────────────────── +def test_drift_missing_then_in_sync(tmp_path): + plan = _plan_with_action(tmp_path) + before = detect(plan) + assert any(i.category == "gitignore" and i.direction == "missing" for i in before.items) + run_plan(plan) + assert not any(i.category == "gitignore" for i in detect(plan).items) + + +def test_drift_flags_modified_block(tmp_path): + gi = tmp_path / ".gitignore" + gi.write_text(f"{gitignore_block_text(['.claude/old/'])}\n", encoding="utf-8") + report = detect(_plan_with_action(tmp_path)) + assert any(i.category == "gitignore" and i.direction == "modified" for i in report.items) + + +def test_drift_flags_conflict(tmp_path): + gi = tmp_path / ".gitignore" + gi.write_text(f"{GITIGNORE_BEGIN_MARKER}\n.claude/worktrees/\n", encoding="utf-8") + report = detect(_plan_with_action(tmp_path)) + # assert the user-facing meaning (a surfaced unbalanced-marker conflict), not the incidental + # direction it maps to — so the test survives a future direction rename. + gi_items = [i for i in report.items if i.category == "gitignore"] + assert gi_items and "unbalanced" in gi_items[0].detail + + +def test_drift_clean_when_block_correct_among_other_lines(tmp_path): + gi = tmp_path / ".gitignore" + gi.write_text(f"node_modules/\n{_block(DEFAULT_ENTRIES)}\n*.log\n", encoding="utf-8") + report = detect(_plan_with_action(tmp_path)) + assert not any(i.category == "gitignore" for i in report.items) + + +# ── verbatim preservation: CRLF, trailing blanks, no-final-newline (review P3) ───── +def test_update_preserves_crlf_and_trailing_blanks_outside_block(tmp_path): + gi = tmp_path / ".gitignore" + # CRLF line endings + a trailing blank line outside the (stale) managed block. + stale = gitignore_block_text([".claude/old/"]).replace("\n", "\r\n") + raw = f"node_modules/\r\n{stale}\r\nbuild/\r\n\r\n" + gi.write_bytes(raw.encode("utf-8")) + assert resolve_gitignore(gi, DEFAULT_ENTRIES).state == "update" + _apply(tmp_path) + out = gi.read_bytes().decode("utf-8") + # the user's CRLF lines and the trailing blank survive byte-for-byte; only the block changed. + assert "node_modules/\r\n" in out + assert "build/\r\n\r\n" in out + assert ".claude/worktrees/" in out and ".claude/old/" not in out + + +def test_create_appends_without_clobbering_no_final_newline(tmp_path): + gi = tmp_path / ".gitignore" + gi.write_text("*.log", encoding="utf-8") # no trailing newline + res = _apply(tmp_path) + assert res.status == "created" + text = gi.read_text() + assert text.startswith("*.log\n\n") # the user's line kept, separated by one blank line + assert _block(DEFAULT_ENTRIES) in text + + +def test_empty_existing_file_creates_block_without_leading_blank(tmp_path): + gi = tmp_path / ".gitignore" + gi.write_text("", encoding="utf-8") # exists but empty (0 bytes) + res = _apply(tmp_path) + assert res.status == "created" and "added block to" in res.detail + assert gi.read_text() == _block(DEFAULT_ENTRIES) + "\n" # no spurious leading blank line + + +# ── io_error: unreadable path is an ERROR, not a silent skip (review P2) ─────────── +def test_directory_at_gitignore_path_is_io_error(tmp_path): + # a directory sits where .gitignore should be — read_text raises, so it's io_error → error. + (tmp_path / ".gitignore").mkdir() + r = resolve_gitignore(tmp_path / ".gitignore", DEFAULT_ENTRIES) + assert r.state == "io_error" + res = _apply(tmp_path) + assert res.status == "error" and "cannot read" in res.detail + assert (tmp_path / ".gitignore").is_dir() # untouched + + +def test_io_error_surfaces_in_drift_not_in_sync(tmp_path): + (tmp_path / ".gitignore").mkdir() + report = detect(_plan_with_action(tmp_path)) + assert any(i.category == "gitignore" and i.direction == "modified" for i in report.items) + + +# ── empty entries: an empty block round-trips as ok (review #12) ─────────────────── +def test_empty_entries_produce_empty_block_that_round_trips(tmp_path): + res = _apply(tmp_path, entries=[]) + assert res.status == "created" + text = (tmp_path / ".gitignore").read_text() + assert text == f"{GITIGNORE_BEGIN_MARKER}\n{GITIGNORE_END_MARKER}\n" + # idempotent: an empty managed block is recognized as ok, not endlessly re-created. + assert resolve_gitignore(tmp_path / ".gitignore", []).state == "ok" + assert _apply(tmp_path, entries=[]).status == "skipped" + + +# ── config: an entry colliding with a marker is rejected (review #11) ────────────── +def test_validate_rejects_entry_containing_marker(): + with pytest.raises(ConfigError, match="may not contain a rig-managed marker"): + validate({"version": 1, "gitignore": {"entries": [GITIGNORE_BEGIN_MARKER]}}) + with pytest.raises(ConfigError, match="may not contain a rig-managed marker"): + validate({"version": 1, "gitignore": {"entries": [f"x {GITIGNORE_END_MARKER}"]}}) + + +# ── disabled-but-installed block surfaces as drift (review P1) ───────────────────── +def test_disabled_category_still_flags_leftover_block(tmp_path): + # config opts out, but a prior apply left the managed block — status must NOT read "in sync". + (tmp_path / ".gitignore").write_text(f"node_modules/\n{_block(DEFAULT_ENTRIES)}\n", encoding="utf-8") + report = DriftReport() + check_disabled_gitignore(tmp_path, report) + assert any(i.category == "gitignore" and i.direction == "extra" for i in report.items) + + +def test_disabled_category_no_block_is_clean(tmp_path): + (tmp_path / ".gitignore").write_text("node_modules/\n", encoding="utf-8") + report = DriftReport() + check_disabled_gitignore(tmp_path, report) + assert not any(i.category == "gitignore" for i in report.items) + + +def test_disabled_category_no_file_is_clean(tmp_path): + report = DriftReport() + check_disabled_gitignore(tmp_path, report) # no .gitignore at all + assert not report.items + + +# ── update through the plan→apply path leaves drift clean (full loop) ────────────── +def test_end_to_end_build_apply_detect_in_sync(fake_agent_tools, tmp_path): + # the real path: build a plan from config (gitignore default ON) → run_plan → detect. + from riglib.catalog import Catalog + + repo = tmp_path / "repo" + repo.mkdir() + (repo / ".gitignore").write_text("# pre-existing user ignore\n.env\n", encoding="utf-8") + cat = Catalog.scan(str(fake_agent_tools)) + plan = build(_cfg({"skills": {"enabled": False}}, repo), cat, project_type="cli") + report = run_plan(plan) + assert not report.errors, [r.detail for r in report.errors] + text = (repo / ".gitignore").read_text() + assert ".env" in text # the user's prior content survives + assert ".claude/worktrees/" in text + # second apply is a no-op for the gitignore action + second = run_plan(plan) + assert all(r.status == "skipped" for r in second.results if r.action.category == "gitignore") + assert not any(i.category == "gitignore" for i in detect(plan).items) From 33e868f313ab649d6530bb6f3bb9a9c7b6c4b992 Mon Sep 17 00:00:00 2001 From: Alex Ultra Date: Tue, 16 Jun 2026 09:49:17 +0200 Subject: [PATCH 2/2] refactor(rig): drift reuses the apply marker scanner; canonicalize gitignore imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the disabled-gitignore drift check detect rig's block the same way apply does, and source the gitignore constants from their home module. - drift.check_disabled_gitignore now matches the begin marker via _find_marker_lines (the offset scanner resolve_gitignore already uses) read with newline="" instead of an ad-hoc splitlines()/.strip() pass — so drift detection can never diverge from apply (CRLF / whitespace-padded marker lines match identically; _find_marker_lines returns an empty list when absent, so the truthiness check is correct). - Import GITIGNORE_BEGIN/END_MARKER and GITIGNORE_DEFAULT_ENTRIES from riglib.config (their canonical schema-layer home) in drift, plan, and tests, instead of re-exporting through runner; lift plan's deferred import to module top (no import cycle — config already imported there). runner's header comment updated to match. Co-Authored-By: Claude Opus 4.8 --- riglib/actions/runner.py | 3 +-- riglib/drift.py | 12 ++++++++---- riglib/plan.py | 4 +--- tests/test_gitignore.py | 11 ++++++++--- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/riglib/actions/runner.py b/riglib/actions/runner.py index af2cf89..011bba2 100644 --- a/riglib/actions/runner.py +++ b/riglib/actions/runner.py @@ -1291,8 +1291,7 @@ def _do_provision_agents_symlink(action: Action, on_conflict: str) -> ActionResu # is preserved verbatim. apply and drift BOTH go through `resolve_gitignore` so they can never # disagree on what the desired block is or whether the file is in sync. The marker constants live # in `config.py` (the schema layer) so validation can reject a marker-colliding entry without an -# import cycle; they are imported at module top and re-used here (drift/tests reach for them via -# `runner.GITIGNORE_*`, which still resolves through this module's namespace). +# import cycle; this module imports them at top for its own block construction/detection. def gitignore_block_text(entries: list[str]) -> str: diff --git a/riglib/drift.py b/riglib/drift.py index 695e2c4..cb3eea7 100644 --- a/riglib/drift.py +++ b/riglib/drift.py @@ -21,6 +21,7 @@ from .actions import fsutil from .actions.runner import ( _ci_companion_files, + _find_marker_lines, _git_global, _launchctl_loaded, _read_crontab, @@ -32,7 +33,6 @@ github_ruleset_state, harness_settings_file, hook_bridge_entries, - GITIGNORE_BEGIN_MARKER, parse_mcp_command, resolve_agents_md, resolve_ci_workflow, @@ -40,6 +40,7 @@ schedule_plan_from_action, skill_harness_link_target, ) +from .config import GITIGNORE_BEGIN_MARKER from .github_ruleset import DEFAULT_RULESET_NAME from .plan import Action, InstallPlan @@ -317,16 +318,19 @@ def check_disabled_gitignore(repo_root: Path, report: DriftReport) -> None: ``.gitignore`` even after the config turns the category off. With the action gone from the plan, ``_check_gitignore`` never runs — so without this scan the leftover block would report as "in sync". Detect a present begin marker in the repo's ``.gitignore`` and report it as - disk→config drift (mirrors :func:`check_disabled_dispatcher` for the global dispatcher). + disk→config drift (mirrors :func:`check_disabled_dispatcher` for the global dispatcher). Marker + detection reuses :func:`_find_marker_lines` (the same offset-based scanner ``resolve_gitignore`` + uses) read with newline translation off, so detection never diverges from apply. """ gi = repo_root / ".gitignore" if not gi.is_file(): return try: - content = gi.read_text(encoding="utf-8") + with gi.open(encoding="utf-8", newline="") as fh: + content = fh.read() except OSError: return - if any(ln.strip() == GITIGNORE_BEGIN_MARKER for ln in content.splitlines()): + if _find_marker_lines(content, GITIGNORE_BEGIN_MARKER): report.items.append( DriftItem("extra", "gitignore", "block", gi, "gitignore disabled in config but the rig-managed block is still in .gitignore") diff --git a/riglib/plan.py b/riglib/plan.py index c871a36..1073bba 100644 --- a/riglib/plan.py +++ b/riglib/plan.py @@ -21,7 +21,7 @@ from typing import Any from .catalog import Catalog, Item -from .config import LoadedConfig +from .config import GITIGNORE_DEFAULT_ENTRIES, LoadedConfig from .github_ruleset import GITHUB_RULESET_DEFAULTS @@ -724,8 +724,6 @@ def _build_gitignore(config: LoadedConfig, plan: InstallPlan) -> None: so the plan emits ONE idempotent action carrying the resolved entries, anchored at the repo root; no carrier in agent-tools. """ - from .config import GITIGNORE_DEFAULT_ENTRIES - gi = config.data.get("gitignore") if gi is None: gi = {} diff --git a/tests/test_gitignore.py b/tests/test_gitignore.py index f5c6430..963d5ad 100644 --- a/tests/test_gitignore.py +++ b/tests/test_gitignore.py @@ -19,14 +19,19 @@ import pytest from riglib.actions.runner import ( - GITIGNORE_BEGIN_MARKER, - GITIGNORE_END_MARKER, _do_provision_gitignore, gitignore_block_text, resolve_gitignore, run_plan, ) -from riglib.config import GITIGNORE_DEFAULT_ENTRIES, ConfigError, LoadedConfig, validate +from riglib.config import ( + GITIGNORE_BEGIN_MARKER, + GITIGNORE_DEFAULT_ENTRIES, + GITIGNORE_END_MARKER, + ConfigError, + LoadedConfig, + validate, +) from riglib.drift import DriftReport, check_disabled_gitignore, detect from riglib.plan import Action, InstallPlan, build