Expand CONTRIBUTING setup guide and Codex workflow helpers#1455
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR updates CONTRIBUTING.md with a detailed development setup and verification flow, adds a ship-and-babysit runbook for PR babysitting, and special-cases codex/cursor agent handoffs in the work scripts. ChangesContributing Guide Enhancement
Codex Agent Workflow & Integration
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GitHub as GitHub
participant CI as CI
participant CodeRabbit as CodeRabbit
Dev->>GitHub: push branch to origin
Dev->>GitHub: open PR against main
GitHub->>CI: trigger checks
CI-->>GitHub: status updates (pass/fail)
GitHub->>CodeRabbit: surface review threads/comments
Dev->>GitHub: push fixes
GitHub->>CI: re-run checks
CI-->>CodeRabbit: updated statuses
CodeRabbit-->>GitHub: review threads resolved
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
CONTRIBUTING.md (1)
124-129: ⚡ Quick winPrefer debug wrappers first for iterative test/check examples
This guide still foregrounds direct
cargo/Vitest-style commands in the main verification matrix. Suggest makingpnpm debug ...the first-class iterative path and keeping direct commands explicitly as CI-parity/manual alternatives.Suggested doc adjustment
-If setup is correct, these commands should all succeed: +If setup is correct, these CI-parity commands should all succeed: @@ -| Frontend unit tests | `pnpm test` or `pnpm test:coverage` | Vitest in `app/`. | +| Frontend unit tests (iterating) | `pnpm debug unit ...` | Preferred wrapper for bounded logs during local iteration. | +| Frontend unit tests (CI parity) | `pnpm test:coverage` | Required for changed-line coverage enforcement. | @@ -| Rust tests | `pnpm test:rust` | Uses the shared mock backend wrapper. | +| Rust tests (iterating) | `pnpm debug rust ...` | Preferred wrapper for focused local iteration. | +| Rust tests (CI parity) | `pnpm test:rust` | Coverage/CI-aligned Rust test path. |As per coding guidelines, “Use agent debug runners (
scripts/debug/wrappers) instead of invoking Vitest, WDIO, or cargo directly when iterating.”Also applies to: 140-145
🤖 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 `@CONTRIBUTING.md` around lines 124 - 129, The verification matrix currently lists direct commands (pnpm typecheck, pnpm lint, pnpm format:check, cargo check ...) as the primary iterative path; change it to prefer the debug-wrapper approach by placing the pnpm debug scripts (e.g., scripts/debug/* and a generic "pnpm debug <target>") first, and then keep the direct commands (pnpm typecheck, pnpm lint, pnpm format:check, cargo check --manifest-path ...) as explicit CI-parity/manual alternatives; apply the same reorder and wording change to the second occurrence referenced around lines 140-145 and add a short note recommending "Use agent debug runners (scripts/debug/) for iterative work, use direct cargo/Vitest commands only for CI/manual parity.".codex/commands/ship-and-babysit.md (1)
32-33: ⚡ Quick winRequire exact failing command and error output when using
--no-verify.At Line 32-33, the exception is too loose. Require the precise blocked command and stderr snippet in the PR body before allowing bypass, so validation claims stay auditable.
Suggested wording
-3. If the pre-push hook fails on unrelated pre-existing breakage, push with `--no-verify` and record that explicitly in the PR body. If the hook fails on your own changes, fix the problem and push again. +3. If the pre-push hook fails on unrelated pre-existing breakage, include the exact blocked command and error output in the PR body, then push with `--no-verify`. If the hook fails on your own changes, fix the problem and push again.As per coding guidelines, "If a command cannot run in the remote environment, report the exact blocked command and error instead of claiming validation passed."
🤖 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 @.codex/commands/ship-and-babysit.md around lines 32 - 33, Update the guidance sentence that permits pushing with --no-verify when pre-push hooks fail so it requires the exact failing command and a stderr snippet to be recorded in the PR body; specifically replace or augment the line that currently reads "If the pre-push hook fails on unrelated pre-existing breakage, push with `--no-verify` and record that explicitly in the PR body" to mandate including the precise blocked command and the error output (stderr) in the PR body before allowing bypass, and ensure the text distinguishes when the failure is from your own changes vs unrelated pre-existing breakage.
🤖 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 `@CONTRIBUTING.md`:
- Line 131: The sentence "If you only changed docs, you usually only need `pnpm
format:check`" is too broad; update it to scope docs-only validation to
non-AI-authored PRs and explicitly state that AI-authored or remote-agent PRs
must still follow the Codex checklist (referencing the codex PR checklist) and
either run the required checklist commands or report the exact blocked command
and error instead of claiming validation passed; replace the original line in
CONTRIBUTING.md with a clarified sentence that (1) limits `pnpm format:check`
guidance to regular docs-only changes and (2) adds a short note that
Codex/remote-agent PRs must follow the Codex checklist and must report blocked
commands/errors when commands cannot run.
In `@scripts/work/start.sh`:
- Around line 144-146: The current branch unconditionally calls codex exec
--dangerously-bypass-approvals-and-sandbox with user-controlled "$prompt" when
agent == "codex", creating a prompt-injection risk; modify start.sh to require
an explicit opt-in flag or environment variable (e.g., CHECK for
ALLOW_DANGEROUS_CODEX or a --allow-dangerous-codex CLI flag) before running the
dangerous command, and if the opt-in is not present fall back to a safe codex
invocation or exit with an error; reference the agent variable, the prompt
variable, and the codex exec --dangerously-bypass-approvals-and-sandbox
invocation when making this change.
---
Nitpick comments:
In @.codex/commands/ship-and-babysit.md:
- Around line 32-33: Update the guidance sentence that permits pushing with
--no-verify when pre-push hooks fail so it requires the exact failing command
and a stderr snippet to be recorded in the PR body; specifically replace or
augment the line that currently reads "If the pre-push hook fails on unrelated
pre-existing breakage, push with `--no-verify` and record that explicitly in the
PR body" to mandate including the precise blocked command and the error output
(stderr) in the PR body before allowing bypass, and ensure the text
distinguishes when the failure is from your own changes vs unrelated
pre-existing breakage.
In `@CONTRIBUTING.md`:
- Around line 124-129: The verification matrix currently lists direct commands
(pnpm typecheck, pnpm lint, pnpm format:check, cargo check ...) as the primary
iterative path; change it to prefer the debug-wrapper approach by placing the
pnpm debug scripts (e.g., scripts/debug/* and a generic "pnpm debug <target>")
first, and then keep the direct commands (pnpm typecheck, pnpm lint, pnpm
format:check, cargo check --manifest-path ...) as explicit CI-parity/manual
alternatives; apply the same reorder and wording change to the second occurrence
referenced around lines 140-145 and add a short note recommending "Use agent
debug runners (scripts/debug/) for iterative work, use direct cargo/Vitest
commands only for CI/manual parity."
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c141b53-1504-4927-9e21-8ab32200cdff
📒 Files selected for processing (5)
.codex/commands/ship-and-babysit.mdCONTRIBUTING.mdscripts/work/README.mdscripts/work/cli.shscripts/work/start.sh
Summary
CONTRIBUTING.mdinto a full development setup guide with prerequisites, clone/install, env configuration, run modes, validation commands, and local data locationsdevelopvsmain, skills submodule language, and the currentopenhuman-core/ Tauri workflowpnpm work --agent codexbehavior and add a Codex-side/ship-and-babysitcommand prompt under.codex/commands/Problem
CONTRIBUTING.mdrequired contributors to cross-reference multiple files to get a working local setup.pnpm workdid not launch Codex in the requested yolo mode, and there was no repo-local Codex equivalent of the Claude ship-and-babysit command.Solution
CONTRIBUTING.mdusing current source-of-truth files (package.json,app/package.json,Cargo.toml,rust-toolchain.toml, and the env templates)scripts/workso--agent codexusescodex exec --dangerously-bypass-approvals-and-sandbox, documented that behavior, and added.codex/commands/ship-and-babysit.mdSubmission Checklist
docs/TEST-COVERAGE-MATRIX.mdwere affected by this contributor/tooling updateCloses #1441in the## RelatedsectionImpact
pnpm work --agent codexnow starts Codex in the requested no-approval mode, and the repo has a matching Codex ship-and-babysit prompt.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
issue/1441-expand-contributing-md-with-hermes-style1ff1f7c5cfc24c6be5c32433723a061a023e51aaValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckN/A: docs and shell workflow change only; validated with bash -n scripts/work/cli.sh scripts/work/start.shN/A: no Rust source files changed; rust fmt checks ran via pnpm format:checkN/A: no Tauri source files changed; rust fmt checks ran via pnpm format:checkValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
pnpm work --agent codexnow launches Codex throughcodex exec --dangerously-bypass-approvals-and-sandbox;.codex/commands/ship-and-babysit.mdadds a repo-local Codex babysit prompt.Parity Contract
pnpm work --agent <tool>launches are unchanged.scripts/work/start.shonly special-cases the literalcodexagent name; all other agent invocations keep the existing direct<tool> "<prompt>"handoff.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Documentation
Chores