Breadway/breadbox
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions

Add persistent input and main-loop diagnostics #17

New issue
Closed
opened 2026-09-30 02:03:53 +08:00 by Breadway · 6 comments
Breadway commented 2026-09-30 02:03:53 +08:00
Owner
Copy link

Add opt-in-safe diagnostic logging to breadbox so an intermittent launcher freeze can be distinguished from lost keyboard delivery. Write rotating structured logs beneath $XDG_STATE_HOME/breadbox/ (falling back to $HOME/.local/state/breadbox/) and expose a clear way for users to locate and share the recent log.

Record GTK key press/release metadata without key text or search query contents, focus enter/leave transitions for the launcher/search entry, per-query elapsed time and result count, and a 500 ms GLib main-loop heartbeat while the popup is open. The heartbeat must be low overhead and stop when the popup closes. Include useful startup context such as desktop/session type and GTK version while excluding environment values that may contain credentials or personal data.

Add tests for log path selection, redaction boundaries, and diagnostic output/rotation where practical. Run the repository CI checks. Do not attempt a functional fix for BOX-8 based on the unconfirmed keyboard-grab hypothesis.

Add opt-in-safe diagnostic logging to breadbox so an intermittent launcher freeze can be distinguished from lost keyboard delivery. Write rotating structured logs beneath `$XDG_STATE_HOME/breadbox/` (falling back to `$HOME/.local/state/breadbox/`) and expose a clear way for users to locate and share the recent log. Record GTK key press/release metadata without key text or search query contents, focus enter/leave transitions for the launcher/search entry, per-query elapsed time and result count, and a 500 ms GLib main-loop heartbeat while the popup is open. The heartbeat must be low overhead and stop when the popup closes. Include useful startup context such as desktop/session type and GTK version while excluding environment values that may contain credentials or personal data. Add tests for log path selection, redaction boundaries, and diagnostic output/rotation where practical. Run the repository CI checks. Do not attempt a functional fix for BOX-8 based on the unconfirmed keyboard-grab hypothesis.
Breadway commented 2026-09-30 02:15:35 +08:00
Author
Owner
Copy link

Implemented persistent diagnostics in breadbox PR #20: #20. Logs use a private 1 MiB rotating file, capture keycodes/modifiers, focus, query timing/counts, startup session metadata and a 500 ms GLib heartbeat, with no typed text. breadbox --bug-report emits only diagnostic-target events and allowlisted environment details. Local workspace tests, clippy and build pass.

Implemented persistent diagnostics in breadbox PR #20: https://git.breadway.dev/Breadway/breadbox/pulls/20. Logs use a private 1 MiB rotating file, capture keycodes/modifiers, focus, query timing/counts, startup session metadata and a 500 ms GLib heartbeat, with no typed text. `breadbox --bug-report` emits only diagnostic-target events and allowlisted environment details. Local workspace tests, clippy and build pass.
Breadway commented 2026-09-30 02:19:59 +08:00
Author
Owner
Copy link

Review verdict: rework required. Acceptance criteria interpretation and mapping:

  1. Private rotating logs use XDG_STATE_HOME or the HOME fallback, and users can locate/share a filtered report: breadbox/src/diagnostics.rs:140-160, 246-286, plus README Diagnostics.
  2. Record key press/release keycode and modifiers, focus/window activation changes, query time and result count, without text: breadbox/src/main.rs:163-165, 215-218, 257-278, 275-315.
  3. Emit a 500 ms heartbeat only while the popup is open: breadbox/src/main.rs:319-339.
  4. Include allowlisted startup context and path/redaction/rotation tests: breadbox/src/diagnostics.rs:230-244, 338-442.

The tests exercise path selection, the desktop-name allowlist, report target filtering, and rotation behavior. The target-filter test is behavioral, though it does not cover the subscriber configuration that creates the leak below.

  • breadbox/src/diagnostics.rs:181-188: the file subscriber applies RUST_LOG directives to every tracing target, then writes all accepted events to the persistent diagnostics file. An override such as RUST_LOG=trace can put other crates' event fields into the file, even though the README says the diagnostic log excludes query text. Keep the shareable-file subscriber restricted to the diagnostics target (or apply an explicit field-safe filter); add a regression test showing a non-diagnostics event containing query text is absent from the actual sink even when RUST_LOG is broad. Apply the same privacy policy to the stderr fallback if it is meant to be diagnostic-only.

CI for head dac831c995dbf118593b18f76139e35fdcbd9ad1 is still pending, so I cannot verify the requested CI checks yet. Rework on the existing branch fix/box-17-diagnostics; keep PR #20 open and do not open a second PR for BOX-17.

Review verdict: rework required. Acceptance criteria interpretation and mapping: 1. Private rotating logs use XDG_STATE_HOME or the HOME fallback, and users can locate/share a filtered report: `breadbox/src/diagnostics.rs:140-160, 246-286`, plus README Diagnostics. 2. Record key press/release keycode and modifiers, focus/window activation changes, query time and result count, without text: `breadbox/src/main.rs:163-165, 215-218, 257-278, 275-315`. 3. Emit a 500 ms heartbeat only while the popup is open: `breadbox/src/main.rs:319-339`. 4. Include allowlisted startup context and path/redaction/rotation tests: `breadbox/src/diagnostics.rs:230-244, 338-442`. The tests exercise path selection, the desktop-name allowlist, report target filtering, and rotation behavior. The target-filter test is behavioral, though it does not cover the subscriber configuration that creates the leak below. - [ ] `breadbox/src/diagnostics.rs:181-188`: the file subscriber applies `RUST_LOG` directives to every tracing target, then writes all accepted events to the persistent diagnostics file. An override such as `RUST_LOG=trace` can put other crates' event fields into the file, even though the README says the diagnostic log excludes query text. Keep the shareable-file subscriber restricted to the diagnostics target (or apply an explicit field-safe filter); add a regression test showing a non-diagnostics event containing query text is absent from the actual sink even when RUST_LOG is broad. Apply the same privacy policy to the stderr fallback if it is meant to be diagnostic-only. CI for head `dac831c995dbf118593b18f76139e35fdcbd9ad1` is still pending, so I cannot verify the requested CI checks yet. Rework on the existing branch `fix/box-17-diagnostics`; keep PR #20 open and do not open a second PR for BOX-17.
Breadway commented 2026-09-30 02:21:14 +08:00
Author
Owner
Copy link

Reworking PR #20 against the review note. The persistent and stderr subscribers will use a fixed allowlist filter for breadbox::diagnostics, independent of RUST_LOG, and a regression test will exercise the actual sink with a non-diagnostics event carrying query text.

Reworking PR #20 against the review note. The persistent and stderr subscribers will use a fixed allowlist filter for `breadbox::diagnostics`, independent of `RUST_LOG`, and a regression test will exercise the actual sink with a non-diagnostics event carrying query text.
Breadway commented 2026-09-30 02:23:01 +08:00
Author
Owner
Copy link

Updated existing PR #20 with the privacy fix on commit e8965ce. Both file and stderr subscribers now use a fixed breadbox::diagnostics allowlist and ignore RUST_LOG; an actual-sink regression test runs in a subprocess with RUST_LOG=trace and confirms a query-bearing event from another crate is absent. RUST_LOG=trace cargo test --workspace (15 tests), clippy, build, touched-file rustfmt, and git diff --check pass.

Updated existing PR #20 with the privacy fix on commit e8965ce. Both file and stderr subscribers now use a fixed `breadbox::diagnostics` allowlist and ignore `RUST_LOG`; an actual-sink regression test runs in a subprocess with `RUST_LOG=trace` and confirms a query-bearing event from another crate is absent. `RUST_LOG=trace cargo test --workspace` (15 tests), clippy, build, touched-file rustfmt, and `git diff --check` pass.
Breadway commented 2026-09-30 02:31:43 +08:00
Author
Owner
Copy link

Re-review of PR #20 at e8965ce: code review passes. Acceptance criteria and mapping:

  1. Store bounded private rotating logs under absolute XDG_STATE_HOME, falling back to $HOME/.local/state, and provide a shareable recent-event report: breadbox/src/diagnostics.rs:140-216, 274-345; documented in README.md:68-81.
  2. Capture keycode/modifier metadata, focus and activation changes, query duration/result counts, and a 500 ms heartbeat while the popup is visible, without recording query text: breadbox/src/main.rs:163-168, 215-220, 257-266, 275-313, 319-337.
  3. Include useful startup context with allowlisted/redacted environment details: breadbox/src/diagnostics.rs:224-271.
  4. Cover path selection, redaction, actual sink filtering, and rotation: breadbox/src/diagnostics.rs:348-474.

The new regression test exercises the configured file sink with RUST_LOG=trace and confirms an event from another target carrying query text is excluded. Both persistent and stderr subscribers use the fixed diagnostics-only filter, so the earlier privacy issue is addressed.

The combined CI status for head e8965ce906d9213483eaa03f1827db742f82fade is currently pending (check / check (push) has started). No failed check is reported; the issue remains in REVIEW until CI completes.

Re-review of PR #20 at `e8965ce`: code review passes. Acceptance criteria and mapping: 1. Store bounded private rotating logs under absolute `XDG_STATE_HOME`, falling back to `$HOME/.local/state`, and provide a shareable recent-event report: `breadbox/src/diagnostics.rs:140-216, 274-345`; documented in `README.md:68-81`. 2. Capture keycode/modifier metadata, focus and activation changes, query duration/result counts, and a 500 ms heartbeat while the popup is visible, without recording query text: `breadbox/src/main.rs:163-168, 215-220, 257-266, 275-313, 319-337`. 3. Include useful startup context with allowlisted/redacted environment details: `breadbox/src/diagnostics.rs:224-271`. 4. Cover path selection, redaction, actual sink filtering, and rotation: `breadbox/src/diagnostics.rs:348-474`. The new regression test exercises the configured file sink with `RUST_LOG=trace` and confirms an event from another target carrying query text is excluded. Both persistent and stderr subscribers use the fixed diagnostics-only filter, so the earlier privacy issue is addressed. The combined CI status for head `e8965ce906d9213483eaa03f1827db742f82fade` is currently pending (check / check (push) has started). No failed check is reported; the issue remains in REVIEW until CI completes.
Breadway commented 2026-09-30 03:41:52 +08:00
Author
Owner
Copy link

Independent re-review of PR #20 at head e8965ce906d9213483eaa03f1827db742f82fade: PASS, ready to merge.

Acceptance criteria mapping:

  1. Private bounded rotation, XDG_STATE_HOME/HOME path selection, and a copyable report: breadbox/src/diagnostics.rs:140-216, 274-345; documented in README.md:68-81.
  2. Key press/release metadata, window/search focus, query duration/result count, and a 500 ms visible-window heartbeat without query text: breadbox/src/main.rs:163-168, 215-220, 257-266, 275-313, 319-337.
  3. Allowlisted desktop/session/GTK startup context: breadbox/src/diagnostics.rs:224-271.
  4. Path/redaction/rotation tests and an actual-sink filter regression: breadbox/src/diagnostics.rs:348-474.

The prior privacy finding is fixed. Both file and stderr subscribers use the fixed off,breadbox::diagnostics=info filter, independent of RUST_LOG. The regression launches an isolated process with RUST_LOG=trace, writes a query-bearing event from another target, and verifies that event is absent from the log sink while the diagnostic event is present.

Combined CI status for this head is success, and PR #20 is mergeable. The issue remains in REVIEW pending merge.

Independent re-review of PR #20 at head `e8965ce906d9213483eaa03f1827db742f82fade`: PASS, ready to merge. Acceptance criteria mapping: 1. Private bounded rotation, XDG_STATE_HOME/HOME path selection, and a copyable report: `breadbox/src/diagnostics.rs:140-216, 274-345`; documented in `README.md:68-81`. 2. Key press/release metadata, window/search focus, query duration/result count, and a 500 ms visible-window heartbeat without query text: `breadbox/src/main.rs:163-168, 215-220, 257-266, 275-313, 319-337`. 3. Allowlisted desktop/session/GTK startup context: `breadbox/src/diagnostics.rs:224-271`. 4. Path/redaction/rotation tests and an actual-sink filter regression: `breadbox/src/diagnostics.rs:348-474`. The prior privacy finding is fixed. Both file and stderr subscribers use the fixed `off,breadbox::diagnostics=info` filter, independent of `RUST_LOG`. The regression launches an isolated process with `RUST_LOG=trace`, writes a query-bearing event from another target, and verifies that event is absent from the log sink while the diagnostic event is present. Combined CI status for this head is success, and PR #20 is mergeable. The issue remains in REVIEW pending merge.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
fix/eco-83-check-checkout-isolation
v0.4.0
v0.4.0-rc.1
v0.3.3
v0.3.2
v0.3.1
v0.3.0
v0.2.7
v0.2.6
v0.2.5
v0.2.4
v0.2.3
v0.2.2
v0.2.1
v0.2.0
v0.1.0
Labels
Clear labels   
ci

   
diagnostics

   
unresolvable
No labels
ci
diagnostics
unresolvable
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Breadway/breadbox#17
No description provided.