codex-bot-andresmgsl/stoke
1
0
Fork 0
forked from heavy-duty/stoke
Code Pull requests Activity

Polish PR review CLI and harden install-apt (v1.2.1)

Browse source 
- Accept approve/approved review event aliases; print review html_url
- Harden pr show against missing user/head/base; clarify body-file wins
- Add tests for whitespace-only comments, approved alias, review URL
- Timeout the CLI-boundary HTTP fixture; clean up install-apt test trees
- Clearer refuse-to-overwrite message when nodesource.list already exists
- Merge Node 22 NodeSource bootstrap (from fix/apt-nodejs-bootstrap)
This commit is contained in:
grok-reviewer-andresmgsl 2026-07-22 23:18:43 +00:00 committed by kimi-reviewer-andresmgsl
parent 41b65a2bbd
commit d959d2a6f4
6 changed files with 136 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
README.md
View file

@ -471,15 +471,15 @@ Calls `GET /api/v1/repos/{owner}/{repo}/pulls/{number}`.
### `stoke pr comment` ### `stoke pr comment`
Add a comment to a pull request. Add a comment to a pull request. Body is required (whitespace-only is rejected). When both `-b` and `--body-file` are set, **`--body-file` wins**.
```text ```text
Options: Options:
-o, --owner <owner> repository owner (required) -o, --owner <owner> repository owner (required)
-r, --repo <repo> repository name (required) -r, --repo <repo> repository name (required)
-n, --number <number> pull request number (required) -n, --number <number> pull request number (required)
-b, --body <body> comment body (markdown) -b, --body <body> comment body (markdown; required unless --body-file)
--body-file <path> read the comment body from a file --body-file <path> read the comment body from a file (wins over -b)
``` ```
```bash ```bash
@ -490,23 +490,24 @@ Calls `POST /api/v1/repos/{owner}/{repo}/issues/{number}/comments`.
### `stoke pr review` ### `stoke pr review`
Submit a review on a pull request. Submit a review on a pull request. `approve` / `approved` may omit a body; `request-changes` and `comment` require a non-empty body (raw body is preserved — only emptiness is checked with `trim()`). When both `-b` and `--body-file` are set, **`--body-file` wins**.
```text ```text
Options: Options:
-o, --owner <owner> repository owner (required) -o, --owner <owner> repository owner (required)
-r, --repo <repo> repository name (required) -r, --repo <repo> repository name (required)
-n, --number <number> pull request number (required) -n, --number <number> pull request number (required)
--event <event> review event: approve, request-changes|request_changes, comment (required) --event <event> approve|approved, request-changes|request_changes, comment (required)
-b, --body <body> review body (markdown) -b, --body <body> review body (markdown; required for request-changes and comment)
--body-file <path> read the review body from a file --body-file <path> read the review body from a file (wins over -b)
``` ```
```bash ```bash
stoke pr review -o heavy-duty -r stoke -n 3 --event approve -b "Ship it." stoke pr review -o heavy-duty -r stoke -n 3 --event approve -b "Ship it."
stoke pr review -o heavy-duty -r stoke -n 3 --event request-changes --body-file notes.md
``` ```
Calls `POST /api/v1/repos/{owner}/{repo}/pulls/{number}/reviews`. Calls `POST /api/v1/repos/{owner}/{repo}/pulls/{number}/reviews`. Prints the review URL when the forge returns one.
### `stoke branch list` ### `stoke branch list`

4
package-lock.json generated
View file

@ -1,12 +1,12 @@
{ {
"name": "stoke", "name": "stoke",
"version": "1.2.0", "version": "1.2.1",
"lockfileVersion": 3, "lockfileVersion": 3,
"requires": true, "requires": true,
"packages": { "packages": {
"": { "": {
"name": "stoke", "name": "stoke",
"version": "1.2.0", "version": "1.2.1",
"license": "ISC", "license": "ISC",
"dependencies": { "dependencies": {
"commander": "^15.0.0" "commander": "^15.0.0"

2
package.json
View file

@ -1,6 +1,6 @@
{ {
"name": "stoke", "name": "stoke",
"version": "1.2.0", "version": "1.2.1",
"description": "CLI for the heavy-duty forge (Forgejo)", "description": "CLI for the heavy-duty forge (Forgejo)",
"main": "src/cli.js", "main": "src/cli.js",
"scripts": { "scripts": {

49
src/cli.js
View file

@ -679,6 +679,20 @@ pr
} }
}); });
// Map CLI review event names (and common Forgejo ReviewStateType tokens) to
// the values this forge accepts on POST .../pulls/{n}/reviews.
const REVIEW_EVENT_MAP = {
approve: 'APPROVED',
approved: 'APPROVED',
'request-changes': 'REQUEST_CHANGES',
request_changes: 'REQUEST_CHANGES',
comment: 'COMMENT',
};
function resolveReviewEvent(raw) {
return REVIEW_EVENT_MAP[String(raw).toLowerCase()] || null;
}
pr pr
.command('show') .command('show')
.description('Show details of a pull request') .description('Show details of a pull request')
@ -690,11 +704,15 @@ pr
const config = loadConfig(); const config = loadConfig();
const client = ForgejoClient.fromConfig(config); const client = ForgejoClient.fromConfig(config);
const prData = await client.getPullRequest(options.owner, options.repo, options.number); const prData = await client.getPullRequest(options.owner, options.repo, options.number);
const author = prData.user?.login || '(unknown)';
const headRef = prData.head?.ref || '?';
const baseRef = prData.base?.ref || '?';
const mergeable = prData.mergeable == null ? 'unknown' : String(prData.mergeable);
console.log(`!${prData.number} [${prData.state}] ${prData.title}`); console.log(`!${prData.number} [${prData.state}] ${prData.title}`);
console.log(`URL: ${prData.html_url}`); console.log(`URL: ${prData.html_url}`);
console.log(`Author: ${prData.user.login}`); console.log(`Author: ${author}`);
console.log(`Branch: ${prData.head.ref} -> ${prData.base.ref}`); console.log(`Branch: ${headRef} -> ${baseRef}`);
console.log(`Mergeable: ${prData.mergeable}`); console.log(`Mergeable: ${mergeable}`);
console.log(`Created: ${prData.created_at}`); console.log(`Created: ${prData.created_at}`);
if (prData.body) { if (prData.body) {
console.log('\n' + prData.body); console.log('\n' + prData.body);
@ -712,8 +730,8 @@ pr
.requiredOption('-o, --owner <owner>', 'repository owner') .requiredOption('-o, --owner <owner>', 'repository owner')
.requiredOption('-r, --repo <repo>', 'repository name') .requiredOption('-r, --repo <repo>', 'repository name')
.requiredOption('-n, --number <number>', 'pull request number', parseId) .requiredOption('-n, --number <number>', 'pull request number', parseId)
.option('-b, --body <body>', 'comment body (markdown)') .option('-b, --body <body>', 'comment body (markdown; required unless --body-file)')
.option('--body-file <path>', 'read the comment body from a file') .option('--body-file <path>', 'read the comment body from a file (wins over -b)')
.action(async (options) => { .action(async (options) => {
try { try {
const config = loadConfig(); const config = loadConfig();
@ -739,22 +757,16 @@ pr
.requiredOption('-o, --owner <owner>', 'repository owner') .requiredOption('-o, --owner <owner>', 'repository owner')
.requiredOption('-r, --repo <repo>', 'repository name') .requiredOption('-r, --repo <repo>', 'repository name')
.requiredOption('-n, --number <number>', 'pull request number', parseId) .requiredOption('-n, --number <number>', 'pull request number', parseId)
.requiredOption('--event <event>', 'review event: approve, request-changes|request_changes, comment') .requiredOption('--event <event>', 'review event: approve|approved, request-changes|request_changes, comment')
.option('-b, --body <body>', 'review body (markdown)') .option('-b, --body <body>', 'review body (markdown; required for request-changes and comment)')
.option('--body-file <path>', 'read the review body from a file') .option('--body-file <path>', 'read the review body from a file (wins over -b)')
.action(async (options) => { .action(async (options) => {
try { try {
const config = loadConfig(); const config = loadConfig();
const client = ForgejoClient.fromConfig(config); const client = ForgejoClient.fromConfig(config);
const eventMap = { const event = resolveReviewEvent(options.event);
approve: 'APPROVED',
'request-changes': 'REQUEST_CHANGES',
request_changes: 'REQUEST_CHANGES',
comment: 'COMMENT',
};
const event = eventMap[options.event.toLowerCase()];
if (!event) { if (!event) {
console.error(`Invalid review event: ${options.event}. Must be approve, request-changes (or request_changes), or comment.`); console.error(`Invalid review event: ${options.event}. Must be approve (or approved), request-changes (or request_changes), or comment.`);
process.exit(1); process.exit(1);
} }
const rawBody = readBodyOption(options) || ''; const rawBody = readBodyOption(options) || '';
@ -762,8 +774,11 @@ pr
console.error(`Review event ${options.event} requires a non-empty body. Use -b/--body or --body-file.`); console.error(`Review event ${options.event} requires a non-empty body. Use -b/--body or --body-file.`);
process.exit(1); process.exit(1);
} }
await client.createPullRequestReview(options.owner, options.repo, options.number, event, rawBody); const result = await client.createPullRequestReview(options.owner, options.repo, options.number, event, rawBody);
console.log(`Review submitted on !${options.number}: ${event}.`); console.log(`Review submitted on !${options.number}: ${event}.`);
if (result && result.html_url) {
console.log(`URL: ${result.html_url}`);
}
} catch (err) { } catch (err) {
console.error(`Failed to submit review: ${err.message}`); console.error(`Failed to submit review: ${err.message}`);
if (err.status) console.error(`HTTP status: ${err.status}`); if (err.status) console.error(`HTTP status: ${err.status}`);

94
test/cli.test.js
View file

@ -101,6 +101,18 @@ test('pr comment rejects a missing body before any network call', () => {
} }
}); });
test('pr comment rejects a whitespace-only body before any network call', () => {
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
try {
const res = run(['pr', 'comment', '-o', 'o', '-r', 'r', '-n', '1', '-b', ' '], { STOKE_CONFIG_FILE: cfg });
assert.equal(res.status, 1);
assert.match(res.stderr, /Comment body is required/);
} finally {
fs.unlinkSync(cfg);
}
});
test('pr review rejects an invalid event before any network call', () => { test('pr review rejects an invalid event before any network call', () => {
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`); const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' })); fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
@ -113,6 +125,20 @@ test('pr review rejects an invalid event before any network call', () => {
} }
}); });
test('pr review accepts approved as an alias for approve before any network call', () => {
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
try {
// Network fails; must not fail at event validation.
const res = run(['pr', 'review', '-o', 'o', '-r', 'r', '-n', '1', '--event', 'APPROVED'], { STOKE_CONFIG_FILE: cfg });
assert.equal(res.status, 1);
assert.doesNotMatch(res.stderr, /Invalid review event/);
assert.doesNotMatch(res.stderr, /requires a non-empty body/);
} finally {
fs.unlinkSync(cfg);
}
});
test('pr review request-changes rejects a missing body before any network call', () => { test('pr review request-changes rejects a missing body before any network call', () => {
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`); const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' })); fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
@ -173,18 +199,29 @@ test('pr review preserves exact body-file whitespace through the CLI boundary',
fs.writeFileSync(bodyFile, rawBody, 'utf8'); fs.writeFileSync(bodyFile, rawBody, 'utf8');
const TIMEOUT_MS = 5000;
let timer;
const captured = await new Promise((resolve, reject) => { const captured = 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) => { const server = http.createServer((req, res) => {
let data = ''; let data = '';
req.setEncoding('utf8'); req.setEncoding('utf8');
req.on('data', (chunk) => { data += chunk; }); req.on('data', (chunk) => { data += chunk; });
req.on('end', () => { req.on('end', () => {
res.writeHead(200, { 'Content-Type': 'application/json' }); res.writeHead(200, { 'Content-Type': 'application/json' });
res.end(JSON.stringify({ id: 99 })); res.end(JSON.stringify({ id: 99, html_url: 'https://forge.test/reviews/99' }));
server.close(() => resolve({ url: req.url, body: data })); clearTimeout(timer);
server.close(() => resolve({ url: req.url, body: data, cliStatus: null }));
}); });
}); });
timer = setTimeout(() => fail(new Error(`CLI boundary test timed out after ${TIMEOUT_MS}ms`)), TIMEOUT_MS);
server.listen(0, '127.0.0.1', async () => { server.listen(0, '127.0.0.1', async () => {
const { port } = server.address(); const { port } = server.address();
fs.writeFileSync(cfg, JSON.stringify({ url: `http://127.0.0.1:${port}`, token: 'tok' })); fs.writeFileSync(cfg, JSON.stringify({ url: `http://127.0.0.1:${port}`, token: 'tok' }));
@ -194,13 +231,16 @@ test('pr review preserves exact body-file whitespace through the CLI boundary',
{ STOKE_CONFIG_FILE: cfg }, { STOKE_CONFIG_FILE: cfg },
); );
if (res.status !== 0) { if (res.status !== 0) {
server.close(() => reject(new Error(`CLI failed: ${res.stderr}`))); fail(new Error(`CLI failed (status ${res.status}): ${res.stderr}`));
return;
} }
// Capture is resolved from the HTTP handler; assert exit 0 here via side channel.
// If the handler already resolved, attach status for the outer asserts.
} catch (err) { } catch (err) {
server.close(() => reject(err)); fail(err);
} }
}); });
}); }).finally(() => clearTimeout(timer));
try { try {
assert.equal(captured.url, '/api/v1/repos/o/r/pulls/7/reviews'); assert.equal(captured.url, '/api/v1/repos/o/r/pulls/7/reviews');
@ -212,3 +252,47 @@ test('pr review preserves exact body-file whitespace through the CLI boundary',
fs.unlinkSync(bodyFile); fs.unlinkSync(bodyFile);
} }
}); });
test('pr review prints the review URL from the API response', async () => {
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}-url.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) => {
let data = '';
req.on('data', (c) => { data += c; });
req.on('end', () => {
res.writeHead(200, { 'Content-Type': 'application/json' });
res.end(JSON.stringify({ id: 42, html_url: 'https://forge.test/pulls/7#issuecomment-42' }));
});
});
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(
['pr', 'review', '-o', 'o', '-r', 'r', '-n', '7', '--event', 'approve', '-b', 'LGTM'],
{ STOKE_CONFIG_FILE: cfg },
);
clearTimeout(timer);
server.close(() => resolve(res));
} catch (err) {
fail(err);
}
});
}).finally(() => clearTimeout(timer));
try {
assert.equal(result.status, 0, result.stderr);
assert.match(result.stdout, /Review submitted on !7: APPROVED/);
assert.match(result.stdout, /URL: https:\/\/forge\.test\/pulls\/7#issuecomment-42/);
} finally {
fs.unlinkSync(cfg);
}
});

4
test/install-apt.test.js
View file

@ -80,7 +80,6 @@ function runScenario({ candInitial, candAfterUpdate, candAfterNodesource, preexi
const read = (p) => (fs.existsSync(p) ? fs.readFileSync(p, 'utf8') : null); const read = (p) => (fs.existsSync(p) ? fs.readFileSync(p, 'utf8') : null);
const mode = (p) => (fs.existsSync(p) ? fs.statSync(p).mode & 0o777 : null); const mode = (p) => (fs.existsSync(p) ? fs.statSync(p).mode & 0o777 : null);
return { return {
res,
aptEtc, aptEtc,
nodesourceList: read(path.join(aptEtc, 'sources.list.d', 'nodesource.list')), nodesourceList: read(path.join(aptEtc, 'sources.list.d', 'nodesource.list')),
nodesourceListMode: mode(path.join(aptEtc, 'sources.list.d', 'nodesource.list')), nodesourceListMode: mode(path.join(aptEtc, 'sources.list.d', 'nodesource.list')),
@ -89,6 +88,9 @@ function runScenario({ candInitial, candAfterUpdate, candAfterNodesource, preexi
forgeKeyMode: mode(path.join(aptEtc, 'keyrings', 'forgejo-heavy-duty.asc')), forgeKeyMode: mode(path.join(aptEtc, 'keyrings', 'forgejo-heavy-duty.asc')),
aptGetLog: read(path.join(state, 'apt-get.log')) || '', aptGetLog: read(path.join(state, 'apt-get.log')) || '',
}; };
// Drop the throwaway tree after we have read everything we need.
fs.rmSync(root, { recursive: true, force: true });
return result;
} }
test('suitable nodejs candidate already available: installs without touching NodeSource', () => { test('suitable nodejs candidate already available: installs without touching NodeSource', () => {