Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
  mathiasandClaude Sonnet 5.5
|
eb8e6d5169 | ||
|
mathias
|
e8521c2555 | ||
|
mathiasandClaude Sonnet 5.5
|
e696db2e68 | ||
|
mathias
|
d1e501cc64 | ||
|
mathias
|
d3faffc4c4 | ||
|
mathiasandClaude Opus 5.5
|
dbf78a3b34 | ||
|
mathias
|
c3f8d6c002 |
+16
-4
@@ -37,9 +37,9 @@
|
||||
|
||||
### Git
|
||||
- Conventional commits: `feat:`, `fix:`, `chore:`, `docs:`, `refactor:`
|
||||
- **Trunk-Based Development:** commit directly to main. One logical change per commit.
|
||||
- **Trunk-Based Development:** one logical change per commit, merged to main by PR (`main` is protected, see below).
|
||||
- Run `task check` locally before every push. CI is the quality gate, not branch protection.
|
||||
- No feature branches, no PRs for solo/agent work.
|
||||
- Short-lived branches and PRs are how work reaches `main` here (`tbd_ship`/`pr_merge`); no long-lived feature branches.
|
||||
- Exception: if a parallel agent session is active on this repo, use a short-lived
|
||||
`agent/<description>` branch and merge within the same session.
|
||||
|
||||
@@ -79,8 +79,8 @@ When acting as a coding agent on this project:
|
||||
3. If unsure about a convention, check `DECISIONS.md` or ask
|
||||
4. Never modify files outside the project root without explicit permission
|
||||
5. When adding a dependency, explain why in the commit message
|
||||
6. Commit directly to main. Run `task check` before every push. Never create
|
||||
feature branches unless a parallel agent is simultaneously active on this repo.
|
||||
6. Ship by PR (`main` is protected, see below). Run `task check` before every push. Never create
|
||||
long-lived feature branches.
|
||||
7. For client projects: never send code or context to cloud APIs — use local models via LiteLLM
|
||||
|
||||
## Current state — v0.2.5 (2026-05-17)
|
||||
@@ -105,3 +105,15 @@ Current main: `e31fd3f`. CI green. Deployed via Flux.
|
||||
|
||||
2. **Issue #19** — end-to-end mirror flow verification.
|
||||
`repo_mirror_push` is implemented but the full flow (create repo → add push mirror → verify sync to GitHub) has not been tested manually. Do this before relying on it in production.
|
||||
|
||||
## `main` is protected: ship by PR (local-dev#42, decided 2026-10-08)
|
||||
|
||||
Gitea refuses direct pushes to `main` in this repo, and a PR merges only after its CI status (`CD / Lint / Test / Vet (pull_request)`) passes.
|
||||
**Why:** local-dev rule 5's reviewed lane (risk/medium+ is reviewed by a human) was enforced on the merge tools
|
||||
(`tbd_ship`, `pr_merge`: gitea-mcp v0.17.0/v0.18.0, merge only for an issue labelled exactly `risk/low` with green CI)
|
||||
but not on a direct commit to `main`, which bypassed it. This overrides rule 5's "risk/low commits straight to main"
|
||||
for this repo; control code is where the lane matters.
|
||||
**How:** `tbd_ship` for a one-file change, or branch -> PR -> `pr_merge` (name the `risk/low` issue); above `risk/low`
|
||||
open the PR and a human merges it in the Gitea UI after review. Tags are not protected. CD jobs push image tags to
|
||||
the **infra** repo with deploy keys, never to this repo's `main`. Undo (Mathias only): Settings -> Branches, or
|
||||
`PATCH /repos/mathias/gitea-mcp/branch_protections/main {"enable_push": true}`.
|
||||
|
||||
+85
-21
@@ -164,6 +164,63 @@ These rules apply to every task across every project, regardless of harness.
|
||||
the thing it was named for was exploitable. If you cannot name the mechanism,
|
||||
you do not yet know what the test observes.
|
||||
|
||||
10. **Short sessions, one rc per repo, edit only in a locked worktree of the repo
|
||||
you are changing** (ADR-0002, accepted 2026-10-06).
|
||||
- One session per issue or task, ending with rule 6's close ritual. Related
|
||||
issues may share a session only when they share a worktree and an owner.
|
||||
State that must outlive the session goes in an issue, not the chat.
|
||||
- Start `claude rc` in the repo you will edit (`infra`, `trucker-ecommerce`,
|
||||
`hyperguild`, ...), not in `~/dev`: the rc worktree is a worktree of the
|
||||
launch directory's repo, so launched from `~/dev` it protects nothing you
|
||||
edit (measured: 73% of edits landed in shared checkouts). Start it only
|
||||
while working there.
|
||||
- Keep at most one thin `~/dev` coordinator session. It files issues and hands
|
||||
out tasks; it reads and edits no sub-repo code.
|
||||
- Split cross-repo work per repo. The contract between the halves lives in an
|
||||
issue on each repo.
|
||||
- Edit in a worktree of that repo, locked at creation:
|
||||
`scripts/wt.sh add <repo> <task>` does it (fetch, `wt/<task>` from origin/main, lock);
|
||||
`scripts/wt.sh done <repo>-<task>` removes it after the push and refuses dirty or
|
||||
unpushed work. By hand: `git worktree add ~/worktrees/<name> -b <branch> origin/main`
|
||||
then `git worktree lock --reason "manual: <purpose>" ~/worktrees/<name>`.
|
||||
- Before working a repo another session may touch, announce your issue list to
|
||||
that session (`SendMessage`) and ask for theirs.
|
||||
- Auto-memory is keyed by cwd, so a repo rc starts without it. Cross-repo
|
||||
working rules live in brain (`wiki/homelab/decisions/2026-10-06-062416-auto.md`, wing homelab, hall
|
||||
decisions) and the section below, not in per-directory memory.
|
||||
|
||||
## Standing footguns
|
||||
|
||||
Each cost real time once. Detail and evidence: brain `wiki/homelab/decisions/2026-10-06-062416-auto.md` (title standing-working-rules).
|
||||
|
||||
- **Gate then commit with `&&`, never `;`.** "Green" means exit code 0 and
|
||||
`grep -c FAIL` is 0, not a filtered grep.
|
||||
- **Stage by path, never `git add -A` or `git add .`** (they stage from the repo
|
||||
root and swept in other sessions' work twice). Check `git status --porcelain` first.
|
||||
- **Before committing in a shared checkout:** `git branch --show-current`.
|
||||
- **Before tagging:** `git fetch origin --tags`, then `git describe --tags --abbrev=0 <sha>`
|
||||
and `git tag --sort=-v:refname | head`; go above both. Never take the last tag
|
||||
from a report.
|
||||
- **Run only what the project's gate runs.** No repo-wide formatter unless the gate
|
||||
runs it.
|
||||
- **Reproduce a CI red at the cited commit before applying an issue's prescribed fix.**
|
||||
- **Read an issue's comments (`issue_list_comments`), not just its body, before working it.**
|
||||
- **A comment naming a control ("behind branch protection") is not evidence; query
|
||||
the control.** A working detector can hide a missing preventer.
|
||||
- **On koala the MCP servers are `mcp__plugin_skills_{brain,gitea,infra}__*`.** The
|
||||
per-turn auth reminder is not evidence; call the tool or run `claude mcp list`.
|
||||
Brain writes always pass `wing` + `hall`.
|
||||
- **`cat /etc/hostname` before assuming which host you are on.**
|
||||
- **Never run bare `git remote -v` or `git config --list`** (url.insteadOf expands
|
||||
into a live token); use `git config --get remote.origin.url`.
|
||||
- **On koala `sudo` is denied outright; k8s exec, env dumps and flux reconcile need
|
||||
per-action approval; decrypting a SOPS file and printing in one Bash call is
|
||||
hook-blocked.** Do the repo-side work and hand the host-side step over in an issue.
|
||||
- **`set -o pipefail` plus an optional `grep | head | awk` dies on no match:** append `|| true`.
|
||||
- **Gitea `file_write_branch` takes plain text, not base64.**
|
||||
- **`OP_SERVICE_ACCOUNT_TOKEN` in the shell is read-only for `op`.** Operational
|
||||
secrets go in the AgentSecrets vault via `set -a; . ~/.op-agent-write.env; set +a`.
|
||||
|
||||
## CAD sprint patterns
|
||||
|
||||
Two CAD sprint archetypes (defs: brain `wiki/homelab/decisions/continuous-agentic-development-cad-concept-2026-06-16`):
|
||||
@@ -244,25 +301,20 @@ Three machines on Tailscale:
|
||||
|
||||
All development repos live at `~/dev/` (softlink from `~/Documents/local-dev/`).
|
||||
|
||||
Organized in thematic folders:
|
||||
Organized by **role**, because the role decides which rules apply (ADR-0003):
|
||||
|
||||
| Folder | Focus | Count |
|
||||
|--------|-------|-------|
|
||||
| `GO/` | Go web frameworks, API integrations, learning projects | ~10 |
|
||||
| `AI/` | ML research, AI frameworks (FinRL, DSPy, crawl4ai) | ~6 |
|
||||
| `AGENTS/` | Autonomous agents, coding agents, MCP servers, infra | ~15 |
|
||||
| `QKX/` | Invoice processing, financial automation, payment systems | ~13 |
|
||||
| `XT/` | Climate data, sustainability (Klimatkollen, Garbo) | ~2 |
|
||||
| Folder | Role | Rules that key off it |
|
||||
|--------|------|-----------------------|
|
||||
| `platform/` | infra, brain, hyperguild, gitea-mcp, dispatch, shared tooling | control code (rule 8), GitOps review lane |
|
||||
| `products/` | tapir, cobalt-dingo, openbanking-api, ... | ordinary trunk-based work |
|
||||
| `clients/` | client work | local models only, never cloud APIs (Client work rules) |
|
||||
| `spikes/` | throwaway | no ceremony |
|
||||
|
||||
See `~/dev/PROJECT_SUMMARY.md` for detailed descriptions of each project.
|
||||
|
||||
### Key active projects
|
||||
|
||||
- **super-koala** (`AGENTS/`) — multi-component agent stack with LangGraph, DSPy, MCP
|
||||
- **azure-tiger** (`QKX/`) — invoice extraction → ISO 20022 payment instructions
|
||||
- **gocrwl** (`AGENTS/`) — Go web crawler with containerized deployment
|
||||
- **koala-ai-stack** (`AGENTS/`) — local AI server infrastructure management
|
||||
- **klimatkollen** (`XT/`) — Swedish municipal climate data platform
|
||||
New projects are born into a role folder (`task new-project -- <name> <client|personal>
|
||||
<role-folder>`); a client tag requires `clients/` and `clients/` requires a client tag.
|
||||
Existing repos still sit in the old `AGENTS/` and `AI/` folders until they are moved one by
|
||||
one (ADR-0003 step 3). `~/dev/PROJECT_SUMMARY.md` (generated) lists every repo and where it
|
||||
is. How a session runs on koala (worktrees, rc per repo, the reaper, memory): `docs/working-on-koala.md`.
|
||||
|
||||
## Knowledge base — actively use it
|
||||
|
||||
@@ -466,9 +518,9 @@ Check a host with `task doctor` in the skills repo; it also runs automatically a
|
||||
|
||||
### Git
|
||||
- Conventional commits: `feat:`, `fix:`, `chore:`, `docs:`, `refactor:`
|
||||
- **Trunk-Based Development:** commit directly to main. One logical change per commit.
|
||||
- **Trunk-Based Development:** one logical change per commit, merged to main by PR (`main` is protected, see below).
|
||||
- Run `task check` locally before every push. CI is the quality gate, not branch protection.
|
||||
- No feature branches, no PRs for solo/agent work.
|
||||
- Short-lived branches and PRs are how work reaches `main` here (`tbd_ship`/`pr_merge`); no long-lived feature branches.
|
||||
- Exception: if a parallel agent session is active on this repo, use a short-lived
|
||||
`agent/<description>` branch and merge within the same session.
|
||||
|
||||
@@ -508,8 +560,8 @@ When acting as a coding agent on this project:
|
||||
3. If unsure about a convention, check `DECISIONS.md` or ask
|
||||
4. Never modify files outside the project root without explicit permission
|
||||
5. When adding a dependency, explain why in the commit message
|
||||
6. Commit directly to main. Run `task check` before every push. Never create
|
||||
feature branches unless a parallel agent is simultaneously active on this repo.
|
||||
6. Ship by PR (`main` is protected, see below). Run `task check` before every push. Never create
|
||||
long-lived feature branches.
|
||||
7. For client projects: never send code or context to cloud APIs — use local models via LiteLLM
|
||||
|
||||
## Current state — v0.2.5 (2026-05-17)
|
||||
@@ -535,4 +587,16 @@ Current main: `e31fd3f`. CI green. Deployed via Flux.
|
||||
2. **Issue #19** — end-to-end mirror flow verification.
|
||||
`repo_mirror_push` is implemented but the full flow (create repo → add push mirror → verify sync to GitHub) has not been tested manually. Do this before relying on it in production.
|
||||
|
||||
## `main` is protected: ship by PR (local-dev#42, decided 2026-10-08)
|
||||
|
||||
Gitea refuses direct pushes to `main` in this repo, and a PR merges only after its CI status (`CD / Lint / Test / Vet (pull_request)`) passes.
|
||||
**Why:** local-dev rule 5's reviewed lane (risk/medium+ is reviewed by a human) was enforced on the merge tools
|
||||
(`tbd_ship`, `pr_merge`: gitea-mcp v0.17.0/v0.18.0, merge only for an issue labelled exactly `risk/low` with green CI)
|
||||
but not on a direct commit to `main`, which bypassed it. This overrides rule 5's "risk/low commits straight to main"
|
||||
for this repo; control code is where the lane matters.
|
||||
**How:** `tbd_ship` for a one-file change, or branch -> PR -> `pr_merge` (name the `risk/low` issue); above `risk/low`
|
||||
open the PR and a human merges it in the Gitea UI after review. Tags are not protected. CD jobs push image tags to
|
||||
the **infra** repo with deploy keys, never to this repo's `main`. Undo (Mathias only): Settings -> Branches, or
|
||||
`PATCH /repos/mathias/gitea-mcp/branch_protections/main {"enable_push": true}`.
|
||||
|
||||
---
|
||||
|
||||
+10
-1
@@ -8,6 +8,12 @@ name: CD
|
||||
# (Only applies to branch pushes; tag pushes always trigger.)
|
||||
paths-ignore:
|
||||
- 'docs/**'
|
||||
# gitea-mcp#68: without this a PR never got a CI run, so the risk/medium+
|
||||
# PR lane merged on local `task check` alone. Only the check job runs for a
|
||||
# PR -- build and deploy stay push/tag-only (no images, no infra writes,
|
||||
# no secrets from a PR).
|
||||
pull_request:
|
||||
branches: [main]
|
||||
|
||||
env:
|
||||
IMAGE: gitea-mcp
|
||||
@@ -44,6 +50,7 @@ jobs:
|
||||
build:
|
||||
name: Build & Import
|
||||
needs: check
|
||||
if: github.event_name == 'push'
|
||||
runs-on: self-hosted
|
||||
outputs:
|
||||
image-tag: ${{ steps.meta.outputs.sha-tag }}
|
||||
@@ -52,10 +59,12 @@ jobs:
|
||||
|
||||
- name: Derive image tags
|
||||
id: meta
|
||||
env:
|
||||
# signals#47: a ref name can carry $(...); pass it via env, not ${{ }}.
|
||||
REF: ${{ github.ref }}
|
||||
run: |
|
||||
SHA=$(git rev-parse --short HEAD)
|
||||
echo "sha-tag=${SHA}" >> "$GITHUB_OUTPUT"
|
||||
REF="${{ github.ref }}"
|
||||
if [[ "$REF" == refs/tags/v* ]]; then
|
||||
echo "version-tag=${REF#refs/tags/}" >> "$GITHUB_OUTPUT"
|
||||
echo "version=${REF#refs/tags/}" >> "$GITHUB_OUTPUT"
|
||||
|
||||
@@ -159,6 +159,63 @@ These rules apply to every task across every project, regardless of harness.
|
||||
the thing it was named for was exploitable. If you cannot name the mechanism,
|
||||
you do not yet know what the test observes.
|
||||
|
||||
10. **Short sessions, one rc per repo, edit only in a locked worktree of the repo
|
||||
you are changing** (ADR-0002, accepted 2026-10-06).
|
||||
- One session per issue or task, ending with rule 6's close ritual. Related
|
||||
issues may share a session only when they share a worktree and an owner.
|
||||
State that must outlive the session goes in an issue, not the chat.
|
||||
- Start `claude rc` in the repo you will edit (`infra`, `trucker-ecommerce`,
|
||||
`hyperguild`, ...), not in `~/dev`: the rc worktree is a worktree of the
|
||||
launch directory's repo, so launched from `~/dev` it protects nothing you
|
||||
edit (measured: 73% of edits landed in shared checkouts). Start it only
|
||||
while working there.
|
||||
- Keep at most one thin `~/dev` coordinator session. It files issues and hands
|
||||
out tasks; it reads and edits no sub-repo code.
|
||||
- Split cross-repo work per repo. The contract between the halves lives in an
|
||||
issue on each repo.
|
||||
- Edit in a worktree of that repo, locked at creation:
|
||||
`scripts/wt.sh add <repo> <task>` does it (fetch, `wt/<task>` from origin/main, lock);
|
||||
`scripts/wt.sh done <repo>-<task>` removes it after the push and refuses dirty or
|
||||
unpushed work. By hand: `git worktree add ~/worktrees/<name> -b <branch> origin/main`
|
||||
then `git worktree lock --reason "manual: <purpose>" ~/worktrees/<name>`.
|
||||
- Before working a repo another session may touch, announce your issue list to
|
||||
that session (`SendMessage`) and ask for theirs.
|
||||
- Auto-memory is keyed by cwd, so a repo rc starts without it. Cross-repo
|
||||
working rules live in brain (`wiki/homelab/decisions/2026-10-06-062416-auto.md`, wing homelab, hall
|
||||
decisions) and the section below, not in per-directory memory.
|
||||
|
||||
## Standing footguns
|
||||
|
||||
Each cost real time once. Detail and evidence: brain `wiki/homelab/decisions/2026-10-06-062416-auto.md` (title standing-working-rules).
|
||||
|
||||
- **Gate then commit with `&&`, never `;`.** "Green" means exit code 0 and
|
||||
`grep -c FAIL` is 0, not a filtered grep.
|
||||
- **Stage by path, never `git add -A` or `git add .`** (they stage from the repo
|
||||
root and swept in other sessions' work twice). Check `git status --porcelain` first.
|
||||
- **Before committing in a shared checkout:** `git branch --show-current`.
|
||||
- **Before tagging:** `git fetch origin --tags`, then `git describe --tags --abbrev=0 <sha>`
|
||||
and `git tag --sort=-v:refname | head`; go above both. Never take the last tag
|
||||
from a report.
|
||||
- **Run only what the project's gate runs.** No repo-wide formatter unless the gate
|
||||
runs it.
|
||||
- **Reproduce a CI red at the cited commit before applying an issue's prescribed fix.**
|
||||
- **Read an issue's comments (`issue_list_comments`), not just its body, before working it.**
|
||||
- **A comment naming a control ("behind branch protection") is not evidence; query
|
||||
the control.** A working detector can hide a missing preventer.
|
||||
- **On koala the MCP servers are `mcp__plugin_skills_{brain,gitea,infra}__*`.** The
|
||||
per-turn auth reminder is not evidence; call the tool or run `claude mcp list`.
|
||||
Brain writes always pass `wing` + `hall`.
|
||||
- **`cat /etc/hostname` before assuming which host you are on.**
|
||||
- **Never run bare `git remote -v` or `git config --list`** (url.insteadOf expands
|
||||
into a live token); use `git config --get remote.origin.url`.
|
||||
- **On koala `sudo` is denied outright; k8s exec, env dumps and flux reconcile need
|
||||
per-action approval; decrypting a SOPS file and printing in one Bash call is
|
||||
hook-blocked.** Do the repo-side work and hand the host-side step over in an issue.
|
||||
- **`set -o pipefail` plus an optional `grep | head | awk` dies on no match:** append `|| true`.
|
||||
- **Gitea `file_write_branch` takes plain text, not base64.**
|
||||
- **`OP_SERVICE_ACCOUNT_TOKEN` in the shell is read-only for `op`.** Operational
|
||||
secrets go in the AgentSecrets vault via `set -a; . ~/.op-agent-write.env; set +a`.
|
||||
|
||||
## CAD sprint patterns
|
||||
|
||||
Two CAD sprint archetypes (defs: brain `wiki/homelab/decisions/continuous-agentic-development-cad-concept-2026-06-16`):
|
||||
@@ -239,25 +296,20 @@ Three machines on Tailscale:
|
||||
|
||||
All development repos live at `~/dev/` (softlink from `~/Documents/local-dev/`).
|
||||
|
||||
Organized in thematic folders:
|
||||
Organized by **role**, because the role decides which rules apply (ADR-0003):
|
||||
|
||||
| Folder | Focus | Count |
|
||||
|--------|-------|-------|
|
||||
| `GO/` | Go web frameworks, API integrations, learning projects | ~10 |
|
||||
| `AI/` | ML research, AI frameworks (FinRL, DSPy, crawl4ai) | ~6 |
|
||||
| `AGENTS/` | Autonomous agents, coding agents, MCP servers, infra | ~15 |
|
||||
| `QKX/` | Invoice processing, financial automation, payment systems | ~13 |
|
||||
| `XT/` | Climate data, sustainability (Klimatkollen, Garbo) | ~2 |
|
||||
| Folder | Role | Rules that key off it |
|
||||
|--------|------|-----------------------|
|
||||
| `platform/` | infra, brain, hyperguild, gitea-mcp, dispatch, shared tooling | control code (rule 8), GitOps review lane |
|
||||
| `products/` | tapir, cobalt-dingo, openbanking-api, ... | ordinary trunk-based work |
|
||||
| `clients/` | client work | local models only, never cloud APIs (Client work rules) |
|
||||
| `spikes/` | throwaway | no ceremony |
|
||||
|
||||
See `~/dev/PROJECT_SUMMARY.md` for detailed descriptions of each project.
|
||||
|
||||
### Key active projects
|
||||
|
||||
- **super-koala** (`AGENTS/`) — multi-component agent stack with LangGraph, DSPy, MCP
|
||||
- **azure-tiger** (`QKX/`) — invoice extraction → ISO 20022 payment instructions
|
||||
- **gocrwl** (`AGENTS/`) — Go web crawler with containerized deployment
|
||||
- **koala-ai-stack** (`AGENTS/`) — local AI server infrastructure management
|
||||
- **klimatkollen** (`XT/`) — Swedish municipal climate data platform
|
||||
New projects are born into a role folder (`task new-project -- <name> <client|personal>
|
||||
<role-folder>`); a client tag requires `clients/` and `clients/` requires a client tag.
|
||||
Existing repos still sit in the old `AGENTS/` and `AI/` folders until they are moved one by
|
||||
one (ADR-0003 step 3). `~/dev/PROJECT_SUMMARY.md` (generated) lists every repo and where it
|
||||
is. How a session runs on koala (worktrees, rc per repo, the reaper, memory): `docs/working-on-koala.md`.
|
||||
|
||||
## Knowledge base — actively use it
|
||||
|
||||
@@ -461,9 +513,9 @@ Check a host with `task doctor` in the skills repo; it also runs automatically a
|
||||
|
||||
### Git
|
||||
- Conventional commits: `feat:`, `fix:`, `chore:`, `docs:`, `refactor:`
|
||||
- **Trunk-Based Development:** commit directly to main. One logical change per commit.
|
||||
- **Trunk-Based Development:** one logical change per commit, merged to main by PR (`main` is protected, see below).
|
||||
- Run `task check` locally before every push. CI is the quality gate, not branch protection.
|
||||
- No feature branches, no PRs for solo/agent work.
|
||||
- Short-lived branches and PRs are how work reaches `main` here (`tbd_ship`/`pr_merge`); no long-lived feature branches.
|
||||
- Exception: if a parallel agent session is active on this repo, use a short-lived
|
||||
`agent/<description>` branch and merge within the same session.
|
||||
|
||||
@@ -503,8 +555,8 @@ When acting as a coding agent on this project:
|
||||
3. If unsure about a convention, check `DECISIONS.md` or ask
|
||||
4. Never modify files outside the project root without explicit permission
|
||||
5. When adding a dependency, explain why in the commit message
|
||||
6. Commit directly to main. Run `task check` before every push. Never create
|
||||
feature branches unless a parallel agent is simultaneously active on this repo.
|
||||
6. Ship by PR (`main` is protected, see below). Run `task check` before every push. Never create
|
||||
long-lived feature branches.
|
||||
7. For client projects: never send code or context to cloud APIs — use local models via LiteLLM
|
||||
|
||||
## Current state — v0.2.5 (2026-05-17)
|
||||
@@ -529,3 +581,15 @@ Current main: `e31fd3f`. CI green. Deployed via Flux.
|
||||
|
||||
2. **Issue #19** — end-to-end mirror flow verification.
|
||||
`repo_mirror_push` is implemented but the full flow (create repo → add push mirror → verify sync to GitHub) has not been tested manually. Do this before relying on it in production.
|
||||
|
||||
## `main` is protected: ship by PR (local-dev#42, decided 2026-10-08)
|
||||
|
||||
Gitea refuses direct pushes to `main` in this repo, and a PR merges only after its CI status (`CD / Lint / Test / Vet (pull_request)`) passes.
|
||||
**Why:** local-dev rule 5's reviewed lane (risk/medium+ is reviewed by a human) was enforced on the merge tools
|
||||
(`tbd_ship`, `pr_merge`: gitea-mcp v0.17.0/v0.18.0, merge only for an issue labelled exactly `risk/low` with green CI)
|
||||
but not on a direct commit to `main`, which bypassed it. This overrides rule 5's "risk/low commits straight to main"
|
||||
for this repo; control code is where the lane matters.
|
||||
**How:** `tbd_ship` for a one-file change, or branch -> PR -> `pr_merge` (name the `risk/low` issue); above `risk/low`
|
||||
open the PR and a human merges it in the Gitea UI after review. Tags are not protected. CD jobs push image tags to
|
||||
the **infra** repo with deploy keys, never to this repo's `main`. Undo (Mathias only): Settings -> Branches, or
|
||||
`PATCH /repos/mathias/gitea-mcp/branch_protections/main {"enable_push": true}`.
|
||||
|
||||
@@ -37,9 +37,9 @@
|
||||
|
||||
### Git
|
||||
- Conventional commits: `feat:`, `fix:`, `chore:`, `docs:`, `refactor:`
|
||||
- **Trunk-Based Development:** commit directly to main. One logical change per commit.
|
||||
- **Trunk-Based Development:** one logical change per commit, merged to main by PR (`main` is protected, see below).
|
||||
- Run `task check` locally before every push. CI is the quality gate, not branch protection.
|
||||
- No feature branches, no PRs for solo/agent work.
|
||||
- Short-lived branches and PRs are how work reaches `main` here (`tbd_ship`/`pr_merge`); no long-lived feature branches.
|
||||
- Exception: if a parallel agent session is active on this repo, use a short-lived
|
||||
`agent/<description>` branch and merge within the same session.
|
||||
|
||||
@@ -79,8 +79,8 @@ When acting as a coding agent on this project:
|
||||
3. If unsure about a convention, check `DECISIONS.md` or ask
|
||||
4. Never modify files outside the project root without explicit permission
|
||||
5. When adding a dependency, explain why in the commit message
|
||||
6. Commit directly to main. Run `task check` before every push. Never create
|
||||
feature branches unless a parallel agent is simultaneously active on this repo.
|
||||
6. Ship by PR (`main` is protected, see below). Run `task check` before every push. Never create
|
||||
long-lived feature branches.
|
||||
7. For client projects: never send code or context to cloud APIs — use local models via LiteLLM
|
||||
|
||||
## Current state — v0.2.5 (2026-05-17)
|
||||
@@ -105,3 +105,15 @@ Current main: `e31fd3f`. CI green. Deployed via Flux.
|
||||
|
||||
2. **Issue #19** — end-to-end mirror flow verification.
|
||||
`repo_mirror_push` is implemented but the full flow (create repo → add push mirror → verify sync to GitHub) has not been tested manually. Do this before relying on it in production.
|
||||
|
||||
## `main` is protected: ship by PR (local-dev#42, decided 2026-10-08)
|
||||
|
||||
Gitea refuses direct pushes to `main` in this repo, and a PR merges only after its CI status (`CD / Lint / Test / Vet (pull_request)`) passes.
|
||||
**Why:** local-dev rule 5's reviewed lane (risk/medium+ is reviewed by a human) was enforced on the merge tools
|
||||
(`tbd_ship`, `pr_merge`: gitea-mcp v0.17.0/v0.18.0, merge only for an issue labelled exactly `risk/low` with green CI)
|
||||
but not on a direct commit to `main`, which bypassed it. This overrides rule 5's "risk/low commits straight to main"
|
||||
for this repo; control code is where the lane matters.
|
||||
**How:** `tbd_ship` for a one-file change, or branch -> PR -> `pr_merge` (name the `risk/low` issue); above `risk/low`
|
||||
open the PR and a human merges it in the Gitea UI after review. Tags are not protected. CD jobs push image tags to
|
||||
the **infra** repo with deploy keys, never to this repo's `main`. Undo (Mathias only): Settings -> Branches, or
|
||||
`PATCH /repos/mathias/gitea-mcp/branch_protections/main {"enable_push": true}`.
|
||||
|
||||
@@ -125,6 +125,22 @@ by area:
|
||||
| Search / Templates | code_search, create_project_from_template |
|
||||
| Composite | tbd_ship (trunk-based ship helper) |
|
||||
|
||||
**Merge gate: `tbd_ship` (gitea-mcp#63) and `pr_merge` (gitea-mcp#74).** The two merge tools share one gate
|
||||
(`evaluateShipGate`). A PR is merged only when ALL of: the head commit's CI is fully green, the base branch is not
|
||||
review-protected, and the issue named in `issue` carries **exactly one** `risk/` label, `risk/low`. No `issue`, no
|
||||
`risk/` label, several, an unknown value (`risk/lowish`), `risk/medium`, `risk/high`, or an issue that cannot be read
|
||||
all fail closed: nothing is merged, the result is `merged:false` with a `reason`, the `ci_status` and the `risk` tier.
|
||||
Unclassified is not low. Above `risk/low` a human reviews and merges in the Gitea UI (which this gate does not touch).
|
||||
`pr_merge` additionally fails with an error, merging nothing, when the PR or the base's branch-protection state cannot
|
||||
be read. A merge conflict is still an error.
|
||||
Both tools bind the merge to the head commit whose CI the gate evaluated (`head_commit_id`), so a push to the PR
|
||||
branch after the check makes Gitea refuse the merge instead of merging unchecked code.
|
||||
|
||||
Limits: the gate enforces the tier of the issue the caller names; it cannot prove that issue is the one the change
|
||||
belongs to, so the link is the caller's claim (local-dev rule 5's convention still applies). Not gated: merging in the
|
||||
Gitea UI, and `file_write_branch` committing directly to a branch (`main` included), which is how `risk/low` work
|
||||
reaches trunk. Platform-level enforcement for those is branch protection on `main` (local-dev#27 option 2).
|
||||
|
||||
Shared tool-layer hygiene (`internal/tools/tool.go`):
|
||||
- **Alias normalization** — `repo`↔`name` and `number`↔`index` are reconciled so
|
||||
a tool works whichever spelling a caller sends (an explicit canonical wins).
|
||||
|
||||
@@ -107,8 +107,10 @@ func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, ind
|
||||
|
||||
type MergePRArgs struct {
|
||||
Do string `json:"Do"`
|
||||
Title string `json:"merge_message_title,omitempty"`
|
||||
Title string `json:"merge_title_field,omitempty"`
|
||||
Body string `json:"merge_message_field,omitempty"`
|
||||
// HeadCommitID binds the merge to the commit whose CI was checked: if the PR branch moved, Gitea refuses.
|
||||
HeadCommitID string `json:"head_commit_id,omitempty"`
|
||||
}
|
||||
|
||||
func (c *Client) MergePullRequest(ctx context.Context, owner, repo string, index int, args MergePRArgs) error {
|
||||
|
||||
+54
-13
@@ -10,6 +10,9 @@ import (
|
||||
"git.d-ma.be/mathias/gitea-mcp/internal/registry"
|
||||
)
|
||||
|
||||
// PRMerge merges an existing PR behind the same gate as tbd_ship (gitea-mcp#74, #63): CI fully green, base not
|
||||
// review-protected, and the issue named in `issue` classified exactly risk/low. Above risk/low a human reviews and
|
||||
// merges in the Gitea UI; this tool then merges nothing and says why.
|
||||
type PRMerge struct {
|
||||
c *gitea.Client
|
||||
a *allowlist.Allowlist
|
||||
@@ -22,16 +25,18 @@ func NewPRMerge(c *gitea.Client, a *allowlist.Allowlist) *PRMerge {
|
||||
func (t *PRMerge) Descriptor() registry.ToolDescriptor {
|
||||
return registry.ToolDescriptor{
|
||||
Name: "pr_merge",
|
||||
Description: "Merge a pull request. style: merge (default), squash, or rebase.",
|
||||
Description: "Merge a pull request, gated like tbd_ship: it merges ONLY when the head commit's CI is fully green AND the base branch is not review-protected AND the issue named in `issue` carries exactly one risk/ label, risk/low. Otherwise it merges nothing and returns merged:false with a reason and the risk tier (no issue named, no or several risk/ labels, an unknown value, risk/medium, risk/high, an unreadable issue, CI pending/red/absent, a review-protected base). Above risk/low a human reviews and merges in the Gitea UI. style: merge (default), squash, or rebase. Set ci_timeout_seconds to poll CI to completion (default 0 = single snapshot). Returns {merged, pr_number, ci_status, risk, reason}.",
|
||||
InputSchema: json.RawMessage(`{
|
||||
"type":"object",
|
||||
"properties":{
|
||||
"owner":{"type":"string"},
|
||||
"repo":{"type":"string"},
|
||||
"number":{"type":"integer","minimum":1},
|
||||
"issue":{"type":"integer","minimum":1,"description":"Issue number in the same repo whose risk/ label classifies this change. Merge happens only for exactly one label, risk/low; no issue, none, several, an unknown value, risk/medium or risk/high means nothing is merged (unclassified is not low)."},
|
||||
"style":{"type":"string","enum":["merge","squash","rebase"]},
|
||||
"merge_message_title":{"type":"string"},
|
||||
"merge_message_field":{"type":"string"}
|
||||
"merge_message_field":{"type":"string"},
|
||||
"ci_timeout_seconds":{"type":"integer","minimum":0,"maximum":600,"description":"Poll CI up to this long for the head commit's runs to complete. Default 0 (single snapshot)."}
|
||||
},
|
||||
"required":["owner","repo","number"]
|
||||
}`),
|
||||
@@ -39,12 +44,14 @@ func (t *PRMerge) Descriptor() registry.ToolDescriptor {
|
||||
}
|
||||
|
||||
type prMergeArgs struct {
|
||||
Owner string `json:"owner"`
|
||||
Repo string `json:"repo"`
|
||||
Number int `json:"number"`
|
||||
Style string `json:"style"`
|
||||
Title string `json:"merge_message_title"`
|
||||
Body string `json:"merge_message_field"`
|
||||
Owner string `json:"owner"`
|
||||
Repo string `json:"repo"`
|
||||
Number int `json:"number"`
|
||||
Issue int `json:"issue"`
|
||||
Style string `json:"style"`
|
||||
Title string `json:"merge_message_title"`
|
||||
Body string `json:"merge_message_field"`
|
||||
CITimeoutSec int `json:"ci_timeout_seconds"`
|
||||
}
|
||||
|
||||
func (t *PRMerge) Call(ctx context.Context, raw json.RawMessage) (json.RawMessage, error) {
|
||||
@@ -64,13 +71,47 @@ func (t *PRMerge) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
|
||||
style = "merge"
|
||||
}
|
||||
|
||||
// The gate's inputs. An unreadable PR or branch-protection probe is an error: the gate cannot be evaluated, so
|
||||
// nothing is merged. An unreadable issue is a tier (unreadable), not an error: it is reported and not merged.
|
||||
pr, err := t.c.GetPullRequest(ctx, args.Owner, args.Repo, args.Number)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("read PR #%d: %w", args.Number, err)
|
||||
}
|
||||
// Gitea refuses closed and merged PRs itself; the gate does not depend on that.
|
||||
if pr.State != "open" || pr.Draft {
|
||||
reason := fmt.Sprintf("the PR is not open (state %q) — nothing to merge", pr.State)
|
||||
if pr.State == "open" {
|
||||
reason = "the PR is a draft — not merged"
|
||||
}
|
||||
return textOK(map[string]any{"merged": false, "pr_number": args.Number, "reason": reason})
|
||||
}
|
||||
bp, err := t.c.GetBranchProtection(ctx, args.Owner, args.Repo, pr.Base.Ref)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("branch protection probe: %w", err)
|
||||
}
|
||||
runs := pollRuns(ctx, t.c, args.Owner, args.Repo, pr.Head.Sha, args.CITimeoutSec)
|
||||
risk := riskTierOf(ctx, t.c, args.Owner, args.Repo, args.Issue)
|
||||
gate := evaluateMergeGate(pr.Head.Sha, runs, bp, risk)
|
||||
|
||||
result := map[string]any{
|
||||
"merged": false,
|
||||
"pr_number": args.Number,
|
||||
"ci_status": gate.ciStatus,
|
||||
"risk": string(risk),
|
||||
}
|
||||
if !gate.merge {
|
||||
result["reason"] = gate.reason
|
||||
return textOK(result)
|
||||
}
|
||||
|
||||
if err := t.c.MergePullRequest(ctx, args.Owner, args.Repo, args.Number, gitea.MergePRArgs{
|
||||
Do: style,
|
||||
Title: args.Title,
|
||||
Body: args.Body,
|
||||
Do: style,
|
||||
Title: args.Title,
|
||||
Body: args.Body,
|
||||
HeadCommitID: pr.Head.Sha,
|
||||
}); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return textOK(map[string]any{"merged": true})
|
||||
result["merged"] = true
|
||||
return textOK(result)
|
||||
}
|
||||
|
||||
+363
-45
@@ -6,6 +6,8 @@ import (
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"git.d-ma.be/mathias/gitea-mcp/internal/allowlist"
|
||||
@@ -15,80 +17,396 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// pr_merge is gated exactly like tbd_ship (gitea-mcp#74, #63): it merges only when the head commit's CI is fully
|
||||
// green, the base is not review-protected AND the issue named in `issue` carries exactly one risk/ label, risk/low.
|
||||
// The fake serves the whole read side the gate needs and records whether (and how) the merge endpoint was hit.
|
||||
|
||||
const (
|
||||
greenRuns = `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`
|
||||
lowLabels = `[{"id":1,"name":"risk/low"}]`
|
||||
)
|
||||
|
||||
type prMergeFakeOpts struct {
|
||||
runs string // workflow_runs array for the head commit
|
||||
issueLabels string // labels array of issue 42
|
||||
issueStatus int
|
||||
prStatus int // GET pulls/7
|
||||
protectionStatus int // 0 → 404 (unprotected); 200 with required approvals when requiredApprovals>0
|
||||
requiredApprovals int
|
||||
mergeStatus int
|
||||
emptyHeadSha bool // the PR reports head.sha == ""
|
||||
prState string // default "open"
|
||||
prDraft bool
|
||||
}
|
||||
|
||||
type prMergeFake struct {
|
||||
*httptest.Server
|
||||
mu sync.Mutex
|
||||
mergeCalls int
|
||||
mergeBody []byte
|
||||
runsQuery string // raw query of the last workflow-runs request
|
||||
}
|
||||
|
||||
func (f *prMergeFake) merged() bool { f.mu.Lock(); defer f.mu.Unlock(); return f.mergeCalls > 0 }
|
||||
|
||||
func newPRMergeFake(t *testing.T, o prMergeFakeOpts) *prMergeFake {
|
||||
t.Helper()
|
||||
if o.runs == "" {
|
||||
o.runs = greenRuns
|
||||
}
|
||||
if o.issueLabels == "" {
|
||||
o.issueLabels = lowLabels
|
||||
}
|
||||
if o.issueStatus == 0 {
|
||||
o.issueStatus = http.StatusOK
|
||||
}
|
||||
if o.prStatus == 0 {
|
||||
o.prStatus = http.StatusOK
|
||||
}
|
||||
if o.mergeStatus == 0 {
|
||||
o.mergeStatus = http.StatusNoContent
|
||||
}
|
||||
if o.prState == "" {
|
||||
o.prState = "open"
|
||||
}
|
||||
sha := "abc"
|
||||
if o.emptyHeadSha {
|
||||
sha = ""
|
||||
}
|
||||
f := &prMergeFake{}
|
||||
f.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
p := r.URL.Path
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
switch {
|
||||
case r.Method == http.MethodGet && p == "/api/v1/repos/owner/repo/pulls/7":
|
||||
w.WriteHeader(o.prStatus)
|
||||
_, _ = w.Write([]byte(`{"number":7,"title":"t","state":"` + o.prState + `","draft":` + boolJSON(o.prDraft) + `,"html_url":"http://x/pulls/7","head":{"ref":"feat","sha":"` + sha + `"},"base":{"ref":"main"}}`))
|
||||
case r.Method == http.MethodGet && strings.Contains(p, "/branch_protections/"):
|
||||
switch {
|
||||
case o.protectionStatus != 0:
|
||||
w.WriteHeader(o.protectionStatus)
|
||||
_, _ = w.Write([]byte(`{"message":"boom"}`))
|
||||
case o.requiredApprovals > 0:
|
||||
_, _ = w.Write([]byte(`{"required_approvals":` + itoa(o.requiredApprovals) + `}`))
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
_, _ = w.Write([]byte(`{"message":"not found"}`))
|
||||
}
|
||||
case r.Method == http.MethodGet && strings.Contains(p, "/actions/runs"):
|
||||
f.mu.Lock()
|
||||
f.runsQuery = r.URL.RawQuery
|
||||
f.mu.Unlock()
|
||||
_, _ = w.Write([]byte(`{"total_count":0,"workflow_runs":` + o.runs + `}`))
|
||||
case r.Method == http.MethodGet && p == "/api/v1/repos/owner/repo/issues/42":
|
||||
w.WriteHeader(o.issueStatus)
|
||||
_, _ = w.Write([]byte(`{"number":42,"title":"t","state":"open","labels":` + o.issueLabels + `}`))
|
||||
case r.Method == http.MethodPost && p == "/api/v1/repos/owner/repo/pulls/7/merge":
|
||||
body, _ := io.ReadAll(r.Body)
|
||||
f.mu.Lock()
|
||||
f.mergeCalls++
|
||||
f.mergeBody = body
|
||||
f.mu.Unlock()
|
||||
w.WriteHeader(o.mergeStatus)
|
||||
_, _ = w.Write([]byte(`{"message":"merge conflict"}`))
|
||||
default:
|
||||
t.Errorf("unexpected request: %s %s", r.Method, p)
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
t.Cleanup(f.Close)
|
||||
return f
|
||||
}
|
||||
|
||||
func boolJSON(b bool) string {
|
||||
if b {
|
||||
return "true"
|
||||
}
|
||||
return "false"
|
||||
}
|
||||
|
||||
func callMerge(t *testing.T, f *prMergeFake, args string) (map[string]any, error) {
|
||||
t.Helper()
|
||||
tool := tools.NewPRMerge(gitea.NewClient(f.URL, "tok"), allowlist.New([]string{"owner"}))
|
||||
out, err := tool.Call(context.Background(), json.RawMessage(args))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
var res map[string]any
|
||||
require.NoError(t, json.Unmarshal(out, &res))
|
||||
return res, nil
|
||||
}
|
||||
|
||||
const mergeArgs = `{"owner":"owner","repo":"repo","number":7,"issue":42}`
|
||||
|
||||
func TestPRMergeSuccess(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Equal(t, "/api/v1/repos/owner/repo/pulls/7/merge", r.URL.Path)
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
defer srv.Close()
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{})
|
||||
|
||||
res, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
tool := tools.NewPRMerge(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"owner"}))
|
||||
out, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"owner","name":"repo","index":7}`))
|
||||
require.NoError(t, err)
|
||||
|
||||
var result map[string]any
|
||||
require.NoError(t, json.Unmarshal(out, &result))
|
||||
assert.Equal(t, true, result["merged"])
|
||||
assert.Equal(t, true, res["merged"])
|
||||
assert.Equal(t, "success", res["ci_status"])
|
||||
assert.Equal(t, "low", res["risk"])
|
||||
assert.Equal(t, float64(7), res["pr_number"])
|
||||
assert.True(t, f.merged())
|
||||
}
|
||||
|
||||
func TestPRMergeDefaultsToMergeStyle(t *testing.T) {
|
||||
var captured []byte
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
var err error
|
||||
captured, err = io.ReadAll(r.Body)
|
||||
require.NoError(t, err)
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
tool := tools.NewPRMerge(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"owner"}))
|
||||
_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"owner","name":"repo","index":7}`))
|
||||
func TestPRMergeDefaultsToMergeStyleAndPassesStyleThrough(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{})
|
||||
_, err := callMerge(t, f, mergeArgs)
|
||||
require.NoError(t, err)
|
||||
|
||||
var payload map[string]any
|
||||
require.NoError(t, json.Unmarshal(captured, &payload))
|
||||
require.NoError(t, json.Unmarshal(f.mergeBody, &payload))
|
||||
assert.Equal(t, "merge", payload["Do"])
|
||||
|
||||
f2 := newPRMergeFake(t, prMergeFakeOpts{})
|
||||
_, err = callMerge(t, f2, `{"owner":"owner","repo":"repo","number":7,"issue":42,"style":"squash"}`)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, json.Unmarshal(f2.mergeBody, &payload))
|
||||
assert.Equal(t, "squash", payload["Do"])
|
||||
}
|
||||
|
||||
// Gitea 1.27's MergePullRequestOption has merge_title_field and merge_message_field. A field it does not know is
|
||||
// ignored without an error, so a custom merge title silently did nothing while this tool sent merge_message_title
|
||||
// (gitea-mcp#77). The tool's own argument keeps its name; the wire field must be Gitea's.
|
||||
func TestPRMerge_ACustomMergeTitleTravelsAsGiteasMergeTitleField(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{})
|
||||
|
||||
_, err := callMerge(t, f, `{"owner":"owner","repo":"repo","number":7,"issue":42,"merge_message_title":"feat: one line","merge_message_field":"body text"}`)
|
||||
|
||||
require.NoError(t, err)
|
||||
var payload map[string]any
|
||||
require.NoError(t, json.Unmarshal(f.mergeBody, &payload))
|
||||
assert.Equal(t, "feat: one line", payload["merge_title_field"])
|
||||
assert.Equal(t, "body text", payload["merge_message_field"])
|
||||
assert.NotContains(t, payload, "merge_message_title", "not a Gitea field: it is ignored without an error")
|
||||
}
|
||||
|
||||
// TOCTOU: the gate read CI for ONE head commit. The merge must be bound to that commit, so a push to the PR branch
|
||||
// after the check makes Gitea refuse the merge instead of merging code nobody checked.
|
||||
func TestPRMerge_TheMergeIsBoundToTheCommitTheGateChecked(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{})
|
||||
|
||||
_, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
require.NoError(t, err)
|
||||
var payload map[string]any
|
||||
require.NoError(t, json.Unmarshal(f.mergeBody, &payload))
|
||||
assert.Equal(t, "abc", payload["head_commit_id"], "the head sha the CI check used")
|
||||
}
|
||||
|
||||
// A merge conflict is still an error, as before the gate existed.
|
||||
func TestPRMergeConflictReturnsError(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusConflict)
|
||||
_, _ = w.Write([]byte(`{"message":"merge conflict"}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{mergeStatus: http.StatusConflict})
|
||||
|
||||
_, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
tool := tools.NewPRMerge(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"owner"}))
|
||||
_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"owner","name":"repo","index":7}`))
|
||||
require.Error(t, err)
|
||||
assert.ErrorIs(t, err, gitea.ErrConflict)
|
||||
assert.True(t, f.merged(), "the merge was attempted: the gate had passed")
|
||||
}
|
||||
|
||||
// #45: pr_merge advertises the canonical `number` (was `index`); `index` stays
|
||||
// an accepted alias via the shim.
|
||||
// #45: pr_merge advertises the canonical `number` (was `index`); `index` stays an accepted alias via the shim.
|
||||
func TestPRMergeNumberCanonical(t *testing.T) {
|
||||
sch := string(tools.NewPRMerge(gitea.NewClient("http://unused", ""), allowlist.New([]string{"owner"})).Descriptor().InputSchema)
|
||||
assert.Contains(t, sch, `"number":`, "pr_merge must advertise number")
|
||||
assert.NotContains(t, sch, `"index":`, "pr_merge must not advertise index")
|
||||
|
||||
for _, args := range []string{
|
||||
`{"owner":"owner","repo":"repo","number":7}`,
|
||||
`{"owner":"owner","repo":"repo","index":7}`,
|
||||
`{"owner":"owner","repo":"repo","number":7,"issue":42}`,
|
||||
`{"owner":"owner","repo":"repo","index":7,"issue":42}`,
|
||||
`{"owner":"owner","name":"repo","index":7,"issue":42}`,
|
||||
} {
|
||||
var gotPath string
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
gotPath = r.URL.Path
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
tool := tools.NewPRMerge(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"owner"}))
|
||||
_, err := tool.Call(context.Background(), json.RawMessage(args))
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{})
|
||||
res, err := callMerge(t, f, args)
|
||||
require.NoError(t, err, args)
|
||||
assert.Equal(t, "/api/v1/repos/owner/repo/pulls/7/merge", gotPath, args)
|
||||
srv.Close()
|
||||
assert.Equal(t, true, res["merged"], args)
|
||||
}
|
||||
}
|
||||
|
||||
// gitea-mcp#74: the descriptor tells the caller about the gate.
|
||||
func TestPRMergeDescriptorAdvertisesTheGate(t *testing.T) {
|
||||
d := tools.NewPRMerge(gitea.NewClient("http://unused", ""), allowlist.New([]string{"owner"})).Descriptor()
|
||||
assert.Contains(t, string(d.InputSchema), `"issue":`)
|
||||
assert.Contains(t, string(d.InputSchema), `"ci_timeout_seconds":`)
|
||||
assert.Contains(t, d.Description, "risk/low")
|
||||
assert.Contains(t, d.Description, "merged:false")
|
||||
}
|
||||
|
||||
func TestPRMergeAllowlistRejects(t *testing.T) {
|
||||
tool := tools.NewPRMerge(gitea.NewClient("http://unused", ""), allowlist.New([]string{"allowed"}))
|
||||
_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"evil","name":"repo","index":1}`))
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
// ---- the tier: only exactly one risk/low merges ----
|
||||
func TestPRMerge_RiskTier_OnlyLowMerges(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
labels string
|
||||
wantMerged bool
|
||||
wantInReason string
|
||||
}{
|
||||
{"risk/low merges", `[{"id":1,"name":"risk/low"}]`, true, ""},
|
||||
{"risk/low among other labels merges", `[{"id":2,"name":"type/fix"},{"id":1,"name":"risk/low"}]`, true, ""},
|
||||
{"risk/medium does not merge", `[{"id":1,"name":"risk/medium"}]`, false, "risk/medium"},
|
||||
{"risk/high does not merge", `[{"id":1,"name":"risk/high"}]`, false, "risk/high"},
|
||||
{"no risk label is unclassified, not low", `[{"id":2,"name":"type/fix"}]`, false, "no risk/"},
|
||||
{"no labels at all", `[]`, false, "no risk/"},
|
||||
{"two risk labels are ambiguous", `[{"id":1,"name":"risk/low"},{"id":2,"name":"risk/high"}]`, false, "more than one"},
|
||||
{"two identical risk/low labels are still ambiguous", `[{"id":1,"name":"risk/low"},{"id":2,"name":"risk/low"}]`, false, "more than one"},
|
||||
{"an unknown value does not merge", `[{"id":1,"name":"risk/lowish"}]`, false, "unknown"},
|
||||
{"the value is case-sensitive: risk/Low", `[{"id":1,"name":"risk/Low"}]`, false, "unknown"},
|
||||
{"trailing space in the value", `[{"id":1,"name":"risk/low "}]`, false, "unknown"},
|
||||
{"RISK/LOW is not a risk/ label", `[{"id":1,"name":"RISK/LOW"}]`, false, "no risk/"},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{issueLabels: tc.labels})
|
||||
|
||||
res, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
require.NoError(t, err, "a refusal is a result, not an error")
|
||||
assert.Equal(t, tc.wantMerged, res["merged"])
|
||||
assert.Equal(t, tc.wantMerged, f.merged(), "the merge endpoint is called iff the tier allows it")
|
||||
assert.Equal(t, "success", res["ci_status"], "CI was green: the tier alone decided")
|
||||
if !tc.wantMerged {
|
||||
assert.Contains(t, res["reason"], tc.wantInReason)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPRMerge_NoIssue_Unclassified_NoMerge(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{})
|
||||
|
||||
res, err := callMerge(t, f, `{"owner":"owner","repo":"repo","number":7}`)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.Equal(t, "none", res["risk"])
|
||||
assert.Contains(t, res["reason"], "issue")
|
||||
assert.False(t, f.merged(), "an unclassified change must not merge")
|
||||
}
|
||||
|
||||
func TestPRMerge_IssueLookupFails_FailsClosed(t *testing.T) {
|
||||
for _, status := range []int{http.StatusInternalServerError, http.StatusNotFound, http.StatusForbidden} {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{issueStatus: status})
|
||||
|
||||
res, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
require.NoError(t, err, "status %d", status)
|
||||
assert.Equal(t, false, res["merged"], "status %d", status)
|
||||
assert.Equal(t, "unreadable", res["risk"], "status %d", status)
|
||||
assert.False(t, f.merged(), "status %d: an unreadable issue must not merge", status)
|
||||
}
|
||||
}
|
||||
|
||||
// ---- risk/low is necessary, not sufficient: CI and branch protection still gate ----
|
||||
func TestPRMerge_RiskLow_DoesNotOverrideCI(t *testing.T) {
|
||||
for name, runs := range map[string]string{
|
||||
"red": `[{"id":1,"status":"completed","conclusion":"failure","head_sha":"abc"}]`,
|
||||
"cancelled": `[{"id":1,"status":"completed","conclusion":"cancelled","head_sha":"abc"}]`,
|
||||
"in_progress": `[{"id":1,"status":"in_progress","conclusion":"","head_sha":"abc"}]`,
|
||||
"no runs": `[]`,
|
||||
"mixed": `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"},{"id":2,"status":"queued","conclusion":"","head_sha":"abc"}]`,
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{runs: runs})
|
||||
|
||||
res, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.NotEmpty(t, res["reason"])
|
||||
assert.False(t, f.merged(), "risk/low must not override CI that is not fully green")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPRMerge_RiskLow_DoesNotOverrideAReviewProtectedBase(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{requiredApprovals: 1})
|
||||
|
||||
res, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.Contains(t, res["reason"], "review")
|
||||
assert.False(t, f.merged())
|
||||
}
|
||||
|
||||
// An unreadable PR or an unreadable branch-protection probe is an error, never a merge.
|
||||
func TestPRMerge_ReadSideFailures_AreErrorsAndNeverMerge(t *testing.T) {
|
||||
for name, o := range map[string]prMergeFakeOpts{
|
||||
"PR lookup 500": {prStatus: http.StatusInternalServerError},
|
||||
"PR lookup 404": {prStatus: http.StatusNotFound},
|
||||
"protection probe 500": {protectionStatus: http.StatusInternalServerError},
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
f := newPRMergeFake(t, o)
|
||||
|
||||
_, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
require.Error(t, err)
|
||||
assert.False(t, f.merged(), "must not merge when the gate cannot be evaluated")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The classification comes from the issue the caller names, not from `number`: a PR number that is also an issue
|
||||
// must not classify itself by accident. (The gate trusts the named issue: documented limit.)
|
||||
func TestPRMerge_TheTierIsReadFromTheNamedIssueOnly(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{issueLabels: `[{"id":1,"name":"risk/high"}]`})
|
||||
|
||||
res, err := callMerge(t, f, `{"owner":"owner","repo":"repo","number":7,"issue":42}`)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "high", res["risk"])
|
||||
assert.False(t, f.merged())
|
||||
}
|
||||
|
||||
// ---- review of PR #76 ----
|
||||
|
||||
// An empty head sha used to disable BOTH the CI scoping (the runs query lost its head_sha filter, so the repo's most
|
||||
// recent runs of any commit were classified) and the merge binding (head_commit_id is omitempty). Fail closed.
|
||||
func TestPRMerge_AnEmptyHeadShaFailsClosed(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{emptyHeadSha: true})
|
||||
|
||||
res, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.Contains(t, res["reason"], "head commit")
|
||||
assert.False(t, f.merged(), "an unknown head commit must never merge")
|
||||
assert.Empty(t, f.runsQuery, "no unscoped CI query may be made when the head sha is unknown")
|
||||
}
|
||||
|
||||
// CI is read for the PR's head commit only: the query names it.
|
||||
func TestPRMerge_CIIsScopedToTheHeadCommit(t *testing.T) {
|
||||
f := newPRMergeFake(t, prMergeFakeOpts{})
|
||||
|
||||
_, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, f.runsQuery, "head_sha=abc")
|
||||
}
|
||||
|
||||
// Defence in depth: Gitea refuses closed/merged PRs itself, but the gate should not depend on that.
|
||||
func TestPRMerge_ClosedOrDraftPRsAreNotMerged(t *testing.T) {
|
||||
for name, o := range map[string]prMergeFakeOpts{
|
||||
"closed": {prState: "closed"},
|
||||
"merged": {prState: "merged"},
|
||||
"draft": {prDraft: true},
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
f := newPRMergeFake(t, o)
|
||||
|
||||
res, err := callMerge(t, f, mergeArgs)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.NotEmpty(t, res["reason"])
|
||||
assert.False(t, f.merged())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+119
-16
@@ -37,7 +37,7 @@ const (
|
||||
func (t *TBDShip) Descriptor() registry.ToolDescriptor {
|
||||
return registry.ToolDescriptor{
|
||||
Name: "tbd_ship",
|
||||
Description: "Trunk-based ship of a single-file change: branch from base, write the file, open a PR, and auto-merge (squash) to base ONLY when the head commit's CI is fully green. Fails closed to PR-only (never merges) when CI is pending, red, or absent, when the base branch requires reviews, or when the merge isn't clean — returning the PR for manual handling with a reason. Set ci_timeout_seconds to poll CI to completion (default 0 = snapshot, which returns pending right after opening the PR). Returns {merged, pr_url, pr_number, ci_status, reason, branch}.",
|
||||
Description: "Trunk-based ship of a single-file change: branch from base, write the file, open a PR, and auto-merge (squash) to base ONLY when the head commit's CI is fully green AND the named issue carries exactly one risk/ label, risk/low. Fails closed to PR-only (never merges) when CI is pending, red, or absent, when no issue is named or its risk/ label is missing, ambiguous, unknown, risk/medium or risk/high, when the issue cannot be read, when the base branch requires reviews, or when the merge isn't clean — returning the PR for manual handling with a reason. Set ci_timeout_seconds to poll CI to completion (default 0 = snapshot, which returns pending right after opening the PR). Returns {merged, pr_url, pr_number, ci_status, risk, reason, branch}.",
|
||||
InputSchema: json.RawMessage(`{
|
||||
"type":"object",
|
||||
"properties":{
|
||||
@@ -50,6 +50,7 @@ func (t *TBDShip) Descriptor() registry.ToolDescriptor {
|
||||
"branch":{"type":"string","description":"Short-lived branch name. Default derived: tbd/<slug>-<hash>."},
|
||||
"pr_title":{"type":"string","description":"PR title. Default: the commit message."},
|
||||
"pr_body":{"type":"string"},
|
||||
"issue":{"type":"integer","minimum":1,"description":"Issue number in the same repo whose risk/ label classifies this change. Auto-merge happens only for exactly one label, risk/low; no issue, none, several, an unknown value, risk/medium or risk/high means PR only, never merged (unclassified is not low)."},
|
||||
"delete_branch":{"type":"boolean","description":"Delete the branch after a successful merge. Default true."},
|
||||
"ci_timeout_seconds":{"type":"integer","minimum":0,"maximum":600,"description":"Poll CI up to this long for the head commit's runs to complete. Default 0 (single snapshot)."}
|
||||
},
|
||||
@@ -68,10 +69,76 @@ type tbdShipArgs struct {
|
||||
Branch string `json:"branch"`
|
||||
PRTitle string `json:"pr_title"`
|
||||
PRBody string `json:"pr_body"`
|
||||
Issue int `json:"issue"`
|
||||
DeleteBranch *bool `json:"delete_branch"`
|
||||
CITimeoutSec int `json:"ci_timeout_seconds"`
|
||||
}
|
||||
|
||||
// riskTier is the change's classification under the lane model (local-dev .context/AGENT.md rule 5): only a change
|
||||
// whose issue carries exactly one risk/ label, risk/low, may merge on green; everything else is reviewed by a human
|
||||
// (gitea-mcp#63). The zero value is deliberately NOT riskLow: a forgotten field must never mean "low".
|
||||
type riskTier string
|
||||
|
||||
const (
|
||||
riskLow riskTier = "low"
|
||||
riskMedium riskTier = "medium"
|
||||
riskHigh riskTier = "high"
|
||||
riskNone riskTier = "none" // no issue named, or the issue has no risk/ label: unclassified is not low
|
||||
riskAmbiguous riskTier = "ambiguous" // more than one risk/ label
|
||||
riskUnknown riskTier = "unknown" // risk/<something else>
|
||||
riskUnreadable riskTier = "unreadable" // the issue lookup failed
|
||||
)
|
||||
|
||||
// classifyRisk reads the tier from an issue's labels. Exact, case-sensitive prefix "risk/"; exactly one such label
|
||||
// is required, so two labels (even identical ones) are ambiguous rather than "the lower one wins".
|
||||
func classifyRisk(labels []gitea.Label) riskTier {
|
||||
var found []string
|
||||
for _, l := range labels {
|
||||
if strings.HasPrefix(l.Name, "risk/") {
|
||||
found = append(found, strings.TrimPrefix(l.Name, "risk/"))
|
||||
}
|
||||
}
|
||||
switch len(found) {
|
||||
case 0:
|
||||
return riskNone
|
||||
case 1:
|
||||
default:
|
||||
return riskAmbiguous
|
||||
}
|
||||
switch found[0] {
|
||||
case "low":
|
||||
return riskLow
|
||||
case "medium":
|
||||
return riskMedium
|
||||
case "high":
|
||||
return riskHigh
|
||||
default:
|
||||
return riskUnknown
|
||||
}
|
||||
}
|
||||
|
||||
const unclassifiedReason = "no risk/ label on the named issue (or no issue named) — unclassified is not low; PR left open, not merged"
|
||||
|
||||
// riskBlockReason explains why a tier other than riskLow blocks the merge.
|
||||
func riskBlockReason(r riskTier) string {
|
||||
switch r {
|
||||
case riskMedium, riskHigh:
|
||||
return fmt.Sprintf("the issue is risk/%s — above risk/low a human reviews the change; PR left open, not merged", r)
|
||||
case riskAmbiguous:
|
||||
return "the issue carries more than one risk/ label — ambiguous classification; PR left open, not merged"
|
||||
case riskUnknown:
|
||||
return "the issue carries an unknown risk/ value — PR left open, not merged"
|
||||
case riskUnreadable:
|
||||
return "could not read the issue's labels — fail-closed; PR left open, not merged"
|
||||
case riskLow:
|
||||
return "" // never asked: only a non-low tier needs explaining
|
||||
case riskNone:
|
||||
return unclassifiedReason
|
||||
default: // the zero value and any tier added later: unclassified is not low
|
||||
return unclassifiedReason
|
||||
}
|
||||
}
|
||||
|
||||
// shipGate is the decision produced by evaluateShipGate.
|
||||
type shipGate struct {
|
||||
merge bool
|
||||
@@ -101,29 +168,46 @@ func classifyCI(runs []gitea.WorkflowRun) string {
|
||||
}
|
||||
|
||||
// evaluateShipGate is the load-bearing safety of tbd_ship. It returns merge=true
|
||||
// only when CI is fully green AND the base branch is not review-protected. Every
|
||||
// other state fails closed to PR-only with an explanatory reason (#40).
|
||||
func evaluateShipGate(runs []gitea.WorkflowRun, bp *gitea.BranchProtection) shipGate {
|
||||
// only when CI is fully green AND the base branch is not review-protected AND the
|
||||
// change is classified risk/low. Every other state fails closed to PR-only with an
|
||||
// explanatory reason (#40, gitea-mcp#63).
|
||||
func evaluateShipGate(runs []gitea.WorkflowRun, bp *gitea.BranchProtection, risk riskTier) shipGate {
|
||||
ci := classifyCI(runs)
|
||||
|
||||
// Review-protected base can't be auto-merged regardless of CI.
|
||||
if bp != nil && bp.Protected && bp.RequiredApprovals > 0 {
|
||||
return shipGate{false, ci, fmt.Sprintf(
|
||||
"base branch requires %d approving review(s) — auto-merge disabled; PR opened for manual merge", bp.RequiredApprovals)}
|
||||
"base branch requires %d approving review(s) — auto-merge disabled; PR left open for manual merge", bp.RequiredApprovals)}
|
||||
}
|
||||
|
||||
// Only risk/low merges on green. Checked before CI so an unclassified change never reads as "waiting for CI".
|
||||
if risk != riskLow {
|
||||
return shipGate{false, ci, riskBlockReason(risk)}
|
||||
}
|
||||
|
||||
switch ci {
|
||||
case "success":
|
||||
return shipGate{true, ci, ""}
|
||||
case "pending":
|
||||
return shipGate{false, ci, "CI is still running for the head commit — PR opened; re-invoke once checks complete, or merge manually"}
|
||||
return shipGate{false, ci, "CI is still running for the head commit — PR left open; re-invoke once checks complete, or merge manually"}
|
||||
case "failed":
|
||||
return shipGate{false, ci, "CI failed for the head commit — PR opened, not merged"}
|
||||
return shipGate{false, ci, "CI failed for the head commit — PR left open, not merged"}
|
||||
default: // none
|
||||
return shipGate{false, ci, "no CI runs found for the head commit — fail-closed: a change with no CI gate is never auto-merged to trunk; PR opened for manual merge"}
|
||||
return shipGate{false, ci, "no CI runs found for the head commit — fail-closed: a change with no CI gate is never auto-merged to trunk; PR left open for manual merge"}
|
||||
}
|
||||
}
|
||||
|
||||
// evaluateMergeGate is what both merge tools call. It adds one precondition to evaluateShipGate: the PR's head commit
|
||||
// must be known. CI is read for that commit and the merge is bound to it (head_commit_id); with an empty sha the runs
|
||||
// query would lose its filter (the repo's most recent runs of ANY commit would be classified) and the binding would be
|
||||
// dropped, so an unknown head commit never merges.
|
||||
func evaluateMergeGate(headSHA string, runs []gitea.WorkflowRun, bp *gitea.BranchProtection, risk riskTier) shipGate {
|
||||
if headSHA == "" {
|
||||
return shipGate{false, "none", "the PR's head commit is unknown — fail-closed (CI cannot be scoped and the merge cannot be bound to it); PR left open, not merged"}
|
||||
}
|
||||
return evaluateShipGate(runs, bp, risk)
|
||||
}
|
||||
|
||||
var shipSlugRe = regexp.MustCompile(`[^a-z0-9]+`)
|
||||
|
||||
// deriveBranch builds a deterministic short-lived branch name from the change,
|
||||
@@ -229,8 +313,9 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
|
||||
}
|
||||
|
||||
// CI gate on the PR head commit.
|
||||
runs := t.pollRuns(ctx, args.Owner, args.Repo, pr.Head.Sha, args.CITimeoutSec)
|
||||
gate := evaluateShipGate(runs, bp)
|
||||
runs := pollRuns(ctx, t.c, args.Owner, args.Repo, pr.Head.Sha, args.CITimeoutSec)
|
||||
risk := riskTierOf(ctx, t.c, args.Owner, args.Repo, args.Issue)
|
||||
gate := evaluateMergeGate(pr.Head.Sha, runs, bp, risk)
|
||||
|
||||
result := map[string]any{
|
||||
"merged": false,
|
||||
@@ -238,6 +323,7 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
|
||||
"pr_url": pr.HTMLURL,
|
||||
"branch": branch,
|
||||
"ci_status": gate.ciStatus,
|
||||
"risk": string(risk),
|
||||
}
|
||||
if !gate.merge {
|
||||
result["reason"] = gate.reason
|
||||
@@ -245,9 +331,9 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
|
||||
}
|
||||
|
||||
// Green + unprotected → squash-merge, then delete the short-lived branch.
|
||||
if err := t.c.MergePullRequest(ctx, args.Owner, args.Repo, pr.Number, gitea.MergePRArgs{Do: "squash"}); err != nil {
|
||||
if err := t.c.MergePullRequest(ctx, args.Owner, args.Repo, pr.Number, gitea.MergePRArgs{Do: "squash", HeadCommitID: pr.Head.Sha}); err != nil {
|
||||
if errors.Is(err, gitea.ErrConflict) {
|
||||
result["reason"] = "base moved and the merge is not clean — PR opened, not merged; resolve the conflict and merge manually"
|
||||
result["reason"] = "base moved and the merge is not clean — PR left open, not merged; resolve the conflict and merge manually"
|
||||
return textOK(result)
|
||||
}
|
||||
return nil, fmt.Errorf("merge PR #%d: %w", pr.Number, err)
|
||||
@@ -261,10 +347,27 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
|
||||
return textOK(result)
|
||||
}
|
||||
|
||||
// riskTierOf classifies the change from the named issue. No issue, or an issue that cannot be read, never yields
|
||||
// riskLow. The link between the PR and the issue is the caller's claim: the gate enforces the tier of the issue it
|
||||
// is given, it cannot prove that issue is the one the change belongs to (stated limit, gitea-mcp#63).
|
||||
func riskTierOf(ctx context.Context, c *gitea.Client, owner, repo string, issue int) riskTier {
|
||||
if issue <= 0 {
|
||||
return riskNone
|
||||
}
|
||||
iss, err := c.GetIssue(ctx, owner, repo, issue)
|
||||
if err != nil || iss == nil {
|
||||
return riskUnreadable
|
||||
}
|
||||
return classifyRisk(iss.Labels)
|
||||
}
|
||||
|
||||
// pollRuns lists the head commit's workflow runs, polling up to timeoutSec for
|
||||
// them to reach a terminal state. timeoutSec == 0 is a single snapshot (no
|
||||
// sleep). A listing error yields no runs → the gate fails closed.
|
||||
func (t *TBDShip) pollRuns(ctx context.Context, owner, repo, headSHA string, timeoutSec int) []gitea.WorkflowRun {
|
||||
func pollRuns(ctx context.Context, c *gitea.Client, owner, repo, headSHA string, timeoutSec int) []gitea.WorkflowRun {
|
||||
if headSHA == "" {
|
||||
return nil // never query without the commit filter: Gitea would return the repo's latest runs of any commit
|
||||
}
|
||||
if timeoutSec < 0 {
|
||||
timeoutSec = 0
|
||||
}
|
||||
@@ -273,7 +376,7 @@ func (t *TBDShip) pollRuns(ctx context.Context, owner, repo, headSHA string, tim
|
||||
}
|
||||
deadline := time.Now().Add(time.Duration(timeoutSec) * time.Second)
|
||||
for {
|
||||
runs := t.listRuns(ctx, owner, repo, headSHA)
|
||||
runs := listRuns(ctx, c, owner, repo, headSHA)
|
||||
switch classifyCI(runs) {
|
||||
case "success", "failed":
|
||||
return runs // terminal — no point waiting
|
||||
@@ -304,8 +407,8 @@ func (t *TBDShip) findOpenPR(ctx context.Context, owner, repo, branch string) (*
|
||||
return nil, fmt.Errorf("no open PR found for head %s: %w", branch, gitea.ErrNotFound)
|
||||
}
|
||||
|
||||
func (t *TBDShip) listRuns(ctx context.Context, owner, repo, headSHA string) []gitea.WorkflowRun {
|
||||
resp, err := t.c.ListWorkflowRuns(ctx, owner, repo, gitea.ListWorkflowRunsArgs{HeadSHA: headSHA, Limit: 50})
|
||||
func listRuns(ctx context.Context, c *gitea.Client, owner, repo, headSHA string) []gitea.WorkflowRun {
|
||||
resp, err := c.ListWorkflowRuns(ctx, owner, repo, gitea.ListWorkflowRunsArgs{HeadSHA: headSHA, Limit: 50})
|
||||
if err != nil || resp == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
|
||||
@@ -20,22 +25,31 @@ func TestEvaluateShipGate(t *testing.T) {
|
||||
name string
|
||||
runs []gitea.WorkflowRun
|
||||
bp *gitea.BranchProtection
|
||||
risk riskTier
|
||||
wantMerge bool
|
||||
wantCI string
|
||||
}{
|
||||
{"all green → merge", []gitea.WorkflowRun{run("completed", "success")}, open, true, "success"},
|
||||
{"in_progress → no merge", []gitea.WorkflowRun{run("in_progress", "")}, open, false, "pending"},
|
||||
{"queued → no merge", []gitea.WorkflowRun{run("queued", "")}, open, false, "pending"},
|
||||
{"failed → no merge", []gitea.WorkflowRun{run("completed", "failure")}, open, false, "failed"},
|
||||
{"cancelled → no merge (fail-closed)", []gitea.WorkflowRun{run("completed", "cancelled")}, open, false, "failed"},
|
||||
{"no runs → no merge (no CI gate)", nil, open, false, "none"},
|
||||
{"mixed success+queued → no merge", []gitea.WorkflowRun{run("completed", "success"), run("queued", "")}, open, false, "pending"},
|
||||
{"green but protected → no merge", []gitea.WorkflowRun{run("completed", "success")}, protected, false, "success"},
|
||||
{"no runs + protected → no merge", nil, protected, false, "none"},
|
||||
{"all green → merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskLow, true, "success"},
|
||||
{"in_progress → no merge", []gitea.WorkflowRun{run("in_progress", "")}, open, riskLow, false, "pending"},
|
||||
{"queued → no merge", []gitea.WorkflowRun{run("queued", "")}, open, riskLow, false, "pending"},
|
||||
{"failed → no merge", []gitea.WorkflowRun{run("completed", "failure")}, open, riskLow, false, "failed"},
|
||||
{"cancelled → no merge (fail-closed)", []gitea.WorkflowRun{run("completed", "cancelled")}, open, riskLow, false, "failed"},
|
||||
{"no runs → no merge (no CI gate)", nil, open, riskLow, false, "none"},
|
||||
{"mixed success+queued → no merge", []gitea.WorkflowRun{run("completed", "success"), run("queued", "")}, open, riskLow, false, "pending"},
|
||||
{"green but protected → no merge", []gitea.WorkflowRun{run("completed", "success")}, protected, riskLow, false, "success"},
|
||||
{"no runs + protected → no merge", nil, protected, riskLow, false, "none"},
|
||||
// gitea-mcp#63: the tier is an input. Green CI never overrides it; unclassified is not low.
|
||||
{"green + risk/medium → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskMedium, false, "success"},
|
||||
{"green + risk/high → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskHigh, false, "success"},
|
||||
{"green + no risk label → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskNone, false, "success"},
|
||||
{"green + ambiguous labels → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskAmbiguous, false, "success"},
|
||||
{"green + unknown value → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskUnknown, false, "success"},
|
||||
{"green + unreadable issue → no merge", []gitea.WorkflowRun{run("completed", "success")}, open, riskUnreadable, false, "success"},
|
||||
{"green + the zero tier → no merge (a forgotten field must not mean low)", []gitea.WorkflowRun{run("completed", "success")}, open, riskTier(""), false, "success"},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
g := evaluateShipGate(tc.runs, tc.bp)
|
||||
g := evaluateShipGate(tc.runs, tc.bp, tc.risk)
|
||||
if g.merge != tc.wantMerge {
|
||||
t.Errorf("merge = %v, want %v (reason: %q)", g.merge, tc.wantMerge, g.reason)
|
||||
}
|
||||
@@ -51,3 +65,74 @@ func TestEvaluateShipGate(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassifyRisk(t *testing.T) {
|
||||
l := func(names ...string) []gitea.Label {
|
||||
out := make([]gitea.Label, len(names))
|
||||
for i, n := range names {
|
||||
out[i] = gitea.Label{Name: n}
|
||||
}
|
||||
return out
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
labels []gitea.Label
|
||||
want riskTier
|
||||
}{
|
||||
{"low", l("risk/low"), riskLow},
|
||||
{"medium", l("risk/medium"), riskMedium},
|
||||
{"high", l("risk/high"), riskHigh},
|
||||
{"low among others", l("type/fix", "risk/low", "size/s"), riskLow},
|
||||
{"none", l("type/fix"), riskNone},
|
||||
{"no labels", nil, riskNone},
|
||||
{"two different", l("risk/low", "risk/high"), riskAmbiguous},
|
||||
{"two identical", l("risk/low", "risk/low"), riskAmbiguous},
|
||||
{"unknown value", l("risk/extreme"), riskUnknown},
|
||||
{"empty value", l("risk/"), riskUnknown},
|
||||
{"wrong case", l("Risk/Low"), riskNone},
|
||||
{"value case matters: risk/Low is unknown, not low", l("risk/Low"), riskUnknown},
|
||||
{"trailing space in the value is unknown, not low", l("risk/low "), riskUnknown},
|
||||
{"leading space in the value is unknown, not low", l("risk/ low"), riskUnknown},
|
||||
{"prefix only inside a name", l("not-risk/low"), riskNone},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := classifyRisk(tc.labels); got != tc.want {
|
||||
t.Errorf("classifyRisk(%v) = %q, want %q", tc.labels, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The merge gate needs a head commit to scope CI to and to bind the merge to: without one nothing merges.
|
||||
func TestEvaluateMergeGate_UnknownHeadCommitNeverMerges(t *testing.T) {
|
||||
green := []gitea.WorkflowRun{{Status: "completed", Conclusion: "success"}}
|
||||
g := evaluateMergeGate("", green, &gitea.BranchProtection{}, riskLow)
|
||||
if g.merge {
|
||||
t.Fatalf("merge = true for an empty head sha (reason %q)", g.reason)
|
||||
}
|
||||
if !strings.Contains(g.reason, "head commit") {
|
||||
t.Errorf("reason %q should name the head commit", g.reason)
|
||||
}
|
||||
if ok := evaluateMergeGate("abc", green, &gitea.BranchProtection{}, riskLow); !ok.merge {
|
||||
t.Errorf("a known head sha with green CI and risk/low must still merge, got %q", ok.reason)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPollRuns_NoShaMakesNoQuery(t *testing.T) {
|
||||
var requests atomic.Int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
requests.Add(1)
|
||||
_, _ = w.Write([]byte(`{"total_count":1,"workflow_runs":[{"id":1,"status":"completed","conclusion":"success"}]}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
c := gitea.NewClient(srv.URL, "tok")
|
||||
|
||||
if runs := pollRuns(context.Background(), c, "o", "r", "", 0); len(runs) != 0 || requests.Load() != 0 {
|
||||
t.Errorf("an empty sha must make no request and return no runs, got %d runs after %d requests", len(runs), requests.Load())
|
||||
}
|
||||
// positive control: with a sha the same server is asked, so the zero above means something
|
||||
if runs := pollRuns(context.Background(), c, "o", "r", "abc", 0); len(runs) != 1 || requests.Load() != 1 {
|
||||
t.Errorf("control: with a sha expected 1 run after 1 request, got %d runs after %d requests", len(runs), requests.Load())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package tools_test
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
@@ -26,11 +27,21 @@ func shipFake(t *testing.T, runsJSON string, merged, deleted *atomic.Bool) *http
|
||||
// shipFakeOpts adds protection (requiredApprovals>0 → protected) and a merge
|
||||
// status code, so tests can drive the review-protected and conflict paths.
|
||||
func shipFakeOpts(t *testing.T, runsJSON string, requiredApprovals, mergeStatus int, merged, deleted *atomic.Bool) *httptest.Server {
|
||||
t.Helper()
|
||||
return shipFakeFull(t, runsJSON, requiredApprovals, mergeStatus, `[{"id":1,"name":"risk/low"}]`, http.StatusOK, merged, deleted)
|
||||
}
|
||||
|
||||
// shipFakeFull also serves the issue whose risk/ label classifies the change (gitea-mcp#63): issueLabels is the
|
||||
// labels JSON array, issueStatus the HTTP status of the issue lookup.
|
||||
func shipFakeFull(t *testing.T, runsJSON string, requiredApprovals, mergeStatus int, issueLabels string, issueStatus int, merged, deleted *atomic.Bool) *httptest.Server {
|
||||
t.Helper()
|
||||
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
p := r.URL.Path
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
switch {
|
||||
case r.Method == http.MethodGet && strings.Contains(p, "/issues/42"):
|
||||
w.WriteHeader(issueStatus)
|
||||
_, _ = w.Write([]byte(`{"number":42,"title":"t","state":"open","labels":` + issueLabels + `}`))
|
||||
case r.Method == http.MethodGet && strings.Contains(p, "/branch_protections/"):
|
||||
if requiredApprovals > 0 {
|
||||
_, _ = w.Write([]byte(`{"required_approvals":` + itoa(requiredApprovals) + `}`))
|
||||
@@ -49,10 +60,12 @@ func shipFakeOpts(t *testing.T, runsJSON string, requiredApprovals, mergeStatus
|
||||
_, _ = w.Write([]byte(`{"content":{"path":"x","sha":"s"},"commit":{"sha":"c"}}`))
|
||||
case r.Method == http.MethodPost && strings.HasSuffix(p, "/pulls"):
|
||||
w.WriteHeader(http.StatusCreated)
|
||||
_, _ = w.Write([]byte(`{"number":7,"title":"t","html_url":"http://x/pulls/7","state":"open","head":{"ref":"tbd/x","sha":"abc"},"base":{"ref":"main"}}`))
|
||||
_, _ = w.Write([]byte(`{"number":7,"title":"t","html_url":"http://x/pulls/7","state":"open","head":{"ref":"tbd/x","sha":"` + shipPRHeadSha + `"},"base":{"ref":"main"}}`))
|
||||
case r.Method == http.MethodGet && strings.Contains(p, "/actions/runs"):
|
||||
_, _ = w.Write([]byte(`{"total_count":0,"workflow_runs":` + runsJSON + `}`))
|
||||
case r.Method == http.MethodPost && strings.HasSuffix(p, "/pulls/7/merge"):
|
||||
body, _ := io.ReadAll(r.Body)
|
||||
shipLastMergeBody.Store(string(body))
|
||||
merged.Store(true)
|
||||
w.WriteHeader(mergeStatus)
|
||||
_, _ = w.Write([]byte(`{}`))
|
||||
@@ -83,7 +96,7 @@ func TestTBDShip_GreenCI_Merges(t *testing.T) {
|
||||
srv := shipFake(t, `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
|
||||
|
||||
assert.Equal(t, true, res["merged"])
|
||||
assert.Equal(t, "success", res["ci_status"])
|
||||
@@ -98,7 +111,7 @@ func TestTBDShip_NoCI_FailsClosed(t *testing.T) {
|
||||
srv := shipFake(t, `[]`, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
|
||||
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.Equal(t, "none", res["ci_status"])
|
||||
@@ -113,7 +126,7 @@ func TestTBDShip_RedCI_FailsClosed(t *testing.T) {
|
||||
srv := shipFake(t, `[{"id":1,"status":"completed","conclusion":"failure","head_sha":"abc"}]`, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
|
||||
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.Equal(t, "failed", res["ci_status"])
|
||||
@@ -128,7 +141,7 @@ func TestTBDShip_ReviewProtected_FailsClosed(t *testing.T) {
|
||||
srv := shipFakeOpts(t, `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`, 1, http.StatusOK, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
|
||||
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.NotEmpty(t, res["reason"])
|
||||
@@ -142,7 +155,7 @@ func TestTBDShip_MergeConflict_FailsClosed(t *testing.T) {
|
||||
srv := shipFakeOpts(t, `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`, 0, http.StatusConflict, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`)
|
||||
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.NotEmpty(t, res["reason"])
|
||||
@@ -160,6 +173,8 @@ func TestTBDShip_Resume_ExistingBranchAndPR(t *testing.T) {
|
||||
p := r.URL.Path
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
switch {
|
||||
case r.Method == http.MethodGet && strings.Contains(p, "/issues/42"):
|
||||
_, _ = w.Write([]byte(`{"number":42,"title":"t","state":"open","labels":[{"id":1,"name":"risk/low"}]}`))
|
||||
case r.Method == http.MethodGet && strings.Contains(p, "/branch_protections/"):
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
_, _ = w.Write([]byte(`{"message":"not found"}`))
|
||||
@@ -196,7 +211,7 @@ func TestTBDShip_Resume_ExistingBranchAndPR(t *testing.T) {
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","branch":"`+branch+`"}`)
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42,"branch":"`+branch+`"}`)
|
||||
|
||||
assert.Equal(t, true, res["merged"], "resume must merge when CI is green")
|
||||
assert.Equal(t, float64(7), res["pr_number"], "must reuse the existing PR #7")
|
||||
@@ -204,6 +219,138 @@ func TestTBDShip_Resume_ExistingBranchAndPR(t *testing.T) {
|
||||
assert.True(t, merged.Load())
|
||||
}
|
||||
|
||||
// shipPRHeadSha is the head sha the fake reports for the PR it opens (tests may blank it, then restore it).
|
||||
var shipPRHeadSha = "abc"
|
||||
|
||||
// shipLastMergeBody is the body of the most recent merge request the fake received.
|
||||
var shipLastMergeBody atomic.Value
|
||||
|
||||
// An empty head sha used to unscope the CI query and drop the merge binding (review of PR #76). Fail closed.
|
||||
func TestTBDShip_AnEmptyHeadShaFailsClosed(t *testing.T) {
|
||||
shipPRHeadSha = ""
|
||||
t.Cleanup(func() { shipPRHeadSha = "abc" })
|
||||
var merged, deleted atomic.Bool
|
||||
srv := shipFake(t, shipGreen, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, shipArgs)
|
||||
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.Contains(t, res["reason"], "head commit")
|
||||
assert.False(t, merged.Load(), "an unknown head commit must never merge")
|
||||
}
|
||||
|
||||
// TOCTOU: the merge is bound to the head commit whose CI the gate evaluated (gitea-mcp#74).
|
||||
func TestTBDShip_TheMergeIsBoundToTheCommitTheGateChecked(t *testing.T) {
|
||||
var merged, deleted atomic.Bool
|
||||
srv := shipFake(t, shipGreen, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, shipArgs)
|
||||
|
||||
require.Equal(t, true, res["merged"])
|
||||
var payload map[string]any
|
||||
require.NoError(t, json.Unmarshal([]byte(shipLastMergeBody.Load().(string)), &payload))
|
||||
assert.Equal(t, "abc", payload["head_commit_id"], "the head sha the CI check used")
|
||||
}
|
||||
|
||||
// ---- gitea-mcp#63: the risk tier is an input to the gate. Only exactly one risk/low label merges. ----
|
||||
const (
|
||||
shipGreen = `[{"id":1,"status":"completed","conclusion":"success","head_sha":"abc"}]`
|
||||
shipArgs = `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x","issue":42}`
|
||||
)
|
||||
|
||||
func TestTBDShip_RiskTier_OnlyLowMerges(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
labels string
|
||||
wantMerged bool
|
||||
wantInReason string
|
||||
}{
|
||||
{"risk/low merges", `[{"id":1,"name":"risk/low"}]`, true, ""},
|
||||
{"risk/low among other labels merges", `[{"id":2,"name":"type/fix"},{"id":1,"name":"risk/low"},{"id":3,"name":"size/s"}]`, true, ""},
|
||||
{"risk/medium does not merge", `[{"id":1,"name":"risk/medium"}]`, false, "risk/medium"},
|
||||
{"risk/high does not merge", `[{"id":1,"name":"risk/high"}]`, false, "risk/high"},
|
||||
{"no risk label is unclassified, not low", `[{"id":2,"name":"type/fix"}]`, false, "no risk/"},
|
||||
{"no labels at all", `[]`, false, "no risk/"},
|
||||
{"two risk labels are ambiguous", `[{"id":1,"name":"risk/low"},{"id":2,"name":"risk/high"}]`, false, "more than one"},
|
||||
{"two identical risk/low labels are still ambiguous", `[{"id":1,"name":"risk/low"},{"id":2,"name":"risk/low"}]`, false, "more than one"},
|
||||
{"an unknown risk value does not merge", `[{"id":1,"name":"risk/lowish"}]`, false, "unknown"},
|
||||
{"an empty risk value does not merge", `[{"id":1,"name":"risk/"}]`, false, "unknown"},
|
||||
{"labels are case-sensitive: RISK/LOW is not risk/low", `[{"id":1,"name":"RISK/LOW"}]`, false, "no risk/"},
|
||||
{"a label that merely contains risk/low does not count", `[{"id":1,"name":"not-risk/low"}]`, false, "no risk/"},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
var merged, deleted atomic.Bool
|
||||
srv := shipFakeFull(t, shipGreen, 0, http.StatusOK, tc.labels, http.StatusOK, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, shipArgs)
|
||||
|
||||
assert.Equal(t, tc.wantMerged, res["merged"])
|
||||
assert.Equal(t, tc.wantMerged, merged.Load(), "the merge endpoint is called iff the tier allows it")
|
||||
assert.Equal(t, "success", res["ci_status"], "CI was green: the tier alone decided")
|
||||
assert.Equal(t, float64(7), res["pr_number"], "the PR is opened either way")
|
||||
if !tc.wantMerged {
|
||||
assert.Contains(t, res["reason"], tc.wantInReason)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// No issue named → nothing classifies the change → no merge, even on green CI.
|
||||
func TestTBDShip_NoIssue_Unclassified_NoMerge(t *testing.T) {
|
||||
var merged, deleted atomic.Bool
|
||||
srv := shipFake(t, shipGreen, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
res := callShip(t, srv.URL, `{"owner":"mathias","repo":"myrepo","path":"docs/x.md","content":"hi","message":"add x"}`)
|
||||
|
||||
assert.Equal(t, false, res["merged"])
|
||||
assert.Contains(t, res["reason"], "issue")
|
||||
assert.False(t, merged.Load(), "an unclassified change must not merge")
|
||||
assert.Equal(t, float64(7), res["pr_number"])
|
||||
}
|
||||
|
||||
// The issue lookup failing is not a reason to merge: fail closed, PR left open.
|
||||
func TestTBDShip_IssueLookupFails_FailsClosed(t *testing.T) {
|
||||
for _, status := range []int{http.StatusInternalServerError, http.StatusNotFound, http.StatusForbidden} {
|
||||
var merged, deleted atomic.Bool
|
||||
srv := shipFakeFull(t, shipGreen, 0, http.StatusOK, `[{"id":1,"name":"risk/low"}]`, status, &merged, &deleted)
|
||||
|
||||
res := callShip(t, srv.URL, shipArgs)
|
||||
srv.Close()
|
||||
|
||||
assert.Equal(t, false, res["merged"], "status %d", status)
|
||||
assert.NotEmpty(t, res["reason"], "status %d", status)
|
||||
assert.False(t, merged.Load(), "status %d: an unreadable issue must not merge", status)
|
||||
}
|
||||
}
|
||||
|
||||
// risk/low is necessary, not sufficient: red CI still blocks it, and a review-protected base still blocks it.
|
||||
func TestTBDShip_RiskLow_DoesNotOverrideTheOtherGates(t *testing.T) {
|
||||
var merged, deleted atomic.Bool
|
||||
red := shipFakeFull(t, `[{"id":1,"status":"completed","conclusion":"failure","head_sha":"abc"}]`, 0, http.StatusOK, `[{"id":1,"name":"risk/low"}]`, http.StatusOK, &merged, &deleted)
|
||||
defer red.Close()
|
||||
assert.Equal(t, false, callShip(t, red.URL, shipArgs)["merged"])
|
||||
assert.False(t, merged.Load())
|
||||
|
||||
prot := shipFakeFull(t, shipGreen, 1, http.StatusOK, `[{"id":1,"name":"risk/low"}]`, http.StatusOK, &merged, &deleted)
|
||||
defer prot.Close()
|
||||
assert.Equal(t, false, callShip(t, prot.URL, shipArgs)["merged"])
|
||||
assert.False(t, merged.Load())
|
||||
}
|
||||
|
||||
// The decision is visible to the caller.
|
||||
func TestTBDShip_ReportsTheRiskTier(t *testing.T) {
|
||||
var merged, deleted atomic.Bool
|
||||
srv := shipFakeFull(t, shipGreen, 0, http.StatusOK, `[{"id":1,"name":"risk/medium"}]`, http.StatusOK, &merged, &deleted)
|
||||
defer srv.Close()
|
||||
|
||||
assert.Equal(t, "medium", callShip(t, srv.URL, shipArgs)["risk"])
|
||||
}
|
||||
|
||||
func TestTBDShip_AllowlistRejects(t *testing.T) {
|
||||
tool := tools.NewTBDShip(gitea.NewClient("http://unused", ""), allowlist.New([]string{"mathias"}))
|
||||
_, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"evil","repo":"r","path":"p","content":"c","message":"m"}`))
|
||||
|
||||
Reference in New Issue
Block a user