heiko/gogogo
heiko/gogogo
Watch
1
0
Fork
You've already forked gogogo
 0
Code Issues 21 Pull requests Projects Releases 4 Packages 1 Wiki Activity Actions

feat(release): route snapshot/pre-release builds to testing suites ai:claude-opus-5 #81

Manually merged
heiko merged 1 commit from feat/testing into master 2026-08-15 11:45:42 +02:00
Conversation 0 Commits 1 Files changed 32 +2476 -121
heiko commented 2026-08-14 20:48:15 +02:00
Owner

Snapshot and pre-release builds no longer land in the release suites.

Behavior

  • A Go pseudo-version (branch build) or a semver pre-release (v1.2.0-rc.1)
    now targets the *-testing suites; a clean tag targets the release suites.
    Build metadata alone (v1.2.3+build.5) is still a release.
  • The suite name is derived from artifacts.deb.testing-suffix (default
    -testing, "" opts out) or an explicit artifacts.deb.testing-distributions
    mapping. Both keys are new in .gogogo.conf, with migration and validation.
  • --testing / --release on build/pack/publish override the automatic
    decision; they are mutually exclusive.
  • One resolved suite list is threaded to every consumer -- the Forgejo registry
    upload, the dupload .changes Distribution: field, and the generated
    source-package debian/changelog -- so a .deb and its own changelog can no
    longer disagree.
  • The decision is made at pack time and recorded in gogogo-state.json, so a
    resumed publish --out uploads where the staged .changes says. A
    conflicting --testing/--release on the resuming invocation is refused
    with an error naming the pack command to re-run; state written before suite
    routing existed resumes on the release suites with a warning.
  • artifacts.deb.distributions entries are now validated (non-empty,
    whitespace-free, unique), as are the testing map's keys and targets.

Suite routing is resolved from the global artifacts.deb scope. Both new keys
merge per package for consistency, but -- as with distributions itself -- a
per-package value does not change the suite an upload targets. This is now
stated in gogogo.conf(5).

Review

The last three commits address a 100 Go Mistakes-style review by
ai:gpt-5.6-sol, including two bugs that predate this branch: writeState
ran before stageAndSignDupload, so gogogo-state.json recorded an empty
dupload list and a later publish --out silently skipped every dupload
destination; and a Debian suite was demanded even for RPM- or blob-only
releases.

Verification

GOWORK=off go build ./... && GOWORK=off go vet ./... && GOWORK=off go test ./...
GOWORK=off go generate ./...   # run twice, hashes stable
gzip -t man/*.gz
gzip -cd man/gogogo.conf.5.gz | groff -man -Tutf8 -ww >/dev/null

All clean. No live-network or repository-publishing test was run.

Note

24e0fd0 refactor(cli)!: rename --local to --skip-upload is a master-only
commit that is not yet on origin/master, so it currently shows up in this
PR. It drops out of the diff as soon as master is pushed.

Snapshot and pre-release builds no longer land in the release suites. ## Behavior * A Go pseudo-version (branch build) or a semver pre-release (`v1.2.0-rc.1`) now targets the `*-testing` suites; a clean tag targets the release suites. Build metadata alone (`v1.2.3+build.5`) is still a release. * The suite name is derived from `artifacts.deb.testing-suffix` (default `-testing`, `""` opts out) or an explicit `artifacts.deb.testing-distributions` mapping. Both keys are new in `.gogogo.conf`, with migration and validation. * `--testing` / `--release` on `build`/`pack`/`publish` override the automatic decision; they are mutually exclusive. * One resolved suite list is threaded to every consumer -- the Forgejo registry upload, the dupload `.changes` `Distribution:` field, and the generated source-package `debian/changelog` -- so a `.deb` and its own changelog can no longer disagree. * The decision is made at pack time and recorded in `gogogo-state.json`, so a resumed `publish --out` uploads where the staged `.changes` says. A conflicting `--testing`/`--release` on the resuming invocation is refused with an error naming the `pack` command to re-run; state written before suite routing existed resumes on the release suites with a warning. * `artifacts.deb.distributions` entries are now validated (non-empty, whitespace-free, unique), as are the testing map's keys and targets. Suite routing is resolved from the global `artifacts.deb` scope. Both new keys merge per package for consistency, but -- as with `distributions` itself -- a per-package value does not change the suite an upload targets. This is now stated in `gogogo.conf(5)`. ## Review The last three commits address a `100 Go Mistakes`-style review by `ai:gpt-5.6-sol`, including two bugs that predate this branch: `writeState` ran before `stageAndSignDupload`, so `gogogo-state.json` recorded an empty dupload list and a later `publish --out` silently skipped every dupload destination; and a Debian suite was demanded even for RPM- or blob-only releases. ## Verification ```sh GOWORK=off go build ./... && GOWORK=off go vet ./... && GOWORK=off go test ./... GOWORK=off go generate ./... # run twice, hashes stable gzip -t man/*.gz gzip -cd man/gogogo.conf.5.gz | groff -man -Tutf8 -ww >/dev/null ``` All clean. No live-network or repository-publishing test was run. ## Note `24e0fd0 refactor(cli)!: rename --local to --skip-upload` is a master-only commit that is not yet on `origin/master`, so it currently shows up in this PR. It drops out of the diff as soon as master is pushed.
refactor(cli)!: rename --local to --skip-upload ai:claude-opus-5
Some checks failed
nagonag (Push) / nagonag (push) Failing after 4m40s
Details
24e0fd05be
 
"--local" described where the build ran, not what it did; the flag actually suppresses the upload. Rename it to say so. No behavior change, and no compatibility alias -- "--local" is now an unknown flag.

Also warn when --skip-upload is used without --out: nothing is uploaded and nothing is written, so the artifacts are built into a temporary directory and discarded. Printing the resolved release plan that way is legitimate, so this warns rather than fails, but it now says so before the build instead of leaving the user to notice afterwards.

Renamed consistently: the flag, Flags.Local -> Flags.SkipUpload, the --no-network error, the dry-run output, and the affected doc comments. gpg --local-user and doctor.Options.Local are unrelated and untouched -- the latter means "skip remote checks", not "skip upload". (ai)
The registry upload, the dupload .changes file, doctor upload plan, and the generated debian/changelog each fell back to a default independently when artifacts.deb.distributions was unset. The source-package path used "unstable" while the other three used "stable", so a .deb built and published without any distributions configured disagreed with its own debian/changelog about which suite it targeted.

Add config.Deb.EffectiveDistributions() as the single resolution point and route all four consumers through it. The remaining "unstable" literal in internal/pack/source is now unreachable from the release pipeline; kept only as a defense-in-depth fallback for direct library use, documented as such and aligned to "stable".

First step toward routing snapshot/pre-release builds to *-testing suites: a single source of truth for the target distribution is required before that can be layered on safely. (ai)
Add artifacts.deb.testing-suffix (*string) and artifacts.deb.testing-distributions (map[string]string), plus the pure Deb.TestingDistributionsFor(release []string) resolver: an explicit per-suite override wins, otherwise each release suite is suffixed (default "-testing"), and an explicit empty suffix with no override is a deliberate opt-out (testing == release). This is config schema and a pure mapping function only -- no CLI flags, no version-detection, no wiring into the release pipeline yet.

Merge semantics: testing-suffix follows the existing *bool scalar-override pattern (Systemd/Lintian); testing-distributions is a new per-key map overlay (mergeStringMap), since it fits neither the existing union-of-slices nor scalar-replace pattern. RPM is untouched -- it has no release/testing suite split concept.

Validation: fatal on empty/whitespace suite names; warns when a testing-distributions key is unreachable (absent from the effective distributions) or when an empty suffix leaves a suite with no covering override.

Fixed during review: the per-package reachability warning compared a testing-distributions key against the package's own unmerged Distributions field, which is empty whenever a package inherits distributions from global -- producing a false unreachable warning for that pattern. It now checks against the fully merged (pkg.MergeDeb(global)) distribution list.

Also fixes a latent default_skel.go parsing gap: default.yml's comment/example extraction rejected hyphenated key names (needed for testing-suffix/testing-distributions), silently dropping such lines and misattaching pending prose to the previous field's path. No existing hyphenated key was ever looked up this way, so this was unreachable before now.

Planned by an Opus 5 pass, implemented by a Sonnet subagent from that plan, reviewed and the above two issues fixed by this session. (ai)
Add --testing/--release (mutually exclusive) to build/pack/publish. Auto-detect when neither is given: a snapshot build (Go pseudo-version, or a semver pre-release tag) targets the *-testing suites via config.Deb.TestingDistributionsFor; a clean release tag targets artifacts.deb.distributions unchanged.

The resolved suite list is decided exactly once per release, in finishStage, and threaded as a plain []string to every consumer -- the registry upload, the dupload .changes Distribution, and the generated source-package debian/changelog -- so none of them can independently re-derive (and disagree about) the mode.

BEHAVIOR CHANGE: a snapshot or pre-release Debian build now defaults to the *-testing suites instead of the release suites. --release opts a specific build back to the release suites; --testing forces testing mode even for a tagged release. Worth a minor version bump when tagging.

Persist the resolved suite list on stage.State (DebDistributions), so a publish resumed from --out state (a separate gogogo invocation, potentially with different --testing/--release flags than the original pack) targets the exact suite already baked into the staged .changes and changelog, not whatever the resuming invocation would newly compute. Found via review: without this, `gogogo pack` (auto -> testing) followed by a later `gogogo publish --release` on the same staged output would upload to the release suite while the already-staged .changes still declared the testing suite.

No backward-compatibility fallback for state missing this field: publishDebPackages fails loudly with a clear error rather than silently re-deriving a suite from the resuming invocation's flags or silently skipping the upload.

Planned by an Opus 5 pass, implemented by a Sonnet subagent from that plan, reviewed by this session -- which found and fixed the resume/flag-drift divergence above, and on request removed backward-compat handling in favor of the fail-loud contract. (ai)
Findings from a review by ai:gpt-5.6-sol of the testing-suite routing
branch.

writeState ran before stageAndSignDupload, so gogogo-state.json recorded
an empty dupload list and a later `publish --out` silently uploaded to no
dupload destination at all. Stage before writing state; a staging or
signing failure now leaves no StagePack state, which is preferable to
persisting state that cannot resume.

Resuming a publish from state written before suite routing existed failed
with "no target Debian distribution resolved". Such state was valid when
it was packed, so fall back to the configured release suites -- what the
gogogo that wrote it uploaded to -- with a warning, instead of refusing.

A conflicting --testing/--release on a resumed publish was silently
ignored. The persisted suite must still win, because the staged .changes
and debian/changelog already declare it, but publishing somewhere other
than explicitly requested is the more dangerous failure mode: refuse with
an error naming the pack command to re-run.

Suite resolution is now keyed on the state's Debian assets, so an RPM- or
blob-only resume never needs a suite and ignores a stray routing flag.
Findings from a review by ai:gpt-5.6-sol.

An empty or whitespace-only entry in artifacts.deb.distributions passed
validation and then uploaded to a suite with no name -- and derived the
bare suffix "-testing" for snapshot builds. A name containing whitespace
silently became two suites in the space-separated .changes Distribution:
field, and a duplicate uploaded the same package to the same suite twice.
Reject all three, at the global and the per-package scope.

An empty testing-distributions key can never match a suite, so it is a
typo rather than an unreachable-override warning: make it fatal. Two
release suites mapping to one testing suite is likewise rejected, except
when the duplication is in distributions itself, which the new suite-name
check already reports.
gogogo.conf(5) documented neither testing-suffix nor
testing-distributions, although .gogogo.conf keys and their manual page
are supposed to stay in sync. Describe both keys, the version-based
decision (pseudo-version and pre-release go to testing, build metadata
alone does not), the opt-out, and the pack-time/resume contract.

Suite routing is resolved once per release from the global artifacts.deb
scope. Both keys merge per package for consistency, but a per-package
value does not change the suite an upload targets -- as is already the
case for distributions itself. Say so, in the manual and on the struct
field, rather than implying an effect that does not exist.

Found in a review by ai:gpt-5.6-sol.
Both branches extended the same seams, so every conflict was "each side added one thing" and was resolved by keeping both, not by choosing: packageBuilds and buildSourcePackages each gained the other branch dists and buildEnv parameters, stage.State gained both DebDistributions and LocalCheckout, and workflowResult gained both debDists and localCheckout.

Two conflicts needed more than that. The stage.State literal in writeState had both sides rewriting the same fields for realignment plus adding one each, so keeping both duplicated ten fields; the compiler caught it. Three buildSourcePackages call sites merged cleanly yet were left wrong, each carrying only its own branch new argument -- clean auto-merges, not conflicts, and only vet found them.

Both branches independently bumped stateSchema to 1 -> 2 for their own field. The merged schema 2 covers both: the fields are additive and omitempty, and a state file missing either resumes on the documented default (release suites, clone-built provenance).

The binary man pages were regenerated rather than resolved by picking a side.

Verified the merge lost nothing by comparing every test function against both parents repo-wide, not just in the conflicted files. Restored sign_test.go and lintian_test.go, which a directory-wide gofmt -w had reformatted although both parents agreed on them; the sign_test.go reformat had added a wsl_v5 issue. True lint count is back to the 28 both parents share (golangci-lint default caps report this as 18). (ai)
This branch deliberately left stateSchema at 1 so state written before deb_distributions existed would still load, and resumeDebDistributions fell back to the release suites for it. Merging master brought stateSchema 2 (bumped there for LocalCheckout). Only master touched that line, so git merged it cleanly and silently invalidated the premise: readState now refuses any file with an older schema, DetectResumable reports nothing to resume, and the run rebuilds.

The fallback, its warning and its two tests therefore described a state no user could reach. The tests passed only because they built stage.State in memory and never went through readState.

An empty suite list on a state carrying Debian artifacts can now only mean a caller bypassed the pack stage, so it fails as an internal error rather than inventing a suite the staged .changes does not declare -- matching publishDebPackages and the repository no-backward-compatibility rule.

Replaced the two tests with one asserting the internal error, and one pinning the claim the deletion rests on: that a pre-suite-routing schema is refused. The latter is differential with a positive control, because its first version passed for the wrong reason -- DetectResumable also skips a state whose artifacts are missing, so it reported "no resume" whatever the schema. It now resumes the identical state once, then rewrites only the schema number. Both were confirmed to fail when their behaviour is reverted. (ai)
The three Debian suite consumers disagreed about an empty suite list, which is an invariant violation rather than a state to handle: finishStage always resolves one before packaging, and EffectiveDistributions never returns empty.

Only the registry upload failed. Source packaging silently substituted the configured default, so debian/changelog could name a different suite than the .changes beside it -- exactly the disagreement suite routing exists to prevent. writeChanges had no guard at all: it emitted a well-formed "Distribution: " with no value, which debsign signs and dupload uploads, deferring the failure to the archive as a rejected or misfiled upload rather than a local error.

All three now fail loudly, and resolveDebDistributions checks its own result so the violation is caught where it would originate rather than three layers down.

Adding the .changes guard immediately failed two existing tests that drove stageAndSignDupload without ever setting a suite, which is what that path had been silently tolerating. Both now state one. Two further callers passed nil to rely on the removed fallback; one of them asserted the fallback output, so it had quietly become a test of the dead branch. TestSuiteInvariant_NoConsumerRepairsAnEmptySuite covers all three consumers and each subtest was confirmed to fail when its guard is reverted. (ai)
The method handed out the config own backing array. Callers keep the result well past the call -- it is stored in the release workflow and serialized into gogogo-state.json -- so a sort or an append anywhere downstream would rewrite the loaded configuration, and every later consumer would resolve the altered suite.

Nothing exploited this yet: the one in-place mutation, doctor sorting the plan, copies first. That copy stays, because a caller that mutates should not depend on a callee promise it can only verify by reading the implementation.

The test covers assignment, append, and the unset-distributions fallback, which must not become a shared package-level slice either. Confirmed to fail when the clone is removed. (ai)
heiko force-pushed feat/testing from cb25bdcd95 to d42cc1d60d 2026-08-15 11:24:21 +02:00 Compare
heiko force-pushed feat/testing from d42cc1d60d to 126bde4d37
All checks were successful
nagonag (Push) / nagonag (push) Successful in 3m39s
Details
2026-08-15 11:41:40 +02:00 Compare
heiko manually merged commit 126bde4d37 into master 2026-08-15 11:45:42 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
Compat/Breaking
Breaking change that won't be backward compatible

  
Kind/Bug
Something is not working

  
Kind/Documentation
Documentation changes

  
Kind/Enhancement
Improve existing functionality

  
Kind/Feature
New functionality

  
Kind/Security
This is security issue

  
Kind/Testing
Issue or pull request related to testing

  
Priority
Critical
 The priority is critical

  
Priority
High
 The priority is high

  
Priority
Low
 The priority is low

  
Priority
Medium
 The priority is medium

  
Reviewed
Confirmed
 Issue has been confirmed

  
Reviewed
Duplicate
 This issue or pull request already exists

  
Reviewed
Invalid
 Invalid issue

  
Reviewed
Won't Fix
 This issue won't be fixed

  
Status
Abandoned
 Somebody has started to work on this but abandoned work

  
Status
Blocked
 Something is blocking this issue or pull request

  
Status
Need More Info
 Feedback is required to reproduce issue or to continue work

  
nagonag
Automated dependency scan finding

  
nagonag/ignore
Ignore this nagonag finding

No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
nagonag
nagonag/ignore
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
heiko/gogogo!81
No description provided.