test(agent-loader): add tests for parseToolsConfig YAML array and CSV handling#2921
Closed
robinmordasiewicz wants to merge 4 commits intocode-yeongyu:devfrom
Closed
test(agent-loader): add tests for parseToolsConfig YAML array and CSV handling#2921robinmordasiewicz wants to merge 4 commits intocode-yeongyu:devfrom
robinmordasiewicz wants to merge 4 commits intocode-yeongyu:devfrom
Conversation
The agent loader's parseToolsConfig() only handled CSV strings (tools: Read, Bash) but not YAML lists (tools:\n - Read\n - Bash). When js-yaml parsed a YAML list into string[], calling .split() on it threw TypeError which was silently caught, skipping the agent. Mirror the fix from 48f6c5e (skill allowed-tools-parser) into both agent loaders: accept string | string[] and use Array.isArray() to branch between array and CSV parsing. Files changed: - claude-code-agent-loader/types.ts: tools?: string | string[] - claude-code-agent-loader/loader.ts: parseToolsConfig handles both - claude-code-plugin-loader/agent-loader.ts: same fix for plugins
… handling Export parseToolsConfig in both agent-loader and plugin-agent-loader for testability. Add comprehensive test coverage for: - undefined and empty string input - CSV comma-separated string parsing - YAML array (string[]) parsing - whitespace trimming and empty entry filtering - case-insensitive key normalization Closes #6 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 8 files
Confidence score: 3/5
- There is a concrete medium-risk issue in
.github/workflows/sync-upstream.yml:gh apifailures are being suppressed, and unknown/error responses can be treated asup-to-date, which may mask real sync breakages. - The severity/confidence signal (6/10, 8/10) suggests this is more than a cosmetic concern; it can cause silent CI workflow regressions even if application code is unchanged.
- Pay close attention to
.github/workflows/sync-upstream.yml- error handling and response classification should fail clearly instead of reporting falseup-to-datestatus.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/sync-upstream.yml">
<violation number="1" location=".github/workflows/sync-upstream.yml:26">
P2: The workflow suppresses `gh api` failures and misclassifies unknown/error responses as `up-to-date`, which can hide real upstream sync failures.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| --method POST \ | ||
| "repos/${{ github.repository }}/merge-upstream" \ | ||
| -f branch=dev \ | ||
| 2>&1) || true |
There was a problem hiding this comment.
P2: The workflow suppresses gh api failures and misclassifies unknown/error responses as up-to-date, which can hide real upstream sync failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/sync-upstream.yml, line 26:
<comment>The workflow suppresses `gh api` failures and misclassifies unknown/error responses as `up-to-date`, which can hide real upstream sync failures.</comment>
<file context>
@@ -0,0 +1,106 @@
+ --method POST \
+ "repos/${{ github.repository }}/merge-upstream" \
+ -f branch=dev \
+ 2>&1) || true
+
+ if echo "$RESPONSE" | grep -q '"merge_type"'; then
</file context>
Owner
|
Thanks for the test improvements! However, this PR mixes fork-specific infrastructure ( Could you split this into two PRs?
Happy to merge the test improvements once separated! |
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
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.
Summary
parseToolsConfigin both agent-loader and plugin-agent-loader for testabilityTest Cases
undefinedand empty string input → returnsundefinedstring[]) parsing → same boolean mapundefinedChanges
src/features/claude-code-agent-loader/loader.ts— exportparseToolsConfigsrc/features/claude-code-plugin-loader/agent-loader.ts— exportparseToolsConfigsrc/features/claude-code-agent-loader/parse-tools-config.test.ts— new (10 test cases)src/features/claude-code-plugin-loader/parse-tools-config.test.ts— new (10 test cases)Summary by cubic
Adds YAML array and CSV support for the
toolsfield in both agent loaders and adds tests to cover parsing and edge cases. Also introduces an upstream sync workflow and new ops issue templates.Bug Fixes
parseToolsConfignow acceptsstring | string[]in both loaders and normalizes keys.undefinedfor empty input.parseToolsConfigfor testability.tools?: string | string[].New Features
.github/workflows/sync-upstream.ymlto auto-merge upstream every 6 hours and rebaseops; creates issues on conflicts..github/ISSUE_TEMPLATE/ops-task.ymlandupstream-contribution.ymlfor structured ops and upstream work.Written for commit 10acb47. Summary will update on new commits.