Skip to content

fix(hooks): keep the Copilot skip guard text fixed across bundler ren… - #1357

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
arnavsingh0418:fix/copilot-guard-stable-text
Oct 24, 2026
Merged

jeff-r2026 merged 1 commit into
Tencent:mainfrom
arnavsingh0418:fix/copilot-guard-stable-text

Conversation

@arnavsingh0418

Copy link
Copy Markdown
Contributor

Summary

The Copilot skip guard embedded copilotRunsEntries.toString(), so every Claude team hook command (and its hash) was whatever the bundler printed for that function. When another module in the chunk declares a top-level entries, esbuild renames the local to entries2 and every hook changes even though none did. The guard now embeds a fixed string, COPILOT_RUNS_ENTRIES_SOURCE, that is byte-identical to what the current build emits, so existing hooks and hashes stay unchanged.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature causing existing behavior to change)
  • Documentation only
  • Refactor / internal cleanup

Test Plan

  • npx tsc --noEmit passes
  • npm run lint passes
  • npx vitest run passes: I ran the related files only. claude-hook-host.test.ts (13) and hooks-team.test.ts pass. hook-dispatch-cli.test.ts (5) and hooks-reconcile-scope.test.ts (2) have Windows-only failures and timeouts that fail the same way on unmodified main. I'm leaving the full run to CI.
  • Added/updated tests for the change: new src/__tests__/claude-hook-host.test.ts runs the guard's copy and copilotRunsEntries on the same 13 inputs (version ≠ 1, disableAllHooks, non-object/non-array shapes, non-command hooks, null entries, every command field, non-string commands, several events) and expects the same entries.

Reproduced the rename. I added a throwaway export async function entries() to src/hooks.ts, ran npm run build, and inspected dist/cli.js:

bundled copilotRunsEntries guard text
main renamed to entries2 built from toString(), gets entries2 ❌
this PR renamed to entries2 fixed string, still [event, entries] ✅

The throwaway change is not part of this PR.

Byte-identical check. COPILOT_RUNS_ENTRIES_SOURCE equals the function text in dist/cli.js built from main at d790b2c.

Real CLI (after npm run build, Windows 11 / Node 24):

  • npm run test:e2e -- src/__tests__/e2e/scope-isolation-issue85.test.ts: 7 passed.
  • hooks-project-isolation-issue373.test.ts: 29 passed, 1 failed. injects both projects, gates execution by cwd… fails at its sh -c step and fails the same way on unmodified main here (Windows sh/$PWD).
  • hooks-team-e2e.test.ts (e2e config): 6 passed, 1 failed. lists and removes standalone Copilot hooks… expects ~/… and gets ~\…, and also fails on unmodified main here.

Only Windows was available to me. Other providers and agents are left to CI.

Related Issues

Fixes #1356

Notes for Reviewers

  • feat(scorers): judge workflow steps with team scorers #1297 merged without the colliding name, so main is fine today. But any future top-level entries (or event, entry, command, file) in the chunk would bring the bug back. This PR removes that hazard.
  • The trade-off is two copies of the predicate (the TypeScript function and the string). The new test fails if they ever behave differently.
  • No other hook rendering uses Function.prototype.toString(). I grepped src/ for .toString(), and the remaining uses are on Buffers, URLs and YAML docs.

…ames

Fixes Tencent#1356

Signed-off-by: Arnav Singh <arnavsingh08@outlook.com>
@github-actions

Copy link
Copy Markdown

No findings in the current PR diff.

  • The fixed guard source preserves the predicate’s logic and removes dependence on bundler-renamed locals.
  • The PR description provides sufficient testing: a post-build real-CLI run with 7 passing tests. The documented Windows limitations and CI-deferred coverage are not blocking.
  • No earlier findings were supplied.

The shallow checkout prevented the literal three-dot diff. Commit metadata identifies d790b2c3 as both commits’ parent; I reviewed only its two-file diff to the PR head. No PR code was run, built, or installed.

@jeff-r2026 jeff-r2026 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified on top of main fb17c48 (together with #1358): tsc, lint, build pass; related tests 157/157. Built main and this head separately and compared the guard source embedded in the bundle: byte-identical (486 bytes), so existing hooks and hashes stay unchanged.

@jeff-r2026
jeff-r2026 merged commit afdfe35 into Tencent:main Oct 24, 2026
18 checks passed
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.

fix(hooks): the Copilot skip guard changes with bundler renames

2 participants