feat(logging): rotate embedded core + shell logs to ~/.openhuman/logs#1278
Merged
senamakel merged 4 commits intotinyhumansai:mainfrom May 6, 2026
Merged
Conversation
The Tauri shell previously initialized `env_logger` to stderr and the
embedded core never installed a tracing subscriber at all (only the CLI
path called `init_for_cli_run`). Packaged GUI builds therefore had no
visible diagnostics when users hit issues like "stuck on Initializing
OpenHuman..." — there was nothing for support to ask them to share.
Add `core::logging::init_for_embedded(data_dir, verbose)` which:
* installs the existing `CleanCliFormat` against both stderr (ANSI when
attached to a TTY) and a non-blocking, daily-rotated file appender at
`<data_dir>/logs/openhuman-YYYY-MM-DD.log` (7-day retention),
* keeps the Sentry tracing layer + `EnvFilter` honoring `RUST_LOG`,
* bridges the shell's `log::*` calls via `tracing_log::LogTracer` so
every shell + core event lands in one file.
Wire it from `app/src-tauri/src/file_logging.rs` (called early in
`run()`, before CEF preflight) and surface the folder in Settings →
Developer Options via two new commands `logs_folder_path` /
`reveal_logs_folder` so users can grab today's log and send it.
Refactor `init_for_cli_run` to share `seed_rust_log`,
`build_env_filter`, and `sentry_tracing_layer` with the embedded path —
single source of truth for the formatter + filter.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds daily-rotated file logging for embedded (Tauri) and CLI contexts, exposes the active log directory and a reveal action to the Tauri frontend, initializes the logger early in startup, updates Tauri permissions for those commands, and adds a Developer Options UI row to view/open the logs folder. (34 words) ChangesFile-Based Logging with Tauri Integration
Sequence DiagramsequenceDiagram
participant Frontend as Developer Panel
participant Tauri as Tauri Shell
participant Core as Embedded Core
participant FS as File System
rect rgba(100, 200, 100, 0.5)
note over Frontend,FS: Fetch Logs Folder Path
Frontend->>Tauri: invoke("logs_folder_path")
Tauri->>Core: core::log_directory() query
Core->>FS: resolve active logs directory
FS-->>Core: path
Core-->>Tauri: path string
Tauri-->>Frontend: path
end
rect rgba(100, 150, 200, 0.5)
note over Frontend,FS: Open Logs Folder
Frontend->>Tauri: invoke("reveal_logs_folder")
Tauri->>FS: spawn platform command (open/explorer/xdg-open)
FS-->>Tauri: success/failure
Tauri-->>Frontend: result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/core/logging.rs (1)
241-241: 💤 Low valueMinor: duplicate constraint parsing.
parse_log_file_constraints()is called twice (lines 241 and 264), each creating a separateVec<String>. Since this runs once at startup, the overhead is negligible. If you want to tighten it, parse once and clone for the second filter closure.♻️ Optional: parse constraints once
appender.map(|appender| { let (writer, guard) = tracing_appender::non_blocking(appender); let _ = FILE_GUARD.set(guard); let _ = LOG_DIR.set(logs_dir.clone()); - let constraints = parse_log_file_constraints(); + let file_constraints = constraints.clone(); tracing_subscriber::fmt::layer() .with_ansi(false) .event_format(CleanCliFormat) .with_writer(writer) .with_filter(tracing_subscriber::filter::filter_fn(move |meta| { - event_matches_file_constraints(meta, &constraints) + event_matches_file_constraints(meta, &file_constraints) })) }) } // ... earlier in the function, before the match: + let constraints = parse_log_file_constraints();Also applies to: 264-264
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/logging.rs` at line 241, parse_log_file_constraints() is being called twice producing two Vec<String>; call it once, store the result in a local variable (e.g. let constraints = parse_log_file_constraints()), then reuse it by cloning when needed for the second filter/closure (e.g. let constraints_for_other = constraints.clone()) so both closures use the same parsed data; update the two places that currently call parse_log_file_constraints() to use the stored variable and its clone instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src-tauri/src/file_logging.rs`:
- Around line 39-41: The fallback to PathBuf::from(".openhuman") in the call to
default_root_openhuman_dir() can put logs in a relative, unpredictable location;
change the fallback to a robust absolute location (for example use
std::env::temp_dir().join("openhuman") or another absolute directory) and emit a
warning when default_root_openhuman_dir() fails so the user/maintainer is aware;
update the call site that uses default_root_openhuman_dir() and the surrounding
error handling to construct an absolute fallback path and call the logger (or
e.g., warn!()) with context including that the home-dir lookup failed and which
fallback was chosen.
---
Nitpick comments:
In `@src/core/logging.rs`:
- Line 241: parse_log_file_constraints() is being called twice producing two
Vec<String>; call it once, store the result in a local variable (e.g. let
constraints = parse_log_file_constraints()), then reuse it by cloning when
needed for the second filter/closure (e.g. let constraints_for_other =
constraints.clone()) so both closures use the same parsed data; update the two
places that currently call parse_log_file_constraints() to use the stored
variable and its clone instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f71d657a-3409-4e6a-b44c-77d3c5ee323e
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.tomlapp/src-tauri/permissions/allow-core-process.tomlapp/src-tauri/src/file_logging.rsapp/src-tauri/src/lib.rsapp/src/components/settings/panels/DeveloperOptionsPanel.tsxsrc/core/logging.rs
Adds unit tests for diff-coverage:
* core/logging.rs — `seed_rust_log` defaults (info / debug / autocomplete
scoping), respect for an existing `RUST_LOG`, `level_tag`,
`short_target`, `parse_log_file_constraints`, smoke `build_env_filter`,
and `log_directory()`-pre-init.
* app/src-tauri file_logging.rs — `resolve_data_dir` honors
`OPENHUMAN_WORKSPACE`, falls through on empty value, and the new
`temp_dir()` fallback (replacing the relative `.openhuman` path
flagged by CodeRabbit). Also asserts `logs_folder_path` /
`reveal_logs_folder` behave correctly pre-init.
Env-mutating tests share a `Mutex` so the parallel test runner can't race
on `RUST_LOG` / `OPENHUMAN_WORKSPACE` / `OPENHUMAN_LOG_FILE_CONSTRAINTS`.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src-tauri/src/file_logging.rs (1)
124-139: ⚡ Quick winAdd backend diagnostics around the folder-reveal command.
This new Tauri command launches an external process and returns user-visible errors, but the shell leaves no trace of which directory it tried to open or why it failed. A small
[file_logging]debug/warn pair here would make support reports much easier to correlate.Suggested fix
pub fn reveal_logs_folder() -> Result<(), String> { let dir = log_directory().ok_or_else(|| "log directory not initialized".to_string())?; + log::debug!("[file_logging] reveal_logs_folder dir={}", dir.display()); #[cfg(target_os = "macos")] let result = std::process::Command::new("open").arg(dir).spawn(); #[cfg(target_os = "windows")] @@ - result - .map(|_| ()) - .map_err(|e| format!("failed to open log directory {}: {e}", dir.display())) + result.map(|_| ()).map_err(|e| { + log::warn!( + "[file_logging] reveal_logs_folder failed dir={} err={e}", + dir.display() + ); + format!("failed to open log directory {}: {e}", dir.display()) + }) }As per coding guidelines, "Default to verbose diagnostics on new/changed flows with entry/exit logging, branches, external calls, retries/timeouts, state transitions, and errors; use stable grep-friendly prefixes and correlation fields."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src-tauri/src/file_logging.rs` around lines 124 - 139, Add structured backend diagnostics around reveal_logs_folder: before invoking the external command in reveal_logs_folder (after resolving dir via log_directory()) emit a debug log (e.g. "[file_logging] reveal_logs_folder: opening directory {dir}") that records the exact dir and platform command being executed, and on failure emit a warn/error log (e.g. "[file_logging] reveal_logs_folder failed to open {dir}: {error}") inside the error mapping branch so the spawned command error and the attempted directory are captured; also emit a debug/info success log when spawn succeeds. Use the existing log/tracing macros consistent with the crate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/logging.rs`:
- Around line 237-240: Currently FILE_GUARD and LOG_DIR are being set before the
subscriber is installed (before try_init()), so if try_init() fails the globals
are left set even though no file layer exists; change the code in the appender
handling (the block that creates tracing_appender::non_blocking and the later
similar block around lines 272-279) to keep the (writer, guard) and the computed
logs_dir in local variables and only call FILE_GUARD.set(guard) and
LOG_DIR.set(logs_dir.clone()) after try_init() returns Ok (or, alternatively,
propagate the init error and do not set LOG_DIR/FILE_GUARD on Err); also ensure
logs_folder_path() and reveal_logs_folder() read the globals only after
successful init.
- Around line 227-249: The file-appender creation currently swallows errors via
build(&logs_dir).ok(), so when
tracing_appender::rolling::Builder::new()...build(&logs_dir).ok() fails you lose
diagnostic info and the code silently falls back to stderr; change the match arm
where you call build(&logs_dir) to handle Result explicitly (e.g., match
build(&logs_dir) { Ok(appender) => { ... same path that calls
tracing_appender::non_blocking, sets FILE_GUARD and LOG_DIR, parses constraints,
returns the layer }, Err(e) => { emit a startup diagnostic log/error (include
the error e and stable/grep-friendly prefix), and continue without file_layer }
}), and ensure the error message uses the same style as the create_dir_all
diagnostic so failures to create the file appender are visible.
---
Nitpick comments:
In `@app/src-tauri/src/file_logging.rs`:
- Around line 124-139: Add structured backend diagnostics around
reveal_logs_folder: before invoking the external command in reveal_logs_folder
(after resolving dir via log_directory()) emit a debug log (e.g. "[file_logging]
reveal_logs_folder: opening directory {dir}") that records the exact dir and
platform command being executed, and on failure emit a warn/error log (e.g.
"[file_logging] reveal_logs_folder failed to open {dir}: {error}") inside the
error mapping branch so the spawned command error and the attempted directory
are captured; also emit a debug/info success log when spawn succeeds. Use the
existing log/tracing macros consistent with the crate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd9b2a7e-e52a-45ee-8c44-be66e7c348a7
📒 Files selected for processing (2)
app/src-tauri/src/file_logging.rssrc/core/logging.rs
…ucceeds
Two CodeRabbit findings on the embedded init path:
1. `build(&logs_dir).ok()` silently dropped the file-appender error so a
packaged build could degrade to stderr-only with no clue why
`logs/openhuman-YYYY-MM-DD.log` never appears. Now matches on the
`Result` and emits an `[logging]`-prefixed `eprintln!` on failure,
matching the existing `create_dir_all` diagnostic style.
2. `FILE_GUARD` and `LOG_DIR` were set inside the layer-builder closure,
i.e. before `try_init()` could fail. If a competing global subscriber
was already installed `try_init` returns `Err`, no file layer ends up
attached, but `log_directory()` would still report a path — leading
`reveal_logs_folder` / the UI to point at an empty directory. Hold
the writer + guard + path in a local `Option` and commit them only
after `try_init().is_ok()`. On failure log the init error and let
the guard drop so the background flushing thread shuts down cleanly.
Diff-cover gate flagged DeveloperOptionsPanel.tsx at 38% on changed lines —
all of the missing lines were the new LogsFolderRow's `useEffect` /
`onClick` / non-Tauri short-circuit / live-region branches.
Adds five Vitest cases against the existing test file:
* isTauri() === false → row is hidden
* mounting calls `invoke('logs_folder_path')` and renders the path
* clicking "Open logs folder" calls `invoke('reveal_logs_folder')`
* reveal rejection surfaces in the role="status" live region
* path-resolve rejection surfaces in the same live region
Also updates the existing Sentry-row beforeEach to reset and stub the
`@tauri-apps/api/core` mocks (the panel now always renders LogsFolderRow,
which fires an `invoke` on mount — without a stub the unhandled rejection
crashes the Sentry suite).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
<data_dir>/logs/openhuman-YYYY-MM-DD.log(7-day retention) viatracing-appender, with the sameCleanCliFormatused byopenhuman run.log::*calls are bridged into the same subscriber viatracing_log::LogTracer— replaces the prior stderr-onlyenv_logger. Sentry tracing layer +RUST_LOGenv filter preserved.logs_folder_path/reveal_logs_folderTauri commands and a "Open logs folder" row in Settings → Developer Options so users can grab today's log file when reporting issues like "stuck on Initializing OpenHuman...".src/core/logging.rssoinit_for_cli_runand the newinit_for_embeddedshareseed_rust_log/build_env_filter/sentry_tracing_layer— single source of truth for formatter + filter.Problem
When the Tauri shell hits a startup issue (RPC bootstrap failure, CEF cache lock, sidecar crash) we have no log file to point users at. The shell's
env_loggerwrites to stderr — invisible in a packaged.app/.exe. The embedded core'stracingsubscriber (init_for_cli_runinsrc/core/logging.rs) was only invoked from CLI paths, so everytracing::info!in the core was a no-op when running inside the Tauri shell. Result: support has nothing concrete to ask the user for.Solution
src/core/logging.rs— extractseed_rust_log/build_env_filter/sentry_tracing_layerand addinit_for_embedded(data_dir, verbose). The new entry point installs a non-blocking, daily-rotated file appender (tracing-appender) at<data_dir>/logs/openhuman-YYYY-MM-DD.logplus the existing stderr layer (still useful undertauri dev). Both shareCleanCliFormat.Once-guarded; first caller wins. The non-blocking writer'sWorkerGuardand the resolved log dir are stashed inOnceLocks so they live for the entire process and the UI can resolve the path on demand.app/src-tauri/src/file_logging.rs— small shell-side wrapper that resolves the data dir the same way the core does (OPENHUMAN_WORKSPACEoverride, thendefault_root_openhuman_dir, withstd::env::temp_dir().join("openhuman")as the absolute-path fallback if home-dir lookup fails). Exposes two Tauri commands:logs_folder_path(returns the absolute path for display) andreveal_logs_folder(open/explorer/xdg-openper platform).app/src-tauri/src/lib.rs— callsfile_logging::init()early inrun()(before CEF preflight, before the Sentry smoke trigger), drops theenv_logger::Builder::try_initpath, and registers the new commands ininvoke_handler. Sentry init still runs first since that just builds a client guard.app/src/components/settings/panels/DeveloperOptionsPanel.tsx— adds a "App logs" row that shows the resolved path (logs_folder_path) and a button that callsreveal_logs_folder. Hidden in non-Tauri builds viaisTauri().app/src-tauri/permissions/allow-core-process.toml— capability allowlist entries for the two new commands.src/core/logging.rsandapp/src-tauri/src/file_logging.rsship#[cfg(test)] mod testscovering:seed_rust_logdefaults +RUST_LOGrespect,level_tag/short_target,parse_log_file_constraints,build_env_filtersmoke,resolve_data_dirworkspace override + empty-string fallthrough + temp-dir fallback, and the pre-init guards onlogs_folder_path/reveal_logs_folder. Env-mutating tests share aMutexto keep the parallel runner stable.Tradeoffs
<data_dir>/logs/(i.e.~/.openhuman/logs/by default, or underOPENHUMAN_WORKSPACE) keeps logs next toactive_user.toml, the per-userusers/tree, and the CEF caches a support engineer would also need. OS-conventional paths (~/Library/Logs/OpenHuman,%LOCALAPPDATA%\OpenHuman\Logs) are whatConsole.app/ Event Viewer expect but split logs per user and need per-platform code; theOPENHUMAN_WORKSPACEtest override would also stop working.info(ordebugifOPENHUMAN_VERBOSE=1/RUST_LOG=debug). Bumpable later if a debug session needs deeper history.init_for_embeddedruns aftersentry::initbut before everything else (CEF preflight included). Anything that fails beforerun()enters our init still hits stderr only — that's currentlymain.rsargv parsing andtauri::cef_entry_point, neither of which has user-actionable failures today.Submission Checklist
docs/TESTING-STRATEGY.md#[cfg(test)] mod testsblocks insrc/core/logging.rsandapp/src-tauri/src/file_logging.rscover the added lines.docs/TEST-COVERAGE-MATRIX.md).tracing-appenderis local-only).Impact
tracing_appender::non_blocking, so the UI thread never blocks on disk I/O. Background thread is owned by a process-lifetimeOnceLock<WorkerGuard>.info, daily logs are typically <1 MB. 7-day cap keeps total footprint bounded.~/.openhuman/data dir, same TCC/permission scope as today's storage.logs/subdir.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckcargo test --lib core::logging(9 pass),cargo test --lib file_logging(4 pass)cargo check --manifest-path Cargo.toml --libcargo check --manifest-path app/src-tauri/Cargo.tomlValidation Blocked
Behavior Changes
<data_dir>/logs/instead of stderr only.Parity Contract
init_for_cli_run(used byopenhuman run/ CLI subcommands) keeps the same stderr output andEnvFilterdefaults.Once-guarded subscriber init; non-blocking writer guard pinned for the process lifetime;reveal_logs_folderreturns a typed error if logging hasn't been initialized.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Chores
Tests