From 268e53818dc7eafb58597f6b8e25910734fa5ac9 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Tue, 28 Jul 2026 11:12:50 +0200 Subject: [PATCH 1/2] nerve config init-repo + config-repo setup runbook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scaffolds a config repo — the tracked layout, a .gitignore that does not leak the machine-local overlay, and a CI workflow — plus a runbook for the PR-to-merge-to-sync flow. The generated CI validates the bundle without installing nerve from the repo under review, so a pull request cannot execute its own unreviewed Python in order to be judged. It enforces what the docs promise rather than a weaker subset, and the documented job matches the one actually generated instead of an idealized version of it. --portable-only now means it. Cron resolution is file-aware and would otherwise fall back to the machine-local directory, handing a run that was asked to judge the shared bundle alone a file the bundle does not contain — so a typo in a local file could fail a config repo that has nothing wrong with it. Paths outside the tracked subtree are reported as declined rather than silently substituted. Co-Authored-By: Claude --- docs/config.md | 157 ++++++- nerve/cli.py | 95 +++- nerve/config_pr.py | 7 + nerve/config_repo.py | 81 ++++ nerve/config_validate.py | 142 +++++- nerve/sync_service.py | 6 + nerve/templates/config-repo/README.md | 34 ++ nerve/templates/config-repo/gitignore | 31 ++ .../templates/config-repo/validate-config.yml | 59 +++ .../templates/skills/nerve-workspace/SKILL.md | 17 + tests/test_config_repo.py | 442 ++++++++++++++++++ tests/test_config_validate.py | 199 ++++++++ 12 files changed, 1248 insertions(+), 22 deletions(-) create mode 100644 nerve/config_repo.py create mode 100644 nerve/templates/config-repo/README.md create mode 100644 nerve/templates/config-repo/gitignore create mode 100644 nerve/templates/config-repo/validate-config.yml create mode 100644 tests/test_config_repo.py diff --git a/docs/config.md b/docs/config.md index ffaddac9..d01bf1ad 100644 --- a/docs/config.md +++ b/docs/config.md @@ -156,12 +156,16 @@ the quietest, because nothing reads it. `--portable-only` judges the tracked `/config/settings.yaml` layer on its own. Use it for a change headed to a shared repo: a local override can otherwise mask an invalid shared value, and, more often, a broken local file can -fail a shared bundle that has nothing wrong with it. Pass `--workspace` with it, -because with the machine layers dropped there is nothing left to read the -workspace location from and it falls back to the default tree. It fails outright -if it opened no file under the workspace's `config/`, so an empty directory, a -`settings.yml` typo or a `settings.yaml` left at the repo root cannot pass as -clean. Every run names the layers it read, in absolute paths: +fail a shared bundle that has nothing wrong with it. Cron is covered too. A +workspace carrying no jobs normally falls back to the machine-local +`~/.nerve/cron`, which is right for an un-migrated install but wrong here, so under +`--portable-only` only the repo's own `config/cron/` counts and anything skipped is +named in the report. Pass `--workspace` as well, because with the machine layers +dropped there is nothing left to read the workspace location from and it falls back +to the default tree. It fails outright if it opened no file under the workspace's +`config/`, so an empty directory, a `settings.yml` typo or a `settings.yaml` left +at the repo root cannot pass as clean. Every run names the layers it read, in +absolute paths: ``` [info] portable layer: /home/you/config-repo/config/settings.yaml @@ -182,10 +186,10 @@ jobs: - uses: astral-sh/setup-uv@v6 # nerve is not published to PyPI: the bare name is an unrelated project. - run: uv pip install --system "nerve @ git+https://github.com/ClickHouse/nerve" - - run: nerve config validate --workspace . --portable-only --strict-keys # Add --assume-lockdown if any instance served by this repo is locked and # the flag comes from the environment. Without it CI checks the unlocked # view and the lockdown checks never run. See "Lockdown" below. + - run: nerve config validate --workspace . --portable-only --strict-keys ``` Installing from the default branch keeps the validator at or ahead of your @@ -195,6 +199,11 @@ is too old to read shows up as a startup warning on the instance and in `nerve doctor`. If a key is ever retired upstream, `--strict-keys` flags it here first; drop the key, or install the ref you deploy (`nerve @ git+...@v1.2.3`) instead. +`nerve config init-repo` scaffolds this workflow, plus a +[gitleaks](https://github.com/gitleaks/gitleaks) scan ahead of it to catch a +credential pasted into a tracked file. See [Setting up the config +repo](#setting-up-the-config-repo) for the generated file and the end-to-end setup. + ## Hot-Reload Many config changes apply without a restart. Not all do, and a config file @@ -330,6 +339,140 @@ reports it too. `status` is the field to branch on: It describes *this call*. Whether the daemon is running the revision currently on disk is a different question, answered by `GET /api/config/sync` below. +## Setting up the config repo + +The workspace *is* the config repo: its root holds `config/` (settings, cron) and +`skills/`. To put it under review and sync it to an instance, turn the workspace +into a git repo with a GitHub remote, add CI validation, and optionally enable +lockdown. + +**1. Scaffold the repo files.** From the instance, or anywhere the workspace is +checked out: + +```bash +nerve config init-repo # scaffolds into the resolved workspace +nerve config init-repo --workspace ./ws # or an explicit path; --dry-run to preview +``` + +This writes four files, never overwriting an existing one: + +- `.github/workflows/validate-config.yml`: the CI check, shown below. +- `.gitignore`: keeps `config.local.yaml`, `.env`, `*.migrated`, databases and the + like out of the shared repo. Secrets belong in the environment, referenced as + `${ENV_VAR}`. +- `README.md`: a short explainer of the PR-based flow. +- `config/settings.yaml`: the commented portable settings scaffold, if the + workspace has none. An instance's workspace already has one; a bare directory + does not, and the CI check fails a repo with no `config/` at all rather than + passing it. + +**2. Create the GitHub repo and push.** The command prints these rather than +running them, since they need a repo name and auth: + +```bash +cd +git init && git add -A && git commit -m "Initial Nerve config" +gh repo create /nerve-config --private --source=. --remote=origin --push +``` + +**3. CI validation.** The scaffolded workflow scans for committed secrets and +validates the bundle on every PR. It needs no secrets or tokens of its own, since +`ClickHouse/nerve` is public: + +```yaml +name: validate-config + +# Validates the Nerve config bundle on every PR so a broken change can't be +# merged and then synced onto a live instance. See docs/config.md. + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +jobs: + validate: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + # Backstop for the one thing review is worst at spotting: a credential + # pasted into a tracked file. Secrets belong in the environment and are + # referenced from settings.yaml as ${ENV_VAR}. Runs before anything else + # is fetched, so it scans this repo and nothing else, and needs no token + # of its own. The image is used directly because the marketplace action + # asks organizations for a license key. + # + # The tag is pinned because an argument renamed in a later gitleaks would + # fail every config repo at once, with no pull request anywhere to explain + # it. (From 8.19 this subcommand is spelled `gitleaks dir `; bump the + # tag and the args together.) A false positive on a high-entropy string is + # silenced with a trailing `# gitleaks:allow`, or repo-wide in a + # .gitleaks.toml. + - name: Scan for committed secrets + uses: docker://ghcr.io/gitleaks/gitleaks:v8.18.4 + with: + args: detect --no-git --source=/github/workspace --redact + + # nerve is not published to PyPI, so this installs from git; the bare name + # would fetch an unrelated project. Tracking the default branch keeps the + # validator at or ahead of the instance, which is the safe direction for + # --strict-keys below: a newer validator knows every key the instance + # reads. To check against one release instead, append a ref (nerve@v1.2.3). + - uses: astral-sh/setup-uv@v6 + - run: uv pip install --system "nerve @ git+https://github.com/ClickHouse/nerve" + + # The config repo root IS the workspace (it holds config/ and skills/). + # + # --portable-only judges the tracked bundle on its own: no machine-local + # config.yaml, and no falling back to a machine-local cron directory when + # this repo carries no jobs. It fails outright if it opened no file under + # config/ at all, so a layout mistake cannot pass as clean. + # --strict-keys makes a misspelled key block the PR rather than warn. + # + # Unset ${ENV_VAR} secret refs are reported as info (CI has no secrets); + # add --strict-env to require them. If any instance served by this repo is + # locked through ${NERVE_LOCKDOWN}, add --assume-lockdown as well, or CI + # only ever checks the unlocked view that instance never loads. + - name: Validate config bundle + run: nerve config validate --workspace . --portable-only --strict-keys +``` + +That is the file verbatim, comments included, because they explain each flag where +the person editing it will be looking. The command is the same one you can run +locally, so a local `nerve config validate --workspace . --portable-only +--strict-keys` gives the verdict CI will. Note that the *config* repo is private; +only nerve itself is public. + +`init-repo` never overwrites an existing workflow, so a repo scaffolded earlier +keeps whatever it has; compare it against the file above after a nerve upgrade. + +Gate plugins need nothing installed here: validation never loads them (see +[Validating Configuration](#validating-configuration)), so their imports are never +resolved in CI. + +**4. Point the instance at the remote and enable sync.** The workspace is already +a git clone of the repo, so the remote and credentials come from git itself. Turn +on periodic pulls in `workspace/config/settings.yaml`: + +```yaml +workspace_sync: + enabled: true + branch: main + interval_minutes: 5 +``` + +**5. Optionally lock it down.** Once secrets are in the environment as `${ENV_VAR}` +refs, set `lockdown: true` in `settings.yaml` so the instance only ever runs the +reviewed, merged remote. See [Lockdown](#lockdown-remote-only-read-only). + +From here the loop is: open a PR, CI validates, review and merge, the instance +syncs and reloads. The agent proposes its own changes the same way, through the +`nerve-workspace` skill. + ## Git-Backed Workspace Sync The workspace can be a git repository whose remote is a shared **config repo**. diff --git a/nerve/cli.py b/nerve/cli.py index 0a37d959..b3b3e94d 100644 --- a/nerve/cli.py +++ b/nerve/cli.py @@ -1469,7 +1469,7 @@ def config_validate( portable_only: bool, assume_locked: bool, ) -> None: """Validate the configuration bundle. Non-zero exit on any error (CI-ready).""" - from nerve.config_validate import validate_config_bundle + from nerve.config_validate import render_result, validate_config_bundle config_dir = Path(ctx.obj["config_dir"]) result = validate_config_bundle( @@ -1477,19 +1477,92 @@ def config_validate( strict_env=strict_env, strict_keys=strict_keys, portable_only=portable_only, assume_locked=assume_locked, ) - for msg in result.info: - click.echo(f"[info] {msg}") - for msg in result.warnings: - click.secho(f"[WARN] {msg}", fg="yellow") - for msg in result.errors: - click.secho(f"[ERR] {msg}", fg="red") - if result.ok: - click.secho("Config OK", fg="green") - else: - click.secho(f"Config invalid: {len(result.errors)} error(s)", fg="red") + # Same renderer as `python -m nerve.config_validate` (the no-install path), + # just colorized here. + lines, summary = render_result(result) + _SEVERITY_COLORS = {"[WARN]": "yellow", "[ERR]": "red"} + for line in lines: + click.secho(line, fg=_SEVERITY_COLORS.get(line.split(" ", 1)[0])) + click.secho(summary, fg="green" if result.ok else "red") + if not result.ok: ctx.exit(1) +@config_group.command("init-repo") +@click.option( + "--workspace", "workspace", type=click.Path(), default=None, + help="Workspace to scaffold (default: the resolved workspace).", +) +@click.option( + "--dry-run", is_flag=True, help="Show what would be created without writing.", +) +@click.pass_context +def config_init_repo( + ctx: click.Context, workspace: str | None, dry_run: bool, +) -> None: + """Scaffold the workspace into a shareable git config repo. + + Drops a CI validation workflow, a secrets-aware .gitignore, and a README into + the workspace (never overwriting existing files), then prints the remaining + git/gh and instance-config steps. Safe to re-run. + """ + from nerve.config_repo import scaffold_config_repo + + if workspace: + ws = Path(workspace).expanduser() + else: + config = ctx.obj["config"] + if config is None: + raise click.ClickException( + "Config could not be loaded; pass --workspace or run 'nerve doctor'." + ) + ws = Path(config.workspace) + + if not ws.is_dir(): + raise click.ClickException(f"Workspace does not exist: {ws}") + + result = scaffold_config_repo(ws, dry_run=dry_run) + + verb = "Would create" if dry_run else "Created" + for rel in result.created: + click.secho(f" {verb}: {rel}", fg="green") + for rel in result.skipped: + click.secho(f" Skipped (exists): {rel}", fg="yellow") + if not result.created: + click.echo("Config-repo scaffold already in place — nothing to create.") + + # Remaining steps the CLI can't do for you (they need a repo URL + auth). + click.secho("\nNext steps:", bold=True) + click.echo(f" cd {ws}") + if not result.is_git_repo: + click.echo(" git init && git add -A && git commit -m 'Initial Nerve config'") + else: + click.echo(" git add -A && git commit -m 'Add config-repo scaffold'") + click.echo( + " gh repo create /nerve-config --private --source=. " + "--remote=origin --push" + ) + click.echo( + "\nCI validates every PR out of the box (no secrets needed) — see\n" + ".github/workflows/validate-config.yml. It installs nerve from the\n" + "default branch, which stays at or ahead of this instance; append a ref\n" + "to that install line to check against one release instead." + ) + click.echo( + "\nOn the instance, enable sync in workspace/config/settings.yaml:\n" + " workspace_sync:\n" + " enabled: true\n" + " branch: main\n" + " interval_minutes: 5\n" + "Then verify — the same check CI runs, so a pass here means a green PR:\n" + f" nerve config validate --workspace {ws} --portable-only --strict-keys\n" + "Drop --portable-only to include this machine's config.yaml and\n" + "config.local.yaml, i.e. what the daemon itself loads.\n" + "When ready to lock down, set 'lockdown: true' in settings.yaml (move\n" + "secrets to ${ENV_VAR} first — see docs/config.md)." + ) + + @main.group() def codex() -> None: """Inspect and authenticate the Codex integration.""" diff --git a/nerve/config_pr.py b/nerve/config_pr.py index a53629ea..edc43028 100644 --- a/nerve/config_pr.py +++ b/nerve/config_pr.py @@ -727,6 +727,13 @@ def propose_config_change( # one, and the boxes that merge it refuse to load. The masking also runs # the other way — an explicit machine-local cron.jobs_file wins, and a # proposed config/cron/jobs.yaml is then never opened at all. + # + # Still not --strict-keys, which the config repo's own CI does add, so a + # typo'd key it would reject only warns here and the PR still opens. That + # asymmetry is on purpose — the reviewer sees the red check and the + # explanation with the change in front of them, which is a better place to + # settle "is this key real" than a refusal here with no diff to look at. + # Erring the other way would block proposals on keys a newer nerve knows. from nerve.config_validate import validate_config_bundle errors = [ e for e in validate_config_bundle( diff --git a/nerve/config_repo.py b/nerve/config_repo.py new file mode 100644 index 00000000..24d5631f --- /dev/null +++ b/nerve/config_repo.py @@ -0,0 +1,81 @@ +"""Scaffold a workspace into a shareable git config repo. + +``nerve config init-repo`` drops the files a config repo needs — the CI +validation workflow, a secrets-aware ``.gitignore``, a README, and a portable +settings layer — into the workspace, after which the CLI prints the remaining +manual git/``gh`` + instance steps. Scaffolding is **idempotent**: an existing +file is never overwritten (it's reported as skipped), so re-running is safe and +won't clobber local edits. + +The workspace root *is* the config repo root (it holds ``config/`` and +``skills/``), so these files land at the top level of the workspace. +""" + +from __future__ import annotations + +import importlib.resources +from dataclasses import dataclass, field +from pathlib import Path + +# Destination path (relative to the workspace root) -> source path under +# nerve/templates/. Order is display order. +_SCAFFOLD: dict[str, str] = { + ".github/workflows/validate-config.yml": "config-repo/validate-config.yml", + ".gitignore": "config-repo/gitignore", + "README.md": "config-repo/README.md", + # The same commented scaffold `nerve init` writes. A repo with no config/ at + # all fails the workflow above on its very first commit: that job validates + # the portable layer only, and finding no file to open is an error there, not + # a pass. An instance's workspace already has this file, and it is skipped. + "config/settings.yaml": "config/settings.yaml", +} + + +@dataclass +class ScaffoldResult: + """What ``scaffold_config_repo`` did (or would do, under ``dry_run``).""" + + created: list[str] = field(default_factory=list) + skipped: list[str] = field(default_factory=list) + is_git_repo: bool = False + + +def _template_dir() -> Path: + """Resolve nerve/templates/ in both source and installed layouts.""" + try: + ref = importlib.resources.files("nerve") / "templates" + p = Path(str(ref)) + if p.is_dir(): + return p + except (TypeError, FileNotFoundError): + pass + p = Path(__file__).parent / "templates" + if p.is_dir(): + return p + raise FileNotFoundError("nerve templates not found") + + +def scaffold_config_repo(workspace: Path, dry_run: bool = False) -> ScaffoldResult: + """Write the config-repo scaffold files into ``workspace``. + + Never overwrites an existing file (reported in ``skipped``). With + ``dry_run=True`` nothing is written but the created/skipped split is still + computed. Returns a :class:`ScaffoldResult`. + """ + workspace = Path(workspace) + tmpl = _template_dir() + result = ScaffoldResult(is_git_repo=(workspace / ".git").exists()) + + for rel, src_name in _SCAFFOLD.items(): + dst = workspace / rel + if dst.exists(): + result.skipped.append(rel) + continue + result.created.append(rel) + body = (tmpl / src_name).read_text(encoding="utf-8") + if dry_run: + continue + dst.parent.mkdir(parents=True, exist_ok=True) + dst.write_text(body, encoding="utf-8") + + return result diff --git a/nerve/config_validate.py b/nerve/config_validate.py index 1eee8d1b..1088cfc4 100644 --- a/nerve/config_validate.py +++ b/nerve/config_validate.py @@ -74,9 +74,10 @@ def validate_config_bundle( ``portable_only`` drops the machine-local layers and judges the portable workspace config alone — the right question for a change headed to a shared repo, and the only way to get an answer that doesn't depend on the host - running the check. Pair it with ``workspace_override``: with no machine - config left to read the workspace location from, it falls back to the - default one. + running the check. It covers cron too: the loader's fallback to a legacy + ``~/.nerve/cron`` is suppressed, so only the bundle's own cron files count. + Pair it with ``workspace_override``: with no machine config left to read the + workspace location from, it falls back to the default one. ``assume_locked`` judges the locked view whatever the bundle's own ``lockdown`` flag resolves to here. A fleet repo states the flag as @@ -299,6 +300,10 @@ def validate_config_bundle( ("jobs", base_cron / "jobs.yaml"), ) + portable_root = cfg.workspace_config_dir(workspace) + if portable_only: + cron_files = _tracked_cron_only(cron_files, portable_root, result) + _validate_cron(cron_files, result, strict_keys=strict_keys) if config is not None: # Source runners are scheduled from the same parser as cron jobs, so a @@ -306,7 +311,6 @@ def validate_config_bundle( _validate_source_schedules(config, result) # Files of the bundle that were actually opened, so "it validated" can be # told from "it found nothing to validate". - portable_root = cfg.workspace_config_dir(workspace) read = [ p for p in (cfg.workspace_settings_file(workspace), *(p for _, p in cron_files)) if p.is_file() and p.is_relative_to(portable_root) @@ -321,6 +325,34 @@ def validate_config_bundle( return result +def _tracked_cron_only(cron_files, portable_root: Path, result: ValidationResult): + """Keep cron on the tracked bundle's own files, for ``portable_only``. + + Cron resolution is file-aware: a workspace carrying no jobs falls back to the + machine-local ``~/.nerve/cron`` so an un-migrated install keeps working. That + fallback is right at load time and wrong here — it hands a run that was asked + to judge the shared bundle alone a file the bundle does not contain, and a + typo in it fails a config repo that has nothing wrong with it. Same substitution + the machine-local layers are skipped to prevent, one directory over. + + An explicit ``cron.jobs_file`` pointing outside the tracked subtree goes the + same way: whatever it is, it isn't what the repo carries. + """ + kept = [] + for label, path in cron_files: + if path.is_relative_to(portable_root): + kept.append((label, path)) + continue + tracked = portable_root / "cron" / f"{label}.yaml" + kept.append((label, tracked)) + if path.exists(): + result.info.append( + f"cron {label} was not read from {path}: it is outside the tracked " + f"config and this run judges the portable bundle alone" + ) + return tuple(kept) + + def _read_layer(path: Path, result: ValidationResult) -> dict[str, Any] | None: """Read one machine-local layer; ``None`` if it could not be used. @@ -808,3 +840,105 @@ def _gate_spec_problem(spec) -> str | None: if not gate_type.strip(): return "gate spec 'type' is blank" return None + + +# -- Standalone entry point ------------------------------------------------ +# +# ``python -m nerve.config_validate`` runs validation straight from a source +# checkout. Everything above needs only the standard library plus PyYAML, so a +# bundle can be checked by cloning nerve and setting PYTHONPATH — no install, no +# dependency resolution of nerve's full runtime (boto3, telethon, ...). The +# generated CI workflow installs nerve and runs ``nerve config validate``; this +# path is for environments that cannot install it. +# +# Both are the same code, and they have to stay in step in both directions: +# identical output, and the same set of flags — a flag the CLI has and this parser +# doesn't is a documented invocation this path cannot run. +# tests/test_config_validate.py asserts both. + + +def render_result(result: ValidationResult) -> tuple[list[str], str]: + """Render ``result`` as (message lines, summary line). + + Shared by this module's ``main`` and the ``nerve config validate`` CLI so + the installed and zero-install paths report identically. + """ + lines = [f"[info] {m}" for m in result.info] + lines += [f"[WARN] {m}" for m in result.warnings] + lines += [f"[ERR] {m}" for m in result.errors] + summary = ( + "Config OK" if result.ok else f"Config invalid: {len(result.errors)} error(s)" + ) + return lines, summary + + +def _build_parser(): + """The standalone entry point's argument parser. + + Separate from ``main`` so a test can compare its flags with the Click + command's without going through ``--help``. + """ + import argparse + + parser = argparse.ArgumentParser( + prog="python -m nerve.config_validate", + description="Validate a Nerve config bundle. Non-zero exit on any error.", + ) + parser.add_argument( + "--workspace", + default=None, + help="Workspace to validate (a checked-out config repo: --workspace .). " + "Defaults to the resolved workspace.", + ) + parser.add_argument( + "--config-dir", + default=None, + help="Machine-local config dir supplying config.yaml/config.local.yaml. " + "Defaults to the resolved config dir; usually absent in CI.", + ) + parser.add_argument( + "--strict-env", action="store_true", + help="Fail if any ${ENV_VAR} reference is unset (default: report as info).", + ) + parser.add_argument( + "--strict-keys", action="store_true", + help="Fail on unknown/misspelled config keys (default: warn).", + ) + parser.add_argument( + "--portable-only", action="store_true", + help="Ignore this machine's config.yaml/config.local.yaml and judge only " + "the portable workspace config — what a shared repo carries. Fails " + "if no file under the workspace's config/ was opened.", + ) + parser.add_argument( + "--assume-lockdown", dest="assume_locked", action="store_true", + help="Validate the locked view whatever this bundle's lockdown flag " + "resolves to here. A fleet repo writes `lockdown: " + "${NERVE_LOCKDOWN:-false}`, so CI resolves it to false and checks a " + "config no locked box will run.", + ) + return parser + + +def main(argv: list[str] | None = None) -> int: + """Validate a config bundle and print the result. Returns the exit code.""" + args = _build_parser().parse_args(argv) + + config_dir, _ = cfg.resolve_config_dir(args.config_dir) + result = validate_config_bundle( + config_dir, + workspace_override=args.workspace, + strict_env=args.strict_env, + strict_keys=args.strict_keys, + portable_only=args.portable_only, + assume_locked=args.assume_locked, + ) + lines, summary = render_result(result) + for line in lines: + print(line) + print(summary) + return 0 if result.ok else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/nerve/sync_service.py b/nerve/sync_service.py index 6d0d6728..ea904e6e 100644 --- a/nerve/sync_service.py +++ b/nerve/sync_service.py @@ -376,6 +376,12 @@ def _validate_rev( the bundle", and this process is the daemon, with the daemon's environment. An unset required reference means ``load_config`` will raise on the next restart, so it has to block the merge. + + Key strictness goes the other way, and also on purpose: an unknown key is + left a warning here, where the config repo's CI makes it an error. A key this + nerve doesn't recognize is inert, and refusing an already-reviewed, merged + change over one would strand the instance on a stale config for a spelling + the next upgrade may well know. """ from nerve.config_validate import ValidationResult, validate_config_bundle diff --git a/nerve/templates/config-repo/README.md b/nerve/templates/config-repo/README.md new file mode 100644 index 00000000..8e2255a5 --- /dev/null +++ b/nerve/templates/config-repo/README.md @@ -0,0 +1,34 @@ +# Nerve config repo + +This repository is the **shared configuration** for a +[Nerve](https://github.com/ClickHouse/nerve) instance — its skills, cron jobs, +sources, and settings. It maps to the instance's **workspace**, so the repo root +holds: + +- `config/settings.yaml` — shareable settings (secrets are `${ENV_VAR}` refs) +- `config/cron/` — cron jobs, system jobs, and drop-in gate plugins +- `skills//SKILL.md` — skills + +## How changes are made + +1. Open a PR with your change. CI (`.github/workflows/validate-config.yml`) + validates the bundle and scans for committed secrets; both must pass. +2. A human reviews and merges. +3. The instance pulls the merged result (workspace sync) and hot-reloads — no + restart. + +The agent proposes its own changes the same way via the `nerve-workspace` skill +(`propose_config_change`). When the instance is **locked** (`lockdown: true`), +this PR flow is the *only* way to change config. + +## Rules + +- **Never commit secrets.** They live in the environment and are referenced with + `${ENV_VAR}` from `settings.yaml`. See `.gitignore`. CI runs a secret scanner + over every PR, but treat it as a backstop: it recognizes common credential + shapes, not every string that happens to be one. It also errs the other way — + a long random-looking value in a prompt or an MCP argument can trip it. Add a + trailing `# gitleaks:allow` on that line, or a `.gitleaks.toml` for a + repo-wide rule, and say why in the PR. +- Gate plugins under `config/cron/gates/*.py` are **executable code** run by the + daemon — review them like any other code. diff --git a/nerve/templates/config-repo/gitignore b/nerve/templates/config-repo/gitignore new file mode 100644 index 00000000..1845f0f5 --- /dev/null +++ b/nerve/templates/config-repo/gitignore @@ -0,0 +1,31 @@ +# Nerve config repo — keep secrets and machine-local state OUT of the shared repo. +# Secrets are referenced from settings.yaml via ${ENV_VAR} and supplied by the +# environment; nothing sensitive should ever be committed here. + +# Machine-local overrides / secrets +config.local.yaml +.env +.env.* + +# Migration breadcrumbs (originals left behind by `nerve migrate`) +*.migrated + +# Runtime state and credentials (only appear if the workspace doubles as a +# runtime dir). These are nerve's secret-bearing state files — a Telethon +# session grants full account access, so never let `git add -A` catch one. +*.db +*.db-journal +*.log +*.pid +.nerve/ +*.session +mcp-token +certs/ + +# The nerve source checkout used by the CI validation workflow +.nerve-src/ + +# OS / editor +.DS_Store +Thumbs.db +*.swp diff --git a/nerve/templates/config-repo/validate-config.yml b/nerve/templates/config-repo/validate-config.yml new file mode 100644 index 00000000..11334751 --- /dev/null +++ b/nerve/templates/config-repo/validate-config.yml @@ -0,0 +1,59 @@ +name: validate-config + +# Validates the Nerve config bundle on every PR so a broken change can't be +# merged and then synced onto a live instance. See docs/config.md. + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +jobs: + validate: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + # Backstop for the one thing review is worst at spotting: a credential + # pasted into a tracked file. Secrets belong in the environment and are + # referenced from settings.yaml as ${ENV_VAR}. Runs before anything else + # is fetched, so it scans this repo and nothing else, and needs no token + # of its own. The image is used directly because the marketplace action + # asks organizations for a license key. + # + # The tag is pinned because an argument renamed in a later gitleaks would + # fail every config repo at once, with no pull request anywhere to explain + # it. (From 8.19 this subcommand is spelled `gitleaks dir `; bump the + # tag and the args together.) A false positive on a high-entropy string is + # silenced with a trailing `# gitleaks:allow`, or repo-wide in a + # .gitleaks.toml. + - name: Scan for committed secrets + uses: docker://ghcr.io/gitleaks/gitleaks:v8.18.4 + with: + args: detect --no-git --source=/github/workspace --redact + + # nerve is not published to PyPI, so this installs from git; the bare name + # would fetch an unrelated project. Tracking the default branch keeps the + # validator at or ahead of the instance, which is the safe direction for + # --strict-keys below: a newer validator knows every key the instance + # reads. To check against one release instead, append a ref (nerve@v1.2.3). + - uses: astral-sh/setup-uv@v6 + - run: uv pip install --system "nerve @ git+https://github.com/ClickHouse/nerve" + + # The config repo root IS the workspace (it holds config/ and skills/). + # + # --portable-only judges the tracked bundle on its own: no machine-local + # config.yaml, and no falling back to a machine-local cron directory when + # this repo carries no jobs. It fails outright if it opened no file under + # config/ at all, so a layout mistake cannot pass as clean. + # --strict-keys makes a misspelled key block the PR rather than warn. + # + # Unset ${ENV_VAR} secret refs are reported as info (CI has no secrets); + # add --strict-env to require them. If any instance served by this repo is + # locked through ${NERVE_LOCKDOWN}, add --assume-lockdown as well, or CI + # only ever checks the unlocked view that instance never loads. + - name: Validate config bundle + run: nerve config validate --workspace . --portable-only --strict-keys diff --git a/nerve/templates/skills/nerve-workspace/SKILL.md b/nerve/templates/skills/nerve-workspace/SKILL.md index 24a9217e..fd6ff011 100644 --- a/nerve/templates/skills/nerve-workspace/SKILL.md +++ b/nerve/templates/skills/nerve-workspace/SKILL.md @@ -143,3 +143,20 @@ propose_config_change( same content straight to the file instead only moves an invalid config onto the instance — that is the one case where editing directly is the wrong answer even on a local workspace. + +## Setting up a config repo (when the user wants review) + +On a local workspace you edit directly, as above — that is not a stopgap. But if +the user wants config changes reviewed before they take effect, or wants one repo +to serve several instances, the workspace has to become a git repo synced from a +remote. That is an **operator** task, not something you do autonomously — walk the +human through it: + +1. Run `nerve config init-repo` — scaffolds the CI workflow, `.gitignore`, a + README and a `config/settings.yaml` into the workspace, and prints the + remaining git/`gh` steps. +2. Create the private GitHub repo and push (`git init` → commit → `gh repo create`). +3. Enable `workspace_sync` (and, when ready, `lockdown`) in `settings.yaml`. + +Full runbook: `docs/config.md` → "Setting up the config repo". Once the remote +exists and sync is on, use `propose_config_change` for all further changes. diff --git a/tests/test_config_repo.py b/tests/test_config_repo.py new file mode 100644 index 00000000..821cc43b --- /dev/null +++ b/tests/test_config_repo.py @@ -0,0 +1,442 @@ +"""Tests for config-repo scaffolding (nerve/config_repo.py + `config init-repo`).""" + +from __future__ import annotations + +import os +import shlex +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml +from click.testing import CliRunner + +import nerve +from nerve import config_repo +from nerve.config_repo import _SCAFFOLD, _template_dir, scaffold_config_repo + +_WORKFLOW = ".github/workflows/validate-config.yml" + + +def _workflow(ws: Path) -> dict: + return yaml.safe_load((ws / _WORKFLOW).read_text(encoding="utf-8")) + + +def _steps(ws: Path) -> list[dict]: + return _workflow(ws)["jobs"]["validate"]["steps"] + + +def _validate_step(ws: Path) -> dict: + """The step that actually runs the validator.""" + return next( + s for s in _steps(ws) if "config validate" in str(s.get("run", "")) + ) + + +def _install_step(ws: Path) -> dict: + """The step that installs the nerve the validator comes from.""" + return next(s for s in _steps(ws) if "pip install" in str(s.get("run", ""))) + + +def _run_ci_validation(ws: Path) -> subprocess.CompletedProcess: + """Run the scaffolded workflow's validate step against ``ws``, as CI would. + + The command, its environment and its working directory are taken from the + generated file rather than restated here, so this exercises the flags an + operator's CI will actually run. The only substitution is where nerve comes + from: CI installs it, here it is the tree the tests are running from. + """ + step = _validate_step(ws) + argv = shlex.split(step["run"]) + assert argv[0] == "nerve", step["run"] + argv[:1] = [sys.executable, "-m", "nerve.cli"] + + env = dict(os.environ) + env.update(step.get("env") or {}) + # The console script CI runs puts its own bin directory on sys.path, not the + # working directory. `-m` would put the config repo there instead, so pin the + # nerve under test explicitly and keep the repo off the path, or the + # not-importable test below would pass for the wrong reason. + env["PYTHONPATH"] = str(Path(nerve.__file__).resolve().parent.parent) + env["PYTHONSAFEPATH"] = "1" + return subprocess.run(argv, cwd=ws, env=env, capture_output=True, text=True) + + +class TestScaffold: + def test_creates_all_files(self, tmp_path): + result = scaffold_config_repo(tmp_path) + assert set(result.created) == set(_SCAFFOLD) + assert result.skipped == [] + for rel in _SCAFFOLD: + assert (tmp_path / rel).is_file() + + def test_ci_workflow_lands_in_github_dir(self, tmp_path): + scaffold_config_repo(tmp_path) + wf = tmp_path / ".github/workflows/validate-config.yml" + assert wf.is_file() + assert "--workspace ." in wf.read_text(encoding="utf-8") + + def test_ci_workflow_is_valid_yaml(self, tmp_path): + # A malformed template would ship and only blow up in the operator's CI. + scaffold_config_repo(tmp_path) + wf = yaml.safe_load( + (tmp_path / ".github/workflows/validate-config.yml").read_text("utf-8") + ) + steps = wf["jobs"]["validate"]["steps"] + assert any("config validate" in str(s.get("run", "")) for s in steps) + + def test_ci_workflow_needs_no_secrets(self, tmp_path): + # nerve is public and the validator needs no credentials, so a config repo + # can run this without anyone provisioning an Actions secret first. + scaffold_config_repo(tmp_path) + assert "secrets." not in ( + tmp_path / ".github/workflows/validate-config.yml" + ).read_text("utf-8") + + def test_gitignore_excludes_secrets(self, tmp_path): + scaffold_config_repo(tmp_path) + body = (tmp_path / ".gitignore").read_text(encoding="utf-8") + assert "config.local.yaml" in body + assert "*.migrated" in body + + def test_gitignore_covers_backup_secret_members(self, tmp_path): + # If the workspace doubles as the state dir, `git add -A` (which the + # runbook tells operators to run) must not sweep up nerve's credentials. + from nerve.backup import SECRET_MEMBERS + + scaffold_config_repo(tmp_path) + patterns = { + ln.strip() + for ln in (tmp_path / ".gitignore").read_text("utf-8").splitlines() + if ln.strip() and not ln.startswith("#") + } + for member in SECRET_MEMBERS: + name = Path(member).name + assert any( + p in (name, f"{name}/", f"*{Path(name).suffix}") + for p in patterns + ), f"{name} (from backup.SECRET_MEMBERS) is not gitignored" + + def test_dry_run_writes_nothing(self, tmp_path): + result = scaffold_config_repo(tmp_path, dry_run=True) + assert set(result.created) == set(_SCAFFOLD) + for rel in _SCAFFOLD: + assert not (tmp_path / rel).exists() + # Not even the parent dirs of nested targets. + assert not (tmp_path / ".github").exists() + assert list(tmp_path.iterdir()) == [] + + def test_templates_are_packaged(self): + # Guards the importlib-vs-source fallback in _template_dir(): every + # scaffold source must actually resolve, or an installed nerve breaks. + tmpl = _template_dir() + for src_name in _SCAFFOLD.values(): + assert (tmpl / src_name).is_file(), f"missing template {src_name}" + + def test_idempotent_skips_existing(self, tmp_path): + scaffold_config_repo(tmp_path) + second = scaffold_config_repo(tmp_path) + assert second.created == [] + assert set(second.skipped) == set(_SCAFFOLD) + + def test_never_overwrites_existing_file(self, tmp_path): + gi = tmp_path / ".gitignore" + gi.write_text("# my custom ignore\n", encoding="utf-8") + result = scaffold_config_repo(tmp_path) + assert ".gitignore" in result.skipped + assert gi.read_text(encoding="utf-8") == "# my custom ignore\n" + # The other two are still created. + assert (tmp_path / "README.md").is_file() + + def test_detects_existing_git_repo(self, tmp_path): + assert scaffold_config_repo(tmp_path).is_git_repo is False + (tmp_path / ".git").mkdir() + assert scaffold_config_repo(tmp_path).is_git_repo is True + + def test_seeds_a_portable_settings_layer(self, tmp_path): + # The workflow validates the portable layer only, and a config/ it can + # read nothing out of is an error there — so a repo scaffolded from a + # bare directory needs one to be valid on its first commit. + scaffold_config_repo(tmp_path) + assert (tmp_path / "config" / "settings.yaml").is_file() + + def test_leaves_an_existing_settings_file_alone(self, tmp_path): + settings = tmp_path / "config" / "settings.yaml" + settings.parent.mkdir(parents=True) + settings.write_text("timezone: UTC\n", encoding="utf-8") + + result = scaffold_config_repo(tmp_path) + + assert "config/settings.yaml" in result.skipped + assert settings.read_text(encoding="utf-8") == "timezone: UTC\n" + + +class TestScaffoldedWorkflow: + """What the generated workflow does, read off the generated file.""" + + def test_the_validator_is_installed_from_the_nerve_repo(self, tmp_path): + """Not from PyPI, where the bare name is an unrelated project.""" + scaffold_config_repo(tmp_path) + run = _install_step(tmp_path)["run"] + assert "git+https://github.com/ClickHouse/nerve" in run + assert "install nerve\n" not in run and not run.endswith("install nerve") + + def test_the_validate_step_runs_the_installed_cli(self, tmp_path): + scaffold_config_repo(tmp_path) + assert _validate_step(tmp_path)["run"].startswith("nerve config validate") + + def test_validates_the_portable_layer_strictly(self, tmp_path): + scaffold_config_repo(tmp_path) + run = _validate_step(tmp_path)["run"] + assert "--portable-only" in run # not the runner's machine config + assert "--strict-keys" in run # a typo'd key blocks the PR + + def test_does_not_assume_lockdown(self, tmp_path): + # Correct default, and load-bearing: the locked view requires secrets a + # fresh repo hasn't got, so passing it here would red-X every new repo. + # The workflow comment points at it for repos that do serve a locked box. + scaffold_config_repo(tmp_path) + assert "--assume-lockdown" not in _validate_step(tmp_path)["run"] + assert "--assume-lockdown" in (tmp_path / _WORKFLOW).read_text("utf-8") + + def test_scans_for_committed_secrets(self, tmp_path): + scaffold_config_repo(tmp_path) + assert any("gitleaks" in str(s.get("uses", "")) for s in _steps(tmp_path)) + + def test_no_step_runs_code_from_the_config_repo(self, tmp_path): + """The job reads the PR's files; it must never execute them. + + Validation itself declines to load the bundle's gate plugins, which is + worth nothing if the job around it installs the repo, runs a script out + of it, or resolves a local action from it. + """ + scaffold_config_repo(tmp_path) + for step in _steps(tmp_path): + uses = str(step.get("uses", "")) + assert not uses.startswith("./"), f"local action from the PR: {uses}" + # A container step can point its entrypoint at the mounted checkout, + # which `uses:` alone doesn't reveal. + entrypoint = str((step.get("with") or {}).get("entrypoint", "")) + assert "/github/workspace" not in entrypoint, entrypoint + run = str(step.get("run", "")) + for forbidden in ( + "-e .", "-r ", "install .", "bash ", "sh ", "make ", + "pre-commit", "setup.py", "uv run", "npm ", "npx ", "tox", + ): + assert forbidden not in run, f"{forbidden!r} in step: {run}" + + def test_docs_show_the_generated_workflow_verbatim(self, tmp_path): + """docs/config.md prints this file inline, comments and all. + + Compared as text, not as parsed YAML: every load-bearing instruction in + that file — which flags matter, where nerve comes from, what silences a + false positive — lives in a comment, so a structural comparison would + wave through exactly the drift worth catching. + """ + scaffold_config_repo(tmp_path) + generated = (tmp_path / _WORKFLOW).read_text(encoding="utf-8").rstrip("\n") + + docs = Path(__file__).resolve().parent.parent / "docs" / "config.md" + text = docs.read_text(encoding="utf-8") + start = text.index("```yaml\nname: validate-config\n") + len("```yaml\n") + documented = text[start:text.index("\n```", start)] + + assert documented == generated + + +class TestFreshRepoPassesItsOwnCI: + """`init-repo` scaffolds the repo *and* the job that gates it, so the two + have to agree on day one — a red X on the first commit teaches operators to + ignore the check.""" + + def test_a_fresh_scaffold_validates_clean(self, tmp_path): + ws = tmp_path / "repo" + ws.mkdir() + scaffold_config_repo(ws) + + proc = _run_ci_validation(ws) + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "Config OK" in proc.stdout + + def test_a_broken_bundle_fails(self, tmp_path): + ws = tmp_path / "repo" + ws.mkdir() + scaffold_config_repo(ws) + (ws / "config" / "settings.yaml").write_text( + "agent:\n backend: bogus\n", encoding="utf-8", + ) + + proc = _run_ci_validation(ws) + + assert proc.returncode == 1 + assert "bogus" in proc.stdout + + def test_a_misspelled_key_blocks_the_pr(self, tmp_path): + ws = tmp_path / "repo" + ws.mkdir() + scaffold_config_repo(ws) + (ws / "config" / "settings.yaml").write_text( + "tiimezone: UTC\n", encoding="utf-8", + ) + + proc = _run_ci_validation(ws) + + assert proc.returncode == 1, proc.stdout + assert "tiimezone" in proc.stdout + + def test_machine_config_beside_the_checkout_is_ignored(self, tmp_path): + """A config.yaml in the working directory is picked up by the config-dir + waterfall; merging it would let the runner's state decide the verdict.""" + ws = tmp_path / "repo" + ws.mkdir() + scaffold_config_repo(ws) + (ws / "config" / "settings.yaml").write_text( + "agent:\n backend: bogus\n", encoding="utf-8", + ) + (ws / "config.yaml").write_text("agent:\n backend: claude\n", encoding="utf-8") + + proc = _run_ci_validation(ws) + + assert proc.returncode == 1, proc.stdout + assert "bogus" in proc.stdout + + def test_a_machine_local_cron_dir_cannot_fail_the_repo( + self, tmp_path, monkeypatch, + ): + """The repo carries no cron jobs, so cron resolution would otherwise fall + back to the machine-local directory — condemning a clean config repo over + a file that isn't in it. Bites hardest where this command is most likely + to be run by hand: the instance box, which has one.""" + ws = tmp_path / "repo" + ws.mkdir() + scaffold_config_repo(ws) + home = tmp_path / "nervehome" + (home / "cron").mkdir(parents=True) + (home / "cron" / "jobs.yaml").write_text("jobs: [oops\n", encoding="utf-8") + monkeypatch.setenv("NERVE_HOME", str(home)) + + proc = _run_ci_validation(ws) + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "oops" not in proc.stdout + + def test_a_gate_plugin_neither_runs_nor_fails_the_job(self, tmp_path): + """--strict-keys must not turn "can't confirm" into "reject". + + A gate type only a plugin provides is unverifiable without importing the + plugin, which the job declines to do — so it stays a warning, and the + plugin's code never executes in CI. + """ + ws = tmp_path / "repo" + ws.mkdir() + scaffold_config_repo(ws) + marker = tmp_path / "executed" + gates = ws / "config" / "cron" / "gates" + gates.mkdir(parents=True) + (gates / "marker.py").write_text( + f"import pathlib; pathlib.Path({str(marker)!r}).write_text('ran')\n", + encoding="utf-8", + ) + (ws / "config" / "cron" / "jobs.yaml").write_text( + "jobs:\n - id: g\n schedule: 1h\n prompt: hi\n" + " run_if:\n - type: marker_test\n", + encoding="utf-8", + ) + + proc = _run_ci_validation(ws) + + assert not marker.exists(), "CI executed a gate plugin from the PR" + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "marker_test" in proc.stdout + + def test_the_checkout_is_not_importable(self, tmp_path): + """`python -m` puts the working directory first on sys.path, so a + `yaml.py` committed to the config repo would be imported in place of the + real one — running a pull request's code before anyone read it.""" + ws = tmp_path / "repo" + ws.mkdir() + scaffold_config_repo(ws) + marker = tmp_path / "imported" + (ws / "yaml.py").write_text( + f"import pathlib; pathlib.Path({str(marker)!r}).write_text('ran')\n", + encoding="utf-8", + ) + + proc = _run_ci_validation(ws) + + assert not marker.exists(), "the config repo's yaml.py was imported" + assert proc.returncode == 0, proc.stdout + proc.stderr + + +class TestInitRepoCommand: + def _run(self, args): + from nerve.cli import main + + return CliRunner().invoke(main, args, obj={"config": None, "config_dir": "."}) + + def test_scaffolds_via_workspace_flag(self, tmp_path): + result = self._run(["config", "init-repo", "--workspace", str(tmp_path)]) + assert result.exit_code == 0, result.output + assert (tmp_path / ".github/workflows/validate-config.yml").is_file() + assert "Created:" in result.output + assert "Next steps:" in result.output + + def test_prints_git_init_when_not_a_repo(self, tmp_path): + out = self._run(["config", "init-repo", "--workspace", str(tmp_path)]).output + assert "git init" in out + + def test_omits_git_init_when_already_a_repo(self, tmp_path): + (tmp_path / ".git").mkdir() + out = self._run(["config", "init-repo", "--workspace", str(tmp_path)]).output + assert "git init" not in out + + def test_dry_run_writes_nothing(self, tmp_path): + out = self._run( + ["config", "init-repo", "--workspace", str(tmp_path), "--dry-run"] + ).output + assert "Would create:" in out + assert not (tmp_path / ".gitignore").exists() + + def test_leaves_an_existing_workflow_alone(self, tmp_path): + wf = tmp_path / _WORKFLOW + wf.parent.mkdir(parents=True) + wf.write_text("name: mine\n", encoding="utf-8") + + out = self._run(["config", "init-repo", "--workspace", str(tmp_path)]).output + + assert "Skipped (exists)" in out + assert wf.read_text(encoding="utf-8") == "name: mine\n" + + def test_the_verify_command_matches_what_ci_runs(self, tmp_path): + """Before this, the runbook handed the operator a laxer command than the + gate it feeds: a typo'd key passed locally and red-X'd on the PR.""" + out = self._run(["config", "init-repo", "--workspace", str(tmp_path)]).output + printed = next( + ln for ln in out.splitlines() if "nerve config validate" in ln + ) + for flag in shlex.split(_validate_step(tmp_path)["run"]): + if flag.startswith("--") and flag != "--workspace": + assert flag in printed, f"{flag} missing from: {printed}" + + def test_missing_workspace_errors(self, tmp_path): + result = self._run( + ["config", "init-repo", "--workspace", str(tmp_path / "nope")] + ) + assert result.exit_code != 0 + assert "does not exist" in result.output + + def test_no_config_no_workspace_errors(self, monkeypatch): + # When config can't load (self-diagnosing → config=None) and no + # --workspace is given, the command must fail with a clear message + # rather than crash on the missing config. + import nerve.cli as cli + + def _raise(config_dir=None): + raise cli.ConfigError("boom") + + monkeypatch.setattr(cli, "load_config", _raise) + result = CliRunner().invoke(cli.main, ["config", "init-repo"]) + assert result.exit_code != 0 + assert "Config could not be loaded" in result.output diff --git a/tests/test_config_validate.py b/tests/test_config_validate.py index a0374cc5..477b9b13 100644 --- a/tests/test_config_validate.py +++ b/tests/test_config_validate.py @@ -1020,6 +1020,50 @@ def test_portable_only_fails_on_a_config_dir_that_holds_nothing_read(self, tmp_p assert not result.ok, name assert any("nothing to validate" in e for e in result.errors), name + def test_portable_only_ignores_the_machine_local_cron_fallback( + self, tmp_path, monkeypatch, + ): + """A workspace carrying no jobs falls back to ~/.nerve/cron, which is + right for an un-migrated install and wrong for judging a shared bundle: + a broken file there condemns a repo that doesn't contain it.""" + home = tmp_path / "home" + (home / "cron").mkdir(parents=True) + (home / "cron" / "jobs.yaml").write_text("jobs: [oops\n", encoding="utf-8") + monkeypatch.setenv("NERVE_HOME", str(home)) + ws = tmp_path / "ws" + _settings(ws, "timezone: UTC\n") + config_dir = _cfg(tmp_path, workspace=ws) + + overlaid = validate_config_bundle(config_dir, workspace_override=ws) + assert not overlaid.ok # the fallback applies, and the bundle is blamed + + portable = validate_config_bundle( + config_dir, workspace_override=ws, portable_only=True, + ) + assert portable.ok, portable.errors + assert any("outside the tracked config" in i for i in portable.info) + + def test_portable_only_reads_the_repos_own_cron(self, tmp_path, monkeypatch): + """Suppressing the fallback must not suppress the real thing.""" + home = tmp_path / "home" + (home / "cron").mkdir(parents=True) + (home / "cron" / "jobs.yaml").write_text( + "jobs:\n - id: machine\n schedule: 1h\n prompt: hi\n", + encoding="utf-8", + ) + monkeypatch.setenv("NERVE_HOME", str(home)) + ws = tmp_path / "ws" + tracked = ws / "config" / "cron" + tracked.mkdir(parents=True) + (tracked / "jobs.yaml").write_text("jobs: [broken\n", encoding="utf-8") + + result = validate_config_bundle( + _cfg(tmp_path, workspace=ws), workspace_override=ws, portable_only=True, + ) + + assert not result.ok + assert any(str(tracked / "jobs.yaml") in e for e in result.errors) + def test_portable_only_passes_on_a_workspace_with_only_cron(self, tmp_path): """A repo may legitimately carry cron and no settings.yaml — that is a bundle, and it was read.""" @@ -1203,3 +1247,158 @@ def test_cli_doctor_reports_unloadable_config(self, tmp_path, monkeypatch): assert result.exit_code == 1 assert "MISSING_SECRET" in result.output assert "Traceback" not in result.output + + +class TestStandaloneEntryPoint: + """`python -m nerve.config_validate` — validation without installing nerve. + + It runs from a source checkout with only PyYAML installed, so this entry point + must not depend on click or any of nerve's runtime dependencies, and it must + agree with the CLI that CI runs. + """ + + def test_main_ok_exit_zero(self, tmp_path, capsys): + from nerve.config_validate import main + + config_dir = _cfg(tmp_path, "timezone: UTC\n") + assert main(["--config-dir", str(config_dir)]) == 0 + assert "Config OK" in capsys.readouterr().out + + def test_main_bad_exit_one(self, tmp_path, capsys): + from nerve.config_validate import main + + config_dir = _cfg(tmp_path, "agent:\n backend: bogus\n") + assert main(["--config-dir", str(config_dir)]) == 1 + assert "[ERR]" in capsys.readouterr().out + + def test_main_strict_keys_flag(self, tmp_path): + from nerve.config_validate import main + + config_dir = _cfg(tmp_path, "tiimezone: UTC\n") + assert main(["--config-dir", str(config_dir)]) == 0 # warning by default + assert main(["--config-dir", str(config_dir), "--strict-keys"]) == 1 + + def test_main_workspace_override(self, tmp_path, capsys): + """--workspace . is how CI points at the checked-out config repo.""" + from nerve.config_validate import main + + ws = tmp_path / "configrepo" + (ws / "config" / "cron").mkdir(parents=True) + (ws / "config" / "settings.yaml").write_text("timezone: UTC\n", "utf-8") + (ws / "config" / "cron" / "jobs.yaml").write_text( + "jobs:\n - id: j\n schedule: '0 6 * * *'\n prompt: hi\n", "utf-8" + ) + assert main(["--config-dir", str(_cfg(tmp_path)), "--workspace", str(ws)]) == 0 + assert "1 job(s)" in capsys.readouterr().out + + def test_agrees_with_cli(self, tmp_path, capsys): + """Both entry points share render_result, so their text must match.""" + from click.testing import CliRunner + + from nerve.cli import main as cli_main + from nerve.config_validate import main + + config_dir = _cfg(tmp_path, "agent:\n backend: bogus\n") + code = main(["--config-dir", str(config_dir)]) + standalone = capsys.readouterr().out + + cli = CliRunner().invoke( + cli_main, ["-c", str(config_dir), "config", "validate"], + color=False, + ) + assert code != 0 and cli.exit_code != 0 + assert standalone.strip() == cli.output.strip() + + def test_main_portable_only_flag(self, tmp_path, capsys): + """The scaffolded CI workflow passes it, so this parser has to take it — + without it CI silently overlays whatever config the runner has.""" + from nerve.config_validate import main + + ws = tmp_path / "ws" + _settings(ws, "agent:\n backend: bogus\n") + config_dir = _cfg(tmp_path, "agent:\n backend: claude\n", workspace=ws) + args = ["--config-dir", str(config_dir), "--workspace", str(ws)] + + assert main(args) == 0 # the machine layer masks the shared error + capsys.readouterr() + assert main([*args, "--portable-only"]) == 1 + assert "bogus" in capsys.readouterr().out + + def test_main_assume_lockdown_flag(self, tmp_path, capsys): + from nerve.config_validate import main + + ws = tmp_path / "ws" + _settings(ws, "lockdown: ${NERVE_LOCKDOWN:-false}\n") + args = ["--config-dir", str(_cfg(tmp_path, workspace=ws)), + "--workspace", str(ws)] + + # A locked-only error to have something to catch: the tracked subtree is a + # symlink out of the workspace, so nothing under it is part of the repo. + outside = tmp_path / "elsewhere" + outside.mkdir() + (outside / "settings.yaml").write_text( + "lockdown: ${NERVE_LOCKDOWN:-false}\n", encoding="utf-8", + ) + (ws / "config").rename(tmp_path / "discarded") + (ws / "config").symlink_to(outside) + + assert main(args) == 0 # unlocked here, so the lockdown checks don't run + capsys.readouterr() + assert main([*args, "--assume-lockdown"]) == 1 + assert "outside the workspace" in capsys.readouterr().out + + def test_flag_set_matches_the_cli(self): + """Same flags, not just the same output. + + The docs recommend one invocation for CI, and CI runs it through this + parser. A flag the Click command grew and this one didn't is an + invocation the config repo cannot run — which is how the workflow ended + up validating with the runner's machine config merged in. + """ + from nerve.cli import config_validate, main as cli_main + from nerve.config_validate import _build_parser + + def _long_opts(params) -> set[str]: + return { + opt for p in params for opt in getattr(p, "opts", []) + if opt.startswith("--") + } + + standalone = { + opt for action in _build_parser()._actions + for opt in action.option_strings if opt.startswith("--") + } - {"--help"} + + command = _long_opts(config_validate.params) + assert command <= standalone, ( + f"`nerve config validate` accepts {sorted(command - standalone)} and " + f"`python -m nerve.config_validate` does not" + ) + # The reverse, allowing for what the CLI takes on the group instead. + group = _long_opts(cli_main.params) + assert standalone <= command | group, ( + f"only the standalone entry point accepts " + f"{sorted(standalone - (command | group))}" + ) + + def test_imports_without_runtime_deps(self): + """The module chain must stay stdlib+PyYAML so CI needs no install. + + A new top-level import of click/fastapi/boto3/... in this chain would + silently break the zero-install workflow; catch it here instead. + """ + import subprocess + import sys + + heavy = ["click", "fastapi", "apscheduler", "telethon", "boto3", "anthropic"] + code = ( + "import sys\n" + "import nerve.config_validate as m\n" + "m.main\n" + f"leaked=[n for n in {heavy!r} if n in sys.modules]\n" + "print(','.join(leaked))\n" + ) + out = subprocess.run( + [sys.executable, "-c", code], capture_output=True, text=True, check=True, + ) + assert out.stdout.strip() == "", f"heavy imports leaked: {out.stdout.strip()}" From 4ec128eebba1311f064dc8dcfcbc370aac8d71d3 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Fri, 31 Jul 2026 10:12:15 +0200 Subject: [PATCH 2/2] Review: keep the machine-local layer out of the config repo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scaffolded ignore rules covered `config.local.yaml` and the runtime state files, but not `config.yaml`. That was defensible when migration consumed the file, and is not now that it writes one: splitting a legacy monolith leaves the per-machine half — TLS paths, an AWS profile, mail accounts — sitting in `config.yaml` beside the settings it just published, and the runbook's own next step is `git add -A`. Anchored, so it covers the machine-local layer at the repo root and not a `config/config.yaml` under the tracked subtree, which is reviewed content and has to stay committable. The test asks git rather than reading the file, since the leading slash is the whole point. Co-Authored-By: Claude --- nerve/templates/config-repo/gitignore | 8 +++++++- tests/test_config_repo.py | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/nerve/templates/config-repo/gitignore b/nerve/templates/config-repo/gitignore index 1845f0f5..793ad3ff 100644 --- a/nerve/templates/config-repo/gitignore +++ b/nerve/templates/config-repo/gitignore @@ -2,7 +2,13 @@ # Secrets are referenced from settings.yaml via ${ENV_VAR} and supplied by the # environment; nothing sensitive should ever be committed here. -# Machine-local overrides / secrets +# Machine-local overrides / secrets. `/config.yaml` is anchored to the repo root +# so it only covers the machine-local layer, not a `config/config.yaml` someone +# adds under the tracked subtree. It is here because migration writes that file: +# splitting a legacy monolith leaves the per-machine half — TLS paths, an AWS +# profile, mail accounts — in `config.yaml` beside the settings it just published, +# and the runbook's own next step is `git add -A`. +/config.yaml config.local.yaml .env .env.* diff --git a/tests/test_config_repo.py b/tests/test_config_repo.py index 821cc43b..0f443769 100644 --- a/tests/test_config_repo.py +++ b/tests/test_config_repo.py @@ -100,6 +100,32 @@ def test_gitignore_excludes_secrets(self, tmp_path): assert "config.local.yaml" in body assert "*.migrated" in body + def test_the_machine_local_layer_is_ignored_at_the_root_only(self, tmp_path): + """Migration writes `config.yaml`, and the runbook then says `git add -A`. + + Asked of git rather than of the file's text, because the whole point is + the leading slash: `config.yaml` unanchored would also swallow a + `config/config.yaml` inside the tracked subtree, which is reviewed + content and has to stay committable. + """ + import shutil + import subprocess + + if not shutil.which("git"): + pytest.skip("git not available") + scaffold_config_repo(tmp_path) + subprocess.run(["git", "init", "-q"], cwd=str(tmp_path), check=True, + capture_output=True) + + def ignored(rel: str) -> bool: + return subprocess.run( + ["git", "check-ignore", "-q", rel], cwd=str(tmp_path), + ).returncode == 0 + + assert ignored("config.yaml"), "the machine-local layer must not be staged" + assert not ignored("config/config.yaml"), "tracked config must stay committable" + assert not ignored("config/settings.yaml") + def test_gitignore_covers_backup_secret_members(self, tmp_path): # If the workspace doubles as the state dir, `git add -A` (which the # runbook tells operators to run) must not sweep up nerve's credentials.