feat(rule):收紧规则与记忆分层,并补齐全局规则读写入口#542
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
Additional findings not repeated inline:
- Medium:
internal/context/builder_test.go:38-60has the same environment coupling assources_test.go:NewBuilder()uses the default loader, so any real~/.neocode/AGENTS.mdmakesTestDefaultBuilderBuildfail even when the package is correct.
I did not find additional performance, security, or documentation issues beyond the items above.
| } | ||
|
|
||
| // ProjectRulePath 返回项目根规则文件 AGENTS.md 的固定路径。 | ||
| func ProjectRulePath(projectRoot string) string { |
There was a problem hiding this comment.
ProjectRulePath can still return a relative path here, and WriteProjectRule will write to it, but Loader.loadProjectDocument later rejects the same path because it is not absolute. That makes the new read/write API non-round-trippable for callers that pass relative roots. Either normalize to an absolute path here as well, or reject relatives consistently.
| t.Parallel() | ||
|
|
||
| sections, err := (&projectRulesSource{}).Sections(context.Background(), BuildInput{ | ||
| sections, err := newRulesPromptSource(nil).Sections(context.Background(), BuildInput{ |
There was a problem hiding this comment.
This test now depends on the reviewer machine not having a real ~/.neocode/AGENTS.md. newRulesPromptSource(nil) uses the default loader, so the assertion fails as soon as global rules exist in the environment. The test should inject an isolated loader/base dir instead of consulting the real home directory.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
OK,我翻遍了opencode和claudecode,它们都没有在网页或应用整这个全局,项目规则的独立配置指令或交互的入口,而是把它当作一个用户自行了解编辑的内容。 |
|
/code 提升覆盖测试率,并提交PR到Fork仓库里 |
Summary已补齐这轮
Verification
Git
|
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com>
test(rules): improve coverage for rule loading
|
@xgopilot 请仔细审阅相关代码(特别是 internal/rules/loader.go, internal/rules/store.go 和 internal/context),并重点围绕以下 4 个维度输出你的审查意见:
检查 ~/.neocode/AGENTS.md 的路径解析是否使用了标准的 os.UserHomeDir(),以确保在 Windows (%USERPROFILE%) 和 Linux/Ubuntu 下均不产生异常。 评估基于“同目录写临时文件再原子替换(os.Rename)”的落盘机制。如果在 Windows 环境下,目标文件正被另一个高频进程读取,os.Rename 是否会触发 ERROR_ACCESS_DENIED?代码中是否包含了带 Backoff 的重试机制?
在 internal/context 中合并 Project Rules、Global Rules 和 Memo 时,检查其长度截断逻辑。 截断是基于字符、行还是 Token?截断机制是否安全(例如:是否会粗暴切断 Markdown 的代码块反引号,导致后续传入大模型时引发严重的格式解析混乱)?
Monorepo 兼容: 废弃子目录规则后,目前的 ProjectRoot 发现机制是否健壮?在深层嵌套的复杂工程中,是否会因为根规则过于宽泛而干扰 Agent 针对特定子目录模块的判断? 热更新丢失: rules 包加载规则是否仅在初始化时发生?如果后续 TUI 通过新 API 修改了全局文件,当前常驻的 ReAct 循环或 Gateway 能否在下一轮交互中动态感知并重载?
检查 store.go 中 API 的并发安全性。 确认非 UTF-8 校验逻辑是否严谨。 对于核心逻辑,提供优化建议或指出潜在的内存泄漏风险。 输出要求: |
Blocker (阻碍合并)
func commitWithRetry(tempPath, path string) error {
var last error
for i := 0; i < 5; i++ {
if err := os.Rename(tempPath, path); err == nil {
return nil
} else {
last = err
}
time.Sleep(time.Duration(25*(1<<i)) * time.Millisecond)
}
return fmt.Errorf("rules: commit file %s: %w", path, last)
}
func truncateRuleMarkdown(s string, max int) (string, bool) {
trimmed, truncated := truncateRunes(s, max)
if !truncated {
return trimmed, false
}
if strings.Count(trimmed, "```")%2 == 1 {
trimmed += "\n```"
}
return strings.TrimRight(trimmed, "\n") + "\n[truncated]\n", true
}Warning (潜在隐患)
Suggestion (优化建议)
func resolveBaseDir(baseDir string) string {
if trimmed := strings.TrimSpace(baseDir); trimmed != "" {
return filepath.Clean(trimmed)
}
home, err := os.UserHomeDir()
if err != nil || strings.TrimSpace(home) == "" {
return ""
}
return filepath.Join(home, defaultRulesDir)
}
if !utf8.Valid(data) {
return Document{}, fmt.Errorf("rules: read %s: content is not valid UTF-8", path)
}
简短结论当前实现的热更新路径是通的,但跨平台写入和截断安全性还不够:Windows 原子替换缺少重试,规则截断会破坏 Markdown 结构,而且 system prompt 没有合并后的总预算控制。这三点里前两点我建议按 blocker 处理。 已本地验证: |
@xgopilot 根据优化建议修复问题,并提交PR到Fork仓库中,PR中需要用中文说明具体做了什么修改,收益是什么 |
已完成已按评论里的优化建议完成修复,并推送到 Fork 分支,新增 Fork PR:phantom5099#67 修改内容
收益
验证
提交信息: |
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com>
fix(rules): 收紧规则读写健壮性


1. 原始问题
当前项目里,
AGENTS.md、memo、skills都会影响模型上下文,但它们的语义边界并不清晰,主要体现在:memo容易被拿来承载“默认行为”或“长期约束”,但它本质上只是参考性记忆。AGENTS.md加载逻辑会沿当前workdir向上发现,导致项目规则会随当前路径漂移,不适合作为稳定的项目级规则入口。context内部,导致 prompt 组装层承担了过多文件系统职责。User Memo或每轮重复说明。2. 采取的解决方案
本 PR 采用最小收紧方案,不引入新的规则文件格式,也不引入通用规则平台,而是先把“显式规则”和“参考记忆”拆开,并把规则来源固定到两个自然入口:
AGENTS.md~/.neocode/AGENTS.md围绕这个目标,本 PR 做了四件事:
internal/rules包,专门负责读取规则快照。AGENTS.md”逻辑,改为只认项目根和全局两个规则入口。context中统一渲染## Rulessection,并明确Project Rules > Global Rules > Memo。3. 具体修改范围
3.1 规则加载模型收紧
新增
internal/rules/loader.go,只负责加载两份自然文本规则文件:AGENTS.md~/.neocode/AGENTS.md不再支持:
AGENTS.mdRULE.mdid / name / description等结构化 metadata3.2 Prompt 中显式分层
internal/context中原有的项目规则注入逻辑被替换为统一的Rulessection,结构固定为:Project RulesGlobal Rules同时继续保留
memo注入,但明确其语义为“仅供参考,不视为当前指令”,并把Rules放在Memo之前,避免两者继续处于同一优先级平面。3.3 项目规则解析稳定化
runtime在构建上下文时显式传递稳定的ProjectRoot,让规则解析不再依赖当前活跃子目录。这样即使在同一项目内切换不同文件或子目录,项目规则集合仍然保持稳定一致。3.4 新增规则文件读写入口
新增
internal/rules/store.go,补齐后续上层接入所需的稳定入口:GlobalRulePathProjectRulePathReadGlobalRuleReadProjectRuleWriteGlobalRuleWriteProjectRule这组函数的职责是:
这一步很关键,因为它把“后续 TUI 直接编辑全局规则”所需的底层能力先补齐到了
rules包中,同时没有把路径规则、文件写入逻辑泄漏到 TUI 或 gateway。4. 预期收益
4.1 语义边界更清楚
这次改动把:
Rules定义为显式约束与默认行为Memo定义为偏好、事实和经验的参考信息从而减少“参考信息”和“正式约束”混用的问题。
4.2 项目规则来源更稳定
项目规则现在只认项目根
AGENTS.md,不再随着当前workdir的嵌套位置而漂移,更符合“项目级规则”的直觉。4.3 全局默认行为有正式落点
像“默认中文输出”这样的跨项目规则,后续可以稳定落到全局
AGENTS.md,不必继续依赖User Memo。4.4 context 职责更纯
context现在只负责:Rulessection不再承担复杂的目录遍历与规则发现职责。
4.5 为后续 TUI 接入提前补齐底层能力
虽然本 PR 没有改
TUI和gateway,但已经把后续最需要的“全局规则读写入口”提前补齐。后面如果 TUI 要支持“直接编辑全局规则”,可以直接复用rules包能力,而不是重新实现一套路径拼接和文件写入逻辑。5. 测试与验证
本 PR 补充和调整了以下测试覆盖:
AGENTS.mdprojectRoot为文件路径时回退父目录Rulessection 的渲染顺序Rules在Memo之前注入workdir漂移已验证:
当前覆盖率结果:
internal/rules:69.8%internal/context:95.5%说明:
internal/runtime使用-run TestDoesNotExist做编译型校验,确认ProjectRoot元数据接入没有打断 runtime 主链路。go test ./internal/runtime/...里仍有现存的 Windows symlink 权限测试问题,与本 PR 无关。6. 当前不做的内容
本 PR 明确不做:
TUIgateway/rules命令或 RPCRULE.md这样做的原因是:这次改动的目标是先解决“规则与记忆分层不清”和“缺少全局规则正式入口”这两个核心问题,而不是提前搭建一套复杂规则平台。
7. 后续 gateway / TUI 需要接入的内容
虽然这次没有修改
gateway和TUI,但后续接入方向已经比较明确:7.1 TUI 侧建议补一个“直接编辑全局规则”的入口
TUI 后续应支持:
AGENTS.md底层可直接复用:
GlobalRulePathReadGlobalRuleWriteGlobalRule7.2 TUI 侧建议补一个“查看当前项目规则”的入口
TUI 可以进一步提供:
AGENTS.md底层可直接复用:
ProjectRulePathReadProjectRule另:
7.3 gateway 后续如需透出调试能力,可基于同一套 rules API
如果后续需要把“当前实际生效的规则来源”暴露给 UI 或调试面板,gateway 应继续走已有边界,只透出规则快照或路径信息,而不是在 gateway 中自行实现规则读写逻辑。
8. 总结
这次 PR 的核心不是“新增更多规则能力”,而是用最小改动把规则和记忆从同一语义平面拆开,并把规则入口收紧为用户自然会写的两份
AGENTS.md。在此基础上,再补齐rules包的稳定读写入口,既解决了当前问题,也为后续 TUI 直接编辑全局规则预留了干净的接入点。