Skip to content

Load external plugin READMEs from source repositories - #4945

Open
sbroenne wants to merge 7 commits into
github:mainfrom
sbroenne:sbroenne-excel-plugin-readme-display
Open

sbroenne wants to merge 7 commits into
github:mainfrom
sbroenne:sbroenne-excel-plugin-readme-display

Conversation

@sbroenne

@sbroenne sbroenne commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Pull Request Checklist

  • I have read and followed the CONTRIBUTING.md guidelines.
  • I have read and followed the Guidance for submissions involving paid services.
  • My contribution adds a new instruction, prompt, agent, skill, workflow, or canvas extension file in the correct directory.
  • The file follows the required naming convention.
  • The content is clearly structured and follows the example format.
  • I have tested my instructions, prompt, agent, skill, workflow, or canvas extension with GitHub Copilot.
  • I have run npm start and verified that README.md is up to date.
  • I am targeting the main branch for this pull request.

Description

Display external plugins' READMEs directly from their source repositories, as suggested in review. No README copies, opt-in documentation folders, extra listing fields, or plugin-specific configuration are needed in this repository.

Website data generation infers each README from the existing plugins/external.json entry: source.repo, source.path, and source.sha (preferred) or source.ref. Unpinned legacy listings use the repository's default branch through HEAD. It checks common Markdown README filename casing in the plugin directory, then the repository root, always at the same revision.

Remote Markdown uses the existing renderer and HTML sanitizer. Relative Markdown and raw HTML links/images resolve against the upstream document rather than this website. Only sanitized HTML is passed to the plugin page's client component. External provenance, installation commands, and built-in plugin README handling are preserved.

Downloads use only GitHub's raw-content host, reject redirects, run at most six plugins concurrently, and have a ten-second request timeout and 512 KiB response limit. Missing documents and retrieval/rendering failures are logged; the page retains a source-repository link instead of failing the website build.

The earlier local-copy approach and its preview workflow change have been removed. There are no listing, marketplace, dependency, or source revision changes. I maintain the Excel plugin that originally motivated this feature, but the implementation applies to every external GitHub plugin.


Type of Contribution

  • New instruction file.
  • New prompt file.
  • New agent file.
  • New plugin.
  • New skill file.
  • New agentic workflow.
  • New canvas extension.
  • Update to existing instruction, prompt, agent, plugin, skill, workflow, or canvas extension.
  • Other (please specify): Automatically display upstream READMEs on external plugin detail pages.

Additional Notes

Validation for the remote README implementation:

  • node --test eng/lib/external-plugin-readme.test.mjs — passed, 9 tests covering path/revision inference, SHA precedence, default branches, casing/root fallback, missing files, HTTP/network errors, size limits, invalid sources, and bounded concurrency.
  • npm run website:data — passed; retrieved READMEs for all 71 external plugins present in this branch.
  • npm run website:build — passed, including catalog generation, remote README retrieval, static pages, and the search index.
  • node --test website/scripts/external-plugin-readme.test.mjs — passed, 5 tests checking sanitizer behavior, relative Markdown/raw HTML URLs, cleanup between remote and local rendering, existing local README handling, and actual README content in generated external plugin pages.
  • npm run plugin:validate — passed; existing catalog warnings remain unchanged.
  • npm start — passed; generated catalog documentation and marketplace remain unchanged.
  • bash eng/fix-line-endings.sh — passed.
  • git diff --check — passed.

README updates are reflected on the next website build. For pinned listings, documentation deliberately follows the listed revision, not a newer moving branch. No live plugin execution is needed or performed by README retrieval.


By submitting this pull request, I confirm that my contribution abides by the Code of Conduct and will be licensed under the MIT License.

Include the Excel MCP README and optional local documentation support without changing installation sources or listings.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5baf3c5b-b695-4f86-b218-b351c9be8311
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

🚦 Submission status: 💬 Review in progress

Risk tier: merge-risk:high — Privileged execution, automation, or review-policy change
Required to merge: passing submission-gate checks plus 1 approval from reviewers with write access, including a maintainer with admin or maintain permission.

Why this tier
  • eng/generate-website-data.mjs is a high-risk path (automation, scripts, MCP config, hooks, or review policy)
  • eng/lib/external-plugin-readme.mjs is a high-risk path (automation, scripts, MCP config, hooks, or review policy)
  • eng/lib/external-plugin-readme.test.mjs is a high-risk path (automation, scripts, MCP config, hooks, or review policy)

Automated checks

Check Status Details
Line endings ✅ Passed Passed · logs
Spelling ✅ Passed Passed · logs
Contributor reputation ✅ Passed Passed · logs
Duplicate resource scan ✅ Passed Passed · logs
PR quality signal ⏭️ Skipped Skipped by its workflow · logs

Review

  • Approvals: 0/1
  • Assigned reviewer: not assigned yet — comment /request-review to ask for one
  • Review target date: not set
  • Still needed: 1 more approval(s); an approval from a maintainer with admin or maintain permission
  • The core-maintainers pool is not staffed yet; an approver with admin or maintain permission is required instead.

Commands

Command Who What it does
/rerun-checks PR author, maintainers Re-runs failed or incomplete checks and re-evaluates this gate
/request-review PR author, maintainers Asks the review rotation to assign a reviewer (adds needs-reviewer)

Updated for 7d949b4 · gate run · This comment is maintained automatically — see submission gate docs.

@github-actions github-actions Bot added new-submission PR adds at least one new contribution website-update PR touches website content or code workflow PR touches workflow automation ready-for-review Submission passed intake validation and is ready for maintainer review awaiting-automation and removed awaiting-automation ready-for-review Submission passed intake validation and is ready for maintainer review labels Oct 12, 2026
Keep documentation provenance in the PR description instead of modifying upstream content.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5baf3c5b-b695-4f86-b218-b351c9be8311
@github-actions github-actions Bot added awaiting-automation ready-for-review Submission passed intake validation and is ready for maintainer review and removed ready-for-review Submission passed intake validation and is ready for maintainer review awaiting-automation labels Oct 12, 2026
@sbroenne
sbroenne marked this pull request as ready for review October 12, 2026 11:55
@sbroenne
sbroenne requested a review from a team as a code owner October 12, 2026 11:55
Copilot AI balanced review requested due to automatic review settings October 12, 2026 11:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

@aaronpowell aaronpowell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that, rather than having the external plugin README added to this repo, it should pull it from the remote repo. We can do inference on the path of the README from the external.json entry for the plugin.

This would mean we have less repo churn and overhead when a plugin needs updating, since their README doesn't need to be updated.

It would also mean that we have a quick win across the website with all external plugins being supported, rather than waiting for maintainers to submit theirs

@github-actions github-actions Bot added review-in-progress and removed ready-for-review Submission passed intake validation and is ready for maintainer review labels Oct 13, 2026
Infer documentation paths and revisions from existing listings, sanitize remote Markdown, and resolve upstream links and images. Remove the copied README and local opt-in workflow.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5baf3c5b-b695-4f86-b218-b351c9be8311
Copilot AI balanced review requested due to automatic review settings October 15, 2026 16:05
@sbroenne sbroenne changed the title Display contributed READMEs on external plugin detail pages Load external plugin READMEs from source repositories Oct 15, 2026
@github-actions github-actions Bot removed the workflow PR touches workflow automation label Oct 15, 2026
@sbroenne

Copy link
Copy Markdown
Contributor Author

@aaronpowell Changed this to fetch READMEs from the external source repositories during website data generation, inferred from the existing repo/path/SHA/ref entries. The copied Excel README and local opt-in approach are removed. Documentation follows the listed revision, relative links and images resolve upstream, and the existing sanitizer and source-link fallback remain. The full website build passed and loaded READMEs for all 71 external plugins in this branch.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Untrusted frontmatter can execute code during website builds, and URL resolution, validation, and catalog payload issues remain.

4 open findings

🧠 Review effort: Balanced

Comment thread eng/lib/external-plugin-readme.mjs Outdated
Comment thread website/src/lib/detail-page.ts Outdated
Comment thread eng/generate-website-data.mjs
Comment thread website/src/lib/sanitize-html.ts Outdated
Support package-style tags with per-plugin validation fallback, reject executable frontmatter, keep README bodies out of catalog props, and preserve repository-root links at the selected revision.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5baf3c5b-b695-4f86-b218-b351c9be8311
Copilot AI balanced review requested due to automatic review settings October 15, 2026 16:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Untrusted upstream HTML enters a rendering pipeline with an unresolved script-injection path, requiring correction and human security review.

1 open finding
4 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Rewrite relative srcset candidates for remote README images

website/​src/​lib/​sanitize-html.ts:54

The URL hook rewrites img.src but leaves relative srcset candidates unchanged on <img> and <picture><source> elements. When browsers select those candidates, they request images from this website instead of the upstream repository, breaking responsive or theme-specific README images. Resolve the candidates against imageBaseUrl while preserving their descriptors, or strip srcset for remote READMEs so browsers use the rewritten fallback image. The suggested fix implements the fallback approach without changing local rendering.

🧠 Review effort: Balanced

Comment thread website/src/lib/detail-page.ts Outdated
Re-sanitize rendered markdown after adding accessibility attributes so hostile text in source attributes cannot become executable markup.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5baf3c5b-b695-4f86-b218-b351c9be8311
Copilot AI balanced review requested due to automatic review settings October 15, 2026 17:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Post-sanitization table-of-contents processing can introduce executable event handlers from upstream README content.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread website/src/pages/plugin/[id].astro Outdated
Run a final sanitizer pass after plugin table-of-contents rewriting and strip remote README srcset values so headings and images cannot bypass the upstream URL and HTML safeguards.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5baf3c5b-b695-4f86-b218-b351c9be8311
Copilot AI balanced review requested due to automatic review settings October 15, 2026 18:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Upstream CSS can affect trusted installation controls, while repository-name validation and fragment links still have correctness issues.

1 open finding
1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Accept dot-prefixed GitHub repository names

eng/​lib/​external-plugin-readme.mjs:19

[P2] Allow dot-prefixed GitHub repository names. A valid source such as owner/.github is accepted by the canonical repository validator (eng/external-plugin-validation.mjs:363), but this pattern rejects it before any request is made, so its README always falls back despite being available. Allow a leading dot in the repository segment while continuing to reject standalone . and .. segments.

Medium severity Generate compatible anchors for all README headings

website/​src/​lib/​sanitize-html.ts:63

Fragment-only links stay on this page, but the renderer does not reproduce GitHub's heading anchors. For example, [Configuration](#configuration) followed by ### Configuration has no target: buildDetailToc only assigns IDs to H2 headings. Its slugging also differs from GitHub's. Upstream tables of contents and section links can therefore stop working when embedded here. Generate GitHub-compatible anchors for all heading levels, or resolve fragments with no local target against the upstream README URL. Add a regression test for an H3 section link.

🧠 Review effort: Balanced

Comment thread website/src/lib/sanitize-html.ts Outdated
Forbid style elements and attributes when sanitizing external plugin documentation, including the final pass after TOC processing, so README CSS cannot cover trusted page controls.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 5baf3c5b-b695-4f86-b218-b351c9be8311
Copilot AI balanced review requested due to automatic review settings October 15, 2026 19:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Third-party HTML rendering needs human security review, and final-rendering failures remain outside the fallback handler.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment on lines +78 to +81
const { html: tocHtml } = buildDetailToc(markdownHtml);
const html = sanitizeHtml(tocHtml, item.external && item.readme
? { linkBaseUrl: item.readme.sourceUrl, imageBaseUrl: item.readme.rawUrl }
: undefined);

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk:high new-submission PR adds at least one new contribution review-in-progress website-update PR touches website content or code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants