ci: enforce the checks a v1.0 tag depends on - #127
Merged
Conversation
The release procedure was documented end to end but unenforced, and four defects were live on main. The fidelity manifest could drift silently. internal/generated is committed but derived, and nothing verified the committed output still matched its sources. The Go tests guard the manifest's shape — floors, registered services, the CRUD registry — but none of them notice an operation a provider gained and the manifest never did; absence from a 7,475-entry map is invisible. A codegen-drift job now regenerates and diffs. Its first catch was this repo: the manifest's own doc comment was a release behind its template. Eight changelog fragments carried an empty Issue field, which changie renders as ([#](https://github.com/skyoo2003/devcloud/issues/)) and batches without complaint. They would have shipped as dead links in the v1.0 release notes. The fragments are fixed and the release workflow now rejects a batched file containing one. One fragment sat in .changes/unreleased/ rather than changes/unreleased/, so it was excluded from every release since it was written. Moved; the batch goes from 25 entries to 26. A tag push published without waiting for tests. CI does trigger on tags, but the two workflows race, so a red commit could still ship binaries, container images and a Homebrew formula. The release job now needs a test job. Also ships the docs tree inside the release archive, so docs are versioned by tag: the docs/ beside a binary describe that binary.
github-actions
Bot
added
documentation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a2a886e4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Six follow-ups from review, each verified against the tree. The drift check did not gate publishing. release.yml waited only on the Go tests, which are exactly the checks that cannot see a stale fidelity manifest — the reason the drift job exists. A tag could publish generated code that misstates what the release serves. The release job now needs a codegen-drift job of its own rather than trusting that CI won the race. The drift check missed untracked output. `git diff --exit-code` ignores a newly generated package, so adding a service model and forgetting to commit its generated directory passed. cmd/codegen/main.go:18-20 already documents this trap and smithy-sync.yml:44-46 already avoids it; both drift checks now use `git status --porcelain`, as that one does. codegen exited 0 after skipping a model. An unreadable or malformed model printed to stderr and continued, so generation could be incomplete while the drift check saw no changed files and called it clean. It now exits non-zero, and does so before writing the CRUD registry and fidelity manifest — those describe the whole fleet, and building them from a partial set would state in generated code that a service's operations do not exist. The issue-number guard only caught the empty case. `Issue: "abc"`, `"0"` and `"-1"` all render links that go nowhere and all batched cleanly. The check now requires every entry to end in a positive integer rather than enumerating the malformed spellings. Manual dispatch tested the wrong commit. actions/checkout defaults to the ref the run was launched from, not the tag input, so the gate vouched for a branch while GoReleaser was asked for a tag. Both jobs now check out the resolved tag. The gate ran on amd64 only while GoReleaser publishes arm64 artifacts. The test job now mirrors ci.yml's architecture matrix.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6b0dd7b4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
compat.yml triggers on branch pushes and pull requests only, so on a tag it does not race the release the way CI does — it never runs at all. The boto3 suite is the guardrail this project leans on hardest, and it had no bearing on what a tag published; docs/release.md listed it as a manual pre-flight step, which is the judgment call the release gate exists to remove. The pre-flight checklist now says which boxes the workflow re-runs and which only a human catches, and quotes the drift check the way CI actually runs it (git status --porcelain, not git diff --exit-code).
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ea4012d25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Regenerating in place only overwrites the filenames the generator still emits. internal/codegen/generator.go has no deletion path — no os.Remove, no RemoveAll — so an output it stops emitting stays on disk, tracked and unchanged, and `git status --porcelain` reports nothing. The drift check then passes on generated code that no longer matches its generator, which is the case the check exists to catch. The repo has hit this before: cmd/codegen/ main.go:16-22 exists because stubs deleted in #91 and #96 kept coming back. Both drift jobs now clear internal/generated before regenerating, so a retired or renamed output shows up as a deletion. Every tracked file under that tree carries a generated marker, so nothing hand-written is at risk, and scripts/generate-imports.sh writes to cmd/devcloud/imports.go, outside it. Verified by committing a file the generator does not emit: in-place regeneration left porcelain empty, clean-tree regeneration reported ` D internal/generated/zz_obsolete_gen.go`. A clean-tree run against the current tree reproduces it byte for byte, so the stricter check starts green.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a3e804da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… locally Adding docs/ to the archive exposed how little of it resolved: measured against a real snapshot archive, README.md and the docs tree reach seven top-level files by relative path that were not shipped. Adding them takes the archive from 26 of 28 relative links broken — the state before docs/ was included at all — to 17 of 134, and every one of those points into source or CI config, which a binary archive has no business carrying. Verified by extracting dist/devcloud_..._linux_amd64.tar.gz and resolving each link against the extracted tree: 117 of 134 work. Archive size 3.2M. The pre-flight checklist also still told developers to regenerate in place, which is the exact check the previous commit taught CI not to trust. It now mirrors the clean-tree sequence.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e0d6a8f5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The issue-link check filtered the '* ' entries and complained about the ones that did not carry a valid link. With no '* ' entries at all the filter matched nothing, `|| true` turned the empty result into success, and the gate passed having checked nothing. changes/v0.1.0.md is exactly that shape — hand-written prose with 25 '- ' bullets and no issue links — so a manual dispatch for that tag would have replaced its release with notes this gate never inspected. Now a notes file must contain at least one changie entry, and any other bullet form is rejected outright: those are hand edits, which is what the link check cannot vouch for and what docs/release.md already forbids. Verified against the real files and synthetic cases: v0.1.0.md and a headers- only file now fail on "no changie entries", a file mixing a changie entry with a hand-written bullet fails on the bullet, a malformed issue link still fails, and v0.2.0.md plus the current unreleased batch still pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 971a1009dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5 tasks
The hand-edit check enumerated bullet markers — '^[-+] ' — and its comment claimed it rejected "any other bullet form". It rejected two of them. An ordered item, an indented item and a pasted paragraph all passed, and the entry check that follows only inspects '* ' lines, so those lines shipped into a GitHub release with nothing about them verified: * Real entry ([#42](.../issues/42)) 1. Hand-written item with no issue link <- old check: pass Inverted it. A batched file holds only what .changie.yaml renders: the version heading, one kind heading per section, and one '* ...' entry per fragment. Allowing those three shapes and rejecting the rest covers every form a human might reach for, including the ones nobody thought to list. Verified against authentic output — `changie batch` (v1.25.0, current config, all 27 unreleased fragments) and changes/v0.2.0.md both pass; '1. text', ' - text', '- text' and a bare paragraph each fail with the offending line numbered. The empty-notes and missing-file gates still fire on changes/v0.1.0.md and an absent path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 539aab14af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
All four jobs resolved `github.event.inputs.tag || github.ref` on their own.
A tag is a mutable pointer: force-update it, or delete and recreate it, while
the run is in flight — which is what a maintainer does on spotting that the
wrong commit got tagged — and the gates check out one commit while the release
job checks out another. "The gates passed" then says nothing about the
artefacts GoReleaser publishes.
A new `resolve` job checks out the ref once, records `git rev-parse HEAD`, and
every job downstream takes `ref: ${{ needs.resolve.outputs.sha }}`. It carries
the tag name and the dry-run flag too, so the `resolve_tag` step inside
`release` and the ref expression repeated four times both collapse into it.
github.sha alone would not do: it is immutable, but on workflow_dispatch it is
the branch the run was launched from, not the tag that was typed in.
The release checkout also needs `fetch-tags: true`, and that is not redundant
with `fetch-depth: 0`. actions/checkout always fetches with --no-tags and
brings tags down only through an explicit refspec; for a SHA ref that refspec
is the bare commit (ref-helper.ts getRefSpec, git-command-manager.ts fetch, at
the pinned de0fac2). Pinning without it hands GoReleaser a repo with no tags
and no way to name the version. With it, a tag that moved after `resolve` no
longer points at HEAD, and `git describe --exact-match` inside GoReleaser's
git pipe fails the run rather than mispublishing.
Verified: the job graph parses, every `needs` resolves, all four downstream
checkouts pin to needs.resolve.outputs.sha, and the three outputs read are the
three declared. The resolve step emits tag+sha on a tag push and adds
extra_flags=--snapshot only under dry_run. The notes gate still passes real
`changie batch` output and still rejects every hand-edited fixture.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f407e137ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ased The allowlist matched '^## ' and '^### ' by prefix, so it accepted any version heading at all. Copy or rename an earlier release's notes file to changes/<new-tag>.md and every check passed: the entries carry valid issue links, nothing is hand-written, and GoReleaser hands the body through verbatim — so the v1.0.0 release opens with a heading linking to the v0.2.0 release, dated to it. The heading is now rebuilt from the tag and required exactly, exactly once. Dots in the tag are escaped, which is not cosmetic: without it the regex for v9.9.9 matches a heading reading v9x9x9. Kind headings are restricted to the labels in .changie.yaml, read out of the config rather than restated here so that adding a kind does not start failing releases. If that extraction ever stops matching it yields an empty set, which rejects every kind heading — the check fails loudly instead of silently checking nothing, the same failure mode the empty-entry gate above exists for. Verified against authentic `changie batch` output, 14 cases: correct tag passes; the same file under a different tag, a stale heading, two headings, no heading, '### Bugfixes', an ordered item, an indented item, a pasted paragraph, an entry without an issue link, changes/v0.1.0.md and an absent path all fail. changes/v0.2.0.md still passes under its own tag. Dot escaping confirmed by control: the unescaped variant passes the v9x9x9 heading.
5 tasks
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.
Summary
The release procedure was documented end to end in
docs/release.mdbut unenforced. Four defects were live onmain, each one caught by a check the repo did not run. This adds the checks and turns the prose into a pre-flight checklist.Related Issue
Refs #126
Changes
The fidelity manifest could drift silently.
internal/generatedis committed but derived, and nothing verified the committed output still matched its sources.cmd/devcloud/fidelity_test.goguards the manifest's shape — floors, registered services, the CRUD registry — but none of those notice an operation a provider gained and the manifest never did: absence from a 7,475-entry map is invisible. A newcodegen-driftCI job regenerates and diffs.Its first catch was this repo — the manifest's own doc comment was a release behind its template, because #126 edited the template without regenerating. That regenerated file is in this PR.
This is what makes the PRD's primary metric ("100% of reachable operations carry a declared fidelity tier, enforced by a test that fails the build") true rather than approximately true. It was enforced for two of the three ways an operation becomes reachable.
Eight changelog fragments rendered dead links.
.changie.yamldeclaresIssueastype: int, minInt: 1, butchangie batchdoes not validate custom fields on hand-written fragments and exits 0. Reproduced withchangie batch v9.9.9 --dry-run:All eight came from #126 and would have shipped verbatim in the v1.0 release notes. Fragments fixed; the release workflow now rejects a batched file containing one.
One fragment was in a directory nothing reads.
.changes/unreleased/Fixed-113.yamlsat beside the realchanges/unreleased/.changesDir: changes, so it was excluded from every release since it was written. Moved — the batch goes 25 → 26 entries.A tag published without waiting for tests.
release.ymlverified the release notes existed, then ran GoReleaser. CI triggers on tags too, but the two workflows race, so a red commit could still publish binaries, container images and a Homebrew formula. The release job nowneeds: test.Plus:
docs/ships inside the release archive, so docs are versioned by tag — thedocs/beside a binary describes that binary. A pre-flight checklist opensdocs/release.md, including a deprecation review with thedashboard→adminrename as the precedent.Test Plan
case "DriftProbeOp":intointernal/services/kms/provider.go, ranmake codegen— the operation appeared in the manifest andgit diff --exit-code internal/generatedreturned 1. Reverted; the manifest no longer contains it.make codegenruns on a clean tree produce byte-identical output across all 370 files ininternal/generated. This resolves the plan's open question in favour of a full regeneration rather than a fidelity-only check.changie batch v9.9.9 --dry-run | grep -c 'issues/)'goes 8 → 0, and the recovered#113entry appears in the batch.CGO_ENABLED=1 go test ./...— 109 packages, exit 0make test-compat— 775 passedgolangci-lint run ./...— 0 issuesgoreleaser checkreports the config valid.Checklist
golangci-lint run)Changed-20260809-180000.yaml, citing this PR)Separately:
goreleaser checkreports thebrewssection as deprecated (homebrew_casksreplaces it), which arrived with #121. Left alone here — casks and formulae differ in whatbrew installactually does, so that is a user-facing call rather than a mechanical rename.