Skip to content

Pin swag version in swagger-install task#5802

Draft
jerm-dro wants to merge 1 commit into
mainfrom
jerm-dro/01KXH2R7Z0XC6209KY653NC0TG
Draft

Pin swag version in swagger-install task#5802
jerm-dro wants to merge 1 commit into
mainfrom
jerm-dro/01KXH2R7Z0XC6209KY653NC0TG

Conversation

@jerm-dro

Copy link
Copy Markdown
Contributor

Summary

  • task docs (via the swagger-install task) installs swag with go install .../swag@latest, so every run resolves to whatever swag release is newest that day — independent of the version already pinned in go.mod. If a future swag release changes internal schema-name disambiguation behavior (or anything else about generation), two contributors running task docs on different days, or one contributor rebasing weeks later, can produce a divergent docs/server/{docs.go,swagger.json,swagger.yaml} with no underlying API change, masquerading as a large, unrelated diff on their PR (as happened on Rate limiting observability (metrics and tracing) PR B #5800, which tripped the "Large PR Detected" gate).
  • This PR resolves the swag version from go.mod (go list -m github.com/swaggo/swag/v2) instead of @latest, so swagger-install always builds the same generator that the rest of the module graph is pinned to.

Investigation notes (for reviewers)

While root-causing #5800's diff, I confirmed a few things worth recording here so this isn't re-litigated later:

  • JSON/YAML key ordering is already deterministic. encoding/json.Marshal sorts map[string]... keys alphabetically, and swag's YAML output goes through sigs.k8s.io/yaml (gopkg.in/yaml.v2), which also sorts map keys. Five to six consecutive fresh swag init runs against identical source (same machine, same swag binary) produced byte-identical output every time. A "canonically sort JSON/YAML output" post-processing step, as one might first assume is needed, would be a no-op today, so I didn't add one.
  • The actual instability is in swag's schema naming, not ordering. ToolHive's dependency graph has many packages that legitimately share simple type names (several internal packages named Config/RunConfig, plus a registry package name collision between pkg/registry, toolhive-core/registry/types, and oras.land/oras-go/v2/registry). Swag's v2.0.0-rc5 disambiguates these by qualifying colliding names with a sanitized full import path, decided while walking an unordered map[*ast.File]*AstFileInfo — this is upstream swag internals, not something we control from this repo. Within one stable environment (fixed swag version + fixed go.sum), this comes out fully deterministic (verified with repeated reruns, and across GOOS=linux vs. native macOS — identical). The divergence in Rate limiting observability (metrics and tracing) PR B #5800's committed file appears tied to some other difference in the authoring environment (e.g., module cache state) at generation time rather than plain map-iteration randomness, and I couldn't fully pin it down beyond that.
  • Given the above, the practical, in-scope fix is closing the one confirmed drift source (@latest) rather than chasing further into swag internals. If drift is observed again after this fix, the strongest follow-up would be a CI job that regenerates docs/server and fails the build on any diff — that's a separate change and not included here.

Type of change

  • Bug fix

Test plan

  • Manual testing (describe below)

Ran task swagger-install and confirmed it now installs the version pinned in go.mod (v2.0.0-rc5) rather than whatever @latest resolves to. Ran task docs three times in a row and diffed docs/server/ between runs — byte-identical every time, and no diff against the currently-committed docs on this branch.

Special notes for reviewers

The deeper schema-naming sensitivity described above is a known swag limitation, not something this PR attempts to fix — see the investigation notes for what's understood about it and why a CI verification job is the recommended fast-follow if it recurs.

Generated with Claude Code

The task installed swag via `go install .../swag@latest`, so
`task docs` could silently pick up a newer swag release with
different schema-name disambiguation behavior than the version
pinned in go.mod, reintroducing drift across contributors' machines.
Resolve the version already recorded in go.mod instead of `@latest`.
@jerm-dro
jerm-dro requested a review from JAORMX as a code owner July 14, 2026 20:28
@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label Jul 14, 2026
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.77%. Comparing base (3a55fad) to head (3647a4c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5802      +/-   ##
==========================================
- Coverage   70.84%   70.77%   -0.07%     
==========================================
  Files         685      685              
  Lines       69458    69458              
==========================================
- Hits        49208    49162      -46     
- Misses      16636    16699      +63     
+ Partials     3614     3597      -17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jerm-dro
jerm-dro marked this pull request as draft July 14, 2026 21:15
@jerm-dro

Copy link
Copy Markdown
Contributor Author

Converting to draft — this pins swag's version to close a real gap (unpinned @latest), but it's a preventive fix, not a confirmed root cause of the #5800 diff. See the "Investigation notes" section below for what was and wasn't verified. Marking ready for review once the cause is confirmed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant