From ba615264bd52c6f64b6d29eac8661c0b08792031 Mon Sep 17 00:00:00 2001 From: Qasim Date: Fri, 12 Jun 2026 14:22:32 -0400 Subject: [PATCH] TW-5381: align Agent Studio plan-ceiling model with documented API behavior The plan ceiling is the billing plan itself, enforced server-side by the Nylas API: limits above the plan maximum are rejected, omitted limits default to the plan maximum, and a workspace with no policy runs at plan maximums. No policy object is special or immutable. Studio previously treated the default workspace's policy as an immutable plan ceiling, which made a stale policy_id reference on the default workspace unrepairable from the UI. - studio server: drop planCeilingPolicyID/requireMutablePolicy/ validateAgainstCeiling and the default-workspace policy-swap lock; policy mutations forward to the API, plan-cap 403s surface as structured plan_limit errors; GetWorkspace only fetched when rule changes need read-modify-write - studio frontend: remove ceilingPolicyID() and locked/plan-ceiling rendering; policies uniformly editable, deletable, draggable; empty and dangling policy slots note that plan maximums apply; drop client-side limit capping in the policy editor - cli: agent overview notes plan maximums for missing/absent policies - tests: studio handler tests encode the new model (plan_limit on create and update); Playwright suite no longer asserts locked chips and tolerates zero-policy boards; overview fallback messaging test - docs: agent-studio plan-limits section, agent-policy default-policy corrections and delete-guard note, agent-getting-started example output fix, agent overview fallback note --- docs/commands/agent-getting-started.md | 17 ++- docs/commands/agent-policy.md | 20 ++- docs/commands/agent-studio.md | 21 +-- docs/commands/agent.md | 3 + internal/cli/agent/overview.go | 4 +- internal/cli/agent/overview_test.go | 25 +++- internal/studio/handlers_mutations_test.go | 21 +-- internal/studio/handlers_policies.go | 120 +----------------- .../handlers_policy_rules_lists_test.go | 83 +++++++----- internal/studio/handlers_workspaces.go | 24 ++-- internal/studio/static/css/studio.css | 2 - internal/studio/static/js/accounts.js | 7 +- internal/studio/static/js/board.js | 33 ++--- internal/studio/static/js/builders.js | 35 ++--- internal/studio/static/js/dragdrop.js | 4 - internal/studio/static/js/drawers.js | 13 +- tests/studio/e2e/smoke.spec.js | 38 +++--- 17 files changed, 195 insertions(+), 275 deletions(-) diff --git a/docs/commands/agent-getting-started.md b/docs/commands/agent-getting-started.md index 2b3b47c..8113cf0 100644 --- a/docs/commands/agent-getting-started.md +++ b/docs/commands/agent-getting-started.md @@ -112,7 +112,11 @@ Notes: ## Step 4 β€” Policies Policies are settings bundles (limits, options, spam detection) that -workspaces attach via `policy_id`. Your account already has a default one. +workspaces attach via `policy_id`. A starter "Default Policy" mirroring your +plan's maximums may already exist. Policies are optional: a workspace with no +policy runs at your billing plan's maximum limits, and policy limits can +never exceed the plan (blank limits default to the plan maximum; higher +values are rejected by the API). ```bash nylas agent policy list @@ -135,13 +139,12 @@ nylas workspace update --policy-id ``` Policies (2) -1. Default Policy - ID: pppppppp-1111-1111-1111-111111111111 - Attached: support@yourapp.nylas.email (workspace aaaaaaaa-...) +1. Default Policy pppppppp-1111-1111-1111-111111111111 + Updated: 2 days ago + Agent: support@yourapp.nylas.email (gggggggg-1111-1111-1111-111111111111) -2. Strict Policy - ID: pppppppp-2222-2222-2222-222222222222 - Attached: (none) +2. Strict Policy pppppppp-2222-2222-2222-222222222222 + Updated: 5 hours ago ``` ## Step 5 β€” Lists diff --git a/docs/commands/agent-policy.md b/docs/commands/agent-policy.md index 838446b..e3ca9a8 100644 --- a/docs/commands/agent-policy.md +++ b/docs/commands/agent-policy.md @@ -64,6 +64,9 @@ nylas agent policy create --data-file policy.json nylas agent policy create --data '{"name":"Strict Policy","rules":["rule-123"]}' ``` +Every limit is optional: omitted limits default to your billing plan's +maximum, and values above the plan maximum are rejected by the API. + Example payload: ```json @@ -130,19 +133,26 @@ Policies attach to workspaces via `policy_id`. To assign a policy to an agent ac nylas workspace update --policy-id ``` -The API auto-creates a default workspace and policy when an agent account is created. +The API auto-creates a default workspace when an agent account is created +(and may create a starter "Default Policy" mirroring your plan's maximums). +A workspace with no `policy_id` attached runs at your billing plan's maximum +limits β€” having zero policies is a valid state. ## Troubleshooting If `nylas agent policy list` returns nothing: -- no policies have been explicitly created via `/v3/policies` -- the API auto-creates a default policy on the workspace, but it does not appear in `/v3/policies` +- no policies currently exist for the application β€” accounts run at your + billing plan's maximum limits +- create one with `nylas agent policy create`; blank limits default to the + plan maximums If `nylas agent policy delete` fails: -- the policy is still attached to one or more agent workspaces -- run `nylas agent policy list` to see the attached workspace mappings +- the policy is still attached to one or more agent workspaces (the CLI + blocks the delete to avoid leaving dangling `policy_id` references) +- run `nylas agent policy list` to see the attached workspace mappings, then + detach with `nylas workspace update --policy-id ` ## See Also diff --git a/docs/commands/agent-studio.md b/docs/commands/agent-studio.md index aab6563..d37766b 100644 --- a/docs/commands/agent-studio.md +++ b/docs/commands/agent-studio.md @@ -41,9 +41,9 @@ a chip. ## Accounts view -One row per account: status dot, email, workspace, governing policy (πŸ”’ when -it is the plan ceiling), rule count, and a shared-workspace badge. Substring -search filters by email, workspace, or policy. Inline quick actions: +One row per account: status dot, email, workspace, governing policy ("plan +maximums" when none is attached), rule count, and a shared-workspace badge. +Substring search filters by email, workspace, or policy. Inline quick actions: - **✈ Test** β€” send a self-addressed test email - **⟳ Rotate** β€” rotate the app password (the new password is shown once) @@ -54,18 +54,19 @@ Account moves use the workspace `manual-assign` API; the workspace can also be chosen up-front when creating the account, or moved from the terminal with `nylas agent account move --workspace `. -## Plan ceiling +## Plan limits -The policy attached to the default workspace is your **plan ceiling**: it -renders locked (πŸ”’) and cannot be edited, deleted, or swapped. Custom policy -limits are validated against the ceiling both in the editor and server-side -(`above_plan_ceiling` on violation). +Your **billing plan** caps every policy limit, enforced by the Nylas API: +omitted limits default to the plan maximum, and values above it are rejected. +A workspace with no policy attached simply runs at plan maximums, so any +policy β€” including the default workspace's β€” can be edited, deleted, or +swapped freely. ## Creating resources The **οΌ‹ New** menu creates agent accounts (with an app-password generator and -optional workspace pick), workspaces, policies (ceiling-bounded limit fields), -rules, and lists β€” plus one-click rule recipes. +optional workspace pick), workspaces, policies (blank limits default to plan +maximums), rules, and lists β€” plus one-click rule recipes. The **rule builder** is sentence-shaped and constrained by the live API matrix: inbound rules only offer `from.*` fields; outbound adds `recipient.*` diff --git a/docs/commands/agent.md b/docs/commands/agent.md index 7351e0b..b0e051e 100644 --- a/docs/commands/agent.md +++ b/docs/commands/agent.md @@ -182,6 +182,9 @@ support@yourapp.nylas.email valid └── Archive newsletters (inbound) [disabled] ``` +Workspaces with no policy (or a dangling `policy_id`) note that plan +maximums apply β€” accounts without a policy run at the billing plan's limits. + The overview also flags problems the API does not prevent: - ⚠ dangling references β€” workspace `policy_id`/`rule_ids` or rule `in_list` conditions pointing at deleted resources diff --git a/internal/cli/agent/overview.go b/internal/cli/agent/overview.go index 241b49c..b896ecd 100644 --- a/internal/cli/agent/overview.go +++ b/internal/cli/agent/overview.go @@ -122,9 +122,9 @@ func printOverviewWorkspace(acct agentgraph.Account) { fmt.Printf("└── Workspace: %s%s\n", name, suffix) if acct.Policy == nil { - fmt.Println(" β”œβ”€β”€ (no policy attached)") + fmt.Println(" β”œβ”€β”€ (no policy attached β€” plan maximums apply)") } else if acct.Policy.Missing { - fmt.Printf(" β”œβ”€β”€ ⚠ Policy %s no longer exists\n", acct.Policy.ID) + fmt.Printf(" β”œβ”€β”€ ⚠ Policy %s no longer exists β€” plan maximums apply\n", acct.Policy.ID) } else { fmt.Printf(" β”œβ”€β”€ Policy: %s\n", acct.Policy.Name) } diff --git a/internal/cli/agent/overview_test.go b/internal/cli/agent/overview_test.go index 1ab0322..7a2a252 100644 --- a/internal/cli/agent/overview_test.go +++ b/internal/cli/agent/overview_test.go @@ -3,11 +3,12 @@ package agent import ( "testing" + "github.com/nylas/cli/internal/agentgraph" "github.com/stretchr/testify/assert" ) // Graph-assembly behavior is tested in internal/agentgraph; this file covers -// only the command wiring. +// the command wiring and the policy-line rendering. func TestAgentOverviewCmd(t *testing.T) { cmd := newOverviewCmd() @@ -16,3 +17,25 @@ func TestAgentOverviewCmd(t *testing.T) { assert.Contains(t, cmd.Aliases, "tree") assert.Contains(t, cmd.Short, "overview") } + +// A workspace without a (live) policy is a normal state, not an error: the +// account runs at the billing plan's maximum limits, and the output must say +// so instead of implying a policy is required. +func TestPrintOverviewWorkspace_PolicyFallbackMessaging(t *testing.T) { + base := agentgraph.Account{WorkspaceID: "ws-1", WorkspaceName: "Support"} + + noPolicy := base + out := captureStdout(t, func() { printOverviewWorkspace(noPolicy) }) + assert.Contains(t, out, "no policy attached β€” plan maximums apply") + + missing := base + missing.Policy = &agentgraph.Policy{ID: "policy-gone", Missing: true} + out = captureStdout(t, func() { printOverviewWorkspace(missing) }) + assert.Contains(t, out, "Policy policy-gone no longer exists β€” plan maximums apply") + + attached := base + attached.Policy = &agentgraph.Policy{ID: "policy-1", Name: "Strict"} + out = captureStdout(t, func() { printOverviewWorkspace(attached) }) + assert.Contains(t, out, "Policy: Strict") + assert.NotContains(t, out, "plan maximums") +} diff --git a/internal/studio/handlers_mutations_test.go b/internal/studio/handlers_mutations_test.go index b63b3f5..61cd7a6 100644 --- a/internal/studio/handlers_mutations_test.go +++ b/internal/studio/handlers_mutations_test.go @@ -97,16 +97,23 @@ func TestHandleWorkspacePatch_SetPolicy(t *testing.T) { } } -func TestHandleWorkspacePatch_DefaultWorkspacePolicyLocked(t *testing.T) { +func TestHandleWorkspacePatch_DefaultWorkspacePolicyAllowed(t *testing.T) { t.Parallel() - server := newTestServer() + rec := &workspaceUpdateRecorder{MockClient: nylasmock.NewMockClient()} + server := NewServer("127.0.0.1:0", rec) - // workspace-1 is the default workspace: its policy defines the plan - // ceiling, so swapping it would bypass the ceiling-policy protection. + // workspace-1 is the default workspace. Its policy is not a plan ceiling + // (the ceiling is the billing plan, enforced by the API), so swapping it + // must work β€” otherwise a stale policy reference on the default workspace + // becomes unrepairable from Studio. w := doJSON(t, server.routeWorkspaces, http.MethodPatch, "/api/workspaces/workspace-1", `{"policy_id":"policy-9"}`) - if w.Code != http.StatusForbidden { - t.Fatalf("default workspace policy swap must be rejected: expected 403, got %d (body: %s)", w.Code, w.Body.String()) + if w.Code != http.StatusOK { + t.Fatalf("default workspace policy swap must be allowed: expected 200, got %d (body: %s)", w.Code, w.Body.String()) + } + decodeMutation(t, w) + if rec.gotPolicyID == nil || *rec.gotPolicyID != "policy-9" { + t.Fatalf("expected policy_id forwarded to UpdateWorkspace, got %v", rec.gotPolicyID) } } @@ -114,8 +121,6 @@ func TestHandleWorkspacePatch_DefaultWorkspaceRuleAttachAllowed(t *testing.T) { t.Parallel() server := newTestServer() - // Only the policy slot is locked on the default workspace; rule - // attachments remain legitimate. w := doJSON(t, server.routeWorkspaces, http.MethodPatch, "/api/workspaces/workspace-1", `{"add_rule_ids":["rule-2"]}`) if w.Code != http.StatusOK { diff --git a/internal/studio/handlers_policies.go b/internal/studio/handlers_policies.go index ff7702d..68766ee 100644 --- a/internal/studio/handlers_policies.go +++ b/internal/studio/handlers_policies.go @@ -1,12 +1,8 @@ package studio import ( - "context" - "fmt" "net/http" "strings" - - "github.com/nylas/cli/internal/domain" ) func (s *Server) routePolicies(w http.ResponseWriter, r *http.Request) { @@ -26,7 +22,7 @@ func (s *Server) routePolicies(w http.ResponseWriter, r *http.Request) { } // handlePolicyGet returns full policy detail (limits, spam detection) for the -// ceiling-bounded editor. +// policy editor. func (s *Server) handlePolicyGet(w http.ResponseWriter, r *http.Request, id string) { ctx, cancel := s.withTimeout(r) defer cancel() @@ -39,6 +35,12 @@ func (s *Server) handlePolicyGet(w http.ResponseWriter, r *http.Request, id stri writeJSON(w, http.StatusOK, policy) } +// Policy mutations forward to the API unchecked: the plan ceiling is the +// billing plan itself, enforced server-side by Nylas β€” limits above the plan +// maximum are rejected upstream, and omitted limits default to the plan +// maximum. No policy is special; a workspace with no policy simply runs at +// plan maximums. + func (s *Server) handlePolicyCreate(w http.ResponseWriter, r *http.Request) { var payload map[string]any if !decodeBody(w, r, &payload) { @@ -52,10 +54,6 @@ func (s *Server) handlePolicyCreate(w http.ResponseWriter, r *http.Request) { ctx, cancel := s.withTimeout(r) defer cancel() - if !s.validateAgainstCeiling(ctx, w, payload) { - return - } - policy, err := s.nylasClient.CreatePolicy(ctx, payload) if err != nil { writeMutationError(w, "Failed to create policy", err) @@ -74,13 +72,6 @@ func (s *Server) handlePolicyPatch(w http.ResponseWriter, r *http.Request, id st ctx, cancel := s.withTimeout(r) defer cancel() - if !s.requireMutablePolicy(ctx, w, id) { - return - } - if !s.validateAgainstCeiling(ctx, w, payload) { - return - } - if _, err := s.nylasClient.UpdatePolicy(ctx, id, payload); err != nil { writeMutationError(w, "Failed to update policy", err) return @@ -93,10 +84,6 @@ func (s *Server) handlePolicyDelete(w http.ResponseWriter, r *http.Request, id s ctx, cancel := s.withTimeout(r) defer cancel() - if !s.requireMutablePolicy(ctx, w, id) { - return - } - if err := s.nylasClient.DeletePolicy(ctx, id); err != nil { writeMutationError(w, "Failed to delete policy", err) return @@ -104,96 +91,3 @@ func (s *Server) handlePolicyDelete(w http.ResponseWriter, r *http.Request, id s s.respondMutation(ctx, w, http.StatusOK, id) } - -// planCeilingPolicyID identifies the plan-ceiling policy: the one attached to -// the default workspace. It is immutable and its limits bound custom policies. -func (s *Server) planCeilingPolicyID(ctx context.Context) (string, error) { - workspaces, err := s.nylasClient.ListWorkspaces(ctx) - if err != nil { - return "", err - } - for _, ws := range workspaces { - if ws.Default { - return strings.TrimSpace(ws.PolicyID), nil - } - } - return "", nil -} - -// requireMutablePolicy rejects writes against the plan-ceiling policy. -func (s *Server) requireMutablePolicy(ctx context.Context, w http.ResponseWriter, id string) bool { - ceilingID, err := s.planCeilingPolicyID(ctx) - if err != nil { - writeMutationError(w, "Failed to resolve plan ceiling policy", err) - return false - } - if ceilingID != "" && id == ceilingID { - writeJSON(w, http.StatusForbidden, map[string]string{ - "error": "default_policy_immutable", - "message": "The default policy is your plan ceiling and cannot be modified", - }) - return false - } - return true -} - -// validateAgainstCeiling rejects numeric limits exceeding the plan ceiling. -func (s *Server) validateAgainstCeiling(ctx context.Context, w http.ResponseWriter, payload map[string]any) bool { - limits, _ := payload["limits"].(map[string]any) - if len(limits) == 0 { - return true - } - - ceilingID, err := s.planCeilingPolicyID(ctx) - if err != nil || ceilingID == "" { - // No resolvable ceiling: let the API enforce its own bounds. - return true - } - ceiling, err := s.nylasClient.GetPolicy(ctx, ceilingID) - if err != nil || ceiling == nil || ceiling.Limits == nil { - return true - } - - for field, max := range ceilingLimitValues(ceiling.Limits) { - requested, ok := limits[field].(float64) - if !ok { - continue - } - if requested > max { - writeJSON(w, http.StatusBadRequest, map[string]string{ - "error": "above_plan_ceiling", - "message": fmt.Sprintf("%s exceeds the plan ceiling (%v > %v)", field, requested, max), - }) - return false - } - } - return true -} - -// ceilingLimitValues flattens the ceiling policy's numeric limits into the -// wire field names used by policy payloads. -func ceilingLimitValues(limits *domain.PolicyLimits) map[string]float64 { - out := make(map[string]float64, 7) - if limits.LimitAttachmentSizeInBytes != nil { - out["limit_attachment_size_limit"] = float64(*limits.LimitAttachmentSizeInBytes) - } - if limits.LimitAttachmentCount != nil { - out["limit_attachment_count_limit"] = float64(*limits.LimitAttachmentCount) - } - if limits.LimitSizeTotalMimeInBytes != nil { - out["limit_size_total_mime"] = float64(*limits.LimitSizeTotalMimeInBytes) - } - if limits.LimitStorageTotalInBytes != nil { - out["limit_storage_total"] = float64(*limits.LimitStorageTotalInBytes) - } - if limits.LimitCountDailyMessagePerGrant != nil { - out["limit_count_daily_message_per_grant"] = float64(*limits.LimitCountDailyMessagePerGrant) - } - if limits.LimitInboxRetentionPeriodInDays != nil { - out["limit_inbox_retention_period"] = float64(*limits.LimitInboxRetentionPeriodInDays) - } - if limits.LimitSpamRetentionPeriodInDays != nil { - out["limit_spam_retention_period"] = float64(*limits.LimitSpamRetentionPeriodInDays) - } - return out -} diff --git a/internal/studio/handlers_policy_rules_lists_test.go b/internal/studio/handlers_policy_rules_lists_test.go index b86f1ac..fd8b933 100644 --- a/internal/studio/handlers_policy_rules_lists_test.go +++ b/internal/studio/handlers_policy_rules_lists_test.go @@ -27,28 +27,34 @@ func TestHandlePolicyCreate(t *testing.T) { } } -func TestHandlePolicyPatch_DefaultPolicyImmutable(t *testing.T) { +func TestHandlePolicyPatch_DefaultWorkspacePolicyMutable(t *testing.T) { t.Parallel() server := newTestServer() - // policy-1 is attached to the default workspace (workspace-1) in the mock, - // making it the plan-ceiling policy: edits must be rejected server-side. + // policy-1 is attached to the default workspace (workspace-1) in the mock. + // Per the documented model, no policy is the plan ceiling β€” the ceiling is + // the billing plan, enforced by the Nylas API β€” so every policy is + // editable, including the default workspace's. w := doJSON(t, server.routePolicies, http.MethodPatch, "/api/policies/policy-1", `{"name":"Renamed"}`) - if w.Code != http.StatusForbidden { - t.Fatalf("default policy is the plan ceiling and must be immutable: expected 403, got %d (body: %s)", w.Code, w.Body.String()) + if w.Code != http.StatusOK { + t.Fatalf("every policy must be editable (plan ceiling is API-enforced): expected 200, got %d (body: %s)", w.Code, w.Body.String()) } + decodeMutation(t, w) } -func TestHandlePolicyDelete_DefaultPolicyImmutable(t *testing.T) { +func TestHandlePolicyDelete_DefaultWorkspacePolicyAllowed(t *testing.T) { t.Parallel() server := newTestServer() + // Deleting any policy is allowed: workspaces left without a policy run at + // the billing plan's maximum limits. w := doJSON(t, server.routePolicies, http.MethodDelete, "/api/policies/policy-1", "") - if w.Code != http.StatusForbidden { - t.Fatalf("expected 403 deleting the plan-ceiling policy, got %d", w.Code) + if w.Code != http.StatusOK { + t.Fatalf("expected 200 deleting the default workspace's policy, got %d (body: %s)", w.Code, w.Body.String()) } + decodeMutation(t, w) } func TestHandlePolicyPatch_CustomPolicyAllowed(t *testing.T) { @@ -63,44 +69,63 @@ func TestHandlePolicyPatch_CustomPolicyAllowed(t *testing.T) { decodeMutation(t, w) } -// ceilingClient gives the plan-ceiling policy (policy-1) explicit limits so -// validation against them can be exercised. -type ceilingClient struct { - *nylasmock.MockClient +// CreatePolicy and UpdatePolicy simulate the Nylas API rejecting a policy +// whose limits exceed the billing plan's maximum (the API returns 403 for +// plan caps). planLimitClient is declared in handlers_mutations_test.go. +func (c *planLimitClient) CreatePolicy(ctx context.Context, payload map[string]any) (*domain.Policy, error) { + return nil, &domain.APIError{StatusCode: http.StatusForbidden, Message: "limit exceeds plan maximum"} } -func (c *ceilingClient) GetPolicy(ctx context.Context, policyID string) (*domain.Policy, error) { - daily := int64(500) - return &domain.Policy{ - ID: policyID, - Name: "Default Policy", - Limits: &domain.PolicyLimits{ - LimitCountDailyMessagePerGrant: &daily, - }, - }, nil +func (c *planLimitClient) UpdatePolicy(ctx context.Context, policyID string, payload map[string]any) (*domain.Policy, error) { + return nil, &domain.APIError{StatusCode: http.StatusForbidden, Message: "limit exceeds plan maximum"} } -func TestHandlePolicyCreate_RejectsLimitsAboveCeiling(t *testing.T) { +func TestHandlePolicyCreate_SurfacesPlanLimitError(t *testing.T) { t.Parallel() - server := NewServer("127.0.0.1:0", &ceilingClient{MockClient: nylasmock.NewMockClient()}) + server := NewServer("127.0.0.1:0", &planLimitClient{MockClient: nylasmock.NewMockClient()}) + // Plan maximums are enforced by the Nylas API, not by Studio: the handler + // forwards the payload and must surface the API's rejection as a + // structured plan_limit error the UI can render. w := doJSON(t, server.routePolicies, http.MethodPost, "/api/policies", `{"name":"Over","limits":{"limit_count_daily_message_per_grant":600}}`) - if w.Code != http.StatusBadRequest { - t.Fatalf("limits above the plan ceiling must be rejected: expected 400, got %d (body: %s)", w.Code, w.Body.String()) + if w.Code != http.StatusForbidden { + t.Fatalf("API plan rejection must be surfaced: expected 403, got %d (body: %s)", w.Code, w.Body.String()) } var resp map[string]string _ = json.NewDecoder(w.Body).Decode(&resp) - if resp["error"] != "above_plan_ceiling" { - t.Fatalf("expected structured above_plan_ceiling error, got %q", resp["error"]) + if resp["error"] != "plan_limit" { + t.Fatalf("expected structured plan_limit error, got %q", resp["error"]) } } -func TestHandlePolicyCreate_AllowsLimitsWithinCeiling(t *testing.T) { +func TestHandlePolicyPatch_SurfacesPlanLimitError(t *testing.T) { t.Parallel() - server := NewServer("127.0.0.1:0", &ceilingClient{MockClient: nylasmock.NewMockClient()}) + server := NewServer("127.0.0.1:0", &planLimitClient{MockClient: nylasmock.NewMockClient()}) + + // With the client-side ceiling validation removed, the PATCH path relies + // entirely on the API's plan enforcement β€” its rejection must surface as + // the same structured plan_limit error as create. + w := doJSON(t, server.routePolicies, http.MethodPatch, "/api/policies/policy-7", + `{"name":"Over","limits":{"limit_count_daily_message_per_grant":600}}`) + + if w.Code != http.StatusForbidden { + t.Fatalf("API plan rejection must be surfaced on update: expected 403, got %d (body: %s)", w.Code, w.Body.String()) + } + var resp map[string]string + _ = json.NewDecoder(w.Body).Decode(&resp) + if resp["error"] != "plan_limit" { + t.Fatalf("expected structured plan_limit error, got %q", resp["error"]) + } +} + +func TestHandlePolicyCreate_ForwardsLimitsToAPI(t *testing.T) { + t.Parallel() + server := newTestServer() + // Studio performs no client-side limit validation β€” payloads go to the + // API as-is and succeed unless the API rejects them. w := doJSON(t, server.routePolicies, http.MethodPost, "/api/policies", `{"name":"Within","limits":{"limit_count_daily_message_per_grant":200}}`) diff --git a/internal/studio/handlers_workspaces.go b/internal/studio/handlers_workspaces.go index d011138..bd5b09b 100644 --- a/internal/studio/handlers_workspaces.go +++ b/internal/studio/handlers_workspaces.go @@ -68,24 +68,16 @@ func (s *Server) handleWorkspacePatch(w http.ResponseWriter, r *http.Request, id ctx, cancel := s.withTimeout(r) defer cancel() - ws, err := s.nylasClient.GetWorkspace(ctx, id) - if err != nil { - writeMutationError(w, "Failed to load workspace", err) - return - } - - // The default workspace's policy defines the plan ceiling; swapping it - // would silently change which policy the ceiling protection guards. - if body.PolicyID != nil && ws.Default { - writeJSON(w, http.StatusForbidden, map[string]string{ - "error": "default_workspace_policy_locked", - "message": "The default workspace's policy is your plan ceiling and cannot be swapped", - }) - return - } - update := &domain.UpdateWorkspaceRequest{PolicyID: body.PolicyID} if len(body.AddRuleIDs) > 0 || len(body.RemoveRuleIDs) > 0 { + // Rule changes are read-modify-write: the API replaces the whole + // rule_ids list, so the current attachments must be fetched first. + // Policy-only patches skip the extra round-trip. + ws, err := s.nylasClient.GetWorkspace(ctx, id) + if err != nil { + writeMutationError(w, "Failed to load workspace", err) + return + } ruleIDs := applyRuleIDChanges(ws.RulesIDs, body.AddRuleIDs, body.RemoveRuleIDs) update.RulesIDs = &ruleIDs } diff --git a/internal/studio/static/css/studio.css b/internal/studio/static/css/studio.css index 98b0995..f62e572 100644 --- a/internal/studio/static/css/studio.css +++ b/internal/studio/static/css/studio.css @@ -44,7 +44,6 @@ body { } .chip .chip-tag { margin-left: auto; font-size: 12px; background: rgba(255,255,255,.08); padding: 2px 6px; border-radius: 99px; color: var(--muted); } .chip-policy { background: linear-gradient(135deg, rgba(124,92,255,.22), rgba(177,75,244,.12)); border-color: rgba(124,92,255,.4); color: #cfc4ff; } -.chip-policy.locked { background: rgba(139,139,158,.12); border-color: rgba(139,139,158,.35); color: var(--soft); } .chip-rule { background: rgba(57,217,138,.1); border-color: rgba(57,217,138,.32); color: #a8f0cd; } .chip-list { background: rgba(255,181,71,.1); border-color: rgba(255,181,71,.32); color: #ffd9a0; } @@ -65,7 +64,6 @@ body { border: 1px solid; cursor: pointer; } .slot-policy { background: rgba(124,92,255,.13); border-color: rgba(124,92,255,.35); color: #cfc4ff; } -.slot-policy.locked { background: rgba(139,139,158,.1); border-color: rgba(139,139,158,.3); color: var(--soft); } .slot-rule { background: rgba(57,217,138,.07); border-color: rgba(57,217,138,.22); color: #a8f0cd; } .slot-rule.disabled { opacity: .55; } .slot-empty { border: 1.5px dashed rgba(255,255,255,.15); color: var(--muted); text-align: center; cursor: default; } diff --git a/internal/studio/static/js/accounts.js b/internal/studio/static/js/accounts.js index 1f264fe..a15de91 100644 --- a/internal/studio/static/js/accounts.js +++ b/internal/studio/static/js/accounts.js @@ -108,7 +108,7 @@ window.StudioAccounts = { if (!q) { return true; } - const policy = acct.policy ? (acct.policy.name || acct.policy.id) : ''; + const policy = acct.policy ? (acct.policy.name || acct.policy.id) : 'plan maximums'; return [acct.email, acct.workspace_name, acct.workspace_id, policy] .some((field) => (field || '').toLowerCase().includes(q)); }, @@ -152,10 +152,9 @@ window.StudioAccounts = { const meta = D.el('div', 'acct-meta'); const wsName = StudioBoard.displayName(acct.workspace_name) || acct.workspace_id || 'no workspace'; D.add(meta, D.el('span', '', (acct.status || 'unknown') + ' Β· ' + wsName)); - const ceiling = StudioBoard.ceilingPolicyID(StudioBoard.state || {}); const policy = acct.policy - ? ((acct.policy.id === ceiling ? 'πŸ”’ ' : 'πŸ›‘ ') + (acct.policy.name || acct.policy.id)) - : 'no policy'; + ? 'πŸ›‘ ' + (acct.policy.name || acct.policy.id) + : 'plan maximums'; D.add(meta, D.el('span', '', policy + ' Β· ' + (acct.rules || []).length + ' ⚑')); D.add(row, meta); diff --git a/internal/studio/static/js/board.js b/internal/studio/static/js/board.js index b4af424..455159d 100644 --- a/internal/studio/static/js/board.js +++ b/internal/studio/static/js/board.js @@ -38,16 +38,6 @@ window.StudioBoard = { return name.replace(/\s*Connector\s+'[0-9a-fA-F-]{36}'/, '').trim(); }, - // ceilingPolicyID is the plan ceiling: the default workspace's policy. - ceilingPolicyID(board) { - for (const ws of board.workspaces || []) { - if (ws.default && ws.policy) { - return ws.policy.id; - } - } - return ''; - }, - renderTotals(totals) { const D = StudioDOM; const el = D.clear(document.getElementById('totals')); @@ -105,19 +95,14 @@ window.StudioBoard = { const D = StudioDOM; const palette = D.clear(document.getElementById('palette')); const { policies, rules, lists, ruleAttached } = this.collectResources(board); - const ceilingID = this.ceilingPolicyID(board); D.add(palette, D.el('div', 'palette-label', 'Policies Β· ' + policies.size)); for (const policy of policies.values()) { - const locked = policy.id === ceilingID; - const chip = D.el('div', locked ? 'chip chip-policy locked' : 'chip chip-policy', - (locked ? 'πŸ”’ ' : 'πŸ›‘ ') + (policy.name || policy.id)); - if (locked) { - D.add(chip, D.el('span', 'chip-tag', 'plan ceiling')); - } else if (policy.missing) { + const chip = D.el('div', 'chip chip-policy', 'πŸ›‘ ' + (policy.name || policy.id)); + if (policy.missing) { D.add(chip, D.el('span', 'chip-tag', 'missing')); } - chip.addEventListener('click', () => StudioDrawer.showPolicy(policy, locked)); + chip.addEventListener('click', () => StudioDrawer.showPolicy(policy)); if (!policy.missing) { StudioDragDrop.makeChipDraggable(chip, 'policy', policy); } @@ -216,17 +201,15 @@ window.StudioBoard = { policySlot(ws) { const D = StudioDOM; if (!ws.policy) { - return D.el('div', 'slot slot-empty', 'No policy attached'); + return D.el('div', 'slot slot-empty', 'No policy β€” plan maximums apply'); } if (ws.policy.missing) { - const slot = D.el('div', 'slot slot-warn', '⚠ Policy ' + ws.policy.id + ' no longer exists'); - slot.addEventListener('click', () => StudioDrawer.showPolicy(ws.policy, false)); + const slot = D.el('div', 'slot slot-warn', '⚠ Policy ' + ws.policy.id + ' no longer exists β€” plan maximums apply'); + slot.addEventListener('click', () => StudioDrawer.showPolicy(ws.policy)); return slot; } - const locked = ws.policy.id === this.ceilingPolicyID(this.state); - const slot = D.el('div', locked ? 'slot slot-policy locked' : 'slot slot-policy', - (locked ? 'πŸ”’ ' : 'πŸ›‘ ') + (ws.policy.name || ws.policy.id)); - slot.addEventListener('click', () => StudioDrawer.showPolicy(ws.policy, locked)); + const slot = D.el('div', 'slot slot-policy', 'πŸ›‘ ' + (ws.policy.name || ws.policy.id)); + slot.addEventListener('click', () => StudioDrawer.showPolicy(ws.policy)); return slot; }, diff --git a/internal/studio/static/js/builders.js b/internal/studio/static/js/builders.js index 4589ef3..79440ed 100644 --- a/internal/studio/static/js/builders.js +++ b/internal/studio/static/js/builders.js @@ -114,7 +114,7 @@ window.StudioBuilders = { const workspace = M.select(wsOptions, ''); D.add(modal, M.field('Workspace', workspace)); - D.add(modal, D.el('div', 'modal-note', 'A workspace and the plan-ceiling policy attach automatically when none is chosen.')); + D.add(modal, D.el('div', 'modal-note', 'Without a workspace, the account lands in the default workspace and runs at your plan\'s limits unless a policy is attached.')); M.actions(modal, 'Create account', () => { const body = { email: email.value.trim() }; @@ -139,19 +139,9 @@ window.StudioBuilders = { }); }, - async policyForm(existing) { + policyForm(existing) { const M = StudioModal; const D = StudioDOM; - const ceilingID = StudioBoard.ceilingPolicyID(StudioBoard.state); - let ceiling = null; - if (ceilingID) { - try { - ceiling = await StudioAPI.getPolicy(ceilingID); - } catch (_error) { - ceiling = null; - } - } - const ceilingLimits = (ceiling && ceiling.limits) || {}; const limitFields = [ ['limit_count_daily_message_per_grant', 'Daily messages / account'], ['limit_attachment_size_limit', 'Attachment size (bytes)'], @@ -161,38 +151,29 @@ window.StudioBuilders = { ]; M.open(existing ? 'Edit policy' : 'New policy', - 'Limits are capped by your plan ceiling', (modal) => { + 'Blank limits default to your plan\'s maximum; values above it are rejected by the API', (modal) => { const name = M.input('Policy name', existing ? existing.name : ''); D.add(modal, M.field('Name', name)); const inputs = {}; for (const [key, label] of limitFields) { - const max = ceilingLimits[key]; - const input = M.input(max !== undefined && max !== null ? 'plan max ' + max : 'API default'); + const input = M.input('plan maximum'); input.type = 'number'; - if (max !== undefined && max !== null) { - input.max = String(max); - } if (existing && existing.limits && existing.limits[key] !== undefined && existing.limits[key] !== null) { input.value = String(existing.limits[key]); } - inputs[key] = { input, max }; - D.add(modal, M.field(label + (max !== undefined && max !== null ? ' Β· max ' + max : ''), input)); + inputs[key] = input; + D.add(modal, M.field(label, input)); } M.actions(modal, existing ? 'Save policy' : 'Create policy', () => { const body = { name: name.value.trim() }; const limits = {}; for (const key of Object.keys(inputs)) { - const { input, max } = inputs[key]; - if (input.value === '') { + if (inputs[key].value === '') { continue; } - const value = Number(input.value); - if (max !== undefined && max !== null && value > max) { - throw new Error(key + ' exceeds the plan ceiling (' + value + ' > ' + max + ')'); - } - limits[key] = value; + limits[key] = Number(inputs[key].value); } if (Object.keys(limits).length) { body.limits = limits; diff --git a/internal/studio/static/js/dragdrop.js b/internal/studio/static/js/dragdrop.js index 6f22365..1e11fe4 100644 --- a/internal/studio/static/js/dragdrop.js +++ b/internal/studio/static/js/dragdrop.js @@ -69,10 +69,6 @@ window.StudioDragDrop = { handleDrop(payload, ws) { if (payload.kind === 'policy') { - if (ws.default) { - this.toast('The default workspace’s policy is your plan ceiling and cannot be swapped.'); - return; - } if (ws.policy && ws.policy.id === payload.id) { return; } diff --git a/internal/studio/static/js/drawers.js b/internal/studio/static/js/drawers.js index f469134..764d108 100644 --- a/internal/studio/static/js/drawers.js +++ b/internal/studio/static/js/drawers.js @@ -95,16 +95,15 @@ window.StudioDrawer = { }); }, - showPolicy(policy, locked) { + showPolicy(policy) { const D = StudioDOM; - const title = (locked ? 'πŸ”’ ' : 'πŸ›‘ ') + (policy.name || policy.id); - const sub = locked ? 'Plan ceiling β€” read-only' : 'Custom policy'; - this.open(title, sub, (drawer) => { + const title = 'πŸ›‘ ' + (policy.name || policy.id); + this.open(title, 'Policy', (drawer) => { D.add(drawer, D.kv('Policy ID', policy.id, true)); if (policy.missing) { - D.add(drawer, D.el('div', 'slot slot-warn', '⚠ This policy no longer exists but is still referenced.')); + D.add(drawer, D.el('div', 'slot slot-warn', '⚠ This policy no longer exists but is still referenced β€” affected accounts run at your plan\'s maximum limits.')); } - if (!locked && !policy.missing) { + if (!policy.missing) { const actions = D.el('div', 'drawer-actions'); const edit = D.el('button', 'btn', '✎ Edit policy'); edit.addEventListener('click', async () => { @@ -119,7 +118,7 @@ window.StudioDrawer = { D.add(drawer, actions); this.dangerZone(drawer, 'Delete policy…', - 'Workspaces referencing it fall back to no policy.', + 'Workspaces referencing it fall back to your plan\'s maximum limits.', () => this.confirmAndRun( 'Delete policy "' + (policy.name || policy.id) + '"?', () => StudioAPI.deletePolicy(policy.id))); diff --git a/tests/studio/e2e/smoke.spec.js b/tests/studio/e2e/smoke.spec.js index 71b5769..192a7a9 100644 --- a/tests/studio/e2e/smoke.spec.js +++ b/tests/studio/e2e/smoke.spec.js @@ -22,10 +22,12 @@ test.describe('Agent Studio', () => { await expect(page.locator('.palette .palette-label').first()).toContainText('Policies'); }); - test('plan-ceiling policy renders locked', async ({ page }) => { - const locked = page.locator('.chip-policy.locked'); - await expect(locked.first()).toBeVisible(); - await expect(locked.first()).toContainText('plan ceiling'); + test('workspace without a policy shows the plan-maximums fallback', async ({ page }) => { + // No policy is special: a workspace with no policy attached (or a stale + // reference) must explain that plan maximums apply. + const noPolicy = page.locator('.ws-card').filter({ hasNot: page.locator('.slot-policy') }); + test.skip(await noPolicy.count() === 0, 'every workspace has a policy attached'); + await expect(noPolicy.first().getByText('plan maximums apply')).toBeVisible(); }); test('clicking an account chip opens the inspector drawer', async ({ page }) => { @@ -38,13 +40,18 @@ test.describe('Agent Studio', () => { await expect(drawer).not.toHaveClass(/open/); }); - test('locked policy drawer offers no edit or delete', async ({ page }) => { - await page.locator('.chip-policy.locked').first().click(); + test('every policy drawer offers edit and delete', async ({ page }) => { + // No policy is a read-only plan ceiling: the ceiling is the billing plan, + // enforced by the API, so every policy is editable and deletable. + // (Missing-policy chips carry a .chip-tag and render without edit/delete, + // so exclude them by class, not by name text.) + const chips = page.locator('.chip-policy').filter({ hasNot: page.locator('.chip-tag') }); + test.skip(await chips.count() === 0, 'no policies in live board state'); + await chips.first().click(); const drawer = page.locator('#drawer'); await expect(drawer).toHaveClass(/open/); - await expect(drawer.getByText('Plan ceiling β€” read-only')).toBeVisible(); - await expect(drawer.getByRole('button', { name: /Edit policy/ })).toHaveCount(0); - await expect(drawer.getByRole('button', { name: /Delete policy/ })).toHaveCount(0); + await expect(drawer.getByRole('button', { name: /Edit policy/ })).toBeVisible(); + await expect(drawer.getByRole('button', { name: /Delete policy/ })).toBeVisible(); }); test('new menu lists all create flows and recipes', async ({ page }) => { @@ -88,12 +95,13 @@ test.describe('Agent Studio', () => { await modal.getByRole('button', { name: 'Cancel' }).click(); }); - test('ceiling chip is draggable but its drop is rejected server-side', async ({ page }) => { - // The locked chip may be dragged onto non-default workspaces (legitimate - // attach); swapping the DEFAULT workspace's policy is rejected both by a - // client guard and a server-side 403. - const locked = page.locator('.chip-policy.locked').first(); - await expect(locked).toHaveAttribute('draggable', 'true'); + test('policy chips are draggable', async ({ page }) => { + // Any policy may be attached to any workspace, including the default one + // β€” there is no locked plan-ceiling chip. (Missing-policy chips carry a + // .chip-tag and are the one non-draggable case.) + const chips = page.locator('.chip-policy').filter({ hasNot: page.locator('.chip-tag') }); + test.skip(await chips.count() === 0, 'no policies in live board state'); + await expect(chips.first()).toHaveAttribute('draggable', 'true'); }); test('view tabs switch between board and accounts', async ({ page }) => {