Skip to content

feat(cardwired): add a DBUS API for blocked apps logs - #147

Merged
luytan merged 12 commits into
mainfrom
analyzer-notify-blocked
Aug 6, 2026
Merged

feat(cardwired): add a DBUS API for blocked apps logs#147
luytan merged 12 commits into
mainfrom
analyzer-notify-blocked

Conversation

@luytan

@luytan luytan commented Aug 5, 2026

Copy link
Copy Markdown
Member

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

  • Copy-Paste this line

Checklist:

  • My code follows the style guidelines of this project (cargo fmt)
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the mdBook documentation
  • My changes generate no new warnings (clippy/clang)
  • New and existing unit tests pass locally with my changes (either use nix flake check or wait for the ci)

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added a D-Bus logger interface for retrieving blocked-process reports.
    • Reports now include timestamps, process details, GPU information, and Wayland app IDs when available.
    • Added notifications for newly recorded blocked-process events.
    • Bounded report history and processing concurrency help maintain responsiveness.
  • Bug Fixes

    • Duplicate reports for the same process are filtered.
    • Malformed events no longer disrupt report processing.
    • Wayland app-ID lookups retry briefly and stop when unavailable or when the process exits.

Walkthrough

The daemon exposes blocked-process reports through D-Bus. The analyzer processes report events in a dedicated task, stores bounded LogEntry history, deduplicates PIDs, resolves Wayland app IDs with retries, and emits optional change signals. CI and eBPF builds use a pinned nightly toolchain.

Changes

Blocked-process logging

Layer / File(s) Summary
Logger interface contract
crates/cardwire-daemon/src/interface/logger.rs, crates/cardwire-daemon/src/interface/mod.rs
LogEntry and LoggerInterface provide shared report storage, a process_blocked getter, and a process_blocked_changed signal.
Daemon logger wiring
crates/cardwire-daemon/src/models.rs, crates/cardwire-daemon/src/daemon.rs
DaemonManager creates and stores the logger interface, registers it on D-Bus, stores its signal emitter, and passes logger state to CardwireAnalyzer::build.
Dedicated report processing
crates/cardwire-daemon/src/analyzer/models.rs, crates/cardwire-daemon/src/analyzer/dynamic_analysis.rs
The analyzer validates events, deduplicates recent PIDs, limits concurrent processing, resolves process identity, stores LogEntry records, and emits optional signals. Wayland app-ID lookup uses a two-second deadline.
Pinned build environment
.github/workflows/cicd.yml, crates/cardwire-ebpf-userspace/build.rs, .gitignore
CI and eBPF builds use nightly-2026-08-04. Git ignores the cardwired launcher wrapper.

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
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description contains only the unfilled template and provides no summary, motivation, issue reference, or completed checklist items. Add a change summary, explain the feature motivation, provide an issue reference if applicable, and complete the relevant checklist items.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding a D-Bus API for blocked application logs.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@luytan
luytan marked this pull request as ready for review August 5, 2026 16:10

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 50c8bf0 and dca45dc.

📒 Files selected for processing (5)
  • crates/cardwire-daemon/src/analyzer/models.rs
  • crates/cardwire-daemon/src/daemon.rs
  • crates/cardwire-daemon/src/interface/logger.rs
  • crates/cardwire-daemon/src/interface/mod.rs
  • crates/cardwire-daemon/src/models.rs

Comment thread crates/cardwire-daemon/src/analyzer/models.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between dca45dc and c453391.

📒 Files selected for processing (4)
  • .gitignore
  • crates/cardwire-daemon/src/analyzer/dynamic_analysis.rs
  • crates/cardwire-daemon/src/analyzer/models.rs
  • crates/cardwire-daemon/src/interface/logger.rs

Comment thread crates/cardwire-daemon/src/analyzer/dynamic_analysis.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between c453391 and 3e11b74.

📒 Files selected for processing (1)
  • crates/cardwire-daemon/src/analyzer/dynamic_analysis.rs

Comment thread crates/cardwire-daemon/src/analyzer/dynamic_analysis.rs
@luytan
luytan marked this pull request as draft August 5, 2026 19:10
@luytan

luytan commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

back to draft, i don't like how the analyzer part turned out

@luytan

luytan commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

will continue tmrw

@luytan
luytan force-pushed the analyzer-notify-blocked branch from 70e120c to b0bf493 Compare August 6, 2026 09:05
@luytan
luytan marked this pull request as ready for review August 6, 2026 10:21
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 50c8bf0 and 6353750.

📒 Files selected for processing (9)
  • .github/workflows/cicd.yml
  • .gitignore
  • crates/cardwire-daemon/src/analyzer/dynamic_analysis.rs
  • crates/cardwire-daemon/src/analyzer/models.rs
  • crates/cardwire-daemon/src/daemon.rs
  • crates/cardwire-daemon/src/interface/logger.rs
  • crates/cardwire-daemon/src/interface/mod.rs
  • crates/cardwire-daemon/src/models.rs
  • crates/cardwire-ebpf-userspace/build.rs

Comment thread .github/workflows/cicd.yml
Comment thread .gitignore Outdated
Comment thread crates/cardwire-daemon/src/analyzer/models.rs
Comment thread crates/cardwire-daemon/src/analyzer/models.rs
@luytan
luytan merged commit 93848d3 into main Aug 6, 2026
7 checks passed
@luytan
luytan deleted the analyzer-notify-blocked branch August 7, 2026 09:46
@coderabbitai coderabbitai Bot mentioned this pull request Aug 7, 2026
7 tasks
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