Skip to content

land: accept a bot clean-review comment (naming the current head SHA) as the silence signal — fn-65#177

Merged
gmickel merged 4 commits into
mainfrom
fn-65-land-accept-a-bot-reviewed-clean
Jun 17, 2026
Merged

land: accept a bot clean-review comment (naming the current head SHA) as the silence signal — fn-65#177
gmickel merged 4 commits into
mainfrom
fn-65-land-accept-a-bot-reviewed-clean

Conversation

@gmickel

@gmickel gmickel commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Spec: fn-65-land-accept-a-bot-reviewed-clean · Plan-review: SHIP (rp, 3 rounds) · Completion-review: SHIP (rp) · Found dogfooding the fn-64 land.

What & why

/flow-next:land's silence merge signal needs "an automated review of the current head." But land detected reviews only via the formal reviews API — and Codex (the bot silence was built for) files a formal review only when it has findings. On a clean pass it posts an issue comment ("Didn't find any major issues. Reviewed commit <sha>") that never hits the reviews API. So a converged-clean PR could sit at NEEDS_HUMAN forever — which is exactly what happened landing fn-64 (PR #176); a human had to merge on judgment.

This teaches silence to also accept a bot's clean-review comment that names the current head SHA as head-current review evidence — closing the blind spot for the default bot-review flow.

R-ID coverage

R-ID What Where
R1 Head-current automated-reviewer clean comment satisfies silence workflow.md §2.6 comment scan
R2 Stale-SHA or no-SHA comment never satisfies — explicit token extraction ([0-9a-fA-F]{7,40}, ≥7, non-empty, prefix-match), no ==$var*-on-empty footgun workflow.md §2.6
R3 Non-automated login ignored (reuses the [bot]/automatedReviewers allowlist) workflow.md §2.6
R4 Never bypasses an unresolved thread or red CI; scan is a read-only GET (dry-run-safe) workflow.md §2.6
R5 Config contract: seeded structured built-in ERE; null→default, explicit ""→disabled; silence-gated; only ever sets AUTO_REVIEW_CURRENT flowctl.py get_default_config() (+ dogfood copy)
R6 Docs: flowctl.md row + flow-next.dev land.mdx (3 spots) + changelog docs/flowctl.md, flow-next.dev
R7 Observability: AUTO_REVIEW_SOURCE=comment + AUTO_REVIEW_EVIDENCE in dry-run/report workflow.md §2.6

Critical changes — where to look

  • plugins/flow-next/skills/flow-next-land/workflow.md §2.6 — the comment scan: paginated issues/<n>/comments, gated on REVIEW_SIGNAL == silence, runs after the reviews loop and before the draft-trigger check; allowlist + pattern + head-current-SHA-token match; sets AUTO_REVIEW_CURRENT=1 (never resets). SHA preferentially read from the Reviewed commit marker line.
  • plugins/flow-next/scripts/flowctl.py (+ dogfood .flow/bin/flowctl.py, lockstep) — land.cleanReviewCommentPattern seeded as a structured ERE; the cfg read distinguishes null (→ built-in default) from explicit "" (→ disabled) — the real off-switch.
  • plugins/flow-next/tests/test_land_config.py — +16 tests (default present, round-trip, explicit-empty-disables distinct from default, no-clobber both directions, docstring) plus a static §2.6 invariant guard (--paginate, silence-gate, before-draft-trigger ordering, SHA non-empty/min-length, set-only-never-reset, null≠empty).
  • Codex mirror regenerated; flowctl.md + flow-next.dev land.mdx + changelog; plugin 2.1.0 → 2.1.1 (additive/opt-in patch). flow-next.dev pushed separately (gmickel/flow-next.dev@57f61da).

Decision context

silence was built for "bots that comment but never APPROVE," yet its detection only read formal reviews — so the feature and its implementation disagreed on exactly the bot it targets. The fix stays conservative: allowlist-gated, head-SHA-anchored (a fix push still forces a fresh clean comment), additive-only (never overrides a formal review / open thread / red CI), and disable-able via empty pattern.

Verification

  • Full suite green (python3 -m unittest discover -s plugins/flow-next/tests); ci_test.sh 67/67; standalone behavioral bash matrix (head-current clean ✓, stale-SHA ✗, no-SHA ✗, non-bot ✗, empty-pattern ✗).
  • Plan-review (RepoPrompt): SHIP after 3 rounds — caught the ==$var*-on-empty footgun and the un-disableable-opt-out config bug.
  • Completion-review (RepoPrompt): SHIP — all R1–R7 Met.

🤖 Generated with Claude Code via /flow-next:make-pr

gmickel added 4 commits June 17, 2026 10:30
…e signal

- workflow.md §2.6: silence-gated, paginated issues/<n>/comments scan after
  the reviews-API loop and before the draft-trigger check; sets
  AUTO_REVIEW_CURRENT=1 on (automated-reviewer allowlist) ∧ (clean-review
  pattern) ∧ (head-current SHA token, empty/min-length guarded)
- never resets the reviews-API result; observability via AUTO_REVIEW_SOURCE=
  comment + AUTO_REVIEW_EVIDENCE in --dry-run + verdict report
- flowctl.py (canonical + dogfood .flow/bin) seed land.cleanReviewCommentPattern
  structured ERE; contract: null/missing→built-in default, explicit ""→disabled
- SKILL.md prose: comment-path supplement + empty-disables contract
- test_land_config.py: +16 tests (default present, round-trip, explicit-empty-
  disables distinct from default, no-clobber, docstring, structured-ERE,
  real-codex-comment behavioral; static §2.6 assertions — paginate, silence
  gate, before-draft-trigger, SHA non-empty/min-length guard, null≠empty cfg)
- Codex mirror regenerated

Task: fn-65-land-accept-a-bot-reviewed-clean.1
…p 2.1.1

- flowctl.md: new land config row, default shown as built-in ERE, empty disables comment scan
- CHANGELOG: 2.1.1 Added entry citing fn-65
- version bump 2.1.0 -> 2.1.1 (patch, additive/opt-in) across all manifests + README badge

Task: fn-65-land-accept-a-bot-reviewed-clean.2
@gmickel gmickel marked this pull request as ready for review June 17, 2026 09:22
@gmickel

gmickel commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: d56d5f80c0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@gmickel gmickel merged commit 4cada36 into main Jun 17, 2026
4 checks passed
@gmickel gmickel deleted the fn-65-land-accept-a-bot-reviewed-clean branch June 17, 2026 10:11
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.

1 participant