Generate sitemap lastmod dates from git history - #20
Merged
Merged
Conversation
The lastmod dates in sitemap.xml were maintained by hand and had drifted months behind the pages they describe, and /legal/ carried no lastmod at all. Add scripts/generate-sitemap.sh, which takes each URL's lastmod from the committer date of the most recent commit touching that page's index.php. The URL list, changefreq, and priority stay explicit in a PAGES table at the top of the script; the script fails if a page exists on disk but is missing from that table, and refuses to run against a shallow clone rather than silently emitting wrong dates. Wire it into both existing workflows: Website checks now fails when the committed sitemap.xml differs from what the script produces, and the production deploy regenerates it just before the SFTP upload. Both checkouts move to fetch-depth 0 so commit dates are available, and the deploy excludes scripts/ from the upload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTJ2WWC1QQxAmMeu75hvbT
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTJ2WWC1QQxAmMeu75hvbT
SonarCloud's quality gate flagged a security issue on the new code. Both temp paths the change introduced were predictable and therefore open to a symlink attack: sitemap.xml.tmp.$PID in the generator, and a hardcoded /tmp/sitemap.expected.xml in the workflow. Both now use mktemp. mktemp creates the file 0600, so the generator restores mode 644 before moving it into place - sitemap.xml is served publicly. Also inline the usage text rather than sed-ing it back out of $0, and regenerate sitemap.xml after merging main: PR #19 edited the HA clustering guide, so that page's lastmod is now 2026-09-18. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTJ2WWC1QQxAmMeu75hvbT
SonarCloud's quality gate failed on a single finding: S5332 "Make sure that using clear-text protocols is safe here" on the http:// in the urlset namespace declaration. That one has to stay as it is. The sitemap protocol defines the namespace as the literal string http://www.sitemaps.org/schemas/sitemap/0.9; it is an identifier that is never dereferenced, and crawlers match it byte for byte, so an https variant would simply not be a sitemap namespace. The same URI is already in the committed sitemap.xml - the warning appeared only because the string now also lives in a shell script. Suppress it on that line with NOSONAR and say why in a comment. The generated XML is byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTJ2WWC1QQxAmMeu75hvbT
|
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
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
2 participants
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.
Requested by Alex · project thread
Before:
sitemap.xmlwas maintained by hand. Itslastmoddates said 2026-07-18 or 2026-08-09 while every page had actually been edited on 2026-09-07, so crawlers were told the site was one to two months staler than it is./legal/carried nolastmodat all. Nothing kept the file honest, so the next content change would have put it back out of date.After:
sitemap.xmlis generated. Every URL'slastmodis the date of the most recent commit that touched that page'sindex.php,/legal/has one, and CI fails if the committed file stops matching git history.Eleven of the twelve dates come out as 2026-09-07, because commit
2fa9097("Fix SonarCloud code findings") touched all twelve pages that day. The twelfth,/guides/rtmp-server-ha-clustering/, reads 2026-09-18 after #19 landed — which is the mechanism doing exactly its job. The rest will spread apart the same way as pages get edited individually.How
scripts/generate-sitemap.shreads each date fromgit log -1 --format=%cs -- <page>and writes the whole file. The URL list,changefreq, andprioritystay explicit in aPAGEStable at the top of the script rather than being inferred, so crawl hints remain a deliberate choice. Two guards keep it from lying quietly: it refuses to run in a shallow clone, where commit dates would be wrong, and it fails if anindex.phpexists on disk but is missing from the table — the drift that step 5 of "Adding a guide" used to invite.Both existing workflows now use it, and both checkouts move to
fetch-depth: 0so the history is actually there:sitemap.xml, failing with the command to run. The existingxmllintvalidation is untouched.scripts/is added to the deploy exclude list so the script itself is not published to the web root.The drift check hard-fails, which means a PR that edits a page also needs its regenerated
sitemap.xmlcommitted. That is stable under this repo's merge-commit history, since a merge preserves the original commit date of the file. If the repo ever switches to squash merges, a squash landing on a different day than the PR commit would shift the date and need onescripts/generate-sitemap.shrun onmain.One suppression worth a look
SonarCloud failed the quality gate on S5332, "using clear-text protocols", pointing at the
http://inxmlns="http://www.sitemaps.org/schemas/sitemap/0.9".That one cannot be changed. The sitemap protocol defines the namespace as that literal string; it is an identifier that is never dereferenced, and crawlers match it byte for byte, so an
httpsvariant would simply not be a sitemap namespace. The same URI is already in the committedsitemap.xmland always has been — the warning only appeared because the string now also lives in a shell script, where Sonar's generic text scan sees it.It is suppressed with
NOSONARand a comment explaining why. Marking that hotspot "Safe" in the SonarCloud UI instead, and dropping theNOSONAR, would be the tidier long-term answer if you would rather keep suppressions out of the source.Verified
scripts/generate-sitemap.shoutput is byte-identical to the committedsitemap.xml(the CI drift check, run locally)xmllint --noout sitemap.xmlpassesshellcheck -s sh scripts/generate-sitemap.shis cleanfind . -name '*.php' | xargs -n1 php -lpasses--depth 1cloneguides/fake-page/index.phpis addedNOSONARcomment🤖 Generated with Claude Code
https://claude.ai/code/session_01NTJ2WWC1QQxAmMeu75hvbT