feat(cli): log to ~/.rawback/logs and add the logs command group - #81
Conversation
The CLI had no verbosity flag and no diagnostics: the only console output was the two sinks inside CommandOutput, so a user reporting a failure had nothing to send. Build on the SDK's logging subsystem. - Global `--log-level` and `-v`/`-vv`, declared beside `--env` and stashed in a new `src/log-level.ts` singleton. Its `@rawback/sdk` import is type-only and erases, so `cli.ts` stays off the SDK barrel and `rawback --help` keeps its startup cost — the trap `src/trace.ts` already documents. - `src/logging.ts` builds the one logger this process owns, lazily and memoized, behind the same boundary that keeps `--help` and `--version` from writing to a user's home. Neither path creates `~/.rawback/logs/`, which is asserted rather than assumed. - `runCli` records the command and its exit code, then flushes in a `finally` — a failed command is exactly the one whose records need to reach the file. - `rawback logs path|show|purge`, modelled on the `config` group. `show` reads only the tail, so it stays fast on a ten-megabyte file, and shows a torn line rather than dropping it. `purge` covers Desktop's log too, since both clients share the directory; it matches only the names this tool writes and never removes the directory itself, because `logging.directory` is user-controlled. Logs go to the file and never to stdout, so raising verbosity cannot disturb a script parsing `--json`. Docs updated across all four surfaces: yargs help, README, the global options table and a `rawback logs` section in docs/commands.md, and a Logging section in docs/configuration.md. The directory path is rendered as a text block rather than a field: the shared fields renderer drops its label padding when a value is long, and a log directory is always a long absolute path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HwoxuU7mv4yFQnecnKoaPY
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e33cf5ff96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| import { commandOutput, type ReadCommandDependencies } from './command.ts' | ||
| import { environmentName } from './config.ts' | ||
| import { logFilesDocument, logLinesDocument, purgeResultDocument } from './features/logs/view.ts' |
There was a problem hiding this comment.
The commit does not contain src/features/logs/view.ts or any other definition of these three exports (checked the full commit tree), so every logs path, logs show, and logs purge handler fails while dynamically importing logs.ts; a normal dependency-enabled typecheck/build will also reject this unresolved module. Add the presenter that this import expects.
AGENTS.md reference: AGENTS.md:L69-L71
Useful? React with 👍 / 👎.
| for (const argument of args) { | ||
| if (argument.startsWith('-')) break | ||
| words.push(argument) |
There was a problem hiding this comment.
This loop treats every positional argument before the first option as part of the command name. For example, rawback camera connect http://user:password@host records the URL, including its password, in the command field whenever the command fails at the default level (and on every run under -v); photo search prompts and local positional paths leak similarly. Determine the matched command independently of user-supplied positional values rather than copying this argv prefix.
Useful? React with 👍 / 👎.
| const { commandLogger, flushCommandLogger } = await import('../../logging.ts') | ||
| const command = commandName(parseArgs) | ||
| const logger = (await commandLogger()).child({ component: 'cli', command }) | ||
| const startedAt = performance.now() | ||
| logger.debug(`running ${command}`, { event: 'cli.command.start' }) | ||
| try { | ||
| await program.parseAsync(parseArgs) |
There was a problem hiding this comment.
commandLogger() runs before program.parseAsync(), but the yargs middleware that calls setSelectedLogLevel only runs during parsing. The memoized root logger is therefore created with no --log-level/-v override, and later calls from command clients return that existing root, so the newly documented verbosity flags have no effect; the same ordering also makes environment-specific logging configuration use the previously selected/default environment instead of --env.
Useful? React with 👍 / 👎.
| const lines = (await readTail(path, count)) | ||
| .map(parseLine) | ||
| .filter((line) => atLeastLevel(line, level)) |
There was a problem hiding this comment.
When --level is supplied, this reads only the last count unfiltered records and then removes lower-level entries. Thus logs show --lines 50 --level error returns no records if 50 later debug lines follow an error, even though the command promises the most recent records at the requested level. Scan enough of the tail to collect up to count matching records, then apply the limit.
Useful? React with 👍 / 👎.
| if (!confirmed) { | ||
| output.info('Left the log files in place.') | ||
| return |
There was a problem hiding this comment.
In an interactive rawback logs purge --json invocation without --yes, declining the prompt takes this branch and writes a styled human message to stdout while exiting successfully. A caller that selected --json will therefore fail to parse the successful result; emit a JSON cancellation result instead, as other mutating JSON commands do.
AGENTS.md reference: AGENTS.md:L83-L84
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| const result = await purgeLogs({ directory, ...(app ? { app } : {}) }) |
There was a problem hiding this comment.
Every non-help invocation opens the memoized CLI logger before dispatch, so rawback logs purge --app cli --yes reaches purgeLogs while this process still holds cli.log open. On Windows—the platform this code already anticipates rejecting deletion of open Desktop logs—the CLI's own file is likewise left behind, causing the default --app all purge to report failure and exit 1 even when Desktop is closed. Flush and close/suspend the root logger before deleting its active file.
Useful? React with 👍 / 👎.
`~/.rawback/logs/` joins the files shared with Desktop, and `logs purge` clears Desktop's log too. Records the two rules a future change would otherwise trip over: diagnostics never go to stdout, and `src/log-level.ts` must stay SDK-free at runtime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HwoxuU7mv4yFQnecnKoaPY
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8ea5800ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!handle) return [] | ||
| try { | ||
| const { size } = await handle.stat() | ||
| const span = Math.min(size, Math.min(count * TAIL_BYTES_PER_LINE, MAX_TAIL_BYTES)) |
There was a problem hiding this comment.
When recent JSONL records average more than 1 KiB—for example, trace records containing request or response bodies—the fixed count * 1024 window contains fewer than count complete records; the 4 MiB cap also prevents --lines 10000 from returning 10,000 moderately sized records. The function then silently returns the undersized result and can expose a sampled fragment as { raw }; read backward in chunks until enough newline-delimited records or the beginning of the file is reached.
Useful? React with 👍 / 👎.
| * `rawback logs show` must not pull all of it into memory to print fifty lines. | ||
| */ | ||
| export async function readTail(path: string, count: number): Promise<string[]> { | ||
| const handle = await open(path, 'r').catch(() => undefined) |
There was a problem hiding this comment.
If the log exists but cannot be opened—for example because of EACCES after permissions or ownership changed—this catch converts the failure into an empty successful result, so logs show misleadingly reports no records and exits zero. Suppress only the missing-file case and propagate other errors so the CLI emits an actionable failure and nonzero status.
AGENTS.md reference: AGENTS.md:L84-L86
Useful? React with 👍 / 👎.
|
|
||
| ```bash | ||
| rawback logs show --level warn # what recently failed, and why | ||
| rawback -v photos upload ~/raw # re-run the failing command with more detail |
There was a problem hiding this comment.
The documented command cannot rerun an upload: photos upload defines the directory as the demanded --path option and has no positional path, so copying this example produces a validation error instead of collecting verbose upload diagnostics. Change it to rawback -v photos upload --path ~/raw.
AGENTS.md reference: AGENTS.md:L124-L125
Useful? React with 👍 / 👎.
The SDK's logging core is now pino, so this tracks its API and its file
naming.
- Records use pino's argument order, `logger.info({ event }, 'message')`.
- `pino-roll` numbers every file from `1`, so there is no `cli.log` to open
by name. `logs show` asks the SDK for the active file, and reports
cleanly when the app has not logged yet; `logs path` and `logs purge`
needed no change, since both already worked off a pattern.
- `--level` filtering keeps its own copy of pino's level numbers rather
than importing a map the SDK no longer exports.
- `flushCommandLogger` goes through the SDK's `flushLogger`: the rolling
destination is asynchronous, and a CLI process exits long before it
would drain on its own.
Docs updated for the numbering and to name pino, since the format is now
one `pino-pretty` reads directly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HwoxuU7mv4yFQnecnKoaPY
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d934668584
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const environment = await readEnvironment( | ||
| dependencies.configPath, | ||
| environmentName(dependencies), | ||
| ).catch(() => undefined) |
There was a problem hiding this comment.
When --env names an unknown environment, this catch silently falls back to the default log directory instead of propagating readEnvironment's validation error. In the destructive case, rawback --env prodcution logs purge --yes can therefore delete the default environment's logs rather than rejecting the typo; malformed configuration is similarly treated as absent. Let configuration errors propagate before listing, reading, or purging files.
AGENTS.md reference: AGENTS.md:L91-L92
Useful? React with 👍 / 👎.
| return { | ||
| ...(typeof record.time === 'string' ? { time: record.time } : {}), | ||
| ...(typeof record.level === 'string' ? { level: record.level } : {}), | ||
| ...(typeof record.component === 'string' ? { component: record.component } : {}), | ||
| ...(typeof record.event === 'string' ? { event: record.event } : {}), | ||
| ...(typeof record.msg === 'string' ? { msg: record.msg } : {}), | ||
| ...(ids?.traceId ? { traceId: ids.traceId } : {}), | ||
| } |
There was a problem hiding this comment.
For valid JSON records, this reconstruction discards every field outside the small allowlist, including the documented HTTP method, path, status, duration, cf-ray, x-request-id, error details, headers, and trace-level bodies. Consequently both the human presenter and logs show --json lose the diagnostic context users enabled -v/-vv to inspect; preserve the parsed record while deriving any presentation-specific fields instead of replacing it.
Useful? React with 👍 / 👎.
| tail -f ~/.rawback/logs/cli.log | jq -c '{time, level, event, ids}' | ||
| jq 'select(.levelValue >= 40)' ~/.rawback/logs/cli.log |
There was a problem hiding this comment.
These troubleshooting commands target cli.log, but the configured pino-roll sink numbers every file and the implementation explicitly discovers the highest-numbered cli.N.log as the active file. A user copying either command therefore gets a missing-file error even while CLI logs exist; make the example discover the active numbered file or direct users through rawback logs path.
AGENTS.md reference: AGENTS.md:L75-L78
Useful? React with 👍 / 👎.
The SDK now rejects `maxFiles: 0` rather than silently rewriting it to 1; a rolling log always has a file open, so `file: false` is how writing to disk is switched off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HwoxuU7mv4yFQnecnKoaPY
2 participants
Important
Blocked on rawback-app/sdk#43. This builds on new
@rawback/sdkAPIs, and the pin here is still the exact0.3.2, so CI cannot go green until that PR merges and release-please publishes. On release, bump the pin and regeneratebun.lock— nothing else in this diff should need to change. It was developed and validated against a local build of that branch.Why
The CLI had no verbosity flag and no diagnostics at all. The only
console.*calls insrc/were the two default sinks insideCommandOutput, so a user reporting a failure had nothing to send and no trace ID to quote.What
Global flags
--log-level <level>and-v/-vv, declared beside--envand stashed in a newsrc/log-level.tssingleton, with the same middleware pattern.That module's
@rawback/sdkimport is type-only and erases at compile time. It matters:cli.tsimports it and runs on every invocation, so a runtime import would drag the SDK barrel — ssh2 included — onto therawback --helppath, worth ~240 ms. This is the trapsrc/trace.tsalready documents.Lazy logger construction
src/logging.tsbuilds the one logger the process owns, memoized, behind the same boundary that keeps--helpand--versionfrom writing to a user's home directory. Neither path creates~/.rawback/logs/— asserted in a test rather than assumed, since a regression there would create files just for asking for help.runClirecords the command and its exit code and flushes in afinally: a failed command is exactly the one whose records need to reach the file, and pino's rolling destination is asynchronous, so a CLI process would otherwise exit before it drains. Only the leading non-flag words are recorded as the command name — an option value can be a search term or a path from the user's disk.rawback logs path|show|purgeModelled on the
configgroup: nested.command(...),.demandCommand(1, …),.strict(), a no-op group handler, and the three per-handler invariants (exit-code guard, lazyawait import,runCommandwrapper). Implementation flat insrc/logs.tswith the house(options, dependencies = {})signature; presenter insrc/features/logs/view.ts.showreads only the tail via a positioned read, so it stays fast on a ten-megabyte log, and shows a line it cannot parse as{ raw }rather than dropping it — a torn record is evidence too. It asks the SDK for the active file rather than building a name.purgedefaults to--app alland so covers Desktop's log, because both clients share one directory. It matches only the filenames the SDK writes, for apps it knows, and never removes the directory itself:logging.directoryis user-controlled and could point anywhere. A file it cannot delete is reported, not thrown — on Windows the Desktop app holds its log open — and the command exits1when any was left behind.File naming
The SDK uses
pino-roll, which numbers every file from1. There is no un-numberedcli.log: files arecli.1.log,cli.2.log, … and the highest number is the one being written. Docs and fixtures reflect that, and the spawn test discovers the file rather than assuming a name.Behaviour at the default level
info, at which a run records every failure and nothing else:-vadds the full request stream and headers,-vvadds bodies.Logs never touch stdout, so raising verbosity cannot disturb a script parsing
--json— covered by a test that runs-vv … --jsonand parses the result. The SDK pins the same invariant on its side, since pino's own default destination is stdout.Docs
All four surfaces CLAUDE.md requires: yargs help in
src/cli.ts, a troubleshooting block inREADME.md, the## Global optionstable plus a## rawback logssection indocs/commands.md, and a## Loggingsection indocs/configuration.mdcovering the file layout, rotation, redaction, env vars and permissions.Verification
bun run checkpasses — typecheck → test → lint → format:check → build. 533 tests (512 pre-existing, all green; 21 new across 3 files), split per CLAUDE.md: spawn-based for help and validation, injected dependencies and temp paths for filesystem behaviour. Re-run against the SDK branch's latest head.Notes for reviewers
bun.lockis deliberately unchanged. My localbun installran under a different Bun than the repo pins and rewrotelockfileVersion2 → 1 plus a transitive bump; committing that would have fought the frozen install. It needs a real regeneration with the SDK bump.--levelfiltering keeps its own small copy of pino's level numbers rather than importing a map the SDK does not export.textblock rather than afieldsrow. The shared fields renderer drops its label padding when a value is too long for the terminal (Directory/tmp/…with no gap), and a log directory is always a long absolute path. That looked like my bug but reproduces on any long value, so I worked around it here rather than changing shared UI in this PR — worth a separate fix.🤖 Generated with Claude Code
https://claude.ai/code/session_01HwoxuU7mv4yFQnecnKoaPY