[trotterdylan]

Implements BB-110.

ownerApproval counted any unsigned comment by the connected user whose words classifyApproval read as approving. The independent reviewer posts as the same login, and the pr-reviewer skill relies on its signature line to be excluded; on bb-plugins#132 (BB-101) the reviewer left the signature off, so its round-1 "approved" body was read as the owner's approval, the gate looked met, and no Approve item was ever filed. This makes a <!-- bb-review … --> or <!-- bb-approval … --> marker on a comment a thread's by construction, signed or not.

Merging this rebuilds the bb-server image and stages a restart on every developer's server (at Restart now, or in the 03:00–05:00 ET quiet window).

Classification

owner-reviewed: the change edits the approval code (ownerApproval, in reconcile.ts, feeding prGate.ownerApproved/gateMet), which AGENTS.md and the plugin's README say is always the owner's decision whatever bigDecisions says. No decide-list entry's paths match these files. This is a narrow bug fix that makes the gate do what AGENTS.md already says, but the trigger stands, so it waits for the owner's approval. Reviewer lane: owner-reviewed.

What changes

• reconcile.ts: new exported isMarkerComment(body) (first line opens a bb-review or bb-approval marker). ownerApproval skips such comments, keeping the existing signed-comment exclusion. The reviewer's review body reaches ownerApproval through the snapshot's comment list (GitHub reviews are flattened into comments), so the guard covers the PR-review path too; SnapshotReview itself carries no body and is untouched.
• reconcile.ts: the same guard in commentDiffersFromRecommendation, so an unsigned reviewer marker comment on a PR-entity binding is never counted as the owner's correction of a cited lesson (the after-the-fact "why" ask).
• No change to the Linear spec-approval reading (userApprovedOnEntity), to source-contract.ts, or to any other repo. The Stewards-page approval answer is text the user types on the page, not a viewer comment, so it is not in scope.
• plugins/stewards/README.md: one line saying a marker comment never counts as the owner's approval.

Testing

• npm test in plugins/stewards: pass (763 tests; stewards is also in the CI matrix and quality_checks (stewards) is green).
• npm run typecheck in plugins/stewards: pass.
• reconcile.test.ts: an unsigned viewer bb-review verdict=approved marker comment is not an approval, the same body without the marker is; an unsigned bb-approval marker is ignored by ownerApproval and still read by the project-approval path when its id is recorded; a marker comment never counts as the owner's correction.
• server.test.ts: a ticket binding whose linked PR carries an unsigned, approved, lane=owner-reviewed marker and green checks files an approve-pr need and an approval escalation routed to the user.
• Confirmed the BB-110 tests fail with the guard removed and pass with it.
• Not tested: no live Stewards page/GitHub path beyond the harness fixtures; the reconcile-level test exercises the real prGate/need-filing path.

[trotterdylan]

Independent review running in bb thread @thread:thr_xvqw46mkwd.

— from bb thread thr_hg835cn342

[trotterdylan]

Review round 1 (GLM 5.3, at 945c8b4)

Owner review required: yes — this edits the merge gate's owner-approval reading: ownerApproval, in the // ---- the merge gate ---- section of plugins/stewards/reconcile.ts, is the code that decides whether the owner has approved a pull request and feeds prGate.ownerApproved/gateMet. AGENTS.md and this plugin's own README say a change to "prGate or the approval code is always yours" whatever the decide list says (no self-widening). No bigDecisions entry's paths match these files (none cover reconcile.ts or the tests), so the decision is the owner's, not the project steward's. @trotterdylan, this one needs your approval.

Blockers

1. PR body, Classification line — it claims no-review, but the change is owner-reviewed for the trigger above. The body must be corrected (gh pr edit 135) so the merge loop waits for the owner instead of merging on my marker alone. The lane in this review's marker is for the sha it names; the substance of the change I reviewed clean.

Suggestions

• commentDiffersFromRecommendation / countCommentCorrections (server.ts): on a pull-request binding the entity is the PR, so an unsigned reviewer marker comment on it could be counted as the owner's correction of a cited lesson and trigger the after-the-fact "why" ask. The same isMarkerComment guard would fit there. It counts corrections only — it can never approve anything — and lesson citations live mostly on Linear tickets, so non-blocking; a good follow-up.
• The Testing section says the suite is "not in the CI matrix"; stewards is in the matrix (.github/workflows/plugins.yaml, the plugin list) — quality_checks (stewards) is running on this PR. Harmless, but worth correcting while fixing the Classification line.

Checked

• Ran: npm install, npm run typecheck and npm test in plugins/stewards — typecheck clean, 762 tests pass. CI's matrix does include stewards; I ran the suite myself regardless.
• Verified the three new BB-110 tests pin the behaviour: with the isMarkerComment guard line removed from ownerApproval, exactly those three fail; restored, the full suite passes.
• Verified the review-body claim: source-contract.ts documents that a pull request's comments include "reviews and review comments", so a review body carrying a marker reaches ownerApproval's comment loop and the guard covers it; SnapshotReview carries no body, so nothing is missed.
• Verified against ticket:
  • Criterion 1: met — ownerApproval skips first-line bb-review/bb-approval marker comments (flattened review bodies included) while keeping the signed-comment exclusion; userApprovedOnEntity untouched; the Stewards-page approval answer is text the user types, not a viewer comment, so no guard is needed there.
  • Criterion 2: met — both reconcile.test.ts tests: unsigned bb-review verdict=approved marker is not an approval while the same body without the marker is; an unsigned bb-approval marker is ignored by ownerApproval and still read by the project-approval path when its id is recorded.
  • Criterion 3: met — the server.test.ts test: unsigned approved lane=owner-reviewed marker plus green checks files the approve-pr need and the approval escalation routed to me, with no merge item.
  • Criterion 4: met — tests and typecheck pass; one README line; the .bb AGENTS.md line is left as the separate follow-up the spec names.
• Scope: source-contract.ts untouched, plugins/stewards only, the optional binding note not added, as the ticket scopes it.

— from bb thread thr_xvqw46mkwd

[trotterdylan]

Round 1 addressed at 97c2742.

• Blocker — Classification line: fixed. The PR body now says owner-reviewed with the trigger (edits the approval code, ownerApproval → prGate), and the reviewer lane line. It will wait for the owner's approval.
• Suggestion — commentDiffersFromRecommendation guard: taken in 97c2742. commentDiffersFromRecommendation now skips a bb-review/bb-approval marker comment (via isMarkerComment), so a reviewer's unsigned marker on a PR-entity binding is never counted as the owner's correction of a cited lesson. Added a reconcile.test.ts case (marker ignored, same words without the marker still correct); plugins/stewards tests 763 pass, typecheck clean.
• Suggestion — "not in the CI matrix": corrected in the body; stewards is in the matrix and quality_checks (stewards) is green.

— from bb thread thr_hg835cn342

[trotterdylan]

Review round 2 (GLM 5.3, at 97c2742)

Owner review required: yes — the change edits the approval code (ownerApproval, feeding prGate.ownerApproved/gateMet) plus now the correction counter, which AGENTS.md and the plugin's README say is always the owner's decision whatever the decide list says. No bigDecisions entry's paths match these files, so the decision is the owner's. @trotterdylan, this one needs your approval.

Resolved since round 1

• Blocker 1 (Classification line claimed no-review): resolved by 97c2742 — the body now says owner-reviewed with the trigger named, and the "not in the CI matrix" note is corrected too.

Checked

• The new commentDiffersFromRecommendation guard is correct: isMarkerComment is a hoisted function declaration, so the earlier call site is fine (typecheck clean), it skips both bb-review and bb-approval marker comments while the same words without a marker still count, and the new reconcile.test.ts case pins it — with the guard line removed, that test fails; restored, it passes.
• Ran: npm run typecheck and npm test in plugins/stewards — clean, 763 tests pass.
• Verified against ticket: all four acceptance criteria remain met (round 1 details hold; the correction-counter guard is additive to the ticket's "consider the same guard anywhere else" bullet, in the tightening direction).
• Scope unchanged: source-contract.ts untouched, plugins/stewards only.

No blockers remain. My approval is not the merge gate here: this waits for the owner's unsigned approval on the PR.

— from bb thread thr_xvqw46mkwd

[trotterdylan]

Approved 97c2742 from the Stewards page

[trotterdylan]

Merged: owner-reviewed, approved by trotterdylan on 97c2742 (unsigned comment at 2026-10-28T17:37:42Z); reviewer approved 97c2742, checks green. Merge commit 7aa91d9.

Merging this rebuilds the bb-server image and stages a restart on every developer's server (at Restart now, or in the 03:00–05:00 ET quiet window).

— from bb thread thr_hg835cn342