From 3e72eaed527cfaf820fe1f69fad709e93802ee35 Mon Sep 17 00:00:00 2001 From: Herbert Damker <52109189+hdamker@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:47:35 +0200 Subject: [PATCH] refactor(ra): drop the release_plan_change trigger path and its dead outputs --- .../workflows/release-automation-reusable.yml | 30 ----- .../docs/technical-architecture.md | 9 +- release_automation/scripts/bot_context.py | 9 +- release_automation/scripts/github_client.py | 22 +--- release_automation/scripts/trigger.py | 59 ++-------- .../scripts/workflow_context.py | 2 - .../bot_messages/config_drift_warning.md | 25 ----- .../templates/bot_messages/issue_created.md | 3 - release_automation/tests/test_bot_context.py | 21 +--- .../tests/test_bot_responder.py | 31 +++--- .../tests/test_command_policy.py | 1 - .../tests/test_github_client.py | 21 ---- .../tests/test_template_context_contract.py | 46 +------- release_automation/tests/test_trigger.py | 104 ++++-------------- 14 files changed, 55 insertions(+), 328 deletions(-) delete mode 100644 release_automation/templates/bot_messages/config_drift_warning.md diff --git a/.github/workflows/release-automation-reusable.yml b/.github/workflows/release-automation-reusable.yml index a35dae2c..10b59d24 100644 --- a/.github/workflows/release-automation-reusable.yml +++ b/.github/workflows/release-automation-reusable.yml @@ -56,8 +56,6 @@ jobs: user: ${{ steps.detect.outputs.user }} should_continue: ${{ steps.detect.outputs.should_continue }} confirm_tag: ${{ steps.detect.outputs.confirm_tag }} - trigger_pr_number: ${{ steps.detect.outputs.trigger_pr_number }} - trigger_pr_url: ${{ steps.detect.outputs.trigger_pr_url }} trigger: ${{ steps.detect.outputs.trigger }} release_tag: ${{ steps.state.outputs.release_tag }} state: ${{ steps.state.outputs.state }} @@ -173,8 +171,6 @@ jobs: id: detect if: steps.fork-guard.outputs.issues_enabled == 'true' working-directory: _tooling - env: - GITHUB_TOKEN: ${{ github.token }} run: python -m release_automation.scripts.trigger - name: Derive Release State @@ -267,8 +263,6 @@ jobs: CTX_IDENTITY_CONSENT_MANAGEMENT_RELEASE: ${{ steps.state.outputs.identity_consent_management_release }} CTX_COMMON_CACHE_STATUS: ${{ steps.cache-sync.outputs.common_cache_status }} CTX_COMMON_CACHE_DETAILS: ${{ steps.cache-sync.outputs.common_cache_details }} - CTX_TRIGGER_PR_NUMBER: ${{ steps.detect.outputs.trigger_pr_number }} - CTX_TRIGGER_PR_URL: ${{ steps.detect.outputs.trigger_pr_url }} GITHUB_TOKEN: ${{ github.token }} # ── Response: one bot comment, template chosen by the policy ── @@ -1893,35 +1887,11 @@ jobs: { "state": "planned", "trigger_type": "${{ needs.plan.outputs.trigger_type }}", - "trigger_pr_number": "${{ needs.plan.outputs.trigger_pr_number }}", - "trigger_pr_url": "${{ needs.plan.outputs.trigger_pr_url }}", "release_plan_url": "${{ github.server_url }}/${{ github.repository }}/blob/main/release-plan.yaml", "closed_issue_number": "${{ github.event.issue.number }}", "closed_issue_url": "${{ github.event.issue.html_url }}" } - - name: Post Config Drift Warning - if: | - needs.plan.outputs.trigger_type == 'release_plan_change' && - needs.plan.outputs.release_issue_number != '' && - (needs.plan.outputs.state == 'snapshot-active' || - needs.plan.outputs.state == 'draft-ready') - uses: ./_tooling/shared-actions/post-bot-comment - with: - issue_number: ${{ needs.plan.outputs.release_issue_number }} - release_tag: ${{ needs.plan.outputs.release_tag }} - run_id: ${{ github.run_id }} - template: config_drift_warning - base_context: ${{ needs.plan.outputs.base_context }} - github_token: ${{ steps.app-token.outputs.token || github.token }} - context: | - { - "trigger_type": "release_plan_change", - "trigger_pr_number": "${{ needs.plan.outputs.trigger_pr_number }}", - "trigger_pr_url": "${{ needs.plan.outputs.trigger_pr_url }}", - "release_plan_url": "${{ github.server_url }}/${{ github.repository }}/blob/main/release-plan.yaml" - } - - name: Post Common Sync PR Created if: | steps.sync-pr.outputs.sync_status == 'created' && diff --git a/release_automation/docs/technical-architecture.md b/release_automation/docs/technical-architecture.md index a9852c7b..3ed6bb65 100644 --- a/release_automation/docs/technical-architecture.md +++ b/release_automation/docs/technical-architecture.md @@ -53,7 +53,6 @@ tooling/ │ ├── templates/ │ │ ├── bot_messages/ │ │ │ ├── command_rejected.md -│ │ │ ├── config_drift_warning.md │ │ │ ├── config_error.md │ │ │ ├── draft_created.md │ │ │ ├── draft_revoked.md @@ -271,7 +270,6 @@ Renders bot messages from Mustache templates using pystache with strict mode (`m | `interim_processing.md` | Immediate feedback for slash commands being processed | | `command_rejected.md` | Command not allowed in current state or insufficient permissions | | `config_error.md` | Invalid `release-plan.yaml` (missing, malformed, or incomplete) | -| `config_drift_warning.md` | `release-plan.yaml` changed on main while snapshot is active | | `snapshot_created.md` | Snapshot and Release PR created successfully | | `snapshot_failed.md` | Snapshot creation failed | | `snapshot_discarded.md` | Snapshot discarded by user | @@ -363,7 +361,6 @@ All context variables are defined in a single `BotContext` dataclass (`bot_conte | `command` | str | `""` | check-trigger | | `command_args` | str | `""` | check-trigger | | `user` | str | `""` | check-trigger | -| `trigger_pr_number` | str | `""` | check-trigger | | **State fields** | | | | | `release_tag` | str | `""` | derive-state | | `state` | str | `""` | derive-state | @@ -806,7 +803,7 @@ stateDiagram-v2 - Auto-reopen only applies when snapshot or draft is active (prevents orphaned artifacts) - NOT_PLANNED is derived from `target_release_type: none`, not from issue closure -**Config drift warning:** When `release-plan.yaml` changes on main while a snapshot is active, the automation posts a `config_drift_warning` message on the Release Issue. The snapshot remains valid — the warning is informational, alerting maintainers that the plan has diverged from the active snapshot. +**Plan changes during a snapshot:** An active snapshot is not affected by a later `release-plan.yaml` edit on main — it carries its own `release-metadata.yaml`. Validation blocks `release-plan.yaml` edits while a snapshot is active, and only `/create-snapshot` is gated on the common-file cache check. ### 5.3 Context Data Flow @@ -829,7 +826,7 @@ release-metadata.yaml (snapshot branch, if exists) │ ▼ ┌──────────────────────┐ - │ assemble-context │◄── check-trigger outputs (command, user, trigger_pr_number) + │ assemble-context │◄── check-trigger outputs (command, user, trigger_type) │ └─ build_context() │◄── derive-state outputs (all above) │ │◄── command action outputs (release_pr_url, draft_release_url, ...) │ │◄── workflow env (workflow_run_url) @@ -933,7 +930,7 @@ fi | No issue | Valid plan (type ≠ none) | Pass | Create issue (PLANNED) | | PLANNED | Plan updated | Pass | Update issue | | PLANNED | type → none | Pass | Set NOT_PLANNED, suggest close | -| SNAPSHOT_ACTIVE | Any change | **Block (exit 1)** | Config drift warning posted | +| SNAPSHOT_ACTIVE | Any change | **Block (exit 1)** | — | | DRAFT_READY | Any change | **Block (exit 1)** | — | | PUBLISHED | Any change | Pass (new release cycle) | Create new issue if type ≠ none | diff --git a/release_automation/scripts/bot_context.py b/release_automation/scripts/bot_context.py index 849afe13..c1816f84 100644 --- a/release_automation/scripts/bot_context.py +++ b/release_automation/scripts/bot_context.py @@ -25,9 +25,7 @@ class BotContext: command: str = "" command_args: str = "" user: str = "" - trigger_pr_number: str = "" - trigger_type: str = "" # "workflow_dispatch", "issue_close", "release_plan_change" - trigger_pr_url: str = "" + trigger_type: str = "" # "slash_command", "workflow_dispatch", "push", "issue_close", ... # Issue creation fields (for issue_created template) closed_issue_number: str = "" @@ -72,7 +70,6 @@ class BotContext: state_published: bool = False trigger_workflow_dispatch: bool = False trigger_issue_close: bool = False - trigger_release_plan_change: bool = False has_meta_release: bool = False has_reason: bool = False @@ -115,7 +112,6 @@ def derive_flags(self) -> None: self.state_published = self.state == config.STATE_PUBLISHED self.trigger_workflow_dispatch = self.trigger_type == "workflow_dispatch" self.trigger_issue_close = self.trigger_type == "issue_close" - self.trigger_release_plan_change = self.trigger_type == "release_plan_change" self.has_meta_release = bool(self.meta_release) self.has_reason = bool(self.reason) self.has_sync_pr = bool(self.sync_pr_url) @@ -142,9 +138,7 @@ def to_dict(self) -> Dict[str, Any]: "command": self.command, "command_args": self.command_args, "user": self.user, - "trigger_pr_number": self.trigger_pr_number, "trigger_type": self.trigger_type, - "trigger_pr_url": self.trigger_pr_url, # Issue creation fields "closed_issue_number": self.closed_issue_number, "closed_issue_url": self.closed_issue_url, @@ -181,7 +175,6 @@ def to_dict(self) -> Dict[str, Any]: "state_published": self.state_published, "trigger_workflow_dispatch": self.trigger_workflow_dispatch, "trigger_issue_close": self.trigger_issue_close, - "trigger_release_plan_change": self.trigger_release_plan_change, "has_meta_release": self.has_meta_release, "has_reason": self.has_reason, # Display fields diff --git a/release_automation/scripts/github_client.py b/release_automation/scripts/github_client.py index d54541d4..9e1a66eb 100644 --- a/release_automation/scripts/github_client.py +++ b/release_automation/scripts/github_client.py @@ -11,7 +11,7 @@ import subprocess import time from dataclasses import dataclass -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional import yaml @@ -562,26 +562,6 @@ def find_pr_for_branch(self, head_branch: str) -> Optional[int]: except (GitHubClientError, ValueError): return None - def find_merged_pr_for_commit(self, sha: str) -> Optional[Tuple[int, str]]: - """ - Find the merged PR a commit belongs to (a push event carries no PR data). - - Args: - sha: Commit SHA, usually the push head commit - - Returns: - (PR number, PR URL) of the first merged PR, or None if there is - none or the lookup fails - """ - try: - output = self._run_gh(["api", f"repos/{self.repo}/commits/{sha}/pulls"]) - for pr in json.loads(output or "[]"): - if pr.get("merged_at"): - return pr["number"], pr["html_url"] - except (GitHubClientError, ValueError) as e: - print(f"Warning: Could not look up PR: {e}") - return None - # ------------------------------------------------------------------------- # Retry helper for eventual consistency # ------------------------------------------------------------------------- diff --git a/release_automation/scripts/trigger.py b/release_automation/scripts/trigger.py index c10948f4..3d7ec6a5 100644 --- a/release_automation/scripts/trigger.py +++ b/release_automation/scripts/trigger.py @@ -4,8 +4,7 @@ Turns the event that started the workflow into a Trigger: which path the workflow takes (slash command, issue event, PR merge, dispatch, push) and the command it carries. parse_trigger() is a pure function of the event name and -the payload at GITHUB_EVENT_PATH; the one network lookup the push path needs -(the PR a commit was merged from) is done by main() through GitHubClient. +the payload at GITHUB_EVENT_PATH. Usage (workflow step): python3 -m release_automation.scripts.trigger @@ -17,9 +16,9 @@ import sys import uuid from dataclasses import asdict, dataclass -from typing import Any, Dict, Optional, Tuple +from typing import Any, Dict -from .config import RELEASE_PLAN_FILE, SNAPSHOT_BRANCH_PREFIX +from .config import SNAPSHOT_BRANCH_PREFIX RELEASE_ISSUE_LABEL = "release-issue" @@ -52,7 +51,7 @@ class Trigger: Attributes: trigger_type: slash_command | issue_close | issue_reopen | pr_merge | - workflow_dispatch | release_plan_change | push | unknown; empty + workflow_dispatch | push | unknown; empty when the event concerns no Release Issue command: Slash command name without the slash, or sync-issue for dispatch and push @@ -62,7 +61,6 @@ class Trigger: user: Commenter, issue author, PR merger or actor, by event should_continue: Whether the workflow proceeds past check-trigger confirm_tag: Tag given with /publish-release --confirm - head_sha: Commit a push event points at (for the merged-PR lookup) """ trigger_type: str = "" command: str = "" @@ -71,7 +69,6 @@ class Trigger: user: str = "" should_continue: bool = False confirm_tag: str = "" - head_sha: str = "" def _is_release_issue(issue: Dict[str, Any]) -> bool: @@ -142,28 +139,10 @@ def _parse_pull_request(payload: Dict[str, Any]) -> Trigger: return Trigger(trigger_type=TRIGGER_UNKNOWN) -def _parse_push(payload: Dict[str, Any], actor: str, sha: str) -> Trigger: - changed = { - path - for commit in payload.get("commits") or [] - for key in ("added", "modified", "removed") - for path in commit.get(key) or [] - } - head_commit = payload.get("head_commit") or {} - return Trigger( - trigger_type="release_plan_change" if RELEASE_PLAN_FILE in changed else "push", - command="sync-issue", - user=actor, - should_continue=True, - head_sha=head_commit.get("id") or sha, - ) - - def parse_trigger( event_name: str, payload: Dict[str, Any], actor: str = "", - sha: str = "", ) -> Trigger: """ Classify an event. @@ -172,7 +151,6 @@ def parse_trigger( event_name: GITHUB_EVENT_NAME (the caller's event in a reusable workflow) payload: Parsed JSON at GITHUB_EVENT_PATH actor: GITHUB_ACTOR, used as user for dispatch and push - sha: GITHUB_SHA, fallback head commit for push Returns: Trigger; should_continue is False for events the workflow ignores @@ -183,16 +161,14 @@ def parse_trigger( return _parse_issue_event(payload) if event_name == "pull_request": return _parse_pull_request(payload) - if event_name == "workflow_dispatch": - # Dispatch only ever syncs the Release Issue; the command input is ignored. + if event_name in ("workflow_dispatch", "push"): + # Both only ever sync the Release Issue; the dispatch command input is ignored. return Trigger( - trigger_type="workflow_dispatch", + trigger_type=event_name, command="sync-issue", user=actor, should_continue=True, ) - if event_name == "push": - return _parse_push(payload, actor, sha) return Trigger(trigger_type=TRIGGER_UNKNOWN) @@ -204,18 +180,15 @@ def write_outputs(path: str, outputs: Dict[str, str]) -> None: f.write(f"{name}<<{delimiter}\n{value}\n{delimiter}\n") -def to_outputs(trigger: Trigger, merged_pr: Optional[Tuple[int, str]]) -> Dict[str, str]: +def to_outputs(trigger: Trigger) -> Dict[str, str]: """String outputs under the check-trigger job's output names, plus the Trigger as JSON.""" outputs = asdict(trigger) - del outputs["head_sha"] outputs["trigger"] = json.dumps(asdict(trigger)) outputs["should_continue"] = "true" if trigger.should_continue else "false" - outputs["trigger_pr_number"] = str(merged_pr[0]) if merged_pr else "" - outputs["trigger_pr_url"] = merged_pr[1] if merged_pr else "" return outputs -def main(client=None) -> int: +def main() -> int: event_name = os.environ["GITHUB_EVENT_NAME"] with open(os.environ["GITHUB_EVENT_PATH"], encoding="utf-8") as f: payload = json.load(f) @@ -224,21 +197,9 @@ def main(client=None) -> int: event_name, payload, actor=os.environ.get("GITHUB_ACTOR", ""), - sha=os.environ.get("GITHUB_SHA", ""), ) - merged_pr = None - if event_name == "push": - if client is None: - from .github_client import GitHubClient - client = GitHubClient( - repo=os.environ["GITHUB_REPOSITORY"], token=os.environ.get("GITHUB_TOKEN") - ) - merged_pr = client.find_merged_pr_for_commit(trigger.head_sha) - if merged_pr: - print(f"Found merged PR: #{merged_pr[0]}") - - outputs = to_outputs(trigger, merged_pr) + outputs = to_outputs(trigger) print(f"Event: {event_name}") print(f"Action: {payload.get('action') or 'N/A'}") for name, value in outputs.items(): diff --git a/release_automation/scripts/workflow_context.py b/release_automation/scripts/workflow_context.py index 3366b916..aad43b57 100644 --- a/release_automation/scripts/workflow_context.py +++ b/release_automation/scripts/workflow_context.py @@ -62,8 +62,6 @@ def parse_json_list(json_str): "src_commit_sha": os.environ.get("CTX_SRC_COMMIT_SHA", ""), # Not in BotContext directly? Check. # Trigger PR (for push events — PR that triggered the release-plan.yaml change) - "trigger_pr_number": os.environ.get("CTX_TRIGGER_PR_NUMBER", ""), - "trigger_pr_url": os.environ.get("CTX_TRIGGER_PR_URL", ""), # Dependencies "commonalities_release": os.environ.get("CTX_COMMONALITIES_RELEASE", ""), diff --git a/release_automation/templates/bot_messages/config_drift_warning.md b/release_automation/templates/bot_messages/config_drift_warning.md deleted file mode 100644 index d215ec31..00000000 --- a/release_automation/templates/bot_messages/config_drift_warning.md +++ /dev/null @@ -1,25 +0,0 @@ -**⚠️ Configuration drift — State: `{{state}}`** -[`release-plan.yaml`]({{release_plan_url}}) was updated on main{{#trigger_pr_number}} (PR [#{{trigger_pr_number}}]({{trigger_pr_url}})){{/trigger_pr_number}} but the change is not reflected in the active {{#state_snapshot_active}}snapshot{{/state_snapshot_active}}{{#state_draft_ready}}draft release{{/state_draft_ready}}. - -
Snapshot Configuration: Release {{release_tag}}{{#short_type}} ({{short_type}}{{#has_meta_release}}, {{meta_release}}{{/has_meta_release}}){{/short_type}} - -| API | Version | -|-----|---------| -{{#apis}} -| {{api_name}} | `{{api_version}}` | -{{/apis}} - -{{#commonalities_release}}**Dependencies:** Commonalities {{commonalities_release}}{{#identity_consent_management_release}}, ICM {{identity_consent_management_release}}{{/identity_consent_management_release}} -{{/commonalities_release}} -
- -{{#state_snapshot_active}} -**Valid actions (post a comment):**
→ `/discard-snapshot ` — discard snapshot, then `/create-snapshot` to pick up the new configuration
→ Update, review, and merge [Release PR]({{release_pr_url}}) — continue to create the release with the snapshot configuration - -_To apply the updated `release-plan.yaml`, discard the current snapshot and create a new one. To continue with the existing snapshot configuration, merge the Release PR._ -{{/state_snapshot_active}} -{{#state_draft_ready}} -**Valid actions (post a comment):**
→ `/delete-draft ` — delete draft, then `/create-snapshot` to pick up the new configuration
→ `/publish-release --confirm {{release_tag}}` — publish the release with the snapshot configuration - -_To apply the updated `release-plan.yaml`, delete the draft and create a new snapshot. To continue with the existing snapshot configuration, publish the release._ -{{/state_draft_ready}} diff --git a/release_automation/templates/bot_messages/issue_created.md b/release_automation/templates/bot_messages/issue_created.md index fa7eb9e9..5fceaf2b 100644 --- a/release_automation/templates/bot_messages/issue_created.md +++ b/release_automation/templates/bot_messages/issue_created.md @@ -5,9 +5,6 @@ Created via workflow dispatch from [`release-plan.yaml`]({{release_plan_url}}). {{#trigger_issue_close}} Created to replace closed [#{{closed_issue_number}}]({{closed_issue_url}}). {{/trigger_issue_close}} -{{#trigger_release_plan_change}} -Created after [`release-plan.yaml`]({{release_plan_url}}) update{{#trigger_pr_number}} (PR [#{{trigger_pr_number}}]({{trigger_pr_url}})){{/trigger_pr_number}}. -{{/trigger_release_plan_change}}
Configuration: Release {{release_tag}}{{#short_type}} ({{short_type}}{{#has_meta_release}}, {{meta_release}}{{/has_meta_release}}){{/short_type}} diff --git a/release_automation/tests/test_bot_context.py b/release_automation/tests/test_bot_context.py index 41c94fde..bb4a81ff 100644 --- a/release_automation/tests/test_bot_context.py +++ b/release_automation/tests/test_bot_context.py @@ -17,9 +17,7 @@ def test_default_values(self): assert ctx.command == "" assert ctx.command_args == "" assert ctx.user == "" - assert ctx.trigger_pr_number == "" assert ctx.trigger_type == "" - assert ctx.trigger_pr_url == "" assert ctx.closed_issue_number == "" assert ctx.closed_issue_url == "" assert ctx.release_plan_url == "" @@ -64,7 +62,6 @@ def test_default_values(self): assert ctx.state_published is False assert ctx.trigger_workflow_dispatch is False assert ctx.trigger_issue_close is False - assert ctx.trigger_release_plan_change is False assert ctx.has_meta_release is False assert ctx.has_reason is False assert ctx.has_sync_pr is False @@ -190,8 +187,7 @@ def test_to_dict_returns_all_keys(self): d = ctx.to_dict() expected_keys = { - "command", "command_args", "user", "trigger_pr_number", - "trigger_type", "trigger_pr_url", + "command", "command_args", "user", "trigger_type", "closed_issue_number", "closed_issue_url", "release_plan_url", "release_tag", "state", "release_type", "meta_release", "short_type", "snapshot_id", "snapshot_branch", "snapshot_branch_url", @@ -203,7 +199,6 @@ def test_to_dict_returns_all_keys(self): "is_missing_file", "is_malformed_yaml", "is_missing_field", "state_snapshot_active", "state_draft_ready", "state_published", "trigger_workflow_dispatch", "trigger_issue_close", - "trigger_release_plan_change", "has_meta_release", "has_reason", "workflow_run_url", "draft_release_url", "candidate_block_stripped", "reason", # Publication fields @@ -272,8 +267,7 @@ def test_returns_complete_dict(self): result = build_context() expected_keys = { - "command", "command_args", "user", "trigger_pr_number", - "trigger_type", "trigger_pr_url", + "command", "command_args", "user", "trigger_type", "closed_issue_number", "closed_issue_url", "release_plan_url", "release_tag", "state", "release_type", "meta_release", "short_type", "snapshot_id", "snapshot_branch", "snapshot_branch_url", @@ -285,7 +279,6 @@ def test_returns_complete_dict(self): "is_missing_file", "is_malformed_yaml", "is_missing_field", "state_snapshot_active", "state_draft_ready", "state_published", "trigger_workflow_dispatch", "trigger_issue_close", - "trigger_release_plan_change", "has_meta_release", "has_reason", "workflow_run_url", "draft_release_url", "candidate_block_stripped", "reason", # Publication fields @@ -392,7 +385,6 @@ def test_trigger_type_workflow_dispatch(self): ctx.derive_flags() assert ctx.trigger_workflow_dispatch is True assert ctx.trigger_issue_close is False - assert ctx.trigger_release_plan_change is False def test_trigger_type_issue_close(self): """trigger_type 'issue_close' sets correct flag.""" @@ -400,15 +392,6 @@ def test_trigger_type_issue_close(self): ctx.derive_flags() assert ctx.trigger_workflow_dispatch is False assert ctx.trigger_issue_close is True - assert ctx.trigger_release_plan_change is False - - def test_trigger_type_release_plan_change(self): - """trigger_type 'release_plan_change' sets correct flag.""" - ctx = BotContext(trigger_type="release_plan_change") - ctx.derive_flags() - assert ctx.trigger_workflow_dispatch is False - assert ctx.trigger_issue_close is False - assert ctx.trigger_release_plan_change is True def test_has_meta_release_true(self): """has_meta_release is True when meta_release is non-empty.""" diff --git a/release_automation/tests/test_bot_responder.py b/release_automation/tests/test_bot_responder.py index 5c8b0b58..530c1825 100644 --- a/release_automation/tests/test_bot_responder.py +++ b/release_automation/tests/test_bot_responder.py @@ -290,37 +290,34 @@ def test_snapshot_failed_template_with_error_message(self, bot_responder): assert "Unexpected error" in result assert "'str' object has no attribute 'get'" in result - def test_issue_created_template_with_pr(self, bot_responder): - """issue_created template shows PR link when trigger_pr_number is set.""" + def test_issue_created_template_after_push(self, bot_responder): + """issue_created template after a push carries no trigger line.""" from release_automation.scripts.context_builder import build_context context = build_context( release_tag="r4.1", state="planned", - trigger_type="release_plan_change", - trigger_pr_number="42", - trigger_pr_url="https://github.com/org/repo/pull/42", + trigger_type="push", + release_plan_url="https://github.com/org/repo/blob/main/release-plan.yaml", apis=[{"api_name": "quality-on-demand", "api_version": "1.0.0"}], ) result = bot_responder.render("issue_created", context) - assert "Release issue created" in result - assert "PR [#42]" in result - assert "pull/42" in result - assert "release-plan.yaml" in result + assert result.startswith("**📋 Release issue created — State: `planned`**\n\n
") + assert "Created " not in result - def test_issue_created_template_without_pr(self, bot_responder): - """issue_created template omits PR link when trigger_pr_number is empty.""" + def test_issue_created_template_after_dispatch(self, bot_responder): + """issue_created template after workflow dispatch links release-plan.yaml.""" from release_automation.scripts.context_builder import build_context context = build_context( release_tag="r4.1", state="planned", - trigger_type="release_plan_change", - apis=[{"api_name": "quality-on-demand", "api_version": "1.0.0"}], + trigger_type="workflow_dispatch", + release_plan_url="https://github.com/org/repo/blob/main/release-plan.yaml", ) result = bot_responder.render("issue_created", context) - assert "Release issue created" in result - assert "release-plan.yaml" in result - assert "[#]" not in result # No broken link - assert "update." in result # Sentence ends cleanly + assert ( + "Created via workflow dispatch from " + "[`release-plan.yaml`](https://github.com/org/repo/blob/main/release-plan.yaml)." + ) in result class TestContextIntegration: diff --git a/release_automation/tests/test_command_policy.py b/release_automation/tests/test_command_policy.py index 9a478c7b..311a4c55 100644 --- a/release_automation/tests/test_command_policy.py +++ b/release_automation/tests/test_command_policy.py @@ -402,7 +402,6 @@ def test_slash_command_plan_responds_with_interim(command): Trigger(trigger_type="issue_reopen", issue_number="42", user="x", should_continue=True), Trigger(trigger_type="pr_merge", user="x", should_continue=True), Trigger(trigger_type="workflow_dispatch", command="sync-issue", user="x", should_continue=True), - Trigger(trigger_type="release_plan_change", command="sync-issue", user="x", should_continue=True), Trigger(trigger_type="push", command="sync-issue", user="x", should_continue=True), ], ) diff --git a/release_automation/tests/test_github_client.py b/release_automation/tests/test_github_client.py index 27121972..819c31b2 100644 --- a/release_automation/tests/test_github_client.py +++ b/release_automation/tests/test_github_client.py @@ -228,27 +228,6 @@ def test_create_branch_at_sha_api_error(self, mock_run_gh): with self.assertRaises(GitHubClientError): self.client.create_branch_at_sha("release/r4.1", "abc123") - @patch("release_automation.scripts.github_client.GitHubClient._run_gh") - def test_find_merged_pr_for_commit(self, mock_run_gh): - mock_run_gh.return_value = json.dumps([ - {"number": 3, "merged_at": None, "html_url": "https://github.com/owner/repo/pull/3"}, - {"number": 5, "merged_at": "2026-10-23T10:00:00Z", "html_url": "https://github.com/owner/repo/pull/5"}, - ]) - result = self.client.find_merged_pr_for_commit("abc123") - self.assertEqual(result, (5, "https://github.com/owner/repo/pull/5")) - args = mock_run_gh.call_args[0][0] - self.assertIn("repos/owner/repo/commits/abc123/pulls", args) - - @patch("release_automation.scripts.github_client.GitHubClient._run_gh") - def test_find_merged_pr_for_commit_none_merged(self, mock_run_gh): - mock_run_gh.return_value = json.dumps([{"number": 3, "merged_at": None, "html_url": "u"}]) - self.assertIsNone(self.client.find_merged_pr_for_commit("abc123")) - - @patch("release_automation.scripts.github_client.GitHubClient._run_gh") - def test_find_merged_pr_for_commit_api_error(self, mock_run_gh): - mock_run_gh.side_effect = GitHubClientError("500: Internal server error") - self.assertIsNone(self.client.find_merged_pr_for_commit("abc123")) - @patch("release_automation.scripts.github_client.GitHubClient._run_gh") def test_get_collaborator_permission(self, mock_run_gh): mock_run_gh.return_value = "maintain\n" diff --git a/release_automation/tests/test_template_context_contract.py b/release_automation/tests/test_template_context_contract.py index b2b36fa9..5e9e2400 100644 --- a/release_automation/tests/test_template_context_contract.py +++ b/release_automation/tests/test_template_context_contract.py @@ -17,7 +17,6 @@ "command_rejected", "common_sync_failed", "common_sync_pr_created", - "config_drift_warning", "config_error", "draft_created", "draft_revoked", @@ -63,7 +62,6 @@ def test_all_templates_render_with_full_context(self, responder): command="/create-snapshot", command_args="", user="testuser", - trigger_pr_number="42", # State fields release_tag="r4.1", state="snapshot-active", @@ -181,46 +179,4 @@ def test_issue_reopened_renders_each_state(self, responder): assert isinstance(result, str) assert len(result) > 0 - def test_config_drift_warning_renders_for_snapshot_active(self, responder): - """config_drift_warning renders for snapshot-active state.""" - context = build_context( - release_tag="r4.1", - state="snapshot-active", - trigger_type="release_plan_change", - trigger_pr_number="55", - trigger_pr_url="https://github.com/org/repo/pull/55", - release_pr_url="https://github.com/org/repo/pull/123", - release_plan_url="https://github.com/org/repo/blob/main/release-plan.yaml", - apis=[ - { - "api_name": "QualityOnDemand", - "api_version": "1.0.0-rc.1", - }, - ], - ) - result = responder.render("config_drift_warning", context) - assert "Configuration drift" in result - assert "#55" in result - assert "/discard-snapshot" in result - assert "Snapshot Configuration" in result - - def test_config_drift_warning_renders_for_draft_ready(self, responder): - """config_drift_warning renders for draft-ready state.""" - context = build_context( - release_tag="r4.1", - state="draft-ready", - trigger_type="release_plan_change", - trigger_pr_number="55", - trigger_pr_url="https://github.com/org/repo/pull/55", - release_plan_url="https://github.com/org/repo/blob/main/release-plan.yaml", - apis=[ - { - "api_name": "QualityOnDemand", - "api_version": "1.0.0-rc.1", - }, - ], - ) - result = responder.render("config_drift_warning", context) - assert "Configuration drift" in result - assert "/delete-draft" in result - assert "/publish-release" in result + diff --git a/release_automation/tests/test_trigger.py b/release_automation/tests/test_trigger.py index 31787b62..5486e336 100644 --- a/release_automation/tests/test_trigger.py +++ b/release_automation/tests/test_trigger.py @@ -23,8 +23,8 @@ def issue_comment_payload(body, labels=RELEASE_ISSUE_LABELS, commenter="alice"): } -def parse(event_name, payload, actor="dispatcher", sha="f" * 40): - return parse_trigger(event_name, payload, actor=actor, sha=sha) +def parse(event_name, payload, actor="dispatcher"): + return parse_trigger(event_name, payload, actor=actor) # ── issue_comment ──────────────────────────────────────────────────────────── @@ -218,58 +218,32 @@ def test_workflow_dispatch_is_sync_issue(): # ── push ───────────────────────────────────────────────────────────────────── -def push_payload(*commits, head="a" * 40): +def push_payload(*commits): return { "ref": "refs/heads/main", - "head_commit": {"id": head} if head else None, + "head_commit": {"id": "a" * 40}, "commits": list(commits), } -def test_push_changing_release_plan(): - payload = push_payload( - {"added": [], "modified": ["README.md"], "removed": []}, - {"added": [], "modified": ["release-plan.yaml"], "removed": []}, - ) - - t = parse("push", payload, actor="erin") - - assert t == Trigger( - trigger_type="release_plan_change", - command="sync-issue", - user="erin", - should_continue=True, - head_sha="a" * 40, - ) - - @pytest.mark.parametrize( "commits", [ + [{"added": [], "modified": ["release-plan.yaml"], "removed": []}], + [{"removed": ["release-plan.yaml"]}], [{"added": ["code/common/CAMARA_common.yaml"], "modified": [], "removed": []}], - [{"modified": [".github/workflows/release-automation.yml"]}], - [{"added": ["docs/release-plan.yaml"]}], [], ], ) -def test_push_without_release_plan_change(commits): +def test_push_is_a_sync_issue_whatever_it_changed(commits): t = parse("push", push_payload(*commits), actor="erin") - assert t.trigger_type == "push" - assert t.command == "sync-issue" - assert t.should_continue is True - - -def test_push_release_plan_removed_counts_as_change(): - t = parse("push", push_payload({"removed": ["release-plan.yaml"]})) - - assert t.trigger_type == "release_plan_change" - - -def test_push_without_head_commit_uses_github_sha(): - t = parse("push", push_payload(head=None), sha="b" * 40) - - assert t.head_sha == "b" * 40 + assert t == Trigger( + trigger_type="push", + command="sync-issue", + user="erin", + should_continue=True, + ) # ── other events ───────────────────────────────────────────────────────────── @@ -282,16 +256,6 @@ def test_unhandled_event(): # ── main(): env → GITHUB_OUTPUT ────────────────────────────────────────────── -class FakeClient: - def __init__(self, merged_pr=None): - self.merged_pr = merged_pr - self.looked_up = [] - - def find_merged_pr_for_commit(self, sha): - self.looked_up.append(sha) - return self.merged_pr - - def read_outputs(path): """Parse GITHUB_OUTPUT heredoc entries (name<