feat: add dynamic sitemap route map#32
Conversation
|
Warning @theDevCx this pull request is linked to an issue that is already assigned to another user. Please link it to an open issue assigned to you or to an unassigned open issue. |
📝 WalkthroughWalkthroughThis PR implements dynamic sitemap and route-map generation for apps and plugins. A new 🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/worker.ts (1)
65-68: 💤 Low valueOptional: Route-map responses bypass logging.
The early return means
/sitemap.xml,/routes.json, and/sitemap.jsonnever reach the sampling/logging logic used for health, RPC, and route handlers. This is likely intentional for low-traffic public endpoints, but consider adding minimal logging if observability is needed later.tests/sitemap.test.ts (1)
14-14: ⚡ Quick winImprove mock robustness.
The fetch mock returns the same response for all URLs and doesn't restore
globalThis.fetchafter the test. Consider:
- Verifying the GitHub API URL in the mock
- Restoring the original fetch in an
afterEachor try-finally blockThe current mock works because the data is small (< 100 items, so pagination stops), but it's fragile.
🔧 Suggested mock improvement
test('builds JSON route map from active GitHub repos', async () => { - globalThis.fetch = (async () => Response.json(githubResponse)) as unknown as typeof fetch + const originalFetch = globalThis.fetch + globalThis.fetch = (async (url: string) => { + if (url.includes('api.github.com/orgs/ubiquity/repos')) { + return Response.json(githubResponse) + } + return originalFetch(url) + }) as unknown as typeof fetch const entries = await buildRouteMap('https://ubq.fi') expect(entries).toEqual([ // ... expectations ]) + + globalThis.fetch = originalFetch })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9b2104d-4f82-4bfd-bfa8-cf27f22b84f6
📒 Files selected for processing (3)
src/sitemap.tssrc/worker.tstests/sitemap.test.ts
| export async function handleRouteMap(requestUrl: URL): Promise<Response | null> { | ||
| if (requestUrl.pathname === '/sitemap.xml') { | ||
| const entries = await buildRouteMap(requestUrl.origin) | ||
| return new Response(toSitemapXml(entries), { | ||
| headers: { ...CACHE_HEADERS, 'Content-Type': 'application/xml; charset=utf-8' }, | ||
| }) | ||
| } | ||
|
|
||
| if (requestUrl.pathname === '/routes.json' || requestUrl.pathname === '/sitemap.json') { | ||
| const entries = await buildRouteMap(requestUrl.origin) | ||
| return new Response(JSON.stringify({ generatedAt: new Date().toISOString(), entries }, null, 2), { | ||
| headers: { ...CACHE_HEADERS, 'Content-Type': 'application/json; charset=utf-8' }, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant parts of the file
FILE="src/sitemap.ts"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE"
exit 1
fi
echo "=== ${FILE} (around handleRouteMap) ==="
sed -n '1,220p' "$FILE" | nl -ba | sed -n '1,220p' | sed -n '1,220p'
echo "=== ${FILE} (search for fetchOrgRepos / buildRouteMap / originForHost / escapeXml) ==="
rg -n "fetchOrgRepos|buildRouteMap|originForHost|escapeXml|toSitemapXml|handleRouteMap" "$FILE" || true
echo "=== ${FILE} (exact definitions) ==="
# Print full function bodies (best-effort by naive line ranges)
# If functions are large, we at least capture their internals around matches.
for pat in "export async function handleRouteMap" "async function buildRouteMap" "async function fetchOrgRepos" "function originForHost" "function escapeXml"; do
line=$(rg -n "$pat" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-5))
end=$((line+220))
echo "--- $pat at line $line (showing $start-$end) ---"
sed -n "${start},${end}p" "$FILE" | nl -ba | sed -n "1,260p"
fi
doneRepository: ubiquity/ubq.fi-router
Length of output: 175
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/sitemap.ts"
[ -f "$FILE" ] || { echo "Missing $FILE"; exit 1; }
echo "=== Lines around handleRouteMap ==="
line=$(rg -n "export async function handleRouteMap" "$FILE" | head -n1 | cut -d: -f1)
echo "handleRouteMap at: $line"
start=$((line-25)); end=$((line+120))
sed -n "${start},${end}p" "$FILE" | cat -n
echo "=== Function bodies / key helpers ==="
for pat in "buildRouteMap" "fetchOrgRepos" "originForHost" "escapeXml" "toSitemapXml"; do
l=$(rg -n "${pat}" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${l:-}" ]; then
echo "--- ${pat} at ${l} ---"
s=$((l-20)); e=$((l+180))
sed -n "${s},${e}p" "$FILE" | cat -n | head -n 260
fi
doneRepository: ubiquity/ubq.fi-router
Length of output: 1965
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/sitemap.ts"
[ -f "$FILE" ] || { echo "Missing $FILE"; exit 1; }
echo "=== handleRouteMap block ==="
line=$(rg -n "export async function handleRouteMap" "$FILE" | head -n1 | cut -d: -f1)
echo "handleRouteMap at: $line"
start=$((line-20)); [ $start -lt 1 ] && start=1
end=$((line+90))
sed -n "${start},${end}p" "$FILE" | cat -n
echo "=== buildRouteMap block ==="
line=$(rg -n "async function buildRouteMap" "$FILE" | head -n1 | cut -d: -f1)
echo "buildRouteMap at: $line"
start=$((line-20)); [ $start -lt 1 ] && start=1
end=$((line+140))
sed -n "${start},${end}p" "$FILE" | cat -n
echo "=== fetchOrgRepos block ==="
line=$(rg -n "async function fetchOrgRepos" "$FILE" | head -n1 | cut -d: -f1)
echo "fetchOrgRepos at: $line"
start=$((line-20)); [ $start -lt 1 ] && start=1
end=$((line+180))
sed -n "${start},${end}p" "$FILE" | cat -n
echo "=== originForHost + escapeXml blocks ==="
for pat in "function originForHost" "function escapeXml"; do
line=$(rg -n "$pat" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -z "${line:-}" ]; then echo "Not found: $pat"; continue; fi
echo "$pat at: $line"
start=$((line-10)); [ $start -lt 1 ] && start=1
end=$((line+120))
sed -n "${start},${end}p" "$FILE" | cat -n
doneRepository: ubiquity/ubq.fi-router
Length of output: 12674
Fail closed on GitHub API errors; dedupe plugin entries that normalize to the same host
fetchOrgRepos() stops pagination on !response.ok and returns partial/empty repos, and handleRouteMap() doesn’t handle failures—so /sitemap.xml and the JSON endpoints can silently return incomplete data with HTTP 200.
Proposed fix
export async function handleRouteMap(requestUrl: URL): Promise<Response | null> {
+ const build = async () => {
+ const entries = await buildRouteMap(requestUrl.origin)
+ return entries
+ }
+
if (requestUrl.pathname === '/sitemap.xml') {
- const entries = await buildRouteMap(requestUrl.origin)
- return new Response(toSitemapXml(entries), {
- headers: { ...CACHE_HEADERS, 'Content-Type': 'application/xml; charset=utf-8' },
- })
+ try {
+ const entries = await build()
+ return new Response(toSitemapXml(entries), {
+ headers: { ...CACHE_HEADERS, 'Content-Type': 'application/xml; charset=utf-8' },
+ })
+ } catch {
+ return new Response('Failed to build sitemap', { status: 502 })
+ }
}
if (requestUrl.pathname === '/routes.json' || requestUrl.pathname === '/sitemap.json') {
- const entries = await buildRouteMap(requestUrl.origin)
- return new Response(JSON.stringify({ generatedAt: new Date().toISOString(), entries }, null, 2), {
- headers: { ...CACHE_HEADERS, 'Content-Type': 'application/json; charset=utf-8' },
- })
+ try {
+ const entries = await build()
+ return new Response(JSON.stringify({ generatedAt: new Date().toISOString(), entries }, null, 2), {
+ headers: { ...CACHE_HEADERS, 'Content-Type': 'application/json; charset=utf-8' },
+ })
+ } catch {
+ return new Response(JSON.stringify({ error: 'Failed to build route map' }), {
+ status: 502,
+ headers: { 'Content-Type': 'application/json; charset=utf-8' },
+ })
+ }
} async function fetchOrgRepos(): Promise<GitHubRepo[]> {
const repos: GitHubRepo[] = []
for (let page = 1; page <= 10; page++) {
const response = await fetch(`${GITHUB_API_BASE}/orgs/${ORG}/repos?per_page=100&page=${page}`, {
@@
- if (!response.ok) break
+ if (!response.ok) {
+ throw new Error(`GitHub API failed: ${response.status}`)
+ }Minor: buildRouteMap() can emit duplicate plugin entries when both ubiquity-os-<x> and plugin-<x> exist, since both normalize to the same host (os-<x>.ubq.fi). Consider deduping by host before returning.
📝 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.
| export async function handleRouteMap(requestUrl: URL): Promise<Response | null> { | |
| if (requestUrl.pathname === '/sitemap.xml') { | |
| const entries = await buildRouteMap(requestUrl.origin) | |
| return new Response(toSitemapXml(entries), { | |
| headers: { ...CACHE_HEADERS, 'Content-Type': 'application/xml; charset=utf-8' }, | |
| }) | |
| } | |
| if (requestUrl.pathname === '/routes.json' || requestUrl.pathname === '/sitemap.json') { | |
| const entries = await buildRouteMap(requestUrl.origin) | |
| return new Response(JSON.stringify({ generatedAt: new Date().toISOString(), entries }, null, 2), { | |
| headers: { ...CACHE_HEADERS, 'Content-Type': 'application/json; charset=utf-8' }, | |
| }) | |
| export async function handleRouteMap(requestUrl: URL): Promise<Response | null> { | |
| const build = async () => { | |
| const entries = await buildRouteMap(requestUrl.origin) | |
| return entries | |
| } | |
| if (requestUrl.pathname === '/sitemap.xml') { | |
| try { | |
| const entries = await build() | |
| return new Response(toSitemapXml(entries), { | |
| headers: { ...CACHE_HEADERS, 'Content-Type': 'application/xml; charset=utf-8' }, | |
| }) | |
| } catch { | |
| return new Response('Failed to build sitemap', { status: 502 }) | |
| } | |
| } | |
| if (requestUrl.pathname === '/routes.json' || requestUrl.pathname === '/sitemap.json') { | |
| try { | |
| const entries = await build() | |
| return new Response(JSON.stringify({ generatedAt: new Date().toISOString(), entries }, null, 2), { | |
| headers: { ...CACHE_HEADERS, 'Content-Type': 'application/json; charset=utf-8' }, | |
| }) | |
| } catch { | |
| return new Response(JSON.stringify({ error: 'Failed to build route map' }), { | |
| status: 502, | |
| headers: { 'Content-Type': 'application/json; charset=utf-8' }, | |
| }) | |
| } | |
| } |
| .filter((repo) => repo.name.startsWith('ubiquity-os-') || repo.name.startsWith('plugin-')) | ||
| .map((repo) => { | ||
| const plugin = repo.name.replace(/^ubiquity-os-/, '').replace(/^plugin-/, '') | ||
| const host = `os-${plugin}.ubq.fi` | ||
| return { | ||
| type: 'plugin' as const, | ||
| name: repo.name, | ||
| host, | ||
| url: `${originForHost(origin, host)}/`, | ||
| upstream: `https://${plugin}-main.deno.dev/`, | ||
| } | ||
| }) | ||
|
|
||
| return [...apps, ...plugins].sort((a, b) => a.host.localeCompare(b.host)) |
There was a problem hiding this comment.
Deduplicate by host after app/plugin merge.
If both ubiquity-os-foo and plugin-foo exist, this produces duplicate os-foo.ubq.fi entries in sitemap/JSON.
Proposed fix
- return [...apps, ...plugins].sort((a, b) => a.host.localeCompare(b.host))
+ const merged = [...apps, ...plugins]
+ const deduped = Array.from(new Map(merged.map((entry) => [entry.host, entry])).values())
+ return deduped.sort((a, b) => a.host.localeCompare(b.host))📝 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.
| .filter((repo) => repo.name.startsWith('ubiquity-os-') || repo.name.startsWith('plugin-')) | |
| .map((repo) => { | |
| const plugin = repo.name.replace(/^ubiquity-os-/, '').replace(/^plugin-/, '') | |
| const host = `os-${plugin}.ubq.fi` | |
| return { | |
| type: 'plugin' as const, | |
| name: repo.name, | |
| host, | |
| url: `${originForHost(origin, host)}/`, | |
| upstream: `https://${plugin}-main.deno.dev/`, | |
| } | |
| }) | |
| return [...apps, ...plugins].sort((a, b) => a.host.localeCompare(b.host)) | |
| .filter((repo) => repo.name.startsWith('ubiquity-os-') || repo.name.startsWith('plugin-')) | |
| .map((repo) => { | |
| const plugin = repo.name.replace(/^ubiquity-os-/, '').replace(/^plugin-/, '') | |
| const host = `os-${plugin}.ubq.fi` | |
| return { | |
| type: 'plugin' as const, | |
| name: repo.name, | |
| host, | |
| url: `${originForHost(origin, host)}/`, | |
| upstream: `https://${plugin}-main.deno.dev/`, | |
| } | |
| }) | |
| const merged = [...apps, ...plugins] | |
| const deduped = Array.from(new Map(merged.map((entry) => [entry.host, entry])).values()) | |
| return deduped.sort((a, b) => a.host.localeCompare(b.host)) |
1 participant
Summary
/sitemap.xmlfor public app/plugin URLs./routes.jsonand/sitemap.jsonfor machine-readable infrastructure maps.ubiquityGitHub repos:*.ubq.firepos become app routes.ubiquity-os-*andplugin-*repos become plugin routes.Verification
npm run type-checknpx bun testCloses #2