feat(config): allow plugin configuration via second options parameter#65
Merged
dialupdisaster merged 3 commits intoJul 4, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthrough
ChangesOtelPluginOptions config resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/config.test.ts (1)
306-419: ⚡ Quick winAdd regression tests for malformed options object and enum fallback behavior.
This suite doesn’t currently assert two critical paths:
loadConfig(null)safety and invalidprotocol/metricsTemporalityoption values falling through to env vars.Suggested test additions
+ test("null options object does not throw and falls back to env/defaults", () => { + process.env["OPENCODE_OTLP_ENDPOINT"] = "http://env:4317" + const cfg = loadConfig(null as unknown as Parameters<typeof loadConfig>[0]) + expect(cfg.endpoint).toBe("http://env:4317") + }) + + test("invalid option protocol falls back to env protocol", () => { + process.env["OPENCODE_OTLP_PROTOCOL"] = "http/json" + const cfg = loadConfig({ protocol: "bad" as unknown as "grpc" }) + expect(cfg.protocol).toBe("http/json") + }) + + test("invalid option metricsTemporality falls back to env temporality", () => { + process.env["OPENCODE_OTLP_METRICS_TEMPORALITY"] = "delta" + const cfg = loadConfig({ metricsTemporality: "bad" as unknown as "delta" }) + expect(cfg.metricsTemporality).toBe("delta") + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/config.test.ts` around lines 306 - 419, The "loadConfig options" test suite is missing coverage for two critical scenarios. Add a new test case within this describe block to verify that loadConfig(null) handles null input safely without throwing errors. Additionally, add test cases that verify invalid protocol option values and invalid metricsTemporality option values (non-enum strings) properly fall back to environment variables or defaults, similar to how the existing "non-number option interval is ignored" test validates invalid interval inputs. These tests ensure the loadConfig function is resilient to unexpected option values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config.ts`:
- Around line 120-122: The issue is that `pickString` extracts values from
`options.metricsTemporality` and `options.protocol` without validation, causing
invalid option values to bypass the environment variable fallback. When an
option is provided but malformed, it should fall through to the environment
variable instead of using the default. Fix this by validating the option value
first before using it; only if the option value is missing or invalid should you
fall back to the environment variable, and only if that's also missing or
invalid should you use the default. Apply this pattern to both the `protocol`
assignment and the `metricsTemporality` assignment, ensuring proper precedence:
validated option → validated env var → default.
- Around line 114-121: The loadConfig function does not guard against cases
where the options parameter might be null or a non-object value, causing runtime
errors when attempting to access properties like options.otlpHeaders,
options.otlpHeadersHelper, options.resourceAttributes, options.traceparent,
options.tracestate, options.metricsTemporality, and options.protocol. Add a
guard at the start of the loadConfig function to ensure options is a valid
object before attempting to access its properties, falling back to the empty
object default if it is null, undefined, or not an object.
---
Nitpick comments:
In `@tests/config.test.ts`:
- Around line 306-419: The "loadConfig options" test suite is missing coverage
for two critical scenarios. Add a new test case within this describe block to
verify that loadConfig(null) handles null input safely without throwing errors.
Additionally, add test cases that verify invalid protocol option values and
invalid metricsTemporality option values (non-enum strings) properly fall back
to environment variables or defaults, similar to how the existing "non-number
option interval is ignored" test validates invalid interval inputs. These tests
ensure the loadConfig function is resilient to unexpected option values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7646e227-8b41-4d79-9af4-be6b513d5aca
📒 Files selected for processing (5)
AGENTS.mdREADME.mdsrc/config.tssrc/index.tstests/config.test.ts
cbrachem
changed the title
Contributor
|
@cbrachem can you rebase on master and fix the conflicts? |
Resolve plugin configuration from opencode's plugin tuple form
(["opencode-plugin-otel", { ... }]) in addition to OPENCODE_* env vars.
Precedence is option > env > default, so existing env-only setups are
unchanged and loadConfig() still works with no arguments.
loadConfig now accepts an OtelPluginOptions object (keys 1:1 with
PluginConfig) and validates every field at runtime, so malformed JSON
options fall through to env/default. The entrypoint threads opencode's
second factory argument through to loadConfig.
cbrachem
force-pushed
the
feat/plugin-tuple-options
branch
from
d880778 to
ceb79a4
Compare
Contributor
Author
@dialupdisaster Sure. Done. Sorry about the delay, another thing came up. |
Contributor
|
Small docs gap: the new plugin options table appears to be missing This was added between when you created this PR and now. |
Co-authored-by: Marc <157699+dialupdisaster@users.noreply.github.com>
Contributor
Author
Yeah I noticed; I added the option in code but forgot about the README when fixing the merge conflict. Thanks! |
Contributor
|
Thanks for the contribution! I really like this config method better than environment variables, lol |
dialupdisaster
approved these changes
Jul 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Since March 2026, Opencode has supported passing options to plugins as a second parameter. This change allows resolving plugin configuration from opencode's plugin tuple form
(["opencode-plugin-otel", { ... }])in addition toOPENCODE_*env vars. Precedence is option > env > default, so existing env-only setups are unchanged andloadConfig()still works with no arguments.loadConfig now accepts an
OtelPluginOptionsobject (keys 1:1 with PluginConfig) and validates every field at runtime, so malformed JSON options fall through to env/default. The entrypoint threads opencode's second factory argument through to loadConfig.Type of change
Checklist
bun run lintpasses with no errorsbun run check:jsdoc-coveragepasses with no errorsbun run typecheckpasses with no errorsbun testpasses with no errorsRelated issues
none
Additional context
none
Summary by CodeRabbit
New Features
opencode.jsontuple syntax, which override matching environment variables and built-in defaults.allexpansion for trace types).Documentation
Tests
null.