fix(logging): harden lifecycle capture and console recovery - #1268
Merged
Merged
Conversation
Contributor
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds typed operational audit context, early CLI lifecycle logging, runtime lifecycle coverage, bounded OpenAI stream artifacts, persistence and API projections, capture-status reporting, and log UI metadata and recovery support. ChangesOperational logging
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: ⚪ Minimal · up to The PR hardens lifecycle logging, redaction, startup ordering, CLI parsing, and typed SSE projections; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/mesh-llm-host-runtime/src/api/routes/logs/events/protocol.rs (1)
182-255: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate every projected context field.
Line 202 accepts arbitrary numeric-summary keys. Lines 244-249 also project arbitrary
subject_kind,reason_code, andoutcomevalues from persisted payloads.A malformed or legacy audit payload can then send unbounded-vocabulary metadata to the live SSE client. Validate
subject_kindagainst the closed subject set. Validate reason, outcome, and numeric keys with the same static-code rules asOperationalAuditContext. Apply the eight-entry limit after key validation.🤖 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 `@crates/mesh-llm-host-runtime/src/api/routes/logs/events/protocol.rs` around lines 182 - 255, Update the context projection around the AuditEntryData construction to reuse the static-code validation rules from OperationalAuditContext: restrict subject_kind to the closed subject set, validate reason_code and outcome, and accept only valid numeric-summary keys. Filter numeric_summaries by key before applying the existing eight-entry limit, and ensure invalid projected fields are omitted rather than sent to the SSE client.
🤖 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 `@crates/mesh-llm-host-runtime/src/logging/service/operational_audit.rs`:
- Around line 145-152: Update bounded_context_value to apply redact_urls_in_text
in the established logging-policy sequence, alongside path sanitization and
generic redaction, before trimming and storing the value. Add a regression test
covering a credential-bearing URL and verify the resulting context value
contains no URL credentials or query secrets.
In `@crates/mesh-llm-host-runtime/src/runtime/control_loop.rs`:
- Around line 349-354: Extend terminal lifecycle audit events to include elapsed
duration in milliseconds. In
crates/mesh-llm-host-runtime/src/runtime/control_loop.rs:349-354, capture the
start time at ShutdownStarted and add elapsed time to ShutdownCompleted; apply
the corresponding per-instance and managed-model shutdown timing in
model_lifecycle.rs:326-329 and 364-368. In model_lifecycle/load.rs:4-14 and
unload.rs:14-33, propagate each operation’s start time through all failure and
completion branches, including resolution and capacity failures. In
startup_handles.rs:1497-1500, pass the current load attempt start time to
record_startup_terminal_failure and include elapsed time in ModelLoadFailed.
In `@crates/mesh-llm-host-runtime/src/runtime/startup_handles.rs`:
- Around line 1215-1219: Move the initial ModelLoadStarted recording ahead of
prepare_startup_local_model_task in the startup flow, ensuring preparation
failures still have a matching start event. Preserve exactly one start event for
each split-model relaunch, and add a regression test covering preparation
failure event ordering.
In `@crates/mesh-llm-ui/src/features/logs/components/LogEventLedgerColumns.tsx`:
- Around line 173-175: Update the audit branch of the context accessor to append
row.audit.entryId after the existing audit fields, while leaving the request
branch unchanged so legacy audit entry IDs remain searchable and filterable.
In `@crates/mesh-llm/src/lib.rs`:
- Around line 201-235: Update parse_failure_family to skip operands belonging to
global options before matching positional command tokens, so values such as
“runtime” after “--config” cannot determine the family; preserve
explicit_surface behavior and existing family mappings. Add a regression test
covering “--config runtime --bad-flag” and assert it does not classify as
Runtime.
---
Outside diff comments:
In `@crates/mesh-llm-host-runtime/src/api/routes/logs/events/protocol.rs`:
- Around line 182-255: Update the context projection around the AuditEntryData
construction to reuse the static-code validation rules from
OperationalAuditContext: restrict subject_kind to the closed subject set,
validate reason_code and outcome, and accept only valid numeric-summary keys.
Filter numeric_summaries by key before applying the existing eight-entry limit,
and ensure invalid projected fields are omitted rather than sent to the SSE
client.
🪄 Autofix
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 Plus
Run ID: 902dd98b-ffb0-43ff-bac7-d2b6a24127d9
📒 Files selected for processing (46)
crates/mesh-llm-commands/src/operational_logging.rscrates/mesh-llm-events/src/command_lifecycle.rscrates/mesh-llm-host-runtime/src/api/routes/logs/dto.rscrates/mesh-llm-host-runtime/src/api/routes/logs/events/protocol.rscrates/mesh-llm-host-runtime/src/api/routes/logs/mod.rscrates/mesh-llm-host-runtime/src/api/routes/logs/tests.rscrates/mesh-llm-host-runtime/src/api/status.rscrates/mesh-llm-host-runtime/src/lib.rscrates/mesh-llm-host-runtime/src/logging/mod.rscrates/mesh-llm-host-runtime/src/logging/openai_lifecycle.rscrates/mesh-llm-host-runtime/src/logging/persistence.rscrates/mesh-llm-host-runtime/src/logging/policy.rscrates/mesh-llm-host-runtime/src/logging/policy/artifact_redaction.rscrates/mesh-llm-host-runtime/src/logging/runtime_state.rscrates/mesh-llm-host-runtime/src/logging/runtime_state/tests.rscrates/mesh-llm-host-runtime/src/logging/service.rscrates/mesh-llm-host-runtime/src/logging/service/operational_audit.rscrates/mesh-llm-host-runtime/src/network/openai/response/stream_translation.rscrates/mesh-llm-host-runtime/src/network/openai/response_adapter.rscrates/mesh-llm-host-runtime/src/runtime/control_loop.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rscrates/mesh-llm-host-runtime/src/runtime/operational_logging.rscrates/mesh-llm-host-runtime/src/runtime/operational_logging/tests.rscrates/mesh-llm-host-runtime/src/runtime/startup_handles.rscrates/mesh-llm-log-store/src/api_acceptance_tests/summary_audit.rscrates/mesh-llm-log-store/src/repositories.rscrates/mesh-llm-ui/src/features/logs/api/schemas.test.tscrates/mesh-llm-ui/src/features/logs/api/schemas.tscrates/mesh-llm-ui/src/features/logs/api/use-logs-live-recovery.test.tsxcrates/mesh-llm-ui/src/features/logs/api/use-logs-live-recovery.tscrates/mesh-llm-ui/src/features/logs/components/LogEventInspector.test.tsxcrates/mesh-llm-ui/src/features/logs/components/LogEventInspector.tsxcrates/mesh-llm-ui/src/features/logs/components/LogEventLedgerColumns.tsxcrates/mesh-llm-ui/src/features/logs/components/LogsLedger.test.tsxcrates/mesh-llm-ui/src/features/logs/components/LogsLedger.tsxcrates/mesh-llm-ui/src/features/logs/pages/LogMaintenanceInvalidation.test.tsxcrates/mesh-llm-ui/src/features/logs/pages/LogsLedgerPage.tsxcrates/mesh-llm-ui/src/lib/api/types.tscrates/mesh-llm/src/commands/mod.rscrates/mesh-llm/src/commands/plugin_cli.rscrates/mesh-llm/src/lib.rsdocs/EXO_COMPARISON.mddocs/LOGGING.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/mesh-llm-host-runtime/src/api/routes/logs/events/protocol.rs`:
- Around line 363-387: Add a ninth valid numeric summary key to the audit entry
fixture used by the numeric summaries test, then assert the serialized data
under numericSummaries contains only eight entries and does not include that
ninth key. Keep the existing filtering assertion for “bad-key” and verify the
cap enforced by audit_entry_frame.
🪄 Autofix
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 Plus
Run ID: 667feccc-10a1-48e9-8577-cc19a97ba175
📒 Files selected for processing (10)
crates/mesh-llm-host-runtime/src/api/routes/logs/events/protocol.rscrates/mesh-llm-host-runtime/src/logging/policy.rscrates/mesh-llm-host-runtime/src/logging/service/operational_audit.rscrates/mesh-llm-host-runtime/src/runtime/control_loop.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rscrates/mesh-llm-host-runtime/src/runtime/startup_handles.rscrates/mesh-llm-ui/src/features/logs/components/LogEventLedgerColumns.tsxcrates/mesh-llm/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- crates/mesh-llm-host-runtime/src/runtime/control_loop.rs
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rs
- crates/mesh-llm-ui/src/features/logs/components/LogEventLedgerColumns.tsx
- crates/mesh-llm-host-runtime/src/logging/policy.rs
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rs
- crates/mesh-llm/src/lib.rs
- crates/mesh-llm-host-runtime/src/runtime/startup_handles.rs
- crates/mesh-llm-host-runtime/src/logging/service/operational_audit.rs
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rs
ndizazzo
force-pushed
the
codex/harden-logging-lifecycle
branch
3 times, most recently
from
August 13, 2026 09:43
600228d to
53864fc
Compare
ndizazzo
force-pushed
the
codex/harden-logging-lifecycle
branch
from
August 13, 2026 10:41
53864fc to
801d412
Compare
This was referenced Aug 15, 2026
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.
Corrective follow-up to the merged logging stack
This PR follows the logging stack merged into
main:The initial stack was validated end-to-end with the embedded console and a local Qwen3.5-4B inference run. A follow-up review then checked every supplied finding against the current code. All six inline findings and the outside-diff SSE projection finding were still valid; none were skipped.
Needed corrections
Typed audit context now applies the established path → URL → generic redaction sequence before storage. URL userinfo is also redacted so credential-bearing URLs cannot leak credentials without a query string. Regression coverage verifies URL credentials and query secrets are absent.
Terminal lifecycle events now carry bounded
duration_msvalues:Operation start times are propagated through all listed completion and failure branches.
Startup emits exactly one
runtime_model_load_startedfor the initial attempt before preparation, so preparation failures have a matching start event. Split-model relaunches emit one new start per relaunch. A regression test covers preparation-failure ordering.Audit row context search retains the legacy
entryId; the request-row branch is unchanged.CLI parse-family detection derives value-taking options from Clap and skips their operands before classifying positional command tokens.
--config runtime --bad-flagtherefore remainsunknown, while explicit runtime surfaces and existing family mappings are preserved.SSE audit projection now reuses the typed audit contract: closed subject kinds, valid static reason/outcome codes, and valid numeric-summary keys. Invalid fields are omitted, and numeric summaries are filtered before the eight-entry bound.
The store, REST API, SSE, and UI remain an allowlisted typed projection; arbitrary
detail_jsonstays private.Scope boundary
This PR does not change mesh protocol fields, Skippy ABI/native callbacks, topology/session/prefill/decode/KV/device/resource hooks, or other underlying elements merely to add events. Event families needing new authoritative hooks remain deferred to
.omo/specs/event-system.md.Invite-token policy
Mesh invite tokens are intentionally public, shareable connection descriptors. They are not credentials and should not be flagged for redaction. Owner keys, authorization headers, API keys, private URLs, and other authentication material remain protected.
Validation
just release-host-buildcargo clippy -p mesh-llm --all-targets -- -D warningscargo fmt --all -- --checkandgit diff --checkmesh-llm-host-runtimelibrary tests: 2,458 passed, 8 ignored, 0 failedSummary by CodeRabbit
New Features
Bug Fixes
Documentation