From cfe655cd43a149b2733ffaa9f4f01d7ad0461897 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Wed, 16 Sep 2026 09:54:06 -0500 Subject: [PATCH] fix: report whether an issue fork exists and stop setup-remote when it does not issue:get-fork printed fork URLs for forks that do not exist, and the json and llm formats gave no way to tell that from a fork with no branches. issue:setup-remote then added the remote, failed on the fetch, and left the dead remote behind. The project lookup already answers the question, so the action now records its outcome as an exists flag on the result. The branch lookup is separate so a failed listing cannot report a missing fork. setup-remote and checkout stop before touching git when the flag is false. Fixes #379 Co-Authored-By: Claude Fable 5.1 --- skill-data/drupalorg-cli/SKILL.md | 4 ++ skill-data/drupalorg-work-on-issue/SKILL.md | 8 ++-- src/Api/Action/Issue/GetIssueForkAction.php | 22 +++++++---- .../Action/Issue/SetupIssueRemoteAction.php | 6 +++ src/Api/Result/Issue/IssueForkResult.php | 5 +++ src/Cli/Command/Issue/Checkout.php | 7 ++++ src/Cli/Command/Issue/GetFork.php | 8 +++- src/Cli/Formatter/LlmFormatter.php | 2 + src/Cli/Formatter/MarkdownFormatter.php | 1 + .../Action/Issue/GetIssueForkActionTest.php | 38 +++++++++++++++++++ .../Issue/SetupIssueRemoteActionTest.php | 28 ++++++++++++++ tests/src/Formatter/LlmFormatterTest.php | 2 + tests/src/Formatter/MarkdownFormatterTest.php | 2 + 13 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 tests/src/Action/Issue/SetupIssueRemoteActionTest.php diff --git a/skill-data/drupalorg-cli/SKILL.md b/skill-data/drupalorg-cli/SKILL.md index dea6e162..362a646f 100644 --- a/skill-data/drupalorg-cli/SKILL.md +++ b/skill-data/drupalorg-cli/SKILL.md @@ -94,10 +94,13 @@ drupalorg issue:show --with-comments --include-bot-comments --format=llm # Show the GitLab issue fork URLs and branches # nid is optional; auto-detected from the branch name if omitted +# false means nobody has clicked "Create issue fork" yet; +# an empty list with true means the fork has no pushes drupalorg issue:get-fork [nid] --format=llm # Add the GitLab issue fork as a git remote and fetch it # nid is optional; auto-detected from the branch name if omitted +# Fails without adding a remote when the fork does not exist yet drupalorg issue:setup-remote [nid] # Check out a branch from the GitLab issue fork @@ -270,6 +273,7 @@ drupalorg mr:list [nid] --format=llm --no-cache | `No patch found on issue` | Issue has no file attachments | Check `issue:show` to confirm files exist | | `No branch configured` | `issue:patch` run outside a git repo or without a tracking branch | Run `issue:branch ` first | | `Remote … does not exist` | `issue:checkout` run before `issue:setup-remote` | Run `issue:setup-remote ` first | +| `No issue fork for … yet` | `issue:setup-remote` or `issue:checkout` run before the fork was created on GitLab | Create the fork from the issue page (or `issue:fork ` on work item projects), then retry | | `429 / 503` | Drupal.org rate limit or maintenance | The client retries automatically; wait and retry if it persists | ## References diff --git a/skill-data/drupalorg-work-on-issue/SKILL.md b/skill-data/drupalorg-work-on-issue/SKILL.md index 875259ae..8f50d000 100644 --- a/skill-data/drupalorg-work-on-issue/SKILL.md +++ b/skill-data/drupalorg-work-on-issue/SKILL.md @@ -66,9 +66,11 @@ asking the user if `CLAUDE.md` provides no guidance. - **No matches** → note that no branches exist yet and ask the user how to proceed (e.g. create a new branch from the upstream project default branch). -**No fork at all (GitLab work item projects):** If `issue:get-fork` reports no fork -exists AND the project uses GitLab work items (the ref is a `project_name#nid` or -work item URL, not a classic Drupal.org NID), offer to create one: +**No fork at all:** `issue:get-fork` prints `false` when nobody has +created the fork yet. `issue:setup-remote` and `issue:checkout` refuse to run in that +state. For a classic Drupal.org issue, ask the user to click "Create issue fork" on +the issue page. If the project uses GitLab work items (the ref is a `project_name#nid` +or work item URL, not a classic Drupal.org NID), offer to create one: ```bash drupalorg issue:fork diff --git a/src/Api/Action/Issue/GetIssueForkAction.php b/src/Api/Action/Issue/GetIssueForkAction.php index 97a610df..29bd7b0d 100644 --- a/src/Api/Action/Issue/GetIssueForkAction.php +++ b/src/Api/Action/Issue/GetIssueForkAction.php @@ -37,16 +37,23 @@ public function __invoke( $sshUrl = 'git@git.drupal.org:' . $gitLabProjectPath . '.git'; $httpsUrl = 'https://git.drupalcode.org/' . $gitLabProjectPath . '.git'; - $branches = []; try { $project = $this->gitLabClient->getProject($gitLabProjectPath); - $branchObjects = $this->gitLabClient->getBranches((int) $project->id); - $branches = array_map( - static fn(\stdClass $b) => (string) $b->name, - $branchObjects - ); } catch (\Exception $e) { - // Fork may not exist yet; return URL info without branches. + // GitLab answers 404 until someone clicks "Create issue fork". + $project = null; + } + + $branches = []; + if ($project !== null) { + try { + $branches = array_map( + static fn(\stdClass $b) => (string) $b->name, + $this->gitLabClient->getBranches((int) $project->id) + ); + } catch (\Exception $e) { + // The fork exists; a failed branch listing must not report it missing. + } } return new IssueForkResult( @@ -54,6 +61,7 @@ public function __invoke( sshUrl: $sshUrl, httpsUrl: $httpsUrl, gitLabProjectPath: $gitLabProjectPath, + exists: $project !== null, branches: $branches, ); } diff --git a/src/Api/Action/Issue/SetupIssueRemoteAction.php b/src/Api/Action/Issue/SetupIssueRemoteAction.php index 842890cb..bcd0ecf6 100644 --- a/src/Api/Action/Issue/SetupIssueRemoteAction.php +++ b/src/Api/Action/Issue/SetupIssueRemoteAction.php @@ -30,6 +30,12 @@ public function __invoke( ): SetupIssueRemoteResult { $getFork = new GetIssueForkAction($this->client, $this->gitLabClient); $fork = $getFork($nid, $projectMachineName, $repositoryProject); + if (!$fork->exists) { + throw new \RuntimeException(sprintf( + 'No issue fork for %s yet. Create it on the issue page and click "Get push access", then run this again.', + $nid + )); + } $remoteName = $fork->remoteName; $sshUrl = $fork->sshUrl; diff --git a/src/Api/Result/Issue/IssueForkResult.php b/src/Api/Result/Issue/IssueForkResult.php index 165c9709..a59cdbb8 100644 --- a/src/Api/Result/Issue/IssueForkResult.php +++ b/src/Api/Result/Issue/IssueForkResult.php @@ -7,6 +7,9 @@ class IssueForkResult implements ResultInterface { /** + * @param bool $exists + * Whether the fork project exists on GitLab. An empty branch list alone + * cannot tell a missing fork from a fork nobody has pushed to. * @param string[] $branches */ public function __construct( @@ -14,6 +17,7 @@ public function __construct( public readonly string $sshUrl, public readonly string $httpsUrl, public readonly string $gitLabProjectPath, + public readonly bool $exists, public readonly array $branches, ) { } @@ -25,6 +29,7 @@ public function jsonSerialize(): mixed 'ssh_url' => $this->sshUrl, 'https_url' => $this->httpsUrl, 'gitlab_project_path' => $this->gitLabProjectPath, + 'exists' => $this->exists, 'branches' => $this->branches, ]; } diff --git a/src/Cli/Command/Issue/Checkout.php b/src/Cli/Command/Issue/Checkout.php index 611da4e9..f384b60b 100644 --- a/src/Cli/Command/Issue/Checkout.php +++ b/src/Cli/Command/Issue/Checkout.php @@ -30,6 +30,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int $repositoryProject = ProjectRemote::detect()?->machineName; $action = new GetIssueForkAction($this->client, $gitLabClient); $fork = $action($this->nid, $explicitProject, $repositoryProject); + if (!$fork->exists) { + $this->stdErr->writeln(sprintf( + 'No issue fork for %s yet. Create it on the issue page and click "Get push access".', + $this->nid + )); + return 1; + } // Verify the remote exists locally; offer to set it up if missing. $checkRemote = new Process(['git', 'remote', 'get-url', $fork->remoteName]); diff --git a/src/Cli/Command/Issue/GetFork.php b/src/Cli/Command/Issue/GetFork.php index 0c056d9f..fe328577 100644 --- a/src/Cli/Command/Issue/GetFork.php +++ b/src/Cli/Command/Issue/GetFork.php @@ -43,6 +43,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->stdOut->writeln(sprintf('HTTPS URL: %s', $result->httpsUrl)); $this->stdOut->writeln(sprintf('GitLab path: %s', $result->gitLabProjectPath)); + if (!$result->exists) { + $this->stdOut->writeln(''); + $this->stdOut->writeln('Fork does not exist yet. Create it on the issue page and click "Get push access".'); + return 0; + } + if ($result->branches !== []) { $this->stdOut->writeln(''); $this->stdOut->writeln('Branches:'); @@ -50,7 +56,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->stdOut->writeln(' ' . $branch); } } else { - $this->stdOut->writeln('No branches found (fork may not exist yet).'); + $this->stdOut->writeln('Fork exists but has no branches yet.'); } return 0; diff --git a/src/Cli/Formatter/LlmFormatter.php b/src/Cli/Formatter/LlmFormatter.php index 96b7448c..b0fb7539 100644 --- a/src/Cli/Formatter/LlmFormatter.php +++ b/src/Cli/Formatter/LlmFormatter.php @@ -147,6 +147,7 @@ protected function formatIssueFork(IssueForkResult $result): string $sshUrl = $this->xmlEscape($result->sshUrl); $httpsUrl = $this->xmlEscape($result->httpsUrl); $gitLabPath = $this->xmlEscape($result->gitLabProjectPath); + $exists = $result->exists ? 'true' : 'false'; $branchItems = ''; foreach ($result->branches as $branch) { @@ -159,6 +160,7 @@ protected function formatIssueFork(IssueForkResult $result): string {$sshUrl} {$httpsUrl} {$gitLabPath} + {$exists} {$branchItems} diff --git a/src/Cli/Formatter/MarkdownFormatter.php b/src/Cli/Formatter/MarkdownFormatter.php index 1d061489..8507de16 100644 --- a/src/Cli/Formatter/MarkdownFormatter.php +++ b/src/Cli/Formatter/MarkdownFormatter.php @@ -118,6 +118,7 @@ protected function formatIssueFork(IssueForkResult $result): string $lines[] = "- **SSH URL:** {$result->sshUrl}"; $lines[] = "- **HTTPS URL:** {$result->httpsUrl}"; $lines[] = "- **GitLab path:** {$result->gitLabProjectPath}"; + $lines[] = '- **Exists:** ' . ($result->exists ? 'yes' : 'no'); if ($result->branches !== []) { $lines[] = ''; $lines[] = '## Branches'; diff --git a/tests/src/Action/Issue/GetIssueForkActionTest.php b/tests/src/Action/Issue/GetIssueForkActionTest.php index 48f300c3..29a8c623 100644 --- a/tests/src/Action/Issue/GetIssueForkActionTest.php +++ b/tests/src/Action/Issue/GetIssueForkActionTest.php @@ -61,9 +61,46 @@ public function testForkWithBranches(): void self::assertSame('git@git.drupal.org:issue/drupal-3383637.git', $result->sshUrl); self::assertSame('https://git.drupalcode.org/issue/drupal-3383637.git', $result->httpsUrl); self::assertSame('issue/drupal-3383637', $result->gitLabProjectPath); + self::assertTrue($result->exists); self::assertSame(['3383637-test-issue', 'main'], $result->branches); } + public function testForkWithoutBranchesStillExists(): void + { + $project = new \stdClass(); + $project->id = 12345; + + $client = $this->createMock(Client::class); + $client->method('getNode')->willReturn(self::makeIssueNode()); + + $gitLabClient = $this->createMock(GitLabClient::class); + $gitLabClient->method('getProject')->willReturn($project); + $gitLabClient->method('getBranches')->willReturn([]); + + $result = (new GetIssueForkAction($client, $gitLabClient))('3383637'); + + self::assertTrue($result->exists); + self::assertSame([], $result->branches); + } + + public function testBranchListingFailureDoesNotReportMissingFork(): void + { + $project = new \stdClass(); + $project->id = 12345; + + $client = $this->createMock(Client::class); + $client->method('getNode')->willReturn(self::makeIssueNode()); + + $gitLabClient = $this->createMock(GitLabClient::class); + $gitLabClient->method('getProject')->willReturn($project); + $gitLabClient->method('getBranches')->willThrowException(new \Exception('Service Unavailable', 503)); + + $result = (new GetIssueForkAction($client, $gitLabClient))('3383637'); + + self::assertTrue($result->exists); + self::assertSame([], $result->branches); + } + public function testExplicitProjectSkipsNodeLookup(): void { $client = $this->createMock(Client::class); @@ -111,6 +148,7 @@ public function testForkNotYetCreated(): void self::assertSame('drupal-3383637', $result->remoteName); self::assertSame('git@git.drupal.org:issue/drupal-3383637.git', $result->sshUrl); self::assertSame('issue/drupal-3383637', $result->gitLabProjectPath); + self::assertFalse($result->exists); self::assertSame([], $result->branches); } } diff --git a/tests/src/Action/Issue/SetupIssueRemoteActionTest.php b/tests/src/Action/Issue/SetupIssueRemoteActionTest.php new file mode 100644 index 00000000..490adc8d --- /dev/null +++ b/tests/src/Action/Issue/SetupIssueRemoteActionTest.php @@ -0,0 +1,28 @@ +createMock(Client::class); + $client->expects(self::never())->method('getNode'); + + $gitLabClient = $this->createMock(GitLabClient::class); + $gitLabClient->method('getProject')->willThrowException(new \Exception('Not Found', 404)); + + $action = new SetupIssueRemoteAction($client, $gitLabClient); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('No issue fork for 3007808 yet.'); + $action('3007808', 'poll'); + } +} diff --git a/tests/src/Formatter/LlmFormatterTest.php b/tests/src/Formatter/LlmFormatterTest.php index 5254d5c3..5be25226 100644 --- a/tests/src/Formatter/LlmFormatterTest.php +++ b/tests/src/Formatter/LlmFormatterTest.php @@ -248,6 +248,7 @@ public function testIssueForkResult(): void sshUrl: 'git@git.drupal.org:issue/drupal-3383637.git', httpsUrl: 'https://git.drupalcode.org/issue/drupal-3383637.git', gitLabProjectPath: 'issue/drupal-3383637', + exists: true, branches: ['3383637-fix-the-thing', 'main'], ); @@ -259,6 +260,7 @@ public function testIssueForkResult(): void self::assertStringContainsString('git@git.drupal.org:issue/drupal-3383637.git', $output); self::assertStringContainsString('https://git.drupalcode.org/issue/drupal-3383637.git', $output); self::assertStringContainsString('issue/drupal-3383637', $output); + self::assertStringContainsString('true', $output); self::assertStringContainsString('3383637-fix-the-thing', $output); self::assertStringContainsString('main', $output); } diff --git a/tests/src/Formatter/MarkdownFormatterTest.php b/tests/src/Formatter/MarkdownFormatterTest.php index e9d0bb1e..f57c391b 100644 --- a/tests/src/Formatter/MarkdownFormatterTest.php +++ b/tests/src/Formatter/MarkdownFormatterTest.php @@ -212,6 +212,7 @@ public function testIssueForkResult(): void sshUrl: 'git@git.drupal.org:issue/drupal-3383637.git', httpsUrl: 'https://git.drupalcode.org/issue/drupal-3383637.git', gitLabProjectPath: 'issue/drupal-3383637', + exists: true, branches: ['3383637-fix-the-thing', 'main'], ); @@ -223,6 +224,7 @@ public function testIssueForkResult(): void self::assertStringContainsString('**SSH URL:** git@git.drupal.org:issue/drupal-3383637.git', $output); self::assertStringContainsString('**HTTPS URL:** https://git.drupalcode.org/issue/drupal-3383637.git', $output); self::assertStringContainsString('**GitLab path:** issue/drupal-3383637', $output); + self::assertStringContainsString('**Exists:** yes', $output); self::assertStringContainsString('## Branches', $output); self::assertStringContainsString('- 3383637-fix-the-thing', $output); self::assertStringContainsString('- main', $output);