Skip to content

SystemConfigReader for Apple platforms using NSUserDefaults#1231

Merged
kpavlov merged 2 commits intodevelopfrom
kpavlov/apple-config-reader
Dec 5, 2025
Merged

SystemConfigReader for Apple platforms using NSUserDefaults#1231
kpavlov merged 2 commits intodevelopfrom
kpavlov/apple-config-reader

Conversation

@kpavlov
Copy link
Copy Markdown
Contributor

@kpavlov kpavlov commented Dec 5, 2025

Add multiplatform system configuration and secrets readers with JVM-specific implementation

Introduce SystemConfigReader and SystemSecretsReader interfaces to standardize access to configuration values and secrets across different platforms. This change utilizes the expect/actual mechanism to define common contracts with specific platform implementations.

  • Implement fully functional readers for the JVM target: the configuration reader resolves values from environment variables and system properties (supports name normalization), while the secrets reader strictly uses environment variables for security.
  • Add stub implementations for Android, Apple, JS, and WasmJS platforms that currently throw a NotImplementedError when accessed.
  • Define the common interfaces and factory functions to allow retrieval of configuration strings and secrets.
  • Add UserDefaultsSystemConfigReader backed by Apple's NSUserDefaults for retrieving configuration properties. Update systemConfigReader to use this implementation and introduce comprehensive unit tests to ensure correctness.

Motivation and Context

Should be used in Debugger Config and LLM Clients

See also #1228

Breaking Changes

No


Type of the changes

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

Checklist

  • The pull request has a description of the proposed change
  • I read the Contributing Guidelines before opening the pull request
  • The pull request uses develop as the base branch
  • Tests for the changes have been added
  • All new and existing tests passed
Additional steps for pull requests adding a new feature
  • An issue describing the proposed change exists
  • The pull request includes a link to the issue
  • The change was discussed and approved in the issue
  • Docs have been added / updated

Introduce `SystemConfigReader` and `SystemSecretsReader` interfaces to standardize access to configuration values and secrets across different platforms. This change utilizes the `expect`/`actual` mechanism to define common contracts with specific platform implementations.

*   Implement fully functional readers for the JVM target: the configuration reader resolves values from environment variables and system properties (supports name normalization), while the secrets reader strictly uses environment variables for security.
*   Add stub implementations for Android, Apple, JS, and WasmJS platforms that currently throw a `NotImplementedError` when accessed.
*   Define the common interfaces and factory functions to allow retrieval of configuration strings and secrets.
Add `UserDefaultsSystemConfigReader` backed by Apple's NSUserDefaults for retrieving configuration properties. Update `systemConfigReader` to use this implementation and introduce comprehensive unit tests to ensure correctness.
@kpavlov kpavlov added the enhancement New feature or request label Dec 5, 2025
@kpavlov kpavlov marked this pull request as ready for review December 5, 2025 17:02
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 5, 2025

Qodana for JVM

1180 new problems were found

Inspection name Severity Problems
Check Kotlin and Java source code coverage 🔶 Warning 1160
Missing KDoc for public API declaration 🔶 Warning 20
@@ Code coverage @@
+ 72% total lines covered
16830 lines analyzed, 12203 lines covered
# Calculated according to the filters of your coverage tool

☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

Copy link
Copy Markdown
Collaborator

@EugeneTheDev EugeneTheDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for a speedy implementation, let's merge it

override fun getConfigVariable(name: String): String? {
return System.getenv(name)
?: System.getProperty(name)
?: System.getProperty(normalizePropertyName(name))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure about this part, but now it looks better, since at least it won't miss variables unexpectedly. We can discuss this part separately later

@kpavlov kpavlov merged commit 5b9c412 into develop Dec 5, 2025
11 checks passed
@kpavlov kpavlov deleted the kpavlov/apple-config-reader branch December 5, 2025 18:07
vova-jb pushed a commit that referenced this pull request Jan 27, 2026
# Add multiplatform system configuration and secrets readers with
JVM-specific implementation

Introduce `SystemConfigReader` and `SystemSecretsReader` interfaces to
standardize access to configuration values and secrets across different
platforms. This change utilizes the `expect`/`actual` mechanism to
define common contracts with specific platform implementations.

* Implement fully functional readers for the JVM target: the
configuration reader resolves values from environment variables and
system properties (supports name normalization), while the secrets
reader strictly uses environment variables for security.
* Add stub implementations for Android, Apple, JS, and WasmJS platforms
that currently throw a `NotImplementedError` when accessed.
* Define the common interfaces and factory functions to allow retrieval
of configuration strings and secrets.
* Add `UserDefaultsSystemConfigReader` backed by Apple's NSUserDefaults
for retrieving configuration properties. Update `systemConfigReader` to
use this implementation and introduce comprehensive unit tests to ensure
correctness.

## Motivation and Context
Should be used in Debugger Config and LLM Clients

See also #1228 

## Breaking Changes
No

---

#### Type of the changes
- [x] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Tests improvement
- [ ] Refactoring

#### Checklist
- [x] The pull request has a description of the proposed change
- [x] I read the [Contributing
Guidelines](https://github.com/JetBrains/koog/blob/main/CONTRIBUTING.md)
before opening the pull request
- [x] The pull request uses **`develop`** as the base branch
- [x] Tests for the changes have been added
- [x] All new and existing tests passed

##### Additional steps for pull requests adding a new feature
- [ ] An issue describing the proposed change exists
- [ ] The pull request includes a link to the issue
- [ ] The change was discussed and approved in the issue
- [ ] Docs have been added / updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants