Address PR #4 review feedback
- pr comment now rejects whitespace-only bodies and preserves raw body. - pr review accepts request_changes alias in addition to request-changes. - Add CLI-boundary regression test proving review body-file whitespace is preserved through the CLI and sent byte-for-byte to the API. - Update README option help text for the new alias.
This commit is contained in:
kimi-reviewer-andresmgsl
parent
d56e509649
commit
d88cb484d3
3 changed files with 72 additions and 7 deletions
|
|
@ -483,7 +483,7 @@ 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, comment (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
|
||||
```
|
||||
|
|
|
|||
11
src/cli.js
11
src/cli.js
|
|
@ -718,12 +718,12 @@ pr
|
|||
try {
|
||||
const config = loadConfig();
|
||||
const client = ForgejoClient.fromConfig(config);
|
||||
const body = readBodyOption(options);
|
||||
if (!body) {
|
||||
const rawBody = readBodyOption(options) || '';
|
||||
if (rawBody.trim().length === 0) {
|
||||
console.error('Comment body is required. Use -b/--body or --body-file.');
|
||||
process.exit(1);
|
||||
}
|
||||
const result = await client.createPullRequestComment(options.owner, options.repo, options.number, body);
|
||||
const result = await client.createPullRequestComment(options.owner, options.repo, options.number, rawBody);
|
||||
console.log(`Comment added to !${options.number}.`);
|
||||
console.log(`URL: ${result.html_url}`);
|
||||
} catch (err) {
|
||||
|
|
@ -739,7 +739,7 @@ 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, comment')
|
||||
.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')
|
||||
.action(async (options) => {
|
||||
|
|
@ -749,11 +749,12 @@ pr
|
|||
const eventMap = {
|
||||
approve: 'APPROVED',
|
||||
'request-changes': 'REQUEST_CHANGES',
|
||||
request_changes: 'REQUEST_CHANGES',
|
||||
comment: 'COMMENT',
|
||||
};
|
||||
const event = eventMap[options.event.toLowerCase()];
|
||||
if (!event) {
|
||||
console.error(`Invalid review event: ${options.event}. Must be approve, request-changes, or comment.`);
|
||||
console.error(`Invalid review event: ${options.event}. Must be approve, request-changes (or request_changes), or comment.`);
|
||||
process.exit(1);
|
||||
}
|
||||
const rawBody = readBodyOption(options) || '';
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const { execFileSync, spawnSync } = require('node:child_process');
|
||||
const { execFileSync, spawn, spawnSync } = require('node:child_process');
|
||||
const fs = require('node:fs');
|
||||
const http = require('node:http');
|
||||
const os = require('node:os');
|
||||
const path = require('node:path');
|
||||
|
||||
|
|
@ -148,3 +149,66 @@ test('pr review approve allows an empty body before any network call', () => {
|
|||
fs.unlinkSync(cfg);
|
||||
}
|
||||
});
|
||||
|
||||
function spawnAsync(args, env = {}) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const child = spawn(process.execPath, [CLI, ...args], {
|
||||
env: { ...process.env, ...env },
|
||||
});
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
child.stdout.setEncoding('utf8');
|
||||
child.stderr.setEncoding('utf8');
|
||||
child.stdout.on('data', (chunk) => { stdout += chunk; });
|
||||
child.stderr.on('data', (chunk) => { stderr += chunk; });
|
||||
child.on('error', reject);
|
||||
child.on('close', (status) => resolve({ status, stdout, stderr }));
|
||||
});
|
||||
}
|
||||
|
||||
test('pr review preserves exact body-file whitespace through the CLI boundary', async () => {
|
||||
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
|
||||
const bodyFile = path.join(os.tmpdir(), `stoke-body-${process.pid}.md`);
|
||||
const rawBody = ' leading spaces\nline\ntrailing newline\n';
|
||||
|
||||
fs.writeFileSync(bodyFile, rawBody, 'utf8');
|
||||
|
||||
const captured = await new Promise((resolve, reject) => {
|
||||
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 }));
|
||||
});
|
||||
});
|
||||
|
||||
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', 'request-changes', '--body-file', bodyFile],
|
||||
{ STOKE_CONFIG_FILE: cfg },
|
||||
);
|
||||
if (res.status !== 0) {
|
||||
server.close(() => reject(new Error(`CLI failed: ${res.stderr}`)));
|
||||
}
|
||||
} catch (err) {
|
||||
server.close(() => reject(err));
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
try {
|
||||
assert.equal(captured.url, '/api/v1/repos/o/r/pulls/7/reviews');
|
||||
const json = JSON.parse(captured.body);
|
||||
assert.equal(json.event, 'REQUEST_CHANGES');
|
||||
assert.equal(json.body, rawBody);
|
||||
} finally {
|
||||
fs.unlinkSync(cfg);
|
||||
fs.unlinkSync(bodyFile);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue