Skip to content

Commit e21869f

Browse files
stack72claude
andcommitted
feat(issue-lifecycle): add pr_open phase and link_pr method
Backport of systeminit/swamp#1143. Restores the PR-linkage capability dropped in #57 (drop GitHub integration) without re-introducing any git-host coupling. The refactor correctly removed the `gh` CLI dependency, but it also removed the concept of PR linkage — an audit-trail concept that isn't coupled to GitHub. This brings back the minimal piece. ## New domain concepts - `pr_open` phase — the PR is linked and we're awaiting CI/review/merge. No new swamp-club status; maps to `in_progress` like `implementing`. - `pullRequest-main` resource — single-instance value object holding the opaque PR URL and a `linkedAt` timestamp. Git-host agnostic. - `link_pr` method — idempotent (valid from both `implementing` and `pr_open`); each call overwrites the resource so re-pushes, URL corrections, or replacement PRs don't need a new phase. ## Backwards compatibility `complete` now accepts both `implementing` and `pr_open` as source phases. Existing records that didn't go through `link_pr` can still reach `done` via the legacy path. New work should flow `implementing → link_pr → pr_open → complete → done`. ## Version bump manifest.yaml: `2026.04.08.1 → 2026.04.08.2`, with the state machine diagram and methods list updated to reflect the new phase and method. Model upgrade entry added; no global-args migration needed. ## Skill doc hardening While updating `implementation.md` for the new `link_pr` step, also scoped all hardcoded tmp filename examples in `planning.md` and `adversarial-review.md`: - `/tmp/plan.yaml` → `/tmp/plan-issue-<N>.yaml` - `/tmp/findings.yaml` → `/tmp/findings-issue-<N>.yaml` - `/tmp/resolutions.yaml` → `/tmp/resolutions-issue-<N>.yaml` Generic filenames collide with stale content from previous lifecycle sessions and can silently leak unrelated data into the current run. ## Test coverage Existing `schemas_test.ts` updated to reflect the new state machine: - Happy-path walk now routes `implementing → link_pr → pr_open → complete`. - New `legacy path` test ensures `complete` still accepts `implementing`. - `completion gate` assertion expanded to `["implementing", "pr_open"]`. New tests added: - `Phase: pr_open sits between implementing and done` - `TRANSITIONS: link_pr is idempotent from implementing and pr_open` - `TRANSITIONS: link_pr is rejected from earlier lifecycle phases` - `PullRequestSchema` positive, empty-rejection, required-field tests New `issue_lifecycle_test.ts` with 7 method-level tests covering the `link_pr` happy path, state transition to `pr_open`, idempotency, zod schema rejection, and model registration smoke tests. Full verification gate (in the issue-lifecycle package): - `deno task check` — clean - `deno task lint` — clean - `deno task fmt:check` — clean - `deno task test` — **23 passed, 0 failed** (16 schema + 7 method) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent cb6c1e6 commit e21869f

8 files changed

Lines changed: 452 additions & 32 deletions

File tree

issue-lifecycle/.claude/skills/issue-lifecycle/references/adversarial-review.md

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,13 @@ For each plan step, **read the actual code**:
4646

4747
## Step 3: Record Findings
4848

49-
Write findings to a YAML file (e.g. `/tmp/findings.yaml`) as a YAML object with
50-
a `findings` key:
49+
Write findings to a YAML file as a YAML object with a `findings` key. **Use an
50+
issue-scoped filename** like `/tmp/findings-issue-<N>.yaml` — a generic
51+
`/tmp/findings.yaml` collides with stale content from previous lifecycle
52+
sessions and can leak unrelated findings into the current review.
5153

5254
```yaml
53-
# /tmp/findings.yaml
55+
# /tmp/findings-issue-<N>.yaml
5456
findings:
5557
- id: ADV-1
5658
severity: high
@@ -66,7 +68,7 @@ Then record them:
6668
6769
```
6870
swamp model method run issue-<N> adversarial_review \
69-
--input-file /tmp/findings.yaml
71+
--input-file /tmp/findings-issue-<N>.yaml
7072
```
7173

7274
Each finding must have:
@@ -104,22 +106,24 @@ If no blocking findings, say:
104106
When the human gives feedback OR adversarial findings need addressing:
105107

106108
1. **Call iterate** with both the feedback text and your revised plan. Write the
107-
revised `steps` and `potentialChallenges` to a YAML file (same format as the
108-
`plan` step), then run:
109+
revised `steps` and `potentialChallenges` to an issue-scoped YAML file (e.g.
110+
`/tmp/plan-issue-<N>-v2.yaml` — the `-v2` suffix keeps each revision distinct
111+
if you want to diff between iterations), then run:
109112

110113
```
111114
swamp model method run issue-<N> iterate \
112115
--input feedback="<human's feedback or adversarial findings>" \
113116
--input summary="..." \
114117
--input dddAnalysis="..." \
115118
--input testingStrategy="..." \
116-
--input-file /tmp/plan.yaml
119+
--input-file /tmp/plan-issue-<N>-v2.yaml
117120
```
118121

119-
2. **Resolve addressed findings**. Write resolutions to a YAML file:
122+
2. **Resolve addressed findings**. Write resolutions to an issue-scoped YAML
123+
file:
120124

121125
```yaml
122-
# /tmp/resolutions.yaml
126+
# /tmp/resolutions-issue-<N>.yaml
123127
resolutions:
124128
- findingId: ADV-1
125129
resolutionNote: "Added domain boundary definition in step 2"
@@ -129,7 +133,7 @@ When the human gives feedback OR adversarial findings need addressing:
129133
130134
```
131135
swamp model method run issue-<N> resolve_findings \
132-
--input-file /tmp/resolutions.yaml
136+
--input-file /tmp/resolutions-issue-<N>.yaml
133137
```
134138

135139
3. **Re-run adversarial review** on the new plan version. The review must be

issue-lifecycle/.claude/skills/issue-lifecycle/references/implementation.md

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,33 @@ Use the repository's normal PR tooling. Read
4141
`agent-constraints/implementation-conventions.md` for repo-specific PR
4242
conventions.
4343

44-
The issue-lifecycle model does not track PRs directly — your PR creation is
45-
outside the model's scope. The swamp-club issue status stays at `in_progress`
46-
until you call `complete`.
44+
## 4a. Link the PR
45+
46+
After the PR is open, record its URL on the lifecycle so the swamp-club record
47+
points to where the fix lives:
48+
49+
```
50+
swamp model method run issue-<N> link_pr --input url=<PR URL>
51+
```
52+
53+
This writes a `pullRequest-main` resource, transitions the phase to `pr_open`,
54+
and posts a `pr_linked` lifecycle entry on the swamp-club issue. The swamp-club
55+
status stays at `in_progress` — there is no new status for `pr_open`; the PR
56+
link is additional evidence attached to the in-progress state.
57+
58+
`link_pr` is **idempotent** — call it again with a new URL if:
59+
60+
- CI fails and you force-push a different PR
61+
- The first PR is closed and a replacement is opened
62+
- You recorded the wrong URL the first time
63+
64+
Each call overwrites `pullRequest-main` with the latest URL and refreshes
65+
`linkedAt`. A new `pr_linked` entry is posted on every call.
66+
67+
Calling `link_pr` is **encouraged but not enforced**`complete` still accepts
68+
`implementing` as a valid source phase for backwards compatibility with legacy
69+
records. Prefer the `implementing → link_pr → complete` flow for all new work so
70+
the lifecycle record carries the PR link.
4771

4872
## 5. Complete the Issue
4973

issue-lifecycle/.claude/skills/issue-lifecycle/references/planning.md

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,19 @@ ready to generate an implementation plan.
55

66
## 6. Generate an Implementation Plan
77

8-
Write a single YAML file (e.g. `/tmp/plan.yaml`) containing both `steps` and
9-
`potentialChallenges` as top-level keys. The CLI only supports one
10-
`--input-file` flag per invocation, and the file must be a YAML object (not a
11-
bare array).
8+
Write a single YAML file containing both `steps` and `potentialChallenges` as
9+
top-level keys. The CLI only supports one `--input-file` flag per invocation,
10+
and the file must be a YAML object (not a bare array).
11+
12+
**Use an issue-scoped filename** like `/tmp/plan-issue-<N>.yaml` rather than a
13+
generic `/tmp/plan.yaml`. Generic filenames collide with stale content left
14+
behind by previous lifecycle sessions — the old content usually fails to `Read`
15+
for you to overwrite, and worse, content from an unrelated issue can silently
16+
leak into the current plan if you forget to check. Issue-scoped filenames make
17+
resuming sessions safe and auditable.
1218

1319
```yaml
14-
# /tmp/plan.yaml
20+
# /tmp/plan-issue-<N>.yaml
1521
steps:
1622
- order: 1
1723
description: "Add the new schema types"
@@ -36,7 +42,7 @@ swamp model method run issue-<N> plan \
3642
--input summary="..." \
3743
--input dddAnalysis="..." \
3844
--input testingStrategy="..." \
39-
--input-file /tmp/plan.yaml
45+
--input-file /tmp/plan-issue-<N>.yaml
4046
```
4147

4248
## 7. Apply Repo-Specific Planning Conventions

issue-lifecycle/extensions/models/_lib/schemas.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const Phase = z.enum([
4343
"plan_generated",
4444
"approved",
4545
"implementing",
46+
"pr_open",
4647
"done",
4748
]);
4849

@@ -57,6 +58,7 @@ export const TRANSITIONS: Record<string, Phase[]> = {
5758
"plan_generated",
5859
"approved",
5960
"implementing",
61+
"pr_open",
6062
],
6163
triage: ["triaging"],
6264
plan: ["classified"],
@@ -65,7 +67,8 @@ export const TRANSITIONS: Record<string, Phase[]> = {
6567
implement: ["approved"],
6668
adversarial_review: ["plan_generated"],
6769
resolve_findings: ["plan_generated"],
68-
complete: ["implementing"],
70+
link_pr: ["implementing", "pr_open"],
71+
complete: ["implementing", "pr_open"],
6972
};
7073

7174
// ---------------------------------------------------------------------------
@@ -161,3 +164,16 @@ export const AdversarialReviewSchema = z.object({
161164
});
162165

163166
export type AdversarialReviewData = z.infer<typeof AdversarialReviewSchema>;
167+
168+
export const PullRequestSchema = z.object({
169+
url: z.string().min(1).describe(
170+
"Canonical URL of the pull request. Opaque to the model — the agent " +
171+
"supplies whatever URL their git host produced.",
172+
),
173+
linkedAt: z.string().describe(
174+
"ISO-8601 timestamp of when link_pr was called. Updated on every " +
175+
"subsequent link_pr call so the record reflects the latest link.",
176+
),
177+
});
178+
179+
export type PullRequestData = z.infer<typeof PullRequestSchema>;

issue-lifecycle/extensions/models/_lib/schemas_test.ts

Lines changed: 95 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// with Swamp. If not, see <https://www.gnu.org/licenses/>.
1616

1717
import { assert, assertEquals } from "@std/assert";
18-
import { Phase, TRANSITIONS } from "./schemas.ts";
18+
import { Phase, PullRequestSchema, TRANSITIONS } from "./schemas.ts";
1919

2020
const validPhases = new Set<string>(Phase.options);
2121

@@ -64,7 +64,7 @@ Deno.test("start is allowed from every non-terminal phase (restart invariant)",
6464
}
6565
});
6666

67-
Deno.test("happy path: created → triaging → classified → plan_generated → approved → implementing → done", () => {
67+
Deno.test("happy path: created → triaging → classified → plan_generated → approved → implementing → pr_open → done", () => {
6868
// Walk the linear happy path one method at a time and verify each method's
6969
// required source phase is in its TRANSITIONS entry. This catches the case
7070
// where someone reorders the state machine and forgets to update an edge.
@@ -74,7 +74,8 @@ Deno.test("happy path: created → triaging → classified → plan_generated
7474
{ method: "plan", from: "classified" },
7575
{ method: "approve", from: "plan_generated" },
7676
{ method: "implement", from: "approved" },
77-
{ method: "complete", from: "implementing" },
77+
{ method: "link_pr", from: "implementing" },
78+
{ method: "complete", from: "pr_open" },
7879
];
7980
for (const { method, from } of happyPath) {
8081
const allowed = TRANSITIONS[method];
@@ -88,6 +89,17 @@ Deno.test("happy path: created → triaging → classified → plan_generated
8889
}
8990
});
9091

92+
Deno.test("legacy path: complete still accepts implementing for records that never linked a PR", () => {
93+
// Records created before link_pr existed (and records where the human
94+
// skips link_pr for docs-only or trivial changes) must still be able to
95+
// reach done directly from implementing. Removing the implementing entry
96+
// from complete's allowed list would orphan those records.
97+
assert(
98+
TRANSITIONS.complete.includes("implementing"),
99+
"complete must accept implementing as a source phase for backwards compatibility",
100+
);
101+
});
102+
91103
Deno.test("plan iteration loop: iterate is allowed from plan_generated only", () => {
92104
// The iterate loop is the heart of the plan-revision feature. If iterate
93105
// accepted any other phase, a user could revise an already-approved plan,
@@ -103,8 +115,84 @@ Deno.test("implementation gate: implement only allowed from approved", () => {
103115
assertEquals(TRANSITIONS.implement, ["approved"]);
104116
});
105117

106-
Deno.test("completion gate: complete only allowed from implementing", () => {
107-
// complete is the single exit path out of implementing — there is no
108-
// ci_review phase or fix loop in the swamp-club workflow.
109-
assertEquals(TRANSITIONS.complete, ["implementing"]);
118+
Deno.test("completion gate: complete allowed from implementing and pr_open", () => {
119+
// complete is the exit path from the implementation span. It accepts
120+
// both 'implementing' (legacy/no-PR path) and 'pr_open' (the new
121+
// link_pr → pr_open → complete path). No ci_review phase or fix loop.
122+
assertEquals(TRANSITIONS.complete, ["implementing", "pr_open"]);
123+
});
124+
125+
// ---------------------------------------------------------------------------
126+
// PR linkage additions (v2026.04.08.2)
127+
// ---------------------------------------------------------------------------
128+
129+
Deno.test("Phase: pr_open sits between implementing and done", () => {
130+
const phases = Phase.options;
131+
const implementingIdx = phases.indexOf("implementing");
132+
const prOpenIdx = phases.indexOf("pr_open");
133+
const doneIdx = phases.indexOf("done");
134+
135+
assertEquals(prOpenIdx, implementingIdx + 1);
136+
assertEquals(doneIdx, prOpenIdx + 1);
137+
});
138+
139+
Deno.test("TRANSITIONS: link_pr is idempotent from implementing and pr_open", () => {
140+
// Accepting both source phases is what makes link_pr re-callable: the
141+
// first call moves implementing → pr_open, subsequent calls overwrite
142+
// the pullRequest resource while staying in pr_open. This supports URL
143+
// corrections, replacement PRs, and force-push workflows without a
144+
// separate phase.
145+
assertEquals(TRANSITIONS.link_pr, ["implementing", "pr_open"]);
146+
});
147+
148+
Deno.test("TRANSITIONS: link_pr is rejected from earlier lifecycle phases", () => {
149+
// link_pr must not be callable before implementation has begun — calling
150+
// it from any earlier phase is a sequencing bug in the agent and must be
151+
// blocked by the valid-transition pre-flight check.
152+
const earlierPhases: ReadonlyArray<typeof Phase.options[number]> = [
153+
"created",
154+
"triaging",
155+
"classified",
156+
"plan_generated",
157+
"approved",
158+
];
159+
for (const phase of earlierPhases) {
160+
assertEquals(
161+
TRANSITIONS.link_pr.includes(phase),
162+
false,
163+
`link_pr must not be allowed from phase '${phase}'`,
164+
);
165+
}
166+
});
167+
168+
Deno.test("PullRequestSchema: accepts any non-empty URL string", () => {
169+
// URLs are opaque to the model — GitHub, GitLab, Gitea, Forgejo, etc.
170+
const samples = [
171+
"https://github.com/systeminit/swamp/pull/1141",
172+
"https://gitlab.com/group/project/-/merge_requests/42",
173+
"https://codeberg.org/user/repo/pulls/7",
174+
"https://git.internal/project/+/123",
175+
];
176+
for (const url of samples) {
177+
const parsed = PullRequestSchema.parse({
178+
url,
179+
linkedAt: "2026-04-08T15:00:00.000Z",
180+
});
181+
assertEquals(parsed.url, url);
182+
}
183+
});
184+
185+
Deno.test("PullRequestSchema: rejects empty url string", () => {
186+
const result = PullRequestSchema.safeParse({
187+
url: "",
188+
linkedAt: "2026-04-08T15:00:00.000Z",
189+
});
190+
assertEquals(result.success, false);
191+
});
192+
193+
Deno.test("PullRequestSchema: requires linkedAt", () => {
194+
const result = PullRequestSchema.safeParse({
195+
url: "https://github.com/systeminit/swamp/pull/1",
196+
});
197+
assertEquals(result.success, false);
110198
});

0 commit comments

Comments
 (0)