Resolve the default branch or fail, never guess master #232

Merged
vnprc merged 5 commits from agent/default-branch-resolve into master 2026-09-27 04:45:37 +01:00
Member

The workspace tools no longer guess master when they cannot tell a repo's default branch. Where a tool would change something (allod change submit picking a PR base, pull-all --switch, flake-update-cascade preflight, the secrets landing check), it now stops and prints the one repair: git -C <repo> remote set-head origin -a. Where a tool only reports (flake-status), it shows the branch as unknown instead of comparing against a guess. A repo in normal use has this ref, so day to day nothing changes; the difference shows only on a checkout built without a fetch or after git remote set-head origin -d. allod change begin and record behave as before. Every suite passes, a cold reviewer from another model family found nothing, and rollback is a revert of four commits.

Closes allod/tools#126

What changes

The helper existed three times, and all three guessed master on failure. Each now fails and every caller decides:

Copy Consumers On an unresolvable default branch
cmd/allod/workspace.go defaultRemoteBranch begin, submit, the unpushed-commit guard in front of cleanup, the secrets landing check submit refuses naming the repair or --base; begin and the secrets check refuse as they already did, now through the helper alone (their duplicate pre-checks are dropped, as the issue's Scope said); the cleanup guard keeps answering "unknown", never "clean"
cmd/flake-update-cascade defaultBranch preflight and the pin graph preflight reports the repo with the repair named and aborts before touching anything
lib/workspace.sh workspace_repo_default_branch pull-all, flake-status pull-all --switch skips that repo with the repair named (a plain pull-all never consulted it); flake-status prints [default branch unknown — run: ...]

The Bash helper's result no longer depends on the caller's pipefail: it prints the name and exits 0, or prints nothing and exits non-zero. The cascade tool was not in the issue's callsite list; it carried its own copy of the same guess, so it is treated the same way here rather than left as the last silent one.

Tests: Go unit tests for both Go helpers on a repo whose origin/HEAD is unset; shell assertions for submit's refusal, record's path, pull-all --switch skipping, flake-status's warning, and cascade preflight. Two fixture repos (nohead, unheaded) carry the unresolvable state on purpose instead of a fixture that fell into it by accident. Docs: workspace/README.md, flake/README.md, docs/allod-secret.md.

Risk

R2. Localized behavior change across several consumers, each on its failure path only; the happy path is untouched. The worst credible failure after validation is a refusal on a checkout that used to be silently guessed at, with the repair printed. The destructive path (cleanup) is unchanged in its "cannot tell" answer. One cross-family reviewer (codex, gpt-5.5) read the full diff cold against the issue and reported no findings after checking the pipefail interactions, every (string, bool) call site, and the fixtures.

Follow-up after merge: the active plan allod/strategy dev-plans/concurrent-agent-workspace.md describes the old helper's behavior and gets one line saying it now fails loudly.

Validation

  • nix build .#checks.x86_64-linux.go-checks, flake-status-suite, cascade-suites, allod-parity: all passed, rerun by the integrator on the final commit.
  • go test ./... including -tags site,secret: ok. gofmt -l, go vet ./...: clean.
  • tests/allod-change.sh: 222 passed. tests/workspace/pull-all.sh: 33 passed (this suite is not wired into a flake check, so it was run directly). tests/flake/flake-status.sh: 36 passed. tests/flake/flake-update-cascade/preflight.sh: 13 passed.
  • shellcheck -s bash -x over the touched shell files: clean. The one finding without -s bash, a missing shebang on the sourced library, predates this change.
The workspace tools no longer guess `master` when they cannot tell a repo's default branch. Where a tool would change something (`allod change submit` picking a PR base, `pull-all --switch`, `flake-update-cascade` preflight, the secrets landing check), it now stops and prints the one repair: `git -C <repo> remote set-head origin -a`. Where a tool only reports (`flake-status`), it shows the branch as unknown instead of comparing against a guess. A repo in normal use has this ref, so day to day nothing changes; the difference shows only on a checkout built without a fetch or after `git remote set-head origin -d`. `allod change begin` and `record` behave as before. Every suite passes, a cold reviewer from another model family found nothing, and rollback is a revert of four commits. Closes allod/tools#126 ## What changes The helper existed three times, and all three guessed `master` on failure. Each now fails and every caller decides: | Copy | Consumers | On an unresolvable default branch | |---|---|---| | `cmd/allod/workspace.go` `defaultRemoteBranch` | `begin`, `submit`, the unpushed-commit guard in front of `cleanup`, the secrets landing check | `submit` refuses naming the repair or `--base`; `begin` and the secrets check refuse as they already did, now through the helper alone (their duplicate pre-checks are dropped, as the issue's Scope said); the `cleanup` guard keeps answering "unknown", never "clean" | | `cmd/flake-update-cascade` `defaultBranch` | preflight and the pin graph | preflight reports the repo with the repair named and aborts before touching anything | | `lib/workspace.sh` `workspace_repo_default_branch` | `pull-all`, `flake-status` | `pull-all --switch` skips that repo with the repair named (a plain `pull-all` never consulted it); `flake-status` prints `[default branch unknown — run: ...]` | The Bash helper's result no longer depends on the caller's `pipefail`: it prints the name and exits 0, or prints nothing and exits non-zero. The cascade tool was not in the issue's callsite list; it carried its own copy of the same guess, so it is treated the same way here rather than left as the last silent one. Tests: Go unit tests for both Go helpers on a repo whose `origin/HEAD` is unset; shell assertions for `submit`'s refusal, `record`'s path, `pull-all --switch` skipping, `flake-status`'s warning, and cascade preflight. Two fixture repos (`nohead`, `unheaded`) carry the unresolvable state on purpose instead of a fixture that fell into it by accident. Docs: `workspace/README.md`, `flake/README.md`, `docs/allod-secret.md`. ## Risk R2. Localized behavior change across several consumers, each on its failure path only; the happy path is untouched. The worst credible failure after validation is a refusal on a checkout that used to be silently guessed at, with the repair printed. The destructive path (`cleanup`) is unchanged in its "cannot tell" answer. One cross-family reviewer (codex, gpt-5.5) read the full diff cold against the issue and reported no findings after checking the `pipefail` interactions, every `(string, bool)` call site, and the fixtures. Follow-up after merge: the active plan `allod/strategy` `dev-plans/concurrent-agent-workspace.md` describes the old helper's behavior and gets one line saying it now fails loudly. ## Validation - `nix build .#checks.x86_64-linux.go-checks`, `flake-status-suite`, `cascade-suites`, `allod-parity`: all passed, rerun by the integrator on the final commit. - `go test ./...` including `-tags site,secret`: ok. `gofmt -l`, `go vet ./...`: clean. - `tests/allod-change.sh`: 222 passed. `tests/workspace/pull-all.sh`: 33 passed (this suite is not wired into a flake check, so it was run directly). `tests/flake/flake-status.sh`: 36 passed. `tests/flake/flake-update-cascade/preflight.sh`: 13 passed. - `shellcheck -s bash -x` over the touched shell files: clean. The one finding without `-s bash`, a missing shebang on the sourced library, predates this change.
defaultRemoteBranch returned "master" whenever origin/HEAD could not be
resolved, inventing a branch nobody asked for. change begin already
asserted symbolic-ref itself first, so it was immune; unpushedCommits
already treated a resolution failure as "unknown" rather than "clean",
so cleanup's guard was already correct. change submit was not immune:
it used the guess as a PR base, so a repo whose default branch is not
master could get a PR opened against a base that does not exist.
secret.go's requireLandingBranch also pre-asserted symbolic-ref, so it
was immune too; its check is left in place for parity with begin, and
its stale comment is updated.

The helper now returns (string, bool) and fails loudly. submit dies
naming the repair (git -C <repo> remote set-head origin -a) instead of
guessing a base; every call site is adapted to the new signature.

Adds a focused test for the new failure contract, plus shell-suite
tests for submit's refusal and record's now-explicit "unknown" path
through unpushedCommits (both already covered the "no origin remote"
case but not the unresolved-origin/HEAD case this issue is about).

Refs allod/tools#126
This tool carries its own Go copy of workspace_repo_default_branch,
guessing 'master' the same way the shared helper used to. Since the
cascade pushes lock commits directly to a repository's default branch
(or opens a PR against it), a guessed base is not a cosmetic problem:
preflight's "not on default branch" check and the protected-branches
match both depend on knowing the real one.

defaultBranch now returns (string, bool) and fails rather than
guessing. Preflight treats a resolution failure as a pre-flight error
naming the repair (git -C <repo> remote set-head origin -a), which
aborts the whole run before anything is touched, the same way every
other pre-flight error does. workspacePins, which also resolves other
repositories' default branches while building the pin graph, treats an
unresolved target as not-on-default rather than propagating the
failure itself: the target's own preflight pass reaches it separately
and aborts the run there if it matters (a repo with no flake.lock,
where it never could, is the only case this silently differs).

Refs allod/tools#126
The Bash helper returned "master" or "" depending on whether the
caller had set pipefail, for the same unresolvable repo: the || bound
to the pipeline (git | sed), not to git, so the pipeline's status was
sed's under a plain shell and git's under pipefail. Every real caller
sets pipefail, so the live behavior was always the silent guess.

The helper now prints the resolved branch and exits 0, or prints
nothing and exits non-zero, regardless of pipefail. Each caller
handles failure explicitly:

- pull-all --switch skips the repo, naming the repair (git -C <repo>
  remote set-head origin -a), instead of comparing the current branch
  against a guess and possibly trying to check out a branch that does
  not exist. Resolution only runs under --switch now, since plain
  pull-all never needed the default branch at all.
- flake-status shows the repo's health as "[default branch unknown —
  run: ...]" instead of comparing against a guess, which could either
  hide a real "wrong branch" warning or invent one.

Refs allod/tools#126
defaultRemoteBranch and workspace_repo_default_branch fail loudly and
name the same repair their callers used to pre-check for by hand. The
separate symbolic-ref assertions in change begin and the secret
landing check duplicated that failure with an identical message, so
this drops them and lets the helper's own failure path carry the
refusal.

Also trims the doc comments on defaultRemoteBranch and
workspace_repo_default_branch to their contract: what a caller gets on
failure and that it must name the repair. The prose narrating what
changed and why belongs in the commit that changed it, not in the
comment.

Refs allod/tools#126
Author
Member

Owner request before merge: the added source and test comments are too verbose. Documentation moves to the READMEs and docs, not the source; remaining comments shrink to at most one line each, contract or trap only, no issue numbers or history.

Owner request before merge: the added source and test comments are too verbose. Documentation moves to the READMEs and docs, not the source; remaining comments shrink to at most one line each, contract or trap only, no issue numbers or history.
The comments added while implementing the default-branch fallback fix
described what changed and cited the issue, which belongs in the
commit and PR rather than the source. Cut each to at most one line
stating a contract or a trap; deleted several test-function doc
comments outright since the test name already says what they say.

Refs allod/tools#126
vnprc force-pushed agent/default-branch-resolve from 4967dd24d9 to cbd1fea458 2026-09-27 04:45:28 +01:00 Compare
vnprc merged commit cbd1fea458 into master 2026-09-27 04:45:37 +01:00
vnprc deleted branch agent/default-branch-resolve 2026-09-27 04:45:38 +01:00
Sign in to join this conversation.
No description provided.