-
Notifications
You must be signed in to change notification settings - Fork 253
Closed
Labels
Description
Objective
Propagate the caller-provided context.Context into checkActorPermission instead of creating an independent context.Background()-derived context, so that MCP client disconnection and server shutdown are respected.
Context
From Sergo analysis run §22372431560 (discussion #18227).
checkActorPermission in pkg/cli/mcp_server_helpers.go creates its own 5-second timeout context from context.Background(), ignoring the live MCP request context passed by its callers. If the MCP client disconnects or the server shuts down, the parent context will be cancelled — but the GitHub API call inside checkActorPermission will continue for up to 5 seconds.
Current Code
// pkg/cli/mcp_server_helpers.go:205
func checkActorPermission(actor, validateActor, toolName string) error {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
permission, err := queryActorRole(ctx, actor, repo)
...
}Callers pass a live ctx but do not thread it through:
// pkg/cli/mcp_tools_privileged.go:72
}, func(ctx context.Context, req *mcp.CallToolRequest, args logsArgs) (*mcp.CallToolResult, any, error) {
if err := checkActorPermission(actor, validateActor, "logs"); err != nil { // ctx not passedApproach
- Add
ctx context.Contextas the first parameter tocheckActorPermission - Derive the timeout context from the incoming
ctxrather thancontext.Background():func checkActorPermission(ctx context.Context, actor, validateActor, toolName string) error { ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() ... }
- Update all call sites in
pkg/cli/mcp_tools_privileged.goto passctx - Update any tests for
checkActorPermission
Files to Modify
pkg/cli/mcp_server_helpers.go— updatecheckActorPermissionsignaturepkg/cli/mcp_tools_privileged.go— update call sites to passctx- Any test files covering these functions
Acceptance Criteria
-
checkActorPermissionacceptscontext.Contextas its first parameter - The 5-second timeout is derived from the incoming context (not
context.Background()) - All call sites pass the live request
ctx - MCP client cancellation propagates into the permission check
- Existing tests pass; add a test verifying context cancellation is respected if feasible
Generated by Plan Command for issue #discussion #18227
- expires on Feb 27, 2026, 6:22 AM UTC
Reactions are currently unavailable