Skip to content

Auto-migrate legacy config into workspace/config (scrub + backup) (8/13) - #211

Open
alex-clickhouse wants to merge 1 commit into
config-refactor/07-validatefrom
config-refactor/08-migrate
Open

Auto-migrate legacy config into workspace/config (scrub + backup) (8/13)#211
alex-clickhouse wants to merge 1 commit into
config-refactor/07-validatefrom
config-refactor/08-migrate

Conversation

@alex-clickhouse

@alex-clickhouse alex-clickhouse commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

When it runs

Three entry points, all calling the same routine:

  • nerve start and nerve upgrade, automatically, and only when config loaded. Best-effort: maybe_migrate never raises, so a failure here cannot stop the daemon. On start this happens before the already-running check, so it also fires when a daemon is already up.
  • nerve migrate [--dry-run], explicitly. This one is a self-diagnosing command, so it runs even when config fails to load (the workspace is then resolved from the machine-local files rather than the loaded object), and it exits non-zero on failure instead of swallowing it.

The config half runs only if all three hold:

  1. <config_dir>/config.yaml exists, where config_dir is ~/.nerve or wherever NERVE_HOME points.
  2. <workspace>/config/settings.yaml carries no configuration: absent, or parsing to an empty mapping. The comments-only file nerve init scaffolds counts as empty, deliberately — treating it as "already migrated" made migration a permanent no-op for every install created after that scaffold shipped. A settings.yaml that will not parse counts as content, because migration must never overwrite what it cannot read.
  3. config.yaml holds at least one key outside _MACHINE_LOCAL_PATHS, which is what makes it a legacy monolith. A post-split config.yaml holding nothing but machine-local keys is left exactly where it is.

If settings.yaml does have content while config.yaml still holds shareable keys, nothing is migrated and the mismatch is reported instead. That state and an interrupted migration are indistinguishable on disk, so it says so rather than guessing.

The cron half runs only if the legacy cron directory exists and at least one file in it, ignoring *.migrated breadcrumbs, has no counterpart at the same relative path under <workspace>/config/cron/. The workspace is what config.workspace points at, or what config.yaml/config.local.yaml name, falling back to the default; the legacy directory is paths.cron_dir(), under NERVE_HOME.

Both halves are idempotent, both are no-ops once migrated, and --dry-run writes nothing.

What

One idempotent routine that moves a pre-refactor install to the new layout, in two independent halves.

Config. A legacy monolithic config.yaml is copied into the tracked layer as <workspace>/config/settings.yaml, secrets move into the machine-local overlay as ${ENV_VAR} references, and the original is tightened to owner-only and left as a *.migrated breadcrumb so it stops shadowing the file that replaced it.

Cron. The legacy ~/.nerve/cron/ becomes <workspace>/config/cron/, which puts jobs in the tracked workspace alongside everything else. The whole directory is copied rather than just jobs.yaml and system.yaml, because a job can reference a sibling by relative path (prompt_file: prompts/daily.md) and taking only the two files migration knows about would break it. Nothing already present in the workspace is overwritten. Only those two files are then renamed to breadcrumbs, which is what stops the legacy directory reading as "has jobs" on a later pass. Because that pass can happen more than once, breadcrumb names are allocated to a free suffix (.migrated, .migrated.1) rather than a fixed one: Path.rename overwrites silently on POSIX, and a fixed suffix would let a second pass destroy the only surviving copy of an earlier original.

Gate plugins come with it, and the copy is per file. cron/gates/*.py is custom gate code the daemon imports at startup and on every reload, and losing it is not a soft failure: build_gates raises on an unknown gate type, and that takes the whole job with it (skipped with an error at startup, a 400 refusing the change set at reload). That is deliberate upstream of this PR, because silently dropping a gate would make a job run more often than its config asks for.

Which is why the cron half copies file by file instead of testing whether the workspace already has cron config. The two sides overlap in the ordinary upgrade: nerve init creates <workspace>/config/cron/gates/ empty, writes system.yaml, and copies only jobs.yaml across. A directory-level test skipped everything else permanently, because the legacy directory stops being consulted the moment the workspace has job files, so gates/*.py and any prompts/ a job names by relative path were stranded where nothing reads them and custom gates quietly stopped loading. Per file, a gates/ that already exists is merged rather than skipped whole.

A file already in the workspace is never overwritten; that copy is the reviewed one. The one case worth reporting is a legacy jobs.yaml or system.yaml that loses to one already in place: those are real cron jobs that now run nowhere, migration cannot merge two sets of jobs, and it is not breadcrumbed either, since renaming it would hide the operator's only copy. Gate and prompt files are copied and not breadcrumbed, so the legacy copies stay readable to anything mid-upgrade still resolving them by path. This also puts executable gate code into the reviewable git-tracked subtree, which is where the lockdown work downstream expects to find it.

Both halves run automatically when the old layout is detected, and are exposed as nerve migrate [--dry-run]. Being independent matters for the common case of an install that has already split its config by hand: it still gets its cron directory moved.

_MACHINE_LOCAL_PATHS is the single list of what stays on the box, used to tell a legacy monolith from an already-split config.yaml. Two tests check it in both directions: one drives the wizard and fails if it emits a machine-local path the list does not cover, the other migrates a monolith and fails if a shareable key is left behind.

Why

Upgrading in place has to work without the operator reading a migration guide, and it has to be safe to run twice — hence idempotent, never destructive, and best-effort so a failure at daemon start does not stop the daemon.

Scrubbing is the part that needs care, because the destination is a file the docs tell you to commit. Secrets are identified by key name, by value shape (an sk-/ghp_ token, a user:pass@host DSN, a token in a query string — the half a key-name list can never catch), by being anywhere inside an env or headers mapping where arbitrarily-named secrets live, and by position: the argv item after a flag that names a credential. That last one matters because --token=abc is a single string that can be matched on shape while ["--token", "abc"] splits the name off the value, and the value alone is unrecognizable — no prefix, no separator, whatever length the vendor chose. Split argv is the form npx and uvx MCP servers are configured with, so without it the common case went to the tracked file in plaintext and was not even flagged as suspect. A flag does not arm another flag: --token --verbose is a malformed command line, not a token named --verbose.

Heuristics cannot be exhaustive, so migration prints what it moved and anything left behind that still looks credential-shaped, and says to review the file before committing.

Telling the two shapes of config.yaml apart is load-bearing in both directions. An emptied settings.yaml is not evidence that a config.yaml is a monolith — a workspace loses its settings file by being repointed, moved, or interrupted mid-init — so the test is a positive one on the machine file's contents. And entries in the list are scoped to the part that is genuinely local: listing a whole subtree keeps every key under it out of the tracked layer, which is why gateway and provider were wrong as whole subtrees. A legacy install's bind port and provider type stayed in config.yaml, which lockdown does not read.

Two keys get special handling on the way across. workspace stays machine-local, or the tracked file would name a path that only resolves on one box. lockdown is dropped, with a note: the machine layers ignore it by design so that a local edit cannot unlock or fake-lock an instance, and copying it into settings.yaml would promote a flag that has never had any effect into the one file where it is authoritative. Locking then drops the machine layers, including the config.local.yaml this migration just moved the secrets into, so the next start would fail on a ${VAR} it used to resolve. Dropping it preserves what the box already does.

Migration is also not one transaction, and it is not treated as one. The config half commits before the cron half runs, so an exception in the cron half leaves settings.yaml written, the secrets relocated and config.yaml renamed away. maybe_migrate returns the partial report rather than None for that: reporting it as "nothing happened" made nerve upgrade print no review prompt for a tracked file that now exists and may hold an unscrubbed credential, and made nerve start skip the config reload it needs once the files have moved underneath it. The log distinguishes "skipped" from "half-applied", and upgrade names what did apply and says to re-run.

Why a new atomic_write_text()

Because migration rewrites config.local.yaml, and that file already holds secrets that exist in no backup anywhere: the wizard puts the API key, the OAuth token and the password hash there. Migration merges its own additions into whatever is present, so that rewrite is the one step in the flow that can destroy a credential outright. Scrubbed secrets are recoverable from the *.migrated breadcrumb; the operator's pre-existing local values are not.

The codex backend already had a private _atomic_write doing temp-file-plus-rename. It was not sufficient here, on four counts:

  • No fsync. replace() is atomic against the process dying, not against the machine dying. Renaming a file whose bytes are still only in page cache is how ext4 delayed allocation produces a zero-length config.local.yaml after a power loss, which is worse than either of the two outcomes the rename exists to guarantee. The parent directory has to be flushed too, or the rename itself is not durable.
  • chmod after the write. The old helper created the temp file at the umask, wrote the secret, and chmod'd afterwards, leaving the secret readable at the umask for the gap between those calls. The new one chmods the descriptor before the first byte goes in.
  • A guessable temp path. .{name}.{pid}.tmp, written with a plain write_text, follows a symlink planted at that path and puts the secret wherever it points. mkstemp uses O_EXCL and an unpredictable name.
  • No way to keep a file's existing permissions. settings.yaml is meant to be shareable and config.local.yaml owner-only, and rewriting either must not silently re-permission it. mode=None reproduces what open(path, "w") would have left, including on a file that already exists.

It also writes through a symlinked destination instead of replacing the link with a regular file, which is what a config.local.yaml linked into a dotfiles repo needs.

Converting the codex backend's copy to the shared helper picks up the same four fixes there. nerve/external_agents/writer.py still has a third copy with the same weaknesses; it writes ~/.codex/config.toml rather than anything Nerve owns, so it is left for a follow-up.

🤖 Generated with Claude Code

@alex-clickhouse alex-clickhouse changed the title Auto-migrate legacy config into workspace/config (scrub + backup) Auto-migrate legacy config into workspace/config (scrub + backup) (8/13) Jul 28, 2026
@alex-clickhouse
alex-clickhouse requested a review from Copilot July 28, 2026 10:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an idempotent “legacy → workspace/config” migration path that scrubs secrets into the machine-local overlay, leaves *.migrated backups, and runs automatically on upgrade/start (plus a nerve migrate command). This supports the broader config refactor by safely moving shareable settings and cron config into the workspace-tracked layout while keeping secrets and machine-local paths out of the tracked layer.

Changes:

  • Introduce nerve.migrate to migrate legacy config.yaml + legacy cron into workspace/config/ with secret scrubbing and backups.
  • Add nerve.utils.fs.atomic_write_text for crash-safe, permission-aware atomic writes and use it in the Codex Ultracode backend.
  • Wire migration into nerve start, nerve upgrade, and add a nerve migrate [--dry-run] CLI command; document the migration behavior and add extensive tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_utils_fs.py Adds coverage for atomic, permission-correct file writing semantics.
tests/test_migrate.py Adds comprehensive tests for config/cron migration, secret scrubbing, idempotency, and crash-safety.
nerve/utils/fs.py Introduces a shared atomic text writer with mode handling and durability guarantees.
nerve/migrate.py Implements legacy-to-workspace migration (scrub + backup + cron move) with reporting.
nerve/cli.py Runs migration on start/upgrade and adds a migrate subcommand.
nerve/agent/backends/codex/ultracode.py Replaces local atomic writer with the shared atomic_write_text.
docs/config.md Documents migration behavior, limits, and operator guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread nerve/utils/fs.py
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from c2eaddd to 127f793 Compare July 28, 2026 12:11
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from 127f793 to 786b30d Compare July 28, 2026 12:36
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from 786b30d to c3960af Compare July 28, 2026 13:14
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from c3960af to be06ac8 Compare July 28, 2026 14:23
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from be06ac8 to 840007a Compare July 28, 2026 14:54
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch 2 times, most recently from 5b7ae3a to 9d63d48 Compare July 28, 2026 16:03
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch 2 times, most recently from 16b41ef to f4fab4f Compare July 28, 2026 18:40
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from f4fab4f to a370972 Compare July 29, 2026 08:29
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from a370972 to ab5dee0 Compare July 29, 2026 09:18
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from ab5dee0 to 145d1c0 Compare July 29, 2026 09:31
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from 145d1c0 to 75bf9c3 Compare July 29, 2026 10:31
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from 75bf9c3 to bc27e72 Compare July 29, 2026 13:28
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from bc27e72 to 1038bbd Compare July 30, 2026 08:01
@alex-clickhouse
alex-clickhouse marked this pull request as ready for review July 30, 2026 08:10
One idempotent routine copies the legacy layout into <workspace>/config/, leaving
the originals as breadcrumbs, and scrubs secrets on the way: known sensitive keys
move to the machine-local overlay and the tracked file gets an ${ENV_VAR}
reference in their place. A value that cannot be classified confidently stays
local rather than being committed, and migration prints exactly what moved. It
runs from upgrade and daemon start, no-ops once migrated, and --dry-run mutates
nothing.

Migration is isolable — it honours NERVE_HOME instead of reaching for real
machine state — crash-safe, and writes owner-only. A config.yaml that is already
the machine half is not migrated a second time, and a workflow journal directory
is not mistaken for shareable config. Rewriting a file no longer re-permissions
it.

Co-Authored-By: Claude <noreply@anthropic.com>
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/08-migrate branch from 1038bbd to a2a987b Compare July 30, 2026 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants