From 2a7bed87db98215c71173104a3cfa7f897970f11 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 3 Nov 2026 00:15:36 +0000 Subject: [PATCH] Classify recovery setup failures by their actual cause --- cmd/releasekit/recovery.go | 2 +- cmd/releasekit/recovery_test.go | 94 +++++++++++++++++++++++++++++++++ docs/roadmap/implementation.md | 17 +++++- docs/roadmap/remediation.md | 6 +++ internal/git/remote.go | 19 +++---- internal/publish/reconcile.go | 27 +++++----- recovery.go | 3 ++ 7 files changed, 144 insertions(+), 24 deletions(-) diff --git a/cmd/releasekit/recovery.go b/cmd/releasekit/recovery.go index dc32383..380400a 100644 --- a/cmd/releasekit/recovery.go +++ b/cmd/releasekit/recovery.go @@ -224,7 +224,7 @@ func recoveryGuidance(err error) string { case "endpoint_changed": return "Review Git URL rewrite settings and restore the captured endpoint mapping before retrying." case "transport_configuration": - return "Review the current supported transport settings for the recorded remote, then retry observation." + return "Review Git configuration and the supported transport settings, then retry observation." case "canceled": return "Retry observation with a context that allows it to complete." case "remote_observation", "inconsistent_observation": diff --git a/cmd/releasekit/recovery_test.go b/cmd/releasekit/recovery_test.go index 19dd5d7..467f5c9 100644 --- a/cmd/releasekit/recovery_test.go +++ b/cmd/releasekit/recovery_test.go @@ -7,7 +7,9 @@ import ( "errors" "fmt" "os" + "os/exec" "path/filepath" + "runtime" "strings" "testing" @@ -15,6 +17,98 @@ import ( "go.maximumdirect.net/releasekit/internal/publish" ) +func TestRecoveryEndpointSetupFailureDiagnostics(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fixture injects Git failures through a POSIX launcher") + } + for _, test := range []struct { + name, remote, kind, guidance string + exitCode int + }{ + {"unnamed configuration", "", "transport_configuration", "Git configuration", 128}, + {"named configuration", "origin", "transport_configuration", "Git configuration", 128}, + {"rewrite", "", "endpoint_changed", "URL rewrite", 0}, + {"tooling", "", "remote_observation", "Git diagnostic", 71}, + } { + t.Run(test.name, func(t *testing.T) { + repo := newCLIRepository(t) + evidence := publish.Evidence{RepositoryDir: repo, Endpoint: repo, Tag: "v1.2.3", Remote: test.remote} + path, err := publish.SaveEvidence(t.Context(), repo, t.TempDir(), evidence) + if err != nil { + t.Fatal(err) + } + if test.kind == "transport_configuration" { + file, err := os.OpenFile(filepath.Join(repo, ".git", "config"), os.O_APPEND|os.O_WRONLY, 0600) + if err != nil { + t.Fatal(err) + } + if _, err := file.WriteString("\n[broken\n"); err != nil { + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + } else if test.kind == "endpoint_changed" { + cliGitRun(t, repo, "config", "url."+t.TempDir()+".insteadOf", repo) + } + realGit, err := exec.LookPath("git") + if err != nil { + t.Fatal(err) + } + marker := filepath.Join(t.TempDir(), "queried") + bin := t.TempDir() + failValidation := "" + if test.name == "tooling" { + failValidation = "[ \"$arg\" = check-ref-format ] && exit 71\n" + } + script := "#!/bin/sh\nquery=0\nget_url=0\nfor arg in \"$@\"; do\n" + failValidation + "[ \"$arg\" = ls-remote ] && query=1\n[ \"$arg\" = --get-url ] && get_url=1\ndone\nif [ \"$query\" = 1 ] && [ \"$get_url\" = 0 ]; then : > " + shellQuote(marker) + "; exit 72; fi\nexec " + shellQuote(realGit) + " \"$@\"\n" + if err := os.WriteFile(filepath.Join(bin, "git"), []byte(script), 0700); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + loaded, err := releasekit.LoadPublicationEvidence(path) + if err != nil { + t.Fatal(err) + } + result, err := releasekit.ReconcilePublication(t.Context(), loaded) + var typed *releasekit.PublicationRecoveryError + if !errors.As(err, &typed) || typed.Kind != test.kind || result.State != releasekit.PublicationUnknown { + t.Fatalf("setup classification = %#v, %v", result, err) + } + if test.exitCode != 0 { + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) || exitErr.ExitCode() != test.exitCode { + t.Fatalf("lost Git cause: %v", err) + } + } + for _, structured := range []bool{false, true} { + args := []string{"reconcile", "--record", path} + if structured { + args = append(args, "--json") + } + var stdout, stderr bytes.Buffer + if status := run(t.Context(), args, &stdout, &stderr); status != 3 { + t.Fatalf("setup status=%d: %s%s", status, &stdout, &stderr) + } + if structured { + var document reconciliationDocument + if err := json.Unmarshal(stdout.Bytes(), &document); err != nil { + t.Fatal(err) + } + if document.State != "unknown" || document.Diagnostic == nil || document.Diagnostic.Kind != test.kind { + t.Fatalf("setup JSON: %s", &stdout) + } + } else if !strings.Contains(stderr.String(), test.kind) || !strings.Contains(stderr.String(), test.guidance) { + t.Fatalf("setup guidance: %s", &stderr) + } + } + if _, err := os.Stat(marker); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("setup failure reached remote query: %v", err) + } + }) + } +} + type failingRecoveryWriter struct{ err error } func (writer failingRecoveryWriter) Write([]byte) (int, error) { return 0, writer.err } diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 9943975..d2be02b 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -67,7 +67,12 @@ classification without sharing publication prerequisites. ref handling. Reading transport settings requires no current fetch/push URL equality check. Missing settings, including after a remote is renamed or removed, permit observation; actual configuration-read failures retain their - causes. Reuse the supported transport keys already implemented. + causes. Reuse the supported transport keys already implemented. Mark + configuration reads and scoped-URL configuration resolution at the Git + boundary so failures map to `transport_configuration` independently of the + optional remote name. Positively detected rewrites use `endpoint_changed`; + other operational failures use `remote_observation`, with cancellation taking + its existing precedence and causes retained throughout. 4. Extract classification so the immediate path can supply its already captured endpoint and transport settings. Keep `reconcilePush` as the publication adapter: retain its existing detached, bounded recovery context, phase @@ -122,6 +127,11 @@ current transport settings, repaired settings, removed/renamed remotes, fixed URLs, and configuration-read failure. Preserve the existing lost-acknowledgment, rejected-push, endpoint-change, and cancellation behavior. +**R16 follow-up completed:** Configuration, rewrite, and operational failures +map by their known cause, with facade/CLI coverage for named and unnamed +configuration, positive rewrites, and tooling failures. Setup failure and rewrite +mismatch tests confirm that no remote tag query occurs. + ## Stage 2: Implement private attempt records **Completed:** 2026-11-02 @@ -388,6 +398,9 @@ releasekit publication-attempts [--repo DIR] [--record-dir DIR] [--json] commit evidence when that is what conflicts. Explain possible next actions from known causes, without guessing that a generic Git failure is an authentication or connectivity failure. + Configuration failures include endpoint/global settings when the remote name + is absent. Use the same cause-based recovery kinds from Stage 1 for JSON + diagnostics and human guidance. 4. Human listing shows path, module identity, full tag, candidate commit, and recording time when available. Show per-record diagnostics alongside usable entries and make incomplete discovery clear. Distinguish an enumeration @@ -507,6 +520,8 @@ contracts and a short consumer recovery workflow. alongside cold registered-tool and temporary-launcher recovery. R15 adds canceled publication and preparation-failure handoff cases using explicit push readiness and fresh later invocations. + R16 covers setup failure classifications and human/JSON guidance without + querying an invalid or substituted endpoint. 2. Establish read-only behavior through resulting refs, worktree/branch state, saved records, and controlled observations of candidate/hosted boundaries. Reuse lower-level classification and codec tests. Tests use local remotes, diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index c02db3b..f1ec802 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -694,6 +694,12 @@ no-recovery-step behavior for successful publication. ## R16: Classify endpoint setup failures by their actual cause — P3 +**Completed:** The Git boundary marks configuration-read/resolution failures; +observation maps those, positive rewrites, and other operational errors to the +existing kinds while preserving causes. Named/unnamed malformed configuration, +rewrite, tooling, cancellation, facade, and CLI diagnostic tests passed. Tests +also verify no tag query after setup failure; Stage 1/6/8 guidance is updated. + **Relevant code:** `internal/publish/reconcile.go:116–128` and `internal/git/remote.go`'s endpoint setup. Affects Stages 1 and 6. diff --git a/internal/git/remote.go b/internal/git/remote.go index 75a9619..86c1145 100644 --- a/internal/git/remote.go +++ b/internal/git/remote.go @@ -13,13 +13,14 @@ import ( ) var ( - ErrInvalidRemote = errors.New("invalid Git remote") - ErrMultiplePushURLs = errors.New("Git remote has multiple push URLs") - ErrEndpointChanged = errors.New("Git publication endpoint changed") - ErrRemoteMainMissing = errors.New("authoritative remote has no main branch") - ErrRemoteMainConflict = errors.New("authoritative main does not identify the candidate commit") - ErrRemoteTagConflict = errors.New("tag already exists on the authoritative remote") - ErrRemoteStateUnreadable = errors.New("authoritative remote state is unreadable") + ErrInvalidRemote = errors.New("invalid Git remote") + ErrMultiplePushURLs = errors.New("Git remote has multiple push URLs") + ErrEndpointChanged = errors.New("Git publication endpoint changed") + ErrTransportConfiguration = errors.New("Git transport configuration is unreadable") + ErrRemoteMainMissing = errors.New("authoritative remote has no main branch") + ErrRemoteMainConflict = errors.New("authoritative main does not identify the candidate commit") + ErrRemoteTagConflict = errors.New("tag already exists on the authoritative remote") + ErrRemoteStateUnreadable = errors.New("authoritative remote state is unreadable") ) // Endpoint identifies the single effective publication destination captured @@ -202,7 +203,7 @@ func (endpoint Endpoint) RequireCandidate(ctx context.Context, repositoryDir, ta func (endpoint Endpoint) verifyScopedURLs(ctx context.Context, repositoryDir string) error { readURL, err := run(ctx, repositoryDir, endpoint.scopedConfig(), "ls-remote", "--get-url", endpoint.ScopedName) if err != nil { - return fmt.Errorf("resolve scoped read URL: %w", err) + return fmt.Errorf("%w: resolve scoped read URL: %w", ErrTransportConfiguration, err) } resolved := outputLines(readURL) if len(resolved) != 1 || resolved[0] != endpoint.PushURL { @@ -238,7 +239,7 @@ func transportConfig(ctx context.Context, repositoryDir, remoteName, scopedName if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { continue } - return nil, fmt.Errorf("read transport setting remote.%s.%s: %w", DisplayURL(remoteName), key, err) + return nil, fmt.Errorf("%w: read transport setting remote.%s.%s: %w", ErrTransportConfiguration, DisplayURL(remoteName), key, err) } for _, value := range outputLines(output) { config = append(config, "remote."+scopedName+"."+key+"="+value) diff --git a/internal/publish/reconcile.go b/internal/publish/reconcile.go index 4e8607a..d51b254 100644 --- a/internal/publish/reconcile.go +++ b/internal/publish/reconcile.go @@ -108,28 +108,18 @@ func observe(ctx context.Context, evidence Evidence) (Observation, error) { } valid, err := git.ValidateTagRef(ctx, evidence.RepositoryDir, evidence.Tag) if err != nil { - return Observation{Evidence: evidence, State: Unknown}, recoveryFailure(ctx, "remote_observation", err) + return Observation{Evidence: evidence, State: Unknown}, observationFailure(ctx, err) } if !valid { return Observation{Evidence: evidence, State: Unknown}, &RecoveryError{Kind: "invalid_evidence"} } endpoint, err := git.ObserveEndpoint(ctx, evidence.RepositoryDir, evidence.Endpoint, evidence.Remote) if err != nil { - if ctx.Err() != nil { - return Observation{Evidence: evidence, State: Unknown}, &RecoveryError{Kind: "canceled", cause: ctx.Err()} - } - kind := "endpoint_changed" - if errors.Is(err, git.ErrEndpointChanged) { - return Observation{Evidence: evidence, State: Unknown}, &RecoveryError{Kind: kind, cause: err} - } - if evidence.Remote != "" { - kind = "transport_configuration" - } - return Observation{Evidence: evidence, State: Unknown}, &RecoveryError{Kind: kind, cause: err} + return Observation{Evidence: evidence, State: Unknown}, observationFailure(ctx, err) } ref, err := endpoint.QueryTag(ctx, evidence.RepositoryDir, evidence.Tag) if err != nil { - return Observation{Evidence: evidence, State: Unknown}, recoveryFailure(ctx, "remote_observation", err) + return Observation{Evidence: evidence, State: Unknown}, observationFailure(ctx, err) } result, err := classify(evidence, ref) if err != nil && ctx.Err() != nil { @@ -138,6 +128,17 @@ func observe(ctx context.Context, evidence Evidence) (Observation, error) { return result, err } +func observationFailure(ctx context.Context, err error) *RecoveryError { + kind := "remote_observation" + switch { + case errors.Is(err, git.ErrEndpointChanged): + kind = "endpoint_changed" + case errors.Is(err, git.ErrTransportConfiguration): + kind = "transport_configuration" + } + return recoveryFailure(ctx, kind, err) +} + // Reconcile observes the recorded endpoint and classifies its exact tag. func Reconcile(ctx context.Context, evidence Evidence) (Observation, error) { return observe(ctx, evidence) diff --git a/recovery.go b/recovery.go index 6e63c1b..9f4c41b 100644 --- a/recovery.go +++ b/recovery.go @@ -42,6 +42,9 @@ type PublicationReconciliationResult struct { // Kind includes missing_evidence, invalid_evidence, canceled, endpoint_changed, // transport_configuration, remote_observation, inconsistent_observation, // record_format, record_read, record_write, and storage_enumeration. +// endpoint_changed identifies a confirmed URL-resolution mismatch; +// transport_configuration identifies configuration-read or resolution failures, +// including when the original remote name is absent. type PublicationRecoveryError struct { Kind string RecordPath string // selected or affected record path, when applicable.