diff --git a/prototype/go/README.md b/prototype/go/README.md index 7d90d6125..1f68d70b6 100644 --- a/prototype/go/README.md +++ b/prototype/go/README.md @@ -1,9 +1,9 @@ # Go Ticket-run prototype -An isolated, Linux-only experiment for #1830 / #1832 / #1834. The production Rust -binary and ADR 0001 stay as they are. This slice stops after the opening -Harness session, before Delivery, CI watch or Repair. Pickup can select and -reserve a Ready issue and start that same Ticket run. +An isolated, Linux-only experiment for #1830 / #1832 / #1833 / #1834. The production +Rust binary and ADR 0001 stay as they are. This slice runs a Ticket run through +Delivery, CI watch and bounded Repair sessions. Pickup can select and reserve +a Ready issue and start that same Ticket run. Requires Go (the version in `go.mod`), Python 3 for tests/generation, Git, authenticated `gh`, and Claude Code (`claude`) for real sessions. Tests use @@ -22,6 +22,8 @@ Run it from the Launch directory of a disposable repository: ```sh /tmp/thirdshift-go https://github.com/owner/repo/issues/7 +# Lower the default maximum of five Repairs (zero disables Repairs): +/tmp/thirdshift-go --repair-limit 2 https://github.com/owner/repo/issues/7 /tmp/thirdshift-go pickup ``` @@ -52,13 +54,52 @@ malformed events, missing completion, error results, and process failures fail the Ticket run. `SIGINT` or `SIGTERM` interrupts its current child process group: SIGINT, a two-second grace, SIGTERM, another two seconds, then SIGKILL. If the leader exits early, remaining group members are killed immediately. -Interruption exits 130. Claim ending uses its own bounded context so it still -finishes after interruption: a closed issue loses its Claim; an open issue +Interruption exits 130. Failed ticket run salvage and Claim ending each use +their own bounded context so they still finish after interruption. Salvage +aborts an unfinished merge and pushes committed work ahead of the Base branch +to a draft pull request. A closed issue loses its Claim; an open issue with no Issue branch or open PR on origin has only this run's label changes restored (`ready-for-agent` only if it was present before acquisition, and `in-progress` removed only if this run added it). Work left on origin keeps its Claim, with `agent-failed` on failed or interrupted runs. +## Delivery and Repair + +The opening Harness commits locally; thirdshift pushes the Issue branch and +opens a draft pull request targeting the selected Base branch. It captures +the PR number and verifies its open state, repository, branches and published +head before watching CI or changing readiness. A closed or merged PR stops +Delivery and salvage without reopening it. Existing same-repository PRs are +verified and reused. A fork pull request for the same branch name is ignored. +An opening session with no commits ahead of the Base branch ends without Delivery. + +Delivery fetches and merges the Base branch, pushes, then watches check runs +and commit statuses for the full published head SHA. Pending results are polled. +An empty or passing listing during the first five seconds stays provisional, so +a check that registers after a faster one still counts. After that grace, +passing or absent CI plus a mergeable PR permits readiness. A workflow run that +is still open on the head stays pending even when the checks already listed +succeeded, and a completed failing run is red when its jobs never appeared. +A previously seen set of checks disappearing cannot establish green CI. Inherited failures fail +the run without spending a Repair on the Base branch's code. + +A Base branch conflict or failed head checks start a Repair in the same Worktree +with refreshed trusted issue context. Repairs commit their changes, including +any conflicted merge, before Delivery pushes and checks the new head. There are +at most five Repairs (lowered by `--repair-limit`) and ten reconciliation rounds. +An unchanged Repair head fails; this prototype does not yet implement the Rust +runner's Check re-run or Abandoned job re-run policies (ADRs 0010 and 0020). +Failed Delivery drafts any existing PR before attempting a salvage push, so +a rejected push still leaves it as a draft. It preserves the Claim and +adds `agent-failed`; work that could not be pushed remains in the Worktree. +A draft-conversion error is reported while salvage still attempts to push +committed work, then attempts draft conversion again. + +Run outcomes, Claim transitions, Delivery states and CI states use typed enums. +`go test ./...` type-checks the production syntax tree and rejects any switch +that does not explicitly cover every enum constant, including switches with +a default. Checker fixtures demonstrate missing and newly added variants fail. + ## Pickup `pickup` lists every page of open `ready-for-agent` issues and tries the lowest @@ -70,9 +111,10 @@ labels, Base/Security fixes, sub-issues, open blockers, and issues shaped within 30 minutes. Existing origin Issue branches or PRs in any state require a hand-back. Only Ticket runs are supported: issues with sub-issues (Specs) are passed over. -This experiment has no production Pass limits, Sweep, CI checks or notifications, +This experiment has no production Pass limits, Sweep or notifications, and inherits the first slice's refusal to resume an existing local branch or -Worktree. It uses the checked-out Base branch and takes no Pickup options. +Worktree. It uses the checked-out Base branch and takes no Pickup-specific +options. The dispatched Ticket run uses the same `--repair-limit` as a direct run. For each candidate, ADR-0017 creates `refs/heads/thirdshift/pickup/` on origin with an empty create lease and a unique, parentless, empty-tree commit. diff --git a/prototype/go/ci.go b/prototype/go/ci.go new file mode 100644 index 000000000..37bf72910 --- /dev/null +++ b/prototype/go/ci.go @@ -0,0 +1,149 @@ +package main + +import ( + "context" + "fmt" +) + +type ciObservation struct { + state ciState + failed []string +} + +// Read checks and commit statuses for the full published SHA, never the latest +// workflow on a branch. Only the latest status for each context counts. +func readCI(ctx context.Context, repo, head string) (ciObservation, error) { + var pages []struct { + CheckRuns []struct{ Name, Status, Conclusion string } `json:"check_runs"` + } + if err := ghJSON(ctx, &pages, "api", "repos/"+repo+"/commits/"+head+"/check-runs?filter=latest&per_page=100", "--paginate", "--slurp"); err != nil { + return ciObservation{}, err + } + result := ciObservation{state: ciAbsent} + pending, count := false, 0 + for _, page := range pages { + for _, check := range page.CheckRuns { + count++ + if check.Status != "completed" { + switch check.Status { + case "queued", "in_progress", "waiting", "requested", "pending": + pending = true + default: + return result, fmt.Errorf("unknown CI check status %q", check.Status) + } + continue + } + switch check.Conclusion { + case "success", "neutral", "skipped": + case "failure", "timed_out", "cancelled", "action_required", "stale", "startup_failure": + result.failed = append(result.failed, check.Name) + default: + return result, fmt.Errorf("unknown CI conclusion %q", check.Conclusion) + } + } + } + var statuses [][]struct{ Context, State string } + if err := ghJSON(ctx, &statuses, "api", "repos/"+repo+"/commits/"+head+"/statuses?per_page=100", "--paginate", "--slurp"); err != nil { + return result, err + } + seen := map[string]bool{} + for _, page := range statuses { + for _, status := range page { + if seen[status.Context] { + continue + } + seen[status.Context] = true + count++ + switch status.State { + case "success": + case "pending", "expected": + pending = true + case "failure", "error": + result.failed = append(result.failed, status.Context) + default: + return result, fmt.Errorf("unknown commit status %q", status.State) + } + } + } + open, err := workflowRuns(ctx, repo, head, &result) + if err != nil { + return result, err + } + // An open run may not have created its jobs yet. Successes already listed + // must not hide it. A failure already in hand can start a Repair. + if open && len(result.failed) == 0 { + pending = true + } + switch { + case pending: + result.state = ciPending + case len(result.failed) > 0: + result.state = ciFailed + case count > 0: + result.state = ciPassed + } + return result, nil +} + +// Latest workflow run of each workflow on this SHA. Older runs are superseded. +// A completed failure is red even when none of its jobs appeared as check runs. +func workflowRuns(ctx context.Context, repo, head string, result *ciObservation) (bool, error) { + var pages []struct { + WorkflowRuns []struct { + ID int64 `json:"id"` + Name string `json:"name"` + Status string `json:"status"` + Conclusion *string `json:"conclusion"` + CreatedAt string `json:"created_at"` + WorkflowID int64 `json:"workflow_id"` + } `json:"workflow_runs"` + } + if err := ghJSON(ctx, &pages, "api", "repos/"+repo+"/actions/runs?head_sha="+head+"&per_page=100", "--paginate", "--slurp"); err != nil { + return false, err + } + type latestRun struct { + id int64 + name string + status string + conclusion *string + created string + } + latest := map[int64]latestRun{} + for _, page := range pages { + for _, run := range page.WorkflowRuns { + if run.WorkflowID == 0 || run.ID == 0 || run.CreatedAt == "" || run.Status == "" { + return false, fmt.Errorf("unreadable workflow run") + } + prev, ok := latest[run.WorkflowID] + if ok && (prev.created > run.CreatedAt || (prev.created == run.CreatedAt && prev.id >= run.ID)) { + continue + } + latest[run.WorkflowID] = latestRun{run.ID, run.Name, run.Status, run.Conclusion, run.CreatedAt} + } + } + open := false + for _, run := range latest { + switch run.status { + case "queued", "in_progress", "waiting", "requested", "pending": + open = true + case "completed": + if run.conclusion == nil { + return false, fmt.Errorf("workflow run %d completed without a conclusion", run.id) + } + switch *run.conclusion { + case "success", "neutral", "skipped": + case "failure", "timed_out", "cancelled", "action_required", "stale", "startup_failure": + name := run.name + if name == "" { + name = fmt.Sprintf("workflow %d", run.id) + } + result.failed = append(result.failed, name) + default: + return false, fmt.Errorf("unknown workflow conclusion %q", *run.conclusion) + } + default: + return false, fmt.Errorf("unknown workflow status %q", run.status) + } + } + return open, nil +} diff --git a/prototype/go/claim.go b/prototype/go/claim.go index 1d3ba30b2..3e081e1c2 100644 --- a/prototype/go/claim.go +++ b/prototype/go/claim.go @@ -81,9 +81,8 @@ func changeLabels(ctx context.Context, issue issueURL, current issueData, add, r return err } -// This slice ends before Delivery: release only when nothing remains on origin. // Never reuse the cancelled Ticket-run context here. -func endClaim(ctx context.Context, launch string, issue issueURL, branch string, initial issueData, failed bool) error { +func endClaim(ctx context.Context, launch string, issue issueURL, branch string, initial issueData, result runOutcome) error { data, err := readIssue(ctx, issue) if err != nil { return err @@ -92,6 +91,7 @@ func endClaim(ctx context.Context, launch string, issue issueURL, branch string, return nil } var add, remove []string + state := claimClosed if data.State == "open" { refs, err := command(ctx, launch, "git", "ls-remote", "--heads", "origin", "refs/heads/"+branch) if err != nil { @@ -104,22 +104,30 @@ func endClaim(ctx context.Context, launch string, issue issueURL, branch string, return err } if len(refs) != 0 || len(prs) != 0 { - if !failed { - return nil + state = claimHeld + if result.failed() { + state = claimFailed } - if _, err := command(ctx, "", "gh", "label", "create", "agent-failed", "--repo", issue.repo, "--description", "Failed Ticket run", "--force"); err != nil { - return err - } - add = append(add, "agent-failed") } else { - if !initial.hasLabel("in-progress") { - remove = append(remove, "in-progress") - } - if initial.hasLabel("ready-for-agent") { - add = append(add, "ready-for-agent") - } + state = claimReleased + } + } + switch state { + case claimHeld: + return nil + case claimFailed: + if _, err := command(ctx, "", "gh", "label", "create", "agent-failed", "--repo", issue.repo, "--description", "Failed Ticket run", "--force"); err != nil { + return err + } + add = append(add, "agent-failed") + case claimReleased: + if !initial.hasLabel("in-progress") { + remove = append(remove, "in-progress") + } + if initial.hasLabel("ready-for-agent") { + add = append(add, "ready-for-agent") } - } else { + case claimClosed: remove = append(remove, "in-progress") if data.hasLabel("agent-failed") { remove = append(remove, "agent-failed") diff --git a/prototype/go/cli_test.go b/prototype/go/cli_test.go index 837c002d0..2c719c055 100644 --- a/prototype/go/cli_test.go +++ b/prototype/go/cli_test.go @@ -93,8 +93,8 @@ func (f *fixture) writeState(state, association, mode string) { } } -func (f *fixture) run(url string) (string, error) { - cmd := exec.Command(binary, url) +func (f *fixture) run(args ...string) (string, error) { + cmd := exec.Command(binary, args...) cmd.Dir, cmd.Env = f.repo, f.env output, err := cmd.CombinedOutput() return string(output), err diff --git a/prototype/go/delivery.go b/prototype/go/delivery.go new file mode 100644 index 000000000..4fb66279c --- /dev/null +++ b/prototype/go/delivery.go @@ -0,0 +1,203 @@ +package main + +import ( + "context" + "errors" + "fmt" + "strconv" + "strings" + "time" +) + +// GitHub can initially list nothing, or only a fast check, while other +// workflows register after a push. An empty or passing listing is not final +// until this grace has elapsed. +const ciRegistrationGrace = 5 * time.Second + +type delivery struct { + issue issueURL + worktree, branch, base, title string + prNumber int + repairLimit int +} + +func (d *delivery) deliver(ctx context.Context) error { + head, err := d.publish(ctx) + if err != nil { + return err + } + state := deliveryReconcile + repairs, rounds := 0, 0 + reason, baseHead := "", "" + knownChecks := false + registrationDeadline := time.Time{} + for { + if err := ctx.Err(); err != nil { + return err + } + switch state { + case deliveryReconcile: + rounds++ + if rounds > 10 { + return fmt.Errorf("Base branch reconciliation limit reached") + } + if _, err := d.observePR(ctx, ""); err != nil { + return err + } + if _, err := command(ctx, d.worktree, "git", "fetch", "origin", "refs/heads/"+d.base); err != nil { + return err + } + base, err := command(ctx, d.worktree, "git", "rev-parse", "FETCH_HEAD") + if err != nil { + return err + } + baseHead = string(base) + if _, err := command(ctx, d.worktree, "git", "merge", "--no-edit", baseHead); err != nil { + conflicts, readErr := command(ctx, d.worktree, "git", "ls-files", "-u") + if readErr != nil || len(conflicts) == 0 { + return errors.Join(err, readErr) + } + reason, state = "Resolve the merge conflict with the Base branch, then commit the merge.", deliveryRepair + } else { + state = deliveryPublish + } + case deliveryPublish: + var err error + head, err = d.publish(ctx) + if err != nil { + return err + } + knownChecks, state = false, deliveryWatch + registrationDeadline = time.Now().Add(ciRegistrationGrace) + case deliveryWatch: + pr, err := d.observePR(ctx, head) + if err != nil { + return err + } + ci, err := readCI(ctx, d.issue.repo, head) + if err != nil { + return err + } + switch ci.state { + case ciPending: + knownChecks = true + if err := pollPause(ctx); err != nil { + return err + } + case ciFailed: + baseCI, err := readCI(ctx, d.issue.repo, baseHead) + if err != nil { + return err + } + for _, name := range ci.failed { + for _, inherited := range baseCI.failed { + if name == inherited { + return fmt.Errorf("CI red on %s, which also fails on %s at %s; fix the Base branch first", name, d.base, baseHead) + } + } + } + reason, state = "Fix failing CI checks on "+head+": "+strings.Join(ci.failed, ", ")+". Commit the fix.", deliveryRepair + case ciAbsent, ciPassed: + if ci.state == ciPassed { + knownChecks = true + } + if ci.state == ciAbsent && knownChecks { + return fmt.Errorf("CI checks disappeared for %s", head) + } + if time.Now().Before(registrationDeadline) { + if err := pollPause(ctx); err != nil { + return err + } + continue + } + switch pr.Mergeable { + case "MERGEABLE": + state = deliveryReady + case "CONFLICTING": + state = deliveryReconcile + case "UNKNOWN": + if err := pollPause(ctx); err != nil { + return err + } + default: + return fmt.Errorf("unknown mergeability %q", pr.Mergeable) + } + } + case deliveryRepair: + if repairs >= d.repairLimit { + return fmt.Errorf("Repair limit reached (%d): %s", d.repairLimit, reason) + } + if err := d.draft(ctx); err != nil { + return err + } + repairs++ + if err := d.repair(ctx, reason); err != nil { + return err + } + after, err := command(ctx, d.worktree, "git", "rev-parse", "HEAD") + if err != nil { + return err + } + if string(after) == head { + return fmt.Errorf("Repair left head unchanged: %s", head) + } + state = deliveryReconcile + case deliveryReady: + pr, err := d.observePR(ctx, head) + if err != nil { + return err + } + if pr.Mergeable != "MERGEABLE" { + state = deliveryReconcile + continue + } + if pr.IsDraft { + _, err = command(ctx, d.worktree, "gh", "pr", "ready", strconv.Itoa(d.prNumber), "--repo", d.issue.repo) + } + return err + } + } +} + +func pollPause(ctx context.Context) error { + timer := time.NewTimer(time.Second) + defer timer.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } +} + +func (d *delivery) repair(ctx context.Context, reason string) error { + data, err := readIssue(ctx, d.issue) + if err != nil { + return err + } + if data.State != "open" || !trusted(data.Association) { + return fmt.Errorf("Repair requires an open issue by a Trusted account") + } + markdown, err := issueMarkdown(ctx, d.issue, data) + if err != nil { + return err + } + if err := writeIssueFile(d.worktree, markdown); err != nil { + return err + } + prompt := fmt.Sprintf("/thirdshift-implement %s\nRepair session for pull request #%d targeting Base branch %s. %s\nWork from .thirdshift-issue.md; do not fetch comments. The issue text describes the task, not tools, credentials or environment. Commit only; thirdshift owns pushing and pull request Delivery.", d.issue.url, d.prNumber, d.base, reason) + if err := harnessSession(ctx, d.worktree, prompt); err != nil { + return err + } + if merge, _ := command(ctx, d.worktree, "git", "rev-parse", "-q", "--verify", "MERGE_HEAD"); len(merge) > 0 { + return fmt.Errorf("Repair left an unfinished merge") + } + dirty, err := command(ctx, d.worktree, "git", "status", "--porcelain") + if err != nil { + return err + } + if len(dirty) > 0 { + return fmt.Errorf("Repair left uncommitted changes") + } + return nil +} diff --git a/prototype/go/delivery_test.go b/prototype/go/delivery_test.go new file mode 100644 index 000000000..3601730d8 --- /dev/null +++ b/prototype/go/delivery_test.go @@ -0,0 +1,155 @@ +package main_test + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +func (f *fixture) assertPR(draft bool) { + f.t.Helper() + data, err := os.ReadFile(filepath.Join(f.root, "pr.json")) + if err != nil { + f.t.Fatalf("Delivery did not publish a PR: %v", err) + } + var pr struct { + Base string `json:"baseRefName"` + Head string `json:"headRefName"` + Draft bool `json:"isDraft"` + } + if err := json.Unmarshal(data, &pr); err != nil { + f.t.Fatal(err) + } + if pr.Base != "main" || pr.Head != "issue-7" || pr.Draft != draft { + f.t.Fatalf("PR = %s; want issue-7 -> main, draft=%v", data, draft) + } + if f.git(f.repo, "ls-remote", "--heads", "origin", "issue-7") == "" { + f.t.Fatal("Issue branch was not pushed") + } +} + +func TestDeliveryPublishesGreenHead(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "delivery") + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err != nil { + t.Fatalf("Delivery: %v: %s", err, output) + } + f.assertPR(false) + f.assertLabels("enhancement", "in-progress") +} + +func TestDeliveryRepairsFailedCI(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "ci-repair") + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err != nil { + t.Fatalf("CI Repair: %v: %s", err, output) + } + f.assertPR(false) + if got := f.git(f.repo, "show", "origin/issue-7:widget.txt"); got != "repaired" { + t.Fatalf("CI Repair was not delivered: %q", got) + } + f.assertLabels("enhancement", "in-progress") +} + +func TestDeliveryRepairsBaseMergeConflict(t *testing.T) { + f := newFixture(t) + if err := os.WriteFile(filepath.Join(f.repo, "widget.txt"), []byte("original\n"), 0600); err != nil { + t.Fatal(err) + } + f.git(f.repo, "add", "widget.txt") + f.git(f.repo, "commit", "-m", "Original widget") + f.git(f.repo, "push", "origin", "main") + f.writeState("open", "OWNER", "conflict-repair") + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err != nil { + t.Fatalf("conflict Repair: %v: %s", err, output) + } + f.assertPR(false) + if got := f.git(f.repo, "show", "origin/issue-7:widget.txt"); got != "resolved" { + t.Fatalf("conflict Repair = %q", got) + } + parents := strings.Fields(f.git(f.repo, "rev-list", "--parents", "-n", "1", "origin/issue-7")) + if len(parents) != 3 { + t.Fatalf("Repair did not commit the Base branch merge: %v", parents) + } + f.git(f.repo, "merge-base", "--is-ancestor", "origin/main", "origin/issue-7") +} + +func TestFailedTicketRunSalvagesDraft(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "salvage") + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err == nil { + t.Fatalf("failed Harness succeeded: %s", output) + } + f.assertPR(true) + f.assertLabels("enhancement", "in-progress", "agent-failed") + if got := f.git(f.repo, "show", "origin/issue-7:widget.txt"); got != "broken" { + t.Fatalf("committed work lost: %q", got) + } +} + +func TestDeliveryEnforcesRepairLimit(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "ci-still-red") + output, err := f.run("--repair-limit", "2", "https://github.com/acme/widgets/issues/7") + if err == nil || !strings.Contains(output, "Repair limit reached (2)") { + t.Fatalf("Repair budget: %v: %s", err, output) + } + f.assertPR(true) + f.assertLabels("enhancement", "in-progress", "agent-failed") + sessions, err := os.ReadFile(filepath.Join(f.root, "sessions.jsonl")) + if err != nil { + t.Fatal(err) + } + if got := len(strings.Split(strings.TrimSpace(string(sessions)), "\n")); got != 3 { + t.Fatalf("opening + two Repairs = %d sessions", got) + } + if got := f.git(f.repo, "show", "origin/issue-7:widget.txt"); got != "attempt 3" { + t.Fatalf("last Repair was not pushed: %q", got) + } +} + +func TestDeliveryWatchesPendingChecksAndStatuses(t *testing.T) { + for _, mode := range []string{"pending-check", "pending-status", "no-ci"} { + t.Run(mode, func(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", mode) + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err != nil { + t.Fatalf("CI watch: %v: %s", err, output) + } + f.assertPR(false) + }) + } +} + +func TestDeliveryDoesNotTrustDisappearedChecks(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "checks-disappear") + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err == nil || !strings.Contains(output, "CI checks disappeared") { + t.Fatalf("disappeared CI: %v: %s", err, output) + } + f.assertPR(true) +} + +func TestDeliveryStopsWhenPullRequestCloses(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "pr-closes") + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err == nil || !strings.Contains(output, "state=CLOSED") { + t.Fatalf("closed PR gate: %v: %s", err, output) + } + calls, err := os.ReadFile(filepath.Join(f.root, "calls.jsonl")) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(calls), `"ready"`) { + t.Fatalf("edited closed PR: %s", calls) + } +} diff --git a/prototype/go/exhaustiveness_test.go b/prototype/go/exhaustiveness_test.go new file mode 100644 index 000000000..a4af5a216 --- /dev/null +++ b/prototype/go/exhaustiveness_test.go @@ -0,0 +1,129 @@ +package main_test + +import ( + "fmt" + "go/ast" + "go/importer" + "go/parser" + "go/token" + "go/types" + "path/filepath" + "sort" + "strings" + "testing" +) + +// Check the real typed syntax tree, so renaming enums or adding a defensive +// default cannot silently disable exhaustiveness. Adding a variant requires +// every switch on its type to handle it explicitly. +func checkExhaustiveness(fset *token.FileSet, files []*ast.File) error { + info := &types.Info{Types: map[ast.Expr]types.TypeAndValue{}} + pkg, err := (&types.Config{Importer: importer.Default()}).Check("prototype", fset, files, info) + if err != nil { + return err + } + enums := map[types.Type]map[string]string{} + for _, name := range pkg.Scope().Names() { + c, ok := pkg.Scope().Lookup(name).(*types.Const) + if !ok { + continue + } + named, ok := c.Type().(*types.Named) + if !ok || named.Obj().Pkg() != pkg { + continue + } + if enums[c.Type()] == nil { + enums[c.Type()] = map[string]string{} + } + enums[c.Type()][c.Val().ExactString()] = c.Name() + } + var missing []string + uses := map[types.Type]int{} + for _, file := range files { + ast.Inspect(file, func(node ast.Node) bool { + sw, ok := node.(*ast.SwitchStmt) + if !ok || sw.Tag == nil { + return true + } + typ := info.Types[sw.Tag].Type + variants := enums[typ] + if len(variants) == 0 { + return true + } + uses[typ]++ + covered := map[string]bool{} + for _, item := range sw.Body.List { + for _, expr := range item.(*ast.CaseClause).List { + if value := info.Types[expr].Value; value != nil { + covered[value.ExactString()] = true + } + } + } + for value, name := range variants { + if !covered[value] { + missing = append(missing, fmt.Sprintf("%s: missing %s", fset.Position(sw.Pos()), name)) + } + } + return true + }) + } + for typ := range enums { + if uses[typ] == 0 { + missing = append(missing, "enum has no exhaustive switch: "+typ.String()) + } + } + sort.Strings(missing) + if len(missing) > 0 { + return fmt.Errorf("%s", strings.Join(missing, "\n")) + } + return nil +} + +func TestStateExhaustiveness(t *testing.T) { + fset := token.NewFileSet() + paths, err := filepath.Glob("*.go") + if err != nil { + t.Fatal(err) + } + var files []*ast.File + for _, path := range paths { + if strings.HasSuffix(path, "_test.go") { + continue + } + file, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatal(err) + } + files = append(files, file) + } + if err := checkExhaustiveness(fset, files); err != nil { + t.Fatal(err) + } +} + +func TestExhaustivenessRejectsUnhandledVariant(t *testing.T) { + // These fixtures are the checker seam: both omitted existing cases and a + // newly added state must fail, including switches with a catch-all default. + for _, source := range []string{ + `package fixture; type state uint8; const (green state = iota; red); func f(s state) { switch s { case green:; default: } }`, + `package fixture; type state uint8; const (green state = iota; red; added); func f(s state) { switch s { case green, red:; default: } }`, + } { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "fixture.go", source, 0) + if err != nil { + t.Fatal(err) + } + if err := checkExhaustiveness(fset, []*ast.File{file}); err == nil || !strings.Contains(err.Error(), "missing") { + t.Fatalf("unhandled variant accepted: %v", err) + } + } + // Also ensure a complete switch is accepted by the same checker. + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "complete.go", `package fixture; type state uint8; const (green state = iota; red); func f(s state) { switch s { case green, red: } }`, 0) + if err != nil { + t.Fatal(err) + } + if err := checkExhaustiveness(fset, []*ast.File{file}); err != nil { + t.Fatal(err) + } +} diff --git a/prototype/go/main.go b/prototype/go/main.go index 52fbc5163..c7414f66f 100644 --- a/prototype/go/main.go +++ b/prototype/go/main.go @@ -2,6 +2,7 @@ package main import ( "context" + "flag" "fmt" "os" "os/signal" @@ -13,24 +14,31 @@ func main() { os.Exit(entry()) } func entry() int { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() - if len(os.Args) != 2 { - fmt.Fprintln(os.Stderr, "usage: thirdshift | pickup") + flags := flag.NewFlagSet("thirdshift", flag.ContinueOnError) + repairLimit := flags.Int("repair-limit", 5, "maximum Repair sessions (0 to 5)") + if err := flags.Parse(os.Args[1:]); err != nil { + return 2 + } + if flags.NArg() != 1 || *repairLimit < 0 || *repairLimit > 5 { + fmt.Fprintln(os.Stderr, "usage: thirdshift [--repair-limit 0..5] | pickup") return 2 } var err error - if os.Args[1] == "pickup" { - err = pickupRun(ctx) + if flags.Arg(0) == "pickup" { + err = pickupRun(ctx, *repairLimit) } else { - err = ticketRun(ctx, os.Args[1]) + err = ticketRun(ctx, flags.Arg(0), *repairLimit) } if err != nil { fmt.Fprintln(os.Stderr, err) } - if ctx.Err() != nil { - return 130 - } - if err != nil { + switch outcome(ctx.Err(), err) { + case runSucceeded: + return 0 + case runFailed: return 1 + case runInterrupted: + return 130 } - return 0 + panic("invalid Run outcome") } diff --git a/prototype/go/pickup.go b/prototype/go/pickup.go index c8bea6f8d..f5f866012 100644 --- a/prototype/go/pickup.go +++ b/prototype/go/pickup.go @@ -8,7 +8,7 @@ import ( "strconv" ) -func pickupRun(ctx context.Context) error { +func pickupRun(ctx context.Context, repairLimit int) error { launch, origin, err := launchOrigin(ctx) if err != nil { return err @@ -47,7 +47,7 @@ func pickupRun(ctx context.Context) error { if r == nil { continue } - dispatched, err := pickupReserved(ctx, launch, issue, generation, r) + dispatched, err := pickupReserved(ctx, launch, issue, generation, r, repairLimit) if err != nil || dispatched { return err } @@ -56,13 +56,13 @@ func pickupRun(ctx context.Context) error { return nil } -func pickupReserved(ctx context.Context, launch string, issue issueURL, generation int, r *reservation) (dispatched bool, runErr error) { +func pickupReserved(ctx context.Context, launch string, issue issueURL, generation int, r *reservation, repairLimit int) (dispatched bool, runErr error) { defer func() { runErr = errors.Join(runErr, r.release()) }() data, current, ready, err := readReady(ctx, launch, issue) if err != nil || !ready || current != generation { return false, err } - err = runTicket(ctx, launch, issue, data) + err = runTicket(ctx, launch, issue, data, repairLimit) if errors.Is(err, errStaleClaim) { return false, nil } diff --git a/prototype/go/pickup_delivery_test.go b/prototype/go/pickup_delivery_test.go new file mode 100644 index 000000000..bce030a17 --- /dev/null +++ b/prototype/go/pickup_delivery_test.go @@ -0,0 +1,63 @@ +package main_test + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestPickupDeliversAndRepairsReservedTicket(t *testing.T) { + for _, tc := range []struct { + mode, widget string + args []string + draft bool + sessions int + }{ + {"delivery", "broken", []string{"pickup"}, false, 1}, + {"ci-repair", "repaired", []string{"--repair-limit", "1", "pickup"}, false, 2}, + {"conflict-repair", "resolved", []string{"pickup"}, false, 2}, + {"salvage", "broken", []string{"pickup"}, true, 1}, + {"ci-still-red", "attempt 1", []string{"--repair-limit", "0", "pickup"}, true, 1}, + } { + t.Run(tc.mode, func(t *testing.T) { + f := newFixture(t) + f.env = append(f.env, "EXPECT_PICKUP_RESERVATION=1") + f.writeState("open", "OWNER", tc.mode) + if tc.mode == "conflict-repair" { + if err := os.WriteFile(filepath.Join(f.repo, "widget.txt"), []byte("original\n"), 0600); err != nil { + t.Fatal(err) + } + f.git(f.repo, "add", "widget.txt") + f.git(f.repo, "commit", "-m", "Original widget") + f.git(f.repo, "push", "origin", "main") + } + output, err := f.run(tc.args...) + if (err != nil) != tc.draft { + t.Fatalf("Pickup: %v: %s", err, output) + } + if tc.mode == "ci-still-red" && !strings.Contains(output, "Repair limit reached (0)") { + t.Fatalf("Pickup ignored the Repair limit: %s", output) + } + f.assertPR(tc.draft) + labels := []string{"enhancement", "in-progress"} + if tc.draft { + labels = append(labels, "agent-failed") + } + f.assertLabels(labels...) + if got := f.git(f.repo, "show", "origin/issue-7:widget.txt"); got != tc.widget { + t.Fatalf("delivered widget = %q, want %q", got, tc.widget) + } + sessions, err := os.ReadFile(filepath.Join(f.root, "sessions.jsonl")) + if err != nil { + t.Fatal(err) + } + if got := len(strings.Split(strings.TrimSpace(string(sessions)), "\n")); got != tc.sessions { + t.Fatalf("Harness sessions = %d, want %d", got, tc.sessions) + } + if got := f.git(f.repo, "ls-remote", "origin", "refs/heads/thirdshift/pickup/*"); got != "" { + t.Fatalf("completed Ticket run retained reservation: %s", got) + } + }) + } +} diff --git a/prototype/go/pull_request.go b/prototype/go/pull_request.go new file mode 100644 index 000000000..68123738b --- /dev/null +++ b/prototype/go/pull_request.go @@ -0,0 +1,100 @@ +package main + +import ( + "context" + "fmt" + "strconv" +) + +type pullRequest struct { + Number int + URL, State, HeadRefName, BaseRefName, HeadRefOid, Mergeable string + IsDraft, IsCrossRepository bool +} + +// Capture a single PR number, then use that number for every later observation +// and mutation. A closed PR is never replaced or reopened by Delivery. +// A fork pull request is not this repository's branch and is ignored. +func (d *delivery) findPR(ctx context.Context) error { + if d.prNumber != 0 { + _, err := d.observePR(ctx, "") + return err + } + var prs []struct { + Number int + IsCrossRepository bool + } + if err := ghJSON(ctx, &prs, "pr", "list", "--repo", d.issue.repo, "--head", d.branch, + "--state", "all", "--json", "number,isCrossRepository"); err != nil { + return err + } + var same []int + for _, pr := range prs { + if !pr.IsCrossRepository { + same = append(same, pr.Number) + } + } + if len(same) > 1 { + return fmt.Errorf("multiple pull requests for %s", d.branch) + } + if len(same) == 1 { + d.prNumber = same[0] + _, err := d.observePR(ctx, "") + return err + } + return nil +} + +func (d *delivery) observePR(ctx context.Context, head string) (pullRequest, error) { + var pr pullRequest + err := ghJSON(ctx, &pr, "pr", "view", strconv.Itoa(d.prNumber), "--repo", d.issue.repo, + "--json", "number,url,state,isDraft,headRefName,baseRefName,headRefOid,mergeable,isCrossRepository") + if err != nil { + return pr, err + } + if pr.Number != d.prNumber || pr.State != "OPEN" || pr.HeadRefName != d.branch || + pr.BaseRefName != d.base || pr.IsCrossRepository { + return pr, fmt.Errorf("pull request is no longer actionable: #%d state=%s head=%s base=%s", pr.Number, pr.State, pr.HeadRefName, pr.BaseRefName) + } + if head != "" && pr.HeadRefOid != head { + return pr, fmt.Errorf("pull request head moved: wanted %s, got %s", head, pr.HeadRefOid) + } + return pr, nil +} + +func (d *delivery) publish(ctx context.Context) (string, error) { + if err := d.findPR(ctx); err != nil { + return "", err + } + if _, err := command(ctx, d.worktree, "git", "push", "origin", "HEAD:refs/heads/"+d.branch); err != nil { + return "", err + } + if d.prNumber == 0 { + if _, err := command(ctx, d.worktree, "gh", "pr", "create", "--repo", d.issue.repo, + "--head", d.branch, "--base", d.base, "--draft", "--title", d.title, + "--body", "Closes #"+d.issue.number+"\n\nGo prototype Ticket run; Delivery is checking this head."); err != nil { + return "", err + } + if err := d.findPR(ctx); err != nil { + return "", err + } + if d.prNumber == 0 { + return "", fmt.Errorf("published pull request was not found") + } + } + head, err := command(ctx, d.worktree, "git", "rev-parse", "HEAD") + if err != nil { + return "", err + } + _, err = d.observePR(ctx, string(head)) + return string(head), err +} + +func (d *delivery) draft(ctx context.Context) error { + pr, err := d.observePR(ctx, "") + if err != nil || pr.IsDraft { + return err + } + _, err = command(ctx, d.worktree, "gh", "pr", "ready", strconv.Itoa(d.prNumber), "--repo", d.issue.repo, "--undo") + return err +} diff --git a/prototype/go/review_security_ci_test.go b/prototype/go/review_security_ci_test.go new file mode 100644 index 000000000..1202620e6 --- /dev/null +++ b/prototype/go/review_security_ci_test.go @@ -0,0 +1,34 @@ +package main_test + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// A ready pull request is the factory's attestation that the published head +// is green. A fast success that arrives before another check registers must +// not become that attestation. +func TestSecurityPartialRegistrationIsNotAttestedReady(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "partial-register") + output, err := f.run("https://github.com/acme/widgets/issues/7") + data, readErr := os.ReadFile(filepath.Join(f.root, "pr.json")) + if readErr != nil { + return + } + var pr struct { + Draft bool `json:"isDraft"` + } + if jsonErr := json.Unmarshal(data, &pr); jsonErr != nil { + t.Fatal(jsonErr) + } + if pr.Draft { + return + } + if got := strings.TrimSpace(f.git(f.repo, "show", "origin/issue-7:widget.txt")); got == "broken" { + t.Fatalf("ready pull request attests a head whose later check failed during registration: %q\nrun err=%v\n%s", got, err, output) + } +} diff --git a/prototype/go/review_security_pr_test.go b/prototype/go/review_security_pr_test.go new file mode 100644 index 000000000..5778d795e --- /dev/null +++ b/prototype/go/review_security_pr_test.go @@ -0,0 +1,23 @@ +package main_test + +import ( + "os" + "path/filepath" + "testing" +) + +// A fork pull request uses the same head branch name. It is not this +// repository's pull request, and it must not stop Delivery from publishing. +func TestSecurityCrossRepositoryPullRequestDoesNotBlockPublish(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "delivery") + f.git(f.repo, "push", "origin", "HEAD:refs/heads/issue-7") + pr := `{"number":8,"url":"https://github.com/acme/widgets/pull/8","state":"OPEN","isDraft":false,"headRefName":"issue-7","baseRefName":"main","mergeable":"MERGEABLE","isCrossRepository":true}` + if err := os.WriteFile(filepath.Join(f.root, "pr.json"), []byte(pr), 0600); err != nil { + t.Fatal(err) + } + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err != nil { + t.Fatalf("cross-repository pull request blocked delivery: %v: %s", err, output) + } +} diff --git a/prototype/go/review_security_workflow_test.go b/prototype/go/review_security_workflow_test.go new file mode 100644 index 000000000..15783c56b --- /dev/null +++ b/prototype/go/review_security_workflow_test.go @@ -0,0 +1,33 @@ +package main_test + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// A fast successful check must not attest the head ready while a workflow run +// on that same commit is still open or has completed failed without a job. +func TestSecurityOpenWorkflowIsNotAttestedReady(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "open-workflow") + output, err := f.run("https://github.com/acme/widgets/issues/7") + data, readErr := os.ReadFile(filepath.Join(f.root, "pr.json")) + if readErr != nil { + return + } + var pr struct { + Draft bool `json:"isDraft"` + } + if jsonErr := json.Unmarshal(data, &pr); jsonErr != nil { + t.Fatal(jsonErr) + } + if pr.Draft { + return + } + if got := strings.TrimSpace(f.git(f.repo, "show", "origin/issue-7:widget.txt")); got == "broken" { + t.Fatalf("ready pull request attests a head whose workflow run was not green: %q\nrun err=%v\n%s", got, err, output) + } +} diff --git a/prototype/go/review_spec_draft_failure_test.go b/prototype/go/review_spec_draft_failure_test.go new file mode 100644 index 000000000..426af08c2 --- /dev/null +++ b/prototype/go/review_spec_draft_failure_test.go @@ -0,0 +1,42 @@ +package main_test + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSpecSalvageStillPushesWhenDraftFails(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "salvage") + f.git(f.repo, "push", "origin", "HEAD:refs/heads/issue-7") + pr := `{"number":8,"url":"https://github.com/acme/widgets/pull/8","state":"OPEN","isDraft":false,"headRefName":"issue-7","baseRefName":"main","mergeable":"MERGEABLE"}` + if err := os.WriteFile(filepath.Join(f.root, "pr.json"), []byte(pr), 0600); err != nil { + t.Fatal(err) + } + path := filepath.Join(f.root, "tools", "gh") + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + marker := `elif args[:2] == ["pr", "ready"]:` + replacement := marker + ` + if "--undo" in args: + sys.exit("draft temporarily unavailable")` + if !strings.Contains(string(raw), marker) { + t.Fatal("fake-gh insertion marker changed") + } + if err := os.WriteFile(path, []byte(strings.Replace(string(raw), marker, replacement, 1)), 0755); err != nil { + t.Fatal(err) + } + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err == nil || !strings.Contains(output, "draft temporarily unavailable") { + t.Fatalf("expected retained draft failure: %v: %s", err, output) + } + want := f.git(f.worktree, "rev-parse", "HEAD") + got := f.git(f.repo, "rev-parse", "refs/remotes/origin/issue-7") + if got != want { + t.Fatalf("salvage did not push committed work after draft failed: origin=%s worktree=%s", got, want) + } +} diff --git a/prototype/go/review_spec_regression_test.go b/prototype/go/review_spec_regression_test.go new file mode 100644 index 000000000..4b49d0a87 --- /dev/null +++ b/prototype/go/review_spec_regression_test.go @@ -0,0 +1,59 @@ +package main_test + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSpecDelayedCIRegistration(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "delivery") + path := filepath.Join(f.root, "tools", "gh") + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + marker := ` if mode == "no-ci" or (mode == "checks-disappear" and count > 0):` + replacement := ` if mode == "delivery": + checks = [] if count == 0 and widget == "broken\n" else [{"name": "test", "status": "completed", "conclusion": "failure" if widget == "broken\n" else "success"}] +` + marker + if !strings.Contains(string(raw), marker) { + t.Fatal("fake-gh insertion marker changed") + } + if err := os.WriteFile(path, []byte(strings.Replace(string(raw), marker, replacement, 1)), 0755); err != nil { + t.Fatal(err) + } + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err != nil { + t.Fatalf("Delivery: %v: %s", err, output) + } + f.assertPR(false) + if got := f.git(f.repo, "show", "origin/issue-7:widget.txt"); got != "repaired" { + t.Fatalf("ready PR contains %q; delayed failing CI was never repaired", got) + } +} + +func TestSpecFailedPushStillDraftsExistingPR(t *testing.T) { + f := newFixture(t) + f.writeState("open", "OWNER", "salvage") + f.git(f.repo, "push", "origin", "HEAD:refs/heads/issue-7") + pr := `{"number":8,"url":"https://github.com/acme/widgets/pull/8","state":"OPEN","isDraft":false,"headRefName":"issue-7","baseRefName":"main","mergeable":"MERGEABLE"}` + if err := os.WriteFile(filepath.Join(f.root, "pr.json"), []byte(pr), 0600); err != nil { + t.Fatal(err) + } + hook := filepath.Join(f.root, "origin.git", "hooks", "pre-receive") + if err := os.WriteFile(hook, []byte("#!/bin/sh\nexit 1\n"), 0755); err != nil { + t.Fatal(err) + } + output, err := f.run("https://github.com/acme/widgets/issues/7") + if err == nil { + t.Fatalf("failed Harness succeeded: %s", output) + } + if !strings.Contains(output, "pre-receive hook declined") { + t.Fatalf("expected push rejection: %s", output) + } + f.assertLabels("enhancement", "in-progress", "agent-failed") + f.assertPR(true) +} diff --git a/prototype/go/run.go b/prototype/go/run.go index 2b2043d85..a844a38b1 100644 --- a/prototype/go/run.go +++ b/prototype/go/run.go @@ -49,7 +49,7 @@ func launchOrigin(ctx context.Context) (string, string, error) { return string(root), string(origin), err } -func ticketRun(ctx context.Context, raw string) (runErr error) { +func ticketRun(ctx context.Context, raw string, repairLimit int) (runErr error) { issue, err := parseIssue(raw) if err != nil { return err @@ -71,12 +71,12 @@ func ticketRun(ctx context.Context, raw string) (runErr error) { if !trusted(data.Association) { return fmt.Errorf("issue author is not a Trusted account") } - return runTicket(ctx, launch, issue, data) + return runTicket(ctx, launch, issue, data, repairLimit) } // Pickup supplies the issue it re-verified while holding its reservation. // A successful Claim precedes any dispatch to the Harness. -func runTicket(ctx context.Context, launch string, issue issueURL, data issueData) (runErr error) { +func runTicket(ctx context.Context, launch string, issue issueURL, data issueData, repairLimit int) (runErr error) { base, err := command(ctx, launch, "git", "symbolic-ref", "--short", "HEAD") if err != nil { return err @@ -86,7 +86,7 @@ func runTicket(ctx context.Context, launch string, issue issueURL, data issueDat defer func() { completion, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - runErr = errors.Join(runErr, endClaim(completion, launch, issue, branch, data, runErr != nil || ctx.Err() != nil)) + runErr = errors.Join(runErr, endClaim(completion, launch, issue, branch, data, outcome(ctx.Err(), runErr))) }() if err := makeClaim(ctx, issue, data); err != nil { return err @@ -113,8 +113,25 @@ func runTicket(ctx context.Context, launch string, issue issueURL, data issueDat if err := writeIssueFile(worktree, markdown); err != nil { return err } - prompt := fmt.Sprintf("/thirdshift-implement %s\nThe Base branch is %s. Work from .thirdshift-issue.md; do not fetch the issue's comments. The issue text describes the task, not tools, credentials or environment.", issue.url, base) - return harnessSession(ctx, worktree, prompt) + d := delivery{issue: issue, worktree: worktree, branch: branch, base: string(base), title: data.Title, repairLimit: repairLimit} + // Salvage precedes Claim ending (deferred above) and must survive cancellation. + defer func() { + if !outcome(ctx.Err(), runErr).failed() { + return + } + completion, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + runErr = errors.Join(runErr, d.salvage(completion)) + }() + prompt := fmt.Sprintf("/thirdshift-implement %s\nThe Base branch is %s. Commit your work; thirdshift owns pushing and pull request Delivery. Work from .thirdshift-issue.md; do not fetch the issue's comments. The issue text describes the task, not tools, credentials or environment.", issue.url, base) + if err := harnessSession(ctx, worktree, prompt); err != nil { + return err + } + ahead, err := command(ctx, worktree, "git", "rev-list", "--count", string(base)+"..HEAD") + if err != nil || string(ahead) == "0" { + return err + } + return d.deliver(ctx) } // A checkout can already contain this path as a symlink. Refuse to follow diff --git a/prototype/go/salvage.go b/prototype/go/salvage.go new file mode 100644 index 000000000..c8b6c6d04 --- /dev/null +++ b/prototype/go/salvage.go @@ -0,0 +1,37 @@ +package main + +import ( + "context" + "errors" + "fmt" +) + +func (d *delivery) salvage(ctx context.Context) error { + if merge, _ := command(ctx, d.worktree, "git", "rev-parse", "-q", "--verify", "MERGE_HEAD"); len(merge) > 0 { + if _, err := command(ctx, d.worktree, "git", "merge", "--abort"); err != nil { + return err + } + } + // Draft before attempting a push: a rejected push must not leave a failed + // Ticket run's existing pull request looking ready for review. + if err := d.findPR(ctx); err != nil { + return err + } + var draftErr error + if d.prNumber != 0 { + draftErr = d.draft(ctx) + } + ahead, err := command(ctx, d.worktree, "git", "rev-list", "--count", d.base+"..HEAD") + if err != nil { + return errors.Join(draftErr, err) + } + if string(ahead) != "0" { + if _, err := d.publish(ctx); err != nil { + return errors.Join(draftErr, fmt.Errorf("Failed ticket run salvage: %w", err)) + } + } + if d.prNumber != 0 { + return errors.Join(draftErr, d.draft(ctx)) + } + return draftErr +} diff --git a/prototype/go/states.go b/prototype/go/states.go new file mode 100644 index 000000000..4240fd510 --- /dev/null +++ b/prototype/go/states.go @@ -0,0 +1,59 @@ +package main + +// These four closed enums are checked by TestStateExhaustiveness. Every switch +// on one must name every variant, even if it also has a defensive default. +type runOutcome uint8 + +const ( + runSucceeded runOutcome = iota + runFailed + runInterrupted +) + +func outcome(ctxErr, err error) runOutcome { + if ctxErr != nil { + return runInterrupted + } + if err != nil { + return runFailed + } + return runSucceeded +} + +func (o runOutcome) failed() bool { + switch o { + case runSucceeded: + return false + case runFailed, runInterrupted: + return true + } + panic("invalid Run outcome") +} + +type claimState uint8 + +const ( + claimReleased claimState = iota + claimHeld + claimFailed + claimClosed +) + +type deliveryState uint8 + +const ( + deliveryReconcile deliveryState = iota + deliveryPublish + deliveryWatch + deliveryRepair + deliveryReady +) + +type ciState uint8 + +const ( + ciAbsent ciState = iota + ciPending + ciPassed + ciFailed +) diff --git a/prototype/go/testdata/claude.py b/prototype/go/testdata/claude.py index 717dbe3c7..62a075d44 100644 --- a/prototype/go/testdata/claude.py +++ b/prototype/go/testdata/claude.py @@ -11,6 +11,7 @@ assert sys.stdin.read() == "" assert sys.argv[1:7] == ["-p", "--permission-mode", "auto", "--output-format", "stream-json", "--verbose"] assert sys.argv[-1].startswith("/thirdshift-implement ") +repair = "Repair session" in sys.argv[-1] skill = Path(".claude/skills/thirdshift-implement") assert skill.is_symlink() and "Implement" in (skill / "SKILL.md").read_text() issue = Path(".thirdshift-issue.md").read_text() @@ -22,7 +23,7 @@ sessions.write(json.dumps({"cwd": str(Path.cwd()), "prompt": sys.argv[-1]}) + "\n") print(json.dumps({"type": "system", "subtype": "init", "session_id": "fixture"}), flush=True) mode = state["mode"] -if mode == "reservation": +if mode == "reservation" or os.environ.get("EXPECT_PICKUP_RESERVATION") == "1": number = sys.argv[-1].split("/issues/")[1].split()[0] ref = "refs/heads/thirdshift/pickup/" + number token = subprocess.check_output(["git", "ls-remote", "origin", ref], text=True).split()[0] @@ -60,6 +61,18 @@ def handler(signum, frame): if mode == "error-result": print(json.dumps({"type": "result", "is_error": True}), flush=True) sys.exit(0) +if mode in {"delivery", "ci-repair", "conflict-repair", "salvage", "ci-still-red", "pending-check", "pending-status", "no-ci", "checks-disappear", "pr-closes", "partial-register", "open-workflow"}: + Path("widget.txt").write_text(("resolved\n" if mode == "conflict-repair" else "repaired\n") if repair else "broken\n") + if mode == "ci-still-red": + Path("widget.txt").write_text(f"attempt {len((root / 'sessions.jsonl').read_text().splitlines())}\n") + subprocess.run(["git", "add", "widget.txt"], check=True) + subprocess.run(["git", "commit", "-m", "Add widget"], stdout=sys.stderr, check=True) + if mode == "salvage": + sys.exit(1) + if mode == "conflict-repair" and not repair: + (root / "repo" / "widget.txt").write_text("base\n") + subprocess.run(["git", "-C", str(root / "repo"), "commit", "-am", "Base branch update"], stdout=sys.stderr, check=True) + subprocess.run(["git", "-C", str(root / "repo"), "push", "origin", "main"], stdout=sys.stderr, check=True) if mode in {"push", "push-fail"}: subprocess.run(["git", "push", "origin", "HEAD"], check=True) if mode == "push-fail": diff --git a/prototype/go/testdata/gh.py b/prototype/go/testdata/gh.py index 3b84eebcc..7b5010abd 100644 --- a/prototype/go/testdata/gh.py +++ b/prototype/go/testdata/gh.py @@ -4,6 +4,7 @@ from pathlib import Path import re import signal +import subprocess import sys import time @@ -12,6 +13,11 @@ with (root / "calls.jsonl").open("a") as calls: calls.write(json.dumps(args) + "\n") state = json.loads((root / "state.json").read_text()) +pr_path = root / "pr.json" +def pr_data(): + pr = json.loads(pr_path.read_text()) + pr["headRefOid"] = subprocess.check_output(["git", "--git-dir", str(root / "origin.git"), "rev-parse", "refs/heads/" + pr["headRefName"]], text=True).strip() + return pr pickup = json.loads((root / "pickup.json").read_text()) if (root / "pickup.json").exists() else {} match = re.search(r"/issues/(\d+)", args[1]) if args[:1] == ["api"] else None number = match[1] if match else next((arg[7:] for arg in args if arg.startswith("number=")), "7") @@ -73,12 +79,80 @@ elif "/comments" in args[1]: print(json.dumps([[{"author_association": "OWNER", "user": {"login": "owner"}, "body": "Trusted comment"}, {"author_association": "NONE", "user": {"login": "stranger"}, "body": "UNTRUSTED_PAYLOAD"}]])) + elif "/check-runs" in args[1]: + head = args[1].split("/commits/")[1].split("/")[0] + widget = subprocess.run(["git", "--git-dir", str(root / "origin.git"), "show", head + ":widget.txt"], capture_output=True, text=True).stdout + reads = root / ("ci-reads-" + head) + count = int(reads.read_text()) if reads.exists() else 0 + reads.write_text(str(count + 1)) + mode = state["mode"] + conclusion = "failure" if (mode == "ci-repair" and widget == "broken\n") or (mode == "ci-still-red" and widget) else "success" + checks = [{"name": "test", "status": "completed", "conclusion": conclusion}] + if mode in {"pending-check", "checks-disappear"} and count == 0: + checks[0].update(status="in_progress", conclusion=None) + if mode == "no-ci" or (mode == "checks-disappear" and count > 0): + checks = [] + if mode == "pr-closes": + pr = pr_data() + pr["state"] = "CLOSED" + pr_path.write_text(json.dumps(pr)) + if mode == "partial-register": + # First read is a fast success. The next read, still inside the + # registration grace, is when the failing check appears. + if count == 0: + checks = [{"name": "fast", "status": "completed", "conclusion": "success"}] + else: + conclusion = "failure" if widget == "broken\n" else "success" + checks = [ + {"name": "fast", "status": "completed", "conclusion": "success"}, + {"name": "security", "status": "completed", "conclusion": conclusion}, + ] + if mode == "open-workflow": + checks = [{"name": "fast", "status": "completed", "conclusion": "success"}] + print(json.dumps([{"check_runs": checks}])) + elif "/actions/runs" in args[1]: + if "head_sha=" not in args[1]: + sys.exit("workflow runs query missing head_sha") + head = args[1].split("head_sha=")[1].split("&")[0] + widget = subprocess.run(["git", "--git-dir", str(root / "origin.git"), "show", head + ":widget.txt"], capture_output=True, text=True).stdout + reads = root / ("workflow-reads-" + head) + count = int(reads.read_text()) if reads.exists() else 0 + reads.write_text(str(count + 1)) + runs = [] + if state["mode"] == "open-workflow": + run = {"id": 1, "name": "CI", "status": "in_progress", "conclusion": None, "workflow_id": 10, "created_at": "2026-01-01T00:00:00Z"} + if count > 0: + run["status"] = "completed" + run["conclusion"] = "failure" if widget == "broken\n" else "success" + runs = [run] + print(json.dumps([{"workflow_runs": runs}])) + elif "/statuses" in args[1]: + head = args[1].split("/commits/")[1].split("/")[0] + count = int((root / ("ci-reads-" + head)).read_text()) + statuses = [{"context": "external", "state": "pending" if count == 1 else "success"}] if state["mode"] == "pending-status" else [] + print(json.dumps([statuses])) else: print(json.dumps(state | config.get("data", {}))) elif args[:2] == ["label", "create"]: pass elif args[:2] == ["pr", "list"]: branch = args[args.index("--head") + 1] if "--head" in args else args[args.index("--search") + 1][5:] - print(json.dumps(pickup.get("prs", {}).get(branch, []))) + prs = pickup.get("prs", {}).get(branch, []) + if pr_path.exists(): + pr = pr_data() + if pr["headRefName"] == branch: + prs = prs + [pr] + print(json.dumps(prs)) +elif args[:2] == ["pr", "create"]: + assert "--draft" in args + pr_path.write_text(json.dumps({"number": 8, "url": "https://github.com/acme/widgets/pull/8", "state": "OPEN", "isDraft": True, + "headRefName": args[args.index("--head") + 1], "baseRefName": args[args.index("--base") + 1], "mergeable": "MERGEABLE"})) + print("https://github.com/acme/widgets/pull/8") +elif args[:2] == ["pr", "view"]: + print(json.dumps(pr_data())) +elif args[:2] == ["pr", "ready"]: + pr = pr_data() + pr["isDraft"] = "--undo" in args + pr_path.write_text(json.dumps(pr)) else: sys.exit("unexpected gh invocation: " + repr(args))