fix(github): read sema_version_req from sema.toml at each tag - #3
Merged
Merged
Conversation
sync_tag took the requirement as a parameter, and only the link handler passed one. The sync endpoint and the webhook both passed None, so every release after the initial link stored NULL. The client reads NULL as "no requirement", which means the gate never fired for the normal release flow of a GitHub-linked package. The link path was wrong in the other direction: it applied the default branch sema.toml to every historic tag, so editing the field rewrote the requirement of past releases. sync_tag now fetches sema.toml at the tag itself and drops the parameter, so all three callers record what each release actually declared. The webhook has no authenticated user, so it reads the repo with an owner stored GitHub token via the new dal::owners::first_user_id. A tag with no readable sema.toml records no requirement, because tags older than the manifest must still import. A manifest that parses but holds an invalid requirement is an error recorded in the sync log, so the maintainer sees it instead of the release silently losing its constraint. sync_tag now takes RepoAccess rather than four more positional &str arguments, which also satisfies clippy::too_many_arguments. Tests: tag_sema_version_req is split out as a pure function and covered for the valid, absent, unparsable, invalid, and non-string cases. Adds an integration test that pins the sema_version_req JSON key end to end through publish and GET, which is the break that would silently disable the client check, and one that a bad requirement is refused without creating the package.
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.
GitHub-linked packages lost
sema_version_reqon every release after the initial link:sync_tagtook it as a parameter and only thelinkhandler passed one — the sync endpoint and webhook both passedNone, so the row stored NULL and the client read that as "no requirement". The gate never fired for the normal release flow.The
linkpath was wrong the other way: it applied the default branch'ssema.tomlto every historic tag, so editing the field rewrote past releases.sync_tagnow fetchessema.tomlat the tag itself and drops the parameter, fixing all three callers at once. The webhook has no authenticated user, so it reads the repo with an owner's stored GitHub token via a newdal::owners::first_user_id.Failure policy: a tag with no readable
sema.tomlrecords no requirement (tags older than the manifest must still import); a manifest that parses but holds an invalid requirement is an error recorded in the sync log.Also adds the end-to-end test that was missing — pinning the
sema_version_reqJSON key through publish → GET, which is the break that would silently disable the client check.123 tests pass, clippy clean.