workspace_repo_default_branch: resolve the default branch or fail, never guess 'master' #126

New issue

Open

opened 2026-07-28 03:14:31 +01:00 by allod-agent · 1 comment

allod-agent commented

2026-07-28 03:14:31 +01:00

Member

Make workspace_repo_default_branch either return a branch it actually resolved or fail loudly, instead of returning a value that depends on whether the caller happened to set pipefail.

Primary goals:

One answer, not two — the helper currently returns master under set -o pipefail and the empty string without it, for the same repo in the same state. No consumer can be written correctly against that.
No silent guess — where the fallback fires it invents a default branch nobody asked for, which is the silent fallback architecture.md principle 11 forbids.
Let each consumer decide — six callsites across four tools consume this, and a display tool and a mutating tool want different things from "cannot resolve".

Current state

lib/workspace.sh:69-74:

workspace_repo_default_branch() {
  local dir="$1"
  git -C "$dir" symbolic-ref refs/remotes/origin/HEAD 2>/dev/null \
    | sed 's|refs/remotes/origin/||' \
    || echo "master"
}

The || binds to the pipeline, not to git. Without pipefail the pipeline's status is sed's, which is always 0, so the fallback never fires and an unresolvable origin/HEAD yields the empty string. With pipefail the status is git's, the fallback does fire, and the same repo yields master. Both branches are reachable and both are silent; verified against git 2.51.2 by sourcing the file with and without set -o pipefail.

Every current consumer sets set -euo pipefail, so the live behaviour today is the master guess, not the empty return: allod:198-200 (wrapping it as default_remote_branch_for_dir), workspace/pull-all:55, flake/flake-status:148, and flake/flake-update-cascade:189, :202, :273, :291.

An earlier reading of this, recorded in allod/strategy dev-plans/concurrent-agent-workspace.md under C5 and repeated in dev-plans/allod-change-always-isolate.md, concluded the fallback is dead and the helper returns empty. That is the no-pipefail half only. Both documents should be corrected when this lands.

Reachability, stated plainly

Narrow, and worth saying so rather than overselling it: git 2.51.2 sets refs/remotes/origin/HEAD on clone and repairs it on any fetch, including a refspec-limited one, so a checkout in normal use has it. Verified. The unresolvable state persists in a repo built by git init plus remote add plus push with no fetch since, in one whose remote has no resolvable HEAD, and transiently after git remote set-head origin -d until the next fetch.

That bounds the severity but not the defect: a helper whose return value depends on the caller's shell options is wrong regardless of how often the branch is taken, and the two places where a wrong answer costs something are worth naming.

What a wrong answer costs

change submit defaults its PR base to this helper (allod:438), so a repo whose default branch is not master gets a PR opened against a base that may not exist.

has_unpushed_commits (allod:216) uses it as its last-resort base, and answers return 1 — "no unpushed commits" — when that base does not resolve. That is the guard standing in front of the destructive change cleanup, and a "cannot tell" reported as "nothing to lose" is a fail-open in the one place the project cannot afford one. Whether that return 1 should become a refusal is a separate question from this helper's contract, and it is now issue allod/tools#127.

pull-all --switch (workspace/pull-all:55) compares the current branch against the guess and, on a mismatch, tries to switch to a branch that may not exist. flake-status:148 emits a false "[on branch main, not master]" warning. flake-update-cascade:189 matches the guess against a protected-branches line, so a miss drops the repo out of skip:protected — the subsequent branch check at :202 catches it with a confusing message rather than letting a direct push through, so this one is a legibility failure, not a fail-open.

Scope

In: lib/workspace.sh:69-74 and the six callsites above, plus their tests under tests/workspace/ and tests/flake/.

The default: have the helper print the resolved branch and exit non-zero with no output when origin/HEAD does not resolve, then handle that explicitly per consumer — the mutating tools (pull-all --switch, flake-update-cascade) skip the repo naming git -C <repo> remote set-head origin -a as the repair, and the reporting tools (flake-status) show the repo with the default branch marked unknown rather than inventing one. Keeping a fallback but making it deterministic is the cheaper alternative and is worse: it preserves the guess and only fixes the inconsistency.

Out: allod change begin, which as of PR allod/tools#125 asserts symbolic-ref refs/remotes/origin/HEAD itself before consuming the helper and so is already immune; that assertion becomes redundant once this lands and can be dropped then. Also out: the has_unpushed_commits fail-open described above, which is issue allod/tools#127.

Found while implementing allod/tools#116.

Make `workspace_repo_default_branch` either return a branch it actually resolved or fail loudly, instead of returning a value that depends on whether the caller happened to set `pipefail`. Primary goals: - **One answer, not two** — the helper currently returns `master` under `set -o pipefail` and the empty string without it, for the same repo in the same state. No consumer can be written correctly against that. - **No silent guess** — where the fallback fires it invents a default branch nobody asked for, which is the silent fallback `architecture.md` principle 11 forbids. - **Let each consumer decide** — six callsites across four tools consume this, and a display tool and a mutating tool want different things from "cannot resolve". ### Current state `lib/workspace.sh:69-74`: workspace_repo_default_branch() { local dir="$1" git -C "$dir" symbolic-ref refs/remotes/origin/HEAD 2>/dev/null \ | sed 's|refs/remotes/origin/||' \ || echo "master" } The `||` binds to the pipeline, not to `git`. Without `pipefail` the pipeline's status is `sed`'s, which is always 0, so the fallback never fires and an unresolvable `origin/HEAD` yields the empty string. With `pipefail` the status is git's, the fallback does fire, and the same repo yields `master`. Both branches are reachable and both are silent; verified against git 2.51.2 by sourcing the file with and without `set -o pipefail`. Every current consumer sets `set -euo pipefail`, so the live behaviour today is the `master` guess, not the empty return: `allod:198-200` (wrapping it as `default_remote_branch_for_dir`), `workspace/pull-all:55`, `flake/flake-status:148`, and `flake/flake-update-cascade:189`, `:202`, `:273`, `:291`. An earlier reading of this, recorded in `allod/strategy` `dev-plans/concurrent-agent-workspace.md` under C5 and repeated in `dev-plans/allod-change-always-isolate.md`, concluded the fallback is dead and the helper returns empty. That is the no-`pipefail` half only. Both documents should be corrected when this lands. ### Reachability, stated plainly Narrow, and worth saying so rather than overselling it: git 2.51.2 sets `refs/remotes/origin/HEAD` on `clone` and repairs it on any `fetch`, including a refspec-limited one, so a checkout in normal use has it. Verified. The unresolvable state persists in a repo built by `git init` plus `remote add` plus `push` with no fetch since, in one whose remote has no resolvable HEAD, and transiently after `git remote set-head origin -d` until the next fetch. That bounds the severity but not the defect: a helper whose return value depends on the caller's shell options is wrong regardless of how often the branch is taken, and the two places where a wrong answer costs something are worth naming. ### What a wrong answer costs `change submit` defaults its PR base to this helper (`allod:438`), so a repo whose default branch is not `master` gets a PR opened against a base that may not exist. `has_unpushed_commits` (`allod:216`) uses it as its last-resort base, and answers `return 1` — "no unpushed commits" — when that base does not resolve. That is the guard standing in front of the destructive `change cleanup`, and a "cannot tell" reported as "nothing to lose" is a fail-open in the one place the project cannot afford one. Whether that `return 1` should become a refusal is a separate question from this helper's contract, and it is now issue allod/tools#127. `pull-all --switch` (`workspace/pull-all:55`) compares the current branch against the guess and, on a mismatch, tries to switch to a branch that may not exist. `flake-status:148` emits a false "[on branch main, not master]" warning. `flake-update-cascade:189` matches the guess against a `protected-branches` line, so a miss drops the repo out of `skip:protected` — the subsequent branch check at `:202` catches it with a confusing message rather than letting a direct push through, so this one is a legibility failure, not a fail-open. ### Scope In: `lib/workspace.sh:69-74` and the six callsites above, plus their tests under `tests/workspace/` and `tests/flake/`. The default: have the helper print the resolved branch and exit non-zero with no output when `origin/HEAD` does not resolve, then handle that explicitly per consumer — the mutating tools (`pull-all --switch`, `flake-update-cascade`) skip the repo naming `git -C <repo> remote set-head origin -a` as the repair, and the reporting tools (`flake-status`) show the repo with the default branch marked unknown rather than inventing one. Keeping a fallback but making it deterministic is the cheaper alternative and is worse: it preserves the guess and only fixes the inconsistency. Out: `allod change begin`, which as of PR allod/tools#125 asserts `symbolic-ref refs/remotes/origin/HEAD` itself before consuming the helper and so is already immune; that assertion becomes redundant once this lands and can be dropped then. Also out: the `has_unpushed_commits` fail-open described above, which is issue allod/tools#127. Found while implementing allod/tools#116.

allod-agent referenced this issue

2026-07-28 03:14:47 +01:00

change: always isolate a -d change in its own worktree, and add 'change list' #125

allod-agent referenced this issue

2026-07-28 03:28:53 +01:00

allod change cleanup deletes a branch when has_unpushed_commits cannot tell whether it has unpushed commits #127

allod-agent commented

2026-07-28 03:28:58 +01:00

Author

Member

The fail-open named in the Scope section above is now issue allod/tools#127. Reproduced: with no origin remote the helper hits allod:223, answers "no unpushed commits", and allod change cleanup removes the worktree and deletes the branch, leaving the commit reachable from no ref.

The fail-open named in the Scope section above is now issue allod/tools#127. Reproduced: with no `origin` remote the helper hits `allod:223`, answers "no unpushed commits", and `allod change cleanup` removes the worktree and deletes the branch, leaving the commit reachable from no ref.