diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 781ce02..06e49f2 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -2,6 +2,15 @@ ## Unreleased +### Typed merge errors + +- Changed stable merge conflict and guard 409 responses to return + `RefUpdateError` in the TypeScript, Python, and Go SDKs. +- Added typed conflict paths, merge base, guard, expected SHA, and actual SHA + fields. Unknown 409 codes and non-409 failures remain API errors. +- This changes merge error behavior and requires a coordinated non-patch SDK + release. + ### Ephemeral merge previews - Added source and target ephemeral namespace flags to merge previews in the TypeScript, Python, and Go SDKs. diff --git a/packages/code-storage-go/README.md b/packages/code-storage-go/README.md index 9273dcf..39b5ee4 100644 --- a/packages/code-storage-go/README.md +++ b/packages/code-storage-go/README.md @@ -300,11 +300,24 @@ result, err := repo.Merge(context.Background(), storage.MergeOptions{ Author: &storage.CommitSignature{Name: "Merge Bot", Email: "merge@example.com"}, }) if err != nil { + if refErr, ok := err.(*storage.RefUpdateError); ok { + switch { + case refErr.Reason == storage.RefUpdateReasonConflict: + fmt.Println(refErr.ConflictPaths, refErr.MergeBaseSHA) + case refErr.Reason == storage.RefUpdateReasonPreconditionFailed && refErr.Guard == storage.MergeGuardTarget: + fmt.Println("Target moved", refErr.ExpectedSHA, refErr.ActualSHA) + case refErr.Reason == storage.RefUpdateReasonPreconditionFailed && refErr.Guard == storage.MergeGuardSource: + fmt.Println("Source moved", refErr.ExpectedSHA, refErr.ActualSHA) + } + } log.Fatal(err) } fmt.Println(result.Result, result.Target.NewSHA) ``` +Known merge 409 responses return `*RefUpdateError`. An unknown merge 409 code +and every non-409 merge failure return `*APIError`. + ### Create a commit ```go diff --git a/packages/code-storage-go/errors.go b/packages/code-storage-go/errors.go index 9e693f1..899ddab 100644 --- a/packages/code-storage-go/errors.go +++ b/packages/code-storage-go/errors.go @@ -1,6 +1,7 @@ package storage import ( + "net/http" "strings" ) @@ -35,12 +36,106 @@ const ( RefUpdateReasonUnknown RefUpdateReason = "unknown" ) +// MergeGuard identifies the ref protected by a failed merge guard. +type MergeGuard string + +const ( + MergeGuardTarget MergeGuard = "target" + MergeGuardSource MergeGuard = "source" +) + // RefUpdateError describes failed ref updates. type RefUpdateError struct { - Message string - Status string - Reason RefUpdateReason - RefUpdate *RefUpdate + Message string + Status string + Reason RefUpdateReason + RefUpdate *RefUpdate + Guard MergeGuard + ExpectedSHA string + ActualSHA string + ConflictPaths []string + MergeBaseSHA string +} + +func parseMergeRefUpdateError(apiErr *APIError) *RefUpdateError { + if apiErr == nil || apiErr.Status != http.StatusConflict { + return nil + } + body, ok := apiErr.Body.(map[string]interface{}) + if !ok { + return nil + } + code, ok := body["code"].(string) + if !ok { + return nil + } + + switch code { + case "merge_conflict": + paths, ok := mergeErrorStringSlice(body, "conflict_paths") + if !ok { + return nil + } + mergeBaseSHA, ok := mergeErrorOptionalString(body, "merge_base_sha") + if !ok { + return nil + } + return &RefUpdateError{ + Message: apiErr.Message, + Status: code, + Reason: RefUpdateReasonConflict, + ConflictPaths: paths, + MergeBaseSHA: mergeBaseSHA, + } + case "precondition_failed": + guard, ok := body["guard"].(string) + if !ok || (guard != string(MergeGuardTarget) && guard != string(MergeGuardSource)) { + return nil + } + expectedSHA, expectedOK := body["expected_sha"].(string) + actualSHA, actualOK := body["actual_sha"].(string) + if !expectedOK || !actualOK { + return nil + } + return &RefUpdateError{ + Message: apiErr.Message, + Status: code, + Reason: RefUpdateReasonPreconditionFailed, + Guard: MergeGuard(guard), + ExpectedSHA: expectedSHA, + ActualSHA: actualSHA, + } + default: + return nil + } +} + +func mergeErrorStringSlice(body map[string]interface{}, key string) ([]string, bool) { + value, exists := body[key] + if !exists { + return []string{}, true + } + items, ok := value.([]interface{}) + if !ok { + return nil, false + } + result := make([]string, len(items)) + for i, item := range items { + result[i], ok = item.(string) + if !ok { + return nil, false + } + } + return result, true +} + +func mergeErrorOptionalString(body map[string]interface{}, key string) (string, bool) { + value, exists := body[key] + if !exists { + return "", true + } + result, ok := value.(string) + return result, ok } func (e *RefUpdateError) Error() string { diff --git a/packages/code-storage-go/repo.go b/packages/code-storage-go/repo.go index c4a49e3..b80994f 100644 --- a/packages/code-storage-go/repo.go +++ b/packages/code-storage-go/repo.go @@ -1300,6 +1300,12 @@ func (r *Repo) Merge(ctx context.Context, options MergeOptions) (MergeResult, er resp, err := r.client.api.post(ctx, r.apiPath("merge"), nil, body, jwtToken, nil) if err != nil { + var apiErr *APIError + if errors.As(err, &apiErr) { + if refUpdateErr := parseMergeRefUpdateError(apiErr); refUpdateErr != nil { + return MergeResult{}, refUpdateErr + } + } return MergeResult{}, err } defer resp.Body.Close() diff --git a/packages/code-storage-go/repo_test.go b/packages/code-storage-go/repo_test.go index 177c4ce..7d47eeb 100644 --- a/packages/code-storage-go/repo_test.go +++ b/packages/code-storage-go/repo_test.go @@ -1145,38 +1145,159 @@ func TestMergeValidation(t *testing.T) { } } -func TestMergeConflictPreservesBody(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/api/repos/repo/merge" { - t.Fatalf("unexpected path: %s", r.URL.Path) - } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusConflict) - _, _ = w.Write([]byte(`{"error":"merge conflict","conflict_paths":["README.md"],"merge_base_sha":"base123"}`)) - })) - defer server.Close() - - client, err := NewClient(Options{Name: "acme", Key: testKey, APIBaseURL: server.URL}) - if err != nil { - t.Fatalf("client error: %v", err) +func TestMergeReturnsTypedRefUpdateErrors(t *testing.T) { + tests := []struct { + name string + body map[string]interface{} + expected RefUpdateError + }{ + { + name: "merge conflict", + body: map[string]interface{}{ + "error": "merge conflict", + "code": "merge_conflict", + "conflict_paths": []string{"README.md"}, + "merge_base_sha": "base123", + }, + expected: RefUpdateError{ + Message: "merge conflict", + Status: "merge_conflict", + Reason: RefUpdateReasonConflict, + ConflictPaths: []string{"README.md"}, + MergeBaseSHA: "base123", + }, + }, + { + name: "stale target guard", + body: map[string]interface{}{ + "error": "target branch moved", + "code": "precondition_failed", + "guard": "target", + "expected_sha": "expected-target", + "actual_sha": "actual-target", + }, + expected: RefUpdateError{ + Message: "target branch moved", + Status: "precondition_failed", + Reason: RefUpdateReasonPreconditionFailed, + Guard: MergeGuardTarget, + ExpectedSHA: "expected-target", + ActualSHA: "actual-target", + }, + }, + { + name: "stale source guard", + body: map[string]interface{}{ + "error": "source ref no longer contains the expected commit", + "code": "precondition_failed", + "guard": "source", + "expected_sha": "expected-source", + "actual_sha": "actual-source", + }, + expected: RefUpdateError{ + Message: "source ref no longer contains the expected commit", + Status: "precondition_failed", + Reason: RefUpdateReasonPreconditionFailed, + Guard: MergeGuardSource, + ExpectedSHA: "expected-source", + ActualSHA: "actual-source", + }, + }, } - repo := &Repo{ID: "repo", DefaultBranch: "main", client: client} - _, err = repo.Merge(nil, MergeOptions{SourceBranch: "feature", TargetBranch: "main", Strategy: MergeStrategyMerge}) - if err == nil { - t.Fatalf("expected conflict error") - } - var apiErr *APIError - if !errors.As(err, &apiErr) { - t.Fatalf("expected APIError, got %T", err) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/repos/repo/merge" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusConflict) + _ = json.NewEncoder(w).Encode(tc.body) + })) + defer server.Close() + + client, err := NewClient(Options{Name: "acme", Key: testKey, APIBaseURL: server.URL}) + if err != nil { + t.Fatalf("client error: %v", err) + } + repo := &Repo{ID: "repo", DefaultBranch: "main", client: client} + + _, err = repo.Merge(nil, MergeOptions{SourceRef: "feature", TargetBranch: "main", Strategy: MergeStrategyMerge}) + var refErr *RefUpdateError + if !errors.As(err, &refErr) { + t.Fatalf("expected RefUpdateError, got %T", err) + } + if !reflect.DeepEqual(*refErr, tc.expected) { + t.Fatalf("unexpected RefUpdateError: %#v", refErr) + } + }) } - body, ok := apiErr.Body.(map[string]interface{}) - if !ok || body["error"] != "merge conflict" || body["merge_base_sha"] != "base123" { - t.Fatalf("unexpected error body: %#v", apiErr.Body) +} + +func TestMergeKeepsOtherAPIErrors(t *testing.T) { + tests := []struct { + name string + statusCode int + body map[string]interface{} + }{ + { + name: "unknown 409 code", + statusCode: http.StatusConflict, + body: map[string]interface{}{ + "error": "merge conflict in README.md", + "code": "future_merge_error", + }, + }, + { + name: "403 response", + statusCode: http.StatusForbidden, + body: map[string]interface{}{ + "error": "merge conflict", + "code": "merge_conflict", + }, + }, + { + name: "500 response", + statusCode: http.StatusInternalServerError, + body: map[string]interface{}{ + "error": "target branch moved", + "code": "precondition_failed", + "guard": "target", + "expected_sha": "expected-target", + "actual_sha": "actual-target", + }, + }, } - paths, ok := body["conflict_paths"].([]interface{}) - if !ok || len(paths) != 1 || paths[0] != "README.md" { - t.Fatalf("unexpected conflict paths: %#v", body["conflict_paths"]) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tc.statusCode) + _ = json.NewEncoder(w).Encode(tc.body) + })) + defer server.Close() + + client, err := NewClient(Options{Name: "acme", Key: testKey, APIBaseURL: server.URL}) + if err != nil { + t.Fatalf("client error: %v", err) + } + repo := &Repo{ID: "repo", DefaultBranch: "main", client: client} + + _, err = repo.Merge(nil, MergeOptions{SourceRef: "feature", TargetBranch: "main", Strategy: MergeStrategyMerge}) + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected APIError, got %T", err) + } + if apiErr.Status != tc.statusCode || !reflect.DeepEqual(apiErr.Body, tc.body) { + t.Fatalf("unexpected APIError: %#v", apiErr) + } + var refErr *RefUpdateError + if errors.As(err, &refErr) { + t.Fatalf("expected APIError only, got RefUpdateError: %#v", refErr) + } + }) } } diff --git a/packages/code-storage-python/README.md b/packages/code-storage-python/README.md index a9ee0ca..1ea8d62 100644 --- a/packages/code-storage-python/README.md +++ b/packages/code-storage-python/README.md @@ -54,7 +54,7 @@ option is still accepted but does not select request routes. ### Basic Setup ```python -from pierre_storage import GitStorage +from pierre_storage import GitStorage, RefUpdateError # Initialize the client with your name and key storage = GitStorage({ @@ -283,21 +283,30 @@ print(preview["status"], preview["result"]) print(preview["conflict_paths"], preview["filtered_conflicts"]) # Merge one branch into another -merge_result = await repo.merge( - source_ref="feature/preview", - source_is_ephemeral=True, # optional; source branch can live in ephemeral namespace - target_branch="main", - target_is_ephemeral=False, # optional; target branch can independently be ephemeral - strategy="merge", # one of: "merge", "ff_only", "ff_prefer" - expected_target_sha="abc123", # optional; 409 if target moved - commit_message="Merge feature/preview", # optional - author={"name": "Bot", "email": "bot@example.com"}, # optional - committer={"name": "Bot", "email": "bot@example.com"}, # optional - allow_unrelated_histories=False, # optional - ttl=900, # optional JWT TTL in seconds -) -print(merge_result["result"], merge_result["commit_sha"]) -print(merge_result["source"]["sha"], merge_result["target"]["new_sha"]) +try: + merge_result = await repo.merge( + source_ref="feature/preview", + source_is_ephemeral=True, # optional; source can be ephemeral + target_branch="main", + target_is_ephemeral=False, # optional; target can be ephemeral + strategy="merge", # "merge", "ff_only", or "ff_prefer" + expected_target_sha="abc123", # optional; 409 if target moved + commit_message="Merge feature/preview", # optional + author={"name": "Bot", "email": "bot@example.com"}, # optional + committer={"name": "Bot", "email": "bot@example.com"}, # optional + allow_unrelated_histories=False, # optional + ttl=900, # optional JWT TTL in seconds + ) +except RefUpdateError as error: + if error.reason == "conflict": + print(error.conflict_paths, error.merge_base_sha) + elif error.reason == "precondition_failed" and error.guard == "target": + print("Target moved", error.expected_sha, error.actual_sha) + elif error.reason == "precondition_failed" and error.guard == "source": + print("Source moved", error.expected_sha, error.actual_sha) +else: + print(merge_result["result"], merge_result["commit_sha"]) + print(merge_result["source"]["sha"], merge_result["target"]["new_sha"]) # Target-tip modes: # - Provide expected_target_sha when target_branch must still point at that commit. # - Omit expected_target_sha to merge into the current target tip. For native @@ -1262,6 +1271,11 @@ except RefUpdateError as e: print(f"Ref update: {e.ref_update}") ``` +Known merge 409 responses also raise `RefUpdateError`. A conflict provides +`conflict_paths` and `merge_base_sha`. A failed guard provides `guard`, +`expected_sha`, and `actual_sha`. An unknown merge 409 code and every non-409 +merge failure remain `ApiError`. + ## Development ### Setup diff --git a/packages/code-storage-python/pierre_storage/errors.py b/packages/code-storage-python/pierre_storage/errors.py index bdd78a8..1d86831 100644 --- a/packages/code-storage-python/pierre_storage/errors.py +++ b/packages/code-storage-python/pierre_storage/errors.py @@ -1,6 +1,6 @@ """Error classes for Pierre Git Storage SDK.""" -from typing import TYPE_CHECKING, Any, Dict, Optional +from typing import TYPE_CHECKING, Any, Dict, List, Literal, Optional if TYPE_CHECKING: from pierre_storage.types import RefUpdate @@ -37,6 +37,11 @@ def __init__( status: Optional[str] = None, reason: Optional[str] = None, ref_update: "Optional[RefUpdate]" = None, + guard: Optional[Literal["target", "source"]] = None, + expected_sha: Optional[str] = None, + actual_sha: Optional[str] = None, + conflict_paths: Optional[List[str]] = None, + merge_base_sha: Optional[str] = None, ) -> None: """Initialize the RefUpdateError. @@ -45,12 +50,67 @@ def __init__( status: Status code from the server reason: Reason for the failure ref_update: Partial ref update information + guard: Failed merge guard + expected_sha: Caller-provided SHA for the failed guard + actual_sha: Authoritative SHA for the failed guard + conflict_paths: Paths that conflicted during a merge + merge_base_sha: Merge base for a merge conflict """ super().__init__(message) self.message = message self.status = status or "unknown" self.reason = reason or self.status self.ref_update: Dict[str, str] = ref_update or {} # type: ignore[assignment] + self.guard = guard + self.expected_sha = expected_sha + self.actual_sha = actual_sha + self.conflict_paths = conflict_paths + self.merge_base_sha = merge_base_sha + + +def parse_merge_ref_update_error( + message: str, status_code: int, body: Any +) -> Optional[RefUpdateError]: + """Parse a stable merge 409 response into a ref update error.""" + if status_code != 409 or not isinstance(body, dict): + return None + + code = body.get("code") + if code == "merge_conflict": + conflict_paths = body.get("conflict_paths", []) + merge_base_sha = body.get("merge_base_sha") + if not isinstance(conflict_paths, list) or not all( + isinstance(path, str) for path in conflict_paths + ): + return None + if merge_base_sha is not None and not isinstance(merge_base_sha, str): + return None + return RefUpdateError( + message, + status=code, + reason="conflict", + conflict_paths=conflict_paths, + merge_base_sha=merge_base_sha, + ) + + if code == "precondition_failed": + guard = body.get("guard") + expected_sha = body.get("expected_sha") + actual_sha = body.get("actual_sha") + if guard not in {"target", "source"}: + return None + if not isinstance(expected_sha, str) or not isinstance(actual_sha, str): + return None + return RefUpdateError( + message, + status=code, + reason="precondition_failed", + guard=guard, + expected_sha=expected_sha, + actual_sha=actual_sha, + ) + + return None def infer_ref_update_reason(status_code: str) -> str: diff --git a/packages/code-storage-python/pierre_storage/repo.py b/packages/code-storage-python/pierre_storage/repo.py index e9218ba..b90de39 100644 --- a/packages/code-storage-python/pierre_storage/repo.py +++ b/packages/code-storage-python/pierre_storage/repo.py @@ -15,7 +15,12 @@ resolve_commit_ttl_seconds, send_diff_commit_request, ) -from pierre_storage.errors import ApiError, RefUpdateError, infer_ref_update_reason +from pierre_storage.errors import ( + ApiError, + RefUpdateError, + infer_ref_update_reason, + parse_merge_ref_update_error, +) from pierre_storage.types import ( BlameLine, BlameResult, @@ -1126,6 +1131,7 @@ async def merge( if response.status_code != 200: message = "Merge failed" + error_data: Any = None try: error_data = response.json() if isinstance(error_data, dict) and error_data.get("message"): @@ -1136,6 +1142,11 @@ async def merge( message = f"{message} with HTTP {response.status_code}" except Exception: message = f"{message} with HTTP {response.status_code}" + ref_update_error = parse_merge_ref_update_error( + message, response.status_code, error_data + ) + if ref_update_error is not None: + raise ref_update_error raise ApiError(message, status_code=response.status_code, response=response) data = response.json() diff --git a/packages/code-storage-python/tests/test_repo.py b/packages/code-storage-python/tests/test_repo.py index 6f6dc08..700583f 100644 --- a/packages/code-storage-python/tests/test_repo.py +++ b/packages/code-storage-python/tests/test_repo.py @@ -1487,8 +1487,64 @@ async def test_merge_omits_expected_target_sha_for_current_target_tip( } @pytest.mark.asyncio - async def test_merge_conflict_keeps_response_body(self, git_storage_options: dict) -> None: - """Merge conflicts should surface the API error and retain the response body.""" + @pytest.mark.parametrize( + ("body", "expected"), + [ + ( + { + "error": "merge conflict", + "code": "merge_conflict", + "conflict_paths": ["README.md"], + "merge_base_sha": "base123", + }, + { + "message": "merge conflict", + "status": "merge_conflict", + "reason": "conflict", + "conflict_paths": ["README.md"], + "merge_base_sha": "base123", + }, + ), + ( + { + "error": "target branch moved", + "code": "precondition_failed", + "guard": "target", + "expected_sha": "expected-target", + "actual_sha": "actual-target", + }, + { + "message": "target branch moved", + "status": "precondition_failed", + "reason": "precondition_failed", + "guard": "target", + "expected_sha": "expected-target", + "actual_sha": "actual-target", + }, + ), + ( + { + "error": "source ref no longer contains the expected commit", + "code": "precondition_failed", + "guard": "source", + "expected_sha": "expected-source", + "actual_sha": "actual-source", + }, + { + "message": "source ref no longer contains the expected commit", + "status": "precondition_failed", + "reason": "precondition_failed", + "guard": "source", + "expected_sha": "expected-source", + "actual_sha": "actual-source", + }, + ), + ], + ) + async def test_merge_returns_typed_ref_update_errors( + self, git_storage_options: dict, body: dict, expected: dict + ) -> None: + """Stable merge 409 codes should become typed ref update errors.""" storage = GitStorage(git_storage_options) create_repo_response = MagicMock() @@ -1496,15 +1552,10 @@ async def test_merge_conflict_keeps_response_body(self, git_storage_options: dic create_repo_response.is_success = True create_repo_response.json.return_value = {"repo_id": "test-repo"} - conflict_body = { - "error": "merge conflict", - "conflict_paths": ["README.md"], - "merge_base_sha": "base123", - } merge_response = MagicMock() merge_response.status_code = 409 merge_response.is_success = False - merge_response.json.return_value = conflict_body + merge_response.json.return_value = body with patch("httpx.AsyncClient") as mock_client: client_instance = mock_client.return_value.__aenter__.return_value @@ -1512,12 +1563,64 @@ async def test_merge_conflict_keeps_response_body(self, git_storage_options: dic repo = await storage.create_repo(id="test-repo") - with pytest.raises(ApiError, match="merge conflict") as exc_info: - await repo.merge(source_branch="feature", target_branch="main", strategy="merge") + with pytest.raises(RefUpdateError) as exc_info: + await repo.merge(source_ref="feature", target_branch="main", strategy="merge") - assert exc_info.value.status_code == 409 + for field, value in expected.items(): + assert getattr(exc_info.value, field) == value + + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("status_code", "body"), + [ + ( + 409, + { + "error": "merge conflict in README.md", + "code": "future_merge_error", + }, + ), + (403, {"error": "merge conflict", "code": "merge_conflict"}), + ( + 500, + { + "error": "target branch moved", + "code": "precondition_failed", + "guard": "target", + "expected_sha": "expected-target", + "actual_sha": "actual-target", + }, + ), + ], + ) + async def test_merge_keeps_other_api_errors( + self, git_storage_options: dict, status_code: int, body: dict + ) -> None: + """Unknown codes and non-409 statuses should remain API errors.""" + storage = GitStorage(git_storage_options) + + create_repo_response = MagicMock() + create_repo_response.status_code = 200 + create_repo_response.is_success = True + create_repo_response.json.return_value = {"repo_id": "test-repo"} + + merge_response = MagicMock() + merge_response.status_code = status_code + merge_response.is_success = False + merge_response.json.return_value = body + + with patch("httpx.AsyncClient") as mock_client: + client_instance = mock_client.return_value.__aenter__.return_value + client_instance.post = AsyncMock(side_effect=[create_repo_response, merge_response]) + + repo = await storage.create_repo(id="test-repo") + + with pytest.raises(ApiError) as exc_info: + await repo.merge(source_ref="feature", target_branch="main", strategy="merge") + + assert type(exc_info.value) is ApiError + assert exc_info.value.status_code == status_code assert exc_info.value.response is merge_response - assert exc_info.value.response.json() == conflict_body @pytest.mark.asyncio async def test_merge_validation(self, git_storage_options: dict) -> None: diff --git a/packages/code-storage-typescript/README.md b/packages/code-storage-typescript/README.md index c860a3e..614eed8 100644 --- a/packages/code-storage-typescript/README.md +++ b/packages/code-storage-typescript/README.md @@ -56,7 +56,7 @@ accepted but does not select request routes. ### Basic Setup ```typescript -import { GitStorage } from '@pierre/storage'; +import { GitStorage, RefUpdateError } from '@pierre/storage'; // Initialize the client with your name and key const store = new GitStorage({ @@ -425,27 +425,39 @@ console.log(preview.conflictPaths, preview.conflicts, preview.filteredConflicts) // Merge one branch into another. Source and target can independently be // ephemeral branches. -const mergeResult = await repo.merge({ - sourceRef: 'feature/demo', - sourceIsEphemeral: true, - targetBranch: 'main', - targetIsEphemeral: false, - expectedTargetSha: '0123456789abcdef0123456789abcdef01234567', // optional; 409 if target moved - strategy: 'merge', // 'merge' | 'ff_only' | 'ff_prefer' - commitMessage: 'Merge feature/demo', // optional - author: { name: 'Merge Bot', email: 'merge@example.com' }, // optional - committer: { name: 'Merge Bot', email: 'merge@example.com' }, // optional - allowUnrelatedHistories: false, // optional - squash: false, // optional; incompatible with ff_only -}); -console.log(mergeResult.result); // 'merge_commit', 'fast_forward', 'no_op', 'squash', or 'unknown' -console.log(mergeResult.commitSha, mergeResult.target.newSha); +try { + const mergeResult = await repo.merge({ + sourceRef: 'feature/demo', + sourceIsEphemeral: true, + targetBranch: 'main', + targetIsEphemeral: false, + expectedTargetSha: '0123456789abcdef0123456789abcdef01234567', // optional; 409 if target moved + strategy: 'merge', // 'merge' | 'ff_only' | 'ff_prefer' + commitMessage: 'Merge feature/demo', // optional + author: { name: 'Merge Bot', email: 'merge@example.com' }, // optional + committer: { name: 'Merge Bot', email: 'merge@example.com' }, // optional + allowUnrelatedHistories: false, // optional + squash: false, // optional; incompatible with ff_only + }); + console.log(mergeResult.result); // 'merge_commit', 'fast_forward', 'no_op', 'squash', or 'unknown' + console.log(mergeResult.commitSha, mergeResult.target.newSha); +} catch (error) { + if (!(error instanceof RefUpdateError)) throw error; + + if (error.reason === 'conflict') { + console.log(error.conflictPaths, error.mergeBaseSha); + } else if (error.reason === 'precondition_failed' && error.guard === 'target') { + console.log('Target moved', error.expectedSha, error.actualSha); + } else if (error.reason === 'precondition_failed' && error.guard === 'source') { + console.log('Source moved', error.expectedSha, error.actualSha); + } +} // repo.merge() requires sourceRef, targetBranch, and strategy. It returns // camelCase metadata for the source tip, target update, merge base (when -// reported), and number of promoted commits. A backend conflict response -// (HTTP 409) is surfaced as an API error with the response body preserved for -// callers that need conflict_paths or merge_base_sha. +// reported), and number of promoted commits. Stable merge conflict and guard +// responses throw RefUpdateError with typed, camelCase fields. Unknown 409 +// codes and non-409 responses remain ApiError values. // Target-tip modes: // - Provide expectedTargetSha when targetBranch must still point at that commit. // - Omit expectedTargetSha to merge into the current target tip. For native @@ -1245,9 +1257,14 @@ try { - ``` -- Mutating operations (commit builder, `restoreCommit`) throw `RefUpdateError` +- Mutating operations (commit builder, `restoreCommit`, and known merge 409s) + throw `RefUpdateError` when the backend reports a ref failure. Inspect `error.status`, - `error.reason`, `error.message`, and `error.refUpdate` for details. + `error.reason`, and `error.message`. Merge conflicts also provide + `error.conflictPaths` and `error.mergeBaseSha`. Failed merge guards provide + `error.guard`, `error.expectedSha`, and `error.actualSha`. Other ref updates + provide `error.refUpdate`. +- An unknown merge 409 code and every non-409 merge failure remain `ApiError`. ## License diff --git a/packages/code-storage-typescript/src/errors.ts b/packages/code-storage-typescript/src/errors.ts index 5271ab4..2ccb43a 100644 --- a/packages/code-storage-typescript/src/errors.ts +++ b/packages/code-storage-typescript/src/errors.ts @@ -1,16 +1,26 @@ -import type { RefUpdate, RefUpdateReason } from './types'; +import type { MergeGuard, RefUpdate, RefUpdateReason } from './types'; export interface RefUpdateErrorOptions { status: string; message?: string; refUpdate?: Partial; reason?: RefUpdateReason; + guard?: MergeGuard; + expectedSha?: string; + actualSha?: string; + conflictPaths?: string[]; + mergeBaseSha?: string; } export class RefUpdateError extends Error { public readonly status: string; public readonly reason: RefUpdateReason; public readonly refUpdate?: Partial; + public readonly guard?: MergeGuard; + public readonly expectedSha?: string; + public readonly actualSha?: string; + public readonly conflictPaths?: string[]; + public readonly mergeBaseSha?: string; constructor(message: string, options: RefUpdateErrorOptions) { super(message); @@ -18,6 +28,11 @@ export class RefUpdateError extends Error { this.status = options.status; this.reason = options.reason ?? inferRefUpdateReason(options.status); this.refUpdate = options.refUpdate; + this.guard = options.guard; + this.expectedSha = options.expectedSha; + this.actualSha = options.actualSha; + this.conflictPaths = options.conflictPaths; + this.mergeBaseSha = options.mergeBaseSha; } } diff --git a/packages/code-storage-typescript/src/index.ts b/packages/code-storage-typescript/src/index.ts index 4914bdc..2029631 100644 --- a/packages/code-storage-typescript/src/index.ts +++ b/packages/code-storage-typescript/src/index.ts @@ -28,6 +28,7 @@ import { deleteBranchResponseSchema, deleteTagResponseSchema, errorEnvelopeSchema, + mergeErrorResponseSchema, blameResponseSchema, getCommitResponseSchema, getBranchResponseSchema, @@ -2032,7 +2033,32 @@ class RepoImpl implements Repo { body.squash = options.squash; } - const response = await this.api.post({ path: this.repoPath('merge'), body }, jwt); + let response: Response; + try { + response = await this.api.post({ path: this.repoPath('merge'), body }, jwt); + } catch (error) { + if (error instanceof ApiError && error.status === 409) { + const parsed = mergeErrorResponseSchema.safeParse(error.body); + if (parsed.success) { + if (parsed.data.code === 'merge_conflict') { + throw new RefUpdateError(error.message, { + status: parsed.data.code, + reason: 'conflict', + conflictPaths: parsed.data.conflict_paths, + mergeBaseSha: parsed.data.merge_base_sha, + }); + } + throw new RefUpdateError(error.message, { + status: parsed.data.code, + reason: 'precondition_failed', + guard: parsed.data.guard, + expectedSha: parsed.data.expected_sha, + actualSha: parsed.data.actual_sha, + }); + } + } + throw error; + } const raw = mergeResponseSchema.parse(await response.json()); return transformMergeResult(raw); } diff --git a/packages/code-storage-typescript/src/schemas.ts b/packages/code-storage-typescript/src/schemas.ts index a412fbe..94e9895 100644 --- a/packages/code-storage-typescript/src/schemas.ts +++ b/packages/code-storage-typescript/src/schemas.ts @@ -391,6 +391,20 @@ export const errorEnvelopeSchema = z.object({ error: z.string(), }); +export const mergeErrorResponseSchema = z.discriminatedUnion('code', [ + z.object({ + code: z.literal('merge_conflict'), + conflict_paths: z.array(z.string()).optional().default([]), + merge_base_sha: z.string().optional(), + }), + z.object({ + code: z.literal('precondition_failed'), + guard: z.enum(['target', 'source']), + expected_sha: z.string(), + actual_sha: z.string(), + }), +]); + export type ListFilesResponseRaw = z.infer; export type RawTreeEntry = z.infer; export type TreeEntryTypeRaw = z.infer; diff --git a/packages/code-storage-typescript/src/types.ts b/packages/code-storage-typescript/src/types.ts index 1dc36a4..de51cc7 100644 --- a/packages/code-storage-typescript/src/types.ts +++ b/packages/code-storage-typescript/src/types.ts @@ -1087,6 +1087,8 @@ export type RefUpdateReason = | "failed" | "unknown"; +export type MergeGuard = "target" | "source"; + export interface CommitResult { commitSha: string; treeSha: string; diff --git a/packages/code-storage-typescript/tests/index.test.ts b/packages/code-storage-typescript/tests/index.test.ts index 8a77ecb..3af76b5 100644 --- a/packages/code-storage-typescript/tests/index.test.ts +++ b/packages/code-storage-typescript/tests/index.test.ts @@ -2,9 +2,11 @@ import { importPKCS8, jwtVerify } from 'jose'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { + ApiError, CodeStorage, GitStorage, OP_VERIFY_SIG, + RefUpdateError, createClient, } from '../src/index'; @@ -2645,6 +2647,143 @@ describe('GitStorage', () => { }); }); + it.each([ + { + name: 'merge conflict', + body: { + error: 'merge conflict', + code: 'merge_conflict', + conflict_paths: ['README.md'], + merge_base_sha: 'base123', + }, + expected: { + message: 'merge conflict', + status: 'merge_conflict', + reason: 'conflict', + conflictPaths: ['README.md'], + mergeBaseSha: 'base123', + }, + }, + { + name: 'stale target guard', + body: { + error: 'target branch moved', + code: 'precondition_failed', + guard: 'target', + expected_sha: 'expected-target', + actual_sha: 'actual-target', + }, + expected: { + message: 'target branch moved', + status: 'precondition_failed', + reason: 'precondition_failed', + guard: 'target', + expectedSha: 'expected-target', + actualSha: 'actual-target', + }, + }, + { + name: 'stale source guard', + body: { + error: 'source ref no longer contains the expected commit', + code: 'precondition_failed', + guard: 'source', + expected_sha: 'expected-source', + actual_sha: 'actual-source', + }, + expected: { + message: 'source ref no longer contains the expected commit', + status: 'precondition_failed', + reason: 'precondition_failed', + guard: 'source', + expectedSha: 'expected-source', + actualSha: 'actual-source', + }, + }, + ])('throws RefUpdateError for $name', async ({ body, expected }) => { + const store = new GitStorage({ name: 'v0', key }); + const repo = store.repo({ id: 'repo-merge-error' }); + + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 409, + statusText: 'Conflict', + headers: { get: () => 'application/json' }, + json: async () => body, + text: async () => JSON.stringify(body), + } as any); + + let error: unknown; + try { + await repo.merge({ + sourceRef: 'feature', + targetBranch: 'main', + strategy: 'merge', + }); + } catch (caught) { + error = caught; + } + + expect(error).toBeInstanceOf(RefUpdateError); + expect(error).toMatchObject(expected); + }); + + it.each([ + { + name: 'unknown 409 code', + status: 409, + statusText: 'Conflict', + body: { + error: 'merge conflict in README.md', + code: 'future_merge_error', + }, + }, + { + name: '403 response', + status: 403, + statusText: 'Forbidden', + body: { error: 'merge conflict', code: 'merge_conflict' }, + }, + { + name: '500 response', + status: 500, + statusText: 'Internal Server Error', + body: { + error: 'target branch moved', + code: 'precondition_failed', + guard: 'target', + expected_sha: 'expected-target', + actual_sha: 'actual-target', + }, + }, + ])('keeps ApiError for $name', async ({ status, statusText, body }) => { + const store = new GitStorage({ name: 'v0', key }); + const repo = store.repo({ id: 'repo-merge-api-error' }); + + mockFetch.mockResolvedValueOnce({ + ok: false, + status, + statusText, + headers: { get: () => 'application/json' }, + json: async () => body, + text: async () => JSON.stringify(body), + } as any); + + let error: unknown; + try { + await repo.merge({ + sourceRef: 'feature', + targetBranch: 'main', + strategy: 'merge', + }); + } catch (caught) { + error = caught; + } + + expect(error).toBeInstanceOf(ApiError); + expect(error).toMatchObject({ status, body }); + }); + it('preserves squash merge results', async () => { const store = new GitStorage({ name: 'v0', key }); const repo = store.repo({ id: 'repo-merge-squash' }); diff --git a/skills/code-storage/SKILL.md b/skills/code-storage/SKILL.md index 3dd3a90..a273a67 100644 --- a/skills/code-storage/SKILL.md +++ b/skills/code-storage/SKILL.md @@ -404,7 +404,18 @@ parent is the current target tip. It is incompatible with `ff_only`. Response: `{ "result": "merge_commit"|"fast_forward"|"no_op"|"squash"|"unknown", "commit_sha", "tree_sha", "source": {ref,ephemeral,sha}, "target": {branch,ephemeral,old_sha,new_sha}, "merge_base_sha?", "promoted_commits" }` -Conflicts return HTTP 409 with `conflict_paths` and `merge_base_sha` preserved on the body. +Every merge 409 has a stable `code`. The SDKs map these caller-actionable codes +to `RefUpdateError`: + +- `merge_conflict` uses reason `conflict` and provides conflict paths and the + merge base. +- `precondition_failed` uses reason `precondition_failed` and provides `guard` + (`target` or `source`), the expected SHA, and the actual SHA. + +TypeScript fields use camelCase. Python fields use snake_case. Go fields use +exported PascalCase names. An unknown 409 code and every non-409 merge failure +remain `ApiError` in TypeScript and Python or `APIError` in Go. Do not classify +an error from its message. ## GET /repos/{repo_name}/merge/preview — Preview Merge