fix(teams): split multi-member update ops into per-member requests - #778
Conversation
TeamCollection.update() emitted one patch op per member in a single call; api-team rejects duplicate attributes in one call, so adding or removing two or more members at once always 400s. Member add/remove ops now go out one request per member while other ops (name, role changes) stay batched. A 206 partial-success response now logs its FailedItems instead of being silently swallowed. Refs: SDK-167
|
Looks good - no bugs or correctness issues found. |
prasad-albert
left a comment
There was a problem hiding this comment.
LGTM! Verified backend contract parity, backward compatibility, caller ergonomics, and SDK conventions. Ready to merge.
# Conflicts: # tests/utils/test_patches.py
|
Looks good - no bugs or correctness issues found. |
# Conflicts: # tests/utils/test_patches.py
|
Looks good - no bugs or correctness issues found. |
# Conflicts: # tests/utils/test_patches.py
|
Looks good - no bugs or correctness issues found. |
CircleCI Integration Tests failedBranch: |
|
Integration failure is the known partial-index poll flake in test_inventory_search_with_name_only_storage_location_filter (poll stopped at first non-empty page before both seeds indexed). Fixed by the poll_until predicate in #798, pending merge. Teams diff is unrelated. |
|
Looks good - no bugs or correctness issues found. |
CircleCI Integration Tests failedBranch: |
What
TeamCollection.update()built one patch operation per member (add/remove) and sent them all in a single PATCH call. The ops for member changes all shareattribute: "ACL". This splits those member ops out so each is sent in its own request, while all other ops (name, role changes) remain batched in one call.Why
SDK-167 (audit §5.30): api-team's patch handler rejects any call containing more than one op for the same attribute (
fgc" role changes excepted) with a 400 "Duplicate entities not allowed" (Team.service.js#Validate -> duplicates()). As a result, adding or removing two or more members in oneupdate()` call always failed.How
TeamCollection._generate_patch_payloads()helper that returns one op list per request: non-member ops (name,fgcrole changes) batched first, then one request per member add/remove op (adds before removes, preserving owner-swap safety).update()sends one PATCH per op list. The yml documents no per-call op maximum; the API constraint is one op per attribute per call, so per-member requests are the split.companies.merge,lots.create,custom_templates.create): when a PATCH returns 206 withFailedItems, a warning is logged instead of silently swallowing the partial failure. When the shared helper from fix(core): surface partial-success failures and skipped items #771 (SDK-156) lands, this can migrate to it as a follow-up.Testing
tests/utils/test_patches.py(sanctioned patch-builder location; noFakeAlbertSession): multi-member split, single-member batching, and no-op cases.test_update_multiple_members(adds and removes two members in oneupdate()call) plus athird_usersession fixture, per the audit's note that tests only exercised single-member updates. Requires live credentials to run; verified with--collect-only.ruff format+ruff check(clean),pytest tests/collections/test_teams.py --collect-only -q(8 collected),pytest tests/core tests/utils tests/unit -q(117 passed).Cake session: https://agents.ai.albertinventdev.com/sessions/910a4725-9404-4336-81dd-e30bfef0ce73