21: Add forgejo_list_milestones tool #35

Merged

david merged 2 commits from feature/issue-21/add-forgejo-list-milestones-tool into main

2026-08-18 08:02:41 +00:00

david commented

2026-08-18 08:02:39 +00:00

Owner

21: Add forgejo_list_milestones tool

Implements 21.

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 8m 28s.

Review findings

The grep tool isn't quoting patterns with spaces. Let me use bash directly.Now let me check the exact test block and search for any other references that need updating.I have full context. Now let me apply the fixes. First, the main tool handler in src/index.ts (findings 1, 2, 3):Now align the limit schema on the two existing search tools (finding 3's parenthetical — same server cap applies):Now the DESIGN.md diagram count (finding 4):Now update the tool handler tests to cover the new behavior (findings 1 and 2):Now let me verify the changes and discover the build/test commands:Let me review the final handler and run the gates:All gates pass. Let me run the coverage gate to confirm the new branches are covered:All gates pass, and milestones.ts remains at 100% coverage. Let me review the final working-tree diff:All changes are complete and gates pass. Here is my remediation summary.

Summary of remediation

Fixed all 4 code/doc findings; 1 finding (commit-message format) is out of scope for the working tree and remains unresolved:

Medium (has_more / pagination hint): forgejo_list_milestones now computes has_more = milestones.length === (params.limit ?? 10) (server default page size), appends a "More milestones may be available — pass page=N (or a larger limit) to continue" line when a full page returns, and includes details.has_more. Tests added for the full-page and partial-page branches.
Low (state-aware empty message): empty results now print No open milestones (pass state="all" to include closed milestones). when state is unset/"open", and No milestones match. otherwise. Tests updated/added.
Low (limit cap): milestone tool limit schema changed from maximum: 100 to maximum: 50; per the finding's suggestion I also aligned the identical server-cap issue in forgejo_issue_search (line 761) and forgejo_pr_search (line 1075), which had the same maximum: 100.
Low (DESIGN.md diagram): "(12 tools)" → "(15 tools)".
Low (AGENTS.md:30): left unresolved — it concerns the already-created commit 21: Add forgejo_list_milestones tool (55670c2) and future PR/commit naming; this pipeline stage must not commit/amend, so there is nothing in the working tree to change.

Gates re-run: npm run check ✅, npm run build ✅, npm test ✅ (9 suites, 148 tests), npm run test:coverage ✅ (milestones.ts at 100% stmt/line/branch/func).

Review Findings — 21

Summary

Stage 1 adds the forgejo_list_milestones tool (name→id lookup for the milestone param), with API function, TypeBox schema, tool handler, docs updates, and tests. Gates: npm run check (tsc --noEmit) passed, npm run build (tsc) passed, npm test (jest) passed — 9 suites / 148 tests (2 added in remediation), with milestones.ts at 100% coverage. No lint configuration exists in the repo (no lint gate). The implementation is correct and follows existing tool patterns; findings below are UX/consistency improvements, no critical or high issues.

Remediation: the pagination-hint finding (Medium), the state-aware empty message (Low), the limit cap (Low, including aligning forgejo_issue_search/forgejo_pr_search to maximum: 50), and the DESIGN.md diagram count (Low) are all fixed. The AGENTS.md commit-message finding (Low) is left unresolved: it concerns the already-created commit 21: Add forgejo_list_milestones tool (55670c2) and future squash-merge/PR naming, which this stage cannot change (no commit/amend permitted); it is recorded for the human operator to apply when merging.

Critical

No critical findings.

High

No high findings.

Medium

extensions/forgejo/src/index.ts:1260 — The result text "Found N milestone(s)" and details.count (line 1266) reflect only the current page, and there is no hint when more pages exist. Since the tool's stated purpose is name→id lookup before passing a milestone id, a repo with more than limit (server default 10) milestones would silently omit later milestones and the model could wrongly conclude a milestone doesn't exist. Suggested fix: when milestones.length === limit (i.e. a full page came back), append a line like "More milestones may be available — pass page=N (or a larger limit) to continue" and/or add details.has_more: milestones.length === (params.limit ?? 10).

Low

extensions/forgejo/src/index.ts:1258 — When no state is passed, the server defaults to state=open; a repo whose milestones are all closed returns [] and the tool prints "No milestones in this repository." even though closed milestones exist — the message overstates the data and could mislead the model. Suggested fix: make the empty message state-aware, e.g. "No open milestones (pass state="all" to include closed milestones)" when state is unset or "open", and "No milestones match" otherwise.
extensions/forgejo/src/index.ts:1238 — The limit schema allows maximum: 100, but the Forgejo/Gitea API caps page size at 50 (setting.API.MaxResponseItems), so values 51–100 are silently truncated by the server. Suggested fix: set maximum: 50 to match server behavior (also consider aligning the existing forgejo_issue_search schema at lines 761/1075).
extensions/forgejo/docs/DESIGN.md:36 — The architecture diagram still says the issue tools box has "(12 tools)", which is now more inconsistent with the updated "Issues (15)" table (line 90). Suggested fix: update the diagram to "(15 tools)" (or reword to count only forgejo_issue_* ops if that is the intent).
AGENTS.md:30 — The commit message for this change is 21: Add forgejo_list_milestones tool (commit 55670c2), but AGENTS.md requires the format issue-<N>: <summary>. Suggested fix: use issue-21: Add forgejo_list_milestones tool for the squash-merge commit / PR title (per AGENTS.md:34), and follow issue-<N>: <summary> for future commits.

## 21: Add forgejo_list_milestones tool Implements [21](https://git.excelera.net/david/pi-extensions-and-skills/issues/21). > 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 8m 28s. ## Review findings The grep tool isn't quoting patterns with spaces. Let me use bash directly.Now let me check the exact test block and search for any other references that need updating.I have full context. Now let me apply the fixes. First, the main tool handler in `src/index.ts` (findings 1, 2, 3):Now align the `limit` schema on the two existing search tools (finding 3's parenthetical — same server cap applies):Now the DESIGN.md diagram count (finding 4):Now update the tool handler tests to cover the new behavior (findings 1 and 2):Now let me verify the changes and discover the build/test commands:Let me review the final handler and run the gates:All gates pass. Let me run the coverage gate to confirm the new branches are covered:All gates pass, and `milestones.ts` remains at 100% coverage. Let me review the final working-tree diff:All changes are complete and gates pass. Here is my remediation summary. ## Summary of remediation Fixed all 4 code/doc findings; 1 finding (commit-message format) is out of scope for the working tree and remains unresolved: - **Medium (has_more / pagination hint):** `forgejo_list_milestones` now computes `has_more = milestones.length === (params.limit ?? 10)` (server default page size), appends a "More milestones may be available — pass page=N (or a larger limit) to continue" line when a full page returns, and includes `details.has_more`. Tests added for the full-page and partial-page branches. - **Low (state-aware empty message):** empty results now print `No open milestones (pass state="all" to include closed milestones).` when `state` is unset/`"open"`, and `No milestones match.` otherwise. Tests updated/added. - **Low (limit cap):** milestone tool `limit` schema changed from `maximum: 100` to `maximum: 50`; per the finding's suggestion I also aligned the identical server-cap issue in `forgejo_issue_search` (line 761) and `forgejo_pr_search` (line 1075), which had the same `maximum: 100`. - **Low (DESIGN.md diagram):** "(12 tools)" → "(15 tools)". - **Low (AGENTS.md:30):** left unresolved — it concerns the already-created commit `21: Add forgejo_list_milestones tool` (55670c2) and future PR/commit naming; this pipeline stage must not commit/amend, so there is nothing in the working tree to change. **Gates re-run:** `npm run check` ✅, `npm run build` ✅, `npm test` ✅ (9 suites, 148 tests), `npm run test:coverage` ✅ (milestones.ts at 100% stmt/line/branch/func). --- # Review Findings — 21 ## Summary Stage 1 adds the `forgejo_list_milestones` tool (name→id lookup for the `milestone` param), with API function, TypeBox schema, tool handler, docs updates, and tests. Gates: `npm run check` (tsc --noEmit) passed, `npm run build` (tsc) passed, `npm test` (jest) passed — 9 suites / 148 tests (2 added in remediation), with milestones.ts at 100% coverage. No lint configuration exists in the repo (no lint gate). The implementation is correct and follows existing tool patterns; findings below are UX/consistency improvements, no critical or high issues. **Remediation:** the pagination-hint finding (Medium), the state-aware empty message (Low), the `limit` cap (Low, including aligning `forgejo_issue_search`/`forgejo_pr_search` to `maximum: 50`), and the DESIGN.md diagram count (Low) are all fixed. The AGENTS.md commit-message finding (Low) is left unresolved: it concerns the already-created commit `21: Add forgejo_list_milestones tool` (55670c2) and future squash-merge/PR naming, which this stage cannot change (no commit/amend permitted); it is recorded for the human operator to apply when merging. ## Critical - [ ] _No critical findings._ ## High - [ ] _No high findings._ ## Medium - [x] extensions/forgejo/src/index.ts:1260 — The result text "Found N milestone(s)" and `details.count` (line 1266) reflect only the current page, and there is no hint when more pages exist. Since the tool's stated purpose is name→id lookup before passing a `milestone` id, a repo with more than `limit` (server default 10) milestones would silently omit later milestones and the model could wrongly conclude a milestone doesn't exist. Suggested fix: when `milestones.length === limit` (i.e. a full page came back), append a line like "More milestones may be available — pass page=N (or a larger limit) to continue" and/or add `details.has_more: milestones.length === (params.limit ?? 10)`. ## Low - [x] extensions/forgejo/src/index.ts:1258 — When no `state` is passed, the server defaults to `state=open`; a repo whose milestones are all closed returns `[]` and the tool prints "No milestones in this repository." even though closed milestones exist — the message overstates the data and could mislead the model. Suggested fix: make the empty message state-aware, e.g. "No open milestones (pass state=\"all\" to include closed milestones)" when `state` is unset or `"open"`, and "No milestones match" otherwise. - [x] extensions/forgejo/src/index.ts:1238 — The `limit` schema allows `maximum: 100`, but the Forgejo/Gitea API caps page size at 50 (`setting.API.MaxResponseItems`), so values 51–100 are silently truncated by the server. Suggested fix: set `maximum: 50` to match server behavior (also consider aligning the existing `forgejo_issue_search` schema at lines 761/1075). - [x] extensions/forgejo/docs/DESIGN.md:36 — The architecture diagram still says the issue tools box has "(12 tools)", which is now more inconsistent with the updated "Issues (15)" table (line 90). Suggested fix: update the diagram to "(15 tools)" (or reword to count only `forgejo_issue_*` ops if that is the intent). - [ ] AGENTS.md:30 — The commit message for this change is `21: Add forgejo_list_milestones tool` (commit 55670c2), but AGENTS.md requires the format `issue-<N>: <summary>`. Suggested fix: use `issue-21: Add forgejo_list_milestones tool` for the squash-merge commit / PR title (per AGENTS.md:34), and follow `issue-<N>: <summary>` for future commits.

david added 2 commits

2026-08-18 08:02:40 +00:00