fix(cli): recover from missing notifyAppReady / public key / appId - #3184
fix(cli): recover from missing notifyAppReady / public key / appId#3184riderx wants to merge 5 commits into
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
Add interactive recovery flows for the three most common CLI configuration failures tracked in PostHog error tracking, instead of silencing them as CliUserError. - notifyAppReady: patch build output or source entry, manual snippet, or explicit skip with warning; keep PostHog capture on unresolved failures - public key: generate or import keys into capacitor config before encrypt - appId: auto-detect from project files, select from Capgo apps, or create app Includes unit tests and --ignore-notify-app-ready flag alias. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
06d0826 to
7ce75f6
Compare
|
Warning Review limit reached
On-demand reviews are free for the next 28 days. After that, they cost $0.25 per reviewed file. Or wait 6 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 51 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
Comment |
Importing init/command pulled Ink into the bundle build and broke CI. Keep notifyAppReady injection self-contained in the recovery module. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
|
There was a problem hiding this comment.
10 issues found across 10 files
Confidence score: 2/5
cli/src/bundle/encrypt.tscan pair a supplied private key with an unrelated default public key, producing bundles that cannot be recovered; ensure the public key is derived or validated against the supplied key before encryption.cli/src/recovery/notify-app-ready.tsmay patch an unresolved import or select a runtime/polyfill script instead of the application entry, then report success without a working update hook; restrict patching to valid entries and verify the rebuilt output contains the call.cli/src/recovery/public-key.tsandcli/src/bundle/encrypt.tscan write key files or rewrite configuration before later validation fails, leaving mismatched or partially mutated recovery state; validate inputs and existing key pairs before any writes.cli/src/recovery/app-id.tscan block manual recovery, accept invalid remoteapp_idvalues, or ignore IDs in a selectedpackage.json, causing incorrect or missing Capacitor configuration; validate candidates and pass the configured package paths through recovery.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cli/src/bundle/encrypt.ts">
<violation number="1" location="cli/src/bundle/encrypt.ts:40">
P1: When `bundle encrypt` supplies `--key` or `--key-data` while `publicKey` is missing, recovery can install an unrelated default public key, but encryption uses the supplied private key. The resulting bundle cannot be consumed; skip automatic recovery for explicit private keys or derive and save their matching public key.</violation>
<violation number="2" location="cli/src/bundle/encrypt.ts:40">
P2: When the zip path is missing, this recovery can generate key files and rewrite capacitor config before the existing zip guard runs. Check the zip path before invoking recovery so an invalid input does not mutate encryption state.</violation>
<violation number="3" location="cli/src/bundle/encrypt.ts:42">
P1: With `--capacitor-config` selecting a source file, recovery writes that file but this refresh reads the active root config. The command can report a missing public key after a successful write; refresh with `getConfigForWrite()`.</violation>
</file>
<file name="cli/src/recovery/public-key.ts">
<violation number="1" location="cli/src/recovery/public-key.ts:47">
P1: When `.capgo_key_v2` exists but `.capgo_key_v2.pub` is missing, this recovery generates and writes a new public key before `createKeyInternal` rejects the existing private key. A retry then sees the mismatched public file and saves it to the config, so bundles encrypted with the old private key cannot be decrypted; refuse generation when the private key already exists or make keypair creation atomic.</violation>
</file>
<file name="cli/src/recovery/app-id.ts">
<violation number="1" location="cli/src/recovery/app-id.ts:31">
P2: When the app ID exists only in a package.json selected with `--package-json`, recovery ignores that file and falls through to the missing-ID prompt or error. Pass the configured package-json path(s) into candidate collection instead of always reading the workspace-root package.json.</violation>
<violation number="2" location="cli/src/recovery/app-id.ts:153">
P1: When `bundle zip` runs without a saved/provided API key and no local candidate is detected, this guard aborts before the manual recovery option appears. Require an API key only for remote listing or app creation, while allowing detected/manual choices for local zip.</violation>
<violation number="3" location="cli/src/recovery/app-id.ts:173">
P2: When the Capgo app-list response contains an invalid or missing `app_id`, the prompt still offers it and selecting it writes the invalid value into Capacitor config. Filter remote rows with `isValidAppId` before constructing selection options.
(Based on your team's feedback about validating filtered app results.)</violation>
</file>
<file name="cli/src/recovery/notify-app-ready.ts">
<violation number="1" location="cli/src/recovery/notify-app-ready.ts:35">
P1: When `index.html` lists runtime or polyfill scripts before the app bundle, this returns the wrong file and `patch-build` reports recovery without changing the application entry. Identify the actual app entry or patch the relevant bundled scripts rather than taking the first script.</violation>
<violation number="2" location="cli/src/recovery/notify-app-ready.ts:63">
P1: When the built bundle does not already contain the updater binding, `patch-build` adds an unresolved bare package import to the static output, then treats the text match as success. Restrict build patching to an already-bundled binding and require source patch plus rebuild otherwise.</violation>
<violation number="3" location="cli/src/recovery/notify-app-ready.ts:200">
P2: When `patch-source` is selected but the rebuild is cancelled or still lacks the call, this emits a successful-recovery event anyway. Move the event after the rebuilt folder passes `searchInDirectory(webDir, 'notifyAppReady')`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| const hasPrivateKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.privateKey | ||
| const hasPublicKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.publicKey | ||
| await ensurePublicKeyInConfig({ interactive, silent: silent || json, json }) |
There was a problem hiding this comment.
P1: When bundle encrypt supplies --key or --key-data while publicKey is missing, recovery can install an unrelated default public key, but encryption uses the supplied private key. The resulting bundle cannot be consumed; skip automatic recovery for explicit private keys or derive and save their matching public key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/bundle/encrypt.ts, line 40:
<comment>When `bundle encrypt` supplies `--key` or `--key-data` while `publicKey` is missing, recovery can install an unrelated default public key, but encryption uses the supplied private key. The resulting bundle cannot be consumed; skip automatic recovery for explicit private keys or derive and save their matching public key.</comment>
<file context>
@@ -34,10 +35,13 @@ export async function encryptZipInternal(
- const hasPrivateKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.privateKey
- const hasPublicKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.publicKey
+ await ensurePublicKeyInConfig({ interactive, silent: silent || json, json })
+
+ const refreshedConfig = await getConfig()
</file context>
| await ensurePublicKeyInConfig({ interactive, silent: silent || json, json }) | |
| if (!options.key && !options.keyData) | |
| await ensurePublicKeyInConfig({ interactive, silent: silent || json, json }) |
| const hasPublicKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.publicKey | ||
| await ensurePublicKeyInConfig({ interactive, silent: silent || json, json }) | ||
|
|
||
| const refreshedConfig = await getConfig() |
There was a problem hiding this comment.
P1: With --capacitor-config selecting a source file, recovery writes that file but this refresh reads the active root config. The command can report a missing public key after a successful write; refresh with getConfigForWrite().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/bundle/encrypt.ts, line 42:
<comment>With `--capacitor-config` selecting a source file, recovery writes that file but this refresh reads the active root config. The command can report a missing public key after a successful write; refresh with `getConfigForWrite()`.</comment>
<file context>
@@ -34,10 +35,13 @@ export async function encryptZipInternal(
- const hasPublicKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.publicKey
+ await ensurePublicKeyInConfig({ interactive, silent: silent || json, json })
+
+ const refreshedConfig = await getConfig()
+ const hasPrivateKeyInConfig = !!refreshedConfig.config.plugins?.CapacitorUpdater?.privateKey
+ const hasPublicKeyInConfig = !!refreshedConfig.config.plugins?.CapacitorUpdater?.publicKey
</file context>
| } | ||
| else { | ||
| log.info('Capgo can generate an RSA keypair, save the public key in capacitor config, and keep the private key in .capgo_key_v2.') | ||
| await createKeyInternal({ force: false, setupChannel: false }, true, extConfig) |
There was a problem hiding this comment.
P1: When .capgo_key_v2 exists but .capgo_key_v2.pub is missing, this recovery generates and writes a new public key before createKeyInternal rejects the existing private key. A retry then sees the mismatched public file and saves it to the config, so bundles encrypted with the old private key cannot be decrypted; refuse generation when the private key already exists or make keypair creation atomic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/recovery/public-key.ts, line 47:
<comment>When `.capgo_key_v2` exists but `.capgo_key_v2.pub` is missing, this recovery generates and writes a new public key before `createKeyInternal` rejects the existing private key. A retry then sees the mismatched public file and saves it to the config, so bundles encrypted with the old private key cannot be decrypted; refuse generation when the private key already exists or make keypair creation atomic.</comment>
<file context>
@@ -0,0 +1,63 @@
+ }
+ else {
+ log.info('Capgo can generate an RSA keypair, save the public key in capacitor config, and keep the private key in .capgo_key_v2.')
+ await createKeyInternal({ force: false, setupChannel: false }, true, extConfig)
+ }
+
</file context>
|
|
||
| const resolvedApikey = options.apikey || findSavedKey() | ||
| if (!resolvedApikey) | ||
| throw new Error('Missing API key. Run `npx @capgo/cli@latest login` first.') |
There was a problem hiding this comment.
P1: When bundle zip runs without a saved/provided API key and no local candidate is detected, this guard aborts before the manual recovery option appears. Require an API key only for remote listing or app creation, while allowing detected/manual choices for local zip.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/recovery/app-id.ts, line 153:
<comment>When `bundle zip` runs without a saved/provided API key and no local candidate is detected, this guard aborts before the manual recovery option appears. Require an API key only for remote listing or app creation, while allowing detected/manual choices for local zip.</comment>
<file context>
@@ -0,0 +1,268 @@
+
+ const resolvedApikey = options.apikey || findSavedKey()
+ if (!resolvedApikey)
+ throw new Error('Missing API key. Run `npx @capgo/cli@latest login` first.')
+
+ while (true) {
</file context>
| continue | ||
| const candidate = join(webDir, src.replace(/^\//, '')) | ||
| if (existsSync(candidate) && extname(candidate) === '.js') | ||
| return candidate |
There was a problem hiding this comment.
P1: When index.html lists runtime or polyfill scripts before the app bundle, this returns the wrong file and patch-build reports recovery without changing the application entry. Identify the actual app entry or patch the relevant bundled scripts rather than taking the first script.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/recovery/notify-app-ready.ts, line 35:
<comment>When `index.html` lists runtime or polyfill scripts before the app bundle, this returns the wrong file and `patch-build` reports recovery without changing the application entry. Identify the actual app entry or patch the relevant bundled scripts rather than taking the first script.</comment>
<file context>
@@ -0,0 +1,213 @@
+ continue
+ const candidate = join(webDir, src.replace(/^\//, ''))
+ if (existsSync(candidate) && extname(candidate) === '.js')
+ return candidate
+ }
+ }
</file context>
|
|
||
| const updaterImport = extname(filePath) === '.cjs' | ||
| ? 'const { CapacitorUpdater } = require(\'@capgo/capacitor-updater\')' | ||
| : 'import { CapacitorUpdater } from \'@capgo/capacitor-updater\'' |
There was a problem hiding this comment.
P1: When the built bundle does not already contain the updater binding, patch-build adds an unresolved bare package import to the static output, then treats the text match as success. Restrict build patching to an already-bundled binding and require source patch plus rebuild otherwise.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/recovery/notify-app-ready.ts, line 63:
<comment>When the built bundle does not already contain the updater binding, `patch-build` adds an unresolved bare package import to the static output, then treats the text match as success. Restrict build patching to an already-bundled binding and require source patch plus rebuild otherwise.</comment>
<file context>
@@ -0,0 +1,213 @@
+
+ const updaterImport = extname(filePath) === '.cjs'
+ ? 'const { CapacitorUpdater } = require(\'@capgo/capacitor-updater\')'
+ : 'import { CapacitorUpdater } from \'@capgo/capacitor-updater\''
+ return `${updaterImport};\n\n${NOTIFY_CALL};\n${content}`
+}
</file context>
|
|
||
| const hasPrivateKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.privateKey | ||
| const hasPublicKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.publicKey | ||
| await ensurePublicKeyInConfig({ interactive, silent: silent || json, json }) |
There was a problem hiding this comment.
P2: When the zip path is missing, this recovery can generate key files and rewrite capacitor config before the existing zip guard runs. Check the zip path before invoking recovery so an invalid input does not mutate encryption state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/bundle/encrypt.ts, line 40:
<comment>When the zip path is missing, this recovery can generate key files and rewrite capacitor config before the existing zip guard runs. Check the zip path before invoking recovery so an invalid input does not mutate encryption state.</comment>
<file context>
@@ -34,10 +35,13 @@ export async function encryptZipInternal(
- const hasPrivateKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.privateKey
- const hasPublicKeyInConfig = !!extConfig.config.plugins?.CapacitorUpdater?.publicKey
+ await ensurePublicKeyInConfig({ interactive, silent: silent || json, json })
+
+ const refreshedConfig = await getConfig()
</file context>
| push(config?.appId) | ||
| push(config?.plugins?.CapacitorUpdater?.appId) | ||
|
|
||
| const packageJsonPath = join(projectRoot, PACKNAME) |
There was a problem hiding this comment.
P2: When the app ID exists only in a package.json selected with --package-json, recovery ignores that file and falls through to the missing-ID prompt or error. Pass the configured package-json path(s) into candidate collection instead of always reading the workspace-root package.json.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/recovery/app-id.ts, line 31:
<comment>When the app ID exists only in a package.json selected with `--package-json`, recovery ignores that file and falls through to the missing-ID prompt or error. Pass the configured package-json path(s) into candidate collection instead of always reading the workspace-root package.json.</comment>
<file context>
@@ -0,0 +1,268 @@
+ push(config?.appId)
+ push(config?.plugins?.CapacitorUpdater?.appId)
+
+ const packageJsonPath = join(projectRoot, PACKNAME)
+ if (existsSync(packageJsonPath)) {
+ try {
</file context>
| }) | ||
| } | ||
| } | ||
| for (const app of remoteApps) { |
There was a problem hiding this comment.
P2: When the Capgo app-list response contains an invalid or missing app_id, the prompt still offers it and selecting it writes the invalid value into Capacitor config. Filter remote rows with isValidAppId before constructing selection options.
(Based on your team's feedback about validating filtered app results.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/recovery/app-id.ts, line 173:
<comment>When the Capgo app-list response contains an invalid or missing `app_id`, the prompt still offers it and selecting it writes the invalid value into Capacitor config. Filter remote rows with `isValidAppId` before constructing selection options.
(Based on your team's feedback about validating filtered app results.) </comment>
<file context>
@@ -0,0 +1,268 @@
+ })
+ }
+ }
+ for (const app of remoteApps) {
+ selectOptions.push({
+ value: `remote:${app.app_id}`,
</file context>
| } | ||
| log.success(`Added notifyAppReady() to ${patchedPath}`) | ||
| log.info('Rebuild your web assets, then retry zip/upload.') | ||
| void trackEvent({ channel: 'bundle', event: 'CLI Recovered Missing NotifyAppReady', tags: { recovery: 'patch-source' } }) |
There was a problem hiding this comment.
P2: When patch-source is selected but the rebuild is cancelled or still lacks the call, this emits a successful-recovery event anyway. Move the event after the rebuilt folder passes searchInDirectory(webDir, 'notifyAppReady').
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/recovery/notify-app-ready.ts, line 200:
<comment>When `patch-source` is selected but the rebuild is cancelled or still lacks the call, this emits a successful-recovery event anyway. Move the event after the rebuilt folder passes `searchInDirectory(webDir, 'notifyAppReady')`.</comment>
<file context>
@@ -0,0 +1,213 @@
+ }
+ log.success(`Added notifyAppReady() to ${patchedPath}`)
+ log.info('Rebuild your web assets, then retry zip/upload.')
+ void trackEvent({ channel: 'bundle', event: 'CLI Recovered Missing NotifyAppReady', tags: { recovery: 'patch-source' } })
+ const rebuilt = await pConfirm({ message: 'Have you rebuilt your app into the build folder?' })
+ if (pIsCancel(rebuilt) || !rebuilt)
</file context>



Summary (AI generated)
bundle zip/bundle uploadnow offer patch-build, patch-source, manual snippet, or explicit skip-with-warning whennotifyAppReady()is missing in the build folder.bundle encryptcan generate/import RSA keys and writeCapacitorUpdater.publicKeyinto capacitor config, then continue.applicationId, Capgo app list selection, manual entry, orapp addcreate flow.--ignore-notify-app-ready(alias escape hatch alongside--no-code-check) andtest:cli-recoveryunit tests.Motivation (AI generated)
PostHog error tracking shows recurring CLI failures that are fixable in-place, but users currently hit a hard stop:
Martin's direction: do not convert these to
CliUserErrorjust to hide PostHog tracking. Keep tracking until a success path actually runs (or emit explicit recovery analytics events).Business Impact (AI generated)
CLI Recovered Missing *) give a success funnel alongside existing$exceptionissues.Test Plan (AI generated)
bunx tsc --noEmitincli/bun run test:cli-recoverybun run test:posthog-exception(confirms unresolved failures remain capturable)clitest suitebundle zipin a project missingnotifyAppReady()and choose patch-buildbundle encryptwithoutpublicKeyin config interactivelybundle uploadwithout appId arg and pick/create an appGenerated with AI
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.