Skip to content

fix(copilot): map edits/urls approvals, stop clobbering mcp.json inputs/sandbox, add the user-scope hooks file - #2467

Merged
dyoshikawa merged 2 commits into
mainfrom
resolve-scrap-issue-2401-copilot
Jul 29, 2026
Merged

fix(copilot): map edits/urls approvals, stop clobbering mcp.json inputs/sandbox, add the user-scope hooks file#2467
dyoshikawa merged 2 commits into
mainfrom
resolve-scrap-issue-2401-copilot

Conversation

@dyoshikawa

Copy link
Copy Markdown
Owner

Background

Addresses gaps 1-4 and 6 of #2401. Gaps 5 and 7 are deliberately left open, so this PR does not close the issue.

Gaps 1-2 — chat.tools.edits.autoApprove / chat.tools.urls.autoApprove

The adapter was hard-coded to a single category (bash → the terminal map). It now drives three flat dotted keys from three canonical categories, all sharing the same pattern-to-boolean shape and the same allowtrue / denyfalse / ask → omitted semantics:

canonical VS Code setting
bash chat.tools.terminal.autoApprove
edit chat.tools.edits.autoApprove
webfetch chat.tools.urls.autoApprove

read has no VS Code approval surface. write is deliberately not folded into the edits map alongside edit — doing so would make the two indistinguishable on import, the same lossiness the Antigravity adapter documents for its write_file collapse. VS Code's per-URL {approveRequest, approveResponse} object form has no canonical action, so it is skipped on import; since rulesync owns the key outright it is replaced when the canonical config carries any webfetch rule, which is the same ownership rule the terminal map already had.

The two new keys are registered in the .vscode/settings.json ownedKeys declaration — the shared-config gateway rejected the write until they were, which is the guard working as intended.

Gap 3 — .vscode/mcp.json no longer clobbers inputs / sandbox

fromRulesyncMcp never read the existing file: it built { servers } and wrote the whole document, so a hand-authored inputs or sandbox section was deleted on the next generate. VS Code recommends committing this file, so the loss was user-visible — once ${input:…} becomes unresolvable, the affected servers fail to start.

It now reads the file first and replaces only servers, preserving inputs, sandbox and any future top-level section (the ClaudecodeMcp pattern). An existing file that cannot be parsed aborts with an error rather than being overwritten, so a malformed file never silently loses its contents.

Gap 4 — user-scope hooks at ~/.copilot/hooks

CopilotHooks.getSettablePaths() ignored its global option, fromRulesyncHooks never read it, and the processor declared supportsGlobal: false, so a global run emitted no Copilot hooks at all. The VS Code hooks doc and the coding-agent hooks doc both document ~/.copilot/hooks as the user scope and state that every *.json in the folder is loaded. Global mode now writes ~/.copilot/hooks/copilot-ide-hooks.json — a distinct name, because the Copilot CLI's global hooks file already occupies copilot-hooks.json in that same folder.

Gap 6 — name frontmatter on *.instructions.md

Added to CopilotRuleFrontmatterSchema and to the copilot section of RulesyncRuleFrontmatter, carried through in both directions alongside excludeAgent.

Gaps 5 and 7 — left open

Gap 5 (preCompact / subagentStart): VS Code's side is now confirmed — the hooks doc lists exactly eight events including PreCompact and SubagentStart. What is still unconfirmed is the gating question the issue itself raised: .github/hooks/*.json is shared with the cloud coding agent, which does not document those two, and whether it ignores or rejects unknown event keys is not stated anywhere. Emitting keys that might break the cloud agent for everyone who enables the copilot target is not a call to make on inference.

Gap 7 (REVIEW.md): the issue notes this is an alternative location rather than a missing capability, since .github/copilot-instructions.md is still read by code review. Whether rulesync should own a review-only rules surface is a scope decision.

Verification

  • Full pnpm cicheck green.
  • e2e-hooks (75), e2e-mcp / e2e-permissions / e2e-rules (308) all pass; the global hooks e2e matrix gained a copilot cell.

…ts/sandbox, add the user-scope hooks file

Five verified drifts against the VS Code and GitHub Copilot references:

- chat.tools.edits.autoApprove and chat.tools.urls.autoApprove had no mapping; canonical edit and webfetch now drive them alongside the existing terminal map. read has no approval surface and write stays unmapped so it does not become indistinguishable from edit on import.
- Regenerating .vscode/mcp.json wrote {servers} over the whole document, deleting any hand-authored inputs or sandbox section; VS Code recommends committing that file, so a dropped inputs entry left ${input:...} unresolvable and servers failed to start. Only servers is replaced now, and an unparseable file aborts the write instead of being overwritten.
- CopilotHooks ignored its global option, so the documented ~/.copilot/hooks user scope emitted nothing. It now writes copilot-ide-hooks.json there, a name that cannot collide with the Copilot CLI global file already in that folder.
- The name frontmatter of *.instructions.md could not be authored or imported.

The preCompact/subagentStart event set and the REVIEW.md surface are left open; both hinge on a maintainer decision.
…pproval map the config does not state

Two regressions the review caught in the previous commit:

- Reading .vscode/mcp.json with JSON.parse aborted the whole generate run on the commented file VS Code's own 'MCP: Add Server' scaffold writes. The file is now declared in the shared-config gateway (format jsonc, mcp owns only 'servers'), which parses it correctly, fails closed on a real syntax error, and drops the hand-rolled merge.

- Every managed key went into the patch, so a config stating only bash rules retracted the user's hand-written chat.tools.edits.autoApprove and chat.tools.urls.autoApprove. A key is now touched only when the canonical config states its category; a category that is stated but yields nothing still retracts, as before.

Also fold the duplicate global hooks dir constant into the existing one and document that ~/.copilot/hooks loads every *.json, so generating both copilot and copilotcli globally makes each hook fire twice.
@dyoshikawa
dyoshikawa merged commit c9599dc into main Jul 29, 2026
9 checks passed
@dyoshikawa
dyoshikawa deleted the resolve-scrap-issue-2401-copilot branch July 29, 2026 02:45
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.

2 participants