Skip to content

Add spdlog CLI integration with --log-level flag (issue #184, PR B) - #186

Merged
dnovick merged 3 commits into
mainfrom
claude-logging-pr-b
May 21, 2026
Merged

Add spdlog CLI integration with --log-level flag (issue #184, PR B)#186
dnovick merged 3 commits into
mainfrom
claude-logging-pr-b

Conversation

@dnovick

@dnovick dnovick commented May 21, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds cmake/PermSpdlog.cmake — FetchContent for spdlog v1.15.3 (header-only, no tests/examples/install)
  • Adds cmake/PermNlohmannJson.cmake — FetchContent for nlohmann/json v3.11.3
  • Adds safe-crypto-cli/cli_logging.hpp — maps a level string to both spdlog threshold and CryptoLogLevel; registers a lambda sink that forwards library log events into spdlog's "scli" stderr colour logger; also provides cli_init_logging_from_config() for JSON-driven sink selection
  • Adds --log-level top-level CLI option (env: SCLI_LOG_LEVEL, default: warn) so users can enable diagnostic output without recompiling
  • Adds --log-config <path> top-level CLI option — reads a JSON file to configure sinks and level; wins over --log-level when both are supplied
  • Supported JSON config sinks: stderr (default), stdout, file (rotating, configurable max_size_mb / max_files)
  • Errors in the config file (missing, malformed JSON, unknown sink type) print a diagnostic to stderr and fall back to default warn-level logging so the operation still runs
  • Updates safe-crypto-cli/cli_error.hppdie() routes through spdlog when the logger will emit at error level, otherwise falls back to std::cerr; fatal errors are always visible including at --log-level off
  • Pre-scans argv in main() before CLI11 parsing so the logger is live when subcommand callbacks execute inside CLI11_PARSE

Tests (41 new, all passing)

Library hook (safe-crypto-lib-test/crypto_log_tests.hpp, 13 tests):

  • Default state is off; enabled after sink registered
  • Threshold filtering — messages below threshold not delivered
  • nullptr sink disables logging
  • Sink receives messages with correct level and text
  • msg() helper formats 1/2/3 key-value pairs correctly
  • Integration: sha() emits entry and success debug events; no events when sink is null; error-only threshold suppresses debug from successful operations

CLI --log-level (safe-crypto-cli-test/logging_tests.hpp, 9 tests):

  • Default and explicit warn produce no stderr on success
  • --log-level debug emits entry/success trace lines
  • --log-level=value equals syntax accepted
  • SCLI_LOG_LEVEL env var triggers debug trace
  • SCLI_LOG_LEVEL=off silences operational output
  • --log-level off still emits fatal error messages
  • --log-level warn emits fatal error exactly once (no duplicate)
  • Unknown level falls back to warn

JSON config (safe-crypto-cli-test/log_config_tests.hpp, 9 tests):

  • stderr sink with debug level emits trace
  • stderr sink with warn level is silent on success
  • --log-config wins over --log-level
  • Missing sinks key defaults to stderr
  • file sink writes log entries to the file
  • file sink without path produces an error
  • Missing config file falls back to default logging with error message
  • Invalid JSON falls back to default logging with error message
  • Unknown sink type falls back to default logging with error message

Depends on

PR #185 (pluggable crypto_log hook) — already merged to main.

JSON config example

{
  "level": "debug",
  "sinks": [
    { "type": "stderr" },
    { "type": "file", "path": "/var/log/scli.log", "max_size_mb": 10, "max_files": 3 }
  ]
}
scli --log-config /etc/scli/logging.json digest --algo sha256 --input -
scli --log-level debug digest --algo sha256 --input -
SCLI_LOG_LEVEL=info scli random --length 32

🤖 Generated with Claude Code

Wires the crypto_log hook to spdlog's stderr colour sink via a new
--log-level option (env: SCLI_LOG_LEVEL, default: warn).  Early argv
pre-scan ensures the logger is live before CLI11 subcommand callbacks
run, so debug/info output is correctly emitted during operations.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dnovick

dnovick commented May 21, 2026

Copy link
Copy Markdown
Owner Author

Blocking finding on latest head f6e114b:

--log-level off currently suppresses fatal CLI error messages. cli_init_logging("off") still creates/registers the scli logger and sets its level to off; then die() sees that logger and calls logger->error(...), but spdlog drops the message because the logger is off. The fallback std::cerr << "Error: ..." path is never used.

Repro:

  • /private/tmp/claude-crypto-pr186-build/safe-crypto-cli/scli --log-level off digest --algo nope --input base64:aGVsbG8= exits 1 with empty stderr.
  • SCLI_LOG_LEVEL=off /private/tmp/claude-crypto-pr186-build/safe-crypto-cli/scli digest --algo nope --input base64:aGVsbG8= also exits 1 with empty stderr.
  • The same command with --log-level warn prints the expected error.

The fix should keep operational/library logging disabled at off while still guaranteeing fatal CLI diagnostics are emitted, either by having die() always write to stderr independently of the filtered logger, or by bypassing the logger level for fatal errors.

Other verification so far:

  • PSA_MBEDTLS Debug configure/build succeeded.
  • Built scli and safe_crypto_cli_test.
  • Targeted CLI tests passed: ctest --test-dir /private/tmp/claude-crypto-pr186-build --output-on-failure -R 'safe_crypto_cli_test|DigestTests|HelpTests|RandomTests|KdfTests|IoTests' => 65/65 passed.

Not ready to merge until the silent-error path is fixed.

When --log-level off was set, spdlog dropped the error() call and the
fallback std::cerr path was never reached, producing a silent exit 1.
Fatal CLI errors must always be visible, so write to stderr directly
first then also forward to spdlog (for formatted/sink output) when the
logger exists.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dnovick

dnovick commented May 21, 2026

Copy link
Copy Markdown
Owner Author

Re-reviewed latest head db4efc3.

The previous silent-error blocker is fixed for --log-level off: invalid commands now emit a visible Error: ... line and exit nonzero.

There is still a user-visible blocker before merge: fatal errors are duplicated at the default/warn logging level because die() now always writes directly to std::cerr and then also calls logger->error(...) when the scli logger exists.

Repro:

  • /private/tmp/claude-crypto-pr186-build/safe-crypto-cli/scli digest --algo nope --input base64:aGVsbG8= exits 1 and writes two stderr lines:
    • Error: unknown --algo ...
    • [timestamp] [scli] [error] unknown --algo ...
  • /private/tmp/claude-crypto-pr186-build/safe-crypto-cli/scli --log-level warn digest --algo nope --input base64:aGVsbG8= does the same.
  • /private/tmp/claude-crypto-pr186-build/safe-crypto-cli/scli --log-level off digest --algo nope --input base64:aGVsbG8= writes one visible stderr line, which is the desired non-silent behavior.

Suggested shape: emit through spdlog when the logger will actually emit the error, otherwise fall back to std::cerr; or otherwise avoid sending the same fatal diagnostic to the same stderr sink twice. --log-level off should still get the direct stderr fallback.

Verification run on latest head:

  • Rebuilt scli and safe_crypto_cli_test in /private/tmp/claude-crypto-pr186-build.
  • Targeted CLI suite passed: ctest --test-dir /private/tmp/claude-crypto-pr186-build --output-on-failure -R 'safe_crypto_cli_test|DigestTests|HelpTests|RandomTests|KdfTests|IoTests' => 65/65 passed.

CI is still pending on several jobs, so I did not merge.

The previous fix wrote to std::cerr unconditionally and then also called
logger->error(), producing two lines at warn/default level.  Use
logger->should_log(spdlog::level::err) to decide the path: when spdlog
will emit the message, route through it only; when the logger is off or
absent, fall back to std::cerr.  Preserves visibility at --log-level off
without duplicating output at normal levels.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dnovick

dnovick commented May 21, 2026

Copy link
Copy Markdown
Owner Author

Re-reviewed latest head 35c99b1. The duplicate fatal-error output blocker is fixed: default/warn/debug now emit one spdlog error line, and --log-level off plus SCLI_LOG_LEVEL=off emit one direct Error: line.

Verification:

  • Rebuilt scli and safe_crypto_cli_test in /private/tmp/claude-crypto-pr186-build.
  • Targeted CLI suite passed: ctest --test-dir /private/tmp/claude-crypto-pr186-build --output-on-failure -R 'safe_crypto_cli_test|DigestTests|HelpTests|RandomTests|KdfTests|IoTests' => 65/65 passed.

I do not see a remaining code blocker from this review. GitHub would not allow me to approve because this account owns the PR. CI still has pending jobs, so I did not merge.

@dnovick
dnovick merged commit 9096ca1 into main May 21, 2026
11 checks passed
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