issue-114: src/actions.ts — metadata queries (listActionRuns, getActionRun, listActionJobs) #126

Merged

david merged 2 commits from feature/issue-114/src-actions-ts-metadata-queries-listactionruns-get into main

2026-08-29 07:34:39 +00:00

david commented

2026-08-29 07:34:38 +00:00

Owner

114: src/actions.ts — metadata queries (listActionRuns, getActionRun, listActionJobs)

Implements 114.

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 7m 35s.

Review findings

I've confirmed the current state. The NETWORK_FAILURE branch in forgejoApiCall returns an ApiError with no statusCode, so the proposed assertions are valid. Now I'll apply the fixes for the four actionable findings (reword test description, add statusCode assertions to network-failure tests, add the missing total_count fallback test, add 403 tests for getActionRun/listActionJobs).

The Medium finding (commit/MR title) cannot be fixed from the working tree — the commit already exists and I'm forbidden from amending/committing — so it will remain unresolved.The overlap detector flagged edits 2 and 5. Let me apply them in separate calls to be safe.The overlap detector is being conservative. Let me split the edits into smaller sequential calls.Now let me add the remaining tests. First, the total_count default test and the 403 tests:Now let me verify the edits and run the gates. First, a quick sanity check of the modified regions:The diff looks correct. Now run the gates discovered from extensions/forgejo/package.json (build, check, test, test:coverage):251 tests pass (was 248 — the 3 new tests added). Now the coverage gate:All gates pass. Coverage confirms actions.ts is now at 100% lines — line 250 (the total_count ?? 0 fallback) is no longer in the uncovered list. Let me confirm the working tree state:All gates pass and only the test file is modified. Here is the annotated findings document.

Review Findings — 114

Summary

Remediation pass applied. Fixed in a single pass:

Low 1 — rewrote the listActionRuns 404 test description from "(unknown repo or run)" to "(unknown repo)".
Low 2 — added expect(result.error.statusCode).toBeUndefined() to all three network-failure tests (listActionRuns, getActionRun, listActionJobs), matching the NETWORK_FAILURE branch in forgejoApiCall which returns an ApiError without statusCode.
Low 3 — added a listActionRuns case with jsonBody: { workflow_runs: [] } (no total_count) asserting total_count === 0; coverage now shows actions.ts at 100% lines (line 250 no longer uncovered).
Low 4 — added 403 ("read scope missing") tests for getActionRun and listActionJobs mirroring the listActionRuns one.

Remains unresolved:

Medium — the commit/MR title misrepresents the test-only diff. This cannot be fixed from the working tree: the commit 7591666 already exists and this stage must not commit/amend/push, so the title must be corrected when the pipeline opens the MR (retitle to 114: tests/actions.test.ts — error-path coverage for metadata queries (404 + NETWORK_FAILURE)).

Gates re-run after the changes (commands from extensions/forgejo/package.json): npm run build (tsc) — PASS; npm run check (tsc --noEmit) — PASS; npm test (jest, 11 suites / 251 tests, up from 248) — PASS; npm run test:coverage — PASS (actions.ts 100% lines / 95.29% statements).

Critical

(no findings)

High

(no findings)

Medium

extensions/forgejo/tests/actions.test.ts:213 — Commit/MR title misrepresents the change: 114: src/actions.ts — metadata queries (listActionRuns, getActionRun, listActionJobs) implies a source implementation, but the diff is test-only (the three functions already exist on main, added by the issue-111 tracking commit bc1d24c). A squash-merged PR with this title would mislead reviewers about what issue 114 delivers. Suggested fix: retitle the commit and PR to reflect the actual change, e.g. 114: tests/actions.test.ts — error-path coverage for metadata queries (404 + NETWORK_FAILURE).

Low

extensions/forgejo/tests/actions.test.ts:213 — Test description "(unknown repo or run)" is inaccurate for the collection endpoint: listActionRuns hits /repos/{owner}/{repo}/actions/runs, so a 404 there means the repo (or path) is unknown; "run not found" applies to getActionRun/listActionJobs. Suggested fix: reword to "(unknown repo)".
extensions/forgejo/tests/actions.test.ts:230 (also 289, 352) — The network-failure tests assert only error.code. The NETWORK_FAILURE branch in forgejoApiCall returns an ApiError with no statusCode; pinning expect(result.error.statusCode).toBeUndefined() would guard against a regression that attaches a bogus statusCode to network errors. Suggested fix: add the statusCode assertion to all three tests.
extensions/forgejo/src/actions.ts:250 — The result.value.total_count ?? 0 fallback is untested: the existing "handles a missing workflow_runs array as empty" test supplies total_count: 0 explicitly, so the branch where the server omits total_count is uncovered (confirmed by the coverage output listing line 250 as uncovered). Suggested fix: add a listActionRuns case with jsonBody: { workflow_runs: [] } (no total_count) asserting total_count === 0.
extensions/forgejo/tests/actions.test.ts:289 — Asymmetric 403 coverage: listActionRuns has a 403 ("read scope missing") test, but getActionRun and listActionJobs only cover 200/404/network. A 403 is a realistic failure for these read-only tools. Suggested fix: add 403 tests for getActionRun and listActionJobs mirroring the listActionRuns one.

## 114: src/actions.ts — metadata queries (listActionRuns, getActionRun, listActionJobs) Implements [114](https://git.excelera.net/david/pi-extensions-and-skills/issues/114). > 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 7m 35s. ## Review findings I've confirmed the current state. The `NETWORK_FAILURE` branch in `forgejoApiCall` returns an `ApiError` with no `statusCode`, so the proposed assertions are valid. Now I'll apply the fixes for the four actionable findings (reword test description, add `statusCode` assertions to network-failure tests, add the missing `total_count` fallback test, add 403 tests for `getActionRun`/`listActionJobs`). The Medium finding (commit/MR title) cannot be fixed from the working tree — the commit already exists and I'm forbidden from amending/committing — so it will remain unresolved.The overlap detector flagged edits 2 and 5. Let me apply them in separate calls to be safe.The overlap detector is being conservative. Let me split the edits into smaller sequential calls.Now let me add the remaining tests. First, the `total_count` default test and the 403 tests:Now let me verify the edits and run the gates. First, a quick sanity check of the modified regions:The diff looks correct. Now run the gates discovered from `extensions/forgejo/package.json` (`build`, `check`, `test`, `test:coverage`):251 tests pass (was 248 — the 3 new tests added). Now the coverage gate:All gates pass. Coverage confirms `actions.ts` is now at 100% lines — line 250 (the `total_count ?? 0` fallback) is no longer in the uncovered list. Let me confirm the working tree state:All gates pass and only the test file is modified. Here is the annotated findings document. # Review Findings — 114 ## Summary Remediation pass applied. Fixed in a single pass: - **Low 1** — rewrote the `listActionRuns` 404 test description from "(unknown repo or run)" to "(unknown repo)". - **Low 2** — added `expect(result.error.statusCode).toBeUndefined()` to all three network-failure tests (`listActionRuns`, `getActionRun`, `listActionJobs`), matching the `NETWORK_FAILURE` branch in `forgejoApiCall` which returns an `ApiError` without `statusCode`. - **Low 3** — added a `listActionRuns` case with `jsonBody: { workflow_runs: [] }` (no `total_count`) asserting `total_count === 0`; coverage now shows `actions.ts` at 100% lines (line 250 no longer uncovered). - **Low 4** — added 403 ("read scope missing") tests for `getActionRun` and `listActionJobs` mirroring the `listActionRuns` one. Remains unresolved: - **Medium** — the commit/MR title misrepresents the test-only diff. This cannot be fixed from the working tree: the commit `7591666` already exists and this stage must not commit/amend/push, so the title must be corrected when the pipeline opens the MR (retitle to `114: tests/actions.test.ts — error-path coverage for metadata queries (404 + NETWORK_FAILURE)`). Gates re-run after the changes (commands from `extensions/forgejo/package.json`): `npm run build` (tsc) — PASS; `npm run check` (tsc --noEmit) — PASS; `npm test` (jest, 11 suites / 251 tests, up from 248) — PASS; `npm run test:coverage` — PASS (actions.ts 100% lines / 95.29% statements). ## Critical (no findings) ## High (no findings) ## Medium - [ ] extensions/forgejo/tests/actions.test.ts:213 — Commit/MR title misrepresents the change: `114: src/actions.ts — metadata queries (listActionRuns, getActionRun, listActionJobs)` implies a source implementation, but the diff is test-only (the three functions already exist on main, added by the issue-111 tracking commit `bc1d24c`). A squash-merged PR with this title would mislead reviewers about what issue 114 delivers. Suggested fix: retitle the commit and PR to reflect the actual change, e.g. `114: tests/actions.test.ts — error-path coverage for metadata queries (404 + NETWORK_FAILURE)`. ## Low - [x] extensions/forgejo/tests/actions.test.ts:213 — Test description "(unknown repo or run)" is inaccurate for the collection endpoint: `listActionRuns` hits `/repos/{owner}/{repo}/actions/runs`, so a 404 there means the repo (or path) is unknown; "run not found" applies to `getActionRun`/`listActionJobs`. Suggested fix: reword to "(unknown repo)". - [x] extensions/forgejo/tests/actions.test.ts:230 (also 289, 352) — The network-failure tests assert only `error.code`. The `NETWORK_FAILURE` branch in `forgejoApiCall` returns an `ApiError` with no `statusCode`; pinning `expect(result.error.statusCode).toBeUndefined()` would guard against a regression that attaches a bogus statusCode to network errors. Suggested fix: add the `statusCode` assertion to all three tests. - [x] extensions/forgejo/src/actions.ts:250 — The `result.value.total_count ?? 0` fallback is untested: the existing "handles a missing workflow_runs array as empty" test supplies `total_count: 0` explicitly, so the branch where the server omits `total_count` is uncovered (confirmed by the coverage output listing line 250 as uncovered). Suggested fix: add a `listActionRuns` case with `jsonBody: { workflow_runs: [] }` (no `total_count`) asserting `total_count === 0`. - [x] extensions/forgejo/tests/actions.test.ts:289 — Asymmetric 403 coverage: `listActionRuns` has a 403 ("read scope missing") test, but `getActionRun` and `listActionJobs` only cover 200/404/network. A 403 is a realistic failure for these read-only tools. Suggested fix: add 403 tests for `getActionRun` and `listActionJobs` mirroring the `listActionRuns` one.

david added 2 commits

2026-08-29 07:34:38 +00:00