24: Add forgejo_milestone_close and forgejo_milestone_reopen tools #38

Merged

david merged 2 commits from feature/issue-24/add-forgejo-milestone-close-and-forgejo-milestone into main

2026-08-18 08:25:22 +00:00

david commented

2026-08-18 08:25:21 +00:00

Owner

24: Add forgejo_milestone_close and forgejo_milestone_reopen tools

Implements 24.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 7m 01s.

Review findings

Now let me check what label tools exist to determine the right wording for the DESIGN.md fix, and look at the tools test for the existing error test pattern.Now I have the full picture. Let me apply the three feasible fixes: DESIGN.md stale future-enhancement line, README smoke-test step 8, and the symmetric reopenMilestone 404 test.Now let me discover the build/test commands from the project files and run the gates.The forgejo extension has build, test, and check scripts (no lint). Let me run the gates.All gates pass — build, check, and 173 tests (up from 172 with the new symmetric 404 test). Let me verify the final diff.All three feasible findings are fixed and gates are green. Here is the annotated findings document.

Review Findings — 24

Summary

The change adds forgejo_milestone_close and forgejo_milestone_reopen tools to the Forgejo extension as thin wrappers over editMilestone with state: "closed"/"open", mirroring the existing issue close/reopen pattern. Gates run: npm run build (tsc) — PASS, npm run check (tsc --noEmit) — PASS, npm test (jest) — PASS (9 suites, 173 tests). Lint gate did not run — no lint script or ESLint config exists in either the root or extensions/forgejo package. Remediation pass fixed 3 of 4 findings: the stale DESIGN.md future-enhancement line now marks milestone CRUD as shipped, README smoke-test step 8 now exercises forgejo_milestone_close/forgejo_milestone_reopen, and a symmetric 404 not-found test was added for reopenMilestone (test count 172 → 173). The AGENTS.md commit-message finding is left unresolved: it concerns the already-committed Stage 1 commit message, and fixing it would require creating/amending a commit, which this stage is prohibited from doing.

Critical

None

High

None

Medium

extensions/forgejo/docs/DESIGN.md:220 — The "Future Enhancements (v2)" line "Repo-scoped ops (labels CRUD beyond list, milestones) if the agent needs them" is now stale: milestone CRUD (create/list/view/edit/close/reopen) is fully shipped, and this very diff updates DESIGN.md (tool counts + endpoint note) without correcting this line, leaving the doc internally contradictory. Suggested fix: change to "Repo-scoped ops (labels CRUD beyond list) if the agent needs them" or mark milestone ops as shipped.

Low

extensions/forgejo/README.md:128 — The manual smoke-test step 8 says "close it and forgejo_list_milestones state="all" still lists it" (referring to the edit tool's state param), but never mentions the new forgejo_milestone_close / forgejo_milestone_reopen tools that are the subject of this issue. Suggested fix: extend step 8 to call forgejo_milestone_close id=<id>, then forgejo_milestone_reopen id=<id> and verify the state round-trips.
extensions/forgejo/tests/milestones.test.ts:348 — The new closeMilestone / reopenMilestone describe block only has an error-propagation test for closeMilestone; reopenMilestone's failure path is untested (the tools-level suite in tests/tools.test.ts also only exercises close errors). The shared editMilestone path means coverage is adequate, but a symmetric 404 test for reopenMilestone would fully harden the new API surface. Suggested fix: add a "propagates a 404 not-found error unchanged" test calling reopenMilestone(target, 999).
AGENTS.md:30 (Stage 1 commit 6c52f46) — The commit message is 24: Add forgejo_milestone_close and forgejo_milestone_reopen tools, but the documented convention is issue-<N>: <summary>. Recent repo history (issues 21–23) uses the same N: style, so this is consistent with practice yet deviates from the stated standard. Suggested fix: use issue-24: Add forgejo_milestone_close and forgejo_milestone_reopen tools for the commit (and issue-24: <title> for the PR) to match AGENTS.md.

Low

extensions/forgejo/README.md:128 — The manual smoke-test step 8 says "close it and forgejo_list_milestones state="all" still lists it" (referring to the edit tool's state param), but never mentions the new forgejo_milestone_close / forgejo_milestone_reopen tools that are the subject of this issue. Suggested fix: extend step 8 to call forgejo_milestone_close id=<id>, then forgejo_milestone_reopen id=<id> and verify the state round-trips.
extensions/forgejo/tests/milestones.test.ts:348 — The new closeMilestone / reopenMilestone describe block only has an error-propagation test for closeMilestone; reopenMilestone's failure path is untested (the tools-level suite in tests/tools.test.ts also only exercises close errors). The shared editMilestone path means coverage is adequate, but a symmetric 404 test for reopenMilestone would fully harden the new API surface. Suggested fix: add a "propagates a 404 not-found error unchanged" test calling reopenMilestone(target, 999).
AGENTS.md:30 (Stage 1 commit 6c52f46) — The commit message is 24: Add forgejo_milestone_close and forgejo_milestone_reopen tools, but the documented convention is issue-<N>: <summary>. Recent repo history (issues 21–23) uses the same N: style, so this is consistent with practice yet deviates from the stated standard. Suggested fix: use issue-24: Add forgejo_milestone_close and forgejo_milestone_reopen tools for the commit (and issue-24: <title> for the PR) to match AGENTS.md.

## 24: Add forgejo_milestone_close and forgejo_milestone_reopen tools Implements [24](https://git.excelera.net/david/pi-extensions-and-skills/issues/24). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 7m 01s. ## Review findings Now let me check what label tools exist to determine the right wording for the DESIGN.md fix, and look at the tools test for the existing error test pattern.Now I have the full picture. Let me apply the three feasible fixes: DESIGN.md stale future-enhancement line, README smoke-test step 8, and the symmetric `reopenMilestone` 404 test.Now let me discover the build/test commands from the project files and run the gates.The forgejo extension has `build`, `test`, and `check` scripts (no lint). Let me run the gates.All gates pass — build, check, and 173 tests (up from 172 with the new symmetric 404 test). Let me verify the final diff.All three feasible findings are fixed and gates are green. Here is the annotated findings document. # Review Findings — 24 ## Summary The change adds `forgejo_milestone_close` and `forgejo_milestone_reopen` tools to the Forgejo extension as thin wrappers over `editMilestone` with `state: "closed"`/`"open"`, mirroring the existing issue close/reopen pattern. Gates run: `npm run build` (tsc) — PASS, `npm run check` (tsc --noEmit) — PASS, `npm test` (jest) — PASS (9 suites, 173 tests). Lint gate did not run — no lint script or ESLint config exists in either the root or `extensions/forgejo` package. Remediation pass fixed 3 of 4 findings: the stale DESIGN.md future-enhancement line now marks milestone CRUD as shipped, README smoke-test step 8 now exercises `forgejo_milestone_close`/`forgejo_milestone_reopen`, and a symmetric 404 not-found test was added for `reopenMilestone` (test count 172 → 173). The AGENTS.md commit-message finding is left unresolved: it concerns the already-committed Stage 1 commit message, and fixing it would require creating/amending a commit, which this stage is prohibited from doing. ## Critical - [ ] None ## High - [ ] None ## Medium - [x] `extensions/forgejo/docs/DESIGN.md:220` — The "Future Enhancements (v2)" line "Repo-scoped ops (labels CRUD beyond list, milestones) if the agent needs them" is now stale: milestone CRUD (create/list/view/edit/close/reopen) is fully shipped, and this very diff updates DESIGN.md (tool counts + endpoint note) without correcting this line, leaving the doc internally contradictory. Suggested fix: change to "Repo-scoped ops (labels CRUD beyond list) if the agent needs them" or mark milestone ops as shipped. ## Low - [x] `extensions/forgejo/README.md:128` — The manual smoke-test step 8 says "close it and `forgejo_list_milestones state="all"` still lists it" (referring to the edit tool's `state` param), but never mentions the new `forgejo_milestone_close` / `forgejo_milestone_reopen` tools that are the subject of this issue. Suggested fix: extend step 8 to call `forgejo_milestone_close id=<id>`, then `forgejo_milestone_reopen id=<id>` and verify the state round-trips. - [x] `extensions/forgejo/tests/milestones.test.ts:348` — The new `closeMilestone / reopenMilestone` describe block only has an error-propagation test for `closeMilestone`; `reopenMilestone`'s failure path is untested (the tools-level suite in `tests/tools.test.ts` also only exercises close errors). The shared `editMilestone` path means coverage is adequate, but a symmetric 404 test for `reopenMilestone` would fully harden the new API surface. Suggested fix: add a "propagates a 404 not-found error unchanged" test calling `reopenMilestone(target, 999)`. - [ ] `AGENTS.md:30` (Stage 1 commit `6c52f46`) — The commit message is `24: Add forgejo_milestone_close and forgejo_milestone_reopen tools`, but the documented convention is `issue-<N>: <summary>`. Recent repo history (issues 21–23) uses the same `N:` style, so this is consistent with practice yet deviates from the stated standard. Suggested fix: use `issue-24: Add forgejo_milestone_close and forgejo_milestone_reopen tools` for the commit (and `issue-24: <title>` for the PR) to match AGENTS.md. ## Low - [x] `extensions/forgejo/README.md:128` — The manual smoke-test step 8 says "close it and `forgejo_list_milestones state="all"` still lists it" (referring to the edit tool's `state` param), but never mentions the new `forgejo_milestone_close` / `forgejo_milestone_reopen` tools that are the subject of this issue. Suggested fix: extend step 8 to call `forgejo_milestone_close id=<id>`, then `forgejo_milestone_reopen id=<id>` and verify the state round-trips. - [x] `extensions/forgejo/tests/milestones.test.ts:348` — The new `closeMilestone / reopenMilestone` describe block only has an error-propagation test for `closeMilestone`; `reopenMilestone`'s failure path is untested (the tools-level suite in `tests/tools.test.ts` also only exercises close errors). The shared `editMilestone` path means coverage is adequate, but a symmetric 404 test for `reopenMilestone` would fully harden the new API surface. Suggested fix: add a "propagates a 404 not-found error unchanged" test calling `reopenMilestone(target, 999)`. - [ ] `AGENTS.md:30` (Stage 1 commit `6c52f46`) — The commit message is `24: Add forgejo_milestone_close and forgejo_milestone_reopen tools`, but the documented convention is `issue-<N>: <summary>`. Recent repo history (issues 21–23) uses the same `N:` style, so this is consistent with practice yet deviates from the stated standard. Suggested fix: use `issue-24: Add forgejo_milestone_close and forgejo_milestone_reopen tools` for the commit (and `issue-24: <title>` for the PR) to match AGENTS.md.

david added 2 commits

2026-08-18 08:25:21 +00:00