Repository navigation
feat: add charm-tech-baseline, the repo-setup audit tool - #2
Draft
tonyandrewmeyer wants to merge 12 commits into
Draft
tonyandrewmeyer wants to merge 12 commits into
tonyandrewmeyer wants to merge 12 commits into
Conversation
tonyandrewmeyer
added a commit
to canonical/charmlint
that referenced
this pull request
Aug 31, 2026
Tags matching `v*` now build an sdist and wheel and publish them to PyPI via Trusted Publishing, following the shape of the template in canonical/charm-tech-code#2, and running the same workflow by hand publishes to TestPyPI instead. * One workflow: the trigger picks the environment, the repository URL, and which of the two mutually exclusive version steps runs, so a manual run now exercises the SBOM generation and both attestations rather than skipping them. * Two attestations rather than one: SLSA provenance for how the artefacts were built, and a CycloneDX SBOM predicate for what is inside them. Both are `actions/attest`, which picks provenance or SBOM depending on whether `sbom-path` is set, and produces one predicate per call. * The publish job fails before the build if the tag and the `version` in `pyproject.toml` disagree, since that version is hand-maintained and nothing else was checking it. The tag reaches the shell through an environment variable rather than being interpolated into the `run:` block. * A manual run rewrites its checked-out `pyproject.toml` to add a `.dev<run number>` suffix. TestPyPI refuses a version it has already seen, so without that, rehearsing the same release twice means burning a version number. The `publish-pypi` and `publish-testpypi` environments exist and the trusted publishers are registered, separately on PyPI and TestPyPI. --------- Signed-off-by: Tony Meyer <tony.meyer@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: James Garner <james.garner@canonical.com>
Collaborator
Author
|
I did not intend to close this 😕. |
tonyandrewmeyer
changed the base branch from
move-ai-failure-notifier
to
main
September 23, 2026 11:06
Collaborator
Author
Ah, it's because it was stacked 😞. |
The audit has been living as 4,500 lines of scripts inside a skill in canonical/charm-tech, where it has no lockfile, no CI and no tests that anything runs. This is the half that is code: 29 checks, 8 mechanical fixes, tier detection, and the templates and question batteries they read. The skill keeps the half that is prose - when a check applies, what a finding means, and which decisions are already settled - and drives this through uvx. Ported rather than rewritten, so the report is byte-identical to what the scripts produced. Three changes were needed to make it a package: * Checks are imported and called by the runner instead of being shelled out to and having their stdout reparsed. emit_check hands the result straight over when a collector is active, and still prints when a check is invoked on its own, which is how the tests drive them. * The runner sets sys.argv for each check rather than letting it read the runner's own command line. That was a real defect: a check only ever saw --tier because the runner happened to have been given the same flag, so a detected tier never reached one. * PyYAML becomes a dependency instead of three `# /// script` blocks. The long message strings are wrapped to the shared 99-column ruff config by implicit concatenation, so no message text changed. Confirmed by diffing a full report against canonical/pebble before and after: the checks and notes are identical.
As of v4, actions/attest-build-provenance is documented as "simply a wrapper on top of actions/attest", and upstream says new implementations should use actions/attest instead. Point both trusted-publishing templates at it, and rewrite the comment in the canonical template that explained the two attestation calls in terms of the wrapper's missing sbom-path input. The attest-build-provenance check matched on the wrapper's name alone, so it would have failed a workflow that took upstream's advice. It now accepts either action, comparing the action name exactly rather than by substring so that actions/attest-sbom does not pass a provenance check on a prefix match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fRGz2wZnuTK1kXNnu7PYx
A separate test-publish workflow drifts from the real one, so the rehearsal stops exercising the steps most likely to break. Both trusted-publishing templates now take a tag push and a manual dispatch, with the trigger selecting the environment, the repository URL and which of the two mutually exclusive version steps runs. A reusable workflow would be tidier, but PyPI validates the job_workflow_ref OIDC claim, so a reusable workflow cannot be the workflow in a trusted publisher (warehouse#11096); the header comments say so, to stop someone refactoring into that shape later. Both templates also gain the tag/version check that charmlint grew, since a hand-maintained version in pyproject.toml is otherwise unchecked. _targets_test_pypi matched TestPyPI hosts anywhere in the repository URL, so the merged publish step, whose URL expression names both hosts, read as TestPyPI-only. That made is_publish_step reject it, and the check then reported 'na' rather than looking for an attestation - a workflow taking the shape we now recommend would silently stop being checked. A step that names a real PyPI host as well as a test one is no longer treated as TestPyPI-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fRGz2wZnuTK1kXNnu7PYx
The reasoning was only in a review thread, which the template does not carry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fRGz2wZnuTK1kXNnu7PYx
…lates The review on canonical/charmlint#199 landed four changes to the workflow that this template is the source for, so bring them back here: * Comment the `environment:` name and url, explaining that the name must match the environment registered with the trusted publisher on each index while the url is only the deployment link in the UI. * Convert the tag/version check and the .dev suffix step from shell to `shell: python`, dropping the `python3 -c` round trip and the sed escaping. * Attach the SBOM-upload comment to the step it explains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015exA46csF9tmRnQ8mLrzvA
api_demo_server, charm-ubuntu, charmlibs, concierge, pebble and pytest-jubilant were already here; charmhub-listing-review, hyrum, jubilant and operator were still stuck in the staging repo's question-batteries-pending/, unreadable by agents-md-battery since it only looks in assets/question-batteries/. Moved unchanged from there. Schema checked against the six already in place (top-level keys, entry keys, verify kinds, answer grades, classifications, hash field lengths) - no mismatches. pytest and ruff both pass, and packaging already ships assets/ by glob (hatchling's default `packages` file selection), so no pyproject.toml change is needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTMLrWGC5hfYJBE8JjQkGc
tonyandrewmeyer
force-pushed
the
feat/charm-tech-baseline
branch
from
September 23, 2026 11:13
3c77f73 to
2d4c1cd
Compare
This branch predates canonical#7, which moved the full `style/python.md` rule set to the monorepo root, so rebasing onto main left 191 findings in `charm-tech-baseline`. None of the changes is a behaviour change. * Every module gets the Apache licence header the other packages carry (`CPY001`). * Docstrings get a one-line summary with the rest moved into the body, and the public functions that had none, mostly each check's and fix's `main()`, get one (`D103`, `D205`, and friends). `cli.py` prints its module docstring as `--help`, so that output gains a blank line after its first line. * A handful of small rewrites: comprehensions or `extend` rather than append loops (`PERF401`), two needless bools (`SIM103`), an unpacked list rather than a concatenation (`RUF005`), and one `contextlib.suppress` (`SIM105`). * The rest is marked rather than changed. `git` and the installed `charm-tech-baseline` script really are called by name (`S607`), and skipping a file that cannot be read or parsed and carrying on is the point of those loops (`PERF203`, `S112`). Long suppressions sit on their own line above the statement, since a trailing `# ruff: ignore[...]` only covers its own physical line. 32 tests pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
canonical#6 listed it as `undocumented-public-inits`, which ruff does not know, so every run warned about an unknown selector and `D107` stayed on despite the comment saying `__init__` does not need a docstring. The rule's name is `undocumented-public-init`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Every mechanical remediation still pointed at `scripts/fixes/<fix>.py` from before the port, which does not exist in the package, while the skill tells an agent that `remediation.script` names the fix to pass to `charm-tech-baseline fix`. It now does, and the prose that mentioned a script path says which command to run instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Tier detection already resolved a personal fork of a canonical/* repo to its upstream, but repo-settings, immutable-releases, sec0045-events and the battery lookup each read the origin remote themselves. Run from a fork, repo-settings reported the fork's settings and never found the upstream's CRA enrolment (it only looks when the owner is canonical), and immutable-releases looked for releases on the fork. The resolution moves out of tier.py into `common.baseline_slug()` and they all use it, as do the two fixes that write the repo's URL into a template. `apply-repo-settings` still patches origin, since that is the repo it has any business changing. The CRA lookup also matches the repo name as a literal rather than inside a regex, where a `.` in a name would match any character. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Run against charmlint, agents-md-content reported five missing paths, four of which were there: * a link to `CONTRIBUTING.md#pull-requests` was looked up with the anchor as part of the filename; * `~/.cache/hyrum/charms` was looked up inside the repo, although a home-directory or absolute path describes the reader's machine; * bare names like `_ast.py` were looked up at the root, when prose uses a bare name for a file whose directory the context makes clear. Links now drop the anchor and query, `~` and absolute paths are not treated as repo paths, and a bare name resolves if it exists anywhere in the repo outside the usual tool and build directories. A path with a directory in it still has to exist as written. The fifth, `_pypi_attest/`, is still reported, correctly: it is a module, not a package. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The check cited "decisions.md § Remote pre-commit hooks", a section the skill has never had, and justified exempting pre-commit-hooks from the tool-pinning rule because it has no Python counterpart. It does: the hooks are console scripts in the `pre-commit-hooks` package on PyPI, so they can run as `language: system` hooks with the version in a dependency group, which is what decisions.md's "Tool pinning" asks for. The remediation now gives that as the preferred fix and SHA-pinning as the alternative. What passes and fails is unchanged, so a repo that already SHA-pinned on the old advice does not start failing. Co-Authored-By: Claude Opus 5.5 (1M context) <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
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 checks and fixes have been living as 4,500 lines of loose scripts inside a skill in
canonical/charm-tech, which gave them no lockfile, no CI, and nothing that ran their tests. This is the half that is code: 29 checks, 8 mechanical fixes, tier detection, and the templates and question batteries they read. The skill keeps the half that is prose, and drives this throughuvx.emit_checkhands the result over directly when a collector is active, and still prints when a check is invoked on its own, which is how the tests drive them.sys.argvfor each check instead of letting it read the runner's own command line. That was a real defect rather than something the port introduced: a check only ever saw--tierbecause the runner happened to have been passed the same flag, so a detected tier never reached one, and only an explicit--tierworked.# /// scriptblocks. The presence-only fallbacks those three carry are left alone, but can no longer trigger.Ported rather than rewritten. The long message strings are wrapped to the root ruff config's 99 columns by implicit concatenation, so no message text changed, and a full report against
canonical/pebbleis byte-identical before and after: checks and notes both. That is the evidence that neither the reflow nor the restructuring changed any behaviour.The skill side is a separate PR against
canonical/charm-tech, and wants merging after this one so it never points at something that is not there.