This repository was archived by the owner on May 1, 2026. It is now read-only.
fix(init): silence stray clack writes during channel add step#579
Merged
Conversation
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.
|
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 (2)
📝 WalkthroughWalkthroughThis PR adds an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
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
When picking "Use production" in the channel-add step of
capgo init, the Ink onboarding UI broke and the "🚀 Capgo OTA Onboarding" header was repainted twice. The root cause was the same class of bug fixed in #560: helpers transitively called fromaddChannelInternal(silent: true)were still writing directly toprocess.stdoutvia@clack/prompts, bypassing Ink and corrupting the frame.This PR continues the whack-a-mole pattern from #560 — no interceptor, no monkey-patching
process.stdout— by threading an optionalsilentflag through the offending helpers and gating every direct clack call site behind it.Changes
src/utils.tsgetConfig,getLocalConfig,getRemoteConfig,createSupabaseClientnow accept an optionalsilentand propagate it through the call chain.sendEventis fire-and-forget telemetry — it now always loads remote config silently and only emits the LogSnag failure log whenverboseis set, so it can never bypass an Ink-controlled stdout (e.g. viamarkStep→markSnag→sendEventduring init).src/channel/add.tsaddChannelInternalnow forwards its existingsilentflag togetConfig,createSupabaseClient, andfindUnknownVersion(the latter already supportedsilentbut was being called without it).All existing call sites continue to work — the new parameters default to
false.Test plan
npm run typecheck(tsc --noEmit) — cleannpx eslint src/utils.ts src/channel/add.ts— cleannpm run build(tsc && bun build.mjs) — built CLI and SDK successfullycapgo initend-to-end on a fresh Capacitor project, pick "Use production" at the channel step, and confirm the Ink frame stays intact (no duplicated header) all the way through to step 3.capgo channel add <name> <appId> --defaultoutside of init and confirm normal (non-silent) clack output is unchanged.Summary by CodeRabbit
New Features
Bug Fixes