Add cloud-only Polar subscriptions and organization quotas - #41
Open
turgaybulut wants to merge 17 commits into
Open
turgaybulut wants to merge 17 commits into
turgaybulut wants to merge 17 commits into
Conversation
turgaybulut
added this pull request to stack #42
September 11, 2026 12:32
The admission stage added "protocol" to the repeat-detection payload, but protocol was absent from the _repeat_material allowlist, so count_tokens and messages produced an identical prompt identity. A token-count call silently consumed a /v1/messages repeat allowance. Add protocol to the allowlist and assert the two share no allowance.
write_spend_denial_preflight wrote usage_summary={"spend_denied": 1},
but both audit readers query usage_summary["denial_reason"]. Nothing
consumed the new key, so spend-limit denials stopped matching the
policy_failed predicate and were counted as technical failures in the
overview summary and trend.
Restore the denial_reason key and pin the writer/reader contract.
Two cloud documents shipped false against the branch that carried them: the proposal said "not implemented" and claimed release built ee/Dockerfile with --all-packages, and the implementation file was an agent checklist. Neither had inbound references and both restated ee/cloud/openapi/cloud.json. Also collapse the required-gate block to its single maintained copy, delete a route table the contract file already owns, and sort the enterprise/cloud route profiles with an assertion so drift fails a test.
configure_organization_quota runs from the cloud auth dependency on every authenticated request and always took SELECT ... FOR UPDATE on the organization row, serializing each tenant's requests before their real work began. Read the tier first and return without locking when the allowance already matches; keep the locked re-read for the drift path. Drop two write-only OrganizationPlan fields while here.
AuditPreflightIntent types usage_summary as int | Decimal, so the writer must store spend_denied=1; the previous string denial_reason value could not validate at all. The writer was corrected earlier, but both audit readers still queried the old string key, so spend-limit denials were counted as technical failures. Align the overview and management predicates with the stored key and pin the writer/reader pairing in both drift directions.
…handlers Move the outbound timeout and retry-disabled policy to the single Polar client construction instead of repeating it at five call sites, express the sellable plan vocabulary once as a Literal, and reuse the locked Team row and shared field validators in the management routes.
Move the billing checks from a model validator to field validators so a failure stays on its own field and the merged input mapping never reaches ValidationError.errors(), which carried the raw POLAR_ACCESS_TOKEN even with hide_input_in_errors set. Add the missing CI step verifying the shim-cloud wheel and sdist against ee/cloud/LICENSE and ee/cloud/NOTICE, the Elastic-2.0 expression, and the absent community and enterprise source trees.
turgaybulut
marked this pull request as ready for review
September 16, 2026 21:08
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
229
to
236
| team_id: UUID | None = None | ||
| allowed_models: list[str] | None = Field(default=None, max_length=200) | ||
|
|
||
| @field_validator("allowed_models") | ||
| @classmethod | ||
| def validate_models(cls, value: list[str] | None) -> list[str] | None: | ||
| if value is not None and any( | ||
| not item or item != item.strip() or len(item) > 200 for item in value | ||
| ): | ||
| raise ValueError( | ||
| "Model identifiers must be nonblank and at most 200 characters" | ||
| ) | ||
| return list(dict.fromkeys(value)) if value is not None else None | ||
|
|
||
| @field_validator("cost_center", "team") | ||
| @classmethod | ||
| def validate_attribution(cls, value: str | None) -> str | None: | ||
| if value is None: | ||
| return None | ||
| return normalize_attribution( | ||
| value, | ||
| maximum_length=settings.COST_TAG_MAX_LENGTH, | ||
| ) | ||
| validate_models = field_validator("allowed_models")(_validate_allowed_models) | ||
| validate_attribution = field_validator("cost_center", "team")(_validate_attribution) | ||
|
|
||
|
|
||
| class ApiKeyView(BaseModel): |
There was a problem hiding this comment.
Bug: The refactored Pydantic validators _validate_allowed_models and _validate_attribution have an incorrect function signature, missing the cls argument, which will cause a TypeError at runtime.
Severity: HIGH
Suggested Fix
Update the _validate_allowed_models and _validate_attribution functions to accept cls as their first argument to match the signature expected by Pydantic's field_validator. For example: def _validate_allowed_models(cls, value: list[str] | None) -> list[str] | None:.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: ee/src/shim_enterprise/api/v1/management.py#L229-L236
Potential issue: The refactored validator functions `_validate_allowed_models` and
`_validate_attribution` are defined to accept only a single `value` argument. However,
they are registered using Pydantic v2's `field_validator`, which invokes validators as
classmethods, passing both `cls` and `value`. When an API key is created or patched via
the `/api-keys` endpoint, this signature mismatch will cause a `TypeError` because the
function receives two arguments instead of the expected one. This will result in a
runtime error, preventing API key creation and modification.
Did we get this right? 👍 / 👎 to inform future reviews.
Contributor
Author
There was a problem hiding this comment.
Verified as a false positive against this PR's locked Pydantic 2.13.5. field_validator(...) accepts a standalone validator callable with the signature (value); it only auto-wraps a callable as a classmethod when the first parameter is named cls. Both ApiKeyInput and ApiKeyPatch were instantiated, and their normalization, deduplication, and rejection paths were exercised without any TypeError. Adding cls is valid but unnecessary, so no code change is needed.
Reference: https://docs.pydantic.dev/latest/concepts/validators/#field-validators
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hosted cloud needs self-service subscriptions while customer-operated distributions remain free of commerce code and payment dependencies. Add a separately packaged cloud application using Polar for fixed monthly/yearly plans, with shared organization quotas and the existing enterprise entitlement controls.
Stacked on #40. This diff contains only the cloud subscription work above the enterprise foundation. Companion dashboard: https://github.com/DurthVadr/SHIM_LP/pull/37. Merge the enterprise foundations, then this backend, then the cloud dashboard.
Changes
ee/cloud; explicitly select community, on-prem and cloud packages, images and API contracts.ee/cloud/README.md.Follow-up fixes on top of the initial implementation
protocolwas passed into the repeat identity but absent from its allowlist, so/v1/messages/count_tokensconsumed a/v1/messagesrepeat allowance.AuditPreflightIntenttypesusage_summaryasint | Decimal, so the stored value must be numeric; the readers still queried the removed string key, which counted spend-limit denials as technical failures in the overview summary and trend.SELECT ... FOR UPDATEon the organization row, serializing each tenant before its real work began. The locked re-read remains for the drift path.ValidationError.errors(), which exposed the rawPOLAR_ACCESS_TOKENeven withhide_input_in_errorsset.shim-cloudwheel and sdist are now checked againstee/cloud/LICENSEandee/cloud/NOTICE, the Elastic-2.0 licence expression, theshim-enterpriserequirement, and the absence of community and enterprise source trees.OrganizationPlanfields, reuse the locked row returned byrequire_team, deduplicate the API-key field validators, and delete two superseded cloud planning documents.Validation
uv run --locked --all-packages python -m pytest -q: 972 passed, 4 skipped.git diff --checkpassed.downgrade basefor cloud then enterprise, re-upgrade, thenalembic checkon both schemas.ee/cloud/NOTICEis altered.pyproject.tomloruv.lockdiff, and no commerce dependency reaches the community package.Tests run against real PostgreSQL 16 and Redis 7 locally. No dependency was added or changed.
Before launch
Draft pending real Polar sandbox checkout and merchant/product/webhook configuration. Local tests use isolated services and mocked vendor transport; no live purchase has been performed. No deployment is requested by this PR.