Detekt configuration by devcrocod · Pull Request #501 · modelcontextprotocol/kotlin-sdk

devcrocod · 2026-02-02T15:40:23Z

generate detekt config
enable several rules
disable ktlint backing property check

Motivation and Context

Centralized control of detekt rules. Using detekt 2 configuration

How Has This Been Tested?

locally

Breaking Changes

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

…fig` and remove related suppressions in codebase

Copilot

Pull request overview

This PR centralizes Detekt configuration by generating a comprehensive detekt.yml configuration file and updating the project to use Detekt 2 configuration format. The changes include enabling several detekt rules, disabling the ktlint backing-property-naming rule, and removing various @Suppress annotations from the codebase that are now either allowed by the configuration or handled differently.

Changes:

Moved detekt configuration from root detekt.yml to config/detekt/detekt.yml with a comprehensive Detekt 2 configuration including 853 lines of rule definitions
Disabled ktlint backing-property-naming rule in .editorconfig
Removed @Suppress annotations for TooManyFunctions, backing-property-naming, LongParameterList, TooGenericExceptionCaught, and LongMethod throughout the codebase

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
config/detekt/detekt.yml	New comprehensive Detekt 2 configuration with detailed rule definitions for complexity, naming, style, and other code quality checks
detekt.yml	Removed old minimal configuration file from repository root
build.gradle.kts	Updated config path to point to new location at `config/detekt/detekt.yml`
.editorconfig	Added configuration to disable ktlint backing-property-naming rule
kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerSession.kt	Removed @Suppress annotations for TooManyFunctions and backing-property-naming rules
kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt	Removed @Suppress annotations for TooManyFunctions, backing-property-naming, LongParameterList, and TooGenericExceptionCaught rules
kotlin-sdk-client/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/StreamableHttpClientTest.kt	Removed @Suppress annotation for LongMethod rule on test class
kotlin-sdk-client/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/OldStreamableHttpClientTest.kt	Removed @Suppress annotation for LongMethod rule on test class
kotlin-sdk-client/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/MockMcp.kt	Removed @Suppress annotations for LongParameterList rule on multiple functions
kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/StdioClientTransport.kt	Removed @Suppress annotations for TooGenericExceptionCaught rule while retaining SwallowedException suppressions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-02T15:45:33Z

kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt

@@ -74,7 +74,6 @@ public class ServerOptions(public val capabilities: ServerCapabilities, enforceS
 * this server. The provider is called each time a new session is started to support dynamic instructions.
 * @param block A block to configure the mcp server.
 */


The Server class has approximately 41 functions (20+ public, 6+ private, and several suspend functions), which exceeds the TooManyFunctions limit of 20 functions per class configured on line 156. Removing the @Suppress("TooManyFunctions") annotation will cause detekt to report this violation. Consider either increasing the allowedFunctionsPerClass threshold or keeping the suppression annotation.

Suggested change

*/

*/

@Suppress("TooManyFunctions")

Copilot · 2026-02-02T15:45:34Z

kotlin-sdk-client/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/MockMcp.kt

@@ -109,7 +108,6 @@ internal class MockMcp(verbose: Boolean = false) {
        }
    }



The handleWithResult function has 9 parameters (8 named parameters plus a vararg parameter), which exceeds the LongParameterList limit of 8 configured in detekt.yml line 119. Removing the @Suppress("LongParameterList") annotation will cause detekt to report this violation. Consider either reducing the number of parameters, combining related parameters into a configuration object, or keeping the suppression annotation.

Suggested change

@Suppress("LongParameterList")

Copilot · 2026-02-02T15:45:34Z

kotlin-sdk-client/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/MockMcp.kt

@@ -170,7 +167,6 @@ internal class MockMcp(verbose: Boolean = false) {
        )
    }



The handleJSONRPCRequest function has 9 parameters (8 named parameters plus a vararg parameter), which exceeds the LongParameterList limit of 8 configured in detekt.yml line 119. Removing the @Suppress("LongParameterList") annotation will cause detekt to report this violation. Consider either reducing the number of parameters, combining related parameters into a configuration object, or keeping the suppression annotation.

Suggested change

@Suppress("LongParameterList")

Copilot · 2026-02-02T15:45:35Z

config/detekt/detekt.yml

@@ -0,0 +1,853 @@
+config:
+  validation: true
+  warningsAsErrors: false


The configuration now sets warningsAsErrors to false, which is a significant change from the original detekt.yml that had warningsAsErrors: true. This means detekt violations will no longer fail the build, potentially allowing code quality issues to accumulate. Consider whether this is intentional and aligns with the project's quality standards. If warnings should still fail the build, set this value back to true.

Copilot · 2026-02-02T15:45:35Z

config/detekt/detekt.yml

+        value: 'STOPSHIP:'
+      - reason: 'Forbidden TODO todo marker in comment, please do the changes.'
+        value: 'TODO:'
+    allowedPatterns: ''


The ForbiddenComment rule is now active and forbids TODO: comments. However, there are existing TODO: comments in the codebase (e.g., in StreamableHttpClientTest.kt line 136 and OldStreamableHttpClientTest.kt line 84). This will cause detekt violations unless these TODO comments are either removed or the allowedPatterns is updated to permit them. Consider either removing the existing TODO comments or adjusting the allowedPatterns to exclude test files or allow specific TODO patterns.

Suggested change

allowedPatterns: ''

allowedPatterns: 'TODO:'

kpavlov

Thank you, @devcrocod,
I have only 2 comments, everything else is fine

kpavlov · 2026-02-02T16:14:48Z

config/detekt/detekt.yml

Let's keep the default rules and override only where necessary. There's already a flag in the configuration: buildUponDefaultConfig = true. The current config is very hard to reason about

kpavlov · 2026-02-02T16:16:26Z

build.gradle.kts


    detekt {
-        config = files("$rootDir/detekt.yml")
+        config = files("$rootDir/config/detekt/detekt.yml")


Let's update this line too, for consistency

kpavlov

LGTM

# Detekt setup hardeing; address some issues - Address some detekt findings: missing KDocs, formatting, use `check*` & `orEmpty()`` - Remove unnecessary KDoc comments from deprecated classes in `types.kt` - Add Detekt baseline XML files for all modules - Enforce stricter Detekt rules by setting `failOnSeverity` from `None` to `Error` ## Motivation and Context Detekt is not actually failing the build on violations. This PR is enabling Detekt to fail on errors. Baseline fines added for historical issues. Follow-up for #501 ## How Has This Been Tested? Regressions ## Breaking Changes No breaking changes. ## Types of changes  - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] Refactoring - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [x] Documentation update ## Checklist  - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed

devcrocod added 3 commits January 30, 2026 20:12

Update detekt.yml to adjust configuration standards

93da614

update config path in Gradle

d72b91a

Disable ktlint_standard_backing-property-naming rule in `.editorcon…

a83e0ba

…fig` and remove related suppressions in codebase

devcrocod requested review from Copilot and kpavlov February 2, 2026 15:40

Copilot started reviewing on behalf of devcrocod February 2, 2026 15:40 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

kpavlov reviewed Feb 2, 2026

View reviewed changes

kpavlov approved these changes Feb 4, 2026

View reviewed changes

kpavlov merged commit 6f1c120 into main Feb 4, 2026
18 checks passed

kpavlov deleted the devcrocod/detekt branch February 4, 2026 15:03

kpavlov mentioned this pull request Feb 6, 2026

refactor: Harden detekt setup, address some issues #507

Merged

10 tasks

devcrocod mentioned this pull request Mar 12, 2026

feat: add SSE reconnection with retry support #596

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detekt configuration#501

Detekt configuration#501
kpavlov merged 3 commits intomainfrom
devcrocod/detekt

devcrocod commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

kpavlov left a comment

Uh oh!

kpavlov Feb 2, 2026

Uh oh!

kpavlov Feb 2, 2026

Uh oh!

kpavlov left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -109,7 +108,6 @@ internal class MockMcp(verbose: Boolean = false) {
		}
		}

		@@ -170,7 +167,6 @@ internal class MockMcp(verbose: Boolean = false) {
		)
		}

Conversation

devcrocod commented Feb 2, 2026

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

kpavlov left a comment

Choose a reason for hiding this comment

Uh oh!

kpavlov Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

kpavlov Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

kpavlov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants