diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 884782c..adc58e3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,8 +14,8 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: '1.24' @@ -46,7 +46,7 @@ jobs: binary: commit-headless-linux-arm64 steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: download artifacts uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 @@ -64,7 +64,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 # needed to make sure we get all tags @@ -127,23 +127,14 @@ jobs: branch: action command: push - - name: check release tag - id: check-tag + - name: print release instructions run: | TAG="action/v$(cat ./dist/VERSION.txt)" + SHA="${{ steps.push-commits.outputs.pushed_ref }}" + if git show-ref --tags --verify --quiet "refs/tags/${TAG}"; then - echo "Release tag ${TAG} already exists. Not releasing." - exit 1 + echo "::notice::Release tag ${TAG} already exists. Nothing to do." + exit 0 fi - echo "tag=${TAG}" >> $GITHUB_OUTPUT - - name: make release - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 - with: - script: | - github.rest.git.createRef({ - owner: context.repo.owner, - repo: context.repo.repo, - ref: 'refs/tags/${{ steps.check-tag.outputs.tag }}', - sha: '${{ steps.push-commits.outputs.pushed_ref }}' - }); + echo "::notice::To release, run: git tag ${TAG} ${SHA} && git push origin ${TAG}" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e0e0bd5..0cc4c33 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,9 +14,9 @@ jobs: unit-tests: runs-on: ubuntu-latest steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 + - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: '1.24' @@ -27,9 +27,9 @@ jobs: runs-on: ubuntu-latest needs: unit-tests steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 + - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: '1.24' @@ -51,7 +51,7 @@ jobs: TEST_BRANCH: test/ci-${{ github.run_id }}-${{ github.job }} steps: - &checkout - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 @@ -163,6 +163,17 @@ jobs: exit 1 fi + - &verify-no-tmp-branches + name: Verify no leftover working branches + run: | + tmp_branches=$(git ls-remote --heads origin | grep -- '${{ env.TEST_BRANCH }}--headless-tmp-' || true) + if [ -n "$tmp_branches" ]; then + echo "ERROR: leftover working branches found:" + echo "$tmp_branches" + exit 1 + fi + echo "No leftover working branches" + - *cleanup test-push-no-op: @@ -280,6 +291,7 @@ jobs: exit 1 fi + - *verify-no-tmp-branches - *cleanup test-commit-create-branch: @@ -421,6 +433,7 @@ jobs: fi echo "Executable bit preserved successfully" + - *verify-no-tmp-branches - *cleanup test-replay: @@ -498,6 +511,7 @@ jobs: echo "Replay test passed: commits were replayed with new hashes" + - *verify-no-tmp-branches - *cleanup test-replay-single: diff --git a/README.md b/README.md index 899ce1e..08f77b8 100644 --- a/README.md +++ b/README.md @@ -2,12 +2,15 @@ A binary tool and GitHub Action for creating signed commits from headless workflows. -`commit-headless` turns local commits into commits on the remote using the GitHub REST API. -When commits are created using the API with a GitHub App or installation token (such as -`github.token` in Actions), they are signed and verified by GitHub. Commits created with a personal -access token or OAuth token will not be signed. +`commit-headless` turns local commits into commits on the remote using the GitHub API. +By default, commits are created using the GraphQL `createCommitOnBranch` mutation, which produces +signed and verified commits regardless of token type. For commits that modify files with non-default +modes (e.g., executables) and a non-user token, the tool automatically falls back to the REST API +to preserve file modes. -File modes (such as the executable bit) are preserved when pushing commits. +File modes (such as the executable bit) are preserved when using the REST API path. The GraphQL API +does not support file modes — all files are treated as regular files (`100644`). For user tokens +(PAT, OAuth, `ghu_`), signing takes priority and the GraphQL path is always used. For the GitHub Action, see [the action branch][action-branch] and the associated `action/` release tags. @@ -138,12 +141,9 @@ By default, `commit-headless` verifies that each commit created via the API is s If a commit is not signed, it retries with exponential backoff (1s, 2s, 4s, ...) up to `--sign-attempts` times (default: 5). -Whether GitHub signs a commit depends on the token type: - -- **GitHub App / installation tokens** (including `github.token` in Actions): commits are signed - and verified by GitHub. -- **Personal access tokens / OAuth tokens**: commits are not signed. Set `--sign-attempts 0` to - skip verification when using these token types. +All token types can produce signed commits. The GraphQL API (used by default) produces signed +commits for both user tokens and app tokens. The REST API fallback also produces signed commits +for GitHub App / installation tokens. Even with a valid token, GitHub may occasionally fail to sign a commit. This has been observed internally and is not consistently reproducible. The retry mechanism exists as a safety net for @@ -152,6 +152,21 @@ these transient failures. If all attempts are exhausted without a signed commit, `commit-headless` exits with an error. This ensures unsigned commits are never silently pushed to the remote. +## API strategy + +`commit-headless` automatically chooses between the GraphQL and REST APIs on a per-commit basis: + +- **GraphQL** (default): Uses `createCommitOnBranch` — fewer API calls, commits are signed + server-side. Does not support file modes (all files are `100644`). +- **REST** (fallback): Uses the Git Database API — more API calls, but preserves file modes. + +The REST fallback is used when a commit modifies files with non-default modes (e.g., `100755` for +executables) and the token is not a user token. User tokens (`ghp_`, `gho_`, `ghu_`, +`github_pat_`) always use GraphQL since signing is the priority. + +The tool logs which strategy is used for each commit. If the GraphQL path is used for a commit with +non-default file modes, a warning is logged. + ## Try it Create a local commit and push it to a new branch: diff --git a/action-template/README.md b/action-template/README.md index 49afe22..9d09370 100644 --- a/action-template/README.md +++ b/action-template/README.md @@ -2,6 +2,10 @@ This action creates signed and verified commits on GitHub from a workflow. +Commits are created using the GraphQL API by default, which produces signed commits for all token +types. When a commit modifies files with non-default modes (e.g., executables) and a non-user token +is used, the action automatically falls back to the REST API to preserve file modes. + For source code and CLI documentation, see the [main branch](https://github.com/DataDog/commit-headless/tree/main). ## Commands diff --git a/action-template/action.yml b/action-template/action.yml index 739d1f5..a18a316 100644 --- a/action-template/action.yml +++ b/action-template/action.yml @@ -58,5 +58,5 @@ outputs: description: 'Commit hash of the last commit created' runs: - using: 'node20' + using: 'node24' main: 'action.js' diff --git a/change.go b/change.go index 9954ff4..baad507 100644 --- a/change.go +++ b/change.go @@ -25,6 +25,17 @@ type Change struct { entries map[string]FileEntry } +// HasNonDefaultModes returns true if any file entry has a mode other than the +// default 100644 (regular file). Empty mode is treated as 100644. +func (c Change) HasNonDefaultModes() bool { + for _, fe := range c.entries { + if fe.Mode != "" && fe.Mode != "100644" { + return true + } + } + return false +} + // Splits a commit message on the first blank line func (c Change) splitMessage() (string, string) { h, b, _ := strings.Cut(c.message, "\n\n") diff --git a/change_test.go b/change_test.go index 1a089d5..de3b88d 100644 --- a/change_test.go +++ b/change_test.go @@ -5,6 +5,28 @@ import ( "testing" ) +func TestHasNonDefaultModes(t *testing.T) { + tests := []struct { + name string + entries map[string]FileEntry + want bool + }{ + {"all default", map[string]FileEntry{"a": {Mode: "100644"}}, false}, + {"empty mode treated as default", map[string]FileEntry{"a": {Mode: ""}}, false}, + {"executable", map[string]FileEntry{"a": {Mode: "100755"}}, true}, + {"symlink", map[string]FileEntry{"a": {Mode: "120000"}}, true}, + {"mixed", map[string]FileEntry{"a": {Mode: "100644"}, "b": {Mode: "100755"}}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := Change{entries: tt.entries} + if got := c.HasNonDefaultModes(); got != tt.want { + t.Errorf("HasNonDefaultModes() = %v, want %v", got, tt.want) + } + }) + } +} + func TestChangeBody(t *testing.T) { testcases := []struct { input string diff --git a/github.go b/github.go index 08b52c9..5b3e281 100644 --- a/github.go +++ b/github.go @@ -1,9 +1,13 @@ package main import ( + "bytes" "context" + "encoding/base64" + "encoding/json" "errors" "fmt" + "math/rand/v2" "net/http" "strings" "time" @@ -27,16 +31,77 @@ type GitAPI interface { CreateTree(ctx context.Context, owner, repo, baseTree string, entries []*github.TreeEntry) (*github.Tree, *github.Response, error) CreateCommit(ctx context.Context, owner, repo string, commit github.Commit, opts *github.CreateCommitOptions) (*github.Commit, *github.Response, error) UpdateRef(ctx context.Context, owner, repo, ref string, updateRef github.UpdateRef) (*github.Reference, *github.Response, error) + DeleteRef(ctx context.Context, owner, repo, ref string) (*github.Response, error) +} + +// GraphQLAPI abstracts the GraphQL endpoint for testing. +type GraphQLAPI interface { + Do(ctx context.Context, query string, variables map[string]any) (json.RawMessage, error) +} + +// graphQLClient is the production implementation of GraphQLAPI using raw HTTP. +type graphQLClient struct { + httpClient *http.Client + endpoint string +} + +func (g *graphQLClient) Do(ctx context.Context, query string, variables map[string]any) (json.RawMessage, error) { + body := map[string]any{ + "query": query, + "variables": variables, + } + buf, err := json.Marshal(body) + if err != nil { + return nil, fmt.Errorf("marshal graphql request: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, g.endpoint, bytes.NewReader(buf)) + if err != nil { + return nil, fmt.Errorf("create graphql request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + resp, err := g.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("graphql request: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("graphql request: http %d", resp.StatusCode) + } + + var result struct { + Data json.RawMessage `json:"data"` + Errors []struct { + Message string `json:"message"` + } `json:"errors"` + } + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil, fmt.Errorf("decode graphql response: %w", err) + } + + if len(result.Errors) > 0 { + msgs := make([]string, len(result.Errors)) + for i, e := range result.Errors { + msgs[i] = e.Message + } + return nil, fmt.Errorf("graphql errors: %s", strings.Join(msgs, "; ")) + } + + return result.Data, nil } // Client provides methods for interacting with a remote repository on GitHub type Client struct { - repos RepositoriesAPI - git GitAPI - owner string - repo string - branch string - + repos RepositoriesAPI + git GitAPI + graphql GraphQLAPI + owner string + repo string + branch string + + userToken bool dryrun bool force bool signAttempts int @@ -50,11 +115,16 @@ func NewClient(ctx context.Context, token, owner, repo, branch string) *Client { ghClient := github.NewClient(httpC) return &Client{ - repos: ghClient.Repositories, - git: ghClient.Git, - owner: owner, - repo: repo, - branch: branch, + repos: ghClient.Repositories, + git: ghClient.Git, + graphql: &graphQLClient{ + httpClient: httpC, + endpoint: "https://api.github.com/graphql", + }, + owner: owner, + repo: repo, + branch: branch, + userToken: isUserToken(token), } } @@ -97,36 +167,107 @@ func (c *Client) CreateBranch(ctx context.Context, headSha string) (string, erro return created.GetObject().GetSHA(), nil } -// PushChanges creates commits for each change, then updates the branch ref once at the end. -// This is all-or-nothing: if any commit fails, the branch ref is not updated. +// commitStrategy indicates which API to use for creating a commit. +type commitStrategy int + +const ( + strategyGraphQL commitStrategy = iota + strategyREST +) + +func (s commitStrategy) String() string { + if s == strategyGraphQL { + return "GraphQL" + } + return "REST" +} + +// chooseStrategy decides whether to use GraphQL or REST for a given change. +func (c *Client) chooseStrategy(change Change) commitStrategy { + if change.HasNonDefaultModes() && !c.userToken { + return strategyREST + } + return strategyGraphQL +} + +// PushChanges creates commits for each change on a throwaway branch, then updates the real branch +// ref once at the end. The throwaway branch is always cleaned up on completion. // It returns the number of changes that were successfully pushed, the new head reference hash, and // any error encountered. func (c *Client) PushChanges(ctx context.Context, headCommit string, changes ...Change) (int, string, error) { - var err error - for i, change := range changes { - headCommit, err = c.CreateChange(ctx, headCommit, change) - if err != nil { - return i + 1, "", fmt.Errorf("push change %d: %w", i+i, err) + if c.dryrun { + for i, change := range changes { + _, _, err := c.CreateChange(ctx, "", headCommit, change) + if err != nil { + return i + 1, "", fmt.Errorf("push change %d: %w", i, err) + } } + return len(changes), strings.Repeat("0", 40), nil + } + + // Create throwaway branch + tmpBranch := fmt.Sprintf("%s--headless-tmp-%s", c.branch, randomSuffix()) + tmpRef := fmt.Sprintf("refs/heads/%s", tmpBranch) + + logger.Printf("Creating working branch %s\n", tmpBranch) + _, _, err := c.git.CreateRef(ctx, c.owner, c.repo, github.CreateRef{ + Ref: tmpRef, + SHA: headCommit, + }) + if err != nil { + return 0, "", fmt.Errorf("create working branch: %w", err) } - if !c.dryrun { - _, _, err = c.git.UpdateRef(ctx, c.owner, c.repo, "refs/heads/"+c.branch, github.UpdateRef{ - SHA: headCommit, - Force: github.Ptr(c.force), - }) + // Always clean up the throwaway branch + defer func() { + logger.Printf("Cleaning up working branch %s\n", tmpBranch) + if _, err := c.git.DeleteRef(ctx, c.owner, c.repo, tmpRef); err != nil { + logger.Warningf("Failed to delete working branch %s: %v", tmpBranch, err) + } + }() + + // Create commits on the throwaway branch + for i, change := range changes { + newHead, strategy, err := c.CreateChange(ctx, tmpBranch, headCommit, change) if err != nil { - return len(changes), "", fmt.Errorf("update ref: %w", err) + return i + 1, "", fmt.Errorf("push change %d: %w", i, err) } + + if strategy == strategyREST { + // REST creates a detached commit; advance the throwaway branch to stay in sync + _, _, err = c.git.UpdateRef(ctx, c.owner, c.repo, tmpRef, github.UpdateRef{ + SHA: newHead, + Force: github.Ptr(true), + }) + if err != nil { + return i + 1, "", fmt.Errorf("advance working branch: %w", err) + } + } + + headCommit = newHead + } + + // Update the real branch + _, _, err = c.git.UpdateRef(ctx, c.owner, c.repo, "refs/heads/"+c.branch, github.UpdateRef{ + SHA: headCommit, + Force: github.Ptr(c.force), + }) + if err != nil { + return len(changes), "", fmt.Errorf("update ref: %w", err) } return len(changes), headCommit, nil } -// CreateChange creates a single commit from a change using the REST API. -// It does not update the branch ref — that is done by PushChanges after all commits succeed. -// It returns the hash of the created commit or an error. -func (c *Client) CreateChange(ctx context.Context, headCommit string, change Change) (string, error) { +// CreateChange creates a single commit from a change, dispatching to GraphQL or REST based on +// the change contents and token type. It handles logging, dry-run, and signature verification +// retry for both strategies. +// +// tmpBranch is the name of the throwaway branch (without refs/heads/ prefix) used for GraphQL +// commits. It may be empty for dry-run mode. +// +// Returns the new commit SHA, the strategy used, and any error. +func (c *Client) CreateChange(ctx context.Context, tmpBranch, headCommit string, change Change) (string, commitStrategy, error) { shortHash := change.hash if len(shortHash) > 8 { shortHash = shortHash[:8] @@ -150,22 +291,94 @@ func (c *Client) CreateChange(ctx context.Context, headCommit string, change Cha logger.Printf(" - %s: %s\n", action, path) } + strategy := c.chooseStrategy(change) + logger.Printf("Strategy: %s\n", strategy) + + if strategy == strategyGraphQL && change.HasNonDefaultModes() { + logger.Warningf("GraphQL API does not preserve file modes; non-default modes in this commit will be reset to 100644") + } + if c.dryrun { logger.Notice("Dry run enabled, not writing commit") - return strings.Repeat("0", len(change.hash)), nil + return strings.Repeat("0", len(change.hash)), strategy, nil } - // Get the parent commit's tree SHA + // Build the commit and reset functions based on strategy. + // resetFn rewinds the throwaway branch before a GraphQL retry so expectedHeadOid matches again. + var commitFn func() (string, bool, error) + var resetFn func() error + + switch strategy { + case strategyGraphQL: + tmpRef := fmt.Sprintf("refs/heads/%s", tmpBranch) + commitFn = func() (string, bool, error) { + return c.execGraphQLCommit(ctx, tmpBranch, headCommit, change) + } + resetFn = func() error { + _, _, err := c.git.UpdateRef(ctx, c.owner, c.repo, tmpRef, github.UpdateRef{ + SHA: headCommit, + Force: github.Ptr(true), + }) + if err != nil { + return fmt.Errorf("reset working branch for retry: %w", err) + } + return nil + } + case strategyREST: + // Prep tree once before the retry loop + treeSHA, err := c.prepTree(ctx, headCommit, change) + if err != nil { + return "", strategy, err + } + commitFn = func() (string, bool, error) { + return c.execRESTCommit(ctx, headCommit, treeSHA, change) + } + resetFn = func() error { return nil } + } + + // Execute with signature verification retry + commitSha, verified, err := commitFn() + if err != nil { + return "", strategy, fmt.Errorf("create commit: %w", err) + } + + if c.signAttempts > 0 { + backoff := 1 * time.Second + for attempt := 1; attempt <= c.signAttempts && !verified; attempt++ { + if attempt == c.signAttempts { + return "", strategy, fmt.Errorf("commit %s was not signed after %d attempt(s)", commitSha, c.signAttempts) + } + + logger.Warningf("Commit %s not signed (attempt %d/%d), retrying in %s...", commitSha, attempt, c.signAttempts, backoff) + time.Sleep(backoff) + backoff *= 2 + + if err := resetFn(); err != nil { + return "", strategy, err + } + + commitSha, verified, err = commitFn() + if err != nil { + return "", strategy, fmt.Errorf("create commit (attempt %d): %w", attempt+1, err) + } + } + } + + logger.Printf("Created: %s\n", c.commitURL(commitSha)) + return commitSha, strategy, nil +} + +// prepTree creates blobs and a tree for the REST commit path. This is called once before the +// retry loop since the tree is immutable and can be reused across retries. +func (c *Client) prepTree(ctx context.Context, headCommit string, change Change) (string, error) { parentCommit, _, err := c.git.GetCommit(ctx, c.owner, c.repo, headCommit) if err != nil { return "", fmt.Errorf("get parent commit: %w", err) } baseTreeSHA := parentCommit.GetTree().GetSHA() - // Build tree entries var entries []*github.TreeEntry for path, fe := range change.entries { - // Use the file's mode, defaulting to 100644 for regular files mode := fe.Mode if mode == "" { mode = "100644" @@ -179,7 +392,6 @@ func (c *Client) CreateChange(ctx context.Context, headCommit string, change Cha if fe.Content == nil { // Deletion: SHA must be empty string for go-github to omit it } else { - // Create blob for additions/modifications blob, _, err := c.git.CreateBlob(ctx, c.owner, c.repo, github.Blob{ Content: github.Ptr(string(fe.Content)), Encoding: github.Ptr("utf-8"), @@ -192,13 +404,16 @@ func (c *Client) CreateChange(ctx context.Context, headCommit string, change Cha entries = append(entries, entry) } - // Create tree tree, _, err := c.git.CreateTree(ctx, c.owner, c.repo, baseTreeSHA, entries) if err != nil { return "", fmt.Errorf("create tree: %w", err) } - // Create commit (with signature verification retry) + return tree.GetSHA(), nil +} + +// execRESTCommit creates a commit using the REST API with a pre-built tree. +func (c *Client) execRESTCommit(ctx context.Context, headCommit, treeSHA string, change Change) (string, bool, error) { message := change.Headline() if body := change.Body(); body != "" { message = message + "\n\n" + body @@ -206,40 +421,105 @@ func (c *Client) CreateChange(ctx context.Context, headCommit string, change Cha commitInput := github.Commit{ Message: github.Ptr(message), - Tree: &github.Tree{SHA: tree.SHA}, + Tree: &github.Tree{SHA: github.Ptr(treeSHA)}, Parents: []*github.Commit{{SHA: github.Ptr(headCommit)}}, } commit, _, err := c.git.CreateCommit(ctx, c.owner, c.repo, commitInput, nil) if err != nil { - return "", fmt.Errorf("create commit: %w", err) + return "", false, fmt.Errorf("REST create commit: %w", err) } - if c.signAttempts > 0 { - backoff := 1 * time.Second - for attempt := 1; attempt <= c.signAttempts; attempt++ { - if commit.GetVerification().GetVerified() { - break - } + return commit.GetSHA(), commit.GetVerification().GetVerified(), nil +} - if attempt == c.signAttempts { - reason := commit.GetVerification().GetReason() - return "", fmt.Errorf("commit %s was not signed after %d attempt(s) (reason: %s)", commit.GetSHA(), c.signAttempts, reason) - } +// createCommitOnBranch GraphQL mutation +const createCommitOnBranchMutation = ` +mutation CreateCommitOnBranch($input: CreateCommitOnBranchInput!) { + createCommitOnBranch(input: $input) { + commit { + oid + signature { + isValid + } + } + } +} +` - logger.Warningf("Commit %s not signed (attempt %d/%d, reason: %s), retrying in %s...", commit.GetSHA(), attempt, c.signAttempts, commit.GetVerification().GetReason(), backoff) - time.Sleep(backoff) - backoff *= 2 +// execGraphQLCommit creates a commit using the GraphQL createCommitOnBranch mutation. +func (c *Client) execGraphQLCommit(ctx context.Context, tmpBranch, headCommit string, change Change) (string, bool, error) { + // Build file additions and deletions + var additions []map[string]string + var deletions []map[string]string - commit, _, err = c.git.CreateCommit(ctx, c.owner, c.repo, commitInput, nil) - if err != nil { - return "", fmt.Errorf("create commit (attempt %d): %w", attempt+1, err) - } + for path, fe := range change.entries { + if fe.Content == nil { + deletions = append(deletions, map[string]string{"path": path}) + } else { + additions = append(additions, map[string]string{ + "path": path, + "contents": base64.StdEncoding.EncodeToString(fe.Content), + }) } } - commitSha := commit.GetSHA() - logger.Printf("Created: %s\n", c.commitURL(commitSha)) + headline := change.Headline() + body := change.Body() + + message := map[string]string{"headline": headline} + if body != "" { + message["body"] = body + } - return commitSha, nil + fileChanges := map[string]any{} + if len(additions) > 0 { + fileChanges["additions"] = additions + } + if len(deletions) > 0 { + fileChanges["deletions"] = deletions + } + + input := map[string]any{ + "branch": map[string]any{ + "repositoryNameWithOwner": fmt.Sprintf("%s/%s", c.owner, c.repo), + "branchName": tmpBranch, + }, + "expectedHeadOid": headCommit, + "message": message, + "fileChanges": fileChanges, + } + + data, err := c.graphql.Do(ctx, createCommitOnBranchMutation, map[string]any{"input": input}) + if err != nil { + return "", false, fmt.Errorf("GraphQL createCommitOnBranch: %w", err) + } + + var result struct { + CreateCommitOnBranch struct { + Commit struct { + OID string `json:"oid"` + Signature struct { + IsValid bool `json:"isValid"` + } `json:"signature"` + } `json:"commit"` + } `json:"createCommitOnBranch"` + } + if err := json.Unmarshal(data, &result); err != nil { + return "", false, fmt.Errorf("unmarshal GraphQL response: %w", err) + } + + oid := result.CreateCommitOnBranch.Commit.OID + signed := result.CreateCommitOnBranch.Commit.Signature.IsValid + + return oid, signed, nil +} + +func randomSuffix() string { + const chars = "abcdefghijklmnopqrstuvwxyz0123456789" + b := make([]byte, 8) + for i := range b { + b[i] = chars[rand.IntN(len(chars))] + } + return string(b) } diff --git a/github_test.go b/github_test.go index e70735d..4fb631f 100644 --- a/github_test.go +++ b/github_test.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" @@ -22,6 +23,23 @@ func newFileEntry(content []byte) FileEntry { return FileEntry{Content: content, Mode: "100644"} } +// mockGraphQL implements GraphQLAPI for testing. +type mockGraphQL struct { + handler func(query string, variables map[string]any) (json.RawMessage, error) +} + +func (m *mockGraphQL) Do(_ context.Context, query string, variables map[string]any) (json.RawMessage, error) { + return m.handler(query, variables) +} + +func signedGraphQLResponse(oid string) json.RawMessage { + return json.RawMessage(`{"createCommitOnBranch":{"commit":{"oid":"` + oid + `","signature":{"isValid":true}}}}`) +} + +func unsignedGraphQLResponse(oid string) json.RawMessage { + return json.RawMessage(`{"createCommitOnBranch":{"commit":{"oid":"` + oid + `","signature":{"isValid":false}}}}`) +} + func TestBuildTreeEntries(t *testing.T) { change := Change{ entries: map[string]FileEntry{ @@ -31,7 +49,7 @@ func TestBuildTreeEntries(t *testing.T) { }, } - // Build tree entries the same way CreateChange does (without making API calls) + // Build tree entries the same way prepTree does (without making API calls) var addedPaths, deletedPaths []string for path, fe := range change.entries { if fe.Content == nil { @@ -207,17 +225,58 @@ func TestCreateChange(t *testing.T) { }, } - sha, err := client.CreateChange(context.Background(), "head-sha", change) + sha, _, err := client.CreateChange(context.Background(), "", "head-sha", change) if err != nil { t.Fatalf("unexpected error: %v", err) } - // Should return zeros with same length as input hash if sha != "000000" { t.Errorf("expected '000000', got %q", sha) } }) - t.Run("successful push with file addition", func(t *testing.T) { + t.Run("graphql commit", func(t *testing.T) { + callCount := 0 + client := &Client{ + owner: "test-owner", + repo: "test-repo", + branch: "test-branch", + graphql: &mockGraphQL{ + handler: func(query string, variables map[string]any) (json.RawMessage, error) { + callCount++ + input := variables["input"].(map[string]any) + branch := input["branch"].(map[string]any) + if branch["branchName"] != "tmp-branch" { + t.Errorf("expected branch 'tmp-branch', got %v", branch["branchName"]) + } + if input["expectedHeadOid"] != "parent-sha" { + t.Errorf("expected expectedHeadOid 'parent-sha', got %v", input["expectedHeadOid"]) + } + return signedGraphQLResponse("new-commit-sha"), nil + }, + }, + } + + change := Change{ + hash: "local-hash", + message: "Test commit", + entries: map[string]FileEntry{ + "file.txt": newFileEntry([]byte("content")), + }, + } + + sha, _, err := client.CreateChange(context.Background(), "tmp-branch", "parent-sha", change) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if sha != "new-commit-sha" { + t.Errorf("expected 'new-commit-sha', got %q", sha) + } + if callCount != 1 { + t.Errorf("expected 1 GraphQL call, got %d", callCount) + } + }) + + t.Run("rest fallback for non-default modes with non-user token", func(t *testing.T) { var ( getCommitCalled bool createBlobCalled bool @@ -240,8 +299,6 @@ func TestCreateChange(t *testing.T) { case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/blobs": createBlobCalled = true - var blob github.Blob - json.NewDecoder(r.Body).Decode(&blob) w.WriteHeader(http.StatusCreated) json.NewEncoder(w).Encode(github.Blob{ SHA: github.Ptr("blob-sha-123"), @@ -256,11 +313,6 @@ func TestCreateChange(t *testing.T) { case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/commits": commitCalled = true - var commit github.Commit - json.NewDecoder(r.Body).Decode(&commit) - if commit.GetMessage() != "Test commit" { - t.Errorf("unexpected commit message: %s", commit.GetMessage()) - } w.WriteHeader(http.StatusCreated) json.NewEncoder(w).Encode(github.Commit{ SHA: github.Ptr("new-commit-sha"), @@ -274,15 +326,17 @@ func TestCreateChange(t *testing.T) { defer server.Close() client := newTestClient(t, server) + client.userToken = false // non-user token + change := Change{ hash: "local-hash", message: "Test commit", entries: map[string]FileEntry{ - "file.txt": newFileEntry([]byte("content")), + "script.sh": {Content: []byte("#!/bin/bash"), Mode: "100755"}, }, } - sha, err := client.CreateChange(context.Background(), "parent-sha", change) + sha, _, err := client.CreateChange(context.Background(), "tmp-branch", "parent-sha", change) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -304,149 +358,90 @@ func TestCreateChange(t *testing.T) { } }) - t.Run("successful push with file deletion", func(t *testing.T) { - var treeEntries []*github.TreeEntry - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - - switch { - case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/"): - json.NewEncoder(w).Encode(github.Commit{ - SHA: github.Ptr("parent-sha"), - Tree: &github.Tree{ - SHA: github.Ptr("parent-tree-sha"), - }, - }) - - case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/trees": - var req struct { - BaseTree string `json:"base_tree"` - Tree []*github.TreeEntry `json:"tree"` - } - json.NewDecoder(r.Body).Decode(&req) - treeEntries = req.Tree - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Tree{ - SHA: github.Ptr("new-tree-sha"), - }) - - case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/commits": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Commit{ - SHA: github.Ptr("new-commit-sha"), - }) - - case r.Method == http.MethodPatch && strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/refs/"): - json.NewEncoder(w).Encode(github.Reference{ - Object: &github.GitObject{ - SHA: github.Ptr("new-commit-sha"), - }, - }) - - default: - t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) - w.WriteHeader(http.StatusNotFound) - } - })) - defer server.Close() + t.Run("graphql used for non-default modes with user token", func(t *testing.T) { + client := &Client{ + owner: "test-owner", + repo: "test-repo", + branch: "test-branch", + userToken: true, + graphql: &mockGraphQL{ + handler: func(query string, variables map[string]any) (json.RawMessage, error) { + return signedGraphQLResponse("graphql-commit-sha"), nil + }, + }, + } - client := newTestClient(t, server) change := Change{ hash: "local-hash", - message: "Delete file", + message: "Test commit", entries: map[string]FileEntry{ - "deleted-file.txt": {Content: nil}, // deletion + "script.sh": {Content: []byte("#!/bin/bash"), Mode: "100755"}, }, } - _, err := client.CreateChange(context.Background(), "parent-sha", change) + sha, _, err := client.CreateChange(context.Background(), "tmp-branch", "parent-sha", change) if err != nil { t.Fatalf("unexpected error: %v", err) } - - // Verify that the tree entry for deletion has no SHA (which signals deletion) - if len(treeEntries) != 1 { - t.Fatalf("expected 1 tree entry, got %d", len(treeEntries)) - } - if treeEntries[0].GetPath() != "deleted-file.txt" { - t.Errorf("expected path 'deleted-file.txt', got %q", treeEntries[0].GetPath()) - } - // For deletions, SHA should be nil/empty - if treeEntries[0].SHA != nil && *treeEntries[0].SHA != "" { - t.Errorf("expected nil/empty SHA for deletion, got %q", *treeEntries[0].SHA) + if sha != "graphql-commit-sha" { + t.Errorf("expected 'graphql-commit-sha', got %q", sha) } }) - t.Run("commit with body", func(t *testing.T) { - var receivedMessage string - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - - switch { - case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/"): - json.NewEncoder(w).Encode(github.Commit{ - Tree: &github.Tree{SHA: github.Ptr("tree-sha")}, - }) - - case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/blobs": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Blob{SHA: github.Ptr("blob-sha")}) - - case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/trees": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Tree{SHA: github.Ptr("tree-sha")}) - - case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/commits": - var commit github.Commit - json.NewDecoder(r.Body).Decode(&commit) - receivedMessage = commit.GetMessage() - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Commit{SHA: github.Ptr("commit-sha")}) + t.Run("commit with body via graphql", func(t *testing.T) { + var receivedHeadline, receivedBody string - case r.Method == http.MethodPatch: - json.NewEncoder(w).Encode(github.Reference{ - Object: &github.GitObject{SHA: github.Ptr("commit-sha")}, - }) - } - })) - defer server.Close() + client := &Client{ + owner: "test-owner", + repo: "test-repo", + branch: "test-branch", + graphql: &mockGraphQL{ + handler: func(query string, variables map[string]any) (json.RawMessage, error) { + input := variables["input"].(map[string]any) + msg := input["message"].(map[string]string) + receivedHeadline = msg["headline"] + if b, ok := msg["body"]; ok { + receivedBody = b + } + return signedGraphQLResponse("commit-sha"), nil + }, + }, + } - client := newTestClient(t, server) change := Change{ hash: "local", message: "Headline\n\nThis is the body\nwith multiple lines", entries: map[string]FileEntry{"file.txt": newFileEntry([]byte("x"))}, } - _, err := client.CreateChange(context.Background(), "parent", change) + _, _, err := client.CreateChange(context.Background(), "tmp", "parent", change) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !strings.Contains(receivedMessage, "Headline") { - t.Errorf("message should contain headline, got %q", receivedMessage) + if receivedHeadline != "Headline" { + t.Errorf("expected headline 'Headline', got %q", receivedHeadline) } - if !strings.Contains(receivedMessage, "This is the body") { - t.Errorf("message should contain body, got %q", receivedMessage) + if !strings.Contains(receivedBody, "This is the body") { + t.Errorf("expected body to contain 'This is the body', got %q", receivedBody) } }) - t.Run("get parent commit fails", func(t *testing.T) { + t.Run("rest get parent commit fails", func(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) })) defer server.Close() client := newTestClient(t, server) + client.userToken = false change := Change{ hash: "local", message: "Test", - entries: map[string]FileEntry{"file.txt": newFileEntry([]byte("x"))}, + entries: map[string]FileEntry{"script.sh": {Content: []byte("x"), Mode: "100755"}}, } - _, err := client.CreateChange(context.Background(), "nonexistent", change) + _, _, err := client.CreateChange(context.Background(), "tmp", "nonexistent", change) if err == nil { t.Fatal("expected error, got nil") } @@ -454,201 +449,222 @@ func TestCreateChange(t *testing.T) { t.Errorf("expected 'get parent commit' error, got %q", err.Error()) } }) +} + +func TestPushChanges(t *testing.T) { + t.Run("graphql with throwaway branch", func(t *testing.T) { + var ( + createdRefs []string + deletedRefs []string + updatedRefs []string + ) + commitCount := 0 - t.Run("create blob fails", func(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - if strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/") { - json.NewEncoder(w).Encode(github.Commit{ - Tree: &github.Tree{SHA: github.Ptr("tree-sha")}, + switch { + case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/refs": + var req github.CreateRef + json.NewDecoder(r.Body).Decode(&req) + createdRefs = append(createdRefs, req.Ref) + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(github.Reference{ + Object: &github.GitObject{SHA: github.Ptr("initial")}, }) - return - } - if r.URL.Path == "/repos/test-owner/test-repo/git/blobs" { - w.WriteHeader(http.StatusInternalServerError) - return - } - })) - defer server.Close() - client := newTestClient(t, server) - change := Change{ - hash: "local", - message: "Test", - entries: map[string]FileEntry{"file.txt": newFileEntry([]byte("x"))}, - } - - _, err := client.CreateChange(context.Background(), "parent", change) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), "create blob") { - t.Errorf("expected 'create blob' error, got %q", err.Error()) - } - }) + case r.Method == http.MethodPatch && strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/refs/"): + ref := strings.TrimPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/refs/") + updatedRefs = append(updatedRefs, ref) + json.NewEncoder(w).Encode(github.Reference{ + Object: &github.GitObject{SHA: github.Ptr("final-sha")}, + }) - t.Run("create tree fails", func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") + case r.Method == http.MethodDelete && strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/refs/"): + ref := strings.TrimPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/refs/") + deletedRefs = append(deletedRefs, ref) + w.WriteHeader(http.StatusNoContent) - switch { - case strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/"): - json.NewEncoder(w).Encode(github.Commit{ - Tree: &github.Tree{SHA: github.Ptr("tree-sha")}, - }) - case r.URL.Path == "/repos/test-owner/test-repo/git/blobs": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Blob{SHA: github.Ptr("blob-sha")}) - case r.URL.Path == "/repos/test-owner/test-repo/git/trees": - w.WriteHeader(http.StatusInternalServerError) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) } })) defer server.Close() client := newTestClient(t, server) - change := Change{ - hash: "local", - message: "Test", - entries: map[string]FileEntry{"file.txt": newFileEntry([]byte("x"))}, + client.graphql = &mockGraphQL{ + handler: func(query string, variables map[string]any) (json.RawMessage, error) { + commitCount++ + return signedGraphQLResponse("commit-sha-" + string(rune('0'+commitCount))), nil + }, } - _, err := client.CreateChange(context.Background(), "parent", change) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), "create tree") { - t.Errorf("expected 'create tree' error, got %q", err.Error()) + changes := []Change{ + {hash: "h1", message: "First", entries: map[string]FileEntry{"a.txt": newFileEntry([]byte("a"))}}, + {hash: "h2", message: "Second", entries: map[string]FileEntry{"b.txt": newFileEntry([]byte("b"))}}, } - }) - t.Run("create commit fails", func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") + count, sha, err := client.PushChanges(context.Background(), "initial", changes...) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if count != 2 { + t.Errorf("expected count 2, got %d", count) + } + if sha == "" { + t.Error("expected non-empty sha") + } - switch { - case strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/"): - json.NewEncoder(w).Encode(github.Commit{ - Tree: &github.Tree{SHA: github.Ptr("tree-sha")}, - }) - case r.URL.Path == "/repos/test-owner/test-repo/git/blobs": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Blob{SHA: github.Ptr("blob-sha")}) - case r.URL.Path == "/repos/test-owner/test-repo/git/trees": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Tree{SHA: github.Ptr("tree-sha")}) - case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/commits": - w.WriteHeader(http.StatusInternalServerError) - } - })) - defer server.Close() + // Should have created a throwaway branch + if len(createdRefs) != 1 { + t.Fatalf("expected 1 created ref, got %d", len(createdRefs)) + } + if !strings.Contains(createdRefs[0], "--headless-tmp-") { + t.Errorf("expected throwaway branch ref, got %q", createdRefs[0]) + } - client := newTestClient(t, server) - change := Change{ - hash: "local", - message: "Test", - entries: map[string]FileEntry{"file.txt": newFileEntry([]byte("x"))}, + // Should have updated only the real branch + if len(updatedRefs) != 1 { + t.Fatalf("expected 1 updated ref, got %d: %v", len(updatedRefs), updatedRefs) + } + if updatedRefs[0] != "heads/test-branch" { + t.Errorf("expected real branch update, got %q", updatedRefs[0]) } - _, err := client.CreateChange(context.Background(), "parent", change) - if err == nil { - t.Fatal("expected error, got nil") + // Should have deleted the throwaway branch + if len(deletedRefs) != 1 { + t.Fatalf("expected 1 deleted ref, got %d", len(deletedRefs)) + } + if !strings.Contains(deletedRefs[0], "--headless-tmp-") { + t.Errorf("expected throwaway branch deletion, got %q", deletedRefs[0]) } - if !strings.Contains(err.Error(), "create commit") { - t.Errorf("expected 'create commit' error, got %q", err.Error()) + + if commitCount != 2 { + t.Errorf("expected 2 GraphQL commits, got %d", commitCount) } }) -} + t.Run("rest fallback advances throwaway branch", func(t *testing.T) { + var updatedRefs []string -func TestPushChanges(t *testing.T) { - t.Run("multiple changes", func(t *testing.T) { - commitCount := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") switch { - case strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/"): + case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/refs": + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(github.Reference{ + Object: &github.GitObject{SHA: github.Ptr("initial")}, + }) + + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/"): json.NewEncoder(w).Encode(github.Commit{ Tree: &github.Tree{SHA: github.Ptr("tree-sha")}, }) + case r.URL.Path == "/repos/test-owner/test-repo/git/blobs": w.WriteHeader(http.StatusCreated) json.NewEncoder(w).Encode(github.Blob{SHA: github.Ptr("blob-sha")}) - case r.URL.Path == "/repos/test-owner/test-repo/git/trees": + + case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/trees": w.WriteHeader(http.StatusCreated) json.NewEncoder(w).Encode(github.Tree{SHA: github.Ptr("tree-sha")}) + case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/commits": - commitCount++ w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Commit{SHA: github.Ptr("commit-sha-" + string(rune('0'+commitCount)))}) - case r.Method == http.MethodPatch: + json.NewEncoder(w).Encode(github.Commit{SHA: github.Ptr("rest-commit-sha")}) + + case r.Method == http.MethodPatch && strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/refs/"): + ref := strings.TrimPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/refs/") + updatedRefs = append(updatedRefs, ref) json.NewEncoder(w).Encode(github.Reference{ - Object: &github.GitObject{SHA: github.Ptr("final-sha")}, + Object: &github.GitObject{SHA: github.Ptr("rest-commit-sha")}, }) + + case r.Method == http.MethodDelete: + w.WriteHeader(http.StatusNoContent) + + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) } })) defer server.Close() client := newTestClient(t, server) + client.userToken = false + changes := []Change{ - {hash: "h1", message: "First", entries: map[string]FileEntry{"a.txt": newFileEntry([]byte("a"))}}, - {hash: "h2", message: "Second", entries: map[string]FileEntry{"b.txt": newFileEntry([]byte("b"))}}, - {hash: "h3", message: "Third", entries: map[string]FileEntry{"c.txt": newFileEntry([]byte("c"))}}, + {hash: "h1", message: "Exec file", entries: map[string]FileEntry{"script.sh": {Content: []byte("#!/bin/bash"), Mode: "100755"}}}, } - count, sha, err := client.PushChanges(context.Background(), "initial", changes...) + count, _, err := client.PushChanges(context.Background(), "initial", changes...) if err != nil { t.Fatalf("unexpected error: %v", err) } - if count != 3 { - t.Errorf("expected count 3, got %d", count) + if count != 1 { + t.Errorf("expected count 1, got %d", count) } - if sha == "" { - t.Error("expected non-empty sha") + + // Should have updated both the throwaway branch (to sync) and the real branch + if len(updatedRefs) != 2 { + t.Fatalf("expected 2 updated refs, got %d: %v", len(updatedRefs), updatedRefs) + } + + // First update should be the throwaway branch sync + if !strings.Contains(updatedRefs[0], "--headless-tmp-") { + t.Errorf("expected first ref update to be throwaway branch, got %q", updatedRefs[0]) } - if commitCount != 3 { - t.Errorf("expected 3 commits to be created, got %d", commitCount) + // Second update should be the real branch + if updatedRefs[1] != "heads/test-branch" { + t.Errorf("expected second ref update to be real branch, got %q", updatedRefs[1]) } }) - t.Run("failure on second change", func(t *testing.T) { - commitCount := 0 + t.Run("failure on commit does not update real branch", func(t *testing.T) { + var updatedRealBranch bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") switch { - case strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/"): - json.NewEncoder(w).Encode(github.Commit{ - Tree: &github.Tree{SHA: github.Ptr("tree-sha")}, - }) - case r.URL.Path == "/repos/test-owner/test-repo/git/blobs": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Blob{SHA: github.Ptr("blob-sha")}) - case r.URL.Path == "/repos/test-owner/test-repo/git/trees": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Tree{SHA: github.Ptr("tree-sha")}) - case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/commits": - commitCount++ - if commitCount == 2 { - w.WriteHeader(http.StatusInternalServerError) - return - } + case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/refs": w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Commit{SHA: github.Ptr("commit-sha")}) - case r.Method == http.MethodPatch: + json.NewEncoder(w).Encode(github.Reference{ + Object: &github.GitObject{SHA: github.Ptr("initial")}, + }) + + case r.Method == http.MethodPatch && strings.Contains(r.URL.Path, "heads/test-branch"): + updatedRealBranch = true json.NewEncoder(w).Encode(github.Reference{ Object: &github.GitObject{SHA: github.Ptr("sha")}, }) + + case r.Method == http.MethodDelete: + w.WriteHeader(http.StatusNoContent) + + default: + w.WriteHeader(http.StatusNotFound) } })) defer server.Close() + commitCount := 0 client := newTestClient(t, server) + client.graphql = &mockGraphQL{ + handler: func(query string, variables map[string]any) (json.RawMessage, error) { + commitCount++ + if commitCount == 2 { + return nil, fmt.Errorf("simulated failure") + } + return signedGraphQLResponse("commit-sha"), nil + }, + } + changes := []Change{ {hash: "h1", message: "First", entries: map[string]FileEntry{"a.txt": newFileEntry([]byte("a"))}}, {hash: "h2", message: "Second", entries: map[string]FileEntry{"b.txt": newFileEntry([]byte("b"))}}, - {hash: "h3", message: "Third", entries: map[string]FileEntry{"c.txt": newFileEntry([]byte("c"))}}, } count, _, err := client.PushChanges(context.Background(), "initial", changes...) @@ -658,6 +674,9 @@ func TestPushChanges(t *testing.T) { if count != 2 { t.Errorf("expected count 2 (failed on second), got %d", count) } + if updatedRealBranch { + t.Error("real branch should not have been updated on failure") + } }) t.Run("update ref fails", func(t *testing.T) { @@ -665,26 +684,31 @@ func TestPushChanges(t *testing.T) { w.Header().Set("Content-Type", "application/json") switch { - case strings.HasPrefix(r.URL.Path, "/repos/test-owner/test-repo/git/commits/"): - json.NewEncoder(w).Encode(github.Commit{ - Tree: &github.Tree{SHA: github.Ptr("tree-sha")}, - }) - case r.URL.Path == "/repos/test-owner/test-repo/git/blobs": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Blob{SHA: github.Ptr("blob-sha")}) - case r.URL.Path == "/repos/test-owner/test-repo/git/trees": - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Tree{SHA: github.Ptr("tree-sha")}) - case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/commits": + case r.Method == http.MethodPost && r.URL.Path == "/repos/test-owner/test-repo/git/refs": w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(github.Commit{SHA: github.Ptr("commit-sha")}) + json.NewEncoder(w).Encode(github.Reference{ + Object: &github.GitObject{SHA: github.Ptr("initial")}, + }) + case r.Method == http.MethodPatch: w.WriteHeader(http.StatusConflict) + + case r.Method == http.MethodDelete: + w.WriteHeader(http.StatusNoContent) + + default: + w.WriteHeader(http.StatusNotFound) } })) defer server.Close() client := newTestClient(t, server) + client.graphql = &mockGraphQL{ + handler: func(query string, variables map[string]any) (json.RawMessage, error) { + return signedGraphQLResponse("commit-sha"), nil + }, + } + changes := []Change{ {hash: "h1", message: "First", entries: map[string]FileEntry{"a.txt": newFileEntry([]byte("a"))}}, } @@ -697,6 +721,50 @@ func TestPushChanges(t *testing.T) { t.Errorf("expected 'update ref' error, got %q", err.Error()) } }) + + t.Run("dry run skips throwaway branch", func(t *testing.T) { + client := &Client{ + owner: "test-owner", + repo: "test-repo", + branch: "test-branch", + dryrun: true, + } + + changes := []Change{ + {hash: "h1", message: "First", entries: map[string]FileEntry{"a.txt": newFileEntry([]byte("a"))}}, + } + + count, _, err := client.PushChanges(context.Background(), "initial", changes...) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if count != 1 { + t.Errorf("expected count 1, got %d", count) + } + }) +} + +func TestChooseStrategy(t *testing.T) { + tests := []struct { + name string + userToken bool + mode string + want commitStrategy + }{ + {"default mode any token", false, "100644", strategyGraphQL}, + {"empty mode any token", false, "", strategyGraphQL}, + {"executable with user token", true, "100755", strategyGraphQL}, + {"executable with non-user token", false, "100755", strategyREST}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &Client{userToken: tt.userToken} + change := Change{entries: map[string]FileEntry{"f": {Content: []byte("x"), Mode: tt.mode}}} + if got := client.chooseStrategy(change); got != tt.want { + t.Errorf("chooseStrategy() = %v, want %v", got, tt.want) + } + }) + } } func TestURLHelpers(t *testing.T) { diff --git a/token.go b/token.go index ac16273..927646e 100644 --- a/token.go +++ b/token.go @@ -1,5 +1,19 @@ package main +import "strings" + +// isUserToken returns true if the token appears to be a user-associated token +// (PAT, OAuth, or user-to-server) based on its prefix. These tokens produce +// signed commits when used with the GraphQL API. +func isUserToken(token string) bool { + for _, prefix := range []string{"ghp_", "gho_", "ghu_", "github_pat_"} { + if strings.HasPrefix(token, prefix) && len(token) > len(prefix) { + return true + } + } + return false +} + type envGetter func(string) string func getToken(getter envGetter) string { diff --git a/token_test.go b/token_test.go new file mode 100644 index 0000000..650578c --- /dev/null +++ b/token_test.go @@ -0,0 +1,27 @@ +package main + +import "testing" + +func TestIsUserToken(t *testing.T) { + tests := []struct { + token string + want bool + }{ + {"ghp_abc123", true}, + {"gho_abc123", true}, + {"ghu_abc123", true}, + {"github_pat_abc123", true}, + {"ghs_abc123", false}, + {"ghr_abc123", false}, + {"some-random-token", false}, + {"", false}, + {"ghp_", false}, // prefix only, no actual token content + } + for _, tt := range tests { + t.Run(tt.token, func(t *testing.T) { + if got := isUserToken(tt.token); got != tt.want { + t.Errorf("isUserToken(%q) = %v, want %v", tt.token, got, tt.want) + } + }) + } +} diff --git a/version.go b/version.go index 15e0e75..0e22c18 100644 --- a/version.go +++ b/version.go @@ -1,3 +1,3 @@ package main -const VERSION = "3.0.0" +const VERSION = "3.1.0"