fix(plugins): materialize plugins as real files instead of symlinks#2443
fix(plugins): materialize plugins as real files instead of symlinks#2443katriendg wants to merge 14 commits into
Conversation
Replace the symlink-or-text-stub materialization in plugin generation with copied files and directory trees so generated plugin packages are self-contained and install on Windows without Developer Mode or elevated privileges. - Copy files and directory trees in New-PluginLink, copy hook sibling script directories, and track copied child files for orphan cleanup - Remove Test-SymlinkCapability, Repair-PluginSymlinkIndex, and the -SymlinkCapable parameter and its generator orchestration - Preserve generated shared directories during refresh cleanup - Assert real in-package content in Validate-Marketplace.ps1 - Update plugin Pester tests for copy-based behavior - Regenerate plugins/** with zero symlinks (mode 100644)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Dependency ReviewThe following issues were found:
|
Eval Execution✅ Status: Passed
No changed AI artifacts required evaluation. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2443 +/- ##
==========================================
- Coverage 82.76% 82.74% -0.03%
==========================================
Files 154 154
Lines 20853 20834 -19
Branches 13 13
==========================================
- Hits 17259 17239 -20
- Misses 3592 3593 +1
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Plugin files under plugins/ are now real copies (not symlinks) of already-validated .github/ sources, so recursive scanners were seeing duplicate content: spell check failed and CodeQL flagged duplicate alerts. Exclude the generated tree from checks that would otherwise re-scan the copies. - Add plugins/** to cspell ignorePaths (fixes failing spell-check) - Add CodeQL config with paths-ignore: plugins, referenced from codeql-analysis.yml init (drops duplicate code-scanning alerts) - Exclude plugins from CI Python project discovery in pr-validation (removes plugin copies from python-tests, fuzz-tests, and pip-audit) - Exclude plugins from Get-PythonSkill and Invoke-PythonTests discovery - Add plugins/** to codecov ignore and the lint:links exclude set - Add parallel plugins-exclusion Pester cases mirroring node_modules
…pied content 🔧 - Generated by Copilot
…/plugin-symlinks-windows
- convert remaining symlink stubs (mode 120000) to real file copies - update modified plugin files to reflect source changes from main - remove orphaned references files no longer present in sources 🔌 - Generated by Copilot
There was a problem hiding this comment.
Was eager to resolve merge conflicts and get this merged, but after so many changes resulting from npm run plugin:generate and the fact that my approval is no longer valid due to being the last pusher, we need one more fresh approval from someone else.
@rezatnoMsirhC Agree and approved :) |
|
Seems like we need to regenerate the plugin files locally and commit the result to fix the failed check.... |
|
Independent Windows verification that this is the right fix, plus a note on the red checks. Repro on On the failing checks: the only substantive one is Plugin Validation → npm ci
npm run plugin:generate
git add plugins && git commit -m "chore(plugins): regenerate after source sync" && git pushPR Validation Success is only red as a cascade of that ( 🤖 Drafted by GitHub Copilot. |
|
@katriendg — this is approved and really close; the only substantive blocker is the npm ci
npm run plugin:generate
git add plugins && git commit -m "chore(plugins): regenerate after source sync" && git pushCould you push that when you get a chance? I'd gladly do it myself but don't have write access to the branch. Thanks for driving this one — it fixes the Windows install for us. 🤖 Drafted by GitHub Copilot. |
|
Heads-up from testing this branch on Windows: the real-file conversion here is a clear improvement, but there looks to be a second, separate Windows issue that this PR alone won't resolve for the larger bundles. I checked out this branch and installed its plugins via a local marketplace (i.e. the real-file
The split correlates with plugin size, not content:
So it reads as a CLI-side As a stopgap, a manual install works on Windows: copy the real-file plugin dir into 🤖 Drafted by GitHub Copilot. |
Thanks for checking and validating! This PR is behind main, and we have to regenerate the plugins for it to align. Still debating internally on the approach taken before we rebase and rebuild the plugins to pass. Your testing feedback is valuable, this will help align the PR to be merge ready. |
- reject links and unsafe package paths - preserve generated directories during orphan cleanup - add regression coverage and refresh plugin outputs 🔧 - Generated by Copilot
- publish plugin trees atomically - validate package paths and archives - exclude generated plugins from freshness checks for MD files - regenerate updated plugins from merge main 🔧 - Generated by Copilot
|
@rezatnoMsirhC @jkim323 Additionally after testing some other stuff on HVE Core, I did some more optimizations since initial PR: Optimizations Since Opening initial PRThe latest hardening commit builds on the original copy-based implementation:
Review FocusThe latest optimization commit changes 11 authored files and regenerates 38 Generation and validation
Tests
CI and configuration |
|
@WilliamBerryiii - as discussed yesterday if you want to take a look before we merge, current as of time of writing |
6 participants
Please focus on the changed code files and not on the
./pluginsfolder contents to assess the completeness of the PR. The 3000+ file changes there are simply copies of the files now.Due to the huge amount of changes, the UI is NOT showing some key file changes, namely these ones under scripts folder which are the main ones to review:
Plugin generation and validation
+276/-193): materializes tracked sources as regular files and directories; validates Git modes, readability, ignored files, nested links, destination containment, and regular ancestors; removes symlink probing and index repair+156/-61): generates complete trees in staging directories, atomically promotes them with backup and rollback, removes stale output, preserves dry-run behavior, and retains-Refreshcompatibility+240/-6): recursively validates regular package objects, path containment, plugin identity and version consistency, component target types, manifest rules, and required in-package contentPlugin tests
+211/-124): covers copy behavior, tracked-source preflight, unsupported modes, ignored and unreadable sources, nested links, destination containment, and invalid ancestors+121/-7): covers dry runs, complete-tree replacement, stale-output removal, promotion rollback, cleanup, and generation without symlink capability checks+167/-11): covers topology, package traversal, links and non-regular objects, identity and version mismatches, component target types, manifest rules, and missing package contentGenerated-content discovery exclusions
+1/-1): excludes generatedplugins/**projects from Python skill discovery+1/-1): excludes generatedplugins/**projects from Python test discovery+15): verifies plugin copies are excluded from Python skill discovery+20): verifies plugin copies are excluded from Python test discovery+9/-3): excludes generatedplugins/**Markdown from recursive and changed-files-only freshness discovery while preserving explicit-file checks+12): verifies the generated-directory exclusion in the changed-files-only laneCI and packaging gates
+6/-7): validates marketplace integrity after generation+13/-1): validates generated packages and rejects non-regular archive entries before upload+2/-2): includes marketplace validation inplugin:validateand excludes generated plugin copies from link scanningDescription
The plugin generator used to materialize every packaged component as a git symbolic link on Linux and macOS, or as a text stub (a small file whose entire contents were a relative path) on Windows without Developer Mode. A follow-up pass then rewrote git index modes from
100644to120000so those stubs would check out as real symlinks on other platforms. This keptgit statusclean, but it left the shipped plugin packages pointing at their sources rather than containing them.That mismatch is what users hit in the field. On Windows the stubs shipped as literal relative-path text, so components arrived malformed. Through the
agentPluginsinstall path, symlinked components did not resolve to real files inside the installed package, so the plugins were effectively empty. This PR replaces the link-and-repair strategy with straightforward file copies, so every generated artifact under plugins/ is a regular file, and adds marketplace validation that fails when a packaged plugin is missing real in-package content.The bulk of the diff is the regenerated plugins/* tree*, where 3,146 previously-symlinked or stubbed entries become real files. The reviewable change is the six PowerShell source and test files described below, plus a set of CI exclusions covered in the CI and workflows section.
Because plugin packages are now materialized as real file copies (rather than symlinks) of already-validated .github/ sources, every recursive check began scanning duplicate content: spell check failed on the copies and CodeQL raised duplicate "clear-text logging" alerts against the copied mural/_cli_auth.py. Since everything under plugins/ is auto-generated and independently guaranteed by plugin validation, it should always be excluded from source-level checks.
Changes
Generation
New-PluginLinkin PluginHelpers.psm1 to copy the source file or directory tree into the plugin destination instead of creating a symbolic link or writing a text stub. It removes any existing destination first, so stale symlinks or files from earlier generations do not survive a re-run, and uses-LiteralPathconsistently.Test-SymlinkCapability(the per-run symlink probe) andRepair-PluginSymlinkIndex(the100644→120000git index rewrite) from PluginHelpers.psm1, along with theirExport-ModuleMemberentries.Add-GeneratedFilesForCopiedTree, which walks a materialized tree and registers every copied file path into the generated-files set so orphan cleanup preserves the full tree rather than pruning nested copied content.-SymlinkCapableparameter fromWrite-PluginHookArtifactandWrite-PluginDirectory. Hook script directories, skill directories, and shared directories now register their copied trees through the new helper.Orchestration
Repair-PluginSymlinkIndexpost-pass fromInvoke-PluginGenerationin Generate-Plugins.ps1.LinkType. It now builds anOrdinalIgnoreCaseset of generated paths and preserves any directory that was generated or contains generated content, checking that prefix before removing empty directories bottom-up.Validation
Test-PluginPackageContentto Validate-Marketplace.ps1. It confirms each packaged plugin has a real in-package README.md and .github/plugin/plugin.json, parses that manifest, and verifies every declaredagents,commands,skills, andhookspath resolves to real content inside the plugin root.Invoke-MarketplaceValidation, running per plugin once the directory-existence check passes, so a plugin that ships without real content now fails validation.Tests
Test-SymlinkCapabilityandRepair-PluginSymlinkIndexsuites from PluginHelpers.Tests.ps1. TheNew-PluginLinktests now assert real copied file content with no reparse point, plus a recursive directory copy that preserves nested files.New-TestPluginPackagehelper in Validate-Marketplace.Tests.ps1 that builds real README and plugin.json packages, updated existing fixtures to use it, and added coverage for a missing README and a missing declared component path.CI and workflows
Excluded the generated plugins/ tree from source-level checks that would otherwise re-scan the copies:
plugins/**to the .cspell.jsonignorePaths(fixes the failing spell-check), to the codecov.ymlignorelist, and to thelint:linksexclude set in package.json.paths-ignore: plugins, wired into codeql-analysis.yml viaconfig-file, to drop the duplicate code-scanning alerts.pluginsguard to the recursivepyproject.tomldiscovery in pr-validation.yml, which transitively removes plugin copies from thepython-tests,fuzz-tests, andpip-auditmatrix jobs, and applied the same guard to the localGet-PythonSkilland Invoke-PythonTests.ps1 discovery helpers.Node-project discovery needed no change since it is already scoped to .github/skills, and markdown linting was intentionally left as-is because the
plugin:postprocessstep formats the generated top-level markdown.Related Issue(s)
Fixes #2409
Related to #1905 which reported malformed agent symlinks on Windows, but the root cause was the same: symlinks and path stubs do not survive packaging and installation intact. To be validated if GitHub Copilot CLI issues also solved, and close issue once we validate.
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
prompt-builderagent and addressed all feedback.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md).github/skills/*/SKILL.md).github/hooks/*/*.json)evals/)Other:
.ps1,.sh,.py)Testing
Ran the plugin PowerShell suites via
npm run test:ps -- -TestPath "scripts/tests/plugins/"and regenerated the tree withnpm run plugin:generateto confirm every artifact under plugins/* materializes as a regular file with no symbolic links or120000index entries. Marketplace validation (npm run lint:marketplace) now exercises the in-package content checks.Checklist
Required Checks
AI Artifact Contributions
/prompt-analyzeto review contributionprompt-builderreviewRequired Automated Checks
The following validation commands must pass before merging:
npm run lint:mdnpm run spell-checknpm run lint:frontmatternpm run validate:skillsnpm run lint:md-linksnpm run lint:psnpm run eval:lint:schemanpm run plugin:generatenpm run docs:testSecurity Considerations
Additional Notes
Removing
Repair-PluginSymlinkIndexalso drops thegit hash-objectandgit update-indexcalls that the generator previously ran against generated paths, reducing the generator's surface area. Reviewers should focus on the six source and test files; the remaining diff is the regenerated plugins/* output* produced bynpm run plugin:generate.