Repository navigation
docs(claude): audit the agent workflow against the Claude Code best practices - #1640
Merged
Merged
Conversation
Four project subagents in .claude/agents/, each for a step of the worker loop that otherwise floods the main session's context or needs a fresh pair of eyes: - ci-triage: classifies failed CI jobs (infra / flake / real / known issue) from the job logs and the CI memory notes; read-only. - spec-reviewer: checks a branch against HANDOVER.md, the issue's acceptance criteria, the verification matrix, Phase 1a bug accounting and ECS cross-binding touch-points; reports requirement gaps only. - live-verifier: drives a live editor over the MCP server for one matrix cell or A/B and returns measurements, PNG paths and log checks. - rules-lookup: Haiku lookup of the agent rules that apply to some files or a symptom. .gitignore whitelists .claude/agents/ and .claude/rules/ (the whole .claude/ tree is ignored by default, so neither would have been committed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ripts /start-work, /cleanup-worktree and /resume-worktrees move from .claude/commands/ to .claude/skills/<name>/SKILL.md with disable-model-invocation: true: they create and delete worktrees, post issue comments and kill processes, so only the user starts them. Deterministic steps move out of prose into scripts: - .claude/scripts/worktree_status.py: the one completed/incomplete gate both cleanup and resume use (read-only). It also classifies detached and non-feature/* worktrees, which neither command handled. - start-work/scripts/new-task-worktree.sh: path validation + `worktree add --no-track` from Bash (the PowerShell tool mangles that command's arguments). - resume-worktrees/scripts/open-vscode-folders.ps1: the EnumWindows window detection. /start-work drops from 41 KB to ~15 KB: the model rubric moves to model-rubric.md, the per-worktree memory junction is deleted (Claude Code derives auto memory from the git repository, so worktrees already share the base store; verified with a fresh worktree session), /goal becomes the default kickoff (the old block ran prose into the condition), and the HANDOVER points at CLAUDE.md's guardrails instead of quoting them. /resume-worktrees tells the user to resume the earlier session rather than re-prime from a stale HANDOVER. cleanup-worktree's trap paragraph cited check (b) for what is check (c). The Codex wrappers, docs/README.md and an olo-known-traps advice line follow the move. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ale allow rules The no-Python fallback of the build-lock PreToolUse hook only recognised the legacy OLO_BUILD_LOCK_BYPASS, so a command correctly marked OLO_NOT_A_BUILD or OLO_BUILD_LOCK_OVERRIDE was denied (reproduced by running the fallback directly). It now accepts all three, and its deny message names OLO_NOT_A_BUILD. The allow list loses four rules for the retired D:/repos checkout and a one-off gtest filter, and gains the build-cached test binary (the default preset's tree) and worktree_status.py. _comment names the current markers and notes that the olo-* mods are installed per machine, not enabled by this file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he index by symptom 79 guides were over the house limit of ~10 KB (1.87 MB together; rhi-abstraction-boundary alone was 206 KB), and agents read them in full before touching a subsystem. Each one is now two files: - <name>-details.md: the complete original text, unchanged apart from a back-link line and retargeted anchors (checked by script against HEAD). - <name>.md: the guide's rules, rule first, each linking to its section in the details file; under 10 KB each, ~600 KB for all 79. The index README.md (155 KB) keeps Part A, one sentence per guide (46 KB); Part B moves to failure-modes.md. Every link of both parts is kept. "Adding a doc here" describes the rules/details layout. Inbound links whose anchor moved to a details file are retargeted, and the one contract doc that was split (vulkan-command-ordered-buffer-writes) keeps its details file under the docs-consistency contract-name check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… loop that uses them CLAUDE.md loads into every session and had grown to 247 lines / 14.8 KB, telling every session to read six guides (~220 KB) regardless of subsystem. It is now 185 lines / 10.7 KB: the per-subsystem sections move to nine path-scoped rules in .claude/rules/ (shaders, renderer verification, ECS components, tests, gameplay scheduler, assets, dependencies, docs, Claude configuration) that load only when a matching file is touched. It gains a compaction section, and its test command points at the default build-cached tree. "For other agents" moves to AGENTS.md, which Claude Code does not read when CLAUDE.md exists; AGENTS.md now also tells other agents to read the matching .claude/rules files. task-loop.md: - Phase 0 no longer re-reads CLAUDE.md or the whole index; it asks rules-lookup, and it proves the olo-* mods are loaded with a positive control (they were never installed on the maintainer box for ten days while the docs described them as enforcing). - Phase 2's build snippet went around build-lock (the hook blocks it), used the msvc tree and wrote to /tmp; it now goes through the lock on build-cached and checks the log. - Phases 2b, 3 and 5b hand capture, spec review and CI log triage to the new subagents. - Turns normally run under the /goal kickoff; MEMORY.md guidance names its 200-line / 25 KB load limit; the memory-junction text is gone. claude-code-mods.md gives the full per-machine install and how to verify it, and records that the mods fire inside subagents too (verified with a refusal and an output-truth note). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 201 files, which is 101 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
📒 Files selected for processing: 201
You can disable this status message by setting the
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings the agent workflow (task picking, worktrees, the task loop, CLAUDE.md and the agent-rules corpus) in line with Anthropic's Claude Code best practices and the memory, skills, subagents, worktrees and
/goalpages behind it. The theme is context cost: a worker session used to be told to read CLAUDE.md, a 155 KB index and six guides totalling ~220 KB before starting.What changed
Workflow commands → manual-only skills (
.claude/skills/{start-work,cleanup-worktree,resume-worktrees}), each withdisable-model-invocation: truebecause they create/delete worktrees, comment on issues and kill processes..claude/scripts/worktree_status.py(one read-only completed/incomplete gate shared by cleanup and resume, now also covering detached and non-feature/*worktrees),start-work/scripts/new-task-worktree.sh,resume-worktrees/scripts/open-vscode-folders.ps1./start-work41 KB → ~15 KB; the model rubric moves tomodel-rubric.md./goalis the default kickoff;/resume-worktreesresumes the earlier session instead of re-priming from a stale HANDOVER.Subagents (
.claude/agents/), each wired into its task-loop phase:ci-triage(5b),spec-reviewer(3, requirement gaps only),live-verifier(2b),rules-lookup(0).Context budget
.claude/rules/that load only when a matching file is touched. Adds compaction instructions. For other agents moves to AGENTS.md.docs/agent-rules/README.md155 KB → 46 KB (one sentence per guide); Part B moves tofailure-modes.md.<name>.mdis the rules (≤10 KB each, ~600 KB total), each linking to its section in<name>-details.md, which is the original text unchanged.task-loop.md: Phase 0 proves the
olo-*mods are loaded; the Phase 2 build snippet goes through build-lock onbuild-cached; phases hand capture, spec review and CI triage to the subagents.Found and fixed
OLO_NOT_A_BUILD/OLO_BUILD_LOCK_OVERRIDE. Reproduced by running the fallback; now accepts all three markers./start-work§5c/goalblock ran prose into the goal condition; cleanup-worktree's trap paragraph cited check (b) for check (c); neither cleanup nor resume could classify a detached or bot-branch worktree./tmp; CLAUDE.md's test command pointed at the msvc tree..gitignoreignores.claude/*except a whitelist, so.claude/rules/and.claude/agents/would never have been committed.olo-*mods had never been installed on the maintainer box since they landed (2026-10-10), so none of their guards ran. Installed; Phase 0 andclaude-code-mods.mdnow say how to verify.Filed, not fixed
None.
ci-triage's test run found three deterministic failures in the AMD nightly; they were already tracked, so the triage went to #1516 as a comment (fixing them needs the AMD runner).Review guide
Where I'd look hardest
.claude/rules/*.md: thepaths:globs decide when each rule loads. Each glob was checked to match real files; a wrong one fails silently..claude/scripts/worktree_status.py: it decides what/cleanup-worktreedeletes. A gh query failure raises rather than reading as "no PR".docs/agent-rules/<name>.mdrule files were written by Sonnet subagents from the details files under a fixed brief. Content cannot be lost (the details files are byte-identical to the originals apart from a back-link and retargeted anchors), but a rule summary can be worded worse than its source.What I verified, and how
worktree_status.pyagainst the four live worktrees: classified the dirtyfeature/*at master's commit asincomplete-dirty(the trap), the detached baseline ascompleted, the renovate branch asincomplete-open-pr.new-task-worktree.shend to end on a throwaway slug (created with no upstream, then removed); its guards refuse a bad slug and an existing path.claude -psession in this worktree, with no junction: its auto-memory dir was the base store andMEMORY.mdwas loaded; all four agents load; the three workflow skills are hidden from the model.ci-triageon PR chore(deps): update ocornut/imgui digest to d88f238 #1639 (correctly found no current-head failures) and on run 39351974689 (three real failures with quoted log lines).olo-command-guardsrefusal andolo-output-truthannotation both reach an Agent-tool subagent.cmake --builddenied.claude plugin test .claude/mods/olo-known-traps: 4/4.git show HEAD:<name>.md; all 79 rule files are ≤10,000 bytes and end with the details link; README/failure-modes keep every link of the old index (multiset compare).pre-commit run --all-filespasses, includingdocs-consistencywith 0 errors.Verification matrix: single path. This changes agent configuration and documentation only; no engine, shader or build code.
Least confident about
spec-reviewer,live-verifierandrules-lookupwere checked to load but not exercised on a real task; their first uses in a worker session are the test.live-verifieralso has not yet confirmed thatolo-evidencerecords captures made inside a subagent (the mod mechanism is the same one verified above).🤖 Generated with Claude Code