Skip to content

Use notification destination ids in tools SDK#135

Open
leo-paz wants to merge 3 commits into
mainfrom
codex/sdk-notification-destination-ids
Open

Use notification destination ids in tools SDK#135
leo-paz wants to merge 3 commits into
mainfrom
codex/sdk-notification-destination-ids

Conversation

@leo-paz

@leo-paz leo-paz commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Update the public notification tool contract to accept destinationIds instead of provider/channel destination objects.
  • Update outlit notify --destination to forward NotificationDestination ids.
  • Add focused tests for the tool contract, client payload, and CLI notify command.

Validation

  • bun test packages/tools/tests/client.test.ts packages/cli/tests/commands/notify.test.ts
  • bunx turbo run typecheck --filter=@outlit/tools --filter=@outlit/cli

Summary by CodeRabbit

  • New Features

    • Changed notification destination specification from provider:channel format to UUID-based destination identifiers.
  • Documentation

    • Updated CLI help text and usage examples to reflect new notification destination ID format.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Walkthrough

This PR migrates notification destination addressing from a provider:channelId format to UUID-based destination IDs. The tool contract schema is updated, the CLI command now validates and forwards comma-separated UUID IDs, and all CLI, Pi extension, and tools tests are updated to reflect the new addressing model.

Changes

Notification Destination ID Migration

Layer / File(s) Summary
Tool contract schema and hash update
packages/tools/src/contracts.ts
The outlit_send_notification tool input schema replaces destinations (provider/channelId/label objects) with destinationIds (UUID string array, validated with min/max items and UUID pattern). Contract hash is updated to 03e7ece1....
CLI notify command UUID destination parsing and forwarding
packages/cli/src/commands/notify.ts
Removes provider-related imports, introduces parseDestinationIds to validate comma-separated UUIDs, and updates examples and --destination argument description. Runtime parsing and tool parameter building now use destinationIds instead of destinations.
CLI, extension, and tools test suite updates
packages/cli/tests/commands/notify.test.ts, packages/pi/tests/extension.test.ts, packages/tools/tests/client.test.ts
All tests updated: CLI tests now pass UUID destination IDs and validate destinationIds in payloads; Pi extension test asserts destinationIds in parameters; tools tests verify UUID schema constraints and destinationIds in request bodies.
Release metadata
.changeset/agent-notification-destinations.md
Marks patch releases for @outlit/cli, @outlit/pi, and @outlit/tools, documenting the notification destination ID change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • OutlitAI/outlit-sdk#107: Added the notification tool feature that this PR refactors to use destination IDs instead of provider:channel format.
  • OutlitAI/outlit-sdk#123: Earlier changes to notification destination handling that conflict with this PR's UUID-based approach.

Poem

🐰 From channels of old to IDs shiny and new,
We swap out the formats, UUID through and through,
No more provider:name, just a string tried and true—
Notifications now routed where UUIDs flew! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Use notification destination ids in tools SDK' directly and clearly summarizes the main change: updating the notification tool contract to accept destination IDs instead of provider/channel objects, which is the central theme across all modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 codex/sdk-notification-destination-ids

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

@leo-paz leo-paz force-pushed the codex/sdk-notification-destination-ids branch from 501dc91 to 6ea6b47 Compare May 12, 2026 06:47
@leo-paz leo-paz marked this pull request as ready for review May 12, 2026 07:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/commands/notify.ts`:
- Around line 12-13: The CLI's destinationIdPattern currently restricts UUID
version to [1-5], causing valid IDs (v7, nil, max) accepted by the contract to
be rejected; update the destinationIdPattern constant to match the contract
schema by allowing version digit [1-8] and explicitly permitting the nil UUID
(00000000-0000-0000-0000-000000000000) and the max UUID
(ffffffff-ffff-ffff-ffff-ffffffffffff) so the CLI validation aligns with
contracts.ts.
🪄 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

Run ID: 7566d807-8462-4341-a4bf-e3fc5cd0f7d0

📥 Commits

Reviewing files that changed from the base of the PR and between 6982f14 and 6ea6b47.

📒 Files selected for processing (6)
  • .changeset/agent-notification-destinations.md
  • packages/cli/src/commands/notify.ts
  • packages/cli/tests/commands/notify.test.ts
  • packages/pi/tests/extension.test.ts
  • packages/tools/src/contracts.ts
  • packages/tools/tests/client.test.ts

Comment on lines +12 to +13
const destinationIdPattern =
/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

UUID pattern mismatch between CLI and contract schema.

The CLI pattern allows only UUID versions 1-5 ([1-5]), while the contract schema in contracts.ts allows versions 1-8 ([1-8]) plus nil/max UUIDs. This means:

  • A UUID v7 (e.g., 01932c6a-7a6c-7e9d-8a1b-123456789012) would pass server-side validation but fail CLI validation.
  • The nil UUID (00000000-0000-0000-0000-000000000000) explicitly allowed in the contract would fail CLI validation.

Consider aligning the CLI pattern with the contract schema to avoid rejecting valid destination IDs.

🔧 Proposed fix to align with contract schema
 const destinationIdPattern =
-  /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i
+  /^([0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}|00000000-0000-0000-0000-000000000000|ffffffff-ffff-ffff-ffff-ffffffffffff)$/i
🤖 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 `@packages/cli/src/commands/notify.ts` around lines 12 - 13, The CLI's
destinationIdPattern currently restricts UUID version to [1-5], causing valid
IDs (v7, nil, max) accepted by the contract to be rejected; update the
destinationIdPattern constant to match the contract schema by allowing version
digit [1-8] and explicitly permitting the nil UUID
(00000000-0000-0000-0000-000000000000) and the max UUID
(ffffffff-ffff-ffff-ffff-ffffffffffff) so the CLI validation aligns with
contracts.ts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant