external docs: build pages from registered GitHub sources#152
Conversation
docs-refresh: add registry-based external docs deploy check
tools: document Hypercerts agent skills
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a build-time external documentation pipeline with validated GitHub sources, immutable snapshots, page integration, link/image rewriting, Mermaid rendering, deterministic fingerprints, refresh automation, and associated tests and site updates. ChangesExternal documentation system
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
components/MermaidDiagram.js (1)
110-115: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider sanitizing SVG before
dangerouslySetInnerHTMLas defense-in-depth.
securityLevel: 'strict'already triggers Mermaid's internal DOMPurify pass on SVG output, so this isn't directly exploitable today. Still, since this renders content sourced from external repositories, an explicitDOMPurify.sanitize(svg)call before injection would add defense-in-depth independent of Mermaid's internal behavior (recommended by security research on diagram-renderer XSS).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/MermaidDiagram.js` around lines 110 - 115, Update the render return in MermaidDiagram to sanitize the generated svg with DOMPurify immediately before assigning it to dangerouslySetInnerHTML. Import or reuse the project’s existing DOMPurify integration, while preserving Mermaid’s strict security configuration and the current diagram rendering behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docs-ci.yml:
- Around line 38-39: Disable credential persistence for the actions/checkout
step in .github/workflows/docs-ci.yml lines 38-39 and
.github/workflows/docs-refresh.yml lines 32-33 by adding the checkout step’s
persist-credentials setting as false under with.
In @.github/workflows/docs-refresh.yml:
- Around line 110-121: Update the “Write summary” step to pass the dynamic diff
outputs and workflow values through the step’s env configuration, then reference
those environment variables inside the bash script instead of inline `${{ }}`
expansions. Preserve the existing fallback values and summary fields while
ensuring shell-safe handling of values such as reason and fingerprints.
In `@components/MermaidDiagram.js`:
- Around line 4-16: Update getMermaid so a rejected dynamic import clears
mermaidModulePromise before propagating the failure, allowing later calls to
retry the import. Preserve the existing successful-module memoization and
module.default fallback behavior.
In `@lib/external-doc-links.js`:
- Line 1: Decode the Git ref extracted from resolved.pathname in both
resolveExternalDocHref and resolveExternalDocImageSrc before reconstructing or
serializing the final GitHub URLs. Preserve the existing URL resolution and path
handling, but ensure branch names such as feature/branch are returned with
literal slashes rather than percent-encoded separators.
In `@lib/external-doc-page.js`:
- Around line 20-23: Update the frontmatter regular expression in
getMarkdownBody to accept both LF and CRLF line endings by allowing optional
carriage returns before newline characters. Preserve the existing frontmatter
matching boundaries and body-slicing behavior.
In `@lib/external-docs.js`:
- Line 147: Update the frontmatter boundary regex in lib/external-docs.js at
lines 147-147 to accept optional carriage returns before each newline,
preserving the existing capture behavior. Apply the same CRLF-aware boundary
update in lib/external-docs-loader.js at lines 10-10 so both parsing and
stripping paths handle LF and CRLF frontmatter consistently.
---
Nitpick comments:
In `@components/MermaidDiagram.js`:
- Around line 110-115: Update the render return in MermaidDiagram to sanitize
the generated svg with DOMPurify immediately before assigning it to
dangerouslySetInnerHTML. Import or reuse the project’s existing DOMPurify
integration, while preserving Mermaid’s strict security configuration and the
current diagram rendering behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c535bc07-8712-4b41-8313-08fbe5ca17ef
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
.github/workflows/docs-ci.yml.github/workflows/docs-refresh.yml.gitignorecomponents/AnnouncementBanner.jscomponents/CopyRawButton.jscomponents/MermaidDiagram.jscomponents/TableOfContents.jsdocs-sources.ymldocs/remote-markdown.mdlib/compare-docs-fingerprint.jslib/external-doc-links.jslib/external-doc-page.jslib/external-docs-loader.jslib/external-docs-snapshot.jslib/external-docs.jslib/generate-docs-fingerprint.jslib/generate-external-docs-manifest.jslib/generate-last-updated.jslib/generate-raw-pages.jslib/generate-search-index.jslib/navigation.jsmarkdoc/nodes/document.markdoc.jsmarkdoc/nodes/fence.markdoc.jsmarkdoc/nodes/image.markdoc.jsmarkdoc/nodes/index.jsmarkdoc/nodes/link.markdoc.jsmarkdoc/tags/br.markdoc.jsmarkdoc/tags/index.jsnext.config.jspackage.jsonpages/_app.jspages/architecture/epds.mdpages/lexicons/hypercerts-lexicons/index.mdpages/tools/hypercerts-agent-skills.mdpages/tools/hyperindex.mdstyles/globals.csstest/external-doc-links.test.jstest/external-docs-snapshot.test.jstest/external-docs.test.js
d342a99 to
915fd80
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/docs-ci.yml (1)
65-68: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the
curlcommand to prevent hanging CI jobs.Network calls in CI pipelines can occasionally hang indefinitely if the target server stops responding but leaves the connection open. Consider adding a timeout (e.g.,
--max-time 60) so the workflow fails fast rather than wasting runner minutes if the staging server is unreachable.♻️ Proposed refactor
- http_code=$(curl -sS -L -w "%{http_code}" \ + http_code=$(curl -sS -L --max-time 60 -w "%{http_code}" \ -o deployed-docs-fingerprint.json \ "$DOCS_FINGERPRINT_URL")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docs-ci.yml around lines 65 - 68, Add a finite timeout option such as --max-time 60 to the curl invocation assigning http_code in the documentation CI workflow, preserving the existing URL, output file, redirect handling, and curl_status capture.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/docs-ci.yml:
- Around line 65-68: Add a finite timeout option such as --max-time 60 to the
curl invocation assigning http_code in the documentation CI workflow, preserving
the existing URL, output file, redirect handling, and curl_status capture.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9acab48d-7422-42d6-8077-71191f98e286
📒 Files selected for processing (22)
.github/workflows/docs-ci.yml.github/workflows/docs-refresh.ymlcomponents/CopyRawButton.jscomponents/MermaidDiagram.jslib/compare-docs-fingerprint.jslib/external-doc-links.jslib/external-doc-page.jslib/external-docs-loader.jslib/external-docs-snapshot.jslib/external-docs.jslib/generate-docs-fingerprint.jslib/generate-last-updated.jslib/generate-raw-pages.jslib/generate-search-index.jsmarkdoc/nodes/document.markdoc.jsmarkdoc/nodes/fence.markdoc.jsmarkdoc/nodes/image.markdoc.jsmarkdoc/nodes/link.markdoc.jsmarkdoc/tags/br.markdoc.jsnext.config.jsstyles/globals.csstest/external-docs.test.js
🚧 Files skipped from review as they are similar to previous changes (20)
- markdoc/tags/br.markdoc.js
- markdoc/nodes/link.markdoc.js
- markdoc/nodes/image.markdoc.js
- next.config.js
- markdoc/nodes/fence.markdoc.js
- styles/globals.css
- markdoc/nodes/document.markdoc.js
- lib/external-doc-links.js
- components/CopyRawButton.js
- lib/generate-raw-pages.js
- lib/generate-search-index.js
- lib/external-doc-page.js
- lib/generate-docs-fingerprint.js
- components/MermaidDiagram.js
- lib/external-docs.js
- lib/external-docs-snapshot.js
- lib/external-docs-loader.js
- lib/compare-docs-fingerprint.js
- test/external-docs.test.js
- .github/workflows/docs-refresh.yml
Kzoeps
changed the title
Kzoeps
changed the title
aspiers
left a comment
aspiers
left a comment
There was a problem hiding this comment.
Looks great! Saw a few opportunities for improvement.
| @@ -0,0 +1,12 @@ | |||
| sources: | |||
| - id: epds | |||
There was a problem hiding this comment.
| - id: epds | |
| - id: epds-tutorial |
| --- | ||
| title: ePDS (extended PDS) | ||
| description: How the ePDS adds email/OTP login on top of AT Protocol without changing the standard OAuth flow for apps. | ||
| externalDoc: epds |
There was a problem hiding this comment.
IIUC, this is including the tutorial docs, which don't belong on the architecture page. (Seems it's a pre-existing problem, but this is a good opportunity to fix it.) There should be a separate section for tutorials. Is it possible to just include part of a page? Ideally we should avoid pages being too long, so splitting long pages into smaller pages would be good. If that requires splitting up doc files in the ePDS repo then we can do that. Actually maybe we should do that even if there is a way to do it with externalDoc.
| ```text | ||
| Client App | ||
| -> starts AT Protocol OAuth against the PDS | ||
|
|
||
| PDS Core | ||
| -> remains the OAuth issuer and token endpoint | ||
| -> advertises the Auth Service as the authorization endpoint | ||
|
|
||
| Auth Service | ||
| -> collects the user's email or OTP | ||
| -> verifies the user | ||
| -> returns control to PDS Core via signed callback | ||
|
|
||
| PDS Core | ||
| -> issues a normal authorization code | ||
|
|
||
| Client App | ||
| -> exchanges the code for tokens | ||
| ``` |
There was a problem hiding this comment.
The architecture page should have a Mermaid version of this, or similar. An architecture diagram is the main item which an architecture page MUST have.
| @@ -0,0 +1,65 @@ | |||
| # Build-time external documentation | |||
There was a problem hiding this comment.
Great to have this, but I thought you already drew a nice diagram showing the whole flow - is that only in Linear? Would be worth having here.
2 participants
check out https://hypercerts-v02-documentation-git-d-210225-hypercerts-foundation.vercel.app/architecture/epds for one of the pages dynamically rendered
Summary
Testing
npm test(24 tests passed)Summary by CodeRabbit
New Features
Documentation
Bug Fixes