Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 139 additions & 0 deletions .claude/rules/node-standard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
---
paths:
- "internal/domain/composition/**"
---

# The Mill Node Standard

Every `NodeType` registered in `internal/domain/composition` (a
`RegisterNodeType` call, one per node — capture/process/apply/trigger/
decision/terminal) is reviewed against this checklist before it ships.
Adopted, not invented (CLAUDE.md's Research→Plan→Implement): converged
from the published conformance guidelines three real workflow/extension
platforms enforce on third-party nodes/plugins —
[n8n's community-node verification guidelines](https://docs.n8n.io/integrations/creating-nodes/build/reference/verification-guidelines/),
[n8n's UX guidelines](https://docs.n8n.io/integrations/creating-nodes/build/reference/ux-guidelines/),
[n8n's error-handling guidelines](https://docs.n8n.io/integrations/creating-nodes/build/reference/error-handling/),
[Zapier's app publishing requirements](https://platform.zapier.com/publish/app-publishing-requirements),
and the [Raycast store guidelines](https://developers.raycast.com/basics/prepare-an-extension-for-store)
(full research summary: `docs/goals/0030-node-standard.md` item 1,
2026-08-12). Mill's own three hard constraints (§1.1: no phone-home, no
AI API calls, single binary) rule out anything in those guidelines that
assumes a hosted marketplace or a network-calling extension host —
resolved item by item below, not silently dropped.

## The 8-item checklist

| # | Requirement | Enforced by |
|---|---|---|
| 1 | Typed `ConfigField`s, not raw JSON, wherever the shape is expressible | `TestNodeTypes` (`Key`/`Label` non-empty) + `typedfield.Field`'s typed `Type`/`Options`/`Default` — reviewed by eye per new node (no full JSON-schema-vs-typed-field detector exists) |
| 2 | Every `ConfigField` documents itself (`Description` non-empty) | `TestNodeTypes` (nodetypes_test.go) — machine-checked |
| 3 | Declared effect class (`Effect`), never the silently-permissive zero value | `TestNodeTypes` via the closed `pureNodeTypes` allow-list — machine-checked, see below |
| 4 | `Output` names what leaves the step, for every `NodeType` | `TestNodeTypes` — machine-checked, no kind exemption (verified: every registered `NodeType`, `apply-*`/terminal included, already declares one) |
| 5 | ID prefixed by its `Kind`'s naming convention | `TestNodeTypes` via the closed `idPrefixExceptions` allow-list — machine-checked |
| 6 | Fail-safe error semantics: an unevaluable/ambiguous condition counts as the *restrictive* outcome, never silently passes (ruleset's "a rule that cannot evaluate counts as failed"; guardrail's own condition-eval-failure rule) | Reviewed per node at authoring time — see "Error-prefix convention" below for why this stays a review checklist, not a grep-test |
| 7 | Seeded proof at the right layer (a built-in workflow/example exercising the node, or a unit/integration test for pure logic) — `.claude/rules/testing.md`'s layering | `TestBuiltInWorkflows_AllNodesFullyResolvedAndExecutable` + the node's own `*_test.go` |
| 8 | Secrets only via an existing credential-backed entity, never a raw `ConfigField` | Reviewed per node — see "The credential rule" below |

Items 1/4/5/6/8 were already true of every node in this package before
this standard was written down (`TestNodeTypes` already checked Key/
Label; `internal/adapters/credential` already write-only; every
existing node already seeded — item 7's own bar, stricter than any of
the three researched platforms, all of which stop at "document an
example," not "ship a runnable one"). Items 2/3/4-verified/5-verified
are what this goal (0030) added as new machine checks; conformance
audit against all of them found and fixed three real gaps (`list-lookup`/
`list-search` had no declared `Effect`, defaulting to the silently-
permissive zero value despite doing a real local read; `child-workflow`
had the same gap, resolved as explicit `ClassNone` per ADR-0022's own
stated design; `decision-route` had no `Output`) — see the commit that
introduced this file for the fixes.

## The credential rule (item 8)

A `ConfigField` never carries a raw secret (an API key, a bearer
token, a client secret) as its value. Every node that needs
authenticated access to something external goes through an existing
credential-backed entity instead — a `Connector`/`HTTPRequest`
(`internal/domain/connector`, `AuthType` dispatched through a
registered `AuthStrategy`, secret resolved via
`internal/adapters/credential`'s `zalando/go-keyring`-backed, **write-
only** storage) or an `MCPServer`. The node's own `ConfigField` only
ever holds that entity's ID (`RefKind: "request"` /
`RefKind: "mcpserver"` — `docs/adr/0009`), resolved server-side at
execution time; `composition` itself never reads a secret out of
`Node.Config` directly. This is why `integration-http`/`mcp-tool-call`
have no "API key" field of their own — the credential lives one layer
down, behind the picker.

## Effect (item 3) — the priority machine check

`NodeType.Effect`'s Go zero value (`""`) is silently indistinguishable
from `guardrail.ClassNone` at run time
(`composition.NodeTypeEffect`, `execute.go`), and every class except
`ClassExternal` defaults to **allow, no guardrail gate at all**
(`guardrail.DefaultEffect`, ADR-0022). A node with real I/O left at the
zero value runs ungated by accident, not by anyone's decision — the one
genuinely dangerous gap this standard exists to close. `TestNodeTypes`
enforces this with a **closed allow-list**
(`pureNodeTypes` in `nodetypes_test.go`): a node may only leave `Effect`
unset if its ID is on that list, with an inline reason (an entry-point
trigger/`decision-route` whose `exec` is `nil` and never reaches the
gate at all, or a node that provably touches only the in-memory
`ExecContext`, no I/O). Every other node must declare `Effect`
explicitly in its `RegisterNodeType` call — `ruleset`/`human-review`'s
`Effect: guardrail.ClassNone` written out is the house style, not
`ClassNone`-by-omission.

## Explicit rejections

Researched and deliberately NOT adopted, one line each:

- **n8n's publishing ceremony** (npm package naming/versioning, README/
changelog requirements, submission review queue) — Mill has no
hosted marketplace; a node ships in the same binary as everything
else (§1.1's single-binary lock), so there is no separate publish
step to gate.
- **CRUD completeness** (n8n/Zapier's expectation that a resource node
expose create/read/update/delete symmetrically) — Mill's nodes are
workflow *steps*, not resource-management SDKs; a node exposes
whatever operation the workflow actually needs, not a full CRUD
surface speculatively.
- **Raycast's no-keychain rule** (the store checklist steers extensions
away from the OS keychain toward Raycast's own encrypted-preferences
storage) — contradicts Mill's own deliberate `go-keyring` design
(SPEC.md, `internal/adapters/credential`), adopted specifically
*because* it's the OS-native secret store; Raycast's constraint comes
from being a hosted extension platform managing many third-party
extensions' secrets centrally, a shape Mill (single binary, single
user, no hosted anything) doesn't have.

## `NodeType`-level versioning — latent, not built

Every one of the three researched platforms versions nodes/extensions
independently of the app that hosts them (n8n's node `version` field,
Zapier's app versions, Raycast's extension releases) so an existing
workflow keeps running against the node shape it was authored with
while a newer node version ships. Mill has the identical real need
(changing a `NodeType`'s `ConfigFields` today can silently break a
persisted `Node.Config`) but nothing analogous is built —
`Workflow.Versions`/`PublishedVersion` (ADR-0021) version the
*workflow*, not the `NodeType` definitions it references. Named here so
it isn't rediscovered as a surprise; not built speculatively ahead of a
concrete `NodeType` shape change that needs it (CLAUDE.md's Research→
Plan→Implement, same discipline `ConfigFieldType`'s own doc comment in
`types.go` already applies to Decision/Parallel's unbuilt field types).

## Error-prefix convention (item 6/8's sibling — reviewed, not grep-tested)

Every `nodeExec` function's returned errors are prefixed with that node
type's ID (e.g. `"child-workflow: %w"`, `"list-lookup: %w"` — see any
`*.go` file in this package) so a run's error trail names which step
failed without re-deriving it from context. This is checked at code
review time, not by an automated test: a test that greps this
package's `*.go` source for `return ctx, fmt.Errorf(...)` call sites to
verify a literal ID-prefix convention is exactly the kind of brittle
grep-over-source check that breaks on a harmless refactor (an extracted
helper, a wrapped error, a renamed local) without catching a real
regression. Skipped deliberately, not by oversight — revisit only if
this convention actually regresses in a way code review misses.
1 change: 1 addition & 0 deletions docs/SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -1661,6 +1661,7 @@ Plan step for this as a standing rule.
| **Guardrail preview / policy gate** | Approve/deny before a step actually runs | Build (core domain: `internal/domain/guardrail`); durable parking: adopt (DBOS `Send`/`Recv`/`SetEvent`, already adopted §7) | `LOCKED`, built — §8/ADR-0022: effect classes on every NodeType, ambient gate + explicit "Wait for approval" node, Configure → Guardrails authoring + dry-run tester |
| **Human review / HITL step** | Pause a run for a person: queue it, take their typed input, resume or stop | Park mechanism: adopt (DBOS Send/Recv, §7). Queue surface + input model: build (thin — composed over ListRuns + pending, not a case-management engine) | `LOCKED`, built — ADR-0023: `human-review` node + the Review queue (sidebar); reviewer input coerces via the same path as the test-input form |
| **Ruleset validation** | Validate the payload/attributes flowing through a step against named business rules | Data model: build (JDM's shape reduced — GoRules ZEN is CGO/Rust, disqualified; grule rejected). Evaluation: adopt (`expr-lang`, already adopted) | `LOCKED`, built — ADR-0023: `ruleset` node, fail-safe (unevaluable rule counts as failed), failures named per rule |
| **Node standard** (minimum conformance every `NodeType` is reviewed against) | Adopt a published node/plugin conformance checklist rather than reviewing each new node ad hoc | Adopt the converged checklist (research against n8n's community-node verification/UX/error-handling guidelines, Zapier's publishing requirements, Raycast's store checklist — never invented); build the enforcement itself, since no library has an opinion on Mill's own `NodeType` shape | `LOCKED`, built — goal 0030, `.claude/rules/node-standard.md`: an 8-item checklist, 5 items machine-checked in `TestNodeTypes` (`nodetypes_test.go`) — `ConfigField.Description` non-empty, an explicit `Effect` class (closed `pureNodeTypes` allow-list catches the zero-value-silently-means-ClassNone/allow danger), `Output` non-empty universally, ID prefixed by its `Kind` (closed `idPrefixExceptions` allow-list for pre-pattern IDs); the error-prefix convention stays review-checked, not grep-tested (fragile-test tradeoff, recorded in the rule file). The audit fixed three real gaps: `list-lookup`/`list-search`/`child-workflow` had no declared `Effect` (child-workflow's fix makes ADR-0022's already-decided `ClassNone` explicit rather than accidental; list-lookup/list-search get `ClassRead`, matching capture-file's precedent), and `decision-route` had no `Output`. `NodeType`-level versioning is named as a real, latent gap (independent of `Workflow.Versions`) — not built speculatively ahead of a concrete need |
| **Visual composition surface** | Author a DAG, not just a list | Adopt (React Flow / `@xyflow/react`) — built ahead of ADR-0005 B2's original deferral trigger, by explicit decision (see the ADR's Update section) | §3, `CompositionCanvas.tsx`, `UX: PROTOTYPE`. **View vs. edit mode (goal 0022):** a workflow row click opens the canvas READ-ONLY — React Flow interactions inert (`nodesDraggable`/`nodesConnectable`/`deleteKeyCode` off; `elementsSelectable` stays on, so a node's config is still inspectable), no authoring toolbar, `NodeInspector` wrapped in a disabled `<fieldset>` (cascades to every sub-editor, no per-field prop threading); Run/step-debug/Runs/Versions all work. Edit is the explicit switch (the row pencil, or a canvas Edit button) — same tab, in place, no remount (`mode: 'view'\|'edit'` on the `workflow-edit` WorkTabSpec). Extends ADR-0014's inspect-vs-edit split (built for Integrations) to workflows |

**React Flow, checked directly against its actual source/docs (not
Expand Down
48 changes: 0 additions & 48 deletions docs/goals/0030-node-standard.md

This file was deleted.

18 changes: 14 additions & 4 deletions docs/goals/BACKLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,20 @@ this pipeline and on this code)**
the DEV·live badge's Go-liveness blind spot, now having claimed a
second scalp (ADR-0035's Consequences note); a third badge state
(amber DEV·go-stale) not yet built.
8. [ ] [0030 — Node standard](0030-node-standard.md) — owner-mandated
2026-08-12: a written, precedent-researched (n8n community-node
review) conformance standard every NodeType is checked against;
not started.
8. [x] [0030 — Node standard](archive/0030-node-standard.md) —
DELIVERED 2026-08-12: `.claude/rules/node-standard.md` (8-item
checklist, citing n8n's community-node/UX/error-handling
guidelines, Zapier's publishing requirements, Raycast's store
checklist; explicit rejections + the credential rule +
NodeType-versioning-is-latent note); `TestNodeTypes`
(nodetypes_test.go) machine-checks 4 of the 8 items (Description
non-empty, explicit Effect via a closed pureNodeTypes allow-list,
universal Output, Kind-ID-prefix via a closed idPrefixExceptions
allow-list) — the error-prefix convention stays review-checked,
not grep-tested (fragility tradeoff recorded in the rule file).
Audit found and fixed 3 real gaps: list-lookup/list-search/
child-workflow had no declared Effect (silently defaulting to the
permissive zero value), decision-route had no Output.
9. [ ] [0031 — AI node family](0031-ai-node-family.md) — owner-engaged
2026-08-12: the guardrailed AI-node family (n8n/Make/Zapier/
Dify taxonomy convergence), Mill's category-defining capability;
Expand Down
Loading
Loading