mirror of
https://github.com/Akkudoktor-EOS/EOS.git
synced 2026-11-13 09:13:30 +01:00
chore(ai): add agent self-review and a CI check for agent-assisted PRs
Additional changes Agents write a task plan to .scratch/<branch>/plan.md and self-review with code-review Agents fix hard violations, report judgement calls and rerun the self-review before every push Agents declare the review in a Self-review section of the pull request body Add the ai-self-review workflow and scripts/check_pr_self_review.py for pull requests into main Detect agents by Assisted-by or Anthropic Co-Authored-By trailers, body markers or the AI box Fail only on a missing Self-review section or a missing, malformed or stale Reviewed SHA Note an unticked AI-assisted box and malformed optional lines without failing Comment on the pull request how to fix a failure and update that one comment on later runs Show the same report in the job summary and accept Windows line endings in the body Skip dependabot and release pull requests Run on pull_request_target with the base branch's script, never pull request code Add the pull request template: the pr skill's sections plus Checklist and Self-review Require always valid optimization parameters in the energy-domain skill Require datetimeutil for date and time handling in new and changed code in AGENTS.md Require the Assisted-by trailer on agent commits in the commit-messages skill Add tests for the check in tests/test_ai.py Add GLOSSARY.md and the agent tracker docs in docs/agents/ Add decision record docs/adr/0001 with an ADR index in the docs toctree Document the workflow in docs/develop/ai.md and CONTRIBUTING.md Exclude docs/agents from Sphinx and PyMarkdown and ignore .scratch/ Document cz check instead of make gitlint for commit messages in docs/develop/ai.md Assisted-by: Claude Code (claude-opus-5-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Bobby Noelte
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
62cbf4cf2a
commit
dd41e5d57c
@@ -0,0 +1,32 @@
|
||||
# CI enforces a self-declared, SHA-bound self-review for agent-assisted pull requests
|
||||
|
||||
Pull requests that a coding agent wrote or co-wrote must carry a `## Self-review` section in the
|
||||
pull request body that names the full head commit SHA it reviewed. CI does not verify the review
|
||||
itself: the project has no AI in CI, and a signing key in the sandbox would be reachable by a misled
|
||||
agent. Instead CI detects agent assistance (an `Assisted-by:` or Anthropic `Co-Authored-By:` commit
|
||||
trailer, an `Assisted-by:` line or a known agent footer in the pull request body, or the
|
||||
"AI-assisted" box in the pull request template). It fails only when the declaration or its
|
||||
`Reviewed:` SHA is missing or malformed, or names a SHA other than the current head; everything
|
||||
else (an unticked box next to a trailer or marker, malformed optional lines) is only noted. Like a
|
||||
DCO sign-off, this makes forgetting impossible and a false claim deliberate; the author review and
|
||||
the maintainers' review remain the actual controls.
|
||||
|
||||
## Considered Options
|
||||
|
||||
- **AI review in CI**: rejected, the project runs no AI in CI.
|
||||
- **Signed review attestation from the sandbox**: rejected, the agent can reach the key and sign
|
||||
anything.
|
||||
- **Honour system (template checkbox only)**: rejected, a declaration that is not bound to a commit
|
||||
silently covers code pushed after the review.
|
||||
|
||||
## Consequences
|
||||
|
||||
- Every push to an agent-assisted pull request needs a fresh self-review before the push and an
|
||||
updated pull request body right after it; rebases and squashes before merge do too.
|
||||
- Open findings do not fail the check, so agents have no incentive to hide them.
|
||||
- The check only blocks merging once maintainers make it a required status check.
|
||||
- The workflow runs on `pull_request_target` with the base branch's script and never runs pull
|
||||
request code, so a pull request cannot weaken its own check; changes to the check take effect
|
||||
only after they are merged.
|
||||
- The workflow needs `pull-requests: write` to comment how to fix a failing check. That is safe
|
||||
only as long as it runs no pull request code.
|
||||
@@ -0,0 +1,11 @@
|
||||
# Architecture Decision Records
|
||||
|
||||
Decisions that are hard to reverse, surprising without context and the result of a real trade-off.
|
||||
Each record states the context, the decision and why it was made.
|
||||
|
||||
```{toctree}
|
||||
:maxdepth: 1
|
||||
:glob:
|
||||
|
||||
0*
|
||||
```
|
||||
@@ -0,0 +1,51 @@
|
||||
# Domain Docs
|
||||
|
||||
How the engineering skills should consume this repo's domain documentation when exploring the codebase.
|
||||
|
||||
## Before exploring, read these
|
||||
|
||||
- **`GLOSSARY.md`** at the repo root, or
|
||||
- **`GLOSSARY-MAP.md`** at the repo root if it exists: it points at one `GLOSSARY.md` per context. Read each one relevant to the topic.
|
||||
- **`docs/adr/`**: read ADRs that touch the area you're about to work in. In multi-context repos, also check `src/<context>/docs/adr/` for context-scoped decisions.
|
||||
|
||||
If any of these files don't exist, **proceed silently**. Don't flag their absence; don't suggest creating them upfront. The `/domain-modeling` skill (reached via `/grill-with-docs` and `/improve-codebase-architecture`) creates them lazily when terms or decisions actually get resolved.
|
||||
|
||||
## File structure
|
||||
|
||||
Single-context repo (most repos):
|
||||
|
||||
```
|
||||
/
|
||||
├── GLOSSARY.md
|
||||
├── docs/adr/
|
||||
│ ├── 0001-event-sourced-orders.md
|
||||
│ └── 0002-postgres-for-write-model.md
|
||||
└── src/
|
||||
```
|
||||
|
||||
Multi-context repo (presence of `GLOSSARY-MAP.md` at the root):
|
||||
|
||||
```
|
||||
/
|
||||
├── GLOSSARY-MAP.md
|
||||
├── docs/adr/ ← system-wide decisions
|
||||
└── src/
|
||||
├── ordering/
|
||||
│ ├── GLOSSARY.md
|
||||
│ └── docs/adr/ ← context-specific decisions
|
||||
└── billing/
|
||||
├── GLOSSARY.md
|
||||
└── docs/adr/
|
||||
```
|
||||
|
||||
## Use the glossary's vocabulary
|
||||
|
||||
When your output names a domain concept (in an issue title, a refactor proposal, a hypothesis, a test name), use the term as defined in `GLOSSARY.md`. Don't drift to synonyms the glossary explicitly avoids.
|
||||
|
||||
If the concept you need isn't in the glossary yet, that's a signal: either you're inventing language the project doesn't use (reconsider) or there's a real gap (note it for `/domain-modeling`).
|
||||
|
||||
## Flag ADR conflicts
|
||||
|
||||
If your output contradicts an existing ADR, surface it explicitly rather than silently overriding:
|
||||
|
||||
> _Contradicts ADR-0007 (event-sourced orders), but worth reopening because…_
|
||||
@@ -0,0 +1,56 @@
|
||||
# Issue tracker: GitHub
|
||||
|
||||
Issues and specs for this repo live as GitHub issues of `Akkudoktor-EOS/EOS`. Use the `gh` CLI for all operations.
|
||||
|
||||
## Conventions
|
||||
|
||||
- **Create an issue**: `gh issue create --title "..." --body "..."`. Use a heredoc for multi-line bodies.
|
||||
- **Read an issue**: `gh issue view <number> --json number,title,body,labels,comments`.
|
||||
- **List issues**: `gh issue list --state open --json number,title,body,labels,comments --jq '[.[] | {number, title, body, labels: [.labels[].name], comments: [.comments[].body]}]'` with appropriate `--label` and `--state` filters.
|
||||
- **Make an issue a sub-issue of a parent**: `gh issue create --parent <parent> ...`, or `gh issue edit <parent> --add-sub-issue <child>` afterwards (`gh` 2.94+). Older `gh`: `gh api --method POST repos/<owner>/<repo>/issues/<parent>/sub_issues -F sub_issue_id=<child-db-id>` (database id, as in **Blocking** below). Without sub-issues, put `Part of #<parent>` at the top of the child body.
|
||||
- **Blocking**: GitHub's **native issue dependencies**, the canonical, UI-visible representation. Add an edge with `gh issue create --blocked-by <n>,<n> ...` or `gh issue edit <child> --add-blocked-by <blocker>` (`gh` 2.94+). Older `gh`: `gh api --method POST repos/<owner>/<repo>/issues/<child>/dependencies/blocked_by -F issue_id=<blocker-db-id>`, where `<blocker-db-id>` is the blocker's numeric **database id** (`gh api repos/<owner>/<repo>/issues/<n> --jq .id`, _not_ the `#number` or `node_id`). Where dependencies aren't available, fall back to a `Blocked by: #<n>, #<n>` line at the top of the child body. A ticket is unblocked when every blocker is closed.
|
||||
- **Count open blockers**: `gh api repos/<owner>/<repo>/issues/<n> --jq .issue_dependencies_summary.blocked_by` (open blockers only; 0 means startable). `gh issue view <n> --json blockedBy` also lists closed blockers, so filter it: `--jq '[.blockedBy.nodes[] | select(.state == "OPEN")] | length'`.
|
||||
- **Comment on an issue**: `gh issue comment <number> --body "..."`
|
||||
- **Apply / remove labels**: `gh issue edit <number> --add-label "..."` / `--remove-label "..."`
|
||||
- **Close**: post the explanation with `gh issue comment <number> --body "..."`, then `gh issue close <number>`. On an already-closed issue, `gh issue close --comment` drops the comment and still exits 0.
|
||||
|
||||
The tracker is the upstream repository **`Akkudoktor-EOS/EOS`**, not a contributor's fork. Pass
|
||||
`--repo Akkudoktor-EOS/EOS` to every `gh issue` command, because a clone of a fork may resolve to
|
||||
the fork.
|
||||
|
||||
Reading issues needs no confirmation. Before creating, editing, labelling, commenting on or closing
|
||||
an issue, show the content to the user and wait for their go: these are public actions in the
|
||||
upstream project. Never file into `mattpocock/skills` (the repo these skills come from).
|
||||
|
||||
Labels are created by the EOS maintainers only; do not run `gh label create`.
|
||||
|
||||
## Pull requests as a triage surface
|
||||
|
||||
**PRs as a request surface: no.** _(Set to `yes` if this repo treats external PRs as feature requests; `/triage` reads this flag.)_
|
||||
|
||||
When set to `yes`, PRs run through the same labels and states as issues, using the `gh pr` equivalents:
|
||||
|
||||
- **Read a PR**: `gh pr view <number>`, then `gh pr view <number> --comments` (off a TTY, `--comments` prints only the comments, not the body), and `gh pr diff <number>` for the diff.
|
||||
- **List external PRs for triage**: `gh api --paginate 'repos/{owner}/{repo}/pulls?state=open' --jq '.[] | select(.author_association | IN("OWNER","MEMBER","COLLABORATOR") | not) | {number, title, author: .user.login, author_association, labels: [.labels[].name]}'`.
|
||||
- **Comment / label / close**: `gh pr comment`, `gh pr edit --add-label`/`--remove-label`, `gh pr close`.
|
||||
|
||||
GitHub shares one number space across issues and PRs, so a bare `#42` may be either: resolve with `gh pr view 42` and fall back to `gh issue view 42`.
|
||||
|
||||
## When a skill says "publish to the issue tracker"
|
||||
|
||||
Create a GitHub issue.
|
||||
|
||||
## When a skill says "fetch the relevant ticket"
|
||||
|
||||
Read it as in **Read an issue** above.
|
||||
|
||||
## Wayfinding operations
|
||||
|
||||
Used by `/wayfinder`. The **map** is a single issue with **child** issues as tickets.
|
||||
|
||||
- **Map**: a single issue labelled `wayfinder:map`, holding the Notes / Decisions-so-far / Fog body. `gh issue create --label wayfinder:map`.
|
||||
- **Child ticket**: an issue linked to the map as a GitHub sub-issue (see **Make an issue a sub-issue of a parent**). Where sub-issues aren't enabled, add the child to a task list in the map body and put `Part of #<map>` at the top of the child body. Labels: `wayfinder:<type>` (`research`/`prototype`/`grilling`/`task`). Once claimed, the ticket is assigned to the driving dev.
|
||||
- **Blocking**: see **Blocking** under Conventions.
|
||||
- **Frontier query**: list the map's open children (`gh issue list --state open`, scoped to the map's sub-issues / task list), drop any with an open blocker (see **Count open blockers**, or an open issue in the `Blocked by` line) or an assignee; first in map order wins.
|
||||
- **Claim**: `gh issue edit <n> --add-assignee @me`, the session's first write.
|
||||
- **Resolve**: `gh issue comment <n> --body "<answer>"`, then `gh issue close <n>`, then append a context pointer (gist + link) to the map's Decisions-so-far.
|
||||
+2
-1
@@ -41,7 +41,8 @@ extensions = [
|
||||
]
|
||||
|
||||
templates_path = ["_templates"]
|
||||
exclude_patterns = []
|
||||
# docs/agents/ holds instructions for coding agents, not user documentation.
|
||||
exclude_patterns = ["agents/**"]
|
||||
|
||||
source_suffix = {
|
||||
".rst": "restructuredtext",
|
||||
|
||||
+137
-4
@@ -77,17 +77,24 @@ the skills, start the sandbox with `AI_SANDBOX_SKILLS_RW=1`.
|
||||
3. **Plan before larger changes.** For anything beyond a small fix, ask the agent for a short
|
||||
design note or plan first and agree on it before code is written. Claude Code has a plan mode
|
||||
(`Shift+Tab`) for this.
|
||||
The agent writes the agreed plan to `.scratch/<branch>/plan.md` (the task plan, see
|
||||
[](#ai-self-review)).
|
||||
4. **Let the agent verify its work.** The skills require the agent to add tests and to run
|
||||
pytest, ruff, mypy and the filename linter. Ask for the results if they are not reported.
|
||||
At the end of the task the agent runs its self-review and writes the pull request body, see
|
||||
[](#ai-self-review).
|
||||
5. **Watch and review.** Open the repository in your IDE on the host. You can follow the edits
|
||||
live, answer the agent's questions with full knowledge of the code, run the result yourself and
|
||||
change things by hand. Review the complete diff before you open or update a pull request, as
|
||||
described in [](#ai-review-checklist). You are the author of what goes upstream.
|
||||
6. **Commit in small steps.** Let the agent commit each logical step with a Conventional Commits
|
||||
message (see the `commit-messages` skill), or commit yourself. Check the messages with
|
||||
`make gitlint`.
|
||||
message (see the `commit-messages` skill), or commit yourself. Agent-written commits end
|
||||
with an `Assisted-by:` trailer. Check the messages with commitizen, which works on every
|
||||
platform: `uv run --frozen cz check --rev-range main..HEAD`.
|
||||
7. **Push and open the pull request from the host.** Then the sandbox needs no GitHub
|
||||
credentials at all. See [](#ai-sandbox-github) if you want the agent to push.
|
||||
credentials at all. See [](#ai-sandbox-github) if you want the agent to push. Use the body the
|
||||
agent wrote. Before every further push, let the agent rerun the self-review and rewrite the
|
||||
body, and replace the body right after the push, see [](#ai-self-review).
|
||||
|
||||
(ai-engineering-skills)=
|
||||
|
||||
@@ -145,6 +152,132 @@ the update needs `AI_SANDBOX_SKILLS_RW=1`, because `ai/skills` is read-only ther
|
||||
If you also installed the skills as an agent plugin, e.g. `mattpocock-skills` in Claude Code, the
|
||||
agent sees every skill twice. Use either the plugin or the copy in the repository.
|
||||
|
||||
(ai-self-review)=
|
||||
|
||||
## Self-review and the pull request check
|
||||
|
||||
### Task plan
|
||||
|
||||
For a task beyond a small fix, the agent writes the plan you agreed on to
|
||||
`.scratch/<branch>/plan.md`, e.g. `.scratch/feat/profile/plan.md`. `.scratch/` is ignored by git
|
||||
and outlives the session, so a later session can pick up the plan. The task plan is the spec the
|
||||
self-review checks the change against, unless the commits reference a GitHub issue: then the issue
|
||||
is the spec.
|
||||
|
||||
### Self-review
|
||||
|
||||
When the task is done, the agent reviews its own change with the `code-review` engineering skill
|
||||
against the base branch, usually `main`. The review has two separate parts:
|
||||
|
||||
- **Standards**: does the change follow `CONTRIBUTING.md`, `AGENTS.md` and the project skills?
|
||||
- **Spec**: does it do what the task plan or the issue asked for, and nothing else?
|
||||
|
||||
The agent fixes every **hard violation** (a breach of a documented project rule) itself, then
|
||||
reruns the checks and the self-review. It reports **judgement calls** (code smells, possible scope
|
||||
creep) to you and leaves the decision to you. The self-review is advisory: it catches cheap
|
||||
mistakes before you spend time on them. It does not replace your author review, and it is no
|
||||
security control, because a misled agent reviews its own change (see [](#ai-review-checklist)).
|
||||
|
||||
### Pull request body
|
||||
|
||||
The agent writes the pull request body to `.scratch/<branch>/pr.md` with the `pr` engineering
|
||||
skill. It follows `.github/pull_request_template.md`, which uses the skill's sections, so agents
|
||||
and people write the same structure:
|
||||
|
||||
- `## Summary`: the smallest view that makes the change clear (pseudocode, a call or file tree, a
|
||||
diff sketch or a diagram) with short text,
|
||||
- `## Evidence`: before and after, e.g. a failing and a passing test run,
|
||||
- `## Merge Danger`: whether the change is a one-way or two-way door, its blast radius, and
|
||||
optional **Breaking:** and **Security:** lines,
|
||||
- `## Checklist`: the "AI-assisted" and author-review boxes,
|
||||
- `## Self-review`: the self-review declaration:
|
||||
|
||||
```markdown
|
||||
## Self-review
|
||||
|
||||
Reviewed: 3f2a9c1e5b7d4a6f8e0c2b4d6f8a0c2e4b6d8f0a
|
||||
Base: main
|
||||
Standards: 0 hard, 2 judgement
|
||||
Spec: 1 open (source: .scratch/feat/profile/plan.md)
|
||||
|
||||
- judgement: possible Feature Envy in `geneticparams.py`, left as is because ...
|
||||
- spec: ...
|
||||
|
||||
Assisted-by: Claude Code (claude-opus-5-5)
|
||||
```
|
||||
|
||||
`Reviewed:` is the full SHA of the commit the review ran on. `Spec:` names the source of the spec:
|
||||
an issue (`#123`), the task plan, or `none`.
|
||||
|
||||
Open the pull request from the host with that body. Before every further push, let the agent
|
||||
rerun the self-review and rewrite `pr.md`, not just edit the SHA; replace the body right after the
|
||||
push:
|
||||
|
||||
```bash
|
||||
gh pr create --repo Akkudoktor-EOS/EOS --body-file .scratch/<branch>/pr.md ...
|
||||
gh pr edit <number> --repo Akkudoktor-EOS/EOS --body-file .scratch/<branch>/pr.md
|
||||
```
|
||||
|
||||
Without `gh`, do it by hand on GitHub: paste the content of `pr.md` as the description when you
|
||||
open the pull request. To replace it later, open the "..." menu of the pull request description,
|
||||
choose "Edit", replace the whole text with the content of `pr.md` and click "Update comment".
|
||||
|
||||
### The pull request check
|
||||
|
||||
The CI workflow `ai-self-review` (`scripts/check_pr_self_review.py`) treats a pull request as
|
||||
agent-assisted when
|
||||
|
||||
- any commit carries an `Assisted-by:` trailer (or Claude Code's
|
||||
`Co-Authored-By: ... <noreply@anthropic.com>`),
|
||||
- the body carries an agent marker: an `Assisted-by:` line, or a footer an agent writes on its own,
|
||||
such as Claude Code's `Generated with [Claude Code](https://claude.com/claude-code)` (the check
|
||||
matches the Markdown text `Generated with [Claude Code]`), or
|
||||
- the "AI-assisted" box in the body is ticked.
|
||||
|
||||
So a pull request that an agent opened is detected even when its commits were written by hand.
|
||||
Template comments (`<!-- ... -->`) never count. The check then fails only when
|
||||
|
||||
- the body has no `## Self-review` section, or the section has no `Reviewed:` line,
|
||||
- `Reviewed:` is not the full 40-character lowercase SHA of the current head of the pull request.
|
||||
|
||||
Everything else is only noted, without failing the check: as a warning in the log, in the job
|
||||
summary, and in the comment if the pull request already has one. A passing check never adds a new
|
||||
comment because of notes. Notes are given for:
|
||||
|
||||
- a commit or the body carries an agent marker but the box is not ticked,
|
||||
- `Base:`, `Standards:` or `Spec:` is present but malformed: the counts must be numbers and
|
||||
`Spec:` needs a source, as in the format above.
|
||||
|
||||
Agents still write all four lines; the check requires only `Reviewed:`, because the SHA is what
|
||||
binds the declaration to the code. A missing optional line is not reported.
|
||||
|
||||
Open findings do not fail the check. Dependabot and release pull requests are skipped.
|
||||
|
||||
When the check fails, the workflow posts a comment on the pull request: what is wrong, how to fix
|
||||
it, and the expected format of the section. It keeps one such comment per pull request and updates
|
||||
it on every run, so pushes and body edits do not add new comments; once the check passes, the
|
||||
comment says so. The same report appears in the job summary of the run. Values taken from the
|
||||
pull request body or the commit messages appear in the comment only inside a code block, so they
|
||||
cannot mention anyone or inject Markdown. If the check script itself fails, the comment says that
|
||||
the check could not run and points to the log of the workflow run; the check then fails.
|
||||
|
||||
Every push changes the head, so the check fails until the body names the new head. Therefore the
|
||||
self-review is rerun and `pr.md` rewritten before the push, and the body is replaced right after
|
||||
it. The check run started by the push sees the old body; the run started by the body edit cancels
|
||||
it if it is still running, and decides the result. This is deliberate: a declaration must not
|
||||
silently cover code pushed after the review.
|
||||
Rebases and squashes before the merge need a fresh declaration too.
|
||||
|
||||
The check cannot tell a real review from a typed-in SHA, as no AI runs in CI. Like a sign-off, it
|
||||
makes forgetting impossible and a false claim deliberate. Your author review and the maintainers'
|
||||
review remain the controls. See the decision record
|
||||
[0001](../adr/0001-self-declared-sha-bound-review-gate.md).
|
||||
|
||||
The workflow runs on `pull_request_target`: it uses the workflow and the script of the base
|
||||
branch, never the code of the pull request, so a pull request cannot weaken its own check. It runs
|
||||
only for pull requests into `main`, never on pushes to `main` or to other branches, and not for
|
||||
pull requests into other branches.
|
||||
|
||||
## The AI development sandbox
|
||||
|
||||
An AI agent runs commands on your behalf, and it can be misled, for example by instructions hidden
|
||||
@@ -292,7 +425,7 @@ The usual commands work:
|
||||
| Pre-commit hooks on committed changes | `uv run --frozen pre-commit run --from-ref main --to-ref HEAD` |
|
||||
| Pre-commit hooks on changed files | `uv run --frozen pre-commit run --files <file> ...` |
|
||||
| Check file names | `uv run --frozen python scripts/lint_filenames.py` |
|
||||
| Check commit messages since `main` | `make gitlint` |
|
||||
| Check commit messages since `main` | `uv run --frozen cz check --rev-range main..HEAD` |
|
||||
| Generate the documentation | `make gen-docs` |
|
||||
| Start the EOS server for development | `make run-dev` (see [](#ai-sandbox-servers)) |
|
||||
| Start EOSdash for development | `make run-dash-dev` (see [](#ai-sandbox-servers)) |
|
||||
|
||||
@@ -68,6 +68,7 @@ develop/develop.md
|
||||
develop/ai.md
|
||||
develop/release.md
|
||||
develop/async.md
|
||||
adr/index.md
|
||||
develop/CHANGELOG.md
|
||||
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user