From 6233ffb1cd042ed6c1c727418b0cd85aef9b4f05 Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Sun, 3 May 2026 21:08:24 +0200 Subject: [PATCH] fix(updatenotification): fix ref diff parsing for fetch --dry-run The regex used to extract the commit range from fetch dry-run output matched the full line, including the branch name. This caused git rev-list to receive an ambiguous argument (e.g. "60e0377..332e429 develop" instead of "60e0377..332e429"), breaking update checks. Replace regex-based parsing with token-based line parsing that extracts only the first whitespace-delimited token, with a fallback to ..origin/ when fetch reports no changes. Add unit tests for the new parsing helper and correct existing test snapshots that previously encoded the broken behavior. --- .../updatenotification/git_helper.js | 36 ++++++++------ .../unit/functions/updatenotification_spec.js | 48 +++++++++++++------ 2 files changed, 56 insertions(+), 28 deletions(-) diff --git a/defaultmodules/updatenotification/git_helper.js b/defaultmodules/updatenotification/git_helper.js index bfc51e4809..1e199d4ce1 100644 --- a/defaultmodules/updatenotification/git_helper.js +++ b/defaultmodules/updatenotification/git_helper.js @@ -10,8 +10,26 @@ class GitHelper { this.gitResultList = []; } - getRefRegex (branch) { - return new RegExp(`s*([a-z,0-9]+[.][.][a-z,0-9]+) ${branch}`, "g"); + /** + * Extract commit range (..) for the current branch from `git fetch -n --dry-run` output. + * Falls back to `..origin/` when no matching update line is found. + * @param {string} fetchOutput fetch dry-run stderr output + * @param {string} currentBranch currently checked local branch + * @returns {string} commit range for rev-list + */ + getRefDiffFromFetchDryRun (fetchOutput, currentBranch) { + const fallbackRefDiff = `${currentBranch}..origin/${currentBranch}`; + + for (const line of fetchOutput.split("\n")) { + const columns = line.trim().split(/\s+/); + const [refDiff, branchName] = columns; + + if (branchName === currentBranch && refDiff?.includes("..")) { + return refDiff; + } + } + + return fallbackRefDiff; } async execGit (moduleFolder, ...args) { @@ -123,20 +141,10 @@ class GitHelper { return gitInfo; } + // Git writes fetch dry-run updates to stderr. const { stderr } = await this.execGit(repo.folder, "fetch", "-n", "--dry-run"); - // example output: - // From https://github.com/MagicMirrorOrg/MagicMirror - // e40ddd4..06389e3 develop -> origin/develop - // here the result is in stderr (this is a git default, don't ask why ...) - const matches = stderr.match(this.getRefRegex(gitInfo.current)); - - // this is the default if there was no match from "git fetch -n --dry-run". - // Its a fallback because if there was a real "git fetch", the above "git fetch -n --dry-run" would deliver nothing. - let refDiff = `${gitInfo.current}..origin/${gitInfo.current}`; - if (matches && matches[0]) { - refDiff = matches[0]; - } + const refDiff = this.getRefDiffFromFetchDryRun(stderr, gitInfo.current); // get behind with refs try { diff --git a/tests/unit/functions/updatenotification_spec.js b/tests/unit/functions/updatenotification_spec.js index c5d29c570b..f72e7ccc63 100644 --- a/tests/unit/functions/updatenotification_spec.js +++ b/tests/unit/functions/updatenotification_spec.js @@ -103,6 +103,26 @@ describe("Updatenotification", () => { vi.resetAllMocks(); }); + describe("getRefDiffFromFetchDryRun", () => { + it("extracts only commit range from matching fetch line", () => { + const fetchOutput = "From github.com:MagicMirrorOrg/MagicMirror\n60e0377..332e429 develop -> origin/develop\n"; + + expect(gitHelper.getRefDiffFromFetchDryRun(fetchOutput, "develop")).toBe("60e0377..332e429"); + }); + + it("matches branch name exactly", () => { + const fetchOutput = "From github.com:MagicMirrorOrg/MagicMirror\n1111111..2222222 main-feature -> origin/main-feature\n3333333..4444444 main -> origin/main\n"; + + expect(gitHelper.getRefDiffFromFetchDryRun(fetchOutput, "main")).toBe("3333333..4444444"); + }); + + it("returns fallback range when matching line is missing", () => { + const fetchOutput = "From github.com:MagicMirrorOrg/MagicMirror\n"; + + expect(gitHelper.getRefDiffFromFetchDryRun(fetchOutput, "develop")).toBe("develop..origin/develop"); + }); + }); + describe("MagicMirror on develop", () => { const moduleName = "MagicMirror"; @@ -124,7 +144,7 @@ describe("Updatenotification", () => { "rev-parse HEAD", "status -sb", "fetch -n --dry-run", - "rev-list --ancestry-path --count 60e0377..332e429 develop", + "rev-list --ancestry-path --count 60e0377..332e429", ] `); }); @@ -174,9 +194,9 @@ describe("Updatenotification", () => { "rev-parse HEAD", "status -sb", "fetch -n --dry-run", - "rev-list --ancestry-path --count 60e0377..332e429 master", + "rev-list --ancestry-path --count 60e0377..332e429", "ls-remote -q --tags --refs", - "rev-list --ancestry-path 60e0377..332e429 master", + "rev-list --ancestry-path 60e0377..332e429", ] `); }); @@ -191,9 +211,9 @@ describe("Updatenotification", () => { "rev-parse HEAD", "status -sb", "fetch -n --dry-run", - "rev-list --ancestry-path --count 60e0377..332e429 master", + "rev-list --ancestry-path --count 60e0377..332e429", "ls-remote -q --tags --refs", - "rev-list --ancestry-path 60e0377..332e429 master", + "rev-list --ancestry-path 60e0377..332e429", ] `); }); @@ -230,9 +250,9 @@ describe("Updatenotification", () => { "rev-parse HEAD", "status -sb", "fetch -n --dry-run", - "rev-list --ancestry-path --count 60e0377..332e429 master", + "rev-list --ancestry-path --count 60e0377..332e429", "ls-remote -q --tags --refs", - "rev-list --ancestry-path 60e0377..332e429 master", + "rev-list --ancestry-path 60e0377..332e429", ] `); }); @@ -247,9 +267,9 @@ describe("Updatenotification", () => { "rev-parse HEAD", "status -sb", "fetch -n --dry-run", - "rev-list --ancestry-path --count 60e0377..332e429 master", + "rev-list --ancestry-path --count 60e0377..332e429", "ls-remote -q --tags --refs", - "rev-list --ancestry-path 60e0377..332e429 master", + "rev-list --ancestry-path 60e0377..332e429", ] `); }); @@ -286,9 +306,9 @@ describe("Updatenotification", () => { "rev-parse HEAD", "status -sb", "fetch -n --dry-run", - "rev-list --ancestry-path --count 60e0377..332e429 master", + "rev-list --ancestry-path --count 60e0377..332e429", "ls-remote -q --tags --refs", - "rev-list --ancestry-path 60e0377..332e429 master", + "rev-list --ancestry-path 60e0377..332e429", ] `); }); @@ -303,9 +323,9 @@ describe("Updatenotification", () => { "rev-parse HEAD", "status -sb", "fetch -n --dry-run", - "rev-list --ancestry-path --count 60e0377..332e429 master", + "rev-list --ancestry-path --count 60e0377..332e429", "ls-remote -q --tags --refs", - "rev-list --ancestry-path 60e0377..332e429 master", + "rev-list --ancestry-path 60e0377..332e429", ] `); }); @@ -339,7 +359,7 @@ describe("Updatenotification", () => { [ "status -sb", "fetch -n --dry-run", - "rev-list --ancestry-path --count 19f7faf..9d83101 master", + "rev-list --ancestry-path --count 19f7faf..9d83101", ] `); });