Skip to content

feat(calendar): add room find workflow#403

Merged
calendar-assistant merged 1 commit intomainfrom
feat/room_find
Apr 10, 2026
Merged

feat(calendar): add room find workflow#403
calendar-assistant merged 1 commit intomainfrom
feat/room_find

Conversation

@calendar-assistant
Copy link
Copy Markdown
Collaborator

@calendar-assistant calendar-assistant commented Apr 10, 2026

Summary

This PR adds a complete meeting-room workflow to calendar shortcuts: users can search available meeting rooms for one or more candidate time slots, get recommended room options grouped by time slot, and complete room booking through calendar event creation.
It also updates the calendar skill guidance so AI agents follow the right room-search and meeting-scheduling flow.

Changes

  • Support meeting room search with calendar +room-find, including filtering by city, building, floor, room name, capacity, attendees, and repeated candidate slots
  • Support meeting room recommendation by aggregating room candidates for multiple time slots, so users can compare recommended rooms under each candidate slot in one response
  • Support meeting room booking workflow by documenting and wiring the follow-up flow from room selection to calendar +create, where the selected room_id is attached as an attendee resource
  • Add multi-slot verification tests for room-find and update related calendar tests
  • Update lark-calendar skill docs and references for room search, room recommendation, and meeting scheduling/booking

Test Plan

  • make unit-test
  • Manual local verification confirms the lark xxx command works as expected
  • go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/main currently fails on existing shortcuts/whiteboard/whiteboard_update.go forbidigo violations unrelated to this branch

Related Issues

  • None

Summary by CodeRabbit

  • New Features

    • Added +room-find to query available meeting rooms for one or more explicit time blocks, with attendee, location, capacity and timezone options; supports dry-run and aggregates per-slot suggestions.
  • Updates

    • Refined suggestion wording to emphasize recommending available time blocks for non-explicit times.
    • Improved error propagation for calendar suggestion failures.
  • Documentation

    • New and updated docs detailing room-find usage, workflows, parameter constraints, and output format.
  • Tests

    • Added tests for concurrency limits and auth/registration coverage.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 10, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Added a new exported calendar shortcut +room-find that parses repeatable explicit time slots, validates inputs (slots, attendees, capacity, strings), concurrently queries the room-find API per slot with a worker limit, aggregates and sorts slot results, and includes tests and documentation updates.

Changes

Cohort / File(s) Summary
Room Find Implementation
shortcuts/calendar/calendar_room_find.go
New exported shortcut calendar.CalendarRoomFind: parses/validates repeatable --slot start~end, attendee/location/capacity constraints, dangerous-character rejection on some flags, dry-run support, concurrent per-slot POST /open-apis/calendar/v4/freebusy/room_find calls with worker limit, aggregates/sorts time_slots, and formats output.
Tests & Registration
shortcuts/calendar/calendar_room_find_test.go, shortcuts/calendar/calendar_test.go, shortcuts/calendar/shortcuts.go
Added concurrency-limit test for collector; expanded calendar tests covering multi-slot aggregation, slot validation, --room-name rejection, attendee ID parsing, dry-run request inspection, auth/error propagation; registered CalendarRoomFind in shortcuts list and updated shortcut-count test.
Suggestion Handling
shortcuts/calendar/calendar_suggestion.go
Adjusted CalendarSuggestion.Description text and changed error handling to return the raw err from the runtime API call.
Skill & Docs
skills/lark-calendar/SKILL.md, skills/lark-calendar/references/.../lark-calendar-room-find.md, .../lark-calendar-schedule-meeting.md, .../lark-calendar-create.md, .../lark-calendar-suggestion.md, .../lark-calendar-freebusy.md
Added room-find reference and schedule-meeting workflow; updated SKILL to include +room-find usage and ordering constraints (explicit-time: +room-find+freebusy; unclear-time: +suggestion+room-find); refined RRULE guidance and suggestion semantics; documented parameters, output formatting, and workflow rules.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant CLI as "CLI"
    participant Validator as "Input Validator"
    participant Parser as "Slot Parser"
    participant Pool as "Worker Pool"
    participant API as "Calendar API"
    participant Aggregator as "Result Aggregator"
    participant Formatter as "Output Formatter"

    User->>CLI: run `calendar +room-find` with slots & constraints
    CLI->>Validator: validate flags (slots, attendees, capacities, strings)
    Validator-->>CLI: ok / error

    alt validation failed
        CLI-->>User: error
    else
        CLI->>Parser: parse slots to RFC3339 start/end
        Parser-->>CLI: parsed slots

        CLI->>Pool: submit slots (worker limit)
        loop per slot (concurrent, limited)
            Pool->>API: POST /freebusy/room_find (slot + constraints)
            API-->>Pool: meeting_rooms for slot
        end

        Pool->>Aggregator: collect slot responses
        Aggregator->>Aggregator: sort time_slots by start
        Aggregator-->>Formatter: structured output
        Formatter-->>User: display time_slots → meeting_rooms or "No suggestions"
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • hugang-lark
  • zhouyue-bytedance

Poem

🐇 I hop through slots both near and far,
Seeking rooms beneath each time-block star,
Workers pair and race to fetch each spot,
I gather choices, tidy what I got,
Pick one, book it — a rabbit's clever plot.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(calendar): add room find workflow' clearly summarizes the main change—adding a meeting-room search workflow to calendar shortcuts.
Description check ✅ Passed The PR description includes all required template sections: Summary, Changes, Test Plan (with checkmarks showing test status), and Related Issues. It provides clear, specific details about the implementation and testing.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/room_find

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

@github-actions github-actions Bot added domain/calendar PR touches the calendar domain size/L Large or sensitive change across domains or core paths labels Apr 10, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 10, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@2091327d76f04c8fd33073c22f8baf70cdd38697

🧩 Skill update

npx skills add larksuite/cli#feat/room_find -y -g

hugang-lark
hugang-lark previously approved these changes Apr 10, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 10, 2026

Greptile Summary

This PR introduces calendar +room-find, a new shortcut that queries available meeting rooms for one or more explicit time slots concurrently, aggregates results per slot, and documents the full booking workflow through +create. Skill documentation and reference guides are updated to guide AI agents through the complete room-search-to-booking flow.

  • The room_id field is now present in the text-format output row map (lines 358–364 of calendar_room_find.go), which resolves the prior concern about agents being unable to complete booking from text output.
  • The unresolved prior concern about collectRoomFindResults discarding all successfully-fetched slots whenever any single slot returns an error remains in the code; only the first error is surfaced.

Confidence Score: 4/5

Safe to merge for most single-slot use cases; multi-slot partial-failure behavior (all results discarded when any one slot errors) remains unaddressed from prior review and is worth resolving before wider rollout.

The implementation is well-structured and goroutine-safe (DoAPI confirmed thread-safe via sync.OnceValues). The room_id field is correctly present in the text-output row map, resolving the prior concern. The one unresolved prior P1 — collectRoomFindResults returning nil and discarding all successful slot results when any single slot fetch fails — is still present in the code (lines 113-117) and can silently lose valid suggestions for the user. No new P1+ issues were found; the timezone display inconsistency is P2.

shortcuts/calendar/calendar_room_find.go — specifically collectRoomFindResults error-handling (lines 100-117) and unixStringToRFC3339 timezone formatting (lines 169-175)

Important Files Changed

Filename Overview
shortcuts/calendar/calendar_room_find.go New room-find shortcut with concurrent multi-slot fetching, semaphore-based concurrency, and structured output; text display times are formatted in server local timezone regardless of --timezone flag, and the goroutine fan-out discards partial results on any slot error (prior P1, unresolved)
shortcuts/calendar/calendar_room_find_test.go Adds concurrency-limit test for collectRoomFindResults; no test for the error-path (partial failure scenario)
shortcuts/calendar/calendar_test.go Adds auth-error preservation tests for room-find and suggestion; updates shortcut count assertion to 6
shortcuts/calendar/shortcuts.go Registers CalendarRoomFind in the shortcut list; straightforward one-line addition
skills/lark-calendar/SKILL.md Adds room-find and schedule-meeting workflow CRITICAL guards; removes outdated inline workflow prose
skills/lark-calendar/references/lark-calendar-room-find.md New reference doc for +room-find with field descriptions, slot rules, rrule/reserve_until_time guidance, and AI display constraints

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as lark-cli calendar
    participant Collector as collectRoomFindResults
    participant API as Lark Room-Find API

    User->>CLI: "+room-find --slot A~B --slot C~D [filters]"
    CLI->>Collector: "slots=[A~B, C~D], workers=10"
    par Concurrent slot fetches
        Collector->>API: "POST /freebusy/room_find (slot A~B)"
        API-->>Collector: available_rooms[]
    and
        Collector->>API: "POST /freebusy/room_find (slot C~D)"
        API-->>Collector: available_rooms[]
    end
    Collector->>Collector: sort TimeSlots by Start
    Collector-->>CLI: "roomFindOutput{TimeSlots}"
    CLI-->>User: "Text table (slot -> rooms) or JSON"
    User->>CLI: "calendar +create --attendee-ids room_id"
    CLI-->>User: Event created with room booked
Loading

Reviews (3): Last reviewed commit: "feat(calendar): add room find workflow" | Re-trigger Greptile

Comment thread shortcuts/calendar/calendar_room_find.go
Comment thread shortcuts/calendar/calendar_room_find.go
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

🧹 Nitpick comments (3)
shortcuts/calendar/calendar_suggestion.go (1)

295-295: Preserve auth passthrough, but normalize non-exit API errors.

return err correctly keeps auth classification, but it also bypasses consistent hinting for generic transport failures. Consider wrapping only non-*output.ExitError errors.

Proposed patch
diff --git a/shortcuts/calendar/calendar_suggestion.go b/shortcuts/calendar/calendar_suggestion.go
@@
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
@@
 		if err != nil {
-			return err
+			var exitErr *output.ExitError
+			if errors.As(err, &exitErr) {
+				return err
+			}
+			return output.ErrWithHint(output.ExitInternal, "request_fail", "api request fail", err.Error())
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/calendar/calendar_suggestion.go` at line 295, At the site in
shortcuts/calendar/calendar_suggestion.go that currently does "return err",
change the behavior to preserve existing auth-classification by returning errors
of type *output.ExitError unchanged, but wrap all other errors with a normalized
transport hint (e.g. using fmt.Errorf("transport error: %w", err)) so generic
failures get consistent hinting; implement this by type-asserting err to
*output.ExitError and returning it directly if that succeeds, otherwise return
the wrapped error.
skills/lark-calendar/references/lark-calendar-schedule-meeting.md (1)

10-11: Consider adding runtime guardrails for the documented blocking rule.

This document makes “must confirm before +create” a hard requirement, but current command validation (e.g., shortcuts/calendar/calendar_create.go:68-95) and room-find invocation path do not enforce that sequencing. A lightweight workflow state check in agent mode would make this rule enforceable instead of policy-only.

Also applies to: 44-47

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-calendar/references/lark-calendar-schedule-meeting.md` around
lines 10 - 11, Add a runtime workflow guard to enforce the doc rule that +create
can only run after explicit user confirmation: introduce a simple agent-mode
state flag (e.g., awaiting_user_confirmation) set when presenting ambiguous
time/room options in the code path that emits +suggestion (reference
ProcessSuggestionSelection/EmitSuggestions) and cleared only when the user
explicitly confirms; then modify the create validation/handler (reference
ValidateCreateCommand or HandleCreateCommand in the calendar_create handler) and
the room-find invocation path (reference InvokeRoomFind) to reject or no-op
+create unless that flag is set, and ensure when a user picks a +suggestion you
skip calling +freebusy and transition directly to clearing the flag and
permitting +create.
shortcuts/calendar/calendar_test.go (1)

1071-1110: Assert both requested slots, not just the time_slots field.

This still passes if aggregation drops one slot but leaves the envelope shape intact. Verifying the slot count or both slot ranges would lock down the multi-slot behavior this PR is adding.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/calendar/calendar_test.go` around lines 1071 - 1110, The test
TestRoomFind_MultiSlot_NewEventContext currently only checks for the presence of
the "time_slots" field; update the test to also assert that both requested slots
are present (either by checking the count of slots in stdout JSON or by
verifying both slot range strings are contained), e.g. after calling mountAndRun
and before finishing, parse or inspect stdout.String() and assert that both
"2026-03-27T14:00:00+08:00~2026-03-27T15:00:00+08:00" and
"2026-03-27T16:00:00+08:00~2026-03-27T17:00:00+08:00" (or that the time_slots
array length == 2) appear in the output produced by CalendarRoomFind so the
multi-slot behavior is enforced.
🤖 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/calendar/calendar_room_find.go`:
- Around line 355-363: The human-readable output loop over out.TimeSlots only
prints room_name, capacity and reserve_until_time which hides the non-unique
room identifier; update the for _, slot := range out.TimeSlots loop to include
room.RoomID in both the fmt.Fprintf line and the rows map (e.g., add "room_id":
room.RoomID) so the printed table and the rows slice expose the unique room_id
needed by the subsequent +create flow; adjust the fmt.Fprintf format string to
display the room_id alongside room_name/capacity/reserve_until_time and ensure
any downstream code that consumes rows expects the new "room_id" key.
- Around line 85-113: The loop currently continues dispatching all slot
goroutines even after firstErr is set; change control flow to stop fan-out and
cancel in-flight work by creating a context.WithCancel (e.g., ctx, cancel :=
context.WithCancel(parentCtx)), pass that context into fetch (or wrap fetch to
return early on ctx.Done), and call cancel immediately when you record firstErr
inside the goroutine; additionally, before starting each goroutine in the for _,
slot loop, check under mu (or atomically) whether firstErr is already set and
skip launching further goroutines if so, and ensure defer cancel() is called on
function exit so any in-flight fetch calls can terminate.

---

Nitpick comments:
In `@shortcuts/calendar/calendar_suggestion.go`:
- Line 295: At the site in shortcuts/calendar/calendar_suggestion.go that
currently does "return err", change the behavior to preserve existing
auth-classification by returning errors of type *output.ExitError unchanged, but
wrap all other errors with a normalized transport hint (e.g. using
fmt.Errorf("transport error: %w", err)) so generic failures get consistent
hinting; implement this by type-asserting err to *output.ExitError and returning
it directly if that succeeds, otherwise return the wrapped error.

In `@shortcuts/calendar/calendar_test.go`:
- Around line 1071-1110: The test TestRoomFind_MultiSlot_NewEventContext
currently only checks for the presence of the "time_slots" field; update the
test to also assert that both requested slots are present (either by checking
the count of slots in stdout JSON or by verifying both slot range strings are
contained), e.g. after calling mountAndRun and before finishing, parse or
inspect stdout.String() and assert that both
"2026-03-27T14:00:00+08:00~2026-03-27T15:00:00+08:00" and
"2026-03-27T16:00:00+08:00~2026-03-27T17:00:00+08:00" (or that the time_slots
array length == 2) appear in the output produced by CalendarRoomFind so the
multi-slot behavior is enforced.

In `@skills/lark-calendar/references/lark-calendar-schedule-meeting.md`:
- Around line 10-11: Add a runtime workflow guard to enforce the doc rule that
+create can only run after explicit user confirmation: introduce a simple
agent-mode state flag (e.g., awaiting_user_confirmation) set when presenting
ambiguous time/room options in the code path that emits +suggestion (reference
ProcessSuggestionSelection/EmitSuggestions) and cleared only when the user
explicitly confirms; then modify the create validation/handler (reference
ValidateCreateCommand or HandleCreateCommand in the calendar_create handler) and
the room-find invocation path (reference InvokeRoomFind) to reject or no-op
+create unless that flag is set, and ensure when a user picks a +suggestion you
skip calling +freebusy and transition directly to clearing the flag and
permitting +create.
🪄 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: f9c56f53-1c8c-43df-a1f2-96afd3574ab4

📥 Commits

Reviewing files that changed from the base of the PR and between 46468a9 and c806630.

📒 Files selected for processing (12)
  • internal/output/format.go
  • shortcuts/calendar/calendar_room_find.go
  • shortcuts/calendar/calendar_room_find_test.go
  • shortcuts/calendar/calendar_suggestion.go
  • shortcuts/calendar/calendar_test.go
  • shortcuts/calendar/shortcuts.go
  • skills/lark-calendar/SKILL.md
  • skills/lark-calendar/references/lark-calendar-create.md
  • skills/lark-calendar/references/lark-calendar-freebusy.md
  • skills/lark-calendar/references/lark-calendar-room-find.md
  • skills/lark-calendar/references/lark-calendar-schedule-meeting.md
  • skills/lark-calendar/references/lark-calendar-suggestion.md

Comment thread shortcuts/calendar/calendar_room_find.go
Comment thread shortcuts/calendar/calendar_room_find.go
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

♻️ Duplicate comments (2)
shortcuts/calendar/calendar_room_find.go (2)

355-363: ⚠️ Potential issue | 🟠 Major

Expose room_id in the human-readable rows.

The follow-up +create flow needs the selected room_id, and room_name is not guaranteed unique. Right now the pretty/table path only renders room_name, capacity, and reserve_until_time, so a human selection cannot be mapped back unambiguously.

🛠️ Minimal fix
 				for _, room := range slot.MeetingRooms {
 					rows = append(rows, map[string]interface{}{
+						"room_id":            room.RoomID,
 						"room_name":          room.RoomName,
 						"capacity":           room.Capacity,
 						"reserve_until_time": room.ReserveUntilTime,
 					})
 				}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/calendar/calendar_room_find.go` around lines 355 - 363, The
human-readable rows built in the loop over out.TimeSlots omit the room
identifier, so add the room_id to each row map so later +create flow can
unambiguously map selections; in the loop that appends to rows (where
room.RoomName, room.Capacity, room.ReserveUntilTime are used) add "room_id":
room.RoomID (or the actual room ID field name on the room struct) to the map.

76-113: ⚠️ Potential issue | 🟠 Major

Cancel remaining slot lookups after the first failure.

firstErr only changes the final return value. The loop still launches the remaining goroutines, and in-flight requests keep hitting the room-find API even after a fatal slot error. Multi-slot failures should stop fan-out promptly instead of waiting for all slots to finish.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/calendar/calendar_room_find.go` around lines 76 - 113,
collectRoomFindResults currently continues launching and running slot goroutines
after the first error; to fix this, create a cancellable context
(context.WithCancel) in collectRoomFindResults, pass that context into the fetch
call (or otherwise make fetch respect ctx), and call cancel() as soon as you set
firstErr so in-flight fetches can abort and new launches can be prevented by
checking ctx.Err() before or when acquiring sem; update the goroutine launch
path in collectRoomFindResults to skip starting work if ctx is cancelled and
ensure fetch uses the provided ctx to stop in-progress requests promptly.
🤖 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/calendar/calendar_room_find.go`:
- Around line 319-326: The capacity validator currently only rejects negative
values by calling runtime.Int(flagMinCapacity)/runtime.Int(flagMaxCapacity) and
returning errors like "--min-capacity must be >= 0", but the spec requires
positive integers; update the checks to enforce >=1 when the flag is explicitly
provided: use runtime.IsSet(flagMinCapacity) and runtime.IsSet(flagMaxCapacity)
(or equivalent presence check) and return errors like "--min-capacity must be >=
1" / "--max-capacity must be >= 1" when set and <1, and adjust the range check
that uses runtime.Int(flagMinCapacity), runtime.Int(flagMaxCapacity) so it
validates min <= max only after confirming both are set and >=1.

In `@skills/lark-calendar/references/lark-calendar-schedule-meeting.md`:
- Around line 103-120: The docs for the +suggestion → +room-find → +create flow
omit how suggestion candidates are converted into exact ISO 8601 slot strings
(required by +room-find's --slot and +create's --start/--end); update
lark-calendar-suggestion.md and this file to state that the suggestion output
must include and preserve raw candidate timestamps and timezone (e.g., include
an unambiguous ISO 8601 start/end per candidate) and show the exact
transformation/field names agents should use to pass those timestamps into
+room-find's --slot and into +create's --start/--end so agents do not need to
parse human-readable text like "10:00 - 10:30".

---

Duplicate comments:
In `@shortcuts/calendar/calendar_room_find.go`:
- Around line 355-363: The human-readable rows built in the loop over
out.TimeSlots omit the room identifier, so add the room_id to each row map so
later +create flow can unambiguously map selections; in the loop that appends to
rows (where room.RoomName, room.Capacity, room.ReserveUntilTime are used) add
"room_id": room.RoomID (or the actual room ID field name on the room struct) to
the map.
- Around line 76-113: collectRoomFindResults currently continues launching and
running slot goroutines after the first error; to fix this, create a cancellable
context (context.WithCancel) in collectRoomFindResults, pass that context into
the fetch call (or otherwise make fetch respect ctx), and call cancel() as soon
as you set firstErr so in-flight fetches can abort and new launches can be
prevented by checking ctx.Err() before or when acquiring sem; update the
goroutine launch path in collectRoomFindResults to skip starting work if ctx is
cancelled and ensure fetch uses the provided ctx to stop in-progress requests
promptly.
🪄 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: 1467594b-78d3-4481-a960-dbedc5c99d0d

📥 Commits

Reviewing files that changed from the base of the PR and between c806630 and 699a9b2.

📒 Files selected for processing (11)
  • shortcuts/calendar/calendar_room_find.go
  • shortcuts/calendar/calendar_room_find_test.go
  • shortcuts/calendar/calendar_suggestion.go
  • shortcuts/calendar/calendar_test.go
  • shortcuts/calendar/shortcuts.go
  • skills/lark-calendar/SKILL.md
  • skills/lark-calendar/references/lark-calendar-create.md
  • skills/lark-calendar/references/lark-calendar-freebusy.md
  • skills/lark-calendar/references/lark-calendar-room-find.md
  • skills/lark-calendar/references/lark-calendar-schedule-meeting.md
  • skills/lark-calendar/references/lark-calendar-suggestion.md
✅ Files skipped from review due to trivial changes (2)
  • shortcuts/calendar/shortcuts.go
  • skills/lark-calendar/references/lark-calendar-freebusy.md
🚧 Files skipped from review as they are similar to previous changes (5)
  • shortcuts/calendar/calendar_suggestion.go
  • skills/lark-calendar/references/lark-calendar-create.md
  • shortcuts/calendar/calendar_room_find_test.go
  • shortcuts/calendar/calendar_test.go
  • skills/lark-calendar/references/lark-calendar-suggestion.md

Comment thread shortcuts/calendar/calendar_room_find.go
Comment thread skills/lark-calendar/references/lark-calendar-schedule-meeting.md
hugang-lark
hugang-lark previously approved these changes Apr 10, 2026
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: 1

♻️ Duplicate comments (3)
shortcuts/calendar/calendar_room_find.go (2)

90-113: ⚠️ Potential issue | 🟠 Major

Stop dispatching room lookups after the first slot failure.

firstErr is recorded, but the loop still launches remaining slot fetches and waits for all of them. A single auth/network failure therefore continues hitting /freebusy/room_find and delays the error path instead of failing fast.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/calendar/calendar_room_find.go` around lines 90 - 113, The loop
currently continues launching goroutines even after a slot fetch sets firstErr;
change the loop to stop dispatching further fetches once firstErr is set by
checking firstErr under the same mutex before adding a worker: before
wg.Add(1)/sem <- struct{}{} check (under mu.Lock()/mu.Unlock() or an
err-specific mutex) if firstErr != nil and break the loop; ensure the existing
goroutine code still sets firstErr when fetch returns an error (the variables to
touch: slots, fetch, firstErr, mu, sem, wg, out.TimeSlots, roomFindSlot) so no
new goroutines are started after the first failure and wait for only the
already-dispatched workers.

319-326: ⚠️ Potential issue | 🟡 Minor

Reject explicit zero capacities to match the documented contract.

The docs describe both capacity filters as positive integers, but this validator still accepts --min-capacity 0 / --max-capacity 0. Because these fields are omitempty, an explicit 0 is then silently dropped from the request instead of being rejected.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/calendar/calendar_room_find.go` around lines 319 - 326, The
validator currently allows explicit zero capacities; change the checks on
flagMinCapacity and flagMaxCapacity to reject zero by using <= 0 and update the
validation messages to state "--min-capacity must be > 0" and "--max-capacity
must be > 0"; keep the existing comparison (minCapacity, maxCapacity :=
runtime.Int(flagMinCapacity), runtime.Int(flagMaxCapacity); minCapacity > 0 &&
maxCapacity > 0 && minCapacity > maxCapacity) or adjust it to use >= 1 if you
prefer, and return output.ErrValidation with the updated messages so explicit 0
is rejected rather than dropped.
skills/lark-calendar/references/lark-calendar-schedule-meeting.md (1)

117-120: ⚠️ Potential issue | 🟠 Major

Document the exact timestamp handoff from +suggestion to +room-find / +create.

This flow still says to batch suggestion candidates into +room-find, but it never states that agents must preserve the raw candidate start/end timestamps and timezone from +suggestion and pass them through unchanged as --slot "<start>~<end>", then reuse the same values for +create --start/--end. Without that, the workflow still invites reconstructing slots from human-readable text.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-calendar/references/lark-calendar-schedule-meeting.md` around
lines 117 - 120, The doc must explicitly require preserving and passing the raw
ISO timestamps and timezone produced by +suggestion unchanged into +room-find
and +create (use the exact start/end strings as --slot "<start>~<end>" for
room-find and the identical values for +create --start/--end), and forbid
reconstructing slots from localized/human-readable text or re-calling +freebusy
after the user selects a +suggestion slot; ensure the guidance applies even when
the user supplied no time (i.e., after default suggestion generation) and call
out the exact symbols +suggestion, +room-find, +create, +freebusy so
implementers know where to preserve and reuse the timestamps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@skills/lark-calendar/references/lark-calendar-room-find.md`:
- Around line 35-38: Update the --event-rrule parameter description to prohibit
COUNT entirely (not just when UNTIL is present) so it matches the behavior
described in lark-calendar-create.md: change the sentence that currently says
"COUNT and UNTIL not supported simultaneously" to explicitly state "COUNT is not
supported; use UNTIL or other RFC5545 fields (e.g., FREQ=DAILY;INTERVAL=1) but
do not include COUNT." Ensure the --event-rrule line (the parameter name
`--event-rrule <rrule>`) and its example reflect this restriction.

---

Duplicate comments:
In `@shortcuts/calendar/calendar_room_find.go`:
- Around line 90-113: The loop currently continues launching goroutines even
after a slot fetch sets firstErr; change the loop to stop dispatching further
fetches once firstErr is set by checking firstErr under the same mutex before
adding a worker: before wg.Add(1)/sem <- struct{}{} check (under
mu.Lock()/mu.Unlock() or an err-specific mutex) if firstErr != nil and break the
loop; ensure the existing goroutine code still sets firstErr when fetch returns
an error (the variables to touch: slots, fetch, firstErr, mu, sem, wg,
out.TimeSlots, roomFindSlot) so no new goroutines are started after the first
failure and wait for only the already-dispatched workers.
- Around line 319-326: The validator currently allows explicit zero capacities;
change the checks on flagMinCapacity and flagMaxCapacity to reject zero by using
<= 0 and update the validation messages to state "--min-capacity must be > 0"
and "--max-capacity must be > 0"; keep the existing comparison (minCapacity,
maxCapacity := runtime.Int(flagMinCapacity), runtime.Int(flagMaxCapacity);
minCapacity > 0 && maxCapacity > 0 && minCapacity > maxCapacity) or adjust it to
use >= 1 if you prefer, and return output.ErrValidation with the updated
messages so explicit 0 is rejected rather than dropped.

In `@skills/lark-calendar/references/lark-calendar-schedule-meeting.md`:
- Around line 117-120: The doc must explicitly require preserving and passing
the raw ISO timestamps and timezone produced by +suggestion unchanged into
+room-find and +create (use the exact start/end strings as --slot
"<start>~<end>" for room-find and the identical values for +create
--start/--end), and forbid reconstructing slots from localized/human-readable
text or re-calling +freebusy after the user selects a +suggestion slot; ensure
the guidance applies even when the user supplied no time (i.e., after default
suggestion generation) and call out the exact symbols +suggestion, +room-find,
+create, +freebusy so implementers know where to preserve and reuse the
timestamps.
🪄 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: 201ad100-377b-4b81-a726-fa50fcb68f00

📥 Commits

Reviewing files that changed from the base of the PR and between 699a9b2 and 2d1cb15.

📒 Files selected for processing (11)
  • shortcuts/calendar/calendar_room_find.go
  • shortcuts/calendar/calendar_room_find_test.go
  • shortcuts/calendar/calendar_suggestion.go
  • shortcuts/calendar/calendar_test.go
  • shortcuts/calendar/shortcuts.go
  • skills/lark-calendar/SKILL.md
  • skills/lark-calendar/references/lark-calendar-create.md
  • skills/lark-calendar/references/lark-calendar-freebusy.md
  • skills/lark-calendar/references/lark-calendar-room-find.md
  • skills/lark-calendar/references/lark-calendar-schedule-meeting.md
  • skills/lark-calendar/references/lark-calendar-suggestion.md
✅ Files skipped from review due to trivial changes (2)
  • shortcuts/calendar/shortcuts.go
  • skills/lark-calendar/references/lark-calendar-freebusy.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • shortcuts/calendar/calendar_suggestion.go
  • shortcuts/calendar/calendar_room_find_test.go
  • skills/lark-calendar/references/lark-calendar-suggestion.md

Comment thread skills/lark-calendar/references/lark-calendar-room-find.md Outdated
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 10, 2026

Tip:

Greploop — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

Fix room-find multi-slot verification.

Change-Id: I3ba4c8dbe30bbb1eb12c0996bb8bc5d54e6339ca
@calendar-assistant calendar-assistant merged commit b8f71d5 into main Apr 10, 2026
15 of 17 checks passed
@calendar-assistant calendar-assistant deleted the feat/room_find branch April 10, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/calendar PR touches the calendar domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants