BWDO-778 sc branch rename, rm_merged and more

[BenjiMilan]

I can only apologise for a bit of a monster review, been quite good at not doing these for a while.

Things doing:
Moved ticket service out into it's own section so it could be reused in sc branch rm_merged and sc show merged_release.
Created sc branch rm_merged, sc branch rename, sc show merged_release.
Fixed an issue in sc delete which would never complete on rerun if it failed the first time around.

[Copilot]

Pull request overview

This PR refactors ticketing support into sc.services.tickets so it can be reused across commands, introduces new branching commands (branch rename, branch rm_merged, show merged_release), and standardizes error handling via a new sc.exceptions + shared sc.prompter.

Changes:

• Extract ticket models/config/service into src/sc/services/tickets/* and migrate review tooling to use it.
• Add new branching commands: sc branch rename, sc branch rm_merged, and sc show merged_release with accompanying tests.
• Improve robustness of branch deletion behavior (handle non-git dirs and failed deletions more gracefully) and unify exception types.

Reviewed changes

Copilot reviewed 31 out of 33 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
tests/tickets/test_ticket_service.py	New unit tests for extracted TicketService behavior.
tests/review/test_ticket_updater.py	Update review tests to use the new ticket service API and Ticket wrapper.
tests/review/test_ticket_service.py	Remove old review-scoped ticket service tests after extraction.
tests/review/test_git_host_service.py	Update to new ConfigError exception source/type.
tests/branching/test_branch_rm_merged.py	New tests for branch rm_merged command behavior.
tests/branching/test_branch_rename.py	New integration-ish tests for branch rename behavior.
src/sc/services/tickets/ticketing_instances/ticketing_instance.py	Adjust typing imports to avoid runtime circular dependencies.
src/sc/services/tickets/ticketing_instances/ticket_instance_factory.py	Update exception import to the new tickets exception module.
src/sc/services/tickets/ticketing_instances/instances/redmine_instance.py	Build TicketData + Ticket wrapper; change Redmine field extraction.
src/sc/services/tickets/ticketing_instances/instances/jira_instance.py	Build TicketData + Ticket wrapper for Jira.
src/sc/services/tickets/ticketing_instances/instances/init.py	New exports for Jira/Redmine instances.
src/sc/services/tickets/ticketing_instances/init.py	New exports for ticketing instance abstractions/factory.
src/sc/services/tickets/ticket.py	New TicketData + Ticket wrapper to centralize formatting + comment addition.
src/sc/services/tickets/ticket_service.py	New shared ticket service (branch parsing, prompting, instance creation).
src/sc/services/tickets/ticket_config.py	New ticket host config model + config access layer.
src/sc/services/tickets/exceptions.py	Move ticket exceptions onto ScError base.
src/sc/services/tickets/init.py	New exports for tickets package.
src/sc/review/ticket_updater.py	Switch ticket updater to use TicketService.get_ticket_from_branch() and Ticket.add_comment().
src/sc/review/ticket_service.py	Remove old review-scoped ticket service (replaced by shared service).
src/sc/review/review.py	Update exception handling to catch ScError instead of review-scoped base exception.
src/sc/review/review_config.py	Remove ticket config from review config; switch to sc.exceptions.ConfigError.
src/sc/review/prompter.py	Remove review-scoped prompter (replaced by shared sc.prompter).
src/sc/review/git_host_service.py	Switch remote-url failure exception to ConfigError.
src/sc/prompter.py	New shared prompter used across features.
src/sc/project_cli.py	Add new CLI commands/groups for branching + merged release output.
src/sc/exceptions.py	New shared exception hierarchy (ScError, ConfigError, PermissionsError).
src/sc/branching/exceptions.py	Make ScInitError derive from ScError.
src/sc/branching/commands/show.py	Add ShowMergedRelease command implementation.
src/sc/branching/commands/push.py	Push tags during push operation.
src/sc/branching/commands/delete.py	Make delete more resilient (invalid repos, deletion failures).
src/sc/branching/commands/branch_rm_merged.py	New implementation for removing merged branches.
src/sc/branching/commands/branch_rename.py	New implementation for renaming branches locally/remotely.
src/sc/branching/branching.py	Wire new commands + unify error handling to ScError.

Comments suppressed due to low confidence (1)

src/sc/services/tickets/exceptions.py:49

• TicketIdentifierNotFound is defined twice in this module, so the second definition overwrites the first. This makes tracebacks/docs confusing and can hide future changes to the earlier class body/docstring.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rdkcmf-jenkins]

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 3 files pending identification.

• Protex Server Path: /home/blackduck/github/sc/66/rdkcentral/sc

• Commit: cdd1479

Report detail: gist'

[rdkcmf-jenkins]

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 3 files pending identification.

• Protex Server Path: /home/blackduck/github/sc/66/rdkcentral/sc

• Commit: ea2e8ff

Report detail: gist'

[rdkcmf-jenkins]

b'## WARNING: A Blackduck scan failure has been waived

A prior failure has been upvoted

• Upvote reason: OK

• Commit: ea2e8ff
  '

[Copilot]

Pull request overview

Copilot reviewed 32 out of 34 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/sc/services/tickets/exceptions.py:49

• TicketIdentifierNotFound is defined twice in this module; the second definition overwrites the first and makes the file harder to reason about. Keep a single definition (and adjust its docstring if needed).

[Copilot]

Pull request overview

Copilot reviewed 32 out of 34 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/sc/services/tickets/exceptions.py:49

• TicketIdentifierNotFound is defined twice in this module; the second definition overrides the first and makes the file confusing to import/use. Keep a single definition and remove the duplicate block.

[Copilot]

Pull request overview

Copilot reviewed 32 out of 34 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (9)

src/sc/branching/commands/branch_rm_merged.py:93

• When --all is passed, merge_type is set to "--all" and then used as if it were a merge-filter flag (and develop is still passed as a positional argument). This produces an invalid/incorrect git branch invocation and likely won’t list the intended branches.

        merge_type = "--merged" if not self.not_merged else "--no-merged"
        merge_type = "--all" if self.all else merge_type
        branches = repo.git.branch("-r", merge_type, develop).splitlines()

src/sc/services/tickets/exceptions.py:49

• TicketIdentifierNotFound is defined twice in this module; the second definition overwrites the first and makes the file misleading (and can hide future changes if only one definition is edited). Keep a single definition.
  src/sc/branching/commands/branch_rename.py:112
• _has_remote_branch() is declared to return bool but returns None when the branch is not found. This forces callers into is not None checks and breaks the type contract.

        return None

src/sc/branching/commands/branch_rm_merged.py:65

• get_ticket_from_branch() can raise ScError (e.g., TicketIdentifierNotFound when a branch name doesn’t contain a ticket reference). As written, one non-conforming branch will abort the whole command.

            ticket = ticket_service.get_ticket_from_branch(branch.name)
            print(ticket.to_terminal(one_line=True))
            print(branch.name)

src/sc/services/tickets/ticketing_instances/instances/redmine_instance.py:77

• issue._decoded_attrs is a private attribute of redminelib objects and may change between versions. Prefer using a public API (or at least fall back safely) to avoid breaking ticket reads after dependency updates.
  src/sc/branching/commands/branch_rename.py:95
• The remote-branch existence check is currently ... is not None, but _has_remote_branch() returns a boolean. This condition will always be true once _has_remote_branch() is fixed to return False, so the old-branch deletion logic won’t behave correctly.

This issue also appears on line 112 of the same file.

        if self._has_remote_branch(repo, remote, self.old_branch) is not None:

src/sc/branching/branching.py:347

• detect_project() returns two values ((Path, ProjectType)), but the return type annotation is tuple[Path | ProjectType] (a 1-tuple containing a union). This is incorrect and will confuse type-checkers.

def detect_project(run_dir: Path) -> tuple[Path | ProjectType]:

src/sc/branching/commands/push.py:39

• git push --tags is executed without specifying the same remote used for the branch push. That can push tags to an unexpected default remote (or fail if none is configured).

        repo.git.push("--tags")

tests/tickets/test_ticket_service.py:24

• This test uses auth_type="config", but the production code expects auth_type values of "token" or "basic" (see TicketHostModel/TicketingInstanceFactory). Using an invalid value here can mask real integration issues.

[Copilot]

Pull request overview

Copilot reviewed 32 out of 34 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

src/sc/services/tickets/exceptions.py:49

• TicketIdentifierNotFound is defined twice in this module (earlier at lines 21–23 and again here). The second definition silently overwrites the first, which is confusing and can hide future edits to the earlier class/docstring.
  src/sc/branching/commands/branch_rm_merged.py:65
• get_ticket_from_branch() can raise TicketIdentifierNotFound (and other ScErrors). Right now any such failure will abort the entire rm_merged run, even though deleting branches could still proceed (or at least the command could continue listing other branches).

        for branch in filtered_branches:
            ticket = ticket_service.get_ticket_from_branch(branch.name)
            print(ticket.to_terminal(one_line=True))
            print(branch.name)

src/sc/branching/commands/branch_rename.py:97

• This condition is always true: _has_remote_branch(...) returns a bool, so is not None will never be false. That means the code will always attempt to delete the old remote branch even when it doesn't exist, potentially failing the rename.

        if self._has_remote_branch(repo, remote, self.old_branch) is not None:

src/sc/branching/commands/push.py:39

• run_git_command() now unconditionally pushes all local tags (git push --tags) after pushing the branch. This is a potentially surprising/dangerous side effect (e.g., pushing stale or private tags) and changes behavior beyond the branch being pushed.

        repo.git.push("--tags")

src/sc/branching/commands/branch_rename.py:16

• Unused import: Enum, auto are imported but never referenced in this file.

from enum import Enum, auto

[TB-1993]

Looks good, ship it!