fix: when user remove ai-rework, loope read the source code in worktree and say: this is implemented (#49)

ai/prompts/comments.md.tmpl

@@ -12,7 +12,7 @@
 
 {{define "park"}}🤖 Parked as `{{.Label}}` — this issue will not be retried automatically.
 
-Remove the `{{.Label}}` label to queue a fresh attempt — any worktree, branch and logs this run produced are preserved and reused, so no work is lost.{{if .Guidance}}
+Remove the `{{.Label}}` label to resume this run in the same Claude session — the worktree, branch, logs and session are preserved, so nothing is redone from zero.{{if .Guidance}}
 
 {{.Guidance}}{{end}}{{if .Error}}
 

ai/prompts/resume.md.tmpl

@@ -0,0 +1,11 @@
+/superpowers:executing-plans — continue
+
+This is a rework pickup: the same worktree from a previous, interrupted run
+(usage limit, crash, or similar) that landed some partial progress. You are
+resuming that same session, not starting over. Do NOT conclude the issue is
+already implemented just because some code already exists in the worktree —
+inspect what is actually there, keep going, and finish: complete the
+remaining implementation, make the full test suite pass, and commit.
+
+HEADLESS: do not ask questions; make reasonable calls and note them in commit
+messages.

loop.go

@@ -222,32 +222,58 @@ func (o *Orchestrator) selectIssues(ctx context.Context, issues []Issue, limit i
 func (o *Orchestrator) handleIssue(ctx context.Context, issue Issue, kind, base string) error {
 	n := issue.Number
 	branch := branchName(n)
+	logDir := o.issueLogDir(n)
+	// A park cause still on disk means this issue was parked as ai-rework and a
+	// human just removed the label — the ONLY way it re-enters ListEligibleIssues
+	// from that state. Nothing scans ai-rework or acts on it automatically; this
+	// only fires on the pickup that follows a human's own action. Resolve the
+	// resumable session (and its pipeline kind) before ai-wip overwrites nothing
+	// on disk that readSession needs.
+	resumeSession, resumeKind := "", ""
+	if hasParkCause(logDir) {
+		if si, err := readSession(logDir); err == nil {
+			resumeSession, resumeKind = si.SessionID, si.Kind
+		}
+	}
+	if resumeSession != "" && resumeKind != "" {
+		kind = resumeKind
+	}
+
 	if err := o.gh.AddLabel(ctx, n, o.cfg.StateLabels.WIP); err != nil {
 		return err
 	}
-	recordState(o.issueLogDir(n), o.cfg.StateLabels.WIP)
+	recordState(logDir, o.cfg.StateLabels.WIP)
 	// Mirror the title next to the state marker: the dashboard otherwise knows
 	// it only for as long as the issue keeps matching its label-scoped query.
-	recordTitle(o.issueLogDir(n), issue.Title)
+	recordTitle(logDir, issue.Title)
 	_ = o.gh.Comment(ctx, n, pickupComment(kind, branch))
 
 	wtPath, err := o.wt.Create(ctx, o.cfg.WorkDir, n, base)
 	if err != nil {
 		return o.abort(ctx, n, err)
 	}
-	content, err := o.gh.FetchIssueContent(ctx, n)
-	if err != nil {
-		return o.abort(ctx, n, err)
-	}
-	content = DownloadIssueImages(ctx, o.runner, content, o.issueLogDir(n))
 
-	c := &Claude{runner: o.runner, logDir: o.issueLogDir(n), configDir: o.cfg.ClaudeConfigDir}
+	c := &Claude{runner: o.runner, logDir: logDir, configDir: o.cfg.ClaudeConfigDir}
 	uat := &UAT{Target: o.gh, Num: n}
 	var perr error
-	if kind == "bug" {
-		perr = RunBugPipeline(ctx, c, o.cfg, wtPath, content, base, uat)
+	if resumeSession != "" {
+		// A rework pickup resumes the exact prior session in its preserved
+		// worktree instead of fetching the issue content and running a fresh
+		// session — a fresh session only sees the worktree's current contents
+		// and can mistake the partial progress already there for a finished
+		// feature, closing the issue with no PR (the bug this branch fixes).
+		perr = RunResumePipeline(ctx, c, o.cfg, wtPath, kind, resumeSession)
 	} else {
-		perr = RunFeaturePipeline(ctx, c, o.cfg, wtPath, content, readPersona(o.cfg.PersonaPath), uat)
+		content, err := o.gh.FetchIssueContent(ctx, n)
+		if err != nil {
+			return o.abort(ctx, n, err)
+		}
+		content = DownloadIssueImages(ctx, o.runner, content, logDir)
+		if kind == "bug" {
+			perr = RunBugPipeline(ctx, c, o.cfg, wtPath, content, base, uat)
+		} else {
+			perr = RunFeaturePipeline(ctx, c, o.cfg, wtPath, content, readPersona(o.cfg.PersonaPath), uat)
+		}
 	}
 	// A Stop landed during the pipeline: skip the normal park/ship/finish outcome
 	// and leave the ticket ai-wip. The launching goroutine's consumeStopping+pause

no_auto_retry_test.go

@@ -9,12 +9,14 @@ import (
 	"time"
 )
 
-// ai-rework is a TERMINAL state: the daemon must not even look at the label. A
-// parked issue moves only when a human removes it, which puts it back in the
-// eligible queue the normal cycle already reads. Retrying failures automatically
-// is what re-ran the whole pipeline on one broken issue every cycle, burning
-// tokens on work nobody was watching.
-func TestDaemonNeverScansOrResumesParkedIssues(t *testing.T) {
+// ai-rework is a TERMINAL state: the daemon must not scan the label or act on
+// a parked issue on its own. It moves only when a human removes it, which puts
+// it back in the eligible queue the normal cycle already reads — that pickup
+// resumes the preserved session instead of scanning/polling ai-rework itself,
+// so this stays a one-shot reaction to the human's own edit, not the
+// every-cycle automatic retry that used to burn tokens on work nobody was
+// watching.
+func TestDaemonNeverScansParkedIssuesButResumesOnceRequeued(t *testing.T) {
 	env := newFakeEnv(t)
 	prepParked(t, env, "claude debug: terminated: api_error; api status 429; You've hit your usage limit")
 	shipped := make(chan struct{}, 1)
@@ -37,7 +39,13 @@ func TestDaemonNeverScansOrResumesParkedIssues(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	done := make(chan struct{})
 	go func() {
-		runLoop(ctx, o, o.cfg, true /* sweep */)
+		// sweep=false: the fake's "issue list" stub answers every query with the
+		// same issue-7 payload regardless of --label, so a startup sweep for
+		// ai-wip would "find" issue 7 and clear the park cause this test set up
+		// before the cycle even runs. Sweeping stale ai-wip is exercised
+		// elsewhere (TestSweepOrphansRequeuesStaleWIP); this test is only about
+		// the pickup that follows a human removing ai-rework.
+		runLoop(ctx, o, o.cfg, false /* sweep */)
 		close(done)
 	}()
 	select {
@@ -55,8 +63,33 @@ func TestDaemonNeverScansOrResumesParkedIssues(t *testing.T) {
 	if got := env.callsMatching("gh", "--label ai-rework"); len(got) != 0 {
 		t.Errorf("a cycle must never scan the rework label, got %v", got)
 	}
-	if got := env.callsMatching("claude", "--resume"); len(got) != 0 {
-		t.Errorf("a cycle must never resume a saved session, got %v", got)
+	if got := env.callsMatching("claude", "--resume s1"); len(got) == 0 {
+		t.Error("the requeued pickup must resume the session prepParked left behind")
+	}
+}
+
+// Reproduces #49: a rework pickup whose resumed session's final reply reads
+// like an already-done claim (plausible — a resumed session mid-implementation
+// looking at its own partially-landed code can sound like that) must still
+// ship a PR from whatever it committed, not close the issue with none.
+func TestReworkPickupShipsEvenWhenReplyLooksAlreadyDone(t *testing.T) {
+	env := newFakeEnv(t)
+	prepParked(t, env, "claude execute: terminated: max_turns")
+	base := env.f.handler
+	env.f.handler = func(c rcall) (string, string, error) {
+		if c.name == "claude" && strings.Contains(c.stdin, "rework pickup") {
+			return claudeJSON("PIPELINE_ALREADY_DONE: this looks finished already", "s2"), "", nil
+		}
+		return base(c)
+	}
+	if err := runCycle(env.orchestrator()); err != nil {
+		t.Fatalf("cycle error = %v", err)
+	}
+	if len(env.callsMatching("gh", "pr create")) == 0 {
+		t.Error("a rework pickup must ship a PR even when the resumed reply looks already-done")
+	}
+	if len(env.callsMatching("gh", "issue close")) != 0 {
+		t.Error("a rework pickup must not close the issue as already implemented")
 	}
 }
 

pipeline_resume.go

@@ -0,0 +1,32 @@
+package main
+
+import "context"
+
+// RunResumePipeline drives a rework pickup: a single Claude turn that resumes
+// the exact session a prior run left off in, in its preserved worktree,
+// instead of starting a fresh session that only sees the worktree's current
+// contents and can mistake partial progress for a finished feature. Unlike
+// RunBugPipeline and RunFeaturePipeline, it never checks for an
+// already-implemented claim — that check exists to let a FRESH session bail
+// out of duplicate work, and is meaningless (and actively harmful: it would
+// close the issue with no PR) applied to a session that is itself mid-way
+// through implementing the issue.
+func RunResumePipeline(ctx context.Context, c *Claude, cfg *Config, wtPath, kind, sessionID string) error {
+	res, err := c.Call(ctx, ClaudeCall{
+		Dir: wtPath, Label: "resume", Prompt: resumePrompt(), Resume: sessionID,
+		Model:           cfg.Models.Architect,
+		SkipPermissions: true,
+		DisallowedTools: []string{"AskUserQuestion"},
+	})
+	// Record before the error check: an errored call (e.g. a 429 session limit)
+	// still returns a session id, so the next rework pickup resumes THAT one
+	// instead of the now-stale id this run started from.
+	if res != nil {
+		c.RecordSession(res.SessionID, kind)
+	}
+	return err
+}
+
+func resumePrompt() string {
+	return mustRender("resume.md.tmpl", promptData())
+}

pipeline_resume_test.go

@@ -0,0 +1,106 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"testing"
+)
+
+func TestResumePipelineResumesTheSavedSession(t *testing.T) {
+	f := &fakeRunner{queue: []rresp{{stdout: claudeJSON("Finished the implementation and committed.", "s2")}}}
+	c := &Claude{runner: f}
+	cfg := &Config{Models: Models{Architect: ModelConfig{Model: "opus", Effort: "high"}}}
+	if err := RunResumePipeline(context.Background(), c, cfg, "/wt", "bug", "s1"); err != nil {
+		t.Fatal(err)
+	}
+	if len(f.calls) != 1 {
+		t.Fatalf("calls = %d, want 1", len(f.calls))
+	}
+	call := f.calls[0]
+	if argAfter(call.args, "--resume") != "s1" {
+		t.Errorf("must resume the saved session id, args = %v", call.args)
+	}
+	if call.dir != "/wt" || !hasArg(call.args, "--dangerously-skip-permissions") ||
+		argAfter(call.args, "--model") != "opus" {
+		t.Errorf("call = %+v", call)
+	}
+}
+
+// A rework pickup must never close the issue as already-implemented: that
+// check exists to let a FRESH session bail out of duplicate work, and applied
+// to a session mid-way through implementing the issue it is the exact bug
+// this pipeline exists to fix (closing with no PR). So even a reply carrying
+// the sentinel is not parsed as one.
+func TestResumePipelineNeverReturnsAlreadyDone(t *testing.T) {
+	f := &fakeRunner{queue: []rresp{{stdout: claudeJSON(
+		"PIPELINE_ALREADY_DONE: looks finished to me", "s2")}}}
+	c := &Claude{runner: f}
+	cfg := &Config{Models: Models{Architect: ModelConfig{Model: "opus"}}}
+	if err := RunResumePipeline(context.Background(), c, cfg, "/wt", "feature", "s1"); err != nil {
+		t.Fatalf("resume must not treat an already-done claim as an error outcome: %v", err)
+	}
+}
+
+func TestResumePipelinePropagatesError(t *testing.T) {
+	f := &fakeRunner{queue: []rresp{{err: fmt.Errorf("exit 1")}}}
+	c := &Claude{runner: f}
+	cfg := &Config{Models: Models{Architect: ModelConfig{Model: "opus"}}}
+	if err := RunResumePipeline(context.Background(), c, cfg, "/wt", "bug", "s1"); err == nil {
+		t.Error("want error, got nil")
+	}
+}
+
+func TestResumePipelineRecordsSession(t *testing.T) {
+	logDir := t.TempDir()
+	f := &fakeRunner{queue: []rresp{{stdout: claudeJSON("Finished and committed.", "s2")}}}
+	c := &Claude{runner: f, logDir: logDir}
+	cfg := &Config{Models: Models{Architect: ModelConfig{Model: "opus"}}}
+	if err := RunResumePipeline(context.Background(), c, cfg, "/wt", "feature", "s1"); err != nil {
+		t.Fatal(err)
+	}
+	si, err := readSession(logDir)
+	if err != nil {
+		t.Fatalf("session not recorded: %v", err)
+	}
+	if si.SessionID != "s2" || si.Kind != "feature" {
+		t.Errorf("session = %+v, want s2/feature", si)
+	}
+}
+
+// A resume that errors again (e.g. a fresh 429) must still advance the saved
+// session id, exactly like the bug/feature pipelines, so the NEXT rework
+// pickup resumes the latest session instead of the now-stale one this run
+// started from.
+func TestResumePipelineRecordsSessionOnError(t *testing.T) {
+	logDir := t.TempDir()
+	f := &fakeRunner{queue: []rresp{{stdout: claudeErrorJSON("hit usage limit again", "s3")}}}
+	c := &Claude{runner: f, logDir: logDir}
+	cfg := &Config{Models: Models{Architect: ModelConfig{Model: "opus"}}}
+	if err := RunResumePipeline(context.Background(), c, cfg, "/wt", "bug", "s1"); err == nil {
+		t.Fatal("want the error propagated so the issue is parked again")
+	}
+	si, err := readSession(logDir)
+	if err != nil {
+		t.Fatalf("session must be recorded even when the call errors: %v", err)
+	}
+	if si.SessionID != "s3" {
+		t.Errorf("session = %+v, want s3", si)
+	}
+}
+
+func TestResumePipelinePromptContinuesRatherThanRestarts(t *testing.T) {
+	f := &fakeRunner{queue: []rresp{{stdout: claudeJSON("done", "s2")}}}
+	c := &Claude{runner: f}
+	cfg := &Config{Models: Models{Architect: ModelConfig{Model: "opus"}}}
+	if err := RunResumePipeline(context.Background(), c, cfg, "/wt", "bug", "s1"); err != nil {
+		t.Fatal(err)
+	}
+	prompt := f.calls[0].stdin
+	if !strings.Contains(strings.ToLower(prompt), "resuming") && !strings.Contains(strings.ToLower(prompt), "continue") {
+		t.Errorf("resume prompt should tell the session to continue rather than restart: %s", prompt)
+	}
+	if strings.Contains(prompt, alreadyDoneSentinel) {
+		t.Errorf("resume prompt must not invite an already-done claim: %s", prompt)
+	}
+}

prompts_golden_test.go

@@ -196,7 +196,7 @@ func TestGoldenNeedsInfoComment(t *testing.T) {
 }
 
 const parkHead = "\U0001f916 Parked as `ai-rework` — this issue will not be retried automatically.\n\n" +
-	"Remove the `ai-rework` label to queue a fresh attempt — any worktree, branch and logs this run produced are preserved and reused, so no work is lost."
+	"Remove the `ai-rework` label to resume this run in the same Claude session — the worktree, branch, logs and session are preserved, so nothing is redone from zero."
 
 // parkTail is the hidden marker every status comment ends with, so
 // FetchIssueContent can strip it back out of the next run's issue content.

prompts_test.go

@@ -17,6 +17,7 @@ var promptTestData = map[string]map[string]any{
 	"plan.md.tmpl":         {"SpecPath": "docs/spec.md"},
 	"execute.md.tmpl":      {"PlanPath": "docs/plan.md"},
 	"debug.md.tmpl":        {"Issue": "I", "Threshold": 70},
+	"resume.md.tmpl":       {},
 	"triage.md.tmpl":       {"List": "[]"},
 	"pickup":               {"Kind": "feature", "Branch": "b"},
 	"already-done":         {"Reason": "R"},

tracker.go

@@ -238,11 +238,12 @@ func clearState(logDir string) {
 	_ = os.Remove(filepath.Join(logDir, stateFile))
 }
 
-// parkCauseFile holds the failure text that parked the issue as ai-rework. It is
-// a diagnostic left next to the logs for whoever inspects the workDir: nothing in
-// the daemon reads it back, because a parked issue only moves when a human
-// removes the label. It is cleared when the issue leaves the parked state, so a
-// stale cause can't outlive the failure it describes.
+// parkCauseFile holds the failure text that parked the issue as ai-rework. It
+// doubles as the on-disk marker handleIssue reads back (via hasParkCause) to
+// tell a rework pickup — a human removed the label, so the issue is eligible
+// again — apart from a genuinely fresh issue. It is cleared when the issue
+// leaves the parked state, so a stale cause can't outlive the failure it
+// describes and can't wrongly mark the issue's next fresh pickup as a rework.
 const parkCauseFile = "park-cause"
 
 // recordParkCause writes the park cause to <logDir>/park-cause. Best-effort,
@@ -265,6 +266,17 @@ func clearParkCause(logDir string) {
 	_ = os.Remove(filepath.Join(logDir, parkCauseFile))
 }
 
+// hasParkCause reports whether logDir still carries a park cause, i.e. this
+// issue was parked as ai-rework and never reached a terminal outcome since —
+// the signal handleIssue uses to tell a rework pickup from a fresh one.
+func hasParkCause(logDir string) bool {
+	if logDir == "" {
+		return false
+	}
+	_, err := os.Stat(filepath.Join(logDir, parkCauseFile))
+	return err == nil
+}
+
 // scanLogs reads cfg.WorkDir/logs and returns one Ticket per issue-<N> dir,
 // steps ordered by seq and cost summed, sorted by attention-priority status
 // tier then Number descending (see sortTickets). A missing logs dir yields an