Skip to content

fix(logs): resolve branch URLs containing double hyphens - #8607

Open
r4venrudy wants to merge 1 commit into
netlify:mainfrom
r4venrudy:fix/logs-branch-url
Open

r4venrudy wants to merge 1 commit into
netlify:mainfrom
r4venrudy:fix/logs-branch-url

Conversation

@r4venrudy

Copy link
Copy Markdown

Summary

Fixes #8606.

netlify logs --url https://feature--logs--my-site.netlify.app rejects the linked site because the resolver splits on the first --. Match the complete linked-site hostname suffix instead, preserving the entire branch name, including when the site name also contains --. Reject empty prefixes and extra hostname labels before making an API call.

Add ten resolver cases covering double-hyphen branch/site names, canonical URLs, deploy permalinks, and invalid hosts. Four cases fail against the original resolver.

Validation

  • Logging tests: 44 passed across three files.
  • npm run typecheck: passed.
  • ESLint and oxfmt on the changed files: passed.
  • git diff --check: passed.
  • Full unit run on Windows/Node 24.14.1: 735 passed, 1 failed. The hosted-release symlink test times out; the same isolated test also times out against the original source. The full run additionally reports an unhandled telemetry rejection from the existing unsupported-Windows traffic-mesh test.
  • No live Netlify sites or credentials were used.

Checklist

@r4venrudy
r4venrudy requested a review from a team as a code owner October 11, 2026 15:47
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 59bd05da-1aa5-48c2-b57f-5846abeafcd7

📥 Commits

Reviewing files that changed from the base of the PR and between 31ed36f and 70b0cf0.


📒 Files selected for processing (2)
  • src/commands/logs/log-api.ts
  • tests/unit/commands/logs/log-api.test.ts

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:


Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.



📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Improved deploy log lookup for preview URLs involving hyphenated site names or branch names.
    • Invalid preview URLs are now rejected rather than being used for a deploy lookup.

Walkthrough

resolveDeployIdFromUrl now matches deploy-preview hostnames against the full --<site-name>.netlify.app suffix. It extracts the preceding branch prefix and rejects an empty prefix or one containing a dot. Unit tests cover hyphenated site and branch names, deploy permalinks, canonical site URLs, and invalid branch URLs.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: amun-sihra


Merge Risk: ⚪ Minimal · up to 70b0c

No actionable issue remains that would prevent merging after normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to 70b0c

The change affects 2 systems.

Changed systems: src, tests

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.
  • observed — tests (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/commands/logs/log-api.ts: resolveDeployIdFromUrl now requires the hostname to end with --<site-name>.netlify.app and extracts the entire preceding prefix, rejecting an empty prefix or one containing a dot. This replaces parsing the first hostname label at its first -- and checking the separately extracted suffix against the site name.
  • observed — Modified behavior in tests/unit/commands/logs/log-api.test.ts: Added tests for four combinations of hyphenated site names and branch names, checking the resolved deploy ID and that deploy lookup uses the supplied site ID, branch, and per_page: 20.
  • observed — Modified behavior in tests/unit/commands/logs/log-api.test.ts: Added tests that a 24-character deploy permalink resolves directly and a canonical site URL resolves to undefined, with neither case calling the deploy-list API.
  • observed — Modified behavior in tests/unit/commands/logs/log-api.test.ts: Added tests for four invalid branch URL shapes, expecting the linked-project mismatch error before any deploy-list API call.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check Passed The title clearly identifies the fix for branch URLs that contain double hyphens, which is the primary change.
Description check Passed The description directly explains the resolver bug, the required behavior, the added regression tests, and validation results.
Linked Issues check Passed Issue #8606 requires branch deploy URLs to preserve -- in branch and site names, and to reject empty prefixes and extra hostname labels before the deploy lookup. resolveDeployIdFromUrl now matches…
Out of Scope Changes check Passed The pull request changes only resolveDeployIdFromUrl and its unit tests. The implementation and tests directly support issue #8606. No unrelated product behavior or unrelated files appear in the rev…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…

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

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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.

logs --url rejects branch deploy URLs when the branch contains double hyphens

1 participant