fix(config/init): respect --brand flag in --new mode#711
Merged
Conversation
Add --queries CSV flag to lark-cli contact +search-user for parallel
multi-name fanout (up to 20 entries, partial-failure tolerant).
Output shape in fanout mode:
- data.users[] rows carry matched_query (string)
- data.queries[] sidecar lists each input with {query, error?, has_more}
- top-level data.has_more removed (per-query in queries[])
- error is omitempty; absent on success
Single --query mode is byte-for-byte unchanged (regression-guarded).
--queries is mutually exclusive with --query and --user-ids; bool
filters propagate to every sub-request.
Workers run with WaitGroup + buffered semaphore + index-slot writes;
each has defer recover() converting panics to internal error: ... in
the sidecar (no stack to stderr). Pre-canceled context returns
context canceled without making the request.
All-failed exit propagates first failure's HTTP/API code via ErrAPI;
falls back to ExitInternal for transport/parse/panic/ctx-canceled
(avoids emitting code 0, which means success in the Lark protocol).
HTTP non-200 ErrMsg now includes truncated response body for diagnosis.
Drive-by: signature field is now omitempty (mostly empty in practice).
Infrastructure:
- internal/httpmock gains BodyFilter/OnMatch/Reusable/CapturedBodies
hooks to support concurrent stub-driven tests
- internal/output adds 'users' to knownArrayFields so CSV picks the
primary array correctly
Change-Id: I3c14195fb8e094ae150002d90c36a0e4a0cc97d0
…ndFeishu in --new mode The --new flag was ignoring the --brand flag and always passing BrandFeishu to runCreateAppFlow. Now it correctly uses parseBrand(opts.Brand) to respect the user's --brand parameter (e.g., --brand lark for international). Change-Id: I1d4d78b3d586142b0210e6ceaeeb467b14e9c1a1
github-actions
Bot
added
domain/contact
📝 WalkthroughWalkthroughThis pull request introduces a multi-query fanout capability for contact search ( Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Parser as Query Parser
participant Executor as Fanout Executor
participant API as Lark API
participant Assembler as Result Assembler
participant Formatter as Output Formatter
User->>Parser: --queries "alice,bob,charlie"
Parser->>Parser: Trim, deduplicate, validate
Parser->>Executor: [query1, query2, query3]
par Concurrent Execution
Executor->>API: POST search(query1)
Executor->>API: POST search(query2)
Executor->>API: POST search(query3)
end
API-->>Executor: {users: [...], has_more: true}
API-->>Executor: {users: [...], has_more: false}
API-->>Executor: Error or {users: [...]}
Executor->>Assembler: Per-query results + errors
Assembler->>Assembler: Add matched_query to users
Assembler->>Assembler: Collect query metadata
Assembler->>Assembler: Flatten user results
Assembler->>Formatter: data.queries[], data.users[]
Formatter->>User: Table/CSV/NDJSON output + stderr summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #711 +/- ##
=======================================
Coverage 64.14% 64.14%
=======================================
Files 504 504
Lines 44285 44285
=======================================
Hits 28406 28406
Misses 13411 13411
Partials 2468 2468 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@de3d0f70b97f78295097550e099aea008e7c10dc🧩 Skill updatenpx skills add larksuite/cli#fix/brand-flag-in-new-mode -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cmd/config/init.go (1)
270-288:⚠️ Potential issue | 🟠 MajorAdd a regression test for
--new --brandpass-through.This changes Mode 3 runtime behavior, but the diff doesn’t include a test that asserts
runCreateAppFlowgets the parsed--brandvalue (e.g.,lark) instead of defaulting to Feishu. Please add a focused unit test for this path.I can draft the exact test case with a stubbed flow assertion if you want.
As per coding guidelines "Every behavior change must have an accompanying test".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/config/init.go` around lines 270 - 288, Add a focused unit/regression test that exercises the Mode 3 path when opts.New is true and a --brand value is provided: stub or mock runCreateAppFlow and assert it is invoked with the parsed brand from parseBrand(opts.Brand) (e.g., "lark") rather than a fallback value; the test should invoke the same call site logic as in init.go (the branch that calls runCreateAppFlow(opts.Ctx, f, parseBrand(opts.Brand), msg)) and verify the brand argument passed into the stubbed runCreateAppFlow, and also assert the rest of the flow returns/handles the stubbed result appropriately (so saveInitConfig and output.PrintJson behavior can be either no-ops or verified via mocks).internal/httpmock/registry.go (1)
101-125:⚠️ Potential issue | 🟠 MajorDon't run
BodyFilterunderr.mu, and don't hand it the live body slice.
BodyFilteris arbitrary caller code, but it currently executes while the registry mutex is held and receives the sharedcapturedBodyslice. That means a filter that re-entersRegister,Verify, or anotherRoundTripcan deadlock the registry, and a mutating filter can rewrite the body seen by later stubs and the stored captures. Evaluate the predicate on a defensive copy outside the critical section, then re-lock only to claim/update the matched stub.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/httpmock/registry.go` around lines 101 - 125, The BodyFilter is invoked while holding r.mu and given the shared capturedBody slice; change match in Registry so BodyFilter runs outside the critical section on a defensive copy: while locked, iterate r.stubs and skip by Method/URL and already matched (s.matched) to shortlist candidate stub pointers (or simply note the index), then make a copy of the body (bodyCopy := append([]byte(nil), capturedBody...)), unlock r.mu, run s.BodyFilter(bodyCopy) for the candidate, then re-lock r.mu and re-check s.matched and URL/Method conditions before claiming the stub and updating s.matched, s.CapturedHeaders, s.CapturedBody and s.CapturedBodies; this prevents deadlocks from Register/Verify/RoundTrip re-entry and avoids letting filters mutate the shared slice.
🧹 Nitpick comments (1)
shortcuts/contact/contact_search_user_fanout.go (1)
86-100: Minor: variable shadowing ofbody.Line 88 declares a new
body(string) that shadows the outerbody(*searchUserAPIRequest) from line 73. While not a bug—the inner variable is only used for error message construction—renaming torespBodyorerrBodywould improve clarity.♻️ Suggested rename
if apiResp.StatusCode != http.StatusOK { - body := strings.TrimSpace(string(apiResp.RawBody)) + respBody := strings.TrimSpace(string(apiResp.RawBody)) const maxBody = 200 - if len(body) > maxBody { - body = body[:maxBody] + "..." + if len(respBody) > maxBody { + respBody = respBody[:maxBody] + "..." } msg := fmt.Sprintf("HTTP %d %s", apiResp.StatusCode, http.StatusText(apiResp.StatusCode)) - if body != "" { - msg = fmt.Sprintf("%s: %s", msg, body) + if respBody != "" { + msg = fmt.Sprintf("%s: %s", msg, respBody) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/contact/contact_search_user_fanout.go` around lines 86 - 100, The inner variable named "body" that’s created when handling non-200 responses shadows the outer "body" (*searchUserAPIRequest) and should be renamed for clarity; update the error-handling block that reads apiResp.RawBody to use a distinct name such as "respBody" or "errBody", adjust the subsequent length check and msg construction to reference that new name, and return the same fanoutResult (Index, Query, ErrMsg, ErrCode) unchanged—this removes the shadowing while preserving the existing logic around apiResp and fanoutResult.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@cmd/config/init.go`:
- Around line 270-288: Add a focused unit/regression test that exercises the
Mode 3 path when opts.New is true and a --brand value is provided: stub or mock
runCreateAppFlow and assert it is invoked with the parsed brand from
parseBrand(opts.Brand) (e.g., "lark") rather than a fallback value; the test
should invoke the same call site logic as in init.go (the branch that calls
runCreateAppFlow(opts.Ctx, f, parseBrand(opts.Brand), msg)) and verify the brand
argument passed into the stubbed runCreateAppFlow, and also assert the rest of
the flow returns/handles the stubbed result appropriately (so saveInitConfig and
output.PrintJson behavior can be either no-ops or verified via mocks).
In `@internal/httpmock/registry.go`:
- Around line 101-125: The BodyFilter is invoked while holding r.mu and given
the shared capturedBody slice; change match in Registry so BodyFilter runs
outside the critical section on a defensive copy: while locked, iterate r.stubs
and skip by Method/URL and already matched (s.matched) to shortlist candidate
stub pointers (or simply note the index), then make a copy of the body (bodyCopy
:= append([]byte(nil), capturedBody...)), unlock r.mu, run
s.BodyFilter(bodyCopy) for the candidate, then re-lock r.mu and re-check
s.matched and URL/Method conditions before claiming the stub and updating
s.matched, s.CapturedHeaders, s.CapturedBody and s.CapturedBodies; this prevents
deadlocks from Register/Verify/RoundTrip re-entry and avoids letting filters
mutate the shared slice.
---
Nitpick comments:
In `@shortcuts/contact/contact_search_user_fanout.go`:
- Around line 86-100: The inner variable named "body" that’s created when
handling non-200 responses shadows the outer "body" (*searchUserAPIRequest) and
should be renamed for clarity; update the error-handling block that reads
apiResp.RawBody to use a distinct name such as "respBody" or "errBody", adjust
the subsequent length check and msg construction to reference that new name, and
return the same fanoutResult (Index, Query, ErrMsg, ErrCode) unchanged—this
removes the shadowing while preserving the existing logic around apiResp and
fanoutResult.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b27d469-0642-4ef1-a797-8a301618cb5f
📒 Files selected for processing (7)
cmd/config/init.gointernal/httpmock/registry.gointernal/output/format.goshortcuts/contact/contact_search_user.goshortcuts/contact/contact_search_user_fanout.goshortcuts/contact/contact_search_user_test.goskills/lark-contact/references/lark-contact-search-user.md
…-mode Change-Id: I7e288cb5dca52d9c29a1002c35b2bc2ab43d3cf4
sang-neo03
approved these changes
Apr 29, 2026
HomyeeKing
pushed a commit
to HomyeeKing/cli
that referenced
this pull request
May 6, 2026
* feat(contact +search-user): add --queries multi-name fanout
Add --queries CSV flag to lark-cli contact +search-user for parallel
multi-name fanout (up to 20 entries, partial-failure tolerant).
Output shape in fanout mode:
- data.users[] rows carry matched_query (string)
- data.queries[] sidecar lists each input with {query, error?, has_more}
- top-level data.has_more removed (per-query in queries[])
- error is omitempty; absent on success
Single --query mode is byte-for-byte unchanged (regression-guarded).
--queries is mutually exclusive with --query and --user-ids; bool
filters propagate to every sub-request.
Workers run with WaitGroup + buffered semaphore + index-slot writes;
each has defer recover() converting panics to internal error: ... in
the sidecar (no stack to stderr). Pre-canceled context returns
context canceled without making the request.
All-failed exit propagates first failure's HTTP/API code via ErrAPI;
falls back to ExitInternal for transport/parse/panic/ctx-canceled
(avoids emitting code 0, which means success in the Lark protocol).
HTTP non-200 ErrMsg now includes truncated response body for diagnosis.
Drive-by: signature field is now omitempty (mostly empty in practice).
Infrastructure:
- internal/httpmock gains BodyFilter/OnMatch/Reusable/CapturedBodies
hooks to support concurrent stub-driven tests
- internal/output adds 'users' to knownArrayFields so CSV picks the
primary array correctly
Change-Id: I3c14195fb8e094ae150002d90c36a0e4a0cc97d0
* fix(config/init): use parseBrand(opts.Brand) instead of hardcoded BrandFeishu in --new mode
The --new flag was ignoring the --brand flag and always passing BrandFeishu
to runCreateAppFlow. Now it correctly uses parseBrand(opts.Brand) to
respect the user's --brand parameter (e.g., --brand lark for international).
Change-Id: I1d4d78b3d586142b0210e6ceaeeb467b14e9c1a1
8 tasks
tuxedomm
pushed a commit
to zhumiaoxin/cli
that referenced
this pull request
Jun 6, 2026
* feat(contact +search-user): add --queries multi-name fanout
Add --queries CSV flag to lark-cli contact +search-user for parallel
multi-name fanout (up to 20 entries, partial-failure tolerant).
Output shape in fanout mode:
- data.users[] rows carry matched_query (string)
- data.queries[] sidecar lists each input with {query, error?, has_more}
- top-level data.has_more removed (per-query in queries[])
- error is omitempty; absent on success
Single --query mode is byte-for-byte unchanged (regression-guarded).
--queries is mutually exclusive with --query and --user-ids; bool
filters propagate to every sub-request.
Workers run with WaitGroup + buffered semaphore + index-slot writes;
each has defer recover() converting panics to internal error: ... in
the sidecar (no stack to stderr). Pre-canceled context returns
context canceled without making the request.
All-failed exit propagates first failure's HTTP/API code via ErrAPI;
falls back to ExitInternal for transport/parse/panic/ctx-canceled
(avoids emitting code 0, which means success in the Lark protocol).
HTTP non-200 ErrMsg now includes truncated response body for diagnosis.
Drive-by: signature field is now omitempty (mostly empty in practice).
Infrastructure:
- internal/httpmock gains BodyFilter/OnMatch/Reusable/CapturedBodies
hooks to support concurrent stub-driven tests
- internal/output adds 'users' to knownArrayFields so CSV picks the
primary array correctly
Change-Id: I3c14195fb8e094ae150002d90c36a0e4a0cc97d0
* fix(config/init): use parseBrand(opts.Brand) instead of hardcoded BrandFeishu in --new mode
The --new flag was ignoring the --brand flag and always passing BrandFeishu
to runCreateAppFlow. Now it correctly uses parseBrand(opts.Brand) to
respect the user's --brand parameter (e.g., --brand lark for international).
Change-Id: I1d4d78b3d586142b0210e6ceaeeb467b14e9c1a1
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
2 participants
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.
Summary
The
--newflag inlark-cli config initwas ignoring the--brandparameter and always passingcore.BrandFeishutorunCreateAppFlow.Fix
Changed
init.go:272from hardcodedcore.BrandFeishutoparseBrand(opts.Brand), which correctly handles the user's--brand larkflag for international (Lark) deployments.Testing
parseBranddefaults toBrandFeishuwhen no brand is specified, so backward compatibility is preservedSummary by CodeRabbit
Release Notes
New Features
--queriesflag tocontact +search-usercommand for parallel multi-query fanout searches (up to 20 queries, 50 characters each).Documentation
--queriesusage, constraints, and output structure.