diff --git a/skills/default/analyze-ci/SKILL.md b/skills/default/analyze-ci/SKILL.md index cd46264ed..d21b5498b 100644 --- a/skills/default/analyze-ci/SKILL.md +++ b/skills/default/analyze-ci/SKILL.md @@ -46,6 +46,7 @@ Analyze downloaded files locally — do not print large log content to the conve - `codegen-outdated` — generated files out of sync with source - `unit-test` — test assertion failures caused by a code bug - `e2e-code-bug` — end-to-end test fails consistently with the same assertion error pointing to a logic defect in the code under test +- `commit-message` — commit title/trailer validation (`check-commits`, commitlint, missing `Signed-off-by`, topic prefix, title length). These require amending existing commit messages, not new file commits. **Not fixable by code change — skip:** - `infra` — CI infrastructure failures (runner unavailable, network timeout, quota exceeded) @@ -64,13 +65,20 @@ Write `.forge/fix-plan.md`: ## Fixable Failures ### {check-name} -**Category**: {compile | lint | format | codegen-outdated | unit-test | e2e-code-bug} +**Category**: {compile | lint | format | codegen-outdated | unit-test | e2e-code-bug | commit-message} **Root Cause**: {exact error or description} **Affected Files**: {list} **Fix**: 1. {exact command or edit} 2. {verification command} +When category is `commit-message`, also include: + +### Amended Commit Message +``` +{full corrected commit message including required trailers} +``` + ## Skipped Failures ### {check-name} diff --git a/skills/default/fix-ci/SKILL.md b/skills/default/fix-ci/SKILL.md index 16e8120dd..25e356058 100644 --- a/skills/default/fix-ci/SKILL.md +++ b/skills/default/fix-ci/SKILL.md @@ -26,6 +26,17 @@ Skip anything listed under **Skipped Failures** — do not attempt to fix them. 1. Commit with a clear message referencing what was fixed 2. Do NOT push — the orchestrator handles that +### Special case: commit-message failures + +If the fix plan category is `commit-message` (or the only failures are commit +title/trailer validation such as `check-commits` / commitlint): + +1. Do **not** create a new commit for message-only fixes +2. Amend HEAD with the corrected message from the plan: + `git commit --amend -m ""` +3. Include required trailers (for example `Signed-off-by`) in that amended message +4. Do NOT push — the orchestrator force-pushes after amend + ## Guidelines - Follow the plan — do not invent additional fixes to the logic diff --git a/src/forge/prompts/v1/fix-ci.md b/src/forge/prompts/v1/fix-ci.md index 475f9b82c..4926d4c6f 100644 --- a/src/forge/prompts/v1/fix-ci.md +++ b/src/forge/prompts/v1/fix-ci.md @@ -5,3 +5,5 @@ Apply the following CI fix plan to the repository. The plan was produced by anal {fix_plan} Follow the fix-ci skill instructions. Apply each fixable failure in order, skip the ones marked as skipped, and commit the changes when done. + +If the plan's category is `commit-message` (commit title/trailer validation only), amend the existing HEAD commit with the corrected message from the plan instead of creating a new commit. Do not push. diff --git a/src/forge/workflow/nodes/ci_evaluator.py b/src/forge/workflow/nodes/ci_evaluator.py index ef7c0605a..ca91281da 100644 --- a/src/forge/workflow/nodes/ci_evaluator.py +++ b/src/forge/workflow/nodes/ci_evaluator.py @@ -2,6 +2,7 @@ import io import logging +import re import zipfile from pathlib import Path from typing import Any @@ -18,6 +19,10 @@ from forge.workflow.nodes.error_handler import notify_error from forge.workflow.nodes.workspace_setup import prepare_workspace from forge.workflow.utils import merge_review_exhaustion, update_state_timestamp +from forge.workflow.utils.commit_message_ci import ( + commit_message_failure_summary, + is_commit_message_formatting_failure, +) from forge.workflow.utils.jira_status import ( post_status_comment, remove_implementing_label, @@ -390,8 +395,18 @@ async def attempt_ci_fix(state: WorkflowState) -> WorkflowState: branch_name = state.get("context", {}).get("branch_name", "") - # Commit any files the container left uncommitted (safety net) - if git.has_uncommitted_changes(): + commit_msg_failure = is_commit_message_formatting_failure(failed_checks) + amended_message = _extract_amended_commit_message(fix_plan) + + # Commit any files the container left uncommitted (safety net). + # Commit-message-only failures need --amend, not a new commit. + if commit_msg_failure: + if amended_message: + git.amend_commit(message=amended_message) + elif git.has_uncommitted_changes(): + # Keep original message if the agent staged code + message fix. + git.amend_commit() + elif git.has_uncommitted_changes(): git.stage_all() git.commit(f"[{ticket_key}] fix: address CI failures (attempt {attempt})") @@ -402,12 +417,27 @@ async def attempt_ci_fix(state: WorkflowState) -> WorkflowState: else: remote_ref = f"origin/{branch_name}" - unpushed = git._run_git( - "log", f"{remote_ref}..HEAD", "--oneline", check=False - ).stdout.strip() + local_sha = git._run_git("rev-parse", "HEAD", check=False).stdout.strip() + remote_sha = git._run_git("rev-parse", remote_ref, check=False).stdout.strip() + head_diverged = bool(local_sha) and local_sha != remote_sha + unpushed = head_diverged if not unpushed: logger.warning(f"Container made no changes for {ticket_key} (attempt {attempt})") + if commit_msg_failure: + # Do not burn the full retry budget on empty commits for + # message-only gates — escalate with an actionable error. + summary = commit_message_failure_summary(failed_checks) + logger.error("Commit-message CI failure unresolved for %s: %s", ticket_key, summary) + return update_state_timestamp( + { + **state, + "ci_status": "failed", + "ci_fix_attempt": state.get("ci_fix_max_attempts", ci_fix_max), + "current_node": "ci_evaluator", + "last_error": summary, + } + ) else: # Only run the expensive review pass when the fix actually changed code _, review_result = await run_post_change_review( @@ -422,12 +452,14 @@ async def attempt_ci_fix(state: WorkflowState) -> WorkflowState: if review_result is not None: state = merge_review_exhaustion(state, review_result, ticket_key, "code_review") - # Push all commits (CI fix + any review corrections) + # Push all commits (CI fix + any review corrections). + # Message rewrites require force-push. + force_push = commit_msg_failure if fork_owner and fork_repo: - git.push_to_fork(force=False) + git.push_to_fork(force=force_push) else: logger.warning("Fork info not in state — pushing to origin instead") - git.push(force=False) + git.push(force=force_push) logger.info(f"CI fix pushed for {ticket_key} (attempt {attempt})") record_ci_fix_attempt(repo=state.get("current_repo", "unknown"), result="pushed") @@ -702,3 +734,28 @@ def _collect_error_info(failed_checks: list[dict[str, Any]]) -> str: parts.append("") return "\n".join(parts) + + +def _extract_amended_commit_message(fix_plan: str) -> str | None: + """Extract a corrected commit message from an analyze-ci fix plan, if present. + + Looks for a fenced or labeled ``Amended Commit Message`` / ``New Commit Message`` + section produced when the failure category is ``commit-message``. + """ + if not fix_plan: + return None + + patterns = ( + r"(?im)^(?:#+\s*)?(?:amended|new|corrected)\s+commit\s+message\s*:?\s*\n+" + r"(?:```(?:text|commit)?\s*\n)?(.+?)(?:\n```|\n\n|\Z)", + r"(?im)^\*\*Amended Commit Message\*\*\s*:?\s*\n+" + r"(?:```(?:text|commit)?\s*\n)?(.+?)(?:\n```|\n\n|\Z)", + ) + + for pattern in patterns: + match = re.search(pattern, fix_plan, re.DOTALL) + if match: + message = match.group(1).strip() + if message: + return message + return None diff --git a/src/forge/workflow/utils/commit_message_ci.py b/src/forge/workflow/utils/commit_message_ci.py new file mode 100644 index 000000000..528516624 --- /dev/null +++ b/src/forge/workflow/utils/commit_message_ci.py @@ -0,0 +1,82 @@ +"""Detect CI failures caused by commit-message formatting rules.""" + +from __future__ import annotations + +import re +from typing import Any + +# Patterns commonly emitted by check-commits / commitlint-style gates. +_COMMIT_MESSAGE_PATTERNS: tuple[re.Pattern[str], ...] = ( + re.compile(r"title is longer than \d+ characters", re.I), + re.compile(r"title lacks a lowercase topic prefix", re.I), + re.compile(r"signed-off-by:.+trailer is missing", re.I), + re.compile(r"missing.*signed-off-by", re.I), + re.compile(r"commit(?:\s+message)?(?:\s+format)?(?:\s+validation)?\s+fail", re.I), + re.compile(r"check-commits", re.I), + re.compile(r"commitlint", re.I), + re.compile(r"conventional commits?", re.I), + re.compile(r"\bgit-commits\b", re.I), + re.compile(r"subject must( not)? be", re.I), +) + +_COMMIT_CHECK_NAME_HINTS = ( + "check-commits", + "git-commits", + "commitlint", + "commit-message", + "commit message", + "conventional-commit", +) + + +def _texts_from_check(check: dict[str, Any]) -> list[str]: + texts = [str(check.get("name") or "")] + output = check.get("output") or {} + if isinstance(output, dict): + for key in ("title", "summary", "text"): + value = output.get(key) + if value: + texts.append(str(value)) + for key in ("error", "message", "details"): + value = check.get(key) + if value: + texts.append(str(value)) + return texts + + +def is_commit_message_formatting_failure(failed_checks: list[dict[str, Any]]) -> bool: + """Return True when every failed check looks like commit-message validation. + + Empty input is False. Mixed code + commit-message failures return False so + normal code-fix retries still run. + """ + if not failed_checks: + return False + + for check in failed_checks: + texts = _texts_from_check(check) + blob = "\n".join(texts) + name = str(check.get("name") or "").lower() + name_hint = any(hint in name for hint in _COMMIT_CHECK_NAME_HINTS) + pattern_hit = any(pat.search(blob) for pat in _COMMIT_MESSAGE_PATTERNS) + if not (name_hint or pattern_hit): + return False + return True + + +def commit_message_failure_summary(failed_checks: list[dict[str, Any]]) -> str: + """Build a short operator-facing summary of commit-message CI failures.""" + snippets: list[str] = [] + for check in failed_checks: + name = check.get("name") or "commit check" + output = check.get("output") or {} + detail = "" + if isinstance(output, dict): + detail = str(output.get("summary") or output.get("title") or output.get("text") or "") + detail = detail.strip().splitlines()[0] if detail.strip() else "commit message formatting" + snippets.append(f"{name}: {detail[:200]}") + joined = "; ".join(snippets) if snippets else "commit message formatting" + return ( + "CI failed due to commit message format — please amend the commit message " + f"to match repository conventions ({joined})" + ) diff --git a/src/forge/workspace/git_ops.py b/src/forge/workspace/git_ops.py index 625164efe..99d617818 100644 --- a/src/forge/workspace/git_ops.py +++ b/src/forge/workspace/git_ops.py @@ -262,6 +262,68 @@ def commit(self, message: str, author_name: str = "Forge") -> bool: logger.info(f"Committed: {message[:50]}...") return True + def amend_commit( + self, + message: str | None = None, + author_name: str = "Forge", + ) -> bool: + """Amend HEAD, optionally rewriting the commit message. + + When ``message`` is provided, HEAD is rewritten even if the tree is + unchanged (commit-message-only CI fixes). When ``message`` is None, + returns False if there are no user-facing changes to fold in. + + Returns: + True if HEAD was amended, False otherwise. + """ + result = self._run_git("status", "--porcelain", check=False) + if not result.stdout.strip() and message is None: + logger.info("Nothing to amend") + return False + + self.stage_all() + staged = self._run_git("diff", "--cached", "--name-only", check=False) + if not staged.stdout.strip() and message is None: + logger.info("Nothing to amend after staging") + return False + + # Allow message-only rewrites with an empty index. + if not staged.stdout.strip() and message is not None: + self._run_git( + "-c", + f"user.name={self.settings.git_user_name}", + "-c", + f"user.email={self.settings.git_user_email}", + "commit", + "--amend", + "--allow-empty", + "-m", + message, + "--author", + f"{author_name} <{self.settings.git_user_email}>", + ) + logger.info("Amended HEAD commit message: %s...", message[:50]) + return True + + args = [ + "-c", + f"user.name={self.settings.git_user_name}", + "-c", + f"user.email={self.settings.git_user_email}", + "commit", + "--amend", + "--author", + f"{author_name} <{self.settings.git_user_email}>", + ] + if message is None: + args.append("--no-edit") + else: + args.extend(["-m", message]) + + self._run_git(*args) + logger.info("Amended HEAD commit%s", "" if message is None else f": {message[:50]}...") + return True + def remote_branch_exists(self, branch_name: str, remote: str = "origin") -> bool: """Check whether a branch exists on the given remote. diff --git a/tests/unit/workflow/utils/test_commit_message_ci.py b/tests/unit/workflow/utils/test_commit_message_ci.py new file mode 100644 index 000000000..52a3d8a13 --- /dev/null +++ b/tests/unit/workflow/utils/test_commit_message_ci.py @@ -0,0 +1,66 @@ +"""Unit tests for commit-message CI failure detection.""" + +from forge.workflow.utils.commit_message_ci import ( + commit_message_failure_summary, + is_commit_message_formatting_failure, +) + + +def test_detects_check_commits_style_errors(): + checks = [ + { + "name": "git-commits", + "output": { + "text": ( + "./check-commits abc..def\n" + "error [1/2] title is longer than 72 characters, please make it shorter\n" + "error [1/2] title lacks a lowercase topic prefix (e.g. 'ipv6:')\n" + "error [1/2] 'Signed-off-by: Forge ' trailer is missing\n" + ) + }, + } + ] + assert is_commit_message_formatting_failure(checks) is True + + +def test_mixed_failures_are_not_commit_message_only(): + checks = [ + {"name": "git-commits", "output": {"text": "title lacks a lowercase topic prefix"}}, + {"name": "unit-tests", "output": {"text": "AssertionError: expected 1"}}, + ] + assert is_commit_message_formatting_failure(checks) is False + + +def test_empty_checks_false(): + assert is_commit_message_formatting_failure([]) is False + + +def test_summary_mentions_amend(): + summary = commit_message_failure_summary( + [{"name": "check-commits", "output": {"summary": "Signed-off-by trailer is missing"}}] + ) + assert "amend the commit message" in summary + assert "check-commits" in summary + + +def test_extract_amended_commit_message_from_plan(): + from forge.workflow.nodes.ci_evaluator import _extract_amended_commit_message + + plan = """# CI Fix Plan + +## Fixable Failures + +### git-commits +**Category**: commit-message + +### Amended Commit Message +``` +openflow: fix drain pending msgs + +Signed-off-by: Forge +``` +""" + msg = _extract_amended_commit_message(plan) + assert msg is not None + assert msg.startswith("openflow: fix drain pending msgs") + assert "Signed-off-by:" in msg