From f58a4941c7c814525259cdd6e270e5034a22e6b3 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Tue, 21 Jul 2026 23:57:07 -0300 Subject: [PATCH 1/2] test(parity): retry parity sides on transient docker build flakes The runtime parity lane's docker builds flake on contended CI runners (BuildKit "failed to solve", a registry hiccup, or a container-setup failure surfaced as `Command failed: docker build`). A flake on one side while the other succeeds shows up as a false TS-vs-Go divergence and fails the job under PARITY_STRICT=true, where an inconclusive skip is red too. This turned unrelated dependabot PRs (#9, #10) and a main daily run red on up.dotfiles-local* cases. Wrap each side's CLI invocation in a bounded retry (2 attempts) gated on isRetryableFailure, a predicate deliberately broader than isInfraError. Retrying is safe: a deterministic product failure reproduces on the next attempt and still goes red. The narrow isInfraError/classifyParitySide oracle-usability path is unchanged, so no real divergence is masked as a skip. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/parity_matrix_test.go | 53 ++++++++++++ internal/cli/parity_retry_test.go | 134 +++++++++++++++++++++++++++++ 2 files changed, 187 insertions(+) create mode 100644 internal/cli/parity_retry_test.go diff --git a/internal/cli/parity_matrix_test.go b/internal/cli/parity_matrix_test.go index 8210aaa..72dd6c2 100644 --- a/internal/cli/parity_matrix_test.go +++ b/internal/cli/parity_matrix_test.go @@ -518,7 +518,34 @@ func runParitySide(t *testing.T, ctx context.Context, repoRoot, cli string, tc p return result } +// parityInfraRetries bounds how many times a side is re-run when it fails with a +// transient build/environment error. The runtime lane's docker builds flake on +// contended CI runners (BuildKit "failed to solve", a registry hiccup, or a +// container-setup failure surfaced as `Command failed: docker build`), which then +// shows up as a false TS-vs-Go divergence (one side's build flaked while the other +// succeeded). A single retry absorbs the flake. Retrying is safe: a deterministic +// product failure reproduces on every attempt and still fails the case. +const parityInfraRetries = 2 // total attempts, not extra retries + func runParityCLI(ctx context.Context, repoRoot, cli, cmdArgs string, env map[string]string) (stdout, stderr string, exitCode int) { + return runWithInfraRetry(ctx, parityInfraRetries, func() (string, string, int) { + return runParityCLIOnce(ctx, repoRoot, cli, cmdArgs, env) + }) +} + +// runWithInfraRetry re-runs run while it fails with a retryable transient error, +// up to maxAttempts total. It stops early on success, on a non-retryable failure, +// or once the context is done (so a retry never runs past the per-case deadline). +func runWithInfraRetry(ctx context.Context, maxAttempts int, run func() (stdout, stderr string, exitCode int)) (stdout, stderr string, exitCode int) { + for attempt := 1; ; attempt++ { + stdout, stderr, exitCode = run() + if exitCode == 0 || attempt >= maxAttempts || !isRetryableFailure(stdout, stderr) || ctx.Err() != nil { + return + } + } +} + +func runParityCLIOnce(ctx context.Context, repoRoot, cli, cmdArgs string, env map[string]string) (stdout, stderr string, exitCode int) { cmd := exec.CommandContext(ctx, "/bin/sh", "-lc", cli+" "+cmdArgs) cmd.Dir = repoRoot cmd.Env = append(os.Environ(), envList(env)...) @@ -1211,6 +1238,32 @@ func isInfraError(stdout, stderr string) bool { return false } +// isRetryableFailure reports whether a non-zero CLI result looks like a transient +// build/environment failure worth re-running, as opposed to a stable product +// divergence. It is deliberately BROADER than isInfraError: a retry is safe (a +// deterministic failure reproduces on the next attempt and still goes RED), so it +// also matches the container-setup / `docker build` wrapper a flaky BuildKit or +// registry hiccup surfaces without a recognizable low-level signal. Do NOT use it +// to mark a side an unusable oracle — classifyParitySide/isInfraError must stay +// narrow so a real divergence is never silently skipped; use this only to decide a +// retry. +func isRetryableFailure(stdout, stderr string) bool { + if isInfraError(stdout, stderr) { + return true + } + combined := strings.ToLower(stdout + stderr) + retryablePatterns := []string{ + "an error occurred setting up the container", + "command failed: docker build", + } + for _, p := range retryablePatterns { + if strings.Contains(combined, p) { + return true + } + } + return false +} + type paritySideStatus struct { Skip bool Timeout bool diff --git a/internal/cli/parity_retry_test.go b/internal/cli/parity_retry_test.go new file mode 100644 index 0000000..d173340 --- /dev/null +++ b/internal/cli/parity_retry_test.go @@ -0,0 +1,134 @@ +package cli + +import ( + "context" + "testing" +) + +// TestIsRetryableFailure guards the classifier that decides whether a non-zero +// parity side is a transient build/environment flake (retryable) or a stable +// product divergence (not). The two positive fixtures are the exact signatures +// that turned dependabot PRs #9/#10 red: a BuildKit "failed to solve" on one +// side and the reference's container-setup wrapper on the other, while the other +// side succeeded. The negative fixtures must stay non-retryable so a real CLI +// contract failure is never masked by a retry. +func TestIsRetryableFailure(t *testing.T) { + cases := []struct { + name string + stdout string + stderr string + wantRetryable bool + }{ + { + name: "buildkit failed to solve (isInfraError)", + stderr: "ERROR: failed to solve: rpc error: code = Unknown desc = failed to fetch", + wantRetryable: true, + }, + { + name: "docker daemon unavailable (isInfraError)", + stderr: "Cannot connect to the Docker daemon at unix:///var/run/docker.sock", + wantRetryable: true, + }, + { + // PR #10 / main 07-14 signature: TS reference build flaked and the + // low-level BuildKit error is hidden behind the generic wrapper. + name: "reference container-setup wrapper", + stdout: `{"description":"An error occurred setting up the container.","message":"Command failed: docker build -f /tmp/x -t vsc-parity-dotfiles --platform linux/amd64 /tmp/x","outcome":"error"}`, + wantRetryable: true, + }, + { + name: "config not found is a stable contract error", + stdout: `{"outcome":"error","message":"Dev container config (path/devcontainer.json) not found."}`, + wantRetryable: false, + }, + { + name: "flag validation is a stable contract error", + stderr: "Unknown argument: --bogus-flag", + wantRetryable: false, + }, + { + name: "empty output is not retryable", + wantRetryable: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := isRetryableFailure(tc.stdout, tc.stderr); got != tc.wantRetryable { + t.Errorf("isRetryableFailure(%q, %q) = %v, want %v", tc.stdout, tc.stderr, got, tc.wantRetryable) + } + }) + } +} + +// TestRunWithInfraRetry asserts the retry loop absorbs a transient build flake +// (success on the second attempt) while never masking a stable failure: a +// deterministic contract error runs exactly once, and a persistent transient +// error is retried up to the cap and then surfaced as the final non-zero result. +func TestRunWithInfraRetry(t *testing.T) { + const transient = "ERROR: failed to solve: connection reset by peer" + const stable = `{"outcome":"error","message":"Dev container config not found."}` + + t.Run("transient failure then success is absorbed", func(t *testing.T) { + calls := 0 + _, _, exit := runWithInfraRetry(context.Background(), parityInfraRetries, func() (string, string, int) { + calls++ + if calls == 1 { + return "", transient, 1 + } + return "ok", "", 0 + }) + if exit != 0 { + t.Errorf("exit = %d, want 0 (flake should be absorbed)", exit) + } + if calls != 2 { + t.Errorf("calls = %d, want 2 (one retry)", calls) + } + }) + + t.Run("stable contract failure is not retried", func(t *testing.T) { + calls := 0 + _, _, exit := runWithInfraRetry(context.Background(), parityInfraRetries, func() (string, string, int) { + calls++ + return stable, "", 1 + }) + if exit != 1 { + t.Errorf("exit = %d, want 1", exit) + } + if calls != 1 { + t.Errorf("calls = %d, want 1 (no retry for a stable contract error)", calls) + } + }) + + t.Run("persistent transient failure surfaces after the cap", func(t *testing.T) { + calls := 0 + _, stderr, exit := runWithInfraRetry(context.Background(), parityInfraRetries, func() (string, string, int) { + calls++ + return "", transient, 1 + }) + if exit != 1 { + t.Errorf("exit = %d, want 1 (real failure must still go red)", exit) + } + if calls != parityInfraRetries { + t.Errorf("calls = %d, want %d (retry cap)", calls, parityInfraRetries) + } + if stderr != transient { + t.Errorf("stderr = %q, want the final attempt's output", stderr) + } + }) + + t.Run("cancelled context stops retries", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + calls := 0 + _, _, exit := runWithInfraRetry(ctx, parityInfraRetries, func() (string, string, int) { + calls++ + return "", transient, 1 + }) + if exit != 1 { + t.Errorf("exit = %d, want 1", exit) + } + if calls != 1 { + t.Errorf("calls = %d, want 1 (no retry once the deadline passed)", calls) + } + }) +} From af81c82a176808f37656479e284049391cdd0f68 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Wed, 22 Jul 2026 01:05:14 -0300 Subject: [PATCH 2/2] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Manuel Alejandro de Brito Fontes --- internal/cli/parity_matrix_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/cli/parity_matrix_test.go b/internal/cli/parity_matrix_test.go index 72dd6c2..7900c14 100644 --- a/internal/cli/parity_matrix_test.go +++ b/internal/cli/parity_matrix_test.go @@ -537,7 +537,15 @@ func runParityCLI(ctx context.Context, repoRoot, cli, cmdArgs string, env map[st // up to maxAttempts total. It stops early on success, on a non-retryable failure, // or once the context is done (so a retry never runs past the per-case deadline). func runWithInfraRetry(ctx context.Context, maxAttempts int, run func() (stdout, stderr string, exitCode int)) (stdout, stderr string, exitCode int) { + if maxAttempts < 1 { + maxAttempts = 1 + } for attempt := 1; ; attempt++ { + // Always allow the first attempt; after that, don't start new work once the + // per-case context is done. + if attempt > 1 && ctx.Err() != nil { + return + } stdout, stderr, exitCode = run() if exitCode == 0 || attempt >= maxAttempts || !isRetryableFailure(stdout, stderr) || ctx.Err() != nil { return