feat: refuse system-wide partition writes while holds exist

docs/adr/0327-hold-store-schema-and-hold-id-plumbing.md

@@ -2,6 +2,9 @@
 
 ## Status
 
+> **Decision 5 superseded by [0343](0343-system-write-hold-fence.md)** (2026-11-04),
+> for the six system-wide partition-write tools only.
+
 Accepted (2026-11-03), issue #1851 (piece 1 of #1231). Implements ADR 0193 Decisions 1, 2, 5
 and 7 on the ADR 0190 store; ADR 0193's prepare/release tool is #1852.
 

docs/adr/0343-system-write-hold-fence.md

@@ -0,0 +1,49 @@
+# 0343 — System-wide partition writes refuse while a system has holds
+
+## Status
+
+Accepted (2026-11-04), issue #1887. Supersedes ADR 0327 Decision 5 only for the
+six system-wide tools enumerated in the accompanying design.
+
+## Context
+
+ADR 0327 leaves system-scoped writes outside the partition-resolution hook. A held
+partition can therefore be stopped or reconfigured through its managed system. The
+operator selected "Refuse while any system hold exists" and accepted releasing holds
+before these writes, without an override or multi-token authorization.
+
+## Decision
+
+Mark `hmc_modify_system`, `hmc_power_on_system`, `hmc_power_off_system`,
+`hmc_activate_system_profile`, `hmc_restore_lpar_profiles` and `hmc_update_firmware`
+as system-wide partition writes in the registry. Their existing managed-system UUID
+resolution checks the local hold store through the front-end-installed guard. One hold
+on that system refuses the call with `system_held`, regardless of caller or hold ID;
+unknown store state also refuses. Holds on another system do not block it.
+
+MCP dispatch, including invoke, and existing CLI mirrors carry the marker centrally.
+This is a whole-tool rule, including rename-only or validate-only argument variants.
+No system hold token or override is introduced. Keep partition-token behavior unchanged.
+Update handoff disclosure and consumer docs. Other ADR 0327 decisions remain in force.
+
+## Consequences
+
+Holders must release all holds on the system before these operations, accepting the
+loss of their hold protection while doing so. Existing ownership/access-policy checks
+still apply. The check has ADR 0193's resolver-time semantics: it does not lock the HMC
+job against a hold placed after admission, and does not fence library/raw/other-host
+writers. No store migration or dependency is needed. Resource-only and creation tools
+remain outside the six-tool set; the design records the complete current inventory.
+
+## Considered & rejected
+
+- **Keep system writes unchecked.** judgment: fit. It leaves the held-partition hazard
+  in issue #1887 despite the operator's chosen refusal policy.
+- **Accept every hold ID or an override.** judgment: complexity and fit. It adds an
+  authorization interface the operator expressly declined.
+- **Check separately in each handler and CLI command.** judgment: complexity. One
+  registry marker and shared resolver keep policy and mirror coverage in the existing owners.
+- **Fence every non-read managed-system tool.** judgment: fit. That broadens the rule
+  to new-partition creation and unrelated resource records beyond the chosen boundary.
+- **Condition the check on individual arguments.** judgment: complexity. It creates
+  exceptions within tools whose safe whole-tool refusal the operator accepted.

docs/kdive-tier-a-contract.md

@@ -122,6 +122,12 @@ declared, and what kdive's doctor/adopt still has to check
   with `partition_held` unless the call presents the matching `hold_id`, whatever
   `authorize_power_operations` is set to. Console capture (`hmc_capture_lpar_console`) and
   reads are not checked.
+- **System-wide partition writes require release first.** System modification, power on/off,
+  system-profile activation, profile restore and firmware update refuse `system_held` while
+  any partition on that system is held ([ADR 0343](adr/0343-system-write-hold-fence.md)).
+  These whole-tool checks have no token or ownership override, including validate-only
+  activation and rename-only modification. Existing CLI mirrors enforce the same rule.
+  Coordinate handoff with already-admitted system operations; this check does not lock a job.
 - **Library calls are not checked.** The hold binds the MCP server and CLI on the host that
   placed it. The library operations above, hmcpctl on other hosts, the HMC GUI and direct
   HMC commands are not fenced, nor is any tool the hold hook does not check; the document's

docs/mcp-server.md

@@ -567,12 +567,20 @@ hold ends `failed`, and a repeat of its `request_id` returns that record, so ret
 `request_id`. Reads, `hmc_capture_lpar_console` and the handoff tool
 are not refused. A store that cannot be read refuses these calls too.
 
-A hold binds only this host's front ends. The library API, other hosts, the HMC GUI and direct
-HMC commands are not fenced, and neither are tools that never resolve a partition: system-scoped
-writes such as `hmc_restore_lpar_profiles`, system power and system profile activation, and
-`hmc_run_command` and `hmcpctl raw` (ADR 0327). While any hold exists, a mutation that names its
-partition only by name over SSH resolves the name through REST first; when REST cannot identify
-it, the call is refused with `partition_hold_unverified`.
+System-wide partition writes also consult the hold store (ADR 0343): `hmc_modify_system`,
+`hmc_power_on_system`, `hmc_power_off_system`, `hmc_activate_system_profile`,
+`hmc_restore_lpar_profiles` and `hmc_update_firmware` refuse with `system_held` when any
+partition on the selected system is held. Their existing CLI mirrors enforce the same rule.
+Release all holds on that system first; neither a matching `hold_id` nor ownership override
+bypasses it. The entire tool is covered, including rename-only modification and validate-only
+profile activation. This is a check at system resolution, not a lock spanning an HMC job:
+coordinate handoff with operations already admitted before the hold was placed.
+
+A hold binds only this host's front ends. The library API, other hosts, the HMC GUI, direct
+HMC commands, `hmc_run_command` and `hmcpctl raw` remain unfenced. Other unchecked tools are
+listed in the handoff document. While any hold exists, a mutation that names its partition
+only by name over SSH resolves the name through REST first; when REST cannot identify it,
+the call is refused with `partition_hold_unverified`.
 
 `hmc_prepare_host_handoff` places and releases holds (ADR 0329). `prepare` with `hold` true and a
 `consumer_label` places the partition's hold and returns its `hold_id` only to the placing agent

docs/workflow/specs/2026-11-04-system-write-hold-fence-design.md

@@ -0,0 +1,151 @@
+# System-wide writes respect external-management holds
+
+Issue #1887; scope token `q1887-92586ba1`; full-spec, M/250, iterating.
+Decision: [ADR 0343](../../adr/0343-system-write-hold-fence.md).
+Source baseline: `0dcb93630a66efcae7aed85d3265ba85f8289c58`.
+
+## Problem and scope
+
+The partition hook cannot see writes that resolve only a managed system. The operator chose
+"Refuse while any system hold exists": holders release first, without an override or a set
+of tokens. This extends the existing front-end resolver-time check to the six tools below.
+It does not turn the local store into a distributed lock or fence library/raw HMC writers.
+
+### Inventory and affected callers
+
+The non-read `TOOL_SECURITY` records with `target_kind == "managed_system"` contain 23 tools.
+Names in this table have the `hmc_` prefix. The checked set is exactly these six:
+
+| Tool | Existing operation owner and effect | CLI mirror |
+| --- | --- | --- |
+| modify_system | operations/systems/core.py: modify_system writes platform configuration, including LPAR start policy | none |
+| power_on_system | operations/systems/core.py: power_system may auto-start partitions | systems power-on |
+| power_off_system | operations/systems/core.py: power_system stops the platform and its partitions | systems power-off |
+| activate_system_profile | operations/systems/profiles.py: activate_system_profile starts contained members | systems profile-activate |
+| restore_lpar_profiles | operations/lpar/configuration.py: restore_system_lpar_profiles rewrites partition profiles | none |
+| update_firmware | operations/updates/service.py: update_firmware submits platform/VIOS/I/O update work | none |
+
+These operation paths call `resource_identity.resolve_system_uuid` before their first
+mutation. Restore reaches it through `operations/lpar/ownership.py` before ownership override.
+Firmware's version read precedes it; profile activation's argument validation precedes it.
+The marker applies to the whole tool: even rename-only modify and validate-only activation
+refuse on a held system. This deliberately avoids argument-dependent exceptions.
+
+The other 17 records are classified by the resources their current code writes:
+
+- create_system_profile, update_system_profile, delete_system_profile change system-profile
+  container membership/metadata, not the contained partition profiles or running state
+  (`operations/systems/profiles.py`, `_metadata`).
+- create_lpar, create_vios, deploy_partition_template create new partitions; their creation
+  paths do not rewrite an existing held partition (`operations/lpar/workflows.py`,
+  `operations/vios/core.py`, `operations/templates/core.py`). Resource competition remains outside
+  the requested existing-partition fence.
+- set_vios_fc_port_label, remove_vios_fc_port_label, create_vios_vfc_group_label,
+  update_vios_vfc_group_label, remove_vios_vfc_group_label edit label resources, not partition
+  configuration (`server_tools/vios/labels.py`).
+- create_virtual_network, delete_virtual_network change virtual-network resources
+  (`server_tools/virtualization/network.py`); remove_memory_pool removes an unused pool only
+  after dedicated-memory checks (`ssh/memory.py`). Shared-resource protection is not added.
+- set_sriov_adapter_mode currently verifies already-selected mode and refuses transitions
+  (`server_tools/virtualization/pcie.py`); it submits no admitted mode change.
+- backup_lpar_profiles and delete_backup_lpar_profile change backup artifacts, not current
+  partition profiles (`server_tools/lpar/profiles.py`).
+
+The existing partition fence remains unchanged. Direct HMC/raw commands and library fencing
+are excluded under ADR 0193 D6; protection beyond system tools affecting partitions belongs
+to the operator. The inventory explains the boundary instead of calling all system tools safe.
+
+## Architecture and behavior
+
+`ToolSecurity.system_partition_write: bool = False` records this classification explicitly
+in the decorator. Reject a true marker on a read tool, a non-managed-system target, or a
+record lacking the required managed-system selector. Add the marker to the six declarations.
+Keep `hold_guarded` as the partition-token predicate: the system marker does not inject
+`hold_id` into schemas or `--hold-id` into CLI help.
+
+Copy the marker into `CallContext` in `authorized()` and `CLICommand.invoke`, using the existing
+mirror table. Nested `hmc_invoke_tool` dispatch gets the selected tool's marker. Existing
+context restoration and worker propagation continue to apply. No handler signature changes.
+
+Extend the installed `PartitionGuard` with an optional system-check callback; the existing
+server/CLI `HOLD_GUARD` supplies it and library code installs none. After resolving a name or
+accepting a UUID, `resolve_system_uuid` calls that callback before returning. There is one
+check path for name, lowercase UUID and uppercase UUID. Existing two-argument guard fixtures
+remain valid because the new callback defaults to absent.
+
+The callback returns immediately unless the current call carries the system marker. For a
+marked call it verifies canonical UUID shape, lowercases the lookup key, and uses the existing
+non-creating `store.read_session` lifecycle to select one hold on that system. Holds on other
+systems and an absent store permit the call. An unreadable/corrupt/lost store propagates its
+existing refusal; it never becomes an empty hold result. No schema or state migration is needed.
+
+A matching hold raises `HoldRefused` with reason `system_held`, naming the system and one held
+partition, advising `hmcpctl holds list` and release before retry. It includes no hold ID.
+The placing agent, matching partition token, ownership override and power-operation setting
+cannot waive this refusal. Hold release remains the existing explicit operation.
+
+`server_tools/lpar/handoff.unchecked_tools` excludes both partition-guarded tools and marked
+system tools. Update `operations/lpar/handoff.UNFENCED`, MCP docs and the kdive contract to
+name the system rule and retain the library/raw/other-host exclusions. No handoff field is
+added or removed. ADR 0327 gets only a Status banner identifying D5's partial supersession.
+
+## Success
+
+- C1: the six-tool checked set and the other 17 system records are classified above; ADR 0343
+  records the policy and tradeoffs and ADR 0327's body remains unchanged.
+- C2: each of the six tools refuses `system_held` before its HMC mutation when its system has
+  at least one hold at the resolver check, including a hold placed by the calling agent.
+  The same tools can pass this check for an unheld system; all holds must be released first.
+- C3: direct MCP and invoke dispatch and the three existing CLI mirrors agree; no system
+  `hold_id` interface appears; handoff disclosure removes exactly the newly checked tools.
+- C4: unknown local store state fails closed for the checked set, reads and library calls keep
+  their prior behavior, and focused tests plus final guardrails pass without live HMC access.
+
+## Failure model
+
+1. **Actors and deployments**
+   - MCP callers and CLI operators on one host sharing its configured ADR 0190 store.
+   - Concurrent local processes may read/place/release holds; existing resolver-time admission.
+2. **Invariants and assets at stake**
+   - A hold observed on the selected system blocks the six declared system tools before write.
+   - Other systems, reads, library callers and existing partition-token semantics retain behavior.
+   - Unknown store state cannot authorize a marked write; refusal text reveals no hold ID.
+3. **Accepted failure classes**
+   - A hold placed after the resolver snapshot does not cancel an already-admitted operation;
+     the existing ADR 0193 hook is a pre-write check, not a lock spanning an HMC job. Operators
+     coordinate handoff with in-flight system operations; no new transaction/job lock is added.
+   - Conservative lockout of non-mutating argument variants within the six marked tools is
+     bounded to those tools and ends after explicit hold release, matching the operator choice.
+4. **Covered elsewhere**
+   - Library, other hosts, GUI and raw HMC writers: ADR 0193 D6, consumer/operator coordination.
+   - Shared-resource competition and tools outside the six-tool set: operator responsibility.
+   - Store permissions/corruption/schema handling: ADR 0190; partition hold IDs: ADR 0193/0327.
+
+## Threat model
+
+- **Boundary inventory:** existing MCP/CLI arguments reach registry context, then resolved
+  system UUID reaches the local hold store. No new remote input, credential or database schema.
+- **Actors:** callers can choose selectors, tokens and ownership flags; they cannot set internal
+  registry markers through public arguments. Local credential holders control store/release.
+- **Controls:** registry shape validation and six-tool coverage test; existing access-policy
+  authorization before handler; authoritative UUID resolution and canonical shape checking;
+  parameterized system lookup and existing store validation; no token override or token leakage.
+- **Out of scope:** hostile local store owner and independent HMC writers already control the
+  excluded surfaces. This does not provide protection against them or span HMC job execution.
+
+## Global constraints
+
+Python 3.11 through 3.14; CI target amd64; local host x86_64. No new dependency or facade export.
+Use `just setup`; never bare `uv sync` or `uv run`. Use private TMPDIR outside the worktree.
+No live HMC access. Final candidate requires `just verify` and `uv run --no-sync prek run --all-files`.
+
+## Validation
+
+Use real temporary hold stores and mocked HMC/SSH boundaries. Add registry-driven composed
+MCP tests for the six tools by name/lowercase UUID/uppercase UUID, directly and via invoke;
+assert `system_held` and zero HMC writes/SSH mutations. Exercise the three CLI mirrors.
+Store tests cover multiple holds, release of one versus last, cross-system isolation, matching
+hold ID/agent non-bypass, missing store, corrupt/lost store and malformed resolved UUID.
+Keep positive paths for reads, unheld systems, library with no installed guard, and unchanged
+partition-token schema behavior. Test marker validation and handoff registry coverage.
+No synthetic prose tests: doc changes are reviewed; regenerate generated tool docs if needed.

src/hmcpctl/call_context.py

@@ -1,7 +1,8 @@
 """The front-end call a partition resolution belongs to (ADR 0193 Decision 2, ADR 0327).
 
 The MCP tool wrapper and each CLI command record which registry tool is running, whether it
-is a read, and any presented ``hold_id``. The hold hook reads it; nothing else does. A
+is a read, any presented ``hold_id``, and the system-wide partition-write marker (ADR 0339).
+The hold hook reads it; nothing else does. A
 resolution with no call context is treated as a mutation presenting no ``hold_id``.
 """
 
@@ -23,6 +24,7 @@ class CallContext:
     tool: str | None
     read: bool
     hold_id: str | None
+    system_partition_write: bool = False
 
 
 _CURRENT: ContextVar[CallContext | None] = ContextVar("hmcpctl_call", default=None)

src/hmcpctl/cli_commands/app.py

@@ -70,7 +70,15 @@ def invoke(self, ctx: _click.Context) -> Any:
         tool = _mirrored_tool(ctx)
         hold_id = ctx.params.pop(_HOLD_ID_OPTION, None)
         read = tool is not None and TOOL_SECURITY[tool].effect == "read"
-        with calling(CallContext(tool=tool, read=read, hold_id=hold_id)):
+        with calling(
+            CallContext(
+                tool=tool,
+                read=read,
+                hold_id=hold_id,
+                system_partition_write=tool is not None
+                and TOOL_SECURITY[tool].system_partition_write,
+            )
+        ):
             return super().invoke(ctx)
 
     def parse_args(self, ctx: _click.Context, args: list[str]) -> list[str]:

src/hmcpctl/operations/logical/holds.py

@@ -16,7 +16,7 @@
 from dataclasses import dataclass
 
 from ...call_context import HOLD_EXEMPT_TOOLS, current_call
-from ...resource_identity import PartitionGuard
+from ...resource_identity import PartitionGuard, is_uuid
 from . import store
 from .store import OperationRefused
 
@@ -272,6 +272,37 @@ def enforce_hold(partition_uuid: str) -> None:
     )
 
 
+def enforce_system_hold(system_uuid: str) -> None:
+    """Refuse marked system writes while a partition on that system is held (ADR 0339)."""
+    call = current_call()
+    if call is None or not call.system_partition_write:
+        return
+    if not is_uuid(system_uuid):
+        raise HoldRefused(
+            "system_hold_unverified",
+            "managed-system identity is not a UUID; retry after inventory recovers",
+        )
+    if not _locatable():
+        return
+    with store.read_session() as conn:
+        row = (
+            None
+            if conn is None
+            else conn.execute(
+                "SELECT partition_uuid FROM holds WHERE system_uuid = ? ORDER BY partition_uuid LIMIT 1",
+                (system_uuid.lower(),),
+            ).fetchone()
+        )
+    if row is not None:
+        raise HoldRefused(
+            "system_held",
+            f"system {system_uuid} has a hold on partition {row['partition_uuid']}; "
+            "use hmcpctl holds list and hmcpctl holds release to release its holds before retrying",
+        )
+
+
 HOLD_GUARD = PartitionGuard(
-    check=enforce_hold, names_need_resolution=_names_need_resolution
+    check=enforce_hold,
+    names_need_resolution=_names_need_resolution,
+    check_system=enforce_system_hold,
 )