feat(cli): add swamp issue get command (swamp-club#118)#1326
Conversation
…club#118) Adds a lightweight CLI command to fetch and display existing Lab issue details without creating a full lifecycle model instance. Supports both log and json output modes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Adversarial Review
Critical / High
None found.
Medium
- Dead
errorevent kind inIssueGetEvent(src/libswamp/issues/get.ts:38): The{ kind: "error"; error: SwampError }variant is defined in the event union but never yielded by the generator. Ifdeps.fetchIssuethrows, it propagates as an uncaught exception — it does not become a{ kind: "error" }event. Theerrorhandlers in bothLogIssueGetRendererandJsonIssueGetRenderer(src/presentation/renderers/issue_get.ts:56,69) are therefore unreachable dead code. This is consistent with the existingIssueCreateEventandIssueCommentEventpatterns, so it's a codebase-wide convention rather than a PR-specific bug, but it means theEventHandlerstype forces callers to handle an event that can never arrive.
Low
-
No null guard on
data.issueafterres.json()(src/infrastructure/http/swamp_club_client.ts:291): If the server returns HTTP 200 with an unexpected JSON shape (e.g.{"success": true}with noissuefield),data.issueisundefinedandissue.numberthrows a rawTypeErrorinstead of a friendlyUserError. Example: a reverse proxy or CDN returning a cached/generic response. The same unguarded pattern exists insubmitIssue(line 208) andsubmitComment(line 263), so this is pre-existing. -
No special handling for HTTP 401 in
fetchIssue(src/infrastructure/http/swamp_club_client.ts:280-287): An expired or invalid API key producesFailed to fetch issue #42 (HTTP 401): <raw body>rather than a clear "not authenticated" message. Compare withwhoami(line 162) which returns{ authenticated: false }on 401. Not a correctness issue — the user gets an error either way — but the message quality differs from thewhoamipattern.
Verdict
PASS — Clean, well-structured PR that follows existing codebase patterns. The generator, client method, CLI command, and renderers are all consistent with their siblings (issue create, issue ripple). Input validation matches the existing issue_ripple.ts pattern. No security, concurrency, or resource management concerns.
There was a problem hiding this comment.
CLI UX Review
Blocking
-
src/presentation/renderers/issue_get.ts— wrong output function for log modeLogIssueGetRendereruseslogger.info()(viagetSwampLogger) for every line of user-facing output. Every other "get" renderer in the codebase —model_get.ts,data_get.ts,report_get.ts— useswriteOutput()instead. ThewriteOutput()function is explicitly documented:"Writes plain text to stdout with no decoration (no timestamp, level, or category prefix). Use for human-readable CLI 'log' mode output that should read like a document rather than a log stream."
Because
logger.info()routes through LogTape, users will see log metadata injected into what should be a clean issue record — lines like#42: My Issuebecome something like[issue][get] #42: My Issueor carry timestamp prefixes depending on log configuration.logger.info("")for blank lines is especially bad. Fix: replacelogger.info(...)calls withwriteOutput(...)and build up the output string similarly to howdata_get.tsandmodel_get.tsdo.
Suggestions
-
No color formatting in log mode. Every other "get" renderer (
model_get.ts,data_get.ts,audit_timeline.ts) usesbold,cyan, anddimfrom@std/fmt/colorsto make labels scannable. Theissue_getlog output is entirely uncolored plain text. Consider formatting field labels withbold(cyan("Type:"))etc. to match the visual language of the rest of the CLI. -
Issue body is markdown — should use
renderMarkdownToTerminal(). The body field of a Lab issue is markdown (the JSON example inoutput_shapes.mdshows"## Problem\n\n...").report_get.tscallsrenderMarkdownToTerminal(r.markdown)for exactly this case. Rendering the body as raw markdown text makes##headers and**bold**visible as literal punctuation in the terminal.
Verdict
NEEDS CHANGES — the log-mode renderer routes all user-facing output through logger.info() instead of writeOutput(), which will inject log metadata into what should be clean document-style output, inconsistent with every other "get" command.
There was a problem hiding this comment.
Code Review
Clean implementation that follows established patterns well. DDD layering is correct — the generator in libswamp defines its own IssueGetDeps interface for dependency inversion, the CLI command wires concrete infrastructure, and the renderer handles presentation. All imports from libswamp/mod.ts are correct (no internal path imports). Both log and json output modes are supported. AGPLv3 headers present on all new files. LogTape usage is correct (tagged template in generator, structured logging in renderer).
Blocking Issues
None.
Suggestions
-
Missing renderer unit test —
src/presentation/renderers/issue_get_test.tsdoesn't exist. The siblingissue_createrenderer has a test file that capturesconsole.logoutput and verifies the JSON shape. At minimum, testing the JSON renderer output would verify the shape matches what's documented inoutput_shapes.md. -
Missing
SwampClubClient.fetchIssuetest — The method has non-trivial response parsing (nesteddata.issue, assignee filtering, comment counting) and error handling (404 →UserError). The client test file exists but has no coverage for the new method. -
IssueGetDeps.fetchIssuereturn type duplicatesIssueGetData— The inline return type onfetchIssueinIssueGetDeps(lines 45-55 ofsrc/libswamp/issues/get.ts) is identical toIssueGetDatadefined just above. Could simplify tofetchIssue: (issueNumber: number) => Promise<IssueGetData>to reduce duplication.
Switch log-mode renderer from logger.info() to writeOutput(), add color formatting with bold/cyan/dim, and render issue body as markdown via renderMarkdownToTerminal(). Deduplicate IssueGetDeps return type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
CLI UX Review
Blocking
None.
Suggestions
-
Cramped metadata line (
src/presentation/renderers/issue_get.ts:34):Type:,Status:, andAuthor:are packed onto a single line with double-space separators.model_get.tsputs each label on its own line usingbold(cyan(...))— a user reading a long author name or status string could find the packed format harder to scan. Consider splitting into separate lines for consistency. -
Label color inconsistency (
src/presentation/renderers/issue_get.ts): The issue number usesbold(cyan(...))but the meta labels (Type:,Status:,Author:, etc.) use plainbold(...). Every other detail renderer (model_get,report_get) usesbold(cyan(...))for all labels. Minor visual inconsistency. -
Comment count shown even when 0 (
src/presentation/renderers/issue_get.ts:41):Comments: 0is always displayed, while the similarAssignees:field is conditionally hidden when empty. Could apply the same pattern (if (d.commentCount > 0)) for a cleaner output on issues with no comments.
Verdict
PASS — both log and JSON modes work correctly, error messages are clear and actionable (Issue #N not found., Not logged in. Run "swamp auth login" first.), the JSON shape matches the documented output, and the command follows the existing issue-command conventions.
There was a problem hiding this comment.
Adversarial Review
Critical / High
None.
Medium
-
src/infrastructure/http/swamp_club_client.ts:291—data.issueaccess without null guard: If the server returns a 200 response with a body wheredata.issueisundefinedornull(e.g.,{}), thenissue.numberon line 293 throws a rawTypeError: Cannot read properties of undefined (reading 'number')instead of a descriptiveUserError. The individual fields (title,type, etc.) are guarded with??, but the top-levelissuekey is not. Example: server deploys a breaking change that wraps the response differently → user sees an opaque stack trace. Fix: add a guard likeif (!data?.issue) throw new UserError("Unexpected response from server.");before accessing fields. This is medium because it's a server contract violation scenario, not a user-input scenario. -
src/infrastructure/http/swamp_club_client.ts:285-287— Server error bodies included verbatim in user-facing error messages: For non-404 errors (401, 403, 500), the full response body text is concatenated into theUserErrormessage. If the server returns verbose debug output or HTML error pages, the user sees a wall of text. Thewhoamimethod (line 162-163) handles 401 specially without exposing the response body. This is inconsistent but not a correctness bug — it's a UX concern. Example: expired API key →Failed to fetch issue #42 (HTTP 401): <!DOCTYPE html>...or a JSON blob with internal server details.
Low
-
src/cli/commands/issue_get.ts:45— Redundant validation: The<number:integer>Cliffy argument type already guaranteesissueNumberis an integer. The manualNumber.isInteger(issueNumber) || issueNumber <= 0check is defense-in-depth, which is fine. The<= 0check does add value since Cliffy doesn't reject negative integers —swamp issue get -1would be caught here rather than producing a confusing 404. Not a bug, just noting it's intentional defense-in-depth. -
src/libswamp/issues/get.ts:37-38—errorevent kind declared but never yielded: TheIssueGetEventunion includes{ kind: "error"; error: SwampError }but the generator never yields this variant. Errors fromdeps.fetchIssuepropagate as thrown exceptions, not as yielded events. This is consistent withissueCommentincomment.tswhich also lets dep errors throw, so it follows the established pattern. Theerrorhandler in renderers is required byEventHandlers<E>exhaustiveness but is effectively dead code for this generator.
Verdict
PASS — Clean, well-structured implementation that follows existing patterns (issueComment, submitIssue). The generator/renderer/CLI split is consistent with the codebase's architecture. No security, correctness, or resource management issues found.
There was a problem hiding this comment.
Code Review
Clean, well-structured implementation of swamp issue get. The architecture follows established patterns correctly:
- DDD:
IssueGetDatais a proper value object,IssueGetDepscleanly abstracts the infrastructure dependency via DI, the generator lives in the application/libswamp layer as expected. - Import boundary: Both the CLI command and presentation renderer import from
src/libswamp/mod.ts— never internal paths. ✓ - License headers: Present on all new
.tsfiles. ✓ - Output modes: Both
logandjsonmodes supported. ✓ - Tracing: Generator uses
withGeneratorSpanconsistently with other generators. ✓ - Security: Auth is verified before API call, API key via
x-api-keyheader (consistent with existing methods), input validated, no injection vectors (issue number is an integer).
Blocking Issues
None.
Suggestions
-
CLI command test missing logging init (
src/cli/commands/issue_get_test.ts): CLAUDE.md states CLI command tests requireinitializeLogging({})andimport "../../domain/models/models.ts". This test importsissueGetCommandstatically (which transitively pulls inlibswamp/mod.ts). It works today because the issue path doesn't trigger model registration side effects, but adding the init would be consistent with the documented convention (seedata_get_test.tsfor the canonical pattern). Other issue command tests (bug, ripple) also skip it, so this is a codebase-wide gap rather than specific to this PR. -
No renderer test (
src/presentation/renderers/issue_get.ts): Many renderers in the codebase have corresponding_test.tsfiles. A test verifying the log-mode output format and JSON-mode output structure would strengthen coverage, though this isn't blocking since the generator tests cover the data flow.
Summary
swamp issue get <number>CLI command to fetch and display Lab issue details (title, type, status, author, body, assignees, comment count)log(human-readable) andjson(structured) output modesfetchIssuemethod toSwampClubClientusingx-api-keyauth (consistent with existing methods)swamp-issueskill with new command documentation and JSON output shapeCloses swamp-club#118
Test Plan
issueGetlibswamp generator (src/libswamp/issues/get_test.ts)src/cli/commands/issue_get_test.ts)deno check— all files pass type checkingdeno lint— no lint errorsdeno fmt— all files formatteddeno run test— 5542 passed (1 pre-existing flaky failure in unrelated workflow test)deno run compile— binary compiles successfully🤖 Generated with Claude Code