Prevent changes that would leak into the Live workspace

We could handle things in a similar way to config read only (https://www.drupal.org/project/config_readonly) by disabling the submit.

To get around the search form issue we could do this only on entity forms? and show as a drupal_set_message() rather than an inline error.

From a UX POV, I think it would be useful for it to just "403" access to the offending link/form without giving the ability for users to even see it. Greying out a submit button 3 page scrolls down when you've already spent 15+ minutes entering content into form fields is pretty frustrating... and little errors on top of the page are easy to miss/bypass especially in an admin context, where your site may be "perma-warning" you of not-that-bad things like a missing ImageMagick, or what have you.

So for example, the button label on the term overview page could change from "+ Add term" to "+ Add term (Live only)" and it could be grey instead of blue and un-clickable.

Won't a 403 beg the question, "Why can't I access this?", though I guess they won't know they can't access it as they will never see the link, thus never seeing the 403.

We discussed this on a call, and there was consensus (albeit somewhat teeth-gnashingly :D) on solving this at an API level being a beta blocker, vs. solving it at a UI level being a separate concern. So I think we need a postponed, "major" follow-up issue for the UI, and focus this one on preventing users from ending up in data loss land.

a not-so-friendly exception in hook_entity_presave()

Do we also need to do the same for predelete()? Are there other considerations for how deletions are done while in a non-live workspace?

The configuration system doesn't have any pre-save event in \Drupal\Core\Config\ConfigEvents, so there's not much else we can do for them..

Open a new issue to add it?

- state
- others?

Yeah, how should we handle these? Do we also need a preSave() for them or is there another way?

We're still talking about experimental code after all, if we're able to cover content entities + config IMHO we're already good.

Of course if Workspace is aiming to provide real site previews, then we'll need APIs to cover not only config but state revisioning and who knows what else.

Entity schema revisioning maybe 😱

Also, I wouldn't block this issue on #2983266: Add PRESAVE and PREDELETE config events

Thanks for opening the issue. I agree it's not a blocker for Workspaces reaching beta. Possibly should be a blocker to Workspaces reaching stable, but we can decide on that later.

Path aliases are neither configuration objects nor entities, so I think they'll need their own special handling?

The approach here seems fine to me in general, is the idea that the UX issue and implementation would block a stable release or just be 'major'?

1. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -119,13 +124,21 @@ public function entityLoad(array &$entities, $entity_type_id) {
   +    if (in_array($entity_type->id(), ['workspace_association', 'workspace'], TRUE)
   

   Let's add a protected static function for returning this hard-coded list.

2. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -119,13 +124,21 @@ public function entityLoad(array &$entities, $entity_type_id) {
   +      throw new EntityStorageException('This entity can only be saved in the default workspace.');
   

   Let's add a test to ensure this actually happens when expected.

3. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -268,22 +305,35 @@ protected function trackEntity(EntityInterface $entity) {
   +    // a pending one.
   

   Add "be" to the beginning of this line.

4. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -292,4 +342,28 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
   +    if ((!$is_workspace_form && !$is_views_exposed_form) || $is_config_form) {
   

   1. This needs a comment for why we're excluding Views Exposed forms.

   2. By catching all config forms, we can't click "Clear all caches" on /admin/config/development/performance from a non-live workspace. Is that what's desired? I wonder if there are other config forms (or buttons on config forms) that don't actually modify config that we should exclude in some way.

   3. For forms that are neither entity forms nor config forms, they also pass the first part of the if statement, so this means all sorts of forms that don't change config are blocked by this. E.g., /search/node. However, there are also plenty of forms that aren't ConfigFormBase forms but that do change config: e.g., /admin/modules. So this is tricky. I wonder if we should add some API for forms (or form buttons) to identify themselves as workspace-safe, so that both core forms like Search and various contrib forms can opt into that to bypass this validation.

5. +++ b/core/modules/workspace/tests/src/Kernel/WorkspaceIntegrationTest.php
   @@ -73,7 +73,7 @@ protected function setUp() {
   -    $this->installEntitySchema('entity_test_mulrev');
   +    $this->installEntitySchema('entity_test_mulrevpub');
   

   Should we retain test coverage for 'entity_test_mulrev' and assert what we now expect for it?

+++ b/core/modules/workspace/src/EntityOperations.php
@@ -119,13 +124,21 @@ public function entityLoad(array &$entities, $entity_type_id) {
+      throw new EntityStorageException('This entity can only be saved in the default workspace.');
...
@@ -292,4 +342,28 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
+      $form_state->setError($form, $this->t('This form can only be submitted in the default workspace.'));

So, forms don't run into the exception (the first hunk), since they're caught by the validation (the second hunk). But what about REST / JsonApi? Will they need to catch this exception and transform it to a friendly error response? Pinging @Wim Leers for input.

Reviewed the entire issue. Things that stood out:

1. Thanks for the clear issue title & summary! 👏
2. #2: how did you even get that strange rendered output in the second screenshot? :O
3. #4: I agree with @webchick that we should inform the user before the user spent time filling out a form.
4. #14: Thanks to @effulgentsia for pointing out that not only the save operation is a problem, but also delete.
5. #17: thanks for opening #2983266: Add PRESAVE and PREDELETE config events — I added it to the related issues. WRT those most crucial State key-value pairs that you point out: does that mean we should make those per-workspace? Also, at least in the case of CSS/JS aggregation, I think we could/should disable aggregation while looking at a workspace?
6. #22: @catch++ for pointing out path aliases. Also: @catch-- for pointing out path aliases. 😱 Path aliases always get in the way — with cache invalidation, REST, revisions, Typed Data, Workspaces. That's one subsystem that definitely need to significantly revise for D9!
7. #23 @amateescu for president! +100 to […] time would be better spent on fixing the underlying "problems", e.g. convert path aliases to entities […]
8. #25 Thanks @effulgentsia for the ping!

Patch review, which also addresses #25:

1. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -119,13 +124,21 @@ public function entityLoad(array &$entities, $entity_type_id) {
   +      throw new EntityStorageException('This entity can only be saved in the default workspace.');
   
   @@ -209,6 +222,30 @@ public function entityUpdate(EntityInterface $entity) {
   +      throw new EntityStorageException('This entity can only be deleted in the default workspace.');
   

   These would also be invoked for REST/JSON API/GraphQL. 👍

2. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -292,4 +342,28 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
   +  public function validateForm(array &$form, FormStateInterface $form_state) {
   +    $form_object = $form_state->getFormObject();
   +
   +    $is_workspace_form = $form_object instanceof WorkspaceFormInterface;
   +    $is_views_exposed_form = $form_object instanceof ViewsExposedForm;
   +    $is_config_form = $form_object instanceof ConfigFormBase;
   

   If we need all this logic to prevent/inform the user upon validation of the form, then why don't we use this exact same logic to instead disable the form when the user accesses it? We can just do a drupal_set_message(…, 'error') with the same message if this condition is met, can't we?

3. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -292,4 +342,28 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
   +    if ((!$is_workspace_form && !$is_views_exposed_form) || $is_config_form) {
   +      $form_state->setError($form, $this->t('This form can only be submitted in the default workspace.'));
   +    }
   

   It'd be great if we could add a comment documenting why we cannot rely on EntityOperations::entityPreSave() to throw its EntityStorageException with basically the same error message.

   (It's because that exception only gets thrown upon submission of the form, at which point we can no longer transform that exception into a validation error, right?)

4. +++ b/core/modules/workspace/src/Form/WorkspaceFormInterface.php
   @@ -0,0 +1,12 @@
   +/**
   + * Defines interface for workspace forms so they can be easily distinguished.
   + */
   +interface WorkspaceFormInterface extends FormInterface {
   

   This is not a pattern we have elsewhere, but I like the DX benefit ❤️ 👍

5. +++ b/core/modules/workspace/workspace.module
   @@ -88,6 +88,15 @@ function workspace_entity_update(EntityInterface $entity) {
    /**
   + * Implements hook_entity_predelete().
   + */
   +function workspace_entity_predelete(EntityInterface $entity) {
   +  return \Drupal::service('class_resolver')
   +    ->getInstanceFromDefinition(EntityOperations::class)
   +    ->entityPredelete($entity);
   

   I was wondering why we're only doing this for deleting.

   Then I realized: we're only doing validation when we're about to save a set of values for an entity: when creating or updating.

   But when deleting, Drupal doesn't do any validation. Hence validation constraints don't run. Hence the need for Workspace to handle this in a special way.

So, to answer #25:

1. Note that Workspaces are not (yet) supported by either core REST, JSON API or GraphQL.
2. They'd indeed need to catch the exception, because if they don't, it'd result in a 500 response. But in this case, a 403 response would be more appropriate. I'd suggest subclassing EntityStorageException, so that it becomes possible for them to catch Workspaces-specific storage exceptions.
   Ideally, they wouldn't be exceptions thrown during pre-save or pre-delete stages, but during validation. However, as pointed out above … Drupal doesn't do validation of about-to-be-deleted entities… and that's of course not Workspace's fault!
3. Much like the form-based UI should adequately inform the user before they fill out a form, only to then be greeted by an error, we should apply the same principle in our Workspace support in core REST/JSON API/… — we can do that by communicating via HATEOAS links which operations are available on a given entity (in a given workspace). We'd hence omit create, edit and delete links while accessing entities in the context of a workspace, and those links would be present in the context of the default workspace (which matches what #4 + #5 describe).

I'm not sure whether we want to put them in a workspace.api.php file since all these protections are meant to be temporary.

Really? What do you envision as the non-temporary solution?

#27:

+++ b/core/modules/workspace/src/EntityOperations.php
@@ -366,4 +369,63 @@ public function entityFormEntityBuild($entity_type_id, RevisionableInterface $en
+    // Allow modules to add their own entity type IDs to the whitelist.
+    \Drupal::moduleHandler()->alter('workspace_entity_type_whitelist', $whitelist);
...
+    // Allow modules to add their own entity type IDs to the whitelist.
+    \Drupal::moduleHandler()->alter('workspace_form_id_whitelist', $whitelist);

If this is meant to be temporary as #27 says, then:

• Ideally we wouldn't need this alter hook. Could we live without it?
• If we can't, then let's at least use alterDeprecated() instead of alter() :)

is already linked to this issue ;)

Oops! 🙈😅 Sorry — missed that!

#29: OMG 😱 However … we can call EntityStorageException::getPrevious() to still get the workspace exception, right? At that point there's less value in doing so of course.

#30++

What do you envision as the non-temporary solution?

+1 to this question.

That makes sense.

I manually tested this patch:

1. with a block
2. with a view that has exposed filters (/admin/content)
3. with a node's title
4. with a node's path alias

This resulted in:

1. 👍
2. 👎 — the exposed filter form was fine, but the bulk operations on the view weren't fine, screenshot:
   
3. 👎 — it saved just fine, but afterwards, I'd nonetheless get an error message:
   
4. 👍 — disallowed correctly, with a helpful message

So, two UX refinements are needed.

A bit of manual testing is exactly what we needed at this point!

:)

+++ b/core/modules/workspace/src/EntityOperations.php
@@ -419,13 +406,25 @@ public static function isFormSafeToEdit(FormStateInterface $form_state) {
+      'views_form_',

Wouldn't views_form_* be more in line with what people expect, i.e. "globbing syntax"?

Also: either way (current implementation or proposed globbing syntax), this is missing an update to the docs in workspace.api.php.

The error message comes from the comment form, since comments are not revisionable, and it shows why we can't display the error message before the form validation step: it would basically show up on every page which includes a comment form

😢 I'm trying to think of a clever work-around here … I can think of three strategies (the top one being my favorite):

1. Don't set an error message, but just disable the form
2. Don't set an error message, but add something to $form with a weight of -1000000 so that it sits at the top.
3. Only set an error message if the form in question is the form for the current route, i.e. a _form entry in the route definition that becomes the controller.

+++ b/core/modules/workspace/src/EntityOperations.php
@@ -292,4 +340,93 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
+      'views_form_',

I don't think basing the whitelist decision by form ID is enough. For example, a VBO form might be safe for supported entity types, like nodes, but not for unsupported ones, like comments. Similarly, there might be base forms that are workspace-safe, but child forms that extend that base form that have a submit handler that isn't workspace-safe.

The first thing that comes to my mind is that we should simply let any of the form build/alter/process functions set something like $form['#workspace_safe'] = TRUE. However, what I don't like about that is:

1. FAPI #properties defined by modules aren't meant to be an API for other modules.
2. If you Google "#workspace_safe", you get links to resources about how to make places where people work safe for them, which is great, but not related to the concept that we're trying to express here.

So another possibility is perhaps to introduce a new service, like workspace.form_validator? And have it implement a method like formIsSafe($form, $form_state) or allowForm($form, $form_state)? The implementation of the method could then set some internal property on $form or $form_state, or set it within a protected property keyed by the form's build ID, or whatever else seems good.

If we go that route, I also think that the formAlter() and validateForm() should live in that service too, at least for non-entity forms. So that EntityOperations only acts on entities / entity forms.

it shows why we can't display the error message before the form validation step

I think it's fine for this issue to do it during the validation step. Let's open a followup (not beta-blocker, possibly stable-blocker) for informing the user earlier.

#38:

For example, a VBO form might be safe for supported entity types, like nodes, but not for unsupported ones, like comments.

Very good point! 👌

If you Google "#workspace_safe"

HAH!

#39: Hm. Why can't this work?

The error message comes from the comment form, since comments are not revisionable, and it shows why we can't display the error message before the form validation step: it would basically show up on every page which includes a comment form

😢 I'm trying to think of a clever work-around here … I can think of three strategies (the top one being my favorite):

1. Don't set an error message, but just disable the form
2. Don't set an error message, but add something to $form with a weight of -1000000 so that it sits at the top.
3. Only set an error message if the form in question is the form for the current route, i.e. a _form entry in the route definition that becomes the controller.

#40:

1. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -119,13 +120,21 @@ public function entityLoad(array &$entities, $entity_type_id) {
   +    if (static::isEntityTypeSafeToEdit($entity_type)
   ...
   +    if (!$this->workspaceManager->isEntityTypeSupported($entity_type)) {
   
   @@ -209,6 +218,30 @@ public function entityUpdate(EntityInterface $entity) {
   +    if (static::isEntityTypeSafeToEdit($entity_type)
   ...
   +    if (!$this->workspaceManager->isEntityTypeSupported($entity_type)) {
   

   To me, as a non-expert in the Workspace module, these look very similar yet are clearly used in different ways. I found this pretty confusing. Documentation explaining why we need ::isEntityTypeSafeToEdit() and workspace_entity_type_whitelist() would be very helpful.

   Sorry for not raising this sooner :( It's only now that I realize this.

2. +++ b/core/modules/workspace/workspace.api.php
   @@ -0,0 +1,32 @@
   +/**
   + * Alter the entity type IDs which are safe to edit in a non-default workspace.
   + *
   + * If a module decides that the CRUD operations on a specific entity type do not
   + * impact the default (Live) workspace, they can use this hook and add the
   + * entity type ID to the list.
   + *
   + * @param array &$whitelist
   + *   An array containing entity type IDs.
   + *
   + * @ingroup workspace_api
   + */
   +function hook_workspace_entity_type_whitelist_alter(array &$whitelist) {
   +  // Add the ID of a custom entity type to the whitelist.
   +  $whitelist[] = 'custom_entity_type';
   +}
   +
   

   Wouldn't it be clearer if this were an entity type annotation, like workspace_unaffected = TRUE?

   Each entity type knows this best for itself, this removes the need for adding a new hook to our API surface (and for modules to implement another hook). And if a particular site needs to change this, they can still alter the entity type annotation.

I much prefer #44 over previous patches since it's no longer introducing an API that seems to be of questionable and temporary nature. 👍

Just one last thing then now:

+++ b/core/modules/workspace/src/Form/WorkspaceActivateForm.php
@@ -12,7 +12,7 @@
+class WorkspaceActivateForm extends EntityConfirmFormBase implements WorkspaceFormInterface {

+++ b/core/modules/workspace/src/Form/WorkspaceDeleteForm.php
@@ -10,7 +10,7 @@
+class WorkspaceDeleteForm extends ContentEntityDeleteForm implements WorkspaceFormInterface {

+++ b/core/modules/workspace/src/Form/WorkspaceDeployForm.php
@@ -14,7 +14,7 @@
+class WorkspaceDeployForm extends ContentEntityForm implements WorkspaceFormInterface {

+++ b/core/modules/workspace/src/Form/WorkspaceForm.php
@@ -15,7 +15,7 @@
+class WorkspaceForm extends ContentEntityForm implements WorkspaceFormInterface {

+++ b/core/modules/workspace/src/Form/WorkspaceFormInterface.php
@@ -0,0 +1,10 @@
+interface WorkspaceFormInterface extends FormInterface {}

+++ b/core/modules/workspace/src/FormOperations.php
@@ -0,0 +1,98 @@
+    $is_workspace_form = $form_object instanceof WorkspaceFormInterface;

At this point, why not also just hardcode these form IDs? I'm asking because it seems this interface also is of a temporary nature? Asked differently: why isn't this also in the scope of #2985528: Inform users that they are not able to submit some forms in non-default workspaces before the form validation step?

P.S.: out of curiosity, why did you remove #2985528: Inform users that they are not able to submit some forms in non-default workspaces before the form validation step as a related issue?

I manually tested again. Works as expected. Comes with explicit test coverage. 👍

There are several things I still don't like about this patch, but they're not the fault of the Workspace module or this issue:

1. This does not yet warn the user before (s)he submits the form; i.e. work may be wasted. #2985528: Inform users that they are not able to submit some forms in non-default workspaces before the form validation step exists to fix that major usability problem. 👍
2. The mechanism to decide which (non-entity) forms are okay to submit regardless of the active workspace is not ideal: it's a combination of hardcoded form IDs plus an interface. But that's necessary pragmatism. 👍
   That being said: I'd prefer to see WorkspaceFormInterface be marked @internal. The code doing the form alterations is marked @internal — great! I don't think it makes sense to hold up this patch on that minute detail though, because there is still time to mark it @internal during alpha, and hopefully #2985528 will get rid of it long before it ships with 8.6.0 :).

All in all, this seems like a solid step forward!

1. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -119,13 +119,21 @@ public function entityLoad(array &$entities, $entity_type_id) {
   +    if ($entity_type->getProvider() === 'workspace' || $this->workspaceManager->getActiveWorkspace()->isDefaultWorkspace()) {
   
   @@ -209,6 +217,30 @@ public function entityUpdate(EntityInterface $entity) {
   +    if ($entity_type->getProvider() === 'workspace' || $this->workspaceManager->getActiveWorkspace()->isDefaultWorkspace()) {
   

   Can we move this common logic into a dedicated method?

2. +++ b/core/modules/workspace/src/EntityOperations.php
   @@ -292,4 +335,13 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
   +    $entity->isDefaultRevision(FALSE);
   

   (Unrelated/not a blocker) I'm wondering whether we should adopt the same approach we have in Content Moderation and invoke $storage->createRevision() from workspace_entity_prepare_form() to support content translation properly. At least until we are able to make any module leveraging pending revision play well with the other ones.

3. +++ b/core/modules/workspace/src/FormOperations.php
   @@ -0,0 +1,98 @@
   +    $form['#validate'][] = [$this, 'validateForm'];
   

   Won't this be ignored when button-level validation handlers are set? See FormValidator::executeValidateHandlers().

4. +++ b/core/modules/workspace/src/FormOperations.php
   @@ -0,0 +1,98 @@
   +    $is_views_form = $form_object instanceof ViewsForm;
   

   Why are we whitelisting Views forms? Wouldn't this allow to run bulk operations say on comments?

5. +++ b/core/modules/workspace/src/FormOperations.php
   @@ -0,0 +1,98 @@
   +    if (!$form_state->has('workspace_safe')) {
   

   Can we move this check before computing the value of $workspace_safe? It looks a bit weird that we perform all those checks and then throw them away if they aren't needed. Of course this is not a big deal performance-wise but still caught my eye :)

6. +++ b/core/modules/workspace/tests/src/Kernel/WorkspaceIntegrationTest.php
   @@ -373,8 +376,8 @@ public function testWorkspaces() {
   +    // Add an entity reference field that targets 'entity_test_mulrevpub' entities.
   

   Not wrapping at column 80 :)

7. +++ b/core/modules/workspace/tests/src/Kernel/WorkspaceIntegrationTest.php
   @@ -435,11 +438,15 @@ public function testEntityQueryRelationship() {
   -      ->condition('field_test_entity.entity.name', 'stage entity_test_mulrev')
   +      // ->condition('field_test_entity.entity.name', 'stage entity_test_mulrevpub')
   

   (Unrelated/not a blocker) How come that commenting this line out doesn't make the test fail? Do we need more assertions?

8. +++ b/core/modules/workspace/tests/src/Kernel/WorkspaceIntegrationTest.php
   @@ -448,6 +455,41 @@ public function testEntityQueryRelationship() {
   +  public function testDisallowedEntityCRUDInNonDefaultWorkspace() {
   

   It would be great if we could quickly add explicit tests for the path/book/menu validators, hopefully that's not too big of an effort.

Also:

+++ b/core/modules/workspace/src/FormOperations.php
@@ -0,0 +1,98 @@
+    $is_search_form = in_array($form_object->getFormId(), ['search_block_form', 'search_form'], TRUE);

Would it make sense to make this alterable via PHP settings?

Needs work for at least #48.3 and #48.4. IMO, the rest of #48 doesn't need to block commit, but if some/all of them are easy to address in a reroll, then great.

Re #49, I don't think so. Contrib can mark other forms safe via $form_state->set('workspace_safe', TRUE). I think in a follow-up we should consider removing the search and views ones from the whitelist and make the search and views modules call $form_state->set('workspace_safe', TRUE) from their form builders, but I don't know if we want to do that while workspace.module is experimental or wait till its stable.

Re #48.3, great catch! One (but not the only) example of where that happens is the language edit form. So, if you enable language.module and then try to edit a language in a non-default worksapce, this patch's form validator doesn't run, but entityPresave() does, so you get an uncaught exception / "The website encountered an unexpected error. Please try again later." page. For now, perhaps all we need to do is iterate $form['actions'] and add a button-level #validate to buttons that already have one, and open a follow-up to deal with that more robustly (perhaps as part of #2985528: Inform users that they are not able to submit some forms in non-default workspaces before the form validation step)? Unless someone has a better idea?

For #48.4, I'd be fine with simply not whitelisting ViewsForm at all (disallowing any to be used within a non-default workspace) and opening a post-alpha followup for enabling the ones that are safe (like VBO on a supported entity type). Or do we know of existing Workspace use-cases that already rely on using VBO or other Views forms within a non-default workspace, and that we need to avoid regressing?

+1 on every suggestion in #50.

I'd wait to set workspace safe flags outside the workspace codebase until it's clear this is the mechanism the stable version will rely on.

@amateescu, #52:

1: The choice of 3 seems rather arbitrary to me: the goal was isolating a standalone piece of logic, so the amount of times it appears doesn't really matter much to me, even one would be fine. Anyway, definitely not a blocker so I won't fight over this :)

2: Definitely a separate issue, but ::createRevision() is already the new recommended way to create new revisions (surprise :)), especially after #2975754: Add hooks to act on a new revision being created has been committed. So I guess it could even be hardcoded in the ContentEntityForm itself.

3:

+++ b/core/modules/workspace/src/FormOperations.php
@@ -66,6 +66,15 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
+    foreach (Element::children($form['actions']) as $action) {
+      $form['actions'][$action]['#validate'][] = [$this, 'validateForm'];
+    }

I don't think doing this unconditionally is ok either: this code will cause any existing form-level validator to be skipped. On a second thought, I guess we should traverse the whole form array and detect any existing button-level validators and only ours in those cases. AAMOF $form['actions'] might not be defined or not be the only place where button-level validators are defined. Setting status back to NW for this.

4: Since this is still experimental, I guess I would be fine with any action being prevented in non-default workspaces. But you're welcome to come up with one of your magic tricks if it doesn't take long :)

8: We know that those validators work, but we have no guarantee that they are applied to Workspace: I was hoping a very quick addition to an existing test could cover that. Anyway, not a blocker if it takes too much time, again this is only experimental. However IMO we should at least add a @todo and create a follow-up to be addressed before stable, unless we are able to solve this entire problem-space properly before that, which I guess is unlikely.

1: I wouldn't call myself a fan, I just try to enrich my experience with that of people more knowledgeable than me, which doesn't imply I always agree with them. Anyway, I stand corrected on the "arbitrary" term, but I thought you had a protected method or two to spare ;)

3:

+++ b/core/modules/workspace/src/FormOperations.php
@@ -65,10 +65,7 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
-    $form['#validate'][] = [$this, 'validateForm'];

@@ -93,9 +90,28 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
+    if (isset($element['#validate'])) {
+      $element['#validate'][] = [$this, 'validateDefaultWorkspace'];
+    }

Won't this skip Workspace validation entirely, if no #validate key is defined at any level? I guess we should restore the form-level validator line.

4: 👍

8: I hope we remember to add test coverage, shouldn't it happen.

Ah, ok, we should be good then: if some code is so evil to manually unset the #validate key, then it deserves some more manual work, I guess.

Thanks! Pushed to 8.7.x and 8.6.x.