From f5c61a40ac9fd7d40142939f96b7f8bd37fa32aa Mon Sep 17 00:00:00 2001
From: Yushan Lin <yushan@uber.com>
Date: Thu, 23 Jul 2026 15:51:01 -0700
Subject: [PATCH 1/7] feat(git): classify git command timeouts as infra errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
Intent:
- Git commands (checkout, fetch, clone, diff, apply patch, commit,
  submodule update) already run under a fixed timeout, but a timeout
  failure was indistinguishable from any other git error, so it always
  surfaced as an unclassified error at the top-level orchestrator.

Changes:
- Added a sentinel `git.ErrTimeout` and a `wrapError` helper in
  `core/git` that wraps a failing git operation's error with its name,
  and additionally wraps ErrTimeout when the failure was caused by the
  command's own timeout (as opposed to a parent context cancellation).
- Added `orchestrator.classifyGitError`, used at the checkout,
  apply-requests, and treehash-compute call sites in
  `native_orchestrator.go`, which classifies the error as
  `tangoerrors.NewInfra` when it wraps `git.ErrTimeout`, per the
  classification conventions in `docs/errors/errors.md`.

---

<sub>Generated by the 🪄 [pr-create](https://sg.uberinternal.com/code.uber.internal/uber-code/devexp-agent-marketplace/-/blob/claude-code/plugins/dev/uber-dev/skills/pr-create/SKILL.md) skill in devexp-agent-marketplace</sub>
---
 core/git/git.go                     | 32 +++++++++++++++++------
 core/git/git_test.go                | 39 +++++++++++++++++++++++++++++
 orchestrator/errors.go              | 11 ++++++++
 orchestrator/errors_test.go         | 33 ++++++++++++++++++++++++
 orchestrator/native_orchestrator.go |  6 ++---
 5 files changed, 111 insertions(+), 10 deletions(-)

diff --git a/core/git/git.go b/core/git/git.go
index 043f8a37..9480f6d1 100644
--- a/core/git/git.go
+++ b/core/git/git.go
@@ -36,6 +36,10 @@ const (
 	_gitTimeout = 10 * time.Minute
 )
 
+// ErrTimeout is returned (wrapped) when a git command does not complete
+// within the configured timeout. Callers can check for it with errors.Is.
+var ErrTimeout = errors.New("git command timed out")
+
 // DiffEntry represents a single file change from git diff --name-status.
 type DiffEntry struct {
 	// Status is the single-character status code: "A", "D", "M", "R", etc.
@@ -79,12 +83,25 @@ func New(directory string, logger *zap.SugaredLogger) Interface {
 	}
 }
 
+// wrapError wraps a non-nil err with the name of the failing git operation,
+// additionally wrapping ErrTimeout if ctx's own timeout (rather than a
+// parent cancellation) has elapsed.
+func wrapError(ctx context.Context, op string, err error) error {
+	if err == nil {
+		return nil
+	}
+	if ctx.Err() == context.DeadlineExceeded {
+		return fmt.Errorf("%s: %w: %w", op, ErrTimeout, err)
+	}
+	return fmt.Errorf("%s: %w", op, err)
+}
+
 // Checkout checks out a specific reference in the repository.
 func (c *impl) Checkout(ctx context.Context, ref string, options ...string) error {
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := append([]string{"checkout", ref}, options...)
-	return c.runner.run(ctx, c.directory, "git", args...)
+	return wrapError(ctx, "checkout", c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // Fetch runs git fetch for a remote ref.
@@ -92,7 +109,7 @@ func (c *impl) Fetch(ctx context.Context, remote, ref string, options ...string)
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := append([]string{"fetch", remote, ref}, options...)
-	return c.runner.run(ctx, c.directory, "git", args...)
+	return wrapError(ctx, "fetch", c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // Clone clones the target repository to the destination.
@@ -101,7 +118,7 @@ func (c *impl) Clone(ctx context.Context, target, destination string, options ..
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := append(append([]string{"clone"}, options...), target, destination)
-	return c.runner.run(ctx, c.directory, "git", args...)
+	return wrapError(ctx, "clone", c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // Diff returns the diff between two references.
@@ -109,14 +126,15 @@ func (c *impl) Diff(ctx context.Context, baseRef, targetRef string, options ...s
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := append([]string{"diff", baseRef, targetRef}, options...)
-	return c.runner.output(ctx, c.directory, "git", args...)
+	out, err := c.runner.output(ctx, c.directory, "git", args...)
+	return out, wrapError(ctx, "diff", err)
 }
 
 // ApplyPatch applies a patch to the repository.
 func (c *impl) ApplyPatch(ctx context.Context, patch []byte) error {
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
-	return c.runner.runWithStdin(ctx, c.directory, "git", patch, "apply", "--3way", "--whitespace", "nowarn", "--index", "-")
+	return wrapError(ctx, "apply patch", c.runner.runWithStdin(ctx, c.directory, "git", patch, "apply", "--3way", "--whitespace", "nowarn", "--index", "-"))
 }
 
 // RevParse returns the revision hash of a reference.
@@ -148,7 +166,7 @@ func (c *impl) Commit(ctx context.Context, message string, options ...string) er
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := append([]string{"commit", "-am", message}, options...)
-	return c.runner.run(ctx, c.directory, "git", args...)
+	return wrapError(ctx, "commit", c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // SubmoduleUpdate updates the submodules in the repository.
@@ -156,7 +174,7 @@ func (c *impl) SubmoduleUpdate(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := []string{"submodule", "update", "--init", "--recursive"}
-	return c.runner.run(ctx, c.directory, "git", args...)
+	return wrapError(ctx, "submodule update", c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // DiffWithStatus returns the list of changed files with their status between two refs,
diff --git a/core/git/git_test.go b/core/git/git_test.go
index 5514593d..68891835 100644
--- a/core/git/git_test.go
+++ b/core/git/git_test.go
@@ -19,6 +19,7 @@ import (
 	"errors"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -52,6 +53,44 @@ func (m *mockRunner) runWithStdin(_ context.Context, dir string, name string, st
 	return m.err
 }
 
+// blockingRunner blocks run/output until ctx is done, then returns ctx.Err().
+type blockingRunner struct{}
+
+func (blockingRunner) run(ctx context.Context, _ string, _ string, _ ...string) error {
+	<-ctx.Done()
+	return ctx.Err()
+}
+func (blockingRunner) output(ctx context.Context, _ string, _ string, _ ...string) ([]byte, error) {
+	<-ctx.Done()
+	return nil, ctx.Err()
+}
+func (blockingRunner) runWithStdin(ctx context.Context, _ string, _ string, _ []byte, _ ...string) error {
+	<-ctx.Done()
+	return ctx.Err()
+}
+
+func TestCheckout_wrapsErrTimeoutWhenTimeoutElapses(t *testing.T) {
+	// A parent deadline already in the past causes the derived timeout
+	// context to be immediately Done with context.DeadlineExceeded.
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Hour))
+	defer cancel()
+	g := &impl{directory: "/repo", runner: blockingRunner{}}
+	err := g.Checkout(ctx, "feature")
+	require.Error(t, err)
+	assert.ErrorIs(t, err, ErrTimeout)
+	assert.ErrorIs(t, err, context.DeadlineExceeded)
+}
+
+func TestCheckout_doesNotWrapErrTimeoutOnParentCancellation(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	g := &impl{directory: "/repo", runner: blockingRunner{}}
+	err := g.Checkout(ctx, "feature")
+	require.Error(t, err)
+	assert.NotErrorIs(t, err, ErrTimeout)
+	assert.ErrorIs(t, err, context.Canceled)
+}
+
 func TestClone_usesRunnerWithDirAndArgs(t *testing.T) {
 	m := &mockRunner{}
 	g := &impl{directory: "/repo", runner: m}
diff --git a/orchestrator/errors.go b/orchestrator/errors.go
index 1a422946..a0033a0b 100644
--- a/orchestrator/errors.go
+++ b/orchestrator/errors.go
@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	tangoerrors "github.com/uber/tango/core/errors"
+	"github.com/uber/tango/core/git"
 	"github.com/uber/tango/core/repomanager"
 )
 
@@ -29,3 +30,13 @@ func classifyLeaseError(err error) error {
 	}
 	return tangoerrors.NewInfra(wrappedErr)
 }
+
+// classifyGitError wraps err with the given message and classifies it as an
+// infra error when it was caused by a git command timing out.
+func classifyGitError(msg string, err error) error {
+	wrappedErr := fmt.Errorf("%s: %w", msg, err)
+	if errors.Is(err, git.ErrTimeout) {
+		return tangoerrors.NewInfra(wrappedErr)
+	}
+	return wrappedErr
+}
diff --git a/orchestrator/errors_test.go b/orchestrator/errors_test.go
index ea8881f5..c38bcb9e 100644
--- a/orchestrator/errors_test.go
+++ b/orchestrator/errors_test.go
@@ -22,9 +22,42 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	tangoerrors "github.com/uber/tango/core/errors"
+	"github.com/uber/tango/core/git"
 	"github.com/uber/tango/core/repomanager"
 )
 
+func TestClassifyGitError(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		err      error
+		wantCode tangoerrors.ErrorCode
+	}{
+		{
+			name:     "git timeout is infra",
+			err:      fmt.Errorf("%w: %w", git.ErrTimeout, context.DeadlineExceeded),
+			wantCode: tangoerrors.ErrorInfra,
+		},
+		{
+			name:     "generic error is unclassified but wrapped",
+			err:      fmt.Errorf("exit status 128"),
+			wantCode: tangoerrors.ErrorInfra,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := classifyGitError("checkout main@abc123", tt.err)
+			require.Error(t, got)
+			assert.Equal(t, tt.wantCode, tangoerrors.GetErrorCode(got))
+			assert.ErrorIs(t, got, tt.err)
+		})
+	}
+}
+
 func TestClassifyLeaseError(t *testing.T) {
 	t.Parallel()
 
diff --git a/orchestrator/native_orchestrator.go b/orchestrator/native_orchestrator.go
index 26d744c7..f47d3b7b 100644
--- a/orchestrator/native_orchestrator.go
+++ b/orchestrator/native_orchestrator.go
@@ -138,7 +138,7 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT
 	err = ws.Checkout(ctx, build.Remote, build.BaseSha)
 	recordStep(e, "checkout_duration", checkoutStart)
 	if err != nil {
-		return nil, fmt.Errorf("checkout %s@%s: %w", build.Remote, build.BaseSha, err)
+		return nil, classifyGitError(fmt.Sprintf("checkout %s@%s", build.Remote, build.BaseSha), err)
 	}
 	logger.Infow("GetTargetGraph: Checked out base revision")
 
@@ -160,14 +160,14 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT
 	err = ws.ApplyRequests(ctx, requests)
 	recordStep(e, "apply_requests_duration", applyStart)
 	if err != nil {
-		return nil, fmt.Errorf("apply requests: %w", err)
+		return nil, classifyGitError("apply requests", err)
 	}
 	logger.Infow("GetTargetGraph: Applied requests", zap.Int("request_count", len(requests)))
 
 	// Compute the treehash and download the target graph from storage if exists.
 	treehash, err := gitModule.RevParse(ctx, "HEAD^{tree}")
 	if err != nil {
-		return nil, fmt.Errorf("compute treehash: %w", err)
+		return nil, classifyGitError("compute treehash", err)
 	}
 	treehashPath := cachekey.GetGraphByTreeHash(build.Remote, treehash, build.Strategy, req.ExcludeFilesRegex)
 	if !req.BypassCache {

From 015cda5f028ff055bc84623b60616abec423aae7 Mon Sep 17 00:00:00 2001
From: Yushan Lin <yushan@uber.com>
Date: Thu, 23 Jul 2026 16:12:02 -0700
Subject: [PATCH 2/7] test(git): drop blockingRunner in favor of
 mockRunner.returnCtxErr
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

blockingRunner never actually needed to block — the derived timeout
context is already Done before the runner is invoked in these tests,
since the parent's deadline/cancellation already elapsed. Reuse the
existing mockRunner with a returnCtxErr flag instead of a second
runner type.
---
 core/git/git_test.go | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/core/git/git_test.go b/core/git/git_test.go
index 68891835..0284f098 100644
--- a/core/git/git_test.go
+++ b/core/git/git_test.go
@@ -38,43 +38,42 @@ type mockRunner struct {
 	calls []runnerCall
 	out   []byte
 	err   error
+	// returnCtxErr, if true, makes run/output/runWithStdin return ctx.Err()
+	// instead of err — simulating what an exec-based runner returns when
+	// its command is killed by a canceled/expired context.
+	returnCtxErr bool
 }
 
-func (m *mockRunner) run(_ context.Context, dir string, name string, args ...string) error {
+func (m *mockRunner) run(ctx context.Context, dir string, name string, args ...string) error {
 	m.calls = append(m.calls, runnerCall{kind: "run", dir: dir, name: name, args: append([]string(nil), args...)})
+	if m.returnCtxErr {
+		return ctx.Err()
+	}
 	return m.err
 }
-func (m *mockRunner) output(_ context.Context, dir string, name string, args ...string) ([]byte, error) {
+func (m *mockRunner) output(ctx context.Context, dir string, name string, args ...string) ([]byte, error) {
 	m.calls = append(m.calls, runnerCall{kind: "output", dir: dir, name: name, args: append([]string(nil), args...)})
+	if m.returnCtxErr {
+		return nil, ctx.Err()
+	}
 	return append([]byte(nil), m.out...), m.err
 }
-func (m *mockRunner) runWithStdin(_ context.Context, dir string, name string, stdin []byte, args ...string) error {
+func (m *mockRunner) runWithStdin(ctx context.Context, dir string, name string, stdin []byte, args ...string) error {
 	m.calls = append(m.calls, runnerCall{kind: "runWithStdin", dir: dir, name: name, args: append([]string(nil), args...), stdin: append([]byte(nil), stdin...)})
+	if m.returnCtxErr {
+		return ctx.Err()
+	}
 	return m.err
 }
 
-// blockingRunner blocks run/output until ctx is done, then returns ctx.Err().
-type blockingRunner struct{}
-
-func (blockingRunner) run(ctx context.Context, _ string, _ string, _ ...string) error {
-	<-ctx.Done()
-	return ctx.Err()
-}
-func (blockingRunner) output(ctx context.Context, _ string, _ string, _ ...string) ([]byte, error) {
-	<-ctx.Done()
-	return nil, ctx.Err()
-}
-func (blockingRunner) runWithStdin(ctx context.Context, _ string, _ string, _ []byte, _ ...string) error {
-	<-ctx.Done()
-	return ctx.Err()
-}
-
 func TestCheckout_wrapsErrTimeoutWhenTimeoutElapses(t *testing.T) {
 	// A parent deadline already in the past causes the derived timeout
-	// context to be immediately Done with context.DeadlineExceeded.
+	// context to be immediately Done with context.DeadlineExceeded, before
+	// the runner is even invoked.
 	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Hour))
 	defer cancel()
-	g := &impl{directory: "/repo", runner: blockingRunner{}}
+	m := &mockRunner{returnCtxErr: true}
+	g := &impl{directory: "/repo", runner: m}
 	err := g.Checkout(ctx, "feature")
 	require.Error(t, err)
 	assert.ErrorIs(t, err, ErrTimeout)
@@ -84,7 +83,8 @@ func TestCheckout_wrapsErrTimeoutWhenTimeoutElapses(t *testing.T) {
 func TestCheckout_doesNotWrapErrTimeoutOnParentCancellation(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
-	g := &impl{directory: "/repo", runner: blockingRunner{}}
+	m := &mockRunner{returnCtxErr: true}
+	g := &impl{directory: "/repo", runner: m}
 	err := g.Checkout(ctx, "feature")
 	require.Error(t, err)
 	assert.NotErrorIs(t, err, ErrTimeout)

From a292c46de8150a76e081877d01bb9900d36ad595 Mon Sep 17 00:00:00 2001
From: Yushan Lin <yushan@uber.com>
Date: Thu, 23 Jul 2026 16:16:25 -0700
Subject: [PATCH 3/7] fix(git): prefix wrapped git operation errors with "git "

---
 core/git/git.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/core/git/git.go b/core/git/git.go
index 9480f6d1..070a7f50 100644
--- a/core/git/git.go
+++ b/core/git/git.go
@@ -91,9 +91,9 @@ func wrapError(ctx context.Context, op string, err error) error {
 		return nil
 	}
 	if ctx.Err() == context.DeadlineExceeded {
-		return fmt.Errorf("%s: %w: %w", op, ErrTimeout, err)
+		return fmt.Errorf("git %s: %w: %w", op, ErrTimeout, err)
 	}
-	return fmt.Errorf("%s: %w", op, err)
+	return fmt.Errorf("git %s: %w", op, err)
 }
 
 // Checkout checks out a specific reference in the repository.

From 236688971c3bf1475c604f0562fabf0e28c20dcc Mon Sep 17 00:00:00 2001
From: Yushan Lin <yushan@uber.com>
Date: Thu, 23 Jul 2026 16:31:27 -0700
Subject: [PATCH 4/7] refactor(orchestrator): drop message wrapping from
 classifyGitError

core/git already wraps failures with the operation name, so
classifyGitError only needs to classify, not wrap again.
---
 orchestrator/errors.go              | 11 +++++------
 orchestrator/errors_test.go         |  2 +-
 orchestrator/native_orchestrator.go |  6 +++---
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/orchestrator/errors.go b/orchestrator/errors.go
index a0033a0b..61e8e459 100644
--- a/orchestrator/errors.go
+++ b/orchestrator/errors.go
@@ -31,12 +31,11 @@ func classifyLeaseError(err error) error {
 	return tangoerrors.NewInfra(wrappedErr)
 }
 
-// classifyGitError wraps err with the given message and classifies it as an
-// infra error when it was caused by a git command timing out.
-func classifyGitError(msg string, err error) error {
-	wrappedErr := fmt.Errorf("%s: %w", msg, err)
+// classifyGitError classifies err as an infra error when it was caused by a
+// git command timing out.
+func classifyGitError(err error) error {
 	if errors.Is(err, git.ErrTimeout) {
-		return tangoerrors.NewInfra(wrappedErr)
+		return tangoerrors.NewInfra(err)
 	}
-	return wrappedErr
+	return err
 }
diff --git a/orchestrator/errors_test.go b/orchestrator/errors_test.go
index c38bcb9e..ce65c4f0 100644
--- a/orchestrator/errors_test.go
+++ b/orchestrator/errors_test.go
@@ -50,7 +50,7 @@ func TestClassifyGitError(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			got := classifyGitError("checkout main@abc123", tt.err)
+			got := classifyGitError(tt.err)
 			require.Error(t, got)
 			assert.Equal(t, tt.wantCode, tangoerrors.GetErrorCode(got))
 			assert.ErrorIs(t, got, tt.err)
diff --git a/orchestrator/native_orchestrator.go b/orchestrator/native_orchestrator.go
index f47d3b7b..0d454454 100644
--- a/orchestrator/native_orchestrator.go
+++ b/orchestrator/native_orchestrator.go
@@ -138,7 +138,7 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT
 	err = ws.Checkout(ctx, build.Remote, build.BaseSha)
 	recordStep(e, "checkout_duration", checkoutStart)
 	if err != nil {
-		return nil, classifyGitError(fmt.Sprintf("checkout %s@%s", build.Remote, build.BaseSha), err)
+		return nil, classifyGitError(err)
 	}
 	logger.Infow("GetTargetGraph: Checked out base revision")
 
@@ -160,14 +160,14 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT
 	err = ws.ApplyRequests(ctx, requests)
 	recordStep(e, "apply_requests_duration", applyStart)
 	if err != nil {
-		return nil, classifyGitError("apply requests", err)
+		return nil, classifyGitError(err)
 	}
 	logger.Infow("GetTargetGraph: Applied requests", zap.Int("request_count", len(requests)))
 
 	// Compute the treehash and download the target graph from storage if exists.
 	treehash, err := gitModule.RevParse(ctx, "HEAD^{tree}")
 	if err != nil {
-		return nil, classifyGitError("compute treehash", err)
+		return nil, classifyGitError(err)
 	}
 	treehashPath := cachekey.GetGraphByTreeHash(build.Remote, treehash, build.Strategy, req.ExcludeFilesRegex)
 	if !req.BypassCache {

From 90cc5c9b8fe31e5d5dd2858e334fa9b9da6bc7de Mon Sep 17 00:00:00 2001
From: Yushan Lin <yushan@uber.com>
Date: Thu, 23 Jul 2026 16:42:19 -0700
Subject: [PATCH 5/7] docs(git): add TODO to classify more git failure modes as
 sentinel errors

---
 core/git/git.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/core/git/git.go b/core/git/git.go
index 070a7f50..dde48bd6 100644
--- a/core/git/git.go
+++ b/core/git/git.go
@@ -38,6 +38,10 @@ const (
 
 // ErrTimeout is returned (wrapped) when a git command does not complete
 // within the configured timeout. Callers can check for it with errors.Is.
+//
+// TODO: classify other git failure modes (e.g. exit codes, network errors,
+// auth failures) as sentinel errors here too, so callers can distinguish
+// them without inspecting raw exec output.
 var ErrTimeout = errors.New("git command timed out")
 
 // DiffEntry represents a single file change from git diff --name-status.

From 5e1df9dc7175df62c20d4b06c969a77ac2d11ba6 Mon Sep 17 00:00:00 2001
From: Yushan Lin <yushan@uber.com>
Date: Thu, 23 Jul 2026 20:07:45 -0700
Subject: [PATCH 6/7] fix(orchestrator): classify all git failures as infra,
 including timeouts

Unconditionally wrap git-command failures with NewInfra instead of only
timeouts. context.Canceled is still correctly reported as ErrorCancelled
downstream since GetErrorCode checks for it ahead of the TangoError code,
and context.DeadlineExceeded correctly falls through to ErrorInfra.
---
 orchestrator/errors.go      | 11 ++++-------
 orchestrator/errors_test.go | 10 ++++++++++
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/orchestrator/errors.go b/orchestrator/errors.go
index 61e8e459..1612880d 100644
--- a/orchestrator/errors.go
+++ b/orchestrator/errors.go
@@ -19,7 +19,6 @@ import (
 	"fmt"
 
 	tangoerrors "github.com/uber/tango/core/errors"
-	"github.com/uber/tango/core/git"
 	"github.com/uber/tango/core/repomanager"
 )
 
@@ -31,11 +30,9 @@ func classifyLeaseError(err error) error {
 	return tangoerrors.NewInfra(wrappedErr)
 }
 
-// classifyGitError classifies err as an infra error when it was caused by a
-// git command timing out.
+// classifyGitError classifies a git-command failure as an infra error: git
+// failures (including timeouts) are always infrastructure issues, not
+// caused by user input.
 func classifyGitError(err error) error {
-	if errors.Is(err, git.ErrTimeout) {
-		return tangoerrors.NewInfra(err)
-	}
-	return err
+	return tangoerrors.NewInfra(err)
 }
diff --git a/orchestrator/errors_test.go b/orchestrator/errors_test.go
index ce65c4f0..ffbd4856 100644
--- a/orchestrator/errors_test.go
+++ b/orchestrator/errors_test.go
@@ -44,6 +44,16 @@ func TestClassifyGitError(t *testing.T) {
 			err:      fmt.Errorf("exit status 128"),
 			wantCode: tangoerrors.ErrorInfra,
 		},
+		{
+			name:     "context cancellation is reported as cancelled despite infra wrapping",
+			err:      fmt.Errorf("checkout: %w", context.Canceled),
+			wantCode: tangoerrors.ErrorCancelled,
+		},
+		{
+			name:     "context deadline exceeded is infra",
+			err:      fmt.Errorf("checkout: %w", context.DeadlineExceeded),
+			wantCode: tangoerrors.ErrorInfra,
+		},
 	}
 
 	for _, tt := range tests {

From 224e64b639135335fccbadb27a65b745cf3f1fc3 Mon Sep 17 00:00:00 2001
From: Yushan Lin <yushan@uber.com>
Date: Fri, 24 Jul 2026 09:12:13 -0700
Subject: [PATCH 7/7] fix(git): include command args in wrapped errors, add
 ErrFatal sentinel

wrapError now includes the failing command's actual arguments (e.g. the
ref/remote being acted on) instead of just the operation name, and wraps
ErrFatal when a command exits with a fatal (128) or usage (129) exit
code, as opposed to a non-fatal, conditional exit code such as 1.

classifyGitError explicitly classifies git.ErrTimeout and git.ErrFatal
as infra; other git failures are returned unclassified, falling
through to core/errors' default infra classification.
---
 core/git/git.go             | 48 ++++++++++++++++++++++++-------------
 core/git/git_test.go        | 29 ++++++++++++++++++++++
 orchestrator/errors.go      | 13 ++++++----
 orchestrator/errors_test.go | 23 +++++++++++-------
 4 files changed, 83 insertions(+), 30 deletions(-)

diff --git a/core/git/git.go b/core/git/git.go
index dde48bd6..610b2972 100644
--- a/core/git/git.go
+++ b/core/git/git.go
@@ -36,12 +36,15 @@ const (
 	_gitTimeout = 10 * time.Minute
 )
 
+// ErrFatal is returned (wrapped) when a git command exits with a fatal (128)
+// or usage (129) exit code -- e.g. an unreachable remote, failed
+// authentication, an invalid repository, or a malformed invocation -- as
+// opposed to a non-fatal, conditional exit code such as 1. Callers can check
+// for it with errors.Is.
+var ErrFatal = errors.New("git command failed fatally")
+
 // ErrTimeout is returned (wrapped) when a git command does not complete
 // within the configured timeout. Callers can check for it with errors.Is.
-//
-// TODO: classify other git failure modes (e.g. exit codes, network errors,
-// auth failures) as sentinel errors here too, so callers can distinguish
-// them without inspecting raw exec output.
 var ErrTimeout = errors.New("git command timed out")
 
 // DiffEntry represents a single file change from git diff --name-status.
@@ -87,17 +90,27 @@ func New(directory string, logger *zap.SugaredLogger) Interface {
 	}
 }
 
-// wrapError wraps a non-nil err with the name of the failing git operation,
-// additionally wrapping ErrTimeout if ctx's own timeout (rather than a
-// parent cancellation) has elapsed.
-func wrapError(ctx context.Context, op string, err error) error {
+// wrapError wraps a non-nil err with the failing git command's arguments,
+// additionally wrapping ErrFatal if the command exited with a fatal (128) or
+// usage (129) exit code, as opposed to a non-fatal, conditional exit code
+// such as 1, and ErrTimeout if ctx's own timeout (rather than a parent
+// cancellation) has elapsed.
+func wrapError(ctx context.Context, args []string, err error) error {
 	if err == nil {
 		return nil
 	}
+	var exitErr *exec.ExitError
+	if errors.As(err, &exitErr) && isFatalExitCode(exitErr.ExitCode()) {
+		err = fmt.Errorf("%w: %w", ErrFatal, err)
+	}
 	if ctx.Err() == context.DeadlineExceeded {
-		return fmt.Errorf("git %s: %w: %w", op, ErrTimeout, err)
+		err = fmt.Errorf("%w: %w", ErrTimeout, err)
 	}
-	return fmt.Errorf("git %s: %w", op, err)
+	return fmt.Errorf("git %s: %w", strings.Join(args, " "), err)
+}
+
+func isFatalExitCode(code int) bool {
+	return code == 128 || code == 129
 }
 
 // Checkout checks out a specific reference in the repository.
@@ -105,7 +118,7 @@ func (c *impl) Checkout(ctx context.Context, ref string, options ...string) erro
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := append([]string{"checkout", ref}, options...)
-	return wrapError(ctx, "checkout", c.runner.run(ctx, c.directory, "git", args...))
+	return wrapError(ctx, args, c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // Fetch runs git fetch for a remote ref.
@@ -113,7 +126,7 @@ func (c *impl) Fetch(ctx context.Context, remote, ref string, options ...string)
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := append([]string{"fetch", remote, ref}, options...)
-	return wrapError(ctx, "fetch", c.runner.run(ctx, c.directory, "git", args...))
+	return wrapError(ctx, args, c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // Clone clones the target repository to the destination.
@@ -122,7 +135,7 @@ func (c *impl) Clone(ctx context.Context, target, destination string, options ..
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := append(append([]string{"clone"}, options...), target, destination)
-	return wrapError(ctx, "clone", c.runner.run(ctx, c.directory, "git", args...))
+	return wrapError(ctx, args, c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // Diff returns the diff between two references.
@@ -131,14 +144,15 @@ func (c *impl) Diff(ctx context.Context, baseRef, targetRef string, options ...s
 	defer cancel()
 	args := append([]string{"diff", baseRef, targetRef}, options...)
 	out, err := c.runner.output(ctx, c.directory, "git", args...)
-	return out, wrapError(ctx, "diff", err)
+	return out, wrapError(ctx, args, err)
 }
 
 // ApplyPatch applies a patch to the repository.
 func (c *impl) ApplyPatch(ctx context.Context, patch []byte) error {
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
-	return wrapError(ctx, "apply patch", c.runner.runWithStdin(ctx, c.directory, "git", patch, "apply", "--3way", "--whitespace", "nowarn", "--index", "-"))
+	args := []string{"apply", "--3way", "--whitespace", "nowarn", "--index", "-"}
+	return wrapError(ctx, args, c.runner.runWithStdin(ctx, c.directory, "git", patch, args...))
 }
 
 // RevParse returns the revision hash of a reference.
@@ -170,7 +184,7 @@ func (c *impl) Commit(ctx context.Context, message string, options ...string) er
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := append([]string{"commit", "-am", message}, options...)
-	return wrapError(ctx, "commit", c.runner.run(ctx, c.directory, "git", args...))
+	return wrapError(ctx, args, c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // SubmoduleUpdate updates the submodules in the repository.
@@ -178,7 +192,7 @@ func (c *impl) SubmoduleUpdate(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
 	defer cancel()
 	args := []string{"submodule", "update", "--init", "--recursive"}
-	return wrapError(ctx, "submodule update", c.runner.run(ctx, c.directory, "git", args...))
+	return wrapError(ctx, args, c.runner.run(ctx, c.directory, "git", args...))
 }
 
 // DiffWithStatus returns the list of changed files with their status between two refs,
diff --git a/core/git/git_test.go b/core/git/git_test.go
index 0284f098..b1f87bff 100644
--- a/core/git/git_test.go
+++ b/core/git/git_test.go
@@ -17,6 +17,8 @@ package git
 import (
 	"context"
 	"errors"
+	"fmt"
+	"os/exec"
 	"reflect"
 	"testing"
 	"time"
@@ -91,6 +93,33 @@ func TestCheckout_doesNotWrapErrTimeoutOnParentCancellation(t *testing.T) {
 	assert.ErrorIs(t, err, context.Canceled)
 }
 
+func TestWrapError_marksFatalExitCodesAsErrFatal(t *testing.T) {
+	tests := []struct {
+		name      string
+		exitCode  int
+		wantFatal bool
+	}{
+		{name: "fatal error exit code", exitCode: 128, wantFatal: true},
+		{name: "usage error exit code", exitCode: 129, wantFatal: true},
+		{name: "generic/conditional exit code", exitCode: 1, wantFatal: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			runErr := exec.Command("sh", "-c", fmt.Sprintf("exit %d", tt.exitCode)).Run()
+			require.Error(t, runErr)
+
+			err := wrapError(context.Background(), []string{"checkout", "feature"}, runErr)
+			require.Error(t, err)
+			if tt.wantFatal {
+				assert.ErrorIs(t, err, ErrFatal)
+			} else {
+				assert.NotErrorIs(t, err, ErrFatal)
+			}
+		})
+	}
+}
+
 func TestClone_usesRunnerWithDirAndArgs(t *testing.T) {
 	m := &mockRunner{}
 	g := &impl{directory: "/repo", runner: m}
diff --git a/orchestrator/errors.go b/orchestrator/errors.go
index 1612880d..df2eee23 100644
--- a/orchestrator/errors.go
+++ b/orchestrator/errors.go
@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	tangoerrors "github.com/uber/tango/core/errors"
+	"github.com/uber/tango/core/git"
 	"github.com/uber/tango/core/repomanager"
 )
 
@@ -30,9 +31,13 @@ func classifyLeaseError(err error) error {
 	return tangoerrors.NewInfra(wrappedErr)
 }
 
-// classifyGitError classifies a git-command failure as an infra error: git
-// failures (including timeouts) are always infrastructure issues, not
-// caused by user input.
+// classifyGitError classifies a git-command failure as an infra error when
+// it was caused by a git command timing out (git.ErrTimeout) or exiting
+// with a fatal or usage error (git.ErrFatal). Other git failures are
+// returned unclassified.
 func classifyGitError(err error) error {
-	return tangoerrors.NewInfra(err)
+	if errors.Is(err, git.ErrTimeout) || errors.Is(err, git.ErrFatal) {
+		return tangoerrors.NewInfra(err)
+	}
+	return err
 }
diff --git a/orchestrator/errors_test.go b/orchestrator/errors_test.go
index ffbd4856..29ef6539 100644
--- a/orchestrator/errors_test.go
+++ b/orchestrator/errors_test.go
@@ -35,25 +35,30 @@ func TestClassifyGitError(t *testing.T) {
 		wantCode tangoerrors.ErrorCode
 	}{
 		{
-			name:     "git timeout is infra",
-			err:      fmt.Errorf("%w: %w", git.ErrTimeout, context.DeadlineExceeded),
-			wantCode: tangoerrors.ErrorInfra,
-		},
-		{
-			name:     "generic error is unclassified but wrapped",
-			err:      fmt.Errorf("exit status 128"),
+			name:     "generic error is unclassified but defaults to infra",
+			err:      fmt.Errorf("exit status 1"),
 			wantCode: tangoerrors.ErrorInfra,
 		},
 		{
-			name:     "context cancellation is reported as cancelled despite infra wrapping",
+			name:     "context cancellation is reported as cancelled despite unclassified wrapping",
 			err:      fmt.Errorf("checkout: %w", context.Canceled),
 			wantCode: tangoerrors.ErrorCancelled,
 		},
 		{
-			name:     "context deadline exceeded is infra",
+			name:     "bare context deadline exceeded is unclassified but defaults to infra",
 			err:      fmt.Errorf("checkout: %w", context.DeadlineExceeded),
 			wantCode: tangoerrors.ErrorInfra,
 		},
+		{
+			name:     "git timeout is infra",
+			err:      fmt.Errorf("checkout: %w", git.ErrTimeout),
+			wantCode: tangoerrors.ErrorInfra,
+		},
+		{
+			name:     "fatal git exit code is infra",
+			err:      fmt.Errorf("checkout: %w", git.ErrFatal),
+			wantCode: tangoerrors.ErrorInfra,
+		},
 	}
 
 	for _, tt := range tests {