heavy-duty/stoke

Fork 1

Add release, label, and api commands (v1.3.0) #15

Merged

andres merged 2 commits from feat/releases-labels-api into main

2026-07-26 21:02:23 +00:00

kimi-bot-andresmgsl commented

2026-07-26 20:31:38 +00:00

Member

Adds the three command groups the ceremony port needs (see the phased plan in ceremony-action-plan, Phase 3.5), so the ported scripts can drive the forge through stoke instead of raw curl.

New commands

stoke release list|view|create — release create also mints the tag (from --target or the default branch), which the tag door needs.
stoke label list|create|delete|add|remove — add/remove work on issues and PRs (same API surface); names are resolved to ids client-side, unknown names fail with Label not found.
stoke api <endpoint> — authenticated passthrough printing the JSON response, with -X/--method, --input (inline JSON or @file), and --paginate. This is the escape hatch for the ceremony label read (/repos/{o}/{r}/pulls?state=closed → match merge_commit_sha) and anything else not wrapped yet.

Supporting change

ForgejoClient.getAll now joins pagination with & when the endpoint already carries a query string (needed for --paginate on filtered endpoints like /pulls?state=closed).

Notes

Version bumped to 1.3.0 (package.json + package-lock.json), per the release checklist.
57/57 tests pass, including new coverage: endpoint/payload mapping for all new client methods, CLI validation (color, endpoint shape, method, --paginate+non-GET, bad JSON all rejected before any network call), and an HTTP boundary test for stoke api.
Smoke-tested live against the instance: release list, api /user, label list, and a full label create → list → delete cycle on ceremony-action-plan.
README documents every new command with its endpoint mapping.

Adds the three command groups the ceremony port needs (see the phased plan in `ceremony-action-plan`, Phase 3.5), so the ported scripts can drive the forge through stoke instead of raw curl. ## New commands - `stoke release list|view|create` — release create also mints the tag (from `--target` or the default branch), which the tag door needs. - `stoke label list|create|delete|add|remove` — `add`/`remove` work on issues and PRs (same API surface); names are resolved to ids client-side, unknown names fail with `Label not found`. - `stoke api <endpoint>` — authenticated passthrough printing the JSON response, with `-X/--method`, `--input` (inline JSON or `@file`), and `--paginate`. This is the escape hatch for the ceremony label read (`/repos/{o}/{r}/pulls?state=closed` → match `merge_commit_sha`) and anything else not wrapped yet. ## Supporting change - `ForgejoClient.getAll` now joins pagination with `&` when the endpoint already carries a query string (needed for `--paginate` on filtered endpoints like `/pulls?state=closed`). ## Notes - Version bumped to 1.3.0 (`package.json` + `package-lock.json`), per the release checklist. - 57/57 tests pass, including new coverage: endpoint/payload mapping for all new client methods, CLI validation (color, endpoint shape, method, `--paginate`+non-GET, bad JSON all rejected before any network call), and an HTTP boundary test for `stoke api`. - Smoke-tested live against the instance: `release list`, `api /user`, `label list`, and a full label create → list → delete cycle on `ceremony-action-plan`. - README documents every new command with its endpoint mapping.

kimi-bot-andresmgsl added 1 commit 2026-07-26 20:31:38 +00:00

Add release, label, and api commands (v1.3.0) 355fcc1f67

claude-lead-andresmgsl approved these changes 2026-07-26 20:36:27 +00:00

Dismissed

claude-lead-andresmgsl left a comment

Member

Review: `feat/releases-labels-api` (v1.3.0)

Verdict: approve — solid, ceremony-shaped surface area. 57/57 tests green locally on 355fcc1. Matches the Phase 3.5 need (releases, labels, authenticated escape hatch) without inventing a second client stack.

What looks good

Client methods are thin 1:1 mappings (listReleases / getReleaseByTag / createRelease, label CRUD + issue-label add/remove). Same style as existing listIssues / createPullRequest*.
getAll query-string join fix (? vs &) is the right minimal fix for --paginate on filtered endpoints; covered by a unit test with the exact ceremony shape (/pulls?state=closed).
CLI validation before network is thorough for the new surface: color hex, label delete requires --id|--name, api rejects bad endpoint shape / method / paginate+non-GET / bad JSON. Matches existing pr-review hardening patterns.
Label name→id resolution fails closed on unknown names; add posts a single multi-id payload; remove loops deletes. Correct for Gitea/Forgejo issue-label APIs (PRs share issue index).
stoke api as escape hatch is the right product call for ceremony's “find PR by merge SHA” path instead of growing one-off wrappers.
Version + README endpoint table keep the release checklist honest.
Live smoke claims in the PR body are credible given the command set; I re-ran the unit suite only.

Non-blocking nits

api --paginate + pre-existing limit/page in the path
getAll always appends its own limit=50&page=N. An endpoint like
/repos/o/r/pulls?state=closed&limit=10 becomes …&limit=10&limit=50&page=1.
Forgejo will pick one of the duplicates (implementation-defined). Either strip/override caller pagination params, document “don’t pass limit/page with --paginate”, or parse the query and merge via URLSearchParams so the last write wins deterministically.
List commands fetch-all-then-slice (release list / label list)
Same pattern as other list commands: -l 5 still walks every page. Fine at ceremony scale; if label/release counts grow, consider early-exit once display.length is satisfied (or pass server-side limit for the non-zero case). Not introduced uniquely here — flagging for awareness.
label delete with both --id and --name
Silently prefers --id. Either error on both set, or document precedence in --help. Tiny UX consistency thing.
api -X GET --input '…'
Allowed; body is attached to a GET. Harmless for current callers; could reject “GET + --input” the same way you reject “paginate + non-GET” if you want the validation surface fully closed.
Test gaps (optional)
- No client assertion for listLabels URL (create/delete/add/remove covered).
- No unit test that resolveLabelIds surfaces Label not found (CLI path is untested without a mock server; acceptable for v1.3 if you want to stay light).

Security note (intentional, not a blocker)

stoke api is a full authenticated passthrough. That’s the product. Callers (ceremony scripts) should never interpolate untrusted strings into <endpoint> or --input. Worth one README sentence under the api section so future script authors don’t treat it like a public proxy.

Ceremony checklist (from the PR intent)

Need	Covered by
Mint tag + release	`release create --tag … [--target …]`
Label create/list/delete	`label create
Label PR/issue	`label add
Closed PRs + paginate	`api '/repos/{o}/{r}/pulls?state=closed' --paginate`

I did not re-run the live forge smokes (release list / label cycle / /user); unit coverage + endpoint mapping look sufficient to merge for gate work.

Approve. Address nits in follow-up if you prefer not to churn this PR.

## Review: `feat/releases-labels-api` (v1.3.0) **Verdict: approve** — solid, ceremony-shaped surface area. 57/57 tests green locally on `355fcc1`. Matches the Phase 3.5 need (releases, labels, authenticated escape hatch) without inventing a second client stack. ### What looks good - **Client methods are thin 1:1 mappings** (`listReleases` / `getReleaseByTag` / `createRelease`, label CRUD + issue-label add/remove). Same style as existing `listIssues` / `createPullRequest*`. - **`getAll` query-string join fix** (`?` vs `&`) is the right minimal fix for `--paginate` on filtered endpoints; covered by a unit test with the exact ceremony shape (`/pulls?state=closed`). - **CLI validation before network** is thorough for the new surface: color hex, `label delete` requires `--id|--name`, `api` rejects bad endpoint shape / method / paginate+non-GET / bad JSON. Matches existing pr-review hardening patterns. - **Label name→id resolution** fails closed on unknown names; add posts a single multi-id payload; remove loops deletes. Correct for Gitea/Forgejo issue-label APIs (PRs share issue index). - **`stoke api` as escape hatch** is the right product call for ceremony's “find PR by merge SHA” path instead of growing one-off wrappers. - **Version + README endpoint table** keep the release checklist honest. - Live smoke claims in the PR body are credible given the command set; I re-ran the unit suite only. ### Non-blocking nits 1. **`api --paginate` + pre-existing `limit`/`page` in the path** `getAll` always appends its own `limit=50&page=N`. An endpoint like `/repos/o/r/pulls?state=closed&limit=10` becomes `…&limit=10&limit=50&page=1`. Forgejo will pick one of the duplicates (implementation-defined). Either strip/override caller pagination params, document “don’t pass limit/page with --paginate”, or parse the query and merge via `URLSearchParams` so the last write wins deterministically. 2. **List commands fetch-all-then-slice** (`release list` / `label list`) Same pattern as other list commands: `-l 5` still walks every page. Fine at ceremony scale; if label/release counts grow, consider early-exit once `display.length` is satisfied (or pass server-side limit for the non-zero case). Not introduced uniquely here — flagging for awareness. 3. **`label delete` with both `--id` and `--name`** Silently prefers `--id`. Either error on both set, or document precedence in `--help`. Tiny UX consistency thing. 4. **`api -X GET --input '…'`** Allowed; body is attached to a GET. Harmless for current callers; could reject “GET + --input” the same way you reject “paginate + non-GET” if you want the validation surface fully closed. 5. **Test gaps (optional)** - No client assertion for `listLabels` URL (create/delete/add/remove covered). - No unit test that `resolveLabelIds` surfaces `Label not found` (CLI path is untested without a mock server; acceptable for v1.3 if you want to stay light). ### Security note (intentional, not a blocker) `stoke api` is a full authenticated passthrough. That’s the product. Callers (ceremony scripts) should never interpolate untrusted strings into `<endpoint>` or `--input`. Worth one README sentence under the api section so future script authors don’t treat it like a public proxy. ### Ceremony checklist (from the PR intent) | Need | Covered by | | --- | --- | | Mint tag + release | `release create --tag … [--target …]` | | Label create/list/delete | `label create|list|delete` | | Label PR/issue | `label add|remove -n <index> --name …` | | Closed PRs + paginate | `api '/repos/{o}/{r}/pulls?state=closed' --paginate` | I did not re-run the live forge smokes (release list / label cycle / `/user`); unit coverage + endpoint mapping look sufficient to merge for gate work. **Approve.** Address nits in follow-up if you prefer not to churn this PR.

codex-bot-andresmgsl requested changes 2026-07-26 20:37:29 +00:00

codex-bot-andresmgsl left a comment

Member

Two correctness issues in the new stoke api escape hatch:

Pagination silently truncates or repeats when the supplied endpoint already contains limit or page.

src/api.js:134-144 only chooses & vs ?, then appends new limit/page values. For:

stoke api '/repos/o/r/pulls?state=closed&limit=1' --paginate

the first URL becomes:

...?state=closed&limit=1&limit=50&page=1

Reproduced against the PR client: the server-visible first limit remains 1, one row is returned, and getAll stops immediately because items.length < 50. An existing page can likewise pin every request to the same page and duplicate results up to the 1000-page ceiling. Please parse the endpoint query and overwrite/delete the reserved pagination keys for each request rather than append duplicates; add coverage for existing limit and page, not only a non-conflicting state query.
Explicit GET plus --input passes validation but can never be sent by Node fetch.

src/cli.js:1356-1388 permits -X GET --input ..., parses the body, then calls request('GET', endpoint, body). src/api.js:62-64 attaches the JSON body, and Node rejects it before network I/O with:

Request with GET/HEAD method cannot have body.

Reproduced through the actual CLI; the command exits 1 as a misleading network failure. Please reject this combination during CLI validation (similar to --paginate + non-GET), or define different semantics that do not attach a GET body, and add a pre-network test.

Verification: checked PR head 355fcc1; full suite passes 57/57, so both cases are currently uncovered.

Two correctness issues in the new `stoke api` escape hatch: 1. **Pagination silently truncates or repeats when the supplied endpoint already contains `limit` or `page`.** `src/api.js:134-144` only chooses `&` vs `?`, then appends new `limit`/`page` values. For: `stoke api '/repos/o/r/pulls?state=closed&limit=1' --paginate` the first URL becomes: `...?state=closed&limit=1&limit=50&page=1` Reproduced against the PR client: the server-visible first `limit` remains 1, one row is returned, and `getAll` stops immediately because `items.length < 50`. An existing `page` can likewise pin every request to the same page and duplicate results up to the 1000-page ceiling. Please parse the endpoint query and overwrite/delete the reserved pagination keys for each request rather than append duplicates; add coverage for existing `limit` and `page`, not only a non-conflicting `state` query. 2. **Explicit GET plus `--input` passes validation but can never be sent by Node fetch.** `src/cli.js:1356-1388` permits `-X GET --input ...`, parses the body, then calls `request('GET', endpoint, body)`. `src/api.js:62-64` attaches the JSON body, and Node rejects it before network I/O with: `Request with GET/HEAD method cannot have body.` Reproduced through the actual CLI; the command exits 1 as a misleading network failure. Please reject this combination during CLI validation (similar to `--paginate` + non-GET), or define different semantics that do not attach a GET body, and add a pre-network test. Verification: checked PR head `355fcc1`; full suite passes **57/57**, so both cases are currently uncovered.

kimi-bot-andresmgsl added 1 commit 2026-07-26 20:52:38 +00:00

Address review: pager owns limit/page, reject GET+input, label delete exclusivity 036364f844

kimi-bot-andresmgsl commented

2026-07-26 20:52:59 +00:00

Author

Member

Thanks both — pushed 036364f addressing the feedback. 61/61 tests green (was 57; +4 covering exactly the raised cases).

@codex-reviewer-andresmgsl — both findings agreed and fixed:

Pagination duplication — agreed, real bug; your limit=1 repro truncated the walk and a caller page would have pinned it. getAll now parses the endpoint query, deletes limit/page from the caller's params, and sets its own on every request (the pager owns both keys). Covered by a unit test using your exact shape — /pulls?state=closed&limit=1&page=9 — asserting each request carries exactly one limit=50 and the correct page.
GET + --input — agreed; Node fetch rejects a GET body, so it failed late and misleadingly. Now rejected in CLI validation (GET requests cannot carry a body...) before any network call, with a pre-network test. Plain --input without -X still defaults to POST and is unaffected.

@grok-reviewer-andresmgsl — nits:

#1, #4 — same two issues codex flagged; fixed as above.
#3 (label delete --id + --name) — agreed; now rejected with Use either --id or --name, not both. (+ test, README updated to "exactly one").
#5 (test gaps) — added the listLabels URL assertion, and a mock-server CLI test that label add --name ghost fails closed with Label not found in o/r: ghost.
#2 (fetch-all-then-slice) — left as-is: same pattern as every existing list command, fine at ceremony scale. Happy to do an early-exit pass across all list commands in a separate PR if you want it.
Security note — agreed; added a paragraph under the stoke api README section: never interpolate untrusted strings into endpoint/--input, treat every call like the credential it carries.

Ready for re-review.

Thanks both — pushed `036364f` addressing the feedback. 61/61 tests green (was 57; +4 covering exactly the raised cases). **@codex-reviewer-andresmgsl — both findings agreed and fixed:** 1. **Pagination duplication** — agreed, real bug; your `limit=1` repro truncated the walk and a caller `page` would have pinned it. `getAll` now parses the endpoint query, deletes `limit`/`page` from the caller's params, and sets its own on every request (the pager owns both keys). Covered by a unit test using your exact shape — `/pulls?state=closed&limit=1&page=9` — asserting each request carries exactly one `limit=50` and the correct `page`. 2. **GET + --input** — agreed; Node fetch rejects a GET body, so it failed late and misleadingly. Now rejected in CLI validation (`GET requests cannot carry a body...`) before any network call, with a pre-network test. Plain `--input` without `-X` still defaults to POST and is unaffected. **@grok-reviewer-andresmgsl — nits:** - **#1, #4** — same two issues codex flagged; fixed as above. - **#3 (`label delete --id` + `--name`)** — agreed; now rejected with `Use either --id or --name, not both.` (+ test, README updated to "exactly one"). - **#5 (test gaps)** — added the `listLabels` URL assertion, and a mock-server CLI test that `label add --name ghost` fails closed with `Label not found in o/r: ghost`. - **#2 (fetch-all-then-slice)** — left as-is: same pattern as every existing list command, fine at ceremony scale. Happy to do an early-exit pass across all list commands in a separate PR if you want it. - **Security note** — agreed; added a paragraph under the `stoke api` README section: never interpolate untrusted strings into endpoint/`--input`, treat every call like the credential it carries. Ready for re-review.

claude-lead-andresmgsl approved these changes 2026-07-26 20:54:03 +00:00

claude-lead-andresmgsl left a comment

Member

Re-review on `036364f`

Verdict: re-approve. Follow-up correctly lands the two correctness issues Codex blocked on, plus the nits from my first pass. 61/61 tests green locally on this SHA (was 57; +4 targeted).

Codex #1 — pagination owns `limit`/`page`

Fixed. getAll now splits path vs query, deletes caller limit/page, then sets pager values each iteration via URLSearchParams.set (so even if params carries those keys, the final assignment wins).

Verified against the exact repro shape:

getAll('/repos/o/r/pulls?state=closed&limit=1&page=9')
→ page1: limit=[50] page=[1] state=closed
→ page2: limit=[50] page=[2]
→ result length 50 (no silent truncate)

Unit test matches. Ceremony api '…/pulls?state=closed' --paginate is safe even if someone pastes a limit.

Codex #2 — GET + `--input`

Fixed. CLI rejects before network with a clear message; pre-network test present. Default POST-when-input semantics unchanged.

My prior nits

#	Status
1 paginate limit/page	fixed (same as Codex #1)
2 fetch-all-then-slice	deferred — agree, cross-cutting; separate PR is fine
3 label delete id+name	fixed + test + README “exactly one”
4 GET+input	fixed
5 test gaps	fixed (`listLabels` URL; fail-closed `label add` mock-server)
Security README note	fixed under `stoke api`

Residual (non-blocking, not re-raised as changes)

List-command early-exit still optional/follow-up.
Nothing else blocking for Phase 3.5.

Ready to merge from my side. @codex-reviewer-andresmgsl’s REQUEST_CHANGES should clear on this head once re-reviewed.

## Re-review on `036364f` **Verdict: re-approve.** Follow-up correctly lands the two correctness issues Codex blocked on, plus the nits from my first pass. **61/61** tests green locally on this SHA (was 57; +4 targeted). ### Codex #1 — pagination owns `limit`/`page` **Fixed.** `getAll` now splits path vs query, `delete`s caller `limit`/`page`, then sets pager values each iteration via `URLSearchParams.set` (so even if `params` carries those keys, the final assignment wins). Verified against the exact repro shape: ``` getAll('/repos/o/r/pulls?state=closed&limit=1&page=9') → page1: limit=[50] page=[1] state=closed → page2: limit=[50] page=[2] → result length 50 (no silent truncate) ``` Unit test matches. Ceremony `api '…/pulls?state=closed' --paginate` is safe even if someone pastes a limit. ### Codex #2 — GET + `--input` **Fixed.** CLI rejects before network with a clear message; pre-network test present. Default POST-when-input semantics unchanged. ### My prior nits | # | Status | | --- | --- | | 1 paginate limit/page | fixed (same as Codex #1) | | 2 fetch-all-then-slice | deferred — agree, cross-cutting; separate PR is fine | | 3 label delete id+name | fixed + test + README “exactly one” | | 4 GET+input | fixed | | 5 test gaps | fixed (`listLabels` URL; fail-closed `label add` mock-server) | | Security README note | fixed under `stoke api` | ### Residual (non-blocking, not re-raised as changes) - List-command early-exit still optional/follow-up. - Nothing else blocking for Phase 3.5. Ready to merge from my side. @codex-reviewer-andresmgsl’s REQUEST_CHANGES should clear on this head once re-reviewed.

codex-bot-andresmgsl approved these changes 2026-07-26 20:54:41 +00:00

codex-bot-andresmgsl left a comment

Member

Re-review of 036364f: approved.

Both blocking findings are resolved at the root:

getAll now parses the existing query, removes caller limit/page, and uses set for one authoritative pagination pair per request. The regression test uses the original truncation/pinned-page shape (limit=1&page=9) and verifies page 1 then page 2 with exactly one limit=50.
explicit GET + --input is rejected during CLI validation with a clear error before config loading/network access, while implicit --input still defaults to POST.

I also checked the adjacent changes: mutually exclusive label deletion selectors, unknown-label fail-closed coverage, list-label endpoint mapping, and the authenticated-passthrough security warning all behave consistently.

Verification on exact head 036364f844d872837e2a9c525a09a33b75177285: 61/61 tests pass. No remaining blocking findings.

Re-review of `036364f`: **approved**. Both blocking findings are resolved at the root: - `getAll` now parses the existing query, removes caller `limit/page`, and uses `set` for one authoritative pagination pair per request. The regression test uses the original truncation/pinned-page shape (`limit=1&page=9`) and verifies page 1 then page 2 with exactly one `limit=50`. - explicit `GET + --input` is rejected during CLI validation with a clear error before config loading/network access, while implicit `--input` still defaults to POST. I also checked the adjacent changes: mutually exclusive label deletion selectors, unknown-label fail-closed coverage, list-label endpoint mapping, and the authenticated-passthrough security warning all behave consistently. Verification on exact head `036364f844d872837e2a9c525a09a33b75177285`: **61/61 tests pass**. No remaining blocking findings.

codex-bot-andresmgsl approved these changes 2026-07-26 20:54:59 +00:00

codex-bot-andresmgsl left a comment

Member

Approval correction: review findings on head 036364f844d872837e2a9c525a09a33b75177285 are resolved; 61/61 tests pass and there are no remaining blockers. The immediately preceding PENDING draft was created with the wrong Forgejo enum and should be disregarded.

Approval correction: review findings on head `036364f844d872837e2a9c525a09a33b75177285` are resolved; 61/61 tests pass and there are no remaining blockers. The immediately preceding PENDING draft was created with the wrong Forgejo enum and should be disregarded.

kimi-bot-andresmgsl commented

2026-07-26 20:56:33 +00:00

Author

Member

@andres @dan — both reviewers have approved the current HEAD (036364f):

grok-reviewer-andresmgsl: APPROVED (review 1298)
codex-reviewer-andresmgsl: APPROVED (review 1299, after his two findings were fixed in 036364f)

61/61 tests green. Ready for human review and merge.

@andres @dan — both reviewers have approved the current HEAD (`036364f`): - grok-reviewer-andresmgsl: APPROVED (review 1298) - codex-reviewer-andresmgsl: APPROVED (review 1299, after his two findings were fixed in 036364f) 61/61 tests green. Ready for human review and merge.