refactor: drop GitHub integration from issue-lifecycle, use swamp-club directly

[stack72]

Summary

Mirrors systeminit/swamp#1139. The @swamp/issue-lifecycle extension and skill now operate directly on swamp-club lab issues. Issues must already exist in swamp-club — the model fetches them by sequential lab number via GET /api/v1/lab/issues/{number} and PATCHes the type field during triage so the classification is reflected back in swamp-club.

• Deleted extensions/models/_lib/issue_tracker.ts (387 lines of gh CLI wrapper) and every tracker/GitHub call in issue_lifecycle.ts.
• Global args changed: repo is dropped; issueNumber now refers to a swamp-club lab issue number. Version bumped to 2026.04.08.1 with an upgrade entry that strips repo from old instances.
• SwampClubClient rewritten to operate on the lab number directly. New fetchIssue() (GET) and updateType() (PATCH) methods. The GitHub-coupled /ensure round-trip is gone.
• Classification types reconciled to swamp-club's bug | feature | security. Regressions are modelled as type: bug with a new isRegression: boolean flag on the classification record. unclear is gone — use confidence: low + clarifyingQuestions instead.
• Methods removed: ci_status, record_pr, fix (all GitHub PR/CI coupled). Resources removed: ciResults, fixDirective. Phase removed: ci_review. State machine is now implementing → done directly.
• schemas_test.ts updated to reflect the new state machine: happy path ends implementing→done, new completion gate test, old fix loop and ci_review tests removed.
• Skill docs rewritten (SKILL.md, triage.md, implementation.md) and README.md + manifest.yaml description/method list updated for the swamp-club-first workflow.

Depends on systeminit/swamp-club#374 (merged), which adds PATCH-type support to swamp-club.

New state machine

created → triaging → classified → plan_generated → approved → implementing → done
                                        ↑              ↑
                                        └─ iterate ────┘
                                        (with adversarial_review, resolve_findings)

Diff stats

10 files changed, 402 insertions(+), 1469 deletions(-) — net −1067 lines.

Test plan

• deno task check — clean
• deno task lint — clean
• deno task fmt — clean
• deno task test — 9 passed, 0 failed (state machine invariants)
• Smoke test against a real swamp-club issue once merged and published: create issue #N in the swamp-club UI, run swamp model create @swamp/issue-lifecycle issue-<N> --global-arg issueNumber=<N>, then start → triage and verify the swamp-club issue type flips and the lifecycle entries appear.

🤖 Generated with Claude Code

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

1. Truncated AGPL license header in swamp_club.ts — two lines were accidentally removed from the license footer (diff shows -// You should have received a copy of the GNU Affero General Public License along and -// with Swamp. If not, see <https://www.gnu.org/licenses/>.). The header in the other files ends with those lines; swamp_club.ts now does not.

2. No tests for new SwampClubClient methods — fetchIssue(), updateType(), and the refactored patchIssue() are untested. The old code had the GitHub CLI as an external dependency that was hard to unit-test; the new HTTP-based client is straightforward to exercise with Deno.serve({ port: 0 }). Consider adding a swamp_club_test.ts covering at least the happy-path fetch, the 404/error-returns-null case, and the PATCH flow.

3. Unvalidated type cast in fetchIssue() — (issue.type ?? "feature") as IssueType silently accepts any string the API returns. If swamp-club ever returns a value outside ["bug", "feature", "security"] (e.g. a new type added later), the cast succeeds here but fails downstream at Zod schema validation when the context resource is written, producing a less-readable error. Consider using IssueType.safeParse(issue.type) and falling back to "feature" only on parse failure.

4. isRegression: true is not guarded to type === "bug" — the schema and code both document that isRegression implies type: bug, but there is no runtime assertion. A caller could pass type=feature, isRegression=true without an error. Low-risk given the skill docs are clear, but a one-line if (args.isRegression && args.type !== "bug") throw would make the invariant explicit.

---

The refactor is clean and well-scoped. Deleting the 387-line GitHub CLI wrapper and the CI/fix loop simplifies the model substantially. The state machine change (implementing → done direct) is correctly reflected in the schema, transitions map, and tests. Upgrade migration correctly strips the repo attribute.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. issue_lifecycle.ts:425-435 — isRegression invariant not enforced at the schema or method level.
   The docs and skill references state isRegression: true "implies type=bug". But nothing prevents calling triage with type=security, isRegression=true. The classification record and the swamp-club lifecycle entry would both contain a contradictory {type: "security", isRegression: true}. This is a data integrity issue — downstream consumers trusting the invariant would draw wrong conclusions.
   Breaking example: swamp model method run issue-42 triage --input type=security --input isRegression=true --input confidence=high --input reasoning="..." succeeds and writes contradictory data.
   Suggested fix: Add a Zod .refine() on the triage arguments or an early guard in execute:

   if (args.isRegression && args.type !== "bug") {
     throw new Error("isRegression=true requires type=bug");
   }

2. swamp_club.ts:111 — issue.type cast bypasses runtime validation.
   (issue.type ?? "feature") as IssueType trusts the API response. If swamp-club returns an unexpected type value (e.g., "task", "incident", or a future type added server-side), it flows into the context resource unchecked. Whether this corrupts data depends on whether the framework validates writeResource payloads against the Zod schema at runtime.
   Breaking example: swamp-club adds a "task" issue type; fetchIssue returns {type: "task"}, which is cast to IssueType and written to the context resource. If the framework doesn't validate, the context.type field silently contains an invalid enum value.
   Suggested fix: Use IssueType.catch("feature").parse(issue.type) instead of as IssueType.

Low

1. swamp_club.ts:151-157,188-195 — Response bodies not consumed on success.
   Both postLifecycleEntry and patchIssue read res.text() on non-OK responses for logging, but on success the response body is never consumed or cancelled. In Deno, unconsumed bodies can keep HTTP connections open until GC. With the 15s timeout this is bounded, but under rapid iteration (many lifecycle calls in sequence) it could temporarily exhaust the connection pool.
   Suggested fix: Add await res.body?.cancel(); after the if (!res.ok) block in each method, or structure as if (!res.ok) { ... } else { await res.body?.cancel(); }.

2. issue_lifecycle.ts:108-112 — Upgrade strips repo but leaves issueNumber semantically reinterpreted.
   The upgrade function deletes repo but keeps issueNumber as-is. For existing instances, issueNumber referred to a GitHub issue number (e.g., 850). Post-upgrade, it refers to a swamp-club lab issue number. Running start on an upgraded instance will attempt to fetch swamp-club issue #850, which almost certainly doesn't exist, and throw. This is the correct behavior (fail-fast), but the error message at line 374 doesn't hint that this might be an upgraded instance with a stale issueNumber. Consider adding a note in the upgrade description or the error message.

3. issue_lifecycle.ts:1008-1010 — approve reads plan after writing state.
   The approve method writes the state to "approved" (line 1001) before reading the plan (line 1008). If readResource fails (throws), the state is already transitioned to approved but the lifecycle entry was never posted to swamp-club. The plan-exists check should prevent plan-less approval, so this is defense-in-depth ordering — but swapping the read before the write would be marginally safer.

Verdict

PASS — This is a clean, well-structured refactor that simplifies the model by removing ~1,000 lines of GitHub integration code. The state machine changes are consistent, the upgrade path is correct, tests are updated, and the new swamp-club-first flow is coherent. The medium findings are minor data integrity gaps that don't block merge.

[github-actions]

Adversarial Review

Critical / High

No critical or high severity findings.

Medium

1. issue_lifecycle.ts:925 — resolve_findings silently skips resolutions with empty resolutionNote.
   The guard if (note) is falsy for empty strings. If a caller passes { findingId: "ADV-1", resolutionNote: "" }, the resolutionMap.get(f.id) returns "", the if (note) branch is skipped, and the finding is not resolved — but the log on line 943 still reports it as resolved (since it counts args.resolutions.length, not actual matches).

   Breaking input: resolutions: [{ findingId: "ADV-1", resolutionNote: "" }] → finding stays unresolved, log says "Resolved 1 finding(s)", user thinks it worked.

   Suggested fix: Change if (note) to if (note !== undefined) (the Map returns undefined for missing keys, not "") so that empty-string resolutions are still applied.

2. issue_lifecycle.ts:943 — resolve_findings reports phantom resolutions for unknown finding IDs.
   const resolved = args.resolutions.length counts all inputs, including those whose findingId doesn't match any existing finding. A typo in a finding ID silently drops the resolution.

   Breaking input: resolutions: [{ findingId: "ADV-TYPO", resolutionNote: "fixed" }] → nothing resolved, log says "Resolved 1 finding(s)".

   Suggested fix: Count the findings that actually flipped resolved to true, or throw/warn when a findingId has no match.

Low

3. swamp_club.ts:92-106 — fetchIssue performs no schema validation on the API response.
   The response is cast via as { issue?: ... } after res.json(). If the swamp-club API returns an unexpected shape (e.g., { data: { issue: ... } } or a flat object), the code silently returns null — which is handled, but indistinguishable from a 404, making debugging harder. Not a correctness bug since the null path is handled, but a diagnostic gap.

4. issue_lifecycle.ts:61 — readState casts deserialized JSON without Zod validation.
   JSON.parse(...) as StateData trusts the stored data shape. If a prior version wrote a different schema (e.g., before the upgrade removed repo), the phase field could be missing or unexpected, causing the valid-transition check to silently pass or fail in confusing ways. The upgrade mechanism should prevent this in practice, but the cast provides no safety net.

5. swamp_club.ts:110 — type defaults to "feature" when API returns no type field.
   (issue.type ?? "feature") as IssueType means an issue with type: null or type: undefined from the API silently becomes a "feature". If the intent is that type is always present on swamp-club issues, this default masks a broken API response.

Verdict

PASS — The code is a straightforward simplification (net -1067 lines) with a clean state machine, good transition guards, and consistent best-effort patterns for the swamp-club API. The empty-string resolution note bug (Medium #1) is the only finding that produces incorrect behavior in a realistic scenario, but it requires a somewhat unusual input. No blockers.

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

1. Unsafe type cast in fetchIssue() — (issue.type ?? "feature") as IssueType silently accepts any string the server returns. Consider using IssueType.safeParse(issue.type) and falling back on a default or throwing, so a server returning an unexpected type doesn't silently corrupt the context resource.

2. SwampClubClient.log only binds warning — the constructor assigns this.log = logger?.warning.bind(logger) ?? (() => {}). This means all client-level log calls go through the warning channel; there's no way to emit info-level messages from within the class. Not a bug, but worth noting if finer-grained log levels are desired later.

---

Overall this is a clean, well-structured refactor. The simplification is significant (−1067 net lines), the state machine tests are solid, no any types in hand-written code, credentials are kept private via #apiKey, and error paths don't expose secrets. The upgrade path correctly strips the deprecated repo arg. Approved.

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

1. Unsafe type cast in fetchIssue() — (issue.type ?? "feature") as IssueType silently accepts any string the server returns. Consider using IssueType.safeParse(issue.type) and falling back on a default or throwing, so a server returning an unexpected type doesn't silently corrupt the context resource.

2. SwampClubClient.log only binds warning — the constructor assigns this.log = logger?.warning.bind(logger) ?? (() => {}). This means all client-level log calls go through the warning channel; there's no way to emit info-level messages from within the class. Not a bug, but worth noting if finer-grained log levels are desired later.

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

1. Unsafe type cast in fetchIssue() - the expression (issue.type ?? 'feature') as IssueType silently accepts any string the server returns. Consider using IssueType.safeParse(issue.type) and falling back on a default or throwing, so a server returning an unexpected type does not silently corrupt the context resource.

2. SwampClubClient.log only binds warning - the constructor assigns this.log = logger?.warning.bind(logger) ?? (() => {}). All client-level log calls go through the warning channel with no way to emit info-level messages from within the class. Not a bug, but worth noting if finer-grained log levels are desired later.

Overall this is a clean, well-structured refactor. The simplification is significant (-1067 net lines), the state machine tests are solid, no any types in hand-written code, credentials are kept private via #apiKey, and error paths do not expose secrets. The upgrade path correctly strips the deprecated repo arg.

[github-actions]

Adversarial Review

Critical / High

No critical or high severity issues found.

Medium

1. swamp_club.ts:114 — Unsafe IssueType cast from API response

   (issue.type ?? "feature") as IssueType casts whatever string the API returns to the IssueType union without validation. If swamp-club returns type: "enhancement" (or any value not in bug | feature | security), it passes through as-is. Downstream, triage would call sc.updateType(args.type) with the user-supplied classification, but the context resource would already contain the invalid type from fetchIssue. Whether this causes a runtime failure depends on whether the framework validates writeResource data against the Zod schema — if it doesn't, the invalid value silently persists.

   Breaking input: swamp-club API returns { "issue": { "number": 42, "type": "enhancement", ... } }.

   Suggested fix: Validate with the Zod enum and fall back:

   type: IssueType.catch("feature").parse(issue.type),

2. issue_lifecycle.ts:943 — resolve_findings misreports resolution count for non-existent finding IDs

   Line 943 reports args.resolutions.length as the "resolved" count, but resolutionMap.get(f.id) only matches findings whose ID exists in current.findings. If a caller passes findingId: "ADV-99" and no such finding exists, the log and lifecycle entry claim it was resolved, but nothing was actually changed. The remaining count (line 944) is correct, but the resolved count is inflated.

   Breaking input: { resolutions: [{ findingId: "ADV-NONEXISTENT", resolutionNote: "fixed" }] } — logs "Resolved 1 finding(s)" but zero findings are actually resolved.

   Suggested fix: Count actual matches:

   const resolved = updatedFindings.filter(
     (f) => f.resolved && resolutionMap.has(f.id),
   ).length;

3. issue_lifecycle.ts:1002-1010 — approve writes state before reading plan

   The approve method writes phase: "approved" (line 1003) before reading the plan (line 1010). If readResource throws (framework error, corrupt data), the state is already transitioned to approved with no rollback. The plan-exists pre-flight check mitigates this for the null case, but a JSON parse failure in the framework would leave the model in approved phase without a valid plan reference.

   Suggested fix: Read the plan first, then write the state transition.

Low

1. swamp_club.ts:65 — Logger uses warning level for all swamp-club operations

   this.log = logger?.warning.bind(logger) ?? (() => {}); binds all logging to the warning level. Successful lifecycle posts produce no log output, but all error/failure messages are warnings. This is fine functionally, but means this.log on line 65 is slightly misleading — it's an error logger, not a general logger.

2. issue_lifecycle.ts:61 — readState parses JSON with as cast, no Zod validation

   JSON.parse(new TextDecoder().decode(content)) as StateData trusts the stored data is valid. If a prior bug or manual edit corrupts the state file, the invalid data passes through silently. The phase field would be an arbitrary string that never matches TRANSITIONS entries, causing confusing error messages rather than a clear "corrupted state" error.

3. issue_lifecycle.ts:609 — review method accessible from any phase including created

   review has no entry in TRANSITIONS, so the valid-transition check returns pass: true (line 157). Calling review before start throws "No plan exists yet" — a correct but slightly confusing error. Not a bug, just an indirect error message.

Verdict

PASS — The code is a clean, well-structured refactoring that removes a large GitHub-coupled subsystem in favor of a simpler HTTP client. The state machine tests are thorough. The medium findings are genuine but non-blocking edge cases: the type cast (#1) is the most worth addressing before merge since it could silently persist invalid data.

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

1. ** health-check overhead**: every method that calls (triage, plan, iterate, adversarial_review, resolve_findings, approve, implement, complete) makes an extra HTTP GET to /api/health before the actual operation. For operations where swamp-club posting is best-effort, the health check cost may not be worth it — if swamp-club is unreachable, the fetch/PATCH will just silently fail anyway. Consider skipping the health check for methods where the client is optional.

2. All swamp_club logs use warning level: this.log = logger?.warning.bind(logger) means every log in the client is a warning, including informational messages. If the host logger distinguishes info vs warning, it may cause noise. Minor, but worth noting.

3. No tests for swamp_club.ts: the new HTTP client (fetchIssue, updateType, patchIssue, postLifecycleEntry, loadAuthFile, createSwampClubClient) has no test coverage. CLAUDE.md only mandates tests for vault/ and datastore/ extensions, so this isn't blocking — but a local Deno.serve mock would give useful regression protection for the new API contract.

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

1. Health-check overhead in createSwampClubClient: every method that calls createSwampClubClient (triage, plan, iterate, adversarial_review, resolve_findings, approve, implement, complete) makes an extra HTTP GET to /api/health before the actual operation. For methods where swamp-club posting is best-effort, the health check cost may not be worth it — if swamp-club is unreachable, the fetch/PATCH will fail silently anyway. Consider skipping the health check for optional-client callers.

2. All swamp_club logs use warning level: this.log = logger?.warning.bind(logger) means every log in the client is a warning, including informational failure messages. If the host logger distinguishes info vs warning, this may cause noise.

3. No tests for swamp_club.ts: the new HTTP client (fetchIssue, updateType, patchIssue, postLifecycleEntry, loadAuthFile, createSwampClubClient) has no test coverage. CLAUDE.md only mandates tests for vault/ and datastore/ extensions, so this is not blocking — but a local Deno.serve mock would give useful regression protection for the new API contract.