Skip to content

feat: add /rebase and /rebaseWithConflicts commands to cloudflare-docs-bot#32121

Open
mvvmm wants to merge 8 commits into
productionfrom
feat/flue-rebase-commands
Open

feat: add /rebase and /rebaseWithConflicts commands to cloudflare-docs-bot#32121
mvvmm wants to merge 8 commits into
productionfrom
feat/flue-rebase-commands

Conversation

@mvvmm

@mvvmm mvvmm commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds /rebase and /rebaseWithConflicts slash commands to the cloudflare-docs-bot (.flue/). Both are codeowner-only and post status updates at the top of the bot's singleton comment throughout the operation.

/rebase — Rebases the PR branch against production using the GitHub update-branch API. Stops with an explanation if:

  • The PR targets a branch other than production
  • The PR is from a fork (can't push)
  • Conflicts are detected

On a clean rebase, posts a success status and triggers a /full-review automatically (since the head SHA changed).

/rebaseWithConflicts — Same as /rebase, but on conflict it attempts AI-assisted resolution:

  • Identifies files changed on both sides since the merge base
  • Fetches both versions of each conflicting file
  • Calls Workers AI (kimi-k2.7-code) with both file contents + PR intent to produce resolved versions and a confidence score
  • High confidence: applies via Git Data API (blob → tree → commit → force-push), then triggers /full-review
  • Medium/low confidence: stops and posts an explanation of why it couldn't resolve automatically

Other changes:

  • postOrUpdateComment extracted from the two workflow files that duplicated it into lib/code-review-render.ts as a shared export — covers the create-if-absent case in one place
  • renderRebaseStatusUpdate() added to the render lib — injects/replaces the rebase status block at the top of the bot comment, preserving existing review content below
  • New FlueRebaseWorkflow Durable Object class added (wrangler.jsonc v9 migration)
  • Commands table in the bot comment updated to list the two new commands

Documentation checklist

@github-actions

Copy link
Copy Markdown
Contributor

This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:

Pattern Owners
*.ts @cloudflare/content-engineering, @kodster28
* @cloudflare/product-owners

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

@mvvmm
mvvmm marked this pull request as ready for review July 16, 2026 21:54
@mvvmm
mvvmm requested review from a team and kodster28 as code owners July 16, 2026 21:54
@cloudflare-docs-bot

cloudflare-docs-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review

⚠️ 2 warnings found in commit b81c255.

Fix in your agent
Fix the following review findings in PR #32121 (https://github.com/cloudflare/cloudflare-docs/pull/32121).

Before making changes, review each finding and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g. the fix requires disproportionate effort for minimal benefit,
  or the finding is factually incorrect), explain why
- If you need clarification before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

After triaging, fix all legitimate findings. For any you decide to skip,
post a comment on this PR with the finding ID and your reasoning.

---

## Code Review

### Warnings (2)

#### CR-2c7d207f2e79 · Nondeterministic deduplication from concurrent updates
- **File:** `.flue/workflows/rebase.ts` line 1050
- **Issue:** The dedup logic at lines 1045-1053 uses last-write-wins based on the order of `treeUpdates`, but the array was populated by `Promise.all(...map(async ... treeUpdates.push(...)))`, so callback completion order (and thus array order) is nondeterministic. The comment itself acknowledges the order is nondeterministic, yet relies on it to pick the winning entry. Two updates for the same path (e.g. a conflict file whose resolution path overlaps another changed file) may therefore produce different trees across runs.
- **Fix:** Build updates deterministically, e.g. iterate `allPrFiles` serially with `for...of` and `await`, or collect updates into a `Map<string, TreeUpdate>` as each callback finishes so winners are chosen by explicit rule rather than push order.

#### CR-4d73854f1bc7 · Production rename detection misses dual-rename case
- **File:** `.flue/workflows/rebase.ts` line 699
- **Issue:** `isProductionRename` is `meta.productionReadPath !== path && !prFiles.find((f) => f.path === path)?.previousPath`. In case 4 (PR renamed A→B and production renamed A→C), the conflict candidate path is B, which has `previousPath` A, so the second clause is false and `isProductionRename` stays false. The resulting prompt at line 749 then labels production's content at C as "the original path before this PR renamed it" instead of production's new path, misleading the AI about which side did the rename.
- **Fix:** Derive `isProductionRename` from the metadata, e.g. `meta.productionReadPath !== meta.baseReadPath`. This is true whenever production renamed the base path (cases 3 and 4) and false otherwise (cases 1 and 2).

Code Review

This code review is in beta and may not always be helpful — use your judgment.

Warnings (2)
File Issue
.flue/workflows/rebase.ts line 1050 Nondeterministic deduplication from concurrent updates — The dedup logic at lines 1045-1053 uses last-write-wins based on the order of treeUpdates, but the array was populated by Promise.all(...map(async ... treeUpdates.push(...))), so callback completion order (and thus array order) is nondeterministic. The comment itself acknowledges the order is nondeterministic, yet relies on it to pick the winning entry. Two updates for the same path (e.g. a conflict file whose resolution path overlaps another changed file) may therefore produce different trees across runs. Fix: Build updates deterministically, e.g. iterate allPrFiles serially with for...of and await, or collect updates into a Map<string, TreeUpdate> as each callback finishes so winners are chosen by explicit rule rather than push order.
.flue/workflows/rebase.ts line 699 Production rename detection misses dual-rename caseisProductionRename is meta.productionReadPath !== path && !prFiles.find((f) => f.path === path)?.previousPath. In case 4 (PR renamed A→B and production renamed A→C), the conflict candidate path is B, which has previousPath A, so the second clause is false and isProductionRename stays false. The resulting prompt at line 749 then labels production's content at C as "the original path before this PR renamed it" instead of production's new path, misleading the AI about which side did the rename. Fix: Derive isProductionRename from the metadata, e.g. meta.productionReadPath !== meta.baseReadPath. This is true whenever production renamed the base path (cases 3 and 4) and false otherwise (cases 1 and 2).

Conventions

No convention issues found.

Style Guide Review

No style-guide issues found.

Commands

Only codeowners can run commands. Post a comment with the command to trigger it.

Command Description
/review Runs a review now. Incremental if a prior review exists, full if not.
/full-review Re-reviews the entire PR diff from scratch, ignoring incremental history. Useful after a rebase, when you want a fresh review, or if the bot gets out of sync and reports issues that no longer exist.
/ignore-review-limit Permanently lifts the 2-review automatic limit for this PR. Future pushes will trigger reviews as normal.
/disable-auto-review Stops automatic reviews from triggering on future pushes to this PR. Codeowners can still run /review or /full-review manually.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

@mvvmm
mvvmm force-pushed the feat/flue-rebase-commands branch from c7fa096 to 1cffd2a Compare July 17, 2026 04:06
@mvvmm

mvvmm commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

CR-e2cb3858a8bc — Acknowledged, intentional.

/rebaseWithConflicts creates a single synthetic commit because the AI resolves all conflicts at once against the production HEAD tree. Replaying individual PR commits would require cherry-picking each one onto production and resolving conflicts per-commit, which is significantly more complex and doesn't add value for a docs repo where commits are rarely meaningful units of history. The squash behavior is an acceptable tradeoff and will be documented in the workflow file comment.

@mvvmm

mvvmm commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

CR-c6138101439e — Disagree, disproportionate.

The detail parameter is placed into a Markdown > blockquote (for halted-conflict and halted-confidence) or a plain prose sentence (for failed). In both cases the only character that is structurally dangerous is a newline, which sanitizeRebaseDetail already strips. Backtick, *, _, etc. render literally inside a blockquote or prose sentence — they don't break the comment structure. The halted-wrong-base case wraps the branch name in a backtick code span, which is correct; a backtick in a branch name would technically break that span, but branch names containing backticks are not valid Git refs and cannot exist in practice.

@mvvmm
mvvmm force-pushed the feat/flue-rebase-commands branch from 1cffd2a to 9dd8749 Compare July 17, 2026 14:33
@mvvmm

mvvmm commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

CR-b9524d294f46 — Documented, not fixed.

Detecting whether the new head is a result of the async rebase (vs. a concurrent push) would require verifying that the new head is on top of the base ref via a compare call, adding a round-trip for a race that requires a push to land in the 60 s polling window. That's disproportionate. A concurrent push invalidates the rebase anyway — the author's push would need to be rebased as well — so treating it as "complete" has no meaningful downside: the subsequent /full-review runs against whatever head SHA is current, which is correct behavior.

Added a JSDoc note to pollForBranchUpdate documenting the limitation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants