forked from heavy-duty/stoke
Merge pull request 'Harden install-apt Node bootstrap and polish pr review CLI (v1.2.1)' (#6) from improve/cli-and-install-hardening into main
This commit is contained in:
commit
92a6741e4d
6 changed files with 137 additions and 34 deletions
17
README.md
17
README.md
|
|
@ -471,15 +471,15 @@ Calls `GET /api/v1/repos/{owner}/{repo}/pulls/{number}`.
|
|||
|
||||
### `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
|
||||
Options:
|
||||
-o, --owner <owner> repository owner (required)
|
||||
-r, --repo <repo> repository name (required)
|
||||
-n, --number <number> pull request number (required)
|
||||
-b, --body <body> comment body (markdown)
|
||||
--body-file <path> read the comment body from a file
|
||||
-b, --body <body> comment body (markdown; required unless --body-file)
|
||||
--body-file <path> read the comment body from a file (wins over -b)
|
||||
```
|
||||
|
||||
```bash
|
||||
|
|
@ -490,23 +490,24 @@ Calls `POST /api/v1/repos/{owner}/{repo}/issues/{number}/comments`.
|
|||
|
||||
### `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
|
||||
Options:
|
||||
-o, --owner <owner> repository owner (required)
|
||||
-r, --repo <repo> repository name (required)
|
||||
-n, --number <number> pull request number (required)
|
||||
--event <event> review event: approve, request-changes|request_changes, comment (required)
|
||||
-b, --body <body> review body (markdown)
|
||||
--body-file <path> read the review body from a file
|
||||
--event <event> approve|approved, request-changes|request_changes, comment (required)
|
||||
-b, --body <body> review body (markdown; required for request-changes and comment)
|
||||
--body-file <path> read the review body from a file (wins over -b)
|
||||
```
|
||||
|
||||
```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 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`
|
||||
|
||||
|
|
|
|||
4
package-lock.json
generated
4
package-lock.json
generated
|
|
@ -1,12 +1,12 @@
|
|||
{
|
||||
"name": "stoke",
|
||||
"version": "1.2.0",
|
||||
"version": "1.2.1",
|
||||
"lockfileVersion": 3,
|
||||
"requires": true,
|
||||
"packages": {
|
||||
"": {
|
||||
"name": "stoke",
|
||||
"version": "1.2.0",
|
||||
"version": "1.2.1",
|
||||
"license": "ISC",
|
||||
"dependencies": {
|
||||
"commander": "^15.0.0"
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "stoke",
|
||||
"version": "1.2.0",
|
||||
"version": "1.2.1",
|
||||
"description": "CLI for the heavy-duty forge (Forgejo)",
|
||||
"main": "src/cli.js",
|
||||
"scripts": {
|
||||
|
|
|
|||
49
src/cli.js
49
src/cli.js
|
|
@ -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
|
||||
.command('show')
|
||||
.description('Show details of a pull request')
|
||||
|
|
@ -690,11 +704,15 @@ pr
|
|||
const config = loadConfig();
|
||||
const client = ForgejoClient.fromConfig(config);
|
||||
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(`URL: ${prData.html_url}`);
|
||||
console.log(`Author: ${prData.user.login}`);
|
||||
console.log(`Branch: ${prData.head.ref} -> ${prData.base.ref}`);
|
||||
console.log(`Mergeable: ${prData.mergeable}`);
|
||||
console.log(`Author: ${author}`);
|
||||
console.log(`Branch: ${headRef} -> ${baseRef}`);
|
||||
console.log(`Mergeable: ${mergeable}`);
|
||||
console.log(`Created: ${prData.created_at}`);
|
||||
if (prData.body) {
|
||||
console.log('\n' + prData.body);
|
||||
|
|
@ -712,8 +730,8 @@ pr
|
|||
.requiredOption('-o, --owner <owner>', 'repository owner')
|
||||
.requiredOption('-r, --repo <repo>', 'repository name')
|
||||
.requiredOption('-n, --number <number>', 'pull request number', parseId)
|
||||
.option('-b, --body <body>', 'comment body (markdown)')
|
||||
.option('--body-file <path>', 'read the comment body from a file')
|
||||
.option('-b, --body <body>', 'comment body (markdown; required unless --body-file)')
|
||||
.option('--body-file <path>', 'read the comment body from a file (wins over -b)')
|
||||
.action(async (options) => {
|
||||
try {
|
||||
const config = loadConfig();
|
||||
|
|
@ -739,22 +757,16 @@ pr
|
|||
.requiredOption('-o, --owner <owner>', 'repository owner')
|
||||
.requiredOption('-r, --repo <repo>', 'repository name')
|
||||
.requiredOption('-n, --number <number>', 'pull request number', parseId)
|
||||
.requiredOption('--event <event>', 'review event: approve, request-changes|request_changes, comment')
|
||||
.option('-b, --body <body>', 'review body (markdown)')
|
||||
.option('--body-file <path>', 'read the review body from a file')
|
||||
.requiredOption('--event <event>', 'review event: approve|approved, request-changes|request_changes, comment')
|
||||
.option('-b, --body <body>', 'review body (markdown; required for request-changes and comment)')
|
||||
.option('--body-file <path>', 'read the review body from a file (wins over -b)')
|
||||
.action(async (options) => {
|
||||
try {
|
||||
const config = loadConfig();
|
||||
const client = ForgejoClient.fromConfig(config);
|
||||
const eventMap = {
|
||||
approve: 'APPROVED',
|
||||
'request-changes': 'REQUEST_CHANGES',
|
||||
request_changes: 'REQUEST_CHANGES',
|
||||
comment: 'COMMENT',
|
||||
};
|
||||
const event = eventMap[options.event.toLowerCase()];
|
||||
const event = resolveReviewEvent(options.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);
|
||||
}
|
||||
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.`);
|
||||
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}.`);
|
||||
if (result && result.html_url) {
|
||||
console.log(`URL: ${result.html_url}`);
|
||||
}
|
||||
} catch (err) {
|
||||
console.error(`Failed to submit review: ${err.message}`);
|
||||
if (err.status) console.error(`HTTP status: ${err.status}`);
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
|
||||
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', () => {
|
||||
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
|
||||
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');
|
||||
|
||||
const TIMEOUT_MS = 5000;
|
||||
let timer;
|
||||
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) => {
|
||||
let data = '';
|
||||
req.setEncoding('utf8');
|
||||
req.on('data', (chunk) => { data += chunk; });
|
||||
req.on('end', () => {
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.end(JSON.stringify({ id: 99 }));
|
||||
server.close(() => resolve({ url: req.url, body: data }));
|
||||
res.end(JSON.stringify({ id: 99, html_url: 'https://forge.test/reviews/99' }));
|
||||
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 () => {
|
||||
const { port } = server.address();
|
||||
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 },
|
||||
);
|
||||
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) {
|
||||
server.close(() => reject(err));
|
||||
fail(err);
|
||||
}
|
||||
});
|
||||
});
|
||||
}).finally(() => clearTimeout(timer));
|
||||
|
||||
try {
|
||||
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);
|
||||
}
|
||||
});
|
||||
|
||||
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);
|
||||
}
|
||||
});
|
||||
|
|
@ -79,7 +79,7 @@ function runScenario({ candInitial, candAfterUpdate, candAfterNodesource, preexi
|
|||
|
||||
const read = (p) => (fs.existsSync(p) ? fs.readFileSync(p, 'utf8') : null);
|
||||
const mode = (p) => (fs.existsSync(p) ? fs.statSync(p).mode & 0o777 : null);
|
||||
return {
|
||||
const result = {
|
||||
res,
|
||||
aptEtc,
|
||||
nodesourceList: read(path.join(aptEtc, 'sources.list.d', 'nodesource.list')),
|
||||
|
|
@ -89,6 +89,9 @@ function runScenario({ candInitial, candAfterUpdate, candAfterNodesource, preexi
|
|||
forgeKeyMode: mode(path.join(aptEtc, 'keyrings', 'forgejo-heavy-duty.asc')),
|
||||
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', () => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue