From 0f43e41c161ca4fee451a1fb6260b4ef2cff9791 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 02:00:50 +0000 Subject: [PATCH 1/2] Add Dynamo codebase patterns and related custom agents Agent-Logs-Url: https://github.com/DynamoDS/.github-private/sessions/f28666c4-424e-436e-84bd-fa1555b2786f Co-authored-by: avidit <1406153+avidit@users.noreply.github.com> --- README.md | 3 + agents/dynamo-codebase-patterns.agent.md | 115 ++++++++++++++++++ .../dynamo-codebase-patterns/assets/README.md | 18 +++ .../assets/dp-001-register-all-ports.md | 47 +++++++ .../dp-002-raises-modification-events.md | 53 ++++++++ .../dp-003-dynamowebview2-init-sequence.md | 50 ++++++++ .../assets/dp-006-asynctask-two-phase-init.md | 61 ++++++++++ ...dp-007-task-completion-scheduler-thread.md | 54 ++++++++ .../assets/dp-008-viewextension-lifecycle.md | 67 ++++++++++ .../assets/dp-009-workspace-type-guard.md | 47 +++++++ .../assets/dp-010-keepreference-ffi.md | 45 +++++++ .../dp-011-arbitrary-dimension-array.md | 45 +++++++ .../assets/dp-012-dispose-derived-geometry.md | 53 ++++++++ .../assets/dp-013-isvisibleindynamolibrary.md | 42 +++++++ .../assets/dp-014-asynctask-canmergewith.md | 64 ++++++++++ .../assets/dp-015-property-change-manager.md | 50 ++++++++ agents/dynamo-ecosystem-reviewer.agent.md | 25 ++++ agents/webview-component-scaffold.agent.md | 82 +++++++++++++ 18 files changed, 921 insertions(+) create mode 100644 agents/dynamo-codebase-patterns.agent.md create mode 100644 agents/dynamo-codebase-patterns/assets/README.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-001-register-all-ports.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-002-raises-modification-events.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-003-dynamowebview2-init-sequence.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-006-asynctask-two-phase-init.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-007-task-completion-scheduler-thread.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-008-viewextension-lifecycle.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-009-workspace-type-guard.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-010-keepreference-ffi.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-011-arbitrary-dimension-array.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-012-dispose-derived-geometry.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-013-isvisibleindynamolibrary.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-014-asynctask-canmergewith.md create mode 100644 agents/dynamo-codebase-patterns/assets/dp-015-property-change-manager.md create mode 100644 agents/dynamo-ecosystem-reviewer.agent.md create mode 100644 agents/webview-component-scaffold.agent.md diff --git a/README.md b/README.md index 675329f..dc06c4c 100644 --- a/README.md +++ b/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 diff --git a/agents/dynamo-codebase-patterns.agent.md b/agents/dynamo-codebase-patterns.agent.md new file mode 100644 index 0000000..25169fe --- /dev/null +++ b/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 three-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. diff --git a/agents/dynamo-codebase-patterns/assets/README.md b/agents/dynamo-codebase-patterns/assets/README.md new file mode 100644 index 0000000..3c4bf0f --- /dev/null +++ b/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 three-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. diff --git a/agents/dynamo-codebase-patterns/assets/dp-001-register-all-ports.md b/agents/dynamo-codebase-patterns/assets/dp-001-register-all-ports.md new file mode 100644 index 0000000..67028d4 --- /dev/null +++ b/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 diff --git a/agents/dynamo-codebase-patterns/assets/dp-002-raises-modification-events.md b/agents/dynamo-codebase-patterns/assets/dp-002-raises-modification-events.md new file mode 100644 index 0000000..af19ed2 --- /dev/null +++ b/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 diff --git a/agents/dynamo-codebase-patterns/assets/dp-003-dynamowebview2-init-sequence.md b/agents/dynamo-codebase-patterns/assets/dp-003-dynamowebview2-init-sequence.md new file mode 100644 index 0000000..3d34825 --- /dev/null +++ b/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 diff --git a/agents/dynamo-codebase-patterns/assets/dp-006-asynctask-two-phase-init.md b/agents/dynamo-codebase-patterns/assets/dp-006-asynctask-two-phase-init.md new file mode 100644 index 0000000..acf1d25 --- /dev/null +++ b/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 diff --git a/agents/dynamo-codebase-patterns/assets/dp-007-task-completion-scheduler-thread.md b/agents/dynamo-codebase-patterns/assets/dp-007-task-completion-scheduler-thread.md new file mode 100644 index 0000000..f80c941 --- /dev/null +++ b/agents/dynamo-codebase-patterns/assets/dp-007-task-completion-scheduler-thread.md @@ -0,0 +1,54 @@ +--- +id: "dp-007" +name: "Task completion handlers run on the scheduler thread" +status: "confirmed" +domain: "DynamoCore/Scheduler" +canonical_file: "src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs" +added: "2026-03-03" +last_verified: "2026-03-03" +sightings: 3 +seen_in: ["scan:DynamoCore/Core"] +--- + +## Intent +Handlers subscribed to `AsyncTask.Completed` run on the scheduler thread, not the UI thread — any UI updates or WPF element access inside them must be marshaled via the dispatcher. + +## Why non-obvious +Subscribing to a `Completed` event looks identical to any other .NET event subscription. Every other completion callback pattern in WPF (`BackgroundWorker.RunWorkerCompleted`, `Task.ContinueWith` with a UI scheduler) runs back on the UI thread by default. Dynamo's scheduler does not do this. A handler that touches a WPF control, raises `PropertyChanged` on a bound property, or modifies an `ObservableCollection` directly will either throw a cross-thread exception or silently corrupt state. + +## Correct form +```csharp +// Documented in source: "This callback is invoked in the context of ISchedulerThread" +private void OnUpdateGraphCompleted(AsyncTask task) +{ + // Safe: only uses data captured during Initialize(), not live workspace state + var updateTask = (UpdateGraphAsyncTask)task; + var workspace = updateTask.TargetedWorkspace; + + // Safe: working with pre-captured node data + foreach (var warning in warnings) + { + var node = workspace.Nodes.FirstOrDefault(n => n.GUID == guid); + node?.Warning(warning.Value); + } + + // If UI update needed, marshal explicitly: + Dispatcher.BeginInvoke(() => SomeUIProperty = newValue); +} +``` + +## Anti-pattern +```csharp +task.Completed += (t) => +{ + // Wrong: accessing live UI state from scheduler thread + var node = dynamoViewModel.CurrentSpaceViewModel.Nodes[0]; // cross-thread exception + someObservableCollection.Add(item); // silent corruption +}; +``` + +## When it applies +Any code that subscribes to `AsyncTask.Completed` or `EvaluationCompleted` events on workspace models. + +## Related patterns +- dp-006 diff --git a/agents/dynamo-codebase-patterns/assets/dp-008-viewextension-lifecycle.md b/agents/dynamo-codebase-patterns/assets/dp-008-viewextension-lifecycle.md new file mode 100644 index 0000000..e371ff8 --- /dev/null +++ b/agents/dynamo-codebase-patterns/assets/dp-008-viewextension-lifecycle.md @@ -0,0 +1,67 @@ +--- +id: "dp-008" +name: "IViewExtension: store ViewLoadedParams, unsubscribe everything in Dispose()" +status: "confirmed" +domain: "ViewExtensions" +canonical_file: "src/LintingViewExtension/LintingViewExtension.cs" +added: "2026-03-03" +last_verified: "2026-03-03" +sightings: 3 +seen_in: ["scan:DynamoCoreWpf"] +--- + +## Intent +Store the `ViewLoadedParams` reference as a field in `Loaded()` and unsubscribe every event handler registered there in `Dispose()`. + +## Why non-obvious +`Loaded()` is the only guaranteed entry point to Dynamo's UI infrastructure (menus, sidebar, `DynamoViewModel`, `DynamoModel`). A developer who doesn't store the reference loses access to all of these for the extension's remaining lifetime. More critically: event handlers registered in `Loaded()` on `MenuItem`, `viewLoadedParams`, and model services are not automatically cleaned up when the extension is disposed. The extension object persists as a GC root held by those events, leaking memory and firing handlers after logical shutdown. + +## Correct form +```csharp +public override void Loaded(ViewLoadedParams viewLoadedParams) +{ + this.viewLoadedParamsReference = viewLoadedParams; // store it + + this.linterMenuItem = new MenuItem { Header = Resources.MenuItemText, IsCheckable = true }; + this.linterMenuItem.Checked += MenuItemCheckHandler; // remember to unsubscribe + this.linterMenuItem.Unchecked += MenuItemUnCheckedHandler; // remember to unsubscribe + + viewLoadedParams.ViewExtensionOpenRequest += OnViewExtensionOpenRequest; // remember to unsubscribe + this.linterManager.PropertyChanged += OnLinterManagerPropertyChange; // remember to unsubscribe +} + +public override void Dispose() +{ + // Mirror of Loaded() — every += has a matching -= + if (linterMenuItem != null) + { + linterMenuItem.Checked -= MenuItemCheckHandler; + linterMenuItem.Unchecked -= MenuItemUnCheckedHandler; + } + viewLoadedParamsReference?.ViewExtensionOpenRequest -= OnViewExtensionOpenRequest; + linterManager?.PropertyChanged -= OnLinterManagerPropertyChange; +} +``` + +## Anti-pattern +```csharp +public override void Loaded(ViewLoadedParams viewLoadedParams) +{ + // Not stored — can't unsubscribe later, can't access services after this method returns + viewLoadedParams.ViewExtensionOpenRequest += OnViewExtensionOpenRequest; + linterManager.PropertyChanged += OnLinterManagerPropertyChange; +} + +public override void Dispose() +{ + // Can't unsubscribe — memory leak, handlers fire after disposal +} +``` + +## When it applies +Every `IViewExtension` implementation. + +**Known violation:** `GraphNodeManagerViewExtension` subscribes to `graphNodeManagerMenuItem.Checked` and `graphNodeManagerMenuItem.Unchecked` in `Loaded()` but never unsubscribes them in `Dispose()` — a live memory leak in the codebase. PRs touching that file should flag it. + +## Related patterns +- dp-009 diff --git a/agents/dynamo-codebase-patterns/assets/dp-009-workspace-type-guard.md b/agents/dynamo-codebase-patterns/assets/dp-009-workspace-type-guard.md new file mode 100644 index 0000000..3fe7231 --- /dev/null +++ b/agents/dynamo-codebase-patterns/assets/dp-009-workspace-type-guard.md @@ -0,0 +1,47 @@ +--- +id: "dp-009" +name: "Guard against CustomNodeWorkspaceModel before creating extension UI" +status: "confirmed" +domain: "ViewExtensions" +canonical_file: "src/NodeAutoCompleteViewExtension/NodeAutoCompleteViewExtension.cs" +added: "2026-03-03" +last_verified: "2026-03-03" +sightings: 3 +seen_in: ["scan:DynamoCoreWpf"] +--- + +## Intent +Before accessing `HomeWorkspaceModel`-specific APIs from a view extension, check that `CurrentWorkspaceModel` is a `HomeWorkspaceModel` — if not, call `this.Closed()` and return. + +## Why non-obvious +Dynamo has two workspace types: `HomeWorkspaceModel` (the main graph) and `CustomNodeWorkspaceModel` (opened when editing a custom node). `CurrentWorkspaceModel` can be either at the time an extension activates. A developer building an extension assumes there is one workspace type. Attempting to cast to `HomeWorkspaceModel`, accessing home-workspace-specific services, or adding sidebar UI when a custom node workspace is active causes `InvalidCastException` or broken UI state that persists after the user returns to the home workspace. + +## Correct form +```csharp +internal void AddToSidebar() +{ + if (!(this.viewLoadedParamsReference.CurrentWorkspaceModel is HomeWorkspaceModel)) + { + this.Closed(); // signal: this extension cannot operate here + return; + } + + this.viewLoadedParamsReference?.AddToExtensionsSideBar(this, DependencyView); +} +``` + +## Anti-pattern +```csharp +internal void AddToSidebar() +{ + // Wrong: assumes CurrentWorkspaceModel is always HomeWorkspaceModel + var homeWorkspace = (HomeWorkspaceModel)viewLoadedParamsReference.CurrentWorkspaceModel; // throws + viewLoadedParamsReference.AddToExtensionsSideBar(this, DependencyView); +} +``` + +## When it applies +Only when your extension accesses `HomeWorkspaceModel`-specific APIs (casting `CurrentWorkspaceModel` to `HomeWorkspaceModel`, accessing home-workspace services or state). Extensions that only add generic sidebar UI or menu items do not need this check — most extensions in the codebase don't implement it. Call the check at the point of API access, not in `Loaded()`. + +## Related patterns +- dp-008 diff --git a/agents/dynamo-codebase-patterns/assets/dp-010-keepreference-ffi.md b/agents/dynamo-codebase-patterns/assets/dp-010-keepreference-ffi.md new file mode 100644 index 0000000..f2fd5cf --- /dev/null +++ b/agents/dynamo-codebase-patterns/assets/dp-010-keepreference-ffi.md @@ -0,0 +1,45 @@ +--- +id: "dp-010" +name: "[KeepReference] on FFI parameters whose lifetime must outlive the call" +status: "confirmed" +domain: "Engine/FFI" +canonical_file: "src/Libraries/GeometryColor/GeometryColor.cs" +added: "2026-03-03" +last_verified: "2026-03-03" +sightings: 3 +seen_in: ["scan:Engine"] +--- + +## Intent +Apply `[KeepReference]` to any FFI method parameter whose value is stored as a field on the returned object, so the DesignScript GC does not dispose the parameter while the return object still holds a reference to it. + +## Why non-obvious +In standard .NET, returning an object that holds a field reference to a parameter keeps the parameter alive automatically through the object graph. At the DesignScript FFI boundary this is not true: DesignScript tracks `StackValue` handles independently. The GC can mark a parameter's `StackValue` for disposal even if a C# field still references the underlying object, because the GC only sees the DesignScript heap. Without `[KeepReference]`, the returned object holds a dangling C# reference to a disposed geometry wrapper — calls on that geometry produce undefined behavior or crashes, often non-deterministically. + +## Correct form +```csharp +public static GeometryColor ByGeometryColor( + [KeepReference] Geometry geometry, // geometry is stored as a field — must keep reference + Color color) +{ + if (geometry == null) throw new ArgumentNullException("geometry"); + return new GeometryColor(geometry, color); +} +``` + +The marshaler detects `[KeepReference]`, adds the parameter's `StackValue` to `referencedParameters`, then expands the return object's heap allocation to embed the reference as a hidden slot — ensuring the GC sees the dependency. + +## Anti-pattern +```csharp +// Wrong: geometry will be independently disposed by DesignScript GC +public static GeometryColor ByGeometryColor(Geometry geometry, Color color) +{ + return new GeometryColor(geometry, color); // dangling reference after GC +} +``` + +## When it applies +Any zero-touch FFI method that stores a parameter in a field of the returned object. Also combine with `[KeepReference]` on `IList` parameters when the list values are stored (see dp-011). + +## Related patterns +- dp-011 diff --git a/agents/dynamo-codebase-patterns/assets/dp-011-arbitrary-dimension-array.md b/agents/dynamo-codebase-patterns/assets/dp-011-arbitrary-dimension-array.md new file mode 100644 index 0000000..9ee5def --- /dev/null +++ b/agents/dynamo-codebase-patterns/assets/dp-011-arbitrary-dimension-array.md @@ -0,0 +1,45 @@ +--- +id: "dp-011" +name: "[ArbitraryDimensionArrayImport] to preserve nested array structure across FFI" +status: "confirmed" +domain: "Engine/FFI" +canonical_file: "src/Libraries/DesignScriptBuiltin/Dictionary.cs" +added: "2026-03-03" +last_verified: "2026-03-03" +sightings: 3 +seen_in: ["scan:Engine"] +--- + +## Intent +Apply `[ArbitraryDimensionArrayImport]` to FFI parameters or return values that should be typed as `var[]..[]` in the DS VM, preventing the VM from attempting automatic replication (lacing) over them. + +## Why non-obvious +Without this attribute, a collection parameter (`IList`, `IEnumerable`) is imported into the DS VM as a fixed-rank array. The VM will then try to auto-replicate over its elements when the function is called — iterating over the list and invoking the function once per element rather than passing the whole list as a single argument. This is the correct behaviour for most node inputs, but wrong for parameters that are meant to receive the entire collection as one value (e.g. a dictionary's values list, or a list being passed to a function that operates on the whole list at once). Adding the attribute tells the VM to type the parameter as `var[]..[]` (arbitrary rank), so it is always passed wholesale without replication. + +## Correct form +```csharp +// Parameter: whole list passed to C# as-is, VM does not replicate over it +public static Dictionary ByKeysValues( + IList keys, + [KeepReference][ArbitraryDimensionArrayImport] IList values) +{ ... } + +// Return: VM treats return value as var[]..[] regardless of actual structure +public IEnumerable Values +{ + [return: ArbitraryDimensionArrayImport] + get => D.Values; +} +``` + +## Anti-pattern +```csharp +// Wrong: VM will replicate over 'values', calling ByKeysValues once per element +public static Dictionary ByKeysValues(IList keys, IList values) { ... } +``` + +## When it applies +FFI parameters or return values where you need the DS VM to treat the collection as a single `var[]..[]` value rather than replicating over it. Typical cases: parameters that receive an entire list as one argument (dictionary values, sort comparators), and return values that themselves contain lists. When also storing the parameter as a field on the return object, combine with `[KeepReference]` (see dp-010). + +## Related patterns +- dp-010 diff --git a/agents/dynamo-codebase-patterns/assets/dp-012-dispose-derived-geometry.md b/agents/dynamo-codebase-patterns/assets/dp-012-dispose-derived-geometry.md new file mode 100644 index 0000000..2c44c8e --- /dev/null +++ b/agents/dynamo-codebase-patterns/assets/dp-012-dispose-derived-geometry.md @@ -0,0 +1,53 @@ +--- +id: "dp-012" +name: "Dispose derived geometry objects immediately after use" +status: "confirmed" +domain: "Engine/Geometry" +canonical_file: "src/Libraries/GeometryColor/GeometryColor.cs" +added: "2026-03-03" +last_verified: "2026-03-03" +sightings: 3 +seen_in: ["scan:Engine"] +--- + +## Intent +Explicitly dispose derived geometry objects (`PerimeterCurves()`, `Edges`, `CurveGeometry`, etc.) immediately after use rather than relying on LibG's deferred cleanup mechanism. + +## Why non-obvious +LibG suppresses the SWIG finalizers on geometry objects and replaces them with a deferred disposal system: when a geometry object is finalized by the CLR, it is added to a per-thread pending list rather than immediately releasing its native resource. Native resources are then released either when the next ProtoGeometry object is created on that thread (triggering a flush of the pending list) or at post-evaluation via a `postEvaluation` handler on `HostFactory`. + +This means skipping `Dispose()` does not cause an immediate crash or permanent leak — LibG will eventually clean up. However, the timing is non-deterministic: within a single graph evaluation, transient geometry objects accumulate in the pending list and hold native ASM resources until the next creation flush or end-of-evaluation cleanup. In tight tessellation loops over complex geometry this accumulation can be significant. The `postEvaluation` cleanup also has known uncertainty in sandbox mode (the thread on which it runs may differ from the object-creating thread, leaving some objects uncleared). + +Explicit `Dispose()` releases the native resource immediately and deterministically, avoiding all of the above. + +## Correct form +```csharp +foreach (var curve in surf.PerimeterCurves()) +{ + curve.Tessellate(package, parameters); + curve.Dispose(); // release immediately, don't wait for deferred cleanup +} + +foreach (var geom in solid.Edges.Select(edge => edge.CurveGeometry)) +{ + geom.Tessellate(package, parameters); + geom.Dispose(); // release immediately +} +``` + +## Anti-pattern +```csharp +// Not a crash, but native resources accumulate in LibG's pending list +// until the next object creation or end-of-evaluation flush +foreach (var curve in surf.PerimeterCurves()) +{ + curve.Tessellate(package, parameters); + // no Dispose() — resource held until deferred cleanup +} +``` + +## When it applies +Any code that calls APIs returning transient derived geometry: `PerimeterCurves()`, `Edges`, `CurveGeometry`, `Faces`, `Vertices`, and similar enumeration methods on `Surface`, `Solid`, and `Curve` types. Does not apply to geometry objects returned to the graph as node outputs — those are managed by the DesignScript GC and LibG's deferred system is the intended mechanism for them. + +## Related patterns +- dp-010 diff --git a/agents/dynamo-codebase-patterns/assets/dp-013-isvisibleindynamolibrary.md b/agents/dynamo-codebase-patterns/assets/dp-013-isvisibleindynamolibrary.md new file mode 100644 index 0000000..d34677f --- /dev/null +++ b/agents/dynamo-codebase-patterns/assets/dp-013-isvisibleindynamolibrary.md @@ -0,0 +1,42 @@ +--- +id: "dp-013" +name: "[IsVisibleInDynamoLibrary(false)] to hide public members from the library browser" +status: "confirmed" +domain: "Libraries" +canonical_file: "src/Libraries/CoreNodes/List.cs" +added: "2026-03-03" +last_verified: "2026-03-03" +sightings: 3 +seen_in: ["scan:Libraries"] +--- + +## Intent +Use `[IsVisibleInDynamoLibrary(false)]` to exclude public C# methods or classes from appearing in the Dynamo library browser, without making them internal to the assembly. + +## Why non-obvious +Standard .NET visibility (`public` / `internal`) is the only access control a developer expects. In Dynamo, the library browser surfaces all `public` members of loaded assemblies as user-facing nodes — there is no middle ground in C# visibility between "accessible to C# code" and "visible to users." `[IsVisibleInDynamoLibrary(false)]` provides that middle ground. Without it, internal helper methods, backward-compatibility shims, and overloads not intended for end users all appear as nodes in the library, polluting the user-facing API surface. + +## Correct form +```csharp +[IsVisibleInDynamoLibrary(false)] // hidden from library browser, still callable from C# +public static bool Contains(IList list, [ArbitraryDimensionArrayImport] object item) +{ ... } + +// Can also be applied at class level to hide the entire class +[IsVisibleInDynamoLibrary(false)] +public static class InternalHelpers { ... } +``` + +## Anti-pattern +```csharp +// Wrong: public method appears as a node in the library browser +public static bool Contains(IList list, object item) // visible to all users +{ ... } +``` + +## When it applies +Any `public` method in a zero-touch library assembly that should be accessible from C# (e.g. for use by other nodes or core code) but is not intended as a user-facing node. Common cases: backward-compatibility overloads, helper methods called by other public methods, methods with parameter types that don't translate cleanly to the node UI. + +## Related patterns +- dp-010 +- dp-011 diff --git a/agents/dynamo-codebase-patterns/assets/dp-014-asynctask-canmergewith.md b/agents/dynamo-codebase-patterns/assets/dp-014-asynctask-canmergewith.md new file mode 100644 index 0000000..6194f9d --- /dev/null +++ b/agents/dynamo-codebase-patterns/assets/dp-014-asynctask-canmergewith.md @@ -0,0 +1,64 @@ +--- +id: "dp-014" +name: "Override CanMergeWithCore() on new AsyncTask types to prevent queue explosion" +status: "candidate" +domain: "DynamoCore/Scheduler" +canonical_file: "src/DynamoCore/Scheduler/UpdateGraphAsyncTask.cs" +added: "2026-03-03" +last_verified: "2026-03-03" +sightings: 1 +seen_in: ["scan:DynamoCore/Core"] +--- + +## Intent +`AsyncTask` subclasses that are scheduled at high frequency must override `CanMergeWithCore()` with correct merge logic — the default `KeepBoth` allows the task queue to grow unboundedly when tasks are scheduled faster than they execute. + +## Why non-obvious +`CanMergeWithCore()` has a safe-looking default (`KeepBoth`) that compiles and runs without error. The problem only manifests under load: rapid graph edits or event floods schedule many tasks that queue up instead of being coalesced. The merge logic is also easy to get wrong — `UpdateGraphAsyncTask` only merges a newer task over an older one if the newer task's changes are a *superset* of the older task's (`CanReplace()`). Naively returning `KeepOther` for any same-type task drops pending additions/deletions that weren't yet included in the newer task's `graphSyncData`. + +## Correct form +```csharp +protected override TaskMergeInstruction CanMergeWithCore(AsyncTask otherTask) +{ + var other = otherTask as UpdateGraphAsyncTask; + if (other == null) + return base.CanMergeWithCore(otherTask); + + // Only merge if the surviving task's changes are a superset of the dropped task's + if (other.IsScheduledAfter(this) && other.CanReplace(this)) + return TaskMergeInstruction.KeepOther; + if (this.IsScheduledAfter(other) && this.CanReplace(other)) + return TaskMergeInstruction.KeepThis; + + return TaskMergeInstruction.KeepBoth; +} + +private bool CanReplace(UpdateGraphAsyncTask other) +{ + // First check ModifiedNodes directly — bail early if other has nodes we don't cover + if (!other.ModifiedNodes.All(ModifiedNodes.Contains)) + return false; + + // Then confirm no adds or deletes are pending in the other task + return other.graphSyncData.AddedNodeIDs.Count() == 0 && + other.graphSyncData.ModifiedNodeIDs.All(graphSyncData.ModifiedNodeIDs.Contains) && + other.graphSyncData.DeletedNodeIDs.Count() == 0; +} +``` + +## Anti-pattern +```csharp +// Wrong: drops queued adds/deletes if any newer task arrives +protected override TaskMergeInstruction CanMergeWithCore(AsyncTask otherTask) +{ + if (otherTask is UpdateGraphAsyncTask) + return TaskMergeInstruction.KeepOther; // silently loses pending graph changes + return TaskMergeInstruction.KeepBoth; +} +``` + +## When it applies +When implementing a new `AsyncTask` subclass that may be scheduled at high frequency (e.g. in response to property change events or UI interactions). + +## Related patterns +- dp-006 diff --git a/agents/dynamo-codebase-patterns/assets/dp-015-property-change-manager.md b/agents/dynamo-codebase-patterns/assets/dp-015-property-change-manager.md new file mode 100644 index 0000000..df7171e --- /dev/null +++ b/agents/dynamo-codebase-patterns/assets/dp-015-property-change-manager.md @@ -0,0 +1,50 @@ +--- +id: "dp-015" +name: "Use PropertyChangeManager with a using block to suppress PropertyChanged notifications" +status: "confirmed" +domain: "DynamoCore/Nodes" +canonical_file: "src/DynamoCore/Core/PropertyChangeManager.cs" +added: "2026-03-03" +last_verified: "2026-03-03" +sightings: 3 +seen_in: ["scan:DynamoCore/Core"] +--- + +## Intent +When setting multiple properties on a `NotificationObject` (including `NodeModel`) that would each fire `PropertyChanged`, suppress them for the duration of the batch using `SetPropsToSuppress()` in a `using` block. + +## Why non-obvious +`NotificationObject` inherits `PropertyChangeManager` as an internal field — it is not visible on the public API. A developer who needs to suppress `PropertyChanged` during a batch update would either set a manual flag, unsubscribe and re-subscribe the event handler, or not suppress at all and accept redundant notifications. The `using`-block suppressor exists and is the correct approach, but there is nothing in the type signatures to suggest it. + +Note: `PropertyChangeManager` suppresses `PropertyChanged` (WPF binding notifications). It is distinct from `RaisesModificationEvents`, which suppresses `OnNodeModified()` / scheduler re-execution. For batch port operations, both may be needed — see dp-002. + +## Correct form +```csharp +// Suppress PropertyChanged for "IsFrozen" during a batch update +using (node.PropertyChangeManager.SetPropsToSuppress("IsFrozen")) +{ + node.IsFrozen = true; + // ... other related changes +} +// PropertyChanged for "IsFrozen" fires again after the using block exits +``` + +## Anti-pattern +```csharp +// Wrong: fires PropertyChanged (and triggers WPF binding updates) for every assignment +node.IsFrozen = true; +node.SomeOtherProp = value; // each one re-evaluates bindings independently +``` + +```csharp +// Also wrong: manual unsubscribe/resubscribe is error-prone and misses other subscribers +node.PropertyChanged -= handler; +node.IsFrozen = true; +node.PropertyChanged += handler; +``` + +## When it applies +Any time multiple properties on a `NodeModel` (or any `NotificationObject`) are being set in a batch where intermediate `PropertyChanged` notifications would trigger redundant or incorrect UI updates. Common cases: deserialization, undo/redo replay, programmatic state resets. + +## Related patterns +- dp-002 diff --git a/agents/dynamo-ecosystem-reviewer.agent.md b/agents/dynamo-ecosystem-reviewer.agent.md new file mode 100644 index 0000000..22be242 --- /dev/null +++ b/agents/dynamo-ecosystem-reviewer.agent.md @@ -0,0 +1,25 @@ +--- +name: Dynamo Ecosystem Reviewer +description: Reviews code changes for compatibility with the broader Dynamo ecosystem. Applies known platform constraints — cross-platform portability, service compatibility, thread safety — across Dynamo's multiple repos. +--- + +# Dynamo Ecosystem Reviewer + +You review code for ecosystem-level impact using constraints that apply across the Dynamo platform. You are aware of multiple Dynamo repos (Dynamo, DynamoRevit, DynamoSandbox, etc.) and consider how changes ripple across them. + +## Core Constraints + +**Cross-platform / service compatibility** +- Code added to Dynamo core must run headless (no UI dependencies) +- Must be compatible with running in Dynamo as a service (no assumptions about a desktop environment) + +**Thread safety — Revit context** +- In Revit, the UI thread is the scheduler thread +- Code that blocks waiting for the UI thread will deadlock in this context +- Flag any dispatcher calls, `Invoke`, or `async` patterns that assume a separate UI thread + +## When reviewing + +- Check which repo/layer the change targets and apply constraints accordingly +- Look across related repos if the change touches shared APIs or contracts +- Be specific: name the constraint, explain why it applies, and point to the problematic code diff --git a/agents/webview-component-scaffold.agent.md b/agents/webview-component-scaffold.agent.md new file mode 100644 index 0000000..e917bdf --- /dev/null +++ b/agents/webview-component-scaffold.agent.md @@ -0,0 +1,82 @@ +--- +name: WebView Component Scaffold +description: Scaffolds a new Dynamo package repo that hosts a view extension with a WebView2-embedded React frontend. Use when setting up a new repo following the Switchboard pattern — Dynamo package structure, build pipeline, system tests, and bidirectional C#/TypeScript messaging. +--- + +# WebView Component Scaffold + +You scaffold new Dynamo extension repos following the Switchboard pattern. The reference implementation is DynamoDS/Dynamo-Switchboard. + +## Project Structure + +``` +src/ + {Product}.sln + Directory.Build.props # central: DynamoVersion, Product, deployment paths + common.tasks # ReplaceFileText MSBuild task + nuget.config # Autodesk Artifactory + nuget.org sources + {Product}.ViewExtension/ # net8.0-windows, IViewExtension + WebView2 host + {Product}.Messages/ # net8.0, shared C#/TS message types + {Product}.SystemTests/ # net8.0-windows, NUnit + SystemTestBase + {Product}.WebView/ # React + Vite + TypeScript frontend + packageItems/ + pkg.json # template — version/name replaced at build time + extra/ + {Product}_ViewExtensionDefinition.xml +scripts/ + build.ps1 # TypeScript codegen → npm build → dotnet build + create-package-zip.ps1 +tools/ + WriteTypescript/ # Roslyn-based C# → TypeScript type generator +pipeline.yml # Jenkins: build → test → publish artifact +``` + +## Package Output Structure + +``` +deploy/{Configuration}/{Product}/ + bin/ # assemblies + dist/ (React build output) + extra/ # ViewExtensionDefinition.xml + pkg.json # populated at build time from Directory.Build.props +``` + +## Key Patterns + +**ViewExtension** +- Implements `IViewExtension` with `UniqueId`, `Startup`, `Loaded`, `Shutdown` +- Loads dependent assemblies in a custom `AssemblyLoadContext` to avoid version conflicts +- Accesses `IOAuth2AuthProvider` and `PathManager` from Dynamo via `ViewLoadedParams` + +**WebView2 hosting** +- `Microsoft.Web.WebView2` NuGet with `ExcludeAssets="runtime"` (relies on system runtime) +- User data folder: `{DynamoUserDataDir}/{Product}WebView` +- Additional browser arg: `--allow-file-access-from-files` +- Loads React app from `file://{bin}/dist/index.html` + +**C# ↔ TypeScript messaging** +- `WebViewMessage` base class with `[JsonDerivedType]` for polymorphic JSON +- C# → TS: `CoreWebView2.PostWebMessageAsString(json)` +- TS → C#: `CoreWebView2.WebMessageReceived` event +- Types auto-generated from C# to TypeScript by WriteTypescript tool during build + +**System tests** +- Inherit `SystemTestBase` from DynamoVisualProgramming.Tests +- `TestServices.dll.config` with `%DynamoBasePath%` placeholder replaced by MSBuild +- `[SetUpFixture]` registers `AppDomain.AssemblyResolve` for Dynamo binaries +- Tests load the actual built package from `src/deploy/{Configuration}/{Product}/` +- NUnit STA apartment state required; JUnit XML output for CI reporting + +**Build** +- `Directory.Build.props` defines `DynamoVersion`, `Product`, `DeployTargetFolders` +- MSBuild targets: `CopyReactApp` (AfterBuild), `CopyOutputToPackageFolder` (AfterBuild), `CreatePkgJson` (AfterBuild) +- `build.ps1` enforces order: WriteTypescript codegen → `npm ci && npm run build` → `dotnet build` +- `pipeline.yml`: Jenkins, Windows node, weekly schedule, publishes `{Product}.zip` to Autodesk package manager + +## When scaffolding a new repo + +1. Replace all instances of `Switchboard`/`DynamoSwitchboard` with the new product name +2. Generate a new `UniqueId` GUID for the ViewExtension +3. Update `DynamoVersion` in `Directory.Build.props` to the target Dynamo version +4. Update NuGet package sources in `nuget.config` if the new repo uses different feeds +5. Stub out the Messages project with at minimum one message type in each direction +6. Verify the ViewExtensionDefinition.xml AssemblyPath matches the output DLL name From 4fdc651a67f153562a3f88e15ee176a869e09013 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 02:02:02 +0000 Subject: [PATCH 2/2] Fix filter wording consistency in codebase patterns agent docs Agent-Logs-Url: https://github.com/DynamoDS/.github-private/sessions/f28666c4-424e-436e-84bd-fa1555b2786f Co-authored-by: avidit <1406153+avidit@users.noreply.github.com> --- agents/dynamo-codebase-patterns.agent.md | 2 +- agents/dynamo-codebase-patterns/assets/README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/agents/dynamo-codebase-patterns.agent.md b/agents/dynamo-codebase-patterns.agent.md index 25169fe..8d90bae 100644 --- a/agents/dynamo-codebase-patterns.agent.md +++ b/agents/dynamo-codebase-patterns.agent.md @@ -31,7 +31,7 @@ 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 three-question filter to each candidate +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 diff --git a/agents/dynamo-codebase-patterns/assets/README.md b/agents/dynamo-codebase-patterns/assets/README.md index 3c4bf0f..5e13ec0 100644 --- a/agents/dynamo-codebase-patterns/assets/README.md +++ b/agents/dynamo-codebase-patterns/assets/README.md @@ -2,7 +2,7 @@ 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 three-question filter. +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