chore: add docs-required-check workflow for fix/feat PRs#683
chore: add docs-required-check workflow for fix/feat PRs#683chaodu-agent wants to merge 3 commits intoopenabdev:mainfrom
Conversation
|
All PRs must reference a prior Discord discussion to ensure community alignment before implementation. Please edit the PR description to include a link like: This PR will be automatically closed in 3 days if the link is not added. |
- Fix git diff to use base.sha...head.sha instead of origin/base.ref...HEAD - Add skip-docs-check escape hatch via PR body keyword - Require space after colon in Conventional Commits regex - Unify error messages to English
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
OpenAB PR ScreeningThis is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Screening report## IntentPR #683 adds a GitHub Actions check that requires documentation changes whenever a PR is labeled by title as a The operator-visible problem is stale documentation: bug fixes and new features can merge without corresponding updates under FeatThis is a CI/process improvement, not an application feature. Behavioral change: on relevant pull request events, CI checks the PR title. If the title starts with Who It ServesPrimary beneficiaries: maintainers and reviewers. Secondary beneficiaries: agent runtime operators, contributors, and end users who depend on Rewritten PromptImplement a GitHub Actions workflow that enforces documentation updates for PRs whose titles follow Conventional Commit types Requirements:
Merge PitchThis is worth advancing because it turns a repeated review expectation into an automated gate. It is low implementation risk and easy to reason about. The main reviewer concern is policy strictness: not every Best-Practice ComparisonOpenClaw principles mostly do not apply directly because this is CI enforcement, not scheduled runtime execution. Relevant ideas are explicit routing and run logs: the workflow should clearly route only OpenClaw principles like gateway-owned scheduling, durable job persistence, isolated executions, and retry/backoff are not central here because GitHub Actions already provides event dispatch, job isolation, and run history. Hermes Agent principles are also only partially relevant. The closest matches are atomic persisted state and self-contained prompts: the workflow should avoid mutable state and make its failure output self-contained enough for contributors to act without asking a maintainer. Hermes concepts like gateway daemon ticks, file locking, and fresh sessions per scheduled run are not a fit for this PR. Implementation Options
Comparison Table
RecommendationAdvance this PR with the balanced option. The core idea is sound, but the workflow should be tightened before merge so it does not become a noisy gate. The next agent should verify title matching, changed-file detection, clear CI output, and whether maintainers want an explicit bypass label for fixes where documentation is genuinely unnecessary. If policy questions block review, split the work: first merge a non-bypass docs-required check with precise matching and good logs, then follow up with configurable exemptions once maintainers agree on the rule. |
PR Review: #683Reviewed by: masami-agent Summary
Core Assessment
Findings🔴 Critical: Mandatory docs requirement is too restrictive for this projectWhat: The workflow requires every Why it matters: Most bug fixes and many small features do not require documentation changes. This creates unnecessary friction for contributors and will likely lead to either:
Suggestion: This policy should be discussed with maintainers before implementation. If docs enforcement is desired, consider a softer approach — e.g., a bot comment reminding contributors to consider docs updates, rather than a hard CI gate. 🔴 Critical: Shell injection vulnerability via PR bodyWhere: What: Why it matters: Security. The Fix: Use an intermediate step that writes the body to a file, or use a dedicated action for string matching instead of shell grep on user input. 🟡 Minor:
|
Supplementary Review: #683 (masami-agent)Based on commit: After a second pass, I found additional issues I missed in my initial review: 🔴 Critical: Inline GitHub expression in
|
REQUEST CHANGES 🔴This PR needs to be reworked before it can be merged. The following issues were identified by the four-monk review team: Issues to address1. Split Conventional Commits title validation into its own PRThe title check (lines 16-24) enforces a repo-wide policy not mentioned in the PR title or description. This should be a separate PR with explicit maintainer discussion. 2. Mandatory docs requirement is too restrictiveMany legitimate
3. Shell injection risk
if [[ "$PR_BODY" =~ skip-docs-check ]]; then4. Inline ${{ }} expressions in run blockPass env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}Recommendation: Split into two PRs — (1) Conventional Commits title check (valuable standalone), (2) docs-required check (needs issue discussion first on scope and strictness). Please address these concerns and update the PR. |
Chloe 複審:PR #683Reviewed by: Chloe (Stage 2 — second review) Summary
Core Assessment
Verification of Prior ReviewsI have reviewed Masami's findings and 超渡法師's request-changes comment. All findings are confirmed correct. No false positives. Additional Findings🔴 Critical:
|
Summary
Add a GitHub Actions workflow that enforces documentation updates for all
fixandfeatPRs.What it does
pull_requestevents (opened, synchronize, reopened, edited)fixorfeat(Conventional Commits format)docs/has been modifiedWhy
This ensures that every bug fix and new feature comes with documentation updates, so agents and users can always find up-to-date best practices in
docs/.Testing
feat: ...orfix: ...without docs/ changes → ❌ CI failsfeat: ...orfix: ...with docs/ changes → ✅ CI passeschore:,ci:) → ⏭️ skipped