diff --git a/issue-lifecycle/.claude/skills/issue-lifecycle/references/adversarial-review.md b/issue-lifecycle/.claude/skills/issue-lifecycle/references/adversarial-review.md index ab8866f0..7a1f2923 100644 --- a/issue-lifecycle/.claude/skills/issue-lifecycle/references/adversarial-review.md +++ b/issue-lifecycle/.claude/skills/issue-lifecycle/references/adversarial-review.md @@ -46,11 +46,13 @@ For each plan step, **read the actual code**: ## Step 3: Record Findings -Write findings to a YAML file (e.g. `/tmp/findings.yaml`) as a YAML object with -a `findings` key: +Write findings to a YAML file as a YAML object with a `findings` key. **Use an +issue-scoped filename** like `/tmp/findings-issue-.yaml` — a generic +`/tmp/findings.yaml` collides with stale content from previous lifecycle +sessions and can leak unrelated findings into the current review. ```yaml -# /tmp/findings.yaml +# /tmp/findings-issue-.yaml findings: - id: ADV-1 severity: high @@ -66,7 +68,7 @@ Then record them: ``` swamp model method run issue- adversarial_review \ - --input-file /tmp/findings.yaml + --input-file /tmp/findings-issue-.yaml ``` Each finding must have: @@ -104,8 +106,9 @@ If no blocking findings, say: When the human gives feedback OR adversarial findings need addressing: 1. **Call iterate** with both the feedback text and your revised plan. Write the - revised `steps` and `potentialChallenges` to a YAML file (same format as the - `plan` step), then run: + revised `steps` and `potentialChallenges` to an issue-scoped YAML file (e.g. + `/tmp/plan-issue--v2.yaml` — the `-v2` suffix keeps each revision distinct + if you want to diff between iterations), then run: ``` swamp model method run issue- iterate \ @@ -113,13 +116,14 @@ When the human gives feedback OR adversarial findings need addressing: --input summary="..." \ --input dddAnalysis="..." \ --input testingStrategy="..." \ - --input-file /tmp/plan.yaml + --input-file /tmp/plan-issue--v2.yaml ``` -2. **Resolve addressed findings**. Write resolutions to a YAML file: +2. **Resolve addressed findings**. Write resolutions to an issue-scoped YAML + file: ```yaml - # /tmp/resolutions.yaml + # /tmp/resolutions-issue-.yaml resolutions: - findingId: ADV-1 resolutionNote: "Added domain boundary definition in step 2" @@ -129,7 +133,7 @@ When the human gives feedback OR adversarial findings need addressing: ``` swamp model method run issue- resolve_findings \ - --input-file /tmp/resolutions.yaml + --input-file /tmp/resolutions-issue-.yaml ``` 3. **Re-run adversarial review** on the new plan version. The review must be diff --git a/issue-lifecycle/.claude/skills/issue-lifecycle/references/implementation.md b/issue-lifecycle/.claude/skills/issue-lifecycle/references/implementation.md index 0f9a5a5c..4812e649 100644 --- a/issue-lifecycle/.claude/skills/issue-lifecycle/references/implementation.md +++ b/issue-lifecycle/.claude/skills/issue-lifecycle/references/implementation.md @@ -41,9 +41,33 @@ Use the repository's normal PR tooling. Read `agent-constraints/implementation-conventions.md` for repo-specific PR conventions. -The issue-lifecycle model does not track PRs directly — your PR creation is -outside the model's scope. The swamp-club issue status stays at `in_progress` -until you call `complete`. +## 4a. Link the PR + +After the PR is open, record its URL on the lifecycle so the swamp-club record +points to where the fix lives: + +``` +swamp model method run issue- link_pr --input url= +``` + +This writes a `pullRequest-main` resource, transitions the phase to `pr_open`, +and posts a `pr_linked` lifecycle entry on the swamp-club issue. The swamp-club +status stays at `in_progress` — there is no new status for `pr_open`; the PR +link is additional evidence attached to the in-progress state. + +`link_pr` is **idempotent** — call it again with a new URL if: + +- CI fails and you force-push a different PR +- The first PR is closed and a replacement is opened +- You recorded the wrong URL the first time + +Each call overwrites `pullRequest-main` with the latest URL and refreshes +`linkedAt`. A new `pr_linked` entry is posted on every call. + +Calling `link_pr` is **encouraged but not enforced** — `complete` still accepts +`implementing` as a valid source phase for backwards compatibility with legacy +records. Prefer the `implementing → link_pr → complete` flow for all new work so +the lifecycle record carries the PR link. ## 5. Complete the Issue diff --git a/issue-lifecycle/.claude/skills/issue-lifecycle/references/planning.md b/issue-lifecycle/.claude/skills/issue-lifecycle/references/planning.md index b0ce975c..e49da72d 100644 --- a/issue-lifecycle/.claude/skills/issue-lifecycle/references/planning.md +++ b/issue-lifecycle/.claude/skills/issue-lifecycle/references/planning.md @@ -5,13 +5,19 @@ ready to generate an implementation plan. ## 6. Generate an Implementation Plan -Write a single YAML file (e.g. `/tmp/plan.yaml`) containing both `steps` and -`potentialChallenges` as top-level keys. The CLI only supports one -`--input-file` flag per invocation, and the file must be a YAML object (not a -bare array). +Write a single YAML file containing both `steps` and `potentialChallenges` as +top-level keys. The CLI only supports one `--input-file` flag per invocation, +and the file must be a YAML object (not a bare array). + +**Use an issue-scoped filename** like `/tmp/plan-issue-.yaml` rather than a +generic `/tmp/plan.yaml`. Generic filenames collide with stale content left +behind by previous lifecycle sessions — the old content usually fails to `Read` +for you to overwrite, and worse, content from an unrelated issue can silently +leak into the current plan if you forget to check. Issue-scoped filenames make +resuming sessions safe and auditable. ```yaml -# /tmp/plan.yaml +# /tmp/plan-issue-.yaml steps: - order: 1 description: "Add the new schema types" @@ -36,7 +42,7 @@ swamp model method run issue- plan \ --input summary="..." \ --input dddAnalysis="..." \ --input testingStrategy="..." \ - --input-file /tmp/plan.yaml + --input-file /tmp/plan-issue-.yaml ``` ## 7. Apply Repo-Specific Planning Conventions diff --git a/issue-lifecycle/extensions/models/_lib/schemas.ts b/issue-lifecycle/extensions/models/_lib/schemas.ts index 376bcb51..949d764e 100644 --- a/issue-lifecycle/extensions/models/_lib/schemas.ts +++ b/issue-lifecycle/extensions/models/_lib/schemas.ts @@ -43,6 +43,7 @@ export const Phase = z.enum([ "plan_generated", "approved", "implementing", + "pr_open", "done", ]); @@ -57,6 +58,7 @@ export const TRANSITIONS: Record = { "plan_generated", "approved", "implementing", + "pr_open", ], triage: ["triaging"], plan: ["classified"], @@ -65,7 +67,8 @@ export const TRANSITIONS: Record = { implement: ["approved"], adversarial_review: ["plan_generated"], resolve_findings: ["plan_generated"], - complete: ["implementing"], + link_pr: ["implementing", "pr_open"], + complete: ["implementing", "pr_open"], }; // --------------------------------------------------------------------------- @@ -161,3 +164,16 @@ export const AdversarialReviewSchema = z.object({ }); export type AdversarialReviewData = z.infer; + +export const PullRequestSchema = z.object({ + url: z.string().min(1).describe( + "Canonical URL of the pull request. Opaque to the model — the agent " + + "supplies whatever URL their git host produced.", + ), + linkedAt: z.string().describe( + "ISO-8601 timestamp of when link_pr was called. Updated on every " + + "subsequent link_pr call so the record reflects the latest link.", + ), +}); + +export type PullRequestData = z.infer; diff --git a/issue-lifecycle/extensions/models/_lib/schemas_test.ts b/issue-lifecycle/extensions/models/_lib/schemas_test.ts index 07687abe..2baf7757 100644 --- a/issue-lifecycle/extensions/models/_lib/schemas_test.ts +++ b/issue-lifecycle/extensions/models/_lib/schemas_test.ts @@ -15,7 +15,7 @@ // with Swamp. If not, see . import { assert, assertEquals } from "@std/assert"; -import { Phase, TRANSITIONS } from "./schemas.ts"; +import { Phase, PullRequestSchema, TRANSITIONS } from "./schemas.ts"; const validPhases = new Set(Phase.options); @@ -64,7 +64,7 @@ Deno.test("start is allowed from every non-terminal phase (restart invariant)", } }); -Deno.test("happy path: created → triaging → classified → plan_generated → approved → implementing → done", () => { +Deno.test("happy path: created → triaging → classified → plan_generated → approved → implementing → pr_open → done", () => { // Walk the linear happy path one method at a time and verify each method's // required source phase is in its TRANSITIONS entry. This catches the case // 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 { method: "plan", from: "classified" }, { method: "approve", from: "plan_generated" }, { method: "implement", from: "approved" }, - { method: "complete", from: "implementing" }, + { method: "link_pr", from: "implementing" }, + { method: "complete", from: "pr_open" }, ]; for (const { method, from } of happyPath) { const allowed = TRANSITIONS[method]; @@ -88,6 +89,17 @@ Deno.test("happy path: created → triaging → classified → plan_generated } }); +Deno.test("legacy path: complete still accepts implementing for records that never linked a PR", () => { + // Records created before link_pr existed (and records where the human + // skips link_pr for docs-only or trivial changes) must still be able to + // reach done directly from implementing. Removing the implementing entry + // from complete's allowed list would orphan those records. + assert( + TRANSITIONS.complete.includes("implementing"), + "complete must accept implementing as a source phase for backwards compatibility", + ); +}); + Deno.test("plan iteration loop: iterate is allowed from plan_generated only", () => { // The iterate loop is the heart of the plan-revision feature. If iterate // 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", () => { assertEquals(TRANSITIONS.implement, ["approved"]); }); -Deno.test("completion gate: complete only allowed from implementing", () => { - // complete is the single exit path out of implementing — there is no - // ci_review phase or fix loop in the swamp-club workflow. - assertEquals(TRANSITIONS.complete, ["implementing"]); +Deno.test("completion gate: complete allowed from implementing and pr_open", () => { + // complete is the exit path from the implementation span. It accepts + // both 'implementing' (legacy/no-PR path) and 'pr_open' (the new + // link_pr → pr_open → complete path). No ci_review phase or fix loop. + assertEquals(TRANSITIONS.complete, ["implementing", "pr_open"]); +}); + +// --------------------------------------------------------------------------- +// PR linkage additions (v2026.04.08.2) +// --------------------------------------------------------------------------- + +Deno.test("Phase: pr_open sits between implementing and done", () => { + const phases = Phase.options; + const implementingIdx = phases.indexOf("implementing"); + const prOpenIdx = phases.indexOf("pr_open"); + const doneIdx = phases.indexOf("done"); + + assertEquals(prOpenIdx, implementingIdx + 1); + assertEquals(doneIdx, prOpenIdx + 1); +}); + +Deno.test("TRANSITIONS: link_pr is idempotent from implementing and pr_open", () => { + // Accepting both source phases is what makes link_pr re-callable: the + // first call moves implementing → pr_open, subsequent calls overwrite + // the pullRequest resource while staying in pr_open. This supports URL + // corrections, replacement PRs, and force-push workflows without a + // separate phase. + assertEquals(TRANSITIONS.link_pr, ["implementing", "pr_open"]); +}); + +Deno.test("TRANSITIONS: link_pr is rejected from earlier lifecycle phases", () => { + // link_pr must not be callable before implementation has begun — calling + // it from any earlier phase is a sequencing bug in the agent and must be + // blocked by the valid-transition pre-flight check. + const earlierPhases: ReadonlyArray = [ + "created", + "triaging", + "classified", + "plan_generated", + "approved", + ]; + for (const phase of earlierPhases) { + assertEquals( + TRANSITIONS.link_pr.includes(phase), + false, + `link_pr must not be allowed from phase '${phase}'`, + ); + } +}); + +Deno.test("PullRequestSchema: accepts any non-empty URL string", () => { + // URLs are opaque to the model — GitHub, GitLab, Gitea, Forgejo, etc. + const samples = [ + "https://github.com/systeminit/swamp/pull/1141", + "https://gitlab.com/group/project/-/merge_requests/42", + "https://codeberg.org/user/repo/pulls/7", + "https://git.internal/project/+/123", + ]; + for (const url of samples) { + const parsed = PullRequestSchema.parse({ + url, + linkedAt: "2026-04-08T15:00:00.000Z", + }); + assertEquals(parsed.url, url); + } +}); + +Deno.test("PullRequestSchema: rejects empty url string", () => { + const result = PullRequestSchema.safeParse({ + url: "", + linkedAt: "2026-04-08T15:00:00.000Z", + }); + assertEquals(result.success, false); +}); + +Deno.test("PullRequestSchema: requires linkedAt", () => { + const result = PullRequestSchema.safeParse({ + url: "https://github.com/systeminit/swamp/pull/1", + }); + assertEquals(result.success, false); }); diff --git a/issue-lifecycle/extensions/models/issue_lifecycle.ts b/issue-lifecycle/extensions/models/issue_lifecycle.ts index 181c35e6..57911122 100644 --- a/issue-lifecycle/extensions/models/issue_lifecycle.ts +++ b/issue-lifecycle/extensions/models/issue_lifecycle.ts @@ -27,6 +27,7 @@ import { type PlanData, PlanSchema, PlanStepSchema, + PullRequestSchema, type StateData, StateSchema, TRANSITIONS, @@ -67,7 +68,7 @@ async function readState( export const model = { type: "@swamp/issue-lifecycle", - version: "2026.04.08.1", + version: "2026.04.08.2", globalArguments: GlobalArgsSchema, upgrades: [ @@ -95,6 +96,15 @@ export const model = { return next; }, }, + { + toVersion: "2026.04.08.2", + description: + "Add pr_open phase, pullRequest resource, and link_pr method. " + + "Restores PR linkage dropped in 2026.04.08.1 without re-introducing git-host coupling — " + + "the model persists whatever PR URL the agent supplies. " + + "complete now accepts pr_open as a valid source phase alongside implementing.", + upgradeAttributes: (old: Record) => old, + }, ], resources: { @@ -134,6 +144,15 @@ export const model = { lifetime: "infinite" as const, garbageCollection: 20, }, + "pullRequest": { + description: + "Pull request linked to the implementation. Single instance, " + + "overwritten by subsequent link_pr calls so the record always " + + "reflects the latest link.", + schema: PullRequestSchema, + lifetime: "infinite" as const, + garbageCollection: 5, + }, }, checks: { @@ -1083,6 +1102,69 @@ export const model = { }, }, + link_pr: { + description: + "Link a pull request to the implementation. Idempotent — calling " + + "again overwrites the recorded URL with the latest link. " + + "Transitions the phase to pr_open if currently implementing.", + arguments: z.object({ + url: z.string().min(1).describe( + "Canonical pull request URL. Opaque to the model — pass whatever " + + "URL your git host produced.", + ), + }), + execute: async ( + args: { url: string }, + context: { + globalArgs: GlobalArgs; + logger: { + info: (msg: string, props: Record) => void; + warning: (msg: string, props: Record) => void; + }; + writeResource: ( + specName: string, + instanceName: string, + data: Record, + ) => Promise<{ name: string }>; + }, + ) => { + const { issueNumber } = context.globalArgs; + const now = new Date().toISOString(); + + const prHandle = await context.writeResource( + "pullRequest", + "pullRequest-main", + { + url: args.url, + linkedAt: now, + }, + ); + + const stateHandle = await context.writeResource("state", "state-main", { + phase: "pr_open", + issueNumber, + updatedAt: now, + }); + + context.logger.info("PR linked: {url}", { url: args.url }); + + const sc = await createSwampClubClient( + context.globalArgs, + context.logger, + ); + await sc?.postLifecycleEntry({ + step: "pr_linked", + targetStatus: "in_progress", + summary: `PR linked: ${args.url}`, + emoji: "\u{1F517}", + payload: { url: args.url }, + isVerbose: false, + }); + + return { dataHandles: [prHandle, stateHandle] }; + }, + }, + complete: { description: "Mark the issue lifecycle as done", arguments: z.object({}), diff --git a/issue-lifecycle/extensions/models/issue_lifecycle_test.ts b/issue-lifecycle/extensions/models/issue_lifecycle_test.ts new file mode 100644 index 00000000..631c35e8 --- /dev/null +++ b/issue-lifecycle/extensions/models/issue_lifecycle_test.ts @@ -0,0 +1,192 @@ +// Swamp, an Automation Framework Copyright (C) 2026 System Initiative, Inc. +// +// This file is part of Swamp. +// +// Swamp is free software: you can redistribute it and/or modify it under the terms +// of the GNU Affero General Public License version 3 as published by the Free +// Software Foundation, with the Swamp Extension and Definition Exception (found in +// the "COPYING-EXCEPTION" file). +// +// Swamp is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A +// PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License along +// with Swamp. If not, see . + +import { assertEquals, assertRejects } from "@std/assert"; +import { model } from "./issue_lifecycle.ts"; + +// --------------------------------------------------------------------------- +// Test helpers +// --------------------------------------------------------------------------- + +interface RecordedWrite { + specName: string; + instanceName: string; + data: Record; +} + +/** + * Build a fake method execution context that records writes and short-circuits + * the swamp-club client by pointing HOME/XDG_CONFIG_HOME at an empty temp dir + * and clearing credential env vars. + * + * Callers must `restore()` in a finally block to put env vars back. + */ +async function buildTestContext( + issueNumber: number, +): Promise<{ + context: Parameters[1]; + writes: RecordedWrite[]; + restore: () => Promise; +}> { + const writes: RecordedWrite[] = []; + const tempDir = await Deno.makeTempDir({ prefix: "issue_lifecycle_test_" }); + + const original = { + SWAMP_API_KEY: Deno.env.get("SWAMP_API_KEY"), + SWAMP_CLUB_URL: Deno.env.get("SWAMP_CLUB_URL"), + HOME: Deno.env.get("HOME"), + XDG_CONFIG_HOME: Deno.env.get("XDG_CONFIG_HOME"), + }; + + Deno.env.delete("SWAMP_API_KEY"); + Deno.env.delete("SWAMP_CLUB_URL"); + Deno.env.set("HOME", tempDir); + Deno.env.set("XDG_CONFIG_HOME", tempDir); + + const restore = async () => { + for (const [k, v] of Object.entries(original)) { + if (v === undefined) { + Deno.env.delete(k); + } else { + Deno.env.set(k, v); + } + } + await Deno.remove(tempDir, { recursive: true }); + }; + + const context = { + globalArgs: { issueNumber }, + logger: { + info: () => {}, + warning: () => {}, + }, + writeResource: ( + specName: string, + instanceName: string, + data: Record, + ) => { + writes.push({ specName, instanceName, data }); + return Promise.resolve({ name: instanceName }); + }, + }; + + return { context, writes, restore }; +} + +// --------------------------------------------------------------------------- +// link_pr +// --------------------------------------------------------------------------- + +Deno.test("link_pr: writes pullRequest-main resource with url and linkedAt", async () => { + const { context, writes, restore } = await buildTestContext(42); + try { + await model.methods.link_pr.execute( + { url: "https://github.com/systeminit/swamp/pull/1141" }, + context, + ); + + const prWrite = writes.find((w) => w.specName === "pullRequest"); + assertEquals(prWrite !== undefined, true); + assertEquals( + prWrite!.instanceName, + "pullRequest-main", + "pullRequest resource must use the single-instance '-main' naming", + ); + assertEquals( + prWrite!.data.url, + "https://github.com/systeminit/swamp/pull/1141", + ); + assertEquals(typeof prWrite!.data.linkedAt, "string"); + } finally { + await restore(); + } +}); + +Deno.test("link_pr: transitions state to pr_open", async () => { + const { context, writes, restore } = await buildTestContext(42); + try { + await model.methods.link_pr.execute( + { url: "https://github.com/systeminit/swamp/pull/1141" }, + context, + ); + + const stateWrite = writes.find((w) => w.specName === "state"); + assertEquals(stateWrite !== undefined, true); + assertEquals(stateWrite!.instanceName, "state-main"); + assertEquals(stateWrite!.data.phase, "pr_open"); + assertEquals(stateWrite!.data.issueNumber, 42); + } finally { + await restore(); + } +}); + +Deno.test("link_pr: is idempotent — second call overwrites the pullRequest resource", async () => { + const { context, writes, restore } = await buildTestContext(42); + try { + await model.methods.link_pr.execute( + { url: "https://github.com/systeminit/swamp/pull/1141" }, + context, + ); + await model.methods.link_pr.execute( + { url: "https://github.com/systeminit/swamp/pull/1142" }, + context, + ); + + const prWrites = writes.filter((w) => w.specName === "pullRequest"); + assertEquals(prWrites.length, 2); + assertEquals( + prWrites[0].instanceName, + prWrites[1].instanceName, + "both writes target the same instance so the second overwrites the first", + ); + assertEquals( + prWrites[1].data.url, + "https://github.com/systeminit/swamp/pull/1142", + ); + } finally { + await restore(); + } +}); + +Deno.test("link_pr: rejects empty url via zod schema", async () => { + await assertRejects( + () => model.methods.link_pr.arguments.parseAsync({ url: "" }), + ); +}); + +// --------------------------------------------------------------------------- +// Model registration smoke tests +// --------------------------------------------------------------------------- + +Deno.test("model: exposes the new pullRequest resource definition", () => { + assertEquals( + "pullRequest" in model.resources, + true, + "pullRequest resource must be registered in model.resources", + ); +}); + +Deno.test("model: exposes the new link_pr method definition", () => { + assertEquals( + "link_pr" in model.methods, + true, + "link_pr method must be registered in model.methods", + ); +}); + +Deno.test("model: version bumped to 2026.04.08.2", () => { + assertEquals(model.version, "2026.04.08.2"); +}); diff --git a/issue-lifecycle/manifest.yaml b/issue-lifecycle/manifest.yaml index 5bab9ae6..f285ad42 100644 --- a/issue-lifecycle/manifest.yaml +++ b/issue-lifecycle/manifest.yaml @@ -1,6 +1,6 @@ manifestVersion: 1 name: "@swamp/issue-lifecycle" -version: "2026.04.08.1" +version: "2026.04.08.2" description: | Interactive triage and implementation planning for swamp-club lab issues. Drives the triage/plan/iterate/approve/implement loop as a local @@ -20,12 +20,17 @@ description: | plan_generated ──[iterate]──> plan_generated (feedback loop) plan_generated ──[approve]──> approved approved ──[implement]──> implementing - implementing ──[complete]──> done + implementing ──[link_pr]──> pr_open + pr_open ──[link_pr]──> pr_open (idempotent re-link) + pr_open ──[complete]──> done + implementing ──[complete]──> done (legacy, no PR linked) ``` Pre-flight checks enforce valid transitions: you cannot approve without a plan, cannot implement without approval, and cannot approve while critical - or high adversarial findings remain unresolved. + or high adversarial findings remain unresolved. `complete` accepts both + `implementing` and `pr_open` as source phases so legacy records (created + before `link_pr` existed) can still reach `done`. ## Methods @@ -40,12 +45,15 @@ description: | - `resolve_findings` — mark adversarial findings as resolved - `approve` — lock the plan and transition to in_progress - `implement` — signal implementation has started + - `link_pr` — link an opaque pull request URL to the lifecycle record + (idempotent; transitions to `pr_open`). Git-host agnostic — the model + persists whatever URL the agent supplies without fetching anything. - `complete` — mark the lifecycle done ## Data All resources are versioned and immutable: `state`, `context`, - `classification`, `plan`, `feedback`, `adversarialReview`. + `classification`, `plan`, `feedback`, `adversarialReview`, `pullRequest`. ## Prerequisites