Skip to content

feat(logging): unify CCS runtime logs and dashboard viewer#927

Merged
kaitranntt merged 8 commits intodevfrom
kai/feat/926-ccs-logging-unification
Apr 8, 2026
Merged

feat(logging): unify CCS runtime logs and dashboard viewer#927
kaitranntt merged 8 commits intodevfrom
kai/feat/926-ccs-logging-unification

Conversation

@kaitranntt
Copy link
Copy Markdown
Owner

Summary

  • add a shared CCS-owned structured logging layer with bounded retention and redaction
  • expose native /api/logs endpoints plus a dedicated System -> Logs dashboard route
  • keep legacy CLIProxy error diagnostics visible on Home while moving the main logs UX into the new workspace
  • update repo-local docs for the new logging architecture and workflow

Validation

  • bun run validate
  • bun run build
  • cd ui && bun run validate
  • cd ui && bun run build

Notes

  • merged the latest origin/dev into this branch before committing and pushing
  • UI build still reports the existing Vite chunk-size warning, but the build passes

Closes #926

@ccs-reviewer
Copy link
Copy Markdown

ccs-reviewer bot commented Apr 8, 2026

📋 Summary

This PR adds a structured logging layer, log storage with rotation and redaction, /api/logs dashboard endpoints, and a /logs dashboard page. The backend is solid but the UI page imports six components from ui/src/components/logs/ that do not exist in the PR. The UI build will fail at module resolution and the UI test will also fail. This is a blocking omission.

🔍 Findings

🔴 High

  • ui/src/pages/logs.tsx:5 — Missing UI component files break the build
    Problem: LogsPage imports six components from @/components/logs/ (logs-config-card, logs-detail-panel, logs-entry-list, logs-filters, logs-overview-cards, logs-page-skeleton) but the directory ui/src/components/logs/ does not exist in the PR.
    Why it matters: The UI build cannot resolve these imports and will fail. The UI test at ui/tests/unit/ui/pages/logs-page.test.tsx also imports LogsPage and will fail for the same reason.
    Suggested fix: Add the missing ui/src/components/logs/ directory with all six component files, or remove the logs page and route until the components are ready.

🟡 Medium

  • src/services/logging/log-reader.ts:19 — readCurrentFileEntries reads entire log file into memory on every dashboard poll
    Problem: readCurrentFileEntries uses fs.readFileSync to load the entire current.jsonl into memory. The dashboard /api/logs/entries endpoint polls every 10 seconds.
    Why it matters: As the file approaches the 10 MB rotation threshold, each poll allocates up to 10 MB plus temporary strings from split/filter/map, creating repeated memory spikes and response latency on every dashboard refresh cycle.
    Suggested fix: Use the in-memory buffer as the primary source for recent entries and only fall back to the file when the buffer is empty or older entries are needed. Alternatively, stream or tail-read only the last N lines.

🟢 Low

  • src/commands/cleanup-command.ts:339 — Cleanup confirmation shows size that includes files it will not delete
    Problem: getDirSize is recursive and includes the archive/ subdirectory, while countFiles and cleanDirectory are non-recursive and only touch top-level files.
    Why it matters: The pre-deletion confirmation displays a total size that includes archived files, but cleanDirectory only deletes top-level files. Users will see a larger size than what is actually freed.
    Suggested fix: Make getDirSize non-recursive to match cleanDirectory scope, or make cleanDirectory recursive so the displayed and actual totals agree.

🔒 Security Checklist

Check Status Notes
Log files written with restrictive file permissions appendFileSync and writeFileSync use mode 0o600
Sensitive values redacted before disk persistence log-redaction.ts covers token, api_key, password, cookie, authorization, secret keys with depth limit and truncation
Dashboard log routes enforce access control requireLocalAccessWhenAuthDisabled gates all /api/logs routes with localhost check when auth is disabled
Request logging avoids recursive self-logging request-logging-middleware.ts skips all /api/logs paths
Path traversal protection on log file operations isPathInsideDirectory guards against directory escape

📊 CCS Compliance

Rule Status Notes
CCS paths must use getCcsDir() All log paths in log-paths.ts derive from getCcsDir(); no os.homedir() usage in the logging module
CLI output must stay ASCII-only No emoji or non-ASCII output added in CLI-facing code
CLI behavior changes require --help update cleanup-command.ts printHelp updated to reflect CCS + CLIProxy scope
Dashboard parity for configuration features Logging config is editable via dashboard /logs page and persisted to config.yaml

💡 Informational

  • error-handler.ts passes the raw error object to logger context (line 99). Redaction covers sensitive keys but nested data under non-sensitive keys could persist. Low risk since data is local-only.
  • tool-sanitization-proxy writeLog uses the level string as the event identifier (line 213), producing entries with event='info' rather than a descriptive event name like 'sanitization.warning'.

✅ What's Done Well

  • The redaction layer is thorough: sensitive key regex covers common patterns, values are truncated at 2000 chars, and object depth is capped at 5 levels.
  • The mtime-based config cache in log-config.ts avoids re-reading config.yaml on every log write while picking up edits within 1 second.
  • Request-logging middleware explicitly skips /api/logs paths to prevent the log viewer from flooding its own logs during polling.

🎯 Overall Assessment

❌ CHANGES REQUESTED — The six UI component files imported by logs.tsx are missing from the PR. The UI build cannot succeed without them. This must be resolved before merge.

🤖 Reviewed by glm-5.1

@ccs-reviewer
Copy link
Copy Markdown

ccs-reviewer bot commented Apr 8, 2026

📋 Summary

Adds a structured logging subsystem: src/services/logging/ backend with JSONL persistence, rotation/retention, redaction, and in-memory buffer; /api/logs dashboard endpoints with localhost fallback; and a dedicated System -> Logs dashboard route. Legacy CLIProxy error diagnostics preserved as a secondary tab. Two findings: stale root help catalog summary for ccs cleanup, and missing tests for the security-sensitive redaction module.

🔍 Findings

🟡 Medium

  • src/commands/command-catalog.ts:140 — Stale cleanup summary in root help catalog
    Problem: The catalog entry for cleanup still reads 'Remove old CLIProxy logs' even though the command now also removes CCS logs.
    Why it matters: Running ccs --help shows an outdated description. The detailed ccs cleanup --help was updated but the root help catalog was missed. CLAUDE.md requires all CLI changes to update the respective help handler.
    Suggested fix: Update line 140 to: summary: 'Remove old CCS and CLIProxy logs'

  • src/services/logging/log-redaction.ts:1 — No unit tests for the log-redaction module
    Problem: The redaction module has zero dedicated test coverage. It is the only barrier preventing sensitive values from being persisted to disk.
    Why it matters: Redaction is security-sensitive. A bug in the regex pattern or recursion could silently leak tokens, passwords, or API keys into ~/.ccs/logs/current.jsonl. The existing routes tests do not assert that specific sensitive keys are stripped from persisted output.
    Suggested fix: Add a unit test covering: sensitive-key patterns produce '[redacted]', non-matching keys pass through, nested objects recurse, arrays are sanitized, MAX_DEPTH produces '[max-depth]', long strings are truncated, Error instances reduce to name/message, null/undefined preserved.

🟢 Low

  • src/services/logging/log-reader.ts:19 — Full file read and parse on every /api/logs/entries request
    Problem: readCurrentFileEntries reads the entire current.jsonl into memory, splits by newline, and JSON-parses every line on each API call. The dashboard polls every 10 seconds.
    Why it matters: As the log file approaches the 10 MB rotation threshold, each poll reads and parses thousands of JSON lines plus dedup and sort, adding unnecessary CPU and GC pressure for a read-heavy polling pattern.
    Suggested fix: Consider caching parsed entries between polls, reading only the file tail when the file has grown, or maintaining an incrementally updated parsed index.

🔒 Security Checklist

Check Status Notes
Log files use restrictive file permissions appendFileSync and writeFileSync both specify mode 0o600
Sensitive data redaction before persistence Redaction on by default with key-pattern matcher, depth limiting, and string truncation.
API logs endpoints enforce access control requireLocalAccessWhenAuthDisabled + authMiddleware provide defense in depth.
Log viewer does not recursively log itself requestLoggingMiddleware skips /api/logs paths.
No path traversal in log read/write paths All paths derived from getCcsDir() with fixed subdirectories.

📊 CCS Compliance

Rule Status Notes
CLI output stays ASCII-only No emoji additions. Console output uses [OK], [!], [X], [i] markers.
Path access uses getCcsDir() log-paths.ts uses getCcsDir() for all path roots. No os.homedir() + '.ccs' concatenation.
CLI help text updated for behavior changes ⚠️ ccs cleanup printHelp() updated but command-catalog.ts root summary was not. Root ccs --help shows stale text.
Dashboard parity for new feature Logging config editable from CLI (config.yaml) and dashboard. Entries viewable in dashboard.
Tests use CCS_HOME for isolation Both test files set process.env.CCS_HOME to a temp dir and restore it in afterEach.

💡 Informational

  • The module-level JSDoc in cleanup-command.ts (line 5) still says 'Removes old CLIProxy logs'. Low priority developer-facing comment.
  • The cleanup command's cleanDirectory only deletes top-level regular files. The archive/ subdirectory is untouched; archive cleanup is handled by automatic retention pruning.
  • tool-sanitization-proxy uses the log level string as both the Logger method name and the event name. Correct but redundant naming.

✅ What's Done Well

  • Redaction module covers key-pattern matching, depth limiting, string truncation, Error handling, and array recursion.
  • Defense-in-depth on API routes: requestLoggingMiddleware + requireLocalAccessWhenAuthDisabled + authMiddleware.
  • Request-logging middleware explicitly skips /api/logs reads to prevent recursive log generation from the polling dashboard.

🎯 Overall Assessment

⚠️ APPROVED WITH NOTES — Core logging subsystem is well-designed with proper redaction, file permissions, access control, and recursive-log prevention. Two medium findings: stale root help catalog entry and missing tests for the security-critical redaction module. Neither blocks merge but both should be tracked for prompt follow-up.

🤖 Reviewed by glm-5.1

- add roadmap note for the new CCS logging surface

- record logging architecture and codebase ownership

- add logging standards for bounded, redacted runtime logs

Refs #926
- add a shared structured logging service with bounded retention

- expose native /api/logs endpoints and the dashboard /logs route

- wire focused runtime emitters, cleanup support, and feature tests

Refs #926
- unignore the new ui logs component directory

- add the missing logs viewer components required by ui build

Refs #926
- reshape the page into a management-center style control surface

- add activity and CLIProxy error tabs with denser list/detail ergonomics

- switch filters to practical chip groups and update the page tests

Refs #926
@kaitranntt kaitranntt force-pushed the kai/feat/926-ccs-logging-unification branch from a912d2c to 6e39f33 Compare April 8, 2026 19:59
@ccs-reviewer
Copy link
Copy Markdown

ccs-reviewer bot commented Apr 8, 2026

📋 Summary

This PR adds a CCS-owned structured logging layer with JSONL persistence, bounded retention, and redaction; exposes /api/logs endpoints; and adds a System - Logs dashboard route. All CCS path access correctly uses getCcsDir(). Log files use mode 0o600. Redaction covers sensitive keys with depth and length caps. API endpoints are gated by requireLocalAccessWhenAuthDisabled. The cleanup command was extended to cover the new CCS logs directory. No blocking issues found.

🔍 Findings

🟡 Medium

  • src/commands/cleanup-command.ts:36 — ccs cleanup skips archive/ subdirectory silently
    Problem: getDirSize, countFiles, and cleanDirectory were changed from recursive to top-level-only. The CCS logs archive/ subdirectory (rotated .jsonl.gz files) is invisible to the cleanup command.
    Why it matters: Users running ccs cleanup --dry-run see only current.jsonl size. The archive directory could hold the bulk of historical data. Help text says 'Remove old CCS and CLIProxy logs' but archives are excluded without mention.
    Suggested fix: Either add the archive directory as a separate cleanup target with its own line in the output, or document in the help text and dry-run output that archives are managed by retention rather than manual cleanup.

🟢 Low

  • src/services/logging/log-storage.ts:1 — Rotation and pruning logic lack direct unit tests
    Problem: rotateCurrentLogIfNeeded and pruneExpiredLogArchives are critical for bounding disk usage but have no dedicated test coverage.
    Why it matters: Rotation reads the full file, gzips, and replaces with a truncated file. Pruning deletes expired archives. Bugs here could cause silent disk growth.
    Suggested fix: Add targeted unit tests for rotation-triggering and for pruning of expired archives. Test with CCS_HOME temp isolation matching the existing test patterns.

🔒 Security Checklist

Check Status Notes
Log file permissions restrict to owner-only appendFileSync and writeFileSync use mode 0o600
Sensitive data redaction before persistence redactContext covers token, api_key, password, etc. with depth/length caps; default redact: true
Logs API endpoints enforce access control requireLocalAccessWhenAuthDisabled gates all /api/logs; auth middleware at app level when dashboard auth enabled
No path traversal in log operations Paths derive from getCcsDir(); isPathInsideDirectory validates; archive names from ISO timestamps
Logging cannot crash the runtime appendStructuredLogEntry wraps all I/O in try-catch; logger calls never throw

📊 CCS Compliance

Rule Status Notes
CCS path access uses getCcsDir() log-paths.ts uses getCcsDir(); no os.homedir()+'.ccs' anywhere
CLI output stays ASCII-only Cleanup help uses [OK], [!], [X], [i]; no emojis in terminal output
Help text updated for CLI changes printHelp() and command-catalog summary updated for cleanup command
Dashboard parity for new config Logging config editable via /api/logs/config PUT and LogsConfigCard component

💡 Informational

  • Module-level createLogger calls create write closures eagerly but perform no I/O until a log method is called, so import-time side effects are safe.
  • request-logging-middleware skips /api/logs URLs to prevent recursive log entries. Future routes under /api/logs needing audit logging will need a separate path.

✅ What's Done Well

  • Redaction layer provides defense in depth: regex key matching, recursive depth cap, string truncation, and Error sanitization.
  • Config caching with mtime-based revalidation avoids stale reads without filesystem hits on every log call.
  • Test isolation correctly uses CCS_HOME temp directories in beforeEach/afterEach across all new test files.

🎯 Overall Assessment

⚠️ APPROVED WITH NOTES — No blocking issues. Logging layer is well-structured with proper security controls. Path access uses getCcsDir() throughout. The two findings are non-blocking: cleanup archive visibility is a UX gap, and missing rotation/pruning tests are a follow-up quality item.

🤖 Reviewed by glm-5.1

@ccs-reviewer
Copy link
Copy Markdown

ccs-reviewer bot commented Apr 8, 2026

📋 Summary

This PR adds a CCS-owned structured logging layer with bounded retention, redaction, JSONL storage under ~/.ccs/logs/, /api/logs dashboard endpoints, and a System -> Logs workspace. Paths use getCcsDir(), files use 0o600, sensitive values are redacted, request logging skips /api/logs to prevent recursion, and tests isolate via CCS_HOME. One confirmed issue: log directories are created without restrictive permissions, inconsistent with the config directory pattern in unified-config-loader.ts.

🔍 Findings

🟡 Medium

  • src/services/logging/log-paths.ts:26 — Log directories created without restrictive permissions
    Problem: ensureLoggingDirectories() calls fs.mkdirSync with { recursive: true } but does not set mode: 0o700. The directories inherit the process umask (typically 0755), making them world-readable on multi-user systems.
    Why it matters: The codebase enforces mode: 0o700 for config directories (unified-config-loader.ts lines 91, 992). Log files contain runtime diagnostics with potentially partially-redacted context and should be equally protected.
    Suggested fix: Add mode: 0o700 to both mkdirSync calls in ensureLoggingDirectories(), matching the pattern in unified-config-loader.ts lines 91 and 992.

🔒 Security Checklist

Check Status Notes
Path access uses getCcsDir() All log paths resolved through getCcsDir() via log-paths.ts
Sensitive data redaction Redacts authorization, cookie, password, token, api_key with depth limiting
Log file permissions appendFileSync and writeFileSync use mode: 0o600
Log directory permissions ⚠️ ensureLoggingDirectories() omits mode: 0o700; directories inherit umask
API auth / localhost guard logs-routes.ts applies requireLocalAccessWhenAuthDisabled

📊 CCS Compliance

Rule Status Notes
getCcsDir() for all path access log-paths.ts routes through getCcsDir(); no os.homedir() usage
ASCII-only CLI output Logging writes JSONL, not terminal output; no emojis in CLI paths
Dashboard parity Config and entries fully accessible via dashboard /logs and /api/logs
Documentation updates code-standards, codebase-summary, system-architecture, roadmap updated

💡 Informational

  • Logging defaults to enabled=true. Users get structured logs on upgrade. Bounded by 7-day retention and 10 MB rotation, but users who opt out should set logging.enabled: false.
  • appendStructuredLogEntry uses synchronous fs calls on every write. Under high-frequency logging this adds event-loop latency but is consistent with existing codebase patterns.

✅ What's Done Well

  • Redaction is thorough: depth-limited recursion, string truncation, Error sanitization, comprehensive sensitive-key regex.
  • Request logging middleware skips /api/logs to prevent recursive log generation.
  • Tests use CCS_HOME isolation and verify cache-refresh, dedup, and remote-access blocking.

🎯 Overall Assessment

⚠️ APPROVED WITH NOTES — One medium finding on directory permissions is a straightforward fix. The logging layer is otherwise well-structured with proper redaction, bounded retention, correct file permissions, auth guards, and tests.

🤖 Reviewed by glm-5.1

@ccs-reviewer
Copy link
Copy Markdown

ccs-reviewer bot commented Apr 8, 2026

📋 Summary

This PR adds a structured logging layer backed by JSONL files under getCcsDir()/logs, with bounded rotation/retention, secret redaction, /api/logs dashboard endpoints, and a System -> Logs dashboard page. The backend and tests are solid: paths use getCcsDir(), files get 0o600/0o700 permissions, redaction is on by default, the middleware skips /api/logs to avoid recursion, and log endpoints enforce localhost access. One confirmed issue: nav.logs is missing from the Chinese, Vietnamese, and Japanese i18n nav sections, so non-English users see a fallback label.

🔍 Findings

🟡 Medium

  • ui/src/lib/i18n.ts:36 — Missing nav.logs i18n key in non-English translations
    Problem: The logs key was added to the English nav section but is absent from the Chinese, Vietnamese, and Japanese nav objects. The sidebar uses t('nav.logs'), so non-English users see a fallback or raw key.
    Why it matters: Every new sidebar entry needs translations in all supported locales. The other nav keys (health, settings, accounts) are present in all four language sections.
    Suggested fix: Add logs entries to zh, vi, and ja nav sections (e.g., zh: logs: '日志', vi: logs: 'Nhật ký', ja: logs: 'ログ').

🔒 Security Checklist

Check Status Notes
Log files created with restrictive permissions Files use 0o600, directories use 0o700.
Secret redaction before persistence Redaction on by default. Covers authorization, cookie, password, token, api_key, management_key.
Dashboard log endpoints enforce access control All /api/logs routes use requireLocalAccessWhenAuthDisabled. PUT also gated by mutation middleware.
Request logging avoids recursive self-logging Middleware skips logging when URL starts with /api/logs.
No path traversal in log reads N/A Paths derived from getCcsDir() constants, not user-supplied input.

📊 CCS Compliance

Rule Status Notes
All CCS path access uses getCcsDir() log-paths.ts uses getCcsDir() for all path resolution. Tests set CCS_HOME for isolation.
CLI output stays ASCII-only Logging writes JSON to files, not terminal. Cleanup help uses ASCII markers.
--help updated for CLI behavior changes cleanup printHelp() and command-catalog summary updated. Test verifies new summary.
Dashboard parity for new logging feature Config editable from CLI (config.yaml) and dashboard. /logs page provides full parity.

💡 Informational

  • Logging is enabled by default (enabled: true). Deliberate choice with bounded retention (7 days, 10 MB) and auto-redaction. Set logging.enabled: false to opt out.
  • Home page logs card strings are hardcoded English, not wrapped in t(). Matches existing pattern but could be addressed in a follow-up i18n pass.

✅ What's Done Well

  • Bounded by design: rotation by size and age, automatic pruning, configurable retention, bounded ring buffer prevent unbounded growth.
  • Defensive error handling: appendStructuredLogEntry wraps all I/O in catch so logging failures never break CLI or server.
  • Comprehensive test coverage: rotation, pruning, cache invalidation, redaction edge cases, route auth, and UI rendering.

🎯 Overall Assessment

⚠️ APPROVED WITH NOTES — One non-blocking i18n gap (missing nav.logs key in three languages). Backend, security, and test coverage are solid. No blocking issues found.

🤖 Reviewed by glm-5.1

@kaitranntt kaitranntt merged commit cb0aac2 into dev Apr 8, 2026
4 checks passed
@kaitranntt kaitranntt deleted the kai/feat/926-ccs-logging-unification branch April 8, 2026 21:01
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