From 902c4e892c1dab03515a39fa812ca36dbafcf4d0 Mon Sep 17 00:00:00 2001 From: "upsun-dispatch[bot]" Date: Thu, 15 Oct 2026 15:25:46 +0000 Subject: [PATCH 1/3] fix(log): validate service, app and worker names with the logs API Unknown --service/--app names were sent to the logs API unchecked and just returned "No logs found", and --worker forced SSH. Names are now checked against the current deployment, and workers are filtered via the API. Requested-by: Patrick Dawkins Dispatch-Run: 01M502FSZ9H4K1AGZWJ45ZYGCA Dispatch-Attempt: 1 --- integration-tests/environment_log_test.go | 91 +++++++++++- .../Environment/EnvironmentLogCommand.php | 90 +++++++++++- legacy/src/Selector/Selector.php | 139 +++++++++++------- 3 files changed, 252 insertions(+), 68 deletions(-) diff --git a/integration-tests/environment_log_test.go b/integration-tests/environment_log_test.go index d8508d38..a9be1304 100644 --- a/integration-tests/environment_log_test.go +++ b/integration-tests/environment_log_test.go @@ -35,7 +35,30 @@ func TestEnvironmentLogAPI(t *testing.T) { "environments=/projects/"+projectID+"/environments"), DefaultBranch: "main", }}) - apiHandler.SetEnvironments([]*mockapi.Environment{makeEnv(projectID, "main", "production", "active", nil)}) + mainEnv := makeEnv(projectID, "main", "production", "active", nil) + app := mockapi.App{Type: "golang:1.23", Size: "M", Disk: 2048, Mounts: map[string]mockapi.Mount{}} + webApp := func(name string) mockapi.App { + a := app + a.Name = name + return a + } + worker := func(name string) mockapi.Worker { + return mockapi.Worker{App: webApp(name)} + } + mainEnv.SetCurrentDeployment(&mockapi.Deployment{ + WebApps: map[string]mockapi.App{"app": webApp("app"), "admin": webApp("admin")}, + Services: map[string]mockapi.App{ + "db": {Name: "db", Type: "mariadb:11.4", Size: "S", Disk: 1024}, + }, + Workers: map[string]mockapi.Worker{ + "app--queue": worker("app--queue"), + "admin--queue": worker("admin--queue"), + "app--mail": worker("app--mail"), + }, + Routes: mockRoutes(), + Links: mockapi.MakeHALLinks("self=/projects/" + projectID + "/environments/main/deployment/current"), + }) + apiHandler.SetEnvironments([]*mockapi.Environment{mainEnv}) // 150 log lines, one per second, ending 20 seconds ago. end := time.Now().Add(-20 * time.Second).UTC().Truncate(time.Second) @@ -129,11 +152,11 @@ func TestEnvironmentLogAPI(t *testing.T) { assert.Equal(t, "-1", last.Get("log_kinds_mode")) // Filters. - f.Run("log", "-p", projectID, "-e", "main", "-s", "app,worker", "--severity", "warning", "--since", "5m") + f.Run("log", "-p", projectID, "-e", "main", "-s", "app,db", "--severity", "warning", "--since", "5m") mu.Lock() last = queries[len(queries)-1] mu.Unlock() - assert.Equal(t, []string{"app", "worker"}, last["services[]"]) + assert.Equal(t, []string{"app", "db"}, last["services[]"]) assert.Equal(t, []string{"EMERGENCY", "ALERT", "CRITICAL", "ERROR", "WARNING"}, last["severities[]"]) from, _ := strconv.ParseInt(last.Get("from"), 10, 64) assert.InDelta(t, time.Now().Add(-5*time.Minute).Unix(), from, 60) @@ -191,6 +214,68 @@ func TestEnvironmentLogAPI(t *testing.T) { assert.Contains(t, stdErr, c.stdErr) } + // Workers are selected with the logs API, by their qualified name. + for _, args := range [][]string{ + {"--worker", "app--queue"}, + {"--app", "app", "--worker", "queue"}, + {"--worker", "mail"}, + } { + _, stdErr, err := f.RunCombinedOutput(append([]string{"log", "-p", projectID, "-e", "main"}, args...)...) + require.NoError(t, err, stdErr) + assert.NotContains(t, stdErr, "Reading log file") + mu.Lock() + last = queries[len(queries)-1] + mu.Unlock() + expected := "app--queue" + if args[len(args)-1] == "mail" { + expected = "app--mail" + } + assert.Equal(t, []string{expected}, last["services[]"], args) + } + + // Service, app and worker names are validated against the deployment. + validNames := "admin, admin--queue, app, app--mail, app--queue, db, router" + for _, c := range []struct { + args []string + stdErr string + hint string + notInStdErr string + }{ + { + args: []string{"-s", "access"}, + stdErr: "Invalid --service: access (expected one of: " + validNames + ")", + hint: "To filter by log type, use it as an argument, e.g.: platform-test log access", + }, + { + args: []string{"-s", "db,missing"}, + stdErr: "Invalid --service: missing (expected one of: " + validNames + ")", + notInStdErr: "To filter by log type", + }, + {args: []string{"--app", "missing"}, stdErr: "Invalid --app: missing (expected one of: admin, app)"}, + {args: []string{"--worker", "queue"}, stdErr: "Ambiguous worker name: queue (matches: admin--queue, app--queue)"}, + {args: []string{"--worker", "missing"}, stdErr: "Worker not found: missing"}, + { + args: []string{"--app", "admin", "--worker", "app--queue"}, + stdErr: `App name "admin" conflicts with worker name "app--queue"`, + }, + } { + mu.Lock() + count := len(queries) + mu.Unlock() + _, stdErr, err := f.RunCombinedOutput(append([]string{"log", "-p", projectID, "-e", "main"}, c.args...)...) + assert.Error(t, err, c.args) + assert.Contains(t, stdErr, c.stdErr) + if c.hint != "" { + assert.Contains(t, stdErr, c.hint) + } + if c.notInStdErr != "" { + assert.NotContains(t, stdErr, c.notInStdErr) + } + mu.Lock() + assert.Len(t, queries, count, "no query is sent: %v", c.args) + mu.Unlock() + } + // The app can be selected by an environment variable. f.extraEnv = []string{EnvPrefix + "LOG_PROTOCOL=auto", "PLATFORM_APPLICATION_NAME=app"} f.Run("log", "-p", projectID, "-e", "main") diff --git a/legacy/src/Command/Environment/EnvironmentLogCommand.php b/legacy/src/Command/Environment/EnvironmentLogCommand.php index 0e7bf047..4d7d3cfa 100644 --- a/legacy/src/Command/Environment/EnvironmentLogCommand.php +++ b/legacy/src/Command/Environment/EnvironmentLogCommand.php @@ -9,6 +9,7 @@ use Platformsh\Cli\Console\Option; use Platformsh\Cli\Selector\Selection; use Platformsh\Cli\Selector\SelectorConfig; +use Platformsh\Cli\Service\Api; use Platformsh\Cli\Service\Config; use Platformsh\Cli\Service\Io; use Platformsh\Cli\Service\Observability; @@ -19,6 +20,7 @@ use Platformsh\Cli\Command\CommandBase; use Platformsh\Cli\Util\OsUtil; use Platformsh\Cli\Util\StringUtil; +use Platformsh\Client\Exception\EnvironmentStateException; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Exception\InvalidArgumentException; use Symfony\Component\Console\Formatter\OutputFormatter; @@ -87,6 +89,7 @@ class EnvironmentLogCommand extends CommandBase private const WINDOW_GROWTH = 4; public function __construct( + private readonly Api $api, private readonly CacheProvider $cacheProvider, private readonly Config $config, private readonly Io $io, @@ -137,7 +140,7 @@ protected function configure(): void Use --fields to choose the fields, or --format json to see everything. Logs take a few seconds to become available, so --tail shows them with a delay of about 15-20 seconds. Logs that become available later are shown when they arrive, unless they are over a minute late. - SSH is still used if the --worker, --instance or --task option is used, + SSH is still used if the --instance or --task option is used, or if the type is not listed above or ends in .log (e.g. "error.log"). EOF); $this->addExample('Display a choice of logs that can be read'); @@ -212,7 +215,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int */ private function sshReason(InputInterface $input, ?string $logType): ?string { - foreach (['worker', 'instance', 'task'] as $option) { + foreach (['instance', 'task'] as $option) { if (Option::stringOrNull($input, $option) !== null) { return sprintf('the --%s option is used', $option); } @@ -434,13 +437,9 @@ private function buildFilters(InputInterface $input, Selection $selection, ?stri $params[] = ['severities_mode', '1']; } - $services = ArrayArgument::getOption($input, 'service'); - // The app may also come from the project URL or an environment variable. - if (($app = $selection->getAppName()) !== null) { - $services[] = $app; - } + $services = $this->resolveServices($input, $selection); if ($services !== []) { - foreach (array_unique($services) as $service) { + foreach ($services as $service) { $params[] = ['services[]', $service]; } $params[] = ['services_mode', '1']; @@ -449,6 +448,81 @@ private function buildFilters(InputInterface $input, Selection $selection, ?stri return $params; } + /** + * Returns the service names to filter logs by, from the --service, --app and --worker options. + * + * The names are validated against the environment's current deployment, if there is one. + * + * @return list + */ + private function resolveServices(InputInterface $input, Selection $selection): array + { + $services = ArrayArgument::getOption($input, 'service'); + // The app may also come from the project URL or an environment variable. + $appName = $selection->getAppName(); + $worker = $input->hasOption('worker') ? Option::stringOrNull($input, 'worker') : null; + if ($worker === '') { + $worker = null; + } + + try { + $deployment = $this->api->getCurrentDeployment($selection->getEnvironment()); + } catch (EnvironmentStateException $e) { + $this->io->debug('Not validating service names: ' . $e->getMessage()); + $deployment = null; + } + + if ($deployment === null) { + if ($worker !== null) { + if (str_contains($worker, '--')) { + $services[] = $worker; + } elseif ($appName !== null) { + $services[] = $appName . '--' . $worker; + } else { + throw new InvalidArgumentException(sprintf('The worker name must include the app name, e.g.: --worker --%s', $worker)); + } + } elseif ($appName !== null) { + $services[] = $appName; + } + + return array_values(array_unique($services)); + } + + $appNames = array_map('strval', array_keys($deployment->webapps)); + $validNames = array_merge( + $appNames, + array_map('strval', array_keys($deployment->workers)), + array_map('strval', array_keys($deployment->services)), + ['router'], + ); + $validNames = array_values(array_unique($validNames)); + sort($validNames); + sort($appNames); + + foreach ($services as $service) { + if (!in_array($service, $validNames, true)) { + $message = sprintf('Invalid --service: %s (expected one of: %s)', $service, implode(', ', $validNames)); + if ($service === 'error' || isset(self::LOG_KINDS[$service])) { + $message .= "\n" . sprintf('To filter by log type, use it as an argument, e.g.: %s %s', $this->config->getStr('application.executable') . ' log', $service); + } + throw new InvalidArgumentException($message); + } + } + + if ($appName !== null && !in_array($appName, $appNames, true)) { + throw new InvalidArgumentException(sprintf('Invalid --app: %s (expected one of: %s)', $appName, implode(', ', $appNames))); + } + + if ($worker !== null) { + // The app name is only used to qualify the worker name. + $services[] = $this->selector->selectWorkerName($deployment, $input, $worker, $appName); + } elseif ($appName !== null) { + $services[] = $appName; + } + + return array_values(array_unique($services)); + } + /** * Queries a page of logs. * diff --git a/legacy/src/Selector/Selector.php b/legacy/src/Selector/Selector.php index 756dc9c7..cd4a30c2 100644 --- a/legacy/src/Selector/Selector.php +++ b/legacy/src/Selector/Selector.php @@ -31,6 +31,7 @@ use Platformsh\Client\Model\Activity; use Platformsh\Client\Model\ApiResourceBase; use Platformsh\Client\Model\BasicProjectInfo; +use Platformsh\Client\Model\Deployment\EnvironmentDeployment; use Platformsh\Client\Model\Deployment\WebApp; use Platformsh\Client\Model\Environment; use Platformsh\Client\Model\Organization\Organization; @@ -733,6 +734,86 @@ public function addTaskOption(InputDefinition $definition): static return $this; } + /** + * Finds the qualified name of the worker selected by the --worker option. + * + * The worker can be given by its qualified name (--), or by + * its short name, which is qualified with the app name if given, or + * otherwise matched against the workers of all apps. + * + * @param EnvironmentDeployment $deployment + * @param InputInterface $input + * @param string $workerOption + * The value of the --worker option. + * @param string|null $appName + * The selected app name, if any. + * + * @return string + * The qualified worker name. + */ + public function selectWorkerName(EnvironmentDeployment $deployment, InputInterface $input, string $workerOption, ?string $appName): string + { + // Check for a conflict with the --app option. + if ($appName !== null + && str_contains($workerOption, '--') + && stripos($workerOption, $appName . '--') !== 0) { + throw new InvalidArgumentException(sprintf( + 'App name "%s" conflicts with worker name "%s"', + $appName, + $workerOption, + )); + } + + // If we have the app name, load the worker directly. + if (str_contains($workerOption, '--') || $appName !== null) { + $qualifiedWorkerName = str_contains($workerOption, '--') + ? $workerOption + : $appName . '--' . $workerOption; + try { + $worker = $deployment->getWorker($qualifiedWorkerName); + } catch (\InvalidArgumentException) { + throw new InvalidArgumentException('Worker not found: ' . $workerOption . ' (in app: ' . $appName . ')'); + } + $this->stdErr->writeln(sprintf('Selected worker: %s', $worker->name), OutputInterface::VERBOSITY_VERBOSE); + + return $worker->name; + } + + // If we don't have the app name, find all the possible matching + // workers, and ask the user to choose. + $suffix = '--' . $workerOption; + $suffixLength = strlen($suffix); + $workerNames = []; + foreach ($deployment->workers as $worker) { + if (substr($worker->name, -$suffixLength) === $suffix) { + $workerNames[] = $worker->name; + } + } + if (count($workerNames) === 0) { + throw new InvalidArgumentException('Worker not found: ' . $workerOption); + } + if (count($workerNames) === 1) { + $workerName = reset($workerNames); + $this->stdErr->writeln(sprintf('Selected worker: %s', $workerName), OutputInterface::VERBOSITY_VERBOSE); + + return $workerName; + } + if (!$input->isInteractive()) { + throw new InvalidArgumentException(sprintf( + 'Ambiguous worker name: %s (matches: %s)', + $workerOption, + implode(', ', $workerNames), + )); + } + $workerName = $this->questionHelper->choose( + array_combine($workerNames, $workerNames), + 'Enter a number to choose a worker:', + ); + $this->stdErr->writeln(sprintf('Selected worker: %s', $workerName), OutputInterface::VERBOSITY_VERBOSE); + + return $workerName; + } + /** * Find what app or worker container the user wants to select. * @@ -782,63 +863,7 @@ private function selectRemoteContainer(Environment $environment, InputInterface // Handle the --worker option first, as it's more specific. $workerOption = $includeWorkers ? Option::stringOrNull($input, 'worker') : null; if ($workerOption !== null) { - // Check for a conflict with the --app option. - if ($appName !== null - && str_contains($workerOption, '--') - && stripos($workerOption, $appName . '--') !== 0) { - throw new InvalidArgumentException(sprintf( - 'App name "%s" conflicts with worker name "%s"', - $appName, - $workerOption, - )); - } - - // If we have the app name, load the worker directly. - if (str_contains($workerOption, '--') || $appName !== null) { - $qualifiedWorkerName = str_contains($workerOption, '--') - ? $workerOption - : $appName . '--' . $workerOption; - try { - $worker = $deployment->getWorker($qualifiedWorkerName); - } catch (\InvalidArgumentException) { - throw new InvalidArgumentException('Worker not found: ' . $workerOption . ' (in app: ' . $appName . ')'); - } - $this->stdErr->writeln(sprintf('Selected worker: %s', $worker->name), OutputInterface::VERBOSITY_VERBOSE); - - return new Worker($worker, $environment); - } - - // If we don't have the app name, find all the possible matching - // workers, and ask the user to choose. - $suffix = '--' . $workerOption; - $suffixLength = strlen($suffix); - $workerNames = []; - foreach ($deployment->workers as $worker) { - if (substr($worker->name, -$suffixLength) === $suffix) { - $workerNames[] = $worker->name; - } - } - if (count($workerNames) === 0) { - throw new InvalidArgumentException('Worker not found: ' . $workerOption); - } - if (count($workerNames) === 1) { - $workerName = reset($workerNames); - $this->stdErr->writeln(sprintf('Selected worker: %s', $workerName), OutputInterface::VERBOSITY_VERBOSE); - - return new Worker($deployment->getWorker($workerName), $environment); - } - if (!$input->isInteractive()) { - throw new InvalidArgumentException(sprintf( - 'Ambiguous worker name: %s (matches: %s)', - $workerOption, - implode(', ', $workerNames), - )); - } - $workerName = $this->questionHelper->choose( - array_combine($workerNames, $workerNames), - 'Enter a number to choose a worker:', - ); - $this->stdErr->writeln(sprintf('Selected worker: %s', $workerName), OutputInterface::VERBOSITY_VERBOSE); + $workerName = $this->selectWorkerName($deployment, $input, $workerOption, $appName); return new Worker($deployment->getWorker($workerName), $environment); } From 2ff5b4c30ad066e3a9cbf5dbbe986221e3ed3f65 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 19:02:45 +0100 Subject: [PATCH 2/3] fix(log): warn about unknown service names instead of failing Logs of removed or renamed services are retained, so a name that is not in the current deployment now prints a warning (with the log type hint) and is still queried, instead of failing. Qualified worker names (--, or a short name with --app) are passed through in the same way, via the new Selector::qualifyWorkerName(). Only a bare short worker name needs the deployment to be resolved. The current deployment is no longer fetched when there are no service, app or worker filters. Co-Authored-By: Claude Opus 5.5 --- integration-tests/environment_log_test.go | 52 +++++++++------- .../Environment/EnvironmentLogCommand.php | 59 ++++++++----------- legacy/src/Selector/Selector.php | 41 ++++++++----- 3 files changed, 82 insertions(+), 70 deletions(-) diff --git a/integration-tests/environment_log_test.go b/integration-tests/environment_log_test.go index a9be1304..b1edee1f 100644 --- a/integration-tests/environment_log_test.go +++ b/integration-tests/environment_log_test.go @@ -233,25 +233,39 @@ func TestEnvironmentLogAPI(t *testing.T) { assert.Equal(t, []string{expected}, last["services[]"], args) } - // Service, app and worker names are validated against the deployment. + // Names that are not in the current deployment produce a warning, but are still queried. validNames := "admin, admin--queue, app, app--mail, app--queue, db, router" for _, c := range []struct { - args []string - stdErr string - hint string - notInStdErr string + args []string + services []string + hint bool + }{ + {args: []string{"-s", "access"}, services: []string{"access"}, hint: true}, + {args: []string{"-s", "db,old"}, services: []string{"db", "old"}}, + {args: []string{"--app", "old"}, services: []string{"old"}}, + {args: []string{"--worker", "app--old"}, services: []string{"app--old"}}, + {args: []string{"--app", "app", "--worker", "old"}, services: []string{"app--old"}}, + } { + _, stdErr, err := f.RunCombinedOutput(append([]string{"log", "-p", projectID, "-e", "main"}, c.args...)...) + require.NoError(t, err, stdErr) + unknown := c.services[len(c.services)-1] + assert.Contains(t, stdErr, "Warning: the service "+unknown+" is not in the current deployment (expected one of: "+validNames+")") + if c.hint { + assert.Contains(t, stdErr, "To filter by log type, use it as an argument, e.g.: platform-test log "+unknown) + } else { + assert.NotContains(t, stdErr, "To filter by log type") + } + mu.Lock() + last = queries[len(queries)-1] + mu.Unlock() + assert.Equal(t, c.services, last["services[]"], c.args) + } + + // Short worker names that cannot be qualified are errors. + for _, c := range []struct { + args []string + stdErr string }{ - { - args: []string{"-s", "access"}, - stdErr: "Invalid --service: access (expected one of: " + validNames + ")", - hint: "To filter by log type, use it as an argument, e.g.: platform-test log access", - }, - { - args: []string{"-s", "db,missing"}, - stdErr: "Invalid --service: missing (expected one of: " + validNames + ")", - notInStdErr: "To filter by log type", - }, - {args: []string{"--app", "missing"}, stdErr: "Invalid --app: missing (expected one of: admin, app)"}, {args: []string{"--worker", "queue"}, stdErr: "Ambiguous worker name: queue (matches: admin--queue, app--queue)"}, {args: []string{"--worker", "missing"}, stdErr: "Worker not found: missing"}, { @@ -265,12 +279,6 @@ func TestEnvironmentLogAPI(t *testing.T) { _, stdErr, err := f.RunCombinedOutput(append([]string{"log", "-p", projectID, "-e", "main"}, c.args...)...) assert.Error(t, err, c.args) assert.Contains(t, stdErr, c.stdErr) - if c.hint != "" { - assert.Contains(t, stdErr, c.hint) - } - if c.notInStdErr != "" { - assert.NotContains(t, stdErr, c.notInStdErr) - } mu.Lock() assert.Len(t, queries, count, "no query is sent: %v", c.args) mu.Unlock() diff --git a/legacy/src/Command/Environment/EnvironmentLogCommand.php b/legacy/src/Command/Environment/EnvironmentLogCommand.php index 4d7d3cfa..4da68127 100644 --- a/legacy/src/Command/Environment/EnvironmentLogCommand.php +++ b/legacy/src/Command/Environment/EnvironmentLogCommand.php @@ -451,7 +451,8 @@ private function buildFilters(InputInterface $input, Selection $selection, ?stri /** * Returns the service names to filter logs by, from the --service, --app and --worker options. * - * The names are validated against the environment's current deployment, if there is one. + * Names that are not in the environment's current deployment produce a + * warning, as logs of removed services can still be read. * * @return list */ @@ -464,6 +465,9 @@ private function resolveServices(InputInterface $input, Selection $selection): a if ($worker === '') { $worker = null; } + if ($services === [] && $appName === null && $worker === null) { + return []; + } try { $deployment = $this->api->getCurrentDeployment($selection->getEnvironment()); @@ -472,55 +476,44 @@ private function resolveServices(InputInterface $input, Selection $selection): a $deployment = null; } - if ($deployment === null) { - if ($worker !== null) { - if (str_contains($worker, '--')) { - $services[] = $worker; - } elseif ($appName !== null) { - $services[] = $appName . '--' . $worker; - } else { + if ($worker !== null) { + // The app name is only used to qualify the worker name. + $qualified = $this->selector->qualifyWorkerName($worker, $appName); + if ($qualified === null) { + if ($deployment === null) { throw new InvalidArgumentException(sprintf('The worker name must include the app name, e.g.: --worker --%s', $worker)); } - } elseif ($appName !== null) { - $services[] = $appName; + $qualified = $this->selector->selectWorkerName($deployment, $input, $worker, null); } + $services[] = $qualified; + } elseif ($appName !== null) { + $services[] = $appName; + } + $services = array_values(array_unique($services)); - return array_values(array_unique($services)); + if ($deployment === null) { + return $services; } - $appNames = array_map('strval', array_keys($deployment->webapps)); $validNames = array_merge( - $appNames, + array_map('strval', array_keys($deployment->webapps)), array_map('strval', array_keys($deployment->workers)), array_map('strval', array_keys($deployment->services)), ['router'], ); $validNames = array_values(array_unique($validNames)); sort($validNames); - sort($appNames); - foreach ($services as $service) { - if (!in_array($service, $validNames, true)) { - $message = sprintf('Invalid --service: %s (expected one of: %s)', $service, implode(', ', $validNames)); - if ($service === 'error' || isset(self::LOG_KINDS[$service])) { - $message .= "\n" . sprintf('To filter by log type, use it as an argument, e.g.: %s %s', $this->config->getStr('application.executable') . ' log', $service); - } - throw new InvalidArgumentException($message); + if (in_array($service, $validNames, true)) { + continue; + } + $this->stdErr->writeln(sprintf('Warning: the service %s is not in the current deployment (expected one of: %s)', $service, implode(', ', $validNames))); + if ($service === 'error' || isset(self::LOG_KINDS[$service])) { + $this->stdErr->writeln(sprintf('To filter by log type, use it as an argument, e.g.: %s log %s', $this->config->getStr('application.executable'), $service)); } } - if ($appName !== null && !in_array($appName, $appNames, true)) { - throw new InvalidArgumentException(sprintf('Invalid --app: %s (expected one of: %s)', $appName, implode(', ', $appNames))); - } - - if ($worker !== null) { - // The app name is only used to qualify the worker name. - $services[] = $this->selector->selectWorkerName($deployment, $input, $worker, $appName); - } elseif ($appName !== null) { - $services[] = $appName; - } - - return array_values(array_unique($services)); + return $services; } /** diff --git a/legacy/src/Selector/Selector.php b/legacy/src/Selector/Selector.php index cd4a30c2..f9c0312b 100644 --- a/legacy/src/Selector/Selector.php +++ b/legacy/src/Selector/Selector.php @@ -734,6 +734,30 @@ public function addTaskOption(InputDefinition $definition): static return $this; } + /** + * Qualifies a --worker option value with the app name, if needed. + * + * @return string|null + * The qualified worker name (--), or null if the worker is + * given by its short name and there is no app name. + */ + public function qualifyWorkerName(string $workerOption, ?string $appName): ?string + { + if (str_contains($workerOption, '--')) { + if ($appName !== null && stripos($workerOption, $appName . '--') !== 0) { + throw new InvalidArgumentException(sprintf( + 'App name "%s" conflicts with worker name "%s"', + $appName, + $workerOption, + )); + } + + return $workerOption; + } + + return $appName !== null ? $appName . '--' . $workerOption : null; + } + /** * Finds the qualified name of the worker selected by the --worker option. * @@ -753,22 +777,9 @@ public function addTaskOption(InputDefinition $definition): static */ public function selectWorkerName(EnvironmentDeployment $deployment, InputInterface $input, string $workerOption, ?string $appName): string { - // Check for a conflict with the --app option. - if ($appName !== null - && str_contains($workerOption, '--') - && stripos($workerOption, $appName . '--') !== 0) { - throw new InvalidArgumentException(sprintf( - 'App name "%s" conflicts with worker name "%s"', - $appName, - $workerOption, - )); - } - // If we have the app name, load the worker directly. - if (str_contains($workerOption, '--') || $appName !== null) { - $qualifiedWorkerName = str_contains($workerOption, '--') - ? $workerOption - : $appName . '--' . $workerOption; + $qualifiedWorkerName = $this->qualifyWorkerName($workerOption, $appName); + if ($qualifiedWorkerName !== null) { try { $worker = $deployment->getWorker($qualifiedWorkerName); } catch (\InvalidArgumentException) { From 62e7fe8d5f04f5c696e46bb704b35e0727b97d1b Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 19:14:06 +0100 Subject: [PATCH 3/3] test(log): wrap a long assertion line Co-Authored-By: Claude Opus 5.5 --- integration-tests/environment_log_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integration-tests/environment_log_test.go b/integration-tests/environment_log_test.go index b1edee1f..c6434165 100644 --- a/integration-tests/environment_log_test.go +++ b/integration-tests/environment_log_test.go @@ -249,7 +249,8 @@ func TestEnvironmentLogAPI(t *testing.T) { _, stdErr, err := f.RunCombinedOutput(append([]string{"log", "-p", projectID, "-e", "main"}, c.args...)...) require.NoError(t, err, stdErr) unknown := c.services[len(c.services)-1] - assert.Contains(t, stdErr, "Warning: the service "+unknown+" is not in the current deployment (expected one of: "+validNames+")") + assert.Contains(t, stdErr, "Warning: the service "+unknown+ + " is not in the current deployment (expected one of: "+validNames+")") if c.hint { assert.Contains(t, stdErr, "To filter by log type, use it as an argument, e.g.: platform-test log "+unknown) } else {