fix(cli): planner create/schedule send correct class and calendar refs - #22
Merged
Merged
Conversation
Two bugs that prevented CLI-created todos from appearing in the right places:
1. action create: resolveEmployeeId() returns either an Employee or Person
ref depending on the workspace model, but createAction hardcoded
attachedToClass as 'contact:class:Person'. Mismatch rejected by
addCollection or routed the todo to the wrong collection. Now returns
{ ref, class } and mirrors the resolved class into attachedToClass,
probing the actual class when the lookup helper returns just a ref.
2. action schedule: set WorkSlot.calendar to todo.user (an Employee ref),
but Event.calendar is a Ref<Calendar>. The Schedule Calendar UI
filters by the user's PersonalCalendar ref, so slots were invisible.
Now resolves the PersonalCalendar via findPrimaryCalendar logic
(Calendar docs for user + PrimaryCalendar preference).
Contributor
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe todo resource now resolves owner references with their classes, uses those classes for action operations, and assigns scheduled work slots to a configured, writable, external, or synthetic account calendar. ChangesTodo resource updates
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@packages/cli/src/resources/todo.ts`:
- Around line 169-171: Update the default-owner branch in the surrounding
account-resolution function to query CLASS.Person using { personUuid:
account.uuid }, return the matched document’s _id with CLASS.Person, and throw
an explicit error when no Person document was provisioned; do not return
account.uuid directly.
- Around line 57-85: Update resolvePrimaryCalendar to match the UI scheduler’s
primary-calendar selection order when the PRIMARY_CALENDAR_PREF lookup does not
yield a matching calendar: select the eligible ExternalCalendar first, then fall
back to the accountUuid-based default calendar before using the first writable
calendar. Reuse the shared selection helper if available, and preserve the
existing preference match as the highest-priority choice.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aed4cfa4-2de9-4a0d-9175-2d859fe4f6c0
📒 Files selected for processing (1)
packages/cli/src/resources/todo.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Kilo Code Review
🔇 Additional comments (1)
packages/cli/src/resources/todo.ts (1)
197-197: LGTM!Also applies to: 433-433
…ry-calendar order
1. resolveEmployeeId default-owner branch previously returned account.uuid
as if it were a workspace-local Person _id. Now queries contact:class:Person
with { personUuid: account.uuid }, mirroring resolveEmailToLocalId, and
throws an explicit error if no Person was provisioned for the current
account.
2. resolvePrimaryCalendar previously fell back to the first writable Calendar
when neither the preference nor a Person-local match was found. The UI
scheduler's getPrimaryCalendar (plugins/calendar/src/utils.ts:432) instead
returns the first eligible ExternalCalendar (default: true, hidden: false)
and, failing that, the synthetic ${accountUuid}_calendar. Match that
order so CLI-scheduled WorkSlots land in the same calendar the UI would
use. Shared helper is inlined because @hcengineering/calendar is not a
CLI dependency.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 3 Issues Remaining | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 Previous Review Summaries (2 snapshots, latest commit 5ab1bb4)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 5ab1bb4)Status: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous review (commit 0e2044c)Status: 6 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (1 file)
Reviewed by minimax-m3 · Input: 55.8K · Output: 4K · Cached: 276.5K |
…obe Employee in default owner
Address 3 of 6 review findings; skip 3 with reasons:
Fixes:
- resolvePrimaryCalendar.findAll: catch now only swallows 'domain not
found' errors so network/auth failures surface instead of silently
falling back to the synthetic ${accountUuid}_calendar.
- resolveEmployeeId probe loop (email branch): same narrow-to-domain-
not-found fix so a transient findOne failure doesn't masquerade as
'class not in this workspace'.
- resolveEmployeeId workspace-local fallback: same narrow-to-domain-
not-found fix.
- resolveEmployeeId default-owner branch: previously only queried
contact:class:Person; if the workspace models the current user only
as Employee, it threw NotFound. Now probes both Person and Employee,
mirroring the email branch.
Skipped:
- findOne(PrimaryCalendar, {}) empty filter: matches platform UI's
findPrimaryCalendar exactly; Preference is treated as a workspace
singleton there.
- ExternalCalendar strict-equality check: OSS platform defines no
ExternalCalendar subclasses (only ExternalCalendar extends Calendar);
the UI uses the same strict equality. Revisit if subclasses land.
- resolveEmployeeId workspace-local limit: 500: matches the
resolveEmailToLocalId pattern; pagination refactor out of scope.
…mployee-first probe
Two leftover SUGGESTION findings from the second review pass:
1. isDomainNotFound: switch err.message.includes('domain not found')
to /domain not found/i so a future SDK message tweak (casing,
translation) doesn't silently break the discriminator. The platform's
Hierarchy error is exact-cased today; the change is purely defensive.
2. resolveEmployeeId default-owner branch was probing
['contact:class:Person', 'contact:class:Employee'] while the email
branch (and the post-ff2ae6f preference) probes Employee first.
Reorder for consistency. Behaviorally a no-op when only one class is
provisioned (findOne returns undefined, loop falls through), but
matches the comment and the email branch.
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
1 participant
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
Two bugs in the Planner (
huly action ...) that prevented CLI-created todos from appearing in the right places:Bug 1 —
action create: wrongattachedToClassresolveEmployeeId()returns either anEmployeeorPersonref depending on the workspace model (afterff2ae6fit prefers Employee), butcreateActionhardcodedattachedToClassas'contact:class:Person'. The class/ref mismatch was rejected byaddCollectionor routed the todo to the wrong collection.Fix:
resolveEmployeeIdnow returns{ ref, class }.createActionmirrors the resolved class intoattachedToClass. When the email lookup helper returns just a ref,createActionprobes each candidate class to find the actual one.Bug 2 —
action schedule:WorkSlot.calendarset to Employee refscheduleActionsetcalendar: todo.user(an Employee ref), butEvent.calendaris aRef<Calendar>. The Schedule Calendar UI filters events by the user's PersonalCalendar ref, so CLI-created WorkSlots were invisible.Fix: new
resolvePrimaryCalendar()helper that mirrorsfindPrimaryCalendarfromplugins/time-resources/src/utils.ts: queriescalendar:class:Calendarforuser: primarySocialId, hidden: false, access ∈ {owner, writer}and honors thecalendar:class:PrimaryCalendarpreference'sattachedTo.Verification
action create --owner iamcoder18@gmail.com→attachedToClass=contact:class:Person(matches the resolved ref)action schedule HULY-1 --start 2026-08-04T21:00:00-06:00 --duration 30→ WorkSlot'scalendar=22608698-..._calendar(matches the user's PersonalCalendar, identical to UI-created HULY-2 slot)pnpm typecheckcleanTest artifacts cleaned up
The first
action createsmoke-test todo (6a72a80922356b1f1835bda5) became un-deletable viahuly action deletebecause the SDK'sremoveCollectionvalidates the storedattachedToClass(the buggy value the fix had stored before the second-pass fix). Removed via a directremoveDocRPC.