feat(cli): build init onboarding TUI — wizard, AI build-debug, adaptive sizing, robust log viewer#2376
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUnified onboarding shell and platform picker; caller-handled AI analysis with captured logs; frame-fit and min-size gates; build-log sanitization; many presentational onboarding steps; schema/telemetry updates; and extensive VT/Ink tests and helpers. ChangesUnified Onboarding, AI Flow, and Frame-fit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b95324554
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 23
🤖 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/package.json`:
- Line 97: The package.json "test" script currently chains 40+ commands into one
long line; split it into smaller, named scripts and/or a top-level orchestrator
to improve readability and failure traceability: create focused scripts like
"test:unit", "test:functional", "test:ci" (or groupings matching your test
categories referenced in the current script names such as "test:bundle",
"test:semver", "test:credentials", etc.), add a single coordinating script
"test:all" (or "test") that runs these group scripts sequentially or in parallel
via a small shell script or a task runner (e.g., npm-run-all) and replace the
long inline value of the "test" property with a call to the orchestrator (e.g.,
"bun run test:all"); update any CI configs to call the new script names as
needed so failures show which group failed.
- Around line 50-53: The package.json engines field was raised to "node":
">=22.0.0" which is a breaking change; either revert the engines bump to
">=20.0.0" to preserve compatibility or document and justify the Node 22
requirement: add a brief note in the release notes/CHANGELOG and README
explaining why Node 22 is required (e.g., specific dependency or build tool
needing Node 22), update CI workflows to enforce node 22 in matrix/runner
settings, and if Node-22-only APIs are used, annotate the exact uses (for
example check cli/src/build/onboarding/macos-signing.ts for import.meta.url or
any node:* module usage) so reviewers can verify the rationale. Ensure the
chosen approach updates package.json engines, CI matrix, and release docs
consistently.
In `@cli/src/build/onboarding/android/ui/app.tsx`:
- Around line 22-24: Remove the unused Select import and collapse split import
statements so imports are not fragmented (fix the import/first lint error);
specifically, delete the unused Select symbol from the '`@inkjs/ui`' import and
merge the separate import that brings in MAX_AI_RETRIES with the other
top-of-file imports (ensure MAX_AI_RETRIES is imported from its original module
in the same combined import block), keeping imports grouped and ordered to
satisfy the linter.
- Around line 1729-1734: Move the early returns for active takeover views before
the terminal size guard so streaming/debug views aren't blocked: check if step
=== 'requesting-build' (return FullscreenBuildOutput with title, buildOutput,
terminalRows) and if step === 'ai-analysis-result-scroll' (return the fullscreen
AI result component used) before calling terminalFitsOnboarding(terminalCols,
terminalRows, 'android') and returning TerminalTooSmallPrompt; update both
places where the min-size gate is used so these step checks take precedence.
In `@cli/src/build/onboarding/build-log.ts`:
- Around line 30-38: Replace the problematic regex ranges and control-character
escapes and stop using trailing .at(-1) style access: update CSI_RE to use an
explicit hex range for the intermediate bytes (e.g.
/\x1B\[[0-9;?]*[\x20-\x2F]*[`@-`~]/g) and update ESC_RE to avoid the obscure
ASCII-range syntax (e.g. /\x1B[`@-Z`\\-_]/ becomes /\x1B(?:[@\x41-\x5A\\\-_])/ or
the equivalent explicit set), change CTRL_RE to use Unicode escapes for control
ranges (e.g. /[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F]/g) to satisfy
control-character-escape, and replace any trailing-element access (the split
result last-element check that used .at(-1) or similar) with a non-.at approach
such as accessing by index (arr[arr.length-1]) or using slice(-1)[0] to satisfy
e18e/prefer-array-at; ensure you update the constants CSI_RE, ESC_RE, CTRL_RE
and the last-element check in the same module.
In `@cli/src/build/onboarding/ui/shell.tsx`:
- Around line 23-31: The imports in shell.tsx are out of the project's
import-sort order causing perfectionist/sort-imports failures; reorder the
import statements to follow the configured groups and alphabetical order (e.g.
external modules first, then internal modules by folder/name) so that symbols
like loadAndroidProgress, AndroidOnboardingApp, loadProgress, OnboardingApp,
PICKER_MIN_COLS/PICKER_MIN_ROWS/terminalFitsPicker, Header, pickPlatformLayout,
TerminalTooSmallPrompt, and PlatformPicker appear in the correct sorted order
per ESLint rules; update the import list accordingly and run the linter to
confirm the perfectionist rule is satisfied.
In `@cli/src/build/onboarding/ui/steps/android-ci.tsx`:
- Line 26: Remove the unused import CiSecretTarget from the top import list and
fix the lint errors for the unused "dense" parameters by either deleting the
unused "dense" param from the function/component signatures or renaming them to
a prefixed unused name (e.g., _dense) so ESLint recognizes them as intentionally
unused; apply this change to all occurrences in this module (the onboarding
android-ci step functions/components that declare a dense param). Ensure type
imports still used (e.g., CiSecretSetupAdvice) remain.
In `@cli/src/build/onboarding/ui/steps/android-keystore.tsx`:
- Line 72: The prop "dense" is declared but unused in the KeystoreExplainerStep
and KeystoreExistingAliasSelectStep components which triggers lint errors and
prevents dense-mode styling; fix by either removing "dense" from the component
props if not needed, or apply it to the rendered markup/child components (e.g.,
pass dense down or conditionally set compact classNames/spacing) inside
KeystoreExplainerStep and KeystoreExistingAliasSelectStep so the prop is
actually consumed (reference the exported component identifiers
KeystoreExplainerStep and KeystoreExistingAliasSelectStep and the "dense" prop
in your changes).
In `@cli/src/build/onboarding/ui/steps/android-sa-gcp.tsx`:
- Line 169: The component SaJsonValidationFailedStep currently declares unused
props (dense, possibly detectedCount) causing ESLint errors; update the
component signature and similar step renderers (e.g., other steps around the
same module) to either use the props (implement the dense rendering branch in
SaJsonValidationFailedStep and any step using detectedCount) or mark them as
intentionally unused by renaming in the destructuring to _dense and
_detectedCount (or remove them entirely) so the linter no longer reports unused
bindings; ensure you update the prop names inside the function (or remove
references) and keep prop types/interfaces consistent
(SaJsonValidationFailedStepProps) when making the changes.
In `@cli/src/build/onboarding/ui/steps/ios-ci.tsx`:
- Line 89: The three React components CiSecretsSetupStep,
ConfirmCiSecretOverwriteStep, and CiSecretsFailedStep declare a dense prop but
never use it, causing lint errors; fix by either removing dense from their prop
types and signatures or wiring it into their rendering (e.g., apply a "compact"
CSS/className or conditional spacing when dense=true) so the prop is consumed;
locate the components (CiSecretsSetupStep, ConfirmCiSecretOverwriteStep,
CiSecretsFailedStep) and either remove the dense parameter from the function
signature and associated prop interface, or reference dense in the returned JSX
to adjust layout/styles accordingly.
In `@cli/src/build/onboarding/ui/steps/ios-import.tsx`:
- Around line 224-237: ImportNoMatchRecoveryStep declares a dense prop but never
uses it, causing lint errors and skipping the compact rendering; update
ImportNoMatchRecoveryStep (and the two sibling step components that also declare
dense) to conditionally render a compact layout when dense === true: for
example, reduce spacing (use marginTop={0} or smaller), remove Newline
components, use smaller Text styling or dimColor only, and pass density/compact
flags into Select if available, otherwise render a tighter Select wrapper;
ensure you reference the existing props (ImportNoMatchRecoveryStepProps,
identityName, options, onChange) and implement the same dense-branch pattern in
the other two components so the prop is either used to produce the compact UI or
remove the dense prop if compact behavior is unnecessary.
In `@cli/src/build/onboarding/ui/steps/ios-shared.tsx`:
- Line 390: The props object for ErrorStep and BuildCompleteStep declares a
unused "dense" prop which triggers ESLint; remove "dense" from the destructuring
in the component signatures (or rename it to "_dense" if you want to keep it to
acknowledge intentionally unused) so the declared prop is no longer unused;
update the component declarations for ErrorStep and BuildCompleteStep to stop
destructuring "dense" (or rename to "_dense") and ensure any usages/prop types
are adjusted accordingly.
In `@cli/src/build/request.ts`:
- Around line 2003-2009: The duplicated log path construction should be replaced
by a single call to the shared helper: use getLogCapturePath(capturedJobId)
(ensure getLogCapturePath is imported) instead of manually building
`${process.env.CAPGO_AI_LOG_BASE_DIR ||
'/tmp/capgo-builds'}/${capturedJobId}.log`; update the assignment to logsPath
and aiAnalysisInfo.capturedLogPath to use that helper (references: logsPath,
aiAnalysisInfo, capturedJobId, getLogCapturePath).
In `@cli/test/helpers/onboarding-fixtures.mjs`:
- Around line 26-33: Reorder the import statements (ks, sa, ash, aci, cred, imp,
ish, cic) into the linter-required sorted order (alphabetical by module path or
alias per perfectionist/sort-imports) and remove any bare top-level function
declaration by converting it into a module-scoped constant arrow function or
exported function (e.g., replace "function foo() {…}" with "const foo = () =>
{…}" or export it) so the antfu/top-level-function rule is satisfied; update any
call sites in the file to use the new identifier form.
In `@cli/test/helpers/vt-grid.mjs`:
- Around line 110-113: The code must actually send the alternate-screen enter
sequence when alt is true; change the conditional so that when alt is truthy you
call writeAndSettle(term, '\x1b[?1049h\x1b[H') (use the proper ESC bytes \x1b
rather than literal escape glyphs), ensuring writeAndSettle, term and frame
usage remains intact (wrap the if body in braces if needed) so the terminal
enters the alternate buffer before writing the CRLF-normalized frame via
writeAndSettle(term, frame.replace(/\n/g, '\r\n')).
In `@cli/test/test-ai-fit.mjs`:
- Line 47: Update the hex escape sequences to use uppercase (change occurrences
like the styled string `styled` and the other hex-escape at line ~75 from `\x1b`
to `\x1B`) and replace direct use of the global process with the Node module
import: add `import process from "node:process"` at the top of the test and
change the `process.exit(1)` call (and any other global process usages) to use
that imported `process`. Ensure both hex escapes and all process references
(including the exit call) are updated.
In `@cli/test/test-frame-fit-android-shared.mjs`:
- Line 47: Replace the top-level arrow function declaration "const noop = () =>
{}" with a function declaration "function noop() {}" to satisfy
antfu/top-level-function; also import the process global by adding "import
process from 'node:process'" and update any direct global calls to
process.exit(...) to use the imported process (i.e., call process.exit(...)) so
the node/prefer-global/process rule is satisfied; ensure references to noop and
process.exit use the updated identifiers.
In `@cli/test/test-frame-fit-ios-shared.mjs`:
- Line 34: Replace the arrow `const noop = () => {}` with a top-level function
declaration `function noop() {}` to satisfy the antfu/top-level-function rule,
and add an explicit import for process from node (e.g., `import process from
"node:process";`) so usages of `process.exit(...)` use the imported symbol and
satisfy node/prefer-global/process; update any calls to `process.exit` to use
the imported `process` if necessary.
In `@cli/test/test-frame-fit-log.mjs`:
- Around line 103-106: The test calls frame(longLog, rows, 80, 6) with an extra
argument so 80 is being used as bodyHeight; update the call to use the correct
3-argument signature by removing the spurious rows argument—i.e. call
frame(longLog, 80, 6) inside frameRows(..., 80) so the uncapped calculation uses
the intended parameters (references: test 'UNCAPPED long log overflows — proving
the cap is what prevents too-small', functions frameRows and frame, variables
longLog and rows).
In `@cli/test/test-frame-fit-resize.mjs`:
- Around line 35-40: Split the one-line try/catch and single-line helper
declarations into multi-line statements and convert the top-level function
declaration to a top-level const arrow function, and prefer the global process
binding; specifically, expand the try block so await fn(); passed++; and
console.log are each on their own lines and expand the catch so failed++; and
console.error are on their own lines; replace function assert(cond, msg) { ... }
with const assert = (cond, msg) => { if (!cond) throw new Error(msg) }; ensure
top-level helpers like assert, h (React.createElement) and noop are declared as
consts (already const for h/noop) and, if any code uses a Process import or
non-global process reference, switch usages to globalThis.process (or use the
global process) to satisfy node/prefer-global/process and antfu
top-level/statement style rules.
In `@cli/test/test-frame-fit-viewer.mjs`:
- Around line 21-24: The compact one-line try/catch block and the one-line
assert function violate style rules and direct use of process triggers
node/prefer-global/process; expand the single-line statements into proper
multi-line blocks (break the try { fn(); passed++; console.log(...) } catch
(error) { ... } into a multi-line try { ... } catch (error) { ... } and rewrite
function assert(cond, msg) { if (!cond) throw new Error(msg) } as a multi-line
function with the if on its own line), and replace any direct use of process
with the preferred global (e.g., globalThis.process) where the file references
process (including the other occurrences noted around the file) so all instances
comply with antfu/if-newline, style/max-statements-per-line, and
node/prefer-global/process.
In `@cli/test/test-min-size-gate.mjs`:
- Around line 7-10: Reorder the import statements so built-in module imports
come before external/third-party imports to satisfy the
perfectionist/sort-imports rule: move the "import process from 'node:process'"
line so it appears before the React and Ink imports (Box, Text, React) while
keeping the MIN_COLS/MIN_ROWS import where it belongs; ensure the resulting
order places node:process first, then external imports, then local relative
imports.
In `@cli/tsconfig.json`:
- Line 7: Update the tsconfig to silence the TS5101 deprecation warning by
changing "ignoreDeprecations" from "5.0" to "6.0" and add an inline comment
documenting that compilerOptions.baseUrl is retained temporarily for module
resolution/legacy build tooling (reference the keys ignoreDeprecations and
compilerOptions.baseUrl and the TS5101 diagnostic); alternatively, if you prefer
migration, replace baseUrl with the advised configuration and remove the
suppression, but the immediate fix is to bump ignoreDeprecations to "6.0" and
add the explanatory comment next to baseUrl.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 32e2cf6e-5d26-4d6c-8921-ea31db803f70
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (52)
cli/package.jsoncli/src/ai/analyze.tscli/src/ai/telemetry.tscli/src/build/onboarding/ai-fit.tscli/src/build/onboarding/android/types.tscli/src/build/onboarding/android/ui/app.tsxcli/src/build/onboarding/build-log.tscli/src/build/onboarding/command.tscli/src/build/onboarding/min-terminal-size.tscli/src/build/onboarding/types.tscli/src/build/onboarding/ui/app.tsxcli/src/build/onboarding/ui/completed-steps-log.tsxcli/src/build/onboarding/ui/components.tsxcli/src/build/onboarding/ui/frame-fit.tscli/src/build/onboarding/ui/min-size-gate.tsxcli/src/build/onboarding/ui/platform-picker.tsxcli/src/build/onboarding/ui/shell.tsxcli/src/build/onboarding/ui/steps/android-ci.tsxcli/src/build/onboarding/ui/steps/android-keystore.tsxcli/src/build/onboarding/ui/steps/android-sa-gcp.tsxcli/src/build/onboarding/ui/steps/android-shared.tsxcli/src/build/onboarding/ui/steps/ios-ci.tsxcli/src/build/onboarding/ui/steps/ios-credentials.tsxcli/src/build/onboarding/ui/steps/ios-import.tsxcli/src/build/onboarding/ui/steps/ios-shared.tsxcli/src/build/request.tscli/src/schemas/build.tscli/test/find-min-onboarding-size.mjscli/test/helpers/frame-fit.mjscli/test/helpers/onboarding-fixtures.mjscli/test/helpers/onboarding-frame.mjscli/test/helpers/size-search.mjscli/test/helpers/vt-grid.mjscli/test/run-frame-fit.mjscli/test/test-ai-fit.mjscli/test/test-ai-onboarding-mode.mjscli/test/test-build-log-sanitize.mjscli/test/test-build-output-viewport.mjscli/test/test-frame-fit-ai.mjscli/test/test-frame-fit-android-shared.mjscli/test/test-frame-fit-build.mjscli/test/test-frame-fit-decision.mjscli/test/test-frame-fit-ios-shared.mjscli/test/test-frame-fit-log.mjscli/test/test-frame-fit-platform.mjscli/test/test-frame-fit-resize.mjscli/test/test-frame-fit-viewer.mjscli/test/test-min-size-gate.mjscli/test/test-onboarding-min-size.mjscli/test/test-platform-layout.mjscli/test/test-shell-size-gate.mjscli/tsconfig.json
Resolves the squash of claude/build-init-onboarding-tui onto current main. The branch rewrote the build-init onboarding apps after forking, while main shipped two features in the same files: • #2315 — GitHub Actions workflow generation + CAPGO_TOKEN auto-push • #2327 — Android service-account onboarding tracking Both features are RE-PORTED into the rewritten state machine (not dropped): workflow step-flow (3-option GitHub prompt → build-script picker → writing-workflow-file / confirm-overwrite, .env export fallback), the diff/secrets viewers, and the Android SA-tracking events — all coexisting with the rewrite's per-platform min size, scrollable iOS error viewer, build-log sanitize, one-row-per-line build viewer, and the OnboardingShell picker. Conflicts resolved: • tsconfig.json — keep ignoreDeprecations (bumped 5.0→6.0 for TS6) + baseUrl • package.json — main's newer versions + @xterm/headless + unioned test scripts • command.ts — OnboardingShell render path • ui/components.tsx — union: AiViewer/BuildOutput + DiffViewer/SecretsTable • ui/app.tsx, android/ui/app.tsx — feature re-port (see above) • bun.lock — regenerated Verified: build green (tsc), both feature test sets pass (workflow-generator 13, diff-utils 8, ci-secrets 17, android-onboarding-progress 11, onboarding-telemetry) AND the rewrite suites (min-size 50, frame-fit 9/9, build-log-sanitize 12, viewport 4, gates 9+7, platform-layout 11, ai-fit 31, ai-onboarding-mode 9). Pre-existing lint debt (unused `dense` params / dead size-gate symbols from the rewrite) is cleaned in a follow-up commit.
The onboarding rewrite dropped the adaptive `dense` flag (forced dense=false) but left unused `dense` parameters across the step components, plus a few dead size-gate imports/vars (TerminalTooSmall, shouldCollapseToDense, isFrameTooSmall, tooSmall, neededRows, COMPACT_HEADER_TOTAL_ROWS) in ui/app.tsx. Remove them so `cli:lint` passes. The `dense?` prop is kept on the Props interfaces (call sites still pass it as a named attr); only the unused destructured param is removed. No behavior change. Build + all onboarding/workflow/tracking test suites green.
1b95324 to
fe14f37
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Merging this PR will not alter performance
Comparing Footnotes
|
- Rename the `aci` import alias (android-ci) to `androidCi` in onboarding-fixtures — `aci` tripped the typos check (`aci`→`acpi`), which failed both "Check for typos (CLI)" and "Run tests". Also alphabetised the step imports (perfectionist). - Remove dead exports knip flagged (leftover dense-flag / frame-budget machinery the rewrite abandoned): totalRenderedRows, isFrameTooSmall, shouldCollapseToDense, COMPACT_HEADER_TOTAL_ROWS, the unused ios-shared SelectOption, and the CheckingCiSecretsStep/UploadingCiSecretsStep spinner steps (superseded by dynamic phase text). Move the test-only BODY_BUDGET_ROWS/MAX_FRAME_ROWS constants into the frame-fit test helper, and delete the orphaned test-frame-fit-decision.mjs (it only exercised the deleted dense logic). De-alias MIN_ROWS to clear knip's duplicate-export. Build green, cli:lint clean, knip 0, and all onboarding/workflow/tracking suites pass.
- android/ui/app.tsx: move the requesting-build + AI-scroll fullscreen takeovers BEFORE the min-size gate (matching the iOS app). Previously a mid-build/mid-AI terminal shrink hid the live streaming view behind the resize prompt. The gate now runs after the takeovers, still guarding the normal step body. (CodeRabbit) - test-frame-fit-log.mjs: fix frame(longLog, rows, 80, 6) → frame(longLog, rows, 6); frame() takes (logEntries, rows, bodyHeight), so 80 was wrongly used as bodyHeight and 6 ignored. Restores the intended uncapped-overflow assertion. (CodeRabbit)
… to >=22) The `>=22.0.0` floor was introduced inadvertently while merging package.json versions during the rebase — main requires `>=20.0.0` and no Node-22-only API is used in cli/src. Revert to `>=20.0.0` to avoid an unintended breaking change for downstream consumers. (Caught by CodeRabbit on PR #2376.)
…rable summary, fix version
Addresses three review findings on the build-init onboarding PR:
- [P1] False success on cancel/error: command.ts printed "✔ Capgo onboarding
complete" after waitUntilExit() on EVERY exit — including missing-platform
(Android exits after 2s) and user-cancel paths. Add an OnboardingResult contract:
the shell defaults to { outcome: 'cancelled' }, and each app reports
{ outcome: 'completed', summary } ONLY when it reaches build-complete. command.ts
prints the ✔ line only on 'completed'; otherwise a neutral "exited — not completed"
line. No false success.
- [P2] Final actionable output lost to the alt-screen teardown: the build URL,
workflow path, .env path and CI-secret summary rendered by BuildCompleteStep were
erased when Ink restored the primary buffer on exit. The completion summary is now
carried out through onResult and reprinted to the PRIMARY buffer by command.ts, so
the build URL + generated file paths survive.
- [P2] Version regression: cli/package.json was 7.119.0 (fork-point) vs base 7.119.1.
Bumped to 7.119.1 (+ lockfile) to avoid a publish/release-automation regression.
Build green, cli:lint clean, knip 0, all onboarding suites pass.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/package.json`:
- Around line 108-119: The package.json aggregate "test" script is missing
entries for the new tests; add two scripts "test:build-log-sanitize": "bun
test/test-build-log-sanitize.mjs" and "test:build-output-viewport": "bun
test/test-build-output-viewport.mjs" to cli/package.json and insert them into
the main "test" chain (the long "test" value) so they run alongside existing
entries like "test:build-platform-selection" and "test:frame-fit"; update the
"scripts" block in cli/package.json accordingly so both new tests are invoked by
the aggregate test command.
In `@cli/src/build/onboarding/ui/min-size-gate.tsx`:
- Around line 87-91: The doc comment for MinSizeGate is out of sync with how the
shell actually gates the picker: shell.tsx uses terminalFitsPicker(...) and
renders TerminalTooSmallPrompt with
minCols={PICKER_MIN_COLS}/minRows={PICKER_MIN_ROWS} and does not wrap the picker
in MinSizeGate, so MinSizeGate (which uses terminalFitsOnboarding(...)) does not
block the platform picker. Fix by either updating the MinSizeGate comment to
state it gates only post-picker onboarding (referencing MinSizeGate,
terminalFitsOnboarding, and TerminalTooSmallPrompt) or wire MinSizeGate into the
shell picker path: replace the shell’s terminalFitsPicker(...) check with
rendering the picker inside <MinSizeGate cols={cols}
rows={rows}>...</MinSizeGate> (or have MinSizeGate accept picker min sizes via
PICKER_MIN_COLS/PICKER_MIN_ROWS) so the picker is properly blocked.
In `@cli/src/build/onboarding/ui/shell.tsx`:
- Around line 99-102: The choose callback currently calls loadReady(platform,
appId) without a rejection handler which can produce unhandled promise
rejections from loadProgress / loadAndroidProgress; update choose (the function
that calls loadReady) to attach a .catch that routes failures into the
onboarding error UX instead of letting the promise reject unhandled — for
example, log/report the error and call the existing onResult?.({ outcome:
'cancelled' }) (do not pass a non‑OnboardingResult payload). Keep
onResolvePlatform behavior intact and ensure errors from loadReady are surfaced
rather than ignored.
In `@cli/src/build/onboarding/ui/steps/ios-credentials.tsx`:
- Around line 120-224: The comment for ApiKeyInstructionsStep describes a dense
mode that is not implemented: when dense is true the UI should collapse to three
terser lines (merge steps, fold the Ctrl+O hint into step 3, remove extra
Newline/standalone hint/Divider spacing), but the code always renders four
numbered steps, the standalone Ctrl+O hint box, and most Newlines; update the
implementation to conditionally render the compact layout when the dense prop is
true (use the dense prop inside ApiKeyInstructionsStep to: 1) render only three
Text lines for the steps (combine step 3 + Ctrl+O hint), 2) skip the standalone
Ctrl+O Box and some Newline/Divider nodes, and 3) keep the control rendering
identical—alternatively, if you prefer not to implement the dense behavior,
update the top comment to reflect the current, non-dense rendering so the
docstring matches the component.
In `@cli/src/build/onboarding/ui/steps/ios-shared.tsx`:
- Around line 458-577: BuildCompleteStep promises a "dense" mode but never uses
the dense prop; destructure dense from the component props in BuildCompleteStep
and implement it by setting the container Box's paddingY to dense ? 0 : 1 and by
conditionally rendering the Newline components (render Newline only when !dense)
around detail, ciSecretUploadSummary, workflowWrittenPath, envExportPath,
envExportError and after the Box so the UI actually drops vertical spacing in
dense mode; keep the existing runHint/detail rendering logic unchanged so
content is identical but with reduced vertical padding when dense is true.
In `@cli/test/helpers/size-search.mjs`:
- Around line 8-15: The header incorrectly states a "binary-search" for minimal
COLS but the implementation uses a linear upward scan in functions minColsAt
(and similarly minRowsAt); update the comment block to reflect a linear scan
approach (or "incremental/linear search") for step 1 and any related wording in
steps 2–3 so the documentation matches the behavior of minColsAt and minRowsAt
and avoids misleading future maintainers.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: ce0b92a0-3e48-43bc-8206-6fdb60b852ef
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
cli/package.jsoncli/src/ai/analyze.tscli/src/ai/telemetry.tscli/src/build/onboarding/ai-fit.tscli/src/build/onboarding/android/types.tscli/src/build/onboarding/android/ui/app.tsxcli/src/build/onboarding/build-log.tscli/src/build/onboarding/command.tscli/src/build/onboarding/min-terminal-size.tscli/src/build/onboarding/types.tscli/src/build/onboarding/ui/app.tsxcli/src/build/onboarding/ui/completed-steps-log.tsxcli/src/build/onboarding/ui/components.tsxcli/src/build/onboarding/ui/frame-fit.tscli/src/build/onboarding/ui/min-size-gate.tsxcli/src/build/onboarding/ui/platform-picker.tsxcli/src/build/onboarding/ui/shell.tsxcli/src/build/onboarding/ui/steps/android-ci.tsxcli/src/build/onboarding/ui/steps/android-keystore.tsxcli/src/build/onboarding/ui/steps/android-sa-gcp.tsxcli/src/build/onboarding/ui/steps/android-shared.tsxcli/src/build/onboarding/ui/steps/ios-ci.tsxcli/src/build/onboarding/ui/steps/ios-credentials.tsxcli/src/build/onboarding/ui/steps/ios-import.tsxcli/src/build/onboarding/ui/steps/ios-shared.tsxcli/src/build/request.tscli/src/schemas/build.tscli/test/find-min-onboarding-size.mjscli/test/helpers/frame-fit.mjscli/test/helpers/onboarding-fixtures.mjscli/test/helpers/onboarding-frame.mjscli/test/helpers/size-search.mjscli/test/helpers/vt-grid.mjscli/test/run-frame-fit.mjscli/test/test-ai-fit.mjscli/test/test-ai-onboarding-mode.mjscli/test/test-build-log-sanitize.mjscli/test/test-build-output-viewport.mjscli/test/test-frame-fit-ai.mjscli/test/test-frame-fit-android-shared.mjscli/test/test-frame-fit-build.mjscli/test/test-frame-fit-ios-shared.mjscli/test/test-frame-fit-log.mjscli/test/test-frame-fit-platform.mjscli/test/test-frame-fit-resize.mjscli/test/test-frame-fit-viewer.mjscli/test/test-min-size-gate.mjscli/test/test-onboarding-min-size.mjscli/test/test-platform-layout.mjscli/test/test-shell-size-gate.mjscli/tsconfig.json
💤 Files with no reviewable changes (1)
- cli/src/build/onboarding/android/ui/app.tsx
# Conflicts: # cli/package.json
…ady rejection, fix stale comments
- package.json: wire test:build-log-sanitize + test:build-output-viewport into the
aggregate `test` chain. These regression guards existed but no runner executed
them (run-frame-fit only globs test-frame-fit-*). (CodeRabbit)
- shell.tsx: choose() now .catch()es loadReady — loadProgress throws on corrupt
saved-progress JSON, which previously produced an unhandled rejection + a stuck
picker. On failure it shows an error frame, reports { outcome: 'cancelled' }, and
exits. (CodeRabbit)
- Doc-only: correct stale comments that described the dropped adaptive `dense`
behavior (ios-credentials ApiKeyInstructionsStep, ios-shared BuildCompleteStep),
the MinSizeGate picker-gating note (the shell gates the picker via
terminalFitsPicker, not MinSizeGate), and "binary-search" → "linear scan" in
size-search.mjs. No behavior change in these four.
Build green, cli:lint clean, knip 0, onboarding suites pass.
…ng line ends
Fastlane indents log lines with literal tabs. The build viewer renders each line
in a single <Text wrap="truncate-end"> that truncates by CHARACTER count, but a
terminal renders a tab as 1–8 columns (advance to the next multiple of 8). So Ink
budgeted a tab as 1 column while the terminal drew up to 8 — the line overflowed
the width and the terminal clipped the tail, dropping the last visible char of
every tab-indented line ("* App" → "* Ap", "* Release" → "* Releas"). Flush-left
lines were unaffected, which is the tell.
sanitizeBuildLogLines now expands tabs to spaces (per-column, to the next tab
stop — not a blind 8-space swap) after stripping ANSI and before truncation, so
the character count matches the rendered width. Regression guards added:
test-build-log-sanitize (tab→spaces, no literal tab survives, mid-line stop math)
and test-build-output-viewport (tab-indented lines render full text through the
real sanitize→VT path). Reproduced the bug + verified the fix with @xterm/headless.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ui/steps/ios-credentials.tsx`:
- Around line 125-131: The comment claims the `dense` prop "no longer alters the
layout" but the JSX still uses `{!dense && <Newline />}` in the
`ios-credentials` step component, so either remove that conditional newline to
make `dense` truly no-op or update the top comment to reflect that `dense` still
controls one spacing Newline; locate the `dense` prop usage and the `<Newline
/>` conditional in the component (search for `{!dense && <Newline />}` and the
`dense` prop definition/typing) and either delete the conditional rendering of
`<Newline />` or edit the comment text to state that `dense` still controls a
single conditional newline spacing.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 5f7d52c7-a822-4deb-b6ee-4afdef02a2a0
📒 Files selected for processing (9)
cli/package.jsoncli/src/build/onboarding/build-log.tscli/src/build/onboarding/ui/min-size-gate.tsxcli/src/build/onboarding/ui/shell.tsxcli/src/build/onboarding/ui/steps/ios-credentials.tsxcli/src/build/onboarding/ui/steps/ios-shared.tsxcli/test/helpers/size-search.mjscli/test/test-build-log-sanitize.mjscli/test/test-build-output-viewport.mjs
…esses one Newline
My prior comment fix over-claimed: it said `dense` "no longer alters the layout",
but ApiKeyInstructionsStep still uses `{!dense && <Newline />}` to drop the spacer
between the picker prompt and its Select. Comment now states `dense` only
suppresses that single Newline (the multi-step copy is unconditional). (CodeRabbit)
…main) main deliberately hides the underlying model name — "Capgo AI", not "Capgo AI (Kimi K2.5)" (see request.ts on main, #2362). The onboarding AI-debug steps reintroduced it in 4 user-facing strings (ios-shared + android-shared: the "analyze the build log…" prompt + the running spinner). Revert to plain "Capgo AI" to match. (The internal ai_analyze.ts code comment that mentions Kimi is unchanged — it's identical to main and not user-facing.)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/build/onboarding/ui/steps/ios-shared.tsx (1)
256-287:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
ErrorStepdocuments adenseform that isn't implemented.The block comment (lines 256-271) and the file header (lines 30-38) describe a "Dense form" for
errorthat clamps the message, caps advice lists with "… +N more", and drops docs. ButErrorStep(line 382) never destructures or usesdense— the only adaptive path here iscollapsed(routing to the scroll viewer). Thedense?prop at line 280 is inert.Same doc-only pattern you resolved earlier for
BuildCompleteStep/ApiKeyInstructionsStep: update these comments to describe thecollapsed-vs-comfortable behavior (and notedenseis accepted for call-site compatibility only), or drop the unuseddensefromErrorStepProps.🤖 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/steps/ios-shared.tsx` around lines 256 - 287, The prop `dense` on ErrorStepProps is declared but unused in ErrorStep (only `collapsed` is used), so either remove `dense` from ErrorStepProps and any call-sites, or update the surrounding comments to reflect the actual behavior (describe collapsed vs comfortable rendering and note that `dense` is accepted only for call-site compatibility). Locate ErrorStep and ErrorStepProps in ios-shared.tsx (symbols: ErrorStep, ErrorStepProps, collapsed, dense) and make a single consistent change: either delete the `dense?: boolean` line and remove usages, or edit the header/block comment (and optionally mention BuildCompleteStep/ApiKeyInstructionsStep) to remove the "Dense form" description and explain that `collapsed` is the adaptive mode; keep call-sites compiling if you choose to keep a noop `dense` by adding a comment saying it is accepted for compatibility.
🤖 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.
Outside diff comments:
In `@cli/src/build/onboarding/ui/steps/ios-shared.tsx`:
- Around line 256-287: The prop `dense` on ErrorStepProps is declared but unused
in ErrorStep (only `collapsed` is used), so either remove `dense` from
ErrorStepProps and any call-sites, or update the surrounding comments to reflect
the actual behavior (describe collapsed vs comfortable rendering and note that
`dense` is accepted only for call-site compatibility). Locate ErrorStep and
ErrorStepProps in ios-shared.tsx (symbols: ErrorStep, ErrorStepProps, collapsed,
dense) and make a single consistent change: either delete the `dense?: boolean`
line and remove usages, or edit the header/block comment (and optionally mention
BuildCompleteStep/ApiKeyInstructionsStep) to remove the "Dense form" description
and explain that `collapsed` is the adaptive mode; keep call-sites compiling if
you choose to keep a noop `dense` by adding a comment saying it is accepted for
compatibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19ad811f-2852-40f4-a581-17d642ae75eb
📒 Files selected for processing (3)
cli/src/build/onboarding/ui/steps/android-shared.tsxcli/src/build/onboarding/ui/steps/ios-credentials.tsxcli/src/build/onboarding/ui/steps/ios-shared.tsx
When you pick the .p8 (named AuthKey_<KEYID>.p8), the Key ID is extracted and pre-filled — but it was only ever PERSISTED when you manually submitted the Key ID step. Quitting after selecting the file but before confirming saved just p8Path, so on resume `keyId` initialized empty and the input showed the generic "ABC123DEF" placeholder instead of the detected id. Two-part fix: - Persist the extracted keyId alongside p8Path at all three .p8-selection sites (file picker + manual path, x2), so a normal resume restores it. - On resume, if keyId wasn't saved, re-derive it from the saved p8Path filename. This also recovers progress files written by the old code. extractKeyIdFromP8Path moved to progress.ts (pure, co-located with the other resume helpers) + exported, with regression tests in test-onboarding-progress (AuthKey_/ApiKey_ prefixes, case-insensitivity, non-matching/renamed files, end-of-path anchoring). Build green, cli:lint clean, knip 0, onboarding suites pass.
…llapsed) Third instance of the same stale-comment family CodeRabbit has been catching: ErrorStep's block comment and the file header described a `dense` collapse form that doesn't exist — ErrorStep only uses `collapsed` (route the too-tall error + advice through the FullscreenAiViewer, then render headline + action Select). Rewrote both comments to describe the collapsed-vs-full behavior and note `dense` is accepted for call-site compatibility but no longer alters layout. Doc-only; no behavior change. Build green, frame-fit 8/8, cli:lint clean.
Proactive sweep for the dense-comment-vs-code mismatch CodeRabbit has caught
several times: ImportNoMatchRecoveryStep's comment described a "Dense:" form, but
the body destructures only { identityName, options, onChange } — no dense. Fixed
the comment to describe the actual always-full layout (dense kept on props for
call-site compatibility only). A per-component scan of every onboarding step now
reports zero remaining stale dense comments. Doc-only; build green, frame-fit 8/8.
… diff build-complete auto-exited 100ms after rendering, so the alt-screen teardown wiped the success frame instantly. It now stays until the user presses Enter/Esc/q (extracted isBuildCompleteDismissKey for testability), then exits and lets command.ts reprint the durable summary. The workflow-file diff viewer rendered inside the wizard Box (header/padding top gap) and reserved 12 chrome rows; it's now a fullscreen early-return takeover that fills via minHeight and reserves only the real chrome — both iOS and Android. Tests: test-build-complete-exit.mjs (pure isBuildCompleteDismissKey predicate) + test-diff-viewer-viewport.mjs (VT-grid render asserts it fills the height + truncates), both wired into the CI test chain.
|
…rofile fix Merges origin/main #2376 (build init onboarding TUI rewrite — wizard, AI build-debug, adaptive sizing, alt-screen, decomposed shell + per-step files) into our long-running PR #2308 (iOS pick-profile empty recovery, confirm-app-id, AVAILABLE/UNAVAILABLE labels, etc.). The two branches evolved the same files for different reasons: main #2376 : full rewrite of ui/app.tsx into shell + ui/steps/*.tsx, alt-screen mounting, AI-driven build-debug, frame-fit / min-size gates, completed-steps log, workflow generator. ours #2308 : iOS import flow features — two-table picker, Apple-side cert validation, no-match recovery menu, .mobileprovision file picker, confirm-app-id when capacitor.config and pbxproj disagree, AVAILABLE/UNAVAILABLE labels. Resolution strategy: * package.json — union both test-script lists; union dependencies (react ^19.2.6 from main + string-width ^8.2.1 from ours for the Table column-width math). * types.ts — auto-merged; both sides extend OnboardingStep + the OnboardingProgress shape additively, no overlap. * progress.ts, apple-api.ts, macos-signing.ts — auto-merged; our helpers (filterProfilesForApp, classifyCertAvailability, findCertBySha1, getImportEntryStep, extractKeyIdFromP8Path) all survived intact alongside main's additions. * command.ts — take main's OnboardingShell render path (alt-screen, platform picker inside the wizard); thread our new `iosBundleIdInitial` prop (sourced from `config.appId`) through to OnboardingApp for Apple-side operations. * file-picker.ts — union; keep main's openServiceAccountJsonPicker and our openMobileprovisionPicker (different recovery paths use each). * ui/components.tsx — union; keep main's box/frame-fit constants + FullscreenAiViewer + DiffSummary + SecretsTable + the Header `compact` back-compat prop, our Table (column-width-by-display-width) + applyConstraints (the FilteredTextInput pipeline used by Key ID / Issuer ID inputs). * ui/shell.tsx — add `iosBundleIdInitial` prop and thread it through to OnboardingApp. * ui/app.tsx — take main's rewrite wholesale as the base; thread `iosBundleIdInitial` through AppProps, hydrate from progress.iosBundleIdOverride (saved by a previous confirm-app-id pick), and use it for every Apple-side operation. The user-facing confirm-app-id UI + staleness detection (savedOverrideIsFresh, redirectIfMismatch helper) ride in on a follow-up commit since they're additive on top of this merged state. Apple-side appId → iosBundleId switched at the call sites that matter: - ensureBundleId(token, iosBundleId) - createProfile(token, …, iosBundleId) (both import + create-new) - the synthesized provisioning profile's bundleId - the import-pick-profile defense check (profile.bundleId vs iosBundleId) - provisioningBundleId default for the create-new path Everything Capgo-side (loadProgress, saveProgress, deleteProgress, loadSavedCredentials, updateSavedCredentials, buildRequestCommand) keeps using `appId` — see the AppProps doc-block in ui/app.tsx for the split. Our Rescan removal (commit 36a7c282) re-applied — the legacy `import-fetching-profile` step is removed from OnboardingStep, ImportFetchingProfileStep is dropped from ios-import.tsx, the parent's import-fetching-profile useEffect is replaced with an explanatory comment, and error-categories.ts's reference to that step is dropped. Verified: typecheck + lint green, build green, all 4 onboarding-feature test suites pass (bundle-id-detector 17, onboarding-recovery 9, onboarding-progress 12, macos-signing 19, apple-api-import-helpers 12), sample of main's new suites green (ai-fit 31, onboarding-min-size 50, android-onboarding-progress 11, build-log-sanitize 15). Follow-up commits on this same branch restore the UI features deferred from the merge: confirm-app-id step + staleness detection, two-table identity picker with AVAILABLE/UNAVAILABLE column, eager batch validation (import-validating-all-certs), per-identity Apple cert check (import-checking-apple-cert), .mobileprovision file picker recovery option, manual portal walkthrough, cert SHA1 defense check at profile pick time.
…idation Final batch of features re-applied on top of main's TUI rewrite (PRs #2376 + #2308 reconciliation). Adds the AVAILABLE / UNAVAILABLE two-table identity picker, the eager batch validation that powers it, and the per-identity Apple cert check + auto-fetch profile path. All three were in PR #2308 pre-merge and are now restored on main's shell + per-step file architecture. 1. Eager batch validation (`import-validating-all-certs`) - Fans out `findCertBySha1` across every scanned distribution identity in parallel (typically 1–3 lookups, max ~10). - Each lookup fails independently — a network blip on one identity doesn't disqualify all certs. - Result is fed through `classifyCertAvailability` and stored in `identityAvailability` keyed by Keychain SHA1. Includes Apple- side cert metadata (name / expirationDate / serialNumber) for the manual portal walkthrough's disambiguation hints. - Routed in from the end of `verifying-key` (importMode) when there's at least one match. 2. Per-identity Apple cert check (`import-checking-apple-cert`) - Triggered when the user picks an identity that has no matching on-disk profile AND we have a verified API key. - Trusts the cached `appleCertId` from the eager batch when present; falls back to a fresh `findCertIdBySha1` otherwise. - Auto-fetches profiles for the cert and sends the user straight to `import-pick-profile` when Apple has a matching one — the happy path skips the recovery menu entirely. - When Apple has profiles but none target this app's bundle id, logs a specific warning listing the bundle ids it did find before routing to the recovery menu, so the user knows their cert is valid but their search has the wrong target. 3. Two-table identity picker - When `identityAvailability` is populated, renders two `Table`s (✅ AVAILABLE / ✖ UNAVAILABLE) with row indices that match the Select options below. - Per-row "Profile" column is a binary readiness signal — green `AVAILABLE` when there's a ready-to-use on-disk profile, red `UNAVAILABLE` when picking will land in the no-match recovery menu (still usable via file picker / Apple create). - Unavailable rows are NOT in the Select — they're informational only. The "Reason" column (yellow) explains why (revoked, wrong team, expired, no private key, etc.). - Falls back to main's flat-list `ImportPickIdentityStep` when no classification exists (ad_hoc without .p8 entry path). - On pick: three-way routing — * local profile matches → straight to `import-pick-profile` * no match but API key available → `import-checking-apple-cert` * otherwise → `import-no-match-recovery` Imports added: `EnrichedIdentityAvailability` type; `classifyCertAvailability`, `findCertBySha1`, `listProfilesForCert` from apple-api.js; `filterProfilesForApp` from macos-signing.js; `Table` from components.js. Build, lint, typecheck green; all onboarding suites pass (bundle-id-detector 17, onboarding-progress 12, macos-signing 19, apple-api-import-helpers 12, onboarding-min-size 50, ai-fit 31).



What
Adds the full
capgo build initonboarding TUI — an Ink/React terminal wizard that walks a user from zero to a first cloud build for iOS and Android, including an on-failure AI build-debug flow. This replaces the previous narrow PR (#2328) and presents the whole feature as a single squashed commit.Why
build initpreviously dropped users into raw prompts. This gives a guided, resumable, full-screen wizard that detects existing state, collects only what's missing, and streams the cloud build — with graceful handling of small terminals and long/garbled remote logs.What's included
Onboarding wizard
OnboardingShell) rendered once in the alternate-screen buffer; platform chosen up front, then the per-platform app mounts inline.On-failure AI build debug
Adaptive terminal sizing
stdouteach render (no one-frame lag), and the resize prompt names the floor of the screen actually shown.Robust build-log viewer
\r(in-place "Cruising 🚗" redraws), strips ANSI/escape/C0 control bytes (the viewer re-applies its own ✔/✖/⚠ colouring).less +F-style follow mode + live elapsed timer.Tests — a VT test harness built on
@xterm/headless(renders real Ink output through a real terminal grid) plus ~20 test files: frame-fit per step, per-platform min-size, size gates, platform layout, AI fit, build-log sanitize, build-output viewport.Test plan
bun run cli:lintbun run cli:typecheckbun install --frozen-lockfile(CLI) — lockfile in syncbun run cli:test(onboarding + frame-fit + size + sanitize suites)capgo build initon iOS and Android at small/large/ resized terminalsThis branch was squashed from a point before main landed its tracking-v2 (actor-scoped) / usage-analytics / hide-AI-model / GitHub-Actions-workflow changes, which touch the same onboarding files. Those conflicts (
command.ts,ui/app.tsx,android/ui/app.tsx,components.tsx,package.json,bun.lock,tsconfig.json) must be resolved at merge time — keeping both main's tracking-v2 calls and this PR's onboarding rewrite, and unioning thetest:*scripts. The config-file conflicts are mechanical (newer versions +@xterm/headless+ script union); the TSX ones graft main's tracking calls into the rewritten flow.Summary by CodeRabbit
New Features
Bug Fixes
Chores