mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
fix: address review round findings on head 3f47dc1
- backend/ and frontend/ guidance was Claude-only. Codex and Pi read AGENTS.md,
not CLAUDE.md, so moving "Verifying Backend/Frontend Changes" and the
$bindable ban out of the root AGENTS.md made them invisible to two of the
three CLIs this repo supports. Renamed both to AGENTS.md with a one-line
@AGENTS.md CLAUDE.md beside them, matching what the repo already does at the
root and in ai_evals/, and retargeted the four references.
- sqlx-cache.sh aborted with exit 2 and no output when .sqlx was empty:
list_entries ran `ls -1 ./*.json`, and an unmatched glob under
`set -euo pipefail` killed the script. An empty cache is precisely what a
failed prepare leaves behind, so it broke in the one case it exists for.
Replaced with a glob loop; reproduced the failure and verified the fix.
- The oneshot prompt ("never leave the PR sitting in draft") contradicted the
"Flip, or ask first" rule added in the same PR, which tells unattended runs to
leave wide-blast-radius changes as clean drafts. The prompt now defers to the
skill for the flip decision and keeps only "never stop at an unreviewed
draft".
- Bundled-resource references in the vendored skills were markdown links to
`.agents/skills/...`, which resolve relative to the file, not the repo root.
Replaced with inline paths stating they are repo-root relative.
- The PR-ready calibration file was write-only: the skill said to record
answers there but never to read it. It is now consulted before deciding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
hugocasa
@@ -110,5 +110,5 @@ Good interfaces make testing natural:
|
||||
|
||||
## Going deeper
|
||||
|
||||
- **Deepening a cluster given its dependencies** — see [DEEPENING.md](.agents/skills/codebase-design/DEEPENING.md): dependency categories, seam discipline, and replace-don't-layer testing.
|
||||
- **Exploring alternative interfaces** — see [DESIGN-IT-TWICE.md](.agents/skills/codebase-design/DESIGN-IT-TWICE.md): spin up parallel sub-agents to design the interface several radically different ways, then compare on depth, locality, and seam placement.
|
||||
- **Deepening a cluster given its dependencies** — see `.agents/skills/codebase-design/DEEPENING.md` (path from the repo root): dependency categories, seam discipline, and replace-don't-layer testing.
|
||||
- **Exploring alternative interfaces** — see `.agents/skills/codebase-design/DESIGN-IT-TWICE.md` (path from the repo root): spin up parallel sub-agents to design the interface several radically different ways, then compare on depth, locality, and seam placement.
|
||||
|
||||
@@ -51,7 +51,7 @@ When the user states how something works, check whether the code agrees. If you
|
||||
|
||||
### Update CONTEXT.md inline
|
||||
|
||||
When a term is resolved, update `CONTEXT.md` right there. Don't batch these up — capture them as they happen. Use the format in [CONTEXT-FORMAT.md](.agents/skills/domain-modeling/CONTEXT-FORMAT.md).
|
||||
When a term is resolved, update `CONTEXT.md` right there. Don't batch these up — capture them as they happen. Use the format in `.agents/skills/domain-modeling/CONTEXT-FORMAT.md` (path from the repo root).
|
||||
|
||||
`CONTEXT.md` should be totally devoid of implementation details. Do not treat `CONTEXT.md` as a spec, a scratch pad, or a repository for implementation decisions. It is a glossary and nothing else.
|
||||
|
||||
|
||||
@@ -53,7 +53,7 @@ End the report with a **Top recommendation** section: which candidate you'd tack
|
||||
|
||||
**Use CONTEXT.md vocabulary for the domain, and the `/codebase-design` vocabulary for the architecture.** If `CONTEXT.md` defines "Order," talk about "the Order intake module" — not "the FooBarHandler," and not "the Order service."
|
||||
|
||||
See [HTML-REPORT.md](.agents/skills/improve-codebase-architecture/HTML-REPORT.md) for the full HTML scaffold, diagram patterns, and styling guidance.
|
||||
See `.agents/skills/improve-codebase-architecture/HTML-REPORT.md` (path from the repo root) for the full HTML scaffold, diagram patterns, and styling guidance.
|
||||
|
||||
Do NOT propose interfaces yet. After the file is written, ask the user: "Which of these would you like to explore?"
|
||||
|
||||
|
||||
@@ -62,7 +62,7 @@ If `git diff main...HEAD --name-only` matches `^frontend/`, the PR body **must**
|
||||
screenshots of the affected UI. Skip only when there is no visible UI effect (types,
|
||||
tests, build config) — and say so in the body.
|
||||
|
||||
1. Verify the change in the browser (frontend/CLAUDE.md → "Verifying Frontend Changes").
|
||||
1. Verify the change in the browser (frontend/AGENTS.md → "Verifying Frontend Changes").
|
||||
2. Screenshot each affected page with `mcp__playwright__browser_take_screenshot` (save to a file).
|
||||
3. Host each image and get its Markdown embed by pushing to the public
|
||||
`windmill-labs/agent-screenshots-internal` repo. **Pipe base64 through stdin** —
|
||||
@@ -216,8 +216,9 @@ description saying why — `left in draft: adds a migration, wants a human look
|
||||
Don't flip a wide-blast-radius change just because the round came back clean, and don't ask a
|
||||
question nobody will read.
|
||||
|
||||
When the call is genuinely ambiguous, ask, then record the answer under "PR ready calibration" in
|
||||
`AGENTS.local.md` so the next one is less ambiguous.
|
||||
`AGENTS.local.md` (gitignored, so it may not exist) carries a "PR ready calibration" section
|
||||
recording how past ambiguous calls went. Read it before deciding; when a call is still genuinely
|
||||
ambiguous, ask, then append the answer there so the next one is less ambiguous.
|
||||
|
||||
### When rounds stop converging
|
||||
|
||||
|
||||
@@ -127,4 +127,4 @@ Use the Svelte MCP tools when working on Svelte code:
|
||||
|
||||
## Verifying in the Browser
|
||||
|
||||
After changing Svelte code, use the **Playwright MCP** (`mcp__playwright__*`) to drive the running frontend and confirm the change works. See frontend/CLAUDE.md → "Verifying Frontend Changes" for the full flow. Use `playwright` (headless) on devboxes; `playwright-headed` when a display is available.
|
||||
After changing Svelte code, use the **Playwright MCP** (`mcp__playwright__*`) to drive the running frontend and confirm the change works. See frontend/AGENTS.md → "Verifying Frontend Changes" for the full flow. Use `playwright` (headless) on devboxes; `playwright-headed` when a display is available.
|
||||
|
||||
@@ -21,8 +21,16 @@ state="${TMPDIR:-/tmp}/wm-sqlx-cache/$(basename "$repo_root")"
|
||||
backup="$state/backup"
|
||||
added="$state/added"
|
||||
|
||||
# `find -printf` is GNU-only; this stays portable to a macOS checkout.
|
||||
list_entries() { (cd "$1" 2>/dev/null && ls -1 ./*.json 2>/dev/null | sed 's|^\./||') | sort; }
|
||||
# `find -printf` is GNU-only; a glob loop stays portable to a macOS checkout and, unlike
|
||||
# `ls *.json`, does not fail the script under `set -e` when the cache is empty — which is
|
||||
# exactly the state a failed `prepare` leaves behind.
|
||||
list_entries() {
|
||||
local f
|
||||
for f in "$1"/*.json; do
|
||||
[ -e "$f" ] || continue
|
||||
basename "$f"
|
||||
done | sort
|
||||
}
|
||||
|
||||
show_query() {
|
||||
if command -v jq >/dev/null 2>&1; then
|
||||
|
||||
+6
-6
@@ -146,12 +146,12 @@ oneshot:
|
||||
— note the choice in the PR description if it matters.
|
||||
|
||||
# PR readiness
|
||||
Always open the PR as a draft, then take it to ready before you stop.
|
||||
Follow the `pr` skill's "Review rounds": run the review round, fix every
|
||||
finding, repeat until all reviewer verdicts are a go, then post the
|
||||
`✅ Review round clean @ <sha>` marker and `gh pr ready`. Never flip to
|
||||
ready without a clean round behind it, and never leave the PR sitting in
|
||||
draft — an unreviewed draft is an unfinished oneshot.
|
||||
Always open the PR as a draft, then drive the `pr` skill's "Review rounds"
|
||||
until every reviewer verdict is a go. Never flip to ready without a clean
|
||||
round behind it, and never stop at an *unreviewed* draft — that is an
|
||||
unfinished oneshot. Whether a clean round then flips the PR is the skill's
|
||||
"Flip, or ask first" call, not this prompt's: self-contained changes flip,
|
||||
wide-blast-radius ones stay a clean draft with the reason in the PR body.
|
||||
|
||||
# Ending your turn
|
||||
Never end your turn with a question, a suggestion to "take a look", or a
|
||||
|
||||
@@ -15,7 +15,7 @@ Open-source platform for internal tools, workflows, API integrations, background
|
||||
change touches, get that path actually running, and stand up whatever that takes — this is
|
||||
expected, not a last resort. A few examples, not a closed list: drive the UI with the Playwright
|
||||
MCP, run a real job of the kind you touched, restart the backend with the cargo features the
|
||||
path needs (`backend/CLAUDE.md`), put a stub in front of an upstream, start MinIO for an S3
|
||||
path needs (`backend/AGENTS.md`), put a stub in front of an upstream, start MinIO for an S3
|
||||
path, plant state with SQL, exercise it through the `wmill` CLI. If the path you need has no
|
||||
obvious way in, invent one rather than skipping it; `docs/` carries recipes for several areas.
|
||||
If it needs a credential or a third-party account, ask for one rather than skipping the test or
|
||||
@@ -54,7 +54,7 @@ Open-source platform for internal tools, workflows, API integrations, background
|
||||
(`$WEBMUX_WORKTREE_PATH` is set) the backend and frontend are already up in sibling tmux panes —
|
||||
use those, don't spawn your own. `tmux list-panes -t "$(tmux display-message -p -t "$TMUX_PANE"
|
||||
'#{window_id}')" -F '#{pane_index} #{pane_current_command}'` shows what is running; read its log
|
||||
with `tmux capture-pane`, and see `backend/CLAUDE.md` to restart it with different cargo features.
|
||||
with `tmux capture-pane`, and see `backend/AGENTS.md` to restart it with different cargo features.
|
||||
A second server started in your own shell fights the first one for the port. The commands below
|
||||
are for a plain checkout with nothing running.
|
||||
|
||||
|
||||
@@ -0,0 +1,209 @@
|
||||
# Backend (Rust)
|
||||
|
||||
- **Coding patterns**: MUST use the `rust-backend` skill when writing Rust code
|
||||
- **Validation**: `docs/validation.md` — which `cargo check` flags to use
|
||||
- **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow
|
||||
- **DB schema**: `backend/summarized_schema.txt`
|
||||
- **API routes entry point**: `windmill-api/src/lib.rs`
|
||||
- **OpenAPI spec**: `windmill-api/openapi.yaml`
|
||||
- **DuckDB local jobs**: build the dynamic FFI library before running DuckDB scripts locally:
|
||||
```bash
|
||||
cd backend/windmill-duckdb-ffi-internal && ./build_dev.sh
|
||||
```
|
||||
Re-run after clean builds or when `target/debug/libwindmill_duckdb_ffi_internal.*` is missing.
|
||||
The bundled DuckDB compile (~2min) is cached in a per-user dir shared across
|
||||
worktrees (keyed by the crate's `Cargo.lock`), so a fresh worktree reuses it and
|
||||
the build is near-instant — you don't pay the full compile per worktree. Editing
|
||||
the FFI crate's own source falls back to an isolated per-worktree `./target`.
|
||||
The engine is a **patched fork** of duckdb-rs, not the crates.io crate — read
|
||||
`docs/duckdb-isolation.md` before bumping it or touching the isolation transform.
|
||||
- **Running data pipelines (DuckLake) from source**: see the section below — a plain build
|
||||
advertises the `duckdb` tag but cannot execute DuckDB scripts and has no working S3 proxy.
|
||||
|
||||
## Cargo features & running the dev backend
|
||||
|
||||
The dev backend runs under `cargo watch` and is launched by default with **only
|
||||
`--features quickjs`** (see the tmux backend pane). That baseline compiles fast but
|
||||
**deliberately omits most functionality** — notably S3/object storage, the S3 proxy, all
|
||||
EE code, MCP, and every non-JS language runtime. A running server never gains a feature you
|
||||
didn't compile in: feature-gated routes 404 or return a `"requires <feature>"` stub. So if
|
||||
you touch code behind a feature gate, or need to *exercise* such a feature at runtime, you
|
||||
MUST **restart the backend with the appropriate features** for what you're working on.
|
||||
|
||||
### Restarting the dev backend with the right features
|
||||
|
||||
Restart in the **same pane**, so the relaunch inherits that pane's `DATABASE_URL`, `BACKEND_PORT`
|
||||
and the rest of `runtime.env`. Scope every kill to this worktree — **never**
|
||||
`pkill -f target/debug/windmill`, which kills every sibling worktree's backend.
|
||||
|
||||
1. Find the backend pane by what it is running, not by index. The index depends on the webmux
|
||||
profile: pane 1 is the backend under `full`, but the *frontend* under `frontendOnly`.
|
||||
|
||||
```bash
|
||||
WIN=$(tmux display-message -p -t "$TMUX_PANE" '#{window_id}')
|
||||
tmux list-panes -t "$WIN" -F '#{pane_index} #{pane_current_command} #{pane_pid}'
|
||||
```
|
||||
|
||||
2. Read the feature set it is **actually** running. `CARGO_FEATURES` in `runtime.env` is only what
|
||||
the pane started with, and goes stale the first time anyone restarts by hand:
|
||||
|
||||
```bash
|
||||
ps --ppid <pane_pid> -o args=
|
||||
# /home/hugo/.cargo/bin/cargo-watch watch -x run --features quickjs
|
||||
```
|
||||
|
||||
3. Stop it and relaunch with the extended set. `PORT` in the pane shell can be stale, so pass it
|
||||
explicitly:
|
||||
|
||||
```bash
|
||||
tmux send-keys -t "$WIN.<idx>" C-c
|
||||
tmux send-keys -t "$WIN.<idx>" 'PORT=$BACKEND_PORT cargo watch -x "run --features quickjs,private,parquet"' Enter
|
||||
```
|
||||
|
||||
Carry over every feature the old command had unless you mean to drop one — rebuilding the list
|
||||
from memory is how a backend silently loses `quickjs`.
|
||||
|
||||
4. Persist the new set so a recreated pane starts with it: set `CARGO_FEATURES` in
|
||||
`$(git rev-parse --git-dir)/webmux/runtime.env`. That file is read at pane startup only, so it
|
||||
changes nothing about the process you just relaunched — step 3 is what takes effect now.
|
||||
|
||||
5. Re-capture the pane until `health check completed` appears before hitting the API. A cold
|
||||
rebuild takes ~60s, and the previous run's success line is still in the scrollback, so a
|
||||
capture taken too early reads as ready when it isn't.
|
||||
|
||||
cargo-watch only re-runs on a file change, so after an idle/failed run `touch README.md` (from
|
||||
`backend/`, where the watch runs) is a cheap retrigger (touching a `.rs` forces a full rebuild).
|
||||
|
||||
### An orphaned backend is holding the port
|
||||
|
||||
If the pane's `cargo watch` looks alive but the API never answers, or the build ends in an
|
||||
address-already-in-use error, a backend from an earlier run is probably still bound to the port.
|
||||
It gets reparented to `systemd --user` when its shell dies, so it survives everything that looks
|
||||
like a cleanup.
|
||||
|
||||
Confirm all three before killing anything — a dozen sibling worktrees run their own backend, and
|
||||
`pkill -f windmill` (or `-f target/debug/windmill`) kills every one of them:
|
||||
|
||||
```bash
|
||||
ss -ltnp | grep ":$BACKEND_PORT" # 1. which pid holds the port
|
||||
readlink /proc/<pid>/cwd # 2. must be THIS worktree's backend/
|
||||
ps -o ppid= -p <pid> # 3. parent is systemd/pid 1, not your pane's cargo-watch
|
||||
```
|
||||
|
||||
Only when the port owner is this worktree's backend **and** it is orphaned, kill that single pid
|
||||
(`kill <pid>`, then `kill -9` if it does not exit). Ask first when there is a human in the loop;
|
||||
unattended, the three checks are what make it safe. Then `touch README.md` to retrigger the
|
||||
watch.
|
||||
|
||||
### What each feature gate does (the ones you'll actually toggle)
|
||||
|
||||
`backend/Cargo.toml` `[features]` is the source of truth; this is the practical dev map. Combine
|
||||
only what you need — build time scales with the set.
|
||||
|
||||
| Feature | Enables | Need it for |
|
||||
|---|---|---|
|
||||
| `quickjs` | Embedded JS engine for inline JS eval (the default dev baseline). | Keep in every dev set. |
|
||||
| `private` | Compiles the `*_ee.rs` files (symlinked from `windmill-ee-private`). Gates **all** EE code, including the real S3 helpers, the S3 proxy, and advanced S3 permission checks. | Any EE code path, S3/object storage. |
|
||||
| `enterprise` | EE business logic (autoscaling, SAML hooks, advanced S3 rule **enforcement**, WAP, forks, …). Pulls in `license`. | Running EE features. Advanced S3 permission rules only take effect with this. |
|
||||
| `license` | License-key/plan plumbing (`LICENSE_KEY`). Pulled in by `enterprise`. Having the feature compiled does **not** require a license *key* at runtime — CE defaults to a free plan and most EE paths still run keyless. | License-gated behavior. |
|
||||
| `parquet` | S3/object-storage support: the `job_helpers/*` and `apps_u/*` S3 endpoints, parquet/CSV preview, workspace large-file storage. Without it those routes return `"requires parquet"`. | Anything touching S3/object storage or datasets. |
|
||||
| `duckdb` | DuckDB script executor (also needs the FFI dylib — see above). | DuckDB scripts, DuckLake. |
|
||||
| `python` `rust` `php` `java` `ruby` `csharp` `nu` `deno_core` `mysql` `mssql` `bigquery` `snowflake` `oracledb` `rlang` | Each enables that language/DB runtime for job execution. | Running jobs in that language. |
|
||||
| `mcp` | MCP gateway routes (baseline `quickjs` does NOT include it → MCP routes 404). | MCP work. |
|
||||
| `websocket` `http_trigger` `kafka` `nats` `mqtt_trigger` `sqs_trigger` `gcp_trigger` `azure_trigger` `postgres_trigger` `native_trigger` | Each native trigger kind; none on by default (creating one 404s without its feature). | Working on / exercising that trigger. |
|
||||
| `no_auth` | Treats every request as an admin superadmin (`CLOUD_HOSTED`-guarded). | Local auth-free experiments only. |
|
||||
|
||||
Convenience bundles (`ce`, `ee`, `oss`, …) exist in `[features]` but are heavy — prefer the
|
||||
minimal explicit set for dev.
|
||||
|
||||
**Common combinations** (run from `backend/`):
|
||||
|
||||
| Goal | `--features` |
|
||||
|---|---|
|
||||
| Plain dev baseline (JS eval only) | `quickjs` |
|
||||
| S3 / object storage / datasets (CE) | `quickjs,private,parquet` |
|
||||
| S3 + EE (advanced S3 rules, on-behalf app reads, WAP, forks) | `quickjs,enterprise,private,parquet` |
|
||||
| DuckLake / DuckDB (CE) | `quickjs,duckdb,parquet,private` (+ build the FFI) |
|
||||
| + Python jobs | append `,python` |
|
||||
|
||||
## Workspace object storage in dev — use the local filesystem
|
||||
|
||||
For a dev workspace you don't need MinIO/S3: use the built-in **`FilesystemStorage`** large-file
|
||||
storage (a root path on local disk). It is intentionally hidden from the settings-UI storage
|
||||
dropdown (dev-only), so set it via the API. Requires the backend built with `parquet` (+ `private`
|
||||
for the real S3 helpers, + `enterprise` if you want advanced permission rules enforced):
|
||||
|
||||
```bash
|
||||
curl -X POST "$BASE/api/w/<ws>/workspaces/edit_large_file_storage_config" \
|
||||
-H "Authorization: Bearer <admin-token>" -H "Content-Type: application/json" \
|
||||
-d '{"large_file_storage":{"type":"FilesystemStorage","root_path":"/abs/writable/dir",
|
||||
"public_resource":false,"advanced_permissions":null,"secondary_storage":{}}}'
|
||||
```
|
||||
|
||||
Optional `advanced_permissions` (EE) is a list of `{"pattern":"<glob>","allow":"read[,write,delete,list]"}`
|
||||
rules: admins bypass them, non-admins are confined to matching grants. Uploads/reads then flow
|
||||
through the normal `job_helpers/*` (viewer-scoped) and `apps_u/*` (app-author on-behalf) S3
|
||||
endpoints. Caveat: direct DuckDB access rejects filesystem stores (`"Filesystem is not supported
|
||||
in DuckDB"`) — DuckLake/datatable go through the S3 proxy instead, which works.
|
||||
|
||||
## Running data pipelines (DuckLake) from source
|
||||
|
||||
DuckLake pipelines need **both** the right cargo features **and** the prebuilt DuckDB FFI. A
|
||||
plain `cargo run` (or `cargo run --features quickjs`) does **not** suffice, and the failure modes
|
||||
are silent-ish, so agents lose time. Verify feature names against `backend/Cargo.toml` `[features]`.
|
||||
|
||||
**Feature sets** (run from `backend/`):
|
||||
|
||||
| Goal | Command |
|
||||
|---|---|
|
||||
| CE DuckLake (DuckDB scripts + S3 proxy) | `cargo run --features quickjs,duckdb,parquet,private` |
|
||||
| + Python scripts | add `,python` |
|
||||
| EE features (WAP, partitioning, forks, …) | add `,enterprise,license` |
|
||||
|
||||
`enterprise` already pulls in `license`, but list both when you want the license-gated paths.
|
||||
`quickjs` is for JS eval, not DuckLake per se — keep it if your baseline build had it.
|
||||
|
||||
**Before running any DuckDB script**, build the FFI (see the bullet above):
|
||||
`cd backend/windmill-duckdb-ffi-internal && ./build_dev.sh`.
|
||||
|
||||
**Two gotchas that a wrong feature set produces:**
|
||||
|
||||
1. **`duckdb` tag advertised, feature missing.** The `duckdb` worker tag is in the *unconditional*
|
||||
default tag list (`windmill-common/src/worker.rs`, `DEFAULT_TAGS`), so a worker advertises it even
|
||||
without the `duckdb` feature. Jobs then dispatch but fail at execution with
|
||||
`"Duck DB requires the duckdb feature to be enabled"` (`windmill-worker/src/worker.rs`). Fix:
|
||||
compile with `--features duckdb`.
|
||||
2. **DuckLake writes 404 (no S3 proxy).** The workspace S3 proxy (`/w/{ws}/s3_proxy/*`) that
|
||||
DuckLake uses for reads/writes only mounts the real service under
|
||||
`#[cfg(all(feature = "private", feature = "parquet"))]` (`windmill-api/src/s3_proxy_oss.rs`);
|
||||
otherwise it's an empty router and every proxied request 404s. Fix: compile with **both**
|
||||
`private` and `parquet`.
|
||||
|
||||
## Cloud vs self-hosted gating
|
||||
|
||||
The `cloud` cargo feature is compiled into **all** EE builds, so `#[cfg(feature = "cloud")]` is **not** a "cloud-only" runtime gate — it only means the code is present. The real gate for behavior specific to the managed cloud (app.windmill.dev) is the runtime flag `*CLOUD_HOSTED` (`windmill_common::worker::CLOUD_HOSTED`, from the `CLOUD_HOSTED` env var; note it's loaded from `.env` via `dotenv`, so it won't show in `/proc/<pid>/environ` — check the running behavior, not the exec env).
|
||||
|
||||
Cloud-only logic must be behind `if *CLOUD_HOSTED { ... }`: feature-gate the helper so it compiles, then **runtime-gate the call**. `#[cfg(feature = "cloud")]` on its own is only sufficient for:
|
||||
- pure helper/struct definitions (they only run when a gated caller invokes them),
|
||||
- code already inside an `if *CLOUD_HOSTED { ... }` block,
|
||||
- handlers that early-return on `!*CLOUD_HOSTED`,
|
||||
- idempotent no-ops that are harmless off-cloud (e.g. cache invalidation).
|
||||
|
||||
## Verifying Backend Changes
|
||||
|
||||
`cargo check` and the unit tests do not exercise a worker code path. **If you changed how
|
||||
a job runs — an executor, `handle_child`, anything spawning or reading from a
|
||||
subprocess — run an actual job of that kind** and confirm it completed, then say so.
|
||||
Whole classes of defect compile and unit-test clean:
|
||||
|
||||
- **Stack overflow from a large buffer in an async block.** An array declared across an
|
||||
`.await` is baked into the future's state; once that future is boxed a few layers deep
|
||||
by the job poller, two 16 KB arrays abort the worker *process* (`thread
|
||||
'tokio-runtime-worker' has overflowed its stack`). Heap-allocate read buffers
|
||||
(`vec![0u8; N]`, not `[0u8; N]`).
|
||||
- Deadlocks from draining only one of a child's pipes, missed cancellation or timeout
|
||||
propagation, and anything depending on the real engine's output format.
|
||||
|
||||
A crash like this takes down every job on that worker, not just yours, so check the
|
||||
backend log after the run rather than only the job's own status. If you cannot run one,
|
||||
say which path went unexercised instead of implying it was verified.
|
||||
+1
-209
@@ -1,209 +1 @@
|
||||
# Backend (Rust)
|
||||
|
||||
- **Coding patterns**: MUST use the `rust-backend` skill when writing Rust code
|
||||
- **Validation**: `docs/validation.md` — which `cargo check` flags to use
|
||||
- **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow
|
||||
- **DB schema**: `backend/summarized_schema.txt`
|
||||
- **API routes entry point**: `windmill-api/src/lib.rs`
|
||||
- **OpenAPI spec**: `windmill-api/openapi.yaml`
|
||||
- **DuckDB local jobs**: build the dynamic FFI library before running DuckDB scripts locally:
|
||||
```bash
|
||||
cd backend/windmill-duckdb-ffi-internal && ./build_dev.sh
|
||||
```
|
||||
Re-run after clean builds or when `target/debug/libwindmill_duckdb_ffi_internal.*` is missing.
|
||||
The bundled DuckDB compile (~2min) is cached in a per-user dir shared across
|
||||
worktrees (keyed by the crate's `Cargo.lock`), so a fresh worktree reuses it and
|
||||
the build is near-instant — you don't pay the full compile per worktree. Editing
|
||||
the FFI crate's own source falls back to an isolated per-worktree `./target`.
|
||||
The engine is a **patched fork** of duckdb-rs, not the crates.io crate — read
|
||||
`docs/duckdb-isolation.md` before bumping it or touching the isolation transform.
|
||||
- **Running data pipelines (DuckLake) from source**: see the section below — a plain build
|
||||
advertises the `duckdb` tag but cannot execute DuckDB scripts and has no working S3 proxy.
|
||||
|
||||
## Cargo features & running the dev backend
|
||||
|
||||
The dev backend runs under `cargo watch` and is launched by default with **only
|
||||
`--features quickjs`** (see the tmux backend pane). That baseline compiles fast but
|
||||
**deliberately omits most functionality** — notably S3/object storage, the S3 proxy, all
|
||||
EE code, MCP, and every non-JS language runtime. A running server never gains a feature you
|
||||
didn't compile in: feature-gated routes 404 or return a `"requires <feature>"` stub. So if
|
||||
you touch code behind a feature gate, or need to *exercise* such a feature at runtime, you
|
||||
MUST **restart the backend with the appropriate features** for what you're working on.
|
||||
|
||||
### Restarting the dev backend with the right features
|
||||
|
||||
Restart in the **same pane**, so the relaunch inherits that pane's `DATABASE_URL`, `BACKEND_PORT`
|
||||
and the rest of `runtime.env`. Scope every kill to this worktree — **never**
|
||||
`pkill -f target/debug/windmill`, which kills every sibling worktree's backend.
|
||||
|
||||
1. Find the backend pane by what it is running, not by index. The index depends on the webmux
|
||||
profile: pane 1 is the backend under `full`, but the *frontend* under `frontendOnly`.
|
||||
|
||||
```bash
|
||||
WIN=$(tmux display-message -p -t "$TMUX_PANE" '#{window_id}')
|
||||
tmux list-panes -t "$WIN" -F '#{pane_index} #{pane_current_command} #{pane_pid}'
|
||||
```
|
||||
|
||||
2. Read the feature set it is **actually** running. `CARGO_FEATURES` in `runtime.env` is only what
|
||||
the pane started with, and goes stale the first time anyone restarts by hand:
|
||||
|
||||
```bash
|
||||
ps --ppid <pane_pid> -o args=
|
||||
# /home/hugo/.cargo/bin/cargo-watch watch -x run --features quickjs
|
||||
```
|
||||
|
||||
3. Stop it and relaunch with the extended set. `PORT` in the pane shell can be stale, so pass it
|
||||
explicitly:
|
||||
|
||||
```bash
|
||||
tmux send-keys -t "$WIN.<idx>" C-c
|
||||
tmux send-keys -t "$WIN.<idx>" 'PORT=$BACKEND_PORT cargo watch -x "run --features quickjs,private,parquet"' Enter
|
||||
```
|
||||
|
||||
Carry over every feature the old command had unless you mean to drop one — rebuilding the list
|
||||
from memory is how a backend silently loses `quickjs`.
|
||||
|
||||
4. Persist the new set so a recreated pane starts with it: set `CARGO_FEATURES` in
|
||||
`$(git rev-parse --git-dir)/webmux/runtime.env`. That file is read at pane startup only, so it
|
||||
changes nothing about the process you just relaunched — step 3 is what takes effect now.
|
||||
|
||||
5. Re-capture the pane until `health check completed` appears before hitting the API. A cold
|
||||
rebuild takes ~60s, and the previous run's success line is still in the scrollback, so a
|
||||
capture taken too early reads as ready when it isn't.
|
||||
|
||||
cargo-watch only re-runs on a file change, so after an idle/failed run `touch README.md` (from
|
||||
`backend/`, where the watch runs) is a cheap retrigger (touching a `.rs` forces a full rebuild).
|
||||
|
||||
### An orphaned backend is holding the port
|
||||
|
||||
If the pane's `cargo watch` looks alive but the API never answers, or the build ends in an
|
||||
address-already-in-use error, a backend from an earlier run is probably still bound to the port.
|
||||
It gets reparented to `systemd --user` when its shell dies, so it survives everything that looks
|
||||
like a cleanup.
|
||||
|
||||
Confirm all three before killing anything — a dozen sibling worktrees run their own backend, and
|
||||
`pkill -f windmill` (or `-f target/debug/windmill`) kills every one of them:
|
||||
|
||||
```bash
|
||||
ss -ltnp | grep ":$BACKEND_PORT" # 1. which pid holds the port
|
||||
readlink /proc/<pid>/cwd # 2. must be THIS worktree's backend/
|
||||
ps -o ppid= -p <pid> # 3. parent is systemd/pid 1, not your pane's cargo-watch
|
||||
```
|
||||
|
||||
Only when the port owner is this worktree's backend **and** it is orphaned, kill that single pid
|
||||
(`kill <pid>`, then `kill -9` if it does not exit). Ask first when there is a human in the loop;
|
||||
unattended, the three checks are what make it safe. Then `touch README.md` to retrigger the
|
||||
watch.
|
||||
|
||||
### What each feature gate does (the ones you'll actually toggle)
|
||||
|
||||
`backend/Cargo.toml` `[features]` is the source of truth; this is the practical dev map. Combine
|
||||
only what you need — build time scales with the set.
|
||||
|
||||
| Feature | Enables | Need it for |
|
||||
|---|---|---|
|
||||
| `quickjs` | Embedded JS engine for inline JS eval (the default dev baseline). | Keep in every dev set. |
|
||||
| `private` | Compiles the `*_ee.rs` files (symlinked from `windmill-ee-private`). Gates **all** EE code, including the real S3 helpers, the S3 proxy, and advanced S3 permission checks. | Any EE code path, S3/object storage. |
|
||||
| `enterprise` | EE business logic (autoscaling, SAML hooks, advanced S3 rule **enforcement**, WAP, forks, …). Pulls in `license`. | Running EE features. Advanced S3 permission rules only take effect with this. |
|
||||
| `license` | License-key/plan plumbing (`LICENSE_KEY`). Pulled in by `enterprise`. Having the feature compiled does **not** require a license *key* at runtime — CE defaults to a free plan and most EE paths still run keyless. | License-gated behavior. |
|
||||
| `parquet` | S3/object-storage support: the `job_helpers/*` and `apps_u/*` S3 endpoints, parquet/CSV preview, workspace large-file storage. Without it those routes return `"requires parquet"`. | Anything touching S3/object storage or datasets. |
|
||||
| `duckdb` | DuckDB script executor (also needs the FFI dylib — see above). | DuckDB scripts, DuckLake. |
|
||||
| `python` `rust` `php` `java` `ruby` `csharp` `nu` `deno_core` `mysql` `mssql` `bigquery` `snowflake` `oracledb` `rlang` | Each enables that language/DB runtime for job execution. | Running jobs in that language. |
|
||||
| `mcp` | MCP gateway routes (baseline `quickjs` does NOT include it → MCP routes 404). | MCP work. |
|
||||
| `websocket` `http_trigger` `kafka` `nats` `mqtt_trigger` `sqs_trigger` `gcp_trigger` `azure_trigger` `postgres_trigger` `native_trigger` | Each native trigger kind; none on by default (creating one 404s without its feature). | Working on / exercising that trigger. |
|
||||
| `no_auth` | Treats every request as an admin superadmin (`CLOUD_HOSTED`-guarded). | Local auth-free experiments only. |
|
||||
|
||||
Convenience bundles (`ce`, `ee`, `oss`, …) exist in `[features]` but are heavy — prefer the
|
||||
minimal explicit set for dev.
|
||||
|
||||
**Common combinations** (run from `backend/`):
|
||||
|
||||
| Goal | `--features` |
|
||||
|---|---|
|
||||
| Plain dev baseline (JS eval only) | `quickjs` |
|
||||
| S3 / object storage / datasets (CE) | `quickjs,private,parquet` |
|
||||
| S3 + EE (advanced S3 rules, on-behalf app reads, WAP, forks) | `quickjs,enterprise,private,parquet` |
|
||||
| DuckLake / DuckDB (CE) | `quickjs,duckdb,parquet,private` (+ build the FFI) |
|
||||
| + Python jobs | append `,python` |
|
||||
|
||||
## Workspace object storage in dev — use the local filesystem
|
||||
|
||||
For a dev workspace you don't need MinIO/S3: use the built-in **`FilesystemStorage`** large-file
|
||||
storage (a root path on local disk). It is intentionally hidden from the settings-UI storage
|
||||
dropdown (dev-only), so set it via the API. Requires the backend built with `parquet` (+ `private`
|
||||
for the real S3 helpers, + `enterprise` if you want advanced permission rules enforced):
|
||||
|
||||
```bash
|
||||
curl -X POST "$BASE/api/w/<ws>/workspaces/edit_large_file_storage_config" \
|
||||
-H "Authorization: Bearer <admin-token>" -H "Content-Type: application/json" \
|
||||
-d '{"large_file_storage":{"type":"FilesystemStorage","root_path":"/abs/writable/dir",
|
||||
"public_resource":false,"advanced_permissions":null,"secondary_storage":{}}}'
|
||||
```
|
||||
|
||||
Optional `advanced_permissions` (EE) is a list of `{"pattern":"<glob>","allow":"read[,write,delete,list]"}`
|
||||
rules: admins bypass them, non-admins are confined to matching grants. Uploads/reads then flow
|
||||
through the normal `job_helpers/*` (viewer-scoped) and `apps_u/*` (app-author on-behalf) S3
|
||||
endpoints. Caveat: direct DuckDB access rejects filesystem stores (`"Filesystem is not supported
|
||||
in DuckDB"`) — DuckLake/datatable go through the S3 proxy instead, which works.
|
||||
|
||||
## Running data pipelines (DuckLake) from source
|
||||
|
||||
DuckLake pipelines need **both** the right cargo features **and** the prebuilt DuckDB FFI. A
|
||||
plain `cargo run` (or `cargo run --features quickjs`) does **not** suffice, and the failure modes
|
||||
are silent-ish, so agents lose time. Verify feature names against `backend/Cargo.toml` `[features]`.
|
||||
|
||||
**Feature sets** (run from `backend/`):
|
||||
|
||||
| Goal | Command |
|
||||
|---|---|
|
||||
| CE DuckLake (DuckDB scripts + S3 proxy) | `cargo run --features quickjs,duckdb,parquet,private` |
|
||||
| + Python scripts | add `,python` |
|
||||
| EE features (WAP, partitioning, forks, …) | add `,enterprise,license` |
|
||||
|
||||
`enterprise` already pulls in `license`, but list both when you want the license-gated paths.
|
||||
`quickjs` is for JS eval, not DuckLake per se — keep it if your baseline build had it.
|
||||
|
||||
**Before running any DuckDB script**, build the FFI (see the bullet above):
|
||||
`cd backend/windmill-duckdb-ffi-internal && ./build_dev.sh`.
|
||||
|
||||
**Two gotchas that a wrong feature set produces:**
|
||||
|
||||
1. **`duckdb` tag advertised, feature missing.** The `duckdb` worker tag is in the *unconditional*
|
||||
default tag list (`windmill-common/src/worker.rs`, `DEFAULT_TAGS`), so a worker advertises it even
|
||||
without the `duckdb` feature. Jobs then dispatch but fail at execution with
|
||||
`"Duck DB requires the duckdb feature to be enabled"` (`windmill-worker/src/worker.rs`). Fix:
|
||||
compile with `--features duckdb`.
|
||||
2. **DuckLake writes 404 (no S3 proxy).** The workspace S3 proxy (`/w/{ws}/s3_proxy/*`) that
|
||||
DuckLake uses for reads/writes only mounts the real service under
|
||||
`#[cfg(all(feature = "private", feature = "parquet"))]` (`windmill-api/src/s3_proxy_oss.rs`);
|
||||
otherwise it's an empty router and every proxied request 404s. Fix: compile with **both**
|
||||
`private` and `parquet`.
|
||||
|
||||
## Cloud vs self-hosted gating
|
||||
|
||||
The `cloud` cargo feature is compiled into **all** EE builds, so `#[cfg(feature = "cloud")]` is **not** a "cloud-only" runtime gate — it only means the code is present. The real gate for behavior specific to the managed cloud (app.windmill.dev) is the runtime flag `*CLOUD_HOSTED` (`windmill_common::worker::CLOUD_HOSTED`, from the `CLOUD_HOSTED` env var; note it's loaded from `.env` via `dotenv`, so it won't show in `/proc/<pid>/environ` — check the running behavior, not the exec env).
|
||||
|
||||
Cloud-only logic must be behind `if *CLOUD_HOSTED { ... }`: feature-gate the helper so it compiles, then **runtime-gate the call**. `#[cfg(feature = "cloud")]` on its own is only sufficient for:
|
||||
- pure helper/struct definitions (they only run when a gated caller invokes them),
|
||||
- code already inside an `if *CLOUD_HOSTED { ... }` block,
|
||||
- handlers that early-return on `!*CLOUD_HOSTED`,
|
||||
- idempotent no-ops that are harmless off-cloud (e.g. cache invalidation).
|
||||
|
||||
## Verifying Backend Changes
|
||||
|
||||
`cargo check` and the unit tests do not exercise a worker code path. **If you changed how
|
||||
a job runs — an executor, `handle_child`, anything spawning or reading from a
|
||||
subprocess — run an actual job of that kind** and confirm it completed, then say so.
|
||||
Whole classes of defect compile and unit-test clean:
|
||||
|
||||
- **Stack overflow from a large buffer in an async block.** An array declared across an
|
||||
`.await` is baked into the future's state; once that future is boxed a few layers deep
|
||||
by the job poller, two 16 KB arrays abort the worker *process* (`thread
|
||||
'tokio-runtime-worker' has overflowed its stack`). Heap-allocate read buffers
|
||||
(`vec![0u8; N]`, not `[0u8; N]`).
|
||||
- Deadlocks from draining only one of a child's pipes, missed cancellation or timeout
|
||||
propagation, and anything depending on the real engine's output format.
|
||||
|
||||
A crash like this takes down every job on that worker, not just yours, so check the
|
||||
backend log after the run rather than only the job's own status. If you cannot run one,
|
||||
say which path went unexercised instead of implying it was verified.
|
||||
@AGENTS.md
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
# Frontend (Svelte 5)
|
||||
|
||||
- **Coding patterns**: MUST use the `svelte-frontend` skill when writing Svelte code
|
||||
- **Validation**: `docs/validation.md` — `npm run check:fast` (2s) for iteration, `npm run check` (50s) for final PR
|
||||
- **UI components**: use Windmill's design-system components — never raw HTML elements. Start from the barrel `src/lib/components/common/index.ts` and grep `src/lib/components/`; the component you need almost certainly exists
|
||||
- **Brand/design**: `frontend/brand-guidelines.md` — read the relevant section before building UI, not after; the `svelte-frontend` skill maps which section covers what
|
||||
- **Backend API**: routes in `../backend/windmill-api/openapi.yaml`, generated types in `src/lib/gen/`
|
||||
- **Regenerate client**: `npm run generate-backend-client` after backend API changes
|
||||
|
||||
## Key Frontend Patterns
|
||||
|
||||
### Prefer Composable State Over Two-Way Binding
|
||||
|
||||
```typescript
|
||||
// Use resource() from runed for async data
|
||||
import { resource } from 'runed'
|
||||
let items = resource(() => args, (args) => SomeService.list(args))
|
||||
// items.loading, items.current
|
||||
|
||||
// Use composables for shared reactive state
|
||||
function useLoader(argsGetter: () => Args) {
|
||||
let items = $state([])
|
||||
let loading = $state(false)
|
||||
$effect(() => { /* react to argsGetter() */ })
|
||||
return { get loading() { return loading }, get items() { return items } }
|
||||
}
|
||||
```
|
||||
|
||||
Two-way binding is fine for simple form inputs. Avoid it for component-to-component state.
|
||||
|
||||
## Verifying Frontend Changes
|
||||
|
||||
After modifying frontend code, drive the running dev server with the **Playwright MCP** to verify the change in a real browser — don't claim a UI change works without exercising it.
|
||||
|
||||
Two MCP servers are registered in `.mcp.json`:
|
||||
- `playwright` — headless Chromium, default for devboxes (no display required)
|
||||
- `playwright-headed` — windowed Chromium, when a display is available
|
||||
|
||||
**One-time setup:** run `npx playwright install chromium` to download the browser binary (Playwright won't fetch it automatically on first use).
|
||||
|
||||
Typical flow:
|
||||
1. Ensure backend (`cargo run`) and frontend (`REMOTE=http://localhost:8000 npm run dev`) are running
|
||||
2. `mcp__playwright__browser_navigate` to the relevant page (login at `admin@windmill.dev` / `changeme`)
|
||||
3. `mcp__playwright__browser_snapshot` to inspect the accessibility tree (preferred over screenshots for reading the DOM)
|
||||
4. `mcp__playwright__browser_click` / `browser_fill_form` / `browser_type` to interact
|
||||
5. `mcp__playwright__browser_take_screenshot` for visual confirmation
|
||||
6. `mcp__playwright__browser_console_messages` / `browser_network_requests` to surface errors
|
||||
|
||||
Write screenshots to an absolute path under `/tmp` (the MCP servers already do; standalone
|
||||
Playwright scripts must be told): moving a PNG out of the checkout afterwards needs a `mv` the
|
||||
permission hooks always prompt on. Same reason to run `rm`/`mv`/`cp` as one plain command per Bash
|
||||
call: those hooks defer on `&&`, `;`, redirects, quotes and `$VAR`.
|
||||
|
||||
**Attach the screenshots to the PR.** For any change under `frontend/`, embed screenshots of the affected UI in the PR body — the `pr` skill requires this and carries the upload recipe.
|
||||
|
||||
If you cannot exercise a UI change (no dev server, etc.), say so explicitly rather than claiming success.
|
||||
|
||||
### Traps while driving the UI
|
||||
|
||||
- `critical_alerts` 404s are expected on CE builds (EE-only endpoint) — ignore them.
|
||||
- VSCode worker 404s are dev-mode artifacts — ignore them.
|
||||
- `<Toggle>` hides its checkbox (`sr-only`). Click the `<label>` wrapper, not the checkbox.
|
||||
|
||||
## Banned Patterns
|
||||
|
||||
### `$bindable(default_value)` on optional props
|
||||
|
||||
Using `$bindable(default_value)` on props that can be `undefined` is **banned**. This pattern causes subtle bugs because the default value masks the `undefined` state.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```svelte
|
||||
let { my_prop = $bindable(default_value) }: { my_prop?: string } = $props()
|
||||
```
|
||||
|
||||
**Correct alternatives:**
|
||||
|
||||
1. **Use `$derived` with nullish coalescing** — handle the potential `undefined` at the usage site:
|
||||
|
||||
```svelte
|
||||
let { my_prop = $bindable() }: { my_prop?: string } = $props()
|
||||
let effective_value = $derived(my_prop ?? default_value)
|
||||
```
|
||||
|
||||
2. **Create a `useMyPropState()` helper** — encapsulate the undefined-handling logic in a reusable function and call it higher in the component tree, so the child component always receives a defined value.
|
||||
+1
-85
@@ -1,85 +1 @@
|
||||
# Frontend (Svelte 5)
|
||||
|
||||
- **Coding patterns**: MUST use the `svelte-frontend` skill when writing Svelte code
|
||||
- **Validation**: `docs/validation.md` — `npm run check:fast` (2s) for iteration, `npm run check` (50s) for final PR
|
||||
- **UI components**: use Windmill's design-system components — never raw HTML elements. Start from the barrel `src/lib/components/common/index.ts` and grep `src/lib/components/`; the component you need almost certainly exists
|
||||
- **Brand/design**: `frontend/brand-guidelines.md` — read the relevant section before building UI, not after; the `svelte-frontend` skill maps which section covers what
|
||||
- **Backend API**: routes in `../backend/windmill-api/openapi.yaml`, generated types in `src/lib/gen/`
|
||||
- **Regenerate client**: `npm run generate-backend-client` after backend API changes
|
||||
|
||||
## Key Frontend Patterns
|
||||
|
||||
### Prefer Composable State Over Two-Way Binding
|
||||
|
||||
```typescript
|
||||
// Use resource() from runed for async data
|
||||
import { resource } from 'runed'
|
||||
let items = resource(() => args, (args) => SomeService.list(args))
|
||||
// items.loading, items.current
|
||||
|
||||
// Use composables for shared reactive state
|
||||
function useLoader(argsGetter: () => Args) {
|
||||
let items = $state([])
|
||||
let loading = $state(false)
|
||||
$effect(() => { /* react to argsGetter() */ })
|
||||
return { get loading() { return loading }, get items() { return items } }
|
||||
}
|
||||
```
|
||||
|
||||
Two-way binding is fine for simple form inputs. Avoid it for component-to-component state.
|
||||
|
||||
## Verifying Frontend Changes
|
||||
|
||||
After modifying frontend code, drive the running dev server with the **Playwright MCP** to verify the change in a real browser — don't claim a UI change works without exercising it.
|
||||
|
||||
Two MCP servers are registered in `.mcp.json`:
|
||||
- `playwright` — headless Chromium, default for devboxes (no display required)
|
||||
- `playwright-headed` — windowed Chromium, when a display is available
|
||||
|
||||
**One-time setup:** run `npx playwright install chromium` to download the browser binary (Playwright won't fetch it automatically on first use).
|
||||
|
||||
Typical flow:
|
||||
1. Ensure backend (`cargo run`) and frontend (`REMOTE=http://localhost:8000 npm run dev`) are running
|
||||
2. `mcp__playwright__browser_navigate` to the relevant page (login at `admin@windmill.dev` / `changeme`)
|
||||
3. `mcp__playwright__browser_snapshot` to inspect the accessibility tree (preferred over screenshots for reading the DOM)
|
||||
4. `mcp__playwright__browser_click` / `browser_fill_form` / `browser_type` to interact
|
||||
5. `mcp__playwright__browser_take_screenshot` for visual confirmation
|
||||
6. `mcp__playwright__browser_console_messages` / `browser_network_requests` to surface errors
|
||||
|
||||
Write screenshots to an absolute path under `/tmp` (the MCP servers already do; standalone
|
||||
Playwright scripts must be told): moving a PNG out of the checkout afterwards needs a `mv` the
|
||||
permission hooks always prompt on. Same reason to run `rm`/`mv`/`cp` as one plain command per Bash
|
||||
call: those hooks defer on `&&`, `;`, redirects, quotes and `$VAR`.
|
||||
|
||||
**Attach the screenshots to the PR.** For any change under `frontend/`, embed screenshots of the affected UI in the PR body — the `pr` skill requires this and carries the upload recipe.
|
||||
|
||||
If you cannot exercise a UI change (no dev server, etc.), say so explicitly rather than claiming success.
|
||||
|
||||
### Traps while driving the UI
|
||||
|
||||
- `critical_alerts` 404s are expected on CE builds (EE-only endpoint) — ignore them.
|
||||
- VSCode worker 404s are dev-mode artifacts — ignore them.
|
||||
- `<Toggle>` hides its checkbox (`sr-only`). Click the `<label>` wrapper, not the checkbox.
|
||||
|
||||
## Banned Patterns
|
||||
|
||||
### `$bindable(default_value)` on optional props
|
||||
|
||||
Using `$bindable(default_value)` on props that can be `undefined` is **banned**. This pattern causes subtle bugs because the default value masks the `undefined` state.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```svelte
|
||||
let { my_prop = $bindable(default_value) }: { my_prop?: string } = $props()
|
||||
```
|
||||
|
||||
**Correct alternatives:**
|
||||
|
||||
1. **Use `$derived` with nullish coalescing** — handle the potential `undefined` at the usage site:
|
||||
|
||||
```svelte
|
||||
let { my_prop = $bindable() }: { my_prop?: string } = $props()
|
||||
let effective_value = $derived(my_prop ?? default_value)
|
||||
```
|
||||
|
||||
2. **Create a `useMyPropState()` helper** — encapsulate the undefined-handling logic in a reusable function and call it higher in the component tree, so the child component always receives a defined value.
|
||||
@AGENTS.md
|
||||
|
||||
Reference in New Issue
Block a user