chore(docs): improved site metadata and tests#152
Open
coryrylan wants to merge 1 commit into
Open
Conversation
github-actions
Bot
added
scope(docs)
dependencies
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Cursor-agent warning path, an availability-report skill and CLI, and updates site URL handling, metadata generation, and starter page metadata. ChangesAgent tooling
Site metadata and starters
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
projects/site/src/_11ty/layouts/metadata.jsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. projects/site/src/_11ty/layouts/metadata.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. projects/site/src/_11ty/plugins/llms-txt.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 @.agents/hooks/pre-tool-use-bash.sh:
- Around line 16-24: The destructive git guard in pre-tool-use-bash.sh is
bypassed too broadly because it exits 0 whenever CURSOR_AGENT is set. Update the
conditional in the hook to require an additional explicit isolated-environment
signal or trusted mode before allowing the warning-only path, and keep blocking
otherwise so the bypass is limited to approved VM contexts.
In @.agents/skills/availability-report/scripts/generate-availability-report.js:
- Around line 504-515: The argument parsing logic in the availability report
generator currently treats the token after --timestamp as a valid value even
when it is another flag, so malformed input can slip through as a synthetic
report failure instead of a parse error. Update the argument handling in
generate-availability-report.js around the --timestamp branch in the parser (and
the associated parsed.errors checks) to reject cases where args.at(index + 1) is
missing or starts with -, and report a missing/invalid value for --timestamp
instead of assigning that token to parsed.timestamp.
- Around line 271-274: The availability install path in
generate-availability-report.js currently runs npm install with lifecycle
scripts enabled, which can execute arbitrary package code during report
generation. Update the install command in the tempDir flow (the runCommand call
after initCommand in generate-availability-report.js) to include
--ignore-scripts while keeping the existing metadata-resolution behavior, and
make sure any matching documentation in SKILL.md reflects the safer command.
In @.agents/skills/availability-report/SKILL.md:
- Around line 37-38: Clarify the CLI exit behavior in the availability-report
SKILL.md so it distinguishes environment failures from generated report
failures. Update the wording around the report-printing behavior to note that
the CLI exits with code 1 both when npm is unavailable before report generation
and when the generated report’s overallStatus is FAIL, and make sure the
guidance around the CLI output and exit status reflects that split clearly.
- Around line 6-10: Add a top-level H1 heading immediately after the front
matter in SKILL.md so the document has a valid first heading and no longer
triggers MD041. Keep the existing “Goal” content under that new heading, and
make sure the body text starts after the heading instead of directly after the
front matter.
In `@projects/site/src/_11ty/layouts/metadata.js`:
- Around line 351-357: The sentence trimming logic in the metadata description
formatter is cutting off mid-word because the `lastSpace >=
MIN_DESCRIPTION_LENGTH` guard prevents clamping to the last available word
boundary. Update the truncation in the description handling block (the
`trimmed`, `lastSpace`, and `sentence` logic) to always prefer the last space
within the sliced text when one exists, even if the result is shorter than the
minimum threshold, and only fall back to the raw trimmed slice when no boundary
is available. Keep the existing punctuation cleanup and final period handling in
place.
In `@projects/site/src/_11ty/utils/env.js`:
- Around line 10-11: The default URL fallbacks in getEnvValue for
ELEMENTS_PAGES_BASE_URL and ELEMENTS_REPO_BASE_URL should preserve
directory-style trailing slashes. Update the constants in env.js so the fallback
values end with /, keeping the existing symbols unchanged and matching the
expected URL composition contract used by tests.
In `@projects/site/src/docs/about/contributions.md`:
- Line 44: The “Proposing a Feature” sentence in the contributions doc is
grammatically broken and reads like an editing artifact; revise the copy in the
feature-request bullet so it flows naturally while keeping the link to the
feature template. Update the text around the “Proposing a Feature” section to
remove the awkward “using File an issue” phrasing and make the instruction clear
and concise in Google-style documentation language.
- Around line 40-48: The issue is that the issue-creation links still use
GitHub’s deprecated issuable_template parameter, so they won’t preselect the
intended template. Update the links in the contributions guidance to use the
correct template= parameter instead, and make the change consistently in both
the “Create a new issue” and “Proposing a Feature” entries so the bug and
feature templates are selected properly.
In `@projects/starters/eleventy/src/_layouts/index.11ty.js`:
- Around line 4-16: The index layout is inserting dynamic metadata directly into
the HTML head without escaping, which can break attributes or inject markup.
Update the Eleventy layout’s title and meta description handling in the
index.11ty.js template so the values used by the <title> tag and the meta
description attribute are HTML-escaped before interpolation, while keeping the
existing title/description fallback logic intact.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 36b94665-a0b7-4c3c-9315-53c91f16da88
📒 Files selected for processing (27)
.agents/hooks/pre-tool-use-bash.sh.agents/skills/availability-report/SKILL.md.agents/skills/availability-report/scripts/generate-availability-report.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/metadata.test.tsprojects/site/src/_11ty/plugins/llms-txt.jsprojects/site/src/_11ty/plugins/llms-txt.test.tsprojects/site/src/_11ty/transforms/site-urls.jsprojects/site/src/_11ty/transforms/site-urls.test.tsprojects/site/src/_11ty/utils/env.jsprojects/site/src/_11ty/utils/env.test.tsprojects/site/src/docs/about/contributions.mdprojects/site/src/docs/api-design/glossary.11ty.jsprojects/site/src/examples/index.11ty.jsprojects/site/src/examples/index.test.tsprojects/starters/eleventy-ssr/src/index.11ty.jsprojects/starters/eleventy/src/_layouts/index.11ty.jsprojects/starters/eleventy/src/about/index.mdprojects/starters/eleventy/src/index.11ty.jsprojects/starters/eleventy/src/settings/index.mdprojects/starters/go/src/index.htmlprojects/starters/importmaps/src/index.htmlprojects/starters/lit-library/index.htmlprojects/starters/nextjs/src/pages/_app.tsxprojects/starters/nuxt/nuxt.config.tsprojects/starters/vue/index.html
Comment on lines
+16
to
+24
| if [[ -n "${CURSOR_AGENT:-}" ]]; then | ||
| echo "WARNING: Destructive git operation detected." >&2 | ||
| echo " Command: $COMMAND" >&2 | ||
| echo " Reason: $1." >&2 | ||
| echo "" >&2 | ||
| echo "Per AGENTS.md policy, destructive git operations require explicit user confirmation on user machines." >&2 | ||
| echo "In isolated VM environments, this hook is warning instead of blocking." >&2 | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Gate the bypass on an explicit isolated-environment signal, not just CURSOR_AGENT.
At Line 16, CURSOR_AGENT alone disables blocking for destructive git commands. That allows policy bypass anywhere Cursor is used, even outside isolated VMs. Require an additional trusted flag (or explicit mode) before returning exit 0.
Suggested patch
- if [[ -n "${CURSOR_AGENT:-}" ]]; then
+ if [[ -n "${CURSOR_AGENT:-}" && "${AGENTS_ISOLATED_VM:-0}" == "1" ]]; then
echo "WARNING: Destructive git operation detected." >&2
echo " Command: $COMMAND" >&2
echo " Reason: $1." >&2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [[ -n "${CURSOR_AGENT:-}" ]]; then | |
| echo "WARNING: Destructive git operation detected." >&2 | |
| echo " Command: $COMMAND" >&2 | |
| echo " Reason: $1." >&2 | |
| echo "" >&2 | |
| echo "Per AGENTS.md policy, destructive git operations require explicit user confirmation on user machines." >&2 | |
| echo "In isolated VM environments, this hook is warning instead of blocking." >&2 | |
| exit 0 | |
| fi | |
| if [[ -n "${CURSOR_AGENT:-}" && "${AGENTS_ISOLATED_VM:-0}" == "1" ]]; then | |
| echo "WARNING: Destructive git operation detected." >&2 | |
| echo " Command: $COMMAND" >&2 | |
| echo " Reason: $1." >&2 | |
| echo "" >&2 | |
| echo "Per AGENTS.md policy, destructive git operations require explicit user confirmation on user machines." >&2 | |
| echo "In isolated VM environments, this hook is warning instead of blocking." >&2 | |
| exit 0 | |
| fi |
🤖 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 @.agents/hooks/pre-tool-use-bash.sh around lines 16 - 24, The destructive git
guard in pre-tool-use-bash.sh is bypassed too broadly because it exits 0
whenever CURSOR_AGENT is set. Update the conditional in the hook to require an
additional explicit isolated-environment signal or trusted mode before allowing
the warning-only path, and keep blocking otherwise so the bypass is limited to
approved VM contexts.
Comment on lines
+37
to
+38
| The CLI prints the formatted report to standard output. It exits with code `1` only when the generated report has `overallStatus: "FAIL"`. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document environment failures separately from report failures.
The CLI also exits with code 1 before generating any report when npm is unavailable, so “only when the generated report has overallStatus: "FAIL"” is too strong. Callers need that distinction so they do not treat an environment failure as a report result.
Also applies to: 67-69
🤖 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 @.agents/skills/availability-report/SKILL.md around lines 37 - 38, Clarify
the CLI exit behavior in the availability-report SKILL.md so it distinguishes
environment failures from generated report failures. Update the wording around
the report-printing behavior to note that the CLI exits with code 1 both when
npm is unavailable before report generation and when the generated report’s
overallStatus is FAIL, and make sure the guidance around the CLI output and exit
status reflects that split clearly.
coryrylan
force-pushed
the
topic-url-fixes
branch
from
152c283 to
ac1316d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/starters/eleventy-ssr/src/index.11ty.js (1)
38-47: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEscape
data.titleanddata.descriptionbefore interpolating them.Line 45, Line 47, and Line 70 render raw metadata into HTML. Any
"or<in page data will corrupt the generated<meta>,<title>, and heading markup.Proposed fix
export function render(data) { + const escapeHtml = value => + String(value) + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('"', '"') + .replaceAll("'", '&`#39`;'); + const title = escapeHtml(data.title); + const description = escapeHtml(data.description); + return /* html */ ` <!DOCTYPE html> <html lang="en" nve-theme="light" nve-transition="auto"> <head> @@ - <meta name="description" content="${data.description}"> + <meta name="description" content="${description}"> <base href="${process.env.PAGES_BASE_URL}" /> - <title>${data.title}</title> + <title>${title}</title> @@ - <h1 nve-text="heading xl">${data.title}${globalThis.process?.env?.NODE_ENV ?? ''}</h1> + <h1 nve-text="heading xl">${title}${globalThis.process?.env?.NODE_ENV ?? ''}</h1>Also applies to: 70-70
🤖 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 `@projects/starters/eleventy-ssr/src/index.11ty.js` around lines 38 - 47, The render function is interpolating unescaped page metadata directly into HTML, which can break the generated markup when data.title or data.description contains special characters. Update render and the heading output near the later data.title usage to escape these values before inserting them into the <meta>, <title>, and heading content, using the existing render flow in index.11ty.js so all raw metadata is safely encoded.
♻️ Duplicate comments (4)
projects/starters/eleventy/src/_layouts/index.11ty.js (1)
4-16: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEscape
titleanddescriptionbefore writing them into the head.Line 13 and Line 16 interpolate raw page metadata into HTML. A quote,
&, or<in front matter will break the generated<meta>/<title>markup.Proposed fix
export function render(data) { - const title = data.title ?? 'Eleventy Starter'; - const description = data.description ?? 'A simple starter using Elements and Eleventy.'; + const escapeHtml = value => + String(value) + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('"', '"') + .replaceAll("'", '&`#39`;'); + + const title = escapeHtml(data.title ?? 'Eleventy Starter'); + const description = escapeHtml(data.description ?? 'A simple starter using Elements and Eleventy.');🤖 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 `@projects/starters/eleventy/src/_layouts/index.11ty.js` around lines 4 - 16, The page metadata in the Eleventy layout is written directly into the head, so `title` and `description` can break markup if they contain HTML-sensitive characters. Update `index.11ty.js` to escape or sanitize these values before interpolating them into the `<meta name="description">` and `<title>` tags, using the existing `title` and `description` variables in the layout.projects/site/src/docs/about/contributions.md (2)
44-44: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRewrite the feature-request sentence.
Line 44 reads like an editing artifact (
using File an issue) and is unlikely to pass the prose bar for this doc. As per coding guidelines,**/*.md: Follow the Google developer documentation style guide and write-good rules using Vale prose linter.🤖 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 `@projects/site/src/docs/about/contributions.md` at line 44, Rewrite the feature-request sentence in the contributions doc so it reads as polished documentation prose instead of an editing artifact; update the “Proposing a Feature” bullet to use clear, concise wording and proper grammar, and verify the surrounding Markdown in the same section still matches the Google developer documentation style guide and Vale write-good rules.Source: Coding guidelines
40-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse GitHub’s
template=query parameter in these links.Line 40, Line 44, and Line 48 still use
issuable_template=..., so GitHub will open the generic issue form instead of preselecting the intended bug/feature template.🤖 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 `@projects/site/src/docs/about/contributions.md` around lines 40 - 48, The issue is that the issue links in the contributions docs still use GitHub’s old issuable_template query, so the intended templates are not selected. Update the links in the sections around “Create a new issue” and “Proposing a Feature” to use GitHub’s template= query parameter instead, keeping the existing bug and feature destinations aligned with the current docs content.projects/site/src/_11ty/layouts/metadata.js (1)
351-357: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClamp at the last available word boundary.
Line 353 still keeps the raw 160-character slice whenever the last space lands before character 150, so long meta descriptions can still end mid-word.
Proposed fix
- const sentence = (lastSpace >= MIN_DESCRIPTION_LENGTH ? trimmed.slice(0, lastSpace) : trimmed) + const sentence = (lastSpace >= 0 ? trimmed.slice(0, lastSpace) : trimmed) .replace(/[,:;—-]+$/u, '') .replace(/[.?!]$/u, '');🤖 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 `@projects/site/src/_11ty/layouts/metadata.js` around lines 351 - 357, The description truncation in the metadata formatter still falls back to the raw MAX_DESCRIPTION_LENGTH slice when lastSpace is before MIN_DESCRIPTION_LENGTH, which can leave a word cut off mid-token. Update the logic in the metadata.js description handling to clamp at the last available word boundary within the trimmed slice instead of using the unbroken slice, so the returned sentence always ends on a whole word; use the existing trimmed, lastSpace, and sentence flow to implement the fix.
🤖 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 @.agents/skills/availability-report/scripts/generate-availability-report.js:
- Around line 329-334: The normal report object returned by the report-building
path is missing overallStatus, so main() can’t detect FAIL items and exits
successfully when it shouldn’t. Update the return object in
generate-availability-report.js (the same place that already returns docs,
notes, packages, and timestamp) to compute and include overallStatus based on
the generated report contents, using the existing report data structures so
main() can correctly set the exit code.
- Around line 378-380: The module contract is exposing generateReport() in
SKILL.md, but the symbol is not exported from generate-availability-report.js,
so consumers cannot call it outside the CLI path. Update the module’s exports to
include generateReport alongside the existing CLI entrypoint logic, and ensure
the exported function still returns the formattedReport and report object from
runAvailabilityReport and formatAvailabilityReport.
In `@projects/site/src/_11ty/plugins/llms-txt.js`:
- Around line 33-35: The fallback title in llms-txt.js is being appended with
the same “NVIDIA Elements context” suffix twice when markdown has no heading,
causing a duplicated page title in the default path. Update the title-building
logic around the object that sets title and description, and the later title
formatting code that appends the suffix, so the fallback title is only applied
once; use the existing title/description flow in llms-txt.js to keep the
no-heading case from re-adding the same text.
---
Outside diff comments:
In `@projects/starters/eleventy-ssr/src/index.11ty.js`:
- Around line 38-47: The render function is interpolating unescaped page
metadata directly into HTML, which can break the generated markup when
data.title or data.description contains special characters. Update render and
the heading output near the later data.title usage to escape these values before
inserting them into the <meta>, <title>, and heading content, using the existing
render flow in index.11ty.js so all raw metadata is safely encoded.
---
Duplicate comments:
In `@projects/site/src/_11ty/layouts/metadata.js`:
- Around line 351-357: The description truncation in the metadata formatter
still falls back to the raw MAX_DESCRIPTION_LENGTH slice when lastSpace is
before MIN_DESCRIPTION_LENGTH, which can leave a word cut off mid-token. Update
the logic in the metadata.js description handling to clamp at the last available
word boundary within the trimmed slice instead of using the unbroken slice, so
the returned sentence always ends on a whole word; use the existing trimmed,
lastSpace, and sentence flow to implement the fix.
In `@projects/site/src/docs/about/contributions.md`:
- Line 44: Rewrite the feature-request sentence in the contributions doc so it
reads as polished documentation prose instead of an editing artifact; update the
“Proposing a Feature” bullet to use clear, concise wording and proper grammar,
and verify the surrounding Markdown in the same section still matches the Google
developer documentation style guide and Vale write-good rules.
- Around line 40-48: The issue is that the issue links in the contributions docs
still use GitHub’s old issuable_template query, so the intended templates are
not selected. Update the links in the sections around “Create a new issue” and
“Proposing a Feature” to use GitHub’s template= query parameter instead, keeping
the existing bug and feature destinations aligned with the current docs content.
In `@projects/starters/eleventy/src/_layouts/index.11ty.js`:
- Around line 4-16: The page metadata in the Eleventy layout is written directly
into the head, so `title` and `description` can break markup if they contain
HTML-sensitive characters. Update `index.11ty.js` to escape or sanitize these
values before interpolating them into the `<meta name="description">` and
`<title>` tags, using the existing `title` and `description` variables in the
layout.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 86289990-2c4d-4985-832f-b5c9ce49d639
📒 Files selected for processing (27)
.agents/hooks/pre-tool-use-bash.sh.agents/skills/availability-report/SKILL.md.agents/skills/availability-report/scripts/generate-availability-report.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/metadata.test.tsprojects/site/src/_11ty/plugins/llms-txt.jsprojects/site/src/_11ty/plugins/llms-txt.test.tsprojects/site/src/_11ty/transforms/site-urls.jsprojects/site/src/_11ty/transforms/site-urls.test.tsprojects/site/src/_11ty/utils/env.jsprojects/site/src/_11ty/utils/env.test.tsprojects/site/src/docs/about/contributions.mdprojects/site/src/docs/api-design/glossary.11ty.jsprojects/site/src/examples/index.11ty.jsprojects/site/src/examples/index.test.tsprojects/starters/eleventy-ssr/src/index.11ty.jsprojects/starters/eleventy/src/_layouts/index.11ty.jsprojects/starters/eleventy/src/about/index.mdprojects/starters/eleventy/src/index.11ty.jsprojects/starters/eleventy/src/settings/index.mdprojects/starters/go/src/index.htmlprojects/starters/importmaps/src/index.htmlprojects/starters/lit-library/index.htmlprojects/starters/nextjs/src/pages/_app.tsxprojects/starters/nuxt/nuxt.config.tsprojects/starters/vue/index.html
Comment on lines
+378
to
+380
| async function generateReport({ timestamp } = {}) { | ||
| const report = await runAvailabilityReport({ timestamp }); | ||
| return { formattedReport: formatAvailabilityReport(report), report }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Export generateReport() to match the documented module contract.
SKILL.md documents const { formattedReport, report } = await generateReport();, but this module does not export that symbol, so the advertised API is unusable outside the CLI entrypoint.
Proposed fix
-async function generateReport({ timestamp } = {}) {
+export async function generateReport({ timestamp } = {}) {
const report = await runAvailabilityReport({ timestamp });
return { formattedReport: formatAvailabilityReport(report), report };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function generateReport({ timestamp } = {}) { | |
| const report = await runAvailabilityReport({ timestamp }); | |
| return { formattedReport: formatAvailabilityReport(report), report }; | |
| export async function generateReport({ timestamp } = {}) { | |
| const report = await runAvailabilityReport({ timestamp }); | |
| return { formattedReport: formatAvailabilityReport(report), report }; |
🤖 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 @.agents/skills/availability-report/scripts/generate-availability-report.js
around lines 378 - 380, The module contract is exposing generateReport() in
SKILL.md, but the symbol is not exported from generate-availability-report.js,
so consumers cannot call it outside the CLI path. Update the module’s exports to
include generateReport alongside the existing CLI entrypoint logic, and ensure
the exported function still returns the formattedReport and report object from
runAvailabilityReport and formatAvailabilityReport.
ac1316d to
1cebe5f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
.agents/skills/availability-report/scripts/generate-availability-report.js (3)
378-380: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExport
generateReport()or drop the documented API contract.
SKILL.mdadvertisesawait generateReport(), but this module keeps the function private. Consumers following the documented contract cannot import it.Suggested fix
-async function generateReport({ timestamp } = {}) { +export async function generateReport({ timestamp } = {}) { const report = await runAvailabilityReport({ timestamp }); return { formattedReport: formatAvailabilityReport(report), report }; }🤖 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 @.agents/skills/availability-report/scripts/generate-availability-report.js around lines 378 - 380, The documented API contract exposes generateReport(), but the implementation keeps it private inside generate-availability-report.js. Export generateReport from this module so consumers can import and await it as documented, and make sure the export is wired alongside the existing runAvailabilityReport and formatAvailabilityReport usage.
329-334: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPopulate
overallStatuson the normal report path.
main()usesreport.overallStatusto decide the exit code, but the success path never sets it. Any docs/packageFAILcurrently prints a failing report and still exits0.Suggested fix
+ const statuses = [...docs.map(doc => doc.status), ...packages.map(pkg => pkg.status)]; + const overallStatus = statuses.includes('FAIL') + ? 'FAIL' + : statuses.includes('WARN') + ? 'WARN' + : 'PASS'; + return { docs, notes, + overallStatus, packages, timestamp: normalizedTimestamp.value };Also applies to: 448-453
🤖 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 @.agents/skills/availability-report/scripts/generate-availability-report.js around lines 329 - 334, The normal report object returned from generate-availability-report.js does not set overallStatus, so main() cannot detect failures and may exit 0 even when docs or packages contain FAIL entries. Update the success-path report निर्माण near the return object to compute and include overallStatus based on the collected docs/packages results, and make sure the same logic is applied in the other matching return path so report.overallStatus stays consistent for main().
239-242: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winDisable lifecycle scripts in the temp install.
This verification flow only needs package metadata and resolution, but
npm installhere will still execute package lifecycle hooks. That lets package code run on the host during a read-only report job. Add--ignore-scripts, and keepSKILL.mdin sync with the safer command.Suggested fix
- ? await runCommand('npm', ['install', '--no-audit', '--no-fund', ...packageNames], { cwd: tempDir }) + ? await runCommand('npm', ['install', '--ignore-scripts', '--no-audit', '--no-fund', ...packageNames], { cwd: tempDir })🤖 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 @.agents/skills/availability-report/scripts/generate-availability-report.js around lines 239 - 242, The temp package verification step is running lifecycle hooks during install, which is unnecessary for metadata resolution and can execute package code on the host. Update the npm install invocation in generate-availability-report.js, around the runCommand call used for installCommand, to include --ignore-scripts alongside the existing audit/fund flags. Also update the related SKILL.md guidance so it matches the safer temp-install command..agents/skills/availability-report/SKILL.md (1)
39-40: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClarify non-report failure exits in the skill contract.
The CLI also exits with code
1before printing any report for invalid arguments and whennpmis unavailable, so "only when the generated report hasoverallStatus: "FAIL"" is still too strong. Callers need the contract to distinguish "report says FAIL" from "no report was produced".Also applies to: 59-63, 69-71
🤖 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 @.agents/skills/availability-report/SKILL.md around lines 39 - 40, The availability-report skill contract currently implies the CLI exits with code 1 only when a generated report has overallStatus "FAIL", but it also exits 1 for non-report failures like invalid arguments and missing npm. Update the wording in the skill documentation around the CLI behavior so it clearly distinguishes "report generated with FAIL" from "no report was produced" and references the CLI contract sections describing output and exit codes.projects/starters/eleventy/src/_layouts/index.11ty.js (1)
4-5: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEscape the new head metadata before interpolation.
titleanddescriptionare still written straight into<title>andmeta[name="description"]. This is the same HTML-injection gap flagged in the earlier review; a quote or markup in front matter can still break the generated head.🔧 Proposed fix
export function render(data) { + const escapeHtml = (value) => + String(value) + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('"', '"') + .replaceAll("'", '&`#39`;'); + const title = data.title ?? 'Eleventy Starter'; const description = data.description ?? 'A simple starter using Elements and Eleventy.'; @@ - <meta name="description" content="${description}"> + <meta name="description" content="${escapeHtml(description)}"> @@ - <title>${title}</title> + <title>${escapeHtml(title)}</title>Also applies to: 13-16
🤖 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 `@projects/starters/eleventy/src/_layouts/index.11ty.js` around lines 4 - 5, The `index.11ty.js` layout is interpolating `data.title` and `data.description` directly into the head, which leaves the `<title>` and `meta[name="description"]` content exposed to HTML injection. Update the head generation logic in this layout to escape or sanitize these values before they are inserted, and make sure the same protection is applied wherever the head metadata is rendered in the related markup using `title` and `description`.
🤖 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 `@projects/site/src/_11ty/layouts/metadata.js`:
- Line 393: The breadcrumb URL assembly in getBreadcrumb still uses SITE_URL
directly, which can diverge from the canonicalUrl behavior and produce
mismatched URLs on deployed base paths. Update getBreadcrumb to build breadcrumb
item URLs with the same deployed-site helper used for canonicalUrl (getSiteUrl),
so page.url, canonical, and breadcrumb links stay consistent across the site.
In `@projects/site/src/docs/about/contributions.md`:
- Around line 40-48: The issue is that the issue-template links in the
contributions docs are hardcoded to NVIDIA/elements and one path still uses an
invalid GitHub route, so they won’t respect ELEMENTS_REPO_BASE_URL. Update all
issue links in the contributions content to be built consistently from
ELEMENTS_REPO_BASE_URL using the issues/new path, and fix the broken
`/-/issues/new` reference so every template link points to the correct
repository base.
In `@projects/starters/eleventy-ssr/src/index.11ty.js`:
- Line 45: The description meta tag in the 11ty template is inserting
data.description directly into an attribute, which can break the head if the
text contains quotes or markup. Update the template around the meta description
emission in the index.11ty.js rendering path to HTML-escape the description
value before interpolation, using the existing template helper or escaping
utility already available in this starter.
---
Duplicate comments:
In @.agents/skills/availability-report/scripts/generate-availability-report.js:
- Around line 378-380: The documented API contract exposes generateReport(), but
the implementation keeps it private inside generate-availability-report.js.
Export generateReport from this module so consumers can import and await it as
documented, and make sure the export is wired alongside the existing
runAvailabilityReport and formatAvailabilityReport usage.
- Around line 329-334: The normal report object returned from
generate-availability-report.js does not set overallStatus, so main() cannot
detect failures and may exit 0 even when docs or packages contain FAIL entries.
Update the success-path report निर्माण near the return object to compute and
include overallStatus based on the collected docs/packages results, and make
sure the same logic is applied in the other matching return path so
report.overallStatus stays consistent for main().
- Around line 239-242: The temp package verification step is running lifecycle
hooks during install, which is unnecessary for metadata resolution and can
execute package code on the host. Update the npm install invocation in
generate-availability-report.js, around the runCommand call used for
installCommand, to include --ignore-scripts alongside the existing audit/fund
flags. Also update the related SKILL.md guidance so it matches the safer
temp-install command.
In @.agents/skills/availability-report/SKILL.md:
- Around line 39-40: The availability-report skill contract currently implies
the CLI exits with code 1 only when a generated report has overallStatus "FAIL",
but it also exits 1 for non-report failures like invalid arguments and missing
npm. Update the wording in the skill documentation around the CLI behavior so it
clearly distinguishes "report generated with FAIL" from "no report was produced"
and references the CLI contract sections describing output and exit codes.
In `@projects/starters/eleventy/src/_layouts/index.11ty.js`:
- Around line 4-5: The `index.11ty.js` layout is interpolating `data.title` and
`data.description` directly into the head, which leaves the `<title>` and
`meta[name="description"]` content exposed to HTML injection. Update the head
generation logic in this layout to escape or sanitize these values before they
are inserted, and make sure the same protection is applied wherever the head
metadata is rendered in the related markup using `title` and `description`.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 3f43e2f4-542f-4bf1-9694-b2f24739f7f2
📒 Files selected for processing (27)
.agents/hooks/pre-tool-use-bash.sh.agents/skills/availability-report/SKILL.md.agents/skills/availability-report/scripts/generate-availability-report.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/metadata.test.tsprojects/site/src/_11ty/plugins/llms-txt.jsprojects/site/src/_11ty/plugins/llms-txt.test.tsprojects/site/src/_11ty/transforms/site-urls.jsprojects/site/src/_11ty/transforms/site-urls.test.tsprojects/site/src/_11ty/utils/env.jsprojects/site/src/_11ty/utils/env.test.tsprojects/site/src/docs/about/contributions.mdprojects/site/src/docs/api-design/glossary.11ty.jsprojects/site/src/examples/index.11ty.jsprojects/site/src/examples/index.test.tsprojects/starters/eleventy-ssr/src/index.11ty.jsprojects/starters/eleventy/src/_layouts/index.11ty.jsprojects/starters/eleventy/src/about/index.mdprojects/starters/eleventy/src/index.11ty.jsprojects/starters/eleventy/src/settings/index.mdprojects/starters/go/src/index.htmlprojects/starters/importmaps/src/index.htmlprojects/starters/lit-library/index.htmlprojects/starters/nextjs/src/pages/_app.tsxprojects/starters/nuxt/nuxt.config.tsprojects/starters/vue/index.html
| description = expandShortDescription(data, description); | ||
|
|
||
| const canonicalUrl = `${SITE_URL}${url}`; | ||
| const canonicalUrl = getSiteUrl(url); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep breadcrumb URLs on the same deployed-site helper.
Line 393 now normalizes canonicalUrl with getSiteUrl(url), but getBreadcrumb() still assembles item URLs from SITE_URL on Lines 512-523. For root-relative page.url values, that emits a canonical under the deployed base path and breadcrumb URLs without it, which gives crawlers conflicting URLs for the same page.
Suggested fix
- const itemListElement = [{ '`@type`': 'ListItem', position: 1, name: 'Home', item: `${SITE_URL}/` }];
+ const itemListElement = [{ '`@type`': 'ListItem', position: 1, name: 'Home', item: getSiteUrl('/') }];
@@
- item: `${SITE_URL}${item}`
+ item: getSiteUrl(item)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const canonicalUrl = getSiteUrl(url); | |
| const itemListElement = [{ '`@type`': 'ListItem', position: 1, name: 'Home', item: getSiteUrl('/') }]; | |
| ... | |
| item: getSiteUrl(item) |
🤖 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 `@projects/site/src/_11ty/layouts/metadata.js` at line 393, The breadcrumb URL
assembly in getBreadcrumb still uses SITE_URL directly, which can diverge from
the canonicalUrl behavior and produce mismatched URLs on deployed base paths.
Update getBreadcrumb to build breadcrumb item URLs with the same deployed-site
helper used for canonicalUrl (getSiteUrl), so page.url, canonical, and
breadcrumb links stay consistent across the site.
Comment on lines
+40
to
+48
| 1. **Create a new issue** using the [bug template](https://github.com/NVIDIA/elements/issues/new?template=default), providing as much detail as you can about the problem and how to reproduce it. | ||
|
|
||
| ## Feature Requests | ||
|
|
||
| 1. **Proposing a Feature**: If you believe a new feature would benefit the design system, please create a new issue using File an issue with the [feature template]({{ELEMENTS_REPO_BASE_URL}}/-/issues/new?issuable_template=feature) | ||
| 1. **Proposing a Feature**: If you believe a new feature would benefit the design system, create an issue with the [feature template](https://github.com/NVIDIA/elements/issues/new?template=feature). | ||
|
|
||
| ## Bug Fixes | ||
|
|
||
| 1. **Create a new issue** using the [bug template]({{ELEMENTS_REPO_BASE_URL}}/-/issues/new?issuable_template=default), providing as much detail as you can about the problem and how to reproduce it. | ||
| 1. **Create a new issue** using the [bug template](https://github.com/NVIDIA/elements/issues/new?template=default), providing as much detail as you can about the problem and how to reproduce it. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Build all issue-template links from ELEMENTS_REPO_BASE_URL consistently.
Lines 40, 44, and 48 now hardcode NVIDIA/elements, so any build that overrides ELEMENTS_REPO_BASE_URL will send contributors to the wrong repository. Line 68 still uses /-/issues/new, which is a broken GitHub path. Please route all four links through {{ELEMENTS_REPO_BASE_URL}}/issues/new?... instead.
Suggested fix
-1. **Create a new issue** using the [bug template](https://github.com/NVIDIA/elements/issues/new?template=default), providing as much detail as you can about the problem and how to reproduce it.
+1. **Create a new issue** using the [bug template]({{ELEMENTS_REPO_BASE_URL}}/issues/new?template=default), providing as much detail as you can about the problem and how to reproduce it.
@@
-1. **Proposing a Feature**: If you believe a new feature would benefit the design system, create an issue with the [feature template](https://github.com/NVIDIA/elements/issues/new?template=feature).
+1. **Proposing a Feature**: If you believe a new feature would benefit the design system, create an issue with the [feature template]({{ELEMENTS_REPO_BASE_URL}}/issues/new?template=feature).
@@
-1. **Create a new issue** using the [bug template](https://github.com/NVIDIA/elements/issues/new?template=default), providing as much detail as you can about the problem and how to reproduce it.
+1. **Create a new issue** using the [bug template]({{ELEMENTS_REPO_BASE_URL}}/issues/new?template=default), providing as much detail as you can about the problem and how to reproduce it.
@@
-File an issue with the [feature template]({{ELEMENTS_REPO_BASE_URL}}/-/issues/new?template=feature).
+File an issue with the [feature template]({{ELEMENTS_REPO_BASE_URL}}/issues/new?template=feature).Also applies to: 68-68
🤖 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 `@projects/site/src/docs/about/contributions.md` around lines 40 - 48, The
issue is that the issue-template links in the contributions docs are hardcoded
to NVIDIA/elements and one path still uses an invalid GitHub route, so they
won’t respect ELEMENTS_REPO_BASE_URL. Update all issue links in the
contributions content to be built consistently from ELEMENTS_REPO_BASE_URL using
the issues/new path, and fix the broken `/-/issues/new` reference so every
template link points to the correct repository base.
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <meta name="description" content="${data.description}"> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Escape the new description meta value.
data.description is interpolated directly into an attribute. If the starter description ever contains quotes or markup, the head breaks; please HTML-escape it before emission.
🤖 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 `@projects/starters/eleventy-ssr/src/index.11ty.js` at line 45, The description
meta tag in the 11ty template is inserting data.description directly into an
attribute, which can break the head if the text contains quotes or markup.
Update the template around the meta description emission in the index.11ty.js
rendering path to HTML-escape the description value before interpolation, using
the existing template helper or escaping utility already available in this
starter.
1cebe5f to
9f0f42e
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (3)
.agents/hooks/pre-tool-use-bash.sh (1)
10-12: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winVerify the
CURSOR_SANDBOXpolarity before relying on it to downgrade blocking to warning.This is the security-critical gate: when
CURSOR_AGENTis set andCURSOR_SANDBOXis unset, the hook treats the session as a cloud agent andhandle_blocked_operationlater routes destructive git commands towarn(exit 0) instead ofblock(exit 2). The previous review asked for an explicit isolated-environment signal; this now keys offCURSOR_SANDBOXabsence rather than a dedicated isolated-VM flag.Two residual concerns:
- If
CURSOR_SANDBOXis actually set inside the isolated VM (and absent on local/privileged contexts), the condition is inverted and would downgrade blocking exactly where it should stay strict — a policy bypass.CURSOR_AGENTis matched via${CURSOR_AGENT+x}, so even an empty value triggers cloud mode; confirm that's intended.Please confirm the runtime semantics of both variables before merge.
In Cursor cloud agents and background agents, are the CURSOR_AGENT and CURSOR_SANDBOX environment variables set, and what values do they take in each environment?🤖 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 @.agents/hooks/pre-tool-use-bash.sh around lines 10 - 12, The cloud-agent detection in pre-tool-use-bash.sh may be inverted because it treats CURSOR_SANDBOX being unset as the signal to set NVE_AGENT, which could downgrade destructive git handling in the wrong environment. Verify the intended runtime semantics of CURSOR_AGENT and CURSOR_SANDBOX, then update the conditional in the shell hook so handle_blocked_operation only switches to warn for the truly isolated cloud/background agent case; also confirm whether `${CURSOR_AGENT+x}` should allow an empty CURSOR_AGENT value to count as set.projects/starters/eleventy-ssr/src/index.11ty.js (1)
45-45: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEscape the new description meta value.
data.descriptionis still interpolated directly into a meta attribute, so quotes or markup will break the head. This was already called out in the previous review and remains unresolved.🤖 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 `@projects/starters/eleventy-ssr/src/index.11ty.js` at line 45, The description value in the Eleventy SSR template is still being injected directly into the meta tag attribute, which can break the head when it contains quotes or markup. Update the template in the same area that renders the description meta tag to escape/sanitize data.description before interpolation, using the existing head/template escaping approach used elsewhere in index.11ty.js.projects/starters/eleventy/src/_layouts/index.11ty.js (1)
4-16: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEscape the dynamic head values before interpolation.
data.titleanddata.descriptionare still written raw into<title>/meta, so quotes or markup in front matter can break the head or inject HTML. This is the same issue previously flagged and it remains unresolved.🤖 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 `@projects/starters/eleventy/src/_layouts/index.11ty.js` around lines 4 - 16, Escape the dynamic head values before interpolating them in the Eleventy layout: the `title` and `description` values derived from `data.title` and `data.description` are still being written directly into the `<title>` and meta description tag. Update the layout logic in the `index.11ty.js` template so these values are sanitized/escaped before rendering, preserving the existing fallbacks while preventing broken markup or injected HTML.
🤖 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.
Duplicate comments:
In @.agents/hooks/pre-tool-use-bash.sh:
- Around line 10-12: The cloud-agent detection in pre-tool-use-bash.sh may be
inverted because it treats CURSOR_SANDBOX being unset as the signal to set
NVE_AGENT, which could downgrade destructive git handling in the wrong
environment. Verify the intended runtime semantics of CURSOR_AGENT and
CURSOR_SANDBOX, then update the conditional in the shell hook so
handle_blocked_operation only switches to warn for the truly isolated
cloud/background agent case; also confirm whether `${CURSOR_AGENT+x}` should
allow an empty CURSOR_AGENT value to count as set.
In `@projects/starters/eleventy-ssr/src/index.11ty.js`:
- Line 45: The description value in the Eleventy SSR template is still being
injected directly into the meta tag attribute, which can break the head when it
contains quotes or markup. Update the template in the same area that renders the
description meta tag to escape/sanitize data.description before interpolation,
using the existing head/template escaping approach used elsewhere in
index.11ty.js.
In `@projects/starters/eleventy/src/_layouts/index.11ty.js`:
- Around line 4-16: Escape the dynamic head values before interpolating them in
the Eleventy layout: the `title` and `description` values derived from
`data.title` and `data.description` are still being written directly into the
`<title>` and meta description tag. Update the layout logic in the
`index.11ty.js` template so these values are sanitized/escaped before rendering,
preserving the existing fallbacks while preventing broken markup or injected
HTML.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: cdaffba2-2c7f-4ca2-8334-b7f69f68e134
📒 Files selected for processing (27)
.agents/hooks/pre-tool-use-bash.sh.agents/skills/availability-report/SKILL.md.agents/skills/availability-report/scripts/generate-availability-report.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/metadata.test.tsprojects/site/src/_11ty/plugins/llms-txt.jsprojects/site/src/_11ty/plugins/llms-txt.test.tsprojects/site/src/_11ty/transforms/site-urls.jsprojects/site/src/_11ty/transforms/site-urls.test.tsprojects/site/src/_11ty/utils/env.jsprojects/site/src/_11ty/utils/env.test.tsprojects/site/src/docs/about/contributions.mdprojects/site/src/docs/api-design/glossary.11ty.jsprojects/site/src/examples/index.11ty.jsprojects/site/src/examples/index.test.tsprojects/starters/eleventy-ssr/src/index.11ty.jsprojects/starters/eleventy/src/_layouts/index.11ty.jsprojects/starters/eleventy/src/about/index.mdprojects/starters/eleventy/src/index.11ty.jsprojects/starters/eleventy/src/settings/index.mdprojects/starters/go/src/index.htmlprojects/starters/importmaps/src/index.htmlprojects/starters/lit-library/index.htmlprojects/starters/nextjs/src/pages/_app.tsxprojects/starters/nuxt/nuxt.config.tsprojects/starters/vue/index.html
- Enhanced metadata handling in various layout files to improve SEO and user experience. - Introduced a new skill to verify the availability of NVIDIA Elements packages on npm and check the documentation site status. - Added a deterministic script for generating the availability report, including npm metadata checks and temporary project management. - Updated the pre-tool use hook to warn about destructive git operations in isolated VM environments. Signed-off-by: Cory Rylan <crylan@nvidia.com>
coryrylan
force-pushed
the
topic-url-fixes
branch
from
9f0f42e to
513957c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
.agents/hooks/pre-tool-use-bash.sh (1)
10-12: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire an explicit trusted isolation signal before warning-only mode.
warn(exit0) is currently reachable via agent/env identity (NVE_AGENTcontainscloud, or auto-derived fromCURSOR_AGENT) rather than a trusted isolation flag, so destructive-git blocking can be bypassed in non-isolated contexts.Suggested fix
-if [[ -z "${NVE_AGENT:-}" && -n "${CURSOR_AGENT+x}" && -z "${CURSOR_SANDBOX+x}" ]]; then +if [[ -z "${NVE_AGENT:-}" && -n "${CURSOR_AGENT+x}" && "${AGENTS_ISOLATED_VM:-0}" == "1" ]]; then export NVE_AGENT="cursor-cloud-agent" fi @@ handle_blocked_operation() { - if [[ "${NVE_AGENT:-}" == *cloud* ]]; then + if [[ "${AGENTS_ISOLATED_VM:-0}" == "1" && "${NVE_AGENT:-}" == *cloud* ]]; then warn "$1" fiAlso applies to: 39-42
🤖 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 @.agents/hooks/pre-tool-use-bash.sh around lines 10 - 12, The warning-only path in pre-tool-use-bash.sh is too loosely gated because NVE_AGENT can be auto-derived from CURSOR_AGENT and then treated as a trusted signal. Update the logic around the NVE_AGENT setup and the warning/exit decision so warn mode is allowed only when an explicit isolation flag is present, and do not infer trust from agent identity alone. Make the change in the conditional flow that currently checks NVE_AGENT, CURSOR_AGENT, and CURSOR_SANDBOX so destructive-git blocking cannot be bypassed outside a verified isolated context.projects/site/src/_11ty/plugins/llms-txt.js (1)
33-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFallback title is duplicated in no-heading pages.
When
titlefalls back to"NVIDIA Elements context", Line 50 appends the same suffix again, producing"NVIDIA Elements context | NVIDIA Elements context".Proposed fix
- return { - title: title || 'NVIDIA Elements context', - description: description || 'NVIDIA Elements context fragment for AI and LLM tools.' - }; + return { + title, + description: description || 'NVIDIA Elements context fragment for AI and LLM tools.' + }; @@ - `<!doctype html><html lang="en"><head><meta charset="utf-8"><meta name="robots" content="noindex,follow"><meta name="description" content="${escapeAttr(meta.description)}"><link rel="canonical" href="${htmlUrl}"><link rel="alternate" type="text/markdown" title="Markdown version" href="${getContextUrl(path, '.md')}" /><meta name="viewport" content="width=device-width, initial-scale=1"><title>${escapeAttr(meta.title)} | NVIDIA Elements context</title><style>${CSS}</style></head><body>${nav}<main>${renderedMarkdown}</main></body></html>` + `<!doctype html><html lang="en"><head><meta charset="utf-8"><meta name="robots" content="noindex,follow"><meta name="description" content="${escapeAttr(meta.description)}"><link rel="canonical" href="${htmlUrl}"><link rel="alternate" type="text/markdown" title="Markdown version" href="${getContextUrl(path, '.md')}" /><meta name="viewport" content="width=device-width, initial-scale=1"><title>${escapeAttr(meta.title ? `${meta.title} | NVIDIA Elements context` : 'NVIDIA Elements context')}</title><style>${CSS}</style></head><body>${nav}<main>${renderedMarkdown}</main></body></html>`Also applies to: 50-50
🤖 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 `@projects/site/src/_11ty/plugins/llms-txt.js` around lines 33 - 34, The fallback title in llmsTxt plugin duplicates the same text for no-heading pages, so adjust the title handling in the llmsTxt generation flow to avoid appending “| NVIDIA Elements context” when the base title is already “NVIDIA Elements context.” Update the logic around the title default and the final title composition in the llmsTxt function so the output remains a single, clean title string.projects/starters/eleventy-ssr/src/index.11ty.js (1)
45-47: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEscape metadata/title values before rendering head tags.
data.description(attribute context) anddata.title(HTML text context) are emitted unescaped; escape both before interpolation.🤖 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 `@projects/starters/eleventy-ssr/src/index.11ty.js` around lines 45 - 47, The head tag rendering in the eleventy template is interpolating unescaped values from data.description and data.title, so update the index.11ty.js template to escape both before insertion. Use the existing head tag rendering logic around the meta description and title output, and make sure the description is escaped for attribute context while the title is escaped for HTML text context.projects/starters/eleventy/src/_layouts/index.11ty.js (1)
4-16: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEscape
titleanddescriptionbefore HTML interpolation.
data.titleanddata.descriptionare inserted directly into text/attribute contexts; they should be escaped to prevent broken head markup and injected HTML.Proposed fix
export function render(data) { - const title = data.title ?? 'Eleventy Starter'; - const description = data.description ?? 'A simple starter using Elements and Eleventy.'; + const escapeHtml = (value) => + String(value) + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('"', '"') + .replaceAll("'", '&`#39`;'); + const title = escapeHtml(data.title ?? 'Eleventy Starter'); + const description = escapeHtml(data.description ?? 'A simple starter using Elements and Eleventy.');🤖 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 `@projects/starters/eleventy/src/_layouts/index.11ty.js` around lines 4 - 16, Escape the interpolated head values in the Eleventy layout so user-provided content cannot break markup or inject HTML. Update the index.11ty.js layout to sanitize both title and description before they are inserted into the <title> element and meta description attribute, using the existing layout variables for those values so the rest of the template remains unchanged.
🤖 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 @.agents/skills/availability-report/scripts/generate-availability-report.js:
- Around line 293-297: The normalizeTimestamp() helper currently accepts any
Date-parsable input, which allows local-time strings to slip through; update the
timestamp validation to reject non-UTC values by requiring an explicit trailing
Z (or equivalent UTC indicator) before constructing the Date. Keep the logic in
generate-availability-report.js centered in normalizeTimestamp() and ensure the
--timestamp path returns a failure reason for inputs like ISO strings without
UTC designators.
In `@projects/starters/nextjs/src/pages/_app.tsx`:
- Around line 10-11: The metadata in the _app page uses inconsistent branding;
update the title and description text to use “Next.js” instead of “NextJS”. Make
the change in the JSX metadata block in _app so both the <title> and the meta
description match the requested naming convention.
---
Duplicate comments:
In @.agents/hooks/pre-tool-use-bash.sh:
- Around line 10-12: The warning-only path in pre-tool-use-bash.sh is too
loosely gated because NVE_AGENT can be auto-derived from CURSOR_AGENT and then
treated as a trusted signal. Update the logic around the NVE_AGENT setup and the
warning/exit decision so warn mode is allowed only when an explicit isolation
flag is present, and do not infer trust from agent identity alone. Make the
change in the conditional flow that currently checks NVE_AGENT, CURSOR_AGENT,
and CURSOR_SANDBOX so destructive-git blocking cannot be bypassed outside a
verified isolated context.
In `@projects/site/src/_11ty/plugins/llms-txt.js`:
- Around line 33-34: The fallback title in llmsTxt plugin duplicates the same
text for no-heading pages, so adjust the title handling in the llmsTxt
generation flow to avoid appending “| NVIDIA Elements context” when the base
title is already “NVIDIA Elements context.” Update the logic around the title
default and the final title composition in the llmsTxt function so the output
remains a single, clean title string.
In `@projects/starters/eleventy-ssr/src/index.11ty.js`:
- Around line 45-47: The head tag rendering in the eleventy template is
interpolating unescaped values from data.description and data.title, so update
the index.11ty.js template to escape both before insertion. Use the existing
head tag rendering logic around the meta description and title output, and make
sure the description is escaped for attribute context while the title is escaped
for HTML text context.
In `@projects/starters/eleventy/src/_layouts/index.11ty.js`:
- Around line 4-16: Escape the interpolated head values in the Eleventy layout
so user-provided content cannot break markup or inject HTML. Update the
index.11ty.js layout to sanitize both title and description before they are
inserted into the <title> element and meta description attribute, using the
existing layout variables for those values so the rest of the template remains
unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: bb0b7a91-389e-4959-a1b4-f5a183f43a38
📒 Files selected for processing (27)
.agents/hooks/pre-tool-use-bash.sh.agents/skills/availability-report/SKILL.md.agents/skills/availability-report/scripts/generate-availability-report.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/metadata.test.tsprojects/site/src/_11ty/plugins/llms-txt.jsprojects/site/src/_11ty/plugins/llms-txt.test.tsprojects/site/src/_11ty/transforms/site-urls.jsprojects/site/src/_11ty/transforms/site-urls.test.tsprojects/site/src/_11ty/utils/env.jsprojects/site/src/_11ty/utils/env.test.tsprojects/site/src/docs/about/contributions.mdprojects/site/src/docs/api-design/glossary.11ty.jsprojects/site/src/examples/index.11ty.jsprojects/site/src/examples/index.test.tsprojects/starters/eleventy-ssr/src/index.11ty.jsprojects/starters/eleventy/src/_layouts/index.11ty.jsprojects/starters/eleventy/src/about/index.mdprojects/starters/eleventy/src/index.11ty.jsprojects/starters/eleventy/src/settings/index.mdprojects/starters/go/src/index.htmlprojects/starters/importmaps/src/index.htmlprojects/starters/lit-library/index.htmlprojects/starters/nextjs/src/pages/_app.tsxprojects/starters/nuxt/nuxt.config.tsprojects/starters/vue/index.html
Comment on lines
+293
to
+297
| function normalizeTimestamp(timestamp = new Date()) { | ||
| const date = timestamp instanceof Date ? timestamp : new Date(timestamp); | ||
| return Number.isNaN(date.getTime()) | ||
| ? { ok: false, reason: `invalid timestamp: ${timestamp}` } | ||
| : { ok: true, value: date.toISOString().replace(/\.\d{3}Z$/, 'Z') }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant function and nearby timestamp handling.
file=".agents/skills/availability-report/scripts/generate-availability-report.js"
wc -l "$file"
sed -n '250,340p' "$file"
# Look for timestamp parsing/validation and CLI arg handling.
rg -n "normalizeTimestamp|timestamp|toISOString|UTC|Z$|--timestamp" "$file" .agents/skills/availability-report -g '!**/node_modules/**'Repository: NVIDIA/elements
Length of output: 10872
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe JavaScript Date parsing for a timestamp without timezone.
node - <<'JS'
const input = '2026-06-24T00:00:00';
for (const tz of ['UTC', 'America/Los_Angeles', 'Asia/Tokyo']) {
process.env.TZ = tz;
const d = new Date(input);
console.log(tz, d.toISOString(), d.getTime());
}
JSRepository: NVIDIA/elements
Length of output: 305
Reject non-UTC --timestamp values.
normalizeTimestamp() accepts any Date-parsable string, so inputs like 2026-06-24T00:00:00 are interpreted in the local timezone and can produce different output across machines. The --timestamp contract should be limited to UTC timestamps (for example, require a trailing Z).
🤖 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 @.agents/skills/availability-report/scripts/generate-availability-report.js
around lines 293 - 297, The normalizeTimestamp() helper currently accepts any
Date-parsable input, which allows local-time strings to slip through; update the
timestamp validation to reject non-UTC values by requiring an explicit trailing
Z (or equivalent UTC indicator) before constructing the Date. Keep the logic in
generate-availability-report.js centered in normalizeTimestamp() and ensure the
--timestamp path returns a failure reason for inputs like ISO strings without
UTC designators.
Comment on lines
+10
to
+11
| <title>NVIDIA Elements + NextJS</title> | ||
| <meta name="description" content="A simple starter using Elements and NextJS." /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
What is the official product name and spelling used in the current Next.js documentation and homepage?
💡 Result:
The official product name and spelling is Next.js [1][2][3]. The name is consistently written as Next.js, with a capital N, a lowercase e, x, t, a period, and a lowercase j followed by a lowercase s [1][2]. The repository and official documentation prioritize this exact casing and punctuation [1][3].
Citations:
Use “Next.js” in the metadata text. NextJS should be changed in both the title and description for consistent branding.
🤖 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 `@projects/starters/nextjs/src/pages/_app.tsx` around lines 10 - 11, The
metadata in the _app page uses inconsistent branding; update the title and
description text to use “Next.js” instead of “NextJS”. Make the change in the
JSX metadata block in _app so both the <title> and the meta description match
the requested naming convention.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit