feat(cli): build init onboarding TUI — wizard, AI build-debug, adaptive sizing, robust log viewer#2376
Open
WcaleNieWolny wants to merge 1 commit into
Open
feat(cli): build init onboarding TUI — wizard, AI build-debug, adaptive sizing, robust log viewer#2376WcaleNieWolny wants to merge 1 commit into
WcaleNieWolny wants to merge 1 commit into
Conversation
…ve sizing, robust log viewer Squashed single commit for the entire `capgo build init` onboarding feature. Highlights: - Full Ink onboarding wizard for `build init` (iOS + Android): credential detection, App Store Connect API key, cert/profile import, Android keystore, GCP service account, CI-secrets upload, GitHub Actions workflow. - Platform-picker shell (OnboardingShell) rendered once in the alt-screen buffer. - On-failure AI build-debug offer + fullscreen scrollable AI analysis viewer. - Per-platform minimum terminal size (iOS 80x38 / Android 80x49), enforced after platform choice; resize-reactive; the picker only needs the banner floor (44x11). - iOS error screen routes through the scroll viewer when taller than the viewport (recovery advice is unbounded) so the retry actions are never clipped. - Build-log viewer: sanitize the remote fastlane stream (split bare \r, strip ANSI/control bytes), render one row per line (truncate, no wrap), and read the terminal size directly each render so resize repaints cleanly. - VT-based test harness (@xterm/headless) + ~20 test files (frame-fit, size gates, per-platform min-size, AI fit, build-log sanitize, viewport). NOTE: branched from an earlier main; resolve the overlap with main's tracking-v2 / analytics / hide-AI-model / GitHub-Actions-workflow changes at merge time.
Contributor
📝 WalkthroughWalkthroughAdds a unified Ink-based onboarding shell, caller-handled AI log analysis and routing, frame-fit/min-size logic, Android/iOS presentational steps, build-log sanitation, schema/telemetry updates, and extensive VT/Ink test harnesses and suites. package.json upgrades Node/React/Ink and test scripts; tsconfig tweaks deprecation handling. ChangesUnified Onboarding + Caller-Handled AI
Sequence Diagram(s)(omitted) Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
⚔️ Resolve merge conflicts
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| test('UNCAPPED long log overflows — proving the cap is what prevents too-small', () => { | ||
| const rows = 19 | ||
| const uncapped = frameRows(frame(longLog, rows, 80, 6), 80) |
| import process from 'node:process' | ||
| import { Alert, ProgressBar, Select } from '@inkjs/ui' | ||
| import { Box, Newline, Text, useApp, useInput, useStdout } from 'ink' | ||
| import { ProgressBar, Select } from '@inkjs/ui' |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 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".
| // restore wiped the wizard's last frame. Written via process.stdout to | ||
| // bypass the project-wide no-console lint rule (one-shot UX message, not | ||
| // application logging). | ||
| process.stdout.write(`\n✔ Capgo onboarding complete for ${appId}${resolvedPlatform ? ` (${resolvedPlatform})` : ''}.\n`) |
There was a problem hiding this comment.
When the user exits from wizard screens such as missing-platform or existing-credentials (those paths call exitOnboarding()), Ink's waitUntilExit() resolves the same way it does after the successful build-complete path. This unconditional write then prints ✔ Capgo onboarding complete... even though setup was cancelled, and because the wizard runs in the alternate screen the earlier "Exiting" message is wiped, leaving a misleading success breadcrumb. Track whether the app actually reached completion and only print this line in that case.
Useful? React with 👍 / 👎.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 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
Comment on lines
50
to
53
| "engines": { | ||
| "npm": ">=8.0.0", | ||
| "node": ">=20.0.0" | ||
| "node": ">=22.0.0" | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Node 22 features are actually required by searching for Node 22-specific APIs
rg -n --type=ts 'import.*from\s+['\''"]node:' cli/src/ | head -20
# Check if any Node 22-specific features like import.meta.dirname are used
rg -n 'import\.meta\.dirname|import\.meta\.filename' cli/src/Repository: Cap-go/capgo
Length of output: 1570
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm engines section change
echo "== cli/package.json engines =="
rg -n '"engines"' -n cli/package.json
sed -n '1,120p' cli/package.json | nl -ba | sed -n '40,80p'
# 2) Look for any import.meta usage at all
echo
echo "== import.meta usage in cli/src =="
rg -n 'import\.meta' cli/src/ || true
# 3) Look for other Node-specific entrypoints that could imply a newer minimum
echo
echo "== node:* module imports in cli/src (focus) =="
rg -n --type=ts "from 'node:(test|timers/promises|readline/promises|node:internal|module|events|util|stream/promises|worker_threads)'" cli/src/ || true
echo
echo "== node:test usage =="
rg -n --type=ts "from 'node:test'|require\(['\"]node:test['\"]\)" cli/src/ || true
echo
echo "== timers/promises usage =="
rg -n --type=ts "timers/promises" cli/src/ || true
echo
echo "== readline/promises usage =="
rg -n --type=ts "readline/promises" cli/src/ || trueRepository: Cap-go/capgo
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cli/package.json engines (show full block) =="
sed -n '45,60p' cli/package.json
echo
echo "== import.meta usage in cli/src =="
rg -n 'import\.meta' cli/src/ || true
echo
echo "== node:* module imports in cli/src (focus) =="
rg -n --type=ts "from 'node:(test|timers/promises|readline/promises|module|events|util|stream/promises|worker_threads|fs/promises|diagnostics_channel)'" cli/src/ || true
echo
echo "== node:test usage (any form) =="
rg -n --type=ts "from 'node:test'|require\\(['\"]node:test['\"]\\)" cli/src/ || true
echo
echo "== timers/promises usage =="
rg -n --type=ts "timers/promises" cli/src/ || true
echo
echo "== readline/promises usage =="
rg -n --type=ts "readline/promises" cli/src/ || trueRepository: Cap-go/capgo
Length of output: 2349
Node engine bump to >=22.0.0 is a breaking change—justify or document it.
cli/package.json raises the minimum Node version to >=22.0.0, but cli/src doesn’t show Node 22–specific APIs (only general import.meta.url usage in cli/src/build/onboarding/macos-signing.ts and other node:* modules; no node:test / timers/promises / readline/promises). If Node 22 is required for dependencies/build tooling, document the rationale in release notes and update CI expectations; otherwise consider keeping Node 20 compatibility.
🤖 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/package.json` around lines 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.
| "test:apple-api-import-helpers": "bun test/test-apple-api-import-helpers.mjs", | ||
| "test:manifest-path-encoding": "bun test/test-manifest-path-encoding.mjs", | ||
| "test": "bun run build && bun run test:version-detection:setup && bun run test:bundle && bun run test:functional && bun run test:semver && bun run test:version-edge-cases && bun run test:regex && bun run test:upload && bun run test:credentials && bun run test:credentials-validation && bun run test:android-service-account-validation && bun run test:build-zip-filter && bun run test:checksum && bun run test:build-needed && bun run test:ci-prompts && bun run test:ci-secrets && bun run test:posthog-exception && bun run test:build-platform-selection && bun run test:onboarding-recovery && bun run test:onboarding-progress && bun run test:onboarding-run-targets && bun run test:run-device-command && bun run test:init-app-conflict && bun run test:init-guardrails && bun run test:prompt-preferences && bun run test:esm-sdk && bun run test:mcp && bun run test:version-detection && bun run test:platform-paths && bun run test:payload-split && bun run test:manifest-path-encoding && bun run test:macos-signing && bun run test:apple-api-import-helpers && bun run test:ai-log-capture && bun run test:ai-analyze-flow && bun run test:ai-render-markdown", | ||
| "test": "bun run build && bun run test:version-detection:setup && bun run test:bundle && bun run test:functional && bun run test:semver && bun run test:version-edge-cases && bun run test:regex && bun run test:upload && bun run test:credentials && bun run test:credentials-validation && bun run test:android-service-account-validation && bun run test:build-zip-filter && bun run test:checksum && bun run test:build-needed && bun run test:ci-prompts && bun run test:ci-secrets && bun run test:posthog-exception && bun run test:build-platform-selection && bun run test:onboarding-recovery && bun run test:onboarding-progress && bun run test:onboarding-run-targets && bun run test:run-device-command && bun run test:init-app-conflict && bun run test:init-guardrails && bun run test:prompt-preferences && bun run test:esm-sdk && bun run test:mcp && bun run test:version-detection && bun run test:platform-paths && bun run test:payload-split && bun run test:manifest-path-encoding && bun run test:macos-signing && bun run test:apple-api-import-helpers && bun run test:ai-log-capture && bun run test:ai-analyze-flow && bun run test:ai-render-markdown && bun run test:ai-onboarding-mode && bun run test:ai-fit && bun run test:platform-layout && bun run test:frame-fit && bun run test:onboarding-min-size && bun run test:min-size-gate && bun run test:shell-size-gate", |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Consider breaking up the monolithic test script.
The test script chains 40+ individual test commands in a single line, making it difficult to read, maintain, or identify which test failed. Consider using a test runner configuration or a shell script.
🤖 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/package.json` at 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.
Comment on lines
+22
to
+24
| import { ProgressBar, Select } from '@inkjs/ui' | ||
| import type { DOMElement } from 'ink' | ||
| import { Box, measureElement, Text, useApp, useInput, useStdout } from 'ink' |
Contributor
There was a problem hiding this comment.
Resolve import/lint blockers in module header.
Select is imported but unused, and MAX_AI_RETRIES splits import declarations, triggering import/first errors across this file. This will fail lint/check pipelines.
Suggested patch
-import { ProgressBar, Select } from '`@inkjs/ui`'
+import { ProgressBar } from '`@inkjs/ui`'
...
-const MAX_AI_RETRIES = 2
import type { CiSecretEntry, CiSecretSetupAdvice, CiSecretTarget } from '../../ci-secrets.js'
...
import { ANDROID_STEP_PROGRESS, getAndroidPhaseLabel } from '../types.js'
+
+const MAX_AI_RETRIES = 2Also applies to: 35-40
🧰 Tools
🪛 ESLint
[error] 22-22: 'Select' is defined but never used.
(unused-imports/no-unused-imports)
[error] 23-23: Expected "ink" (type-import) to come before "@inkjs/ui" (value-external).
(perfectionist/sort-imports)
🤖 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/android/ui/app.tsx` around lines 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.
Comment on lines
+1729
to
+1734
| if (!terminalFitsOnboarding(terminalCols, terminalRows, 'android')) | ||
| return <TerminalTooSmallPrompt cols={terminalCols} rows={terminalRows} minRows={ANDROID_MIN_ROWS} /> | ||
|
|
||
| if (step === 'requesting-build') | ||
| return <FullscreenBuildOutput title="Building..." lines={buildOutput} terminalRows={terminalRows} /> | ||
|
|
Contributor
There was a problem hiding this comment.
Reorder early returns so fullscreen build/AI views are not blocked by min-size gate.
The min-size guard currently runs before requesting-build and ai-analysis-result-scroll, so shrinking the terminal can hide active streaming/debug views behind the resize prompt. That contradicts the intended takeover behavior described in nearby comments.
Suggested patch
-if (!terminalFitsOnboarding(terminalCols, terminalRows, 'android'))
- return <TerminalTooSmallPrompt cols={terminalCols} rows={terminalRows} minRows={ANDROID_MIN_ROWS} />
-
if (step === 'requesting-build')
return <FullscreenBuildOutput title='Building...' lines={buildOutput} terminalRows={terminalRows} />
+if (isAiResultScroll && aiAnalysisText)
+ return (
+ <FullscreenAiViewer
+ title='AI analysis'
+ subtitle={`${aiAnalysisText.split('\n').length} lines — scrollable because the analysis is taller than your terminal`}
+ lines={aiAnalysisText.split('\n')}
+ terminalRows={terminalRows}
+ onExit={() => {
+ setAiViewedFull(true)
+ setStep('ai-analysis-result')
+ }}
+ />
+ )
+
+if (!terminalFitsOnboarding(terminalCols, terminalRows, 'android'))
+ return <TerminalTooSmallPrompt cols={terminalCols} rows={terminalRows} minRows={ANDROID_MIN_ROWS} />
+
- if (isAiResultScroll && aiAnalysisText)
- return ( ... )Also applies to: 1742-1754
🤖 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/android/ui/app.tsx` around lines 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.
Comment on lines
+30
to
+38
| const CSI_RE = /\x1B\[[0-9;?]*[ -/]*[@-~]/g | ||
| // Other escape sequences: ESC + a single intermediate/final byte (OSC start, | ||
| // charset selection, etc.). Stripped defensively so no lone ESC survives. | ||
| // eslint-disable-next-line no-control-regex | ||
| const ESC_RE = /\x1B[@-Z\\-_]/g | ||
| // Remaining C0 control bytes + DEL, EXCLUDING tab (\x09), LF (\x0A) and CR | ||
| // (\x0D) — LF/CR are handled by the split above, tab is legitimate indentation. | ||
| // eslint-disable-next-line no-control-regex | ||
| const CTRL_RE = /[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="cli/src/build/onboarding/build-log.ts"
if [ ! -f "$file" ]; then
echo "Missing file: $file" >&2
exit 1
fi
nl -ba "$file" | sed -n '1,120p'
echo "----"
nl -ba "$file" | sed -n '120,220p'Repository: Cap-go/capgo
Length of output: 99
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="cli/src/build/onboarding/build-log.ts"
if [ ! -f "$file" ]; then
echo "Missing file: $file" >&2
exit 1
fi
echo "=== lines 1-120 ==="
cat -n "$file" | sed -n '1,120p'
echo "=== lines 120-220 ==="
cat -n "$file" | sed -n '120,220p'Repository: Cap-go/capgo
Length of output: 3804
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "e18e/prefer-array-at|regexp/no-obscure-range|regexp/control-character-escape" .Repository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a -t f "eslint*.*|\.eslintrc*|\.eslint*|eslint-config*|bunfig.*|package\.json" . | head -n 200
echo "----"
ls -la
echo "----"
# Search for the relevant rule names in the config
rg -n "regexp/no-obscure-range|regexp/control-character-escape|e18e/prefer-array-at" . || true
echo "----"
# Also inspect ESLint config packages
rg -n "\"eslint\"|eslintConfig|`@antfu/eslint-config`|regexp/" package.json **/package.json 2>/dev/null || trueRepository: Cap-go/capgo
Length of output: 5861
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== root eslint.config.js matches ==="
rg -n "no-obscure-range|control-character-escape|e18e/prefer-array-at" eslint.config.js cli/eslint.config.mjs .clinerules || true
echo "----"
echo "=== cli/eslint.config.mjs (relevant section) ==="
# show whole file if short; otherwise show first 220 lines
wc -l cli/eslint.config.mjs
sed -n '1,240p' cli/eslint.config.mjs
echo "----"
echo "=== root eslint.config.js (relevant section) ==="
wc -l eslint.config.js
sed -n '1,260p' eslint.config.jsRepository: Cap-go/capgo
Length of output: 2180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== bun availability =="
command -v bun || true
bun -v || true
echo "== scripts in cli/package.json =="
cat -n cli/package.json | sed -n '1,220p'
echo "== check how cli:check is run (root package.json) =="
cat -n package.json | sed -n '240,380p'
echo "== Attempt: bun run cli:check =="
# If bun isn't available, fallback to npm/yarn/pnpm not guaranteed; just try bun.
bun run cli:check --silent || bun run cli:check || true
echo "== Attempt: run eslint directly on the file =="
# Use bunx/eslint if available; otherwise just try npx.
if command -v bunx >/dev/null 2>&1; then
bunx eslint cli/src/build/onboarding/build-log.ts || true
else
npx eslint cli/src/build/onboarding/build-log.ts || true
fiRepository: Cap-go/capgo
Length of output: 15638
Fix blocking lint violations in build-log.ts regex declarations and trailing-element access
cli/src/build/onboarding/build-log.ts currently trips regexp/no-obscure-range (in CSI_RE/ESC_RE), regexp/control-character-escape (in CTRL_RE), and e18e/prefer-array-at (trailing empty-string check). Apply the following changes:
Suggested patch
-// eslint-disable-next-line no-control-regex
+// eslint-disable-next-line no-control-regex, regexp/no-obscure-range
const CSI_RE = /\x1B\[[0-9;?]*[ -/]*[`@-`~]/g
@@
-// eslint-disable-next-line no-control-regex
+// eslint-disable-next-line no-control-regex, regexp/no-obscure-range
const ESC_RE = /\x1B[`@-Z`\\-_]/g
@@
-// eslint-disable-next-line no-control-regex
-const CTRL_RE = /[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g
+// eslint-disable-next-line no-control-regex, regexp/control-character-escape
+const CTRL_RE = /[\x00-\x08\v\f\x0E-\x1F\x7F]/g
@@
- if (parts.length > 1 && parts[parts.length - 1] === '')
+ if (parts.length > 1 && parts.at(-1) === '')
parts.pop()🧰 Tools
🪛 ESLint
[error] 30-30: Unexpected obscure character range. The characters of ' -/' (U+0020 - U+002f) are not obvious.
(regexp/no-obscure-range)
[error] 30-30: Unexpected obscure character range. The characters of '@-~' (U+0040 - U+007e) are not obvious.
(regexp/no-obscure-range)
[error] 34-34: Unexpected obscure character range. The characters of '@-Z' (U+0040 - U+005a) are not obvious.
(regexp/no-obscure-range)
[error] 34-34: Unexpected obscure character range. The characters of '\-_' (U+005c - U+005f) are not obvious.
(regexp/no-obscure-range)
[error] 38-38: Unexpected control character escape '\x0B' (U+000b). Use '\v' instead.
(regexp/control-character-escape)
[error] 38-38: Unexpected control character escape '\x0C' (U+000c). Use '\f' instead.
(regexp/control-character-escape)
🪛 GitHub Check: Run CLI tests
[failure] 38-38:
Unexpected control character escape '\x0C' (U+000c). Use '\f' instead
[failure] 38-38:
Unexpected control character escape '\x0B' (U+000b). Use '\v' instead
[failure] 34-34:
Unexpected obscure character range. The characters of '\-_' (U+005c - U+005f) are not obvious
[failure] 34-34:
Unexpected obscure character range. The characters of '@-Z' (U+0040 - U+005a) are not obvious
[failure] 30-30:
Unexpected obscure character range. The characters of '@-~' (U+0040 - U+007e) are not obvious
[failure] 30-30:
Unexpected obscure character range. The characters of ' -/' (U+0020 - U+002f) are not obvious
🤖 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/build-log.ts` around lines 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.
Comment on lines
+103
to
+106
| test('UNCAPPED long log overflows — proving the cap is what prevents too-small', () => { | ||
| const rows = 19 | ||
| const uncapped = frameRows(frame(longLog, rows, 80, 6), 80) | ||
| assert(uncapped > rows, `expected the uncapped 30-entry log to overflow ${rows} rows, got ${uncapped}`) |
Contributor
There was a problem hiding this comment.
Fix incorrect frame() arguments in uncapped-overflow assertion.
At Line 105, frame(longLog, rows, 80, 6) passes 80 as bodyHeight (the function takes 3 params), so this test can pass for the wrong reason and stop validating uncapped-log overflow behavior.
Proposed fix
- const uncapped = frameRows(frame(longLog, rows, 80, 6), 80)
+ const uncapped = frameRows(frame(longLog, rows, 6), 80)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/test/test-frame-fit-log.mjs` around lines 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).
Comment on lines
+35
to
+40
| try { await fn(); passed++; console.log(`✔ ${name}`) } | ||
| catch (error) { failed++; console.error(`✖ ${name}\n ${error.message}`) } | ||
| } | ||
| function assert(cond, msg) { if (!cond) throw new Error(msg) } | ||
| const h = React.createElement | ||
| const noop = () => {} |
Contributor
There was a problem hiding this comment.
Resolve lint blockers in helper declarations and process usage.
This test currently fails multiple ESLint rules (style/max-statements-per-line, antfu/if-newline, antfu/top-level-function, node/prefer-global/process).
Suggested patch
+import process from 'node:process'
import { Box } from 'ink'
import React from 'react'
@@
async function test(name, fn) {
- try { await fn(); passed++; console.log(`✔ ${name}`) }
- catch (error) { failed++; console.error(`✖ ${name}\n ${error.message}`) }
+ try {
+ await fn()
+ passed++
+ console.log(`✔ ${name}`)
+ }
+ catch (error) {
+ failed++
+ console.error(`✖ ${name}\n ${error.message}`)
+ }
}
-function assert(cond, msg) { if (!cond) throw new Error(msg) }
+function assert(cond, msg) {
+ if (!cond)
+ throw new Error(msg)
+}
const h = React.createElement
-const noop = () => {}
+function noop() {}Also applies to: 96-96
🧰 Tools
🪛 ESLint
[error] 35-35: This line has 4 statements. Maximum allowed is 1.
(style/max-statements-per-line)
[error] 36-36: This line has 2 statements. Maximum allowed is 1.
(style/max-statements-per-line)
[error] 38-38: This line has 2 statements. Maximum allowed is 1.
(style/max-statements-per-line)
[error] 38-38: Expect newline after if
(antfu/if-newline)
[error] 40-40: Top-level functions should be declared with function keyword
(antfu/top-level-function)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/test/test-frame-fit-resize.mjs` around lines 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.
Comment on lines
+21
to
+24
| try { fn(); passed++; console.log(`✔ ${name}`) } | ||
| catch (error) { failed++; console.error(`✖ ${name}\n ${error.message}`) } | ||
| } | ||
| function assert(cond, msg) { if (!cond) throw new Error(msg) } |
Contributor
There was a problem hiding this comment.
Clean up lint violations in compact one-line statements and process access.
This file currently violates style/max-statements-per-line, antfu/if-newline, and node/prefer-global/process.
Suggested patch
+import process from 'node:process'
import { EventEmitter } from 'node:events'
import { render as inkRender } from 'ink'
@@
function test(name, fn) {
- try { fn(); passed++; console.log(`✔ ${name}`) }
- catch (error) { failed++; console.error(`✖ ${name}\n ${error.message}`) }
+ try {
+ fn()
+ passed++
+ console.log(`✔ ${name}`)
+ }
+ catch (error) {
+ failed++
+ console.error(`✖ ${name}\n ${error.message}`)
+ }
}
-function assert(cond, msg) { if (!cond) throw new Error(msg) }
+function assert(cond, msg) {
+ if (!cond)
+ throw new Error(msg)
+}
@@
- s.write = (f) => { s.frames.push(f); s.lastFrame = f }
+ s.write = (f) => {
+ s.frames.push(f)
+ s.lastFrame = f
+ return true
+ }Also applies to: 32-32, 139-139
🧰 Tools
🪛 ESLint
[error] 21-21: This line has 4 statements. Maximum allowed is 1.
(style/max-statements-per-line)
[error] 22-22: This line has 2 statements. Maximum allowed is 1.
(style/max-statements-per-line)
[error] 24-24: This line has 2 statements. Maximum allowed is 1.
(style/max-statements-per-line)
[error] 24-24: Expect newline after if
(antfu/if-newline)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/test/test-frame-fit-viewer.mjs` around lines 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.
Comment on lines
+7
to
+10
| import { Box, Text } from 'ink' | ||
| import process from 'node:process' | ||
| import React from 'react' | ||
| import { MIN_COLS, MIN_ROWS } from '../src/build/onboarding/min-terminal-size.ts' |
Contributor
There was a problem hiding this comment.
Reorder imports to satisfy configured sort rule.
node:process should be ordered before external imports to pass perfectionist/sort-imports.
Suggested patch
-import { Box, Text } from 'ink'
import process from 'node:process'
+import { Box, Text } from 'ink'
import React from 'react'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { Box, Text } from 'ink' | |
| import process from 'node:process' | |
| import React from 'react' | |
| import { MIN_COLS, MIN_ROWS } from '../src/build/onboarding/min-terminal-size.ts' | |
| import process from 'node:process' | |
| import { Box, Text } from 'ink' | |
| import React from 'react' | |
| import { MIN_COLS, MIN_ROWS } from '../src/build/onboarding/min-terminal-size.ts' |
🧰 Tools
🪛 ESLint
[error] 8-8: Expected "node:process" (value-builtin) to come before "ink" (value-external).
(perfectionist/sort-imports)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/test/test-min-size-gate.mjs` around lines 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.
| "lib": [ | ||
| "es2023" | ||
| ], | ||
| "ignoreDeprecations": "5.0", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check TypeScript version and identify deprecated features in use
cat cli/package.json | grep -E '"typescript"'
# Run tsc without ignoreDeprecations to see what warnings appear
cd cli && npx tsc --noEmit 2>&1 | head -30Repository: Cap-go/capgo
Length of output: 727
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cli/tsconfig.json =="
nl -ba cli/tsconfig.json | sed -n '1,120p'
echo
echo "== running tsc (current config) =="
cd cli
npx tsc --noEmit 2>&1 | head -40Repository: Cap-go/capgo
Length of output: 141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cli/tsconfig.json (numbered) =="
cat -n cli/tsconfig.json | sed -n '1,120p'
echo
echo "== running tsc (current config) =="
cd cli
npx tsc --noEmit 2>&1 | head -40Repository: Cap-go/capgo
Length of output: 1720
Document what ignoreDeprecations is suppressing—and fix the TS5101 mismatch
ignoreDeprecations: "5.0" is currently meant to suppress the deprecation for compilerOptions.baseUrl (since baseUrl is what TS5101 complains about). However, running npx tsc --noEmit still fails with TS5101 and explicitly says to use ignoreDeprecations: "6.0" to silence it.
- Add a comment explaining why
compilerOptions.baseUrlmust remain for now. - Update
ignoreDeprecationsto"6.0"(or migrate away frombaseUrl) sotscno longer errors.
🤖 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/tsconfig.json` at 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Release Notes
New Features
Bug Fixes
Dependencies