Skip to content

fix(native): wait for the KDE feedback test listener - #10645

Merged
juliusmarminge merged 1 commit into
mainfrom
t3code/investigate-flaky-rust-ci
Sep 8, 2026
Merged

fix(native): wait for the KDE feedback test listener#10645
juliusmarminge merged 1 commit into
mainfrom
t3code/investigate-flaky-rust-ci

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 8, 2026

Copy link
Copy Markdown
Member

What changed

Register the feedback test bridge with Builder::serve_at before building its D-Bus connection. zbus waits for the method-call listener during this build, so the first authorization call cannot race listener startup. Keep the existing authorization checks and five-second deadline.

Why

The Rust job on main failed in only_kwin_can_read_commands_or_acknowledge_feedback even though the latest commit only changed web files. Registering the object on an already-running connection starts the listener asynchronously; the first Next call can arrive before its subscription exists and time out instead of returning AccessDenied.

Verification

Before, the unchanged crate reproduced the exact CI failure on iteration 13 when running four test processes on four CPU cores with Rust 1.98.1:

thread 'feedback::tests::only_kwin_can_read_commands_or_acknowledge_feedback' panicked at src/feedback.rs:249:9:
InputOutput(Custom { kind: TimedOut, error: "timed out" })
test result: FAILED. 10 passed; 1 failed

After, cargo test --locked --manifest-path native/kde-snap-shot/Cargo.toml passes all 11 tests. All 2,000 full-crate runs passed under the same contention, with zero failures. The PR Rust CI job also passes. Rust formatting and git diff --check pass.

Screenshots do not apply to this test-only change.

Checklist

  • Small and focused
  • Explained the change and cause
  • Included before/after test evidence

Model: GPT-6. Harness: Codex.

Summary by CodeRabbit

  • Tests
    • Improved authorization-flow test reliability by validating request handling through an active client connection.
    • Strengthened verification that authorization requests are processed and cleaned up correctly after testing.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 8, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4b7c501

Macroscope's review found this PR approvable — This is a focused test-only fix that synchronizes D-Bus listener setup and changes no production code or customer-facing behavior. Its runtime impact is limited to making the existing KDE feedback test more reliable.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.6 KiB +18 B (+0.1%) 15.1 KiB
Codex Thread snapshot wire 7.0 KiB 7.0 KiB +2 B (+0.0%) 7.3 KiB
Codex Live turn WebSocket wire 6.5 KiB 6.5 KiB +16 B (+0.2%) 7.8 KiB
Codex Live turn WebSocket decoded 57.0 KiB 57.0 KiB 0 B (0.0%) 66.4 KiB
Codex Live turn messages 8 8 0 (0.0%) 21
Claude Total thread wire 13.6 KiB 13.6 KiB +10 B (+0.1%) 15.1 KiB
Claude Thread snapshot wire 7.0 KiB 7.0 KiB −4 B (−0.1%) 7.3 KiB
Claude Live turn WebSocket wire 6.5 KiB 6.5 KiB +14 B (+0.2%) 7.8 KiB
Claude Live turn WebSocket decoded 57.8 KiB 57.8 KiB 0 B (0.0%) 66.4 KiB
Claude Live turn messages 9 9 0 (0.0%) 21

Baseline: 299404a · PR result: 4b7c501 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: c411de20-fbe5-4fb3-a21a-3d2aa94a6cb1

📥 Commits

Reviewing files that changed from the base of the PR and between 8588d7f and 4b7c501.

📒 Files selected for processing (1)
  • native/kde-snap-shot/src/feedback.rs

Included review availability: 9 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.


📝 Walkthrough

Walkthrough

The authorization test now uses a dedicated zbus connection that registers Bridge before requests and removes it during cleanup.

Changes

Authorization Test

Layer / File(s) Summary
Dedicated Bridge connection
native/kde-snap-shot/src/feedback.rs
The test registers Bridge during construction of a dedicated zbus connection, uses that connection for proxy calls, and removes Bridge from the same connection.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 4b7c5

The authorization test now registers its feedback bridge before sending requests, removing the timing race that caused intermittent timeouts. The change is ready to merge with no remaining material risk identified.

Suggested reviewers: bil0000

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
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.
Title check ✅ Passed The title clearly identifies the main change: fixing the KDE feedback test listener race.
Description check ✅ Passed The description explains what changed, why the race occurred, how it was verified, and why UI evidence does not apply. It is focused and includes a completed checklist.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/investigate-flaky-rust-ci

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

@juliusmarminge
juliusmarminge merged commit 9fe4d65 into main Sep 8, 2026
20 checks passed
@juliusmarminge
juliusmarminge deleted the t3code/investigate-flaky-rust-ci branch September 8, 2026 02:43
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 8, 2026
## What's Changed
* fix(web): open proactive panels when entering threads by @maria-rcks in pingdotgg/t3code#10610
* fix(native): wait for the KDE feedback test listener by @juliusmarminge in pingdotgg/t3code#10645
* fix(desktop): resolve local media linked from remote threads by @maria-rcks in pingdotgg/t3code#10619
* fix(web): add bottom padding to project actions header by @flamboh in pingdotgg/t3code#10634
* fix(web): update machines together in auto balance by @maria-rcks in pingdotgg/t3code#10596
* fix(preview): transfer recordings to the agent environment by @maria-rcks in pingdotgg/t3code#10572
* fix(web): navigate markdown images as galleries by @maria-rcks in pingdotgg/t3code#10625
* chore: upgrade to TypeScript 7.0.2 by @juliusmarminge in pingdotgg/t3code#10663
* fix: hide email-bearing account labels in usage limits by @juliusmarminge in pingdotgg/t3code#10668
* fix(web): keep scroll-to-end button close to composer by @Bil0000 in pingdotgg/t3code#10543
* chore(deps): upgrade Effect to rc.112 and Alchemy to beta.76 by @juliusmarminge in pingdotgg/t3code#10652
* chore(refs): sync Effect reference to rc.112 by @juliusmarminge in pingdotgg/t3code#10653
* chore(refs): sync Alchemy reference to beta.76 by @juliusmarminge in pingdotgg/t3code#10654
* fix: generate thread titles with the selected model across connections by @Bil0000 in pingdotgg/t3code#10526
* fix(desktop): enable context menus in the browser by @juliusmarminge in pingdotgg/t3code#10670
* fix(desktop): stop generating declarations during bundling by @juliusmarminge in pingdotgg/t3code#10679
* fix(desktop): restore layout control hit targets by @juliusmarminge in pingdotgg/t3code#10673


**Full Changelog**: pingdotgg/t3code@v0.0.41-nightly.20260908.1377...v0.0.41-nightly.20260908.1387

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.41-nightly.20260908.1387
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant