deploy: never let a non-main ref change flake inputs (guard + fail-closed on regression) #255

Merged

ymrtech merged 1 commit from fix/deploy-non-main-lock-guard into main 2026-09-15 17:33:08 -06:00

Conversation 1 Commits 1 Files changed 1 +191 -10

hermes commented 2026-09-15 17:26:05 -06:00

Collaborator

Copy link

Closes #253.

A workflow_dispatch on a non-main ref deploys ?rev=<ref head>, whose flake.lock is a copy of the ref's base — so every input main has moved since is silently reverted on the target host. Seen live: a PR-branch deploy on public reverted clientsv@244dc35 → @92d02cd and killed the bandwidth collector that had just shipped, with the run green.

The rule, now enforced

ref	what deploys
main	github.sha — unchanged
non-main, no input touched	the ref's tree with main's pins, pushed to deploy-lock/<ref> so each host job can fetch it by rev
non-main, adds an input	allowed — adding is a legitimate rare change; the new node survives
non-main, removes an input	allowed — cleanup of something unused can't regress anything; the reconcile does not resurrect it
non-main, changes an input	FAIL — existing inputs are owned by the weekly flake-update pipeline on main

Emergency escape hatch: allow_input_lock_change=true on the dispatch (documented as emergency-only; it deploys the ref's own lock with a loud warning).

Why classification uses the merge base, not main

Comparing straight to main would confuse "stale copy of the base" (normal, reconciled away) with "the ref intends this change". So it diffs the ref against merge-base(origin/main, HEAD) — what the ref itself did — and only that decides allow/refuse.

The merged lock is then re-verified before anything deploys: every input main pins must come out at main's rev, and every node reference must still resolve.

Fail-closed by design. If the guard cannot prove "no existing input moved" (can't clone/fetch/push), it fails the run instead of regressing a host silently. That means a provisioning dispatch now fails rather than silently downgrading inputs when git.ymrtech.com is unreachable — deliberate, and the allow_input_lock_change=true bypass is the escape hatch. Worth a look if you'd rather degrade differently.

Where it lives

One new deploy-rev job (runs-on: [ci], runs on both refs so no host job ever sees a skipped dependency); the four host jobs gained needs: [scope, deploy-rev] and use ${{ needs.deploy-rev.outputs.rev }} instead of github.sha. deployed.jsonl now records that resolved rev alongside the dispatched sha.

Verification

• The four jq programs were extracted from the shipped file and run against the real locks from the incident, with negative controls:
  • incident lock vs its own base → {added:[],removed:[],changed:[]} → reconcile; clientsv goes 92d02cd → 244dc35 (main's pin) ✓
  • bumped clientsv → changed:[clientsv] → refuse ✓
  • added node → added:[newdep] → allowed, node kept at its own rev, everything else from main ✓
  • removed node → removed:[comfyui-nix] → allowed, not resurrected from main ✓
  • verify catches a regressed pin ✓; dangling check catches a dropped node reference ✓ and passes a good lock ✓
• Three real bugs were caught by that exercise and fixed before this PR: $m|has(.key) re-binds . to $m (→ has(null), a hard error on every non-main deploy), input values can be a bare node string or [node,...], and flake.lock's top-level root is a node name (a string) — the root node itself lives in .nodes.
• yaml parses; the guard script passes bash -n both raw and after ${{ }} substitution; job graph, needs and the 6 resolved-rev uses verified.
• Not yet exercised end-to-end: the job's real run (clone → reconcile → push → output rev) is driven through a dispatch with an invalid hosts value, so the guard executes while the scope job aborts the run and nothing deploys.

Closes #253. A `workflow_dispatch` on a non-main ref deploys `?rev=<ref head>`, whose `flake.lock` is a copy of the ref's **base** — so every input main has moved since is silently reverted on the target host. Seen live: a PR-branch deploy on public reverted `clientsv@244dc35` → `@92d02cd` and killed the bandwidth collector that had just shipped, with the run green. ## The rule, now enforced | ref | what deploys | |---|---| | `main` | `github.sha` — unchanged | | non-main, no input touched | the ref's tree **with main's pins**, pushed to `deploy-lock/<ref>` so each host job can fetch it by rev | | non-main, **adds** an input | allowed — adding is a legitimate rare change; the new node survives | | non-main, **removes** an input | allowed — cleanup of something unused can't regress anything; the reconcile does not resurrect it | | non-main, **changes** an input | **FAIL** — existing inputs are owned by the weekly flake-update pipeline on main | Emergency escape hatch: `allow_input_lock_change=true` on the dispatch (documented as emergency-only; it deploys the ref's own lock with a loud warning). ## Why classification uses the merge base, not main Comparing straight to main would confuse *"stale copy of the base"* (normal, reconciled away) with *"the ref intends this change"*. So it diffs the ref against `merge-base(origin/main, HEAD)` — what the ref itself did — and only that decides allow/refuse. The merged lock is then re-verified before anything deploys: **every input main pins must come out at main's rev**, and **every node reference must still resolve**. **Fail-closed by design.** If the guard cannot prove "no existing input moved" (can't clone/fetch/push), it fails the run instead of regressing a host silently. That means a provisioning dispatch now fails rather than silently downgrading inputs when git.ymrtech.com is unreachable — deliberate, and the `allow_input_lock_change=true` bypass is the escape hatch. Worth a look if you'd rather degrade differently. ## Where it lives One new `deploy-rev` job (`runs-on: [ci]`, runs on both refs so no host job ever sees a skipped dependency); the four host jobs gained `needs: [scope, deploy-rev]` and use `${{ needs.deploy-rev.outputs.rev }}` instead of `github.sha`. `deployed.jsonl` now records that resolved `rev` alongside the dispatched `sha`. ## Verification - The four jq programs were **extracted from the shipped file** and run against the real locks from the incident, with negative controls: - incident lock vs its own base → `{added:[],removed:[],changed:[]}` → reconcile; `clientsv` goes `92d02cd → 244dc35` (main's pin) ✓ - bumped `clientsv` → `changed:[clientsv]` → refuse ✓ - added node → `added:[newdep]` → allowed, node kept at its own rev, everything else from main ✓ - removed node → `removed:[comfyui-nix]` → allowed, not resurrected from main ✓ - verify catches a regressed pin ✓; dangling check catches a dropped node reference ✓ and passes a good lock ✓ - Three real bugs were caught by that exercise and fixed before this PR: `$m|has(.key)` re-binds `.` to `$m` (→ `has(null)`, a hard error on every non-main deploy), input values can be a bare node string or `[node,...]`, and `flake.lock`'s top-level `root` is a node **name** (a string) — the root node itself lives in `.nodes`. - `yaml` parses; the guard script passes `bash -n` both raw and after `${{ }}` substitution; job graph, `needs` and the 6 resolved-rev uses verified. - Not yet exercised end-to-end: the job's real run (clone → reconcile → push → output rev) is driven through a dispatch with an invalid `hosts` value, so the guard executes while the scope job aborts the run and **nothing deploys**.

hermes added 1 commit 2026-09-15 17:26:05 -06:00

deploy: never let a non-main ref change flake inputs (issue #253) ... b462021dff

A workflow_dispatch on a non-main ref (PR preview, provision dispatch) runs
`nixos-rebuild switch --flake git+...#<host>?rev=${{ github.sha }}`, and that
sha is the ref's head — whose flake.lock is a copy of the ref's BASE. Every
input main has moved since is then silently reverted on the target host.

Seen live: a PR-branch deploy on public reverted clientsv@244dc35 to
@92d02cd, killing the per-peer bandwidth collector that had just shipped.
The branch never touched flake.lock; it just carried a stale copy. Nothing
was red — the health gate only checks critical services.

New `deploy-rev` job resolves the rev every host job deploys, and is where the
rule now lives: existing flake inputs come from main, always.

  - main ref                         → github.sha, unchanged.
  - non-main, no input changed       → the ref's TREE with main's pins, pushed
                                       to deploy-lock/<ref> so each host job
                                       can fetch it by rev (provenance and the
                                       ?rev= contract are preserved).
  - non-main, ADDS an input          → allowed: adding is a legitimate rare
                                       change, and the new node survives.
  - non-main, REMOVES an input       → allowed: cleanup of something unused
                                       cannot regress anything still in use,
                                       and the reconcile does not resurrect it.
  - non-main, CHANGES an input       → FAIL. Existing inputs are owned by the
                                       weekly flake-update pipeline on main.

Classification compares the ref against its merge base (what the REF did),
never against main (which would confuse a stale copy with intent). The merged
lock is then re-verified: every input main pins must come out at main's rev,
and every node reference must still resolve.

Fail-closed by design: if the guard cannot prove "no existing input moved" it
fails the run rather than regressing a host silently. The only bypass is an
explicit allow_input_lock_change=true dispatch input (emergency only).

The four programs were extracted from this file and exercised against the real
locks from the incident, plus negative controls (a bumped input must be
refused, a regressed pin and a dangling node reference must each be caught).

hermes force-pushed fix/deploy-non-main-lock-guard from b462021dff to 3c488621a5 2026-09-15 17:30:27 -06:00 Compare

hermes referenced this pull request 2026-09-15 17:30:40 -06:00

deploy: a PR-branch dispatch reverts every other flake input (non-main refs deploy the branch's stale flake.lock) #253

hermes commented 2026-09-15 17:30:50 -06:00

Author

Collaborator

Copy link

Rebased onto main (now 3c48862) — #250/#251 landed in the same file, and the single conflict was giga's needs: (they want it after deploy-vpn, the guard wants deploy-rev); resolved to [scope, deploy-rev, deploy-vpn], keeping their always() && needs.scope.outputs.giga_check == '1' and their DNS-quiesce comment intact. Re-validated after the rebase: YAML, bash -n, job graph, inputs, 6 resolved-rev uses, and the four guard programs re-run against the incident locks.

End-to-end, deploy-free. A throwaway branch forked before the clientsv bump (its lock pinned clientsv@92d02cd; main 244dc35; the branch itself never touched flake.lock — the incident's exact shape) was dispatched with an invalid hosts value:

• the scope job aborted the run in 1s, so no host job ran and nothing deployed;
• the guard job really ran: it pushed deploy-lock/guard-test/stale, whose clientsv pin is 244dc35 (main's), with all 30 shared nodes at main's pin, and the reconciled commit carries the guard's own workflow.

Both throwaway branches were then deleted.

Three real bugs were caught and fixed before this PR: $m|has(.key) re-binds . to $m (→ has(null), a hard error on every non-main deploy); input values may be a bare node string or [node,...]; and flake.lock's top-level root is a node name (a string) — the root node itself lives in .nodes, so my .root.inputs clause errored on every lock.

One thing worth your call: the guard is fail-closed, so a provisioning dispatch (hosts=vpn on a device branch) now fails rather than silently downgrading inputs if git.ymrtech.com is unreachable. The allow_input_lock_change=true bypass is the escape hatch. Say the word if you'd rather it degrade differently for provisioning.

Also worth noting for review: the guard only reconciles — it does not stop a merge on main from carrying a reverted flake.lock (that is #255's sibling problem: main-level lock reverts still need the weekly watchdog or a PR-level check).

Rebased onto main (now `3c48862`) — #250/#251 landed in the same file, and the single conflict was giga's `needs:` (they want it after `deploy-vpn`, the guard wants `deploy-rev`); resolved to `[scope, deploy-rev, deploy-vpn]`, keeping their `always() && needs.scope.outputs.giga_check == '1'` and their DNS-quiesce comment intact. Re-validated after the rebase: YAML, `bash -n`, job graph, inputs, 6 resolved-rev uses, and the four guard programs re-run against the incident locks. **End-to-end, deploy-free.** A throwaway branch forked before the clientsv bump (its lock pinned `clientsv@92d02cd`; main `244dc35`; the branch itself never touched `flake.lock` — the incident's exact shape) was dispatched with an invalid `hosts` value: - the scope job aborted the run in 1s, so **no host job ran and nothing deployed**; - the guard job really ran: it pushed `deploy-lock/guard-test/stale`, whose `clientsv` pin is **`244dc35`** (main's), with all 30 shared nodes at main's pin, and the reconciled commit carries the guard's own workflow. Both throwaway branches were then deleted. **Three real bugs were caught and fixed before this PR:** `$m|has(.key)` re-binds `.` to `$m` (→ `has(null)`, a hard error on every non-main deploy); input values may be a bare node string *or* `[node,...]`; and `flake.lock`'s top-level `root` is a node **name** (a string) — the root node itself lives in `.nodes`, so my `.root.inputs` clause errored on every lock. **One thing worth your call:** the guard is fail-closed, so a provisioning dispatch (`hosts=vpn` on a device branch) now fails rather than silently downgrading inputs if git.ymrtech.com is unreachable. The `allow_input_lock_change=true` bypass is the escape hatch. Say the word if you'd rather it degrade differently for provisioning. Also worth noting for review: the guard only *reconciles* — it does not stop a merge on main from carrying a reverted `flake.lock` (that is #255's sibling problem: main-level lock reverts still need the weekly watchdog or a PR-level check).

ymrtech merged commit 56cc118594 into main 2026-09-15 17:33:08 -06:00

ymrtech deleted branch fix/deploy-non-main-lock-guard 2026-09-15 17:33:09 -06:00

ymrtech referenced this pull request from a commit 2026-09-15 17:33:13 -06:00

Merge pull request 'deploy: never let a non-main ref change flake inputs (guard + fail-closed on regression)' (#255) from fix/deploy-non-main-lock-guard into main

hermes referenced this pull request from a commit 2026-09-15 17:37:33 -06:00

ci: fail a PR that moves an existing flake input backwards (issue #253)

hermes referenced this pull request 2026-09-15 17:38:28 -06:00

ci: fail a PR that moves an existing flake input backwards #257

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backlog

  

bug

  

enhancement

  

in-progress

  

review

  

todo

No labels

backlog

bug

enhancement

in-progress

review

todo

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

ymrtech/nix-config!255

No description provided.