feat: organization session policy SDK methods#152
Merged
Conversation
…proto target Adds get_organization_session_policy and update_organization_session_policy wrapper methods, regenerates gRPC stubs from local protos, and adds generate-local Makefile target.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds OrganizationClient methods to get and update organization session policies (including optional absolute and idle timeout fields), a test suite validating default and custom policy behavior, and Makefile/gitignore updates to support generating protos from a local repository. ChangesOrganization Session Policy Management
Local Proto Generation Support
Sequence DiagramsequenceDiagram
participant Client
participant OrganizationClient
participant OrganizationService
Client->>OrganizationClient: get_organization_session_policy(org_id)
OrganizationClient->>OrganizationService: GetOrganizationSessionPolicy RPC
OrganizationService-->>OrganizationClient: OrganizationSessionPolicySettings
OrganizationClient-->>Client: policy (source, timeouts)
Client->>OrganizationClient: update_organization_session_policy(org_id, CUSTOM, timeouts)
OrganizationClient->>OrganizationService: UpdateOrganizationSessionPolicy RPC
OrganizationService-->>OrganizationClient: updated settings
OrganizationClient-->>Client: updated policy
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Updates generated stubs to match new flat UpdateOrganizationSessionPolicyRequest (SessionPolicyType enum rename, flat fields). Adds agentkit_logs generated module and removes stale buf/validate generated files. Adds integration test coverage for get/update/revert organization session policy.
… stubs The generate-local target was missing --include-imports, causing buf/validate generated files to be absent while generated stubs still imported them.
…ate-local target Resets pb2 files to main baseline then regenerates using local ../scalekit proto via `buf generate ../scalekit --include-imports` to avoid GitHub clone and resolve buf/validate deps correctly. Adds generate-local Makefile target for future use.
Merges origin/main (up to 67e7171), resolves pb2 conflicts by taking main's baseline, then regenerates all stubs via `make generate-local` using the local ../scalekit proto source.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
Makefile (1)
113-129: ⚡ Quick winExtract
generate-localbody into a helper to reduce drift and lint noise.This recipe is long and duplicates flow already present in
generate; factoring it out improves maintainability and addresses the checkmake warning.♻️ Suggested direction
-generate-local: tools-check - `@echo` "Using local proto sources from $(LOCAL_PROTO_REPO)..." - `@set` -euo pipefail; \ - ... \ - $(MAKE) cleanup - `@echo` "Code generation complete." +generate-local: tools-check + @./scripts/generate-local.sh "$(LOCAL_PROTO_REPO)" "$(TEMP_DIR)" "$(SCALEKIT_DIR)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 113 - 129, Extract the long recipe body from the generate-local target into a shared helper target (e.g., generate_local_helper or generate_common_generate) and have generate-local simply invoke that helper; move the shell block that sets prepared, rollback_if_needed(), trap, calls to $(MAKE) prepare/restore/generate_init_files/cleanup, and the buf generate $(LOCAL_PROTO_REPO) --include-imports line into the helper so both generate and generate-local can call it, preserving use of variables TEMP_DIR, SCALEKIT_DIR, LOCAL_PROTO_REPO and the exact control flow (prepared flag, trap, rsync rollback) to avoid behavioral changes and to satisfy the lint/checkmake rule.tests/test_organization_session_policy.py (1)
71-77: ⚡ Quick winRevert test should verify persisted state with a follow-up fetch.
Line 71–77 currently validates only the immediate update response. Add a
get_organization_session_policyassertion after revert to confirm the stored policy source is actuallyAPPLICATION.Suggested test hardening
reverted = self.scalekit_client.organization.update_organization_session_policy( organization_id=org_id, policy_source=SessionPolicyType.APPLICATION, ) self.assertIsNotNone(reverted) self.assertEqual(reverted.policy_source, SessionPolicyType.APPLICATION) + fetched = self.scalekit_client.organization.get_organization_session_policy( + organization_id=org_id + ) + self.assertEqual(fetched.policy_source, SessionPolicyType.APPLICATION)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_organization_session_policy.py` around lines 71 - 77, The test currently asserts only the immediate response from scalekit_client.organization.update_organization_session_policy (reverted) but does not verify persisted state; after calling update_organization_session_policy with policy_source=SessionPolicyType.APPLICATION, call scalekit_client.organization.get_organization_session_policy(org_id) and assert the returned policy_source equals SessionPolicyType.APPLICATION to confirm persistence; keep the existing reverted assertions and add the additional fetch + assertion using the same org_id and SessionPolicyType.APPLICATION.scalekit/organization.py (1)
243-297: ⚡ Quick winAdd client-side validation for incompatible policy arguments.
Line 273 onward builds requests even for inconsistent combinations (e.g., timeout value without unit, or custom timeout fields with
SessionPolicyType.APPLICATION). Guarding these in the SDK improves error clarity and prevents avoidable bad requests.Proposed validation patch
def update_organization_session_policy( self, organization_id: str, policy_source: SessionPolicyType, absolute_session_timeout: Optional[int] = None, absolute_session_timeout_unit: Optional[TimeUnit] = None, idle_session_timeout_enabled: Optional[bool] = None, idle_session_timeout: Optional[int] = None, idle_session_timeout_unit: Optional[TimeUnit] = None, ) -> OrganizationSessionPolicySettings: @@ + if absolute_session_timeout is not None and absolute_session_timeout_unit is None: + raise ValueError("absolute_session_timeout_unit is required when absolute_session_timeout is provided") + if idle_session_timeout is not None and idle_session_timeout_unit is None: + raise ValueError("idle_session_timeout_unit is required when idle_session_timeout is provided") + if policy_source == SessionPolicyType.APPLICATION and any( + v is not None + for v in ( + absolute_session_timeout, + absolute_session_timeout_unit, + idle_session_timeout_enabled, + idle_session_timeout, + idle_session_timeout_unit, + ) + ): + raise ValueError("Do not pass custom timeout fields when policy_source is APPLICATION") + req = UpdateOrganizationSessionPolicyRequest( organization_id=organization_id, policy_source=policy_source, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scalekit/organization.py` around lines 243 - 297, The update_organization_session_policy method currently builds requests for inconsistent argument combinations; add client-side validation in update_organization_session_policy to (1) reject any custom timeout fields (absolute_session_timeout, absolute_session_timeout_unit, idle_session_timeout_enabled, idle_session_timeout, idle_session_timeout_unit) when policy_source == SessionPolicyType.APPLICATION, (2) require that an absolute timeout value and absolute_session_timeout_unit are provided together (if one is set the other must be set), and (3) require that idle_session_timeout and idle_session_timeout_unit are provided together (and treat idle_session_timeout_enabled as only valid with SessionPolicyType.CUSTOM). When validation fails, raise a ValueError with a clear message referencing the offending parameters so callers get immediate, explicit feedback before the gRPC call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Makefile`:
- Around line 113-129: Extract the long recipe body from the generate-local
target into a shared helper target (e.g., generate_local_helper or
generate_common_generate) and have generate-local simply invoke that helper;
move the shell block that sets prepared, rollback_if_needed(), trap, calls to
$(MAKE) prepare/restore/generate_init_files/cleanup, and the buf generate
$(LOCAL_PROTO_REPO) --include-imports line into the helper so both generate and
generate-local can call it, preserving use of variables TEMP_DIR, SCALEKIT_DIR,
LOCAL_PROTO_REPO and the exact control flow (prepared flag, trap, rsync
rollback) to avoid behavioral changes and to satisfy the lint/checkmake rule.
In `@scalekit/organization.py`:
- Around line 243-297: The update_organization_session_policy method currently
builds requests for inconsistent argument combinations; add client-side
validation in update_organization_session_policy to (1) reject any custom
timeout fields (absolute_session_timeout, absolute_session_timeout_unit,
idle_session_timeout_enabled, idle_session_timeout, idle_session_timeout_unit)
when policy_source == SessionPolicyType.APPLICATION, (2) require that an
absolute timeout value and absolute_session_timeout_unit are provided together
(if one is set the other must be set), and (3) require that idle_session_timeout
and idle_session_timeout_unit are provided together (and treat
idle_session_timeout_enabled as only valid with SessionPolicyType.CUSTOM). When
validation fails, raise a ValueError with a clear message referencing the
offending parameters so callers get immediate, explicit feedback before the gRPC
call.
In `@tests/test_organization_session_policy.py`:
- Around line 71-77: The test currently asserts only the immediate response from
scalekit_client.organization.update_organization_session_policy (reverted) but
does not verify persisted state; after calling
update_organization_session_policy with
policy_source=SessionPolicyType.APPLICATION, call
scalekit_client.organization.get_organization_session_policy(org_id) and assert
the returned policy_source equals SessionPolicyType.APPLICATION to confirm
persistence; keep the existing reverted assertions and add the additional fetch
+ assertion using the same org_id and SessionPolicyType.APPLICATION.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db36111b-635c-4b82-85a9-7bd13b89e209
📒 Files selected for processing (36)
.gitignoreMakefilescalekit/organization.pyscalekit/v1/agentkit_logs/__init__.pyscalekit/v1/agentkit_logs/agentkit_analytics_pb2.pyscalekit/v1/agentkit_logs/agentkit_analytics_pb2.pyiscalekit/v1/agentkit_logs/agentkit_analytics_pb2_grpc.pyscalekit/v1/agentkit_logs/agentkit_logs_pb2.pyscalekit/v1/agentkit_logs/agentkit_logs_pb2.pyiscalekit/v1/agentkit_logs/agentkit_logs_pb2_grpc.pyscalekit/v1/auth/auth_pb2.pyscalekit/v1/auth/auth_pb2.pyiscalekit/v1/clients/clients_pb2.pyscalekit/v1/clients/clients_pb2.pyiscalekit/v1/commons/commons_pb2.pyscalekit/v1/commons/commons_pb2.pyiscalekit/v1/connected_accounts/connected_accounts_pb2.pyscalekit/v1/connected_accounts/connected_accounts_pb2.pyiscalekit/v1/connected_accounts/connected_accounts_pb2_grpc.pyscalekit/v1/connections/connections_pb2.pyscalekit/v1/connections/connections_pb2.pyiscalekit/v1/domains/domains_pb2.pyscalekit/v1/domains/domains_pb2.pyiscalekit/v1/domains/domains_pb2_grpc.pyscalekit/v1/environments/environments_pb2.pyscalekit/v1/environments/environments_pb2.pyiscalekit/v1/options/options_pb2.pyscalekit/v1/options/options_pb2.pyiscalekit/v1/organizations/organizations_pb2.pyscalekit/v1/organizations/organizations_pb2.pyiscalekit/v1/organizations/organizations_pb2_grpc.pyscalekit/v1/providers/providers_pb2.pyscalekit/v1/providers/providers_pb2.pyiscalekit/v1/tools/tools_pb2.pyscalekit/v1/tools/tools_pb2.pyitests/test_organization_session_policy.py
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
- Bump PROTO_REF to v0.1.121.2 from main - Accept main's regenerated pb2/pyi stubs - Keep proto/ in .gitignore from our branch - Add .claude/settings.json and .DS_Store to .gitignore from main
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 113-129: The generate-local target's failure rollback
(rollback_if_needed) only restores $(SCALEKIT_DIR) and leaves $(TEMP_DIR) and
intermediate generated directories behind; update rollback_if_needed (and the
EXIT trap handling) so that on failure it also deletes $(TEMP_DIR) and any
intermediate generated output directories created during buf generate, and
ensure the trap is removed or prepared reset on successful completion (after
prepare/restore/generate_init_files/cleanup) so cleanup isn't skipped; reference
the generate-local target, rollback_if_needed function, TEMP_DIR, SCALEKIT_DIR,
the trap 'rollback_if_needed' EXIT, and the existing prepare/restore/cleanup
steps when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 119e479d-b7c7-453b-9a94-d8bbd5365815
📒 Files selected for processing (2)
.gitignoreMakefile
✅ Files skipped from review due to trivial changes (1)
- .gitignore
- generate-local: rename trap handler to rollback_and_cleanup, add cleanup of TEMP_DIR and intermediate dirs on failure, and clear trap on success - test_revert_to_application_policy: add follow-up get to verify persisted state
…rsion to 2.10.0 - Remove response[0].policy extraction from get/update_organization_session_policy to match existing SDK patterns - Bump SDK version 2.9.0 → 2.10.0 - Update api_version date to 20260513
Avinash-Kamath
approved these changes
May 13, 2026
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
2 participants
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.
Summary
get_organization_session_policyandupdate_organization_session_policymethods toOrganizationClientOrganizationSessionPolicySettings,SessionPolicySource, andTimeUnitfrom generated proto stubsgenerate-localMakefile targetTest plan
make lintpassesget_organization_session_policyreturns current policy for an orgupdate_organization_session_policywithSessionPolicySource.CUSTOMapplies custom timeoutsupdate_organization_session_policywithSessionPolicySource.APPLICATIONreverts to defaultsSummary by CodeRabbit
New Features
Tests
Chores