fix(calendar): объединить напоминания одного события - #89
Conversation
|
Отдельно отмечу архитектурное решение: сценарии разделены на create, reschedule, update и delete, потому что Eve сначала видит только короткие descriptions и загружает полный текст выбранного навыка по необходимости. Так создание события получает подробные правила профиля и напоминаний, а перенос, правка и удаление сохраняют компактный контекст. Буду признателен за замечания по формату Calendar defaults в CORE и границам каждого сценария. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughОбновлены инструкции Google Calendar: удалён прежний CLI-пример, добавлены правила формирования объектов событий, повторяющихся и событий на весь день, а также настройки напоминаний через ChangesРуководство по событиям Google Calendar
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
scripts/google-calendar-skill.test.mjs (3)
14-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the changed root routing contract.
This test only loads
google-workspace.md; regressions inagent/instructions.mdlines 29-31 would remain green. Load that file and assert the structured-JSON requirement, Calendar scenario-skill routing, and exit-code-2 onboarding requirement.🤖 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 `@scripts/google-calendar-skill.test.mjs` around lines 14 - 23, Extend the test around the existing Google Workspace routing assertions to also load agent/instructions.md and verify its root routing contract: structured JSON responses, Calendar requests routed to scenario skills, and exit-code-2 onboarding behavior. Keep the existing focused-skill checks unchanged.
64-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Unicode-aware boundaries for the affirmative-language gate.
The separator whitelist excludes Markdown delimiters, quotes, and em dashes, so forms such as
—неor«неcan evade the check. It also flagsбезwithout understanding whether it is an instruction. Normalize punctuation or use Unicode letter/non-letter boundaries and add representative fixtures.🤖 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 `@scripts/google-calendar-skill.test.mjs` around lines 64 - 70, Update the negativeInstruction check in the “focused Calendar skills use affirmative instructions” test to use Unicode-aware letter/non-letter boundaries rather than the narrow separator whitelist, so forms such as —не and «не are detected without incorrectly matching inside words. Add representative fixtures covering Markdown delimiters, quotes, em dashes, and the existing negative terms, while preserving the current skill coverage.
25-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert safety sequencing, not just keyword presence.
These checks would pass even if the live insert occurs before confirmation,
sendUpdatesis placed in the event body, pagination is omitted, or sensitive values are echoed. Add order-sensitive assertions for dry-run → confirmation → live write, plus explicit checks for parameter placement and redaction wording.🤖 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 `@scripts/google-calendar-skill.test.mjs` around lines 25 - 45, Strengthen the Google Calendar policy tests around the create policy content by asserting the required sequence of dry-run, explicit confirmation, and live insert; verify sendUpdates is passed as an API parameter rather than included in the event body; require pagination handling; and assert wording that sensitive values are redacted rather than echoed. Use order-sensitive matching for the workflow and targeted assertions for parameter placement and redaction.agent/skills/google-calendar-create.md (1)
48-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDisambiguate Google Calendar defaults from the saved CORE profile.
Line 50 says to ignore “calendar defaults”, while line 52 says to use
Calendar defaults. Explicitly name the former as Google CalendardefaultRemindersand state that only those API defaults are ignored; otherwise the persisted CORE profile may be skipped. The API distinguishes calendar defaults from per-event reminder overrides. (developers.google.com)🤖 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 `@agent/skills/google-calendar-create.md` around lines 48 - 53, Clarify the reminders guidance by identifying “calendar defaults” as Google Calendar API defaultReminders and stating that only those defaults must be ignored. Ensure the workflow still uses the persisted CORE profile when available, then the current request’s reminders, and only falls back to Calendar defaults when neither source provides reminders.
🤖 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 `@agent/skills/google-calendar-create.md`:
- Around line 78-81: Update the sensitive-data confirmation flow in the calendar
creation instructions to display only categories and redacted identifiers, never
the exact detected values. Preserve the warning about calendar participant and
reader access, request explicit confirmation, and store exact sensitive values
only after confirmation.
- Around line 67-74: Update the workflow around the dry-run and live `gws
calendar events insert` steps so the JSON request is dry-run first, followed by
the event summary and an explicit user confirmation gate. Execute the
non-dry-run insert only after confirmation; clarify that an explicit creation
request does not bypass this gate.
- Around line 45-46: Update the participant-handling instructions in the
calendar event creation guidance to require sendUpdates in --params alongside
calendarId, using all, externalOnly, or none. Keep --json limited to the event
resource and do not place sendUpdates in the request body.
- Around line 62-65: Update the calendar conflict-checking flow in the
events-list step to retrieve every paginated result using --page-all with NDJSON
handling, or an equivalent pageToken loop. Run the existing time-and-meaning
duplicate detection against the complete event set before offering an update or
creating a new event.
In `@agent/skills/google-calendar-delete.md`:
- Around line 10-13: Преобразуйте выбранную область recurring-события в явный
API-target перед каждой mutable-операцией: в
agent/skills/google-calendar-delete.md (строки 10-13) укажите для series
recurringEventId, а для instance eventId и originalStartTime; в
agent/skills/google-calendar-reschedule.md (строки 11-17) добавьте те же targets
для patch и eventId плюс originalStartTime для move; в
agent/skills/google-calendar-update.md (строки 9-10) укажите parent
recurringEventId для patch серии и eventId с originalStartTime для экземпляра.
- Around line 11-16: Добавь явный выбор режима sendUpdates для операций delete,
patch и move: в google-calendar-delete.md запроси режим для удаления, отобрази
его в dry-run и подтверждении и передай в API; в google-calendar-reschedule.md
сделай то же для patch и move. Убедись, что выбранный режим явно показывается
пользователю и используется при выполнении соответствующего запроса.
- Around line 14-16: Закройте TOCTOU во всех mutation skills: в
agent/skills/google-calendar-delete.md (строки 14-16),
agent/skills/google-calendar-reschedule.md (строки 22-24) и
agent/skills/google-calendar-update.md (строки 24-26) замените шаги 7-10 так,
чтобы после подтверждения заново получать etag и передавать If-Match:<etag> в
delete/patch, а перед move выполнять отдельную проверку etag. При конфликте не
выполнять операцию и запрашивать новое подтверждение.
In `@agent/skills/google-calendar-reschedule.md`:
- Around line 32-33: Уточни описание в разделе о переносе событий: в тексте про
сохранение исходных полей явно укажи, что напоминания сохраняются, кроме случая
применения профиля `Calendar defaults` через `useDefault:true`. Согласуй
формулировку со строками 19–20, не изменяя поведение остальных полей.
- Around line 18-24: Дополните инструкцию для сценария useDefault:true явной
остановкой после первой ошибки patch или move: повторно получите событие, точно
сообщите выполненные и невыполненные шаги, затем предложите компенсационное
действие или ручное продолжение вместо безусловного повтора. Сохраните move как
отдельный API-запрос без тела patch и опишите обработку частичных результатов
после каждого шага.
In `@agent/skills/google-calendar-update.md`:
- Around line 18-20: Обнови инструкцию вокруг проверки чувствительных сведений в
сценарии обновления календаря: до финального подтверждения, dry-run и успешной
проверки etag не записывай их в календарь, а сохраняй только в подготовленном
patch. Выполняй запись в указанное пользователем поле лишь после финального
подтверждения и успешной условной мутации.
---
Nitpick comments:
In `@agent/skills/google-calendar-create.md`:
- Around line 48-53: Clarify the reminders guidance by identifying “calendar
defaults” as Google Calendar API defaultReminders and stating that only those
defaults must be ignored. Ensure the workflow still uses the persisted CORE
profile when available, then the current request’s reminders, and only falls
back to Calendar defaults when neither source provides reminders.
In `@scripts/google-calendar-skill.test.mjs`:
- Around line 14-23: Extend the test around the existing Google Workspace
routing assertions to also load agent/instructions.md and verify its root
routing contract: structured JSON responses, Calendar requests routed to
scenario skills, and exit-code-2 onboarding behavior. Keep the existing
focused-skill checks unchanged.
- Around line 64-70: Update the negativeInstruction check in the “focused
Calendar skills use affirmative instructions” test to use Unicode-aware
letter/non-letter boundaries rather than the narrow separator whitelist, so
forms such as —не and «не are detected without incorrectly matching inside
words. Add representative fixtures covering Markdown delimiters, quotes, em
dashes, and the existing negative terms, while preserving the current skill
coverage.
- Around line 25-45: Strengthen the Google Calendar policy tests around the
create policy content by asserting the required sequence of dry-run, explicit
confirmation, and live insert; verify sendUpdates is passed as an API parameter
rather than included in the event body; require pagination handling; and assert
wording that sensitive values are redacted rather than echoed. Use
order-sensitive matching for the workflow and targeted assertions for parameter
placement and redaction.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 35b3e650-224b-45b6-9deb-fe7a56998826
📒 Files selected for processing (7)
agent/instructions.mdagent/skills/google-calendar-create.mdagent/skills/google-calendar-delete.mdagent/skills/google-calendar-reschedule.mdagent/skills/google-calendar-update.mdagent/skills/google-workspace.mdscripts/google-calendar-skill.test.mjs
|
Обновил PR по замечаниям CodeRabbit в коммите 34c3ea7. Исправлено:
По If-Match оставил отдельное техническое объяснение в inline-треде: текущий gws не поддерживает произвольные HTTP-заголовки. @coderabbitai review |
|
✏️ Learnings added
✅ Action performedReview finished.
|
yakovmakovets
changed the title
|
Уточнил реализацию после замечания и сверки с актуальной документацией Eve. В отдельном коммите
Также переписал заголовок и описание PR: сначала сформулирована пользовательская проблема и ожидаемое поведение, затем устройство решения и ссылки на документацию Eve и Google. |
|
Спасибо за серьёзную проработку - два раунда правок по ревью, сверка с докой eve, продуманная маскировка чувствительных данных. Проблема, которую ты решаешь, реальная и фикс мы хотим: «напомни за день, за час и за 10 минут» действительно должно быть одним событием с тремя Но в текущем объёме PR не возьмём - он решает 15-строчную проблему 340 строками. Разбор по пунктам, чтобы было понятно, что именно смущает: Объём и UX. С этими скиллами каждое «поставь встречу завтра в 15» проходит: анкету профиля из 6 вопросов → чтение схемы API → выгрузку всех страниц событий → поиск дублей → dry-run → показ итогов → явное подтверждение → insert. Для личного ассистента это слишком церемонно: простая просьба должна исполняться одним шагом. Дубли. Блок «серия vs экземпляр» повторён дословно в 3 скиллах, правила слияния напоминаний - в 3, etag-перепроверка - в 3, допрос про
etag. Как выяснилось в треде, Тест. Греп русской прозы завязывает CI на формулировки: любая редакция текста скилла уронит сборку. А запрет слов «не/ни/без/никогда» конфликтует со стилем самого репо (instructions.md пишет «НИКОГДА» капсом). Инструкции-скиллы мы тестами прозы не покрываем. instructions.md. Замена «ОБЯЗАТЕЛЬНО сначала загрузи скилл Что предлагаем оставить - урезать PR до ~15-20 строк в существующем
Description Урежешь до этого объёма - смержим быстро. Если некогда, скажи - сделаем компактный вариант сами с кредитом тебе в CHANGELOG. |
yakovmakovets
changed the title
|
Спасибо за подробный разбор — согласен с границей задачи. В коммите
Финальный diff: один файл, |
715497f to
9da93c8
Compare
3 participants
Проблема
Запрос «напомни за день, за час и за 10 минут» должен создавать одно событие нужной длительности с
несколькими напоминаниями, а не несколько коротких событий.
Изменение
В существующий навык
google-workspaceдобавлены компактные правила работы с событиями GoogleCalendar:
reminders.overridesсreminders.useDefault: false;popup/emailи интервала;end.dateуказывает день после последнего дня;gws calendar events insertс тремя напоминаниями.OAuth-flow,
gws,instructions.md, Telegram-канал и остальные навыки не изменяются.Документация
Summary by CodeRabbit