update_issue cannot clear a milestone, assignees, or body #4

New issue

Closed

opened 2026-07-29 05:59:14 +00:00 by trtmn · 0 comments

trtmn commented

2026-07-29 05:59:14 +00:00

Owner

Problem

update_issue in src/tools/issues.js can only set values, never unset them.

Milestone: the resolve step is gated on truthiness, so an empty string is treated as "not provided":

let milestoneId;
if (milestone) {
  const resolved = await resolveMilestoneId(owner, repo, milestone);
  ...
}

milestoneId therefore stays undefined and is dropped from the JSON body. There is no input that produces milestone: 0, which is what the Forgejo API expects in order to detach a milestone.

Assignees: assignees: z.array(z.string()).optional(). Passing [] does reach the API, so that path may work, but it is untested and undocumented. Worth confirming with a real call and adding a test.
Same shape of gap for clearing the body text - an empty string is falsy in the resolve helpers but does pass through here, so behaviour differs field to field and nothing pins it down.

Suggested fix

Accept an explicit sentinel for milestone detachment (for example milestone: "" meaning detach) and map it to milestone: 0 in the request body.
Distinguish "argument absent" from "argument empty" using Object.hasOwn(args, "milestone") rather than truthiness.
Add integration tests covering: set milestone, then detach it; set assignees, then clear them.

Severity

Low - missing capability rather than incorrect behaviour, but it fails silently (the call returns HTTP 200 and the milestone is simply still attached), which is the annoying kind.

## Problem `update_issue` in `src/tools/issues.js` can only set values, never unset them. 1. Milestone: the resolve step is gated on truthiness, so an empty string is treated as "not provided": ```js let milestoneId; if (milestone) { const resolved = await resolveMilestoneId(owner, repo, milestone); ... } ``` `milestoneId` therefore stays `undefined` and is dropped from the JSON body. There is no input that produces `milestone: 0`, which is what the Forgejo API expects in order to detach a milestone. 2. Assignees: `assignees: z.array(z.string()).optional()`. Passing `[]` does reach the API, so that path may work, but it is untested and undocumented. Worth confirming with a real call and adding a test. 3. Same shape of gap for clearing the body text - an empty string is falsy in the resolve helpers but does pass through here, so behaviour differs field to field and nothing pins it down. ## Suggested fix - Accept an explicit sentinel for milestone detachment (for example `milestone: ""` meaning detach) and map it to `milestone: 0` in the request body. - Distinguish "argument absent" from "argument empty" using `Object.hasOwn(args, "milestone")` rather than truthiness. - Add integration tests covering: set milestone, then detach it; set assignees, then clear them. ## Severity Low - missing capability rather than incorrect behaviour, but it fails silently (the call returns HTTP 200 and the milestone is simply still attached), which is the annoying kind.

trtmn added the

enhancement

tests

labels

2026-07-29 06:02:02 +00:00

trtmn referenced this issue from a commit

2026-07-29 13:55:57 +00:00

Fix code-review findings: path encoding, string bodies, timeouts

trtmn referenced this issue

2026-07-29 13:56:39 +00:00

Fix code-review findings: path encoding, string bodies, timeouts #6

trtmn