fix: validate a new node before overwriting the existing one

Opening or creating a node with mode="w" or overwrite=True deleted the
existing node before the new one was validated, so an invalid argument
destroyed the existing data and created nothing. For example,
zarr.open(store, mode="w", config=...) on an existing array erased the
store and then raised TypeError from open_group.

Two deletion points ran too early:
- make_store_path(mode="w") erased the path before dispatch in open,
  open_array, open_group, create and save_array. These now build the
  store path with mode "a" and create the node with overwrite=True.
- overwrite=True deleted the node before its metadata was built in
  AsyncArray._create_v2/_create_v3, init_array and AsyncGroup.from_store.
  save_new_metadata now encodes the metadata first, then deletes, then
  writes.

save_group checks its arguments before erasing the path.

Assisted-by: ClaudeCode:claude-opus-5-5

Author: d-v-b

src/zarr/api/asynchronous.py

@@ -108,6 +108,18 @@ def _infer_overwrite(mode: AccessModeLiteral) -> bool:
     return mode in _OVERWRITE_MODES
 
 
+def _deferred_overwrite_mode(mode: AccessModeLiteral) -> AccessModeLiteral:
+    """
+    The mode for building the store path of a node that `mode` may overwrite.
+
+    Building a store path with mode `"w"` deletes everything under the path before the new
+    node has been validated, so an invalid argument would destroy the existing node and create
+    nothing. Callers build the store path with mode `"a"` instead and create the node with
+    `overwrite=True`, which deletes the existing node only once the new metadata is valid.
+    """
+    return "a" if mode == "w" else mode
+
+
 def _warn_unimplemented_kwargs(kwargs: dict[str, Any]) -> None:
     """
     Emit a "not yet implemented" warning for each provided keyword argument that is not None.
@@ -388,23 +400,32 @@ async def open(
             mode = "r"
         else:
             mode = "a"
-    store_path = await make_store_path(store, mode=mode, path=path, storage_options=storage_options)
+    store_path = await make_store_path(
+        store,
+        mode=_deferred_overwrite_mode(mode),
+        path=path,
+        storage_options=storage_options,
+    )
 
     # TODO: the mode check below seems wrong!
     if "shape" not in kwargs and mode in {"a", "r", "r+", "w"}:
-        try:
-            metadata_dict = await get_array_metadata(store_path, zarr_format=zarr_format)
-            # TODO: remove this cast when we fix typing for array metadata dicts
-            _metadata_dict = cast("ArrayMetadataDict", metadata_dict)
-            # for v2, the above would already have raised an exception if not an array
-            zarr_format = _metadata_dict["zarr_format"]
-            is_v3_array = zarr_format == 3 and _metadata_dict.get("node_type") == "array"
-            if is_v3_array or zarr_format == 2:
-                return AsyncArray(
-                    store_path=store_path, metadata=_metadata_dict, config=kwargs.get("config")
-                )
-        except (FileNotFoundError, NodeTypeValidationError):
-            pass
+        # mode "w" replaces any existing node, so there is nothing to open
+        if mode != "w":
+            try:
+                metadata_dict = await get_array_metadata(store_path, zarr_format=zarr_format)
+                # TODO: remove this cast when we fix typing for array metadata dicts
+                _metadata_dict = cast("ArrayMetadataDict", metadata_dict)
+                # for v2, the above would already have raised an exception if not an array
+                zarr_format = _metadata_dict["zarr_format"]
+                is_v3_array = zarr_format == 3 and _metadata_dict.get("node_type") == "array"
+                if is_v3_array or zarr_format == 2:
+                    return AsyncArray(
+                        store_path=store_path,
+                        metadata=_metadata_dict,
+                        config=kwargs.get("config"),
+                    )
+            except (FileNotFoundError, NodeTypeValidationError):
+                pass
         return await open_group(store=store_path, zarr_format=zarr_format, mode=mode, **kwargs)
 
     try:
@@ -500,7 +521,9 @@ async def save_array(
         raise TypeError("arr argument must be numpy or other NDArrayLike array")
 
     mode = kwargs.pop("mode", "a")
-    store_path = await make_store_path(store, path=path, mode=mode, storage_options=storage_options)
+    store_path = await make_store_path(
+        store, path=path, mode=_deferred_overwrite_mode(mode), storage_options=storage_options
+    )
     if np.isscalar(arr):
         arr = np.array(arr)
     shape = arr.shape
@@ -549,8 +572,6 @@ async def save_group(
         NumPy arrays with data to save.
     """
 
-    store_path = await make_store_path(store, path=path, mode="w", storage_options=storage_options)
-
     if zarr_format is None:
         zarr_format = _default_zarr_format()
 
@@ -565,6 +586,9 @@ async def save_group(
 
     if len(args) == 0 and len(kwargs) == 0:
         raise ValueError("at least one array must be provided")
+
+    # mode "w" deletes everything under the path, so the arguments are checked first
+    store_path = await make_store_path(store, path=path, mode="w", storage_options=storage_options)
     aws = []
     for i, arr in enumerate(args):
         aws.append(
@@ -856,7 +880,9 @@ async def open_group(
         }
     )
 
-    store_path = await make_store_path(store, mode=mode, storage_options=storage_options, path=path)
+    store_path = await make_store_path(
+        store, mode=_deferred_overwrite_mode(mode), storage_options=storage_options, path=path
+    )
     if attributes is None:
         attributes = {}
 
@@ -1062,7 +1088,10 @@ async def create(
     mode = kwargs.pop("mode", None)
     if mode is None:
         mode = "a"
-    store_path = await make_store_path(store, path=path, mode=mode, storage_options=storage_options)
+    overwrite = overwrite or _infer_overwrite(mode)
+    store_path = await make_store_path(
+        store, path=path, mode=_deferred_overwrite_mode(mode), storage_options=storage_options
+    )
 
     config_parsed = parse_array_config(config)
 
@@ -1261,25 +1290,30 @@ async def open_array(
     """
 
     mode = kwargs.pop("mode", None)
-    store_path = await make_store_path(store, path=path, mode=mode, storage_options=storage_options)
+    store_path = await make_store_path(
+        store,
+        path=path,
+        mode=None if mode is None else _deferred_overwrite_mode(mode),
+        storage_options=storage_options,
+    )
 
     if "write_empty_chunks" in kwargs:
         _warn_write_empty_chunks_kwarg()
 
-    try:
-        return await AsyncArray.open(store_path, zarr_format=zarr_format)
-    except FileNotFoundError as err:
-        if not store_path.read_only and mode in _CREATE_MODES:
-            overwrite = _infer_overwrite(mode)
-            _zarr_format = zarr_format or _default_zarr_format()
-            return await create(
-                store=store_path,
-                zarr_format=_zarr_format,
-                overwrite=overwrite,
-                **kwargs,
-            )
-        msg = f"No array found in store {store_path.store} at path {store_path.path}"
-        raise ArrayNotFoundError(msg) from err
+    # mode "w" replaces any existing array, so there is nothing to open
+    if mode != "w":
+        try:
+            return await AsyncArray.open(store_path, zarr_format=zarr_format)
+        except FileNotFoundError as err:
+            if store_path.read_only or mode not in _CREATE_MODES:
+                msg = f"No array found in store {store_path.store} at path {store_path.path}"
+                raise ArrayNotFoundError(msg) from err
+    return await create(
+        store=store_path,
+        zarr_format=zarr_format or _default_zarr_format(),
+        overwrite=_infer_overwrite(mode),
+        **kwargs,
+    )
 
 
 async def open_like(a: ArrayLike, path: str, **kwargs: Any) -> AnyAsyncArray:

src/zarr/core/array.py

@@ -118,7 +118,7 @@
     ArrayV2MetadataDict,
     ArrayV3Metadata,
 )
-from zarr.core.metadata.io import save_metadata
+from zarr.core.metadata.io import save_metadata, save_new_metadata
 from zarr.core.metadata.v2 import (
     CompressorLikev2,
     get_object_codec_id,
@@ -146,7 +146,7 @@
     _parse_bytes_bytes_codec,
     get_pipeline_class,
 )
-from zarr.storage._common import StorePath, ensure_no_existing_node, make_store_path
+from zarr.storage._common import StorePath, make_store_path
 from zarr.storage._utils import _relativize_path
 
 if TYPE_CHECKING:
@@ -334,22 +334,6 @@ def _array_metadata_dict_v3(zarr_json_bytes: Buffer) -> dict[str, JSON]:
     return metadata_dict
 
 
-async def _prepare_overwrite(
-    store_path: StorePath, *, zarr_format: ZarrFormat, overwrite: bool
-) -> None:
-    """
-    Prepare a store path for writing a new node.
-
-    If `overwrite` is true and the store supports deletes, any existing node at
-    `store_path` is deleted. Otherwise, the absence of an existing node is enforced
-    (raising if one is present).
-    """
-    if overwrite and store_path.store.supports_deletes:
-        await store_path.delete_dir()
-    else:
-        await ensure_no_existing_node(store_path, zarr_format=zarr_format)
-
-
 @dataclass(frozen=True)
 class AsyncArray[T_ArrayMetadata: (ArrayV2Metadata, ArrayV3Metadata)]:
     """
@@ -629,8 +613,6 @@ async def _create_v3(
         attributes: dict[str, JSON] | None = None,
         overwrite: bool = False,
     ) -> AsyncArrayV3:
-        await _prepare_overwrite(store_path, zarr_format=3, overwrite=overwrite)
-
         if isinstance(chunk_key_encoding, tuple):
             chunk_key_encoding = (
                 V2ChunkKeyEncoding(separator=chunk_key_encoding[1])
@@ -650,7 +632,7 @@ async def _create_v3(
         )
 
         array = cls(metadata=metadata, store_path=store_path, config=config)
-        await array._save_metadata(metadata, ensure_parents=True)
+        await save_new_metadata(store_path, metadata, overwrite=overwrite, ensure_parents=True)
         return array
 
     @staticmethod
@@ -704,8 +686,6 @@ async def _create_v2(
         attributes: dict[str, JSON] | None = None,
         overwrite: bool = False,
     ) -> AsyncArrayV2:
-        await _prepare_overwrite(store_path, zarr_format=2, overwrite=overwrite)
-
         compressor_parsed: CompressorLikev2
         if compressor == "auto":
             compressor_parsed = default_compressor_v2(dtype)
@@ -733,7 +713,7 @@ async def _create_v2(
         )
 
         array = cls(metadata=metadata, store_path=store_path, config=config)
-        await array._save_metadata(metadata, ensure_parents=True)
+        await save_new_metadata(store_path, metadata, overwrite=overwrite, ensure_parents=True)
         return array
 
     @classmethod
@@ -4530,8 +4510,6 @@ async def init_array(
         chunk_key_encoding, zarr_format=zarr_format
     )
 
-    await _prepare_overwrite(store_path, zarr_format=zarr_format, overwrite=overwrite)
-
     # Validate rectilinear chunks constraints
     if _is_rectilinear_chunks(chunks):
         if zarr_format == 2:
@@ -4645,7 +4623,7 @@ async def init_array(
         )
 
     arr = AsyncArray(metadata=meta, store_path=store_path, config=config)
-    await arr._save_metadata(meta, ensure_parents=True)
+    await save_new_metadata(store_path, meta, overwrite=overwrite, ensure_parents=True)
     return arr
 
 

src/zarr/core/group.py

@@ -48,7 +48,7 @@
 from zarr.core.dtype import parse_data_type
 from zarr.core.json_parse import parse_field
 from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata
-from zarr.core.metadata.io import save_metadata
+from zarr.core.metadata.io import save_metadata, save_new_metadata
 from zarr.core.sync import SyncMixin, sync
 from zarr.errors import (
     ArrayNotFoundError,
@@ -60,7 +60,7 @@
     ZarrUserWarning,
 )
 from zarr.storage import StoreLike, StorePath
-from zarr.storage._common import ensure_no_existing_node, make_store_path
+from zarr.storage._common import make_store_path
 from zarr.storage._utils import _join_paths, _normalize_path_keys, normalize_path
 
 if TYPE_CHECKING:
@@ -477,20 +477,14 @@ async def from_store(
         zarr_format: ZarrFormat = 3,
     ) -> AsyncGroup:
         store_path = await make_store_path(store)
-
-        if overwrite:
-            if store_path.store.supports_deletes:
-                await store_path.delete_dir()
-            else:
-                await ensure_no_existing_node(store_path, zarr_format=zarr_format)
-        else:
-            await ensure_no_existing_node(store_path, zarr_format=zarr_format)
         attributes = attributes or {}
         group = cls(
             metadata=GroupMetadata(attributes=attributes, zarr_format=zarr_format),
             store_path=store_path,
         )
-        await group._save_metadata(ensure_parents=True)
+        await save_new_metadata(
+            store_path, group.metadata, overwrite=overwrite, ensure_parents=True
+        )
         return group
 
     @classmethod

src/zarr/core/metadata/io.py

@@ -9,6 +9,7 @@
 from zarr.storage._common import StorePath, ensure_no_existing_node
 
 if TYPE_CHECKING:
+    from zarr.core.buffer import Buffer
     from zarr.core.common import ZarrFormat
     from zarr.core.group import GroupMetadata
     from zarr.core.metadata import ArrayMetadata
@@ -52,11 +53,53 @@ async def save_metadata(
     ValueError
     """
     to_save = metadata.to_buffer_dict(default_buffer_prototype())
+    await _write_metadata(store_path, to_save, metadata.zarr_format, ensure_parents=ensure_parents)
+
+
+async def save_new_metadata(
+    store_path: StorePath,
+    metadata: ArrayMetadata | GroupMetadata,
+    *,
+    overwrite: bool,
+    ensure_parents: bool = False,
+) -> None:
+    """Save the metadata of a new array or group, replacing any existing node if requested.
+
+    The metadata is encoded before the store is modified, so metadata that cannot be
+    encoded raises without deleting an existing node.
+
+    Parameters
+    ----------
+    store_path : StorePath
+        Location of the new node.
+    metadata : ArrayMetadata | GroupMetadata
+        Metadata of the new node.
+    overwrite : bool
+        If true and the store supports deletes, delete any existing node at `store_path`.
+        Otherwise, raise if a node already exists at `store_path`.
+    ensure_parents : bool, optional
+        Create any missing parent groups, and check no existing parents are arrays.
+    """
+    to_save = metadata.to_buffer_dict(default_buffer_prototype())
+    if overwrite and store_path.store.supports_deletes:
+        await store_path.delete_dir()
+    else:
+        await ensure_no_existing_node(store_path, zarr_format=metadata.zarr_format)
+    await _write_metadata(store_path, to_save, metadata.zarr_format, ensure_parents=ensure_parents)
+
+
+async def _write_metadata(
+    store_path: StorePath,
+    to_save: dict[str, Buffer],
+    zarr_format: ZarrFormat,
+    *,
+    ensure_parents: bool,
+) -> None:
     set_awaitables = [set_or_delete(store_path / key, value) for key, value in to_save.items()]
 
     if ensure_parents:
         # To enable zarr.create(store, path="a/b/c"), we need to create all the intermediate groups.
-        parents = _build_parents(store_path, metadata.zarr_format)
+        parents = _build_parents(store_path, zarr_format)
         ensure_array_awaitables = []
 
         for parent_path, parent_metadata in parents.items():