Conversation
📝 WalkthroughWalkthroughThis refactors the GitHub workflow step responsible for fetching the latest AppBuilder version from GHCR. Previously implemented using shell commands with curl and jq, the logic is now delegated to a dedicated TypeScript script that performs token retrieval, tag fetching, filtering, and version sorting. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ 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: 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.
| @@ -0,0 +1,22 @@ | |||
| import { compareVersions } from './stringUtils'; | |||
|
|
|||
| const verNumRegExp = /\d+(\.\d+){0,2}/; | |||
There was a problem hiding this comment.
Anchor the regex to avoid matching non-version tags.
/\d+(\.\d+){0,2}/ matches any tag containing a digit sequence (e.g., a hypothetical "rc1-staging" tag would pass the filter). Since the goal is to match tags that are purely numeric versions, anchor the regex.
Proposed fix
-const verNumRegExp = /\d+(\.\d+){0,2}/;
+const verNumRegExp = /^\d+(\.\d+){0,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.
| const verNumRegExp = /\d+(\.\d+){0,2}/; | |
| const verNumRegExp = /^\d+(\.\d+){0,2}$/; |
🤖 Prompt for AI Agents
In `@convert/fetchTags.ts` at 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.
| (async () => { | ||
| const tokenJSON = await fetch( | ||
| 'https://ghcr.io/token?scope=repository:sillsdev/app-builders:pull' | ||
| ).then((r) => r.json()); | ||
| const token = tokenJSON.token; | ||
| const tagJSON = await fetch('https://ghcr.io/v2/sillsdev/app-builders/tags/list', { | ||
| headers: { Authorization: `Bearer ${token}` } | ||
| }).then((r) => r.json()); | ||
|
|
||
| const tags = tagJSON.tags as string[]; | ||
|
|
||
| const latest = tags | ||
| .filter((t) => t.match(verNumRegExp)) | ||
| .sort(compareVersions) | ||
| .at(-1); | ||
|
|
||
| console.log(`appbuilder_version=${latest}`); | ||
| })(); |
There was a problem hiding this comment.
Unhandled async IIFE rejection may not fail the CI step.
The top-level async IIFE has no .catch(). If any await throws, the promise rejects silently. Depending on the Node.js version, this may or may not exit with a non-zero code (older Node versions only warn). Add a .catch() to ensure a non-zero exit.
Proposed fix
-(async () => {
+(async () => {
// ... body unchanged ...
-})();
+})().catch((e) => {
+ console.error(e);
+ process.exit(1);
+});🤖 Prompt for AI Agents
In `@convert/fetchTags.ts` around lines 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.
| const tokenJSON = await fetch( | ||
| 'https://ghcr.io/token?scope=repository:sillsdev/app-builders:pull' | ||
| ).then((r) => r.json()); | ||
| const token = tokenJSON.token; | ||
| const tagJSON = await fetch('https://ghcr.io/v2/sillsdev/app-builders/tags/list', { | ||
| headers: { Authorization: `Bearer ${token}` } | ||
| }).then((r) => r.json()); |
There was a problem hiding this comment.
No error handling on HTTP responses — failures will silently corrupt CI output.
If either fetch returns a non-2xx status (e.g., rate-limited, auth failure), .json() will either throw or return unexpected data. The script would then silently output appbuilder_version=undefined, and the workflow would continue with a bogus version string instead of failing the step. Per the fail-fast convention for convert/ files, check response.ok and throw on failure.
Proposed fix
- const tokenJSON = await fetch(
- 'https://ghcr.io/token?scope=repository:sillsdev/app-builders:pull'
- ).then((r) => r.json());
- const token = tokenJSON.token;
- const tagJSON = await fetch('https://ghcr.io/v2/sillsdev/app-builders/tags/list', {
- headers: { Authorization: `Bearer ${token}` }
- }).then((r) => r.json());
+ 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}`);
+ const tokenJSON = await tokenRes.json();
+ const token = tokenJSON.token;
+
+ 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}`);
+ const tagJSON = await tagRes.json();Based on learnings: for conversion modules under convert/, adopt a fail-fast approach and allow runtime errors when data is invalid, rather than silently handling or recovering.
📝 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.
| const tokenJSON = await fetch( | |
| 'https://ghcr.io/token?scope=repository:sillsdev/app-builders:pull' | |
| ).then((r) => r.json()); | |
| const token = tokenJSON.token; | |
| const tagJSON = await fetch('https://ghcr.io/v2/sillsdev/app-builders/tags/list', { | |
| headers: { Authorization: `Bearer ${token}` } | |
| }).then((r) => r.json()); | |
| 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}`); | |
| const tokenJSON = await tokenRes.json(); | |
| const token = tokenJSON.token; | |
| 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}`); | |
| const tagJSON = await tagRes.json(); |
🤖 Prompt for AI Agents
In `@convert/fetchTags.ts` around lines 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.
| const latest = tags | ||
| .filter((t) => t.match(verNumRegExp)) | ||
| .sort(compareVersions) | ||
| .at(-1); | ||
|
|
||
| console.log(`appbuilder_version=${latest}`); |
There was a problem hiding this comment.
latest can be undefined, producing appbuilder_version=undefined in CI.
If no tags match the filter (or the tags array is empty), .at(-1) returns undefined. The workflow step would silently set appbuilder_version=undefined and continue. Add a guard to fail explicitly.
Proposed fix
const latest = tags
.filter((t) => t.match(verNumRegExp))
.sort(compareVersions)
.at(-1);
+ if (!latest) {
+ throw new Error('No version tags found');
+ }
console.log(`appbuilder_version=${latest}`);📝 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.
| const latest = tags | |
| .filter((t) => t.match(verNumRegExp)) | |
| .sort(compareVersions) | |
| .at(-1); | |
| console.log(`appbuilder_version=${latest}`); | |
| const latest = tags | |
| .filter((t) => t.match(verNumRegExp)) | |
| .sort(compareVersions) | |
| .at(-1); | |
| if (!latest) { | |
| throw new Error('No version tags found'); | |
| } | |
| console.log(`appbuilder_version=${latest}`); |
🤖 Prompt for AI Agents
In `@convert/fetchTags.ts` around lines 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.
1 participant
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