Files
windmill/AGENTS.md
T
ca44043e12 feat: let operators compose flows when the workspace grants the right (#11228)
* feat: let a workspace withdraw operator schedule and trigger writes

Operators can create, edit and delete schedules and triggers today through the
API, CLI and MCP, while the operator_settings flags beside them only hide those
pages. An admin who wants operators to see what is scheduled without letting
them change it cannot express that. Add manage_schedules and manage_triggers as
enforced settings, gated at the schedule handlers and at the generic TriggerCrud
routes so every trigger kind is covered by one check.

They name capabilities operators already hold, so they are granted unless
withdrawn, and absence has to mean "never configured" rather than a value. The
read coalesces to true; the update endpoint merges into the stored jsonb with
the two fields as Option<bool>, so an omitted key keeps what is stored.
operator_settings is git-synced as a whole object, so a settings file written
before these keys existed reaches the endpoint on every pull, and a serde or SQL
default of either polarity would turn that pull into a silent withdrawal or
restoration.

The rights are read through a per-process cache, so withdrawing one publishes a
notify_event that drops the entry on every replica.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dsf6VC4MVLisiEoeQkgbr4

* feat: let operators compose flows when the workspace grants the right

Adds operator_settings.builder_flows: a workspace setting that lets every
operator compose flows out of runnables that already exist. It does not make
them authors. The boundary the operator role draws is authoring code and running
arbitrary code, and this does not move it: check_flow_is_composition_only walks
the value and refuses anything carrying code, including the shapes an obvious
walk misses (code hoisted into a flow_node, an AI agent step's tools, and a
linked ai_agent resource whose tool list is resolved at run time).

What the walk cannot settle it returns for the caller to authorize under RLS:
the worker tags the steps pin, every runnable they reference, and the (path,
hash) of every version-pinned step. Composing a path is enough to run it and to
run it as whoever it runs as, since the worker resolves a step's path with the
root DB handle and adopts that runnable's on_behalf_of. A pinned hash needs its
own check because dispatch ignores the path beside it.

The gate runs on every write and on both request-supplied-value paths, flow
preview and flow dependencies, or either becomes the way to run what the write
path refuses.

Operators of a builder workspace consume a full author seat; the EE companion
carries the counting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dsf6VC4MVLisiEoeQkgbr4

* feat: enforce operator write rights on the router and in the UI

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: close the capture gap and gate the trigger editors' write actions

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: gate acl writes and the native trigger drawer behind manage rights

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: refuse operator writes with 403 and gate sharing at the drawer

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf: resolve identity in the operator write gate only for writes

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: gate the suspended-jobs actions and stop the route check refusing reads

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: explain the empty-state create button when operator writes are withdrawn

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: audit operator settings changes and fold path writes into native rows

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: open locked editors read-only and group the operator settings

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: skip email and azure lookups on editor open while triggers are locked

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* docs: state each operator-rights rationale once in comments

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: address CI review findings on operator write rights

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: keep capture move gated and skip it in the builders while locked

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: refuse builder-rights violations with 403 so operators stay logged in

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* refactor: trim duplication in the operator builder gates

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: hide build app from builder operators on the flow page

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore: point at the companion EE PR merged with EE main

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: keep a builder's drafts list loading past drafts they cannot write

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: hide saved agents from builder operators in the step picker

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore: note the inlined seat rule and drop orphaned sqlx entries

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: stop a builder's step test from logging them out

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: refuse a builder's dependency job on a path it cannot write

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: keep builders from adding dynamic dropdown code to a flow

A flow's dropdown code runs as whoever loads its form, so a builder may keep or drop the code stored on the flow it updates, never add or change it. The builder's editor hides the dropdown types and code, and previews options through the deployed flow; the inline dropdown refusal is a 403 so it no longer logs operators out. Also trims rationale comments repeated across sites.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: show why saving operator settings failed

The seat-cap refusal on granting builder rights explains what to do; the toast now carries the server's message instead of a generic failure.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: check builder flow drafts like deploys and treat dropdown code as code

A developer who loads a builder's flow draft in the editor runs its dynamic dropdown code as themselves, so a builder's draft now passes the same checks as a deploy. Dropdown code is refused like step code rather than kept or dropped, which also removes the exact-match comparison that refused builders over whitespace.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: bill a builder workspace's operators as developers on cloud

The cloud seat count behind the Premium page, the sidebar usage and the fork cap still weighed every operator at half a seat, while the builder right makes them authors. The out-of-repo invoicing job must follow the same rule.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: check a builder's flow draft as it will be stored

Draft storage strips NUL escapes after the builder check, so a key ending in one (value\u0000, x-windmill-dyn-select-code\u0000) passed the check as an unknown field and was stored under its plain name. The check now reads the sanitized text.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: list a builder's flow drafts and hide hub imports from builders

A builder's undeployed flows now appear in the home list, the flow list and the folder counts. Hub project imports and templates bring scripts and apps along, so builders are no longer offered them. The docs record builders' JavaScript expressions as an accepted risk.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: keep the stored builder right when a settings payload omits it

A git-synced settings file written before the key existed withdrew the right on every push. builder_flows now follows the manage_* rights: an omitted key leaves the stored value, and the CLI does not count it as a difference.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: word builder refusals by what the flow contains, test the tag refusal

A builder refused on a developer's flow never changed its code, so the refusals now describe the flow ("has inline code, so only a developer can edit this flow") rather than an authoring attempt. The grant confirmation uses the neutral dialog: granting changes billing but destroys nothing. The integration test pins the refusal of a worker tag the workspace cannot use.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: read builder rights from the operating workspace, gate the flow page's audit logs entry

Builder rights now come from useOperatorBuilderFlows(), next to the schedule and trigger locks, so an editor embedded for another workspace answers about that workspace; the legacy AI chat, one instance for the whole app, reads the navigation workspace. The flow page's Audit logs entry follows the operator audit_logs setting now that builders open that menu. Operator settings reset every value on load, null settings included, so nothing carries over from the previous workspace.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: pick a dynamic dropdown's code source by the operating user's role

The flow input editor, the flow test panel and the flow chat send the dropdown request to the operating workspace, so they now also choose inline versus deployed code by the role held there, through useOperatingUser(), instead of the navigation workspace's.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore: update ee-repo-ref to 40ac1c5f8cbce3843b582d9b392d3f3cc7eca3e6

This commit updates the EE repository reference after PR #815 was merged in windmill-ee-private.

Previous ee-repo-ref: 31c9e66884b8ca805b20bbfad41fc428fbedbc0e

New ee-repo-ref: 40ac1c5f8cbce3843b582d9b392d3f3cc7eca3e6

Automated by sync-ee-ref workflow.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
2026-09-30 18:09:14 +02:00

19 KiB

Windmill

Open-source platform for internal tools, workflows, API integrations, background jobs, and UIs. Rust backend + Svelte 5 frontend.

Workflow

  1. Understand: Before coding, explore the codebase (see Code Navigation below). Use outline to understand file structure, body to read specific symbols, def/callers/callees to trace code, Grep to find usages. Read docs/ for domain context.
  2. Plan: For non-trivial changes, use plan mode. For large features, break into reviewable stages. For a new user-facing feature, put the feature_usage telemetry in the plan as a proposed item (see docs/feature-telemetry.md) so the user can keep or drop it — don't ask separately, and don't instrument bugfixes or refactors.
  3. Execute: Follow coding patterns from skills (rust-backend, svelte-frontend)
  4. Validate: After every change, run the appropriate checks per docs/validation.md, then exercise the change on the running instance. Type-checks are not verification. Whatever the change touches, get that path actually running, and stand up whatever that takes — this is expected, not a last resort. A few examples, not a closed list: drive the UI with the Playwright MCP, run a real job of the kind you touched, restart the backend with the cargo features the path needs (backend/AGENTS.md), put a stub in front of an upstream, start MinIO for an S3 path, plant state with SQL, exercise it through the wmill CLI. If the path you need has no obvious way in, invent one rather than skipping it; docs/ carries recipes for several areas. If it needs a credential or a third-party account, ask for one rather than skipping the test or inventing a value. If you genuinely cannot exercise it, say which path went unexercised instead of implying it was verified.

Documentation

  • Validation: docs/validation.md — what checks to run based on what you changed
  • Unreleased SDK changes: docs/wac-sdk-e2e.md — exercising a client change on a real worker
  • Agent workers: docs/agent-worker-e2e.md — building and running one locally. An agent reaches the DB only through the API, so Connection::Http paths are never taken by a plain cargo run; a normal build cannot start one at all.
  • External instance data tables: docs/external-instance-datatables.md — the cluster Windmill administers behind external_instance data tables and Ducklake catalogs: its invariants (one lifecycle lock, managed-object markers, the setup gate, per-cluster roles, fork copy ownership) and how to run one locally
  • Enterprise: docs/enterprise.md — EE file conventions and PR workflow
  • Operator write rights: docs/operator-write-rights.md — which operator_settings flags are enforced rather than cosmetic, and why a right that is granted-unless-withdrawn needs Option fields and a jsonb merge rather than a serde default
  • Operator builder rights: docs/operator-builder-rights.md — the workspace setting that lets operators compose flows, what the composition check must cover, and why it costs a full seat
  • Auth surface: docs/auth-surface.md — credential precedence, session/cache invalidation scope, which token labels email their owner at expiry, how OAuth login matches login_type, and that every superadmin route refuses $WM_TOKEN. Read before designing anything that creates users, tokens or sessions.
  • Product telemetry: docs/feature-telemetry.md — when to instrument a new feature with feature_usage, and the four-step recipe. An unregistered (feature, kind) pair is dropped silently, so frontend-only instrumentation records nothing.
  • Backend patterns: use the rust-backend skill when writing Rust code
  • Frontend patterns: use the svelte-frontend skill when writing Svelte code. Do NOT edit svelte files unless you have read that skill.
  • Frontend UUIDs: do not call crypto.randomUUID() in frontend code. Import randomUUID from $lib/utils/uuid instead.
  • Code review: review the current PR or branch against the shared review policy in REVIEW.md (severity triage, public-surface checklist, AGENTS.md compliance, test-coverage assessment). The skill at .agents/skills/local-review/SKILL.md orchestrates it. All three CLIs auto-discover the same SKILL — Claude reads .claude/skills/ (symlinked to the canonical .agents/skills/ file), Codex and Pi read .agents/skills/ directly. Invoke with /local-review in Claude Code, $local-review (or /skills selector) in Codex, or pi --skill local-review / /skill:local-review in Pi. For a Codex-driven pass that mirrors the codex-pr-review GitHub action against your unpushed work (committed + uncommitted) before you push, use /local-review-codex (.agents/skills/local-review-codex/) — same REVIEW.md policy and xhigh reasoning, on gpt-6-astra rather than the action's gpt-5.6-sol; requires the codex CLI >= 0.153.4.
  • Domain guides: .claude/skills/native-trigger/
  • Brand/UI guidelines: frontend/brand-guidelines.md
  • Domain vocabulary: CONTEXT.md — the words this codebase uses for its own concepts (step, step setting, trigger step, …). Name things the way it does.
  • CLI commands: when adding/modifying/removing a command, subcommand, option, or description in cli/src/commands/, run python system_prompts/generate.py to refresh system_prompts/auto-generated/ and cli/src/guidance/skills.gen.ts. The CLI docs the agents use to operate wmill are derived from the source — stale generated files give agents the wrong flags.
  • AI guidance (chat prompts and CLI skills): what the AI knows about Windmill itself — how flows, scripts, apps, pipelines and resources work — is written once, in system_prompts/base/ and system_prompts/languages/, and system_prompts/generate.py builds both the chat's $system_prompts exports and the CLI's skills from it. Add or change such guidance there, never inline in one consumer: a sentence only one side should see goes in a <!-- cli-only --> / <!-- chat-only --> block, and a new file reaches both through the TOPICS table. The chat prompt builders under frontend/src/lib/components/copilot/chat/ add only tool plumbing and runtime values, and shared markdown names no chat tool, since it reaches every chat mode. Rerun generate.py after editing; system_prompts/README.md has the details.
  • Session recorder: frontend/src/lib/components/recording/ is also the recorder wmill app dev --recording serves, vendored into the CLI as cli/src/commands/app/devRecorderBundle.gen.ts. After changing rawAppSnapshot.ts or rawAppRecording.svelte.ts, run bun run gen:dev-recorder from cli/ (cli/test/dev_recorder_bundle_unit.test.ts fails otherwise).
  • Raw-app policy: frontend/src/lib/components/raw_apps/rawAppPolicy.ts also derives the policy the server's raw-app deploy stores, vendored into the bundle job as backend/windmill-api/src/apps_raw_policy.gen.js. After changing it or anything it imports, run bun run gen:app-policy from cli/ (cli/test/app_policy_bundle_unit.test.ts fails otherwise). It rides in the job rather than being read from the CLI the job runs because the images install windmill-cli unpinned, so an image can carry one older than its server.

Dev Environment

In a git worktree, the ports and database below are NOT the ones to use. Each worktree gets its own backend port, frontend port and Postgres database, so the defaults in this section apply only to a plain single checkout. Discover the real values before running anything — see "Per-worktree ports and database" below.

Check whether they are already running before starting anything. In a managed worktree the backend and frontend are already up in sibling panes — use those, don't spawn your own. A second server started in your own shell fights the first one for the port.

herdr is the worktree manager; $HERDR_ENV=1 marks one of its panes. herdr pane list --workspace "$HERDR_WORKSPACE_ID" lists this worktree's panes — match on cwd, since the servers run from backend/ and frontend/ — and herdr pane read <pane_id> --source recent-unwrapped --lines 50 reads one's log; herdr --skill prints the full reference for inspecting and driving panes and agents. The plugins that provision these worktrees live in the private windmill-labs/windmill-herdr — clone it and run ./setup.sh to install them and the keybindings they need.

A worktree from the older webmux setup sets $WEBMUX_WORKTREE_PATH instead and is driven through tmux: tmux list-panes -t "$(tmux display-message -p -t "$TMUX_PANE" '#{window_id}')" -F '#{pane_index} #{pane_current_command}' for what is running, tmux capture-pane for a pane's log.

See backend/AGENTS.md to restart the backend with different cargo features. The commands below are for a plain checkout with nothing running.

  • Backend: cargo run from backend/ (API at http://localhost:8000)
  • Frontend: REMOTE=http://localhost:8000 npm run dev from frontend/ (port 3000+)
  • DB: psql postgres://postgres:changeme@localhost:5432/windmill
  • Login: admin@windmill.dev / changeme
  • Instance settings: navigate to /#superadmin-settings
  • Migrations: use cargo sqlx migrate add -r <name> from backend/ to create new migrations (never generate timestamps manually)

Per-worktree ports and database

.env.local in the worktree root holds the real values — BACKEND_PORT, FRONTEND_PORT, REMOTE, DATABASE_URL, CARGO_FEATURES, WM_DB_NAME. Every manager writes those fields, so it is correct whichever one you are in. Read it with cat .env.local from a shell: the file-read tool denies every .env.* path, and DATABASE_URL is written with an export prefix that a ^DATABASE_URL= grep misses.

herdr keeps nothing besides that file; its hooks write it and read it back. A webmux worktree additionally has $(git rev-parse --git-dir)/webmux/runtime.env, which every pane sources at startup and which carries the extras .env.local lacks: WEBMUX_*, WM_CLONE_DB, USE_RUST_PLUGIN.

In a plain checkout, fall back to .env / .env.local (repo root) and backend/.env.

Each worktree gets a brand-new database, created and migrated from scratch by the post-create hook. It is not a copy of the main dev instance: you get the admins workspace, the admin@windmill.dev superadmin, the license key copied from the base database, and whatever the migrations seed — and none of your own workspaces, scripts, flows or apps. Create whatever a test needs. Cloning the base windmill database instead is opt-in via WM_CLONE_DB (the windmill.worktree plugin's config.env under herdr, or .webmux.yaml under webmux); read the note in .webmux.yaml before turning it on.

The database is named after the worktree directory, not the branch (scripts/worktree-common.sh): windmill_ + the directory basename with - → _, which Postgres then truncates at 63 characters. herdr creates the directory under ~/.herdr/worktrees/<repo>/ from the branch name with each / turned into -, so branch hugo/win-2544-add-options-field-to-the-postgresql-resource-type gets the database windmill_hugo_win_2544_add_options_field_to_the_postgresql_reso — the whole branch name, cut mid-word at the limit. The two drift apart as soon as the branch is renamed, and webmux named its directories after the ticket alone, so reconstructing the name from the branch you are on is wrong in both. Take WM_DB_NAME from .env.local instead. Read that, or discover from what is already running:

# match on the worktree directory, truncated the way Postgres truncates it (63 - len('windmill_')):
psql postgres://postgres:changeme@localhost:5432/postgres -tAc \
  "select datname from pg_database where datname like 'windmill%'" \
  | grep -F "$(basename "$(git rev-parse --show-toplevel)" | tr '/-' '_' | cut -c1-54)"
# the port the frontend actually proxies to (REMOTE of this worktree's vite):
for p in $(pgrep -f vite); do case "$(readlink /proc/$p/cwd)" in *"$(basename "$(git rev-parse --show-toplevel)")"*)
  tr '\0' '\n' < /proc/$p/environ | grep -E '^REMOTE=|^PORT=';; esac; done

Getting these wrong is not a cheap mistake:

  • DATABASE_URL pointed at another worktree's database silently destroys the sqlx cache. cargo run and cargo sqlx prepare both compile sqlx::query! against the live database, so the wrong one fails with relation "<your_new_table>" does not exist — and prepare deletes the whole .sqlx/ directory before it fails, leaving it gutted. Always cp -r backend/.sqlx <tmp>/sqlx_backup first (see the update-sqlx skill).
  • The frontend proxies to its own worktree's backend port, not 8000. Starting a backend on the wrong port leaves the UI up but every API call 502s, which reads like an application bug rather than a misconfiguration.
  • Kill backends by pid scoped to this worktree's cwd (readlink /proc/<pid>/cwd), never pkill -f target/debug/windmill — that kills every sibling worktree's backend. Beware that a pgrep -f "<pattern>" in a shell whose own command line contains <pattern> matches the shell itself.

Code Navigation

wm-ts-nav is an AST-aware code navigator. Use wm-ts-nav for structural queries — it skips comments/strings and understands symbol boundaries.

MUST use outline before Read on unfamiliar files — a 500-line file costs ~500 lines of context, while outline costs ~20. Then MUST use body "X" instead of reading a full file to see one function/struct. Use Read with offset/limit only when you need surrounding context that body doesn't capture.

  • refs "X" --caller instead of reading files to find which function contains each reference
  • callers "X" / callees "X" for call-graph questions

EE files (*_ee.rs, *_ee.ts, *_ee.svelte) are indexed — you can outline, def, body, refs etc. on them just like regular files.

NAV="sh wm-ts-nav/nav"
# Use --root backend for Rust, --root frontend/src for TS/Svelte
$NAV --root backend outline backend/path/to/file.rs      # file structure
$NAV --root backend def "ServiceName"                     # find definition
$NAV --root backend body "decrypt_oauth_data"             # extract source code
$NAV --root backend search "%" --parent ServiceName       # methods on a type
$NAV --root backend search "Trigger" --kind struct        # find by kind
$NAV --root backend refs "X" --file handler.rs --caller   # scoped refs with caller
$NAV --root backend callers "X"                           # who calls X?
$NAV --root backend callees "X"                           # what does X call?

Limitations — syntax-level analysis, no type inference. Use Grep instead when completeness matters (finding all usages, exhaustiveness checks):

  • refs/callers/callees can't follow re-exports, glob imports, or different import paths to the same symbol
  • Trait impls, macro-generated symbols (sqlx::FromRow), and namespace member access (ns.X) are invisible
  • callees shows all identifiers in a function body, not just actual calls

Core Principles

  • MUST outline before Read on unfamiliar files — then body or Read with offset/limit for specifics
  • Scratch stays outside the checkout. Temp scripts, data dumps, cache backups and screenshots go in the session scratch directory or /tmp, so nothing temporary can end up committed. Write the paths in rm/mv/cp out literally: a PreToolUse hook proves each operand, and auto-allows deletes, moves, copies and mode changes under /tmp, inside a git checkout under $HOME, or in the Playwright MCP browser caches (~/Library/Caches/ms-playwright and ms-playwright-mcp, ~/.cache/… on Linux), as long as one operation stays within a single root — a sibling checkout is a root of its own (tar and unzip stay /tmp-only). Chain deletes freely, each proved on its own operands, but keep writes to one per line, name the destination rather than a directory to drop it in, and put anything else on its own line: a command the hook does not prove drops the whole line back to the normal permission flow. A leading ~/ or $HOME/ is expanded and proved; a quoted operand, any other $VAR, a redirect, a $(…), a relative cd, or a wrapper like xargs rm cannot be, and that deferral is what turns a cleanup into a prompt.
  • Change files with Edit/Write, not the shell. sed -i, cat > file <<'EOF' and inline python3 - <<'PY' scripts put an edit through the PreToolUse guards and the permission classifier, which match Bash and nothing else, so a routine edit arrives as a prompt. Bash stays right for running things — tests, builds, git, one-off queries.
  • Search for existing code to reuse before writing new code
  • Follow established patterns in the codebase
  • Keep changes focused — don't refactor beyond what's asked
  • A simpler design found late is still the design. Work already spent is not an argument for a shape, and neither is a clean review round, a passing suite, or a long PR thread. The signal to stop and re-derive rather than patch again is a change that keeps growing to defend its own structure: each review finding fixing an assumption the previous fix broke, the same class of bug reappearing somewhere new, or most of the diff being consequences of one early choice rather than the thing you set out to do. When that happens, say plainly what the simpler design is and what switching costs — a migration, a review cycle restarted from zero, work discarded — and let the user decide. Do not keep paying down the harder one because it is nearly finished, and do not present the accumulated cost as a reason to continue.
  • Ship only the tests the PR needs. A committed test must pin behavior a future change could plausibly break, and be the smallest setup that exercises the new logic. While developing, write as many exhaustive tests and do as much manual testing as you need to convince yourself the change works — then remove that scaffolding before marking the PR ready, keeping only the essential regression guard(s). A test that merely re-exercises pre-existing behavior, or needs elaborate fixtures to assert something trivial, is scaffolding: delete it. If nothing meaningful is left to guard, ship no test rather than a ceremonial one.
  • Comments record constraints, not narration. Write a comment only for what the code can't show: why a non-obvious approach is required, what breaks if it's "simplified" away. State each invariant once, at the place where someone would break it, in ≤4 lines. Don't describe what the next line does, don't repeat the same rationale at multiple sites, and don't address the PR reviewer (justifying a change belongs in the PR description, not the code). Reference nothing ephemeral — no numbered steps from your dev flow, no "the poller / the test does X" scaffolding, no transient state that won't exist for the next reader; keep only the essential, durable rationale. Describe the code as it is, never its drafting history: "we no longer do X", "unchanged behavior", "instead of the previous approach" are meaningless to a reader who never saw the earlier iteration — before finishing, reread your comments as if the current state is the only state that ever existed.
  • Never attribute work to a specific customer, account, or "requested by a customer" in repo-tracked content (PR descriptions, commit messages, code comments, docs). Describe changes by their technical motivation instead.