fix(tick): groom again when an issue changes after its contract

card.go

@@ -49,6 +49,7 @@ type cardState struct {
 	Repairs  int                 `json:"repairs,omitempty"`
 	Contract string              `json:"contract,omitempty"`
 	Approved string              `json:"approved,omitempty"`
+	Groomed  string              `json:"groomed,omitempty"`
 	Items    map[int]cardItem    `json:"items,omitempty"`
 	History  []cardEvent         `json:"history,omitempty"`
 }

docs/security.md

@@ -168,6 +168,9 @@ instructions. ghafk applies these limits:
 - An issue from a person without write access waits for `/start` from a
   maintainer. If its title or description changes after `/start`, the
   issue parks. It then needs a new `/start`.
+- When the title or description of any issue changes after ghafk wrote
+  its contract, ghafk writes a new contract from the new text before it
+  starts the work, so a stale contract is never built.
 - Slash commands count only from people with write access.
 - ghafk works and merges only the pull requests that it opened, from the
   same repository.

groom.go

@@ -51,6 +51,7 @@ func groomIssue(home, repo, base, login string, wf workflow, is issue) error {
 	case strings.HasPrefix(answer, "contract:") && strings.TrimSpace(strings.TrimPrefix(answer, "contract:")) != "":
 		stepf(base, is.Number, "contract")
 		c.st.Contract = strings.TrimSpace(strings.TrimPrefix(answer, "contract:"))
+		c.st.Groomed = issueDigest(is)
 		c.finish("done", tokens)
 		c.queue()
 		if c.err != nil {

implement_test.go

@@ -238,3 +238,30 @@ func TestAContractWithASecretIsStoredRedacted(t *testing.T) {
 		t.Fatalf("no card with the contract was written: %v", *calls)
 	}
 }
+
+func TestGroomingRecordsTheIssueTextItWasGiven(t *testing.T) {
+	needSandbox(t)
+	repo := gitClone(t)
+	calls := fakeGH(t, func(cmd string) (string, error) {
+		if strings.HasPrefix(cmd, "repo view") {
+			return "main", nil
+		}
+		return "", nil
+	})
+	is := issue{Number: 5, Title: "t", Body: "the text"}
+	wf := workflow{label: "agent", groomer: harness.Role{Command: `printf 'contract:\n## Change\ndo it\n'`}, timeout: time.Minute}
+	if err := groomIssue(t.TempDir(), repo, "demo", "owner", wf, is); err != nil {
+		t.Fatal(err)
+	}
+	for _, c := range *calls {
+		for _, arg := range c.args {
+			if st, ok := parseCard(strings.TrimPrefix(arg, "body=")); ok && st.Contract != "" {
+				if st.Groomed != issueDigest(is) {
+					t.Fatalf("the card must record the issue text the contract came from, got %q", st.Groomed)
+				}
+				return
+			}
+		}
+	}
+	t.Fatalf("no card with the contract was written: %v", *calls)
+}

tick.go

@@ -146,10 +146,11 @@ func workRepo(home, repo, login string, henv harness.Env) error {
 	if tooManyComments(is.Comments) {
 		return parkTooManyComments(repo, base, login, wf.label, is)
 	}
-	if needsApproval(is, openCard(repo, is, login).st) {
+	st := openCard(repo, is, login).st
+	if needsApproval(is, st) {
 		return parkForApproval(repo, base, login, wf.label, is)
 	}
-	if contractText(is.Comments, login) != "" && !staleAfterContract(is.Comments, login) {
+	if contractText(is.Comments, login) != "" && !staleAfterContract(is.Comments, login) && !editedSinceGroom(is, st) {
 		if err := runIssue(home, repo, base, login, wf, contractText(is.Comments, login), is); err != nil {
 			return fmt.Errorf("#%d: %w", is.Number, err)
 		}

tick_test.go

@@ -4,8 +4,10 @@ import (
 	"encoding/json"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/mike-diff/ghafk/internal/harness"
 )
@@ -76,3 +78,66 @@ func TestAFloodedPullRequestParks(t *testing.T) {
 		t.Fatalf("a PR with more comments than gh returns was worked blind: %v", *calls)
 	}
 }
+
+func tickWithGroomedCard(t *testing.T, groomedBody, currentBody string) []ghCall {
+	t.Helper()
+	needSandbox(t)
+	repo := gitClone(t)
+	if err := os.MkdirAll(filepath.Join(repo, ".ghafk"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	flow := "---\nlabel: agent\nchecks: true\ngroomer: echo groomer-ran; false\nworker: echo worker-ran; false\njudge: false\n---\n"
+	if err := os.WriteFile(filepath.Join(repo, ".ghafk", "WORKFLOW.md"), []byte(flow), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	groomed := issue{Title: "t", Body: groomedBody}
+	st := cardState{Number: 5, Title: "t", Phase: "queued", Contract: "## Change\nthe old contract", Groomed: issueDigest(groomed)}
+	card, _ := json.Marshal(renderCardBody(st, time.UTC, false))
+	issues := `[{"number":5,"author":{"login":"owner"},"title":"t","body":` + strconv.Quote(currentBody) + `,"labels":[{"name":"agent"}],"comments":[{"body":` + string(card) + `,"author":{"login":"owner"},"authorAssociation":"OWNER"}]}]`
+	calls := fakeGH(t, func(cmd string) (string, error) {
+		switch {
+		case strings.HasPrefix(cmd, "issue list"):
+			return issues, nil
+		case strings.HasPrefix(cmd, "pr list"):
+			return "[]", nil
+		case strings.HasPrefix(cmd, "repo view"):
+			return "main", nil
+		case strings.Contains(cmd, "/permission"):
+			return "admin", nil
+		}
+		return "", nil
+	})
+	_ = workRepo(t.TempDir(), repo, "owner", harness.Env{})
+	return *calls
+}
+
+func ranRole(calls []ghCall, role string) bool {
+	for _, c := range calls {
+		for _, arg := range c.args {
+			if strings.Contains(arg, "**ghafk** · "+role+" ·") {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+func TestAnIssueEditedAfterGroomingIsGroomedAgain(t *testing.T) {
+	calls := tickWithGroomedCard(t, "the old text", "the new text")
+	if !ranRole(calls, "groomer") || ranRole(calls, "worker") {
+		t.Fatalf("an issue whose text changed after grooming must be groomed again, not built from the old contract: %v", calls)
+	}
+}
+
+func TestAnUnchangedIssueKeepsItsContract(t *testing.T) {
+	calls := tickWithGroomedCard(t, "the same text", "the same text")
+	if !ranRole(calls, "worker") || ranRole(calls, "groomer") {
+		t.Fatalf("an issue whose text did not change must go on to the worker with its contract: %v", calls)
+	}
+}
+
+func TestACardFromBeforeTheDigestKeepsItsContract(t *testing.T) {
+	if editedSinceGroom(issue{Title: "t", Body: "any text"}, cardState{Contract: "c"}) {
+		t.Fatal("a card written before ghafk recorded the groomed text must keep its contract, not re-groom every issue in flight")
+	}
+}

trust.go

@@ -40,6 +40,10 @@ func issueDigest(is issue) string {
 	return hex.EncodeToString(sum[:])
 }
 
+func editedSinceGroom(is issue, st cardState) bool {
+	return st.Groomed != "" && st.Groomed != issueDigest(is)
+}
+
 func needsApproval(is issue, st cardState) bool {
 	return !canWrite(is.Author.Login) && st.Approved != issueDigest(is)
 }