Skip to content

feat: add charm-tech-baseline, the repo-setup audit tool - #2

Draft
tonyandrewmeyer wants to merge 12 commits into
canonical:mainfrom
tonyandrewmeyer:feat/charm-tech-baseline
Draft

tonyandrewmeyer wants to merge 12 commits into
canonical:mainfrom
tonyandrewmeyer:feat/charm-tech-baseline

Conversation

@tonyandrewmeyer

@tonyandrewmeyer tonyandrewmeyer commented Aug 26, 2026 •

Copy link
Copy Markdown
Collaborator

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 through uvx.

  • The runner imports each check and calls it rather than shelling out and reparsing its stdout. emit_check hands 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.
  • The runner sets sys.argv for 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 --tier because the runner happened to have been passed the same flag, so a detected tier never reached one, and only an explicit --tier worked.
  • PyYAML becomes a dependency rather than three # /// script blocks. 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/pebble is 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.

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>
@tonyandrewmeyer
tonyandrewmeyer deleted the branch canonical:main September 10, 2026 00:58
@tonyandrewmeyer

Copy link
Copy Markdown
Collaborator Author

I did not intend to close this 😕.

@tonyandrewmeyer
tonyandrewmeyer changed the base branch from move-ai-failure-notifier to main September 23, 2026 11:06
@tonyandrewmeyer

Copy link
Copy Markdown
Collaborator Author

I did not intend to close this 😕.

Ah, it's because it was stacked 😞.

tonyandrewmeyer and others added 6 commits September 23, 2026 23:08
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 and others added 6 commits September 23, 2026 23:20
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants