From b84305352b80efa92cff05fa9c8df0e191c5e14a Mon Sep 17 00:00:00 2001 From: Jamie Holwill Date: Wed, 9 Sep 2026 17:34:42 +0100 Subject: [PATCH 1/2] feat(paywalls): let the CLI edit a chosen paywall screen 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) --- docs/command-surface.md | 3 +- docs/specs/cli-coverage.yaml | 2 + internal/api/paywalls.go | 103 ++++++++++ internal/api/paywalls_test.go | 117 ++++++++++++ internal/cli/paywalls.go | 66 +++++++ internal/cli/paywalls_ai.go | 260 +++++++++++++++++++++----- internal/cli/paywalls_ai_step_test.go | 257 +++++++++++++++++++++++++ internal/cli/paywalls_screens_test.go | 75 ++++++++ internal/paywallai/paywallai.go | 7 +- scripts/gen-paths.py | 1 + 10 files changed, 844 insertions(+), 47 deletions(-) create mode 100644 internal/cli/paywalls_ai_step_test.go create mode 100644 internal/cli/paywalls_screens_test.go diff --git a/docs/command-surface.md b/docs/command-surface.md index 348069db..4db793c4 100644 --- a/docs/command-surface.md +++ b/docs/command-surface.md @@ -180,8 +180,9 @@ rc products store screenshot # upload an App Review rc paywalls # help; under npx a TTY shows a generate/edit picker (npm launcher sets RC_GUIDED — paywalls-only for now) rc paywalls list rc paywalls show +rc paywalls screens # list a paywall's screens (id, name, position, purchase screen) from its graph rc paywalls generate [--offering-id ] --prompt "..." # create a paywall; standalone unless --offering-id attaches it to an offering -rc paywalls edit |--session --prompt # AI-edit any paywall (draft components fetched via v2) or continue a session +rc paywalls edit |--session --prompt [--step-id ] # AI-edit any paywall (draft components fetched via v2, or via the graph for --step-id) or continue a session rc paywalls rewind --session # undo the last editor action rc paywalls publish [id] # publish the current draft; confirmation/--yes rc paywalls unpublish [id] # remove the published paywall; confirmation/--yes diff --git a/docs/specs/cli-coverage.yaml b/docs/specs/cli-coverage.yaml index ccafd5d1..921ad7c5 100644 --- a/docs/specs/cli-coverage.yaml +++ b/docs/specs/cli-coverage.yaml @@ -159,6 +159,8 @@ endpoints: path: /projects/{project_id}/paywalls/{paywall_id}/actions/unpublish - method: DELETE path: /projects/{project_id}/paywalls/{paywall_id} + - method: GET + path: /projects/{project_id}/paywalls/{paywall_id}/graph # Media assets - method: GET diff --git a/internal/api/paywalls.go b/internal/api/paywalls.go index 7dd3dee4..be0bab4e 100644 --- a/internal/api/paywalls.go +++ b/internal/api/paywalls.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "net/http" + "net/url" ) type PaywallsService struct{ c *Client } @@ -118,3 +119,105 @@ func (s *PaywallsService) Unpublish(ctx context.Context, projectID, id string) ( func (s *PaywallsService) Delete(ctx context.Context, projectID, id string) error { return s.c.do(ctx, http.MethodDelete, pathPaywall(projectID, id), nil, nil) } + +// PaywallGraph is the envelope from GET .../paywalls/{id}/graph. Graph is nil +// only for a genuinely standalone V2 paywall with no screen graph. +// +// Hand-written, not generated: this route isn't in the vendored OpenAPI spec +// yet, so it's listed in scripts/gen-paths.py's NON_SPEC_PATHS instead of +// getting a generated type. +type PaywallGraph struct { + Object string `json:"object"` + ID string `json:"id"` + Version string `json:"version"` + Graph *Graph `json:"graph"` +} + +// Graph describes a paywall's screens and how they connect. PaywallStepID is +// the authoritative purchase screen; a step's IsTerminal means "no outgoing +// edges", not "is the purchase screen" — do not conflate the two. +type Graph struct { + Revision *int `json:"revision"` + InitialStepID *string `json:"initial_step_id"` + PaywallStepID string `json:"paywall_step_id"` + TotalSteps int `json:"total_steps"` + Steps []GraphStep `json:"steps"` +} + +// GraphStep is one node in the graph. Paywall carries the step's editable +// content and is present only when the graph is fetched with +// expand=graph.steps.paywall. +type GraphStep struct { + ID string `json:"id"` + Name *string `json:"name"` + Type string `json:"type"` + ScreenTypes []string `json:"screen_types"` + IsTerminal bool `json:"is_terminal"` + PaywallID *string `json:"paywall_id"` + Edges []GraphEdge `json:"edges"` + UnwiredTriggers []UnwiredTrigger `json:"unwired_triggers"` + Paywall *ScreenContent `json:"paywall,omitempty"` +} + +type GraphEdge struct { + To string `json:"to"` + TriggerID string `json:"trigger_id"` + Condition json.RawMessage `json:"condition"` + IsDefault bool `json:"is_default"` + TriggerComponentID *string `json:"trigger_component_id"` + TriggerName *string `json:"trigger_name"` + TriggerType *string `json:"trigger_type"` +} + +type UnwiredTrigger struct { + ActionID string `json:"action_id"` + ComponentID *string `json:"component_id"` + Name *string `json:"name"` + Type *string `json:"type"` + Reason string `json:"reason"` +} + +// ScreenContent is a graph step's editable content — the same fields the +// dashboard's paywall editor reads and writes, scoped to one screen. +type ScreenContent struct { + ID string `json:"id"` + Revision int `json:"revision"` + ComponentsConfig json.RawMessage `json:"components_config"` + ComponentsLocalizations json.RawMessage `json:"components_localizations"` + DefaultLocale string `json:"default_locale"` + StateDeclarations json.RawMessage `json:"state_declarations"` +} + +// GetGraph fetches a paywall's screen topology (no screen content). version +// is "draft" or "published". +func (s *PaywallsService) GetGraph(ctx context.Context, projectID, id, version string) (*PaywallGraph, error) { + path := encodePath("projects", projectID, "paywalls", id, "graph") + "?version=" + url.QueryEscape(version) + var out PaywallGraph + err := s.c.do(ctx, http.MethodGet, path, nil, &out) + return &out, err +} + +// GetGraphWithScreenContent fetches the graph with each step's editable +// content included — the only way to read a sibling screen's content; the +// plain paywall GET can only ever return the fallback screen's own version. +func (s *PaywallsService) GetGraphWithScreenContent(ctx context.Context, projectID, id, version string) (*PaywallGraph, error) { + path := encodePath("projects", projectID, "paywalls", id, "graph") + + "?version=" + url.QueryEscape(version) + "&expand=graph.steps.paywall" + var out PaywallGraph + err := s.c.do(ctx, http.MethodGet, path, nil, &out) + return &out, err +} + +// UpdateDraftStep saves component state onto one screen selected from the +// paywall's graph instead of onto its fallback draft. parentID stays in the +// 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. +func (s *PaywallsService) UpdateDraftStep(ctx context.Context, projectID, parentID, stepID string, body PaywallDraftUpdate) (*Paywall, error) { + body.Name = nil + path := pathPaywall(projectID, parentID) + "?step_id=" + url.QueryEscape(stepID) + var out Paywall + err := s.c.do(ctx, http.MethodPatch, path, body, &out) + return &out, err +} diff --git a/internal/api/paywalls_test.go b/internal/api/paywalls_test.go index 839d1c8e..fad59883 100644 --- a/internal/api/paywalls_test.go +++ b/internal/api/paywalls_test.go @@ -110,6 +110,123 @@ func TestPaywallsSetOffering(t *testing.T) { } } +func TestPaywallsGetGraph(t *testing.T) { + tests := []struct { + name string + fetch func(client *api.Client) (*api.PaywallGraph, error) + wantQuery string + response string + wantGraph bool + wantSteps int + wantScreen bool // first step's Paywall content present + }{ + { + name: "draft, no expand", + fetch: func(client *api.Client) (*api.PaywallGraph, error) { + return client.Paywalls.GetGraph(context.Background(), "proj", "pw", "draft") + }, + wantQuery: "version=draft", + response: `{"object":"paywall_graph","id":"pw","version":"draft","graph":{"revision":3,"initial_step_id":"step_1","paywall_step_id":"step_1","total_steps":1,"steps":[{"id":"step_1","name":"Purchase","type":"screen","screen_types":["paywall"],"is_terminal":true,"paywall_id":"pw","edges":[],"unwired_triggers":[]}]}}`, + wantGraph: true, + wantSteps: 1, + }, + { + name: "published, expanded", + fetch: func(client *api.Client) (*api.PaywallGraph, error) { + return client.Paywalls.GetGraphWithScreenContent(context.Background(), "proj", "pw", "published") + }, + wantQuery: "version=published&expand=graph.steps.paywall", + response: `{"object":"paywall_graph","id":"pw","version":"published","graph":{"revision":3,"initial_step_id":"step_1","paywall_step_id":"step_1","total_steps":1,"steps":[{"id":"step_1","name":"Purchase","type":"screen","screen_types":["paywall"],"is_terminal":true,"paywall_id":"pw","edges":[],"unwired_triggers":[],"paywall":{"id":"pw","revision":5,"components_config":{},"components_localizations":{},"default_locale":"en_US","state_declarations":{}}}]}}`, + wantGraph: true, + wantSteps: 1, + wantScreen: true, + }, + { + name: "standalone paywall has a null graph", + fetch: func(client *api.Client) (*api.PaywallGraph, error) { + return client.Paywalls.GetGraph(context.Background(), "proj", "pw", "draft") + }, + wantQuery: "version=draft", + response: `{"object":"paywall_graph","id":"pw","version":"draft","graph":null}`, + wantGraph: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var gotPath, gotQuery string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath, gotQuery = r.URL.Path, r.URL.RawQuery + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, tt.response) + })) + t.Cleanup(srv.Close) + + client := api.NewClient(api.Options{APIKey: "sk_test", BaseURL: srv.URL}) + graph, err := tt.fetch(client) + if err != nil { + t.Fatal(err) + } + if gotPath != "/projects/proj/paywalls/pw/graph" { + t.Fatalf("path = %s", gotPath) + } + if gotQuery != tt.wantQuery { + t.Fatalf("query = %s, want %s", gotQuery, tt.wantQuery) + } + if (graph.Graph != nil) != tt.wantGraph { + t.Fatalf("graph = %+v, want present=%v", graph.Graph, tt.wantGraph) + } + if !tt.wantGraph { + return + } + if len(graph.Graph.Steps) != tt.wantSteps { + t.Fatalf("steps = %d, want %d", len(graph.Graph.Steps), tt.wantSteps) + } + if (graph.Graph.Steps[0].Paywall != nil) != tt.wantScreen { + t.Fatalf("step content present = %v, want %v", graph.Graph.Steps[0].Paywall != nil, tt.wantScreen) + } + }) + } +} + +func TestPaywallsUpdateDraftStep(t *testing.T) { + var gotPath, gotQuery string + var gotBody map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath, gotQuery = r.URL.Path, r.URL.RawQuery + _ = json.NewDecoder(r.Body).Decode(&gotBody) + w.Header().Set("Content-Type", "application/json") + // The response id is the sibling's own canonical id, deliberately + // different from the parent id in the path. + _, _ = io.WriteString(w, `{"id":"pw_sibling","created_at":1,"published_at":null,"object":"paywall"}`) + })) + t.Cleanup(srv.Close) + + client := api.NewClient(api.Options{APIKey: "sk_test", BaseURL: srv.URL}) + name := "should never be sent for a sibling" + updated, err := client.Paywalls.UpdateDraftStep(context.Background(), "proj", "pw_parent", "step_2", api.PaywallDraftUpdate{ + Revision: 5, + ComponentsConfig: json.RawMessage(`{}`), + ComponentsLocalizations: json.RawMessage(`{}`), + DefaultLocale: "en_US", + Name: &name, + }) + if err != nil { + t.Fatal(err) + } + if gotPath != "/projects/proj/paywalls/pw_parent" { + t.Fatalf("path = %s, want the parent's path, not the sibling's", gotPath) + } + if gotQuery != "step_id=step_2" { + t.Fatalf("query = %s, want step_id=step_2", gotQuery) + } + if _, present := gotBody["name"]; present { + t.Fatalf("body = %v, must never send name in selected mode", gotBody) + } + if updated.ID != "pw_sibling" { + t.Fatalf("response id = %s, want pw_sibling (callers must read it from the response, not assume it equals the path id)", updated.ID) + } +} + func TestPaywallsUnpublishPreservesDraftState(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost || r.URL.Path != "/projects/proj/paywalls/pw/actions/unpublish" { diff --git a/internal/cli/paywalls.go b/internal/cli/paywalls.go index e6c1cd1b..115e3171 100644 --- a/internal/cli/paywalls.go +++ b/internal/cli/paywalls.go @@ -62,6 +62,7 @@ URL, get the user's approval, then rc paywalls publish.`, cmd.AddCommand( newPaywallsListCmd(), newPaywallsShowCmd(), + newPaywallsScreensCmd(), newPaywallsGenerateCmd(), newPaywallsEditCmd(), newPaywallsRewindCmd(), @@ -413,6 +414,71 @@ builder URL instead — show returns metadata, not visuals.`, } } +// newPaywallsScreensCmd lists a paywall's graph steps — the id shown here is +// what --step-id on rc paywalls edit expects. Purchase-screen status comes +// from the graph's paywall_step_id, never from a step's is_terminal (that +// only means "no outgoing edges"). +func newPaywallsScreensCmd() *cobra.Command { + var version string + cmd := &cobra.Command{ + Use: "screens [paywall-id]", + Short: "List a Paywall's screens", + Long: `Lists the screens in a paywall's graph: id, name, position, and which one is +the purchase screen. Pass a screen's id to rc paywalls edit --step-id to edit +it instead of the default fallback screen. + +A standalone paywall (one with no screen graph) has no screens to list — it +is edited as a single screen with rc paywalls edit.`, + Example: ` rc paywalls screens pw_abc + rc paywalls screens pw_abc --version published --json`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + rt := RuntimeFrom(cmd.Context()) + projectID, err := requireProject(rt) + if err != nil { + return err + } + client, err := rt.API() + if err != nil { + return err + } + paywallID, err := requireID(rt, argAt(args, 0), "paywall", func() ([]PickerItem, error) { + return paywallPickerItems(cmd.Context(), client, projectID) + }) + if err != nil { + return err + } + graph, err := client.Paywalls.GetGraph(cmd.Context(), projectID, paywallID, version) + if err != nil { + return err + } + if graph.Graph == nil { + rt.Out.Info(fmt.Sprintf("Paywall %s is standalone — it has no screen graph; rc paywalls edit always edits its one screen.", paywallID)) + return rt.Out.Render(graph) + } + rows := make([][]string, len(graph.Graph.Steps)) + for i, step := range graph.Graph.Steps { + name := "—" + if step.Name != nil && *step.Name != "" { + name = *step.Name + } + purchase := "no" + if step.ID == graph.Graph.PaywallStepID { + purchase = "yes" + } + rows[i] = []string{fmt.Sprintf("%d", i+1), name, step.ID, purchase, step.Type} + } + return rt.Out.RenderTable(output.Table{ + Columns: []string{"POSITION", "NAME", "ID", "PURCHASE SCREEN", "TYPE"}, + Rows: rows, + Raw: graph, + }) + }, + } + cmd.Flags().StringVar(&version, "version", "draft", "graph version to inspect (draft or published)") + return cmd +} + func newPaywallsDeleteCmd() *cobra.Command { var force bool cmd := &cobra.Command{ diff --git a/internal/cli/paywalls_ai.go b/internal/cli/paywalls_ai.go index 648d1418..d9cb6a1a 100644 --- a/internal/cli/paywalls_ai.go +++ b/internal/cli/paywalls_ai.go @@ -25,9 +25,16 @@ import ( // the full paywall plus the opaque session blobs every turn, so the CLI // persists them here (the dashboard holds the same data in builder state). type paywallAISession struct { - Version int `json:"version"` - ProjectID string `json:"project_id"` - PaywallID string `json:"paywall_id"` + Version int `json:"version"` + ProjectID string `json:"project_id"` + PaywallID string `json:"paywall_id"` + // StepID selects a screen from PaywallID's graph; nil edits the draft + // fallback/purchase screen, matching pre-selection behavior exactly. + StepID *string `json:"step_id,omitempty"` + // TargetID is the id the last successful save's PATCH response returned — + // the selected screen's own canonical id, which can differ from + // PaywallID once StepID selects a sibling. Never assume it equals PaywallID. + TargetID string `json:"target_id,omitempty"` SessionID string `json:"session_id,omitempty"` TraceID string `json:"trace_id,omitempty"` Revision *int `json:"revision"` @@ -47,7 +54,10 @@ func screenshotBase(sessionPath string) string { return strings.TrimSuffix(sessionPath, filepath.Ext(sessionPath)) } -func defaultPaywallSessionPath(projectID, paywallID string) (string, error) { +// defaultPaywallSessionPath namespaces the session file by stepID so editing +// two different screens of the same paywall never share (and clobber) one +// session file. +func defaultPaywallSessionPath(projectID, paywallID, stepID string) (string, error) { dir, err := config.Dir() if err != nil { return "", err @@ -56,7 +66,11 @@ func defaultPaywallSessionPath(projectID, paywallID string) (string, error) { if err := os.MkdirAll(sessionDir, 0o700); err != nil { return "", err } - return filepath.Join(sessionDir, "session"+paywallSessionSuffix), nil + name := "session" + paywallSessionSuffix + if stepID != "" { + name = "session." + stepID + paywallSessionSuffix + } + return filepath.Join(sessionDir, name), nil } func pickOfferingOrStandalone(ctx context.Context, rt *Runtime, client *api.Client, projectID string) (string, error) { @@ -103,6 +117,7 @@ type paywallAIOptions struct { offeringID string name string sessionPath string + stepID string images []string baseURL string timeout time.Duration @@ -227,7 +242,7 @@ screenshots via --image, audience via --context.`, SessionItems: json.RawMessage(`{}`), } if opts.sessionPath == "" { - opts.sessionPath, err = defaultPaywallSessionPath(projectID, paywall.ID) + opts.sessionPath, err = defaultPaywallSessionPath(projectID, paywall.ID, "") if err != nil { return err } @@ -308,6 +323,9 @@ Using it well: Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { rt := RuntimeFrom(cmd.Context()) + if opts.stepID != "" && opts.sessionPath != "" { + return fmt.Errorf("--step-id cannot be combined with --session; the session file already selects a screen") + } var session *paywallAISession var err error switch { @@ -331,9 +349,9 @@ Using it well: if perr != nil { return perr } - opts.sessionPath, err = defaultPaywallSessionPath(projectID, paywallID) + opts.sessionPath, err = defaultPaywallSessionPath(projectID, paywallID, opts.stepID) if err == nil { - session, err = resumeOrSeedSession(cmd.Context(), rt, projectID, paywallID, opts.sessionPath) + session, err = resumeOrSeedSession(cmd.Context(), rt, projectID, paywallID, opts.stepID, opts.sessionPath) } default: return fmt.Errorf("pass a paywall ID or --session ") @@ -348,6 +366,7 @@ Using it well: }, } addPaywallAIFlags(cmd, &opts) + cmd.Flags().StringVar(&opts.stepID, "step-id", "", "screen to edit, from rc paywalls screens — default edits the fallback/purchase screen") return cmd } @@ -361,45 +380,169 @@ func preflightSessionRevision(ctx context.Context, rt *Runtime, session *paywall if err != nil { return nil, err } - version, err := currentDraftVersion(ctx, client, session.ProjectID, session.PaywallID) + target, err := fetchTargetState(ctx, client, session) if err != nil { return nil, err } - if *version.Revision == *session.Revision { - hydrateStateDeclarations(session, version) + if target.revision == *session.Revision { + hydrateStateDeclarations(session, target.stateDeclarations) return session, nil } - rt.Out.Warn(fmt.Sprintf("The draft for %s changed outside this session — the dashboard, its AI editor, or the API wrote revision %d, the session has %d.", session.PaywallID, *version.Revision, *session.Revision)) + rt.Out.Warn(fmt.Sprintf("The draft for %s changed outside this session — the dashboard, its AI editor, or the API wrote revision %d, the session has %d.", session.PaywallID, target.revision, *session.Revision)) rt.Out.Info("A session can't continue against diverged state. Continuing starts fresh from the server's current draft; the conversation context in this session file is lost.") if err := confirmOrAbort(rt, "Start fresh from the server's current draft?", - "run rc paywalls edit "+session.PaywallID+" to start fresh deliberately"); err != nil { + "run rc paywalls edit "+session.PaywallID+stepIDFlagSuffix(session.StepID)+" to start fresh deliberately"); err != nil { return nil, err } - return seedSessionFromServer(ctx, rt, session.ProjectID, session.PaywallID) + return seedSession(ctx, rt, session.ProjectID, session.PaywallID, stepIDValue(session.StepID)) } // resumeOrSeedSession reuses the default-path session for an `edit` turn without -// an explicit --session when it still matches the server's draft, else seeds fresh. -func resumeOrSeedSession(ctx context.Context, rt *Runtime, projectID, paywallID, sessionPath string) (*paywallAISession, error) { +// an explicit --session when it still matches the server's draft and selects the +// same screen, else seeds fresh. +func resumeOrSeedSession(ctx context.Context, rt *Runtime, projectID, paywallID, stepID, sessionPath string) (*paywallAISession, error) { stored, err := loadPaywallAISession(rt, sessionPath) - if err != nil { - return seedSessionFromServer(ctx, rt, projectID, paywallID) + if err != nil || !stepIDMatches(stored.StepID, stepID) { + return seedSession(ctx, rt, projectID, paywallID, stepID) } client, err := rt.API() if err != nil { return nil, err } - version, err := currentDraftVersion(ctx, client, projectID, paywallID) + target, err := fetchTargetState(ctx, client, stored) if err != nil { return nil, err } - if stored.Revision != nil && *version.Revision == *stored.Revision { - hydrateStateDeclarations(stored, version) + if stored.Revision != nil && target.revision == *stored.Revision { + hydrateStateDeclarations(stored, target.stateDeclarations) return stored, nil } + return seedSession(ctx, rt, projectID, paywallID, stepID) +} + +// seedSession seeds from stepID's screen when set, otherwise the draft +// fallback — the single dispatch point every reseed path shares. +func seedSession(ctx context.Context, rt *Runtime, projectID, paywallID, stepID string) (*paywallAISession, error) { + if stepID != "" { + return seedSessionFromServerForStep(ctx, rt, projectID, paywallID, stepID) + } return seedSessionFromServer(ctx, rt, projectID, paywallID) } +// stepIDMatches reports whether a stored session's selection matches the +// requested one; "" (no --step-id) only matches an unselected session. +func stepIDMatches(stored *string, requested string) bool { + if requested == "" { + return stored == nil + } + return stored != nil && *stored == requested +} + +func stepIDValue(stepID *string) string { + if stepID == nil { + return "" + } + return *stepID +} + +func stepIDFlagSuffix(stepID *string) string { + if stepID == nil { + return "" + } + return " --step-id " + *stepID +} + +// targetState is the live revision (and state declarations, for hydration) +// of whatever a session currently targets. +type targetState struct { + revision int + stateDeclarations json.RawMessage +} + +// fetchTargetState reads the live state a session's next save would be +// guarded against: the selected sibling's own content row via the graph, or +// the parent draft/published version. +func fetchTargetState(ctx context.Context, client *api.Client, session *paywallAISession) (*targetState, error) { + if session.StepID != nil { + step, err := fetchEditableGraphStep(ctx, client, session.ProjectID, session.PaywallID, *session.StepID) + if err != nil { + return nil, err + } + return &targetState{revision: step.Paywall.Revision, stateDeclarations: step.Paywall.StateDeclarations}, nil + } + version, err := currentDraftVersion(ctx, client, session.ProjectID, session.PaywallID) + if err != nil { + return nil, err + } + return &targetState{revision: *version.Revision, stateDeclarations: version.StateDeclarations}, nil +} + +// fetchEditableGraphStep resolves stepID against paywallID's draft graph and +// returns it with content loaded, or a clear error — never a silent fallback +// to the purchase/default screen. Explicit selection either finds exactly +// what was asked for or fails. +func fetchEditableGraphStep(ctx context.Context, client *api.Client, projectID, paywallID, stepID string) (*api.GraphStep, error) { + graph, err := client.Paywalls.GetGraphWithScreenContent(ctx, projectID, paywallID, "draft") + if err != nil { + return nil, err + } + if graph.Graph == nil { + return nil, fmt.Errorf("paywall %s is standalone (it has no screen graph) — --step-id is not supported for it", paywallID) + } + for i := range graph.Graph.Steps { + step := &graph.Graph.Steps[i] + if step.ID != stepID { + continue + } + if step.Type != "screen" || step.PaywallID == nil || step.Paywall == nil { + return nil, fmt.Errorf("screen %s has no editable content — pick an editable screen with rc paywalls screens %s", stepID, paywallID) + } + return step, nil + } + return nil, fmt.Errorf("paywall %s has no screen %s — list its screens with rc paywalls screens %s", paywallID, stepID, paywallID) +} + +// seedSessionFromServerForStep starts an editor session from one screen's +// content, read from the graph — the plain paywall GET can only ever return +// the fallback screen's own version, never a sibling's. +func seedSessionFromServerForStep(ctx context.Context, rt *Runtime, projectID, paywallID, stepID string) (*paywallAISession, error) { + client, err := rt.API() + if err != nil { + return nil, err + } + step, err := fetchEditableGraphStep(ctx, client, projectID, paywallID, stepID) + if err != nil { + return nil, err + } + content := step.Paywall + locale := content.DefaultLocale + if locale == "" { + locale = "en_US" + } + localizations := content.ComponentsLocalizations + if len(localizations) == 0 { + localizations = json.RawMessage(`{"` + locale + `": {}}`) + } + revision := content.Revision + return &paywallAISession{ + Version: 1, + ProjectID: projectID, + PaywallID: paywallID, + StepID: &stepID, + TargetID: content.ID, + Revision: &revision, + Paywall: paywallai.PaywallData{ + DefaultLocale: locale, + ComponentsConfig: content.ComponentsConfig, + ComponentsLocalizations: localizations, + StateDeclarations: serverStateDeclarations(content.StateDeclarations), + }, + UIConfig: json.RawMessage(minimalUIConfig), + ProductVariables: map[string]string{}, + SessionItems: json.RawMessage(`{}`), + }, nil +} + // seedSessionFromServer starts an editor session from the paywall's current // RevenueCat state (draft components, falling back to published). func seedSessionFromServer(ctx context.Context, rt *Runtime, projectID string, paywallID string) (*paywallAISession, error) { @@ -449,7 +592,7 @@ func seedSessionFromServer(ctx context.Context, rt *Runtime, projectID string, p OfferingID: offeringID, ComponentsConfig: version.ComponentsConfig, ComponentsLocalizations: localizations, - StateDeclarations: serverStateDeclarations(version), + StateDeclarations: serverStateDeclarations(version.StateDeclarations), }, UIConfig: json.RawMessage(minimalUIConfig), ProductVariables: map[string]string{}, @@ -533,6 +676,7 @@ func runPaywallAI(ctx context.Context, rt *Runtime, opts paywallAIOptions, sessi stream, err := client.Stream(ctx, paywallai.EditorRequest{ ProjectID: session.ProjectID, PaywallID: session.PaywallID, + StepID: session.StepID, Revision: session.Revision, SessionID: session.SessionID, Paywall: session.Paywall, @@ -616,14 +760,14 @@ func applySessionEvent(session *paywallAISession, event *paywallai.Event) { // CLI from before they existed, so the editor can round-trip them again. The // server's value, not {}: the stored draft may hold dashboard-authored // declarations that an empty replacement would wipe. -func hydrateStateDeclarations(session *paywallAISession, version *api.PaywallComponentsVersion) { +func hydrateStateDeclarations(session *paywallAISession, serverValue json.RawMessage) { if presentJSON(session.Paywall.StateDeclarations) == nil { - session.Paywall.StateDeclarations = serverStateDeclarations(version) + session.Paywall.StateDeclarations = serverStateDeclarations(serverValue) } } -func serverStateDeclarations(version *api.PaywallComponentsVersion) json.RawMessage { - if declarations := presentJSON(version.StateDeclarations); declarations != nil { +func serverStateDeclarations(raw json.RawMessage) json.RawMessage { + if declarations := presentJSON(raw); declarations != nil { return declarations } return json.RawMessage(`{}`) @@ -670,10 +814,17 @@ func finishPaywallAI(ctx context.Context, rt *Runtime, opts paywallAIOptions, se saved := false if err := persistPaywallDesign(ctx, rt, session); err != nil { var apiErr *api.APIError - if errors.As(err, &apiErr) && apiErr.Status == 409 { + switch { + case errors.As(err, &apiErr) && apiErr.Status == 409: rt.Out.Warn("Could not save the design: the draft changed during the run (dashboard, its AI editor, or API), and this session can't be saved over it.") - rt.Out.Hint("Start fresh from the current draft: rc paywalls edit " + session.PaywallID) - } else { + rt.Out.Hint("Start fresh from the current draft: rc paywalls edit " + session.PaywallID + stepIDFlagSuffix(session.StepID)) + case errors.As(err, &apiErr) && apiErr.Status == 422 && session.StepID != nil: + rt.Out.Warn("Could not save the design: screen " + *session.StepID + " is no longer editable.") + rt.Out.Hint("Pick an editable screen: rc paywalls screens " + session.PaywallID) + case errors.As(err, &apiErr) && apiErr.Status == 404: + rt.Out.Warn("Could not save the design: paywall " + session.PaywallID + " no longer exists.") + rt.Out.Hint("The design is safe in " + opts.sessionPath + ", but this save can't be retried against a paywall that's gone.") + default: rt.Out.Warn("Could not save the design to RevenueCat: " + err.Error()) rt.Out.Hint("The design is safe in " + opts.sessionPath + " — re-run rc paywalls edit to retry saving.") } @@ -684,7 +835,11 @@ func finishPaywallAI(ctx context.Context, rt *Runtime, opts paywallAIOptions, se if err := savePaywallAISession(opts.sessionPath, session); err != nil { return err } - rt.Out.Success("Design saved to paywall draft " + session.PaywallID) + if session.StepID != nil { + rt.Out.Success(fmt.Sprintf("Design saved to screen %s on paywall %s", session.TargetID, session.PaywallID)) + } else { + rt.Out.Success("Design saved to paywall draft " + session.PaywallID) + } if errored := countErroredActivity(event.Activity); errored > 0 { rt.Out.Info(fmt.Sprintf("%d editor step(s) errored during the run and were retried by the Paywalls AI Editor — the saved draft is the complete final state (nothing partial is ever saved).", errored)) } @@ -697,15 +852,18 @@ func finishPaywallAI(ctx context.Context, rt *Runtime, opts paywallAIOptions, se } rt.Out.Field("View it", paywallBuilderURL(session.ProjectID, session.PaywallID)) rt.Out.Field("Keep designing", "rc paywalls edit --session "+opts.sessionPath) - if session.Paywall.OfferingID == nil { - rt.Out.Field("Attach it", "rc paywalls attach "+session.PaywallID+" ") - } else { - rt.Out.Field("Publish when ready", "rc paywalls publish "+session.PaywallID) + if session.StepID == nil { + if session.Paywall.OfferingID == nil { + rt.Out.Field("Attach it", "rc paywalls attach "+session.PaywallID+" ") + } else { + rt.Out.Field("Publish when ready", "rc paywalls publish "+session.PaywallID) + } } } if rt.Out.IsJSON() { return rt.Out.Render(map[string]any{ "paywall_id": session.PaywallID, + "target_id": session.TargetID, "dashboard_url": paywallBuilderURL(session.ProjectID, session.PaywallID), "session_id": session.SessionID, "trace_id": session.TraceID, @@ -752,8 +910,9 @@ func paywallBuilderURL(projectID, paywallID string) string { } // persistPaywallDesign PATCHes the designed components onto the RevenueCat -// paywall draft, guarded by the session's own revision: a draft that changed -// outside the session comes back as a 409 instead of being overwritten. +// paywall draft (or, when StepID is set, onto that selected screen), guarded +// by the session's own revision: a draft that changed outside the session +// comes back as a 409 instead of being overwritten. func persistPaywallDesign(ctx context.Context, rt *Runtime, session *paywallAISession) error { client, err := rt.API() if err != nil { @@ -768,20 +927,33 @@ func persistPaywallDesign(ctx context.Context, rt *Runtime, session *paywallAISe // Always the session's own revision — refetching a fresh one here would // sail past the conflict guard and clobber out-of-band changes. update.Revision = *session.Revision - updated, err := client.Paywalls.UpdateDraft(ctx, session.ProjectID, session.PaywallID, update) + var updated *api.Paywall + if session.StepID != nil { + updated, err = client.Paywalls.UpdateDraftStep(ctx, session.ProjectID, session.PaywallID, *session.StepID, update) + } else { + updated, err = client.Paywalls.UpdateDraft(ctx, session.ProjectID, session.PaywallID, update) + } if err != nil { return err } + // The response id is the saved screen's own canonical id — for a selected + // sibling it differs from session.PaywallID (the parent), so it must come + // from here, never be assumed to equal the pre-edit input id. + session.TargetID = updated.ID if updated.Components != nil && updated.Components.Draft != nil && updated.Components.Draft.Revision != nil { session.Revision = updated.Components.Draft.Revision } - // Offering attachment can change out-of-band (dashboard); the PATCH - // response carries current server truth, so refresh it — it drives - // the attach/publish hint and the editor's product context next turn. - if updated.OfferingID != "" { - session.Paywall.OfferingID = &updated.OfferingID - } else { - session.Paywall.OfferingID = nil + if session.StepID == nil { + // Offering attachment can change out-of-band (dashboard); the PATCH + // response carries current server truth, so refresh it — it drives + // the attach/publish hint and the editor's product context next turn. + // A sibling screen has no offering of its own, so this stays untouched + // in selected mode rather than being cleared by an empty response field. + if updated.OfferingID != "" { + session.Paywall.OfferingID = &updated.OfferingID + } else { + session.Paywall.OfferingID = nil + } } return nil } diff --git a/internal/cli/paywalls_ai_step_test.go b/internal/cli/paywalls_ai_step_test.go new file mode 100644 index 00000000..db599a87 --- /dev/null +++ b/internal/cli/paywalls_ai_step_test.go @@ -0,0 +1,257 @@ +package cli + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/revenuecat/cli/internal/config" + "github.com/revenuecat/cli/internal/output" +) + +// rcGraphMock serves a two-screen graph (a terminal, non-fallback step_1 and +// a fallback step_2) plus a PATCH endpoint that records the query string and +// body it received. It fails the test if the plain (non-graph) paywall GET +// route is ever hit — the whole point of --step-id is that a sibling's +// content can no longer be reached that way. +type rcGraphMock struct { + mu sync.Mutex + patchedQuery []string + patchedBody []map[string]any + // patchStatus, when non-zero, makes the PATCH endpoint fail with this + // status instead of succeeding. + patchStatus int +} + +func (m *rcGraphMock) server(t *testing.T) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/graph"): + io.WriteString(w, `{"object":"paywall_graph","id":"pw_parent","version":"draft","graph":{ + "revision":9,"initial_step_id":"step_1","paywall_step_id":"step_1","total_steps":2, + "steps":[ + {"id":"step_1","name":"Purchase","type":"screen","screen_types":["paywall"],"is_terminal":true,"paywall_id":"pw_parent","edges":[],"unwired_triggers":[]}, + {"id":"step_2","name":"Intro","type":"screen","screen_types":["generic"],"is_terminal":false,"paywall_id":"pw_sibling","edges":[],"unwired_triggers":[], + "paywall":{"id":"pw_sibling","revision":5,"components_config":{"base":{}},"components_localizations":{"en_US":{}},"default_locale":"en_US","state_declarations":{}}} + ]}}`) + case r.Method == http.MethodPatch: + m.mu.Lock() + m.patchedQuery = append(m.patchedQuery, r.URL.RawQuery) + var body map[string]any + _ = json.NewDecoder(r.Body).Decode(&body) + m.patchedBody = append(m.patchedBody, body) + status := m.patchStatus + m.mu.Unlock() + if status != 0 { + w.WriteHeader(status) + io.WriteString(w, `{"type":"conflict","message":"draft changed"}`) + return + } + // The response id is the sibling's own canonical id — different + // from pw_parent, the path/parent id. + io.WriteString(w, `{"id":"pw_sibling","offering_id":"","created_at":1,"components":{"published":null,"draft":{"revision":6,"components_config":{"base":{}},"components_localizations":{"en_US":{}},"default_locale":"en_US"}}}`) + default: + t.Errorf("unexpected request that should have gone through the graph instead: %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) +} + +// stepEchoEditorServer records the step_id each editor request carried and +// completes the turn immediately. +type stepEchoEditorServer struct { + mu sync.Mutex + stepIDs []string + componentsConfig []string +} + +func (s *stepEchoEditorServer) server(t *testing.T) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + StepID *string `json:"step_id"` + Paywall struct { + ComponentsConfig json.RawMessage `json:"components_config"` + } `json:"paywall"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + s.mu.Lock() + if body.StepID != nil { + s.stepIDs = append(s.stepIDs, *body.StepID) + } else { + s.stepIDs = append(s.stepIDs, "") + } + s.componentsConfig = append(s.componentsConfig, string(body.Paywall.ComponentsConfig)) + s.mu.Unlock() + + w.Header().Set("Content-Type", "text/event-stream") + paywall := `{"default_locale":"en_US","offering_id":null,"components_config":{"designed":true},"components_localizations":{"en_US":{}}}` + fmt.Fprint(w, "data: {\"type\":\"run.started\",\"session_id\":\"sess1\"}\n\n") + fmt.Fprintf(w, "data: {\"type\":\"turn.snapshot\",\"session_id\":\"sess1\",\"turn_index\":0,\"paywall\":%s,\"activity\":[]}\n\n", paywall) + fmt.Fprintf(w, "data: {\"type\":\"run.completed\",\"session_id\":\"sess1\",\"trace_id\":\"tr1\",\"paywall\":%s,\"activity\":[]}\n\n", paywall) + })) +} + +func TestPaywallsEdit_StepIDFetchesFromGraphAndSavesToSelectedStep(t *testing.T) { + rc := &rcGraphMock{} + rcServer := rc.server(t) + defer rcServer.Close() + editor := &stepEchoEditorServer{} + editorServer := editor.server(t) + defer editorServer.Close() + + t.Setenv("RC_CONFIG_DIR", t.TempDir()) + t.Setenv("RC_PAYWALL_AI_BASE_URL", editorServer.URL) + cmd := newPaywallsEditCmd() + var stdout bytes.Buffer + rt := &Runtime{ + Globals: &Globals{JSON: true, NoInput: true, Version: "test"}, + Config: &config.Config{APIKey: "sk_test", ProjectID: "proj", BaseURL: rcServer.URL}, + Ctx: context.Background(), + Out: output.NewRenderer(&stdout, io.Discard, true, true, false, ""), + } + cmd.SetContext(WithRuntime(context.Background(), rt)) + cmd.SetOut(&stdout) + cmd.SetErr(io.Discard) + cmd.SetArgs([]string{"pw_parent", "--step-id", "step_2", "--prompt", "make it pop"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("edit turn failed: %v, stdout=%s", err, stdout.String()) + } + + editor.mu.Lock() + stepIDs := append([]string(nil), editor.stepIDs...) + editor.mu.Unlock() + if len(stepIDs) != 1 || stepIDs[0] != "step_2" { + t.Fatalf("EditorRequest.step_id = %v, want [step_2]", stepIDs) + } + + editor.mu.Lock() + sentConfig := append([]string(nil), editor.componentsConfig...) + editor.mu.Unlock() + if len(sentConfig) != 1 || sentConfig[0] != `{"base":{}}` { + t.Fatalf("editor received components_config = %v, want the sibling step_2's own content", sentConfig) + } + + rc.mu.Lock() + queries := append([]string(nil), rc.patchedQuery...) + rc.mu.Unlock() + if len(queries) != 1 || queries[0] != "step_id=step_2" { + t.Fatalf("PATCH query = %v, want [step_id=step_2] against the parent path", queries) + } + + var envelope struct { + Data struct { + PaywallID string `json:"paywall_id"` + TargetID string `json:"target_id"` + } `json:"data"` + } + if err := json.Unmarshal(stdout.Bytes(), &envelope); err != nil { + t.Fatalf("decoding output: %v\n%s", err, stdout.String()) + } + if envelope.Data.PaywallID != "pw_parent" { + t.Fatalf("paywall_id = %q, want the parent pw_parent", envelope.Data.PaywallID) + } + if envelope.Data.TargetID != "pw_sibling" { + t.Fatalf("target_id = %q, want the PATCH response's own id pw_sibling, not the pre-edit parent id", envelope.Data.TargetID) + } +} + +// The recovery hint on a failed save must point back at the selected screen, +// not at the paywall's fallback — running the bare hint command would +// otherwise silently reseed and edit the purchase screen instead. +func TestPaywallsEdit_StepIDConflictHintKeepsStepSelection(t *testing.T) { + rc := &rcGraphMock{patchStatus: http.StatusConflict} + rcServer := rc.server(t) + defer rcServer.Close() + editor := &stepEchoEditorServer{} + editorServer := editor.server(t) + defer editorServer.Close() + + t.Setenv("RC_CONFIG_DIR", t.TempDir()) + t.Setenv("RC_PAYWALL_AI_BASE_URL", editorServer.URL) + cmd := newPaywallsEditCmd() + var stdout, stderr bytes.Buffer + rt := &Runtime{ + Globals: &Globals{NoInput: true, Version: "test"}, + Config: &config.Config{APIKey: "sk_test", ProjectID: "proj", BaseURL: rcServer.URL}, + Ctx: context.Background(), + Out: output.NewRenderer(&stdout, &stderr, false, true, false, ""), + } + cmd.SetContext(WithRuntime(context.Background(), rt)) + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"pw_parent", "--step-id", "step_2", "--prompt", "make it pop"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("edit turn failed: %v, stderr=%s", err, stderr.String()) + } + + if !strings.Contains(stderr.String(), "rc paywalls edit pw_parent --step-id step_2") { + t.Fatalf("hint must re-run with the same --step-id, got stderr:\n%s", stderr.String()) + } +} + +// An unresolvable --step-id must fail clearly and must never silently +// retarget the purchase/fallback screen. +func TestSeedSessionFromServerForStep_UnknownStepFailsClearly(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + io.WriteString(w, `{"object":"paywall_graph","id":"pw","version":"draft","graph":{ + "revision":1,"initial_step_id":"step_1","paywall_step_id":"step_1","total_steps":1, + "steps":[{"id":"step_1","name":"Purchase","type":"screen","screen_types":["paywall"],"is_terminal":true,"paywall_id":"pw","edges":[],"unwired_triggers":[], + "paywall":{"id":"pw","revision":1,"components_config":{},"components_localizations":{},"default_locale":"en_US","state_declarations":{}}}]}}`) + })) + defer server.Close() + + rt := newSessionTestRuntime(server.URL) + session, err := seedSessionFromServerForStep(context.Background(), rt, "proj", "pw", "step_does_not_exist") + if err == nil { + t.Fatalf("expected an error, got session %+v", session) + } + if !strings.Contains(err.Error(), "step_does_not_exist") { + t.Fatalf("error should name the unresolvable step id: %v", err) + } +} + +// A standalone paywall (graph: null) must reject --step-id rather than +// silently editing the fallback as if no step had been selected. +func TestSeedSessionFromServerForStep_StandalonePaywallFailsClearly(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + io.WriteString(w, `{"object":"paywall_graph","id":"pw_solo","version":"draft","graph":null}`) + })) + defer server.Close() + + rt := newSessionTestRuntime(server.URL) + session, err := seedSessionFromServerForStep(context.Background(), rt, "proj", "pw_solo", "step_1") + if err == nil { + t.Fatalf("expected an error, got session %+v", session) + } + if !strings.Contains(err.Error(), "standalone") { + t.Fatalf("error should explain the paywall is standalone: %v", err) + } +} + +// --step-id combined with --session is rejected outright rather than +// guessing which selection wins. +func TestPaywallsEdit_StepIDConflictsWithSession(t *testing.T) { + t.Setenv("RC_CONFIG_DIR", t.TempDir()) + cmd := newPaywallsEditCmd() + rt := newSessionTestRuntime("https://rc.invalid") + cmd.SetContext(WithRuntime(context.Background(), rt)) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs([]string{"--session", "some-file.json", "--step-id", "step_2", "--prompt", "x"}) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "--step-id") { + t.Fatalf("err = %v, want a clear rejection of --step-id with --session", err) + } +} diff --git a/internal/cli/paywalls_screens_test.go b/internal/cli/paywalls_screens_test.go new file mode 100644 index 00000000..4a1955d0 --- /dev/null +++ b/internal/cli/paywalls_screens_test.go @@ -0,0 +1,75 @@ +package cli + +import ( + "bytes" + "context" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func runScreens(t *testing.T, serverURL string, args ...string) (stdout, stderr string, err error) { + t.Helper() + t.Setenv("RC_CONFIG_DIR", t.TempDir()) + t.Setenv("RC_BASE_URL", serverURL) + root := NewRootCmd("test") + var out, errb bytes.Buffer + root.SetOut(&out) + root.SetErr(&errb) + root.SetArgs(append([]string{"paywalls", "screens"}, append(args, "--api-key", "sk_test", "--project-id", "proj")...)) + err = root.ExecuteContext(context.Background()) + return out.String(), errb.String(), err +} + +// The purchase screen must come from the graph's paywall_step_id, never from +// a step's is_terminal — step_1 here is terminal but step_2 is the fallback. +func TestPaywallsScreens_MarksPurchaseScreenFromPaywallStepID(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/projects/proj/paywalls/pw_abc/graph" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + io.WriteString(w, `{"object":"paywall_graph","id":"pw_abc","version":"draft","graph":{ + "revision":3,"initial_step_id":"step_1","paywall_step_id":"step_2","total_steps":2, + "steps":[ + {"id":"step_1","name":"Intro","type":"screen","screen_types":["generic"],"is_terminal":true,"paywall_id":"pw_intro","edges":[],"unwired_triggers":[]}, + {"id":"step_2","name":"Purchase","type":"screen","screen_types":["paywall"],"is_terminal":false,"paywall_id":"pw_abc","edges":[],"unwired_triggers":[]} + ]}}`) + })) + defer server.Close() + + stdout, _, err := runScreens(t, server.URL, "pw_abc") + if err != nil { + t.Fatalf("err = %v, stdout = %s", err, stdout) + } + lines := strings.Split(strings.TrimSpace(stdout), "\n") + if len(lines) != 3 { // header + 2 rows + t.Fatalf("expected a header and 2 rows, got:\n%s", stdout) + } + if !strings.Contains(lines[1], "step_1") || strings.Contains(lines[1], " yes") { + t.Fatalf("step_1 (terminal, not the fallback) must not be marked purchase: %s", lines[1]) + } + if !strings.Contains(lines[2], "step_2") || !strings.Contains(lines[2], "yes") { + t.Fatalf("step_2 (the graph's paywall_step_id) must be marked purchase: %s", lines[2]) + } +} + +// A standalone V2 paywall reports graph: null; screens must say so plainly +// rather than crashing or fabricating a row. +func TestPaywallsScreens_StandaloneHasNoGraph(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + io.WriteString(w, `{"object":"paywall_graph","id":"pw_solo","version":"draft","graph":null}`) + })) + defer server.Close() + + stdout, stderr, err := runScreens(t, server.URL, "pw_solo") + if err != nil { + t.Fatalf("err = %v", err) + } + if !strings.Contains(stdout+stderr, "standalone") { + t.Fatalf("expected a standalone explanation, got stdout:\n%s\nstderr:\n%s", stdout, stderr) + } +} diff --git a/internal/paywallai/paywallai.go b/internal/paywallai/paywallai.go index 214ffdf3..9ea6ae30 100644 --- a/internal/paywallai/paywallai.go +++ b/internal/paywallai/paywallai.go @@ -49,8 +49,11 @@ type InputAttachment struct { // EditorRequest is the POST /editor/v1/stream body. UIConfig, SessionItems, // and AppContext are opaque server round-trips. type EditorRequest struct { - ProjectID string `json:"project_id"` - PaywallID string `json:"paywall_id"` + ProjectID string `json:"project_id"` + PaywallID string `json:"paywall_id"` + // StepID selects a specific screen from PaywallID's graph to edit; + // omitted, the editor falls back to the draft purchase screen. + StepID *string `json:"step_id,omitempty"` Revision *int `json:"revision"` SessionID string `json:"session_id,omitempty"` Paywall PaywallData `json:"paywall"` diff --git a/scripts/gen-paths.py b/scripts/gen-paths.py index de054cb4..0a0d78a1 100644 --- a/scripts/gen-paths.py +++ b/scripts/gen-paths.py @@ -26,6 +26,7 @@ NON_SPEC_PATHS = { "/projects/{project_id}/invoices/{invoice_id}", # rc invoices get — not in public v2 spec "/projects/{project_id}/fonts", # rc fonts — live in the backend, not yet in published spec + "/projects/{project_id}/paywalls/{paywall_id}/graph", # khepri PR adding this route hasn't merged yet } From 0ff872d0f8ebec327f30844c95e72f99753356ef Mon Sep 17 00:00:00 2001 From: Jamie Holwill Date: Thu, 10 Sep 2026 16:04:31 +0100 Subject: [PATCH 2/2] fix(paywalls): seed a selected screen with the parent's offering 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) --- internal/api/paywalls.go | 4 ++-- internal/cli/paywalls_ai.go | 11 +++++++++++ internal/cli/paywalls_ai_step_test.go | 27 ++++++++++++++++++++++++--- scripts/gen-paths.py | 2 +- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/internal/api/paywalls.go b/internal/api/paywalls.go index be0bab4e..6c926738 100644 --- a/internal/api/paywalls.go +++ b/internal/api/paywalls.go @@ -212,8 +212,8 @@ func (s *PaywallsService) GetGraphWithScreenContent(ctx context.Context, project // paywall's graph instead of onto its fallback draft. parentID stays in the // 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. +// parentID. The API rejects name for a sibling screen, so this clears Name +// regardless of what the caller set. func (s *PaywallsService) UpdateDraftStep(ctx context.Context, projectID, parentID, stepID string, body PaywallDraftUpdate) (*Paywall, error) { body.Name = nil path := pathPaywall(projectID, parentID) + "?step_id=" + url.QueryEscape(stepID) diff --git a/internal/cli/paywalls_ai.go b/internal/cli/paywalls_ai.go index d9cb6a1a..0dbee478 100644 --- a/internal/cli/paywalls_ai.go +++ b/internal/cli/paywalls_ai.go @@ -524,6 +524,16 @@ func seedSessionFromServerForStep(ctx context.Context, rt *Runtime, projectID, p localizations = json.RawMessage(`{"` + locale + `": {}}`) } revision := content.Revision + // The offering lives on the parent and drives the editor's product context; + // every screen in the graph designs against the same one. + parent, err := client.Paywalls.Get(ctx, projectID, paywallID) + if err != nil { + return nil, err + } + var offeringID *string + if parent.OfferingID != "" { + offeringID = &parent.OfferingID + } return &paywallAISession{ Version: 1, ProjectID: projectID, @@ -533,6 +543,7 @@ func seedSessionFromServerForStep(ctx context.Context, rt *Runtime, projectID, p Revision: &revision, Paywall: paywallai.PaywallData{ DefaultLocale: locale, + OfferingID: offeringID, ComponentsConfig: content.ComponentsConfig, ComponentsLocalizations: localizations, StateDeclarations: serverStateDeclarations(content.StateDeclarations), diff --git a/internal/cli/paywalls_ai_step_test.go b/internal/cli/paywalls_ai_step_test.go index db599a87..b1e4cd4f 100644 --- a/internal/cli/paywalls_ai_step_test.go +++ b/internal/cli/paywalls_ai_step_test.go @@ -18,9 +18,9 @@ import ( // rcGraphMock serves a two-screen graph (a terminal, non-fallback step_1 and // a fallback step_2) plus a PATCH endpoint that records the query string and -// body it received. It fails the test if the plain (non-graph) paywall GET -// route is ever hit — the whole point of --step-id is that a sibling's -// content can no longer be reached that way. +// body it received. The plain paywall GET serves only the parent's offering; +// it fails the test if asked to expand components, since a sibling's content +// is reachable only through the graph. type rcGraphMock struct { mu sync.Mutex patchedQuery []string @@ -43,6 +43,13 @@ func (m *rcGraphMock) server(t *testing.T) *httptest.Server { {"id":"step_2","name":"Intro","type":"screen","screen_types":["generic"],"is_terminal":false,"paywall_id":"pw_sibling","edges":[],"unwired_triggers":[], "paywall":{"id":"pw_sibling","revision":5,"components_config":{"base":{}},"components_localizations":{"en_US":{}},"default_locale":"en_US","state_declarations":{}}} ]}}`) + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/paywalls/pw_parent"): + if r.URL.Query().Has("expand") { + t.Errorf("parent GET must not expand components: %s", r.URL.RawQuery) + w.WriteHeader(http.StatusNotFound) + return + } + io.WriteString(w, `{"id":"pw_parent","offering_id":"ofrng_parent","created_at":1}`) case r.Method == http.MethodPatch: m.mu.Lock() m.patchedQuery = append(m.patchedQuery, r.URL.RawQuery) @@ -72,6 +79,7 @@ type stepEchoEditorServer struct { mu sync.Mutex stepIDs []string componentsConfig []string + offeringIDs []string } func (s *stepEchoEditorServer) server(t *testing.T) *httptest.Server { @@ -81,6 +89,7 @@ func (s *stepEchoEditorServer) server(t *testing.T) *httptest.Server { StepID *string `json:"step_id"` Paywall struct { ComponentsConfig json.RawMessage `json:"components_config"` + OfferingID *string `json:"offering_id"` } `json:"paywall"` } _ = json.NewDecoder(r.Body).Decode(&body) @@ -90,6 +99,11 @@ func (s *stepEchoEditorServer) server(t *testing.T) *httptest.Server { } else { s.stepIDs = append(s.stepIDs, "") } + if body.Paywall.OfferingID != nil { + s.offeringIDs = append(s.offeringIDs, *body.Paywall.OfferingID) + } else { + s.offeringIDs = append(s.offeringIDs, "") + } s.componentsConfig = append(s.componentsConfig, string(body.Paywall.ComponentsConfig)) s.mu.Unlock() @@ -141,6 +155,13 @@ func TestPaywallsEdit_StepIDFetchesFromGraphAndSavesToSelectedStep(t *testing.T) t.Fatalf("editor received components_config = %v, want the sibling step_2's own content", sentConfig) } + editor.mu.Lock() + sentOfferings := append([]string(nil), editor.offeringIDs...) + editor.mu.Unlock() + if len(sentOfferings) != 1 || sentOfferings[0] != "ofrng_parent" { + t.Fatalf("editor received offering_id = %v, want the parent's ofrng_parent so products resolve", sentOfferings) + } + rc.mu.Lock() queries := append([]string(nil), rc.patchedQuery...) rc.mu.Unlock() diff --git a/scripts/gen-paths.py b/scripts/gen-paths.py index 0a0d78a1..50fb4f90 100644 --- a/scripts/gen-paths.py +++ b/scripts/gen-paths.py @@ -26,7 +26,7 @@ NON_SPEC_PATHS = { "/projects/{project_id}/invoices/{invoice_id}", # rc invoices get — not in public v2 spec "/projects/{project_id}/fonts", # rc fonts — live in the backend, not yet in published spec - "/projects/{project_id}/paywalls/{paywall_id}/graph", # khepri PR adding this route hasn't merged yet + "/projects/{project_id}/paywalls/{paywall_id}/graph", # rc paywalls screens — not yet in published spec }