DEVXT-1965 remove users from enterprise - #36
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Node.js CLI utility under utils/github/ent-remove-users to remove members from a GitHub Enterprise based on usernames listed in a CSV file, with a dry-run default and an explicit --remove opt-in for destructive behavior.
Changes:
- Implemented the CLI flow (CSV parsing, confirmation prompt, GraphQL lookups, and enterprise-member removal).
- Added an Octokit factory configured with throttling/retry behavior.
- Added project scaffolding (README, Node engine pinning, dependencies, and ignore rules).
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/github/ent-remove-users/src/index.js | CLI entrypoint: parses args, reads CSV, confirms, and performs (or simulates) removals via GraphQL. |
| utils/github/ent-remove-users/src/octokit.js | Creates a throttled Octokit client and defines rate-limit handling callbacks. |
| utils/github/ent-remove-users/src/logger.js | Introduces a pino logger used across the utility. |
| utils/github/ent-remove-users/README.md | Documents purpose, prerequisites, usage, and CSV format. |
| utils/github/ent-remove-users/package.json | Defines package metadata, scripts, dependencies, and Node engine requirement. |
| utils/github/ent-remove-users/package-lock.json | Locks dependency versions for repeatable installs. |
| utils/github/ent-remove-users/.tool-versions | Pins Node.js toolchain version for local dev tooling. |
| utils/github/ent-remove-users/.gitignore | Ignores logs, CSVs, and node_modules for the utility directory. |
Files not reviewed (1)
- utils/github/ent-remove-users/package-lock.json: Generated file
Suppressed comments (1)
utils/github/ent-remove-users/src/index.js:187
pinologgers don't reliably log additional arguments;logger.error('...', error)will typically drop theerrorobject/stack. Use pino’s error logging signature so the exception details are recorded.
logger.error('An unexpected error occurred:', error);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- utils/github/ent-remove-users/package-lock.json: Generated file
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
utils/github/ent-remove-users/src/index.js:170
- The
--filevalidation usesawait fs.stat(file)inside theifcondition. If the path does not exist or is unreadable,fs.statthrows and the script will crash before printing the intended error message / settingprocess.exitCode.
Handle the fs.stat failure explicitly and treat it as an invalid file.
if (!file || !(await fs.stat(file)).isFile()) {
console.error('Error: --file must be specified and must be a valid file.');
process.exitCode = 1;
return;
}
utils/github/ent-remove-users/src/index.js:158
parseArgs(...)can throw (e.g., unknown flag, missing value whenstrict: true). Because this runs before the maintry/catch, the script will exit with a stack trace instead of a user-friendly message.
Wrap argument parsing in a try/catch and exit cleanly on invalid CLI input.
const {
values,
} = parseArgs({ options, strict: true, allowPositionals: false });
utils/github/ent-remove-users/src/index.js:191
logger.error('An unexpected error occurred:', error)passes the Error as a second formatting argument, which typically loses structured error fields/stack in pino output.
Log the error as an err object so the stack trace is preserved.
} catch (error) {
logger.error('An unexpected error occurred:', error);
process.exitCode = 1;
}
This PR does the following: