diff --git a/.agents/skills/UPSTREAM.md b/.agents/skills/UPSTREAM.md new file mode 100644 index 0000000000..83ba47964e --- /dev/null +++ b/.agents/skills/UPSTREAM.md @@ -0,0 +1,63 @@ +# Vendored skills + +These five skills are copied from an external repository, not written here: + +- `grill-me`, `grilling` +- `improve-codebase-architecture`, `codebase-design`, `domain-modeling` + +Source: https://github.com/mattpocock/skills +Pinned at commit `84fdeffd12f2ee307994d1eb6feb48173b6e0502`. + +They form one dependency closure — `grill-me` is a stub that runs `grilling`, and +`improve-codebase-architecture` draws its vocabulary from `codebase-design` and its +CONTEXT.md upkeep from `domain-modeling`. Removing any one breaks the others. + +Local changes on top of upstream, kept to the minimum so a refresh stays a diff: + +- Flattened the upstream `skills/engineering/` and `skills/productivity/` split, since this + repo's skills are flat. +- Replaced each SKILL.md's markdown links to its own bundled files with plain repo-root paths + in prose (`.agents/skills//FILE.md`). Upstream's sibling-relative links break when the + file is read through the `.claude/skills//SKILL.md` symlink, which mirrors only + SKILL.md — and a repo-root *link* is equally wrong, since a markdown target resolves relative + to the file containing it. Companion files keep their sibling-relative links; they are only + ever read at their real path, never through the symlink. +- Dropped the upstream `agents/openai.yaml` files — Codex packaging metadata for that repo's + own plugin distribution, unused here. +- **Removed every ADR path.** Upstream, `domain-modeling` offers to write Architecture Decision + Records into `docs/adr/` and `improve-codebase-architecture` reads and cites them. This repo has + not adopted ADRs, and a skill that offers to create them is how the practice arrives by side + effect rather than by decision. Deleted `domain-modeling/ADR-FORMAT.md`, its "Offer ADRs + sparingly" section, and the `docs/adr/` entries in its file-structure diagrams; dropped the ADR + clauses from `improve-codebase-architecture` (intro, explore step, "ADR conflicts", the + offer-an-ADR bullet in the grilling loop) and the ADR callout row in `HTML-REPORT.md`. Also cut + "record an architectural decision" from `domain-modeling`'s description, since that phrase is an + invocation trigger. What remains is CONTEXT.md and ubiquitous-language work only. + +To refresh, diff against the same paths at a newer commit and re-apply these four changes. The +ADR removal is the one that needs judgement: if the team later adopts ADRs, take upstream's +version of those sections back rather than rewriting them here. + +## License + +MIT License + +Copyright (c) 2026 Matt Pocock + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/.agents/skills/codebase-design/DEEPENING.md b/.agents/skills/codebase-design/DEEPENING.md new file mode 100644 index 0000000000..3938457b88 --- /dev/null +++ b/.agents/skills/codebase-design/DEEPENING.md @@ -0,0 +1,37 @@ +# Deepening + +How to deepen a cluster of shallow modules safely, given its dependencies. Assumes the vocabulary in [SKILL.md](SKILL.md) — **module**, **interface**, **seam**, **adapter**. + +## Dependency categories + +When assessing a candidate for deepening, classify its dependencies. The category determines how the deepened module is tested across its seam. + +### 1. In-process + +Pure computation, in-memory state, no I/O. Always deepenable — merge the modules and test through the new interface directly. No adapter needed. + +### 2. Local-substitutable + +Dependencies that have local test stand-ins (PGLite for Postgres, in-memory filesystem). Deepenable if the stand-in exists. The deepened module is tested with the stand-in running in the test suite. The seam is internal; no port at the module's external interface. + +### 3. Remote but owned (Ports & Adapters) + +Your own services across a network boundary (microservices, internal APIs). Define a **port** (interface) at the seam. The deep module owns the logic; the transport is injected as an **adapter**. Tests use an in-memory adapter. Production uses an HTTP/gRPC/queue adapter. + +Recommendation shape: *"Define a port at the seam, implement an HTTP adapter for production and an in-memory adapter for testing, so the logic sits in one deep module even though it's deployed across a network."* + +### 4. True external (Mock) + +Third-party services (Stripe, Twilio, etc.) you don't control. The deepened module takes the external dependency as an injected port; tests provide a mock adapter. + +## Seam discipline + +- **One adapter means a hypothetical seam. Two adapters means a real one.** Don't introduce a port unless at least two adapters are justified (typically production + test). A single-adapter seam is just indirection. +- **Internal seams vs external seams.** A deep module can have internal seams (private to its implementation, used by its own tests) as well as the external seam at its interface. Don't expose internal seams through the interface just because tests use them. + +## Testing strategy: replace, don't layer + +- Old unit tests on shallow modules become waste once tests at the deepened module's interface exist — delete them. +- Write new tests at the deepened module's interface. The **interface is the test surface**. +- Tests assert on observable outcomes through the interface, not internal state. +- Tests should survive internal refactors — they describe behaviour, not implementation. If a test has to change when the implementation changes, it's testing past the interface. diff --git a/.agents/skills/codebase-design/DESIGN-IT-TWICE.md b/.agents/skills/codebase-design/DESIGN-IT-TWICE.md new file mode 100644 index 0000000000..8419ad6fa9 --- /dev/null +++ b/.agents/skills/codebase-design/DESIGN-IT-TWICE.md @@ -0,0 +1,44 @@ +# Design It Twice + +When the user wants to explore alternative interfaces for a chosen deepening candidate, use this parallel sub-agent pattern. Based on "Design It Twice" (Ousterhout) — your first idea is unlikely to be the best. + +Uses the vocabulary in [SKILL.md](SKILL.md) — **module**, **interface**, **seam**, **adapter**, **leverage**. + +## Process + +### 1. Frame the problem space + +Before spawning sub-agents, write a user-facing explanation of the problem space for the chosen candidate: + +- The constraints any new interface would need to satisfy +- The dependencies it would rely on, and which category they fall into (see [DEEPENING.md](DEEPENING.md)) +- A rough illustrative code sketch to ground the constraints — not a proposal, just a way to make the constraints concrete + +Show this to the user, then immediately proceed to Step 2. The user reads and thinks while the sub-agents work in parallel. + +### 2. Spawn sub-agents + +Spawn 3+ sub-agents in parallel. Each must produce a **radically different** interface for the deepened module. + +Prompt each sub-agent with a separate technical brief (file paths, coupling details, dependency category from [DEEPENING.md](DEEPENING.md), what sits behind the seam). The brief is independent of the user-facing problem-space explanation in Step 1. Give each agent a different design constraint: + +- Agent 1: "Minimize the interface — aim for 1–3 entry points max. Maximise leverage per entry point." +- Agent 2: "Maximise flexibility — support many use cases and extension." +- Agent 3: "Optimise for the most common caller — make the default case trivial." +- Agent 4 (if applicable): "Design around ports & adapters for cross-seam dependencies." + +Include both [SKILL.md](SKILL.md) vocabulary and CONTEXT.md vocabulary in the brief so each sub-agent names things consistently with the architecture language and the project's domain language. + +Each sub-agent outputs: + +1. Interface (types, methods, params — plus invariants, ordering, error modes) +2. Usage example showing how callers use it +3. What the implementation hides behind the seam +4. Dependency strategy and adapters (see [DEEPENING.md](DEEPENING.md)) +5. Trade-offs — where leverage is high, where it's thin + +### 3. Present and compare + +Present designs sequentially so the user can absorb each one, then compare them in prose. Contrast by **depth** (leverage at the interface), **locality** (where change concentrates), and **seam placement**. + +After comparing, give your own recommendation: which design you think is strongest and why. If elements from different designs would combine well, propose a hybrid. Be opinionated — the user wants a strong read, not a menu. diff --git a/.agents/skills/codebase-design/SKILL.md b/.agents/skills/codebase-design/SKILL.md new file mode 100644 index 0000000000..b7cedf4732 --- /dev/null +++ b/.agents/skills/codebase-design/SKILL.md @@ -0,0 +1,114 @@ +--- +name: codebase-design +description: Shared vocabulary for designing deep modules. Use when the user wants to design or improve a module's interface, find deepening opportunities, decide where a seam goes, make code more testable or AI-navigable, or when another skill needs the deep-module vocabulary. +--- + +# Codebase Design + +Design **deep modules**: a lot of behaviour behind a small interface, placed at a clean seam, testable through that interface. Use this language and these principles wherever code is being designed or restructured. The aim is leverage for callers, locality for maintainers, and testability for everyone. + +## Glossary + +Use these terms exactly — don't substitute "component," "service," "API," or "boundary." Consistent language is the whole point. + +**Module** — anything with an interface and an implementation. Deliberately scale-agnostic: a function, class, package, or tier-spanning slice. _Avoid_: unit, component, service. + +**Interface** — everything a caller must know to use the module correctly: the type signature, but also invariants, ordering constraints, error modes, required configuration, and performance characteristics. _Avoid_: API, signature (too narrow — they refer only to the type-level surface). + +**Implementation** — what's inside a module, its body of code. Distinct from **Adapter**: a thing can be a small adapter with a large implementation (a Postgres repo) or a large adapter with a small implementation (an in-memory fake). Reach for "adapter" when the seam is the topic; "implementation" otherwise. + +**Depth** — leverage at the interface: the amount of behaviour a caller (or test) can exercise per unit of interface they have to learn. A module is **deep** when a large amount of behaviour sits behind a small interface, **shallow** when the interface is nearly as complex as the implementation. + +**Seam** _(Michael Feathers)_ — a place where you can alter behaviour without editing in that place; the *location* at which a module's interface lives. Where to put the seam is its own design decision, distinct from what goes behind it. _Avoid_: boundary (overloaded with DDD's bounded context). + +**Adapter** — a concrete thing that satisfies an interface at a seam. Describes *role* (what slot it fills), not substance (what's inside). + +**Leverage** — what callers get from depth: more capability per unit of interface they learn. One implementation pays back across N call sites and M tests. + +**Locality** — what maintainers get from depth: change, bugs, knowledge, and verification concentrate in one place rather than spreading across callers. Fix once, fixed everywhere. + +## Deep vs shallow + +**Deep module** = small interface + lots of implementation: + +``` +┌─────────────────────┐ +│ Small Interface │ ← Few methods, simple params +├─────────────────────┤ +│ │ +│ Deep Implementation│ ← Complex logic hidden +│ │ +└─────────────────────┘ +``` + +**Shallow module** = large interface + little implementation (avoid): + +``` +┌─────────────────────────────────┐ +│ Large Interface │ ← Many methods, complex params +├─────────────────────────────────┤ +│ Thin Implementation │ ← Just passes through +└─────────────────────────────────┘ +``` + +When designing an interface, ask: + +- Can I reduce the number of methods? +- Can I simplify the parameters? +- Can I hide more complexity inside? + +## Principles + +- **Depth is a property of the interface, not the implementation.** A deep module can be internally composed of small, mockable, swappable parts — they just aren't part of the interface. A module can have **internal seams** (private to its implementation, used by its own tests) as well as the **external seam** at its interface. +- **The deletion test.** Imagine deleting the module. If complexity vanishes, it was a pass-through. If complexity reappears across N callers, it was earning its keep. +- **The interface is the test surface.** Callers and tests cross the same seam. If you want to test *past* the interface, the module is probably the wrong shape. +- **One adapter means a hypothetical seam. Two adapters means a real one.** Don't introduce a seam unless something actually varies across it. + +## Designing for testability + +Good interfaces make testing natural: + +1. **Accept dependencies, don't create them.** + + ```typescript + // Testable + function processOrder(order, paymentGateway) {} + + // Hard to test + function processOrder(order) { + const gateway = new StripeGateway(); + } + ``` + +2. **Return results, don't produce side effects.** + + ```typescript + // Testable + function calculateDiscount(cart): Discount {} + + // Hard to test + function applyDiscount(cart): void { + cart.total -= discount; + } + ``` + +3. **Small surface area.** Fewer methods = fewer tests needed. Fewer params = simpler test setup. + +## Relationships + +- A **Module** has exactly one **Interface** (the surface it presents to callers and tests). +- **Depth** is a property of a **Module**, measured against its **Interface**. +- A **Seam** is where a **Module**'s **Interface** lives. +- An **Adapter** sits at a **Seam** and satisfies the **Interface**. +- **Depth** produces **Leverage** for callers and **Locality** for maintainers. + +## Rejected framings + +- **Depth as ratio of implementation-lines to interface-lines** (Ousterhout): rewards padding the implementation. We use depth-as-leverage instead. +- **"Interface" as the TypeScript `interface` keyword or a class's public methods**: too narrow — interface here includes every fact a caller must know. +- **"Boundary"**: overloaded with DDD's bounded context. Say **seam** or **interface**. + +## Going deeper + +- **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. diff --git a/.agents/skills/domain-modeling/CONTEXT-FORMAT.md b/.agents/skills/domain-modeling/CONTEXT-FORMAT.md new file mode 100644 index 0000000000..eaf2a18573 --- /dev/null +++ b/.agents/skills/domain-modeling/CONTEXT-FORMAT.md @@ -0,0 +1,60 @@ +# CONTEXT.md Format + +## Structure + +```md +# {Context Name} + +{One or two sentence description of what this context is and why it exists.} + +## Language + +**Order**: +{A one or two sentence description of the term} +_Avoid_: Purchase, transaction + +**Invoice**: +A request for payment sent to a customer after delivery. +_Avoid_: Bill, payment request + +**Customer**: +A person or organization that places orders. +_Avoid_: Client, buyer, account +``` + +## Rules + +- **Be opinionated.** When multiple words exist for the same concept, pick the best one and list the others under `_Avoid_`. +- **Keep definitions tight.** One or two sentences max. Define what it IS, not what it does. +- **Only include terms specific to this project's context.** General programming concepts (timeouts, error types, utility patterns) don't belong even if the project uses them extensively. Before adding a term, ask: is this a concept unique to this context, or a general programming concept? Only the former belongs. +- **Group terms under subheadings** when natural clusters emerge. If all terms belong to a single cohesive area, a flat list is fine. + +## Single vs multi-context repos + +**Single context (most repos):** One `CONTEXT.md` at the repo root. + +**Multiple contexts:** A `CONTEXT-MAP.md` at the repo root lists the contexts, where they live, and how they relate to each other: + +```md +# Context Map + +## Contexts + +- [Ordering](./src/ordering/CONTEXT.md) — receives and tracks customer orders +- [Billing](./src/billing/CONTEXT.md) — generates invoices and processes payments +- [Fulfillment](./src/fulfillment/CONTEXT.md) — manages warehouse picking and shipping + +## Relationships + +- **Ordering → Fulfillment**: Ordering emits `OrderPlaced` events; Fulfillment consumes them to start picking +- **Fulfillment → Billing**: Fulfillment emits `ShipmentDispatched` events; Billing consumes them to generate invoices +- **Ordering ↔ Billing**: Shared types for `CustomerId` and `Money` +``` + +The skill infers which structure applies: + +- If `CONTEXT-MAP.md` exists, read it to find contexts +- If only a root `CONTEXT.md` exists, single context +- If neither exists, create a root `CONTEXT.md` lazily when the first term is resolved + +When multiple contexts exist, infer which one the current topic relates to. If unclear, ask. diff --git a/.agents/skills/domain-modeling/SKILL.md b/.agents/skills/domain-modeling/SKILL.md new file mode 100644 index 0000000000..b0372a62b8 --- /dev/null +++ b/.agents/skills/domain-modeling/SKILL.md @@ -0,0 +1,57 @@ +--- +name: domain-modeling +description: Build and sharpen a project's domain model. Use when the user wants to pin down domain terminology or a ubiquitous language, or when another skill needs to maintain the domain model. +--- + +# Domain Modeling + +Actively build and sharpen the project's domain model as you design. This is the *active* discipline — challenging terms, inventing edge-case scenarios, and writing the glossary and decisions down the moment they crystallise. (Merely *reading* `CONTEXT.md` for vocabulary is not this skill — that's a one-line habit any skill can do. This skill is for when you're changing the model, not just consuming it.) + +## File structure + +Most repos have a single context: + +``` +/ +├── CONTEXT.md +└── src/ +``` + +If a `CONTEXT-MAP.md` exists at the root, the repo has multiple contexts. The map points to where each one lives: + +``` +/ +├── CONTEXT-MAP.md +└── src/ + ├── ordering/ + │ └── CONTEXT.md + └── billing/ + └── CONTEXT.md +``` + +Create files lazily — only when you have something to write. If no `CONTEXT.md` exists, create one when the first term is resolved. + +## During the session + +### Challenge against the glossary + +When the user uses a term that conflicts with the existing language in `CONTEXT.md`, call it out immediately. "Your glossary defines 'cancellation' as X, but you seem to mean Y — which is it?" + +### Sharpen fuzzy language + +When the user uses vague or overloaded terms, propose a precise canonical term. "You're saying 'account' — do you mean the Customer or the User? Those are different things." + +### Discuss concrete scenarios + +When domain relationships are being discussed, stress-test them with specific scenarios. Invent scenarios that probe edge cases and force the user to be precise about the boundaries between concepts. + +### Cross-reference with code + +When the user states how something works, check whether the code agrees. If you find a contradiction, surface it: "Your code cancels entire Orders, but you just said partial cancellation is possible — which is right?" + +### 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 `.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. + diff --git a/.agents/skills/grill-me/SKILL.md b/.agents/skills/grill-me/SKILL.md new file mode 100644 index 0000000000..9470cfcfe2 --- /dev/null +++ b/.agents/skills/grill-me/SKILL.md @@ -0,0 +1,7 @@ +--- +name: grill-me +description: A relentless interview to sharpen a plan or design. +disable-model-invocation: true +--- + +Run a `/grilling` session. diff --git a/.agents/skills/grilling/SKILL.md b/.agents/skills/grilling/SKILL.md new file mode 100644 index 0000000000..95bd01ee90 --- /dev/null +++ b/.agents/skills/grilling/SKILL.md @@ -0,0 +1,22 @@ +--- +name: grilling +description: Grill the user relentlessly about a plan, decision, or idea. Use when the user wants to stress-test their thinking, or uses any 'grill' trigger phrases. +--- + +Interview the user relentlessly until you reach a shared understanding. Map this as a **design tree**: every decision branches into the decisions that hang off it. + +Work the tree in **rounds**. The **frontier** is every decision whose prerequisites are already settled — the questions you can ask _now_ without guessing at answers you haven't heard yet. Ask the whole frontier in one round: number each question and give your recommended answer. Then wait for the user's answers before the next round. + +Each question should be formatted like so: + +``` +❓ **Q1** - ****: + +➡️ +``` + +Each round the user answers reshapes the tree — settled decisions push the frontier outward and unblock questions that depended on them. Recompute the frontier and ask the next round. A question whose answer depends on another question still open in this round belongs to a _later_ round, not this one. + +Finding _facts_ is your job, never the user's. When a frontier question needs a fact from the environment (filesystem, tools, etc.), dispatch a sub-agent to find it — don't ask the user for anything you could look up yourself. Don't block on it: a running exploration is an unsettled prerequisite, so only the questions downstream of it wait for the sub-agent to report — ask the rest of the frontier now. The _decisions_ are the user's — put each to them and wait. + +The session is done when the frontier is empty: every branch of the design tree visited, nothing left silently assumed. Do not act on it until the user confirms you have reached a shared understanding. diff --git a/.agents/skills/improve-codebase-architecture/HTML-REPORT.md b/.agents/skills/improve-codebase-architecture/HTML-REPORT.md new file mode 100644 index 0000000000..ecec59a00f --- /dev/null +++ b/.agents/skills/improve-codebase-architecture/HTML-REPORT.md @@ -0,0 +1,122 @@ +# HTML Report Format + +The architectural review is rendered as a single self-contained HTML file in the OS temp directory. Tailwind and Mermaid both come from CDNs. Mermaid handles graph-shaped diagrams reliably; hand-built divs and inline SVG handle the more editorial visuals (mass diagrams, cross-sections). Mix the two — don't lean on Mermaid for everything, it'll start to look generic. + +## Scaffold + +```html + + + + + Architecture review — {{repo name}} + + + + + +
+
...
+
...
+
...
+
+ + +``` + +## Header + +Repo name, date, and a compact legend: solid box = module, dashed line = seam, red arrow = leakage, thick dark box = deep module. No introduction paragraph — straight into the candidates. + +## Candidate card + +The diagrams carry the weight. Prose is sparse, plain, and uses the glossary terms (from the `/codebase-design` skill) without ceremony. + +Each candidate is one `
`: + +- **Title** — short, names the deepening (e.g. "Collapse the Order intake pipeline"). +- **Badge row** — recommendation strength (`Strong` = emerald, `Worth exploring` = amber, `Speculative` = slate), plus a tag for the dependency category (`in-process`, `local-substitutable`, `ports & adapters`, `mock`). +- **Files** — monospaced list, `font-mono text-sm`. +- **Before / After diagram** — the centrepiece. Two columns, side by side. See patterns below. +- **Problem** — one sentence. What hurts. +- **Solution** — one sentence. What changes. +- **Wins** — bullets, ≤6 words each. e.g. "Tests hit one interface", "Pricing logic stops leaking", "Delete 4 shallow wrappers". + +No paragraphs of explanation. If the diagram needs a paragraph to be understood, redraw the diagram. + +## Diagram patterns + +Pick the pattern that fits the candidate. Mix them. Don't make every diagram look the same — variety is part of the point. + +### Mermaid graph (the workhorse for dependencies / call flow) + +Use a Mermaid `flowchart` or `graph` when the point is "X calls Y calls Z, and look at the mess." Wrap it in a Tailwind-styled card so it doesn't feel parachuted in. Style with classDef to colour leakage edges red and the deep module dark. Sequence diagrams work well for "before: 6 round-trips; after: 1." + +```html +
+ 
+    flowchart LR
+      A[OrderHandler] --> B[OrderValidator]
+      B --> C[OrderRepo]
+      C -.leak.-> D[PricingClient]
+      classDef leak stroke:#dc2626,stroke-width:2px;
+      class C,D leak
+
+
+``` + +### Hand-built boxes-and-arrows (when Mermaid's layout fights you) + +Modules as `
`s with borders and labels. Arrows as inline SVG `` or `` elements positioned absolutely over a relative container. Reach for this when you want the "after" diagram to feel like one thick-bordered deep module with greyed-out internals — Mermaid won't render that with the right weight. + +### Cross-section (good for layered shallowness) + +Stack horizontal bands (`h-12 border-l-4`) to show layers a call passes through. Before: 6 thin layers each doing nothing. After: 1 thick band labelled with the consolidated responsibility. + +### Mass diagram (good for "interface as wide as implementation") + +Two rectangles per module — one for interface surface area, one for implementation. Before: interface rectangle is nearly as tall as the implementation rectangle (shallow). After: interface rectangle is short, implementation rectangle is tall (deep). + +### Call-graph collapse + +Before: a tree of function calls rendered as nested boxes. After: the same tree collapsed into one box, with the now-internal calls shown faded inside it. + +## Style guidance + +- Lean editorial, not corporate-dashboard. Generous whitespace. Serif optional for headings (`font-serif` works well with stone/slate). +- Colour sparingly: one accent (emerald or indigo) plus red for leakage and amber for warnings. +- Keep diagrams ~320px tall so before/after sits comfortably side by side without scrolling. +- Use `text-xs uppercase tracking-wider` for module labels inside diagrams — they should read as schematic, not as UI. +- The only scripts are the Tailwind CDN and the Mermaid ESM import. The report is otherwise static — no app code, no interactivity beyond Mermaid's own rendering. + +## Top recommendation section + +One larger card. Candidate name, one sentence on why, anchor link to its card. That's it. + +## Tone + +Plain English, concise — but the architectural nouns and verbs come straight from the `/codebase-design` skill. Concision is not an excuse to drift. + +**Use exactly:** module, interface, implementation, depth, deep, shallow, seam, adapter, leverage, locality. + +**Never substitute:** component, service, unit (for module) · API, signature (for interface) · boundary (for seam) · layer, wrapper (for module, when you mean module). + +**Phrasings that fit the style:** + +- "Order intake module is shallow — interface nearly matches the implementation." +- "Pricing leaks across the seam." +- "Deepen: one interface, one place to test." +- "Two adapters justify the seam: HTTP in prod, in-memory in tests." + +**Wins bullets** name the gain in glossary terms: *"locality: bugs concentrate in one module"*, *"leverage: one interface, N call sites"*, *"interface shrinks; implementation absorbs the wrappers"*. Don't write *"easier to maintain"* or *"cleaner code"* — those terms aren't in the glossary and don't earn their place. + +No hedging, no throat-clearing, no "it's worth noting that…". If a sentence could be a bullet, make it a bullet. If a bullet could be cut, cut it. If a term isn't in the `/codebase-design` glossary, reach for one that is before inventing a new one. diff --git a/.agents/skills/improve-codebase-architecture/SKILL.md b/.agents/skills/improve-codebase-architecture/SKILL.md new file mode 100644 index 0000000000..488c850990 --- /dev/null +++ b/.agents/skills/improve-codebase-architecture/SKILL.md @@ -0,0 +1,68 @@ +--- +name: improve-codebase-architecture +description: Scan a codebase for deepening opportunities, present them as a visual HTML report, then grill through whichever one you pick. +disable-model-invocation: true +--- + +# Improve Codebase Architecture + +Surface architectural friction and propose **deepening opportunities** — refactors that turn shallow modules into deep ones. The aim is testability and AI-navigability. + +This command is _informed_ by the project's domain model and built on a shared design vocabulary: + +- Run the `/codebase-design` skill for the architecture vocabulary (**module**, **interface**, **depth**, **seam**, **adapter**, **leverage**, **locality**) and its principles (the deletion test, "the interface is the test surface", "one adapter = hypothetical seam, two = real"). Use these terms exactly in every suggestion — don't drift into "component," "service," "API," or "boundary." +- The domain language in `CONTEXT.md` gives names to good seams. + +## Process + +### 1. Explore + +**Scope before you scan — YAGNI.** Deepening a module pays off by making future changes to it easier, so put extra weight on the parts of the codebase that have recently changed. Decide *where* to look before you look: + +- If the user named a direction — a module, a subsystem, a pain point — take it, and skip the inference below. +- Otherwise, walk back a good stretch of the commit history (`git log --oneline`) to find the codebase's hot spots — the files and areas that keep coming up — and let those paths pull your attention first. If the changes are scattered with no clear hot spot, widen the net. + +Read the project's domain glossary (`CONTEXT.md`) first. + +Then spawn a sub-agent to walk the codebase. Don't follow rigid heuristics — explore organically and note where you experience friction: + +- Where does understanding one concept require bouncing between many small modules? +- Where are modules **shallow** — interface nearly as complex as the implementation? +- Where have pure functions been extracted just for testability, but the real bugs hide in how they're called (no **locality**)? +- Where do tightly-coupled modules leak across their seams? +- Which parts of the codebase are untested, or hard to test through their current interface? + +Apply the **deletion test** to anything you suspect is shallow: would deleting it concentrate complexity, or just move it? A "yes, concentrates" is the signal you want. + +### 2. Present candidates as an HTML report + +Write a self-contained HTML file to the OS temp directory so nothing lands in the repo. Resolve the temp dir from `$TMPDIR`, falling back to `/tmp` (or `%TEMP%` on Windows), and write to `/architecture-review-.html` so each run gets a fresh file. Open it for the user — `xdg-open ` on Linux, `open ` on macOS, `start ` on Windows — and tell them the absolute path. + +The report uses **Tailwind via CDN** for layout and styling, and **Mermaid via CDN** for diagrams where a graph/flow/sequence reliably communicates the structure. Mix Mermaid with hand-crafted CSS/SVG visuals — use Mermaid when relationships are graph-shaped (call graphs, dependencies, sequences), and hand-built divs/SVG when you want something more editorial (mass diagrams, cross-sections, collapse animations). Each candidate gets a **before/after visualisation**. Be visual. + +For each candidate, render a card with: + +- **Files** — which files/modules are involved +- **Problem** — why the current architecture is causing friction +- **Solution** — plain English description of what would change +- **Benefits** — explained in terms of locality and leverage, and how tests would improve +- **Before / After diagram** — side-by-side, custom-drawn, illustrating the shallowness and the deepening +- **Recommendation strength** — one of `Strong`, `Worth exploring`, `Speculative`, rendered as a badge + +End the report with a **Top recommendation** section: which candidate you'd tackle first and why. + +**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 `.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?" + +### 3. Grilling loop + +Once the user picks a candidate, run the `/grilling` skill to walk the decision tree with them — constraints, dependencies, the shape of the deepened module, what sits behind the seam, what tests survive. + +Side effects happen inline as decisions crystallize — run the `/domain-modeling` skill to keep the domain model current as you go: + +- **Naming a deepened module after a concept not in `CONTEXT.md`?** Add the term to `CONTEXT.md`. Create the file lazily if it doesn't exist. +- **Sharpening a fuzzy term during the conversation?** Update `CONTEXT.md` right there. +- **Want to explore alternative interfaces for the deepened module?** Run the `/codebase-design` skill and use its design-it-twice parallel sub-agent pattern. diff --git a/.agents/skills/pr/SKILL.md b/.agents/skills/pr/SKILL.md index bd40b6c472..6a72949c96 100644 --- a/.agents/skills/pr/SKILL.md +++ b/.agents/skills/pr/SKILL.md @@ -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 (AGENTS.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** — @@ -130,6 +130,10 @@ and continue once they confirm it's done. ## Review rounds (draft → ready) A PR leaves draft **only after a clean CI review round**. Never run `gh pr ready` before that. +This is the rule in every mode, autonomous included. A clean round is necessary but not always +sufficient — see "Flip, or ask first" below. The one standing exception is an explicit request to +leave that PR in draft (usually so it can be tested first) — honour it for that PR, and don't +carry it over to the next one. 1. **Trigger a round and wait for it**: launch the waiter as a background Bash task (a round takes 10–30 min; you are woken when it exits — do not stop the session or poll in the foreground while it runs): @@ -162,6 +166,77 @@ A PR leaves draft **only after a clean CI review round**. Never run `gh pr ready If any P0/P1 finding is unaddressed or the head moved for reasons other than nit fixes, do **not** post the marker or flip — run another round instead. +### A round that never starts is usually a conflict + +The review workflows don't run on a PR that cannot merge, so a round that produces no verdict is +more often a conflict with `main` than a CI outage. Check before assuming anything is broken: + +```bash +gh pr view --json mergeable,mergeStateStatus +``` + +Resolve by **merging, not rebasing** — a rebase rewrites the head SHA that round verdicts and the +clean-round marker are keyed to, invalidating work you have already paid for: + +```bash +git fetch origin main +git merge origin/main +``` + +**If that merge changed `backend/ee-repo-ref.txt`, move the EE worktree to match.** The file pins +the EE commit CE builds against, so a merge that advances it leaves the EE checkout behind what CE +now expects, and `cargo check --features private` compiles a tree neither you nor CI intends: + +```bash +git -C merge "$(tr -d '[:space:]' < backend/ee-repo-ref.txt)" +``` + +Push both, then start a fresh round — the head moved, so the earlier verdicts no longer apply. + +### Flip, or ask first + +A clean round earns the flip; it does not always earn it *unattended*. Judge the blast radius from +the diff first — `git diff --name-only main...HEAD` answers most of these. + +**Ask before flipping** when the change: + +- touches `*_ee.rs` (it spans the EE repo through symlinks and has a companion PR) +- adds a migration under `backend/migrations/` +- changes `openapi.yaml`, `openflow.openapi.yaml`, or the generated client +- touches auth, permission, or token paths +- changes shared worker infrastructure — the job poller, `handle_child`, an executor +- trips `REVIEW.md`'s "Checklist for new public surfaces" + +**Flip without asking** when it is self-contained: a single-file fix, test-only, docs-only, one +call site, no new public surface. + +Unattended (webmux oneshot) there is nobody to ask, so the judgement holds and the action +degrades: flip the self-contained ones, and leave the rest at a clean draft with a line in the PR +description saying why — `left in draft: adds a migration, wants a human look before ready`. +Don't flip a wide-blast-radius change just because the round came back clean, and don't ask a +question nobody will read. + +`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 + +Three or more rounds without a clean verdict usually means the change's shape is wrong, not that +there is an endless supply of independent bugs. The tells: + +- findings keep landing in the same files round after round +- fixing one finding creates the next +- the findings are about coupling, duplication, or state threaded through many places, rather + than logic errors + +When that pattern holds, stop running rounds — each one costs a CI cycle and is not going to +converge. Say plainly that the remaining findings look structural rather than incidental, and +name the module or seam they cluster around. With a user present, suggest they run +`/improve-codebase-architecture` over that area: it is slash-only so you cannot invoke it +yourself, and reshaping the code is a scope change they should choose. Unattended, put the +diagnosis in the PR description and stop there rather than grinding out more rounds. + ## 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. diff --git a/.agents/skills/refine/SKILL.md b/.agents/skills/refine/SKILL.md index aaf747cd29..51b29564c6 100644 --- a/.agents/skills/refine/SKILL.md +++ b/.agents/skills/refine/SKILL.md @@ -17,7 +17,6 @@ Reflect on the current session and update documentation with lessons learned. 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: diff --git a/.agents/skills/rust-backend/SKILL.md b/.agents/skills/rust-backend/SKILL.md index f0c52002bc..2c6f077f0f 100644 --- a/.agents/skills/rust-backend/SKILL.md +++ b/.agents/skills/rust-backend/SKILL.md @@ -94,6 +94,13 @@ Use `tokio::sync::mpsc` (bounded) for channels. Avoid `std::thread::sleep` in as Always use rust-analyzer LSP for go-to-definition, find-references, and type info. Do not guess at module paths. +## Feature Telemetry + +`FEATURE_USAGE_KINDS` in `windmill-api-workspaces/src/workspaces.rs` is an allowlist: a +`(feature, kind)` pair missing from it is dropped by `valid_feature_usage_event` with a bare +`continue` — no error, and the route still returns 204. Adding a counter on the frontend without +registering it here records nothing. See `docs/feature-telemetry.md`. + ## Axum Handlers Destructure extractors directly in function signatures: diff --git a/.agents/skills/svelte-frontend/SKILL.md b/.agents/skills/svelte-frontend/SKILL.md index b0c4b39939..6aceedc25c 100644 --- a/.agents/skills/svelte-frontend/SKILL.md +++ b/.agents/skills/svelte-frontend/SKILL.md @@ -7,9 +7,47 @@ description: Svelte coding guidelines for the Windmill frontend. MUST use when w 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. +## Before writing any UI (MUST) + +Do both of these before the first line of markup — not after, and not only when something +looks unfamiliar. + +**1. Find the component that already exists.** `frontend/src/lib/components/common/index.ts` +is the design-system barrel — 28 lines, read it in full. It exports far more than the three +documented below: `Alert`, `Badge`, `Breadcrumb`, `Drawer`/`DrawerContent`, `Menu`/`MenuItem`, +`Tabs`/`Tab`/`TabContent`, `Skeleton`, `FileInput`, `RadioCard`, `Section`, `Kbd`, `ActionRow`, +`ClearableInput`, `CopyButton`, `SecondsInput`, `UndoRedo`, `Url`. + +The barrel is not the full picture either: `common/` has 34 subdirectories and only 23 exports, +so `modal/`, `popup/`, `stepper/`, `tooltip/`, `checkbox/`, `table/`, `contextmenu/`, +`confirmationModal/`, `calendarPicker/`, `fileUpload/`, `toggleButton-v2/` and more exist but +must be imported by path. Selects, text inputs and melt-based primitives sit next to `common/` +in `components/select/`, `components/text_input/`, `components/meltComponents/`. + +The tree holds 1,600+ components — grep `frontend/src/lib/components` for the thing you're about +to build; it almost certainly exists. Building a new one is the last resort, not the first move. + +**2. Read the guideline for what you're building.** `frontend/brand-guidelines.md` is the +authority on how it should look and read. Don't load all 34k chars — jump to the section: + +| Building | Section to read | +|---|---| +| Any new screen or component | `# Components` (Core Rules, Quick Reference) | +| Buttons, CTAs | `## Buttons` — hierarchy matters, only one Accent per view | +| Colors, surfaces, borders | `# Color system` (Quick Reference, Do's and Don'ts) | +| Text, labels, headings | `# Typography` — note `## Text Casing`, sentence case throughout | +| Spacing, grids, page structure | `# Spacing & Layout`; `# Layout` → `## Form` for forms | +| Shadows, overlays, depth | `# Elevation` | +| Icons | `# Iconography` | +| Wording of any UI copy | `# Voice & Communication`, `# Tone of Voice` | + +Get the line range with `grep -n '^#' frontend/brand-guidelines.md`, then read just that span. + ## Windmill UI Components (MUST use) -Always use Windmill's design-system components. Never use raw HTML elements. +Always use Windmill's design-system components. Never use raw HTML elements. The three below +are the ones you'll reach for most often — they are examples, not the catalog. For anything +else, go back to the barrel and grep. ### Buttons — ` {/snippet} @@ -206,14 +215,28 @@ {/if} {#if resourceType == 'postgresql' && supabaseWizard} - - -
Connect Supabase
- + {#if wizardEnabled} + + {#await import('./workspaceSettings/SupabaseResourceConnect.svelte')} + + {:then Module} + + {/await} + {:else} + + + +
Connect Supabase
+ + {/if} {/if} 1 ? resourceType : undefined)} id="add-resource-drawer" on:close={drawer?.closeDrawer} tooltip="Resources represent connections to third party systems. Learn more on how to integrate external APIs." documentationLink="https://www.windmill.dev/docs/integrations/integrations_on_windmill" > + {#snippet titleExtra()} + {#if step > 1 && resourceType} + + {/if} + {/snippet} void } | undefined = $state(undefined) + let filter = $state('') let value: string = $state('') let valueToken: TokenResponse | undefined = undefined @@ -98,6 +109,21 @@ let connectClient: string = $state('') let connectsManual: { key: string; img?: string; instructions: string[] }[] | undefined = $state(undefined) + let resourceTypeDescriptions: Record = $state({}) + // Types made in this workspace, by the `c_` prefix the resources page adds or by the + // workspace they live in — the hub sync writes its own into `admins`, which every + // workspace reads from. `created_by` looks like the same signal but isn't: seeded hub + // types carry a username too. + let customResourceTypes: Set = $state(new Set()) + + // Hub descriptions are markdown; a row shows one line of it, where fenced blocks and + // backticks read as noise. + const plainDescription = (d: string) => + d + .replace(/```[\s\S]*?```/g, '') + .replace(/`/g, '') + .replace(/\s+/g, ' ') + .trim() let args: any = $state({}) let renderDescription = $state(true) @@ -274,6 +300,14 @@ * credentials form with the user's own credentials — even when the instance * has shared ones (the "Instance-configured OAuth APIs" section is the entry * point for those). Every other type opens the raw manual form. */ + function connectOauth(key: string) { + manual = false + connectClient = key + resourceType = stripSandboxSuffix(key) + resetClientCredentialsState() + next() + } + function selectFromOthers(key: string) { connectClient = key resourceType = key @@ -298,6 +332,8 @@ loadResourceTypes() } step = 1 //express && !manual ? 3 : 1 + // The list is keyboard-driven from the search field, so it takes focus on open. + tick().then(() => searchInput?.focus()) value = '' description = '' labels = undefined @@ -356,9 +392,13 @@ } } + // Google's terms require its own button on the control that starts the sign-in, which is + // the step-2 Connect: step 1 only picks a type, and a manual step 2 saves a resource + // without ever reaching Google. run(() => { isGoogleSignin = - step == 1 && + step == 2 && + !manual && (resourceType == 'google' || resourceType == 'gmail' || resourceType == 'gcal' || @@ -395,6 +435,30 @@ const availableRts = await ResourceService.listResourceTypeNames({ workspace: effectiveWorkspace }) + // The prefix alone identifies a workspace-made type, and it rides on the names call the + // list already needs — so the custom section survives the full list below 403ing. + customResourceTypes = new Set(availableRts.filter(isCustomResourceTypeName)) + + // Descriptions only feed search, so they are fetched off the critical path and + // allowed to fail: `resources/type/list` is not on the public app domain's route + // allow-list (`listnames` is), and it carries every type's full schema. Awaiting it + // would hold the list behind a request nothing on screen needs -- in a published + // app, behind one that is guaranteed to 403. resourceTypeDescriptions feeds a + // $derived, so search re-ranks when they land. + ResourceService.listResourceType({ workspace: effectiveWorkspace }) + .then((types) => { + resourceTypeDescriptions = Object.fromEntries( + types.filter((t) => t.description).map((t) => [t.name, t.description!]) + ) + // A type sitting in this workspace was made here too, but only the full list carries + // `workspace_id`. Inside `admins` the two are indistinguishable — every type lives + // there — so the prefix is all there is to go on. + customResourceTypes = new Set([ + ...customResourceTypes, + ...types.filter((t) => t.workspace_id && t.workspace_id !== 'admins').map((t) => t.name) + ]) + }) + .catch(() => {}) // "Others" lists every resource type — including instance-configured OAuth // providers — so any of them can also be connected with the user's own @@ -561,7 +625,9 @@ args = {} } else { getResourceTypeInfo() - getScopesAndParams() + // Awaited: the popup is built from `scopes`, so advancing before this + // resolves sends the user to an authorize url with no scope at all. + await getScopesAndParams() } step += 1 } else if (step == 2 && !manual) { @@ -868,6 +934,129 @@ let filteredConnects: { key: string }[] = $state([]) let filteredConnectsManual: { key: string; img?: string; instructions: string[] }[] = $state([]) + // uFuzzy scores the name and the description as one string, so searching "google" ranks + // every type whose description mentions Google alongside the ones named after it. Re-sort + // on which field matched, keeping uFuzzy's order within a tier. + const rank = (items: { key: string }[] | undefined) => + items && + sortResourceTypesByMatch( + items, + filter, + (x) => x.key, + (x) => resourceTypeDescriptions[x.key] + ) + let rankedConnects = $derived(rank(filteredConnects)) + let rankedConnectsManual = $derived( + rank(filteredConnectsManual) as typeof filteredConnectsManual | undefined + ) + + let searching = $derived(filter.trim() !== '') + + // Browsing, the "Others" list leads with the native database types. Searching, that + // grouping would outrank the search itself — `ms_sql_server` sorting under `mysql` on + // "sql" — so the ranked order stands on its own. + let manualOrderedKeys = $derived( + !searching + ? [ + ...(rankedConnectsManual ?? []) + .filter((x) => nativeLanguagesCategory.includes(x.key)) + .map((x) => x.key), + ...(rankedConnectsManual ?? []) + .filter((x) => !nativeLanguagesCategory.includes(x.key)) + .map((x) => x.key) + ] + : (rankedConnectsManual ?? []).map((x) => x.key) + ) + + let customKeys = $derived(manualOrderedKeys.filter((key) => customResourceTypes.has(key))) + let otherKeys = $derived(manualOrderedKeys.filter((key) => !customResourceTypes.has(key))) + + // Every row in the order it is rendered, so arrow keys walk the sections as one list. + // A provider appears in more than one, so rows are addressed by index, not by name. + let navItems = $derived([ + ...customKeys.map((key) => ({ key, oauth: false })), + ...(rankedConnects ?? []).map((x) => ({ key: x.key, oauth: true })), + ...otherKeys.map((key) => ({ key, oauth: false })) + ]) + // Both lists start undefined and render skeletons; "nothing found" only means something + // once they have landed. + let listsLoaded = $derived(rankedConnectsManual !== undefined && rankedConnects !== undefined) + let highlightedIndex = $state(-1) + const rowDomId = (index: number) => `resource-type-row-${index}` + + // Set at hover time rather than up front, so only the descriptions the row actually cut + // off carry a tooltip. + function titleIfTruncated(e: MouseEvent & { currentTarget: HTMLElement }) { + const el = e.currentTarget + el.title = el.scrollWidth > el.clientWidth ? (el.textContent?.trim() ?? '') : '' + } + const oauthRowOffset = $derived(customKeys.length) + const otherRowOffset = $derived(customKeys.length + (rankedConnects?.length ?? 0)) + + // Sections are rendered in a fixed order, so the best match is not necessarily the first + // row: rank the rows against the query to find it. + function bestMatchIndex(): number { + let best = navItems.length > 0 ? 0 : -1 + let bestRank = Infinity + navItems.forEach((item, index) => { + const rank = resourceTypeMatchRank(item.key, resourceTypeDescriptions[item.key], filter) + if (rank < bestRank) { + bestRank = rank + best = index + } + }) + return best + } + + // Filtering reshuffles the rows under the highlight: point it at the best match so Enter + // takes the top hit, and drop it entirely once the filter is cleared. + $effect(() => { + navItems + filter + untrack(() => (highlightedIndex = searching ? bestMatchIndex() : -1)) + }) + + // Scrolling rows under a resting pointer makes the browser fire `mouseenter` on each one, + // which would drag the highlight back under the cursor as the arrow keys move it. Only a + // real pointer move hands the highlight back to the mouse. + let pointerOwnsHighlight = $state(true) + + function highlightHovered(index: number) { + if (pointerOwnsHighlight) highlightedIndex = index + } + + function moveHighlight(delta: number) { + const count = navItems.length + if (count === 0) return + pointerOwnsHighlight = false + // Rows are tabbable buttons, so focus can sit on one. Enter then activates whatever is + // focused, which has to stay the highlighted row. + const rowWasFocused = document.activeElement?.id?.startsWith('resource-type-row-') ?? false + highlightedIndex = + highlightedIndex < 0 + ? delta > 0 + ? 0 + : count - 1 + : (highlightedIndex + delta + count) % count + const row = document.getElementById(rowDomId(highlightedIndex)) + row?.scrollIntoView({ block: 'nearest' }) + if (rowWasFocused) row?.focus() + } + + function onListKeydown(e: KeyboardEvent) { + if (step !== 1) return + if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { + e.preventDefault() + moveHighlight(e.key === 'ArrowDown' ? 1 : -1) + } else if (e.key === 'Enter' && (e.target as HTMLElement)?.id === SEARCH_INPUT_ID) { + // A focused row activates itself on Enter; this covers Enter typed in the search field. + const item = navItems[highlightedIndex] + if (!item) return + e.preventDefault() + item.oauth ? connectOauth(item.key) : selectFromOthers(item.key) + } + } + let editScopes = $state(false) @@ -880,118 +1069,184 @@ })) : undefined} bind:filteredItems={filteredConnects} - f={(x) => x.key} + f={(x) => resourceTypeSearchText(x.key, resourceTypeDescriptions[x.key])} /> x.key} + f={(x) => resourceTypeSearchText(x.key, resourceTypeDescriptions[x.key])} /> {#if step == 1} -
-
- - + + +
(pointerOwnsHighlight = true)} + > +
+
+ + +
+
+ + {#snippet resourceRow(key: string)} +
+
+ +
+
+
+ {resourceTypeDisplayName(key)} + {key} +
+ {#if resourceTypeDescriptions[key]} + + {plainDescription(resourceTypeDescriptions[key])} + + {/if} +
+
+ {/snippet} + + {#snippet sectionHeading(title: string, count: number)} +

+ {title}{#if searching}{count}{/if} +

+ {/snippet} + + {#snippet resourceButton(key: string, index: number, oauth: boolean)} + + {/snippet} + +
+ {#if searching && listsLoaded && navItems.length === 0} +
+ No resource type matches “{filter.trim()}” + + Search on the name, the product or what the resource holds — or sync resource types + with the hub for more. + +
+ {:else} + +
+ {#if customKeys.length > 0} +
+ {@render sectionHeading('Custom resource types', customKeys.length)} +
+ {#each customKeys as key, i} + {@render resourceButton(key, i, false)} + {/each} +
+
+ {/if} + + {#if !searching || (rankedConnects?.length ?? 0) > 0} +
+ {@render sectionHeading( + 'Instance-configured OAuth APIs', + rankedConnects?.length ?? 0 + )} +
+ {#if rankedConnects} + {#each rankedConnects as { key }, i} + {@render resourceButton(key, oauthRowOffset + i, true)} + {/each} + {:else} + {#each new Array(3) as _} + + {/each} + {/if} +
+ {#if !searching && connects && connects.filter(isSharedConnect).length == 0} +
No OAuth APIs have been set up on this instance. To add OAuth APIs, first sync + the resource types with the hub, then add OAuth configuration. See documentation +
+ {/if} +
+ {/if} + + {#if !searching || otherKeys.length > 0} +
+ {@render sectionHeading('Others', otherKeys.length)} + + {#if !searching && connectsManual && connectsManual?.length < 10} +
+ Resource types have not been synced with the hub +
+ {/if} + +
+ {#if rankedConnectsManual} + {#each otherKeys as key, i} + {@render resourceButton(key, otherRowOffset + i, false)} + {/each} + {:else} + {#each new Array(9) as _} + + {/each} + {/if} +
+
+ {/if} +
+ {/if} +
+
+ { + connectsManual = undefined + await loadResourceTypes() + connects = undefined + await loadConnects() + }} />
- -

Instance-configured OAuth APIs

-
- {#if filteredConnects} - {#each filteredConnects as { key }} - - {/each} - {:else} - {#each new Array(3) as _} - - {/each} - {/if} -
- {#if connects && connects.filter(isSharedConnect).length == 0} -
No OAuth APIs have been set up on this instance. To add OAuth APIs, first sync the resource - types with the hub, then add OAuth configuration. See documentation -
- {/if} - -

Others

- - {#if connectsManual && connectsManual?.length < 10} -
- Resource types have not been synced with the hub -
- {/if} - -
- {#if filteredConnectsManual} - {#each filteredConnectsManual as { key }} - {#if nativeLanguagesCategory.includes(key)} - - {/if} - {/each} - {/if} - {#if filteredConnectsManual} - {#each filteredConnectsManual as { key }} - {#if !nativeLanguagesCategory.includes(key)} - - - {/if} - {/each} - {:else} - {#each new Array(9) as _} - - {/each} - {/if} -
-
- { - connectsManual = undefined - await loadResourceTypes() - connects = undefined - await loadConnects() - }} - /> -
{:else if step == 2 && manual} -
+
+ {#if !emptyString(resourceTypeInfo?.description)} + + {/if}
{:else if step == 2 && !manual} {#if manual == false && resourceType != ''} @@ -1122,12 +1372,11 @@
{#if resourceTypeInfo?.description} -
-

Description

-
- -
-
+ {/if} diff --git a/frontend/src/lib/components/AppConnectLightweightResourcePicker.svelte b/frontend/src/lib/components/AppConnectLightweightResourcePicker.svelte index 53671974b2..73cc0c6633 100644 --- a/frontend/src/lib/components/AppConnectLightweightResourcePicker.svelte +++ b/frontend/src/lib/components/AppConnectLightweightResourcePicker.svelte @@ -34,9 +34,11 @@ -
+ +
{#if !express} -
+
{#if step > 2} @@ -54,14 +56,16 @@
{/if} - +
+ +
diff --git a/frontend/src/lib/components/ArgInput.svelte b/frontend/src/lib/components/ArgInput.svelte index 5ee7a5f429..8550a58b57 100644 --- a/frontend/src/lib/components/ArgInput.svelte +++ b/frontend/src/lib/components/ArgInput.svelte @@ -841,6 +841,7 @@ {disablePortal} {disabled} {prettifyHeader} + {workspace} {schema} bind:args={value} /> @@ -983,6 +984,7 @@ {disablePortal} {disabled} {prettifyHeader} + {workspace} schema={getSchemaFromProperties(itemsType?.properties)} bind:args={value[i]} /> @@ -1150,6 +1152,7 @@ {disablePortal} {disabled} {prettifyHeader} + {workspace} bind:schema={ () => ({ properties: obj.properties ?? {}, @@ -1186,6 +1189,7 @@ {disabled} {prettifyHeader} {chatInputEnabled} + {workspace} hiddenArgs={['label', 'kind']} schema={{ properties: obj.properties, @@ -1270,6 +1274,7 @@ {disablePortal} {disabled} {prettifyHeader} + {workspace} schema={{ properties, $schema: '', @@ -1301,6 +1306,7 @@ {disablePortal} {disabled} {prettifyHeader} + {workspace} schema={{ properties, order, @@ -1471,7 +1477,7 @@ /> {/if} {:else} - + {/if} {:else} {#key extra?.['minRows']} diff --git a/frontend/src/lib/components/ArrayTypeNarrowing.svelte b/frontend/src/lib/components/ArrayTypeNarrowing.svelte index 9c68f4523a..2dc5956ffd 100644 --- a/frontend/src/lib/components/ArrayTypeNarrowing.svelte +++ b/frontend/src/lib/components/ArrayTypeNarrowing.svelte @@ -25,13 +25,15 @@ } | undefined nonEmpty?: boolean | undefined + workspace?: string | undefined } let { canEditResourceType = false, originalType = undefined, itemsType = $bindable(), - nonEmpty = $bindable() + nonEmpty = $bindable(), + workspace }: Props = $props() let selected: @@ -199,6 +201,7 @@ /> {#if itemsType?.properties != undefined} { return { diff --git a/frontend/src/lib/components/AuthSettings.svelte b/frontend/src/lib/components/AuthSettings.svelte index 2ad6ab917a..c5f0a2040a 100644 --- a/frontend/src/lib/components/AuthSettings.svelte +++ b/frontend/src/lib/components/AuthSettings.svelte @@ -53,6 +53,13 @@ hideTabs = false }: Props = $props() + // The callback lands on a frontend route, so a base url that is not the origin + // the admin is browsing is almost always a misconfiguration. + let browserOrigin = typeof window !== 'undefined' ? window.location.origin : '' + let baseUrlMismatch = $derived( + !!baseUrl && !!browserOrigin && baseUrl.replace(/\/$/, '') !== browserOrigin + ) + $effect(() => { if (oauths == undefined) { oauths = {} @@ -522,6 +529,27 @@ bind:password={oauths[k]['secret']} /> +
+ Redirect URL + {#if !baseUrl} + + Set it in Core settings. The redirect url is built from it, and {k} needs the exact + value. + + {:else} + + {/if} + {#if baseUrlMismatch} + + This is built from the instance base url. Update it in Core settings if it is + wrong, or {k} will reject the callback. + + {/if} +
These credentials are for {#if !windmillBuiltins.includes(k) || (registryCcCapable(k) && registryAuthCodeCapable(k))} diff --git a/frontend/src/lib/components/BatchLoadProgress.svelte b/frontend/src/lib/components/BatchLoadProgress.svelte new file mode 100644 index 0000000000..16cf2d7212 --- /dev/null +++ b/frontend/src/lib/components/BatchLoadProgress.svelte @@ -0,0 +1,62 @@ + + +
+ Loading {itemsLabel}: {loaded} of {total}... +
+
+
+ {#if batchSize != null} + Batch size: + { + const v = parseInt(e.currentTarget.value) + if (v >= 1 && v <= maxBatchSize) { + onBatchSizeChange?.(v) + } else { + e.currentTarget.value = String(batchSize) + } + } + }} + /> + {/if} + +
diff --git a/frontend/src/lib/components/CenteredModal.svelte b/frontend/src/lib/components/CenteredModal.svelte index a72df74002..e3f8ea1604 100644 --- a/frontend/src/lib/components/CenteredModal.svelte +++ b/frontend/src/lib/components/CenteredModal.svelte @@ -1,14 +1,12 @@ -
+
{#if asPlainText}

{md}

{:else} diff --git a/frontend/src/lib/components/InputTransformForm.svelte b/frontend/src/lib/components/InputTransformForm.svelte index 3f81c821de..630d052ce4 100644 --- a/frontend/src/lib/components/InputTransformForm.svelte +++ b/frontend/src/lib/components/InputTransformForm.svelte @@ -94,6 +94,7 @@ allowedAiTransforms?: string[] | undefined s3StorageConfigured?: boolean chatInputEnabled?: boolean + workspace?: string | undefined } let { @@ -131,7 +132,8 @@ isAgentTool = false, allowedAiTransforms = isAgentTool ? undefined : [], s3StorageConfigured = true, - chatInputEnabled = false + chatInputEnabled = false, + workspace }: Props = $props() let monaco: SimpleEditor | undefined = $state(undefined) @@ -882,6 +884,7 @@ {:else if (propertyType === undefined || propertyType == 'static') && schema?.properties?.[argName]} { @@ -81,8 +85,8 @@ async function checkS3Storage() { try { - if ($workspaceStore) { - const settings = await WorkspaceService.getPublicSettings({ workspace: $workspaceStore }) + if (ws) { + const settings = await WorkspaceService.getPublicSettings({ workspace: ws }) s3StorageConfigured = settings.large_file_storage?.s3_resource_path !== undefined } } catch (error) { @@ -146,6 +150,7 @@ {allowedAiTransforms} {s3StorageConfigured} {chatInputEnabled} + {workspace} otherArgs={Object.fromEntries( Object.entries(args ?? {}).filter(([key]) => key !== argName) )} @@ -168,7 +173,7 @@ itemName="Variable" extraField="path" loadItems={async () => - (await VariableService.listVariable({ workspace: $workspaceStore ?? '' })).map((x) => ({ + (await VariableService.listVariable({ workspace: ws ?? '' })).map((x) => ({ name: x.path, ...x }))} @@ -189,4 +194,4 @@ {/snippet} - + diff --git a/frontend/src/lib/components/InstanceSettings.svelte b/frontend/src/lib/components/InstanceSettings.svelte index bbe5432f06..62bbe1ef93 100644 --- a/frontend/src/lib/components/InstanceSettings.svelte +++ b/frontend/src/lib/components/InstanceSettings.svelte @@ -1,5 +1,10 @@ + -
- {#if autoRedirecting} -

Signing you in…

- {/if} -
+ +{#snippet errorMessage()} + +{/snippet} + + +{#snippet lastUsedBadge()} + +
+ + Last used + +
+{/snippet} + +{#snippet providerButtons()} +
{#if !logins} {#each Array(4) as _} {/each} {:else} - {#each providers as { type, icon }} - {#if logins?.some((login) => login.type === type)} + {#each orderedThirdParty as entry (entry.method.kind === 'saml' ? 'saml:' : `oauth:${entry.method.provider}`)} +
+ {#if sameLoginMethod(lastUsed, entry.method)} + {@render lastUsedBadge()} + {/if} - {/if} +
{/each} - {#each logins.filter((login) => !providersType?.includes(login.type)) as login} - - {/each} - {/if} - {#if saml} - {/if}
- {#if !autoRedirecting && !disablePasswordLogin && (saml || (logins && logins.length > 0))} -
0 ? 'mt-6' : '')}> - -
+{/snippet} + +{#snippet orDivider()} +
+
+ or +
+
+{/snippet} + +
+ {#if autoRedirecting} +

Signing you in…

+ {/if} + + {#if !passwordFirst} + {@render providerButtons()} + {#if !autoRedirecting && !disablePasswordLogin && (saml || (logins && logins.length > 0))} + {@render orDivider()} + + {#if !showPassword} +
+ +
+ {/if} + {/if} {/if} {#if !autoRedirecting && showPassword && !disablePasswordLogin} @@ -517,33 +725,73 @@ Welcome! Default credentials admin@windmill.dev / changeme have been prefilled.

{/if} -
- {#if isCloudHosted()} +
+ {#if cloudHosted}

To get credentials without the OAuth providers above, send an email at contact@windmill.dev

{/if} -
- -
- +
+
+ +
+ { + // Only move on once the field holds something: while the browser's + // credential dropdown is open, Enter belongs to the dropdown + if (e.key === 'Enter' && !e.isComposing && !e.repeat && e.currentTarget.value) { + e.preventDefault() + passwordField?.focus() + } + } + }} + /> +
+ +
+ +
+ +
+ +
+ {@render errorMessage()}
-
- -
- -
+ {/if} + + {#if passwordFirst && (saml || (logins && logins.length > 0))} + {@render orDivider()} + {@render providerButtons()} + {/if}
diff --git a/frontend/src/lib/components/LoginHeading.svelte b/frontend/src/lib/components/LoginHeading.svelte new file mode 100644 index 0000000000..478a5e3a53 --- /dev/null +++ b/frontend/src/lib/components/LoginHeading.svelte @@ -0,0 +1,28 @@ + + + +
+ {#if hasThirdParty !== undefined} +

+ {hasThirdParty ? `Log in or sign up to ${instanceName}` : `Log in to ${instanceName}`} +

+

+ {hasThirdParty + ? 'Log in or sign up with any of the methods below' + : 'Log in with your email and password'} +

+ {/if} +
diff --git a/frontend/src/lib/components/LoginPageHeader.svelte b/frontend/src/lib/components/LoginPageHeader.svelte index d841f48aa1..7b49c97e34 100644 --- a/frontend/src/lib/components/LoginPageHeader.svelte +++ b/frontend/src/lib/components/LoginPageHeader.svelte @@ -1,11 +1,34 @@ - -
-
+
+ +
+ {#if showBrand} + {#if $whitelabelNameStore} + {capitalize($whitelabelNameStore)} + {:else} + + Windmill + {/if} + {/if} +
+ +
diff --git a/frontend/src/lib/components/ModulePreviewForm.svelte b/frontend/src/lib/components/ModulePreviewForm.svelte index b8867c75e4..88b99a05c5 100644 --- a/frontend/src/lib/components/ModulePreviewForm.svelte +++ b/frontend/src/lib/components/ModulePreviewForm.svelte @@ -13,6 +13,7 @@ import type SimpleEditor from './SimpleEditor.svelte' import { getResourceTypes } from './resourceTypesStore' import { twMerge } from 'tailwind-merge' + import { workspaceStore } from '$lib/stores' interface Props { schema: Schema | { properties?: Record; required?: string[] } @@ -32,9 +33,11 @@ focusArg = undefined }: Props = $props() - const { stepsInputArgs, flowStateStore, flowStore, previewArgs } = + const { stepsInputArgs, flowStateStore, flowStore, previewArgs, opWorkspace } = getContext('FlowEditorContext') + let opWs = $derived(opWorkspace?.() ?? $workspaceStore) + let inputCheck: { [id: string]: boolean } = $state({}) $effect(() => { isValid = allTrue(inputCheck) ?? false @@ -152,6 +155,7 @@ nullable={schema.properties[argName].nullable} title={schema.properties[argName].title} placeholder={schema.properties[argName].placeholder} + workspace={opWs} > {#snippet fieldHeaderActions()} {#if stepsInputArgs?.isArgManuallySet(mod.id, argName)} diff --git a/frontend/src/lib/components/ModulePreviewResultViewer.svelte b/frontend/src/lib/components/ModulePreviewResultViewer.svelte index 6a0fef6f79..3f39531b0a 100644 --- a/frontend/src/lib/components/ModulePreviewResultViewer.svelte +++ b/frontend/src/lib/components/ModulePreviewResultViewer.svelte @@ -85,8 +85,12 @@ bind:this={outputPickerInner} > {#snippet copilot_fix()} - {#if lang && editor && diffEditor && stepsInputArgs.getStepArgs(mod.id) && selectedJob?.type === 'CompletedJob' && !selectedJob.success && getStringError(selectedJob.result)} - + {@const stepError = + selectedJob?.type === 'CompletedJob' && !selectedJob.success + ? getStringError(selectedJob.result) + : undefined} + {#if lang && editor && diffEditor && stepsInputArgs.getStepArgs(mod.id) && stepError} + {/if} {/snippet} diff --git a/frontend/src/lib/components/Password.svelte b/frontend/src/lib/components/Password.svelte index 12c374bf70..569150f7b4 100644 --- a/frontend/src/lib/components/Password.svelte +++ b/frontend/src/lib/components/Password.svelte @@ -4,6 +4,7 @@ import Button from './common/button/Button.svelte' import TextInput from './text_input/TextInput.svelte' import { Eye, EyeClosed } from 'lucide-svelte' + import type { HTMLInputAttributes } from 'svelte/elements' const bubble = createBubbler() interface Props { @@ -14,6 +15,13 @@ small?: boolean minRows?: number id?: string + autocomplete?: HTMLInputAttributes['autocomplete'] + /** Off for login-style fields: keeps Enter free to submit. Overrides `minRows`. */ + allowMultiline?: boolean + /** Renders the field in its error state; the message itself is the caller's to display. */ + error?: boolean + /** id of the element holding that message, wired up as aria-describedby. */ + describedBy?: string onKeyDown?: (event: KeyboardEvent) => void onBlur?: (event: FocusEvent) => void } @@ -26,18 +34,35 @@ small = false, minRows, id, + autocomplete = 'new-password', + allowMultiline = true, + error = false, + describedBy = undefined, onKeyDown, onBlur }: Props = $props() let red = $derived(required && (password == '' || password == undefined)) + let hasError = $derived(red || error) let hideValue = $state(true) let forceMultiline = $state(false) let isMultiline = $derived( - forceMultiline || (minRows != null && minRows > 1) || (password?.includes('\n') ?? false) + allowMultiline && + (forceMultiline || (minRows != null && minRows > 1) || (password?.includes('\n') ?? false)) ) let textareaRef: TextInput<'textarea'> | undefined = $state() + let inputRef: TextInput<'input'> | undefined = $state() + + export function focus() { + ;(isMultiline ? textareaRef : inputRef)?.focus() + } + + // Revealing swaps the input to type="text". Auth forms conceal again before submitting, + // so the browser sees a password field when it decides whether to save the credential. + export function conceal() { + hideValue = true + } function insertAndSwitchToMultiline(input: HTMLInputElement, text: string) { const start = input.selectionStart @@ -54,21 +79,11 @@
-
-
{#if isMultiline} onBlur?.(e), onkeydown: (e) => { onKeyDown?.(e) @@ -89,17 +106,20 @@ /> {:else} onBlur?.(e), onkeydown: (e) => { - if (e.key === 'Enter') { + if (allowMultiline && e.key === 'Enter') { e.preventDefault() insertAndSwitchToMultiline(e.currentTarget as HTMLInputElement, '\n') return @@ -109,7 +129,7 @@ }, onpaste: (e) => { const text = e.clipboardData?.getData('text') - if (text?.includes('\n')) { + if (allowMultiline && text?.includes('\n')) { e.preventDefault() insertAndSwitchToMultiline(e.currentTarget as HTMLInputElement, text) } @@ -119,6 +139,18 @@ class="pr-8" /> {/if} + +
+
{#if red}
This field is required
diff --git a/frontend/src/lib/components/PasswordArgInput.svelte b/frontend/src/lib/components/PasswordArgInput.svelte index 60474e53cb..a45ad6579d 100644 --- a/frontend/src/lib/components/PasswordArgInput.svelte +++ b/frontend/src/lib/components/PasswordArgInput.svelte @@ -2,6 +2,7 @@ import { VariableService } from '$lib/gen' import { userStore, workspaceStore } from '$lib/stores' import { generateRandomString } from '$lib/utils' + import { sendUserToast } from '$lib/toast' import { Button } from './common' import Password from './Password.svelte' import { untrack } from 'svelte' @@ -10,14 +11,28 @@ value?: string | undefined disabled: boolean minRows?: number + /** Workspace the ephemeral secret is minted in; defaults to the nav workspace. + * Session editors pass their acting workspace. */ + workspace?: string | undefined } - let { value = $bindable(undefined), disabled, minRows }: Props = $props() + let { value = $bindable(undefined), disabled, minRows, workspace }: Props = $props() + + let ws = $derived(workspace ?? $workspaceStore) let path = $state('') - let password = $state( - value && typeof value === 'string' && !value.startsWith('$var:') ? value : '' - ) + // Workspace the variable at `path` actually lives in; `ws` can move away from it. + let mintedIn = $state(undefined) + // What the field mints from: an argument already holding a `$var:` ref has nothing to mint. + function plaintextOf(v: unknown): string { + return typeof v === 'string' && v !== '' && !v.startsWith('$var:') ? v : '' + } + let password = $state(plaintextOf(value)) + + // The argument no longer holds what this field would mint from — a parent can replace the whole + // args object without remounting it (previewing a saved input, say). Minting now would describe a + // secret the argument does not point at, and binding it would discard the replacement. + let argReplaced = $derived(path !== '' && value !== '$var:' + path) let isGenerating = false @@ -25,13 +40,15 @@ 'u/' + ($userStore?.username ?? $userStore?.email)?.split('@')[0] + '/secret_arg/' ) async function generateValue() { - if (isGenerating) return + if (isGenerating || argReplaced) return isGenerating = true + const mintWs = ws! + const boundBefore = value try { let npath = userPrefix + generateRandomString(12) let nvalue = '$var:' + npath await VariableService.createVariable({ - workspace: $workspaceStore!, + workspace: mintWs, requestBody: { value: password, is_secret: true, @@ -40,26 +57,49 @@ expires_at: new Date(Date.now() + 1000 * 60 * 60 * 24 * 7).toISOString() } }) + // The arg can be replaced the same way while the create is in flight. Nothing ever + // referenced the variable just minted, so delete it; it expires on its own if that fails. + if (value !== boundBefore) { + VariableService.deleteVariable({ workspace: mintWs, path: npath }).catch(() => {}) + return + } path = npath + mintedIn = mintWs console.log('generated', nvalue) value = nvalue debouncedUpdate() } finally { + // Ended without binding: discarded just above, or the create failed after the argument + // moved. The field would otherwise keep showing a secret the argument does not hold, and + // the mint effect tracks `ws` — a workspace move would bind that stale plaintext over the + // replacement. Re-seeding leaves the field describing the argument again. + if (path === '' && value !== boundBefore) { + password = plaintextOf(value) + } isGenerating = false } } async function updateValue() { + // The first keystroke queues an update before anything is minted: letting it run would 404 and + // retry the mint, binding over an argument that was replaced while the first mint was in flight. + if (path === '') return + const updating = path try { await VariableService.updateVariable({ - workspace: $workspaceStore!, + workspace: mintedIn ?? ws!, path: path, requestBody: { value: password } }) } catch (e) { - generateValue() + // A re-mint can bind a fresh variable while this update is in flight; recovering then + // would orphan the one it just bound. + if (path !== updating) return + generateValue().catch((e) => + sendUserToast(`Could not create the secret: ${e?.body ?? e?.message ?? e}`, true) + ) } } @@ -74,11 +114,31 @@ }) $effect(() => { - $workspaceStore && + ws && ($userStore?.username || $userStore?.email) && path == '' && password != '' && - untrack(() => generateValue()) + untrack(() => + // A failed mint leaves the plaintext bound to nothing and the argument empty. Only a + // further keystroke re-runs this, so say so rather than submitting the job without it. + generateValue().catch((e) => + sendUserToast(`Could not create the secret: ${e?.body ?? e?.message ?? e}`, true) + ) + ) + }) + + // The operating workspace can move after minting (a session forking, say), leaving the + // variable behind where the job will not find it: mint a fresh one in the new workspace. + // Bounded to a live instance: a field mounted onto an existing `$var:` holds neither the + // plaintext nor the workspace it was minted in, so it can only be moved by retyping it. + $effect(() => { + const cur = ws + if (!cur || path === '' || password === '' || mintedIn === cur || argReplaced) return + untrack(() => + generateValue().catch((e) => + sendUserToast(`Could not create the secret in ${cur}: ${e?.body ?? e?.message ?? e}`, true) + ) + ) }) diff --git a/frontend/src/lib/components/Path.svelte b/frontend/src/lib/components/Path.svelte index 284677ce7c..37eef75bf5 100644 --- a/frontend/src/lib/components/Path.svelte +++ b/frontend/src/lib/components/Path.svelte @@ -85,6 +85,13 @@ * workspace when the editor operates on a workspace other than the one the * top nav points at (see the sessions preview / dev-workspace flows). */ workspaceOverride?: string + /** One path that does not count as taken, for a caller creating something that may + * already have written there itself — a setup flow correcting its own failed attempt. + * Every other existing path is still refused. */ + allowedExistingPath?: string + /** Show the "moving may break other items" warning on a rename. Off for items nothing + * can reference by path and whose dependents move with them (eval datasets). */ + warnOnRename?: boolean } let { @@ -102,7 +109,9 @@ disableEditing = false, size = 'md', drawerOffset = 0, - workspaceOverride = undefined + workspaceOverride = undefined, + allowedExistingPath = undefined, + warnOnRename = true }: Props = $props() let ws = $derived(workspaceOverride ?? $workspaceStore) @@ -240,6 +249,7 @@ } validateTimeout = setTimeout(async () => { if ( + path !== allowedExistingPath && (path == '' || checkInitialPathExistence || path != initialPath) && (await pathExists(path, kind)) ) { @@ -420,8 +430,13 @@ }) } }) + // Nothing depends on an item that does not exist yet, so editing a *suggested* path is not a + // rename. `checkInitialPathExistence` is what callers set when they are creating something, + // which is the same question asked the other way round. let displayPathChangedWarning = $derived( - (['flow', 'script', 'resource', 'variable'] as PathKind[]).includes(kind) && + warnOnRename && + (['flow', 'script', 'resource', 'variable'] as PathKind[]).includes(kind) && + !checkInitialPathExistence && initialPath && initialPath !== path ) diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index 8b10841512..367888655f 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -154,12 +154,6 @@ ) }) - let linkedVars = $derived( - Object.entries(current?.args ?? {}) - .filter(([_, v]) => typeof v == 'string' && v == `$var:${initialPath}`) - .map(([k, _]) => k) - ) - const dirtyWorkspaces = $derived( Object.keys(states).filter((ws) => !draftValuesEqual(states[ws].draft, initialStates[ws])) ) @@ -312,16 +306,19 @@ }) }) - $effect(() => { + /** Sole writer of `current.path` — an arg still holding `$var:` is the resource's own linked secret, which the backend renames + * along with the resource, so the reference moves with it. An arg pointing at + * any other variable was set by the user and is left alone. */ + function setPath(npath: string): void { if (!current) return - if (linkedVars.length > 0 && current.path) { - untrack(() => { - linkedVars.forEach((k) => { - current!.args[k] = `$var:${current!.path}` - }) - }) + const prev = current.path + // `args` is whatever the raw JSON editor parsed — `null` included. + for (const [k, v] of Object.entries(current.args ?? {})) { + if (v === `$var:${prev}`) current.args[k] = `$var:${npath}` } - }) + current.path = npath + } export async function save(): Promise { const dirty = dirtyWorkspaces @@ -388,7 +385,7 @@ {#if current} {#key current} current!.path, setPath} bind:labels={current.labels} bind:description={current.description} bind:args={current.args} diff --git a/frontend/src/lib/components/ResourceEditorDrawer.svelte b/frontend/src/lib/components/ResourceEditorDrawer.svelte index 6c0da69675..eec4c7579b 100644 --- a/frontend/src/lib/components/ResourceEditorDrawer.svelte +++ b/frontend/src/lib/components/ResourceEditorDrawer.svelte @@ -3,17 +3,30 @@ import DrawerContent from './common/drawer/DrawerContent.svelte' - import { Loader2, Save } from 'lucide-svelte' + import { History, Loader2, Save } from 'lucide-svelte' import WsSpecificVersions from './WsSpecificVersions.svelte' - import { workspaceStore } from '$lib/stores' + import { userStore, workspaceStore } from '$lib/stores' + import { isOwner } from '$lib/utils' import LocalDraftBanner from './LocalDraftBanner.svelte' + import OpenInSessionButton from './sessions/OpenInSessionButton.svelte' + import { + clearPageDrawerAnchor, + pageDrawerSessionSource, + setPageDrawerAnchor + } from './sessions/pageDrawerSession' + import { RESOURCES_PATH } from './sessions/previewPaths' + import ResourceVersionHistory from './ResourceVersionHistory.svelte' + import IconedResourceType from './IconedResourceType.svelte' + import { addResourceTitle } from './resourceTypeDisplay' let { workspace = undefined, - disableChatOffset = false - }: { workspace?: string; disableChatOffset?: boolean } = $props() + disableChatOffset = false, + onRestored = undefined + }: { workspace?: string; disableChatOffset?: boolean; onRestored?: () => void } = $props() let drawer: Drawer | undefined = $state() + let historyDrawer: Drawer | undefined = $state() let canSave = $state(true) let resource_type: string | undefined = $state(undefined) let defaultValues: Record | undefined = $state(undefined) @@ -33,12 +46,23 @@ let selected: string | undefined = $state(undefined) let effectiveWorkspace = $derived(workspace ?? $workspaceStore!) + // The editor renders whichever workspace-specific variant `selected` points at, so history has + // to follow it too — otherwise a restore would write over the variant the user is not looking at. + let historyWorkspace = $derived(selected ?? effectiveWorkspace) + // Clearing is irreversible and the backend gates it on ownership, not write access. $userStore + // describes the user in the workspace they are signed into, so it can only answer for that one: + // history pointed anywhere else — a ws-specific variant, or an explicit `workspace` prop — gets + // no Clear button rather than a verdict computed from the wrong membership. + let canClearSelected = $derived( + historyWorkspace === $workspaceStore && isOwner(path ?? '', $userStore, $workspaceStore) + ) export async function initEdit(p: string): Promise { resource_type = undefined path = p selected = effectiveWorkspace drawer?.openDrawer?.() + setPageDrawerAnchor(RESOURCES_PATH, p) } export async function initNew( @@ -53,14 +77,30 @@ } let mode: 'edit' | 'new' = $derived(!path ? 'new' : 'edit') + + // `selected`, not `effectiveWorkspace`: WsSpecificVersions re-points this drawer + // at another workspace's version, and the session must act on the one shown. + const sessionSource = $derived( + pageDrawerSessionSource(RESOURCES_PATH, path, selected ?? effectiveWorkspace) + ) - + clearPageDrawerAnchor(RESOURCES_PATH)} +> + {#snippet titleExtra()} + {#if mode == 'new' && resource_type} + + {/if} + {/snippet} {#await import('./ResourceEditor.svelte')} {:then Module} @@ -88,7 +128,16 @@ /> {/snippet} {#snippet actions()} + {#if mode == 'edit' && path && effectiveWorkspace} + + + + + {#if path && historyWorkspace} + { + historyDrawer?.closeDrawer() + // Close the editor too. It holds a baseline captured before the restore, and + // any local draft on top of it, so saving from it afterwards would write the + // pre-restore value straight back over the version just restored. + drawer?.closeDrawer() + // Its own callback rather than the `refresh` event: callers bind that to + // reopening a picker (EditorBar), which a restore should not trigger. + onRestored?.() + }} + /> + {/if} + + diff --git a/frontend/src/lib/components/ResourceForm.svelte b/frontend/src/lib/components/ResourceForm.svelte index 3820004593..cddd83aae7 100644 --- a/frontend/src/lib/components/ResourceForm.svelte +++ b/frontend/src/lib/components/ResourceForm.svelte @@ -1,6 +1,8 @@ +{#if !emptyString(resourceTypeInfo?.description)} + +{/if} + {#if !hidePath}
{#if !can_write} @@ -136,6 +156,7 @@
{/if}
@@ -921,35 +935,16 @@
{#if batchProgress} -
- Loading jobs: {batchProgress.loaded} of {batchProgress.total}... -
-
-
- {#if currentBatchSize != null} - Batch size: - { - const v = parseInt(e.currentTarget.value) - if (v >= 1 && v <= 1000) { - jobsLoader.restreamWithBatchSize(v) - } - }} - /> - {/if} - +
+ jobsLoader.restreamWithBatchSize(v)} + onStop={() => jobsLoader.stopBatchLoading()} + />
{/if} diff --git a/frontend/src/lib/components/S3FilePreview.svelte b/frontend/src/lib/components/S3FilePreview.svelte index a9c168d68c..62c95581e7 100644 --- a/frontend/src/lib/components/S3FilePreview.svelte +++ b/frontend/src/lib/components/S3FilePreview.svelte @@ -97,13 +97,17 @@ function isNotFoundError(err: any): boolean { // HelpersService surfaces backend errors as ApiError with a `status` - // field plus a serialized body. We accept either a 404 status or a - // "not found" substring (case-insensitive) to be robust against - // future error wrapping changes. + // field plus a serialized body. A missing object arrives as a 500 that + // merely *says* "not found" (`load_file_metadata` wraps the object-store + // error), so the substring test carries this and cannot be dropped. 400 + // must short-circuit ahead of it: those messages echo back a + // caller-supplied storage name, and one like `archive not found` would + // otherwise read as a missing object and hide the diagnostic. const status = err?.status ?? err?.response?.status if (status === 404) return true + if (status === 400) return false const body = String(err?.body ?? err?.message ?? err ?? '').toLowerCase() - return body.includes('not found') || body.includes('404') + return body.includes('not found') } // Reload whenever the file key, workspace, or external refreshKey diff --git a/frontend/src/lib/components/SchemaForm.svelte b/frontend/src/lib/components/SchemaForm.svelte index 4df7b4dc0b..0bd2f23e61 100644 --- a/frontend/src/lib/components/SchemaForm.svelte +++ b/frontend/src/lib/components/SchemaForm.svelte @@ -363,127 +363,122 @@ />
{/if} - -
{ - dispatch('click', argName) - }} - > - {#if args && typeof args == 'object' && prop} - - {#if !hidden[argName]} - { - dispatch('change') - }} - on:nestedChange={() => { - dispatch('nestedChange') - }} - on:acceptChange={(e) => dispatch('acceptChange', e.detail)} - on:rejectChange={(e) => dispatch('rejectChange', e.detail)} - on:keydownCmdEnter={() => dispatch('keydownCmdEnter')} - {disablePortal} - {resourceTypes} - {prettifyHeader} - autofocus={i == 0 && autofocus ? true : null} - label={argName} - description={prop?.description} - bind:value={args[argName]} - type={prop?.type} - oneOf={prop?.oneOf} - required={schema?.required?.includes(argName)} - pattern={prop?.pattern} - bind:valid={inputCheck[argName]} - defaultValue={defaultValues?.[argName] ?? - structuredClone($state.snapshot(prop?.default))} - enum_={dynamicEnums?.[argName] ?? prop?.enum} - format={prop?.format} - contentEncoding={prop?.contentEncoding} - customErrorMessage={prop?.customErrorMessage} - bind:properties={ - () => prop?.properties, - (v) => { - if (prop) prop.properties = v - } + + {#if args && typeof args == 'object' && prop && !hidden[argName]} + +
{ + dispatch('click', argName) + }} + > + { + dispatch('change') + }} + on:nestedChange={() => { + dispatch('nestedChange') + }} + on:acceptChange={(e) => dispatch('acceptChange', e.detail)} + on:rejectChange={(e) => dispatch('rejectChange', e.detail)} + on:keydownCmdEnter={() => dispatch('keydownCmdEnter')} + {disablePortal} + {resourceTypes} + {prettifyHeader} + autofocus={i == 0 && autofocus ? true : null} + label={argName} + description={prop?.description} + bind:value={args[argName]} + type={prop?.type} + oneOf={prop?.oneOf} + required={schema?.required?.includes(argName)} + pattern={prop?.pattern} + bind:valid={inputCheck[argName]} + defaultValue={defaultValues?.[argName] ?? + structuredClone($state.snapshot(prop?.default))} + enum_={dynamicEnums?.[argName] ?? prop?.enum} + format={prop?.format} + contentEncoding={prop?.contentEncoding} + customErrorMessage={prop?.customErrorMessage} + bind:properties={ + () => prop?.properties, + (v) => { + if (prop) prop.properties = v } - bind:order={ - () => prop?.order, - (v) => { - if (prop) prop.order = v - } + } + bind:order={ + () => prop?.order, + (v) => { + if (prop) prop.order = v } - nestedRequired={prop?.required} - itemsType={prop?.items} - disabled={disabledArgs.includes(argName) || disabled || prop?.disabled} - {compact} - {variableEditor} - {itemPicker} - bind:pickForField - password={linkedSecrets.includes(argName)} - extra={prop} - {showSchemaExplorer} - simpleTooltip={schemaFieldTooltip[argName]} - {onlyMaskPassword} - nullable={prop?.nullable} - title={prop?.title} - placeholder={prop?.placeholder} - orderEditable={dndConfig != undefined} - otherArgs={{ ...args, [argName]: undefined }} - {helperScript} - {lightHeader} - diffStatus={diff[argName] ?? undefined} - {nestedParent} - {shouldDispatchChanges} - {nestedClasses} - {appPath} - {computeS3ForceViewerPolicies} - {workspace} - {css} - {displayType} - > - {#snippet actions()} - {@render actions_render?.({ item })} - {#if linkedSecretCandidates?.includes(argName)} -
- { - if (e.detail === 'secret') { - if (!linkedSecrets.includes(argName)) { - linkedSecrets = [...linkedSecrets, argName] - } - } else { - linkedSecrets = linkedSecrets.filter((s) => s !== argName) + } + nestedRequired={prop?.required} + itemsType={prop?.items} + disabled={disabledArgs.includes(argName) || disabled || prop?.disabled} + {compact} + {variableEditor} + {itemPicker} + bind:pickForField + password={linkedSecrets.includes(argName)} + extra={prop} + {showSchemaExplorer} + simpleTooltip={schemaFieldTooltip[argName]} + {onlyMaskPassword} + nullable={prop?.nullable} + title={prop?.title} + placeholder={prop?.placeholder} + orderEditable={dndConfig != undefined} + otherArgs={{ ...args, [argName]: undefined }} + {helperScript} + {lightHeader} + diffStatus={diff[argName] ?? undefined} + {nestedParent} + {shouldDispatchChanges} + {nestedClasses} + {appPath} + {computeS3ForceViewerPolicies} + {workspace} + {css} + {displayType} + > + {#snippet actions()} + {@render actions_render?.({ item })} + {#if linkedSecretCandidates?.includes(argName)} +
+ { + if (e.detail === 'secret') { + if (!linkedSecrets.includes(argName)) { + linkedSecrets = [...linkedSecrets, argName] } - }} - > - {#snippet children({ item })} - - - {/snippet} - -
{/if} - {/snippet} - - {/if} - - - {/if} -
+ } else { + linkedSecrets = linkedSecrets.filter((s) => s !== argName) + } + }} + > + {#snippet children({ item })} + + + {/snippet} + +
{/if} + {/snippet} + +
+ {/if} {/if} {/each} diff --git a/frontend/src/lib/components/ScriptBuilder.svelte b/frontend/src/lib/components/ScriptBuilder.svelte index 9ce8c17022..e069fe50b8 100644 --- a/frontend/src/lib/components/ScriptBuilder.svelte +++ b/frontend/src/lib/components/ScriptBuilder.svelte @@ -700,6 +700,7 @@ ws_error_handler_muted: script.ws_error_handler_muted, priority: script.priority, restart_unless_cancelled: script.restart_unless_cancelled, + delete_after_secs: script.delete_after_secs, timeout: script.timeout, concurrency_key: emptyString(script.concurrency_key) ? undefined : script.concurrency_key, visible_to_runner_only: script.visible_to_runner_only, @@ -1465,13 +1466,22 @@ corresponding action. {/snippet} + { - template = 'script' - script.kind = detail - initContent(script.language, detail, template) - }} + bind:selected={ + () => script.kind ?? 'script', + (kind) => { + // Load-bearing: any write to script.kind echoes back through the + // group, and initContent replaces the editor content outright. + if (kind === (script.kind ?? 'script')) return + template = 'script' + script.kind = kind as Script['kind'] + initContent(script.language, script.kind, template) + } + } > {#snippet children({ item })} {#each scriptKindOptions as { value, title, desc, documentationLink, Icon }} @@ -2009,6 +2019,7 @@ {/if} diff --git a/frontend/src/lib/components/ScriptEditor.svelte b/frontend/src/lib/components/ScriptEditor.svelte index 936d49f3ee..7bc4774e9f 100644 --- a/frontend/src/lib/components/ScriptEditor.svelte +++ b/frontend/src/lib/components/ScriptEditor.svelte @@ -95,6 +95,7 @@ import OpenInSessionButton, { type OpenInSessionSource } from './sessions/OpenInSessionButton.svelte' + import { setOpenInSessionHandoff } from './sessions/openInSessionContext' // Forward-looking hook for the upcoming session-pane feature: that PR will // `setContext('aiChatManager', ...)` from the session wrapper so this editor @@ -278,6 +279,14 @@ let opWs = $derived(workspaceOverride ?? $workspaceStore) + // Publish this editor's hand-off for AI entry points below it (the preview + // panel's "AI Fix"), withheld under `disableAi` so an embed that turned AI off + // gets no entry point that navigates its host to /sessions. Shadows an + // ancestor's hand-off deliberately: ScriptEditorDrawer mounts this without a + // `sessionOpen`, and falling through to FlowBuilder's would answer "fix this + // script" by opening the flow and abandoning the drawer's unsaved content. + setOpenInSessionHandoff({ source: () => (disableAi ? undefined : sessionOpen) }) + $effect(() => { onTestStateChange?.(testIsLoading) }) @@ -2270,14 +2279,10 @@
{:else} {#key previewLayout} - + + {#if previewLayout === 'bottom' && !(debugMode && isDebuggableScript)}
diff --git a/frontend/src/lib/components/ScriptSchema.svelte b/frontend/src/lib/components/ScriptSchema.svelte index 4cedaf388d..7a5661c538 100644 --- a/frontend/src/lib/components/ScriptSchema.svelte +++ b/frontend/src/lib/components/ScriptSchema.svelte @@ -7,9 +7,17 @@ interface Props { schema: Schema | any customUi?: EditableSchemaFormUi | undefined + workspace?: string | undefined } - let { schema = $bindable(), customUi = undefined }: Props = $props() + let { schema = $bindable(), customUi = undefined, workspace = undefined }: Props = $props() - + diff --git a/frontend/src/lib/components/ScriptVersionHistory.svelte b/frontend/src/lib/components/ScriptVersionHistory.svelte index 4d053fc433..693ddef47c 100644 --- a/frontend/src/lib/components/ScriptVersionHistory.svelte +++ b/frontend/src/lib/components/ScriptVersionHistory.svelte @@ -1,6 +1,6 @@ - + clearPageDrawerAnchor(VARIABLES_PATH)}> {#snippet actions()} + {#if edit && curWs} {/if} diff --git a/frontend/src/lib/components/VariableForm.svelte b/frontend/src/lib/components/VariableForm.svelte index 7f35c0435a..662f949957 100644 --- a/frontend/src/lib/components/VariableForm.svelte +++ b/frontend/src/lib/components/VariableForm.svelte @@ -52,6 +52,12 @@ let ws = $derived(workspace ?? $workspaceStore) + // Loading the deployed secret overwrites the draft row this form shares with the AI + // chat, so every path that would trigger it has to be blocked while that row stages a + // value — otherwise the staged one is replaced and the next deploy carries the old one. + // '' is the sentinel for "stages nothing", matching the deploy bodies. + let hasStagedValue = $derived(variable.value !== '') + const MAX_VARIABLE_LENGTH = 10000 let editorKind: 'plain' | 'json' | 'yaml' = $state('plain') @@ -77,10 +83,13 @@
diff --git a/frontend/src/lib/components/WhitelistIp.svelte b/frontend/src/lib/components/WhitelistIp.svelte index 8d8bac1a5d..fbc32d79b6 100644 --- a/frontend/src/lib/components/WhitelistIp.svelte +++ b/frontend/src/lib/components/WhitelistIp.svelte @@ -4,12 +4,16 @@ let ips: string[] | undefined = $state(undefined) + // Sentinels the backend stores when a worker has no external IP to report: 'NO IP' while the + // lookup is still in flight, 'unretrievable IP' once it has failed. + const UNKNOWN_IPS = ['NO IP', 'unretrievable IP'] + WorkerService.listWorkers({ pingSince: 300 }).then((workers) => { ips = [ ...new Set( workers .filter((worker) => { - return worker.ip != 'unretrievable IP' && worker.last_ping && worker.last_ping < 300 + return !UNKNOWN_IPS.includes(worker.ip) && worker.last_ping && worker.last_ping < 300 }) .map((worker) => worker.ip) ) @@ -18,7 +22,6 @@ {#if ips} -
If necessary, the workers IPs to whitelist are: {ips.join(', ')} diff --git a/frontend/src/lib/components/aiEvals/AddScorer.svelte b/frontend/src/lib/components/aiEvals/AddScorer.svelte new file mode 100644 index 0000000000..590a5ed573 --- /dev/null +++ b/frontend/src/lib/components/aiEvals/AddScorer.svelte @@ -0,0 +1,410 @@ + + +
+ {#if mode === 'new'} + {#if kind === 'agent'} + + An agent handed one whole run to grade. It is an ordinary AI agent resource: this creates it + with the prompt below, and editing the column later means editing that agent. + + {:else} + + A script handed the same run, returning a number, a boolean or {'{ score, reason, checks }'}. + The template scores the answer against the case's expected one, reports how the agent got + there as checks beside it, and leaves a case with no expected answer unmeasured. Helpers + below it cover exact and structural matches, which tools were called, arguments against each + tool's schema, repeated calls, step errors, latency and cost. + + {/if} + + + + + + + + {#if kind === 'agent'} + + + + + + {/if} + {:else} + {#if recent.length > 0} + + {#snippet children({ item })} + + + {/snippet} + + {/if} + + {#if usingRecent} +
+ {#each recent as scorer (scorer.path)} + {@const measures = datasetSummary(datasets, scorer.dataset)} + + {/each} +
+ {:else if kind === 'agent'} + + {:else} + + {/if} + {/if} +
diff --git a/frontend/src/lib/components/aiEvals/AgentEvalModal.svelte b/frontend/src/lib/components/aiEvals/AgentEvalModal.svelte new file mode 100644 index 0000000000..ff014f403a --- /dev/null +++ b/frontend/src/lib/components/aiEvals/AgentEvalModal.svelte @@ -0,0 +1,68 @@ + + + + + {#snippet titleBadge()} + Beta + {/snippet} +
+ {#if agentPath} + + {#key `${opWorkspace ?? ''}:${agentPath}`} + + {/key} + {:else} +
+ Evals run against a saved agent + + This agent is written into the flow step rather than saved as its own agent, so there is + nothing for a dataset and its runs to belong to. Save it as a reusable agent from the + step, and its evals start there. + +
+ {/if} +
+ diff --git a/frontend/src/lib/components/aiEvals/EvalCasesGrid.svelte b/frontend/src/lib/components/aiEvals/EvalCasesGrid.svelte new file mode 100644 index 0000000000..f7fec7a89f --- /dev/null +++ b/frontend/src/lib/components/aiEvals/EvalCasesGrid.svelte @@ -0,0 +1,132 @@ + + + + +
diff --git a/frontend/src/lib/components/aiEvals/EvalDatasetDrawer.svelte b/frontend/src/lib/components/aiEvals/EvalDatasetDrawer.svelte new file mode 100644 index 0000000000..3da84cf1ad --- /dev/null +++ b/frontend/src/lib/components/aiEvals/EvalDatasetDrawer.svelte @@ -0,0 +1,419 @@ + + + onClosed?.()}> + + (removingCase = undefined)} + on:confirmed={() => { + const target = removingCase + removingCase = undefined + if (target?.id) deleteCase(target.id) + }} + > + + {caseLabel(removingCase ?? { input: {} })} goes from the dataset. The runs that executed it keep + their results: a run that happened is not undone by curating the case away. + + + (removingDataset = false)} + on:confirmed={() => { + removingDataset = false + deleteDataset() + }} + > + + {datasetPath} goes with its cases and every run recorded against it. The jobs those runs produced + are kept. + + + drawer?.closeDrawer()} + > +
+ + {mode === 'edit' + ? 'The cases this agent is measured on. Editing them leaves the runs that already executed them as they were.' + : 'A set of cases to measure this agent on, and the scorers that read them.'} + + {#key formGeneration} + +
+ + +
+ {/key} + + +
+ (scorersWriting = w)} + /> +
+
+
+ Cases + {workingCases.length} +
+ +
+
+ (casesEditing = v)} + /> +
+
+
+ {#snippet actions()} + {#if mode === 'edit'} + + + {:else} + + {/if} + {/snippet} + + diff --git a/frontend/src/lib/components/aiEvals/EvalRunDialog.svelte b/frontend/src/lib/components/aiEvals/EvalRunDialog.svelte new file mode 100644 index 0000000000..d4d1714fd0 --- /dev/null +++ b/frontend/src/lib/components/aiEvals/EvalRunDialog.svelte @@ -0,0 +1,279 @@ + + + +
+ + + + + + +
+ {#snippet actions()} + + {/snippet} + diff --git a/frontend/src/lib/components/aiEvals/EvalRunsList.svelte b/frontend/src/lib/components/aiEvals/EvalRunsList.svelte new file mode 100644 index 0000000000..5d84ef88ce --- /dev/null +++ b/frontend/src/lib/components/aiEvals/EvalRunsList.svelte @@ -0,0 +1,170 @@ + + + + + + + + + + + + + Run + Dataset + Cases + Scores + When + + + + {#each experiments as experiment (experiment.id)} + onOpen(experiment)}> + +
+
+ {experimentName(experiment)} + + {subjectLabel(experiment, deployedHash, currentVersion)} + +
+ {experiment.created_by} +
+ + + {@const summary = datasetSummary(datasets, experiment.dataset)} + + + + {experiment.case_count} + + +
+ {#each experiment.scores ?? [] as score (score.scorer_id)} + {@const value = headline(score)} + + + {#if score.kind === 'agent'} + + {:else} + + {/if} + {score.name} + {#if value != undefined} + {value} + {:else if score.failed > 0} + failed + {:else if experiment.running} + + {:else} + — + {/if} + + + {/each} + {#if (experiment.scores ?? []).length === 0} + {#if experiment.running} + + + scoring + + {:else} + not scored + {/if} + {/if} +
+ + + + + + + + {/each} + {#if experiments.length === 0 && !loaded} + + + + + + {:else if experiments.length === 0} + + +
+ No runs yet + + A run answers every case of a dataset and scores the answers. Each one is kept, so the + next has something to be compared against. + + +
+ + + {/if} + + diff --git a/frontend/src/lib/components/aiEvals/EvalScorers.svelte b/frontend/src/lib/components/aiEvals/EvalScorers.svelte new file mode 100644 index 0000000000..68287b0fbc --- /dev/null +++ b/frontend/src/lib/components/aiEvals/EvalScorers.svelte @@ -0,0 +1,383 @@ + + +
+
+ Scorers + {scorers.length} +
+ openAdd('agent', 'new') }, + { + displayName: 'Existing AI judge', + icon: Bot, + action: () => openAdd('agent', 'existing') + }, + { displayName: 'New code scorer', icon: Code2, action: () => openAdd('script', 'new') }, + { + displayName: 'Existing code scorer', + icon: Code2, + action: () => openAdd('script', 'existing') + } + ]} + placement="bottom-end" + > + {#snippet buttonReplacement()} + + {/snippet} + +
+ +
+ {#if scorers.length === 0} +
+ A scorer reads one run and returns a number. Every run of this dataset is measured by all of + them, which is what makes two runs comparable. +
+ {:else} +
+ {#each scorers as scorer (scorer.id)} +
+ {#if scorer.kind === 'agent'} + + {:else} + + {/if} +
+ + {scorerLabel(scorer)} + + {scorer.path} +
+ {#if scorer.pass_if != undefined} + + ≥ {scorer.pass_if} + + {/if} +
+ {/each} +
+ {/if} +
+
+ + + scorerDrawer?.closeDrawer()} + > + {#if workspace && datasetPath} + {#key scorerFormGeneration} + + scriptEditorDrawer + ?.openDrawer(hash, onChanged) + .catch((e) => sendUserToast(`Failed to open the scorer: ${e}`, true))} + /> + {/key} + {/if} + {#snippet actions()} + {@const state = addScorerForm?.submitState()} + + {/snippet} + + + + + settingsDrawer?.closeDrawer()}> + {#if settingsScorer} +
+ + {#if settingsScorer.kind === 'agent'} + + {:else} + + {/if} + {settingsScorer.path} + + + + + +
+ {/if} + {#snippet actions()} + + {/snippet} + + + + + + + + (removingScorer = undefined)} + on:confirmed={async () => { + const target = removingScorer + removingScorer = undefined + if (!target) return + try { + await saveScorers(scorers.filter((s) => s.id !== target.id)) + } catch (e) { + sendUserToast(`Failed to remove the scorer: ${e}`, true) + } + }} +> + + The column goes from every run of this dataset, the ones already recorded included. Adding it + again starts a new column, which fills from the next run on. + + diff --git a/frontend/src/lib/components/aiEvals/EvalsPane.svelte b/frontend/src/lib/components/aiEvals/EvalsPane.svelte new file mode 100644 index 0000000000..d9e5b16fe4 --- /dev/null +++ b/frontend/src/lib/components/aiEvals/EvalsPane.svelte @@ -0,0 +1,978 @@ + + +
+
+ {#if viewingRun} + + {/if} +
+ {#if viewingRun && experiment?.run_job_id} + + Open the job + + + {/if} + {#if !viewingRun && loaded && datasets.length > 0} + + {#if experiments.length > 0} + + + {/if} + {/if} +
+ +
+ + +
+ {#if loaded && loadError} +
+ Could not load evals + + The datasets or runs could not be read. Check your access to this agent and reload. + +
+ {:else if loaded && datasets.length === 0} +
+ No dataset yet + + A dataset is the set of cases this agent is measured on. Runs are of a dataset, so + it is the first thing to make. + + +
+ {:else if !viewingRun || !loaded} + openRun(e.id)} + onEditDataset={async (path) => { + if (await useDataset(path)) datasetDrawer?.openDrawer('edit') + }} + onNew={() => (runDialogOpen = true)} + /> + {:else} + + + + + {#each scorers as scorer (scorer.id)} + + {/each} + + + + Case + Answer + {#each scorers as scorer, index (scorer.id)} + {@const mean = means.find((m) => m.scorer_id === scorer.id)} + {@const headline = columnHeadline(scorer, mean)} + + +
+ + {#if scorer.kind === 'agent'} + + {:else} + + {/if} + {scorerLabel(scorer)} + + + {#if headline} + + {headline.value} + + {#if headline.delta && headline.direction !== 0} + 0 ? 'text-green-500' : headline.direction < 0 ? 'text-red-500' : 'text-tertiary'}`} + > + {headline.delta} + + {/if} + {/if} + +
+ + {/each} + + + + {#each displayRows as row (row.case_id)} + {@const status = statusOf(row.status)} + openCase(row)} + > + + {caseLabel(row)} + + + + + {#if row.output != undefined} + {row.output} + {:else if status === STATUS.not_run} + not run + {:else} + {status.label.toLowerCase()} + {/if} + + + {#each scorers as scorer, index (scorer.id)} + {@const cell = row.scores.find((s) => s.scorer_id === scorer.id)} + + {#if cell?.pending} + + + + {:else if cell?.score != undefined} + + {#snippet text()} +
+ {#if cell.reason} + {cell.reason} + {/if} + {#each checksOf(cell) as check (check.name)} + + + {check.passed ? '✓' : '✗'} + + {check.name} + {#if check.detail} + {check.detail} + {/if} + + {/each} +
+ {/snippet} + + {#if cell.passed != undefined} + + {cell.passed ? '✓' : '✗'} + + {/if} + + {formatScore(cell.score)} + + {#if cell.baseline != undefined && cell.score !== cell.baseline} + {@const delta = cell.score - cell.baseline} + 0 ? 'text-green-500' : 'text-red-500'}`} + > + {formatDelta(delta)} + + {/if} + + + {:else if cell?.not_applicable} + + {#snippet text()} + {cell.reason} + {/snippet} + + n/a + + + {:else if cell?.error} + + {#snippet text()} + {cell.error} + {/snippet} + failed + + {:else} + — + {/if} + + {/each} + + {/each} + + + {/if} +
+ + {#if selectedRow} + {@const openRow = selectedRow} + +
+
+ + {openRow.input?.user_message ?? caseLabel(openRow)} + +
+ {#if openRow.job_id} + + Open the case job + + + {/if} +
+
+ {#if openRow.expected != undefined && openRow.expected !== ''} + + {/if} + {#if scorers.length > 0 && openRow.scores.length > 0} + + {/if} + {#if experiment && (openRow.job_id || openRow.output != undefined)} +
+
+ + Case result + +
+
+ {#if openRow.output != undefined} +
+ +
+ {:else if openRow.status === 'running'} + + + Running + + {:else} + {statusOf(openRow.status).label} + {/if} +
+
+ {/if} +
+
+ + {/if} + +
+
+ + { + if (await useDataset(path)) { + resumeRunDialog = true + datasetDrawer?.openDrawer('edit') + } + }} + onNewDataset={() => { + resumeRunDialog = true + datasetDrawer?.openDrawer('new') + }} +/> + + { + if (!resumeRunDialog) return + resumeRunDialog = false + // On the dataset the drawer was just in: the dialog opens on the pane's own, which + // creating or editing one has already moved to it. + runDialogOpen = true + }} +/> diff --git a/frontend/src/lib/components/aiEvals/evalUtils.test.ts b/frontend/src/lib/components/aiEvals/evalUtils.test.ts new file mode 100644 index 0000000000..d1f65e0d96 --- /dev/null +++ b/frontend/src/lib/components/aiEvals/evalUtils.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest' +import type { EvalExperiment } from '$lib/gen' +import { parseThreshold, subjectLabel } from './evalUtils' + +describe('parseThreshold', () => { + it('keeps 0 as a threshold and reads only empty text as no threshold', () => { + expect(parseThreshold(0)).toEqual({ value: 0, error: false }) + expect(parseThreshold('0')).toEqual({ value: 0, error: false }) + expect(parseThreshold('')).toEqual({ error: false }) + expect(parseThreshold(' ')).toEqual({ error: false }) + expect(parseThreshold(null)).toEqual({ error: false }) + expect(parseThreshold(undefined)).toEqual({ error: false }) + }) + + it('refuses anything outside 0 to 1 or not a number', () => { + expect(parseThreshold('0.5')).toEqual({ value: 0.5, error: false }) + expect(parseThreshold('1')).toEqual({ value: 1, error: false }) + expect(parseThreshold('1.5')).toEqual({ error: true }) + expect(parseThreshold('-0.1')).toEqual({ error: true }) + expect(parseThreshold('abc')).toEqual({ error: true }) + }) +}) + +describe('subjectLabel', () => { + function run(subject: Record): EvalExperiment { + return { subject: { path: 'u/me/agent', ...subject } } as unknown as EvalExperiment + } + + it('names a deployed run and a pinned version by their number', () => { + expect(subjectLabel(run({ kind: 'agent', version: 4 }))).toBe('v4') + expect(subjectLabel(run({ kind: 'agent_version', version: 2 }))).toBe('v2') + }) + + it('says a draft run is edits on top of the version it was an edit of', () => { + expect(subjectLabel(run({ kind: 'agent_draft', version: 4, draft_hash: 'h1' }))).toBe( + 'v4 + edits' + ) + expect(subjectLabel(run({ kind: 'agent_draft', draft_hash: 'h1' }))).toBe('edits') + }) + + it('reads a draft whose configuration is now deployed as the current version', () => { + const draft = run({ kind: 'agent_draft', version: 4, draft_hash: 'h1' }) + expect(subjectLabel(draft, 'h1', 5)).toBe('v5') + expect(subjectLabel(draft, 'other', 5)).toBe('v4 + edits') + }) +}) diff --git a/frontend/src/lib/components/aiEvals/evalUtils.ts b/frontend/src/lib/components/aiEvals/evalUtils.ts new file mode 100644 index 0000000000..fcc5419f48 --- /dev/null +++ b/frontend/src/lib/components/aiEvals/evalUtils.ts @@ -0,0 +1,107 @@ +import type { + EvalCase, + EvalCaseInput, + EvalDataset, + EvalExperiment, + NewEvalCase, + Scorer +} from '$lib/gen' + +/** The case being edited in the drawer, before it is either run or saved to a dataset. */ +export type CaseDraft = NewEvalCase & { id?: string } + +/** A level the evals pane is on, and the way out of it. */ +export type EvalsLocation = { label: string; back: () => void } + +export type ScorerKind = Scorer['kind'] + +export function emptyCase(): CaseDraft { + return { input: { user_message: '' } } +} + +export function fromStoredCase(c: EvalCase): CaseDraft { + const { created_at: _created_at, created_by: _created_by, ...rest } = c + return rest +} + +export function caseLabel(c: { input?: EvalCaseInput }): string { + const message = c.input?.user_message?.trim() + if (message) return message.length > 60 ? message.slice(0, 60) + '…' : message + return 'Untitled case' +} + +export function experimentName(experiment: EvalExperiment): string { + return `Run ${experiment.run_number}` +} + +/** + * What ran: a deployed version, or a version with edits sitting on top of it. + * + * The list and the results endpoint restamp a draft run whose configuration was later deployed, so + * the kind is usually enough; `deployedHash` and `currentVersion` resolve the one still unstamped. + */ +export function subjectLabel( + experiment: EvalExperiment, + deployedHash?: string, + currentVersion?: number +): string { + if (experiment.subject.kind === 'agent_version') { + return experiment.subject.version ? `v${experiment.subject.version}` : 'a past version' + } + const deployed = + experiment.subject.kind === 'agent' || + (experiment.subject.draft_hash != undefined && experiment.subject.draft_hash === deployedHash) + if (deployed) { + const version = + experiment.subject.kind === 'agent' ? experiment.subject.version : currentVersion + return version ? `v${version}` : 'deployed' + } + return experiment.subject.version ? `v${experiment.subject.version} + edits` : 'edits' +} + +/** A scorer keeps its id when renamed, so its name is the column header and nothing else. */ +export function scorerLabel(scorer: Scorer): string { + return scorer.name || scorer.path.split('/').pop() || scorer.path +} + +export function kindLabel(kind: ScorerKind): string { + return kind === 'agent' ? 'Judge agent' : 'Script' +} + +export function formatScore(score: number | undefined): string { + return score == undefined ? '—' : score.toFixed(2) +} + +export function formatDelta(delta: number): string { + if (delta === 0) return '0.00' + return `${delta > 0 ? '+' : '−'}${Math.abs(delta).toFixed(2)}` +} + +/** What a dataset is for, where it says so: the path names it either way. */ +export function datasetSummary(datasets: EvalDataset[], path: unknown): string | undefined { + return datasets.find((d) => d.path === path)?.summary || undefined +} + +/** + * A pass threshold, as a field holds it. Empty is `''` or null, never a number: a number input + * coerces the text, so a valid threshold of 0 would otherwise read as empty and be dropped. The + * server refuses anything outside 0 to 1, caught here so the form blocks instead of the save. + */ +export function parseThreshold(text: string | number | null | undefined): { + value?: number + error: boolean +} { + const trimmed = typeof text === 'string' ? text.trim() : text + if (trimmed === '' || trimmed == undefined) return { error: false } + const value = Number(trimmed) + if (Number.isNaN(value) || value < 0 || value > 1) return { error: true } + return { value, error: false } +} + +export function summaryToName(summary: string): string { + return summary + .toLowerCase() + .replace(/[^a-z0-9_]/g, '_') + .replace(/_+/g, '_') + .replace(/^_|_$/g, '') +} diff --git a/frontend/src/lib/components/apps/components/display/dbtable/duckdbQuicksearchColumns.test.ts b/frontend/src/lib/components/apps/components/display/dbtable/duckdbQuicksearchColumns.test.ts new file mode 100644 index 0000000000..9b26c3dcb8 --- /dev/null +++ b/frontend/src/lib/components/apps/components/display/dbtable/duckdbQuicksearchColumns.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest' +import { buildVisibleFieldList, duckdbQuicksearchColumns, type ColumnDef } from './utils' + +function col(field: string, datatype: string, extra: Partial = {}): ColumnDef { + return { field, datatype, ...extra } as ColumnDef +} + +describe('duckdbQuicksearchColumns', () => { + it('casts list and array columns, and nothing else', () => { + expect( + duckdbQuicksearchColumns([ + col('id', 'VARCHAR'), + col('tags', 'VARCHAR[]'), + col('pos', 'INTEGER[3]'), + col('meta', 'STRUCT(a INTEGER)') + ]) + ).toBe('"id", CAST("tags" AS VARCHAR), CAST("pos" AS VARCHAR), "meta"') + }) + + // This byte-identity is what keeps the policy digest of an already-deployed + // Database Studio app valid; a table with no list column must produce the + // query it produced before quicksearch learned to cast anything. + it('emits the plain column list when no column is a list', () => { + const columnDefs = [col('id', 'VARCHAR'), col('n', 'INTEGER'), col('at', 'TIMESTAMP')] + expect(duckdbQuicksearchColumns(columnDefs)).toBe( + buildVisibleFieldList(columnDefs, 'duckdb').join(', ') + ) + }) + + it('skips ignored columns', () => { + expect( + duckdbQuicksearchColumns([col('id', 'VARCHAR'), col('tags', 'VARCHAR[]', { ignored: true })]) + ).toBe('"id"') + }) +}) diff --git a/frontend/src/lib/components/apps/components/display/dbtable/queries/count.ts b/frontend/src/lib/components/apps/components/display/dbtable/queries/count.ts index dc422f48e6..b93a1d669d 100644 --- a/frontend/src/lib/components/apps/components/display/dbtable/queries/count.ts +++ b/frontend/src/lib/components/apps/components/display/dbtable/queries/count.ts @@ -11,7 +11,12 @@ import type { AppInput, RunnableByName } from '$lib/components/apps/inputType' import { wrapDucklakeQuery } from '../../../../../ducklake' import type { DbType, DbInput } from '$lib/components/dbTypes' import { buildParameters } from '../utils' -import { getLanguageByResourceType, type ColumnDef, buildVisibleFieldList } from '../utils' +import { + getLanguageByResourceType, + type ColumnDef, + buildVisibleFieldList, + duckdbQuicksearchColumns +} from '../utils' export function makeCountQuery( dbType: DbType, @@ -118,8 +123,8 @@ export function makeCountQuery( } case 'duckdb': if (filteredColumns.length > 0) { - quicksearchCondition += ` ($quicksearch = '' OR CONCAT(' ', ${filteredColumns.join( - ', ' + quicksearchCondition += ` ($quicksearch = '' OR CONCAT(' ', ${duckdbQuicksearchColumns( + columnDefs )}) LIKE CONCAT('%', $quicksearch, '%'))` } else { quicksearchCondition += ` ($quicksearch = '' OR 1 = 1)` diff --git a/frontend/src/lib/components/apps/components/display/dbtable/queries/dbQueriesUtils.ts b/frontend/src/lib/components/apps/components/display/dbtable/queries/dbQueriesUtils.ts index 711fb7ed38..7ad28248d2 100644 --- a/frontend/src/lib/components/apps/components/display/dbtable/queries/dbQueriesUtils.ts +++ b/frontend/src/lib/components/apps/components/display/dbtable/queries/dbQueriesUtils.ts @@ -1,5 +1,6 @@ import type { DbType } from '$lib/components/dbTypes' import type { TableEditorForeignKey, TableEditorValuesColumn } from '../tableEditor' +import { renderDbQuotedIdentifier } from '../utils' export function formatDefaultValue(str: string, datatype: string, resourceType: DbType): string { if (!str) return '' @@ -33,6 +34,13 @@ export function renderForeignKey( useSchema: boolean dbType: DbType tableName: string + /** + * Table to name in the REFERENCES clause, quoted per dot-separated part so a + * schema-qualified target survives identifiers that need quoting. The constraint + * name stays derived from `fk.targetTable`, so qualifying a target here never + * renames a constraint an earlier migration created under the bare name. + */ + qualifiedTarget?: string } ): string { const sourceColumns = fk.columns.map((c) => c.sourceColumn).filter(Boolean) @@ -53,9 +61,16 @@ export function renderForeignKey( .join('_') .replaceAll('.', '_')} `.substring(0, 60) - sql += ` FOREIGN KEY (${sourceColumns.join( + const targetRef = options.qualifiedTarget + ? options.qualifiedTarget + .split('.') + .map((part) => renderDbQuotedIdentifier(part, options.dbType)) + .join('.') + : targetTable + + sql += ` FOREIGN KEY (${sourceColumns.join(', ')}) REFERENCES ${targetRef} (${targetColumns.join( ', ' - )}) REFERENCES ${targetTable} (${targetColumns.join(', ')})` + )})` if (fk.onDelete !== 'NO ACTION') sql += ` ON DELETE ${fk.onDelete}` if (fk.onUpdate !== 'NO ACTION') sql += ` ON UPDATE ${fk.onUpdate}` return sql diff --git a/frontend/src/lib/components/apps/components/display/dbtable/queries/select.ts b/frontend/src/lib/components/apps/components/display/dbtable/queries/select.ts index 737dfdc329..be01457434 100644 --- a/frontend/src/lib/components/apps/components/display/dbtable/queries/select.ts +++ b/frontend/src/lib/components/apps/components/display/dbtable/queries/select.ts @@ -11,7 +11,12 @@ import type { AppInput, RunnableByName } from '$lib/components/apps/inputType' import { wrapDucklakeQuery } from '../../../../../ducklake' import type { DbType, DbInput } from '$lib/components/dbTypes' import { buildParameters } from '../utils' -import { getLanguageByResourceType, type ColumnDef, buildVisibleFieldList } from '../utils' +import { + getLanguageByResourceType, + type ColumnDef, + buildVisibleFieldList, + duckdbQuicksearchColumns +} from '../utils' function makeSnowflakeSelectQuery( table: string, @@ -298,8 +303,8 @@ CASE WHEN :order_by = '${column.field}' AND :is_desc IS true THEN \`${column.fie ) .join(',\n')}` - quicksearchCondition = `($quicksearch = '' OR CONCAT(${filteredColumns.join( - ', ' + quicksearchCondition = `($quicksearch = '' OR CONCAT(${duckdbQuicksearchColumns( + columnDefs )}) ILIKE '%' || $quicksearch || '%')` query += `SELECT ${filteredColumns.join(', ')} FROM ${table}\n` diff --git a/frontend/src/lib/components/apps/components/display/dbtable/utils.ts b/frontend/src/lib/components/apps/components/display/dbtable/utils.ts index 3304bbdf0b..bf81ec07bb 100644 --- a/frontend/src/lib/components/apps/components/display/dbtable/utils.ts +++ b/frontend/src/lib/components/apps/components/display/dbtable/utils.ts @@ -305,11 +305,32 @@ export async function formatGraphqlSchema(schema: IntrospectionQuery): Promise