Feat/ai gen#11
Conversation
- Added LLM/Agent integration section in README.md with command tree documentation and MCP resource URIs. - Updated MCP.md to include new sections for MCP resources, prompts, and agent hints, detailing their usage and examples. - Modified handler.go to include schema generation for response types, enhancing type metadata visibility. - Added tests for schema generation and agent hints in handler_test.go. - Implemented resource registration for JSON schemas and prompts in mcpserver, including detailed descriptions and usage guides. - Extended tools.go to support additional types in schema generation and improved command descriptions with agent hints. - Created prompts.go to manage prompt registration and generation for command usage and overview.
- Introduced `vizcmd` package with commands for generating Mermaid diagrams: - `tree`: visualizes command tree structure. - `dispatch`: illustrates command dispatch flow. - `mcp-sequence`: displays MCP tool invocation sequence. - Implemented tests for visualization commands to ensure correct output and formatting. - Added `doccmd` package with documentation tests for command structure and API responses.
- Removed `--env`, `-e`, and `--env-file` flags from the command completion and documentation. - Eliminated the `preloadEnvFromArgs` function and its associated tests, which handled environment variable preloading. - Updated tests to reflect the removal of environment variable handling. - Adjusted global options in documentation to remove references to environment variable flags.
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the redant CLI framework by introducing new visualization and documentation tools, including viz for Mermaid diagrams, doc for an interactive documentation site, and llms-txt for LLM-optimized command tree summaries. It also deepens integration with the Model Context Protocol (MCP) by adding support for resources, prompts, and agent-specific metadata (hints). Notably, the global --env and --env-file flags have been removed in favor of option-level environment variable handling. Feedback on the new JSON schema generation logic suggests improving the handling of anonymous fields, special types like time.Time, and []byte slices, as well as increasing the default schema recursion depth and using more idiomatic string splitting.
| props := map[string]any{} | ||
| for i := range t.NumField() { | ||
| f := t.Field(i) | ||
| if !f.IsExported() { | ||
| continue | ||
| } | ||
| name := f.Name | ||
| omitempty := false | ||
| if tag, ok := f.Tag.Lookup("json"); ok { | ||
| parts := splitTag(tag) | ||
| if parts[0] == "-" { | ||
| continue | ||
| } | ||
| if parts[0] != "" { | ||
| name = parts[0] | ||
| } | ||
| for _, p := range parts[1:] { | ||
| if p == "omitempty" { | ||
| omitempty = true | ||
| } | ||
| } | ||
| } | ||
| fieldSchema := reflectTypeSchemaInner(f.Type, depth+1) | ||
| if omitempty { | ||
| fieldSchema["x-omitempty"] = true | ||
| } | ||
| props[name] = fieldSchema | ||
| } | ||
| return map[string]any{ | ||
| "type": "object", | ||
| "properties": props, | ||
| } |
There was a problem hiding this comment.
The struct schema generation has two significant issues:
- Anonymous Fields: It doesn't handle embedded (anonymous) fields correctly. Standard Go JSON encoding flattens these fields into the parent object, but this implementation treats them as nested objects named after the type.
- Special Types: Common types like
time.Timeare expanded into their internal struct fields (wall, ext, loc) instead of being represented as a string withdate-timeformat.
Consider adding special handling for time.Time and logic to flatten anonymous fields to match encoding/json behavior.
| return reflectTypeSchemaInner(t, 0) | ||
| } | ||
|
|
||
| const maxSchemaDepth = 5 |
There was a problem hiding this comment.
The maxSchemaDepth of 5 is quite restrictive for real-world data structures. Deeply nested structs or complex API responses can easily exceed this limit, resulting in truncated and less useful schemas for AI agents. Consider increasing this to a more reasonable default like 10 or 15.
| const maxSchemaDepth = 5 | |
| const maxSchemaDepth = 15 |
| case reflect.Slice, reflect.Array: | ||
| return map[string]any{ | ||
| "type": "array", | ||
| "items": reflectTypeSchemaInner(t.Elem(), depth+1), | ||
| } |
There was a problem hiding this comment.
The current implementation treats []byte (slice of uint8) as an array of integers in the JSON schema. However, encoding/json by default serializes []byte as a base64-encoded string. The schema should reflect this behavior to avoid confusing consumers.
case reflect.Slice, reflect.Array:
if t.Elem().Kind() == reflect.Uint8 {
return map[string]any{"type": "string", "format": "byte"}
}
return map[string]any{
"type": "array",
"items": reflectTypeSchemaInner(t.Elem(), depth+1),
}| func splitTag(tag string) []string { | ||
| var parts []string | ||
| for tag != "" { | ||
| i := 0 | ||
| for i < len(tag) && tag[i] != ',' { | ||
| i++ | ||
| } | ||
| parts = append(parts, tag[:i]) | ||
| if i < len(tag) { | ||
| tag = tag[i+1:] | ||
| } else { | ||
| break | ||
| } | ||
| } | ||
| return parts | ||
| } |
There was a problem hiding this comment.
| d, err := time.ParseDuration(v) | ||
| if err != nil || d <= 0 { | ||
| return 0 |
There was a problem hiding this comment.
Errors from time.ParseDuration are silently ignored. If a user provides an invalid duration string (e.g., in the agent.timeout metadata), the command will proceed without any timeout instead of notifying the user about the malformed configuration. It would be better to log a warning or handle this during the tool collection phase.
- Removed the `cmds/vizcmd/` command group responsible for generating Mermaid diagrams for command trees, dispatch flows, and MCP sequences. - Integrated the visualization functionality into the `cmds/doccmd/` command, allowing for the generation of interactive documentation with embedded Mermaid diagrams. - Updated documentation to reflect the changes in command structure and usage. - Adjusted related tests and examples to ensure compatibility with the new command structure.
kooksee
left a comment
There was a problem hiding this comment.
PR #11 分轮审查报告(Round 0 → Round 4)
审查概要
| 指标 | 值 |
|---|---|
| 审查模式 | full-review |
| PR 范围 | 64 文件,+4653/-1323 |
| 模块覆盖 | 16/16 |
| 分类覆盖 | 26/26 |
问题统计
| 等级 | 数量 |
|---|---|
| 🔴 Blocker | 0 |
| 🟠 Major | 2 |
| 🟡 Minor | 5 |
| 🟢 Nit | 2 |
Major 问题
-
[CPT]
internalArgsOverrideFlag = "args"冲突风险 —args.go:280- 全局 hidden flag
--args可能与业务命令同名 flag 冲突,导致位置参数被静默覆盖
- 全局 hidden flag
-
[DOCS] Changelog 遗漏多项新增功能 —
.version/changelog/v0.3.0.md- 缺少 llmstxtcmd、MCP Resources/Prompts/Annotations/Timeout、--list-format json、Stream seq/ts、reflectTypeSchema 等条目
Minor 问题
- [RBST]
WriteLLMSTxt错误被_忽略 —resources.go:37、prompts.go:32 - [SEC] doccmd 中
innerHTML使用有微小 XSS 风险 —doc.go:583 - [LOGI]
reflectTypeSchema对interface返回空 map,语义不明确 —handler.go:403(gemini-code-assist 已在同行提出相关 anonymous/time.Time 问题) - 测试缺口:
--args冲突场景 +option.gostderr warning 输出无直接验证
审查建议:Request changes
由于是自己的 PR 无法提交 REQUEST_CHANGES 状态,此处以 COMMENT 形式记录建议。
合入条件
- 补齐 v0.3.0.md changelog(所有新增功能条目)
- 确认
--args内部 flag 命名策略(改名或文档明确保留) - 上述两项解决后可 Approve
门禁自检
modules_total: 16modules_checked: 16missing_modules: 无categories_total: 26categories_checked: 26unresolved_blockers: 0unresolved_majors: 2
| Value: EnumOf(new(string), "text", "json"), | ||
| }, | ||
| { | ||
| Flag: internalArgsOverrideFlag, |
There was a problem hiding this comment.
分类:[CPT] 兼容性
模块:args
等级:Major
问题:内部覆盖参数标志使用公开语义名 --args,存在与业务命令自定义 --args 冲突风险。
原因:const internalArgsOverrideFlag = "args" 注册为全局 hidden flag。若用户业务命令也定义了 --args(常见场景),将触发 flag 冲突或静默覆盖位置参数,破坏既有参数解析语义。在 command.go:843 中 internalArgsFlag.Changed 为 true 时直接覆盖 inv.Args,无「仅内部模式可覆盖」的前置条件检查。
修改意见:将内部 flag 改为保留命名(如 __redant-args),并仅在内部模式启用;或在文档中明确标注 --args 为框架保留名称禁止用户使用。同时补充冲突回归测试 TestInternalArgsFlagConflictWithUserArgs。
|
|
||
| ## 新增 | ||
|
|
||
| - 新增 `cmds/doccmd/` 交互式文档站命令(`doc`):从命令树自动生成类 Swagger UI 的浏览界面,集成 Mermaid 图渲染(命令树、分发流程、MCP 时序)、命令搜索、参数/选项表格。 |
There was a problem hiding this comment.
分类:[DOCS] 文档字符串
模块:docs
等级:Major
问题:v0.3.0 changelog 遗漏多项新增功能与行为变更。
原因:当前 changelog 仅记录 doccmd、env 移除、option warning、ParseJSONArgs 改进,但本 PR 还包括以下未记录变更:
--list-format json+PrintCommandsJSON/PrintFlagsJSONcmds/llmstxtcmd/新模块- MCP Resources / Prompts / ToolAnnotations / Timeout / Agent Hints(
internal/mcpserver/) - Stream envelope
seq/ts字段 ResponseTypeInfo.Schema类型从string变为map[string]anyreflectTypeSchema自动 JSON Schema 生成
用户或下游消费者无法从 changelog 中发现本版本包含的完整行为变更。
修改意见:在"新增"分类补齐 llmstxtcmd、MCP Resources/Prompts/Annotations/Timeout、--list-format json、Stream seq/ts 等条目;在"变更"分类补齐ResponseTypeInfo.Schema类型变更条目。
| Contents: []*mcp.ResourceContents{{ | ||
| URI: req.Params.URI, | ||
| MIMEType: "text/markdown", | ||
| Text: buf.String(), |
There was a problem hiding this comment.
分类:[RBST] 健壮性
模块:mcpserver
等级:Minor
问题:WriteLLMSTxt 返回值被 _ 忽略,错误静默丢弃。
原因:若 WriteLLMSTxt 生成文档出错,MCP 客户端会收到空或不完整的资源内容,无任何错误提示,可能导致 Agent 基于不完整信息做出决策。同样的问题存在于 prompts.go:32。
修改意见:在错误时返回 MCP 错误响应,或至少在 buf 中写入错误说明文本(如 fmt.Fprintf(&buf, "Error generating llms.txt: %v", err))。
| } catch (e) { | ||
| el.style.display = 'none'; | ||
| if (errEl) { | ||
| errEl.innerHTML = '<strong>渲染失败:</strong> ' + (e.message || e).toString().replace(/</g, '<'); |
There was a problem hiding this comment.
分类:[SEC] 安全问题
模块:doccmd
等级:Minor
问题:使用 innerHTML 拼接错误消息,仅对 < 做了转义,> 和引号未转义,存在微小 XSS 风险。
原因:虽然数据源来自本地 API(Mermaid 渲染错误消息),风险较低,但 innerHTML 路径本身不安全。若未来错误消息中包含用户可控内容(如命令名称含特殊字符),可能导致 HTML 注入。
修改意见:改用 textContent 替代 innerHTML(此处无需保留 <strong> 渲染,可改为纯文本前缀 "渲染失败: "),或补全所有 HTML 特殊字符转义(包括 >、"、')。
No description provided.