Resolve desktop artifact version from HEAD semver tags

[torturado]

Summary

• add a deterministic desktop artifact version resolver for exact semver tags on HEAD
• preserve precedence: explicit build version, HEAD semver tag, apps/server package version
• cover explicit override, no-tag fallback, leading-v stripping, and multiple-tag ordering in focused tests

Credit

This carries forward the tagged desktop artifact version portion of upstream PR pingdotgg#337 while intentionally leaving out the unrelated shell/PATH changes that current main no longer needs.

Fixes #3

Validation

• bun fmt
• bun lint
• bun typecheck
• bun --cwd scripts run test build-desktop-artifact.test.ts

ClawSweeper 🐠 replacement reef notes:

• Cluster: torturado-t3code-issue-3-tagged-artifact-version
• Source PRs: Fix Linux Codex PATH resolution and tagged desktop artifact version pingdotgg/t3code#337
• Credit: Replacement carries forward the useful tagged desktop artifact version idea from upstream PR Fix Linux Codex PATH resolution and tagged desktop artifact version pingdotgg/t3code#337.; Do not copy unrelated fixPath/syncShellEnvironment changes from upstream PR Fix Linux Codex PATH resolution and tagged desktop artifact version pingdotgg/t3code#337.
• Validation: bun run fmt:check; bun run lint; bun run typecheck; bun --cwd scripts run test build-desktop-artifact.test.ts

fish notes: model gpt-5.5, reasoning low; reviewed against d947550.

[torturado]

ClawSweeper status: review started.

I am starting a fresh review of this pull request: Resolve desktop artifact version from HEAD semver tags This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

[torturado]

Codex review: needs changes before merge.

Summary
The PR adds a desktop artifact version resolver that prefers an explicit build version, then semver tags pointing at HEAD, then the server package version, with focused unit tests.

Reproducibility: yes. from source inspection: current main ignores HEAD tags when resolving desktop artifact versions, and the PR’s new multi-tag path can be checked directly through resolveDesktopArtifactVersion inputs.

Next step before merge
A narrow automated repair can replace lexicographic tag ordering with semver-aware ordering and add one regression test on the PR branch.

Security
Cleared: The diff only adds local git tag inspection and unit tests; I found no concrete security or supply-chain regression.

Review findings

• [P2] Use semver ordering for HEAD tag precedence — scripts/build-desktop-artifact.ts:575

Review details

Best possible solution:

Land a tag-aware resolver, but compare candidate tags with real semver precedence and add a regression case for numeric ordering such as 1.10.0 versus 1.2.0.

Do we have a high-confidence way to reproduce the issue?

Yes, from source inspection: current main ignores HEAD tags when resolving desktop artifact versions, and the PR’s new multi-tag path can be checked directly through resolveDesktopArtifactVersion inputs.

Is this the best way to solve the issue?

No, not as written; the precedence idea is narrow and maintainable, but the ordering implementation should use semver comparison rather than localeCompare.

Full review comments:

• [P2] Use semver ordering for HEAD tag precedence — scripts/build-desktop-artifact.ts:575
  The resolver sorts version strings with localeCompare, so numeric semver ordering is wrong for tags like v1.10.0 and v1.2.0; the current code would put 1.2.0 last and use the older release for the artifact metadata. Please parse/compare semver components and cover this case in the tests.
  Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.9

Acceptance criteria:

• bun fmt
• bun lint
• bun typecheck
• bun --cwd scripts run test build-desktop-artifact.test.ts

What I checked:

• Current main fallback: Current main computes desktop appVersion as options.version ?? serverPackageJson.version, so HEAD release tags are ignored when no explicit build version is provided. (scripts/build-desktop-artifact.ts:713, 35721d9a08b2)
• Release docs expect tag-driven releases: The release checklist tells maintainers to create and push a vX.Y.Z release tag, which supports the need for tag-aware desktop artifact metadata. (docs/release.md:158, 35721d9a08b2)
• PR provenance: The proposed change is only on the PR branch, not current main or a release tag. (scripts/build-desktop-artifact.ts:560, d9475507ca88)
• Patch review finding: The PR sorts candidate versions with localeCompare, so examples such as 1.10.0 and 1.2.0 are ordered as strings rather than semantic versions. (scripts/build-desktop-artifact.ts:575, d9475507ca88)

Likely related people:

• Pedro Silva: The desktop artifact build script and its existing version fallback behavior appear to come from the initial current-main commit that added this file. (role: introduced behavior / adjacent owner; confidence: medium; commits: 0d55a4284023; files: scripts/build-desktop-artifact.ts, scripts/build-desktop-artifact.test.ts)

Remaining risk / open question:

• I did not run the validation commands because this review is explicitly read-only; the finding is based on source inspection of the PR diff.

Codex review notes: model gpt-5.5, reasoning low; reviewed against 35721d9a08b2.

[torturado]

Codex review: needs changes before merge.

Summary
The PR adds a desktop artifact version resolver that prefers an explicit build version, then semver tags pointing at HEAD, then the server package version, with focused unit tests.

Reproducibility: yes. from source inspection: current main ignores HEAD tags when resolving desktop artifact versions, and the PR’s new multi-tag path can be checked directly through resolveDesktopArtifactVersion inputs.

Next step before merge
A narrow automated repair can replace lexicographic tag ordering with semver-aware ordering and add one regression test on the PR branch.

Security
Cleared: The diff only adds local git tag inspection and unit tests; I found no concrete security or supply-chain regression.

Review findings

• [P2] Use semver ordering for HEAD tag precedence — scripts/build-desktop-artifact.ts:575

Review details

Best possible solution:

Land a tag-aware resolver, but compare candidate tags with real semver precedence and add a regression case for numeric ordering such as 1.10.0 versus 1.2.0.

Do we have a high-confidence way to reproduce the issue?

Yes, from source inspection: current main ignores HEAD tags when resolving desktop artifact versions, and the PR’s new multi-tag path can be checked directly through resolveDesktopArtifactVersion inputs.

Is this the best way to solve the issue?

No, not as written; the precedence idea is narrow and maintainable, but the ordering implementation should use semver comparison rather than localeCompare.

Full review comments:

• [P2] Use semver ordering for HEAD tag precedence — scripts/build-desktop-artifact.ts:575
  The resolver sorts version strings with localeCompare, so numeric semver ordering is wrong for tags like v1.10.0 and v1.2.0; the current code would put 1.2.0 last and use the older release for the artifact metadata. Please parse/compare semver components and cover this case in the tests.
  Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.9

Acceptance criteria:

• bun fmt
• bun lint
• bun typecheck
• bun --cwd scripts run test build-desktop-artifact.test.ts

What I checked:

• Current main fallback: Current main computes desktop appVersion as options.version ?? serverPackageJson.version, so HEAD release tags are ignored when no explicit build version is provided. (scripts/build-desktop-artifact.ts:713, 35721d9a08b2)
• Release docs expect tag-driven releases: The release checklist tells maintainers to create and push a vX.Y.Z release tag, which supports the need for tag-aware desktop artifact metadata. (docs/release.md:158, 35721d9a08b2)
• PR provenance: The proposed change is only on the PR branch, not current main or a release tag. (scripts/build-desktop-artifact.ts:560, d9475507ca88)
• Patch review finding: The PR sorts candidate versions with localeCompare, so examples such as 1.10.0 and 1.2.0 are ordered as strings rather than semantic versions. (scripts/build-desktop-artifact.ts:575, d9475507ca88)

Likely related people:

• Pedro Silva: The desktop artifact build script and its existing version fallback behavior appear to come from the initial current-main commit that added this file. (role: introduced behavior / adjacent owner; confidence: medium; commits: 0d55a4284023; files: scripts/build-desktop-artifact.ts, scripts/build-desktop-artifact.test.ts)

Remaining risk / open question:

• I did not run the validation commands because this review is explicitly read-only; the finding is based on source inspection of the PR diff.

Codex review notes: model gpt-5.5, reasoning low; reviewed against 35721d9a08b2.

[torturado]

ClawSweeper 🐠 reef update

Thanks for the contribution here. ClawSweeper pushed the narrow repair here so review, credit, and history stay together.

Validation: bun run fmt:check; bun run lint; bun run typecheck; bun --cwd scripts run test build-desktop-artifact.test.ts
The contribution stays credited here. no vanishing act, no suspicious bubbles.

fish notes: model gpt-5.5, reasoning low; reviewed against 314437c.

[torturado]

Codex review: needs maintainer review before merge.

Summary
The PR adds HEAD semver tag discovery to the desktop artifact build script, resolves artifact versions with explicit-version > HEAD tag > server package precedence, and adds focused resolver tests.

Reproducibility: yes. from source inspection: current main ignores HEAD tags when resolving desktop artifact versions, and the PR exposes a pure resolver path that exercises explicit, tag, and fallback precedence.

Next step before merge
The PR is owner-authored and appears review-clean after the automated repair, so the remaining action is normal maintainer approval and CI gating rather than a new fix PR.

Security
Cleared: The diff only adds local git tag inspection and unit tests, with no concrete security or supply-chain regression found.

Review details

Best possible solution:

Have a maintainer review and merge the repaired tag-aware resolver if CI passes, preserving the narrower scope that excludes unrelated upstream shell/PATH changes.

Do we have a high-confidence way to reproduce the issue?

Yes, from source inspection: current main ignores HEAD tags when resolving desktop artifact versions, and the PR exposes a pure resolver path that exercises explicit, tag, and fallback precedence.

Is this the best way to solve the issue?

Yes, the latest PR head is a narrow maintainable solution: it keeps the build-script behavior local, uses semver-aware ordering, and adds focused tests for the corrected precedence rules.

Acceptance criteria:

• bun fmt
• bun lint
• bun typecheck
• bun --cwd scripts run test build-desktop-artifact.test.ts

What I checked:

• Current main ignores HEAD tags: Current main computes the desktop appVersion from options.version ?? serverPackageJson.version, so tag-driven release versions are not considered. (scripts/build-desktop-artifact.ts:713, 35721d9a08b2)
• Release checklist uses semver tags: The release docs instruct maintainers to create and push a vX.Y.Z release tag, which supports using HEAD tags for release artifact metadata. (docs/release.md:159, 35721d9a08b2)
• PR adds tag-aware resolver: The PR adds resolveGitHeadTags and resolveDesktopArtifactVersion, then uses them in buildDesktopArtifact to prefer explicit versions, HEAD semver tags, then the package fallback. (scripts/build-desktop-artifact.ts:619, 314437c5268e)
• Prior review finding repaired: The latest PR head now compares semver components and includes a 1.2.0 versus 1.10.0 regression test, addressing the earlier localeCompare ordering finding. (scripts/build-desktop-artifact.test.ts:48, 314437c5268e)
• Feature history: The desktop artifact build script and its existing version fallback behavior appear to come from the initial current-main commit that added this file. (scripts/build-desktop-artifact.ts:713, 0d55a4284023)

Likely related people:

• Pedro Silva: The current desktop artifact script, tests, and original package-version fallback all trace to the initial current-main commit for this area. (role: introduced behavior / adjacent owner; confidence: medium; commits: 0d55a4284023; files: scripts/build-desktop-artifact.ts, scripts/build-desktop-artifact.test.ts)

Remaining risk / open question:

• I did not run validation commands because this review is explicitly read-only; the verdict is based on source and diff inspection.

Codex review notes: model gpt-5.5, reasoning low; reviewed against 35721d9a08b2.

[torturado]

Codex review: needs maintainer review before merge.

Summary
The PR adds HEAD semver tag discovery to the desktop artifact build script, resolves artifact versions with explicit-version > HEAD tag > server package precedence, and adds focused resolver tests.

Reproducibility: yes. from source inspection: current main ignores HEAD tags when resolving desktop artifact versions, and the PR exposes a pure resolver path that exercises explicit, tag, and fallback precedence.

Next step before merge
The PR is owner-authored and appears review-clean after the automated repair, so the remaining action is normal maintainer approval and CI gating rather than a new fix PR.

Security
Cleared: The diff only adds local git tag inspection and unit tests, with no concrete security or supply-chain regression found.

Review details

Best possible solution:

Have a maintainer review and merge the repaired tag-aware resolver if CI passes, preserving the narrower scope that excludes unrelated upstream shell/PATH changes.

Do we have a high-confidence way to reproduce the issue?

Yes, from source inspection: current main ignores HEAD tags when resolving desktop artifact versions, and the PR exposes a pure resolver path that exercises explicit, tag, and fallback precedence.

Is this the best way to solve the issue?

Yes, the latest PR head is a narrow maintainable solution: it keeps the build-script behavior local, uses semver-aware ordering, and adds focused tests for the corrected precedence rules.

Acceptance criteria:

• bun fmt
• bun lint
• bun typecheck
• bun --cwd scripts run test build-desktop-artifact.test.ts

What I checked:

• Current main ignores HEAD tags: Current main computes the desktop appVersion from options.version ?? serverPackageJson.version, so tag-driven release versions are not considered. (scripts/build-desktop-artifact.ts:713, 35721d9a08b2)
• Release checklist uses semver tags: The release docs instruct maintainers to create and push a vX.Y.Z release tag, which supports using HEAD tags for release artifact metadata. (docs/release.md:159, 35721d9a08b2)
• PR adds tag-aware resolver: The PR adds resolveGitHeadTags and resolveDesktopArtifactVersion, then uses them in buildDesktopArtifact to prefer explicit versions, HEAD semver tags, then the package fallback. (scripts/build-desktop-artifact.ts:619, 314437c5268e)
• Prior review finding repaired: The latest PR head now compares semver components and includes a 1.2.0 versus 1.10.0 regression test, addressing the earlier localeCompare ordering finding. (scripts/build-desktop-artifact.test.ts:48, 314437c5268e)
• Feature history: The desktop artifact build script and its existing version fallback behavior appear to come from the initial current-main commit that added this file. (scripts/build-desktop-artifact.ts:713, 0d55a4284023)

Likely related people:

• Pedro Silva: The current desktop artifact script, tests, and original package-version fallback all trace to the initial current-main commit for this area. (role: introduced behavior / adjacent owner; confidence: medium; commits: 0d55a4284023; files: scripts/build-desktop-artifact.ts, scripts/build-desktop-artifact.test.ts)

Remaining risk / open question:

• I did not run validation commands because this review is explicitly read-only; the verdict is based on source and diff inspection.

Codex review notes: model gpt-5.5, reasoning low; reviewed against 35721d9a08b2.