Skip to content

feat: enforce complete PR communication - #4

Merged
bingb0t5 merged 9 commits into
mainfrom
fm/fm-pr-comms-gate
Aug 24, 2026
Merged

bingb0t5 merged 9 commits into
mainfrom
fm/fm-pr-comms-gate

Conversation

@bingb0t5

@bingb0t5 bingb0t5 commented Aug 24, 2026 •

Copy link
Copy Markdown
Owner

CEO overview

  • What is changing: Firstmate now checks every pull request description for the same four-part explanation already required in the Lalo repos: what is changing, why it matters, customer or business impact, and risk and rollout. Until now this repo had no such check, so a description could skip that explanation and still merge.
  • Why it matters: A missing explanation is blocked automatically before merge, the same way it already is in the Lalo repos. Rich can review each change in plain language instead of reconstructing intent from a code-only summary.
  • Customer or business impact: No customer-facing product change. This only changes how engineers and agents write pull request descriptions in this repository, so every change is explained clearly enough to review before it ships.
  • Risk and rollout: Very low risk. This adds one GitHub Actions check and a pull request template. It does not change application behavior, stored data, or existing required tests. If the new check had a bug, the worst case is a complete description being blocked, which is easy to notice and fix.

What changed technically

Added the proven Lalo/Beanz PR communication checker: vendored assessor, drift pin, workflow, tests, and pull request template. An untouched template no longer satisfies Module-boundary decision. Transient network and 5xx remote source failures fall back to the local pin with a warning. The introducing PR uses pull_request so the new checker can run before it exists on the default branch.

Validation

  • Checks passed: Local executable checker fail and pass, untouched-template rejection, local pin, transient remote fallback, required-remote fail-closed, lint, and repository audience check. Live GitHub on feat: enforce complete PR communication #4 showed Require no-mistakes passing and pr-communication failing on this non-compliant description (run 32720666105) with the missing CEO-overview section list.
  • Checks not run: Did not merge this pull request. Upstream fork Actions on kunchenguid/firstmate remain approval-gated.
  • Evidence and limitations: Red proof is the live pr-communication failure on PR 4, not only a local CLI transcript. This edit is the matching green attempt.

Module-boundary decision

Not applicable: this is a CI workflow and vendored checker addition; no product module was created, moved, or extracted.

Decision needed

No decision required.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 error
  • 🚨 scripts/pr-communication/check-drift.mjs:90 - The required constraint says, "Do not add a forgotten secret that makes drift self-compare," while the accepted fallback is limited to transient network/5xx failures. Here, missing or invalid private-repository credentials produce 401/403/404, but authFailure is treated like a transient failure and the check exits successfully using the PR-editable vendored file plus PR-editable SOURCE.sha256. A same-repo PR can therefore change both files and still pass whenever PR_COMMUNICATION_SOT_TOKEN is absent or loses access. Fail closed on authentication/not-found responses, reserving local-pin fallback for network, 408/429, and 5xx failures unless remote verification is explicitly disabled by an accepted policy.
⚠️ **Test** - 1 warning
  • ⚠️ .github/workflows/pr-communication.yml:17 - Required real-CI red-then-green proof is incomplete. GitHub PR feat: enforce complete PR communication #4 has only one pr-communication run, which failed with MODULE_NOT_FOUND on the earlier base-branch checkout implementation. Target commit 3de9707 is not yet on the remote PR, and there is no real-CI failure caused by a non-compliant description followed by a passing compliant description. The outer pipeline must push the target, then perform and preserve that description-edit red-then-green sequence.
  • tests/pr-communication.test.sh
  • Direct CLI rejection using PR_TITLE='WIP' and a non-compliant PR_BODY
  • Direct CLI acceptance using a fully compliant CEO overview, validation, module-boundary decision, and decision-needed body
  • gh-axi run list --repo=bingb0t5/firstmate --workflow pr-communication.yml --limit 20
  • gh-axi run view 32719166367 -R bingb0t5/firstmate --log-failed
  • git status --short after evidence collection
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

kunchenguid and others added 9 commits August 24, 2026 02:33
Vendor the lalo-admin assessor already proven on mrbeanz-brains, with the
same drift pin and live SoT comparison, so firstmate PRs cannot skip the
required overview, decision, module-boundary, and validation sections.
Checking out the base branch cannot execute the new checker until it
lands, so restore pull_request and assess the proposed head instead.
@bingb0t5
bingb0t5 merged commit 7b1a356 into main Aug 24, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants