Release readiness for 0.2.0 — verify job, sdist hygiene, single-sourced version - #19
Merged
Conversation
Dependabot bumped upload-artifact to v7 and left both downloads on v4. The majors are not parallel, so "align them" is not literally possible: download-artifact is at v8, and its README names v8 as the counterpart to upload-artifact@v7. Both downloads now say v8. The stated failure mode would not have occurred, and that is worth recording. upload-artifact@v7's unzipped direct upload is opt-in via archive: false, which this job does not set, and it supports a single file only - dist/ holds a wheel and an sdist. v7 therefore produced a zipped artifact that v4 unzips correctly. What leaving it actually cost: download-artifact@v4 runs on Node 20, which GitHub is deprecating and already warns about in this repository's logs, while the build side had moved to Node 24. And v8 defaults digest-mismatch to error rather than warn, which is what a release pipeline should do with a corrupted download. Right fix, different reason, lower urgency than the finding claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5nXA9BJKdmye9a9FaHdc6
publish reached PyPI without anything having run. A verify job now gates build: full suite, both bench scopes, the linearity gate, lint, mypy, and a tag/version consistency check. Chain is verify -> build -> publish -> github-release. The version was hardcoded in two places, not the one the finding named. src/agentguard/__init__.py:7 held it too, and that is the copy feeding --version, the JSON report's tool.version, and the SARIF driver version. Bumping only pyproject.toml would have published 0.2.0 while every report the tool emitted said 0.1.0, including SARIF uploaded to code scanning. __init__.py now reads importlib.metadata, so pyproject.toml is the only place a number is written, and the tag check compares against both it and the installed metadata so the single-sourcing cannot quietly come undone. The CLI test asserts against metadata rather than a literal, since a hardcoded assertion is a third site to edit in lockstep - the drift it exists to catch. hatch-vcs was considered and rejected: a build-time dependency and a dependency on git history at build time, to solve what five lines solve, in a project whose pitch includes a small dependency tree. Tag check exercised against v0.2.0, v0.2.1 and v1.0.0 - only the matching tag passes. Trusted publishing untouched. Version is 0.2.0. Nothing is tagged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5nXA9BJKdmye9a9FaHdc6
… pytest The sdist carried 36 corpus entries: credential-shaped fixtures, three .env files, two requirements.txt, and code using eval/exec/shell=True by design. Any organisation running a secret scanner over its package cache would get hits on AgentGuard itself, and a security tool that trips security scanners is quarantined at the organisations worth adopting it. Verified by building and listing both artifacts, not by reading config: sdist 88 entries with 36 corpus, now 39 with 0. The wheel was always clean. Excluded /tests entirely rather than /tests/corpus. Excluding only the corpus leaves tests/test_corpus.py in the sdist where it cannot pass, since the manifest it loads is gone - a packager running the bundled suite would see failures that are not defects. tools/bench.py now explains a missing corpus instead of emitting 34 missing-file errors. This is the third external consumer to read the corpus as a real artifact, after GitHub's dependency graph and dependency review. The generalisation is now written down: any file whose shape implies a role will be interpreted by something, whatever directory it sits in. Root .env is ignored, anchored so tests/corpus/** stays tracked - checked with git check-ignore rather than by reading the pattern. Coverage flags left addopts, so `pytest tests/test_rules.py` no longer fails on coverage after every test passed. That is the first thing a contributor runs, and four have open PRs. The gate moved to make test, CI, and the release verify job, and was confirmed to still bite: --cov-fail-under=99 exits 1. py.typed added, verified absent first and present in both artifacts after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5nXA9BJKdmye9a9FaHdc6
twine check passed both artifacts and raised no deprecation warning about
the Apache classifier, so the stated trigger did not fire. Reading the built
metadata found something a warning would not have caught:
license = {file = "LICENSE"} under Metadata-Version 2.4 embedded the entire
Apache licence text in the License field, which renders as a wall of text on
a PyPI page that a release freezes. Now License-Expression: Apache-2.0 via
the PEP 639 form, with the redundant classifier dropped.
The 14-true-positives-over-13-files discrepancy was not one file yielding
two findings - no corpus case expects more than one rule. publishable_keys.py
sat in true_negatives/ while expecting AG001, because its assertion is about
severity rather than silence. Moved it to true_positives/ so the directory
means what it says: 14 positives that must fire, 20 negatives that must not,
findings one-to-one with files. A test keeps it that way.
Explaining in prose why a number looks wrong is a worse fix than the number
not looking wrong. The README still distinguishes files from findings,
because that distinction is worth stating even when the counts agree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5nXA9BJKdmye9a9FaHdc6
The queue asked for a TestPyPI publish. There is no credential available - no ~/.pypirc, no TWINE_* or PYPI_* variable - so the upload did not happen and is recorded as not done rather than implied. Installing both built artifacts into clean virtualenvs covers everything except the upload transport: version consistency end to end, an sdist without the corpus still installing and scanning, and py.typed landing in site-packages. What remains unexercised is PyPI's metadata validation on receipt, the OIDC exchange, and page rendering - and the first tag is the first time any of those runs. Both agentguard-sast and agentguard return 404 on PyPI. The good name is free, which makes this a reservation decision rather than a squatting one. Three options recorded with costs; recommending a placeholder release under agentguard while continuing to publish agentguard-sast, since renaming the distribution in the same change that first exercises publishing is a bad trade on the eve of a first tag. Not actioned - it is a namespace decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5nXA9BJKdmye9a9FaHdc6
![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: 4c977a32bc
ℹ️ 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".
Comment on lines
+11
to
+12
| license = "Apache-2.0" | ||
| license-files = ["LICENSE"] |
There was a problem hiding this comment.
When a downstream or offline build resolves the declared minimum hatchling==1.25, this new PEP 639 form is unsupported and the backend rejects the project metadata before producing an sdist or wheel. Hatchling added PEP 639 support in 1.27.0, so the build-system requirement must be raised accordingly rather than continuing to promise compatibility with hatchling>=1.25.
Useful? React with 👍 / 👎.
Comment on lines
+40
to
+42
| The table counts *findings*, not files: 14 true positives from the 14 positive files, and | ||
| the single false positive is one finding on one negative file. They are one-to-one here | ||
| because no corpus case expects more than one rule to fire. |
There was a problem hiding this comment.
The benchmark does not count findings: measure() groups findings by rule ID and increments each tally only once per rule per file, regardless of how many locations fired. For example, hardcoded_secrets.py can emit four AG001 findings and command_execution.py can emit three AG002 findings while each contributes only one TP, so the newly added claim that the table counts findings materially misstates both its unit and the published precision; describe these as rule-file outcomes or change the manifest and tallying to represent multiplicity.
Useful? React with 👍 / 👎.
amic25
added a commit
that referenced
this pull request
Jul 31, 2026
…band settings checklist (#20) * Rename the distribution to agentguard while it is still free Both names are unclaimed on PyPI, and renaming is free exactly once. After 0.2.0 ships it costs a shim package, a deprecation notice, split download statistics, and two names in search results permanently. Nine sites, not the six a textual reading finds. Three of them arrived with #19 and are the ones that would have failed loudest: the importlib.metadata lookup in __init__.py, its assertion in tests/test_cli.py, and the tag check inside release.yml's verify job. Renaming without those would leave __version__ falling back to 0.0.0+unknown - so --version, the JSON tool.version and the SARIF driver version would all report a version that does not exist - and the release would fail its own tag check after the tag was pushed. The failure #19 exists to prevent, reintroduced by a rename that looked like string replacement. Verified after uninstalling the old distribution first, since a stale editable install would have hidden a broken lookup: metadata resolves to agentguard, the old name raises PackageNotFoundError, both artifacts build and pass twine check, and a clean venv reports AgentGuard 0.2.0. WORKLOG.md keeps its references on purpose. It records what was true at the time. CHANGELOG had two Changed and two Fixed sections under [Unreleased], accumulated across earlier commits; adding the rename note made it three, which is how it surfaced. Consolidated, with every relocated bullet checked to still be there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5nXA9BJKdmye9a9FaHdc6 * Add a TestPyPI rehearsal, and stop a dispatch reaching production workflow_dispatch now publishes to TestPyPI through the same trusted-publishing shape as production - same verify -> build gate, same OIDC permissions, differing only in repository-url and environment. It exercises what a local venv install cannot: metadata validation on receipt, the token exchange, and page rendering. Writing it exposed a hole in the existing gating. workflow_dispatch can be run against a tag ref, so a production job gated only on startsWith(github.ref, 'refs/tags/') would have been satisfied by a manual dispatch on a tag - putting the one irreversible action in this pipeline behind a dropdown. Production now also requires the push event, checked across every event and ref combination. The dispatch takes no input selecting a target. A dry run that can be aimed at production by picking the wrong entry is a worse hazard than the one it exists to remove. Two things the rehearsal forced. The tag check is split: declared-versus- installed runs on both triggers, and tag-versus-declared is tag-only, because GITHUB_REF_NAME is a branch name on a dispatch and would fail every dry run for the wrong reason. And skip-existing is set on TestPyPI only, since it accepts a version once - on production a version conflict is a real signal and must not be swallowed. The run itself is not done: it needs a TestPyPI trusted publisher and a testpypi environment. Recorded, with the residual gap stated - a green dry run validates the mechanism, not the production publisher, which has its own separate record and is verifiable only by inspection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5nXA9BJKdmye9a9FaHdc6 * Write down the settings CI cannot see, and why containment is per-consumer Checking each setting rather than describing it found three things wrong now. No GitHub environments exist, though release.yml names two - a publish would still work, implicitly and with no protection rules, so anyone able to push a tag can publish. The About field still advertises "vulnerable dependencies" although AG009 was deleted in #17, which is the clearest demonstration of why this checklist exists: nothing in CI renders that string, so nothing could fail. And GITHUB_SETUP.md line 6 was telling maintainers to require the `test` check - one of the three phantom contexts that blocked every merge for two sessions. The onboarding document was teaching the incident. The checklist covers six surfaces with a verify command and a recoverability verdict each. The PyPI trusted publisher is marked unrecoverable, because it fails the OIDC exchange after the tag is pushed and a spent version cannot be reused. Two DECISIONS entries. The first states plainly that an inspection checklist is a weak control - manual, staleable, run only by someone who remembers - and that it exists because the alternative is nothing; four incidents share one shape, the repository green and the fault outside it. The second records that corpus containment cannot be solved once: four readers so far, no two sharing a mechanism, and the directory irrelevant to all of them. The About field is not changed here. It is a setting, and settings are yours; the corrected wording is in GITHUB_SETUP.md to be pasted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5nXA9BJKdmye9a9FaHdc6 --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
Addresses R1–R7. Version is
0.2.0inpyproject.toml; nothing is tagged. Do not merge this yourself — and the tag stays with you.Two findings whose severity changed under verification
R1 — artifact actions. The mismatch was real, but "align the majors" isn't possible:
upload-artifactis at v7,download-artifactat v8. Per download-artifact's own release notes, v8 is the documented counterpart to upload-artifact@v7. Both downloads now say v8.The stated failure mode would not have occurred.
upload-artifact@v7's unzipped direct upload is opt-in viaarchive: false, which this job doesn't set, and it supports a single file only —dist/holds a wheel and an sdist. v7 produced a zipped artifact that v4 unzips fine. What it actually cost: v4 runs on Node 20, which GitHub is deprecating and already warns about in this repo's logs, while the build side had moved to Node 24; and v8 defaultsdigest-mismatchtoerrorrather than warn, which is what a release pipeline should do with a corrupted download. Right fix, different reason, lower urgency.R3 — the version lived in two places, not one.
src/agentguard/__init__.py:7also hardcoded it, and that copy feeds--version, the JSONtool.version, and the SARIF driver version. Bumping onlypyproject.tomlwould have published 0.2.0 while every emitted report claimed 0.1.0 — including SARIF uploaded to code scanning.__init__.pynow readsimportlib.metadata, sopyproject.tomlis the only place a number is written.The rest
verifyjob gatesbuild: full suite, both bench scopes, linearity gate, lint, mypy, and the tag/version check. Chain:verify → build → publish → github-release. Exercised againstv0.2.0,v0.2.1,v1.0.0— only the matching tag passes./testsexclusion over corpus-only: excluding only the corpus leavestests/test_corpus.pyin an sdist where it cannot pass, so a packager would see failures that aren't defects..envignored, anchored sotests/corpus/**stays tracked. Verified withgit check-ignore -v.addopts.pytest tests/test_rules.pynow passes; the gate moved tomake test/CI/verifyand was confirmed to still bite (--cov-fail-under=99exits 1).py.typedadded, verified absent first and present in both artifacts after.twine checkraised no classifier warning, but the built metadata embedded the entire Apache licence text in theLicense:field. NowLicense-Expression: Apache-2.0via PEP 639. Fixed now because a release freezes page metadata.publishable_keys.pysat intrue_negatives/while expecting AG001. Moved, so the directory means what it says — 14 positives, 20 negatives, findings one-to-one with files — and a test keeps it that way.Not verified — needs you
The TestPyPI upload did not happen. No
~/.pypirc, noTWINE_*/PYPI_*variable. Installing both artifacts into clean virtualenvs covers version consistency, sdist installability, andpy.typed, but not PyPI's metadata validation on receipt, the OIDC exchange, or page rendering — and the first tag is the first time any of those runs. Recommend running that leg before tagging.Package name — both are unclaimed
agentguard-sastandagentguardboth return 404 on PyPI. The good name is free, so this is a reservation decision, not a squatting one. Recommending a placeholder release underagentguardwhile continuing to publishagentguard-sast; renaming the distribution in the same change that first exercises publishing is a bad trade on the eve of a first tag. Not actioned — it's your namespace.🤖 Generated with Claude Code
https://claude.ai/code/session_01Q5nXA9BJKdmye9a9FaHdc6