Re-land canvas-apps sync reminder hook, fixed for Claude Code - #335
Conversation
There was a problem hiding this comment.
Pull request overview
This PR re-lands the canvas-apps “sync reminder” prompt hook while fixing the Claude Code incompatibilities that caused the original change to be reverted. It restores the hook implementation and adjusts registration so Claude Code and Copilot CLI each see only the hook event(s) they support.
Changes:
- Re-introduces the sync reminder hook implementation and registers it via Claude Code auto-discovered
hooks/hooks.json(UserPromptSubmit) and Copilot CLI inline.plugin/plugin.jsonhooks (userPromptTransformed). - Removes the deprecated
generate-canvas-appskill wrapper. - Updates
plugins/canvas-apps/AGENTS.mdto document the host-specific manifest/hook split and the rationale.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/canvas-apps/skills/generate-canvas-app/SKILL.md | Removes the deprecated redirect skill wrapper. |
| plugins/canvas-apps/hooks/inject-sync-reminder.cs | Adds the reminder injector hook implementation that outputs the expected shape per host. |
| plugins/canvas-apps/hooks/hooks.json | Registers the reminder hook for Claude Code via UserPromptSubmit. |
| plugins/canvas-apps/AGENTS.md | Documents the host-specific hook registration strategy and plugin structure. |
| plugins/canvas-apps/.plugin/plugin.json | Bumps version and adds inline Copilot CLI userPromptTransformed hook registration. |
| plugins/canvas-apps/.claude-plugin/plugin.json | Bumps version for Claude Code manifest without adding hooks fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
scripts/validate-legacy-compatibility.js:52
assertPluginManifestMirror()now ignores thehooksfield entirely, which means the validator will no longer catch a reintroduction of the original Claude-breaking defect (a.claude-plugin/plugin.jsonthat declareshooks, including the invalid string-path form). Since Claude auto-discovershooks/hooks.json, it’s safer for this repo-level validator to explicitly assert that legacy manifests do not declarehooks, while still allowing.plugin/plugin.jsonto contain Copilot-only hooks.
function withoutHooks(manifest) {
const { hooks, ...rest } = manifest;
return rest;
}
// Plugin manifests mirror each other for metadata, but `hooks` is deliberately
// host-specific and must not be forced into sync. Claude Code and Copilot CLI expose
// different prompt hook events (`UserPromptSubmit` vs `userPromptTransformed`), and
// Claude Code fails to load a plugin whose manifest declares an event it does not
// recognize — taking that plugin's MCP servers down with it. So a Copilot-only hook
// belongs in `.plugin/plugin.json`, which Claude Code never reads.
function assertPluginManifestMirror(legacyPath, sourcePath) {
assert.deepEqual(withoutHooks(readJson(legacyPath)), withoutHooks(readJson(sourcePath)));
}
plugins/canvas-apps/hooks/hooks.json:9
- The hook command relies on POSIX-style
${CLAUDE_PLUGIN_ROOT}expansion. Other plugins’ hooks avoid${...}incommandstrings (they readprocess.env.PLUGIN_ROOT || process.env.CLAUDE_PLUGIN_ROOTinside Node) to avoid shell-/platform-dependent expansion (notably on Windows shells). Consider switching this to a Node wrapper that resolves the plugin root from env vars and forwards stdin/stdout todotnet run.
"type": "command",
"command": "dotnet run --file \"${CLAUDE_PLUGIN_ROOT}/hooks/inject-sync-reminder.cs\"",
"timeout": 30
plugins/canvas-apps/.plugin/plugin.json:18
- This
commandstring relies on${PLUGIN_ROOT}expansion, which is shell-dependent. In the repo’s existing hook commands (e.g., model-apps/power-pages), commands avoid${...}substitution and instead resolveprocess.env.PLUGIN_ROOT || process.env.CLAUDE_PLUGIN_ROOTinside Node for better cross-platform behavior. Consider using the same pattern here to avoid the Copilot hook breaking on hosts/shells that don’t expand${VAR}.
{
"type": "command",
"command": "dotnet run --file \"${PLUGIN_ROOT}/hooks/inject-sync-reminder.cs\"",
"timeout": 30
}
Re-applies the hook from microsoft#320, which was reverted in microsoft#327 because it broke Claude Code. The hook logic (inject-sync-reminder.cs) is restored byte-for-byte; only the registration wiring changed. Two defects in microsoft#320, either one fatal: 1. `"hooks": "hooks/hooks.json"` in .claude-plugin/plugin.json. Claude Code rejects the path unless it is `./`-relative, failing installation with `Validation errors: hooks: Invalid input`. Even written as `"./hooks/hooks.json"` it is redundant — Claude auto-discovers the file, then reports the explicit reference as a duplicate hook source and disables the plugin's MCP servers. model-apps and power-pages ship working hooks with no `hooks` field at all. 2. `userPromptTransformed` in hooks/hooks.json. Claude Code reads that file and rejects unrecognized event names, silently discarding the whole file and disabling the plugin's canvas-authoring MCP server: [ERROR] Failed to load hooks for canvas-apps [DEBUG] Plugin not available for MCP - error type: hook-load-failed Registration now uses the two documented plugin hook formats, so each host reads only the events it supports: Claude Code | hooks/hooks.json (Claude format) | UserPromptSubmit Copilot CLI | hooks.json at plugin root (Copilot) | userPromptTransformed Two events are required because the hosts share no usable one: Copilot accepts UserPromptSubmit as an alias for userPromptSubmitted, but that event has no output processing, so the reminder would be generated and silently discarded. Both manifests stay byte-identical mirrors, so the legacy compatibility check is unaffected. Also re-applies microsoft#320's removal of the deprecated generate-canvas-app skill, which the revert restored, and corrects the AGENTS.md claim that Claude Code "ignores the Copilot-only event and continues loading UserPromptSubmit" — the behaviour that caused this outage. Verified on Claude Code 2.1.220 and Copilot CLI 1.0.76-1: plugin installs and loads (status enabled, MCP intact) and the reminder reaches the model in both hosts. All four repository validation scripts pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4fc283c to
8aadf85
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
plugins/canvas-apps/hooks/hooks.json:9
- The hook command path is built using
${CLAUDE_PLUGIN_ROOT}only. Elsewhere in this repo, hooks resolve the plugin root fromPLUGIN_ROOT || CLAUDE_PLUGIN_ROOT(e.g., plugins/power-pages/hooks/hooks.json) to stay compatible across hosts and launchers. Using onlyCLAUDE_PLUGIN_ROOTcan cause this hook to fail to locate the script in environments where onlyPLUGIN_ROOTis set.
"command": "dotnet run --file \"${CLAUDE_PLUGIN_ROOT}/hooks/inject-sync-reminder.cs\"",
|
McCall Saltzman (@lesaltzm) — the hook is working in both hosts now, and none of the below blocks this PR. Just worth having on record. Global registrationYou're likely already across this, but plugin hooks register host-wide rather than per-plugin: once canvas-apps is installed, Related precedent here: #323 removed the mobile-apps hooks for the mirror-image problem, where their write hook fired during Canvas workflows and denied Canvas file mutations. It added this to
Ours is materially safer — Cost, and a way to gate it
Gating on an active session. The hook currently can't tell whether
If The marker would need a lifecycle — removed on disconnect, ideally timestamped — or a crashed session leaves it behind and the gate silently reverts to always-on. (Claude's Cheaper process. Claude's branch emits a constant string (the reminder), so it could be a plain Neither idea changes your design, and the reminder is already worded conditionally so the agent self-skips when no session is active. Both are follow-ups if the per-turn cost turns out to matter in practice. |
Summary
Re-lands the canvas-apps sync reminder hook from #320, which was reverted in #327 because it broke Claude Code.
hooks/inject-sync-reminder.csis restored byte-for-byte — the hook logic was never the problem. Only the registration wiring changed.Root cause
#320 had two independent defects, either one fatal.
1.
"hooks": "hooks/hooks.json"in.claude-plugin/plugin.json. Claude Code rejects the path unless it is./-relative, failing installation outright:Written as
"./hooks/hooks.json"it installs, but is then redundant: Claude auto-discovers the file, logs the explicit reference as a duplicate hook source, and still reportshook-load-failed.model-appsandpower-pagesship working hooks with nohooksfield at all — that is the convention this PR follows.2.
userPromptTransformedinhooks/hooks.json. Claude Code reads that file too and rejects unrecognized event names. The whole file is discarded, silently, and the failure takes the plugin's MCP servers down with it:For canvas-apps that means
canvas-authoringnever starts, so the plugin is not degraded — it is gone. Removing that single key flips the plugin from✘ failed to loadback to✔ enabled.Fix
Registration now uses the two documented plugin hook formats, so each host reads only the event it supports:
hooks/hooks.json(Claude format)UserPromptSubmithookSpecificOutput.additionalContexthooks.jsonat plugin root (Copilot format)userPromptTransformedmodifiedTransformedPromptEach host ignores the other's file. Both manifests remain byte-identical mirrors, so
validate-legacy-compatibilityneeds no changes.Two events are required because the hosts share no usable one: Copilot accepts
UserPromptSubmitas an alias foruserPromptSubmitted, but that event has no output processing — the reminder would be generated and silently discarded.Claude Code is not spec compliant here
The Open Plugins hooks spec states:
Claude Code does the opposite: one unrecognized event name discards the entire hooks file and disables the plugin's MCP servers.
This is a known and unaddressed class of bug. anthropics/claude-code#31763 reports the same strict-validation failure mode — a
hooks.jsoncontaining[]or{}prevents the plugin from loading — and carries thebug,area:hooks, andarea:pluginslabels. It was auto-closed as not planned after going stale, with no fix. anthropics/claude-code#16288 (plugin hooks not loaded from an externalhooks.json) is still open.Until that behaviour changes, keeping the two formats in separate files is required rather than stylistic.
Verification
Claude Code 2.1.220 and Copilot CLI 1.0.76-1, run against the files in this PR:
✔ enabledat2.2.2, MCP intact, reminder arrives asUserPromptSubmithook contexthooks.json, with${PLUGIN_ROOT}expanding correctlyAlso included
generate-canvas-appskill, which the revert restoredAGENTS.mdclaim that Claude Code "ignores the Copilot-only event and continues loadingUserPromptSubmit" — the assumption behind the outage — and links the hook format referenceNote for reviewers
This registers a global
UserPromptSubmithook. It fires on every turn (~0.35s warm, 1.8s cold fordotnet run --file) for every canvas-apps user, including those working in unrelated plugins — the same class of cross-plugin side effect #323 addressed for mobile-apps. It injects text rather than gating tool calls, so it cannot deny operations the way the mobile write hook did.