Skip to content

Repository navigation

feat(pr): fail the description check on a missing Compatibility line or grouped AC rows and warn when the base moved #840

New issue
New issue
Open
Feature
Open
feat(pr): fail the description check on a missing Compatibility line or grouped AC rows and warn when the base moved#840
Feature
Labels
change:featPR adds or changes a feature (Conventional Commit type feat)refinement:readyPassed the Definition of Ready; may be picked up

Description

@tbhb-agent
tbhb-agent
opened on Oct 27, 2026

Problem

Two publish failures cost review rounds that an earlier mechanical check would have caught. A Verification using grouped AC IDs, such as "(AC1, AC2)" in a row or bullet, passes the description check and is then refused by the post-time review as provenance filler, so the author learns of it only at publish. A publish also refuses after the base moves, because the review stamp binds the base the draft names resolved to its remote-tracking ref, and the author learns of it only after the full test suite has run.

The Compatibility declaration that consumer-path changes require is already enforced by the description validator and has its own tests, so this issue adds nothing there.

Part of #588.

Acceptance criteria

  1. AC1: Given a Verification section in which a table row or a bullet names more than one AC ID in a parenthesized group or a comma-joined or "and"-joined list, when the description check runs, then it fails with the line number and states that each AC needs its own row.
  2. AC2: Given a Verification with one AC per row, and a row that names one AC ID in prose, when the check runs, then it passes.
  3. AC3: Given a draft with a review stamp and a base commit that differs from the commit the draft's named base now resolves to (its remote-tracking ref where one exists), when the check or the review step runs, then it prints a warning that publish will refuse, naming the stamped and current base commits and the steps to renew it: rebase, regenerate, recommit and review again.
  4. AC4: Given the AC3 warning, when publish runs, then its refusal, exit status and gates are unchanged, and the warning does not change the check's exit status.
  5. AC5: Given a draft whose base is not the default branch and whose stamped base still matches the resolved base, when the check runs, then it prints no warning.
  6. AC6: Given a draft that satisfies AC1 to AC5, when the check runs, then its output and exit status are unchanged from today.

Scope and non-goals

Scope: behavior of the description check and review step only, described as outcomes. Non-goal: changing the review criteria, the Compatibility rule, the stamp binding, or any publish gate. Non-goal: adding shell logic, which #588 freezes.

Approach

Meet AC1 to AC6 with failing cases first. The mechanism is the implementer's choice within those outcomes.

Dependencies

Blocked by #718, which ports the description validator to Go. No work starts in the frozen shell. If #718 has not landed when this reaches a builder, the builder waits.

Size

S. One pull request.

Verify plan

  • AC1: tests cover a parenthesized group, a comma-joined list and an "and"-joined list, each in a table row and in a bullet, and show each failing with the line number and the one-AC-per-row message.
  • AC2: a test shows a one-AC-per-row table and a prose row naming one AC passing.
  • AC3: a test stamps a draft, moves the resolved base, and shows the warning with both base commits and the renewal steps, for the check and for the review step.
  • AC4: a test runs publish after the warning and shows the same refusal and exit status as before this change, and the check's exit status unchanged.
  • AC5: a test uses a non-default base with a matching stamp and shows no warning.
  • AC6: a test runs a clean draft and shows output and exit status unchanged.
  • Definition of done: D1 each AC is verified at the merged head, with the commands and results cited in the pull request's Verification. D2 an independent review approves the exact merged head, and every finding has a recorded disposition. D3 required checks pass with no gate loosened. D4 the leak scans the repository specifies are clean, and every post made for this work passes the per-post leak review before it is posted. D5 every commit is signed. D6 the documentation matches the change or the Verification says why there is no impact. D7 every commit has an Assisted-by trailer and the pull request has the provenance:agent-assisted label. D8 the pull request stays within the size limits. D9 follow-ups are filed as issues and listed under Related. D10 the merge closes the issue through a closing reference, the board shows Done, and the final Status: comment names the merge commit. D9 is met when every deferred finding is filed as an issue with at least a Problem and numbered ACs, listed under Related in the pull request, and linked from any code comment that marks the deferred work.

Risks

A false positive blocks a valid draft, which AC2, AC5 and AC6 pin. A warning against the wrong base misleads, which AC3 and AC5 pin by binding it to the resolved base. Undo: revert the change, which removes the early diagnostics and leaves the publish gates as they are.

Operator authority

A standing rule names the operator: #588 freezes complex shell, and this issue honors that by waiting on #718. It grants no release, signing or publication authority and changes no gate.

Activity

  1. tbhb-agent
    added
    change:featPR adds or changes a feature (Conventional Commit type feat)
    on Oct 27, 2026
  2. github-project-automation
    added this to devtoolson Oct 27, 2026
  3. github-project-automation
    moved this to Backlog in devtoolson Oct 27, 2026
  4. tbhb-agent
    added the
    Feature
    issue type on Oct 27, 2026
  5. tbhb-agent
    added a parent issue on Oct 27, 2026

  6. tbhb-agent commented on Oct 27, 2026

    @tbhb-agent
    tbhb-agent
    on Oct 27, 2026
    ContributorAuthor

    Refinement: NOT READY — existing behavior and base-warning requirements need reconciliation
    Model: OpenAI/GPT-6, medium

    Checked

    Gaps

    1. Major — AC1 describes existing behavior without identifying a remaining defect.
      packages/agents/source/skills/pr/shared/scripts/validate-description.sh:485 already selects consumer paths and rejects missing or malformed Compatibility declarations. The check invokes that validator at packages/agents/source/skills/pr/shared/scripts/pr-agent.sh:48. Existing cases cover valid declarations at tests/skills/pr/scripts/compatibility.bats:53 and malformed or misplaced declarations at tests/skills/pr/scripts/compatibility.bats:64. Update the Problem to distinguish the current baseline from the historical failure. Treat AC1 as regression protection, or identify the observable case the existing check misses.

    2. Major — AC3’s comparison and recovery do not match publication’s contract.
      Review binds the base named by the draft, not necessarily the default branch: packages/agents/source/skills/pr/shared/scripts/review.sh:77. Resolution prefers its local remote-tracking ref at packages/agents/source/skills/pr/shared/scripts/description.sh:89. Publication compares those same bindings and requests renewed review at packages/agents/source/skills/pr/shared/scripts/create-pr.sh:162. Comparing a valid non-default base with the default tip can therefore falsely announce refusal. Rebasing alone also cannot renew the stamp. Define the warning against the publication-relevant base, clarify whether “current” means locally known or live remote state, and require guidance that includes renewed review. Preserve the existing refusal gate.

    3. Major — Scope and dependencies conflict with the named epic’s standing constraint.
      Replace complex shell with tested Go, starting with gates and merging #588 freezes complex shell and requires its replacement through the migration sequence. The issue instead limits scope to the description script and validator, whose current logic is shell (packages/agents/source/skills/pr/shared/scripts/pr-agent.sh:46, packages/agents/source/skills/pr/shared/scripts/validate-description.sh:418), while declaring no dependencies or operator authority. Express scope as behavioral requirements and reconcile delivery with the epic’s freeze: identify a real prerequisite if needed, or record an applicable operator decision. Leave implementation design to the PR.

    4. Moderate — The Verify plan does not prove the complete promised behavior.
      AC2’s single grouped-row example does not establish both grouping forms across rows and bullets, or the required line-specific corrective diagnostic. AC3’s unchanged exit status does not establish an unchanged verdict, correct recovery guidance, or preserved publish refusal. AC4 checks output but omits result preservation. Expand those checks, including a valid non-default-base case for AC3. D9’s planning statement also omits minimum follow-up issue content and links from deferred code work.

    Split

    No mandatory split. The remaining diagnostics can fit one PR once the baseline and migration constraint are reconciled. A broader prerequisite migration should remain separate.

    Definition of done

    The definition of done for #840 (Feature). Tick each item and give its evidence.

    Ticks indicate planning coverage only, not completed work.

    • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link.
      Evidence: Verify plan D1 explicitly requires merged-head verification and commands/results by AC.
    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.
      Evidence: Verify plan D2 requires exact-head independent approval and recorded finding dispositions; D9 requires follow-up issues.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
      Evidence: Verify plan D3 requires passing checks with no gate loosened.
    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.
      Evidence: Verify plan D4 incorporates repository-required scans and per-post leak review.
    • D5 The author signed each commit, the squash included.
      Evidence: Verify plan D5 requires every commit to be signed.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.
      Evidence: Verify plan D6 requires matching documentation or an explanation of no impact.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.
      Evidence: Verify plan D7 requires trailers on every commit and the specified PR label.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
      Evidence: Verify plan D8 requires staying within size limits.
    • D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue.
      Evidence: Verify plan D9 covers filing and Related references, but omits minimum issue content and code links.
    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.
      Evidence: Verify plan D10 explicitly includes all three requirements.

    Round: 1 of 3

  7. tbhb-agent commented on Oct 27, 2026

    @tbhb-agent
    tbhb-agent
    on Oct 27, 2026
    ContributorAuthor

    Refinement: NOT READY — scope still prescribes implementation placement
    Model: OpenAI/GPT-6, medium

    Checked

    Gaps

    1. Moderate — Scope prescribes implementation placement outside Operator authority. “The behavior is delivered in the Go validator” selects the component that must own both diagnostics. The shell freeze and prerequisite are valid constraints, but they do not require placing stamp inspection inside the validator. Remove that placement requirement and leave the mechanism to the PR. Retain the freeze and Port the pull request description validator to Go #718 dependency; record any already-authorized implementation decision under Operator authority. This closes the remaining violation of the requirements-only rule without adding scope or another prerequisite.

    Split

    No split required. The diagnostics fit one PR after #718; restoration and evidence reuse remain separate.

    Definition of done

    The definition of done for #840 (Feature). Tick each item and give its evidence.

    Ticks indicate planning coverage only, not completed work.

    • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link.

      Evidence: Verify plan D1 requires merged-head verification and commands/results for every AC.

    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.

      Evidence: D2 requires exact-head independent approval and finding dispositions; D9 requires deferred findings as issues.

    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.

      Evidence: D3 requires passing checks with no gate loosened; AC4 preserves publication gates.

    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.

      Evidence: D4 incorporates repository-required leak scans and per-post leak review.

    • D5 The author signed each commit, the squash included.

      Evidence: D5 requires every commit to be signed.

    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.

      Evidence: D6 requires matching documentation or an explanation of no impact in Verification.

    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.

      Evidence: D7 requires trailers on every commit and the specified label.

    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.

      Evidence: D8 requires staying within size limits.

    • D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue.

      Evidence: The amended D9 explicitly covers issue content, Related references, and links from deferred-code comments.

    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.

      Evidence: D10 explicitly requires all three outcomes.

    Round: 2 of 3

  8. tbhb-agent commented on Oct 27, 2026

    @tbhb-agent
    tbhb-agent
    on Oct 27, 2026
    ContributorAuthor

    Refinement: READY — the placement constraint is removed and earlier gaps are closed
    Model: OpenAI/GPT-6, medium

    Checked

    • Read the live body and both comments of feat(pr): fail the description check on a missing Compatibility line or grouped AC rows and warn when the base moved #840, its named issues Replace complex shell with tested Go, starting with gates and merging #588 and Port the pull request description validator to Go #718, and the requested comparisons feat(pr): move shared description restoration into Go #795 and Revalidate review evidence after a content-preserving base update #738. Verified the independent clone’s origin identifies tbhb-dev/devtools. Inspected origin/main at 32b9ee2fe14ea1e35350684f84fe93a74d7f9d9c.
    • Round 2 gap closed: Scope now states behavioral outcomes and prohibits adding shell logic. It no longer assigns stamp inspection or either diagnostic to a particular component. Approach explicitly leaves the mechanism to the implementer.
    • Round 1 gap 1 closed: Compatibility enforcement is acknowledged as existing behavior and excluded from this change. Enforcement appears at packages/agents/source/skills/pr/shared/scripts/validate-description.sh:485; positive and negative fixtures appear at tests/skills/pr/scripts/compatibility.bats:53 and tests/skills/pr/scripts/compatibility.bats:64.
    • Round 1 gap 2 closed: AC3 uses the draft’s named base and its locally resolved remote-tracking ref, matching packages/agents/source/skills/pr/shared/scripts/description.sh:93. Review stamps that base commit at packages/agents/source/skills/pr/shared/scripts/review.sh:77; publication compares it at packages/agents/source/skills/pr/shared/scripts/create-pr.sh:162. Recovery includes renewed review. AC5 protects a valid non-default base.
    • Round 1 gap 3 closed: The body honors Replace complex shell with tested Go, starting with gates and merging #588’s freeze and explicitly waits for Port the pull request description validator to Go #718. That prerequisite supplies the migrated validation entry point already reached by check and review at packages/agents/source/skills/pr/shared/scripts/pr-agent.sh:48 and packages/agents/source/skills/pr/shared/scripts/review.sh:67. Its completion is a delivery prerequisite, not implementation authorization.
    • Dependency placement is appropriate: feat(pr): move shared description restoration into Go #795 restores descriptions and explicitly excludes validation, review and staleness decisions. Revalidate review evidence after a content-preserving base update #738 permits carefully constrained evidence reuse after a base update. This issue adds an early warning while preserving refusal; neither comparison issue should absorb that warning or replace Port the pull request description validator to Go #718.
    • Round 1 gap 4 closed: AC1 verification covers all three grouping forms in both tables and bullets, including line numbers and corrective messages. AC2 covers accepted single-AC forms. AC3 covers both entry points, both commits and recovery guidance. AC4 checks preserved publication refusal and statuses. AC5 covers a matching non-default base; AC6 checks clean-draft output and status. D9 now specifies minimum follow-up content and deferred-code links.
    • The requirements describe observable diagnostics and regression behavior without prescribing layout or implementation. Non-functional coverage preserves exit status, unchanged clean output and existing gates. False-positive and wrong-base risks have corresponding checks; reverting the change is a suitable undo.
    • Operator authority remains constrained: no publication, signing or gate-changing authority is granted. The named epic retains owner review for protected-path changes; exact-head owner approval remains required for golden files or gate loosening under AGENTS.md:70. No protection change is required by this body.

    Gaps

    None.

    Split

    No split required. The two early diagnostics fit one focused PR after #718. Migration, description restoration and evidence reuse remain separate work.

    Definition of done

    The definition of done for #840 (Feature). Tick each item and give its evidence.

    Ticks indicate planning coverage only, not completed work.

    • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link.

      Evidence: Verify plan D1 explicitly requires merged-head verification with commands and results for every AC.

    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.

      Evidence: D2 requires independent approval of the exact merged head and recorded dispositions; D9 requires deferred findings as issues.

    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.

      Evidence: D3 requires passing checks with no gate loosened; AC4 explicitly preserves publication gates.

    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.

      Evidence: D4 incorporates repository-required leak scans and requires each post to pass its leak review before posting.

    • D5 The author signed each commit, the squash included.

      Evidence: D5 requires every commit to be signed.

    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.

      Evidence: D6 requires matching documentation or an explanation of no impact in Verification.

    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.

      Evidence: D7 requires trailers on every commit and the specified PR label.

    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.

      Evidence: D8 requires staying within size limits; Size specifies one S-sized PR.

    • D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue.

      Evidence: The amended D9 explicitly requires minimum issue content, Related references and links from comments marking deferred work.

    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.

      Evidence: D10 explicitly requires all three outcomes.

    Round: 3 of 3

  9. tbhb-agent
    added
    refinement:readyPassed the Definition of Ready; may be picked up
    on Oct 27, 2026
  10. tbhb-agent
    marked this as blocked on Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    change:featPR adds or changes a feature (Conventional Commit type feat)refinement:readyPassed the Definition of Ready; may be picked up

    Type

    Feature

    Fields

    Priority

    None yet

    Work type

    None yet

    Projects

    • devtools
      Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions