Skip to content

feat(cli+sdk): add triggers functionality to CLI+SDK#2261

Merged
LevinCeglie merged 19 commits into
devfrom
feat/add_trigger_functionality_cli-sdk
Jun 8, 2026
Merged

feat(cli+sdk): add triggers functionality to CLI+SDK#2261
LevinCeglie merged 19 commits into
devfrom
feat/add_trigger_functionality_cli-sdk

Conversation

@LevinCeglie

@LevinCeglie LevinCeglie commented May 26, 2026

Copy link
Copy Markdown
Collaborator

This PR adds full support for triggers to the CLI and SDK. The PR also includes tests for the new functionality.

@LevinCeglie LevinCeglie marked this pull request as ready for review June 3, 2026 10:29
@greptile-apps

greptile-apps Bot commented Jun 3, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds full trigger management (FILE, TIME, WEBHOOK) to both the CLI (klein triggers) and the Python SDK, following the same layered architecture used for existing resources (wrappers → core → routes).

  • New models and deserialization: ActionTrigger, FileConfig, TimeConfig, and WebhookConfig are added with proper frozen dataclasses; the _parse_action_trigger deserializer handles all three types with a defensive .get()-based approach for FileConfig but uses **dict splat-unpacking for TimeConfig/WebhookConfig.
  • Core validation: create_trigger and update_trigger in core.py validate mission existence, template existence, name uniqueness per mission, and type/config consistency before dispatching to the API layer.
  • Tests: Unit-level CRUD tests for all trigger types and validation edge-case tests (name rules, duplicates, bad entities, config mismatch) are included alongside an E2E CLI test.

Confidence Score: 4/5

The feature is largely correct and well-tested, but a few serialization and mutation concerns from prior review rounds remain unresolved in the API layer.

The trigger create/list/delete paths are solid and the type/config validation is thorough. The update path in core.update_trigger always forwards mission_uuid to the PATCH body (falling back to the existing trigger value when the caller passes None), meaning the backend will always receive a missionUuid field even on description-only updates. The WebhookConfig.__dict__ serialization producing {"extra_options": {}} in both the create and update payloads is also still present.

cli/kleinkram/core.py (update_trigger) and cli/kleinkram/api/routes.py (_create_trigger, _update_trigger) deserve a second look before merging.

Important Files Changed

Filename Overview
cli/kleinkram/api/deser.py Adds _parse_action_trigger with correct FileConfig parsing; TimeConfig and WebhookConfig use fragile **dict splat-unpacking instead of explicit field extraction.
cli/kleinkram/api/routes.py Adds trigger CRUD routes; get_trigger intentionally uses PATCH (no backend GET endpoint); _create_trigger and _update_trigger serialize TriggerConfig via config.__dict__ which produces {"extra_options": {}} for WebhookConfig.
cli/kleinkram/core.py Adds create_trigger, update_trigger, and delete_trigger with validation; update_trigger always passes a non-None mission_uuid to _update_trigger (falling back to the existing trigger value), which bypasses the "at least one field must be updated" guard.
cli/kleinkram/cli/_triggers.py New trigger CLI commands; FileConfig.event correctly defaults to () on both create and update paths; delete_trigger_cli and update_trigger_cli call parse_uuid_like without prior is_valid_uuid4 check unlike the other trigger commands.
cli/kleinkram/printing.py Adds trigger table and info printing; cleans up existing JSON serialization with a shared kleinkram_json_default helper; trigger.template_name (Optional[str]) passed directly to add_row, consistent with the existing execution table pattern.
cli/kleinkram/models.py Adds TriggerType, FileTriggerEvent, FileConfig, TimeConfig, WebhookConfig, and ActionTrigger dataclasses; all properly typed with frozen=True.
cli/tests/test_triggers.py New test file with CRUD tests for all three trigger types and validation tests for name rules, duplicate names, entity existence, and config mismatch.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as CLI (_triggers.py)
    participant Core as core.py
    participant Routes as routes.py
    participant Backend as Backend API

    User->>CLI: klein triggers create --name ... --type FILE
    CLI->>Core: create_trigger(name, mission_uuid, template_uuid, type_, config)
    Core->>Routes: get_mission(MissionQuery)
    Routes->>Backend: GET /missions
    Backend-->>Routes: mission data
    Core->>Core: _validate_trigger_name()
    Core->>Routes: get_triggers(TriggerQuery(mission_uuid))
    Routes->>Backend: "GET /triggers?missionUuid=..."
    Backend-->>Routes: triggers list
    Core->>Core: _validate_template_existence()
    Core->>Routes: get_template(template_id)
    Routes->>Backend: GET /templates/...
    Backend-->>Routes: template data
    Core->>Routes: _create_trigger(name, desc, template_uuid, mission_uuid, type_, config)
    Routes->>Backend: POST /triggers
    Backend-->>Routes: "{uuid: ...}"
    Routes-->>Core: UUID
    Core-->>CLI: UUID
    CLI-->>User: Trigger created with UUID ...
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
cli/kleinkram/api/deser.py:370-373
**Fragile splat-unpacking for `TimeConfig` and `WebhookConfig` parsing**

`FileConfig` is parsed defensively with explicit `.get()` calls, but `TimeConfig` and `WebhookConfig` are both constructed via `**trigger_object[CONFIG]`, which will raise `TypeError` if the backend ever returns any key not declared in the dataclass (e.g. a newly added backend field, a timezone hint, or a debug key). The exception is caught by the broad `except Exception` and surfaces as a `ParsingError`, which makes root-causing the failure harder. Consider using explicit field extraction with `.get()` for these types too, to match the `FileConfig` pattern and tolerate unknown keys gracefully.

Reviews (6): Last reviewed commit: "Merge branch 'dev' into feat/add_trigger..." | Re-trigger Greptile

Comment thread cli/kleinkram/api/routes.py
Comment thread cli/kleinkram/api/deser.py
Comment thread cli/kleinkram/api/routes.py
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Comment thread cli/kleinkram/cli/_triggers.py Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Jun 3, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Try greploops.

@LevinCeglie LevinCeglie requested a review from wp99cp June 3, 2026 11:26
Comment thread cli/kleinkram/cli/_triggers.py Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@LevinCeglie LevinCeglie force-pushed the feat/add_trigger_functionality_cli-sdk branch from 1ba74db to 25b41a0 Compare June 3, 2026 11:51
Comment thread cli/kleinkram/core.py
@wp99cp

wp99cp commented Jun 8, 2026

Copy link
Copy Markdown
Member

@LevinCeglie there are some merge conflics, can you resolve?

@LevinCeglie LevinCeglie merged commit 6d79fb9 into dev Jun 8, 2026
2 of 4 checks passed
@LevinCeglie LevinCeglie deleted the feat/add_trigger_functionality_cli-sdk branch June 8, 2026 08:29
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.

2 participants