From d9d1ca87edbe111afe661f5057c3cb63f64e8efc Mon Sep 17 00:00:00 2001 From: Emerick Rogul Date: Mon, 13 Apr 2026 11:57:54 -0400 Subject: [PATCH] Create initial rebasing-related Claude commands (#35428) --- .claude/commands/rebasing/fix-broken-test.md | 167 ++++++++++++++++++ .claude/commands/rebasing/fix-wip.md | 126 +++++++++++++ .../rebasing/generate-qa-test-plan.md | 106 +++++++++++ tools/cr/find_rebase_commits.py | 162 +++++++++++++++++ 4 files changed, 561 insertions(+) create mode 100644 .claude/commands/rebasing/fix-broken-test.md create mode 100644 .claude/commands/rebasing/fix-wip.md create mode 100644 .claude/commands/rebasing/generate-qa-test-plan.md create mode 100755 tools/cr/find_rebase_commits.py diff --git a/.claude/commands/rebasing/fix-broken-test.md b/.claude/commands/rebasing/fix-broken-test.md new file mode 100644 index 00000000000..1ccb4e20ab3 --- /dev/null +++ b/.claude/commands/rebasing/fix-broken-test.md @@ -0,0 +1,167 @@ +--- +name: fix-broken-test +description: + Diagnose and fix a broken Brave unit or browser test with minimal, + review-ready changes +--- + +Goal: + +- Identify why a Brave unit test or browser test is failing. +- Determine whether the failure is caused by Brave changes or upstream Chromium + changes. +- Implement the smallest possible clean fix. +- Verify by rebuilding and rerunning the test. +- Produce a clear, descriptive commit message referencing root cause and + upstream changes (if applicable). + +Inputs: + +- Test name or test suite + +Phases: + +1. Build and reproduce failure + +- Build Brave and the Brave unit tests or browser tests (whichever is + applicable) +- Reuse existing out directories when valid; avoid full clean builds unless + necessary +- Build the relevant test target (e.g., brave_unit_tests, brave_browser_tests) +- Run the specified test using --gtest_filter with verbose output and stack + traces enabled +- Capture: + - Full stack trace + - Assertion failures + - Logs and stderr output +- If test is flaky, run multiple times to confirm reproducibility + +2. Analyze failure + +- Identify: + - Exact failing assertion or crash site + - File and line number of failure + - Call stack leading to failure +- Classify failure type: + - Assertion mismatch + - Null/invalid pointer + - API mismatch + - Behavior change + - Timing / async issue +- Determine the _first point of divergence_ from expected behavior + +3. Check recent Brave changes + +- Inspect recent commits (last ~1-2 weeks) affecting: + - The failing test + - Related components/files in the stack trace +- Use `git blame` on failing lines +- Identify any Brave-specific patches that could explain: + - API changes + - Behavior overrides + - Feature flags or guards +- If a likely culprit is found: + - Validate by reviewing diff and intent + +4. Check recent upstream Chromium changes + +- Use git log on corresponding Chromium paths +- Identify corresponding upstream files/modules +- Review upstream commits from the past several weeks affecting: + - Same files + - APIs used in the failing stack +- Look for: + - Signature changes + - Behavior changes + - Test expectation updates +- Compare Brave code vs upstream to detect divergence + +5. Determine root cause + +- Decide whether the failure is due to: + - Brave regression + - Upstream API/behavior change + - Test becoming outdated + - Integration mismatch +- Clearly identify: + - What changed + - Why the test now fails + +6. Implement minimal fix + +- Apply the smallest possible change that: + - Fixes the root cause + - Preserves intended behavior +- Prefer: + - Updating Brave adaptation layer over patching upstream code + - Adjusting test expectations only if behavior change is correct +- Avoid: + - Broad refactors + - Unnecessary formatting or unrelated edits +- Keep diff minimal and localized +- Prefer aligning Brave behavior with upstream unless Brave intentionally + diverges + +7. Validate fix + +- Rebuild affected targets +- Rerun the failing test +- Confirm: + - Test passes consistently + - No new failures introduced in related tests or same test binary + +8. Commit changes + +- Create a single clean commit +- Commit message format: + + : Fix failing + + Problem: + + - Brief description of failure and symptom + + Root cause: + + - Explanation of what caused the issue + - Reference Brave or upstream change + + Fix: + + - Description of minimal change made + + Notes: + + - Mention if behavior aligns with upstream + - Include upstream commit hash, CL, or bug link if identifiable + +- Ensure commit is review-ready: + - No debug code + - No unrelated changes + - Clear and concise message + +Heuristics: + +- If failure is due to API mismatch -> likely upstream change +- If failure involves brave/ or chromium_src/ code -> likely Brave regression +- If test expectations differ from upstream -> check upstream test updates first +- If null/segfault -> prioritize ownership and lifetime changes in recent + commits + +Constraints: + +- Do not modify upstream files unless absolutely necessary (prefer chromium_src + overrides or minimal patching if required) +- Preserve existing architecture and intent +- Do not guess root cause; if unclear, gather more evidence before modifying + code +- Avoid speculative fixes; ground decisions in evidence from stack trace and + commits +- Keep changes as small and surgical as possible + +Anti-patterns to avoid: + +- Blindly updating test expectations without confirming correct behavior +- Adding sleeps/timeouts to fix timing issues without root cause +- Overriding upstream behavior unnecessarily +- Large refactors unrelated to the failure diff --git a/.claude/commands/rebasing/fix-wip.md b/.claude/commands/rebasing/fix-wip.md new file mode 100644 index 00000000000..1f6880f2b0d --- /dev/null +++ b/.claude/commands/rebasing/fix-wip.md @@ -0,0 +1,126 @@ +--- +name: fix-wip +description: Fix a single WIP commit according to project rules +--- + +Goal: + +- Complete a WIP commit so it is review-ready +- Ensure minimal diffs and preserve intent +- Ensure that code compiles and tests pass + +Input: + +- Commit (SHA or reference) + +Phases: + +1. Inspect commit + +- Identify WIP indicators in commit message (e.g., "WIP", "tmp", "temp", + "debug", "DO NOT SUBMIT") +- Inspect commit message and commit +- Inspect upstream commit message and upstream commit only if explicitly + provided +- If no WIP indicators are present and no incomplete work is detected and the + commit appears complete and compliant: + - Do not modify it + - Report as "skipped" + +2. Identify and make change + +- If an upstream commit is explicitly provided: + - Confirm whether it caused the issue +- Otherwise: + - Do not infer or invent an upstream cause +- Only treat an upstream commit as identifiable if directly referenced in the + commit or provided context +- If upstream commit is relevant: + - Align behavior with upstream intent + - Do not diverge unless necessary +- Apply the smallest possible diff that fully resolves the issue +- Do not produce partial fixes that leave the code in a broken or inconsistent + state +- Do not modify code outside the direct scope of the problem +- Avoid renaming, reformatting, or moving code unless required +- Preserve functionality +- Prefer adapting existing code over introducing new patterns +- Do not refactor unless required to fix the issue +- Do not create new files unless strictly required to fix the issue + +3. Build and test + +- Ensure that project builds successfully using the project's standard build + commands +- Ensure that all tests pass using the project's standard test commands +- Run build before tests +- If build fails: + - Attempt minimal fixes + - If minimal fixes do not resolve the issue, report failure + - Do not proceed to tests if build fails +- Only proceed to commit rewrite if build and tests both succeed + +4. Rewrite commit message + +- Remove WIP marker from commit subject +- Ensure message follows conventional commit / project style +- Use imperative voice (e.g., "Fix crash in X") +- Be concise but descriptive +- Preserve author and timestamp +- Preserve the original intent of the commit message while improving clarity +- Add "Note: AI-assisted: requires thorough review" to end of commit message + +5. Output + +- Status: success | skipped | failed +- Change type: code fix | test fix | build fix | no-op +- Approximate size of change (e.g., lines added/removed) +- Confidence: high | medium | low +- New commit SHA (if modified) +- Files changed (in the final amended commit) +- Summary of changes made +- One-line commit subject +- 2-3 sentence commit description +- Reference relevant upstream commits if applicable + +Assumptions: + +- The repository is in a clean working state +- Required build tools and dependencies are available +- Do not attempt to fix unrelated environment or infrastructure issues + +Commit constraints: + +- Modify the existing commit in-place using --fixup=amend, do not create + additional commits unless absolutely necessary +- Do not reorder commits +- Do not squash with other commits +- Do not perform an interactive rebase + +Execution: + +- Indicate which commit is being processed +- Print progress at each phase +- Do not repeatedly attempt fixes beyond a small number of iterations +- Stop execution immediately after a failure is reported +- Report success, skip, or failure explicitly + +Rules: + +- Do not modify unrelated files +- Do not introduce new dependencies +- Do not modify tests unless they are clearly incorrect or outdated +- Keep diffs minimal and reviewable +- Never lose test coverage or public API intent +- Prefer fixing code over relaxing tests + +Safety: + +- Do not perform push, force-push, or any remote operations +- Do not modify branches outside the specified commit + +Failure handling: + +- If the issue cannot be confidently resolved: + - Do not modify the commit + - Output a clear explanation of the blocker diff --git a/.claude/commands/rebasing/generate-qa-test-plan.md b/.claude/commands/rebasing/generate-qa-test-plan.md new file mode 100644 index 00000000000..d130c3a29ac --- /dev/null +++ b/.claude/commands/rebasing/generate-qa-test-plan.md @@ -0,0 +1,106 @@ +--- +name: generate-qa-test-plan +description: Generate QA test checklist from current Chromium rebase commits +--- + +Goal: + +- Identify commits related to the current Chromium rebase +- Extract user-visible or behavior-impacting changes +- Convert them into actionable QA test items +- Group results into Desktop, iOS, and Android sections +- Output clean Markdown + +Steps: + +1. Run commit discovery script + +- Execute: python3 find_rebase_commits.py --verbose +- Capture: + - Commit range (first line) + - Full verbose commit list (hash, author, subject) + +2. Parse commits + +- Ignore: + - Pure refactors + - Formatting changes + - Test-only changes unless they imply behavior change +- Prioritize: + - UI changes + - Networking / privacy / security + - Permissions (Bluetooth, camera, etc.) + - Platform-specific code paths (ios/, android/, chrome/, browser/) + - Feature flags and toggles + - Crash fixes + - Performance changes + +3. Infer platform relevance + +- Desktop: + - Default bucket for most changes + - Anything in browser/, chrome/, components/, ui/ +- iOS: + - Commit mentions ios or has an "[ios]" tag (with our without brackets) + - Paths containing ios/ + - Mentions of WKWebView, UIKit, iOS APIs +- Android: + - Commit mentions Android or has an "[Android]" tag (with or without brackets) + - Paths containing android/ + - Mentions of JNI, Java, Android permissions, SDK +- If unclear: + - Include in Desktop + - Optionally duplicate in all platforms if clearly cross-platform + +4. Convert commits to QA test items For each important/relevant commit: + +- Rewrite subject into a test instruction: + - Start with a verb: "Verify", "Test", "Ensure" +- Focus on observable behavior, not implementation +- Expand slightly if needed for clarity + +Example: + +- "Fix WebBluetooth permission prompt not showing" -> "Verify WebBluetooth + permission prompt appears when a site requests access" + +5. Deduplicate and merge + +- Combine similar commits into a single QA item +- Keep wording concise + +6. Output format (Markdown) + +Output EXACTLY: + +## Rebase QA Checklist + +**Commit Range:** + +### Desktop + +- ... +- ... + +### iOS + +- ... +- ... + +### Android + +- ... +- ... + +7. Constraints + +- Do not include commit hashes in final output +- Do not include authors +- Do not include internal jargon +- Keep each bullet to one line when possible +- Prefer clarity over completeness + +8. Optional enhancement + +- If no obvious QA impact: Output: + - "No significant user-facing changes detected" diff --git a/tools/cr/find_rebase_commits.py b/tools/cr/find_rebase_commits.py new file mode 100755 index 00000000000..33275429c84 --- /dev/null +++ b/tools/cr/find_rebase_commits.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2026 The Brave Authors. All rights reserved. +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at https://mozilla.org/MPL/2.0/. +""" +find_rebase_commits.py + +Purpose: + Scan a git repository to determine a range of commits associated with a + Chromium rebase number (e.g., cr148) in commit subjects. Outputs a commit + range in `oldest..newest` format suitable for git operations. + +Usage: + python find_rebase_commits.py --tag cr148 [--start ] [--verbose] + python find_rebase_commits.py [--start ] [--verbose] # auto-detect rebase number + +Arguments: + --start Starting commit (default: HEAD) + --tag Rebase number to look for (e.g., cr123). If omitted, + automatically detects the most frequent rebase number in the + first N commits. + --verbose Include commit hash, author, and truncated subject in output. +""" + +import subprocess +import argparse +import re +import sys +from collections import Counter + +MAX_LOOKAHEAD_NO_CR = 20 # commits to scan if rebase number not supplied +MAX_EMPTY_COMMITS = 20 # stop after this many commits without match +TRUNCATE_SUBJECT_LEN = 80 # max length of commit subject in verbose mode +CR_REGEX = r"cr\d{3,}" # pattern to detect rebase numbers + + +def run_git_log(start_ref, max_count=None): + # Fetch commits with UTF-8 decoding, replace invalid bytes + cmd = ["git", "-c", "i18n.logOutputEncoding=utf-8", "log"] + if max_count is not None: + cmd.append(f"-n{max_count}") + cmd.append("--pretty=format:%H%x01%an%x01%s") + cmd.append(start_ref) + + try: + output = subprocess.check_output(cmd, + encoding="utf-8", + errors="replace") + except subprocess.CalledProcessError as e: + print("Error running git log:", e, file=sys.stderr) + sys.exit(1) + + commits = [] + for line in output.splitlines(): + parts = line.split("\x01", 2) + if len(parts) == 3: + commits.append((parts[0], parts[1], parts[2])) + return commits + + +def find_most_frequent_cr(commits): + # Pick the most frequently occurring rebase number in commit subjects + cr_pattern = re.compile(CR_REGEX) + cr_list = [] + for _, _, subject in commits: + match = cr_pattern.search(subject) + if match: + cr_list.append(match.group(0)) + if not cr_list: + return None + counter = Counter(cr_list) + most_common_cr, _ = counter.most_common(1)[0] + return most_common_cr + + +def find_commit_range(commits, cr_id): + # Walk commits until MAX_EMPTY_COMMITS consecutive commits do not match + # rebase number + cr_regex = re.compile(cr_id) + last_match_index = None + empty_count = 0 + + for i, (_, _, subject) in enumerate(commits): + if cr_regex.search(subject): + last_match_index = i + empty_count = 0 + else: + empty_count += 1 + + if last_match_index is not None and empty_count >= MAX_EMPTY_COMMITS: + break + + if last_match_index is None: + return [] + + return commits[:last_match_index + 1] + + +def truncate(text, max_len=TRUNCATE_SUBJECT_LEN): + # Truncate commit subjects for verbose output + if len(text) <= max_len: + return text + return text[:max_len - 3] + "..." + + +def main(): + parser = argparse.ArgumentParser( + description="Find commit range by rebase number.") + parser.add_argument("--start", + help="Starting commit (default: HEAD)", + default="HEAD") + parser.add_argument( + "--tag", help="Rebase number (e.g., cr123). Auto-detect if omitted.") + parser.add_argument("--verbose", + action="store_true", + help="Show commit details") + args = parser.parse_args() + + auto_detected = False + + if args.tag is None: + # auto-detect rebase number from first N commits + first_commits = run_git_log(args.start, max_count=MAX_LOOKAHEAD_NO_CR) + cr_id = find_most_frequent_cr(first_commits) + if cr_id is None: + print( + f"No rebase number found in first {MAX_LOOKAHEAD_NO_CR} commits.", + file=sys.stderr) + sys.exit(1) + auto_detected = True + else: + cr_id = args.tag + + commits = run_git_log(args.start) + commit_range = find_commit_range(commits, cr_id) + + if not commit_range: + print(f"No commits found matching rebase number {cr_id}.") + sys.exit(0) + + # commits are newest -> oldest; git range is oldest..newest + newest = commit_range[0][0] + oldest = commit_range[-1][0] + + print(f"{oldest}..{newest}") + + if args.verbose: + if auto_detected: + print(f"\nAuto-detected rebase number: {cr_id}") + else: + print(f"\nUsing rebase number: {cr_id}") + + print("\nCommits:") + for commit_hash, author, subject in commit_range: + short_hash = commit_hash[:10] + print(f"{short_hash} {author:20} {truncate(subject)}") + + +if __name__ == "__main__": + main()