Detailed changes
@@ -21,11 +21,13 @@ func (m *mockBashPermissionService) Request(ctx context.Context, req permission.
return true, nil
}
-func (m *mockBashPermissionService) Grant(req permission.PermissionRequest) {}
+func (m *mockBashPermissionService) Grant(req permission.PermissionRequest) bool { return true }
-func (m *mockBashPermissionService) Deny(req permission.PermissionRequest) {}
+func (m *mockBashPermissionService) Deny(req permission.PermissionRequest) bool { return true }
-func (m *mockBashPermissionService) GrantPersistent(req permission.PermissionRequest) {}
+func (m *mockBashPermissionService) GrantPersistent(req permission.PermissionRequest) bool {
+ return true
+}
func (m *mockBashPermissionService) AutoApproveSession(sessionID string) {}
@@ -90,11 +92,13 @@ func (m *recordingPermissionService) Request(ctx context.Context, req permission
return m.allow, nil
}
-func (m *recordingPermissionService) Grant(req permission.PermissionRequest) {}
+func (m *recordingPermissionService) Grant(req permission.PermissionRequest) bool { return true }
-func (m *recordingPermissionService) Deny(req permission.PermissionRequest) {}
+func (m *recordingPermissionService) Deny(req permission.PermissionRequest) bool { return true }
-func (m *recordingPermissionService) GrantPersistent(req permission.PermissionRequest) {}
+func (m *recordingPermissionService) GrantPersistent(req permission.PermissionRequest) bool {
+ return true
+}
func (m *recordingPermissionService) AutoApproveSession(sessionID string) {}
@@ -20,11 +20,13 @@ func (m *mockPermissionService) Request(ctx context.Context, req permission.Crea
return true, nil
}
-func (m *mockPermissionService) Grant(req permission.PermissionRequest) {}
+func (m *mockPermissionService) Grant(req permission.PermissionRequest) bool { return true }
-func (m *mockPermissionService) Deny(req permission.PermissionRequest) {}
+func (m *mockPermissionService) Deny(req permission.PermissionRequest) bool { return true }
-func (m *mockPermissionService) GrantPersistent(req permission.PermissionRequest) {}
+func (m *mockPermissionService) GrantPersistent(req permission.PermissionRequest) bool {
+ return true
+}
func (m *mockPermissionService) AutoApproveSession(sessionID string) {}
@@ -216,11 +216,13 @@ func (m *mockViewPermissionService) Request(ctx context.Context, req permission.
return true, nil
}
-func (m *mockViewPermissionService) Grant(req permission.PermissionRequest) {}
+func (m *mockViewPermissionService) Grant(req permission.PermissionRequest) bool { return true }
-func (m *mockViewPermissionService) Deny(req permission.PermissionRequest) {}
+func (m *mockViewPermissionService) Deny(req permission.PermissionRequest) bool { return true }
-func (m *mockViewPermissionService) GrantPersistent(req permission.PermissionRequest) {}
+func (m *mockViewPermissionService) GrantPersistent(req permission.PermissionRequest) bool {
+ return true
+}
func (m *mockViewPermissionService) AutoApproveSession(sessionID string) {}
@@ -6,11 +6,13 @@ import (
)
// GrantPermission grants, denies, or persistently grants a permission
-// request.
-func (b *Backend) GrantPermission(workspaceID string, req proto.PermissionGrant) error {
+// request. The returned bool reports whether this call resolved the
+// pending request (true) or found it already resolved by a previous
+// caller (false). A false return is not an error.
+func (b *Backend) GrantPermission(workspaceID string, req proto.PermissionGrant) (bool, error) {
ws, err := b.GetWorkspace(workspaceID)
if err != nil {
- return err
+ return false, err
}
perm := permission.PermissionRequest{
@@ -26,15 +28,14 @@ func (b *Backend) GrantPermission(workspaceID string, req proto.PermissionGrant)
switch req.Action {
case proto.PermissionAllow:
- ws.Permissions.Grant(perm)
+ return ws.Permissions.Grant(perm), nil
case proto.PermissionAllowForSession:
- ws.Permissions.GrantPersistent(perm)
+ return ws.Permissions.GrantPersistent(perm), nil
case proto.PermissionDeny:
- ws.Permissions.Deny(perm)
+ return ws.Permissions.Deny(perm), nil
default:
- return ErrInvalidPermissionAction
+ return false, ErrInvalidPermissionAction
}
- return nil
}
// SetPermissionsSkip sets whether permission prompts are skipped.
@@ -490,17 +490,25 @@ func (c *Client) ListSessions(ctx context.Context, id string) ([]proto.Session,
return sessions, nil
}
-// GrantPermission grants a permission on a workspace.
-func (c *Client) GrantPermission(ctx context.Context, id string, req proto.PermissionGrant) error {
+// GrantPermission grants a permission on a workspace. The returned
+// bool reports whether this call resolved the pending request (true)
+// or found it already resolved by a previous caller (false). A false
+// value is not an error — it just means another subscriber resolved
+// the same request first.
+func (c *Client) GrantPermission(ctx context.Context, id string, req proto.PermissionGrant) (bool, error) {
rsp, err := c.post(ctx, fmt.Sprintf("/workspaces/%s/permissions/grant", id), nil, jsonBody(req), http.Header{"Content-Type": []string{"application/json"}})
if err != nil {
- return fmt.Errorf("failed to grant permission: %w", err)
+ return false, fmt.Errorf("failed to grant permission: %w", err)
}
defer rsp.Body.Close()
if rsp.StatusCode != http.StatusOK {
- return fmt.Errorf("failed to grant permission: status code %d", rsp.StatusCode)
+ return false, fmt.Errorf("failed to grant permission: status code %d", rsp.StatusCode)
}
- return nil
+ var resp proto.PermissionGrantResponse
+ if err := json.NewDecoder(rsp.Body).Decode(&resp); err != nil {
+ return false, fmt.Errorf("failed to decode grant permission response: %w", err)
+ }
+ return resp.Resolved, nil
}
// SetPermissionsSkipRequests sets the skip-requests flag for a workspace.
@@ -63,9 +63,19 @@ type PermissionRequest struct {
type Service interface {
pubsub.Subscriber[PermissionRequest]
- GrantPersistent(permission PermissionRequest)
- Grant(permission PermissionRequest)
- Deny(permission PermissionRequest)
+ // GrantPersistent grants a permission request and remembers the grant
+ // for the session. It returns true if this call actually resolved the
+ // pending request; false if the request had already been resolved
+ // (e.g., by another concurrent caller) or is unknown.
+ GrantPersistent(permission PermissionRequest) bool
+ // Grant grants a permission request. It returns true if this call
+ // actually resolved the pending request; false if the request had
+ // already been resolved or is unknown.
+ Grant(permission PermissionRequest) bool
+ // Deny denies a permission request. It returns true if this call
+ // actually resolved the pending request; false if the request had
+ // already been resolved or is unknown.
+ Deny(permission PermissionRequest) bool
Request(ctx context.Context, opts CreatePermissionRequest) (bool, error)
AutoApproveSession(sessionID string)
SetSkipRequests(skip bool)
@@ -99,63 +109,72 @@ type permissionService struct {
activeRequestMu sync.Mutex
}
-func (s *permissionService) GrantPersistent(permission PermissionRequest) {
- s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
- ToolCallID: permission.ToolCallID,
- Granted: true,
- })
- respCh, ok := s.pendingRequests.Get(permission.ID)
- if ok {
- respCh <- true
+// resolve atomically removes the pending request entry for the given
+// permission and, if it was still pending, publishes exactly one
+// PermissionNotification and forwards the outcome to the waiter on
+// respCh. It returns true if this call resolved the request, false if
+// it had already been resolved (e.g., by another concurrent caller) or
+// the request ID is unknown.
+//
+// If onResolve is non-nil it runs after the pending entry has been
+// taken but before the notification is published or the waiter is
+// unblocked. This lets GrantPersistent record the session permission
+// only when it actually wins the race, so a losing GrantPersistent
+// that lost to a Deny does not leak an auto-approve entry.
+//
+// All three public resolution methods (Grant, GrantPersistent, Deny)
+// route through this helper so multi-subscriber UIs can race safely:
+// the first caller wins, the rest become no-ops.
+func (s *permissionService) resolve(permission PermissionRequest, granted, denied bool, onResolve func()) bool {
+ respCh, ok := s.pendingRequests.Take(permission.ID)
+ if !ok {
+ return false
}
- s.sessionPermissions.Set(PermissionKey{
- SessionID: permission.SessionID,
- ToolName: permission.ToolName,
- Action: permission.Action,
- Path: permission.Path,
- }, true)
-
- s.activeRequestMu.Lock()
- if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
- s.activeRequest = nil
+ if onResolve != nil {
+ onResolve()
}
- s.activeRequestMu.Unlock()
-}
-func (s *permissionService) Grant(permission PermissionRequest) {
s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
ToolCallID: permission.ToolCallID,
- Granted: true,
+ Granted: granted,
+ Denied: denied,
})
- respCh, ok := s.pendingRequests.Get(permission.ID)
- if ok {
- respCh <- true
- }
+
+ // respCh is buffered (cap 1) and only ever has at most one sender
+ // per request because Take removes the entry under the map lock,
+ // so this send never blocks.
+ respCh <- granted
s.activeRequestMu.Lock()
if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
s.activeRequest = nil
}
s.activeRequestMu.Unlock()
+ return true
}
-func (s *permissionService) Deny(permission PermissionRequest) {
- s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
- ToolCallID: permission.ToolCallID,
- Granted: false,
- Denied: true,
+func (s *permissionService) GrantPersistent(permission PermissionRequest) bool {
+ // Record the persistent grant only if this call wins the
+ // pending-request race. Otherwise a losing GrantPersistent that
+ // lost to a Deny would still leave an auto-approve entry behind,
+ // silently flipping later denied calls to allowed.
+ return s.resolve(permission, true, false, func() {
+ s.sessionPermissions.Set(PermissionKey{
+ SessionID: permission.SessionID,
+ ToolName: permission.ToolName,
+ Action: permission.Action,
+ Path: permission.Path,
+ }, true)
})
- respCh, ok := s.pendingRequests.Get(permission.ID)
- if ok {
- respCh <- false
- }
+}
- s.activeRequestMu.Lock()
- if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
- s.activeRequest = nil
- }
- s.activeRequestMu.Unlock()
+func (s *permissionService) Grant(permission PermissionRequest) bool {
+ return s.resolve(permission, true, false, nil)
+}
+
+func (s *permissionService) Deny(permission PermissionRequest) bool {
+ return s.resolve(permission, false, true, nil)
}
func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRequest) (bool, error) {
@@ -2,7 +2,9 @@ package permission
import (
"sync"
+ "sync/atomic"
"testing"
+ "time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -327,3 +329,272 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
assert.True(t, result, "Repeated request should be auto-approved due to persistent permission")
})
}
+
+// TestPermissionService_ResolveIdempotency covers the multi-subscriber
+// resolve guarantees added for client/server mode: exactly one
+// notification per resolution, racing callers see "already resolved",
+// and stray Grant/Deny calls for unknown IDs are safe no-ops.
+func TestPermissionService_ResolveIdempotency(t *testing.T) {
+ t.Parallel()
+
+ t.Run("concurrent grants resolve exactly once", func(t *testing.T) {
+ t.Parallel()
+ service := NewPermissionService("/tmp", false, nil)
+
+ events := service.Subscribe(t.Context())
+ notifications := service.SubscribeNotifications(t.Context())
+
+ req := CreatePermissionRequest{
+ SessionID: "race-session",
+ ToolCallID: "race-call",
+ ToolName: "tool",
+ Action: "act",
+ Path: "/tmp/race",
+ }
+
+ var (
+ wg sync.WaitGroup
+ granted bool
+ requestErr error
+ )
+ wg.Go(func() {
+ granted, requestErr = service.Request(t.Context(), req)
+ })
+
+ // Wait for the request to be published so we have a real
+ // PermissionRequest (with its server-side ID) to race on.
+ var pending PermissionRequest
+ select {
+ case ev := <-events:
+ pending = ev.Payload
+ case <-time.After(2 * time.Second):
+ t.Fatal("permission request was never published")
+ }
+
+ // Drain the initial "request opened" notification (Granted ==
+ // false && Denied == false) so the next read is the resolution
+ // itself.
+ select {
+ case ev := <-notifications:
+ require.False(t, ev.Payload.Granted, "initial notification must not be granted")
+ require.False(t, ev.Payload.Denied, "initial notification must not be denied")
+ case <-time.After(2 * time.Second):
+ t.Fatal("initial notification was never published")
+ }
+
+ // Race two grants from two goroutines.
+ var (
+ resolvedCount atomic.Int32
+ start = make(chan struct{})
+ racers sync.WaitGroup
+ )
+ for range 2 {
+ racers.Go(func() {
+ <-start
+ if service.Grant(pending) {
+ resolvedCount.Add(1)
+ }
+ })
+ }
+ close(start)
+ racers.Wait()
+
+ // Original Request must return granted exactly once.
+ wg.Wait()
+ require.NoError(t, requestErr)
+ assert.True(t, granted, "request should observe its grant")
+
+ // Exactly one of the two grants resolved the request.
+ assert.Equal(t, int32(1), resolvedCount.Load(),
+ "exactly one Grant should report it resolved the request")
+
+ // Exactly one resolution notification, and no further ones.
+ select {
+ case ev := <-notifications:
+ assert.True(t, ev.Payload.Granted, "resolution notification should be granted")
+ assert.Equal(t, "race-call", ev.Payload.ToolCallID)
+ case <-time.After(2 * time.Second):
+ t.Fatal("resolution notification was never published")
+ }
+ select {
+ case ev := <-notifications:
+ t.Fatalf("unexpected duplicate notification: %+v", ev.Payload)
+ case <-time.After(50 * time.Millisecond):
+ // good: no duplicate.
+ }
+
+ // pendingRequests must be empty: no goroutine is left blocked
+ // on a send, and a future Grant for the same ID is a no-op.
+ ps := service.(*permissionService)
+ assert.Equal(t, 0, ps.pendingRequests.Len(),
+ "pendingRequests must be empty after resolution")
+
+ assert.False(t, service.Grant(pending),
+ "a third Grant should report already-resolved")
+ })
+
+ t.Run("grant after deny is a no-op", func(t *testing.T) {
+ t.Parallel()
+ service := NewPermissionService("/tmp", false, nil)
+
+ events := service.Subscribe(t.Context())
+ notifications := service.SubscribeNotifications(t.Context())
+
+ req := CreatePermissionRequest{
+ SessionID: "deny-first",
+ ToolCallID: "df-call",
+ ToolName: "tool",
+ Action: "act",
+ Path: "/tmp/df",
+ }
+
+ var (
+ wg sync.WaitGroup
+ granted bool
+ requestErr error
+ )
+ wg.Go(func() {
+ granted, requestErr = service.Request(t.Context(), req)
+ })
+
+ var pending PermissionRequest
+ select {
+ case ev := <-events:
+ pending = ev.Payload
+ case <-time.After(2 * time.Second):
+ t.Fatal("permission request was never published")
+ }
+
+ // Drain the initial neither-granted-nor-denied notification.
+ <-notifications
+
+ assert.True(t, service.Deny(pending), "Deny should resolve the request")
+ wg.Wait()
+ require.NoError(t, requestErr)
+ assert.False(t, granted, "request should observe denial")
+
+ // A follow-up Grant must be a no-op and must not flip the
+ // outcome or publish anything new.
+ assert.False(t, service.Grant(pending),
+ "Grant after Deny should report already-resolved")
+
+ select {
+ case ev := <-notifications:
+ // The first resolution notification (denial) is expected;
+ // anything after that is a bug.
+ require.True(t, ev.Payload.Denied,
+ "the only post-initial notification must be the denial")
+ case <-time.After(2 * time.Second):
+ t.Fatal("denial notification was never published")
+ }
+ select {
+ case ev := <-notifications:
+ t.Fatalf("Grant after Deny must not publish: %+v", ev.Payload)
+ case <-time.After(50 * time.Millisecond):
+ // good.
+ }
+ })
+
+ t.Run("losing GrantPersistent does not record session permission", func(t *testing.T) {
+ t.Parallel()
+ service := NewPermissionService("/tmp", false, nil)
+
+ events := service.Subscribe(t.Context())
+ notifications := service.SubscribeNotifications(t.Context())
+
+ req := CreatePermissionRequest{
+ SessionID: "race-persist",
+ ToolCallID: "rp-call",
+ ToolName: "tool",
+ Action: "act",
+ Path: "/tmp/rp",
+ }
+
+ var (
+ wg sync.WaitGroup
+ granted bool
+ requestErr error
+ )
+ wg.Go(func() {
+ granted, requestErr = service.Request(t.Context(), req)
+ })
+
+ // Wait for the request to be published so we have the real
+ // pending PermissionRequest to race on.
+ var pending PermissionRequest
+ select {
+ case ev := <-events:
+ pending = ev.Payload
+ case <-time.After(2 * time.Second):
+ t.Fatal("permission request was never published")
+ }
+
+ // Drain the initial neither-granted-nor-denied notification.
+ <-notifications
+
+ // Deny wins, then a competing GrantPersistent loses.
+ assert.True(t, service.Deny(pending), "Deny should resolve the request")
+ assert.False(t, service.GrantPersistent(pending),
+ "GrantPersistent after Deny should report already-resolved")
+
+ wg.Wait()
+ require.NoError(t, requestErr)
+ assert.False(t, granted, "request should observe denial")
+
+ // The losing GrantPersistent must not have inserted an
+ // auto-approve entry. Issue a matching follow-up request and
+ // confirm the service still publishes a pending request (i.e.
+ // not auto-approved). We then Deny it to drain the goroutine.
+ var (
+ wg2 sync.WaitGroup
+ granted2 bool
+ requestErr2 error
+ )
+ wg2.Go(func() {
+ granted2, requestErr2 = service.Request(t.Context(), req)
+ })
+
+ select {
+ case ev := <-events:
+ assert.Equal(t, pending.SessionID, ev.Payload.SessionID)
+ service.Deny(ev.Payload)
+ case <-time.After(2 * time.Second):
+ t.Fatal("follow-up request was auto-approved; persistent grant leaked")
+ }
+
+ wg2.Wait()
+ require.NoError(t, requestErr2)
+ assert.False(t, granted2, "follow-up request should be denied, not auto-approved")
+ })
+
+ t.Run("grant for unknown id is a safe no-op", func(t *testing.T) {
+ t.Parallel()
+ service := NewPermissionService("/tmp", false, nil)
+
+ notifications := service.SubscribeNotifications(t.Context())
+
+ bogus := PermissionRequest{
+ ID: "does-not-exist",
+ ToolCallID: "ghost",
+ ToolName: "tool",
+ Action: "act",
+ Path: "/tmp/ghost",
+ }
+
+ assert.NotPanics(t, func() {
+ assert.False(t, service.Grant(bogus),
+ "Grant for unknown ID should report already-resolved")
+ assert.False(t, service.GrantPersistent(bogus),
+ "GrantPersistent for unknown ID should report already-resolved")
+ assert.False(t, service.Deny(bogus),
+ "Deny for unknown ID should report already-resolved")
+ })
+
+ select {
+ case ev := <-notifications:
+ t.Fatalf("unknown-ID resolution must not publish: %+v", ev.Payload)
+ case <-time.After(50 * time.Millisecond):
+ // good: no notification.
+ }
+ })
+}
@@ -86,6 +86,15 @@ type PermissionGrant struct {
Action PermissionAction `json:"action"`
}
+// PermissionGrantResponse is the server's response to a permission
+// grant call. Resolved is true when this call resolved the pending
+// request, and false when the request had already been resolved by a
+// previous caller (e.g., another client in a multi-subscriber UI). A
+// false value is not an error.
+type PermissionGrantResponse struct {
+ Resolved bool `json:"resolved"`
+}
+
// PermissionSkipRequest represents a request to skip permission prompts.
type PermissionSkipRequest struct {
Skip bool `json:"skip"`
@@ -898,7 +898,7 @@ func (c *controllerV1) handleGetWorkspaceAgentDefaultSmallModel(w http.ResponseW
// @Accept json
// @Param id path string true "Workspace ID"
// @Param request body proto.PermissionGrant true "Permission grant"
-// @Success 200
+// @Success 200 {object} proto.PermissionGrantResponse
// @Failure 400 {object} proto.Error
// @Failure 404 {object} proto.Error
// @Failure 500 {object} proto.Error
@@ -913,11 +913,12 @@ func (c *controllerV1) handlePostWorkspacePermissionsGrant(w http.ResponseWriter
return
}
- if err := c.backend.GrantPermission(id, req); err != nil {
+ resolved, err := c.backend.GrantPermission(id, req)
+ if err != nil {
c.handleError(w, r, err)
return
}
- w.WriteHeader(http.StatusOK)
+ jsonEncode(w, proto.PermissionGrantResponse{Resolved: resolved})
}
// handlePostWorkspacePermissionsSkip sets whether to skip permission prompts.
@@ -173,16 +173,16 @@ func (w *AppWorkspace) GetDefaultSmallModel(providerID string) config.SelectedMo
// -- Permissions --
-func (w *AppWorkspace) PermissionGrant(perm permission.PermissionRequest) {
- w.app.Permissions.Grant(perm)
+func (w *AppWorkspace) PermissionGrant(perm permission.PermissionRequest) bool {
+ return w.app.Permissions.Grant(perm)
}
-func (w *AppWorkspace) PermissionGrantPersistent(perm permission.PermissionRequest) {
- w.app.Permissions.GrantPersistent(perm)
+func (w *AppWorkspace) PermissionGrantPersistent(perm permission.PermissionRequest) bool {
+ return w.app.Permissions.GrantPersistent(perm)
}
-func (w *AppWorkspace) PermissionDeny(perm permission.PermissionRequest) {
- w.app.Permissions.Deny(perm)
+func (w *AppWorkspace) PermissionDeny(perm permission.PermissionRequest) bool {
+ return w.app.Permissions.Deny(perm)
}
func (w *AppWorkspace) PermissionSkipRequests() bool {
@@ -244,8 +244,8 @@ func (w *ClientWorkspace) GetDefaultSmallModel(providerID string) config.Selecte
// -- Permissions --
-func (w *ClientWorkspace) PermissionGrant(perm permission.PermissionRequest) {
- _ = w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
+func (w *ClientWorkspace) PermissionGrant(perm permission.PermissionRequest) bool {
+ resolved, _ := w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
Permission: proto.PermissionRequest{
ID: perm.ID,
SessionID: perm.SessionID,
@@ -256,12 +256,13 @@ func (w *ClientWorkspace) PermissionGrant(perm permission.PermissionRequest) {
Path: perm.Path,
Params: perm.Params,
},
- Action: proto.PermissionAllowForSession,
+ Action: proto.PermissionAllow,
})
+ return resolved
}
-func (w *ClientWorkspace) PermissionGrantPersistent(perm permission.PermissionRequest) {
- _ = w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
+func (w *ClientWorkspace) PermissionGrantPersistent(perm permission.PermissionRequest) bool {
+ resolved, _ := w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
Permission: proto.PermissionRequest{
ID: perm.ID,
SessionID: perm.SessionID,
@@ -272,12 +273,13 @@ func (w *ClientWorkspace) PermissionGrantPersistent(perm permission.PermissionRe
Path: perm.Path,
Params: perm.Params,
},
- Action: proto.PermissionAllow,
+ Action: proto.PermissionAllowForSession,
})
+ return resolved
}
-func (w *ClientWorkspace) PermissionDeny(perm permission.PermissionRequest) {
- _ = w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
+func (w *ClientWorkspace) PermissionDeny(perm permission.PermissionRequest) bool {
+ resolved, _ := w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
Permission: proto.PermissionRequest{
ID: perm.ID,
SessionID: perm.SessionID,
@@ -290,6 +292,7 @@ func (w *ClientWorkspace) PermissionDeny(perm permission.PermissionRequest) {
},
Action: proto.PermissionDeny,
})
+ return resolved
}
func (w *ClientWorkspace) PermissionSkipRequests() bool {
@@ -1,9 +1,16 @@
package workspace
import (
+ "encoding/json"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
"testing"
+ "github.com/charmbracelet/crush/internal/client"
"github.com/charmbracelet/crush/internal/message"
+ "github.com/charmbracelet/crush/internal/permission"
"github.com/charmbracelet/crush/internal/proto"
"github.com/stretchr/testify/require"
)
@@ -44,3 +51,84 @@ func TestProtoToMessageToolResult(t *testing.T) {
require.Equal(t, `{"file_path":"/tmp/x","content":"hi"}`, tr.Metadata)
require.False(t, tr.IsError)
}
+
+// TestClientWorkspace_PermissionGrantMapping verifies that
+// PermissionGrant on the ClientWorkspace serializes a one-time grant
+// (proto.PermissionAllow) and PermissionGrantPersistent serializes a
+// persistent grant (proto.PermissionAllowForSession). A swap between
+// these two would silently flip "allow once" into "remember for the
+// session", and vice versa, so we pin the wire mapping here.
+func TestClientWorkspace_PermissionGrantMapping(t *testing.T) {
+ t.Parallel()
+
+ cases := []struct {
+ name string
+ call func(*ClientWorkspace, permission.PermissionRequest)
+ want proto.PermissionAction
+ }{
+ {
+ name: "Grant -> PermissionAllow",
+ call: func(w *ClientWorkspace, p permission.PermissionRequest) {
+ w.PermissionGrant(p)
+ },
+ want: proto.PermissionAllow,
+ },
+ {
+ name: "GrantPersistent -> PermissionAllowForSession",
+ call: func(w *ClientWorkspace, p permission.PermissionRequest) {
+ w.PermissionGrantPersistent(p)
+ },
+ want: proto.PermissionAllowForSession,
+ },
+ {
+ name: "Deny -> PermissionDeny",
+ call: func(w *ClientWorkspace, p permission.PermissionRequest) {
+ w.PermissionDeny(p)
+ },
+ want: proto.PermissionDeny,
+ },
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+
+ var got proto.PermissionGrant
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ require.Equal(t, http.MethodPost, r.Method)
+ require.Equal(t, "/v1/workspaces/ws-1/permissions/grant", r.URL.Path)
+ body, err := io.ReadAll(r.Body)
+ require.NoError(t, err)
+ require.NoError(t, json.Unmarshal(body, &got))
+ require.NoError(t, json.NewEncoder(w).Encode(proto.PermissionGrantResponse{Resolved: true}))
+ }))
+ defer srv.Close()
+
+ u, err := url.Parse(srv.URL)
+ require.NoError(t, err)
+ c, err := client.NewClient(t.TempDir(), "tcp", u.Host)
+ require.NoError(t, err)
+
+ ws := NewClientWorkspace(c, proto.Workspace{ID: "ws-1"})
+
+ perm := permission.PermissionRequest{
+ ID: "req-1",
+ SessionID: "sess-1",
+ ToolCallID: "tc-1",
+ ToolName: "tool",
+ Description: "do thing",
+ Action: "act",
+ Path: "/tmp/p",
+ }
+ tc.call(ws, perm)
+
+ require.Equal(t, tc.want, got.Action)
+ require.Equal(t, "req-1", got.Permission.ID)
+ require.Equal(t, "sess-1", got.Permission.SessionID)
+ require.Equal(t, "tc-1", got.Permission.ToolCallID)
+ require.Equal(t, "tool", got.Permission.ToolName)
+ require.Equal(t, "act", got.Permission.Action)
+ require.Equal(t, "/tmp/p", got.Permission.Path)
+ })
+ }
+}
@@ -89,9 +89,17 @@ type Workspace interface {
GetDefaultSmallModel(providerID string) config.SelectedModel
// Permissions
- PermissionGrant(perm permission.PermissionRequest)
- PermissionGrantPersistent(perm permission.PermissionRequest)
- PermissionDeny(perm permission.PermissionRequest)
+ //
+ // PermissionGrant, PermissionGrantPersistent, and PermissionDeny
+ // return true if the call resolved the pending request and false if
+ // it had already been resolved by another subscriber (or is no
+ // longer pending). A false return is not an error; the modal can
+ // still close locally because the resolution will arrive via the
+ // PermissionNotification event stream regardless of which client
+ // won the race.
+ PermissionGrant(perm permission.PermissionRequest) bool
+ PermissionGrantPersistent(perm permission.PermissionRequest) bool
+ PermissionDeny(perm permission.PermissionRequest) bool
PermissionSkipRequests() bool
PermissionSetSkipRequests(skip bool)