Badge caching, HTTP semantics, and dev server fixes#2
Merged
Conversation
BadgeVersion: override prose's img margin with a scoped class and use inline-flex so badges sit inline within table cells. BadgeVersionGo: add optional dir prop for subpackages that share a parent go.mod — badge fetches the parent module version while the link points to the specific package on pkg.go.dev. Signed-off-by: Alejandro Mery <amery@apptly.co>
The MDC module pushes remark/rehype dependencies into Vite's optimizeDeps.include after config merges, but they can't be resolved (server-only). The static `include: []` didn't help. Use a vite:extendConfig hook to filter out entries containing "@nuxtjs/mdc >" after all modules have registered theirs. Signed-off-by: Alejandro Mery <amery@apptly.co>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
awesome-apptly | b3f8941 | Commit Preview URL Branch Preview URL |
Apr 02 2026, 01:29 AM |
Review Summary by QodoBadge caching, HTTP semantics, and dev server fixes
WalkthroughsDescription• Add throttled version caching with HTTP semantics (ETag, Last-Modified, split Cache-Control) • Implement DELETE method to bust in-memory cache and purge CF edge cache • Fix badge vertical alignment in prose tables with scoped CSS • Add dir prop to BadgeVersionGo for subpackages sharing parent go.mod • Strip unresolvable @nuxtjs/mdc optimizeDeps entries via vite:extendConfig hook • Prepend nuxt cleanup to dev script to prevent stale client-side SQL dumps • Add consola for structured logging in badge handlers Diagramflowchart LR
A["Badge Requests<br/>GET/HEAD/DELETE"] -->|"assertBadgeMethod"| B["Method Guard<br/>405 + Allow header"]
A -->|"fetchVersion"| C["Version Cache<br/>1hr throttle"]
C -->|"Poll Upstream"| D["Go Proxy/<br/>npm Registry"]
A -->|"DELETE"| E["Cache Bust<br/>+ CF Purge"]
C -->|"renderBadge"| F["HTTP Headers<br/>ETag, Last-Modified<br/>Cache-Control"]
F -->|"SVG Response"| G["Badge Image"]
H["Dev Server"] -->|"nuxt cleanup"| I["Remove Stale<br/>SQL Dumps"]
J["Vite Config"] -->|"extendConfig hook"| K["Filter MDC<br/>optimizeDeps"]
File Changes1. app/components/badge-version-go.vue
|
Code Review by Qodo
1. BadgeHandler allows HEAD
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
The client-side SQLite dump from a previous build can become stale, causing SPA 404s while SSR works fine. Prepending nuxt cleanup to the dev script removes the old output on every restart. Signed-off-by: Alejandro Mery <amery@apptly.co>
Contributor
Author
Branch update (force-push)Split the monolithic badge commit into two logical commits and
The earlier commits (badge alignment + dir prop, optimizeDeps fix, |
|
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:
📝 WalkthroughWalkthroughCentralized badge logic into a BadgeHandler that validates requests, renders SVGs, manages ETag/Last-Modified and split caching, and supports GET/HEAD/DELETE. Added a Cloudflare KV-backed version cache with per-key TTLs and in-flight request deduplication; endpoints and components updated accordingly. Changes
sequenceDiagram
participant Client
participant Endpoint as Badge Endpoint
participant Handler as BadgeHandler
participant Cache as Version Cache
participant KV as Cloudflare KV
participant Registry as Upstream Registry
Client->>Endpoint: GET /api/badge/{type}/{pkg}
Endpoint->>Handler: invoke eventHandler (pattern + fetch callback)
Handler->>Handler: validate package
alt invalid
Handler-->>Client: 400 Bad Request
else valid
Handler->>Cache: fetchVersion(cacheKey, fetcher)
Cache->>KV: get(cacheKey)
alt cached
KV-->>Cache: {version, lastModified}
Cache-->>Handler: {version, lastModified}
else miss or KV error
Cache->>Cache: check in-flight map
alt join in-flight
Cache-->>Handler: await existing promise
else fetch upstream
Cache->>Registry: fetcher(pkg)
Registry-->>Cache: {version?, time?}
Cache->>KV: put(cacheKey, {version?, lastModified}, TTL)
Cache-->>Handler: {version?, lastModified}
end
end
Handler->>Handler: render SVG (or unknown)
Handler-->>Client: 200 + SVG + Cache-Control, ETag, Last-Modified?
end
alt DELETE
Client->>Endpoint: DELETE /api/badge/...
Endpoint->>Handler: eventHandler handles DELETE
Handler->>Cache: deleteCachedVersion(cacheKey)
Cache->>KV: delete(cacheKey)
Handler-->>Client: 204 No Content
else other method
Client->>Endpoint: POST/PUT ...
Handler-->>Client: 405 Method Not Allowed (Allow: GET, HEAD, DELETE)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Author
Branch updateCreated KV namespaces ( |
![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 (1)
server/utils/badge.ts (1)
130-138: Consider usingthis.loggerfor consistency.Line 136 uses the module-level
logger(taggedbadge) instead ofthis.logger(tagged per-handler, e.g.,badge:go). This makes it harder to correlate edge cache purge failures with specific badge endpoints in logs.♻️ Suggested fix
private async purgeBadgeCache(event: H3Event): Promise<void> { try { if (typeof caches !== 'undefined' && caches.default) { await caches.default.delete(getRequestURL(event).href); } } catch (error) { - logger.warn('edge cache purge failed:', error); + this.logger.warn('edge cache purge failed:', error); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/badge.ts` around lines 130 - 138, The purgeBadgeCache method uses the module-level logger variable (logger) when catching errors; change it to use the instance-level this.logger so logs are tagged per-handler (e.g., badge:go). In the purgeBadgeCache function, replace usages of logger.warn(...) with this.logger.warn(...) and keep the same error message and context (including getRequestURL(event).href and the caught error) so the edge cache purge failure is correlated to the specific badge handler; ensure the class has this.logger initialized where other handlers do.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/api/badge/go/`[...package].ts:
- Around line 12-19: The fetch function builds proxyUrl using modulePath which
must have each uppercase letter encoded as "!" + lowercase (e.g., "A" -> "!a")
before calling proxy.golang.org; update the code in async fetch(modulePath,
logger) to transform modulePath by replacing every A–Z with "!" followed by its
lowercase counterpart (preserving other characters) prior to constructing
proxyUrl so requests for modules like github.com/Azure/azure-sdk-for-go do not
404; keep the rest of the logic (the $fetch call, logger.warn, and returning {
version: info?.Version, lastModified: parseDate(info?.Time) }) unchanged.
In `@server/utils/version-cache.ts`:
- Around line 70-86: The IIFE assigned to pending currently lets exceptions from
fetcher() bubble up and reject the shared promise; wrap the fetcher() call in a
try/catch inside that IIFE so any thrown error is caught, log the error (use
logger.warn with the key), treat the fetch as a failure by setting
result/version to undefined, set ttl to TTL_ERROR and firstSeen to now (or use
result?.time fallback), attempt the same storage.setItem write with the failure
values, and ensure the IIFE always resolves (never throws) so concurrent callers
awaiting pending get a graceful degraded response instead of a rejection.
---
Nitpick comments:
In `@server/utils/badge.ts`:
- Around line 130-138: The purgeBadgeCache method uses the module-level logger
variable (logger) when catching errors; change it to use the instance-level
this.logger so logs are tagged per-handler (e.g., badge:go). In the
purgeBadgeCache function, replace usages of logger.warn(...) with
this.logger.warn(...) and keep the same error message and context (including
getRequestURL(event).href and the caught error) so the edge cache purge failure
is correlated to the specific badge handler; ensure the class has this.logger
initialized where other handlers do.
🪄 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: 8224b145-7352-4ba3-af69-7ea790e0e76e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
AGENTS.mdREADME.mdapp/components/badge-version-go.vueapp/components/badge-version.vuenuxt.config.tspackage.jsonserver/api/badge/go/[...package].tsserver/api/badge/npm/[...package].tsserver/utils/badge.tsserver/utils/version-cache.tswrangler.toml
Contributor
Author
Branch update (force-push)Three bug fixes squashed into their respective commits
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/api/badge/go/[...package].ts (1)
6-8: Consider usingreplaceAll()for clarity.While
replace()with a global regex (/g) works correctly,replaceAll()is more explicit about the intent to replace all occurrences.♻️ Suggested change
// Go proxy requires uppercase letters encoded as !lowercase const escapeModulePath = (path: string) => - path.replace(/[A-Z]/g, (c) => `!${c.toLowerCase()}`); + path.replaceAll(/[A-Z]/g, (c) => `!${c.toLowerCase()}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/badge/go/`[...package].ts around lines 6 - 8, The helper escapeModulePath currently uses path.replace(/[A-Z]/g, ...) to transform uppercase letters; update it to call replaceAll with the same regex and replacer so the intent to replace all matches is explicit (i.e., in escapeModulePath use path.replaceAll(/[A-Z]/g, c => `!${c.toLowerCase()}`) while keeping the same replacement behavior).server/utils/badge.ts (1)
104-126: The practical risk is low, but consider hashing the version for defensive ETag handling.npm and Go registries enforce semantic versioning formats that don't allow quote characters, making this scenario extremely unlikely. However, if you want defensive programming around header generation, hashing the version is safer than removing quotes, since it guarantees a valid ETag regardless of version content.
♻️ Safer ETag generation with hashing
+import { createHash } from 'crypto'; + private renderBadge(event: H3Event, { version, lastModified }: BadgeResult): string { if (version) { setHeader(event, 'cache-control', `public, max-age=${CACHE_MAX_AGE}, s-maxage=${CACHE_S_MAX_AGE}`); - setHeader(event, 'etag', `W/"${version}"`); + const versionHash = createHash('sha256').update(version).digest('hex').slice(0, 16); + setHeader(event, 'etag', `W/"${versionHash}"`);This avoids potential edge cases and follows HTTP caching best practices.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/badge.ts` around lines 104 - 126, The ETag currently uses the raw version string in renderBadge (private renderBadge(event: H3Event, { version, lastModified }: BadgeResult)), which could contain unexpected characters; instead compute a stable hash (e.g., SHA-256 or other digest) of version and use that hashed value in the setHeader call for 'etag' (wrap as W/"<hash>") so the header always contains a safe token; leave last-modified and cache-control logic unchanged and use the same hashed value only when version is present, otherwise keep the existing error cache header path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/api/badge/go/`[...package].ts:
- Around line 6-8: The helper escapeModulePath currently uses
path.replace(/[A-Z]/g, ...) to transform uppercase letters; update it to call
replaceAll with the same regex and replacer so the intent to replace all matches
is explicit (i.e., in escapeModulePath use path.replaceAll(/[A-Z]/g, c =>
`!${c.toLowerCase()}`) while keeping the same replacement behavior).
In `@server/utils/badge.ts`:
- Around line 104-126: The ETag currently uses the raw version string in
renderBadge (private renderBadge(event: H3Event, { version, lastModified }:
BadgeResult)), which could contain unexpected characters; instead compute a
stable hash (e.g., SHA-256 or other digest) of version and use that hashed value
in the setHeader call for 'etag' (wrap as W/"<hash>") so the header always
contains a safe token; leave last-modified and cache-control logic unchanged and
use the same hashed value only when version is present, otherwise keep the
existing error cache header path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b67d4bb5-f812-4dc6-8b70-51c1d59a4fd3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
AGENTS.mdREADME.mdnuxt.config.tspackage.jsonserver/api/badge/go/[...package].tsserver/api/badge/npm/[...package].tsserver/utils/badge.tsserver/utils/version-cache.tswrangler.toml
✅ Files skipped from review due to trivial changes (1)
- wrangler.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- README.md
- package.json
- server/api/badge/npm/[...package].ts
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 91-95: Update the AGENTS.md text describing response headers so
the Last-Modified behaviour matches implementation: change the sentences that
say Last-Modified is sent only when the upstream provides a timestamp to state
that handlers will use the upstream timestamp if present, otherwise they fall
back to the locally tracked firstSeen time; apply the same clarification to the
other paragraph that documents Last-Modified (the second occurrence of this
description) so both sections accurately reflect the cached-response contract.
In `@server/utils/badge.ts`:
- Around line 104-117: The renderBadge method sets validator headers but never
invokes h3's conditional-check helper; import handleCacheHeaders and call it
(await handleCacheHeaders(event, { maxAge: CACHE_MAX_AGE })) inside renderBadge
after you set the ETag/Last-Modified/cache-control headers (and before returning
the badge) so h3 can send 304 responses; ensure the import for
handleCacheHeaders is added at the top of the file and that the call uses the
appropriate maxAge when version exists and the error max-age constant
(CACHE_MAX_AGE_ERROR) when not.
🪄 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: af372fe6-28d1-4811-bc60-8459e5a6e300
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
AGENTS.mdREADME.mdapp/components/badge-version-go.vueapp/components/badge-version.vuenuxt.config.tspackage.jsonserver/api/badge/go/[...package].tsserver/api/badge/npm/[...package].tsserver/utils/badge.tsserver/utils/version-cache.tswrangler.toml
Replace loose utility functions (renderBadge, renderBadgeError, assertBadgeMethod) with a BadgeHandler class that encapsulates method dispatch, input validation, cache headers, SVG rendering, and edge cache purge. Each endpoint now provides only its identity and fetch callback. HTTP improvements: split Cache-Control (max-age=3600 for browsers, s-maxage=300 for the CF edge), weak ETag from the version string, Last-Modified when the upstream provides a timestamp, DELETE for edge cache purging (current PoP), and 405 for unsupported methods. The npm handler switches to $fetch.raw to extract Last-Modified from registry response headers. Both handlers use a shared parseDate helper to guard against invalid upstream timestamps. Logging moves from console.warn to consola tagged instances. Signed-off-by: Alejandro Mery <amery@apptly.co>
Cache upstream version lookups in Cloudflare KV (VERSIONS_KV)
via Nitro's useStorage('versions'), with per-entry TTL: 1 hour
for successful lookups, 60 seconds for errors. Failed entries
auto-expire, preventing unbounded growth from bogus package
names. Local dev uses a memory driver.
The BadgeHandler derives cache keys from idSuffix (go:, npm:)
and wraps each endpoint's fetch callback with fetchVersion for
transparent KV caching. DELETE calls deleteCachedVersion before
the edge cache purge, which is now fault-tolerant (try-catch).
Concurrent requests at the TTL boundary are coalesced via an
in-flight promise map — only the originating caller writes to
KV. All KV operations degrade gracefully: failures are logged
but the badge is still returned.
Signed-off-by: Alejandro Mery <amery@apptly.co>
Contributor
Author
Branch update (force-push)Changes since last push (
/review |
|
Triggering a full review now. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/utils/version-cache.ts (1)
82-83: Consider storingversioneven when it's an empty string.The condition
if (version) entry.version = versiontreats empty string as falsy, so an empty version would not be stored. While empty strings are unlikely valid versions, if the upstream ever returned one, the cache would treat it as a failed lookup (no version stored, 60s TTL) rather than a successful one.This is likely fine in practice since npm and Go registries don't return empty version strings, but worth being aware of.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/version-cache.ts` around lines 82 - 83, The code currently skips setting entry.version when version is an empty string because it uses a truthy check; update the logic around StoredVersion creation so entry.version is set when version is present but possibly empty (e.g., check for version !== undefined/null or assign unconditionally) — modify the block that builds entry (the variable entry and the version assignment) to store empty-string versions as valid cache values so an empty upstream version counts as a successful lookup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/utils/version-cache.ts`:
- Around line 82-83: The code currently skips setting entry.version when version
is an empty string because it uses a truthy check; update the logic around
StoredVersion creation so entry.version is set when version is present but
possibly empty (e.g., check for version !== undefined/null or assign
unconditionally) — modify the block that builds entry (the variable entry and
the version assignment) to store empty-string versions as valid cache values so
an empty upstream version counts as a successful lookup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f56ced0d-9345-4b0c-abc9-8ce8096fcea3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
AGENTS.mdREADME.mdnuxt.config.tspackage.jsonserver/api/badge/go/[...package].tsserver/api/badge/npm/[...package].tsserver/utils/badge.tsserver/utils/version-cache.tswrangler.toml
✅ Files skipped from review due to trivial changes (1)
- wrangler.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- nuxt.config.ts
- server/api/badge/npm/[...package].ts
|
Persistent review updated to latest commit b3f8941 |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
AGENTS.md (1)
91-95:⚠️ Potential issue | 🟡 MinorDocument the
Last-Modifiedfallback accurately.
server/utils/version-cache.ts, Lines 78-88, storesresult?.lastModified ?? new Date(), so known-version badges can still emitLast-Modifiedwhen the upstream omits one. These docs still describe the header as upstream-only.📝 Suggested wording
-Successful responses carry split `Cache-Control` (`max-age=3600` -for browsers, `s-maxage=300` for the CF edge), a weak `ETag` -from the version string (conditional requests return `304`), -and `Last-Modified` from the upstream timestamp. Unknown packages render a +Successful responses carry split `Cache-Control` (`max-age=3600` +for browsers, `s-maxage=300` for the CF edge), a weak `ETag` +from the version string (conditional requests return `304`), +and `Last-Modified` from the upstream timestamp when available, +falling back to the cached fetch time otherwise. Unknown packages render a🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 91 - 95, Docs claim Last-Modified comes only from upstream, but the code in version-cache (the result?.lastModified ?? new Date() assignment in server/utils/version-cache.ts) falls back to the current date when upstream omits it, so update the documentation to state that known-version badges will emit a Last-Modified header using upstream's timestamp when present and otherwise a fallback (current time) stored by the version-cache. Mention the fallback behavior and its practical effect on conditional requests and caching TTLs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/badge-version-go.vue`:
- Around line 9-11: The tooltip/title currently always calls `${pkg.value}` a
"module" even when this component is rendering a subpackage (when `props.dir` is
set); change the computed properties (`title` and `alt`, and adjust `href` if
needed) to conditionally reflect a package vs module: detect `props.dir` (or
`dir`) and use "Go package <pkg.value>" when dir is present, otherwise "Go
module <pkg.value>"; keep the badge source on `props.mod` as-is. Update the
`title` and `alt` computed functions to switch their text based on `props.dir`
so the tooltip is accurate for subpackages.
In `@server/utils/badge.ts`:
- Around line 111-120: The current conditional revalidation (returning
ifNoneMatch === etag) is non-compliant: update the logic that reads
getRequestHeader(event, 'if-none-match') and getRequestHeader(event,
'if-modified-since') to (1) handle the If-None-Match wildcard "*" which must
match any current representation, (2) parse comma-separated/quoted token lists
and perform a weak comparison against the response etag by normalizing both
sides (strip optional W/ prefix and surrounding quotes) when checking
membership, and (3) if If-None-Match is absent, fall back to If-Modified-Since
by parsing the header date and comparing it to lastModified (treat unparseable
dates as no-match; treat <= lastModified as not-modified). Keep setting of
'etag' and 'last-modified' headers unchanged and return true only when the
RFC-compliant matching rules determine the representation is not modified.
In `@server/utils/version-cache.ts`:
- Around line 55-60: The read path in storage.getItem for StoredVersion assumes
stored.lastModified exists; detect legacy entries written under the old
firstSeen schema and treat them as cache misses by returning null instead of
returning an object with lastModified undefined. Concretely, in the code that
handles the result of storage.getItem<StoredVersion>(key) (the stored variable),
if stored.firstSeen is present and stored.lastModified is missing (or falsy),
return null (a cache miss) so the caller (e.g., badge generation code) won’t
emit an invalid Last-Modified header; otherwise continue returning { version:
stored.version, lastModified: new Date(stored.lastModified) } as before. Ensure
you reference StoredVersion, stored.firstSeen, and stored.lastModified when
implementing the check.
---
Duplicate comments:
In `@AGENTS.md`:
- Around line 91-95: Docs claim Last-Modified comes only from upstream, but the
code in version-cache (the result?.lastModified ?? new Date() assignment in
server/utils/version-cache.ts) falls back to the current date when upstream
omits it, so update the documentation to state that known-version badges will
emit a Last-Modified header using upstream's timestamp when present and
otherwise a fallback (current time) stored by the version-cache. Mention the
fallback behavior and its practical effect on conditional requests and caching
TTLs.
🪄 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: 6f139fb1-3ab1-45b0-90e1-7899ea41c736
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
AGENTS.mdREADME.mdapp/components/badge-version-go.vueapp/components/badge-version.vuenuxt.config.tspackage.jsonserver/api/badge/go/[...package].tsserver/api/badge/npm/[...package].tsserver/utils/badge.tsserver/utils/version-cache.tswrangler.toml
Comment on lines
+9
to
+11
| const href = computed(() => `https://pkg.go.dev/${pkg.value}`); | ||
| const alt = computed(() => `Go version for ${pkg.value}`); | ||
| const title = computed(() => `Go module ${pkg.value}`); |
There was a problem hiding this comment.
Clarify the tooltip for subpackages.
Keeping the badge source on props.mod makes sense for shared-go.mod subpackages, but Line 11 still labels ${pkg.value} as a module. When dir is set, that string is a package path, so the tooltip becomes misleading.
Suggested tweak
-const title = computed(() => `Go module ${pkg.value}`);
+const title = computed(() =>
+ props.dir ? `Go package ${pkg.value}` : `Go module ${pkg.value}`,
+);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/badge-version-go.vue` around lines 9 - 11, The tooltip/title
currently always calls `${pkg.value}` a "module" even when this component is
rendering a subpackage (when `props.dir` is set); change the computed properties
(`title` and `alt`, and adjust `href` if needed) to conditionally reflect a
package vs module: detect `props.dir` (or `dir`) and use "Go package
<pkg.value>" when dir is present, otherwise "Go module <pkg.value>"; keep the
badge source on `props.mod` as-is. Update the `title` and `alt` computed
functions to switch their text based on `props.dir` so the tooltip is accurate
for subpackages.
Comment on lines
+111
to
+120
| const etag = `W/"${version}"`; | ||
| setHeader(event, 'cache-control', `public, max-age=${CACHE_MAX_AGE}, s-maxage=${CACHE_S_MAX_AGE}`); | ||
| setHeader(event, 'etag', etag); | ||
| if (lastModified) { | ||
| setHeader(event, 'last-modified', lastModified.toUTCString()); | ||
| } | ||
|
|
||
| // h3 doesn't check conditional headers automatically | ||
| const ifNoneMatch = getRequestHeader(event, 'if-none-match'); | ||
| return ifNoneMatch === etag; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "badge.ts" --path "*/server/*" 2>/dev/null | head -20Repository: apptly-oss/awesome-apptly
Length of output: 51
🏁 Script executed:
cat -n server/utils/badge.ts | sed -n '100,130p'Repository: apptly-oss/awesome-apptly
Length of output: 1374
🏁 Script executed:
cat -n server/utils/badge.ts | head -50Repository: apptly-oss/awesome-apptly
Length of output: 1898
🏁 Script executed:
cat -n server/utils/badge.ts | sed -n '1,150p'Repository: apptly-oss/awesome-apptly
Length of output: 5842
🏁 Script executed:
# Check the imports and h3 library usage
rg "getRequestHeader|setHeader|setResponseStatus" server/utils/badge.ts -A 2Repository: apptly-oss/awesome-apptly
Length of output: 1040
🏁 Script executed:
# Verify the RFC 7232 conditional request semantics
cat <<'EOF' | python3
# Verify the HTTP/1.1 conditional request rules from RFC 7232
# If-None-Match semantics:
# A recipient MUST use the weak comparison function when comparing
# entity-tags for If-None-Match (Section 2.3.2)
#
# The special value "*" matches any representation
# Current code issue examples:
current_code = 'return ifNoneMatch === etag;'
print("Current code: " + current_code)
print()
print("Issues with current code:")
print("1. Only exact string match - doesn't handle multi-value headers")
print(" Example: If-None-Match: \"tag1\", \"tag2\"")
print(" The code would check: '\"tag1\", \"tag2\"' === 'W/\"version\"' -> false")
print()
print("2. Doesn't use weak comparison - ignores W/ prefix")
print(" Example: If-None-Match: \"tag\" (no W/ prefix)")
print(" The code would check: '\"tag\"' === 'W/\"version\"' -> false")
print(" But weak comparison should match if content is the same")
print()
print("3. Doesn't handle special value '*'")
print(" Example: If-None-Match: * (means 'any representation')")
print(" The code would check: '*' === 'W/\"version\"' -> false")
print()
print("4. Ignores If-Modified-Since header completely")
print(" This is a fallback when If-None-Match is not present")
print(" Current code: always returns false if If-None-Match doesn't match exactly")
EOFRepository: apptly-oss/awesome-apptly
Length of output: 864
Conditional revalidation is incomplete and non-compliant with RFC 7232.
Line 120 uses raw string equality (ifNoneMatch === etag), which fails for:
- Weak etag variants:
If-None-Match: "tag"won't match theW/"tag"response etag (weak comparison required) - Multi-value headers:
If-None-Match: "tag1", "tag2"won't parse - Wildcard matching:
If-None-Match: *won't match any representation If-Modified-Sinceheader is never checked, even thoughlastModifiedis available and set as a response header
Per RFC 7232, weak comparison (ignoring W/ prefix) is mandatory for If-None-Match, and If-Modified-Since should be a fallback when If-None-Match is absent.
🔧 One way to complete the validator handling
- const ifNoneMatch = getRequestHeader(event, 'if-none-match');
- return ifNoneMatch === etag;
+ const ifNoneMatch = getRequestHeader(event, 'if-none-match');
+ if (ifNoneMatch) {
+ const normalized = etag.replace(/^W\//, '');
+ return ifNoneMatch.split(',').some((candidate) => {
+ const tag = candidate.trim();
+ return tag === '*' || tag.replace(/^W\//, '') === normalized;
+ });
+ }
+
+ if (!lastModified) return false;
+
+ const ifModifiedSince = getRequestHeader(event, 'if-modified-since');
+ if (!ifModifiedSince) return false;
+
+ const modifiedSince = new Date(ifModifiedSince);
+ if (Number.isNaN(modifiedSince.getTime())) return false;
+
+ return Math.floor(lastModified.getTime() / 1000)
+ <= Math.floor(modifiedSince.getTime() / 1000);📝 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 etag = `W/"${version}"`; | |
| setHeader(event, 'cache-control', `public, max-age=${CACHE_MAX_AGE}, s-maxage=${CACHE_S_MAX_AGE}`); | |
| setHeader(event, 'etag', etag); | |
| if (lastModified) { | |
| setHeader(event, 'last-modified', lastModified.toUTCString()); | |
| } | |
| // h3 doesn't check conditional headers automatically | |
| const ifNoneMatch = getRequestHeader(event, 'if-none-match'); | |
| return ifNoneMatch === etag; | |
| const etag = `W/"${version}"`; | |
| setHeader(event, 'cache-control', `public, max-age=${CACHE_MAX_AGE}, s-maxage=${CACHE_S_MAX_AGE}`); | |
| setHeader(event, 'etag', etag); | |
| if (lastModified) { | |
| setHeader(event, 'last-modified', lastModified.toUTCString()); | |
| } | |
| // h3 doesn't check conditional headers automatically | |
| const ifNoneMatch = getRequestHeader(event, 'if-none-match'); | |
| if (ifNoneMatch) { | |
| const normalized = etag.replace(/^W\//, ''); | |
| return ifNoneMatch.split(',').some((candidate) => { | |
| const tag = candidate.trim(); | |
| return tag === '*' || tag.replace(/^W\//, '') === normalized; | |
| }); | |
| } | |
| if (!lastModified) return false; | |
| const ifModifiedSince = getRequestHeader(event, 'if-modified-since'); | |
| if (!ifModifiedSince) return false; | |
| const modifiedSince = new Date(ifModifiedSince); | |
| if (Number.isNaN(modifiedSince.getTime())) return false; | |
| return Math.floor(lastModified.getTime() / 1000) | |
| <= Math.floor(modifiedSince.getTime() / 1000); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/utils/badge.ts` around lines 111 - 120, The current conditional
revalidation (returning ifNoneMatch === etag) is non-compliant: update the logic
that reads getRequestHeader(event, 'if-none-match') and getRequestHeader(event,
'if-modified-since') to (1) handle the If-None-Match wildcard "*" which must
match any current representation, (2) parse comma-separated/quoted token lists
and perform a weak comparison against the response etag by normalizing both
sides (strip optional W/ prefix and surrounding quotes) when checking
membership, and (3) if If-None-Match is absent, fall back to If-Modified-Since
by parsing the header date and comparing it to lastModified (treat unparseable
dates as no-match; treat <= lastModified as not-modified). Keep setting of
'etag' and 'last-modified' headers unchanged and return true only when the
RFC-compliant matching rules determine the representation is not modified.
Comment on lines
+55
to
+60
| const stored = await storage.getItem<StoredVersion>(key); | ||
| if (stored) { | ||
| return { | ||
| version: stored.version, | ||
| lastModified: new Date(stored.lastModified), | ||
| }; |
There was a problem hiding this comment.
Treat legacy KV timestamps as cache misses.
This read path assumes every record already has the renamed lastModified field. Entries written under the previous firstSeen schema will come back with stored.lastModified === undefined, and server/utils/badge.ts, Lines 114-115, will then emit an invalid Last-Modified value until those entries expire.
🛡️ Backward-compatible read path
- const stored = await storage.getItem<StoredVersion>(key);
+ const stored = await storage.getItem<StoredVersion & { firstSeen?: string }>(key);
if (stored) {
+ const rawLastModified = stored.lastModified ?? stored.firstSeen;
+ const lastModified = rawLastModified ? new Date(rawLastModified) : undefined;
+ if (!lastModified || Number.isNaN(lastModified.getTime())) {
+ logger.warn(`invalid cached timestamp for ${key}, refetching`);
+ } else {
return {
version: stored.version,
- lastModified: new Date(stored.lastModified),
+ lastModified,
};
+ }
}📝 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 stored = await storage.getItem<StoredVersion>(key); | |
| if (stored) { | |
| return { | |
| version: stored.version, | |
| lastModified: new Date(stored.lastModified), | |
| }; | |
| const stored = await storage.getItem<StoredVersion & { firstSeen?: string }>(key); | |
| if (stored) { | |
| const rawLastModified = stored.lastModified ?? stored.firstSeen; | |
| const lastModified = rawLastModified ? new Date(rawLastModified) : undefined; | |
| if (!lastModified || Number.isNaN(lastModified.getTime())) { | |
| logger.warn(`invalid cached timestamp for ${key}, refetching`); | |
| } else { | |
| return { | |
| version: stored.version, | |
| lastModified, | |
| }; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/utils/version-cache.ts` around lines 55 - 60, The read path in
storage.getItem for StoredVersion assumes stored.lastModified exists; detect
legacy entries written under the old firstSeen schema and treat them as cache
misses by returning null instead of returning an object with lastModified
undefined. Concretely, in the code that handles the result of
storage.getItem<StoredVersion>(key) (the stored variable), if stored.firstSeen
is present and stored.lastModified is missing (or falsy), return null (a cache
miss) so the caller (e.g., badge generation code) won’t emit an invalid
Last-Modified header; otherwise continue returning { version: stored.version,
lastModified: new Date(stored.lastModified) } as before. Ensure you reference
StoredVersion, stored.firstSeen, and stored.lastModified when implementing the
check.
Comment on lines
+62
to
+70
| switch (event.method) { | ||
| case 'GET': | ||
| case 'HEAD': | ||
| case 'DELETE': | ||
| break; | ||
| default: | ||
| setResponseStatus(event, 405); | ||
| setHeader(event, 'allow', 'GET, HEAD, DELETE'); | ||
| return ''; |
There was a problem hiding this comment.
1. badgehandler allows head 📘 Rule violation ≡ Correctness
The badge handler explicitly allows HEAD requests and includes HEAD in the Allow header, but the compliance checklist requires only GET and DELETE to be accepted (all others must return 405). This can cause non-compliant method behaviour across all badge endpoints using BadgeHandler.
Agent Prompt
## Issue description
`BadgeHandler` currently treats `HEAD` as an allowed method and returns `Allow: GET, HEAD, DELETE`, but the compliance requirement states only `GET` and `DELETE` may be accepted; all other methods must return `405` with an `Allow` header.
## Issue Context
This affects all badge endpoints that instantiate `BadgeHandler` (e.g. go/npm), since the method guard is centralised in `server/utils/badge.ts`.
## Fix Focus Areas
- server/utils/badge.ts[62-70]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
Why do you say HEAD isn't handled?
There was a problem hiding this comment.
I’m not saying it’s unhandled — it is handled/allowed right now (case 'HEAD' and Allow: GET, HEAD, DELETE). The point is the compliance checklist you referenced requires rejecting anything except GET/DELETE with a 405, so HEAD should fall into the default 405 path and be removed from the Allow header.
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
dirprop toBadgeVersionGofor subpackages sharing a parentgo.modBadgeHandlerclass that encapsulates methoddispatch, input validation, cache headers, SVG rendering, and cache management
— each endpoint now provides only its identity and fetch callback
ETag,Last-Modified, splitCache-Control(
max-age=3600browser,s-maxage=300CF edge),405method guard. The npmhandler uses
$fetch.rawto extractLast-Modifiedfrom registry headersDELETEon a badge URL busts the KV cache and purges the CF edge60 seconds error — failed entries auto-expire). In-flight deduplication
coalesces concurrent requests; all KV operations degrade gracefully
@nuxtjs/mdcoptimizeDepsentries viavite:extendConfighooknuxt cleanuptodevscript to prevent stale client-side SQL dumpsconsolafor structured logging in badge handlersTest plan
pnpm precommitpasses (generate, lint, typecheck)/projects/darvaza-slog(8 Go module badges in tables)curl -IshowsETag,Last-Modified,Cache-Controlwiths-maxage=300curl -X DELETEreturns 204 and next GET fetches freshcurl -X POSTreturns 405 withAllow: GET, HEAD, DELETEoptimizeDepswarnings from@nuxtjs/mdcin dev server outputSummary by CodeRabbit
New Features
UI / Style
Documentation
Chores