20: Add forgejo_milestone_create tool #34

Merged

david merged 2 commits from feature/issue-20/add-forgejo-milestone-create-tool into main

2026-08-18 07:54:09 +00:00

david commented

2026-08-18 07:54:08 +00:00

Owner

20: Add forgejo_milestone_create tool

Implements 20.

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 37s.

Review findings

I have a full picture now. Let me apply the fixes. Starting with the source code fixes:Now the docs fixes (README and DESIGN.md):Now add the formatting-branch tests to tests/tools.test.ts:Now run the gates (build, check, test) as detected from package.json:All gates pass (136 tests, up from 134 with the two new formatting-branch tests). Let me verify the final diff of my remediation changes:All fixes are applied and gates are green. Here is the annotated findings document.

Review Findings — 20

Summary

Stage 1 adds forgejo_milestone_create (tool handler in src/index.ts, API wrapper in new src/milestones.ts, unit tests in tests/milestones.test.ts and tests/tools.test.ts, registration updated in tests/index.test.ts). Gates run: npm run build (tsc) — PASS; npm run check (tsc --noEmit) — PASS; npm test (jest) — PASS (136 tests / 9 suites after adding 2 formatting-branch tests); lint — N/A (no lint script or config exists in the project). Remediation pass resolved all code/docs/test findings: reworded the promptGuidelines (no more nonexistent forgejo_list_milestones call), documented the tool in README.md and docs/DESIGN.md (with the v2 future-enhancement entry adjusted to scope it to list/lookup), dropped the dead id parameter from milestonePath, hardened the open_issues/closed_issues guards to != null, and added tests locking the formatMilestone formatting branches. The only unresolved finding is the existing commit message (20: ... vs the issue-20: ... convention), which this stage cannot amend because the pipeline does not commit or rewrite history.

Critical

(none)

High

extensions/forgejo/src/index.ts:1219 — The new tool's promptGuidelines tells the agent to "Use forgejo_list_milestones afterwards to confirm the new milestone's id", but forgejo_list_milestones is not registered anywhere in the extension (only forgejo_milestone_create exists; docs/DESIGN.md:21,208 explicitly defer forgejo_list_milestones to v2). Every follow-up agent turn would attempt a nonexistent tool call and fail. Suggested fix: reword the guideline to "The created milestone's id is returned in the tool result; use it as the milestone param of forgejo_issue_create/forgejo_pr_create."

Medium

extensions/forgejo/README.md:58, extensions/forgejo/docs/DESIGN.md:105 — The new tool is missing from the README tool tables and the DESIGN.md tool matrix, and DESIGN.md:206-208 still lists milestone operations under "Future Enhancements (v2)". The repo convention (prior PR #33 for forgejo_label_create) updates README.md and docs/DESIGN.md when adding a tool. Suggested fix: add a forgejo_milestone_create row to the README tool table (and DESIGN.md matrix), and remove/adjust the "Milestones (v2)" future-enhancement entry now that a milestone create tool exists.

Low

extensions/forgejo/src/milestones.ts:33 — milestonePath(target, id?) has a dead id parameter and an untested /milestones/5 branch; only createMilestone calls it, always without an id. Suggested fix: drop the id parameter until a view/list-milestone function needs it.
extensions/forgejo/src/index.ts:225 — open_issues/closed_issues are guarded with !== undefined while description/due_on use truthy checks. If Forgejo ever returns null for these (the codebase already normalizes nulls from the API — see the issue #29 regression fix for null labels/assignees), the text would print "Open issues: null" and details would carry null. Suggested fix: use != null (or truthy) guards for consistency with the other fields.
extensions/forgejo/tests/tools.test.ts:491 — formatMilestone's conditional branches ("Due:" line, open/closed issue counts, description-absent) are untested; the new handler tests only cover a description-present milestone. Suggested fix: add a test case with due_on set and one with no description/due_on to lock the formatting branches.
extensions/forgejo (commit c3ec43f) — Commit message 20: Add forgejo_milestone_create tool deviates from the AGENTS.md format issue-<N>: <summary> (e.g. issue-20: ...). Suggested fix: use the issue-20: prefix on the squash-merge commit (prior commits deviate too, but the standard is explicit).

## 20: Add forgejo_milestone_create tool Implements [20](https://git.excelera.net/david/pi-extensions-and-skills/issues/20). > 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 37s. ## Review findings I have a full picture now. Let me apply the fixes. Starting with the source code fixes:Now the docs fixes (README and DESIGN.md):Now add the formatting-branch tests to `tests/tools.test.ts`:Now run the gates (build, check, test) as detected from `package.json`:All gates pass (136 tests, up from 134 with the two new formatting-branch tests). Let me verify the final diff of my remediation changes:All fixes are applied and gates are green. Here is the annotated findings document. # Review Findings — 20 ## Summary Stage 1 adds `forgejo_milestone_create` (tool handler in `src/index.ts`, API wrapper in new `src/milestones.ts`, unit tests in `tests/milestones.test.ts` and `tests/tools.test.ts`, registration updated in `tests/index.test.ts`). Gates run: `npm run build` (tsc) — **PASS**; `npm run check` (tsc --noEmit) — **PASS**; `npm test` (jest) — **PASS** (136 tests / 9 suites after adding 2 formatting-branch tests); **lint — N/A** (no lint script or config exists in the project). Remediation pass resolved all code/docs/test findings: reworded the `promptGuidelines` (no more nonexistent `forgejo_list_milestones` call), documented the tool in README.md and docs/DESIGN.md (with the v2 future-enhancement entry adjusted to scope it to list/lookup), dropped the dead `id` parameter from `milestonePath`, hardened the `open_issues`/`closed_issues` guards to `!= null`, and added tests locking the `formatMilestone` formatting branches. The only unresolved finding is the existing commit message (`20: ...` vs the `issue-20: ...` convention), which this stage cannot amend because the pipeline does not commit or rewrite history. ## Critical - (none) ## High - [x] extensions/forgejo/src/index.ts:1219 — The new tool's `promptGuidelines` tells the agent to "Use forgejo_list_milestones afterwards to confirm the new milestone's id", but `forgejo_list_milestones` is **not registered** anywhere in the extension (only `forgejo_milestone_create` exists; docs/DESIGN.md:21,208 explicitly defer `forgejo_list_milestones` to v2). Every follow-up agent turn would attempt a nonexistent tool call and fail. Suggested fix: reword the guideline to "The created milestone's id is returned in the tool result; use it as the `milestone` param of forgejo_issue_create/forgejo_pr_create." ## Medium - [x] extensions/forgejo/README.md:58, extensions/forgejo/docs/DESIGN.md:105 — The new tool is missing from the README tool tables and the DESIGN.md tool matrix, and DESIGN.md:206-208 still lists milestone operations under "Future Enhancements (v2)". The repo convention (prior PR #33 for `forgejo_label_create`) updates README.md and docs/DESIGN.md when adding a tool. Suggested fix: add a `forgejo_milestone_create` row to the README tool table (and DESIGN.md matrix), and remove/adjust the "Milestones (v2)" future-enhancement entry now that a milestone create tool exists. ## Low - [x] extensions/forgejo/src/milestones.ts:33 — `milestonePath(target, id?)` has a dead `id` parameter and an untested `/milestones/5` branch; only `createMilestone` calls it, always without an id. Suggested fix: drop the `id` parameter until a view/list-milestone function needs it. - [x] extensions/forgejo/src/index.ts:225 — `open_issues`/`closed_issues` are guarded with `!== undefined` while `description`/`due_on` use truthy checks. If Forgejo ever returns `null` for these (the codebase already normalizes nulls from the API — see the issue #29 regression fix for null labels/assignees), the text would print "Open issues: null" and details would carry `null`. Suggested fix: use `!= null` (or truthy) guards for consistency with the other fields. - [x] extensions/forgejo/tests/tools.test.ts:491 — `formatMilestone`'s conditional branches ("Due:" line, open/closed issue counts, description-absent) are untested; the new handler tests only cover a description-present milestone. Suggested fix: add a test case with `due_on` set and one with no description/due_on to lock the formatting branches. - [ ] extensions/forgejo (commit c3ec43f) — Commit message `20: Add forgejo_milestone_create tool` deviates from the AGENTS.md format `issue-<N>: <summary>` (e.g. `issue-20: ...`). Suggested fix: use the `issue-20:` prefix on the squash-merge commit (prior commits deviate too, but the standard is explicit).

david added 2 commits

2026-08-18 07:54:08 +00:00