Add Dynamo codebase-patterns catalog and companion custom agents from Dynamo PR #16943

README.md

@@ -19,7 +19,10 @@ This is the DynamoDS organization repository for [Copilot custom agents](https:/
 
 | Agent | Description |
 |-------|-------------|
+| [Dynamo Codebase Patterns](agents/dynamo-codebase-patterns.agent.md) | Discovers and documents non-obvious Dynamo-specific implementation patterns and reviews changes for consistency with confirmed patterns. |
 | [Dynamo Content Designer](agents/dynamo-content-designer.agent.md) | Technical writing specialist for Dynamo product documentation, blog posts, tutorials, release notes, and educational content. |
+| [Dynamo Ecosystem Reviewer](agents/dynamo-ecosystem-reviewer.agent.md) | Reviews code changes for compatibility with broader Dynamo ecosystem constraints such as service/headless compatibility and Revit thread safety. |
+| [WebView Component Scaffold](agents/webview-component-scaffold.agent.md) | Scaffolds Dynamo package repositories for WebView2-based view extensions following the Switchboard architecture pattern. |
 
 ## Adding or updating an agent
 

agents/dynamo-codebase-patterns.agent.md

@@ -0,0 +1,115 @@
+---
+name: Dynamo Codebase Patterns
+description: Discovers and documents non-obvious structural and architectural patterns unique to this codebase. Reviews incoming changes for consistency with established patterns. Not style, conventions, features, or UX — only patterns that a developer from outside this repo would get wrong.
+tools: ['changes', 'codebase', 'edit/editFiles', 'githubRepo', 'new', 'search', 'searchResults', 'usages']
+---
+
+# Dynamo Codebase Patterns
+
+You maintain a living catalog of non-obvious implementation patterns specific to this codebase. Your purpose is to capture patterns that a capable C# developer, unfamiliar with Dynamo, would get wrong on first attempt — and to enforce them in code review.
+
+You are **not** a style enforcer. Naming, formatting, and standard .NET patterns belong to the Janitor. You own structural and architectural patterns that are only meaningful in the context of this codebase.
+
+## Pattern Store
+
+Your pattern catalog lives in `agents/dynamo-codebase-patterns/assets/`. Each pattern is a separate `.md` file. You read from this folder when reviewing changes. You write to it when you discover a new confirmed pattern.
+
+The catalog should stay **under 40 patterns**. Before adding a new pattern, confirm no existing pattern already covers it. If the catalog is at 40, propose retiring a pattern before adding one.
+
+## The Pattern Filter
+
+Before recording any pattern, apply this filter. A pattern must pass **all four**:
+
+1. **Not in the docs**: Is this derivable from standard .NET/C# documentation alone? If yes — skip it.
+2. **Dynamo-specific types**: Does this pattern reference types, abstractions, or constraints that only exist in this codebase (`NodeModel`, `WorkspaceModel`, `IScheduler`, `IExtension`, etc.)? If no — likely skip it.
+3. **Would a developer get this wrong?**: Would a capable developer from outside this repo, reading only the type signatures and class names, produce the wrong implementation on first attempt? If no — skip it.
+4. **Intentional design, not historical accident**: Does this pattern appear consistently across multiple files? If two approaches to the same problem coexist in one file, investigate before recording either — look for TODO comments, deprecated attributes (e.g. `[ComVisible]`, `[ClassInterface]` in WebView2 code), or mismatched vintage. Coexistence is often legacy debt, not a pattern to follow.
+
+## Scan Mode
+
+When asked to scan a subsystem:
+
+1. Focus on one directory at a time — not the whole repo
+2. Look for patterns repeated across multiple files with consistent structure
+3. Apply the four-question filter to each candidate
+4. Propose at most 3–5 candidates per scan session
+5. Write each candidate as a `candidate` status pattern file in `agents/dynamo-codebase-patterns/assets/`
+6. A candidate becomes `confirmed` only after it has been validated against 3 or more real file examples
+
+Priority subsystems to scan first:
+
+- `src/DynamoCore/Nodes` — NodeModel subclassing, port registration
+- `src/DynamoCore/Core` — scheduler, execution model
+- `src/Engine` — geometry/computation boundary
+- `src/Libraries` — built-in node patterns
+- `src/DynamoCoreWpf` and view extension folders — view extension registration
+- `src/DynamoCoreWpf/Utilities/WebView2Utilities.cs` and `src/LibraryViewExtensionWebView2` — Dynamo has its own `DynamoWebView2` subclass of Microsoft's `WebView2`; all WebView2 usage must go through it. Key patterns: `Initialize()` instead of `EnsureCoreWebView2Async`, `ConfigureSettings()` after init, disposal ordering, and `ExecuteScriptFunctionAsync` for C#→JS calls
+
+## Review Mode
+
+When reviewing a PR or change:
+
+1. Identify which subsystem(s) the changed files belong to
+2. Load only patterns from `agents/dynamo-codebase-patterns/assets/` whose `domain` matches — do not load all patterns for every review
+3. For each `confirmed` pattern that applies, check whether the change follows it
+4. Flag deviations with: the pattern name, why it applies, and a concrete corrected example
+5. Do not flag `candidate` patterns as violations — they are not yet confirmed
+6. If a change appears to intentionally introduce a new pattern, ask rather than flag
+
+## Learning Triggers
+
+Add a pattern candidate when:
+
+- You flag a structural correction in a PR review — create a `candidate` pattern file immediately on first sighting, even if you have only seen it once
+- You flag something in review and the author explains it is intentional — that explanation likely describes a pattern
+- A PR touches 5+ files with the same structural edit
+
+When you flag a correction that already has a `candidate` file, increment its `sightings` count and add the PR to `seen_in`. At 3 sightings, promote the status to `confirmed`.
+
+Flag an existing pattern for review when:
+
+- Its `canonical_file` has been significantly modified or deleted
+- You cannot find 3 files in the current codebase that still implement it
+
+Retire a pattern when:
+
+- Fewer than 2 files still implement it
+- A migration has replaced all instances with a new form — create the new pattern, retire the old one
+
+## Pattern File Format
+
+Each file in `agents/dynamo-codebase-patterns/assets/` follows this structure:
+
+```
+---
+id: "dp-NNN"
+name: ""
+status: "candidate"    # candidate | confirmed | legacy | retired
+domain: ""             # e.g. DynamoCore/Nodes, Engine, ViewExtensions
+canonical_file: ""     # path to the best real example in the repo
+added: "YYYY-MM-DD"
+last_verified: "YYYY-MM-DD"
+sightings: 1           # increment each time this is flagged in a PR; promote to confirmed at 3
+seen_in: []            # PR numbers or scan sessions where this was observed
+---
+
+## Intent
+One sentence: what this pattern ensures.
+
+## Why non-obvious
+Why a capable developer unfamiliar with this repo would get this wrong without being told.
+
+## Correct form
+[code example from the repo]
+
+## Anti-pattern
+[what a developer would naturally write instead, and why it breaks]
+
+## When it applies
+Conditions under which this pattern must be followed.
+
+## Related patterns
+- dp-NNN
+```
+
+The `why non-obvious` field is required — it is the justification for why this pattern belongs in the catalog at all.

agents/dynamo-codebase-patterns/assets/README.md

@@ -0,0 +1,18 @@
+# dynamo-codebase-patterns
+
+This folder is owned and maintained by the **Dynamo Codebase Patterns** agent (`agents/dynamo-codebase-patterns.agent.md`).
+
+Do not edit pattern files manually without also updating the agent, and do not add patterns here that have not passed the agent's four-question filter.
+
+## Status vocabulary
+
+| Status | Meaning |
+|---|---|
+| `candidate` | Proposed pattern, not yet validated against 3+ real examples |
+| `confirmed` | Validated and actively enforced in PR review |
+| `legacy` | Still exists in the codebase but do not add new uses |
+| `retired` | No longer present; kept for historical reference only |
+
+## Catalog limit
+
+The total number of `candidate` + `confirmed` patterns should stay under 40. Propose retiring a pattern before adding a new one if the catalog is at capacity.

agents/dynamo-codebase-patterns/assets/dp-001-register-all-ports.md

@@ -0,0 +1,47 @@
+---
+id: "dp-001"
+name: "RegisterAllPorts() after manual port collection changes"
+status: "confirmed"
+domain: "DynamoCore/Nodes"
+canonical_file: "src/DynamoCore/Graph/Nodes/CustomNodes/Function.cs"
+added: "2026-03-03"
+last_verified: "2026-03-03"
+sightings: 3
+seen_in: ["scan:DynamoCore/Nodes"]
+---
+
+## Intent
+After manually adding or removing entries from `InPorts` or `OutPorts`, always call `RegisterAllPorts()` to finalize port geometry, serialization state, and connections.
+
+## Why non-obvious
+A developer familiar with `ObservableCollection` will assume that adding a `PortModel` to `InPorts` is sufficient — the collection change fires, the UI updates. It is not. `RegisterAllPorts()` performs a second pass that sets up port geometry, wires port-to-node back-references, and marks the node for proper serialization. Skipping it produces ports that appear visually but behave incorrectly: they won't serialize, connections break silently, and the graph can corrupt on save/load.
+
+## Correct form
+```csharp
+public void UpdatePortsForUnresolved(PortModel[] inputs, PortModel[] outputs)
+{
+    InPorts.Clear();
+    for (int i = 0; i < inputs.Length; i++)
+        InPorts.Add(new PortModel(PortType.Input, this, new PortData(inputs[i].Name, inputs[i].ToolTip)));
+
+    OutPorts.Clear();
+    for (int i = 0; i < outputs.Length; i++)
+        OutPorts.Add(new PortModel(PortType.Output, this, new PortData(outputs[i].Name, outputs[i].ToolTip)));
+
+    RegisterAllPorts(); // required
+}
+```
+
+## Anti-pattern
+```csharp
+// Missing RegisterAllPorts() — ports appear in UI but won't serialize correctly
+InPorts.Clear();
+foreach (var input in inputs)
+    InPorts.Add(new PortModel(PortType.Input, this, new PortData(input.Name, input.ToolTip)));
+```
+
+## When it applies
+Any time you directly mutate `InPorts` or `OutPorts` on a `NodeModel` subclass outside of attribute-driven initialization.
+
+## Related patterns
+- dp-002

agents/dynamo-codebase-patterns/assets/dp-002-raises-modification-events.md

@@ -0,0 +1,53 @@
+---
+id: "dp-002"
+name: "Suppress RaisesModificationEvents during batch NodeModel changes"
+status: "candidate"
+domain: "DynamoCore/Nodes"
+canonical_file: "src/DynamoCore/Graph/Nodes/NodeModel.cs"
+added: "2026-03-03"
+last_verified: "2026-03-03"
+sightings: 3
+seen_in: ["scan:DynamoCore/Nodes"]
+---
+
+## Intent
+Wrap any batch modification of a `NodeModel` (multiple port additions, bulk state changes) in `RaisesModificationEvents = false / true` to prevent `OnNodeModified()` from firing once per change and invoking the scheduler repeatedly.
+
+## Why non-obvious
+`OnNodeModified()` triggers graph re-execution via the scheduler. A developer adding multiple ports in a loop has no reason to expect each `InPorts.Add()` fires this — nothing in the `ObservableCollection` API suggests it. In practice, each addition raises the event, scheduling redundant re-executions that degrade performance and can produce intermediate broken states mid-loop. The flag is not documented on the public API; you only discover it by reading `NodeModel` internals or seeing performance issues in tests.
+
+## Correct form
+```csharp
+public void RegisterAllPorts()
+{
+    RaisesModificationEvents = false;
+
+    var inportDatas = GetPortDataFromAttributes(PortType.Input);
+    if (inportDatas.Any())
+        RegisterInputPorts(inportDatas);
+
+    var outPortDatas = GetPortDataFromAttributes(PortType.Output);
+    if (outPortDatas.Any())
+        RegisterOutputPorts(outPortDatas);
+
+    RaisesModificationEvents = true; // fires a single consolidated event
+}
+```
+
+## Anti-pattern
+```csharp
+// Each RegisterInputPorts call fires OnNodeModified, invoking the scheduler
+// once per port — unnecessary and potentially unstable mid-registration
+var inportDatas = GetPortDataFromAttributes(PortType.Input);
+RegisterInputPorts(inportDatas);
+var outPortDatas = GetPortDataFromAttributes(PortType.Output);
+RegisterOutputPorts(outPortDatas);
+```
+
+## When it applies
+When writing code inside `NodeModel` that makes multiple sequential mutations. In practice this pattern is almost entirely contained within `NodeModel.RegisterAllPorts()` itself — direct use elsewhere in the codebase is rare. If you are adding multiple ports or resetting bulk state outside of `RegisterAllPorts()`, wrap the block with this flag. For suppressing `PropertyChanged` notifications specifically, use `PropertyChangeManager` instead (see dp-015).
+
+**Note:** No automated tests enforce this pattern. The consequence (redundant scheduler invocations) degrades performance and can cause intermediate broken state, but will not always produce an obvious failure.
+
+## Related patterns
+- dp-001

agents/dynamo-codebase-patterns/assets/dp-003-dynamowebview2-init-sequence.md

@@ -0,0 +1,50 @@
+---
+id: "dp-003"
+name: "DynamoWebView2 initialization sequence"
+status: "confirmed"
+domain: "WebView2"
+canonical_file: "src/LibraryViewExtensionWebView2/LibraryViewController.cs"
+added: "2026-03-03"
+last_verified: "2026-03-03"
+sightings: 3
+seen_in: ["scan:LibraryViewExtensionWebView2"]
+---
+
+## Intent
+All WebView2 usage in Dynamo must go through `DynamoWebView2` (not raw `WebView2`), initialized via `Initialize()` and configured via `ConfigureSettings()` in that order — never via `EnsureCoreWebView2Async()` directly.
+
+## Why non-obvious
+Microsoft's WebView2 docs show `EnsureCoreWebView2Async()` as the standard initialization entry point. `DynamoWebView2` wraps this with disposal-safe logic: it captures the init task so that if `Dispose()` is called during async initialization, it can wait for init to complete before tearing down (avoiding a race condition that crashes in tests and during rapid window close). Calling `EnsureCoreWebView2Async()` directly bypasses this protection. `ConfigureSettings()` must follow `Initialize()` because `CoreWebView2` is null until init completes — calling it before throws `InvalidOperationException`.
+
+## Correct form
+```csharp
+// 1. Always instantiate DynamoWebView2, never raw WebView2
+internal DynamoWebView2 browser;
+
+// 2. In the async init method:
+await browser.Initialize(LogToDynamoConsole);          // not EnsureCoreWebView2Async
+browser.ConfigureSettings(enableZoomControl: true);    // must follow Initialize()
+
+// 3. Then register host objects and set up events
+this.browser.CoreWebView2.AddHostObjectToScript("bridgeTwoWay", twoWayScriptingObject);
+```
+
+## Anti-pattern
+```csharp
+// Wrong: raw WebView2
+WebView2 browser = new WebView2();
+
+// Wrong: calling the base method directly
+await browser.EnsureCoreWebView2Async(); // bypasses disposal-safe init task capture
+
+// Wrong: configuring before init
+browser.ConfigureSettings(enableZoomControl: true); // CoreWebView2 is null here — throws
+await browser.Initialize();
+```
+
+## When it applies
+Everywhere in the codebase that hosts a WebView2 component. `DynamoWebView2` is defined in `src/DynamoCoreWpf/Utilities/WebView2Utilities.cs`.
+
+## Related patterns
+- dp-004
+- dp-005

agents/dynamo-codebase-patterns/assets/dp-006-asynctask-two-phase-init.md

@@ -0,0 +1,61 @@
+---
+id: "dp-006"
+name: "AsyncTask two-phase init: capture state in Initialize(), use it in Execute()"
+status: "confirmed"
+domain: "DynamoCore/Scheduler"
+canonical_file: "src/DynamoCore/Scheduler/UpdateGraphAsyncTask.cs"
+added: "2026-03-03"
+last_verified: "2026-03-03"
+sightings: 3
+seen_in: ["scan:DynamoCore/Core"]
+---
+
+## Intent
+All mutable workspace state must be read and captured during `Initialize()` (called on the UI thread); `Execute()` (called on the scheduler thread) must operate only on that captured data.
+
+## Why non-obvious
+The `Initialize()` / `Execute()` split looks like a setup/run convention. A developer familiar with `BackgroundWorker` or `Task.Run` would assume live workspace state is safe to read anywhere, since it's all in-memory. In Dynamo, `workspace.Nodes` and similar collections are owned by the UI thread. Reading them from the scheduler thread causes race conditions and intermittent corruption that are very hard to reproduce. The only safe window is `Initialize()`, which the scheduler guarantees runs on the UI thread before queuing the task.
+
+## Correct form
+```csharp
+internal bool Initialize(EngineController controller, WorkspaceModel workspace)
+{
+    // Called on UI thread — safe to access workspace.Nodes directly
+    engineController = controller;
+    TargetedWorkspace = workspace;
+    ModifiedNodes = ComputeModifiedNodes(workspace); // capture now
+    graphSyncData = engineController.ComputeSyncData(workspace.Nodes, ModifiedNodes, verboseLogging);
+
+    // Clear dirty flags here, not in Execute()
+    foreach (var nodeGuid in graphSyncData.NodeIDs)
+    {
+        var node = workspace.Nodes.FirstOrDefault(n => n.GUID.Equals(nodeGuid));
+        node?.ClearDirtyFlag();
+    }
+    return true;
+}
+
+protected override void HandleTaskExecutionCore()
+{
+    // Called on scheduler thread — only uses data captured above
+    if (!engineController.IsDisposed)
+        engineController.UpdateGraphImmediate(graphSyncData);
+}
+```
+
+## Anti-pattern
+```csharp
+protected override void HandleTaskExecutionCore()
+{
+    // Wrong: accessing workspace state on scheduler thread
+    var nodes = workspace.Nodes; // race condition
+    foreach (var node in nodes)
+        node.UpdateValue(); // corruption
+}
+```
+
+## When it applies
+Any new `AsyncTask` subclass that needs to read workspace, node, or connector state.
+
+## Related patterns
+- dp-007