fix: resolve tenant-specific URLs via drive meta API#932
Closed
KhanCold wants to merge 8 commits into
Closed
Conversation
…ack to brand-standard hosts Replace direct BuildResourceURL calls with FetchTenantResourceURL in all shortcuts that create resources and previously emitted hard-coded www.feishu.cn / www.larksuite.com links: - drive +upload - drive +import - docs +create (v1 fallback) - docs +create (v2 fallback) - markdown +create - wiki +node-create FetchTenantResourceURL queries the drive meta batch_query API with with_url=true to obtain the tenant's actual vanity domain (e.g. example.feishu.cn). This fixes broken links for private-deployment and custom-domain tenants where the generic hosts do not redirect correctly. If the meta lookup fails or returns no URL, the function falls back to BuildResourceURL, preserving existing behavior for public-cloud tenants. Closes #810, closes #925
…ack to brand-standard hosts Replace direct BuildResourceURL calls with FetchTenantResourceURL in all shortcuts that create resources and previously emitted hard-coded www.feishu.cn / www.larksuite.com links: - drive +upload - drive +import - docs +create (v1 fallback) - docs +create (v2 fallback) - markdown +create - wiki +node-create FetchTenantResourceURL queries the drive meta batch_query API with with_url=true to obtain the tenant actual vanity domain (e.g. example.feishu.cn). This fixes broken links for private-deployment and custom-domain tenants where the generic hosts do not redirect correctly. If the meta lookup fails or returns no URL, the function falls back to BuildResourceURL, preserving existing behavior for public-cloud tenants. Closes #810, closes #925
…ack to brand-standard hosts Replace direct BuildResourceURL calls with FetchTenantResourceURL in all shortcuts that create resources and previously emitted hard-coded www.feishu.cn / www.larksuite.com links: - drive +upload - drive +import - docs +create (v1 fallback) - docs +create (v2 fallback) - markdown +create - wiki +node-create FetchTenantResourceURL queries the drive meta batch_query API with with_url=true to obtain the tenant actual vanity domain (e.g. example.feishu.cn). This fixes broken links for private-deployment and custom-domain tenants where the generic hosts do not redirect correctly. If the meta lookup fails or returns no URL, the function falls back to BuildResourceURL, preserving existing behavior for public-cloud tenants. Closes #810, closes #925
…ack to brand-standard hosts Replace direct BuildResourceURL calls with FetchTenantResourceURL in all shortcuts that create resources and previously emitted hard-coded www.feishu.cn / www.larksuite.com links: - drive +upload - drive +import - docs +create (v1 fallback) - docs +create (v2 fallback) - markdown +create - wiki +node-create FetchTenantResourceURL queries the drive meta batch_query API with with_url=true to obtain the tenant actual vanity domain (e.g. example.feishu.cn). This fixes broken links for private-deployment and custom-domain tenants where the generic hosts do not redirect correctly. If the meta lookup fails or returns no URL, the function falls back to BuildResourceURL, preserving existing behavior for public-cloud tenants. Closes #810, closes #925
…ack to brand-standard hosts Replace direct BuildResourceURL calls with FetchTenantResourceURL in all shortcuts that create resources and previously emitted hard-coded www.feishu.cn / www.larksuite.com links: - drive +upload - drive +import - docs +create (v1 fallback) - docs +create (v2 fallback) - markdown +create - wiki +node-create FetchTenantResourceURL queries the drive meta batch_query API with with_url=true to obtain the tenant actual vanity domain (e.g. example.feishu.cn). This fixes broken links for private-deployment and custom-domain tenants where the generic hosts do not redirect correctly. If the meta lookup fails or returns no URL, the function falls back to BuildResourceURL, preserving existing behavior for public-cloud tenants. Closes #810, closes #925
…ack to brand-standard hosts Replace direct BuildResourceURL calls with FetchTenantResourceURL in all shortcuts that create resources and previously emitted hard-coded www.feishu.cn / www.larksuite.com links: - drive +upload - drive +import - docs +create (v1 fallback) - docs +create (v2 fallback) - markdown +create - wiki +node-create FetchTenantResourceURL queries the drive meta batch_query API with with_url=true to obtain the tenant actual vanity domain (e.g. example.feishu.cn). This fixes broken links for private-deployment and custom-domain tenants where the generic hosts do not redirect correctly. If the meta lookup fails or returns no URL, the function falls back to BuildResourceURL, preserving existing behavior for public-cloud tenants. Closes #810, closes #925
…ack to brand-standard hosts Replace direct BuildResourceURL calls with FetchTenantResourceURL in all shortcuts that create resources and previously emitted hard-coded www.feishu.cn / www.larksuite.com links: - drive +upload - drive +import - docs +create (v1 fallback) - docs +create (v2 fallback) - markdown +create - wiki +node-create FetchTenantResourceURL queries the drive meta batch_query API with with_url=true to obtain the tenant actual vanity domain (e.g. example.feishu.cn). This fixes broken links for private-deployment and custom-domain tenants where the generic hosts do not redirect correctly. If the meta lookup fails or returns no URL, the function falls back to BuildResourceURL, preserving existing behavior for public-cloud tenants. Closes #810, closes #925
|
|
📝 WalkthroughWalkthroughThis PR adds tenant-aware resource URL resolution via a new ChangesTenant-aware URL Resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
domain/ccm
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/common/tenant_url_test.go`:
- Around line 204-205: The test helper is ignoring the error returned from
f.Config(), so update the setup to handle the error explicitly: call f.Config()
and check the returned error, e.g. if err != nil { t.Fatalf(...) } (or return
the error up) before passing cfg into TestNewRuntimeContextForAPI; locate the
call to f.Config() and the return into TestNewRuntimeContextForAPI in the test
helper and add proper error handling for the cfg, preserving use of
TestNewRuntimeContextForAPI(context.Background(), cmd, cfg, f, core.AsBot).
In `@shortcuts/common/tenant_url.go`:
- Line 59: Normalize the incoming kind before using it for fallback URL
building: trim whitespace and normalize case (e.g., strings.TrimSpace +
strings.ToLower) on the kind variable and use that normalized value when calling
BuildResourceURL and any meta lookup via kindToDocType so inputs like " DOCX "
produce a correct fallback URL; update the call site that currently does
BuildResourceURL(runtime.Config.Brand, kind, token) to pass the normalizedKind
instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fba107b3-a85c-41c7-a7d7-fe4ec98c4274
📒 Files selected for processing (8)
shortcuts/common/tenant_url.goshortcuts/common/tenant_url_test.goshortcuts/doc/docs_create.goshortcuts/doc/docs_create_v2.goshortcuts/drive/drive_import.goshortcuts/drive/drive_upload.goshortcuts/markdown/markdown_create.goshortcuts/wiki/wiki_node_create.go
Comment on lines
+204
to
+205
| cfg, _ := f.Config() | ||
| return TestNewRuntimeContextForAPI(context.Background(), cmd, cfg, f, core.AsBot) |
There was a problem hiding this comment.
Handle f.Config() errors explicitly in test helper.
Ignoring the error can hide setup failures and make test breakage harder to diagnose.
Suggested fix
func newTestRuntimeForTenantURL(t *testing.T, f *cmdutil.Factory) *RuntimeContext {
t.Helper()
cmd := &cobra.Command{Use: "test"}
cmd.SetContext(context.Background())
- cfg, _ := f.Config()
+ cfg, err := f.Config()
+ if err != nil {
+ t.Fatalf("f.Config() failed: %v", err)
+ }
return TestNewRuntimeContextForAPI(context.Background(), cmd, cfg, f, core.AsBot)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cfg, _ := f.Config() | |
| return TestNewRuntimeContextForAPI(context.Background(), cmd, cfg, f, core.AsBot) | |
| func newTestRuntimeForTenantURL(t *testing.T, f *cmdutil.Factory) *RuntimeContext { | |
| t.Helper() | |
| cmd := &cobra.Command{Use: "test"} | |
| cmd.SetContext(context.Background()) | |
| cfg, err := f.Config() | |
| if err != nil { | |
| t.Fatalf("f.Config() failed: %v", err) | |
| } | |
| return TestNewRuntimeContextForAPI(context.Background(), cmd, cfg, f, core.AsBot) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shortcuts/common/tenant_url_test.go` around lines 204 - 205, The test helper
is ignoring the error returned from f.Config(), so update the setup to handle
the error explicitly: call f.Config() and check the returned error, e.g. if err
!= nil { t.Fatalf(...) } (or return the error up) before passing cfg into
TestNewRuntimeContextForAPI; locate the call to f.Config() and the return into
TestNewRuntimeContextForAPI in the test helper and add proper error handling for
the cfg, preserving use of TestNewRuntimeContextForAPI(context.Background(),
cmd, cfg, f, core.AsBot).
| } | ||
| } | ||
|
|
||
| return BuildResourceURL(runtime.Config.Brand, kind, token) |
There was a problem hiding this comment.
Normalize kind before fallback URL building.
kindToDocType accepts trimmed/case-insensitive kinds, but fallback uses raw kind. If meta lookup fails, valid inputs like " DOCX " can still produce a broken/empty fallback URL.
Suggested fix
func FetchTenantResourceURL(runtime *RuntimeContext, kind, token string) string {
token = strings.TrimSpace(token)
+ normalizedKind := strings.ToLower(strings.TrimSpace(kind))
if token == "" {
return ""
}
- docType := kindToDocType(kind)
+ docType := kindToDocType(normalizedKind)
if docType != "" {
if metaData, err := runtime.CallAPI(
"POST",
"/open-apis/drive/v1/metas/batch_query",
@@
- return BuildResourceURL(runtime.Config.Brand, kind, token)
+ return BuildResourceURL(runtime.Config.Brand, normalizedKind, token)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shortcuts/common/tenant_url.go` at line 59, Normalize the incoming kind
before using it for fallback URL building: trim whitespace and normalize case
(e.g., strings.TrimSpace + strings.ToLower) on the kind variable and use that
normalized value when calling BuildResourceURL and any meta lookup via
kindToDocType so inputs like " DOCX " produce a correct fallback URL; update the
call site that currently does BuildResourceURL(runtime.Config.Brand, kind,
token) to pass the normalizedKind instead.
There was a problem hiding this comment.
Pull request overview
This PR fixes broken user-facing URLs for newly created Drive/Docs/Wiki resources in tenants that use private deployments or custom vanity domains by resolving URLs via the Drive meta batch_query API (with with_url=true) and falling back to the existing brand-standard URL builder when needed.
Changes:
- Added
FetchTenantResourceURLhelper that attempts to fetch a tenant-specific URL via/open-apis/drive/v1/metas/batch_query, with fallback toBuildResourceURL. - Added unit tests covering meta URL resolution, fallback behavior, empty token, unknown kind, and whitespace trimming.
- Updated several shortcuts to use
FetchTenantResourceURLinstead of hard-coded-hostBuildResourceURLwhen emitting URLs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| shortcuts/common/tenant_url.go | Adds tenant-aware URL resolution via Drive meta API with fallback to brand-standard URLs. |
| shortcuts/common/tenant_url_test.go | Adds unit tests for the new tenant URL resolution behavior. |
| shortcuts/wiki/wiki_node_create.go | Uses tenant-aware URL resolution for newly created wiki nodes. |
| shortcuts/markdown/markdown_create.go | Uses tenant-aware URL resolution for created markdown files. |
| shortcuts/drive/drive_upload.go | Uses tenant-aware URL resolution for uploaded files (when applicable). |
| shortcuts/drive/drive_import.go | Uses tenant-aware URL resolution for imported document results when URL is missing. |
| shortcuts/doc/docs_create.go | Uses tenant-aware URL resolution for v1 docs-create URL fallback. |
| shortcuts/doc/docs_create_v2.go | Uses tenant-aware URL resolution for v2 docs-create URL fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+66
to
+69
| switch strings.ToLower(strings.TrimSpace(kind)) { | ||
| case "docx", "doc": | ||
| return "docx" | ||
| case "sheet": |
Comment on lines
+20
to
+25
| // The kind parameter must be a value BuildResourceURL recognizes (docx, sheet, | ||
| // wiki, file, folder, slides, bitable, mindnote). doc_type for the meta API | ||
| // is derived from kind automatically. | ||
| // | ||
| // Returns "" only when both the meta lookup and the brand fallback fail | ||
| // (typically because token is empty). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Several CLI shortcuts that create resources (
drive +upload,drive +import,docs +create,markdown +create,wiki +node-create) useBuildResourceURLto generate user-facing URLs.BuildResourceURLhard-codeswww.feishu.cnandwww.larksuite.comas hosts. For tenants with private deployments or custom vanity domains, these URLs do not redirect correctly, leaving users with broken links.Issues: #810, #925
Solution
Introduce
FetchTenantResourceURLinshortcuts/common/tenant_url.go. This helper:batch_queryAPI (with_url=true) to obtain the tenant-specific URL for the newly created resource.BuildResourceURLwhen the API is unavailable, returns no URL, or the resource kind has no drive meta mapping.This mirrors the existing pattern already used by
slides +create(which callsbatch_querydirectly) and is consistent withsheets +create/drive +create-folder(which read theurlfield from the create API response).Changes
shortcuts/common/tenant_url.go—FetchTenantResourceURL+kindToDocTypeshortcuts/common/tenant_url_test.go— 7 unit tests covering meta URL resolution, fallback, empty token, unknown kind, and whitespace trimmingBuildResourceURLwithFetchTenantResourceURLin:shortcuts/drive/drive_upload.goshortcuts/drive/drive_import.goshortcuts/doc/docs_create.goshortcuts/doc/docs_create_v2.goshortcuts/markdown/markdown_create.goshortcuts/wiki/wiki_node_create.goTesting
go test ./shortcuts/common/ -run TestFetchTenantResourceURL— PASS (7/7)go test ./shortcuts/wiki/— PASSgo test ./shortcuts/markdown/— PASSgo test ./shortcuts/doc/— PASSgo test ./shortcuts/slides/— PASSgo test ./shortcuts/sheets/— PASSgo vet ./shortcuts/common/ ./shortcuts/drive/ ./shortcuts/wiki/ ./shortcuts/markdown/ ./shortcuts/doc/ ./shortcuts/slides/ ./shortcuts/sheets/— cleangofmt -lon all modified files — cleanBackward Compatibility
Public-cloud tenants continue to receive the same brand-standard URLs because
BuildResourceURLis still the fallback whenbatch_querydoes not return a tenant-specific URL. No breaking changes to CLI flags or output shape.Summary by CodeRabbit
Refactor
Tests