Files

Claude Code AI - Gitea MCP 598a7c326a refactor(skills): retrofit the corpus to the ADR-0020 context contract (#129 )

Retrofits all 39 skills to ADR-0020's description/body context contract, then fixes what six rounds of independent review found in that retrofit — including four ways the hot gate itself failed open.

Closes #99, #107, #108, #110, #111, #114, #115, #120.

## The retrofit (waves 1-5)

| | Start | Now |
|---|---|---|
| Description FAILs (>400 chars) | 26 | **0** |
| Body FAILs (>900 words, body-only) | 9 | **0** |
| Dangling routing targets | 2 | **0** |
| `Kyberforge.CompositionNote` | 10 | **0** |
| Preload tax | 21,005 chars | **~10,500** |

Under the 12,000-char success criterion. Per-wave detail is on #99.

## The review fixes

**The gate failed open four ways, three of them found after the retrofit shipped.** An unrecognised follower token made a dangling target vanish. A skill directory with no `SKILL.md` resolved as a valid target, so a commit could be green locally and red in a fresh clone — three existing fixtures were relying on that, one of which made the install-leak A/B pass vacuously. Then the free-standing `/name` sweep turned out to be gated on the sentence carrying a boundary marker, so route notation in any other sentence was invisible — not an ERROR, not a SUGGESTION, not an INFO — which left the documented "`/name` always blocks" promise false from a second direction. All four fixed and pinned.

**Two checks were silently not running.** `validate-provenance.sh` checks 7-8 were dead across nine skills. Waking them exposed a deeper problem: they assume `Research doc:` names a source index, but 30 of 121 entries point at topic content documents, so every new check-7 INFO was a false positive and check 8 was saved from a false-FAIL flood only by an *unannounced* skip. Checks 7/8 are now scoped to source indexes and every skip announces itself (#121).

**The retrofit's own anti-goal, four times.** ADR-0020 warns that a blunt gate gets satisfied by deleting content rather than relocating it. `diagnose` and `skill-audit` relocated prose and then read it unconditionally; `prototype` and `vale-config` deleted rules outright that survived nowhere. All four addressed.

## Verification

- `bash tests/run-tests.sh --strict` — 24 suites, 0 skipped, 0 failed
- `bash tests/run-bats.sh` — 325 tests, 0 failures
- `pre-commit run --all-files` — 17/17
- `pre-commit run --hook-stage pre-push --all-files` — 16/16, with `apm marketplace check` and `apm pack --check-clean` run against the remote, not skipped
- `scripts/skill-size-check.sh` over all 39 skills — rc 0, 0 ERROR/FAIL, SUGGESTION-only
- Preload tax measured at **10,498 chars**, max description 390 — both inside budget
- Every new test proven non-vacuous by a deliberate mutation of the behaviour it covers

**Per-commit sync, stated accurately:** the ten commits from the latest review round each pass `check-plugin-content-sync` in isolation, verified by checking each out in a detached worktree with a clean between. The earlier gitea window (`dfacf05..bedbd1d`, nine commits) does **not** — its mirror was regenerated in one batch at `bbc7300`. An earlier revision of this description claimed the property held for every commit; it does not, and a bisect through that window lands on a red commit. **Squash-merge** to collapse it, or accept that this range is not bisectable.

## Version bump

Six plugins and the catalog take a **patch**, not a minor. The branch is **89 commits — 40 `fix` / 30 `refactor` / 12 `docs` / 5 `chore` / 2 `test` — zero `feat`, zero `!`, zero `BREAKING CHANGE`** — and adds no skill, agent, command or hook. (Two earlier revisions of this section cited a stale histogram, most recently 78 commits; the figures above are measured at HEAD.) Both rules this repo ships (`forge/references/version-bump.md`, landing in this PR, and `git-commits/references/conventional-commits-spec.md`) make that a patch, and the catalog set is unchanged at 7 entries.

Not settled by that: four published files were removed from the installed tree, three moved, and `caveman` gained `disable-model-invocation`, retiring its old triggers. Under a strict reading those are major-class and currently ship under `refactor:` with no marker. Whether the deployed skill surface is a public contract is written down nowhere — worth deciding, but it outlives this PR.

## Deliberately not in scope

#112 (cherry-pick ownership, now resolved in favour of `git-commits`), #113 (`rtk git` normalisation), #116 (research fan-out), #101 (audit-skill merge), #122 (non-spec skill-root files), #123 (no PRD producer) stay open. #117 is the one worth reading: the contract's remedy is to move prose into `references/`, which is exactly where neither the size gate nor Vale looks — and the blind spot is wider than #117 currently records, since there is no root `.vale.ini` at all, so every ADR, `CONTEXT.md` and `README.md` is unlinted too.

That blind spot let this branch carry two `level: error` `Kyberforge.SentenceOpenerThereIs` violations into `references/` files it created — `provider-adapter-author/references/provider-matrix.md:31` and `agent-audit/references/finding-criteria.md:95`. Both are reworded in `afadaae`, confirmed by routing each file through the audit's own `vale-wrap.sh` (1 error each before, 0 after). Five further occurrences sit in `references/` files already on `main`; those are the pre-existing corpus and stay with #117, which is the real fix.

Also unfixed and not this PR's: `apm install` appends a duplicate `SessionStart` entry to `.claude/settings.json`, so a fresh clone cannot get pre-push green without an edit AGENTS.md warns against. Reproduces identically on `main`.

Co-authored-by: Defame1297 <gitea@rkdr.net>
Reviewed-on: https://git.dev.rkdr.net/Defame1297/holocron/pulls/129
Co-authored-by: Claude Code AI - Gitea MCP <claude@noreply.git.dev.rkdr.net>
Co-committed-by: Claude Code AI - Gitea MCP <claude@noreply.git.dev.rkdr.net>

2026-09-01 13:47:46 +00:00

6.4 KiB

Raw Blame History

topic, source_keys

topic

source_keys

reviews

gitea-mcp-repo

gitea-mcp-slim-go

PR review execution detail

Parameter signatures below are cross-checked live against the deployed gitea-mcp server tool schema, not copied from the plugin's research doc verbatim — same sourcing discipline as references/pull-requests.md. Last verified against v1.7.0, as reported by get_gitea_mcp_server_version.

Review state machine

A review is not a single write. It moves through states:

create — opens a review in "PENDING" state, optionally attaching inline comments. Nothing is visible to other users yet.
submit — finalizes the pending review with a terminal state: "APPROVED", "REQUEST_CHANGES", or "COMMENT". This is the point at which the review becomes visible and counts toward merge-gate requirements (see references/merging.md).
dismiss — invalidates an already-submitted review (e.g. an approval that's no longer valid after force-push), with an optional message giving the reason. Dismissal does not delete the review record — it stays visible but marked dismissed.
delete — removes a review outright. Use only for a review that was never submitted (e.g. abandoning a PENDING draft); do not use delete to retract a submitted review — use dismiss instead.

`pull_request_review_write`

Parameters:

method (string, required) — "create" | "submit" | "delete" | "dismiss" | "reply_comment" | "resolve_thread" | "unresolve_thread"
owner (string, required)
repo (string, required)
pull_number (number, required for every method except "resolve_thread" and "unresolve_thread", which locate the thread from comment_id alone) — the schema's own required list is only method/owner/repo, so a missing pull_number surfaces as a runtime error, not client-side validation
review_id (number) — required for "submit", "delete" and "dismiss"; "create" returns the ID to use for that follow-up call. Not used by "reply_comment", "resolve_thread" or "unresolve_thread", which key off comment_id instead
comment_id (number, required for "reply_comment", "resolve_thread" and "unresolve_thread") — an individual review comment's ID, obtained from pull_request_read method: "get_review_comments". For the two thread methods this must be the thread's first comment, not an arbitrary one in it
state (string, optional) — "APPROVED" | "REQUEST_CHANGES" | "COMMENT" | "PENDING" — set on "create" (typically "PENDING", or a terminal state to create-and-submit in one call if the server supports it) or "submit" (terminal state)
body (string, optional) — the overall review comment text on "create"/"submit"; on "reply_comment" it is the reply text and is the payload of the call
commit_id (string, optional, for "create") — anchors inline comments to a specific commit SHA (typically the PR's current head SHA from pull_request_read method: "get")
message (string, optional, for "dismiss") — dismissal reason
comments (array of objects, optional, for "create") — inline comments, each: {path, body, old_line_num, new_line_num} — path is the file path, body is the comment text, new_line_num anchors to a line in the new (added) side of the diff, old_line_num anchors to a line in the old (removed) side; use whichever side the comment applies to, not both

Comment threads

Three further methods act on an individual review comment rather than on a review as a whole. They sit outside the state machine above — a thread can be replied to or resolved whatever state its parent review is in — and none of them takes a review_id.

reply_comment — posts body as a reply to the comment named by comment_id, threading under it rather than starting a new top-level comment. Takes pull_number.
resolve_thread — marks the thread containing comment_id resolved. Pass the thread's first comment ID; another ID in the same thread is not equivalent. Does not take pull_number.
unresolve_thread — reopens a resolved thread, under the same first-comment rule.

Get the comment_id from pull_request_read method: "get_review_comments". Call it with no review_id to list every inline comment on the PR, then pick the thread to act on; scoping it to one review_id only finds threads opened by that review.

Reading reviews (`pull_request_read`)

method: "get_reviews" — array of review summaries: id, state, body, user (login), comments_count, submitted_at, html_url, stale (bool — the PR was pushed to after this review was submitted, meaning it may be outdated), official (bool), dismissed (bool).
method: "get_review" (requires review_id — omitting it fails with review_id is required) — single review detail.
method: "get_review_comments" (review_id optional — omit it to list every inline comment on the PR in one call, rather than one review's) — array of inline comments: id, body, path, position, old_position, diff_hunk, user, html_url, created_at, updated_at.

review_comments on the "get" response is a count, not the comments — and it may be absent. Where the full PR object returned by pull_request_read method: "get" carries review_comments, it is an integer: the number of inline review comments. Presence is not guaranteed. A live "get" against a PR with zero inline comments carried no such key at all — only comments, which counts issue-style comments, not review ones. Treat it as present only when non-zero, and check for the key before reading it rather than assuming the response shape. Either way it is distinct from the get_review_comments method above, which returns the actual comment objects; reading the count is no substitute for that call. Older gitea-mcp releases misspelled this key as review_scomments; the misspelling was corrected upstream and the deployed v1.7.0 response carries no such key, so treat any instruction that reaches for review_scomments as stale.

Inline-comment field names differ between write and read. The comments array on pull_request_review_write method: "create" uses old_line_num/new_line_num. The get_review_comments read response uses different field names for the same concept — position (new-side line) and old_position (old-side line). Do not assume the same key names apply on both sides of the round trip.

6.4 KiB Raw Blame History