-
Notifications
You must be signed in to change notification settings - Fork 344
fix: add parentheses to JSDoc type cast in copilot_driver.cjs for TypeScript compatibility #25406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2990e99
518fec6
402879a
6af6202
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
| "use strict"; | ||
|
|
||
| const { spawn } = require("child_process"); | ||
| const fs = require("fs"); | ||
|
|
||
| // Maximum number of retry attempts after the initial run | ||
| const MAX_RETRIES = 3; | ||
|
|
@@ -67,6 +68,29 @@ function sleep(ms) { | |
| return new Promise(resolve => setTimeout(resolve, ms)); | ||
| } | ||
|
|
||
| /** | ||
| * Check whether a command path is accessible and executable, logging the result. | ||
| * Returns true if the command is usable, false otherwise. | ||
| * @param {string} command - Absolute or relative path to the executable | ||
| * @returns {Promise<boolean>} | ||
| */ | ||
| async function checkCommandAccessible(command) { | ||
| try { | ||
| await fs.promises.access(command, fs.constants.F_OK); | ||
| } catch { | ||
| log(`pre-flight: command not found: ${command} (F_OK check failed — binary does not exist at this path)`); | ||
|
Comment on lines
+77
to
+81
|
||
| return false; | ||
| } | ||
| try { | ||
| await fs.promises.access(command, fs.constants.X_OK); | ||
| log(`pre-flight: command is accessible and executable: ${command}`); | ||
| return true; | ||
| } catch { | ||
| log(`pre-flight: command exists but is not executable: ${command} (X_OK check failed — permission denied)`); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Format elapsed milliseconds as a human-readable string (e.g. "3m 12s"). | ||
| * @param {number} ms | ||
|
|
@@ -148,7 +172,11 @@ function runProcess(command, args, attempt) { | |
|
|
||
| child.on("error", err => { | ||
| const durationMs = Date.now() - startTime; | ||
| log(`attempt ${attempt + 1}: failed to start process '${command}': ${err.message}`); | ||
| // prettier-ignore | ||
| const errno = /** @type {NodeJS.ErrnoException} */ (err); | ||
| const errCode = errno.code ?? "unknown"; | ||
| const errSyscall = errno.syscall ?? "unknown"; | ||
| log(`attempt ${attempt + 1}: failed to start process '${command}': ${err.message}` + ` (code=${errCode} syscall=${errSyscall})`); | ||
| resolve({ | ||
| exitCode: 1, | ||
| output: collectedOutput, | ||
|
|
@@ -170,7 +198,9 @@ async function main() { | |
| process.exit(1); | ||
| } | ||
|
|
||
| log(`starting: command=${command} maxRetries=${MAX_RETRIES} initialDelayMs=${INITIAL_DELAY_MS} backoffMultiplier=${BACKOFF_MULTIPLIER} maxDelayMs=${MAX_DELAY_MS}`); | ||
| log(`starting: command=${command} maxRetries=${MAX_RETRIES} initialDelayMs=${INITIAL_DELAY_MS}` + ` backoffMultiplier=${BACKOFF_MULTIPLIER} maxDelayMs=${MAX_DELAY_MS}` + ` nodeVersion=${process.version} platform=${process.platform}`); | ||
|
|
||
| await checkCommandAccessible(command); | ||
|
|
||
| let delay = INITIAL_DELAY_MS; | ||
| let lastExitCode = 1; | ||
|
|
@@ -212,7 +242,7 @@ async function main() { | |
| if (attempt >= MAX_RETRIES) { | ||
| log(`all ${MAX_RETRIES} retries exhausted — giving up (exitCode=${lastExitCode})`); | ||
| } else { | ||
| log(`attempt ${attempt + 1}: no output produced — not retrying (process may have failed to start)`); | ||
| log(`attempt ${attempt + 1}: no output produced — not retrying` + ` (possible causes: binary not found, permission denied, auth failure, or silent startup crash)`); | ||
| } | ||
|
|
||
| // Non-retryable error or retries exhausted — propagate exit code | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,4 +153,35 @@ describe("copilot_driver.cjs", () => { | |
| expect(logLine).toMatch(/^\[copilot-driver\] \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/); | ||
| }); | ||
| }); | ||
|
|
||
| describe("startup log includes node version and platform", () => { | ||
| it("starting log line contains nodeVersion and platform fields", () => { | ||
| const command = "/usr/local/bin/copilot"; | ||
| const startingLine = `starting: command=${command} maxRetries=3 initialDelayMs=5000` + ` backoffMultiplier=2 maxDelayMs=60000` + ` nodeVersion=${process.version} platform=${process.platform}`; | ||
| expect(startingLine).toContain("nodeVersion="); | ||
| expect(startingLine).toContain("platform="); | ||
|
Comment on lines
+158
to
+162
|
||
| expect(startingLine).toMatch(/nodeVersion=v\d+\.\d+/); | ||
| }); | ||
| }); | ||
|
|
||
| describe("no-output failure message", () => { | ||
| it("includes actionable possible causes", () => { | ||
| const msg = `attempt 1: no output produced — not retrying` + ` (possible causes: binary not found, permission denied, auth failure, or silent startup crash)`; | ||
| expect(msg).toContain("binary not found"); | ||
| expect(msg).toContain("permission denied"); | ||
| expect(msg).toContain("auth failure"); | ||
| expect(msg).toContain("silent startup crash"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("error event message", () => { | ||
| it("includes code and syscall fields", () => { | ||
| const errMessage = "spawn /usr/local/bin/copilot ENOENT"; | ||
| const errCode = "ENOENT"; | ||
| const errSyscall = "spawn"; | ||
| const logMsg = `attempt 1: failed to start process '/usr/local/bin/copilot': ${errMessage}` + ` (code=${errCode} syscall=${errSyscall})`; | ||
| expect(logMsg).toContain("code=ENOENT"); | ||
| expect(logMsg).toContain("syscall=spawn"); | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR description/title focus on the JSDoc cast parentheses/Prettier interaction, but this PR also adds a new pre-flight
fs-based accessibility check plus changes to startup/no-output logging. Please either update the PR description to cover these additional behavior changes (and motivation), or split them into a separate PR to keep the fix scoped.