ci: extract each APP_VERSION_* component instead of relying on macro order - #11
Merged
Merged
Conversation
Pressing OK could call CheckAndApplyAutoSwitch() three times: once in the auto-switch settings block, once in the base-mouse-count block, and once in the forced re-apply after the cached device state is discarded. Each call enumerates raw input devices, retrying GetRawInputDeviceList up to three times, so a single click cost up to three enumerations. The repeats were harmless — the direction is persisted before all three calls and ApplyMouseOrientation() sets an absolute value rather than toggling — but only the last call could do useful work in the common case, since the first two early-return on unchanged state. Remove the two earlier calls and keep the forced re-apply at the end of the handler. By that point every setting the check reads is persisted, so one pass applies them all; the first call previously ran before SetBaseMouseCount(), so it could not see a changed base count anyway. Dropping the base-mouse-count call also stops a re-apply from running when the auto-switch flag itself failed to persist: that call was gated on autoSwitchEnabled alone, not on autoSwitchWritten. The surviving call is gated on both, matching 3a00a42. Closes #2 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jonathan Bnayahu <bnayahu@il.ibm.com>
build.yml declared no permissions: block, so the workflow ran with the default GITHUB_TOKEN permissions. The job only checks out the repository, builds, and uploads an artifact, so read access to contents is all it needs. actions/upload-artifact authenticates with the Actions runtime token rather than GITHUB_TOKEN, so it is unaffected. Nothing was broken; this narrows the token to what the job actually uses. The comment warns against copying the block into release.yml, which genuinely needs contents: write for `gh release create`. Closes #3 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jonathan Bnayahu <bnayahu@il.ibm.com>
…order Both workflows derived the source version with a single alternation piped through `paste`: grep -oP 'APP_VERSION_(MAJOR|MINOR|PATCH)\s+\K\d+' ... | paste -sd. That takes the macros in the order they appear in the header rather than in semantic order. They currently appear as MAJOR, MINOR, PATCH on adjacent lines, so the result is correct today, but nothing enforced it — reordering them silently produced a transposed version string. The existing failure mode was fail-closed: release.yml's tag gate rejects a mismatch loudly rather than publishing something mislabelled. This makes the extraction correct in the first place, so a reorder is a non-event instead of a confusing release failure. Extract each component by name, and assert all three are non-empty — the old form would emit a short "1.2" if a macro went missing, which could then match a substring of the binary's version strings. Closes #6 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jonathan Bnayahu <bnayahu@il.ibm.com>
bnayahu
force-pushed
the
ci/order-independent-version-extraction
branch
from
August 11, 2026 18:40
2cf5c92 to
2125cce
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6
Problem
Both workflows derived the source version with one alternation piped through
paste:This takes the macros in the order they appear in the header, not in semantic order. They currently appear as MAJOR, MINOR, PATCH on adjacent lines, so it is correct today — but nothing enforced the ordering.
As the issue notes, the failure mode was fail-closed: a transposed version no longer matches the tag, so
release.yml's gate rejects the release rather than publishing something mislabelled. This makes the extraction correct in the first place, so a reorder becomes a non-event instead of a confusing release failure.Change
Extract each component by name, in both
build.ymlandrelease.yml, and assert all three are non-empty.The non-empty guard covers a second sharp edge in the old form: with a macro missing it emitted a short
1.2rather than failing, and1.2can match a substring of the binary's version strings — sostrings ... | grep -qF "$src"could pass on an incomplete read.Verification
Ran both forms against mutated copies of the header:
1.2.31.2.33.2.1❌1.2.3✅APP_VERSION_MINORmoved elsewhere in the file2.1.3❌1.2.3✅APP_VERSION_PATCHdeleted1.2❌ silently shortAlso confirmed end-to-end on the real tree:
APP_VERSION_A/APP_VERSIONstill do not false-match — extraction yields1.0.0.build.yml's step run verbatim against the freshly builtPrimary.exe: source version1.0.0, binary reports it, step passes.release.yml's tag gate simulated both ways:v1.0.0passes,v9.9.9is rejected.Note on overlap
#5's PR (#10) adds a manifest-version check that already uses this per-macro form, so the two are consistent once both land. They touch different steps; if #10 merges first this may need a trivial rebase.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com