Skip to content

feat: support branch-scoped preview deployments + cleanup#16

Open
lustsazeus-lab wants to merge 5 commits into
ubiquity:mainfrom
lustsazeus-lab:feat/issue-7-branch-previews
Open

feat: support branch-scoped preview deployments + cleanup#16
lustsazeus-lab wants to merge 5 commits into
ubiquity:mainfrom
lustsazeus-lab:feat/issue-7-branch-previews

Conversation

@lustsazeus-lab
Copy link
Copy Markdown

Summary

This PR implements branch-scoped preview deployments for the reusable Deno Deploy workflow, plus automatic cleanup support when branches are deleted.

Changes

  • Added preview_strategy input to deno-deploy-reusable.yml
    • default: branch
    • optional legacy mode: shared
  • Branch preview naming now derives from branch refs (slugified + length-safe hash fallback)
    • example: feat/widget + pay-ubq-fi -> feat-widget-pay-ubq-fi
  • Router preview URL computation now supports branch-specific hosts (<branch>-<subdomain>.ubq.fi)
  • Added reusable cleanup workflow:
    • .github/workflows/deno-deploy-preview-cleanup.yml
    • deletes branch preview projects on delete-triggered runs
  • Updated README with usage docs for:
    • preview_strategy
    • delete-event cleanup integration

Validation

  • ./.tools/actionlint/actionlint

Fixes #7
/claim #7

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 41beacbd-3019-441f-87d2-a86db8901169

📥 Commits

Reviewing files that changed from the base of the PR and between c8e1400 and c750009.

📒 Files selected for processing (2)
  • .github/workflows/deno-deploy-preview-cleanup.yml
  • .github/workflows/deno-deploy-reusable.yml

📝 Walkthrough

Walkthrough

Adds a new reusable GitHub Actions workflow (deno-deploy-preview-cleanup.yml) to delete Deno Deploy preview projects on branch deletion. The cleanup workflow validates inputs, computes or accepts the preview project name (with branch/shared strategies, slugging, truncation and sha1 suffixing), can skip deletion for production branches, and issues a DELETE to the Deno API (treating 404 as success). The deployment workflow gains preview_strategy (default branch), branch slugging, length/hash handling, router_host and branch_slug outputs, and URL resolution preferring ROUTER_HOST. README updated accordingly.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding branch-scoped preview deployments and automatic cleanup for the Deno Deploy workflow.
Description check ✅ Passed The description is directly related to the changeset, detailing branch-scoped previews, cleanup workflow, and README updates with validation confirmation.
Linked Issues check ✅ Passed The PR meets all objectives from issue #7: supports branch-name prefixes for preview hostnames, creates separate projects per branch, detects deletions, and auto-deletes corresponding projects.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #7: new reusable workflows, preview strategy logic, router URL computation, and documentation updates align with stated objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e18a3d0ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +516 to +517
if [ $max_branch_len -lt 6 ]; then
echo "::error::Cannot build branch preview name for base '${base}' within Deno project length limits"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid hard-failing branch previews for longer project names

Because preview_strategy now defaults to branch, this check causes preview deployments to fail for any valid project whose base name is longer than 12 characters (for example notifications-ubq-fi, where max_branch_len becomes 5). Existing repos that do not explicitly opt back to shared will start failing on non-production branches, even though those previews previously deployed successfully.

Useful? React with 👍 / 👎.

RyanAI and others added 2 commits March 4, 2026 07:56
…t names

When a project's base name is too long (e.g. notifications-ubq-fi with
base='notifications'), max_branch_len becomes <6, making branch-based
preview naming impractical (0-char branch slug + hash = ugly leading
hyphens like '-a1b2c-notifications-ubq-fi').

Instead of exiting with an error, fall back to shared preview naming
silently so repos with long names don't break after upgrading without
having to explicitly set strategy=shared.

P1 feedback from chatgpt-codex-connector[bot] on PR ubiquity#16.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ed9b06fe-0992-41fd-96de-0cea0c9b7c1c

📥 Commits

Reviewing files that changed from the base of the PR and between 9c32848 and 27cbead.

📒 Files selected for processing (3)
  • .github/workflows/deno-deploy-preview-cleanup.yml
  • .github/workflows/deno-deploy-reusable.yml
  • README.md

Comment thread .github/workflows/deno-deploy-preview-cleanup.yml
Comment thread .github/workflows/deno-deploy-reusable.yml
Comment thread README.md
… naming

- deno-deploy-reusable.yml: Add explicit 'strategy=shared' block to handle the case
  where strategy is explicitly set to 'shared' with mode=preview. Previously the
  preview variable was never set in this case, causing DNS-validation to fail on
  an empty string. Now shared strategy (whether explicit or auto-switched due to
  long branch names) is handled consistently.

- deno-deploy-preview-cleanup.yml: Align empty-slug handling with deploy workflow.
  When slugify returns empty, fall back to 'preview' slug (like deploy does)
  instead of silently skipping. Also add auto-switch to shared strategy when
  max_branch_len < 6, consistent with deploy behavior.

- README.md: Document the auto-fallback-to-shared behavior for repos with
  very long base names that cannot fit any branch preview prefix.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 95a0b676-1b7a-4d93-8144-1871c9a53bfc

📥 Commits

Reviewing files that changed from the base of the PR and between 27cbead and c8e1400.

📒 Files selected for processing (3)
  • .github/workflows/deno-deploy-preview-cleanup.yml
  • .github/workflows/deno-deploy-reusable.yml
  • README.md
✅ Files skipped from review due to trivial changes (2)
  • README.md
  • .github/workflows/deno-deploy-reusable.yml

Comment thread .github/workflows/deno-deploy-preview-cleanup.yml
Comment thread .github/workflows/deno-deploy-preview-cleanup.yml
… project deletion

Address CodeRabbit feedback:
- Validate preview_strategy is 'branch' or 'shared' in both deno-deploy-reusable.yml
  and deno-deploy-preview-cleanup.yml; fail fast on unknown values to prevent
  the wrong preview project from being deleted.
- Add explicit guard in cleanup workflow: refuse to DELETE the production project
  if target resolves to the base project name (prevents catastrophic deletion).

Fixes remaining issues from ubiquity#16 CodeRabbit review.
@nagiexplorer88
Copy link
Copy Markdown

There is still a branch-collision case in the default branch strategy.

In both the deploy and cleanup workflows, when the base project name leaves fewer than six chars for the branch prefix, the code switches strategy="shared":

suffix="-${base}-ubq-fi"
max_branch_len=$((26 - ${#suffix}))
if [ $max_branch_len -lt 6 ]; then
  strategy="shared"
fi

For a repo like the comment's own example notifications-ubq-fi, base=notifications, so suffix=-notifications-ubq-fi is 21 chars and max_branch_len is 5. That means preview_strategy: branch silently falls back to p-notifications-ubq-fi / preview-notifications.ubq.fi, so feat/a and fix/b still deploy to the same preview target instead of separate branch-scoped previews.

This seems to miss the core requirement from #7: "support any branch as the prefix instead of collapsing all of them into preview- prefixes." It probably needs a deterministic shortened/hashed base for long project names, or should fail loudly instead of silently reverting to shared preview mode.

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.

All Branches Supported for Previews

2 participants