feat: agent quality workflows and worktree ship gates - #40
Conversation
Port Wealthuman-style agent workflows into openfindata: docs/agents map, in-repo ship skill, MCP trust review, preflight evidence, and hooks that keep root/main inspect-only. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 51 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe change adds agent and MCP trust documentation, formalizes the ship workflow, enforces worktree-aware Git hooks, and introduces readiness and preflight quality gates with recorded evidence. ChangesAgent workflow and repository guardrails
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Preflight as scripts/ship/preflight.sh
participant Readiness as readiness.sh
participant Python as Ruff, mypy, and pytest
participant Evidence as Git common-directory evidence
Preflight->>Readiness: Run readiness checks with selected base reference
Readiness-->>Preflight: Return readiness status
Preflight->>Python: Run configured quality gates
Python-->>Preflight: Return validation status
Preflight->>Evidence: Record mode, steps, SHA, path, and UTC timestamp
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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.
Actionable comments posted: 7
🧹 Nitpick comments (5)
.githooks/post-checkout (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGate the warning on the branch-checkout flag.
Git calls
post-checkoutwith three arguments. The third argument is1for a branch checkout and0for a file checkout. The hook currently ignores it, sogit checkout -- <path>andgit worktree addalso print the warnings. Gate the call to reduce noise.♻️ Proposed change
-guardrails_warn_post_checkout +# $3 == 1 marks a branch checkout; skip file checkouts. +if [[ "${3:-1}" == "1" ]]; then + guardrails_warn_post_checkout +fi🤖 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 @.githooks/post-checkout at line 15, Update the post-checkout hook around guardrails_warn_post_checkout to accept Git’s third hook argument and invoke the warning only when that flag equals 1, suppressing warnings for file checkouts and worktree operations.scripts/git/guardrails.sh (3)
236-247: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse an array for staged files to survive paths with spaces.
filesholds newline-separated paths and expands unquoted. Bash splits on spaces and tabs, so a path such assrc/my file.pybecomes two arguments andrufffails or checks the wrong path. Read the list into an array with a NUL delimiter.♻️ Proposed change
- local files - files="$(guardrails_staged_py_files)" - if [[ -z "$files" ]]; then + local files=() + while IFS= read -r -d '' f; do + files+=("$f") + done < <(git diff --cached --name-only -z --diff-filter=ACMR -- '*.py' || true) + if [[ ${`#files`[@]} -eq 0 ]]; then guardrails_log "no staged Python files — skipping Ruff" else guardrails_log "ruff check (staged only)" - # shellcheck disable=SC2086 - "$py" -m ruff check $files + "$py" -m ruff check -- "${files[@]}" guardrails_log "ruff format --check (staged only)" - # shellcheck disable=SC2086 - "$py" -m ruff format --check $files + "$py" -m ruff format --check -- "${files[@]}" fi🤖 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 `@scripts/git/guardrails.sh` around lines 236 - 247, Update the staged-file handling around guardrails_staged_py_files to read paths into a Bash array using a NUL delimiter, preserving filenames containing spaces. Use the array when invoking both ruff check and ruff format --check, while retaining the no-files skip behavior.
282-286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInstall hooks from the shared repository root, not the current checkout.
checkout_rootcomes fromguardrails_current_checkout, so the source is the worktree that runs the installer.core.hooksPathis a repository-level setting shared by every worktree. A worktree on an old or experimental branch therefore installs its own.githooksandguardrails.shfor all worktrees. Useguardrails_repo_rootas the source, or log the source path so the operator sees which branch supplied the hooks.♻️ Proposed change
- checkout_root="$(guardrails_current_checkout)" + checkout_root="$(guardrails_repo_root)" common_git_dir="$(git rev-parse --path-format=absolute --git-common-dir)" install_dir="${common_git_dir}/openfindata-hooks" source_hooks="${checkout_root}/.githooks"The copy model also means the installed hooks become stale after a pull that changes
.githooks/*orscripts/git/guardrails.sh. That risk is already covered by the guideline that requiresbash scripts/git/install-hooks.shafter such a pull. As per coding guidelines: "After a pull or merge that changes.githooks/*orscripts/git/guardrails.sh, runbash scripts/git/install-hooks.shbefore relying on local hooks."🤖 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 `@scripts/git/guardrails.sh` around lines 282 - 286, Update the hook source selection near guardrails_current_checkout so installation copies from guardrails_repo_root, while retaining common_git_dir for the shared install destination. Ensure source_hooks resolves to the repository root’s .githooks directory rather than the active worktree.Source: Coding guidelines
292-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCopy every hook in
.githooksand clear stale files.The installer copies three fixed names. If a hook is added to
.githooks/, the installer silently skips it. If a hook is removed from.githooks/, the stale copy stays ininstall_dirand keeps running. A loop over the source directory fixes both cases.♻️ Proposed change
- mkdir -p "$install_dir" - cp "${source_hooks}/pre-commit" "${install_dir}/pre-commit" - cp "${source_hooks}/pre-push" "${install_dir}/pre-push" - cp "${source_hooks}/post-checkout" "${install_dir}/post-checkout" - cp "${checkout_root}/scripts/git/guardrails.sh" "${install_dir}/guardrails.sh" - chmod +x "${install_dir}/pre-commit" "${install_dir}/pre-push" "${install_dir}/post-checkout" "${install_dir}/guardrails.sh" + rm -rf "$install_dir" + mkdir -p "$install_dir" + local hook + for hook in "${source_hooks}"/*; do + [[ -f "$hook" ]] || continue + cp "$hook" "${install_dir}/$(basename "$hook")" + done + cp "${checkout_root}/scripts/git/guardrails.sh" "${install_dir}/guardrails.sh" + chmod +x "${install_dir}"/*🤖 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 `@scripts/git/guardrails.sh` around lines 292 - 297, Update the installer around install_dir setup to remove stale hook files and copy every hook from source_hooks dynamically, replacing the three fixed cp commands. Preserve installation of guardrails.sh and its executable permissions, while ensuring hooks removed from .githooks no longer remain active.docs/agents/orientation.md (1)
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the approximate tool count from the orientation pointer.
docs/MCP_SURFACE.mdis the canonical catalog. The~25 toolsclaim can become stale when the curated surface changes. Refer to the curated catalog without embedding a count.🤖 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 `@docs/agents/orientation.md` at line 26, Update the orientation table entry for MCP in docs/agents/orientation.md to remove the approximate “~25 tools” count while still directing readers to the curated mcp_app catalog and docs/MCP_SURFACE.md.
🤖 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 @.claude/skills/mcp-trust-reviewer/SKILL.md:
- Around line 31-38: Update the “Limites duros” section of the
mcp-trust-reviewer skill to explicitly prohibit executing repository-controlled
code, including tests, hooks, scripts, MCP servers, and networked commands, as
well as triggering side effects. Keep the reviewer inspection-only unless an
isolated verification procedure is explicitly defined.
In `@CLAUDE.md`:
- Around line 76-78: Define one worktree-aware Python environment contract
across the documentation: in CLAUDE.md (lines 76-78), document the supported
worktree-local then shared-root .venv fallback and interpreter resolution; in
AGENTS.md (lines 43-49), update the expanded gate to use that same resolver as
scripts/ship/preflight.sh; and in docs/agents/quality.md (lines 16-20), make the
expanded commands executable under the documented setup, including when only the
shared root environment exists.
In `@CONTRIBUTING.md`:
- Around line 15-16: Reconcile the branch naming guidance in CONTRIBUTING.md:
update the worktree example and the branch-name table to use only
claude/<feature-slug>, cursor/<feature-slug>, or codex/<feature-slug>, or
explicitly state that the broader feature/<slug> and fix/<slug> patterns apply
only to non-agent branches. Keep all documented examples and rules consistent.
In `@docs/agents/domain.md`:
- Around line 18-19: Update the missing-file handling guidance in
docs/agents/domain.md so absent required security documents such as
SOURCES_WITH_AUTH.md and MCP_SURFACE.md stop processing with a named
MISSING_REFERENCE result. Preserve canonical-source fallback only for optional
notes, and do not silently skip missing security-sensitive references.
In `@docs/agents/openfindata-ship/README.md`:
- Around line 17-18: Update the helper references in the README entries for
scripts/readiness.sh and scripts/check-pr-threads.sh to use
repository-root-relative paths including docs/agents/openfindata-ship/, while
preserving their existing descriptions.
In `@docs/agents/openfindata-ship/scripts/check-pr-threads.sh`:
- Around line 9-17: Update the REPO_SLUG validation case before OWNER and NAME
extraction to accept only exactly one slash with non-empty owner and repository
components. Reject values with no slash, multiple slashes, or leading/trailing
slashes, while preserving the existing invalid-slug error and exit behavior.
In `@docs/agents/openfindata-ship/scripts/readiness.sh`:
- Around line 13-14: Update the root-checkout detection in the readiness script
to compare the absolute Git directory and absolute common Git directory returned
by git metadata, rather than testing whether .git is a directory. Use this
comparison for the related shipping guard so the primary checkout is always
treated as inspection-only, including when .git is a file.
---
Nitpick comments:
In @.githooks/post-checkout:
- Line 15: Update the post-checkout hook around guardrails_warn_post_checkout to
accept Git’s third hook argument and invoke the warning only when that flag
equals 1, suppressing warnings for file checkouts and worktree operations.
In `@docs/agents/orientation.md`:
- Line 26: Update the orientation table entry for MCP in
docs/agents/orientation.md to remove the approximate “~25 tools” count while
still directing readers to the curated mcp_app catalog and docs/MCP_SURFACE.md.
In `@scripts/git/guardrails.sh`:
- Around line 236-247: Update the staged-file handling around
guardrails_staged_py_files to read paths into a Bash array using a NUL
delimiter, preserving filenames containing spaces. Use the array when invoking
both ruff check and ruff format --check, while retaining the no-files skip
behavior.
- Around line 282-286: Update the hook source selection near
guardrails_current_checkout so installation copies from guardrails_repo_root,
while retaining common_git_dir for the shared install destination. Ensure
source_hooks resolves to the repository root’s .githooks directory rather than
the active worktree.
- Around line 292-297: Update the installer around install_dir setup to remove
stale hook files and copy every hook from source_hooks dynamically, replacing
the three fixed cp commands. Preserve installation of guardrails.sh and its
executable permissions, while ensuring hooks removed from .githooks no longer
remain active.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5195df4-bc70-4ff1-afb7-a52933cf1210
📒 Files selected for processing (18)
.claude/skills/mcp-trust-reviewer/SKILL.md.githooks/post-checkout.githooks/pre-commit.githooks/pre-pushAGENTS.mdCLAUDE.mdCONTRIBUTING.mddocs/agents/domain.mddocs/agents/mcp-trust-review.mddocs/agents/openfindata-ship/README.mddocs/agents/openfindata-ship/SKILL.mddocs/agents/openfindata-ship/scripts/check-pr-threads.shdocs/agents/openfindata-ship/scripts/readiness.shdocs/agents/orientation.mddocs/agents/quality.mdscripts/git/guardrails.shscripts/git/install-hooks.shscripts/ship/preflight.sh
Tighten MCP trust reviewer isolation, root-checkout detection, PR slug validation, and docs contracts for worktrees/venv/branch policy. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
docs/agents/orientation, domain, quality, MCP trust checklist, and in-repoopenfindata-shipskill (no external skill install).mainare inspect-only; humans and agents work from dedicated worktrees.scripts/ship/preflight.sh(readiness + ruff/mypy/pytest with SHA evidence) and MCP trust reviewer skill.Reviews
CROSS_FAMILY(author: Cursor/Composer; reviewer: GPT-5.6 Terra via adversarial-reviewer). Initial High findings fixed (ship review-after-commit + clean tree; CONTRIBUTING worktree-first). Re-review: clear to open PR.MCP_SURFACE/ PASS (docs + reviewer skill only; no runtimemcp_app/ code-mode change).Test plan
bash scripts/git/install-hooks.shthen confirm commit blocked on root/mainbash scripts/ship/preflight.shpassesdocs/agents/openfindata-ship/scripts/readiness.shci.ymlgreen on this PRMade with Cursor
Summary by CodeRabbit
New Features
Documentation