feat(contact +search-user): add --queries multi-name fanout#707
Merged
Conversation
liangshuo-1
added
enhancement
github-actions
Bot
added
size/L
📝 WalkthroughWalkthroughAdds body-aware matching, per-match hooks, and reusable-hit semantics to the HTTP mock Stub; implements concurrent multi-query fanout for the contact +search-user shortcut with parsing/deduplication, per-query error aggregation, and enriched fanout output; updates output field prioritization and tests/docs for fanout behavior. Changes
Sequence DiagramsequenceDiagram
participant User as "User"
participant CLI as "Runtime (+search-user)"
participant Parser as "Query Parser"
participant Executor as "Concurrent Executor"
participant API as "User API"
participant Aggregator as "Response Aggregator"
participant Output as "Output Formatter"
User->>CLI: invoke `+search-user --queries "alice,bob"`
CLI->>Parser: parseAndDedupQueries()
Parser-->>CLI: ["alice","bob"]
CLI->>Executor: execute queries (bounded concurrency)
par
Executor->>API: run query "alice"
API-->>Executor: users[] or error
and
Executor->>API: run query "bob"
API-->>Executor: users[] or error
end
Executor->>Aggregator: per-query results
Aggregator-->>CLI: combined response (flattened users + queries[])
CLI->>Output: format results (pretty/table/csv/ndjson)
Output-->>User: rendered output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 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 |
liangshuo-1
force-pushed
the
feat/search-user-multi-queries
branch
from
April 29, 2026 04:28
e387a74 to
06b970f
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@d48af8d27c7695f51e1ba471749786fd081a15fd🧩 Skill updatenpx skills add larksuite/cli#feat/search-user-multi-queries -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #707 +/- ##
==========================================
+ Coverage 63.80% 63.87% +0.07%
==========================================
Files 500 501 +1
Lines 43531 43725 +194
==========================================
+ Hits 27773 27930 +157
- Misses 13317 13346 +29
- Partials 2441 2449 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/httpmock/registry.go (1)
78-92:⚠️ Potential issue | 🔴 CriticalGuard mutex release when
BodyFilterpanics.
BodyFilteris user-provided and runs whiler.muis held; a panic here skips Line 102 and can leave the registry permanently locked.🔧 Proposed fix
- r.mu.Lock() - var matched *Stub - for _, s := range r.stubs { + var matched *Stub + func() { + r.mu.Lock() + defer r.mu.Unlock() + for _, s := range r.stubs { if s.matched { continue } if s.Method != "" && s.Method != req.Method { continue } if s.URL != "" && !strings.Contains(urlStr, s.URL) { continue } if s.BodyFilter != nil && !s.BodyFilter(capturedBody) { continue } if !s.Reusable { s.matched = true } s.CapturedHeaders = req.Header.Clone() s.CapturedBody = capturedBody s.CapturedBodies = append(s.CapturedBodies, capturedBody) matched = s break - } - r.mu.Unlock() + } + }()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/httpmock/registry.go` around lines 78 - 92, The loop in registry.go holds r.mu while calling the user-provided BodyFilter on a Stub, so if BodyFilter panics the mutex r.mu remains locked; modify the matching loop (where r.mu.Lock() is called and you iterate r.stubs) to ensure r.mu is always unlocked by using defer r.mu.Unlock() in the function scope or by wrapping the BodyFilter invocation with a local recover: capture the body, call BodyFilter inside a small closure that defers a recover and returns (false, err) on panic, and use that return value to continue; keep references to Stub, r.mu, BodyFilter, and the matched check so the lock is never left held if BodyFilter panics.
🧹 Nitpick comments (1)
internal/httpmock/registry.go (1)
45-47: AlignCapturedBodiesbehavior with its documented semantics.The comment says
CapturedBodiesis for reusable stubs, but Line 98 appends for all stubs. Either gate the append bys.Reusableor update the comment to reflect current behavior.Also applies to: 98-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/httpmock/registry.go` around lines 45 - 47, The comment for CapturedBodies says it "records every captured request body when Reusable is set" but the implementation always appends to CapturedBodies; update registry.go so the append to s.CapturedBodies only happens when s.Reusable is true (leave s.CapturedBody behavior intact for back-compat), or alternatively update the comment to reflect that CapturedBodies is appended for all stubs; locate the append call that pushes the captured bytes (the code referencing s.CapturedBodies and s.CapturedBody and the s.Reusable flag) and either wrap that append in an if s.Reusable { ... } or change the field comment to match current behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/httpmock/registry.go`:
- Around line 73-76: The RoundTrip implementation reads and replaces req.Body
but doesn't close the original body or handle read errors; update the logic in
the method implementing http.RoundTripper.RoundTrip (the block that reads into
capturedBody) to check and handle the error returned by io.ReadAll, call
req.Body.Close() on the original body after reading (or defer Close immediately
after verifying req.Body != nil), and only then set req.Body =
io.NopCloser(bytes.NewReader(capturedBody)); ensure any read error is propagated
or logged instead of being ignored to avoid resource leaks.
In `@skills/lark-contact/references/lark-contact-search-user.md`:
- Line 73: Remove the trailing space inside the inline code span that currently
reads the CSV example with an extra space (the inline code token containing `,,,
`) to satisfy markdownlint MD038; locate the inline code in the markdown
(`lark-contact-search-user.md`) where the CSV example is shown and replace it so
the code span contains exactly three commas with no surrounding internal spaces.
---
Outside diff comments:
In `@internal/httpmock/registry.go`:
- Around line 78-92: The loop in registry.go holds r.mu while calling the
user-provided BodyFilter on a Stub, so if BodyFilter panics the mutex r.mu
remains locked; modify the matching loop (where r.mu.Lock() is called and you
iterate r.stubs) to ensure r.mu is always unlocked by using defer r.mu.Unlock()
in the function scope or by wrapping the BodyFilter invocation with a local
recover: capture the body, call BodyFilter inside a small closure that defers a
recover and returns (false, err) on panic, and use that return value to
continue; keep references to Stub, r.mu, BodyFilter, and the matched check so
the lock is never left held if BodyFilter panics.
---
Nitpick comments:
In `@internal/httpmock/registry.go`:
- Around line 45-47: The comment for CapturedBodies says it "records every
captured request body when Reusable is set" but the implementation always
appends to CapturedBodies; update registry.go so the append to s.CapturedBodies
only happens when s.Reusable is true (leave s.CapturedBody behavior intact for
back-compat), or alternatively update the comment to reflect that CapturedBodies
is appended for all stubs; locate the append call that pushes the captured bytes
(the code referencing s.CapturedBodies and s.CapturedBody and the s.Reusable
flag) and either wrap that append in an if s.Reusable { ... } or change the
field comment to match current behavior.
🪄 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: 8e5bee4f-927f-46f9-beed-1b752ff1f149
📒 Files selected for processing (6)
internal/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
liangshuo-1
force-pushed
the
feat/search-user-multi-queries
branch
from
April 29, 2026 04:55
06b970f to
cd534ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/contact/contact_search_user_test.go`:
- Around line 1258-1271: Update the TestFanout_AllFailed_ExitNonZero test to
assert the specific HTTP 500 failure rather than only err != nil: after calling
mountAndRun(t, ContactSearchUser, ...), inspect the returned err (from
mountAndRun/ContactSearchUser path) and assert it encodes the 500 status (e.g.,
unwrap the error to the command/http error type or check err.Error() contains
"500" or "status 500") so the test fails if a different/generic error is
returned; keep references to TestFanout_AllFailed_ExitNonZero, mountAndRun, and
ContactSearchUser so you update the exact failure assertion in that test.
In `@shortcuts/contact/contact_search_user.go`:
- Line 126: The JSON struct tag change for the Signature field may regress
behavior when meta.description is empty; add a regression test that exercises
the JSON serialization path for the ContactSearchUser response (or the existing
JSON-mode test suite) where meta.description == "" and assert that the
"signature" key is omitted (not present) in the serialized JSON rather than
present with an empty string; locate the struct field Signature in
contact_search_user.go and create a test (e.g.,
TestContactSearchUser_JSON_EmptySignature) that marshals the object (or calls
the JSON-mode serializer), inspects the resulting JSON (unmarshal to
map[string]interface{} or check bytes) and fails if "signature" appears with ""
value.
🪄 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: 0090ffa2-ff8c-4064-84c8-22c56fc3909f
📒 Files selected for processing (6)
internal/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
✅ Files skipped from review due to trivial changes (2)
- internal/output/format.go
- skills/lark-contact/references/lark-contact-search-user.md
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
liangshuo-1
force-pushed
the
feat/search-user-multi-queries
branch
from
April 29, 2026 07:00
cd534ef to
d48af8d
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/contact/contact_search_user.go (1)
160-161: Clarify thehas_moretip for fanout mode.The current tip wording references
--query/top-levelhas_more; with--queries,has_moreis per-query. Consider a short fanout-specific hint to avoid ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/contact/contact_search_user.go` around lines 160 - 161, The help tip is ambiguous for fanout mode: update the message strings that mention "--queries" and "has_more" in contact_search_user.go so they explicitly state that when using --queries (multi-name fanout) has_more is returned per individual query (not a top-level aggregate); add a brief fanout-specific hint recommending tightening or adding filters per query if any query's has_more=true and remind there is no auto-pagination.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/contact/contact_search_user.go`:
- Around line 160-161: The help tip is ambiguous for fanout mode: update the
message strings that mention "--queries" and "has_more" in
contact_search_user.go so they explicitly state that when using --queries
(multi-name fanout) has_more is returned per individual query (not a top-level
aggregate); add a brief fanout-specific hint recommending tightening or adding
filters per query if any query's has_more=true and remind there is no
auto-pagination.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79478bc0-c9bf-40af-bfd3-d7914e831ab6
📒 Files selected for processing (6)
internal/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
🚧 Files skipped from review as they are similar to previous changes (3)
- shortcuts/contact/contact_search_user_fanout.go
- skills/lark-contact/references/lark-contact-search-user.md
- shortcuts/contact/contact_search_user_test.go
sang-neo03
approved these changes
Apr 29, 2026
HomyeeKing
pushed a commit
to HomyeeKing/cli
that referenced
this pull request
May 6, 2026
…e#707) 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
5 tasks
tuxedomm
pushed a commit
to zhumiaoxin/cli
that referenced
this pull request
Jun 6, 2026
…e#707) 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
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
Add
--queries <csv>tolark-cli contact +search-userfor parallelmulti-name fanout. Eliminates the shell-loop pattern when an agent needs
to look up multiple users by keyword in one shot.
Changes
New flag
--queries <csv>— comma-separated keywords searched in parallel(up to 20 entries, partial-failure tolerant)
--queryand--user-ids--has-chatted,--exclude-external-users, etc.)propagate to every sub-request
Output shape (fanout mode only)
data.users[]rows carrymatched_query(string)data.queries[]sidecar lists each input with{query, error?, has_more}data.has_moreremoved (per-query inqueries[])errorisomitempty— absent on success--querymode is byte-for-byte unchanged (regression-guarded)Reliability
sync.WaitGroup+ buffered semaphore + index-slotwrites; each has
defer recover()converting panics tointernal error: ...in the sidecar (no stack to stderr)context canceledwithout making therequest
ErrAPI;falls back to
ExitInternalfor transport/parse/panic/ctx-canceled(avoids emitting code
0which means success in the Lark protocol)ErrMsgnow includes a truncated response body fordiagnosis
Drive-by
signaturefield is nowomitempty(mostly empty in practice;reduces JSON noise)
Infrastructure
internal/httpmockgainsBodyFilter/OnMatch/Reusable/CapturedBodieshooks for concurrent stub-driven testsinternal/outputaddsuserstoknownArrayFieldsso CSV picks theprimary array correctly when both
usersandqueriesare presentDocs
skills/lark-contact/references/lark-contact-search-user.mdupdatedwith fanout section + extended field contract; tightened existing
copy along the way
Test Plan
no-regression,
runOneQueryerror mapping (transport / HTTP non-200/ API code != 0 / parse / panic / ctx-canceled), assembler ordering,
partial and total failure, panic recovery, concurrency cap,
matched_queryfidelity, format-specific output, dry-rungo test ./shortcuts/contact/ ./internal/httpmock/ ./internal/output/ -race -count=1cleango build ./...andgo vet ./...clean--queries '梁,test_no_such_user'verifies output shape,
erroromitempty, per-queryhas_more,single-query path unchanged
json/csv/ndjson/pretty--query,mutex with
--user-ids, all-separators, over-20, over-50Related Issues
N/A
Summary by CodeRabbit
New Features
Bug Fixes
Formatting
Documentation
Tests