feat: auto-move new GitHub issues to swamp.club lab#1134
Conversation
Repurposes the issue auto-responder workflow into an auto-mover and updates the swamp-club extension client to consume the numeric lab issue ids introduced by systeminit/swamp-club#364 and #369. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
Blocking Issues
None.
Suggestions
-
Stale class-level doc comment (
extensions/models/_lib/swamp_club.ts:34-36): TheSwampClubClientclass doc still says "Resolves the swamp-club issue ID lazily" — should say "lab issue number" to match the rest of the migration. -
All issues hardcoded as
type: "feature"(.github/workflows/auto-response.yml:54): Every auto-moved issue is sent withtype: "feature"regardless of content. If the lab supports bug reports, consider inferring the type from GitHub labels or defaulting to something more neutral. Fine for now if the lab always triages type separately. -
No error handling on the comment/close calls (
.github/workflows/auto-response.yml:92-106): IfcreateCommentsucceeds butissues.update(close) throws, the issue stays open with a "now managed in lab" comment but isn't actually closed. Unlikely but worth noting — the current fail-open behavior is safe, just potentially confusing for the reporter.
Overall this is a clean, well-reasoned migration. The sequential gating in the GitHub Action (ensure → validate → comment → close) correctly preserves the safety invariant that no issue is closed without a confirmed lab counterpart. The Number.isSafeInteger validation mirrors the server contract. The TypeScript client changes are straightforward renames with tighter runtime validation replacing the old UUID regex. DDD-wise, the SwampClubClient remains a well-scoped infrastructure/integration service — no domain concerns here.
There was a problem hiding this comment.
CI Security Review
Critical / High
None.
Medium
None.
Low
None.
Verdict
PASS — The workflow changes are security-clean. Issue data is handled safely via JavaScript (context.payload.issue) rather than GitHub Actions expression interpolation, secrets are properly scoped, permissions are minimal, the GitHub-owned action uses an acceptable tag pin, and the lab API response is validated with Number.isSafeInteger before use in URL construction. The close-after-confirm flow correctly prevents closing a GitHub issue without a confirmed lab counterpart.
Summary
Adds a GitHub Action that auto-moves every new issue filed against
systeminit/swampinto the swamp.club lab, posts a comment on the GitHub issue linking to the new lab home, and closes the GitHub issue. Also migrates the existing@swamp/issue-lifecycleextension client to the numeric lab issue id contract introduced by systeminit/swamp-club#364 and #369.User-visible behaviour change
When anyone files an issue against
systeminit/swamp:Automoved by swampadmin from GitHub issue #<n>footer appended) is POSTed tohttps://swamp.club/api/v1/lab/issues/ensure.If the swamp-club call fails for any reason, the action fails and the GitHub issue stays open — there is no failure mode in which we close a GitHub issue without confirming the lab counterpart exists. See "Why the close is gated" below.
The previous behaviour (welcome comment +
needs-triagelabel, with a maintainer skip) is removed. Every new issue gets moved unconditionally — issues filed by maintainers go through the same path as community issues, by design.Contract change in swamp-club#369
systeminit/swamp-club#364 introduced a sequential, human-friendly
numberon every lab issue (allocated by an atomic Mongo counter) and migrated the UI route from/lab/{uuid}to/lab/{number}.systeminit/swamp-club#369 finished the job: it collapsed
/api/v1/lab/issues/{uuid}and/api/v1/lab/issues/by-number/{n}into a single/api/v1/lab/issues/{number}tree, so every endpoint that used to take a UUID now takes the sequential number. The internal UUID is still the Mongo foreign key (comments, events, lifecycle entries reference it) but it never appears in a URL.Endpoints affected, all of which this client now hits via the new shape:
/api/v1/lab/issues/{number}PATCH(used bytransitionStatus)/api/v1/lab/issues/{number}/lifecyclePOST(used bypostLifecycleEntry)/api/v1/lab/issues/ensurePOST(unchanged — still no path id)This client previously cached the UUID it received from
/ensureand interpolated it into the lifecycle/PATCH paths. After #369, those calls would 400 with\"Invalid issue number\"because the server-sideparseLabIssueNumberParamvalidator (^\d+$+Number.isSafeInteger) rejects UUIDs. The migration in this PR is necessary to keep the issue-lifecycle integration working at all.Tracking issue
This PR closes the client-side half of the work I tracked in systeminit/swamp-club#367, which I filed when I first noticed the contract drift. The server-side half shipped as #369 (already merged).
Safe-integer validation rationale
swamp-club#369 added a strict validator
lib/lab-issue-number-param.tsthat usesNumber.isSafeIntegerinstead ofNumber.isInteger. The PR description explains why: during live testing,Number.parseInt(\"7d03f113-80a5-...\", 10)silently truncated a UUID to7and routed a request meant for one issue onto issue #7, clobbering its title. The strict validator catches that hazard.This client doesn't parse strings (it consumes a JSON number from
/ensureand emits it back into a URL path), so the parseInt hazard is not directly reachable. But mirroring the server's safe-integer bound is still the right call for two reasons:ascast — runtime-trivial. The new code readsdata?.issue?.numberdefensively (typed asunknown) and runs it through the sameNumber.isSafeInteger+ positive check. A malformed response can no longer throw aTypeErrorbefore we hit our validator, and a numerically-suspicious response (NaN, Infinity, fractions, anything past 2^53-1) is rejected with a structured log line.Why the GitHub Action close is gated on a valid lab number
The action's only safety property is: a GitHub issue must never be closed unless its lab counterpart exists. If we close before confirming, an issue can disappear from GitHub without ever being filed in the lab — losing the report.
The flow is therefore strictly sequential and short-circuits on every error:
/api/v1/lab/issues/ensure. If non-2xx →core.setFailed(...)andreturn(issue stays open).data.issue.numberis missing or not a safe positive integer →core.setFailed(...)andreturn(issue stays open).core.setFaileddoes not stop execution by itself inactions/github-script, so each guard is paired with an explicitreturn. Steps 3 and 4 are unreachable unless we have a confirmedlabIssueNumber.Files changed
.github/workflows/auto-response.ymlRepurposed end-to-end:
needs-triagelabel step and the welcome-comment step.actions/github-script@v7, Node 20 fetch) that:context.payload.issue.\n\n---\nAutomoved by swampadmin from GitHub issue #<n>footer (handles empty bodies).https://swamp.club/api/v1/lab/issues/ensurewith{ githubRepoFullName, githubIssueNumber, title, body, type: \"feature\", githubAuthorLogin }, 15sAbortSignal.timeout.data?.issue?.numberaccess,Number.isSafeInteger,> 0. Mirrors swamp-club'sparseLabIssueNumberParam.\"Thank you for opening an issue. This issue is now managed in the Claude lab.\\n\\nhttps://swamp.club/lab/<number>\".issues.update({ state: \"closed\" }).SWAMP_API_KEY(already configured).on.issues.types: [opened]. Permissions are unchanged:issues: write,contents: read.extensions/models/_lib/swamp_club.tsprivate issueId: string | null→private labIssueNumber: number | null. The cached value is now the sequential lab id, not the UUID.ensureIssuenow returnsPromise<number | null>(wasPromise<string | null>).data?.issue?.numberwithunknowntyping (wasdata.issue.idwith a{ id: string }cast).typeof === \"number\"+Number.isSafeInteger+ positive (was a UUID regex). Mirrors swamp-club'sparseLabIssueNumberParam.\"swamp-club returned invalid lab issue number: {number}\"on validation failure (was\"...invalid issue ID...\").postLifecycleEntrynow builds${baseUrl}/api/v1/lab/issues/${this.labIssueNumber}/lifecycleand gates onthis.labIssueNumber === null(was!this.issueId).transitionStatusnow builds${baseUrl}/api/v1/lab/issues/${this.labIssueNumber}and gates onthis.labIssueNumber === null(was!this.issueId).labUrl()helper: returns${baseUrl}/lab/${labIssueNumber}if the issue has been ensured, ornullotherwise. Lets callers display the public lab URL without re-fetching anything.createSwampClubClientdoc comment: "The lab issue number is resolved lazily — no extra arg needed." (was "The issue ID is resolved lazily — no swampClubIssueId arg needed.")extensions/models/issue_lifecycle.tsensureSwampClub: renamed localid→labIssueNumber, replacedif (!id)withif (labIssueNumber === null). The strict null check guards against a future0being misread as missing — the server validates> 0, so0is currently unreachable, but=== nullis the right pattern for anumber | nullreturn type either way.ensureIssuecall site (triage_startedlifecycle entry, line ~435) discards the return value, so no change needed.extensions/models/README.mdTest plan
Local verification
deno fmt --check— clean (1114 files)deno lint— clean (1008 files)deno run test— 4193 passed, 0 failed (3m30s)deno check extensions/models/_lib/swamp_club.ts extensions/models/issue_lifecycle.ts— cleanClient behaviour (manual / by-inspection)
ensureIssuehappy path: server returns{ issue: { number: 42, ... }, created: true }→ cache populated,42returned.ensureIssuecache hit: second call short-circuits viathis.labIssueNumber !== nulland returns the cached number without re-fetching.ensureIssuerejects malformed responses without throwing: missingissuekey, missingnumberkey,number: \"42\"(string),number: 0,number: -1,number: 1.5,number: NaN,number: 2 ** 53(past safe integer). Each path logs a structured warning and returnsnull.postLifecycleEntryandtransitionStatusno-op whenlabIssueNumber === null(i.e. beforeensureIssuesucceeds).labUrl()returnsnullbeforeensureIssue, then${baseUrl}/lab/{number}after.GitHub Action
systeminit/swampafter this PR merges. Expect:https://swamp.club/lab/<n>link.Automoved by swampadmin from GitHub issue #<n>footer.SWAMP_API_KEYsecret value (or point at an unreachable host) and file a throwaway issue. Expect:core.setFailedmessage.idfield, expect the action to fail with\"swamp.club ensure returned no lab issue number: ...\"and the issue to stay open.I have not pre-merged the smoke tests because they require the workflow to be on
mainto fire on realissues: openedevents. Suggest doing them as the first action after merge with throwaway issues.Related
parseLabIssueNumberParamand caught a real data-corruption hazard withNumber.parseInt.numberfield on lab issues and migrated the UI route to/lab/{number}.🤖 Generated with Claude Code