This repository was archived by the owner on May 1, 2026. It is now read-only.
fix: prevent clack stdout corruption in Ink onboarding UI#560
Merged
Conversation
…ing UI
During init, functions like findProjectType() and checkAlerts() wrote
directly to stdout via @clack/prompts while the Ink terminal UI had
control, causing garbled rendering and a broken header banner.
- Split checkAlerts into checkVersionStatus (pure data) + checkAlerts (clack UI)
- Init flow now uses checkVersionStatus and renders an Ink Alert for outdated CLI
- findProjectType accepts { quiet: true } to suppress clack logging during init
- loginInternal skips clack output when called with silent=true from init
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR refactors version-checking logic by extracting status information into a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
3 tasks
WcaleNieWolny
added a commit
that referenced
this pull request
Apr 9, 2026
addChannelInternal is invoked from `capgo init` with silent=true while the Ink onboarding UI owns stdout. Several helpers it transitively reaches still wrote directly to process.stdout via @clack/prompts, which broke the Ink frame and caused the 'Capgo OTA Onboarding' header to be repainted twice when the user picked 'production' in the channel step. Continuing the whack-a-mole pattern from #560, thread an opt-in silent flag through the offending helpers and gate every clack write with it: - getConfig, getLocalConfig, getRemoteConfig, createSupabaseClient now accept silent and propagate it through the call chain. - sendEvent always loads remote config silently and only logs telemetry failures when verbose is set, since it is invoked in fire-and-forget contexts (including init markStep -> markSnag). - addChannelInternal forwards its existing silent flag to getConfig, createSupabaseClient, and findUnknownVersion. No interceptor / monkey-patching: each fix is at the actual call site.
5 tasks
WcaleNieWolny
added a commit
that referenced
this pull request
Apr 9, 2026
addChannelInternal is invoked from `capgo init` with silent=true while the Ink onboarding UI owns stdout. Several helpers it transitively reaches still wrote directly to process.stdout via @clack/prompts, which broke the Ink frame and caused the 'Capgo OTA Onboarding' header to be repainted twice when the user picked 'production' in the channel step. Continuing the whack-a-mole pattern from #560, thread an opt-in silent flag through the offending helpers and gate every clack write with it: - getConfig, getLocalConfig, getRemoteConfig, createSupabaseClient now accept silent and propagate it through the call chain. - sendEvent always loads remote config silently and only logs telemetry failures when verbose is set, since it is invoked in fire-and-forget contexts (including init markStep -> markSnag). - addChannelInternal forwards its existing silent flag to getConfig, createSupabaseClient, and findUnknownVersion. No interceptor / monkey-patching: each fix is at the actual call site.
5 tasks
WcaleNieWolny
added a commit
that referenced
this pull request
Apr 9, 2026
addEncryptionStep was calling createKeyInternal({ force: true }, false)
which, in non-silent mode:
1. Writes to stdout directly via clack's intro(), log.*, and pConfirm
(the exact whack-a-mole gating problem PR #560/#579 fixed for the
rest of the init flow — it renders garbled output when ink is also
drawing).
2. Internally calls promptAndSyncCapacitor with validateIosUpdater=true,
which throws 'iOS sync validation failed. Delete your iOS folder…'
and bubbles up as 'Error during onboarding' on any mis-synced iOS
project.
Step 7 (buildProjectStep) already runs `cap sync` with proper iOS
recovery via handleBrokenIosSync, so a second sync inside the
encryption step is strictly redundant.
Pass silent=true and setupChannel=false to skip all of the above:
keys get written, config is updated, and the ink-managed spinner
stays as the only thing drawing to the screen. Also wrap the call
in try/catch so the existing 'retry / skip' recovery path actually
fires on failure (createKeyInternal throws rather than returning
false on errors, so the old `if (keyRes)` else branch was dead).
WcaleNieWolny
added a commit
that referenced
this pull request
Apr 10, 2026
* feat(init): persist encryption outcome as summary panel on step 6
Previously, the tail of addEncryptionStep pushed 3-7 pLog.info lines
(outcome bullets for enabled / skipped / failed paths) that were
immediately wiped when the next step's renderInitOnboardingFrame
called clearInitLogs() — they only appeared as a flash.
Mirror the step-4 code-diff panel pattern: introduce
InitEncryptionSummary state + setInitEncryptionSummary runtime setter
and render an EncryptionSummaryPanel (green border when enabled,
yellow when skipped / failed) that survives the step-5→step-6 frame
transition.
The summary is built in addEncryptionStep based on the final decision
(enabled / user skipped / key creation failed) and cleared in the
main onboarding loop after step 6 completes, so it shows as a
persistent 'what happened' callout on the Select Platform screen.
Also persisted into the resume JSON and restored on resume-at-step-6
so the panel survives interrupted sessions.
Follow-up fixes included:
- hide 'What is encryption? (learn more)' option after user has
already viewed the overview (re-offering it made no sense)
- always print the docs URL as part of the overview (so users can
copy it even if they decline to open their browser right now)
- use an embedded newline before the docs URL instead of a separate
empty log entry (Ink collapses empty Text to zero height)
- use canonical '${pm.runner} @capgo/cli@latest key create' rather
than a non-existent 'capgo key create' shortcut in the summary
* fix(init): drop redundant second encryption prompt
Q1 already labels its yes branch as 'Yes — set up end-to-end
encryption', so the follow-up 'Do you want to use encryption for X?'
select with the same yes/no options was just re-asking the same
question. The 3 intermediate info lines ('Capgo bundles are web
assets...' / 'That is why we recommend encryption...' / 'Do not put
private API keys...') were also redundant — learn-more already
covers the rationale and the summary panel covers the warning.
Critical answers at Q1 now go straight into key creation.
* fix(init): run key creation in silent mode to avoid ink/clack collision
addEncryptionStep was calling createKeyInternal({ force: true }, false)
which, in non-silent mode:
1. Writes to stdout directly via clack's intro(), log.*, and pConfirm
(the exact whack-a-mole gating problem PR #560/#579 fixed for the
rest of the init flow — it renders garbled output when ink is also
drawing).
2. Internally calls promptAndSyncCapacitor with validateIosUpdater=true,
which throws 'iOS sync validation failed. Delete your iOS folder…'
and bubbles up as 'Error during onboarding' on any mis-synced iOS
project.
Step 7 (buildProjectStep) already runs `cap sync` with proper iOS
recovery via handleBrokenIosSync, so a second sync inside the
encryption step is strictly redundant.
Pass silent=true and setupChannel=false to skip all of the above:
keys get written, config is updated, and the ink-managed spinner
stays as the only thing drawing to the screen. Also wrap the call
in try/catch so the existing 'retry / skip' recovery path actually
fires on failure (createKeyInternal throws rather than returning
false on errors, so the old `if (keyRes)` else branch was dead).
* fix(init): suppress key-creation success flash and add dev helpers
- Stop the key-creation spinner without a success message. Passing a
message routes it through pushInitLog into the rolling log buffer,
which renderInitOnboardingFrame wipes when step 6 renders — producing
a visible "Keys created 🔑" flash. The persistent EncryptionSummaryPanel
already surfaces the outcome on step 6, so nothing is lost.
- Add scripts/remove-encryption-key.mjs: TEST-ONLY helper that deletes
.capgo_key{,_v2}{,.pub} and scrubs publicKey/privateKey from both
capacitor.config.json and capacitor.config.{ts,js} so the encryption
step can be re-run from a clean slate. Supports --show and --dry-run.
- Track scripts/reset-onboarding-step.mjs and
scripts/remove-notify-app-ready.mjs, the sibling dev helpers used for
iterating on the onboarding wizard.
* feat(init): stream cap sync output after key creation in step 5
Instead of deferring the native sync to step 7 and showing a
misleading "Encryption ENABLED" panel, we now run `cap sync` right
inside step 5 with a full-screen streaming output panel. This mirrors
the log-streaming UX from the builder onboarding.
Changes:
- Add InitStreamingOutput state + streaming panel to runtime/UI. When
active it takes over the entire viewport (header + tail-view of
stdout/stderr lines + spinner/status footer).
- Add streamCommandInInitPanel() helper that spawns a child process
with piped stdio and feeds lines into the streaming panel.
- After createKeyInternal succeeds, stream `cap sync` (no platform —
syncs all native projects) so the public key is actually bundled
before we claim encryption is enabled.
- 3.5s dwell after sync completes so the user can read the result.
- On sync success → green "Encryption ENABLED" summary.
- On sync failure → yellow "pending-sync" fallback; step 7 build+sync
still gets a chance to promote via promoteEncryptionSummaryToEnabled.
- Color-code streaming lines by prefix: green for success markers,
red for errors, yellow for warnings, cyan for info, blue for
[capacitor] lines, dim gray for indented file paths.
- Remove the "debugging update failures is harder" bullet from the
enabled summary — it was pointless.
- Bump post-sync dwell from 1.5s to 3.5s.
* chore: remove dev-only remove-encryption-key script from PR
Keep it locally but out of the tracked tree — it is a personal
test helper, not something that should ship.
* chore: remove dev-only scripts from PR
Keep them locally but out of the tracked tree — they are personal
test helpers, not something that should ship.
* fix(init): avoid global Buffer reference to satisfy lint rule
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
capgo init, functions likefindProjectType()andcheckAlerts()wrote directly to stdout via@clack/promptswhile the Ink terminal UI had control, causing garbled rendering (broken header banner with ghost border box)checkAlertsintocheckVersionStatus()(pure data, no UI) andcheckAlerts()(clack UI wrapper) — init uses the former and renders an<Alert variant="warning">via@inkjs/uifor outdated CLI versionsfindProjectType()now accepts{ quiet: true }to suppress clack logging when called during initloginInternal()skips all clack output when called withsilent=truefrom initTest plan
node dist/index.js i— header banner should render cleanly without ghost border boxnode dist/index.js iwith an outdated CLI version — should show an Ink warning alert below the headernode dist/index.js login— should still show clack prompts/alerts as beforeupload,channel list) —checkAlertsstill works via clackSummary by CodeRabbit