feat(cli): AI-conducted Capgo Builder onboarding via MCP (2-tool spine)#2370
Closed
WcaleNieWolny wants to merge 19 commits into
Closed
feat(cli): AI-conducted Capgo Builder onboarding via MCP (2-tool spine)#2370WcaleNieWolny wants to merge 19 commits into
WcaleNieWolny wants to merge 19 commits into
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 15 minutes and 40 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR implements a complete MCP-driven onboarding system for Capgo Builder that guides users through iOS and Android credential setup via two registered MCP tools. It includes a state-machine engine with pure decision logic, dependency-injected side effects, comprehensive tests, and design documentation. ChangesCapgo Builder MCP Onboarding
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
![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: 3f94ac9770
ℹ️ 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".
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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: 14
🤖 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/mcp/contract.ts`:
- Around line 56-62: Rename the exported constant ONBOARDING_RULES to a
camelCase name (e.g., onboardingRules) and update its declaration to export
const onboardingRules: string[] = [...]; then search the repo for all
references/imports of ONBOARDING_RULES and update them to onboardingRules
(including any tests or re-exports) so imports remain consistent; ensure
TypeScript compiles after the rename and run tests to verify no remaining
references to ONBOARDING_RULES.
In `@cli/src/build/onboarding/mcp/engine.ts`:
- Around line 19-26: The constants are using SCREAMING_SNAKE_CASE; rename
ROADMAP to roadmap, NEXT_STEP_TOOL to nextStepTool, and MAX_AUTO_STEPS to
maxAutoSteps (and any other similarly cased constants) and update all
references/usages within this module so imports, function calls, and tests
(e.g., any use in engine.ts) reference the new camelCase names; ensure exports
(if any) keep the same external behavior but follow camelCase in declarations
and update any internal comments or documentation strings that mention the old
names.
- Around line 305-310: Reject out-of-order onboarding inputs by validating the
session’s expected step and allowed values before mutating state: in the code
paths that call drive and decideAdvance (and where decideStart/platformChosen
are invoked), first check that the session/progress is currently awaiting a
platform or credential input and that input.platform exists in
platformsDetected; if not, return the non-mutating decideStart/appropriate error
instead of persisting credentials or calling platformChosen/decideAdvance.
Ensure capgo_builder_onboarding_next_step calls are only reached after these
guards so no direct call can jump past the active step.
- Around line 276-294: decideBuildPhase presents a "skip" option but the
onboarding runner ("drive") only checks for runBuild === true, so returning
runBuild: false falls through and never completes onboarding; update the logic
so that a skip ends onboarding: in decideBuildPhase adjust the option/next
messaging to explicitly indicate skip should return { runBuild: false } and in
the onboarding executor (drive) add a branch that treats runBuild === false as a
terminal success (set onboarding complete/state 'complete' and stop further
steps); ensure you update both decideBuildPhase and drive to reference runBuild
(and NEXT_STEP_TOOL usage) so skip is handled as a final outcome.
In `@cli/src/build/onboarding/mcp/onboarding-tools.ts`:
- Around line 208-216: Extract the inline Zod schema object from
onboarding-tools.ts into a new exported schema (e.g., OnboardingMcpInputSchema)
under cli/src/schemas/, then import that schema into onboarding-tools.ts and
replace the inline schema with the imported OnboardingMcpInputSchema; update the
async handler signature to use z.infer<typeof OnboardingMcpInputSchema> for its
parameter type (replacing the manual annotation that references Platform,
serviceAccountJsonPath, runBuild, keyId, issuerId, p8Path) so runtime validation
and the TS type are derived from the single shared Zod schema.
- Around line 80-97: generateAndroidKeystore currently writes secret artifacts
(p12 base64 and passwords) into onboarding progress via saveAndroidProgress and
the _keystoreBase64 / keystoreStorePassword / keystoreKeyPassword fields;
similarly finalizeAndroidCredentials and updateSavedCredentials propagate those
secrets into saved progress and completedSteps. Change these functions so
onboarding progress only stores non-secret metadata (e.g.,
keystoreReady.isGenerated, keystoreAlias, keystoreCommonName, timestamps, and
flags) and do NOT persist _keystoreBase64, keystoreStorePassword,
keystoreKeyPassword, p12Base64, profileBase64, or any secret blob; instead
transfer secrets directly into the secure credential store in
finalizeAndroidCredentials/updateSavedCredentials and then scrub any transient
secret properties from the in-memory prog before calling saveAndroidProgress (or
avoid setting them at all). Locate usages in generateAndroidKeystore,
finalizeAndroidCredentials, updateSavedCredentials, loadAndroidProgress,
saveAndroidProgress and completedSteps to remove secret fields and ensure only
metadata is persisted.
- Around line 181-182: The catch block in onboarding-tools.ts currently returns
raw exception text (return { ok: false, error: err instanceof Error ?
err.message : String(err) }); change it to use the shared formatter by calling
formatError(err) and return { ok: false, error: formatError(err) } (import
formatError if not already imported) so iOS/provider/filesystem internals are
not leaked and onboarding errors stay consistent; update the catch in the
function that produces the MCP response accordingly.
In
`@docs/superpowers/plans/2026-05-30-capgo-builder-mcp-onboarding-plan-3-android-brief.md`:
- Line 48: Fix the malformed fragment "pastes/【provides" in the Play developer
id instruction: replace it with a clean phrasing such as "pastes/provides their
Play developer id (non-secret identifier → fine for chat); parse with
extractDeveloperId. Then call inviteServiceAccount (auto)." Keep the
`human_gate` example and the note about non-secret identifiers intact and ensure
extractDeveloperId and inviteServiceAccount are mentioned exactly as shown.
- Around line 9-13: The doc contains a contradiction between the line that drops
"OAuth / GCP-generate / Play-invite-via-API" and the Goal that still lists
"Google sign-in → GCP service account → Play invite"; decide which direction to
take and update the text to be consistent: either remove the OAuth/GCP/Play
invite flow from the Goal and change "Google sign-in → GCP service account →
Play invite" to a headless/managed credential path (keeping references to
capgo_request_build and cli/src/build/onboarding/android/*), or re-enable the
OAuth flow by deleting the "dropped for v1" statement and make the earlier line
match the Goal; ensure the phrases "OAuth / GCP-generate / Play-invite-via-API"
and "Google sign-in → GCP service account → Play invite" are reconciled so the
plan unambiguously reflects the chosen implementation.
In `@docs/superpowers/plans/2026-05-30-capgo-builder-mcp-onboarding.md`:
- Around line 112-114: Add a language tag to the fenced code block that contains
"&& bun run test:mcp-onboarding" by changing the opening fence from ``` to
```bash (or ```text) so the block is properly recognized and MD040 is resolved;
locate the fenced block around the "&& bun run test:mcp-onboarding" line and add
the language identifier after the opening backticks.
In `@docs/superpowers/specs/2026-05-30-capgo-builder-mcp-onboarding-design.html`:
- Around line 355-356: The spec uses outdated tool names like "..._status" and
"..._advance"; update all occurrences (including the repeated locations) to
match the implemented contract by replacing those stale names with
start_capgo_builder_onboarding and capgo_builder_onboarding_next_step so the
document references the actual tool identifiers used in code and tests (search
for the phrases "..._status" and "..._advance" and swap them to the two concrete
symbols: start_capgo_builder_onboarding and capgo_builder_onboarding_next_step).
- Around line 452-453: The tab buttons (<button role="tab" aria-selected="true"
data-tab="ios"> and <button role="tab" aria-selected="false"
data-tab="android">) lack an explicit type and will default to submit in forms;
update both button elements to include type="button" to avoid accidental form
submission and satisfy HTMLHint, keeping the existing attributes (role,
aria-selected, data-tab) unchanged.
In
`@docs/superpowers/specs/2026-05-30-capgo-builder-onboarding-user-journey.html`:
- Around line 289-290: The two tab control buttons (the <button> elements with
data-tab="ios" and data-tab="android") lack an explicit type and can act as
implicit submit buttons; update both button elements to include type="button"
(e.g., the buttons that have aria-selected and data-tab attributes) so they
won’t trigger form submission and to satisfy HTMLHint.
- Line 396: The onboarding copy inside the rcard that currently reads "Let's
connect Capgo first — here's the link to grab an API key, paste it here." must
be changed to remove any instruction to paste secrets and instead instruct users
to use the local terminal login flow; update the HTML fragment that contains the
sentence (the <div class="rcard"> / text "Let's connect Capgo first — here's the
link to grab an API key, paste it here.") to instead say something like "Run
`capgo login` in your terminal to authenticate locally" (or equivalent
messaging), and apply the same replacement to the other identical snippet noted
in the file so no guidance suggests sharing/pasting API keys in chat.
🪄 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: 34a20523-4aaf-4379-953c-154bf7afa798
📒 Files selected for processing (12)
cli/package.jsoncli/src/build/onboarding/mcp/contract.tscli/src/build/onboarding/mcp/engine.tscli/src/build/onboarding/mcp/onboarding-tools.tscli/src/mcp/server.tscli/test/test-mcp-onboarding.mjscli/test/test-mcp.mjsdocs/superpowers/plans/2026-05-30-capgo-builder-mcp-onboarding-plan-2-app-phase.mddocs/superpowers/plans/2026-05-30-capgo-builder-mcp-onboarding-plan-3-android-brief.mddocs/superpowers/plans/2026-05-30-capgo-builder-mcp-onboarding.mddocs/superpowers/specs/2026-05-30-capgo-builder-mcp-onboarding-design.htmldocs/superpowers/specs/2026-05-30-capgo-builder-onboarding-user-journey.html
Comment on lines
+19
to
+26
| const ROADMAP: string[] = [ | ||
| 'Preflight — detect your project & account', | ||
| 'Register the app in Capgo', | ||
| 'Set up signing credentials', | ||
| 'Run your first cloud build', | ||
| ] | ||
|
|
||
| const NEXT_STEP_TOOL = 'capgo_builder_onboarding_next_step' |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Rename the new constants to camelCase.
ROADMAP, NEXT_STEP_TOOL, and MAX_AUTO_STEPS don't follow the repository's TS naming rule.
As per coding guidelines, **/*.{ts,tsx,js}: Use camelCase for variable and function names.
Also applies to: 349-349
🤖 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/mcp/engine.ts` around lines 19 - 26, The constants
are using SCREAMING_SNAKE_CASE; rename ROADMAP to roadmap, NEXT_STEP_TOOL to
nextStepTool, and MAX_AUTO_STEPS to maxAutoSteps (and any other similarly cased
constants) and update all references/usages within this module so imports,
function calls, and tests (e.g., any use in engine.ts) reference the new
camelCase names; ensure exports (if any) keep the same external behavior but
follow camelCase in declarations and update any internal comments or
documentation strings that mention the old names.
Contributor
Author
There was a problem hiding this comment.
Not changing — same reason: SCREAMING_SNAKE is the established convention for module constants here (csr.ts, gcp-api.ts, play-api.ts).
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Resolve the review comment via GitHub CLI
gh api graphql -f query='
mutation {
resolveReviewThread(input: { threadId: "PRRT_kwDOBXFWNs5mHfXQ" }) {
thread {
isResolved
}
}
}'Length of output: 714
I wasn't able to auto-resolve the comment due to permission constraints — you can dismiss it manually. The learning about SCREAMING_SNAKE_CASE being the established convention for module-level constants in the Capgo CLI codebase has been recorded for future reviews.
Comment on lines
+80
to
+97
| generateAndroidKeystore: async (appId: string) => { | ||
| const storePassword = generateRandomPassword() | ||
| const keyPassword = generateRandomPassword() | ||
| const alias = 'release' | ||
| const ks = generateKeystore({ alias, storePassword, keyPassword, dname: { commonName: appId } }) | ||
| const base = (await loadAndroidProgress(appId)) ?? { platform: 'android' as const, appId, startedAt: new Date().toISOString(), completedSteps: {} } | ||
| await saveAndroidProgress(appId, { | ||
| ...base, | ||
| platform: 'android', | ||
| appId, | ||
| keystoreMethod: 'generate', | ||
| keystoreAlias: alias, | ||
| keystoreStorePassword: storePassword, | ||
| keystoreKeyPassword: keyPassword, | ||
| keystoreCommonName: appId, | ||
| _keystoreBase64: ks.p12Base64, | ||
| completedSteps: { ...base.completedSteps, keystoreReady: { keystorePath: '', alias, isGenerated: true } }, | ||
| }) |
Contributor
There was a problem hiding this comment.
Do not persist credential blobs and passwords in onboarding progress.
generateAndroidKeystore stores the keystore blob and passwords in progress, finalizeAndroidCredentials preserves them via ...prog, and iOS finalization adds p12Base64 / profileBase64 to completedSteps after those values were already written to updateSavedCredentials. That leaves long-lived secrets outside the credential store and increases the leak surface. Persist only non-secret progress metadata here, and scrub transient secrets once they have been transferred.
Also applies to: 132-135, 170-177
🤖 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/mcp/onboarding-tools.ts` around lines 80 - 97,
generateAndroidKeystore currently writes secret artifacts (p12 base64 and
passwords) into onboarding progress via saveAndroidProgress and the
_keystoreBase64 / keystoreStorePassword / keystoreKeyPassword fields; similarly
finalizeAndroidCredentials and updateSavedCredentials propagate those secrets
into saved progress and completedSteps. Change these functions so onboarding
progress only stores non-secret metadata (e.g., keystoreReady.isGenerated,
keystoreAlias, keystoreCommonName, timestamps, and flags) and do NOT persist
_keystoreBase64, keystoreStorePassword, keystoreKeyPassword, p12Base64,
profileBase64, or any secret blob; instead transfer secrets directly into the
secure credential store in finalizeAndroidCredentials/updateSavedCredentials and
then scrub any transient secret properties from the in-memory prog before
calling saveAndroidProgress (or avoid setting them at all). Locate usages in
generateAndroidKeystore, finalizeAndroidCredentials, updateSavedCredentials,
loadAndroidProgress, saveAndroidProgress and completedSteps to remove secret
fields and ensure only metadata is persisted.
Contributor
Author
There was a problem hiding this comment.
Not changing in this PR — this mirrors the existing wizard persistence model (shared progress types include _keystoreBase64 and CertificateData.p12Base64, and resume depends on them); the progress file is written mode 0600 in a 0700 dir. Re-architecting secret storage is tracked as a follow-up.
Contributor
There was a problem hiding this comment.
@WcaleNieWolny, understood — given that the shared progress types already include _keystoreBase64 / CertificateData.p12Base64 and resume logic depends on them, keeping the current persistence model consistent within this PR is reasonable. The 0600/0700 file permissions mitigate the immediate exposure risk.
Would you like me to open a follow-up GitHub issue to track the secret-storage re-architecture (moving credential blobs out of progress files and into a secure credential store, scrubbing transient secrets after transfer)?
✏️ Learnings added
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 2370
File: cli/src/build/onboarding/mcp/onboarding-tools.ts:81-98
Timestamp: 2026-05-30T10:49:28.863Z
Learning: In `cli/src/build/onboarding/mcp/onboarding-tools.ts` (and related Android/iOS progress files), the onboarding progress intentionally persists secret artifacts such as `_keystoreBase64`, `keystoreStorePassword`, `keystoreKeyPassword`, `p12Base64`, and `profileBase64`. This mirrors the existing wizard persistence model where shared progress types (e.g., `CertificateData.p12Base64`) include these fields and resume logic depends on them. The progress file is written with mode 0600 in a 0700 directory. A follow-up refactor to move secrets exclusively into a secure credential store is planned but out of scope for the current PR.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
…erminal, formatError, shared Zod schema; docs + security nits
|
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds AI-conducted onboarding for Capgo Builder (native iOS/Android cloud builds) to the CLI's MCP server. Instead of the TTY-only Ink wizard, the MCP server now drives the whole flow as a server-owned state machine: the AI only ever calls a 2-tool spine —
start_capgo_builder_onboarding(orient/resume, no args) andcapgo_builder_onboarding_next_step(advance) — and every tool result tells the AI exactly what to do next via akind(auto/human_gate/choice/done/error) and a literalnextaction. Heavy work runs server-side by reusing the existing headlessbuild/onboarding/modules; secrets travel by file path / the local credential store, never through the chat.End-to-end, a developer goes from zero to a first cloud build for either platform:
Architecture: pure deciders (
decideStart/decideAndroid/decideIos/decideBuildPhase) + a deps-injecteddrive()auto-executor that performs side-effect steps and coalesces them until the next human gate/choice. All IO is injected (EngineDeps) so the flow is unit-testable headlessly — mirroring the existinggetResumeStep/getAndroidResumeSteppattern.New files (under
cli/src/build/onboarding/mcp/):contract.ts— theNextStepResultresult contract +renderResult()(directive-first text)engine.ts— the state machine (pure deciders + injected orchestrators)onboarding-tools.ts— registers the 2 MCP tools and wires real deps to the existing modulesReuses without rewrites:
apple-api.ts,csr.ts,android/keystore.ts,android/service-account-validation.ts,credentials.ts,onboarding/progress.ts,android/progress.ts,init/app-conflict.ts. Touchesmcp/server.tsto register the tools.Design notes are committed under
docs/superpowers/specs/(architecture + end-user journey, as HTML) anddocs/superpowers/plans/(the phased implementation plans).Scope / deliberate deferrals
oauth-google/gcp-api/play-apimodules remain available for a follow-up).Test plan
All green locally: 40/40 unit tests, build OK, smoke OK.
Manual MCP walkthrough (from a Capacitor project dir, server connected):
start_capgo_builder_onboarding→ returns preflight + roadmap + the first step.next: relayhuman_gateinstructions to the user, passchoice/collected values back viacapgo_builder_onboarding_next_step..p8path + Key ID + Issuer ID → distribution cert + provisioning profile create → creds save.capgo_request_buildis triggered.Screenshots
N/A — MCP/CLI backend feature, no UI. The architecture and end-user journey are documented as HTML under
docs/superpowers/specs/2026-05-30-capgo-builder-mcp-onboarding-*.html.Checklist
bun run lint:backend && bun run lint. —oxlintisn't installed in this dev environment, so I could not run lint locally (CI will). New files follow the existing import/style conventions; please flag any oxlint nits.usageskill) should mention the two new MCP tools.docs/superpowers/.Summary by CodeRabbit
New Features
Documentation
Tests