feat(control-plane): idempotent grant revoke + route-wiring tests · Entire
feat(control-plane): idempotent grant revoke + route-wiring tests
827fc6c→main·
toothbrush·3w ago·2 files·+165 added/-45 removed
Two gaps in the grant revoke surface:
- Revoking an already-revoked grantee surfaced a raw 404. RemoveOrgMember, RevokeProjectAccess(ByProvider), and RevokeRepoAccess(ByProvider) now route through a shared revokeGrant helper that treats 404 as a no-op ("no such grant; nothing to revoke"), matching runControlPlaneDelete's idempotency.
- The grantee-mode -> route selection had no command-level coverage (only pure helpers were tested). Add TestGrantRemove_RouteWiring asserting --provider/--provider-user-id hits the by-provider route and --grantee-type/--grantee-id hits the typed-id route, across org/project/repo; plus TestGrantRemove_Idempotent for the 404-as-no-op behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Sessions
7504c7424c52View transcript
Changes
2
cmd/entire/cli
Mgrant.go+52/-45
Agrant_wiring_test.go+113
183 unmodified lines
184
185
186
187
188
189
190
191
192
193
194
195
187
188
189
190
191
192
193
194
195
196
102 unmodified lines
299
300
301
304
305
306
307
308
309
310
311
312
302
303
304
305
306
307
308
309
314
315
316
317
318
319
320
321
322
310
311
312
313
314
315
316
317
318
319
143 unmodified lines
463
464
465
472
473
474
475
476
477
478
479
480
466
467
468
469
470
471
472
473
482
483
484
485
486
487
488
489
490
474
475
476
477
478
479
480
481
482
483
13 unmodified lines
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
183 unmodified lines
if err != nil {
return err
}
if err := c.RemoveOrgMember(ctx, coreapi.RemoveOrgMemberParams{
OrgId: orgID,
Provider: provider,
ProviderUserId: providerUserID,
}); err != nil {
return err
}
cmd.Printf("Removed %s/%s from org %s\n", provider, providerUserID, args[0])
return nil
return revokeGrant(cmd, "Removed", fmt.Sprintf("%s/%s from org %s", provider, providerUserID, args[0]), func() error {
return c.RemoveOrgMember(ctx, coreapi.RemoveOrgMemberParams{
OrgId: orgID,
Provider: provider,
ProviderUserId: providerUserID,
})
})
}
}
Mcmd/entire/cli/grant.go+52/-45
package cli
import (
"net/http"
"net/http/httptest"
"testing"
"github.com/spf13/cobra"
"github.com/stretchr/testify/require"
)
// Valid ULID-shaped refs (26 Crockford base32 chars, no I/L/O/U) so the remove
// commands short-circuit ref resolution and issue exactly the revoke call.
const (
wiringRepoULID = "01HZX7QABCDEFGHJKMNPQRSTVW"
wiringProjULID = "01HZX7QABCDEFGHJKMNPQRSTVX"
wiringOrgULID = "01HZX7QABCDEFGHJKMNPQRSTVY"
wiringGranteeULID = "01HZX7QABCDEFGHJKMNPQRSTVZ"
)
// TestGrantRemove_RouteWiring drives the grant remove commands through cobra and
// asserts the grantee-mode → route selection: --provider/--provider-user-id must
// hit the by-provider revoke route, while --grantee-type/--grantee-id must hit
// the typed-id route. This locks in the mode→route mapping that grant_test.go's
// pure-helper tests (parseGranteeMode) can't observe.
// Not parallel: runDeleteCmd swaps the package-level activeCoreClient seam.
func TestGrantRemove_RouteWiring(t *testing.T) {
cases := []struct {
name string
newCmd func() *cobra.Command
args []string
wantPath string
}{
{
"repo/by-provider",
newGrantRepoRemoveCmd,
[]string{wiringRepoULID, "--provider", "github", "--provider-user-id", "12345"},
"/api/v1/repos/" + wiringRepoULID + "/grants/account/github/12345",
},
{
"repo/by-grantee-id",
newGrantRepoRemoveCmd,
[]string{wiringRepoULID, "--grantee-type", "account", "--grantee-id", wiringGranteeULID},
"/api/v1/repos/" + wiringRepoULID + "/grants/account/" + wiringGranteeULID,
},
{
"project/by-provider",
newGrantProjectRemoveCmd,
[]string{wiringProjULID, "--provider", "github", "--provider-user-id", "12345"},
"/api/v1/projects/" + wiringProjULID + "/grants/account/github/12345",
},
{
"project/by-grantee-id",
newGrantProjectRemoveCmd,
[]string{wiringProjULID, "--grantee-type", "account", "--grantee-id", wiringGranteeULID},
"/api/v1/projects/" + wiringProjULID + "/grants/account/" + wiringGranteeULID,
},
{
"org/by-provider",
newGrantOrgRemoveCmd,
[]string{wiringOrgULID, "--provider", "github", "--provider-user-id", "12345"},
"/api/v1/orgs/" + wiringOrgULID + "/members/github/12345",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
var gotMethod, gotPath string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
gotMethod, gotPath = r.Method, r.URL.Path
w.WriteHeader(http.StatusNoContent)
}))
t.Cleanup(srv.Close)
_, err := runDeleteCmd(t, tc.newCmd, srv.URL, tc.args...)
require.NoError(t, err)
require.Equal(t, http.MethodDelete, gotMethod)
require.Equal(t, tc.wantPath, gotPath)
})
}
}
// TestGrantRemove_Idempotent asserts that revoking an already-revoked grantee
// (the server answers 404) is a no-op success — "no such grant; nothing to
// revoke" — rather than surfacing a raw 404, matching the typed deletes.
// Not parallel: runDeleteCmd swaps the package-level activeCoreClient seam.
func TestGrantRemove_Idempotent(t *testing.T) {
cases := []struct {
name string
newCmd func() *cobra.Command
args []string
}{
{"repo/by-provider", newGrantRepoRemoveCmd, []string{wiringRepoULID, "--provider", "github", "--provider-user-id", "12345"}},
{"repo/by-grantee-id", newGrantRepoRemoveCmd, []string{wiringRepoULID, "--grantee-type", "account", "--grantee-id", wiringGranteeULID}},
{"project/by-provider", newGrantProjectRemoveCmd, []string{wiringProjULID, "--provider", "github", "--provider-user-id", "12345"}},
{"org/by-provider", newGrantOrgRemoveCmd, []string{wiringOrgULID, "--provider", "github", "--provider-user-id", "12345"}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
writeNotFoundProblem(t, w)
}))
t.Cleanup(srv.Close)
out, err := runDeleteCmd(t, tc.newCmd, srv.URL, tc.args...)
require.NoError(t, err)
require.Contains(t, out, "nothing to revoke")
})
}
}