Detailed changes
@@ -93,7 +93,10 @@ func (c *coordinator) agenticFetchTool(_ context.Context, client *http.Client) (
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -228,6 +231,6 @@ func (c *coordinator) agenticFetchTool(_ context.Context, client *http.Client) (
return fantasy.ToolResponse{}, fmt.Errorf("error saving parent session: %s", err)
}
- return fantasy.NewTextResponse(result.Response.Content.Text()), nil
+ return fantasy.NewTextResponse(p.AppendCommentary(result.Response.Content.Text())), nil
}), nil
}
@@ -214,8 +214,10 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
if sessionID == "" {
return fantasy.ToolResponse{}, fmt.Errorf("session ID is required for executing shell command")
}
+ var permResult permission.PermissionResult
if !isSafeReadOnly {
- p, err := permissions.Request(ctx,
+ var err error
+ permResult, err = permissions.Request(ctx,
permission.CreatePermissionRequest{
SessionID: sessionID,
Path: execWorkingDir,
@@ -229,7 +231,10 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !permResult.Granted {
+ if permResult.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(permResult.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
}
@@ -270,10 +275,10 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
WorkingDirectory: bgShell.WorkingDir,
}
if stdout == "" {
- return fantasy.WithResponseMetadata(fantasy.NewTextResponse(BashNoOutput), metadata), nil
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(permResult.AppendCommentary(BashNoOutput)), metadata), nil
}
stdout += fmt.Sprintf("\n\n<cwd>%s</cwd>", normalizeWorkingDir(bgShell.WorkingDir))
- return fantasy.WithResponseMetadata(fantasy.NewTextResponse(stdout), metadata), nil
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(permResult.AppendCommentary(stdout)), metadata), nil
}
// Still running after fast-failure check - return as background job
@@ -286,7 +291,7 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
ShellID: bgShell.ID,
}
response := fmt.Sprintf("Background shell started with ID: %s\n\nUse job_output tool to view output or job_kill to terminate.", bgShell.ID)
- return fantasy.WithResponseMetadata(fantasy.NewTextResponse(response), metadata), nil
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(permResult.AppendCommentary(response)), metadata), nil
}
// Start synchronous execution with auto-background support
@@ -351,10 +356,10 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
WorkingDirectory: bgShell.WorkingDir,
}
if stdout == "" {
- return fantasy.WithResponseMetadata(fantasy.NewTextResponse(BashNoOutput), metadata), nil
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(permResult.AppendCommentary(BashNoOutput)), metadata), nil
}
stdout += fmt.Sprintf("\n\n<cwd>%s</cwd>", normalizeWorkingDir(bgShell.WorkingDir))
- return fantasy.WithResponseMetadata(fantasy.NewTextResponse(stdout), metadata), nil
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(permResult.AppendCommentary(stdout)), metadata), nil
}
// Still running - keep as background job
@@ -367,7 +372,7 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
ShellID: bgShell.ID,
}
response := fmt.Sprintf("Command is taking longer than expected and has been moved to background.\n\nBackground shell ID: %s\n\nUse job_output tool to view output or job_kill to terminate.", bgShell.ID)
- return fantasy.WithResponseMetadata(fantasy.NewTextResponse(response), metadata), nil
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(permResult.AppendCommentary(response)), metadata), nil
})
}
@@ -83,7 +83,10 @@ func NewDownloadTool(permissions permission.Service, workingDir string, client *
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -142,6 +145,6 @@ func NewDownloadTool(permissions permission.Service, workingDir string, client *
responseMsg += fmt.Sprintf(" (Content-Type: %s)", contentType)
}
- return fantasy.NewTextResponse(responseMsg), nil
+ return fantasy.NewTextResponse(p.AppendCommentary(responseMsg)), nil
})
}
@@ -145,7 +145,10 @@ func createNewFile(edit editContext, filePath, content string, call fantasy.Tool
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -172,7 +175,7 @@ func createNewFile(edit editContext, filePath, content string, call fantasy.Tool
filetracker.RecordRead(filePath)
return fantasy.WithResponseMetadata(
- fantasy.NewTextResponse("File created: "+filePath),
+ fantasy.NewTextResponse(p.AppendCommentary("File created: "+filePath)),
EditResponseMetadata{
OldContent: "",
NewContent: content,
@@ -266,7 +269,10 @@ func deleteContent(edit editContext, filePath, oldString string, replaceAll bool
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -305,7 +311,7 @@ func deleteContent(edit editContext, filePath, oldString string, replaceAll bool
filetracker.RecordRead(filePath)
return fantasy.WithResponseMetadata(
- fantasy.NewTextResponse("Content deleted from file: "+filePath),
+ fantasy.NewTextResponse(p.AppendCommentary("Content deleted from file: "+filePath)),
EditResponseMetadata{
OldContent: oldContent,
NewContent: newContent,
@@ -398,7 +404,10 @@ func replaceContent(edit editContext, filePath, oldString, newString string, rep
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -437,7 +446,7 @@ func replaceContent(edit editContext, filePath, oldString, newString string, rep
filetracker.RecordRead(filePath)
return fantasy.WithResponseMetadata(
- fantasy.NewTextResponse("Content replaced in file: "+filePath),
+ fantasy.NewTextResponse(p.AppendCommentary("Content replaced in file: "+filePath)),
EditResponseMetadata{
OldContent: oldContent,
NewContent: newContent,
@@ -69,7 +69,10 @@ func NewFetchTool(permissions permission.Service, workingDir string, client *htt
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -165,7 +168,7 @@ func NewFetchTool(permissions permission.Service, workingDir string, client *htt
content += fmt.Sprintf("\n\n[Content truncated to %d bytes]", MaxReadSize)
}
- return fantasy.NewTextResponse(content), nil
+ return fantasy.NewTextResponse(p.AppendCommentary(content)), nil
})
}
@@ -78,6 +78,7 @@ func NewLsTool(permissions permission.Service, workingDir string, lsConfig confi
return fantasy.NewTextErrorResponse(fmt.Sprintf("error resolving search path: %v", err)), nil
}
+ var permResult permission.PermissionResult
relPath, err := filepath.Rel(absWorkingDir, absSearchPath)
if err != nil || strings.HasPrefix(relPath, "..") {
// Directory is outside working directory, request permission
@@ -86,7 +87,7 @@ func NewLsTool(permissions permission.Service, workingDir string, lsConfig confi
return fantasy.ToolResponse{}, fmt.Errorf("session ID is required for accessing directories outside working directory")
}
- granted, err := permissions.Request(ctx,
+ permResult, err = permissions.Request(ctx,
permission.CreatePermissionRequest{
SessionID: sessionID,
Path: absSearchPath,
@@ -100,7 +101,10 @@ func NewLsTool(permissions permission.Service, workingDir string, lsConfig confi
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !granted {
+ if !permResult.Granted {
+ if permResult.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(permResult.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
}
@@ -111,7 +115,7 @@ func NewLsTool(permissions permission.Service, workingDir string, lsConfig confi
}
return fantasy.WithResponseMetadata(
- fantasy.NewTextResponse(output),
+ fantasy.NewTextResponse(permResult.AppendCommentary(output)),
metadata,
), nil
})
@@ -103,7 +103,10 @@ func (m *Tool) Run(ctx context.Context, params fantasy.ToolCall) (fantasy.ToolRe
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -125,9 +128,9 @@ func (m *Tool) Run(ctx context.Context, params fantasy.ToolCall) (fantasy.ToolRe
} else {
response = fantasy.NewMediaResponse(result.Data, result.MediaType)
}
- response.Content = result.Content
+ response.Content = p.AppendCommentary(result.Content)
return response, nil
default:
- return fantasy.NewTextResponse(result.Content), nil
+ return fantasy.NewTextResponse(p.AppendCommentary(result.Content)), nil
}
}
@@ -189,7 +189,10 @@ func processMultiEditWithCreation(edit editContext, params MultiEditParams, call
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -221,7 +224,7 @@ func processMultiEditWithCreation(edit editContext, params MultiEditParams, call
}
return fantasy.WithResponseMetadata(
- fantasy.NewTextResponse(message),
+ fantasy.NewTextResponse(p.AppendCommentary(message)),
MultiEditResponseMetadata{
OldContent: "",
NewContent: currentContent,
@@ -333,7 +336,10 @@ func processMultiEditExistingFile(edit editContext, params MultiEditParams, call
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -380,7 +386,7 @@ func processMultiEditExistingFile(edit editContext, params MultiEditParams, call
}
return fantasy.WithResponseMetadata(
- fantasy.NewTextResponse(message),
+ fantasy.NewTextResponse(p.AppendCommentary(message)),
MultiEditResponseMetadata{
OldContent: oldContent,
NewContent: currentContent,
@@ -19,15 +19,15 @@ type mockPermissionService struct {
*pubsub.Broker[permission.PermissionRequest]
}
-func (m *mockPermissionService) Request(ctx context.Context, req permission.CreatePermissionRequest) (bool, error) {
- return true, nil
+func (m *mockPermissionService) Request(ctx context.Context, req permission.CreatePermissionRequest) (permission.PermissionResult, error) {
+ return permission.PermissionResult{Granted: true}, nil
}
-func (m *mockPermissionService) Grant(req permission.PermissionRequest) {}
+func (m *mockPermissionService) Grant(req permission.PermissionRequest, message string) {}
-func (m *mockPermissionService) Deny(req permission.PermissionRequest) {}
+func (m *mockPermissionService) Deny(req permission.PermissionRequest, message string) {}
-func (m *mockPermissionService) GrantPersistent(req permission.PermissionRequest) {}
+func (m *mockPermissionService) GrantPersistent(req permission.PermissionRequest, message string) {}
func (m *mockPermissionService) AutoApproveSession(sessionID string) {}
@@ -74,6 +74,7 @@ func NewViewTool(lspClients *csync.Map[string, *lsp.Client], permissions permiss
isOutsideWorkDir := err != nil || strings.HasPrefix(relPath, "..")
isSkillFile := isInSkillsPath(absFilePath, skillsPaths)
+ var permResult permission.PermissionResult
// Request permission for files outside working directory, unless it's a skill file.
if isOutsideWorkDir && !isSkillFile {
sessionID := GetSessionFromContext(ctx)
@@ -81,7 +82,7 @@ func NewViewTool(lspClients *csync.Map[string, *lsp.Client], permissions permiss
return fantasy.ToolResponse{}, fmt.Errorf("session ID is required for accessing files outside working directory")
}
- granted, err := permissions.Request(ctx,
+ permResult, err = permissions.Request(ctx,
permission.CreatePermissionRequest{
SessionID: sessionID,
Path: absFilePath,
@@ -95,7 +96,10 @@ func NewViewTool(lspClients *csync.Map[string, *lsp.Client], permissions permiss
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !granted {
+ if !permResult.Granted {
+ if permResult.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(permResult.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
}
@@ -192,7 +196,7 @@ func NewViewTool(lspClients *csync.Map[string, *lsp.Client], permissions permiss
output += getDiagnostics(filePath, lspClients)
filetracker.RecordRead(filePath)
return fantasy.WithResponseMetadata(
- fantasy.NewTextResponse(output),
+ fantasy.NewTextResponse(permResult.AppendCommentary(output)),
ViewResponseMetadata{
FilePath: filePath,
Content: content,
@@ -122,7 +122,10 @@ func NewWriteTool(lspClients *csync.Map[string, *lsp.Client], permissions permis
if err != nil {
return fantasy.ToolResponse{}, err
}
- if !p {
+ if !p.Granted {
+ if p.Message != "" {
+ return fantasy.NewTextErrorResponse("User denied permission." + permission.UserCommentaryTag(p.Message)), nil
+ }
return fantasy.ToolResponse{}, permission.ErrorPermissionDenied
}
@@ -161,7 +164,7 @@ func NewWriteTool(lspClients *csync.Map[string, *lsp.Client], permissions permis
result := fmt.Sprintf("File successfully written: %s", filePath)
result = fmt.Sprintf("<result>\n%s\n</result>", result)
result += getDiagnostics(filePath, lspClients)
- return fantasy.WithResponseMetadata(fantasy.NewTextResponse(result),
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(p.AppendCommentary(result)),
WriteResponseMetadata{
Diff: diff,
Additions: additions,
@@ -15,6 +15,14 @@ import (
var ErrorPermissionDenied = errors.New("user denied permission")
+// UserCommentaryTag formats user commentary for the LLM.
+func UserCommentaryTag(message string) string {
+ if message == "" {
+ return ""
+ }
+ return "\n\nUser feedback: " + message
+}
+
type CreatePermissionRequest struct {
SessionID string `json:"session_id"`
ToolCallID string `json:"tool_call_id"`
@@ -29,6 +37,7 @@ type PermissionNotification struct {
ToolCallID string `json:"tool_call_id"`
Granted bool `json:"granted"`
Denied bool `json:"denied"`
+ Message string `json:"message,omitempty"` // User commentary or instructions.
}
type PermissionRequest struct {
@@ -44,16 +53,30 @@ type PermissionRequest struct {
type Service interface {
pubsub.Subscriber[PermissionRequest]
- GrantPersistent(permission PermissionRequest)
- Grant(permission PermissionRequest)
- Deny(permission PermissionRequest)
- Request(ctx context.Context, opts CreatePermissionRequest) (bool, error)
+ GrantPersistent(permission PermissionRequest, message string)
+ Grant(permission PermissionRequest, message string)
+ Deny(permission PermissionRequest, message string)
+ Request(ctx context.Context, opts CreatePermissionRequest) (PermissionResult, error)
AutoApproveSession(sessionID string)
SetSkipRequests(skip bool)
SkipRequests() bool
SubscribeNotifications(ctx context.Context) <-chan pubsub.Event[PermissionNotification]
}
+// PermissionResult contains the result of a permission request.
+type PermissionResult struct {
+ Granted bool
+ Message string // User's commentary or instructions.
+}
+
+// AppendCommentary appends user commentary to content if present.
+func (r PermissionResult) AppendCommentary(content string) string {
+ if r.Message == "" {
+ return content
+ }
+ return content + UserCommentaryTag(r.Message)
+}
+
type permissionService struct {
*pubsub.Broker[PermissionRequest]
@@ -61,7 +84,7 @@ type permissionService struct {
workingDir string
sessionPermissions []PermissionRequest
sessionPermissionsMu sync.RWMutex
- pendingRequests *csync.Map[string, chan bool]
+ pendingRequests *csync.Map[string, chan PermissionResult]
autoApproveSessions map[string]bool
autoApproveSessionsMu sync.RWMutex
skip bool
@@ -73,14 +96,15 @@ type permissionService struct {
activeRequestMu sync.Mutex
}
-func (s *permissionService) GrantPersistent(permission PermissionRequest) {
+func (s *permissionService) GrantPersistent(permission PermissionRequest, message string) {
s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
ToolCallID: permission.ToolCallID,
Granted: true,
+ Message: message,
})
respCh, ok := s.pendingRequests.Get(permission.ID)
if ok {
- respCh <- true
+ respCh <- PermissionResult{Granted: true, Message: message}
}
s.sessionPermissionsMu.Lock()
@@ -94,14 +118,15 @@ func (s *permissionService) GrantPersistent(permission PermissionRequest) {
s.activeRequestMu.Unlock()
}
-func (s *permissionService) Grant(permission PermissionRequest) {
+func (s *permissionService) Grant(permission PermissionRequest, message string) {
s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
ToolCallID: permission.ToolCallID,
Granted: true,
+ Message: message,
})
respCh, ok := s.pendingRequests.Get(permission.ID)
if ok {
- respCh <- true
+ respCh <- PermissionResult{Granted: true, Message: message}
}
s.activeRequestMu.Lock()
@@ -111,15 +136,16 @@ func (s *permissionService) Grant(permission PermissionRequest) {
s.activeRequestMu.Unlock()
}
-func (s *permissionService) Deny(permission PermissionRequest) {
+func (s *permissionService) Deny(permission PermissionRequest, message string) {
s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
ToolCallID: permission.ToolCallID,
Granted: false,
Denied: true,
+ Message: message,
})
respCh, ok := s.pendingRequests.Get(permission.ID)
if ok {
- respCh <- false
+ respCh <- PermissionResult{Granted: false, Message: message}
}
s.activeRequestMu.Lock()
@@ -129,9 +155,9 @@ func (s *permissionService) Deny(permission PermissionRequest) {
s.activeRequestMu.Unlock()
}
-func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRequest) (bool, error) {
+func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRequest) (PermissionResult, error) {
if s.skip {
- return true, nil
+ return PermissionResult{Granted: true}, nil
}
// tell the UI that a permission was requested
@@ -144,7 +170,7 @@ func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRe
// Check if the tool/action combination is in the allowlist
commandKey := opts.ToolName + ":" + opts.Action
if slices.Contains(s.allowedTools, commandKey) || slices.Contains(s.allowedTools, opts.ToolName) {
- return true, nil
+ return PermissionResult{Granted: true}, nil
}
s.autoApproveSessionsMu.RLock()
@@ -152,7 +178,7 @@ func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRe
s.autoApproveSessionsMu.RUnlock()
if autoApprove {
- return true, nil
+ return PermissionResult{Granted: true}, nil
}
fileInfo, err := os.Stat(opts.Path)
@@ -183,7 +209,7 @@ func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRe
for _, p := range s.sessionPermissions {
if p.ToolName == permission.ToolName && p.Action == permission.Action && p.SessionID == permission.SessionID && p.Path == permission.Path {
s.sessionPermissionsMu.RUnlock()
- return true, nil
+ return PermissionResult{Granted: true}, nil
}
}
s.sessionPermissionsMu.RUnlock()
@@ -192,7 +218,7 @@ func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRe
s.activeRequest = &permission
s.activeRequestMu.Unlock()
- respCh := make(chan bool, 1)
+ respCh := make(chan PermissionResult, 1)
s.pendingRequests.Set(permission.ID, respCh)
defer s.pendingRequests.Del(permission.ID)
@@ -201,9 +227,9 @@ func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRe
select {
case <-ctx.Done():
- return false, ctx.Err()
- case granted := <-respCh:
- return granted, nil
+ return PermissionResult{Granted: false}, ctx.Err()
+ case result := <-respCh:
+ return result, nil
}
}
@@ -234,6 +260,6 @@ func NewPermissionService(workingDir string, skip bool, allowedTools []string) S
autoApproveSessions: make(map[string]bool),
skip: skip,
allowedTools: allowedTools,
- pendingRequests: csync.NewMap[string, chan bool](),
+ pendingRequests: csync.NewMap[string, chan PermissionResult](),
}
}
@@ -92,7 +92,7 @@ func TestPermissionService_SkipMode(t *testing.T) {
if err != nil {
t.Errorf("unexpected error: %v", err)
}
- if !result {
+ if !result.Granted {
t.Error("expected permission to be granted in skip mode")
}
}
@@ -110,7 +110,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
Path: "/tmp/test.txt",
}
- var result1 bool
+ var result1 PermissionResult
var wg sync.WaitGroup
wg.Add(1)
@@ -125,10 +125,10 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
event := <-events
permissionReq = event.Payload
- service.GrantPersistent(permissionReq)
+ service.GrantPersistent(permissionReq, "")
wg.Wait()
- assert.True(t, result1, "First request should be granted")
+ assert.True(t, result1.Granted, "First request should be granted")
// Second identical request should be automatically approved due to persistent permission
req2 := CreatePermissionRequest{
@@ -141,7 +141,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
}
result2, err := service.Request(t.Context(), req2)
require.NoError(t, err)
- assert.True(t, result2, "Second request should be auto-approved")
+ assert.True(t, result2.Granted, "Second request should be auto-approved")
})
t.Run("Sequential requests with temporary grants", func(t *testing.T) {
service := NewPermissionService("/tmp", false, []string{})
@@ -156,7 +156,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
}
events := service.Subscribe(t.Context())
- var result1 bool
+ var result1 PermissionResult
var wg sync.WaitGroup
wg.Go(func() {
@@ -167,11 +167,11 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
event := <-events
permissionReq = event.Payload
- service.Grant(permissionReq)
+ service.Grant(permissionReq, "")
wg.Wait()
- assert.True(t, result1, "First request should be granted")
+ assert.True(t, result1.Granted, "First request should be granted")
- var result2 bool
+ var result2 PermissionResult
wg.Go(func() {
result2, _ = service.Request(t.Context(), req)
@@ -179,9 +179,9 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
event = <-events
permissionReq = event.Payload
- service.Deny(permissionReq)
+ service.Deny(permissionReq, "")
wg.Wait()
- assert.False(t, result2, "Second request should be denied")
+ assert.False(t, result2.Granted, "Second request should be denied")
})
t.Run("Concurrent requests with different outcomes", func(t *testing.T) {
service := NewPermissionService("/tmp", false, []string{})
@@ -189,7 +189,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
events := service.Subscribe(t.Context())
var wg sync.WaitGroup
- results := make([]bool, 3)
+ results := make([]PermissionResult, 3)
requests := []CreatePermissionRequest{
{
@@ -228,17 +228,17 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
event := <-events
switch event.Payload.ToolName {
case "tool1":
- service.Grant(event.Payload)
+ service.Grant(event.Payload, "")
case "tool2":
- service.GrantPersistent(event.Payload)
+ service.GrantPersistent(event.Payload, "")
case "tool3":
- service.Deny(event.Payload)
+ service.Deny(event.Payload, "")
}
}
wg.Wait()
grantedCount := 0
for _, result := range results {
- if result {
+ if result.Granted {
grantedCount++
}
}
@@ -248,6 +248,6 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
secondReq.Description = "Repeat of second request"
result, err := service.Request(t.Context(), secondReq)
require.NoError(t, err)
- assert.True(t, result, "Repeated request should be auto-approved due to persistent permission")
+ assert.True(t, result.Granted, "Repeated request should be auto-approved due to persistent permission")
})
}
@@ -325,11 +325,11 @@ func (a *appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
case permissions.PermissionResponseMsg:
switch msg.Action {
case permissions.PermissionAllow:
- a.app.Permissions.Grant(msg.Permission)
+ a.app.Permissions.Grant(msg.Permission, "")
case permissions.PermissionAllowForSession:
- a.app.Permissions.GrantPersistent(msg.Permission)
+ a.app.Permissions.GrantPersistent(msg.Permission, "")
case permissions.PermissionDeny:
- a.app.Permissions.Deny(msg.Permission)
+ a.app.Permissions.Deny(msg.Permission, "")
}
return a, nil
case splash.OnboardingCompleteMsg:
@@ -61,6 +61,7 @@ type (
ActionPermissionResponse struct {
Permission permission.PermissionRequest
Action PermissionAction
+ Commentary string // User's commentary or instructions for the action.
}
// ActionRunCustomCommand is a message to run a custom command.
ActionRunCustomCommand struct {
@@ -7,6 +7,7 @@ import (
"charm.land/bubbles/v2/help"
"charm.land/bubbles/v2/key"
+ "charm.land/bubbles/v2/textinput"
"charm.land/bubbles/v2/viewport"
tea "charm.land/bubbletea/v2"
"charm.land/lipgloss/v2"
@@ -73,6 +74,10 @@ type Permissions struct {
unifiedDiffContent string
splitDiffContent string
+ // Commentary input for user feedback.
+ input textinput.Model
+ inputFocused bool
+
help help.Model
keyMap permissionsKeyMap
}
@@ -85,6 +90,9 @@ type permissionsKeyMap struct {
Allow key.Binding
AllowSession key.Binding
Deny key.Binding
+ CtrlAllow key.Binding
+ CtrlAllowSession key.Binding
+ CtrlDeny key.Binding
Close key.Binding
ToggleDiffMode key.Binding
ToggleFullscreen key.Binding
@@ -94,6 +102,7 @@ type permissionsKeyMap struct {
ScrollRight key.Binding
Choose key.Binding
Scroll key.Binding
+ FocusInput key.Binding
}
func defaultPermissionsKeyMap() permissionsKeyMap {
@@ -115,17 +124,29 @@ func defaultPermissionsKeyMap() permissionsKeyMap {
key.WithHelp("enter", "confirm"),
),
Allow: key.NewBinding(
- key.WithKeys("a", "A", "ctrl+a"),
+ key.WithKeys("a"),
key.WithHelp("a", "allow"),
),
AllowSession: key.NewBinding(
- key.WithKeys("s", "S", "ctrl+s"),
+ key.WithKeys("s"),
key.WithHelp("s", "allow session"),
),
Deny: key.NewBinding(
- key.WithKeys("d", "D"),
+ key.WithKeys("d"),
key.WithHelp("d", "deny"),
),
+ CtrlAllow: key.NewBinding(
+ key.WithKeys("ctrl+a"),
+ key.WithHelp("ctrl+a", "allow"),
+ ),
+ CtrlAllowSession: key.NewBinding(
+ key.WithKeys("ctrl+s"),
+ key.WithHelp("ctrl+s", "session"),
+ ),
+ CtrlDeny: key.NewBinding(
+ key.WithKeys("ctrl+d"),
+ key.WithHelp("ctrl+d", "deny"),
+ ),
Close: CloseKey,
ToggleDiffMode: key.NewBinding(
key.WithKeys("t"),
@@ -159,6 +180,10 @@ func defaultPermissionsKeyMap() permissionsKeyMap {
key.WithKeys("shift+left", "shift+down", "shift+up", "shift+right"),
key.WithHelp("shift+←↓↑→", "scroll"),
),
+ FocusInput: key.NewBinding(
+ key.WithKeys("/"),
+ key.WithHelp("/", "add comment"),
+ ),
}
}
@@ -195,11 +220,18 @@ func NewPermissions(com *common.Common, perm permission.PermissionRequest, opts
HalfPageDown: key.NewBinding(key.WithDisabled()),
}
+ // Configure text input for user commentary.
+ input := textinput.New()
+ input.SetVirtualCursor(false)
+ input.Placeholder = "Feedback for the agent (optional)..."
+ input.SetStyles(com.Styles.TextInput)
+
p := &Permissions{
com: com,
permission: perm,
selectedOption: 0,
viewport: vp,
+ input: input,
help: h,
keyMap: km,
}
@@ -227,10 +259,42 @@ func (*Permissions) ID() string {
func (p *Permissions) HandleMsg(msg tea.Msg) Action {
switch msg := msg.(type) {
case tea.KeyPressMsg:
+ // When input is focused, handle navigation and shortcuts first.
+ if p.inputFocused {
+ switch {
+ case key.Matches(msg, p.keyMap.Close):
+ // Escape unfocuses the input.
+ p.inputFocused = false
+ p.input.Blur()
+ return nil
+ case key.Matches(msg, p.keyMap.Select):
+ // Enter confirms the current selection with the comment.
+ return p.selectCurrentOption()
+ case key.Matches(msg, p.keyMap.Tab):
+ p.selectedOption = (p.selectedOption + 1) % 3
+ return nil
+ case key.Matches(msg, p.keyMap.CtrlAllow):
+ return p.respond(PermissionAllow)
+ case key.Matches(msg, p.keyMap.CtrlAllowSession):
+ return p.respond(PermissionAllowForSession)
+ case key.Matches(msg, p.keyMap.CtrlDeny):
+ return p.respond(PermissionDeny)
+ default:
+ // Pass other keys to the text input.
+ p.input, _ = p.input.Update(msg)
+ return nil
+ }
+ }
+
+ // Normal dialog navigation when input is not focused.
switch {
case key.Matches(msg, p.keyMap.Close):
// Escape denies the permission request.
return p.respond(PermissionDeny)
+ case key.Matches(msg, p.keyMap.FocusInput):
+ p.inputFocused = true
+ p.input.Focus()
+ return nil
case key.Matches(msg, p.keyMap.Right), key.Matches(msg, p.keyMap.Tab):
p.selectedOption = (p.selectedOption + 1) % 3
case key.Matches(msg, p.keyMap.Left):
@@ -238,11 +302,11 @@ func (p *Permissions) HandleMsg(msg tea.Msg) Action {
p.selectedOption = (p.selectedOption + 2) % 3
case key.Matches(msg, p.keyMap.Select):
return p.selectCurrentOption()
- case key.Matches(msg, p.keyMap.Allow):
+ case key.Matches(msg, p.keyMap.Allow), key.Matches(msg, p.keyMap.CtrlAllow):
return p.respond(PermissionAllow)
- case key.Matches(msg, p.keyMap.AllowSession):
+ case key.Matches(msg, p.keyMap.AllowSession), key.Matches(msg, p.keyMap.CtrlAllowSession):
return p.respond(PermissionAllowForSession)
- case key.Matches(msg, p.keyMap.Deny):
+ case key.Matches(msg, p.keyMap.Deny), key.Matches(msg, p.keyMap.CtrlDeny):
return p.respond(PermissionDeny)
case key.Matches(msg, p.keyMap.ToggleDiffMode):
if p.hasDiffView() {
@@ -291,6 +355,7 @@ func (p *Permissions) respond(action PermissionAction) tea.Msg {
return ActionPermissionResponse{
Permission: p.permission,
Action: action,
+ Commentary: p.input.Value(),
}
}
@@ -336,13 +401,20 @@ func (p *Permissions) Draw(scr uv.Screen, area uv.Rectangle) *tea.Cursor {
contentWidth := p.calculateContentWidth(width)
header := p.renderHeader(contentWidth)
buttons := p.renderButtons(contentWidth)
+
+ // Render the input field.
+ p.input.SetWidth(contentWidth - t.Dialog.InputPrompt.GetHorizontalFrameSize() - 1)
+ inputView := t.Dialog.InputPrompt.Render(p.input.View())
+
helpView := p.help.View(p)
// Calculate available height for content.
headerHeight := lipgloss.Height(header)
buttonsHeight := lipgloss.Height(buttons)
+ inputHeight := lipgloss.Height(inputView)
helpHeight := lipgloss.Height(helpView)
- frameHeight := dialogStyle.GetVerticalFrameSize() + layoutSpacingLines
+ // Add extra spacing lines for input section.
+ frameHeight := dialogStyle.GetVerticalFrameSize() + layoutSpacingLines + 2
p.defaultDiffSplitMode = width >= splitModeMinWidth
@@ -361,7 +433,7 @@ func (p *Permissions) Draw(scr uv.Screen, area uv.Rectangle) *tea.Cursor {
availableHeight = maxHeight - fixedHeight
}
} else {
- availableHeight = maxHeight - headerHeight - buttonsHeight - helpHeight - frameHeight
+ availableHeight = maxHeight - headerHeight - buttonsHeight - inputHeight - helpHeight - frameHeight
}
// Determine if scrollbar is needed.
@@ -379,6 +451,7 @@ func (p *Permissions) Draw(scr uv.Screen, area uv.Rectangle) *tea.Cursor {
var content string
var scrollbar string
+ availableHeight = min(availableHeight, lipgloss.Height(renderedContent))
p.viewport.SetWidth(viewportWidth)
p.viewport.SetHeight(availableHeight)
if p.viewportDirty {
@@ -387,6 +460,7 @@ func (p *Permissions) Draw(scr uv.Screen, area uv.Rectangle) *tea.Cursor {
p.viewportDirty = false
}
content = p.viewport.View()
+
if needsScrollbar {
scrollbar = common.Scrollbar(t, availableHeight, p.viewport.TotalLineCount(), availableHeight, p.viewport.YOffset())
}
@@ -400,11 +474,28 @@ func (p *Permissions) Draw(scr uv.Screen, area uv.Rectangle) *tea.Cursor {
if content != "" {
parts = append(parts, "", content)
}
- parts = append(parts, "", buttons, "", helpView)
+ parts = append(parts, "", inputView, "", buttons, "", helpView)
innerContent := lipgloss.JoinVertical(lipgloss.Left, parts...)
- DrawCenterCursor(scr, area, dialogStyle.Render(innerContent), nil)
- return nil
+
+ var cur *tea.Cursor
+ if p.inputFocused {
+ cur = p.input.Cursor()
+ if cur != nil {
+ // Calculate Y offset: header + empty line + content + empty line.
+ yOffset := headerHeight
+ if content != "" {
+ yOffset += 1 + lipgloss.Height(content)
+ }
+ yOffset += 1 // Empty line before input.
+
+ // Add dialog frame offsets.
+ cur.X += dialogStyle.GetHorizontalFrameSize()/2 + 1
+ cur.Y += dialogStyle.GetVerticalFrameSize()/2 + yOffset + 1
+ }
+ }
+ DrawCenterCursor(scr, area, dialogStyle.Render(innerContent), cur)
+ return cur
}
func (p *Permissions) renderHeader(contentWidth int) string {
@@ -734,9 +825,22 @@ func (p *Permissions) canScroll() bool {
// ShortHelp implements [help.KeyMap].
func (p *Permissions) ShortHelp() []key.Binding {
+ // When input is focused, show different help.
+ if p.inputFocused {
+ return []key.Binding{
+ p.keyMap.Tab,
+ p.keyMap.Select,
+ p.keyMap.CtrlAllow,
+ p.keyMap.CtrlAllowSession,
+ p.keyMap.CtrlDeny,
+ p.keyMap.Close,
+ }
+ }
+
bindings := []key.Binding{
p.keyMap.Choose,
p.keyMap.Select,
+ p.keyMap.FocusInput,
p.keyMap.Close,
}
@@ -1190,11 +1190,11 @@ func (m *UI) handleDialogMsg(msg tea.Msg) tea.Cmd {
m.dialog.CloseDialog(dialog.PermissionsID)
switch msg.Action {
case dialog.PermissionAllow:
- m.com.App.Permissions.Grant(msg.Permission)
+ m.com.App.Permissions.Grant(msg.Permission, msg.Commentary)
case dialog.PermissionAllowForSession:
- m.com.App.Permissions.GrantPersistent(msg.Permission)
+ m.com.App.Permissions.GrantPersistent(msg.Permission, msg.Commentary)
case dialog.PermissionDeny:
- m.com.App.Permissions.Deny(msg.Permission)
+ m.com.App.Permissions.Deny(msg.Permission, msg.Commentary)
}
case dialog.ActionFilePickerSelected: