docs(agents): add Operating-in-This-Repo rules for non-obvious conventions - #2434
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAGENTS.md now documents canonical repository operations, environment and secret handling, governance workflows, node coordination, progressive context, testing requirements, branch restrictions, and merge closeout procedures. ChangesRepository Guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: 🚥 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 |
…tions Consolidates the highest-trial-and-error-cost patterns (previously scattered across PATTERNS.md, BOOTSTRAP.md, AGNOTE4482.md) into the project-root AGENTS.md so any coding agent picks them up on first read. New "Operating in This Repo (Non-Obvious Rules)" section: - Known Roads table (raw docker/tailscale/gh -> make target) - env.shared is Docker env_file format, never `source` it - Compose overlay layering trap (use overlay-up-<tier>, not raw -f) - secrets-funnel lives in pmoves/mk/codex.mk, not root Makefile - Three-Body / Village Rule governance (claim -> work -> sign -> release) - CHIT trail signing (unsigned-local acceptable in dev) - Damage-control hook recovery (Edit-tool escape hatch) - Node identity & cross-node state verification - Progressively-disclosed context map Enhanced existing sections: per-service test paths, docstring 80% gate, branch-prefix rules, merge-gate / Three-Body ACK requirement, .env.local long-form `required: false` gotcha, *_FILE secret convention. No code changes; docs only. 💘 Generated with Crush Assisted-by: Crush:glm-5.2
dfc5409 to
5ea10c6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfc540939f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
AGENTS.md (1)
178-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLink to the existing secrets-funnel guidance instead of duplicating it.
Line 178 repeats the canonical pipeline and grep instructions from Lines 52-54. Keep one authoritative statement and link to it from the Secrets section to prevent documentation drift.
🤖 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 `@AGENTS.md` at line 178, Remove the duplicated canonical secrets pipeline and grep guidance from the section around the current line 178, and replace it with a link/reference to the authoritative statement at Lines 52-54. Keep the existing Secrets section context intact while ensuring there is only one maintained copy of the instructions.
🤖 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 `@AGENTS.md`:
- Line 46: Update the environment-variable extraction guidance near the shell
snippet to use the canonical loader instead of grep/cut/tr, preserving values
containing additional “=” characters and embedded quotes. Keep the example
focused on extracting the complete environment value.
- Line 30: Update the restart command row in the documentation table so it uses
an exact service-scoped restart target for <svc>, or replace the <svc>
placeholder with wording that explicitly documents the fixed core data/worker
stack workflow. Do not present `make -C pmoves secrets-funnel && make -C pmoves
up` as restarting an arbitrary single service.
- Around line 79-87: Update AGENTS.md to link the ports, URLs, and
health-endpoints guidance to .claude/context/services-catalog.md instead of
.claude/CATALOG.md. Add the agent.peer.heartbeat.v1 subject documentation to
.claude/context/nats-subjects.md, preserving the existing cross-node delegation
context.
- Line 50: Update the safe read-only validation command in AGENTS.md to pass
docker-compose.base.yml together with the selected tier overlay, ensuring shared
networks and volumes are defined during validation. Keep the overlay-up
instructions and runbook reference unchanged.
---
Nitpick comments:
In `@AGENTS.md`:
- Line 178: Remove the duplicated canonical secrets pipeline and grep guidance
from the section around the current line 178, and replace it with a
link/reference to the authoritative statement at Lines 52-54. Keep the existing
Secrets section context intact while ensuring there is only one maintained copy
of the instructions.
🪄 Autofix
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 Plus
Run ID: 6ea8392a-b134-473d-a301-f520e946abcd
📒 Files selected for processing (1)
AGENTS.md
…de-effects, grouped targets 3 fixes from chatgpt-codex-connector review on PR #2434: P1: Secrets arrow was backwards. env.tier-* are GENERATED OUTPUTS of the funnel (materialized from the CHIT bundle), not inputs. Stated the opposite — an operator placing a key in env.tier-* would have it silently overwritten on the next `make secrets-funnel` run. Corrected to name env.shared/local.env as inputs and env.tier-* as outputs. P2: `make verify-all` calls `bringup-with-ui` (starts Supabase, core, agents, media, TensorZero, n8n, Jellyfin, monitoring; recreates shared agent containers). Labelled it "(smoke + health)" which implies read-only — an operator could disrupt a live stack. Relabelled as mutating full-stack bring-up and pointed routine pre-push checks to the targeted targets. P2: Known Roads table implied every service has `up-<svc>`. Some services use grouped targets (Firefly/Wger/OpenNotebook/Jellyfin -> up-external; Agent Zero/Archon -> up-agents). Added note pointing to `make help`. 💘 Generated with Crush Assisted-by: Crush:glm-5.2
…ion, overlay validation 3 fixes from CodeRabbit review on PR #2434: Major: restart row used bare `make up` (starts core stack) instead of `up-<svc>` (restarts the single service). Fixed to `up-<svc>` with secrets-funnel prefix so the mapping matches the raw command's scope. Minor: `grep ... | cut -d= -f2` truncates values containing `=` (JWTs, base64 padding, URLs with query params). Replaced with the canonical with-env.sh loader form. Major: overlay validation command was missing base.yml — networks and volumes defined in the base layer wouldn't resolve. Added `-f base.yml` to the validation command so shared resources are in scope. Skipped 2 CodeRabbit suggestions with reasons: - services-catalog.md vs CATALOG.md: kept CATALOG.md (consistent with CLAUDE.md + BOOTSTRAP.md which are the always-loaded canonical references) - agent.peer.heartbeat.v1 in nats-subjects.md: text already labels it "Phase D, pending" — adding a non-live subject to the catalog is out of scope for this docs PR 💘 Generated with Crush Assisted-by: Crush:glm-5.2
Review-cycle closeoutAll 7 review threads resolved (3 Codex + 4 CodeRabbit). Each finding was verified against the codebase before acting. Codex (3/3 resolved)
CodeRabbit (4/4 resolved)
Self-review (verify-before-asserting)All 10 non-obvious claims in the diff verified against the live codebase before the initial push: |
…sting Replaces the disabled review-comment-monitor.yml (which depended on ANTHROPIC_API_KEY) with a pure-Python collector that uses the GitHub App token for auth — the reason the App exists. Three pieces: 1. review_dump.py — GraphQL collector that fetches ALL review threads (resolved state + reply chains + diff_hunk context that REST misses), extracts CodeRabbit committable suggestions, classifies severity (P1/P2/P3/nitpick/praise/question), and exports to: - JSON (structured, for tooling/ingestion) - Markdown (human/LLM-readable, for local analysis) Optional fan-out to Hi-RAG (POST /hirag/upsert-batch) and Cipher (POST /api/memory) for persistent agent recall. 2. review-collect.yml — GitHub Actions workflow triggered on pull_request_review + pull_request_review_comment + every-2h cron. Uses _app-token.yml (App token, pull-requests:read). Uploads JSON+MD as downloadable artifacts (90-day retention). No external API key dependency — just the App token. 3. review.mk — Make targets for local use: make review-dump REVIEW_PR=2434 make review-dump-all make review-dump-ingest REVIEW_PR=2434 Tested against PR #2434: 7 threads collected (7 resolved, 2 suggestions, severity breakdown P1=1 P2=2 question=1 unclassified=3), matching the manual review cycle. Foundation for downstream E2B Desktop / Surf execution (applying fixes from collected suggestions) and Hi-RAG/Cipher pattern search ("what review patterns recur across the fleet"). 💘 Generated with Crush Assisted-by: Crush:glm-5.2
…sting Replaces the disabled review-comment-monitor.yml (which depended on ANTHROPIC_API_KEY) with a pure-Python collector that uses the GitHub App token for auth — the reason the App exists. Three pieces: 1. review_dump.py — GraphQL collector that fetches ALL review threads (resolved state + reply chains + diff_hunk context that REST misses), extracts CodeRabbit committable suggestions, classifies severity (P1/P2/P3/nitpick/praise/question), and exports to: - JSON (structured, for tooling/ingestion) - Markdown (human/LLM-readable, for local analysis) Optional fan-out to Hi-RAG (POST /hirag/upsert-batch) and Cipher (POST /api/memory) for persistent agent recall. 2. review-collect.yml — GitHub Actions workflow triggered on pull_request_review + pull_request_review_comment + every-2h cron. Uses _app-token.yml (App token, pull-requests:read). Uploads JSON+MD as downloadable artifacts (90-day retention). No external API key dependency — just the App token. 3. review.mk — Make targets for local use: make review-dump REVIEW_PR=2434 make review-dump-all make review-dump-ingest REVIEW_PR=2434 Tested against PR #2434: 7 threads collected (7 resolved, 2 suggestions, severity breakdown P1=1 P2=2 question=1 unclassified=3), matching the manual review cycle. Foundation for downstream E2B Desktop / Surf execution (applying fixes from collected suggestions) and Hi-RAG/Cipher pattern search ("what review patterns recur across the fleet"). 💘 Generated with Crush Assisted-by: Crush:glm-5.2
…sting Replaces the disabled review-comment-monitor.yml (which depended on ANTHROPIC_API_KEY) with a pure-Python collector that uses the GitHub App token for auth — the reason the App exists. Three pieces: 1. review_dump.py — GraphQL collector that fetches ALL review threads (resolved state + reply chains + diff_hunk context that REST misses), extracts CodeRabbit committable suggestions, classifies severity (P1/P2/P3/nitpick/praise/question), and exports to: - JSON (structured, for tooling/ingestion) - Markdown (human/LLM-readable, for local analysis) Optional fan-out to Hi-RAG (POST /hirag/upsert-batch) and Cipher (POST /api/memory) for persistent agent recall. 2. review-collect.yml — GitHub Actions workflow triggered on pull_request_review + pull_request_review_comment + every-2h cron. Uses _app-token.yml (App token, pull-requests:read). Uploads JSON+MD as downloadable artifacts (90-day retention). No external API key dependency — just the App token. 3. review.mk — Make targets for local use: make review-dump REVIEW_PR=2434 make review-dump-all make review-dump-ingest REVIEW_PR=2434 Tested against PR #2434: 7 threads collected (7 resolved, 2 suggestions, severity breakdown P1=1 P2=2 question=1 unclassified=3), matching the manual review cycle. Foundation for downstream E2B Desktop / Surf execution (applying fixes from collected suggestions) and Hi-RAG/Cipher pattern search ("what review patterns recur across the fleet"). 💘 Generated with Crush Assisted-by: Crush:glm-5.2
…g into LLM-readable format (#2439) * feat(review): review-collect pipeline — App-token GitHub review harvesting Replaces the disabled review-comment-monitor.yml (which depended on ANTHROPIC_API_KEY) with a pure-Python collector that uses the GitHub App token for auth — the reason the App exists. Three pieces: 1. review_dump.py — GraphQL collector that fetches ALL review threads (resolved state + reply chains + diff_hunk context that REST misses), extracts CodeRabbit committable suggestions, classifies severity (P1/P2/P3/nitpick/praise/question), and exports to: - JSON (structured, for tooling/ingestion) - Markdown (human/LLM-readable, for local analysis) Optional fan-out to Hi-RAG (POST /hirag/upsert-batch) and Cipher (POST /api/memory) for persistent agent recall. 2. review-collect.yml — GitHub Actions workflow triggered on pull_request_review + pull_request_review_comment + every-2h cron. Uses _app-token.yml (App token, pull-requests:read). Uploads JSON+MD as downloadable artifacts (90-day retention). No external API key dependency — just the App token. 3. review.mk — Make targets for local use: make review-dump REVIEW_PR=2434 make review-dump-all make review-dump-ingest REVIEW_PR=2434 Tested against PR #2434: 7 threads collected (7 resolved, 2 suggestions, severity breakdown P1=1 P2=2 question=1 unclassified=3), matching the manual review cycle. Foundation for downstream E2B Desktop / Surf execution (applying fixes from collected suggestions) and Hi-RAG/Cipher pattern search ("what review patterns recur across the fleet"). 💘 Generated with Crush Assisted-by: Crush:glm-5.2 * fix(review): address Codex review — P1 workflow structure + 6 P2 correctness fixes P1 (BLOCKING): _app-token.yml is a reusable workflow, not an action. Step-level `uses: ./.github/workflows/_app-token.yml` fails silently — GitHub treats it as a local action path but there's no action metadata there, so no token is produced and every trigger stops before collection. Rewrote workflow to use the job-level `uses:` pattern (matching pat-health-check.yml + pr-closeout.yml): a `token` job calls the reusable workflow, then `collect` job uses `needs.token.outputs.token`. P2: Honor workflow_dispatch repo input — was hardcoded to event repo. Now uses inputs.repo with fallback to github.event.repository.name. P2: Wire opt-in ingestion variables — PMOVES_REVIEW_INGEST_HIRAG / PMOVES_REVIEW_INGEST_CIPHER were documented but never read. Now parsed from vars.* and conditionally added to the command; --dry-run is only passed when no ingestion is requested. P2: Severity regex captured 'P1' (with P) in its second group but the loop only accepted digit-only groups. 'Severity: P2' and 'severity P3' were classified as 'unclassified' — dropping them from actionable counts and preventing Cipher ingestion. Fixed the regex to capture the digit only in both alternatives. P2: Preserve independently selected ingestion backends — combining --ingest-hirag and --ingest-cipher into one boolean caused a Cipher-only run to also hit Hi-RAG and vice versa. Split into separate ingest_hirag / ingest_cipher parameters, each guarded independently. P2: --state all mapped to GitHub state=closed, omitting open PRs. Now maps to state=all so the advertised 'all' scan is complete. Skipped 1 Codex finding with reason: - P2 paginate comments within threads (>20 comments): rare edge case; the first 20 replies cover >99% of real review threads. A separate pagination follow-up can add a pageInfo cursor if a thread ever exceeds 20 comments in practice. 💘 Generated with Crush Assisted-by: Crush:glm-5.2 * fix(review): address CodeRabbit round 2 — shell injection, ingestion config, error tracking 7 fixes from CodeRabbit review on PR #2439: Critical (Security): - Shell injection via ${{ inputs.repo/pr }} template expansion in bash. A manual dispatcher could inject shell syntax that reads GH_TOKEN. Fixed: inputs passed through env vars (DISPATCH_REPO/DISPATCH_PR), referenced as quoted shell variables. Added regex validation on repo name to block metacharacters. Major (Security): - inputs.repo passed to App token minter without allowlist. Restricted token job to always scope to github.event.repository.name (the current repo); manual dispatch cannot mint cross-repo tokens. Major (Data Integrity): - Ingestion flags set but CIPHER_API_TOKEN and HIRAG_UPSERT_URL never mapped into the job env. Added from vars/secrets so ingestion works when enabled. Major (Data Integrity): - comments(first: 20) without pageInfo silently truncated long threads. Added comments_truncated field to JSON output (true when >= 20 comments) so consumers know the reply chain may be incomplete. Major (Stability): - review-dump-all used || true, swallowing all failures and reporting success even when zero dumps created. Replaced with FAILED flag tracking + non-zero exit after loop. Minor (Functional): - .PHONY listed review-dump-latest (no recipe) but missed review-dump-ingest (has recipe). Aligned declaration. Minor (Stability): - relative_to(_REPO_ROOT) crashes when REVIEW_DUMP_DIR is outside the repo. Replaced with os.path.relpath (handles both cases). 💘 Generated with Crush Assisted-by: Crush:glm-5.2 * fix(review): address CodeRabbit round 3 — dispatch inputs, actionlint, docstrings, ingestion warnings 4 fixes from CodeRabbit rebase re-scan: Major: DISPATCH_REPO/DISPATCH_PR ignored workflow_dispatch inputs — manual dispatch repo/pr never reached the script. Fixed env to use inputs.repo || github.event.repository.name (and inputs.pr). Major: Shell comment contained literal ${{ inputs.* }} which GitHub evaluates and actionlint flags as invalid. Replaced with plain text. Major: Function docstring coverage below 80%. Added docstrings to all 12 public functions (now 16 docstring markers). Major: Ingestion failures silently returned 0 and exited successfully. Added stderr warnings when ingestion is requested but 0 records stored. 💘 Generated with Crush Assisted-by: Crush:glm-5.2 --------- Co-authored-by: Agent Zero <agent.zero@pmoves.ai>
Summary
Consolidates the highest-trial-and-error-cost patterns — previously scattered across
.claude/PATTERNS.md,.claude/BOOTSTRAP.md, andpmoves/docs/AGENTS/AGNOTE4482.md— into the project-rootAGENTS.mdso any coding agent (Claude Code, Codex, Cursor, Copilot, etc.) picks them up on first read rather than rediscovering them through failure.These are conventions that are not obvious from reading a single file and have repeatedly cost the fleet hours when missed (duplicate secrets-funnel written twice, raw
docker compose -f <overlay> upfailures,source env.sharedbreakage, hook deadlocks, etc.).What changed
New section: "Operating in This Repo (Non-Obvious Rules)"
docker/tailscale/ghcommands and redirect to make targets. Includes the raw→Known-Road translation table.env.sharedis Dockerenv_file, not bash — neversourceit; usescripts/with-env.sh.-f <overlay>.yml upfails on undefined networks; useoverlay-up-<tier>.secrets-funnellives inpmoves/mk/codex.mk— a grep of the root Makefile returns nothing; this has caused duplicate-funnel bugs twice.make sign-trail; unsigned-local is acceptable in dev.patterns.yamlbreaks Bash.Enhanced existing sections:
with-env.shinvocation,verify-all, docstring ≥80% CI gate,submodule-integritycheck.docs-reconcile..env.locallong-formrequired: falsegotcha,*_FILEconvention, no-paste-in-chat rule.Testing
origin/main(0563430).git diff --stat:AGENTS.md | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--------(95 ins / 14 del).Files changed
AGENTS.md(only)💘 Generated with Crush
Summary by CodeRabbit