chore: close the known risks left open after the review - #49
Merged
Conversation
The release, helm publish, scheduled sync, and Azure DevOps extension workflows were still on actions targeting Node 20, which GitHub force migrates to Node 24 today and will stop supporting. They were left behind earlier because no pull request exercises them; that reasoning holds for proving them, not for leaving them broken, so they are updated now and the next tag exercises the release and helm paths for real. goreleaser is pinned to its v2 major rather than latest. The action warned that latest is ambiguous and locks to ~> v2 regardless, and a release is not where a new major should first run.
golang.org/x/text carried an infinite loop on invalid input (GO-2026-5970) and klauspost/compress an out of bounds read in s2 (GO-2026-5841). Neither was reachable from SyncerD's own code, which is why they were tolerated, but both are transitive dependencies that a later change could easily start calling, and both had a published fix. govulncheck now reports nothing at all rather than nothing reachable.
Everything in this repository was checked against fakes written by the same hand as the code, and three defects that made pull request mirroring inoperable survived a green suite, a passing verification gate, and a review of their own change, because every fake agreed with the mistake. Adds a live suite behind the "live" build tag: it creates two throwaway private repositories, seeds one, opens a pull request, mirrors branches and then objects, and asserts what actually landed. It covers exactly the ground the fakes could not: that the destination repository path is the one the API accepts, that a pull request is findable by its head branch after state is lost, that a mirrored comment appears once rather than on every run, that mentions do not survive into the destination, and that closing the source closes the mirror. Then it deletes both repositories. It is inert without credentials and excluded from an ordinary go test, so CI is unchanged. make test-live runs it, and the runbook says when to.
Once the mirror owns labels, a source pull request with none produced a nil slice, which marshals to null, and GitHub answers 422: the field has to be an array. Clearing labels is a legitimate and common request, so this broke the update of nearly every mirrored pull request, and the label failure was wrapped in a way that failed the whole pull request rather than just its labels. Found by the live suite on its first real run against GitHub, in the first minute it existed. No fake had an opinion about null.
This was referenced Aug 19, 2026
Merged
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
2 participants
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.
The three things I flagged as known-but-open after #48. All addressed.
1. Deprecated actions in the workflows nothing tests
release.yml,helm-release.yml,syncerd.yml, and the Azure DevOps extension publish were still on actions targeting Node 20, which GitHub force-migrates today and will stop supporting. I had left them behind because no pull request exercises them — but that argues for being careful about proving them, not for leaving them broken. All are current now, and the next tag exercises the release and helm paths for real.goreleaser is pinned to
~> v2rather thanlatest: the action warned thatlatestis ambiguous and locks to that major anyway, and a release is not where a new major should first run.2. Two advisories in transitive modules
An infinite loop in
golang.org/x/text(GO-2026-5970) and an out-of-bounds read inklauspost/compress(GO-2026-5841). I had called these noise because nothing reachable calls them. That was true and it was the wrong bar: both had published fixes, and reachability is a property of today's code, not tomorrow's.govulnchecknow reports nothing at all rather than nothing reachable.3. Nothing had ever run against a real provider
This is the one that mattered. Every test in this repository checks SyncerD against fakes written by the same hand as the code, and that has already failed us: three defects that made pull request mirroring completely inoperable passed a green suite, a passing verification gate, and a review of their own pull request, because every fake agreed with the mistake.
Adds
internal/livetest, behind thelivebuild tag and inert without credentials, so CI is unchanged.make test-livecreates two throwaway private repositories, seeds one, opens a pull request, mirrors branches and then objects, and asserts what actually landed:Then it deletes both repositories. Each of those assertions maps to a defect that actually shipped.
What this does not do
It has not been run yet. The suite exists and compiles; running it creates real repositories under a real account, which is a decision rather than a detail. Say the word and where, and I will run it before the release.