fix(nacos-skill): handle block-sequence arrays in SKILL.md frontmatter#1445
fix(nacos-skill): handle block-sequence arrays in SKILL.md frontmatter#1445sunwg2 wants to merge 9 commits into
Conversation
|
|
|
AgentScopeJavaBot
added
bug
Nacos exports use multi-line `- item` syntax for array fields. The old normalizer concatenated items into an invalid scalar, causing SnakeYAML to throw "sequence entries are not allowed here in 'string'". Collect `- item` lines into a list and emit them as an inline YAML array `["a", "b"]`; strip YAML quoting from raw items via unquoteYamlString(). Closes agentscope-ai#1438
|
recheck |
AgentScopeJavaBot
left a comment
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR fixes Bug #1438 by adding support for YAML block-sequence arrays (- item) in SKILL.md frontmatter within the Nacos skill repository. When NacosSkillRepository normalizes Nacos-exported YAML for the downstream MarkdownSkillParser, block-sequence entries under a key with no inline value are now collected into a list and serialized as an inline YAML array. The implementation is clean and well-scoped with four integration tests covering multi-item, single-item, special-character, and mixed scalar/array scenarios. No critical or blocking issues were found.
| char last = value.charAt(value.length() - 1); | ||
| if (first == '"' && last == '"') { | ||
| String inner = value.substring(1, value.length() - 1); | ||
| return inner.replace("\\\"", "\"").replace("\\\\", "\\"); |
There was a problem hiding this comment.
[nit] The double-quoted unescape in unquoteYamlString only handles \" and \\. Other valid YAML double-quoted escape sequences (\n, \t, \uXXXX, etc.) pass through as literal characters. The Javadoc should note this is an intentional subset.
There was a problem hiding this comment.
Good point. Updated the Javadoc to explicitly note that only " and \ are unescaped — other YAML double-quoted escape sequences (\n, \t, unicode escapes, etc.) pass through as literal characters, and this is an intentional subset covering the Nacos export format.
| assertFalse(repository.delete("any-skill")); | ||
| } | ||
|
|
||
| // -------- Bug #1438: multi-line block-sequence array in frontmatter -------- |
There was a problem hiding this comment.
[nit] The test suite covers double-quoted and unquoted list items but has no test for single-quoted values (e.g., - 'value with ''apostrophe'''). Adding a test case would exercise the single-quote branch of unquoteYamlString.
There was a problem hiding this comment.
Good catch. Added testGetSkillWithSingleQuotedArrayItems to cover the single-quote branch of unquoteYamlString, including a case with '' (escaped apostrophe) to verify it unescapes correctly to '.
…oted array test Address review comments on PR agentscope-ai#1445: - Clarify in Javadoc that only backslash-quote and double-backslash are unescaped in double-quoted mode; other YAML escape sequences (e.g. \n, \t, unicode escapes) pass through as literals. This is an intentional subset covering the Nacos export format. - Add testGetSkillWithSingleQuotedArrayItems to exercise the single-quote branch of unquoteYamlString, including the apostrophe unescape case ('' -> ').
4 participants
Nacos exports use multi-line
- itemsyntax for array fields. The old normalizer concatenated items into an invalid scalar, causing SnakeYAML to throw "sequence entries are not allowed here in 'string'".Collect
- itemlines into a list and emit them as an inline YAML array["a", "b"]; strip YAML quoting from raw items via unquoteYamlString().Closes #1438
AgentScope-Java Version
[The version of AgentScope-Java you are working on, e.g. 1.0.12, check your pom.xml dependency version or run
mvn dependency:tree | grep agentscope-parent:pom(only mac/linux)]Description
Fix #1438 —
NacosSkillRepositorycrashes withsequence entries are not allowed here in 'string'when aSKILL.mdexported from Nacos uses YAML block-sequence syntax for array fields
such as
trigger_intents.normalizeFoldedFlatYaml()was designed to fold indented continuationlines back into the previous key's value. It had no awareness of YAML
block-sequence entry lines (
- item), so it treated them as plaincontinuation text and concatenated all items into a single invalid scalar:
Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)