Skip to content

feat(cli): offer Capgo Builder on incompatible upload#2368

Merged
WcaleNieWolny merged 15 commits into
mainfrom
feat/builder-cta-on-incompatible-upload
May 30, 2026
Merged

feat(cli): offer Capgo Builder on incompatible upload#2368
WcaleNieWolny merged 15 commits into
mainfrom
feat/builder-cta-on-incompatible-upload

Conversation

@WcaleNieWolny
Copy link
Copy Markdown
Contributor

@WcaleNieWolny WcaleNieWolny commented May 30, 2026

Why

When capgo bundle upload detects an incompatible bundle (native deps changed → a native build is required to reach users), there's nothing guiding the user toward the fix. This nudges them to Capgo Builder at exactly that moment. Builds on the Bundle Upload Compatibility Checked signal added in #2364.

Behavior

Triggered only when the bundle is incompatible (and not --ignore-metadata-check):

  • Interactive (TTY) — branched on whether the app has saved build credentials:
    • No credentials → "Set up Capgo Builder now?" → on accept, skips the OTA upload and launches build onboarding.
    • Has credentials → "Run a native build now?" → on accept, skips the upload and launches build request.
    • Decline → a low-fear confirm ("…native changes ship via an app-store build rather than OTA. Skip the Builder for now?"). If they confirm skip → snooze the prompt for this app for 3 days and continue the OTA upload. If unsure → continue, no snooze (asked again next time).
  • CI / non-interactive → prints a one-line ad (no prompt), upload proceeds.
  • Opt-out: CAPGO_NO_BUILDER_PROMPT=1 suppresses everything.
  • Programmatic SDK path is unaffected (silent) — no prompt, ad, or telemetry.

Tracking (PostHog, via existing trackEvent)

Builder CTA Shown / Accepted / Declined / Snoozed — carry appId + orgId (org-grouped) and join the existing Capgo Builder funnel.

Design notes

  • upload.ts (and the programmatic SDK bundle) stays free of ink. The CTA decision (maybePromptBuilderCta) lives in cli/src/bundle/builder-cta.ts (uses @clack/prompts, injectable for tests); the Ink-based launch runs in the CLI entry point (index.ts, which already imports those commands). uploadBundleInternal just returns a builderAction in its skip result.
  • Snooze state: ~/.capgo-builder-prompt.json (same pattern as promptPreferences), per-app, time-injectable.
  • Credential detection reuses loadSavedCredentials(appId).

Files

  • cli/src/bundle/builder-snooze.ts (new) + cli/src/bundle/builder-cta.ts (new)
  • cli/src/bundle/upload.ts, cli/src/index.ts, cli/src/schemas/bundle.ts
  • tests/builder-snooze.unit.test.ts, tests/builder-cta.unit.test.ts
  • docs/superpowers/specs|plans/2026-05-30-builder-cta-on-incompatible-upload*

Test plan

  • tests/builder-snooze.unit.test.ts + tests/builder-cta.unit.test.ts — 16 passing (decision matrix, orchestrator branches, snooze set/honor/expire/per-app)
  • bun run --cwd cli typecheck (tsgo) clean
  • bun run --cwd cli build — CLI + SDK build green (SDK bundle has no ink)
  • CI: full CLI suite + integration
  • Manual (TTY): incompatible upload with no creds → onboarding prompt; decline+confirm → snoozed 3 days (re-run shows no prompt); CAPGO_NO_BUILDER_PROMPT=1 → silent; piped/non-TTY → ad only, upload proceeds

Summary by CodeRabbit

  • New Features

    • Added a Builder CTA prompt for incompatible bundle uploads: interactive users may launch a native build or set up Builder; CI/non-interactive runs show an informational notice. Prompts can be disabled via CAPGO_NO_BUILDER_PROMPT.
  • Documentation

    • Added a design spec describing UX, telemetry, and decision flows for the Builder CTA.
  • Tests

    • Added unit tests covering CTA decision logic and prompt outcomes.

Review Change Stack

verifyCompatibility now reports incompatibility; uploadBundleInternal runs the
Builder CTA and, on accept, skips the OTA upload and returns the chosen action.
The Ink-based launch (build onboarding / build request) happens in the CLI entry
point so upload.ts (and the programmatic SDK bundle) stays free of ink.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 30, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a Builder CTA surfaced during incompatible capgo bundle upload: decision logic and printing, credential detection, telemetry events, upload early-exit with builderAction, CLI command handler wiring to launch onboarding/build, unit tests, and a design spec.

Changes

Builder CTA Feature

Layer / File(s) Summary
CTA decision and prompting logic
cli/src/bundle/builder-cta.ts
decideBuilderCtaSurface selects skip/ci-ad/prompt-onboarding/prompt-build based on incompatibility, env opt-out, interactivity, and saved credentials. printBuilderCiAd prints CI-mode copy. maybePromptBuilderCta loads credentials, emits "Builder CTA Shown"/"Accepted"/"Declined" telemetry, prints CI ad or shows a single confirm prompt with an OSC-8 Learn link, and returns continue/launch-onboarding/launch-build.
Upload flow integration and result schema
cli/src/bundle/upload.ts, cli/src/schemas/bundle.ts
verifyCompatibility now exposes incompatible and incompatibleCount. uploadBundleInternal calls maybePromptBuilderCta when incompatible and not silent; if the returned action is not continue, upload returns early with reason: 'NATIVE_BUILD' and includes optional builderAction. Schema adds optional builderAction enum.
CLI command handler and entry point
cli/src/bundle/upload-command.ts, cli/src/index.ts
Adds handleBundleUploadCommand which delegates to uploadBundle and, when result.builderAction is present, launches onboarding (launch-onboarding) or requests a build (launch-build) via existing Ink commands; index.ts wired to use the new handler for bundle upload.
CTA decision and prompting unit tests
tests/builder-cta.unit.test.ts
Vitest suite exercises decideBuilderCtaSurface decision matrix and maybePromptBuilderCta outcomes: continue for compatible/non-interactive, launch-onboarding/build on accept with/without credentials, and continue on decline.
Design specification
docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md
Design doc describing gating rules, UX for CI vs interactive, credential detection, telemetry events and tags, early-exit semantics, implementation plan, and test/regression checklist.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Cap-go/capgo#2364: Modifies bundle upload compatibility flow and emits compatibility telemetry; closely related to the compatibility verdict and incompatible-summary usage in this change.

Suggested reviewers

  • zinc-builds
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main feature: offering Capgo Builder when bundle upload detects incompatibility.
Description check ✅ Passed The description provides Why, Behavior, Tracking, Design notes, Files, and Test plan sections covering the feature comprehensively; aligns well with the template's Summary, Test plan, and Checklist structure.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added the codex label May 30, 2026
@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 30, 2026

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing feat/builder-cta-on-incompatible-upload (93eb529) with main (63d1253)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a2e35f53c

ℹ️ 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".

Comment thread cli/src/index.ts Outdated
bundle upload's --path is the web asset dir; build request treats path as the
Capacitor project root. Omitting it defaults to cwd() (the project root).
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🤖 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/bundle/builder-cta.ts`:
- Around line 68-127: maybePromptBuilderCta can throw because several awaited
calls (loadSavedCredentials, isBuilderPromptSnoozed, confirm/pConfirm,
snoozeBuilderPrompt) are unprotected; wrap the function body or the specific
await sequences in a try/catch and on any caught error log it and return
'continue' (i.e., degrade silently rather than rethrowing). Ensure you catch
failures from loadSavedCredentials, isBuilderPromptSnoozed, the confirm/pConfirm
calls and snoozeBuilderPrompt, keep trackEvent calls as appropriate, and use
pIsCancel logic as before; the goal is to guarantee maybePromptBuilderCta always
resolves to a BuilderCtaAction instead of throwing.

In `@cli/src/index.ts`:
- Around line 175-186: Extract the inline .action body around uploadBundle into
a new exported handler function (e.g., handleUploadBundleCommand) in a dedicated
module; move the await uploadBundle(...args) call and the subsequent branching
that checks result?.builderAction and calls onboardingBuilderCommand or
requestBuildCommand into that handler, preserving the same parameter shape
(appId, options) and behavior, export it, then replace the inline async action
in cli/src/index.ts with a simple call to the imported handleUploadBundleCommand
so index.ts only wires the command and options while the implementation lives in
the new handler module.

In `@docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md`:
- Around line 446-481: The plan incorrectly inlines Ink-based builder commands
into uploadBundleInternal; instead change the implementation to return early
with builderAction so the CLI entry point launches Ink. Update the call to
verifyCompatibility to destructure { nativePackages, minUpdateVersion,
incompatible, incompatibleCount }, then if incompatible and not silent call
maybePromptBuilderCta (passing interactive, appId: appid, orgId, apikey,
incompatibleCount); if builderAction !== 'continue' return the skipped result
object including builderAction, reason: 'NATIVE_BUILD', bundle, checksum: null,
encryptionMethod, and storageProvider: defaultStorageProvider (do not call
onboardingBuilderCommand or requestBuildCommand here). Also remove import of
onboardingBuilderCommand and requestBuildCommand and keep only
maybePromptBuilderCta.
- Around line 505-509: Add a blank line immediately before the opening fenced
code block marker (```bash) and a blank line immediately after the closing
fenced code block marker (```) so the three lines of commands ("bunx vitest
...", "bun run --cwd cli typecheck", "bun run --cwd cli build") are separated
from surrounding text; update the fenced block delimeters (```bash and ```)
accordingly to follow Markdown best practices.
- Around line 322-356: The params interface MaybePromptBuilderCtaParams and
function maybePromptBuilderCta are missing two injected inputs: add interactive:
boolean to MaybePromptBuilderCtaParams and add confirm?: BuilderConfirm (or
similar injectable) to the maybePromptBuilderCta params; remove the internal
canPromptInteractively() call and use the passed interactive value, and replace
direct pConfirm(...) calls with the injected confirm (falling back to pConfirm
if confirm is undefined) so tests can inject a fake confirm and callers (e.g.,
upload flow) can pass interactive.

In
`@docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md`:
- Line 152: The spec's BuilderCtaAction currently defines objects with a kind
field but the codebase uses a string-literal union; update the spec to match the
implementation by changing BuilderCtaAction to a string union 'continue' |
'launch-onboarding' | 'launch-build' so it aligns with the types used in
cli/src/bundle/builder-cta.ts (export type BuilderCtaAction) and the schema's
builderAction (z.enum in cli/src/schemas/bundle.ts).
- Around line 54-91: Several fenced code blocks in the doc (the top warning
block starting with "⚠️ This update changes native code...", the "No credentials
→ onboarding CTA" prompt block beginning "This update includes native
changes...", the nested confirmation block starting "Heads up: this update
includes native changes...", and the "Has credentials → build CTA" block "This
update needs a native build...") are missing language specifiers; update each
triple-backtick fence to include a language (e.g., ```text or ```shell) and add
a blank line before and after the nested confirmation fenced block to satisfy
markdownlint around that inner block.
- Around line 156-160: Update the spec to match the implemented helpers: remove
the bogus hasLocalCredentials reference and instead document using
loadSavedCredentials (which returns credentials|null) for credential detection
(see usage in builder-cta), and correct the snooze module location to the actual
builder-snooze implementation and its exported helpers isSnoozed and snooze
(i.e., reference the implemented builder-snooze module rather than a
non-existent utils/builder-snooze).

In `@tests/builder-cta.unit.test.ts`:
- Around line 51-90: Convert the tests inside the
describe('maybePromptBuilderCta') block (from the current it(...) cases starting
at the first it) to use it.concurrent(...) so they run in parallel, and
relocate/mock setup from the shared beforeEach into each test to keep them
deterministic: for each test set mockSnoozed.mockResolvedValue(false) and
mockLoadCreds.mockResolvedValue(null) (or the test-specific variant) and set
mockConfirm/mockSnooze as needed before calling maybePromptBuilderCta(params);
also call vi.resetAllMocks() or vi.clearAllMocks() at the start of each
concurrent test to isolate mocks. Reference symbols: maybePromptBuilderCta,
mockConfirm, mockSnooze, mockSnoozed, mockLoadCreds, params.

In `@tests/builder-snooze.unit.test.ts`:
- Around line 7-40: Tests share mutable globals dir and statePath and use
sequential it(...) causing races when parallelized; change each test to use
it.concurrent(...) and remove the top-level dir/statePath by creating per-test
temp dirs/statePath inside each test (or use a beforeEach that assigns local
variables scoped to the test) so calls to snoozeBuilderPrompt and
isBuilderPromptSnoozed operate on isolated files; ensure the cleanup (rmSync)
runs per-test or use tmpdir-specific removal in each test to avoid cross-test
interference.
🪄 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: ced9a212-954f-4bc4-a0bf-c187c06567ee

📥 Commits

Reviewing files that changed from the base of the PR and between 4be0de1 and 9a2e35f.

📒 Files selected for processing (9)
  • cli/src/bundle/builder-cta.ts
  • cli/src/bundle/builder-snooze.ts
  • cli/src/bundle/upload.ts
  • cli/src/index.ts
  • cli/src/schemas/bundle.ts
  • docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md
  • docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md
  • tests/builder-cta.unit.test.ts
  • tests/builder-snooze.unit.test.ts

Comment thread cli/src/bundle/builder-cta.ts
Comment thread cli/src/index.ts Outdated
Comment thread docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md Outdated
Comment thread docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md Outdated
Comment thread docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md Outdated
Comment thread docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md Outdated
Comment thread docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md Outdated
Comment thread docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md Outdated
Comment thread tests/builder-cta.unit.test.ts
Comment thread tests/builder-snooze.unit.test.ts Outdated
- maybePromptBuilderCta wrapped so it never throws (degrades to continue)
- extract bundle upload action into cli/src/bundle/upload-command.ts (keeps
  index.ts to wiring; keeps ink out of the SDK bundle)
- builder-snooze tests run concurrently with per-test temp dirs
- spec doc: correct BuilderCtaAction (string union), credential detection
  (loadSavedCredentials), snooze module path, and deferred-launch architecture
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md (1)

110-111: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix snooze file path typo in spec. The spec lists ~/.capgo/builder-prompt.json (line 110), but the implementation uses ~/.capgo-builder-prompt.json via builderPromptStatePath (matches line 161). Update line 110 to ~/.capgo-builder-prompt.json.

🤖 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
`@docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md`
around lines 110 - 111, Update the spec to match the implementation: change the
snooze file path string on the listed line from "~/.capgo/builder-prompt.json"
to "~/.capgo-builder-prompt.json" so it matches the implementation
constant/variable builderPromptStatePath referenced elsewhere; ensure the Shape
description remains the same and only the file path string is corrected.
♻️ Duplicate comments (1)
docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md (1)

54-91: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Language specifiers still missing from fenced code blocks.

The fenced code blocks showing prompt examples and messages still lack language identifiers (text or shell), and the nested confirmation block (lines 77-80) still needs blank lines before and after it.

📝 Proposed fix
 ### Non-interactive / CI
 
 Print a single warning + ad block, **no prompt**, then continue the upload unchanged:
 
-```
+```text
 ⚠️  This update changes native code, so it needs a native build to reach users
     on current app-store binaries.

Apply similar fixes to the other code blocks at lines 70, 77, and 86, and add blank lines around the nested block at 77-80 as shown in the previous review.

🤖 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
`@docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md`
around lines 54 - 91, Add missing fenced-code language specifiers and blank
lines: for the block starting with "⚠️  This update changes native code, so it
needs a native build to reach users" and the other example blocks containing
"This update includes native changes, which ship via an app-store build rather
than OTA." and "This update needs a native build. Run one now with Capgo
Builder? (Y/n)" add appropriate language tags (e.g., ```text or ```shell) to
each fenced code block; also add a blank line before and after the nested
confirmation block that begins "Heads up: this update includes native changes,
which ship via an app-store build rather than OTA." so it is separated from
surrounding fenced blocks.
🤖 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
`@docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md`:
- Around line 110-111: Update the spec to match the implementation: change the
snooze file path string on the listed line from "~/.capgo/builder-prompt.json"
to "~/.capgo-builder-prompt.json" so it matches the implementation
constant/variable builderPromptStatePath referenced elsewhere; ensure the Shape
description remains the same and only the file path string is corrected.

---

Duplicate comments:
In
`@docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md`:
- Around line 54-91: Add missing fenced-code language specifiers and blank
lines: for the block starting with "⚠️  This update changes native code, so it
needs a native build to reach users" and the other example blocks containing
"This update includes native changes, which ship via an app-store build rather
than OTA." and "This update needs a native build. Run one now with Capgo
Builder? (Y/n)" add appropriate language tags (e.g., ```text or ```shell) to
each fenced code block; also add a blank line before and after the nested
confirmation block that begins "Heads up: this update includes native changes,
which ship via an app-store build rather than OTA." so it is separated from
surrounding fenced blocks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8e76a8db-0f0f-431e-9dc3-5c80291041e9

📥 Commits

Reviewing files that changed from the base of the PR and between 9a2e35f and ef5b5a7.

📒 Files selected for processing (5)
  • cli/src/bundle/builder-cta.ts
  • cli/src/bundle/upload-command.ts
  • cli/src/index.ts
  • docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md
  • tests/builder-snooze.unit.test.ts

- fold the reason (native changes ship via an app-store build, not OTA) into the
  one prompt
- drop the second 'are you sure?' confirmation — declining just continues
- remove the 3-day snooze entirely (delete builder-snooze module + tests)
- CAPGO_NO_BUILDER_PROMPT still opts out; spec/plan updated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md (1)

448-487: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Task 5 still prescribes launching build commands inside upload.ts, which conflicts with shipped architecture.

This step still inlines onboardingBuilderCommand/requestBuildCommand in uploadBundleInternal, instead of returning builderAction and letting the CLI entry point launch Ink flows. That reintroduces the SDK/Ink coupling the feature explicitly avoids.

🤖 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 `@docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md`
around lines 448 - 487, The PR inlines Ink/CLI build commands into
uploadBundleInternal by calling onboardingBuilderCommand and requestBuildCommand
after maybePromptBuilderCta; instead, modify uploadBundleInternal (where
verifyCompatibility and maybePromptBuilderCta are used) to stop launching
onboardingBuilderCommand/requestBuildCommand and instead return a result that
includes the builderAction (or an explicit skipped reason) when incompatible is
true so the CLI entrypoint can read that builderAction and invoke
onboardingBuilderCommand/requestBuildCommand; update the returned object shape
to include builderAction/incompatibleCount/skipped metadata and ensure
maybePromptBuilderCta is still called here but only to produce the action, not
execute it.
🤖 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 `@docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md`:
- Around line 13-16: The plan text still instructs implementing a removed
snooze/second-confirm flow; update the document so the actionable tasks match
the shipped single-prompt/no-snooze UX: remove or clearly mark Tasks 1/3/6 (and
any references to builder-snooze.ts, snooze tests, decline-confirmation flow,
snooze tracking) as historical/non-executable, and replace them with concise
instructions to implement and test the current flow (a single yes/no prompt with
the reason folded into the question and no snooze), including references to the
new handler/component names used in the codebase and the specific tests to
add/update for the single-prompt behavior.

---

Duplicate comments:
In `@docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md`:
- Around line 448-487: The PR inlines Ink/CLI build commands into
uploadBundleInternal by calling onboardingBuilderCommand and requestBuildCommand
after maybePromptBuilderCta; instead, modify uploadBundleInternal (where
verifyCompatibility and maybePromptBuilderCta are used) to stop launching
onboardingBuilderCommand/requestBuildCommand and instead return a result that
includes the builderAction (or an explicit skipped reason) when incompatible is
true so the CLI entrypoint can read that builderAction and invoke
onboardingBuilderCommand/requestBuildCommand; update the returned object shape
to include builderAction/incompatibleCount/skipped metadata and ensure
maybePromptBuilderCta is still called here but only to produce the action, not
execute it.
🪄 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: e1e43be1-4047-4a4e-afae-bfa6b60feaa3

📥 Commits

Reviewing files that changed from the base of the PR and between ef5b5a7 and 5ff914c.

📒 Files selected for processing (4)
  • cli/src/bundle/builder-cta.ts
  • docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md
  • docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md
  • tests/builder-cta.unit.test.ts

Comment thread docs/superpowers/plans/2026-05-30-builder-cta-on-incompatible-upload.md Outdated
The plan was a point-in-time execution guide; the feature shipped and the flow
evolved (single prompt, no snooze, deferred Ink launch), leaving the plan's tasks
inconsistent with the code. The spec remains as the design record.
- exact prompt copy: '… An app store update may be required … Capgo Builder can
  help you build and publish the required native update. Would you like to
  configure / Start a native build with Capgo Builder now?'
- add an OSC 8 'Learn what Capgo Builder is' hyperlink (opens
  https://capgo.app/native-build/ without dismissing the prompt)
- CI ad + spec updated to match
Print the explanation (and keep it out of the prompt) via log.info, then ask a
short yes/no question. The clickable 'Learn what Capgo Builder is' hyperlink rides
on the prompt line.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
tests/builder-cta.unit.test.ts (1)

49-79: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Convert the maybePromptBuilderCta cases to it.concurrent(...) and isolate their mock setup.

This block still uses plain it(...), and the shared beforeEach/top-level mocks will race once you switch it to concurrent execution. Move the reset/setup into each test as part of the conversion so the file actually satisfies the repo’s parallel-test requirement. As per coding guidelines, tests/**/*.test.ts: “Design all tests for parallel execution across files; use it.concurrent() instead of it() to run tests in parallel within the same file for faster CI/CD”.

#!/bin/bash
sed -n '43,80p' tests/builder-cta.unit.test.ts
printf '\n--- non-concurrent tests in this file ---\n'
rg -nP '^\s*it\s*\(' tests/builder-cta.unit.test.ts
🤖 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 `@tests/builder-cta.unit.test.ts` around lines 49 - 79, Tests using
maybePromptBuilderCta are not safe for parallel runs; convert each it(...) to
it.concurrent(...) and move all mock setup/reset into the individual tests so
they don't rely on shared beforeEach state. For each test, call
mockConfirm.mockReset()/mockClear() and mockLoadCreds.mockReset()/mockClear(),
then set per-test behaviors (e.g.,
mockConfirm.mockResolvedValueOnce(true/false),
mockLoadCreds.mockResolvedValue(...) as needed) and use a fresh params object
via {...params} to avoid shared mutations; keep assertions the same but ensure
no top-level or shared beforeEach mocks remain that could race when using
it.concurrent. Ensure references: maybePromptBuilderCta, mockConfirm,
mockLoadCreds are initialized in each test body.
🤖 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/bundle/builder-cta.ts`:
- Around line 49-55: printBuilderCiAd currently emits four separate logs; keep
the REASON warning but replace the three subsequent log.info calls with one
consolidated info line. In printBuilderCiAd(hasCredentials) build a single
message string that picks the one-line CTA based on hasCredentials (use the
existing ternary content for the command) and append the learn/docs links inline
(e.g. "→ ...  Learn: ${LEARN_URL} · Docs: ${DOCS_URL}"), then call log.info
once; reference printBuilderCiAd, REASON, LEARN_URL and DOCS_URL when locating
the change.

---

Duplicate comments:
In `@tests/builder-cta.unit.test.ts`:
- Around line 49-79: Tests using maybePromptBuilderCta are not safe for parallel
runs; convert each it(...) to it.concurrent(...) and move all mock setup/reset
into the individual tests so they don't rely on shared beforeEach state. For
each test, call mockConfirm.mockReset()/mockClear() and
mockLoadCreds.mockReset()/mockClear(), then set per-test behaviors (e.g.,
mockConfirm.mockResolvedValueOnce(true/false),
mockLoadCreds.mockResolvedValue(...) as needed) and use a fresh params object
via {...params} to avoid shared mutations; keep assertions the same but ensure
no top-level or shared beforeEach mocks remain that could race when using
it.concurrent. Ensure references: maybePromptBuilderCta, mockConfirm,
mockLoadCreds are initialized in each test body.
🪄 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: 8e08ac68-bb8c-423b-a921-0b221d92737c

📥 Commits

Reviewing files that changed from the base of the PR and between 5ff914c and b97e7be.

📒 Files selected for processing (3)
  • cli/src/bundle/builder-cta.ts
  • docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md
  • tests/builder-cta.unit.test.ts

Comment thread cli/src/bundle/builder-cta.ts Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
`@docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md`:
- Around line 71-73: Update the "Decisions" section language that currently
states the reason is folded into a single yes/no prompt so it matches the new
two-message prompt flow: change the note that references a single combined
prompt to explicitly describe the two-step interaction (first a context line,
then a short yes/no prompt containing the clickable "Learn what Capgo Builder
is" link (OSC 8) that opens https://capgo.app/native-build/ without dismissing
the prompt), ensuring the decision text aligns with the two-message behavior and
removes the contradictory guidance.
🪄 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: cc6faf00-21f3-4eee-ae0e-b4f5dcc371d4

📥 Commits

Reviewing files that changed from the base of the PR and between b97e7be and 12aad49.

📒 Files selected for processing (2)
  • cli/src/bundle/builder-cta.ts
  • docs/superpowers/specs/2026-05-30-builder-cta-on-incompatible-upload-design.md

- printBuilderCiAd now emits a single log.warn line (was 4 log calls)
  per CodeRabbit feedback on the CI ad block.
- maybePromptBuilderCta takes hasCredentials as a param resolved by the
  caller (upload.ts) instead of calling loadSavedCredentials internally,
  so the unit tests run fully it.concurrent with no shared mock state.
- Update the design spec CI-ad section to the consolidated single line.
The interactive prompt is already TTY-gated and only fires on incompatible
uploads (dismissed with one keystroke); the CI path is a single log line.
An env opt-out added surface area for no real use case, so remove it:
- builder-cta.ts: drop envDisabled from BuilderCtaContext + the env check;
  decideBuilderCtaSurface now skips only on compatible bundles.
- tests: remove the env-disable case (9 tests, all concurrent).
- spec: drop the trigger condition, Opt-out env bullet, and Decisions note.
Resolve conflict in cli/src/bundle/upload.ts: main restructured
verifyCompatibility to return a compatibility:{result,versionOldId,
versionOldName} object (feeding the new 'Bundle Incompatible' Bento signal),
while this branch added incompatible/incompatibleCount for the Builder CTA.
Kept main's richer compatibility object plus incompatibleCount, and derive
incompatible locally from compatibility.result === 'incompatible' (no
redundant field). Both the Bento signal and the Builder CTA are preserved.
@sonarqubecloud
Copy link
Copy Markdown

@WcaleNieWolny WcaleNieWolny merged commit c798c1c into main May 30, 2026
44 checks passed
@WcaleNieWolny WcaleNieWolny deleted the feat/builder-cta-on-incompatible-upload branch May 30, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant