feat(cardwired): add a DBUS API for blocked apps logs - #147
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe daemon exposes blocked-process reports through D-Bus. The analyzer processes report events in a dedicated task, stores bounded ChangesBlocked-process logging
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReportRing
participant CardwireAnalyzer
participant LoggerInterface
participant DBusClient
ReportRing->>CardwireAnalyzer: ReportEvent
CardwireAnalyzer->>CardwireAnalyzer: Validate, deduplicate, and resolve process identity
CardwireAnalyzer->>LoggerInterface: Append LogEntry
CardwireAnalyzer->>DBusClient: process_blocked_changed(LogEntry)
DBusClient->>LoggerInterface: process_blocked()
LoggerInterface-->>DBusClient: Queued LogEntry records
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
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/cardwire-daemon/src/analyzer/models.rs`:
- Around line 160-170: Bound report processing and retention in the
event-handling flow around the spawned task and report_vec: replace per-event
task creation with a fixed worker mechanism, enforce a maximum VecDeque capacity
using an explicit eviction or drop policy, and prevent unbounded cloning during
process_blocked. Ensure the report_vec write guard is released before any await
of process_blocked_changed.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9e92ecf2-061c-44ed-86a6-f819a27b0c41
📒 Files selected for processing (5)
crates/cardwire-daemon/src/analyzer/models.rscrates/cardwire-daemon/src/daemon.rscrates/cardwire-daemon/src/interface/logger.rscrates/cardwire-daemon/src/interface/mod.rscrates/cardwire-daemon/src/models.rs
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/cardwire-daemon/src/analyzer/dynamic_analysis.rs`:
- Around line 156-162: Update the retry loop in the dynamic analysis lookup
around get_app_id_wayland to wrap each asynchronous lookup in
tokio::time::timeout using the remaining duration until deadline. Treat timeout
expiration as a failed lookup and return None when the overall lookup deadline
is reached, ensuring a stalled Niri IPC read cannot retain a report semaphore
permit indefinitely.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9a745993-dbc2-45c0-a2a8-7b2eb06e2175
📒 Files selected for processing (4)
.gitignorecrates/cardwire-daemon/src/analyzer/dynamic_analysis.rscrates/cardwire-daemon/src/analyzer/models.rscrates/cardwire-daemon/src/interface/logger.rs
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/cardwire-daemon/src/analyzer/dynamic_analysis.rs`:
- Around line 156-162: Update the retry loop in the dynamic-analysis lookup
around get_app_id_wayland and the remaining deadline calculation so that, after
an unsuccessful lookup, it returns immediately when the budget is exhausted;
otherwise cap the retry sleep at the current remaining deadline using the
smaller of delay and the recalculated deadline duration, preventing the sleep
from extending past the timeout.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8eb980b6-59a4-4bc0-8df9-a8177e78156d
📒 Files selected for processing (1)
crates/cardwire-daemon/src/analyzer/dynamic_analysis.rs
|
back to draft, i don't like how the analyzer part turned out |
|
will continue tmrw |
non-desktop processes
70e120c to
b0bf493
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/cicd.yml:
- Line 73: Update the release workflow’s toolchain installation to explicitly
install the pinned nightly-2026-08-04 toolchain used by build.rs, rather than
relying only on the `@nightly` alias; preserve the existing release build
configuration.
In @.gitignore:
- Around line 27-28: Update the root .gitignore entry for cardwired to use the
repository-root-scoped pattern /cardwired, so only the root launcher is ignored
and same-named files or directories elsewhere remain visible.
In `@crates/cardwire-daemon/src/analyzer/models.rs`:
- Around line 280-282: Update the Wayland-success branches around report_blocked
to pass the process command as the name argument while retaining app_id as the
wayland_app_id argument. Use the existing process-command value already
available for the event, and apply the same mapping to the additional branch
noted in the comment.
- Around line 355-360: The ReportEvent flow currently loses the GPU identifier
and hardcodes gpu_id in LogEntry. Add a GPU ID field to both ReportEvent
layouts, populate it at every report_event call from is_inode_blocked’s
ino_gpu_id, and pass it through to report_blocked so LogEntry uses that value
instead of gpu_id: 1.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2eff45ac-ed15-4bad-8333-08340f0a7971
📒 Files selected for processing (9)
.github/workflows/cicd.yml.gitignorecrates/cardwire-daemon/src/analyzer/dynamic_analysis.rscrates/cardwire-daemon/src/analyzer/models.rscrates/cardwire-daemon/src/daemon.rscrates/cardwire-daemon/src/interface/logger.rscrates/cardwire-daemon/src/interface/mod.rscrates/cardwire-daemon/src/models.rscrates/cardwire-ebpf-userspace/build.rs
Description
Please include a summary of the changes and if applicable, a related issue.
If this PR introduce a new feature, explain your motivations
Fixes # (issue)
TODO
Checklist: