Repository navigation
fix: correct update-check docs, Go SDK version and make generate/lint - #37
Merged
Merged
Conversation
Since 8199ec1 the update check is turned off with SPINNERET_UPDATE_CHECK_ENABLED=false. A blank SPINNERET_UPDATE_CHECK_URL reads as unset, like every blank variable, and selects the built-in feed (internal/appconfig), so it never turned anything off. That commit fixed .env.example and the Compose file, but the console and the manual kept telling operators to blank the URL: the System page's "turned off" message and its introduction card in both languages, the CheckForUpdate comment in system.proto (and so the generated Go and TypeScript), the UpdateCard doc comment, and the configuration, console and operations pages in both languages. An operator who followed them on a host that must make no outbound call kept the call. They now name the switch and say what it does: with it off, CheckForUpdate makes no outbound call and answers disabled with only current_version and current_is_release set. The introduction card no longer claims the server then reaches nothing at all, which was never true. The configuration reference gains the SPINNERET_UPDATE_CHECK_ENABLED row it never had, and the installation guide's list of outbound connections, which ended in "nothing else", now includes the release feed, the exit-IP lookup and the OTLP collector. Generated code was regenerated with buf v1.73.0, protoc-gen-go v1.36.12 and protoc-gen-connect-go v1.21.0 (today's @latest, which CI installs) and protoc-gen-es 2.15.0; only comment lines changed. The UpdateCard test now fails if the disabled message names the URL instead of the switch. The 0.1.0 changelog entry that describes the blank URL stays as released; the 0.1.4 entry records the correction.
The Go SDK sends User-Agent spinneret-go/<Version>, and Version has been "0.1.0" in every tag from v0.1.0 to v0.1.5: each release commit (bd20168 and 2eb45d9, for example) bumped the Python SDK's _version.py and web/package.json but not this constant. Every Go node therefore announced 0.1.0 whatever release it was built from, so the header could not tell a reader of proxy or access logs which release a node runs. Version is 0.1.5 now, the version the other two carry. A new test in the SDK package, which CI's go test ./... runs, reads _version.py and web/package.json and fails when the three disagree. Its message lists what each file says and names every file behind the newest version, so a release that forgets one is told which file to bump. The SDK manual stated the version as 0.1.0 in both languages; it now says the SDK is versioned with each release, as #36 made the Python section say, and shows the User-Agent as spinneret-go/<SDK version>. No document lists the files a release bumps, so there was no release checklist to extend; the changelog's note on version spelling, which names the files that carry the version, now names this one as well.
make generate reached sqlc through scripts/sqlc-generate.sh, which ran a
bare `sqlc generate` from the repository root. That reads the root
sqlc.yaml only, so fifteen of the sixteen query packages were never
regenerated by it, while CI's generated-code check loops over every
sqlc.yaml outside web/ and then fails on any `git diff -- gen internal`.
A change to a package's queries passed make generate locally and failed
in CI, and the pull-request template's "regenerated with make generate"
box could be ticked on stale code. make lint ran golangci-lint without
the e2e build tag CI passes, so test/e2e, whose twelve files are all
e2e-tagged, was never linted locally.
The script now runs `sqlc generate -f` for every sqlc.yaml outside web/,
the set CI's loop regenerates, inside the same lock; arguments still go
to a single `sqlc generate`, so `-f internal/auth/sqlc.yaml` regenerates
one package as before. make lint passes --build-tags e2e; the 10m
timeout CI also passes is already in .golangci.yml.
Checked: with internal/auth/authdb and internal/proxy/proxydb
perturbed, origin/main's script left both stale and the new one
restored them, after which `git diff --exit-code -- gen internal` is
clean; a failing config fails the run and still releases the lock; two
concurrent runs both succeed and leave no lock behind. An unused
function planted in test/e2e is invisible to the old lint command ("no
go files to analyze") and reported by the new one, and make lint finds
0 issues in the tree.
CONTRIBUTING.md and the contributor guide said, in both languages, that
CI is stricter than these targets and had contributors run CI's loop by
hand. They now say the targets do what CI does, and Before you push uses
make generate and make lint.
sdk/python/CHANGELOG.md, which the sdist ships (the include list in pyproject.toml), claimed to document every notable change to the SDK but stops at 0.1.0. The SDK has gone out with every tag since under the same version, each release commit bumping _version.py, and what changed in it is in the repository's CHANGELOG.md: the development pins in 0.1.3 and 0.1.5, and the install and ConfigurationError fixes under [Unreleased]. 0.1.1, 0.1.2 and 0.1.4 changed only its version. Rather than reconstruct five sections from that file, this one now says it covers the SDK up to 0.1.0 and points at the repository's changelog for everything after, by absolute URL because the sdist carries no repository around it. The 0.1.0 entries are unchanged. Their date is now 2026-09-18, the day v0.1.0 was tagged and the date the repository's changelog gives that release: the SDK has never been released on its own, so its 0.1.0 is that tag, not 2026-09-17. uv build succeeds and the sdist contains the new text. sdk/go ships no changelog, so it has nothing similar to correct.
The lockstep test orders versions with updatecheck.Compare, which reads "v0.1.6" and "0.1.6" as the same release. On that tie it kept the first file, the Go const, so a const written the way the tag is made the failure tell the releaser to rewrite the Python SDK and the console as "v0.1.6" and call that a bump, against the rule that files carry the version bare. The test still failed as it should; only its advice was wrong. A tie now goes to the bare spelling, and a file that names the right release in another spelling is told to respell it, with the tag's v named when that is the difference. A table test pins the advice on made-up versions, so every case is covered without editing the repository's version files.
golangci-lint-action@v9 runs `golangci-lint config verify` before `golangci-lint run`: its `verify` input defaults to true and ci.yml does not override it. So a key the schema does not know fails CI's Lint step, while make lint, which ran only `run`, passed it. With `errorff: true` planted under linters.settings.errorlint, `golangci-lint run --build-tags e2e` printed "0 issues." and exited 0, and `golangci-lint config verify` exited 3. The Makefile, its help and the contributing guide already said make lint runs the way CI does; now it does. The check needs no network, because the schema is built into the binary, and takes about 50 ms. The guide's table of CI's go job now names the verify step too, in both languages.
The paragraph rewritten for make generate and make lint ended by calling the guide's Before you push list "the whole gate". CI has four jobs and merges a change only when all of them pass; the fourth, image, builds deploy/docker/Dockerfile, and nothing in that list builds it, so a change to the Dockerfile or .dockerignore can pass the list and still fail CI. The paragraph now says the list runs the checks of the go, web and python-sdk jobs and that make docker is the image build's local equivalent, and the guide's section on the image job says when to run it. The paragraph also said make generate regenerates "every sqlc.yaml", which reads as the config files rather than the query code they describe, and it now names the configuration check make lint runs, as CI does. Both languages.
…se table The security guide says the server makes no network egress at runtime except to PostgreSQL, Redis/Valkey, ClickHouse, the proxies, the proxy health-check URL and the notification channels. Since 0.1.0 it has also called the release feed when an operator checks for updates (internal/updatecheck), and, when they are set, SPINNERET_PROXY_EXIT_IP_URL through every proxy (internal/proxy/healthprobe.go) and the OTLP collector (internal/observability/tracing.go). This branch already corrected the installation page's list; the security page, the one an air-gapped operator reads, now says the same in both languages, and the CHANGELOG entry names both pages. The configuration page's table of what the Compose stack sets ended at SPINNERET_PROXY_CHECK_URL, and the sentence after it sends any other variable to an override file, although docker-compose.yml has passed SPINNERET_UPDATE_CHECK_URL and SPINNERET_UPDATE_CHECK_ENABLED through since 0.1.4 and .env.example offers SPINNERET_UPDATE_CHECK_ENABLED=false as a line to uncomment. The table now lists both.
The note added to the SDK's changelog said that "from 0.1.1 on" the SDK is released together with the rest of Spinneret under the same version, which implies 0.1.0 was released apart or under another version. It was not: tag v0.1.0 carries __version__ = "0.1.0" next to a console at "0.1.0", and the root CHANGELOG's 0.1.0 section lists the Python SDK among what it added. What changed after 0.1.0 is only where the SDK's changes are written down. The intro now says the file records the first release, that the SDK ships with Spinneret under the same version and tag, and that from 0.1.1 on its changes are in the repository's CHANGELOG.md and this file is no longer updated. The sdist ships this file, so the wording reaches whoever unpacks it.
The comment on New said a nil checker answers "disabled". It never did: Checker.Enabled is nil-safe, but Check then builds its answer from c.cfg, so a nil *Checker panics with a nil pointer dereference, as it has since the update check was added in c052ae6. Nothing hits it: the server always passes updatecheck.New (internal/server/components.go), and the handler tests that pass nil never call CheckForUpdate. The comment now states the contract: updates must not be nil if CheckForUpdate can be called, and the disabled check is a Checker built with an empty Config.URL, which answers "disabled" without any outbound call. That is the Checker a deployment gets with SPINNERET_UPDATE_CHECK_ENABLED=false. Comment only.
The README said those were the egress reachability check and alert delivery, "both to URLs you configure". Since 0.1.4 the server also calls the release feed when an operator presses Check for updates, it fetches the exit-IP lookup URL through each proxy when one is set, and it exports traces when an OTLP collector is configured; the reachability check also has a built-in default URL. Name them the way the installation and security guides now do, and say how to turn the update check off.
The paragraph listed six variables as the only ones .env could set and named SPINNERET_MAX_WATCHERS, SPINNERET_STREAM_MAXLEN, the retention set, SPINNERET_CLICKHOUSE_TTL_DAYS and SPINNERET_ACQUIRE_FLEET_INFLIGHT as needing an anchor line first. d78a9ac (0.1.2) passed all of those through, and later releases added the notification opt-out and the update check. Say that the x-spinneret-env anchor is the list, give today's set grouped by purpose, and keep only variables that really are absent as examples of what still needs a line. Checked mechanically against the anchor in both languages.
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.
What and why
Four stale items found during the pre-release cleanup after #36, the problems review found next to them, and two older doc claims the final verification found. Nothing on the wire changes. The branch changes one constant and two developer targets; everything else is comments, copy and docs.
A. The update check's off switch. Since 0.1.4 (8199ec1) the check is turned off with
SPINNERET_UPDATE_CHECK_ENABLED=false.internal/appconfigreads a blankSPINNERET_UPDATE_CHECK_URLas unset, which selects the built-in feed.falseblanks the URL whatever it was set to (appconfig.go:233). TheCheckerthen makes no HTTP call, andCheckForUpdateanswersdisabled: truewith onlycurrent_versionandcurrent_is_releaseset.The console's disabled message and its System intro card still told operators to blank the URL. So did the
CheckForUpdateproto comment, theUpdateCardandupdatecheck.Configcomments, and the configuration, console and operations pages. An operator who followed that advice still got the outbound call. All of these now name the switch, in both languages.gen/andweb/src/genwere regenerated; only comments changed there.Other changes under A:
SPINNERET_UPDATE_CHECK_ENABLEDrow.docker-compose.ymlhas had since 0.1.4.SPINNERET_PROXY_EXIT_IP_URLand the OTLP collector. This makes the installation page's "nothing else" true.systemapi.Newno longer claims a nil checker answers "disabled". A nil checker would panic, and production never passes nil.B. The Go SDK's version was frozen.
spinneret.Versionwas"0.1.0"at every tag from v0.1.0 to v0.1.5. The release commits (bd20168 and 2eb45d9, for example) bumped only_version.pyandweb/package.json, so every Go node's User-Agent saidspinneret-go/0.1.0. The const is now0.1.5.sdk/go/spinneret/version_test.gois in the root module, so CI'sgo test ./...runs it. It fails unless the Go const,_version.pyandweb/package.jsonagree, and it names the fix, for examplebump const Version in sdk/go/spinneret/types.go from 0.1.0 to 0.1.5. A const written with the tag'svgets "write … as 0.1.5, without the tag's v" instead.The SDK manual no longer says 0.1.0. No document lists the release steps, so the CHANGELOG's version-spelling note, which names the files that carry the version, now names the const as well.
C.
make generateandmake lintwere looser than CI.scripts/sqlc-generate.shran a baresqlc generate, which reads only the rootsqlc.yaml, 1 of 16. CI regenerates everysqlc.yamloutsideweb/.make lintdid not pass CI's--build-tags e2e. It also skippedgolangci-lint config verify, which golangci-lint-action@v9 runs first.The script now regenerates all sixteen under the same lock; arguments still go to a single
sqlc generate.make lintnow runsconfig verify, thenrun --build-tags e2e ./...; the 10m timeout is already in.golangci.yml. CONTRIBUTING.md anddocuments/{en,zh}/20-contributing.mdnow say so. They also say that Before you push covers CI'sgo,webandpython-sdkjobs, and that theimagejob's build ismake dockerlocally.D.
sdk/python/CHANGELOG.mdstopped at 0.1.0, and the sdist ships it. It now says the SDK is released with Spinneret under the same version and tag, and that its changes from 0.1.1 on are in the repository's CHANGELOG.md. The 0.1.0 section is kept, with its date corrected to the tag's, 2026-09-18.sdk/goships no changelog.CHANGELOG.md[Unreleased] records A and B. The released [0.1.0] entry that describes the blank URL is left as released, because the 0.1.4 entry records the correction.No linked issue: these were found during the pre-release cleanup.
E. Two older doc claims, found during verification.
README.mdandREADME.zh-CN.mdsaid the server's own requests were the egress reachability check and alert delivery, "both to URLs you configure". They now list the release feed (and how to turn it off), the exit-IP lookup and the OTLP collector, the way the installation and security guides do.documents/{en,zh}/18-troubleshooting.mdstill listedSPINNERET_MAX_WATCHERS,SPINNERET_STREAM_MAXLEN, the retention set,SPINNERET_CLICKHOUSE_TTL_DAYSandSPINNERET_ACQUIRE_FLEET_INFLIGHTas needing an anchor line before.envcan set them. d78a9ac (0.1.2) passed all of them through, so adding the line as the page advised madedocker compose configfail on a duplicate key. The paragraph now says thex-spinneret-envanchor is the list and gives today's set, keeping only variables that really are absent as examples.How it was verified
I verified the branch independently in a fresh detached worktree at bb4d7ae.
Codegen, run as CI does. I ran CI's "Verify generated code is up to date" commands verbatim with the versions it installs today: buf v1.73.0, sqlc v1.31.1, and protoc-gen-go v1.36.12 and protoc-gen-connect-go v1.21.0, both of which are
@latest. All 16sqlc.yamlfiles were regenerated andgit diff --exit-code -- gen internalwas clean.pnpm gen, with the pinned protoc-gen-es 2.15.0, leftweb/src/genclean. Against main, thegen/andweb/src/gendiffs contain comment lines only.make generate. I editedinternal/auth/authdb/db.goandinternal/proxy/proxydb/db.go. The old baresqlc generateleft both edited;make generaterestored both../scripts/sqlc-generate.sh -f internal/auth/sqlc.yamlstill regenerates a single package. shellcheck is clean.Lockstep test. It passes as is. It fails with the right fix for each mismatch I tried:
v0.1.5make lint. It reports 0 issues. With an unknown key added to.golangci.yml,make lintfails, whilegolangci-lint runalone still reports 0 issues.Go.
go build ./...,go vet ./...andgo vet -tags e2e ./test/e2e/...are OK.go test -race -count=1 ./sdk/go/... ./internal/updatecheck ./internal/api/systemapi ./internal/appconfigpasses.Web.
pnpm install --frozen-lockfile,typecheck,lint,format:check,testandbuildall pass. The tests are 98 files and 632 tests; the UpdateCard assertion fails on the old copy.web/pnpm-lock.yamlis unchanged, and the en and zh-CN key sets are identical across all 17 namespaces. Local node is v22.16.0, below the engines floor; pnpm only warned.Troubleshooting and README text. A script compared the settable and not-settable variables in both languages' 18-troubleshooting paragraph with the
${...}entries of thex-spinneret-envanchor. Every claim matches, and en and zh name the same variables.Python SDK. On 3.10, 3.12 and 3.13:
pytest -q375 passed, andruff checkandruff format --checkare clean.mypy srcon 3.12 is clean.uv buildproduces spinneret-0.1.5, and the sdist'sCHANGELOG.mdcarries the new text.make test: not run in full, because it needs the PostgreSQL/Valkey/ClickHouse stack. The touched packages ran with-race, as listed above.make lintmake web-test, pluspnpm buildmake e2e: not applicable, because nothing on the wire changed.Checklist
documents/en/anddocuments/zh/both updated, and they say the same thingCHANGELOG.mdupdated if a user can see the changemake generate, not edited by hand.web/src/genwas regenerated withpnpm gen.