Report unresolvable template references from analyze_workflow, and close two activity-reference gaps - #220
Merged
Conversation
…ose two activity-reference gaps
analyze_workflow never ran the unresolved-reference check. It existed, but only
behind its own tool (find_unresolved_references), so the tool an agent actually
reaches for -- and the one the in-canvas assistant is built around -- answered
ok:true / findings:[] for a workflow that fails on its first run. Measured
against a 1.2.6 install: a log message reading {{gibtsnicht.output}} analysed
clean, while the orphan node in the same definition was reported correctly.
The findings now come back from Analyze itself, at the severity the engine
actually applies: an error where an unresolved placeholder aborts the step, a
warning on runScript and custom activities, which resolve their own templates and
tolerate a leftover {{...}} because it can be legitimate script text. References
on disabled nodes are skipped -- that node never runs.
Two entries in activity-config-reference.json described a shape the executor
rejects. wmiQuery's captureProperties is a JSON array and refuses a
comma-separated string; startProgram's filePath must be an absolute local path
and refuses a bare "cmd.exe". Both failed on first use in the lab. That file
feeds the AI prompt catalog and the MCP config tools, so an imprecise description
there becomes a generated workflow that looks right and fails.
CLAUDE.md documented the debug resume body without stepId, which
ResumeDebugRequest requires -- posting the documented body returns 400.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+135
to
+150
| foreach (var unresolved in VariableResolver.FindUnresolved(definition)) | ||
| { | ||
| if (doc.DisabledNodeIds.Contains(unresolved.NodeId)) continue; | ||
|
|
||
| var nodeType = typeByNodeId.GetValueOrDefault(unresolved.NodeId, string.Empty); | ||
| var tolerated = nodeType.Equals("runScript", StringComparison.OrdinalIgnoreCase) | ||
| || nodeType.StartsWith("custom:", StringComparison.OrdinalIgnoreCase); | ||
|
|
||
| var severity = tolerated ? "warning" : "error"; | ||
| var consequence = tolerated | ||
| ? "the step keeps running and the literal reaches the script" | ||
| : "the step fails with an unresolved-template error"; | ||
|
|
||
| findings.Add(new Finding(severity, unresolved.Code, unresolved.NodeId, | ||
| $"{unresolved.Reference}: {unresolved.Reason} At run time {consequence}.")); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Small batch of the remaining findings from the lab test run.
1.
analyze_workflowwas silent about unresolvable referencesThe check existed — but only behind its own tool,
find_unresolved_references.analyze_workflowis what an agent reaches for and what the in-canvas assistant is built around, so the whole class was invisible to a caller asking the obvious question.Measured against a 1.2.6 install, this definition:
{"nodes":[{"id":"trig",...,"manualTrigger"}, {"id":"n1",...,"log","config":{"message":"hallo {{gibtsnicht.output}}"}}, {"id":"waise",...,"log"}], "edges":[{"source":"trig","target":"n1"}]}returned
{"ok":true,...,"findings":[{"code":"unreachable-node","nodeId":"waise"}]}— the orphan was caught, the reference that fails the run on first execution was not.Findings now come from
Analyzeitself, at the severity the engine applies:errorrunScript,custom:*warningReferences on disabled nodes are skipped — that node never runs.
find_unresolved_referencesstays as-is for callers who want just that list.2. Two
activity-config-reference.jsonentries described a shape the executor rejectswmiQuery.capturePropertiesstartProgram.filePathcmd.exeis rejected, PATH is not searchedBoth failed on first use in the lab. That file feeds the AI prompt catalog and the MCP config tools, so an imprecise description there becomes a generated workflow that looks right and fails.
3.
CLAUDE.mddocumented the debug resume body withoutstepIdResumeDebugRequestrequires it; posting the documented{"mode":"continue","overrides":{}}returns400 The StepId field is required.Tests
WorkflowAnalyzerFrontendParityTests— three cases: an unresolvable reference is an error and flipsokto false; the same reference on arunScriptis a warning and leavesoktrue; a reference on a disabled node is not reported.Ran:
dotnet test tests/NodePilot.Mcp.Tests(181 passed), plus the guard tests for the touched surfaces —ActivityConfigReferenceTests(5) andPromptCatalogDriftTest(5).