diff --git a/.agents/skills/adding-a-trigger/SKILL.md b/.agents/skills/adding-a-trigger/SKILL.md new file mode 100644 index 0000000000..7d8643b862 --- /dev/null +++ b/.agents/skills/adding-a-trigger/SKILL.md @@ -0,0 +1,267 @@ +--- +name: adding-a-trigger +description: Checklist for adding a new TriggerCrud-based trigger type to Windmill (Azure, GCP, Kafka, etc.). Use when wiring a new trigger kind across backend, frontend, CLI, and capture infrastructure. +--- + +# Skill: Adding a New Trigger Type + +Use this skill when adding a trigger kind that implements `TriggerCrud` (Kafka, GCP, Azure, MQTT, SQS, NATS, Postgres, Email…). For native triggers (Nextcloud, Google Drive — things wired through `windmill-native-triggers`), use the `native-trigger` skill instead. + +The goal of this doc is to enumerate every file that needs to change. Missing any one of them leads to silent regressions: sync drops the trigger, capture button does nothing, workspace forks lose it, sidebar counters undercount. Follow the checklist top-to-bottom — each section is independent enough to be validated on its own. + +Throughout this doc, substitute `{kind}` for the new trigger kind (`azure`, `kafka`, …), `{Kind}` for PascalCase (`Azure`, `Kafka`), `{KIND}` for SCREAMING (`AZURE`, `KAFKA`). + +## Reference implementations + +- **GCP** — closest analogue to Azure. Has push + pull, OIDC auth, ARM-like resource paths, capture handler. Grep for `gcp_trigger` / `GcpTrigger`. +- **Kafka** — simpler (pull-only, streaming). Good for trivial integrations. +- **Azure** — most recently added (2026). Shared-secret push auth, Event Grid namespaces + basic topics, ARM resource discovery, Namespace-pull data-plane. Grep for `azure_trigger` / `AzureTrigger`. + +## 1. Database migration + +Create a migration: `cargo sqlx migrate add -r add_{kind}_trigger` from `backend/`. Never write timestamps manually. + +The `up.sql` usually defines: +- An optional enum type (e.g. `AZURE_MODE`) if the trigger has sub-kinds +- The `{kind}_trigger` table with at minimum these columns (mirrored from kafka/gcp): + - primary: `(workspace_id, path)` + - `script_path`, `is_flow`, `enabled`, `mode`, `permissioned_as`, `edited_by`, `email` + - `edited_at`, `error`, `server_id`, `last_server_ping` + - `error_handler_path`, `error_handler_args jsonb`, `retry jsonb` + - trigger-specific fields +- Indexes on foreign keys + any frequently-filtered columns +- Foreign key to `workspace` + +Down migration drops the table and any enum types. + +## 2. Backend crate (`windmill-trigger-{kind}`) + +Create a new crate under `backend/windmill-trigger-{kind}/` with: + +- `Cargo.toml`: features `enterprise`, `private` if EE, standard deps +- `src/lib.rs`: `pub use mod_ee::*;` behind `#[cfg(all(feature = "enterprise", feature = "private"))]` +- `src/mod_ee.rs`: core types + helpers +- `src/handler_ee.rs`: `TriggerCrud` impl + route handlers +- `src/listener_ee.rs`: (only if streaming/pull-based) `Listener` trait impl + +Required in `mod_ee.rs`: +- `{Kind}Config` struct (persisted shape, `FromRow`) +- `{Kind}ConfigRequest` struct (what API receives — usually similar to Config but with validation fields) +- `{Kind}Trigger` unit struct (implements the traits) +- `impl TriggerJobArgs for {Kind}Trigger` — sets `TRIGGER_KIND`, `Payload`, `v1_payload_fn` + +Required in `handler_ee.rs`: +- `#[async_trait] impl TriggerCrud for {Kind}Trigger` with: + - `type Trigger = Trigger<{Kind}Config>` + - `type TriggerConfigRequest = {Kind}ConfigRequest` + - `const ROUTE_PREFIX: &'static str = "/{kind}_triggers";` + - `const TABLE_NAME`, `ADDITIONAL_SELECT_FIELDS` + - `get_deployed_object`, `validate_config`, `create_trigger`, `update_trigger`, `delete_trigger`, `test_connection` + - `additional_routes` (optional — mount extra endpoints for things like ARM resource listing, topic discovery) + +Register the crate in `backend/Cargo.toml` as a workspace member and as a dep of `windmill-api` behind the feature flag. + +## 3. Wire into `windmill-api` (feature-gated everywhere) + +**`backend/windmill-api/src/triggers/handler.rs`** — mount the trigger crate: +```rust +#[cfg(all(feature = "enterprise", feature = "{kind}_trigger", feature = "private"))] +{ + use crate::triggers::{kind}::{Kind}Trigger; + router = router.nest({Kind}Trigger::ROUTE_PREFIX, complete_trigger_routes({Kind}Trigger)); +} +``` + +**`backend/windmill-api/src/triggers/{kind}/mod.rs`** — re-export the crate: +```rust +pub use windmill_trigger_{kind}::*; +``` + +**`backend/windmill-api/src/lib.rs`** — if the trigger receives inbound pushes, add a webhook route: +```rust +.nest("/{kind}/w/{workspace_id}", { + #[cfg(all(feature = "enterprise", feature = "{kind}_trigger", feature = "private"))] + { triggers::{kind}::handler_oss::{kind}_push_route_handler() } + #[cfg(not(...))] + { Router::new() } +}) +``` + +## 4. `TriggerKind` enum (`backend/windmill-types/src/triggers.rs`) + +Already has slots for most triggers but verify your variant exists: +- Add `{Kind}` to the `TriggerKind` enum +- Add match arm in `to_key()` +- Add match arm in `from_str` +- Add match arm in `JobTriggerKind` (if jobs need kind tagging) + +## 5. OpenAPI (`backend/windmill-api/openapi.yaml`) + +This file is huge and the single most-forgotten place. Add: + +- `/w/{workspace}/{kind}_triggers/create` + `/update/{path}` + `/delete/{path}` + `/get/{path}` + `/list` + `/exists/{path}` + `/setmode/{path}` + `/test` paths (mirror gcp section) +- Any `additional_routes` your handler exposes (resource discovery, etc.) +- Schemas: `{Kind}Trigger`, `{Kind}TriggerData`, `{Kind}Mode` (if enum), `{Kind}DeliveryConfig`, helper request/response types +- Add `{kind}` to `CaptureTriggerKind` enum +- Add `{kind}_used: boolean` to the `UsedTriggers` response schema + +Regenerate frontend client: `npm run generate-backend-client` from `frontend/`. + +## 6. `UsedTriggers` + workspace export + +**`backend/windmill-api-workspaces/src/workspaces.rs`** — add `{kind}_used: bool` to the `UsedTriggers` struct and add an `EXISTS(SELECT 1 FROM {kind}_trigger …)` to the `get_used_triggers` query. + +**`backend/windmill-api/src/workspaces_export.rs`** — add export block mirroring gcp's (export lists all triggers, serializes them to YAML/JSON). The block re-uses the `trigger_ignore_keys` variable so the new kind automatically participates in fork-export stripping (`mode` field is omitted when the source workspace is a fork — keeps fork→parent merges from flipping the parent's enabled state). + +**Fork cloning (`clone_triggers_and_schedules` in workspaces.rs)** — add an `INSERT INTO {kind}_trigger ... SELECT ...` block that copies all rows from the parent workspace, forcing `mode = 'disabled'::TRIGGER_MODE`. Always runs at fork creation; forgetting this means users can't carry `{kind}` triggers into their forks. + +## 6.5 Hardcoded trigger-kind arrays (silent-failure hotspots) + +Several files keep **hardcoded arrays** of trigger kind strings. Miss one and ACL checks / user offboarding / trash drop your kind: + +- **`backend/windmill-api-groups/src/granular_acls.rs`** — `KINDS: [&str; N]`. **Increment N** (the compile error is cryptic otherwise). Controls which kinds accept granular ACL operations. +- **`backend/windmill-api-users/src/users.rs`** (`extra_perms_tables`) — which tables get `extra_perms` entries cleaned when a user is deleted. +- **`backend/windmill-api/src/offboarding.rs`** — three separate arrays (enumeration, fork-copy, and delete paths). **All three** need the new kind. +- **`backend/windmill-api/src/trash.rs`** — `valid_tables` for the trash / restore API. +- **`backend/windmill-git-sync/src/lib.rs`** — add a test assertion for `DeployedObject::{Kind}Trigger.get_kind() == "{kind}_trigger"` (the `get_kind` match arm itself lives in the enum impl — already required by the Rust compiler). +- **`backend/windmill-api-auth/src/scopes.rs`** — add the `{Kind}Triggers` variant to `ScopeDomain` enum + `as_str` match + `from_str` match. Required for the OAuth/token system to recognise `{kind}_triggers:read|write` scopes. +- **`backend/windmill-api/src/token.rs`** (`build_trigger_scope_domains` → `TRIGGER_DOMAINS`) — add `("{kind}_triggers", "{Kind display name}")` so the CreateToken UI's scope selector surfaces the `read` / `write` checkboxes. + +**OpenAPI enums** to extend (do NOT forget — generated client will allow it but server rejects as 400): +- `CaptureTriggerKind` enum +- Three `kind` enums under `/w/{workspace}/acls/{get,add,remove}/{kind}/{path}` (yes, same list repeated three times) + +After editing any of these, run a full `cargo check` with your feature flag + `gcp_trigger` + other core flags — the `KINDS: [&str; N]` length mismatch only surfaces when the crate compiles. + +## 7. Capture infrastructure (`backend/windmill-api/src/capture.rs`) + +If the trigger supports push delivery, it also needs a capture endpoint so users can test it: + +- `{Kind}TriggerConfig` struct (gated by feature flags) +- `TriggerConfig::{Kind}` variant +- `set_{kind}_trigger_config` function (creates the subscription/equivalent pointing at the capture URL — use your `manage_{kind}_subscription` helper with `trigger_mode=false`) +- Both real + no-op versions behind feature gates +- `TriggerKind::{Kind} => set_{kind}_trigger_config(...)` arm in `set_config` +- `{kind}_payload` async handler — validates auth (if any), processes payload, calls `insert_capture_payload` +- Route: `.route("/{kind}/{runnable_kind}/{*path}", post({kind}_payload))` inside `workspaced_unauthed_service` — and expand the surrounding `#[cfg(any(...))]` to include your feature flag + +## 8. CLI (`cli/`) — easy to miss, breaks sync silently + +Check all of these: + +**`cli/src/types.ts`:** +- Add `"{kind}"` to `TRIGGER_TYPES` array +- Add `"{kind}_trigger"` to `getTypeStrFromPath` return union +- Add match case in `getTypeStrFromPath`'s `typeEnding ===` chain +- Add `pushTrigger("{kind}", ...)` branch in `pushObj` + +**`cli/src/commands/trigger/trigger.ts`:** +- Import `{Kind}Trigger` type +- Add `{kind}: {Kind}Trigger` to the `Trigger` type map +- Add `{kind}: wmill.get{Kind}Trigger`, `update{Kind}Trigger`, `create{Kind}Trigger` to each function map +- Add `{kind}: { ... }` template to `triggerTemplates` +- Add `list{Kind}Triggers` call + spread in the `list` aggregation +- Update `--kind` option descriptions to mention the new kind + +**`cli/src/commands/sync/sync.ts`:** +- Add `path.endsWith(".{kind}_trigger" + ext)` in the file-type filter +- Add `typ == "{kind}_trigger"` in `getTypeOrder` +- Add `"{kind}_trigger"` to the delete-suffix regex (~line 3092) +- Add a `case "{kind}_trigger"` in the delete switch + +**`cli/src/guidance/skills.ts`** — **DO NOT EDIT DIRECTLY**. It's auto-generated by `system_prompts/generate.py`. Instead: +- Edit `system_prompts/utils.py` → append `('{Kind}Trigger', '{kind}_trigger')` to the `SCHEMA_MAPPINGS['triggers']` list (this is the master list — the one in `generate.py` is duplicated and `utils.py` wins) +- Then run `python3 system_prompts/generate.py` — it regenerates `cli/src/guidance/skills.ts` with the schema extracted from `backend/windmill-api/openapi.yaml` +- Commit the regenerated file + +## 9. Frontend — editor + drawer + +Under `frontend/src/lib/components/triggers/{kind}/`: + +- `{Kind}TriggerPanel.svelte` — the tile shown in the triggers listing +- `{Kind}TriggerEditor.svelte` — outer drawer wrapper +- `{Kind}TriggerEditorInner.svelte` — state + business logic; must expose: + - `openEdit(path, isFlow, defaultValues?)` method + - `isEditor` prop, `onConfigChange` + `onCaptureConfigChange` callbacks + - `get{Kind}Config()` + `get{Kind}CaptureConfig()` helpers + - `captureConfig = $derived.by(untrack(() => isEditor) ? get{Kind}CaptureConfig : () => ({}))` + - `$effect(() => { const args = [captureConfig, isValid] as const; untrack(() => onCaptureConfigChange?.(...args)) })` +- `{Kind}TriggerEditorConfigSection.svelte` — form fields; use design-system components (`TextInput`, `Select`, `Toggle`, `ToggleButtonGroup`), never raw `` +- `{Kind}Capture.svelte` — capture panel; wraps `CaptureSection` with `captureType="{kind}"` +- `utils.ts` — `requestBody` builders and any trigger-type-specific helpers + +## 10. Frontend — global integration + +Easy to miss: + +- **`frontend/src/lib/components/triggers.ts`** — add `'{kind}'` to the `TriggerKind` union +- **`frontend/src/lib/components/triggers/CaptureWrapper.svelte`**: + - Import `{Kind}Capture` + - Add to `isStreamingCapture()` array (streaming = pull-style; push-style is typically `false`) + - Add `{:else if captureType === '{kind}'}` branch with the `<{Kind}Capture>` render +- **`frontend/src/lib/components/sidebar/SidebarContent.svelte`** — import the icon, add the nav entry +- **`frontend/src/lib/components/sidebar/OperatorMenu.svelte`** — add the operator-mode entry +- **`frontend/src/routes/(root)/(logged)/+layout.svelte`** — destructure `{kind}_used` from `/get_used_triggers` response, push `'{kind}'` into `usedKinds` +- **`frontend/src/lib/components/search/GlobalSearchModal.svelte`** — import icon, add "Go to {Kind} ..." entry +- **`frontend/src/lib/components/offboarding-utils.ts`** — add mappings `{kind}_trigger: '{kind}_triggers'` and `{kind}_trigger: '{kind} trigger'` +- **`frontend/src/lib/components/icons/{Kind}Icon.svelte`** — single-path SVG, `fill={color ?? 'currentColor'}`, `size` prop default 16 (match existing icons — don't hardcode colors, don't use `width`/`height` props) +- **`frontend/src/routes/(root)/(logged)/{kind}_triggers/+page.svelte`** — listing page (mirror `gcp_triggers/+page.svelte` for push+pull, `kafka_triggers` for pure streaming) +- **`frontend/src/lib/components/CompareWorkspaces.svelte`** — workspace fork / compare tool. Needs: service import, editor import, `{kind}Editor` `$state`, `case '{kind}'` in `openTriggerDetails()`, entry in `triggerServices` object (list/delete/normalize), and `<{Kind}TriggerEditor bind:this={{kind}Editor} />` in the template + +## 10.5 AI system prompts (`system_prompts/`) + +- **`system_prompts/utils.py`** — append `('{Kind}Trigger', '{kind}_trigger')` to `SCHEMA_MAPPINGS['triggers']` (master list used by code generation + CLI skills) +- **`system_prompts/generate.py`** — also has a duplicated `schema_types` list (~line 903) for the AI `triggers` skill content. Add `('{Kind}Trigger', '{kind}_trigger')` there too +- **`system_prompts/generate.py`** `schema_names` (~line 1192) — add `'{Kind}Trigger'` (add `'New{Kind}Trigger'` only if the OpenAPI declares one; GCP and Azure don't) +- Run `python3 system_prompts/generate.py` — this rewrites `cli/src/guidance/skills.ts` and all `auto-generated/` docs. Commit the regenerated files + +## 11. Validation + +Run all of these before declaring done: + +```bash +# Backend +cd backend +cargo check --features enterprise,{kind}_trigger,private # minimal +cargo check --features enterprise,azure_trigger,private,gcp_trigger,http_trigger,mqtt_trigger,postgres_trigger,sqs_trigger,kafka,nats,smtp,websocket # full + +# SQLx offline data (never run `cargo sqlx prepare` directly — use the wrapper) +./update_sqlx.sh + +# Frontend +cd frontend +npm run generate-backend-client +npm run check:fast +``` + +Smoke test in the UI: create a trigger, save, check it appears in sidebar + search, delete, re-create via CLI `wmill sync`. + +## 12. Common pitfalls + +- **Forgetting feature gates in `workspaced_unauthed_service()`** — the surrounding `#[cfg(any(...))]` expression must include your feature flag, not just the inner `#[cfg]` on the route +- **`.route(path, ...).route(path, ...)` with same path and different methods** — older axum replaced; use `.route(path, post(h1).options(h2))` to chain methods on the same `MethodRouter` +- **`on:event` directives** — legacy Svelte 4, no-op in runes mode. Use callback props (`onSelected`, `onConfigChange`) +- **`$bindable(default_value)` on optional props** — banned by project CLAUDE.md. Use `$bindable()` + `$derived(prop ?? default)` instead +- **CORS layer intercepting OPTIONS** — tower-http CorsLayer short-circuits OPTIONS before reaching your handler. For server-to-server webhook endpoints, drop the CORS layer entirely (CORS is browser-only) +- **DeliveryAttributeMappings / custom headers for auth** — prefer HMAC or sha256-hashed shared secrets over opaque JWTs when the provider doesn't support signed tokens natively. Store only the hash; regenerate secret on every save +- **ARM / API resource-listing cascades** — if the trigger's resource type is deep (Azure: subscription → RG → namespace → topic), offer dropdowns in the UI populated from the provider's APIs using the user's credential resource +- **Clearing stale selections on dependency change** — when a dropdown's underlying data reloads (e.g., user changes SP or edition), clear selections that no longer match the new list +- **Workspace-scoped tag compatibility** — if the trigger has tags, verify forked workspaces handle them (see commit `0773b5bc85` for a historical fix) + +## 13. EE file split + +If the trigger is enterprise-only, the code lives in `windmill-ee-private__worktrees/.../windmill-trigger-{kind}/src/*_ee.rs` and is symlinked into the OSS tree. The `windmill-ee-private__worktrees/` directory holds the real files; changes propagate via symlinks. See `docs/enterprise.md` for the workflow. + +## 14. Final checklist before PR + +- [ ] Migration up/down tested (revert + re-apply) +- [ ] `./update_sqlx.sh` committed the updated `.sqlx/` offline data +- [ ] `cargo check` passes with your feature flag + with all trigger features +- [ ] `npm run check:fast` passes +- [ ] Trigger visible in sidebar with correct icon weight (not oversized/colored — use `currentColor`) +- [ ] Create, edit, delete flow all work in the UI +- [ ] Capture button works (if push-capable) +- [ ] Trigger appears in `/get_used_triggers` → sidebar pulse +- [ ] `wmill sync pull` + `wmill sync push` both round-trip the trigger +- [ ] `wmill trigger list` includes it +- [ ] OpenAPI schemas are complete (no `null` in generated types) diff --git a/.agents/skills/commit/SKILL.md b/.agents/skills/commit/SKILL.md index 3f97552466..114531570e 100644 --- a/.agents/skills/commit/SKILL.md +++ b/.agents/skills/commit/SKILL.md @@ -1,5 +1,6 @@ --- name: commit +user_invocable: true description: Create a git commit with conventional commit format. MUST use anytime you want to commit changes. --- diff --git a/.agents/skills/local-review/SKILL.md b/.agents/skills/local-review/SKILL.md index ad701ac367..3477911176 100644 --- a/.agents/skills/local-review/SKILL.md +++ b/.agents/skills/local-review/SKILL.md @@ -1,97 +1,66 @@ --- name: local-review -description: Code review a pull request for bugs and CLAUDE.md compliance. MUST use when asked to review code. +description: Code review the current PR (or branch diff against main) for bugs, security, and AGENTS.md compliance. MUST use when asked to review code. --- -# Local Code Review Skill +# Local Code Review -Review a pull request for real bugs and CLAUDE.md compliance violations. This review targets HIGH SIGNAL issues only. +Run the same review locally that the GitHub auto-review actions run on PRs (Claude / Codex / Pi). The review policy lives in `.github/review-prompt-shared.md` — read that first. -## Review Philosophy +## Steps -- **Only flag issues you are certain about.** If you are not sure an issue is real, do not flag it. False positives erode trust and waste reviewer time. -- Think like a senior engineer doing a final review — flag things that would cause incidents, not things that are merely imperfect. +1. **Read `.github/review-prompt-shared.md`** for the review policy: severity triage (P0 / P1 / P2), the new-public-surface checklist, AGENTS.md compliance, and the test-coverage assessment. -## What to Flag - -- Code that won't compile or parse (syntax errors, type errors, missing imports) -- Code that will definitely produce wrong results regardless of inputs -- Clear, unambiguous CLAUDE.md violations (quote the exact rule being violated) -- Security issues in introduced code (injection, auth bypass, data exposure) -- Incorrect logic that will fail in production - -## What NOT to Flag - -- Code style or quality concerns -- Potential issues that depend on specific inputs or runtime state -- Subjective suggestions or improvements -- Pre-existing issues not introduced by this PR -- Pedantic nitpicks a senior engineer wouldn't flag -- Issues a linter or type checker will catch -- General quality concerns unless explicitly prohibited in CLAUDE.md -- Issues silenced via lint ignore comments - -## Execution Steps - -1. **Determine the PR scope**: - - If an argument is provided, use it as the PR number or branch - - Otherwise, detect from the current branch vs main - - Run `gh pr view` if a PR exists, or use `git diff main...HEAD` - -2. **Find relevant CLAUDE.md files**: - - Read the root `CLAUDE.md` - - Check for CLAUDE.md files in directories containing changed files +2. **Determine the PR scope**: + - If an argument is provided, treat it as a PR number or branch. + - Otherwise, detect from the current branch vs `main`. + - Run `gh pr view` if a PR exists; otherwise compare against `main` with `git diff main...HEAD`. 3. **Get the diff and metadata**: - - `gh pr diff` or `git diff main...HEAD` for the full diff - - `gh pr view` or `git log main..HEAD --oneline` for context + - `gh pr diff` or `git diff main...HEAD` for the full diff. + - `gh pr view` or `git log main..HEAD --oneline` for context. -4. **Read changed files** where the diff alone is insufficient to understand context +4. **Read changed files** when the diff alone is insufficient. -5. **Review for**: - - CLAUDE.md compliance — check each rule against the changed code - - Bugs and logic errors — will this code work correctly? - - Security issues — injection, auth, data exposure in new code +5. **Apply the policy** from `.github/review-prompt-shared.md`. Self-validate each finding before reporting (real issue? would a senior engineer flag it?). -6. **Self-validate each finding**: Before reporting, ask yourself: - - "Is this definitely a real issue, not a false positive?" - - "Would a senior engineer flag this in review?" - - If the answer to either is no, discard the finding +6. **Output findings** to the terminal (default) or post as PR comments (with `--comment` flag). -7. **Output findings** to the terminal (default) or post as PR comments (with `--comment` flag) - -## Output Format +## Output format ``` ## Code review Found N issues: -1. () +1. [P0|P1|P2] -2. () +2. [P0|P1|P2] ``` +End with a `Test coverage` section per the shared policy. + If no issues are found: ``` ## Code review -No issues found. Checked for bugs and CLAUDE.md compliance. +No issues found. Checked for bugs, security, and AGENTS.md compliance. ``` -## Posting Comments (--comment flag) +## Posting comments (`--comment`) -If the user passes `--comment`, post findings as inline PR comments using: +For a top-level PR comment: ```bash gh pr review --comment --body "" ``` -Or for inline comments on specific lines: +For inline comments on specific lines: ```bash -gh api repos/{owner}/{repo}/pulls/{pr}/reviews -f body="" -f event="COMMENT" -f comments="[...]" +gh api repos/{owner}/{repo}/pulls/{pr}/reviews \ + -f body="" -f event="COMMENT" -f comments="[...]" ``` diff --git a/.agents/skills/native-trigger/SKILL.md b/.agents/skills/native-trigger/SKILL.md index 781e200d0c..ae38f70b32 100644 --- a/.agents/skills/native-trigger/SKILL.md +++ b/.agents/skills/native-trigger/SKILL.md @@ -607,7 +607,18 @@ In `frontend/src/lib/components/triggers/TriggersEditor.svelte`: Add your service to the `nativeTriggerServices` map in `deleteDeployedTrigger()`. Native triggers use `NativeTriggerService.deleteNativeTrigger({ workspace, serviceName, externalId })` instead of the standard `path`-based delete. -### Step 17: Update OpenAPI Spec and Regenerate Types +### Step 17: Update `getUsedTriggers` for Sidebar Visibility + +The sidebar (`frontend/src/lib/components/sidebar/SidebarContent.svelte`) shows native-trigger links only if `$usedTriggerKinds` includes the service — without this, your trigger page will never appear in the nav bar even when triggers exist. + +1. **Backend** — add `{service}_used: bool` to the `UsedTriggers` struct and SELECT in `backend/windmill-api-workspaces/src/workspaces.rs::get_used_triggers()`: + ```rust + EXISTS(SELECT 1 FROM native_trigger WHERE workspace_id = $1 AND service_name = '{service}'::native_trigger_service) AS "{service}_used!" + ``` +2. **OpenAPI** — add `{service}_used: boolean` to the response schema for `GET /w/{workspace}/workspaces/used_triggers` (under both `properties` and `required`). +3. **Layout** — in `frontend/src/routes/(root)/(logged)/+layout.svelte::loadUsedTriggerKinds()`, destructure `{service}_used` and push `'{service}'` to `usedKinds`. + +### Step 18: Update OpenAPI Spec and Regenerate Types Add to `JobTriggerKind` enum in `backend/windmill-api/openapi.yaml`, then: diff --git a/.agents/skills/pr/SKILL.md b/.agents/skills/pr/SKILL.md index 2efcc4e0a6..ef52d6e110 100644 --- a/.agents/skills/pr/SKILL.md +++ b/.agents/skills/pr/SKILL.md @@ -1,5 +1,6 @@ --- name: pr +user_invocable: true description: Open a draft pull request on GitHub. MUST use when you want to create/open a PR. --- @@ -50,22 +51,22 @@ The body MUST be explicit about what changed. Structure: ## Test plan - [ ] - [ ] - ---- -Generated with [Claude Code](https://claude.com/claude-code) ``` +The harness/tooling that invoked the skill may add its own attribution trailer; the skill itself does not prescribe one. + ## Execution Steps 1. Run `git status` to check for uncommitted changes 2. Run `git log main..HEAD --oneline` to see all commits in this branch 3. Run `git diff main...HEAD` to see the full diff against main -4. Check if remote branch exists and is up to date: +4. **Invoke the `local-review` skill** before creating the PR (`/local-review` in Claude Code, `$local-review` in Codex, `pi --skill local-review` / `/skill:local-review` in Pi). If issues are found, fix them and commit before proceeding. Do not skip this step. +5. Check if remote branch exists and is up to date: ```bash git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null || echo "no upstream" ``` -5. Push to remote if needed: `git push -u origin HEAD` -6. Create draft PR using gh CLI: +6. Push to remote if needed: `git push -u origin HEAD` +7. Create draft PR using gh CLI: ```bash gh pr create --draft --title ": " --body "$(cat <<'EOF' ## Summary @@ -78,13 +79,10 @@ Generated with [Claude Code](https://claude.com/claude-code) ## Test plan - [ ] - [ ] - - --- - Generated with [Claude Code](https://claude.com/claude-code) EOF )" ``` -7. Return the PR URL to the user +8. Return the PR URL to the user ## EE Companion PR (when `*_ee.rs` files were modified) @@ -100,9 +98,6 @@ Follow the full EE PR workflow in `docs/enterprise.md`. The key PR-specific deta ```bash gh pr create --draft --repo windmill-labs/windmill-ee-private --title ": " --body "$(cat <<'EOF' Companion PR for windmill-labs/windmill# - - --- - Generated with [Claude Code](https://claude.com/claude-code) EOF )" ``` diff --git a/.agents/skills/refine/SKILL.md b/.agents/skills/refine/SKILL.md index b96e97e8a2..aaf747cd29 100644 --- a/.agents/skills/refine/SKILL.md +++ b/.agents/skills/refine/SKILL.md @@ -1,5 +1,6 @@ --- name: refine +user_invocable: true description: End-of-session reflection. Reviews friction encountered during the session and proposes updates to docs/ to capture lessons learned. --- diff --git a/.claude/skills/adding-a-trigger/SKILL.md b/.claude/skills/adding-a-trigger/SKILL.md deleted file mode 100644 index 7d8643b862..0000000000 --- a/.claude/skills/adding-a-trigger/SKILL.md +++ /dev/null @@ -1,267 +0,0 @@ ---- -name: adding-a-trigger -description: Checklist for adding a new TriggerCrud-based trigger type to Windmill (Azure, GCP, Kafka, etc.). Use when wiring a new trigger kind across backend, frontend, CLI, and capture infrastructure. ---- - -# Skill: Adding a New Trigger Type - -Use this skill when adding a trigger kind that implements `TriggerCrud` (Kafka, GCP, Azure, MQTT, SQS, NATS, Postgres, Email…). For native triggers (Nextcloud, Google Drive — things wired through `windmill-native-triggers`), use the `native-trigger` skill instead. - -The goal of this doc is to enumerate every file that needs to change. Missing any one of them leads to silent regressions: sync drops the trigger, capture button does nothing, workspace forks lose it, sidebar counters undercount. Follow the checklist top-to-bottom — each section is independent enough to be validated on its own. - -Throughout this doc, substitute `{kind}` for the new trigger kind (`azure`, `kafka`, …), `{Kind}` for PascalCase (`Azure`, `Kafka`), `{KIND}` for SCREAMING (`AZURE`, `KAFKA`). - -## Reference implementations - -- **GCP** — closest analogue to Azure. Has push + pull, OIDC auth, ARM-like resource paths, capture handler. Grep for `gcp_trigger` / `GcpTrigger`. -- **Kafka** — simpler (pull-only, streaming). Good for trivial integrations. -- **Azure** — most recently added (2026). Shared-secret push auth, Event Grid namespaces + basic topics, ARM resource discovery, Namespace-pull data-plane. Grep for `azure_trigger` / `AzureTrigger`. - -## 1. Database migration - -Create a migration: `cargo sqlx migrate add -r add_{kind}_trigger` from `backend/`. Never write timestamps manually. - -The `up.sql` usually defines: -- An optional enum type (e.g. `AZURE_MODE`) if the trigger has sub-kinds -- The `{kind}_trigger` table with at minimum these columns (mirrored from kafka/gcp): - - primary: `(workspace_id, path)` - - `script_path`, `is_flow`, `enabled`, `mode`, `permissioned_as`, `edited_by`, `email` - - `edited_at`, `error`, `server_id`, `last_server_ping` - - `error_handler_path`, `error_handler_args jsonb`, `retry jsonb` - - trigger-specific fields -- Indexes on foreign keys + any frequently-filtered columns -- Foreign key to `workspace` - -Down migration drops the table and any enum types. - -## 2. Backend crate (`windmill-trigger-{kind}`) - -Create a new crate under `backend/windmill-trigger-{kind}/` with: - -- `Cargo.toml`: features `enterprise`, `private` if EE, standard deps -- `src/lib.rs`: `pub use mod_ee::*;` behind `#[cfg(all(feature = "enterprise", feature = "private"))]` -- `src/mod_ee.rs`: core types + helpers -- `src/handler_ee.rs`: `TriggerCrud` impl + route handlers -- `src/listener_ee.rs`: (only if streaming/pull-based) `Listener` trait impl - -Required in `mod_ee.rs`: -- `{Kind}Config` struct (persisted shape, `FromRow`) -- `{Kind}ConfigRequest` struct (what API receives — usually similar to Config but with validation fields) -- `{Kind}Trigger` unit struct (implements the traits) -- `impl TriggerJobArgs for {Kind}Trigger` — sets `TRIGGER_KIND`, `Payload`, `v1_payload_fn` - -Required in `handler_ee.rs`: -- `#[async_trait] impl TriggerCrud for {Kind}Trigger` with: - - `type Trigger = Trigger<{Kind}Config>` - - `type TriggerConfigRequest = {Kind}ConfigRequest` - - `const ROUTE_PREFIX: &'static str = "/{kind}_triggers";` - - `const TABLE_NAME`, `ADDITIONAL_SELECT_FIELDS` - - `get_deployed_object`, `validate_config`, `create_trigger`, `update_trigger`, `delete_trigger`, `test_connection` - - `additional_routes` (optional — mount extra endpoints for things like ARM resource listing, topic discovery) - -Register the crate in `backend/Cargo.toml` as a workspace member and as a dep of `windmill-api` behind the feature flag. - -## 3. Wire into `windmill-api` (feature-gated everywhere) - -**`backend/windmill-api/src/triggers/handler.rs`** — mount the trigger crate: -```rust -#[cfg(all(feature = "enterprise", feature = "{kind}_trigger", feature = "private"))] -{ - use crate::triggers::{kind}::{Kind}Trigger; - router = router.nest({Kind}Trigger::ROUTE_PREFIX, complete_trigger_routes({Kind}Trigger)); -} -``` - -**`backend/windmill-api/src/triggers/{kind}/mod.rs`** — re-export the crate: -```rust -pub use windmill_trigger_{kind}::*; -``` - -**`backend/windmill-api/src/lib.rs`** — if the trigger receives inbound pushes, add a webhook route: -```rust -.nest("/{kind}/w/{workspace_id}", { - #[cfg(all(feature = "enterprise", feature = "{kind}_trigger", feature = "private"))] - { triggers::{kind}::handler_oss::{kind}_push_route_handler() } - #[cfg(not(...))] - { Router::new() } -}) -``` - -## 4. `TriggerKind` enum (`backend/windmill-types/src/triggers.rs`) - -Already has slots for most triggers but verify your variant exists: -- Add `{Kind}` to the `TriggerKind` enum -- Add match arm in `to_key()` -- Add match arm in `from_str` -- Add match arm in `JobTriggerKind` (if jobs need kind tagging) - -## 5. OpenAPI (`backend/windmill-api/openapi.yaml`) - -This file is huge and the single most-forgotten place. Add: - -- `/w/{workspace}/{kind}_triggers/create` + `/update/{path}` + `/delete/{path}` + `/get/{path}` + `/list` + `/exists/{path}` + `/setmode/{path}` + `/test` paths (mirror gcp section) -- Any `additional_routes` your handler exposes (resource discovery, etc.) -- Schemas: `{Kind}Trigger`, `{Kind}TriggerData`, `{Kind}Mode` (if enum), `{Kind}DeliveryConfig`, helper request/response types -- Add `{kind}` to `CaptureTriggerKind` enum -- Add `{kind}_used: boolean` to the `UsedTriggers` response schema - -Regenerate frontend client: `npm run generate-backend-client` from `frontend/`. - -## 6. `UsedTriggers` + workspace export - -**`backend/windmill-api-workspaces/src/workspaces.rs`** — add `{kind}_used: bool` to the `UsedTriggers` struct and add an `EXISTS(SELECT 1 FROM {kind}_trigger …)` to the `get_used_triggers` query. - -**`backend/windmill-api/src/workspaces_export.rs`** — add export block mirroring gcp's (export lists all triggers, serializes them to YAML/JSON). The block re-uses the `trigger_ignore_keys` variable so the new kind automatically participates in fork-export stripping (`mode` field is omitted when the source workspace is a fork — keeps fork→parent merges from flipping the parent's enabled state). - -**Fork cloning (`clone_triggers_and_schedules` in workspaces.rs)** — add an `INSERT INTO {kind}_trigger ... SELECT ...` block that copies all rows from the parent workspace, forcing `mode = 'disabled'::TRIGGER_MODE`. Always runs at fork creation; forgetting this means users can't carry `{kind}` triggers into their forks. - -## 6.5 Hardcoded trigger-kind arrays (silent-failure hotspots) - -Several files keep **hardcoded arrays** of trigger kind strings. Miss one and ACL checks / user offboarding / trash drop your kind: - -- **`backend/windmill-api-groups/src/granular_acls.rs`** — `KINDS: [&str; N]`. **Increment N** (the compile error is cryptic otherwise). Controls which kinds accept granular ACL operations. -- **`backend/windmill-api-users/src/users.rs`** (`extra_perms_tables`) — which tables get `extra_perms` entries cleaned when a user is deleted. -- **`backend/windmill-api/src/offboarding.rs`** — three separate arrays (enumeration, fork-copy, and delete paths). **All three** need the new kind. -- **`backend/windmill-api/src/trash.rs`** — `valid_tables` for the trash / restore API. -- **`backend/windmill-git-sync/src/lib.rs`** — add a test assertion for `DeployedObject::{Kind}Trigger.get_kind() == "{kind}_trigger"` (the `get_kind` match arm itself lives in the enum impl — already required by the Rust compiler). -- **`backend/windmill-api-auth/src/scopes.rs`** — add the `{Kind}Triggers` variant to `ScopeDomain` enum + `as_str` match + `from_str` match. Required for the OAuth/token system to recognise `{kind}_triggers:read|write` scopes. -- **`backend/windmill-api/src/token.rs`** (`build_trigger_scope_domains` → `TRIGGER_DOMAINS`) — add `("{kind}_triggers", "{Kind display name}")` so the CreateToken UI's scope selector surfaces the `read` / `write` checkboxes. - -**OpenAPI enums** to extend (do NOT forget — generated client will allow it but server rejects as 400): -- `CaptureTriggerKind` enum -- Three `kind` enums under `/w/{workspace}/acls/{get,add,remove}/{kind}/{path}` (yes, same list repeated three times) - -After editing any of these, run a full `cargo check` with your feature flag + `gcp_trigger` + other core flags — the `KINDS: [&str; N]` length mismatch only surfaces when the crate compiles. - -## 7. Capture infrastructure (`backend/windmill-api/src/capture.rs`) - -If the trigger supports push delivery, it also needs a capture endpoint so users can test it: - -- `{Kind}TriggerConfig` struct (gated by feature flags) -- `TriggerConfig::{Kind}` variant -- `set_{kind}_trigger_config` function (creates the subscription/equivalent pointing at the capture URL — use your `manage_{kind}_subscription` helper with `trigger_mode=false`) -- Both real + no-op versions behind feature gates -- `TriggerKind::{Kind} => set_{kind}_trigger_config(...)` arm in `set_config` -- `{kind}_payload` async handler — validates auth (if any), processes payload, calls `insert_capture_payload` -- Route: `.route("/{kind}/{runnable_kind}/{*path}", post({kind}_payload))` inside `workspaced_unauthed_service` — and expand the surrounding `#[cfg(any(...))]` to include your feature flag - -## 8. CLI (`cli/`) — easy to miss, breaks sync silently - -Check all of these: - -**`cli/src/types.ts`:** -- Add `"{kind}"` to `TRIGGER_TYPES` array -- Add `"{kind}_trigger"` to `getTypeStrFromPath` return union -- Add match case in `getTypeStrFromPath`'s `typeEnding ===` chain -- Add `pushTrigger("{kind}", ...)` branch in `pushObj` - -**`cli/src/commands/trigger/trigger.ts`:** -- Import `{Kind}Trigger` type -- Add `{kind}: {Kind}Trigger` to the `Trigger` type map -- Add `{kind}: wmill.get{Kind}Trigger`, `update{Kind}Trigger`, `create{Kind}Trigger` to each function map -- Add `{kind}: { ... }` template to `triggerTemplates` -- Add `list{Kind}Triggers` call + spread in the `list` aggregation -- Update `--kind` option descriptions to mention the new kind - -**`cli/src/commands/sync/sync.ts`:** -- Add `path.endsWith(".{kind}_trigger" + ext)` in the file-type filter -- Add `typ == "{kind}_trigger"` in `getTypeOrder` -- Add `"{kind}_trigger"` to the delete-suffix regex (~line 3092) -- Add a `case "{kind}_trigger"` in the delete switch - -**`cli/src/guidance/skills.ts`** — **DO NOT EDIT DIRECTLY**. It's auto-generated by `system_prompts/generate.py`. Instead: -- Edit `system_prompts/utils.py` → append `('{Kind}Trigger', '{kind}_trigger')` to the `SCHEMA_MAPPINGS['triggers']` list (this is the master list — the one in `generate.py` is duplicated and `utils.py` wins) -- Then run `python3 system_prompts/generate.py` — it regenerates `cli/src/guidance/skills.ts` with the schema extracted from `backend/windmill-api/openapi.yaml` -- Commit the regenerated file - -## 9. Frontend — editor + drawer - -Under `frontend/src/lib/components/triggers/{kind}/`: - -- `{Kind}TriggerPanel.svelte` — the tile shown in the triggers listing -- `{Kind}TriggerEditor.svelte` — outer drawer wrapper -- `{Kind}TriggerEditorInner.svelte` — state + business logic; must expose: - - `openEdit(path, isFlow, defaultValues?)` method - - `isEditor` prop, `onConfigChange` + `onCaptureConfigChange` callbacks - - `get{Kind}Config()` + `get{Kind}CaptureConfig()` helpers - - `captureConfig = $derived.by(untrack(() => isEditor) ? get{Kind}CaptureConfig : () => ({}))` - - `$effect(() => { const args = [captureConfig, isValid] as const; untrack(() => onCaptureConfigChange?.(...args)) })` -- `{Kind}TriggerEditorConfigSection.svelte` — form fields; use design-system components (`TextInput`, `Select`, `Toggle`, `ToggleButtonGroup`), never raw `` -- `{Kind}Capture.svelte` — capture panel; wraps `CaptureSection` with `captureType="{kind}"` -- `utils.ts` — `requestBody` builders and any trigger-type-specific helpers - -## 10. Frontend — global integration - -Easy to miss: - -- **`frontend/src/lib/components/triggers.ts`** — add `'{kind}'` to the `TriggerKind` union -- **`frontend/src/lib/components/triggers/CaptureWrapper.svelte`**: - - Import `{Kind}Capture` - - Add to `isStreamingCapture()` array (streaming = pull-style; push-style is typically `false`) - - Add `{:else if captureType === '{kind}'}` branch with the `<{Kind}Capture>` render -- **`frontend/src/lib/components/sidebar/SidebarContent.svelte`** — import the icon, add the nav entry -- **`frontend/src/lib/components/sidebar/OperatorMenu.svelte`** — add the operator-mode entry -- **`frontend/src/routes/(root)/(logged)/+layout.svelte`** — destructure `{kind}_used` from `/get_used_triggers` response, push `'{kind}'` into `usedKinds` -- **`frontend/src/lib/components/search/GlobalSearchModal.svelte`** — import icon, add "Go to {Kind} ..." entry -- **`frontend/src/lib/components/offboarding-utils.ts`** — add mappings `{kind}_trigger: '{kind}_triggers'` and `{kind}_trigger: '{kind} trigger'` -- **`frontend/src/lib/components/icons/{Kind}Icon.svelte`** — single-path SVG, `fill={color ?? 'currentColor'}`, `size` prop default 16 (match existing icons — don't hardcode colors, don't use `width`/`height` props) -- **`frontend/src/routes/(root)/(logged)/{kind}_triggers/+page.svelte`** — listing page (mirror `gcp_triggers/+page.svelte` for push+pull, `kafka_triggers` for pure streaming) -- **`frontend/src/lib/components/CompareWorkspaces.svelte`** — workspace fork / compare tool. Needs: service import, editor import, `{kind}Editor` `$state`, `case '{kind}'` in `openTriggerDetails()`, entry in `triggerServices` object (list/delete/normalize), and `<{Kind}TriggerEditor bind:this={{kind}Editor} />` in the template - -## 10.5 AI system prompts (`system_prompts/`) - -- **`system_prompts/utils.py`** — append `('{Kind}Trigger', '{kind}_trigger')` to `SCHEMA_MAPPINGS['triggers']` (master list used by code generation + CLI skills) -- **`system_prompts/generate.py`** — also has a duplicated `schema_types` list (~line 903) for the AI `triggers` skill content. Add `('{Kind}Trigger', '{kind}_trigger')` there too -- **`system_prompts/generate.py`** `schema_names` (~line 1192) — add `'{Kind}Trigger'` (add `'New{Kind}Trigger'` only if the OpenAPI declares one; GCP and Azure don't) -- Run `python3 system_prompts/generate.py` — this rewrites `cli/src/guidance/skills.ts` and all `auto-generated/` docs. Commit the regenerated files - -## 11. Validation - -Run all of these before declaring done: - -```bash -# Backend -cd backend -cargo check --features enterprise,{kind}_trigger,private # minimal -cargo check --features enterprise,azure_trigger,private,gcp_trigger,http_trigger,mqtt_trigger,postgres_trigger,sqs_trigger,kafka,nats,smtp,websocket # full - -# SQLx offline data (never run `cargo sqlx prepare` directly — use the wrapper) -./update_sqlx.sh - -# Frontend -cd frontend -npm run generate-backend-client -npm run check:fast -``` - -Smoke test in the UI: create a trigger, save, check it appears in sidebar + search, delete, re-create via CLI `wmill sync`. - -## 12. Common pitfalls - -- **Forgetting feature gates in `workspaced_unauthed_service()`** — the surrounding `#[cfg(any(...))]` expression must include your feature flag, not just the inner `#[cfg]` on the route -- **`.route(path, ...).route(path, ...)` with same path and different methods** — older axum replaced; use `.route(path, post(h1).options(h2))` to chain methods on the same `MethodRouter` -- **`on:event` directives** — legacy Svelte 4, no-op in runes mode. Use callback props (`onSelected`, `onConfigChange`) -- **`$bindable(default_value)` on optional props** — banned by project CLAUDE.md. Use `$bindable()` + `$derived(prop ?? default)` instead -- **CORS layer intercepting OPTIONS** — tower-http CorsLayer short-circuits OPTIONS before reaching your handler. For server-to-server webhook endpoints, drop the CORS layer entirely (CORS is browser-only) -- **DeliveryAttributeMappings / custom headers for auth** — prefer HMAC or sha256-hashed shared secrets over opaque JWTs when the provider doesn't support signed tokens natively. Store only the hash; regenerate secret on every save -- **ARM / API resource-listing cascades** — if the trigger's resource type is deep (Azure: subscription → RG → namespace → topic), offer dropdowns in the UI populated from the provider's APIs using the user's credential resource -- **Clearing stale selections on dependency change** — when a dropdown's underlying data reloads (e.g., user changes SP or edition), clear selections that no longer match the new list -- **Workspace-scoped tag compatibility** — if the trigger has tags, verify forked workspaces handle them (see commit `0773b5bc85` for a historical fix) - -## 13. EE file split - -If the trigger is enterprise-only, the code lives in `windmill-ee-private__worktrees/.../windmill-trigger-{kind}/src/*_ee.rs` and is symlinked into the OSS tree. The `windmill-ee-private__worktrees/` directory holds the real files; changes propagate via symlinks. See `docs/enterprise.md` for the workflow. - -## 14. Final checklist before PR - -- [ ] Migration up/down tested (revert + re-apply) -- [ ] `./update_sqlx.sh` committed the updated `.sqlx/` offline data -- [ ] `cargo check` passes with your feature flag + with all trigger features -- [ ] `npm run check:fast` passes -- [ ] Trigger visible in sidebar with correct icon weight (not oversized/colored — use `currentColor`) -- [ ] Create, edit, delete flow all work in the UI -- [ ] Capture button works (if push-capable) -- [ ] Trigger appears in `/get_used_triggers` → sidebar pulse -- [ ] `wmill sync pull` + `wmill sync push` both round-trip the trigger -- [ ] `wmill trigger list` includes it -- [ ] OpenAPI schemas are complete (no `null` in generated types) diff --git a/.claude/skills/adding-a-trigger/SKILL.md b/.claude/skills/adding-a-trigger/SKILL.md new file mode 120000 index 0000000000..a2060ad897 --- /dev/null +++ b/.claude/skills/adding-a-trigger/SKILL.md @@ -0,0 +1 @@ +../../../.agents/skills/adding-a-trigger/SKILL.md \ No newline at end of file diff --git a/.claude/skills/commit/SKILL.md b/.claude/skills/commit/SKILL.md deleted file mode 100644 index 2094dbab06..0000000000 --- a/.claude/skills/commit/SKILL.md +++ /dev/null @@ -1,60 +0,0 @@ ---- -name: commit -user_invocable: true -description: Create a git commit with conventional commit format. MUST use anytime you want to commit changes. ---- - -# Git Commit Skill - -Create a focused, single-line commit following conventional commit conventions. - -## Instructions - -1. **Analyze changes**: Run `git status` and `git diff` to understand what was modified -2. **Stage only modified files**: Add files individually by name. NEVER use `git add -A` or `git add .` -3. **Write commit message**: Follow the conventional commit format as a single line - -## Conventional Commit Format - -``` -: -``` - -### Types -- `feat`: New feature or capability -- `fix`: Bug fix -- `refactor`: Code change that neither fixes a bug nor adds a feature -- `docs`: Documentation only changes -- `style`: Formatting, missing semicolons, etc (no code change) -- `test`: Adding or correcting tests -- `chore`: Maintenance tasks, dependency updates, etc -- `perf`: Performance improvement - -### Rules -- Message MUST be a single line (no multi-line messages) -- Description should be lowercase, imperative mood ("add" not "added") -- No period at the end -- Keep under 72 characters total - -### Examples -``` -feat: add token usage tracking for AI providers -fix: resolve null pointer in job executor -refactor: extract common validation logic -docs: update API endpoint documentation -chore: upgrade sqlx to 0.7 -``` - -## Execution Steps - -1. Run `git status` to see all changes -2. Run `git diff` to understand the changes in detail -3. Run `git log --oneline -5` to see recent commit style -4. Stage ONLY the modified/relevant files: `git add ...` -5. Create the commit with conventional format: - ```bash - git commit -m ": - - Co-Authored-By: Claude Opus 4.5 " - ``` -6. Run `git status` to verify the commit succeeded diff --git a/.claude/skills/commit/SKILL.md b/.claude/skills/commit/SKILL.md new file mode 120000 index 0000000000..11493a3d1e --- /dev/null +++ b/.claude/skills/commit/SKILL.md @@ -0,0 +1 @@ +../../../.agents/skills/commit/SKILL.md \ No newline at end of file diff --git a/.claude/skills/local-review/SKILL.md b/.claude/skills/local-review/SKILL.md deleted file mode 100644 index 58a9d473e2..0000000000 --- a/.claude/skills/local-review/SKILL.md +++ /dev/null @@ -1,71 +0,0 @@ ---- -name: local-review -user_invocable: true -description: Code review a pull request for bugs and CLAUDE.md compliance. MUST use when asked to review code. ---- - -# Local Code Review Skill - -Run the same review locally that the GitHub Claude Auto Review action runs on PRs. The review policy lives in `.github/review-prompt-shared.md` (severity triage, public-surface checklist, `AGENTS.md` compliance, test-coverage assessment); `.claude/review-prompt.md` holds Claude-specific output preferences. Read both before reviewing. - -## Execution Steps - -1. **Read `.github/review-prompt-shared.md`** for the review policy and `.claude/review-prompt.md` for the Claude output format - -2. **Determine the PR scope**: - - If an argument is provided, use it as the PR number or branch - - Otherwise, detect from the current branch vs main - - Run `gh pr view` if a PR exists, or use `git diff main...HEAD` - -3. **Get the diff and metadata**: - - `gh pr diff` or `git diff main...HEAD` for the full diff - - `gh pr view` or `git log main..HEAD --oneline` for context - -4. **Read changed files** where the diff alone is insufficient to understand context - -5. **Apply the review policy from `.github/review-prompt-shared.md`** (and the output format from `.claude/review-prompt.md`) - -6. **Self-validate each finding**: Before reporting, ask yourself: - - "Is this definitely a real issue, not a false positive?" - - "Would a senior engineer flag this in review?" - - If the answer to either is no, discard the finding - -7. **Output findings** to the terminal (default) or post as PR comments (with `--comment` flag) - -## Output Format - -``` -## Code review - -Found N issues: - -1. [P0|P1|P2] - - -2. [P0|P1|P2] - -``` - -End with a Test coverage section per `.github/review-prompt-shared.md`. - -If no issues are found: - -``` -## Code review - -No issues found. Checked for bugs, security, and AGENTS.md compliance. -``` - -## Posting Comments (--comment flag) - -If the user passes `--comment`, post findings as inline PR comments using: - -```bash -gh pr review --comment --body "" -``` - -Or for inline comments on specific lines: - -```bash -gh api repos/{owner}/{repo}/pulls/{pr}/reviews -f body="" -f event="COMMENT" -f comments="[...]" -``` diff --git a/.claude/skills/local-review/SKILL.md b/.claude/skills/local-review/SKILL.md new file mode 120000 index 0000000000..8072aff10d --- /dev/null +++ b/.claude/skills/local-review/SKILL.md @@ -0,0 +1 @@ +../../../.agents/skills/local-review/SKILL.md \ No newline at end of file diff --git a/.claude/skills/native-trigger/SKILL.md b/.claude/skills/native-trigger/SKILL.md deleted file mode 100644 index ae38f70b32..0000000000 --- a/.claude/skills/native-trigger/SKILL.md +++ /dev/null @@ -1,793 +0,0 @@ ---- -name: native-trigger -description: Guidance for adding native trigger services to Windmill. Use when implementing or modifying native trigger integrations across the backend and frontend. ---- - -# Skill: Adding Native Trigger Services - -This skill provides comprehensive guidance for adding new native trigger services to Windmill. Native triggers allow external services (like Nextcloud, Google Drive, etc.) to trigger Windmill scripts/flows via webhooks or push notifications. - -## Architecture Overview - -The native trigger system consists of: - -1. **Database Layer** - PostgreSQL tables and enum types -2. **Backend Rust Implementation** - Core trait, handlers, and service modules in the `windmill-native-triggers` crate -3. **Frontend Svelte Components** - Configuration forms and UI components - -### Key Files - -| Component | Path | -|-----------|------| -| Core module with `External` trait | `backend/windmill-native-triggers/src/lib.rs` | -| Generic CRUD handlers | `backend/windmill-native-triggers/src/handler.rs` | -| Background sync logic | `backend/windmill-native-triggers/src/sync.rs` | -| OAuth/workspace integration | `backend/windmill-native-triggers/src/workspace_integrations.rs` | -| Re-export shim (windmill-api) | `backend/windmill-api/src/native_triggers/mod.rs` | -| TriggerKind enum | `backend/windmill-common/src/triggers.rs` | -| JobTriggerKind enum | `backend/windmill-common/src/jobs.rs` | -| Frontend service registry | `frontend/src/lib/components/triggers/native/utils.ts` | -| Frontend trigger utilities | `frontend/src/lib/components/triggers/utils.ts` | -| Trigger badges (icons + counts) | `frontend/src/lib/components/graph/renderers/triggers/TriggersBadge.svelte` | -| Workspace integrations UI | `frontend/src/lib/components/workspaceSettings/WorkspaceIntegrations.svelte` | -| OAuth config form component | `frontend/src/lib/components/workspaceSettings/OAuthClientConfig.svelte` | -| OpenAPI spec | `backend/windmill-api/openapi.yaml` | -| Reference: Nextcloud module | `backend/windmill-native-triggers/src/nextcloud/` | -| Reference: Google module | `backend/windmill-native-triggers/src/google/` | - -### Crate Structure - -The native trigger code lives in the `windmill-native-triggers` crate (`backend/windmill-native-triggers/`). The `windmill-api` crate re-exports everything via a shim: - -```rust -// backend/windmill-api/src/native_triggers/mod.rs -pub use windmill_native_triggers::*; -``` - -All new service modules go in `backend/windmill-native-triggers/src/`. - ---- - -## Core Concepts - -### The `External` Trait - -Every native trigger service implements the `External` trait defined in `lib.rs`: - -```rust -#[async_trait] -pub trait External: Send + Sync + 'static { - // Associated types: - type ServiceConfig: Debug + DeserializeOwned + Serialize + Send + Sync; - type TriggerData: Debug + Serialize + Send + Sync; - type OAuthData: DeserializeOwned + Serialize + Clone + Send + Sync; - type CreateResponse: DeserializeOwned + Send + Sync; - - // Constants: - const SUPPORT_WEBHOOK: bool; - const SERVICE_NAME: ServiceName; - const DISPLAY_NAME: &'static str; - const TOKEN_ENDPOINT: &'static str; - const REFRESH_ENDPOINT: &'static str; - const AUTH_ENDPOINT: &'static str; - - // Required methods: - async fn create(&self, w_id, oauth_data, webhook_token, data, db, tx) -> Result; - async fn update(&self, w_id, oauth_data, external_id, webhook_token, data, db, tx) -> Result; - async fn get(&self, w_id, oauth_data, external_id, db, tx) -> Result; - async fn delete(&self, w_id, oauth_data, external_id, db, tx) -> Result<()>; - async fn exists(&self, w_id, oauth_data, external_id, db, tx) -> Result; - async fn maintain_triggers(&self, db, workspace_id, triggers, oauth_data, synced, errors); - fn external_id_and_metadata_from_response(&self, resp) -> (String, Option); - - // Methods with defaults: - async fn prepare_webhook(&self, db, w_id, headers, body, script_path, is_flow) -> Result; - fn service_config_from_create_response(&self, data, resp) -> Option; - fn additional_routes(&self) -> axum::Router; - async fn http_client_request(&self, url, method, workspace_id, tx, db, headers, body) -> Result; -} -``` - -Key design points: -- **`update()` returns `serde_json::Value`** - the resolved service_config to store. Each service is responsible for building the final config. -- **`maintain_triggers()`** - periodic background maintenance. Each service implements its own strategy (Nextcloud: reconcile with external state; Google: renew expiring channels). -- **No `list_all()` in the trait** - services that need it (Nextcloud) implement it privately; services that don't (Google) use different maintenance strategies. -- **No `get_external_id_from_trigger_data()` or `extract_service_config_from_trigger_data()`** - removed in favor of the `maintain_triggers` pattern. - -### Create Lifecycle: Two Paths - -The `create_native_trigger` handler in `handler.rs` supports two creation flows, controlled by `service_config_from_create_response()`: - -**Path A: Short (Google pattern)** - `service_config_from_create_response()` returns `Some(config)`: -1. `create()` registers on external service -2. `external_id_and_metadata_from_response()` extracts the ID -3. `service_config_from_create_response()` builds the config directly from input data + response metadata -4. Stores trigger in DB -- done, no extra round-trip - -Use this when the external_id is known before the create call (e.g., Google generates the channel_id as a UUID upfront and includes it in the webhook URL). - -**Path B: Long (Nextcloud pattern)** - `service_config_from_create_response()` returns `None` (default): -1. `create()` registers on external service (webhook URL has no external_id yet) -2. `external_id_and_metadata_from_response()` extracts the ID -3. `update()` is called to fix the webhook URL with the now-known external_id -4. `update()` returns the resolved service_config -5. Stores trigger in DB - -Use this when the external_id is assigned by the remote service and the webhook URL needs to be corrected after creation. - -### OAuth Token Storage (Three-Table Pattern) - -OAuth tokens are stored across three tables, NOT in `workspace_integrations.oauth_data` directly: - -| Table | What's Stored | -|-------|---------------| -| `workspace_integrations` | `oauth_data` JSON with `base_url`, `client_id`, `client_secret`, `instance_shared` flag; `resource_path` pointing to the variable | -| `variable` | Encrypted `access_token` (at the path stored in `resource_path`), linked to `account` via `account` column | -| `account` | `refresh_token`, keyed by `workspace_id` + `client` (service name) + `is_workspace_integration = true` | - -The `decrypt_oauth_data()` function in `lib.rs` assembles these into a unified struct: -```rust -pub struct OAuthConfig { - pub base_url: String, - pub access_token: String, // decrypted from variable - pub refresh_token: Option, // from account table - pub client_id: String, // from oauth_data or instance settings - pub client_secret: String, // from oauth_data or instance settings -} -``` - -Instance-level sharing: when `oauth_data.instance_shared == true`, `client_id` and `client_secret` are read from global settings instead of workspace_integrations. - -### URL Resolution - -The `resolve_endpoint()` helper handles both absolute and relative OAuth URLs: - -```rust -pub fn resolve_endpoint(base_url: &str, endpoint: &str) -> String { - if endpoint.starts_with("http://") || endpoint.starts_with("https://") { - endpoint.to_string() // Google: absolute URLs - } else { - format!("{}{}", base_url, endpoint) // Nextcloud: relative paths - } -} -``` - -### ServiceName Methods - -`ServiceName` is the central registry enum. Each variant must implement these match arms: - -| Method | Purpose | -|--------|---------| -| `as_str()` | Lowercase identifier (e.g., `"google"`) | -| `as_trigger_kind()` | Maps to `TriggerKind` enum | -| `as_job_trigger_kind()` | Maps to `JobTriggerKind` enum | -| `token_endpoint()` | OAuth token endpoint (relative or absolute) | -| `auth_endpoint()` | OAuth authorization endpoint | -| `oauth_scopes()` | Space-separated OAuth scopes | -| `resource_type()` | Resource type for token storage (e.g., `"gworkspace"`) | -| `extra_auth_params()` | Extra OAuth params (e.g., Google needs `access_type=offline`, `prompt=consent`) | -| `integration_service()` | Maps to the workspace integration service (usually `*self`) | -| `TryFrom` | Parse from string | -| `Display` | Delegates to `as_str()` | - ---- - -## Step-by-Step Implementation Guide - -### Step 1: Database Migration - -Create a new migration file: `backend/migrations/YYYYMMDDHHMMSS_newservice_trigger.up.sql` - -```sql --- Add the service to the native_trigger_service enum -ALTER TYPE native_trigger_service ADD VALUE IF NOT EXISTS 'newservice'; - --- Add to TRIGGER_KIND enum (used for trigger tracking) -ALTER TYPE TRIGGER_KIND ADD VALUE IF NOT EXISTS 'newservice'; - --- Add to job_trigger_kind enum (used for job tracking) -ALTER TYPE job_trigger_kind ADD VALUE IF NOT EXISTS 'newservice'; -``` - -Also create the corresponding down migration. - -### Step 2: Update windmill-common Enums - -#### `backend/windmill-common/src/triggers.rs` - -Add variant to `TriggerKind` enum, and update `to_key()` and `fmt()` implementations. - -#### `backend/windmill-common/src/jobs.rs` - -Add variant to `JobTriggerKind` enum and update the `Display` implementation. - -### Step 3: Backend Service Module - -Create a new directory: `backend/windmill-native-triggers/src/newservice/` - -#### `mod.rs` - Type Definitions - -```rust -use serde::{Deserialize, Serialize}; - -pub mod external; -// pub mod routes; // Only if you need additional service-specific routes - -/// OAuth data deserialized from the three-table pattern. -/// The actual structure is built by decrypt_oauth_data() from variable + account + workspace_integrations. -#[derive(Debug, Clone, Deserialize, Serialize)] -pub struct NewServiceOAuthData { - pub base_url: String, // from workspace_integrations.oauth_data - pub access_token: String, // decrypted from variable table - pub refresh_token: Option, // from account table - // Note: client_id and client_secret are in OAuthConfig, not here - // unless the service needs them at runtime for API calls -} - -/// Configuration provided by user when creating/updating a trigger. -/// Stored as JSON in native_trigger.service_config. -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct NewServiceConfig { - // Service-specific configuration fields - pub folder_path: String, - pub file_filter: Option, -} - -/// Data retrieved from the external service about a trigger. -/// Returned by the get() method and shown in the UI. -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct NewServiceTriggerData { - pub folder_path: String, - pub file_filter: Option, - // Fields that shouldn't affect service_config comparison should use #[serde(skip_serializing)] -} - -/// Response from external service when creating a trigger/webhook. -#[derive(Debug, Deserialize)] -pub struct CreateTriggerResponse { - pub id: String, -} - -/// Handler struct (stateless, used for routing) -#[derive(Copy, Clone)] -pub struct NewService; -``` - -#### `external.rs` - External Trait Implementation - -```rust -use async_trait::async_trait; -use reqwest::Method; -use sqlx::PgConnection; -use std::collections::HashMap; -use windmill_common::{ - error::{Error, Result}, - BASE_URL, DB, -}; - -use crate::{ - generate_webhook_service_url, External, NativeTrigger, NativeTriggerData, ServiceName, - sync::{SyncError, TriggerSyncInfo}, -}; -use super::{NewService, NewServiceConfig, NewServiceOAuthData, NewServiceTriggerData, CreateTriggerResponse}; - -#[async_trait] -impl External for NewService { - type ServiceConfig = NewServiceConfig; - type TriggerData = NewServiceTriggerData; - type OAuthData = NewServiceOAuthData; - type CreateResponse = CreateTriggerResponse; - - const SERVICE_NAME: ServiceName = ServiceName::NewService; - const DISPLAY_NAME: &'static str = "New Service"; - const SUPPORT_WEBHOOK: bool = true; - const TOKEN_ENDPOINT: &'static str = "/oauth/token"; - const REFRESH_ENDPOINT: &'static str = "/oauth/token"; - const AUTH_ENDPOINT: &'static str = "/oauth/authorize"; - - async fn create( - &self, - w_id: &str, - oauth_data: &Self::OAuthData, - webhook_token: &str, - data: &NativeTriggerData, - db: &DB, - tx: &mut PgConnection, - ) -> Result { - let base_url = &*BASE_URL.read().await; - - // external_id is None during create (we get it from the response) - let webhook_url = generate_webhook_service_url( - base_url, w_id, &data.script_path, data.is_flow, - None, Self::SERVICE_NAME, webhook_token, - ); - - let url = format!("{}/api/webhooks/create", oauth_data.base_url); - let payload = serde_json::json!({ - "callback_url": webhook_url, - "folder_path": data.service_config.folder_path, - }); - - let response: CreateTriggerResponse = self - .http_client_request(&url, Method::POST, w_id, tx, db, None, Some(&payload)) - .await?; - - Ok(response) - } - - /// Update returns the resolved service_config as JSON. - /// For services using the update+get pattern, call self.get() and serialize. - async fn update( - &self, - w_id: &str, - oauth_data: &Self::OAuthData, - external_id: &str, - webhook_token: &str, - data: &NativeTriggerData, - db: &DB, - tx: &mut PgConnection, - ) -> Result { - let base_url = &*BASE_URL.read().await; - - let webhook_url = generate_webhook_service_url( - base_url, w_id, &data.script_path, data.is_flow, - Some(external_id), Self::SERVICE_NAME, webhook_token, - ); - - let url = format!("{}/api/webhooks/{}", oauth_data.base_url, external_id); - let payload = serde_json::json!({ - "callback_url": webhook_url, - "folder_path": data.service_config.folder_path, - }); - - let _: serde_json::Value = self - .http_client_request(&url, Method::PUT, w_id, tx, db, None, Some(&payload)) - .await?; - - // Fetch back the updated state to get the resolved config - let trigger_data = self.get(w_id, oauth_data, external_id, db, tx).await?; - serde_json::to_value(&trigger_data) - .map_err(|e| Error::InternalErr(format!("Failed to serialize trigger data: {}", e))) - } - - async fn get( - &self, - w_id: &str, - oauth_data: &Self::OAuthData, - external_id: &str, - db: &DB, - tx: &mut PgConnection, - ) -> Result { - let url = format!("{}/api/webhooks/{}", oauth_data.base_url, external_id); - self.http_client_request::<_, ()>(&url, Method::GET, w_id, tx, db, None, None).await - } - - async fn delete( - &self, - w_id: &str, - oauth_data: &Self::OAuthData, - external_id: &str, - db: &DB, - tx: &mut PgConnection, - ) -> Result<()> { - let url = format!("{}/api/webhooks/{}", oauth_data.base_url, external_id); - let _: serde_json::Value = self - .http_client_request::<_, ()>(&url, Method::DELETE, w_id, tx, db, None, None) - .await - .or_else(|e| match &e { - Error::InternalErr(msg) if msg.contains("404") => Ok(serde_json::Value::Null), - _ => Err(e), - })?; - Ok(()) - } - - async fn exists( - &self, - w_id: &str, - oauth_data: &Self::OAuthData, - external_id: &str, - db: &DB, - tx: &mut PgConnection, - ) -> Result { - match self.get(w_id, oauth_data, external_id, db, tx).await { - Ok(_) => Ok(true), - Err(Error::NotFound(_)) => Ok(false), - Err(e) => Err(e), - } - } - - /// Background maintenance. Choose the right pattern for your service: - /// - For services with queryable external state: use reconcile_with_external_state() - /// - For channel-based services with expiration: implement renewal logic - async fn maintain_triggers( - &self, - db: &DB, - workspace_id: &str, - triggers: &[NativeTrigger], - oauth_data: &Self::OAuthData, - synced: &mut Vec, - errors: &mut Vec, - ) { - // Option A: Reconcile with external state (Nextcloud pattern) - // Fetch all triggers from external service and compare with DB - let external_triggers = match self.list_all(workspace_id, oauth_data, db).await { - Ok(triggers) => triggers, - Err(e) => { - errors.push(SyncError { - resource_path: format!("workspace:{}", workspace_id), - error_message: format!("Failed to list triggers: {}", e), - error_type: "api_error".to_string(), - }); - return; - } - }; - - // Convert to (external_id, config_json) pairs - let external_pairs: Vec<(String, serde_json::Value)> = external_triggers - .into_iter() - .map(|t| (t.id.clone(), serde_json::to_value(&t).unwrap_or_default())) - .collect(); - - crate::sync::reconcile_with_external_state( - db, workspace_id, Self::SERVICE_NAME, triggers, &external_pairs, synced, errors, - ).await; - } - - fn external_id_and_metadata_from_response( - &self, - resp: &Self::CreateResponse, - ) -> (String, Option) { - (resp.id.clone(), None) - } - - // service_config_from_create_response: NOT overridden (returns None). - // This means the handler uses the update+get pattern after create. - // Override and return Some(...) to skip the update+get cycle (Google pattern). -} - -impl NewService { - /// Private helper to list all triggers from the external service. - async fn list_all( - &self, - w_id: &str, - oauth_data: &::OAuthData, - db: &DB, - ) -> Result::TriggerData>> { - // Implementation depends on the external service's API - todo!() - } -} -``` - -### Step 4: Update lib.rs Registry - -In `backend/windmill-native-triggers/src/lib.rs`: - -```rust -// Service modules - add new services here: -#[cfg(feature = "native_trigger")] -pub mod newservice; // <-- Add this - -// ServiceName enum - add variant: -pub enum ServiceName { - Nextcloud, - Google, - NewService, // <-- Add this -} - -// Then add match arms in ALL ServiceName methods: -// as_str(), as_trigger_kind(), as_job_trigger_kind(), token_endpoint(), -// auth_endpoint(), oauth_scopes(), resource_type(), extra_auth_params(), -// integration_service(), TryFrom, Display -``` - -### Step 5: Update handler.rs Routes - -In `backend/windmill-native-triggers/src/handler.rs`: - -```rust -pub fn generate_native_trigger_routers() -> Router { - // ... - #[cfg(feature = "native_trigger")] - { - use crate::newservice::NewService; - return router - .nest("/nextcloud", service_routes(NextCloud)) - .nest("/google", service_routes(Google)) - .nest("/newservice", service_routes(NewService)); // <-- Add this - } - // ... -} -``` - -### Step 6: Update sync.rs - -In `backend/windmill-native-triggers/src/sync.rs`: - -```rust -pub async fn sync_all_triggers(db: &DB) -> Result { - // ... - #[cfg(feature = "native_trigger")] - { - use crate::newservice::NewService; - - // ... existing service syncs ... - - // New service sync - let (service_name, result) = sync_service_triggers(db, NewService).await; - total_synced += result.synced_triggers.len(); - total_errors += result.errors.len(); - service_results.insert(service_name, result); - } - // ... -} -``` - -### Step 7: Frontend Service Registry - -In `frontend/src/lib/components/triggers/native/utils.ts`: - -Add to `NATIVE_TRIGGER_SERVICES`, `getTriggerIconName()`, and `getServiceIcon()`. - -### Step 8: Frontend Trigger Form Component - -Create: `frontend/src/lib/components/triggers/native/services/newservice/NewServiceTriggerForm.svelte` - -### Step 9: Frontend Icon Component - -Create: `frontend/src/lib/components/icons/NewServiceIcon.svelte` - -### Step 10: Update NativeTriggerEditor - -Check `frontend/src/lib/components/triggers/native/NativeTriggerEditor.svelte` to ensure it dynamically loads form components based on service name. - -### Step 11: Workspace Integration UI - -Add your service to the `supportedServices` map in `frontend/src/lib/components/workspaceSettings/WorkspaceIntegrations.svelte`: - -```typescript -const supportedServices: Record = { - // ... existing services ... - newservice: { - name: 'newservice', - displayName: 'New Service', - description: 'Connect to New Service for triggers', - icon: NewServiceIcon, - docsUrl: 'https://www.windmill.dev/docs/integrations/newservice', - requiresBaseUrl: false, // false for cloud services, true for self-hosted - setupInstructions: [ - 'Step 1: Create an OAuth app on the service', - 'Step 2: Configure the redirect URI shown below', - 'Step 3: Enter the client credentials below' - ] - } -} -``` - -### Step 12: Update `frontend/src/lib/components/triggers/utils.ts` - -Update ALL of these maps/functions: -1. `triggerIconMap` - import and add icon -2. `triggerDisplayNamesMap` - add display name -3. `triggerTypeOrder` in `sortTriggers()` - add type -4. `getLightConfig()` - add case for your service -5. `getTriggerLabel()` - add case for your service -6. `jobTriggerKinds` - add to array -7. `countPropertyMap` - add count property -8. `triggerSaveFunctions` - add save function - -### Step 13: Update TriggersBadge Component - -In `frontend/src/lib/components/graph/renderers/triggers/TriggersBadge.svelte`: - -1. Import the icon -2. Add to `baseConfig` with `countKey` (the dynamic `availableNativeServices` loop does NOT set `countKey`) -3. Add to the `allTypes` array - -### Step 14: Update TriggersWrapper.svelte - -In `frontend/src/lib/components/triggers/TriggersWrapper.svelte`: - -Add a `{:else if selectedTrigger.type === 'yourservice'}` case that renders `` with the same props pattern as the existing native trigger cases (e.g., `nextcloud`). - -### Step 15: Update AddTriggersButton.svelte - -In `frontend/src/lib/components/triggers/AddTriggersButton.svelte`: - -1. Add `yourserviceAvailable` state variable -2. Add `setYourserviceState()` async function using `isServiceAvailable('yourservice', $workspaceStore!)` -3. Call it at module level -4. Add a dropdown entry to `addTriggerItems` with `hidden: !yourserviceAvailable` - -### Step 16: Update TriggersEditor.svelte Delete Handling - -In `frontend/src/lib/components/triggers/TriggersEditor.svelte`: - -Add your service to the `nativeTriggerServices` map in `deleteDeployedTrigger()`. Native triggers use `NativeTriggerService.deleteNativeTrigger({ workspace, serviceName, externalId })` instead of the standard `path`-based delete. - -### Step 17: Update `getUsedTriggers` for Sidebar Visibility - -The sidebar (`frontend/src/lib/components/sidebar/SidebarContent.svelte`) shows native-trigger links only if `$usedTriggerKinds` includes the service — without this, your trigger page will never appear in the nav bar even when triggers exist. - -1. **Backend** — add `{service}_used: bool` to the `UsedTriggers` struct and SELECT in `backend/windmill-api-workspaces/src/workspaces.rs::get_used_triggers()`: - ```rust - EXISTS(SELECT 1 FROM native_trigger WHERE workspace_id = $1 AND service_name = '{service}'::native_trigger_service) AS "{service}_used!" - ``` -2. **OpenAPI** — add `{service}_used: boolean` to the response schema for `GET /w/{workspace}/workspaces/used_triggers` (under both `properties` and `required`). -3. **Layout** — in `frontend/src/routes/(root)/(logged)/+layout.svelte::loadUsedTriggerKinds()`, destructure `{service}_used` and push `'{service}'` to `usedKinds`. - -### Step 18: Update OpenAPI Spec and Regenerate Types - -Add to `JobTriggerKind` enum in `backend/windmill-api/openapi.yaml`, then: - -```bash -cd frontend && npm run generate-backend-client -``` - ---- - -## Special Patterns - -### Unified Service with `trigger_type` (Google Pattern) - -When a single service handles multiple trigger types (e.g., Google Drive + Calendar share OAuth and API patterns), use a single `ServiceName` variant with a discriminator field: - -```rust -pub enum GoogleTriggerType { Drive, Calendar } - -pub struct GoogleServiceConfig { - pub trigger_type: GoogleTriggerType, - // Drive-specific fields (only used when trigger_type = Drive) - pub resource_id: Option, - pub resource_name: Option, - // Calendar-specific fields (only used when trigger_type = Calendar) - pub calendar_id: Option, - pub calendar_name: Option, - // Metadata set after creation - pub google_resource_id: Option, - pub expiration: Option, -} -``` - -Branch in trait methods based on `trigger_type`. Frontend uses a `ToggleButtonGroup` to switch between types. This keeps the codebase simpler (one service, one OAuth flow, one set of routes). - -See `backend/windmill-native-triggers/src/google/` for the reference implementation. - -### Skipping update+get After Create (Google Pattern) - -Override `service_config_from_create_response()` to return `Some(config)` when the external_id is known before the create call: - -```rust -fn service_config_from_create_response( - &self, - data: &NativeTriggerData, - resp: &Self::CreateResponse, -) -> Option { - // Clone input config, add metadata from response - let mut config = data.service_config.clone(); - config.google_resource_id = Some(resp.resource_id.clone()); - config.expiration = Some(resp.expiration.clone()); - Some(serde_json::to_value(&config).unwrap()) -} -``` - -### Services with Absolute OAuth Endpoints (Google) - -Unlike self-hosted services where OAuth endpoints are relative paths appended to `base_url`, services like Google have absolute URLs: - -```rust -// Nextcloud: relative paths -ServiceName::Nextcloud => "/apps/oauth2/api/v1/token", -// Google: absolute URLs -ServiceName::Google => "https://oauth2.googleapis.com/token", -``` - -The `resolve_endpoint()` function handles both. For services with absolute endpoints: -- `base_url` can be empty -- `requiresBaseUrl: false` in the frontend workspace integration config -- Add `extra_auth_params()` if needed (Google requires `access_type=offline` and `prompt=consent`) - -### Channel-Based Push Notifications with Renewal (Google Pattern) - -For services using expiring watch channels instead of persistent webhooks: - -1. Store expiration in `service_config` (as part of `ServiceConfig`) -2. In `maintain_triggers()`, implement renewal logic instead of using `reconcile_with_external_state()`: - ```rust - async fn maintain_triggers(&self, db, workspace_id, triggers, oauth_data, synced, errors) { - for trigger in triggers { - if should_renew_channel(trigger) { - self.renew_channel(db, trigger, oauth_data).await; - } - } - } - ``` -3. Renewal: best-effort stop old channel, create new one with same external_id, update service_config with new expiration -4. Google example: Drive channels expire in 24h (renew when <1h left), Calendar channels expire in 7 days (renew when <1 day left) - -### reconcile_with_external_state (Nextcloud Pattern) - -The reusable function in `sync.rs` compares external triggers with DB state: -- Triggers missing externally: sets error "Trigger no longer exists on external service" -- Triggers present externally: clears errors, updates service_config if it differs - -Usage in `maintain_triggers()`: -```rust -let external_pairs: Vec<(String, serde_json::Value)> = /* fetch from external */; -crate::sync::reconcile_with_external_state( - db, workspace_id, Self::SERVICE_NAME, triggers, &external_pairs, synced, errors, -).await; -``` - -### Webhook Payload Processing - -Override `prepare_webhook()` to parse service-specific payloads into script/flow args: - -```rust -async fn prepare_webhook(&self, db, w_id, headers, body, script_path, is_flow) -> Result { - let mut args = HashMap::new(); - args.insert("event_type".to_string(), Box::new(headers.get("x-event-type").cloned()) as _); - args.insert("payload".to_string(), Box::new(serde_json::from_str::(&body)?) as _); - Ok(PushArgsOwned { extra: None, args }) -} -``` - -Then register in `prepare_native_trigger_args()` in `lib.rs`: -```rust -pub async fn prepare_native_trigger_args(service_name, db, w_id, headers, body) -> Result> { - match service_name { - ServiceName::Google => { /* ... */ Ok(Some(args)) } - ServiceName::NewService => { /* ... */ Ok(Some(args)) } - ServiceName::Nextcloud => Ok(None), // Uses default body parsing - } -} -``` - -### Instance-Level OAuth Credentials - -When `workspace_integrations.oauth_data.instance_shared == true`, `decrypt_oauth_data()` reads `client_id` and `client_secret` from instance-level global settings instead of workspace-level. This allows admins to share OAuth app credentials across workspaces. - -The frontend handles this via the `generate_instance_connect_url` endpoint in `workspace_integrations.rs`. - ---- - -## Testing Checklist - -- [ ] Database migration runs successfully -- [ ] `cargo check -p windmill-native-triggers --features native_trigger` passes -- [ ] `npx svelte-check --threshold error` passes (in frontend/) -- [ ] Service appears in workspace integrations list -- [ ] OAuth flow completes successfully -- [ ] Can create a new trigger -- [ ] Can view trigger details -- [ ] Can update trigger configuration -- [ ] Can delete trigger -- [ ] Webhook receives and processes payloads -- [ ] Background sync works correctly (reconciliation or channel renewal) -- [ ] Error handling works (expired tokens, service unavailable) - ---- - -## Reference Implementations - -### Nextcloud (Self-Hosted, Update+Get Pattern) - -| File | Purpose | -|------|---------| -| `nextcloud/mod.rs` | Types: NextCloudOAuthData, NextcloudServiceConfig, NextCloudTriggerData | -| `nextcloud/external.rs` | External trait: uses update+get pattern, reconcile_with_external_state for sync | -| `nextcloud/routes.rs` | Additional route: `GET /events` | - -Key patterns: relative OAuth endpoints, base_url required, list_all + reconcile for sync, update returns JSON from get(). - -### Google (Cloud, Unified Service, Short Create) - -| File | Purpose | -|------|---------| -| `google/mod.rs` | Types: GoogleServiceConfig with trigger_type discriminator, GoogleTriggerType enum | -| `google/external.rs` | External trait: overrides service_config_from_create_response, channel renewal for sync | -| `google/routes.rs` | Additional routes: `GET /calendars`, `GET /drive/files`, `GET /drive/shared_drives` | - -Key patterns: absolute OAuth endpoints, empty base_url, trigger_type for Drive/Calendar, expiring watch channels with renewal, service_config_from_create_response skips update+get, get() reconstructs data from stored service_config (no external "get channel" API). diff --git a/.claude/skills/native-trigger/SKILL.md b/.claude/skills/native-trigger/SKILL.md new file mode 120000 index 0000000000..18548efdba --- /dev/null +++ b/.claude/skills/native-trigger/SKILL.md @@ -0,0 +1 @@ +../../../.agents/skills/native-trigger/SKILL.md \ No newline at end of file diff --git a/.claude/skills/pr/SKILL.md b/.claude/skills/pr/SKILL.md deleted file mode 100644 index 2c7bd691ca..0000000000 --- a/.claude/skills/pr/SKILL.md +++ /dev/null @@ -1,111 +0,0 @@ ---- -name: pr -user_invocable: true -description: Open a draft pull request on GitHub. MUST use when you want to create/open a PR. ---- - -# Pull Request Skill - -Create a draft pull request with a clear title and explicit description of changes. - -## Instructions - -1. **Analyze branch changes**: Understand all commits since diverging from main -2. **Push to remote**: Ensure all commits are pushed -3. **Create draft PR**: Always open as draft for review before merging - -## PR Title Format - -Follow conventional commit format for the PR title: -``` -: -``` - -### Types -- `feat`: New feature or capability -- `fix`: Bug fix -- `refactor`: Code restructuring -- `docs`: Documentation changes -- `chore`: Maintenance tasks -- `perf`: Performance improvements - -### Title Rules -- Keep under 70 characters -- Use lowercase, imperative mood -- No period at the end -- If `*_ee.rs` files were modified, prefix with `[ee]`: `[ee] : ` - -## PR Body Format - -The body MUST be explicit about what changed. Structure: - -```markdown -## Summary - - -## Changes -- -- -- - -## Test plan -- [ ] -- [ ] - ---- -Generated with [Claude Code](https://claude.com/claude-code) -``` - -## Execution Steps - -1. Run `git status` to check for uncommitted changes -2. Run `git log main..HEAD --oneline` to see all commits in this branch -3. Run `git diff main...HEAD` to see the full diff against main -4. **Run `/local-review`** before creating the PR. If issues are found, fix them and commit before proceeding. Do not skip this step. -5. Check if remote branch exists and is up to date: - ```bash - git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null || echo "no upstream" - ``` -6. Push to remote if needed: `git push -u origin HEAD` -7. Create draft PR using gh CLI: - ```bash - gh pr create --draft --title ": " --body "$(cat <<'EOF' - ## Summary - - - ## Changes - - - - - - ## Test plan - - [ ] - - [ ] - - --- - Generated with [Claude Code](https://claude.com/claude-code) - EOF - )" - ``` -8. Return the PR URL to the user - -## EE Companion PR (when `*_ee.rs` files were modified) - -The `*_ee.rs` files in the windmill repo are **symlinks** to `windmill-ee-private` — changes won't appear in `git diff` of the windmill repo. Instead, check the EE repo for uncommitted or unpushed changes. - -Follow the full EE PR workflow in `docs/enterprise.md`. The key PR-specific details: - -1. Find the EE repo/worktree: see "Finding the EE Repo" in `docs/enterprise.md` -2. Check for changes: `git -C status --short` - - If there are no changes in the EE repo, skip this entire section -3. Follow steps 1–5 from the "EE PR Workflow" in `docs/enterprise.md` -4. Create the companion PR (title does NOT get the `[ee]` prefix): - ```bash - gh pr create --draft --repo windmill-labs/windmill-ee-private --title ": " --body "$(cat <<'EOF' - Companion PR for windmill-labs/windmill# - - --- - Generated with [Claude Code](https://claude.com/claude-code) - EOF - )" - ``` -5. Commit `ee-repo-ref.txt` and push the updated windmill branch diff --git a/.claude/skills/pr/SKILL.md b/.claude/skills/pr/SKILL.md new file mode 120000 index 0000000000..9458ad7097 --- /dev/null +++ b/.claude/skills/pr/SKILL.md @@ -0,0 +1 @@ +../../../.agents/skills/pr/SKILL.md \ No newline at end of file diff --git a/.claude/skills/refine/SKILL.md b/.claude/skills/refine/SKILL.md deleted file mode 100644 index aaf747cd29..0000000000 --- a/.claude/skills/refine/SKILL.md +++ /dev/null @@ -1,39 +0,0 @@ ---- -name: refine -user_invocable: true -description: End-of-session reflection. Reviews friction encountered during the session and proposes updates to docs/ to capture lessons learned. ---- - -# Refine Skill - -Reflect on the current session and update documentation with lessons learned. - -## Instructions - -1. **Identify friction**: Review what happened in this session: - - Run `git diff main...HEAD --stat` to see what files were touched - - Think about: what was slow, what failed, what required multiple attempts, what information was missing or hard to find - -2. **Read current docs**: Read the docs that were relevant to this session: - - `docs/validation.md` - - `docs/enterprise.md` - - `docs/autonomous-mode.md` - - Any skills that were invoked - -3. **Propose updates**: For each piece of friction, decide if it warrants a doc update: - - **Missing knowledge**: Information you had to discover that should be documented - - **Wrong guidance**: Instructions that led you astray - - **Missing validation rule**: A check that should be in the validation matrix - - **New pattern**: A codebase pattern worth capturing for next time - -4. **Apply updates**: Edit the relevant `docs/` files. Keep changes minimal and specific — add only what would have saved time this session. - -5. **Report**: Summarize what was added/changed and why. - -## Rules - -- Only add knowledge confirmed by this session — no speculative additions -- Keep docs concise — add a line or two, not a paragraph -- If a whole new doc is needed, create it in `docs/` and add a pointer in `CLAUDE.md` -- Don't update skills unless a coding pattern was genuinely wrong -- Don't add things Claude already knows — only Windmill-specific knowledge diff --git a/.claude/skills/refine/SKILL.md b/.claude/skills/refine/SKILL.md new file mode 120000 index 0000000000..39580df5d0 --- /dev/null +++ b/.claude/skills/refine/SKILL.md @@ -0,0 +1 @@ +../../../.agents/skills/refine/SKILL.md \ No newline at end of file diff --git a/.claude/skills/rust-backend/SKILL.md b/.claude/skills/rust-backend/SKILL.md deleted file mode 100644 index f0c52002bc..0000000000 --- a/.claude/skills/rust-backend/SKILL.md +++ /dev/null @@ -1,107 +0,0 @@ ---- -name: rust-backend -description: Rust coding guidelines for the Windmill backend. MUST use when writing or modifying Rust code in the backend directory. ---- - -# Windmill Rust Patterns - -Apply these Windmill-specific patterns when writing Rust code in `backend/`. - -## Error Handling - -Use `Error` from `windmill_common::error`. Return `Result` or `JsonResult`: - -```rust -use windmill_common::error::{Error, Result}; - -pub async fn get_job(db: &DB, id: Uuid) -> Result { - sqlx::query_as!(Job, "SELECT id, workspace_id FROM v2_job WHERE id = $1", id) - .fetch_optional(db) - .await? - .ok_or_else(|| Error::NotFound("job not found".to_string()))?; -} -``` - -Never panic in library code. Reserve `.unwrap()` for compile-time guarantees. - -## SQLx Patterns - -**Never use `SELECT *`** — always list columns explicitly. Critical for backwards compatibility when workers lag behind API version: - -```rust -// Correct -sqlx::query_as!(Job, "SELECT id, workspace_id, path FROM v2_job WHERE id = $1", id) - -// Wrong — breaks when columns are added -sqlx::query_as!(Job, "SELECT * FROM v2_job WHERE id = $1", id) -``` - -Use batch operations to avoid N+1: - -```rust -// Preferred — single query with IN clause -sqlx::query!("SELECT ... WHERE id = ANY($1)", &ids[..]).fetch_all(db).await? -``` - -Use transactions for multi-step operations. Parameterize all queries. - -## JSON Handling - -Prefer `Box` over `serde_json::Value` when storing/passing JSON without inspection: - -```rust -pub struct Job { - pub args: Option>, -} -``` - -Only use `serde_json::Value` when you need to inspect or modify the JSON. - -## Serde Optimizations - -```rust -#[derive(Serialize, Deserialize)] -pub struct Job { - #[serde(skip_serializing_if = "Option::is_none")] - pub parent_job: Option, - #[serde(skip_serializing_if = "Vec::is_empty")] - pub tags: Vec, - #[serde(default)] - pub priority: i32, -} -``` - -## Async & Concurrency - -Never block the async runtime. Use `spawn_blocking` for CPU-intensive work: - -```rust -let result = tokio::task::spawn_blocking(move || expensive_computation(&data)).await?; -``` - -**Mutex selection**: Prefer `std::sync::Mutex` (or `parking_lot::Mutex`) for data protection. Only use `tokio::sync::Mutex` when holding locks across `.await` points. - -Use `tokio::sync::mpsc` (bounded) for channels. Avoid `std::thread::sleep` in async contexts. - -## Module Structure & Visibility - -- Use `pub(crate)` instead of `pub` when possible -- Place new code in the appropriate crate based on functionality -- API endpoints go in `windmill-api/src/` organized by domain -- Shared functionality goes in `windmill-common/src/` - -## Code Navigation - -Always use rust-analyzer LSP for go-to-definition, find-references, and type info. Do not guess at module paths. - -## Axum Handlers - -Destructure extractors directly in function signatures: - -```rust -async fn process_job( - Extension(db): Extension, - Path((workspace, job_id)): Path<(String, Uuid)>, - Query(pagination): Query, -) -> Result> { ... } -``` diff --git a/.claude/skills/rust-backend/SKILL.md b/.claude/skills/rust-backend/SKILL.md new file mode 120000 index 0000000000..2500c55046 --- /dev/null +++ b/.claude/skills/rust-backend/SKILL.md @@ -0,0 +1 @@ +../../../.agents/skills/rust-backend/SKILL.md \ No newline at end of file diff --git a/.claude/skills/svelte-frontend/SKILL.md b/.claude/skills/svelte-frontend/SKILL.md deleted file mode 100644 index 57cac70302..0000000000 --- a/.claude/skills/svelte-frontend/SKILL.md +++ /dev/null @@ -1,80 +0,0 @@ ---- -name: svelte-frontend -description: Svelte coding guidelines for the Windmill frontend. MUST use when writing or modifying code in the frontend directory. ---- - -# Windmill Svelte Patterns - -Apply these Windmill-specific patterns when writing Svelte code in `frontend/`. For general Svelte 5 syntax (runes, snippets, event handling), use the Svelte MCP server. - -## Windmill UI Components (MUST use) - -Always use Windmill's design-system components. Never use raw HTML elements. - -### Buttons — ` -