fix: add parentheses to JSDoc type cast in copilot_driver.cjs for TypeScript compatibility#25406
Conversation
…tions-workflow Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…eScript compatibility Agent-Logs-Url: https://github.com/github/gh-aw/sessions/34f7e8b3-df09-41bc-b786-8bb4b22ebb7e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes TypeScript tsc --noEmit failure in actions/setup/js/copilot_driver.cjs by adjusting a JSDoc type cast so TypeScript correctly infers NodeJS.ErrnoException, and adds additional debug logging in both the JS driver and several CLI Go packages.
Changes:
- Add parentheses (and
// prettier-ignore) to the JSDoc cast incopilot_driver.cjsso TS sees.code/.syscall. - Add JS-driver diagnostics (startup fields, pre-flight command accessibility check, richer spawn error/no-output messages) and accompanying tests.
- Add DEBUG-gated logging to multiple Go CLI helpers (workflow discovery/suggestions, manifest cache writes, firewall policy rule matching, advisory matching, audit log parsing).
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/workflows.go | Adds debug logs around workflow name suggestion and markdown workflow file scanning. |
| pkg/cli/mcp_safe_update_cache.go | Adds debug logs when writing the prior manifest cache temp file. |
| pkg/cli/firewall_policy.go | Adds detailed debug logs for rule matching decisions. |
| pkg/cli/deps_security.go | Adds debug logs for advisory API query and dependency/advisory matches. |
| pkg/cli/audit.go | Adds debug logs for step-output extraction and failing-step discovery. |
| actions/setup/js/copilot_driver.test.cjs | Adds tests around new/changed log message formats. |
| actions/setup/js/copilot_driver.cjs | Fixes TS JSDoc cast for errno fields; adds new pre-flight check and expanded logging. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 3
| 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)`); |
There was a problem hiding this comment.
checkCommandAccessible() uses fs.promises.access(command, ...) on the raw command string. This only works when command is an actual filesystem path; if callers pass a command name resolved via PATH (e.g. copilot), this will incorrectly log “command not found” even though spawning would succeed. Consider either (1) updating the contract/docs to require a path (and rename to commandPath), or (2) resolving PATH (or skipping the access check when the command has no path separators).
| const { spawn } = require("child_process"); | ||
| const fs = require("fs"); | ||
|
|
||
| // Maximum number of retry attempts after the initial run | ||
| const MAX_RETRIES = 3; |
There was a problem hiding this comment.
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.
| 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="); |
There was a problem hiding this comment.
These new tests only assert against strings constructed within the test itself, so they won’t fail if the implementation’s log format changes (they don’t exercise copilot_driver.cjs). If the intent is regression protection, consider refactoring the driver to expose pure helpers (e.g., log-message builders) or making main() opt-in so tests can import the module and assert real output formatting/behavior.
See below for a potential fix:
…25366) * Initial plan * Initial plan for CLI proxy: start difc-proxy on host, pass new AWF flags Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cd08abe8-65f6-4cd4-aca7-a2cfa59d7e81 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * feat: replace --enable-cli-proxy with --difc-proxy-host, start difc-proxy on host When features.cli-proxy is enabled, the compiler now: 1. Starts a difc-proxy container on the host before AWF execution 2. Passes --difc-proxy-host host.docker.internal:18443 and --difc-proxy-ca-cert /tmp/gh-aw/difc-proxy-tls/ca.crt to AWF 3. Injects GH_TOKEN into the AWF step env with --exclude-env GH_TOKEN 4. Stops the CLI proxy container after AWF execution Removed deprecated flags: --enable-cli-proxy, --cli-proxy-policy. Minimum AWF version bumped to v0.26.0 for CLI proxy support. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cd08abe8-65f6-4cd4-aca7-a2cfa59d7e81 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: address code review feedback for CLI proxy - Handle empty policy gracefully in start_cli_proxy.sh (proxy starts without guard filtering when no policy is configured) - Exit with error when proxy fails to start (prevents AWF from running with a non-functional proxy) - Rename hasCliProxyNeeded to isCliProxyNeeded for naming consistency Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cd08abe8-65f6-4cd4-aca7-a2cfa59d7e81 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: address review feedback and recompile agentic workflows - Bump DefaultFirewallVersion to v0.26.0 to align with AWFCliProxyMinVersion - Gate addCliProxyGHTokenToEnv on awfSupportsCliProxy and awfSupportsExcludeEnv to prevent leaking GH_TOKEN into the agent container on older AWF versions - Make start_cli_proxy.sh idempotent by removing any leftover container first - Update changeset to describe current behavior (difc-proxy-host flags) - Recompile all agentic workflows with updated DefaultFirewallVersion Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e59645aa-2981-470c-bd44-1075fd88317a Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com> * fix: update golden files and lock file for AWF v0.26.0 version bump (#25400) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f16055db-4d7a-479e-acae-0713caf5344d Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: update DefaultFirewallVersion to v0.25.17, fix shell quoting and docstring Agent-Logs-Url: https://github.com/github/gh-aw/sessions/35642b32-32d1-4a2d-bea7-8041bed78e77 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: add parentheses to JSDoc type cast in copilot_driver.cjs for TypeScript compatibility (#25406) * feat: increase logging in copilot driver for silent startup failures (#issue) (#25390) * feat(logging): add debug logging to 5 CLI files for improved troubleshooting (#25393) * fix: add parentheses to JSDoc type cast in copilot_driver.cjs for TypeScript compatibility Agent-Logs-Url: https://github.com/github/gh-aw/sessions/34f7e8b3-df09-41bc-b786-8bb4b22ebb7e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Landon Cox <landon.cox@microsoft.com>
Summary
Fixes the TypeScript
tsc --noEmitfailure in thejsCI workflow for PR #25366.Check run: https://github.com/github/gh-aw/actions/runs/24161653431/job/70514208958
Root Cause
The
mainbranch (commit2990e99) added errno/code/syscall handling to thechild.on("error")handler incopilot_driver.cjswith a JSDoc type cast:TypeScript requires parentheses around the expression being cast:
Without parentheses,
errnois inferred asError(the callback parameter type), which lacks.codeand.syscallproperties, causing:Additionally, Prettier strips the parentheses from JSDoc type casts during formatting, so a
// prettier-ignorecomment is needed to preserve them.Changes
// prettier-ignorecomment and parentheses to the JSDoc type castmaininto the branch to bring in the code that needs fixing