Skip to content

feat: implement issue #2237 — template-drift AHEAD detection: two narrow misclassifications and three untested paths left open by PR 2219 - #2244

Merged
don-petry merged 1 commit into
mainfrom
dev-lead/issue-2237-20261011-0053
Oct 11, 2026
Merged

don-petry merged 1 commit into
mainfrom
dev-lead/issue-2237-20261011-0053

Conversation

@don-petry

@don-petry don-petry commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

template-drift AHEAD detection: two narrow misclassifications and three untested paths left open by PR 2219

From the issue: PR #2219 ("template-drift classifies a template ahead of the standards channel as AHEAD, not DRIFTED", issue #1926) merged with two reviewer findings unanswered and three test gaps the qa-lead advisory listed. None blocked the merge. This issue tracks them.

Risk

Low — changes automation shell logic under scripts/, covered by shellcheck (--severity=warning) and the bats suite.

Test plan

Tests added/updated: tests/template_stub_drift.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

This PR's Lint (shellcheck) and bats checks show pass/fail; watch subsequent dev-lead / pr-review runs for behavioral regressions.

Closes #2237

View guided diff Turn on auto-fix

…row misclassifications and three untested paths left open by PR 2219
@don-petry
don-petry requested a review from a team as a code owner October 11, 2026 00:59
@don-petry don-petry added the auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label) label Oct 11, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

This comment has been minimized.

@codeant-ai

codeant-ai Bot commented Oct 11, 2026

Copy link
Copy Markdown

CodeAnt PR Risk: Low Risk

  • The PR appears safe to merge; the ancestry check prevents non-ancestor standards refs from being mislabeled as newer than HEAD.
  • Added Bats tests cover ahead, behind, diverged, and unreadable comparisons, plus the intermediate-commit guidance for drifted files.

Assessed commit: aad65d4e3568

@sonarqubecloud

Copy link
Copy Markdown

@don-petry

Copy link
Copy Markdown
Collaborator Author

QA Lead — test-risk advisory

Risk tier: LOW. This changes a lint-time guard in CI automation, and every new path is covered by deterministic, stubbed bats cases. The worst a miss can do is send someone to the wrong re-seed step. It can't touch production data.

What I'd shore up (highest leverage first):

  • Pin the compare argument order. Every driver test stubs _template_drift_compare out, and the two unit tests use a fake gh that ignores its arguments. If someone swaps N...HEAD to HEAD...N, ahead and behind trade places, which brings back the misclassification this PR fixes, and the suite would still pass. Have the fake gh log its arguments, then assert compare/${DRV_N_SHA}...${DRV_HEAD_SHA}.
  • Add the mirror of the AC Go-live improvements for PR review agent #2 driver test. In the diverged/behind cases, assert that the DRIFTED message is the bare re-seed remedy and that it does not contain STANDARDS_REF=<commit>. That shows head_sha="" actually clears the new elif branch. Also add the missing ::notice:: assertion to the behind case.
  • Make "makes no call" true. The empty-baseline _template_drift_compare test only checks for empty output. Its name says no call is made, so have the fake gh write to a call log and assert the log is empty, as the local-seam test already does with DRV_HEAD_LOG.
  • Optional: add one driver case for identical. It is probably unreachable once the SHAs differ, but a single assertion keeps a future refactor of that branch honest.

All three AC #3 tests are present and match the issue's wording. The suite has no live-network or timing dependencies, so I see no flakiness risk.

Escalate? no

Advisory only. I comment; I do not change code. Ground: BMAD Test Architecture (bmad-tea v1.19.0). To opt out on this item, add the qa-lead:hands-off label.

@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-11T02:08:07Z.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge October 11, 2026 01:09
@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@don-petry
don-petry disabled auto-merge October 11, 2026 01:38
@don-petry

Copy link
Copy Markdown
Collaborator Author

This is a CodeRabbit OSS review rate-limit notice — the bot has exhausted its free review quota for this public repository. All PR CI checks pass, and no code changes are needed.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0 (rate-limit notice only)
Files changed: None
Disposition: informational
This was a CodeRabbit OSS review rate-limit notice with no code findings. The comment indicates the bot exhausted its free review quota, but contains no Security Architecture Review, actionable comments, or other finding-bearing sections. All 48 CI checks pass, and no code changes are needed. Dispositioned as informational per the rate-limit guidance.
```

@don-petry
don-petry enabled auto-merge October 11, 2026 01:39
@don-petry
don-petry merged commit 82461dc into main Oct 11, 2026
58 of 60 checks passed
@don-petry
don-petry deleted the dev-lead/issue-2237-20261011-0053 branch October 11, 2026 03:17
don-petry added a commit that referenced this pull request Oct 11, 2026
…and refuse downgrades (#1925) (#2256)

* feat: implement issue #2237 — template-drift AHEAD detection: two narrow misclassifications and three untested paths left open by PR 2219

* feat: seed PRs record their standards ref, supersede older seed PRs, and refuse downgrades (#1925)

seed-repo-template.sh opened seed PRs that did not say which standards ref they
came from, left older seed PRs open, and could overwrite a template file with an
older version of itself.

- Provenance: the seed PR body states the resolved standards ref, how it was
  chosen and the commit. A reused PR's body is rewritten on each run.
- Supersede: #2233 already updates an open seed PR in place. When several are
  open the newest is kept and each older one is now closed with a comment
  linking it. A PR that cannot be closed fails the run.
- No downgrades: a file that differs from the run's emission but matches the
  emission at standards HEAD is refused, with "standards channel is behind the
  template — promote standards". If nothing else needs writing, no PR is opened
  and the empty branch is removed.
- A newer-commit check that cannot be made aborts the seed. It is never read as
  "not ahead".
- docs/release/seeding-repo-template.md is the operator procedure.

The no-downgrade check reuses the drift guard's HEAD and ancestry reads, so this
branch includes the unmerged #2244 (issue #2237) that adds the ancestry read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(review): restore a pre-existing dated seed branch, and list every open PR

- If today's seed branch already exists it may carry an earlier run's edits. It
  is now handled like an open seed PR's branch: every file is made to match what
  this run wants. Before, a file this run skipped, preserved or refused kept the
  earlier run's content, so a refused downgrade could still ship.
- The open-PR listing limit is applied before the seed-branch filter, so it is
  raised from 100 to 1000. A seed PR cut off by the limit would be missed and a
  second one opened.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: seed-repo-template.sh has one home, this repo (#1927)

The standards repo's same-named script is a different tool and is being renamed
to emit-standard-file.sh in petry-projects/.github#1294.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(review): stop on an unreadable PR list, fetch at the recorded commit, and test the real ancestry path

- If the open pull requests cannot be listed, the seed stops before it writes
  anything. Before, a failed listing read as "no seed PR open", which opens a
  duplicate and leaves the older ones open.
- A seed run now fetches every standards file at the commit it records in the PR
  body, not through the ref name. A channel that moved mid-run could otherwise
  leave the PR naming a commit its content did not come from.
- A dated branch left by an earlier run, with nothing written and nothing the
  base lacks, opens no PR.
- Docs: a channel that cannot be resolved falls back to HEAD and the operator
  stops; a local STANDARDS_DIR has no commit and no downgrade check, so it is
  not for a real seed.
- Tests: three run the real HEAD read, the real ancestry read and the real
  child emission with only gh replaced. Others cover the unreadable PR list, a
  seed PR beyond the first 100 open PRs, the commit pin, a failed provenance
  update, and a PR number that cannot be read back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

template-drift AHEAD detection: two narrow misclassifications and three untested paths left open by PR 2219

2 participants