Skip to content

refactor(cli): WS4 part 1 — readonly split, renderInkReport, shared http client (BAKIN_URL fix), output helpers#503

Open
markhayden wants to merge 6 commits into
mainfrom
refactor/cli
Open

refactor(cli): WS4 part 1 — readonly split, renderInkReport, shared http client (BAKIN_URL fix), output helpers#503
markhayden wants to merge 6 commits into
mainfrom
refactor/cli

Conversation

@markhayden

Copy link
Copy Markdown
Owner

WS4 part 1 — the low-risk, fully-verified slice of the audit's CLI consolidation (refactor/cli).
The remaining large/fragile phase (B5: per-scope command modules + slim router + emit() dispatcher

  • no-arg/update/help-registry divergence fixes) lands as a focused follow-up, mirroring the
    WS3 → WS3b split.

Spec: .claude/specs/audit-2026-06/REPORT.md + APPENDIX-cohesion.md. Plan: tasks/plan.md.

What's in it (one commit per phase)

  • B1 — split readonly.tsx (2,809 lines) into 11 per-domain modules under
    src/core/cli/ui/reports/ + a readonly.ts barrel. Pure relocation per the appendix mapping; the
    ~30 dynamic import('.../readonly') call sites + readonly-ui.test.tsx (imports all 34 components
    by name) resolve through the barrel unchanged. doctor.tsx/doctor-repair.tsx adopt the shared
    plural(). (Kept doctor-repair's valueText local — not a true dup; object/array handling differs.)
  • B2 — renderInkReport helper replacing 41 byte-identical printXxxTui wrappers (−204 net lines).
    Lazy internals so nothing new enters the binary entry's import graph.
  • B3 — shared src/cli/http.ts (one BAKIN_URL-aware client) + fixes the schedule BAKIN_URL bug
    (bakin schedule … silently hit localhost regardless of BAKIN_URL).
  • B4 — src/cli/output.ts — 8 pure output/formatting helpers extracted from cli/bakin.ts.

Net: cli/bakin.ts 4,632 → ~4,360; readonly.tsx 2,809 → 12 focused files.

Scope discipline

Behavior-sensitive items are deliberately deferred to B5, not forced here: the emit() isTTY
dispatcher (~95 sites), the confirm-prompt unification (two of three deliberately lack an isTTY
guard), the exit-helper move (entangled with TUI wrappers used by non-exit sites), and the
isServerConnectionError typed-error change (moved verbatim). All noted in tasks/plan.md.

Verification

  • bun run typecheck clean; bun run lint clean.
  • bun test tests/cli/ --isolate: 252 pass / 0 fail at every phase (readonly-commands.test.ts
    checks actual rendered TUI text, so output is byte-identical).
  • bun run build: full 3-target binary compiled after every cli-touching phase (readonly compiles
    into the single-file binary via src/core/cli.ts); build-stamp files reverted.
  • Not run here (B5 merge gate): dockerized-rig E2E — reserved for the behavior-changing B5 phase.

🤖 Generated with Claude Code

markhayden and others added 6 commits June 15, 2026 10:36
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rrel

src/core/cli/ui/readonly.tsx (2,809 lines) → 11 per-domain modules under
src/core/cli/ui/reports/ (format, command-meta, runtime, settings, tasks, workflows,
plugins, packages, search, schedule, trash) + a readonly.ts barrel (export * from each).
Pure mechanical relocation per APPENDIX-cohesion.md — no behavior change, no DTO retyping,
every export name preserved. The ~30 dynamic import('.../cli/ui/readonly') call sites and
tests/cli/readonly-ui.test.tsx (imports all 34 components by name) resolve through the
barrel unchanged. doctor.tsx + doctor-repair.tsx now import the shared plural() from
reports/format.

Deviation from the appendix: doctor-repair's local valueText is NOT a true dup of
format's (format stringifies objects/arrays; doctor-repair returns the fallback) — kept
local to preserve rendering. Only plural() was de-duped there.

Verified: typecheck clean; tests/cli 252/0 (--isolate); readonly-ui 24/0; full binary
build (3 targets) compiles the split through src/core/cli.ts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…appers

Adds src/core/cli/ui/render-report.ts — a single generic renderInkReport(loadModule,
pick, props) that lazily imports the UI module + render-to-string + react and renders one
report component to stdout. Collapses the 41 byte-identical printXxxTui wrappers in
cli/bakin.ts (each a Promise.all([import(ui), import(render-to-string), import(react)]) +
console.log) onto it; the wrapper functions stay (name + typed signature) so the ~95 call
sites are unchanged. cli/bakin.ts: 4,632 → 4,428 (−204 net).

The helper takes a loadModule selector because wrappers render from three UI modules
(readonly barrel, onboarding, doctor-repair), not just readonly. Only dynamic imports
inside, so the static import from cli/bakin.ts never pulls react/ink into the binary
entry graph. Public signature is fully typed (no any leaks); one contained createElement
cast works around generic-overload resolution.

Left out of scope (not standalone wrappers): the inline DoctorReport render, a
Fragment-with-children logs header, and two onboarding inline renders entangled with
surrounding command state.

Verified: typecheck clean; tests/cli 252/0 (--isolate, identical rendered TUI text);
eslint clean; full binary build (3 targets).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_URL bug

Extracts the CLI HTTP client to src/cli/http.ts — one BASE_URL resolution
(process.env.BAKIN_URL || localhost:PORT) plus api/apiGet/apiPost/apiPostJson/apiDelete,
the jsonObject/parseJsonText/apiErrorPayload/isServerConnectionError guards, and
getCliAgent/getCliRoster — moved verbatim out of cli/bakin.ts (which now imports them;
call sites unchanged). Only fetch + the lightweight api-error helpers are imported, so
nothing new enters the binary entry's import graph.

Fixes the real bug: src/cli/schedule.ts hardcoded "http://localhost:PORT" and ignored
BAKIN_URL, so "BAKIN_URL=... bakin schedule list" silently hit the wrong host. It now
imports the shared BASE_URL. Its thin schedule-prefixed typed wrappers stay (they use a
distinct 'API error' prefix; folding them into the generic client would change error text)
— the duplicated, buggy BASE_URL is gone.

Deferred (noted): replacing isServerConnectionError's message-text classification with a
typed connection error — moved verbatim for now; the typed-error improvement is a separate,
optional follow-up.

Verified: typecheck + lint clean; tests/cli 252/0 (--isolate); full binary build.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Moves 8 self-contained stdout/formatting helpers out of cli/bakin.ts into a new
src/cli/output.ts (print, printTable, invocationCommand, normalizeUsage, usageLine,
statusIcon, formatBytes, daysUntil); cli/bakin.ts imports them back. No behavior change.

Deliberately scoped: the exit helpers (exitCommandIssue/Usage/UnknownSubcommand/
CommandFailure) stay in bakin.ts — they render through the printCommandIssueTui/
printCommandFailureTui wrappers, and printCommandFailureTui is also used by three non-exit
command sites, so the wrappers can't move yet. The emit() dispatcher, isTTY-branch rework,
and confirm-prompt unification remain out of scope (later focused step).

Verified: typecheck + lint clean; tests/cli 252/0 (--isolate); full binary build.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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