Conversation
Contributor
📝 WalkthroughWalkthroughReplaces inline shell steps in the GitHub Actions workflow that fetched GHCR tags with a new TypeScript script ( Changes
Sequence Diagram(s)sequenceDiagram
participant WF as GitHub Actions Workflow
participant Script as convert/fetchTags.ts
participant GHCR as GitHub Container Registry API
rect rgba(200,220,255,0.5)
WF->>Script: invoke `npx ts-node convert/fetchTags.ts`
end
rect rgba(200,255,220,0.5)
Script->>GHCR: request token (actions identity)
GHCR-->>Script: return token
Script->>GHCR: fetch tags for sillsdev/app-builders (Bearer token)
GHCR-->>Script: return tags list
Script->>Script: filter numeric tags, sort versions, fetch/latest digests
Script->>GHCR: request digest for candidate tags
GHCR-->>Script: return digest(s)
Script->>WF: output `appbuilder_version=<latest_tag>` (stdout -> GITHUB_OUTPUT)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@convert/fetchTags.ts`:
- Line 3: The regex verNumRegExp currently matches any string containing a
numeric version fragment; change it to anchor the pattern so it only matches
entire tags that are purely numeric versions (update verNumRegExp to use start
and end anchors, e.g., require the whole string to match digits and up to two
dot-separated groups) and ensure any code using verNumRegExp (the filter/match
logic in fetchTags) continues to test the whole tag against this anchored
pattern.
- Around line 5-22: The top-level async IIFE that fetches tokenJSON/tagJSON and
prints latest can reject silently; wrap the IIFE invocation in a .catch(...)
handler so any thrown error is logged (e.g., processLogger or console.error) and
the process exits non‑zero (process.exit(1)). Specifically update the immediate
call to the async function that contains tokenJSON, token, tagJSON, tags, latest
and console.log to append a .catch(block) that logs the error and calls
process.exit(1) to ensure CI fails on errors.
- Around line 16-21: The variable `latest` (computed from
`tags.filter(...).sort(compareVersions).at(-1)`) can be undefined when no tags
match `verNumRegExp`, so add an explicit guard immediately after computing
`latest` to fail the process instead of printing `appbuilder_version=undefined`;
for example, check `if (!latest) { console.error("No matching tags found for
verNumRegExp"); process.exit(1); }` (or throw an Error) before the existing
`console.log`, referencing `latest`, `tags`, `verNumRegExp`, and
`compareVersions` so the workflow step fails clearly when no version is found.
- Around line 6-12: The fetch calls in convert/fetchTags.ts don't check HTTP
responses and can produce undefined outputs; change both requests to await the
Response (e.g., respToken and respTags), verify resp.ok and throw a descriptive
Error including status and statusText when not ok, then call resp.json(); after
parsing, validate that tokenJSON.token exists before using it and that tagJSON
has the expected shape (e.g., tagJSON.tags array) and throw if not — update the
tokenJSON/token and tagJSON usages accordingly so failures fail-fast instead of
producing undefined values.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
convert/fetchTags.ts (2)
6-11:⚠️ Potential issue | 🟠 MajorAdd response-status checks and payload validation before using token/tags/digest.
Right now
fetch(...).json()is used directly; if GHCR returns a non-2xx response or unexpected JSON shape, the script may fail later with less context. Please fail explicitly on!res.okand validate required fields so CI fails fast with a clear error.Suggested fix
async function getDigest(token: string, ref: string) { - return ( - await fetch(`https://ghcr.io/v2/sillsdev/app-builders/manifests/${ref}`, { - headers: { Authorization: `Bearer ${token}` } - }).then((r) => r.json()) - ).config.digest as string; + const res = await fetch(`https://ghcr.io/v2/sillsdev/app-builders/manifests/${ref}`, { + headers: { Authorization: `Bearer ${token}` } + }); + if (!res.ok) { + throw new Error(`Manifest fetch failed for ${ref}: ${res.status} ${res.statusText}`); + } + const json = await res.json(); + if (!json?.config?.digest) { + throw new Error(`Missing config.digest for ${ref}`); + } + return json.config.digest as string; } (async () => { try { - const token = ( - await fetch('https://ghcr.io/token?scope=repository:sillsdev/app-builders:pull').then( - (r) => r.json() - ) - ).token as string; + const tokenRes = await fetch( + 'https://ghcr.io/token?scope=repository:sillsdev/app-builders:pull' + ); + if (!tokenRes.ok) { + throw new Error(`Token fetch failed: ${tokenRes.status} ${tokenRes.statusText}`); + } + const tokenJSON = await tokenRes.json(); + if (!tokenJSON?.token) { + throw new Error('Token response missing token'); + } + const token = tokenJSON.token as string; - const tags = ( - await fetch('https://ghcr.io/v2/sillsdev/app-builders/tags/list', { - headers: { Authorization: `Bearer ${token}` } - }).then((r) => r.json()) - ).tags as string[]; + const tagRes = await fetch('https://ghcr.io/v2/sillsdev/app-builders/tags/list', { + headers: { Authorization: `Bearer ${token}` } + }); + if (!tagRes.ok) { + throw new Error(`Tag fetch failed: ${tagRes.status} ${tagRes.statusText}`); + } + const tagJSON = await tagRes.json(); + if (!Array.isArray(tagJSON?.tags)) { + throw new Error('Tag response missing tags array'); + } + const tags = tagJSON.tags as string[];Based on learnings: For conversion modules (e.g., files under convert/), adopt a fail-fast approach: use non-null assertions and allow runtime errors when source XML data is invalid or missing attributes, rather than silently handling or recovering from such errors. This ensures data issues are surfaced and fixed upstream. Apply this guideline to all TypeScript conversion files under convert/.
Also applies to: 16-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convert/fetchTags.ts` around lines 6 - 11, getDigest currently calls fetch(...).json() directly which can hide HTTP errors and unexpected payloads; change it to await fetch(...) into a response variable (e.g., const res = await fetch(...)), check if (!res.ok) and throw a descriptive error including res.status and await res.text(), then parse const payload = await res.json(); validate that payload.config and payload.config.digest exist and are strings before returning payload.config.digest as string (throw a clear error if validation fails). Apply the same pattern to any other tag/token/digest lookups in this file (and related convert/* modules) so failures are explicit and fail-fast.
28-39:⚠️ Potential issue | 🟠 MajorFail if no version tag matches the
latestdigest.The current do/while always prints
sortedTags[i]even when no digest match exists (e.g., latest points to a non-version tag). That can silently emit the wrong version. Add an explicit guard for “no match” and “no version tags.”Suggested fix
const sortedTags = tags.filter((t) => t.match(verNumRegExp)).sort(compareVersions); + if (sortedTags.length === 0) { + throw new Error('No version tags found'); + } + const latestDigest = await getDigest(token, 'latest'); - let i = sortedTags.length; - let digest = ''; - do { - i--; - digest = await getDigest(token, sortedTags[i]); - } while (i > 0 && latestDigest !== digest); - - console.log(`appbuilder_version=${sortedTags[i]}`); + let matchedTag: string | undefined; + for (let i = sortedTags.length - 1; i >= 0; i--) { + const digest = await getDigest(token, sortedTags[i]); + if (latestDigest === digest) { + matchedTag = sortedTags[i]; + break; + } + } + + if (!matchedTag) { + throw new Error('No version tag matches latest digest'); + } + console.log(`appbuilder_version=${matchedTag}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convert/fetchTags.ts` around lines 28 - 39, The code prints sortedTags[i] even when no version tags exist or none match the latest digest; update the logic in the block that uses sortedTags, verNumRegExp, compareVersions and getDigest to explicitly handle these cases: first guard against an empty sortedTags and fail/exit with a clear error if none; then when iterating (do/while) track whether a matching digest was found (or after the loop compare latestDigest to digest) and if no match was found, fail/exit or emit an explicit error instead of printing sortedTags[i]; only console.log the matched tag when a valid match exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@convert/fetchTags.ts`:
- Around line 6-11: getDigest currently calls fetch(...).json() directly which
can hide HTTP errors and unexpected payloads; change it to await fetch(...) into
a response variable (e.g., const res = await fetch(...)), check if (!res.ok) and
throw a descriptive error including res.status and await res.text(), then parse
const payload = await res.json(); validate that payload.config and
payload.config.digest exist and are strings before returning
payload.config.digest as string (throw a clear error if validation fails). Apply
the same pattern to any other tag/token/digest lookups in this file (and related
convert/* modules) so failures are explicit and fail-fast.
- Around line 28-39: The code prints sortedTags[i] even when no version tags
exist or none match the latest digest; update the logic in the block that uses
sortedTags, verNumRegExp, compareVersions and getDigest to explicitly handle
these cases: first guard against an empty sortedTags and fail/exit with a clear
error if none; then when iterating (do/while) track whether a matching digest
was found (or after the loop compare latestDigest to digest) and if no match was
found, fail/exit or emit an explicit error instead of printing sortedTags[i];
only console.log the matched tag when a valid match exists.
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.
Uses a script written in TS to find the latest AppBuilders version number. The GHCR recently added the tag
stagingto the list of tags, which invalidated prior assumptions about getting the latest version number tag from GHCR.Summary by CodeRabbit