Skip to content

fix: address AI findings in qtpasssettings#960

Merged
annejan merged 1 commit intomainfrom
fix/qtpasssettings-ai-findings
Apr 10, 2026
Merged

fix: address AI findings in qtpasssettings#960
annejan merged 1 commit intomainfrom
fix/qtpasssettings-ai-findings

Conversation

@nogeenharrie
Copy link
Copy Markdown
Contributor

@nogeenharrie nogeenharrie commented Apr 10, 2026

User description

Summary

  • Change Pass* to QScopedPointer for proper cleanup
  • Replace foreach with std::as_const range-based for loops
  • Add null check before calling pass->init()

CodeAnt-AI Description

Prevent crashes when switching password backends and clean up settings handling

What Changed

  • The password backend is now created and initialized only when it exists, which avoids a crash if setup fails.
  • Switching between the real password backend and the fallback backend now updates the active backend safely.
  • Profile settings are still read and written the same way, but the code now uses safer list handling for profile entries.

Impact

✅ Fewer startup crashes
✅ Safer backend switching
✅ More reliable settings loading

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

  • Refactor
    • Improved internal iteration and memory handling for greater stability and maintainability.
    • Hardened lazy-initialization and caching so implementations are selected and initialized only when valid, reducing runtime errors and unexpected switches.
    • Minor build reliability improvement to ensure cleaner compilation across platforms.

@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai Bot commented Apr 10, 2026

CodeAnt AI is reviewing your PR.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 134c89a6-3bf9-4fbd-bfef-53f2a3054ec4

📥 Commits

Reviewing files that changed from the base of the PR and between 49632af and 9966c6c.

📒 Files selected for processing (2)
  • src/qtpasssettings.cpp
  • src/qtpasssettings.h

📝 Walkthrough

Walkthrough

Updated internal implementation of QtPassSettings: replaced foreach with range-based for using std::as_const, adjusted caching/initialization and assignment behavior for the pass pointer (added null-guarded init()), and added #include <utility> and <QScopedPointer>. No public APIs changed.

Changes

Cohort / File(s) Summary
QtPassSettings implementation
src/qtpasssettings.cpp
Replaced foreach loops with range-based for over std::as_const(...); changed getPass()/selection logic so assignments use the local pass variable and pass->init() is called only when pass is non-null; setUsePass() stops eagerly swapping cached pass and sets pass = nullptr to force re-evaluation; added #include <utility>.
QtPassSettings header
src/qtpasssettings.h
Added missing #include <QScopedPointer> to satisfy compile-time dependency for existing QScopedPointer<...> members. No public signature changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nudged through loops and pointers light,
Swapped old foreach for clearer sight.
I guarded init with careful paw,
Reset the cache, then hopped in awe.
Small hops, clean code — a merry night.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: address AI findings in qtpasssettings' accurately reflects the main objective of the PR, which addresses multiple findings in the qtpasssettings component.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 fix/qtpasssettings-ai-findings

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

@codeant-ai codeant-ai Bot added the size:S This PR changes 10-29 lines, ignoring generated files label Apr 10, 2026
Comment thread src/qtpasssettings.h Outdated
Comment thread src/qtpasssettings.cpp Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/qtpasssettings.cpp`:
- Around line 297-304: QtPassSettings::setUsePass currently calls
pass.reset(getRealPass()) / pass.reset(getImitatePass()) which reuses pointers
owned elsewhere and causes double-free; change the ownership semantics so
setUsePass takes ownership safely — either make getRealPass() and
getImitatePass() return a std::unique_ptr (or return by value) and then assign
via move (e.g., pass = getRealPass()), or if the getters must return raw
pointers return a fresh copy and call pass.reset(new ...) to own a distinct
object; update the getPass()/getRealPass()/getImitatePass() implementations
accordingly to eliminate shared ownership of the same pointer.
- Around line 171-183: The current getPass() causes double ownership by calling
pass.reset(getRealPass()/getImitatePass()) where getRealPass() returns
realPass.data() (owned by QScopedPointer<RealPass> realPass), so both realPass
and pass will delete the same object; change the ownership model by making
member pass a non-owning raw Pass* (revert from QScopedPointer) and in getPass()
assign pass = getRealPass() or pass = getImitatePass() (do not call reset or
take ownership), keep the existing null check and call pass->init() only when
pass is non-null, and ensure any code that previously relied on pass owning
lifetime is updated to use the real QScopedPointer owners (realPass/imitatePass)
instead of pass.

In `@src/qtpasssettings.h`:
- Around line 49-51: The header introduced double-ownership by making "pass" a
QScopedPointer<Pass>; revert "pass" to a non-owning raw pointer (Pass*) so it
does not try to take ownership of objects returned by getRealPass() /
getImitatePass(), or alternatively refactor ownership so only one QScopedPointer
owns the object and other accessors return raw pointers/references; locate the
declarations static QScopedPointer<Pass> pass, static QScopedPointer<RealPass>
realPass, static QScopedPointer<ImitatePass> imitatePass and the uses of
pass.reset(getRealPass()) / pass.reset(getImitatePass()) and change "pass" to
Pass* (and remove/reset calls that transfer ownership) or convert
realPass/imitatePass to unique owners and make pass a non-owning alias
consistently.
🪄 Autofix (Beta)

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 (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 22a2d228-c74d-4c3e-acd5-9e3b6a60b5a6

📥 Commits

Reviewing files that changed from the base of the PR and between cd389dd and 3cb933e.

📒 Files selected for processing (2)
  • src/qtpasssettings.cpp
  • src/qtpasssettings.h

Comment thread src/qtpasssettings.cpp
Comment thread src/qtpasssettings.cpp
Comment thread src/qtpasssettings.h Outdated
Comment thread src/qtpasssettings.cpp Outdated
Comment thread src/qtpasssettings.cpp Outdated
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai Bot commented Apr 10, 2026

CodeAnt AI finished reviewing your PR.

@nogeenharrie nogeenharrie force-pushed the fix/qtpasssettings-ai-findings branch from 3cb933e to 49632af Compare April 10, 2026 12:16
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 10, 2026

Coverage Status

Coverage is 20.907%fix/qtpasssettings-ai-findings into main. No base build found for main.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In `@src/qtpasssettings.cpp`:
- Around line 297-303: The current setUsePass in QtPassSettings assigns pass to
getRealPass()/getImitatePass(), which creates an uninitialized backend alias and
bypasses the lazy init in getPass(); change setUsePass (method
QtPassSettings::setUsePass) to not bind pass directly but clear/reset the
existing pass pointer/alias (e.g. set pass to null/clear cached backend) after
updating the stored setting (getInstance()->setValue(SettingsConstants::usePass,
usePass)) so that subsequent calls to QtPassSettings::getPass() will go through
the existing select-and-init path (invoking init()) and return a fully
initialized backend; ensure references to getRealPass/getImitatePass are only
used inside getPass/init logic, not assigned here.
🪄 Autofix (Beta)

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 (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3c563513-68e1-4038-bd5e-fcc767da4c23

📥 Commits

Reviewing files that changed from the base of the PR and between 3cb933e and 49632af.

📒 Files selected for processing (2)
  • src/qtpasssettings.cpp
  • src/qtpasssettings.h

Comment thread src/qtpasssettings.cpp
- Keep Pass* as non-owning pointer to avoid double-free
- Replace foreach with std::as_const range-based for loops
- Add null check before calling pass->init()
- Clear pass pointer in setUsePass() so next getPass() reinitializes
@nogeenharrie nogeenharrie force-pushed the fix/qtpasssettings-ai-findings branch from 49632af to 9966c6c Compare April 10, 2026 12:27
@annejan annejan merged commit e88f774 into main Apr 10, 2026
22 of 24 checks passed
@annejan annejan deleted the fix/qtpasssettings-ai-findings branch April 10, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants