-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(agent-config): use mergeAgentConfig for prometheus and opencode-builder #719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 2 files
Confidence score: 4/5
- Overall risk looks low, with only a test-gap concern preventing full confidence in coverage of the merge logic.
src/plugin-handlers/config-handler.test.tscurrently exercises manual string concatenation rather than the real mergeAgentConfig/resolveCategoryConfig path, so regressions in those functions could slip through without failing tests.- Pay close attention to
src/plugin-handlers/config-handler.test.ts- strengthen coverage to ensure merge logic is truly exercised.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/plugin-handlers/config-handler.test.ts">
<violation number="1" location="src/plugin-handlers/config-handler.test.ts:105">
P2: Test does not exercise production merge logic; manual string concat will pass even if mergeAgentConfig/resolveCategoryConfig are broken</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…r prometheus and opencode-builder
|
@vsumner I'm getting a test error from this: |
|
Once it passes tests, it will lgtm |
Addressed feedback: Updated OpenCode-Builder test to properly exercise production
mergeAgentConfiglogic.Changes
mergeAgentConfigfromagents/utils.tsin test filemergeAgentConfig()function withBUILD_SYSTEM_PROMPTprompt_appendis appended with "\n" separator via real merge logicDetails
The test now:
BUILD_SYSTEM_PROMPTfrombuild-prompt.tsuserConfigobject withprompt_appendmergeAgentConfig({ prompt: BUILD_SYSTEM_PROMPT }, userConfig)This properly tests the production code path, not just manual string concatenation.