Skip to content

fix: a few regressions from previous PRs - #10

Open
tomerqodo wants to merge 5 commits into
qodo_full_base_fix_a_few_regressions_from_previous_prs_pr10from
qodo_full_head_fix_a_few_regressions_from_previous_prs_pr10
Open

tomerqodo wants to merge 5 commits into
qodo_full_base_fix_a_few_regressions_from_previous_prs_pr10from
qodo_full_head_fix_a_few_regressions_from_previous_prs_pr10

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#10

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (4) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Cookie re-export lacks docs 📘 Rule violation ✓ Correctness
Description
• pub use tauri_runtime::Cookie; is a public API surface but is preceded only by a non-doc
  comment, so it will not appear in generated docs.
• This violates the requirement that public API elements include /// documentation comments,
  reducing discoverability and increasing downstream confusion.
Code

crates/tauri/src/webview/mod.rs[R22-23]

+// Remove this re-export in v3
+pub use tauri_runtime::Cookie;
Evidence
PR Compliance ID 13 requires public APIs to include documentation comments. The added `pub use
tauri_runtime::Cookie; is a public re-export with no /// docs (only a //` comment).

AGENTS.md
crates/tauri/src/webview/mod.rs[21-23]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A public re-export (`pub use tauri_runtime::Cookie;`) was added without Rust doc comments (`///`), which violates the requirement that public APIs be documented.

## Issue Context
`//` comments do not become API documentation and will not show up in rustdoc output for downstream users.

## Fix Focus Areas
- crates/tauri/src/webview/mod.rs[21-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. window_features rustfmt violations 📘 Rule violation ✓ Correctness
Description
• The new implementation contains spacing inconsistencies (extra spaces and missing spaces after
  commas) that cargo fmt would rewrite.
• This likely causes cargo fmt --all -- --check to fail, violating formatting compliance and
  creating noisy diffs.
Code

crates/tauri/src/webview/webview_window.rs[R1315-1322]

+  pub fn window_features(mut self, features: NewWindowFeatures) -> Self {
+    if let Some(size) = features.size() {
+      self.window_builder = self.window_builder.inner_size(size.width,    size.height);
    }

-    if let Some(size) = features.size() {
-      self.window_builder = self.window_builder.inner_size(size.width, size.height);
+    if let Some(position) = features.position() {
+      self.window_builder = self.window_builder.position(position.x,position.y);
    }
Evidence
PR Compliance ID 9 requires Rust code to be formatted according to rustfmt. The newly added lines
include obvious formatting issues (e.g., multiple spaces and missing spaces after commas) that
rustfmt would change.

AGENTS.md
crates/tauri/src/webview/webview_window.rs[1315-1322]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `window_features` implementation has formatting that does not match rustfmt (extra spaces and missing spaces after commas), so `cargo fmt --check` will likely fail.

## Issue Context
Formatting compliance is enforced repository-wide; changes that rustfmt would rewrite should be committed in rustfmt-compliant form.

## Fix Focus Areas
- crates/tauri/src/webview/webview_window.rs[1315-1322]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. builder.build() uses unwrap() 📘 Rule violation ⛯ Reliability
Description
• The new on_new_window handler uses multiple unwrap() calls (parse().unwrap(),
  set_title(...).unwrap(), builder.build().unwrap()), which can panic at runtime.
• This violates the requirement to avoid panicking for fallible operations and to handle errors with
  meaningful context and graceful behavior.
Code

examples/api/src-tauri/src/lib.rs[R89-101]

+            let builder = tauri::WebviewWindowBuilder::new(
+              &app_,
+              format!("new-window-{number}"),
+              tauri::WebviewUrl::External("about:blank".parse().unwrap()),
+            )
+            .window_features(features)
+            .on_document_title_changed(|window, title| {
+              window.set_title(&title).unwrap();
+            })
+            .title(url.as_str());
+
+            let window = builder.build().unwrap();
+            tauri::webview::NewWindowResponse::Create { window }
Evidence
PR Compliance ID 22 requires fallible operations to return/propagate Result rather than panicking
via unwrap. The added code introduces new unwrap() usages in a window-creation path that can
fail (URL parsing, title setting, window building).

Rule 3: Generic: Robust Error Handling and Edge Case Management
AGENTS.md
examples/api/src-tauri/src/lib.rs[89-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New code introduces `unwrap()` on fallible operations inside `on_new_window`, which can panic in production-like runs.

## Issue Context
This code path includes URL parsing, window creation, and title setting—each can fail depending on platform/runtime conditions.

## Fix Focus Areas
- examples/api/src-tauri/src/lib.rs[89-101]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (2)
4. delete_cookie sets cookie 🐞 Bug ✓ Correctness
Description
• delete_cookie now dispatches WebviewMessage::SetCookie instead of DeleteCookie, so the
  cookie is not removed.
• This silently breaks expected behavior (e.g., logout/session cleanup) and can leave sensitive
  session cookies behind.
• The runtime message loop still has a DeleteCookie branch, so this looks like an accidental
  wiring regression.
Code

crates/tauri-runtime-wry/src/lib.rs[1718]

+        WebviewMessage::SetCookie(cookie.into_owned()),
Evidence
WryWebviewDispatcher::delete_cookie currently emits WebviewMessage::SetCookie, while the message
handler clearly differentiates SetCookie (calls webview.set_cookie) vs DeleteCookie (calls
webview.delete_cookie). Therefore delete_cookie cannot trigger deletion.

crates/tauri-runtime-wry/src/lib.rs[1712-1721]
crates/tauri-runtime-wry/src/lib.rs[1407-1411]
crates/tauri-runtime-wry/src/lib.rs[3731-3741]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`WryWebviewDispatcher::delete_cookie` currently sends `WebviewMessage::SetCookie`, which causes the cookie to be set instead of deleted.

### Issue Context
The runtime message loop treats `SetCookie` and `DeleteCookie` differently (`webview.set_cookie` vs `webview.delete_cookie`).

### Fix Focus Areas
- crates/tauri-runtime-wry/src/lib.rs[1712-1721]
- crates/tauri-runtime-wry/src/lib.rs[3731-3741]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Semver-breaking API rename 🐞 Bug ⛯ Reliability
Description
• WebviewWindowBuilder::with_window_features was removed/renamed to window_features without
  keeping a deprecated alias.
• In tauri v2.7.0, removing a public method in a minor release is a semver-breaking change that
  will break downstream crates at compile time.
• If this rename is intentional, it should be introduced via a deprecated wrapper and removed in v3,
  not v2.x.
Code

crates/tauri/src/webview/webview_window.rs[1315]

+  pub fn window_features(mut self, features: NewWindowFeatures) -> Self {
Evidence
The crate is currently version 2.7.0. The builder impl now exposes window_features(...) but does
not include with_window_features(...) in the same impl block, so any downstream usage of the old
method will fail to compile.

crates/tauri/Cargo.toml[1-4]
crates/tauri/src/webview/webview_window.rs[1304-1323]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A public method rename (`with_window_features` → `window_features`) was done without a compatibility shim, which is semver-breaking for v2.x.

### Issue Context
The crate version is 2.7.0, and downstream code using the old method will fail to compile.

### Fix Focus Areas
- crates/tauri/src/webview/webview_window.rs[1304-1351]
- crates/tauri/Cargo.toml[1-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

6. println! logs URL/title 📘 Rule violation ⛨ Security
Description
• The new code logs url, features, and document title using println!, which is unstructured
  and may include sensitive or user-controlled data.
• This violates the secure logging requirement (structured logging and no sensitive data in logs),
  and also makes log ingestion/monitoring harder.
Code

examples/api/src-tauri/src/lib.rs[R70-86]

        .on_document_title_changed(|_window, title| {
          println!("document title changed: {title}");
        });

      #[cfg(all(desktop, not(test)))]
      {
+        let app_ = app.handle().clone();
+        let mut created_window_count = std::sync::atomic::AtomicUsize::new(0);
+
        window_builder = window_builder
          .title("Tauri API Validation")
          .inner_size(1000., 800.)
          .min_inner_size(600., 400.)
-          .menu(tauri::menu::Menu::default(app.handle())?);
+          .menu(tauri::menu::Menu::default(app.handle())?)
+          .on_new_window(move |url, features| {
+            println!("new window requested: {url:?} {features:?}");
+
Evidence
PR Compliance ID 5 requires structured logging and prohibits sensitive data in logs. The added
println! statements emit unstructured output and include potentially user-controlled values
(URL/title).

Rule 5: Generic: Secure Logging Practices
examples/api/src-tauri/src/lib.rs[69-86]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The example app uses `println!` to log potentially user-controlled values (URL/title) in an unstructured way.

## Issue Context
Compliance requires structured logs and avoiding sensitive/user data leakage in logs; `println!` bypasses normal logging configuration and formatting.

## Fix Focus Areas
- examples/api/src-tauri/src/lib.rs[69-86]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



ⓘ The new review experience is currently in Beta. Learn more

Qodo Logo

Comment on lines +22 to +23
// Remove this re-export in v3
pub use tauri_runtime::Cookie;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. cookie re-export lacks docs 📘 Rule violation ✓ Correctness

• pub use tauri_runtime::Cookie; is a public API surface but is preceded only by a non-doc
  comment, so it will not appear in generated docs.
• This violates the requirement that public API elements include /// documentation comments,
  reducing discoverability and increasing downstream confusion.
Agent prompt
## Issue description
A public re-export (`pub use tauri_runtime::Cookie;`) was added without Rust doc comments (`///`), which violates the requirement that public APIs be documented.

## Issue Context
`//` comments do not become API documentation and will not show up in rustdoc output for downstream users.

## Fix Focus Areas
- crates/tauri/src/webview/mod.rs[21-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +1315 to 1322
pub fn window_features(mut self, features: NewWindowFeatures) -> Self {
if let Some(size) = features.size() {
self.window_builder = self.window_builder.inner_size(size.width, size.height);
}

if let Some(size) = features.size() {
self.window_builder = self.window_builder.inner_size(size.width, size.height);
if let Some(position) = features.position() {
self.window_builder = self.window_builder.position(position.x,position.y);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. window_features rustfmt violations 📘 Rule violation ✓ Correctness

• The new implementation contains spacing inconsistencies (extra spaces and missing spaces after
  commas) that cargo fmt would rewrite.
• This likely causes cargo fmt --all -- --check to fail, violating formatting compliance and
  creating noisy diffs.
Agent prompt
## Issue description
The new `window_features` implementation has formatting that does not match rustfmt (extra spaces and missing spaces after commas), so `cargo fmt --check` will likely fail.

## Issue Context
Formatting compliance is enforced repository-wide; changes that rustfmt would rewrite should be committed in rustfmt-compliant form.

## Fix Focus Areas
- crates/tauri/src/webview/webview_window.rs[1315-1322]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +89 to +101
let builder = tauri::WebviewWindowBuilder::new(
&app_,
format!("new-window-{number}"),
tauri::WebviewUrl::External("about:blank".parse().unwrap()),
)
.window_features(features)
.on_document_title_changed(|window, title| {
window.set_title(&title).unwrap();
})
.title(url.as_str());

let window = builder.build().unwrap();
tauri::webview::NewWindowResponse::Create { window }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

3. builder.build() uses unwrap() 📘 Rule violation ⛯ Reliability

• The new on_new_window handler uses multiple unwrap() calls (parse().unwrap(),
  set_title(...).unwrap(), builder.build().unwrap()), which can panic at runtime.
• This violates the requirement to avoid panicking for fallible operations and to handle errors with
  meaningful context and graceful behavior.
Agent prompt
## Issue description
New code introduces `unwrap()` on fallible operations inside `on_new_window`, which can panic in production-like runs.

## Issue Context
This code path includes URL parsing, window creation, and title setting—each can fail depending on platform/runtime conditions.

## Fix Focus Areas
- examples/api/src-tauri/src/lib.rs[89-101]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines 70 to +86
.on_document_title_changed(|_window, title| {
println!("document title changed: {title}");
});

#[cfg(all(desktop, not(test)))]
{
let app_ = app.handle().clone();
let mut created_window_count = std::sync::atomic::AtomicUsize::new(0);

window_builder = window_builder
.title("Tauri API Validation")
.inner_size(1000., 800.)
.min_inner_size(600., 400.)
.menu(tauri::menu::Menu::default(app.handle())?);
.menu(tauri::menu::Menu::default(app.handle())?)
.on_new_window(move |url, features| {
println!("new window requested: {url:?} {features:?}");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

4. println! logs url/title 📘 Rule violation ⛨ Security

• The new code logs url, features, and document title using println!, which is unstructured
  and may include sensitive or user-controlled data.
• This violates the secure logging requirement (structured logging and no sensitive data in logs),
  and also makes log ingestion/monitoring harder.
Agent prompt
## Issue description
The example app uses `println!` to log potentially user-controlled values (URL/title) in an unstructured way.

## Issue Context
Compliance requires structured logs and avoiding sensitive/user data leakage in logs; `println!` bypasses normal logging configuration and formatting.

## Fix Focus Areas
- examples/api/src-tauri/src/lib.rs[69-86]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

*self.window_id.lock().unwrap(),
self.webview_id,
WebviewMessage::DeleteCookie(cookie.clone().into_owned()),
WebviewMessage::SetCookie(cookie.into_owned()),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

5. Delete_cookie sets cookie 🐞 Bug ✓ Correctness

• delete_cookie now dispatches WebviewMessage::SetCookie instead of DeleteCookie, so the
  cookie is not removed.
• This silently breaks expected behavior (e.g., logout/session cleanup) and can leave sensitive
  session cookies behind.
• The runtime message loop still has a DeleteCookie branch, so this looks like an accidental
  wiring regression.
Agent prompt
### Issue description
`WryWebviewDispatcher::delete_cookie` currently sends `WebviewMessage::SetCookie`, which causes the cookie to be set instead of deleted.

### Issue Context
The runtime message loop treats `SetCookie` and `DeleteCookie` differently (`webview.set_cookie` vs `webview.delete_cookie`).

### Fix Focus Areas
- crates/tauri-runtime-wry/src/lib.rs[1712-1721]
- crates/tauri-runtime-wry/src/lib.rs[3731-3741]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

pub fn with_window_features(mut self, features: NewWindowFeatures) -> Self {
if let Some(position) = features.position() {
self.window_builder = self.window_builder.position(position.x, position.y);
pub fn window_features(mut self, features: NewWindowFeatures) -> Self {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

6. Semver-breaking api rename 🐞 Bug ⛯ Reliability

• WebviewWindowBuilder::with_window_features was removed/renamed to window_features without
  keeping a deprecated alias.
• In tauri v2.7.0, removing a public method in a minor release is a semver-breaking change that
  will break downstream crates at compile time.
• If this rename is intentional, it should be introduced via a deprecated wrapper and removed in v3,
  not v2.x.
Agent prompt
### Issue description
A public method rename (`with_window_features` → `window_features`) was done without a compatibility shim, which is semver-breaking for v2.x.

### Issue Context
The crate version is 2.7.0, and downstream code using the old method will fail to compile.

### Fix Focus Areas
- crates/tauri/src/webview/webview_window.rs[1304-1351]
- crates/tauri/Cargo.toml[1-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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.

3 participants