Skip to content

refactor: route live session commands through ApiGateway#13

Merged
riglar merged 2 commits into
devfrom
refactor/live-gateway
Jun 13, 2026
Merged

refactor: route live session commands through ApiGateway#13
riglar merged 2 commits into
devfrom
refactor/live-gateway

Conversation

@riglar

@riglar riglar commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Final item from the #3 code review: the five live subcommands (start/install/exec/stop/status) each built their own raw fetch() with hand-spread auth.headers and a per-call handleApiError — the one architecture violation flagged in the review (CLAUDE.md mandates commands orchestrate services with no I/O logic; HTTP belongs in ApiGateway).

More than a tidiness issue: because they bypassed the gateway, they also bypassed its network-error enhancement. A DNS/connection failure surfaced as a bare fetch failed TypeError instead of the friendly "Network request failed... Possible causes" diagnostic every other command gives.

This adds five methods to ApiGatewaystartLiveSession, installLiveBinary, execLiveYaml, stopLiveSession, getLiveSession — each following the established try / fetch / handleApiError / catch → enhanceFetchError skeleton, plus typed LiveSession / LiveSessionSummary / LiveExecResult interfaces (the swagger has no /live schema, so these are hand-defined). live.ts keeps all its presentation logic and just calls the gateway: −66 lines in the command.

Verification

Smoke-tested against the mock API:

  • network-error path (live status → unreachable URL): now the enhanced "Network request failed" message — previously a bare fetch failed ← the actual fix
  • auth-error path (live stop → invalid key): auth-mode-neutral message with operation context
  • happy path and 415s flow through handleApiError unchanged

pnpm test 122/122 · pnpm typecheck clean (strict) · pnpm lint 0 errors · pnpm build clean

🤖 Generated with Claude Code

The five live subcommands (start/install/exec/stop/status) each built
their own fetch() with hand-spread auth.headers and per-call
handleApiError, the one architecture violation flagged in the review
(CLAUDE.md: commands orchestrate services, no I/O logic). They also
bypassed ApiGateway's network-error enhancement, so a DNS/connection
failure surfaced as a bare "fetch failed" TypeError instead of the
friendly diagnostic every other command gives.

Adds startLiveSession/installLiveBinary/execLiveYaml/stopLiveSession/
getLiveSession to ApiGateway (with typed LiveSession/LiveSessionSummary/
LiveExecResult interfaces, since the swagger has no /live schema), each
following the existing try/fetch/handleApiError/enhanceFetchError
skeleton. live.ts keeps all its presentation logic and just calls the
gateway. Net -66 lines in the command.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread src/gateways/api-gateway.ts Outdated
Comment on lines +40 to +59
/** Summary returned when a live session is created. */
export interface LiveSessionSummary {
id: number;
platform: string;
session_name: string;
status: string;
}

/** Full live session record returned by the status endpoint. */
export interface LiveSession extends LiveSessionSummary {
binary_upload_id: null | string;
created_at: string;
}

/** Result of executing Maestro YAML against a live session. */
export interface LiveExecResult {
error?: string;
output?: string;
success: boolean;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The three JSDoc comments describe only what each type is — information already fully conveyed by the interface names. Per CLAUDE.md: "Default to writing no comments. Only add one when the WHY is non-obvious… Don't explain WHAT the code does, since well-named identifiers already do that."

The one genuinely non-obvious WHY — that these are hand-defined because the swagger schema has no /live routes and openapi-typescript can't generate them — is currently absent from the code. That belongs as a single comment.

Suggested change
/** Summary returned when a live session is created. */
export interface LiveSessionSummary {
id: number;
platform: string;
session_name: string;
status: string;
}
/** Full live session record returned by the status endpoint. */
export interface LiveSession extends LiveSessionSummary {
binary_upload_id: null | string;
created_at: string;
}
/** Result of executing Maestro YAML against a live session. */
export interface LiveExecResult {
error?: string;
output?: string;
success: boolean;
}
// Hand-defined: swagger has no /live routes so openapi-typescript cannot generate these.
export interface LiveSessionSummary {
id: number;
platform: string;
session_name: string;
status: string;
}
export interface LiveSession extends LiveSessionSummary {
binary_upload_id: null | string;
created_at: string;
}
export interface LiveExecResult {
error?: string;
output?: string;
success: boolean;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially applied. The non-obvious WHY (swagger has no /live routes for openapi-typescript to generate) is now a header comment on the new domain file. I kept the brief per-interface comments: they note which endpoint returns each type, which clarifies the otherwise-confusing Summary-vs-full distinction, and that's consistent with the contextual doc comments in the existing auth.types.ts. Note: the quoted "Default to writing no comments…" text isn't actually in this repo's CLAUDE.md — the verifiable rule (domain types location) drove the change.

Comment thread src/gateways/api-gateway.ts Outdated
}

/** Summary returned when a live session is created. */
export interface LiveSessionSummary {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LiveSessionSummary, LiveSession, and LiveExecResult are exported from this file and consumed in src/commands/live.ts, making them cross-layer types. Per CLAUDE.md: "Domain types live in src/types/domain/."

The existing src/types/domain/auth.types.ts and device.types.ts follow this pattern for types that cross layer boundaries. Consider moving these three interfaces to a new src/types/domain/live.types.ts and importing them back into api-gateway.ts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f0082fa — moved all three interfaces to a new src/types/domain/live.types.ts and imported them back into api-gateway.ts, matching the auth.types.ts/device.types.ts pattern. Verified the CLAUDE.md:31 citation, good catch.

Addresses code review on PR #13: cross-layer domain types belong in
src/types/domain/ per CLAUDE.md (alongside auth.types.ts and
device.types.ts), and the non-obvious reason these are hand-defined
(swagger has no /live routes for openapi-typescript to generate from)
is now captured in a comment.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jun 13, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@riglar riglar merged commit 6757aa5 into dev Jun 13, 2026
6 checks passed
@riglar riglar deleted the refactor/live-gateway branch June 18, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant