Skip to content

Fix/auto commit contract and test discovery #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MrTango wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix/auto commit contract and test discovery #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52c7406
Add auto-commit evaluation harness
MrTango Aug 15, 2026
f3e2e43
Fix the auto-commit contract in setup and skill install
MrTango Aug 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,20 @@
## 7.0.0b16 (unreleased)


- Nothing changed yet.
- Honour `auto_commit` in `plonecli setup` and add its `--no-git` flag, so the
command no longer commits (or initialises) a repository the user opted out of.
It now reports the commit like `create` and `add`.
[MrTango]

- Commit the skills installed by `plonecli skill install/update --scope project`,
so the project is not left dirty for the next `plonecli add`. Opt out with
`--no-git` or `auto_commit = false`.
[MrTango]

- Locate the copier-templates checkout for the integration tests via the repo's
own `develop/plone/src/copier-templates` and the configured clone, so the
suite no longer silently skips 23 of its 24 cases.
[MrTango]


## 7.0.0b15 (2026-08-15)
Expand Down
2 changes: 2 additions & 0 deletions evals/scaffolding/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
workspaces/
workspaces-git/
results/
results-git/
__pycache__/
*.py[cod]
241 changes: 178 additions & 63 deletions evals/scaffolding/EVALUATION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,89 +1,204 @@
# Scaffolding evaluation findings

> Historical baseline from before the fixes. A full verification run passed
> 245/245 cases with no warnings on 2026-08-13. The ignored
> `results/report.md` is mutable and may instead contain the latest quick or
> CI-validation run.
Run on 2026-08-15 against plonecli `7.0.0b16.dev0` (`c8ebe13`) and
copier-templates `2705bf3`. The ignored `results/` and `results-git/` trees hold
the runtime reports; they are replaced on every run and may contain a later
quick or CI-validation run instead.

The three Medium problems were fixed in plonecli after the run; each carries its
fix and its regression test below. The three Low problems live in
copier-templates and are still open.

## Scope

The full run executed 245 cases against the development templates checkout:
Two harnesses were executed.

`run_evals.py` (full mode, 245 cases) covers:

- all 3 project templates;
- all 20 feature templates individually;
- explicit finite high-interaction matrices for backend/Svelte booleans, behavior booleans, REST booleans crossed with target mode, reachable content-type states, view choices/booleans, vocabulary types, all viewlet managers and template states, and Zope distribution/storage choices;
- combined, reversed-order, repeated-application, hostile-input, and command-chain cases;
- explicit finite high-interaction matrices for backend/Svelte booleans,
behavior booleans, REST booleans crossed with target mode, reachable
content-type states, view choices/booleans, vocabulary types, all viewlet
managers and template states, and Zope distribution/storage choices;
- combined, reversed-order, repeated-application, hostile-input, and
command-chain cases;
- harmless root commands and the CLI command unit suite.

Open-ended strings, integers, and environment-discovered choices cannot have a literal exhaustive Cartesian product. They are covered with defaults, non-default valid values, manual-choice paths, and hostile quote/newline/backslash partitions. Finite domains are exhaustive only in the explicitly named high-interaction matrices; other templates receive individual non-default cases plus their focused unit tests.

Result: **237 passed, 8 failed**. See the ignored runtime report at `results/report.md` and per-case logs under `results/logs/`.

## Problems

### High: free text can generate invalid TOML

`backend_addon` and `zope-setup` interpolate title, description, and author values directly into quoted TOML. Quotes, newlines, and backslashes can produce an invalid `pyproject.toml`.

Evidence:
`run_git_evals.py` (25 cases, new in this round) covers the auto-commit path
that `run_evals.py` deliberately disables — see "Auto-commit verification".

- `hostile-backend-toml-strings`: generation returned success, but TOML validation failed.
- `hostile-zope-toml-strings`: the generated TOML was invalid and the post-copy hook aborted while parsing it.
Open-ended strings, integers, and environment-discovered choices cannot have a
literal exhaustive Cartesian product. They are covered with defaults,
non-default valid values, manual-choice paths, and hostile
quote/newline/backslash partitions. Finite domains are exhaustive only in the
explicitly named high-interaction matrices; other templates receive individual
non-default cases plus their focused unit tests.

Use a TOML-safe Jinja filter or generate these values through `tomlkit` instead of interpolating raw strings.
## Result

### High: `zope_instance` cannot be added through plonecli
**245 passed, 0 failed** in the scaffolding matrix (1050s), and **25 passed, 0
failed** in the auto-commit harness (40s).

All four `zope_instance` CLI cases failed because `plonecli add` did not list `zope_instance` in a standalone `zope-setup` project. The generated project contains both `[tool.plone.project.settings]` and `[tool.plone.backend_addon.settings]`. Project detection checks backend settings first, classifies the project as `backend_addon`, and exposes the wrong subtemplate set.
Every problem in the previous round is fixed and stayed fixed: free-text TOML
values serialize safely, standalone Zope projects are detected as such,
`create` → `setup` chains refresh the project context, conflicting theme
overlays are rejected, and the Barceloneta integration test targets the
generated package path. The copier-template suite now emits **zero** warnings,
so the deprecated `ContextHook.update` and `click.__version__` opportunities are
also closed.

Either avoid writing backend-addon settings for standalone Zope projects or make project detection prefer the substantive project settings in this mixed layout.
## Auto-commit verification

### High: chained `create` then `setup` fails
`plonecli` promises that every generated or modified file is committed, so a
package always has reviewable history. `run_evals.py` cannot check that: it
passes `--no-git` on every case to keep generated trees disposable, which left
the default user-facing path with no coverage at all.

The CLI declares `chain=True`, but `chain-create-then-setup` failed after successfully creating the backend add-on. The group retains the project context detected before `create`, so `setup` still reports that it is outside a package.
`run_git_evals.py` closes that gap. It runs the same commands with git enabled
and, after each one, asserts the project is a git repository, that
`git status --porcelain` is empty, and that the run produced the expected new
commit.

Refresh project context after creation, or remove command chaining if cross-context chains are not supported.
| Command | Cases | Result |
|---|---|---|
| `create backend_addon` / `zope-setup` / `addon` | 3 | clean tree, one commit per template step |
| `skill install --scope project` | 1 | clean tree, `Add plonecli skills` |
| `setup` | 1 | clean tree, `Add zope-setup template` |
| `add <subtemplate>` (all 19) + `add zope_instance` | 20 | clean tree, `Add <name> subtemplate` |

### Medium: theme variants conflict without non-interactive resolution
`serve`, `debug`, and `test` delegate to invoke and write only into gitignored
paths (`runtime/*/var/`, `.pytest_cache/`, `.coverage`); they are not expected
to commit and are covered by the root CLI command unit suite. `config`,
`update` and `completion` do not touch a project tree.

The all-feature sequence failed when `theme_barceloneta` followed `theme`: both own `profiles/default/theme.xml` and related theme paths. Copier requested an interactive overwrite despite `--defaults`, then aborted in the non-TTY evaluation.
Two commands did **not** hold the contract — both are fixed, see problems 1 and
2 below.

Treat theme templates as explicit alternatives and reject a second theme with a clear message, or add a documented overwrite/replacement flow.

### Medium: Barceloneta integration test uses a stale path

The root integration suite generated the test at `src/collective/mythemetest/tests/test_theme_my_test_theme.py`, but `tests/test_theme_barceloneta_integration.py` expects it under top-level `tests/`. Result: 23 integration cases passed and 1 failed.
## Problems

Update the assertion and pytest target to the generated `src/<package>/tests/` layout.
### Fixed, was Medium: `plonecli setup` ignored `auto_commit = false` and had no `--no-git`

`cli.py` called `run_create()` without a `git_commit` argument, and
`templates.py:139` defaults it to `True`. `create` and `add` both compute
`config.auto_commit and not no_git`; `setup` computed nothing and always
committed. It was also the only file-writing command with no `--no-git` flag,
and it discarded `run_create`'s return value, so it never printed the
`Committed: ...` line the other two print.

Evidence: with `auto_commit = false` in `~/.plonecli/config.toml`, `create`
correctly produced a package with no repository, and the following `setup` ran
`git init` and committed it anyway — as `Create package with zope-setup
template`, for a package it did not create.

**Fixed.** `setup` now passes `git_commit=config.auto_commit and not no_git`,
accepts `--no-git`, carries a chained `--no-git` across `create ... setup`, and
echoes the returned commit message. Re-verified end to end: with
`auto_commit = false` the package still has no repository after `setup`; with
the default it commits `Add zope-setup template` and leaves a clean tree.
Covered by `tests/test_setup_command.py` and `tests/test_plonecli.py`.

### Fixed, was Medium: `plonecli skill install --scope project` left the repo dirty

The installer writes `.agents/skills/<name>` and `.claude/skills/<name>` into
the project root and never committed. Neither path is in the generated
`.gitignore`, so the working tree was left with two untracked directories.

That broke the next command: `plonecli add <sub> --defaults` aborted with
"Refusing to run on a git repository with uncommitted changes", so plonecli's
own command put a project into a state plonecli refuses to work in.

**Fixed.** A project-scope `install`/`update` now commits the installed skills
under the same `auto_commit` contract, as `Add plonecli skills` /
`Update plonecli skills`, with `--no-git` to opt out. It uses the new
`git.commit_paths()`, which commits only the skill directories: it never runs
`git init` and never sweeps unrelated user changes into the commit. User-scope
installs are untouched. Covered by `tests/test_skill.py` and
`tests/test_git.py`.

### Fixed, was Medium: the root integration suite silently skipped 23 of its 24 cases

`_templates_dir()` in `tests/test_theme_barceloneta_integration.py` and
`tests/test_all_templates_data.py` resolved the template checkout from
`PLONECLI_TEMPLATES_DIR` or two hard-coded `/home/node/...` paths. This
repository keeps its development checkout at `develop/plone/src/copier-templates`
(per `AGENTS.md`), which is neither.

So a plain `uv run pytest -m integration` reported "1 passed, 2 skipped" —
green, and almost entirely empty, because the parametrized template sweep
collapsed to an empty parameter set.

**Fixed.** All three integration modules now share
`tests/helpers.find_templates_checkout()`, which tries `PLONECLI_TEMPLATES_DIR`,
the repo-relative `develop/plone/src/copier-templates`, the configured
`PlonecliConfig().templates_dir`, then the devcontainer paths. A plain
`uv run pytest -m integration --collect-only` now collects 24 cases with no
environment variable set. Covered by `tests/test_templates_checkout.py`, whose
last test asserts the sweep is populated whenever a checkout exists — the
regression itself.

### Low: the template git warning prints a mangled filename

`shared/hooks/git_check.py` calls `.strip()` on the whole `git status
--porcelain` output before splitting it into lines. Porcelain lines are
`XY<space><path>`, so stripping removes the leading status space of the *first*
line and shifts every subsequent slice by one: ` M .gitignore` is reported as
`gitignore`.

Observed in the clean-tree `setup` run:

```text
Modified files:
- gitignore
```

Split with `splitlines()` on the unstripped output. `plonecli/git.py` already
does this correctly; only the template hook is affected.

### Low: the git warning fires on changes plonecli itself just made

The same warning appears during `setup` and `add zope_instance` even when the
tree was clean before the command, because `zope-setup` invokes the
`zope_instance` template from a post-copy hook after the current copy has
already written files. The user is told to commit or stash changes that
plonecli made moments earlier and is about to commit itself.

Scope the check to the state before the copy operation, or suppress it for
nested template invocations.

### Low: `plonecli setup` replaces the backend addon's `.gitignore` wholesale

`backend_addon` and `zope-setup` each ship a full `.gitignore.jinja`, and
`setup` overwrites the first with the second. Today the loss is only
`Thumbs.db` — `dist/` and `.cache` are still matched by the remaining bare
patterns, verified with `git check-ignore` — so there is no functional impact.
But the two files are independently maintained copies, so any backend-only
ignore rule added later disappears on `setup` without a signal.

Share one base fragment between the two templates, or append instead of
overwrite.

## Optimization opportunities

- Copier template extensions emitted hundreds of deprecation warnings because `ContextHook.update` is deprecated. Migrate hooks to modify context in `hook`.
- `click_aliases` reads deprecated `click.__version__`; update or replace the dependency before Click 9.1.
- Feature generation inside these nested, `--no-git` workspaces reports the outer plonecli repository as dirty. Git cleanliness checks should be scoped to the detected generated project rather than walking into an unrelated parent repository.
- Keep the generated TOML/XML/Python validators as CI checks. They found failures that successful Copier exit codes did not detect.

## Resolution

All findings above have been addressed:

- free-text TOML values use serialization filters;
- standalone Zope projects are detected correctly;
- chained `create` → `setup` refreshes project context;
- theme variants reject conflicting overlays;
- the Barceloneta integration test uses the generated package test path;
- context hooks use the current in-place API;
- the deprecated command-alias dependency was removed;
- Git checks are scoped to the generated project;
- subtemplate validation tasks use Copier's `_copier_operation` value and no
longer report files generated earlier in the same copy as pre-existing
changes;
- generated TOML/XML/Python validation runs in CI.

## Baseline test receipts

- Root unit suite: **209 passed, 16 skipped**.
- Copier-template unit suite: **386 passed, 2 integration tests deselected**.
- CI runs only `run_evals.py --ci-validation`. Add `run_git_evals.py --quick`
so the auto-commit contract cannot regress unnoticed. The integration suite no
longer needs `PLONECLI_TEMPLATES_DIR` to find the checkout, but setting it in
CI still pins which checkout is used.
- The five `WARNING: Git repository has uncommitted changes!` entries in the
matrix report are the two Low findings above, not template failures; fixing
them removes the noise from the report's warning section.
- Keep the generated TOML/XML/Python validators as CI checks. They caught
failures in the previous round that successful Copier exit codes did not.

## Test receipts

- Root unit suite: **280 passed, 12 skipped** (the skips are the model-billing
skill evals; 216 passed / 1 skipped before the third fix, which restored the
template sweep and added the new tests).
- Root integration suite: **24 collected and passed**, with or without
`PLONECLI_TEMPLATES_DIR`.
- Copier-template unit suite: **391 passed**, 2 integration tests deselected,
**0 warnings**.
- Copier-template integration suite: **2 passed**.
- Root integration suite: **23 passed, 1 failed** (stale Barceloneta test path above).
- Full scaffolding matrix: **199 passed, 8 failed** (the eight cases map to four product problems above).
- Full scaffolding matrix: **245 passed, 0 failed**.
- Auto-commit harness: **25 passed, 0 failed**.
16 changes: 16 additions & 0 deletions evals/scaffolding/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,22 @@ repository template inventory and fails when a new template has no lane. The run
PLONECLI_TEMPLATES_DIR=/workspaces/plonecli/develop/plone/src/copier-templates
```

`run_git_evals.py` is the complement. Because the matrix above passes
`--no-git` everywhere, it never exercises auto-commit — the behaviour users get
by default. This harness runs the same commands with git enabled and asserts,
after each one, that the project is a git repository, that `git status
--porcelain` is empty, and that the expected commit was created:

```sh
uv run python evals/scaffolding/run_git_evals.py --quick
uv run python evals/scaffolding/run_git_evals.py
```

`--quick` runs only the project-level cases (`create` × 3, `skill install
--scope project`, `setup`); the full run adds every subtemplate plus
`zope_instance`. It writes to `workspaces-git/` and `results-git/`, so the two
harnesses never share state.

Generated trees are disposable and always live beneath `workspaces/`. Reports
and per-case command logs are written beneath `results/`:

Expand Down
Loading
Loading