Repository: canonical/operator

Issue:
Cannot call Secret.set_info and Secret.set_content in the same hook
If `Secret.set_info()` and `Secret.set_content()` are both called in the same hook, then whichever is first is ignored.

I expect that from a Juju point of view, this is because `secret_set` has been called twice and so it assumes that the later call replaces the earlier one, rather than trying to merge the two together. This is particularly unexpected behaviour in ops because we have two different methods exposing parts of `secret_set`, so a charmer doesn't necessarily know that it's the same underlying Juju hook tool.

This can be reproduced with this small charm:

```python
class SecretInfoAndContentSetCharm(ops.CharmBase):
    """Charm the application."""

    def __init__(self, framework: ops.Framework):
        super().__init__(framework)
        framework.observe(self.on.start, self._on_start)
        framework.observe(self.on.set_action, self._on_set)

    def _on_start(self, event: ops.StartEvent):
        self.unit.add_secret(label="my-secret", content={"foo": "bar"})
        self.unit.status = ops.ActiveStatus()

    def _on_set(self, event):
        secret = self.model.get_secret(label="my-secret")
        secret.set_content(event.params)
        secret.set_info(description="hello world")
```

If you call the action (`juju run unit-name/0 set one=two`) with the code like it is above, then the description will be set, but the content will not. If you swap the code so that the `set_content` and `set_info` calls are around the other way, then the action will set the content but not the description.

Discussion:
@tonyandrewmeyer: This seems non-trivial to fix - ops would need to keep the data passed to both `set_content` and `set_info` in a cache (merging as each was called) and then do the `secret_set` call only on conclusion of the hook (note that it's not just the current event, as the calls might be in the Juju event but also in a deferred or ops Lifecycle event).

At minimum, we should document this restriction so that it's easier to figure out what's happening.@benhoyt: Thanks for the report. It does seem odd to me that the second `secret-set` call (without content args) wipes out the content, so maybe this is a Juju bug. I'll put it on the list to discuss with Ian next Monday.@benhoyt: We discussed this in daily sync today, and decided this is probably is worth fixing. You'd be likely to call `set_info` when a secret expires, and you'd want to call `set_content` then too. So this could really happen in practice.

Our idea is to keep a cache on the backend (dict of secret id to something), and whenever set_info or set_content is called, it updates the info, merging info and content. But setting content a second time would replace.@tonyandrewmeyer: Interestingly, the [tls-certificates-interface lib](https://github.com/canonical/manual-tls-certificates-operator/blob/e22219b9107770d25bcbe4105d469b28b56feff7/lib/charms/tls_certificates_interface/v3/tls_certificates.py#L1896) does have a `set_content()` followed immediately by `set_info()` in relation-changed. It seems like that can't work at the moment - there is another path where the secret doesn't already exist and a new one is created - maybe that is the one that always gets exercised?

It would be good if whoever works on this ticket takes a closer look at that, before and after the change.@saltiyazan: > Interestingly, the [tls-certificates-interface lib](https://github.com/canonical/manual-tls-certificates-operator/blob/e22219b9107770d25bcbe4105d469b28b56feff7/lib/charms/tls_certificates_interface/v3/tls_certificates.py#L1896) does have a `set_content()` followed immediately by `set_info()` in relation-changed. It seems like that can't work at the moment - there is another path where the secret doesn't already exist and a new one is created - maybe that is the one that always gets exercised?
> 
> It would be good if whoever works on this ticket takes a closer look at that, before and after the change.

I looked into this, and it appears to be dead code that will no longer be executed due to some changes introduced to the library. I'll investigate this further to confirm.@saltiyazan: I came across this issue and I was about to open a bug, happy that you are aware if it.
Do you suggest a specific workaround until a fix is merged?@tonyandrewmeyer: > I came across this issue and I was about to open a bug, happy that you are aware if it.

Thanks for letting us know - I'll see if we can get a fix in for the next release.

> Do you suggest a specific workaround until a fix is merged?

Unfortunately, I think the only workaround would be to call the hook tool yourself - basically, add a method to your charm that does [what `set_info` does](https://github.com/canonical/operator/blob/d3b878e222e89f427527ef64c194129f40e1e928/ops/model.py#L1478) but can also set the content, using the private attributes to get the model backend, and use that.

Solution patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index d4ed43f22..1649f28d4 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,6 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
+    rev: v4.5.0
     hooks:
       - id: check-added-large-files
       - id: check-ast
@@ -24,3 +24,10 @@ repos:
         args: [ --preview ]
       - id: ruff-format
         args: [ --preview ]
+  # Spellcheck the code.
+  - repo: https://github.com/codespell-project/codespell
+    rev: v2.2.4
+    hooks:
+    - id: codespell
+      additional_dependencies:
+        - tomli
diff --git a/CHANGES.md b/CHANGES.md
index bcf3a1c3e..69d6d69a5 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -159,7 +159,7 @@
 
 ## Features
 
-* `StopEvent`, `RemoveEvent`, and all `LifeCycleEvent`s are no longer deferrable, and will raise a `RuntimeError` if `defer()` is called on the event object (#1122)
+* `StopEvent`, `RemoveEvent`, and all `LifecycleEvent`s are no longer deferrable, and will raise a `RuntimeError` if `defer()` is called on the event object (#1122)
 * Add `ActionEvent.id`, exposing the JUJU_ACTION_UUID environment variable (#1124)
 * Add support for creating `pebble.Plan` objects by passing in a `pebble.PlanDict`, the
   ability to compare two `Plan` objects with `==`, and the ability to create an empty Plan with `Plan()` (#1134)
diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md
index b5bc1b1f8..073221512 100644
--- a/CODE_OF_CONDUCT.md
+++ b/CODE_OF_CONDUCT.md
@@ -6,7 +6,7 @@ In the interest of fostering an open and welcoming environment, we as
 contributors and maintainers pledge to making participation in our project and
 our community a harassment-free experience for everyone, regardless of age, body
 size, disability, ethnicity, sex characteristics, gender identity and expression,
-level of experience, education, socio-economic status, nationality, personal
+level of experience, education, socioeconomic status, nationality, personal
 appearance, race, religion, or sexual identity and orientation.
 
 ## Our Standards
diff --git a/docs/index.rst b/docs/index.rst
index 84f58b3f8..c27e2bbe5 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -2,10 +2,31 @@
 ops library API reference
 =========================
 
+The `ops` library is a Python framework for writing and testing Juju charms.
+
+  See more: `Charm SDK documentation <https://juju.is/docs/sdk>`_
+
+The library provides:
+
+- :ref:`ops_main_entry_point`, used to initialise and run your charm;
+- :ref:`ops_module`, the API to respond to Juju events and manage the application;
+- :ref:`ops_pebble_module`, the Pebble client, a low-level API for Kubernetes containers;
+- :ref:`ops_testing_module`, the framework for unit testing charms in a simulated environment;
+
+You can structure your charm however you like, but with the `ops` library, you
+get a framework that promotes consistency and readability by following best
+practices. It also helps you organise your code better by separating different
+aspects of the charm, such as managing the application’s state, handling
+integrations with other services, and making the charm easier to test.
+
+
 .. toctree::
    :maxdepth: 2
    :caption: Contents:
 
+
+.. _ops_module:
+
 ops module
 ==========
 
@@ -13,8 +34,13 @@ ops module
    :exclude-members: main
 
 
+.. _ops_main_entry_point:
+
 ops.main entry point
 ====================
+
+The main entry point to initialise and run your charm.
+
 .. autofunction:: ops.main
 
 
@@ -25,12 +51,16 @@ legacy main module
    :noindex:
 
 
+.. _ops_pebble_module:
+
 ops.pebble module
 =================
 
 .. automodule:: ops.pebble
 
 
+.. _ops_testing_module:
+
 ops.testing module
 ==================
 
diff --git a/ops/__init__.py b/ops/__init__.py
index 3108647db..0e34ef169 100644
--- a/ops/__init__.py
+++ b/ops/__init__.py
@@ -12,31 +12,42 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""The ops library: a Python framework for writing Juju charms.
+"""The API to respond to Juju events and manage the application.
 
-The ops library is a Python framework (`available on PyPI`_) for developing
-and testing Juju charms in a consistent way, using standard Python constructs
-to allow for clean, maintainable, and reusable code.
+This module provides code freatures to your charm, including:
 
-A charm is an operator -- business logic encapsulated in a reusable software
-package that automates every aspect of an application's life.
+- :class:`~ops.CharmBase`, the base class for charms and :class:`~ops.Object`,
+  the base class for charm libraries.
+- :class:`~ops.framework.EventBase` class and individual event types, like
+  the :class:`~ops.ActionEvent` class.
+- :class:`~ops.Framework` class, accessible as ``self.framework`` in a charm,
+  the main interface for the charm to `ops` library infrastructure, including:
 
-Charms written with ops support Kubernetes using Juju's "sidecar charm"
-pattern, as well as charms that deploy to Linux-based machines and containers.
+  - :attr:`~ops.Framework.on` shorthand property used to
+    :meth:`~ops.Framework.observe` and react to Juju events.
+  - :attr:`~ops.Framework.model` attribute to get hold of the Model instance.
 
-Charms should do one thing and do it well. Each charm drives a single
-application and can be integrated with other charms to deliver a complex
-system. A charm handles creating the application in addition to scaling,
-configuration, optimisation, networking, service mesh, observability, and other
-day-2 operations specific to the application.
+- :class:`~ops.model.Model` class that represents the Juju model, accessible as
+  ``self.model`` in a charm, including:
 
-The ops library is part of the Charm SDK (the other part being Charmcraft).
-Full developer documentation for the Charm SDK is available at
-https://juju.is/docs/sdk.
+  - :attr:`~ops.Model.app` attribute, representing the application associated
+    with the charm.
+  - :attr:`~ops.Model.unit` attribute, representing the unit of the application
+    the charm is running on.
+  - :attr:`~ops.Model.relations` attribute, which provides access to relations
+    (integrations) defined in the charm, allowing interaction with other applications.
 
-To learn more about Juju, visit https://juju.is/docs/olm.
+- :class:`~ops.Container` class to control Kubernetes workloads, including:
 
-.. _available on PyPI: https://pypi.org/project/ops/
+  - :meth:`~ops.Container.add_layer` and :meth:`~ops.Container.replan` methods
+    to update Pebble configuration.
+  - :meth:`~ops.Container.pull` and :meth:`~ops.Container.push` methods to copy
+    data to and from a container, respectively.
+  - :meth:`~ops.Container.exec` method to run arbitrary commands inside the
+    container.
+
+- :class:`~ops.StatusBase` class and individual status types, like the
+  :class:`~ops.ActiveStatus` class.
 """
 
 # The "from .X import Y" imports below don't explicitly tell Pyright (or MyPy)
diff --git a/ops/_private/harness.py b/ops/_private/harness.py
index 09334406e..c762b454a 100644
--- a/ops/_private/harness.py
+++ b/ops/_private/harness.py
@@ -380,7 +380,7 @@ def begin(self) -> None:
         if self._charm is not None:
             raise RuntimeError('cannot call the begin method on the harness more than once')
 
-        # The Framework adds attributes to class objects for events, etc. As such, we can't re-use
+        # The Framework adds attributes to class objects for events, etc. As such, we can't reuse
         # the original class against multiple Frameworks. So create a locally defined class
         # and register it.
         # TODO: jam 2020-03-16 We are looking to changes this to Instance attributes instead of
diff --git a/ops/charm.py b/ops/charm.py
index edef8db32..4d580081e 100644
--- a/ops/charm.py
+++ b/ops/charm.py
@@ -941,7 +941,12 @@ def secret(self) -> model.Secret:
         until :meth:`Secret.get_content()` is called.
         """
         backend = self.framework.model._backend
-        return model.Secret(backend=backend, id=self._id, label=self._label)
+        return model.Secret(
+            backend=backend,
+            id=self._id,
+            label=self._label,
+            _secret_set_cache=self.framework.model._cache._secret_set_cache,
+        )
 
     def snapshot(self) -> Dict[str, Any]:
         """Used by the framework to serialize the event to disk.
@@ -1105,12 +1110,13 @@ class CollectStatusEvent(LifecycleEvent):
 
     The order of priorities is as follows, from highest to lowest:
 
-    * error
     * blocked
     * maintenance
     * waiting
     * active
-    * unknown
+
+    It is an error to call :meth:`add_status` with an instance of
+    :class:`ErrorStatus` or :class:`UnknownStatus`.
 
     If there are multiple statuses with the same priority, the first one added
     wins (and if an event is observed multiple times, the handlers are called
@@ -1151,6 +1157,10 @@ def add_status(self, status: model.StatusBase):
         """
         if not isinstance(status, model.StatusBase):
             raise TypeError(f'status should be a StatusBase, not {type(status).__name__}')
+        if status.name not in model._SETTABLE_STATUS_NAMES:
+            raise model.InvalidStatusError(
+                f'status.name must be in {model._SETTABLE_STATUS_NAMES}, not {status.name!r}'
+            )
         model_ = self.framework.model
         if self.handle.kind == 'collect_app_status':
             if not isinstance(status, model.ActiveStatus):
diff --git a/ops/framework.py b/ops/framework.py
index 11c09e50c..0fd089510 100644
--- a/ops/framework.py
+++ b/ops/framework.py
@@ -518,7 +518,11 @@ def __getattr__(self, name: str) -> BoundEvent:
 
 
 class LifecycleEvent(EventBase):
-    """Events tied to the lifecycle of the Framework object."""
+    """Events tied to the lifecycle of the Framework object.
+
+    Note: Here, "lifecycle" was poorly named and has nothing to do with the
+    Juju [application] lifecycle.
+    """
 
     def defer(self) -> NoReturn:
         """Lifecycle events are not deferrable like other events.
diff --git a/ops/jujuversion.py b/ops/jujuversion.py
index 8e25b3eba..ea27eb3e2 100644
--- a/ops/jujuversion.py
+++ b/ops/jujuversion.py
@@ -144,3 +144,13 @@ def supports_exec_service_context(self) -> bool:
             # releases have the change (3.2.1 tag was never released).
             return False
         return True
+
+    @property
+    def supports_pebble_log_forwarding(self) -> bool:
+        """Report whether the Pebble bundled with this Juju version supports log forwarding."""
+        # Log forwarding was available from Pebble 1.4, but labels were added in
+        # 1.6, and that's when this became really useful, so we use that as a
+        # cutoff. Juju 3.4.0 was the first to have Pebble 1.6 (actually 1.7).
+        # https://github.com/canonical/pebble/releases/tag/v1.6.0
+        # https://github.com/juju/juju/blob/e1b7dcd7390348c37f8b860011e7436e6ed3f4cc/go.mod#L27
+        return (self.major, self.minor, self.patch) >= (3, 4, 0)
diff --git a/ops/model.py b/ops/model.py
index 335dd9aa1..970a08ee7 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -14,6 +14,7 @@
 
 """Representations of Juju's model, application, unit, and other entities."""
 
+import collections
 import dataclasses
 import datetime
 import enum
@@ -304,7 +305,13 @@ def get_secret(self, *, id: Optional[str] = None, label: Optional[str] = None) -
             # Canonicalize to "secret:<id>" form for consistency in backend calls.
             id = Secret._canonicalize_id(id, self.uuid)
         content = self._backend.secret_get(id=id, label=label)
-        return Secret(self._backend, id=id, label=label, content=content)
+        return Secret(
+            self._backend,
+            id=id,
+            label=label,
+            content=content,
+            _secret_set_cache=self._cache._secret_set_cache,
+        )
 
     def get_cloud_spec(self) -> 'CloudSpec':
         """Get details of the cloud in which the model is deployed.
@@ -333,6 +340,9 @@ class _ModelCache:
     def __init__(self, meta: 'ops.charm.CharmMeta', backend: '_ModelBackend'):
         self._meta = meta
         self._backend = backend
+        self._secret_set_cache: collections.defaultdict[str, Dict[str, Any]] = (
+            collections.defaultdict(dict)
+        )
         self._weakrefs: _WeakCacheType = weakref.WeakValueDictionary()
 
     @typing.overload
@@ -502,7 +512,13 @@ def add_secret(
             rotate=rotate,
             owner='application',
         )
-        return Secret(self._backend, id=id, label=label, content=content)
+        return Secret(
+            self._backend,
+            id=id,
+            label=label,
+            content=content,
+            _secret_set_cache=self._cache._secret_set_cache,
+        )
 
 
 def _calculate_expiry(
@@ -685,7 +701,13 @@ def add_secret(
             rotate=rotate,
             owner='unit',
         )
-        return Secret(self._backend, id=id, label=label, content=content)
+        return Secret(
+            self._backend,
+            id=id,
+            label=label,
+            content=content,
+            _secret_set_cache=self._cache._secret_set_cache,
+        )
 
     def open_port(
         self, protocol: typing.Literal['tcp', 'udp', 'icmp'], port: Optional[int] = None
@@ -1270,6 +1292,7 @@ def __init__(
         id: Optional[str] = None,
         label: Optional[str] = None,
         content: Optional[Dict[str, str]] = None,
+        _secret_set_cache: Optional['collections.defaultdict[str, Dict[str, Any]]'] = None,
     ):
         if not (id or label):
             raise TypeError('Must provide an id or label, or both')
@@ -1279,6 +1302,9 @@ def __init__(
         self._id = id
         self._label = label
         self._content = content
+        self._secret_set_cache: collections.defaultdict[str, Dict[str, Any]] = (
+            collections.defaultdict(dict) if _secret_set_cache is None else _secret_set_cache
+        )
 
     def __repr__(self):
         fields: List[str] = []
@@ -1483,7 +1509,26 @@ def set_content(self, content: Dict[str, str]):
         self._validate_content(content)
         if self._id is None:
             self._id = self.get_info().id
-        self._backend.secret_set(typing.cast(str, self.id), content=content)
+
+        # If `_backend.secret_set` has already been called, this call will
+        # overwrite anything done in the previous call (even if it was in a
+        # different event handler, as long as it was in the same Juju hook
+        # context). We want to provide more predictable behavior, so we cache
+        # the values and provide them in subsequent calls.
+        cached_data = self._secret_set_cache[self._id]
+        expire = _calculate_expiry(cached_data['expire']) if 'expire' in cached_data else None
+        # Don't store the actual secret content in memory, but put a sentinel
+        # there to indicate that the content has been set during this hook.
+        cached_data['content'] = object()
+
+        self._backend.secret_set(
+            typing.cast(str, self.id),
+            content=content,
+            label=cached_data.get('label'),
+            description=cached_data.get('description'),
+            expire=expire,
+            rotate=cached_data.get('rotate'),
+        )
         # We do not need to invalidate the cache here, as the content is the
         # same until `refresh` is used, at which point the cache is invalidated.
 
@@ -1513,12 +1558,33 @@ def set_info(
         """
         if label is None and description is None and expire is None and rotate is None:
             raise TypeError(
-                'Must provide a label, description, expiration time, ' 'or rotation policy'
+                'Must provide a label, description, expiration time, or rotation policy'
             )
         if self._id is None:
             self._id = self.get_info().id
+
+        # If `_backend.secret_set` has already been called, this call will
+        # overwrite anything done in the previous call (even if it was in a
+        # different event handler, as long as it was in the same Juju hook
+        # context). We want to provide more predictable behavior, so we cache
+        # the values and provide them in subsequent calls.
+        cached_data = self._secret_set_cache[self._id]
+        label = cached_data.get('label') if label is None else label
+        cached_data['label'] = label
+        description = cached_data.get('description') if description is None else description
+        cached_data['description'] = description
+        expire = cached_data.get('expire') if expire is None else expire
+        cached_data['expire'] = expire
+        rotate = cached_data.get('rotate') if rotate is None else rotate
+        cached_data['rotate'] = rotate
+        # Get the previous content from the unit agent's cache.
+        content = (
+            self._backend.secret_get(id=self._id, peek=True) if 'content' in cached_data else None
+        )
+
         self._backend.secret_set(
             typing.cast(str, self.id),
+            content=content,
             label=label,
             description=description,
             expire=_calculate_expiry(expire),
@@ -1907,6 +1973,9 @@ def __init__(self, message: str = ''):
             raise TypeError('cannot instantiate a base class')
         self.message = message
 
+    def __init_subclass__(cls):
+        StatusBase._register(cls)
+
     def __eq__(self, other: 'StatusBase') -> bool:
         if not isinstance(self, type(other)):
             return False
@@ -1938,14 +2007,21 @@ def from_name(cls, name: str, message: str):
 
     @classmethod
     def register(cls, child: Type['StatusBase']):
-        """Register a Status for the child's name."""
+        """.. deprecated:: 2.17.0 Deprecated - this was for internal use only."""
+        warnings.warn(
+            'StatusBase.register is for internal use only', DeprecationWarning, stacklevel=2
+        )
+        cls._register(child)
+        return child
+
+    @classmethod
+    def _register(cls, child: Type['StatusBase']) -> None:
         if not (hasattr(child, 'name') and isinstance(child.name, str)):
             raise TypeError(
                 f"Can't register StatusBase subclass {child}: ",
                 'missing required `name: str` class attribute',
             )
         cls._statuses[child.name] = child
-        return child
 
     _priorities = {
         'error': 5,
@@ -1965,7 +2041,6 @@ def _get_highest_priority(cls, statuses: 'List[StatusBase]') -> 'StatusBase':
         return max(statuses, key=lambda status: cls._priorities.get(status.name, 0))
 
 
-@StatusBase.register
 class UnknownStatus(StatusBase):
     """The unit status is unknown.
 
@@ -1986,7 +2061,6 @@ def __repr__(self):
         return 'UnknownStatus()'
 
 
-@StatusBase.register
 class ErrorStatus(StatusBase):
     """The unit status is error.
 
@@ -2000,7 +2074,6 @@ class ErrorStatus(StatusBase):
     name = 'error'
 
 
-@StatusBase.register
 class ActiveStatus(StatusBase):
     """The unit or application is ready and active.
 
@@ -2016,7 +2089,6 @@ def __init__(self, message: str = ''):
         super().__init__(message)
 
 
-@StatusBase.register
 class BlockedStatus(StatusBase):
     """The unit or application requires manual intervention.
 
@@ -2027,7 +2099,6 @@ class BlockedStatus(StatusBase):
     name = 'blocked'
 
 
-@StatusBase.register
 class MaintenanceStatus(StatusBase):
     """The unit or application is performing maintenance tasks.
 
@@ -2041,7 +2112,6 @@ class MaintenanceStatus(StatusBase):
     name = 'maintenance'
 
 
-@StatusBase.register
 class WaitingStatus(StatusBase):
     """The unit or application is waiting on a charm it's integrated with.
 
@@ -2317,7 +2387,11 @@ def start(self, *service_names: str):
         self._pebble.start_services(service_names)
 
     def restart(self, *service_names: str):
-        """Restart the given service(s) by name."""
+        """Restart the given service(s) by name.
+
+        Listed running services will be stopped and restarted, and listed stopped
+        services will be started.
+        """
         if not service_names:
             raise TypeError('restart expected at least 1 argument, got 0')
 
@@ -3125,7 +3199,7 @@ def _format_action_result_dict(
     parent_key: Optional[str] = None,
     output: Optional[Dict[str, str]] = None,
 ) -> Dict[str, str]:
-    """Turn a nested dictionary into a flattened dictionary, using '.' as a key seperator.
+    """Turn a nested dictionary into a flattened dictionary, using '.' as a key separator.
 
     This is used to allow nested dictionaries to be translated into the dotted format required by
     the Juju `action-set` hook tool in order to set nested data on an action.
@@ -3664,7 +3738,8 @@ def secret_set(
         if expire is not None:
             args.extend(['--expire', expire.isoformat()])
         if rotate is not None:
-            args += ['--rotate', rotate.value]
+            args.extend(['--rotate', rotate.value])
+
         with tempfile.TemporaryDirectory() as tmp:
             # The content is None or has already been validated with Secret._validate_content
             for k, v in (content or {}).items():
@@ -3691,9 +3766,9 @@ def secret_add(
         if expire is not None:
             args.extend(['--expire', expire.isoformat()])
         if rotate is not None:
-            args += ['--rotate', rotate.value]
+            args.extend(['--rotate', rotate.value])
         if owner is not None:
-            args += ['--owner', owner]
+            args.extend(['--owner', owner])
         with tempfile.TemporaryDirectory() as tmp:
             # The content has already been validated with Secret._validate_content
             for k, v in content.items():
@@ -3965,7 +4040,7 @@ class CloudSpec:
     """A list of CA certificates."""
 
     skip_tls_verify: bool = False
-    """Whether to skip TLS verfication."""
+    """Whether to skip TLS verification."""
 
     is_controller_cloud: bool = False
     """If this is the cloud used by the controller, defaults to False."""
diff --git a/ops/pebble.py b/ops/pebble.py
index 785941566..5c642fd01 100644
--- a/ops/pebble.py
+++ b/ops/pebble.py
@@ -12,9 +12,21 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Client for the Pebble API (HTTP over Unix socket).
+"""Client for the Pebble API.
 
-For a command-line interface for local testing, see test/pebble_cli.py.
+This module provides a way to interact with Pebble, including:
+
+- :class:`~ops.pebble.Client`; communicates directly with the Pebble API.
+- :class:`~ops.pebble.Layer` class to define Pebble configuration layers,
+  including:
+
+  - :class:`~ops.pebble.Check` class to represent Pebble checks.
+  - :class:`~ops.pebble.LogTarget` class to represent Pebble log targets.
+  - :class:`~ops.pebble.Service` class to represent Pebble service descriptions.
+
+For a command-line interface for local testing, see ``test/pebble_cli.py``.
+
+  See more: `Pebble documentation <https://canonical-pebble.readthedocs-hosted.com/>`_
 """
 
 from __future__ import annotations
@@ -2187,6 +2199,9 @@ def restart_services(
     ) -> ChangeID:
         """Restart services by name and wait (poll) for them to be started.
 
+        Listed running services will be stopped and restarted, and listed stopped
+        services will be started.
+
         Args:
             services: Non-empty list of services to restart.
             timeout: Seconds before restart change is considered timed out (float). If
diff --git a/ops/testing.py b/ops/testing.py
index a633be3d3..e93ba7bcb 100644
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -11,8 +11,26 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+"""Framework for unit testing charms in a simulated Juju environment.
 
-"""Infrastructure to build unit tests for charms using the ops library."""
+The module includes:
+
+- :class:`ops.testing.Harness`, a class to set up the simulated environment,
+  that provides:
+
+  - :meth:`~ops.testing.Harness.add_relation` method, to declare a relation
+    (integration) with another app.
+  - :meth:`~ops.testing.Harness.begin` and :meth:`~ops.testing.Harness.cleanup`
+    methods to start and end the testing lifecycle.
+  - :meth:`~ops.testing.Harness.evaluate_status` method, which aggregates the
+    status of the charm after test interactions.
+  - :attr:`~ops.testing.Harness.model` attribute, which exposes e.g. the
+    :attr:`~ops.Model.unit` attribute for detailed assertions on the unit's state.
+
+.. note::
+    Unit testing is only one aspect of a comprehensive testing strategy. For more
+    on testing charms, see `Charm SDK | Testing <https://juju.is/docs/sdk/testing>`_.
+"""
 
 from ._private.harness import (
     ActionFailed,
diff --git a/pyproject.toml b/pyproject.toml
index 4bfa2d1db..237962926 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -224,3 +224,7 @@ reportUnnecessaryComparison = false
 reportUnnecessaryTypeIgnoreComment = "error"
 disableBytesTypePromotions = false
 stubPath = ""
+
+[tool.codespell]
+skip = './docs/_build,.venv,venv,build'
+quiet-level = 3
diff --git a/tox.ini b/tox.ini
index cf097a12f..505aa79bb 100644
--- a/tox.ini
+++ b/tox.ini
@@ -68,9 +68,11 @@ commands =
 description = Check code against coding style standards
 deps =
     ruff==0.4.5
+    codespell==2.3.0
 commands =
     ruff check --preview
     ruff format --preview --check
+    codespell
 
 [testenv:static]
 description = Run static type checker


Test patch:
diff --git a/test/test_charm.py b/test/test_charm.py
index 0939fbb6b..ecb9cb56e 100644
--- a/test/test_charm.py
+++ b/test/test_charm.py
@@ -797,6 +797,32 @@ def on_secret_expired(self, event: ops.SecretExpiredEvent):
     ]
 
 
+def test_secret_event_caches_secret_set(request: pytest.FixtureRequest, fake_script: FakeScript):
+    class MyCharm(ops.CharmBase):
+        def __init__(self, framework: ops.Framework):
+            super().__init__(framework)
+            self.secrets: typing.List[ops.Secret] = []
+            self.framework.observe(self.on.secret_changed, self.on_secret_changed)
+
+        def on_secret_changed(self, event: ops.SecretChangedEvent):
+            event.secret.set_info(description='desc')
+            event.secret.set_content({'key': 'value'})
+            self.secrets.append(event.secret)
+
+    fake_script.write('secret-get', """echo '{"key": "value"}'""")
+    fake_script.write('secret-set', 'exit 0')
+
+    framework = create_framework(request)
+    charm = MyCharm(framework)
+
+    charm.on.secret_changed.emit('secret:changed', None)
+    charm.on.secret_changed.emit('secret:changed', None)
+    cache = charm.secrets[0]._secret_set_cache
+    assert cache is charm.secrets[1]._secret_set_cache
+    assert charm.secrets[0]._secret_set_cache['secret:changed']['description'] == 'desc'
+    assert 'content' in cache['secret:changed']
+
+
 def test_collect_app_status_leader(request: pytest.FixtureRequest, fake_script: FakeScript):
     class MyCharm(ops.CharmBase):
         def __init__(self, framework: ops.Framework):
@@ -959,7 +985,6 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
         (['waiting', 'blocked'], 'blocked'),
         (['waiting', 'maintenance'], 'maintenance'),
         (['active', 'waiting'], 'waiting'),
-        (['active', 'unknown'], 'active'),
     ],
 )
 def test_collect_status_priority_valid(
@@ -994,6 +1019,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
     [
         ['blocked', 'error'],
         ['unknown'],
+        ['active', 'unknown'],
     ],
 )
 def test_collect_status_priority_invalid(
@@ -1015,7 +1041,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
 
     framework = create_framework(request)
     charm = MyCharm(framework, statuses=statuses)
-    with pytest.raises(ModelError):
+    with pytest.raises(ops.InvalidStatusError):
         ops.charm._evaluate_status(charm)
 
 
diff --git a/test/test_jujuversion.py b/test/test_jujuversion.py
index 713248bfa..fe2e457c0 100644
--- a/test/test_jujuversion.py
+++ b/test/test_jujuversion.py
@@ -100,6 +100,20 @@ def test_supports_exec_service_context():
     assert ops.JujuVersion('3.4.0').supports_exec_service_context
 
 
+@pytest.mark.parametrize(
+    'version,has_support',
+    [
+        ('2.9.50', False),
+        ('3.3.6', False),
+        ('3.4.0', True),
+        ('3.4.1', True),
+        ('4.0.0', True),
+    ],
+)
+def test_supports_pebble_log_forwarding(version: str, has_support: bool):
+    assert ops.JujuVersion(version).supports_pebble_log_forwarding is has_support
+
+
 @pytest.mark.parametrize(
     'invalid_version',
     [
diff --git a/test/test_model.py b/test/test_model.py
index 0eadd4d2c..7eb2ffafe 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -820,11 +820,19 @@ def test_base_status_instance_raises(self):
         with pytest.raises(TypeError):
             ops.StatusBase('test')
 
-        class NoNameStatus(ops.StatusBase):
-            pass
+        with pytest.raises(TypeError):
+            # TypeError due to missing `name` attribute
+            class NoNameStatus(ops.StatusBase):  # pyright: ignore[reportUnusedClass]
+                pass
 
         with pytest.raises(TypeError):
-            ops.StatusBase.register(NoNameStatus)
+            # TypeError due to non str type `name` attribute
+            class NonStringNameStatus(ops.StatusBase):  # pyright: ignore[reportUnusedClass]
+                name = None  # pyright: ignore[reportAssignmentType]
+
+    def test_base_status_register_is_deprecated(self):
+        with pytest.deprecated_call():
+            ops.StatusBase.register(ops.ActiveStatus)
 
     def test_status_repr(self):
         test_cases = {
@@ -3242,7 +3250,7 @@ def test_relation_remote_app_name_env(self, monkeypatch: pytest.MonkeyPatch):
         monkeypatch.setenv('JUJU_RELATION_ID', 'x:5')
         monkeypatch.setenv('JUJU_REMOTE_APP', 'remoteapp1')
         assert self.backend.relation_remote_app_name(5) == 'remoteapp1'
-        os.environ['JUJU_RELATION_ID'] = '5'
+        monkeypatch.setenv('JUJU_RELATION_ID', '5')
         assert self.backend.relation_remote_app_name(5) == 'remoteapp1'
 
     def test_relation_remote_app_name_script_success(
@@ -3266,7 +3274,7 @@ def test_relation_remote_app_name_script_success(
 
         # JUJU_RELATION_ID unset but JUJU_REMOTE_APP set
         monkeypatch.delenv('JUJU_RELATION_ID')
-        os.environ['JUJU_REMOTE_APP'] = 'remoteapp1'
+        monkeypatch.setenv('JUJU_REMOTE_APP', 'remoteapp1')
         assert self.backend.relation_remote_app_name(5) == 'remoteapp2'
 
         # Both set, but JUJU_RELATION_ID a different relation
@@ -3823,6 +3831,67 @@ def test_set_info(self, model: ops.Model, fake_script: FakeScript):
         with pytest.raises(TypeError):
             secret.set_info()  # no args provided
 
+    def test_set_content_then_info(self, model: ops.Model, fake_script: FakeScript):
+        fake_script.write('secret-set', """exit 0""")
+        fake_script.write('secret-get', """echo '{"foo": "bar"}'""")
+
+        secret = self.make_secret(model, id='q')
+        secret.set_content({'foo': 'bar'})
+        description = 'desc'
+        secret.set_info(description=description)
+
+        calls = fake_script.calls(clear=True)
+        assert calls[0][:-1] == ['secret-set', f'secret://{model._backend.model_uuid}/q']
+        assert calls[0][-1].startswith('foo#file=') and calls[0][-1].endswith('/foo')
+        assert calls[1] == [
+            'secret-get',
+            f'secret://{model._backend.model_uuid}/q',
+            '--peek',
+            '--format=json',
+        ]
+        assert calls[2][:-1] == [
+            'secret-set',
+            f'secret://{model._backend.model_uuid}/q',
+            '--description',
+            description,
+        ]
+        assert calls[2][-1].startswith('foo#file=') and calls[0][-1].endswith('/foo')
+
+    def test_set_info_then_content(self, model: ops.Model, fake_script: FakeScript):
+        fake_script.write('secret-set', """exit 0""")
+
+        secret = self.make_secret(model, id='q')
+        description = 'desc'
+        secret.set_info(description=description)
+        secret.set_content({'foo': 'bar'})
+
+        calls = fake_script.calls(clear=True)
+        assert calls[0] == [
+            'secret-set',
+            f'secret://{model._backend.model_uuid}/q',
+            '--description',
+            description,
+        ]
+        assert calls[1][:-1] == [
+            'secret-set',
+            f'secret://{model._backend.model_uuid}/q',
+            '--description',
+            description,
+        ]
+        assert calls[1][-1].startswith('foo#file=') and calls[1][-1].endswith('/foo')
+
+    def test_set_content_aggregates(self, model: ops.Model, fake_script: FakeScript):
+        fake_script.write('secret-set', """exit 0""")
+
+        secret = self.make_secret(model, id='q')
+        secret.set_content({'foo': 'bar'})
+        secret.set_content({'baz': 'qux', 'foo': 'newbar'})
+
+        calls = fake_script.calls(clear=True)
+        assert calls[0][:-1] == ['secret-set', f'secret://{model._backend.model_uuid}/q']
+        assert calls[0][:-1] == ['secret-set', f'secret://{model._backend.model_uuid}/q']
+        assert fake_script.secrets() == {'foo': 'newbar', 'baz': 'qux'}
+
     def test_grant(self, model: ops.Model, fake_script: FakeScript):
         fake_script.write('relation-list', """echo '[]'""")
         fake_script.write('secret-grant', """exit 0""")
diff --git a/test/test_testing.py b/test/test_testing.py
index 4c88f232b..965ebc894 100644
--- a/test/test_testing.py
+++ b/test/test_testing.py
@@ -3232,6 +3232,9 @@ def test_get_filesystem_root(self):
 
     def test_evaluate_status(self):
         class TestCharm(ops.CharmBase):
+            app_status_to_add: ops.StatusBase
+            unit_status_to_add: ops.StatusBase
+
             def __init__(self, framework: ops.Framework):
                 super().__init__(framework)
                 self.framework.observe(self.on.collect_app_status, self._on_collect_app_status)