From 16297c11f277feb2c9fb7271f5eeb4ba2bb2bfcd Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:41:49 +0100 Subject: [PATCH] Fix regressions from the unreleased designer and test-suite changes - A workflow saved earlier with two active nodes sharing an output variable loads, triggers and runs again. The last node keeps the alias, as before; saving and importing still reject new duplicates. Annotation nodes no longer count as alias owners (#789). - Renaming an alias also rewrites templates inside literal condition operands, since the engine resolves them (#789). - The portOpen lint accepts a templated or quoted port, as the engine does (#790). - scheduledTask registers a managed service account with -LogonType Password. With ServiceAccount the task was registered but never ran; checked on a lab host (#790). - A first test-suite install creates the webhook workflow before it judges prerequisites, so both webhook workflows are enabled, and the release check verifies the integration profile it installs (#776). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 17 +++++++---- RELEASING.md | 3 +- scripts/test-suite/Install-TestSuite.ps1 | 30 ++++++++++++------- scripts/test-suite/Test-TestSuiteSafety.ps1 | 30 +++++++++++++++++++ .../WorkflowDefinitionDocument.cs | 9 +++--- .../WorkflowDefinitionStructuralValidator.cs | 2 ++ .../Activities/ScheduledTaskActivity.cs | 15 +++++----- .../__tests__/lib/activityConfigFacts.test.ts | 11 +++++++ .../__tests__/lib/editorGraphHelpers.test.ts | 4 +-- .../__tests__/lib/workflowFragment.test.ts | 2 +- .../src/lib/activityConfigFacts.ts | 9 +++++- .../src/lib/workflowReferences.ts | 9 ++++-- .../Activities/ScheduledTaskActivityTests.cs | 19 ++++++++++++ .../Execution/WorkflowDefinitionCacheTests.cs | 26 ++++++++++++++++ .../WorkflowDefinitionDocumentTests.cs | 23 ++++++++++++++ 15 files changed, 175 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b45bf57..2ce344f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,10 @@ Planned version: **1.5.4**. characters fails the run and names the edge, instead of counting as "no match" (#680). - Two active nodes can no longer share an output variable, regardless of case: saving, importing or an MCP change with such a pair is rejected. A disabled node is ignored, so a disabled copy can - no longer take over the alias of the active one (#703). + no longer take over the alias of the active one. A workflow saved earlier with such a pair keeps + running and triggering as before, with the last node holding the alias; the designer flags the + pair, and it must be resolved before the next save. An older export with such a pair is + rejected on import and names the alias (#703, #789). - `GET /api/executions/{id}/paused-steps`, `np exec paused-steps` and the MCP tool `list_paused_steps` return one snapshot per paused step (step id, label, pause time, reason and redacted variables) instead of a list of step ids. The MCP result field is now `pausedSteps` @@ -36,7 +39,8 @@ Planned version: **1.5.4**. - **Designer** - Renaming an output variable updates every reference to it: templates in any field, a dynamic - target machine or credential, breakpoint conditions, decision cases and edge conditions (#689). + target machine or credential, breakpoint conditions, decision cases and edge conditions, + including templates in their fixed comparison values (#689, #789). - Copying, pasting and duplicating give each copy its own output variable (`name_2`) and rewrite the references inside the copy, so it no longer takes over the original's bindings (#703). - Inserting an activity on a conditional edge keeps the condition in front of the new activity, @@ -50,9 +54,11 @@ Planned version: **1.5.4**. expression, an agent's host list shows the restored values (#695). - Text-file delete can switch to line-range and pattern mode again (#601). - `waitForCondition` in `portOpen` mode saves the default port 443 it shows, and a probe without - host or valid port is flagged (#702). + host or valid port is flagged. A port given as a template or as a number in quotes is accepted, + as the engine accepts it (#702, #790). - `scheduledTask` registration accepts only built-in or managed service accounts and no longer - suggests ordinary domain users with delegation (#705). + suggests ordinary domain users with delegation. A task registered for a managed service account + (`DOMAIN\gmsa$`) now runs under it; before, it was registered but never started (#705, #790). - The step details of a past run label the workflow's current configuration as such and show the target machine the run actually used (#603). - After a reconnect or a reload, the designer shows a paused debug step with its variables again @@ -116,7 +122,8 @@ Planned version: **1.5.4**. - The `powerManagement` test moves to the invasive set and needs a disposable host, because `abort` cancels a pending host restart (#720). Acknowledgement files accept only GUID correlation IDs (#721). Verification checks every run in the window and fails when required - workflows are missing (#718, #719). + workflows are missing (#718, #719). A first install already enables the two webhook + workflows, and the release check verifies the integration workflows it installs (#776). - Nightly retention deletes only the run folders it created (#716). The failover drill restarts the original leader also after a failed takeover (#715). - The SSO lab drills check the requested run and use the configured account prefix throughout diff --git a/RELEASING.md b/RELEASING.md index dbbbac86..0b6975d9 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -106,7 +106,8 @@ Afterwards the same server setup is installed as an update on the lab's long-run ```powershell .\scripts\test-suite\Install-TestSuite.ps1 -BaseUrl https://:8443 -Password ` -Profiles continuous, integration -ProbeUrl http:/// -.\scripts\test-suite\Verify-TestSuite.ps1 -BaseUrl https://:8443 -Password -Once +.\scripts\test-suite\Verify-TestSuite.ps1 -BaseUrl https://:8443 -Password ` + -Profiles continuous, integration -Once ``` The probe URL points at another host, since an instance cannot probe itself by its own name. **The suite has to finish with `fail=0`.** Right after the update the database trigger skips its first round by design. Should the trigger driver fail on that alone, the verifier is run once more. diff --git a/scripts/test-suite/Install-TestSuite.ps1 b/scripts/test-suite/Install-TestSuite.ps1 index bb6940ad..22498f9b 100644 --- a/scripts/test-suite/Install-TestSuite.ps1 +++ b/scripts/test-suite/Install-TestSuite.ps1 @@ -314,6 +314,26 @@ if (Test-Path -LiteralPath $customPath) { } } +# --- webhook endpoint -------------------------------------------------------------- +# Addressed by workflow id rather than name: the route accepts either, and a name like +# "[TestSuite] trigger: webhook" would have to survive URL encoding on every hop. Both webhook +# workflows require this global, so a first install creates the webhook workflow as an empty, +# unlocked draft before the loop judges prerequisites; the loop then publishes it. +$hookWorkflow = $existing | Where-Object { $_.name -eq '[TestSuite] trigger: webhook' } | Select-Object -First 1 +if ($null -eq $hookWorkflow) { + $hookWorkflow = Invoke-NodePilotJson -Method POST -Path '/api/workflows' -Body @{ + name = '[TestSuite] trigger: webhook' + description = '' + definitionJson = '{"nodes":[],"edges":[]}' + folderId = $folderId + } + $null = Invoke-NodePilotJson -Method POST -Path "/api/workflows/$($hookWorkflow.id)/unlock" + $existing += $hookWorkflow +} +Set-Global -Name 'NP_TESTSUITE_WEBHOOK_URL' ` + -Value "$BaseUrl/api/webhooks/$($hookWorkflow.id)/suite" ` + -Description 'TestSuite: endpoint the trigger driver posts to.' + # --- install ------------------------------------------------------------------------- # Manifest order is generation order, and the child workflow is number 0, so parents are # never published before the child they resolve by name. @@ -406,16 +426,6 @@ foreach ($oldName in @('[TestSuite] powerManagement')) { } } -# --- webhook endpoint -------------------------------------------------------------- -# Addressed by workflow id rather than name: the route accepts either, and a name like -# "[TestSuite] trigger: webhook" would have to survive URL encoding on every hop. -$hookWorkflow = $existing | Where-Object { $_.name -eq '[TestSuite] trigger: webhook' } | Select-Object -First 1 -if ($null -ne $hookWorkflow) { - Set-Global -Name 'NP_TESTSUITE_WEBHOOK_URL' ` - -Value "$BaseUrl/api/webhooks/$($hookWorkflow.id)/suite" ` - -Description 'TestSuite: endpoint the trigger driver posts to.' -} - # --- legacy removal (never automatic) ------------------------------------------------ if ($RemoveLegacy) { $legacyPatterns = @('^Test — ', '^\[Dauertest 1m\] ', '^Dauertest — ', '^Muster — ', '^Muster Test: ') diff --git a/scripts/test-suite/Test-TestSuiteSafety.ps1 b/scripts/test-suite/Test-TestSuiteSafety.ps1 index bc19e738..85281f2b 100644 --- a/scripts/test-suite/Test-TestSuiteSafety.ps1 +++ b/scripts/test-suite/Test-TestSuiteSafety.ps1 @@ -166,4 +166,34 @@ foreach ($existing in @($null, } Assert-Safety (@($secrets | Sort-Object -Unique).Count -eq 3) 'A reinstall reused a webhook secret instead of rotating it.' +# --- a first install already has the webhook URL when it judges prerequisites ---------- +. ([scriptblock]::Create((Import-InstallerFunction 'Test-GlobalPresent'))) +$hookStart = $installerAst.Find({ param($node) $node -is [Management.Automation.Language.AssignmentStatementAst] -and $node.Left.Extent.Text -eq '$hookWorkflow' }, $true) +$hookEnd = $installerAst.Find({ param($node) $node -is [Management.Automation.Language.CommandAst] -and $node.Extent.Text -like "Set-Global -Name 'NP_TESTSUITE_WEBHOOK_URL'*" }, $true) +$hookBlock = $installerAst.Extent.Text.Substring($hookStart.Extent.StartOffset, $hookEnd.Extent.EndOffset - $hookStart.Extent.StartOffset) +$hookConsumers = @(@($manifest.workflows) | Where-Object { @($_.requires) -contains 'globals:NP_TESTSUITE_WEBHOOK_URL' }) +Assert-Safety ($hookConsumers.Count -ge 1) 'No suite workflow requires NP_TESTSUITE_WEBHOOK_URL any more.' +$BaseUrl = 'https://suite.test' +$folderId = 'folder' +function Invoke-NodePilotJson { + param($Method, $Path, $Body) + $script:calls += [pscustomobject]@{ Method = $Method; Path = $Path; Body = $Body } + if ($null -eq $Body) { return $null } + return [pscustomobject]@{ id = 'created'; name = $Body.name; value = $Body['value'] } +} +foreach ($installed in @($null, [pscustomobject]@{ id = 'hook'; name = '[TestSuite] trigger: webhook' })) { + $existing = @($installed | Where-Object { $null -ne $_ }) + $globals = @([pscustomobject]@{ id = 'secret'; name = 'NP_TESTSUITE_WEBHOOK_SECRET'; value = '***'; isSecret = $true }) + $script:calls = @() + . ([scriptblock]::Create($hookBlock)) + $hookId = if ($null -eq $installed) { 'created' } else { 'hook' } + $created = @($script:calls | Where-Object { $_.Method -eq 'POST' -and $_.Path -eq '/api/workflows' }) + Assert-Safety ($created.Count -eq [int]($null -eq $installed)) "The webhook workflow was created $($created.Count) time(s) with installed='$($null -ne $installed)'." + $urlWrite = @($script:calls | Where-Object { $_.Path -like '/api/global-variables*' -and $_.Body.name -eq 'NP_TESTSUITE_WEBHOOK_URL' }) + Assert-Safety ($urlWrite.Count -eq 1 -and $urlWrite[0].Body.value -eq "$BaseUrl/api/webhooks/$hookId/suite") 'The webhook URL global does not address the webhook workflow by id.' + foreach ($consumer in $hookConsumers) { + Assert-Safety (Test-Prerequisites -Requires $consumer.requires) "'$($consumer.name)' would be installed disabled on this install." + } +} + Write-Host "TestSuite safety: $script:passed assertions passed." -ForegroundColor Green diff --git a/src/NodePilot.Core/WorkflowDefinitions/WorkflowDefinitionDocument.cs b/src/NodePilot.Core/WorkflowDefinitions/WorkflowDefinitionDocument.cs index 7d0fbb4f..f9520f1d 100644 --- a/src/NodePilot.Core/WorkflowDefinitions/WorkflowDefinitionDocument.cs +++ b/src/NodePilot.Core/WorkflowDefinitions/WorkflowDefinitionDocument.cs @@ -192,12 +192,11 @@ public static Dictionary BuildOutputVariableAliasMap(IReadOnlyLi var dict = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var node in allNodes) { - if (node.Data.Disabled) continue; + if (node.Data.Disabled || WorkflowDefinitionStructuralValidator.IsAnnotationNodeType(node.Type)) continue; + // Save and import reject duplicate aliases. A definition stored before that check still + // loads, and the last node keeps the alias, as it did when it was saved. if (node.Data.OutputVariable is { Length: > 0 } variable) - { - if (!dict.TryAdd(variable, node.Id)) - throw new InvalidOperationException($"Duplicate active output variable '{variable}'."); - } + dict[variable] = node.Id; } return dict; diff --git a/src/NodePilot.Core/WorkflowDefinitions/WorkflowDefinitionStructuralValidator.cs b/src/NodePilot.Core/WorkflowDefinitions/WorkflowDefinitionStructuralValidator.cs index 3e05d124..feb1a230 100644 --- a/src/NodePilot.Core/WorkflowDefinitions/WorkflowDefinitionStructuralValidator.cs +++ b/src/NodePilot.Core/WorkflowDefinitions/WorkflowDefinitionStructuralValidator.cs @@ -12,6 +12,8 @@ public static class WorkflowDefinitionStructuralValidator private static readonly HashSet _annotationNodeTypes = new(StringComparer.OrdinalIgnoreCase) { "stickyNote", "group" }; + internal static bool IsAnnotationNodeType(string type) => _annotationNodeTypes.Contains(type); + public static WorkflowDefinitionValidationResult Validate(JsonElement definition) { if (definition.ValueKind != JsonValueKind.Object) diff --git a/src/NodePilot.Engine/Activities/ScheduledTaskActivity.cs b/src/NodePilot.Engine/Activities/ScheduledTaskActivity.cs index 97773654..db0aeb3c 100644 --- a/src/NodePilot.Engine/Activities/ScheduledTaskActivity.cs +++ b/src/NodePilot.Engine/Activities/ScheduledTaskActivity.cs @@ -43,9 +43,9 @@ namespace NodePilot.Engine.Activities; /// Notes: /// - Most actions require admin rights on the target. The activity does not enforce this; /// the cmdlet will return the appropriate error. -/// - Register's principal/runAsUser path uses NT AUTHORITY built-ins ("SYSTEM", -/// "NETWORK SERVICE", "LOCAL SERVICE") implicitly. Domain users would also need a -/// password, which this activity does not support; the UI should note the limitation. +/// - Register's runAsUser is an NT AUTHORITY built-in ("SYSTEM", "NETWORK SERVICE", +/// "LOCAL SERVICE") or a managed service account ("DOMAIN\gmsa$"). Ordinary domain users +/// would need a password, which this activity does not support. /// public class ScheduledTaskActivity : BaseRemoteActivity { @@ -231,14 +231,15 @@ private static string BuildRegisterScript(JsonElement config, string qName, stri // Trigger object — switch by triggerType. startTime is validated to exist where required. sb.Append("$__npTrigger = ").Append(BuildTriggerExpression(config, triggerType!)).Append("; "); - // Principal (user + run level). NT AUTHORITY built-ins accept -LogonType ServiceAccount - // without a password; other account names need one and are not checked here — the - // cmdlet reports the failure directly. + // Principal (user + run level). NT AUTHORITY built-ins use -LogonType ServiceAccount. + // A managed service account (name ends in '$') uses -LogonType Password without a + // password; Task Scheduler retrieves it from AD. Other names fail in the cmdlet. var qUser = PowerShellOperation.Literal(runAsUser); var psRunLevel = runLevel == "highest" ? "Highest" : "Limited"; + var logonType = runAsUser.Trim().EndsWith('$') ? "Password" : "ServiceAccount"; sb.Append("$__npPrincipal = New-ScheduledTaskPrincipal -UserId ").Append(qUser) .Append(" -RunLevel ").Append(psRunLevel) - .Append(" -LogonType ServiceAccount; "); + .Append(" -LogonType ").Append(logonType).Append("; "); sb.Append("$__npSettings = New-ScheduledTaskSettingsSet; "); diff --git a/src/nodepilot-ui/src/__tests__/lib/activityConfigFacts.test.ts b/src/nodepilot-ui/src/__tests__/lib/activityConfigFacts.test.ts index 49e77048..ac6a4673 100644 --- a/src/nodepilot-ui/src/__tests__/lib/activityConfigFacts.test.ts +++ b/src/nodepilot-ui/src/__tests__/lib/activityConfigFacts.test.ts @@ -223,6 +223,17 @@ describe('summarizeActivityConfig', () => { }); }); + describe('waitForCondition', () => { + it.each([443, '8443', ' 5000 ', '{{disc.param.selfPort}}'])('portOpen_port_%s_isAccepted', (port) => { + expect(checkRequiredActivityConfig('waitForCondition', { conditionType: 'portOpen', host: 'example.test', port })).toBeNull(); + }); + + it.each([0, 65536, '70000', 'https', '', 1.5])('portOpen_port_%s_isRejected', (port) => { + expect(checkRequiredActivityConfig('waitForCondition', { conditionType: 'portOpen', host: 'example.test', port })) + .toBe(i18n.t('activities:validation.portRequired')); + }); + }); + describe('scheduledTask', () => { it.each(['SYSTEM', 'NT AUTHORITY\\NETWORK SERVICE', 'local service', 'CONTOSO\\svc-tasks$', '{{globals.TaskAccount}}'])( 'registerAs_%s_isSupported', (runAsUser) => { diff --git a/src/nodepilot-ui/src/__tests__/lib/editorGraphHelpers.test.ts b/src/nodepilot-ui/src/__tests__/lib/editorGraphHelpers.test.ts index c601ec76..20275d06 100644 --- a/src/nodepilot-ui/src/__tests__/lib/editorGraphHelpers.test.ts +++ b/src/nodepilot-ui/src/__tests__/lib/editorGraphHelpers.test.ts @@ -140,7 +140,7 @@ describe('renameVariableInEdge', () => { expect((result.data as { condition: string }).condition).toBe('new.failed'); }); - it('rewrites actual builder step operands without changing literal/global/manual operands', () => { + it('rewrites step operands and templates in literals without changing global/manual operands', () => { const edge = { id: 'e1', source: 'a', target: 'b', data: { @@ -159,7 +159,7 @@ describe('renameVariableInEdge', () => { children: Array<{ left: Record; right: Record }>; }; expect(expr.children[0].left.stepId).toBe('new'); - expect(expr.children[0].right.value).toBe('{{old.output}}'); + expect(expr.children[0].right.value).toBe('{{new.output}}'); expect(expr.children[1].left.name).toBe('old'); expect(expr.children[1].right.name).toBe('old'); expect(result).not.toBe(edge); diff --git a/src/nodepilot-ui/src/__tests__/lib/workflowFragment.test.ts b/src/nodepilot-ui/src/__tests__/lib/workflowFragment.test.ts index 0329d4ec..2081dfd5 100644 --- a/src/nodepilot-ui/src/__tests__/lib/workflowFragment.test.ts +++ b/src/nodepilot-ui/src/__tests__/lib/workflowFragment.test.ts @@ -316,7 +316,7 @@ describe('remapFragment', () => { expect(pasted.nodes[1].data.targetMachineId).toBe('{{result_3.output}}'); expect(pasted.nodes[1].data.credentialId).toBe(`{{${pasted.nodes[0].id}.param.credential}}`); expect(pasted.nodes[1].data.config).toMatchObject({ cases: [{ condition: { - left: { stepId: 'result_3' }, right: { value: '{{result.output}}' }, + left: { stepId: 'result_3' }, right: { value: '{{result_3.output}}' }, } }] }); expect(pasted.edges[0].data?.condition).toBe('result_3.success'); expect(fragment.nodes[0].data.outputVariable).toBe('result'); diff --git a/src/nodepilot-ui/src/lib/activityConfigFacts.ts b/src/nodepilot-ui/src/lib/activityConfigFacts.ts index 8708c306..038f85c7 100644 --- a/src/nodepilot-ui/src/lib/activityConfigFacts.ts +++ b/src/nodepilot-ui/src/lib/activityConfigFacts.ts @@ -31,6 +31,13 @@ const hasValue = (value: unknown): boolean => const hasValueOrTemplate = (value: unknown): boolean => isTemplate(value) || hasValue(value); +// The engine also accepts a numeric string, which is what a resolved template becomes. +const isPortOrTemplate = (value: unknown): boolean => { + if (isTemplate(value)) return true; + const port = typeof value === 'string' && /^\s*\d+\s*$/.test(value) ? Number(value) : value; + return typeof port === 'number' && Number.isInteger(port) && port >= 1 && port <= 65535; +}; + const inheritExecutionContext = ({ lastData }: SmartDefaultContext): SmartDefaults => ({ targetMachineId: (lastData.targetMachineId as string | null) ?? null, credentialId: (lastData.credentialId as string | null) ?? null, @@ -214,7 +221,7 @@ const ACTIVITY_CONFIG_FACTS: Record = { : mode === 'httpOk' ? ['url', 'urlRequired'] : mode === 'portOpen' ? ['host', 'hostRequired'] : ['script', 'scriptRequired']; if (!hasValueOrTemplate(config[required[0]])) return i18n.t(`activities:validation.${required[1]}`); - if (mode === 'portOpen' && !(typeof config.port === 'number' && Number.isInteger(config.port) && config.port >= 1 && config.port <= 65535)) { + if (mode === 'portOpen' && !isPortOrTemplate(config.port)) { return i18n.t('activities:validation.portRequired'); } return null; diff --git a/src/nodepilot-ui/src/lib/workflowReferences.ts b/src/nodepilot-ui/src/lib/workflowReferences.ts index 65e56699..038f34c4 100644 --- a/src/nodepilot-ui/src/lib/workflowReferences.ts +++ b/src/nodepilot-ui/src/lib/workflowReferences.ts @@ -1,5 +1,6 @@ /** Reference rewriting shared by authoring operations. Values outside reference-bearing - * fields, including literal condition operands, retain their meaning and identity. */ + * fields retain their meaning and identity. Templates inside literal condition operands are + * rewritten, because the engine resolves them. */ export function rewriteTemplates(value: unknown, names: ReadonlyMap): unknown { if (isRecord(value) && (value.type === 'comparison' || value.type === 'group' || value.type === 'not')) { return rewriteCondition(value, names); @@ -19,7 +20,11 @@ export function rewriteTemplates(value: unknown, names: ReadonlyMap): unknown { if (!isRecord(value)) return value; - if (value.kind === 'literal') return value; + if (value.kind === 'literal') { + if (typeof value.value !== 'string') return value; + const rewritten = rewriteTemplates(value.value, names); + return rewritten === value.value ? value : { ...value, value: rewritten }; + } if (value.kind === 'variable') { if (value.source !== undefined && value.source !== 'step') return value; const mapped = typeof value.stepId === 'string' ? names.get(value.stepId) : undefined; diff --git a/tests/NodePilot.Engine.Tests/Activities/ScheduledTaskActivityTests.cs b/tests/NodePilot.Engine.Tests/Activities/ScheduledTaskActivityTests.cs index d0a4130a..9be83564 100644 --- a/tests/NodePilot.Engine.Tests/Activities/ScheduledTaskActivityTests.cs +++ b/tests/NodePilot.Engine.Tests/Activities/ScheduledTaskActivityTests.cs @@ -254,6 +254,25 @@ await act.ExecuteAsync(Ctx(), Cfg(@"{ _capturedScript.Should().NotContain("Schedule.Service"); } + [Theory] + [InlineData("SYSTEM", "ServiceAccount")] + [InlineData("NT AUTHORITY\\NETWORK SERVICE", "ServiceAccount")] + [InlineData("CONTOSO\\svc-tasks$", "Password")] + public async Task Register_PrincipalLogonType_MatchesAccountKind(string runAsUser, string logonType) + { + var act = CreateActivity(); + var config = JsonSerializer.Serialize(new + { + action = "register", + taskName = "Nightly", + program = "C:\\Tools\\backup.exe", + triggerType = "atStartup", + runAsUser, + }); + await act.ExecuteAsync(Ctx(), Cfg(config), CancellationToken.None); + _capturedScript.Should().Contain($"-RunLevel Limited -LogonType {logonType}; "); + } + [Fact] public async Task RegisterWeekly_RequiresDaysOfWeek() { diff --git a/tests/NodePilot.Engine.Tests/Execution/WorkflowDefinitionCacheTests.cs b/tests/NodePilot.Engine.Tests/Execution/WorkflowDefinitionCacheTests.cs index 91022faf..20404d04 100644 --- a/tests/NodePilot.Engine.Tests/Execution/WorkflowDefinitionCacheTests.cs +++ b/tests/NodePilot.Engine.Tests/Execution/WorkflowDefinitionCacheTests.cs @@ -37,6 +37,32 @@ public void GetOrCompile_ReusesCompiledGraphForSameWorkflowVersion() second.RootNodes.Should().ContainSingle(n => n.Id == "manual"); } + [Fact] + public void GetOrCompile_StoredDuplicateActiveAlias_CompilesForTheRun() + { + var workflow = new Workflow + { + Id = Guid.NewGuid(), + Version = 1, + UpdatedAt = DateTime.UtcNow, + DefinitionJson = """ + {"nodes":[ + {"id":"manual","type":"activity","data":{"activityType":"manualTrigger","config":{}}}, + {"id":"first","type":"activity","data":{"activityType":"log","outputVariable":"shared","config":{}}}, + {"id":"second","type":"activity","data":{"activityType":"log","outputVariable":"shared","config":{}}} + ],"edges":[ + {"id":"e1","source":"manual","target":"first"}, + {"id":"e2","source":"manual","target":"second"} + ]} + """, + }; + + var compiled = WorkflowDefinitionCache.GetOrCompile(workflow); + + compiled.RootNodes.Should().ContainSingle(n => n.Id == "manual"); + compiled.OutputVariableToStepId["shared"].Should().Be("second"); + } + [Fact] public void GetOrCompile_InvalidatesWhenWorkflowVersionChanges() { diff --git a/tests/NodePilot.Engine.Tests/WorkflowDefinitions/WorkflowDefinitionDocumentTests.cs b/tests/NodePilot.Engine.Tests/WorkflowDefinitions/WorkflowDefinitionDocumentTests.cs index 5d4e3db8..e8ba7ccd 100644 --- a/tests/NodePilot.Engine.Tests/WorkflowDefinitions/WorkflowDefinitionDocumentTests.cs +++ b/tests/NodePilot.Engine.Tests/WorkflowDefinitions/WorkflowDefinitionDocumentTests.cs @@ -33,6 +33,29 @@ public void Parse_CentralizesDisabledRuntimeSemanticsAndMetadata() definition.Metadata.TriggerTypes.Should().Equal("manualTrigger"); } + [Fact] + public void TryParse_StoredDuplicateActiveAlias_LoadsWithTriggersAndLastNodeKeepsAlias() + { + var parsed = WorkflowDefinitionDocument.TryParse(""" + { + "nodes": [ + { "id": "schedule", "type": "activity", "data": { "activityType": "scheduleTrigger", "config": { "cronExpression": "0 0/5 * * * ?" } } }, + { "id": "first", "type": "activity", "data": { "activityType": "log", "outputVariable": "Value", "config": {} } }, + { "id": "second", "type": "activity", "data": { "activityType": "log", "outputVariable": "value", "config": {} } }, + { "id": "note", "type": "stickyNote", "data": { "outputVariable": "value" } } + ], + "edges": [ + { "id": "e1", "source": "schedule", "target": "first", "data": {} }, + { "id": "e2", "source": "schedule", "target": "second", "data": {} } + ] + } + """, out var definition); + + parsed.Should().BeTrue(); + definition!.TriggerDescriptors.Should().ContainSingle(d => d.ActivityType == "scheduleTrigger"); + definition.OutputVariableToStepId.Should().ContainKey("VALUE").WhoseValue.Should().Be("second"); + } + [Fact] public void Parse_TriggerlessGraph_HasNoRootNodes() {