docs: improve AI agent instructions and dev workflow guidance [skip ci] (#8284) [skip ci] - #8284
Conversation
- Add allow permissions to .claude/settings.json for curl (github.com, raw.githubusercontent.com, buildkite.com), read-only gh subcommands (issue/pr/run list/view/checks/diff/status), make, go test, gofmt, ls - Add upstream/main comparison guidance to CLAUDE.md and AGENTS.md: prefer upstream/main if remote exists, fall back to origin/main Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Make explicit that the PreToolUse hook runs `make staticrequired` automatically before every `git commit`, and that gofmt/markdownlint run via PostToolUse hooks after edits. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Allow common read-only commands: cat, head, tail, wc, sort, uniq, cut, tr, diff, find, which, type, echo, printf, jq, awk, grep, sed, git read subcommands (log/diff/status/show/blame/branch/remote), golangci-lint, markdownlint. Deny destructive variants: sed -i, find -delete, find -exec rm. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ddev.site URLs resolve locally but use external domain names, so the localhost deny rules were unnecessary friction. WebFetch(domain:*) is sufficient. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jonesrussell
left a comment
There was a problem hiding this comment.
Review by Claude Code
Reviewed the three changed files (.claude/settings.json, CLAUDE.md, AGENTS.md). The documentation improvements are solid — notably correcting the inaccurate "PreToolUse and PostToolUse" description to correctly say PostToolUse for gofmt/markdownlint. The PreToolUse hook description for make staticrequired is verified accurate against the actual settings.json config.
A few things worth considering:
curl allow patterns match arbitrary domains
The patterns like Bash(curl*github.com*) will match any curl invocation where github.com appears anywhere in the argument string — including as a query parameter or path fragment on a different host (e.g., curl https://evil.com/?ref=github.com).
Consider anchoring to the scheme + domain:
"Bash(curl https://github.com*)"
"Bash(curl https://raw.githubusercontent.com*)"
"Bash(curl https://buildkite.com*)"
git fetch upstream 2>/dev/null silently swallows real errors
In both CLAUDE.md and AGENTS.md, the pattern:
git fetch upstream 2>/dev/null || git fetch originIf upstream exists but fetch fails for a real reason (expired SSH key, network timeout), stderr is suppressed and the fallback silently fetches origin instead. The agent then diffs against a potentially wrong base with no warning.
Separating remote detection from fetching would be more robust:
if git remote get-url upstream >/dev/null 2>&1; then
git fetch upstream
else
git fetch origin
fiMinor pattern concerns in the allow/deny lists
Bash(tr*)— matchestruncateas well astr. Could silently allowtruncate -s 0 <file>.Bash(git branch*)— matchesgit branch -D(force delete). If the intent is read-only listing, consider denyingBash(git branch -D*)andBash(git branch -d*).Bash(find*-exec rm*)deny — doesn't catchfind -exec /usr/bin/rmorfind -exec unlink. Consider broadening toBash(find*-exec*)if the goal is blocking destructive find operations.
Duplicated section across CLAUDE.md and AGENTS.md
The "Comparing Against Upstream" section is word-for-word identical in both files. The PR body explains why both files exist (different audiences), which makes sense. A small <!-- Keep in sync with AGENTS.md --> comment in each might help prevent drift over time.
Overall this is a useful improvement to the AI agent workflow. The documentation accuracy fixes are a clear win. The permission patterns are a good idea — the suggestions above are about tightening the globs rather than questioning the approach.
|
Think these are probably "good enough" for now. |
The Issue
AI agent instructions in CLAUDE.md and AGENTS.md had gaps: no
allowpermissions for common safe operations, no guidance on usingupstream/mainfor PR comparisons, and themake staticrequiredhook behavior wasn't clearly documented.How This PR Solves The Issue
.claude/settings.json— addallowpermissions:curlto github.com, raw.githubusercontent.com, buildkite.comghsubcommands:issue/pr/runlist, view, checks, diff, statusmake*,go test*,gofmt*,ls*These are safe, frequently used operations that shouldn't require manual approval on every invocation.
CLAUDE.mdandAGENTS.md— upstream/main comparison guidance:When generating diffs for a PR, prefer
upstream/mainas the base if the remote exists, fall back toorigin/main. Localmainmay be stale. Not all developers use anupstreamremote, so the guidance handles both cases gracefully.CLAUDE.md— clarifymake staticrequiredhook behavior:Made explicit that a PreToolUse hook in
.claude/settings.jsonrunsmake staticrequiredautomatically before everygit commit, and thatgofmt/markdownlint --fixrun via PostToolUse hooks after edits.Note on CLAUDE.md vs AGENTS.md:
Claude Code now reads both
CLAUDE.mdandAGENTS.md. We are not consolidating them at this time because AGENTS.md serves other AI tools (Codex, Gemini, etc.) and the two files have meaningfully different audiences. Consolidation would be worth revisiting if the content continues to diverge in maintenance cost.Manual Testing Instructions
make,go test,ls, andgh pr listno longer prompt for approvalcurl https://api.github.com/...no longer prompts for approvalgit commitstill triggersmake staticrequiredvia the hookAutomated Testing Overview
Documentation and config only — no code changes. Markdown linting passes.
Release/Deployment Notes
No impact on DDEV functionality.