feat(agent): inject per-user config.UserConfigDir()/zero/ZERO.md guidelines into system prompt#475
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds per-user ChangesPer-user system prompt guidelines
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/agent/system_prompt.go (1)
277-304: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared guideline reader/truncator
projectGuidelines()anduserGuidelines()both do the same read → trim → empty-check → rune-safe truncate → wrap flow. Pull that into a shared helper so the truncation behavior stays aligned and doesn’t drift between project and user context handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agent/system_prompt.go` around lines 277 - 304, The shared guideline loading logic is duplicated between projectGuidelines() and userGuidelines(), so extract the common read/trim/empty-check/rune-safe truncation/wrap flow into a helper and have both functions call it. Preserve the existing behavior in system_prompt.go by keeping the same maxProjectContextBytes truncation and the same title prefix formatting while centralizing the shared logic to prevent future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/agent/system_prompt.go`:
- Around line 277-304: The shared guideline loading logic is duplicated between
projectGuidelines() and userGuidelines(), so extract the common
read/trim/empty-check/rune-safe truncation/wrap flow into a helper and have both
functions call it. Preserve the existing behavior in system_prompt.go by keeping
the same maxProjectContextBytes truncation and the same title prefix formatting
while centralizing the shared logic to prevent future drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 715866c2-c7f8-450a-bcc0-9f411a71258a
📒 Files selected for processing (2)
internal/agent/system_prompt.gointernal/agent/system_prompt_test.go
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Request changes — genuinely useful feature; one location fix stands between it and merge
Worth merging: yes, once relocated. A user-global ZERO.md guidelines file injected into the system prompt is a real, wanted feature; the code is clean and Windows-safe (I checked the read/truncation bounds — no path-separator, PATHEXT, CRLF, or DACL surface); and it's not a duplicate — there's no pre-existing user-guideline loader. Thanks @euxaristia.
🔴 Blocker — wrong directory. It reads from os.UserHomeDir()/.zero/ZERO.md, but every other user-level config in Zero lives under config.UserConfigDir()/zero/ — commands, specialists, config.json, hooks, plugins. Reading from ~/.zero/ instead means a user who configured Zero the normal way won't get their guidelines honored where all their other config lives (Linux ~/.config/zero, macOS ~/Library/Application Support/zero, Windows %AppData%\Roaming\zero). To be clear, the feature works as documented on every OS — this isn't a silent no-op — but the location diverges from Zero's own convention. Resolve the path via config.UserConfigDir()/zero/ZERO.md and it's consistent.
🟡 Process: no linked parent issue with the issue-approved label — your own PR body notes CONTRIBUTING requires one before merge.
Minor (non-blocking): the project-guideline loader has case-insensitive resolution (TestBuildSystemPromptInjectsProjectGuidelinesCaseInsensitive); userGuidelines() uses a plain os.ReadFile, so on case-sensitive Linux the file must be exactly ZERO.md. Worth matching for consistency.
Fix the location and link an approved issue, and this is a clean merge — the rest is solid.
Review feedback: every other user-level config (config.json, commands, specialists) lives under config.UserConfigDir()/zero, so the per-user guidelines file now resolves there instead of ~/.zero. The basename match is case-insensitive to mirror the project guideline loader. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Pushed 3a58b6a addressing the review: the guidelines file now resolves under config.UserConfigDir()/zero/ZERO.md, next to the rest of the per-user config, and the basename match is case-insensitive to mirror the project guideline loader. PR body updated to match. The linked parent issue is still outstanding. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/agent/system_prompt.go (1)
284-328: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared helpers instead of duplicating the project-guideline truncation/lookup logic.
The rune-boundary truncation block at Lines 305-311 is a verbatim copy of the one in
projectGuidelines(Lines 265-271), andfindUserContextFilereimplements the same case-insensitive-basename-in-directory scan the doc comment says it mirrors for the project loader. Any future fix to the truncation edge cases or lookup semantics now needs to be applied in two places, risking silent divergence.Consider extracting a shared
truncateContent(content string, limit int) stringand a sharedfindCaseInsensitiveFile(dir, basename string) stringused by both the project and user guideline loaders.♻️ Sketch of shared helpers
+func truncateContent(content string, limit int) string { + if len(content) <= limit { + return content + } + cut := limit + for cut > 0 && !utf8.RuneStart(content[cut]) { + cut-- + } + return content[:cut] + "\n… (truncated)" +} + +func findCaseInsensitiveFile(dir, basename string) string { + entries, err := os.ReadDir(dir) + if err != nil { + return "" + } + for _, e := range entries { + if !e.IsDir() && strings.EqualFold(e.Name(), basename) { + return filepath.Join(dir, e.Name()) + } + } + return "" +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agent/system_prompt.go` around lines 284 - 328, The guideline loading logic in userGuidelines duplicates both the rune-safe truncation used by projectGuidelines and the case-insensitive file scan logic from findUserContextFile, so extract shared helpers and use them from both paths. Introduce a reusable truncateContent(content string, limit int) helper for the maxProjectContextBytes trimming behavior, and a shared findCaseInsensitiveFile(dir, basename string) helper that both project and user loaders call instead of keeping separate lookup implementations. Keep the existing userGuidelines and findUserContextFile symbols as thin wrappers or callers so the semantics stay aligned in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/agent/system_prompt.go`:
- Around line 284-328: The guideline loading logic in userGuidelines duplicates
both the rune-safe truncation used by projectGuidelines and the case-insensitive
file scan logic from findUserContextFile, so extract shared helpers and use them
from both paths. Introduce a reusable truncateContent(content string, limit int)
helper for the maxProjectContextBytes trimming behavior, and a shared
findCaseInsensitiveFile(dir, basename string) helper that both project and user
loaders call instead of keeping separate lookup implementations. Keep the
existing userGuidelines and findUserContextFile symbols as thin wrappers or
callers so the semantics stay aligned in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 07896f2b-79fd-47f8-8c61-228c58b23b5a
📒 Files selected for processing (2)
internal/agent/system_prompt.gointernal/agent/system_prompt_test.go
CodeRabbit review feedback: the rune-safe truncation block and the case-insensitive basename scan were duplicated across the project and user guideline loaders. Extract truncateGuidelineContent and findCaseInsensitiveFile and use them in both paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/agent/system_prompt.go`:
- Around line 305-314: The truncateGuidelineContent function can return more
than the requested limit because it appends the truncation marker after slicing.
Update truncateGuidelineContent to reserve space for the marker before choosing
the cut point, then trim to a valid rune boundary and append the marker only
within the remaining budget. Keep the fix localized to truncateGuidelineContent
in system_prompt.go so the guideline size guarantee remains enforced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6cfc858a-eef8-455d-844b-9b3209e911da
📒 Files selected for processing (1)
internal/agent/system_prompt.go
CodeRabbit review feedback: truncateGuidelineContent appended the truncation marker after slicing to the limit, so the result could exceed the requested budget by the marker length. Reserve space for the marker before choosing the cut point. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found a issues that need to be addressed before this is ready.
Findings
- [P3] Update the stale PR title to match the implemented config path
GitHub PR title
The current title still says this injects~/.zero/ZERO.md, but the latest implementation and PR body now resolve the file underconfig.UserConfigDir()/zero/ZERO.md(~/.config/zero/ZERO.mdon Linux/macOS and%AppData%\Roaming\zero\ZERO.mdon Windows). Please update the title so reviewers and release notes do not advertise the old location.
euxaristia
changed the title
|
Status update on the open findings:
Still open: no linked issue with the issue-approved label per CONTRIBUTING. That's a process item for a maintainer to weigh in on, not a code change. @Vasanthdev2004 @jatmn both your CHANGES_REQUESTED predate the fixes above, ready for re-review whenever you get a chance. |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found a few issues that need to be addressed before this is ready.
Findings
-
[P2] Link an approved parent issue before merge
CONTRIBUTING.md:17
The PR body still says there is no linked issue yet, and GitHub currently shows no linked/closing issue for this PR. The contribution policy requires community PRs to link an existing issue that already has theissue-approvedlabel before the PR is merged, so please link the approved parent issue or get an explicit maintainer exception recorded on the PR. -
[P2] Keep project instructions above personal defaults
internal/agent/system_prompt.go:95
userGuidelines()is appended afterworkspaceContext(), which is where the projectAGENTS.md/ZERO.mdcontent is injected. That gives a global personal file the last instruction block after the repo's team guidance, even though the rest of Zero's extension surfaces document project-over-user precedence (AGENTS.mdsays project specialists override user specialists, MCP project config wins on conflicts, and the config layer table puts project config after user config). With the current order, a user's~/.config/zero/ZERO.mdcan contradict a repositoryAGENTS.mdand be the later same-role instruction, so team conventions like build commands, forbidden paths, or sandbox expectations can be accidentally weakened in every repo. Please make the precedence explicit and preserve project/team instructions as the more specific layer, for example by placing user guidelines before workspace/project guidelines or by adding conflict wording that tells the model project guidelines win. -
[P2] Isolate prompt tests from the real user config directory
internal/agent/system_prompt_models_test.go:43
The new user-guidelines loader means any test that callsbuildSystemPrompt(Options{})can read the developer's realconfig.UserConfigDir()/zero/ZERO.md. Several existing tests still do that without stubbinguserConfigDirForPrompt; for example this assertion fails if the developer has<model_guidance>in their personal ZERO.md, and similar prompt tests can now include arbitrary host-local text in their failure output. Please default the prompt tests to an empty temp config directory (or otherwise stubuserConfigDirForPromptfor tests that are not specifically exercising user guidelines) so focused and package-wide runs are deterministic on contributor machines. -
[P3] Keep the truncation helper within its limit for small budgets
internal/agent/system_prompt.go:310
truncateGuidelineContentstill returnstruncationMarkereven whenlimitis smaller than the marker length, so the result can exceed the caller's requested budget. That is reachable fromprojectGuidelines: when earlier guideline files leave only a few bytes ofmaxProjectContextTotalBytes, line 265 passes that small remaining budget into the helper and line 268 then counts a string longer than the remaining cap. Please handle thelimit <= len(truncationMarker)case so the helper's "never exceeds limit" guarantee holds for every caller, and add a small-limit test. -
[P3] Document the new per-user ZERO.md location
AGENTS.md:19
The runtime now loadsconfig.UserConfigDir()/zero/ZERO.md, but the user-facing extension guide still documents only project instruction files (./AGENTS.md,./ZERO.md, and./.zero/AGENTS.md) and the end-to-end example still lists only~/.config/zero/config.jsonand skills as per-user files. Please update the guide so users know where to put personal global instructions and how that file interacts with project guidelines.
…n for user guidelines Address remaining review feedback on PR Gitlawb#475: - truncateGuidelineContent still exceeded the limit when limit was smaller than the truncation marker itself. Fall back to a hard, rune-safe cut with no marker in that case so the result never exceeds the caller's budget. - Reorder buildSystemPrompt so user guidelines are injected before workspace/project guidelines, and add an explicit precedence note in the user guidelines section text, so a repo's AGENTS.md/ZERO.md is the later, more specific instruction and wins on conflict. - Add a TestMain in the agent package that stubs userConfigDirForPrompt to an empty temp directory by default, so buildSystemPrompt(Options{}) calls in unrelated tests don't pick up a contributor's real ~/.config/zero/ZERO.md. - Document the per-user ZERO.md location and its precedence relative to project guidelines in AGENTS.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and found one issue that still needs to be addressed.
Findings
- [P2] Link an approved parent issue before merge
CONTRIBUTING.md:17
The PR body still says there is no linked issue yet, and the current PR timeline/search did not show a linked parent issue with theissue-approvedlabel. The contribution policy requires community PRs to link an existing approved issue before merge, and the current author update also acknowledges this is still open. Please link the approved parent issue or get an explicit maintainer exception recorded on the PR.
|
@Vasanthdev2004 I don't have permission to re-request your review directly, so flagging here: your change-request was on the directory location ( |
|
checking one by one will check urs now @euxaristia |
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Re-reviewing — the location fix is exactly right, so approving.
My blocker was that it read from ~/.zero/ZERO.md while every other per-user thing in Zero (commands, specialists, config.json, hooks, plugins) lives under config.UserConfigDir()/zero/. It now resolves config.UserConfigDir()/zero/ZERO.md, so someone who set Zero up the normal way gets their guidelines honored where the rest of their config already lives. You also picked up the case-insensitive resolution I'd only flagged as a nit — findCaseInsensitiveFile matches the project-guideline loader, so a lowercase zero.md still resolves on case-sensitive Linux.
Precedence reads correctly: the user block goes in earlier/less-specific and explicitly says the project's AGENTS.md/ZERO.md win on conflict, which is the right call — a repo's own rules should beat your global defaults. Still Windows-safe (the read/truncation bounds I checked before are unchanged), and the title matches the implemented path now.
Opened #513 and marked it issue-approved to clear the community-PR gate — a user-global guidelines file is a genuinely wanted feature and two of us have vetted it.
Good to merge — over to kevin.
- Correct project-scope plugin discovery to <cwd>/.zero/plugins, not the repo root. - Clarify that user-scope plugin discovery always resolves under ~/.config regardless of OS, separate from config.UserConfigDir() (which is OS-aware and used for the per-user ZERO.md file). - Document the per-user ZERO.md guidelines file added in Gitlawb#475. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: document AGENTS.md/ZERO.md and plugin manifest format Add an "Extending Zero" section to the README covering the AGENTS.md/ZERO.md project-instructions lookup chain and the plugin.json manifest fields, neither of which were previously documented outside the source. Closes #521 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * docs: fix plugin path scope and add per-user ZERO.md guidelines - Correct project-scope plugin discovery to <cwd>/.zero/plugins, not the repo root. - Clarify that user-scope plugin discovery always resolves under ~/.config regardless of OS, separate from config.UserConfigDir() (which is OS-aware and used for the per-user ZERO.md file). - Document the per-user ZERO.md guidelines file added in #475. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
4 participants
Summary
Inject per-user instructions from
ZERO.mdin the user config directory (config.UserConfigDir()/zero/ZERO.md) into the system prompt, alongside existing project-level guideline files (AGENTS.md,ZERO.md,.zero/AGENTS.md). This lets operators keep personal preferences (tone, tooling, workflow) outside individual repositories while still having them apply to every Zero session.The file resolves to
~/.config/zero/ZERO.mdon Linux and macOS and%AppData%\Roaming\zero\ZERO.mdon Windows, next to the rest of Zero's per-user config (config.json, commands, specialists).Linked issue
No linked issue yet. Per CONTRIBUTING.md, this PR may need a parent issue with the
issue-approvedlabel before merge.Changes
internal/agent/system_prompt.gouserContextFileconstant (ZERO.md) resolved underconfig.UserConfigDir()/zero/, with auserConfigDirForPrompthook for tests.userGuidelines()to read, trim, and cap user instructions (8 KiB, same per-file limit as project docs). The basename match is case-insensitive, mirroring the project guideline loader.buildSystemPromptafter workspace context.internal/agent/system_prompt_test.gozero.mdvariant.userConfigDirForPrompt.Test plan
go test ./internal/agent/... -run "UserGuidelines|BuildSystemPrompt" -count=1upstream/maincleanly.ZERO.mdunder the user config dir with a short preference, start a session, confirm## User guidelines (ZERO.md)appears in the injected system prompt.Summary by CodeRabbit
New Features
~/.config/zero/ZERO.md(case-insensitive), injected as a dedicated User guidelines section.Bug Fixes
Documentation
AGENTS.mdto document personal guidelines discovery and precedence, and refreshed the example file list.Tests