22: Add forgejo_milestone_view tool #36

Merged
david merged 2 commits from feature/issue-22/add-forgejo-milestone-view-tool into main 2026-08-18 08:09:42 +00:00
Owner

22: Add forgejo_milestone_view tool

Implements 22.

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 6m 57s.

Review findings

All locations confirmed. Now applying the fixes:Now running the gates discovered in extensions/forgejo/package.json:All gates pass. Let me verify the final state of each fix:All five fixable findings are resolved and all gates pass. Final annotated findings:

Review Findings — 22

Summary

The change adds the forgejo_milestone_view tool (registration, viewMilestone API call, README/DESIGN doc updates, and unit + handler + registration tests). Gates run: npm run build (tsc) PASS, npm run check (tsc --noEmit) PASS, npm test (jest) PASS (9 suites / 154 tests, 100% stmt/branch coverage on milestones.ts). No lint script or ESLint config exists in the repo, so there is no lint gate to run. The implementation is correct and well-tested; remaining findings are doc-consistency and output-polish issues only.

Remediation fixed 5 of the 6 findings: updated the architecture diagram count (15→16), changed the view tool's output verb to "Viewed" (plus the now-accurate test comment), dropped the misleading "(fallback: name)" claim from the endpoint table, reworded the promptGuidelines to reference the real milestone-assignment flow, and generalized the shared 404 message to "id/number". The commit-message finding is left unresolved because this pipeline forbids git commit/amend operations.

Critical

  • (none)

High

  • (none)

Medium

  • extensions/forgejo/docs/DESIGN.md:36 — The architecture diagram still says forgejo_issue_* tools (15 tools) while this diff bumped the "Issues" tool-table heading to ### Issues (16) (line 90). The count is now internally inconsistent within the same file. Suggested fix: update the diagram box to (16 tools).

Low

  • extensions/forgejo/src/index.ts:1313 — formatMilestone(result.value, "Milestone") renders Milestone milestone #5: v1.0 (the action string is concatenated with the literal milestone #id in formatMilestone at line 223). The tool's primary output duplicates the word "milestone". Suggested fix: pass a verb such as "Viewed" (or "View") so the output reads Viewed milestone #5: v1.0; existing tests assert toContain("milestone #5: v1.0") and would still pass.

  • extensions/forgejo/docs/DESIGN.md:150 — The endpoint-table row claims "by numeric id (fallback: name)", but the tool schema is Type.Integer({ minimum: 1 }) (index.ts:1300) and viewMilestone accepts only a numeric id — a name would be rejected by validation. The claim is misleading about what the tool can do. Suggested fix: drop "(fallback: name)" from the row (or, if a name lookup is intended, add a name param resolved via forgejo_list_milestones).

  • extensions/forgejo/src/index.ts:1297 — The promptGuidelines advise using the tool "before editing or closing a milestone", but the registry has no milestone edit/close tools (only list/create/view), so the guideline references capabilities that don't exist. Suggested fix: reword to a real flow, e.g. "before assigning issues/PRs to a milestone" or "before closing a milestone in the web UI".

  • extensions/forgejo/src/api.ts:98 — The shared 404 mapper message says "verify owner/repo and the issue/PR number". For forgejo_milestone_view, a bad milestone id surfaces this message, which tells the agent to check an issue/PR number. Suggested fix: generalize the shared message to "...and the id/number (or run forgejo_detect)", or wrap milestone 404s in the tool with a milestone-id hint.

  • Commit 0662969 (22: Add forgejo_milestone_view tool) — AGENTS.md specifies commit format issue-<N>: <summary> (e.g. issue-42: ...); the commit omits the issue- prefix. Suggested fix: git commit --amend to issue-22: Add forgejo_milestone_view tool. (Note: prior commits in history use the same non-conforming pattern; still worth aligning going forward.)

## 22: Add forgejo_milestone_view tool Implements [22](https://git.excelera.net/david/pi-extensions-and-skills/issues/22). > 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 6m 57s. ## Review findings All locations confirmed. Now applying the fixes:Now running the gates discovered in `extensions/forgejo/package.json`:All gates pass. Let me verify the final state of each fix:All five fixable findings are resolved and all gates pass. Final annotated findings: # Review Findings — 22 ## Summary The change adds the `forgejo_milestone_view` tool (registration, `viewMilestone` API call, README/DESIGN doc updates, and unit + handler + registration tests). Gates run: `npm run build` (tsc) ✅ PASS, `npm run check` (tsc --noEmit) ✅ PASS, `npm test` (jest) ✅ PASS (9 suites / 154 tests, 100% stmt/branch coverage on `milestones.ts`). No lint script or ESLint config exists in the repo, so there is no lint gate to run. The implementation is correct and well-tested; remaining findings are doc-consistency and output-polish issues only. Remediation fixed 5 of the 6 findings: updated the architecture diagram count (15→16), changed the view tool's output verb to `"Viewed"` (plus the now-accurate test comment), dropped the misleading "(fallback: name)" claim from the endpoint table, reworded the `promptGuidelines` to reference the real milestone-assignment flow, and generalized the shared 404 message to "id/number". The commit-message finding is left unresolved because this pipeline forbids git commit/amend operations. ## Critical - [ ] (none) ## High - [ ] (none) ## Medium - [x] extensions/forgejo/docs/DESIGN.md:36 — The architecture diagram still says `forgejo_issue_* tools (15 tools)` while this diff bumped the "Issues" tool-table heading to `### Issues (16)` (line 90). The count is now internally inconsistent within the same file. Suggested fix: update the diagram box to `(16 tools)`. ## Low - [x] extensions/forgejo/src/index.ts:1313 — `formatMilestone(result.value, "Milestone")` renders `Milestone milestone #5: v1.0` (the action string is concatenated with the literal `milestone #id` in `formatMilestone` at line 223). The tool's primary output duplicates the word "milestone". Suggested fix: pass a verb such as `"Viewed"` (or `"View"`) so the output reads `Viewed milestone #5: v1.0`; existing tests assert `toContain("milestone #5: v1.0")` and would still pass. - [x] extensions/forgejo/docs/DESIGN.md:150 — The endpoint-table row claims "by numeric id (fallback: name)", but the tool schema is `Type.Integer({ minimum: 1 })` (index.ts:1300) and `viewMilestone` accepts only a numeric `id` — a name would be rejected by validation. The claim is misleading about what the tool can do. Suggested fix: drop "(fallback: name)" from the row (or, if a name lookup is intended, add a `name` param resolved via `forgejo_list_milestones`). - [x] extensions/forgejo/src/index.ts:1297 — The `promptGuidelines` advise using the tool "before editing or closing a milestone", but the registry has no milestone edit/close tools (only list/create/view), so the guideline references capabilities that don't exist. Suggested fix: reword to a real flow, e.g. "before assigning issues/PRs to a milestone" or "before closing a milestone in the web UI". - [x] extensions/forgejo/src/api.ts:98 — The shared 404 mapper message says "verify owner/repo and the issue/PR number". For `forgejo_milestone_view`, a bad milestone id surfaces this message, which tells the agent to check an issue/PR number. Suggested fix: generalize the shared message to "...and the id/number (or run forgejo_detect)", or wrap milestone 404s in the tool with a milestone-id hint. - [ ] Commit `0662969` (`22: Add forgejo_milestone_view tool`) — AGENTS.md specifies commit format `issue-<N>: <summary>` (e.g. `issue-42: ...`); the commit omits the `issue-` prefix. Suggested fix: `git commit --amend` to `issue-22: Add forgejo_milestone_view tool`. (Note: prior commits in history use the same non-conforming pattern; still worth aligning going forward.)
david merged commit 708350c2b0 into main 2026-08-18 08:09:42 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
david/pi-extensions-and-skills!36
No description provided.