git.iko.soy/crush
on main help sign in
perf(permission): release requestMu before the human wait
d88bfb9iko
internal/event/event.go+15−2
compare with main
unifiedsplit
@@ -7,6 +7,7 @@ import (
7 "path/filepath"
7 "path/filepath"
8 "reflect"
8 "reflect"
9 "runtime"
9 "runtime"
10+ "sync"
10 "time"
11 "time"
11
12
12 "github.com/charmbracelet/crush/internal/version"
13 "github.com/charmbracelet/crush/internal/version"
@@ -25,7 +26,10 @@ const (
25var (
26var (
26 client posthog.Client
27 client posthog.Client
27
28
29+ // basePropsMu guards baseProps: the Set* mutators reassign it during
30+ // startup while send() reads it concurrently from event goroutines.
31+ basePropsMu sync.RWMutex
28− baseProps = posthog.NewProperties().
32+ baseProps = posthog.NewProperties().
29 Set("GOOS", runtime.GOOS).
33 Set("GOOS", runtime.GOOS).
30 Set("GOARCH", runtime.GOARCH).
34 Set("GOARCH", runtime.GOARCH).
31 Set("TERM", os.Getenv("TERM")).
35 Set("TERM", os.Getenv("TERM")).
@@ -36,14 +40,20 @@ var (
36)
40)
37
41
38func SetNonInteractive(nonInteractive bool) {
42func SetNonInteractive(nonInteractive bool) {
43+ basePropsMu.Lock()
44+ defer basePropsMu.Unlock()
39 baseProps = baseProps.Set(nonInteractiveAttrName, nonInteractive)
45 baseProps = baseProps.Set(nonInteractiveAttrName, nonInteractive)
40}
46}
41
47
42func SetContinueBySessionID(continueBySessionID bool) {
48func SetContinueBySessionID(continueBySessionID bool) {
49+ basePropsMu.Lock()
50+ defer basePropsMu.Unlock()
43 baseProps = baseProps.Set(continueSessionByIDAttrName, continueBySessionID)
51 baseProps = baseProps.Set(continueSessionByIDAttrName, continueBySessionID)
44}
52}
45
53
46func SetContinueLastSession(continueLastSession bool) {
54func SetContinueLastSession(continueLastSession bool) {
55+ basePropsMu.Lock()
56+ defer basePropsMu.Unlock()
47 baseProps = baseProps.Set(continueLastSessionAttrName, continueLastSession)
57 baseProps = baseProps.Set(continueLastSessionAttrName, continueLastSession)
48}
58}
49
59
@@ -81,10 +91,13 @@ func send(event string, props ...any) {
81 if client == nil {
91 if client == nil {
82 return
92 return
83 }
93 }
94+ basePropsMu.RLock()
95+ properties := pairsToProps(props...).Merge(baseProps)
96+ basePropsMu.RUnlock()
84 err := client.Enqueue(posthog.Capture{
97 err := client.Enqueue(posthog.Capture{
85 DistinctId: distinctId,
98 DistinctId: distinctId,
86 Event: event,
99 Event: event,
87− Properties: pairsToProps(props...).Merge(baseProps),
100+ Properties: properties,
88 })
101 })
89 if err != nil {
102 if err != nil {
90 slog.Error("Failed to enqueue PostHog event", "event", event, "props", props, "error", err)
103 slog.Error("Failed to enqueue PostHog event", "event", event, "props", props, "error", err)
internal/permission/permission.go+70−57
@@ -201,73 +201,86 @@ func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRe
201 return true, nil
201 return true, nil
202 }
202 }
203
203
204+ // Set up the request under requestMu, then release it *before*
205+ // blocking on the human response. Holding requestMu across the wait
206+ // would serialize every permission prompt process-wide, so parallel
207+ // sub-agents each needing approval would queue behind one another.
208+ // Per-request resolution is scoped by the pendingRequests map (keyed
209+ // by request ID), so the wait needs no global lock.
210+ respCh, requestID, resolved, granted := func() (chan bool, string, bool, bool) {
204− s.requestMu.Lock()
211+ s.requestMu.Lock()
205− defer s.requestMu.Unlock()
212+ defer s.requestMu.Unlock()
206−
213+
207− // tell the UI that a permission was requested
214+ // tell the UI that a permission was requested
208− s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
209− ToolCallID: opts.ToolCallID,
210− })
211−
212− s.autoApproveSessionsMu.RLock()
213− autoApprove := s.autoApproveSessions[opts.SessionID]
214− s.autoApproveSessionsMu.RUnlock()
215−
216− if autoApprove {
217 s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
215 s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
218 ToolCallID: opts.ToolCallID,
216 ToolCallID: opts.ToolCallID,
219− Granted: true,
220 })
217 })
221− return true, nil
222− }
223
218
224− fileInfo, err := os.Stat(opts.Path)
219+ s.autoApproveSessionsMu.RLock()
225− dir := opts.Path
220+ autoApprove := s.autoApproveSessions[opts.SessionID]
226− if err == nil {
221+ s.autoApproveSessionsMu.RUnlock()
227− if fileInfo.IsDir() {
222+
228− dir = opts.Path
223+ if autoApprove {
229− } else {
224+ s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
230− dir = filepath.Dir(opts.Path)
225+ ToolCallID: opts.ToolCallID,
226+ Granted: true,
227+ })
228+ return nil, "", true, true
231 }
229 }
232− }
233
230
234− if dir == "." {
231+ fileInfo, err := os.Stat(opts.Path)
235− dir = s.workingDir
232+ dir := opts.Path
236− }
233+ if err == nil {
237− permission := PermissionRequest{
234+ if fileInfo.IsDir() {
238− ID: uuid.New().String(),
235+ dir = opts.Path
239− Path: dir,
236+ } else {
240− SessionID: opts.SessionID,
237+ dir = filepath.Dir(opts.Path)
241− ToolCallID: opts.ToolCallID,
238+ }
242− ToolName: opts.ToolName,
243− Description: opts.Description,
244− Action: opts.Action,
245− Params: opts.Params,
246− }
239+ }
247
240
248− if _, ok := s.sessionPermissions.Get(PermissionKey{
241+ if dir == "." {
242+ dir = s.workingDir
243+ }
244+ permission := PermissionRequest{
245+ ID: uuid.New().String(),
246+ Path: dir,
249− SessionID: permission.SessionID,
247+ SessionID: opts.SessionID,
250− ToolName: permission.ToolName,
248+ ToolCallID: opts.ToolCallID,
251− Action: permission.Action,
249+ ToolName: opts.ToolName,
252− Path: permission.Path,
250+ Description: opts.Description,
253− }); ok {
251+ Action: opts.Action,
254− s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
252+ Params: opts.Params,
255− ToolCallID: opts.ToolCallID,
256− Granted: true,
257− })
258− return true, nil
259− }
253+ }
260
254
261− s.activeRequestMu.Lock()
255+ if _, ok := s.sessionPermissions.Get(PermissionKey{
262− s.activeRequest = &permission
256+ SessionID: permission.SessionID,
263− s.activeRequestMu.Unlock()
257+ ToolName: permission.ToolName,
258+ Action: permission.Action,
259+ Path: permission.Path,
260+ }); ok {
261+ s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
262+ ToolCallID: opts.ToolCallID,
263+ Granted: true,
264+ })
265+ return nil, "", true, true
266+ }
264
267
265− respCh := make(chan bool, 1)
268+ s.activeRequestMu.Lock()
266− s.pendingRequests.Set(permission.ID, respCh)
269+ s.activeRequest = &permission
267− defer s.pendingRequests.Del(permission.ID)
270+ s.activeRequestMu.Unlock()
268
271
272+ respCh := make(chan bool, 1)
273+ s.pendingRequests.Set(permission.ID, respCh)
274+
269− // Publish the request
275+ // Publish the request
270− s.Publish(pubsub.CreatedEvent, permission)
276+ s.Publish(pubsub.CreatedEvent, permission)
277+ return respCh, permission.ID, false, false
278+ }()
279+
280+ if resolved {
281+ return granted, nil
282+ }
283+ defer s.pendingRequests.Del(requestID)
271
284
272 select {
285 select {
273 case <-ctx.Done():
286 case <-ctx.Done():