Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 56 additions & 15 deletions src/VCS/Adapter/Git/GitHub.php
Original file line number Diff line number Diff line change
Expand Up @@ -636,8 +636,9 @@ protected function generateAccessToken(string $privateKey, ?string $appId): void
$this->jwtToken = $token;
$response = $this->call(self::METHOD_POST, '/app/installations/' . $this->installationId . '/access_tokens', ['Authorization' => 'Bearer ' . $token]);
$responseBody = $response['body'] ?? [];
$statusCode = $response['headers']['status-code'] ?? 0;
if (!array_key_exists('token', $responseBody)) {
throw new Exception('Failed to retrieve access token from GitHub API.');
throw new Exception('Failed to retrieve access token from GitHub API. Status: ' . $statusCode . '. Response: ' . \json_encode($responseBody));
}
$this->accessToken = $responseBody['token'] ?? '';
}
Expand Down Expand Up @@ -742,32 +743,72 @@ public function getPullRequestFromBranch(string $owner, string $repositoryName,
}

/**
* Lists branches for a given repository
* Lists branches using GitHub GraphQL with cursor pagination and optional substring search.
*
* @param string $owner Owner name of the repository
* @param string $repositoryName Name of the GitHub repository
* @param int $perPage Number of branches to fetch per page
* @param int $page Page number to start fetching from
* @return array<string> List of branch names as array
* Uses refs(query:) for substring search — 'auth' matches 'feature/auth' and 'fix-auth'.
* Returns items, totalCount and nextCursor for proper infinite scroll support.
*
* @param string $owner
* @param string $repositoryName
* @param int $perPage Clamped to [1, 100]
* @param string|null $cursor Opaque cursor from previous nextCursor to resume pagination
* @param string $search Substring filter; empty returns all branches
* @return array{items: array<string>, total: int, nextCursor: string|null}
*/
public function listBranches(string $owner, string $repositoryName, int $perPage = 100, int $page = 1): array
public function listBranches(string $owner, string $repositoryName, int $perPage = 100, string|null $cursor = null, string $search = ''): array

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Preserve branch contract

listBranches() is part of the shared adapter API and is documented as returning an array of branch names. This GitHub implementation now returns a wrapper with items, total, and nextCursor, with the final shape assembled later around lines 807-811, while the other adapters still return plain branch-name arrays. Code written against Adapter can iterate this result or call in_array($branch, $adapter->listBranches(...), true) and fail only for GitHub because it receives metadata keys instead of branch names. Please keep listBranches() returning branch names and move the paginated/search shape to a separate method, or update the abstract contract and all adapters/callers together.

Artifacts

Repro: PHP script exercising GitHub::listBranches() with mocked GraphQL response

  • Contains supporting evidence from the run (text/x-php; charset=utf-8).

Repro: failing output showing wrapper shape and broken in_array/foreach behavior

  • Keeps the command output available without making the summary code-heavy.

View artifacts

T-Rex Ran code and verified through T-Rex

{
$url = "/repos/$owner/$repositoryName/branches";
$perPage = min(max($perPage, 1), 100);

$response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"], [
'page' => $page,
'per_page' => $perPage,
$gql = <<<'GRAPHQL'
query ListBranches($owner: String!, $name: String!, $first: Int!, $after: String, $query: String) {
repository(owner: $owner, name: $name) {
refs(refPrefix: "refs/heads/", first: $first, after: $after, orderBy: {field: ALPHABETICAL, direction: ASC}, query: $query) {
totalCount
pageInfo {
hasNextPage
endCursor
}
nodes {
name
}
}
}
}
GRAPHQL;

$response = $this->call(self::METHOD_POST, '/graphql', ['Authorization' => "Bearer $this->accessToken"], [
'query' => $gql,
'variables' => [
'owner' => $owner,
'name' => $repositoryName,
'first' => $perPage,
'after' => $cursor,
'query' => $search !== '' ? $search : null,
],
]);

$statusCode = $response['headers']['status-code'] ?? 0;
$responseBody = $response['body'] ?? [];

if ($statusCode < 200 || $statusCode >= 300 || !is_array($responseBody)) {
return [];
if ($statusCode < 200 || $statusCode >= 300 || !is_array($responseBody) || array_key_exists('errors', $responseBody)) {
return ['items' => [], 'total' => 0, 'nextCursor' => null];
}

$repository = $responseBody['data']['repository'] ?? null;
$refs = is_array($repository) ? ($repository['refs'] ?? null) : null;

if (!is_array($refs)) {
return ['items' => [], 'total' => 0, 'nextCursor' => null];
}

return array_values(array_map(fn ($branch) => $branch['name'] ?? '', $responseBody));
$pageInfo = $refs['pageInfo'] ?? [];
$hasNextPage = (bool) ($pageInfo['hasNextPage'] ?? false);

return [
'items' => array_map(fn ($node) => $node['name'] ?? '', $refs['nodes'] ?? []),
'total' => (int) ($refs['totalCount'] ?? 0),
'nextCursor' => $hasNextPage ? ($pageInfo['endCursor'] ?? null) : null,
];
}

/**
Expand Down
55 changes: 37 additions & 18 deletions tests/VCS/Adapter/GitHubTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -473,11 +473,12 @@ public function testListBranches(): void
try {
$this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test');

$branches = $this->vcsAdapter->listBranches(static::$owner, $repositoryName);
$result = $this->vcsAdapter->listBranches(static::$owner, $repositoryName);

$this->assertIsArray($branches);
$this->assertNotEmpty($branches);
$this->assertContains(static::$defaultBranch, $branches);
$this->assertIsArray($result['items']);
$this->assertNotEmpty($result['items']);
$this->assertContains(static::$defaultBranch, $result['items']);
$this->assertGreaterThan(0, $result['total']);
} finally {
$this->vcsAdapter->deleteRepository(static::$owner, $repositoryName);
}
Expand Down Expand Up @@ -540,14 +541,30 @@ public function testListBranchesPagination(): void
/** @var GitHub $adapter */
$adapter = $this->vcsAdapter;

$page1 = $adapter->listBranches(static::$owner, $repositoryName, 1, 1);
$this->assertSame(['branch-a'], $page1);

$page2 = $adapter->listBranches(static::$owner, $repositoryName, 1, 2);
$this->assertSame(['branch-b'], $page2);

$all = $adapter->listBranches(static::$owner, $repositoryName, 100, 1);
$this->assertEqualsCanonicalizing([static::$defaultBranch, 'branch-a', 'branch-b'], $all);
// Page 1: first branch alphabetically
$page1 = $adapter->listBranches(static::$owner, $repositoryName, 1);
$this->assertSame(['branch-a'], $page1['items']);
$this->assertSame(3, $page1['total']);
$this->assertNotNull($page1['nextCursor']);

// Page 2: use cursor from page 1
$page2 = $adapter->listBranches(static::$owner, $repositoryName, 1, $page1['nextCursor']);
$this->assertSame(['branch-b'], $page2['items']);
$this->assertNotNull($page2['nextCursor']);

// All branches
$all = $adapter->listBranches(static::$owner, $repositoryName, 100);
$this->assertEqualsCanonicalizing([static::$defaultBranch, 'branch-a', 'branch-b'], $all['items']);
$this->assertSame(3, $all['total']);
$this->assertNull($all['nextCursor']);

// Search: substring matching
$searchResults = $adapter->listBranches(static::$owner, $repositoryName, 100, null, 'branch');
$this->assertEqualsCanonicalizing(['branch-a', 'branch-b'], $searchResults['items']);

// GitHub refs(query:) does substring matching, so 'ranch' matches 'branch-a' and 'branch-b'
$substringSearch = $adapter->listBranches(static::$owner, $repositoryName, 100, null, 'ranch');
$this->assertEqualsCanonicalizing(['branch-a', 'branch-b'], $substringSearch['items']);
} finally {
$this->vcsAdapter->deleteRepository(static::$owner, $repositoryName);
}
Expand All @@ -559,21 +576,23 @@ public function testListBranchesEmptyRepository(): void
$this->vcsAdapter->createRepository(static::$owner, $repositoryName, false);

try {
$branches = $this->vcsAdapter->listBranches(static::$owner, $repositoryName);
$result = $this->vcsAdapter->listBranches(static::$owner, $repositoryName);

$this->assertIsArray($branches);
$this->assertEmpty($branches);
$this->assertEmpty($result['items']);
$this->assertSame(0, $result['total']);
$this->assertNull($result['nextCursor']);
} finally {
$this->vcsAdapter->deleteRepository(static::$owner, $repositoryName);
}
}

public function testListBranchesNonExistingRepository(): void
{
$branches = $this->vcsAdapter->listBranches(static::$owner, 'non-existing-repo-' . \uniqid());
$result = $this->vcsAdapter->listBranches(static::$owner, 'non-existing-repo-' . \uniqid());

$this->assertIsArray($branches);
$this->assertEmpty($branches);
$this->assertEmpty($result['items']);
$this->assertSame(0, $result['total']);
$this->assertNull($result['nextCursor']);
}

public function testGetLatestCommit(): void
Expand Down