From 355fcc1f6710d76dfeeb32905bb2731c29c9e2bc Mon Sep 17 00:00:00 2001 From: kimi-reviewer-andresmgsl Date: Sun, 26 Jul 2026 20:31:07 +0000 Subject: [PATCH 1/2] Add release, label, and api commands (v1.3.0) --- README.md | 156 ++++++++++++++++++++++++ package-lock.json | 4 +- package.json | 2 +- src/api.js | 39 +++++- src/cli.js | 300 ++++++++++++++++++++++++++++++++++++++++++++++ test/api.test.js | 44 +++++++ test/cli.test.js | 77 +++++++++++- 7 files changed, 617 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index f5be331..bc1ac15 100644 --- a/README.md +++ b/README.md @@ -509,6 +509,138 @@ stoke pr review -o heavy-duty -r stoke -n 3 --event request-changes --body-file Calls `POST /api/v1/repos/{owner}/{repo}/pulls/{number}/reviews`. Prints the review URL when the forge returns one. +### `stoke release list` + +List releases in a repository. + +```text +Options: + -o, --owner repository owner (required) + -r, --repo repository name (required) + -l, --limit maximum releases to display (default: 50; use 0 for all) +``` + +```bash +stoke release list -o heavy-duty -r stoke +``` + +Calls `GET /api/v1/repos/{owner}/{repo}/releases` and auto-paginates. + +### `stoke release view` + +Show the release for a tag, including its notes. + +```text +Options: + -o, --owner repository owner (required) + -r, --repo repository name (required) + --tag tag name of the release (required) +``` + +```bash +stoke release view -o heavy-duty -r stoke --tag v1.2.1 +``` + +Calls `GET /api/v1/repos/{owner}/{repo}/releases/tags/{tag}`. + +### `stoke release create` + +Create a release. If the tag does not exist yet, Forgejo creates it from `--target` (or the repository default branch). When both `-b` and `--body-file` are set, **`--body-file` wins**. + +```text +Options: + -o, --owner repository owner (required) + -r, --repo repository name (required) + --tag tag name for the release (required) + --target branch or commit the tag is created from (default: default branch) + -t, --title release title (default: the tag name) + -b, --body <body> release notes (markdown) + --body-file <path> read the release notes from a file (wins over -b) + --draft create as a draft release + --prerelease mark as a prerelease +``` + +```bash +stoke release create -o heavy-duty -r stoke --tag v1.3.0 --body-file release-notes.md +``` + +Calls `POST /api/v1/repos/{owner}/{repo}/releases`. + +### `stoke label list` + +List labels in a repository. + +```text +Options: + -o, --owner <owner> repository owner (required) + -r, --repo <repo> repository name (required) + -l, --limit <number> maximum labels to display (default: 50; use 0 for all) +``` + +```bash +stoke label list -o heavy-duty -r stoke +``` + +Calls `GET /api/v1/repos/{owner}/{repo}/labels` and auto-paginates. + +### `stoke label create` + +Create a label in a repository. The color is validated (6 hex digits, with or without a leading `#`) before any network call. + +```text +Options: + -o, --owner <owner> repository owner (required) + -r, --repo <repo> repository name (required) + --name <name> label name (required) + --color <color> label color, 6 hex digits (required) + -d, --description <description> label description +``` + +```bash +stoke label create -o heavy-duty -r stoke --name release --color 0E8A16 \ + -d "Release flow and version/packaging work" +``` + +Calls `POST /api/v1/repos/{owner}/{repo}/labels`. + +### `stoke label delete` + +Delete a label from a repository, by `--id` or `--name` (one is required). + +```bash +stoke label delete -o heavy-duty -r stoke --name needs-triage +``` + +Calls `DELETE /api/v1/repos/{owner}/{repo}/labels/{id}`. A `--name` is resolved to an id via the repository label list first. + +### `stoke label add` + +Add labels to an issue or pull request (PRs are issues as far as labels are concerned). + +```text +Options: + -o, --owner <owner> repository owner (required) + -r, --repo <repo> repository name (required) + -n, --number <number> issue or pull request number (required) + --name <name...> one or more label names (required) +``` + +```bash +stoke label add -o heavy-duty -r stoke -n 12 --name release scope:cli +``` + +Calls `POST /api/v1/repos/{owner}/{repo}/issues/{number}/labels`. Names are resolved to ids first; an unknown name fails with `Label not found`. + +### `stoke label remove` + +Remove labels from an issue or pull request. + +```bash +stoke label remove -o heavy-duty -r stoke -n 12 --name needs-triage +``` + +Calls `DELETE /api/v1/repos/{owner}/{repo}/issues/{number}/labels/{id}` once per label. + ### `stoke branch list` List branches in a repository. @@ -709,6 +841,30 @@ stoke user show -u andres Calls `GET /api/v1/users/{username}`. +### `stoke api` + +Make an authenticated request to any Forgejo API endpoint and print the JSON response. The escape hatch for everything stoke does not wrap yet — pass the endpoint path without the `/api/v1` prefix. + +```text +Arguments: + <endpoint> endpoint path starting with / (required) + +Options: + -X, --method <method> GET, POST, PUT, PATCH or DELETE + (default: GET, or POST when --input is given) + --input <json> JSON request body, inline or @path to read from a file + --paginate fetch all pages (GET endpoints returning a JSON array) +``` + +```bash +stoke api /user +stoke api "/repos/heavy-duty/stoke/pulls?state=closed" --paginate +stoke api /repos/heavy-duty/stoke/issues/12/comments --input '{"body":"hi"}' +stoke api /repos/heavy-duty/stoke/contents/CHANGELOG.md --input @payload.json +``` + +Calls `{METHOD} /api/v1{endpoint}` with the stored token. The endpoint must start with `/`; the method, `--paginate` + non-GET, and malformed `--input` JSON are all rejected before any network call. + ## Architecture ```text diff --git a/package-lock.json b/package-lock.json index a7fcf38..821d1e2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "stoke", - "version": "1.2.1", + "version": "1.3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "stoke", - "version": "1.2.1", + "version": "1.3.0", "license": "ISC", "dependencies": { "commander": "^15.0.0" diff --git a/package.json b/package.json index 5a971c3..5992e54 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "stoke", - "version": "1.2.1", + "version": "1.3.0", "description": "CLI for the heavy-duty forge (Forgejo)", "main": "src/cli.js", "scripts": { diff --git a/src/api.js b/src/api.js index ea45bc2..9a77c88 100644 --- a/src/api.js +++ b/src/api.js @@ -134,9 +134,10 @@ class ForgejoClient { async getAll(endpoint, params = {}) { const pageSize = 50; const all = []; + const separator = endpoint.includes('?') ? '&' : '?'; for (let page = 1; page <= 1000; page += 1) { const query = new URLSearchParams({ ...params, limit: String(pageSize), page: String(page) }).toString(); - const items = await this.get(`${endpoint}?${query}`); + const items = await this.get(`${endpoint}${separator}${query}`); if (!Array.isArray(items) || items.length === 0) break; all.push(...items); if (items.length < pageSize) break; @@ -190,6 +191,42 @@ class ForgejoClient { return this.getAll(`/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/branches`, opts); } + async listReleases(owner, repo, opts = {}) { + return this.getAll(`/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/releases`, opts); + } + + async getReleaseByTag(owner, repo, tag) { + return this.get(`/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/releases/tags/${encodeURIComponent(tag)}`); + } + + async createRelease(owner, repo, payload) { + return this.post(`/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/releases`, payload); + } + + async listLabels(owner, repo, opts = {}) { + return this.getAll(`/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/labels`, opts); + } + + async createLabel(owner, repo, payload) { + return this.post(`/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/labels`, payload); + } + + async deleteLabel(owner, repo, id) { + return this.del(`/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/labels/${id}`); + } + + // Pull requests are issues as far as labels are concerned, so these two + // serve both surfaces. + async addIssueLabels(owner, repo, index, labelIds) { + return this.post(`/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/issues/${index}/labels`, { + labels: labelIds, + }); + } + + async removeIssueLabel(owner, repo, index, labelId) { + return this.del(`/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/issues/${index}/labels/${labelId}`); + } + async addCollaborator(owner, repo, username, permission) { return this.request('PUT', `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/collaborators/${encodeURIComponent(username)}`, { permission, diff --git a/src/cli.js b/src/cli.js index bb70816..35d99b1 100755 --- a/src/cli.js +++ b/src/cli.js @@ -786,6 +786,251 @@ pr } }); +const release = program + .command('release') + .description('Manage releases'); + +release + .command('list') + .description('List releases in a repository') + .requiredOption('-o, --owner <owner>', 'repository owner') + .requiredOption('-r, --repo <repo>', 'repository name') + .option('-l, --limit <number>', 'maximum releases to return (0 for all)', parseLimit, 50) + .action(async (options) => { + try { + const config = loadConfig(); + const client = ForgejoClient.fromConfig(config); + const releases = await client.listReleases(options.owner, options.repo); + const display = options.limit > 0 ? releases.slice(0, options.limit) : releases; + if (!display.length) { + console.log('No releases found.'); + return; + } + for (const rel of display) { + const flags = [rel.draft && 'draft', rel.prerelease && 'prerelease'].filter(Boolean).join('|'); + const tag = flags ? `${rel.tag_name} [${flags}]` : rel.tag_name; + console.log(`${tag} ${rel.name || ''}`.trimEnd()); + } + if (releases.length > display.length) { + console.log(`...and ${releases.length - display.length} more (use -l 0 for all).`); + } + } catch (err) { + console.error(`Failed to list releases: ${err.message}`); + process.exit(1); + } + }); + +release + .command('view') + .description('Show the release for a tag') + .requiredOption('-o, --owner <owner>', 'repository owner') + .requiredOption('-r, --repo <repo>', 'repository name') + .requiredOption('--tag <tag>', 'tag name of the release') + .action(async (options) => { + try { + const config = loadConfig(); + const client = ForgejoClient.fromConfig(config); + const rel = await client.getReleaseByTag(options.owner, options.repo, options.tag); + const flags = [rel.draft && 'draft', rel.prerelease && 'prerelease'].filter(Boolean).join('|'); + console.log(`${rel.tag_name}${flags ? ` [${flags}]` : ''} ${rel.name || ''}`.trimEnd()); + console.log(`URL: ${rel.html_url}`); + console.log(`Target: ${rel.target_commitish}`); + console.log(`Author: ${rel.author?.login || '(unknown)'}`); + console.log(`Published: ${rel.published_at}`); + if (rel.body) { + console.log('\n' + rel.body); + } + } catch (err) { + console.error(`Failed to show release: ${err.message}`); + if (err.status) console.error(`HTTP status: ${err.status}`); + process.exit(1); + } + }); + +release + .command('create') + .description('Create a release (creates the tag too if it does not exist)') + .requiredOption('-o, --owner <owner>', 'repository owner') + .requiredOption('-r, --repo <repo>', 'repository name') + .requiredOption('--tag <tag>', 'tag name for the release') + .option('--target <ref>', 'branch or commit the tag is created from (default: repository default branch)') + .option('-t, --title <title>', 'release title (default: the tag name)') + .option('-b, --body <body>', 'release notes (markdown)') + .option('--body-file <path>', 'read the release notes from a file (wins over -b)') + .option('--draft', 'create as a draft release', false) + .option('--prerelease', 'mark as a prerelease', false) + .action(async (options) => { + try { + const config = loadConfig(); + const client = ForgejoClient.fromConfig(config); + const payload = { + tag_name: options.tag, + name: options.title || options.tag, + body: readBodyOption(options) || '', + draft: options.draft, + prerelease: options.prerelease, + }; + if (options.target) payload.target_commitish = options.target; + const result = await client.createRelease(options.owner, options.repo, payload); + console.log(`Release created: ${result.tag_name} ${result.name || ''}`.trimEnd()); + console.log(`URL: ${result.html_url}`); + } catch (err) { + console.error(`Release creation failed: ${err.message}`); + if (err.status) console.error(`HTTP status: ${err.status}`); + process.exit(1); + } + }); + +function parseColor(value) { + const hex = value.replace(/^#/, ''); + if (!/^[0-9a-fA-F]{6}$/.test(hex)) { + throw new InvalidArgumentError('Color must be 6 hex digits (with or without a leading #).'); + } + return hex.toLowerCase(); +} + +// Label add/remove/lookup go through names on the CLI but ids on the wire, +// so every caller resolves against the repo's label list first. +async function resolveLabelIds(client, owner, repo, names) { + const labels = await client.listLabels(owner, repo); + const byName = new Map(labels.map((l) => [l.name, l.id])); + return names.map((name) => { + const id = byName.get(name); + if (id === undefined) { + throw new Error(`Label not found in ${owner}/${repo}: ${name}`); + } + return id; + }); +} + +const label = program + .command('label') + .description('Manage repository labels'); + +label + .command('list') + .description('List labels in a repository') + .requiredOption('-o, --owner <owner>', 'repository owner') + .requiredOption('-r, --repo <repo>', 'repository name') + .option('-l, --limit <number>', 'maximum labels to return (0 for all)', parseLimit, 50) + .action(async (options) => { + try { + const config = loadConfig(); + const client = ForgejoClient.fromConfig(config); + const labels = await client.listLabels(options.owner, options.repo); + const display = options.limit > 0 ? labels.slice(0, options.limit) : labels; + if (!display.length) { + console.log('No labels found.'); + return; + } + for (const l of display) { + console.log(`#${l.id} ${l.name} #${l.color}${l.description ? ` — ${l.description}` : ''}`); + } + if (labels.length > display.length) { + console.log(`...and ${labels.length - display.length} more (use -l 0 for all).`); + } + } catch (err) { + console.error(`Failed to list labels: ${err.message}`); + process.exit(1); + } + }); + +label + .command('create') + .description('Create a label in a repository') + .requiredOption('-o, --owner <owner>', 'repository owner') + .requiredOption('-r, --repo <repo>', 'repository name') + .requiredOption('--name <name>', 'label name') + .requiredOption('--color <color>', 'label color, 6 hex digits (with or without #)', parseColor) + .option('-d, --description <description>', 'label description', '') + .action(async (options) => { + try { + const config = loadConfig(); + const client = ForgejoClient.fromConfig(config); + const result = await client.createLabel(options.owner, options.repo, { + name: options.name, + color: options.color, + description: options.description, + }); + console.log(`Label created: #${result.id} ${result.name} #${result.color}`); + } catch (err) { + console.error(`Label creation failed: ${err.message}`); + if (err.status) console.error(`HTTP status: ${err.status}`); + process.exit(1); + } + }); + +label + .command('delete') + .description('Delete a label from a repository (by --id or --name)') + .requiredOption('-o, --owner <owner>', 'repository owner') + .requiredOption('-r, --repo <repo>', 'repository name') + .option('--id <id>', 'label id', parseId) + .option('--name <name>', 'label name') + .action(async (options) => { + try { + if (!options.id && !options.name) { + console.error('One of --id or --name is required.'); + process.exit(1); + } + const config = loadConfig(); + const client = ForgejoClient.fromConfig(config); + const ids = options.id + ? [options.id] + : await resolveLabelIds(client, options.owner, options.repo, [options.name]); + await client.deleteLabel(options.owner, options.repo, ids[0]); + console.log(`Label deleted: ${options.name || `#${options.id}`} from ${options.owner}/${options.repo}.`); + } catch (err) { + console.error(`Label deletion failed: ${err.message}`); + if (err.status) console.error(`HTTP status: ${err.status}`); + process.exit(1); + } + }); + +label + .command('add') + .description('Add labels to an issue or pull request') + .requiredOption('-o, --owner <owner>', 'repository owner') + .requiredOption('-r, --repo <repo>', 'repository name') + .requiredOption('-n, --number <number>', 'issue or pull request number', parseId) + .requiredOption('--name <name...>', 'one or more label names') + .action(async (options) => { + try { + const config = loadConfig(); + const client = ForgejoClient.fromConfig(config); + const ids = await resolveLabelIds(client, options.owner, options.repo, options.name); + await client.addIssueLabels(options.owner, options.repo, options.number, ids); + console.log(`Labels added to #${options.number} in ${options.owner}/${options.repo}: ${options.name.join(', ')}`); + } catch (err) { + console.error(`Failed to add labels: ${err.message}`); + if (err.status) console.error(`HTTP status: ${err.status}`); + process.exit(1); + } + }); + +label + .command('remove') + .description('Remove labels from an issue or pull request') + .requiredOption('-o, --owner <owner>', 'repository owner') + .requiredOption('-r, --repo <repo>', 'repository name') + .requiredOption('-n, --number <number>', 'issue or pull request number', parseId) + .requiredOption('--name <name...>', 'one or more label names') + .action(async (options) => { + try { + const config = loadConfig(); + const client = ForgejoClient.fromConfig(config); + const ids = await resolveLabelIds(client, options.owner, options.repo, options.name); + for (const id of ids) { + await client.removeIssueLabel(options.owner, options.repo, options.number, id); + } + console.log(`Labels removed from #${options.number} in ${options.owner}/${options.repo}: ${options.name.join(', ')}`); + } catch (err) { + console.error(`Failed to remove labels: ${err.message}`); + if (err.status) console.error(`HTTP status: ${err.status}`); + process.exit(1); + } + }); + const branchCmd = program .command('branch') .description('Manage branches'); @@ -1098,6 +1343,61 @@ user } }); +program + .command('api') + .description('Make an authenticated request to any Forgejo API endpoint and print the JSON response') + .argument('<endpoint>', 'endpoint path starting with / (the /api/v1 prefix is added for you)') + .option('-X, --method <method>', 'HTTP method (default: GET, or POST when --input is given)') + .option('--input <json>', 'JSON request body, inline or @path to read it from a file') + .option('--paginate', 'fetch all pages (GET endpoints returning a JSON array)', false) + .action(async (endpoint, options) => { + try { + if (!endpoint.startsWith('/')) { + console.error('Endpoint must start with / (e.g. /repos/owner/repo/pulls?state=closed).'); + process.exit(1); + } + const method = (options.method || (options.input ? 'POST' : 'GET')).toUpperCase(); + if (!['GET', 'POST', 'PUT', 'PATCH', 'DELETE'].includes(method)) { + console.error(`Unsupported method: ${method}. Use GET, POST, PUT, PATCH or DELETE.`); + process.exit(1); + } + if (options.paginate && method !== 'GET') { + console.error('--paginate only works with GET.'); + process.exit(1); + } + + let body = null; + if (options.input !== undefined) { + const raw = options.input.startsWith('@') + ? (() => { + try { + return fs.readFileSync(options.input.slice(1), 'utf8'); + } catch (err) { + throw new Error(`Could not read input file ${options.input.slice(1)}: ${err.message}`); + } + })() + : options.input; + try { + body = JSON.parse(raw); + } catch (err) { + console.error(`--input is not valid JSON: ${err.message}`); + process.exit(1); + } + } + + const config = loadConfig(); + const client = ForgejoClient.fromConfig(config); + const data = options.paginate + ? await client.getAll(endpoint) + : await client.request(method, endpoint, body); + console.log(JSON.stringify(data, null, 2)); + } catch (err) { + console.error(`API request failed: ${err.message}`); + if (err.status) console.error(`HTTP status: ${err.status}`); + process.exit(1); + } + }); + program.parseAsync(process.argv).catch((err) => { console.error(err); process.exit(1); diff --git a/test/api.test.js b/test/api.test.js index 4fede09..24ee8eb 100644 --- a/test/api.test.js +++ b/test/api.test.js @@ -179,3 +179,47 @@ test('createPullRequestReview preserves leading and trailing whitespace in the b const body = JSON.parse(calls[0].opts.body); assert.equal(body.body, rawBody); }); + +test('getAll joins pagination with & when the endpoint already has a query', async () => { + const calls = mockFetch(() => jsonResponse([])); + const client = new ForgejoClient('https://forge.test', 'tok'); + await client.getAll('/repos/o/r/pulls?state=closed'); + assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/o/r/pulls?state=closed&limit=50&page=1'); +}); + +test('release endpoints map to the expected URLs and payloads', async () => { + const calls = mockFetch(() => jsonResponse({ tag_name: '1.0.0' })); + const client = new ForgejoClient('https://forge.test', 'tok'); + await client.listReleases('owner', 'repo'); + await client.getReleaseByTag('owner', 'repo', '1.0.0-rc1'); + await client.createRelease('owner', 'repo', { tag_name: '1.0.0', name: '1.0.0', body: '', draft: false, prerelease: false }); + assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/owner/repo/releases?limit=50&page=1'); + assert.equal(calls[1].url, 'https://forge.test/api/v1/repos/owner/repo/releases/tags/1.0.0-rc1'); + assert.equal(calls[2].url, 'https://forge.test/api/v1/repos/owner/repo/releases'); + assert.equal(calls[2].opts.method, 'POST'); + assert.equal(JSON.parse(calls[2].opts.body).tag_name, '1.0.0'); +}); + +test('label endpoints map to the expected URLs and payloads', async () => { + const calls = mockFetch(() => jsonResponse({ id: 3 })); + const client = new ForgejoClient('https://forge.test', 'tok'); + await client.createLabel('owner', 'repo', { name: 'release', color: '0e8a16', description: '' }); + await client.deleteLabel('owner', 'repo', 3); + assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/owner/repo/labels'); + assert.equal(calls[0].opts.method, 'POST'); + assert.deepEqual(JSON.parse(calls[0].opts.body), { name: 'release', color: '0e8a16', description: '' }); + assert.equal(calls[1].url, 'https://forge.test/api/v1/repos/owner/repo/labels/3'); + assert.equal(calls[1].opts.method, 'DELETE'); +}); + +test('issue label add/remove hit the issue labels endpoints', async () => { + const calls = mockFetch(() => jsonResponse(null, 204)); + const client = new ForgejoClient('https://forge.test', 'tok'); + await client.addIssueLabels('owner', 'repo', 7, [3, 4]); + await client.removeIssueLabel('owner', 'repo', 7, 3); + assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/owner/repo/issues/7/labels'); + assert.equal(calls[0].opts.method, 'POST'); + assert.deepEqual(JSON.parse(calls[0].opts.body), { labels: [3, 4] }); + assert.equal(calls[1].url, 'https://forge.test/api/v1/repos/owner/repo/issues/7/labels/3'); + assert.equal(calls[1].opts.method, 'DELETE'); +}); diff --git a/test/cli.test.js b/test/cli.test.js index e60a278..0db0743 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -23,7 +23,7 @@ test('--version matches package.json', () => { test('--help lists every top-level command', () => { const out = execFileSync(process.execPath, [CLI, '--help'], { encoding: 'utf8' }); - for (const cmd of ['auth', 'repo', 'issue', 'pr', 'branch', 'collaborator', 'org', 'user']) { + for (const cmd of ['auth', 'repo', 'issue', 'pr', 'release', 'label', 'branch', 'collaborator', 'org', 'user', 'api']) { assert.match(out, new RegExp(`^\\s+${cmd}`, 'm'), `missing command: ${cmd}`); } }); @@ -176,6 +176,81 @@ test('pr review approve allows an empty body before any network call', () => { } }); +test('label create rejects an invalid color before any network call', () => { + const res = run(['label', 'create', '-o', 'o', '-r', 'r', '--name', 'x', '--color', 'red']); + assert.equal(res.status, 1); + assert.match(res.stderr, /Color must be 6 hex digits/); +}); + +test('label delete requires one of --id or --name before any network call', () => { + const res = run(['label', 'delete', '-o', 'o', '-r', 'r']); + assert.equal(res.status, 1); + assert.match(res.stderr, /One of --id or --name is required/); +}); + +test('api rejects an endpoint without a leading slash before any network call', () => { + const res = run(['api', 'repos/o/r']); + assert.equal(res.status, 1); + assert.match(res.stderr, /Endpoint must start with \//); +}); + +test('api rejects an unsupported method before any network call', () => { + const res = run(['api', '/user', '-X', 'HEAD']); + assert.equal(res.status, 1); + assert.match(res.stderr, /Unsupported method/); +}); + +test('api rejects --paginate with a non-GET method before any network call', () => { + const res = run(['api', '/user', '-X', 'POST', '--paginate']); + assert.equal(res.status, 1); + assert.match(res.stderr, /--paginate only works with GET/); +}); + +test('api rejects invalid JSON input before any network call', () => { + const res = run(['api', '/user', '--input', '{nope']); + assert.equal(res.status, 1); + assert.match(res.stderr, /not valid JSON/); +}); + +test('api sends the token and prints the JSON response', async () => { + const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}-api.json`); + const TIMEOUT_MS = 5000; + let timer; + const result = await new Promise((resolve, reject) => { + const fail = (err) => { + clearTimeout(timer); + try { server.close(); } catch { /* already closed */ } + reject(err instanceof Error ? err : new Error(String(err))); + }; + let authHeader = null; + const server = http.createServer((req, res) => { + authHeader = req.headers.authorization; + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ login: 'bot' })); + }); + timer = setTimeout(() => fail(new Error('timeout')), TIMEOUT_MS); + server.listen(0, '127.0.0.1', async () => { + const { port } = server.address(); + fs.writeFileSync(cfg, JSON.stringify({ url: `http://127.0.0.1:${port}`, token: 'tok' })); + try { + const res = await spawnAsync(['api', '/user'], { STOKE_CONFIG_FILE: cfg }); + clearTimeout(timer); + server.close(() => resolve({ res, authHeader })); + } catch (err) { + fail(err); + } + }); + }).finally(() => clearTimeout(timer)); + + try { + assert.equal(result.res.status, 0, result.res.stderr); + assert.equal(result.authHeader, 'token tok'); + assert.deepEqual(JSON.parse(result.res.stdout), { login: 'bot' }); + } finally { + fs.unlinkSync(cfg); + } +}); + function spawnAsync(args, env = {}) { return new Promise((resolve, reject) => { const child = spawn(process.execPath, [CLI, ...args], { -- 2.45.2 From 036364f844d872837e2a9c525a09a33b75177285 Mon Sep 17 00:00:00 2001 From: kimi-reviewer-andresmgsl <andres+4@heavyduty.builders> Date: Sun, 26 Jul 2026 20:52:29 +0000 Subject: [PATCH 2/2] Address review: pager owns limit/page, reject GET+input, label delete exclusivity --- README.md | 9 ++++++--- src/api.js | 16 ++++++++++++--- src/cli.js | 8 ++++++++ test/api.test.js | 28 +++++++++++++++++++++----- test/cli.test.js | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 101 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index bc1ac15..4da74e0 100644 --- a/README.md +++ b/README.md @@ -605,7 +605,7 @@ Calls `POST /api/v1/repos/{owner}/{repo}/labels`. ### `stoke label delete` -Delete a label from a repository, by `--id` or `--name` (one is required). +Delete a label from a repository, by `--id` or `--name` (exactly one is required; passing both is rejected). ```bash stoke label delete -o heavy-duty -r stoke --name needs-triage @@ -853,7 +853,8 @@ Options: -X, --method <method> GET, POST, PUT, PATCH or DELETE (default: GET, or POST when --input is given) --input <json> JSON request body, inline or @path to read from a file - --paginate fetch all pages (GET endpoints returning a JSON array) + --paginate fetch all pages (GET endpoints returning a JSON array); + overrides any limit/page in the endpoint ``` ```bash @@ -863,7 +864,9 @@ stoke api /repos/heavy-duty/stoke/issues/12/comments --input '{"body":"hi"}' stoke api /repos/heavy-duty/stoke/contents/CHANGELOG.md --input @payload.json ``` -Calls `{METHOD} /api/v1{endpoint}` with the stored token. The endpoint must start with `/`; the method, `--paginate` + non-GET, and malformed `--input` JSON are all rejected before any network call. +Calls `{METHOD} /api/v1{endpoint}` with the stored token. The endpoint must start with `/`; the method, `--paginate` + non-GET, `GET` + `--input` (a GET cannot carry a body), and malformed `--input` JSON are all rejected before any network call. + +**Security:** `stoke api` is a full authenticated passthrough — it does anything the stored token is allowed to do. Never interpolate untrusted strings (issue titles, PR bodies, user input) into the endpoint or `--input`; treat every call like the credential it carries. ## Architecture diff --git a/src/api.js b/src/api.js index 9a77c88..d0c5ba3 100644 --- a/src/api.js +++ b/src/api.js @@ -134,10 +134,20 @@ class ForgejoClient { async getAll(endpoint, params = {}) { const pageSize = 50; const all = []; - const separator = endpoint.includes('?') ? '&' : '?'; + // The pager owns limit/page: a caller-supplied pair must be overridden, + // not duplicated — a duplicated limit pins the page size the server + // honors first and can truncate or loop the walk. + const queryIndex = endpoint.indexOf('?'); + const path = queryIndex === -1 ? endpoint : endpoint.slice(0, queryIndex); + const baseQuery = new URLSearchParams(queryIndex === -1 ? '' : endpoint.slice(queryIndex + 1)); + baseQuery.delete('limit'); + baseQuery.delete('page'); for (let page = 1; page <= 1000; page += 1) { - const query = new URLSearchParams({ ...params, limit: String(pageSize), page: String(page) }).toString(); - const items = await this.get(`${endpoint}${separator}${query}`); + const query = new URLSearchParams(baseQuery); + for (const [key, value] of Object.entries(params)) query.set(key, value); + query.set('limit', String(pageSize)); + query.set('page', String(page)); + const items = await this.get(`${path}?${query.toString()}`); if (!Array.isArray(items) || items.length === 0) break; all.push(...items); if (items.length < pageSize) break; diff --git a/src/cli.js b/src/cli.js index 35d99b1..e7357fa 100755 --- a/src/cli.js +++ b/src/cli.js @@ -973,6 +973,10 @@ label console.error('One of --id or --name is required.'); process.exit(1); } + if (options.id && options.name) { + console.error('Use either --id or --name, not both.'); + process.exit(1); + } const config = loadConfig(); const client = ForgejoClient.fromConfig(config); const ids = options.id @@ -1365,6 +1369,10 @@ program console.error('--paginate only works with GET.'); process.exit(1); } + if (method === 'GET' && options.input !== undefined) { + console.error('GET requests cannot carry a body. Drop --input, or use -X POST/PUT/PATCH/DELETE.'); + process.exit(1); + } let body = null; if (options.input !== undefined) { diff --git a/test/api.test.js b/test/api.test.js index 24ee8eb..cd60fc8 100644 --- a/test/api.test.js +++ b/test/api.test.js @@ -187,6 +187,22 @@ test('getAll joins pagination with & when the endpoint already has a query', asy assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/o/r/pulls?state=closed&limit=50&page=1'); }); +test('getAll overrides caller-supplied limit/page instead of duplicating them', async () => { + const calls = mockFetch((url) => { + const page = Number(new URL(url).searchParams.get('page')); + return jsonResponse(page === 1 ? Array.from({ length: 50 }, (_, i) => ({ id: i })) : []); + }); + const client = new ForgejoClient('https://forge.test', 'tok'); + const all = await client.getAll('/repos/o/r/pulls?state=closed&limit=1&page=9'); + assert.equal(all.length, 50); + const first = new URL(calls[0].url).searchParams; + const second = new URL(calls[1].url).searchParams; + assert.deepEqual(first.getAll('limit'), ['50']); + assert.deepEqual(first.getAll('page'), ['1']); + assert.deepEqual(second.getAll('page'), ['2']); + assert.equal(first.get('state'), 'closed'); +}); + test('release endpoints map to the expected URLs and payloads', async () => { const calls = mockFetch(() => jsonResponse({ tag_name: '1.0.0' })); const client = new ForgejoClient('https://forge.test', 'tok'); @@ -203,13 +219,15 @@ test('release endpoints map to the expected URLs and payloads', async () => { test('label endpoints map to the expected URLs and payloads', async () => { const calls = mockFetch(() => jsonResponse({ id: 3 })); const client = new ForgejoClient('https://forge.test', 'tok'); + await client.listLabels('owner', 'repo'); await client.createLabel('owner', 'repo', { name: 'release', color: '0e8a16', description: '' }); await client.deleteLabel('owner', 'repo', 3); - assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/owner/repo/labels'); - assert.equal(calls[0].opts.method, 'POST'); - assert.deepEqual(JSON.parse(calls[0].opts.body), { name: 'release', color: '0e8a16', description: '' }); - assert.equal(calls[1].url, 'https://forge.test/api/v1/repos/owner/repo/labels/3'); - assert.equal(calls[1].opts.method, 'DELETE'); + assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/owner/repo/labels?limit=50&page=1'); + assert.equal(calls[1].url, 'https://forge.test/api/v1/repos/owner/repo/labels'); + assert.equal(calls[1].opts.method, 'POST'); + assert.deepEqual(JSON.parse(calls[1].opts.body), { name: 'release', color: '0e8a16', description: '' }); + assert.equal(calls[2].url, 'https://forge.test/api/v1/repos/owner/repo/labels/3'); + assert.equal(calls[2].opts.method, 'DELETE'); }); test('issue label add/remove hit the issue labels endpoints', async () => { diff --git a/test/cli.test.js b/test/cli.test.js index 0db0743..02ce46b 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -206,6 +206,18 @@ test('api rejects --paginate with a non-GET method before any network call', () assert.match(res.stderr, /--paginate only works with GET/); }); +test('api rejects GET with --input before any network call', () => { + const res = run(['api', '/user', '-X', 'GET', '--input', '{}']); + assert.equal(res.status, 1); + assert.match(res.stderr, /GET requests cannot carry a body/); +}); + +test('label delete rejects --id and --name together before any network call', () => { + const res = run(['label', 'delete', '-o', 'o', '-r', 'r', '--id', '3', '--name', 'x']); + assert.equal(res.status, 1); + assert.match(res.stderr, /either --id or --name, not both/); +}); + test('api rejects invalid JSON input before any network call', () => { const res = run(['api', '/user', '--input', '{nope']); assert.equal(res.status, 1); @@ -251,6 +263,45 @@ test('api sends the token and prints the JSON response', async () => { } }); +test('label add fails closed on an unknown label name', async () => { + const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}-lbl.json`); + const TIMEOUT_MS = 5000; + let timer; + const result = await new Promise((resolve, reject) => { + const fail = (err) => { + clearTimeout(timer); + try { server.close(); } catch { /* already closed */ } + reject(err instanceof Error ? err : new Error(String(err))); + }; + const server = http.createServer((req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify([{ id: 1, name: 'bug', color: 'd73a4a' }])); + }); + timer = setTimeout(() => fail(new Error('timeout')), TIMEOUT_MS); + server.listen(0, '127.0.0.1', async () => { + const { port } = server.address(); + fs.writeFileSync(cfg, JSON.stringify({ url: `http://127.0.0.1:${port}`, token: 'tok' })); + try { + const res = await spawnAsync( + ['label', 'add', '-o', 'o', '-r', 'r', '-n', '7', '--name', 'ghost'], + { STOKE_CONFIG_FILE: cfg }, + ); + clearTimeout(timer); + server.close(() => resolve(res)); + } catch (err) { + fail(err); + } + }); + }).finally(() => clearTimeout(timer)); + + try { + assert.equal(result.status, 1); + assert.match(result.stderr, /Label not found in o\/r: ghost/); + } finally { + fs.unlinkSync(cfg); + } +}); + function spawnAsync(args, env = {}) { return new Promise((resolve, reject) => { const child = spawn(process.execPath, [CLI, ...args], { -- 2.45.2