diff --git a/docs/gh-post.md b/docs/gh-post.md index 1d9469cc..43dfacc4 100644 --- a/docs/gh-post.md +++ b/docs/gh-post.md @@ -119,6 +119,40 @@ After the identity check, the command reads the target's labels, and reads the r Support covers repositories whose owner-review label doesn't start with `change:`, and the command doesn't look up a repository's configuration to check. Because no devtools gate treats a `change:` label as an authority, every gate stays in place. A consumer that sets a `change:` label as its owner-review label falls outside this support, because an agent could then clear that hold. +### Rerun failed CI jobs + +Use this exact command form to request a rerun of failed jobs in one workflow run: + +```bash +gh-post run rerun-failed 123456789 --repo owner/project +``` + +Supply a positive decimal run ID without leading zeros, followed by `--repo` exactly as shown. Go's command allowlist rejects aliases, reordered or extra arguments, body flags, job selectors, full reruns, and cancellation. The command leaves stdin unread. Before reading the run or requesting a rerun, the existing machine `post_identities` guard verifies the caller. Returned run ID and repository URL must match the requested target. + +Only status `completed` with conclusion `failure`, `cancelled`, or `timed_out` qualifies. A state refusal exits 1 and prints status, conclusion, and attempt. + +An eligible request sends one POST to `/repos/{owner}/{repo}/actions/runs/{run_id}/rerun-failed-jobs`. Exit 0 prints the run URL after GitHub accepts it, without waiting for CI. + +#### Attempt reservations + +Before the POST, the command atomically creates and syncs a receipt under `${XDG_STATE_HOME:-~/.local/state}/devtools/gh-post-reruns/{owner}/{repo}/{run_id}/{run_attempt}`. It stores repository names in lowercase. Processes sharing that directory can't reserve the same attempt twice. A duplicate exits 1 without a POST. Once a later attempt completes with an eligible conclusion, the command can reserve it independently. + +Windows doesn't allow syncing a directory handle, so the receipt file's own sync is the strongest durability available there. + +Keep these receipts. They remain after success, failure, interruption, or audit failure. Removing one removes its duplicate protection. + +Before writing to GitHub, the command appends and syncs an audit record with UTC time, operation `run rerun-failed`, canonical run URL, `run_attempt`, and outcome `reserved`. This records intent, including requests that GitHub never receives. Audit fields omit body text and credentials. A reservation or audit failure prevents the POST and exits 2. + +#### Uncertain outcomes and guards + +If a write fails, the command reads the run back once, reports status, conclusion, and attempt (or a read failure), and exits 2. It never repeats the POST, polls, or removes the receipt. Diagnostics omit raw GitHub errors and response bodies. + +Inspect the run before further action. An accepted request can take time to appear. + +Receipts coordinate only callers sharing the state directory. GitHub doesn't support a conditional attempt parameter or a duplicate-request key for this endpoint. Another host or external client can race between read and POST. A crash after reservation can withhold a request GitHub never receives. Keeping the receipt prevents automatic repeats in either case. + +The PR skill's shell guard governs raw `gh pr` commands and already permits `gh-post`. The machine posting hook also permits the wrapper by basename. Both keep their existing restrictions. The Go parser admits the exact new run form without shell changes. + ## Results and audit | Exit | Meaning | diff --git a/internal/ghpost/args.go b/internal/ghpost/args.go index 16e53a8e..f1203550 100644 --- a/internal/ghpost/args.go +++ b/internal/ghpost/args.go @@ -11,7 +11,7 @@ import ( ) var errUsage = errors.New( - "expected issue create|comment|edit|label|close|reopen, pr create|edit|comment|review|ready|close|label, comment edit, review edit, or release create|edit; use only supported flags and file input", + "expected issue create|comment|edit|label|close|reopen, pr create|edit|comment|review|ready|close|label, comment edit, review edit, release create|edit, or run rerun-failed --repo owner/repo; use only supported flags and file input", ) type request struct { @@ -58,6 +58,9 @@ const ( ) func parse(args []string) (request, error) { + if len(args) > 0 && args[0] == "run" { + return parseRerunFailed(args) + } r := request{} if len(args) < operationWords { return r, errUsage diff --git a/internal/ghpost/help.go b/internal/ghpost/help.go index 1064f248..e6dfeb37 100644 --- a/internal/ghpost/help.go +++ b/internal/ghpost/help.go @@ -18,7 +18,7 @@ Global flags: --help Show all operations and flags without network access. --check-deps Check git and gh on PATH without executing them. -All operations accept -R, --repo OWNER/REPO. +Operations accept -R, --repo OWNER/REPO unless an exact form is shown. Content operations accept --body-file PATH and default to stdin; --body-file - also reads stdin. PR ready and close never read stdin. An issue edit with --type reads no stdin unless --body-file is given. @@ -47,6 +47,14 @@ Operations and additional flags: review edit release create --title TEXT --target REF --draft --prerelease --verify-tag release edit --title TEXT --draft --prerelease + run rerun-failed --repo OWNER/REPO + +Run rerun-failed accepts exactly that form, with a positive decimal run ID, +and reads no stdin. Only completed failure, cancelled or timed_out runs +qualify. It reserves each run attempt in machine state before one failed-jobs +POST and audits the reservation. A repeated attempt is refused. After an +uncertain write it reads the run once, exits 2, and never retries the write. +It never reruns every job, cancels a run, or polls. Keep the state receipts. New reviews require a 40-hex --commit matching the current PR head and first body line Reviewed head: (the SHA may be wrapped in backticks). diff --git a/internal/ghpost/rerun.go b/internal/ghpost/rerun.go new file mode 100644 index 00000000..98833083 --- /dev/null +++ b/internal/ghpost/rerun.go @@ -0,0 +1,167 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Tony Burns + +package ghpost + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "time" +) + +const runRerunFailed = "run rerun-failed" + +var errRerunReserved = errors.New("rerun already reserved for this run attempt; do not retry") + +// This is the entire run-command allowlist: no aliases, inferred target, +// extra flags, full rerun, or cancellation can reach the write path. +func parseRerunFailed(args []string) (request, error) { + if len(args) != 5 || args[1] != "rerun-failed" || args[3] != repoFlag || + !regexp.MustCompile(`^[1-9][0-9]*$`).MatchString(args[2]) || + !regexp.MustCompile(`^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`).MatchString(args[4]) { + return request{}, errUsage + } + if _, err := strconv.ParseUint(args[2], 10, 64); err != nil { + return request{}, errUsage + } + return request{operation: runRerunFailed, operand: args[2], repo: args[4]}, nil +} + +type workflowRun struct { + ID uint64 `json:"id"` + Attempt uint64 `json:"run_attempt"` + Status string `json:"status"` + Conclusion string `json:"conclusion"` + URL string `json:"html_url"` +} + +func (r *request) runURL() string { + return "https://github.com/" + strings.TrimPrefix(r.runEndpoint(), "repos/") +} + +func (r *request) runEndpoint() string { + return "repos/" + r.repo + "/actions/runs/" + r.operand +} + +func (r *request) readRun(ctx context.Context) (workflowRun, error) { + data, err := gh(ctx, []string{apiCommand, methodFlag, "GET", r.runEndpoint()}) + var state workflowRun + if err != nil || json.Unmarshal(data, &state) != nil || state.Attempt == 0 || state.Status == "" || + strconv.FormatUint(state.ID, 10) != r.operand || !strings.EqualFold(state.URL, r.runURL()) { + return workflowRun{}, errEnvironment + } + return state, nil +} + +func printRunState(out io.Writer, state workflowRun) { + fmt.Fprintf(out, "status=%q conclusion=%q attempt=%d", state.Status, state.Conclusion, state.Attempt) +} + +func (r *request) rerunFailed( + ctx context.Context, log *os.File, out, errOut io.Writer, syncFile func(*os.File) error, +) int { + state, err := r.readRun(ctx) + if err != nil { + fmt.Fprintln(errOut, "gh-post: cannot read exact workflow run") + return exitEnvironment + } + if state.Status != "completed" || + (state.Conclusion != "failure" && state.Conclusion != "cancelled" && state.Conclusion != "timed_out") { + fmt.Fprint(errOut, "gh-post: run refused: ") + printRunState(errOut, state) + fmt.Fprintln(errOut) + return 1 + } + if err = r.reserveRerun(filepath.Dir(log.Name()), state.Attempt, syncFile); err != nil { + if errors.Is(err, errRerunReserved) { + fmt.Fprintln(errOut, "gh-post:", err) + return 1 + } + fmt.Fprintln(errOut, "gh-post: cannot reserve run attempt; no rerun requested") + return exitEnvironment + } + if err = r.auditRerun(log, state.Attempt); err != nil { + fmt.Fprintln(errOut, "gh-post: cannot audit reservation; no rerun requested; do not retry this attempt") + return exitEnvironment + } + _, err = gh(ctx, []string{apiCommand, methodFlag, "POST", r.runEndpoint() + "/rerun-failed-jobs"}) + if err != nil { + // A transport failure can follow acceptance. Keep the reservation, + // read once for evidence, and never repeat the POST. + after, readErr := r.readRun(ctx) + fmt.Fprint(errOut, "gh-post: rerun outcome uncertain; do not retry this attempt; ") + if readErr != nil { + fmt.Fprintln(errOut, "cannot read run back") + } else { + printRunState(errOut, after) + fmt.Fprintln(errOut) + } + return exitEnvironment + } + fmt.Fprintln(out, r.runURL()) + return 0 +} + +func (r *request) reserveRerun(dir string, attempt uint64, syncFile func(*os.File) error) error { + dir = filepath.Join(dir, "gh-post-reruns", strings.ToLower(r.repo), r.operand) + if err := os.MkdirAll(dir, 0o700); err != nil { + return fmt.Errorf("reserve directory: %w", err) + } + // O_EXCL prevents concurrent processes from reserving the same attempt, + // including after an uncertain result. Keep receipts after write or sync failures. + file, err := os.OpenFile( + filepath.Join(dir, strconv.FormatUint(attempt, 10)), + os.O_WRONLY|os.O_CREATE|os.O_EXCL, + 0o600, + ) + if errors.Is(err, os.ErrExist) { + return errRerunReserved + } + if err != nil { + return fmt.Errorf("reserve attempt: %w", err) + } + defer file.Close() + if _, err = file.WriteString("reserved\n"); err != nil { + return fmt.Errorf("write reservation: %w", err) + } + if err = syncFile(file); err != nil { + return fmt.Errorf("sync reservation: %w", err) + } + // Persist the receipt name and each directory entry created by MkdirAll, + // including ancestors created during audit setup, before any POST. + for { + if err = syncDir(dir, syncFile); err != nil { + return fmt.Errorf("sync reservation directory: %w", err) + } + if filepath.Dir(dir) == dir { + break + } + dir = filepath.Dir(dir) + } + return nil +} + +func (r *request) auditRerun(log *os.File, attempt uint64) error { + record := struct { + Time string `json:"time"` + Operation string `json:"operation"` + URL string `json:"url"` + Attempt uint64 `json:"run_attempt"` + Outcome string `json:"outcome"` + }{time.Now().UTC().Format(time.RFC3339Nano), r.operation, r.runURL(), attempt, "reserved"} + if err := json.NewEncoder(log).Encode(record); err != nil { + return fmt.Errorf("audit reservation: %w", err) + } + if err := log.Sync(); err != nil { + return fmt.Errorf("sync audit: %w", err) + } + return nil +} diff --git a/internal/ghpost/rerun_internal_test.go b/internal/ghpost/rerun_internal_test.go new file mode 100644 index 00000000..e07abed2 --- /dev/null +++ b/internal/ghpost/rerun_internal_test.go @@ -0,0 +1,437 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Tony Burns + +package ghpost + +import ( + "bytes" + "encoding/json" + "flag" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +const rerunURL = "https://github.com/owner/repo/actions/runs/123" + +func rerunArguments() []string { + return []string{"run", "rerun-failed", "123", "--repo", "owner/repo"} +} + +// Each CLI case runs in a subprocess so its environment fixture can't affect +// other cases or tests running in parallel in the parent process. +func rerunCases() map[string]func(*testing.T) { + return map[string]func(*testing.T){ + "ExactForm": testRerunFailedExactForm, + "EligibleAndAudited": testRerunFailedEligibleAndAudited, + "StateRefusals": testRerunFailedStateRefusals, + "ExactRunAndIdentity": testRerunFailedExactRunAndIdentity, + "UncertainAndDuplicate": testRerunFailedUncertainAndDuplicate, + "NewAttempt": testRerunFailedNewAttempt, + "ReservationAndAuditFailures": testRerunFailedReservationAndAuditFailures, + "NoStdinAndHelp": testRerunFailedNoStdinAndHelp, + "PRGuard": testRerunFailedPRGuard, + "Durability": testRerunFailedDurability, + "SyncFailures": testRerunFailedSyncFailures, + } +} + +func TestRerunFailed(t *testing.T) { + t.Parallel() + var childLock rerunProcessLock + for name := range rerunCases() { + t.Run(name, func(t *testing.T) { + t.Parallel() + binary, err := os.Executable() + require.NoError(t, err) + cmd := exec.CommandContext(t.Context(), binary, "-test.run=^TestRerunFailedProcess$") + // Merge subprocess counters into the parent test's coverage data. + if coverageDir := flag.Lookup("test.gocoverdir").Value.String(); coverageDir != "" { + cmd.Args = append(cmd.Args, "-test.gocoverdir="+coverageDir) + } + cmd.Env = append(os.Environ(), "GHPOST_RERUN_CASE="+name) + childLock.Lock() + output, err := cmd.CombinedOutput() + childLock.Unlock() + require.NoError(t, err, "%s", output) + }) + } +} + +// Only the subprocess runs environment-dependent cases. Clear the selector +// before executing the case so its child commands can't inherit it. +func TestRerunFailedProcess(t *testing.T) { + name := os.Getenv("GHPOST_RERUN_CASE") + if name == "" { + return + } + t.Setenv("GHPOST_RERUN_CASE", "") + test, ok := rerunCases()[name] + require.True(t, ok, "unknown rerun case") + test(t) +} + +func setupRerun(t *testing.T) string { + t.Helper() + dir := setup(t) + t.Setenv("RERUN_DIR", dir) + t.Setenv("RERUN_ERROR", "0") + writeRun(t, dir, "before", "completed", "failure", 1) + writeRun(t, dir, "after", "queued", "", 2) + write(t, filepath.Join(dir, "gh"), `#!/bin/sh +printf '%s\n' "$*" >> "$CALLS" +case "$*" in + 'api user --jq .login') echo "${LOGIN:-agent}"; exit "${IDENTITY_STATUS:-0}" ;; + 'api graphql '*) echo 'app[bot]'; exit "${GRAPHQL_STATUS:-1}" ;; + 'api --method GET repos/owner/repo/actions/runs/123') + if [ -f "$RERUN_DIR/posted" ]; then cat "$RERUN_DIR/after"; else cat "$RERUN_DIR/before"; fi ;; + 'api --method POST repos/owner/repo/actions/runs/123/rerun-failed-jobs') + touch "$RERUN_DIR/posted" + echo 'ghp_abcdefghijklmnopqrstuvwxyz1234567890' >&2 + exit "$RERUN_ERROR" ;; + *) exit 99 ;; +esac +`) + return dir +} + +func writeRun(t *testing.T, dir, file, status, conclusion string, attempt uint64) { + t.Helper() + data, err := json.Marshal( + workflowRun{ID: 123, Attempt: attempt, Status: status, Conclusion: conclusion, URL: rerunURL}, + ) + require.NoError(t, err) + write(t, filepath.Join(dir, file), string(data)) +} + +func testRerunFailedExactForm(t *testing.T) { + t.Helper() + for _, args := range [][]string{ + {"run"}, + {"run", "rerun", "123", "--repo", "owner/repo"}, + {"run", "cancel", "123", "--repo", "owner/repo"}, + {"run", "rerun-failed", "123"}, + {"run", "rerun-failed", "123", "-R", "owner/repo"}, + {"run", "rerun-failed", "123", "--repo=owner/repo"}, + {"run", "rerun-failed", "123", "--repo", "https://github.com/owner/repo"}, + {"run", "rerun-failed", "123", "--repo", "owner/repo", "--repo", "other/repo"}, + {"run", "rerun-failed", "123", "--repo", "owner/repo", "--body-file", "-"}, + {"run", "rerun-failed", "123", "--repo", "owner/repo", "--failed"}, + {"run", "rerun-failed", "123", "--repo", "owner/repo", "--job", "456"}, + {"run", "rerun-failed", "123", "--repo", "owner/repo", "--dry-run"}, + {"run", "rerun-failed", "0", "--repo", "owner/repo"}, + {"run", "rerun-failed", "0123", "--repo", "owner/repo"}, + {"run", "rerun-failed", "-123", "--repo", "owner/repo"}, + {"run", "rerun-failed", "123/attempts/1", "--repo", "owner/repo"}, + {"run", "rerun-failed", "18446744073709551616", "--repo", "owner/repo"}, + } { + t.Run(strings.Join(args, " "), func(t *testing.T) { + dir := setupRerun(t) + code, _, diagnostic := run(t, "", args...) + require.Equal(t, 2, code, diagnostic) + require.NoFileExists(t, filepath.Join(dir, "calls")) + }) + } +} + +func testRerunFailedEligibleAndAudited(t *testing.T) { + t.Helper() + for _, conclusion := range []string{"failure", "cancelled", "timed_out"} { + t.Run(conclusion, func(t *testing.T) { + dir := setupRerun(t) + writeRun(t, dir, "before", "completed", conclusion, 7) + code, out, diagnostic := run(t, "ignored", rerunArguments()...) + require.Equal(t, 0, code, diagnostic) + require.Equal(t, rerunURL+"\n", out) + require.Empty(t, diagnostic) + require.Equal(t, "api user --jq .login\napi --method GET repos/owner/repo/actions/runs/123\n"+ + "api --method POST repos/owner/repo/actions/runs/123/rerun-failed-jobs\n", fileText(t, filepath.Join(dir, "calls"))) + var record struct { + auditRecord + + Attempt uint64 `json:"run_attempt"` + Outcome string `json:"outcome"` + } + audit := fileText(t, filepath.Join(dir, "state", "devtools", "gh-post.log")) + require.NoError(t, json.Unmarshal([]byte(audit), &record)) + require.Equal(t, runRerunFailed, record.Operation) + require.Equal(t, rerunURL, record.URL) + require.Equal(t, uint64(7), record.Attempt) + require.Equal(t, "reserved", record.Outcome) + _, err := time.Parse(time.RFC3339Nano, record.Time) + require.NoError(t, err) + require.NotContains(t, audit, "ghp_") + }) + } +} + +func testRerunFailedStateRefusals(t *testing.T) { + t.Helper() + for _, pair := range [][2]string{ + {"completed", "success"}, + {"completed", "skipped"}, + {"completed", "neutral"}, + {"completed", "action_required"}, + {"completed", "stale"}, + {"completed", ""}, + {"queued", "failure"}, + {"in_progress", "failure"}, + {"waiting", ""}, + } { + t.Run(pair[0]+"-"+pair[1], func(t *testing.T) { + dir := setupRerun(t) + writeRun(t, dir, "before", pair[0], pair[1], 1) + code, _, diagnostic := run(t, "", rerunArguments()...) + require.Equal(t, 1, code, diagnostic) + require.Contains(t, diagnostic, `status="`+pair[0]+`" conclusion="`+pair[1]+`"`) + require.NotContains(t, fileText(t, filepath.Join(dir, "calls")), "POST") + }) + } +} + +func testRerunFailedExactRunAndIdentity(t *testing.T) { + t.Helper() + for _, body := range []string{ + `{}`, `invalid`, + `{"id":456,"run_attempt":1,"status":"completed","conclusion":"failure","html_url":"` + rerunURL + `"}`, + `{"id":123,"run_attempt":1,"status":"completed","conclusion":"failure","html_url":"https://github.com/other/repo/actions/runs/123"}`, + `{"id":123,"run_attempt":0,"status":"completed","conclusion":"failure","html_url":"` + rerunURL + `"}`, + } { + t.Run(body, func(t *testing.T) { + dir := setupRerun(t) + write(t, filepath.Join(dir, "before"), body) + code, _, diagnostic := run(t, "", rerunArguments()...) + require.Equal(t, 2, code, diagnostic) + require.NotContains(t, fileText(t, filepath.Join(dir, "calls")), "POST") + }) + } + for _, login := range []string{"outsider", "unreadable"} { + t.Run(login, func(t *testing.T) { + dir := setupRerun(t) + t.Setenv("LOGIN", login) + if login == "unreadable" { + t.Setenv("IDENTITY_STATUS", "1") + } + code, _, diagnostic := run(t, "", rerunArguments()...) + require.NotZero(t, code, diagnostic) + require.NotContains(t, fileText(t, filepath.Join(dir, "calls")), "actions/runs") + }) + } +} + +func testRerunFailedUncertainAndDuplicate(t *testing.T) { + t.Helper() + for _, after := range []string{"accepted", "unchanged", "unreadable"} { + t.Run(after, func(t *testing.T) { + dir := setupRerun(t) + t.Setenv("RERUN_ERROR", "1") + switch after { + case "unchanged": + writeRun(t, dir, "after", "completed", "failure", 1) + case "unreadable": + write(t, filepath.Join(dir, "after"), "invalid") + } + code, _, diagnostic := run(t, "", rerunArguments()...) + require.Equal(t, 2, code, diagnostic) + require.Contains(t, diagnostic, "outcome uncertain; do not retry") + require.NotContains(t, diagnostic, "ghp_") + switch after { + case "accepted": + require.Contains(t, diagnostic, `status="queued" conclusion="" attempt=2`) + case "unchanged": + require.Contains(t, diagnostic, `status="completed" conclusion="failure" attempt=1`) + default: + require.Contains(t, diagnostic, "cannot read run back") + } + calls := fileText(t, filepath.Join(dir, "calls")) + require.Equal(t, 2, strings.Count(calls, "GET repos/")) + require.Equal(t, 1, strings.Count(calls, "POST repos/")) + // A fresh invocation sees the old attempt again, even if the + // read-back was delayed or unavailable after the first POST. + require.NoError(t, os.Remove(filepath.Join(dir, "posted"))) + code, _, diagnostic = run(t, "", rerunArguments()...) + require.Equal(t, 1, code, diagnostic) + require.Contains(t, diagnostic, "already reserved") + require.Equal(t, 1, strings.Count(fileText(t, filepath.Join(dir, "calls")), "POST repos/")) + }) + } +} + +func testRerunFailedNewAttempt(t *testing.T) { + t.Helper() + dir := setupRerun(t) + code, _, diagnostic := run(t, "", rerunArguments()...) + require.Zero(t, code, diagnostic) + require.NoError(t, os.Remove(filepath.Join(dir, "posted"))) + code, _, diagnostic = run(t, "", rerunArguments()...) + require.Equal(t, 1, code, diagnostic) + writeRun(t, dir, "before", "completed", "failure", 2) + code, _, diagnostic = run(t, "", rerunArguments()...) + require.Zero(t, code, diagnostic) + require.Equal(t, 2, strings.Count(fileText(t, filepath.Join(dir, "calls")), "POST repos/")) +} + +func testRerunFailedReservationAndAuditFailures(t *testing.T) { + t.Helper() + for _, failure := range []string{"reservation", "audit-open", "audit-append"} { + t.Run(failure, func(t *testing.T) { + dir := setupRerun(t) + stateDir := filepath.Join(dir, "state", "devtools") + var code int + var diagnostic bytes.Buffer + if failure == "audit-open" { + require.NoError(t, os.MkdirAll(filepath.Join(stateDir, "gh-post.log"), 0o700)) + } + if failure == "reservation" { + write(t, filepath.Join(stateDir, "gh-post-reruns"), "not a directory") + } + if failure == "audit-append" { + log, err := openAudit() + require.NoError(t, err) + require.NoError(t, log.Close()) + r, err := parse(rerunArguments()) + require.NoError(t, err) + code = r.rerunFailed(t.Context(), log, &bytes.Buffer{}, &diagnostic, (*os.File).Sync) + } else { + code, _, _ = run(t, "", rerunArguments()...) + } + require.Equal(t, 2, code, diagnostic.String()) + require.NotContains(t, fileText(t, filepath.Join(dir, "calls")), "POST") + }) + } +} + +func TestRerunReservationConcurrent(t *testing.T) { + t.Parallel() + dir := t.TempDir() + r, err := parse(rerunArguments()) + require.NoError(t, err) + results := make(chan error, 8) + for range 8 { + go func() { results <- r.reserveRerun(dir, 1, (*os.File).Sync) }() + } + successes := 0 + for range 8 { + err = <-results + if err == nil { + successes++ + } else { + require.ErrorIs(t, err, errRerunReserved) + } + } + require.Equal(t, 1, successes) + r.repo = "OWNER/REPO" + require.ErrorIs(t, r.reserveRerun(dir, 1, (*os.File).Sync), errRerunReserved) +} + +func testRerunFailedNoStdinAndHelp(t *testing.T) { + t.Helper() + setupRerun(t) + var out, diagnostic bytes.Buffer + code := Run(t.Context(), rerunArguments(), brokenIO{}, &out, &diagnostic) + require.Zero(t, code, diagnostic.String()) + require.Contains(t, usage, "run rerun-failed --repo OWNER/REPO") +} + +func testRerunFailedPRGuard(t *testing.T) { + t.Helper() + setup(t) + for _, command := range []string{ + "gh-post run rerun-failed 123 --repo owner/repo", + "gh pr edit 12 --title example", + } { + payload, err := json.Marshal(map[string]any{"tool_input": map[string]string{"command": command}}) + require.NoError(t, err) + cmd := exec.CommandContext( + t.Context(), + "bash", + "../../packages/agents/source/skills/pr/claude/scripts/guard-gh.sh", + ) + cmd.Stdin = bytes.NewReader(payload) + output, err := cmd.CombinedOutput() + if strings.HasPrefix(command, "gh-post ") { + require.NoError(t, err, "%s", output) + } else { + var exit *exec.ExitError + require.ErrorAs(t, err, &exit) + require.Equal(t, 2, exit.ExitCode()) + } + } +} + +func testRerunFailedDurability(t *testing.T) { + t.Helper() + dir := setupRerun(t) + stateDir := filepath.Join(dir, "state", "devtools") + log, err := openAudit() + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, log.Close()) }) + r, err := parse(rerunArguments()) + require.NoError(t, err) + var events []string + syncFile := func(file *os.File) error { + info, statErr := file.Stat() + require.NoError(t, statErr) + name := file.Name() + if !info.IsDir() { + // The real O_EXCL create and write have already happened. + require.Equal(t, "reserved\n", fileText(t, file.Name())) + events = append(events, "create") + name = "file" + } + events = append(events, "sync "+name) + // No write may precede any of the required syncs. + require.NotContains(t, fileText(t, filepath.Join(dir, "calls")), "POST") + return file.Sync() + } + var out, diagnostic bytes.Buffer + code := r.rerunFailed(t.Context(), log, &out, &diagnostic, syncFile) + require.Zero(t, code, diagnostic.String()) + require.Equal(t, 1, strings.Count(fileText(t, filepath.Join(dir, "calls")), "POST")) + events = append(events, "POST") + expected := []string{"create", "sync file"} + if dirSyncSupported { + for path := filepath.Join(stateDir, "gh-post-reruns", "owner", "repo", "123"); ; path = filepath.Dir(path) { + expected = append(expected, "sync "+path) + if filepath.Dir(path) == path { + break + } + } + } + require.Equal(t, append(expected, "POST"), events) +} + +func testRerunFailedSyncFailures(t *testing.T) { + t.Helper() + failures := []string{"1"} + if dirSyncSupported { + failures = append(failures, "123", "repo", "owner", "gh-post-reruns", "devtools", "state") + } + for _, failure := range failures { + t.Run("sync-"+failure, func(t *testing.T) { + dir := setupRerun(t) + stateDir := filepath.Join(dir, "state", "devtools") + log, err := openAudit() + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, log.Close()) }) + r, err := parse(rerunArguments()) + require.NoError(t, err) + syncFile := func(file *os.File) error { + if filepath.Base(file.Name()) == failure { + return os.ErrPermission + } + return file.Sync() + } + var out, diagnostic bytes.Buffer + code := r.rerunFailed(t.Context(), log, &out, &diagnostic, syncFile) + require.Equal(t, 2, code, diagnostic.String()) + require.NotContains(t, fileText(t, filepath.Join(dir, "calls")), "POST") + require.ErrorIs(t, r.reserveRerun(stateDir, 1, (*os.File).Sync), errRerunReserved) + }) + } +} diff --git a/internal/ghpost/rerun_process_internal_test.go b/internal/ghpost/rerun_process_internal_test.go new file mode 100644 index 00000000..e889f734 --- /dev/null +++ b/internal/ghpost/rerun_process_internal_test.go @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Tony Burns + +package ghpost + +import ( + "runtime" + "sync" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" +) + +// Windows children must not write the shared coverage metadata concurrently. +type rerunProcessLock struct { + mu sync.Mutex +} + +func (l *rerunProcessLock) Lock() { + if runtime.GOOS == "windows" { + l.mu.Lock() + } +} + +func (l *rerunProcessLock) Unlock() { + if runtime.GOOS == "windows" { + l.mu.Unlock() + } +} + +func TestRerunProcessLock(t *testing.T) { + t.Parallel() + if runtime.GOOS != "windows" { + var lock rerunProcessLock + lock.mu.Lock() + defer lock.mu.Unlock() + lock.Lock() + require.False(t, lock.mu.TryLock()) + lock.Unlock() + require.False(t, lock.mu.TryLock()) + return + } + var lock rerunProcessLock + var active atomic.Int32 + counts := make(chan int32, 32) + for range cap(counts) { + go func() { + lock.Lock() + counts <- active.Add(1) + runtime.Gosched() + active.Add(-1) + lock.Unlock() + }() + } + for range cap(counts) { + require.Equal(t, int32(1), <-counts) + } +} diff --git a/internal/ghpost/rerun_syncdir.go b/internal/ghpost/rerun_syncdir.go new file mode 100644 index 00000000..1165119e --- /dev/null +++ b/internal/ghpost/rerun_syncdir.go @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Tony Burns + +package ghpost + +import ( + "fmt" + "os" + "runtime" +) + +const dirSyncSupported = runtime.GOOS != "windows" + +func syncDir(dir string, syncFile func(*os.File) error) error { + // Windows refuses directory handle syncs; the receipt file sync is the + // strongest durability available there. + if !dirSyncSupported { + return nil + } + parent, err := os.Open(dir) + if err != nil { + return fmt.Errorf("open directory: %w", err) + } + syncErr := syncFile(parent) + closeErr := parent.Close() + if syncErr != nil { + return syncErr + } + if closeErr != nil { + return fmt.Errorf("close directory: %w", closeErr) + } + return nil +} diff --git a/internal/ghpost/run.go b/internal/ghpost/run.go index 050fde65..7ad8b4ae 100644 --- a/internal/ghpost/run.go +++ b/internal/ghpost/run.go @@ -216,6 +216,9 @@ func post(ctx context.Context, r request, body []byte, identities string, out, e return exitEnvironment } defer log.Close() + if r.operation == runRerunFailed { + return r.rerunFailed(ctx, log, out, errOut, (*os.File).Sync) + } if r.isIssueStateChange() { return r.changeIssueState(ctx, body, log, out, errOut) } diff --git a/internal/ghpost/state.go b/internal/ghpost/state.go index 30b998a6..dff3b0f2 100644 --- a/internal/ghpost/state.go +++ b/internal/ghpost/state.go @@ -27,7 +27,8 @@ var ( ) func (r *request) readInput(in io.Reader) ([]byte, error) { - if r.operation == prReady || r.isLabelChange() || r.issueType != "" && r.bodyFile == "" { + if r.operation == runRerunFailed || r.operation == prReady || r.isLabelChange() || + r.issueType != "" && r.bodyFile == "" { return nil, nil } path := r.bodyFile