diff --git a/.agents/skills/docs-authoring/SKILL.md b/.agents/skills/docs-authoring/SKILL.md index 20186bc2..05e69b5d 100644 --- a/.agents/skills/docs-authoring/SKILL.md +++ b/.agents/skills/docs-authoring/SKILL.md @@ -11,8 +11,8 @@ description: >- Use this skill for repo documentation and standards work. Pair it with `markdown` for syntax and low-churn Markdown editing. Invoke -`issue-workflow` when the task adds issue templates, issue policy, or -proactive follow-up issue handling. +`planning` for planning-only work, and use `issue-workflow` when the task adds +issue templates, issue policy, or proactive follow-up issue handling. ## Workflow @@ -31,6 +31,8 @@ proactive follow-up issue handling. 4. Preserve frontmatter, generated markers, and doc type boundaries. 5. Keep tracked docs and control-plane text neutral, durable, and free of scratch workflow bookkeeping. + Keep live docs enforcement script-owned; do not add live-repo Markdown or + control-plane policy assertions to pytest. 6. Validate docs changes with: - `make docs-check` 7. When the change touches forward-looking target docs, titles, summaries, or diff --git a/.agents/skills/implementation-workflow/SKILL.md b/.agents/skills/implementation-workflow/SKILL.md index c3504520..a4c359a9 100644 --- a/.agents/skills/implementation-workflow/SKILL.md +++ b/.agents/skills/implementation-workflow/SKILL.md @@ -11,8 +11,9 @@ description: >- Use this skill for the normal repo implementation path. Pair it with `markdown` only when the task edits Markdown files. Invoke -`issue-workflow` when implementation uncovers meaningful out-of-scope repo -work that should be captured as a follow-up issue. +`planning` for planning-only work, and use `issue-workflow` when +implementation uncovers meaningful out-of-scope repo work that should be +captured as a follow-up issue. ## Workflow @@ -43,6 +44,8 @@ work that should be captured as a follow-up issue. Use the shell-safe commit/PR authoring path from `docs/standards/commits.md` whenever the structured metadata includes backticks, quotes, or other shell-sensitive text. +10. Keep durable delivery metadata and ordinary branch names phase-free and + roadmap-free. ## Focus diff --git a/.agents/skills/planning/SKILL.md b/.agents/skills/planning/SKILL.md new file mode 100644 index 00000000..19f67d41 --- /dev/null +++ b/.agents/skills/planning/SKILL.md @@ -0,0 +1,54 @@ +--- +name: planning +description: >- + Start planning-only repo work with the narrow planning bundle and hand + execution to the owning workflow skill once the plan is settled. +--- + +# Planning + +Use this skill for the repo's general planning path. It is the repo's general planning skill, not a replacement for execution workflows. + +Pair it with `markdown` only when the planning task edits Markdown, and hand +execution off to `docs-authoring`, `implementation-workflow`, `pr-review`, or +`issue-workflow` once the plan is settled. + +## Workflow + +1. Read the narrow planning bundle first: + - `AGENTS.md` + - `docs/README.md` + - `docs/status/current-state.md` + - `docs/reference/repository-history.md` + - `docs/standards/implementation.md` + - `docs/standards/delivery-guardrails.md` + - `docs/standards/commits.md` + - `tools/docs_maintenance/cli.py` + - `tools/docs_maintenance/metadata.py` +2. Confirm the planning surface, the owning execution workflow, and whether the + task actually needs roadmap, migration, architecture, or area-specific docs + before loading more context. +3. Keep durable surfaces phase-free and roadmap-free. Keep roadmap-owned phase + tags confined to planning and forward-looking docs. +4. Keep live docs enforcement in the owning script. Use synthetic strings or + temp repos for docs-tooling tests instead of turning pytest into the live- + repo docs-policy gate. +5. Write an execution-ready plan with scope, exclusions, execution order, + verification inventory or TDD-first tests, bounded checkpoint commits for + non-trivial work, and the assumptions or defaults that affect execution. +6. Keep the plan compaction-safe: prefer concrete file paths, commands, and + check ids, explain script-versus-pytest ownership when that boundary + matters, and avoid duplicating execution details owned by another workflow + skill. +7. Keep the plan narrow enough to hand execution to an existing workflow skill + instead of duplicating that workflow here. +8. After compaction or context loss, reload the same narrow planning bundle + before extending or revising the plan. + +## Focus + +- planning-only repo work +- execution-ready plans with clear verification, checkpoints, and assumptions +- routing and skill handoffs +- docs-check ownership and validator placement +- delivery metadata policy diff --git a/.agents/skills/planning/agents/openai.yaml b/.agents/skills/planning/agents/openai.yaml new file mode 100644 index 00000000..770b1903 --- /dev/null +++ b/.agents/skills/planning/agents/openai.yaml @@ -0,0 +1,7 @@ +interface: + display_name: "Planning" + short_description: "Start planning-only repo work with the narrow planning bundle and produce a compaction-safe execution plan." + default_prompt: "Use $planning to load tallylot's general planning route before drafting a compaction-safe repo plan or policy change." + +policy: + allow_implicit_invocation: true diff --git a/.agents/skills/pr-review/SKILL.md b/.agents/skills/pr-review/SKILL.md index 45fa65e7..9926f49e 100644 --- a/.agents/skills/pr-review/SKILL.md +++ b/.agents/skills/pr-review/SKILL.md @@ -30,7 +30,11 @@ Use this skill for repeatable review passes on an active branch or draft PR. Do not treat green `tools.run_pr_review_checks` output as a no-findings decision by itself; it is only verification evidence for the current red-team pass. -5. Use `issue-workflow`, `docs-authoring`, or `implementation-workflow` when +5. Review for approved branch-root usage, phase leakage on durable metadata, + and docs-policy duplication in pytest. +6. Use `planning` when a finding points to broader planning rather than a + narrow patch. +7. Use `issue-workflow`, `docs-authoring`, or `implementation-workflow` when the repair work moves into those repo-local workflows. ## Focus diff --git a/.claude/commands/implementation-checkpoint.md b/.claude/commands/implementation-checkpoint.md index 4e44e29e..98d93c3d 100644 --- a/.claude/commands/implementation-checkpoint.md +++ b/.claude/commands/implementation-checkpoint.md @@ -29,6 +29,9 @@ Use this route before closing any non-trivial coding task. coverage 5. Confirm tracked docs, templates, and control-plane text stayed neutral and did not pick up scratch workflow bookkeeping. + Confirm durable delivery metadata stayed phase-free and roadmap-free, branch + naming uses an approved `/` root plus slug, and live docs policy + enforcement stayed in the owning scripts rather than duplicated in pytest. 6. Confirm meaningful out-of-scope repo work is not stranded in notes: - search for an existing issue first - create the follow-up issue immediately when no suitable issue exists @@ -92,6 +95,9 @@ Use this route before closing any non-trivial coding task. closing the older PR; if the user explicitly requested a one-time protected-branch repair, verify the remote branch tip afterward and return to PR-only flow. + For ordinary delivery work, confirm the active branch uses an approved root + from `docs/standards/commits.md` and that branch names, commit messages, + PR titles, and PR bodies stay phase-free and roadmap-free. If a needed structural fix is already obvious and bounded, include it in the same checkpoint instead of deferring it. diff --git a/.claude/commands/pr-review.md b/.claude/commands/pr-review.md index 79688b6d..782328bd 100644 --- a/.claude/commands/pr-review.md +++ b/.claude/commands/pr-review.md @@ -20,10 +20,17 @@ Use this route for repeatable review passes on an active branch or draft PR. 3. Re-check every prior fix first, then inspect the next applicable changed file group that has not yet been re-checked in the current full review cycle. + Pay extra attention to: + - non-standard branch roots or nested ordinary-work branch trees + - roadmap or phase leakage on durable standards, control-plane, or delivery + metadata surfaces + - docs-policy duplication in pytest when the owning script should enforce it 4. Repair every finding from that pass before starting the next pass. - reload the narrow repo guidance for each repaired area using `AGENTS.md`, its task-routing table, and the owning roadmap, architecture, migration, or delivery docs identified by that route or by repo search hints + - when a finding implies broader planning rather than a narrow local patch, + use the repo-local `planning` skill before picking the execution workflow - add or tighten tests, docs, automation, and validation where the fix belongs instead of leaving the repair in prose alone - when a meaningful finding should stay out of the active PR, search for an diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index 1faddcac..cdac435d 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -104,6 +104,7 @@ jobs: env: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} + BRANCH_NAME: ${{ github.event.pull_request.head.ref }} PR_TITLE: ${{ github.event.pull_request.title }} run: | uv run python -m tools.run_review_check \ @@ -111,6 +112,7 @@ jobs: --trigger pull_request \ --base-sha "$BASE_SHA" \ --head-sha "$HEAD_SHA" \ + --branch-name "$BRANCH_NAME" \ --pr-title "$PR_TITLE" \ --pr-body-file pr-body.md diff --git a/AGENTS.md b/AGENTS.md index 45b65222..355fe77e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,8 @@ Do not pre-load every repo doc by default. | ---- | ---- | | Code placement, typing, modularization, naming | `docs/standards/engineering.md` | | Active implementation execution discipline | `docs/standards/implementation.md`, `docs/standards/commits.md` | -| Repo standards, docs placement, doc authoring rules, or agent-default enforcement changes | `AGENTS.md`, `docs/README.md`, `docs/status/current-state.md`, `docs/reference/repository-history.md`, `docs/standards/implementation.md`, `docs/standards/commits.md`, `tools/docs_maintenance/cli.py`, `tools/docs_maintenance/metadata.py` | +| Repo standards, docs placement, doc authoring rules, or agent-default enforcement changes | `AGENTS.md`, `docs/README.md`, `docs/status/current-state.md`, `docs/reference/repository-history.md`, `docs/standards/implementation.md`, `docs/standards/delivery-guardrails.md`, `docs/standards/commits.md`, `tools/docs_maintenance/cli.py`, `tools/docs_maintenance/metadata.py` | +| Planning-only repo work, routing design, skill design, docs-check ownership, validator placement, or delivery metadata policy | `AGENTS.md`, `docs/README.md`, `docs/status/current-state.md`, `docs/reference/repository-history.md`, `docs/standards/implementation.md`, `docs/standards/delivery-guardrails.md`, `docs/standards/commits.md`, `tools/docs_maintenance/cli.py`, `tools/docs_maintenance/metadata.py` | | Issue templates, issue-writing policy, or proactive follow-up issue creation | `AGENTS.md`, `docs/standards/issues.md`, `docs/standards/implementation.md`, `docs/standards/delivery-guardrails.md`, `docs/standards/commits.md`, `.claude/commands/issue-workflow.md` | | Delivery guardrails, protected-branch behavior, or agent-assisted Git operations | `docs/standards/delivery-guardrails.md`, `docs/standards/commits.md`, `tools/audit_delivery_guardrails.py` | | PR review or review-loop recovery | `docs/standards/delivery-guardrails.md`, `docs/standards/implementation.md`, `docs/standards/commits.md`, `.claude/commands/pr-review.md` | @@ -62,6 +63,11 @@ Do not pre-load every repo doc by default. - When editing repo standards, automation, or other control-plane files, use the repo-local workflow for the active area and reload the narrow repo guidance listed in this file before editing. +- Planning-only repo work should start with the repo-local `planning` skill, + then hand execution off to the owning workflow skill. +- Route phase-tag policy and docs-check-boundary work through the standards + path; keep live docs enforcement in the owning script rather than duplicating + that policy in pytest. - Keep live repo document checks in script-owned verification surfaces, not in pytest: - `docs-maintenance` owns generated docs sections, metadata, links, and @@ -75,6 +81,14 @@ Do not pre-load every repo doc by default. - Keep pytest coverage for docs tooling on synthetic strings or temp repos, not on the live repo Markdown tree or live control-plane prose. - Keep tracked docs, templates, and control-plane text neutral and durable. +- Planning and forward-looking docs may use roadmap-owned ephemeral tags when + the planning surface itself owns them, but durable control-plane, delivery + metadata, branch names, code, tooling, filenames, and generated durable + examples must stay phase-free and roadmap-free. +- Keep ordinary test names, assertions, and examples phase-free and + roadmap-free. Synthetic tooling fixtures may intentionally mention forbidden + roadmap or phase labels only when they are exercising validator or audit + behavior on synthetic strings or temp repos. - Keep current-state docs accurate to the implemented runtime, and keep forward-looking docs detailed enough to implement from without inventing missing stage structure later. diff --git a/Makefile b/Makefile index 8b402a63..1ee6ba8e 100644 --- a/Makefile +++ b/Makefile @@ -61,7 +61,7 @@ help: ' make tool TOOL=docs_maintenance ARGS="sync --check" Run any tools. entrypoint.' \ '' \ 'Individual tools:' \ - ' make pytest ARGS="tests/unit/test_x.py -q --no-cov"' \ + ' make pytest ARGS="tests/unit/test_x.py -q"' \ ' make ruff ARGS="check ."' \ ' make mypy ARGS="src tools repo_support"' \ ' make pyright ARGS=""' \ @@ -143,7 +143,7 @@ pylint: pylint $(ARGS) pytest: - pytest $(ARGS) + pytest --no-cov $(ARGS) actionlint: actionlint $(ARGS) diff --git a/docs/standards/commits.md b/docs/standards/commits.md index fa146e46..89358fc9 100644 --- a/docs/standards/commits.md +++ b/docs/standards/commits.md @@ -50,6 +50,44 @@ Subject rules: - no trailing period - do not use generic summaries such as `cleanup`, `misc fixes`, or `update branch` +- do not use roadmap or phase labels on durable delivery metadata surfaces; + use durable repo-area or behavior wording instead + +## Standard Branch Tree + +Ordinary delivery work uses this branch format: + +```text +/ +``` + +Rules: + +- use exactly one approved top-level root +- use exactly one lowercase kebab-case slug after the root +- use lowercase ASCII only +- do not add extra slash nesting +- do not use roadmap or phase labels in either segment +- do not invent new ordinary-work roots such as `control-plane` + +Approved roots: + +- `docs` +- `chore` +- `ci` +- `build` +- `feat` +- `fix` +- `refactor` +- `test` +- `repair` + +Notes: + +- this policy governs every ordinary delivery branch that will ship through the + normal PR flow +- `backup/*`, `codex/*`, and similar ad hoc roots are not approved ordinary + delivery branches ## Body Template @@ -128,6 +166,8 @@ Protected-branch rule: - if a protected-branch repair exception is explicitly requested, limit that exception to the exact repair action, verify the remote branch tip immediately afterward, and restore PR-only flow before continuing +- active ordinary-work branch names must match the approved + `/` tree and stay phase-free plus roadmap-free PR title rules: @@ -136,6 +176,7 @@ PR title rules: and, for a single-commit PR, the squash subject becomes ` (#)` - do not use generic titles such as `update branch`, `cleanup`, or `misc fixes` +- do not use roadmap or phase labels in the title PR body rules: @@ -169,6 +210,8 @@ PR body rules: issue-closing keywords unless the user explicitly requests otherwise - for a one-commit PR, still list that single commit under `Included checkpoints:` +- do not use roadmap or phase labels in PR bodies; keep delivery metadata + phrased in durable repo-area and behavior terms Merge method rules: @@ -288,6 +331,15 @@ make validate-commit-message ARGS='.git/COMMIT_EDITMSG' make validate-commit-message ARGS='--rev-range HEAD~3..HEAD' ``` +Validate branch names and PR metadata with the repo tooling when needed: + +```bash +python -m tools.validate_pr_metadata \ + --branch-name docs/standards-skill-hardening \ + --title 'docs(commits): harden durable metadata policy' \ + --body "$(cat /tmp/pr-body.md)" +``` + When structured commit messages or PR bodies include backticks, quotes, or other shell-sensitive text, use file/stdin authoring forms rather than inline `-m` or `--body` arguments so the metadata stays literal. In this repo, use the @@ -336,10 +388,10 @@ changing CI, packaging, release, or other workflow areas where the local verification pass should mirror the final non-draft PR suite before handoff. Add `--pr-title` plus `--pr-body-file` when you also want the full review run to validate the -current branch PR title, body, and `Included checkpoints:` list against the -branch history. Treat `tools.run_quality_gates --full-tests` as an explicit -full-suite escape hatch rather than the normal agent close-out path, and avoid -it unless there is a specific reason to use the override. Do not run +current branch name, PR title, body, and `Included checkpoints:` list against +the branch history. Treat `tools.run_quality_gates --full-tests` as an +explicit full-suite escape hatch rather than the normal agent close-out path, +and avoid it unless there is a specific reason to use the override. Do not run `tools.run_quality_gates --full-tests` immediately before `tools.run_pr_review_checks --mode full`; the full PR-review runner already includes the full quality gate pass plus the extra @@ -369,7 +421,7 @@ that exact file before treating the checkpoint as closed: ```bash make pylint ARGS='' -make pytest ARGS='-q --no-cov ' +make pytest ARGS='-q ' git show HEAD: ``` diff --git a/docs/standards/delivery-guardrails.md b/docs/standards/delivery-guardrails.md index 5f09f9b2..ca529f2a 100644 --- a/docs/standards/delivery-guardrails.md +++ b/docs/standards/delivery-guardrails.md @@ -44,6 +44,12 @@ when the higher-layer control is available. ## Default Delivery Posture - protected branches are PR-only branches +- ordinary delivery branches use the approved `/` tree from + `docs/standards/commits.md` +- active branch names, commit messages, PR titles, and PR bodies stay + phase-free and roadmap-free because they are durable delivery metadata +- planning and forward-looking docs may still use roadmap-owned ephemeral tags + where the planning surface itself owns that reference - pull requests open as draft by default - a PR becomes ready for review only after the full issue-finding hardening loop yields no new meaningful findings @@ -144,6 +150,13 @@ Encode the repo's delivery rules in versioned artifacts: - contract tests that pin the standards - templates that match the validators +Keep live docs policy ownership split in code as well as prose: + +- tests verify tooling logic and code behavior +- tests do not serve as the live-repo enforcement surface for docs policy +- add new live-repo docs assertions to the owning script, not to pytest +- keep pytest coverage for docs tooling synthetic or temp-repo based + When a delivery failure repeats or has high repair cost, add or tighten a machine-checkable guard instead of only adding more prose. @@ -276,11 +289,11 @@ push-to-mainline CI. - the `commit-messages` PR status validates the branch commit-message range and PR metadata on pull requests only -- the `pr-metadata` PR status validates the pull request title, body, and - checkpoint linkage as its own visible required status +- the `pr-metadata` PR status validates the active branch name, pull request + title, body, and checkpoint linkage as its own visible required status - the repo-installed `pre-push` hook mirrors `tools.validate_pr_metadata` - against the current open pull request before push when `gh pr view` can - resolve PR metadata for the branch + against the current branch name first, then the current open pull request + before push when `gh pr view` can resolve PR metadata for the branch - the `plan-pr-review` workflow job audits the diff with `tools.audit_pr_review`, publishes the selected checks for transparency, and keeps the human review routing visible while choosing planned mode diff --git a/docs/standards/engineering.md b/docs/standards/engineering.md index cb22f818..c732d5bb 100644 --- a/docs/standards/engineering.md +++ b/docs/standards/engineering.md @@ -904,7 +904,10 @@ Rules: docs-maintenance-owned structure, not target-naming rules - required `naming_scope` frontmatter/default behavior is docs-maintenance-owned frontmatter validation, not target-naming drift -- forward-target contract docs must stay free of transient planning language +- planning and forward-looking docs may use roadmap-owned ephemeral tags when + the planning surface itself owns that reference +- durable standards, control-plane, and delivery metadata surfaces must stay + free of roadmap-owned phase tags and other ephemeral planning labels - keep frontmatter summaries and generated `docs/README.md` blurbs content-first; do not lead with page-role or authority-first labels that foreground governance instead of the held contract diff --git a/docs/standards/implementation.md b/docs/standards/implementation.md index 22a4201b..34c4aa0a 100644 --- a/docs/standards/implementation.md +++ b/docs/standards/implementation.md @@ -125,6 +125,30 @@ Live repo document enforcement stays in script-owned tools, not pytest: - keep pytest coverage for docs tooling on synthetic strings or temp repos instead of the live repo Markdown tree or live control-plane prose +Durable versus planning surfaces: + +- planning and forward-looking docs may use roadmap-owned ephemeral tags when + that reference convenience belongs to the planning surface itself +- durable control-plane and delivery surfaces must stay phase-free and + roadmap-free: + - `docs/standards/**` + - `AGENTS.md` + - `.claude/commands/**` + - `.agents/skills/**` + - `.github/pull_request_template.md` + - commit messages + - PR titles and PR bodies + - branch names + - code, tooling, filenames, and generated durable examples +- keep ordinary repo-facing tests and named examples phase-free and + roadmap-free +- synthetic tooling fixtures may intentionally mention forbidden roadmap or + phase labels only when they are exercising validator or audit behavior on + synthetic strings or temp repos +- when a new live-repo docs policy needs enforcement, add it to the owning + script instead of adding live-repo Markdown or control-plane prose checks to + pytest + ## Default Coding Expectations Agents should assume all of these are expected unless the task explicitly says @@ -342,6 +366,9 @@ Expected behavior: commands such as `rm -rf`, `git restore`, `git reset`, or `git checkout --` unless the user explicitly requests that cleanup in the current thread - keep tracked docs, templates, and control-plane artifacts neutral and durable +- keep durable delivery metadata and ordinary branch names phase-free and + roadmap-free; use durable repo-area or behavior wording instead of planning + tags - keep scratch review notes, temporary review ledgers, and compaction aids untracked; recover from deterministic repo facts instead diff --git a/repo_support/docs_audit/rules/policy_alignment.py b/repo_support/docs_audit/rules/policy_alignment.py index 5adfe2f9..a86c0932 100644 --- a/repo_support/docs_audit/rules/policy_alignment.py +++ b/repo_support/docs_audit/rules/policy_alignment.py @@ -2,14 +2,13 @@ import re -from repo_support.paths import repo_root - -from ._common import build_rule -from ..helpers import ( +from repo_support.docs_audit.helpers import ( architecture_doc_paths, joined, repo_text, ) +from repo_support.docs_audit.rules._common import build_rule +from repo_support.paths import repo_root POLICY_ALIGNMENT_RULES = ( @@ -67,7 +66,7 @@ for needle in ( "Do not describe `mypy` or `pyright` as covering `pylint` findings.", "make pylint ARGS=''", - "make pytest ARGS='-q --no-cov '", + "make pytest ARGS='-q '", "git show HEAD:", ) if needle not in repo_text("docs/standards/commits.md") @@ -277,7 +276,8 @@ "bridge cutover matrix inventory, owner, compatibility, reader, and gate", "`docs/README.md` generated reference-group headings/order are", "required `naming_scope` frontmatter/default behavior is", - "forward-target contract docs must stay free of transient planning language", + "planning and forward-looking docs may use roadmap-owned ephemeral tags", + "delivery metadata surfaces must stay", "treat the blocking `target-naming` review check as the repo-native guard", ) if needle not in repo_text("docs/standards/engineering.md") diff --git a/repo_support/review_verification/catalog.py b/repo_support/review_verification/catalog.py index 4280ca64..dbc7f8fd 100644 --- a/repo_support/review_verification/catalog.py +++ b/repo_support/review_verification/catalog.py @@ -118,6 +118,8 @@ def _spec( "python", "-m", "tools.validate_pr_metadata", + "--branch-name", + "{branch_name}", "--title", "{pr_title}", "--body", diff --git a/repo_support/review_verification/executor.py b/repo_support/review_verification/executor.py index d69e74e0..601d0daa 100644 --- a/repo_support/review_verification/executor.py +++ b/repo_support/review_verification/executor.py @@ -24,6 +24,7 @@ class CheckExecutionContext: trigger: Literal["pull_request", "push_main", "local"] base_sha: str | None = None head_sha: str | None = None + branch_name: str | None = None pr_title: str | None = None pr_body: str | None = None changed_paths: tuple[str, ...] = () @@ -79,6 +80,15 @@ def _resolve_default_base_head(context: CheckExecutionContext) -> tuple[str, str return base_sha, head_sha +def _resolve_branch_name(context: CheckExecutionContext) -> str: + if context.branch_name is not None: + return context.branch_name + branch_name = _git_stdout("branch", "--show-current") + if branch_name == "": + raise RuntimeError("branch name is required for PR metadata validation") + return branch_name + + def _load_open_pr_metadata() -> tuple[str, str, str] | None: if shutil.which("gh") is None: return None @@ -157,6 +167,8 @@ def resolve_check_command( replacements["{pr_body}"] = body replacements["{base_sha}"] = base_sha replacements["{head_sha}"] = head_sha + if any("{branch_name}" in part for part in command_parts): + replacements["{branch_name}"] = _resolve_branch_name(context) command = tuple( part if not replacements else _replace_placeholders(part, replacements) diff --git a/tests/contract/test_standards_guards.py b/tests/contract/test_standards_guards.py index e07994a5..80ba7ac8 100644 --- a/tests/contract/test_standards_guards.py +++ b/tests/contract/test_standards_guards.py @@ -748,6 +748,7 @@ def test_makefile_uses_home_relative_external_env_path() -> None: assert "PROJECT_ENV ?= $(HOME)/.venvs/tallylot-py312" in makefile_text assert "export PATH := $(PROJECT_BIN):$(PATH)" in makefile_text + assert "pytest --no-cov $(ARGS)" in makefile_text for target in ( "install-hooks:", "docs-check:", diff --git a/tests/unit/test_commit_message_validator.py b/tests/unit/test_commit_message_validator.py index 57af3c0f..36810b58 100644 --- a/tests/unit/test_commit_message_validator.py +++ b/tests/unit/test_commit_message_validator.py @@ -158,7 +158,7 @@ def test_squash_merge_commit_message_allows_wrapped_bullet_lines() -> None: tests/unit/test_docs_audit.py -q --no-cov'` Included checkpoints: -- `docs(roadmap): clarify planning and state docs` +- `docs(standards): clarify planning and state docs` """ errors = _validate_commit_message_text(message) @@ -166,6 +166,72 @@ def test_squash_merge_commit_message_allows_wrapped_bullet_lines() -> None: assert not errors +def test_commit_message_rejects_roadmap_label_in_subject() -> None: + message = """\ +docs(roadmap): harden planning guidance + +Why: +- keep durable metadata phase-free + +What: +- tighten the standards text + +Checks: +- make pytest +""" + + errors = _validate_commit_message_text(message) + + assert errors == ( + "commit message must not use roadmap/phase labels on durable metadata " + "surfaces: roadmap", + ) + + +def test_commit_message_rejects_phase_label_in_body() -> None: + message = """\ +docs(standards): harden planning guidance + +Why: +- keep Phase 2 bookkeeping out of durable metadata + +What: +- tighten the standards text + +Checks: +- make pytest +""" + + errors = _validate_commit_message_text(message) + + assert errors == ( + "commit message must not use roadmap/phase labels on durable metadata " + "surfaces: Phase 2", + ) + + +def test_commit_message_rejects_spelled_out_phase_label_in_body() -> None: + message = """\ +docs(standards): harden planning guidance + +Why: +- keep phase-zero bookkeeping out of durable metadata + +What: +- tighten the standards text + +Checks: +- make pytest +""" + + errors = _validate_commit_message_text(message) + + assert errors == ( + "commit message must not use roadmap/phase labels on durable metadata " + "surfaces: phase-zero", + ) + + def test_invalid_type_is_rejected() -> None: errors = _validate_commit_message_text( """\ diff --git a/tests/unit/test_pr_metadata_validator.py b/tests/unit/test_pr_metadata_validator.py index d7a2945b..28620748 100644 --- a/tests/unit/test_pr_metadata_validator.py +++ b/tests/unit/test_pr_metadata_validator.py @@ -2,6 +2,7 @@ from pytest import MonkeyPatch +from tools.message_standards import validate_branch_name from tools.validate_pr_metadata import ( _validate_pr_body, _validate_pr_checkpoints, @@ -58,12 +59,92 @@ def test_pr_title_with_generic_summary_is_rejected() -> None: ) +def test_pr_branch_with_standard_root_and_slug_is_valid() -> None: + errors = validate_branch_name("docs/standards-skill-hardening") + + assert not errors + + +def test_pr_branch_rejects_non_standard_root() -> None: + errors = validate_branch_name("control-plane/standards-hardening") + + assert errors == ( + "branch root must be one of: docs, chore, ci, build, feat, fix, " + "refactor, test, repair", + ) + + +def test_pr_branch_rejects_nested_tree() -> None: + errors = validate_branch_name("docs/standards/skill-hardening") + + assert errors == ( + "branch name must match `/` with exactly one approved root " + "and one lowercase kebab-case slug", + ) + + +def test_pr_branch_rejects_missing_root() -> None: + errors = validate_branch_name("standards-skill-hardening") + + assert errors == ( + "branch name must match `/` with exactly one approved root " + "and one lowercase kebab-case slug", + ) + + +def test_pr_branch_rejects_uppercase_root_or_slug() -> None: + root_errors = validate_branch_name("Docs/standards-hardening") + slug_errors = validate_branch_name("docs/Standards-hardening") + + assert root_errors == ( + "branch root must be one of: docs, chore, ci, build, feat, fix, " + "refactor, test, repair", + ) + assert slug_errors == ("branch slug must be lowercase kebab-case ASCII",) + + +def test_pr_branch_rejects_phase_label() -> None: + errors = validate_branch_name("docs/phase-0-hardening") + + assert errors == ( + "branch name must not use roadmap/phase labels on durable metadata " + "surfaces: phase-0", + ) + + +def test_pr_branch_rejects_spelled_out_phase_label() -> None: + errors = validate_branch_name("docs/phase-zero-hardening") + + assert errors == ( + "branch name must not use roadmap/phase labels on durable metadata " + "surfaces: phase-zero", + ) + + def test_pr_body_with_required_sections_is_valid() -> None: errors = _validate_pr_body(_body()) assert not errors +def test_pr_title_rejects_phase_label() -> None: + errors = _validate_pr_title("docs(standards): Phase 2 hardening") + + assert errors == ( + "PR title must not use roadmap/phase labels on durable metadata " + "surfaces: Phase 2", + ) + + +def test_pr_title_rejects_spelled_out_phase_label() -> None: + errors = _validate_pr_title("docs(standards): Phase Zero hardening") + + assert errors == ( + "PR title must not use roadmap/phase labels on durable metadata " + "surfaces: Phase Zero", + ) + + def test_pr_body_tolerates_leading_html_comment() -> None: body = f"\n\n{_body()}" @@ -92,6 +173,28 @@ def test_pr_body_rejects_missing_section() -> None: assert errors == ("missing `Included checkpoints:` section",) +def test_pr_body_rejects_roadmap_label() -> None: + errors = _validate_pr_body( + _body(why="- keep roadmap references out of PR metadata") + ) + + assert errors == ( + "PR body must not use roadmap/phase labels on durable metadata " + "surfaces: roadmap", + ) + + +def test_pr_body_rejects_spelled_out_phase_label() -> None: + errors = _validate_pr_body( + _body(why="- keep phase-zero references out of PR metadata") + ) + + assert errors == ( + "PR body must not use roadmap/phase labels on durable metadata " + "surfaces: phase-zero", + ) + + def test_pr_body_rejects_missing_issue_linkage_section() -> None: body = """\ Why: diff --git a/tests/unit/test_pre_push_hook.py b/tests/unit/test_pre_push_hook.py index 638df23d..97f2029e 100644 --- a/tests/unit/test_pre_push_hook.py +++ b/tests/unit/test_pre_push_hook.py @@ -16,7 +16,7 @@ def test_pre_push_hook_skips_without_current_branch( def test_pre_push_hook_skips_without_upstream( monkeypatch: pytest.MonkeyPatch, ) -> None: - monkeypatch.setattr(tools.pre_push_hook, "_current_branch", lambda: "feature") + monkeypatch.setattr(tools.pre_push_hook, "_current_branch", lambda: "feat/review") monkeypatch.setattr(tools.pre_push_hook, "_upstream_ref", lambda: None) assert tools.pre_push_hook.main([]) == 0 @@ -25,8 +25,10 @@ def test_pre_push_hook_skips_without_upstream( def test_pre_push_hook_skips_when_branch_has_no_open_pr( monkeypatch: pytest.MonkeyPatch, ) -> None: - monkeypatch.setattr(tools.pre_push_hook, "_current_branch", lambda: "feature") - monkeypatch.setattr(tools.pre_push_hook, "_upstream_ref", lambda: "origin/feature") + monkeypatch.setattr(tools.pre_push_hook, "_current_branch", lambda: "feat/review") + monkeypatch.setattr( + tools.pre_push_hook, "_upstream_ref", lambda: "origin/feat/review" + ) monkeypatch.setattr(tools.pre_push_hook, "_load_open_pr_metadata", lambda: None) assert tools.pre_push_hook.main([]) == 0 @@ -40,8 +42,14 @@ def test_pre_push_hook_validates_current_head_against_open_pr_metadata( def record_base_sha(sha: str) -> None: recorded["base"] = sha - monkeypatch.setattr(tools.pre_push_hook, "_current_branch", lambda: "feature") - monkeypatch.setattr(tools.pre_push_hook, "_upstream_ref", lambda: "origin/feature") + monkeypatch.setattr( + tools.pre_push_hook, "_current_branch", lambda: "docs/metadata-hardening" + ) + monkeypatch.setattr( + tools.pre_push_hook, + "_upstream_ref", + lambda: "origin/docs/metadata-hardening", + ) monkeypatch.setattr( tools.pre_push_hook, "_load_open_pr_metadata", @@ -52,7 +60,15 @@ def record_base_sha(sha: str) -> None: ) monkeypatch.setattr(tools.pre_push_hook, "_current_head_sha", lambda: "head456") - def fake_validate(*, title: str, body: str, base_sha: str, head_sha: str) -> int: + def fake_validate( + *, + branch_name: str, + title: str, + body: str, + base_sha: str, + head_sha: str, + ) -> int: + recorded["branch_name"] = branch_name recorded["title"] = title recorded["body"] = body recorded["base_sha"] = base_sha @@ -64,6 +80,7 @@ def fake_validate(*, title: str, body: str, base_sha: str, head_sha: str) -> int assert tools.pre_push_hook.main([]) == 0 assert recorded == { "base": "base123", + "branch_name": "docs/metadata-hardening", "title": "docs(commits): update policy", "body": "Why:\n- test\n", "base_sha": "base123", @@ -71,12 +88,33 @@ def fake_validate(*, title: str, body: str, base_sha: str, head_sha: str) -> int } +def test_pre_push_hook_rejects_invalid_branch_before_pr_lookup( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + monkeypatch.setattr( + tools.pre_push_hook, + "_current_branch", + lambda: "control-plane/metadata-hardening", + ) + + def fail_if_called() -> None: + raise AssertionError("PR lookup should not run for an invalid branch") + + monkeypatch.setattr(tools.pre_push_hook, "_load_open_pr_metadata", fail_if_called) + + assert tools.pre_push_hook.main([]) == 1 + assert "branch root must be one of" in capsys.readouterr().err + + def test_pre_push_hook_fails_when_pr_metadata_query_errors( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], ) -> None: - monkeypatch.setattr(tools.pre_push_hook, "_current_branch", lambda: "feature") - monkeypatch.setattr(tools.pre_push_hook, "_upstream_ref", lambda: "origin/feature") + monkeypatch.setattr(tools.pre_push_hook, "_current_branch", lambda: "feat/review") + monkeypatch.setattr( + tools.pre_push_hook, "_upstream_ref", lambda: "origin/feat/review" + ) def fail() -> None: raise RuntimeError("boom") diff --git a/tests/unit/test_repo_agent_skills.py b/tests/unit/test_repo_agent_skills.py index ea19a360..21e60625 100644 --- a/tests/unit/test_repo_agent_skills.py +++ b/tests/unit/test_repo_agent_skills.py @@ -37,6 +37,8 @@ class ExpectedSkill: "docs/standards/commits.md", "tools/docs_maintenance/cli.py", "tools/docs_maintenance/metadata.py", + "`planning` for planning-only work", + "Keep live docs enforcement script-owned", "make docs-check", "make naming-check", ), @@ -51,8 +53,42 @@ class ExpectedSkill: "make naming-check", "shell-safe commit/PR authoring path", "shell-sensitive text", + "`planning` for planning-only work", + "phase-free and", + "roadmap-free", ), ), + "planning": ExpectedSkill( + display_name="Planning", + required_fragments=( + "general planning skill", + "not a replacement", + "docs/README.md", + "docs/status/current-state.md", + "docs/reference/repository-history.md", + "docs/standards/implementation.md", + "docs/standards/delivery-guardrails.md", + "docs/standards/commits.md", + "tools/docs_maintenance/cli.py", + "tools/docs_maintenance/metadata.py", + "Pair it with `markdown` only when the planning task edits Markdown", + "task actually needs roadmap, migration, architecture, or area-specific docs", + "Write an execution-ready plan with scope, exclusions, execution order", + "verification inventory or TDD-first tests", + "bounded checkpoint commits", + "assumptions or defaults", + "Keep the plan compaction-safe", + "concrete file paths, commands, and", + "avoid duplicating execution details", + "Keep the plan narrow enough to hand execution to an existing workflow skill", + "After compaction or context loss", + "docs-authoring", + "implementation-workflow", + "pr-review", + "issue-workflow", + ), + short_description_fragments=("planning-only", "compaction-safe"), + ), "issue-workflow": ExpectedSkill( display_name="Issue Workflow", required_fragments=( @@ -76,6 +112,9 @@ class ExpectedSkill: "make audit-pr-review", "applicable file groups", "issue-finding with open outcome", + "approved branch-root usage", + "phase leakage on durable metadata", + "`planning`", ), ), "balance-submission-operations": ExpectedSkill( diff --git a/tests/unit/test_review_verification_executor.py b/tests/unit/test_review_verification_executor.py index b546db4c..0fecc5b6 100644 --- a/tests/unit/test_review_verification_executor.py +++ b/tests/unit/test_review_verification_executor.py @@ -41,6 +41,23 @@ def fake_git_stdout(*args: str) -> str: assert command[-1] == "abc123..def456" +def test_resolve_pr_metadata_command_includes_branch_name() -> None: + command = resolve_check_command( + check_spec("pr-metadata"), + context=CheckExecutionContext( + trigger="pull_request", + base_sha="abc123", + head_sha="def456", + branch_name="docs/metadata-hardening", + pr_title="docs(commits): harden durable metadata policy", + pr_body="Why:\n- test\n\nWhat:\n- test\n\nChecks:\n- test\n\nIssue linkage:\n- None: test\n\nIncluded checkpoints:\n- `docs(commits): harden durable metadata policy`\n", + ), + ) + + assert "--branch-name" in command + assert "docs/metadata-hardening" in command + + def test_run_plan_blocks_dependency_when_build_fails( monkeypatch: MonkeyPatch, ) -> None: diff --git a/tests/unit/test_review_verification_workflows.py b/tests/unit/test_review_verification_workflows.py index 339b7f2c..8f3c65cf 100644 --- a/tests/unit/test_review_verification_workflows.py +++ b/tests/unit/test_review_verification_workflows.py @@ -50,6 +50,8 @@ def test_pr_review_workflow_uses_draft_aware_planner_gated_atomic_jobs() -> None assert "tools.run_review_check --check-id target-naming" in workflow_text assert "tools.run_review_check --check-id docs-audit" in workflow_text assert "tools.run_review_check --check-id pytest-full" in workflow_text + assert "BRANCH_NAME: ${{ github.event.pull_request.head.ref }}" in workflow_text + assert '--branch-name "$BRANCH_NAME" \\' in workflow_text assert "needs.build.result == 'success'" in workflow_text assert "needs.pytest-full.result == 'success'" in workflow_text assert "tools.evaluate_review_results" in workflow_text diff --git a/tests/unit/test_run_pr_review_checks.py b/tests/unit/test_run_pr_review_checks.py index 9ace3f7a..5d0d5557 100644 --- a/tests/unit/test_run_pr_review_checks.py +++ b/tests/unit/test_run_pr_review_checks.py @@ -131,6 +131,31 @@ def fake_run_plan(*_args: object, **kwargs: object) -> ExecutionSummary: assert seen_contexts[0].pr_body == "Why:\n- explain\n" +def test_run_pr_review_checks_passes_branch_name_override( + monkeypatch: MonkeyPatch, +) -> None: + monkeypatch.setattr(run_pr_review_checks, "changed_paths", _docs_changed_paths) + monkeypatch.setattr(run_pr_review_checks, "run_local_autofix", lambda: 0) + seen_contexts: list[CheckExecutionContext] = [] + + def fake_run_plan(*_args: object, **kwargs: object) -> ExecutionSummary: + seen_contexts.append(cast(CheckExecutionContext, kwargs["context"])) + return ExecutionSummary(results=()) + + monkeypatch.setattr(run_pr_review_checks, "run_plan", fake_run_plan) + + assert ( + run_pr_review_checks.main( + [ + "--branch-name", + "docs/metadata-hardening", + ] + ) + == 0 + ) + assert seen_contexts[0].branch_name == "docs/metadata-hardening" + + def test_run_pr_review_checks_can_skip_local_autofix( monkeypatch: MonkeyPatch, ) -> None: diff --git a/tools/message_standards.py b/tools/message_standards.py index ce23f9df..eebd9bff 100644 --- a/tools/message_standards.py +++ b/tools/message_standards.py @@ -16,6 +16,18 @@ ) TYPE_PATTERN = "|".join(ALLOWED_TYPES) SCOPE_PATTERN = r"[a-z0-9]+(?:-[a-z0-9]+)*" +APPROVED_BRANCH_ROOTS = ( + "docs", + "chore", + "ci", + "build", + "feat", + "fix", + "refactor", + "test", + "repair", +) +BRANCH_SLUG_PATTERN = r"[a-z0-9]+(?:-[a-z0-9]+)*" AUTHORED_COMMIT_REQUIRED_SECTIONS = ("Why", "What", "Checks") PR_BODY_REQUIRED_SECTIONS = ( "Why", @@ -43,6 +55,36 @@ "update branch", } ) +PHASE_NUMBER_WORDS = ( + "zero", + "one", + "two", + "three", + "four", + "five", + "six", + "seven", + "eight", + "nine", + "ten", + "eleven", + "twelve", + "thirteen", + "fourteen", + "fifteen", + "sixteen", + "seventeen", + "eighteen", + "nineteen", + "twenty", +) +PHASE_NUMBER_WORD_PATTERN = "|".join(PHASE_NUMBER_WORDS) +FORBIDDEN_DURABLE_METADATA_PATTERNS = ( + re.compile(r"\broadmap\b", re.IGNORECASE), + re.compile( + r"\bphase(?:[ _-]?(?:\d+|" + PHASE_NUMBER_WORD_PATTERN + r"))\b", re.IGNORECASE + ), +) def validate_subject_line(subject: str) -> tuple[str, ...]: @@ -68,6 +110,55 @@ def validate_subject_line(subject: str) -> tuple[str, ...]: return tuple(errors) +def _forbidden_durable_metadata_matches(text: str) -> tuple[str, ...]: + matches: list[str] = [] + seen: set[str] = set() + for pattern in FORBIDDEN_DURABLE_METADATA_PATTERNS: + for match in pattern.finditer(text): + found = match.group(0) + normalized = found.lower() + if normalized in seen: + continue + seen.add(normalized) + matches.append(found) + return tuple(matches) + + +def validate_durable_metadata_text(text: str, *, label: str) -> tuple[str, ...]: + matches = _forbidden_durable_metadata_matches(text) + if not matches: + return () + found = ", ".join(matches) + return ( + f"{label} must not use roadmap/phase labels on durable metadata surfaces: " + f"{found}", + ) + + +def validate_branch_name(branch_name: str) -> tuple[str, ...]: + stripped = branch_name.strip() + if stripped == "": + return ("branch name is required",) + + errors: list[str] = [] + parts = stripped.split("/") + if len(parts) != 2 or any(part == "" for part in parts): + errors.append( + "branch name must match `/` with exactly one approved " + "root and one lowercase kebab-case slug" + ) + else: + root, slug = parts + if root not in APPROVED_BRANCH_ROOTS: + approved = ", ".join(APPROVED_BRANCH_ROOTS) + errors.append(f"branch root must be one of: {approved}") + if re.fullmatch(BRANCH_SLUG_PATTERN, slug) is None: + errors.append("branch slug must be lowercase kebab-case ASCII") + + errors.extend(validate_durable_metadata_text(stripped, label="branch name")) + return tuple(errors) + + def _skip_blank_lines(lines: tuple[str, ...], index: int) -> int: while index < len(lines) and lines[index] == "": index += 1 diff --git a/tools/pre_push_hook.py b/tools/pre_push_hook.py index 4a887ba5..30a37aa2 100644 --- a/tools/pre_push_hook.py +++ b/tools/pre_push_hook.py @@ -6,6 +6,8 @@ import sys from collections.abc import Sequence +from tools.message_standards import validate_branch_name + def _run_git(*args: str) -> subprocess.CompletedProcess[str]: return subprocess.run( @@ -93,13 +95,15 @@ def _load_open_pr_metadata() -> tuple[str, str, str] | None: def _validate_pr_metadata( - *, title: str, body: str, base_sha: str, head_sha: str + *, branch_name: str, title: str, body: str, base_sha: str, head_sha: str ) -> int: return subprocess.run( [ sys.executable, "-m", "tools.validate_pr_metadata", + "--branch-name", + branch_name, "--title", title, "--body", @@ -118,16 +122,20 @@ def main(argv: Sequence[str] | None = None) -> int: branch = _current_branch() if branch is None: return 0 - if _upstream_ref() is None: - return 0 try: + branch_errors = validate_branch_name(branch) + if branch_errors: + raise RuntimeError("; ".join(branch_errors)) + if _upstream_ref() is None: + return 0 metadata = _load_open_pr_metadata() if metadata is None: return 0 title, body, base_sha = metadata _ensure_commit_available(base_sha) return _validate_pr_metadata( + branch_name=branch, title=title, body=body, base_sha=base_sha, diff --git a/tools/run_pr_review_checks.py b/tools/run_pr_review_checks.py index 47715ddb..3f988760 100644 --- a/tools/run_pr_review_checks.py +++ b/tools/run_pr_review_checks.py @@ -37,6 +37,7 @@ def _parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: default="planned", help="Verification selection mode.", ) + parser.add_argument("--branch-name", help="Branch name override.") parser.add_argument("--pr-title", help="Pull request title override.") parser.add_argument("--pr-body-file", help="Pull request body file override.") parser.add_argument( @@ -92,6 +93,7 @@ def main(argv: Sequence[str] | None = None) -> int: trigger=args.trigger, base_sha=args.base_sha, head_sha=args.head_sha, + branch_name=args.branch_name, pr_title=args.pr_title, pr_body=pr_body, ), diff --git a/tools/run_review_check.py b/tools/run_review_check.py index 156c2786..e918d414 100644 --- a/tools/run_review_check.py +++ b/tools/run_review_check.py @@ -22,6 +22,7 @@ def _parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: ) parser.add_argument("--base-sha", help="Base SHA override.") parser.add_argument("--head-sha", help="Head SHA override.") + parser.add_argument("--branch-name", help="Branch name override.") parser.add_argument("--pr-title", help="Pull request title override.") parser.add_argument("--pr-body-file", type=Path, help="Pull request body file.") return parser.parse_args(argv) @@ -40,6 +41,7 @@ def main(argv: Sequence[str] | None = None) -> int: trigger=args.trigger, base_sha=args.base_sha, head_sha=args.head_sha, + branch_name=args.branch_name, pr_title=args.pr_title, pr_body=pr_body, ), diff --git a/tools/validate_commit_message.py b/tools/validate_commit_message.py index b6fb367d..3cd5b9b3 100644 --- a/tools/validate_commit_message.py +++ b/tools/validate_commit_message.py @@ -11,6 +11,7 @@ from tools.message_standards import ( AUTHORED_COMMIT_REQUIRED_SECTIONS, GENERATED_MAINLINE_COMMIT_OPTIONAL_SECTIONS, + validate_durable_metadata_text, validate_structured_sections, validate_subject_line, ) @@ -56,6 +57,10 @@ def _validate_commit_message_text(message: str) -> tuple[str, ...]: errors = [ *validate_subject_line(subject), + *validate_durable_metadata_text( + "\n".join(lines), + label="commit message", + ), *validate_structured_sections( lines, required_sections=AUTHORED_COMMIT_REQUIRED_SECTIONS, diff --git a/tools/validate_pr_metadata.py b/tools/validate_pr_metadata.py index 72f9b6a8..e6716ec3 100644 --- a/tools/validate_pr_metadata.py +++ b/tools/validate_pr_metadata.py @@ -9,6 +9,8 @@ from tools.message_standards import ( PR_BODY_OPTIONAL_SECTIONS, PR_BODY_REQUIRED_SECTIONS, + validate_branch_name, + validate_durable_metadata_text, validate_structured_sections, validate_subject_line, ) @@ -88,7 +90,10 @@ def _validate_pr_title(title: str) -> tuple[str, ...]: stripped = title.strip() if stripped == "": return ("PR title is required",) - return validate_subject_line(stripped) + return ( + *validate_subject_line(stripped), + *validate_durable_metadata_text(stripped, label="PR title"), + ) def _validate_why_entries(entries: tuple[str, ...]) -> tuple[str, ...]: @@ -161,16 +166,17 @@ def _validate_pr_body(body: str) -> tuple[str, ...]: lines = _normalize_body_lines(body) if not lines: return ("PR body is required",) - errors = list( - validate_structured_sections( + errors = [ + *validate_durable_metadata_text("\n".join(lines), label="PR body"), + *validate_structured_sections( ("placeholder", "", *lines), required_sections=PR_BODY_REQUIRED_SECTIONS, optional_sections=PR_BODY_OPTIONAL_SECTIONS, require_body=True, label="PR", allow_footers=False, - ) - ) + ), + ] parsed_sections = _parse_sections(body) errors.extend(_validate_why_entries(parsed_sections["Why"])) errors.extend(_validate_issue_linkage_entries(parsed_sections["Issue linkage"])) @@ -213,6 +219,7 @@ def _build_argument_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( description="Validate pull request title and body for the repo merge strategy." ) + parser.add_argument("--branch-name", help="Current branch name.") parser.add_argument("--title", required=True, help="Pull request title.") parser.add_argument("--body", required=True, help="Pull request body.") parser.add_argument( @@ -227,7 +234,11 @@ def _build_argument_parser() -> argparse.ArgumentParser: def main(argv: Sequence[str] | None = None) -> int: args = _build_argument_parser().parse_args(argv) - errors = [*_validate_pr_title(args.title), *_validate_pr_body(args.body)] + errors = [ + *(() if args.branch_name is None else validate_branch_name(args.branch_name)), + *_validate_pr_title(args.title), + *_validate_pr_body(args.body), + ] if args.base_sha and args.head_sha: errors.extend( _validate_pr_checkpoints(