From e1ae16698ed33de9abdcff5b18daea839ccb6a0a Mon Sep 17 00:00:00 2001 From: "tbhb.dev Agent" <334376894+tbhb-agent@users.noreply.github.com> Date: Sun, 1 Nov 2026 11:58:31 -0500 Subject: [PATCH 1/5] feat(gh-post): rerun failed jobs once per run attempt An agent needs a permitted route to recover failed CI without a human click or another push. Reserving the exact run attempt before requesting failed jobs prevents repeated writes when GitHub's response is uncertain. The reservation stays in place even when the request may not have arrived. Related to #1052. Assisted-by: codex:gpt-6 Signed-off-by: Tony Burns --- docs/gh-post.md | 32 +++ internal/ghpost/args.go | 5 +- internal/ghpost/help.go | 10 +- internal/ghpost/rerun.go | 154 +++++++++++++ internal/ghpost/rerun_internal_test.go | 302 +++++++++++++++++++++++++ internal/ghpost/run.go | 3 + internal/ghpost/state.go | 3 +- 7 files changed, 506 insertions(+), 3 deletions(-) create mode 100644 internal/ghpost/rerun.go create mode 100644 internal/ghpost/rerun_internal_test.go diff --git a/docs/gh-post.md b/docs/gh-post.md index 1d9469cc..c4b37797 100644 --- a/docs/gh-post.md +++ b/docs/gh-post.md @@ -119,6 +119,38 @@ 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. + +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..a7ff9c44 --- /dev/null +++ b/internal/ghpost/rerun.go @@ -0,0 +1,154 @@ +// 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) 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); 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) 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 = file.Sync(); err != nil { + return fmt.Errorf("sync reservation: %w", err) + } + 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..5dd39b08 --- /dev/null +++ b/internal/ghpost/rerun_internal_test.go @@ -0,0 +1,302 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Tony Burns + +//nolint:paralleltest // The recording gh fixture changes the process environment. +package ghpost + +import ( + "bytes" + "encoding/json" + "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"} +} + +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) { + 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) { + 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) { + 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) { + 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) { + 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) { + 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) { + 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) + } 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) { + 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) }() + } + 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), errRerunReserved) +} + +func TestRerunFailedNoStdinAndHelp(t *testing.T) { + 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) { + 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) + err = cmd.Run() + if strings.HasPrefix(command, "gh-post ") { + require.NoError(t, err) + } else { + var exit *exec.ExitError + require.ErrorAs(t, err, &exit) + require.Equal(t, 2, exit.ExitCode()) + } + } +} diff --git a/internal/ghpost/run.go b/internal/ghpost/run.go index 050fde65..cec80309 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) + } 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 From 1a59a92de89cd0407431d9a219e2c6dd6d5ac7b5 Mon Sep 17 00:00:00 2001 From: "tbhb.dev Agent" <334376894+tbhb-agent@users.noreply.github.com> Date: Sun, 1 Nov 2026 13:42:36 -0500 Subject: [PATCH 2/5] fix(gh-post): sync directories before requesting failed jobs Syncing a receipt alone doesn't persist its name. After a crash, a later caller could request the same attempt again. The reservation must survive interruption before the request leaves the machine. Environment fixtures need separate processes so their configuration can't interfere with parallel cases or other tests. Related to #1052. Assisted-by: codex:gpt-6 Signed-off-by: Tony Burns --- internal/ghpost/rerun.go | 30 ++++- internal/ghpost/rerun_internal_test.go | 156 ++++++++++++++++++++++--- internal/ghpost/run.go | 2 +- 3 files changed, 168 insertions(+), 20 deletions(-) diff --git a/internal/ghpost/rerun.go b/internal/ghpost/rerun.go index a7ff9c44..9ab022b2 100644 --- a/internal/ghpost/rerun.go +++ b/internal/ghpost/rerun.go @@ -65,7 +65,9 @@ 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) int { +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") @@ -78,7 +80,7 @@ func (r *request) rerunFailed(ctx context.Context, log *os.File, out, errOut io. fmt.Fprintln(errOut) return 1 } - if err = r.reserveRerun(filepath.Dir(log.Name()), state.Attempt); err != nil { + 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 @@ -108,7 +110,7 @@ func (r *request) rerunFailed(ctx context.Context, log *os.File, out, errOut io. return 0 } -func (r *request) reserveRerun(dir string, attempt uint64) error { +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) @@ -130,9 +132,29 @@ func (r *request) reserveRerun(dir string, attempt uint64) error { if _, err = file.WriteString("reserved\n"); err != nil { return fmt.Errorf("write reservation: %w", err) } - if err = file.Sync(); err != nil { + 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 { + parent, openErr := os.Open(dir) + if openErr != nil { + return fmt.Errorf("open reservation directory: %w", openErr) + } + syncErr := syncFile(parent) + closeErr := parent.Close() + if syncErr != nil { + return fmt.Errorf("sync reservation directory: %w", syncErr) + } + if closeErr != nil { + return fmt.Errorf("close reservation directory: %w", closeErr) + } + if filepath.Dir(dir) == dir { + break + } + dir = filepath.Dir(dir) + } return nil } diff --git a/internal/ghpost/rerun_internal_test.go b/internal/ghpost/rerun_internal_test.go index 5dd39b08..f7d02559 100644 --- a/internal/ghpost/rerun_internal_test.go +++ b/internal/ghpost/rerun_internal_test.go @@ -1,12 +1,12 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright Tony Burns -//nolint:paralleltest // The recording gh fixture changes the process environment. package ghpost import ( "bytes" "encoding/json" + "flag" "os" "os/exec" "path/filepath" @@ -23,6 +23,56 @@ 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() + 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) + output, err := cmd.CombinedOutput() + 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) @@ -56,7 +106,8 @@ func writeRun(t *testing.T, dir, file, status, conclusion string, attempt uint64 write(t, filepath.Join(dir, file), string(data)) } -func TestRerunFailedExactForm(t *testing.T) { +func testRerunFailedExactForm(t *testing.T) { + t.Helper() for _, args := range [][]string{ {"run"}, {"run", "rerun", "123", "--repo", "owner/repo"}, @@ -85,7 +136,8 @@ func TestRerunFailedExactForm(t *testing.T) { } } -func TestRerunFailedEligibleAndAudited(t *testing.T) { +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) @@ -115,7 +167,8 @@ func TestRerunFailedEligibleAndAudited(t *testing.T) { } } -func TestRerunFailedStateRefusals(t *testing.T) { +func testRerunFailedStateRefusals(t *testing.T) { + t.Helper() for _, pair := range [][2]string{ {"completed", "success"}, {"completed", "skipped"}, @@ -138,7 +191,8 @@ func TestRerunFailedStateRefusals(t *testing.T) { } } -func TestRerunFailedExactRunAndIdentity(t *testing.T) { +func testRerunFailedExactRunAndIdentity(t *testing.T) { + t.Helper() for _, body := range []string{ `{}`, `invalid`, `{"id":456,"run_attempt":1,"status":"completed","conclusion":"failure","html_url":"` + rerunURL + `"}`, @@ -167,7 +221,8 @@ func TestRerunFailedExactRunAndIdentity(t *testing.T) { } } -func TestRerunFailedUncertainAndDuplicate(t *testing.T) { +func testRerunFailedUncertainAndDuplicate(t *testing.T) { + t.Helper() for _, after := range []string{"accepted", "unchanged", "unreadable"} { t.Run(after, func(t *testing.T) { dir := setupRerun(t) @@ -204,7 +259,8 @@ func TestRerunFailedUncertainAndDuplicate(t *testing.T) { } } -func TestRerunFailedNewAttempt(t *testing.T) { +func testRerunFailedNewAttempt(t *testing.T) { + t.Helper() dir := setupRerun(t) code, _, diagnostic := run(t, "", rerunArguments()...) require.Zero(t, code, diagnostic) @@ -217,7 +273,8 @@ func TestRerunFailedNewAttempt(t *testing.T) { require.Equal(t, 2, strings.Count(fileText(t, filepath.Join(dir, "calls")), "POST repos/")) } -func TestRerunFailedReservationAndAuditFailures(t *testing.T) { +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) @@ -236,7 +293,7 @@ func TestRerunFailedReservationAndAuditFailures(t *testing.T) { require.NoError(t, log.Close()) r, err := parse(rerunArguments()) require.NoError(t, err) - code = r.rerunFailed(t.Context(), log, &bytes.Buffer{}, &diagnostic) + code = r.rerunFailed(t.Context(), log, &bytes.Buffer{}, &diagnostic, (*os.File).Sync) } else { code, _, _ = run(t, "", rerunArguments()...) } @@ -247,12 +304,13 @@ func TestRerunFailedReservationAndAuditFailures(t *testing.T) { } 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) }() + go func() { results <- r.reserveRerun(dir, 1, (*os.File).Sync) }() } successes := 0 for range 8 { @@ -265,10 +323,11 @@ func TestRerunReservationConcurrent(t *testing.T) { } require.Equal(t, 1, successes) r.repo = "OWNER/REPO" - require.ErrorIs(t, r.reserveRerun(dir, 1), errRerunReserved) + require.ErrorIs(t, r.reserveRerun(dir, 1, (*os.File).Sync), errRerunReserved) } -func TestRerunFailedNoStdinAndHelp(t *testing.T) { +func testRerunFailedNoStdinAndHelp(t *testing.T) { + t.Helper() setupRerun(t) var out, diagnostic bytes.Buffer code := Run(t.Context(), rerunArguments(), brokenIO{}, &out, &diagnostic) @@ -276,7 +335,8 @@ func TestRerunFailedNoStdinAndHelp(t *testing.T) { require.Contains(t, usage, "run rerun-failed --repo OWNER/REPO") } -func TestRerunFailedPRGuard(t *testing.T) { +func testRerunFailedPRGuard(t *testing.T) { + t.Helper() setup(t) for _, command := range []string{ "gh-post run rerun-failed 123 --repo owner/repo", @@ -290,9 +350,9 @@ func TestRerunFailedPRGuard(t *testing.T) { "../../packages/agents/source/skills/pr/claude/scripts/guard-gh.sh", ) cmd.Stdin = bytes.NewReader(payload) - err = cmd.Run() + output, err := cmd.CombinedOutput() if strings.HasPrefix(command, "gh-post ") { - require.NoError(t, err) + require.NoError(t, err, "%s", output) } else { var exit *exec.ExitError require.ErrorAs(t, err, &exit) @@ -300,3 +360,69 @@ func TestRerunFailedPRGuard(t *testing.T) { } } } + +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"} + 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() + for _, failure := range []string{"1", "123", "repo", "owner", "gh-post-reruns", "devtools", "state"} { + 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/run.go b/internal/ghpost/run.go index cec80309..7ad8b4ae 100644 --- a/internal/ghpost/run.go +++ b/internal/ghpost/run.go @@ -217,7 +217,7 @@ func post(ctx context.Context, r request, body []byte, identities string, out, e } defer log.Close() if r.operation == runRerunFailed { - return r.rerunFailed(ctx, log, out, errOut) + return r.rerunFailed(ctx, log, out, errOut, (*os.File).Sync) } if r.isIssueStateChange() { return r.changeIssueState(ctx, body, log, out, errOut) From 66e469c41a058e062e87f4991f964317692e4f61 Mon Sep 17 00:00:00 2001 From: "tbhb.dev Agent" <334376894+tbhb-agent@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:35:05 -0500 Subject: [PATCH 3/5] fix(gh-post): preserve rerun reservations on Windows Windows refuses to sync directory handles, preventing eligible reruns from reaching GitHub. The receipt's own sync is the strongest durability available there. Other platforms still persist directory entries before the request. Related to #1052. Assisted-by: codex:gpt-6 Signed-off-by: Tony Burns --- docs/gh-post.md | 2 ++ internal/ghpost/rerun.go | 13 ++--------- internal/ghpost/rerun_internal_test.go | 16 +++++++++---- internal/ghpost/rerun_syncdir_other.go | 29 ++++++++++++++++++++++++ internal/ghpost/rerun_syncdir_windows.go | 16 +++++++++++++ 5 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 internal/ghpost/rerun_syncdir_other.go create mode 100644 internal/ghpost/rerun_syncdir_windows.go diff --git a/docs/gh-post.md b/docs/gh-post.md index c4b37797..43dfacc4 100644 --- a/docs/gh-post.md +++ b/docs/gh-post.md @@ -137,6 +137,8 @@ An eligible request sends one POST to `/repos/{owner}/{repo}/actions/runs/{run_i 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. diff --git a/internal/ghpost/rerun.go b/internal/ghpost/rerun.go index 9ab022b2..98833083 100644 --- a/internal/ghpost/rerun.go +++ b/internal/ghpost/rerun.go @@ -138,17 +138,8 @@ func (r *request) reserveRerun(dir string, attempt uint64, syncFile func(*os.Fil // Persist the receipt name and each directory entry created by MkdirAll, // including ancestors created during audit setup, before any POST. for { - parent, openErr := os.Open(dir) - if openErr != nil { - return fmt.Errorf("open reservation directory: %w", openErr) - } - syncErr := syncFile(parent) - closeErr := parent.Close() - if syncErr != nil { - return fmt.Errorf("sync reservation directory: %w", syncErr) - } - if closeErr != nil { - return fmt.Errorf("close reservation directory: %w", closeErr) + if err = syncDir(dir, syncFile); err != nil { + return fmt.Errorf("sync reservation directory: %w", err) } if filepath.Dir(dir) == dir { break diff --git a/internal/ghpost/rerun_internal_test.go b/internal/ghpost/rerun_internal_test.go index f7d02559..a50e28cd 100644 --- a/internal/ghpost/rerun_internal_test.go +++ b/internal/ghpost/rerun_internal_test.go @@ -392,10 +392,12 @@ func testRerunFailedDurability(t *testing.T) { require.Equal(t, 1, strings.Count(fileText(t, filepath.Join(dir, "calls")), "POST")) events = append(events, "POST") expected := []string{"create", "sync file"} - 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 + 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) @@ -403,7 +405,11 @@ func testRerunFailedDurability(t *testing.T) { func testRerunFailedSyncFailures(t *testing.T) { t.Helper() - for _, failure := range []string{"1", "123", "repo", "owner", "gh-post-reruns", "devtools", "state"} { + 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") diff --git a/internal/ghpost/rerun_syncdir_other.go b/internal/ghpost/rerun_syncdir_other.go new file mode 100644 index 00000000..700c929c --- /dev/null +++ b/internal/ghpost/rerun_syncdir_other.go @@ -0,0 +1,29 @@ +//go:build !windows + +// SPDX-License-Identifier: Apache-2.0 +// Copyright Tony Burns + +package ghpost + +import ( + "fmt" + "os" +) + +const dirSyncSupported = true + +func syncDir(dir string, syncFile func(*os.File) error) error { + 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/rerun_syncdir_windows.go b/internal/ghpost/rerun_syncdir_windows.go new file mode 100644 index 00000000..3202e465 --- /dev/null +++ b/internal/ghpost/rerun_syncdir_windows.go @@ -0,0 +1,16 @@ +//go:build windows + +// SPDX-License-Identifier: Apache-2.0 +// Copyright Tony Burns + +package ghpost + +import "os" + +const dirSyncSupported = false + +// Windows doesn't allow syncing a directory handle, so the receipt file's +// own Sync is the strongest durability available there. +func syncDir(_ string, _ func(*os.File) error) error { + return nil +} From ea0b69a8cdea5d6bbb141df4ecc4ad3496ff6256 Mon Sep 17 00:00:00 2001 From: "tbhb.dev Agent" <334376894+tbhb-agent@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:21:37 -0500 Subject: [PATCH 4/5] fix(gh-post): serialize Windows rerun test children Parallel test children contend on shared coverage metadata on Windows. Keep the subtests parallel while allowing only one child to execute at a time there. Coverage collection and its floor remain unchanged. Related to #1052. Assisted-by: codex:gpt-6 Signed-off-by: Tony Burns --- internal/ghpost/rerun_internal_test.go | 3 ++ .../rerun_process_other_internal_test.go | 11 ++++++ .../rerun_process_windows_internal_test.go | 39 +++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 internal/ghpost/rerun_process_other_internal_test.go create mode 100644 internal/ghpost/rerun_process_windows_internal_test.go diff --git a/internal/ghpost/rerun_internal_test.go b/internal/ghpost/rerun_internal_test.go index a50e28cd..e07abed2 100644 --- a/internal/ghpost/rerun_internal_test.go +++ b/internal/ghpost/rerun_internal_test.go @@ -43,6 +43,7 @@ func rerunCases() map[string]func(*testing.T) { func TestRerunFailed(t *testing.T) { t.Parallel() + var childLock rerunProcessLock for name := range rerunCases() { t.Run(name, func(t *testing.T) { t.Parallel() @@ -54,7 +55,9 @@ func TestRerunFailed(t *testing.T) { 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) }) } diff --git a/internal/ghpost/rerun_process_other_internal_test.go b/internal/ghpost/rerun_process_other_internal_test.go new file mode 100644 index 00000000..a19e1937 --- /dev/null +++ b/internal/ghpost/rerun_process_other_internal_test.go @@ -0,0 +1,11 @@ +//go:build !windows + +// SPDX-License-Identifier: Apache-2.0 +// Copyright Tony Burns + +package ghpost + +type rerunProcessLock struct{} + +func (*rerunProcessLock) Lock() {} +func (*rerunProcessLock) Unlock() {} diff --git a/internal/ghpost/rerun_process_windows_internal_test.go b/internal/ghpost/rerun_process_windows_internal_test.go new file mode 100644 index 00000000..d385c290 --- /dev/null +++ b/internal/ghpost/rerun_process_windows_internal_test.go @@ -0,0 +1,39 @@ +//go:build windows + +// 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 { + sync.Mutex +} + +func TestRerunProcessLock(t *testing.T) { + t.Parallel() + 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) + } +} From 6fcd1e70e6076f620f96ac1f398ecb74338a9929 Mon Sep 17 00:00:00 2001 From: "tbhb.dev Agent" <334376894+tbhb-agent@users.noreply.github.com> Date: Sun, 1 Nov 2026 16:00:04 -0500 Subject: [PATCH 5/5] fix(gh-post): keep rerun helpers visible to coverage lookup The coverage converter resolves profile paths through Linux Go files. Platform-specific source files prevent conversion when coverage includes Windows helpers. Runtime platform decisions keep those paths resolvable while preserving directory syncs and Windows child serialization. Related to #1052. Assisted-by: codex:gpt-6 Signed-off-by: Tony Burns --- ...test.go => rerun_process_internal_test.go} | 26 ++++++++++++++++--- .../rerun_process_other_internal_test.go | 11 -------- ...erun_syncdir_other.go => rerun_syncdir.go} | 10 ++++--- internal/ghpost/rerun_syncdir_windows.go | 16 ------------ 4 files changed, 30 insertions(+), 33 deletions(-) rename internal/ghpost/{rerun_process_windows_internal_test.go => rerun_process_internal_test.go} (60%) delete mode 100644 internal/ghpost/rerun_process_other_internal_test.go rename internal/ghpost/{rerun_syncdir_other.go => rerun_syncdir.go} (67%) delete mode 100644 internal/ghpost/rerun_syncdir_windows.go diff --git a/internal/ghpost/rerun_process_windows_internal_test.go b/internal/ghpost/rerun_process_internal_test.go similarity index 60% rename from internal/ghpost/rerun_process_windows_internal_test.go rename to internal/ghpost/rerun_process_internal_test.go index d385c290..e889f734 100644 --- a/internal/ghpost/rerun_process_windows_internal_test.go +++ b/internal/ghpost/rerun_process_internal_test.go @@ -1,5 +1,3 @@ -//go:build windows - // SPDX-License-Identifier: Apache-2.0 // Copyright Tony Burns @@ -16,11 +14,33 @@ import ( // Windows children must not write the shared coverage metadata concurrently. type rerunProcessLock struct { - sync.Mutex + 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) diff --git a/internal/ghpost/rerun_process_other_internal_test.go b/internal/ghpost/rerun_process_other_internal_test.go deleted file mode 100644 index a19e1937..00000000 --- a/internal/ghpost/rerun_process_other_internal_test.go +++ /dev/null @@ -1,11 +0,0 @@ -//go:build !windows - -// SPDX-License-Identifier: Apache-2.0 -// Copyright Tony Burns - -package ghpost - -type rerunProcessLock struct{} - -func (*rerunProcessLock) Lock() {} -func (*rerunProcessLock) Unlock() {} diff --git a/internal/ghpost/rerun_syncdir_other.go b/internal/ghpost/rerun_syncdir.go similarity index 67% rename from internal/ghpost/rerun_syncdir_other.go rename to internal/ghpost/rerun_syncdir.go index 700c929c..1165119e 100644 --- a/internal/ghpost/rerun_syncdir_other.go +++ b/internal/ghpost/rerun_syncdir.go @@ -1,5 +1,3 @@ -//go:build !windows - // SPDX-License-Identifier: Apache-2.0 // Copyright Tony Burns @@ -8,11 +6,17 @@ package ghpost import ( "fmt" "os" + "runtime" ) -const dirSyncSupported = true +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) diff --git a/internal/ghpost/rerun_syncdir_windows.go b/internal/ghpost/rerun_syncdir_windows.go deleted file mode 100644 index 3202e465..00000000 --- a/internal/ghpost/rerun_syncdir_windows.go +++ /dev/null @@ -1,16 +0,0 @@ -//go:build windows - -// SPDX-License-Identifier: Apache-2.0 -// Copyright Tony Burns - -package ghpost - -import "os" - -const dirSyncSupported = false - -// Windows doesn't allow syncing a directory handle, so the receipt file's -// own Sync is the strongest durability available there. -func syncDir(_ string, _ func(*os.File) error) error { - return nil -}