Offer to init repo when adding existing repo fails (#1799)

[SaurabhCodesAI]

Description

When ExistingRepoWindow gets a "does not exist" or "is not a valid repository" error from borg, prompt the user to initialize a new repo at that location. If accepted, open AddRepoWindow pre filled with the URL and name from the original dialog. If declined, fall through to the normal error message.

Changes:

• ExistingRepoWindow: added init_requested signal and run_result() override that detects repo not found errors and shows a confirmation dialog
• RepoTab: connected init_requested, added _open_init_from_existing() which opens AddRepoWindow pre filled and sets local/remote mode correctly
• Tests: 5 parametrized cases covering both error strings, user accept/decline, unrelated errors, and pre-fill verification

Note: #1816 addresses the same issue but has been inactive since mid 2024. This is an independent implementation using a signal-based approach with test coverage.

Related Issue

Closes #1799

Motivation and Context

Users who try to add a non existent or uninitialized path via "Existing Repository" get a generic "Unable to add your repository" error with no guidance. This change detects that specific failure and offers to open the init dialog instead, reducing confusion between the two repo-adding workflows.

How Has This Been Tested?

• Manually tested on Ubuntu (WSL) with borg 1.2.8
• Tested both scenarios: path that doesn't exist, and path that exists but isn't a borg repo
• Both local paths and remote URLs handled correctly
• 5 unit tests added covering: both borg error strings (parametrized), user accepting init, user declining init, unrelated errors not triggering the prompt, and pre fill verification for local paths
• All existing unit tests pass alongside the 5 new ones (28 total)

Screenshots (if appropriate):

Types of changes

• Bug fix (non breaking change which fixes an issue)
• New feature (non breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have read the CONTRIBUTING guide.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

[SaurabhCodesAI]

@m3nu can u review?

[m3nu]

If we add this, why not remove the distinction between new/existing repo?

[SaurabhCodesAI]

If we add this, why not remove the distinction between new/existing repo?

@m3nu
yeah sure good point, unifying them into one flow that auto detects would be cleaner...

[SaurabhCodesAI]

@m3nu please review?

[m3nu]

PR Review

Design Concern: UX Regression for "Add Existing Repo" Flow

The old ExistingRepoWindow had a deliberately simpler UI:

• Single password field (no confirmation needed — you're entering an existing password)
• No encryption selector (encryption is auto-detected from the repo)
• Title: "Connect to existing Repository"

The unified AddRepoWindow always shows dual password fields with min 9-char validation, an encryption selector dropdown, and a generic "Add Repository" title. When a user just wants to connect to an existing repo, the encryption selector and password confirmation are confusing and irrelevant.

Suggestion: Consider hiding the encryption combobox and confirm password field initially, and only showing them if the user accepts the "init" prompt.

---

Bug: RepoWindow.validate(self) — Explicit Parent Call Bypasses Polymorphism

def run(self):
    if not RepoWindow.validate(self):  # <-- explicit parent class call
        return

This deliberately bypasses AddRepoWindow.validate() (which includes self.passwordInput.validate()) to skip password validation during the probe phase. This is fragile and non-idiomatic — readers expect self.validate(), and future modifications to the validation chain won't behave as expected.

Suggestion: Extract a _validate_repo_fields() method for URL/name validation, separate from validate().

---

UX Issue: Password Validation Timing for Init Path

When the probe fails and the user accepts "Initialize a new one?", the code calls self.passwordInput.validate(). But the user likely only entered a password in the first field (expecting to connect to an existing repo). They'll get a confusing "Passwords must be identical" error with no guidance about why confirmation is suddenly needed.

Suggestion: Show a clear message when switching from probe to init mode, e.g. "Please enter and confirm a password for the new repository."

---

Test Issue: test_repo_add_name_validation Assertion Change

# Old:
('test_repo_name', ''),       # valid repo name — no error
# New:
('test_repo_name', 'Checking repository…'),  # valid repo name — shows progress

The parameter is called error_text but 'Checking repository…' is not an error — it's a progress message. This masks the behavior change and makes the test assert on transient status rather than validation state.

---

Missing Error Detail in Status Messages

When the probe fails with a non-"does not exist" error (e.g., "Connection refused", "Permission denied"), the actual borg error from result['errors'] is discarded in favor of generic "Unable to add your repository." Including the borg error message would help users diagnose the issue.

---

Minor: Translation Contexts Orphaned

The i18n .ts files (12+ languages) reference ExistingRepoWindow as a translation context. Removing the class orphans those translations.

---

What Looks Good

• The probe-then-init approach is a solid UX improvement over making users guess
• Error string detection ('does not exist', 'is not a valid repository') covers known borg messages
• Good test coverage with parametrized error strings, accept/decline paths, unrelated errors, and probe success
• Clean removal of ExistingRepoWindow with no dangling references in production code
• _init_repo() correctly delegates to self.run_result for result handling
• Integration test properly mocks QMessageBox.question for the new confirmation step

[SaurabhCodesAI]

PR Review

Design Concern: UX Regression for "Add Existing Repo" Flow

The old ExistingRepoWindow had a deliberately simpler UI:

• Single password field (no confirmation needed — you're entering an existing password)
• No encryption selector (encryption is auto-detected from the repo)
• Title: "Connect to existing Repository"

The unified AddRepoWindow always shows dual password fields with min 9-char validation, an encryption selector dropdown, and a generic "Add Repository" title. When a user just wants to connect to an existing repo, the encryption selector and password confirmation are confusing and irrelevant.

Suggestion: Consider hiding the encryption combobox and confirm password field initially, and only showing them if the user accepts the "init" prompt.

Bug: RepoWindow.validate(self) — Explicit Parent Call Bypasses Polymorphism

def run(self):
    if not RepoWindow.validate(self):  # <-- explicit parent class call
        return

This deliberately bypasses AddRepoWindow.validate() (which includes self.passwordInput.validate()) to skip password validation during the probe phase. This is fragile and non-idiomatic — readers expect self.validate(), and future modifications to the validation chain won't behave as expected.

Suggestion: Extract a _validate_repo_fields() method for URL/name validation, separate from validate().

UX Issue: Password Validation Timing for Init Path

When the probe fails and the user accepts "Initialize a new one?", the code calls self.passwordInput.validate(). But the user likely only entered a password in the first field (expecting to connect to an existing repo). They'll get a confusing "Passwords must be identical" error with no guidance about why confirmation is suddenly needed.

Suggestion: Show a clear message when switching from probe to init mode, e.g. "Please enter and confirm a password for the new repository."

Test Issue: test_repo_add_name_validation Assertion Change

# Old:
('test_repo_name', ''),       # valid repo name — no error
# New:
('test_repo_name', 'Checking repository…'),  # valid repo name — shows progress

The parameter is called error_text but 'Checking repository…' is not an error — it's a progress message. This masks the behavior change and makes the test assert on transient status rather than validation state.

Missing Error Detail in Status Messages

When the probe fails with a non-"does not exist" error (e.g., "Connection refused", "Permission denied"), the actual borg error from result['errors'] is discarded in favor of generic "Unable to add your repository." Including the borg error message would help users diagnose the issue.

Minor: Translation Contexts Orphaned

The i18n .ts files (12+ languages) reference ExistingRepoWindow as a translation context. Removing the class orphans those translations.

What Looks Good

• The probe-then-init approach is a solid UX improvement over making users guess
• Error string detection ('does not exist', 'is not a valid repository') covers known borg messages
• Good test coverage with parametrized error strings, accept/decline paths, unrelated errors, and probe success
• Clean removal of ExistingRepoWindow with no dangling references in production code
• _init_repo() correctly delegates to self.run_result for result handling
• Integration test properly mocks QMessageBox.question for the new confirmation step

@m3nu Thanks, these are all fair points. Learned a few things here I'll fix them up:

1. Hide encryption + confirm until init is actually needed
2. Pull out a _validate_repo_fields() instead of the weird parent call
3. Show a clear message when switching to init so the confirm field makes sense
4. Fix the test param name, you're right it's not an error
5. Show the actual borg error instead of the generic one
6. Translations I'll leave for the next sync

I will Push fixes shortly, will keep it as a separate commit so it's easy to review.

[SaurabhCodesAI]

@m3nu i would need your guidance one thing i am unsure about for point 3, once the user says yes to init,
i will show the confirm field and encryption picker. but should i also clear
the password they already entered, or keep it and just have them re type it
in confirm?

[m3nu]

You can decide it. You just look at it from a user perspective.

[SaurabhCodesAI]

You can decide it. You just look at it from a user perspective.

sure.

[SaurabhCodesAI]

@m3nu sorry for the delay i spent the week going through the codebase to get fully up to speed before the GSoC contribution window opens...

Just pushed a new commit with the fixes:

Hid init widgets by default.
Fixed the validate() bypass.
UI now surfaces raw borg errors.
for the password transition, i left their first password input intact so they only have to type the confirmation.