Skip to content

feat(paywalls): let the CLI edit a chosen paywall screen - #170

Draft
jholwill wants to merge 2 commits into
mainfrom
worktree-paywall-step-selection
Draft

feat(paywalls): let the CLI edit a chosen paywall screen#170
jholwill wants to merge 2 commits into
mainfrom
worktree-paywall-step-selection

Conversation

@jholwill

@jholwill jholwill commented Sep 9, 2026

Copy link
Copy Markdown

Fixes FUN-2339

Checklist

  • Tests added or updated, if applicable
  • go build ./..., go test ./..., and gofmt pass

Motivation

A paywall flow can have several screens, but rc paywalls edit could only ever reach the draft fallback one. There was no way to see the other screens, and a sibling screen's content is not readable through the paywall route at all — so even knowing a screen existed, there was nothing to edit it with.

This is the CLI half of cross-client paywall editing parity. Server side: RevenueCat/khepri#25024 (graph read) and #25028 (selected PATCH). Sibling clients: RevenueCat/agents#1322 and #1323 (Astra, Rico, MCP).

Description

Adds rc paywalls screens <paywall-id>, listing each screen with its position, name, id and type, and marking the purchase screen. Adds an optional --step-id to rc paywalls edit.

With a selection, the screen's content is seeded from the graph rather than fetched through a route that cannot serve a sibling, and the selection is carried all the way into the save — PATCH /v2/projects/{id}/paywalls/{parent}?step_id=… — so the edit lands on the screen the user chose rather than the fallback. Session files are namespaced by step, so editing two screens of one paywall doesn't make them share state.

Three details worth a reviewer's attention:

Purchase-screen status comes from the graph's paywall_step_id. A step's is_terminal only means it has no outgoing edges, which is a different question.

The saved id comes from the PATCH response. It's the edited screen's own id and can differ from the parent the command was given. The previous code echoed the input id, which was harmless while every save hit the fallback and wrong as soon as one doesn't. --json now reports target_id alongside the unchanged paywall_id parent locator, so downstream commands like publish still work off the parent.

Save-failure hints keep --step-id. A review caught that the original 409 hint suggested a bare rc paywalls edit <paywall_id>, which on retry would silently reseed and edit the purchase screen — the exact outcome this feature must avoid. 404 and 422 now get their own hints too; 422 on step_id points at rc paywalls screens rather than suggesting a retry, since it isn't transient.

Standalone paywalls (graph: null) and unresolvable selections fail with a message naming the problem, never by falling back to default editing.

Codegen: the graph route isn't in the vendored docs/specs/v2-developer.yaml yet, so its types and methods in internal/api/paywalls.go are hand-written to the public contract — the same convention this repo already uses for not-yet-public routes. The route is registered through the existing NON_SPEC_PATHS escape hatch in scripts/gen-paths.py (as fonts and invoices already are); I dry-ran the generator and diffed against the committed paths_gen.go to confirm no drift. Once the khepri PR merges, the NON_SPEC_PATHS entry and the hand-written types should be removed and reconciled with whatever oapi-codegen produces — field names may not match exactly, since these were written against documentation rather than a real spec fragment.

Testing: go build ./..., gofmt -l ., go vet ./... clean; go test ./... and go test -race ./... pass across 13 packages; golangci-lint run ./... reports 0 issues. New tests cover the screens listing and its purchase-screen marker, --step-id forwarding, the sibling's own content reaching the editor, the PATCH targeting parent-plus-step_id with target_id differing from paywall_id, --step-id with --session rejected, and standalone/unresolvable/409-hint behavior.

Not verified against a live server: no backend was available, so this is written against the documented contract.


Note

Medium Risk
Changes paywall AI edit/save paths and draft PATCH behavior for multi-screen flows; mistakes could write to the wrong screen, though revision guards and explicit step validation reduce that risk.

Overview
Adds multi-screen paywall support to the CLI so AI editing is not limited to the fallback/purchase screen.

rc paywalls screens lists screens from the paywall graph API (position, name, id, purchase screen from paywall_step_id, not is_terminal). Standalone paywalls (graph: null) get an explicit message instead of a fake table.

rc paywalls edit --step-id loads sibling screen content via the expanded graph, forwards step_id to the Paywalls AI editor, saves with PATCH …/paywalls/{parent}?step_id=… (UpdateDraftStep), and tracks target_id from the PATCH response (can differ from the parent paywall id). Session files are per step; --step-id cannot be combined with --session. Invalid/standalone selections and save failures (409/422/404) error or hint without silently reverting to the default screen.

The API layer adds hand-written graph types and methods until the route is in the OpenAPI spec (NON_SPEC_PATHS + cli-coverage.yaml updated).

Reviewed by Cursor Bugbot for commit b843053. Bugbot is set up for automated code reviews on this repo. Configure here.

The paywall AI edit flow could only ever reach a paywall's draft fallback
screen: there was no way to see the rest of the flow, and a sibling's
content is not readable through the paywall route.

Add rc paywalls screens to list a flow's screens, and an optional --step-id
on rc paywalls edit. The selected screen's content is seeded from the graph
rather than fetched through a route that cannot serve it, and the selection
is carried into the save so the edit applies to the screen the user picked.
Session files are namespaced by step so two screens of one paywall do not
share state.

Purchase-screen status comes from the graph's paywall_step_id; a step's
is_terminal only means it has no outgoing edges.

The saved id now comes from the PATCH response, which is that screen's own
id and can differ from the parent the command was given. Save-failure hints
keep --step-id, so retrying after a conflict cannot quietly move the edit
onto the purchase screen. Standalone paywalls and unresolvable screen
selections fail with a message naming the problem.

The graph route is hand-written against the public contract until it
reaches the vendored spec that codegen reads.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jholwill

Copy link
Copy Markdown
Author

@claude please review.

Focus areas: the saved id now comes from the PATCH response rather than echoing the input — it is the edited screen's own id and can differ from the parent the command was given. --json reports target_id alongside the unchanged paywall_id parent locator so publish still works off the parent; please check nothing else treats the returned id as a parent.

Session files are namespaced by step so editing two screens of one paywall does not share state — worth confirming the key covers every case.

Save-failure hints intentionally preserve --step-id: a retry hint suggesting a bare rc paywalls edit <paywall_id> would silently reseed and edit the purchase screen, which is the exact outcome this feature exists to prevent.

Codegen: the graph route is not yet in the vendored docs/specs/v2-developer.yaml, so types in internal/api/paywalls.go are hand-written to the public contract and registered via the existing NON_SPEC_PATHS escape hatch. Once the khepri PR merges, both should be removed and reconciled with make gen — field names may not match, since these were written against prose.

@jholwill

Copy link
Copy Markdown
Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b843053. Configure here.

Comment thread internal/api/paywalls.go Outdated
// path; the target screen is addressed only by the step_id query param, and
// the response's id is that screen's own canonical id, which can differ from
// parentID. khepri rejects name and automatically_scale_font_size for a
// sibling screen, so this clears Name regardless of what the caller set.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Internal codename leaked in comments

Low Severity

Comments in UpdateDraftStep and the NON_SPEC_PATHS entry name the internal backend khepri. That codename is not allowed in comments or other in-repo text; the public wording is the API or the backend.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by learned rule: Avoid internal codename "Khepri" and leaked internal details in public repo

Reviewed by Cursor Bugbot for commit b843053. Configure here.

UIConfig: json.RawMessage(minimalUIConfig),
ProductVariables: map[string]string{},
SessionItems: json.RawMessage(`{}`),
}, nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sibling seed omits offering context

Medium Severity

seedSessionFromServerForStep never copies the parent paywall's OfferingID onto the session. Fallback seeding does, and persist even avoids clearing it on a sibling save because it is what the editor uses for product context. A --step-id edit therefore sends a null offering and the model designs that screen without the offering's products.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b843053. Configure here.

The editor resolves products from the offering, so a --step-id edit designed
that screen against placeholder prices. Content still comes from the graph;
only the offering is read from the parent.

Also names the API rather than the service behind it in two comments.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jholwill
jholwill force-pushed the worktree-paywall-step-selection branch from c8418f9 to 0ff872d Compare September 11, 2026 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant