Release/0.1.0#5
Conversation
feature: support binary upload and publish flow trigger as independen…
feature: log publish steps with status icons and meaningful transitio…
📝 WalkthroughWalkthroughThis PR splits the GitHub action into independent ChangesUpload/Publish switch and publish orchestration
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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
🧹 Nitpick comments (1)
src/main.ts (1)
130-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider replacing
as stringassertions with an explicit guard.The control flow guarantees
appVersionIdis defined by this point (set on line 110 or line 124), but theas stringassertions bypass TypeScript's type checking and would silently passundefineddownstream if the flow is ever refactored.♻️ Proposed guard before publish operations
appVersionId = await getReleaseCandidateVersionId({ platform, publishProfileId }) } + if (!appVersionId) { + core.setFailed('No app version ID resolved for publishing.') + return + } + const publishId = await getPublishId({ platform, publishProfileId, - appVersionId: appVersionId as string + appVersionId }) await startPublish({ platform, publishProfileId, publishId }) console.log(`Publish flow started for profile '${publishProfile}'.`) const success = await pollPublishStatus({ platform, publishProfileId, - appVersionId: appVersionId as string + appVersionId })Also applies to: 138-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.ts` around lines 130 - 134, The publish flow in main.ts is using `appVersionId as string` in the `getPublishId` call and the later publish operation, which bypasses type safety. Replace these assertions with an explicit guard in the surrounding publish logic (near the `appVersionId` assignment paths and the `getPublishId` invocation) so the code narrows `appVersionId` before use and fails early if it is still missing. Keep the fix localized to the publish branch and ensure both affected call sites use the guarded value instead of a cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@action.yml`:
- Around line 52-54: The current runtime validation in src/main.ts treats
appPath as one shared extension list, which allows invalid iOS/Android file
combinations to pass. Update the upload/appPath validation logic to branch on
platform and enforce .ipa for iOS and .apk/.aab for Android, matching the
contract documented in action.yml. Keep the check in the existing appPath
validation path so the correct platform-specific error is raised before upload
proceeds.
In `@src/api/publishApi.ts`:
- Around line 294-301: The publish flow lookup in the `getPublishId` path
currently assumes `steps[0]` has the `publishId`, which fails when the first
step is metadata-only or missing it. Update the logic to scan the `steps` array
for the first step with a valid `publishId` and return that value instead of
only checking the first element, while keeping the existing error path if no
step contains one.
- Around line 368-376: Update the polling/result handling in publishApi’s status
check so that the publish flow treats terminal statuses 2, 3, and 201 as
immediate failures alongside the existing 1 case. In the logic around the status
variable, return false as soon as these terminal cancelled/timeout/stopped
states are seen, and make sure the polling loop exits instead of waiting for the
full timeout. Keep the change localized to the status handling branch in
publishApi so the terminal state mapping stays consistent with the local status
model.
- Around line 144-154: The getLatestAppVersionId helper is relying on
versions[0], but getAppVersions does not guarantee newest-first ordering, so it
can return the wrong upload. Update getLatestAppVersionId to identify the newest
app version using a stable field like updateDate or creation/modified timestamp,
or preferably thread through the version id returned by the upload flow, so the
selection is deterministic regardless of list order.
---
Nitpick comments:
In `@src/main.ts`:
- Around line 130-134: The publish flow in main.ts is using `appVersionId as
string` in the `getPublishId` call and the later publish operation, which
bypasses type safety. Replace these assertions with an explicit guard in the
surrounding publish logic (near the `appVersionId` assignment paths and the
`getPublishId` invocation) so the code narrows `appVersionId` before use and
fails early if it is still missing. Keep the fix localized to the publish branch
and ensure both affected call sites use the guarded value instead of a cast.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f76b224-c4be-42e1-a087-95bbd232de59
⛔ Files ignored due to path filters (2)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (5)
README.mdRELEASE_NOTE.mdaction.ymlsrc/api/publishApi.tssrc/main.ts
💤 Files with no reviewable changes (1)
- RELEASE_NOTE.md
| description: | ||
| 'Specify the path to your application file. For iOS, this can be a .ipa | ||
| file path. For Android, specify the .apk or .aab file | ||
| path' | ||
| required: true | ||
| "Path to the application file. Required when 'upload' is true. For iOS use | ||
| a .ipa file; for Android use a .apk or .aab file." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforce the platform-specific appPath contract at runtime.
Line 53 documents .ipa for iOS and .apk/.aab for Android, but src/main.ts currently validates against one shared extension list, so invalid platform/file combinations can pass local validation and fail later.
Proposed validation shape
- const validExtensions = ['.apk', '.aab', '.ipa']
+ const validExtensionsByPlatform: Record<string, string[]> = {
+ ios: ['.ipa'],
+ android: ['.apk', '.aab']
+ }
const fileExtension = appPath
.slice(appPath.lastIndexOf('.'))
.toLowerCase()
- if (!validExtensions.includes(fileExtension)) {
+ if (!validExtensionsByPlatform[platform].includes(fileExtension)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@action.yml` around lines 52 - 54, The current runtime validation in
src/main.ts treats appPath as one shared extension list, which allows invalid
iOS/Android file combinations to pass. Update the upload/appPath validation
logic to branch on platform and enforce .ipa for iOS and .apk/.aab for Android,
matching the contract documented in action.yml. Keep the check in the existing
appPath validation path so the correct platform-specific error is raised before
upload proceeds.
| // The most recently created app version (first item) — used after an upload to | ||
| // identify the version that was just created. | ||
| export async function getLatestAppVersionId(options: { | ||
| platform: string | ||
| publishProfileId: string | ||
| }): Promise<string> { | ||
| const versions = await getAppVersions(options) | ||
| if (!versions.length) { | ||
| throw new Error('No app versions found on the publish profile after upload.') | ||
| } | ||
| return versions[0].id |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect tests/docs/usages for an explicit newest-first contract or returned appVersionId.
rg -n -C3 'getLatestAppVersionId|app-versions|created|createdAt|appVersionId|taskId' .Repository: appcircleio/appcircle-publish-githubaction
Length of output: 50400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant API file and inspect the exact implementation.
ast-grep outline src/api/publishApi.ts --view expanded || true
sed -n '130,170p' src/api/publishApi.ts
# Look for any explicit contract about ordering or a returned version id.
rg -n -C2 'app-versions|newest-first|first item|appVersionId|version id|createdAt|created at|taskId|releaseCandidate' src test tests docs README.md .github || true
# Inspect the upload / publish flow around the suspect call site.
sed -n '90,150p' src/main.tsRepository: appcircleio/appcircle-publish-githubaction
Length of output: 11244
🌐 Web query:
Appcircle publish v2 app-versions ordering newest first upload response appVersionId commit taskId
💡 Result:
The Appcircle API, specifically via the /v2/profiles/{profileId}/app-versions endpoint, allows you to retrieve a list of app versions associated with a specific Enterprise App Store profile [1][2]. Regarding your specific requirements: Ordering and Metadata: The Appcircle API response for app versions typically provides a list of objects containing version details [1]. While the API documentation does not explicitly detail a "newest first" query parameter for the endpoint, you can identify the latest versions by parsing the returned data [1]. The response objects generally include fields such as id (appVersionId), version string (e.g., CFBundleShortVersionString), and updateDate, which you can use to sort or filter the list programmatically [1][3]. Commit and Task Information: The core /app-versions API endpoint primarily focuses on versioning and metadata relevant to the Enterprise App Store (such as publish status and version numbers) [1][3]. Detailed build-specific information, such as associated commit hashes or specific task IDs, is typically accessed through the Build History or Build Detail endpoints rather than the store-specific app-versions list [4][5]. To retrieve this information, you should: 1. Use the /v2/profiles/{profileId}/app-versions endpoint to list the available versions [1][2]. 2. Iterate through the results and compare version strings or update timestamps to identify the newest version [1]. 3. If specific commit or task information is required for a build, use the Build History API (e.g., /builds or similar endpoints found in the Appcircle API documentation) to map build IDs to those specific details [4]. For the most accurate and up-to-date schema definitions, consult the interactive Swagger/OpenAPI documentation provided by Appcircle directly within your organization's Appcircle portal [6].
Citations:
- 1: https://docs.appcircle.io/enterprise-app-store/in-app-updates
- 2: https://docs.composio.dev/tools/appcircle
- 3: https://javadoc.jenkins.io/plugin/appcircle-enterprise-store/io/jenkins/plugins/appcircle/enterprise/app/store/Models/AppVersions.html
- 4: https://docs.appcircle.io/appcircle-ai/appcircle-mcp-server
- 5: https://github.com/appcircleio/appcircle-mcp/blob/main/docs/tool%5Fcontract.md
- 6: https://api.appcircle.io/openapi/index.html?urls.primaryName=publish
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the upload/commit flow and the returned shapes.
sed -n '204,266p' src/api/publishApi.ts
# Search for any handling of upload/commit response fields that could already expose a version id.
rg -n -C2 'commitResponse|uploadResponse|taskId|appVersionId|fileId|fileName|releaseCandidate' src/api/publishApi.ts src/main.tsRepository: appcircleio/appcircle-publish-githubaction
Length of output: 14461
Don't rely on versions[0] here
The /app-versions list has no explicit newest-first contract, so this can pick an older upload and publish the wrong binary. Select the new version by a stable field such as updateDate/creation time, or use a version id returned from the upload flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/publishApi.ts` around lines 144 - 154, The getLatestAppVersionId
helper is relying on versions[0], but getAppVersions does not guarantee
newest-first ordering, so it can return the wrong upload. Update
getLatestAppVersionId to identify the newest app version using a stable field
like updateDate or creation/modified timestamp, or preferably thread through the
version id returned by the upload flow, so the selection is deterministic
regardless of list order.
| const steps = response.data?.steps ?? [] | ||
| const publishId = steps[0]?.publishId | ||
| if (!publishId) { | ||
| throw new Error( | ||
| 'No publish flow steps found for the app version. Configure a publish flow on the profile first.' | ||
| ) | ||
| } | ||
| return publishId |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Find the first available publishId instead of assuming it is on the first step.
If the first step is metadata-only or missing publishId, this throws even when later steps contain the run id.
Proposed fix
const steps = response.data?.steps ?? []
- const publishId = steps[0]?.publishId
+ const publishId = steps.find((step: any) => step?.publishId)?.publishId📝 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 steps = response.data?.steps ?? [] | |
| const publishId = steps[0]?.publishId | |
| if (!publishId) { | |
| throw new Error( | |
| 'No publish flow steps found for the app version. Configure a publish flow on the profile first.' | |
| ) | |
| } | |
| return publishId | |
| const steps = response.data?.steps ?? [] | |
| const publishId = steps.find((step: any) => step?.publishId)?.publishId | |
| if (!publishId) { | |
| throw new Error( | |
| 'No publish flow steps found for the app version. Configure a publish flow on the profile first.' | |
| ) | |
| } | |
| return publishId |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/publishApi.ts` around lines 294 - 301, The publish flow lookup in the
`getPublishId` path currently assumes `steps[0]` has the `publishId`, which
fails when the first step is metadata-only or missing it. Update the logic to
scan the `steps` array for the first step with a valid `publishId` and return
that value instead of only checking the first element, while keeping the
existing error path if no step contains one.
| const status = typeof data.status === 'number' ? data.status : 99 | ||
| if (status === 0) { | ||
| console.log('Publish completed successfully.') | ||
| return true | ||
| } | ||
| if (status === 1) { | ||
| console.log('Publish failed.') | ||
| return false | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Treat cancelled/timeout/stopped publish statuses as terminal failures.
The local status model marks 2, 3, and 201 as terminal, but the polling loop only exits for top-level 0 and 1. A cancelled or timed-out publish can wait until the 20-minute polling timeout and report the wrong failure reason.
Proposed fix
if (status === 0) {
console.log('Publish completed successfully.')
return true
}
- if (status === 1) {
- console.log('Publish failed.')
+ if ([1, 2, 3, 201].includes(status)) {
+ console.log(`Publish failed: ${stepStatusName(status)}.`)
return false
}📝 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 status = typeof data.status === 'number' ? data.status : 99 | |
| if (status === 0) { | |
| console.log('Publish completed successfully.') | |
| return true | |
| } | |
| if (status === 1) { | |
| console.log('Publish failed.') | |
| return false | |
| } | |
| const status = typeof data.status === 'number' ? data.status : 99 | |
| if (status === 0) { | |
| console.log('Publish completed successfully.') | |
| return true | |
| } | |
| if ([1, 2, 3, 201].includes(status)) { | |
| console.log(`Publish failed: ${stepStatusName(status)}.`) | |
| return false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/publishApi.ts` around lines 368 - 376, Update the polling/result
handling in publishApi’s status check so that the publish flow treats terminal
statuses 2, 3, and 201 as immediate failures alongside the existing 1 case. In
the logic around the status variable, return false as soon as these terminal
cancelled/timeout/stopped states are seen, and make sure the polling loop exits
instead of waiting for the full timeout. Keep the change localized to the status
handling branch in publishApi so the terminal state mapping stays consistent
with the local status model.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes