test: cover project users, organization users and teams

[pjcdawkins]

Adds integration tests for the commands that grant or revoke access, and the mock API they need.

Mock API (pkg/mockapi), in new files to keep edits to shared files small:

• user_access.go: project user-access GET/PATCH/DELETE (items now have self/update/delete links), project and organization invitations (409 when already invited, unless forced), organization members, and project environment types.
• teams.go: teams, team members, team project access, and /projects/{id}/team-access.
• Projects can have an owner.

None of these changes return activities: the CLI does not wait on them with centralized permissions, which the test config already enables.

Tests:

• user:add, user:update, user:delete: invitations (including a repeated one), role merging and changes, the project owner guards, validation errors, and deletion with and without confirmation. user:get at the environment level, for an unknown user, and with the deprecated --role option.
• organization:user:add/update/delete/get/list: invitation, adding an existing user, permission changes, "already set", the owner guard, and deletion with and without confirmation.
• team:create (roles, --output-id, label uniqueness and conflicts), team:get/list/update/delete, team:project:add/delete/list, team:user:add/delete/list.

🤖 Generated with Claude Code

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 2 minor points

🔍 Full review · 10 files reviewed

🔵 Minor points

• pkg/mockapi/teams.go:358 — handleAddTeamProjects validates and appends inside the same loop. Take a body like [{"project_id":"betaprojectid"},{"project_id":"unknown"}]: the first item is appended to h.teamProjects[teamID] before the second fails, and the handler then returns 400. The request is reported as failed, but the team now has access to betaprojectid. A later team:projects call, or an assertion that a failed add changed nothing, sees that partial state. To fix it, validate every item before appending any.
• pkg/mockapi/user_access.go:306 — handlePatchOrgMember writes h.orgMembers[orgID][i] = &patched straight into the backing array. That array is the slice the test passed to SetOrgMembers, so a PATCH changes the caller's slice. handlePatchProjectUserAccess (line 178) does the same with the SetUserGrants slice. The delete handlers in this file call slices.Clone first to avoid exactly this aliasing, so the two patch handlers break the convention and leak mutations into the test's own data.

Verification

• handleDeleteTeam removes the team's members and project access too, so /projects/{id}/team-access never lists a deleted team.
• lowercaseAlphanumericID and the /ref/users handler, which derives emails as <id>@example.com, exist in pkg/mockapi, so the expected user-id-2@example.com strings in the tests match what the mock serves.
• The project and org invitation handlers return 409 only when force is false, so the repeated user:add (confirmed, then forced) records a second invitation, as TestUserAdd asserts.
• All new handlers lock the embedded store: RLock for reads and Lock for writes, including initTeams, which documents that its caller holds the lock.

The change is test-only: new integration tests in integration-tests/ (org_user_test.go, team_test.go, user_access_test.go and the extended user_get_test.go) run against new mock API handlers. Nothing tests the mock handlers themselves beyond those integration tests, which run through the repository's make test-go target.

Review details

• Commit: e8a740d
• Model: claude-opus-5-5

---

Review 1 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 1 minor point

🔍 Full review · 10 files reviewed

🔵 Minor point

• pkg/mockapi/teams.go:137 — handleListTeams always sorts by Label and only checks whether sort starts with -. The field name is never read. TeamListCommand sends sort=<field> from its --sort option, so a request like teams --sort created_at (or updated_at) still comes back in label order. A test that relies on any sort field other than label would get the wrong order with no warning.

Verification

• handleAddTeamProjects validates every project ID (it must exist and belong to the team's organization) before appending any access.
• handlePatchOrgMember and handlePatchProjectUserAccess both clone the slice before replacing the element, so the slices tests pass to SetOrgMembers/SetUserGrants are left unchanged.
• SetTeams resets the member and project maps, and setupTeams calls SetTeamMembers/SetTeamProjects after it, so the seeded data survives.
• handleProjectUserAccess now returns items built by newProjectUserAccess, which embeds *ProjectUserGrant, so the JSON keeps the same fields and adds self/update/delete links.

The new integration tests (TestUserAdd/Update/Delete, TestOrgUser, TestTeam*, plus the extra TestUserGet cases) exercise the new mock API routes. They run under the repository's Go integration test job. Apart from what those tests call, nothing covers the mock handlers.

Review details

• Commit: 218b38b
• Model: claude-opus-5-5

---

Review 3 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 1 minor point

🔍 Full review · 10 files reviewed

🔵 Minor point

• pkg/mockapi/teams.go:414 — handleAddTeamProjects checks each item only against the team's existing h.teamProjects[teamID]. It never checks items against each other in the same request body. A body like [{"project_id":"betaprojectid"},{"project_id":"betaprojectid"}] passes validation, and the second loop then appends two TeamProjectAccess entries for the same project. team:projects then lists the project twice and project_count counts it twice. Calling the same endpoint twice gives 409 instead, so the duplicate is accepted only when it arrives in a single request.

Verification

• handleAddTeamProjects now runs every validation check before it appends anything, so a failed request leaves teamProjects unchanged.
• handlePatchProjectUserAccess and handlePatchOrgMember call slices.Clone on the stored slice before they write to an index, so tests' slices are not mutated.
• handleListTeams handles the sort fields label, created_at and updated_at, and returns 400 for any other field; TeamListCommand sends sort=label by default, which the label-order assertions rely on.
• The helpers the new files use (timeCompare, lowercaseAlphanumericID, writeProblem, uniqueMap) exist in the base package.

The change is test-only: new integration tests in integration-tests/{org_user,team,user_access}_test.go and new cases in user_get_test.go, all run against the extended mock API. The repository's Go test target (make test-go) runs them. Nothing tests the mock handlers directly.

Review details

• Commit: 94dd22a
• Model: claude-opus-5-5

---

Review 6 of 10 for this pull request · View the full run