flake-update-cascade: read protected-branches by the same rule as the rails #245

Merged

vnprc merged 2 commits from agent/cascade-protection-rule into master

2026-09-29 00:41:07 +01:00

allod-agent commented

2026-09-28 16:17:07 +01:00

Member

Owner summary

What changes on a running machine: flake-update-cascade decides whether a branch is protected by the same rule as allod change and the git hook. A protected entry written with extra spaces or a trailing slash is now honoured, where before it silently read as "not protected" and a direct run pushed to that branch.
Two new refusals before the cascade touches anything: a protected list that is present but cannot be read, and a protected repository checked out at the wrong path. Each stops the run and names the file or both paths.
What stays identical: allod change behaves exactly as before; its rule only moved into a shared package. A correctly written list on a machine with the default work folder gives the same answers as before.
The cascade never protects less than it did. The old key, work/<repo>, still matches, so a custom WORK_DIR keeps its protection.
What proves it: the repository's whole suite passes through allod check; the new cases go red on the code before each fix; forcing protection off turns the suite red.
If it is wrong: revert this PR. Nothing is stored or migrated.

Closes allod/tools#237

What changed

390 lines added and 247 removed, most of it the rule moving.

The rule moves from cmd/allod/workspace.go to a new package, internal/protection. It returns errors and never exits or prints. The shared case table, tests/fixtures/protection-cases.tsv, is now tested there, once, for both commands.
cmd/allod calls the package through wrappers that produce the messages and exit codes it produced before.
flake-update-cascade calls the package in place of its exact-text match.
The stand-in git used by the cascade's test suites answers two more questions, because the rule now asks git where a repository's main checkout is.

Where this departs from the issue

The issue says a misplaced checkout is "skipped with a message". It is a preflight error instead: that mechanism already existed, stops the run, and matches allod change, which refuses outright in the same situation.

Risk

R2. The change is in how a tool that pushes to default branches decides what is protected. The rule itself was already in use and tested; what is new is one call site.

Known limits:

The lookup runs only for repositories the run would touch. A repository skipped for another reason (no flake.lock, an active PR, no matching input) gets no verdict, so an unreadable list goes unreported if every repository is skipped first. The README now says so.
Two suites are not part of the flake checks because they need real Nix or a terminal: flake-update-cascade-chain.sh, which the implementing agent ran by hand on the first commit and not after the fix round, and flake-update-cascade-nixconfig.sh, which was not run.
cmd/flake-update-cascade/workspace.go still has its own copy of the expression that reads owner/repo from a remote URL. Left alone.

Validation

Run by the integrator on acc67de:

Command	Result
`allod check`	exit 0, "All 11 steps passed (checks: 9 built)"
`tests/flake/flake-update-cascade/preflight.sh` with a build of `acc67de`	22 of 22 pass
The same suite with a build of `be3c65d`, before the fix round	red at "skips a protected repository in a workspace outside $HOME/work": the run pulled and updated the repository
The first version of that suite with protection forced to false	red at "skips protected branches in direct mode"

Reported by the implementing agent and not repeated by the integrator: restoring the old exact-text rule turns the two-spaces case red while the single-space case stays green; tests/allod-change.sh 270 and tests/git-hooks/protected-refs-policy.sh 89 run directly (both also run inside allod check).

Review

One cold review of be3c65d by gpt-5.6-sol at high effort, followed by one fix round in acc67de. The model was run through the pi runner with provider nullsink, inside a read-only sandbox, because the Codex account had reached its usage limit. The reviewer was also asked what in the change should not exist and found nothing.

Finding	Severity	Outcome
With `WORK_DIR` set to anything but `$HOME/work`, a repository the old rule protected was pushed to. The reviewer executed it	high	Fixed: the old key is kept beside the new rule; the reviewer's fixture is now a test
Lookup runs after the early skips, so some repositories get no verdict	medium	Discarded, see Known limits; README reworded
The new tests would pass if misplacement depended on the default branch or the mode	low	One case added. It was already green
Six comments that restate the code	low	Removed or shrunk

## Owner summary - **What changes on a running machine:** `flake-update-cascade` decides whether a branch is protected by the same rule as `allod change` and the git hook. A protected entry written with extra spaces or a trailing slash is now honoured, where before it silently read as "not protected" and a direct run pushed to that branch. - **Two new refusals before the cascade touches anything:** a protected list that is present but cannot be read, and a protected repository checked out at the wrong path. Each stops the run and names the file or both paths. - **What stays identical:** `allod change` behaves exactly as before; its rule only moved into a shared package. A correctly written list on a machine with the default work folder gives the same answers as before. - **The cascade never protects less than it did.** The old key, `work/<repo>`, still matches, so a custom `WORK_DIR` keeps its protection. - **What proves it:** the repository's whole suite passes through `allod check`; the new cases go red on the code before each fix; forcing protection off turns the suite red. - **If it is wrong:** revert this PR. Nothing is stored or migrated. Closes allod/tools#237 ## What changed 390 lines added and 247 removed, most of it the rule moving. - The rule moves from `cmd/allod/workspace.go` to a new package, `internal/protection`. It returns errors and never exits or prints. The shared case table, `tests/fixtures/protection-cases.tsv`, is now tested there, once, for both commands. - `cmd/allod` calls the package through wrappers that produce the messages and exit codes it produced before. - `flake-update-cascade` calls the package in place of its exact-text match. - The stand-in `git` used by the cascade's test suites answers two more questions, because the rule now asks git where a repository's main checkout is. ## Where this departs from the issue The issue says a misplaced checkout is "skipped with a message". It is a preflight error instead: that mechanism already existed, stops the run, and matches `allod change`, which refuses outright in the same situation. ## Risk R2. The change is in how a tool that pushes to default branches decides what is protected. The rule itself was already in use and tested; what is new is one call site. Known limits: - **The lookup runs only for repositories the run would touch.** A repository skipped for another reason (no `flake.lock`, an active PR, no matching input) gets no verdict, so an unreadable list goes unreported if every repository is skipped first. The README now says so. - **Two suites are not part of the flake checks** because they need real Nix or a terminal: `flake-update-cascade-chain.sh`, which the implementing agent ran by hand on the first commit and not after the fix round, and `flake-update-cascade-nixconfig.sh`, which was not run. - `cmd/flake-update-cascade/workspace.go` still has its own copy of the expression that reads `owner/repo` from a remote URL. Left alone. ## Validation Run by the integrator on `acc67de`: | Command | Result | | --- | --- | | `allod check` | exit 0, "All 11 steps passed (checks: 9 built)" | | `tests/flake/flake-update-cascade/preflight.sh` with a build of `acc67de` | 22 of 22 pass | | The same suite with a build of `be3c65d`, before the fix round | red at "skips a protected repository in a workspace outside $HOME/work": the run pulled and updated the repository | | The first version of that suite with protection forced to false | red at "skips protected branches in direct mode" | Reported by the implementing agent and not repeated by the integrator: restoring the old exact-text rule turns the two-spaces case red while the single-space case stays green; `tests/allod-change.sh` 270 and `tests/git-hooks/protected-refs-policy.sh` 89 run directly (both also run inside `allod check`). ### Review One cold review of `be3c65d` by `gpt-5.6-sol` at `high` effort, followed by one fix round in `acc67de`. The model was run through the `pi` runner with provider `nullsink`, inside a read-only sandbox, because the Codex account had reached its usage limit. The reviewer was also asked what in the change should not exist and found nothing. | Finding | Severity | Outcome | | --- | --- | --- | | With `WORK_DIR` set to anything but `$HOME/work`, a repository the old rule protected was pushed to. The reviewer executed it | high | Fixed: the old key is kept beside the new rule; the reviewer's fixture is now a test | | Lookup runs after the early skips, so some repositories get no verdict | medium | Discarded, see Known limits; README reworded | | The new tests would pass if misplacement depended on the default branch or the mode | low | One case added. It was already green | | Six comments that restate the code | low | Removed or shrunk |

allod-agent added 2 commits

2026-09-28 16:17:07 +01:00

Read protected-branches by one rule in every Go reader be3c65db6e

flake-update-cascade decided protection by comparing a whole line of
~/.config/git/protected-branches, so two spaces, a trailing slash, an
unreadable list, or a checkout at another path all read as "not
protected". Move the rule 'allod change' already uses into a new
internal/protection package and have both commands call it.

cmd/allod keeps mainRepoDir, repoLookupKey and lookupProtection as
wrappers that turn the package's errors into the die they produce today,
so no behaviour of 'allod change' changes. The cascade records a
preflight error for a list it cannot read and for a checkout the list
places elsewhere, in every mode, and otherwise asks the package whether
the default branch is listed.

The shared-table tests move with the code and still read
tests/fixtures/protection-cases.tsv. The cascade preflight suite gains
the three cases that prove the new call site, and the mock git learns
the two rev-parse forms the rule asks of every repository.

Refs allod/tools#237

Keep the work/<repo> key the cascade always had acc67de684

With WORK_DIR pointing anywhere but $HOME/work, the list entry
'work/<repo> <branch>' matched the old rule and not the new one, so a
direct run committed and pushed to a protected branch. Lookup now takes
further list paths that name the same repository, and the cascade passes
work/<repo>; a match there is a path match, never a misplaced checkout.
'allod change' passes none and is unchanged.

The preflight suite gains that fixture and one case pinning that a
misplaced checkout is refused in a dry run even when its entry names a
branch that is not the default. The pre-flight list in flake/README.md
no longer claims every repository is checked: a repository the run skips
for another reason is never asked for a verdict, because the lookup
guards a push.

Refs allod/tools#237

vnprc force-pushed agent/cascade-protection-rule from acc67de684 to 3f917ebfa9

2026-09-29 00:39:43 +01:00

Compare