From e5c00332cce34e1c87c5e5b3235d4b73bd61b841 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 30 Aug 2026 23:03:47 -0700 Subject: [PATCH] fix: report portable-store reset recovery failures Return the reset error after a dirty merge so operators see the failure that actually prevented recovery. Cover both recovery branches and preserve the portable runner's sanitized error classification. Adapted from the diagnostic correction in #164; the initial clone already has an operation-wide deadline on current main. Co-authored-by: Sebastien Tardif --- CHANGELOG.md | 2 + docs/portable-stores.md | 3 ++ internal/cli/app.go | 2 +- internal/cli/portable_safety_unix_test.go | 51 +++++++++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c776c70..6f7970a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## 0.9.4 - Unreleased +- Report the actual reset failure when portable-store initialization cannot recover from a dirty merge. Thanks @SebTardif. + ## 0.9.3 - 2026-08-29 - Fix repeated gzip-only portable initialization and add a locked, bounded, preservation-first `portable refresh` subscriber command with automatic Git maintenance disabled across portable operations. diff --git a/docs/portable-stores.md b/docs/portable-stores.md index 207b6d2..0f44983 100644 --- a/docs/portable-stores.md +++ b/docs/portable-stores.md @@ -59,6 +59,9 @@ Initialization validates portable arguments before invoking Git and validates the artifact before saving configuration. Repeated initialization and a publisher's raw-to-gzip transition do not require a raw `.db` in the checkout. Use `init` for setup; it still regenerates configuration on success. +If a dirty merge triggers legacy reset recovery and that reset fails, `init` +reports the reset failure with the same credential-safe Git diagnostics used +elsewhere, so the error identifies the recovery step that needs attention. ## Routine subscriber refresh diff --git a/internal/cli/app.go b/internal/cli/app.go index 45a6c9c..1bf148d 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -3756,7 +3756,7 @@ func syncPortableStore(ctx context.Context, remoteURL, dir string) (string, erro return "", err } if resetErr := runGit(ctx, "", "-C", dir, "reset", "--hard", "HEAD"); resetErr != nil { - return "", err + return "", resetErr } if retryErr := fastForwardGitCheckout(ctx, dir, false); retryErr != nil { return "", retryErr diff --git a/internal/cli/portable_safety_unix_test.go b/internal/cli/portable_safety_unix_test.go index e4af9c4..6c28e46 100644 --- a/internal/cli/portable_safety_unix_test.go +++ b/internal/cli/portable_safety_unix_test.go @@ -6,6 +6,7 @@ import ( "bytes" "context" "errors" + "fmt" "io" "os" "os/exec" @@ -135,6 +136,56 @@ func TestPortableGitFailureDiagnostics(t *testing.T) { } } +func TestSyncPortableStoreReturnsResetFailure(t *testing.T) { + for _, dirtyBeforePull := range []bool{false, true} { + t.Run(fmt.Sprintf("dirty-before-pull=%t", dirtyBeforePull), func(t *testing.T) { + fixture := newPortableRefreshFixture(t, false) + if err := os.WriteFile(filepath.Join(fixture.remote, "incoming.txt"), []byte("remote data\n"), 0o600); err != nil { + t.Fatal(err) + } + portableTestCommit(t, fixture.remote) + localPath := filepath.Join(fixture.checkout, "incoming.txt") + if dirtyBeforePull { + localPath = filepath.Join(fixture.checkout, fixture.relative) + } + if err := os.WriteFile(localPath, []byte("local data\n"), 0o600); err != nil { + t.Fatal(err) + } + if clean := gitWorktreeClean(context.Background(), fixture.checkout); clean == dirtyBeforePull { + t.Fatalf("unexpected tracked worktree cleanliness: %t", clean) + } + + realGit, err := exec.LookPath("git") + if err != nil { + t.Fatal(err) + } + wrapper := filepath.Join(t.TempDir(), "git") + script := `#!/bin/sh +for arg in "$@"; do + if [ "$arg" = reset ]; then + echo 'synthetic-private-path: No space left on device' >&2 + exit 79 + fi +done +exec "$GITCRAWL_TEST_REAL_GIT" "$@" +` + if err := os.WriteFile(wrapper, []byte(script), 0o700); err != nil { + t.Fatal(err) + } + t.Setenv("GITCRAWL_TEST_REAL_GIT", realGit) + t.Setenv("GITCRAWL_PORTABLE_GIT", wrapper) + _, err = syncPortableStore(context.Background(), fixture.remote, fixture.checkout) + var exit *exec.ExitError + if !errors.As(err, &exit) || exit.ExitCode() != 79 || !strings.Contains(err.Error(), "insufficient disk space for Git") { + t.Fatalf("expected reset failure with exit 79 and disk-space guidance, got %v", err) + } + if strings.Contains(err.Error(), "synthetic-private-path") || isDirtyPortablePullError(err) { + t.Fatalf("reset error exposed raw diagnostics or retained the earlier merge failure: %v", err) + } + }) + } +} + func TestPortableGitCancellationAllowsOwnedCleanup(t *testing.T) { dir := t.TempDir() owned := filepath.Join(dir, "tmp_pack_owned")