Skip to content

feat(definitions): reject publish when allowedAssemblies don't resolve - #1069

Merged
yilmaztayfun merged 5 commits into
masterfrom
36-a4-script-derlemesi-publishte-dogrulansin
Oct 2, 2026
Merged

yilmaztayfun merged 5 commits into
masterfrom
36-a4-script-derlemesi-publishte-dogrulansin

Conversation

@yilmaztayfun

Copy link
Copy Markdown
Contributor

Summary

  • A component that declares scripts.allowedAssemblies (at flow level or on any script slot) is now checked at publish time: every listed simple assembly name must resolve in the publishing runtime, either as a framework (TPA) assembly or as a DLL in Scripting:Sandbox:PluginDirectory.
  • Before this change an unresolvable name was silently dropped by SandboxedReferenceSet.Build, and the script only failed mid-transition with CS0012/CS1069 (e.g. a child reaching its final state but ending FAULTED). Now the publish returns 400, and the error names the exact field, e.g. sys-flows.states[0].onEntries[1].mapping.scripts.allowedAssemblies[0].
  • Scope is deliberately narrow (burgan-tech/vnext-client-sdk-core#36): no script is compiled or read, helpers/REF references are not resolved, and a script that needs an assembly it never declares still fails only at run time.

Changes

  • modules/.../Sandbox/SandboxedReferenceSet.cs: new IsResolvable(options, name) over the same cached TPA and plugin maps Build uses, so publish and compile share one resolution rule.
  • modules/.../Sandbox/IScriptAssemblyCatalog.cs, SandboxScriptAssemblyCatalog.cs: the abstraction validators depend on.
  • src/.../Definitions/Validators/AllowedAssembliesPublishCheck.cs: a component-agnostic walk over the published attributes JSON. It only looks at scripts objects whose allowedAssemblies is a string array, and only for sys-flows, sys-tasks, sys-functions and sys-extensions. It reports one error per unresolvable name.
  • ComponentValidatorProcessor.Validate runs the check after the type-specific validator; TryValidate (seed data) is untouched. The catalog is registered in AddComponentValidators and falls back to default sandbox options on read-only hosts.
  • docs/custom-script-helpers.md, vnext-meta/migrations.json (publish-rejects-unavailable-allowed-assemblies, since 0.0.98).

Test Plan

  • SandboxScriptAssemblyCatalogTests 7/7: framework name, case-insensitivity, .dll suffix, empty/whitespace, plugin-directory DLL.
  • AllowedAssembliesPublishCheckTests 13/13: flow-level, nested slot path, one error per name, look-alike non-array ignored, scanned component types.
  • ComponentValidatorProcessorTests 11/11: the check fails a component the type validator passed; an out-of-scope type and seed data are not scanned.
  • dotnet build vnext.sln: 0 errors.
  • BBT.Workflow.Application.Tests: the 17 failing tests also fail on base 3fc16d3c. SubflowDescentTracingTests (2) fails only in some full parallel runs and passes 12/12 when run alone, on both this branch and base.
  • BBT.Workflow.Domain.Tests: 22 failing tests, the same 22 on base 3fc16d3c.
  • Not done: a live publish against a locally built runtime. The 400 path (DefinitionController.PublishAsync → FromResult → WorkflowResultActionResultMapper, with ValidationErrors[].Members) was verified by reading the code only.

Notes

  • ⚠️ Behaviour change on upgrade: with Scripting:Sandbox:Enabled=false the compiler ignores the per-script grant, so a stale or misspelled name used to be harmless. The publish check runs regardless of that flag, so such a package now gets 400 on its next publish. Fix: remove the name, use the simple name without .dll, or have the assembly mounted. This is recorded in vnext-meta/migrations.json.
  • The check reflects the host that receives the publish (Orchestration); Execution's plugin set is not consulted. The plugin-directory listing is read once per process, so a DLL mounted after start reads as unavailable until restart (the same as compile).
  • Only one component in sibling repos declares allowedAssemblies (vnext-example account-opening: System.Security.Cryptography, a framework assembly), and it passes.
  • Pre-existing and unrelated: vnext-meta/deprecations.json is invalid JSON on master (line 168).

🤖 Generated with Claude Code

yilmaztayfun and others added 5 commits October 2, 2026 13:54
…yCatalog

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…components

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…vailable assembly

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…heck

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@yilmaztayfun
yilmaztayfun requested review from a team October 2, 2026 14:41
@coldtea-pr-lens

coldtea-pr-lens Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

◈ PR Lens

🟢 +1 new · 🟠 ~2 changed · 🔴 -0 removed · 1 flow · 11 files · commit 989d5f2


Architecture

Architecture diagram for burgan-tech/vnext at 989d5f2

3 components touched across 4 lanes.

Play the interactive walkthrough


Inside the changed components — 1 view

Component view — Publish validation

Publish-time verification of allowed script assemblies in component definitions.

Architecture view of Component view — Publish validation in burgan-tech/vnext

Data flow

Data flow diagram for burgan-tech/vnext at 989d5f2

Validating script assemblies at publish

Follow each request, response and payload


View

  • Architecture lens
  • Data flow lens
  • Expand every detail

Tip

Open a diagram on the canvas, then press W or click play to walk through the change one step at a time

🪧 More tips
  • Run npx skills add coldteadotai/pr-lens, then tell your coding agent: "Diagram the change you just made with PR Lens and attach it to the pull request."
  • Run npx @coldtea/pr-lens-cli analyze --base origin/main on a branch, then npx @coldtea/pr-lens-cli render .pr-lens/graph.json. Same lenses, your own model key, before the pull request exists
  • Untick Architecture lens or Data flow lens under View to hide a diagram, or tick Expand every detail to open every section. The comment redraws in a few seconds
  • Click the link under each diagram to open it on a canvas you can zoom, pan and step through
  • The diagrams are links. Click one to open it on the canvas, then press W or click play to walk through the change
  • The CLI's render reads .github/pr-lens.yml and applies your renames, exclusions and lane pins at draw time
  • Set github.comment.collapsed: true in .github/pr-lens.yml to fold the comment behind one View architecture and data flow row. Drawing still runs as before
  • Set github.draw: on-demand in .github/pr-lens.yml and PR Lens stops drawing pull requests on its own. Comment @pr-lens draw on a pull request when you want that one drawn
  • Add .github/workflows/pr-lens.yml with coldteadotai/pr-lens/packages/action@v0 and your model provider's key as its api-key to run PR Lens from your own CI. Any /chat/completions endpoint works
  • Push a commit and the drawing stays, with a note that it is out of date. Tick Redraw in the note to draw the new head
  • Switch GitHub to dark mode and the diagrams follow. The moving dots are this pull request's data in motion

Thanks for using PR Lens! It's built by Coldtea, free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5ebabcd7-a851-46b8-8eed-184a4f5ad79f

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@deepsource-io

deepsource-io Bot commented Oct 2, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in a58f1f1...989d5f2 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
C# Oct 2, 2026 2:41p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@yilmaztayfun yilmaztayfun self-assigned this Oct 2, 2026
@yilmaztayfun yilmaztayfun added this to the v0.0.99 milestone Oct 2, 2026
@yilmaztayfun
yilmaztayfun merged commit 0556eda into master Oct 2, 2026
5 of 6 checks passed
@sonarqubecloud

sonarqubecloud Bot commented Oct 2, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant