diff --git a/README.md b/README.md index eac6994..9e46649 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,7 @@ Or run straight from a checkout — `uv run bin/rig …` / `python3 bin/rig …` | `rig status` | Detect + report **drift in both directions** (config says X but disk has Y; disk has Z not in config). | | `rig doctor` | Detect + (offer to) install every tool rig/agent-tools need, across brew / apt / dnf / pacman / zypper. `--yes` installs non-interactively. | | `rig export` | Write a starter `rig.yaml` from detected defaults without a TUI (recommends **auto-mode on**). | +| `rig config get\|set` | **The recommended way to read/change one setting.** `get ` reads a nested key (e.g. `harness.auto_mode`); `set ` writes it (sensible scalar coercion, fail-closed validation) and **reconciles** (runs the `apply` engine) so the change takes effect immediately. `--global` targets `~/.config/rig/config.yaml`; `--no-apply` writes only. | | `rig install-skill` | Register the `rig` agent skill so harnesses auto-discover it. | | `rig stats show` | **Tool-adoption analytics.** Parse the session logs of every agent harness on the machine and report how often each tool is invoked, bucketed into baseline / ours / external-advertised / other — so you can see whether the rig + agent-tools ecosystem is actually being used vs the built-in baseline. `` `--format json|tui|web` ``, breakdowns by repo/harness, a daily trend (the `json` output additionally exposes the weekly series). | diff --git a/docs/config-schema.md b/docs/config-schema.md index c5fea80..eb12736 100644 --- a/docs/config-schema.md +++ b/docs/config-schema.md @@ -748,6 +748,64 @@ tests/smoke never touch the real launchd domain or delete the predecessor file. --- +## Editing a single key — `rig config get|set` + +The recommended way to read or change one setting without hand-editing YAML. `` is +dot-notation into the tree above (`harness.auto_mode`, `ci.items.secret-scan.tier`, +`defaults.on_conflict`). `--global` targets `~/.config/rig/config.yaml` (XDG-aware) instead +of `./rig.yaml`. + +```bash +rig config get harness.auto_mode # read one key (from ./rig.yaml) +rig config get harness.auto_mode --json # machine-readable (JSON value) +rig config get harness # a subtree prints as YAML +rig config get defaults.on_conflict --global # read the global config instead + +rig config set harness.auto_mode false # write, then RECONCILE (rig apply engine) +rig config set ci.items.secret-scan.tier warn # creates intermediate keys as needed +rig config set defaults.on_conflict overwrite --global # edit the global config +rig config set harness.auto_mode false --no-apply # write only, print the plan, skip apply +``` + +- **`get`** reads the single target file (NOT the cascade): `./rig.yaml`, or the global file + with `--global`. A missing file or absent path exits non-zero (fail-closed). `--json` emits + the raw JSON value; a mapping/list subtree prints as YAML; bools print as `true`/`false`. +- **`set`** coerces the value conservatively — `true`/`false` → bool, a plain integer → int, + `1.5` → float, `null`/`none`/`~` → null, everything else (including `1e3`, `nan`, `1_000`) + stays a string. Quote-wrap to force a literal string (`rig config set k '"true"'` stores the + string `"true"`). It creates intermediate mappings as needed, then guards the write with two + **pre-apply** gates: the schema (`config.validate` — enums/types, e.g. a bad `ci` `tier`) and + the catalog-backed plan build (`rig apply`'s engine — when a category is enabled, an unknown + item it references; a bad `agent_tools_source`; or any otherwise-unbuildable config). If + **either** gate rejects the + edit, the target file is rolled back to its prior contents and the command exits non-zero. + (Errors *during* the apply itself are reported and set a non-zero exit, but do not revert the + already-valid config — same as re-running `rig apply`.) Validation matches `rig apply`: a + typo'd nested key in a section *without* strict key-checking (e.g. `harness.aut_mode`) is + tolerated and simply has no effect, exactly as a hand-edited `rig.yaml` is; sections that *do* + enforce their key set (`models`, `agents_md`) reject an unknown key. +- A value that **starts with `-`** (and is not a negative number) needs the `--` separator so + argparse doesn't read it as a flag: `rig config set k -- -weird`. `get` errors go to stderr, + so `rig config get k --json | jq` keeps a clean stdout even when the key is missing. +- **Dot paths cannot address a key that itself contains a dot.** `` always splits on + `.`, so a (hypothetical) item id like `a.b` is unreachable — `ci.items.a.b.tier` nests + `items → a → b`. Every real catalog id is dash-cased (`secret-scan`), so this is a + non-issue in practice; for such a key, edit `rig.yaml` directly. +- **A repo-local `set` requires an existing `./rig.yaml`.** It edits a committed config; it does + not bootstrap one — run `rig init` (or `rig export -o rig.yaml`) first, so built-in defaults + never reconcile onto disk without a committed source of truth (the same guard `rig apply` + has). `--global` may create the machine-wide `~/.config/rig/config.yaml` if it is absent. +- **`set` rewrites the whole file** through rig's serializer, so it normalizes formatting and + **drops comments** — the value is the source of truth, not the surrounding YAML prose. It is + also **repo-scoped**: even a `--global` edit resolves and reconciles the current repo, so run + it from inside one (use `--no-apply` to skip the reconcile, not the repo resolution). +- After a successful `set`, rig **reconciles**: it runs the same plan + apply engine as + `rig apply`, scoped to the repo in front of you, so the change takes effect immediately. A + `--global` edit still converges the current repo (it reads the global layer via the cascade). + `--no-apply` writes the key and prints the resulting plan without converging. + +--- + ## Validation `apply`/`status`/`init` validate before touching disk and **fail closed** on: @@ -766,3 +824,17 @@ not an int >= 1, a non-bool `tmux` boolean knob, a non-mapping `gitignore` block unknown `tg_ctl` key, a non-bool `tg_ctl.enabled`/`tg_ctl.boot`, a non-string `tg_ctl.label`/`bun_path`/`tg_ctl_path`/`config_dir`, and an `agent_tools_source` that is not an agent-tools checkout. `--dry-run` prints the resolved plan and exits 0 without writing. + +`rig config set` validates in two stages: first the schema (`config.validate`) on the edited +target file, then — after writing — the catalog-backed plan build over the full cascade +(`rig apply`'s engine: bad `agent_tools_source`, unknown CI item, otherwise-unbuildable plan). +A malformed dot path, a non-mapping intermediate, a schema-rejected value, a write IO error, or +a plan-**build** failure aborts; the write is kept only if both stages pass — otherwise the file +is rolled back to its prior contents (a freshly-created file, and the immediate dir `set` created +for it, are removed). The second stage builds the same plan as `rig apply`, so a config that +cannot even be *planned* never persists. For a `--global` edit, the global file is **also** +validated **in isolation** (its own layer, no repo overlay) when it pins its own +`agent_tools_source`, so a catalog-backed break in the global config can't be masked by the +current repo overriding that key — it would otherwise persist and fail in every other repo. Errors that surface only while *executing* the plan +(a permission denied on a copy, say) are reported with a non-zero exit but leave the +already-valid config in place — identical to re-running `rig apply`. diff --git a/install.sh b/install.sh index d95e094..1a601f8 100755 --- a/install.sh +++ b/install.sh @@ -84,5 +84,6 @@ echo " Usage: rig doctor — check/install dependencies" echo " rig init — first-run onboarding (wizard, or --config/--yes headless)" echo " rig apply — reconcile the repo to rig.yaml (idempotent)" echo " rig status — report config↔disk drift (both ways)" +echo " rig config get|set — read/change one key (dot path), then reconcile" echo " rig --help — full usage" echo "" diff --git a/riglib/cli.py b/riglib/cli.py index 72fe144..3475637 100644 --- a/riglib/cli.py +++ b/riglib/cli.py @@ -13,11 +13,13 @@ rig doctor detect + (offer to) install required/optional dependencies rig export serialize default/current config to rig.yaml without a TUI rig stats tool-adoption analytics over agent-harness session logs (sub: `show`) + rig config get/set a single config key by dot path; set reconciles (the apply engine) """ from __future__ import annotations import argparse +import contextlib import sys from pathlib import Path @@ -55,6 +57,19 @@ def _bold(s: str) -> str: return _c("1", s) +def _fmt_scalar(value: object) -> str: + """Render a coerced scalar in YAML/CLI casing (true/false/null), not Python's repr. + + Shared by `config get` (the printed value) and `config set` (the confirmation line) so a + bool a user typed as ``false`` never echoes back as Python's ``False``. + """ + if isinstance(value, bool): + return "true" if value else "false" + if value is None: + return "null" + return str(value) + + # The GLOBAL/REPO layer headings, shared by the area summary and the per-item drift dump so the # two renderers can never disagree on the wording. _LAYER_HEADERS = { @@ -129,6 +144,33 @@ def _add_setup_args(parser: argparse.ArgumentParser) -> None: ep.add_argument("-o", "--output", default="rig.yaml", help="output path (default: rig.yaml)") ep.add_argument("--force", action="store_true", help="overwrite an existing file") + # `config get|set` — the recommended way to read/change a single setting. get reads a + # nested key; set writes it and RECONCILES (runs the apply engine) so the change takes + # effect immediately. --global targets ~/.config/rig/config.yaml instead of ./rig.yaml. + cp = sub.add_parser("config", help="get/set a single config key (dot path), then reconcile") + csub = cp.add_subparsers(dest="config_command", metavar="") + + def _add_config_target_args(parser: argparse.ArgumentParser) -> None: + # -C/--cwd and --global are identical on get and set; declare them once so the two + # never drift apart (same help text, same dest). + parser.add_argument("-C", "--cwd", default=".", help="repo root (default: cwd)") + parser.add_argument( + "--global", dest="is_global", action="store_true", + help="target the global config (~/.config/rig/config.yaml) instead of ./rig.yaml", + ) + + cg = csub.add_parser("get", help="read a nested config key (e.g. harness.mode)") + cg.add_argument("path", help="dot path into the config tree (e.g. ci.items.secret-scan.tier)") + _add_config_target_args(cg) + cg.add_argument("--json", action="store_true", help="emit the value as JSON") + + cs = csub.add_parser("set", help="write a nested config key, then reconcile (apply)") + cs.add_argument("path", help="dot path into the config tree (e.g. harness.mode)") + cs.add_argument("value", help="value to set (coerced: true/false/int/float/null, else string)") + _add_config_target_args(cs) + cs.add_argument("--no-apply", action="store_true", + help="write the key but skip the reconcile (print the resulting plan only)") + sub.add_parser("install-skill", help="register the rig agent skill with harnesses") _add_stats_parser(sub) @@ -187,6 +229,7 @@ def main(argv: list[str] | None = None) -> int: "status": cmd_status, "doctor": cmd_doctor, "export": cmd_export, + "config": cmd_config, "install-skill": cmd_install_skill, "stats": cmd_stats, } @@ -220,6 +263,35 @@ def _load_plan(cwd: str, config: str | None, project_type_override: str | None = return plan, loaded, env +def _validate_layer_in_isolation(layer_path: Path) -> None: + """Build a plan over ONE config file alone (no cascade), so a catalog-backed value the file + DECLARES can't be masked by a layer that merges over it. + + A `rig config set … --global` edit is otherwise only validated by the merged cascade + (`_load_plan`), where a repo's `rig.yaml` overriding the just-broken global key (e.g. a bad + `agent_tools_source`, or a global catalog item ref) hides the breakage — the write persists a + config that fails in every OTHER repo. Loading the file as the sole explicit layer with + `include_global=False` (its own directory has no `rig.yaml` to overlay) surfaces those errors + against the file itself. Raises ConfigError / CatalogError / PlanError on rejection. + + Scope: only what the file ITSELF declares. A global file legitimately omits + `agent_tools_source` (the checkout is supplied per-repo or via env), so we only run the + catalog-backed plan build when this file pins its own `agent_tools_source` — there is nothing + global-specific to catalog-validate otherwise, and demanding an independently-resolvable + checkout would reject a valid deferring global config. Schema (`config.load`) still runs either + way. + """ + from .catalog import Catalog + from .config import load + from .plan import build + + loaded = load(layer_path.parent, explicit_config=layer_path, include_global=False) + if loaded.agent_tools_source is None: + return # no own catalog coordinate to mask — schema validation already ran in load() + catalog = Catalog.scan(loaded.agent_tools_source) + build(loaded, catalog) + + def _print_plan(plan) -> None: print(_bold(f"\nPlan: {len(plan)} action(s) [on_conflict={plan.on_conflict}]")) for a in plan.actions: @@ -885,6 +957,211 @@ def cmd_export(args: argparse.Namespace) -> int: return 0 +def _config_target(args: argparse.Namespace, *, need_repo: bool) -> tuple[Path, Path | None]: + """Resolve (config_file, repo_root) for a `config get|set` invocation. + + --global → the XDG global config; otherwise the per-repo ./rig.yaml at the detected git + root (so the command works from any subdirectory, like apply/status). + + ``need_repo`` gates the repo lookup: ``set`` always reconciles the repo afterwards (even a + --global edit must converge the repo in front of you), so it needs the root. A ``get + --global`` does NOT touch any repo, so it must NOT require one — ``detect_environment`` + is skipped, and ``rig config get … --global`` works outside any git repo. + """ + from .config import global_config_path, repo_config_path + from .detect import detect_environment + + if args.is_global and not need_repo: + return global_config_path(), None + env = detect_environment(Path(args.cwd).resolve()) + target = global_config_path() if args.is_global else repo_config_path(env.repo_root) + return target, env.repo_root + + +def _read_target_yaml(path: Path): + """Parse a single config file to a dict (NOT the cascade), or None if it doesn't exist. + + Delegates to the loader's single-file reader, which already wraps YAML errors and rejects + a non-mapping top level as a fail-closed :class:`ConfigError`. + """ + from .config import read_yaml_file + + if not path.is_file(): + return None + return read_yaml_file(path) + + +def cmd_config(args: argparse.Namespace) -> int: + if not getattr(args, "config_command", None): + print(_err("error: `rig config` needs a subcommand: get or set")) + print(_dim(" rig config get # read a key")) + print(_dim(" rig config set # write a key, then reconcile")) + return 2 + if args.config_command == "get": + return _cmd_config_get(args) + return _cmd_config_set(args) + + +def _cmd_config_get(args: argparse.Namespace) -> int: + from .config import ConfigError, get_path + + try: + target, _repo_root = _config_target(args, need_repo=False) + data = _read_target_yaml(target) + if data is None: + raise ConfigError(f"config file not found: {target}") + value = get_path(data, args.path) + except ConfigError as exc: + # diagnostics go to STDERR so `get --json | jq` keeps a clean stdout (the exit code + # already signals failure); this matters most for the machine-readable --json path. + print(_err(f"error: {exc}"), file=sys.stderr) + return 2 + except Exception as exc: # noqa: BLE001 + # symmetric with set: anything unexpected (e.g. environment detection failing outside a + # git repo on a non-global get) must fail closed with a message, not a traceback. + print(_err(f"error: {type(exc).__name__}: {exc}"), file=sys.stderr) + return 2 + + if args.json: + import json + + # default=str so a YAML date/datetime (or any non-JSON scalar) serializes instead of + # throwing an unhandled TypeError — get must always fail soft on output. + print(json.dumps(value, default=str)) + elif isinstance(value, (dict, list)): + # a subtree: dump it as YAML so the structure is readable (and re-feedable to set's + # scalar children). Lazy yaml import keeps the no-yaml path light. + import yaml + + print(yaml.safe_dump(value, sort_keys=False, default_flow_style=False).rstrip()) + else: + print(_fmt_scalar(value)) # YAML/CLI casing for bool/null, str otherwise + return 0 + + +def _cmd_config_set(args: argparse.Namespace) -> int: + from .actions import run_plan + from .catalog import CatalogError + from .config import ConfigError, coerce_scalar, set_path, validate + from .plan import PlanError + from .state import SetupState + + try: + target, _repo_root = _config_target(args, need_repo=True) + except ConfigError as exc: + print(_err(f"error: {exc}"), file=sys.stderr) + return 2 + except Exception as exc: # noqa: BLE001 — environment detection failing must fail soft + print(_err(f"error: {type(exc).__name__}: {exc}"), file=sys.stderr) + return 2 + + # Refuse a repo-local `set` when ./rig.yaml does not exist yet: starting from {} and + # reconciling would let built-in defaults mutate disk with no committed source of truth — + # the same hazard `rig apply`'s no-config guard prevents. `rig init` (or `rig export`) + # creates the file first. (--global may legitimately create the machine-wide file, so it is + # not guarded here; its second gate still validates over the cascade.) + if not args.is_global and not target.is_file(): + print(_err(f"error: no {target} — run `rig init` (or `rig export -o rig.yaml`) first.")) + print(_dim(" `config set` edits an existing committed config; it does not bootstrap one.")) + return 2 + + # `scope` is a removed key (the cascade is location-based). Refuse to SET it — the + # recommended editor must never (re)introduce a dead setting, even though the loader still + # tolerates it in old committed files. + if args.path == "scope" or args.path.startswith("scope."): + print(_err("error: `scope` is a removed setting — the cascade is by location " + "(global vs repo), not a flag. Nothing to set."), file=sys.stderr) + return 2 + + try: + data = _read_target_yaml(target) or {} + # drop any legacy `scope` already in the file (mirrors config.load): we re-serialize the + # whole file, so leaving it would re-emit a key the schema no longer recognizes. + data.pop("scope", None) + value = coerce_scalar(args.value) + set_path(data, args.path, value) + # First gate: schema validation of the WHOLE edited tree (enum/type checks). This + # catches e.g. harness.auto_mode="yes" before anything touches disk. + validate(data) + except ConfigError as exc: + print(_err(f"error: {exc}")) + return 2 + except Exception as exc: # noqa: BLE001 + # nothing has touched disk yet (read/coerce/validate only). Anything unexpected here + # must still fail closed with a message, not a traceback. + print(_err(f"error: {type(exc).__name__}: {exc}")) + return 2 + + # Capture the previous bytes BEFORE writing so a write IO error OR a SECOND, deeper + # validation failure (catalog-backed: a bad agent_tools_source or an unknown CI item — + # these live in plan.build(), not config.validate()) can fully ROLL BACK. The file must be + # untouched on ANY failure, exactly as the docs promise. The repo file carries the + # committed-source-of-truth header; the global file is a plain machine-wide dump. + original = target.read_text(encoding="utf-8") if target.is_file() else None + parent_existed = target.parent.exists() # so rollback only removes a dir WE created + + def _rollback() -> bool: + """Restore the file to its pre-set state. Returns True on success, False if the restore + itself failed (so the caller can tell the user the file is NOT actually untouched, + rather than printing a false reassurance).""" + try: + if original is None: + target.unlink(missing_ok=True) # we created the file; remove our partial write + # only remove the parent if WE created it (a fresh ~/.config/rig for --global) — + # never rmdir a pre-existing dir like the repo root. + if not parent_existed: + with contextlib.suppress(OSError): + target.parent.rmdir() # no-op if non-empty (e.g. a repo root has .git) + else: + target.write_text(original, encoding="utf-8") # restore prior contents + except OSError: + return False + return True + + # Write, then second gate: build the plan from the on-disk cascade (so a --global edit is + # validated together with the repo layer it merges into). A write IO error or a plan + # failure means the edit didn't take → roll back and fail closed. + try: + target.parent.mkdir(parents=True, exist_ok=True) + state = SetupState.from_dict(data) + if args.is_global: + target.write_text(state.to_yaml(), encoding="utf-8") + # Validate the global file ALONE first: the cascade plan below merges the repo + # overlay over it, which can mask a catalog-backed error in the global layer (a repo + # `rig.yaml` overriding the just-broken key). Check the written file in isolation so a + # globally-broken config never persists just because THIS repo happens to override it. + _validate_layer_in_isolation(target) + else: + state.write(target) + plan, _loaded, _env = _load_plan(args.cwd, config=None) + except Exception as exc: # noqa: BLE001 + # the write happened (or partially did) — ANY failure from here (catalog/plan rejection, + # a write IO error, or a serializer error on data the first gate let through) must roll + # the file back and fail closed. Never leave a written-but-unreconcilable config behind. + restored = _rollback() + kind = "" if isinstance(exc, (ConfigError, CatalogError, PlanError, OSError)) else f"{type(exc).__name__}: " + print(_err(f"error: {kind}{exc}")) + if restored: + print(_dim(f" (config not changed — {target} left untouched)")) + else: + # the restore itself failed — be honest: the file may hold the rejected edit. + print(_err(f" WARNING: could not restore {target} — it may contain the rejected edit")) + return 2 + + print(_ok(f"set {args.path} = {_fmt_scalar(value)} → {target}")) + + # RECONCILE: a config change only matters once the disk reflects it. Run the SAME apply + # engine `rig apply` uses (scoped to the repo in front of you — a --global edit still has + # to converge this repo). --no-apply writes the key and prints the plan only. + _print_plan(plan) + if args.no_apply: + print(_dim("\n(--no-apply: config written, nothing reconciled)")) + return 0 + report = run_plan(plan) + _print_results(report) + return 1 if report.errors else 0 + + def cmd_install_skill(args: argparse.Namespace) -> int: from .install import install_skill diff --git a/riglib/config.py b/riglib/config.py index 454161e..8cd49b8 100644 --- a/riglib/config.py +++ b/riglib/config.py @@ -67,8 +67,116 @@ def repo_config_path(repo_root: Path) -> Path: return repo_root / CONFIG_FILENAME -def _load_yaml(path: Path) -> dict[str, Any]: - """Parse a YAML file to a dict. Lazy yaml import; empty file → {}.""" +# ── dot-path get/set + scalar coercion (the `rig config get|set` engine) ──────────── +# A dot path indexes the YAML mapping tree: ``harness.mode`` → ``data["harness"]["mode"]``. +# Used ONLY for the targeted config edit/read commands — not for the cascade loader, which +# always merges whole layers. Kept here (next to the loader/validator) so the path syntax +# and the schema it indexes never drift into a separate module. + + +def split_path(dotted: str) -> list[str]: + """Split a dot path into segments, rejecting empties (``a..b``, leading/trailing dot). + + Fail-closed: a malformed path is a :class:`ConfigError`, not a silent no-op — a typo in + ``rig config set`` must abort before it writes a key the schema can't reach. + """ + if not dotted or not dotted.strip(): + raise ConfigError("empty config path") + parts = [p.strip() for p in dotted.split(".")] + if any(p == "" for p in parts): + # empty segment ("a..b", a leading/trailing dot, or a whitespace-only segment "a. .b") + raise ConfigError(f"invalid config path {dotted!r}: empty segment") + return parts + + +def get_path(data: dict[str, Any], dotted: str) -> Any: + """Read a nested value by dot path. Raises :class:`ConfigError` if the path is absent. + + Traversal fails closed when an intermediate segment is missing OR is a non-mapping + (``a.b`` where ``a`` is a scalar/list) — both are "the path does not exist", reported + with the exact segment that broke so the caller can fix the typo. + """ + node: Any = data + walked: list[str] = [] + for seg in split_path(dotted): + if not isinstance(node, dict) or seg not in node: + where = ".".join(walked) or "" + raise ConfigError(f"config path {dotted!r} not found (no {seg!r} under {where})") + node = node[seg] + walked.append(seg) + return node + + +def set_path(data: dict[str, Any], dotted: str, value: Any) -> None: + """Set a nested value by dot path, creating intermediate mappings in place. + + Fail-closed if an existing intermediate segment is a non-mapping: overwriting + ``ci.items`` (a dict) by setting ``ci.items.x`` is fine, but setting ``a.b`` when ``a`` + is already a scalar would silently clobber it — refuse instead, naming the segment. + """ + parts = split_path(dotted) + node = data + for seg in parts[:-1]: + if seg not in node: + node[seg] = {} + elif not isinstance(node[seg], dict): + raise ConfigError( + f"cannot set {dotted!r}: {seg!r} is a {type(node[seg]).__name__}, not a mapping" + ) + node = node[seg] + node[parts[-1]] = value + + +def coerce_scalar(raw: str) -> Any: + """Coerce a CLI string to the obvious scalar type: bool, int, float, null, else string. + + The shell hands every value to us as text; ``tier=block`` must stay a string while + ``auto_mode=true`` must become a real bool (the schema validators reject a string there). + Quote-wrap (``'"true"'`` → the string ``true``) forces a literal string for the rare case + a stringly-typed value collides with a keyword. + + Coercion is deliberately CONSERVATIVE — only the unambiguous forms convert, everything + else stays a string. So ``int``/``float`` accept a plain optionally-signed number but + NOT Python's surprising extras (``1_000``, ``nan``, ``inf``, whitespace), which a config + author would never mean to type and which would otherwise smuggle a NaN/underscore-int + into the tree. + """ + if len(raw) >= 2 and raw[0] == raw[-1] and raw[0] in ("'", '"'): + return raw[1:-1] # explicit string escape: "true" / '12' stay strings + low = raw.lower() + if low in ("true", "false"): + return low == "true" + if low in ("null", "none", "~"): + return None + # require ASCII so str.isdigit()'s Unicode digits (superscripts like '²') — which int()/ + # float() then choke on — can't sneak past as a number; they stay strings. The try/except + # is a belt-and-suspenders guard so coercion NEVER raises (fail-closed → fall back to str). + if raw.isascii(): + body = raw[1:] if raw[:1] in ("+", "-") else raw # allow a single leading sign + # plain integer, but a leading zero ("0644", "007") stays a string — it's almost + # always a file mode / zero-padded id / version the author means literally, and int() + # would silently drop the zero. + if body.isdigit() and not (len(body) > 1 and body[0] == "0"): + try: + return int(raw) + except ValueError: + pass + # plain decimal float: digits, exactly one dot, optional sign — rejects nan/inf/1e3/_. + elif body.count(".") == 1 and body.replace(".", "", 1).isdigit(): + try: + return float(raw) + except ValueError: + pass + return raw + + +def read_yaml_file(path: Path) -> dict[str, Any]: + """Parse a single YAML config file to a dict (fail-closed). Lazy yaml import; empty → {}. + + Public so the targeted `config get|set` commands can read ONE file (not the cascade) and + inherit the same error handling: YAML syntax errors and a non-mapping top level both raise + :class:`ConfigError` rather than leaking a PyYAML traceback or a list/scalar. + """ import yaml # lazy: keeps `rig --help` dependency-free try: @@ -188,7 +296,7 @@ def _merge_layer(path: Path, label: str) -> None: ``_deep_merge`` returns a fresh dict rather than mutating in place. """ nonlocal merged - data = _load_yaml(path) + data = read_yaml_file(path) merged = _deep_merge(merged, data) for k in data: key_sources[k] = path @@ -236,7 +344,9 @@ def validate(data: dict[str, Any]) -> None: raise ConfigError(f"unknown top-level key(s): {', '.join(sorted(unknown))}") version = data.get("version", 1) - if not isinstance(version, int): + # bool is an int subclass and True == 1, so guard it explicitly — `version: true` is a typo, + # not v1 (this also blocks `rig config set version true` from coercing past the check). + if isinstance(version, bool) or not isinstance(version, int): raise ConfigError(f"version must be an int, got {version!r}") if version != 1: raise ConfigError(f"unsupported config version {version} (this rig supports v1)") diff --git a/riglib/install.py b/riglib/install.py index 4ec20da..6ba1772 100644 --- a/riglib/install.py +++ b/riglib/install.py @@ -22,9 +22,10 @@ Set up a repository (and a dev machine) from a committed rig.yaml by applying agent-tools content — skills, agent-hooks, git-hooks/dispatcher, CI gates, MCP. Use when the user wants to bootstrap a repo's guardrails, reconcile it to its config, - check for config/disk drift, or install the toolchain dependencies. Commands: - `rig init` (wizard or --config/--yes), `rig apply` (idempotent reconcile), - `rig status` (two-way drift), `rig doctor` (dependency bootstrap). + check for config/disk drift, change a single setting, or install the toolchain + dependencies. Commands: `rig init` (wizard or --config/--yes), `rig apply` (idempotent + reconcile), `rig status` (two-way drift), `rig config get|set` (read/change one key, then + reconcile), `rig doctor` (dependency bootstrap). metadata: author: alex-mextner repo: https://github.com/alex-mextner/rig-cli @@ -42,6 +43,8 @@ rig apply # reconcile the repo to rig.yaml (idempotent) rig apply --dry-run # print the resolved plan, write nothing rig status # report drift BOTH ways (config↔disk) +rig config get harness.auto_mode # read one nested key (--global for ~/.config/rig/config.yaml) +rig config set harness.auto_mode false # write one key, then reconcile (the apply engine) rig doctor # detect deps; --yes to install across brew/apt/dnf/pacman/zypper rig export -o rig.yaml # write a starter rig.yaml from detected defaults ``` diff --git a/tests/test_config_getset.py b/tests/test_config_getset.py new file mode 100644 index 0000000..393f630 --- /dev/null +++ b/tests/test_config_getset.py @@ -0,0 +1,598 @@ +"""`rig config get|set` — the targeted read/edit-then-reconcile command. + +Covers the pure dot-path engine (split/get/set/coerce in riglib.config) and the CLI +front-end (riglib.cli) end to end: get a nested key, scalar coercion on set, intermediate +key creation, the --global target, fail-closed validation, and that `set` triggers the same +plan+apply reconcile `rig apply` runs (mocked so no real install touches disk). +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from riglib import config +from riglib.cli import main + + +# ── pure engine: split / get / set / coerce ──────────────────────────────────────── +def test_get_path_reads_nested_key(): + data = {"harness": {"mode": "auto"}, "ci": {"items": {"secret-scan": {"tier": "block"}}}} + assert config.get_path(data, "harness.mode") == "auto" + assert config.get_path(data, "ci.items.secret-scan.tier") == "block" + + +def test_get_path_missing_fails_closed(): + with pytest.raises(config.ConfigError, match="not found"): + config.get_path({"harness": {}}, "harness.mode") + + +def test_get_path_through_scalar_fails_closed(): + # harness.mode is a scalar; indexing into it (harness.mode.x) is "not found", not a crash. + with pytest.raises(config.ConfigError, match="not found"): + config.get_path({"harness": {"mode": "auto"}}, "harness.mode.x") + + +def test_set_path_creates_intermediate_mappings(): + data: dict = {} + config.set_path(data, "a.b.c", 1) + assert data == {"a": {"b": {"c": 1}}} + + +def test_set_path_refuses_to_clobber_scalar_intermediate(): + with pytest.raises(config.ConfigError, match="not a mapping"): + config.set_path({"a": 1}, "a.b", 2) + + +def test_split_path_rejects_empty_segments(): + with pytest.raises(config.ConfigError, match="empty segment"): + config.split_path("a..b") + with pytest.raises(config.ConfigError, match="empty"): + config.split_path("") + + +@pytest.mark.parametrize( + "raw,expected", + [ + ("true", True), + ("false", False), + ("True", True), + ("42", 42), + ("-7", -7), + ("3.14", 3.14), + ("null", None), + ("none", None), + ("~", None), + ("block", "block"), + ("warn", "warn"), + ("~/.agents/skills", "~/.agents/skills"), + ], +) +def test_coerce_scalar(raw, expected): + assert config.coerce_scalar(raw) == expected + + +@pytest.mark.parametrize("raw,expected", [('"true"', "true"), ("'42'", "42"), ('"a"', "a")]) +def test_coerce_scalar_quote_wrap_forces_string(raw, expected): + # a quote-wrapped keyword/number stays a literal string, never the bool/int + out = config.coerce_scalar(raw) + assert out == expected + assert isinstance(out, str) + + +@pytest.mark.parametrize("raw", ["nan", "inf", "-inf", "1e3", "1_000", " 3 ", "0x10", "²", "².³"]) +def test_coerce_scalar_rejects_surprising_numbers(raw): + # conservative coercion: Python's int()/float() extras must NOT smuggle a NaN/odd int in, + # and a Unicode superscript ('²') that str.isdigit() calls a digit but int() chokes on must + # NOT raise — all stay strings, fail-closed. + out = config.coerce_scalar(raw) + assert isinstance(out, str) + assert out == raw + + +# ── CLI: get ─────────────────────────────────────────────────────────────────────── +def _w(p: Path, s: str) -> None: + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(s, encoding="utf-8") + + +def test_cli_get_nested_key(tmp_path, capsys, monkeypatch): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w(repo / "rig.yaml", "version: 1\nharness: {mode: auto, auto_mode: true}\n") + rc = main(["config", "get", "harness.mode", "-C", str(repo)]) + assert rc == 0 + assert capsys.readouterr().out.strip() == "auto" + + +def test_cli_get_bool_prints_yaml_casing(tmp_path, capsys, monkeypatch): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w(repo / "rig.yaml", "version: 1\nharness: {auto_mode: true}\n") + rc = main(["config", "get", "harness.auto_mode", "-C", str(repo)]) + assert rc == 0 + assert capsys.readouterr().out.strip() == "true" # not Python's "True" + + +def test_cli_get_json_output(tmp_path, capsys, monkeypatch): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w(repo / "rig.yaml", "version: 1\nharness: {auto_mode: true}\n") + rc = main(["config", "get", "harness.auto_mode", "-C", str(repo), "--json"]) + assert rc == 0 + assert capsys.readouterr().out.strip() == "true" # JSON bool + + +def test_cli_get_json_string_is_quoted(tmp_path, capsys, monkeypatch): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w(repo / "rig.yaml", "version: 1\nharness: {mode: auto}\n") + rc = main(["config", "get", "harness.mode", "-C", str(repo), "--json"]) + assert rc == 0 + assert capsys.readouterr().out.strip() == '"auto"' # JSON-quoted string, not bare auto + + +def test_cli_get_subtree_prints_yaml(tmp_path, capsys, monkeypatch): + # `get` on a mapping prints the subtree as YAML (sort_keys=False keeps insertion order). + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w(repo / "rig.yaml", "version: 1\nharness: {mode: auto, auto_mode: true}\n") + rc = main(["config", "get", "harness", "-C", str(repo)]) + assert rc == 0 + out = capsys.readouterr().out + import yaml + + assert yaml.safe_load(out) == {"mode": "auto", "auto_mode": True} + assert out.index("mode") < out.index("auto_mode") # insertion order preserved + + +def test_cli_get_json_on_subtree(tmp_path, capsys, monkeypatch): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w(repo / "rig.yaml", "version: 1\nharness: {mode: auto, auto_mode: true}\n") + rc = main(["config", "get", "harness", "-C", str(repo), "--json"]) + assert rc == 0 + import json + + assert json.loads(capsys.readouterr().out) == {"mode": "auto", "auto_mode": True} + + +def test_cli_get_missing_path_fails_closed(tmp_path, capsys, monkeypatch): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w(repo / "rig.yaml", "version: 1\nharness: {mode: auto}\n") + rc = main(["config", "get", "harness.nonexistent", "-C", str(repo)]) + assert rc == 2 + captured = capsys.readouterr() + assert "not found" in captured.err # diagnostics on stderr, not stdout + assert captured.out == "" # stdout stays clean (matters for --json piping) + + +def test_cli_get_missing_file_fails_closed(tmp_path, capsys, monkeypatch): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + repo.mkdir() + rc = main(["config", "get", "harness.mode", "-C", str(repo)]) + assert rc == 2 + assert "not found" in capsys.readouterr().err + + +def test_cli_get_json_error_keeps_stdout_clean(tmp_path, capsys, monkeypatch): + # the machine-readable contract: on error, stdout carries NO partial/garbage output so a + # `get --json | jq` pipe never chokes on a non-JSON error line. + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w(repo / "rig.yaml", "version: 1\nharness: {mode: auto}\n") + rc = main(["config", "get", "harness.nope", "-C", str(repo), "--json"]) + assert rc == 2 + captured = capsys.readouterr() + assert captured.out == "" # nothing on stdout + assert "not found" in captured.err + + +# ── CLI: set (reconcile mocked) ───────────────────────────────────────────────────── +@pytest.fixture +def _mock_apply(monkeypatch): + """Replace the apply engine so `config set` reconcile is observable, never real. + + `_cmd_config_set` does `from .actions import run_plan`, which binds the name from the + `riglib.actions` package namespace — so that is the seam to patch. + """ + import riglib.actions as actions_pkg + from riglib.actions.runner import ApplyReport + + calls: list = [] + + def _fake_run_plan(plan, **_kwargs): # signature-compatible stub + calls.append(plan) + return ApplyReport(results=[]) + + monkeypatch.setattr(actions_pkg, "run_plan", _fake_run_plan) + return calls + + +def test_cli_set_scalar_coercion_writes_bool(tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w( + repo / "rig.yaml", + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nci: {enabled: false}\n" + "mcp: {enabled: false}\ngit_hooks: {dispatcher: {enabled: false}}\n" + "harness: {auto_mode: true}\n", + ) + rc = main(["config", "set", "harness.auto_mode", "false", "-C", str(repo)]) + assert rc == 0 + written = config.load(repo) + assert written.data["harness"]["auto_mode"] is False # real bool, not the string "false" + assert len(_mock_apply) == 1 # reconcile ran + + +def test_cli_set_creates_intermediate_keys(tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w( + repo / "rig.yaml", + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n", + ) + # ci.items.secret-scan.tier — none of items/secret-scan exist yet + rc = main(["config", "set", "ci.items.secret-scan.tier", "warn", "-C", str(repo)]) + assert rc == 0 + written = config.load(repo) + assert written.data["ci"]["items"]["secret-scan"]["tier"] == "warn" + + +def test_cli_set_validation_rejects_bad_value(tmp_path, capsys, monkeypatch, _mock_apply): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + original = "version: 1\nci: {items: {secret-scan: {tier: block}}}\n" + _w(repo / "rig.yaml", original) + rc = main(["config", "set", "ci.items.secret-scan.tier", "loud", "-C", str(repo)]) + assert rc == 2 + assert "tier" in capsys.readouterr().out + # fail-closed: the bad value never reached disk + assert (repo / "rig.yaml").read_text(encoding="utf-8") == original + assert len(_mock_apply) == 0 # no reconcile on a rejected write + + +def test_cli_set_rejects_version_bool(tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply): + # `set version true` coerces to the bool True; validate() must reject it (bool is an int + # subclass and True == 1, so a naive isinstance check would let it through). File untouched. + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + original = ( + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n" + ) + _w(repo / "rig.yaml", original) + rc = main(["config", "set", "version", "true", "-C", str(repo)]) + assert rc == 2 + assert "version must be an int" in capsys.readouterr().out + assert (repo / "rig.yaml").read_text(encoding="utf-8") == original + assert len(_mock_apply) == 0 + + +def test_cli_set_repo_refuses_when_no_config_exists(tmp_path, capsys, monkeypatch, _mock_apply): + # a repo-local `set` with no ./rig.yaml must REFUSE (point to rig init) rather than start + # from {} and reconcile built-in defaults onto disk — the same hazard `rig apply` guards. + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + repo.mkdir() + assert not (repo / "rig.yaml").exists() + rc = main(["config", "set", "harness.auto_mode", "false", "-C", str(repo)]) + out = capsys.readouterr().out + assert rc == 2 + assert "rig init" in out + assert not (repo / "rig.yaml").exists() # nothing was bootstrapped + assert len(_mock_apply) == 0 # never reconciled + + +def test_cli_set_apply_error_keeps_config_and_returns_1(tmp_path, capsys, fake_agent_tools, monkeypatch): + # an error DURING apply (not a build failure) is reported with rc=1 but does NOT revert the + # already-valid config — identical to re-running `rig apply`. + import riglib.actions as actions_pkg + from riglib.actions.runner import ActionResult, ApplyReport + + def _erroring_run_plan(plan, **_kwargs): + fake_action = plan.actions[0] if plan.actions else None + results = [ActionResult(fake_action, "error", "permission denied")] if fake_action else [] + return ApplyReport(results=results) + + monkeypatch.setattr(actions_pkg, "run_plan", _erroring_run_plan) + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w( + repo / "rig.yaml", + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {universal: {all: true}, by_type: {enable: [cli]}}\n" + "agent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\nharness: {auto_mode: true}\n", + ) + rc = main(["config", "set", "harness.auto_mode", "false", "-C", str(repo)]) + assert rc == 1 # apply reported an error + # the valid config was NOT reverted — the edit persists + assert config.load(repo).data["harness"]["auto_mode"] is False + + +def test_cli_set_no_apply_skips_reconcile(tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w( + repo / "rig.yaml", + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n" + "harness: {auto_mode: true}\n", + ) + rc = main(["config", "set", "harness.auto_mode", "false", "-C", str(repo), "--no-apply"]) + out = capsys.readouterr().out + assert rc == 0 + assert "Plan:" in out # the plan is printed + assert "no-apply" in out + assert len(_mock_apply) == 0 # but apply never ran + # the write still happened + assert config.load(repo).data["harness"]["auto_mode"] is False + + +def test_cli_set_refuses_to_set_removed_scope_key(tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply): + # `set scope both` must be refused — the recommended editor never (re)introduces the removed + # `scope` key, even though the loader still tolerates it in legacy files. + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + original = ( + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n" + ) + _w(repo / "rig.yaml", original) + rc = main(["config", "set", "scope", "both", "-C", str(repo)]) + assert rc == 2 + assert "removed setting" in capsys.readouterr().err + assert (repo / "rig.yaml").read_text(encoding="utf-8") == original # untouched + assert len(_mock_apply) == 0 + + +def test_cli_set_drops_legacy_scope_key(tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply): + # an old config still carrying the removed `scope:` key must not have it re-emitted by a + # successful set (we reserialize the whole file, so it would otherwise linger). + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w( + repo / "rig.yaml", + f"version: 1\nscope: both\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n" + "harness: {auto_mode: true}\n", + ) + rc = main(["config", "set", "harness.auto_mode", "false", "-C", str(repo)]) + assert rc == 0 + import yaml + + written = yaml.safe_load((repo / "rig.yaml").read_text(encoding="utf-8")) + assert "scope" not in written # the legacy key was dropped, not re-emitted + assert written["harness"]["auto_mode"] is False + + +def test_cli_set_preserves_unknown_nested_key_round_trip(tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply): + # docs claim a typo'd nested key in a non-strict section is tolerated and survives, exactly + # like a hand-edited rig.yaml. Prove the serializer round-trip keeps it (from_dict copies + # the whole dict — it does NOT drop unknown nested keys). + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w( + repo / "rig.yaml", + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n" + "harness: {auto_mode: true, aut_mode: true}\n", # aut_mode is a typo (no effect) + ) + rc = main(["config", "set", "harness.auto_mode", "false", "-C", str(repo)]) + assert rc == 0 + written = config.load(repo).data + assert written["harness"]["auto_mode"] is False # the real key was edited + assert written["harness"]["aut_mode"] is True # the typo key survived the round-trip + + +def test_cli_set_global_targets_xdg_config(tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply): + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / ".config")) + repo = tmp_path / "repo" + # repo rig.yaml exists so the post-set reconcile has a config layer to plan from + _w( + repo / "rig.yaml", + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n", + ) + rc = main(["config", "set", "defaults.on_conflict", "overwrite", "-C", str(repo), "--global"]) + assert rc == 0 + gpath = config.global_config_path() + assert gpath.is_file() + assert config.global_config_path() == tmp_path / ".config" / "rig" / "config.yaml" + # the value landed in the GLOBAL file, not the repo file + import yaml + + gdata = yaml.safe_load(gpath.read_text(encoding="utf-8")) + assert gdata["defaults"]["on_conflict"] == "overwrite" + + +def test_cli_set_global_bad_value_not_masked_by_repo_override( + tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply +): + # A --global set of a catalog-backed key (agent_tools_source) to a BAD value must be rejected + # even when THIS repo's rig.yaml overrides the same key with a valid one. The merged cascade + # would mask the breakage (repo wins), persisting a global config that fails in every other + # repo. The global layer is validated in isolation, so the write rolls back. (codex P2) + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / ".config")) + gpath = config.global_config_path() + original_global = f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + _w(gpath, original_global) + repo = tmp_path / "repo" + # the repo overrides agent_tools_source with a VALID checkout → the cascade alone would pass + _w( + repo / "rig.yaml", + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n", + ) + bad = tmp_path / "not-a-checkout" + bad.mkdir() + rc = main(["config", "set", "agent_tools_source", str(bad), "-C", str(repo), "--global"]) + assert rc == 2 + out = capsys.readouterr().out + assert "is not an agent-tools checkout" in out + # fail-closed: the global file is rolled back to its prior, valid contents + assert gpath.read_text(encoding="utf-8") == original_global + assert len(_mock_apply) == 0 # rejected before any reconcile + + +def test_cli_get_outside_repo_fails_soft(tmp_path, capsys, monkeypatch): + # a non-global `get` from a plain dir (no .git, no rig.yaml) must fail closed with a clean + # message — never a traceback (symmetric with set's broad guard). + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + plain = tmp_path / "plain" + plain.mkdir() + rc = main(["config", "get", "harness.mode", "-C", str(plain)]) + assert rc == 2 + assert "error:" in capsys.readouterr().err + + +def test_cli_get_global_works_outside_a_repo(tmp_path, capsys, monkeypatch): + # `get --global` must NOT require a git repo — it reads only the global file. Point cwd at + # a plain dir (no .git) and assert it still resolves the value. + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / ".config")) + _w(config.global_config_path(), "version: 1\ndefaults: {on_conflict: skip}\n") + plain = tmp_path / "not-a-repo" + plain.mkdir() + rc = main(["config", "get", "defaults.on_conflict", "-C", str(plain), "--global"]) + assert rc == 0 + assert capsys.readouterr().out.strip() == "skip" + + +def test_cli_set_global_preserves_neighbors_and_stays_minimal( + tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply +): + # a --global set must edit ONE key and leave the global file otherwise as-is — it must NOT + # materialize defaults (that would turn the partial global overlay into a full config and + # change cascade semantics). Neighbor keys survive; no unexpected keys appear. + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / ".config")) + gpath = config.global_config_path() + _w(gpath, "version: 1\nskills: {harness_link: false}\ndefaults: {on_conflict: skip}\n") + repo = tmp_path / "repo" + _w( + repo / "rig.yaml", + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n", + ) + rc = main(["config", "set", "defaults.on_conflict", "overwrite", "-C", str(repo), "--global"]) + assert rc == 0 + import yaml + + gdata = yaml.safe_load(gpath.read_text(encoding="utf-8")) + assert gdata["defaults"]["on_conflict"] == "overwrite" # the edit + assert gdata["skills"]["harness_link"] is False # neighbor survived + # exactly the keys that were there before — no defaults block, no ci/mcp/harness injected + assert set(gdata.keys()) == {"version", "skills", "defaults"} + + +def test_cli_set_rolls_back_on_catalog_failure(tmp_path, capsys, monkeypatch, _mock_apply): + # a value config.validate() accepts but the CATALOG rejects (a bad agent_tools_source lives + # in plan.build(), not validate()) must roll the file back to its prior bytes — the docs + # promise "untouched on failure". + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + original = ( + "version: 1\n" + "agent_tools_source: /nonexistent/agent-tools\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n" + ) + _w(repo / "rig.yaml", original) + # on_conflict=overwrite is schema-valid, so validate() passes; the bad agent_tools_source + # only blows up at plan build → must roll back. + rc = main(["config", "set", "defaults.on_conflict", "overwrite", "-C", str(repo)]) + assert rc == 2 + assert (repo / "rig.yaml").read_text(encoding="utf-8") == original # rolled back + assert len(_mock_apply) == 0 # never reconciled + + +def test_cli_set_repo_file_stays_minimal(tmp_path, capsys, fake_agent_tools, monkeypatch, _mock_apply): + # the REPO write path goes through state.write() (with header) — it must NOT materialize + # defaults either. A set on a partial ./rig.yaml edits one key; neighbors survive and no + # default block (mcp/agents_md/models/…) is injected. + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "no-global")) + repo = tmp_path / "repo" + _w( + repo / "rig.yaml", + f"version: 1\nagent_tools_source: {fake_agent_tools}\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false, all: false}\n" + "harness: {auto_mode: true}\n", + ) + rc = main(["config", "set", "harness.auto_mode", "false", "-C", str(repo)]) + assert rc == 0 + import yaml + + written = yaml.safe_load((repo / "rig.yaml").read_text(encoding="utf-8")) + assert written["harness"]["auto_mode"] is False # the edit + assert written["skills"]["enabled"] is False # neighbor survived + # exactly the keys that were there — no defaults block, no models/agents_md injected + assert set(written.keys()) == { + "version", "agent_tools_source", "skills", "agent_hooks", "mcp", + "git_hooks", "ci", "harness", + } + + +def test_cli_set_global_fresh_file_rolled_back_on_failure(tmp_path, capsys, monkeypatch, _mock_apply): + # the original-is-None rollback branch: a --global set that CREATES a fresh + # ~/.config/rig/config.yaml but then fails the second gate (bad repo agent_tools_source) + # must delete both the file AND the freshly-created rig dir. + xdg = tmp_path / ".config" + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + repo = tmp_path / "repo" + _w( # repo config has a bad source → the post-write reconcile plan fails + repo / "rig.yaml", + "version: 1\nagent_tools_source: /nonexistent/agent-tools\n" + "skills: {enabled: false}\nagent_hooks: {enabled: false}\nmcp: {enabled: false}\n" + "git_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n", + ) + gpath = config.global_config_path() + assert not gpath.exists() # no global file yet → set will CREATE it + rc = main(["config", "set", "defaults.on_conflict", "overwrite", "-C", str(repo), "--global"]) + assert rc == 2 + assert not gpath.exists() # the freshly-created file was removed + assert not gpath.parent.exists() # and the rig/ dir we created was cleaned up + assert len(_mock_apply) == 0 + + +def test_cli_set_global_bad_value_caught_by_second_gate(tmp_path, capsys, monkeypatch, _mock_apply): + # prove the second (plan-build) gate validates the GLOBAL layer too: a bad agent_tools_source + # written into the global file must fail the cascade plan build and roll the global file back, + # even though the repo file is otherwise fine. + xdg = tmp_path / ".config" + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + gpath = config.global_config_path() + _w(gpath, "version: 1\ndefaults: {on_conflict: skip}\n") + repo = tmp_path / "repo" + _w( # repo has no source → the global agent_tools_source is what the cascade resolves + repo / "rig.yaml", + "version: 1\nskills: {enabled: false}\nagent_hooks: {enabled: false}\n" + "mcp: {enabled: false}\ngit_hooks: {dispatcher: {enabled: false}}\nci: {enabled: false}\n", + ) + original = gpath.read_text(encoding="utf-8") + rc = main(["config", "set", "agent_tools_source", "/nonexistent/agent-tools", "-C", str(repo), "--global"]) + assert rc == 2 + assert "not an agent-tools checkout" in capsys.readouterr().out + assert gpath.read_text(encoding="utf-8") == original # global file rolled back + assert len(_mock_apply) == 0 + + +def test_cli_config_no_subcommand_errors(tmp_path, capsys, monkeypatch): + rc = main(["config"]) + out = capsys.readouterr().out + assert rc == 2 + assert "get or set" in out diff --git a/tests/test_install.py b/tests/test_install.py index f9e5d37..c38cc0c 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -29,6 +29,17 @@ def test_install_skill_writes_md_and_harness_link(tmp_path, monkeypatch, capsys) assert (link / "SKILL.md").is_file() # the link resolves to the real skill +def test_install_skill_md_lists_config_command(tmp_path, monkeypatch): + # the embedded SKILL.md is the agent-facing command catalog — it must mention `rig config` + # so agents discover the recommended single-key edit path (help-docs-sync across files). + home = tmp_path / "home" + monkeypatch.setenv("HOME", str(home)) + assert install_skill() == 0 + md = (home / ".agents" / "skills" / SKILL_NAME / "SKILL.md").read_text(encoding="utf-8") + assert "rig config" in md + assert "rig config set" in md + + def test_install_skill_idempotent(tmp_path, monkeypatch, capsys): home = tmp_path / "home" monkeypatch.setenv("HOME", str(home))