246: Batch --scan support for GitLab (GitLabIssueClient.listIssues + per-platform scan wiring) #295

david · 2026-08-19T04:48:12Z

david commented

2026-08-19 04:48:12 +00:00

246: Batch --scan support for GitLab (GitLabIssueClient.listIssues + per-platform scan wiring)

Implements 246.

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 13m 32s.

Review findings

Now let's fix both docs, and address the Low finding (unused client variable).Now the Low finding in the test file — remove the unused client local.Malformed JSON from my typo. Let me redo correctly.Now let's leave the Low duplication finding unresolved (out of scope, too risky for single pass). Run the gates.Build passes. Now lint:Lint clean. Now tests:All 144 test files / 2070 tests pass, build and lint are clean.All fixes applied and gates pass. Both Medium findings resolved; the Low duplication finding is deliberately left as a future-refactor note (out of scope for a single pass); the Low void client finding was fixed.

Review Findings — 246

Summary

Fixed both Medium documentation-staleness findings: updated README.md and docs/design-batch.md to say "Forgejo and GitLab" instead of "Forgejo only", matching the AGENTS.md wording already updated in the implementation. Also fixed the Low finding about the unused client local in the new test file by removing the dead instance and its void no-op. Left the remaining Low finding (duplicated extractPriority/pagination logic between Forgejo and GitLab clients) unresolved — it's an intentional, documented convention per the implementation plan and extracting a shared helper is a larger refactor better done when a third platform needs it, not a single-pass fix. Re-ran build (tsc), lint (eslint), and test (144 files / 2070 tests) — all green with no regressions.

Critical

(none)

High

(none)

Medium

README.md:375 — Scan mode is documented as "(Forgejo only)" but this change adds GitLab scan support and updates the equivalent row in AGENTS.md to "Forgejo and GitLab", leaving README.md stale/contradictory (in-scope for docs sync: README.md is a default docs-stage path, not under docs/reference/**). Suggested fix: update the sentence to "(Forgejo and GitLab)" to match AGENTS.md.
docs/design-batch.md:73 — "Scan mode discovers issues from the origin platform (currently Forgejo only):" is now inaccurate now that GitLabIssueClient implements IssueListClient. Suggested fix: update to "(Forgejo and GitLab)" or drop the platform-specific parenthetical and point to the per-client implementations.

Low

src/gitlab/services/gitLabIssueClient.ts:191-231 and src/forgejo/services/forgejoIssueClient.ts (analogous section) — extractPriority, PRIORITY_RANKING, and the pagination-loop/termination logic are duplicated verbatim between the Forgejo and GitLab clients (pre-existing pattern, intentionally mirrored per the implementation plan). Suggested fix: consider extracting a shared src/shared/issueList/ helper (e.g. extractPriorityFromLabels) the next time a third platform needs IssueListClient, to avoid a third copy of the same logic.
src/gitlab/services/gitLabIssueClient.listIssues.test.ts:209 — void client; is used only to silence an unused-variable warning for a throwaway client instance created solely to exercise httpFetch construction; the client itself is never used for an assertion. Suggested fix: drop the unused client local entirely (the openClient/closedClient/allClient instances already cover the assertions) rather than keeping it around with a void no-op.

## 246: Batch --scan support for GitLab (GitLabIssueClient.listIssues + per-platform scan wiring) Implements [246](https://git.excelera.net/david/pi-loop/issues/246). > 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 13m 32s. ## Review findings Now let's fix both docs, and address the Low finding (unused `client` variable).Now the Low finding in the test file — remove the unused `client` local.Malformed JSON from my typo. Let me redo correctly.Now let's leave the Low duplication finding unresolved (out of scope, too risky for single pass). Run the gates.Build passes. Now lint:Lint clean. Now tests:All 144 test files / 2070 tests pass, build and lint are clean.All fixes applied and gates pass. Both Medium findings resolved; the Low duplication finding is deliberately left as a future-refactor note (out of scope for a single pass); the Low `void client` finding was fixed. # Review Findings — 246 ## Summary Fixed both Medium documentation-staleness findings: updated README.md and docs/design-batch.md to say "Forgejo and GitLab" instead of "Forgejo only", matching the AGENTS.md wording already updated in the implementation. Also fixed the Low finding about the unused `client` local in the new test file by removing the dead instance and its `void` no-op. Left the remaining Low finding (duplicated `extractPriority`/pagination logic between Forgejo and GitLab clients) unresolved — it's an intentional, documented convention per the implementation plan and extracting a shared helper is a larger refactor better done when a third platform needs it, not a single-pass fix. Re-ran build (tsc), lint (eslint), and test (144 files / 2070 tests) — all green with no regressions. ## Critical *(none)* ## High *(none)* ## Medium - [x] README.md:375 — Scan mode is documented as "(Forgejo only)" but this change adds GitLab scan support and updates the equivalent row in `AGENTS.md` to "Forgejo and GitLab", leaving README.md stale/contradictory (in-scope for docs sync: `README.md` is a default docs-stage path, not under `docs/reference/**`). Suggested fix: update the sentence to "(Forgejo and GitLab)" to match `AGENTS.md`. - [x] docs/design-batch.md:73 — "Scan mode discovers issues from the origin platform (currently Forgejo only):" is now inaccurate now that `GitLabIssueClient` implements `IssueListClient`. Suggested fix: update to "(Forgejo and GitLab)" or drop the platform-specific parenthetical and point to the per-client implementations. ## Low - [ ] src/gitlab/services/gitLabIssueClient.ts:191-231 and src/forgejo/services/forgejoIssueClient.ts (analogous section) — `extractPriority`, `PRIORITY_RANKING`, and the pagination-loop/termination logic are duplicated verbatim between the Forgejo and GitLab clients (pre-existing pattern, intentionally mirrored per the implementation plan). Suggested fix: consider extracting a shared `src/shared/issueList/` helper (e.g. `extractPriorityFromLabels`) the next time a third platform needs `IssueListClient`, to avoid a third copy of the same logic. - [x] src/gitlab/services/gitLabIssueClient.listIssues.test.ts:209 — `void client;` is used only to silence an unused-variable warning for a throwaway client instance created solely to exercise `httpFetch` construction; the client itself is never used for an assertion. Suggested fix: drop the unused `client` local entirely (the `openClient`/`closedClient`/`allClient` instances already cover the assertions) rather than keeping it around with a `void` no-op.

david added 2 commits

2026-08-19 04:48:12 +00:00

246: Batch --scan support for GitLab (GitLabIssueClient.listIssues + per-platform scan wiring) c41893a077