feat(cli): build init --renew for iOS cert and Capgo-managed profiles#2281
Open
WcaleNieWolny wants to merge 2 commits into
Open
feat(cli): build init --renew for iOS cert and Capgo-managed profiles#2281WcaleNieWolny wants to merge 2 commits into
WcaleNieWolny wants to merge 2 commits into
Conversation
Adds `build init --renew`, a single-command renewal flow for iOS distribution certificates and Capgo-issued provisioning profiles. The flow inspects saved credentials, computes a plan (default threshold: anything expiring within 30 days), then re-issues only what's needed via the App Store Connect API. Renewal reuses the existing onboarding Ink UI by switching it into a new `mode: 'renew'` branch. When the cert is being renewed, all Capgo-named profiles in the saved provisioning map are re-issued to bind to the new cert; user-imported profiles (whose names don't match the `Capgo <appId> AppStore` convention) are flagged and skipped because we can't re-create them via Apple's API. If the saved `.p8` API key is rejected (401/403), the flow drops into the onboarding `.p8` input chain so the user can supply a fresh key without restarting. A second entry point is added to `build credentials manage`: a new "Renew expired credentials" action on the top-level menu that hands off to the same flow for the picked app. Flags: --renew, --force (renew everything), --days N (threshold, default 30), --dry-run (print the plan, no changes), --local (operate on the project-local credentials file), --appId (override capacitor.config detection). Design doc: docs/plans/2026-05-18-ios-credential-renewal-design.md
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an end-to-end iOS credential renewal feature: expiry extraction (P12 and mobileprovision), renewal-plan computation, execution helpers, onboarding command wiring, a renew-mode onboarding state machine and UI, TUI "Renew" action, tests, and a design doc. ChangesiOS Credential Renewal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6f9145a28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+978
to
+983
| const existingProfiles = await findCapgoProfiles(token, appId) | ||
| for (const existing of existingProfiles) { | ||
| if (cancelled) | ||
| return | ||
| try { | ||
| await deleteProfile(token, existing.id) |
There was a problem hiding this comment.
In the multi-bundle renew path, each iteration calls findCapgoProfiles(token, appId) and deletes every returned profile before creating the next one. Because Capgo profiles are named from appId (not bundle ID), the profile created in an earlier iteration is matched and deleted in the next iteration, so only the last bundle's profile remains valid. This can leave extension targets with saved profile content that has already been revoked.
Useful? React with 👍 / 👎.
Comment on lines
94
to
98
| const extConfig = await getConfig() | ||
| appId = getAppId(undefined, extConfig?.config) | ||
| appId = getAppId(options.appId, extConfig?.config) | ||
| iosDir = getPlatformDirFromCapacitorConfig(extConfig?.config, 'ios') | ||
| androidDir = getPlatformDirFromCapacitorConfig(extConfig?.config, 'android') | ||
| } |
There was a problem hiding this comment.
The new --appId override is assigned inside the getConfig() try-block, so if config loading throws (the exact case this flag is meant to support outside project root), appId stays undefined and the command exits with "Could not detect app ID". This makes the documented override path unusable unless Capacitor config can already be loaded.
Useful? React with 👍 / 👎.
Comment on lines
+966
to
+967
| const completedSoFar = renewCompletedProfilesRef.current | ||
| const remaining = targets.filter(bid => !completedSoFar.some(c => c.bundleId === bid)) |
There was a problem hiding this comment.
The code persists completedSteps.renewedProfiles, but resume logic ignores it and computes remaining work from renewCompletedProfilesRef.current only. After restarting the CLI, that in-memory array is empty, so already-renewed bundle IDs are reprocessed instead of resuming at the next unfinished target, causing unnecessary API churn and defeating the advertised per-profile resume behavior.
Useful? React with 👍 / 👎.
![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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cli/src/build/credentials-manage.ts (1)
643-655:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHide renew help text when renew is not selectable.
Line 653 always advertises Renew, but Line 663 conditionally removes the action when
hasIosis false. That creates a misleading menu.Suggested fix
async function pickAction(entry: AppEntry, canGoBack: boolean, extraIntro?: string[]): Promise<string | symbol> { + const hasIos = entry.platforms.includes('ios') const platformsLine = entry.platforms.length === 0 ? 'no platforms configured' : entry.platforms.map(p => `${p === 'ios' ? 'iOS' : 'Android'}: ${summarizePlatformContent(entry.saved[p])}`).join(' · ') setManagerScreen({ @@ 'View — flat list of every credential across platforms (show, decode, copy, edit, explain, remove).', 'Add… — add a new platform via onboarding, or add a configuration option.', - 'Renew — re-issue an expiring iOS cert and Capgo-managed provisioning profiles.', + ...(hasIos ? ['Renew — re-issue an expiring iOS cert and Capgo-managed provisioning profiles.'] : []), 'Export — write a .env file ready for CI/CD secrets (asks which platform if both are configured).', 'Delete — wipe all credentials for one platform (asks which if both are configured).', ], @@ - const hasIos = entry.platforms.includes('ios') const options = [Also applies to: 659-665
🤖 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 `@cli/src/build/credentials-manage.ts` around lines 643 - 655, The introLines passed into setManagerScreen always includes a static "Renew — re-issue an expiring iOS cert..." entry even when iOS renewals aren't available, causing a misleading menu; update the introLines construction in the setManagerScreen call to conditionally include that "Renew" help line only when hasIos is true (i.e., push or spread the Renew string into introLines when hasIos is truthy) and ensure the same conditional logic is applied to the other similar help text block referenced around the same area so the displayed intro matches available actions.cli/src/build/onboarding/command.ts (1)
93-105:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHonor
--appIdeven whengetConfig()fails.Right now an explicit
options.appIdis only applied inside thegetConfig()try. Outside a Capacitor project,getConfig()throws,appIdstaysundefined, andbuild init --renew --appId ...exits even though the override should be enough.Suggested fix
- let appId: string | undefined + let appId: string | undefined = options.appId let iosDir = 'ios' let androidDir = 'android' try { const extConfig = await getConfig() - appId = getAppId(options.appId, extConfig?.config) + appId = getAppId(appId, extConfig?.config) iosDir = getPlatformDirFromCapacitorConfig(extConfig?.config, 'ios') androidDir = getPlatformDirFromCapacitorConfig(extConfig?.config, 'android') } catch { // getConfig may throw if not in a Capacitor project }🤖 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 `@cli/src/build/onboarding/command.ts` around lines 93 - 105, The code currently sets appId only inside the try block so an explicit options.appId is ignored if getConfig() throws; move the appId resolution out of the try so the CLI honors --appId even when not in a Capacitor project: call getConfig() in the try to populate extConfig, but after the try assign appId = getAppId(options.appId, extConfig?.config) (or appId = options.appId ?? getAppId(undefined, extConfig?.config)) and only call getPlatformDirFromCapacitorConfig to set iosDir/androidDir if extConfig?.config is present; ensure the existing error/exit check uses the resolved appId variable.
🤖 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 `@cli/src/build/onboarding/progress.ts`:
- Around line 96-116: When resuming renew flows, don't short-circuit the user
confirmation or mis-handle cert-less renewals: if completedSteps.renewPlan is
missing, return the 'renew-plan' step (not skip to analyzing), and when
certificateCreated is false consult the stored plan (completedSteps.renewPlan or
its plan.cert.needsRenewal flag) to choose between 'renew-revoking-cert' (when
cert.needsRenewal === true) and 'renew-creating-profiles' (when
cert.needsRenewal === false); update the logic in the progress.mode === 'renew'
branch (referencing progress, completedSteps.renewPlan, and certificateCreated)
to implement these checks and return the correct step.
In `@cli/src/build/onboarding/renew-detection.ts`:
- Around line 21-27: The JSON parsing currently casts blindly in
parseProvisioningMap; instead validate each entry's shape before casting and
only include map entries where the value is a non-null object with the expected
keys (e.g., check typeof value === 'object' && value !== null && typeof
value.name === 'string' etc.) so malformed entries like { "com.app": null } are
skipped; update parseProvisioningMap to build and return a filtered
Record<string, ProvisioningMapEntry> and also add a defensive guard where the
map is consumed (the renew-detection code that accesses entry.name) to check
entry is defined and entry.name is a string before dereferencing.
In `@cli/src/build/onboarding/ui/app.tsx`:
- Around line 196-197: The persisted resume data only records bundle IDs
(completedSteps.renewedProfiles) while runtime state uses
renewCompletedProfiles/renewCompletedProfilesRef (array of {bundleId,
profileBase64, profileName}), so after restart you lose profile payloads and
recreate profiles; fix by persisting the full per-profile payload (bundleId,
profileBase64, profileName) into completedSteps.renewedProfiles (or change the
resume logic to re-fetch full profile payloads before saving), and initialize
renewCompletedProfiles and renewCompletedProfilesRef from that persisted
structure; update all places that read/write completedSteps.renewedProfiles and
the "renew-saving" merge logic and the remaining calculation to expect the
object shape instead of just bundleId.
- Around line 870-885: When analysis finds nothing to renew
(plan.hasAnythingToRenew is false) we must clear the stale renew state saved
earlier; before calling setStep('renew-nothing-to-do') remove the renew progress
entry from the persisted OnboardingProgress (loaded via loadProgress) and
persist the cleaned object via saveProgress so completedSteps.renewPlan is
deleted/empty for that appId; update the code that builds progressPayload (or
load the existing progress with loadProgress) to delete the renewPlan key from
completedSteps and call saveProgress(appId, cleanedProgress) before returning.
In `@cli/src/build/onboarding/ui/renew-complete.tsx`:
- Line 95: The displayed manual command inside the Text JSX ("build credentials
update --ios-provisioning-profile <path>") is missing the CLI binary; update the
Text component in renew-complete.tsx that renders this snippet so it reads
"capgo build credentials update --ios-provisioning-profile <path>" (i.e.,
prepend "capgo " to the existing command string) so users can copy-and-run it
directly.
In `@cli/src/build/onboarding/ui/renew-plan.tsx`:
- Around line 129-133: Update the warning text in onboarding/ui/renew-plan.tsx
so the displayed CLI command is runnable by prefixing it with the capgo binary;
modify the Text node that currently renders "build credentials update
--ios-provisioning-profile <path>" to instead render "capgo build credentials
update --ios-provisioning-profile <path>" (the Text component instance around
that string).
In `@cli/src/index.ts`:
- Around line 783-786: The --days option value parsed with Number.parseInt can
be NaN, negative, or partial; validate it in the CLI (where option('--days
<days>', ...) is defined) before forwarding to the renew planning code (which
expects thresholdDays in command.ts) and coerce to a sensible default of 30 when
invalid. Specifically, ensure the parsed value is an integer > 0 (e.g.,
Number.isInteger and > 0), and if it fails validation replace it with 30 (or the
existing default) so thresholdDays never receives NaN or negative values; update
the code path that sets/forwards thresholdDays to use the validated/coerced
value.
In `@cli/test/test-renew-detection.mjs`:
- Around line 148-164: The test name and assertions disagree: the test title
says "hasAnythingToRenew is false…" but the assertions expect true; update
either the test name or the assertions to be consistent. Locate the test using
computeRenewPlan and the plan variable in the test function (the case starting
with "hasAnythingToRenew is false when nothing needs renewal (force=false and
everything ok)"), and change the description to reflect that a missing cert
causes hasAnythingToRenew to be true and cert.needsRenewal true, or
alternatively change the assertions to expect false if you intend the scenario
to have nothing to renew; ensure plan.hasAnythingToRenew and
plan.cert.needsRenewal expectations match the new title.
In `@docs/plans/2026-05-18-ios-credential-renewal-design.md`:
- Line 85: The fenced code blocks starting with the "Renewal plan for
com.example.app" snippet, the "✅ Renewed for com.example.app" snippet, the
"cli/src/build/onboarding/" file tree snippet, and the "types.ts / index.ts"
snippet are missing language identifiers and trigger MD040; edit each opening
triple-backtick to include a language tag (use "text") so they read ```text for
those four blocks (e.g., the blocks containing "Renewal plan for
com.example.app:", "✅ Renewed for com.example.app", the CLI file-tree snippet
under cli/src/build/onboarding/, and the snippet listing types.ts/index.ts and
CLI flags) to satisfy markdownlint.
---
Outside diff comments:
In `@cli/src/build/credentials-manage.ts`:
- Around line 643-655: The introLines passed into setManagerScreen always
includes a static "Renew — re-issue an expiring iOS cert..." entry even when
iOS renewals aren't available, causing a misleading menu; update the introLines
construction in the setManagerScreen call to conditionally include that "Renew"
help line only when hasIos is true (i.e., push or spread the Renew string into
introLines when hasIos is truthy) and ensure the same conditional logic is
applied to the other similar help text block referenced around the same area so
the displayed intro matches available actions.
In `@cli/src/build/onboarding/command.ts`:
- Around line 93-105: The code currently sets appId only inside the try block so
an explicit options.appId is ignored if getConfig() throws; move the appId
resolution out of the try so the CLI honors --appId even when not in a Capacitor
project: call getConfig() in the try to populate extConfig, but after the try
assign appId = getAppId(options.appId, extConfig?.config) (or appId =
options.appId ?? getAppId(undefined, extConfig?.config)) and only call
getPlatformDirFromCapacitorConfig to set iosDir/androidDir if extConfig?.config
is present; ensure the existing error/exit check uses the resolved appId
variable.
🪄 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: e3992f14-f5c5-405c-a9cf-193c60c01a79
📒 Files selected for processing (18)
cli/package.jsoncli/src/build/credentials-manage.tscli/src/build/mobileprovision-parser.tscli/src/build/onboarding/command.tscli/src/build/onboarding/csr.tscli/src/build/onboarding/progress.tscli/src/build/onboarding/renew-detection.tscli/src/build/onboarding/renew-execution.tscli/src/build/onboarding/types.tscli/src/build/onboarding/ui/app.tsxcli/src/build/onboarding/ui/renew-complete.tsxcli/src/build/onboarding/ui/renew-plan.tsxcli/src/build/onboarding/ui/renew-progress.tsxcli/src/index.tscli/test/test-cert-expiry.mjscli/test/test-mobileprovision-parser.mjscli/test/test-renew-detection.mjsdocs/plans/2026-05-18-ios-credential-renewal-design.md
Comment on lines
+96
to
+116
| if (progress.mode === 'renew') { | ||
| if (!completedSteps.renewPlan) | ||
| return 'renew-analyzing' | ||
| if (!completedSteps.apiKeyVerified) { | ||
| if (progress.issuerId && progress.keyId && progress.p8Path) | ||
| return 'verifying-key' | ||
| if (progress.keyId && progress.p8Path) | ||
| return 'input-issuer-id' | ||
| if (progress.p8Path) | ||
| return 'input-key-id' | ||
| return 'api-key-instructions' | ||
| } | ||
| if (!completedSteps.certificateCreated) { | ||
| // Plan tells us whether the cert needs renewing; the renew-revoking-cert | ||
| // and creating-certificate handlers will short-circuit when it doesn't. | ||
| return 'renew-revoking-cert' | ||
| } | ||
| // Cert is done (or wasn't needed). Profiles either in progress or about to save. | ||
| if ((completedSteps.renewedProfiles?.length ?? 0) === 0) | ||
| return 'renew-creating-profiles' | ||
| return 'renew-creating-profiles' |
Contributor
There was a problem hiding this comment.
The renew resume branch is skipping required state and can renew the cert unnecessarily.
Two cases break here:
- As soon as
completedSteps.renewPlanexists, resume jumps pastrenew-plan, so an interrupted run can bypass the confirmation/warning screen entirely. - After
apiKeyVerified, this always resumes atrenew-revoking-certwhencertificateCreatedis unset, even for plans wherecert.needsRenewal === false. That turns a profile-only renew into an unintended cert revoke/recreate on resume.
This branch needs to read the stored plan and use it to decide between renew-plan, renew-revoking-cert, and renew-creating-profiles.
Suggested direction
if (progress.mode === 'renew') {
if (!completedSteps.renewPlan)
return 'renew-analyzing'
- if (!completedSteps.apiKeyVerified) {
- if (progress.issuerId && progress.keyId && progress.p8Path)
- return 'verifying-key'
- if (progress.keyId && progress.p8Path)
- return 'input-issuer-id'
- if (progress.p8Path)
- return 'input-key-id'
- return 'api-key-instructions'
- }
- if (!completedSteps.certificateCreated) {
- return 'renew-revoking-cert'
- }
+ const plan = JSON.parse(completedSteps.renewPlan) as { cert: { needsRenewal: boolean } }
+ if (!completedSteps.apiKeyVerified)
+ return 'renew-plan'
+ if (!completedSteps.certificateCreated)
+ return plan.cert.needsRenewal ? 'renew-revoking-cert' : 'renew-creating-profiles'
if ((completedSteps.renewedProfiles?.length ?? 0) === 0)
return 'renew-creating-profiles'
return 'renew-creating-profiles'
}🤖 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 `@cli/src/build/onboarding/progress.ts` around lines 96 - 116, When resuming
renew flows, don't short-circuit the user confirmation or mis-handle cert-less
renewals: if completedSteps.renewPlan is missing, return the 'renew-plan' step
(not skip to analyzing), and when certificateCreated is false consult the stored
plan (completedSteps.renewPlan or its plan.cert.needsRenewal flag) to choose
between 'renew-revoking-cert' (when cert.needsRenewal === true) and
'renew-creating-profiles' (when cert.needsRenewal === false); update the logic
in the progress.mode === 'renew' branch (referencing progress,
completedSteps.renewPlan, and certificateCreated) to implement these checks and
return the correct step.
Comment on lines
+21
to
+27
| function parseProvisioningMap(raw: string | undefined): Record<string, ProvisioningMapEntry> { | ||
| if (!raw) | ||
| return {} | ||
| try { | ||
| const parsed = JSON.parse(raw) | ||
| if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) | ||
| return parsed as Record<string, ProvisioningMapEntry> |
Contributor
There was a problem hiding this comment.
Validate provisioning-map entry shape before casting.
Line 27 trusts parsed JSON shape, and Line 177 dereferences entry.name directly. A malformed but valid JSON map (e.g. { "com.app": null }) will throw at runtime and break renew analysis.
💡 Proposed fix
function parseProvisioningMap(raw: string | undefined): Record<string, ProvisioningMapEntry> {
if (!raw)
return {}
try {
const parsed = JSON.parse(raw)
- if (parsed && typeof parsed === 'object' && !Array.isArray(parsed))
- return parsed as Record<string, ProvisioningMapEntry>
+ if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
+ const normalized: Record<string, ProvisioningMapEntry> = {}
+ for (const [bundleId, value] of Object.entries(parsed as Record<string, unknown>)) {
+ if (
+ value
+ && typeof value === 'object'
+ && typeof (value as { profile?: unknown }).profile === 'string'
+ && typeof (value as { name?: unknown }).name === 'string'
+ ) {
+ normalized[bundleId] = {
+ profile: (value as { profile: string }).profile,
+ name: (value as { name: string }).name,
+ }
+ }
+ }
+ return normalized
+ }
return {}
}
catch {
return {}
}
}Also applies to: 176-180
🤖 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 `@cli/src/build/onboarding/renew-detection.ts` around lines 21 - 27, The JSON
parsing currently casts blindly in parseProvisioningMap; instead validate each
entry's shape before casting and only include map entries where the value is a
non-null object with the expected keys (e.g., check typeof value === 'object' &&
value !== null && typeof value.name === 'string' etc.) so malformed entries like
{ "com.app": null } are skipped; update parseProvisioningMap to build and return
a filtered Record<string, ProvisioningMapEntry> and also add a defensive guard
where the map is consumed (the renew-detection code that accesses entry.name) to
check entry is defined and entry.name is a string before dereferencing.
Comment on lines
+196
to
+197
| const [renewCompletedProfiles, setRenewCompletedProfiles] = useState<Array<{ bundleId: string, profileBase64: string, profileName: string }>>([]) | ||
| const renewCompletedProfilesRef = useRef(renewCompletedProfiles) |
Contributor
There was a problem hiding this comment.
Per-profile resume persistence is incomplete, so mid-run resume loses real progress.
completedSteps.renewedProfiles only stores bundle IDs, but the resumed flow needs profileBase64/profileName too:
remainingis computed fromrenewCompletedProfilesRef.current, which resets to[]after restart.renew-savingalso builds the merged provisioning map from that in-memory array only.
So an interrupted run can recreate already-renewed profiles and still have no persisted profile payload to write back into credentials. If this PR promises per-profile resume, the progress record needs enough data to rehydrate the renewed profiles, or the resume path needs to refetch them before saving.
Also applies to: 966-1007, 1055-1062
🤖 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 `@cli/src/build/onboarding/ui/app.tsx` around lines 196 - 197, The persisted
resume data only records bundle IDs (completedSteps.renewedProfiles) while
runtime state uses renewCompletedProfiles/renewCompletedProfilesRef (array of
{bundleId, profileBase64, profileName}), so after restart you lose profile
payloads and recreate profiles; fix by persisting the full per-profile payload
(bundleId, profileBase64, profileName) into completedSteps.renewedProfiles (or
change the resume logic to re-fetch full profile payloads before saving), and
initialize renewCompletedProfiles and renewCompletedProfilesRef from that
persisted structure; update all places that read/write
completedSteps.renewedProfiles and the "renew-saving" merge logic and the
remaining calculation to expect the object shape instead of just bundleId.
Comment on lines
+870
to
+885
| // Persist plan into progress for resume — Dates serialize fine via toJSON. | ||
| const existing = await loadProgress(appId) | ||
| const progressPayload: OnboardingProgress = existing | ||
| ? { ...existing, mode: 'renew', completedSteps: { ...existing.completedSteps, renewPlan: JSON.stringify(plan) } } | ||
| : { | ||
| platform: 'ios', | ||
| appId, | ||
| startedAt: new Date().toISOString(), | ||
| mode: 'renew', | ||
| completedSteps: { renewPlan: JSON.stringify(plan) }, | ||
| } | ||
| await saveProgress(appId, progressPayload) | ||
|
|
||
| if (!plan.hasAnythingToRenew) { | ||
| setStep('renew-nothing-to-do') | ||
| return |
Contributor
There was a problem hiding this comment.
Clear renew progress when analysis finds nothing to do.
renew-analyzing persists a renew progress file before the !plan.hasAnythingToRenew early return, but the renew-nothing-to-do exit path never deletes it. That leaves a stale renew session behind and can send the next build init --renew run down the wrong resume path.
Suggested fix
await saveProgress(appId, progressPayload)
if (!plan.hasAnythingToRenew) {
+ await deleteProgress(appId)
setStep('renew-nothing-to-do')
return
}Also applies to: 1820-1838
🤖 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 `@cli/src/build/onboarding/ui/app.tsx` around lines 870 - 885, When analysis
finds nothing to renew (plan.hasAnythingToRenew is false) we must clear the
stale renew state saved earlier; before calling setStep('renew-nothing-to-do')
remove the renew progress entry from the persisted OnboardingProgress (loaded
via loadProgress) and persist the cleaned object via saveProgress so
completedSteps.renewPlan is deleted/empty for that appId; update the code that
builds progressPayload (or load the existing progress with loadProgress) to
delete the renewPlan key from completedSteps and call saveProgress(appId,
cleanedProgress) before returning.
| {' '} | ||
| Re-generate skipped profiles with: | ||
| {' '} | ||
| <Text bold>build credentials update --ios-provisioning-profile <path></Text> |
Contributor
There was a problem hiding this comment.
Fix the manual command snippet to include the CLI binary.
Line 95 should include capgo so users can run it as-is.
Suggested fix
- <Text bold>build credentials update --ios-provisioning-profile <path></Text>
+ <Text bold>capgo build credentials update --ios-provisioning-profile <path></Text>📝 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
| <Text bold>build credentials update --ios-provisioning-profile <path></Text> | |
| <Text bold>capgo build credentials update --ios-provisioning-profile <path></Text> |
🤖 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 `@cli/src/build/onboarding/ui/renew-complete.tsx` at line 95, The displayed
manual command inside the Text JSX ("build credentials update
--ios-provisioning-profile <path>") is missing the CLI binary; update the Text
component in renew-complete.tsx that renders this snippet so it reads "capgo
build credentials update --ios-provisioning-profile <path>" (i.e., prepend
"capgo " to the existing command string) so users can copy-and-run it directly.
Comment on lines
+129
to
+133
| User-imported provisioning profiles will be invalidated when the cert is renewed. | ||
| Re-generate them manually with | ||
| {' '} | ||
| <Text bold>build credentials update --ios-provisioning-profile <path></Text> | ||
| {' '} |
Contributor
There was a problem hiding this comment.
Use a runnable CLI command in the warning text.
Line 132 omits the capgo binary prefix, so the pasted command is not directly executable in a shell.
Suggested fix
- <Text bold>build credentials update --ios-provisioning-profile <path></Text>
+ <Text bold>capgo build credentials update --ios-provisioning-profile <path></Text>📝 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
| User-imported provisioning profiles will be invalidated when the cert is renewed. | |
| Re-generate them manually with | |
| {' '} | |
| <Text bold>build credentials update --ios-provisioning-profile <path></Text> | |
| {' '} | |
| User-imported provisioning profiles will be invalidated when the cert is renewed. | |
| Re-generate them manually with | |
| {' '} | |
| <Text bold>capgo build credentials update --ios-provisioning-profile <path></Text> | |
| {' '} |
🤖 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 `@cli/src/build/onboarding/ui/renew-plan.tsx` around lines 129 - 133, Update
the warning text in onboarding/ui/renew-plan.tsx so the displayed CLI command is
runnable by prefixing it with the capgo binary; modify the Text node that
currently renders "build credentials update --ios-provisioning-profile <path>"
to instead render "capgo build credentials update --ios-provisioning-profile
<path>" (the Text component instance around that string).
Comment on lines
+783
to
+786
| .option('--force', '(--renew) Re-issue cert and profiles regardless of expiry.') | ||
| .option('--days <days>', '(--renew) Threshold in days for "expiring soon" (default: 30).', value => Number.parseInt(value, 10)) | ||
| .option('--dry-run', '(--renew) Print the renewal plan and exit without making changes.') | ||
| .option('--local', '(--renew) Operate on local .capgo-credentials.json instead of the global file.') |
Contributor
There was a problem hiding this comment.
Validate --days before passing it into renew planning.
Number.parseInt() here accepts NaN, negatives, and partial values, and command.ts will forward that straight into thresholdDays. That can turn the renewal plan into a silent misclassification instead of falling back to 30.
Suggested fix
- .option('--days <days>', '(--renew) Threshold in days for "expiring soon" (default: 30).', value => Number.parseInt(value, 10))
+ .option('--days <days>', '(--renew) Threshold in days for "expiring soon" (default: 30).', (value) => {
+ const days = Number.parseInt(value, 10)
+ if (!Number.isInteger(days) || days < 0)
+ throw new Error('--days must be a non-negative integer')
+ return days
+ })📝 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
| .option('--force', '(--renew) Re-issue cert and profiles regardless of expiry.') | |
| .option('--days <days>', '(--renew) Threshold in days for "expiring soon" (default: 30).', value => Number.parseInt(value, 10)) | |
| .option('--dry-run', '(--renew) Print the renewal plan and exit without making changes.') | |
| .option('--local', '(--renew) Operate on local .capgo-credentials.json instead of the global file.') | |
| .option('--force', '(--renew) Re-issue cert and profiles regardless of expiry.') | |
| .option('--days <days>', '(--renew) Threshold in days for "expiring soon" (default: 30).', (value) => { | |
| const days = Number.parseInt(value, 10) | |
| if (!Number.isInteger(days) || days < 0) | |
| throw new Error('--days must be a non-negative integer') | |
| return days | |
| }) | |
| .option('--dry-run', '(--renew) Print the renewal plan and exit without making changes.') | |
| .option('--local', '(--renew) Operate on local .capgo-credentials.json instead of the global file.') |
🤖 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 `@cli/src/index.ts` around lines 783 - 786, The --days option value parsed with
Number.parseInt can be NaN, negative, or partial; validate it in the CLI (where
option('--days <days>', ...) is defined) before forwarding to the renew planning
code (which expects thresholdDays in command.ts) and coerce to a sensible
default of 30 when invalid. Specifically, ensure the parsed value is an integer
> 0 (e.g., Number.isInteger and > 0), and if it fails validation replace it with
30 (or the existing default) so thresholdDays never receives NaN or negative
values; update the code path that sets/forwards thresholdDays to use the
validated/coerced value.
Comment on lines
+148
to
+164
| t('hasAnythingToRenew is false when nothing needs renewal (force=false and everything ok)', () => { | ||
| // We can't easily produce a "valid cert" without forging a real P12, so skip via mock: | ||
| // mock by ALSO putting a far-future cert. Since we have no P12 base64, cert will be marked | ||
| // expired. To represent "everything ok," we'd need a forged P12 — outside the unit test scope. | ||
| // Verify the inverse instead: when all profiles are user-imported AND cert is missing, only | ||
| // cert needs renewing. | ||
| const saved = { | ||
| CAPGO_IOS_PROVISIONING_MAP: JSON.stringify(makeMap([ | ||
| { bundleId: 'com.example.app.widget', name: 'match AdHoc com.example.app.widget', expDays: 365 }, | ||
| ])), | ||
| } | ||
| const plan = computeRenewPlan(saved, APP_ID, { thresholdDays: 30, force: false }, NOW) | ||
| assert.equal(plan.hasAnythingToRenew, true) // because cert is missing | ||
| assert.equal(plan.cert.needsRenewal, true) | ||
| assert.equal(plan.profiles.length, 1) | ||
| assert.equal(plan.profiles[0].needsRenewal, false) | ||
| }) |
Contributor
There was a problem hiding this comment.
Test name contradicts its own assertion.
Line 148 says “hasAnythingToRenew is false…”, but Line 160 asserts true. This makes the suite harder to trust when diagnosing regressions.
💡 Proposed fix
-t('hasAnythingToRenew is false when nothing needs renewal (force=false and everything ok)', () => {
+t('hasAnythingToRenew is true when cert is missing even if profiles are skipped', () => {📝 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
| t('hasAnythingToRenew is false when nothing needs renewal (force=false and everything ok)', () => { | |
| // We can't easily produce a "valid cert" without forging a real P12, so skip via mock: | |
| // mock by ALSO putting a far-future cert. Since we have no P12 base64, cert will be marked | |
| // expired. To represent "everything ok," we'd need a forged P12 — outside the unit test scope. | |
| // Verify the inverse instead: when all profiles are user-imported AND cert is missing, only | |
| // cert needs renewing. | |
| const saved = { | |
| CAPGO_IOS_PROVISIONING_MAP: JSON.stringify(makeMap([ | |
| { bundleId: 'com.example.app.widget', name: 'match AdHoc com.example.app.widget', expDays: 365 }, | |
| ])), | |
| } | |
| const plan = computeRenewPlan(saved, APP_ID, { thresholdDays: 30, force: false }, NOW) | |
| assert.equal(plan.hasAnythingToRenew, true) // because cert is missing | |
| assert.equal(plan.cert.needsRenewal, true) | |
| assert.equal(plan.profiles.length, 1) | |
| assert.equal(plan.profiles[0].needsRenewal, false) | |
| }) | |
| t('hasAnythingToRenew is true when cert is missing even if profiles are skipped', () => { | |
| // We can't easily produce a "valid cert" without forging a real P12, so skip via mock: | |
| // mock by ALSO putting a far-future cert. Since we have no P12 base64, cert will be marked | |
| // expired. To represent "everything ok," we'd need a forged P12 — outside the unit test scope. | |
| // Verify the inverse instead: when all profiles are user-imported AND cert is missing, only | |
| // cert needs renewing. | |
| const saved = { | |
| CAPGO_IOS_PROVISIONING_MAP: JSON.stringify(makeMap([ | |
| { bundleId: 'com.example.app.widget', name: 'match AdHoc com.example.app.widget', expDays: 365 }, | |
| ])), | |
| } | |
| const plan = computeRenewPlan(saved, APP_ID, { thresholdDays: 30, force: false }, NOW) | |
| assert.equal(plan.hasAnythingToRenew, true) // because cert is missing | |
| assert.equal(plan.cert.needsRenewal, true) | |
| assert.equal(plan.profiles.length, 1) | |
| assert.equal(plan.profiles[0].needsRenewal, 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 `@cli/test/test-renew-detection.mjs` around lines 148 - 164, The test name and
assertions disagree: the test title says "hasAnythingToRenew is false…" but the
assertions expect true; update either the test name or the assertions to be
consistent. Locate the test using computeRenewPlan and the plan variable in the
test function (the case starting with "hasAnythingToRenew is false when nothing
needs renewal (force=false and everything ok)"), and change the description to
reflect that a missing cert causes hasAnythingToRenew to be true and
cert.needsRenewal true, or alternatively change the assertions to expect false
if you intend the scenario to have nothing to renew; ensure
plan.hasAnythingToRenew and plan.cert.needsRenewal expectations match the new
title.
| ### Step D — Show plan, ask to confirm | ||
| Render a table: | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks to satisfy markdownlint.
Line 85, Line 125, Line 166, and Line 178 start fenced blocks without a language, which triggers MD040 and can fail doc lint checks.
💡 Suggested patch
-```
+```text
Renewal plan for com.example.app:
...
Continue? [Y/n]
-```
+```
-```
+```text
✅ Renewed for com.example.app
...
Run a test build now? [Y/n]
-```
+```
-```
+```text
cli/src/build/onboarding/
renew-detection.ts Pure plan computation
...
renew-complete.tsx Completion summary
-```
+```
-```
+```text
cli/src/build/onboarding/
types.ts Add renew-specific OnboardingStep values; add mode field to OnboardingProgress
...
index.ts Add --renew, --force, --days, --dry-run options to build init
-```
+```Also applies to: 125-125, 166-166, 178-178
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 85-85: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/plans/2026-05-18-ios-credential-renewal-design.md` at line 85, The
fenced code blocks starting with the "Renewal plan for com.example.app" snippet,
the "✅ Renewed for com.example.app" snippet, the "cli/src/build/onboarding/"
file tree snippet, and the "types.ts / index.ts" snippet are missing language
identifiers and trigger MD040; edit each opening triple-backtick to include a
language tag (use "text") so they read ```text for those four blocks (e.g., the
blocks containing "Renewal plan for com.example.app:", "✅ Renewed for
com.example.app", the CLI file-tree snippet under cli/src/build/onboarding/, and
the snippet listing types.ts/index.ts and CLI flags) to satisfy markdownlint.
|
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
build init --renew, a one-command renewal flow for iOS distribution certificates and Capgo-issued provisioning profiles. Inspects saved credentials, computes a plan (default: anything expiring in ≤30 days), then re-issues only what's needed via the App Store Connect API.mode: 'renew'branch onOnboardingApp— same Apple-API helpers, same error screens, same support-bundle diagnostics on failure.build credentials manage(visible on iOS apps only). Selecting it confirms, stops the manage Ink session, and hands off to the same renew flow for the picked app..p8input chain when the saved App Store Connect API key is rejected (401/403), so a rotated key doesn't force the user to start over.Capgo <appId> AppStoreconvention) are surfaced in the plan but skipped — we don't have enough metadata (profile type, ad-hoc device list, etc.) to re-create them via Apple's API. When the cert is being renewed, the confirm prompt's default flips to No and anAlertwarning is rendered so the user knows those profiles will need manual regeneration.Flags
--renewbuild initinto renewal mode (iOS only).--force--days <N>30).--dry-run--local.capgo-credentials.jsoninstead of the global file.--appId <id>capacitor.configauto-detection (also useful for non-renew runs).Design doc
docs/plans/2026-05-18-ios-credential-renewal-design.md — fully spec'd: behavior, flow, edge cases, telemetry, testing strategy, migration notes, explicit non-goals (Android renewal, auto-renew during
build request, cross-app bulk, etc.).Notable implementation choices
cert-limit-promptflow if no matching cert is found and Apple still reports the limit.updateSavedCredentialsis called exactly once — saved creds are not mutated until every plan step succeeds.OnboardingProgress.modeis optional; a progress file without it is treated asmode: 'init'so existing in-flight onboardings continue working.Test plan
bunx tsc --noEmit— cleancli/test/test-renew-detection.mjs— 15 new tests covering threshold logic, force, user-imported skip, multi-target sort order, malformed map, legacy-format detectioncli/test/test-cert-expiry.mjs— 5 new tests coveringextractCertExpiry/extractCertSerialwith default password, wrong password fallback, empty password, and malformed inputcli/test/test-mobileprovision-parser.mjs— extended withexpirationDateassertions (3 cases)cli/test/test-credentials.mjs,test-credentials-validation.mjs,test-onboarding-recovery.mjs— re-run, no regressions🔒 No DB / server-side / shared-protocol changes. Strictly additive to the CLI workspace.
Summary by CodeRabbit
New Features
Tests
Documentation