Bug: forgejo_issue_create/view/edit/close/etc. crash when assignees is null in the API response #29

Closed
opened 2026-08-18 03:02:22 +00:00 by david · 1 comment
Owner

Summary

forgejo_issue_create, forgejo_issue_view, forgejo_issue_edit, forgejo_issue_close, forgejo_issue_reopen, forgejo_issue_assign, forgejo_issue_unassign, forgejo_issue_add_labels, and the equivalent PR tools all throw Cannot read properties of null (reading 'length') whenever the Forgejo API returns assignees: null for the issue/PR being formatted — which is the normal, valid response shape for any issue/PR with no assignees. The underlying API call succeeds; only the tool's response-formatting step crashes, which still surfaces as a hard tool failure to the caller.

Background

This was discovered while using forgejo_issue_create to file tracking issues in this repo: every call to create an issue with no assignees (the common case) returned the error Cannot read properties of null (reading 'length') even though the issue was actually created successfully server-side (confirmed via the raw REST API afterwards). This caused at least one accidental duplicate issue before the pattern was noticed and worked around by calling the raw Forgejo REST API directly instead of the extension's tools.

Root cause (extensions/forgejo/):

  • Forgejo's REST API legitimately returns "assignees": null (not []) when an issue/PR has no assignees — this is normal API behavior, not a Forgejo server bug.
  • src/issues.ts's ForgejoIssue interface (and src/pulls.ts's ForgejoPullRequest interface) type assignees as ForgejoAssignee[] — never null/undefined.
  • forgejoApiCall<T>() in src/api.ts does JSON.parse(rawText) as T — a compile-time-only type assertion with no runtime validation. So issue.assignees can be null at runtime despite TypeScript believing it's always an array.
  • formatIssue() and formatPullRequest() in src/index.ts then call issue.assignees.length / pr.assignees.length unconditionally (and .map(...) on the same field for the details object), which throws when the value is null.

This affects every tool that formats an issue or PR response whenever that issue/PR currently has no assignees — i.e. most issues, since assigning is opt-in.

Implementation Details

In extensions/forgejo/src/index.ts:

  1. formatIssue() (around the issue.assignees.length > 0 check and the assignees: issue.assignees.map(...) details field): guard both usages, e.g. (issue.assignees ?? []).length > 0 and (issue.assignees ?? []).map((assignee) => assignee.login).
  2. formatPullRequest() (the equivalent pr.assignees.length > 0 check and assignees: pr.assignees.map(...) details field): apply the same guard.
  3. Consider (optional, for type honesty) widening ForgejoIssue.assignees and ForgejoPullRequest.assignees in src/issues.ts / src/pulls.ts to ForgejoAssignee[] | null, so the type system reflects what the API can actually return, rather than relying solely on defensive formatting code. If this widening is done, downstream call sites that assume a non-null array (e.g. assignIssue's/unassignIssue's callers, other formatters) should be re-checked for the same null-safety gap.
  4. Add regression tests in tests/index.test.ts (or wherever tool-level formatting is tested) that mock a Forgejo API response with assignees: null (and separately labels: null, if that field has the same risk — check formatIssue's issue.labels.length usage too) and assert the tool call succeeds instead of throwing.

Acceptance Criteria

  • forgejo_issue_create/_view/_edit/_close/_reopen/_assign/_unassign/_add_labels/_remove_labels all succeed (no crash) when the Forgejo API response has assignees: null.
  • forgejo_pr_create/_view/_edit/_close/_assign all succeed (no crash) under the same condition.
  • The same null-safety check is applied to labels in formatIssue/formatPullRequest if that field can also be null in practice (verify against the API, don't assume).
  • A regression test exists that reproduces the original crash (mocked assignees: null response) and confirms it now passes.
  • npm test and npm run check pass with no regressions.

Test Plan

  1. Add a unit test that mocks forgejoApiCall to return an issue/PR payload with assignees: null, call the corresponding tool's execute(), and assert it returns a successful ToolResult (not a thrown exception).
  2. Run npm test — confirm the new regression test passes and the full suite remains green.
  3. Manual smoke test: call forgejo_issue_create with no assignees on a real repo and confirm it succeeds and returns the created issue's number/URL without error.
## Summary `forgejo_issue_create`, `forgejo_issue_view`, `forgejo_issue_edit`, `forgejo_issue_close`, `forgejo_issue_reopen`, `forgejo_issue_assign`, `forgejo_issue_unassign`, `forgejo_issue_add_labels`, and the equivalent PR tools all throw `Cannot read properties of null (reading 'length')` whenever the Forgejo API returns `assignees: null` for the issue/PR being formatted — which is the normal, valid response shape for any issue/PR with no assignees. The underlying API call succeeds; only the tool's response-formatting step crashes, which still surfaces as a hard tool failure to the caller. ## Background This was discovered while using `forgejo_issue_create` to file tracking issues in this repo: every call to create an issue with no assignees (the common case) returned the error `Cannot read properties of null (reading 'length')` even though the issue was actually created successfully server-side (confirmed via the raw REST API afterwards). This caused at least one accidental duplicate issue before the pattern was noticed and worked around by calling the raw Forgejo REST API directly instead of the extension's tools. **Root cause** (`extensions/forgejo/`): - Forgejo's REST API legitimately returns `"assignees": null` (not `[]`) when an issue/PR has no assignees — this is normal API behavior, not a Forgejo server bug. - `src/issues.ts`'s `ForgejoIssue` interface (and `src/pulls.ts`'s `ForgejoPullRequest` interface) type `assignees` as `ForgejoAssignee[]` — never `null`/`undefined`. - `forgejoApiCall<T>()` in `src/api.ts` does `JSON.parse(rawText) as T` — a compile-time-only type assertion with no runtime validation. So `issue.assignees` can be `null` at runtime despite TypeScript believing it's always an array. - `formatIssue()` and `formatPullRequest()` in `src/index.ts` then call `issue.assignees.length` / `pr.assignees.length` unconditionally (and `.map(...)` on the same field for the `details` object), which throws when the value is `null`. This affects every tool that formats an issue or PR response whenever that issue/PR currently has no assignees — i.e. most issues, since assigning is opt-in. ## Implementation Details In `extensions/forgejo/src/index.ts`: 1. `formatIssue()` (around the `issue.assignees.length > 0` check and the `assignees: issue.assignees.map(...)` details field): guard both usages, e.g. `(issue.assignees ?? []).length > 0` and `(issue.assignees ?? []).map((assignee) => assignee.login)`. 2. `formatPullRequest()` (the equivalent `pr.assignees.length > 0` check and `assignees: pr.assignees.map(...)` details field): apply the same guard. 3. Consider (optional, for type honesty) widening `ForgejoIssue.assignees` and `ForgejoPullRequest.assignees` in `src/issues.ts` / `src/pulls.ts` to `ForgejoAssignee[] | null`, so the type system reflects what the API can actually return, rather than relying solely on defensive formatting code. If this widening is done, downstream call sites that assume a non-null array (e.g. `assignIssue`'s/`unassignIssue`'s callers, other formatters) should be re-checked for the same null-safety gap. 4. Add regression tests in `tests/index.test.ts` (or wherever tool-level formatting is tested) that mock a Forgejo API response with `assignees: null` (and separately `labels: null`, if that field has the same risk — check `formatIssue`'s `issue.labels.length` usage too) and assert the tool call succeeds instead of throwing. ## Acceptance Criteria - [ ] `forgejo_issue_create`/`_view`/`_edit`/`_close`/`_reopen`/`_assign`/`_unassign`/`_add_labels`/`_remove_labels` all succeed (no crash) when the Forgejo API response has `assignees: null`. - [ ] `forgejo_pr_create`/`_view`/`_edit`/`_close`/`_assign` all succeed (no crash) under the same condition. - [ ] The same null-safety check is applied to `labels` in `formatIssue`/`formatPullRequest` if that field can also be `null` in practice (verify against the API, don't assume). - [ ] A regression test exists that reproduces the original crash (mocked `assignees: null` response) and confirms it now passes. - [ ] `npm test` and `npm run check` pass with no regressions. ## Test Plan 1. Add a unit test that mocks `forgejoApiCall` to return an issue/PR payload with `assignees: null`, call the corresponding tool's `execute()`, and assert it returns a successful `ToolResult` (not a thrown exception). 2. Run `npm test` — confirm the new regression test passes and the full suite remains green. 3. Manual smoke test: call `forgejo_issue_create` with no `assignees` on a real repo and confirm it succeeds and returns the created issue's number/URL without error.
david closed this issue 2026-08-18 07:29:58 +00:00
Author
Owner

pi-loop opened and merged a pull request for this issue: #32

pi-loop opened and merged a pull request for this issue: https://git.excelera.net/david/pi-extensions-and-skills/pulls/32
Sign in to join this conversation.
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#29
No description provided.