25: Add forgejo_milestone_delete tool #39

Merged

david merged 2 commits from feature/issue-25/add-forgejo-milestone-delete-tool into main

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

david commented

2026-08-18 08:36:23 +00:00

Owner

25: Add forgejo_milestone_delete tool

Implements 25.

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 10m 59s.

Review findings

The working tree is clean and the diff is already committed. Now let me apply the two findings. First, let me confirm the exact current lines and check Forgejo's delete behavior documented in the repo:The grep failed due to shell pipe interpretation. Let me check the tool list and Gitea/Forgejo delete-milestone behavior documentation:Let me check the issue_edit tool and milestones.ts doc to craft a consistent wording fix:Let me check the issue_create tool params to see if the smoke test can assign a milestone to an issue:Let me check the formatIssue to see if the issue view shows milestone, and look at how the smoke test could verify the claim:Let me check whether forgejo_issue_view displays the milestone field (needed for the smoke test verification) and look at the issues format function:Let me check whether the issue view formats the milestone field (needed for an accurate smoke-test instruction):Let me check the issue format/view to see whether the milestone is displayed (for the smoke test wording), and confirm Gitea's delete-milestone behavior:Now I have full context. Let me check the package.json scripts before making changes:Now I understand the findings. The Medium finding is about the inaccurate claim "does not delete or unassign" — in Gitea/Forgejo, deleting a milestone clears milestone_id on its issues (issues remain but are unassigned from the milestone). Let me verify the Gitea behavior claim and then apply fixes.The Medium finding is well-founded: in Gitea/Forgejo, DeleteMilestone clears milestone_id on the issues (issues remain but lose the milestone association), so "does not delete or unassign" is inaccurate. Now let me apply the fixes:Now let me update the README to align the wording (Medium finding also covers README.md:64) and strengthen the smoke test so the claim is verifiable by the manual test:Now let me align the milestones.ts doc comment for consistency, since it makes the same claim:Now let me verify the changes and run the gates:Now let me run the build/test gates:All gates pass. Let me verify no other occurrences of the inaccurate claim remain:Let me refine the smoke test wording — forgejo_issue_view output doesn't surface the milestone field (formatIssue omits it), so the manual verification must be via the web UI:Let me re-run the full gate suite to confirm everything is green after the final README tweak:All gates pass. Let me confirm coverage for milestones.ts is still intact, and review the final diff:All gates pass and both findings are resolved. Final annotated findings:

Review Findings — 25

Summary

Fixed both findings in a single pass. The Medium finding (inaccurate "does not delete or unassign" claim) was resolved by aligning the wording to Forgejo's actual behavior — deleting a milestone clears the milestone association on its issues/PRs (they remain, but their milestone assignment is removed). The corrected wording was applied consistently across the tool description and promptGuidelines (index.ts), the README tool table (README.md:64), the deleteMilestone doc comment (milestones.ts), and the README manual smoke test, which now exercises the full flow (create issue → assign milestone → delete milestone → verify the issue still exists with its milestone cleared). The Low finding (id param description) was resolved by matching the sibling milestone tools' convention. Gates: npm run build — PASS; npm run check — PASS; npm test — PASS (9 suites, 178 tests); npm run test:coverage — PASS (milestones.ts at 100% coverage).

Critical

No critical findings.

High

No high findings.

Medium

extensions/forgejo/src/index.ts:1479 — The tool's promptGuidelines states as fact: "Deleting a milestone does not delete or unassign its issues/PRs." In Gitea/Forgejo, deleting a milestone clears the milestone association on its issues (issues remain but are unassigned from the milestone), so the "does not unassign" claim is likely inaccurate and could mislead the agent. The README smoke test (extensions/forgejo/README.md:129) only verifies the milestone disappears from forgejo_list_milestones; it never checks what happens to an issue assigned to the deleted milestone, so the claim is unverified by the manual test. Suggested fix: verify on a live scratch instance (create issue → assign milestone → delete milestone → inspect the issue's milestone field), then align the wording (e.g. "does not delete its issues/PRs, but removes their milestone assignment") or soften to the verifiable claim only. Apply the same check to README.md:64, which currently claims "does not delete its issues/PRs". Resolved: reworded promptGuidelines (and the tool description, the deleteMilestone doc comment, and README.md:64) to "does not delete its issues/PRs, but removes their milestone assignment"; extended the README smoke test step 8 to create an issue assigned to the milestone and verify the issue survives with its milestone cleared.

Low

extensions/forgejo/src/index.ts:1483 — The id parameter description is "Milestone id", while the sibling milestone tools all use "Milestone id (from forgejo_list_milestones)" (see lines 1308, 1377, 1432, 1457). The hint helps the agent run forgejo_list_milestones first for id lookup and is part of the established convention. Suggested fix: change to "Milestone id (from forgejo_list_milestones)". Resolved: changed the id parameter description to match the sibling milestone tools.

## 25: Add forgejo_milestone_delete tool Implements [25](https://git.excelera.net/david/pi-extensions-and-skills/issues/25). > 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 10m 59s. ## Review findings The working tree is clean and the diff is already committed. Now let me apply the two findings. First, let me confirm the exact current lines and check Forgejo's delete behavior documented in the repo:The grep failed due to shell pipe interpretation. Let me check the tool list and Gitea/Forgejo delete-milestone behavior documentation:Let me check the issue_edit tool and milestones.ts doc to craft a consistent wording fix:Let me check the issue_create tool params to see if the smoke test can assign a milestone to an issue:Let me check the formatIssue to see if the issue view shows milestone, and look at how the smoke test could verify the claim:Let me check whether `forgejo_issue_view` displays the milestone field (needed for the smoke test verification) and look at the issues format function:Let me check whether the issue view formats the milestone field (needed for an accurate smoke-test instruction):Let me check the issue format/view to see whether the milestone is displayed (for the smoke test wording), and confirm Gitea's delete-milestone behavior:Now I have full context. Let me check the package.json scripts before making changes:Now I understand the findings. The Medium finding is about the inaccurate claim "does not delete or unassign" — in Gitea/Forgejo, deleting a milestone clears `milestone_id` on its issues (issues remain but are unassigned from the milestone). Let me verify the Gitea behavior claim and then apply fixes.The Medium finding is well-founded: in Gitea/Forgejo, `DeleteMilestone` clears `milestone_id` on the issues (issues remain but lose the milestone association), so "does not delete or unassign" is inaccurate. Now let me apply the fixes:Now let me update the README to align the wording (Medium finding also covers README.md:64) and strengthen the smoke test so the claim is verifiable by the manual test:Now let me align the milestones.ts doc comment for consistency, since it makes the same claim:Now let me verify the changes and run the gates:Now let me run the build/test gates:All gates pass. Let me verify no other occurrences of the inaccurate claim remain:Let me refine the smoke test wording — `forgejo_issue_view` output doesn't surface the milestone field (formatIssue omits it), so the manual verification must be via the web UI:Let me re-run the full gate suite to confirm everything is green after the final README tweak:All gates pass. Let me confirm coverage for milestones.ts is still intact, and review the final diff:All gates pass and both findings are resolved. Final annotated findings: # Review Findings — 25 ## Summary Fixed both findings in a single pass. The Medium finding (inaccurate "does not delete or unassign" claim) was resolved by aligning the wording to Forgejo's actual behavior — deleting a milestone clears the milestone association on its issues/PRs (they remain, but their milestone assignment is removed). The corrected wording was applied consistently across the tool `description` and `promptGuidelines` (index.ts), the README tool table (README.md:64), the `deleteMilestone` doc comment (milestones.ts), and the README manual smoke test, which now exercises the full flow (create issue → assign milestone → delete milestone → verify the issue still exists with its milestone cleared). The Low finding (id param description) was resolved by matching the sibling milestone tools' convention. Gates: `npm run build` — PASS; `npm run check` — PASS; `npm test` — PASS (9 suites, 178 tests); `npm run test:coverage` — PASS (milestones.ts at 100% coverage). ## Critical - No critical findings. ## High - No high findings. ## Medium - [x] extensions/forgejo/src/index.ts:1479 — The tool's `promptGuidelines` states as fact: "Deleting a milestone does not delete or unassign its issues/PRs." In Gitea/Forgejo, deleting a milestone clears the milestone association on its issues (issues remain but are unassigned from the milestone), so the "does not unassign" claim is likely inaccurate and could mislead the agent. The README smoke test (extensions/forgejo/README.md:129) only verifies the milestone disappears from `forgejo_list_milestones`; it never checks what happens to an issue assigned to the deleted milestone, so the claim is unverified by the manual test. Suggested fix: verify on a live scratch instance (create issue → assign milestone → delete milestone → inspect the issue's milestone field), then align the wording (e.g. "does not delete its issues/PRs, but removes their milestone assignment") or soften to the verifiable claim only. Apply the same check to README.md:64, which currently claims "does not delete its issues/PRs". **Resolved:** reworded `promptGuidelines` (and the tool `description`, the `deleteMilestone` doc comment, and README.md:64) to "does not delete its issues/PRs, but removes their milestone assignment"; extended the README smoke test step 8 to create an issue assigned to the milestone and verify the issue survives with its milestone cleared. ## Low - [x] extensions/forgejo/src/index.ts:1483 — The `id` parameter description is `"Milestone id"`, while the sibling milestone tools all use `"Milestone id (from forgejo_list_milestones)"` (see lines 1308, 1377, 1432, 1457). The hint helps the agent run `forgejo_list_milestones` first for id lookup and is part of the established convention. Suggested fix: change to `"Milestone id (from forgejo_list_milestones)"`. **Resolved:** changed the `id` parameter description to match the sibling milestone tools.

david added 2 commits

2026-08-18 08:36:23 +00:00