@@ -2,7 +2,9 @@ package permission
22
33import (
44 "sync"
5+ "sync/atomic"
56 "testing"
7+ "time"
68
79 "github.com/stretchr/testify/assert"
810 "github.com/stretchr/testify/require"
@@ -327,3 +329,272 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
327329 assert.True(t, result, "Repeated request should be auto-approved due to persistent permission")
328330 })
329331}
332+
333+// TestPermissionService_ResolveIdempotency covers the multi-subscriber
334+// resolve guarantees added for client/server mode: exactly one
335+// notification per resolution, racing callers see "already resolved",
336+// and stray Grant/Deny calls for unknown IDs are safe no-ops.
337+func TestPermissionService_ResolveIdempotency(t *testing.T) {
338+ t.Parallel()
339+
340+ t.Run("concurrent grants resolve exactly once", func(t *testing.T) {
341+ t.Parallel()
342+ service := NewPermissionService("/tmp", false, nil)
343+
344+ events := service.Subscribe(t.Context())
345+ notifications := service.SubscribeNotifications(t.Context())
346+
347+ req := CreatePermissionRequest{
348+ SessionID: "race-session",
349+ ToolCallID: "race-call",
350+ ToolName: "tool",
351+ Action: "act",
352+ Path: "/tmp/race",
353+ }
354+
355+ var (
356+ wg sync.WaitGroup
357+ granted bool
358+ requestErr error
359+ )
360+ wg.Go(func() {
361+ granted, requestErr = service.Request(t.Context(), req)
362+ })
363+
364+ // Wait for the request to be published so we have a real
365+ // PermissionRequest (with its server-side ID) to race on.
366+ var pending PermissionRequest
367+ select {
368+ case ev := <-events:
369+ pending = ev.Payload
370+ case <-time.After(2 * time.Second):
371+ t.Fatal("permission request was never published")
372+ }
373+
374+ // Drain the initial "request opened" notification (Granted ==
375+ // false && Denied == false) so the next read is the resolution
376+ // itself.
377+ select {
378+ case ev := <-notifications:
379+ require.False(t, ev.Payload.Granted, "initial notification must not be granted")
380+ require.False(t, ev.Payload.Denied, "initial notification must not be denied")
381+ case <-time.After(2 * time.Second):
382+ t.Fatal("initial notification was never published")
383+ }
384+
385+ // Race two grants from two goroutines.
386+ var (
387+ resolvedCount atomic.Int32
388+ start = make(chan struct{})
389+ racers sync.WaitGroup
390+ )
391+ for range 2 {
392+ racers.Go(func() {
393+ <-start
394+ if service.Grant(pending) {
395+ resolvedCount.Add(1)
396+ }
397+ })
398+ }
399+ close(start)
400+ racers.Wait()
401+
402+ // Original Request must return granted exactly once.
403+ wg.Wait()
404+ require.NoError(t, requestErr)
405+ assert.True(t, granted, "request should observe its grant")
406+
407+ // Exactly one of the two grants resolved the request.
408+ assert.Equal(t, int32(1), resolvedCount.Load(),
409+ "exactly one Grant should report it resolved the request")
410+
411+ // Exactly one resolution notification, and no further ones.
412+ select {
413+ case ev := <-notifications:
414+ assert.True(t, ev.Payload.Granted, "resolution notification should be granted")
415+ assert.Equal(t, "race-call", ev.Payload.ToolCallID)
416+ case <-time.After(2 * time.Second):
417+ t.Fatal("resolution notification was never published")
418+ }
419+ select {
420+ case ev := <-notifications:
421+ t.Fatalf("unexpected duplicate notification: %+v", ev.Payload)
422+ case <-time.After(50 * time.Millisecond):
423+ // good: no duplicate.
424+ }
425+
426+ // pendingRequests must be empty: no goroutine is left blocked
427+ // on a send, and a future Grant for the same ID is a no-op.
428+ ps := service.(*permissionService)
429+ assert.Equal(t, 0, ps.pendingRequests.Len(),
430+ "pendingRequests must be empty after resolution")
431+
432+ assert.False(t, service.Grant(pending),
433+ "a third Grant should report already-resolved")
434+ })
435+
436+ t.Run("grant after deny is a no-op", func(t *testing.T) {
437+ t.Parallel()
438+ service := NewPermissionService("/tmp", false, nil)
439+
440+ events := service.Subscribe(t.Context())
441+ notifications := service.SubscribeNotifications(t.Context())
442+
443+ req := CreatePermissionRequest{
444+ SessionID: "deny-first",
445+ ToolCallID: "df-call",
446+ ToolName: "tool",
447+ Action: "act",
448+ Path: "/tmp/df",
449+ }
450+
451+ var (
452+ wg sync.WaitGroup
453+ granted bool
454+ requestErr error
455+ )
456+ wg.Go(func() {
457+ granted, requestErr = service.Request(t.Context(), req)
458+ })
459+
460+ var pending PermissionRequest
461+ select {
462+ case ev := <-events:
463+ pending = ev.Payload
464+ case <-time.After(2 * time.Second):
465+ t.Fatal("permission request was never published")
466+ }
467+
468+ // Drain the initial neither-granted-nor-denied notification.
469+ <-notifications
470+
471+ assert.True(t, service.Deny(pending), "Deny should resolve the request")
472+ wg.Wait()
473+ require.NoError(t, requestErr)
474+ assert.False(t, granted, "request should observe denial")
475+
476+ // A follow-up Grant must be a no-op and must not flip the
477+ // outcome or publish anything new.
478+ assert.False(t, service.Grant(pending),
479+ "Grant after Deny should report already-resolved")
480+
481+ select {
482+ case ev := <-notifications:
483+ // The first resolution notification (denial) is expected;
484+ // anything after that is a bug.
485+ require.True(t, ev.Payload.Denied,
486+ "the only post-initial notification must be the denial")
487+ case <-time.After(2 * time.Second):
488+ t.Fatal("denial notification was never published")
489+ }
490+ select {
491+ case ev := <-notifications:
492+ t.Fatalf("Grant after Deny must not publish: %+v", ev.Payload)
493+ case <-time.After(50 * time.Millisecond):
494+ // good.
495+ }
496+ })
497+
498+ t.Run("losing GrantPersistent does not record session permission", func(t *testing.T) {
499+ t.Parallel()
500+ service := NewPermissionService("/tmp", false, nil)
501+
502+ events := service.Subscribe(t.Context())
503+ notifications := service.SubscribeNotifications(t.Context())
504+
505+ req := CreatePermissionRequest{
506+ SessionID: "race-persist",
507+ ToolCallID: "rp-call",
508+ ToolName: "tool",
509+ Action: "act",
510+ Path: "/tmp/rp",
511+ }
512+
513+ var (
514+ wg sync.WaitGroup
515+ granted bool
516+ requestErr error
517+ )
518+ wg.Go(func() {
519+ granted, requestErr = service.Request(t.Context(), req)
520+ })
521+
522+ // Wait for the request to be published so we have the real
523+ // pending PermissionRequest to race on.
524+ var pending PermissionRequest
525+ select {
526+ case ev := <-events:
527+ pending = ev.Payload
528+ case <-time.After(2 * time.Second):
529+ t.Fatal("permission request was never published")
530+ }
531+
532+ // Drain the initial neither-granted-nor-denied notification.
533+ <-notifications
534+
535+ // Deny wins, then a competing GrantPersistent loses.
536+ assert.True(t, service.Deny(pending), "Deny should resolve the request")
537+ assert.False(t, service.GrantPersistent(pending),
538+ "GrantPersistent after Deny should report already-resolved")
539+
540+ wg.Wait()
541+ require.NoError(t, requestErr)
542+ assert.False(t, granted, "request should observe denial")
543+
544+ // The losing GrantPersistent must not have inserted an
545+ // auto-approve entry. Issue a matching follow-up request and
546+ // confirm the service still publishes a pending request (i.e.
547+ // not auto-approved). We then Deny it to drain the goroutine.
548+ var (
549+ wg2 sync.WaitGroup
550+ granted2 bool
551+ requestErr2 error
552+ )
553+ wg2.Go(func() {
554+ granted2, requestErr2 = service.Request(t.Context(), req)
555+ })
556+
557+ select {
558+ case ev := <-events:
559+ assert.Equal(t, pending.SessionID, ev.Payload.SessionID)
560+ service.Deny(ev.Payload)
561+ case <-time.After(2 * time.Second):
562+ t.Fatal("follow-up request was auto-approved; persistent grant leaked")
563+ }
564+
565+ wg2.Wait()
566+ require.NoError(t, requestErr2)
567+ assert.False(t, granted2, "follow-up request should be denied, not auto-approved")
568+ })
569+
570+ t.Run("grant for unknown id is a safe no-op", func(t *testing.T) {
571+ t.Parallel()
572+ service := NewPermissionService("/tmp", false, nil)
573+
574+ notifications := service.SubscribeNotifications(t.Context())
575+
576+ bogus := PermissionRequest{
577+ ID: "does-not-exist",
578+ ToolCallID: "ghost",
579+ ToolName: "tool",
580+ Action: "act",
581+ Path: "/tmp/ghost",
582+ }
583+
584+ assert.NotPanics(t, func() {
585+ assert.False(t, service.Grant(bogus),
586+ "Grant for unknown ID should report already-resolved")
587+ assert.False(t, service.GrantPersistent(bogus),
588+ "GrantPersistent for unknown ID should report already-resolved")
589+ assert.False(t, service.Deny(bogus),
590+ "Deny for unknown ID should report already-resolved")
591+ })
592+
593+ select {
594+ case ev := <-notifications:
595+ t.Fatalf("unknown-ID resolution must not publish: %+v", ev.Payload)
596+ case <-time.After(50 * time.Millisecond):
597+ // good: no notification.
598+ }
599+ })
600+}