fix: report why a git command failed, and stop blaming an unpushed branch for it ai:gpt-5.6-sol #83

Manually merged

heiko merged 2 commits from fix/ls-remote-error-reporting into master

2026-08-18 11:05:32 +02:00

heiko commented

2026-08-18 09:32:54 +02:00

Owner

Two defects found while diagnosing a gogogo pack failure in another repository. The run aborted with:

gogogo: repo: compare HEAD against remote: git ls-remote origin: exit status 128 (push your branch, or build the local checkout with --ignore-remote (still uploads) or --no-network (uploads nothing))

Neither half of that message was usable. The branch was pushed, and the actual cause was a transient transport failure — which the message managed to both hide and misattribute. Diagnosis cost a repository audit (gitignore rules, symlinks, payload selection) before it became clear that nothing had been packed at all: the run died in the pre-flight check.

1. `exit status 128` hid git's explanation

Cmd.Output() captures standard error into exec.ExitError.Stderr, but exec.ExitError.Error() prints only the status, so the reason was dropped on the way to the operator. Exit 128 is git's generic fatal — it cannot distinguish an unreachable host from a bad URL, stale credentials, or a server outage.

Cmd.Output now returns the previously unused internal/exec.ExitError wrapper, whose Error() appends the captured text. Fixing it at that one point rather than at the call site covers all ten sites in internal/git alone, plus every other Output() caller, with no call-site churn. Unwrap keeps errors.As/errors.Is working for callers that inspect the exit status.

Before and after, same repository, same broken remote:

✗ repo: compare HEAD against remote: git ls-remote origin: exit status 128

✗ repo: compare HEAD against remote: git ls-remote origin: exit status 128: fatal: unable to access 'https://git.invalid.example/heiko/cert-proxy.git/': Could not resolve host: git.invalid.example

Three details worth reviewing:

Credentials are redacted first. Git echoes the remote URL it failed on, and that URL may carry a password. The regexp already doing this in internal/build moves to internal/exec as RedactCredentials, so there is one implementation instead of two rather than a parallel abstraction.
Output is capped at 4096 runes, so a runaway command cannot turn one diagnostic into a screenful.
A silent failure still reads exactly exit status 1, with no dangling colon.

2. The hint blamed the branch for a network problem

checkRepoState fails for two unrelated reasons — the remote answered and does not carry HEAD, or the remote could not be queried at all — and both got "push your branch, …". For a transport failure that is wrong: the branch may already be pushed, and pushing cannot fix DNS, credentials or an outage. It is what sent me looking at branch state and then at packaging for a network fault.

The unpushed case becomes the sentinel ErrHeadNotOnRemote; repoHint selects from it:

✗ repo: HEAD is not the tip of a branch or tag on the remote
  hint: push your branch, or build the local checkout with --ignore-remote (still uploads) or --no-network (uploads nothing)

✗ repo: compare HEAD against remote: git ls-remote origin: exit status 128: fatal: ...
  hint: the remote could not be queried: check network, credentials and the remote URL, or build the local checkout with --ignore-remote (still uploads) or --no-network (uploads nothing)

Both keep the --ignore-remote / --no-network escape hatches, which apply either way. internal/cmd/release already separates these two cases (release.go:304 vs :309); only doctor conflated them.

Tests

Regression tests at each layer, all hermetic — no network, no live remote:

internal/exec: stderr reaches the message, errors.As still finds the exec.ExitError and its exit code, silent failures stay unchanged, credentials are redacted, floods are truncated, and the detail survives the fmt.Errorf("…: %w") wrapping every caller applies.
internal/git: RemoteContainsCommit against a non-repository path reports git's explanation, not just a status.
internal/cmd/doctor: hint selection for both causes, including a wrapped sentinel.

Verification

go build ./..., go test ./..., go vet ./... all pass. golangci-lint run --new-from-rev origin/master reports 0 issues; the 18 wsl_v5 findings on a full run are pre-existing on master and untouched. Both commits build and test standing alone.

Behaviour verified end-to-end by rebuilding the binary and reproducing the original failure, both for an unreachable remote and for a genuinely unpushed branch.

(authored by ai:gpt-5.6-sol)

Two defects found while diagnosing a `gogogo pack` failure in another repository. The run aborted with: ``` gogogo: repo: compare HEAD against remote: git ls-remote origin: exit status 128 (push your branch, or build the local checkout with --ignore-remote (still uploads) or --no-network (uploads nothing)) ``` Neither half of that message was usable. The branch *was* pushed, and the actual cause was a transient transport failure — which the message managed to both hide and misattribute. Diagnosis cost a repository audit (gitignore rules, symlinks, payload selection) before it became clear that nothing had been packed at all: the run died in the pre-flight check. ## 1. `exit status 128` hid git's explanation `Cmd.Output()` captures standard error into `exec.ExitError.Stderr`, but `exec.ExitError.Error()` prints only the status, so the reason was dropped on the way to the operator. Exit 128 is git's generic fatal — it cannot distinguish an unreachable host from a bad URL, stale credentials, or a server outage. `Cmd.Output` now returns the previously **unused** `internal/exec.ExitError` wrapper, whose `Error()` appends the captured text. Fixing it at that one point rather than at the call site covers all ten sites in `internal/git` alone, plus every other `Output()` caller, with no call-site churn. `Unwrap` keeps `errors.As`/`errors.Is` working for callers that inspect the exit status. Before and after, same repository, same broken remote: ``` ✗ repo: compare HEAD against remote: git ls-remote origin: exit status 128 ✗ repo: compare HEAD against remote: git ls-remote origin: exit status 128: fatal: unable to access 'https://git.invalid.example/heiko/cert-proxy.git/': Could not resolve host: git.invalid.example ``` Three details worth reviewing: * **Credentials are redacted first.** Git echoes the remote URL it failed on, and that URL may carry a password. The regexp already doing this in `internal/build` moves to `internal/exec` as `RedactCredentials`, so there is one implementation instead of two rather than a parallel abstraction. * **Output is capped** at 4096 runes, so a runaway command cannot turn one diagnostic into a screenful. * **A silent failure still reads exactly `exit status 1`**, with no dangling colon. ## 2. The hint blamed the branch for a network problem `checkRepoState` fails for two unrelated reasons — the remote answered and does not carry HEAD, or the remote could not be queried at all — and both got *"push your branch, …"*. For a transport failure that is wrong: the branch may already be pushed, and pushing cannot fix DNS, credentials or an outage. It is what sent me looking at branch state and then at packaging for a network fault. The unpushed case becomes the sentinel `ErrHeadNotOnRemote`; `repoHint` selects from it: ``` ✗ repo: HEAD is not the tip of a branch or tag on the remote hint: push your branch, or build the local checkout with --ignore-remote (still uploads) or --no-network (uploads nothing) ✗ repo: compare HEAD against remote: git ls-remote origin: exit status 128: fatal: ... hint: the remote could not be queried: check network, credentials and the remote URL, or build the local checkout with --ignore-remote (still uploads) or --no-network (uploads nothing) ``` Both keep the `--ignore-remote` / `--no-network` escape hatches, which apply either way. `internal/cmd/release` already separates these two cases (`release.go:304` vs `:309`); only `doctor` conflated them. ## Tests Regression tests at each layer, all hermetic — no network, no live remote: * `internal/exec`: stderr reaches the message, `errors.As` still finds the `exec.ExitError` and its exit code, silent failures stay unchanged, credentials are redacted, floods are truncated, and the detail survives the `fmt.Errorf("…: %w")` wrapping every caller applies. * `internal/git`: `RemoteContainsCommit` against a non-repository path reports git's explanation, not just a status. * `internal/cmd/doctor`: hint selection for both causes, including a wrapped sentinel. ## Verification `go build ./...`, `go test ./...`, `go vet ./...` all pass. `golangci-lint run --new-from-rev origin/master` reports **0 issues**; the 18 `wsl_v5` findings on a full run are pre-existing on `master` and untouched. Both commits build and test standing alone. Behaviour verified end-to-end by rebuilding the binary and reproducing the original failure, both for an unreachable remote and for a genuinely unpushed branch. *(authored by ai:gpt-5.6-sol)*

heiko added 2 commits

2026-08-18 09:32:54 +02:00

fix(exec): report what a failed command said, not just its status ai:gpt-5.6-sol 2c94d6ebdc

A pack run aborted with "git ls-remote origin: exit status 128". Exit 128 is
git's generic fatal, and the reason -- "fatal: unable to access ...: Could
not resolve host" -- was captured by Output() into exec.ExitError.Stderr and
then dropped, because exec.ExitError.Error() prints only the status. The
operator could not tell an unreachable remote from a broken URL, bad
credentials, or a server outage.

Cmd.Output now returns the previously unused internal ExitError wrapper,
whose Error() appends the captured standard error. Fixing it there rather
than at the call site covers all ten sites in internal/git alone, plus every
other Output() caller, and keeps errors.As/errors.Is working via Unwrap.

The captured text is credential-redacted first: git echoes the remote URL it
failed on, and that URL may carry a password. The regexp doing this moves
from internal/build to internal/exec as RedactCredentials, so there is one
implementation rather than two.

Output is capped at 4096 runes so a runaway command cannot turn one
diagnostic into a screenful, and a silent failure still reads exactly
"exit status 1" with no dangling colon.

fix(doctor): stop blaming an unpushed branch for an unreachable remote ai:gpt-5.6-sol c66d446b7d

checkRepoState fails for two unrelated reasons: the remote answered and does
not carry HEAD, or the remote could not be queried at all. Both got the same
advice, "push your branch, or build the local checkout with --ignore-remote
... or --no-network ...".

For a transport failure that advice is wrong and actively misleading: the
branch may well be pushed already, and pushing cannot fix DNS, credentials
or a server outage. It sends the reader looking at branch state, then at
packaging, for a network problem.

The unpushed case becomes the sentinel ErrHeadNotOnRemote, and repoHint
selects advice from it. The remote-unreachable case now points at network,
credentials and the remote URL. Both keep the --ignore-remote and
--no-network escape hatches, which apply either way.

internal/cmd/release already distinguishes the two; only doctor conflated
them.

heiko referenced this pull request

2026-08-18 09:42:02 +02:00

fix(source): copy symlinks as symlinks, not through them ai:gpt-5.6-sol #84

heiko referenced this pull request

2026-08-18 09:55:38 +02:00

fix(release): copy before signing in publish mode too ai:gpt-5.6-sol #85

heiko force-pushed fix/ls-remote-error-reporting from c66d446b7d to d90671185a

2026-08-18 11:03:27 +02:00

Compare

heiko referenced this pull request from a commit

2026-08-18 11:05:32 +02:00

Merge branch 'fix/ls-remote-error-reporting' into master ai:claude-opus-5