Decouple resources overview from chorus

[mharacewiat]

Migrated from platformsh/legacy-cli#1596.

[Copilot]

Pull request overview

This PR updates the legacy CLI metrics flow to obtain the “resources overview” URL via the environment’s /observability/ entrypoint response, rather than relying on the #observability-pipeline link (decoupling the overview lookup from chorus-specific linkage).

Changes:

• Replace #observability-pipeline link usage with a GET to {environmentUri}/observability/ to discover _links.resources_overview.href.
• Adjust return docstring to reflect returning a URL (or false) when unavailable.

---

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

[pjcdawkins]

Reviewed. The change itself is sound: I checked the happy path against the live API and the observability/ entrypoint returns an absolute href and responds 200 for inactive and paused environments, so the discovery works. The earlier Copilot points (404-only handling, the is_array() guard) are addressed in 908b343. All four checks pass.

Nothing here is blocking. There is one question that needs a maintainer decision rather than a code fix, and three smaller items.

The decision: host trust

MetricsCommandBase.php:135 follows the returned href verbatim with the OAuth-authenticated client, with no host check. Two pieces of context that make this worth an explicit call rather than a nit:

• vendor/platformsh/oauth2/src/GuzzleMiddleware.php:181-185 attaches the bearer token to every request made through Api::getHttpClient(), regardless of host.
• ApiResourceBase::makeAbsoluteUrl() (vendor/platformsh/client/src/Model/ApiResourceBase.php:568-581) deliberately overrides an absolute link host with the resource's base host, specifically so that link hrefs cannot redirect requests elsewhere.

So the client library has an existing guard for exactly this pattern, and this code path sits outside it: the entrypoint is fetched from the region and the host it returns is then trusted for a token-bearing request. There is precedent for following body-supplied hrefs (TeamListCommand.php:187), so this is hardening rather than a new class of issue — but it is a region-to-central-API hop, so it should be a deliberate choice.

Smaller items

• MetricsCommandBase.php:135 — the href is mixed while the method declares false|string. Under declare(strict_types=1) a non-string href raises a TypeError instead of taking the handled "not available" path. PHPStan will not catch this below level 9 (checkExplicitMixed is off, and there is no baseline entry for this file). Folding is_string(...) into the existing guard covers it.
• MetricsCommandBase.php:169 — the query string is still appended by concatenation, but the URL is now server-controlled. An href carrying its own query would produce ...?view=default?range=600. Latent today since the current response has no query part; building the request with Guzzle's query option would remove the class of problem.
• MetricsCommandBase.php:117 — one extra serial, uncached round trip per metrics invocation, and a transient 5xx on the entrypoint now aborts the command even when the metrics endpoint itself is healthy. The URL is stable per environment and Api already has a cache.

Tests

None added. For context rather than criticism: legacy/tests/ has no Command/Metrics/ coverage and no Guzzle MockHandler infrastructure at all, so this matches existing practice rather than regressing it. The new branching is pure logic and would be testable if the discovery were extracted out of the private method.

One thing I did not check: whether the observability entrypoint is present for every region and vendor this binary ships to. The fallback is "metrics not available", so it is worth confirming on the rollout side.

---

Review by Claude Code.