🐛 Various sandbox fixes, and a setup that runs itself

docs/adr/11-code-execution.md

@@ -16,7 +16,7 @@ That raises the obvious objection. Executing model-written Python against a rese
 
 **Generated code runs against a disposable copy of the user's storage.**
 
-`aiida-agents sandbox init` copies the profile's storage and registers a profile pointing at the copy; `sandbox check` verifies the copy shares no database and no repository with any real profile; `refresh` rebuilds it when it has drifted, and `teardown` removes it. A read-only PostgreSQL role over the copy is available as a second layer, and is no longer the mechanism.
+`aiida-agents sandbox init` copies the profile's storage and registers a profile pointing at the copy; `sandbox check` verifies the copy shares no database and no repository with any real profile; `teardown` removes it. A read-only PostgreSQL role over the copy is available as a second layer, and is no longer the mechanism.
 
 > **Revised (2026-08).** This originally read: *"Generated code runs against a profile whose PostgreSQL role holds no write privilege. The sandbox profile points at **the same database** as the user's own. A scratch profile would be safer and useless: an empty database cannot answer any question worth asking about someone's data."*
 >
@@ -46,8 +46,9 @@ Two further layers sit above it, and neither is containment:
 - A question needing an unanticipated combination of filters is answerable without a new tool.
 - Answers come with output that was actually produced, and the agent fixes its own mistakes from real tracebacks before the user sees them.
 - **The feature is inert until someone runs `sandbox init`.** `run_aiida_code` refuses to run when no sandbox profile is configured, and says the snippet is unverified rather than falling back to the user's writable profile. Silently falling back is the one failure that would make all of the above worthless.
-- **The copy drifts.** It is a snapshot, so anything the user has run since is missing until `sandbox refresh`. That is the price of not sharing their storage, and it is the right way round: a stale answer is visible and correctable, a destroyed database is not.
-- **Copying is not free.** A large provenance database takes time and disk to copy, which is why the copy has a lifecycle (`init`/`refresh`/`teardown`) rather than being made per query.
+- **The copy drifts.** It is a snapshot, so anything the user has run since is missing until it is rebuilt, which is `teardown` then `init`. That is the price of not sharing their storage, and it is the right way round: a stale answer is visible and correctable, a destroyed database is not.
+- **Copying is not free.** A large provenance database takes time and disk to copy, which is why the copy has a lifecycle (`init`/`teardown`) rather than being made per query. `init` says what the copy will cost and asks before making it.
+- **There is deliberately no `refresh`.** A word promising to bring the copy up to date would hide both the cost, which is the whole repository again, and the loss, which is anything the sandbox holds that the source does not. The version worth having syncs incrementally against the source, and that wants [`verdi collab`](https://github.com/aiidateam/aiida-core/pull/7516) underneath it.
 - **The containment covers the database and nothing else.** Code that gets past the guard can still read the filesystem and reach the network. The environment scrub and the rlimits narrow that; OS-level isolation (`bwrap`, `nsjail`, a container) would close it, and is not implemented.
 - Writes remain impossible from this path. A user who wants to submit is told so and routed to the Execution agent, which asks first.
 

src/aiida_agents/_settings.py

@@ -194,10 +194,10 @@ class SandboxSettings(_Base):
     """Where and how generated code is executed (``AIIDA_AGENTS_*``)."""
 
     sandbox_profile: str = "agents-sandbox"
-    """AiiDA profile generated code runs against. Must be one whose database
-    role cannot write --- ``aiida-agents sandbox init`` creates one, and
-    ``sandbox check`` proves it. Nothing downstream can tell a read-only
-    profile from a writable one, so this setting is the whole safety boundary."""
+    """AiiDA profile generated code runs against. Must be a disposable copy of
+    the user's storage --- ``aiida-agents sandbox init`` makes one, and
+    ``sandbox check`` proves it shares nothing. Nothing downstream can tell a
+    copy from the real thing, so this setting is the whole safety boundary."""
 
     sandbox_timeout: float = Field(default=30.0, gt=0)
     """Seconds a snippet may run before it is killed. A query over a large

src/aiida_agents/agents/codegen/__init__.py

@@ -17,8 +17,8 @@
 What it does *not* have is a write tool, and that is deliberate. Executing
 generated code sounds like the most dangerous thing in this project, and it
 would be if it ran against the user's live profile. It does not: it runs
-against one whose database role has no INSERT privilege, so the danger is
-handled at the database rather than by asking a user to vet Python at a prompt.
+against a disposable copy of their storage, so the danger is handled by what
+the code can reach rather than by asking a user to vet Python at a prompt.
 Approving twenty lines of unexecuted code is a weak check. Reading what the
 code actually returned is a real one --- which is why this agent is not gated
 and every tool that genuinely writes still is, on the Execution agent, where

src/aiida_agents/agents/codegen/prompt.md

@@ -43,11 +43,19 @@ output you did not see.
 user's actual database, so the results are their real data. Do not hedge them
 as "example" or "illustrative" values.
 
-**You cannot write, and must not pretend otherwise.** The profile you run
-against has no write privilege at the database level. If the user wants to
-submit a workflow, import a structure or delete something, say that is the
-Execution agent's job and that it will ask for their approval first. Never
-report having submitted, stored or changed anything.
+**Nothing you run reaches the user's own profile.** You run against a copy of
+their storage, and writes are refused before the code runs. If one ever gets
+past that, it changes the copy and nothing else: the user cannot see it, and it
+is thrown away the next time the sandbox is rebuilt. So never report having
+submitted, stored or changed anything, even if the output says you did. If the
+user wants to submit a workflow, import a structure or delete something, say
+that is the Execution agent's job and that it will ask for their approval
+first.
+
+The copy protects their AiiDA storage, not their machine. The filesystem and
+the network are real, so do not treat reading files or reaching the network as
+harmless just because the profile is a copy: neither belongs in a snippet that
+answers a question about provenance.
 
 ## Style
 

src/aiida_agents/cli/doctor.py

@@ -143,11 +143,13 @@ def _check_sandbox() -> _DiagnosticRow:
 
         sharing = profiles_sharing_storage(config, name)
         if sharing:
+            # No remedy named here: `check` is where the two cases are told
+            # apart, and only one of them is the user's to fix.
             return _DiagnosticRow(
                 label,
                 False,
-                f"shares storage with {', '.join(sharing)}; deleting it would "
-                "destroy that data. Rebuild with `aiida-agents sandbox refresh`",
+                "; ".join(entry.describe() for entry in sharing)
+                + "; `aiida-agents sandbox check` spells out what to do",
             )
         return _DiagnosticRow(label, True, "own copy, shared with nothing")
     except Exception as exc:

src/aiida_agents/cli/sandbox.py

@@ -1,12 +1,20 @@
 """``aiida-agents sandbox`` --- build and verify the profile generated code reads.
 
-Four commands over one idea: the sandbox is a **disposable copy** of the user's
-storage, never the storage itself.
+Three commands over one idea: the sandbox is a **disposable copy** of the
+user's storage, never the storage itself.
 
 ``init`` makes the copy and registers a profile for it. ``check`` proves the
 copy shares nothing with a real profile, which is the property that matters.
-``refresh`` rebuilds it when it has drifted. ``teardown`` removes it, and can
-do so safely precisely because nothing else is pointing at what it deletes.
+``teardown`` removes it, and can do so safely precisely because nothing else is
+pointing at what it deletes.
+
+There is deliberately no ``refresh``. Rebuilding is ``teardown`` then ``init``,
+which is two commands that say what they do, where a single word promising to
+bring the copy up to date would hide both the cost (the whole repository again)
+and the loss (anything the sandbox holds that the source does not). The version
+worth having is an incremental sync against the source rather than a fresh
+copy, and that wants `verdi collab
+<https://github.com/aiidateam/aiida-core/pull/7516>`_ underneath it.
 
 The design this replaces pointed the sandbox at the same database through a
 read-only role, and cost a maintainer his data when he deleted the sandbox
@@ -16,10 +24,22 @@
 
 from __future__ import annotations
 
+import contextlib
 import secrets
 import shutil
+from collections.abc import Callable, Iterator
+from pathlib import Path
 
 import rich_click as click
+from rich.filesize import decimal
+from rich.progress import (
+    BarColumn,
+    DownloadColumn,
+    Progress,
+    SpinnerColumn,
+    TextColumn,
+    TimeElapsedColumn,
+)
 
 from aiida_agents.cli.output import console
 
@@ -31,6 +51,63 @@
 DEFAULT_PROFILE = "agents-sandbox"
 
 
+def _agreed_to_copy(source: Path, target: Path, size: int, *, yes: bool) -> bool:
+    """Say what the copy costs, and get agreement unless ``yes``.
+
+    The copy is the whole repository, which on a real profile runs to
+    gigabytes. Announcing it after the fact is how somebody finds out from
+    ``df``, so the size, both paths and the room left go out before anything is
+    written.
+    """
+    anchor = target
+    while not anchor.exists():
+        anchor = anchor.parent
+
+    console.print(f"This copies [bold]{decimal(size)}[/bold]")
+    # `soft_wrap` so a path longer than the terminal breaks where the terminal
+    # breaks it, rather than being reflowed with a space in the middle of a
+    # directory name, which is a path nobody can paste back.
+    console.print(f"  from [cyan]{source}[/cyan]", soft_wrap=True)
+    console.print(f"  to   [cyan]{target}[/cyan]", soft_wrap=True)
+    console.print(f"  [dim]{decimal(shutil.disk_usage(anchor).free)} free there[/dim]")
+    return yes or click.confirm("Proceed?")
+
+
+@contextlib.contextmanager
+def _copy_progress(size: int) -> Iterator[Callable[[int], None]]:
+    """A progress bar counting bytes, yielding the callback that advances it.
+
+    Bytes rather than files: a packed disk-objectstore is a handful of very
+    large files, so a file count would sit at nothing and then finish.
+    """
+    with Progress(
+        SpinnerColumn(),
+        TextColumn("[progress.description]{task.description}"),
+        BarColumn(),
+        DownloadColumn(),
+        TimeElapsedColumn(),
+        console=console,
+    ) as progress:
+        task = progress.add_task("Copying storage", total=size)
+        yield lambda advance: progress.advance(task, advance)
+
+
+def _refuse_if_sharing(source: object, sandbox: object, profile: object) -> None:
+    """Stop unless the sandbox's storage is provably the source's alone.
+
+    Called before the copy as well as before registration: a source directory
+    that contains the sandbox root would otherwise be copied into itself, and
+    the refusal would arrive after the gigabytes had landed.
+    """
+    from aiida_agents.sandbox.copy import shares_storage
+
+    if shares_storage(source, sandbox):  # type: ignore[arg-type]
+        raise click.ClickException(
+            "Refusing to build a sandbox that shares storage with "
+            f"{profile.name!r}. Deleting it would take the real data with it."  # type: ignore[attr-defined]
+        )
+
+
 def _source_profile(name: str | None) -> object:
     from aiida.manage.configuration import get_config
 
@@ -63,7 +140,8 @@ def sandbox() -> None:
     show_default=True,
     help="PostgreSQL role to read the copy through. Optional hardening.",
 )
-def init(profile: str | None, sandbox_name: str, role: str) -> None:
+@click.option("--yes", is_flag=True, help="Do not ask before copying.")
+def init(profile: str | None, sandbox_name: str, role: str, yes: bool) -> None:
     """Copy the profile's storage and register a sandbox profile for it.
 
     On SQLite this is done for you: the storage directory is copied and the
@@ -77,13 +155,14 @@ def init(profile: str | None, sandbox_name: str, role: str) -> None:
 
     from aiida_agents.sandbox.copy import (
         SUPPORTED_BACKENDS,
-        SandboxStorage,
+        ProfileStorage,
         copy_sqlite_storage,
         postgres_copy_commands,
+        postgres_sandbox_storage,
         register_profile,
         sandbox_profile_dictionary,
         sandbox_storage_root,
-        shares_storage,
+        storage_size,
     )
 
     config = get_config()
@@ -98,22 +177,28 @@ def init(profile: str | None, sandbox_name: str, role: str) -> None:
         )
     if sandbox_name in {existing.name for existing in config.profiles}:
         raise click.ClickException(
-            f"Profile {sandbox_name!r} already exists. Use `sandbox refresh` to "
-            "rebuild it, or `sandbox teardown` to remove it first."
+            f"Profile {sandbox_name!r} already exists. Remove it with `sandbox "
+            "teardown` first, then run this again to rebuild it."
         )
 
     root = sandbox_storage_root(sandbox_name)
 
     if backend == "core.sqlite_dos":
-        from pathlib import Path
-
+        storage = Path(storage_config["filepath"])
         target = root / "storage"
-        console.print(f"Copying storage to [cyan]{target}[/cyan] ...")
+        new_storage = ProfileStorage(backend, {"filepath": str(target)})
+        # Before the copy, not after. A source directory that contains the
+        # sandbox root would be copied into itself, and the check below would
+        # then refuse to register what had just been written to disk.
+        _refuse_if_sharing(ProfileStorage(backend, storage_config), new_storage, source)
         try:
-            copy_sqlite_storage(Path(storage_config["filepath"]), target)
+            size = storage_size(storage)
+            if not _agreed_to_copy(storage, target, size, yes=yes):
+                return
+            with _copy_progress(size) as advance:
+                copy_sqlite_storage(storage, target, progress=advance)
         except (FileNotFoundError, FileExistsError, OSError) as exc:
             raise click.ClickException(str(exc)) from exc
-        new_storage = SandboxStorage(backend, {"filepath": str(target)})
     else:
         sandbox_database = (
             f"{storage_config.get('database_name', 'aiida')}_agents_sandbox"
@@ -142,23 +227,15 @@ def init(profile: str | None, sandbox_name: str, role: str) -> None:
             f"Then rerun [cyan]aiida-agents sandbox init --sandbox-name "
             f"{sandbox_name}[/cyan] to register the profile."
         )
-        new_storage = SandboxStorage(
-            backend,
-            {
-                **storage_config,
-                "database_name": sandbox_database,
-                "repository_uri": str(repository),
-            },
+        new_storage = postgres_sandbox_storage(
+            storage_config, database=sandbox_database, repository=repository
         )
         if not _database_exists(new_storage.config):
             return
 
-    # The whole point of the exercise, checked rather than assumed.
-    if shares_storage(backend, storage_config, new_storage.backend, new_storage.config):
-        raise click.ClickException(
-            "Refusing to register a sandbox that shares storage with "
-            f"{source.name!r}. Deleting it would take the real data with it."  # type: ignore[attr-defined]
-        )
+    # The whole point of the exercise, checked rather than assumed. Checked
+    # again here because the PostgreSQL path builds its storage further down.
+    _refuse_if_sharing(ProfileStorage(backend, storage_config), new_storage, source)
 
     register_profile(
         config,
@@ -209,7 +286,7 @@ def check(profile: str) -> None:
     """
     from aiida.manage.configuration import get_config
 
-    from aiida_agents.sandbox.copy import profiles_sharing_storage
+    from aiida_agents.sandbox.copy import Overlap, profiles_sharing_storage
     from aiida_agents.sandbox.setup import verify_read_only
 
     config = get_config()
@@ -220,11 +297,28 @@ def check(profile: str) -> None:
 
     failures = profiles_sharing_storage(config, profile)
     if failures:
-        console.print(
-            f"[red]✗[/red] {profile!r} shares storage with "
-            f"{', '.join(repr(name) for name in failures)}. Deleting it would "
-            "destroy their data too. This must not be used as a sandbox."
-        )
+        # A proven overlap and a config that could not be read both stop the
+        # command, and saying which one it was is the difference between "your
+        # sandbox is pointed at your own storage" and "some profile here does
+        # not say where its storage is". Only the first is about their data.
+        proven = any(entry.overlap is Overlap.SHARED for entry in failures)
+        verdict = "must not be used" if proven else "cannot be cleared for use"
+        console.print(f"[red]✗[/red] {profile!r} {verdict} as a sandbox:")
+        for entry in failures:
+            console.print(f"    {entry.describe()}")
+        # Both remedies when there is one of each: `init` compares the copy
+        # against its source alone, so a rebuild clears the overlap and leaves
+        # the unreadable profile failing the next check for the same reason.
+        if proven:
+            console.print(
+                f"  Delete it with [cyan]verdi profile delete --keep-data "
+                f"{profile}[/cyan], then rebuild with [cyan]aiida-agents "
+                "sandbox init[/cyan]."
+            )
+        if any(entry.overlap is Overlap.UNKNOWN for entry in failures):
+            console.print(
+                "  What cannot be read is treated as sharing until it can be ruled out."
+            )
         raise SystemExit(1)
 
     console.print(f"[green]✓[/green] {profile!r} shares no storage with any profile")
@@ -235,24 +329,6 @@ def check(profile: str) -> None:
         console.print(f"{mark} {result.detail}")
 
 
-@sandbox.command("refresh")
-@click.option(
-    "--profile", default=DEFAULT_PROFILE, show_default=True, help="Sandbox to rebuild."
-)
-@click.option(
-    "--source", default=None, help="Profile to copy from. Default: the default profile."
-)
-@click.pass_context
-def refresh(ctx: click.Context, profile: str, source: str | None) -> None:
-    """Rebuild the copy from the source profile.
-
-    The copy drifts the moment the user runs anything, which is the accepted
-    cost of not sharing their storage. This is how it catches up.
-    """
-    ctx.invoke(teardown, profile=profile, yes=True)
-    ctx.invoke(init, profile=source, sandbox_name=profile, role=DEFAULT_ROLE)
-
-
 @sandbox.command("teardown")
 @click.option(
     "--profile", default=DEFAULT_PROFILE, show_default=True, help="Sandbox to remove."
@@ -279,9 +355,10 @@ def teardown(profile: str, yes: bool) -> None:
 
     sharing = profiles_sharing_storage(config, profile)
     if sharing:
+        reasons = "\n".join(f"  {entry.describe()}" for entry in sharing)
         raise click.ClickException(
-            f"{profile!r} shares storage with {', '.join(sharing)}. Refusing to "
-            "delete its data. Remove the profile by hand if you are certain."
+            f"Refusing to delete {profile!r} and its storage:\n{reasons}\n"
+            "Remove the profile by hand if you are certain."
         )
 
     if not yes and not click.confirm(f"Delete profile {profile!r} and its copy?"):