Skip to content

fix: update deployment run links#273

Merged
nathanyoung merged 2 commits into
mainfrom
feat/update-run-links
Jun 11, 2026
Merged

fix: update deployment run links#273
nathanyoung merged 2 commits into
mainfrom
feat/update-run-links

Conversation

@nathanyoung

@nathanyoung nathanyoung commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

related: https://github.com/Hyphen/hyphen-app/pull/1335

Summary

  • Route deployment run links through internal/hyphenApp instead of formatting the legacy deployment URL inline.
  • Prefer project/environment run URLs when deployment metadata is available, falling back to the legacy deployment route when it is not.
  • Use the run deployment snapshot when it has route metadata so links survive selected deployment payloads without project/environment details.
  • Add unit coverage for alternate IDs, ID fallbacks, legacy fallbacks, and snapshot selection.

Impact

Screenshot 2026-06-11 at 11 13 51 AM

Deployment commands now return app links that match the project/environment run route when enough metadata is available, while keeping the existing deployment URL fallback for incomplete payloads.

Validation

  • env HOME=/private/tmp/hx-test-home GOCACHE=/private/tmp/hx-go-cache GOMODCACHE=/Users/nathanyoung/go/pkg/mod go test ./cmd/deploy ./internal/hyphenApp

Note: go test ./... was also attempted. The sandboxed run first hit Go build-cache permissions, then an isolated-home run passed acceptance coverage but failed internal/oauth because TestStartOAuthServer could not start its local OAuth callback server in this environment.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the deployment run link generation logic by moving it from cmd/deploy/deploy.go into a new helper function DeploymentRunLinkForRun in internal/hyphenApp/hyphenApp.go, and adds comprehensive unit tests. Feedback on these changes suggests passing the models.Deployment and models.DeploymentRun structs by pointer rather than by value to avoid inefficient copying and potential runtime panics from dereferencing a nil pointer.

Comment thread internal/hyphenApp/hyphenApp.go Outdated
Comment thread cmd/deploy/deploy.go Outdated

Copilot AI left a comment

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.

Pull request overview

This PR updates how the CLI constructs deployment run links by routing them through internal/hyphenApp, preferring the newer project/environment run route when the deployment (or run snapshot) contains enough metadata, and falling back to the legacy deployment route otherwise.

Changes:

  • Added DeploymentRunLink helpers in internal/hyphenApp that build project/environment run URLs when available, with a legacy /deploy/.../runs/... fallback.
  • Updated cmd/deploy to use the centralized link builder instead of formatting the legacy URL inline.
  • Added unit tests covering alternate IDs, ID fallbacks, legacy fallbacks, and snapshot-vs-selected deployment selection.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
internal/hyphenApp/hyphenApp.go Centralizes deployment run link generation and adds logic for project/environment routes with fallback.
internal/hyphenApp/hyphenApp_test.go Adds unit coverage for the new URL selection and fallback behavior.
cmd/deploy/deploy.go Switches deploy output to use the new run link builder.

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

@nathanyoung nathanyoung requested a review from Copilot June 11, 2026 18:36
@nathanyoung nathanyoung marked this pull request as ready for review June 11, 2026 18:37
@nathanyoung nathanyoung changed the title [codex] update deployment run links fix: update deployment run links Jun 11, 2026

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@nathanyoung nathanyoung merged commit a8a3021 into main Jun 11, 2026
7 checks passed
@nathanyoung nathanyoung deleted the feat/update-run-links branch June 11, 2026 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants