Skip to content

fix(macOS): compare FinderSync broker application versions - #10599

Merged
Rello merged 1 commit into
masterfrom
bugfix/brokerVersion
Aug 13, 2026
Merged

fix(macOS): compare FinderSync broker application versions#10599
Rello merged 1 commit into
masterfrom
bugfix/brokerVersion

Conversation

@Rello

@Rello Rello commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator
  • The FinderSync broker reported CFBundleVersion, which was 0 when MIRALL_VERSION_BUILD was not supplied (which is the case in brander as of qt6)

  • The client compared this with its full bundle version, causing a false mismatch on every startup:

    • Broker is running version "0" but we are "34.0.50.0" - restarting it
    • Already restarted the broker for version "0" and it came back the same
  • Use CFBundleShortVersionString on the broker side and compare it with the client's compiled MIRALL_VERSION. Both sides now compare the application version, such as 34.0.50, without requiring build-ID propagation.

Assisted-by: Codex:GPT-5

- The FinderSync broker reported CFBundleVersion, which was 0 when
MIRALL_VERSION_BUILD was not supplied. The client compared this with its
full bundle version, causing a false mismatch on every startup:

- Broker is running version "0" but we are "34.0.50.0" - restarting it
- Already restarted the broker for version "0" and it came back the same

- Use CFBundleShortVersionString on the broker side and compare it with the
client's compiled MIRALL_VERSION. Both sides now compare the application
version, such as 34.0.50, without requiring build-ID propagation.

Assisted-by: Codex:GPT-5

Assisted-by: Codex:GPT-5
Signed-off-by: Rello <github@scherello.de>
@Rello Rello added this to the 34.0.2 milestone Aug 13, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

馃挕 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2f6b63e81

鈩癸笍 About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 馃憤.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

teamIdentifier: team,
applicationRevDomain: revDomain,
version: info["CFBundleVersion"] as? String ?? "0")
version: info["CFBundleShortVersionString"] as? String ?? "")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve the build identity in broker comparisons

When two packages share a marketing version but have different MIRALL_VERSION_BUILD values鈥攕uch as successive daily or same-version rebuilds鈥攂oth processes now report only MIRALL_VERSION, so the client treats the already-running broker as current and leaves the previous binary executing. VERSION.cmake:40-51 explicitly defines the build number as the per-build identity, so the comparison needs to retain a revision/build component without reintroducing the default-0 mismatch.

Useful? React with 馃憤聽/ 馃憥.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

comparison on build-id is very complex due to adjustments also on the brander. customers only have normal version updates. same is used for the other extensions

// Info.plist reads here use literals too.
const auto ourVersion = QString::fromNSString(
[[NSBundle mainBundle] objectForInfoDictionaryKey:@"CFBundleVersion"]);
const auto ourVersion = QString::fromLatin1(MIRALL_STRINGIFY(MIRALL_VERSION));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add regression coverage for broker version matching

This AI-assisted bugfix changes the version source on both sides but adds no automated coverage for the original false mismatch or boundaries such as distinct builds with the same application version. Extracting the comparison behind a testable seam and adding a regression test is required before this fix can be considered complete.

AGENTS.md reference: AGENTS.md:L207-L209

Useful? React with 馃憤聽/ 馃憥.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

there is no sense in doing a test for something that can technically not happen anymore

@github-actions

Copy link
Copy Markdown
Contributor

Artifact containing the AppImage: nextcloud-appimage-pr-10599.zip

Digest: sha256:5d5699d24aadcc49dc9b3308b8a13b0d02e3b8d67e6f560e8ba798abc2b5008f

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

@Rello Rello self-assigned this Aug 13, 2026
@Rello
Rello merged commit 51306cd into master Aug 13, 2026
20 of 21 checks passed
@Rello
Rello deleted the bugfix/brokerVersion branch August 13, 2026 15:24
@Rello

Rello commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

/backport to stable-34.0

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.

2 participants