feat(logger): standardize output and fold the spinner into the logger#31
feat(logger): standardize output and fold the spinner into the logger#31rgdevme wants to merge 1 commit into
Conversation
Introduce a reusable, domain-aware log-line component. Every line renders
as `[domain] message [status]` with a dimmed domain-colored prefix, a
level-colored message (white/green/yellow/red/blue), an italic-dim status,
and white extra lines. Structured LogInput ({ message, status, extra })
replaces ad-hoc strings; plain strings stay valid.
- Scope ctx.logger per domain across the run pipeline, CLI command
dispatch, and init steps via withDomain, so every line carries its
[domain] prefix and color automatically (docs=cyan, skills=magenta,
rules=green, mcp=blue, hooks=yellow, agents=gray; supervisor -> [agnos]).
- Fold the spinner in: log methods accept an optional waitFor promise
(plus done) that animates a standardized, domain-colored spinner until
it settles, then returns the resolved value. Remove withSpinner /
createSpinner and migrate all call sites.
- Strip now-redundant domain-name prefixes from messages and drop the
inline metadata-shape block from the docs/rules warnings (the shape
lives in the doc-authoring skill).
PR Summary by Qodofeat(logger): Standardize CLI output and fold spinner into logger
AI Description
Diagram
High-Level Assessment
Files changed (19)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
64 rules✅ Skills:
doc-authoring, doc-authoring-with-logs 1. make() uses unjustified any
|
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (msg: any): any => { | ||
| if (msg !== null && typeof msg === "object" && "waitFor" in msg) return spin(level, msg); | ||
| emit(level, msg); |
There was a problem hiding this comment.
1. make() uses unjustified any 📘 Rule violation ⚙ Maintainability
src/core/logger.ts introduces any types in the log method implementation and in the indentedLogger wrapper signature without a directly attached rationale comment explaining why any is necessary. This violates the requirement to justify TypeScript any, weakening type-safety in a core logging utility and making future refactors riskier.
Agent Prompt
## Issue description
`src/core/logger.ts` uses TypeScript `any` in the logger method implementation and in the `indentedLogger` wrapper signature without an adjacent rationale comment directly tied to each `any` usage.
## Issue Context
Compliance (PR Compliance ID 704843) requires every TypeScript `any` to be justified with an explicit comment directly attached to the `any` line (adjacent or inline) explaining why the escape hatch is necessary; an eslint-disable alone is not sufficient. Where feasible, prefer typing the overload implementation and wrappers without `any`.
## Fix Focus Areas
- src/core/logger.ts[190-193]
- src/core/logger.ts[240-242]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| function renderActive(): void { | ||
| if (!active) return; | ||
| const glyph = SPINNER_FRAMES[frame++ % SPINNER_FRAMES.length]; | ||
| process.stderr.write(`\r${COLORS.cyan}${glyph}${COLORS.reset} ${active.message}`); | ||
| const glyph = SPINNER_FRAMES[frame++ % SPINNER_FRAMES.length]!; | ||
| process.stderr.write(`\r${active.prefix} ${DOMAIN_PAINT[active.color](glyph)} ${active.message}`); | ||
| } |
There was a problem hiding this comment.
2. Spinner leaves trailing text 🐞 Bug ☼ Reliability
The new spinner frame renderer writes with a carriage return but does not clear the line first, so a shorter subsequent spinner line can leave leftover characters visible on stderr and corrupt the standardized output shape. This is triggered when one spinner supersedes another (or any future case where the rendered spinner line length decreases).
Agent Prompt
### Issue description
The spinner redraw uses `\r` without clearing the terminal line. When a new spinner line is shorter than the previous one, stale characters remain at the end of the line, visually corrupting output.
### Issue Context
This happens only in TTY stderr mode (when spinners are active), but it directly contradicts the PR goal of standardized output shapes.
### Fix Focus Areas
- src/core/logger.ts[72-76]
- src/core/logger.ts[99-113]
### Suggested fix
- Clear the current stderr line before writing each spinner frame (e.g. prepend `\r\x1b[2K` in `renderActive()`), **or** clear once when starting/superseding a spinner before the first `renderActive()`.
- Ensure the supersede path clears the previous spinner line before rendering the new spinner’s first frame.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
What
Standardizes all CLI output into a single reusable, domain-aware logging component and folds the spinner into it.
Every line now renders in one shape:
[domain]— padded, dimmed, in the domain's color (docs=cyan, skills=magenta, rules=green, mcp=blue, hooks=yellow, agents=gray; supervisor/core lines use[agnos]).changed).How
formatLogcomponent + structuredLogInput({ message, status, extra }); plain strings stay valid.ctx.loggeris scoped per domain viawithDomainacross the run pipeline, CLI command dispatch, and init steps — so every line carries its prefix/color automatically. Removes now-redundant hand-rolleddomain:prefixes and the inline metadata-shape block from the docs/rules warnings (the shape lives in the doc-authoring skill).waitForpromise (plusdone) that animates a standardized, domain-colored spinner until it settles, then returns the resolved value.withSpinner/createSpinnerare removed and all 8 call sites migrated; the hand-passed{ quiet }is gone (the logger owns it).Verification
pnpm typecheck— cleanpnpm test— 241/241 passpnpm lint— 0 errorsSpinners are no-ops in the non-TTY test env, so migrated sites behave exactly as before (await + return value); live spinner appearance is best eyeballed in a real terminal.
Notes
Scoped to
src/**+test/**. Unrelated in-progress working-tree edits (.docs/plans/prd.md— which currently has adescripti0ontypo —.docs/index.md,AGENTS.md,package.json) were intentionally left out of this branch.