Auto-fetch the target ref and model branch/tag as a sum type

Make the user's `--branch` or `--tag` choice a typed `Branch | Tag` produced by
`validate_input`, and have every downstream helper take that `ReleaseTarget`
instead of `(branch: str | None, tag: str | None)`. This puts the "exactly one
is set" invariant in the type system and removes the type-narrowing `assert`s
that would otherwise turn into an `AssertionError` with no context if anyone
broke the invariant.

While at it, drop the `git ls-remote` probe + "please run `git fetch` and try
again" hint and fetch the ref ourselves. The new `fetch_target` runs
`git fetch --quiet --depth=1 origin refs/heads/<branch>:refs/remotes/origin/<branch>`
(or the equivalent `refs/tags/...:refs/tags/...` for `--tag`), which both confirms
the ref exists on origin and populates the local refs we need to read the
workflow files. If `git fetch` reports `couldn't find remote ref`, the abort
message stays the same as before (`Branch X not found on origin`); other git
errors surface verbatim through `Failed to fetch ... from origin: ...`.

Tests now mock `GitRepository.run` (the fetch) instead of `GitRepository.capture`
(the ls-remote). Two new tests pin the exact refspec the command must send so a
future refactor that breaks the fetch shape can't slip through. The
"please-fetch-first" test goes away; that branch is unreachable now.

Author: AAraKKe

ddev/src/ddev/cli/release/test_agent/__init__.py

@@ -36,22 +36,21 @@ def test_agent(app: Application, branch: str | None, tag: str | None, dry_run: b
     from ddev.cli.release.test_agent.dispatch import dispatch_both
     from ddev.cli.release.test_agent.images import build_image_refs, resolve_version, validate_images_exist
     from ddev.cli.release.test_agent.validation import (
+        Branch,
+        fetch_target,
         validate_input,
-        verify_ref_exists,
         verify_workflows_present_on_ref,
     )
 
-    branch, tag = validate_input(app, branch, tag)
-    ref = branch or tag
-    assert ref is not None
+    target = validate_input(app, branch, tag)
 
     if not app.config.github.token:
         app.abort('GitHub token required. Set `github.token` via `ddev config set github.token <token>`.')
 
-    verify_ref_exists(app, branch=branch, tag=tag)
-    verify_workflows_present_on_ref(app, branch=branch, tag=tag)
+    fetch_target(app, target)
+    verify_workflows_present_on_ref(app, target)
 
-    version = resolve_version(app, branch=branch, tag=tag)
+    version = resolve_version(app, target)
     validate_images_exist(app, version)
     linux_image, windows_image = build_image_refs(version)
 
@@ -66,7 +65,8 @@ def test_agent(app: Application, branch: str | None, tag: str | None, dry_run: b
         'agent-image': linux_image,
         'agent-image-windows': windows_image,
     }
-    _print_plan(app, ref=ref, version=version, branch=branch, inputs=inputs)
+    is_branch = isinstance(target, Branch)
+    _print_plan(app, ref=target.name, version=version, is_branch=is_branch, inputs=inputs)
 
     if dry_run:
         app.display_info('Dry run — no workflows dispatched.')
@@ -76,7 +76,7 @@ def test_agent(app: Application, branch: str | None, tag: str | None, dry_run: b
         app.abort('Aborted by user.')
 
     try:
-        linux_url, windows_url = dispatch_both(app.config.github.token, ref=ref, inputs=inputs)
+        linux_url, windows_url = dispatch_both(app.config.github.token, ref=target.name, inputs=inputs)
     except RuntimeError as e:
         app.abort(str(e))
     else:
@@ -88,7 +88,7 @@ def _print_plan(
     *,
     ref: str,
     version: str,
-    branch: str | None,
+    is_branch: bool,
     inputs: dict[str, str],
 ) -> None:
     """Render the resolved dispatch plan via the stderr-bound `display_info` channel.
@@ -102,7 +102,7 @@ def _print_plan(
     app.display_info('Dispatch plan')
     app.display_info(f'  Workflows: {WORKFLOW_LINUX}, {WORKFLOW_WINDOWS}')
     app.display_info(f'  Ref: {ref}')
-    if branch is not None:
+    if is_branch:
         app.display_info(f'  Resolved RC: {version}')
     for key, value in inputs.items():
         app.display_info(f'  {key}: {value}')

ddev/src/ddev/cli/release/test_agent/images.py

@@ -11,6 +11,7 @@
 import httpx
 
 from ddev.cli.release.test_agent import registry
+from ddev.cli.release.test_agent.validation import Branch, ReleaseTarget
 
 if TYPE_CHECKING:
     from collections.abc import Iterator
@@ -19,25 +20,24 @@
 
 
 @contextlib.contextmanager
-def registry_errors(app: Application, target: str) -> Iterator[None]:
+def registry_errors(app: Application, scope: str) -> Iterator[None]:
     """Translate any `httpx.HTTPError` raised inside the block into a clean `app.abort` message.
 
-    `target` is interpolated into the abort text — e.g. `'tags'` for the tag listing or an
+    `scope` is interpolated into the abort text — e.g. `'tags'` for the tag listing or an
     image ref like `'registry.datadoghq.com/agent:7.80.0-rc.3'` for a manifest probe.
     """
     try:
         yield
     except httpx.HTTPError as e:
-        app.abort(f'Failed to query registry.datadoghq.com for {target}: {e}')
+        app.abort(f'Failed to query registry.datadoghq.com for {scope}: {e}')
 
 
-def resolve_version(app: Application, *, branch: str | None, tag: str | None) -> str:
+def resolve_version(app: Application, target: ReleaseTarget) -> str:
     """Pick the Agent image tag to test: the explicit tag, or the highest published RC for a branch."""
-    if tag is not None:
-        return tag
+    if not isinstance(target, Branch):
+        return target.name
 
-    assert branch is not None
-    major_str, minor_str, _ = branch.split('.')
+    major_str, minor_str, _ = target.name.split('.')
     major, minor = int(major_str), int(minor_str)
 
     app.display_waiting(f'Looking up latest {major}.{minor}.0-rc.* in registry.datadoghq.com...')

ddev/src/ddev/cli/release/test_agent/validation.py

@@ -1,11 +1,20 @@
 # (C) Datadog, Inc. 2026-present
 # All rights reserved
 # Licensed under a 3-clause BSD style license (see LICENSE)
-"""Input validation and git-ref checks for `ddev release test-agent`."""
+"""Input validation, ref fetching, and workflow-file checks for `ddev release test-agent`.
+
+The user picks either a release branch or a release tag. Downstream code needs to treat
+those two paths differently (the fetch refspec, the local ref name for `git show`, the
+version-resolution logic), but the type system has to know which one is in hand. Modelling
+the choice as a sum type (`Branch | Tag`) — produced by `validate_input` after the
+user-facing checks — keeps the branch/tag distinction explicit at every call site and
+eliminates the type-narrowing `assert`s that would otherwise be needed.
+"""
 
 from __future__ import annotations
 
 import re
+from dataclasses import dataclass
 from typing import TYPE_CHECKING
 
 from ddev.cli.release.test_agent.dispatch import WORKFLOW_LINUX, WORKFLOW_WINDOWS
@@ -22,82 +31,112 @@
 ]
 
 # git error fragments that mean "ref exists but file is not in that tree" — i.e. the workflow
-# really isn't on this branch/tag, as opposed to the ref itself being unreachable locally.
+# really isn't on this branch/tag, as opposed to git failing for some other reason.
 GIT_FILE_MISSING_FRAGMENTS = (
     'exists on disk',
     'does not exist',
     'no such path',
 )
-GIT_REF_MISSING_FRAGMENTS = (
-    'invalid object name',
-    'unknown revision',
-    'bad revision',
-    'ambiguous argument',
+
+# git error fragments emitted when `git fetch` cannot resolve the named ref on origin.
+GIT_FETCH_MISSING_FRAGMENTS = (
+    "couldn't find remote ref",
+    'no such ref',
 )
 
 
-def validate_input(app: Application, branch: str | None, tag: str | None) -> tuple[str | None, str | None]:
-    """Normalize and validate inputs, returning (branch, tag) with exactly one set."""
-    if bool(branch) == bool(tag):
-        app.abort('Exactly one of --branch or --tag must be provided.')
+@dataclass(frozen=True)
+class Branch:
+    """A validated release branch the user selected via `--branch`."""
+
+    name: str
+
+
+@dataclass(frozen=True)
+class Tag:
+    """A validated release tag the user selected via `--tag` (with any leading `v` stripped)."""
+
+    name: str
+
+
+ReleaseTarget = Branch | Tag
+
+
+def validate_input(app: Application, branch: str | None, tag: str | None) -> ReleaseTarget:
+    """Normalize and validate `--branch`/`--tag`, returning a typed `Branch` or `Tag`.
 
-    if branch is not None and not re.match(BRANCH_PATTERN, branch):
-        app.abort(f'Invalid branch: {branch!r}. Must match {BRANCH_PATTERN}.')
+    Click can't enforce the mutually-exclusive constraint by itself, so this is the single
+    point that does. Every downstream helper takes the resulting `ReleaseTarget` instead of
+    two optional strings, which keeps the "exactly one is set" invariant in the type system
+    rather than restated as `assert` at every call site.
+    """
+    if branch is not None and tag is not None:
+        app.abort('Cannot use --branch and --tag together; pick one.')
+
+    if branch is not None:
+        if not re.match(BRANCH_PATTERN, branch):
+            app.abort(f'Invalid branch: {branch!r}. Must match {BRANCH_PATTERN}.')
+        return Branch(branch)
 
     if tag is not None:
         normalized = tag.removeprefix('v')
         if not re.match(TAG_PATTERN, normalized):
             app.abort(f'Invalid tag: {tag!r}. Must match {TAG_PATTERN}.')
-        tag = normalized
+        return Tag(normalized)
 
-    return branch, tag
+    app.abort('Exactly one of --branch or --tag must be provided.')
 
 
-def verify_ref_exists(app: Application, *, branch: str | None, tag: str | None) -> None:
-    """Confirm the ref is published on origin via `git ls-remote`."""
-    if branch is not None:
-        kind, value, flag = 'branch', branch, '--heads'
-    else:
-        assert tag is not None
-        kind, value, flag = 'tag', tag, '--tags'
-
-    output = app.repo.git.capture('ls-remote', flag, 'origin', value)
-    if not output.strip():
-        app.abort(f'{kind.capitalize()} `{value}` not found on origin.')
+def local_ref_for(target: ReleaseTarget) -> str:
+    """The local refname that `fetch_target` populates and that `git show` can read from."""
+    if isinstance(target, Branch):
+        return f'origin/{target.name}'
+    return target.name
 
 
-def verify_workflows_present_on_ref(app: Application, *, branch: str | None, tag: str | None) -> None:
-    """Confirm both workflow files exist at the target ref.
+def fetch_target(app: Application, target: ReleaseTarget) -> None:
+    """Fetch the user's release target from origin so `local_ref_for(target)` resolves locally.
 
-    `git show <ref>:<path>` only resolves against local refs, so a branch the user has not yet
-    fetched will not be found under its bare name. For branches we read `origin/<branch>` to
-    consult the remote-tracking ref; for tags we use the tag name directly. Either way, the
-    git error text is inspected to distinguish "file missing from the tree" from "ref not
-    local" so the abort message points at the real problem.
+    Combines the existence check (would otherwise be a `git ls-remote` probe) with the side
+    effect of populating the local refs we need to read the workflow files. After this call,
+    `git show origin/<branch>:<path>` or `git show <tag>:<path>` is guaranteed to find the
+    target in the local clone.
     """
-    if branch is not None:
-        local_ref = f'origin/{branch}'
-        fetch_hint = f'Run `git fetch origin {branch}` and try again.'
+    if isinstance(target, Branch):
+        kind = 'branch'
+        refspec = f'refs/heads/{target.name}:refs/remotes/origin/{target.name}'
     else:
-        assert tag is not None
-        local_ref = tag
-        fetch_hint = f'Run `git fetch origin tag {tag}` and try again.'
+        kind = 'tag'
+        refspec = f'refs/tags/{target.name}:refs/tags/{target.name}'
+
+    app.display_waiting(f'Fetching {kind} `{target.name}` from origin...')
+    try:
+        app.repo.git.run('fetch', '--quiet', '--depth=1', 'origin', refspec)
+    except OSError as e:
+        msg = str(e).lower()
+        if any(fragment in msg for fragment in GIT_FETCH_MISSING_FRAGMENTS):
+            app.abort(f'{kind.capitalize()} `{target.name}` not found on origin.')
+        app.abort(f'Failed to fetch {kind} `{target.name}` from origin: {e}')
+
+
+def verify_workflows_present_on_ref(app: Application, target: ReleaseTarget) -> None:
+    """Confirm both workflow files exist at the target ref (which `fetch_target` made local)."""
+    ref = local_ref_for(target)
+    kind = 'branch' if isinstance(target, Branch) else 'tag'
 
     missing: list[str] = []
     for path in WORKFLOW_FILES:
         try:
-            app.repo.git.show_file(path, local_ref)
+            app.repo.git.show_file(path, ref)
         except OSError as e:
             msg = str(e).lower()
             if any(fragment in msg for fragment in GIT_FILE_MISSING_FRAGMENTS):
                 missing.append(path)
-            elif any(fragment in msg for fragment in GIT_REF_MISSING_FRAGMENTS):
-                app.abort(f'Ref `{local_ref}` is not in your local clone. {fetch_hint} (git error: {e})')
             else:
-                app.abort(f'Failed to read `{path}` from `{local_ref}`: {e}')
+                app.abort(f'Failed to read `{path}` from `{ref}`: {e}')
 
     if missing:
         app.abort(
-            f'Ref `{local_ref}` is missing required workflow file(s): {", ".join(missing)}. '
+            f'{kind.capitalize()} `{target.name}` is missing required workflow file(s): {", ".join(missing)}. '
             'Pick a newer ref that includes both `test-agent.yml` and `test-agent-windows.yml`.'
         )

ddev/tests/cli/release/test_agent/test_command.py

@@ -20,8 +20,8 @@
 
 @pytest.fixture(autouse=True)
 def _silence_git(mocker: MockerFixture) -> None:
-    """Default git mocks: ref exists on origin and both workflow files are present at the ref."""
-    mocker.patch('ddev.utils.git.GitRepository.capture', return_value='abc123\trefs/heads/7.80.x\n')
+    """Default git mocks: `fetch_target` succeeds and `git show` returns workflow yaml."""
+    mocker.patch('ddev.utils.git.GitRepository.run', return_value=None)
     mocker.patch('ddev.utils.git.GitRepository.show_file', return_value='workflow yaml')
 
 
@@ -39,7 +39,7 @@ def _silence_registry(mocker: MockerFixture) -> None:
     'args, expected',
     [
         pytest.param([], 'Exactly one of --branch or --tag', id='neither'),
-        pytest.param(['--branch', '7.80.x', '--tag', '7.80.0'], 'Exactly one of --branch or --tag', id='both'),
+        pytest.param(['--branch', '7.80.x', '--tag', '7.80.0'], 'Cannot use --branch and --tag together', id='both'),
         pytest.param(['--branch', '7.80'], 'Invalid branch', id='bad-branch'),
         pytest.param(['--tag', '7.80'], 'Invalid tag', id='bad-tag'),
     ],
@@ -130,48 +130,76 @@ def test_missing_image_aborts(ddev: CliRunner, mocker: MockerFixture, fake_async
 
 
 def test_missing_ref_aborts(ddev: CliRunner, mocker: MockerFixture, fake_async_github: FakeAsyncGitHubClient) -> None:
-    mocker.patch('ddev.utils.git.GitRepository.capture', return_value='')
+    """When `git fetch` reports the ref does not exist on origin, surface a clean abort."""
+    mocker.patch(
+        'ddev.utils.git.GitRepository.run',
+        side_effect=OSError("fatal: couldn't find remote ref refs/heads/7.80.x"),
+    )
 
     result = ddev('release', 'test-agent', '--branch', '7.80.x', '--yes')
 
     assert result.exit_code != 0, result.output
-    assert 'not found on origin' in result.output
+    assert 'Branch `7.80.x` not found on origin' in result.output
     fake_async_github.assert_not_called('create_workflow_dispatch')
 
 
-def test_missing_workflow_file_aborts(
+def test_fetch_target_is_called_with_branch_refspec(
     ddev: CliRunner, mocker: MockerFixture, fake_async_github: FakeAsyncGitHubClient
 ) -> None:
-    """A 'file exists on disk, but not in <ref>' git error reads as the workflow really being absent."""
+    """The command must fetch the branch into `origin/<branch>` before reading workflow files."""
+    run_spy = mocker.patch('ddev.utils.git.GitRepository.run', return_value=None)
+
+    result = ddev('release', 'test-agent', '--branch', '7.80.x', '--yes')
+
+    assert result.exit_code == 0, result.output
+    run_spy.assert_any_call('fetch', '--quiet', '--depth=1', 'origin', 'refs/heads/7.80.x:refs/remotes/origin/7.80.x')
+
+
+def test_fetch_target_is_called_with_tag_refspec(
+    ddev: CliRunner, mocker: MockerFixture, fake_async_github: FakeAsyncGitHubClient
+) -> None:
+    """A `--tag` invocation must fetch into `refs/tags/<tag>` so `git show <tag>:path` works."""
+    run_spy = mocker.patch('ddev.utils.git.GitRepository.run', return_value=None)
+
+    result = ddev('release', 'test-agent', '--tag', '7.80.0-rc.1', '--yes')
+
+    assert result.exit_code == 0, result.output
+    run_spy.assert_any_call('fetch', '--quiet', '--depth=1', 'origin', 'refs/tags/7.80.0-rc.1:refs/tags/7.80.0-rc.1')
+
+
+def test_fetch_target_other_error_surfaces_original(
+    ddev: CliRunner, mocker: MockerFixture, fake_async_github: FakeAsyncGitHubClient
+) -> None:
+    """Anything other than 'remote ref not found' must surface as a generic fetch-failed abort."""
     mocker.patch(
-        'ddev.utils.git.GitRepository.show_file',
-        side_effect=OSError(
-            "fatal: path '.github/workflows/test-agent.yml' exists on disk, but not in 'origin/7.80.x'"
-        ),
+        'ddev.utils.git.GitRepository.run',
+        side_effect=OSError('fatal: unable to access https://github.com/...: Could not resolve host'),
     )
 
     result = ddev('release', 'test-agent', '--branch', '7.80.x', '--yes')
 
     assert result.exit_code != 0, result.output
-    assert 'missing required workflow file' in result.output
-    assert 'origin/7.80.x' in result.output
+    assert 'Failed to fetch branch `7.80.x` from origin' in result.output
+    assert 'Could not resolve host' in result.output
     fake_async_github.assert_not_called('create_workflow_dispatch')
 
 
-def test_unfetched_branch_surfaces_fetch_hint(
+def test_missing_workflow_file_aborts(
     ddev: CliRunner, mocker: MockerFixture, fake_async_github: FakeAsyncGitHubClient
 ) -> None:
-    """When `origin/<branch>` isn't in the local clone, the abort should tell the user to fetch."""
+    """A 'file exists on disk, but not in <ref>' git error reads as the workflow really being absent."""
     mocker.patch(
         'ddev.utils.git.GitRepository.show_file',
-        side_effect=OSError("fatal: invalid object name 'origin/7.80.x'"),
+        side_effect=OSError(
+            "fatal: path '.github/workflows/test-agent.yml' exists on disk, but not in 'origin/7.80.x'"
+        ),
     )
 
     result = ddev('release', 'test-agent', '--branch', '7.80.x', '--yes')
 
     assert result.exit_code != 0, result.output
-    assert 'not in your local clone' in result.output
-    assert 'git fetch origin 7.80.x' in result.output
+    assert 'missing required workflow file' in result.output
+    assert '7.80.x' in result.output
     fake_async_github.assert_not_called('create_workflow_dispatch')