Skip to content

fix(copilot): make copilotcli hook import platform-independent and preserve cwd on copilot import - #2635

Merged
dyoshikawa merged 2 commits into
mainfrom
resolve-issue-2622-copilot-hooks-asymmetries
Aug 8, 2026
Merged

fix(copilot): make copilotcli hook import platform-independent and preserve cwd on copilot import#2635
dyoshikawa merged 2 commits into
mainfrom
resolve-issue-2622-copilot-hooks-asymmetries

Conversation

@dyoshikawa

Copy link
Copy Markdown
Owner

Two hooks asymmetries between the copilot and copilotcli adapters, where each file already contained the correct implementation of the other's bug.

Fix A — copilotcli import was machine-dependent

resolveImportCommand in src/features/hooks/copilotcli-hooks.ts tie-broke a hook entry carrying both bash and powershell on process.platform === "win32". Importing the same copilotcli-hooks.json therefore produced a different rulesync hooks file depending on which contributor ran the import — churn for anyone who checks the result in.

Ported the fix already applied to the copilot sibling in #2621: always take { command: entry.bash, shell: "bash" } and warn naming the ignored powershell, with the warning worded to match the sibling's.

Fix B — copilot import dropped documented keys

copilotHooksToCanonical in src/features/hooks/copilot-hooks.ts rebuilt each canonical definition from only command/shell/env/timeout. Generate re-emits any non-canonical key verbatim through rest, so a documented field like cwd survived generate but was destroyed by an import → regenerate cycle.

Mirrored the importPassthrough helper from copilotcli-hooks.ts:289, preserving cwd and env. No canonical schema change was needed — HookDefinitionSchema is a z.looseObject. url/headers/allowedEnvVars are deliberately not preserved here: unlike the CLI, the copilot generate path emits only command hooks, so those keys have no round trip to protect.

Fix C — stale doc comment

The comment on the command field at copilotcli-hooks.ts:122-124 claimed the portable field is "import-only here — generate always writes the platform-specific field". Generate has keyed off the canonical shell selector since #2621 (const commandField = def.shell ?? "command";), so it does write the portable field whenever shell is unset. Corrected.

Also aligned

copilot import read only timeoutSec and ignored the documented timeout alias that copilotcli already accepts. Both spellings are now read, with timeoutSec winning when both are present — same precedence as the sibling.

Tests

Round-trip cases in both .test.ts files, including the cross-platform-identical import assertion introduced by #2621 (process.platform mocked to win32, same canonical output expected).

Verification

  • pnpm cicheck
  • npx vitest run --config vitest.e2e.config.ts src/e2e/e2e-hooks.spec.ts

Closes #2622
Part of #2401 (the two hooks asymmetries recorded in its 2026-08-07 comment)

…e cwd

- copilotcli: when a hook entry carries both bash and powershell, always take
  bash instead of tie-breaking on process.platform, so importing the same file
  yields the same canonical config on every machine (matches the copilot fix
  from #2621).
- copilot: import dropped documented non-command keys such as cwd, which
  generate re-emits through rest, so an import then regenerate destroyed them.
  Preserve them via an importPassthrough helper mirroring the copilotcli
  sibling, and read the timeout alias the sibling already accepts.
- copilotcli: correct a doc comment that claimed generate never writes the
  portable command field.

Closes #2622
@dyoshikawa
dyoshikawa merged commit a936799 into main Aug 8, 2026
9 checks passed
@dyoshikawa
dyoshikawa deleted the resolve-issue-2622-copilot-hooks-asymmetries branch August 8, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

copilot/copilotcli hooks: platform-dependent import tie-break and passthrough-key loss asymmetries

2 participants