fix(macos): return false from isAppSetupCompleted for empty URL

[eulicesl]

Summary

Fixes a latent correctness bug in APIClient.isAppSetupCompleted(url:uid:) (macOS desktop app). When passed an empty URL, the method returned true ("setup completed") — the wrong default. An empty/unknown URL means setup cannot be verified, so the safe answer is false, consistent with every other failure path in the method (invalid URL, network error, missing/false JSON field all return false).

Returning true would, if reached, wrongly mark an unconfigured external integration app as already set up and auto-enable it.

Why this is safe

Both current callers in MainWindow/Pages/AppsPage.swift already guard !completionUrl.isEmpty before calling this method (lines ~2710 and ~2777), so the bad branch is not reachable from existing callers. This means the fix:

• corrects the latent default and hardens the method's contract for any future caller, and
• changes no existing runtime behavior (current callers never pass an empty URL).

Changes

• desktop/macos/Desktop/Sources/APIClient.swift — guard !url.isEmpty else { return true } → return false (+ explanatory comment).
• desktop/macos/Desktop/Tests/APIClientSetupCompletedTests.swift (new) — regression test asserting the empty-URL contract. The empty-URL case short-circuits before any network request, so no HTTP mocking is needed.

Testing

• This is a macOS-only (AppKit/SwiftUI) target; it builds on macOS / Codemagic CI. The change is a one-line default flip plus a network-free unit test.
• No CHANGELOG.json entry: internal-only correctness fix with no user-visible behavior change (per desktop/macos/AGENTS.md "skip internal-only changes").

Overlap check

No conflict with any open or recently merged PR. The only other open PR touching APIClient.swift is #7673 (trial opt-in), which edits a different method ~800 lines away — merges cleanly.

🤖 Generated with Claude Code

---

Generated by Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[greptile-apps]

Greptile Summary

Fixes a latent correctness bug in APIClient.isAppSetupCompleted(url:uid:) where an empty URL previously returned true ("setup completed") instead of false. A companion regression test verifies the corrected contract.

• APIClient.swift: The guard !url.isEmpty else { return true } guard is changed to return false, making the empty-URL path consistent with the invalid-URL, network-error, and missing-JSON-field paths that already return false. An explanatory comment is added.
• APIClientSetupCompletedTests.swift (new): Adds a network-free XCTest async unit test asserting that an empty URL returns false, exercising only the short-circuit path with no HTTP mocking required.

Confidence Score: 5/5

Safe to merge — a one-line default flip with no effect on existing callers (both already guard against empty URLs) and a clear regression test.

The change corrects a wrong default that was never reachable from existing callers, aligns the empty-URL path with every other failure path in the same method, and is covered by a new focused unit test. No behavioral change to current call sites.

No files require special attention.

Important Files Changed

Filename	Overview
desktop/macos/Desktop/Sources/APIClient.swift	One-line fix: return true → return false for the empty-URL guard, with an explanatory comment added. All other failure paths in the same method already return false; this aligns the default.
desktop/macos/Desktop/Tests/APIClientSetupCompletedTests.swift	New regression test asserting that an empty URL returns false; uses APIClient.shared against the real singleton but correctly relies only on the short-circuit path, so no HTTP mocking is needed.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller as AppsPage (caller)
    participant Guard as isEmpty guard
    participant Network as HTTP request

    Caller->>Guard: isAppSetupCompleted(url: "", uid: ...)
    Note over Guard: url.isEmpty == true
    Guard-->>Caller: return false ✅ (was: return true ❌)

    Caller->>Guard: isAppSetupCompleted(url: "https://...", uid: ...)
    Note over Guard: url.isEmpty == false
    Guard->>Network: "GET url?uid=..."
    Network-->>Guard: JSON response
    Guard-->>Caller: json["is_setup_completed"] ?? false

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller as AppsPage (caller)
    participant Guard as isEmpty guard
    participant Network as HTTP request

    Caller->>Guard: isAppSetupCompleted(url: "", uid: ...)
    Note over Guard: url.isEmpty == true
    Guard-->>Caller: return false ✅ (was: return true ❌)

    Caller->>Guard: isAppSetupCompleted(url: "https://...", uid: ...)
    Note over Guard: url.isEmpty == false
    Guard->>Network: "GET url?uid=..."
    Network-->>Guard: JSON response
    Guard-->>Caller: json["is_setup_completed"] ?? false

Loading

_{Reviews (1): Last reviewed commit: "test(macos): cover empty-URL contract fo..." | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR fixes a latent correctness issue in the macOS desktop APIClient.isAppSetupCompleted(url:uid:) helper by returning false (not completed) when passed an empty completion URL, aligning it with the method’s other failure paths and preventing accidental auto-enablement of unconfigured integrations.

Changes:

• Flip the empty-URL early return in isAppSetupCompleted(url:uid:) from true to false, with an explanatory comment.
• Add a regression unit test asserting the empty-URL contract (no network mocking needed due to short-circuiting).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
desktop/macos/Desktop/Sources/APIClient.swift	Corrects empty-URL behavior in isAppSetupCompleted(url:uid:) to return false consistently with other failure paths.
desktop/macos/Desktop/Tests/APIClientSetupCompletedTests.swift	Adds regression coverage for the empty-URL behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kodjima33]

Desktop/macOS bug fix: empty completion URL must report setup NOT completed (matches every other failure path); has regression test. Owner-override merge.